Two long-stale attach smokes assert the behaviour that replaced them (#655)

[philcunliffe]

Both smokes named in #655 fail on origin/master, and both are stale smokes, not code defects. Each asserts a contract that a later, documented decision deliberately replaced, and neither smoke was updated alongside it. Neither is in the CLAUDE.md release battery, which is why the drift went unnoticed for weeks.

I reproduced both failures on origin/master before changing anything, and bisected each to an exact commit by confirming a pass at the parent and the identical failure at the commit itself.

claude_attach_detach

Failure on master

settings: SessionStart hook installed with --state-file pointing at the plugin state dir

The predicate requires hooks.SessionStart.length === 1. Actual value is two groups: claude-hook session-context --state-file .../session-context.jsonl and claude-hook classify-cwd.

When it started: 41ea01c (2026-07-14, "Enrollment privacy review", #316). Verified: passes at 3766f50 (the parent), fails identically at 41ea01c.

Stale, not a defect. LLP 0106 (Accepted) decided that a session-start classification hook is installed alongside the existing session-context hook. settings.js's MANAGED_HOOK_SPECS implements it: session-context rides all four managed events, classify-cwd rides only the two where a fresh working directory appears (SessionStart, CwdChanged). The unit test test/plugins/claude-settings-attach.test.js was updated for this at the time and asserts exactly this shape on the marker's managed hook list. The smoke's on-disk golden compare simply was not.

Change: assert both sides of the split rather than just the surviving one. SessionStart and CwdChanged each carry the pair in install order; UserPromptSubmit and PostToolUse carry session-context alone. Dropping either kind, or leaking classify-cwd onto the per-prompt / per-tool events, now fails here. Added a hookCommands() helper that flattens a hooks.<event> block to its command strings.

client_attach_on_join

Failure on master

no re-attach: the attach.claude marker timestamp is unchanged (done short-circuits)

Telemetry from the failing run shows two client_action.attach_perform events at different endpoints (http://127.0.0.1:42635 then http://127.0.0.1:40765): the attach was re-performed on the relaunch.

When it started: 1ce40da (2026-07-07, "Attach lifecycle: track the gateway's ephemeral port", #277 / #278). Verified: passes at 1936122 (the parent), fails identically at 1ce40da.

Stale, not a defect. The smoke was written in 96cd19d (LLP 0044/0045/0046) under the attach once, done forever model, and its step 3 encodes it: a relaunch on an unchanged revision must not re-attach. LLP 0086 (Active) D1 #re-attach-on-drift replaced that model, calling it out as the root cause of a real user report: the marker now records the endpoint it attached at, and a done marker at a moved endpoint is a forward gap that re-performs. rev-1 in this smoke binds 127.0.0.1:0, so every relaunch is at a new port and the re-attach is the documented, intended outcome. The smoke's assertion is the pre-0086 behaviour.

Change: cover both branches of the freshness check rather than only the one that survived.

1. The relaunch on rev-1 (ephemeral bind) is now the drift branch: the marker is refreshed at a new endpoint, and env.ANTHROPIC_BASE_URL follows it to the newly bound port. This is the property LLP 0086 exists to protect, and nothing else covered it end to end.
2. A new rev-1b pins the gateway's listen to a port the smoke reserves, so the input the freshness check watches stops moving. The relaunch onto rev-1b re-attaches once at the pinned port, and a further relaunch at that same stable endpoint is the no-op branch: the marker timestamp and the client settings are byte-for-byte unchanged, which is what step 3 originally meant to prove and now proves honestly.

The rev-2 drop / reverse-gap step is unchanged apart from taking the pinned listen.

Verification

• claude_attach_detach: FAIL on origin/master before, ok after.
• client_attach_on_join: FAIL on origin/master before, ok after (run 3x, stable).
• node scripts/run-tests.js: 3574 pass, 0 fail, 1 skipped.
• npx tsc -p tsconfig.json --noEmit: clean.
• Collateral: gateway_claude_capture ok, cli_bundled_plugins_activated ok. client_attach_idempotent still fails, but identically on pristine origin/master (the adapter_not_enabled message split, Release-battery smoke client_attach_idempotent fails on master (passes at v1.19.0) #652), so it is untouched by this change.

Note on the release battery

Neither smoke is in the CLAUDE.md battery. Both are now green and both cover contracts nothing else covers end to end (the on-disk hook block after attach; the LLP 0086 drift/no-drift split against a real daemon). Adding them is worth doing, but client_attach_on_join now runs six daemon launches and takes roughly 15s, so it is a judgement call about battery runtime rather than an obvious edit. I have deliberately not touched CLAUDE.md here.

Fixes #655

[philcunliffe]

Neutral review round 1 - 58ca0ff

Both stale-vs-defect rulings were verified against the LLPs and both bisects re-run end to end, rather than taken on trust. "The smoke was stale" is the cheaper conclusion, so it got the harder look.

claude_attach_detach - stale, confirmed. LLP 0106 is Accepted and its Decision reads "a session-start hook, installed at attach alongside the existing session-context hook", which is exactly what the old length === 1 assertion contradicted. Bisect re-run: 41ea01c^ ok, 41ea01c fails identically to master. Corroborating: that commit updated claude-settings-attach.test.js (+21, asserting classifyEvents) and touched no smoke, the exact signature of a missed golden-compare update.

client_attach_on_join - stale, confirmed. LLP 0086 is Active and D1 #re-attach-on-drift says a false result "makes the unit a forward gap that re-perform()s this pass instead of short-circuiting", with Consequences spelling out that ANTHROPIC_BASE_URL follows the moving port. The old step asserted the literal opposite. Bisect re-run: 1ce40da^ ok, 1ce40da fails identically.

Neither smoke was weakened. claude_attach_detach went from 1 assertion to 4, covering all four managed events with the pair in order on the fresh-cwd ones, so classify-cwd leaking onto per-prompt or per-tool events now fails. client_attach_on_join replaced one weak assertion with both branches of the same check.

Flakiness, measured rather than assumed

11 consecutive runs plus 4 concurrent, all green. But the review found the new reserveLocalPort was a bind-then-release TOCTOU: the probe closes and the gateway binds ~5s later, so anything can take the port between, including the smoke's own stub central server. Measured collision rate ~1 in 20000, and the failure mode is at least loud. The real objection was structural: it made this the only smoke flow that can collide with a co-resident process, where all ~20 others are port-agnostic.

Findings, fixed in d55a90b

LOW - the reservation race is gone. Adopted the race-free alternative: read the gateway's live port from status.json via resolveLiveGatewayEndpointFromStatus (LLP 0086 D2) and pin rev-1b to that, so the daemon reclaims a port it already holds and no reservation window opens. Verified the read is available at that point in the flow, and that there is no TIME_WAIT problem (the listener never enters it, and Node sets SO_REUSEADDR); the reclaim held 12 of 12 runs.

The fix surfaced a subtlety worth recording: pinning a port the daemon already holds means the relaunch resolves the same endpoint the marker recorded, so the old "marker moved to the pinned endpoint" step would have passed vacuously, satisfied on the first poll by the drift-era marker. Rather than leave that, the boot was folded into the no-reattach step as an explicit guard branch, and a negative control was run to prove non-vacuity: serving the ephemeral port instead of the pinned one makes the smoke fail at exactly the no-reattach assertion. Two assertions were also added that did not exist before, including a startedAt check so a failed rebind or an LLP 0114 port fallback fails loudly instead of looking like a clean short-circuit.

LOW - an @ref gloss claimed LLP authority it did not have. The LLP 0106#decision gloss said the hook rides the fresh-cwd events "and only those", but 0106 never mentions CwdChanged and never scopes the hook away from other events; that rationale lives in the comment above MANAGED_HOOK_SPECS. Narrowed to what 0106 settles, with the event scoping attributed to the code that owns it. The same misattribution in the flow's header comment was fixed too.

Results

8 consecutive runs of client_attach_on_join plus 4 concurrent, all ok. claude_attach_detach, gateway_claude_capture, cli_bundled_plugins_activated all ok. npm test 3574 pass / 0 fail / 1 skip, tsc clean. Scope is the two smoke files only, no production code.

client_attach_idempotent fails identically on origin/master, verified in a separate worktree. Pre-existing, PR #654's territory.

Noted, not fixed

Pre-existing and unchanged by this PR: the await sleep(500) before the no-reattach comparison is a fixed sleep after probation clears, so on a heavily loaded machine the boot-already-confirmed pass might not have run yet. The negative control above shows the assertion is not vacuous today, but the sleep is the weak link if this ever does flake.

[philcunliffe]

Neutral review round 2 - d55a90b - CLEAN

Both round-1 findings fixed. The restructure got the hardest look, since it was done specifically to avoid a green-but-meaningless assertion, and a botched restructure would leave exactly that.

Four negative controls, run independently rather than trusting the one reported:

mutant	result
rev-1b serves the ephemeral port instead of the pin	FAIL at the new reclaim guard
isCurrent forced false (short-circuit disabled)	FAIL at no re-attach ... (done short-circuits)
isCurrent forced true (drift detection disabled)	FAIL at marker was not refreshed after the gateway rebound
gatewaySourceDetails returns port + 1	FAIL at the drifted gateway reports its live bound endpoint

The second is the one that matters: it proves the restructured short-circuit assertion is not vacuous under the pin. The first alone would not have shown that, since it fails at the guard rather than at the assertion.

Both freshness branches survive. What the restructure dropped was a second instance of the drift branch (drift caused by a config change rather than by ephemeral rebinding, same isCurrent path), not a distinct branch. Re-attach-on-drift is proven live by NC-C with ANTHROPIC_BASE_URL === drifted.endpoint; short-circuit-on-unmoved is proven live by NC-B. One incidental gain: the marker being short-circuited is now written by a different daemon boot, so the guard is proven across a restart boundary, where at 58ca0ff both boots were at the pinned port.

The startedAt guard is sound. It is computed per runDaemon call and never persisted, so two boots cannot collide at ms resolution; the new boot's first writeStatusFile carries the new startedAt with sources: [], so there is no window where the timestamp is fresh but the sources are stale (the resolver returns undefined and polling continues). The reviewer could not construct a wrong pass. Worth recording: the pid-liveness gate inside resolveLiveGatewayEndpointFromStatus is a no-op here because the daemon runs in-process, so all staleness protection comes from these explicit equality assertions.

reserveLocalPort is gone (repo-wide grep returns nothing) and no fixed port remains; the only literals are 127.0.0.1:0 and the stub's listen(0). 12 consecutive runs, all green. A narrow rebind window remains between the drift daemon releasing the port and the relaunch reclaiming it, but that is inherent to testing "a pinned port is reclaimed across a restart" and is far narrower than the old reserve-then-release window.

Corroboration for the round-1 ruling: the reviewer ran client_attach_on_join on origin/master and it FAILS at the pre-0086 timestamp assertion, so this PR is fixing a genuinely red smoke.

Finding 2 verified: LLP 0106's Decision says nothing about which events, and the narrowed gloss now claims exactly the alongside-session-context split and no more. The redirected attribution is accurate: MANAGED_HOOK_SPECS at settings.js:47 with the event-scoping rationale in the comment directly above it.

Scope is the two smoke files only. npm test 3574 pass / 0 fail / 1 skip, tsc clean, claude_attach_detach, gateway_claude_capture, cli_bundled_plugins_activated all ok. client_attach_idempotent fails identically on master, PR #654's territory.

A methodology note worth carrying: the reviewer's first negative control used a python3 heredoc, python3 is absent on this host, and the mutation silently never applied, so the smoke passed and looked like a vacuous assertion. It was caught and redone with node. A mutation that silently fails to apply reads exactly like a test that fails to catch it.