fix(server): retain scheduled dispatch refusal reasons#32115

open1
main
GitHub fork: fix/scheduled-dispatch-refusal
opened5d agobyMmacodev00
View on GitHubTwo-way sync enabledPR last imported 2d ago

Problem

When an orchestrator refuses a bound scheduled task's fresh-session send, the task records only Failed to dispatch orchestration command message.dispatch (...). The actionable reason is present in the typed error's direct cause but is lost before lastRunError is persisted.

Related to the diagnostic symptom in #32100. This does not fix its background-work lifecycle problem or close that issue.

Change

After squashing the dispatch result's Effect cause, recognize OrchestratorDispatchError and format its direct string cause with the existing userFacingDispatchErrorMessage. Keep the existing outer-message fallback for missing, empty, generic, or non-string causes. The releaseStuckRun caller retains the unchanged formatter.

The boundary is specific to scheduled sends. The helper accepts a nongeneric direct string; it is not an allowlist or sanitizer. Unknown nested causes, including SQL details and cyclic objects, are not traversed. New-thread launch failures remain behind their existing wrapper.

Scope and approval

This is a small, focused repair of an obvious diagnostic loss: the service already receives a source-defined refusal, and the existing persisted error field drops it. It uses the focused bug-fix exception in CONTRIBUTING.md. No schedule state, bookkeeping, queue, retry, cancellation, session release, admission guard, contract, MCP schema, or client behavior changes.

Overlap checked against current main 6577c6b61851 and exact open heads of #23131 (017184de74c1) and #31366 (a037820fa42b): those PRs own terminal-outcome accounting and retain this formatter loss. This preserves #29889's stack-free diagnostics and follows the existing dispatch formatter precedent in #21027.

Verification

  • Extended the existing fresh-session ScheduledTaskService.test.ts case. Before the source change it failed at the intended assertion: expected the background-work refusal, received the generic dispatch wrapper. Failed status, retained binding, run count and next-run assertions passed first. After the fix, the returned task, direct SQLite read and a newly constructed service all retain the reason. A later successful dispatch clears it.
  • Added opaque-failure controls at the same service layer for defects, strings, interruption, missing/empty/generic wrapper causes, cyclic causes, nested Error text and private SQL detail. These fallback controls enter through the defect channel; the principal refusal regression exercises an actual typed failure. The existing webhook completion-write-failure test also checks that releaseStuckRun still records its safe outer error.
  • vp test run apps/server/src/scheduledTasks/ScheduledTaskService.test.ts apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts apps/server/src/orchestration-v2/UserFacingErrors.test.ts: 73 tests passed.
  • Final vp run check:changed: format, lint, both knip steps, GraphQL, affected typechecks (3 packages), and 88 tests in 5 files passed.
  • Independent Codex review of the exact diff found no blocking correctness or privacy findings for the bounded scheduled-send path.

Runtime evidence: macOS, Node 26.13.0, two separate synthetic file-backed SQLite databases, actual ScheduledTaskService, ThreadManagementService, orchestrator, event/projection persistence and effect worker. The repository's ProviderReplayHarness.layerWithRegistry used the test adapter pattern from ProviderSwitch.integration.test.ts, with Claude capabilities and a session-wide background-work probe returning true after the root completed. No provider CLI or paid inference ran.

  1. Create a thread and complete its first test-provider turn while its session's background probe remains true.
  2. Persist an enabled interval task bound to that thread with threadMode: "fresh_session", then call the real runNow.
  3. Drain the effect worker and inspect the returned task, SQL row and new service readback. Compare against the unchanged main service, with identical production dependencies.
text
1 main 6577c6b61851 this change
2root run completed completed
3live session background probe true true
4lastRunStatus failed failed
5returned dispatch threadId null null
6persisted task binding retained retained
7runCount 1 1
8nextRunAt completion + interval completion + interval
9provider turn count 1 1
10session close count during send 0 0
11fresh-service / SQL readback matches returned task matches returned task
12
13before lastRunError:
14Failed to dispatch orchestration command message.dispatch
15(scheduled-task:task:scheduled-refusal-runtime:1793986433691:manual).
16
17after lastRunError:
18The current session is still running background agents or commands, and a
19fresh session would end them. Wait for them to finish, or press Stop, then
20send the message again.

The main comparison imported the exact main service source with only its relative imports relocated. The patched run imported the worktree service. Assertions also verified that provider sessions and root runs were unchanged; synchronization used worker draining and persisted events.

Not checked: a real Claude process, the reporter's indefinite lifecycle failure, or a UI capture. This proves diagnostic propagation through the real server services with a deterministic test provider.

Model and harness: GPT-6-based Codex; independent Codex review.

Checks

23 successful · 8 skipped
All checks passed
45fa589

Added 1 commit

  • fix(server): retain scheduled dispatch refusal reasons45fa589
Synced from GitHub

Approvability

Verdict: Approved at 45fa589

Macroscope's review found this PR approvable — Scheduled-task failures now preserve readable orchestration refusal reasons instead of generic dispatch errors. The change is limited to failure reporting, with nested defects retaining existing handling to avoid exposing private details.

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

Synced from GitHub

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The service maps orchestrator dispatch errors to user-facing messages when recording failed scheduled-task runs. Tests cover refusal handling, error diagnostics, persisted run state, and completion-bookkeeping failures.

Changes: Scheduled task error reporting

Layer / File(s)Summary
Dispatch error mapping and run records
apps/server/src/scheduledTasks/ScheduledTaskService.ts, apps/server/src/scheduledTasks/ScheduledTaskService.test.ts, apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts
Failed runs use mapped messages for orchestrator dispatch errors and retain existing error formatting for other failures. Tests cover refusal and retry behavior, safe error diagnostics, and completion-bookkeeping failure state.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Possibly related issues

  • pingdotgg/t3code#32100 — The PR records the direct refusal reason in lastRunError, but does not resolve the reported background-work lifecycle failure.

Possibly related PRs

  • pingdotgg/t3code#31366 — That PR adds lastRunError to MCP scheduled-task results; this PR changes the dispatch refusal message stored in that field.

Suggested labels: bugfix

Suggested reviewers: juliusmarminge

Merge Risk: External image Minimal · up to 45fa5

Scheduled runs retain the refusal reason as intended. No actionable merge-blocking issue remains after normal checks.

Pre-merge checks: Passed 4

Check nameStatusExplanation
Title checkPassedThe title clearly identifies the scheduled dispatch refusal reason fix.
Description checkPassedThe description covers the problem, change, scope exception, and verification results. It also identifies checks that were not performed and names the model and harness.
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
External image Recent review info

External image Run configuration

External image Commits

Reviewing files that changed from the base of the PR and between 6577c6b6185167c0e502fa01f9e8e8f2dee6ff25 and 45fa58992207812d240c9e430dc5891716283751.

External image Files selected for processing: 3

  • apps/server/src/scheduledTasks/ScheduledTaskService.test.ts
  • apps/server/src/scheduledTasks/ScheduledTaskService.ts
  • apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.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.