From 2bef1f199aa9f1b14054795f42988e4bad3d8269 Mon Sep 17 00:00:00 2001 From: George Ng Date: Thu, 3 Sep 2026 17:05:04 -0700 Subject: [PATCH 1/8] Add structured workspace command action Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ts/.gitignore | 1 + .../agents/code/src/codeActionHandler.ts | 239 +++++++++++-- .../code/src/codeAgentWebSocketServer.ts | 83 ++++- .../vscode/workbenchCommandActionsSchema.agr | 6 +- .../vscode/workbenchCommandActionsSchema.ts | 31 +- .../src/vscode/workbenchSchema.tests.json | 10 + .../code/test/codeActionResults.spec.ts | 47 +++ ts/packages/coda/README.md | 20 ++ ts/packages/coda/package.json | 5 +- ts/packages/coda/src/extension.ts | 5 +- .../coda/src/handleWorkBenchActions.ts | 272 +++++++++++++++ .../src/test/workspaceCommandPolicy.spec.ts | 35 ++ .../src/test/workspaceCommandRunner.spec.ts | 138 ++++++++ .../coda/src/workspaceCommandPolicy.ts | 63 ++++ .../coda/src/workspaceCommandRunner.ts | 310 +++++++++++++++++ ts/packages/coda/src/wsConnect.ts | 58 +++- .../commandExecutor/VSCODE_CAPABILITIES.md | 35 ++ .../commandExecutor/src/commandServer.ts | 316 +++++++++++++++++- .../src/generatedSchemaRegistry.json | 8 + .../src/workspaceCommandMcpSchema.ts | 101 ++++++ .../test/workspaceCommandMcpSchema.spec.ts | 70 ++++ 21 files changed, 1800 insertions(+), 53 deletions(-) create mode 100644 ts/packages/agents/code/test/codeActionResults.spec.ts create mode 100644 ts/packages/coda/src/test/workspaceCommandPolicy.spec.ts create mode 100644 ts/packages/coda/src/test/workspaceCommandRunner.spec.ts create mode 100644 ts/packages/coda/src/workspaceCommandPolicy.ts create mode 100644 ts/packages/coda/src/workspaceCommandRunner.ts create mode 100644 ts/packages/commandExecutor/src/workspaceCommandMcpSchema.ts create mode 100644 ts/packages/commandExecutor/test/workspaceCommandMcpSchema.spec.ts diff --git a/ts/.gitignore b/ts/.gitignore index aed177c288..e0d557d568 100644 --- a/ts/.gitignore +++ b/ts/.gitignore @@ -4,6 +4,7 @@ .cert/ prod/ coverage/ +packages/coda/dist-test/ # Playwright /test-results/ diff --git a/ts/packages/agents/code/src/codeActionHandler.ts b/ts/packages/agents/code/src/codeActionHandler.ts index caf2f7722c..8e37a4ce5f 100644 --- a/ts/packages/agents/code/src/codeActionHandler.ts +++ b/ts/packages/agents/code/src/codeActionHandler.ts @@ -7,12 +7,14 @@ import { ActionContext, AppAction, AppAgent, + DisplayContent, ReadinessReport, SessionContext, } from "@typeagent/agent-sdk"; import Database from "better-sqlite3"; import path from "path"; import { exec } from "child_process"; +import { randomUUID } from "crypto"; import { fileURLToPath } from "url"; import os from "os"; import registerDebug from "debug"; @@ -21,6 +23,7 @@ import { ChoiceManager, createActionResultFromError, } from "@typeagent/agent-sdk/helpers/action"; +import { createStructuredContent } from "@typeagent/agent-sdk/helpers/display"; import { evaluateCodeReadiness, resolveCodePortOverride, @@ -61,14 +64,94 @@ const sharedPendingCalls: Map< { resolve: (value?: undefined) => void; context?: ActionContext | undefined; + clientId?: string | undefined; } > = new Map(); +const cancellationControlCalls = new Map< + number, + { clientId: string; responseId: unknown } +>(); // Global call-id counter. The pending-calls map is module-scoped (one // websocket server is shared across all sessions), so the id space must // also be global — per-session counters would collide on 0,1,2,... and // route a response to the wrong session's pending call. let nextSharedCallId = 0; +export function displayCodaResult(result: unknown): DisplayContent { + const message = + typeof result === "string" + ? result + : typeof result === "object" && + result !== null && + "message" in result && + typeof result.message === "string" + ? result.message + : undefined; + if (message === undefined) { + return JSON.stringify(result); + } + if (!message.startsWith("{") || !message.endsWith("}")) { + return message; + } + try { + const commandResult: unknown = JSON.parse(message); + if ( + typeof commandResult === "object" && + commandResult !== null && + "exitCode" in commandResult && + "success" in commandResult + ) { + return createStructuredContent( + [ + { + kind: "text", + text: JSON.stringify(commandResult, null, 2), + }, + ], + { rawData: commandResult }, + ); + } + } catch { + // Existing Coda actions return plain-text messages. + } + return message; +} + +export function getActionResponseTimeoutMs(action: AppAction): number { + if (action.actionName !== "runWorkspaceCommand") { + return 5_000; + } + + const requestedTimeout = action.parameters?.["timeoutMs"]; + const commandTimeout = + typeof requestedTimeout === "number" && + Number.isInteger(requestedTimeout) && + requestedTimeout > 0 && + requestedTimeout <= 5 * 60 * 1000 + ? requestedTimeout + : 2 * 60 * 1000; + // Leave enough time for Coda to terminate the shell process and send its + // final structured result after the command timeout expires. + return commandTimeout + 10_000; +} + +function getActionParameters( + action: AppAction, +): Record | undefined { + if (action.actionName !== "runWorkspaceCommand") { + return action.parameters; + } + const parameters = action.parameters ?? {}; + return { + ...parameters, + executionId: + typeof parameters.executionId === "string" && + parameters.executionId.length > 0 + ? parameters.executionId + : randomUUID(), + }; +} + export function instantiate(): AppAgent { return { initializeAgentContext: initializeCodeContext, @@ -101,6 +184,7 @@ type CodeActionContext = { { resolve: (value?: undefined) => void; context?: ActionContext | undefined; + clientId?: string | undefined; } >; // Manages yes/no choice callbacks (currently only the setup-flow card). @@ -191,18 +275,85 @@ function getCodeBindPort(): number { // server itself is module-scoped — all sessions route their pending-call // completions through the same handler. function attachSharedOnMessage(server: CodeAgentWebSocketServer): void { - server.onMessage = (message: string) => { + server.onMessage = (message: string, clientId: string) => { try { const data = JSON.parse(message) as WebSocketMessageV2; + if ( + data.method === "code/cancelWorkspaceCommand" && + typeof data.params?.executionId === "string" + ) { + const targetClientId = + server.getOnlyConnectedClientId(clientId); + if (targetClientId === undefined) { + server.sendToClient( + clientId, + JSON.stringify({ + id: data.id, + result: JSON.stringify({ + success: false, + error: "Exactly one Coda workspace must be connected.", + cancelled: false, + pendingCancellation: false, + executionId: data.params.executionId, + }), + }), + ); + return; + } + const callId = nextSharedCallId++; + cancellationControlCalls.set(callId, { + clientId, + responseId: data.id, + }); + if ( + !server.sendToClient( + targetClientId, + JSON.stringify({ + id: callId, + method: data.method, + params: { + ...data.params, + allowPendingCancellation: true, + }, + }), + ) + ) { + cancellationControlCalls.delete(callId); + } + return; + } + if (data.id !== undefined && data.result !== undefined) { + const controlCall = cancellationControlCalls.get( + Number(data.id), + ); + if (controlCall !== undefined) { + cancellationControlCalls.delete(Number(data.id)); + server.sendToClient( + controlCall.clientId, + JSON.stringify({ + ...data, + id: controlCall.responseId, + }), + ); + return; + } const pendingCall = sharedPendingCalls.get(Number(data.id)); if (pendingCall) { + if ( + pendingCall.clientId !== undefined && + pendingCall.clientId !== clientId + ) { + return; + } sharedPendingCalls.delete(Number(data.id)); const { resolve, context } = pendingCall; if (context?.actionIO) { - context.actionIO.setDisplay(data.result); + context.actionIO.setDisplay( + displayCodaResult(data.result), + ); } resolve(); } @@ -440,6 +591,7 @@ async function ensureVSCodeProcess(): Promise { async function sendPingToCodaExtension( agentContext: CodeActionContext, + clientId?: string, ): Promise { const server = agentContext.webSocketServer; if (!server || !server.isConnected()) return false; @@ -457,15 +609,23 @@ async function sendPingToCodaExtension( resolve(true); }, context: undefined as any, + clientId, }); - server.broadcast( - JSON.stringify({ - id: callId, - method: "code/ping", - params: {}, - }), - ); + const message = JSON.stringify({ + id: callId, + method: "code/ping", + params: {}, + }); + const sent = + clientId === undefined + ? server.broadcast(message) > 0 + : server.sendToClient(clientId, message); + if (!sent) { + clearTimeout(timeout); + agentContext.pendingCall.delete(callId); + resolve(false); + } }); } @@ -531,11 +691,25 @@ async function executeCodeAction( const agentContext = context.sessionContext.agentContext; const webSocketServer = agentContext.webSocketServer; + const actionParameters = getActionParameters(action); + const isStructuredWorkspaceCommand = + action.actionName === "runWorkspaceCommand" || + action.actionName === "cancelWorkspaceCommand"; if (webSocketServer && webSocketServer.isConnected()) { + const targetClientId = isStructuredWorkspaceCommand + ? webSocketServer.getOnlyConnectedClientId() + : undefined; + if (isStructuredWorkspaceCommand && targetClientId === undefined) { + return createActionResultFromError( + "Structured workspace commands require exactly one connected Coda workspace to avoid running in an unintended window.", + ); + } try { - const isExtensionAlive = - await sendPingToCodaExtension(agentContext); + const isExtensionAlive = await sendPingToCodaExtension( + agentContext, + targetClientId, + ); if (!isExtensionAlive) { return createActionResultFromError( "❌ Coda VSCode extension is not connected.", @@ -544,10 +718,26 @@ async function executeCodeAction( const callId = nextSharedCallId++; return new Promise((resolve) => { - const timeoutMs = 5000; + const timeoutMs = getActionResponseTimeoutMs(action); const timeoutHandle = setTimeout(() => { if (agentContext.pendingCall.has(callId)) { agentContext.pendingCall.delete(callId); + if ( + action.actionName === "runWorkspaceCommand" && + typeof actionParameters?.executionId === "string" + ) { + webSocketServer.sendToClient( + targetClientId!, + JSON.stringify({ + id: nextSharedCallId++, + method: "code/cancelWorkspaceCommand", + params: { + executionId: + actionParameters.executionId, + }, + }), + ); + } if (context.actionIO) { context.actionIO.setDisplay( `No connected coda extension handled action "${action.actionName}". If multiple VS Code windows are open, reload the others (Ctrl+Shift+P → Developer: Reload Window) so they pick up the latest coda bundle.`, @@ -562,14 +752,25 @@ async function executeCodeAction( resolve(value); }, context, + clientId: targetClientId, }); - webSocketServer.broadcast( - JSON.stringify({ - id: callId, - method: `code/${action.actionName}`, - params: action.parameters, - }), - ); + const message = JSON.stringify({ + id: callId, + method: `code/${action.actionName}`, + params: actionParameters, + }); + const sent = + targetClientId === undefined + ? webSocketServer.broadcast(message) > 0 + : webSocketServer.sendToClient(targetClientId, message); + if (!sent) { + clearTimeout(timeoutHandle); + agentContext.pendingCall.delete(callId); + context.actionIO?.setDisplay( + "The Coda workspace disconnected before the command could be delivered.", + ); + resolve(undefined); + } }); } catch { throw new Error("Unable to contact code backend."); diff --git a/ts/packages/agents/code/src/codeAgentWebSocketServer.ts b/ts/packages/agents/code/src/codeAgentWebSocketServer.ts index 62bfb15555..d8a63b0863 100644 --- a/ts/packages/agents/code/src/codeAgentWebSocketServer.ts +++ b/ts/packages/agents/code/src/codeAgentWebSocketServer.ts @@ -12,9 +12,10 @@ const debug = registerDebug("typeagent:code:websocket"); export class CodeAgentWebSocketServer { private clients: Map = new Map(); + private controlClientIds = new Set(); private clientIdCounter = 0; private readonly stopHeartbeat: () => void; - public onMessage?: (message: string) => void; + public onMessage?: (message: string, clientId: string) => void; /** * Fired after the {@link clients} map mutation completes for any * connect / disconnect, with the post-mutation total. Used by the @@ -118,11 +119,19 @@ export class CodeAgentWebSocketServer { } private setupHandlers(): void { - this.server.on("connection", (ws: WebSocket) => { + this.server.on("connection", (ws: WebSocket, request) => { const clientId = `client-${++this.clientIdCounter}-${Date.now()}`; + const role = new URL( + request.url ?? "", + "ws://localhost", + ).searchParams.get("role"); + const isControlClient = role === "command-executor-control"; debug("New client connected"); this.clients.set(clientId, ws); - this.onClientCountChanged?.(this.clients.size); + if (isControlClient) { + this.controlClientIds.add(clientId); + } + this.onClientCountChanged?.(this.getConnectedCount()); // Store client ID on the WebSocket for reference (ws as any).clientId = clientId; @@ -130,20 +139,22 @@ export class CodeAgentWebSocketServer { ws.on("message", (message: Buffer) => { const messageStr = message.toString(); if (this.onMessage) { - this.onMessage(messageStr); + this.onMessage(messageStr, clientId); } }); ws.on("close", () => { debug("Client disconnected"); this.clients.delete(clientId); - this.onClientCountChanged?.(this.clients.size); + this.controlClientIds.delete(clientId); + this.onClientCountChanged?.(this.getConnectedCount()); }); ws.on("error", (error) => { debug("Client error:", error); if (this.clients.delete(clientId)) { - this.onClientCountChanged?.(this.clients.size); + this.controlClientIds.delete(clientId); + this.onClientCountChanged?.(this.getConnectedCount()); } }); }); @@ -154,6 +165,9 @@ export class CodeAgentWebSocketServer { const clientsToRemove: string[] = []; for (const [clientId, client] of this.clients.entries()) { + if (this.controlClientIds.has(clientId)) { + continue; + } if (client.readyState === WebSocket.OPEN) { try { client.send(message); @@ -168,14 +182,57 @@ export class CodeAgentWebSocketServer { } // Remove failed clients - clientsToRemove.forEach((clientId) => this.clients.delete(clientId)); + clientsToRemove.forEach((clientId) => { + this.clients.delete(clientId); + this.controlClientIds.delete(clientId); + }); return successCount; } + public getOnlyConnectedClientId( + excludeClientId?: string, + ): string | undefined { + let connectedClientId: string | undefined; + for (const [clientId, client] of this.clients.entries()) { + if ( + clientId === excludeClientId || + this.controlClientIds.has(clientId) || + client.readyState !== WebSocket.OPEN + ) { + continue; + } + if (connectedClientId !== undefined) { + return undefined; + } + connectedClientId = clientId; + } + return connectedClientId; + } + + public sendToClient(clientId: string, message: string): boolean { + const client = this.clients.get(clientId); + if (client?.readyState !== WebSocket.OPEN) { + return false; + } + try { + client.send(message); + return true; + } catch (error) { + debug("Failed to send to client:", error); + this.clients.delete(clientId); + this.controlClientIds.delete(clientId); + this.onClientCountChanged?.(this.getConnectedCount()); + return false; + } + } + public isConnected(): boolean { - for (const [, client] of this.clients.entries()) { - if (client.readyState === WebSocket.OPEN) { + for (const [clientId, client] of this.clients.entries()) { + if ( + !this.controlClientIds.has(clientId) && + client.readyState === WebSocket.OPEN + ) { return true; } } @@ -184,8 +241,11 @@ export class CodeAgentWebSocketServer { public getConnectedCount(): number { let count = 0; - for (const [, client] of this.clients.entries()) { - if (client.readyState === WebSocket.OPEN) { + for (const [clientId, client] of this.clients.entries()) { + if ( + !this.controlClientIds.has(clientId) && + client.readyState === WebSocket.OPEN + ) { count++; } } @@ -228,6 +288,7 @@ export class CodeAgentWebSocketServer { } } this.clients.clear(); + this.controlClientIds.clear(); return new Promise((resolve) => { this.server.close(() => resolve()); }); diff --git a/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.agr b/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.agr index 46908822bc..d29d087fd9 100644 --- a/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.agr +++ b/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.agr @@ -13,7 +13,8 @@ import { CodeWorkbenchActions } from "./workbenchCommandActionsSchema.ts"; | | | - | ; + | + | ; = 'build' -> "build" | 'rebuild' -> "rebuild" @@ -43,4 +44,7 @@ import { CodeWorkbenchActions } from "./workbenchCommandActionsSchema.ts"; // "open the integrated terminal" = ('open' | 'show') ('the')? ('integrated')? 'terminal' -> { actionName: "openInIntegratedTerminal", parameters: {} }; +// "run command pnpm test" + = ('run' | 'execute') ('the')? 'command' $(command:wildcard) -> { actionName: "runWorkspaceCommand", parameters: { command: command } }; + = ("can you" | "please" | "would you" | "i need" | "let's")?; diff --git a/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.ts b/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.ts index 6c8f7f776e..66e3196f40 100644 --- a/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.ts +++ b/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.ts @@ -6,7 +6,9 @@ export type CodeWorkbenchActions = | WorkbenchActionFilesOpenFolder | WorkbenchActionFilesCreateFolderFromExplorer | WorkbenchActionBuildRelatedFolderTask - | WorkbenchActionOpenInIntegratedTerminal; + | WorkbenchActionOpenInIntegratedTerminal + | WorkbenchActionRunWorkspaceCommand + | WorkbenchActionCancelWorkspaceCommand; export type WorkbenchActionFilesOpenFile = { actionName: "workbenchOpenFile"; @@ -65,3 +67,30 @@ export type WorkbenchActionOpenInIntegratedTerminal = { reuseExistingTerminal?: boolean; }; }; + +// ACTION: Directly run an explicitly requested focused test, build, lint, or diagnostic command in an open workspace and return structured output. Use this instead of opening an integrated terminal when TypeAgent needs the command result. +export type WorkbenchActionRunWorkspaceCommand = { + actionName: "runWorkspaceCommand"; + parameters: { + // Exact shell command to run in an open workspace. + command: string; + // Workspace-root name or absolute path. Required for ambiguous multi-root workspaces. + workspaceFolder?: string; + // Optional path relative to the selected workspace root. + workingDirectory?: string; + // Declared command safety classification. Coda also classifies the exact command and blocks high-risk commands. + commandRiskLevel?: "low" | "medium" | "high"; + // Bounded execution time in milliseconds. + timeoutMs?: number; + // Optional caller-provided identifier; Code Agent assigns one when omitted. + executionId?: string; + }; +}; + +// ACTION: Cancel an active structured workspace command by execution ID. +export type WorkbenchActionCancelWorkspaceCommand = { + actionName: "cancelWorkspaceCommand"; + parameters: { + executionId: string; + }; +}; diff --git a/ts/packages/agents/code/src/vscode/workbenchSchema.tests.json b/ts/packages/agents/code/src/vscode/workbenchSchema.tests.json index 35c8dd3cb7..97fc03192a 100644 --- a/ts/packages/agents/code/src/vscode/workbenchSchema.tests.json +++ b/ts/packages/agents/code/src/vscode/workbenchSchema.tests.json @@ -94,5 +94,15 @@ "actionName": "openInIntegratedTerminal", "parameters": {} } + }, + { + "request": "Run command pnpm test", + "schemaName": "workbenchSchema", + "action": { + "actionName": "runWorkspaceCommand", + "parameters": { + "command": "pnpm test" + } + } } ] diff --git a/ts/packages/agents/code/test/codeActionResults.spec.ts b/ts/packages/agents/code/test/codeActionResults.spec.ts new file mode 100644 index 0000000000..c9449812b3 --- /dev/null +++ b/ts/packages/agents/code/test/codeActionResults.spec.ts @@ -0,0 +1,47 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { + displayCodaResult, + getActionResponseTimeoutMs, +} from "../src/codeActionHandler.js"; + +describe("code action structured results", () => { + test("relays a Coda workspace command result as structured raw data", () => { + const result = displayCodaResult( + JSON.stringify({ + success: true, + exitCode: 0, + durationMs: 125, + stdout: { text: "ok", truncated: false, totalBytes: 2 }, + stderr: { text: "", truncated: false, totalBytes: 0 }, + timedOut: false, + cancelled: false, + executionId: "test", + }), + ); + + expect(result).toMatchObject({ + type: "structured", + rawData: { + success: true, + exitCode: 0, + stdout: { text: "ok" }, + }, + }); + }); + + test("uses the command timeout plus transport cleanup allowance", () => { + expect( + getActionResponseTimeoutMs({ + actionName: "runWorkspaceCommand", + parameters: { timeoutMs: 10_000 }, + }), + ).toBe(20_000); + expect( + getActionResponseTimeoutMs({ + actionName: "workbenchOpenFile", + }), + ).toBe(5_000); + }); +}); diff --git a/ts/packages/coda/README.md b/ts/packages/coda/README.md index 871e108446..ddf1fa1724 100644 --- a/ts/packages/coda/README.md +++ b/ts/packages/coda/README.md @@ -22,6 +22,26 @@ To deploy the extension locally in your vscode environment, run `pnpm run deploy - Create a new python code file that merges two sorted arrays of numbers - etc +### Structured workspace commands + +The `code-workbench.runWorkspaceCommand` action executes an explicitly supplied +shell command and returns JSON with stdout, stderr, exit code, duration, status, +timeout, cancellation, and output-truncation metadata. Commands run in the active +editor's workspace root, the only workspace root, or a requested root in +multi-root workspaces. A relative `workingDirectory` may select a subdirectory. + +The action uses `cmd.exe /d /s /c` on Windows and `/bin/sh -c` on Unix, passing +the command as one shell argument. Commands are limited to 16 KiB; each output +stream is limited to 64 KiB and reports its original byte count. The timeout +defaults to two minutes and is capped at five. High-risk commands remain blocked +by the existing Coda command-risk policy. To avoid treating this as an arbitrary +terminal shortcut, structured execution accepts only focused build, test, lint, +and diagnostic tools and rejects shell composition. Commands receive an `executionId`; +independent IDs run concurrently inside one Coda window and +`cancelWorkspaceCommand` cancels one. The Code Agent sends structured commands +only when exactly one Coda workspace is connected, preventing a command from +running in an unintended VS Code window. + > Tip: If you have any problems, please check to see if the `coda` extension was installed correctly using the command `code --list-extensions`. You should see `aisystems.copilot-coda` in the list of vscode extensions. ## Requirements diff --git a/ts/packages/coda/package.json b/ts/packages/coda/package.json index ad644fc723..3ff1d38b1e 100644 --- a/ts/packages/coda/package.json +++ b/ts/packages/coda/package.json @@ -20,7 +20,7 @@ "scripts": { "build": "pnpm run esbuild-base --sourcemap", "build:ext:vscode": "pnpm run package", - "clean": "rimraf --glob out dist-pub *.tsbuildinfo *.done.build.log", + "clean": "rimraf --glob out dist-pub dist-test *.tsbuildinfo *.done.build.log", "deploy:local": "pnpm run package && code --install-extension dist-pub/aisystems-coda.vsix --force", "esbuild-base": "esbuild ./src/extension.ts --bundle --outfile=out/extension.js --external:vscode --format=cjs --platform=node --log-level=warning", "esbuild-watch": "pnpm run esbuild-base --sourcemap --watch", @@ -29,7 +29,8 @@ "prettier": "prettier --check . --ignore-path ../../.prettierignore", "prettier:fix": "prettier --write . --ignore-path ../../.prettierignore", "pretest": "pnpm run build", - "test-compile": "tsc -p ./src", + "test-compile": "pnpm --dir ../agents/code exec tsc -p ../../coda/src", + "test:local": "pnpm --dir ../agents/code exec tsc --target ES2022 --module Node16 --moduleResolution Node16 --esModuleInterop --strict --types node --outDir ../../coda/dist-test --rootDir ../../coda/src ../../coda/src/workspaceCommandRunner.ts ../../coda/src/workspaceCommandPolicy.ts ../../coda/src/test/workspaceCommandRunner.spec.ts ../../coda/src/test/workspaceCommandPolicy.spec.ts && node --test dist-test/test/workspaceCommandRunner.spec.js dist-test/test/workspaceCommandPolicy.spec.js", "test:full": "vscode-test", "vscode:prepublish": "pnpm run esbuild-base --minify", "watch": "tsc -w" diff --git a/ts/packages/coda/src/extension.ts b/ts/packages/coda/src/extension.ts index 9b654c9e14..864fd58ee5 100644 --- a/ts/packages/coda/src/extension.ts +++ b/ts/packages/coda/src/extension.ts @@ -7,6 +7,7 @@ import * as fs from "fs"; import * as vscode from "vscode"; import { initializeWS } from "./wsConnect"; import { initializeAliasManager } from "./commandAliasMgr"; +import { cancelWorkspaceCommands } from "./handleWorkBenchActions"; // This method is called when your extension is activated // Your extension is activated the very first time the command is executed @@ -46,4 +47,6 @@ export function activate(context: vscode.ExtensionContext) { } // This method is called when your extension is deactivated -export function deactivate() {} +export function deactivate() { + cancelWorkspaceCommands(); +} diff --git a/ts/packages/coda/src/handleWorkBenchActions.ts b/ts/packages/coda/src/handleWorkBenchActions.ts index 16159427d4..726f4c8bff 100644 --- a/ts/packages/coda/src/handleWorkBenchActions.ts +++ b/ts/packages/coda/src/handleWorkBenchActions.ts @@ -10,6 +10,272 @@ import * as path from "path"; import * as vscode from "vscode"; import * as fs from "fs/promises"; import { aliasManager } from "./commandAliasMgr"; +import { + WorkspaceCommandRunner, + WorkspaceCommandResult, +} from "./workspaceCommandRunner"; +import { validateFocusedWorkspaceCommand } from "./workspaceCommandPolicy"; + +const workspaceCommandRunner = new WorkspaceCommandRunner(); + +type WorkspaceCommandParameters = { + command?: unknown; + workspaceFolder?: unknown; + workingDirectory?: unknown; + commandRiskLevel?: unknown; + timeoutMs?: unknown; + executionId?: unknown; + allowPendingCancellation?: unknown; +}; + +function workspaceCommandResponse( + result: + | WorkspaceCommandResult + | { + success: boolean; + error?: string; + executionId?: string; + cancelled?: boolean; + pendingCancellation?: boolean; + }, +): ActionResult { + if ("exitCode" in result) { + return { handled: true, message: JSON.stringify(result) }; + } + return { + handled: true, + message: JSON.stringify({ + success: result.success, + error: result.error, + exitCode: null, + durationMs: 0, + stdout: { text: "", truncated: false, totalBytes: 0 }, + stderr: { text: "", truncated: false, totalBytes: 0 }, + timedOut: false, + cancelled: result.cancelled ?? false, + pendingCancellation: result.pendingCancellation ?? false, + executionId: result.executionId, + }), + }; +} + +function workspaceCommandError( + error: string, + executionId?: string, +): ActionResult { + return workspaceCommandResponse({ success: false, error, executionId }); +} + +async function resolveWorkspaceCommandDirectory( + parameters: WorkspaceCommandParameters, +): Promise { + const workspaceFolders = vscode.workspace.workspaceFolders; + if (!workspaceFolders || workspaceFolders.length === 0) { + return { error: "No workspace or repository is currently open." }; + } + if ( + parameters.workspaceFolder !== undefined && + (typeof parameters.workspaceFolder !== "string" || + parameters.workspaceFolder.trim().length === 0) + ) { + return { error: "workspaceFolder must be a non-empty string." }; + } + if ( + parameters.workingDirectory !== undefined && + (typeof parameters.workingDirectory !== "string" || + parameters.workingDirectory.trim().length === 0) + ) { + return { error: "workingDirectory must be a non-empty string." }; + } + + let workspaceFolder: vscode.WorkspaceFolder | undefined; + if (typeof parameters.workspaceFolder === "string") { + const requested = parameters.workspaceFolder; + const requestedPath = path.resolve(requested); + workspaceFolder = workspaceFolders.find( + (folder) => + folder.name === requested || + path.resolve(folder.uri.fsPath) === requestedPath, + ); + if (workspaceFolder === undefined) { + return { + error: `No open workspace root matches '${requested}'.`, + }; + } + } else { + const activeUri = vscode.window.activeTextEditor?.document.uri; + workspaceFolder = activeUri + ? vscode.workspace.getWorkspaceFolder(activeUri) + : undefined; + if (workspaceFolder === undefined && workspaceFolders.length === 1) { + workspaceFolder = workspaceFolders[0]; + } + if (workspaceFolder === undefined) { + return { + error: "Multiple workspace roots are open. Specify workspaceFolder by name or absolute path.", + }; + } + } + if (workspaceFolder.uri.scheme !== "file") { + return { + error: `Workspace root '${workspaceFolder.name}' is not a local filesystem folder.`, + }; + } + + const workingDirectory = + typeof parameters.workingDirectory === "string" + ? parameters.workingDirectory + : undefined; + const root = workspaceFolder.uri.fsPath; + const cwd = + workingDirectory !== undefined + ? path.resolve(root, workingDirectory) + : root; + const relativePath = path.relative(root, cwd); + if ( + (workingDirectory !== undefined && path.isAbsolute(workingDirectory)) || + relativePath === ".." || + relativePath.startsWith(`..${path.sep}`) || + path.isAbsolute(relativePath) + ) { + return { + error: "workingDirectory must stay within the selected workspace root.", + }; + } + try { + const stat = await fs.stat(cwd); + if (!stat.isDirectory()) { + return { error: `workingDirectory is not a directory: ${cwd}` }; + } + } catch (error) { + return { + error: `workingDirectory does not exist: ${ + error instanceof Error ? error.message : String(error) + }`, + }; + } + return cwd; +} + +export async function handleRunWorkspaceCommand(action: { + parameters?: WorkspaceCommandParameters; +}): Promise { + const parameters = action.parameters ?? {}; + const executionId = + typeof parameters.executionId === "string" + ? parameters.executionId + : undefined; + if (typeof parameters.command !== "string") { + return workspaceCommandError("command must be a string.", executionId); + } + if ( + parameters.timeoutMs !== undefined && + typeof parameters.timeoutMs !== "number" + ) { + return workspaceCommandError( + "timeoutMs must be a number.", + executionId, + ); + } + if ( + parameters.executionId !== undefined && + (typeof parameters.executionId !== "string" || + parameters.executionId.trim().length === 0 || + parameters.executionId.length > 128) + ) { + return workspaceCommandError( + "executionId must be a non-empty string no longer than 128 characters.", + ); + } + if ( + executionId !== undefined && + workspaceCommandRunner.consumePendingCancellation(executionId) + ) { + return workspaceCommandResponse({ + success: false, + exitCode: null, + durationMs: 0, + command: parameters.command, + cwd: "", + stdout: { text: "", truncated: false, totalBytes: 0 }, + stderr: { text: "", truncated: false, totalBytes: 0 }, + timedOut: false, + cancelled: true, + executionId, + }); + } + const declaredRiskLevel = + parameters.commandRiskLevel === undefined + ? "low" + : parameters.commandRiskLevel; + if ( + declaredRiskLevel !== "low" && + declaredRiskLevel !== "medium" && + declaredRiskLevel !== "high" + ) { + return workspaceCommandError( + "commandRiskLevel must be low, medium, or high.", + executionId, + ); + } + const cwd = await resolveWorkspaceCommandDirectory(parameters); + if (typeof cwd !== "string") { + return workspaceCommandError(cwd.error, executionId); + } + + if (declaredRiskLevel === "high") { + return workspaceCommandError( + "Command execution blocked due to high risk.", + executionId, + ); + } + const commandPolicyError = validateFocusedWorkspaceCommand( + parameters.command, + ); + if (commandPolicyError !== undefined) { + return workspaceCommandError(commandPolicyError, executionId); + } + const result = await workspaceCommandRunner.run({ + command: parameters.command, + cwd, + ...(parameters.timeoutMs === undefined + ? {} + : { timeoutMs: parameters.timeoutMs }), + ...(parameters.executionId === undefined + ? {} + : { executionId: parameters.executionId }), + }); + return "error" in result + ? workspaceCommandError(result.error, executionId) + : workspaceCommandResponse(result); +} + +export function handleCancelWorkspaceCommand(action: { + parameters?: WorkspaceCommandParameters; +}): ActionResult { + const executionId = action.parameters?.executionId; + if (typeof executionId !== "string" || executionId.trim().length === 0) { + return workspaceCommandError("executionId must be a non-empty string."); + } + const cancellation = workspaceCommandRunner.cancel( + executionId, + action.parameters?.allowPendingCancellation === true, + ); + const cancelled = cancellation === "cancelled"; + return workspaceCommandResponse({ + success: cancellation !== "notFound", + cancelled, + pendingCancellation: cancellation === "pending", + executionId, + ...(cancellation !== "notFound" + ? {} + : { error: "No active command has that executionId." }), + }); +} + +export function cancelWorkspaceCommands(): void { + workspaceCommandRunner.cancelAll(); +} async function handleOpenFileAction(action: any): Promise { const actionResult: ActionResult = { @@ -539,6 +805,12 @@ export async function handleWorkbenchActions( case "openInIntegratedTerminal": actionResult = await handleOpenInIntegratedTerminal(action); break; + case "runWorkspaceCommand": + actionResult = await handleRunWorkspaceCommand(action); + break; + case "cancelWorkspaceCommand": + actionResult = handleCancelWorkspaceCommand(action); + break; default: { actionResult.message = `Did not understand the request for action: "${actionName}"`; actionResult.handled = false; diff --git a/ts/packages/coda/src/test/workspaceCommandPolicy.spec.ts b/ts/packages/coda/src/test/workspaceCommandPolicy.spec.ts new file mode 100644 index 0000000000..6dab67b7a5 --- /dev/null +++ b/ts/packages/coda/src/test/workspaceCommandPolicy.spec.ts @@ -0,0 +1,35 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import assert from "node:assert/strict"; +import test from "node:test"; +import { validateFocusedWorkspaceCommand } from "../workspaceCommandPolicy"; + +test("allows focused test and build commands", () => { + assert.equal( + validateFocusedWorkspaceCommand("pnpm test -- --runInBand"), + undefined, + ); + assert.equal( + validateFocusedWorkspaceCommand("dotnet build project.sln"), + undefined, + ); + assert.equal( + validateFocusedWorkspaceCommand("vitest run src/example.spec.ts"), + undefined, + ); +}); + +test("rejects shell composition and non-focused commands", () => { + for (const command of [ + " rm -rf target", + "echo preparing\nrm -rf target", + "sh -c 'rm -rf target'", + "powershell Remove-Item -Recurse target", + "pnpm test && rm -rf target", + "bun x cowsay", + "bun run ./script.ts", + ]) { + assert.notEqual(validateFocusedWorkspaceCommand(command), undefined); + } +}); diff --git a/ts/packages/coda/src/test/workspaceCommandRunner.spec.ts b/ts/packages/coda/src/test/workspaceCommandRunner.spec.ts new file mode 100644 index 0000000000..020a5fd0e3 --- /dev/null +++ b/ts/packages/coda/src/test/workspaceCommandRunner.spec.ts @@ -0,0 +1,138 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import assert from "node:assert/strict"; +import test from "node:test"; +import { + MAX_COMMAND_BYTES, + WorkspaceCommandRunner, +} from "../workspaceCommandRunner.js"; + +test("runs a requested command and returns separate structured streams", async () => { + const runner = new WorkspaceCommandRunner(); + const result = await runner.run({ + command: `node -e "console.log('out'); console.error('err')"`, + cwd: process.cwd(), + executionId: "streams", + }); + assert.ok(!("error" in result)); + assert.ok(!("error" in result)); + assert.equal(result.success, true); + assert.equal(result.exitCode, 0); + assert.equal(result.timedOut, false); + assert.equal(result.cancelled, false); + assert.match(result.stdout.text, /out/); + assert.match(result.stderr.text, /err/); +}); + +test("reports a timed-out process", async () => { + const runner = new WorkspaceCommandRunner(); + const result = await runner.run({ + command: `node -e "setTimeout(() => {}, 10000)"`, + cwd: process.cwd(), + timeoutMs: 50, + }); + + assert.ok(!("error" in result)); + assert.equal(result.success, false); + assert.equal(result.timedOut, true); + assert.equal(result.cancelled, false); +}); + +test("rejects an oversized command before spawning", async () => { + const runner = new WorkspaceCommandRunner(); + const result = await runner.run({ + command: "x".repeat(MAX_COMMAND_BYTES + 1), + cwd: process.cwd(), + }); + + assert.deepEqual(result, { + error: `Command exceeds the ${MAX_COMMAND_BYTES}-byte limit.`, + }); +}); + +test("tracks concurrent commands separately by execution ID", async () => { + const runner = new WorkspaceCommandRunner(); + const [first, second] = await Promise.all([ + runner.run({ + command: `node -e "console.log('first')"`, + cwd: process.cwd(), + executionId: "first", + }), + runner.run({ + command: `node -e "console.log('second')"`, + cwd: process.cwd(), + executionId: "second", + }), + ]); + + assert.ok(!("error" in first)); + assert.ok(!("error" in second)); + assert.match(first.stdout.text, /first/); + assert.match(second.stdout.text, /second/); +}); + +test("preserves a quoted Windows executable path", async () => { + if (process.platform !== "win32") { + return; + } + const runner = new WorkspaceCommandRunner(); + const result = await runner.run({ + command: `"${process.execPath}" -e "console.log('quoted-exe')"`, + cwd: process.cwd(), + }); + + assert.ok(!("error" in result)); + assert.equal(result.success, true); + assert.match(result.stdout.text, /quoted-exe/); +}); + +test("cancels an active command by execution ID", async () => { + const runner = new WorkspaceCommandRunner(); + const pending = runner.run({ + command: `node -e "setTimeout(() => {}, 10000)"`, + cwd: process.cwd(), + executionId: "cancel-me", + }); + await new Promise((resolve) => setTimeout(resolve, 100)); + + assert.equal(runner.cancel("cancel-me"), "cancelled"); + const result = await pending; + assert.ok(!("error" in result)); + assert.equal(result.success, false); + assert.equal(result.cancelled, true); + assert.equal(result.timedOut, false); +}); + +test("honors cancellation received before command registration", async () => { + const runner = new WorkspaceCommandRunner(); + assert.equal(runner.cancel("cancel-before-run", true), "pending"); + const result = await runner.run({ + command: `node -e "console.log('must not run')"`, + cwd: process.cwd(), + executionId: "cancel-before-run", + }); + + assert.ok(!("error" in result)); + assert.equal(result.success, false); + assert.equal(result.cancelled, true); + assert.equal(result.stdout.text, ""); +}); + +test("does not arm an unknown ordinary cancellation request", () => { + const runner = new WorkspaceCommandRunner(); + assert.equal(runner.cancel("unknown"), "notFound"); +}); + +test("consumes a pending cancellation before command validation", async () => { + const runner = new WorkspaceCommandRunner(); + assert.equal(runner.cancel("cancel-invalid", true), "pending"); + const result = await runner.run({ + command: "", + cwd: process.cwd(), + executionId: "cancel-invalid", + }); + + assert.ok(!("error" in result)); + assert.equal(result.cancelled, true); +}); diff --git a/ts/packages/coda/src/workspaceCommandPolicy.ts b/ts/packages/coda/src/workspaceCommandPolicy.ts new file mode 100644 index 0000000000..bdb60520bd --- /dev/null +++ b/ts/packages/coda/src/workspaceCommandPolicy.ts @@ -0,0 +1,63 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +const shellSyntax = /[;&|<>`$()\r\n^]/; + +const directTools = new Set([ + "ava", + "bazel", + "eslint", + "flake8", + "jest", + "mocha", + "msbuild", + "ninja", + "prettier", + "pytest", + "ruff", + "tsc", + "vitest", + "xcodebuild", +]); + +const subcommandTools = new Map([ + ["bun", /^(?:run\s+(?:build|check|format|lint|test|typecheck)\b|test\b)/i], + ["cargo", /^(?:build|check|clippy|fmt|test)\b/i], + ["cmake", /^--build\b/i], + ["dotnet", /^(?:build|format|test)\b/i], + ["go", /^(?:build|test|vet)\b/i], + ["gradle", /^(?:build|check|test)\b/i], + ["gradlew", /^(?:build|check|test)\b/i], + ["mvn", /^(?:compile|package|test|verify)\b/i], + ["npm", /^(?:run\s+(?:build|check|format|lint|test|typecheck)\b|test\b)/i], + ["pnpm", /^(?:run\s+(?:build|check|format|lint|test|typecheck)\b|test\b)/i], + ["yarn", /^(?:run\s+(?:build|check|format|lint|test|typecheck)\b|test\b)/i], +]); + +/** + * Commands are intentionally restricted to focused build, test, lint, and + * diagnostic tools. Accepting general shell syntax would make safety depend on + * an incomplete denylist. + */ +export function validateFocusedWorkspaceCommand( + command: string, +): string | undefined { + const trimmed = command.trim(); + if (shellSyntax.test(trimmed)) { + return "Shell composition is not supported for structured workspace commands."; + } + const match = /^([A-Za-z0-9_.-]+)(?:\s+(.*))?$/.exec(trimmed); + if (!match) { + return "Command must begin with a supported focused build, test, lint, or diagnostic tool."; + } + const [, executable, argumentsText = ""] = match; + const normalizedExecutable = executable.toLowerCase(); + if (directTools.has(normalizedExecutable)) { + return undefined; + } + const subcommand = subcommandTools.get(normalizedExecutable); + if (subcommand?.test(argumentsText)) { + return undefined; + } + return "Only focused build, test, lint, and diagnostic commands are supported."; +} diff --git a/ts/packages/coda/src/workspaceCommandRunner.ts b/ts/packages/coda/src/workspaceCommandRunner.ts new file mode 100644 index 0000000000..5249d623eb --- /dev/null +++ b/ts/packages/coda/src/workspaceCommandRunner.ts @@ -0,0 +1,310 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { ChildProcess, spawn } from "child_process"; +import { randomUUID } from "crypto"; + +export const MAX_COMMAND_BYTES = 16 * 1024; +export const MAX_OUTPUT_BYTES = 64 * 1024; +export const DEFAULT_TIMEOUT_MS = 2 * 60 * 1000; +export const MAX_TIMEOUT_MS = 5 * 60 * 1000; +const MAX_PENDING_CANCELLATIONS = 64; + +export type WorkspaceCommandOutput = { + text: string; + truncated: boolean; + totalBytes: number; +}; + +export type WorkspaceCommandResult = { + success: boolean; + exitCode: number | null; + durationMs: number; + command: string; + cwd: string; + stdout: WorkspaceCommandOutput; + stderr: WorkspaceCommandOutput; + timedOut: boolean; + cancelled: boolean; + executionId: string; +}; + +export type WorkspaceCommandRunOptions = { + command: string; + cwd: string; + timeoutMs?: number; + executionId?: string; +}; + +export type WorkspaceCommandCancellation = "cancelled" | "pending" | "notFound"; + +type CapturedOutput = { + chunks: Buffer[]; + capturedBytes: number; + totalBytes: number; +}; + +function captureOutput(captured: CapturedOutput, chunk: Buffer | string): void { + const bytes = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); + captured.totalBytes += bytes.length; + const remaining = MAX_OUTPUT_BYTES - captured.capturedBytes; + if (remaining <= 0) { + return; + } + const accepted = bytes.subarray(0, remaining); + captured.chunks.push(accepted); + captured.capturedBytes += accepted.length; +} + +function output(captured: CapturedOutput): WorkspaceCommandOutput { + return { + text: Buffer.concat(captured.chunks).toString("utf8"), + truncated: captured.totalBytes > captured.capturedBytes, + totalBytes: captured.totalBytes, + }; +} + +function validateCommand(command: string): string | undefined { + if (command.trim().length === 0) { + return "A non-empty command is required."; + } + if (Buffer.byteLength(command, "utf8") > MAX_COMMAND_BYTES) { + return `Command exceeds the ${MAX_COMMAND_BYTES}-byte limit.`; + } + return undefined; +} + +function validateTimeout(timeoutMs: number | undefined): number | string { + if (timeoutMs === undefined) { + return DEFAULT_TIMEOUT_MS; + } + if ( + !Number.isInteger(timeoutMs) || + timeoutMs <= 0 || + timeoutMs > MAX_TIMEOUT_MS + ) { + return `timeoutMs must be an integer between 1 and ${MAX_TIMEOUT_MS}.`; + } + return timeoutMs; +} + +async function terminateProcessTree(child: ChildProcess): Promise { + if (child.pid === undefined) { + child.kill(); + return; + } + if (process.platform === "win32") { + await new Promise((resolve) => { + const killer = spawn( + "taskkill", + ["/PID", String(child.pid), "/T", "/F"], + { + stdio: "ignore", + windowsHide: true, + }, + ); + killer.once("error", () => { + child.kill(); + resolve(); + }); + killer.once("close", () => resolve()); + }); + return; + } + try { + process.kill(-child.pid, "SIGTERM"); + } catch (error: unknown) { + if (!(error instanceof Error) || error.name !== "Error") { + throw error; + } + const code = (error as NodeJS.ErrnoException).code; + if (code !== "ESRCH") { + throw error; + } + } + setTimeout(() => { + try { + process.kill(-child.pid!, "SIGKILL"); + } catch { + // The process group exited during the grace period. + } + }, 1_000).unref(); +} + +export class WorkspaceCommandRunner { + private readonly activeCommands = new Map< + string, + { child: ChildProcess; cancel: () => void } + >(); + private readonly pendingCancellations = new Set(); + + public async run( + options: WorkspaceCommandRunOptions, + ): Promise { + const executionId = options.executionId ?? randomUUID(); + if (this.consumePendingCancellation(executionId)) { + return { + success: false, + exitCode: null, + durationMs: 0, + command: options.command, + cwd: options.cwd, + stdout: { text: "", truncated: false, totalBytes: 0 }, + stderr: { text: "", truncated: false, totalBytes: 0 }, + timedOut: false, + cancelled: true, + executionId, + }; + } + const commandError = validateCommand(options.command); + if (commandError !== undefined) { + return { error: commandError }; + } + const timeout = validateTimeout(options.timeoutMs); + if (typeof timeout === "string") { + return { error: timeout }; + } + if (this.activeCommands.has(executionId)) { + return { + error: `A command with executionId '${executionId}' is already running.`, + }; + } + + const startedAt = Date.now(); + const stdout: CapturedOutput = { + chunks: [], + capturedBytes: 0, + totalBytes: 0, + }; + const stderr: CapturedOutput = { + chunks: [], + capturedBytes: 0, + totalBytes: 0, + }; + let timedOut = false; + let cancelled = false; + let stopping = false; + const isWindows = process.platform === "win32"; + const shell = isWindows ? process.env.ComSpec || "cmd.exe" : "/bin/sh"; + const shellArgs = isWindows + ? ["/d", "/s", "/c", `"${options.command}"`] + : ["-c", options.command]; + + return new Promise((resolve) => { + let child: ChildProcess; + try { + child = spawn(shell, shellArgs, { + cwd: options.cwd, + detached: !isWindows, + shell: false, + stdio: ["ignore", "pipe", "pipe"], + // cmd.exe parses its command after /c. Node's normal + // Windows argument escaping changes embedded quotes. + windowsVerbatimArguments: isWindows, + windowsHide: true, + }); + } catch (error) { + resolve({ + error: + error instanceof Error ? error.message : String(error), + }); + return; + } + + const finish = (exitCode: number | null) => { + clearTimeout(timeoutHandle); + this.activeCommands.delete(executionId); + resolve({ + success: exitCode === 0 && !timedOut && !cancelled, + exitCode, + durationMs: Date.now() - startedAt, + command: options.command, + cwd: options.cwd, + stdout: output(stdout), + stderr: output(stderr), + timedOut, + cancelled, + executionId, + }); + }; + const stop = (reason: "timeout" | "cancel") => { + if (stopping) { + return; + } + stopping = true; + if (reason === "timeout") { + timedOut = true; + } else { + cancelled = true; + } + void terminateProcessTree(child).catch((error: unknown) => { + captureOutput( + stderr, + `Failed to terminate command: ${ + error instanceof Error + ? error.message + : String(error) + }`, + ); + }); + }; + const timeoutHandle = setTimeout(() => stop("timeout"), timeout); + + this.activeCommands.set(executionId, { + child, + cancel: () => stop("cancel"), + }); + child.stdout?.on("data", (chunk: Buffer | string) => + captureOutput(stdout, chunk), + ); + child.stderr?.on("data", (chunk: Buffer | string) => + captureOutput(stderr, chunk), + ); + child.once("error", (error) => { + captureOutput(stderr, error.message); + }); + child.once("close", (exitCode) => finish(exitCode)); + child.once("spawn", () => { + if (cancelled) { + stop("cancel"); + } + }); + }); + } + + public cancel( + executionId: string, + allowPendingCancellation = false, + ): WorkspaceCommandCancellation { + const command = this.activeCommands.get(executionId); + if (command === undefined) { + if (this.pendingCancellations.has(executionId)) { + return "pending"; + } + if (!allowPendingCancellation) { + return "notFound"; + } + if (this.pendingCancellations.size >= MAX_PENDING_CANCELLATIONS) { + const oldest = this.pendingCancellations.values().next().value; + if (oldest !== undefined) { + this.pendingCancellations.delete(oldest); + } + } + this.pendingCancellations.add(executionId); + return "pending"; + } + command.cancel(); + return "cancelled"; + } + + public consumePendingCancellation(executionId: string): boolean { + return this.pendingCancellations.delete(executionId); + } + + public cancelAll(): void { + for (const command of this.activeCommands.values()) { + command.cancel(); + } + this.pendingCancellations.clear(); + } +} diff --git a/ts/packages/coda/src/wsConnect.ts b/ts/packages/coda/src/wsConnect.ts index 51602a919a..27dc4903d2 100644 --- a/ts/packages/coda/src/wsConnect.ts +++ b/ts/packages/coda/src/wsConnect.ts @@ -4,6 +4,7 @@ import WebSocket from "ws"; import { createWebSocket, keepWebSocketAlive } from "./webSocket"; import { handleVSCodeActions } from "./handleVSCodeActions"; +import { cancelWorkspaceCommands } from "./handleWorkBenchActions"; type WebSocketMessageV2 = { id?: string; @@ -18,6 +19,44 @@ type WebSocketMessageV2 = { let webSocket: WebSocket | undefined = undefined; +async function handleActionMessage(data: WebSocketMessageV2): Promise { + const [schema, actionName] = data.method.split("/"); + if (schema !== "code") { + return; + } + try { + const message = await handleVSCodeActions({ + actionName, + parameters: data.params ?? {}, + }); + webSocket?.send( + JSON.stringify({ + id: data.id, + result: message, + }), + ); + } catch (error) { + console.error("Error handling websocket action:", error); + webSocket?.send( + JSON.stringify({ + id: data.id, + result: JSON.stringify({ + success: false, + error: + error instanceof Error ? error.message : String(error), + exitCode: null, + durationMs: 0, + stdout: { text: "", truncated: false, totalBytes: 0 }, + stderr: { text: "", truncated: false, totalBytes: 0 }, + timedOut: false, + cancelled: false, + executionId: data.params?.executionId, + }), + }), + ); + } +} + async function ensureWebsocketConnected() { if (webSocket && webSocket.readyState === WebSocket.OPEN) { return; @@ -69,21 +108,9 @@ async function ensureWebsocketConnected() { } if (data.method !== undefined && data.method.indexOf("/") > 0) { - const [schema, actionName] = data.method?.split("/"); - - if (schema == "code") { - const message = await handleVSCodeActions({ - actionName: actionName, - parameters: data?.params ?? {}, - }); - - webSocket?.send( - JSON.stringify({ - id: data.id, - result: message, - }), - ); - } + // Do not await a long-running command here: cancellation and other + // requests must continue to reach the extension while it runs. + void handleActionMessage(data); } console.log( `vscode extension websocket client received message: ${JSON.stringify(data, null, 2)}`, @@ -92,6 +119,7 @@ async function ensureWebsocketConnected() { webSocket.onclose = (event: any) => { console.log("websocket connection closed"); + cancelWorkspaceCommands(); webSocket = undefined; reconnectWebSocket(); }; diff --git a/ts/packages/commandExecutor/VSCODE_CAPABILITIES.md b/ts/packages/commandExecutor/VSCODE_CAPABILITIES.md index d50a6acfc0..5fddfa02f1 100644 --- a/ts/packages/commandExecutor/VSCODE_CAPABILITIES.md +++ b/ts/packages/commandExecutor/VSCODE_CAPABILITIES.md @@ -128,6 +128,39 @@ The Coda VSCode extension connects to TypeAgent's dispatcher and can execute var - "open terminal in src folder" - "open terminal and run npm install" +### Structured workspace commands + +For an explicit focused command, call the direct MCP tool instead of +`execute_command`: + +```json +{ + "tool": "run_workspace_command", + "arguments": { + "command": "pnpm test -- --runInBand", + "workingDirectory": "ts/packages/coda", + "timeoutMs": 120000, + "executionId": "coda-tests-1" + } +} +``` + +The tool bypasses natural-language translation and terminal UI. It returns +structured stdout, stderr, exit code, duration, success, timeout, cancellation, +and stream-truncation metadata. In a multi-root workspace, pass +`workspaceFolder` by root name or absolute path unless the active editor +identifies the target root. Use `cancel_workspace_command` with an +`executionId` to terminate a running command. The caller must provide this ID +when starting a command. Coda accepts focused build, test, lint, and diagnostic +tools only; shell composition is rejected. A cancellation issued while the +command is still in transit reports `pendingCancellation: true` and prevents +that command from starting. + +Commands with distinct IDs can run concurrently in Coda. Each Code Agent only +routes structured commands when exactly one Coda workspace is connected. The +command result remains synchronous; when a caller sets a timeout, Code Agent +uses its cleanup allowance to send Coda an out-of-band cancellation request. + ### Tasks & Build **Run Tasks:** @@ -281,6 +314,8 @@ Here are the internal action names (useful for understanding the code): - `workbenchCreateFolderFromExplorer` - `workbenchBuildRelatedTask` - `openInIntegratedTerminal` +- `runWorkspaceCommand` +- `cancelWorkspaceCommand` ## Prerequisites diff --git a/ts/packages/commandExecutor/src/commandServer.ts b/ts/packages/commandExecutor/src/commandServer.ts index 4ac6245354..49685b2f3c 100644 --- a/ts/packages/commandExecutor/src/commandServer.ts +++ b/ts/packages/commandExecutor/src/commandServer.ts @@ -11,6 +11,7 @@ import { AgentServerConnection, AGENT_SERVER_DEFAULT_URL, } from "@typeagent/agent-server-client"; +import { discoverPort } from "@typeagent/agent-server-client/discovery"; import type { AgentSchemaInfo, ClientIO, @@ -30,6 +31,14 @@ import * as path from "path"; import * as os from "os"; import { convert } from "html-to-text"; import { loadConfig, type ResolvedAgentServerConfig } from "./config/index.js"; +import { + CancelWorkspaceCommandInput, + CancelWorkspaceCommandInputSchema, + CancelWorkspaceCommandResultSchema, + WorkspaceCommandInput, + WorkspaceCommandInputSchema, + WorkspaceCommandResultSchema, +} from "./workspaceCommandMcpSchema.js"; // ── Agent filter ────────────────────────────────────────────────────────────── @@ -109,6 +118,60 @@ function toolResult(result: string, rawData?: unknown): CallToolResult { return out; } +function resultText(result: CallToolResult): string { + return result.content + .map((content) => (content.type === "text" ? content.text : "")) + .filter((text) => text.length > 0) + .join("\n"); +} + +function workspaceCommandFailure( + error: string, + executionId: string, +): CallToolResult { + const failure = { + success: false, + error, + exitCode: null, + durationMs: 0, + stdout: { text: "", truncated: false, totalBytes: 0 }, + stderr: { text: "", truncated: false, totalBytes: 0 }, + timedOut: false, + cancelled: false, + executionId, + }; + return toolResult(JSON.stringify(failure, null, 2), failure); +} + +function cancelledWorkspaceCommandResult(executionId: string): CallToolResult { + const result = { + success: false, + error: "The command request was cancelled before it was dispatched.", + exitCode: null, + durationMs: 0, + stdout: { text: "", truncated: false, totalBytes: 0 }, + stderr: { text: "", truncated: false, totalBytes: 0 }, + timedOut: false, + cancelled: true, + executionId, + }; + return toolResult(JSON.stringify(result, null, 2), result); +} + +function cancellationFailure( + error: string, + executionId: string, +): CallToolResult { + const failure = { + success: false, + error, + cancelled: false, + pendingCancellation: false, + executionId, + }; + return toolResult(JSON.stringify(failure, null, 2), failure); +} + function stripAnsi(text: string): string { return text.replace(/\x1b\[[0-9;]*m/g, ""); } @@ -352,6 +415,8 @@ export class CommandServer { messages: [], }; private currentRequestConfirmed: boolean = false; + private dispatcherRequestInFlight = false; + private workspaceCommandInFlight = false; private config: ResolvedAgentServerConfig; constructor(agentServerUrl?: string) { @@ -632,8 +697,32 @@ export class CommandServer { "- naturalLanguage: The original natural language request from the user (e.g. 'play shake it off'). ALWAYS provide this when you have the user's original request — the dispatcher uses it to populate its NL cache so future identical or similar requests can be handled without LLM translation.\n\n" + "The action is dispatched directly to the agent, bypassing the LLM translation step for maximum speed.", }, - async (request: ExecuteActionRequest) => - this.executeAction(request), + async (request: ExecuteActionRequest, extra) => + this.executeAction(request, false, extra.signal), + ); + + this.server.registerTool( + "run_workspace_command", + { + inputSchema: WorkspaceCommandInputSchema.shape, + outputSchema: WorkspaceCommandResultSchema.shape, + description: + "Run one explicitly requested build, test, lint, or diagnostic command in the open VS Code workspace through Coda. This is a direct TypeAgent action: it does not use natural-language translation or a terminal UI. Returns structured stdout, stderr, exitCode, durationMs, success, timedOut, cancelled, and truncation metadata. Example: { command: 'pnpm test -- --runInBand', workingDirectory: 'ts/packages/coda', executionId: 'coda-tests-1' }. Coda blocks high-risk commands and rejects shell composition.", + }, + async (request: WorkspaceCommandInput, extra) => + this.runWorkspaceCommand(request, extra.signal), + ); + + this.server.registerTool( + "cancel_workspace_command", + { + inputSchema: CancelWorkspaceCommandInputSchema.shape, + outputSchema: CancelWorkspaceCommandResultSchema.shape, + description: + "Cancel one active run_workspace_command request by executionId. The result reports whether a running command was cancelled or whether cancellation is pending before its command reaches Coda.", + }, + async (request: CancelWorkspaceCommandInput) => + this.cancelWorkspaceCommand(request), ); // 4. User/editor context - delegates to the code agent (VS Code CODA @@ -739,6 +828,22 @@ export class CommandServer { public async executeCommand( request: ExecuteCommandRequest, + ): Promise { + if (this.dispatcherRequestInFlight) { + return toolResult( + "Another request is already using this Command Executor. Wait for it to complete before sending another command.", + ); + } + this.dispatcherRequestInFlight = true; + try { + return await this.executeCommandUnlocked(request); + } finally { + this.dispatcherRequestInFlight = false; + } + } + + private async executeCommandUnlocked( + request: ExecuteCommandRequest, ): Promise { this.logger.log(`execute_command: ${request.request}`); @@ -971,8 +1076,211 @@ export class CommandServer { }); } + private async runWorkspaceCommand( + request: WorkspaceCommandInput, + signal?: AbortSignal, + ): Promise { + if (this.workspaceCommandInFlight) { + return workspaceCommandFailure( + "This Command Executor already has a workspace command in progress. Use a separate MCP connection for a concurrent command.", + request.executionId, + ); + } + this.workspaceCommandInFlight = true; + if (signal?.aborted) { + this.workspaceCommandInFlight = false; + return cancelledWorkspaceCommandResult(request.executionId); + } + let acquiredDispatcherLock = false; + const cancelOnAbort = () => { + void this.cancelWorkspaceCommand({ + executionId: request.executionId, + }); + }; + signal?.addEventListener("abort", cancelOnAbort, { once: true }); + try { + if (this.dispatcherRequestInFlight) { + return workspaceCommandFailure( + "Another request is already using this Command Executor. Wait for it to complete before sending another command.", + request.executionId, + ); + } + this.dispatcherRequestInFlight = true; + acquiredDispatcherLock = true; + const result = await this.executeActionUnlocked( + { + schemaName: "code.code-workbench", + actionName: "runWorkspaceCommand", + parameters: request, + }, + true, + ); + if ( + result.structuredContent !== undefined && + WorkspaceCommandResultSchema.safeParse(result.structuredContent) + .success + ) { + return result; + } + return workspaceCommandFailure( + resultText(result), + request.executionId, + ); + } finally { + if (acquiredDispatcherLock) { + this.dispatcherRequestInFlight = false; + } + signal?.removeEventListener("abort", cancelOnAbort); + this.workspaceCommandInFlight = false; + } + } + + private async cancelWorkspaceCommand( + request: CancelWorkspaceCommandInput, + ): Promise { + let endpoint: string | undefined; + try { + const discovered = await discoverPort("code", undefined, { + url: this.agentServerUrl, + }); + if (discovered.kind === "found") { + endpoint = + discovered.url ?? `ws://localhost:${discovered.port}`; + } + } catch (error) { + return cancellationFailure( + error instanceof Error ? error.message : String(error), + request.executionId, + ); + } + if (endpoint === undefined) { + return cancellationFailure( + "The Code Agent websocket is not available.", + request.executionId, + ); + } + + const url = new URL(endpoint); + url.searchParams.set("channel", "code"); + url.searchParams.set("role", "command-executor-control"); + return new Promise((resolve) => { + const socket = new WebSocket(url); + const timeout = setTimeout(() => { + socket.close(); + resolve( + cancellationFailure( + "Timed out waiting for Coda to cancel the command.", + request.executionId, + ), + ); + }, 10_000); + const finish = (result: CallToolResult) => { + clearTimeout(timeout); + socket.close(); + resolve(result); + }; + socket.addEventListener("open", () => { + socket.send( + JSON.stringify({ + id: request.executionId, + method: "code/cancelWorkspaceCommand", + params: request, + }), + ); + }); + socket.addEventListener("message", (event) => { + try { + const response = JSON.parse(String(event.data)) as { + id?: unknown; + result?: unknown; + }; + if (response.id !== request.executionId) { + return; + } + const result = + typeof response.result === "string" + ? JSON.parse(response.result) + : response.result; + if ( + CancelWorkspaceCommandResultSchema.safeParse(result) + .success + ) { + finish( + toolResult(JSON.stringify(result, null, 2), result), + ); + return; + } + } catch { + // The schema-normalized failure below gives callers a stable result. + } + finish( + cancellationFailure( + "Coda returned an invalid cancellation response.", + request.executionId, + ), + ); + }); + socket.addEventListener("error", () => { + finish( + cancellationFailure( + "Unable to contact the Code Agent websocket.", + request.executionId, + ), + ); + }); + }); + } + private async executeAction( request: ExecuteActionRequest, + preserveDisplayText = false, + signal?: AbortSignal, + ): Promise { + if ( + request.schemaName === "code.code-workbench" && + request.actionName === "runWorkspaceCommand" + ) { + const parsed = WorkspaceCommandInputSchema.safeParse( + request.parameters, + ); + return parsed.success + ? this.runWorkspaceCommand(parsed.data, signal) + : toolResult( + `Action parameters are invalid: ${parsed.error.message}`, + ); + } + if ( + request.schemaName === "code.code-workbench" && + request.actionName === "cancelWorkspaceCommand" + ) { + const parsed = CancelWorkspaceCommandInputSchema.safeParse( + request.parameters, + ); + return parsed.success + ? this.cancelWorkspaceCommand(parsed.data) + : toolResult( + `Action parameters are invalid: ${parsed.error.message}`, + ); + } + if (this.dispatcherRequestInFlight) { + return toolResult( + "Another request is already using this Command Executor. Wait for it to complete before sending another command.", + ); + } + this.dispatcherRequestInFlight = true; + try { + return await this.executeActionUnlocked( + request, + preserveDisplayText, + ); + } finally { + this.dispatcherRequestInFlight = false; + } + } + + private async executeActionUnlocked( + request: ExecuteActionRequest, + preserveDisplayText = false, ): Promise { this.logger.log( `execute_action: ${request.schemaName}.${request.actionName} params=${JSON.stringify(request.parameters ?? {})}`, @@ -1065,7 +1373,9 @@ export class CommandServer { if (this.responseCollector.messages.length > 0) { const response = this.responseCollector.messages.join("\n\n"); return toolResult( - await processHtmlContent(response), + preserveDisplayText + ? response + : await processHtmlContent(response), this.responseCollector.rawData, ); } diff --git a/ts/packages/commandExecutor/src/generatedSchemaRegistry.json b/ts/packages/commandExecutor/src/generatedSchemaRegistry.json index d473ba1b65..af01afb42b 100644 --- a/ts/packages/commandExecutor/src/generatedSchemaRegistry.json +++ b/ts/packages/commandExecutor/src/generatedSchemaRegistry.json @@ -269,6 +269,14 @@ { "name": "openInIntegratedTerminal", "description": "Open In Integrated Terminal" + }, + { + "name": "runWorkspaceCommand", + "description": "Directly run an explicitly requested focused test, build, lint, or diagnostic command and return structured output" + }, + { + "name": "cancelWorkspaceCommand", + "description": "Cancel an active structured workspace command by execution ID" } ] }, diff --git a/ts/packages/commandExecutor/src/workspaceCommandMcpSchema.ts b/ts/packages/commandExecutor/src/workspaceCommandMcpSchema.ts new file mode 100644 index 0000000000..3a82d1467b --- /dev/null +++ b/ts/packages/commandExecutor/src/workspaceCommandMcpSchema.ts @@ -0,0 +1,101 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { z } from "zod/v4"; + +export const WorkspaceCommandInputSchema = z.object({ + command: z + .string() + .min(1) + .max(16 * 1024) + .refine((value) => Buffer.byteLength(value, "utf8") <= 16 * 1024, { + message: "command must not exceed 16384 UTF-8 bytes", + }) + .describe( + "Exact shell command to execute, for example `pnpm test -- --runInBand`.", + ), + workspaceFolder: z + .string() + .min(1) + .optional() + .describe( + "Open workspace-root name or absolute path. Required when multiple roots are open and no active editor identifies one.", + ), + workingDirectory: z + .string() + .min(1) + .optional() + .describe( + "Optional path relative to the selected workspace root. It must remain inside that root.", + ), + commandRiskLevel: z + .enum(["low", "medium", "high"]) + .optional() + .describe( + "Optional declared command-risk level. High-risk commands are blocked; structured execution also restricts commands to focused build, test, lint, and diagnostic tools.", + ), + timeoutMs: z + .number() + .int() + .positive() + .max(5 * 60 * 1000) + .optional() + .describe( + "Maximum execution time in milliseconds. Defaults to 120000 and is capped at 300000.", + ), + executionId: z + .string() + .min(1) + .max(128) + .describe( + "Unique caller-generated ID used to correlate this command and cancel it later.", + ), +}); + +export type WorkspaceCommandInput = z.infer; + +export const WorkspaceCommandResultSchema = z.object({ + success: z.boolean(), + error: z.string().optional(), + exitCode: z.number().int().nullable(), + durationMs: z.number().nonnegative(), + command: z.string().optional(), + cwd: z.string().optional(), + stdout: z.object({ + text: z.string(), + truncated: z.boolean(), + totalBytes: z.number().int().nonnegative(), + }), + stderr: z.object({ + text: z.string(), + truncated: z.boolean(), + totalBytes: z.number().int().nonnegative(), + }), + timedOut: z.boolean(), + cancelled: z.boolean(), + executionId: z.string().min(1).max(128), +}); + +export type WorkspaceCommandResult = z.infer< + typeof WorkspaceCommandResultSchema +>; + +export const CancelWorkspaceCommandInputSchema = z.object({ + executionId: z + .string() + .min(1) + .max(128) + .describe("The executionId returned or assigned to a running command."), +}); + +export type CancelWorkspaceCommandInput = z.infer< + typeof CancelWorkspaceCommandInputSchema +>; + +export const CancelWorkspaceCommandResultSchema = z.object({ + success: z.boolean(), + error: z.string().optional(), + cancelled: z.boolean(), + pendingCancellation: z.boolean(), + executionId: z.string().min(1).max(128), +}); diff --git a/ts/packages/commandExecutor/test/workspaceCommandMcpSchema.spec.ts b/ts/packages/commandExecutor/test/workspaceCommandMcpSchema.spec.ts new file mode 100644 index 0000000000..6678ce22aa --- /dev/null +++ b/ts/packages/commandExecutor/test/workspaceCommandMcpSchema.spec.ts @@ -0,0 +1,70 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { + CancelWorkspaceCommandInputSchema, + CancelWorkspaceCommandResultSchema, + WorkspaceCommandInputSchema, + WorkspaceCommandResultSchema, +} from "../src/workspaceCommandMcpSchema.js"; + +describe("workspace command MCP schemas", () => { + test("accepts a focused test command with a workspace-relative directory", () => { + expect( + WorkspaceCommandInputSchema.parse({ + command: "pnpm test -- --runInBand", + workspaceFolder: "TypeAgent", + workingDirectory: "ts/packages/coda", + timeoutMs: 120_000, + executionId: "coda-tests", + }), + ).toEqual({ + command: "pnpm test -- --runInBand", + workspaceFolder: "TypeAgent", + workingDirectory: "ts/packages/coda", + timeoutMs: 120_000, + executionId: "coda-tests", + }); + }); + + test("rejects an invalid timeout, oversized UTF-8 command, and empty execution ID", () => { + expect(() => + WorkspaceCommandInputSchema.parse({ + command: "pnpm test", + timeoutMs: 300_001, + }), + ).toThrow(); + expect(() => + WorkspaceCommandInputSchema.parse({ + command: "😀".repeat(4_097), + executionId: "coda-tests", + }), + ).toThrow(); + expect(() => + CancelWorkspaceCommandInputSchema.parse({ executionId: "" }), + ).toThrow(); + }); + + test("keeps success, failure, and cancellation result contracts distinct", () => { + expect( + WorkspaceCommandResultSchema.parse({ + success: false, + exitCode: 1, + durationMs: 25, + stdout: { text: "", truncated: false, totalBytes: 0 }, + stderr: { text: "failed", truncated: false, totalBytes: 6 }, + timedOut: false, + cancelled: false, + executionId: "coda-tests", + }), + ).toMatchObject({ success: false, exitCode: 1 }); + expect( + CancelWorkspaceCommandResultSchema.parse({ + success: true, + cancelled: true, + pendingCancellation: false, + executionId: "coda-tests", + }), + ).toMatchObject({ cancelled: true }); + }); +}); From 2d7cf403380fafb0e3f4db6a011f9d5991bf1a64 Mon Sep 17 00:00:00 2001 From: typeagent-bot Date: Fri, 4 Sep 2026 00:13:02 +0000 Subject: [PATCH 2/8] style: apply prettier formatting and policy fixes --- ts/packages/coda/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ts/packages/coda/package.json b/ts/packages/coda/package.json index 3ff1d38b1e..a8aa40fef3 100644 --- a/ts/packages/coda/package.json +++ b/ts/packages/coda/package.json @@ -30,8 +30,8 @@ "prettier:fix": "prettier --write . --ignore-path ../../.prettierignore", "pretest": "pnpm run build", "test-compile": "pnpm --dir ../agents/code exec tsc -p ../../coda/src", - "test:local": "pnpm --dir ../agents/code exec tsc --target ES2022 --module Node16 --moduleResolution Node16 --esModuleInterop --strict --types node --outDir ../../coda/dist-test --rootDir ../../coda/src ../../coda/src/workspaceCommandRunner.ts ../../coda/src/workspaceCommandPolicy.ts ../../coda/src/test/workspaceCommandRunner.spec.ts ../../coda/src/test/workspaceCommandPolicy.spec.ts && node --test dist-test/test/workspaceCommandRunner.spec.js dist-test/test/workspaceCommandPolicy.spec.js", "test:full": "vscode-test", + "test:local": "pnpm --dir ../agents/code exec tsc --target ES2022 --module Node16 --moduleResolution Node16 --esModuleInterop --strict --types node --outDir ../../coda/dist-test --rootDir ../../coda/src ../../coda/src/workspaceCommandRunner.ts ../../coda/src/workspaceCommandPolicy.ts ../../coda/src/test/workspaceCommandRunner.spec.ts ../../coda/src/test/workspaceCommandPolicy.spec.ts && node --test dist-test/test/workspaceCommandRunner.spec.js dist-test/test/workspaceCommandPolicy.spec.js", "vscode:prepublish": "pnpm run esbuild-base --minify", "watch": "tsc -w" }, From 04c487aadfd86af779bfd5e668ca58b55d1f2187 Mon Sep 17 00:00:00 2001 From: George Ng Date: Thu, 3 Sep 2026 23:05:34 -0700 Subject: [PATCH 3/8] Reduce command action complexity Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../coda/src/handleWorkBenchActions.ts | 284 ++++++++++-------- .../commandExecutor/src/commandServer.ts | 141 +++++---- 2 files changed, 254 insertions(+), 171 deletions(-) diff --git a/ts/packages/coda/src/handleWorkBenchActions.ts b/ts/packages/coda/src/handleWorkBenchActions.ts index 726f4c8bff..8d5c621b9f 100644 --- a/ts/packages/coda/src/handleWorkBenchActions.ts +++ b/ts/packages/coda/src/handleWorkBenchActions.ts @@ -66,67 +66,57 @@ function workspaceCommandError( return workspaceCommandResponse({ success: false, error, executionId }); } -async function resolveWorkspaceCommandDirectory( +function validateWorkspaceCommandPaths( parameters: WorkspaceCommandParameters, -): Promise { - const workspaceFolders = vscode.workspace.workspaceFolders; - if (!workspaceFolders || workspaceFolders.length === 0) { - return { error: "No workspace or repository is currently open." }; - } - if ( - parameters.workspaceFolder !== undefined && - (typeof parameters.workspaceFolder !== "string" || - parameters.workspaceFolder.trim().length === 0) - ) { - return { error: "workspaceFolder must be a non-empty string." }; - } - if ( - parameters.workingDirectory !== undefined && - (typeof parameters.workingDirectory !== "string" || - parameters.workingDirectory.trim().length === 0) - ) { - return { error: "workingDirectory must be a non-empty string." }; +): string | undefined { + for (const parameter of ["workspaceFolder", "workingDirectory"] as const) { + const value = parameters[parameter]; + if ( + value !== undefined && + (typeof value !== "string" || value.trim().length === 0) + ) { + return `${parameter} must be a non-empty string.`; + } } + return undefined; +} - let workspaceFolder: vscode.WorkspaceFolder | undefined; - if (typeof parameters.workspaceFolder === "string") { - const requested = parameters.workspaceFolder; - const requestedPath = path.resolve(requested); - workspaceFolder = workspaceFolders.find( +function selectWorkspaceFolder( + workspaceFolders: readonly vscode.WorkspaceFolder[], + requestedWorkspaceFolder: string | undefined, +): vscode.WorkspaceFolder | { error: string } { + if (requestedWorkspaceFolder !== undefined) { + const requestedPath = path.resolve(requestedWorkspaceFolder); + const workspaceFolder = workspaceFolders.find( (folder) => - folder.name === requested || + folder.name === requestedWorkspaceFolder || path.resolve(folder.uri.fsPath) === requestedPath, ); - if (workspaceFolder === undefined) { - return { - error: `No open workspace root matches '${requested}'.`, - }; - } - } else { - const activeUri = vscode.window.activeTextEditor?.document.uri; - workspaceFolder = activeUri - ? vscode.workspace.getWorkspaceFolder(activeUri) - : undefined; - if (workspaceFolder === undefined && workspaceFolders.length === 1) { - workspaceFolder = workspaceFolders[0]; - } - if (workspaceFolder === undefined) { - return { - error: "Multiple workspace roots are open. Specify workspaceFolder by name or absolute path.", - }; - } + return ( + workspaceFolder ?? { + error: `No open workspace root matches '${requestedWorkspaceFolder}'.`, + } + ); } - if (workspaceFolder.uri.scheme !== "file") { - return { - error: `Workspace root '${workspaceFolder.name}' is not a local filesystem folder.`, - }; + const activeUri = vscode.window.activeTextEditor?.document.uri; + const activeWorkspaceFolder = activeUri + ? vscode.workspace.getWorkspaceFolder(activeUri) + : undefined; + if (activeWorkspaceFolder !== undefined) { + return activeWorkspaceFolder; + } + if (workspaceFolders.length === 1) { + return workspaceFolders[0]; } + return { + error: "Multiple workspace roots are open. Specify workspaceFolder by name or absolute path.", + }; +} - const workingDirectory = - typeof parameters.workingDirectory === "string" - ? parameters.workingDirectory - : undefined; - const root = workspaceFolder.uri.fsPath; +function resolveWorkspaceChildDirectory( + root: string, + workingDirectory: string | undefined, +): string | { error: string } { const cwd = workingDirectory !== undefined ? path.resolve(root, workingDirectory) @@ -142,9 +132,14 @@ async function resolveWorkspaceCommandDirectory( error: "workingDirectory must stay within the selected workspace root.", }; } + return cwd; +} + +async function verifyWorkspaceDirectory( + cwd: string, +): Promise { try { - const stat = await fs.stat(cwd); - if (!stat.isDirectory()) { + if (!(await fs.stat(cwd)).isDirectory()) { return { error: `workingDirectory is not a directory: ${cwd}` }; } } catch (error) { @@ -157,6 +152,39 @@ async function resolveWorkspaceCommandDirectory( return cwd; } +async function resolveWorkspaceCommandDirectory( + parameters: WorkspaceCommandParameters, +): Promise { + const workspaceFolders = vscode.workspace.workspaceFolders; + if (!workspaceFolders || workspaceFolders.length === 0) { + return { error: "No workspace or repository is currently open." }; + } + const pathError = validateWorkspaceCommandPaths(parameters); + if (pathError !== undefined) { + return { error: pathError }; + } + const workspaceFolder = selectWorkspaceFolder( + workspaceFolders, + parameters.workspaceFolder as string | undefined, + ); + if ("error" in workspaceFolder) { + return workspaceFolder; + } + if (workspaceFolder.uri.scheme !== "file") { + return { + error: `Workspace root '${workspaceFolder.name}' is not a local filesystem folder.`, + }; + } + + const workingDirectory = + typeof parameters.workingDirectory === "string" + ? parameters.workingDirectory + : undefined; + const root = workspaceFolder.uri.fsPath; + const cwd = resolveWorkspaceChildDirectory(root, workingDirectory); + return typeof cwd === "string" ? verifyWorkspaceDirectory(cwd) : cwd; +} + export async function handleRunWorkspaceCommand(action: { parameters?: WorkspaceCommandParameters; }): Promise { @@ -582,6 +610,76 @@ async function resolveCommandToExecute( return { resolvedCommand }; } +async function resolveTerminalDirectory( + folderName: string | undefined, +): Promise { + if (!folderName) { + return vscode.workspace.workspaceFolders?.[0]?.uri.fsPath; + } + const matches = await findMatchingFolders(path.basename(folderName)); + if (matches.length === 0) { + const msg = `❌ No folders found matching '${folderName}'.`; + vscode.window.showErrorMessage(msg); + return { handled: false, message: msg }; + } + const targetFolder = + matches.length === 1 + ? matches[0] + : ( + await vscode.window.showQuickPick( + matches.map((uri) => ({ + label: vscode.workspace.asRelativePath(uri), + uri, + })), + { + placeHolder: `Multiple folders found. Select where to open the terminal:`, + }, + ) + )?.uri; + if (!targetFolder) { + const msg = "⚠️ Terminal opening cancelled by user."; + vscode.window.showInformationMessage(msg); + return { handled: false, message: msg }; + } + return targetFolder.fsPath; +} + +async function executeVsCodeTerminalCommand( + resolvedCommand: string | undefined, +): Promise { + if ( + !resolvedCommand || + resolvedCommand.includes(" ") || + !resolvedCommand.includes(".") + ) { + return undefined; + } + try { + await vscode.commands.executeCommand(resolvedCommand); + const msg = `✅ Executed VSCode command: ${resolvedCommand}.`; + vscode.window.showInformationMessage(msg); + return { handled: true, message: msg }; + } catch (err) { + const msg = `❌ Failed to execute VSCode command: ${resolvedCommand}. ${err}`; + vscode.window.showErrorMessage(msg); + return { handled: false, message: msg }; + } +} + +function createOrReuseTerminal( + reuseExistingTerminal: boolean, + cwd: string | undefined, + folderName: string | undefined, +): vscode.Terminal { + if (reuseExistingTerminal && vscode.window.activeTerminal) { + return vscode.window.activeTerminal; + } + const name = folderName ? `Terminal: ${folderName}` : "Terminal"; + return cwd + ? vscode.window.createTerminal({ name, cwd: vscode.Uri.file(cwd) }) + : vscode.window.createTerminal(name); +} + export async function handleOpenInIntegratedTerminal( action: any, ): Promise { @@ -593,44 +691,11 @@ export async function handleOpenInIntegratedTerminal( const reuseExistingTerminal = parameters.reuseExistingTerminal ?? true; await aliasManager.ready; - let cwd: string | undefined; - - if (folderName) { - const matches = await findMatchingFolders(path.basename(folderName)); - if (matches.length === 0) { - const msg = `❌ No folders found matching '${folderName}'.`; - vscode.window.showErrorMessage(msg); - return { handled: false, message: msg }; - } - - const targetFolder = - matches.length === 1 - ? matches[0] - : ( - await vscode.window.showQuickPick( - matches.map((uri) => ({ - label: vscode.workspace.asRelativePath(uri), - uri, - })), - { - placeHolder: `Multiple folders found. Select where to open the terminal:`, - }, - ) - )?.uri; - - if (!targetFolder) { - const msg = "⚠️ Terminal opening cancelled by user."; - vscode.window.showInformationMessage(msg); - return { handled: false, message: msg }; - } - - cwd = targetFolder.fsPath; - } else { - const workspaceFolders = vscode.workspace.workspaceFolders; - if (workspaceFolders && workspaceFolders.length > 0) { - cwd = workspaceFolders[0].uri.fsPath; - } + const directoryResult = await resolveTerminalDirectory(folderName); + if (directoryResult && typeof directoryResult !== "string") { + return directoryResult; } + const cwd = directoryResult; let resolvedCommand: string | undefined; if (commandToExecute) { @@ -646,39 +711,20 @@ export async function handleOpenInIntegratedTerminal( resolvedCommand = cmd; } - // If the resolved command is a VSCode command, execute it directly - if ( - resolvedCommand && - !resolvedCommand.includes(" ") && - resolvedCommand.includes(".") - ) { - try { - await vscode.commands.executeCommand(resolvedCommand); - const msg = `✅ Executed VSCode command: ${resolvedCommand}.`; - vscode.window.showInformationMessage(msg); - return { handled: true, message: msg }; - } catch (err) { - const msg = `❌ Failed to execute VSCode command: ${resolvedCommand}. ${err}`; - vscode.window.showErrorMessage(msg); - return { handled: false, message: msg }; - } + const vsCodeCommandResult = + await executeVsCodeTerminalCommand(resolvedCommand); + if (vsCodeCommandResult !== undefined) { + return vsCodeCommandResult; } // Otherwise, open the terminal and send the command let terminal: vscode.Terminal; try { - if (reuseExistingTerminal && vscode.window.activeTerminal) { - terminal = vscode.window.activeTerminal; - } else { - terminal = cwd - ? vscode.window.createTerminal({ - name: folderName ? `Terminal: ${folderName}` : `Terminal`, - cwd: vscode.Uri.file(cwd), - }) - : vscode.window.createTerminal( - folderName ? `Terminal: ${folderName}` : `Terminal`, - ); - } + terminal = createOrReuseTerminal( + reuseExistingTerminal, + cwd, + folderName, + ); terminal.show(); if (resolvedCommand) { diff --git a/ts/packages/commandExecutor/src/commandServer.ts b/ts/packages/commandExecutor/src/commandServer.ts index 49685b2f3c..04fb1512e6 100644 --- a/ts/packages/commandExecutor/src/commandServer.ts +++ b/ts/packages/commandExecutor/src/commandServer.ts @@ -191,6 +191,93 @@ async function processHtmlContent(content: string): Promise { return htmlToPlainText(content); } +function remapWebflowAction(request: ExecuteActionRequest): { + schemaName: string; + actionName: string; + parameters: Record | undefined; +} { + if ( + request.schemaName !== "webflow" || + !["run_draft", "list", "execute"].includes(request.actionName) + ) { + return { + schemaName: request.schemaName, + actionName: request.actionName, + parameters: request.parameters, + }; + } + + const parameters = request.parameters; + if (request.actionName === "run_draft") { + const p = parameters as + | { + script?: unknown; + params?: unknown; + parameters?: unknown; + timeout?: unknown; + } + | undefined; + const mappedParameters: Record = { + script: p?.script, + }; + if (p?.params !== undefined) { + mappedParameters.params = + typeof p.params === "string" + ? p.params + : JSON.stringify(p.params); + } + if (p?.parameters !== undefined) { + mappedParameters.params = + typeof p.parameters === "string" + ? p.parameters + : JSON.stringify(p.parameters); + } + if (p?.timeout !== undefined) { + mappedParameters.timeout = p.timeout; + } + return { + schemaName: "browser", + actionName: "executeAdHocScript", + parameters: mappedParameters, + }; + } + if (request.actionName === "list") { + const domain = (parameters as { domain?: unknown } | undefined)?.domain; + return domain + ? { + schemaName: "browser", + actionName: "getWebFlowsForDomain", + parameters: { domain }, + } + : { + schemaName: "browser", + actionName: "getAllWebFlows", + parameters: {}, + }; + } + + const p = parameters as + | { flowName?: unknown; parameters?: unknown } + | undefined; + let flowParams = p?.parameters; + if (typeof flowParams === "string") { + try { + flowParams = JSON.parse(flowParams); + } catch { + flowParams = {}; + } + } + return { + schemaName: "browser.webFlows", + actionName: + typeof p?.flowName === "string" ? p.flowName : request.actionName, + parameters: + flowParams && typeof flowParams === "object" + ? (flowParams as Record) + : {}, + }; +} + // ── Logger ──────────────────────────────────────────────────────────────────── class Logger { @@ -1296,58 +1383,8 @@ export class CommandServer { ); } - // Remap webflow schema calls to browser agent actions - let schemaName = request.schemaName; - let actionName = request.actionName; - let parameters = request.parameters; - - if (schemaName === "webflow" && actionName === "run_draft") { - schemaName = "browser"; - actionName = "executeAdHocScript"; - const p = parameters as any; - parameters = { - script: p?.script, - ...(p?.params && { - params: - typeof p.params === "string" - ? p.params - : JSON.stringify(p.params), - }), - ...(p?.parameters && { - params: - typeof p.parameters === "string" - ? p.parameters - : JSON.stringify(p.parameters), - }), - ...(p?.timeout && { timeout: p.timeout }), - }; - } else if (schemaName === "webflow" && actionName === "list") { - // Route to discovery handler which returns webflows - const p = parameters as any; - if (p?.domain) { - schemaName = "browser"; - actionName = "getWebFlowsForDomain"; - parameters = { domain: p.domain }; - } else { - schemaName = "browser"; - actionName = "getAllWebFlows"; - parameters = {}; - } - } else if (schemaName === "webflow" && actionName === "execute") { - const p = parameters as any; - const flowName = p?.flowName; - let flowParams = p?.parameters; - if (typeof flowParams === "string") { - try { - flowParams = JSON.parse(flowParams); - } catch { - flowParams = {}; - } - } - schemaName = "browser.webFlows"; - actionName = flowName || actionName; - parameters = flowParams || {}; - } + const { schemaName, actionName, parameters } = + remapWebflowAction(request); const paramStr = parameters && Object.keys(parameters).length > 0 From e768f3b9837ce71eab591f453df191632091367c Mon Sep 17 00:00:00 2001 From: George Ng Date: Fri, 4 Sep 2026 15:31:01 -0700 Subject: [PATCH 4/8] Address structured command review feedback Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../agents/code/src/codeActionHandler.ts | 101 ++++++++++++- .../code/src/codeAgentWebSocketServer.ts | 28 ++-- ...orkbenchCommandActionsSchema.keywords.json | 25 +++- ts/packages/coda/.vscodeignore | 1 + ts/packages/coda/package.json | 8 +- .../coda/src/handleWorkBenchActions.ts | 36 +++-- .../src/test/workspaceCommandPolicy.spec.ts | 73 ++++++++++ .../coda/src/workspaceCommandPolicy.ts | 135 ++++++++++++++++-- .../coda/src/workspaceCommandRunner.ts | 19 ++- ts/pnpm-lock.yaml | 6 + 10 files changed, 391 insertions(+), 41 deletions(-) diff --git a/ts/packages/agents/code/src/codeActionHandler.ts b/ts/packages/agents/code/src/codeActionHandler.ts index 8e37a4ce5f..de491d723d 100644 --- a/ts/packages/agents/code/src/codeActionHandler.ts +++ b/ts/packages/agents/code/src/codeActionHandler.ts @@ -69,14 +69,66 @@ const sharedPendingCalls: Map< > = new Map(); const cancellationControlCalls = new Map< number, - { clientId: string; responseId: unknown } + { + clientId: string; + targetClientId: string; + responseId: unknown; + executionId: string; + timeout: NodeJS.Timeout; + } >(); +const CANCELLATION_CONTROL_TIMEOUT_MS = 5_000; // Global call-id counter. The pending-calls map is module-scoped (one // websocket server is shared across all sessions), so the id space must // also be global — per-session counters would collide on 0,1,2,... and // route a response to the wrong session's pending call. let nextSharedCallId = 0; +function deleteCancellationControlCall(callId: number) { + const call = cancellationControlCalls.get(callId); + if (call !== undefined) { + clearTimeout(call.timeout); + cancellationControlCalls.delete(callId); + } + return call; +} + +function clearCancellationControlCalls(clientId?: string): void { + for (const [callId, call] of cancellationControlCalls) { + if ( + clientId === undefined || + call.clientId === clientId || + call.targetClientId === clientId + ) { + deleteCancellationControlCall(callId); + } + } +} + +function sendCancellationControlFailure( + server: CodeAgentWebSocketServer, + call: { + clientId: string; + responseId: unknown; + executionId: string; + }, + error: string, +): void { + server.sendToClient( + call.clientId, + JSON.stringify({ + id: call.responseId, + result: JSON.stringify({ + success: false, + error, + cancelled: false, + pendingCancellation: false, + executionId: call.executionId, + }), + }), + ); +} + export function displayCodaResult(result: unknown): DisplayContent { const message = typeof result === "string" @@ -302,9 +354,23 @@ function attachSharedOnMessage(server: CodeAgentWebSocketServer): void { return; } const callId = nextSharedCallId++; + const timeout = setTimeout(() => { + const call = deleteCancellationControlCall(callId); + if (call !== undefined) { + sendCancellationControlFailure( + server, + call, + "The Coda workspace did not respond to the cancellation request.", + ); + } + }, CANCELLATION_CONTROL_TIMEOUT_MS); + timeout.unref(); cancellationControlCalls.set(callId, { clientId, + targetClientId, responseId: data.id, + executionId: data.params.executionId, + timeout, }); if ( !server.sendToClient( @@ -319,17 +385,23 @@ function attachSharedOnMessage(server: CodeAgentWebSocketServer): void { }), ) ) { - cancellationControlCalls.delete(callId); + const call = deleteCancellationControlCall(callId); + if (call !== undefined) { + sendCancellationControlFailure( + server, + call, + "The Coda workspace disconnected before the cancellation request could be delivered.", + ); + } } return; } if (data.id !== undefined && data.result !== undefined) { - const controlCall = cancellationControlCalls.get( + const controlCall = deleteCancellationControlCall( Number(data.id), ); if (controlCall !== undefined) { - cancellationControlCalls.delete(Number(data.id)); server.sendToClient( controlCall.clientId, JSON.stringify({ @@ -386,6 +458,26 @@ function attachSharedOnMessage(server: CodeAgentWebSocketServer): void { void sc.notifyReadinessChanged(); } }; + server.onClientDisconnected = (clientId: string) => { + for (const [callId, call] of cancellationControlCalls) { + if ( + call.clientId === clientId || + call.targetClientId === clientId + ) { + deleteCancellationControlCall(callId); + if ( + call.targetClientId === clientId && + call.clientId !== clientId + ) { + sendCancellationControlFailure( + server, + call, + "The Coda workspace disconnected before responding to the cancellation request.", + ); + } + } + } + }; } // Start (or attach to an in-flight start of) the shared WebSocket server. @@ -488,6 +580,7 @@ async function updateCodeContext( const server = sharedWebSocketServer; sharedWebSocketServer = undefined; sharedPendingCalls.clear(); + clearCancellationControlCalls(); // Track the in-flight close so a rapid re-enable awaits // port release under a fixed-port override. sharedClosingPromise = server.close().finally(() => { diff --git a/ts/packages/agents/code/src/codeAgentWebSocketServer.ts b/ts/packages/agents/code/src/codeAgentWebSocketServer.ts index d8a63b0863..c213dd345a 100644 --- a/ts/packages/agents/code/src/codeAgentWebSocketServer.ts +++ b/ts/packages/agents/code/src/codeAgentWebSocketServer.ts @@ -16,6 +16,7 @@ export class CodeAgentWebSocketServer { private clientIdCounter = 0; private readonly stopHeartbeat: () => void; public onMessage?: (message: string, clientId: string) => void; + public onClientDisconnected?: (clientId: string) => void; /** * Fired after the {@link clients} map mutation completes for any * connect / disconnect, with the post-mutation total. Used by the @@ -145,21 +146,24 @@ export class CodeAgentWebSocketServer { ws.on("close", () => { debug("Client disconnected"); - this.clients.delete(clientId); - this.controlClientIds.delete(clientId); - this.onClientCountChanged?.(this.getConnectedCount()); + this.removeClient(clientId); }); ws.on("error", (error) => { debug("Client error:", error); - if (this.clients.delete(clientId)) { - this.controlClientIds.delete(clientId); - this.onClientCountChanged?.(this.getConnectedCount()); - } + this.removeClient(clientId); }); }); } + private removeClient(clientId: string): void { + if (this.clients.delete(clientId)) { + this.controlClientIds.delete(clientId); + this.onClientDisconnected?.(clientId); + this.onClientCountChanged?.(this.getConnectedCount()); + } + } + public broadcast(message: string): number { let successCount = 0; const clientsToRemove: string[] = []; @@ -183,8 +187,7 @@ export class CodeAgentWebSocketServer { // Remove failed clients clientsToRemove.forEach((clientId) => { - this.clients.delete(clientId); - this.controlClientIds.delete(clientId); + this.removeClient(clientId); }); return successCount; @@ -220,9 +223,7 @@ export class CodeAgentWebSocketServer { return true; } catch (error) { debug("Failed to send to client:", error); - this.clients.delete(clientId); - this.controlClientIds.delete(clientId); - this.onClientCountChanged?.(this.getConnectedCount()); + this.removeClient(clientId); return false; } } @@ -282,10 +283,11 @@ export class CodeAgentWebSocketServer { public close(): Promise { debug("Closing CodeAgentWebSocketServer"); this.stopHeartbeat(); - for (const [, client] of this.clients.entries()) { + for (const [clientId, client] of this.clients.entries()) { if (client.readyState === WebSocket.OPEN) { client.close(); } + this.onClientDisconnected?.(clientId); } this.clients.clear(); this.controlClientIds.clear(); diff --git a/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.keywords.json b/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.keywords.json index 04fd69727c..2981e59203 100644 --- a/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.keywords.json +++ b/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.keywords.json @@ -2,8 +2,8 @@ "schemaVersion": 1, "schema": "code.code-workbench", "generatedBy": "llm", - "generatedAt": "2026-07-08T09:37:29.694Z", - "sourceHash": "+dVRLLm3/u9G1X4GL1PBki8JTEOlFEBeNqpOTZQo3K8=", + "generatedAt": "2026-09-04T21:47:59.558Z", + "sourceHash": "gr8T/xLRyyJ5HzjdEpeh4Gh3zn8E6CGvT4N4aUjiQbc=", "actions": { "workbenchOpenFile": [ "file", @@ -113,6 +113,27 @@ "programming", "software", "coding" + ], + "runWorkspaceCommand": [ + "workspace", + "command", + "test", + "build", + "lint", + "diagnostic", + "execute", + "output", + "terminal", + "timeout", + "directory" + ], + "cancelWorkspaceCommand": [ + "cancel", + "stop", + "terminate", + "workspace", + "command", + "execution" ] } } diff --git a/ts/packages/coda/.vscodeignore b/ts/packages/coda/.vscodeignore index 72aa0fe2e7..7344c6b496 100644 --- a/ts/packages/coda/.vscodeignore +++ b/ts/packages/coda/.vscodeignore @@ -1,5 +1,6 @@ .vscode/** .vscode-test/** +dist-test/** src/** .gitignore .yarnrc diff --git a/ts/packages/coda/package.json b/ts/packages/coda/package.json index a8aa40fef3..1011effde7 100644 --- a/ts/packages/coda/package.json +++ b/ts/packages/coda/package.json @@ -29,9 +29,9 @@ "prettier": "prettier --check . --ignore-path ../../.prettierignore", "prettier:fix": "prettier --write . --ignore-path ../../.prettierignore", "pretest": "pnpm run build", - "test-compile": "pnpm --dir ../agents/code exec tsc -p ../../coda/src", + "test-compile": "tsc -p src", "test:full": "vscode-test", - "test:local": "pnpm --dir ../agents/code exec tsc --target ES2022 --module Node16 --moduleResolution Node16 --esModuleInterop --strict --types node --outDir ../../coda/dist-test --rootDir ../../coda/src ../../coda/src/workspaceCommandRunner.ts ../../coda/src/workspaceCommandPolicy.ts ../../coda/src/test/workspaceCommandRunner.spec.ts ../../coda/src/test/workspaceCommandPolicy.spec.ts && node --test dist-test/test/workspaceCommandRunner.spec.js dist-test/test/workspaceCommandPolicy.spec.js", + "test:local": "tsc --target ES2022 --module Node16 --moduleResolution Node16 --esModuleInterop --strict --types node --outDir dist-test --rootDir src src/workspaceCommandRunner.ts src/workspaceCommandPolicy.ts src/test/workspaceCommandRunner.spec.ts src/test/workspaceCommandPolicy.spec.ts && node --test dist-test/test/workspaceCommandRunner.spec.js dist-test/test/workspaceCommandPolicy.spec.js", "vscode:prepublish": "pnpm run esbuild-base --minify", "watch": "tsc -w" }, @@ -68,6 +68,7 @@ "@types/debug": "^4.1.12", "@types/express": "^4.17.17", "@types/mocha": "^10.0.6", + "@types/node": "^22.0.0", "@types/vscode": "^1.88.0", "@types/ws": "^8.5.10", "@vscode/test-cli": "^0.0.12", @@ -76,7 +77,8 @@ "esbuild": "^0.28.2", "mkdirp": "^3.0.1", "prettier": "^3.5.3", - "rimraf": "^6.0.1" + "rimraf": "^6.0.1", + "typescript": "~5.4.5" }, "engines": { "vscode": "^1.88.0" diff --git a/ts/packages/coda/src/handleWorkBenchActions.ts b/ts/packages/coda/src/handleWorkBenchActions.ts index 8d5c621b9f..fdfb37bc94 100644 --- a/ts/packages/coda/src/handleWorkBenchActions.ts +++ b/ts/packages/coda/src/handleWorkBenchActions.ts @@ -137,11 +137,27 @@ function resolveWorkspaceChildDirectory( async function verifyWorkspaceDirectory( cwd: string, -): Promise { + workspaceRoot: string, +): Promise<{ cwd: string; workspaceRoot: string } | { error: string }> { try { if (!(await fs.stat(cwd)).isDirectory()) { return { error: `workingDirectory is not a directory: ${cwd}` }; } + const [realCwd, realWorkspaceRoot] = await Promise.all([ + fs.realpath(cwd), + fs.realpath(workspaceRoot), + ]); + const relativePath = path.relative(realWorkspaceRoot, realCwd); + if ( + relativePath === ".." || + relativePath.startsWith(`..${path.sep}`) || + path.isAbsolute(relativePath) + ) { + return { + error: "workingDirectory must stay within the selected workspace root.", + }; + } + return { cwd: realCwd, workspaceRoot: realWorkspaceRoot }; } catch (error) { return { error: `workingDirectory does not exist: ${ @@ -149,12 +165,11 @@ async function verifyWorkspaceDirectory( }`, }; } - return cwd; } async function resolveWorkspaceCommandDirectory( parameters: WorkspaceCommandParameters, -): Promise { +): Promise<{ cwd: string; workspaceRoot: string } | { error: string }> { const workspaceFolders = vscode.workspace.workspaceFolders; if (!workspaceFolders || workspaceFolders.length === 0) { return { error: "No workspace or repository is currently open." }; @@ -182,7 +197,10 @@ async function resolveWorkspaceCommandDirectory( : undefined; const root = workspaceFolder.uri.fsPath; const cwd = resolveWorkspaceChildDirectory(root, workingDirectory); - return typeof cwd === "string" ? verifyWorkspaceDirectory(cwd) : cwd; + if (typeof cwd !== "string") { + return cwd; + } + return verifyWorkspaceDirectory(cwd, root); } export async function handleRunWorkspaceCommand(action: { @@ -246,9 +264,9 @@ export async function handleRunWorkspaceCommand(action: { executionId, ); } - const cwd = await resolveWorkspaceCommandDirectory(parameters); - if (typeof cwd !== "string") { - return workspaceCommandError(cwd.error, executionId); + const directory = await resolveWorkspaceCommandDirectory(parameters); + if ("error" in directory) { + return workspaceCommandError(directory.error, executionId); } if (declaredRiskLevel === "high") { @@ -259,13 +277,15 @@ export async function handleRunWorkspaceCommand(action: { } const commandPolicyError = validateFocusedWorkspaceCommand( parameters.command, + directory.cwd, + directory.workspaceRoot, ); if (commandPolicyError !== undefined) { return workspaceCommandError(commandPolicyError, executionId); } const result = await workspaceCommandRunner.run({ command: parameters.command, - cwd, + cwd: directory.cwd, ...(parameters.timeoutMs === undefined ? {} : { timeoutMs: parameters.timeoutMs }), diff --git a/ts/packages/coda/src/test/workspaceCommandPolicy.spec.ts b/ts/packages/coda/src/test/workspaceCommandPolicy.spec.ts index 6dab67b7a5..a493e5cdfc 100644 --- a/ts/packages/coda/src/test/workspaceCommandPolicy.spec.ts +++ b/ts/packages/coda/src/test/workspaceCommandPolicy.spec.ts @@ -2,9 +2,15 @@ // Licensed under the MIT License. import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import test from "node:test"; import { validateFocusedWorkspaceCommand } from "../workspaceCommandPolicy"; +const workspaceRoot = path.resolve(__dirname, "../../../.."); +const workspaceCwd = path.join(workspaceRoot, "packages", "coda"); + test("allows focused test and build commands", () => { assert.equal( validateFocusedWorkspaceCommand("pnpm test -- --runInBand"), @@ -20,6 +26,70 @@ test("allows focused test and build commands", () => { ); }); +test("rejects path arguments outside the workspace root", () => { + for (const command of [ + "npm test --prefix ../../..", + "jest --config ../../../config.js", + "pytest -c../../../outside.ini", + "tsc -p../../..", + "pytest ../../../outside=test.py", + `tsc --project "${path.resolve( + workspaceRoot, + "..", + "outside", + "tsconfig.json", + )}"`, + ]) { + assert.match( + validateFocusedWorkspaceCommand( + command, + workspaceCwd, + workspaceRoot, + ) ?? "", + /must stay within/, + ); + } +}); + +test("allows path arguments that remain inside the workspace root", () => { + assert.equal( + validateFocusedWorkspaceCommand( + "vitest run ./src/example.spec.ts", + workspaceCwd, + workspaceRoot, + ), + undefined, + ); + assert.equal( + validateFocusedWorkspaceCommand( + "tsc --project ../../tsconfig.json", + workspaceCwd, + workspaceRoot, + ), + undefined, + ); +}); + +test("rejects a bare symlink argument that resolves outside the workspace", () => { + const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "coda-policy-")); + try { + const root = path.join(tempRoot, "workspace"); + const cwd = path.join(root, "package"); + const outside = path.join(tempRoot, "outside"); + fs.mkdirSync(cwd, { recursive: true }); + fs.mkdirSync(outside); + fs.symlinkSync(outside, path.join(cwd, "outside-link"), "junction"); + + assert.match( + validateFocusedWorkspaceCommand("pytest outside-link", cwd, root) ?? + "", + /must stay within/, + ); + } finally { + fs.rmSync(tempRoot, { recursive: true, force: true }); + } +}); + test("rejects shell composition and non-focused commands", () => { for (const command of [ " rm -rf target", @@ -27,6 +97,9 @@ test("rejects shell composition and non-focused commands", () => { "sh -c 'rm -rf target'", "powershell Remove-Item -Recurse target", "pnpm test && rm -rf target", + "pytest ~/outside.py", + "pytest %USERPROFILE%\\outside.py", + "pytest *.py", "bun x cowsay", "bun run ./script.ts", ]) { diff --git a/ts/packages/coda/src/workspaceCommandPolicy.ts b/ts/packages/coda/src/workspaceCommandPolicy.ts index bdb60520bd..0fb427a90f 100644 --- a/ts/packages/coda/src/workspaceCommandPolicy.ts +++ b/ts/packages/coda/src/workspaceCommandPolicy.ts @@ -1,7 +1,11 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +import * as fs from "fs"; +import * as path from "path"; + const shellSyntax = /[;&|<>`$()\r\n^]/; +const shellExpansionSyntax = /[%!~*?[\]{}]/; const directTools = new Set([ "ava", @@ -34,6 +38,104 @@ const subcommandTools = new Map([ ["yarn", /^(?:run\s+(?:build|check|format|lint|test|typecheck)\b|test\b)/i], ]); +function tokenizeCommand(command: string): string[] | undefined { + const tokens: string[] = []; + let current = ""; + let quote: "'" | '"' | undefined; + for (const character of command) { + if (character === "'" || character === '"') { + if (quote === character) { + quote = undefined; + } else if (quote === undefined) { + quote = character; + } else { + current += character; + } + } else if (/\s/.test(character) && quote === undefined) { + if (current.length > 0) { + tokens.push(current); + current = ""; + } + } else { + current += character; + } + } + if (quote !== undefined) { + return undefined; + } + if (current.length > 0) { + tokens.push(current); + } + return tokens; +} + +function isOutsideRoot(root: string, candidate: string): boolean { + const relative = path.relative(root, candidate); + return ( + relative === ".." || + relative.startsWith(`..${path.sep}`) || + path.isAbsolute(relative) + ); +} + +function nearestExistingPath(candidate: string): string { + let current = candidate; + while (!fs.existsSync(current)) { + const parent = path.dirname(current); + if (parent === current) { + return current; + } + current = parent; + } + return current; +} + +function validatePathArguments( + tokens: string[], + cwd: string, + workspaceRoot: string, +): string | undefined { + const resolvedRoot = path.resolve(workspaceRoot); + const realRoot = fs.realpathSync.native(resolvedRoot); + for (const token of tokens.slice(1)) { + const equalsIndex = token.indexOf("="); + let value = + token.startsWith("-") && equalsIndex > 1 + ? token.slice(equalsIndex + 1) + : token; + if (equalsIndex < 0) { + const attachedShortOption = /^-[A-Za-z](.+)$/.exec(token); + if (attachedShortOption !== null) { + value = attachedShortOption[1]; + } + } + const resolved = path.resolve(cwd, value); + const isPath = + value === "." || + value === ".." || + path.isAbsolute(value) || + /^[A-Za-z]:[\\/]/.test(value) || + value.startsWith("\\\\") || + value.includes("/") || + value.includes("\\") || + fs.existsSync(resolved); + if (!isPath) { + continue; + } + + if (isOutsideRoot(resolvedRoot, resolved)) { + return "Command path arguments must stay within the selected workspace root."; + } + const realCandidate = fs.realpathSync.native( + nearestExistingPath(resolved), + ); + if (isOutsideRoot(realRoot, realCandidate)) { + return "Command path arguments must stay within the selected workspace root."; + } + } + return undefined; +} + /** * Commands are intentionally restricted to focused build, test, lint, and * diagnostic tools. Accepting general shell syntax would make safety depend on @@ -41,23 +143,36 @@ const subcommandTools = new Map([ */ export function validateFocusedWorkspaceCommand( command: string, + cwd?: string, + workspaceRoot?: string, ): string | undefined { const trimmed = command.trim(); if (shellSyntax.test(trimmed)) { return "Shell composition is not supported for structured workspace commands."; } - const match = /^([A-Za-z0-9_.-]+)(?:\s+(.*))?$/.exec(trimmed); - if (!match) { + if ( + shellExpansionSyntax.test(trimmed) || + (process.platform !== "win32" && trimmed.includes("\\")) + ) { + return "Shell path expansion is not supported for structured workspace commands."; + } + const tokens = tokenizeCommand(trimmed); + if (tokens === undefined) { + return "Command contains an unterminated quoted argument."; + } + const [executable, ...argumentTokens] = tokens; + if (executable === undefined || !/^[A-Za-z0-9_.-]+$/.test(executable)) { return "Command must begin with a supported focused build, test, lint, or diagnostic tool."; } - const [, executable, argumentsText = ""] = match; + const argumentsText = argumentTokens.join(" "); const normalizedExecutable = executable.toLowerCase(); - if (directTools.has(normalizedExecutable)) { - return undefined; - } - const subcommand = subcommandTools.get(normalizedExecutable); - if (subcommand?.test(argumentsText)) { - return undefined; + const isAllowed = + directTools.has(normalizedExecutable) || + subcommandTools.get(normalizedExecutable)?.test(argumentsText) === true; + if (!isAllowed) { + return "Only focused build, test, lint, and diagnostic commands are supported."; } - return "Only focused build, test, lint, and diagnostic commands are supported."; + return cwd !== undefined && workspaceRoot !== undefined + ? validatePathArguments(tokens, cwd, workspaceRoot) + : undefined; } diff --git a/ts/packages/coda/src/workspaceCommandRunner.ts b/ts/packages/coda/src/workspaceCommandRunner.ts index 5249d623eb..0314e8f62a 100644 --- a/ts/packages/coda/src/workspaceCommandRunner.ts +++ b/ts/packages/coda/src/workspaceCommandRunner.ts @@ -9,6 +9,7 @@ export const MAX_OUTPUT_BYTES = 64 * 1024; export const DEFAULT_TIMEOUT_MS = 2 * 60 * 1000; export const MAX_TIMEOUT_MS = 5 * 60 * 1000; const MAX_PENDING_CANCELLATIONS = 64; +const STOP_FALLBACK_MS = 5_000; export type WorkspaceCommandOutput = { text: string; @@ -191,6 +192,9 @@ export class WorkspaceCommandRunner { : ["-c", options.command]; return new Promise((resolve) => { + let settled = false; + let stopFallbackHandle: NodeJS.Timeout | undefined; + let timeoutHandle: NodeJS.Timeout; let child: ChildProcess; try { child = spawn(shell, shellArgs, { @@ -212,7 +216,14 @@ export class WorkspaceCommandRunner { } const finish = (exitCode: number | null) => { + if (settled) { + return; + } + settled = true; clearTimeout(timeoutHandle); + if (stopFallbackHandle !== undefined) { + clearTimeout(stopFallbackHandle); + } this.activeCommands.delete(executionId); resolve({ success: exitCode === 0 && !timedOut && !cancelled, @@ -247,8 +258,14 @@ export class WorkspaceCommandRunner { }`, ); }); + stopFallbackHandle = setTimeout(() => { + child.stdout?.destroy(); + child.stderr?.destroy(); + finish(child.exitCode); + }, STOP_FALLBACK_MS); + stopFallbackHandle.unref(); }; - const timeoutHandle = setTimeout(() => stop("timeout"), timeout); + timeoutHandle = setTimeout(() => stop("timeout"), timeout); this.activeCommands.set(executionId, { child, diff --git a/ts/pnpm-lock.yaml b/ts/pnpm-lock.yaml index 23f2e1807f..7bbda67fc0 100644 --- a/ts/pnpm-lock.yaml +++ b/ts/pnpm-lock.yaml @@ -4440,6 +4440,9 @@ importers: '@types/mocha': specifier: ^10.0.6 version: 10.0.10 + '@types/node': + specifier: ^22.0.0 + version: 22.15.18 '@types/vscode': specifier: ^1.88.0 version: 1.100.0 @@ -4467,6 +4470,9 @@ importers: rimraf: specifier: ^6.0.1 version: 6.0.1 + typescript: + specifier: ~5.4.5 + version: 5.4.5 packages/codeProcessor: dependencies: From 27d39c19f7389c67a5d7e2c217c0749e5c6ba066 Mon Sep 17 00:00:00 2001 From: George Ng Date: Fri, 4 Sep 2026 15:43:47 -0700 Subject: [PATCH 5/8] Fix workspace command error shape, action ordering, and executionId contract Scope the command-result failure shape to the two workspace-command actions. Every other code action failing in the websocket handler was answering with a stdout/stderr/exitCode payload, which displayCodaResult rendered as a fake command result. Only runWorkspaceCommand skips the await in the message loop. Making every action fire-and-forget let unrelated editor mutations interleave; cancellation only needs the long-running action to be non-blocking. Make executionId optional in the MCP input schema. It was required there while the action schema and the .agr grammar both omit it, so grammar-sourced actions failed validation. The Command Executor now resolves one up front and forwards it, so the result and any cancellation refer to the same command. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fb716021-4cea-41f8-9b92-36508a78285b --- .../coda/src/handleWorkBenchActions.ts | 13 +++++- ts/packages/coda/src/wsConnect.ts | 41 +++++++++++-------- .../commandExecutor/src/commandServer.ts | 21 +++++----- .../src/workspaceCommandMcpSchema.ts | 3 +- .../test/workspaceCommandMcpSchema.spec.ts | 15 +++++++ 5 files changed, 64 insertions(+), 29 deletions(-) diff --git a/ts/packages/coda/src/handleWorkBenchActions.ts b/ts/packages/coda/src/handleWorkBenchActions.ts index fdfb37bc94..cffed9f15d 100644 --- a/ts/packages/coda/src/handleWorkBenchActions.ts +++ b/ts/packages/coda/src/handleWorkBenchActions.ts @@ -59,13 +59,24 @@ function workspaceCommandResponse( }; } -function workspaceCommandError( +export function workspaceCommandError( error: string, executionId?: string, ): ActionResult { return workspaceCommandResponse({ success: false, error, executionId }); } +const workspaceCommandActionNames = new Set([ + "runWorkspaceCommand", + "cancelWorkspaceCommand", +]); + +// Only these actions answer with the structured command-result shape. Every +// other action keeps the plain ActionResult contract. +export function isWorkspaceCommandAction(actionName: string): boolean { + return workspaceCommandActionNames.has(actionName); +} + function validateWorkspaceCommandPaths( parameters: WorkspaceCommandParameters, ): string | undefined { diff --git a/ts/packages/coda/src/wsConnect.ts b/ts/packages/coda/src/wsConnect.ts index 27dc4903d2..c1f129e16a 100644 --- a/ts/packages/coda/src/wsConnect.ts +++ b/ts/packages/coda/src/wsConnect.ts @@ -4,7 +4,11 @@ import WebSocket from "ws"; import { createWebSocket, keepWebSocketAlive } from "./webSocket"; import { handleVSCodeActions } from "./handleVSCodeActions"; -import { cancelWorkspaceCommands } from "./handleWorkBenchActions"; +import { + cancelWorkspaceCommands, + isWorkspaceCommandAction, + workspaceCommandError, +} from "./handleWorkBenchActions"; type WebSocketMessageV2 = { id?: string; @@ -37,21 +41,20 @@ async function handleActionMessage(data: WebSocketMessageV2): Promise { ); } catch (error) { console.error("Error handling websocket action:", error); + const message = error instanceof Error ? error.message : String(error); + const executionId = data.params?.executionId; webSocket?.send( JSON.stringify({ id: data.id, - result: JSON.stringify({ - success: false, - error: - error instanceof Error ? error.message : String(error), - exitCode: null, - durationMs: 0, - stdout: { text: "", truncated: false, totalBytes: 0 }, - stderr: { text: "", truncated: false, totalBytes: 0 }, - timedOut: false, - cancelled: false, - executionId: data.params?.executionId, - }), + result: isWorkspaceCommandAction(actionName) + ? workspaceCommandError( + message, + typeof executionId === "string" && + executionId.length > 0 + ? executionId + : undefined, + ) + : { handled: false, message }, }), ); } @@ -108,9 +111,15 @@ async function ensureWebsocketConnected() { } if (data.method !== undefined && data.method.indexOf("/") > 0) { - // Do not await a long-running command here: cancellation and other - // requests must continue to reach the extension while it runs. - void handleActionMessage(data); + // Only a workspace command runs long enough to block the message + // loop, and it must not: cancellation has to reach the extension + // while it runs. Every other action stays awaited so action + // ordering is preserved. + if (data.method === "code/runWorkspaceCommand") { + void handleActionMessage(data); + } else { + await handleActionMessage(data); + } } console.log( `vscode extension websocket client received message: ${JSON.stringify(data, null, 2)}`, diff --git a/ts/packages/commandExecutor/src/commandServer.ts b/ts/packages/commandExecutor/src/commandServer.ts index 04fb1512e6..7e6fbf70bb 100644 --- a/ts/packages/commandExecutor/src/commandServer.ts +++ b/ts/packages/commandExecutor/src/commandServer.ts @@ -29,6 +29,7 @@ import { import * as fs from "fs"; import * as path from "path"; import * as os from "os"; +import { randomUUID } from "crypto"; import { convert } from "html-to-text"; import { loadConfig, type ResolvedAgentServerConfig } from "./config/index.js"; import { @@ -1167,29 +1168,30 @@ export class CommandServer { request: WorkspaceCommandInput, signal?: AbortSignal, ): Promise { + // The Code Agent assigns an ID when the caller omits one. Resolve it + // here so the result and any cancellation refer to the same command. + const executionId = request.executionId ?? randomUUID(); if (this.workspaceCommandInFlight) { return workspaceCommandFailure( "This Command Executor already has a workspace command in progress. Use a separate MCP connection for a concurrent command.", - request.executionId, + executionId, ); } this.workspaceCommandInFlight = true; if (signal?.aborted) { this.workspaceCommandInFlight = false; - return cancelledWorkspaceCommandResult(request.executionId); + return cancelledWorkspaceCommandResult(executionId); } let acquiredDispatcherLock = false; const cancelOnAbort = () => { - void this.cancelWorkspaceCommand({ - executionId: request.executionId, - }); + void this.cancelWorkspaceCommand({ executionId }); }; signal?.addEventListener("abort", cancelOnAbort, { once: true }); try { if (this.dispatcherRequestInFlight) { return workspaceCommandFailure( "Another request is already using this Command Executor. Wait for it to complete before sending another command.", - request.executionId, + executionId, ); } this.dispatcherRequestInFlight = true; @@ -1198,7 +1200,7 @@ export class CommandServer { { schemaName: "code.code-workbench", actionName: "runWorkspaceCommand", - parameters: request, + parameters: { ...request, executionId }, }, true, ); @@ -1209,10 +1211,7 @@ export class CommandServer { ) { return result; } - return workspaceCommandFailure( - resultText(result), - request.executionId, - ); + return workspaceCommandFailure(resultText(result), executionId); } finally { if (acquiredDispatcherLock) { this.dispatcherRequestInFlight = false; diff --git a/ts/packages/commandExecutor/src/workspaceCommandMcpSchema.ts b/ts/packages/commandExecutor/src/workspaceCommandMcpSchema.ts index 3a82d1467b..4b62d4c090 100644 --- a/ts/packages/commandExecutor/src/workspaceCommandMcpSchema.ts +++ b/ts/packages/commandExecutor/src/workspaceCommandMcpSchema.ts @@ -47,8 +47,9 @@ export const WorkspaceCommandInputSchema = z.object({ .string() .min(1) .max(128) + .optional() .describe( - "Unique caller-generated ID used to correlate this command and cancel it later.", + "Optional unique caller-generated ID used to correlate this command and cancel it later. One is assigned when omitted, and returned in the result.", ), }); diff --git a/ts/packages/commandExecutor/test/workspaceCommandMcpSchema.spec.ts b/ts/packages/commandExecutor/test/workspaceCommandMcpSchema.spec.ts index 6678ce22aa..8c62d25915 100644 --- a/ts/packages/commandExecutor/test/workspaceCommandMcpSchema.spec.ts +++ b/ts/packages/commandExecutor/test/workspaceCommandMcpSchema.spec.ts @@ -45,6 +45,21 @@ describe("workspace command MCP schemas", () => { ).toThrow(); }); + test("treats executionId as optional so grammar-sourced actions stay valid", () => { + // The .agr grammar and the code-workbench action schema both omit + // executionId; the Code Agent assigns one. Requiring it here would + // reject actions the action schema declares valid. + expect( + WorkspaceCommandInputSchema.parse({ command: "pnpm test" }), + ).toEqual({ command: "pnpm test" }); + expect(() => + WorkspaceCommandInputSchema.parse({ + command: "pnpm test", + executionId: "", + }), + ).toThrow(); + }); + test("keeps success, failure, and cancellation result contracts distinct", () => { expect( WorkspaceCommandResultSchema.parse({ From 2eec0f1e775a7afe74b3e4d2668b995d19919e10 Mon Sep 17 00:00:00 2001 From: George Ng Date: Fri, 4 Sep 2026 15:51:19 -0700 Subject: [PATCH 6/8] Extract and test cancellation control routing; tighten docs and test config Move the cancellation-control routing table out of codeActionHandler into cancellationControl.ts behind a minimal sendToClient interface, and cover it with tests. The table's invariant is that every entry is removed exactly once, on a response, either side disconnecting, a delivery failure, the timeout, or teardown. That is the most stateful code in the feature and had no coverage; each path now asserts the table drains. Correct the command-risk documentation. commandRiskLevel is declared by the caller and is advisory. The enforced boundary is the focused-tool allowlist plus workspace-root path confinement, not a classification Coda performs. Regenerate the workbench keyword file for the changed schema comment. Collapse the two duplicated workspace-command failure literals in the Command Executor into one builder, and note the timeout constants that are duplicated across packages and the two unrelated 5s constants. Replace the hardcoded test file list in coda's test:local with a tsconfig, so adding a test file no longer means editing package.json. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fb716021-4cea-41f8-9b92-36508a78285b --- .../agents/code/src/cancellationControl.ts | 167 +++++++++++++++++ .../agents/code/src/codeActionHandler.ts | 133 ++----------- ...orkbenchCommandActionsSchema.keywords.json | 4 +- .../vscode/workbenchCommandActionsSchema.ts | 2 +- .../code/test/cancellationControl.spec.ts | 174 ++++++++++++++++++ ts/packages/coda/README.md | 24 ++- ts/packages/coda/package.json | 2 +- ts/packages/coda/src/test/tsconfig.json | 13 ++ .../coda/src/workspaceCommandRunner.ts | 5 + .../commandExecutor/src/commandServer.ts | 39 ++-- .../src/workspaceCommandMcpSchema.ts | 2 +- 11 files changed, 414 insertions(+), 151 deletions(-) create mode 100644 ts/packages/agents/code/src/cancellationControl.ts create mode 100644 ts/packages/agents/code/test/cancellationControl.spec.ts create mode 100644 ts/packages/coda/src/test/tsconfig.json diff --git a/ts/packages/agents/code/src/cancellationControl.ts b/ts/packages/agents/code/src/cancellationControl.ts new file mode 100644 index 0000000000..2d102aede9 --- /dev/null +++ b/ts/packages/agents/code/src/cancellationControl.ts @@ -0,0 +1,167 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// Routing table for cancellation requests that arrive from a control client +// (the Command Executor) and must be forwarded to a Coda workspace client. +// The forwarded message carries an internally allocated call id so responses +// can be matched, then mapped back to the id the requester used. +// +// Every entry must be removed exactly once, on one of: a response, the target +// disconnecting, the requester disconnecting, a delivery failure, the timeout, +// or server teardown. A leaked entry hangs the requester until its own +// timeout, so the invariant worth testing is that the table returns to empty. + +// How long a forwarded cancellation may sit unanswered by the Coda workspace +// before the requester is told it failed. Unrelated to the runner's +// STOP_FALLBACK_MS in the Coda extension. +export const CANCELLATION_CONTROL_TIMEOUT_MS = 5_000; + +// Only the piece of CodeAgentWebSocketServer this table needs, so the routing +// can be exercised without a live websocket server. +export interface CancellationControlTarget { + sendToClient(clientId: string, message: string): boolean; +} + +type CancellationControlCall = { + clientId: string; + targetClientId: string; + responseId: unknown; + executionId: string; + timeout: NodeJS.Timeout; +}; + +const cancellationControlCalls = new Map(); + +export function cancellationControlCallCount(): number { + return cancellationControlCalls.size; +} + +function deleteCancellationControlCall( + callId: number, +): CancellationControlCall | undefined { + const call = cancellationControlCalls.get(callId); + if (call !== undefined) { + clearTimeout(call.timeout); + cancellationControlCalls.delete(callId); + } + return call; +} + +export function sendCancellationControlFailure( + server: CancellationControlTarget, + call: { clientId: string; responseId: unknown; executionId: string }, + error: string, +): void { + server.sendToClient( + call.clientId, + JSON.stringify({ + id: call.responseId, + result: JSON.stringify({ + success: false, + error, + cancelled: false, + pendingCancellation: false, + executionId: call.executionId, + }), + }), + ); +} + +// Drops entries without answering the requester. Only correct when the server +// is going away, since the requester's connection is going away with it. +export function clearCancellationControlCalls(): void { + for (const callId of [...cancellationControlCalls.keys()]) { + deleteCancellationControlCall(callId); + } +} + +export function forwardCancellationControlRequest( + server: CancellationControlTarget, + request: { + callId: number; + clientId: string; + targetClientId: string; + responseId: unknown; + executionId: string; + method: string; + params: Record; + // Overridable so tests can exercise the timeout without waiting. + timeoutMs?: number; + }, +): void { + const { callId } = request; + const timeout = setTimeout(() => { + const call = deleteCancellationControlCall(callId); + if (call !== undefined) { + sendCancellationControlFailure( + server, + call, + "The Coda workspace did not respond to the cancellation request.", + ); + } + }, request.timeoutMs ?? CANCELLATION_CONTROL_TIMEOUT_MS); + timeout.unref(); + cancellationControlCalls.set(callId, { + clientId: request.clientId, + targetClientId: request.targetClientId, + responseId: request.responseId, + executionId: request.executionId, + timeout, + }); + if ( + !server.sendToClient( + request.targetClientId, + JSON.stringify({ + id: callId, + method: request.method, + params: { ...request.params, allowPendingCancellation: true }, + }), + ) + ) { + const call = deleteCancellationControlCall(callId); + if (call !== undefined) { + sendCancellationControlFailure( + server, + call, + "The Coda workspace disconnected before the cancellation request could be delivered.", + ); + } + } +} + +// Returns true when the message was a forwarded cancellation response and has +// been routed back to the requester under its original id. +export function resolveCancellationControlResponse( + server: CancellationControlTarget, + data: { id?: unknown; result?: unknown }, +): boolean { + const call = deleteCancellationControlCall(Number(data.id)); + if (call === undefined) { + return false; + } + server.sendToClient( + call.clientId, + JSON.stringify({ ...data, id: call.responseId }), + ); + return true; +} + +export function handleCancellationControlDisconnect( + server: CancellationControlTarget, + clientId: string, +): void { + for (const [callId, call] of [...cancellationControlCalls]) { + if (call.clientId !== clientId && call.targetClientId !== clientId) { + continue; + } + deleteCancellationControlCall(callId); + // Only the requester is still around to hear about it. + if (call.targetClientId === clientId && call.clientId !== clientId) { + sendCancellationControlFailure( + server, + call, + "The Coda workspace disconnected before responding to the cancellation request.", + ); + } + } +} diff --git a/ts/packages/agents/code/src/codeActionHandler.ts b/ts/packages/agents/code/src/codeActionHandler.ts index de491d723d..28df1adcf7 100644 --- a/ts/packages/agents/code/src/codeActionHandler.ts +++ b/ts/packages/agents/code/src/codeActionHandler.ts @@ -3,6 +3,12 @@ import { WebSocketMessageV2 } from "@typeagent/websocket-utils"; import { CodeAgentWebSocketServer } from "./codeAgentWebSocketServer.js"; +import { + clearCancellationControlCalls, + forwardCancellationControlRequest, + handleCancellationControlDisconnect, + resolveCancellationControlResponse, +} from "./cancellationControl.js"; import { ActionContext, AppAction, @@ -67,68 +73,12 @@ const sharedPendingCalls: Map< clientId?: string | undefined; } > = new Map(); -const cancellationControlCalls = new Map< - number, - { - clientId: string; - targetClientId: string; - responseId: unknown; - executionId: string; - timeout: NodeJS.Timeout; - } ->(); -const CANCELLATION_CONTROL_TIMEOUT_MS = 5_000; // Global call-id counter. The pending-calls map is module-scoped (one // websocket server is shared across all sessions), so the id space must // also be global — per-session counters would collide on 0,1,2,... and // route a response to the wrong session's pending call. let nextSharedCallId = 0; -function deleteCancellationControlCall(callId: number) { - const call = cancellationControlCalls.get(callId); - if (call !== undefined) { - clearTimeout(call.timeout); - cancellationControlCalls.delete(callId); - } - return call; -} - -function clearCancellationControlCalls(clientId?: string): void { - for (const [callId, call] of cancellationControlCalls) { - if ( - clientId === undefined || - call.clientId === clientId || - call.targetClientId === clientId - ) { - deleteCancellationControlCall(callId); - } - } -} - -function sendCancellationControlFailure( - server: CodeAgentWebSocketServer, - call: { - clientId: string; - responseId: unknown; - executionId: string; - }, - error: string, -): void { - server.sendToClient( - call.clientId, - JSON.stringify({ - id: call.responseId, - result: JSON.stringify({ - success: false, - error, - cancelled: false, - pendingCancellation: false, - executionId: call.executionId, - }), - }), - ); -} - export function displayCodaResult(result: unknown): DisplayContent { const message = typeof result === "string" @@ -353,62 +303,20 @@ function attachSharedOnMessage(server: CodeAgentWebSocketServer): void { ); return; } - const callId = nextSharedCallId++; - const timeout = setTimeout(() => { - const call = deleteCancellationControlCall(callId); - if (call !== undefined) { - sendCancellationControlFailure( - server, - call, - "The Coda workspace did not respond to the cancellation request.", - ); - } - }, CANCELLATION_CONTROL_TIMEOUT_MS); - timeout.unref(); - cancellationControlCalls.set(callId, { + forwardCancellationControlRequest(server, { + callId: nextSharedCallId++, clientId, targetClientId, responseId: data.id, executionId: data.params.executionId, - timeout, + method: data.method, + params: data.params, }); - if ( - !server.sendToClient( - targetClientId, - JSON.stringify({ - id: callId, - method: data.method, - params: { - ...data.params, - allowPendingCancellation: true, - }, - }), - ) - ) { - const call = deleteCancellationControlCall(callId); - if (call !== undefined) { - sendCancellationControlFailure( - server, - call, - "The Coda workspace disconnected before the cancellation request could be delivered.", - ); - } - } return; } if (data.id !== undefined && data.result !== undefined) { - const controlCall = deleteCancellationControlCall( - Number(data.id), - ); - if (controlCall !== undefined) { - server.sendToClient( - controlCall.clientId, - JSON.stringify({ - ...data, - id: controlCall.responseId, - }), - ); + if (resolveCancellationControlResponse(server, data)) { return; } const pendingCall = sharedPendingCalls.get(Number(data.id)); @@ -459,24 +367,7 @@ function attachSharedOnMessage(server: CodeAgentWebSocketServer): void { } }; server.onClientDisconnected = (clientId: string) => { - for (const [callId, call] of cancellationControlCalls) { - if ( - call.clientId === clientId || - call.targetClientId === clientId - ) { - deleteCancellationControlCall(callId); - if ( - call.targetClientId === clientId && - call.clientId !== clientId - ) { - sendCancellationControlFailure( - server, - call, - "The Coda workspace disconnected before responding to the cancellation request.", - ); - } - } - } + handleCancellationControlDisconnect(server, clientId); }; } diff --git a/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.keywords.json b/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.keywords.json index 2981e59203..bdfcb632ee 100644 --- a/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.keywords.json +++ b/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.keywords.json @@ -2,8 +2,8 @@ "schemaVersion": 1, "schema": "code.code-workbench", "generatedBy": "llm", - "generatedAt": "2026-09-04T21:47:59.558Z", - "sourceHash": "gr8T/xLRyyJ5HzjdEpeh4Gh3zn8E6CGvT4N4aUjiQbc=", + "generatedAt": "2026-09-04T22:52:00.000Z", + "sourceHash": "OkaEixG6KFnLpeREFOlMdv1slaWzWaPKbXkzJbEKelU=", "actions": { "workbenchOpenFile": [ "file", diff --git a/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.ts b/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.ts index 66e3196f40..c3c108d052 100644 --- a/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.ts +++ b/ts/packages/agents/code/src/vscode/workbenchCommandActionsSchema.ts @@ -78,7 +78,7 @@ export type WorkbenchActionRunWorkspaceCommand = { workspaceFolder?: string; // Optional path relative to the selected workspace root. workingDirectory?: string; - // Declared command safety classification. Coda also classifies the exact command and blocks high-risk commands. + // Caller-declared risk level. "high" is rejected. Advisory only: the enforced limits are the focused-tool allowlist and workspace-root path confinement. commandRiskLevel?: "low" | "medium" | "high"; // Bounded execution time in milliseconds. timeoutMs?: number; diff --git a/ts/packages/agents/code/test/cancellationControl.spec.ts b/ts/packages/agents/code/test/cancellationControl.spec.ts new file mode 100644 index 0000000000..c06fcb6b4b --- /dev/null +++ b/ts/packages/agents/code/test/cancellationControl.spec.ts @@ -0,0 +1,174 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { + CancellationControlTarget, + cancellationControlCallCount, + clearCancellationControlCalls, + forwardCancellationControlRequest, + handleCancellationControlDisconnect, + resolveCancellationControlResponse, +} from "../src/cancellationControl.js"; + +type SentMessage = { clientId: string; payload: any }; + +class FakeServer implements CancellationControlTarget { + public sent: SentMessage[] = []; + private unreachable = new Set(); + + public disconnect(clientId: string) { + this.unreachable.add(clientId); + } + + public sendToClient(clientId: string, message: string): boolean { + if (this.unreachable.has(clientId)) { + return false; + } + this.sent.push({ clientId, payload: JSON.parse(message) }); + return true; + } + + public to(clientId: string): SentMessage[] { + return this.sent.filter((message) => message.clientId === clientId); + } +} + +const requester = "control-client"; +const target = "coda-client"; + +function forward( + server: FakeServer, + callId = 1, + responseId: unknown = "exec-1", +) { + forwardCancellationControlRequest(server, { + callId, + clientId: requester, + targetClientId: target, + responseId, + executionId: "exec-1", + method: "code/cancelWorkspaceCommand", + params: { executionId: "exec-1" }, + }); +} + +function failureSentTo(server: FakeServer, clientId: string) { + const messages = server.to(clientId); + expect(messages).toHaveLength(1); + return JSON.parse(messages[0].payload.result); +} + +describe("cancellation control routing", () => { + afterEach(() => { + clearCancellationControlCalls(); + }); + + test("forwards to the target and maps the response back to the requester id", () => { + const server = new FakeServer(); + forward(server, 7, "caller-supplied-id"); + + // The target sees the internal call id, not the requester's id. + expect(server.to(target)[0].payload).toMatchObject({ + id: 7, + method: "code/cancelWorkspaceCommand", + params: { executionId: "exec-1", allowPendingCancellation: true }, + }); + expect(cancellationControlCallCount()).toBe(1); + + const routed = resolveCancellationControlResponse(server, { + id: 7, + result: JSON.stringify({ success: true }), + }); + + expect(routed).toBe(true); + expect(server.to(requester)[0].payload).toMatchObject({ + id: "caller-supplied-id", + }); + expect(cancellationControlCallCount()).toBe(0); + }); + + test("ignores a response that does not belong to the control table", () => { + const server = new FakeServer(); + expect( + resolveCancellationControlResponse(server, { + id: 99, + result: "{}", + }), + ).toBe(false); + expect(server.sent).toHaveLength(0); + }); + + test("answers the requester when the target disconnects", () => { + const server = new FakeServer(); + forward(server); + server.sent = []; + + handleCancellationControlDisconnect(server, target); + + expect(failureSentTo(server, requester)).toMatchObject({ + success: false, + cancelled: false, + executionId: "exec-1", + error: "The Coda workspace disconnected before responding to the cancellation request.", + }); + expect(cancellationControlCallCount()).toBe(0); + }); + + test("drops the entry silently when the requester disconnects", () => { + const server = new FakeServer(); + forward(server); + server.sent = []; + + handleCancellationControlDisconnect(server, requester); + + expect(server.sent).toHaveLength(0); + expect(cancellationControlCallCount()).toBe(0); + }); + + test("answers the requester when the forward cannot be delivered", () => { + const server = new FakeServer(); + server.disconnect(target); + + forward(server); + + expect(failureSentTo(server, requester)).toMatchObject({ + success: false, + error: "The Coda workspace disconnected before the cancellation request could be delivered.", + }); + expect(cancellationControlCallCount()).toBe(0); + }); + + test("answers the requester when the target never responds", async () => { + const server = new FakeServer(); + forwardCancellationControlRequest(server, { + callId: 1, + clientId: requester, + targetClientId: target, + responseId: "exec-1", + executionId: "exec-1", + method: "code/cancelWorkspaceCommand", + params: { executionId: "exec-1" }, + timeoutMs: 10, + }); + server.sent = []; + + await new Promise((resolve) => setTimeout(resolve, 50)); + + expect(failureSentTo(server, requester)).toMatchObject({ + success: false, + error: "The Coda workspace did not respond to the cancellation request.", + }); + expect(cancellationControlCallCount()).toBe(0); + }); + + test("clears every entry on teardown", () => { + const server = new FakeServer(); + forward(server, 1, "a"); + forward(server, 2, "b"); + expect(cancellationControlCallCount()).toBe(2); + + clearCancellationControlCalls(); + + expect(cancellationControlCallCount()).toBe(0); + }); +}); diff --git a/ts/packages/coda/README.md b/ts/packages/coda/README.md index ddf1fa1724..f06129f9a1 100644 --- a/ts/packages/coda/README.md +++ b/ts/packages/coda/README.md @@ -33,14 +33,22 @@ multi-root workspaces. A relative `workingDirectory` may select a subdirectory. The action uses `cmd.exe /d /s /c` on Windows and `/bin/sh -c` on Unix, passing the command as one shell argument. Commands are limited to 16 KiB; each output stream is limited to 64 KiB and reports its original byte count. The timeout -defaults to two minutes and is capped at five. High-risk commands remain blocked -by the existing Coda command-risk policy. To avoid treating this as an arbitrary -terminal shortcut, structured execution accepts only focused build, test, lint, -and diagnostic tools and rejects shell composition. Commands receive an `executionId`; -independent IDs run concurrently inside one Coda window and -`cancelWorkspaceCommand` cancels one. The Code Agent sends structured commands -only when exactly one Coda workspace is connected, preventing a command from -running in an unintended VS Code window. +defaults to two minutes and is capped at five. + +What is actually enforced, in order: a `commandRiskLevel` of `high` is rejected; +the executable must be one of the focused build, test, lint, and diagnostic tools +in the allowlist; shell composition and path-expansion syntax are rejected; and +every path argument must resolve inside the selected workspace root, both +lexically and after symlinks are followed. Note that `commandRiskLevel` is +declared by the caller, so it is advisory. The allowlist and the path +confinement are the enforced boundary, not the declared risk level. Coda does +not classify the command itself. + +Commands receive an `executionId`; independent IDs run concurrently inside one +Coda window and `cancelWorkspaceCommand` cancels one. The `executionId` is +optional on the way in and the Code Agent assigns one when it is omitted. The +Code Agent sends structured commands only when exactly one Coda workspace is +connected, preventing a command from running in an unintended VS Code window. > Tip: If you have any problems, please check to see if the `coda` extension was installed correctly using the command `code --list-extensions`. You should see `aisystems.copilot-coda` in the list of vscode extensions. diff --git a/ts/packages/coda/package.json b/ts/packages/coda/package.json index 1011effde7..97f4c4cdb5 100644 --- a/ts/packages/coda/package.json +++ b/ts/packages/coda/package.json @@ -31,7 +31,7 @@ "pretest": "pnpm run build", "test-compile": "tsc -p src", "test:full": "vscode-test", - "test:local": "tsc --target ES2022 --module Node16 --moduleResolution Node16 --esModuleInterop --strict --types node --outDir dist-test --rootDir src src/workspaceCommandRunner.ts src/workspaceCommandPolicy.ts src/test/workspaceCommandRunner.spec.ts src/test/workspaceCommandPolicy.spec.ts && node --test dist-test/test/workspaceCommandRunner.spec.js dist-test/test/workspaceCommandPolicy.spec.js", + "test:local": "tsc -p src/test && node --test \"dist-test/test/*.spec.js\"", "vscode:prepublish": "pnpm run esbuild-base --minify", "watch": "tsc -w" }, diff --git a/ts/packages/coda/src/test/tsconfig.json b/ts/packages/coda/src/test/tsconfig.json new file mode 100644 index 0000000000..5e6991ea7f --- /dev/null +++ b/ts/packages/coda/src/test/tsconfig.json @@ -0,0 +1,13 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "Node16", + "moduleResolution": "Node16", + "esModuleInterop": true, + "strict": true, + "types": ["node"], + "outDir": "../../dist-test", + "rootDir": ".." + }, + "include": ["./**/*.spec.ts"] +} diff --git a/ts/packages/coda/src/workspaceCommandRunner.ts b/ts/packages/coda/src/workspaceCommandRunner.ts index 0314e8f62a..57e83b8b79 100644 --- a/ts/packages/coda/src/workspaceCommandRunner.ts +++ b/ts/packages/coda/src/workspaceCommandRunner.ts @@ -6,9 +6,14 @@ import { randomUUID } from "crypto"; export const MAX_COMMAND_BYTES = 16 * 1024; export const MAX_OUTPUT_BYTES = 64 * 1024; +// Source of truth for the command timeout bounds. The MCP input schema in +// packages/commandExecutor and getActionResponseTimeoutMs in the code agent +// encode the same numbers; keep them in step when changing these. export const DEFAULT_TIMEOUT_MS = 2 * 60 * 1000; export const MAX_TIMEOUT_MS = 5 * 60 * 1000; const MAX_PENDING_CANCELLATIONS = 64; +// How long a SIGTERM'd child may keep the streams open before completion is +// forced. Unrelated to the code agent's CANCELLATION_CONTROL_TIMEOUT_MS. const STOP_FALLBACK_MS = 5_000; export type WorkspaceCommandOutput = { diff --git a/ts/packages/commandExecutor/src/commandServer.ts b/ts/packages/commandExecutor/src/commandServer.ts index 7e6fbf70bb..beff05c084 100644 --- a/ts/packages/commandExecutor/src/commandServer.ts +++ b/ts/packages/commandExecutor/src/commandServer.ts @@ -126,37 +126,42 @@ function resultText(result: CallToolResult): string { .join("\n"); } -function workspaceCommandFailure( - error: string, +function workspaceCommandNonRunResult( + fields: { error: string; cancelled: boolean }, executionId: string, ): CallToolResult { - const failure = { + const result = { success: false, - error, + error: fields.error, exitCode: null, durationMs: 0, stdout: { text: "", truncated: false, totalBytes: 0 }, stderr: { text: "", truncated: false, totalBytes: 0 }, timedOut: false, - cancelled: false, + cancelled: fields.cancelled, executionId, }; - return toolResult(JSON.stringify(failure, null, 2), failure); + return toolResult(JSON.stringify(result, null, 2), result); +} + +function workspaceCommandFailure( + error: string, + executionId: string, +): CallToolResult { + return workspaceCommandNonRunResult( + { error, cancelled: false }, + executionId, + ); } function cancelledWorkspaceCommandResult(executionId: string): CallToolResult { - const result = { - success: false, - error: "The command request was cancelled before it was dispatched.", - exitCode: null, - durationMs: 0, - stdout: { text: "", truncated: false, totalBytes: 0 }, - stderr: { text: "", truncated: false, totalBytes: 0 }, - timedOut: false, - cancelled: true, + return workspaceCommandNonRunResult( + { + error: "The command request was cancelled before it was dispatched.", + cancelled: true, + }, executionId, - }; - return toolResult(JSON.stringify(result, null, 2), result); + ); } function cancellationFailure( diff --git a/ts/packages/commandExecutor/src/workspaceCommandMcpSchema.ts b/ts/packages/commandExecutor/src/workspaceCommandMcpSchema.ts index 4b62d4c090..c0f9c81351 100644 --- a/ts/packages/commandExecutor/src/workspaceCommandMcpSchema.ts +++ b/ts/packages/commandExecutor/src/workspaceCommandMcpSchema.ts @@ -32,7 +32,7 @@ export const WorkspaceCommandInputSchema = z.object({ .enum(["low", "medium", "high"]) .optional() .describe( - "Optional declared command-risk level. High-risk commands are blocked; structured execution also restricts commands to focused build, test, lint, and diagnostic tools.", + 'Optional caller-declared command-risk level. "high" is rejected. Advisory only: the enforced limits are Coda\'s focused build/test/lint/diagnostic tool allowlist and workspace-root path confinement.', ), timeoutMs: z .number() From fe0661d304c098b0edc8ffeae39c4ea740094023 Mon Sep 17 00:00:00 2001 From: George Ng Date: Fri, 4 Sep 2026 16:00:27 -0700 Subject: [PATCH 7/8] Document the dispatcher lock and the cancel transport bypass The run_workspace_command tool description now states that the tool holds the Command Executor for the whole run, so callers know to use a separate MCP connection for concurrent work. cancelWorkspaceCommand gains a comment explaining why it talks to the Code Agent websocket directly instead of going through the dispatcher, and notes the known agent-discovery limitation. Also renames workspaceCommandNonRunResult to unexecutedWorkspaceCommandResult. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fb716021-4cea-41f8-9b92-36508a78285b --- .../commandExecutor/src/commandServer.ts | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/ts/packages/commandExecutor/src/commandServer.ts b/ts/packages/commandExecutor/src/commandServer.ts index beff05c084..771cf9232f 100644 --- a/ts/packages/commandExecutor/src/commandServer.ts +++ b/ts/packages/commandExecutor/src/commandServer.ts @@ -126,7 +126,9 @@ function resultText(result: CallToolResult): string { .join("\n"); } -function workspaceCommandNonRunResult( +// One shape for every result where the command never actually ran, so the +// failure and pre-dispatch-cancellation paths cannot drift apart. +function unexecutedWorkspaceCommandResult( fields: { error: string; cancelled: boolean }, executionId: string, ): CallToolResult { @@ -148,14 +150,14 @@ function workspaceCommandFailure( error: string, executionId: string, ): CallToolResult { - return workspaceCommandNonRunResult( + return unexecutedWorkspaceCommandResult( { error, cancelled: false }, executionId, ); } function cancelledWorkspaceCommandResult(executionId: string): CallToolResult { - return workspaceCommandNonRunResult( + return unexecutedWorkspaceCommandResult( { error: "The command request was cancelled before it was dispatched.", cancelled: true, @@ -800,7 +802,7 @@ export class CommandServer { inputSchema: WorkspaceCommandInputSchema.shape, outputSchema: WorkspaceCommandResultSchema.shape, description: - "Run one explicitly requested build, test, lint, or diagnostic command in the open VS Code workspace through Coda. This is a direct TypeAgent action: it does not use natural-language translation or a terminal UI. Returns structured stdout, stderr, exitCode, durationMs, success, timedOut, cancelled, and truncation metadata. Example: { command: 'pnpm test -- --runInBand', workingDirectory: 'ts/packages/coda', executionId: 'coda-tests-1' }. Coda blocks high-risk commands and rejects shell composition.", + "Run one explicitly requested build, test, lint, or diagnostic command in the open VS Code workspace through Coda. This is a direct TypeAgent action: it does not use natural-language translation or a terminal UI. Returns structured stdout, stderr, exitCode, durationMs, success, timedOut, cancelled, and truncation metadata. Example: { command: 'pnpm test -- --runInBand', workingDirectory: 'ts/packages/coda', executionId: 'coda-tests-1' }. Coda rejects shell composition and restricts commands to an allowlist of focused tools, with path arguments confined to the workspace root. This tool holds the Command Executor for the whole run, so execute_command and execute_action are unavailable until it finishes; use a separate MCP connection for concurrent work. cancel_workspace_command still works while it runs.", }, async (request: WorkspaceCommandInput, extra) => this.runWorkspaceCommand(request, extra.signal), @@ -1226,6 +1228,18 @@ export class CommandServer { } } + // Cancellation deliberately bypasses the dispatcher and talks to the Code + // Agent websocket directly. It has to: a running run_workspace_command + // holds dispatcherRequestInFlight for its whole duration, so a cancel + // routed through executeAction would queue behind the very command it is + // meant to stop. The lock itself is load-bearing, since responseCollector + // is a single buffer shared by every dispatcher request, so the second + // transport is the consequence of that and not an alternative to it. + // + // Known limitation: the target is resolved by discovering the "code" agent + // independently of where the run was dispatched. With more than one + // reachable agent server this can address a different Code Agent than the + // one running the command. private async cancelWorkspaceCommand( request: CancelWorkspaceCommandInput, ): Promise { From 82151bda39226e04c0c3bd09640aba4db71515ac Mon Sep 17 00:00:00 2001 From: George Ng Date: Fri, 4 Sep 2026 17:45:29 -0700 Subject: [PATCH 8/8] Accept Windows-style command switches Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/test/workspaceCommandPolicy.spec.ts | 17 ++++++++++++++++ .../coda/src/workspaceCommandPolicy.ts | 20 +++++++++++++++---- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/ts/packages/coda/src/test/workspaceCommandPolicy.spec.ts b/ts/packages/coda/src/test/workspaceCommandPolicy.spec.ts index a493e5cdfc..3a58ca69d8 100644 --- a/ts/packages/coda/src/test/workspaceCommandPolicy.spec.ts +++ b/ts/packages/coda/src/test/workspaceCommandPolicy.spec.ts @@ -33,6 +33,7 @@ test("rejects path arguments outside the workspace root", () => { "pytest -c../../../outside.ini", "tsc -p../../..", "pytest ../../../outside=test.py", + "msbuild /p:OutputPath=../../../outside", `tsc --project "${path.resolve( workspaceRoot, "..", @@ -70,6 +71,22 @@ test("allows path arguments that remain inside the workspace root", () => { ); }); +test("allows Windows-style slash-prefixed switches", () => { + for (const command of [ + "msbuild /t:Build", + "dotnet build /p:Configuration=Release", + ]) { + assert.equal( + validateFocusedWorkspaceCommand( + command, + workspaceCwd, + workspaceRoot, + ), + undefined, + ); + } +}); + test("rejects a bare symlink argument that resolves outside the workspace", () => { const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "coda-policy-")); try { diff --git a/ts/packages/coda/src/workspaceCommandPolicy.ts b/ts/packages/coda/src/workspaceCommandPolicy.ts index 0fb427a90f..fd7701ad27 100644 --- a/ts/packages/coda/src/workspaceCommandPolicy.ts +++ b/ts/packages/coda/src/workspaceCommandPolicy.ts @@ -99,10 +99,22 @@ function validatePathArguments( const realRoot = fs.realpathSync.native(resolvedRoot); for (const token of tokens.slice(1)) { const equalsIndex = token.indexOf("="); - let value = - token.startsWith("-") && equalsIndex > 1 - ? token.slice(equalsIndex + 1) - : token; + const windowsOption = /^\/[A-Za-z][A-Za-z0-9_-]*(?::|=)(.*)$/.exec( + token, + ); + const windowsOptionValue = windowsOption?.[1]; + const windowsOptionEqualsIndex = windowsOptionValue?.indexOf("=") ?? -1; + let value = windowsOptionValue ?? token; + if (windowsOptionEqualsIndex >= 0) { + value = value.slice(windowsOptionEqualsIndex + 1); + } + if ( + windowsOption === null && + token.startsWith("-") && + equalsIndex > 1 + ) { + value = token.slice(equalsIndex + 1); + } if (equalsIndex < 0) { const attachedShortOption = /^-[A-Za-z](.+)$/.exec(token); if (attachedShortOption !== null) {