Skip to content

fix(server): preserve delegated results when a wait is interrupted - #27379

Closed
voltcrash wants to merge 1 commit into
pingdotgg:mainfrom
voltcrash:fix/interrupted-delegation-delivery
Closed

voltcrash wants to merge 1 commit into
pingdotgg:mainfrom
voltcrash:fix/interrupted-delegation-delivery

Conversation

@voltcrash

Copy link
Copy Markdown
Contributor

Steering a parent while delegate_task(mode: "wait") is waiting cancels the tool call without cancelling its child. The child keeps running with completionWake: "settled_only", so its result can miss the parent after the blocking delivery path disappears.

Upgrade the wake policy in the shared wait's failure finalizer. Both local and linked-environment waits use this path. The child continues running, and existing timeout behavior remains intact.

Refs #27362: fixes the lost completion delivery. Work-log wording for an interrupted wait remains outside this backend change.

This is a small, focused fix for an obvious cancellation bug under the contribution exception: a finalizer restores the existing asynchronous delivery path when blocking delivery is lost. It introduces no workflow, contract, or timeout default.

Verification on Linux arm64:

  • Added one regression in the existing service suite. It waits on a Deferred marking stream subscription, interrupts the actual service fiber, checks the wake policy and confirms that the child remains running. On the base it fails because no wake-policy command is dispatched.
  • vp test run apps/server/src/mcp/OrchestratorMcpService.test.ts: 38/38 passed, also independently rerun by the reviewer.
  • vp run check:changed HEAD --concurrency 2 against the base passes format, lint, both knip checks, GraphQL generation check, affected typechecks, and 42 tests across three files.

Short service-runtime transcript from the deterministic regression (provider and persistence dependencies are test doubles):

Before: delegated_task.request -> subscribe -> interrupt wait
        no wake-policy update
After:  delegated_task.request -> subscribe -> interrupt wait
        delegated_task.wake-policy(completionWake=always)
        task_status: running

No paid provider turn, deployed relay, or packaged client was exercised. Clients share the server behavior; no UI changed, so screenshots are not applicable.

Reviewed with sol-loop: 1 round with GPT-6.1 Sol on high. Sol verdict: approved.

Created with GPT-6.1 Sol in the Codex harness through T3 Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 30, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 7abd689

Macroscope's review found this PR approvable — Interrupted delegated-task waits now restore the existing asynchronous completion-delivery path so child results can reach the parent. The change is limited to failed or interrupted waits and leaves normal waits and timeout defaults unchanged.

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

@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 7abd689 and 100ee4f.

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

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between aa11ef6 and 7abd689.

Files selected for processing: 2
  • apps/server/src/mcp/OrchestratorMcpService.test.ts
  • apps/server/src/mcp/OrchestratorMcpService.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Walkthrough

When waitForTask exits with a failure or timeout, the service dispatches a delegated_task.wake-policy update with completionWake: "always". A new test verifies this behavior when the waiting fiber is interrupted.

Changes: Delegated-task wait recovery

Layer / File(s) Summary
Restore completion delivery after wait failure
apps/server/src/mcp/OrchestratorMcpService.ts, apps/server/src/mcp/OrchestratorMcpService.test.ts
Non-successful wait exits dispatch a wake-policy update; dispatch failures are ignored and logged. The interruption test checks that the request and "always" policy are dispatched and that the task remains running.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related issues

  • pingdotgg/t3code#27362: The change addresses the interrupted-wait completion delivery behavior described in this issue.
  • pingdotgg/t3code#11168: The change addresses interrupted-wait result delivery, but not the client transport timeout described in this issue.

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · previous review 7abd6

Interrupted waits preserve the completion delivery path while the child continues running. No merge-blocking risk was identified.

Pre-merge checks: Passed 3 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Description check Inconclusive The description follows the template's main sections and gives enough context. Problem: interrupting delegate_task(mode: "wait") cancels the tool call but leaves the child running with completionWake … Link the triaged bug issue or the maintainer-approved discussion for this fix, including the approval comment. If no prior issue exists, explain why this qualifies as a very small, focused fix of an obvious bug under the contribution except…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title names the main change: a delegated task's result is preserved when a wait is interrupted. The fix upgrades the child's completionWake policy to "always" in the wait failure finalizer, so the…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the main change: a delegated task's result is preserved when a wait is interrupted. The fix upgrades the child's completionWake policy to "always" in the wait failure finalizer, so the title matches the changeset's purpose. The title follows the conventional commit format from the repository template.

Full details: Description check

Explanation

The description follows the template's main sections and gives enough context. Problem: interrupting delegate_task(mode: "wait") cancels the tool call but leaves the child running with completionWake "settled_only", so the parent can miss the result. Change: the wait's failure finalizer upgrades the wake policy for local and linked-environment waits. Verification: a focused regression test, the 38/38 service suite run, and a check:changed run are described. The description links the prior-approval exception, cites Refs #27362, and states the change is out of scope for work-log wording. The Scope and approval section is the weakest point. It cites a contribution exception and does not link a maintainer-approved issue or discussion, which the template asks for when claiming a small-fix exemption. The description also omits the author's model and harness line, which the template requests, although the author does name GPT-6.1 Sol in the Codex harness. The Refs #27362 link is ambiguous because the PR objectives reference PR #27379, and the description does not clearly state which issue the fix closes.

Resolution

Link the triaged bug issue or the maintainer-approved discussion for this fix, including the approval comment. If no prior issue exists, explain why this qualifies as a very small, focused fix of an obvious bug under the contribution exception. Clarify whether #27362 is the issue this PR resolves. Confirm the Verification section covers the changed behavior, which it does, and keep the model and harness line at the end as the template requests.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@voltcrash
voltcrash force-pushed the fix/interrupted-delegation-delivery branch from 7abd689 to 100ee4f Compare October 30, 2026 04:42
@voltcrash

Copy link
Copy Markdown
Contributor Author

Note

🤖 GPT-6.1 Sol responding on behalf of voltcrash

The description-check concerns are already covered in the PR body. This uses CONTRIBUTING.md’s small, focused obvious-bug exception: cancellation removes the blocking delivery path, so the finalizer restores the existing asynchronous wake policy. No maintainer approval is claimed.

The linked issue is #27362. The PR fixes its lost-result failure but deliberately uses “Refs” rather than closing the issue because the work-log wording is outside this backend change. The final line names GPT-6.1 Sol and the Codex harness through T3 Code. The focused regression and verification results cover the changed behavior.

The rebase left the reviewed patch unchanged; check:changed also passes on the rebased head 100ee4fa3.

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

Labels

size:S 10-29 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