fix: prevent MCP server restart when toggling tool permissions

[roomote]

Description

This PR fixes an issue where toggling the "Always allow" checkbox for an MCP tool in Chatview causes the connected MCP server to restart unnecessarily, disrupting the chat flow and potentially interrupting in-flight operations.

Problem

When users toggle the "Always allow" checkbox for MCP tools, the server configuration file is updated, which triggers file watchers that restart the entire server connection. This is disruptive and unnecessary for such minor configuration changes.

Solution

The fix introduces a mechanism to distinguish between programmatic configuration updates (like toggling tool permissions) and external file changes that require a server restart:

1. Added an isProgrammaticUpdate flag to track when config changes are made programmatically
2. Modified the file watcher's debounce handler to skip processing during programmatic updates
3. Updated updateServerToolList and updateServerConfig methods to set this flag when writing config changes
4. Added logic to identify "minor" updates (alwaysAllow, disabledTools, timeout) that don't require server restarts

Changes

• src/services/mcp/McpHub.ts:
  • Added isProgrammaticUpdate flag to prevent file watcher triggers
  • Modified debounceConfigChange() to check the flag
  • Updated updateServerToolList() to set flag during writes
  • Enhanced updateServerConfig() to handle minor updates without restarts

Testing

• ✅ All existing tests pass (42 tests in McpHub.spec.ts)
• ✅ Linting passes
• ✅ Type checking passes

Impact

This change improves the user experience by:

• Preventing unnecessary server disconnections when toggling tool permissions
• Maintaining active MCP sessions during configuration updates
• Reducing delays and interruptions in the chat workflow

Fixes #8231

---

Important

Fixes unnecessary MCP server restarts by introducing a flag to track programmatic updates in McpHub.ts.

• Behavior:
  • Introduces isProgrammaticUpdate flag in McpHub.ts to prevent unnecessary server restarts when toggling tool permissions.
  • Modifies debounceConfigChange() to skip processing during programmatic updates.
  • Updates updateServerToolList() and updateServerConfig() to set the flag for minor updates (e.g., alwaysAllow, disabledTools, timeout).
• Testing:
  • All existing tests pass in McpHub.spec.ts.
  • Linting and type checking pass.

^{This description was created by for ca27bbb. You can customize this summary. It will automatically update as commits are pushed.}

[roomote]

Self-review complete. I promise I didn't just approve my own homework without reading it.

[roomote]

Potential race condition: There's a risk that file system watcher events could fire between setting/unsetting the isProgrammaticUpdate flag and the actual file operations. Consider using a more robust synchronization mechanism, such as:

• A promise-based queue for file operations
• A unique operation ID system to track programmatic updates
• Or increasing the debounce timeout in debounceConfigChange to be longer than the 100ms wait

[roomote]

Hardcoded timeout concern: The 100ms timeout might not be sufficient on slower systems or under heavy I/O load. Consider:

• Making this configurable via settings
• Using file system events to confirm write completion
• Or implementing exponential backoff if issues persist

[roomote]

Code duplication: This pattern of setting flag → writing → waiting → resetting flag is duplicated in updateServerToolList. Consider extracting into a helper method:

private async writeConfigWithoutTriggeringWatcher(
  configPath: string,
  config: any
): Promise<void> {
  this.isProgrammaticUpdate = true;
  try {
    await fs.writeFile(configPath, JSON.stringify(config, null, 2));
    await new Promise((resolve) => setTimeout(resolve, 100));
  } finally {
    this.isProgrammaticUpdate = false;
  }
}

[roomote]

Missing test coverage: Consider adding specific test cases for the isProgrammaticUpdate flag behavior to ensure file watchers are properly skipped during programmatic updates. This would help catch any regression in this critical functionality.