From 5f5abdff0339de3387a5e8050d4d82e03915be64 Mon Sep 17 00:00:00 2001 From: Pixel Perfect Date: Fri, 10 Jul 2026 14:23:23 -0700 Subject: [PATCH 1/2] feat(orchestrator): pass model options through MCP thread targets Delegated tasks and MCP-created threads previously had no way to set the child's model options (reasoning effort, fast mode, ...): target only carried providerInstanceId/driverKind/model and resolveTarget dropped options entirely. Accept canonical and shorthand option selections on every MCP target, validate them against the model's advertised option descriptors, carry them into the child's model selection, and advertise per-model option descriptors in orchestrator_capabilities so agents can discover valid ids and values. --- apps/server/src/mcp/OrchestratorMcpService.ts | 59 ++++++++++++++++++- ...OrchestratorMcpToolkit.integration.test.ts | 59 ++++++++++++++++++- .../src/mcp/toolkits/orchestrator/tools.ts | 4 +- .../contracts/src/orchestratorMcp.test.ts | 25 ++++++++ packages/contracts/src/orchestratorMcp.ts | 11 ++++ 5 files changed, 153 insertions(+), 5 deletions(-) diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index 282fff173a1..fc8a922c347 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -43,6 +43,8 @@ import { type OrchestratorMcpThreadWaitResult, type ProviderInteractionMode, ProviderInstanceId, + type ProviderOptionDescriptor, + type ProviderOptionSelection, type RuntimeMode, type ScheduledTask, type ScheduledTaskUpsertInput, @@ -210,6 +212,38 @@ function providerConstraints( return constraints; } +/** + * Checks requested option selections against the descriptors a model + * advertises. Models without descriptors skip validation entirely (mirroring + * how model slugs are only validated when the provider advertises models). + */ +function invalidOptionSelections( + selections: ReadonlyArray, + descriptors: ReadonlyArray, +): ReadonlyArray { + const problems: Array = []; + for (const selection of selections) { + const descriptor = descriptors.find((candidate) => candidate.id === selection.id); + if (descriptor === undefined) { + const known = descriptors.map((candidate) => candidate.id).join(", "); + problems.push(`Unknown option ${selection.id}; supported options: ${known || "none"}.`); + continue; + } + if (descriptor.type === "boolean" && typeof selection.value !== "boolean") { + problems.push(`Option ${selection.id} expects a boolean value.`); + continue; + } + if ( + descriptor.type === "select" && + !descriptor.options.some((choice) => choice.id === selection.value) + ) { + const choices = descriptor.options.map((choice) => choice.id).join(", "); + problems.push(`Option ${selection.id} must be one of: ${choices}.`); + } + } + return problems; +} + function taskStatusForRun( run: OrchestrationV2Run | undefined, ): OrchestratorMcpDelegateTaskResult["status"] { @@ -676,11 +710,29 @@ const make = Effect.gen(function* () { ); } + const requestedOptions = input.target?.options; + if (requestedOptions !== undefined) { + const descriptors = provider.models.find((candidate) => candidate.slug === model) + ?.capabilities?.optionDescriptors; + const invalid = + descriptors === undefined ? [] : invalidOptionSelections(requestedOptions, descriptors); + if (invalid.length > 0) { + return yield* failure( + "invalid_request", + `Model ${model} on provider ${instanceId} rejected options: ${invalid.join(" ")}`, + ); + } + } + return { modelSelection: - instanceId === inheritedSelection.instanceId && model === inheritedSelection.model + instanceId === inheritedSelection.instanceId && + model === inheritedSelection.model && + requestedOptions === undefined ? inheritedSelection - : { instanceId, model }, + : requestedOptions === undefined + ? { instanceId, model } + : { instanceId, model, options: requestedOptions }, }; }); @@ -919,6 +971,9 @@ const make = Effect.gen(function* () { provider?.models.map((model) => ({ id: model.slug, label: model.name ?? null, + ...(model.capabilities?.optionDescriptors === undefined + ? {} + : { options: model.capabilities.optionDescriptors }), })) ?? [], canRunChildTask: constraints.length === 0, canRunCrossProviderChildTask: constraints.length === 0, diff --git a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts index 4890a2bfc9d..c1fe4ef59e4 100644 --- a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts @@ -22,6 +22,7 @@ import { ProjectId, ProviderDriverKind, ProviderInstanceId, + type ProviderOptionDescriptor, ProviderThreadId, ProviderTurnId, type ScheduledTask, @@ -106,6 +107,7 @@ function makeProviderSnapshot(input: { readonly instanceId: ProviderInstanceId; readonly driver: ProviderDriverKind; readonly model: string; + readonly optionDescriptors?: ReadonlyArray; }): ServerProvider { return { instanceId: input.instanceId, @@ -121,7 +123,10 @@ function makeProviderSnapshot(input: { slug: input.model, name: input.model, isCustom: false, - capabilities: null, + capabilities: + input.optionDescriptors === undefined + ? null + : { optionDescriptors: input.optionDescriptors }, }, ], slashCommands: [], @@ -430,6 +435,18 @@ describe("orchestrator MCP toolkit", () => { instanceId: codexInstanceId, driver: ProviderDriverKind.make("codex"), model: codexModel, + optionDescriptors: [ + { + id: "reasoning", + label: "Reasoning effort", + type: "select", + options: [ + { id: "low", label: "Low" }, + { id: "medium", label: "Medium" }, + { id: "high", label: "High" }, + ], + }, + ], }), makeProviderSnapshot({ instanceId: claudeInstanceId, @@ -580,6 +597,17 @@ describe("orchestrator MCP toolkit", () => { providerInstanceId: "opencode", canRunChildTask: true, }), + // Models advertise their option descriptors so agents can + // discover valid target.options ids and values. + expect.objectContaining({ + providerInstanceId: codexInstanceId, + models: [ + expect.objectContaining({ + id: codexModel, + options: [expect.objectContaining({ id: "reasoning", type: "select" })], + }), + ], + }), ]), }); @@ -751,11 +779,31 @@ describe("orchestrator MCP toolkit", () => { ), ).toHaveLength(1); + // Options outside the model's advertised descriptors are rejected + // before any child thread is created. + const rejectedOptionsCall = yield* invoke("delegate_task", { + task: cancellationPrompt, + target: { + providerInstanceId: codexInstanceId, + model: codexModel, + options: { reasoning: "extreme" }, + }, + mode: "async", + clientRequestId: "delegate-rejected-options-1", + }); + expect(rejectedOptionsCall.structuredContent).toMatchObject({ + _tag: "OrchestratorMcpFailure", + code: "invalid_request", + message: expect.stringContaining("rejected options"), + }); + const cancellableCall = yield* invoke("delegate_task", { task: cancellationPrompt, target: { providerInstanceId: codexInstanceId, model: codexModel, + // Shorthand record form; decodes to the canonical array. + options: { reasoning: "low" }, }, mode: "async", clientRequestId: "delegate-cancel-1", @@ -767,6 +815,15 @@ describe("orchestrator MCP toolkit", () => { yield* waitForProjection(orchestrator, cancellable.childThreadId, (projection) => projection.providerTurns.some((turn) => turn.status === "running"), ); + // The requested model options reach the child thread's selection. + const optionedChild = yield* orchestrator.getThreadProjection( + cancellable.childThreadId, + ); + expect(optionedChild.thread.modelSelection).toEqual({ + instanceId: codexInstanceId, + model: codexModel, + options: [{ id: "reasoning", value: "low" }], + }); const cancelCall = yield* invoke("task_cancel", { taskId: cancellable.taskId, reason: "Parent no longer needs this work.", diff --git a/apps/server/src/mcp/toolkits/orchestrator/tools.ts b/apps/server/src/mcp/toolkits/orchestrator/tools.ts index e6a5abda7f6..3443ac2eae2 100644 --- a/apps/server/src/mcp/toolkits/orchestrator/tools.ts +++ b/apps/server/src/mcp/toolkits/orchestrator/tools.ts @@ -49,7 +49,7 @@ export const OrchestratorCapabilitiesTool = Tool.make("orchestrator_capabilities export const DelegateTaskTool = Tool.make("delegate_task", { description: - "Create a T3-owned child agent thread and run it with only the supplied task prompt, without copying parent conversation history. Provider, model, runtime mode, and interaction mode inherit from the parent unless overridden. Prefer mode='async' and poll task_status for long work; mode='wait' blocks until completion or timeout.", + "Create a T3-owned child agent thread and run it with only the supplied task prompt, without copying parent conversation history. Provider, model, model options (e.g. reasoning effort — see orchestrator_capabilities for valid ids), runtime mode, and interaction mode inherit from the parent unless overridden via target. Prefer mode='async' and poll task_status for long work; mode='wait' blocks until completion or timeout.", parameters: OrchestratorMcpDelegateTaskInput, success: OrchestratorMcpDelegateTaskResult, failure: OrchestratorMcpFailure, @@ -138,7 +138,7 @@ export const DeleteScheduledTaskTool = Tool.make("delete_scheduled_task", { export const CreateThreadsTool = Tool.make("create_threads", { description: - "Create one or more ordinary top-level T3 V2 threads. Each entry may have its own prompt, title, provider instance or driver, model, runtime mode, and interaction mode. Omitted provider/model/settings inherit from the calling thread; entries without prompts create empty threads.", + "Create one or more ordinary top-level T3 V2 threads. Each entry may have its own prompt, title, provider instance or driver, model, model options (e.g. reasoning effort), runtime mode, and interaction mode. Omitted provider/model/settings inherit from the calling thread; entries without prompts create empty threads.", parameters: OrchestratorMcpCreateThreadsInput, success: OrchestratorMcpCreateThreadsResult, failure: OrchestratorMcpFailure, diff --git a/packages/contracts/src/orchestratorMcp.test.ts b/packages/contracts/src/orchestratorMcp.test.ts index 30dbc1e2340..fd4fc921e96 100644 --- a/packages/contracts/src/orchestratorMcp.test.ts +++ b/packages/contracts/src/orchestratorMcp.test.ts @@ -55,6 +55,31 @@ describe("orchestrator MCP contracts", () => { expect(result.summary).toBe("Workspace inspected."); }); + it("decodes target model options in canonical and shorthand shapes", () => { + const canonical = decodeDelegateTaskInput({ + task: "Say hello.", + target: { + providerInstanceId: "codex", + model: "gpt-5.6-luna", + options: [{ id: "reasoning", value: "low" }], + }, + }); + const shorthand = decodeDelegateTaskInput({ + task: "Say hello.", + target: { + providerInstanceId: "codex", + model: "gpt-5.6-luna", + options: { reasoning: "low", fastMode: true }, + }, + }); + + expect(canonical.target?.options).toEqual([{ id: "reasoning", value: "low" }]); + expect(shorthand.target?.options).toEqual([ + { id: "reasoning", value: "low" }, + { id: "fastMode", value: true }, + ]); + }); + it("decodes mixed prompted and empty thread batches", () => { const request = decodeCreateThreadsInput({ clientRequestId: "threads-1", diff --git a/packages/contracts/src/orchestratorMcp.ts b/packages/contracts/src/orchestratorMcp.ts index ce818f879d8..79872630ee7 100644 --- a/packages/contracts/src/orchestratorMcp.ts +++ b/packages/contracts/src/orchestratorMcp.ts @@ -22,6 +22,7 @@ import { OrchestrationV2RunStatus, OrchestrationV2TurnItemStatus, } from "./orchestrationV2.ts"; +import { ProviderOptionDescriptor, ProviderOptionSelections } from "./model.ts"; import { ProviderDriverKind, ProviderInstanceId } from "./providerInstance.ts"; const OrchestratorMcpPrompt = TrimmedNonEmptyString.check(Schema.isMaxLength(120_000)); @@ -32,6 +33,14 @@ export const OrchestratorMcpTarget = Schema.Struct({ providerInstanceId: Schema.optional(ProviderInstanceId), driverKind: Schema.optional(ProviderDriverKind), model: Schema.optional(TrimmedNonEmptyString), + /** + * Model option selections for the child (for example reasoning effort). + * Accepts the canonical `[{ id, value }]` array or the shorthand + * `{ id: value }` record; valid ids come from the option descriptors + * advertised by orchestrator_capabilities. When omitted, options inherit + * from the parent only when the child runs the parent's provider and model. + */ + options: Schema.optional(ProviderOptionSelections), }); export type OrchestratorMcpTarget = typeof OrchestratorMcpTarget.Type; @@ -343,6 +352,8 @@ export const OrchestratorMcpProviderCapability = Schema.Struct({ Schema.Struct({ id: Schema.String, label: Schema.NullOr(Schema.String), + /** Model options a target may select (for example reasoning effort). */ + options: Schema.optional(Schema.Array(ProviderOptionDescriptor)), }), ), canRunChildTask: Schema.Boolean, From 45b3605e5880cea6b3b3016d6ad7ad506a3c87f1 Mon Sep 17 00:00:00 2001 From: Pixel Perfect Date: Fri, 10 Jul 2026 14:38:06 -0700 Subject: [PATCH 2/2] fix(orchestrator): decode target options strictly and reject duplicates Review follow-ups: the shorthand record form previously reused the legacy-tolerant persistence schema, which silently dropped values that are not strings or booleans instead of failing the request; decode it strictly instead. Duplicate option ids now fail validation regardless of descriptor availability, since downstream consumers disagree on whether the first or last value wins. --- apps/server/src/mcp/OrchestratorMcpService.ts | 21 +++++++--- ...OrchestratorMcpToolkit.integration.test.ts | 21 ++++++++++ .../contracts/src/orchestratorMcp.test.ts | 24 ++++++++++++ packages/contracts/src/orchestratorMcp.ts | 39 ++++++++++++++++++- 4 files changed, 97 insertions(+), 8 deletions(-) diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index fc8a922c347..bbb4355e12c 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -213,16 +213,26 @@ function providerConstraints( } /** - * Checks requested option selections against the descriptors a model - * advertises. Models without descriptors skip validation entirely (mirroring - * how model slugs are only validated when the provider advertises models). + * Checks requested option selections for duplicates and, when the model + * advertises option descriptors, against those descriptors. Models without + * descriptors skip the descriptor checks (mirroring how model slugs are only + * validated when the provider advertises models), but duplicate ids always + * fail: downstream consumers disagree on whether the first or last value of + * a duplicated id wins. */ function invalidOptionSelections( selections: ReadonlyArray, - descriptors: ReadonlyArray, + descriptors: ReadonlyArray | undefined, ): ReadonlyArray { const problems: Array = []; + const seen = new Set(); for (const selection of selections) { + if (seen.has(selection.id)) { + problems.push(`Option ${selection.id} was specified more than once.`); + continue; + } + seen.add(selection.id); + if (descriptors === undefined) continue; const descriptor = descriptors.find((candidate) => candidate.id === selection.id); if (descriptor === undefined) { const known = descriptors.map((candidate) => candidate.id).join(", "); @@ -714,8 +724,7 @@ const make = Effect.gen(function* () { if (requestedOptions !== undefined) { const descriptors = provider.models.find((candidate) => candidate.slug === model) ?.capabilities?.optionDescriptors; - const invalid = - descriptors === undefined ? [] : invalidOptionSelections(requestedOptions, descriptors); + const invalid = invalidOptionSelections(requestedOptions, descriptors); if (invalid.length > 0) { return yield* failure( "invalid_request", diff --git a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts index c1fe4ef59e4..e185741740d 100644 --- a/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts @@ -797,6 +797,27 @@ describe("orchestrator MCP toolkit", () => { message: expect.stringContaining("rejected options"), }); + // Duplicate option ids fail fast: downstream consumers disagree on + // whether the first or last value of a duplicated id wins. + const duplicateOptionsCall = yield* invoke("delegate_task", { + task: cancellationPrompt, + target: { + providerInstanceId: codexInstanceId, + model: codexModel, + options: [ + { id: "reasoning", value: "low" }, + { id: "reasoning", value: "high" }, + ], + }, + mode: "async", + clientRequestId: "delegate-duplicate-options-1", + }); + expect(duplicateOptionsCall.structuredContent).toMatchObject({ + _tag: "OrchestratorMcpFailure", + code: "invalid_request", + message: expect.stringContaining("more than once"), + }); + const cancellableCall = yield* invoke("delegate_task", { task: cancellationPrompt, target: { diff --git a/packages/contracts/src/orchestratorMcp.test.ts b/packages/contracts/src/orchestratorMcp.test.ts index fd4fc921e96..c668de287a9 100644 --- a/packages/contracts/src/orchestratorMcp.test.ts +++ b/packages/contracts/src/orchestratorMcp.test.ts @@ -80,6 +80,30 @@ describe("orchestrator MCP contracts", () => { ]); }); + it("rejects target model options that are not strings or booleans", () => { + expect(() => + decodeDelegateTaskInput({ + task: "Say hello.", + target: { + providerInstanceId: "codex", + model: "gpt-5.6-luna", + // Must fail loudly instead of being dropped like legacy persistence. + options: { reasoning: 3 }, + }, + }), + ).toThrow(); + expect(() => + decodeDelegateTaskInput({ + task: "Say hello.", + target: { + providerInstanceId: "codex", + model: "gpt-5.6-luna", + options: [{ id: "reasoning", value: null }], + }, + }), + ).toThrow(); + }); + it("decodes mixed prompted and empty thread batches", () => { const request = decodeCreateThreadsInput({ clientRequestId: "threads-1", diff --git a/packages/contracts/src/orchestratorMcp.ts b/packages/contracts/src/orchestratorMcp.ts index 79872630ee7..00b52072d49 100644 --- a/packages/contracts/src/orchestratorMcp.ts +++ b/packages/contracts/src/orchestratorMcp.ts @@ -1,4 +1,6 @@ +import * as Effect from "effect/Effect"; import * as Schema from "effect/Schema"; +import * as SchemaTransformation from "effect/SchemaTransformation"; import { ContextTransferId, @@ -22,13 +24,46 @@ import { OrchestrationV2RunStatus, OrchestrationV2TurnItemStatus, } from "./orchestrationV2.ts"; -import { ProviderOptionDescriptor, ProviderOptionSelections } from "./model.ts"; +import { + ProviderOptionDescriptor, + ProviderOptionSelection, + ProviderOptionSelectionValue, +} from "./model.ts"; import { ProviderDriverKind, ProviderInstanceId } from "./providerInstance.ts"; const OrchestratorMcpPrompt = TrimmedNonEmptyString.check(Schema.isMaxLength(120_000)); const OrchestratorMcpTitle = TrimmedNonEmptyString.check(Schema.isMaxLength(512)); const OrchestratorMcpClientRequestId = TrimmedNonEmptyString.check(Schema.isMaxLength(256)); +/** + * Shorthand `{ id: value }` record form for target model options. Unlike the + * legacy-tolerant `ProviderOptionSelections` persistence schema, this decodes + * strictly: a value that is not a string or boolean fails the request rather + * than being silently dropped. + */ +const OrchestratorMcpTargetOptionsFromRecord = Schema.Record( + Schema.String, + ProviderOptionSelectionValue, +).pipe( + Schema.decodeTo( + Schema.Array(ProviderOptionSelection), + SchemaTransformation.transformOrFail({ + decode: (record) => + Effect.succeed(Object.entries(record).map(([id, value]) => ({ id, value }))), + encode: (selections: ReadonlyArray) => + Effect.succeed( + Object.fromEntries(selections.map((selection) => [selection.id, selection.value])), + ), + }), + ), +); + +export const OrchestratorMcpTargetOptions = Schema.Union([ + Schema.Array(ProviderOptionSelection), + OrchestratorMcpTargetOptionsFromRecord, +]); +export type OrchestratorMcpTargetOptions = typeof OrchestratorMcpTargetOptions.Type; + export const OrchestratorMcpTarget = Schema.Struct({ providerInstanceId: Schema.optional(ProviderInstanceId), driverKind: Schema.optional(ProviderDriverKind), @@ -40,7 +75,7 @@ export const OrchestratorMcpTarget = Schema.Struct({ * advertised by orchestrator_capabilities. When omitted, options inherit * from the parent only when the child runs the parent's provider and model. */ - options: Schema.optional(ProviderOptionSelections), + options: Schema.optional(OrchestratorMcpTargetOptions), }); export type OrchestratorMcpTarget = typeof OrchestratorMcpTarget.Type;