Skip to content

perf(orchestration): stop and resume wake only that thread's subtree - #488

Merged
lukemaj merged 2 commits into
mainfrom
perf/450-subtree-wake
Oct 19, 2026
Merged

lukemaj merged 2 commits into
mainfrom
perf/450-subtree-wake

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

What: Stop and resume now reconcile child requests only for that thread's subtree, once per transition, instead of on every later metadata update.
Why: A resumed thread keeps forkResumedRetirements set, and the event listener treated that persistent field as a trigger, so every later metadata event of the thread ran the global sweep over every subagent thread (12 to 15 s on rocky).
So what: Luna review, Grok roast and CI, then the backlog coordinator merges.

Problem

Orchestrator.ts called the global requestWake() whenever a thread.metadata-updated event had a Stop token matching its command or a non-empty forkResumedRetirements. The second condition stays true after resume, so title updates, queue.resume, limit recovery and similar events each triggered a full getShellSnapshot plus reconcileChildRequests for every subagent thread. Stop, which already had an exact trigger, also swept globally.

Fix

  • childThreads/requestWake.ts: new reconcileSubtreeChildRequests(threadId, ...) reconciles the thread and its subagent descendants (found with the existing readStopDescendants, so deleted intermediate parents still connect descendants).
  • Orchestrator.ts:
    • The listener's Stop trigger (forkRetirement.token === commandId) now wakes the stopped thread's subtree. The forkResumedRetirements trigger is removed.
    • Resume: before commit, an explicit-input message.dispatch or thread.unsettle checks its planned thread.metadata-updated events; any that acknowledges a token the thread did not already have is an un-retire transition. After commit, each such thread's subtree is woken once (a resumed thread whose parent is resumed in the same commit is covered by the parent's wake). This covers the core resume path and Spectrum reopen, which acknowledges tokens on the Spectrum and every Drafter (spectrum/commandPlan.ts). The wake is forked into requestWakeScope because it takes ancestor locks.
    • requestWake(threadId?) keeps its original thread or environment meaning; subtree sites call requestSubtreeWake, which runs reconcileSubtreeChildRequests, so no flag can turn a missing id into an environment-wide sweep.

Not handled here by design: a child request skipped while its parent was archived, and a crash between commit and the forked wake, are reconciled by the boot sweep in recoverDelegatedTasks. Unarchive no longer gets an incidental sweep from unrelated metadata updates, so such a request can wait until the next boot, request event, Stop or resume.

Upstream

No match in pingdotgg/t3code issues, PRs or code for forkResumedRetirements, requestWake or reconcilePendingChildRequests. apps/server/src/childThreads/ is fork-only (added by #181); Orchestrator.ts hunks are in the existing fork-allowlisted file.

Proof

Run one file at a time in apps/server at the head commit:

  • src/childThreads/requestWake.test.ts: 9 passed.
  • src/childThreads/requestWake.noRequests.test.ts: 1 passed.
  • src/childThreads/requestWake.startup.test.ts: 7 passed.
  • src/spectrum/Controller.integration.test.ts: 17 passed.
  • src/orchestration-v2/ThreadStop.test.ts: 49 passed.
  • vp run typecheck in apps/server: exit 0. vp lint on the changed files: no new findings (one pre-existing unused-variable warning in Orchestrator.ts). scripts/fork-check.sh: OK.

Failing first:

  • requestWake.test.ts "committed ancestor resume wakes only its subtree, once": with the original source, fails at the assertion that an unrelated subtree's pending request was not delivered (expected false to be true). It covers Stop, resume, and a later metadata update of the resumed thread.
  • Controller.integration.test.ts "reopening a stopped council wakes a Drafter request suppressed by Stop" (Luna round 1): with the round 1 Orchestrator.ts, the Drafter's request is never delivered after thread.unsettle and the test times out; with this head it is delivered.

Not done here: rocky trace check after deploy (Issue required proof) and the optional RecoveryReactor.ts:131 note.

Elon record

  • Requirements and who asked: The user, via Issue perf(orchestration): Stop and resume wake only that thread's subtree, not every child thread #450: no environment-wide sweep per metadata event; a child's pending request must still wake its parent after resume (Port child threads onto upstream lineage and delegation #168), including Spectrum reopen (Luna round 1).
  • Deleted: The forkResumedRetirements trigger in the event listener, the global sweep on Stop, and the scope flag on requestWake (Grok roast).
  • Bottleneck: The persistent resumed-token field used as an event trigger, causing a global getShellSnapshot and per-child reads on every metadata update of a resumed thread.
  • Checked myself: Read the listener, the core resume commit path and Spectrum's reopen plan; ran both regressions against the old code (fail) and this head (pass).

Closes #450

Implemented by Claude Opus 5.5 via Claude Code in T3 Code.

🤖 Generated with Claude Code

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M labels Oct 18, 2026
@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ The exact PR base did not have a successful artifact. Baseline uses the latest successful main measurement shown below.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 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 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.9 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 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 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: ad1bab6 · PR result: 936972c · 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 18, 2026

Copy link
Copy Markdown
Contributor Author

Grok roast at c7ce2fa: BLOCKING

The subtree wake on an explicit message resume is the right cut for that one transition. It is not every un-retire the old sweep was actually delivering.

Blocking. Spectrum thread.unsettle un-retires drafters and then nobody wakes them.

Stop a Spectrum while a drafter has a pending approval. The stop reconcile skips it because the drafter is retired. Unsettle the Spectrum without sending a message. spectrum/commandPlan.ts writes thread.metadata-updated with forkResumedRetirements set to that drafter's tokens (participants around lines 167-182, the Spectrum thread around 222-236). Those events used to match the listener, because the list is non-empty, and the global sweep delivered the approval. The listener now matches only forkRetirement.token === commandId (Orchestrator.ts around 12703), which is still the stop token, not the unsettle command. The new fork runs only when resume && state?.retired (around 12180 and 12500), and resume is only an explicit message.dispatch. Unsettle is not an admission command, so retirement state is never read and no wake is forked. The approval stays pending until the next process start. An explicit message to that retired Spectrum is fine: that path forks the subtree wake, and drafters are subagents (LaunchService.ts around 101), so they are inside it.

Non-blocking. The other heals the sweep was doing now wait for boot.

recoverDelegatedTasks still calls environment-wide requestWake() once per boot (Orchestrator.ts around 12749, from serverRuntimeStartup.ts around 426). That is the remaining healer. Unarchive emits thread.unarchived, not thread.metadata-updated (Orchestrator.ts around 3491), so it never entered the old listener by itself. What used to deliver a request that arrived while the parent was archived was the next metadata event on any already-resumed thread. That is the bug this PR removes. After unarchive, that request waits for boot, a new runtime-request.updated on that child, or a later stop or resume of the subtree. Issue #450 called this check optional. The new test's pre-subagent request is the same class and is correctly left undelivered. Imported lineage backfill still runs before the boot sweep, so those links are still reconciled.

Non-blocking, and cleared for a process crash. The separate resume fork is safe.

The fork is after commitCommand returns. The commit applies the projection before it publishes, so the wake does not read the pre-resume retirement. The listener does not replay this event: it subscribes at the boot latestSequence (Orchestrator.ts around 12679). A crash after the commit and before the fiber finishes is re-run by the boot sweep, and command receipts make the delivery idempotent. A retry of the same command sees the thread already resumed and does not fork again. A fiber that fails while the process stays up is swallowed by the existing catchCause and is not retried until boot. That swallow is shared with every other wake. The persistent forkResumedRetirements trigger was the live retry, and removing it is the point of the change. Stop can stay on the listener because token === commandId is true only on the stop event. Resume cannot use that predicate, because the acknowledgement list is copied onto later metadata. The fork is the right trigger for the message path.

Non-blocking. Drop the scope flag.

requestWake(threadId?, scope: "thread" | "subtree" = "thread") treats a missing id plus "subtree" as the environment sweep. The three real calls are already distinct: requestWake() at boot, requestWake(id) for one child, and the subtree at the stop listener and the resume fork. Keep one private runner for the projection, lock, and receipt services, and call reconcileSubtreeChildRequests at those two sites.

@lukemaj

lukemaj commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor Author

Luna review round 1 at c7ce2fa: CHANGES REQUESTED

Blocking: Spectrum unsettle can un-retire Drafters without waking their pending child requests. Repro: Stop a Spectrum while a Drafter has a pending approval, then issue thread.unsettle with no message. The stop-time subtree wake skips the retired Drafter; spectrum/commandPlan.ts:167-181 writes the resumed-retirement metadata for participants, but this path is not an explicit message.dispatch, so Orchestrator.ts:12500-12504 does not fork a wake. The listener at Orchestrator.ts:12703-12706 also ignores that unsettle event because its command ID differs from the persisted stop token. The pending approval remains undelivered until boot or another request event. Please cover this un-retire transition, or otherwise ensure the resumed participant subtree is reconciled.

Non-blocking: unarchive can leave a child request waiting after the parent becomes available. A child request created while its parent is archived is skipped by reconcileChildRequests; thread.unarchived is not a wake trigger, and unrelated metadata updates no longer incidentally sweep it. It can wait until boot or a later request/Stop/resume event. This was an optional Issue #450 note, so I am not treating it as a blocker for the stated Stop/resume scope.

Checks: the added regression exercises observable delivery and the PR body reports it fails on the old implementation. I did not run tests. Issue #450 records the upstream search; PR CI's Fork Stack Model check is green.

@lukemaj
lukemaj force-pushed the perf/450-subtree-wake branch from c7ce2fa to 7fe94b6 Compare October 18, 2026 23:57
@github-actions github-actions Bot added size:L and removed size:M labels Oct 19, 2026
@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Luna review round 2 at 7fe94b6: APPROVE

No blocking findings. thread.unsettle now detects newly acknowledged retirement tokens and forks a subtree reconciliation after commit; the Spectrum integration regression creates a pending Drafter approval while stopped and requires delivery after unsettle. The committed explicit-resume regression proves one delivery in the resumed subtree, excludes an unrelated subtree, and proves a later metadata update does not redeliver. The scope flag is gone: boot, one-thread, and subtree wakes now have distinct entry points. I found no other reachable stop/retirement exit that acknowledges retirement without a corresponding wake; unarchive remains a separate non-resume path, with boot sweep recovery as described. I inspected the regression assertions and control flow only; I did not run tests, as requested.

@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Grok roast round 2 at 7fe94b6: PASS

Round 1 blocker is fixed. thread.unsettle and an explicit message.dispatch both run newlyResumedThreadIds on the plan events before commit (Orchestrator.ts around 12488-12491). A thread.metadata-updated whose forkResumedRetirements adds a token absent from the current projection is a resume. After commitCommand returns, that thread's subtree is woken once (around 12539-12541). commitCommand applies the projection before it publishes, so the wake does not observe the pre-resume retirement. Spectrum unsettle writes those tokens onto each Drafter and the council (spectrum/commandPlan.ts around 167-182 and 222-236) while the council is not active. The council id stays in the wake set and the Drafter ids are dropped as descendants of it. reconcileSubtreeChildRequests then walks them. The new integration test is that path: stop, pending Drafter approval, thread.unsettle, one delivery.

The scope flag is gone. Boot is requestWake(), one child is requestWake(threadId), and stop/resume call requestSubtreeWake, which calls reconcileSubtreeChildRequests directly.

Double wake, cleared. Stop still wakes only from the listener, and only when forkRetirement.token === commandId (around 12747-12751). Resume and unsettle leave the old stop token on the payload, so the listener does not match those events. The dispatch fork is the only resume wake. A council and its Drafters in one commit collapse to one subtree wake. A request listener still in flight and the resume fork both take the parent lock inside reconcileChildRequests and share command:child-request:<child>:<request>:<parent>:0. The later one sees the accepted receipt and does not append a second message. A second explicit message, and a later metadata write that still carries the tokens, add no new token, so they do not fork.

Missing exit, cleared. The only writers of forkResumedRetirements are the explicit-message admission event, the stop event (it clears the list), and Spectrum's message/unsettle events. Stop is the listener. Explicit input and unsettle are the fork. queue.resume, unarchive, and unsettle of a non-Spectrum thread add no token. A descendant stays retired until its own list contains the token (retirement.test.ts, resumes only the named thread). Spectrum writes the Drafter lists in the same command, which is the hole from round 1. Unarchive remains the optional boot-heal from #450.

Lock order, cleared. The fork still starts inside dispatchWithReceiptEffect, while withLock is held. KeyedLock is not reentrant. The wake is a different fiber and the dispatch does not join it. After the fork the dispatch takes no further lock, so it releases and the wake's withLock(parent) proceeds. Explicit resume of an app-owned subagent already holds the parent lock outside the child lock, the same root-down order the terminal-run path requires. The wake dispatches through dispatchWithReceiptEffect, which does not take that lock again.

I did not run tests.

lukemaj and others added 2 commits October 19, 2026 04:30
A resumed thread keeps forkResumedRetirements set, so every later
metadata update of it ran the environment-wide child request sweep.
Stop now reconciles only the stopped thread's subtree, and resume
reconciles its subtree once, from the commit that un-retires it.

Closes #450

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hread

Spectrum unsettle acknowledges retirement tokens on its Drafters without
an explicit message, so their requests stayed asleep after reopen. Any
explicit-input or unsettle commit that adds an acknowledged token now
wakes that thread's subtree once. requestWake loses its scope flag;
subtree sites call the subtree reconcile directly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lukemaj
lukemaj force-pushed the perf/450-subtree-wake branch from 7fe94b6 to 936972c Compare October 19, 2026 02:31
@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Luna review round 3 at 936972c: APPROVE

Both commits are patch-identical to round 2. The new-resume detection and post-commit subtree wake remain compatible with #492's Stop/restart-cut handling; no semantic interaction found.

@lukemaj
lukemaj merged commit 69a5b9e into main Oct 19, 2026
27 checks passed
@lukemaj
lukemaj deleted the perf/450-subtree-wake branch October 19, 2026 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 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.

perf(orchestration): Stop and resume wake only that thread's subtree, not every child thread

1 participant