fix: restart ql-mcp server on vscode workspace folder changes

[data-douser]

Problem

When workspace folders were added or removed, fireDidChange() notified VS Code that the MCP server definitions changed, causing it to stop the running server. However, because the returned McpStdioServerDefinition had the same version as before, VS Code did not restart the ql-mcp server -- which was left in an inconsistent, partially running state.

Summary of Changes

This pull request improves how the extension handles workspace folder changes and MCP server restarts in VS Code. The main changes ensure that the environment cache is correctly invalidated when workspace folders change, and that MCP server restarts are triggered only when necessary (such as on configuration changes, not folder changes). It also introduces versioning logic for MCP server definitions to ensure VS Code can reliably determine when to restart the server. Unit and integration tests are added to verify this behavior.

Outline of Changes

Workspace folder change handling:

• The extension now only invalidates its cached environment (via envBuilder.invalidate()) when workspace folders change, relying on VS Code to manage MCP server restarts, rather than triggering a restart or sending unnecessary notifications.
• Added a new integration test suite (workspace-folder-change.integration.test.ts) to verify that the environment cache is properly rebuilt when workspace folders are added or removed, and that original folders persist after workspace mutations. [1] [2]
• Expanded the multi-root workspace fixture to include additional folders (folder-a through folder-d) for more robust testing. The fixture is now limited to repo-relative folders only; workspace-mutation tests add/remove temp folders at runtime via updateWorkspaceFolders.

MCP server restart mechanism:

• Introduced a requestRestart() method in McpProvider that increments a revision counter and appends it to the server version string, ensuring VS Code detects a version change and restarts the server when needed (e.g., on configuration changes). [1] [2] [3]
• Updated the extension activation logic to use requestRestart() on configuration changes, and clarified the distinction between soft and hard server update signals.

Testing improvements:

• Added unit tests to verify the new versioning and restart logic in McpProvider, including scenarios for pinned and unpinned versions, soft and hard signals, and event firing. [1] [2] [3]
• Updated mocks and test setup to support the new methods and behaviors. [1] [2] [3]

Fixture cleanup:

• Removed a machine-specific absolute temp folder entry (/private/var/.../T/ql-mcp-seq2-DMXgqX) from test.code-workspace that made the fixture non-reproducible across machines and CI environments.

These changes make the extension's behavior more predictable and robust in multi-root workspaces and dynamic workspace scenarios.

---

📝 Update Information

Primitive Details

• Type: Both
• Name: McpProvider / VS Code extension workspace folder handling
• Update Category: Bug Fix

⚠️ CRITICAL: PR SCOPE VALIDATION

This PR is for updating an existing MCP server primitive and must ONLY include these file types:

✅ ALLOWED FILES:

• Server implementation files (server/src/**/*.ts)
• Updated primitive implementations
• Modified registration files (server/src/tools/*.ts)
• Updated or new test files (server/test/**/*.ts)
• Documentation updates (README.md, server docs)
• Updated type definitions (server/src/types/*.ts)
• Modified supporting library files (server/src/lib/*.ts)
• Configuration updates if needed (package.json, tsconfig.json)

🚫 FORBIDDEN FILES:

• Files unrelated to the primitive update
• Temporary or test output files
• IDE configuration files
• Log files or debug output
• Analysis or summary files

Rationale: This PR should contain only the files necessary to update and test the primitive.

🚨 PRs that include forbidden files will be rejected and must be revised.

---

🛑 MANDATORY PR VALIDATION CHECKLIST

BEFORE SUBMITTING THIS PR, CONFIRM:

• ONLY server implementation files are included
• NO temporary or output files are included
• NO unrelated configuration files are included
• ALL existing tests continue to pass
• NEW functionality is properly tested

---

• Impact Scope: Moderate

Update Metadata

• Breaking Changes: No
• API Compatibility: Maintained
• Performance Impact: Neutral

🎯 Changes Description

Current Behavior

When workspace folders were added or removed, fireDidChange() notified VS Code that the MCP server definitions changed, causing it to stop the running server. Because the returned McpStdioServerDefinition had the same version as before, VS Code did not restart the ql-mcp server, leaving it in an inconsistent, partially running state. Additionally, the multi-root workspace test fixture contained a machine-specific absolute temp folder path that caused non-reproducible test failures on other machines and CI.

Updated Behavior

The extension now correctly invalidates its environment cache on workspace folder changes and uses a monotonically increasing revision counter (via requestRestart()) to ensure VS Code reliably detects version changes and restarts the server when needed. The test fixture is cleaned up to contain only repo-relative folders; runtime workspace mutations add/remove temp folders dynamically via updateWorkspaceFolders.

Motivation

The previous implementation left the ql-mcp server in a broken state after workspace folder changes, and the committed fixture was non-reproducible across different development machines and CI environments.

🔄 Before vs. After Comparison

Functionality Changes

// BEFORE: Workspace folder changes triggered fireDidChange() with same version,
// leaving server stopped but not restarted
vscode.workspace.onDidChangeWorkspaceFolders(() => {
  this._onDidChangeDefinitions.fire();
});

// AFTER: Workspace folder changes only invalidate env cache; requestRestart()
// bumps the revision/version so VS Code restarts the server on config changes
vscode.workspace.onDidChangeWorkspaceFolders(() => {
  envBuilder.invalidate();
});

// requestRestart() increments _revision and fires the change event
requestRestart(): void {
  this._revision++;
  this._onDidChangeDefinitions.fire();
}

API Changes

// New method added to McpProvider
requestRestart(): void;

Output Format Changes

// BEFORE: Version string was static (e.g., "1.2.3")
{ "version": "1.2.3" }

// AFTER: Version string includes monotonically increasing revision suffix
{ "version": "1.2.3+r1" }

🧪 Testing & Validation

Test Coverage Updates

• Existing Tests: All existing tests continue to pass
• New Test Cases: Added tests for version bumping, revision suffixing, and workspace folder change handling
• Regression Tests: Added tests to prevent regression of the server-not-restarting issue
• Edge Case Tests: Tests for pinned/unpinned versions, soft/hard signals, and event firing

Validation Scenarios

1. Backward Compatibility: Existing workspace configurations continue to work without breaking changes
2. New Functionality: requestRestart() correctly increments revision and fires change event; version string changes on each call
3. Error Handling: Cleanup failures in integration tests are caught and do not fail tests
4. Performance: No performance regressions; version bumping is a simple integer increment

Test Results

• Unit Tests: All pass
• Integration Tests: All pass
• Manual Testing: Validated with real workspace folder add/remove scenarios
• Performance Testing: No regressions detected

📋 Implementation Details

Files Modified

• Core Implementation: extensions/vscode/src/server/mcp-provider.ts
• Supporting Libraries: extensions/vscode/src/extension.ts
• Tests: extensions/vscode/test/server/mcp-provider.test.ts, extensions/vscode/test/suite/workspace-folder-change.integration.test.ts
• Fixtures: extensions/vscode/test/fixtures/multi-root-workspace/test.code-workspace (removed machine-specific temp folder entry)

Code Changes Summary

• Algorithm Improvements: Revision-based versioning logic in McpProvider
• Error Handling: Integration test cleanup wrapped in try/catch; deprecated fs.rmdirSync replaced with fs.rmSync
• Type Safety: New requestRestart() method properly typed
• Input Validation: Path delimiter splitting for CODEQL_MCP_WORKSPACE_FOLDERS assertions

Dependencies

• No New Dependencies: Update uses existing dependencies only

🔍 Quality Improvements

Bug Fixes (if applicable)

• Issue: ql-mcp server stopped but not restarted after workspace folder changes; test fixture contained machine-specific path
• Root Cause: fireDidChange() was called with identical version string; fixture was committed with absolute temp path from developer machine
• Solution: Introduced requestRestart() with revision counter; removed machine-specific entry from fixture
• Prevention: Unit tests verify version changes on each requestRestart() call; fixture is now limited to repo-relative folders

Code Quality Enhancements

• Readability: Clearer separation between soft (env invalidation) and hard (server restart) update signals
• Maintainability: Centralized restart logic in requestRestart()
• Testability: New integration tests cover workspace folder lifecycle end-to-end
• Reusability: requestRestart() can be called from any handler that needs to force a server restart

🔗 References

Related Issues/PRs

External References

Validation Materials

• Test Cases: mcp-provider.test.ts (unit), workspace-folder-change.integration.test.ts (integration)

🚀 Compatibility & Migration

Backward Compatibility

• Fully Compatible: No breaking changes

API Evolution

• Enhanced Parameters: requestRestart() method added to McpProvider
• Improved Responses: Server version now includes revision suffix for reliable change detection
• Maintained Contracts: Core API contracts preserved

👥 Review Guidelines

For Reviewers

Please verify:

• ⚠️ SCOPE COMPLIANCE: PR contains only extension and test implementation files
• ⚠️ NO UNRELATED FILES: Machine-specific fixture entry removed; no temp or output files
• ⚠️ BACKWARD COMPATIBILITY: Existing functionality preserved
• Functionality: Workspace folder changes correctly invalidate env cache; config changes trigger server restart via requestRestart()
• Test Coverage: All existing tests pass; new unit and integration tests added
• Code Quality: Maintains or improves code quality

Testing Instructions

# Full test suite
npm install
npm run build
npm test

# Extension-host integration tests
cd extensions/vscode
npm test

# Code quality checks
npm run lint
npm run format
npm run type-check

Validation Checklist

1. Regression Testing: Verify no existing functionality is broken
2. New Feature Testing: Validate requestRestart() increments revision and changes version string
3. Error Testing: Test workspace folder add/remove scenarios
4. Integration Testing: Verify environment cache is rebuilt after workspace mutations
5. Fixture Testing: Confirm test fixture loads correctly on all platforms without machine-specific paths

📊 Impact Assessment

Server Impact

• Startup Time: No significant impact on server startup
• Runtime Stability: Improved — server is no longer left in a broken state after workspace folder changes
• Resource Usage: Reasonable resource consumption
• Concurrent Usage: Safe for concurrent access

AI Assistant Impact

• Improved Reliability: More reliable MCP server lifecycle in dynamic workspace scenarios
• Enhanced User Experience: Server correctly restarts when workspace configuration changes

🔄 Deployment Strategy

Rollout Considerations

• Safe Deployment: Can be deployed safely to production
• Monitoring: Standard VS Code extension telemetry applies

---

Update Methodology: This update follows best practices:

1. ✅ Comprehensive backward compatibility analysis
2. ✅ Thorough testing of all changes
3. ✅ Performance impact assessment
4. ✅ Clear documentation of changes
5. ✅ Robust error handling improvements
6. ✅ Maintained code quality standards

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 28f1776.

Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

[Copilot]

Pull request overview

This PR fixes a VS Code MCP lifecycle issue where workspace folder changes would stop (but not restart) the ql-mcp server by ensuring the provided server definition’s version changes on each definition refresh.

Changes:

• Add a monotonically increasing revision counter to McpProvider and append it to the effective server definition version after fireDidChange().
• Add unit tests validating version bumping behavior across fireDidChange() calls.
• Add an extension-host integration test suite covering workspace folder add/remove scenarios and env refresh.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
extensions/vscode/src/server/mcp-provider.ts	Adds _revision and computes an “effective” version that changes after fireDidChange() to force VS Code to restart the server.
extensions/vscode/test/server/mcp-provider.test.ts	Adds unit tests verifying version changes and revision suffixing for pinned/latest cases.
extensions/vscode/test/suite/workspace-folder-change.integration.test.ts	Adds integration coverage for workspace-folder-driven definition changes and environment refresh behavior.

Comments suppressed due to low confidence (2)

extensions/vscode/test/suite/workspace-folder-change.integration.test.ts:201

• fs.rmdirSync is deprecated in newer Node versions. Prefer fs.rmSync(tempDir, { recursive: true, force: true }) here as well (and wrap in try/catch if you want best-effort cleanup).

    if (!added) {
      fs.rmdirSync(tempDir);
      this.skip();

extensions/vscode/test/suite/workspace-folder-change.integration.test.ts:132

• The comment says Mocha's timeout is 60s, but the test runner sets Mocha's timeout to 30s. Either update this comment or set a longer timeout for this test to avoid confusion / unexpected timeouts on slow CI.

    // Listen for the onDidChangeMcpServerDefinitions event.
    // Mocha's test timeout (60 s) handles the failure case.
    const changePromise = new Promise<void>((resolve) => {

[Copilot]

Pull request overview

Copilot reviewed 7 out of 9 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 9 changed files in this pull request and generated 3 comments.

[Copilot]

This test overwrites vscode.workspace.onDidChangeWorkspaceFolders by assignment and never restores it. If Vitest runs test files in parallel within the same worker, this mutation can leak into other tests and cause order-dependent failures. Prefer vi.spyOn(vscode.workspace, 'onDidChangeWorkspaceFolders') (or save/restore the original) and restore the spy at the end of the test.

[Copilot]

requestRestart() is documented as restarting the server “with a fresh environment”, but it only bumps the revision and fires the change event. Because EnvironmentBuilder caches results, a caller that forgets to call envBuilder.invalidate() could trigger a restart while still reusing a stale cached env. Either invalidate the env cache inside requestRestart() or adjust the docstring to state that callers must invalidate the environment separately.

[data-douser]

@copilot apply changes based on the comments in this thread