fix(settings): buffer Save-managed settings in cachedState until Save

[JunyongParkDev]

Related GitHub Issue

Closes: #862

Description

Several controls inside SettingsView wrote their changes to the extension host immediately via updateSettings, instead of buffering them in the local cachedState and waiting for the user to click Save. Because Discard reverts edits by resetting cachedState back to the persisted state (setCachedState(extensionState)), any control that had already posted updateSettings poisoned that source of truth — so Discard could no longer undo the change, silently breaking the Save/Discard contract.

This PR routes all five Save-managed values exclusively through cachedState / setCachedStateField while SettingsView is open:

• allowedCommands / deniedCommands (AutoApproveSettings) — drop the immediate updateSettings on command add/remove.
• profileThresholds (ContextManagementSettings) — drop the immediate updateSettings on the non-default profile branch; remove the now-unused vscode import.
• includeTaskHistoryInEnhance (PromptsSettings) — drop the immediate updateSettings on toggle.
• mcpEnabled (McpEnabledToggle / McpView) — adopt the existing "props first, fall back to context" pattern so SettingsView can pass the buffered value and a cachedState-backed setter. Because mcpEnabled is also read by McpView to gate the server list, the prop is threaded through McpView down to the toggle.

Design choices / trade-offs:

• The extension host side (webviewMessageHandler.ts) is correct and left untouched — persisting on receipt of updateSettings is expected. The fix belongs on the webview side: stop sending it before Save.
• McpEnabledToggle and McpView stay backward compatible: when used uncontrolled (no props), they fall back to live extension state and keep the original immediate-persist behavior. Only the SettingsView (controlled) path buffers.
• Genuinely-immediate actions are intentionally left unchanged because they are not part of the Save payload: autoApprovalEnabled and the per-server MCP actions (toggleMcpServer, deleteMcpServer, restartMcpServer, updateMcpTimeout).

Test Procedure

Automated (webview-ui component/unit layer — the lowest layer that would have failed):

cd webview-ui
npx vitest run \
  src/components/settings/__tests__/AutoApproveSettings.spec.tsx \
  src/components/settings/__tests__/ContextManagementSettings.spec.tsx \
  src/components/settings/__tests__/PromptsSettings.spec.tsx \
  src/components/mcp/__tests__/McpEnabledToggle.spec.tsx \
  src/components/settings/__tests__/SettingsView.spec.tsx

Added / updated tests:

• AutoApproveSettings — adding/removing allowed & denied commands buffers via setCachedStateField and posts no updateSettings.
• ContextManagementSettings — changing a non-default profile threshold buffers and posts no updateSettings.
• PromptsSettings — toggling include-task-history calls the setter and posts no updateSettings.
• McpEnabledToggle — controlled usage buffers via the setter prop and posts nothing; uncontrolled usage keeps the original immediate behavior.
• SettingsView — existing allowed-command tests updated to assert no pre-Save persistence, plus a new Discard regression test.

Manual (reproduces the original bug; using Allowed Commands as the example):

1. Open Settings → Auto Approve → enable Execute.
2. Type npm test in the allowed-commands input and click Add.
3. Do not click Save. Click Done and choose Discard changes.
4. Reopen Settings → Auto Approve → Execute.
5. Before: npm test is still there. After this PR: it is gone.

The same flow reproduces for denied commands, non-default profile thresholds,
Prompts → Enhance → include task history, and MCP → Enable MCP.

Pre-Submission Checklist

• Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
• Scope: My changes are focused on the linked issue (one major feature/fix per PR).
• Self-Review: I have performed a thorough self-review of my code.
• Testing: New and/or updated tests have been added to cover my changes (if applicable).
• Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
• Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Documentation Updates

Additional Notes

The behavior now matches the repository's Settings View Pattern: Save-managed values bind to local cachedState, Save posts them once via handleSubmit, and Discard fully reverts. No changes to the extension host or message protocol.

Get in Touch

Summary by CodeRabbit

• New Features
  • Enhanced MCP settings to support buffered (unsaved) edits with explicit Save/Discard behavior.
• Bug Fixes
  • Prevented multiple settings changes from persisting immediately; command allow/deny edits, non-default context threshold edits, and prompt task-history changes now update only locally until Save.
• Tests
  • Added/updated test coverage to verify Save/Discard buffering for MCP, command approvals, context thresholds, and prompt preferences, including discard scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fda7f2ab-6b56-4b66-87c0-4788b6598c73

📥 Commits

Reviewing files that changed from the base of the PR and between 4223f1c and c6dacd4.

📒 Files selected for processing (2)

• webview-ui/src/components/mcp/__tests__/McpView.spec.tsx
• webview-ui/src/components/settings/__tests__/SettingsView.unsaved-changes.spec.tsx

---

📝 Walkthrough

Walkthrough

Settings-managed controls now update cached state without immediate persistence. MCP enablement supports controlled state in SettingsView and retains context-based behavior elsewhere. Tests cover buffering, controlled/uncontrolled MCP behavior, and discard handling.

Changes

Settings buffering and MCP integration

Layer / File(s)	Summary
Controlled MCP enable flow webview-ui/src/components/mcp/*, webview-ui/src/components/settings/SettingsView.tsx	MCP components accept optional state props, allowing SettingsView to buffer mcpEnabled through cached state while preserving uncontrolled context updates.
Buffered settings handlers webview-ui/src/components/settings/AutoApproveSettings.tsx, ContextManagementSettings.tsx, PromptsSettings.tsx, webview-ui/src/components/settings/__tests__/*	Command lists, profile thresholds, and task-history settings now update cached state without immediate updateSettings messages, with component tests covering each path.
SettingsView discard coverage webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx	Allowed-command tests verify that edits are not persisted before Save or after discarding unsaved changes.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested reviewers: taltas, JamesRobert20, navedmerchant, hannesrudolph, edelauna

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: buffering settings in cached state until Save.
Description check	✅ Passed	The PR description follows the template and includes the linked issue, implementation details, tests, checklist, and notes.
Linked Issues check	✅ Passed	The changes match issue #862 by buffering all five Save-managed values and adding tests for Save/Discard behavior.
Out of Scope Changes check	✅ Passed	The changes stay focused on the Save/Discard buffering fix and supporting tests, with no clear unrelated additions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!