diff --git a/.oxlintrc.json b/.oxlintrc.json index 269097aab4..d1566195e9 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -84,6 +84,45 @@ "max-nested-callbacks": ["error", { "max": 3 }] }, "overrides": [ + { + "files": [ + "plugins/*-provider/server/**/*.ts", + "packages/plugin/src/server/acp-internal/**/*.ts" + ], + "rules": { + "no-restricted-imports": [ + "error", + { + "paths": [ + { + "name": "node:child_process", + "importNames": [ + "spawn", + "exec", + "execFile", + "spawnSync", + "execSync", + "execFileSync" + ], + "message": "Use the plugin server process helpers so Windows command scripts and argv are handled consistently." + }, + { + "name": "child_process", + "importNames": [ + "spawn", + "exec", + "execFile", + "spawnSync", + "execSync", + "execFileSync" + ], + "message": "Use the plugin server process helpers so Windows command scripts and argv are handled consistently." + } + ] + } + ] + } + }, { "files": ["packages/app/src/**/*.{ts,tsx}"], "rules": { diff --git a/docs/plugins.md b/docs/plugins.md index 510ef68af1..401e33b94b 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -248,7 +248,7 @@ dependency; shared types must not refer to React components, hooks, Node APIs, o | Entry | Owns | May depend on | | ---------------------------------------------------- | -------------------------------------------------------------------------- | ---------------------- | | `@getpaseo/plugin` | Shared data, schemas, RPC/settings definitions, runtime-neutral helpers | Shared code only | -| `@getpaseo/plugin/server` | Server contribution/handler contexts and lifecycle contracts | Shared and server code | +| `@getpaseo/plugin/server` | Server contexts, lifecycle contracts, and CLI process launch | Shared and server code | | `@getpaseo/plugin/server/provider`, `/server/acp` | Server provider contracts and adapters | Shared and server code | | `@getpaseo/plugin/client` | Client contribution contexts, hooks, navigation, and UI contribution types | Shared and client code | | `@getpaseo/plugin/client/react-native`, `/client/ui` | Host-provided UI components | Shared and client code | diff --git a/docs/providers.md b/docs/providers.md index 47c39fd1a8..c5f37d4161 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -10,7 +10,9 @@ Keep a bundled provider in `plugins//` and register it through import rules belong to [plugins.md](plugins.md#built-in-plugins); the [public provider guide](../public-docs/plugins/providers.md) covers the provider contract. -The plugin owns the CLI transport, session state, catalog, and capabilities. The daemon owns +The plugin owns the CLI transport, session state, catalog, and capabilities. Launch CLI transports +and probes through the [SDK process helpers](../public-docs/plugins/providers.md#launch-the-provider-cli), +which share Windows launcher handling with core providers. The daemon owns executable resolution and applies `agents.providers..command` and `env` before connecting. Register the provider's icon with the plugin rather than adding it to the app's provider icon map. You do not need a core manifest entry or provider factory. diff --git a/packages/plugin/src/boundaries.test.ts b/packages/plugin/src/boundaries.test.ts index 38197e4cb6..5b1ce4f3dc 100644 --- a/packages/plugin/src/boundaries.test.ts +++ b/packages/plugin/src/boundaries.test.ts @@ -4,6 +4,7 @@ import { isBuiltin } from "node:module"; import { fileURLToPath } from "node:url"; import ts from "typescript"; import { describe, expect, it } from "vitest"; +import * as server from "./server/index.js"; const sourceDirectory = path.dirname(fileURLToPath(import.meta.url)); const entries = { @@ -64,6 +65,10 @@ function boundaryViolations(entry: string, runtime: "shared" | "server" | "clien } describe("plugin SDK import boundaries", () => { + it("publishes process capabilities without their Windows implementation helpers", () => { + expect(Object.keys(server).sort()).toEqual(["execCommand", "spawnProcess", "terminateProcess"]); + }); + it("classifies every published entry", () => { const manifest = JSON.parse(readFileSync(new URL("../package.json", import.meta.url), "utf8")); expect(Object.keys(manifest.exports).sort()).toEqual(Object.keys(entries).sort()); diff --git a/packages/plugin/src/server/acp-internal/connection.ts b/packages/plugin/src/server/acp-internal/connection.ts index a71f2ca696..fc78d2a8c4 100644 --- a/packages/plugin/src/server/acp-internal/connection.ts +++ b/packages/plugin/src/server/acp-internal/connection.ts @@ -1,4 +1,5 @@ -import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; +import type { ChildProcessWithoutNullStreams } from "node:child_process"; +import { spawnProcess, terminateProcess } from "../process.js"; import { Readable, Writable } from "node:stream"; import { ClientSideConnection, @@ -400,9 +401,9 @@ class AcpRuntime { args: options.options.command.slice(1), env: process.env, }; - child = spawn(launch.command, launch.args, { + child = spawnProcess(launch.command, launch.args, { env: { ...launch.env, ...options.env }, - stdio: ["pipe", "pipe", "pipe"], + stdio: "pipe", }); spawnFailure = new Promise((_resolve, reject) => child!.once("error", reject)); child.stderr.on("data", () => undefined); @@ -734,9 +735,9 @@ class AcpRuntime { if (!child || this.processFailed) return; if (child.exitCode !== null || child.signalCode !== null) return; const closed = new Promise((resolve) => child.once("close", () => resolve())); - child.kill("SIGTERM"); + await terminateProcess(child, "SIGTERM"); if (await settlesWithin(closed, 1_000)) return; - child.kill("SIGKILL"); + await terminateProcess(child); if (!(await settlesWithin(closed, 1_000))) { throw new Error(`ACP provider ${this.options.options.id} did not terminate after SIGKILL`); } diff --git a/packages/plugin/src/server/index.ts b/packages/plugin/src/server/index.ts index dc211f61bf..9180023d19 100644 --- a/packages/plugin/src/server/index.ts +++ b/packages/plugin/src/server/index.ts @@ -22,3 +22,5 @@ export type { PluginBeforeRequests, PluginLifecycleRegistration, } from "./lifecycle.js"; + +export { spawnProcess, execCommand, terminateProcess } from "./process.js"; diff --git a/packages/plugin/src/server/process-internal/windows-command.test.ts b/packages/plugin/src/server/process-internal/windows-command.test.ts new file mode 100644 index 0000000000..3eb5e2e63b --- /dev/null +++ b/packages/plugin/src/server/process-internal/windows-command.test.ts @@ -0,0 +1,117 @@ +import { afterEach, describe, expect, test } from "vitest"; +import { quoteWindowsArgument, quoteWindowsCommand } from "./windows-command.js"; + +describe("quoteWindowsCommand", () => { + const originalPlatform = process.platform; + + function setPlatform(value: string) { + Object.defineProperty(process, "platform", { value, writable: true }); + } + + afterEach(() => { + setPlatform(originalPlatform); + }); + + test("quotes a Windows path with spaces", () => { + setPlatform("win32"); + expect(quoteWindowsCommand("C:\\Program Files\\Anthropic\\claude.exe")).toBe( + '"C:\\Program Files\\Anthropic\\claude.exe"', + ); + }); + + test("does not double-quote an already-quoted path", () => { + setPlatform("win32"); + expect(quoteWindowsCommand('"C:\\Program Files\\Anthropic\\claude.exe"')).toBe( + '"C:\\Program Files\\Anthropic\\claude.exe"', + ); + }); + + test("returns the command unchanged when there are no spaces", () => { + setPlatform("win32"); + expect(quoteWindowsCommand("C:\\nvm4w\\nodejs\\codex")).toBe("C:\\nvm4w\\nodejs\\codex"); + }); + + test("escapes ampersands", () => { + setPlatform("win32"); + expect(quoteWindowsCommand("feature&bugfix")).toBe("feature^&bugfix"); + }); + + test("escapes pipes", () => { + setPlatform("win32"); + expect(quoteWindowsCommand("feature|bugfix")).toBe("feature^|bugfix"); + }); + + test("does not double percent signs", () => { + setPlatform("win32"); + // cmd.exe only collapses %% → % inside batch files; on the command line + // it stays literal, which corrupts git --format atoms etc. + expect(quoteWindowsCommand("100%")).toBe("100%"); + }); + + test("preserves git --format atoms verbatim", () => { + setPlatform("win32"); + expect(quoteWindowsCommand("--format=%(refname)%09%(committerdate:unix)")).toBe( + "--format=%^(refname^)%09%^(committerdate:unix^)", + ); + }); + + test("escapes carets", () => { + setPlatform("win32"); + expect(quoteWindowsCommand("feature^bugfix")).toBe("feature^^bugfix"); + }); + + test("escapes multiple metacharacters", () => { + setPlatform("win32"); + expect(quoteWindowsCommand("build&(test|deploy)!")).toBe( + "build^&^(test^|deploy^)^!^", + ); + }); + + test("quotes commands with spaces after escaping metacharacters", () => { + setPlatform("win32"); + 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", () => { + setPlatform("darwin"); + expect(quoteWindowsCommand("/usr/local/bin/claude code")).toBe("/usr/local/bin/claude code"); + }); +}); + +describe("quoteWindowsArgument", () => { + const originalPlatform = process.platform; + + function setPlatform(value: string) { + Object.defineProperty(process, "platform", { value, writable: true }); + } + + afterEach(() => { + setPlatform(originalPlatform); + }); + + test("quotes a Windows argument with spaces", () => { + setPlatform("win32"); + expect(quoteWindowsArgument("C:\\Program Files\\Anthropic\\cli.js")).toBe( + '"C:\\Program Files\\Anthropic\\cli.js"', + ); + }); + + test("does not double-quote an already-quoted argument", () => { + setPlatform("win32"); + expect(quoteWindowsArgument('"C:\\Program Files\\Anthropic\\cli.js"')).toBe( + '"C:\\Program Files\\Anthropic\\cli.js"', + ); + }); + + test("returns the argument unchanged when there are no spaces", () => { + setPlatform("win32"); + expect(quoteWindowsArgument("--version")).toBe("--version"); + }); + + test("returns the argument unchanged on non-Windows platforms", () => { + setPlatform("darwin"); + expect(quoteWindowsArgument("/usr/local/bin/claude code")).toBe("/usr/local/bin/claude code"); + }); +}); diff --git a/packages/server/src/utils/windows-command.ts b/packages/plugin/src/server/process-internal/windows-command.ts similarity index 100% rename from packages/server/src/utils/windows-command.ts rename to packages/plugin/src/server/process-internal/windows-command.ts diff --git a/packages/plugin/src/server/process.test.ts b/packages/plugin/src/server/process.test.ts new file mode 100644 index 0000000000..e9b3000da5 --- /dev/null +++ b/packages/plugin/src/server/process.test.ts @@ -0,0 +1,53 @@ +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, expect, test } from "vitest"; +import { execCommand, spawnProcess } from "./index.js"; + +const roots: string[] = []; +afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))); +}); + +async function writeArgumentReader(): Promise { + const root = await mkdtemp(path.join(os.tmpdir(), "plugin process arguments ")); + roots.push(root); + const script = path.join(root, "read arguments.mjs"); + await writeFile(script, "process.stdout.write(JSON.stringify(process.argv.slice(2)));\n"); + return script; +} + +const argumentsToPreserve = [ + "argument with spaces", + "--format=%(refname)%09%(committerdate:unix)", + "100%", + "feature&bugfix", + "feature|bugfix", + "feature^bugfix", +]; + +// A bare extensionless command selects the Windows shell; an absolute node.exe path would +// skip that path. The real child checks quoting through the public process capabilities. +test("execCommand delivers shell arguments to the real child unchanged", async () => { + const script = await writeArgumentReader(); + const { stdout } = await execCommand("node", [script, ...argumentsToPreserve]); + expect(JSON.parse(stdout)).toEqual(argumentsToPreserve); +}); + +test("spawnProcess delivers shell arguments to the real child unchanged", async () => { + const script = await writeArgumentReader(); + const child = spawnProcess("node", [script, ...argumentsToPreserve], { stdio: "pipe" }); + let stdout = ""; + child.stdout.setEncoding("utf8"); + child.stdout.on("data", (chunk: string) => { + stdout += chunk; + }); + await new Promise((resolve, reject) => { + child.once("error", reject); + child.once("close", (code) => { + if (code === 0) resolve(); + else reject(new Error(`Argument reader exited with ${code}`)); + }); + }); + expect(JSON.parse(stdout)).toEqual(argumentsToPreserve); +}); diff --git a/packages/plugin/src/server/process.ts b/packages/plugin/src/server/process.ts new file mode 100644 index 0000000000..b2ba70ac12 --- /dev/null +++ b/packages/plugin/src/server/process.ts @@ -0,0 +1,119 @@ +import { + execFile, + spawn, + type ChildProcess, + type ChildProcessWithoutNullStreams, + type SpawnOptions, +} from "node:child_process"; +import { extname } from "node:path"; +import { promisify } from "node:util"; +import { + isWindowsCommandScript, + quoteWindowsArgument, + quoteWindowsCommand, +} from "./process-internal/windows-command.js"; +const execFileAsync = promisify(execFile); +export interface ExecCommandOptions { + env?: NodeJS.ProcessEnv; + cwd?: string; + encoding?: BufferEncoding; + killSignal?: NodeJS.Signals; + timeout?: number; + maxBuffer?: number; + shell?: boolean | string; + signal?: AbortSignal; +} +interface ExecCommandResult { + stdout: string; + stderr: string; +} +function hasPathSeparator(value: string): boolean { + return value.includes("/") || value.includes("\\"); +} + +function shouldUseWindowsShell( + command: string, + requestedShell?: boolean | string, +): boolean | string { + if (isWindowsCommandScript(command)) { + return true; + } + if (requestedShell !== undefined) { + return requestedShell; + } + return process.platform === "win32" && !hasPathSeparator(command) && !extname(command); +} + +/** Launch a CLI with Windows command-script handling and argv quoting. The caller owns env. */ +export function spawnProcess( + command: string, + args: string[], + options: SpawnOptions & { stdio: "pipe" }, +): ChildProcessWithoutNullStreams; +export function spawnProcess(command: string, args: string[], options?: SpawnOptions): ChildProcess; +export function spawnProcess( + command: string, + args: string[], + options?: SpawnOptions, +): ChildProcess { + const spawnOptions = options ?? {}; + const isWindows = process.platform === "win32"; + const shell = shouldUseWindowsShell(command, spawnOptions.shell); + + const shouldQuoteForShell = isWindows && shell !== false; + const resolvedCommand = shouldQuoteForShell ? quoteWindowsCommand(command) : command; + const resolvedArgs = shouldQuoteForShell ? args.map(quoteWindowsArgument) : args; + + return spawn(resolvedCommand, resolvedArgs, { + ...spawnOptions, + env: options?.env, + shell, + signal: options?.signal, + windowsHide: true, + }); +} + +export async function execCommand( + command: string, + args: string[], + options?: ExecCommandOptions, +): Promise { + const isWindows = process.platform === "win32"; + const shell = shouldUseWindowsShell(command, options?.shell); + const shouldQuoteForShell = isWindows && shell !== false; + const resolvedCommand = shouldQuoteForShell ? quoteWindowsCommand(command) : command; + const resolvedArgs = shouldQuoteForShell ? args.map(quoteWindowsArgument) : args; + + return execFileAsync(resolvedCommand, resolvedArgs, { + cwd: options?.cwd, + env: options?.env, + encoding: options?.encoding ?? "utf8", + signal: options?.signal, + killSignal: options?.killSignal, + timeout: options?.timeout, + maxBuffer: options?.maxBuffer, + shell, + windowsHide: true, + }) as Promise; +} + +/** Force-stop a CLI, including descendants behind Windows command-script launchers. */ +export async function terminateProcess( + child: ChildProcess, + signal: NodeJS.Signals = "SIGKILL", +): Promise { + if (process.platform !== "win32") { + child.kill(signal); + return; + } + if (child.pid === undefined) return; + try { + await execCommand("taskkill.exe", ["/PID", String(child.pid), "/T", "/F"], { + shell: false, + timeout: 5000, + }); + } catch (error) { + // The process can finish between its close check and taskkill. + if (!(error instanceof Error && "code" in error && error.code === 128)) throw error; + } +} diff --git a/packages/server/src/executable-resolution/executable-resolution.test.ts b/packages/server/src/executable-resolution/executable-resolution.test.ts index 4434089e98..2ee0bd90df 100644 --- a/packages/server/src/executable-resolution/executable-resolution.test.ts +++ b/packages/server/src/executable-resolution/executable-resolution.test.ts @@ -3,12 +3,7 @@ import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, test, vi } from "vitest"; -import { - executableExists, - findExecutable, - quoteWindowsArgument, - quoteWindowsCommand, -} from "./executable-resolution.js"; +import { executableExists, findExecutable } from "./executable-resolution.js"; import { windowsExecutableResolution } from "./windows.js"; import { isPlatform } from "../test-utils/platform.js"; @@ -217,118 +212,3 @@ describe("executableExists", () => { expect(executableExists("/missing/codex", () => false)).toBeNull(); }); }); - -describe("quoteWindowsCommand", () => { - const originalPlatform = process.platform; - - function setPlatform(value: string) { - Object.defineProperty(process, "platform", { value, writable: true }); - } - - afterEach(() => { - setPlatform(originalPlatform); - }); - - test("quotes a Windows path with spaces", () => { - setPlatform("win32"); - expect(quoteWindowsCommand("C:\\Program Files\\Anthropic\\claude.exe")).toBe( - '"C:\\Program Files\\Anthropic\\claude.exe"', - ); - }); - - test("does not double-quote an already-quoted path", () => { - setPlatform("win32"); - expect(quoteWindowsCommand('"C:\\Program Files\\Anthropic\\claude.exe"')).toBe( - '"C:\\Program Files\\Anthropic\\claude.exe"', - ); - }); - - test("returns the command unchanged when there are no spaces", () => { - setPlatform("win32"); - expect(quoteWindowsCommand("C:\\nvm4w\\nodejs\\codex")).toBe("C:\\nvm4w\\nodejs\\codex"); - }); - - test("escapes ampersands", () => { - setPlatform("win32"); - expect(quoteWindowsCommand("feature&bugfix")).toBe("feature^&bugfix"); - }); - - test("escapes pipes", () => { - setPlatform("win32"); - expect(quoteWindowsCommand("feature|bugfix")).toBe("feature^|bugfix"); - }); - - test("does not double percent signs", () => { - setPlatform("win32"); - // cmd.exe only collapses %% → % inside batch files; on the command line - // it stays literal, which corrupts git --format atoms etc. - expect(quoteWindowsCommand("100%")).toBe("100%"); - }); - - test("preserves git --format atoms verbatim", () => { - setPlatform("win32"); - expect(quoteWindowsCommand("--format=%(refname)%09%(committerdate:unix)")).toBe( - "--format=%^(refname^)%09%^(committerdate:unix^)", - ); - }); - - test("escapes carets", () => { - setPlatform("win32"); - expect(quoteWindowsCommand("feature^bugfix")).toBe("feature^^bugfix"); - }); - - test("escapes multiple metacharacters", () => { - setPlatform("win32"); - expect(quoteWindowsCommand("build&(test|deploy)!")).toBe( - "build^&^(test^|deploy^)^!^", - ); - }); - - test("quotes commands with spaces after escaping metacharacters", () => { - setPlatform("win32"); - 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", () => { - setPlatform("darwin"); - expect(quoteWindowsCommand("/usr/local/bin/claude code")).toBe("/usr/local/bin/claude code"); - }); -}); - -describe("quoteWindowsArgument", () => { - const originalPlatform = process.platform; - - function setPlatform(value: string) { - Object.defineProperty(process, "platform", { value, writable: true }); - } - - afterEach(() => { - setPlatform(originalPlatform); - }); - - test("quotes a Windows argument with spaces", () => { - setPlatform("win32"); - expect(quoteWindowsArgument("C:\\Program Files\\Anthropic\\cli.js")).toBe( - '"C:\\Program Files\\Anthropic\\cli.js"', - ); - }); - - test("does not double-quote an already-quoted argument", () => { - setPlatform("win32"); - expect(quoteWindowsArgument('"C:\\Program Files\\Anthropic\\cli.js"')).toBe( - '"C:\\Program Files\\Anthropic\\cli.js"', - ); - }); - - test("returns the argument unchanged when there are no spaces", () => { - setPlatform("win32"); - expect(quoteWindowsArgument("--version")).toBe("--version"); - }); - - test("returns the argument unchanged on non-Windows platforms", () => { - setPlatform("darwin"); - expect(quoteWindowsArgument("/usr/local/bin/claude code")).toBe("/usr/local/bin/claude code"); - }); -}); diff --git a/packages/server/src/executable-resolution/executable-resolution.ts b/packages/server/src/executable-resolution/executable-resolution.ts index 75f230409f..59c218511b 100644 --- a/packages/server/src/executable-resolution/executable-resolution.ts +++ b/packages/server/src/executable-resolution/executable-resolution.ts @@ -2,11 +2,8 @@ import { createRequire } from "node:module"; import { existsSync } from "node:fs"; import type { ProcessEnvRecord } from "../server/paseo-env.js"; import { execCommand } from "../utils/spawn.js"; -import { isWindowsCommandScript } from "../utils/windows-command.js"; import { windowsExecutableResolution } from "./windows.js"; -export { quoteWindowsArgument, quoteWindowsCommand } from "../utils/windows-command.js"; - type Which = ( command: string, options: { all: true; path?: string; pathExt?: string }, @@ -82,7 +79,6 @@ export async function probeExecutable( timeout: timeoutMs, killSignal: "SIGKILL", maxBuffer: 64 * 1024, - shell: isWindowsCommandScript(executablePath), }); return true; } catch (error) { diff --git a/packages/server/src/server/plugins/bundle-evaluator.ts b/packages/server/src/server/plugins/bundle-evaluator.ts index 2941d4c4ff..e80db885ff 100644 --- a/packages/server/src/server/plugins/bundle-evaluator.ts +++ b/packages/server/src/server/plugins/bundle-evaluator.ts @@ -3,6 +3,7 @@ import * as pluginSharedRuntime from "@getpaseo/plugin"; import * as pluginProviderRuntime from "@getpaseo/plugin/server/provider"; import * as pluginAcpRuntime from "@getpaseo/plugin/server/acp"; import * as pluginUsageRuntime from "@getpaseo/plugin/server/usage"; +import * as pluginServerRuntime from "@getpaseo/plugin/server"; import type { PluginServerContribution } from "@getpaseo/plugin/server"; import * as zod from "zod"; import { isPluginClientOnlySdkSpecifier } from "./plugin-sdk-specifiers.js"; @@ -14,7 +15,7 @@ function runtimeRequire(name: string): unknown { throw new Error(`${name} is available only in plugin client code`); } if (name === "@getpaseo/plugin") return pluginSharedRuntime; - if (name === "@getpaseo/plugin/server") return {}; + if (name === "@getpaseo/plugin/server") return pluginServerRuntime; if (name === "@getpaseo/plugin/server/provider") return pluginProviderRuntime; if (name === "@getpaseo/plugin/server/acp") return pluginAcpRuntime; if (name === "@getpaseo/plugin/server/usage") return pluginUsageRuntime; diff --git a/packages/server/src/server/plugins/plugin-process.test.ts b/packages/server/src/server/plugins/plugin-process.test.ts index 6a4380d503..ddee3cb291 100644 --- a/packages/server/src/server/plugins/plugin-process.test.ts +++ b/packages/server/src/server/plugins/plugin-process.test.ts @@ -37,3 +37,14 @@ it("keeps React out of the plugin host's runtime dependency graph", async () => }), ).resolves.toMatchObject({ errors: [] }); }); + +it("exposes CLI probes through the plugin host's server SDK", async () => { + const { evaluateBundle } = await import("./bundle-evaluator.js"); + const setup = evaluateBundle(`(require) => ({ default: async () => { + const { execCommand } = require("@getpaseo/plugin/server"); + const { stdout } = await execCommand(${JSON.stringify(process.execPath)}, ["-e", "process.stdout.write('plugin CLI ready')"]); + if (stdout !== "plugin CLI ready") throw new Error("CLI output was lost"); + } })`); + // This contribution uses no server context APIs; it exercises the host's real SDK binding. + await expect(setup(undefined as never)).resolves.toBeUndefined(); +}); diff --git a/packages/server/src/utils/spawn.ts b/packages/server/src/utils/spawn.ts index 8fac115f00..b26104c63e 100644 --- a/packages/server/src/utils/spawn.ts +++ b/packages/server/src/utils/spawn.ts @@ -1,15 +1,6 @@ -import { execFile, spawn, type ChildProcess, type SpawnOptions } from "node:child_process"; -import { extname } from "node:path"; -import { promisify } from "node:util"; - +import type { ChildProcess, SpawnOptions } from "node:child_process"; +import { spawnProcess as spawnCli, execCommand as execCli } from "@getpaseo/plugin/server"; import { createExternalCommandProcessEnv, type ProcessEnvRecord } from "../server/paseo-env.js"; -import { - isWindowsCommandScript, - quoteWindowsArgument, - quoteWindowsCommand, -} from "./windows-command.js"; - -const execFileAsync = promisify(execFile); interface ExternalEnvOptions { baseEnv?: ProcessEnvRecord; @@ -35,23 +26,6 @@ interface ExecCommandResult { stderr: string; } -function hasPathSeparator(value: string): boolean { - return value.includes("/") || value.includes("\\"); -} - -function shouldUseWindowsShell( - command: string, - requestedShell?: boolean | string, -): boolean | string { - if (isWindowsCommandScript(command)) { - return true; - } - if (requestedShell !== undefined) { - return requestedShell; - } - return process.platform === "win32" && !hasPathSeparator(command) && !extname(command); -} - export function spawnProcess( command: string, args: string[], @@ -59,12 +33,6 @@ export function spawnProcess( ): ChildProcess { const { baseEnv, env, envOverlay, ...spawnOptions } = options ?? {}; const resolvedBaseEnv = env ?? baseEnv ?? process.env; - const isWindows = process.platform === "win32"; - const shell = shouldUseWindowsShell(command, spawnOptions.shell); - - const shouldQuoteForShell = isWindows && shell !== false; - const resolvedCommand = shouldQuoteForShell ? quoteWindowsCommand(command) : command; - const resolvedArgs = shouldQuoteForShell ? args.map(quoteWindowsArgument) : args; const childEnv = options?.envMode === "internal" ? ({ ...resolvedBaseEnv, ...envOverlay } as NodeJS.ProcessEnv) @@ -74,12 +42,10 @@ export function spawnProcess( ...(envOverlay ? [envOverlay] : []), ); - return spawn(resolvedCommand, resolvedArgs, { + return spawnCli(command, args, { ...spawnOptions, env: childEnv, - shell, signal: options?.signal, - windowsHide: true, }); } @@ -90,11 +56,6 @@ export async function execCommand( ): Promise { const { baseEnv, env, envOverlay } = options ?? {}; const resolvedBaseEnv = env ?? baseEnv ?? process.env; - const isWindows = process.platform === "win32"; - const shell = shouldUseWindowsShell(command, options?.shell); - const shouldQuoteForShell = isWindows && shell !== false; - const resolvedCommand = shouldQuoteForShell ? quoteWindowsCommand(command) : command; - const resolvedArgs = shouldQuoteForShell ? args.map(quoteWindowsArgument) : args; const childEnv = options?.envMode === "internal" ? ({ ...resolvedBaseEnv, ...envOverlay } as NodeJS.ProcessEnv) @@ -104,14 +65,14 @@ export async function execCommand( ...(envOverlay ? [envOverlay] : []), ); - return execFileAsync(resolvedCommand, resolvedArgs, { + return execCli(command, args, { cwd: options?.cwd, env: childEnv, + shell: options?.shell, encoding: options?.encoding ?? "utf8", + signal: options?.signal, killSignal: options?.killSignal, timeout: options?.timeout, maxBuffer: options?.maxBuffer, - shell, - windowsHide: true, }) as Promise; } diff --git a/plugins/antigravity-provider/server/internal/process.ts b/plugins/antigravity-provider/server/internal/process.ts index 899a88e5cd..c87a17aadb 100644 --- a/plugins/antigravity-provider/server/internal/process.ts +++ b/plugins/antigravity-provider/server/internal/process.ts @@ -1,4 +1,5 @@ -import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; +import type { ChildProcessWithoutNullStreams } from "node:child_process"; +import { spawnProcess } from "@getpaseo/plugin/server"; import { signalProcess } from "./signals.js"; import { createInterface } from "node:readline"; import type { ProviderLaunch, ProviderSessionConfig } from "@getpaseo/plugin/server/provider"; @@ -49,7 +50,7 @@ function driverArgs(options: DriverOptions): string[] { } export function startDriver(options: DriverOptions): Driver { - const child = spawn(options.launch.command, driverArgs(options), { + const child = spawnProcess(options.launch.command, driverArgs(options), { cwd: options.config.cwd, env: { ...options.launch.env, ...options.config.env }, detached: process.platform !== "win32", @@ -183,7 +184,7 @@ interface ProbeOptions { } export function probe(options: ProbeOptions): Promise { return new Promise((resolve, reject) => { - const child = spawn(options.launch.command, [...options.launch.args, ...options.args], { + const child = spawnProcess(options.launch.command, [...options.launch.args, ...options.args], { env: options.launch.env, cwd: options.cwd, stdio: "pipe", diff --git a/plugins/antigravity-provider/server/internal/signals.ts b/plugins/antigravity-provider/server/internal/signals.ts index be34ecc8c9..f7f5b64eab 100644 --- a/plugins/antigravity-provider/server/internal/signals.ts +++ b/plugins/antigravity-provider/server/internal/signals.ts @@ -1,5 +1,4 @@ -import { execFile } from "node:child_process"; -import { promisify } from "node:util"; +import { execCommand } from "@getpaseo/plugin/server"; interface SignalOptions { platform: NodeJS.Platform; @@ -11,8 +10,6 @@ type SignalPlan = | { type: "group"; pid: number; signal: NodeJS.Signals } | { type: "tree"; command: "taskkill"; args: string[] }; -const executeFile = promisify(execFile); - export function signalPlan(options: SignalOptions): SignalPlan { if (options.platform === "win32") { return { type: "tree", command: "taskkill", args: ["/PID", String(options.pid), "/T", "/F"] }; @@ -32,7 +29,7 @@ export function signalProcess(options: SignalOptions): void | Promise { async function killTree(plan: Extract): Promise { try { - await executeFile(plan.command, plan.args, { windowsHide: true }); + await execCommand(plan.command, plan.args, { shell: false }); } catch (error) { // taskkill reports an already-exited PID with exit code 128. if (!(error instanceof Error && "code" in error && error.code === 128)) throw error; diff --git a/plugins/antigravity-provider/server/provider.test.ts b/plugins/antigravity-provider/server/provider.test.ts index 8ba12841e0..631f10a6fe 100644 --- a/plugins/antigravity-provider/server/provider.test.ts +++ b/plugins/antigravity-provider/server/provider.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, readFile, rm, stat } from "node:fs/promises"; +import { mkdtemp, readFile, rm, stat, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; @@ -773,3 +773,19 @@ describe("image prompts", () => { expect((await h.records()).filter((entry) => entry.input)).toHaveLength(0); }); }); + +it.runIf(process.platform === "win32")( + "launches an Antigravity command shim for probes and conversation startup", + async () => { + const h = await harness(); + const shim = path.join(h.cwd, "agy launcher.cmd"); + await writeFile(shim, `@echo off\r\n"${process.execPath}" "${fake}" %*\r\n`); + h.launch.command = shim; + h.launch.args = ["--launch-prefix"]; + expect(await provider.status!({ launch: h.launch })).toEqual({ available: true }); + await h.open(); + expect(await h.wait((event) => event.type === "session.opened")).toMatchObject({ + sessionId: "s", + }); + }, +); diff --git a/plugins/muse-provider/server/connection.ts b/plugins/muse-provider/server/connection.ts index f502705f8f..3fca09ef5e 100644 --- a/plugins/muse-provider/server/connection.ts +++ b/plugins/muse-provider/server/connection.ts @@ -1,5 +1,5 @@ import type { ProviderLaunch } from "@getpaseo/plugin/server/provider"; -import { spawn } from "node:child_process"; +import { spawnProcess, terminateProcess } from "@getpaseo/plugin/server"; import { randomBytes } from "node:crypto"; import { createInterface } from "node:readline"; import { setTimeout as delay } from "node:timers/promises"; @@ -35,6 +35,7 @@ export class MspConnection { private stderr = ""; private failure: MuseError | null = null; private closing = false; + private closeResult: Promise | null = null; private readonly exited: Promise; constructor( @@ -42,15 +43,20 @@ export class MspConnection { launch: ProviderLaunch; cwd?: string; timeoutMs?: number; + startupTimeoutMs?: number; serveArgs?: string[]; }, ) { const { launch, cwd } = options; - this.child = spawn(launch.command, [...launch.args, "serve", ...(options.serveArgs ?? [])], { - env: launch.env, - cwd, - stdio: "pipe", - }); + this.child = spawnProcess( + launch.command, + [...launch.args, "serve", ...(options.serveArgs ?? [])], + { + env: launch.env, + cwd, + stdio: "pipe", + }, + ); this.child.stderr.setEncoding("utf8"); this.child.stderr.on("data", (chunk: string) => { this.stderr = (this.stderr + chunk).slice(-8192); @@ -62,7 +68,7 @@ export class MspConnection { } catch (error) { const message = error instanceof Error ? error.message : String(error); this.fail(new MuseError("invalidFrame", `Invalid MSP frame: ${message}`)); - this.child.kill(); + void this.close().catch(() => {}); } }); this.child.stdin.on("error", () => { @@ -89,6 +95,7 @@ export class MspConnection { }, }, initializedSchema, + this.options.startupTimeoutMs ?? 30000, ); if (response.schema.fingerprint !== fingerprint) { process.stderr.write( @@ -123,7 +130,12 @@ export class MspConnection { } } } - async request(method: string, params: object, schema: z.ZodType): Promise { + async request( + method: string, + params: object, + schema: z.ZodType, + timeoutMs = this.options.timeoutMs ?? 10000, + ): Promise { if (this.failure) throw this.failure; if (this.closing) throw new MuseError("closed", "Muse host is closing"); const id = ++this.sequence; @@ -131,22 +143,31 @@ export class MspConnection { const timer = setTimeout(() => { this.pending.delete(id); reject(new MuseError("timeout", `Muse ${method} timed out`)); - }, this.options.timeoutMs ?? 10000); + }, timeoutMs); this.pending.set(id, { resolve, reject, timer }); this.write({ jsonrpc: "2.0", id, method, params }); }); return schema.parse(response); } - async close(): Promise { - if (this.closing) return this.exited; + close(): Promise { + if (this.closeResult) return this.closeResult; this.closing = true; + this.closeResult = this.finishClose(); + return this.closeResult; + } + private async finishClose(): Promise { this.child.stdin.end(); const drained = await Promise.race([ this.exited.then(() => true), - delay(1000).then(() => false), + delay(1000, undefined, { ref: false }).then(() => false), ]); - if (!drained) this.child.kill("SIGKILL"); - await this.exited; + if (drained) return; + await terminateProcess(this.child); + const stopped = await Promise.race([ + this.exited.then(() => true), + delay(5000, undefined, { ref: false }).then(() => false), + ]); + if (!stopped) throw new MuseError("timeout", "Muse host did not close after termination"); } private write(frame: object): void { this.child.stdin.write(JSON.stringify(frame) + "\n"); @@ -173,7 +194,7 @@ export class MspConnection { if (frame.id === null) { if (!frame.error) throw new MuseError("invalidFrame", "MSP result has a null id"); this.fail(new MuseError(frame.error.data?.kind ?? "rpc", frame.error.message)); - this.child.kill(); + void this.close().catch(() => {}); return; } const pending = this.pending.get(frame.id); diff --git a/plugins/muse-provider/server/provider.ts b/plugins/muse-provider/server/provider.ts index bef6693f91..8d9a3ddfea 100644 --- a/plugins/muse-provider/server/provider.ts +++ b/plugins/muse-provider/server/provider.ts @@ -10,7 +10,7 @@ import { } from "@getpaseo/plugin/server/provider"; import { serveArgs } from "./options.js"; import { Usage } from "./usage.js"; -import { execFile } from "node:child_process"; +import { execCommand } from "@getpaseo/plugin/server"; import { Catalog, launchKey } from "./catalog.js"; import { MspConnection } from "./connection.js"; import { MuseError, actionableError } from "./errors.js"; @@ -63,24 +63,23 @@ function requireLaunch(launch: ProviderLaunch | undefined): ProviderLaunch { async function status(launch: ProviderLaunch): Promise { let host: MspConnection | undefined; try { - const versionText = await new Promise((resolve, reject) => { - execFile( - launch.command, - [...launch.args, "--version"], - { env: launch.env, timeout: 3000, maxBuffer: 8192 }, - (error, stdout) => { - if (error) reject(new MuseError("version", error.message)); - else resolve(stdout); - }, - ); - }); + const { stdout: versionText } = await execCommand( + launch.command, + [...launch.args, "--version"], + { + env: launch.env, + timeout: 3000, + maxBuffer: 8192, + }, + ); const match = /\b(\d+)\.(\d+)\.(\d+)\b/.exec(versionText); if (!match) return { available: false, diagnostic: "Muse returned an unrecognized version." }; const major = Number(match[1]); const minor = Number(match[2]); if (major < 1 || (major === 1 && minor < 3)) return { available: false, diagnostic: `Update Muse Code: found ${match[0]}, need ≥1.3.0` }; - host = new MspConnection({ launch, timeoutMs: 3000 }); + // Leave room for the version probe, account read and cleanup within the status RPC deadline. + host = new MspConnection({ launch, timeoutMs: 3000, startupTimeoutMs: 20000 }); await host.initialize(); const account = await host.request("account/read", {}, accountSchema); if (account.state === "loggedOut") diff --git a/plugins/muse-provider/test/fake-muse.mjs b/plugins/muse-provider/test/fake-muse.mjs index 65e1b88fcf..6dec29f8e5 100644 --- a/plugins/muse-provider/test/fake-muse.mjs +++ b/plugins/muse-provider/test/fake-muse.mjs @@ -6,9 +6,20 @@ import { fileURLToPath } from "node:url"; const root = path.join(path.dirname(fileURLToPath(import.meta.url)), "fixtures"); const scenario = process.env.MUSE_TEST_SCENARIO || "text-reasoning"; if (process.argv.includes("--version")) { + if (process.env.MUSE_TEST_VERSION_ERROR) { + process.stderr.write(process.env.MUSE_TEST_VERSION_ERROR + "\n"); + process.exit(17); + } process.stdout.write(`Muse Code ${process.env.MUSE_TEST_VERSION || "1.4.1"}\n`); process.exit(0); } +if (process.env.MUSE_TEST_STUBBORN) { + setInterval(() => {}, 1000); + appendFileSync( + process.env.MUSE_TEST_REQUESTS, + JSON.stringify({ event: "stubbornHost", pid: process.pid }) + "\n", + ); +} const rows = readFixture(scenario); if (process.env.MUSE_TEST_REQUESTS) appendFileSync( @@ -22,6 +33,14 @@ const receipts = new Set(); const lines = createInterface({ input: process.stdin }); lines.on("line", receive); function receive(line) { + const parsed = JSON.parse(line); + if (parsed.method === "initialize" && process.env.MUSE_TEST_INITIALIZE_DELAY_MS) { + setTimeout(() => receiveFrame(line), Number(process.env.MUSE_TEST_INITIALIZE_DELAY_MS)); + return; + } + receiveFrame(line); +} +function receiveFrame(line) { const frame = JSON.parse(line); if (process.env.MUSE_TEST_REQUESTS) appendFileSync(process.env.MUSE_TEST_REQUESTS, JSON.stringify(frame) + "\n"); diff --git a/plugins/muse-provider/test/provider.test.ts b/plugins/muse-provider/test/provider.test.ts index d19415d544..800a010da9 100644 --- a/plugins/muse-provider/test/provider.test.ts +++ b/plugins/muse-provider/test/provider.test.ts @@ -1,3 +1,4 @@ +import { execFile } from "node:child_process"; import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { setTimeout as delay } from "node:timers/promises"; import os from "node:os"; @@ -14,6 +15,7 @@ import { type ProviderInput, } from "@getpaseo/plugin/server/provider"; import type { UsageSourceRegistration } from "@getpaseo/plugin/server"; +import { execCommand } from "@getpaseo/plugin/server"; import contribute from "../index.server.js"; const connections: ProviderConnection[] = []; @@ -460,6 +462,25 @@ for (const [env, expected] of [ expect(await h.provider.status!({ launch: h.launch })).toEqual(expected); }); } +test("status preserves the version-probe command and stderr diagnostic", async () => { + const h = await harness("catalog-controls", { + MUSE_TEST_VERSION_ERROR: "Muse version probe could not read its installation metadata", + }); + // Compare against the original execFile callback: wrapping in MuseError only changed its kind. + const previousDiagnostic = await new Promise((resolve, reject) => { + execFile(h.launch.command, [...h.launch.args, "--version"], { env: h.launch.env }, (error) => { + if (error) resolve(error.message); + else reject(new Error("Expected the version probe to fail")); + }); + }); + const status = await h.provider.status!({ launch: h.launch }); + expect(status).toEqual({ available: false, diagnostic: previousDiagnostic }); + expect(status.diagnostic).toContain("--version"); + expect(status.diagnostic).toContain( + "Muse version probe could not read its installation metadata", + ); + expect(await h.recorded()).toEqual([]); +}); test("status reports a missing executable", async () => { const h = await harness(); expect( @@ -1530,3 +1551,90 @@ test("Muse window identity follows the reported duration instead of assuming fiv ], }); }); + +test("status accepts initialization slower than the account deadline without isolating history", async () => { + const h = await harness("catalog-controls", { MUSE_TEST_INITIALIZE_DELAY_MS: "3500" }); + expect(await h.provider.status!({ launch: h.launch })).toEqual({ available: true }); + expect(await h.recorded()).toContainEqual({ event: "hostClosed" }); +}, 10000); + +for (const extension of ["cmd", "bat"]) { + test.runIf(process.platform === "win32")( + `launches the discovered Muse .${extension} for status, catalog and sessions`, + async () => { + const h = await harness("catalog-controls"); + const shim = path.join(h.root, `muse launcher.${extension}`); + await writeFile(shim, `@echo off\r\n"${process.execPath}" "${h.launch.args[0]}" %*\r\n`); + h.launch.command = shim; + h.launch.args = []; + expect(await h.provider.status!({ launch: h.launch })).toEqual({ available: true }); + await h.send({ type: "catalog", requestId: "shim-catalog" }); + expect(await h.wait((event) => event.type === "catalog")).toMatchObject({ + requestId: "shim-catalog", + }); + await h.open(); + expect(await h.wait((event) => event.type === "session.opened")).toMatchObject({ + sessionId: "paseo-session", + }); + }, + 20000, + ); +} + +test("force-stops a Muse host that ignores stdin EOF", async () => { + const h = await stubbornHostHarness(); + try { + const result = await Promise.race([ + h.provider.status!({ launch: h.launch }), + delay(10000).then(() => { + throw new Error("Muse status did not finish closing its host"); + }), + ]); + expect(result).toEqual({ available: true }); + const hosts = (await h.recorded()).filter((row) => row.event === "stubbornHost"); + expect(hosts).toHaveLength(1); + expect(() => process.kill(hosts[0].pid, 0)).toThrow(); + } finally { + await cleanupStubbornHosts(h); + } +}, 15000); + +test("closes the whole Muse host after an uncorrelated MSP error", async () => { + const h = await stubbornHostHarness({ MUSE_TEST_NULL_ERROR_ID: "1" }); + try { + const result = await h.provider.status!({ launch: h.launch }); + expect(result).toEqual({ available: false, diagnostic: "Cannot recover request id" }); + const recorded = await h.recorded(); + expect(recorded.some((row) => row.event === "hostClosed")).toBe(true); + const hosts = recorded.filter((row) => row.event === "stubbornHost"); + expect(hosts).toHaveLength(1); + expect(() => process.kill(hosts[0].pid, 0)).toThrow(); + } finally { + await cleanupStubbornHosts(h); + } +}, 15000); + +async function stubbornHostHarness(env: Record = {}) { + const h = await harness("catalog-controls", { MUSE_TEST_STUBBORN: "1", ...env }); + if (process.platform === "win32") { + const shim = path.join(h.root, "stubborn muse.cmd"); + await writeFile(shim, `@echo off\r\n"${process.execPath}" "${h.launch.args[0]}" %*\r\n`); + h.launch.command = shim; + h.launch.args = []; + } + return h; +} +async function cleanupStubbornHosts(h: Awaited>) { + // Keep a failing Windows regression from leaving its deliberately stubborn child alive. + for (const host of (await h.recorded()).filter((row) => row.event === "stubbornHost")) { + if (process.platform === "win32") { + await execCommand("taskkill.exe", ["/PID", String(host.pid), "/T", "/F"], { + shell: false, + }).catch(() => {}); + } else { + try { + process.kill(host.pid, "SIGKILL"); + } catch {} + } + } +} diff --git a/public-docs/plugins/providers.md b/public-docs/plugins/providers.md index 69cf206641..2f4951669c 100644 --- a/public-docs/plugins/providers.md +++ b/public-docs/plugins/providers.md @@ -73,6 +73,32 @@ The connection has three operations: Keep the native SDK, process, and stream inside the connection implementation. Convert its output to `ProviderEvent` objects before publishing it. +## Launch the provider CLI + +Use `spawnProcess` and `execCommand` from `@getpaseo/plugin/server` for process transports +and CLI probes. Both handle Windows `.cmd`/`.bat` launchers and quote shell arguments. +Raw Node `spawn` and `execFile` reject those launchers on Windows. + +The daemon supplies `request.launch` with the resolved command, argument prefix, and complete +environment. Pass that environment explicitly; the helpers do not apply daemon environment policy. +Keep the process lifecycle and protocol deadlines inside your provider. End stdin for graceful +shutdown; use `terminateProcess(child)` for a forced stop. It kills the process tree on Windows, +including the CLI behind a command-script launcher. Bound your final wait for closed streams. + +```ts +import { spawnProcess, execCommand } from "@getpaseo/plugin/server"; + +const child = spawnProcess(launch.command, [...launch.args, "serve"], { + env: launch.env, + cwd, + stdio: "pipe", +}); +const { stdout } = await execCommand(launch.command, [...launch.args, "--version"], { + env: launch.env, + timeout: 5000, +}); +``` + ## Return models, modes, and thinking options Paseo requests the catalog before creating a session. Return the choices needed by the agent form: