feat: allow approved external MCP tools in root plan mode (#24509)

## Summary

Allow root plan-mode chats to use MCP tools from external servers that
an admin has explicitly approved for plan mode. Workspace MCP and
plan-mode subagents remain blocked.

## Problem

`chatd.go` excluded every MCP tool when `isPlanModeTurn` was true, so
planning had no access to tools like docs search, ticketing, etc.
Lifting that guard wholesale was unsafe: `mcp_server_configs` already
has centralized admin governance, but workspace-local MCP (discovered
from agent `.mcp.json`) does not, and subagents use a narrower trust
boundary.

## Fix

Add an admin-controlled per-server `allow_in_plan_mode` flag (default
`false`) and gate plan-mode MCP access on it.

### Backend / schema
- New migration `000472_mcp_server_allow_in_plan_mode.{up,down}.sql` and
matching fixture update.
- `mcpserverconfigs.sql` + generated code: persist and read the new
column.
- `codersdk/mcp.go`: thread the field through `MCPServerConfig`,
`Create*`, and `Update*` request types.
- `coderd/mcp.go`: validate, persist, and return the flag in
get/list/create/update handlers.

### chatd
- `coderd/x/chatd/chatd.go`: pre-filter selected external MCP configs by
`AllowInPlanMode` before calling `mcpclient.ConnectAll` on plan-mode
root turns. Workspace MCP discovery is skipped entirely on plan-mode
turns.
- Single helper decides whether a tool is available in plan mode, used
both at construction and for active-tool filtering (defense in depth).
Plan-mode subagents, dynamic tools, provider-native tools, computer-use,
and workspace MCP stay unchanged.
- `coderd/x/chatd/prompt.go`: update the root plan-mode overlay text to
match the new boundary.

### UI
- `MCPServerAdminPanel.tsx`: add an explicit toggle ("Allow all tools
from this MCP server in root plan mode") next to the existing governance
controls.
- Regenerated `site/src/api/typesGenerated.ts`.

### Docs
- `docs/ai-coder/agents/architecture.md`: replace the blanket "MCP is
unavailable in plan mode" note with the new root-only, external-only,
admin-approved policy. Explicitly call out that workspace MCP and
plan-mode subagents are still excluded.

### Tests
- Plan-mode visibility (approved vs non-approved external server).
- Plan-mode invocation of an approved external MCP tool.
- End-to-end plan-mode workflow that uses an approved MCP tool and then
reaches `propose_plan`.
- Regressions: workspace MCP still excluded in plan mode; plan-mode
subagents still on the restricted tool boundary; existing tool
allow/deny list filtering still applies.

## Policy precedence

`allow_in_plan_mode` is an **additional** requirement on top of existing
`enabled`, availability, chat-selected / forced server IDs, and tool
allow/deny lists. It approves **all tools on that server** for root plan
mode; a per-tool plan allowlist is deliberately deferred.

## Follow-ups (explicitly out of scope)

- Whether plan-mode subagents should inherit approved external MCP
tools.
- Workspace-local MCP safety model (agent-side `.mcp.json` schema vs. a
coderd-managed workspace MCP config).

## Validation

- `go vet ./coderd/x/chatd/...`
- `go test ./coderd/x/chatd -run 'TestPlan.*|TestMCP.*' -count=1`
- `go test ./coderd/x/chatd -count=1 -timeout 5m` (full chatd suite)
- `make fmt` (no diff)

> Mux opened this PR on Mike's behalf.
This commit is contained in:
Michael Suchacz
2026-04-21 12:26:12 +02:00
committed by GitHub
parent c968a1f3a3
commit 9d0469fc4c
21 changed files with 998 additions and 113 deletions
+1
View File
@@ -1788,6 +1788,7 @@ CREATE TABLE mcp_server_configs (
created_at timestamp with time zone DEFAULT now() NOT NULL, created_at timestamp with time zone DEFAULT now() NOT NULL,
updated_at timestamp with time zone DEFAULT now() NOT NULL, updated_at timestamp with time zone DEFAULT now() NOT NULL,
model_intent boolean DEFAULT false NOT NULL, model_intent boolean DEFAULT false NOT NULL,
allow_in_plan_mode boolean DEFAULT false NOT NULL,
CONSTRAINT mcp_server_configs_auth_type_check CHECK ((auth_type = ANY (ARRAY['none'::text, 'oauth2'::text, 'api_key'::text, 'custom_headers'::text]))), CONSTRAINT mcp_server_configs_auth_type_check CHECK ((auth_type = ANY (ARRAY['none'::text, 'oauth2'::text, 'api_key'::text, 'custom_headers'::text]))),
CONSTRAINT mcp_server_configs_availability_check CHECK ((availability = ANY (ARRAY['force_on'::text, 'default_on'::text, 'default_off'::text]))), CONSTRAINT mcp_server_configs_availability_check CHECK ((availability = ANY (ARRAY['force_on'::text, 'default_on'::text, 'default_off'::text]))),
CONSTRAINT mcp_server_configs_transport_check CHECK ((transport = ANY (ARRAY['streamable_http'::text, 'sse'::text]))) CONSTRAINT mcp_server_configs_transport_check CHECK ((transport = ANY (ARRAY['streamable_http'::text, 'sse'::text])))
@@ -0,0 +1,2 @@
ALTER TABLE mcp_server_configs
DROP COLUMN allow_in_plan_mode;
@@ -0,0 +1,2 @@
ALTER TABLE mcp_server_configs
ADD COLUMN allow_in_plan_mode BOOLEAN NOT NULL DEFAULT false;
@@ -0,0 +1,6 @@
-- Migration 472 adds allow_in_plan_mode with a default of false.
-- Flip the existing fixture row to true here so fixture data exercises
-- the non-default state only after the column exists.
UPDATE mcp_server_configs
SET allow_in_plan_mode = TRUE
WHERE id = 'a1b2c3d4-e5f6-7890-abcd-ef1234567890';
+1
View File
@@ -4729,6 +4729,7 @@ type MCPServerConfig struct {
CreatedAt time.Time `db:"created_at" json:"created_at"` CreatedAt time.Time `db:"created_at" json:"created_at"`
UpdatedAt time.Time `db:"updated_at" json:"updated_at"` UpdatedAt time.Time `db:"updated_at" json:"updated_at"`
ModelIntent bool `db:"model_intent" json:"model_intent"` ModelIntent bool `db:"model_intent" json:"model_intent"`
AllowInPlanMode bool `db:"allow_in_plan_mode" json:"allow_in_plan_mode"`
} }
type MCPServerUserToken struct { type MCPServerUserToken struct {
+27 -12
View File
@@ -12764,7 +12764,7 @@ func (q *sqlQuerier) DeleteMCPServerUserToken(ctx context.Context, arg DeleteMCP
const getEnabledMCPServerConfigs = `-- name: GetEnabledMCPServerConfigs :many const getEnabledMCPServerConfigs = `-- name: GetEnabledMCPServerConfigs :many
SELECT SELECT
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode
FROM FROM
mcp_server_configs mcp_server_configs
WHERE WHERE
@@ -12811,6 +12811,7 @@ func (q *sqlQuerier) GetEnabledMCPServerConfigs(ctx context.Context) ([]MCPServe
&i.CreatedAt, &i.CreatedAt,
&i.UpdatedAt, &i.UpdatedAt,
&i.ModelIntent, &i.ModelIntent,
&i.AllowInPlanMode,
); err != nil { ); err != nil {
return nil, err return nil, err
} }
@@ -12827,7 +12828,7 @@ func (q *sqlQuerier) GetEnabledMCPServerConfigs(ctx context.Context) ([]MCPServe
const getForcedMCPServerConfigs = `-- name: GetForcedMCPServerConfigs :many const getForcedMCPServerConfigs = `-- name: GetForcedMCPServerConfigs :many
SELECT SELECT
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode
FROM FROM
mcp_server_configs mcp_server_configs
WHERE WHERE
@@ -12875,6 +12876,7 @@ func (q *sqlQuerier) GetForcedMCPServerConfigs(ctx context.Context) ([]MCPServer
&i.CreatedAt, &i.CreatedAt,
&i.UpdatedAt, &i.UpdatedAt,
&i.ModelIntent, &i.ModelIntent,
&i.AllowInPlanMode,
); err != nil { ); err != nil {
return nil, err return nil, err
} }
@@ -12891,7 +12893,7 @@ func (q *sqlQuerier) GetForcedMCPServerConfigs(ctx context.Context) ([]MCPServer
const getMCPServerConfigByID = `-- name: GetMCPServerConfigByID :one const getMCPServerConfigByID = `-- name: GetMCPServerConfigByID :one
SELECT SELECT
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode
FROM FROM
mcp_server_configs mcp_server_configs
WHERE WHERE
@@ -12930,13 +12932,14 @@ func (q *sqlQuerier) GetMCPServerConfigByID(ctx context.Context, id uuid.UUID) (
&i.CreatedAt, &i.CreatedAt,
&i.UpdatedAt, &i.UpdatedAt,
&i.ModelIntent, &i.ModelIntent,
&i.AllowInPlanMode,
) )
return i, err return i, err
} }
const getMCPServerConfigBySlug = `-- name: GetMCPServerConfigBySlug :one const getMCPServerConfigBySlug = `-- name: GetMCPServerConfigBySlug :one
SELECT SELECT
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode
FROM FROM
mcp_server_configs mcp_server_configs
WHERE WHERE
@@ -12975,13 +12978,14 @@ func (q *sqlQuerier) GetMCPServerConfigBySlug(ctx context.Context, slug string)
&i.CreatedAt, &i.CreatedAt,
&i.UpdatedAt, &i.UpdatedAt,
&i.ModelIntent, &i.ModelIntent,
&i.AllowInPlanMode,
) )
return i, err return i, err
} }
const getMCPServerConfigs = `-- name: GetMCPServerConfigs :many const getMCPServerConfigs = `-- name: GetMCPServerConfigs :many
SELECT SELECT
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode
FROM FROM
mcp_server_configs mcp_server_configs
ORDER BY ORDER BY
@@ -13026,6 +13030,7 @@ func (q *sqlQuerier) GetMCPServerConfigs(ctx context.Context) ([]MCPServerConfig
&i.CreatedAt, &i.CreatedAt,
&i.UpdatedAt, &i.UpdatedAt,
&i.ModelIntent, &i.ModelIntent,
&i.AllowInPlanMode,
); err != nil { ); err != nil {
return nil, err return nil, err
} }
@@ -13042,7 +13047,7 @@ func (q *sqlQuerier) GetMCPServerConfigs(ctx context.Context) ([]MCPServerConfig
const getMCPServerConfigsByIDs = `-- name: GetMCPServerConfigsByIDs :many const getMCPServerConfigsByIDs = `-- name: GetMCPServerConfigsByIDs :many
SELECT SELECT
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode
FROM FROM
mcp_server_configs mcp_server_configs
WHERE WHERE
@@ -13089,6 +13094,7 @@ func (q *sqlQuerier) GetMCPServerConfigsByIDs(ctx context.Context, ids []uuid.UU
&i.CreatedAt, &i.CreatedAt,
&i.UpdatedAt, &i.UpdatedAt,
&i.ModelIntent, &i.ModelIntent,
&i.AllowInPlanMode,
); err != nil { ); err != nil {
return nil, err return nil, err
} }
@@ -13206,6 +13212,7 @@ INSERT INTO mcp_server_configs (
availability, availability,
enabled, enabled,
model_intent, model_intent,
allow_in_plan_mode,
created_by, created_by,
updated_by updated_by
) VALUES ( ) VALUES (
@@ -13232,11 +13239,12 @@ INSERT INTO mcp_server_configs (
$21::text, $21::text,
$22::boolean, $22::boolean,
$23::boolean, $23::boolean,
$24::uuid, $24::boolean,
$25::uuid $25::uuid,
$26::uuid
) )
RETURNING RETURNING
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode
` `
type InsertMCPServerConfigParams struct { type InsertMCPServerConfigParams struct {
@@ -13263,6 +13271,7 @@ type InsertMCPServerConfigParams struct {
Availability string `db:"availability" json:"availability"` Availability string `db:"availability" json:"availability"`
Enabled bool `db:"enabled" json:"enabled"` Enabled bool `db:"enabled" json:"enabled"`
ModelIntent bool `db:"model_intent" json:"model_intent"` ModelIntent bool `db:"model_intent" json:"model_intent"`
AllowInPlanMode bool `db:"allow_in_plan_mode" json:"allow_in_plan_mode"`
CreatedBy uuid.UUID `db:"created_by" json:"created_by"` CreatedBy uuid.UUID `db:"created_by" json:"created_by"`
UpdatedBy uuid.UUID `db:"updated_by" json:"updated_by"` UpdatedBy uuid.UUID `db:"updated_by" json:"updated_by"`
} }
@@ -13292,6 +13301,7 @@ func (q *sqlQuerier) InsertMCPServerConfig(ctx context.Context, arg InsertMCPSer
arg.Availability, arg.Availability,
arg.Enabled, arg.Enabled,
arg.ModelIntent, arg.ModelIntent,
arg.AllowInPlanMode,
arg.CreatedBy, arg.CreatedBy,
arg.UpdatedBy, arg.UpdatedBy,
) )
@@ -13325,6 +13335,7 @@ func (q *sqlQuerier) InsertMCPServerConfig(ctx context.Context, arg InsertMCPSer
&i.CreatedAt, &i.CreatedAt,
&i.UpdatedAt, &i.UpdatedAt,
&i.ModelIntent, &i.ModelIntent,
&i.AllowInPlanMode,
) )
return i, err return i, err
} }
@@ -13356,12 +13367,13 @@ SET
availability = $21::text, availability = $21::text,
enabled = $22::boolean, enabled = $22::boolean,
model_intent = $23::boolean, model_intent = $23::boolean,
updated_by = $24::uuid, allow_in_plan_mode = $24::boolean,
updated_by = $25::uuid,
updated_at = NOW() updated_at = NOW()
WHERE WHERE
id = $25::uuid id = $26::uuid
RETURNING RETURNING
id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode
` `
type UpdateMCPServerConfigParams struct { type UpdateMCPServerConfigParams struct {
@@ -13388,6 +13400,7 @@ type UpdateMCPServerConfigParams struct {
Availability string `db:"availability" json:"availability"` Availability string `db:"availability" json:"availability"`
Enabled bool `db:"enabled" json:"enabled"` Enabled bool `db:"enabled" json:"enabled"`
ModelIntent bool `db:"model_intent" json:"model_intent"` ModelIntent bool `db:"model_intent" json:"model_intent"`
AllowInPlanMode bool `db:"allow_in_plan_mode" json:"allow_in_plan_mode"`
UpdatedBy uuid.UUID `db:"updated_by" json:"updated_by"` UpdatedBy uuid.UUID `db:"updated_by" json:"updated_by"`
ID uuid.UUID `db:"id" json:"id"` ID uuid.UUID `db:"id" json:"id"`
} }
@@ -13417,6 +13430,7 @@ func (q *sqlQuerier) UpdateMCPServerConfig(ctx context.Context, arg UpdateMCPSer
arg.Availability, arg.Availability,
arg.Enabled, arg.Enabled,
arg.ModelIntent, arg.ModelIntent,
arg.AllowInPlanMode,
arg.UpdatedBy, arg.UpdatedBy,
arg.ID, arg.ID,
) )
@@ -13450,6 +13464,7 @@ func (q *sqlQuerier) UpdateMCPServerConfig(ctx context.Context, arg UpdateMCPSer
&i.CreatedAt, &i.CreatedAt,
&i.UpdatedAt, &i.UpdatedAt,
&i.ModelIntent, &i.ModelIntent,
&i.AllowInPlanMode,
) )
return i, err return i, err
} }
@@ -78,6 +78,7 @@ INSERT INTO mcp_server_configs (
availability, availability,
enabled, enabled,
model_intent, model_intent,
allow_in_plan_mode,
created_by, created_by,
updated_by updated_by
) VALUES ( ) VALUES (
@@ -104,6 +105,7 @@ INSERT INTO mcp_server_configs (
@availability::text, @availability::text,
@enabled::boolean, @enabled::boolean,
@model_intent::boolean, @model_intent::boolean,
@allow_in_plan_mode::boolean,
@created_by::uuid, @created_by::uuid,
@updated_by::uuid @updated_by::uuid
) )
@@ -137,6 +139,7 @@ SET
availability = @availability::text, availability = @availability::text,
enabled = @enabled::boolean, enabled = @enabled::boolean,
model_intent = @model_intent::boolean, model_intent = @model_intent::boolean,
allow_in_plan_mode = @allow_in_plan_mode::boolean,
updated_by = @updated_by::uuid, updated_by = @updated_by::uuid,
updated_at = NOW() updated_at = NOW()
WHERE WHERE
+14 -4
View File
@@ -171,6 +171,7 @@ func (api *API) createMCPServerConfig(rw http.ResponseWriter, r *http.Request) {
Availability: strings.TrimSpace(req.Availability), Availability: strings.TrimSpace(req.Availability),
Enabled: req.Enabled, Enabled: req.Enabled,
ModelIntent: req.ModelIntent, ModelIntent: req.ModelIntent,
AllowInPlanMode: req.AllowInPlanMode,
CreatedBy: apiKey.UserID, CreatedBy: apiKey.UserID,
UpdatedBy: apiKey.UserID, UpdatedBy: apiKey.UserID,
}) })
@@ -258,6 +259,7 @@ func (api *API) createMCPServerConfig(rw http.ResponseWriter, r *http.Request) {
Availability: inserted.Availability, Availability: inserted.Availability,
Enabled: inserted.Enabled, Enabled: inserted.Enabled,
ModelIntent: inserted.ModelIntent, ModelIntent: inserted.ModelIntent,
AllowInPlanMode: inserted.AllowInPlanMode,
UpdatedBy: apiKey.UserID, UpdatedBy: apiKey.UserID,
}) })
if err != nil { if err != nil {
@@ -326,6 +328,7 @@ func (api *API) createMCPServerConfig(rw http.ResponseWriter, r *http.Request) {
Availability: strings.TrimSpace(req.Availability), Availability: strings.TrimSpace(req.Availability),
Enabled: req.Enabled, Enabled: req.Enabled,
ModelIntent: req.ModelIntent, ModelIntent: req.ModelIntent,
AllowInPlanMode: req.AllowInPlanMode,
CreatedBy: apiKey.UserID, CreatedBy: apiKey.UserID,
UpdatedBy: apiKey.UserID, UpdatedBy: apiKey.UserID,
}) })
@@ -580,6 +583,11 @@ func (api *API) updateMCPServerConfig(rw http.ResponseWriter, r *http.Request) {
modelIntent = *req.ModelIntent modelIntent = *req.ModelIntent
} }
allowInPlanMode := existing.AllowInPlanMode
if req.AllowInPlanMode != nil {
allowInPlanMode = *req.AllowInPlanMode
}
// When auth_type changes, clear fields belonging to the // When auth_type changes, clear fields belonging to the
// previous auth type so stale secrets don't persist. // previous auth type so stale secrets don't persist.
if authType != existing.AuthType { if authType != existing.AuthType {
@@ -648,6 +656,7 @@ func (api *API) updateMCPServerConfig(rw http.ResponseWriter, r *http.Request) {
Availability: availability, Availability: availability,
Enabled: enabled, Enabled: enabled,
ModelIntent: modelIntent, ModelIntent: modelIntent,
AllowInPlanMode: allowInPlanMode,
UpdatedBy: apiKey.UserID, UpdatedBy: apiKey.UserID,
ID: existing.ID, ID: existing.ID,
}) })
@@ -1129,10 +1138,11 @@ func convertMCPServerConfig(config database.MCPServerConfig) codersdk.MCPServerC
Availability: config.Availability, Availability: config.Availability,
Enabled: config.Enabled, Enabled: config.Enabled,
ModelIntent: config.ModelIntent, ModelIntent: config.ModelIntent,
CreatedAt: config.CreatedAt, AllowInPlanMode: config.AllowInPlanMode,
UpdatedAt: config.UpdatedAt, CreatedAt: config.CreatedAt,
UpdatedAt: config.UpdatedAt,
} }
} }
+20 -5
View File
@@ -100,37 +100,52 @@ func TestMCPServerConfigsCRUD(t *testing.T) {
require.Equal(t, "client-id-123", created.OAuth2ClientID) require.Equal(t, "client-id-123", created.OAuth2ClientID)
require.Equal(t, "default_on", created.Availability) require.Equal(t, "default_on", created.Availability)
require.True(t, created.Enabled) require.True(t, created.Enabled)
require.False(t, created.AllowInPlanMode)
// Verify the secret is indicated but never returned. // Verify the secret is indicated but never returned.
require.True(t, created.HasOAuth2Secret) require.True(t, created.HasOAuth2Secret)
// Verify the config appears in the list. // Verify the config appears in the list and direct get responses.
configs, err := client.MCPServerConfigs(ctx) configs, err := client.MCPServerConfigs(ctx)
require.NoError(t, err) require.NoError(t, err)
require.Len(t, configs, 1) require.Len(t, configs, 1)
require.Equal(t, created.ID, configs[0].ID) require.Equal(t, created.ID, configs[0].ID)
require.True(t, configs[0].HasOAuth2Secret) require.True(t, configs[0].HasOAuth2Secret)
require.False(t, configs[0].AllowInPlanMode)
// Update display name and availability. fetched, err := client.MCPServerConfigByID(ctx, created.ID)
require.NoError(t, err)
require.Equal(t, created.ID, fetched.ID)
require.False(t, fetched.AllowInPlanMode)
// Update display name, availability, and allow_in_plan_mode.
newName := "Renamed Server" newName := "Renamed Server"
newAvail := "force_on" newAvail := "force_on"
allowInPlanMode := true
updated, err := client.UpdateMCPServerConfig(ctx, created.ID, codersdk.UpdateMCPServerConfigRequest{ updated, err := client.UpdateMCPServerConfig(ctx, created.ID, codersdk.UpdateMCPServerConfigRequest{
DisplayName: &newName, DisplayName: &newName,
Availability: &newAvail, Availability: &newAvail,
AllowInPlanMode: &allowInPlanMode,
}) })
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, "Renamed Server", updated.DisplayName) require.Equal(t, "Renamed Server", updated.DisplayName)
require.Equal(t, "force_on", updated.Availability) require.Equal(t, "force_on", updated.Availability)
require.True(t, updated.AllowInPlanMode)
// Unchanged fields should remain the same. // Unchanged fields should remain the same.
require.Equal(t, "my-mcp-server", updated.Slug) require.Equal(t, "my-mcp-server", updated.Slug)
require.Equal(t, "oauth2", updated.AuthType) require.Equal(t, "oauth2", updated.AuthType)
// Verify the update took effect through the list. // Verify the update took effect through the list and direct get.
configs, err = client.MCPServerConfigs(ctx) configs, err = client.MCPServerConfigs(ctx)
require.NoError(t, err) require.NoError(t, err)
require.Len(t, configs, 1) require.Len(t, configs, 1)
require.Equal(t, "Renamed Server", configs[0].DisplayName) require.Equal(t, "Renamed Server", configs[0].DisplayName)
require.Equal(t, "force_on", configs[0].Availability) require.Equal(t, "force_on", configs[0].Availability)
require.True(t, configs[0].AllowInPlanMode)
fetched, err = client.MCPServerConfigByID(ctx, created.ID)
require.NoError(t, err)
require.True(t, fetched.AllowInPlanMode)
// Delete it. // Delete it.
err = client.DeleteMCPServerConfig(ctx, created.ID) err = client.DeleteMCPServerConfig(ctx, created.ID)
+142 -53
View File
@@ -5148,40 +5148,99 @@ func isExploreSubagentMode(mode database.NullChatMode) bool {
return mode.Valid && mode.ChatMode == database.ChatModeExplore return mode.Valid && mode.ChatMode == database.ChatModeExplore
} }
func allowedPlanToolNames( func filterExternalMCPConfigsForTurn(
allTools []fantasy.AgentTool, configs []database.MCPServerConfig,
mode database.NullChatPlanMode,
parentChatID uuid.NullUUID, parentChatID uuid.NullUUID,
) []string { ) ([]database.MCPServerConfig, map[uuid.UUID]struct{}) {
isRootChat := !parentChatID.Valid if !mode.Valid || mode.ChatPlanMode != database.ChatPlanModePlan {
builtinPlanPolicy := map[string]bool{ return configs, nil
"read_file": true, }
"write_file": isRootChat, if parentChatID.Valid {
"edit_files": isRootChat, // Plan-mode subagents do not receive external MCP tools because
"execute": true, // their trust boundary is narrower than the root chat's.
"process_output": true, return nil, map[uuid.UUID]struct{}{}
"process_list": false,
"process_signal": false,
"list_templates": isRootChat,
"read_template": isRootChat,
"create_workspace": isRootChat,
"start_workspace": isRootChat,
"propose_plan": isRootChat,
"spawn_agent": isRootChat,
"spawn_explore_agent": isRootChat,
"wait_agent": isRootChat,
"message_agent": false,
"close_agent": false,
"spawn_computer_use_agent": false,
"read_skill": true,
"read_skill_file": true,
"ask_user_question": isRootChat,
} }
filtered := make([]database.MCPServerConfig, 0, len(configs))
approvedIDs := make(map[uuid.UUID]struct{})
for _, cfg := range configs {
if !cfg.AllowInPlanMode {
continue
}
filtered = append(filtered, cfg)
approvedIDs[cfg.ID] = struct{}{}
}
return filtered, approvedIDs
}
func builtinPlanToolAllowed(name string, isRootChat bool) bool {
switch name {
case "read_file", "execute", "process_output", "read_skill", "read_skill_file":
return true
case "write_file", "edit_files", "list_templates", "read_template",
"create_workspace", "start_workspace", "propose_plan", "spawn_agent",
"spawn_explore_agent", "wait_agent", "ask_user_question":
return isRootChat
case "process_list", "process_signal", "message_agent", "close_agent",
"spawn_computer_use_agent":
return false
default:
return false
}
}
func toolAllowedForTurn(
tool fantasy.AgentTool,
mode database.NullChatPlanMode,
parentChatID uuid.NullUUID,
approvedMCPConfigIDs map[uuid.UUID]struct{},
) bool {
if !mode.Valid || mode.ChatPlanMode != database.ChatPlanModePlan {
return true
}
if builtinPlanToolAllowed(tool.Info().Name, !parentChatID.Valid) {
return true
}
mcpTool, ok := tool.(mcpclient.MCPToolIdentifier)
if !ok {
return false
}
_, approved := approvedMCPConfigIDs[mcpTool.MCPServerConfigID()]
return approved
}
func filterToolsForTurn(
allTools []fantasy.AgentTool,
mode database.NullChatPlanMode,
parentChatID uuid.NullUUID,
approvedMCPConfigIDs map[uuid.UUID]struct{},
) []fantasy.AgentTool {
if !mode.Valid || mode.ChatPlanMode != database.ChatPlanModePlan {
return allTools
}
filtered := make([]fantasy.AgentTool, 0, len(allTools))
for _, tool := range allTools {
if toolAllowedForTurn(tool, mode, parentChatID, approvedMCPConfigIDs) {
filtered = append(filtered, tool)
}
}
return filtered
}
// activeToolNamesForTurn extends the built-in plan allowlist with approved
// external MCP tools for root plan-mode chats.
func activeToolNamesForTurn(
allTools []fantasy.AgentTool,
mode database.NullChatPlanMode,
parentChatID uuid.NullUUID,
approvedMCPConfigIDs map[uuid.UUID]struct{},
) []string {
toolNames := make([]string, 0, len(allTools)) toolNames := make([]string, 0, len(allTools))
for _, tool := range allTools { for _, tool := range allTools {
name := tool.Info().Name if toolAllowedForTurn(tool, mode, parentChatID, approvedMCPConfigIDs) {
if builtinPlanPolicy[name] { toolNames = append(toolNames, tool.Info().Name)
toolNames = append(toolNames, name)
} }
} }
return toolNames return toolNames
@@ -5222,30 +5281,24 @@ func allowedExploreToolNames(allTools []fantasy.AgentTool) []string {
return toolNames return toolNames
} }
// allowedBehaviorToolNames applies behavior-specific precedence for // allowedBehaviorToolNames runs only on non-plan turns because
// tool filtering: Explore mode wins over plan mode, and plan mode wins // appendDynamicTools returns early for plan mode. Within that boundary,
// over the default behavior that allows all tools. // Explore mode wins over the default behavior that allows all tools.
func allowedBehaviorToolNames( func allowedBehaviorToolNames(
allTools []fantasy.AgentTool, allTools []fantasy.AgentTool,
planMode database.NullChatPlanMode,
chatMode database.NullChatMode, chatMode database.NullChatMode,
parentChatID uuid.NullUUID,
) []string { ) []string {
if isExploreSubagentMode(chatMode) { if isExploreSubagentMode(chatMode) {
return allowedExploreToolNames(allTools) return allowedExploreToolNames(allTools)
} }
if planMode.Valid && planMode.ChatPlanMode == database.ChatPlanModePlan {
return allowedPlanToolNames(allTools, parentChatID)
}
return allToolNames(allTools) return allToolNames(allTools)
} }
func stopAfterBehaviorTools( func stopAfterPlanTools(
planMode database.NullChatPlanMode, planMode database.NullChatPlanMode,
chatMode database.NullChatMode,
parentChatID uuid.NullUUID, parentChatID uuid.NullUUID,
) map[string]struct{} { ) map[string]struct{} {
if isExploreSubagentMode(chatMode) || !planMode.Valid || planMode.ChatPlanMode != database.ChatPlanModePlan { if !planMode.Valid || planMode.ChatPlanMode != database.ChatPlanModePlan {
return nil return nil
} }
stopTools := map[string]struct{}{ stopTools := map[string]struct{}{
@@ -5257,6 +5310,17 @@ func stopAfterBehaviorTools(
return stopTools return stopTools
} }
func stopAfterBehaviorTools(
planMode database.NullChatPlanMode,
chatMode database.NullChatMode,
parentChatID uuid.NullUUID,
) map[string]struct{} {
if isExploreSubagentMode(chatMode) {
return nil
}
return stopAfterPlanTools(planMode, parentChatID)
}
type systemPromptBehaviorContext struct { type systemPromptBehaviorContext struct {
planMode database.NullChatPlanMode planMode database.NullChatPlanMode
chatMode database.NullChatMode chatMode database.NullChatMode
@@ -5432,7 +5496,6 @@ func appendDynamicTools(
raw pqtype.NullRawMessage, raw pqtype.NullRawMessage,
planMode database.NullChatPlanMode, planMode database.NullChatPlanMode,
chatMode database.NullChatMode, chatMode database.NullChatMode,
parentChatID uuid.NullUUID,
) ([]fantasy.AgentTool, map[string]bool, error) { ) ([]fantasy.AgentTool, map[string]bool, error) {
if isExploreSubagentMode(chatMode) || (planMode.Valid && planMode.ChatPlanMode == database.ChatPlanModePlan) { if isExploreSubagentMode(chatMode) || (planMode.Valid && planMode.ChatPlanMode == database.ChatPlanModePlan) {
return tools, nil, nil return tools, nil, nil
@@ -5454,7 +5517,7 @@ func appendDynamicTools(
} }
activeToolNames := make(map[string]struct{}, len(tools)) activeToolNames := make(map[string]struct{}, len(tools))
for _, name := range allowedBehaviorToolNames(tools, planMode, chatMode, parentChatID) { for _, name := range allowedBehaviorToolNames(tools, chatMode) {
activeToolNames[name] = struct{}{} activeToolNames[name] = struct{}{}
} }
for _, t := range tools { for _, t := range tools {
@@ -5569,6 +5632,12 @@ func (p *Server) runChat(
currentPlanMode := chat.PlanMode currentPlanMode := chat.PlanMode
isPlanModeTurn := currentPlanMode.Valid && currentPlanMode.ChatPlanMode == database.ChatPlanModePlan isPlanModeTurn := currentPlanMode.Valid && currentPlanMode.ChatPlanMode == database.ChatPlanModePlan
isExploreSubagent := isExploreSubagentMode(chat.Mode) isExploreSubagent := isExploreSubagentMode(chat.Mode)
isRootChat := !chat.ParentChatID.Valid
mcpConnectConfigs, approvedPlanMCPConfigIDs := filterExternalMCPConfigsForTurn(
mcpConfigs,
currentPlanMode,
chat.ParentChatID,
)
planModeInstructions := p.loadPlanModeInstructions(ctx, currentPlanMode, logger) planModeInstructions := p.loadPlanModeInstructions(ctx, currentPlanMode, logger)
chainInfo := resolveChainMode(messages) chainInfo := resolveChainMode(messages)
@@ -5767,17 +5836,20 @@ func (p *Server) runChat(
resolvedUserPrompt = p.resolveUserPrompt(ctx, chat.OwnerID) resolvedUserPrompt = p.resolveUserPrompt(ctx, chat.OwnerID)
return nil return nil
}) })
if len(mcpConfigs) > 0 { if len(mcpConnectConfigs) > 0 {
g2.Go(func() error { g2.Go(func() error {
// Refresh expired OAuth2 tokens before connecting. // Refresh expired OAuth2 tokens before connecting.
mcpTokens = p.refreshExpiredMCPTokens(ctx, logger, mcpConfigs, mcpTokens) mcpTokens = p.refreshExpiredMCPTokens(ctx, logger, mcpConnectConfigs, mcpTokens)
mcpTools, mcpCleanup = mcpclient.ConnectAll( mcpTools, mcpCleanup = mcpclient.ConnectAll(
ctx, logger, mcpConfigs, mcpTokens, ctx, logger, mcpConnectConfigs, mcpTokens,
) )
return nil return nil
}) })
} }
if chat.WorkspaceID.Valid { // Workspace MCP discovery stays disabled for all plan-mode turns.
// Root plan mode only gets approved external MCP servers, and
// plan-mode subagents get no MCP tools.
if chat.WorkspaceID.Valid && !isPlanModeTurn {
g2.Go(func() error { g2.Go(func() error {
// Fast path: check cache using the in-memory cached // Fast path: check cache using the in-memory cached
// agent (ensureWorkspaceAgent is free when already // agent (ensureWorkspaceAgent is free when already
@@ -5848,7 +5920,6 @@ func (p *Server) runChat(
if err := g2.Wait(); err != nil { if err := g2.Wait(); err != nil {
return result, err return result, err
} }
isRootChat := !chat.ParentChatID.Valid
subagentInstruction := "" subagentInstruction := ""
if !isRootChat { if !isRootChat {
subagentInstruction = defaultSubagentInstruction subagentInstruction = defaultSubagentInstruction
@@ -6280,13 +6351,22 @@ func (p *Server) runChat(
builtinToolNames[t.Info().Name] = true builtinToolNames[t.Info().Name] = true
} }
// Append tools from external MCP servers. These appear // Append external and workspace MCP tools after the built-ins so the
// after the built-in tools so the LLM sees them as // LLM sees them as additional capabilities. Explore subagents keep
// additional capabilities. // the narrower built-in-only boundary from main. Root plan mode gets
if !isPlanModeTurn && !isExploreSubagent { // only approved external MCP tools because mcpConnectConfigs was
// pre-filtered above, and filterToolsForTurn removes any remaining
// plan-mode ineligible tools from the assembled set.
if !isExploreSubagent {
tools = append(tools, mcpTools...) tools = append(tools, mcpTools...)
tools = append(tools, workspaceMCPTools...) tools = append(tools, workspaceMCPTools...)
} }
tools = filterToolsForTurn(
tools,
currentPlanMode,
chat.ParentChatID,
approvedPlanMCPConfigIDs,
)
// Append dynamic tools declared by the client at chat // Append dynamic tools declared by the client at chat
// creation time. These appear in the LLM's tool list but // creation time. These appear in the LLM's tool list but
// are never executed by the chatloop. The client handles // are never executed by the chatloop. The client handles
@@ -6299,7 +6379,6 @@ func (p *Server) runChat(
chat.DynamicTools, chat.DynamicTools,
currentPlanMode, currentPlanMode,
chat.Mode, chat.Mode,
chat.ParentChatID,
) )
if err != nil { if err != nil {
return result, err return result, err
@@ -6351,6 +6430,16 @@ func (p *Server) runChat(
) )
prompt = filterPromptForChainMode(prompt, chainInfo) prompt = filterPromptForChainMode(prompt, chainInfo)
} }
activeToolNames := activeToolNamesForTurn(
tools,
currentPlanMode,
chat.ParentChatID,
approvedPlanMCPConfigIDs,
)
if isExploreSubagent {
activeToolNames = allowedExploreToolNames(tools)
}
var loopErr error var loopErr error
triggerMessageID, historyTipMessageID, triggerLabel := deriveChatDebugSeed(messages) triggerMessageID, historyTipMessageID, triggerLabel := deriveChatDebugSeed(messages)
result.TriggerMessageID = triggerMessageID result.TriggerMessageID = triggerMessageID
@@ -6382,7 +6471,7 @@ func (p *Server) runChat(
Model: model, Model: model,
Messages: prompt, Messages: prompt,
Tools: tools, Tools: tools,
ActiveTools: allowedBehaviorToolNames(tools, currentPlanMode, chat.Mode, chat.ParentChatID), ActiveTools: activeToolNames,
StopAfterTools: stopAfterBehaviorTools(currentPlanMode, chat.Mode, chat.ParentChatID), StopAfterTools: stopAfterBehaviorTools(currentPlanMode, chat.Mode, chat.ParentChatID),
MaxSteps: maxChatSteps, MaxSteps: maxChatSteps,
Metrics: p.metrics, Metrics: p.metrics,
+188 -25
View File
@@ -59,7 +59,75 @@ func (t *testAgentTool) SetProviderOptions(opts fantasy.ProviderOptions) {
t.providerOptions = opts t.providerOptions = opts
} }
func TestAllowedPlanToolNames(t *testing.T) { type testMCPAgentTool struct {
*testAgentTool
configID uuid.UUID
}
func newTestMCPAgentTool(name string, configID uuid.UUID) fantasy.AgentTool {
return &testMCPAgentTool{
testAgentTool: &testAgentTool{info: fantasy.ToolInfo{Name: name}},
configID: configID,
}
}
func (t *testMCPAgentTool) MCPServerConfigID() uuid.UUID {
return t.configID
}
func TestFilterExternalMCPConfigsForTurn(t *testing.T) {
t.Parallel()
approvedConfig := database.MCPServerConfig{ID: uuid.New(), AllowInPlanMode: true}
blockedConfig := database.MCPServerConfig{ID: uuid.New(), AllowInPlanMode: false}
configs := []database.MCPServerConfig{approvedConfig, blockedConfig}
planMode := database.NullChatPlanMode{
ChatPlanMode: database.ChatPlanModePlan,
Valid: true,
}
t.Run("NonPlanModePassesThroughAllConfigs", func(t *testing.T) {
t.Parallel()
filtered, approvedIDs := filterExternalMCPConfigsForTurn(
configs,
database.NullChatPlanMode{},
uuid.NullUUID{},
)
require.Equal(t, configs, filtered)
require.Nil(t, approvedIDs)
})
t.Run("PlanModeSubagentsReturnNoConfigs", func(t *testing.T) {
t.Parallel()
filtered, approvedIDs := filterExternalMCPConfigsForTurn(
configs,
planMode,
uuid.NullUUID{UUID: uuid.New(), Valid: true},
)
require.Nil(t, filtered)
require.NotNil(t, approvedIDs)
require.Empty(t, approvedIDs)
})
t.Run("PlanModeRootFiltersToApprovedConfigs", func(t *testing.T) {
t.Parallel()
filtered, approvedIDs := filterExternalMCPConfigsForTurn(
configs,
planMode,
uuid.NullUUID{},
)
require.Equal(t, []database.MCPServerConfig{approvedConfig}, filtered)
require.Equal(t, map[uuid.UUID]struct{}{approvedConfig.ID: {}}, approvedIDs)
})
}
func TestActiveToolNamesForTurn(t *testing.T) {
t.Parallel() t.Parallel()
makeTools := func(names ...string) []fantasy.AgentTool { makeTools := func(names ...string) []fantasy.AgentTool {
@@ -70,10 +138,33 @@ func TestAllowedPlanToolNames(t *testing.T) {
return tools return tools
} }
t.Run("RootPlanModeIncludesOnlyAllowlistedBuiltIns", func(t *testing.T) { planMode := database.NullChatPlanMode{
ChatPlanMode: database.ChatPlanModePlan,
Valid: true,
}
t.Run("NormalModeReturnsAllRegisteredTools", func(t *testing.T) {
t.Parallel() t.Parallel()
got := allowedPlanToolNames(makeTools( got := activeToolNamesForTurn(makeTools(
"read_file",
"propose_plan",
"custom_tool",
"execute",
), database.NullChatPlanMode{}, uuid.NullUUID{}, nil)
require.Equal(t, []string{
"read_file",
"propose_plan",
"custom_tool",
"execute",
}, got)
})
t.Run("PlanModeIncludesOnlyAllowlistedBuiltIns", func(t *testing.T) {
t.Parallel()
got := activeToolNamesForTurn(makeTools(
"read_file", "read_file",
"write_file", "write_file",
"edit_files", "edit_files",
@@ -95,7 +186,7 @@ func TestAllowedPlanToolNames(t *testing.T) {
"read_skill", "read_skill",
"read_skill_file", "read_skill_file",
"ask_user_question", "ask_user_question",
), uuid.NullUUID{}) ), planMode, uuid.NullUUID{}, nil)
require.Equal(t, []string{ require.Equal(t, []string{
"read_file", "read_file",
@@ -117,10 +208,10 @@ func TestAllowedPlanToolNames(t *testing.T) {
}, got) }, got)
}) })
t.Run("ChildPlanModeAllowsExplorationOnly", func(t *testing.T) { t.Run("PlanModeChildChatsAllowExplorationOnly", func(t *testing.T) {
t.Parallel() t.Parallel()
got := allowedPlanToolNames(makeTools( got := activeToolNamesForTurn(makeTools(
"read_file", "read_file",
"write_file", "write_file",
"edit_files", "edit_files",
@@ -137,7 +228,7 @@ func TestAllowedPlanToolNames(t *testing.T) {
"read_skill", "read_skill",
"read_skill_file", "read_skill_file",
"ask_user_question", "ask_user_question",
), uuid.NullUUID{UUID: uuid.New(), Valid: true}) ), planMode, uuid.NullUUID{UUID: uuid.New(), Valid: true}, nil)
require.Equal(t, []string{ require.Equal(t, []string{
"read_file", "read_file",
@@ -146,6 +237,67 @@ func TestAllowedPlanToolNames(t *testing.T) {
"read_skill", "read_skill",
"read_skill_file", "read_skill_file",
}, got) }, got)
require.NotContains(t, got, "write_file")
require.NotContains(t, got, "edit_files")
require.NotContains(t, got, "ask_user_question")
require.NotContains(t, got, "propose_plan")
require.NotContains(t, got, "spawn_explore_agent")
})
t.Run("PlanModeStillExcludesDangerousTools", func(t *testing.T) {
t.Parallel()
got := activeToolNamesForTurn(makeTools(
"execute",
"process_output",
"message_agent",
"spawn_computer_use_agent",
"propose_plan",
), planMode, uuid.NullUUID{}, nil)
require.Equal(t, []string{"execute", "process_output", "propose_plan"}, got)
require.NotContains(t, got, "message_agent")
require.NotContains(t, got, "spawn_computer_use_agent")
})
t.Run("PlanModeExcludesUnknownTools", func(t *testing.T) {
t.Parallel()
got := activeToolNamesForTurn(makeTools(
"read_file",
"custom_tool",
"another_custom_tool",
"propose_plan",
), planMode, uuid.NullUUID{}, nil)
require.Equal(t, []string{
"read_file",
"propose_plan",
}, got)
require.NotContains(t, got, "custom_tool")
require.NotContains(t, got, "another_custom_tool")
})
t.Run("PlanModeIncludesOnlyApprovedExternalMCPTools", func(t *testing.T) {
t.Parallel()
approvedConfigID := uuid.New()
blockedConfigID := uuid.New()
got := activeToolNamesForTurn([]fantasy.AgentTool{
newTestAgentTool("read_file"),
newTestMCPAgentTool("approved-mcp__echo", approvedConfigID),
newTestMCPAgentTool("blocked-mcp__echo", blockedConfigID),
newTestAgentTool("workspace-mcp__echo"),
}, planMode, uuid.NullUUID{}, map[uuid.UUID]struct{}{
approvedConfigID: {},
})
require.Equal(t, []string{
"read_file",
"approved-mcp__echo",
}, got)
require.NotContains(t, got, "blocked-mcp__echo")
require.NotContains(t, got, "workspace-mcp__echo")
}) })
} }
@@ -197,10 +349,6 @@ func TestAllowedBehaviorToolNames(t *testing.T) {
} }
allTools := makeTools("read_file", "custom_tool", "spawn_explore_agent") allTools := makeTools("read_file", "custom_tool", "spawn_explore_agent")
planMode := database.NullChatPlanMode{
ChatPlanMode: database.ChatPlanModePlan,
Valid: true,
}
exploreMode := database.NullChatMode{ exploreMode := database.NullChatMode{
ChatMode: database.ChatModeExplore, ChatMode: database.ChatModeExplore,
Valid: true, Valid: true,
@@ -210,19 +358,7 @@ func TestAllowedBehaviorToolNames(t *testing.T) {
t.Parallel() t.Parallel()
require.Equal(t, []string{"read_file", "custom_tool", "spawn_explore_agent"}, allowedBehaviorToolNames( require.Equal(t, []string{"read_file", "custom_tool", "spawn_explore_agent"}, allowedBehaviorToolNames(
allTools, allTools,
database.NullChatPlanMode{},
database.NullChatMode{}, database.NullChatMode{},
uuid.NullUUID{},
))
})
t.Run("PlanModeUsesPlanAllowlist", func(t *testing.T) {
t.Parallel()
require.Equal(t, []string{"read_file", "spawn_explore_agent"}, allowedBehaviorToolNames(
allTools,
planMode,
database.NullChatMode{},
uuid.NullUUID{},
)) ))
}) })
@@ -230,13 +366,40 @@ func TestAllowedBehaviorToolNames(t *testing.T) {
t.Parallel() t.Parallel()
require.Equal(t, []string{"read_file"}, allowedBehaviorToolNames( require.Equal(t, []string{"read_file"}, allowedBehaviorToolNames(
allTools, allTools,
database.NullChatPlanMode{},
exploreMode, exploreMode,
uuid.NullUUID{UUID: uuid.New(), Valid: true},
)) ))
}) })
} }
func TestStopAfterPlanTools(t *testing.T) {
t.Parallel()
planMode := database.NullChatPlanMode{
ChatPlanMode: database.ChatPlanModePlan,
Valid: true,
}
t.Run("NormalModeReturnsNil", func(t *testing.T) {
t.Parallel()
require.Nil(t, stopAfterPlanTools(database.NullChatPlanMode{}, uuid.NullUUID{}))
})
t.Run("RootPlanModeIncludesClarificationTool", func(t *testing.T) {
t.Parallel()
require.Equal(t, map[string]struct{}{
"propose_plan": {},
"ask_user_question": {},
}, stopAfterPlanTools(planMode, uuid.NullUUID{}))
})
t.Run("ChildPlanModeSkipsClarificationTool", func(t *testing.T) {
t.Parallel()
require.Equal(t, map[string]struct{}{
"propose_plan": {},
}, stopAfterPlanTools(planMode, uuid.NullUUID{UUID: uuid.New(), Valid: true}))
})
}
func TestStopAfterBehaviorTools(t *testing.T) { func TestStopAfterBehaviorTools(t *testing.T) {
t.Parallel() t.Parallel()
+540 -1
View File
@@ -379,6 +379,37 @@ func TestPlanModeSubagentChatExcludesAskUserQuestion(t *testing.T) {
_ = agenttest.New(t, client.URL, agentToken) _ = agenttest.New(t, client.URL, agentToken)
// Start an external MCP server whose tools should remain available to the
// root plan-mode chat but stay hidden from plan-mode subagents.
mcpSrv := mcpserver.NewMCPServer("plan-root-mcp", "1.0.0")
mcpSrv.AddTools(mcpserver.ServerTool{
Tool: mcpgo.NewTool("echo",
mcpgo.WithDescription("Echoes the input"),
mcpgo.WithString("input",
mcpgo.Description("The input string"),
mcpgo.Required(),
),
),
Handler: func(_ context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
input, _ := req.GetArguments()["input"].(string)
return mcpgo.NewToolResultText("echo: " + input), nil
},
})
mcpTS := httptest.NewServer(mcpserver.NewStreamableHTTPServer(mcpSrv))
t.Cleanup(mcpTS.Close)
mcpConfig, err := client.CreateMCPServerConfig(ctx, codersdk.CreateMCPServerConfigRequest{
DisplayName: "Plan Root MCP",
Slug: "plan-root-mcp",
Transport: "streamable_http",
URL: mcpTS.URL,
AuthType: "none",
Availability: "default_off",
Enabled: true,
AllowInPlanMode: true,
})
require.NoError(t, err)
var toolsMu sync.Mutex var toolsMu sync.Mutex
toolsByCall := make([][]string, 0, 2) toolsByCall := make([][]string, 0, 2)
requestsByCall := make([]recordedOpenAIRequest, 0, 2) requestsByCall := make([]recordedOpenAIRequest, 0, 2)
@@ -408,7 +439,7 @@ func TestPlanModeSubagentChatExcludesAskUserQuestion(t *testing.T) {
) )
}) })
_, err := expClient.CreateChatProvider(ctx, codersdk.CreateChatProviderConfigRequest{ _, err = expClient.CreateChatProvider(ctx, codersdk.CreateChatProviderConfigRequest{
Provider: "openai-compat", Provider: "openai-compat",
APIKey: "test-api-key", APIKey: "test-api-key",
BaseURL: openAIURL, BaseURL: openAIURL,
@@ -428,6 +459,7 @@ func TestPlanModeSubagentChatExcludesAskUserQuestion(t *testing.T) {
chat, err := expClient.CreateChat(ctx, codersdk.CreateChatRequest{ chat, err := expClient.CreateChat(ctx, codersdk.CreateChatRequest{
OrganizationID: user.OrganizationID, OrganizationID: user.OrganizationID,
PlanMode: codersdk.ChatPlanModePlan, PlanMode: codersdk.ChatPlanModePlan,
MCPServerIDs: []uuid.UUID{mcpConfig.ID},
Content: []codersdk.ChatInputPart{ Content: []codersdk.ChatInputPart{
{ {
Type: codersdk.ChatInputPartTypeText, Type: codersdk.ChatInputPartTypeText,
@@ -486,6 +518,8 @@ func TestPlanModeSubagentChatExcludesAskUserQuestion(t *testing.T) {
"root plan-mode chat should have execute") "root plan-mode chat should have execute")
require.Contains(t, rootCalls[0], "process_output", require.Contains(t, rootCalls[0], "process_output",
"root plan-mode chat should have process_output") "root plan-mode chat should have process_output")
require.Contains(t, rootCalls[0], "plan-root-mcp__echo",
"root plan-mode chat should have approved external MCP tools")
require.NotContains(t, childCalls[0], "ask_user_question", require.NotContains(t, childCalls[0], "ask_user_question",
"plan-mode subagent should NOT have ask_user_question") "plan-mode subagent should NOT have ask_user_question")
require.NotContains(t, childCalls[0], "write_file", require.NotContains(t, childCalls[0], "write_file",
@@ -496,6 +530,8 @@ func TestPlanModeSubagentChatExcludesAskUserQuestion(t *testing.T) {
"plan-mode subagent should have execute") "plan-mode subagent should have execute")
require.Contains(t, childCalls[0], "process_output", require.Contains(t, childCalls[0], "process_output",
"plan-mode subagent should have process_output") "plan-mode subagent should have process_output")
require.NotContains(t, childCalls[0], "plan-root-mcp__echo",
"plan-mode subagent should NOT have external MCP tools")
require.True(t, requestHasSystemSubstring(rootRequests[0], "You are in Plan Mode.")) require.True(t, requestHasSystemSubstring(rootRequests[0], "You are in Plan Mode."))
require.True(t, requestHasSystemSubstring(childRequests[0], "You are in Plan Mode as a delegated sub-agent.")) require.True(t, requestHasSystemSubstring(childRequests[0], "You are in Plan Mode as a delegated sub-agent."))
require.False(t, requestHasSystemSubstring(childRequests[0], "When the plan is ready, call propose_plan")) require.False(t, requestHasSystemSubstring(childRequests[0], "When the plan is ready, call propose_plan"))
@@ -663,6 +699,227 @@ func TestExploreSubagentIsReadOnly(t *testing.T) {
require.Len(t, exploreChildren, 1) require.Len(t, exploreChildren, 1)
} }
func TestPlanModeRootChatAllowsApprovedExternalMCPTools(t *testing.T) {
t.Parallel()
db, ps := dbtestutil.NewDB(t)
ctx := testutil.Context(t, testutil.WaitLong)
echoMCP := mcpserver.NewMCPServer("plan-visibility-echo", "1.0.0")
echoMCP.AddTools(mcpserver.ServerTool{
Tool: mcpgo.NewTool("echo",
mcpgo.WithDescription("Echoes the input"),
mcpgo.WithString("input",
mcpgo.Description("The input string"),
mcpgo.Required(),
),
),
Handler: func(_ context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
input, _ := req.GetArguments()["input"].(string)
return mcpgo.NewToolResultText("echo: " + input), nil
},
})
echoTS := httptest.NewServer(mcpserver.NewStreamableHTTPServer(echoMCP))
t.Cleanup(echoTS.Close)
filteredMCP := mcpserver.NewMCPServer("plan-visibility-filtered", "1.0.0")
filteredMCP.AddTools(
mcpserver.ServerTool{
Tool: mcpgo.NewTool("visible",
mcpgo.WithDescription("Visible tool"),
mcpgo.WithString("input",
mcpgo.Description("The input string"),
mcpgo.Required(),
),
),
Handler: func(_ context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
input, _ := req.GetArguments()["input"].(string)
return mcpgo.NewToolResultText("visible: " + input), nil
},
},
mcpserver.ServerTool{
Tool: mcpgo.NewTool("hidden",
mcpgo.WithDescription("Hidden tool"),
mcpgo.WithString("input",
mcpgo.Description("The input string"),
mcpgo.Required(),
),
),
Handler: func(_ context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
input, _ := req.GetArguments()["input"].(string)
return mcpgo.NewToolResultText("hidden: " + input), nil
},
},
)
filteredTS := httptest.NewServer(mcpserver.NewStreamableHTTPServer(filteredMCP))
t.Cleanup(filteredTS.Close)
var (
requests []recordedOpenAIRequest
requestsMu sync.Mutex
)
openAIURL := chattest.NewOpenAI(t, func(req *chattest.OpenAIRequest) chattest.OpenAIResponse {
if !req.Stream {
return chattest.OpenAINonStreamingResponse("title")
}
requestsMu.Lock()
requests = append(requests, recordOpenAIRequest(req))
requestsMu.Unlock()
return chattest.OpenAIStreamingResponse(
chattest.OpenAITextChunks("Done.")...,
)
})
user, org, model := seedChatDependenciesWithProvider(ctx, t, db, "openai-compat", openAIURL)
approvedConfig, err := db.InsertMCPServerConfig(ctx, database.InsertMCPServerConfigParams{
DisplayName: "Plan Approved MCP",
Slug: "plan-approved-mcp",
Url: echoTS.URL,
Transport: "streamable_http",
AuthType: "none",
Availability: "default_off",
Enabled: true,
AllowInPlanMode: true,
ToolAllowList: []string{},
ToolDenyList: []string{},
CreatedBy: user.ID,
UpdatedBy: user.ID,
})
require.NoError(t, err)
blockedConfig, err := db.InsertMCPServerConfig(ctx, database.InsertMCPServerConfigParams{
DisplayName: "Plan Blocked MCP",
Slug: "plan-blocked-mcp",
Url: echoTS.URL,
Transport: "streamable_http",
AuthType: "none",
Availability: "default_off",
Enabled: true,
AllowInPlanMode: false,
ToolAllowList: []string{},
ToolDenyList: []string{},
CreatedBy: user.ID,
UpdatedBy: user.ID,
})
require.NoError(t, err)
filteredConfig, err := db.InsertMCPServerConfig(ctx, database.InsertMCPServerConfigParams{
DisplayName: "Plan Filtered MCP",
Slug: "plan-filtered-mcp",
Url: filteredTS.URL,
Transport: "streamable_http",
AuthType: "none",
Availability: "default_off",
Enabled: true,
AllowInPlanMode: true,
ToolAllowList: []string{"visible"},
ToolDenyList: []string{},
CreatedBy: user.ID,
UpdatedBy: user.ID,
})
require.NoError(t, err)
ws, dbAgent := seedWorkspaceWithAgent(t, db, user.ID)
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
mockConn.EXPECT().SetExtraHeaders(gomock.Any()).AnyTimes()
mockConn.EXPECT().ContextConfig(gomock.Any()).
Return(workspacesdk.ContextConfigResponse{}, xerrors.New("not supported")).AnyTimes()
workspaceToolName := "workspace-plan-mcp__echo"
mockConn.EXPECT().ListMCPTools(gomock.Any()).
Return(workspacesdk.ListMCPToolsResponse{Tools: []workspacesdk.MCPToolInfo{{
ServerName: "workspace-plan-mcp",
Name: workspaceToolName,
Description: "Workspace echo tool",
Schema: map[string]any{
"input": map[string]any{"type": "string"},
},
Required: []string{"input"},
}}}, nil).
Times(1)
mockConn.EXPECT().LS(gomock.Any(), gomock.Any(), gomock.Any()).
Return(workspacesdk.LSResponse{AbsolutePathString: "/home/coder"}, nil).AnyTimes()
mockConn.EXPECT().ReadFile(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
Return(io.NopCloser(strings.NewReader("")), "", nil).AnyTimes()
server := newActiveTestServer(t, db, ps, func(cfg *chatd.Config) {
cfg.AgentConn = func(_ context.Context, agentID uuid.UUID) (workspacesdk.AgentConn, func(), error) {
require.Equal(t, dbAgent.ID, agentID)
return mockConn, func() {}, nil
}
})
planChat, err := server.CreateChat(ctx, chatd.CreateOptions{
OrganizationID: org.ID,
OwnerID: user.ID,
Title: "plan-mode-root-mcp-visibility",
ModelConfigID: model.ID,
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
PlanMode: database.NullChatPlanMode{ChatPlanMode: database.ChatPlanModePlan, Valid: true},
MCPServerIDs: []uuid.UUID{approvedConfig.ID, blockedConfig.ID, filteredConfig.ID},
InitialUserContent: []codersdk.ChatMessagePart{
codersdk.ChatMessageText("List the available tools in plan mode."),
},
})
require.NoError(t, err)
waitForChatProcessed(ctx, t, db, planChat.ID, server)
planChatResult, err := db.GetChatByID(ctx, planChat.ID)
require.NoError(t, err)
require.Equal(t, database.ChatStatusWaiting, planChatResult.Status)
askChat, err := server.CreateChat(ctx, chatd.CreateOptions{
OrganizationID: org.ID,
OwnerID: user.ID,
Title: "ask-mode-root-mcp-visibility",
ModelConfigID: model.ID,
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
MCPServerIDs: []uuid.UUID{approvedConfig.ID, blockedConfig.ID, filteredConfig.ID},
InitialUserContent: []codersdk.ChatMessagePart{
codersdk.ChatMessageText("List the available tools outside plan mode."),
},
})
require.NoError(t, err)
waitForChatProcessed(ctx, t, db, askChat.ID, server)
askChatResult, err := db.GetChatByID(ctx, askChat.ID)
require.NoError(t, err)
require.Equal(t, database.ChatStatusWaiting, askChatResult.Status)
requestsMu.Lock()
recorded := append([]recordedOpenAIRequest(nil), requests...)
requestsMu.Unlock()
require.Len(t, recorded, 2, "expected exactly one streamed model call per chat")
planTools := recorded[0].Tools
askTools := recorded[1].Tools
require.Contains(t, planTools, "plan-approved-mcp__echo",
"root plan mode should expose approved external MCP tools")
require.NotContains(t, planTools, "plan-blocked-mcp__echo",
"root plan mode should hide unapproved external MCP tools")
require.Contains(t, planTools, "plan-filtered-mcp__visible",
"root plan mode should keep allowlisted tools from approved MCP servers")
require.NotContains(t, planTools, "plan-filtered-mcp__hidden",
"root plan mode should still respect MCP tool allowlists")
require.NotContains(t, planTools, workspaceToolName,
"root plan mode should exclude workspace MCP tools")
require.Contains(t, askTools, "plan-approved-mcp__echo",
"ask mode should keep approved external MCP tools")
require.Contains(t, askTools, "plan-blocked-mcp__echo",
"ask mode should keep unapproved-for-plan external MCP tools")
require.Contains(t, askTools, "plan-filtered-mcp__visible",
"ask mode should keep allowlisted tools from external MCP servers")
require.NotContains(t, askTools, "plan-filtered-mcp__hidden",
"ask mode should continue respecting MCP tool allowlists")
require.Contains(t, askTools, workspaceToolName,
"ask mode should continue exposing workspace MCP tools")
}
func TestInterruptChatClearsWorkerInDatabase(t *testing.T) { func TestInterruptChatClearsWorkerInDatabase(t *testing.T) {
t.Parallel() t.Parallel()
@@ -1151,6 +1408,8 @@ func TestPlanTurnPromptContract(t *testing.T) {
require.True(t, requestHasSystemSubstring(recorded[0], "You are in Plan Mode.")) require.True(t, requestHasSystemSubstring(recorded[0], "You are in Plan Mode."))
require.True(t, requestHasSystemSubstring(recorded[0], "The only intentional authored workspace artifact is the plan file")) require.True(t, requestHasSystemSubstring(recorded[0], "The only intentional authored workspace artifact is the plan file"))
require.True(t, requestHasSystemSubstring(recorded[0], "You may use execute and process_output for exploration")) require.True(t, requestHasSystemSubstring(recorded[0], "You may use execute and process_output for exploration"))
require.True(t, requestHasSystemSubstring(recorded[0], "approved external MCP tools when available"))
require.True(t, requestHasSystemSubstring(recorded[0], "Workspace MCP tools are not available in root plan mode"))
require.True(t, requestHasSystemSubstring(recorded[0], "After a successful propose_plan call, stop immediately")) require.True(t, requestHasSystemSubstring(recorded[0], "After a successful propose_plan call, stop immediately"))
require.True(t, requestHasSystemSubstring(recorded[0], planModeInstructions)) require.True(t, requestHasSystemSubstring(recorded[0], planModeInstructions))
for _, msg := range recorded[0].Messages { for _, msg := range recorded[0].Messages {
@@ -5971,6 +6230,286 @@ func TestMCPServerToolInvocation(t *testing.T) {
"MCP tool result should be persisted as a tool message in the database") "MCP tool result should be persisted as a tool message in the database")
} }
func TestPlanModeRootChatApprovedExternalMCPToolInvocation(t *testing.T) {
t.Parallel()
db, ps := dbtestutil.NewDB(t)
ctx := testutil.Context(t, testutil.WaitLong)
mcpSrv := mcpserver.NewMCPServer("plan-mode-mcp", "1.0.0")
mcpSrv.AddTools(mcpserver.ServerTool{
Tool: mcpgo.NewTool("echo",
mcpgo.WithDescription("Echoes the input"),
mcpgo.WithString("input",
mcpgo.Description("The input string"),
mcpgo.Required(),
),
),
Handler: func(_ context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
input, _ := req.GetArguments()["input"].(string)
return mcpgo.NewToolResultText("echo: " + input), nil
},
})
mcpTS := httptest.NewServer(mcpserver.NewStreamableHTTPServer(mcpSrv))
t.Cleanup(mcpTS.Close)
var (
callCount atomic.Int32
llmToolNames []string
llmToolsMu sync.Mutex
foundMCPResult atomic.Bool
)
openAIURL := chattest.NewOpenAI(t, func(req *chattest.OpenAIRequest) chattest.OpenAIResponse {
if !req.Stream {
return chattest.OpenAINonStreamingResponse("title")
}
if callCount.Add(1) == 1 {
names := make([]string, 0, len(req.Tools))
for _, tool := range req.Tools {
names = append(names, tool.Function.Name)
}
llmToolsMu.Lock()
llmToolNames = names
llmToolsMu.Unlock()
return chattest.OpenAIStreamingResponse(
chattest.OpenAIToolCallChunk(
"plan-mode-mcp__echo",
`{"input":"hello from root plan mode"}`,
),
)
}
for _, msg := range req.Messages {
if msg.Role == "tool" && strings.Contains(msg.Content, "echo: hello from root plan mode") {
foundMCPResult.Store(true)
}
}
return chattest.OpenAIStreamingResponse(
chattest.OpenAITextChunks("Planning complete.")...,
)
})
user, org, model := seedChatDependenciesWithProvider(ctx, t, db, "openai-compat", openAIURL)
mcpConfig, err := db.InsertMCPServerConfig(ctx, database.InsertMCPServerConfigParams{
DisplayName: "Plan Mode MCP",
Slug: "plan-mode-mcp",
Url: mcpTS.URL,
Transport: "streamable_http",
AuthType: "none",
Availability: "default_off",
Enabled: true,
AllowInPlanMode: true,
ToolAllowList: []string{},
ToolDenyList: []string{},
CreatedBy: user.ID,
UpdatedBy: user.ID,
})
require.NoError(t, err)
server := newActiveTestServer(t, db, ps)
chat, err := server.CreateChat(ctx, chatd.CreateOptions{
OrganizationID: org.ID,
OwnerID: user.ID,
Title: "plan-mode-mcp-invocation",
ModelConfigID: model.ID,
PlanMode: database.NullChatPlanMode{ChatPlanMode: database.ChatPlanModePlan, Valid: true},
MCPServerIDs: []uuid.UUID{mcpConfig.ID},
InitialUserContent: []codersdk.ChatMessagePart{
codersdk.ChatMessageText("Use the approved MCP tool while planning."),
},
})
require.NoError(t, err)
waitForChatProcessed(ctx, t, db, chat.ID, server)
chatResult, err := db.GetChatByID(ctx, chat.ID)
require.NoError(t, err)
require.Equal(t, database.ChatStatusWaiting, chatResult.Status)
llmToolsMu.Lock()
recordedNames := append([]string(nil), llmToolNames...)
llmToolsMu.Unlock()
require.Contains(t, recordedNames, "plan-mode-mcp__echo",
"approved external MCP tools should be available in root plan mode")
require.True(t, foundMCPResult.Load(),
"approved external MCP tool results should feed back into the follow-up plan-mode turn")
}
func TestPlanModeRootChatApprovedExternalMCPWorkflowCanReachProposePlan(t *testing.T) {
t.Parallel()
db, ps := dbtestutil.NewDB(t)
ctx := testutil.Context(t, testutil.WaitLong)
mcpSrv := mcpserver.NewMCPServer("plan-workflow-mcp", "1.0.0")
mcpSrv.AddTools(mcpserver.ServerTool{
Tool: mcpgo.NewTool("echo",
mcpgo.WithDescription("Echoes the input"),
mcpgo.WithString("input",
mcpgo.Description("The input string"),
mcpgo.Required(),
),
),
Handler: func(_ context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
input, _ := req.GetArguments()["input"].(string)
return mcpgo.NewToolResultText("echo: " + input), nil
},
})
mcpTS := httptest.NewServer(mcpserver.NewStreamableHTTPServer(mcpSrv))
t.Cleanup(mcpTS.Close)
var (
callCount atomic.Int32
llmToolNames []string
llmToolsMu sync.Mutex
sawMCPResult atomic.Bool
proposePlanReached atomic.Bool
)
openAIURL := chattest.NewOpenAI(t, func(req *chattest.OpenAIRequest) chattest.OpenAIResponse {
if !req.Stream {
return chattest.OpenAINonStreamingResponse("title")
}
switch callCount.Add(1) {
case 1:
names := make([]string, 0, len(req.Tools))
for _, tool := range req.Tools {
names = append(names, tool.Function.Name)
}
llmToolsMu.Lock()
llmToolNames = names
llmToolsMu.Unlock()
return chattest.OpenAIStreamingResponse(
chattest.OpenAIToolCallChunk(
"plan-workflow-mcp__echo",
`{"input":"prepare the plan"}`,
),
)
case 2:
for _, msg := range req.Messages {
if msg.Role == "tool" && strings.Contains(msg.Content, "echo: prepare the plan") {
sawMCPResult.Store(true)
}
}
proposePlanReached.Store(true)
return chattest.OpenAIStreamingResponse(
chattest.OpenAIToolCallChunk("propose_plan", `{}`),
)
default:
return chattest.OpenAIStreamingResponse(
chattest.OpenAITextChunks("should not continue")...,
)
}
})
user, org, model := seedChatDependenciesWithProvider(ctx, t, db, "openai-compat", openAIURL)
mcpConfig, err := db.InsertMCPServerConfig(ctx, database.InsertMCPServerConfigParams{
DisplayName: "Plan Workflow MCP",
Slug: "plan-workflow-mcp",
Url: mcpTS.URL,
Transport: "streamable_http",
AuthType: "none",
Availability: "default_off",
Enabled: true,
AllowInPlanMode: true,
ToolAllowList: []string{},
ToolDenyList: []string{},
CreatedBy: user.ID,
UpdatedBy: user.ID,
})
require.NoError(t, err)
ws, dbAgent := seedWorkspaceWithAgent(t, db, user.ID)
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
mockConn.EXPECT().SetExtraHeaders(gomock.Any()).AnyTimes()
mockConn.EXPECT().ContextConfig(gomock.Any()).
Return(workspacesdk.ContextConfigResponse{}, xerrors.New("not supported")).AnyTimes()
mockConn.EXPECT().LS(gomock.Any(), gomock.Any(), gomock.Any()).
Return(workspacesdk.LSResponse{AbsolutePathString: "/home/coder"}, nil).AnyTimes()
mockConn.EXPECT().ReadFile(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
DoAndReturn(func(_ context.Context, path string, _, _ int64) (io.ReadCloser, string, error) {
if strings.HasSuffix(path, ".md") {
return io.NopCloser(strings.NewReader("# Plan\n- Use the approved MCP tool findings.\n")), "", nil
}
return io.NopCloser(strings.NewReader("")), "", nil
}).AnyTimes()
server := newActiveTestServer(t, db, ps, func(cfg *chatd.Config) {
cfg.AgentConn = func(_ context.Context, agentID uuid.UUID) (workspacesdk.AgentConn, func(), error) {
require.Equal(t, dbAgent.ID, agentID)
return mockConn, func() {}, nil
}
})
chat, err := server.CreateChat(ctx, chatd.CreateOptions{
OrganizationID: org.ID,
OwnerID: user.ID,
Title: "plan-mode-mcp-propose-plan",
ModelConfigID: model.ID,
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
PlanMode: database.NullChatPlanMode{ChatPlanMode: database.ChatPlanModePlan, Valid: true},
MCPServerIDs: []uuid.UUID{mcpConfig.ID},
InitialUserContent: []codersdk.ChatMessagePart{
codersdk.ChatMessageText("Use the approved MCP tool, then propose the plan."),
},
})
require.NoError(t, err)
waitForChatProcessed(ctx, t, db, chat.ID, server)
chatResult, err := db.GetChatByID(ctx, chat.ID)
require.NoError(t, err)
require.Equal(t, database.ChatStatusWaiting, chatResult.Status)
llmToolsMu.Lock()
recordedNames := append([]string(nil), llmToolNames...)
llmToolsMu.Unlock()
require.Contains(t, recordedNames, "plan-workflow-mcp__echo",
"approved external MCP tools should be available in the root plan-mode workflow")
require.True(t, sawMCPResult.Load(),
"the root plan-mode workflow should feed the approved MCP result into the propose_plan turn")
require.True(t, proposePlanReached.Load(),
"the root plan-mode workflow should reach propose_plan after using the approved MCP tool")
require.Equal(t, int32(2), callCount.Load(),
"the workflow should stop immediately after propose_plan succeeds")
var foundProposePlanResult bool
testutil.Eventually(ctx, t, func(ctx context.Context) bool {
messages, dbErr := db.GetChatMessagesByChatID(ctx, database.GetChatMessagesByChatIDParams{
ChatID: chat.ID,
AfterID: 0,
})
if dbErr != nil {
return false
}
for _, msg := range messages {
if msg.Role != database.ChatMessageRoleTool {
continue
}
parts, parseErr := chatprompt.ParseContent(msg)
if parseErr != nil {
continue
}
for _, part := range parts {
if part.Type == codersdk.ChatMessagePartTypeToolResult && part.ToolName == "propose_plan" {
foundProposePlanResult = true
return true
}
}
}
return false
}, testutil.IntervalFast)
require.True(t, foundProposePlanResult,
"the root plan-mode workflow should persist a propose_plan tool result")
}
// TestMCPServerOAuth2TokenRefresh verifies that when a chat uses an // TestMCPServerOAuth2TokenRefresh verifies that when a chat uses an
// MCP server with OAuth2 auth and the stored access token is expired, // MCP server with OAuth2 auth and the stored access token is expired,
// chatd refreshes the token using the stored refresh_token before // chatd refreshes the token using the stored refresh_token before
+5 -1
View File
@@ -110,13 +110,17 @@ You may use execute and process_output for exploration, including cloning reposi
Do not use Plan Mode to implement the requested changes or intentionally modify project files outside the plan file. Do not use Plan Mode to implement the requested changes or intentionally modify project files outside the plan file.
If no workspace is attached to this chat yet, create and start one with create_workspace and start_workspace before investigating. If no workspace is attached to this chat yet, create and start one with create_workspace and start_workspace before investigating.
If the plan file already exists, read it first with read_file before replacing or refining it. If the plan file already exists, read it first with read_file before replacing or refining it.
Use read_file, execute, process_output, list_templates, read_template, and spawn_agent to gather context. In Plan Mode, spawn_agent delegation is for investigation and planning support, not code writing or implementation. Use read_file, execute, process_output, list_templates, read_template, spawn_agent, and approved external MCP tools when available to gather context. Workspace MCP tools are not available in root plan mode, and side-effecting built-in tools such as process_list, process_signal, message_agent, close_agent, and spawn_computer_use_agent remain unavailable. In Plan Mode, spawn_agent delegation is for investigation and planning support, not code writing or implementation.
Use write_file to create the plan file and edit_files to refine it. Use write_file to create the plan file and edit_files to refine it.
Use ask_user_question for structured clarification instead of freeform questions. Use ask_user_question for structured clarification instead of freeform questions.
When the plan is ready, call propose_plan with the plan file path. When the plan is ready, call propose_plan with the plan file path.
After a successful propose_plan call, stop immediately. Do not produce follow-up output. After a successful propose_plan call, stop immediately. Do not produce follow-up output.
` + defaultSystemPromptPlanPathBlockPlaceholder ` + defaultSystemPromptPlanPathBlockPlaceholder
// Root plan mode may use approved external MCP tools, but delegated
// plan-mode subagents stay on the narrower built-in-only boundary
// because their trust boundary is narrower than the root chat's.
// PlanningSubagentOverlayPrompt contains plan-mode instructions for // PlanningSubagentOverlayPrompt contains plan-mode instructions for
// delegated child chats. Child chats may investigate with shell tools // delegated child chats. Child chats may investigate with shell tools
// but should return findings to the parent instead of authoring the // but should return findings to the parent instead of authoring the
+13 -10
View File
@@ -64,10 +64,11 @@ type MCPServerConfig struct {
// Availability policy set by admin. // Availability policy set by admin.
Availability string `json:"availability"` // "force_on", "default_on", "default_off" Availability string `json:"availability"` // "force_on", "default_on", "default_off"
Enabled bool `json:"enabled"` Enabled bool `json:"enabled"`
ModelIntent bool `json:"model_intent"` ModelIntent bool `json:"model_intent"`
CreatedAt time.Time `json:"created_at" format:"date-time"` AllowInPlanMode bool `json:"allow_in_plan_mode"`
UpdatedAt time.Time `json:"updated_at" format:"date-time"` CreatedAt time.Time `json:"created_at" format:"date-time"`
UpdatedAt time.Time `json:"updated_at" format:"date-time"`
// Per-user state (populated for non-admin requests). // Per-user state (populated for non-admin requests).
AuthConnected bool `json:"auth_connected"` AuthConnected bool `json:"auth_connected"`
@@ -96,9 +97,10 @@ type CreateMCPServerConfigRequest struct {
ToolAllowList []string `json:"tool_allow_list,omitempty"` ToolAllowList []string `json:"tool_allow_list,omitempty"`
ToolDenyList []string `json:"tool_deny_list,omitempty"` ToolDenyList []string `json:"tool_deny_list,omitempty"`
Availability string `json:"availability" validate:"required,oneof=force_on default_on default_off"` Availability string `json:"availability" validate:"required,oneof=force_on default_on default_off"`
Enabled bool `json:"enabled"` Enabled bool `json:"enabled"`
ModelIntent bool `json:"model_intent"` ModelIntent bool `json:"model_intent"`
AllowInPlanMode bool `json:"allow_in_plan_mode"`
} }
// UpdateMCPServerConfigRequest is the request to update an MCP server config. // UpdateMCPServerConfigRequest is the request to update an MCP server config.
@@ -124,9 +126,10 @@ type UpdateMCPServerConfigRequest struct {
ToolAllowList *[]string `json:"tool_allow_list,omitempty"` ToolAllowList *[]string `json:"tool_allow_list,omitempty"`
ToolDenyList *[]string `json:"tool_deny_list,omitempty"` ToolDenyList *[]string `json:"tool_deny_list,omitempty"`
Availability *string `json:"availability,omitempty" validate:"omitempty,oneof=force_on default_on default_off"` Availability *string `json:"availability,omitempty" validate:"omitempty,oneof=force_on default_on default_off"`
Enabled *bool `json:"enabled,omitempty"` Enabled *bool `json:"enabled,omitempty"`
ModelIntent *bool `json:"model_intent,omitempty"` ModelIntent *bool `json:"model_intent,omitempty"`
AllowInPlanMode *bool `json:"allow_in_plan_mode,omitempty"`
} }
func (c *Client) MCPServerConfigs(ctx context.Context) ([]MCPServerConfig, error) { func (c *Client) MCPServerConfigs(ctx context.Context) ([]MCPServerConfig, error) {
+4 -2
View File
@@ -162,8 +162,10 @@ and cannot create workspaces or spawn further sub-agents.
active. In that mode, `write_file` and `edit_files` are restricted to the active. In that mode, `write_file` and `edit_files` are restricted to the
chat-specific plan file, while `execute` and `process_output` remain available chat-specific plan file, while `execute` and `process_output` remain available
for exploration such as cloning repositories, searching code, and running for exploration such as cloning repositories, searching code, and running
inspection commands. MCP, dynamic, provider-native, and computer-use tools are inspection commands. Root plan-mode chats may also receive administrator-approved
not available. external MCP tools. Workspace MCP tools remain unavailable in plan mode, and
plan-mode sub-agents still do not receive any MCP tools. Dynamic,
provider-native, and computer-use tools are not available.
### Orchestration tools ### Orchestration tools
+3
View File
@@ -2790,6 +2790,7 @@ export interface CreateMCPServerConfigRequest {
readonly availability: string; readonly availability: string;
readonly enabled: boolean; readonly enabled: boolean;
readonly model_intent: boolean; readonly model_intent: boolean;
readonly allow_in_plan_mode: boolean;
} }
// From codersdk/organizations.go // From codersdk/organizations.go
@@ -4461,6 +4462,7 @@ export interface MCPServerConfig {
readonly availability: string; // "force_on", "default_on", "default_off" readonly availability: string; // "force_on", "default_on", "default_off"
readonly enabled: boolean; readonly enabled: boolean;
readonly model_intent: boolean; readonly model_intent: boolean;
readonly allow_in_plan_mode: boolean;
readonly created_at: string; readonly created_at: string;
readonly updated_at: string; readonly updated_at: string;
/** /**
@@ -7796,6 +7798,7 @@ export interface UpdateMCPServerConfigRequest {
readonly availability?: string; readonly availability?: string;
readonly enabled?: boolean; readonly enabled?: boolean;
readonly model_intent?: boolean; readonly model_intent?: boolean;
readonly allow_in_plan_mode?: boolean;
} }
// From codersdk/notifications.go // From codersdk/notifications.go
@@ -528,6 +528,7 @@ const makeMCPServer = (
availability: overrides.availability ?? "default_on", availability: overrides.availability ?? "default_on",
enabled: overrides.enabled ?? true, enabled: overrides.enabled ?? true,
model_intent: overrides.model_intent ?? false, model_intent: overrides.model_intent ?? false,
allow_in_plan_mode: overrides.allow_in_plan_mode ?? false,
created_at: overrides.created_at ?? now, created_at: overrides.created_at ?? now,
updated_at: overrides.updated_at ?? now, updated_at: overrides.updated_at ?? now,
auth_connected: overrides.auth_connected ?? false, auth_connected: overrides.auth_connected ?? false,
@@ -517,6 +517,7 @@ const sampleMCPServers = [
availability: "default_on", availability: "default_on",
enabled: true, enabled: true,
model_intent: false, model_intent: false,
allow_in_plan_mode: false,
auth_connected: true, auth_connected: true,
created_at: "2025-01-01T00:00:00Z", created_at: "2025-01-01T00:00:00Z",
updated_at: "2025-01-01T00:00:00Z", updated_at: "2025-01-01T00:00:00Z",
@@ -33,6 +33,7 @@ const createServerConfig = (
availability: overrides.availability ?? "default_on", availability: overrides.availability ?? "default_on",
enabled: overrides.enabled ?? true, enabled: overrides.enabled ?? true,
model_intent: overrides.model_intent ?? false, model_intent: overrides.model_intent ?? false,
allow_in_plan_mode: overrides.allow_in_plan_mode ?? false,
created_at: overrides.created_at ?? now, created_at: overrides.created_at ?? now,
updated_at: overrides.updated_at ?? now, updated_at: overrides.updated_at ?? now,
auth_connected: overrides.auth_connected ?? false, auth_connected: overrides.auth_connected ?? false,
@@ -228,6 +228,7 @@ interface MCPServerFormValues {
availability: string; availability: string;
enabled: boolean; enabled: boolean;
modelIntent: boolean; modelIntent: boolean;
allowInPlanMode: boolean;
toolAllowList: string; toolAllowList: string;
toolDenyList: string; toolDenyList: string;
customHeaders: Array<{ key: string; value: string }>; customHeaders: Array<{ key: string; value: string }>;
@@ -257,6 +258,7 @@ const buildInitialValues = (
availability: server?.availability ?? "default_off", availability: server?.availability ?? "default_off",
enabled: server?.enabled ?? true, enabled: server?.enabled ?? true,
modelIntent: server?.model_intent ?? false, modelIntent: server?.model_intent ?? false,
allowInPlanMode: server?.allow_in_plan_mode ?? false,
toolAllowList: joinList(server?.tool_allow_list), toolAllowList: joinList(server?.tool_allow_list),
toolDenyList: joinList(server?.tool_deny_list), toolDenyList: joinList(server?.tool_deny_list),
customHeaders: [], customHeaders: [],
@@ -311,6 +313,7 @@ const ServerForm: FC<ServerFormProps> = ({
availability: values.availability, availability: values.availability,
enabled: values.enabled, enabled: values.enabled,
model_intent: values.modelIntent, model_intent: values.modelIntent,
allow_in_plan_mode: values.allowInPlanMode,
...(values.authType === "oauth2" && { ...(values.authType === "oauth2" && {
oauth2_client_id: values.oauth2ClientID.trim(), oauth2_client_id: values.oauth2ClientID.trim(),
oauth2_client_secret: effectiveOAuth2Secret, oauth2_client_secret: effectiveOAuth2Secret,
@@ -778,6 +781,26 @@ const ServerForm: FC<ServerFormProps> = ({
disabled={isDisabled} disabled={isDisabled}
/> />
</div> </div>
<div className="flex items-center justify-between">
<div>
<Label htmlFor={`${formId}-allow-in-plan-mode`}>
Allow all tools from this MCP server in root plan mode
</Label>
<p className="text-sm text-content-secondary">
When enabled, the root plan-mode agent can call these tools
during planning. Workspace MCP and plan-mode subagents remain
restricted.
</p>
</div>
<Switch
id={`${formId}-allow-in-plan-mode`}
checked={form.values.allowInPlanMode}
onCheckedChange={(v) => {
form.setFieldValue("allowInPlanMode", v);
}}
disabled={isDisabled}
/>
</div>
<div className="grid items-start gap-5 sm:grid-cols-2"> <div className="grid items-start gap-5 sm:grid-cols-2">
{" "} {" "}
<Field <Field
@@ -32,6 +32,7 @@ const createServerConfig = (
availability: overrides.availability ?? "default_on", availability: overrides.availability ?? "default_on",
enabled: overrides.enabled ?? true, enabled: overrides.enabled ?? true,
model_intent: overrides.model_intent ?? false, model_intent: overrides.model_intent ?? false,
allow_in_plan_mode: overrides.allow_in_plan_mode ?? false,
created_at: overrides.created_at ?? now, created_at: overrides.created_at ?? now,
updated_at: overrides.updated_at ?? now, updated_at: overrides.updated_at ?? now,
auth_connected: overrides.auth_connected ?? false, auth_connected: overrides.auth_connected ?? false,