Skip to content

fix(e2e): withDaemon waits for admin.token before handing the daemon over - #455

Merged
V3RON merged 2 commits into
mainfrom
bug/404
Oct 11, 2026
Merged

V3RON merged 2 commits into
mainfrom
bug/404

Conversation

@V3RON

@V3RON V3RON commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Root cause

withDaemon handed the test a daemon before the daemon's admin.token file existed. Every admin CLI call after that raced the daemon's write of the file.

  1. withDaemon → startDaemon() ran simlock daemon start and returned when the command exited (e2e/helpers/env.ts:241 on main).
  2. daemon start exits 0 once status.get answers (src/cli/index.ts:1618-1631). The daemon answers hello and status.get from the moment it claims its socket (src/daemon/server.ts:447). It writes admin.token only after that, in adminSecret.persist() (src/daemon/server.ts:454, ADR 0003 §5).
  3. token create --role worker (e2e/gateway-fast-fail.test.ts:35) reads its credential from admin.token, three tries 50 ms apart (src/cli/index.ts:219-232). If the file is still missing, the CLI connects as agent and prints the notice (src/cli/index.ts:294-300). token.create needs admin, so the daemon answers FORBIDDEN. stdout is empty, minted.json is undefined, and line 36 throws Cannot destructure property 'secret' of 'minted.json'.
  4. Teardown (e2e/helpers/env.ts:360-361) records the pid holding the socket, then runs daemon stop. That command reads the same missing file, connects as agent, and gets FORBIDDEN from daemon.stop (src/daemon/server.ts:768-776). The daemon keeps running, so the grace window ends with withDaemon teardown found stray daemon process(es) that outlived daemon stop.

So the two errors in the issue have one cause: admin.token was not on disk when either command read it. The write is one temp-file write plus a rename. On a loaded box it can finish later than the CLI's 100 ms retry window, and nothing in the helper waited for it. The product does what ADR 0003 §5 and docs/CLI.md say it does here: a short retry, then an agent session with a notice. Architecture rule 11 needs that wait to be bounded. The false assumption was in the helper: "daemon start has exited, so admin commands work".

Fix

  • e2e/helpers/env.ts: when daemon start exits 0, startDaemon() now waits until admin.token exists before it returns. restartDaemon() (and withConfig() through it) now starts the daemon through startDaemon(), so the same wait covers restarts.
  • e2e/gateway-fast-fail.test.ts: startGateway checks minted.code and prints minted.stderr before it destructures. A failure now names the CLI's own error, which the issue asked for.

Why this removes the race by construction: writeFileAtomic renames a complete temp file into place, so once admin.token exists it holds the full secret. Only the daemon that won the socket claim writes the file. It is removed only when that daemon stops gracefully. withDaemon no longer returns before the file exists, so every later CLI call in the test reads it and connects as admin, however long the write took. That includes token create and the teardown's daemon stop. The ordering no longer depends on how fast the daemon's I/O is compared with the CLI's 100 ms retry.

Why it is the simplest fix: it is one wait in the one helper that holds the false assumption. The alternatives change the product. One is to make hello wait for the file, which contradicts ADR 0003 §5 ("a credential can be checked before the file lands"). Another is to make daemon start or the CLI retry wait longer, which only moves a wall-clock bound.

The wait uses waitFor's default 30 s budget. It is a backstop for a daemon that never writes the file, not the mechanism that orders the calls.

Evidence

Forcing the interleaving. A temporary patch, not committed, delays the daemon's write of admin.token by 3 s. That keeps the file missing for the whole window in which the test calls token create and the teardown calls daemon stop:

--- a/src/daemon/admin-secret.ts
+++ b/src/daemon/admin-secret.ts
@@ -52,6 +52,7 @@ export class AdminSecretManager {
    * class doc.
    */
   async persist(): Promise<void> {
+    await new Promise((resolve) => setTimeout(resolve, 3_000));
     await this.#filesystem.writeFileAtomic(this.#path, `${this.#secret}\n`, {
       mode: ADMIN_SECRET_FILE_MODE,
     });

Unfixed code (main 8eb9c87) with the patch: pnpm test:e2e e2e/gateway-fast-fail.test.ts -t "drained"

 FAIL  |e2e| e2e/gateway-fast-fail.test.ts > ... > fails a request waiting for a busy worker with NO_CAPACITY when that worker, the only one able to serve it, drained
TypeError: Cannot destructure property 'secret' of 'minted.json' as it is undefined.
 ❯ startGateway e2e/gateway-fast-fail.test.ts:36:11
 FAIL  |e2e| e2e/gateway-fast-fail.test.ts > ... > fails a request waiting for a busy worker with NO_CAPACITY when that worker, the only one able to serve it, drained
Error: withDaemon teardown found stray daemon process(es) that outlived `daemon stop` for /var/folders/.../simlock-e2e-GQD4Ln/daemon.sock: 45015
      Tests  2 failed | 5 skipped (7)

These are both errors from the issue, word for word. The other failing test, "fails a request only a drained worker…", also matches -t drained and fails the same way. With only this PR's minted.code assertion added, the CLI's own output shows the mechanism:

AssertionError: {"notice":"admin credential unavailable; connecting as agent (admin-only commands will fail with FORBIDDEN)"}
{"error":{"code":"FORBIDDEN","message":"Operation token.create requires role admin, session is agent"}}
: expected 1 to be +0
Error: withDaemon teardown found stray daemon process(es) that outlived `daemon stop` for .../daemon.sock: 46009

This PR with the same patch (every daemon's write delayed 3 s, gateway and workers): pnpm test:e2e e2e/gateway-fast-fail.test.ts

      Tests  7 passed (7)

Sanity check without the patch: pnpm test:e2e e2e/gateway-fast-fail.test.ts e2e/contract-surface.test.ts e2e/held-lease-liveness.test.ts e2e/event-history.test.ts e2e/startup-reconcile.test.ts e2e/mcp-session.test.ts passes 38 of 38. These files call startDaemon, restartDaemon or withConfig.

No permanent regression test: forcing a slow write needs a delay hook in the daemon, and the rules do not allow a production test hook for this. The diff above is how to re-apply the reproduction.

Assumptions

  • Assumption: a daemon that daemon start reports running always writes admin.token (DaemonServer#start writes it right after the socket claim, and a failed write fails the start). So waiting for the file cannot hang on a healthy daemon, and the 30 s budget only catches a broken one.
  • Assumption: out of scope here, and the same as on main: after killDaemon("SIGKILL") the dead daemon's admin.token stays behind. The wait sees that old file and returns at once, so a test that restarts after a SIGKILL still races the new daemon's write.

Review

Spec review: 0 blocking, 0 fixed, 3 notes. Code review: 0 blocking, 0 fixed, 3 notes. Claims review: 1 blocking, 1 fixed, 1 notes. Comment review: 3 blocking, 2 fixed, 0 notes.
Mutate: 0 mutants, 0 alive.

Rejected:

  • code: e2e/helpers/env.ts:248 a startDaemon timeout before activeEnvs.add leaks the daemon — not blocking: only a live daemon that never writes admin.token for 30 s
  • code: e2e/helpers/env.ts:246 the comment's "every later CLI call" is false after SIGKILL (tests lens) — duplicate of the claims finding at env.ts:246, fixed there
  • spec: e2e/helpers/env.ts:248 the new waitFor branch has no test that goes red [testing 3] — not blocking: testing 3 covers production code; this is an e2e helper
  • comments: e2e/gateway-fast-fail.test.ts:30 the driver "none" comment only describes behaviour and cites an ADR — it says why a gateway passes driver "none", which rule 17 allows

Closes #404

Written by an agent.

V3RON added 2 commits October 11, 2026 12:55
…over

`daemon start` exits once the daemon answers status.get, which it does from
its socket claim on; admin.token lands only after that claim. An admin command
run next read no file, connected as agent and got FORBIDDEN: token create
printed no JSON and the teardown's daemon stop left the daemon running.

Closes #404
@V3RON

V3RON commented Oct 11, 2026

Copy link
Copy Markdown
Contributor Author

Review notes

Not blocking, not verified. Each is one reviewer's claim.

  • spec: e2e/helpers/env.ts:240 adds a wait for admin.token to every withDaemon test, but the issue asked only for diagnostics (print minted.stderr and minted.code); the wait may hide the cause the new assertion should catch
  • spec: e2e/helpers/env.ts:241 the comment states the FORBIDDEN race over admin.token as the cause, but the issue says the CLI's stderr was never captured, so that cause is unverified by the issue itself
  • spec: e2e/helpers/env.ts:248 deleting the new waitFor branch turns no test red with certainty; only the rare flake would come back
  • code: e2e/helpers/env.ts:248 after SIGKILL the old admin.token stays on disk, so existsSync passes at once and the next CLI call still races the new daemon's write (the PR's second Assumption says the same)
  • code: e2e/helpers/env.ts:248 a graceful stop unlinks the socket (src/daemon/server.ts:632) before it removes admin.token (src/daemon/server.ts:637); stopDaemon in e2e/startup-reconcile.test.ts:69 waits only for the socket, so the old token can pass the wait
  • code: e2e/helpers/env.ts:248 a waitFor timeout in the initial startDaemon throws before activeEnvs.add (e2e/helpers/env.ts:320), so that daemon and its home would not be torn down
  • claims: e2e/helpers/env.ts:246-247 "after a restart the previous run's file may still be there" is true only for kill-then-start; restartDaemon's graceful stop removes admin.token (src/daemon/admin-secret.ts:59-64)

Written by an agent.

@V3RON
V3RON marked this pull request as ready for review October 11, 2026 13:10
@V3RON
V3RON merged commit fd35520 into main Oct 11, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky e2e: gateway-fast-fail's 'token create --role worker' on a fresh gateway prints no JSON

1 participant