diff --git a/bridged/src/main/java/dev/ltms/bridged/Bridged.java b/bridged/src/main/java/dev/ltms/bridged/Bridged.java index 412a61b..7fe2186 100644 --- a/bridged/src/main/java/dev/ltms/bridged/Bridged.java +++ b/bridged/src/main/java/dev/ltms/bridged/Bridged.java @@ -83,6 +83,10 @@ public final class Bridged { static void main(String[] args) { Path configPath = Path.of(args.length > 0 ? args[0] : "bridged.yaml"); BridgedConfig cfg = BridgedConfig.load(configPath); + // CB-594: report which secret env vars the config actually needs, by name, before anything + // else can fail on a silently-empty one. A daemon started without a login shell (launchd) + // boots fine either way — this is the only thing that says so out loud. + reportRequiredSecrets(cfg); // CB-559: `cfg` stays the startup snapshot — every validation and every piece of one-time // wiring below reads it, and must, because those decisions cannot be unmade. `config` is the // live reference the hot paths read per use. Which keys can actually move is ConfigRef's @@ -543,6 +547,63 @@ public final class Bridged { return target -> presence.isPresent(target) || leads.get().containsKey(target); } + /** + * CB-594: which env vars the loaded config actually needs, and why — every non-{@code + * subscription} profile's {@code tokenEnv} (a subscription profile never reads one, see + * {@link BridgedConfig.Profile#isSubscription()}), plus every profile's {@code gitTokenEnv} + * where set (opt-in). Derived from the config, not hard-coded, so a new profile is covered for + * free. A var required by more than one profile is one entry naming every profile that needs + * it. Deliberately excludes {@code auth.tokenEnv}: that one is already enforced loudly, by a + * startup throw, a few lines above this method's call site. + * + *

Package-private and pure (no I/O, no logging) so the derivation is unit-testable without + * capturing log output; {@link #reportRequiredSecrets(BridgedConfig)} is the logging caller. + */ + static Map> requiredSecretEnvVars(BridgedConfig cfg) { + Map> requiredBy = new LinkedHashMap<>(); + cfg.profiles().forEach((name, profile) -> { + if (!profile.isSubscription()) { + requiredBy.computeIfAbsent(profile.tokenEnv(), _ -> new ArrayList<>()) + .add("profile '" + name + "' tokenEnv"); + } + if (profile.hasGitToken()) { + requiredBy.computeIfAbsent(profile.gitTokenEnv(), _ -> new ArrayList<>()) + .add("profile '" + name + "' gitTokenEnv"); + } + }); + return requiredBy; + } + + /** + * CB-594: log, by name only, which required env vars (see {@link #requiredSecretEnvVars}) are + * set in the daemon's own process environment — the environment every profile's {@code + * tokenEnv}/{@code gitTokenEnv} is read from at spawn time (see + * {@code HerdrPeerLauncher.resolveEnv}). Never logs a value, a prefix, or a length. + * + *

A missing entry only warns — it must never refuse to start. A daemon that boots and says + * what is wrong is strictly more useful than one that will not boot at all. + */ + private static void reportRequiredSecrets(BridgedConfig cfg) { + Map> requiredBy = requiredSecretEnvVars(cfg); + if (requiredBy.isEmpty()) { + log.info("startup secrets: no profile references a token env var — nothing to check"); + return; + } + Map env = System.getenv(); + requiredBy.forEach((varName, sources) -> { + String value = env.get(varName); + if (value != null && !value.isBlank()) { + log.info("startup secret {}: set ({})", varName, String.join(", ", sources)); + } else { + log.warn("startup secret {}: MISSING ({}) — the daemon will start anyway, and this " + + "failure stays invisible until a worker actually needs it. Fix " + + "${SHARED_ENV}/tools/secrets.sh and restart bridged from a LOGIN " + + "shell (see scripts/redeploy-bridged.sh).", + varName, String.join(", ", sources)); + } + }); + } + /** * Poll herdr's {@code ping} until it answers or {@link #HERDR_WAIT_SECONDS} elapses (CB-504). * diff --git a/bridged/src/test/java/dev/ltms/bridged/RequiredSecretEnvVarsTest.java b/bridged/src/test/java/dev/ltms/bridged/RequiredSecretEnvVarsTest.java new file mode 100644 index 0000000..4ce7ca9 --- /dev/null +++ b/bridged/src/test/java/dev/ltms/bridged/RequiredSecretEnvVarsTest.java @@ -0,0 +1,114 @@ +package dev.ltms.bridged; + +import dev.ltms.bridged.config.BridgedConfig; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * CB-594: {@link Bridged#requiredSecretEnvVars(BridgedConfig)} is what decides what the startup + * secret report checks — it must derive that set from the config, not a hand-written list, or a + * new profile's token silently stops being reported. + */ +class RequiredSecretEnvVarsTest { + + private static BridgedConfig load(Path dir, String yaml) throws Exception { + Path f = dir.resolve("bridged.yaml"); + Files.writeString(f, yaml); + return BridgedConfig.load(f); + } + + @Test + void collectsATokenEnvPerNonSubscriptionProfile(@TempDir Path dir) throws Exception { + BridgedConfig cfg = load(dir, """ + profiles: + local: + baseUrl: http://gx00.gw:8000 + tokenEnv: AI_GATEWAY_TOKEN + """); + + Map> required = Bridged.requiredSecretEnvVars(cfg); + + assertTrue(required.containsKey("AI_GATEWAY_TOKEN")); + assertEquals(List.of("profile 'local' tokenEnv"), required.get("AI_GATEWAY_TOKEN")); + } + + @Test + void aSubscriptionProfileNeedsNoTokenEnv(@TempDir Path dir) throws Exception { + BridgedConfig cfg = load(dir, """ + profiles: + opus: + subscription: true + model: claude-opus-5 + """); + + assertTrue(Bridged.requiredSecretEnvVars(cfg).isEmpty(), + "subscription: true never reads ANTHROPIC_AUTH_TOKEN — see Profile#isSubscription"); + } + + @Test + void gitTokenEnvIsOptInAndCollectedWhenSet(@TempDir Path dir) throws Exception { + BridgedConfig cfg = load(dir, """ + profiles: + local: + baseUrl: http://gx00.gw:8000 + tokenEnv: AI_GATEWAY_TOKEN + gitTokenEnv: WORKER_GITEA_TOKEN + """); + + Map> required = Bridged.requiredSecretEnvVars(cfg); + + assertTrue(required.containsKey("WORKER_GITEA_TOKEN")); + assertEquals(List.of("profile 'local' gitTokenEnv"), required.get("WORKER_GITEA_TOKEN")); + } + + @Test + void noGitTokenEnvMeansNothingIsRequiredForIt(@TempDir Path dir) throws Exception { + BridgedConfig cfg = load(dir, """ + profiles: + local: + baseUrl: http://gx00.gw:8000 + tokenEnv: AI_GATEWAY_TOKEN + """); + + assertFalse(Bridged.requiredSecretEnvVars(cfg).containsKey("WORKER_GITEA_TOKEN")); + } + + @Test + void aVarSharedByTwoProfilesIsReportedOnceNamingBoth(@TempDir Path dir) throws Exception { + BridgedConfig cfg = load(dir, """ + profiles: + local: + baseUrl: http://gx00.gw:8000 + tokenEnv: AI_GATEWAY_TOKEN + gitTokenEnv: WORKER_GITEA_TOKEN + gx: + kind: opencode + baseUrl: https://llm.ltms.dev/v1 + tokenEnv: AI_GATEWAY_TOKEN + gitTokenEnv: WORKER_GITEA_TOKEN + """); + + Map> required = Bridged.requiredSecretEnvVars(cfg); + + assertEquals(List.of("profile 'local' tokenEnv", "profile 'gx' tokenEnv"), + required.get("AI_GATEWAY_TOKEN")); + assertEquals(List.of("profile 'local' gitTokenEnv", "profile 'gx' gitTokenEnv"), + required.get("WORKER_GITEA_TOKEN")); + } + + @Test + void noProfilesMeansNothingIsRequired(@TempDir Path dir) throws Exception { + BridgedConfig cfg = load(dir, "bind:\n host: 127.0.0.1\n port: 8765\n"); + + assertTrue(Bridged.requiredSecretEnvVars(cfg).isEmpty()); + } +} diff --git a/deploy/dev.ltms.bridged.plist b/deploy/dev.ltms.bridged.plist index 8fcee44..073e138 100644 --- a/deploy/dev.ltms.bridged.plist +++ b/deploy/dev.ltms.bridged.plist @@ -1,7 +1,7 @@ @@ -25,22 +44,23 @@ ProgramArguments - /Users/CHANGEME/Tool/jdk-25.0.2.jdk/Contents/Home/bin/java + /Users/dai.ha/LTMS/claude-bridge/scripts/bridged-launchd-wrapper.sh + /Users/dai.ha/Softwares/jdks/jdk-25.0.3.jdk/Contents/Home/bin/java -jar - /Users/CHANGEME/src/claude-bridge/bridged/target/bridged.jar + /Users/dai.ha/LTMS/claude-bridge/bridged/target/bridged.jar bridged.yaml WorkingDirectory - /Users/CHANGEME/src/claude-bridge/bridged + /Users/dai.ha/LTMS/claude-bridge/bridged EnvironmentVariables JAVA_HOME - /Users/CHANGEME/Tool/jdk-25.0.2.jdk/Contents/Home + /Users/dai.ha/Softwares/jdks/jdk-25.0.3.jdk/Contents/Home HERDR_SOCKET_PATH - /Users/CHANGEME/.config/herdr/herdr.sock + /Users/dai.ha/.config/herdr/herdr.sock PATH - /Users/CHANGEME/Tool/jdk-25.0.2.jdk/Contents/Home/bin:/Users/CHANGEME/Tool/apache-maven-3.9.16/bin:/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin + /Users/dai.ha/Softwares/jdks/jdk-25.0.3.jdk/Contents/Home/bin:/Users/dai.ha/Softwares/apache-maven/bin:/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin @@ -69,10 +92,17 @@ ThrottleInterval 10 + StandardOutPath - /Users/CHANGEME/src/claude-bridge/bridged/logs/bridged.out.log + /Users/dai.ha/LTMS/claude-bridge/bridged/bridged.out StandardErrorPath - /Users/CHANGEME/src/claude-bridge/bridged/logs/bridged.err.log + /Users/dai.ha/LTMS/claude-bridge/bridged/bridged.out ProcessType Background diff --git a/scripts/bridged-launchd-wrapper.sh b/scripts/bridged-launchd-wrapper.sh new file mode 100755 index 0000000..3c54527 --- /dev/null +++ b/scripts/bridged-launchd-wrapper.sh @@ -0,0 +1,32 @@ +#!/usr/bin/env bash +# +# CB-594 — the only reason this file exists: launchd does not run a login shell. +# +# WORKER_GITEA_TOKEN and AI_GATEWAY_TOKEN live in ${SHARED_ENV}/tools/secrets.sh, sourced only by a +# LOGIN shell (.zprofile/.zshrc etc). launchd execs a job's ProgramArguments directly — no shell, no +# profile, nothing sourced (the plist's own PATH comment documents the same gap one variable over). +# A daemon started that way boots fine and looks healthy; the failure is invisible until a worker +# tries to open a PR (WORKER_GITEA_TOKEN empty) or a gateway profile gets a 401 (AI_GATEWAY_TOKEN +# empty) — hours later, with nothing tying the two together (CB-591, CLAUDE.md "Redeploying the +# daemon"). Bridged now also logs which required secret names resolved at startup (see +# Bridged.reportRequiredSecrets), but that log line can only tell the truth if the tokens had a +# chance to be sourced in the first place — which is this script's entire job. +# +# So: launchd execs THIS script instead of java directly. This script execs a login shell +# ('zsh -l'), which sources secrets.sh, and that shell execs the real command in its place — one +# process throughout (exec, not a subshell fork), so launchd's PID tracking, KeepAlive, and +# StandardOut/ErrorPath all still see the one process they expect. +# +# The plist passes the full command as THIS script's own arguments, e.g.: +# ProgramArguments = [ .../bridged-launchd-wrapper.sh, /path/to/java, -jar, /path/to/bridged.jar, +# bridged.yaml ] +# so the wrapper stays generic and the actual command lives in exactly one place (the plist), not +# duplicated here. +set -euo pipefail + +if [ "$#" -eq 0 ]; then + echo "bridged-launchd-wrapper.sh: no command given — check the plist's ProgramArguments" >&2 + exit 2 +fi + +exec /bin/zsh -lc 'exec "$@"' -- "$@" diff --git a/scripts/redeploy-bridged.sh b/scripts/redeploy-bridged.sh index b23b519..a5622e7 100755 --- a/scripts/redeploy-bridged.sh +++ b/scripts/redeploy-bridged.sh @@ -5,13 +5,14 @@ # A merge is not a deployment: the running daemon holds the jar it was started with, so code merged # to main does nothing until this runs. See CLAUDE.md -> "Redeploying the daemon". # -# This script exists to turn five remembered traps into one auditable command: +# This script exists to turn six remembered traps into one auditable command: # # 1. A piped `mvn` hides BUILD FAILURE behind a zero exit, so the build here is never piped. # 2. The daemon must start from a LOGIN shell, or the tokens it hands to members are empty: # WORKER_GITEA_TOKEN (workers cannot open a PR) and AI_GATEWAY_TOKEN (401 at llm.ltms.dev). # Both are read from the DAEMON's own environment at spawn time, so a value added to -# secrets.sh after startup is absent. Nothing logs this, so the script checks and says so. +# secrets.sh after startup is absent. Nothing logs this here, so the script checks and says +# so — and since CB-594, bridged's own startup log says so too, by env var name. # 3. An old daemon that never actually died looks identical from the outside, so the script waits # for the process to exit and for the port to free before it starts a new one. # 4. "It started" is not "it works": the script polls /healthz until it answers, and reports the @@ -19,6 +20,13 @@ # mismatch. # 5. Restarting under live members drops their tickets, so the script refuses unless you confirm # the fleet is drained. +# 6. CB-594 — the launchd agent (deploy/dev.ltms.bridged.plist), if installed and loaded, is a +# SECOND supervisor: its KeepAlive.SuccessfulExit=false restarts the daemon on any nonzero +# exit, and a bare SIGTERM makes this JVM exit 143 even with its shutdown hook running to +# completion (measured — see the CB-594 report). A plain `kill` here would race launchd's own +# restart of the OLD jar. So this script detects whether the agent is loaded and, only then, +# swaps `kill` + manual `nohup` for `launchctl unload`/`load` — the one supervisor in control +# at any moment is whichever one you asked to act, never both. # # Usage: # scripts/redeploy-bridged.sh # build, confirm, restart, verify @@ -43,13 +51,17 @@ HEALTH='http://127.0.0.1:8765/healthz' STOP_WAIT=30 # seconds to wait for a clean exit before reporting failure HEALTH_WAIT=60 # seconds to wait for /healthz to answer after start +# CB-594: the launchd agent this script must not fight with (see trap 6 above). +LAUNCHD_LABEL='dev.ltms.bridged' +LAUNCHD_PLIST="$HOME/Library/LaunchAgents/$LAUNCHD_LABEL.plist" + DO_BUILD=1; ASSUME_YES=0; CHECK_ONLY=0 for arg in "$@"; do case "$arg" in --yes|-y) ASSUME_YES=1 ;; --no-build) DO_BUILD=0 ;; --check) CHECK_ONLY=1 ;; - -h|--help) sed -n '3,30p' "${BASH_SOURCE[0]}"; exit 0 ;; + -h|--help) sed -n '3,37p' "${BASH_SOURCE[0]}"; exit 0 ;; *) echo "unknown option: $arg (try --help)" >&2; exit 2 ;; esac done @@ -61,6 +73,11 @@ die() { printf '\n FAIL %s\n\n' "$*" >&2; exit 1; } jar_id() { [ -f "$JAR" ] && shasum -a 256 "$JAR" | cut -c1-12 || echo "absent"; } running_pid() { pgrep -f "$PATTERN" || true; } +# `launchctl list