Skip to content

fix(childThreads): skip ancestor loads for children with no requests (#256) - #257

Merged
lukemaj merged 1 commit into
mainfrom
fix/256-reconcile-oom
Oct 13, 2026
Merged

lukemaj merged 1 commit into
mainfrom
fix/256-reconcile-oom

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

What changed: A child thread with no requests to deliver no longer loads its ancestors during request reconciliation.
Why: Startup reconciles every subagent thread. Each one loaded its whole ancestor chain, including the parent's full projection, runs and messages. On rocky, a large planner thread with many children made the server run out of memory at boot: 25+ V8 OOM aborts since 2026-10-13 20:09Z, on both the new and the previous build.
So what: After Sol and Grok review, the planner merges this, builds, and redeploys rocky to stop the crash loop.

Partly fixes #256 (the hot event-decode loop on the shared connection). It does not close it.

Change

  • reconcileChildRequests returns right after reading the child's own runtimeRequests when none of them is a deliverable kind (dynamic_tool_call and auth_refresh were already skipped inside the loop). The loop body does nothing for such a child, so behavior is unchanged; only the ancestor and parent reads are skipped.
  • A new request still triggers runtime-request.updated, which reconciles that child again, so an early return can't lose a request.

Proof

  • The new test requestWake.noRequests.test.ts fails on 916b0feb with loaded ancestor shell parent and passes with the fix.
  • All src/childThreads/ tests pass: 30 passed, 1 skipped, including the existing startup reconcile test that uses real requests.
  • Live data on rocky (read-only): only 3 threads have any runtime request, out of hundreds of subagent threads.
  • Evidence on rocky: macOS crash reports Chromeria-2026-10-13-*.ips, 25 of them, all node::OOMErrorHandler. The server's own trace is dominated by childThreads.readRetirementState, childThreads.reconcileRequests and OrchestrationEventStore.rowToV2StoredEvent. Rolling back to a6fc446 didn't help: the server reached 3.6 GB in 2 minutes.
  • No version policy exists in this repository, so there is no bump.

Elon record

  • Requirements and who asked: Rocky's Chromeria must stay up overnight for the user's agents and experiments. The user asked for the overnight work on 2026-10-13.
  • Deleted: A memory-limit raise and thread cleanup, which treat the symptom. Any wider rework of rocky: background reads keep the shared database connection busy; ContextMD moments queue behind them #256's shared-connection reads, which needs its own design.
  • Bottleneck: The server crash-loops every few minutes and cancels every agent run on rocky.
  • Checked myself: Read the crash reports and server traces, read requestWake.ts, retirement.ts and the Orchestrator startup path, ran the failing-then-passing test and the childThreads suite, and queried the runtime request counts read-only.

🤖 Generated with Claude Code

…to deliver (#256)

Startup reconciles every subagent thread. Each child loaded its ancestors'
full projections and the parent's runs and messages, so a large parent with
many children ran the server out of memory at boot, in a crash loop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS labels Oct 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 4.9 KiB −23 B (−0.5%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB −23 B (−1.9%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.8 KiB −41 B (−0.2%) 29.3 KiB ✅
Codex Live turn messages 2 1 −1 (−50.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB +31 B (+0.6%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB +31 B (+2.6%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB +41 B (+0.2%) 29.3 KiB ✅
Claude Live turn messages 1 2 +1 (+100.0%) 8 ✅

Baseline: 916b0fe · PR result: 142d61b · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@lukemaj

lukemaj commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent Sol review passes at exact head 142d61be05e037124520e748efbcf1081d1de897, with no blocking findings.
Why: The early return skips only requests the existing loop already ignores, while deliverable requests retain delivery and cancellation behavior.
So what: This review is complete; the planner can proceed through the remaining review and delivery gates.

Round 1 retry. Read the PR body in full, inspected gh pr diff 257 -R toolboxmd/chromeria and git show, and traced requestWake.ts, retirement reads, the Orchestrator listener, and EventSink projection commit/publication ordering.

  • The guard is kind-only. Empty arrays and arrays containing only dynamic_tool_call/auth_refresh skip ancestor reads. Those kinds previously continued before any dispatch or receipt processing. Any deliverable kind, including a resolved or cancelled request, prevents the early return and retains queued-notice cancellation.
  • A request arriving after the initial read still emits runtime-request.updated. The unchanged listener reconciles that child again, with projections applied before publication. The existing test live pending-request listener wakes an ancestor without invoking recovery passed, as did startup delivery, resolution cancellation, resume, and offline recovery tests.
  • Exact scope is two files: the guard and shared predicate in requestWake.ts, plus requestWake.noRequests.test.ts. No other changes relative to 916b0feb2194d20d244cb18830a254c45cd516bb.

Proof executed only in an independent /tmp clone:

  1. pnpm install --frozen-lockfile: passed. Environment reported Node v26.8.1 against requested ^24.13.1, with pnpm 11.10.0.
  2. At the exact candidate: cd apps/server && ./node_modules/.bin/vp test run src/childThreads/: 7 files passed, 30 tests passed, 1 skipped, exit 0.
  3. At exact base 916b0feb2194d20d244cb18830a254c45cd516bb, carrying the candidate's unchanged new test: same command produced 1 failed, 29 passed, 1 skipped, exit 1. The sole failure was requestWake.noRequests.test.ts with loaded ancestor shell parent.

The production OOM disappearance was not independently live-verified. This review verifies the skipped ancestor-read mechanism and the scoped behavior above. No writes were made in ~/dev/toolboxmd/chromeria, and Chromeria was not restarted.

Reviewer: gpt-6.1-sol, Codex harness, medium effort.

@lukemaj

lukemaj commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Independent review of 142d61be05e037124520e748efbcf1081d1de897. No blocking findings.

The early return matches the loop. isUndeliveredKind is the same check the loop already used to skip a request (dynamic_tool_call and auth_refresh). runtimeRequests.every(...) is true only when every request is one of those kinds, or when the list is empty. In both cases the loop body never dispatches and never cancels. A resolved deliverable request (any other kind, including user_input and command) makes every false, so it still enters the loop, fails the pending check, and cancels queued notices. Status is not part of the early return.

A new request still reconciles. Orchestrator is unchanged: runtime-request.updated calls requestWake(threadId), which calls reconcileChildRequests. EventSink applies the projection before it publishes, so that call reads the new request and a deliverable kind does not return early. Startup still calls requestWake() with no id and reconciles every subagent from the current projection.

The new check reads before the parent lock, while the loop re-reads under the lock. A request that appears only after that first read is not dropped: it is its own runtime-request.updated, which starts another reconcile. The ancestor walk is not required to observe it.

Scope: parent is 916b0feb2194d20d244cb18830a254c45cd516bb. The diff against that parent, and against main, is only apps/server/src/childThreads/requestWake.ts and apps/server/src/childThreads/requestWake.noRequests.test.ts.

Proof, in /tmp/chromeria-review-257 only:

  • On 916b0feb, the new test failed with Error: loaded ancestor shell parent.
  • On 142d61be, cd apps/server && ./node_modules/.bin/vp test run src/childThreads/ passed: 30 passed, 1 skipped, including the new test and the existing startup test that cancels queued notices when a request is resolved.

@lukemaj
lukemaj merged commit 7184184 into main Oct 13, 2026
28 checks passed
@lukemaj
lukemaj deleted the fix/256-reconcile-oom branch October 13, 2026 21:56
lukemaj added a commit that referenced this pull request Oct 14, 2026
…to deliver (#256) (#257)

Startup reconciles every subagent thread. Each child loaded its ancestors'
full projections and the parent's runs and messages, so a large parent with
many children ran the server out of memory at boot, in a crash loop.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rocky: background reads keep the shared database connection busy; ContextMD moments queue behind them

1 participant