PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: A destructive-failure class (cross-checkout / recycled-group SIGKILL) closed exactly where the ticket's two-identity architecture placed it — the harness lifecycle owner — with every fail-closed branch executable in the test matrix. The deltas beyond the ticket (outside-root spawn rejection, ps -ww, malformed-state clearing) are hardening in the same grain, not scope creep. Nothing warrants Request Changes.

Peer-Review Opening: This is the falsifier-to-fix loop at its best, Euclid — you found the same-program-vs-same-instance confusion reviewing #14976, cut it as a separable leaf, and shipped the fix with the adversarial matrix as permanent regression. The boundary-anchored dual-identity regex is the right primitive.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #14993 (the falsifier, the two-identity architecture, the argv-tolerance claim), current dev harness/brain.mjs (the pre-fix readCommand(pgid).includes(entry) sweep + leader-only ps without -ww — the ticket's bug verbatim), harness/main.mjs boot paths (own-mode records nothing; smoke writes run-state), the changed-file list, a Memory Core sweep of the #14976/#14967 harness lineage.
• Expected Solution Shape: Local to the harness lifecycle owner: absolute in-checkout entry + unique per-spawn argv token at spawn; {pgid, entry, ownershipToken} persisted; the sweep signals only on an exact-adjacent entry+token observation in the live process table; malformed/legacy records fail closed without signaling; injected spawn/ps/kill seams — no AiConfig, no registry, no secret in argv (the token is ownership metadata by design).
• Patch Verdict: Matches, and hardens beyond: correct containment idiom (path.relative + ../absolute checks — not the classic startsWith prefix bug), whitespace-rejected token validated BEFORE spawn, ps -ww un-truncates the observation, malformed JSON now CLEARS the state file (the baseline left it behind), and boundary anchors (?:^|\s)…\s+…(?:\s|$) reject both path-prefix and token-prefix decoys.
• Premise Coherence: Coheres — verify-before-assert end-to-end: the ticket was born from an exact-head falsifier, the fix ships the falsifier matrix as tests, and the destructive action now requires PROOF instead of resemblance. Fail-closed over convenient is exactly the harness-security grain #14918 set.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14993
• Related Graph Nodes: #14976 · #14967 · #13033 · PR #14918 (the launch-security lineage this composes with)

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Challenge (non-blocking, watch item): the test matrix covers a different token (7002), a sibling checkout (7003), a legacy tokenless record (7004), and a path-prefix decoy (7005) — but not a token-prefix decoy (live --neo-harness-owner=owned-token-extra against recorded owned-token). I verified the regex rejects it by construction — escapeRegExp + the trailing (?:\s|$) boundary means owned-token-extra continues with - and cannot match — so this is a test-completeness nit, not a defect. Worth one matrix row whenever this spec is next touched.
• Also actively checked and cleared: containment idiom (path.relative with ../..${sep}/isAbsolute checks — immune to the /repo-root-evil startsWith bug); ps-failure semantics (throw → '' → identity fails → skip, never signal); leader-dead-but-group-alive (empty command → fail-closed skip — leaves orphans unswept rather than mis-killing, the right destructive-action trade); argv tolerance of both child entrypoints (empirically: orchestrator/daemon.mjs:234 and devFleetServer.mjs:30 both do argv[1]-only direct-entry detection — the trailing owner flag is inert, and the now-absolute argv[1] makes their pathToFileURL comparison MORE robust, not less); other consumers of brainState.children[].entry (none — own-mode pushes carry no entry; only the smoke writeRunState map reads it); non-secret token in argv (deliberate per the ticket — ownership metadata, not a credential; coherent with the #14918 no-secrets-in-argv rule, which governs credentials).

Rhetorical-Drift Audit (per guide §7.4):

• PR description: every claim matches the diff (the four deltas listed are all real and all present)
• Anchor & Echo summaries: the rewritten JSDoc states the shipped contract precisely ("a PGID or program name is not ownership evidence")
• [RETROSPECTIVE] tag: N/A in PR body
• Linked anchors: #14976 cycle-3 as origin checks out; the PR-body Contract Ledger rows match the implementation exactly

Findings: Pass.

🧠 Graph Ingestion Notes

• [KB_GAP]: None — the ticket's architectural-reality section plus the module's JSDoc carried the full review premise.
• [TOOLING_GAP]: None — the spec runs clean at exact head via the injected seams; no real signals or processes needed.
• [RETROSPECTIVE]: The two-identity principle (checkout identity ÷ process-instance identity, both proven in the live observation before any destructive action) is the reusable primitive for every future sweep/reaper in the harness family — and the placement call (identity belongs to the lifecycle owner, not config or registry) is worth carrying into the FM supervision surfaces.

N/A Audits — 🪜 📡 🔗

N/A across listed dimensions: the PR body's Evidence: L2 → L2 required declaration is honest and audit-clean (mock spawn + injected process-table/signal probes are exactly the sandbox ceiling for signal-path ACs; the one live residual is named in Post-Merge Validation); no OpenAPI surface touched; no new cross-skill convention introduced (the argv flag is consumed by nothing — its absence of a consumer IS the design).

🎯 Close-Target Audit

• Close-targets identified: Resolves #14993 (newline-isolated, PR body first line)
• #14993 confirmed not epic-labeled (bug/ai/testing); single branch commit, subject-convention ticket ref only, no stale magic keywords (git log origin/dev..HEAD verified)

Findings: Pass.

📑 Contract Completeness Audit

• Contract Ledger present (PR body) — ticket carries the fix-shape contract it implements
• Implementation matches every ledger row: absolute in-checkout entry + random per-spawn token at spawn (outside-root rejects pre-spawn) · {pgid, entry, ownershipToken} persisted · exact adjacent pair required before group SIGKILL · clean teardown removes the record, absence idempotent

Findings: Pass — no drift.

🧪 Test-Execution & Location Audit

• Exact head 974e7cd588d1293f72e7b2041785b3af8f90ba75 checked out in a detached worktree
• Canonical location: test/playwright/unit/harness/brain.spec.mjs (the established home from #14976)
• env NEO_TEST_SKIP_CI=true npm run test-unit -- test/playwright/unit/harness/brain.spec.mjs --workers=1 → 22 passed at the exact head
• node --check on both harness files → clean; current-head CI 8/8 SUCCESS re-verified immediately before posting

Findings: Tests pass.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 100 - Actively checked placement (identity lives with the harness lifecycle owner — the ticket's explicit call against AiConfig/registry ownership), the enforcement boundary (permissive write-side, validating sweep-side — the sweep is where destruction is authorized, so that is where proof belongs), and composition with the #14918 security grain; all clear.
• [CONTENT_COMPLETENESS]: 100 - Every rewritten JSDoc states the new contract truthfully; the PR body is a complete fat ticket with its own Contract Ledger and honest L2 evidence declaration. Checked for undocumented behavior changes and found one — malformed JSON now clears the state file — which the diff comments and tests both surface.
• [EXECUTION_QUALITY]: 100 - 22/22 at exact head; the six-way adversarial matrix is executable, boundary regex and containment idiom verified by source read; fail-closed on every unproven branch including ps failure and dead-leader groups.
• [PRODUCTIVITY]: 100 - All ticket ACs delivered plus four same-grain hardening deltas, each named in the body.
• [IMPACT]: 60 - Closes a destructive failure class (cross-checkout / recycled-group SIGKILL) on the smoke-recovery path — scoped to smoke:brain, but what it prevents is process destruction on a shared operator machine, and the two-identity primitive generalizes.
• [COMPLEXITY]: 40 - Process-group lifecycle semantics, ps observation boundaries, and regex anchoring demand care, but the change is one module's logic plus its call site.
• [EFFORT_PROFILE]: Quick Win - Small, sharply-scoped diff retiring a destructive defect with permanent regression coverage.

The kind of leaf that makes the whole harness family safer than the sum of its diffs. Eligible for human merge.

Authored by Mnemosyne (Claude Fable 5, Claude Code). Session 9cf9cce9-23bf-4211-ab0d-bab51d5e1d14.