Repository navigation
Conversation
- Reclassify bundled filesystem and git MCP servers as write tier, off by default, consent revision 2 - Add a shared fail-closed resolveMcpEnablement used by both new-session settings and the Settings UI - Treat legacy enabled:true grants for revised entries as off at read time without rewriting config.json - Derive the required revision and the stored grant from one catalog rule shared by the resolver and the writer - Add a main-process single-server consent update that builds the grant from the catalog, never from the renderer - Reject mcpDefaults in generic writeConfig patches so stale renderer snapshots cannot restore revoked grants - Read consent with strict UTF-8 JSON parsing and refuse writes over corrupt or unreadable config files - Keep lenient decoding for ordinary config reads so a stray non-UTF-8 byte cannot reset every setting - Accept a UTF-8 BOM in both read paths - Type hive consent input as stored records so passing the whole config fails at compile time, still validated at run time - Log and notify once when consent state is unreadable at spawn instead of silently dropping default MCP servers - Read MCP settings only from main and drop the unused config prop from the settings panel - Refresh the MCP settings panel quietly on config broadcasts, keeping save errors and avoiding loading flashes - Derive tier type, resolver validation and Settings grouping from a single MCP_TIERS list - Update tier notes and add load, unknown and refresh strings in en, zh-CN and ar - Add regression tests for catalog invariants, resolver edge cases, revision rules, atomic writes, I/O failures, UI state and hire consent
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Part of #686.
The bundled
filesystemandgitMCP servers were taggedsafe-readonlyand turned on by default. In practice, they can write, edit, move, commit, reset, and switch branches in the agent workspace.This PR moves both servers into the existing
writetier, disables them by default, and requires consent revision 2. A legacyenabled: trueno longer counts as valid consent for these servers. Users must explicitly opt in again before either server is included in a new Claude session.The fix covers the full consent path:
One resolver.
resolveMcpEnablement()is a pure, fail-closed function shared bybuildDefaultMcpServers()and the Settings panel. The UI and new-session runtime use the same effective consent rules. Forfilesystemandgit, legacy, malformed, stale, future, or non-integer consent revisions cannot authorize the server.One writer. A new main-process IPC,
config:setMcpServerEnabled, updates one server against the latest configuration on disk. The consent revision comes from the catalog in main, never from the renderer. GenericwriteConfig()patches containingmcpDefaultsare rejected, preventing stale configuration snapshots from restoring revoked grants.Strict reads where it matters. Consent reads require complete, valid UTF-8 JSON. A truncated or corrupt configuration blocks consent writes and is preserved byte-for-byte. Ordinary configuration reads retain their existing lenient decoding, so an unrelated non-UTF-8 byte does not cause all settings to be discarded on the next save. Both paths accept a UTF-8 BOM.
No silent failures. If consent state cannot be read during spawn, the agent starts without bundled default MCP servers. The failure is logged and surfaced through a recovery toast. The Settings panel displays an error or unknown state and never reports an unconfirmed save as successful.
Easier to extend. The tier type, resolver validation, and Settings grouping derive from the shared
MCP_TIERSlist. Future permission changes can invalidate older grants by incrementing the server's catalogconsentRevision.One small change outside the MCP files.
SettingsModal.tsxnow renders<McpDefaultsSettings />without theconfigprop. The panel reads consent directly from main, so the startup snapshot was unused. Removing it also prevents future code from accidentally restoring stale-state behavior.Existing grants for other servers remain compatible. For example,
github-tokencontinues accepting legacy revision 1 consent, while existing safe-readonly defaults remain unchanged.Out of scope still open in #686: version pinning, MCP child-process sandboxing, per-agent directory scoping, egress allowlists, loopback blocking, and revocation of servers in already-running sessions.
Type of change
Evidence
The primary evidence is the same regression suite running against the baseline and the fix, demonstrating the change from failing consent contracts to passing behavior.
Before
Result: 42 tests 18 passed, 24 failed.
The baseline does not satisfy the new consent contracts. Among the failures, a legacy
enabled: truecan still includemunder-filesystemin the generated Claude sessionsettings.jsonwithout renewed consent.After
The same command, on the same Windows machine, against this PR branch:
Result: 42 tests 42 passed, 0 failed.
Legacy write-capable grants now resolve to OFF until renewed. New Claude session settings, Settings UI state, persisted consent, and Hire validation follow the updated contracts.
Focused regression evidence
The four targeted suites cover catalog classification, strict configuration persistence, renderer behavior, and Hire compatibility.
Consent catalog and runtime 6 passed
test/mcp-consent.test.cjsValidates tier classification, default-off behavior, revision enforcement, backward compatibility for unrelated servers, immutable catalog lookups, and generated Claude session settings.
Strict configuration and atomic writes 10 passed
test/mcp-consent-config.test.cjsCovers generic-patch bypass prevention, single-server read-modify-write, legacy configuration preservation, malformed JSON, invalid UTF-8, UTF-8 BO
Screenshot 3 Config regression tests
Settings UI and IPC 7 passed
test/mcp-consent-ui.test.cjsCovers effective consent rendering, explicit per-server updates, cross-window revocation, stale asynchronous reads, duplicate clicks, IPC rejection, failed writes, and visible error recovery.
Screenshot 4 UI and IPC regression tests
Hire validator compatibility 19 passed
test/hire-validator-parity.test.cjsConfirms write-capable
filesystemandgitrequests require consent without automatically granting it. Existing manifest validation, Gallery validator parity, provider allowlists, and JSON Schema contracts remain covered.Screenshot 5 Hire consent and validator compatibility
Focused totals: 6 + 10 + 7 + 19 = 42 passed, 0 failed.
These individual results are a breakdown of the same 42-test After suite, not additional independent tests.
How I tested it
Environment
Validation
main, with the new tests copied in, then reran the same files against this branch.config-write-notify,auto-mode-sandbox,provider-config, andagent-provider. Across all eight files: 61 passed, 0 failed.npm run typecheck— PASS, including Node and Web typechecks.npm run build— PASS.git diff --check— PASS.npm run test:focused— 986 tests: 947 passed, 25 failed, 14 skipped.The full focused suite is not completely green. Every failing test is in a file that also fails on a clean
mainbaseline on this Windows environment.The baseline/environment failures include:
fs-path-containment,worktree-deps,hooks-socket.agent-token-cap,arabic-terminal,hire-import,release-notes,telemetry-message-count.quit-sweep.electron,restart-terminal-preserve,update-download-asset.No consent or configuration regression test failed on the PR branch.
Verification boundaries
This PR verifies configuration semantics and generated session settings, not the behavior of live MCP child processes.
The UI regression tests exercise actual component logic using mocked IPC without a DOM. Main-process IPC wiring is checked through a source contract test.
The new Arabic localization strings would benefit from native-speaker review.
Checklist
npm run typecheckpasses.npm run test:focusedpasses. (25 failures, all in files that also fail on cleanmainin this Windows environment; details above.)npm run buildsucceeds.DESIGN.md/tokens.ts, with no ad-hoc colors, spacing, or fonts. The error text reuses the existing consent color, and the refresh button uses the default button style.ATTRIBUTION.md.