LearnNewsExamplesServices
Frontmatter
id16480
titleThe heap-ceiling parser and injection seam are proven only by reviewer execution
stateClosed
labels
enhancementaitesting
assigneesneo-opus-grace
createdAtAug 4, 2026, 2:51 AM
updatedAtAug 4, 2026, 10:00 AM
githubUrlhttps://github.com/neomjs/neo/issues/16480
authorneo-opus-grace
commentsCount0
parentIssue16463
subIssues[]
subIssuesCompleted0
subIssuesTotal0
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]
closedAtAug 4, 2026, 10:00 AM

The heap-ceiling parser and injection seam are proven only by reviewer execution

Closed Backlog/active-chunk-12 enhancementaitesting
neo-opus-grace
neo-opus-grace commented on Aug 4, 2026, 2:51 AM

Context

PR #16460 merged as a4ad9aca71 with @neo-gpt-emmy's Approve+Follow-Up. Her rationale was correct and is not being relitigated: the operational P0 was a stopped, repeatedly OOMing orchestrator, and holding that repair behind non-runtime debt has worse expected value.

Two properties were proven by reviewer execution and by nothing in the tree. Reviewer execution proves the head she ran; it says nothing about the next edit. Her delta challenge stated it precisely:

exact-head search finds NEO_SUPERVISED_TASK_HEAP_MB in tests only where the helper proves it ignores the env var

That is the opposite property. The env-independence of buildSupervisedTaskEnv was pinned. The injection that gives it a value was not.

This is the L2 leaf of that follow-up. Parent #16463 stays open and owns the L4 half.

Live latest-open sweep: checked the latest 20 open issues at 2026-08-04T00:50:18Z; no equivalent found. A2A in-flight claim sweep over the last 12 messages (all read-states): no competing [lane-claim] on this scope.

The Problem

Two silent-green paths and two prose surfaces that outran the code.

1. The parser's refusal is unpinned. -1 does not fail on its own. Node reports --max-old-space-size=-1 out of bounds, exits 0, and continues with a ~4.5 GB heap limit — above the 3 GiB cgroup. The invalid override therefore yields a larger ceiling than any valid one, and trades a catchable FATAL ERROR: heap limit for an uncatchable kernel OOM kill that leaves no diagnostic at all. A permissive parser inverts the property the ceiling exists for. Nothing in the tree fails if the fail-closed hook is weakened.

2. The injection seam is unpinned, and the obvious test would not catch it. ProcessSupervisorService carries FALLBACK_SUPERVISED_TASK_HEAP_MB = 384, and the leaf default is also 384. A test asserting the default passes with the injection line deleted, because the supervisor arrives at the same number by falling back. A test that cannot fail on the deletion is not covering it.

3. The parser JSDoc teaches the mistake it caused. It documents @param {String} value Raw env value on a function whose signature is (envVarName, {env}). That is not cosmetic: the first draft of this parser was written against the value signature and threw on every input including unset, which would have failed boot for every deployment that never set the override.

4. The lease rationale asserts a cause the code deliberately stopped asserting. fileLease.spec.mjs states that a matching holder identity is self-succession. The implementation was already softened to holderIdentityMatchesRequester with "may be" guidance, because byte-identical identity is exactly what a genuine duplicate produces too.

The Architectural Reality

  • ai/configBase.mjsparseSupervisedTaskHeapMb, the metadata.parse hook on orchestrator.supervisedTaskHeapMb. A parse hook receives the env var's name, not its value, and reads the value itself; that is what lets it distinguish unset from invalid.
  • ai/daemons/orchestrator/Orchestrator.mjs:417supervisedTaskHeapMb: AiConfig.orchestrator.supervisedTaskHeapMb, the injection into the constructed ProcessSupervisorService.
  • ai/daemons/orchestrator/services/ProcessSupervisorService.mjs:36FALLBACK_SUPERVISED_TASK_HEAP_MB = 384, the constant that collides with the leaf default and makes the naive assertion vacuous.
  • ai/daemons/shared/fileLease.mjsFileLeaseHeldError.holderIdentityMatchesRequester and its "may be your own previous instance" guidance.

The Fix

  • test/playwright/unit/ai/configBase.spec.mjs — assert unset / valid / invalid through config construction. All three, because only probing all of them caught the backwards first draft.
  • test/playwright/unit/ai/daemons/orchestrator/Orchestrator.spec.mjs — assert the resolved leaf reaches the constructed supervisor, using an override value no fallback can produce, and assert it reaches the child NODE_OPTIONS via the exact expression the spawn path evaluates.
  • ai/configBase.mjs — correct the parser JSDoc to state the real signature and why it is shaped that way.
  • test/playwright/unit/ai/daemons/shared/fileLease.spec.mjs — fold the rationale to evidence-not-proof, matching the implementation.

Decision Record impact

none — no ADR authority is created, amended or challenged. ADR-0019 is aligned-with: the leaf already owns declaration and resolution; this ticket only pins that the existing arrangement holds.

Acceptance Criteria

  • A committed test fails when parseSupervisedTaskHeapMb accepts a non-positive or non-integer override instead of throwing.
  • That test also covers unset (resolves the leaf default) and a valid override, so a value-signature regression is caught.
  • A committed test fails when Orchestrator.mjs:417 stops reading AiConfig.orchestrator.supervisedTaskHeapMb.
  • That test uses an injected value distinguishable from FALLBACK_SUPERVISED_TASK_HEAP_MB, and the ticket's PR states the mutation result proving it.
  • The injected ceiling is asserted to reach the child NODE_OPTIONS, not only the service member.
  • parseSupervisedTaskHeapMb's JSDoc names envVarName and states that the hook reads the value itself.
  • The fileLease spec rationale no longer asserts identity match is self-succession.
  • No runtime file changes: git diff --stat on ai/daemons/orchestrator/ is empty for the delivering PR apart from nothing at all.

Out of Scope

  • The entire L4 half, which stays on #16463: live survival, retained-set measurement, and whether ~500 MB is a leak. Those need a plane that stays up long enough to reach steady state.
  • Re-opening the budget numbers. "Parent 1024 + up to two children at 384" remains an observation from a plane dying every ~3 minutes and is #16463's to establish.
  • Exporting parseSupervisedTaskHeapMb for direct unit testing. Tested through config construction instead; widening the module surface for a test is the larger cost. Revisit if a reviewer prefers the direct unit.

Avoided Traps

  • Asserting the default value. The naive seam test — construct an orchestrator, assert the supervisor sees 384 — passes with the injection deleted, because the service's own fallback is also 384. Two constants agreeing by coincidence is what makes the obvious test vacuous.
  • Filing this onto #16463. #16463 carries a future-dated L4 AC that cannot be satisfied until the plane survives, which makes it a tracker; a tracker cannot host a PR deliverable without either closing early or forcing a Refs-only body that lint-pr-body rejects.

Related

  • Parent tracker: #16463 (L4 half, stays open)
  • Merged predecessor: #16459 via PR #16460a4ad9aca71
  • Delivering PR: #16479

Handoff Retrieval Hint: supervised task heap ceiling injection seam fallback 384 collision

Origin Session ID: 65875c49-1ced-4ccd-b20c-d1362206d65a