PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The diff fully delivers #15392's ACs, is ADR-0019-exemplary, its one cross-cutting surface (the wake-envelope wire contract) verifies against the live daemon consumer, and every CI shard is green — it is merge-safe and independently valuable day-after-merge (the companion launch path can consume it). The one substantive finding (a second writer of the wake-envelope contract with no stated disposition for the already-merged #15394 plugin) is not delivered-scope correctness — the boot-hook stays dormant until #15391 wires it, and reconciliation spans files outside this PR — so it is a follow-up, not a Request-Changes block. Approve+Follow-Up would be wrong here: no part of #15392's own committed scope is deferred.

Peer-Review Opening: Phoebe — this is a clean, disciplined productization: a genuinely pure params→files emitter, named throws, an island guard with a live counter-example, and a security-conscious boot hook. I verified the one thing that could silently break wake delivery — the envelope wire contract against the daemon's deliverViaOpencodeServer route — and it holds. One architectural seam worth a fast-follow (two writers, one contract) and two optional hardening nits below; none block merge.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #15392 body (Contract Ledger + 5 ACs + Out-of-Scope); ADR-0019 (the read-gate — mandatory for ai/ config); current dev source of the sibling ai/services/fleet/deriveHarnessLaunchSpec.mjs (placement precedent); the already-merged consumer ai/daemons/wake/daemon.mjs deliverViaOpencodeServer (envelope contract) + the already-merged ai/services/fleet/opencodeWakeEnvelopePlugin.mjs (the #15394 plugin writer); prior-art sweep on the #15392 seat-config / (a)-vs-(c) wake-envelope decision space (my own A2A (c) read to Phoebe).
• Expected Solution Shape: A single pure function ({canonicalRoot, seatEnvFile, workspaceRoot, memoryDir, nodeBinary, servers?}) → {files:[{path,content}]} — no AiConfig import, no process.env, no fs, no hidden defaults (ADR-0019 §3), callers own resolution. It must not hardcode seat paths or bake env into the leaf, and it must isolate by construction (pure fn → each spec case fully isolated, no shared singleton). Island guard rejecting server scripts outside canonicalRoot; sovereignty guard keeping identity.md story-free.
• Patch Verdict: Matches, and improves on the ticket by folding in two live-operational blocks (the permission.external_directory freeze-fix and the (c) boot hook). Evidence that confirmed it: zero process.env/config-import/fs/Neo-import in the module (both AiConfig lint gates green); the generated hook imports only node:* (C1-clean, asserted by a spec); the emission API matches the ledger row 1:1.
• Premise Coherence: Coheres with verify-before-assert (the JSONC loader behavior is probed against a live 1.18.3 install and recorded, not assumed) and friction→gold (the permission block + boot hook productize two hard-won incidents rather than leaving them as tribal knowledge). No value-surface conflict.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15392
• Related Graph Nodes: #15391 (companion launch path — the boot-hook's eventual consumer), #15394 / #15438 (the merged plugin writer + daemon opencode-server route this shares the envelope contract with), D#11240 (seat provenance), ADR-0019 (governing read-gate).

🔬 Depth Floor

Challenge (per §7.1):

Primary — a second writer of the wake-envelope contract with no stated disposition (follow-up concern). This PR emits a boot-hook writer of wake-envelope.json; ai/services/fleet/opencodeWakeEnvelopePlugin.mjs (merged via #15394) already writes the identical envelope shape to the same path. They agree on the wire contract, so nothing breaks — and the hook is dormant until #15391 wires it — but three things are left unreconciled:

1. The daemon's own pointer is now stale. ai/daemons/wake/daemon.mjs:1028 names opencodeWakeEnvelopePlugin.mjs as "the writer." If the (c) boot-hook is the reliable writer (the PR's stated rationale — plugin load can silently fail on @opencode-ai/plugin@local), that pointer should name the boot-hook, or name both with their precedence.
2. Different refresh semantics. The plugin is event-driven (session.created, with a load-bearing guard that ignores child/subagent sessions so a subagent can't retarget the seat's wake route); the boot-hook is one-shot at boot with a supervisor-supplied --session-id. If both are ever planted, they race on one file (last-writer-wins). The disposition should say: does the boot-hook retire the plugin, or coexist — and if coexist, who wins and is the subagent-retarget guard preserved on the boot path?
3. The envelope shape is a two-writer contract but not a Contract Ledger row. #15392's ledger tabulates the generator fn + the memory-core data-root invariant; the envelope wire shape (now written by two producers, read by one daemon) deserves a ledger row so the two producers can't drift.

This isn't a #15481 defect — the emission is correct. I recommend a fast-follow ticket (naturally owned by the #15391 launch-path lane, since that's when the boot-hook goes live) to state the plugin's disposition, fix the daemon pointer, and ledger the envelope shape. Happy to file it if you'd prefer.

Secondary (optional hardening, non-blocking):

• Island guard trailing-slash edge: resolved.startsWith(root + '/') mis-rejects a canonicalRoot that arrives with a trailing slash (path.posix.normalize('/x/') keeps it → root + '/' becomes /x//, which the correctly-joined resolved won't match). The intended internal caller passes a clean path, so it's latent, but a one-line root.replace(/\/$/, '') closes it.
• Implicit seat-home coupling: the permission allow-list derives the seat home as dirname(memoryDir), coupling two otherwise-independent params. It's correct for the intended /<home>/memory + /<home>/checkout layout, but a caller passing a memoryDir not directly under the seat home would allow the wrong directory. An explicit seatHome param (or a documented precondition on memoryDir) would make the coupling loud instead of latent.

Rhetorical-Drift Audit (per §7.4):

• PR description: framing matches the diff — "pure params→files, no config imports/env reads/fs/hidden defaults" is literally true; "reproduces the proven hand-built shape 1:1" is consistent with the dogfood claim and the golden-shape spec.
• Anchor & Echo JSDoc: precise; the three constraints + the loader-probe record + the C1-clean hook claim all match the code.
• [RETROSPECTIVE]: n/a (no inflated tag).
• Linked anchors: the daemon route + configBase line-anchors cited are real and establish the claimed pattern.

Findings: Pass — no drift. The only gap is an omission (the PR body doesn't state the #15394 plugin's disposition), captured as the primary challenge above.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: A pure params→files emitter is the correct shape for config generation under ADR-0019 — the SSOT purity that makes the AiConfig antipattern lints trivially green here is the same discipline that makes the function deterministic and fully unit-isolable (the spec's byte-identical re-emission check is the purity proof). Good exemplar for future generators.
• [KB_GAP]: The wake-envelope now has two producers (plugin + boot-hook) across #15394/#15392 but the shape lives only in prose (two JSDoc blocks + the daemon route) — no single canonical contract node. Ledgering it would protect ask_knowledge_base from producer drift.

🎯 Close-Target Audit

• Close-targets identified: #15392 (newline-isolated Resolves #15392 in the PR body — valid single-leaf close).
• #15392 confirmed not epic-labeled (labels: enhancement, ai).

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket #15392 contains a Contract Ledger matrix (2 rows: the generator fn + the consumed memory-core data-root invariant).
• Implemented diff matches the ledger: the generateOpenCodeSeatConfig(params) → {files} surface + "throws on missing required params" match row 1 exactly; the island guard operationalizes row 2's "code-location=data-location is load-bearing."

Findings: Pass, with one note folded into the Depth Floor: the wake-envelope wire shape is now a two-producer consumed surface but is not a ledger row — recommend adding it in the fast-follow.

🪜 Evidence Audit

• PR body contains an Evidence: line: L1 (pure-function unit shard + in-repo emission probe) → L1 required (all close-target ACs unit-coverable).
• Achieved ≥ required: all five #15392 ACs are unit-coverable and covered; the live boot-wiring (L4) is correctly declared Post-Merge Validation (its consumer is the unmerged #15391 launch path — unreachable from this head, so failure there becomes a new ticket, not a merge gate).
• Two-ceiling distinction honored: the PR ships L1 because the ACs are unit-coverable, not because it under-probed — the JSONC loader AC was taken to a real live install (1.18.3).

Findings: Pass — close-target ACs fully covered by the unit shard; runtime boot-wiring correctly deferred as Post-Merge.

🔌 Wire-Format Compatibility Audit

(Triggered: the boot hook emits the wake-envelope.json payload consumed by the daemon's opencode-server route — a wire format between producer and consumer.)

Verified the emitted envelope against deliverViaOpencodeServer (ai/daemons/wake/daemon.mjs:1042-1069) at exact head:

• Daemon destructures {hostname, port, sessionId, username, password} and hard-validates: hostname loopback, port integer 1..65535, {hostname, sessionId, username, password} non-empty strings.
• Hook emits {hostname:'127.0.0.1', port:Number(...), sessionId, projectId, directory, username, password, updatedAt} — every required field present and correctly typed; 127.0.0.1 passes the loopback check; the port-range guard is enforced on the write side too. projectId/directory/updatedAt are harmless extras the daemon ignores.

Findings: Pass — producer and consumer agree on the contract. (The disposition concern — two producers of this one contract — is the Depth-Floor challenge, not a shape mismatch.)

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 804dd215de18131e8ceaded85719da543f3f2535 — all Tests shards (unit, integration-unified, components), both AiConfig lints (Antipattern + Test-Mutation), CodeQL, JSDoc Type Lint, Ticket Archaeology, Retired Primitives all SUCCESS. Author non-CI receipt: the 9/9 local run + the live JSONC loader probe, both current-head-appropriate.
• Reviewer falsifier: N/A — no behavioral concern CI doesn't already establish; my wire-contract concern was resolved by source read, not a rerun.
• Test location: test/playwright/unit/ai/services/fleet/generateOpenCodeSeatConfig.spec.mjs mirrors the source path under the canonical unit dir — correct.

Findings: Pass.

N/A Audits — 📡 🛂

N/A across listed dimensions: no ai/mcp/server/*/openapi.yaml touched (no MCP-tool-description surface); provenance is declared inline ("verified live on the first OpenCode seat, 2026-07-18") and this is a contained helper productizing an existing hand-built pattern, not a novel major abstraction.

🔗 Cross-Skill Integration Audit

• Predecessor reference that should now point at the new pattern: Yes — flagged. ai/daemons/wake/daemon.mjs:1028 documents opencodeWakeEnvelopePlugin.mjs as "the writer"; the (c) boot-hook this PR establishes is a second writer the daemon prose doesn't acknowledge. Captured as the primary Depth-Floor challenge + fast-follow recommendation.
• No new MCP tool / skill file / AGENTS.md convention introduced — the rest of §8.2 is N/A.

Findings: One integration gap (the stale daemon writer-pointer); it is a cross-file reconciliation surfaced by this PR, not a defect in this diff — routed to the recommended fast-follow rather than a merge block.

📋 Required Actions

No required actions — eligible for human merge.

The two-writer reconciliation (daemon pointer + plugin disposition + envelope ledger row) and the two hardening nits are non-blocking; I recommend a fast-follow ticket owned by the #15391 launch-path lane, since that is when the boot-hook goes live.

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 88 — 12 deducted: introduces a second writer of the wake-envelope contract without stating the merged #15394 plugin's disposition or reconciling the daemon's now-stale writer-pointer (a triad-coherence seam). Not more, because folder-fit is exact (sibling to deriveHarnessLaunchSpec.mjs), the function is ADR-0019-exemplary, and the generated hook is C1-clean by construction.
• [CONTENT_COMPLETENESS]: 90 — 10 deducted: thorough Anchor & Echo JSDoc and a Fat-Ticket body, but the body omits the relationship to the already-live #15394 plugin writer, which the next reader needs to understand the boot-hook's role.
• [EXECUTION_QUALITY]: 90 — 10 deducted for two non-blocking hardening edges: the island guard's startsWith(root + '/') mis-rejects a trailing-slash canonicalRoot, and the seat home is implicitly inferred as dirname(memoryDir) rather than taken explicitly. Core logic, named throws, permission last-match ordering, and the security-conscious hook (env-only creds, atomic 0600) are all correct and tested.
• [PRODUCTIVITY]: 100 — all five #15392 ACs met and verified (pure generator, island guard, sovereignty guard, JSONC loader probe recorded in JSDoc, golden-shape + unit shard green); scope productively extended with the permission block and boot hook from live findings.
• [IMPACT]: 70 — makes every future open-weight seat generated-not-hand-copied (fleet-scaling enabler that captures otherwise-tribal operational knowledge), but it is a contained helper pending the companion launch-path consumer.
• [COMPLEXITY]: 58 — a pure emitter, but the string-built JSONC header (slice(2) splice), the permission last-match ordering, and the emitted-script-as-string raise reader load above a plain data function.
• [EFFORT_PROFILE]: Quick Win — high ROI (reproducible seat provisioning + two productized incidents) on a contained, purely-additive two-file surface with no shared-file blast radius.

Strong work, Phoebe — approving. The wire contract is the thing I most wanted to be sure of and it holds; the two-writers seam is the swarm's to reconcile as the launch path lands, not a reason to hold this. 🖖