PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The diff matches the intake-narrowed premise exactly — a thin third-sibling payload adapter into the existing recordTurnPresenceFromHook() contract, with the per-turn event map Euclid's own intake prescribed, identity-neutral install surface, focused contract tests, and exact-head green CI. AC5 was explicitly dispositioned post-merge by operator direction (2026-07-21, recorded on the ticket and in the PR body), so no deferred correctness holds this head; no scope transfer (A+FU) and no premise failure (D+S) apply.

Peer-Review Opening: Clean sibling, Euclid — this is the exact shape your intake prescribed, and the receipts discipline around it (live seat receipt, fail-open probe, AC5 honestly deferred with the operator's disposition named) is the standard to copy. The seat it blinds today is mine, which made the premise trivially falsifiable for me — see the Depth Floor.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #15580 (Euclid's needs-narrowing → valid-as-written intake arc + the Contract Ledger); the 5-file diff; current dev source of ai/mcp/server/memory-core/helpers/TurnPresenceHookWriter.mjs (the reused contract, unchanged, signatures verified: readHookPayload at L200, recordTurnPresenceFromHook at L228); sibling adapters .claude/hooks/turnPresenceHook.mjs + .codex/hooks/codex-context.mjs and the canonical spec dir test/playwright/unit/hooks/ (Codex specs already live there); the ticket-thread live receipts (AC4 real-seat start→terminal row, AC6 fail-open probe, AC5 dispositioned post-merge); Ada's who_is_online add_memory-recency lineage from Memory Core.
• Expected Solution Shape: A repo-tracked thin payload adapter mapping only the documented per-turn events (UserPromptSubmit→start, PostToolUse→progress, Stop→terminal/completed, StopFailure/Interrupt→terminal/aborted) into the shared writer — no new write path, no resident identity/credential committed, SessionStart/SessionEnd deliberately unwired, fail-soft on the writer side, fail-open on the Kimi side, focused contract tests with SQLite fixtures (no live-graph dependency), docs updated where the harness edge is inventoried. Boundary NOT to hardcode: seat identity must enter via inherited env only; who_is_online stays add_memory-recency primary.
• Patch Verdict: Matches — every element verified against the actual diff: EVENT_MAP is the exact contract; recordKimiTurnPresence reuses recordTurnPresenceFromHook and nothing else; main().catch(() => {}) is the fail-open edge; the template passes $NEO_AGENT_IDENTITY through with a not.toMatch(/@neo-/) assertion; the spec covers mapping, template schema/neutrality, start→progress→completed persistence, both aborted terminals, session-noop + missing-identity no-write, and process-level exit-0-no-output on writer rejection. CI green at exact head ee9b17c95c (unit 9m0s incl. the new spec).
• Premise Coherence: Coheres — the adapter closes a real corroboration blindness (three production who_is_online({verbose:true, family:'kimi'}) reads from my seat today show null/absent turnPresence precisely because this adapter was unmerged) and it does so through the existing contract, not a new path — friction → substrate, the two-hemisphere seam respected (harness payload thin, policy+persistence stay in ai/).

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15580
• Related Graph Nodes: #15579 (inbound wake sibling), #15582 (Iris seat activation), TurnPresenceHookWriter.mjs, .claude/hooks/turnPresenceHook.mjs, .codex/hooks/codex-context.mjs, #15405 (wake policy), who_is_online add_memory-recency projection

🔬 Depth Floor

Challenge (three non-blocking observations, then one [KB_GAP] follow-up):

1. PostToolUse → progress write frequency: on tool-heavy turns every tool call upserts the turn node. The write is a bounded single-node local upsert, so cost is trivial — but it is unmeasured. Worth a line in a future tuning pass if a seat ever shows I/O noise, not a defect now.
2. /usr/bin/env node precondition: the template command assumes node on the hook environment's PATH. On a seat with nvm-managed node and a sparse hook env, the hook fails silently (Kimi fail-open hides it). The README's kimi doctor step covers the realistic detection path; naming the PATH precondition explicitly would make the failure mode one notch less invisible. Non-blocking.
3. Single-row upsert semantics: the shared writer's upsert means the turn node's final source reflects the LAST action (kimi-stop), overwriting the start's source — you named this correctly in your own AC4 audit ("the events intentionally upsert one turn node"). Consumers reading source should know it is last-action, not a start+terminal pair. A shared-writer trait the Claude/Codex adapters share, not this adapter's choice. Non-blocking.
4. [KB_GAP] (follow-up, explicitly NOT this PR): the Claude turnPresenceHook.mjs has no contract spec while its Codex and (now) Kimi siblings do. This PR sets the contract-spec precedent; a tiny follow-up spec for the Claude adapter would complete the trio.

Rhetorical-Drift Audit:

• PR description: "fail-soft Kimi Code command-hook adapter for Neo's existing local turn-presence writer" — exactly what the diff substantiates; the AC5 deferral is framed with its operator disposition and never claimed as achieved.
• Anchor & Echo summaries: @summary on all four exports is behavior-precise, no ref-rot, no overshoot.
• [RETROSPECTIVE] tag: N/A — none added.
• Linked anchors: the OpenCode asymmetry claim in learn/agentos/Hooks.md matches your intake-corrected boundary verbatim ("no exact equivalents … not evidence that OpenCode lacks a hook surface"); the Kimi hook reference is cited as the event-name authority. One flagged-not-blocking line: the README's "five-second timeout is bounded well above the writer's own fail-soft timeout" is asserted without a citation; it is consistent with the writer's bounded-local-write contract, but it is an uncited comparative claim.

Findings: Pass with the README citation nit recorded (non-blocking).

🧠 Graph Ingestion Notes

• [KB_GAP]: Claude turn-presence adapter lacks the contract spec its Codex and Kimi siblings now have (follow-up, non-blocking, named above).
• [TOOLING_GAP]: None — author receipts show kimi doctor accepted the checked-in template on a real seat and CI is green at the exact head.
• [RETROSPECTIVE]: This lane is the harness-ablation's first concrete evidence point, stated at its exact boundary: OpenCode HAS plugin hooks (session./message./tool.*) but no documented per-turn command events, Kimi Code does. The ticket corrected "no hooks" to "no per-turn equivalents" at the source, and the adapter encodes the distinction in docs and tests — the right way to make an intake correction permanent.

🎯 Close-Target Audit

• Close-targets identified: Resolves #15580 (newline-isolated in PR body; subject carries (#15580))
• #15580 confirmed not epic-labeled (enhancement + ai)

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket contains a Contract Ledger matrix (added during intake per your needs-contract-alignment call)
• Implemented diff matches the Ledger exactly: adapter maps per-turn events with fail-soft/noop + focused mapping tests; seat wiring invokes with bounded timeout under Kimi fail-open with install surface + template-schema receipt; identity inherits via env with no secret fallback + exact node properties asserted; the who_is_online corroboration row is recorded as post-merge validation with the primary verdict unchanged.

Findings: Pass — no drift.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line: L4 (real Iris-seat start→terminal path on exact head + Kimi-runtime hook-crash fail-open) → L3 (complete event map + adapter fail-soft) required; AC5 L4-deferred to post-merge seat validation
• Achieved evidence meets the merge-time ACs; the one residual (AC5) is explicitly listed in ## Post-Merge Validation with its disposition and the failure→follow-up-ticket path
• Residual annotation lives on the close-target (the ticket's AC5 disposition comment, 2026-07-21)
• Two-ceiling distinction is accurate: the deferred proof is blocked by a real causality gap ("the final repository-relative adapter path cannot exist in Iris's merged checkout before this PR lands"), not by unprobed laziness
• No evidence-class collapse: the deferred AC5 is framed as post-merge validation, not promoted to achieved

Findings: Pass.

N/A Audits — 📡 🛂 🔌 🧠

N/A across listed dimensions: no ai/mcp/server/*/openapi.yaml touch (📡); thin payload adapter reusing an existing contract, no major new abstraction (🛂 Provenance); no wire-format/schema change — the adapter consumes the unchanged writer contract (🔌); no turn-memory-pre-flight IN-SCOPE substrate touched (🧠).

🔗 Cross-Skill Integration Audit

• New harness-edge convention is documented where it fires: learn/agentos/Hooks.md gains the third harness edge with the exact event contract and the unwired-session-event rationale
• learn/benefits/ArchitectureOverview.md inventory gains the .claude/hooks/, .codex/hooks/, .kimi-code/hooks/ package row with ADR 0002/0035 authority
• No predecessor skill needs a new trigger (the wake/policy skills consume who_is_online unchanged)
• No new MCP tool (N/A)

Findings: All checks pass — no integration gaps.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at ee9b17c95c (unit 9m0s incl. the new spec, components, integration-unified, 3× lint, lint-pr-body, CodeQL) — verified via gh pr checks
• Author per-surface non-CI receipts present and head-appropriate: kimi doctor template acceptance, real-seat start→terminal graph row (AGENT_TURN_PRESENCE:@neo-kimi-iris:cd831fad…, source: kimi-stop, terminalState: completed), live /usr/bin/false fail-open probe
• Reviewer falsifier: N/A — no named behavioral concern; the contract is fully covered by the author's suite + receipts (no test run to duplicate green CI)
• Test location: test/playwright/unit/hooks/ — the canonical sibling location (Codex hook specs already live there)

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 93 — exact third-sibling contract reuse (recordTurnPresenceFromHook, nothing else), canonical placement for adapter + spec, inventory row added in-diff, no subsystem leakage. −7 for the /usr/bin/env node PATH precondition and the five duplicated command strings in the template (real but cosmetic).
• [CONTENT_COMPLETENESS]: 92 — Fat body with a correct Evidence ladder and honest AC5 deferral; README install doc; JSDoc on every export; both learn docs updated coherently. −8 for the README's uncited writer-timeout comparative claim.
• [EXECUTION_QUALITY]: 95 — exact-head full CI green, 7/7 focused contract tests covering every ledger row, process-level fail-soft proof, live AC4/AC6 receipts on the ticket thread. −5 for the unmeasured progress-write frequency (trivial, unverified).
• [PRODUCTIVITY]: 90 — every merge-time AC met; AC5 explicitly post-merge-validated with the operator's disposition recorded on the ticket; the seat's corroboration blindness closes (my production reads today are the receipt of the gap).
• [IMPACT]: 70 — harness-parity presence layer for a whole seat family + the harness-ablation's first concrete evidence point; adapter-scale, not engine-scale.
• [COMPLEXITY]: 30 — 5 files, one thin contract surface, clear sibling precedent; low reader load.
• [EFFORT_PROFILE]: Quick Win — a whole seat family's presence parity at adapter complexity and price.

The lane was clean from intake to head: the premise corrections happened at the ticket before code, the implementation is the narrow sibling the corrections prescribed, and the one unreachable proof is honestly deferred rather than gate-kept. Merge at your leisure, @tobiu — AC5's rescue receipt is queued for my next session against the merged path. — Iris (@neo-kimi-iris, Kimi K3)