Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions docs/architecture/remove-mcp-permission-system/plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# Plan

## Current Owners

- MCP runtime permission checks: `src/main/presenter/mcpPresenter/toolManager.ts`
- MCP server config defaults and normalization: `src/main/presenter/configPresenter/mcpConfHelper.ts`
- Deep link and marketplace MCP import defaults: `src/main/presenter/deeplinkPresenter/index.ts`,
`src/main/presenter/mcpPresenter/mcprouterManager.ts`,
`src/main/presenter/llmProviderPresenter/modelScopeMcp.ts`
- Plugin MCP manifest mapping: `src/main/presenter/pluginPresenter/index.ts`
- MCP server form UI: `src/renderer/src/components/mcp-config/McpServerForm.vue`
- Public MCP config type: `src/shared/types/presenters/core.presenter.d.ts`
- Existing tests: MCP form, config import, tool manager, plugin presenter, deeplink presenter, sync import.

## Target Behavior

MCP should execute tools when the agent/session layer has allowed the tool call to proceed. MCP
should still return normal tool errors for transport/server/tool failures, but it should not create
permission request blocks.

```text
Before
agent/session permission -> MCP autoApprove/session cache/plugin policy -> MCP tool call

After
agent/session permission -> MCP tool call
```

Agent-scoped MCP server/plugin selection stays outside this removal:

```text
agent selected servers/plugins -> tool list filtering -> agent/session permission -> MCP tool call
```

## Implementation Steps

1. Runtime removal
- Remove `checkToolPermission`, `determinePermissionType`, MCP session permission cache, and
`updateServerPermissions` from `ToolManager`.
- Remove MCP-generated `requiresPermission` / `permissionRequest` responses.
- Preserve server/tool availability checks and normal error handling.

2. Config migration and normalization
- Strip `autoApprove` from persisted MCP server configs when reading or migrating settings.
- Ensure built-in MCP defaults no longer include `autoApprove`.
- Ensure imported/synced/deeplink/marketplace/plugin MCP configs drop `autoApprove`.
- Treat unknown legacy `autoApprove` values as ignored until the field is fully removed from
shared types.

3. UI removal
- Remove auto-approve controls and state from `McpServerForm.vue`.
- Remove `settings.mcp.serverForm.autoApprove*` and `mcp.server.autoApprove*` i18n keys after no
code references them.
- Update tests that currently assert editable auto-approve controls.

4. Type cleanup
- Remove `autoApprove` from `MCPServerConfig` after all producers are updated.
- If needed, introduce a private legacy input type for import/migration code only.
- Keep route contracts structurally compatible by parsing legacy payloads and normalizing them
before persistence.

5. Test strategy
- Tool manager: MCP tool calls no longer produce permission requests from `autoApprove`.
- Config presenter: persisted legacy `autoApprove` is stripped on read/write migration.
- MCP form: no auto-approve controls render or submit.
- Import/deeplink/plugin/marketplace sync: incoming `autoApprove` is ignored.
- Agent/session permission tests stay in the agent runtime suites, not MCP suites.

## Migration Notes

Prefer normalizing at the config boundary so old configs cannot leak back into renderer state:

```text
read stored mcpServers
-> normalize server config
-> delete autoApprove
-> persist normalized config when settings are next saved or during explicit migration
```

This keeps runtime code simple and prevents UI/API clients from seeing obsolete permission data.

## Risks

- Some tests or fixtures assume `autoApprove: []` is required. Those should be updated to omit the
field.
- Plugin manifests may still carry `autoApprove`; import code should ignore it instead of rejecting
older manifests.
- Removing MCP permission requests must not remove agent/session permission prompts.
69 changes: 69 additions & 0 deletions docs/architecture/remove-mcp-permission-system/spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# Remove MCP Permission System

## User Need

DeepChat should have one permission owner for tool execution: the agent/session permission system.
MCP should provide transports, tool discovery, tool execution, authentication, and server selection,
but it should not maintain a second permission layer.

Today MCP stores and checks per-server `autoApprove` permissions. That creates duplicated policy:

- agent/session permission mode decides whether a tool action needs review;
- MCP `autoApprove` can independently bypass or request permission;
- historical MCP permission settings survive upgrades and keep affecting behavior.

## Goal

Remove MCP-specific permission handling so MCP does no extra approval, denial, or auto-approval
processing. After upgrade, historical MCP permission settings are cleared or ignored.

## Acceptance Criteria

- MCP tool execution no longer checks `MCPServerConfig.autoApprove`.
- MCP no longer persists new per-server approval settings.
- Existing persisted MCP permission settings are removed during migration or normalized away on read.
- MCP add/edit/import/sync paths do not reintroduce `autoApprove`.
- MCP UI no longer displays per-server auto-approve controls.
- MCP permission request/session-cache code paths are removed from the MCP presenter.
- Agent/session permission mode remains the only tool-execution permission gate.
- MCP OAuth authentication remains separate and unchanged; authentication is not a permission gate.
- Agent-scoped MCP server/plugin selection remains unchanged; selection is not a permission system.

## Non-Goals

- Do not remove agent/session permission modes such as `default`, `full_access`, or `auto_approve`.
- Do not change ACP `session/request_permission` handling.
- Do not change plugin installation trust or plugin ownership metadata.
- Do not change MCP OAuth credential storage or authentication prompts.
- Do not change server enablement, agent server selection, or plugin server selection.

## Compatibility

Historical config may contain:

```json
{
"mcpServers": {
"linear": {
"autoApprove": ["all"]
}
}
}
```

After migration/read normalization, DeepChat should behave as if the field does not exist:

```json
{
"mcpServers": {
"linear": {}
}
}
```

The implementation may keep a temporary optional type field only as a read-compatibility shim, but
runtime logic must not use it.

## Open Questions

None.
12 changes: 12 additions & 0 deletions docs/architecture/remove-mcp-permission-system/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Tasks

- [ ] Remove MCP runtime permission checks from `ToolManager`.
- [ ] Remove MCP session permission cache/update paths.
- [ ] Strip `autoApprove` from built-in/default MCP configs.
- [ ] Normalize persisted MCP server configs to remove historical `autoApprove`.
- [ ] Drop `autoApprove` from deeplink, marketplace, ModelScope, sync import, and plugin MCP mapping.
- [ ] Remove MCP server form auto-approve controls and related local state.
- [ ] Remove unused MCP auto-approve i18n keys after code references are gone.
- [ ] Remove `autoApprove` from shared MCP config types or confine it to legacy input normalization.
- [ ] Update tests and fixtures that still include `autoApprove`.
- [ ] Validate with format, i18n, lint, typecheck, and focused MCP/tool permission tests.
Loading