Repository navigation
fix(server): a delegated child's finished follow-up turn wakes its parent - #213
Conversation
…rent finalizeAppOwnedSubagent skipped every turn after a child's first result, so work sent with t3_thread_send never woke the parent (pingdotgg/t3code#13490). Each finished child run now transfers once and reclaims the parent wake; Monitor wakes and cancelled tasks stay quiet. Closes #212 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Independent review of 3b40d6c (effort: high)Verdict: pass, no blocking findings. The per-run dedupe, the Monitor exclusion, the disposed guard and the Checked
Findings (non-blocking)
Not verified: a failing-first run against the base (no scratch checkout was used). From the code, the old early return on any prior transfer makes the new test's second |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 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.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
Review of #213 found two follow-up turns that still never woke the parent: one finished after the user stopped the delegating turn (it was marked disposed), and one of a wait-mode task that finished while the delegating turn ran. A follow-up now reopens a stopped cohort and is delivered like an async result. The orchestration guidance no longer suggests t3_thread_wait, which would add a second wake. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Independent review, round 2 (head Verdict: pass. No blocking finding. One medium gap remains in the round-1 "stop" fix. Focused tests at this SHA: Findings
Questions from the brief
|
With #17144 an end the agent never reads before settle no longer holds the turn; Grok's report arrives in a continuation run instead. That continuation was named a generic background task, so a delegated child's Monitor wake counted as new work (#211) and could replace its result or wake the parent twice (#213). finalizeTurn now names the continuation after the unread ends' own reports, so a Monitor wake stays one. Found in review of #216. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…and reasoning effort (#216) * test(server): replay Grok 1.0.46 background commands that end in the turn Three hand-ported replay fixtures for the Grok shapes that leave a run running forever: a TaskOutput poll on a call-* task id and a MultiResult poll, a KillTask kill, and a task_completed with no poll at all. Refs #15888 * fix(server): Grok runs finish when background commands end in the turn Grok 1.0.46 reports background command ends in shapes the adapter missed, so deferred finalize held the run open forever: - task_completed before the prompt settles was dropped. It now finishes the command row, without marking it read, so an unread end still gets its continuation. - TaskOutput results for call-<uuid>-<n> task ids were ignored. - TaskOutput MultiResult was not read. - KillTask kills were not recognized. A kill now ends only the tasks its result reports as killed or already exited. A row with a genuine end no longer reopens when normalization reads its kept start acknowledgement as running, so a kill or poll with no output also ends it. A poll of a command task_completed already finished no longer appends its output a second time. Fixes #15888 * fix(server): a Grok poll fills a command task_completed ended without output A task_completed without task_snapshot.output ends the row with only its start acknowledgement or partial stdout. The agent's later TaskOutput poll now replaces that with the full output instead of being dropped; a poll that matches what the row already shows still changes nothing. * fix(server): Grok ACP threads run at the selected reasoning effort The Grok flavor's applyModelSelection called applyGrokAcpModelSelection without the stored reasoning effort, so session/set_model carried only the model id and a same-model selection was skipped. The generic option loop could not apply it either: the composer stores the effort under the capability id reasoningEffort while the Grok session exposes it as the thought_level option reasoning_effort. Pass the effort the way text generation already does. On protocol 1 it rides on session/set_model metadata, with the current effort read from the session setup; on newer protocols the wrapper writes the session's thought_level option through setConfigOption and only calls setModel when the model itself changes. Fixes #15251 * fix(server): Grok protocol 1 effort baseline follows the last applied selection Same-runtime switches re-run the flavor against the stored session setup, which only keeps the model id current, so changing the effort away and back on protocol 1 compared against the setup-time value and was skipped as a no-op. Hand the flavor the selection the session was last configured with and baseline protocol 1 on it; activation still starts from the setup alone. Also narrows the newer-protocol baseline to a string: the previous commit left `string | boolean` reaching `currentReasoningEffort` and failed the server typecheck. * fix(server): Grok effort write survives model switches and effort-free selections On newer protocols a model switch can change which efforts the live `thought_level` option offers; writing a value it no longer advertises, or one the agent rejects, failed the session open and with it the turn. Re-read the live option after the switch and, like the generic option loop, skip the value with a warning so the agent's default applies. On protocol 1 an effort-free selection between two effort changes made the baseline fall back to stale setup metadata, so a return to the setup-time effort was skipped. Once a selection has run on the session, baseline on the effort it carried; an effort-free selection leaves the live value unknown and the next requested effort is sent. * test(server): fork proof for the carried Grok background command fix Regression test from the stuck Grok child frames (pingdotgg/t3code#15888): a killed UUID task and a polled call-id task that both end before the prompt settles no longer hold the finished turn. It fails on 93dddfb ('held') and passes with #17144. #17144 deliberately finishes a row on a pre-settle task_completed, so the old 'settle holds for the injected monitor report' test asserted the removed hold. It now asserts the requirement it protected: one wake, no second continuation for the late report. #15319's new test used upstream's testLayer; the fork names it layerTest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(fork): record the carried Grok fixes #17144 and #15319 Refs #215 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(server): the regression snapshot uses only known task_snapshot fields Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(server): keep #17144's fixture prompts module-local for knip Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(server): an unread pre-settle Grok end keeps its report kind With #17144 an end the agent never reads before settle no longer holds the turn; Grok's report arrives in a continuation run instead. That continuation was named a generic background task, so a delegated child's Monitor wake counted as new work (#211) and could replace its result or wake the parent twice (#213). finalizeTurn now names the continuation after the unread ends' own reports, so a Monitor wake stays one. Found in review of #216. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Henrique Caiano <5713877+hcaiano@users.noreply.github.com> Co-authored-by: abderbj <115119179+abderbj@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…rent (#213) * fix(server): a delegated child's finished follow-up turn wakes its parent finalizeAppOwnedSubagent skipped every turn after a child's first result, so work sent with t3_thread_send never woke the parent (pingdotgg/t3code#13490). Each finished child run now transfers once and reclaims the parent wake; Monitor wakes and cancelled tasks stay quiet. Closes #212 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(fork): link child-follow-up-wake to #213 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(server): a follow-up wakes the parent after a stop or in wait mode Review of #213 found two follow-up turns that still never woke the parent: one finished after the user stopped the delegating turn (it was marked disposed), and one of a wait-mode task that finished while the delegating turn ran. A follow-up now reopens a stopped cohort and is delivered like an async result. The orchestration guidance no longer suggests t3_thread_wait, which would add a second wake. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…and reasoning effort (#216) * test(server): replay Grok 1.0.46 background commands that end in the turn Three hand-ported replay fixtures for the Grok shapes that leave a run running forever: a TaskOutput poll on a call-* task id and a MultiResult poll, a KillTask kill, and a task_completed with no poll at all. Refs #15888 * fix(server): Grok runs finish when background commands end in the turn Grok 1.0.46 reports background command ends in shapes the adapter missed, so deferred finalize held the run open forever: - task_completed before the prompt settles was dropped. It now finishes the command row, without marking it read, so an unread end still gets its continuation. - TaskOutput results for call-<uuid>-<n> task ids were ignored. - TaskOutput MultiResult was not read. - KillTask kills were not recognized. A kill now ends only the tasks its result reports as killed or already exited. A row with a genuine end no longer reopens when normalization reads its kept start acknowledgement as running, so a kill or poll with no output also ends it. A poll of a command task_completed already finished no longer appends its output a second time. Fixes #15888 * fix(server): a Grok poll fills a command task_completed ended without output A task_completed without task_snapshot.output ends the row with only its start acknowledgement or partial stdout. The agent's later TaskOutput poll now replaces that with the full output instead of being dropped; a poll that matches what the row already shows still changes nothing. * fix(server): Grok ACP threads run at the selected reasoning effort The Grok flavor's applyModelSelection called applyGrokAcpModelSelection without the stored reasoning effort, so session/set_model carried only the model id and a same-model selection was skipped. The generic option loop could not apply it either: the composer stores the effort under the capability id reasoningEffort while the Grok session exposes it as the thought_level option reasoning_effort. Pass the effort the way text generation already does. On protocol 1 it rides on session/set_model metadata, with the current effort read from the session setup; on newer protocols the wrapper writes the session's thought_level option through setConfigOption and only calls setModel when the model itself changes. Fixes #15251 * fix(server): Grok protocol 1 effort baseline follows the last applied selection Same-runtime switches re-run the flavor against the stored session setup, which only keeps the model id current, so changing the effort away and back on protocol 1 compared against the setup-time value and was skipped as a no-op. Hand the flavor the selection the session was last configured with and baseline protocol 1 on it; activation still starts from the setup alone. Also narrows the newer-protocol baseline to a string: the previous commit left `string | boolean` reaching `currentReasoningEffort` and failed the server typecheck. * fix(server): Grok effort write survives model switches and effort-free selections On newer protocols a model switch can change which efforts the live `thought_level` option offers; writing a value it no longer advertises, or one the agent rejects, failed the session open and with it the turn. Re-read the live option after the switch and, like the generic option loop, skip the value with a warning so the agent's default applies. On protocol 1 an effort-free selection between two effort changes made the baseline fall back to stale setup metadata, so a return to the setup-time effort was skipped. Once a selection has run on the session, baseline on the effort it carried; an effort-free selection leaves the live value unknown and the next requested effort is sent. * test(server): fork proof for the carried Grok background command fix Regression test from the stuck Grok child frames (pingdotgg/t3code#15888): a killed UUID task and a polled call-id task that both end before the prompt settles no longer hold the finished turn. It fails on 93dddfb ('held') and passes with #17144. old 'settle holds for the injected monitor report' test asserted the removed hold. It now asserts the requirement it protected: one wake, no second continuation for the late report. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(fork): record the carried Grok fixes #17144 and #15319 Refs #215 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(server): the regression snapshot uses only known task_snapshot fields Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(server): keep #17144's fixture prompts module-local for knip Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(server): an unread pre-settle Grok end keeps its report kind With #17144 an end the agent never reads before settle no longer holds the turn; Grok's report arrives in a continuation run instead. That continuation was named a generic background task, so a delegated child's Monitor wake counted as new work (#211) and could replace its result or wake the parent twice (#213). finalizeTurn now names the continuation after the unread ends' own reports, so a Monitor wake stays one. Found in review of #216. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Henrique Caiano <5713877+hcaiano@users.noreply.github.com> Co-authored-by: abderbj <115119179+abderbj@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…rk contracts Upstream's newer Stop, queue and follow-up behavior collided with retained fork contracts and left 35 focused tests failing. - A usage-limit continuation the user re-armed after Stop now resumes that thread through the retirement guard, which rechecks the arm at commit. - A failed Codex root still settles at once, but stays reachable so Stop and cut-in reach its live child and background processes (fork #332). - Tests now model the fork's same-task follow-ups (#213/#428), parked completions (#431), per-Stop retirement tokens and upstream's message-led queue release, instead of upstream's fresh-task and dispose-on-Stop shapes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What: When a parent sends more work to a delegated child and the child finishes that turn, the parent is woken with the result, like the first result.
Why:
finalizeAppOwnedSubagentreturned once a child had anysubagent_resulttransfer, so follow-up turns only showed up by pollingtask_status(pingdotgg/t3code#13490).So what: Reviewed twice (pass); merge when CI is green. The stop-while-running case is tracked in #214. Drop this carried edit and take upstream's when #13490 is fixed there.
Problem
After a parent continues a delegated job with
t3_thread_send, the finished follow-up turn never wakes the parent. Upstream: pingdotgg/t3code#13490 (open). #203 deliberately left this out and told agents to wait instead.Change
Orchestrator.ts: dedupe the result transfer per child run instead of per child thread. A follow-up keeps the task's firstresult(shown aslatestTerminal*intask_status), reopens a wake the parent already acknowledged or was delivered, and adds no new parent node or turn item. A cancelled (disposed) task stays quiet. Monitor wakes stay excluded (fix(server): a Monitor wake no longer holds a delegated task's result #211).mode: "wait"task whose delegating turn still runs. The orchestration guidance no longer suggestst3_thread_wait, which would add a second wake.T3OrchestrationInstructions.ts,toolInstructions.ts,t3_thread_senddescription) no longer say follow-ups do not wake the parent.child-follow-up-wakeindocs/fork-features.mdandscripts/fork-upstream-edits.txt, retired when upstream fixes #13490.Scope and approval
Closes #212. The user decided to fix #13490 in Chromeria now. Upstream PR pingdotgg/t3code#14030 (closed, unmerged) also reopened the task to
running; that is left out here.Review
Verification
delegated task follow-up turns > wakes the parent once per finished follow-up turn, not for a Monitor wakeinDelegatedCompletionDelivery.test.ts. Without theOrchestrator.tschange it fails:expected [ 'run:follow-up-child:1' ] to deeply equal [ 'run:follow-up-child:1', …(1) ]. With it, it passes and also checks: wake reclaimed (claimed), replay of the same run adds no second result, Monitor wake adds no result and no wake.delegated task follow-up turns after the delegating turn) fail without the round-1 fix (expected 'disposed' to equal 'claimed',expected 'acknowledged' to equal 'claimed') and pass with it.OrchestratorMcpToolkit.integration.test.tswait-mode site asserted zero continuation offers; the completed follow-up now offers one, so it asserts exactly one and resets the counter for the later absence checks.OrchestratorMcpToolkit.integration.test.tsasserted the old contract (follow-uplatestTerminalResultContextTransferId: null). It now asserts the new requirement: a non-null transfer distinct from the first result.vp test runon DelegatedCompletionDelivery, prism Recovery integration, OrchestratorMcpToolkit integration, delegatedTaskContinuation, T3OrchestrationInstructions: 5 files, 29 tests passed. Server typecheck: 0 errors.node scripts/fork-features.mjs check: OK. Lint on changed files: only a pre-existing warning (layerUnavailableunused).Elon record
Requirements and who asked: User: wake the parent for every finished follow-up turn, no duplicates, no Monitor-only notices.
Deleted: Reopening task status to
runningand startup recovery of missed follow-ups (upstream #14030's extra scope).Bottleneck: The single per-thread dedupe check in
finalizeAppOwnedSubagent.Checked myself: Ran the new test red without the fix and green with it; read the dedupe, acknowledge handler and
task_statusper-run transfer lookup.Model and harness: Claude Opus 5.5 in Claude Code (T3 Code).
🤖 Generated with Claude Code