Skip to content

fix(server): MCP tools say why the orchestrator refused - #36083

Merged
juliusmarminge merged 4 commits into
pingdotgg:mainfrom
SouthCarpet:fix/mcp-dispatch-refusal-reason
Nov 14, 2026
Merged

juliusmarminge merged 4 commits into
pingdotgg:mainfrom
SouthCarpet:fix/mcp-dispatch-refusal-reason

Conversation

@SouthCarpet

Copy link
Copy Markdown
Contributor

claude-sonnet-5 on behalf of SouthCarpet

Replaces #35198. The open PR limit closed it when it was opened. The branch and the code are unchanged.

Problem

Four MCP tools drop the orchestrator's refusal reason: delegate_task, t3_thread_interrupt (the non-stop path), task_cancel and create_threads. Each builds its failure message with the local errorMessage(error) helper alone, which returns only the outer wrapper, for example Unable to create delegated task: Failed to dispatch orchestration command delegated_task.request (<command id>). An agent that calls one of these tools cannot tell a transient failure from one it could act on.

userFacingDispatchErrorMessage (apps/server/src/orchestration-v2/UserFacingErrors.ts) already recovers the reason from the error's cause chain, and sendToThread (t3_thread_send) has used it since #21027. That PR named the first sibling gap: "interruptThread drops the cause the same way and could get the same one-line change in a follow-up."

Change

Seven dispatch-error mappings in apps/server/src/mcp/OrchestratorMcpService.ts now read userFacingDispatchErrorMessage(error) ?? errorMessage(error) instead of errorMessage(error) alone, the idiom sendToThread already uses:

  • delegateTask, dispatch error (delegate_task).
  • interruptThread, dispatch error (t3_thread_interrupt, non-stop path).
  • cancelTask, completion-delivery dispose error (task_cancel).
  • cancelTask, stop-the-child error (task_cancel).
  • createThreads, thread.create dispatch error (create_threads).
  • createThreads, message.dispatch dispatch error (create_threads).
  • createThreads, thread.created.record dispatch error (create_threads).

Nothing else changes. Success paths, error codes and the existing special branches stay as they are.

This PR adds no redaction, sanitization or safety property. It calls an existing helper in seven places that skipped it, so these tools now return the same kind of cause text that t3_thread_send already returns.

Call sites that keep errorMessage(error) alone

Line numbers are at this branch's head.

  • 234, 1108, 1124: the shared threadManagementFailure fallback and the shared loadThreadRecords and loadTasks reads. Nearly every tool in the file uses them; changing them is a wider change than this PR.
  • 1662, 1775: the "Unable to acknowledge delegated task" mappings in the task-status reads. task_cancel does not reach them. They are a real dispatch path and still drop the cause.
  • 1817: the stream error of delegate_task with mode: "wait".
  • 2229, 2254: request_secret.
  • 2970: listThreads.

I can add 1662, 1775 and 1817 here or in a follow-up, as you prefer.

Scope and approval

No prior issue. This uses the route for a very small, focused fix of an obvious bug in CONTRIBUTING.md: one dropped error cause, repeated at seven mappings of four tools in one file, fixed with the one-line idiom that #21027 merged for a fifth tool. I searched open and recently merged issues and pull requests for the four tool names and found no item that covers them. Related: #21027 (merged, the t3_thread_send precedent) and #15586 / #15627 (closed, a different function, dispatchFailure in threadAccess.ts).

Size: 356 changed lines in 2 files. Without the test file, 14 changed lines (7 added, 7 deleted).

Verification

  1. Seven new test cases in apps/server/src/mcp/OrchestratorMcpService.test.ts, titled "reports the dispatch cause instead of the generic wrapper when ...": delegate_task, t3_thread_interrupt, task_cancel (two) and create_threads (three, through it.effect.each). Each one makes the mocked ThreadManagementService fail with an OrchestratorDispatchError whose cause is new Error("private-storage-path").

  2. Mutation check: with the seven source lines reverted and the tests kept, all seven cases fail, each because the message holds the wrapper and not the cause. With the lines restored they pass.

  3. vp test run apps/server/src/mcp/OrchestratorMcpService.test.ts: 66 of 66 pass after the first commit, 71 of 71 at the head.

  4. vp run check:changed: format, lint, knip, graphql, typecheck (3 packages) and the selected tests pass. One earlier run on the same Windows machine stopped with Access is denied. and passed when it ran again with no code change.

  5. Before and after, tool reply text at the service boundary. The tests mock ThreadManagementService; no live provider ran.

    Before (captured with the seven source lines reverted):

    Unable to create delegated task: Failed to dispatch orchestration command delegated_task.request (command:mcp:provider-session%3Amcp-providers:delegate-task:delegate-dispatch-cause).
    Unable to interrupt thread thread:mcp-providers-parent: Failed to dispatch orchestration command thread.interrupt (command:mcp:provider-session%3Amcp-providers:thread-interrupt:interrupt-dispatch-cause).
    Unable to stop delegated task node:mcp-cancel-stop-cause-task: Failed to dispatch orchestration command thread.stop (command:mcp:provider-session%3Amcp-cancel-stop-cause:cancel-task:cancel-stop-dispatch-cause).
    Unable to dispose delegated task node:mcp-cancel-dispose-cause-task completion delivery: Failed to dispatch orchestration command delegated_task.completion-delivery.dispose (command:mcp:provider-session%3Amcp-cancel-dispose-cause:cancel-task-completion-delivery:cancel-dispose-dispatch-cause).
    Unable to create thread 1: Failed to dispatch orchestration command thread.create (command:mcp:provider-session%3Amcp-providers:create-thread:create-threads-dispatch-cause-thread.create:0).
    Unable to start thread 1: Failed to dispatch orchestration command message.dispatch (command:mcp:provider-session%3Amcp-providers:dispatch-thread:create-threads-dispatch-cause-message.dispatch:0).
    Unable to record thread 1 in the parent timeline: Failed to dispatch orchestration command thread.created.record (command:mcp:provider-session%3Amcp-providers:record-created-thread:create-threads-dispatch-cause-thread.created.record:0).
    

    After (the literal prefix plus the value the helper returns for the mocked cause; the five cases of the second commit assert the whole string, the two of the first commit assert the cause text and the absence of the wrapper):

    Unable to create delegated task: private-storage-path
    Unable to interrupt thread thread:mcp-providers-parent: private-storage-path
    Unable to stop delegated task node:mcp-cancel-stop-cause-task: private-storage-path
    Unable to dispose delegated task node:mcp-cancel-dispose-cause-task completion delivery: private-storage-path
    Unable to create thread 1: private-storage-path
    Unable to start thread 1: private-storage-path
    Unable to record thread 1 in the parent timeline: private-storage-path
    
  6. Not checked: a live provider session, and the evals/mcp-tool-use eval (it calls paid models).

  7. No UI surface, so no screenshots.

claude-sonnet-5 on T3 Code wrote the code. gpt-6.1-sol on T3 Code checked it independently, twice. claude-fable-5-2 on T3 Code edited this text.

…trator refused

The caller only saw "Failed to dispatch orchestration command <type>
(<id>)." for both tools. userFacingDispatchErrorMessage already
recovers the real cause and is already used by sendToThread (pingdotgg#21027);
delegateTask and interruptThread did not call it.
…refused

Same defect, same file: the dispose and stop steps in task_cancel,
and all three dispatch steps in create_threads, used errorMessage(error)
alone and lost the real cause the same way delegate_task and
t3_thread_interrupt did before the last commit.
@github-actions github-actions Bot added vouch:unvouched Author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 13, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 5096c9a

Macroscope's review found this PR approvable — MCP failures now surface actionable orchestrator refusal reasons instead of generic wrapper errors, with the existing fallback preserved. Orchestration behavior and permissions are unchanged.

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

@coderabbitai

coderabbitai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Walkthrough

Orchestrator failure responses now use user-facing dispatch messages when available and fall back to the underlying error message. Tests cover task cancellation and delegation, completion-delivery disposal, thread creation, and thread interruption.

Changes: Dispatch error handling

Layer / File(s) Summary
Dispatch error mapping and operation coverage
apps/server/src/mcp/OrchestratorMcpService.ts, apps/server/src/mcp/OrchestratorMcpService.test.ts
Seven failure paths now prefer user-facing dispatch messages and fall back to the underlying error message. Tests assert error codes, causes, and stage-specific prefixes. The provider-instance-disabled mapping remains unchanged.

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

Change: Bug fix

Suggested reviewers: juliusmarminge, fjooord

Merge Risk: Low · up to e1d46

Failures from delegating, cancelling, creating or interrupting threads now return the underlying cause text to MCP callers. This improves diagnosability. Owners should be aware that internal error messages may be exposed, but no concrete sensitive leak was found, so the change is low risk to merge.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title "fix(server): MCP tools say why the orchestrator refused" matches the main change: seven dispatch-error mappings in OrchestratorMcpService.ts now include the underlying cause via userFacingD…
Description check Passed The description follows the template's main sections: Problem, Change, Scope, and Verification. Problem and Change are clear and specific. Scope names the bug, explains why no issue is linked, and cit…
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 "fix(server): MCP tools say why the orchestrator refused" matches the main change: seven dispatch-error mappings in OrchestratorMcpService.ts now include the underlying cause via userFacingDispatchErrorMessage, so MCP tools report the orchestrator's refusal reason. It is a conventional commit title in plain language and is specific enough for a teammate scanning history.

Full details: Description check

Explanation

The description follows the template's main sections: Problem, Change, Scope, and Verification. Problem and Change are clear and specific. Scope names the bug, explains why no issue is linked, and cites related PRs. Verification lists focused tests, a mutation check, exact commands, observed results, and items not checked. The template's optional agent attribution line is present. The description does not link a specific issue, but the template allows a small obvious-bug fix to explain why no issue is needed, and this one does. The author also notes that the PR is a replacement for #35198.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@juliusmarminge juliusmarminge added the takeover:in-progress A takeover lead is working on this PR label Nov 14, 2026

@coderabbitai coderabbitai 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.

Additional comments: 1

apps/server/src/mcp/OrchestratorMcpService.ts-2664-2664: 1

2664-2664: Minor · Security & Privacy · Quick win

Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Expose known refusal messages, not arbitrary defect text.

When OrchestratorDispatchError wraps an arbitrary Error, these mappings now put the nested error text into the MCP response. Previously, that path returned the fixed dispatch wrapper. The added tests explicitly exercise this behavior.

Allow known domain refusal messages through. For unknown defects, retain the safe wrapper and keep diagnostic text in cause.

As per coding guidelines, “The message is fixed or built from those attributes, never from cause, cause.message, or a stringified defect.”

Also applies to: 2772-2772, 2803-2803, 3007-3007, 3045-3045, 3071-3071, 3472-3472

Source: Coding guidelines

Review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 5096c9a and e1d4623.

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; 9 remain after this review.

@juliusmarminge

Copy link
Copy Markdown
Member

Evidence

Started each commit's real dev server with a fresh isolated database, added an empty git project, and ran OpenCode ACP with openai/gpt-5.6-luna. The same agent-executed driver held a 30-second SQLite write lock, waited one second, then called create_threads. Both calls returned isError; base hid the cause, while head preserved Unable to create thread 1: and explained SQLITE_BUSY with retry guidance. These are timed command-output recordings, not web screenshots.

Before After
9803d1adb5 e1d46236f3
Before: MCP reply hides the database cause After: MCP reply explains SQLITE_BUSY and suggests retrying
Before recording After recording
Actual base MCP reply Actual head MCP reply

Reproduction driver · Setup instructions · Recording renderer

Not shown:

  • The other six changed mappings were not exercised live: delegate_task creation, task_cancel completion disposal and stopping, create_threads starting and parent-timeline recording, and non-stop t3_thread_interrupt.
  • Other explanatory dispatch causes and fallback behavior were not exercised; this proof covers the reported SQLITE_BUSY failure.
  • No replay scenario was used: the reproduction requires a real provider session executing a shell command under an external SQLite lock.

@juliusmarminge juliusmarminge added takeover:ready Takeover done; auto-lands when gates pass and removed takeover:in-progress A takeover lead is working on this PR labels Nov 14, 2026
@juliusmarminge
juliusmarminge merged commit e1f5b68 into pingdotgg:main Nov 14, 2026
49 checks passed
@juliusmarminge juliusmarminge removed the takeover:ready Takeover done; auto-lands when gates pass label Nov 14, 2026
Pipyakas pushed a commit to Pipyakas/t3code that referenced this pull request Nov 15, 2026
)

Co-authored-by: Julius Marminge <julius0216@outlook.com>
(cherry picked from commit e1f5b68)
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 Author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants