feat: reload the broker configuration without restart

.changeset/live-reload.md

@@ -0,0 +1,9 @@
+---
+'gcb-host': minor
+---
+
+Reload `broker.config.yml` without restarting: use a directory watch plus a 5-second safety poll,
+accept `SIGHUP`, keep the last good snapshot on a parse/validation failure, and swap providers
+atomically. The generated sidecar mounts the configuration directory at
+`/etc/git-cred-broker/config` and passes the matching `--config`; the key and refresh token stay
+outside that mount. `socketPath`, `socketMode` and `auditPath` still need a restart.

DEVELOPMENT.md

@@ -110,7 +110,7 @@ gcb compose --dir "$DEPLOY" --socket-dir "$SOCKET_DIR" \
 Change only the `exec npx --yes --package … gcb` prefix to `exec gcb`, keeping `daemon --config …`:
 
 ```sh
-exec gcb daemon --config /etc/git-cred-broker/broker.config.yml
+exec gcb daemon --config /etc/git-cred-broker/config/broker.config.yml
 ```
 
 The dev image already installs the package; do **not** keep a staged `/opt/gcb` mount, or the

README.md

@@ -5,7 +5,7 @@ the only long-lived secret. The GitHub App private key never enters the containe
 still works.
 
 > **Roadmap:** YAML configuration, the three-package split and the `gcb gh` broker/client path are
-> implemented; the host image with `gh` is available. Host-user delegation and live reload are planned.
+> implemented; the host image with `gh` is available. Live reload is implemented; host-user delegation is planned.
 > [`ROADMAP.md`](ROADMAP.md) is the target; this README documents the current tree.
 
 Two commands are installed:
@@ -322,7 +322,7 @@ a sidecar it is inside the container:
 DEPLOY=/srv/git-cred-broker
 cd "$DEPLOY" && docker compose exec gcb \
   npx --yes --package gcb-host gcb \
-  diagnose --config /etc/git-cred-broker/broker.config.yml
+  diagnose --config /etc/git-cred-broker/config/broker.config.yml
 ```
 
 ### Reading a CI log
@@ -564,6 +564,8 @@ file.
 
 ## Configuration reference
 
+The daemon watches the configuration directory with `fs.watch` for low latency and also runs a 5-second digest poll as a safety net; if the native watcher is unavailable, polling runs alone. A valid change is applied without restarting, and an invalid one keeps the last good snapshot. Send `SIGHUP` to force a reload after fixing a file. `socketPath`, `socketMode` and `auditPath` require a restart.
+
 The broker only reads this file. It is YAML. The loader discovers `broker.config.yml` by default;
 the old `broker.config.json` name is no longer discovered, so rename or convert an existing
 deployment, or pass `--config broker.config.json` explicitly (the YAML parser accepts JSON). TOML is

ROADMAP.md

@@ -4,7 +4,7 @@ This is the staged plan for the next version. `README.md` documents the tree as
 this file documents where it is going. YAML configuration, the three-package split, the typed action
 catalog, the `gcb gh` broker/client path and a host image with `gh` are implemented. The executor
 exists, but several Phase 2 items remain incomplete; the old `pr`/`logs` commands remain, and
-host-user delegation/live reload are not implemented.
+host-user delegation is not implemented; live reload is implemented except `gcb doctor` reporting its digest/last result.
 
 ## Locked decisions
 
@@ -126,21 +126,20 @@ The configuration is the source of truth and must be comfortable to edit by hand
       - `gcb actions list|describe|check|discover` reports built-in and configured actions, validates
         them, and lists gh commands that are not yet covered without enabling them.
 - [ ] Live reload (no restart):
-      - watch the configuration **directory**, not a single file, with `fs.watch`; debounce
-        200–500ms and re-add the watcher after rename/delete;
-      - poll every 5 seconds (configurable) and compare mtime+size+content hash. SMB/NFS/9p and
-        single-file bind mounts do not reliably deliver inotify events, so polling is the normal
-        path on a NAS, not a crash fallback;
-      - read the whole file, parse, validate, then atomically swap a frozen snapshot. Any parse or
+      - [x] watch the configuration **directory**, not a single file, with `fs.watch`; debounce
+        300ms; an unavailable watch falls back to polling;
+      - [x] poll every 5 seconds and compare a content hash alongside `fs.watch`; events have been
+        observed to be missed, so the poll is the safety net rather than the primary trigger;
+      - [x] read the whole file, parse, validate, then atomically swap a frozen snapshot. Any parse or
         validation failure keeps the last good snapshot, logs and audits `config reload failed`,
         and leaves the service running;
-      - `SIGHUP` forces a reload;
-      - reload applies allowlists, groups, actions, timeouts and accounts. Provider/key changes
-        rebuild the affected providers; in-flight requests keep the snapshot they started with;
-        already-issued git tokens are not revoked and expire normally;
-      - deployment mounts the config **directory** into a container, not the file, so a host-side
+      - [x] `SIGHUP` forces a reload;
+      - [x] reload applies the fields that exist today (allowlists, timeouts, token/account settings);
+        provider/key changes rebuild providers; in-flight requests keep their snapshot; already-issued
+        git tokens are not revoked. Groups/actions reload when those configuration features land;
+      - [x] deployment mounts the config **directory** into a container, not the file, so a host-side
         atomic replace is visible;
-      - `gcb doctor` reports the active config digest and the last reload result;
+      - [ ] `gcb doctor` reports the active config digest and the last reload result;
         `gcb config check` is the one-shot validation.
 - [x] One-time migration: convert an existing `broker.config.json` to YAML before upgrade. The
       old `broker.config.json` filename is no longer discovered; an explicitly passed JSON file is

docker-compose.example.yml

@@ -78,10 +78,12 @@ services:
     command:
       - sh
       - -c
-      - exec npx --yes --package 'gcb-host' gcb daemon --config /etc/git-cred-broker/broker.config.yml
+      - exec npx --yes --package 'gcb-host' gcb daemon --config /etc/git-cred-broker/config/broker.config.yml
     volumes:
-      # The configuration and the key: read-only, mounted here and nowhere else.
-      - ./broker.config.yml:/etc/git-cred-broker/broker.config.yml:ro
+      # The configuration directory, not one file: an atomic replacement on the host must stay
+      # visible to the container's watcher. Put broker.config.yml in ./config; the key and refresh
+      # token stay outside it.
+      - ./config:/etc/git-cred-broker/config:ro
       - ./app.pem:/etc/git-cred-broker/app.pem:ro
       # Writable, and the only secret that is: GitHub rotates the refresh token on every exchange,
       # so the broker has to write the new one back. It sits beside the key on the host and is

packages/host/src/broker.ts

@@ -69,11 +69,17 @@ interface DecisionContext {
   helper_pid: number | null;
 }
 
+/** The mutable part of a running broker: reload swaps these together. */
+export interface HandlerState {
+  config: BrokerConfig;
+  providers: ReadonlyMap<string, Provider>;
+}
+
 /** Options for {@link createRequestHandler}. */
 export interface RequestHandlerOptions {
-  readonly config: BrokerConfig;
   readonly audit: AuditSink;
-  readonly providers: ReadonlyMap<string, Provider>;
+  /** The live snapshot; startBroker uses this so reload can swap it atomically. */
+  readonly state: HandlerState;
   /** Host-side diagnostics sink; never seen by the caller. */
   readonly log?: (message: string) => void;
 }
@@ -137,7 +143,8 @@ export function prepareSocketPath(socketPath: string): void {
 export function createRequestHandler(
   options: RequestHandlerOptions,
 ): (message: Record<string, unknown>) => Promise<WireResponse> {
-  const { config, audit, providers, log = () => {} } = options;
+  const { audit, log = () => {} } = options;
+  const state = options.state;
 
   /**
    * Record a refusal and produce the caller-visible response.
@@ -166,6 +173,8 @@ export function createRequestHandler(
   }
 
   async function handleRequest(message: Record<string, unknown>, proxy: ProxyChoice): Promise<WireResponse> {
+    // One request keeps the snapshot it started with, even if a reload lands mid-flight.
+    const current = { config: state.config, providers: state.providers };
     const auditEvent = auditEventFor(message['op']);
     const context: DecisionContext = {
       host: typeof message['host'] === 'string' ? message['host'] : null,
@@ -178,7 +187,7 @@ export function createRequestHandler(
     const denyPullRequest = (code: string, reason: string, message: string, hostDetail?: string): WireResponse =>
       deny(context, code, reason, message, hostDetail, auditEvent);
     if (message['op'] === 'ping') {
-      return { ok: true, version: BROKER_VERSION, hosts: [...providers.keys()] };
+      return { ok: true, version: BROKER_VERSION, hosts: [...current.providers.keys()] };
     }
     if (
       message['op'] !== 'credential' &&
@@ -196,7 +205,7 @@ export function createRequestHandler(
 
     // Host first: whether plaintext is tolerated is a property of the *host's* configuration,
     // not of the request, so the protocol can only be judged once the host is known.
-    const block = hostConfig(config, message['host']);
+    const block = hostConfig(current.config, message['host']);
     if (!block) {
       return deny(
         context,
@@ -252,7 +261,7 @@ export function createRequestHandler(
       );
     }
 
-    const provider = providers.get(block.host);
+    const provider = current.providers.get(block.host);
     if (!provider) {
       return deny(
         context,
@@ -607,6 +616,8 @@ export interface StartBrokerOptions {
 /** A running broker. */
 export interface RunningBroker {
   readonly socketPath: string;
+  /** Swap the configuration and providers; the socket path is immutable for one process. */
+  reload(config: BrokerConfig, providers: ReadonlyMap<string, Provider>): void;
   close(): Promise<void>;
 }
 
@@ -619,7 +630,12 @@ export interface RunningBroker {
 export async function startBroker(options: StartBrokerOptions): Promise<RunningBroker> {
   const { config, audit, providers, log = () => {} } = options;
   prepareSocketPath(config.socketPath);
-  const handle = createRequestHandler({ config, audit, providers, log });
+  const state: HandlerState = { config, providers };
+  const handle = createRequestHandler({ audit, log, state });
+  const reload = (nextConfig: BrokerConfig, nextProviders: ReadonlyMap<string, Provider>): void => {
+    state.config = nextConfig;
+    state.providers = nextProviders;
+  };
 
   const server = net.createServer({ allowHalfOpen: false }, (socket) => {
     socket.setTimeout(REQUEST_TIMEOUT_MS);
@@ -677,6 +693,7 @@ export async function startBroker(options: StartBrokerOptions): Promise<RunningB
 
   return {
     socketPath: config.socketPath,
+    reload,
     async close(): Promise<void> {
       await new Promise<void>((resolve) => server.close(() => resolve()));
       try {

packages/host/src/commands/compose.ts

@@ -44,6 +44,24 @@ export interface ComposeInput {
 /** Where a code-mounted sidecar expects the broker. */
 export const CODE_DIR = '/opt/gcb';
 
+/** A volume scalar as YAML, with compose's dollar escaping applied first. */
+function yamlVolume(host: string, container: string, mode?: string): string {
+  const raw = `${host}:${container}${mode === undefined ? '' : `:${mode}`}`;
+  const escaped = raw.replace(/\$/g, () => '$$');
+  // Plain YAML is fine until a path has a space, '#' or another indicator; JSON quoting is valid YAML.
+  return /^[A-Za-z0-9_./:@%+=,~-]+$/.test(raw) ? escaped : JSON.stringify(escaped);
+}
+
+/** Follow a selected config symlink; a missing file is the only fallback. */
+function selectedConfigFile(configPath: string): string {
+  try {
+    return fs.realpathSync(configPath);
+  } catch (error) {
+    if ((error as NodeJS.ErrnoException).code === 'ENOENT') return configPath;
+    throw error;
+  }
+}
+
 /**
  * Render the compose file.
  *
@@ -53,6 +71,11 @@ export const CODE_DIR = '/opt/gcb';
 export function renderCompose(input: ComposeInput): string {
   const socketDir = input.socketDir.replace(/\/+$/, '');
   const configPath = input.configPath ?? `${input.dir}/broker.config.yml`;
+  // Follow a selected symlink: the directory bind exposes the link, not a target outside it.
+  const configFile = selectedConfigFile(configPath);
+  const mountedConfigPath = path.posix.join(path.posix.dirname(SIDECAR.configPath), path.basename(configFile));
+  // One shell word; compose interpolates $ before sh runs, so a literal dollar is doubled.
+  const mountedConfigArg = `'${mountedConfigPath.replace(/'/g, "'\\''")}'`.replace(/\$/g, () => '$$');
   const fromCode = input.code !== undefined;
 
   const origin = fromCode
@@ -66,7 +89,7 @@ export function renderCompose(input: ComposeInput): string {
 # To avoid depending on the registry at boot, either render this file with --code <directory> to
 # run staged code from a mount, or point --image at an image of your own that already has the
 # package installed, and change \`command\` to the binary in it:
-#   ["gcb", "daemon", "--config", "${SIDECAR.configPath}"]`;
+#   ["gcb", "daemon", "--config", "${mountedConfigPath}"]`;
 
   // The egress lines belong to both renders: a --code deployment is a sidecar too, and without them it
   // would carry no proxy variables at all, so the daemon would go direct and hang rather than refuse.
@@ -92,12 +115,12 @@ export function renderCompose(input: ComposeInput): string {
   // directory instead of a token — for every action, not only the personal ones. Emit it once `authorize`
   // has written the file, and say what to run until then.
   const userRefreshMount = fs.existsSync(`${input.dir}/${USER_TOKEN_FILE}`)
-    ? `\n      # The authorized person's refresh token: the only secret mounted writable, because GitHub\n      # rotates it on every exchange.\n      - ${input.dir}/${USER_TOKEN_FILE}:${SIDECAR.userTokenPath}:rw`
+    ? `\n      # The authorized person's refresh token: the only secret mounted writable, because GitHub\n      # rotates it on every exchange.\n      - ${yamlVolume(`${input.dir}/${USER_TOKEN_FILE}`, SIDECAR.userTokenPath, 'rw')}`
     : '\n      # No user.refresh yet: run `gcb authorize` and render this again. A mount' +
       '\n      # for a path that does not exist would become a directory, and the broker would read that.';
   const start = fromCode
-    ? `exec node --conditions=development ${CODE_DIR}/packages/host/bin/gcb.js daemon --config ${SIDECAR.configPath}`
-    : `exec npx --yes --package '${input.packageSpec}' gcb daemon --config ${SIDECAR.configPath}`;
+    ? `exec node --conditions=development ${CODE_DIR}/packages/host/bin/gcb.js daemon --config ${mountedConfigArg}`
+    : `exec npx --yes --package '${input.packageSpec}' gcb daemon --config ${mountedConfigArg}`;
   const command = `      - sh
       - -c
       - |
@@ -111,7 +134,7 @@ export function renderCompose(input: ComposeInput): string {
   const codeMount = fromCode
     ? `
       # The code, read-only, from the commit you reviewed and staged.
-      - ${input.code}:${CODE_DIR}:ro`
+      - ${yamlVolume(input.code, CODE_DIR, 'ro')}`
     : '';
 
   return `# Docker sidecar for gcb-host@${PACKAGE_VERSION}
@@ -153,11 +176,14 @@ services:
     command:
 ${command}
     volumes:${codeMount}
-      - ${configPath}:${SIDECAR.configPath}:ro
-      - ${input.dir}/app.pem:${SIDECAR.keyPath}:ro
+      # The directory, not the file: an atomic replacement on the host must be visible for reload.
+      # Keep broker.config.yml in its own subdirectory to keep this mount narrow; init's default
+      # layout puts it beside app.pem/user.refresh, which this mount also exposes read-only.
+      - ${yamlVolume(path.dirname(configFile), path.dirname(SIDECAR.configPath), 'ro')}
+      - ${yamlVolume(`${input.dir}/app.pem`, SIDECAR.keyPath, 'ro')}
 ${userRefreshMount}
-      - ${input.dir}/log:${SIDECAR.auditDir}
-      - ${socketDir}:${SIDECAR.socketDir}
+      - ${yamlVolume(`${input.dir}/log`, SIDECAR.auditDir)}
+      - ${yamlVolume(socketDir, SIDECAR.socketDir)}
 # The socket appears at ${socketDir}${SIDECAR.socketPath.slice(SIDECAR.socketDir.length)} on the host. Point the pushing
 # environment at the same socket with:
 #   gcb setup --socket <that path as the pusher sees it>
@@ -204,24 +230,25 @@ export function runCompose(argv: readonly string[]): number {
         ? path.join(dirArg, 'broker.config.yml')
         : resolveConfigPath(),
   );
-  const dir = path.resolve(dirArg ?? deploymentDir(configPath));
+  const configFile = selectedConfigFile(configPath);
+  const dir = path.resolve(dirArg ?? deploymentDir(configFile));
   const socketDir = args.value('socket-dir');
   if (!socketDir) {
     fail('--socket-dir is required (a host directory the pushing container already mounts)');
   }
 
   // Check before emitting. A config written for the other deployment produces a sidecar that
   // starts and then cannot find its own private key, which is a confusing way to find out.
-  if (fs.existsSync(configPath)) {
-    const mode = inferMode(loadConfig(configPath));
+  if (fs.existsSync(configFile)) {
+    const mode = inferMode(loadConfig(configFile));
     if (mode !== 'sidecar') {
       fail(
-        `${configPath} was written with --mode host; re-run ` +
+        `${configFile} was written with --mode host; re-run ` +
           '`gcb init --mode sidecar ...` before using compose',
       );
     }
   } else {
-    warn(`no configuration at ${configPath} yet; create it with \`init --mode sidecar\` first`);
+    warn(`no configuration at ${configFile} yet; create it with \`init --mode sidecar\` first`);
   }
 
   // POSIX only, and guaranteed to exist by the refusal above: the file this prints binds host paths
@@ -237,7 +264,7 @@ export function runCompose(argv: readonly string[]): number {
   say(
     renderCompose({
       dir,
-      configPath,
+      configPath: configFile,
       socketDir,
       user,
       image: args.value('image') ?? 'node:24-slim',

packages/host/src/config.ts

@@ -73,7 +73,7 @@ export function userTokenPath(privateKeyPath: string): string {
 }
 
 export const SIDECAR = {
-  configPath: '/etc/git-cred-broker/broker.config.yml',
+  configPath: '/etc/git-cred-broker/config/broker.config.yml',
   keyPath: '/etc/git-cred-broker/app.pem',
   // Beside the key, because that is how the provider derives it without being told.
   // Derived from the key rather than written again: the mount target and the reader's path at once.
@@ -437,19 +437,27 @@ export function validateConfig(raw: unknown, source = '<config>'): BrokerConfig
   };
 }
 
+/** Read configuration text, with the deployment path named on failure. */
+export function readConfigText(path: string): string {
+  try {
+    return readFileSync(path, 'utf8');
+  } catch (error) {
+    throw new Error(`cannot read config ${path}: ${(error as Error).message}`);
+  }
+}
+
 /**
  * Read and validate a configuration file.
  *
  * @param path - path to the YAML config.
  * @returns the validated configuration.
  */
 export function loadConfig(path: string): BrokerConfig {
-  let text: string;
-  try {
-    text = readFileSync(path, 'utf8');
-  } catch (error) {
-    throw new Error(`cannot read config ${path}: ${(error as Error).message}`);
-  }
+  return loadConfigText(readConfigText(path), path);
+}
+
+/** Parse and validate configuration text already read from `path`. */
+export function loadConfigText(text: string, path: string): BrokerConfig {
   const document = parseDocument(text, {
     schema: 'core',
     uniqueKeys: true,