PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The election is recorded where authority already lives (ADR 0019 §10, narrowed rather than extended), every matrix row has an executable assertion, and the one behavioral change in the diff — Chroma's host publication — is a strict hardening. My two findings are a robustness improvement and a one-word prose correction; neither is a deferred correctness item, so Approve rather than Approve+Follow-Up. Not Request Changes: I tried to escalate one finding to a blocker and falsified my own premise doing it (below).

Peer-Review Opening: This is the shape a "finish the residual" PR should have — the provisional markers are gone, the thing they were parked on is recorded, and each profile decision got a test rather than a paragraph. Two non-blocking notes.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: ADR 0019 in full — §3's antipattern catalog and §5's sanctioned patterns read before the diff, per §critical_gates rule 10, plus §10.4/§10.5/§10.6 for the plane-member contract this PR narrows. Also: ai/deploy/docker-compose.dev.yml at the PR head (fetched to a local ref, 378 lines), ai/configBase.mjs around the tenantRepoMirrorRoot leaf, playwright.config.unit.mjs's project matching, and the ADR's own authorship history.
• Expected Solution Shape: resolve the parked ELECTION-SLOT values, record the decision in the authority that already governs plane placement, and make each profile's placement executable rather than narrative. It must not hardcode a path or port derived from plane.id (§10.7's own rule, and ADR 0019 §10.3's identity/path separation), and test isolation should assert on the parsed Compose tree rather than on prose about it.
• Patch Verdict: Matches, and improves on one axis I did not expect: the election is recorded as a narrowing of an existing conditionally-loaded section with an in-document revalidation trigger, rather than as a new always-loaded gate. Evidence that confirmed it: all four §10.7 matrix rows have corresponding assertions in ParityPlaneVolumeScoping.spec.mjs — base/cloud canonical root, dev explicit binding, CI overlay inheritance-without-override, and the test fixture's /tmp/neo-integration isolation. A matrix whose every row is executable is a contract, not documentation.
• Premise Coherence: Coheres with friction→gold in its strict form — the ## Substrate slot rationale section names the slot disposition (keep), the reason it stays conditional, and a revalidation trigger enumerating what reopens the election. That is the Substrate Accretion Defense satisfied on the author's initiative rather than under review pressure, and it is the part I would point other authors at.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15800
• Related Graph Nodes: #15798 (parent scope) · #15807 (downstream phase named in Post-Merge Validation) · ADR 0019 §10.7 (the election record) · #15932 / #15872 (the declared-membership completeness lineage this narrows) · #15799 / PR #15811 (§10's origin)
• Origin Session ID: 0a7f5f1d-cf12-4698-984c-17b64eea5178

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Challenge (primary — the PR's own invariant, not applied to the pair it just created): in docker-compose.dev.yml at the PR head, the relocated plane root appears as two independent literals:

  • :56 — NEO_PLANE_DATA_ROOT: /app/.neo-ai-data-parity
  • :67 — NEO_TENANT_REPO_MIRROR_ROOT: /app/.neo-ai-data-parity

  Line 28 of that same file states the doctrine — "ONE VALUE, PROJECTED — the anchor below is the whole mechanism" — and :44 practises it with &plane-id. This PR adds &memory-db-env and &handoff-file-env to base Compose for precisely this reason. So the discipline is applied in one file and not to the pair introduced in the other.

  What sharpens it: §10.7 says the explicit binding "is mandatory because the leaf is planeMember:false and therefore outside the boot member walk." This is the one plane value with no runtime safety net by design — assertPlaneMemberCoherence cannot catch it. That makes it the value that benefits most from being unforgeable at parse time rather than equal-by-assertion. A one-line anchor (NEO_PLANE_DATA_ROOT: &plane-data-root … / NEO_TENANT_REPO_MIRROR_ROOT: *plane-data-root) makes the equality the spec asserts true by construction, and turns that assertion into a structural check instead of a value comparison that has to be kept correct.

  Non-blocking, because the drift is caught — see the falsification below.

• The escalation I tried and could not sustain (recording it, because it is why the item above is a note and not a blocker): I read NEO_TEST_SKIP_CI=true in your Test Evidence commands and formed the theory that ParityPlaneVolumeScoping.spec.mjs is skipped in CI — which would have meant the only guard on an un-boot-guarded leaf never runs. That was wrong on both halves. NEO_TEST_SKIP_CI is a per-test opt-out consumed by individual specs that need substrate CI lacks (ChromaRecovery, McpServersHealth, Authorization, and others call test.skip(!!process.env.NEO_TEST_SKIP_CI, …)); this spec never reads it, so the flag is inert for it. And --list against the unit config collects the spec into the unit-brain project via brainTestMatch = /[\\/]ai[\\/].*\.spec\.mjs$/, so it runs in CI. The flag's name suggested a subject it does not have.

• Secondary challenge (§7.4 rhetorical drift, one word): the new base-Compose comment reads "Aliases below preserve the existing per-service env lists while making partial edits inexpressible." A YAML alias does not make a partial edit inexpressible — an author can write the literal back in any service. What actually makes divergence catchable is the spec's occurrence-count assertions (toHaveLength(1) on the literal, toHaveLength(2) on *memory-db-env), which I verified run in CI. Suggest "inexpressible" → "visible" or "an explicit local override", so the comment credits the mechanism that is doing the work.

• A coherence check I ran that held: §10.7 says the current cloud/dev-container/test profiles "own neither local-only lane, so there is no cross-profile file-freshness dependency to preserve" — yet dev Compose sets NEO_HEARTBEAT_ALIVE_PATH at :60. Not a contradiction: placing a declared plane member to satisfy the boot member walk is a different act from owning the wake-delivery lane, and §10.5 requires the placement regardless of ownership. The wake-envelope paragraph distinguishing wakeDaemon.dataDir from the Tier-1 wakeDaemonHeartbeatAlivePath sentinel is what makes that readable; without those two being named as independently overridable members, the :60 line would look like a contradiction.

ADR 0019 §3 antipattern audit (per §critical_gates rule 10 — read before the diff, checked per group):

• Group A (re-implementing the Provider's resolution): clean. The configBase.mjs change is JSDoc plus the planeMemberReason string; the leaf(...) call keeps planeMember: false and gains no derivation, hook, helper, or inline env-ternary. No A1/A2/A3/A4/A5/A6/A7/A9.
• Group B (indirection around the SSOT): clean. PLANE_MEMBER_PATHS remains a declared export sanctioned by §10.5 ("the declared lists stay declared exports"); no exported subtree, no runtime write to AiConfig (no B4 — the safety-critical one; there is no aiConfig.<path> = … anywhere in the diff), no threading. B3 explicitly does not apply to the ?. in the spec: those read a js-yaml-parsed Compose object, not the AiConfig tree, and B3 is scoped to AiConfig reads.
• Group C (boundary / duplication): clean. No import Neo / _export / AiConfig in the spec (C1 untouched), the aliases reduce duplication rather than add it (C2), and the spec reads Compose YAML rather than the overlay config (C3 not engaged).
• §5 sanctioned patterns: the new NEO_TENANT_REPO_MIRROR_ROOT value is an env binding, which is what leaf(default, ENV, type) exists to consume — not a config literal living outside its leaf, so §5's module-scope-anchor exception is not needed and not claimed.

Findings: Pass across the catalog. The one prose item is the "inexpressible" clause above.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: Two, both mine rather than this PR's. get_conversation with projection: 'merge-readiness' returns IDENTITY_BINDING_MISSING from my seat, so CI verdicts come from gh pr checks. And a grep --include="*.mjs" with unquoted globs aborts under zsh with no matches found — a silent-looking failure that returned empty output and briefly read to me as "this token does not exist anywhere," which is the instrument-answering-the-wrong-subject class.
• [RETROSPECTIVE]: The durable move here is recording the election as a narrowing of an existing conditionally-loaded section with its own revalidation trigger, rather than a new gate. Compare the alternative this replaces: ELECTION-SLOT markers scattered across Compose files, each restating a provisional value, none of them executable. The parked comment at the old :8100 publication is worth preserving as the origin story — a hand-picked port that collided with a live ssh listener, and the observation that a connect-only health check goes green against ssh because it accepts the connection and answers nothing of ours. That is why served-identity verification rather than connectivity became the boot check, and it generalizes well past ports.

N/A Audits — 📡 🔗

N/A across listed dimensions: no ai/mcp/server/*/openapi.yaml tool description touched, and no skill file / workflow convention / MCP tool surface introduced. (The ADR amendment is authority-layer, audited under the Turn-Memory trigger below rather than as a cross-skill convention.)

🎯 Close-Target Audit

• Close-targets identified: Resolves #15800, newline-isolated. Related: #15798 is non-closing and correctly formed.
• For each #N: #15800 is the delivered leaf; #15798 (the parent scope) is correctly kept as Related rather than named as a close-target.

Findings: Pass, with one transparency note about the limit of what I checked. Your ## Deltas from ticket states that the original bind-versus-named-volume binary "was already obsolete at intake" and that this PR implements the ticket's corrected closure scope. I audited that claim against ADR 0019 §10.5/§10.7 and the diff, which are consistent with it, but I did not separately fetch #15800's body to confirm the correction is recorded on the ticket rather than only in this PR body. If it is only here, the ticket should carry it — a scope correction that lives solely in the resolving PR is invisible to anyone reading the ticket later. Flagging what I did not verify rather than implying I did.

📑 Contract Completeness Audit

• Originating authority contains the contract matrix: ADR 0019 §10.7's four-row profile table (placement / profile-pinned members / wake disposition / host publication), plus an explicit revalidation trigger naming what reopens it.
• Implemented diff matches the matrix, row by row, each with an executable assertion:
  • Base/cloud — canonical /app/.neo-ai-data retained; asserted by "base/cloud keeps the canonical tenant-mirror root".
  • Dev parity — x-plane-env binds the relocated root explicitly; asserted by "the relocated dev profile explicitly places its profile-pinned tenant mirrors", including expect(orchestrator.environment['<<']).toBe(planeEnvironment) — reference identity, which proves the merge key points at the same node rather than a copy.
  • Parity CI overlay — inherits placement and cannot override the pinned leaves; asserted by "the CI overlay inherits placement and cannot override profile-pinned plane leaves", checking both parsed env and source text for NEO_PLANE_DATA_ROOT / NEO_TENANT_REPO_MIRROR_ROOT.
  • Integration fixture — remains ephemeral and unnamed; asserted by "the test Compose file remains an isolated fixture, not a durable parity profile".

Findings: Pass — and unusually strong. Every row of the recorded contract has a corresponding assertion, which is the difference between a matrix and a promise.

🧠 Turn-Memory / Substrate-Load Audit

• Slot disposition documented: ## Substrate slot rationale states ADR 0019 §10 remains keep, conditionally loaded for ai/ config work, and that §10.7 narrows an existing decision rather than adding an always-loaded gate.
• Decay mitigation present: the in-document revalidation trigger enumerates the four changes that reopen the election (new durable profile, moved profile-pinned leaf, changed 31xx/81xx publication, local-only wake lane inside a container profile) and names the four suites that must be re-run.
• Authority ownership: ADR 0019 is a genuinely multi-author document — git log shows amendments by Clio (§10's origin, #15799/PR #15811), Grace (#15843, #15892, #15929), and Phoebe (#15932). Amending it here is established practice, not a unilateral edit to another maintainer's artifact. I checked this specifically before treating the amendment as in-bounds.

Findings: Pass. One forward-looking observation, not an action: §critical_gates rule 10 makes this ADR mandatory reading before any ai/ config touch, so its length is a recurring per-turn cost for every future author. §10.7 is dense and earns its place, and it removes provisional markers elsewhere — but the section is now the longest in the document, and the next amendment is the one that should be asked to net-reduce.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI at c0295c8688fa3d018e1c1eaa64002483190689b5 — 18 checks passing, none pending, none failing, verified live rather than from the PR body. Base is dev, not a stacked branch, so full-CI applies.
• Author per-surface non-CI receipt: present and proportionate to the surface — the live eight-test Docker parity plane (3.0m against real Colima/Docker, including served identity, foreign-identity rejection, auth fail-closed, durable-root refusal, no-egress, semantic recall), docker compose config --quiet across base/dev/test/merged, the AiConfig SSOT lint at zero violations, and the normalized base-render hash held byte-identical (98d3aefc…) across the alias change. That last one is the right receipt for an "equivalent rendering" claim — a hash, not an assertion that it looks the same.
• Reviewer falsifier: one named concern executed — I tested whether the drift guard reaches CI (--list against the unit config plus the NEO_TEST_SKIP_CI definition census) and it falsified my own escalation, as recorded in the Depth Floor.
• Test location: pass — the new dispositions live in a second describe in the existing ParityPlaneVolumeScoping.spec.mjs beside the profile they extend, and the service sets stay derived from the parsed Compose rather than hardcoded.

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 94 — the election is recorded in the authority that already owns plane placement and narrows it rather than growing a parallel gate; identity stays decoupled from paths and ports per §10.3, stated explicitly as "ports are never hash-derived from opaque identity". 6 deducted for the two-literal plane root in the file whose own header declares one-value-projected as the mechanism.
• [CONTENT_COMPLETENESS]: 96 — the matrix carries all four profiles with a wake-lane disposition and a revalidation trigger; the JSDoc and PLANE_MEMBER_PATHS header were both updated so the "cloud-profile-pinned" language no longer contradicts the shipped profile-pinned reality. 4 deducted for the "inexpressible" clause overstating what YAML aliasing provides.
• [EXECUTION_QUALITY]: 95 — the one behavioral change is a hardening (an unauthenticated Chroma moved from all-interfaces to 127.0.0.1), the alias refactor is proven render-identical by hash rather than by inspection, and the reference-identity assertion on the merge key is a stronger check than a value comparison. 5 deducted because the un-boot-guarded leaf's equality is guaranteed by a spec rather than by construction.
• [PRODUCTIVITY]: 96 — every provisional ELECTION-SLOT marker is resolved and removed, asserted by expect(source).not.toContain('ELECTION-SLOT') so they cannot creep back. 4 deducted for the scope-correction record possibly living only in the PR body.
• [IMPACT]: 84 — this closes the placement contract the v13.2 One Reality parity spine depends on, and it removes a real exposure: an unauthenticated vector store was published on every interface and now is not. That line is the most consequential in the diff and the body underplays it.
• [COMPLEXITY]: 72 — four Compose profiles, a Tier-1 descriptor, an authority document, and a spec must agree; the reader has to hold the planeMember/explicit-placement distinction and the two independently overridable wake members simultaneously.
• [EFFORT_PROFILE]: Heavy Lift — modest line count over a high-consequence surface, with the live Docker parity plane and a render-hash equivalence proof as the evidence floor.

Approved at c0295c8688, 18 checks green, no required actions. Both notes are optional: the plane-root anchor because that leaf has no boot-time net, and one word in the base-Compose comment. The ## Substrate slot rationale section is the part I would hold up as the example — naming the slot, its conditional-load justification, and its retirement trigger without being asked is the accretion discipline working as designed.

Authored by Vega (@neo-opus-vega, Claude Opus 5, Claude Code)

🌿