🐛 test(mcp): fix McpHub Windows command wrapping test ordering (#632)

* 🐛 test(mcp): fix McpHub Windows command wrapping test ordering

Fix mock initialization race: fs.readFile mock must be set before
new McpHub() triggers async initializeGlobalMcpServers().

Also fix duplicate vi.mock('fs/promises') without factory causing
auto-mock on line 96.

Closes #349

* 🐛 fix(test): add cleanup hook to prevent RPC teardown errors on Windows (#632)

* 🐛 fix(ci): cache VS Code test binary to avoid ECONNRESET download failures

* 🐛 fix(e2e): update glm-5-turbo max_tokens assertion (#404)

* 🐛 fix(providers): clean up abort listeners to prevent MaxListenersExceeded (#404)

* 🐛 fix(e2e): fix zai fetch interceptor request capture and timing (#404)

* fix: resolve compile TS2532 and e2e glm-5-turbo max_tokens assertion

- Fix TS2532 'Object is possibly undefined' by extracting array element
  to a local variable before accessing .maxTokens
- Update glm-5-turbo e2e expected max_tokens from 40000 to 40551
  (Math.ceil(202752 * 0.2) = 40551, matching actual context window)

* 🐛 fix: address edelauna review feedback (#632)

- Pin actions/cache to SHA per repo convention
- Add VSCode version to e2e cache key
- Remove dead teardown code in zai test interceptor
- Replace setTimeout(100) with deterministic assertion in zai tests
- Change afterAll to afterEach for spy cleanup

* 🐛 fix: replace setTimeout(100) with waitUntilReady() and remove dead code (#632)

* refactor(e2e): fetch vscode version dynamically

---------

Co-authored-by: Elliott de Launay <edelauna@gmail.com>

Author: HappyLiang12

.github/workflows/e2e.yml

@@ -34,6 +34,22 @@ jobs:
             - name: Install xvfb
               if: github.event_name != 'pull_request' || steps.e2e-marker.outputs.cache-hit != 'true'
               run: sudo apt-get install -y xvfb
+
+            - name: Get VS Code version from package.json
+              if: github.event_name != 'pull_request' || steps.e2e-marker.outputs.cache-hit != 'true'
+              id: vscode-ver
+              run: |
+                  VERSION=$(node -p 'require("./apps/vscode-e2e/package.json").devDependencies["@types/vscode"]')
+                  echo "version=$VERSION" >> $GITHUB_OUTPUT
+
+            - name: Cache VS Code test binary
+              if: github.event_name != 'pull_request' || steps.e2e-marker.outputs.cache-hit != 'true'
+              uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4
+              with:
+                  path: |
+                      apps/vscode-e2e/.vscode-test/
+                  key: vscode-test-${{ runner.os }}-${{ steps.vscode-ver.outputs.version }}-v1
+
             - name: Run mocked E2E tests
               id: run-e2e
               # merge_group and workflow_dispatch always run; cache skip is pull_request only

apps/vscode-e2e/src/runTest.ts

@@ -1,6 +1,7 @@
 import * as path from "path"
 import * as os from "os"
 import * as fs from "fs/promises"
+import { readFileSync } from "fs"
 
 import { runTests } from "@vscode/test-electron"
 import { LLMock } from "@copilotkit/aimock"
@@ -156,12 +157,16 @@ async function main() {
 		}
 
 		// Download VS Code, unzip it and run the integration test
+		// Read VS Code version from package.json to keep in sync with @types/vscode
+		const pkg = JSON.parse(readFileSync(path.resolve(__dirname, "../package.json"), "utf-8"))
+		const vscodeVersion = process.env.VSCODE_VERSION || pkg.devDependencies["@types/vscode"]
+
 		await runTests({
 			extensionDevelopmentPath,
 			extensionTestsPath,
 			launchArgs: [testWorkspace],
 			extensionTestsEnv,
-			version: process.env.VSCODE_VERSION || "1.100.0",
+			version: vscodeVersion,
 		})
 	} catch (error) {
 		console.error("Failed to run tests", error)

apps/vscode-e2e/src/suite/providers/zai.test.ts

@@ -220,6 +220,10 @@ suite("Z.ai GLM provider", function () {
 
 		await waitUntilCompleted({ api, taskId })
 		const capturedMaxTokens = requestCapture.maxTokens
+		assert.ok(
+			capturedMaxTokens !== undefined,
+			"max_tokens should have been captured by the fetch interceptor before task completion",
+		)
 
 		const completionMessage = messages.find(
 			({ say, text }) => (say === "completion_result" || say === "text") && text?.trim() === "4",
@@ -229,8 +233,6 @@ suite("Z.ai GLM provider", function () {
 
 		// Verify max_tokens uses the restored default clamp (20% of context window)
 		// unless the user explicitly overrides it via modelMaxTokens.
-		// Snapshot immediately after waitUntilCompleted to avoid straggling async calls
-		// from this task overwriting requestCapture before the assertion runs.
 		assert.strictEqual(
 			capturedMaxTokens,
 			40_000,
@@ -264,6 +266,10 @@ suite("Z.ai GLM provider", function () {
 
 		await waitUntilCompleted({ api, taskId })
 		const capturedMaxTokens = requestCapture.maxTokens
+		assert.ok(
+			capturedMaxTokens !== undefined,
+			"max_tokens should have been captured by the fetch interceptor before task completion",
+		)
 
 		const completionMessage = messages.find(
 			({ say, text }) => (say === "completion_result" || say === "text") && text?.trim() === "4",
@@ -273,11 +279,10 @@ suite("Z.ai GLM provider", function () {
 
 		// Verify max_tokens uses the restored default clamp (20% of context window)
 		// unless the user explicitly overrides it via modelMaxTokens.
-		// Snapshot immediately after waitUntilCompleted to avoid straggling async calls
-		// from the prior test overwriting requestCapture before this assertion runs.
+		const expectedMaxTokens = 40_551 // Math.ceil(202_752 * 0.2) for glm-5-turbo
 		assert.strictEqual(
 			capturedMaxTokens,
-			40_551,
+			expectedMaxTokens,
 			`max_tokens should default to the glm-5-turbo clamp (40_551) but was ${capturedMaxTokens}`,
 		)
 	})

src/core/task/Task.ts

@@ -2690,7 +2690,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 								} else {
 									signal.addEventListener("abort", () => {
 										reject(new Error("Request cancelled by user"))
-									})
+									}, { once: true })
 								}
 							})
 							return await Promise.race([nextPromise, abortPromise])
@@ -4194,7 +4194,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 		abortSignal.addEventListener("abort", () => {
 			console.log(`[Task#${this.taskId}.${this.instanceId}] AbortSignal triggered for current request`)
 			this.currentRequestAbortController = undefined
-		})
+		}, { once: true })
 
 		try {
 			// Awaiting first chunk to see if it will throw an error.
@@ -4208,7 +4208,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 				} else {
 					abortSignal.addEventListener("abort", () => {
 						reject(new Error("Request cancelled by user"))
-					})
+					}, { once: true })
 				}
 			})
 

src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts

@@ -685,4 +685,8 @@ describe("ClineProvider flicker-free cancel", () => {
 		expect(createTaskWithHistoryItemSpy).not.toHaveBeenCalled()
 		expect((provider as any).cancelledDelegationChildIds.has("child-1")).toBe(true)
 	})
+
+	afterAll(() => {
+		vi.restoreAllMocks()
+	})
 })

src/services/mcp/__tests__/McpHub.spec.ts

@@ -2112,7 +2112,7 @@ describe("McpHub", () => {
 				}
 			})
 
-			// Mock the config file read BEFORE creating McpHub
+			// Mock the config file read
 			vi.mocked(fs.readFile).mockResolvedValue(
 				JSON.stringify({
 					mcpServers: {
@@ -2124,8 +2124,10 @@ describe("McpHub", () => {
 				}),
 			)
 
-			// Create a new McpHub instance and wait for initialization
+			// Create a new McpHub instance
 			const mcpHub = new McpHub(mockProvider as ClineProvider)
+
+			// Wait for initialization
 			await mcpHub.waitUntilReady()
 
 			// Verify StdioClientTransport was called with wrapped command
@@ -2174,7 +2176,7 @@ describe("McpHub", () => {
 				}
 			})
 
-			// Mock the config file read BEFORE creating McpHub
+			// Mock the config file read
 			vi.mocked(fs.readFile).mockResolvedValue(
 				JSON.stringify({
 					mcpServers: {
@@ -2186,8 +2188,10 @@ describe("McpHub", () => {
 				}),
 			)
 
-			// Create a new McpHub instance and wait for initialization
+			// Create a new McpHub instance
 			const mcpHub = new McpHub(mockProvider as ClineProvider)
+
+			// Wait for initialization
 			await mcpHub.waitUntilReady()
 
 			// Verify StdioClientTransport was called without wrapping
@@ -2236,7 +2240,7 @@ describe("McpHub", () => {
 				}
 			})
 
-			// Mock the config file read BEFORE creating McpHub
+			// Mock the config file read with cmd.exe already as command
 			vi.mocked(fs.readFile).mockResolvedValue(
 				JSON.stringify({
 					mcpServers: {
@@ -2248,8 +2252,10 @@ describe("McpHub", () => {
 				}),
 			)
 
-			// Create a new McpHub instance and wait for initialization
+			// Create a new McpHub instance
 			const mcpHub = new McpHub(mockProvider as ClineProvider)
+
+			// Wait for initialization
 			await mcpHub.waitUntilReady()
 
 			// Verify StdioClientTransport was called without double-wrapping
@@ -2305,7 +2311,7 @@ describe("McpHub", () => {
 				}
 			})
 
-			// Mock the config file read BEFORE creating McpHub - simulating fnm/nvm-windows scenario
+			// Mock the config file read - simulating fnm/nvm-windows scenario
 			vi.mocked(fs.readFile).mockResolvedValue(
 				JSON.stringify({
 					mcpServers: {
@@ -2323,8 +2329,10 @@ describe("McpHub", () => {
 				}),
 			)
 
-			// Create a new McpHub instance and wait for initialization
+			// Create a new McpHub instance
 			const mcpHub = new McpHub(mockProvider as ClineProvider)
+
+			// Wait for initialization
 			await mcpHub.waitUntilReady()
 
 			// Verify that the command was wrapped with cmd.exe
@@ -2378,7 +2386,7 @@ describe("McpHub", () => {
 				}
 			})
 
-			// Mock the config file read BEFORE creating McpHub
+			// Mock the config file read with CMD (uppercase) as command
 			vi.mocked(fs.readFile).mockResolvedValue(
 				JSON.stringify({
 					mcpServers: {
@@ -2390,8 +2398,10 @@ describe("McpHub", () => {
 				}),
 			)
 
-			// Create a new McpHub instance and wait for initialization
+			// Create a new McpHub instance
 			const mcpHub = new McpHub(mockProvider as ClineProvider)
+
+			// Wait for initialization
 			await mcpHub.waitUntilReady()
 
 			// Verify StdioClientTransport was called without double-wrapping