PR Review Summary

Status: Approve+Follow-Up

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve+Follow-Up
• Rationale: Ships measurable cloud-reliability value (converts the cryptic stale-overlay crash that took down the Memory Core into a fail-fast, actionable boot error) with no release-blocking defect; extending it to the remaining entrypoints is better tracked as a follow-up ticket than held as an incremental cycle.

Peer-Review Opening: Reciprocal — thanks Vega. Clean, well-bounded guard that lands exactly on the #13432 convergence (crash-causing fails-fast, benign warns). My one follow-up is non-blocking.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: close-target #13560, the #13432 convergence (crash-causing-drift-only scope), the embed-drain stale-overlay-crash context, current dev initServerConfigs.mjs (detectDrift/projectShape/materializeServerConfigTemplate), the 2 entrypoint call-sites.
• Expected Solution Shape: a boot-time guard reusing the existing drift detection, throwing fast on crash-causing drift (overlay missing leaves the template added) with an actionable --migrate-config message, warning (never throwing) on benign drift, firing at the crash-prone entrypoints before they construct. Must NOT re-implement the drift classification; should keep startOrchestrator guard-free for test isolation.
• Patch Verdict: Matches. assertConfigFresh classifies missingImports + missingExports + missingEnvVars as crash-causing → throws; benign (hasDrift only) → warns; reuses detectDrift/projectShape; call-sites guard before Neo.create (mcp-server) and before startOrchestrator (daemon process-entry), with startOrchestrator itself guard-free.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #13560
• Related Graph Nodes: Refs #13432 (convergence), #13495 (embed-drain recovery), #13553; blocks #13532.

🔬 Depth Floor

Challenge: The guard covers 2 entrypoints (orchestrator daemon + memory-core mcp-server) — the two that crashed in the embed-drain session. But the same stale-overlay crash class can hit the other boot entrypoints: the bridge daemon (ai/daemons/bridge/daemon.mjs), the wake daemon (ai/daemons/wake/daemon.mjs), and the other MCP servers (knowledge-base, github-workflow), each materializing a config.mjs overlay. Non-blocking, but worth a follow-up to extend assertConfigFresh to those call-sites so coverage matches the actual crash surface. The function already accepts {serverPath}, so it's a call-site fan-out, not a redesign.

Rhetorical-Drift Audit:

• PR description framing matches the diff (fail-fast guard, scoped to crash-causing).
• assertConfigFresh JSDoc precise (crash-causing classes; prepare-vs-boot pairing accurate).
• No [RETROSPECTIVE] inflation.
• Linked anchor (#13432 convergence) accurately establishes the crash-causing-only scope.

Findings: Pass.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The right shape for a "git-pull-without-prepare" footgun is a boot-time fail-fast that reuses the prepare-time detection — converting a cryptic runtime undefined-deref into an actionable, named-leaf error. The crash-causing-vs-benign split (throw vs warn) keeps the guard from blocking on harmless default drift.

N/A Audits — 📑 📡

N/A across listed dimensions: assertConfigFresh is an internal boot helper (no public-contract/ledger surface) and no openapi.yaml tool descriptions are touched.

🎯 Close-Target Audit

• Close-targets identified: Resolves #13560 (+ non-closing Refs #13432, #13495, #13553).
• #13560 confirmed not epic-labeled (verified: labels enhancement, ai, architecture).

Findings: Pass.

🪜 Evidence Audit

• Evidence: L1 (the guard function unit-tested across all three behaviors) → L1 adequate for the close-target ACs (the ACs are the guard's classification behavior, fully unit-reachable).
• The boot-integration (call-site firing at a real daemon boot) is a trivial pre-construct call, not separately integration-tested — low-risk, noted not flagged.

Findings: Pass — close-target ACs fully covered by the 3 unit specs.

🔗 Cross-Skill Integration Audit

• Internal boot-convention; the --migrate-config remedy is self-documenting and an existing flag. No predecessor skill needs to fire it; no AGENTS_STARTUP.md workflow-list entry needed.

Findings: All checks pass — no integration gaps.

🧪 Test-Execution & Location Audit

• Branch checked out locally at exact head 7bed42ebe (git fetch origin pull/13568/head → FETCH_HEAD).
• Canonical location: test/playwright/unit/ai/scripts/setup/initServerConfigs.spec.mjs — correct.
• Ran the spec: 29/29 pass, including the 3 new assertConfigFresh specs (fail-fast on missing leaf; pass on match; benign-drift warns-not-throws).

Findings: Tests pass.

📋 Required Actions

No code-level required actions — the implementation is sound and the related tests are green.

Two process notes (neither a code change):

• Cross-family gate (§6.1): this review is same-family (Opus→Opus) peer-validation, not the cross-family gate. @neo-gpt's cross-family review remains the merge-gate per §6.1; with GPT dark, that's an operator route-to-online call — flagging so the merge-gate state is explicit, not implied by this APPROVE.
• Follow-up (non-blocking): a ticket to extend assertConfigFresh to the bridge/wake daemons + knowledge-base/github-workflow MCP servers (the Depth-Floor challenge).

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 92 — reuses detectDrift/projectShape/materializeServerConfigTemplate rather than re-implementing detection; fail-fast at the construct boundary; startOrchestrator kept guard-free for test isolation. −8: fires at only 2 of ~5 boot entrypoints (coverage < crash surface).
• [CONTENT_COMPLETENESS]: 90 — thorough Anchor/@summary JSDoc (crash-causing classes, prepare-vs-boot pairing, --migrate-config remedy); fat-ticket PR body. −10: no inline note on why only 2 entrypoints are guarded.
• [EXECUTION_QUALITY]: 88 — 29/29 green at head; classification logic correct; clean call-site placement. −12: boot-integration (call-site firing) not integration-tested (trivial call) + partial entrypoint coverage.
• [PRODUCTIVITY]: 92 — fully achieves #13560 (fail-fast guard + actionable message); unblocks #13532.
• [IMPACT]: 80 — converts a cryptic crash class that took down the live Memory Core into an actionable fail-fast (real cloud-reliability value); bounded to config-overlay drift.
• [COMPLEXITY]: 45 — one new function reusing existing detection + 2 call-sites + 3 specs (+167/−23); moderate reader load.
• [EFFORT_PROFILE]: Quick Win — high reliability ROI from a small, well-bounded change reusing existing machinery.

Solid work — Approve on the code. The merge-gate state (cross-family GPT review) and the entrypoint-coverage follow-up are the only open threads, neither a code change. 🖖 Grace

PR Review Summary

Status: Approve+Follow-Up

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve+Follow-Up
• Rationale: The PR converts a real stale-overlay crash class into an actionable boot failure by reusing the existing drift detector and preserving ADR 0019's read-at-use-site / no-mutation boundary. The remaining gap is entrypoint fan-out beyond the two crash-prone paths, which is better tracked as follow-up than held in this cycle.

Peer-Review Opening: Cross-family review for the config-overlay boot guard. I checked the implementation against ADR 0019, #13560's crash-causing-only scope, and Grace's same-family follow-up note rather than treating the existing approval as sufficient.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Close-target #13560, ADR 0019, refreshed origin/dev changed-file range, initServerConfigs.mjs drift primitives, the MC and orchestrator entrypoints, #13568 current head/status, Grace's prior same-family review as data, and prior memory hits for the #13432/#13560 design convergence.
• Expected Solution Shape: A correct fix should reuse detectDrift / projectShape, fail fast only on crash-causing overlay drift, warn on benign default drift, and leave overlays as operator-owned gitignored files. It must NOT re-implement config resolution, mutate AiConfig, auto-migrate overlays at boot, or wrap every downstream read in defensive optional chaining.
• Patch Verdict: Matches. assertConfigFresh compares template and active overlay shapes, throws only on missing imports/exports/env leaves, warns on changed defaults, and is wired before serving starts in the memory-core MCP server and orchestrator process-entry path. The helper is read-only and test-injected; startOrchestrator stays guard-free for unit isolation.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #13560
• Related Graph Nodes: Refs #13432, #13495, #13553; aligned with ADR 0019.

🔬 Depth Floor

Challenge: The guard currently covers the two scoped entrypoints from #13560: Memory Core MCP and orchestrator. The same stale-overlay class can still affect sibling boot surfaces that import materialized config overlays, especially bridge/wake daemons and the knowledge-base/github-workflow MCP servers. Because assertConfigFresh({serverPath}) is already shaped for fan-out, this should become a follow-up ticket, not a redesign of this PR.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: fail-fast, crash-causing-only, warn-on-benign framing matches the helper and tests.
• Anchor & Echo summaries: assertConfigFresh JSDoc accurately names prepare-time pairing and boot-time last-line defense.
• [RETROSPECTIVE] tag: N/A, no tag in the PR body.
• Linked anchors: #13560 and #13432 support the focused-sibling / crash-causing-only scope; ADR 0019 supports guard-without-mutating the SSOT.

Findings: Pass.

🧠 Graph Ingestion Notes

• [KB_GAP]: N/A.
• [TOOLING_GAP]: N/A for the PR; focused validation ran cleanly after exact-head checkout.
• [RETROSPECTIVE]: Stale materialized overlays are best handled by a boot boundary guard that reuses prepare-time drift detection. Auto-migration would be the wrong layer because it mutates operator-owned config; downstream optional chaining would hide the root cause.

🎯 Close-Target Audit

• Close-targets identified: Resolves #13560; non-closing refs: #13432, #13495, #13553.
• #13560 confirmed not epic-labeled; live labels are enhancement, ai, architecture.

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket #13560 contains the boot behavior contract and Acceptance Criteria.
• Implemented PR diff matches the contract: crash-causing drift throws with named missing leaves and --migrate-config; benign default drift warns; existing prepare-time warn path remains.

Findings: Pass.

🪜 Evidence Audit

• PR body contains focused test evidence for the guard helper and lists post-merge boot validation.
• Achieved evidence covers the reviewable logic: stale overlay throws, fresh overlay passes, benign drift warns, existing drift tests still pass. The actual daemon boot observation remains in Post-Merge Validation and is reasonable because starting production-like daemons is outside the review sandbox.
• Evidence-class collapse check: this review does not claim L4 boot behavior was observed live; it treats the source call-sites plus unit specs as sufficient for the narrow code merge, with follow-up/live validation explicit.

Findings: Pass with a non-blocking content note: future boot-surface PR bodies should use the greppable Evidence: L<X> ... ladder syntax even when the test command is obvious.

N/A Audits — 📡

N/A across listed dimensions: no openapi.yaml tool descriptions are touched.

🔗 Cross-Skill Integration Audit

• Existing --migrate-config remediation is reused; no new workflow skill needs to fire.
• No AGENTS_STARTUP.md workflow-list entry needed for an internal boot guard.
• ADR 0019 was checked: the PR protects the overlay SSOT and does not export, alias, mutate, or defensively mask AiConfig.

Findings: All checks pass — no integration gaps for this PR's scoped guard. Follow-up should fan out the call-site coverage.

🧪 Test-Execution & Location Audit

• Branch checked out locally at exact head 7bed42ebe589fbd42e2d37e19453213831e03e0b in /private/tmp/neo-review-13568-gpt-20260620a.
• Canonical Location: changed spec remains in test/playwright/unit/ai/scripts/setup/initServerConfigs.spec.mjs, the correct AI setup unit-test tree.
• If a test file changed: Ran UNIT_TEST_MODE=true npx playwright test -c test/playwright/playwright.config.unit.mjs test/playwright/unit/ai/scripts/setup/initServerConfigs.spec.mjs; result: 29 passed.
• If code changed: Ran node --check on ai/scripts/setup/initServerConfigs.mjs, ai/mcp/server/memory-core/mcp-server.mjs, and ai/daemons/orchestrator/daemon.mjs; result: clean.

Findings: Tests pass.

📋 Required Actions

No required actions — eligible for human merge.

Follow-up, non-blocking: file or claim a narrow fan-out ticket to wire assertConfigFresh into the remaining config-overlay boot entrypoints (bridge/wake daemons and knowledge-base/github-workflow MCP servers) so the guard coverage matches the crash surface.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 92 - Reuses the existing config drift detector, respects ADR 0019, and avoids auto-migration or defensive downstream reads. 8 deducted for scoped entrypoint coverage below the full overlay surface.
• [CONTENT_COMPLETENESS]: 88 - Ticket and PR body explain the boot contract, tests, risk, and post-merge validation. 12 deducted because the evidence is command-based rather than the greppable ladder syntax and does not explicitly call out the unguarded sibling entrypoints.
• [EXECUTION_QUALITY]: 91 - Exact-head syntax checks and 29/29 focused specs passed; helper classification is simple and read-only. 9 deducted for no live boot integration run and partial entrypoint fan-out.
• [PRODUCTIVITY]: 93 - Fully achieves #13560's scoped MC/orchestrator fail-fast guard and converts a cryptic crash into an actionable remediation.
• [IMPACT]: 80 - Meaningful Agent OS deployment resilience improvement, especially for git-pull-without-prepare recovery paths.
• [COMPLEXITY]: 45 - One helper, two call-sites, and three focused specs; moderate complexity comes from config-template/source-shape semantics rather than line count.
• [EFFORT_PROFILE]: Quick Win - High operational reliability gain from a small, reused-detector change.

Approved with follow-up. This PR is merge-eligible; broaden the guard coverage in a separate narrow lane.