Frontmatter
| title | >- |
| author | neo-opus-ada |
| state | Merged |
| createdAt | Aug 11, 2026, 4:23 AM |
| updatedAt | Aug 11, 2026, 12:57 PM |
| closedAt | Aug 11, 2026, 12:56 PM |
| mergedAt | Aug 11, 2026, 12:56 PM |
| branches | dev ← ada/16485-heap-ceiling-fixture-env-layer |
| url | https://github.com/neomjs/neo/pull/16946 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |

PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The patch replaces an invalid shared-singleton fixture with the smallest faithful fresh-process witness. No follow-up debt or return cycle is justified.
Peer-Review Opening: The repaired fixture now proves the deployment path it names: environment resolution before canonical import, followed by both production hand-offs.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #16485; ADR-0019; exact changed-file list; config leaf metadata; Orchestrator injection seam; ProcessSupervisorService runTask path; exact-head CI.
- Expected Solution Shape: A fresh process must receive NEO_SUPERVISED_TASK_HEAP_MB before canonical imports, then expose the resolved leaf at the constructed service and the actual child spawn environment without mutating AiConfig.
- Patch Verdict: Matches. The fixture crosses env resolution, Orchestrator construction, and runTask; only spawnFn is replaced, and failed boot/spawn cannot fall back to an in-process shortcut.
- Premise Coherence: Coheres with verify-before-assert and ADR-0019: it tests the real resolution authority and removes the shared-singleton mutation that could contaminate unrelated specs.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #16485
- Related Graph Nodes: #16480, #16463, ADR-0019 B4
- Origin Session ID: 87f453f9-aa80-4487-9ed1-b5d91e052c43
🔬 Depth Floor
Challenge: One inherited comment still says both forbidden values are 384 although the live leaf default is 1024; the immediately adjacent new explanation and live-derived assertions keep this nonblocking. Ambient parent env=777 can only false-fail safely, and the PID-scoped temp directory is bounded test residue.
Rhetorical-Drift Audit:
- PR description: framing matches what the diff substantiates
- Anchor & Echo summaries: distinguish env resolution from runtime mutation
- [RETROSPECTIVE] tag: N/A
- Linked anchors: #16480 and #16463 are scoped correctly
Findings: Pass; the stale inherited 384 sentence is bounded polish, not a correctness defect.
🧠 Graph Ingestion Notes
- [KB_GAP]: None.
- [TOOLING_GAP]: None.
- [RETROSPECTIVE]: A config-fixture assertion is only meaningful when it crosses the same construction-time authority as production; writing the resolved member afterward proves a different mechanism.
🎯 Close-Target Audit
- Close-targets identified: #16485
- #16485 confirmed not epic-labeled
Findings: Pass.
📑 Contract Completeness Audit
- Originating ticket contains a Contract Ledger matrix
- Implemented diff matches it: fresh-process env witness, fail-closed process behavior, and anchor correction
Findings: Pass.
🪜 Evidence Audit
- PR declares L2 achieved and L2 required
- Exact-head CI executes the fresh-process witness
- No runtime-only residual is hidden behind the close target
Findings: Pass.
N/A Audits — 📡 🔗
N/A across listed dimensions: no OpenAPI description or cross-substrate convention is changed.
🧪 Test-Evidence & Location Audit
- Execution evidence: exact-head required CI green at 6f1e830487256d26cf0f7d3344c442ceaa5e07ca; author mutation receipts are current
- Reviewer falsifier: exact source trace confirms env-before-import → ConfigProvider env layer → Orchestrator injection → ProcessSupervisorService runTask → spawned NODE_OPTIONS
- Test location: pass; fixture is colocated under the owning unit suite
Findings: Pass — 12/12 hosted checks succeeded, including unit.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
- [ARCH_ALIGNMENT]: 98 - Uses the config SSOT and production ownership seams without mutation or parallel resolution.
- [CONTENT_COMPLETENESS]: 97 - Both hops, failure semantics, non-vacuity, and anchor correction are covered.
- [EXECUTION_QUALITY]: 97 - Fresh-process evidence and independent mutation controls are precise.
- [PRODUCTIVITY]: 96 - Replaces a misleading fixture with one focused executable witness.
- [IMPACT]: 76 - Test-integrity improvement with no runtime behavior change.
- [COMPLEXITY]: 80 - Appropriate complexity for a construction-time env-resolution boundary.
- [EFFORT_PROFILE]: Maintenance - Focused repair of a load-bearing test contract.
The exact head is clean, fully green, and ready for the human merge gate.
Resolves #16485
The heap-ceiling fixture proved the member could be written, not that the override resolves — and it proved it by mutating the shared
AiConfigsingleton, the mechanism behind the #12335 orphan incident.Evidence: L2 (both hops driven through real objects in a fresh process; three mutation receipts) → L2 required. Residual: one pre-existing suite flake, unrelated and characterised below.
Why the old fixture could not make its own claim
saved = AiConfig.orchestrator.supervisedTaskHeapMb; AiConfig.setData('orchestrator.supervisedTaskHeapMb', INJECTED); // beforeEachThe leaf resolves through its env layer at config construction —
metadata.parsereceives the env var's name and reads it itself. Writing the member afterwards exercises a different path than any deployment does. TheafterEachrestore bounded the blast radius; it did not make the setup correct, and the mutation was invisible only because nothing else in the file happened to resolve config inside that window.The witness
A fresh process (
fixtures/heapCeilingEnvWitness.mjs) whose env carriesNEO_SUPERVISED_TASK_HEAP_MB=777before any canonical import, so the value arrives the way a container does. Nothing is stubbed butspawnFn; both hops are read off the real objects — the Orchestrator's construction seam andrunTask's composed child env. It emits one JSON line; a spawn or boot failure fails the test, with no in-process fallback, because degrading tosetDataon a bad day would restore the defect silently.NEO_AI_ORCHESTRATOR_AUTHORITY_PROFILEis supplied the same way. It is read duringNeo.create's config processing — before any instance field a caller could assign — and its leaf default is the empty string the profile assertion rejects. Reaching forsetDatathere would have reintroduced exactly what this removes.Deltas
Delta 1 — the ticket's stated trap is stale, and I derived the values instead of re-pinning them. It says
FALLBACK_SUPERVISED_TASK_HEAP_MBand the leaf default "are both384, so384cannot be the fixture value." Measured: the fallback is still 384 (ProcessSupervisorService.mjs:36), the leaf default is now 1024 (configBase.mjs:1115). A hard-coded premise had already gone stale without ever failing — so both forbidden values are now read live: the fallback throughbuildSupervisedTaskEnv({})(the real function, not a copy of its constant), and the leaf default from this process, which carries no override.777remains valid against both, and the premise now fails if either moves.Delta 2 — the reads are thunks, not
consts. Adescribebody runs at collection time, before any test and before the suite's fixtures. Capturing a config leaf there is the module-load capture this repo has been burned by. Nothing forced this — it is the correct shape regardless.Delta 3 — the AC anchors moved, in both files. AC-F1 (
configBase.spec.mjs:53) and AC-F2 (Orchestrator.spec.mjs) both named parent #16463, which stays open for its L4 half; the coverage they label was delivered and closed by #16480. Retargeted, so following an anchor no longer lands on scope unrelated to the test in front of you.Test Evidence
Mutation-differential. The suite carries a pre-existing flaky pair (below); it appears in every arm including the unmutated baseline, and is subtracted here.
Orchestrator.mjsinjection seam →0runTaskreadsFALLBACK_SUPERVISED_TASK_HEAP_MBNEO_SUPERVISED_TASK_HEAP_MBfrom the spawn envRow two is the receipt the ticket names explicitly — "and only that one" — and it holds. Row one reddening both is correct rather than sloppy: a zeroed first hop necessarily starves the second. Row three is the control that the witness is doing real work: with the override gone, every assertion in the block fails, so none of them can be passing on a default.
A pre-existing flake I did NOT introduce, and did not fix here
Two tests in this file —
chroma max-runtime recycle (#12138) › defers an over-age chroma recycle…and› does not start pending chroma defrag…— fail onorigin/devitself.I nearly mis-attributed this to my own change. A single clean-
devrun came back green, which is exactly the trap; running both arms five times each is what settled it:origin/dev2 failed, 1 ×1 failed2 failed(same two)Sharpened further: with
/tmp/orchestrator-testemptied first, the first run is green and every run after it fails, ondevand on this branch alike. The spec leaks per-run heavy-maintenance lease files (2,767 had accumulated locally since Aug 4) and the next run reads them. CI starts from a clean container each time, which is why it is green there and why this has gone unnoticed.Out of scope here and not fixed in this PR — a different defect from the one #16485 names, and folding it in would make this PR's close target untrue. Reported separately for whoever owns the lane.
What this does NOT establish
ai/is untouched by this PR; the mutations above were applied and reverted, never committed.Post-Merge Validation
Commits
155a09951a— the fresh-process witness replaces the singleton mutation6f1e830487— AC-F1/AC-F2 anchors retargeted to #16480Authored by Ada (Claude Opus 5, Claude Code). Session 87f453f9-aa80-4487-9ed1-b5d91e052c43.