feat(mcp): expose model_id, agent_id, reasoning_id on start_workspace

[OdinHoang03]

Summary

Add model_id, agent_id, and reasoning_id as optional string fields on the MCP start_workspace tool and forward them into ExecutorConfig. Non-breaking.

Fixes #3381.

Problem

crates/mcp/src/task_server/tools/task_attempts.rs exposes only executor and variant, even though ExecutorConfig (crates/executors/src/profile.rs) has three more user-selectable override fields — model_id, agent_id, reasoning_id — plus permission_policy. The web UI and REST API reach those fields; MCP clients cannot.

Because the bundled default_profiles.json ships only the DEFAULT variant per executor, variant alone cannot distinguish between e.g. sonnet:medium vs sonnet:high from an MCP caller. That removes a whole axis of cost/quality control for orchestrators.

Change

1. Add three optional schema-annotated fields to StartWorkspaceRequest: model_id, agent_id, reasoning_id — all Option<String>.
2. Destructure them in the handler and forward them into ExecutorConfig, trimming empty strings to None via a small trim_to_option helper.

Scope notes

• permission_policy is intentionally left for a follow-up — it is an enum (PermissionPolicy) rather than a string, so threading it through the MCP schema needs a separate decision on deriving JsonSchema upstream in crates/executors vs mirroring it in the MCP crate.
• The secondary nit in [Feature] Expose model_id + reasoning_id overrides in MCP start_workspace #3381 (enumerating valid variant names in the schema description / adding a list_executor_profiles tool) is also out of scope here.

Compatibility

Existing MCP callers that omit the new fields see identical behaviour — the three fields collapse to None on the wire, which matches the current hard-coded None values. Only callers that opt in get the new behaviour.

Test plan

• cargo check -p mcp on the branch — clean compile.
• cargo fmt -p mcp — no formatting delta.

Checklist

• Rust compiles
• Formatted
• Non-breaking for existing MCP clients
• Linked to tracking issue ([Feature] Expose model_id + reasoning_id overrides in MCP start_workspace #3381)

---

Note

Low Risk
Low risk: adds optional parameters to the MCP start_workspace tool and forwards them into the existing ExecutorConfig without changing defaults for current callers.

Overview
Adds optional model_id, agent_id, and reasoning_id fields to the MCP start_workspace request schema and threads them through the handler into ExecutorConfig when starting a workspace.

Empty/whitespace values are normalized to None via a small trim_to_option helper, preserving existing behavior when the new fields are omitted.

^{Reviewed by Cursor Bugbot for commit e568c47. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 9f22c62. Configure here.}

[cursor]

New helper duplicates existing inline trim logic

Low Severity

The newly introduced trim_to_option helper performs exactly the same trim-and-collapse-to-None logic that already exists inline for prompt and variant in the same function. Introducing the helper but not applying it to the existing identical closures creates duplicated logic within a single function — if the trimming behavior ever changes, it would need updating in three places instead of one.

Additional Locations (2)

• crates/mcp/src/task_server/tools/task_attempts.rs#L138-L146
• crates/mcp/src/task_server/tools/task_attempts.rs#L157-L165

 

^{Reviewed by Cursor Bugbot for commit 9f22c62. Configure here.}

[Juanlucasbg]

Aggressive review summary — PR #3382

30-line feature in crates/mcp/src/task_server/tools/task_attempts.rs that exposes three optional MCP fields (model_id, agent_id, reasoning_id) on start_workspace and forwards them to ExecutorConfig. Pre-PR those fields were hardcoded to None. Verdict: clean — recommend merge.

Why the change is correct

StartWorkspaceRequest gets three new Option<String> fields with #[schemars(description = ...)] so they appear in the MCP tool schema for clients. Each is plumbed via the pattern model_id.and_then(trim_to_option) which:

• Stays None if the client omits the field.
• Returns None if the client sends an empty/whitespace string (avoids forwarding Some("") to executor config and tripping downstream "model not found" errors).
• Otherwise forwards the trimmed value.

trim_to_option signature fn(String) -> Option<String> matches Option::and_then's closure expectation, so the chaining type-checks.

Findings

• Structural: 0 HIGH. Pattern is symmetric across all three fields. Helper is small and reusable.
• Adversarial: 0 HIGH. Empty-string case is explicitly normalized to None. Whitespace-only also collapses to None.
• Security — LOW: An MCP caller can now influence which model/agent runs. If the MCP server is reachable to lower-trust callers than the local user, this is a cost-shift vector (caller could request the most expensive model). Acceptable in the local-tooling threat model where MCP is trusted; worth flagging if remote MCP becomes a thing. Not a regression — the workspace creation surface already gives MCP callers significant control.
• Conventions: PASS. Matches the existing executor/variant field pattern.

NITs

• No length cap on the new strings. A 10MB model_id would be accepted and forwarded. Adding if trimmed.len() > 256 { return None } (or a documented cap) would short-circuit pathological input. Non-blocking.
• No test added. A small #[test] for trim_to_option (None / "" / " " / "x" / " x ") would lock in the contract.
• The MCP schema descriptions are good — they include the example format ('anthropic/claude-sonnet-4-20250514', 'high'/'medium'/'low').

Verdict

Approve.

— Reviewed by automated single-pass review (MCP tool-surface triage; full 4-tool battery skipped — diff is 30 lines, all forwarding, no new validation logic).