fix: use PATH-aware resolution for provider availability and fix Windows cmd escaping
Provider isAvailable() was using executableExists() which only checks filesystem paths, not PATH. Commands like ["claude", "--flag"] would show as unavailable even though they'd launch fine. Switch to isCommandAvailable() which uses findExecutable() for proper PATH resolution. Un-export executableExists from the public API. Fix Windows cmd.exe metacharacter escaping — &, |, ^, <, >, (, ), ! are now properly escaped with ^, and % is doubled. Add shared escapeWindowsCmdValue helper used by both quoteWindowsCommand and quoteWindowsArgument. Replace local quoteForCmd in spawn.ts. Add server-tests-windows CI job to catch Windows-specific regressions.
This commit is contained in:
@@ -76,6 +76,36 @@ jobs:
|
|||||||
CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
|
CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
|
||||||
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
|
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
|
||||||
|
|
||||||
|
server-tests-windows:
|
||||||
|
runs-on: windows-latest
|
||||||
|
steps:
|
||||||
|
- uses: actions/checkout@v4
|
||||||
|
with:
|
||||||
|
fetch-depth: 0
|
||||||
|
|
||||||
|
- uses: actions/setup-node@v4
|
||||||
|
with:
|
||||||
|
node-version: '20'
|
||||||
|
cache: 'npm'
|
||||||
|
|
||||||
|
- name: Fetch origin/main (worktree tests)
|
||||||
|
run: git fetch --no-tags origin main:refs/remotes/origin/main
|
||||||
|
|
||||||
|
- name: Install dependencies
|
||||||
|
run: npm install
|
||||||
|
|
||||||
|
- name: Build highlight dependency
|
||||||
|
run: npm run build --workspace=@getpaseo/highlight
|
||||||
|
|
||||||
|
- name: Build relay dependency
|
||||||
|
run: npm run build --workspace=@getpaseo/relay
|
||||||
|
|
||||||
|
- name: Run server tests
|
||||||
|
run: npm run test --workspace=@getpaseo/server
|
||||||
|
env:
|
||||||
|
CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
|
||||||
|
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
|
||||||
|
|
||||||
app-tests:
|
app-tests:
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
steps:
|
steps:
|
||||||
|
|||||||
@@ -20,6 +20,7 @@ const mockState = vi.hoisted(() => {
|
|||||||
env?: Record<string, string>;
|
env?: Record<string, string>;
|
||||||
}>,
|
}>,
|
||||||
},
|
},
|
||||||
|
isCommandAvailable: vi.fn(async (_command: string) => false),
|
||||||
runtimeModels: new Map<string, AgentModelDefinition[]>(),
|
runtimeModels: new Map<string, AgentModelDefinition[]>(),
|
||||||
reset() {
|
reset() {
|
||||||
for (const key of Object.keys(this.constructorArgs) as Array<
|
for (const key of Object.keys(this.constructorArgs) as Array<
|
||||||
@@ -27,11 +28,17 @@ const mockState = vi.hoisted(() => {
|
|||||||
>) {
|
>) {
|
||||||
this.constructorArgs[key] = [];
|
this.constructorArgs[key] = [];
|
||||||
}
|
}
|
||||||
|
this.isCommandAvailable.mockReset();
|
||||||
|
this.isCommandAvailable.mockImplementation(async (_command: string) => false);
|
||||||
this.runtimeModels.clear();
|
this.runtimeModels.clear();
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
});
|
});
|
||||||
|
|
||||||
|
vi.mock("../../utils/executable.js", () => ({
|
||||||
|
isCommandAvailable: mockState.isCommandAvailable,
|
||||||
|
}));
|
||||||
|
|
||||||
vi.mock("./providers/claude-agent.js", () => ({
|
vi.mock("./providers/claude-agent.js", () => ({
|
||||||
ClaudeAgentClient: class ClaudeAgentClient {
|
ClaudeAgentClient: class ClaudeAgentClient {
|
||||||
readonly capabilities = {
|
readonly capabilities = {
|
||||||
@@ -69,6 +76,12 @@ vi.mock("./providers/claude-agent.js", () => ({
|
|||||||
}
|
}
|
||||||
|
|
||||||
async isAvailable(): Promise<boolean> {
|
async isAvailable(): Promise<boolean> {
|
||||||
|
const command = (this.runtimeSettings as { command?: { mode?: string; argv?: string[] } })
|
||||||
|
?.command;
|
||||||
|
if (command?.mode === "replace") {
|
||||||
|
const { isCommandAvailable } = await import("../../utils/executable.js");
|
||||||
|
return await isCommandAvailable(command.argv?.[0] ?? "");
|
||||||
|
}
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
@@ -109,6 +122,12 @@ vi.mock("./providers/codex-app-server-agent.js", () => ({
|
|||||||
}
|
}
|
||||||
|
|
||||||
async isAvailable(): Promise<boolean> {
|
async isAvailable(): Promise<boolean> {
|
||||||
|
const command = (this.runtimeSettings as { command?: { mode?: string; argv?: string[] } })
|
||||||
|
?.command;
|
||||||
|
if (command?.mode === "replace") {
|
||||||
|
const { isCommandAvailable } = await import("../../utils/executable.js");
|
||||||
|
return await isCommandAvailable(command.argv?.[0] ?? "");
|
||||||
|
}
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
@@ -151,6 +170,12 @@ vi.mock("./providers/copilot-acp-agent.js", () => ({
|
|||||||
}
|
}
|
||||||
|
|
||||||
async isAvailable(): Promise<boolean> {
|
async isAvailable(): Promise<boolean> {
|
||||||
|
const command = (this.runtimeSettings as { command?: { mode?: string; argv?: string[] } })
|
||||||
|
?.command;
|
||||||
|
if (command?.mode === "replace") {
|
||||||
|
const { isCommandAvailable } = await import("../../utils/executable.js");
|
||||||
|
return await isCommandAvailable(command.argv?.[0] ?? "");
|
||||||
|
}
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
@@ -433,6 +458,21 @@ describe("buildProviderRegistry", () => {
|
|||||||
expect(registry.claude).toBeUndefined();
|
expect(registry.claude).toBeUndefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("provider override command can be PATH-resolved and still report available", async () => {
|
||||||
|
mockState.isCommandAvailable.mockResolvedValue(true);
|
||||||
|
|
||||||
|
const registry = buildProviderRegistry(logger, {
|
||||||
|
providerOverrides: {
|
||||||
|
claude: {
|
||||||
|
command: ["claude", "--flag"],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(registry.claude.createClient(logger).isAvailable()).resolves.toBe(true);
|
||||||
|
expect(mockState.isCommandAvailable).toHaveBeenCalledWith("claude");
|
||||||
|
});
|
||||||
|
|
||||||
test("extension inherits base override — override claude command, zai extends claude gets overridden command", () => {
|
test("extension inherits base override — override claude command, zai extends claude gets overridden command", () => {
|
||||||
buildProviderRegistry(logger, {
|
buildProviderRegistry(logger, {
|
||||||
providerOverrides: {
|
providerOverrides: {
|
||||||
|
|||||||
@@ -72,7 +72,7 @@ import type {
|
|||||||
PersistedAgentDescriptor,
|
PersistedAgentDescriptor,
|
||||||
} from "../agent-sdk-types.js";
|
} from "../agent-sdk-types.js";
|
||||||
import { applyProviderEnv, type ProviderRuntimeSettings } from "../provider-launch-config.js";
|
import { applyProviderEnv, type ProviderRuntimeSettings } from "../provider-launch-config.js";
|
||||||
import { executableExists, findExecutable } from "../../../utils/executable.js";
|
import { findExecutable, isCommandAvailable } from "../../../utils/executable.js";
|
||||||
import { execCommand, spawnProcess } from "../../../utils/spawn.js";
|
import { execCommand, spawnProcess } from "../../../utils/spawn.js";
|
||||||
import { getOrchestratorModeInstructions } from "../orchestrator-instructions.js";
|
import { getOrchestratorModeInstructions } from "../orchestrator-instructions.js";
|
||||||
|
|
||||||
@@ -1128,7 +1128,7 @@ export class ClaudeAgentClient implements AgentClient {
|
|||||||
async isAvailable(): Promise<boolean> {
|
async isAvailable(): Promise<boolean> {
|
||||||
const command = this.runtimeSettings?.command;
|
const command = this.runtimeSettings?.command;
|
||||||
if (command?.mode === "replace") {
|
if (command?.mode === "replace") {
|
||||||
return executableExists(command.argv[0]) !== null;
|
return await isCommandAvailable(command.argv[0]);
|
||||||
}
|
}
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -47,7 +47,7 @@ import {
|
|||||||
resolveProviderCommandPrefix,
|
resolveProviderCommandPrefix,
|
||||||
type ProviderRuntimeSettings,
|
type ProviderRuntimeSettings,
|
||||||
} from "../provider-launch-config.js";
|
} from "../provider-launch-config.js";
|
||||||
import { executableExists, findExecutable } from "../../../utils/executable.js";
|
import { findExecutable, isCommandAvailable } from "../../../utils/executable.js";
|
||||||
import { spawnProcess } from "../../../utils/spawn.js";
|
import { spawnProcess } from "../../../utils/spawn.js";
|
||||||
import { extractCodexTerminalSessionId, nonEmptyString } from "./tool-call-mapper-utils.js";
|
import { extractCodexTerminalSessionId, nonEmptyString } from "./tool-call-mapper-utils.js";
|
||||||
import { buildCodexFeatures, codexModelSupportsFastMode } from "./codex-feature-definitions.js";
|
import { buildCodexFeatures, codexModelSupportsFastMode } from "./codex-feature-definitions.js";
|
||||||
@@ -4203,7 +4203,7 @@ export class CodexAppServerAgentClient implements AgentClient {
|
|||||||
async isAvailable(): Promise<boolean> {
|
async isAvailable(): Promise<boolean> {
|
||||||
const command = this.runtimeSettings?.command;
|
const command = this.runtimeSettings?.command;
|
||||||
if (command?.mode === "replace") {
|
if (command?.mode === "replace") {
|
||||||
return executableExists(command.argv[0]) !== null;
|
return await isCommandAvailable(command.argv[0]);
|
||||||
}
|
}
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -43,7 +43,7 @@ import {
|
|||||||
resolveProviderCommandPrefix,
|
resolveProviderCommandPrefix,
|
||||||
type ProviderRuntimeSettings,
|
type ProviderRuntimeSettings,
|
||||||
} from "../provider-launch-config.js";
|
} from "../provider-launch-config.js";
|
||||||
import { executableExists, findExecutable } from "../../../utils/executable.js";
|
import { findExecutable, isCommandAvailable } from "../../../utils/executable.js";
|
||||||
import { spawnProcess } from "../../../utils/spawn.js";
|
import { spawnProcess } from "../../../utils/spawn.js";
|
||||||
import { mapOpencodeToolCall } from "./opencode/tool-call-mapper.js";
|
import { mapOpencodeToolCall } from "./opencode/tool-call-mapper.js";
|
||||||
import {
|
import {
|
||||||
@@ -916,7 +916,7 @@ export class OpenCodeAgentClient implements AgentClient {
|
|||||||
async isAvailable(): Promise<boolean> {
|
async isAvailable(): Promise<boolean> {
|
||||||
const command = this.runtimeSettings?.command;
|
const command = this.runtimeSettings?.command;
|
||||||
if (command?.mode === "replace") {
|
if (command?.mode === "replace") {
|
||||||
return executableExists(command.argv[0]) !== null;
|
return await isCommandAvailable(command.argv[0]);
|
||||||
}
|
}
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -37,7 +37,6 @@ export {
|
|||||||
} from "./agent/provider-launch-config.js";
|
} from "./agent/provider-launch-config.js";
|
||||||
export {
|
export {
|
||||||
findExecutable,
|
findExecutable,
|
||||||
executableExists,
|
|
||||||
quoteWindowsArgument,
|
quoteWindowsArgument,
|
||||||
quoteWindowsCommand,
|
quoteWindowsCommand,
|
||||||
} from "../utils/executable.js";
|
} from "../utils/executable.js";
|
||||||
|
|||||||
@@ -240,6 +240,46 @@ describe("quoteWindowsCommand", () => {
|
|||||||
expect(quoteWindowsCommand("C:\\nvm4w\\nodejs\\codex")).toBe("C:\\nvm4w\\nodejs\\codex");
|
expect(quoteWindowsCommand("C:\\nvm4w\\nodejs\\codex")).toBe("C:\\nvm4w\\nodejs\\codex");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("escapes ampersands", async () => {
|
||||||
|
setPlatform("win32");
|
||||||
|
const { quoteWindowsCommand } = await loadExecutableModule();
|
||||||
|
expect(quoteWindowsCommand("feature&bugfix")).toBe("feature^&bugfix");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("escapes pipes", async () => {
|
||||||
|
setPlatform("win32");
|
||||||
|
const { quoteWindowsCommand } = await loadExecutableModule();
|
||||||
|
expect(quoteWindowsCommand("feature|bugfix")).toBe("feature^|bugfix");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("doubles percent signs", async () => {
|
||||||
|
setPlatform("win32");
|
||||||
|
const { quoteWindowsCommand } = await loadExecutableModule();
|
||||||
|
expect(quoteWindowsCommand("100%")).toBe("100%%");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("escapes carets", async () => {
|
||||||
|
setPlatform("win32");
|
||||||
|
const { quoteWindowsCommand } = await loadExecutableModule();
|
||||||
|
expect(quoteWindowsCommand("feature^bugfix")).toBe("feature^^bugfix");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("escapes multiple metacharacters", async () => {
|
||||||
|
setPlatform("win32");
|
||||||
|
const { quoteWindowsCommand } = await loadExecutableModule();
|
||||||
|
expect(quoteWindowsCommand("build&(test|deploy)!<output>")).toBe(
|
||||||
|
"build^&^(test^|deploy^)^!^<output^>",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("quotes commands with spaces after escaping metacharacters", async () => {
|
||||||
|
setPlatform("win32");
|
||||||
|
const { quoteWindowsCommand } = await loadExecutableModule();
|
||||||
|
expect(quoteWindowsCommand("C:\\Program Files\\My Tool&Stuff\\run 100%.cmd")).toBe(
|
||||||
|
'"C:\\Program Files\\My Tool^&Stuff\\run 100%%.cmd"',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
test("returns the command unchanged on non-Windows platforms", async () => {
|
test("returns the command unchanged on non-Windows platforms", async () => {
|
||||||
setPlatform("darwin");
|
setPlatform("darwin");
|
||||||
const { quoteWindowsCommand } = await loadExecutableModule();
|
const { quoteWindowsCommand } = await loadExecutableModule();
|
||||||
|
|||||||
@@ -105,6 +105,20 @@ export async function isCommandAvailable(command: string): Promise<boolean> {
|
|||||||
return (await findExecutable(command)) !== null;
|
return (await findExecutable(command)) !== null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function escapeWindowsCmdValue(value: string): string {
|
||||||
|
if (process.platform !== "win32") return value;
|
||||||
|
|
||||||
|
const isQuoted = value.startsWith('"') && value.endsWith('"');
|
||||||
|
const unquoted = isQuoted ? value.slice(1, -1) : value;
|
||||||
|
const escaped = unquoted.replace(/%/g, "%%").replace(/([&|^<>()!])/g, "^$1");
|
||||||
|
|
||||||
|
if (isQuoted || escaped.includes(" ")) {
|
||||||
|
return `"${escaped}"`;
|
||||||
|
}
|
||||||
|
|
||||||
|
return escaped;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* When spawning with `shell: true` on Windows, the command is passed to
|
* When spawning with `shell: true` on Windows, the command is passed to
|
||||||
* `cmd.exe /d /s /c "command args"`. The `/s` strips outer quotes, so a
|
* `cmd.exe /d /s /c "command args"`. The `/s` strips outer quotes, so a
|
||||||
@@ -112,10 +126,7 @@ export async function isCommandAvailable(command: string): Promise<boolean> {
|
|||||||
* space. Wrapping it in quotes produces the correct `"C:\Program Files\..." args`.
|
* space. Wrapping it in quotes produces the correct `"C:\Program Files\..." args`.
|
||||||
*/
|
*/
|
||||||
export function quoteWindowsCommand(command: string): string {
|
export function quoteWindowsCommand(command: string): string {
|
||||||
if (process.platform !== "win32") return command;
|
return escapeWindowsCmdValue(command);
|
||||||
if (!command.includes(" ")) return command;
|
|
||||||
if (command.startsWith('"') && command.endsWith('"')) return command;
|
|
||||||
return `"${command}"`;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -124,8 +135,5 @@ export function quoteWindowsCommand(command: string): string {
|
|||||||
* child process sees it.
|
* child process sees it.
|
||||||
*/
|
*/
|
||||||
export function quoteWindowsArgument(argument: string): string {
|
export function quoteWindowsArgument(argument: string): string {
|
||||||
if (process.platform !== "win32") return argument;
|
return escapeWindowsCmdValue(argument);
|
||||||
if (!argument.includes(" ")) return argument;
|
|
||||||
if (argument.startsWith('"') && argument.endsWith('"')) return argument;
|
|
||||||
return `"${argument}"`;
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
import { execFile, spawn, type ChildProcess, type SpawnOptions } from "node:child_process";
|
import { execFile, spawn, type ChildProcess, type SpawnOptions } from "node:child_process";
|
||||||
import { promisify } from "node:util";
|
import { promisify } from "node:util";
|
||||||
|
|
||||||
|
import { quoteWindowsArgument, quoteWindowsCommand } from "./executable.js";
|
||||||
|
|
||||||
const execFileAsync = promisify(execFile);
|
const execFileAsync = promisify(execFile);
|
||||||
|
|
||||||
interface ExecCommandOptions {
|
interface ExecCommandOptions {
|
||||||
@@ -16,12 +18,6 @@ interface ExecCommandResult {
|
|||||||
stderr: string;
|
stderr: string;
|
||||||
}
|
}
|
||||||
|
|
||||||
function quoteForCmd(value: string): string {
|
|
||||||
if (!value.includes(" ")) return value;
|
|
||||||
if (value.startsWith('"') && value.endsWith('"')) return value;
|
|
||||||
return `"${value}"`;
|
|
||||||
}
|
|
||||||
|
|
||||||
export function spawnProcess(
|
export function spawnProcess(
|
||||||
command: string,
|
command: string,
|
||||||
args: string[],
|
args: string[],
|
||||||
@@ -29,8 +25,8 @@ export function spawnProcess(
|
|||||||
): ChildProcess {
|
): ChildProcess {
|
||||||
const isWindows = process.platform === "win32";
|
const isWindows = process.platform === "win32";
|
||||||
|
|
||||||
const resolvedCommand = isWindows ? quoteForCmd(command) : command;
|
const resolvedCommand = isWindows ? quoteWindowsCommand(command) : command;
|
||||||
const resolvedArgs = isWindows ? args.map(quoteForCmd) : args;
|
const resolvedArgs = isWindows ? args.map(quoteWindowsArgument) : args;
|
||||||
|
|
||||||
return spawn(resolvedCommand, resolvedArgs, {
|
return spawn(resolvedCommand, resolvedArgs, {
|
||||||
...options,
|
...options,
|
||||||
@@ -45,8 +41,8 @@ export async function execCommand(
|
|||||||
options?: ExecCommandOptions,
|
options?: ExecCommandOptions,
|
||||||
): Promise<ExecCommandResult> {
|
): Promise<ExecCommandResult> {
|
||||||
const isWindows = process.platform === "win32";
|
const isWindows = process.platform === "win32";
|
||||||
const resolvedCommand = isWindows ? quoteForCmd(command) : command;
|
const resolvedCommand = isWindows ? quoteWindowsCommand(command) : command;
|
||||||
const resolvedArgs = isWindows ? args.map(quoteForCmd) : args;
|
const resolvedArgs = isWindows ? args.map(quoteWindowsArgument) : args;
|
||||||
|
|
||||||
return execFileAsync(resolvedCommand, resolvedArgs, {
|
return execFileAsync(resolvedCommand, resolvedArgs, {
|
||||||
cwd: options?.cwd,
|
cwd: options?.cwd,
|
||||||
|
|||||||
Reference in New Issue
Block a user