Skip to content

fix(orchestrator): Wake settled parents when delegated children finish#4499

Open
mwolson wants to merge 1 commit into
pingdotgg:t3code/codex-turn-mappingfrom
mwolson:fix/delegated-task-parent-wake
Open

fix(orchestrator): Wake settled parents when delegated children finish#4499
mwolson wants to merge 1 commit into
pingdotgg:t3code/codex-turn-mappingfrom
mwolson:fix/delegated-task-parent-wake

Conversation

@mwolson

@mwolson mwolson commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Wake a settled parent thread when an app-owned delegate_task child
    terminalizes, so an async delegation's result is acted on instead of waiting
    for the user's next message.
  • Gate the wake on a per-task completionWake policy so async delegations and
    blocking waits do not double-deliver.
  • Deliver the wake as a real prompt. A wake that only starts a run does not help
    if the provider never sees the text.

Problem and Fix

Problem and Why it Happened Fix
finalizeAppOwnedSubagent wrote the child result transfer but had no wake path, so the parent never ran again after an async delegation. The result card appeared and the thread simply parked. Offer a ProviderContinuationRequest for the parent when a delegated child terminalizes. The existing ProviderContinuationService worker dispatches it as a queue_after_active message.dispatch that starts a wake run.
An unconditional wake would double-deliver for blocking waits, where the tool call already returns the result, while a settled-only wake would miss async children that finish mid-turn. Record a per-task completionWake policy. "always" for async delegations offers on every child terminal and queues behind a live parent run. "settled_only" for wait-mode delegations, and for legacy records without the field, offers only when the parent has no run in preparing/starting/running.
A settled_only wait child that terminalized under a live parent lost its wake permanently once the blocking wait had timed out. New delegated_task.wake-policy command. On wait timeout the MCP service upgrades the task to "always" best-effort, and for an already-terminal task it offers the wake directly when the parent has a live run, which is exactly the state where finalize's gate skipped it.
The gate could read a stale parent snapshot, and finalize wrote the parent subagent row while holding only the child dispatch lock, racing the wake-policy command's write of the same row. Resolve the parent from the child's immutable lineage, then run finalize under the parent thread lock and startNextQueuedRun under the child lock, sequentially and never nested. Gates re-read run state fresh inside the parent lock.
The wake started a run but never prompted the model, so the agent still could not act on the result. ProviderContinuationRequests is provider-native: its dispatch only triggers ingestion of output an adapter already buffered, and both ClaudeAdapterV2 and AcpAdapterV2 discard the message text when it is marked creationSource: "provider". An app-owned child buffers nothing, so the adapter drained an empty buffer and finalized the turn immediately. Add delivery to ProviderContinuationRequest. adapter_buffered stays the default and preserves provider-native behavior; message_text dispatches creationSource: "server", which no adapter mistakes for a buffered wake. Both offer sites build the request through one delegatedTaskWakeRequest helper so they cannot diverge.
Agents polled or spawned watchers for async children because the tool description did not say the thread wakes. delegate_task description now states that async children wake the thread, so callers should end the turn rather than poll or spawn watchers.

Validation

  • Focused suites: contracts decode tests for completionWake and the
    delegated_task.wake-policy command struct; ProviderContinuationService
    tests for both delivery mappings; MCP toolkit integration tests covering
    wait-completes, async-cancel under a live parent, wait-timeout upgrade, an
    already-terminal upgrade in both sub-cases, and a legacy field-omitted child.
    The integration probes assert delivery at every offer site.
  • Full apps/server suite passes on this base: 1674 passed, 10 skipped.
  • vp check and vp run typecheck clean.
  • Live-verified on a packaged desktop build, reading the provider session
    transcript rather than the projection, because a projected message does not
    prove delivery:
    • Settled parent: the wake prompt reached the model, it called task_status,
      and replied with the child's result. The wake run took 5.5s, versus an 89ms
      empty run before the delivery fix.
    • Mid-turn always path: the child terminalized 35s before the root turn
      ended, the wake was requested while the parent was still live, queued behind
      it, and was delivered exactly once after the root settled.
    • Provider-native regression guard: a background Bash wake still attaches its
      buffered CLI output and responds normally.

The unit and integration layers prove the offer and the delivery mapping; the
packaged run is what proves the model is actually prompted end to end.

Known limitations

  • The offer queue is in-memory. A crash between the result-transfer write and
    the worker dispatch loses that wake on replay, since the transfer guard blocks
    re-offering. A durable outbox keyed by the result transfer is the proper fix.
  • One documented missed-wake window remains: finalize skips under
    settled_only with a live parent, the parent settles, and a late upgrade then
    also skips because the parent is no longer live. The MCP path awaits the
    upgrade inside the parent's turn, so this mostly affects direct command
    callers. Tracking wake delivery explicitly, rather than dual-gating on
    liveness, would close it.
  • Reconcile-path run updates skip finalizeAppOwnedSubagent entirely, so
    recovery-cancelled children produce no transfer and no wake. Pre-existing.
  • A wake can start ahead of a queued-only user run, because queue_after_active
    only queues behind blocking runs.

Note

Wake settled parent threads when delegated child tasks finish

  • Adds a completionWake policy to delegated tasks (always or settled_only) that controls when a parent thread is woken after a child terminates. always fires unconditionally; settled_only fires only when the parent has no live run.
  • Introduces a delegated_task.wake-policy command to update a task's wake policy after creation. Upgrading a terminal task to always while the parent is live immediately enqueues a continuation wake.
  • In wait mode, if the blocking wait times out, the policy is upgraded to always before returning so the parent is notified when the child eventually finishes.
  • Wake delivery uses a new message_text mode in ProviderContinuationRequests, which causes a real prompt to be dispatched to the provider (with creationSource: 'server') rather than relying on adapter buffers.
  • Reworks terminal-run event handling in the orchestrator to acquire parent and child locks separately, reducing deadlock risk.

Macroscope summarized cf467be.


Note

High Risk
Changes orchestration command handling, parent/child locking on terminal runs, and provider continuation delivery—areas where missed or duplicate wakes and prompt delivery bugs directly affect agent behavior.

Overview
Settled parent threads now get a continuation when app-owned delegated children finish, so async delegate_task results are acted on instead of sitting in the projection until the user sends another message.

Each delegated task carries completionWake: always for async MCP delegations (wake on every terminal, queue_after_active behind a live parent run) and settled_only for wait (no wake while the parent run is preparing/starting/running; the blocking tool already returns the result). Legacy subagents without the field behave as settled_only.

When wait times out, MCP best-effort upgrades the task via delegated_task.wake-policy to always so a later terminal can still wake the parent. The same command can retroactively offer a wake for an already-terminal task when finalize skipped under settled_only with a live parent, without double-waking if the parent already settled.

Wakes use ProviderContinuationRequest.delivery: "message_text" and creationSource: "server" so the prompt is not treated as adapter-buffered output (which adapters would discard). finalizeAppOwnedSubagent now runs under the parent thread lock (child lock only for startNextQueuedRun) to avoid races with wake-policy updates.

The delegate_task tool description tells agents that async completion wakes the thread and they should end the turn instead of polling.

Reviewed by Cursor Bugbot for commit cf467be. Bugbot is set up for automated code reviews on this repo. Configure here.

An app-owned delegated child wrote its result into the parent projection but
never started a parent run, so an async delegation's result sat unread until the
user's next message. Offer a provider continuation request when a delegated
child terminalizes, with a per-task completionWake policy separating async
delegations from blocking waits.

The wake must also reach the model. ProviderContinuationRequests is a
provider-native mechanism whose dispatch only triggers ingestion of output the
adapter already buffered, and both ClaudeAdapterV2 and AcpAdapterV2 discard the
message text when it is marked creationSource provider. An app-owned child
buffers nothing, so a wake marked that way settled instantly having prompted
nothing. App-owned wakes now dispatch as message_text via a shared
delegatedTaskWakeRequest helper used by both producers.
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e8919d10-04b6-4ca8-8695-dad5db84bc9c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jul 25, 2026
@mwolson
mwolson marked this pull request as ready for review July 25, 2026 04:05

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit cf467be. Configure here.

* that lock with a full-row payload, and unserialized writers clobber each
* other (stale policy on the terminal row, or a terminal row regressed to
* running).
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Transfer guard blocks wake retry

Medium Severity

In finalizeAppOwnedSubagent, an existing subagent_result transfer causes an immediate return before the continuation wake is offered. The wake runs only after writeSystemEvents persists that transfer, so any retry or replay after a successful transfer write but before a successful offer permanently skips the parent wake even though the result is already on the parent.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit cf467be. Configure here.

@macroscopeapp

macroscopeapp Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

Introduces significant new orchestrator behavior (wake policies, new command type, lock ordering changes) rather than a simple fix. An unresolved medium-severity bug comment about wake retry being permanently skipped also warrants human review.

You can customize Macroscope's approvability policy. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant