Repository navigation
Conversation
ApprovabilityVerdict: Approved at 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. |
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
|
| 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:
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: 
3 · 
1
-
Failed checks: 1 inconclusive Passed checks: 3 passed-
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#27362link 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
#27362is 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.
7abd689 to
100ee4f
Compare
|
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; |
Steering a parent while
delegate_task(mode: "wait")is waiting cancels the tool call without cancelling its child. The child keeps running withcompletionWake: "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:
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 2against 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):
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.