Skip to content

fix(server): stop stranding late delegated task reports - #13338

Closed
saphid wants to merge 1 commit into
pingdotgg:t3code/codex-turn-mappingfrom
saphid:fix/v2-subagent-stop-reports
Closed

saphid wants to merge 1 commit into
pingdotgg:t3code/codex-turn-mappingfrom
saphid:fix/v2-subagent-stop-reports

Conversation

@saphid

@saphid saphid commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

What Changed

A parent never heard about a delegated task that finished late. Since #5311, each parent-run cohort gets one completion delivery plus one successor. A child that finishes after the successor starts is marked pending and stays that way. The parent keeps working without learning that the child failed or finished. The result is still readable through task_status, but only if the parent already knows to check.

This change drops the lifetime cap in the three places that enforce it:

  • planDelegatedCompletionDelivery, for a child that finishes when no delivery is outstanding
  • finalizeDelegatedCompletionDelivery, when a delivery run settles with siblings pending
  • dispatchNotificationAccepted, when a steering provider accepts a delivery with siblings pending

Coalescing already bounds parent wakes. A cohort holds at most one outstanding delivery, and siblings that finish while it runs join the next one. With the cap gone, each settled delivery reserves the next one for results that arrived while it ran.

The cap also bounded one other thing: retrying a delivery run that was cancelled (Cursor, for example, can report a native cancel). A cancelled delivery puts its batch back to pending, and without a bound it would be re-reserved forever. So that is now bounded directly. A cancelled batch is retried once. If the retry carrying the same results is cancelled too, it waits for a new child result and rides along with that one, instead of re-arming the parent with the same results. settledDeliveryCount is still recorded, but nothing gates on it anymore.

Why

We hit this during a run of Claude rate limits. Child tasks failed with "Claude API rate limit reached". Their results stayed pending for hours while the parent went on to 18 more runs, and nothing told the parent. #5311 capped deliveries so the parent would not re-arm itself for every child. Coalescing already prevents that fan-out, and the cap turns the leftover results into silent losses.

Scope is deliberately narrow:

  • The fix only applies going forward. Results that are already stuck pending in an existing database are not replayed at startup, so an upgrade does not wake old threads in a burst.
  • It does not touch delegate_task wait timeouts or wake-policy upgrade races. fix(mcp): bound delegation waits and recover completion delivery #11997 covers those.
  • It is the first of a few focused fixes to make sure a parent learns when a child stops. The others cover children stopped by restart recovery, children blocked on an approval or question, and children whose provider stream ends without a terminal event. Each will be a separate PR.

Tests

  • OrchestratorMcpToolkit.integration.test.ts: the scenario that pinned the cap ("a third terminal ... cannot recursively create a third parent run") now asserts that the third late child gets its own delivery and that the cohort drains after it settles.
  • DelegatedCompletionDelivery.test.ts: new test "keeps delivering late siblings after earlier deliveries settled" covers the provider-accept path with two settled deliveries.
  • Both fail on t3code/codex-turn-mapping and pass with this change.
  • DelegatedCompletionDelivery.test.ts: new test "retries a cancelled delivery once without re-arming the parent forever" cancels the same delivery twice with no new child result. It waits on the persisted cohort update and asserts one retry and no third reservation. With the retry bound disabled, it fails with a third reservation.
  • DelegatedCompletionDelivery.test.ts: new test "retries a fresh batch even after an unrelated delivery was cancelled" makes sure an earlier cancelled delivery for other results cannot use up a new batch's retry.
  • 62 tests pass in OrchestratorMcpToolkit.integration, DelegatedCompletionDelivery, SteeringCompletion.integration, ThreadDeletion, CheckpointCaptureService, ProjectionStore, and ProviderContinuationService. Server and contracts typecheck clean.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes (no UI change)
  • I included a video for animation/interaction changes (no UI change)

This change was made by Claude Opus 5.5 with Claude Code and reviewed by GPT-6 Astra with Codex.

🤖 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 30-99 changed lines (additions + deletions). labels Sep 24, 2026
@saphid
saphid marked this pull request as ready for review September 24, 2026 01:43
@macroscopeapp

macroscopeapp Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The change removes a lifetime cap on delegated-completion deliveries, allowing late child results to trigger additional parent provider runs beyond the previous two-delivery limit. Although cancellation retries are bounded and the behavior is tested, this is a material production orchestration-policy change requiring human review.

No code changes detected at e1ff11d. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@AmishHillBilly

Copy link
Copy Markdown

Thanks for this series, it's hitting exactly what we're seeing with multi-level delegation.

One related case I don't think any of the planned PRs cover: when a parent sends a completed child more work with t3_thread_send, the follow-up turn's result is never delivered, because finalizeAppOwnedSubagent returns early once a subagent_result transfer exists. I filed it as #13490.

Would this fit into your series, or would you mind if I opened a small PR for it on top of this one? Happy to go whichever way avoids conflicts.

@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch 3 times, most recently from fe4f6ad to 87c67bd Compare September 25, 2026 05:55
A parent-run cohort allowed one completion delivery plus one successor.
Any child that finished after the successor started was marked pending
and never delivered, so the parent kept working without learning that
the child had failed or finished.

Coalescing already keeps at most one outstanding delivery per cohort, so
drop the lifetime cap and let each settled delivery reserve the next one
for results that arrived while it ran. The cap also bounded retries of a
cancelled delivery, so bound that directly: a cancelled batch is retried
once, and after a second cancellation it waits for a fresh child result
instead of re-arming the parent with the same results.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@saphid
saphid force-pushed the fix/v2-subagent-stop-reports branch from dfc702c to e1ff11d Compare September 25, 2026 07:34
@saphid

saphid commented Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

Closing: #13938 removed the settledDeliveryCount delivery cap in the same three places this PR did (planning, successor reservation, mailbox batching) and dropped the field from the contract, so the stranded late reports are fixed on t3code/codex-turn-mapping. The one piece this PR had that #13938 doesn't is a bound on re-arming a delivery the provider itself cancels. I haven't reproduced that loop through a real provider, and #13938 explains why it deliberately has no limit, so I'm not carrying it forward without a live repro. #13343 and #13345 stay open and will be rebased onto the new head.

@saphid saphid closed this Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). 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.

2 participants