From 7c369e0ac98f683d7ead97a2d8a9e885a3f6b222 Mon Sep 17 00:00:00 2001 From: Dallin Romney Date: Mon, 24 Nov 2025 13:49:10 -0800 Subject: [PATCH 1/4] fix: only set cwd to file URI if found --- core/tools/implementations/runTerminalCommand.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/tools/implementations/runTerminalCommand.ts b/core/tools/implementations/runTerminalCommand.ts index 15b1dc1bffb..38964a60b80 100644 --- a/core/tools/implementations/runTerminalCommand.ts +++ b/core/tools/implementations/runTerminalCommand.ts @@ -284,7 +284,7 @@ export const runTerminalCommandImpl: ToolImpl = async (args, extras) => { // Handle case where no workspace is available let cwd: string; - if (workspaceDirs.length > 0) { + if (workspaceDirs.length > 0 && workspaceDirs[0].startsWith("file:/")) { cwd = fileURLToPath(workspaceDirs[0]); } else { // Default to user's home directory with fallbacks From 4837ef13529ae04a53180d5fdec59a25011f7c1c Mon Sep 17 00:00:00 2001 From: Dallin Romney Date: Mon, 24 Nov 2025 13:49:29 -0800 Subject: [PATCH 2/4] fix: only set cwd to file URI if file uri found --- core/tools/implementations/runTerminalCommand.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/core/tools/implementations/runTerminalCommand.ts b/core/tools/implementations/runTerminalCommand.ts index 38964a60b80..5fa0288ffe9 100644 --- a/core/tools/implementations/runTerminalCommand.ts +++ b/core/tools/implementations/runTerminalCommand.ts @@ -284,8 +284,11 @@ export const runTerminalCommandImpl: ToolImpl = async (args, extras) => { // Handle case where no workspace is available let cwd: string; - if (workspaceDirs.length > 0 && workspaceDirs[0].startsWith("file:/")) { - cwd = fileURLToPath(workspaceDirs[0]); + const fileWorkspaceDir = workspaceDirs.find((dir) => + dir.startsWith("file:/"), + ); + if (fileWorkspaceDir) { + cwd = fileURLToPath(fileWorkspaceDir); } else { // Default to user's home directory with fallbacks try { From 857903aa5bf9c458f38cc49b86ed04fb633fc2b3 Mon Sep 17 00:00:00 2001 From: Continue Agent Date: Mon, 24 Nov 2025 21:56:19 +0000 Subject: [PATCH 3/4] Add test coverage for runTerminalCommand cwd handling with non-file URIs Tests verify that the fix correctly: - Filters for file:// URIs before calling fileURLToPath - Falls back to HOME/USERPROFILE/cwd/tmpdir when no file URIs exist - Handles various workspace configurations (empty, non-file URIs, mixed) - Works across different remote environments (local, WSL, dev-container) Co-authored-by: nate Generated with [Continue](https://continue.dev) Co-Authored-By: Continue --- .../runTerminalCommand.test.ts | 219 ++++++++++++++++++ 1 file changed, 219 insertions(+) create mode 100644 core/tools/implementations/runTerminalCommand.test.ts diff --git a/core/tools/implementations/runTerminalCommand.test.ts b/core/tools/implementations/runTerminalCommand.test.ts new file mode 100644 index 00000000000..c9a3a0c1cea --- /dev/null +++ b/core/tools/implementations/runTerminalCommand.test.ts @@ -0,0 +1,219 @@ +import { fileURLToPath } from "node:url"; +import os from "node:os"; +import { runTerminalCommandImpl } from "./runTerminalCommand"; + +describe("runTerminalCommand cwd handling", () => { + const mockExtras = { + ide: { + getIdeInfo: jest.fn().async().mockResolvedValue({ remoteName: "local" }), + getWorkspaceDirs: jest.fn().async(), + runCommand: jest.fn().async(), + }, + toolCallId: "test-tool-call", + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe("workspace directory handling", () => { + it("should use file:// URI when available", async () => { + const fileUri = "file:///home/user/workspace"; + mockExtras.ide.getWorkspaceDirs.mockResolvedValue([fileUri]); + + // We can't easily test the internal cwd without mocking child_process, + // but we can verify the function doesn't throw with file URIs + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + mockExtras as any, + ), + ).resolves.toBeDefined(); + }); + + it("should skip non-file URIs and use the first file:// URI", async () => { + const workspaceDirs = [ + "vscode-vfs://github/user/repo", + "untitled:/Untitled-1", + "file:///home/user/workspace", + "file:///home/user/other-workspace", + ]; + mockExtras.ide.getWorkspaceDirs.mockResolvedValue(workspaceDirs); + + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + mockExtras as any, + ), + ).resolves.toBeDefined(); + }); + + it("should handle workspace with only non-file URIs", async () => { + const workspaceDirs = [ + "vscode-vfs://github/user/repo", + "untitled:/Untitled-1", + ]; + mockExtras.ide.getWorkspaceDirs.mockResolvedValue(workspaceDirs); + + // Should fall back to HOME/USERPROFILE or process.cwd() without throwing + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + mockExtras as any, + ), + ).resolves.toBeDefined(); + }); + + it("should handle empty workspace directories", async () => { + mockExtras.ide.getWorkspaceDirs.mockResolvedValue([]); + + // Should fall back to HOME/USERPROFILE or process.cwd() without throwing + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + mockExtras as any, + ), + ).resolves.toBeDefined(); + }); + + it("should properly convert file:// URIs to paths", () => { + const fileUri = "file:///home/user/workspace"; + const expectedPath = "/home/user/workspace"; + + // Test that fileURLToPath works correctly with file:// URIs + expect(fileURLToPath(fileUri)).toBe(expectedPath); + }); + + it("should throw error when trying to convert non-file URI", () => { + const nonFileUri = "vscode-vfs://github/user/repo"; + + // This demonstrates why the fix is needed - fileURLToPath throws on non-file URIs + expect(() => fileURLToPath(nonFileUri)).toThrow(); + }); + }); + + describe("remote environment handling", () => { + it("should use ide.runCommand for non-enabled remote environments", async () => { + mockExtras.ide.getIdeInfo.mockResolvedValue({ + remoteName: "some-unsupported-remote", + }); + + const result = await runTerminalCommandImpl( + { command: "echo test" }, + mockExtras as any, + ); + + expect(mockExtras.ide.runCommand).toHaveBeenCalledWith("echo test"); + expect(result[0].content).toContain("Terminal output not available"); + }); + + it("should handle local environment with file URIs", async () => { + mockExtras.ide.getIdeInfo.mockResolvedValue({ remoteName: "local" }); + mockExtras.ide.getWorkspaceDirs.mockResolvedValue([ + "file:///home/user/workspace", + ]); + + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + mockExtras as any, + ), + ).resolves.toBeDefined(); + }); + + it("should handle WSL environment", async () => { + mockExtras.ide.getIdeInfo.mockResolvedValue({ remoteName: "wsl" }); + mockExtras.ide.getWorkspaceDirs.mockResolvedValue([ + "file:///home/user/workspace", + ]); + + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + mockExtras as any, + ), + ).resolves.toBeDefined(); + }); + + it("should handle dev-container environment", async () => { + mockExtras.ide.getIdeInfo.mockResolvedValue({ + remoteName: "dev-container", + }); + mockExtras.ide.getWorkspaceDirs.mockResolvedValue(["file:///workspace"]); + + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + mockExtras as any, + ), + ).resolves.toBeDefined(); + }); + }); + + describe("fallback behavior", () => { + it("should use HOME environment variable as fallback", async () => { + const originalHome = process.env.HOME; + process.env.HOME = "/home/testuser"; + + mockExtras.ide.getWorkspaceDirs.mockResolvedValue([ + "vscode-vfs://github/user/repo", + ]); + + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + mockExtras as any, + ), + ).resolves.toBeDefined(); + + process.env.HOME = originalHome; + }); + + it("should use USERPROFILE on Windows as fallback", async () => { + const originalHome = process.env.HOME; + const originalUserProfile = process.env.USERPROFILE; + + delete process.env.HOME; + process.env.USERPROFILE = "C:\\Users\\TestUser"; + + mockExtras.ide.getWorkspaceDirs.mockResolvedValue([]); + + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + mockExtras as any, + ), + ).resolves.toBeDefined(); + + process.env.HOME = originalHome; + process.env.USERPROFILE = originalUserProfile; + }); + + it("should use os.tmpdir() as final fallback", async () => { + const originalHome = process.env.HOME; + const originalUserProfile = process.env.USERPROFILE; + const originalCwd = process.cwd; + + delete process.env.HOME; + delete process.env.USERPROFILE; + // Mock process.cwd to throw an error + process.cwd = jest.fn().mockImplementation(() => { + throw new Error("No cwd available"); + }); + + mockExtras.ide.getWorkspaceDirs.mockResolvedValue([]); + + // Should fall back to os.tmpdir() without throwing + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + mockExtras as any, + ), + ).resolves.toBeDefined(); + + process.env.HOME = originalHome; + process.env.USERPROFILE = originalUserProfile; + process.cwd = originalCwd; + }); + }); +}); From 462e7168bdcb23be06411a3cc29caff29b3fe985 Mon Sep 17 00:00:00 2001 From: Dallin Romney Date: Mon, 24 Nov 2025 15:12:20 -0800 Subject: [PATCH 4/4] fix: terminal cwd tests --- .../runTerminalCommand.test.ts | 219 ------------------ .../runTerminalCommand.vitest.ts | 219 +++++++++++++++++- 2 files changed, 210 insertions(+), 228 deletions(-) delete mode 100644 core/tools/implementations/runTerminalCommand.test.ts diff --git a/core/tools/implementations/runTerminalCommand.test.ts b/core/tools/implementations/runTerminalCommand.test.ts deleted file mode 100644 index c9a3a0c1cea..00000000000 --- a/core/tools/implementations/runTerminalCommand.test.ts +++ /dev/null @@ -1,219 +0,0 @@ -import { fileURLToPath } from "node:url"; -import os from "node:os"; -import { runTerminalCommandImpl } from "./runTerminalCommand"; - -describe("runTerminalCommand cwd handling", () => { - const mockExtras = { - ide: { - getIdeInfo: jest.fn().async().mockResolvedValue({ remoteName: "local" }), - getWorkspaceDirs: jest.fn().async(), - runCommand: jest.fn().async(), - }, - toolCallId: "test-tool-call", - }; - - beforeEach(() => { - jest.clearAllMocks(); - }); - - describe("workspace directory handling", () => { - it("should use file:// URI when available", async () => { - const fileUri = "file:///home/user/workspace"; - mockExtras.ide.getWorkspaceDirs.mockResolvedValue([fileUri]); - - // We can't easily test the internal cwd without mocking child_process, - // but we can verify the function doesn't throw with file URIs - await expect( - runTerminalCommandImpl( - { command: "echo test", waitForCompletion: false }, - mockExtras as any, - ), - ).resolves.toBeDefined(); - }); - - it("should skip non-file URIs and use the first file:// URI", async () => { - const workspaceDirs = [ - "vscode-vfs://github/user/repo", - "untitled:/Untitled-1", - "file:///home/user/workspace", - "file:///home/user/other-workspace", - ]; - mockExtras.ide.getWorkspaceDirs.mockResolvedValue(workspaceDirs); - - await expect( - runTerminalCommandImpl( - { command: "echo test", waitForCompletion: false }, - mockExtras as any, - ), - ).resolves.toBeDefined(); - }); - - it("should handle workspace with only non-file URIs", async () => { - const workspaceDirs = [ - "vscode-vfs://github/user/repo", - "untitled:/Untitled-1", - ]; - mockExtras.ide.getWorkspaceDirs.mockResolvedValue(workspaceDirs); - - // Should fall back to HOME/USERPROFILE or process.cwd() without throwing - await expect( - runTerminalCommandImpl( - { command: "echo test", waitForCompletion: false }, - mockExtras as any, - ), - ).resolves.toBeDefined(); - }); - - it("should handle empty workspace directories", async () => { - mockExtras.ide.getWorkspaceDirs.mockResolvedValue([]); - - // Should fall back to HOME/USERPROFILE or process.cwd() without throwing - await expect( - runTerminalCommandImpl( - { command: "echo test", waitForCompletion: false }, - mockExtras as any, - ), - ).resolves.toBeDefined(); - }); - - it("should properly convert file:// URIs to paths", () => { - const fileUri = "file:///home/user/workspace"; - const expectedPath = "/home/user/workspace"; - - // Test that fileURLToPath works correctly with file:// URIs - expect(fileURLToPath(fileUri)).toBe(expectedPath); - }); - - it("should throw error when trying to convert non-file URI", () => { - const nonFileUri = "vscode-vfs://github/user/repo"; - - // This demonstrates why the fix is needed - fileURLToPath throws on non-file URIs - expect(() => fileURLToPath(nonFileUri)).toThrow(); - }); - }); - - describe("remote environment handling", () => { - it("should use ide.runCommand for non-enabled remote environments", async () => { - mockExtras.ide.getIdeInfo.mockResolvedValue({ - remoteName: "some-unsupported-remote", - }); - - const result = await runTerminalCommandImpl( - { command: "echo test" }, - mockExtras as any, - ); - - expect(mockExtras.ide.runCommand).toHaveBeenCalledWith("echo test"); - expect(result[0].content).toContain("Terminal output not available"); - }); - - it("should handle local environment with file URIs", async () => { - mockExtras.ide.getIdeInfo.mockResolvedValue({ remoteName: "local" }); - mockExtras.ide.getWorkspaceDirs.mockResolvedValue([ - "file:///home/user/workspace", - ]); - - await expect( - runTerminalCommandImpl( - { command: "echo test", waitForCompletion: false }, - mockExtras as any, - ), - ).resolves.toBeDefined(); - }); - - it("should handle WSL environment", async () => { - mockExtras.ide.getIdeInfo.mockResolvedValue({ remoteName: "wsl" }); - mockExtras.ide.getWorkspaceDirs.mockResolvedValue([ - "file:///home/user/workspace", - ]); - - await expect( - runTerminalCommandImpl( - { command: "echo test", waitForCompletion: false }, - mockExtras as any, - ), - ).resolves.toBeDefined(); - }); - - it("should handle dev-container environment", async () => { - mockExtras.ide.getIdeInfo.mockResolvedValue({ - remoteName: "dev-container", - }); - mockExtras.ide.getWorkspaceDirs.mockResolvedValue(["file:///workspace"]); - - await expect( - runTerminalCommandImpl( - { command: "echo test", waitForCompletion: false }, - mockExtras as any, - ), - ).resolves.toBeDefined(); - }); - }); - - describe("fallback behavior", () => { - it("should use HOME environment variable as fallback", async () => { - const originalHome = process.env.HOME; - process.env.HOME = "/home/testuser"; - - mockExtras.ide.getWorkspaceDirs.mockResolvedValue([ - "vscode-vfs://github/user/repo", - ]); - - await expect( - runTerminalCommandImpl( - { command: "echo test", waitForCompletion: false }, - mockExtras as any, - ), - ).resolves.toBeDefined(); - - process.env.HOME = originalHome; - }); - - it("should use USERPROFILE on Windows as fallback", async () => { - const originalHome = process.env.HOME; - const originalUserProfile = process.env.USERPROFILE; - - delete process.env.HOME; - process.env.USERPROFILE = "C:\\Users\\TestUser"; - - mockExtras.ide.getWorkspaceDirs.mockResolvedValue([]); - - await expect( - runTerminalCommandImpl( - { command: "echo test", waitForCompletion: false }, - mockExtras as any, - ), - ).resolves.toBeDefined(); - - process.env.HOME = originalHome; - process.env.USERPROFILE = originalUserProfile; - }); - - it("should use os.tmpdir() as final fallback", async () => { - const originalHome = process.env.HOME; - const originalUserProfile = process.env.USERPROFILE; - const originalCwd = process.cwd; - - delete process.env.HOME; - delete process.env.USERPROFILE; - // Mock process.cwd to throw an error - process.cwd = jest.fn().mockImplementation(() => { - throw new Error("No cwd available"); - }); - - mockExtras.ide.getWorkspaceDirs.mockResolvedValue([]); - - // Should fall back to os.tmpdir() without throwing - await expect( - runTerminalCommandImpl( - { command: "echo test", waitForCompletion: false }, - mockExtras as any, - ), - ).resolves.toBeDefined(); - - process.env.HOME = originalHome; - process.env.USERPROFILE = originalUserProfile; - process.cwd = originalCwd; - }); - }); -}); diff --git a/core/tools/implementations/runTerminalCommand.vitest.ts b/core/tools/implementations/runTerminalCommand.vitest.ts index 230d482f70c..428cc83065a 100644 --- a/core/tools/implementations/runTerminalCommand.vitest.ts +++ b/core/tools/implementations/runTerminalCommand.vitest.ts @@ -1,20 +1,21 @@ +import * as childProcess from "node:child_process"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { fileURLToPath } from "node:url"; import { + afterAll, + afterEach, + beforeEach, describe, - it, expect, - beforeEach, - afterEach, - afterAll, + it, vi, } from "vitest"; -import * as childProcess from "node:child_process"; -import * as fs from "node:fs"; -import * as os from "node:os"; -import * as path from "node:path"; import { IDE, ToolExtras } from "../.."; import * as processTerminalStates from "../../util/processTerminalStates"; -import { runTerminalCommandImpl } from "./runTerminalCommand"; import { runTerminalCommandTool } from "../definitions/runTerminalCommand"; +import { runTerminalCommandImpl } from "./runTerminalCommand"; // We're using real child processes, so ensure these aren't mocked vi.unmock("node:child_process"); @@ -476,6 +477,206 @@ describe("runTerminalCommandImpl", () => { process.cwd = originalCwd; } }); + + describe("cwd handling", () => { + describe("workspace directory handling", () => { + it("should use file:// URI when available", async () => { + const fileUri = "file:///home/user/workspace"; + mockGetWorkspaceDirs.mockResolvedValue([fileUri]); + + // We can't easily test the internal cwd without mocking child_process, + // but we can verify the function doesn't throw with file URIs + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + createMockExtras(), + ), + ).resolves.toBeDefined(); + }); + + it("should skip non-file URIs and use the first file:// URI", async () => { + const workspaceDirs = [ + "vscode-vfs://github/user/repo", + "untitled:/Untitled-1", + "file:///home/user/workspace", + "file:///home/user/other-workspace", + ]; + mockGetWorkspaceDirs.mockResolvedValue(workspaceDirs); + + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + createMockExtras(), + ), + ).resolves.toBeDefined(); + }); + + it("should handle workspace with only non-file URIs", async () => { + const workspaceDirs = [ + "vscode-vfs://github/user/repo", + "untitled:/Untitled-1", + ]; + mockGetWorkspaceDirs.mockResolvedValue(workspaceDirs); + + // Should fall back to HOME/USERPROFILE or process.cwd() without throwing + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + createMockExtras(), + ), + ).resolves.toBeDefined(); + }); + + it("should handle empty workspace directories", async () => { + mockGetWorkspaceDirs.mockResolvedValue([]); + + // Should fall back to HOME/USERPROFILE or process.cwd() without throwing + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + createMockExtras(), + ), + ).resolves.toBeDefined(); + }); + + it("should properly convert file:// URIs to paths", () => { + const fileUri = "file:///home/user/workspace"; + const expectedPath = "/home/user/workspace"; + + // Test that fileURLToPath works correctly with file:// URIs + expect(fileURLToPath(fileUri)).toBe(expectedPath); + }); + + it("should throw error when trying to convert non-file URI", () => { + const nonFileUri = "vscode-vfs://github/user/repo"; + + // This demonstrates why the fix is needed - fileURLToPath throws on non-file URIs + expect(() => fileURLToPath(nonFileUri)).toThrow(); + }); + }); + + describe("remote environment handling", () => { + it("should use ide.runCommand for non-enabled remote environments", async () => { + const extras = createMockExtras({ + remoteName: "some-unsupported-remote", + }); + + const result = await runTerminalCommandImpl( + { command: "echo test" }, + extras, + ); + + expect(mockRunCommand).toHaveBeenCalledWith("echo test"); + expect(result[0].content).toContain("Terminal output not available"); + }); + + it("should handle local environment with file URIs", async () => { + mockGetWorkspaceDirs.mockResolvedValue(["file:///home/user/workspace"]); + + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + createMockExtras({ remoteName: "local" }), + ), + ).resolves.toBeDefined(); + }); + + it("should handle WSL environment", async () => { + mockGetWorkspaceDirs.mockResolvedValue(["file:///home/user/workspace"]); + + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + createMockExtras({ remoteName: "wsl" }), + ), + ).resolves.toBeDefined(); + }); + + it("should handle dev-container environment", async () => { + mockGetWorkspaceDirs.mockResolvedValue(["file:///workspace"]); + + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + createMockExtras({ remoteName: "dev-container" }), + ), + ).resolves.toBeDefined(); + }); + }); + + describe("fallback behavior", () => { + it("should use HOME environment variable as fallback", async () => { + const originalHome = process.env.HOME; + process.env.HOME = "/home/testuser"; + + mockGetWorkspaceDirs.mockResolvedValue([ + "vscode-vfs://github/user/repo", + ]); + + try { + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + createMockExtras(), + ), + ).resolves.toBeDefined(); + } finally { + process.env.HOME = originalHome; + } + }); + + it("should use USERPROFILE on Windows as fallback", async () => { + const originalHome = process.env.HOME; + const originalUserProfile = process.env.USERPROFILE; + + delete process.env.HOME; + process.env.USERPROFILE = "C:\\Users\\TestUser"; + + mockGetWorkspaceDirs.mockResolvedValue([]); + + try { + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + createMockExtras(), + ), + ).resolves.toBeDefined(); + } finally { + process.env.HOME = originalHome; + process.env.USERPROFILE = originalUserProfile; + } + }); + + it("should use os.tmpdir() as final fallback", async () => { + const originalHome = process.env.HOME; + const originalUserProfile = process.env.USERPROFILE; + const originalCwd = process.cwd; + + delete process.env.HOME; + delete process.env.USERPROFILE; + // Mock process.cwd to throw an error + process.cwd = vi.fn().mockImplementation(() => { + throw new Error("No cwd available"); + }) as typeof process.cwd; + + mockGetWorkspaceDirs.mockResolvedValue([]); + + try { + // Should fall back to os.tmpdir() without throwing + await expect( + runTerminalCommandImpl( + { command: "echo test", waitForCompletion: false }, + createMockExtras(), + ), + ).resolves.toBeDefined(); + } finally { + process.env.HOME = originalHome; + process.env.USERPROFILE = originalUserProfile; + process.cwd = originalCwd; + } + }); + }); + }); }); describe("runTerminalCommandTool.evaluateToolCallPolicy", () => {