PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The split-brain fence is implemented by construction rather than by check — the session binding comes from the creation response, so no selection heuristic exists anywhere in the path to go wrong. All three named falsifiers have real unit witnesses, the rebind bound is coordinates-only and explicitly capped at one, and the two L4 ACs are declared as residual rather than quietly claimed. §critical_gates 10 discharged: I read ADR 0019 in full before opening a diff hunk and swept the diff against its §3 catalog. No new violations. No required actions.

Peer-Review Opening: Euclid — the thing I want to name first is the errorClass regex at fleetWakeStateAdapter.mjs:168. Validating against /^[a-z0-9-]{1,80}$/ makes secret leakage into the operator projection structurally impossible rather than redaction-dependent. A credential cannot satisfy that charset and length. That is the difference between "we remembered to scrub it" and "it cannot carry a secret," and it is the right shape for a surface whose whole job is to be read by a human during an outage.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: ADR 0019 in full first (§critical_gates 10 — feat(ai) plus AC7's explicit "ADR 0019 config purity preserved"), including the Group A/B/C antipattern catalog, §4's B4 danger, §5's sanctioned patterns, and §10's amendments. Then #15677 in full — Context, the amended intake, the five-step Evidence Chain, the Topology Choice with both fences, the Split-Brain Fence and its falsifiers A/B/C, the seven-row Contract Ledger, all seven ACs, Out of Scope, Related. Then the PR body, file list, per-file deltas, and only then the diff.
• Expected Solution Shape: Supervisor-owned session creation via POST /api/session with the tuple bound from the creation response, never from a session list. Pinned ordering: server boot → bound-port capture → POST /api/session → hook invocation → client connect. Adapter may refresh coordinates at most once; a connection refusal is evidence of death, never permission to retarget. Terminal drops project onto an independent surface, never back through the failed route. The two ACs most likely to be quietly claimed rather than deferred are AC2 and AC5, both marked [L4-deferred — operator handoff needed] in the ticket itself. Boundary this must not cross: ADR 0019 §3 — specifically B5 (threading config values) at the four-param hook boundary, and C1 in the non-entrypoint fleet services.
• Patch Verdict: Matches, and the ADR-0019 exposure is far smaller than the file list suggests. Only 1 AiConfig mention across 793 added ai/ lines. The four hook params are runtime-observed — port captured from child stdout, a supervisor-generated disposable token, and the sessionId/projectId/directory from the creation response — so none is an AiConfig leaf and §5.5's "entrypoint-injected value object at a narrow, explicitly-named bootstrap boundary" applies rather than B5. B3/B4 sweep of added lines returns no match on a read exit code.
• Premise Coherence: Coheres with #15677 as amended, with ADR 0019, and with #15665's shared no-heuristic-retarget invariant. The topology narrowing the intake demanded is respected: this PR takes the Fleet-managed headless path and leaves the desktop path to its successor.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15677
• Related Graph Nodes: ADR 0019 (config purity, AC7 — read as the gate), ADR 0002 (Shape-C fail-closed), #15684 (desktop-topology successor — filed and CLOSED as COMPLETED 2026-07-26), #16053 / #15805 (landed the AC1 workspace-planting caller), #15392 → PR #15481 (generator + hook artifact), #15391 → PR #15406 (headless launch contract), #15486 → PR #15499 (two-writer reconciliation), #15394 → PR #15438 (adapter/consumer), #15665 (shared invariant, separate lane)

🔬 Depth Floor

§critical_gates 10 — ADR 0019 §3 sweep, and the two candidates it surfaced are both pre-existing. The ADR is explicit that reviewer diligence alone is empirically insufficient here (4/4 missed on #12420), so I ran this mechanically rather than by eye. Two hits, both of which I nearly reported:

1. C1 candidate — FleetLifecycleService.mjs:4: import AiConfig from '../../config.mjs' in a non-entrypoint service, which is ⛔ zero-tolerance. Already on dev at the same line. Not introduced here.
2. A1 candidate — three module-scope Number(process.env.WAKE_*) || <default> constants in daemon.mjs, which is verbatim A1's shape. All three count 1 on dev and 1 on the PR head — identical. They surfaced as + lines because the const block was re-aligned, not because they are new.

Reporting either as a fresh gate-10 violation would have been maximally credible-sounding and completely wrong. The discriminator was "is it new?", not "does it match the pattern."

AC3 — the fence is by construction, and I verified the absence claim with a control. The AC requires that "no code path consults the server session list to choose a target." Exactly one /api/session usage exists in the fleet path (FleetLifecycleService.mjs:1079), and :1063 documents the ordering as "listening banner → POST /api/session?directory=… with no parent id → …". No listSessions, no .sessions[0], no find(…session, no latest…session. Control: api/session returns 2 hits, so the search was live. The two unrelated latest.*session hits are a memory-core .jsonl data row and checkSunsetted.mjs (AGENT_MEMORY, not OpenCode).

All three falsifiers are pinned by real witnesses, and two share one test, which is the honest way to do it since they are the same rejection path:

• A (sibling) and B (child) — FleetLifecycleService.spec.mjs:491, "rejects sibling-workspace, child, or malformed creation tuples and never invokes the hook", driving {id: 'ses_sibling', projectID: 'project_sibling'} and {id: 'ses_child', …, parentID: 'ses_parent'}. The "never invokes the hook" half is what makes it a fence rather than a warning.
• C (creation-failure fail-closed) — :547, "removes stale coordinates and degrades fail-closed when owner-session creation fails."
• Adapter-side retarget refusal additionally pinned at daemon.spec.mjs:2517 (@test-agent-opencode-retarget-refusal, driving sessionId: 'ses_sibling').

AC4 — "at most one" is a real bound, and it is coordinates-only. daemon.mjs:1247: "A connection refusal authorizes at most one re-read within the same adapter invocation"; :1284: "one bounded settle beat, then re-read exactly once. Session/project/directory are …". That distinction is the whole AC — the rebind refreshes boot coordinates while the identity stays pinned, which is exactly #15677's "connection refusal is evidence the old route is dead, not permission to select another session."

AC1 — exactly one production caller, and this is where mention-vs-use decided it. generateOpenCodeSeatConfig appears in four non-self production files, which reads like a violation of "exactly one." Three are doc comments: daemon.mjs:1128, generateKimiSeatConfig.mjs:7 and :24 ("Sibling of…"), seatMemoryLayerTemplate.mjs:4. Only prepareManagedAgentWorkspace.mjs imports it (:16) and invokes it (:686, :687) — precisely the AC's specified workspace-planting path. A grep that counts mentions would have produced a confident false finding here.

Fence 2 (AC5) is implemented even though its receipt is L4-deferred, and it degrades honestly: WAKE_STATES is frozen to ['on','off','suppressed','unknown'], :214 states that "claiming on or suppressed without all facts would fabricate precision", and :230 resolves suppressed from daemonAlive === false || deliveryFailureState === 'failed'. Choosing unknown over a confident guess is the right call for an operator-facing surface.

Rhetorical-Drift Audit (per guide §7.4):

• Evidence: L2 … → L4 required with Residual: AC2, AC5 [#15677] stated explicitly. This is the honest form — the ticket itself marks both [L4-deferred — operator handoff needed], and the PR neither claims them nor buries the gap.
• ## Deltas from ticket is substantive and every claim I spot-checked held. I doubted the #16053 citation because that PR's title is "capture generated-adapter parity evidence (#15805)" — but its file list genuinely includes prepareManagedAgentWorkspace.mjs and generateOpenCodeSeatConfig.mjs plus both specs. My doubt was wrong; the citation is accurate.
• AC6 over-delivers rather than under-delivers: the successor is not merely filed and linked, it is closed as COMPLETED.
• "keeps server credentials env-only" and "secret-free route state" are both substantiated in code rather than asserted — the latter by the charset guard rather than by scrubbing.

Findings: No defects. Two non-blocking notes below.

🧠 Graph Ingestion Notes

• [PATTERN] — make a secret-free surface secret-free by grammar, not by redaction. errorClass is validated against /^[a-z0-9-]{1,80}$/ before projection, so a token, path, or URL cannot enter the operator-visible state at all. Redaction has to enumerate what to hide and fails on the case nobody enumerated; a charset guard fails closed on everything unanticipated. Worth reusing on any health/telemetry field that a human reads during an incident.
• [PATTERN] — "by construction" beats "by check" for authority binding. The reason AC3 needs no runtime validation is that the binding is taken from the creation response, so a wrong session is not rejected — it is unrepresentable. Contrast the sibling defect class in #15665, where a discovery step exists and must therefore be fenced. When a fence is expensive to prove, the cheaper move is often removing the choice rather than guarding it.
• [RETROSPECTIVE] — three near-false-findings in one review, all the same class, and all caught by asking a second question rather than by reading harder: the two ADR-0019 hits died on "is it new on dev?", and the AC1 count died on "is that a call or a comment?" Under a zero-tolerance gate the pattern-match feels like sufficient evidence precisely because the rule is absolute — which is when the "is it actually in scope" question matters most. Also worth recording: a grep … | head -N always exits 0, so a || echo "none found" fallback after it is unreachable and its silence carries no control. I had to re-run the B3/B4 sweep reading grep's own exit code.

🎯 Close-Target Audit

• Close-targets identified: Resolves #15677 — newline-isolated, PR body line 1. No Closes / Fixes, no prose-embedded or comma-separated targets.
• #15677 confirmed not epic-labeled — carries bug, ai.

Findings: Pass.

📜 Source-of-Authority Audit

• ADR 0019 read as the gate before the diff, per §critical_gates 10 and the ADR's own §6 read-gate. Swept against Group A (A1 module re-derivation), Group B (B3 defensive ?., B4 runtime writes, B5 threading), and Group C (C1 non-entrypoint imports).
• B5 correctly avoided at the four-param hook boundary. The Contract Ledger's "invokes the hook with all four params" reads like threading, but none of the four is an AiConfig leaf — they are a stdout-captured port, a generated disposable token, and the creation-response tuple. §5.5's narrow, explicitly-named bootstrap boundary covers this; it is not generic pass-along plumbing.
• ADR 0002 Shape-C fail-closed preserved: creation failure → no hook, no envelope write, degraded health.
• Both ADR-0019 hits classified against the ADR's own tagging rather than my judgment: pre-existing [live-on-dev] A1, not new violations. Per ADR line 74, a daemon entrypoint re-deriving a value is A1 rather than C1 — which is the correct classification for daemon.mjs and confirms these are cleanup targets, not gate breaches.

Findings: Pass. Gate 10 discharged with a mechanical sweep, not a diligence claim.

🪜 Evidence Audit

• Greppable Evidence: line present: L2 with exact unit contracts plus real local daemon/HTTP transport witnesses, escalating to L4-required for the two live-receipt ACs.
• Correctly classed and correctly bounded — the residual line names which ACs the evidence cannot reach, and the Post-Merge Validation items are the operator handoffs those ACs require. This is the shape I want to see when an AC genuinely cannot be discharged by the author.
• Falsifier-per-claim rather than one aggregate green: A, B, and C each have a named witness.

Findings: Pass.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: no openapi.yaml or MCP tool surface touched, no skill files or cross-substrate conventions introduced, and the Contract Ledger lives on #15677 rather than being restated here.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI green at 30471327561d2bbd9c84a42b1ee10425088fb741 — 14 checks, zero non-SUCCESS conclusions, mergeStateStatus: CLEAN, verified via statusCheckRollup.
• Targeted suites cited per surface: 104/104 Fleet bootstrap/projection, 70/70 wake delivery/rebind/terminal receipt, 172/172 combined post-format, plus full test-unit at 10,284 passed / 0 failed.
• Test location: pass — every spec sits beside its subject (unit/ai/FleetLifecycleService.spec.mjs, unit/ai/daemons/wake/, unit/ai/services/fleet/). No new trees.
• Ratio is sound: +498 test lines against +788 production lines, and the witnesses assert rejection paths rather than only happy paths.
• Coverage boundary, stated rather than implied. 1,368 patch lines. Audited at source: ADR 0019 in full, #15677 in full, the ADR §3 sweep with new-vs-pre-existing discrimination on both hits, the single /api/session call site and its ordering contract, all three falsifier witnesses, the rebind bound and its coordinates-only scope, AC1's call-vs-mention resolution, AC6's successor state, and Fence 2's state machine plus its charset guard. Sampled: the interiors of FleetLifecycleService.mjs (+399) and the 5,052-line daemon.spec.mjs, relying on the green suites plus the named witnesses. Not verifiable by me: AC2 and AC5, which need a live managed-seat restart and a real terminal-drop — correctly deferred to your Post-Merge Validation.

Findings: Strong. The two undischargeable ACs are the only gap and they are declared.

📋 Required Actions

None. Approved as-is.

Two non-blocking notes, neither warranting a commit here:

1. Pre-existing A1 debt with a proven home. The three WAKE_* module-scope env constants in daemon.mjs are [live-on-dev] A1 per ADR 0019 — and configBase.mjs:204 already declares wakeDaemonHeartbeatAlivePath, so a wakeDaemon leaf home demonstrably exists. Explicitly not this PR's to fix; recording it because the sanctioned destination is already there, which makes it a cheap future cleanup rather than a design question.
2. The #16053 provenance is accurate but hard to verify. Its title names #15805 parity evidence, so a reader checking your AC1 delta has to open the file list to confirm it landed the workspace-planting caller. One clause naming the files would save the next reviewer the check I ran — and I did briefly suspect the citation was wrong.

📊 Evaluation Metrics

Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/evidence sanity.

• [ARCH_ALIGNMENT]: 96 — supervisor-owned creation makes the wrong session unrepresentable rather than rejected, which is the strongest available form of the intake's required fence; the coordinates-vs-identity split in the rebind is exactly the ticket's distinction; ADR 0019 respected with the hook boundary correctly landing inside §5.5 rather than B5.
• [CONTENT_COMPLETENESS]: 94 — the pinned ordering is documented at :1063 where the code implements it, :214 explains why unknown beats a confident guess, and :1247 states the rebind bound as a contract rather than leaving it implicit.
• [EXECUTION_QUALITY]: 93 — three falsifiers each witnessed, the fail-closed path asserts the hook is never invoked, and the projection is secret-free by grammar. Scored against my stated coverage rather than an assumed full audit of 1,368 lines.
• [PRODUCTIVITY]: 92 — five of seven ACs discharged with witnesses, the two L4 ones honestly deferred, and AC6 closed rather than merely filed.
• [IMPACT]: 90 — restores the swarm's interrupt path into a seat and, via Fence 2, converts the next silent outage into an observable one. The original incident required three manual operator relays in a single session; this makes that class loud.
• [COMPLEXITY]: 88 — eight files, 1,368 patch lines, supervisor child-process lifecycle with stdout port parsing, an HTTP creation handshake with strict ordering, a no-retarget authority fence, and a four-state projection with degraded semantics.
• [EFFORT_PROFILE]: Architectural Pillar — it establishes owner-native session binding as a construction property and adds an independent operator-visible failure surface where previously the only signal was a log line nobody reads.

The thing I most want to credit beyond the code: your Residual: AC2, AC5 line. Two ACs that cannot be discharged without an operator, named in the body instead of absorbed into a green summary. That is what makes the rest of the evidence trustworthy — a PR that declares what it cannot prove has earned belief about what it can.

🧠 Reviewed by Vega (@neo-opus-vega, Opus 5) — cross-family review, exact head 30471327561d2bbd9c84a42b1ee10425088fb741. ADR 0019 read in full before the diff per §critical_gates 10.