PR Review Summary

Status: Approved

Posted as COMMENT per identityRoots (same-family for Claude-authored PRs); operator gate per the night's pattern. Night-shift marker: single-family — calibration-deferred-to-merge-gate.

🪜 Strategic-Fit Decision

• Decision: Approve
• Rationale: Implements the architecture call from the lane handoff exactly, and both deviations from my reference hunk are deployment-correct improvements, not drift. This closes the last surface of #12859's deployment story.

Peer-Review Opening: Shipped nine minutes after a context compaction, off the recovery anchor, with zero lane confusion — the PR is good and the provenance chain it rode is better.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: my own architecture call + reference hunk (I prescribed this fix in the lane handoff: the FLAG, not an embed-daemon service); docker-compose.test.yml's mc-server blocks (mine, from #12859); both canonical composes at the flagged lines; the inProcessDrain leaf JSDoc + tonight's #12871 drain-lock (which makes the two-mode story mechanically safe); the doc surfaces I'd named as candidates.
• Expected Solution Shape: NEO_MEMORY_WAL_IN_PROCESS_DRAIN=true + cadence on mc-server in BOTH canonical composes, with the sole-drainer comment; no embed-daemon sidecar; docs only if they enumerate WAL env (verified: they don't — zero drift).
• Patch Verdict: Matches and improves. The two deviations from my test-compose reference are both right: (1) base pins NEO_MEMORY_WAL_DIR onto the persistent shared-sqlite-data volume — production pending segments MUST survive container recreates; my tmpfs shape is correct for ephemeral tests and would have been a data-loss bug here; (2) dev skips the override because the bind-mount already persists on the host. Per-environment durability reasoning, each documented in-place.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #12874
• Related Graph Nodes: #12859 (the decouple this completes), #12871 (the drain-lock making dual-host misconfig refuse loudly), the private-deployment validation thread (the empirical origin)

🔬 Depth Floor

Design question answered (the author flagged it for me): WAL-as-sqlite-volume-subdir vs dedicated volume — subdir is correct, endorsed: the WAL is the same durability class as the graph DB it sits beside, shares the same backup story (orchestrator bundles capture the volume), and a dedicated volume adds operational surface for zero benefit at this scale. Documented search: I actively checked (1) the oidc-variant question — the canonical base has ONE mc-server (unlike test.yml's mc + mc-oidc pair), so no second service needs the flag; (2) drain-lock interaction — the in-process loop claims .drain-lock inside the overridden WAL dir on the persistent volume, which is exactly where it should live (lock survives with the WAL it guards; a recreated container's dead-pid lock reclaims cleanly per #12871's stale path); (3) cadence — 250ms matches the integration matrix; for a production default it's aggressive but harmless (idle cycles are a single pending-read against a mostly-empty dir) and env-overridable. No concerns.

Rhetorical-Drift Audit: Pass — the body's "CI stayed green because test.yml drains" is an honest blind-spot admission, and the [RETROSPECTIVE]-worthy lesson: per-compose env divergence is structurally invisible to CI.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: Deployment-shape sweeps must cover ALL compose siblings (test/dev/base) — tonight's third instance of the fix-one-copy-miss-the-siblings class (roster files, timeout classes, now composes). And: env-var divergence between compose variants is invisible to CI by construction; the only detector is an empirical boot of the variant CI doesn't run.

N/A Audits — 🎯 📑 🪜 📡 🛂 🔌 🧪

N/A across dimensions: Resolves #12874 verified leaf + newline-isolated; compose-env-only change (no contract/OpenAPI/wire surface; no novel abstraction); no executable test surface — the author's L2 empirical full-stack boot (healthy stack, confirmed non-draining mc-server pre-fix) is the correct and sufficient evidence class, exceeding what unit CI could show for this change.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 100 - I actively considered sidecar-vs-flag (the call held), volume durability semantics (both deviations correct per environment), drain-lock interaction, and oidc-variant coverage — none violated.
• [CONTENT_COMPLETENESS]: 100 - In-place comments carry the symptom, the invariant, and the per-environment dir rationale; the PR body documents the empirical method and the design question explicitly.
• [EXECUTION_QUALITY]: 100 - Two files, exactly the needed lines, yaml-parse verified, CI green; the empirical pre-fix boot is the strongest evidence available for compose changes.
• [PRODUCTIVITY]: 100 - The ticket's single AC delivered; #12859's deployment story now complete across all four surfaces (test, base, dev, and the private stack pending its MR).
• [IMPACT]: 55 - Every deployment built from the canonical compose gets working semantic recall back — the public face of the v13 cloud chapter.
• [COMPLEXITY]: 15 - Minimal: env lines + comments; the depth was in the diagnosis, which the empirical boot already paid.
• [EFFORT_PROFILE]: Quick Win - Found, ticketed, fixed, and evidence-backed inside an hour — across a context compaction.

The decouple's deployment story closes. Eligible for human merge.