Skip to content

fix(server): run_scheduled_task_now returns the thread the run launched - #17458

Open
sbansal1999 wants to merge 1 commit into
pingdotgg:mainfrom
sbansal1999:fix/run-scheduled-task-now-thread-id
Open

sbansal1999 wants to merge 1 commit into
pingdotgg:mainfrom
sbansal1999:fix/run-scheduled-task-now-thread-id

Conversation

@sbansal1999

@sbansal1999 sbansal1999 commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Problem

run_scheduled_task_now returns threadId: null for scheduled tasks that launch a fresh thread per run: unbound tasks, created with bindToCurrentThread=false or scheduled into another project. The handler returned task.threadId, which is the task's bound thread. So the caller can't tell which thread the run created, and can't t3_thread_wait on it, read it, or link it. t3_thread_list doesn't expose scheduledTaskId, so there's no reliable way to find it afterwards.

Expected: the tool returns the thread the run posted to or launched. For a bound task that's the bound thread. For a fresh-thread task it's the thread the run created.

Change

  • runTask in ScheduledTaskService now returns { task, threadId }, where threadId is the bound thread or the thread ThreadLaunchService.launch created (null if the launch failed). runNow passes it through.
  • The MCP handler returns it in the existing threadId field. The tool's output schema and packages/contracts are unchanged; only the server-side runNow type gained threadId.

Scope and approval

There's no prior issue. This is a small fix for an obvious bug: the tool's threadId output never carried information the caller didn't already have. For bound tasks it echoed the known binding, and for fresh-thread tasks it was always null. The closed #8731, which first proposed this tool, meant it to return the launched thread's identity. The change is limited to the server's scheduled-task service and this one MCP tool, with no client or contract changes.

Verification

  • ThreadLaunchService.test.ts now asserts that runNow returns the thread the run posted to, for new-thread and bound-thread tasks (each with user- and agent-created tasks).
  • With the fix reverted, those 4 cases fail: expected undefined to equal 'thread:project:…' for new threads and expected undefined to equal 'thread:existing' for bound threads. With the fix, they pass.
  • vp test run on ThreadLaunchService.test.ts plus src/scheduledTasks/ (8 files, 107 tests), and on OrchestratorMcpToolkit.integration.test.ts and ClaudeAutomaticDelivery.integration.test.ts (2 files, 5 tests): all pass.
  • apps/server typecheck: no errors.
  • Not checked: calling the tool against a live server.

Implemented by Claude Opus 5.5 via Claude Code in T3 Code.

🤖 Generated with Claude 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 9, 2026
@sbansal1999 sbansal1999 changed the title fix(server): run_scheduled_task_now returns the thread the run launched fix(server): return launched thread ID for scheduled task runs Oct 9, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 07cb1a7

Macroscope's review found this PR approvable — This is a contained scheduler bug fix that propagates the actual launched thread ID through an existing response field without changing task execution, schemas, defaults, or deployment behavior. Existing bound-thread, failure, scheduled, and webhook paths remain accounted for, with coverage for new and existing threads.

No code changes detected at e16fab3. Prior analysis still applies.

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

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5b9f63d6-15b9-4c6c-bcd6-04a8767c5244
📥 Commits

Reviewing files that changed from the base of the PR and between e16fab3 and 405ee64.

📒 Files selected for processing: 3
  • apps/server/src/mcp/toolkits/thread/handlers.ts
  • apps/server/src/orchestration-v2/ThreadLaunchService.test.ts
  • apps/server/src/scheduledTasks/ScheduledTaskService.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.

📝 Walkthrough

Scheduled task runs now return a threadId alongside the task. The result contains the bound thread ID, the launched thread ID after a successful launch, or null when no thread is available. The MCP handler returns this value.

Changes: Scheduled task run results

Layer / File(s) Summary
Resolve the thread ID for each run
apps/server/src/scheduledTasks/ScheduledTaskService.ts
runTask returns the task with a threadId. It uses the bound thread ID or the launched thread ID after a successful launch. Non-dispatch paths and failed launches return null.
Return and consume the run result
apps/server/src/scheduledTasks/ScheduledTaskService.ts, apps/server/src/mcp/toolkits/thread/handlers.ts, apps/server/src/orchestration-v2/ThreadLaunchService.test.ts
runNow returns the expanded result. Webhook handling reads the task from the result. The MCP handler reads its threadId, and a test checks that value against the selected thread.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 405ee

No actionable issue is established for this change; it is ready to merge after normal checks.

Security architecture risk: 🔵 Low · up to 405ee

The returned identifier comes from the task’s bound thread or the thread launched for that run. Existing full-access requirements and task-selected execution targets remain unchanged. No introduced authorization bypass was established, although concurrent replacement and recovery behavior are not fully validated.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The relevant authority is environment-level rather than limited to the calling thread: an authorized caller selects an existing scheduled task, whose configuration determines the target project and thread. The change makes a fresh run’s thread directly addressable to that caller but does not add target-selection inputs or establish a new tenant or service boundary.

Trust Boundaries and Controls

  • observed — The unchanged MCP wrapper rejects read-only clients, requires orchestration capability, checks live provider ownership for thread callers, and requires full-access/default authority. The handler also verifies task existence before calling runNow. Returning the thread identity does not replace these controls.

Resilience and Maintainability Implications

  • observed — The result refactor preserves in-memory run reservation, dispatch failure capture, best-effort stuck-run recovery, and guaranteed reservation release. Webhook failure accounting still depends on task status rather than the new identifier. No changed failure-containment mechanism was found in the base-to-head comparison.
Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: returning the thread launched by run_scheduled_task_now.
Description check Passed The description includes the problem, expected behavior, implementation details, scope justification, focused verification results, and the unverified live-server check. It is complete and aligned wit…
Linked Issues check Passed No active directly linked targets remain. Issue #8731 is closed and supplies historical context only. Therefore, this PR has no linked-issue coding requirements to assess.
Out of Scope Changes check Passed The handler, ScheduledTaskService, and focused test changes implement the stated threadId behavior for run_scheduled_task_now. The changes remain within the server scheduled-task service and MCP tool …
Full details: Description check

Explanation

The description includes the problem, expected behavior, implementation details, scope justification, focused verification results, and the unverified live-server check. It is complete and aligned with the template.

Full details: Out of Scope Changes check

Explanation

The handler, ScheduledTaskService, and focused test changes implement the stated threadId behavior for run_scheduled_task_now. The changes remain within the server scheduled-task service and MCP tool scope. The test changes support that implementation.

✨ 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.

@sbansal1999 sbansal1999 changed the title fix(server): return launched thread ID for scheduled task runs then make the description accod Oct 9, 2026
@sbansal1999 sbansal1999 changed the title then make the description accod fix(server): run_scheduled_task_now returns the thread the run launched Oct 9, 2026
@sbansal1999
sbansal1999 force-pushed the fix/run-scheduled-task-now-thread-id branch from 07cb1a7 to e16fab3 Compare October 9, 2026 12:23
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 11, 2026 — with ChatGPT Codex Connector
AdEx-Partners-DE added a commit to AdEx-Partners-DE/t3code that referenced this pull request Oct 11, 2026
AdEx-Partners-DE added a commit to AdEx-Partners-DE/t3code that referenced this pull request Oct 11, 2026
…result shape of upstream pingdotgg#17458

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AdEx-Partners-DE added a commit to AdEx-Partners-DE/t3code that referenced this pull request Oct 11, 2026
…stream pingdotgg#17589)

Applies the pull request's own 43 commits as one change, without the
background-update stack it sits on (pingdotgg#17586, pingdotgg#17588): the fork is a self-built
server and must not install official updates over itself, so the
automaticUpdates setting is left out. Fork settings keep their entries in the
MCP environment tools. The run-now result shape from pingdotgg#17458 is carried into
the new return path, and the restart-continuation test drives the timed poll
by name because the fork registers a second poll for idle tasks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fresh-thread-per-run tasks have no bound thread, so the tool returned
threadId: null. runTask now reports the thread it posted to or launched,
and runNow surfaces it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sbansal1999
sbansal1999 force-pushed the fix/run-scheduled-task-now-thread-id branch from e16fab3 to 405ee64 Compare October 16, 2026 08:00

This branch has not been deployed

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

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews 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.

2 participants