feat(server): T3 MCP tools take explicit thread and project targets#15219

🟠 High

https://github.com/pingdotgg/t3code/blob/333d9d7c8d644aba3ba0b3a08efa7deffde2affc/apps/server/src/mcp/OrchestratorMcpService.ts#L1361

updateScheduledTask lets a low-ceiling client replace a scheduled task's prompt while preserving existing.runtimeMode and existing.interactionMode. Updating a pre-existing full-access task therefore allows a client capped at approval-required to cause its new prompt to run with full-access; clamp the persisted modes to the caller's limits before upserting.

Fixed in da529ee6a9: update_scheduled_task and delete_scheduled_task now refuse a task whose runtime or interaction mode is broader than the caller's limits (runtime_mode_escalation_denied), so a capped caller can't rewrite a full-access task's prompt. Refusing rather than clamping keeps a user-created task's modes from being silently lowered by an agent. Covered by a caller cannot rewrite a scheduled task that runs above its own modes in toolkits/core.test.ts.

🟠 High thread/tools.ts:245

When a client without a calling thread omits projectId, t3_thread_search returns matching titles and content from every project instead of rejecting the request for lacking a search target. Because this schema marks projectId optional and the handler only calls readCaller(), require projectId for such invocations or reject requests with neither target.

Partly agree; changed in 64b9ceff21. An in-thread agent that omits projectId now searches its own project, like the other project tools. A client outside a thread that omits it still searches every project on purpose: per the design for this stack, an approved outside agent can read every project (t3_thread_list/t3_thread_read already reach any thread), so search across them exposes nothing new. The tool description now says this.

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Move launch policy into the launch service.

The handler now selects the target project and default model. It also resolves the caller’s runtime limit before dispatch. Put those decisions in a domain service method, and make the handler call that method. This keeps the new client-launch rules available to other entry points.

As per coding guidelines, “A transport handler does three things: decode the request, call one service method, and map the service's typed errors to the transport's error. Nothing else.”

Source: Coding guidelines

Not changing this in this PR. t3_thread_launch already lived in this handler on main, with its full-access gate and model/mode defaults inline. This PR only adds the client branch: the target project, the project default model, and the ceiling check via the shared resolveRuntimeMode. Its only other entry point is the WebSocket launch, which has its own (user) policy rather than this MCP caller policy, so a service method here would have one caller. Moving all MCP launch policy into a service is a reasonable follow-up, but it is a refactor of existing code, not something this change needs.

@juliusmarminge Thanks for clarifying. I overstated this as a required change for this PR. The launch policy already lived in the handler, and the WebSocket launch uses a different user policy. I withdraw the finding as a blocker for #15219.

Would you like me to open a follow-up issue for moving MCP launch policy into a service?

✅ Review thread resolved.

You are interacting with an AI system.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Enforce credential limits on explicit mutation targets. Both target selectors allow mutations of another thread without comparing that thread's runtime and interaction modes with the caller's limits.

• apps/server/src/mcp/ThreadMetadataMcpService.ts#L149-L149: validate the selected thread against the caller's limits before dispatching a metadata update.
• apps/server/src/mcp/toolkits/pullRequests/handlers.ts#L175-L175: apply the same limit check before link, unlink, or watch mutations use the selected thread.

The PR objective requires thread callers to remain within their mode limits.

✅ Addressed in commit 8f02fe9

Fixed in 8f02fe9c5a: a new assertTargetWithinLimits (shared with readWritableThread) now gates ThreadMetadataMcpService.update and the pull-request link/unlink/watch/unwatch tools whenever the target is not the caller's own thread. A broader target gets runtime_mode_escalation_denied or PullRequestThreadAboveLimitsError. Test: refuses to change a thread that runs above the caller's modes.

@juliusmarminge Thanks for the fix. The shared assertTargetWithinLimits check addresses both mutation paths while preserving updates to the caller’s own thread. The regression test covers rejection of a target above the caller’s modes.

✅ Review thread resolved.

You are interacting with an AI system.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Restrict updates of tasks bound to other threads to the owner, or cap them by the caller's limits.

updateScheduledTask and deleteScheduledTask now look up any task by ID. They no longer check the task's project or bound thread. As a result, any caller with orchestration capability can edit the prompt of any scheduled task. This includes a client with an approval-required ceiling and a thread in another project. The task keeps its stored runtimeMode, for example full-access, and its modelSelection. A low-privilege caller can therefore change the prompt that a full-access scheduled run executes. This bypasses the "caller limits cap writes" rule that sendToThread enforces.

Before the update, check existing.runtimeMode and existing.interactionMode against limits with resolveRuntimeMode and resolveInteractionMode. Apply the same check in deleteScheduledTask.

✅ Confirmed as addressed by @juliusmarminge

Fixed in da529ee6a9: update_scheduled_task and delete_scheduled_task check the task's runtime and interaction modes against the caller's limits with resolveRuntimeMode/resolveInteractionMode and refuse with runtime_mode_escalation_denied. Test: a caller cannot rewrite a scheduled task that runs above its own modes.

@juliusmarminge Thanks for the fix. Both update and delete now use loadScheduledTask to check the task’s runtime and interaction modes against the caller’s limits before mutation. This addresses the privilege-escalation concern.

✅ Review thread resolved.

You are interacting with an AI system.

🟠 High mcp/OrchestratorMcpService.ts:2007

interruptThread lets a caller dispatch run.interrupt for a target whose runtimeMode or interactionMode exceeds the caller's limits. This allows an approval-required credential or restricted thread caller to terminate work on a full-access target; validate both modes before dispatching.

Fixed in 6397a3bfb7: t3_thread_interrupt now checks the target's runtime and interaction modes against the caller's limits before dispatching, like t3_thread_send. Test: a caller cannot interrupt a thread that runs above its own modes.

🟠 High mcp/OrchestratorMcpService.ts:1263

scheduleTask creates tasks for an explicit projectId even when that project no longer exists, so the task is reported as created but later fails when execution resolves the project. The explicit-project path inherits the thread's model and skips projectDefaultModelSelection, while ScheduledTaskService.upsert does not validate the project; validate projectId before persisting.

Fixed in 6397a3bfb7: schedule_task loads the target project first and fails with invalid_request if it doesn't exist; the same lookup now supplies the project default model, so it is one read.

🟠 High mcp/ThreadMetadataMcpService.ts:171

A stale provider-session credential can mutate another thread's metadata after its caller run ends or is replaced. The cross-thread path at 171 only applies the caller shell's mode limits and never verifies an active run or matching providerInstanceId; call threadAccess.assertLiveCaller (or the equivalent live-caller check) before allowing the mutation.

Fixed in 6397a3bfb7: changing another thread's metadata, or another thread's pull requests, now needs the calling thread's live run (active run, matching provider instance, not archived), the same rule readWritableThread applies. Test: refuses another thread's metadata to a thread caller whose run has ended.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Require a live caller run before cross-thread sends and interrupts.

loadCaller loads a thread caller’s modes without checking its active run. This PR also lets loadScopedThread load a target from another project, but sendToThread and interruptThread check only mode limits before writing. A thread credential can remain valid for up to 24 hours after an unclean provider-session stop. During that window, a caller can send to or interrupt another project’s thread after its run ends. For thread callers targeting another thread, apply the same active-run and provider-instance guard as ThreadMetadataMcpService.update before dispatching either write.

✅ Confirmed as addressed by @juliusmarminge

Fixed in 84fcb10407: sendToThread and interruptThread now require a thread caller to own an active run (matching provider instance, not archived) before writing to any thread other than its own, the same guard as ThreadMetadataMcpService.update and the pull-request tools. Reads are unchanged. Test: the cross-project case in OrchestratorMcpService.activity.test.ts now checks that a caller whose run ended can still read but gets parent_not_active from send and interrupt.

@juliusmarminge, thanks for the fix. The supplied code calls assertLiveCallerForOtherThread before both writes and retains the mode-limit checks. The guard rejects archived callers, callers without an active run, and provider-instance mismatches when targeting another thread. Reads remain unchanged.

✅ Review thread resolved.

You are interacting with an AI system.

🟠 High mcp/OrchestratorMcpService.ts:1416

A stale thread credential can create, update, or delete scheduled work in another project after its provider run has ended. loadCaller and loadScheduledTask only load the projection and enforce mode limits; these paths never verify a live caller run or otherwise restrict cross-project mutations. Require the same live-session guard used for cross-thread writes before these operations (and retain project ownership checks where applicable).

Fixed in f191501ac1: schedule_task, update_scheduled_task and delete_scheduled_task now require a thread caller to own an active run before acting on a project other than its own (same guard as cross-thread sends). The mode-ceiling check on existing tasks stays. Scheduling within the caller's own project keeps main's behaviour. Test: the stale-caller case in OrchestratorMcpService.activity.test.ts gets parent_not_active from all three against another project.