diff --git a/install.sh b/install.sh index 29ccac992..e7c197efa 100755 --- a/install.sh +++ b/install.sh @@ -1960,6 +1960,26 @@ prime_agent_native_validate_release_metadata() { case "$native_metadata_source" in http://*|https://*) ;; *) return 1 ;; esac } +prime_agent_native_probe_timeout() { + # The first run of a freshly extracted executable can be slow: Rosetta 2 translates + # the whole binary before it prints anything, and a cold page cache adds more. A + # 51 MB x64 executable measured 11.3s cold and 0.2s warm on Apple Silicon. + native_probe_timeout=60 + case "${PRIME_AGENT_PROBE_TIMEOUT_SECONDS:-}" in + ''|*[!0-9]*) ;; + *) + # $((...)) reads leading-zero constants as octal: "010" would become an + # 8-second deadline while being reported as 10, and "08"/"09" are invalid + # octal and abort the arithmetic under set -eu. Strip the leading zeroes + # so the accepted value stays decimal; an all-zero value strips to the + # empty string and falls back to the default like any invalid value. + native_probe_timeout="${PRIME_AGENT_PROBE_TIMEOUT_SECONDS#"${PRIME_AGENT_PROBE_TIMEOUT_SECONDS%%[!0]*}"}" + [ -n "$native_probe_timeout" ] && [ "$native_probe_timeout" -le 600 ] || native_probe_timeout=60 + ;; + esac + printf '%s\n' "$native_probe_timeout" +} + prime_agent_native_probe() ( # macOS does not ship timeout; keep the deadline independent of Node and Python. native_probe_pid= @@ -1974,10 +1994,11 @@ prime_agent_native_probe() ( trap 'exit 129' HUP "$@" & native_probe_pid=$! - native_probe_deadline=$(($(date +%s) + 10)) + native_probe_timeout=$(prime_agent_native_probe_timeout) + native_probe_deadline=$(($(date +%s) + native_probe_timeout)) while kill -0 "$native_probe_pid" 2>/dev/null; do if [ "$(date +%s)" -ge "$native_probe_deadline" ]; then - printf 'error: executable probe timed out after 10 seconds.\n' >&2 + printf 'error: executable probe timed out after %s seconds.\n' "$native_probe_timeout" >&2 kill -KILL "$native_probe_pid" 2>/dev/null || : wait "$native_probe_pid" 2>/dev/null || : native_probe_pid= @@ -2235,7 +2256,10 @@ prime_agent_install_native() { for native_asset in prime-agent package.json install.sh prime-agent-runtime/pyproject.toml prime-agent-runtime/src/rlm/repl.py theme/prime.json export-html/template.html photon_rs_bg.wasm; do [ -f "$native_extracted/$native_asset" ] || { printf 'error: missing archive asset: %s\n' "$native_asset" >&2; exit 1; } done - if ! prime_agent_native_probe "$native_extracted/prime-agent" --version >"$prime_agent_native_stage/version" 2>"$prime_agent_native_stage/probe.log"; then + native_probe_status=0 + prime_agent_native_probe "$native_extracted/prime-agent" --version >"$prime_agent_native_stage/version" 2>"$prime_agent_native_stage/probe.log" || + native_probe_status=$? + if [ "$native_probe_status" -ne 0 ]; then # A missing libstdc++ is one package away, so name it instead of downloading Node. if prime_agent_native_probe_missing_libstdcxx "$prime_agent_native_stage/probe.log"; then prime_agent_native_cleanup @@ -2244,6 +2268,18 @@ prime_agent_install_native() { fi cat "$prime_agent_native_stage/probe.log" >&2 prime_agent_native_cleanup + # A timeout means the executable never answered, not that it cannot run here. + if [ "$native_probe_status" -eq 124 ]; then + native_probe_hint="Set PRIME_AGENT_PROBE_TIMEOUT_SECONDS to a larger value and run the installer again." + if [ "${PRIME_AGENT_INSTALL_METHOD:-auto}" = auto ]; then + printf 'The compiled executable did not answer in time; using the Node installation. %s\n' "$native_probe_hint" >&2 + prime_agent_install_node "$native_version" + return + fi + printf 'error: the compiled executable did not answer within %s seconds. %s\n' \ + "$(prime_agent_native_probe_timeout)" "$native_probe_hint" >&2 + exit 1 + fi if [ "${PRIME_AGENT_INSTALL_METHOD:-auto}" = auto ]; then printf 'The compiled executable cannot run here; using the Node installation.\n' >&2 prime_agent_install_node "$native_version" diff --git a/packages/coding-agent/.changes/cli-global-flags-before-command.md b/packages/coding-agent/.changes/cli-global-flags-before-command.md new file mode 100644 index 000000000..b015fba06 --- /dev/null +++ b/packages/coding-agent/.changes/cli-global-flags-before-command.md @@ -0,0 +1,3 @@ +- Fixed global flags written before a command routing the command to the model as a chat message; `prime-agent --offline model list` now runs the command, `--` still sends the word as a message, and a global flag a command does not accept fails with a clear error. +- Fixed moved global flags leaking past a `--` separator into an `mcp add` child command or a scheduled message; they now stay ahead of any `--`. +- Fixed `prime-agent --offline help` and `--offline help status` printing help instead of chatting; global run flags no longer count as help arguments. diff --git a/packages/coding-agent/.changes/hermetic-version-check-tests.md b/packages/coding-agent/.changes/hermetic-version-check-tests.md new file mode 100644 index 000000000..2ed662b4e --- /dev/null +++ b/packages/coding-agent/.changes/hermetic-version-check-tests.md @@ -0,0 +1 @@ +- Made the version check and npm bridge test suites hermetic. They now clear the update and daemon-worker environment variables they depend on, and the bridge sanitises the environment it hands to spawned children, so both suites pass from inside a running Prime Agent session instead of only in CI. diff --git a/packages/coding-agent/.changes/home-scoped-daemon-discovery.md b/packages/coding-agent/.changes/home-scoped-daemon-discovery.md new file mode 100644 index 000000000..ccbaf86cb --- /dev/null +++ b/packages/coding-agent/.changes/home-scoped-daemon-discovery.md @@ -0,0 +1,2 @@ +- Scoped background service discovery to the state root the command runs in, so a run with an isolated HOME or agent dir only lists and stops its own daemons and `shutdown --force` no longer reaches daemons that belong to another root. +- Scoped discovery now also reads the pre-move supervisor registry (so daemons from before the registry relocation stay reachable under the same agent dir) and skips records whose agent dir can no longer be resolved, so one stale record cannot abort the sweep. diff --git a/packages/coding-agent/.changes/native-probe-timeout-budget.md b/packages/coding-agent/.changes/native-probe-timeout-budget.md new file mode 100644 index 000000000..8315cdde6 --- /dev/null +++ b/packages/coding-agent/.changes/native-probe-timeout-budget.md @@ -0,0 +1 @@ +- Fixed installs that failed on a slow first run of the compiled executable, such as Rosetta 2 translation on Apple Silicon; the startup probe now waits up to 60 seconds, accepts a `PRIME_AGENT_PROBE_TIMEOUT_SECONDS` override, and reports a timeout as a timeout instead of claiming the executable cannot run on this machine. diff --git a/packages/coding-agent/.changes/telemetry-platform-fidelity.md b/packages/coding-agent/.changes/telemetry-platform-fidelity.md new file mode 100644 index 000000000..d635e106e --- /dev/null +++ b/packages/coding-agent/.changes/telemetry-platform-fidelity.md @@ -0,0 +1 @@ +- Usage analytics now report `libc`, `libc_version`, `cpu_baseline`, `os_release`, and `os_product_version` so musl and non-AVX2 coverage is measurable before the standalone-Node install path is retired; every probe is memoized, the linked glibc runtime outranks a merely installed musl loader, and `os_release` is the raw kernel release string, which custom kernels can make identifying. diff --git a/packages/coding-agent/docs/settings.md b/packages/coding-agent/docs/settings.md index 40d20c977..04c10714d 100644 --- a/packages/coding-agent/docs/settings.md +++ b/packages/coding-agent/docs/settings.md @@ -92,6 +92,21 @@ The stable `latest.json` and beta `beta.json` manifests use the same JSON shape: Prime Agent sends pseudonymous, aggregate usage and performance events to Prime Intellect. These events include version and operating-system category, onboarding outcome and duration, execution mode (`interactive`, `print`, `json`, `rpc`, or `acp`), run outcomes, TTFT and latency, prompt and turn counts, token usage, tool success counts, retries, and compactions. +Every event also carries a small platform descriptor, used to decide which prebuilt binaries Prime Agent has to ship: + +| Property | Values | +|----------|--------| +| `os_family` | `linux`, `darwin`, `win32`, ... | +| `architecture` | `arm64`, `x64`, ... | +| `install_method` | `bun-binary`, `homebrew`, `npm`, `pnpm`, `yarn`, `bun`, `unknown` | +| `libc` | `glibc`, `musl`, `none` (not Linux), `unknown` | +| `libc_version` | glibc runtime version such as `2.39`, else `unknown` | +| `cpu_baseline` | `avx2`, `no_avx2` (both measured on x86_64), `avx2_assumed` (Intel Macs, inferred), `not_applicable` (not x86_64), `unknown` | +| `os_release` | kernel version such as `6.8.0-45-generic` or `24.6.0` | +| `os_product_version` | macOS product version such as `15.6`; `unknown` elsewhere | + +Most of these are coarse platform categories shared by millions of machines. `os_release` is the one exception: it is the raw kernel release string, capped at 64 characters. Stock kernel names such as `6.8.0-45-generic` are shared widely, but custom or self-built kernels can embed organization-, user-, or machine-specific labels in that string, so `os_release` is not guaranteed to be non-identifying. The remaining fields contain no hostname, username, path, serial number, or other hardware identifier. + Prime Agent does not send prompts, responses, thinking, tool arguments or results, command text, filenames, paths, repository information, environment variables, credentials, raw error messages, hostnames, usernames, emails, or hardware identifiers. A random installation ID is stored as `telemetry.json` in the configured agent directory (normally `~/.prime/agent/`). Telemetry can be disabled globally or for an individual project. Project settings can only further restrict telemetry: they cannot re-enable a global opt-out or suppress the global one-time disclosure. diff --git a/packages/coding-agent/src/cli/daemon-launch.ts b/packages/coding-agent/src/cli/daemon-launch.ts index 40943d0a1..a34d62861 100644 --- a/packages/coding-agent/src/cli/daemon-launch.ts +++ b/packages/coding-agent/src/cli/daemon-launch.ts @@ -23,7 +23,13 @@ import { DAEMON_WORKER_TOKEN_ENV, } from "../modes/daemon/daemon-worker-protocol.js"; import { spawnHidden } from "../utils/child-process.js"; -import { isHelpCommandRequest, PUBLIC_COMMAND_NAMES, REMOVED_COMMAND_NAMES } from "./command-registry.js"; +import { isHelpCommandRequest, REMOVED_COMMAND_NAMES } from "./command-registry.js"; +import { + extractHelpCommandPath, + findFirstPositionalArgument, + isCommandPositional, + PROMPT_RUN_FLAGS, +} from "./global-flags.js"; import { createCliSubprocessEnv, formatCurrentCliCommand } from "./subprocess-launch.js"; const DAEMON_STARTUP_TIMEOUT_MS = 30_000; @@ -505,59 +511,6 @@ export function ensureInteractiveDaemonRunning(socketPath: string, spawnCwd?: st } const EARLY_LAUNCH_EXCLUDED_FLAGS = new Set(["--help", "-h", "--version", "-v", "--list-models", "--export"]); -const EARLY_LAUNCH_VALUE_FLAGS = new Set([ - "--mode", - "--daemon-socket", - "--provider", - "--model", - "--api-key", - "--cwd", - "--system-prompt", - "--append-system-prompt", - "--fork", - "--session-dir", - "--models", - "--tools", - "-t", - "--thinking", - "--extension", - "-e", - "--skill", - "--prompt-template", - "--theme", - "--autonomous-gate", - "--autonomous-gate-retries", - "--autonomous-gate-timeout-ms", - "--autonomous-max-continuations", - "--autonomous-max-turns", - "--autonomous-max-tokens", - "--autonomous-timeout-ms", - "--goal", - "--goal-token-budget", -]); - -function findFirstEarlyLaunchPositional(args: readonly string[]): { index: number; value: string } | undefined { - for (let index = 0; index < args.length; index++) { - const arg = args[index]!; - if (arg === "--") { - return args[index + 1] === undefined ? undefined : { index: index + 1, value: args[index + 1]! }; - } - if (EARLY_LAUNCH_VALUE_FLAGS.has(arg)) { - index++; - continue; - } - if (arg === "--resume" || arg === "-r") { - if (args[index + 1] && !args[index + 1]!.startsWith("-")) { - index++; - } - continue; - } - if (!arg.startsWith("-")) { - return { index, value: arg }; - } - } - return undefined; -} export function shouldStartDaemonEarly(args: readonly string[], startupBenchmark: boolean): boolean { if (startupBenchmark) { @@ -573,15 +526,21 @@ export function shouldStartDaemonEarly(args: readonly string[], startupBenchmark if (args.includes("--print") || args.includes("-p")) { return true; } - const firstPositional = findFirstEarlyLaunchPositional(args); - const isHelpCommand = - firstPositional?.value === "help" && isHelpCommandRequest(args.slice(firstPositional.index + 1)); + const firstPositional = findFirstPositionalArgument(args); + if (!firstPositional || !isCommandPositional(firstPositional)) { + return true; + } + // Prompt-run flags keep a following command word on the chat path (the + // rotation skips them), so these runs need the early daemon boot too. + if (args.slice(0, firstPositional.index).some((arg) => PROMPT_RUN_FLAGS.has(arg))) { + return true; + } + const helpPath = + firstPositional.value === "help" ? extractHelpCommandPath(args, firstPositional.index + 1) : undefined; + const isHelpCommand = helpPath !== undefined && isHelpCommandRequest(helpPath); if ( - firstPositional && - (REMOVED_COMMAND_NAMES.has(firstPositional.value) || - (PUBLIC_COMMAND_NAMES.has(firstPositional.value) && - firstPositional.value !== "agents" && - (firstPositional.value !== "help" || isHelpCommand))) + REMOVED_COMMAND_NAMES.has(firstPositional.value) || + (firstPositional.value !== "agents" && (firstPositional.value !== "help" || isHelpCommand)) ) { return false; } diff --git a/packages/coding-agent/src/cli/daemon-ps.ts b/packages/coding-agent/src/cli/daemon-ps.ts index ae73eebbb..48be425bc 100644 --- a/packages/coding-agent/src/cli/daemon-ps.ts +++ b/packages/coding-agent/src/cli/daemon-ps.ts @@ -16,6 +16,7 @@ import { type DaemonRuntimeIdentity, } from "../modes/daemon/daemon-protocol.js"; import { defaultDaemonSocketDir, defaultDaemonSocketPath, normalizeSocketPath } from "../modes/daemon/daemon-socket.js"; +import { createDaemonStateRootMatcher } from "../modes/daemon/daemon-state-root.js"; import { acquireDaemonShutdownAdmission } from "../modes/daemon/daemon-supervisor-ownership.js"; import type { DaemonWorkerDescriptor } from "../modes/daemon/daemon-worker-protocol.js"; import { @@ -29,15 +30,18 @@ import { formatDaemonListTable } from "./daemon-ps-format.js"; import { promptYesNo } from "./daemon-stop-confirm.js"; /** - * `daemon ps` discovers every prime-agent daemon on the machine, not just the - * one on a single socket. Discovery has two sources merged by socket path: + * `daemon ps` discovers every prime-agent daemon in *our state root*, not just + * the one on a single socket. Discovery has two sources merged by socket path: * * 1. The OS list of listening unix sockets owned by a prime-agent process * (`ss -lxp` on Linux, `lsof` on macOS). Daemons set process.title to * APP_NAME and carry nothing useful in argv, so the socket→pid mapping the * kernel keeps is the only reliable way to find daemons on arbitrary * `--daemon-socket` paths. This is the same data as `ss -lxp | grep - * prime-agent`, just parsed. + * prime-agent`, just parsed. The kernel list is machine-wide, so it is + * filtered to our state root (see createDaemonStateRootMatcher): a daemon + * started under a different HOME or agent dir is another root's business, + * and stopping it from here would kill unrelated live sessions. * 2. A sweep of the default socket dir, which catches orphaned socket *files* * left behind by daemons that are no longer running. * @@ -176,7 +180,18 @@ export function parsePsEtimes(stdout: string): Map { return uptimes; } +/** + * Listening prime-agent daemons that belong to this state root. The OS sweep + * behind it sees every daemon the user can observe, including daemons from an + * isolated HOME or a different agent dir, so the result is filtered before any + * caller can list, signal or reap it. + */ function scanListeningDaemons(): DiscoveredDaemonProcess[] { + const belongsToStateRoot = createDaemonStateRootMatcher(); + return scanAllListeningDaemons().filter((daemon) => belongsToStateRoot(daemon.socketPath)); +} + +function scanAllListeningDaemons(): DiscoveredDaemonProcess[] { if (process.platform === "win32") { return []; } @@ -343,7 +358,7 @@ export function verifyHelloSupervisorPid( return pid; } -/** Discover every daemon on the machine and probe each for version + session count. */ +/** Discover every daemon in this state root and probe each for version + session count. */ export async function discoverDaemons(): Promise { const processBySocket = new Map(); for (const daemon of scanListeningDaemons()) { diff --git a/packages/coding-agent/src/cli/global-flags.ts b/packages/coding-agent/src/cli/global-flags.ts new file mode 100644 index 000000000..1a1ab53c1 --- /dev/null +++ b/packages/coding-agent/src/cli/global-flags.ts @@ -0,0 +1,222 @@ +/** + * Shared scanning of leading global flags for the CLI entry paths. + * + * Both the early daemon-launch decision and public command routing must agree + * on which token is the subcommand. When they disagreed, a management command + * written as `prime-agent --offline model list` was routed to the model as a + * chat message. Keep this module dependency-free apart from the command + * registry: daemon-launch.ts loads it before the heavy module graph. + */ + +import { PUBLIC_COMMAND_NAMES, REMOVED_COMMAND_NAMES } from "./command-registry.js"; + +/** Global flags that consume the next argument as their value. */ +export const GLOBAL_VALUE_FLAGS: ReadonlySet = new Set([ + "--mode", + "--daemon-socket", + "--provider", + "--model", + "--api-key", + "--cwd", + "--system-prompt", + "--append-system-prompt", + "--fork", + "--session-dir", + "--models", + "--tools", + "-t", + "--thinking", + "--extension", + "-e", + "--skill", + "--prompt-template", + "--theme", + "--autonomous-gate", + "--autonomous-gate-retries", + "--autonomous-gate-timeout-ms", + "--autonomous-max-continuations", + "--autonomous-max-turns", + "--autonomous-max-tokens", + "--autonomous-timeout-ms", + "--goal", + "--goal-token-budget", +]); + +/** Flags that mark the run as a one-shot prompt, so its positional is a message. */ +export const PROMPT_RUN_FLAGS: ReadonlySet = new Set([ + "--print", + "-p", + "--system-prompt", + "--append-system-prompt", +]); + +/** + * parseArgs-known long flags that take no separate value, so the token after + * them stays free. Keep in sync with the value-less branches in args.ts. + */ +const GLOBAL_BOOLEAN_FLAGS: ReadonlySet = new Set([ + "--help", + "--version", + "--continue", + "--no-session", + "--no-tools", + "--no-builtin-tools", + "--no-extensions", + "--no-skills", + "--no-prompt-templates", + "--no-themes", + "--no-context-files", + "--autonomous", + "--verbose", + "--offline", +]); + +/** Value flags whose free-form text may start with a dash (args.ts). */ +const FREEFORM_VALUE_FLAGS: ReadonlySet = new Set(["--goal", "--autonomous-gate"]); + +/** Value flags whose arbitrary prompt text may look like a long option (args.ts). */ +const PROMPT_VALUE_FLAGS: ReadonlySet = new Set(["--system-prompt", "--append-system-prompt"]); + +/** + * True when parseArgs consumes the token after args[index] as part of the flag + * at args[index], so that token never becomes free for command routing. + * Mirrors args.ts exactly: `--` is never a value and stays an end-of-options + * marker; value flags take the next token unless it is option-like (free-form + * values only reject `--`-prefixed ones, prompt text accepts anything); + * --resume/-r take a selector only when it is neither option-like nor an + * @file reference; --print/-p take a message unless it is option-like text + * other than a `---` word or an @file reference; unknown long options take + * the following token as their value, while known boolean flags, `=`-attached + * values, and unknown short options take none. + */ +function consumesFollowingToken(args: readonly string[], index: number): boolean { + const next = args[index + 1]; + if (next === undefined || next === "--") { + return false; + } + const arg = args[index]!; + if (GLOBAL_VALUE_FLAGS.has(arg)) { + if (PROMPT_VALUE_FLAGS.has(arg)) { + return true; + } + return FREEFORM_VALUE_FLAGS.has(arg) ? !next.startsWith("--") : !next.startsWith("-"); + } + if (arg === "--resume" || arg === "-r") { + return !next.startsWith("-") && !next.startsWith("@"); + } + if (arg === "--print" || arg === "-p") { + return !next.startsWith("@") && (!next.startsWith("-") || next.startsWith("---")); + } + return ( + arg.startsWith("--") && + !arg.includes("=") && + !GLOBAL_BOOLEAN_FLAGS.has(arg) && + !next.startsWith("-") && + !next.startsWith("@") + ); +} + +export interface FirstPositionalArgument { + index: number; + value: string; + /** True when the token only became positional because of a `--` terminator. */ + afterSeparator: boolean; +} + +/** + * Find the first positional argument, skipping global flags exactly the way + * parseArgs skips them: flags whose value parseArgs consumes never free that + * value for command routing, unknown long options included. + */ +export function findFirstPositionalArgument(args: readonly string[]): FirstPositionalArgument | undefined { + for (let index = 0; index < args.length; index++) { + const arg = args[index]!; + if (arg === "--") { + const value = args[index + 1]; + return value === undefined ? undefined : { index: index + 1, value, afterSeparator: true }; + } + if (arg.startsWith("-")) { + if (consumesFollowingToken(args, index)) { + index++; + } + continue; + } + return { index, value: arg, afterSeparator: false }; + } + return undefined; +} + +/** True when the first positional names a command instead of starting a message. */ +export function isCommandPositional(positional: FirstPositionalArgument | undefined): boolean { + if (!positional || positional.afterSeparator) { + return false; + } + return PUBLIC_COMMAND_NAMES.has(positional.value) || REMOVED_COMMAND_NAMES.has(positional.value); +} + +/** + * Move leading global flags behind the subcommand they were written in front of, + * so `prime-agent --offline model list` runs the command instead of chatting. + * Arguments are returned unchanged when no known command is present, when `--` + * already escaped the token, and for one-shot prompt runs, whose positional is + * the message. Moved flags stay ahead of any `--` separator: arguments behind + * it are operand text (a child command or a scheduled message), so a flag + * landing there would be forwarded to the child verbatim. + */ +export function rotateGlobalFlagsBeforeCommand(args: readonly string[]): string[] { + const positional = findFirstPositionalArgument(args); + if (!positional || positional.index === 0 || !isCommandPositional(positional)) { + return [...args]; + } + if ( + args.slice(0, positional.index).some((arg) => PROMPT_RUN_FLAGS.has(arg) || arg === "--version" || arg === "-v") + ) { + // `--version` and its short form stay ahead of routing: parseArgs + // answers them wherever they appear, while a rotated + // `status --version` would die as an unknown option before the runtime + // ever sees it. + return [...args]; + } + const moved = args.slice(0, positional.index); + const rest = args.slice(positional.index + 1); + const separatorIndex = rest.indexOf("--"); + if (separatorIndex === -1) { + return [positional.value, ...rest, ...moved]; + } + return [positional.value, ...rest.slice(0, separatorIndex), ...moved, ...rest.slice(separatorIndex)]; +} + +/** + * The command path a `help` request names, with global run flags (and their + * values) excluded: they are run options, not help arguments, so + * `prime-agent --offline help status` asks about `status`. Flag values + * parseArgs consumes are excluded the same way, so `help --resume status` + * asks about nothing (status is the resume selector) and `help --print hi` + * asks about nothing (hi is the print message). Returns undefined when the + * tail contains `--` (everything behind it stays literal message text) or + * an explicit --help/-h flag with no topic yet (the generic per-command help + * path handles those). + */ +export function extractHelpCommandPath(args: readonly string[], from: number): string[] | undefined { + const path: string[] = []; + for (let index = from; index < args.length; index++) { + const arg = args[index]!; + if (arg === "--") { + return undefined; + } + if (arg === "--help" || arg === "-h") { + // An explicit help flag after a topic still asks about that topic; + // with no topic it defers to the generic per-command help block so + // `help --help` keeps asking about help itself. + return path.length > 0 ? path : undefined; + } + if (arg.startsWith("-")) { + if (consumesFollowingToken(args, index)) { + index++; + } + continue; + } + path.push(arg); + } + return path; +} diff --git a/packages/coding-agent/src/cli/public-command.ts b/packages/coding-agent/src/cli/public-command.ts index 252af2a98..5fa05c40c 100644 --- a/packages/coding-agent/src/cli/public-command.ts +++ b/packages/coding-agent/src/cli/public-command.ts @@ -18,6 +18,7 @@ import { import { handleDaemonCommand } from "./daemon-command.js"; import { runPs, runReap, runShutdownAll } from "./daemon-ps.js"; import { DAEMON_UPDATE_RESTART_COORDINATOR_FLAG } from "./daemon-update-restart.js"; +import { extractHelpCommandPath, rotateGlobalFlagsBeforeCommand } from "./global-flags.js"; export interface PublicCommandResult { handled: boolean; @@ -37,9 +38,13 @@ export async function handlePublicCommand(args: string[]): Promise { - args = normalizeLeadingDaemonSocketOption(args); - if (args[0] === "help" && isHelpCommandRequest(args.slice(1))) { - return printRequestedHelp(args.slice(1)); + args = rotateGlobalFlagsBeforeCommand(args); + // Global run flags are excluded from the help request, not forwarded as help + // arguments: `prime-agent --offline help` must print help, not chat the + // rotated argv to the model. + const helpPath = args[0] === "help" ? extractHelpCommandPath(args, 1) : undefined; + if (helpPath !== undefined && isHelpCommandRequest(helpPath)) { + return printRequestedHelp(helpPath); } const command = args[0]; @@ -154,19 +159,6 @@ async function runPublicCommand(args: string[]): Promise { } } -function normalizeLeadingDaemonSocketOption(args: string[]): string[] { - const option = args[0]; - if (option !== "--daemon-socket") { - return args; - } - const socketPath = args[1]; - const command = args[2]; - if (socketPath === undefined || (command !== "stop" && command !== "rename")) { - return args; - } - return [command, ...args.slice(3), option, socketPath]; -} - function continueWith(args: string[]): PublicCommandResult { return { handled: false, args, explicitAgentsView: false }; } diff --git a/packages/coding-agent/src/core/platform-fidelity.ts b/packages/coding-agent/src/core/platform-fidelity.ts new file mode 100644 index 000000000..05c750b50 --- /dev/null +++ b/packages/coding-agent/src/core/platform-fidelity.ts @@ -0,0 +1,280 @@ +import { closeSync, existsSync, openSync, readSync } from "node:fs"; +import { arch as osArch, platform as osPlatform, release as osRelease } from "node:os"; + +/** + * Platform-fidelity telemetry. + * + * `os_family` + `architecture` alone cannot answer two questions that gate + * dropping the standalone-Node bootstrap in favour of a single native binary: + * + * 1. How many Linux users are on musl (Alpine) instead of glibc? + * 2. How many x86_64 users run CPUs without AVX2, which the default + * Bun linux-x64 build requires? + * + * Every probe here is cheap (no subprocesses, bounded reads), memoised for the + * process, and must never throw: any failure degrades to "unknown". + */ + +export type TelemetryLibc = "glibc" | "musl" | "none" | "unknown"; + +/** + * "avx2" / "no_avx2" are measured. "avx2_assumed" is reported for Intel Macs, + * where the value is inferred rather than read (see detectCpuBaseline). + * "not_applicable" means the CPU is not x86_64, so AVX2 is not a concept that + * applies. "unknown" means x86_64 but no supported probe (e.g. Windows). + */ +export type TelemetryCpuBaseline = "avx2" | "no_avx2" | "avx2_assumed" | "not_applicable" | "unknown"; + +export interface PlatformFidelity { + /** C library family the process is linked against. */ + libc: TelemetryLibc; + /** glibc runtime version such as "2.39", else "unknown". */ + libc_version: string; + /** AVX2 availability on x86_64. */ + cpu_baseline: TelemetryCpuBaseline; + /** Kernel version, e.g. "6.8.0-45-generic" or "24.6.0". */ + os_release: string; + /** macOS product version such as "15.6"; "unknown" elsewhere. */ + os_product_version: string; +} + +/** Injectable environment probe so detection is testable without a real host. */ +export interface PlatformProbe { + platform: string; + arch: string; + release: () => string; + fileExists: (path: string) => boolean; + readTextFile: (path: string, maxBytes: number) => string | undefined; + glibcVersionRuntime: () => string | undefined; +} + +const UNKNOWN = "unknown" as const; +/** Kernel strings are short; cap anyway so a pathological value cannot bloat an event. */ +const MAX_VERSION_LENGTH = 64; +/** The first `flags:` line of /proc/cpuinfo is well inside this window. */ +const CPUINFO_READ_BYTES = 16_384; +/** SystemVersion.plist is ~500 bytes of XML. */ +const SYSTEM_VERSION_READ_BYTES = 4_096; +const SYSTEM_VERSION_PLIST = "/System/Library/CoreServices/SystemVersion.plist"; + +/** + * musl's dynamic loader is named per architecture and always lives at a fixed + * path, so an existence check is O(1) and does not require reading any file. + */ +const MUSL_LOADER_NAMES: Record = { + x64: ["x86_64"], + arm64: ["aarch64"], + arm: ["armhf", "arm"], + ia32: ["i386"], + ppc64: ["powerpc64le"], + s390x: ["s390x"], + riscv64: ["riscv64"], + loong64: ["loongarch64"], +}; + +function sanitizeVersion(value: string | undefined): string { + const trimmed = (value ?? "").trim(); + if (!trimmed) { + return UNKNOWN; + } + return trimmed.slice(0, MAX_VERSION_LENGTH); +} + +export function detectLibc(probe: PlatformProbe): { libc: TelemetryLibc; version: string } { + if (probe.platform !== "linux") { + return { libc: "none", version: UNKNOWN }; + } + + // The runtime report header observes how THIS process was actually linked, + // so it outranks filesystem heuristics: a Debian/Ubuntu host with the musl + // package installed ships a musl loader file but is not a musl host. The + // header is absent inside compiled musl binaries (glibcVersionRuntime is + // null there), which only means the decision falls through to the loader + // checks below — it must not terminate them early in musl's favour. + const glibcVersion = probe.glibcVersionRuntime(); + if (glibcVersion) { + return { libc: "glibc", version: sanitizeVersion(glibcVersion) }; + } + + const loaderNames = MUSL_LOADER_NAMES[probe.arch] ?? []; + for (const loaderName of loaderNames) { + for (const prefix of ["/lib", "/usr/lib"]) { + if (probe.fileExists(`${prefix}/ld-musl-${loaderName}.so.1`)) { + // musl exposes no runtime version symbol worth probing cheaply. + return { libc: "musl", version: UNKNOWN }; + } + } + } + + // No runtime version (some builds omit it): fall back to the presence of a + // glibc dynamic loader before giving up. + const glibcLoaders = [ + `/lib/ld-linux-${probe.arch === "arm64" ? "aarch64" : "x86-64"}.so.1`, + "/lib64/ld-linux-x86-64.so.1", + "/lib/ld-linux-aarch64.so.1", + "/lib/ld-linux.so.2", + ]; + if (glibcLoaders.some((path) => probe.fileExists(path))) { + return { libc: "glibc", version: UNKNOWN }; + } + + return { libc: UNKNOWN, version: UNKNOWN }; +} + +export function detectCpuBaseline(probe: PlatformProbe): TelemetryCpuBaseline { + if (probe.arch !== "x64") { + return "not_applicable"; + } + + if (probe.platform === "linux") { + const cpuinfo = probe.readTextFile("/proc/cpuinfo", CPUINFO_READ_BYTES); + if (!cpuinfo) { + return UNKNOWN; + } + const flagsLine = cpuinfo.split("\n").find((line) => line.startsWith("flags")); + if (!flagsLine) { + return UNKNOWN; + } + return / avx2( |$)/.test(`${flagsLine.slice(flagsLine.indexOf(":") + 1)} `) ? "avx2" : "no_avx2"; + } + + if (probe.platform === "darwin") { + // Reading machdep.cpu.leaf7_features needs a `sysctl` subprocess, which + // is not worth the startup cost for this segment. Every Intel Mac that + // can run a supported macOS (13+) is Haswell or newer, so AVX2 is + // present — reported as a distinct value so the inference stays visible. + return "avx2_assumed"; + } + + return UNKNOWN; +} + +export function detectOsProductVersion(probe: PlatformProbe): string { + if (probe.platform !== "darwin") { + return UNKNOWN; + } + const plist = probe.readTextFile(SYSTEM_VERSION_PLIST, SYSTEM_VERSION_READ_BYTES); + if (!plist) { + return UNKNOWN; + } + const match = /ProductVersion<\/key>\s*([^<]+)<\/string>/.exec(plist); + return sanitizeVersion(match?.[1]); +} + +export function detectPlatformFidelity(probe: PlatformProbe): PlatformFidelity { + let libc: TelemetryLibc = UNKNOWN; + let libcVersion: string = UNKNOWN; + try { + ({ libc, version: libcVersion } = detectLibc(probe)); + } catch { + libc = UNKNOWN; + libcVersion = UNKNOWN; + } + + let cpuBaseline: TelemetryCpuBaseline = UNKNOWN; + try { + cpuBaseline = detectCpuBaseline(probe); + } catch { + cpuBaseline = UNKNOWN; + } + + let release: string = UNKNOWN; + try { + release = sanitizeVersion(probe.release()); + } catch { + release = UNKNOWN; + } + + let productVersion: string = UNKNOWN; + try { + productVersion = detectOsProductVersion(probe); + } catch { + productVersion = UNKNOWN; + } + + return { + libc, + libc_version: libcVersion, + cpu_baseline: cpuBaseline, + os_release: release, + os_product_version: productVersion, + }; +} + +function readTextFilePrefix(path: string, maxBytes: number): string | undefined { + let fd: number | undefined; + try { + fd = openSync(path, "r"); + const buffer = Buffer.allocUnsafe(maxBytes); + const bytesRead = readSync(fd, buffer, 0, maxBytes, 0); + return buffer.toString("utf8", 0, bytesRead); + } catch { + return undefined; + } finally { + if (fd !== undefined) { + try { + closeSync(fd); + } catch { + // ignore + } + } + } +} + +function glibcVersionRuntime(): string | undefined { + try { + // Node and Bun both expose the linked glibc version in the diagnostic + // report header. Absent on musl and on non-glibc builds. + const report = process.report?.getReport?.(); + if (!report || typeof report !== "object") { + return undefined; + } + const header = (report as { header?: { glibcVersionRuntime?: unknown } }).header; + const version = header?.glibcVersionRuntime; + return typeof version === "string" && version.length > 0 ? version : undefined; + } catch { + return undefined; + } +} + +export function createDefaultPlatformProbe(): PlatformProbe { + return { + platform: osPlatform(), + arch: osArch(), + release: osRelease, + fileExists: (path) => { + try { + return existsSync(path); + } catch { + return false; + } + }, + readTextFile: readTextFilePrefix, + glibcVersionRuntime, + }; +} + +let cached: PlatformFidelity | undefined; + +/** Memoised per process: probes run at most once regardless of event volume. */ +export function platformFidelity(): PlatformFidelity { + if (!cached) { + try { + cached = detectPlatformFidelity(createDefaultPlatformProbe()); + } catch { + cached = { + libc: UNKNOWN, + libc_version: UNKNOWN, + cpu_baseline: UNKNOWN, + os_release: UNKNOWN, + os_product_version: UNKNOWN, + }; + } + } + return cached; +} + +/** Test-only: drop the memoised value. */ +export function resetPlatformFidelityCacheForTests(): void { + cached = undefined; +} diff --git a/packages/coding-agent/src/core/telemetry.ts b/packages/coding-agent/src/core/telemetry.ts index 15dbcb5fd..d781c9060 100644 --- a/packages/coding-agent/src/core/telemetry.ts +++ b/packages/coding-agent/src/core/telemetry.ts @@ -8,6 +8,7 @@ import { writeFileAtomicSync } from "../utils/atomic-file.js"; import type { AgentSession, AgentSessionEvent } from "./agent-session.js"; import type { AgentExecutionMode } from "./agent-session-config.js"; import type { AuthCredential, AuthStatus } from "./auth-storage.js"; +import { platformFidelity } from "./platform-fidelity.js"; import type { SettingsManager } from "./settings-manager.js"; import { isBuiltinSlashCommandName, resolveBuiltinSlashCommandName } from "./slash-commands.js"; @@ -475,12 +476,18 @@ export function telemetryAuthCategory( } function baseProperties(executionMode: TelemetryExecutionMode): TelemetryProperties { + const fidelity = platformFidelity(); return { version: VERSION, os_family: platform(), architecture: arch(), install_method: detectInstallMethod(), execution_mode: executionMode, + libc: fidelity.libc, + libc_version: fidelity.libc_version, + cpu_baseline: fidelity.cpu_baseline, + os_release: fidelity.os_release, + os_product_version: fidelity.os_product_version, }; } diff --git a/packages/coding-agent/src/modes/daemon/daemon-state-root.ts b/packages/coding-agent/src/modes/daemon/daemon-state-root.ts new file mode 100644 index 000000000..8870cc0e6 --- /dev/null +++ b/packages/coding-agent/src/modes/daemon/daemon-state-root.ts @@ -0,0 +1,85 @@ +import { dirname, isAbsolute, relative, resolve } from "node:path"; +import { getAgentDir } from "../../config.js"; +import { defaultDaemonSocketDir, defaultDaemonSocketPath, normalizeSocketPath } from "./daemon-socket.js"; +import { listDaemonSupervisorSocketPathsForAgentDir } from "./daemon-supervisor-ownership.js"; + +/** + * The state root an invocation reads and writes: the agent dir (config, logs, + * worker descriptors) and the default daemon socket dir. Both already follow + * HOME, TMPDIR and the agent-dir env override, so an isolated root resolves to + * isolated paths; this type just carries the pair around so daemon discovery can + * be scoped the same way instead of sweeping the whole machine. + */ +export interface DaemonStateRoot { + agentDir: string; + socketDir: string; + defaultSocketPath: string; +} + +export function currentDaemonStateRoot(): DaemonStateRoot { + return { + agentDir: getAgentDir(), + socketDir: defaultDaemonSocketDir(), + defaultSocketPath: normalizeSocketPath(defaultDaemonSocketPath()), + }; +} + +/** + * Predicate for "this daemon belongs to our state root". A daemon is ours when + * its socket sits in our socket dir, when it sits anywhere inside our agent dir, + * or when the supervisor ownership registry (itself scoped to our HOME) records + * it under our agent dir — the last two rules keep daemons on custom + * `--daemon-socket` paths in scope for the root that started them. + * + * The agent-dir rule is what keeps *hidden* supervisors reachable. A supervisor + * that has handed its runtime to a successor is no longer the registered owner, + * so the registry cannot name it; the OS socket sweep is the only thing that can + * still see it. Scoping by the registry alone would silently put those daemons + * out of reach of `daemon ps` and `shutdown --force` and leak them forever, which + * is the regression ENG-4603 covers. A socket inside our agent dir is + * unambiguously ours — another HOME resolves to another agent dir — so honouring + * it restores that reach without widening scope back to the whole machine. + * + * The registry read is deferred and memoised so the common case (every socket in + * our own socket dir) costs nothing, and a fresh matcher per sweep keeps results + * current for callers that poll. + * + * The socket dir is deliberately part of the root: it is per-uid, per-TMPDIR + * state, so two invocations that share TMPDIR also share that one daemon + * namespace no matter which HOME or agent dir they run under — they could not + * run concurrent default daemons on one socket path anyway. Scoping the dir away + * would orphan every unregistered listener there (workers, abandoned and + * pre-registry daemons), which `shutdown --force` must still reap. + */ +export function createDaemonStateRootMatcher( + root: DaemonStateRoot = currentDaemonStateRoot(), +): (socketPath: string) => boolean { + if (process.platform === "win32") { + // Windows daemons share one named pipe per machine, so there is nothing to scope. + return () => true; + } + const socketDir = resolve(root.socketDir); + const agentDir = resolve(root.agentDir); + let registeredSocketPaths: Set | undefined; + return (socketPath: string): boolean => { + const normalized = normalizeSocketPath(socketPath); + const directory = resolve(dirname(normalized)); + if (normalized === root.defaultSocketPath || directory === socketDir || isInside(directory, agentDir)) { + return true; + } + registeredSocketPaths ??= new Set(listDaemonSupervisorSocketPathsForAgentDir(root.agentDir)); + return registeredSocketPaths.has(normalized); + }; +} + +/** + * True when `directory` is `parent` itself or sits below it. `relative` is used + * rather than a prefix comparison so a sibling whose name merely starts with the + * parent's name (`/tmp/agent-other` against `/tmp/agent`) is not mistaken for a + * child, and so an absolute result on a different volume is rejected outright. + */ +function isInside(directory: string, parent: string): boolean { + if (directory === parent) return true; + const offset = relative(parent, directory); + return offset.length > 0 && !offset.startsWith("..") && !isAbsolute(offset); +} diff --git a/packages/coding-agent/src/modes/daemon/daemon-supervisor-ownership.ts b/packages/coding-agent/src/modes/daemon/daemon-supervisor-ownership.ts index ba6920114..47c354ec3 100644 --- a/packages/coding-agent/src/modes/daemon/daemon-supervisor-ownership.ts +++ b/packages/coding-agent/src/modes/daemon/daemon-supervisor-ownership.ts @@ -526,6 +526,65 @@ function isOwnerProcessAlive(pid: number): boolean { return true; } +/** + * Socket paths of the supervisors registered under `agentDir`, read straight + * from the registry record each daemon writes for itself. Callers use this to + * tell their own state root's daemons apart from daemons that belong to another + * HOME, agent dir, or socket dir on the same machine. + * + * Read-only and lock-free on purpose: records are written rename-atomically so a + * torn read is impossible, and a momentarily stale answer only affects discovery, + * never ownership. + * + * Daemons started before the registry moved out of the socket dir only have + * records in the legacy location, so that registry is read too (unless the + * caller overrides the registry). A record whose agent dir cannot be + * canonicalized (permissions, replaced paths, corrupt records) is skipped + * instead of aborting discovery for every other record. + */ +export function listDaemonSupervisorSocketPathsForAgentDir( + agentDir: string, + registryDir?: string, + legacyRegistryDir: string | undefined = registryDir === undefined ? legacyDaemonSupervisorRegistryDir() : undefined, +): string[] { + let canonicalAgentDir: string; + try { + canonicalAgentDir = canonicalizeFilesystemPath(agentDir); + } catch { + return []; + } + const directories = ownerDirectoriesForDiscovery(registryDir ?? defaultDaemonSupervisorRegistryDir()); + if (legacyRegistryDir) { + directories.push(...ownerDirectoriesForDiscovery(legacyRegistryDir)); + } + const socketPaths: string[] = []; + for (const directory of directories) { + const owner = readOwnerRecord(directory); + if (!owner) { + continue; + } + let canonicalOwnerAgentDir: string; + try { + canonicalOwnerAgentDir = canonicalizeFilesystemPath(owner.agentDir); + } catch { + continue; + } + if (canonicalOwnerAgentDir === canonicalAgentDir) { + socketPaths.push(normalizeSocketPath(owner.socketPath)); + } + } + return socketPaths; +} + +/** Non-mutating registry listing: discovery must never reclaim abandoned directories. */ +function ownerDirectoriesForDiscovery(registryDir: string): string[] { + try { + return listOwnerDirectories(registryDir); + } catch { + return []; + } +} + export async function assertDaemonSupervisorOwnerCurrent( owner: { generation: string; diff --git a/packages/coding-agent/test/ambient-env.ts b/packages/coding-agent/test/ambient-env.ts new file mode 100644 index 000000000..a686a1515 --- /dev/null +++ b/packages/coding-agent/test/ambient-env.ts @@ -0,0 +1,65 @@ +/** + * Ambient environment sanitation for tests. + * + * A running Prime Agent session exports its own runtime configuration into every + * shell it spawns. A developer who runs the test suite from inside such a session + * therefore inherits variables that silently change product behaviour: version + * checks short-circuit, and the npm bridge classifies itself as an internal daemon + * worker instead of a foreground migration. CI runs with a clean environment, so + * suites that cover those code paths pass in CI and fail locally for no visible + * reason. + * + * Tests that exercise update, version-check, or npm bridge behaviour must own these + * variables explicitly instead of inheriting whatever the host shell happens to + * export. + */ + +/** + * Every variable read by the update, version-check, and npm bridge code paths. + * Keep this in sync with `src/utils/version-check.ts` and `src/cli/npm-native-bridge.ts`. + */ +export const AMBIENT_RUNTIME_ENV_VARS: readonly string[] = [ + // src/utils/version-check.ts + "PI_OFFLINE", + "PI_SKIP_VERSION_CHECK", + "PRIME_AGENT_DOWNLOAD_BASE_URL", + // src/cli/npm-native-bridge.ts + "PI_STARTUP_BENCHMARK", + "PRIME_AGENT_INSTALL_DIR", + "PRIME_AGENT_INSTALL_METHOD", + "PRIME_AGENT_INTERACTIVE_SELF_UPDATE", + "PRIME_AGENT_INTERNAL_DAEMON_CATALOG", + "PRIME_AGENT_INTERNAL_DAEMON_WORKER", + "PRIME_AGENT_INTERNAL_OWNED_WORKER", + "PRIME_AGENT_MIGRATE_RETRY", +]; + +/** + * Removes the ambient variables from this process and returns a callback that puts + * the original values back. Call it from `beforeEach` and invoke the callback from + * `afterEach` so each test starts from a known environment. + */ +export function clearAmbientRuntimeEnv(): () => void { + const saved = new Map(); + for (const name of AMBIENT_RUNTIME_ENV_VARS) { + saved.set(name, process.env[name]); + delete process.env[name]; + } + return () => { + for (const [name, value] of saved) { + if (value === undefined) delete process.env[name]; + else process.env[name] = value; + } + }; +} + +/** + * Builds a child-process environment from this process with the ambient variables + * stripped, so spawned children do not inherit them. Overrides are applied last and + * may deliberately reintroduce any of the variables under test. + */ +export function ambientFreeEnv(overrides: NodeJS.ProcessEnv = {}): NodeJS.ProcessEnv { + const env: NodeJS.ProcessEnv = { ...process.env }; + for (const name of AMBIENT_RUNTIME_ENV_VARS) delete env[name]; + return { ...env, ...overrides }; +} diff --git a/packages/coding-agent/test/daemon-launch.test.ts b/packages/coding-agent/test/daemon-launch.test.ts index 26cc049ef..76e89b327 100644 --- a/packages/coding-agent/test/daemon-launch.test.ts +++ b/packages/coding-agent/test/daemon-launch.test.ts @@ -217,6 +217,14 @@ describe("shouldStartDaemonEarly", () => { ["json", ["--mode", "json", "hello"]], ["rpc", ["--mode", "rpc"]], ["no-session", ["--no-session"]], + ["escaped command word", ["--", "status"]], + ["print with a command word", ["--print", "status"]], + ["prompt that only starts like a command", ["--offline", "statuses", "of", "my", "agents"]], + ["extension flag with a command-like value", ["--extension-option", "status"]], + ["value flag before a -- separator", ["--cwd", "--", "status"]], + ["resume selector that is an @file reference", ["--resume", "@prompt.md", "status"]], + ["prompt-value flag before a command word", ["--system-prompt", "be terse", "status"]], + ["appended prompt-value flag before a command word", ["--append-system-prompt", "be terse", "status"]], ])("starts early for the %s client", (_label, args) => { expect(shouldStartDaemonEarly(args, false)).toBe(true); }); @@ -227,6 +235,13 @@ describe("shouldStartDaemonEarly", () => { ["version", ["--version"]], ["model listing", ["--list-models"]], ["management command after global flags", ["--daemon-socket", "/tmp/prime.sock", "status"]], + ["nested command after global flags", ["--offline", "model", "list"]], + ["help after global flags", ["--offline", "help"]], + ["nested help after global flags", ["--offline", "help", "status"]], + ["help before global flags", ["help", "--verbose"]], + ["boolean run flag before the command", ["--verbose", "model", "list"]], + ["unknown short option before the command", ["-x", "model", "list"]], + ["inline extension flag before the command", ["--extension-option=status", "model", "list"]], ["startup benchmark", []], ])("does not start early for %s", (label, args) => { expect(shouldStartDaemonEarly(args, label === "startup benchmark")).toBe(false); diff --git a/packages/coding-agent/test/daemon-state-root.test.ts b/packages/coding-agent/test/daemon-state-root.test.ts new file mode 100644 index 000000000..f4b2e5fc1 --- /dev/null +++ b/packages/coding-agent/test/daemon-state-root.test.ts @@ -0,0 +1,135 @@ +import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { defaultDaemonSocketDir } from "../src/modes/daemon/daemon-socket.js"; +import { + createDaemonStateRootMatcher, + currentDaemonStateRoot, + type DaemonStateRoot, +} from "../src/modes/daemon/daemon-state-root.js"; +import { acquireDaemonSupervisorOwnership } from "../src/modes/daemon/daemon-supervisor-ownership.js"; + +const registryDirEnv = "PRIME_AGENT_INTERNAL_DAEMON_SUPERVISOR_REGISTRY_DIR"; +const previousRegistryDirEnv = process.env[registryDirEnv]; +const cleanupDirs: string[] = []; +const releaseOwnerships: Array<() => Promise> = []; + +afterEach(async () => { + while (releaseOwnerships.length > 0) { + await releaseOwnerships.pop()?.(); + } + if (previousRegistryDirEnv === undefined) { + delete process.env[registryDirEnv]; + } else { + process.env[registryDirEnv] = previousRegistryDirEnv; + } + while (cleanupDirs.length > 0) { + const dir = cleanupDirs.pop(); + if (dir) rmSync(dir, { recursive: true, force: true }); + } +}); + +function createRoot(): { root: DaemonStateRoot; base: string; registryDir: string } { + const base = mkdtempSync(join(tmpdir(), "state-root-")); + cleanupDirs.push(base); + const agentDir = join(base, "agent"); + const socketDir = join(base, "sockets"); + const registryDir = join(base, "registry"); + mkdirSync(agentDir, { recursive: true }); + mkdirSync(socketDir, { recursive: true }); + process.env[registryDirEnv] = registryDir; + return { + base, + registryDir, + root: { agentDir, socketDir, defaultSocketPath: join(socketDir, "daemon.sock") }, + }; +} + +async function registerDaemon( + registryDir: string, + agentDir: string, + socketPath: string, + generation: string, +): Promise { + const ownership = await acquireDaemonSupervisorOwnership({ + agentDir, + appVersion: "test", + descriptorDir: join(agentDir, "workers", generation), + generation, + registryDir, + socketPath, + }); + releaseOwnerships.push(() => ownership.release()); +} + +describe.runIf(process.platform !== "win32")("daemon state root scoping", () => { + it("claims sockets in our own socket dir", () => { + const { root } = createRoot(); + const belongsToStateRoot = createDaemonStateRootMatcher(root); + expect(belongsToStateRoot(join(root.socketDir, "daemon.sock"))).toBe(true); + expect(belongsToStateRoot(join(root.socketDir, "worker-abc.sock"))).toBe(true); + }); + + it("claims a custom socket path registered under our agent dir", async () => { + const { root, base, registryDir } = createRoot(); + const customSocket = join(base, "custom.sock"); + await registerDaemon(registryDir, root.agentDir, customSocket, "ours"); + expect(createDaemonStateRootMatcher(root)(customSocket)).toBe(true); + }); + + it("disowns a daemon registered under a different agent dir", async () => { + const { root, base, registryDir } = createRoot(); + const otherAgentDir = join(base, "other-agent"); + const otherSocket = join(base, "other.sock"); + mkdirSync(otherAgentDir, { recursive: true }); + await registerDaemon(registryDir, otherAgentDir, otherSocket, "theirs"); + expect(createDaemonStateRootMatcher(root)(otherSocket)).toBe(false); + }); + + it("disowns an unregistered daemon outside our socket dir", () => { + const { root, base } = createRoot(); + expect(createDaemonStateRootMatcher(root)(join(base, "stranger.sock"))).toBe(false); + }); + + // A supervisor that has handed its runtime to a successor is no longer the + // registered owner, so only the OS socket sweep can still see it. It must stay + // in scope or `shutdown --force` leaks it, which is the ENG-4603 regression. + it("claims a hidden unregistered supervisor whose socket sits in our agent dir", () => { + const { root } = createRoot(); + const belongsToStateRoot = createDaemonStateRootMatcher(root); + expect(belongsToStateRoot(join(root.agentDir, "daemon.sock"))).toBe(true); + expect(belongsToStateRoot(join(root.agentDir, "nested", "worker-command.sock"))).toBe(true); + }); + + it("does not mistake an agent dir with a shared name prefix for our own", () => { + const { root, base } = createRoot(); + expect(createDaemonStateRootMatcher(root)(join(`${base}/agent-other`, "daemon.sock"))).toBe(false); + }); + + it("still answers when the registry does not exist yet", () => { + const { root, base } = createRoot(); + rmSync(root.socketDir, { recursive: true, force: true }); + const belongsToStateRoot = createDaemonStateRootMatcher(root); + expect(belongsToStateRoot(join(root.socketDir, "daemon.sock"))).toBe(true); + expect(belongsToStateRoot(join(base, "stranger.sock"))).toBe(false); + }); + + it("follows the environment instead of a value captured at import time", () => { + const previousAgentDir = process.env.PRIME_AGENT_CODING_AGENT_DIR; + const base = mkdtempSync(join(tmpdir(), "state-root-env-")); + cleanupDirs.push(base); + process.env.PRIME_AGENT_CODING_AGENT_DIR = join(base, "agent"); + try { + const root = currentDaemonStateRoot(); + expect(root.agentDir).toBe(join(base, "agent")); + expect(root.socketDir).toBe(defaultDaemonSocketDir()); + } finally { + if (previousAgentDir === undefined) { + delete process.env.PRIME_AGENT_CODING_AGENT_DIR; + } else { + process.env.PRIME_AGENT_CODING_AGENT_DIR = previousAgentDir; + } + } + }); +}); diff --git a/packages/coding-agent/test/daemon-supervisor-ownership.test.ts b/packages/coding-agent/test/daemon-supervisor-ownership.test.ts index 21941b3b0..41c07928e 100644 --- a/packages/coding-agent/test/daemon-supervisor-ownership.test.ts +++ b/packages/coding-agent/test/daemon-supervisor-ownership.test.ts @@ -9,6 +9,7 @@ import { acquireDaemonShutdownAdmission, acquireDaemonSupervisorOwnership, assertDaemonSupervisorOwnerCurrent, + listDaemonSupervisorSocketPathsForAgentDir, persistDaemonStartupFenceFromOwner, } from "../src/modes/daemon/daemon-supervisor-ownership.js"; import * as childProcessModule from "../src/utils/child-process.js"; @@ -143,6 +144,57 @@ describe("daemon supervisor ownership registry", () => { await legacyOwner.release(); }); + it("lists supervisor sockets from the legacy registry for the same agent dir", async () => { + const paths = createPaths(); + const legacyDir = join(paths.root, "legacy-registry"); + const legacyOwner = await acquireDaemonSupervisorOwnership({ + agentDir: paths.agentDir, + appVersion: "test", + descriptorDir: paths.descriptorDir, + generation: "legacy-discovery-owner", + registryDir: legacyDir, + socketPath: paths.socketPath, + }); + + // The pre-move record keeps the daemon discoverable by the state root + // that owns the agent dir; the current registry alone has nothing. + expect(listDaemonSupervisorSocketPathsForAgentDir(paths.agentDir, paths.registryDir)).toEqual([]); + expect(listDaemonSupervisorSocketPathsForAgentDir(paths.agentDir, paths.registryDir, legacyDir)).toEqual([ + legacyOwner.record.socketPath, + ]); + + // The legacy registry scopes by agent dir exactly like the current one. + expect( + listDaemonSupervisorSocketPathsForAgentDir(join(paths.root, "other-agent"), paths.registryDir, legacyDir), + ).toEqual([]); + + await legacyOwner.release(); + }); + + it("skips records whose agent dir cannot be canonicalized instead of aborting discovery", async () => { + const paths = createPaths(); + const owner = await acquire(paths, "discovery-owner"); + const brokenDir = join(paths.registryDir, "broken.owner"); + mkdirSync(brokenDir, { recursive: true, mode: 0o700 }); + // A valid record shape whose agent dir path can no longer be canonicalized + // (ENOTDIR under /dev/null): one stale record must not hide the others. + const broken = { + ...owner.record, + token: "broken-token", + generation: "broken-owner", + agentDir: "/dev/null/agent", + socketPath: join(paths.root, "broken.sock"), + }; + writeFileSync(join(brokenDir, "owner.json"), `${JSON.stringify(broken, null, 2)}\n`); + + expect(listDaemonSupervisorSocketPathsForAgentDir(paths.agentDir, paths.registryDir)).toEqual([ + owner.record.socketPath, + ]); + expect(listDaemonSupervisorSocketPathsForAgentDir("/dev/null/agent", paths.registryDir)).toEqual([]); + + await owner.release(); + }); + it("legacy registry reads never reclaim abandoned legacy directories", async () => { const paths = createPaths(); const legacyDir = join(paths.root, "legacy-registry"); diff --git a/packages/coding-agent/test/mcp-command.test.ts b/packages/coding-agent/test/mcp-command.test.ts index 81f19581b..6b40a9abb 100644 --- a/packages/coding-agent/test/mcp-command.test.ts +++ b/packages/coding-agent/test/mcp-command.test.ts @@ -33,6 +33,12 @@ describe("MCP management commands", () => { }); }); + it("rejects a global run flag rotated ahead of the -- separator instead of leaking it into the child command", () => { + expect(() => parseMcpAddArgs(["local", "--offline", "--", "node", "server.js"])).toThrow( + "Unknown MCP add option: --offline", + ); + }); + it("accepts environment names inherited from Object.prototype", () => { const { config } = parseMcpAddArgs([ "local", diff --git a/packages/coding-agent/test/native-installer.test.ts b/packages/coding-agent/test/native-installer.test.ts index 42e741b94..f9d6893c1 100644 --- a/packages/coding-agent/test/native-installer.test.ts +++ b/packages/coding-agent/test/native-installer.test.ts @@ -60,7 +60,14 @@ let fixtureDispatcher: Agent; function publish( version: string, - options: { broken?: boolean; missing?: boolean; link?: boolean; installer?: string; libstdcxx?: boolean } = {}, + options: { + broken?: boolean; + missing?: boolean; + link?: boolean; + installer?: string; + libstdcxx?: boolean; + slow?: number; + } = {}, ) { const source = mkdtempSync(join(root, "archive-")); for (const asset of assets) { @@ -70,6 +77,7 @@ function publish( } writeFileSync(join(source, "package.json"), JSON.stringify({ version })); writeFileSync(join(source, "install.sh"), options.installer ?? readFileSync(installer)); + const startup = options.slow ? `sleep ${options.slow}\n` : ""; // A musl host without libstdc++ fails in the loader, exactly as reproduced on Alpine. const missingLibstdcxx = `#!/bin/sh printf 'Error loading shared library libstdc++.so.6: No such file or directory (needed by %s)\\n' "$0" >&2 @@ -83,7 +91,8 @@ exit 1 ? missingLibstdcxx : options.broken ? "#!/bin/sh\nexit 1\n" - : `#!/bin/sh\nprintf '%s\\n' '${version}'\n`, + : `#!/bin/sh\n${startup}printf '%s\\n' '${version}'\n`, + { mode: 0o755 }, ); if (options.link) symlinkSync("/tmp", join(source, "outside")); @@ -1099,6 +1108,49 @@ exec /bin/${operation} "$@" expect(existsSync(join(home, "data/prime-agent/.install-lock"))).toBe(false); }); + it("reports a slow first run as a timeout and installs it within a larger budget", async () => { + publish("1.0.0", { slow: 5 }); + const impatient = await install("1.0.0", { PRIME_AGENT_PROBE_TIMEOUT_SECONDS: "2" }); + expect(impatient.code, impatient.output).not.toBe(0); + expect(impatient.output).toContain("probe timed out after 2 seconds"); + expect(impatient.output).toContain("did not answer within 2 seconds"); + expect(impatient.output).not.toContain("cannot run on this machine"); + expect(existsSync(command())).toBe(false); + const patient = await install("1.0.0", { PRIME_AGENT_PROBE_TIMEOUT_SECONDS: "60" }); + expect(patient.code, patient.output).toBe(0); + expect(existsSync(command())).toBe(true); + }, 90000); + + it("reads a leading-zero probe timeout override as decimal, not octal", async () => { + const harness = join(root, "probe-timeout.sh"); + writeFileSync( + harness, + readFileSync(installer, "utf8").replace( + /\nmain "\$@"\s*$/, + () => + '\nfor value in 010 08 09 000 600 601 ""; do\n' + + '\tPRIME_AGENT_PROBE_TIMEOUT_SECONDS="$value"\n' + + "\tnative_probe_timeout=$(prime_agent_native_probe_timeout)\n" + + "\tnative_probe_deadline=$(($(date +%s) + native_probe_timeout))\n" + + "\tprintf '%s\\n' \"$native_probe_timeout\"\n" + + "done\n", + ), + ); + const result = await install("", {}, harness); + expect(result.code, result.output).toBe(0); + expect(result.output.trim().split("\n")).toEqual(["10", "8", "9", "60", "600", "60", "60"]); + }); + + it("installs a slow first run within a leading-zero timeout budget read as decimal", async () => { + publish("1.0.0", { slow: 5 }); + // "09" previously aborted the deadline arithmetic under set -eu; as a + // decimal 9-second budget it must cover a 5-second first run. + const result = await install("1.0.0", { PRIME_AGENT_PROBE_TIMEOUT_SECONDS: "09" }); + expect(result.code, result.output).toBe(0); + expect(result.output).not.toContain("probe timed out"); + expect(existsSync(command())).toBe(true); + }, 90000); + it("reports the supported native platform without installation or release discovery", async () => { const result = await install("--native-platform", { PRIME_AGENT_DOWNLOAD_BASE_URL: "http://127.0.0.1:1" }); expect(result).toEqual({ code: 0, output: platform }); diff --git a/packages/coding-agent/test/native-probe-timeout.test.ts b/packages/coding-agent/test/native-probe-timeout.test.ts index c62f3f9b1..c36b8b0d3 100644 --- a/packages/coding-agent/test/native-probe-timeout.test.ts +++ b/packages/coding-agent/test/native-probe-timeout.test.ts @@ -77,6 +77,7 @@ printf '%s\\n' '${version}' PRIME_AGENT_INSTALL_DIR: managed, BLOCK_ARGUMENT: argument, PROBE_PIDS: probePids, + PRIME_AGENT_PROBE_TIMEOUT_SECONDS: "10", }, stdio: ["ignore", "pipe", "pipe"], timeout: operation === "recovery" ? 27000 : 17000, diff --git a/packages/coding-agent/test/npm-native-bridge.test.ts b/packages/coding-agent/test/npm-native-bridge.test.ts index 536f0f58e..48cf5dabf 100644 --- a/packages/coding-agent/test/npm-native-bridge.test.ts +++ b/packages/coding-agent/test/npm-native-bridge.test.ts @@ -18,6 +18,7 @@ import { dirname, join, resolve } from "node:path"; import { pathToFileURL } from "node:url"; import { transformSync } from "esbuild"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { ambientFreeEnv } from "./ambient-env.js"; let root: string; let home: string; @@ -60,13 +61,15 @@ function native(version = "1.0.0", suffix = "", platform = `${process.platform}- function start(args: string[] = [], extra: NodeJS.ProcessEnv = {}) { const child = spawn(process.execPath, [entry, ...args], { - env: { - ...process.env, + // The bridge branches on update and daemon-worker variables, so the child gets a + // sanitised environment instead of whatever the host shell exports. Cases that + // cover those branches pass the variable through `extra`. + env: ambientFreeEnv({ HOME: home, XDG_DATA_HOME: join(home, "data"), PRIME_AGENT_ALLOW_INSECURE_HTTP_FOR_TESTS: "1", ...extra, - }, + }), cwd: home, stdio: ["ignore", "pipe", "pipe"], }); diff --git a/packages/coding-agent/test/platform-fidelity.test.ts b/packages/coding-agent/test/platform-fidelity.test.ts new file mode 100644 index 000000000..c7b743e37 --- /dev/null +++ b/packages/coding-agent/test/platform-fidelity.test.ts @@ -0,0 +1,247 @@ +import { describe, expect, it } from "vitest"; +import { + createDefaultPlatformProbe, + detectCpuBaseline, + detectLibc, + detectOsProductVersion, + detectPlatformFidelity, + type PlatformProbe, + platformFidelity, + resetPlatformFidelityCacheForTests, +} from "../src/core/platform-fidelity.js"; + +function probe(overrides: Partial = {}): PlatformProbe { + return { + platform: "linux", + arch: "x64", + release: () => "6.8.0-45-generic", + fileExists: () => false, + readTextFile: () => undefined, + glibcVersionRuntime: () => undefined, + ...overrides, + }; +} + +const AVX2_CPUINFO = [ + "processor\t: 0", + "vendor_id\t: GenuineIntel", + "flags\t\t: fpu vme de pse tsc msr pae sse2 avx avx2 bmi2", + "", +].join("\n"); + +const NO_AVX2_CPUINFO = [ + "processor\t: 0", + "vendor_id\t: GenuineIntel", + "flags\t\t: fpu vme de pse tsc msr pae sse2 avx", + "", +].join("\n"); + +const SYSTEM_VERSION_PLIST = ` + + + ProductName + macOS + ProductVersion + 15.6 + +`; + +describe("detectLibc", () => { + it("reports musl when the musl loader for the architecture exists", () => { + expect(detectLibc(probe({ fileExists: (path) => path === "/lib/ld-musl-x86_64.so.1" }))).toEqual({ + libc: "musl", + version: "unknown", + }); + expect(detectLibc(probe({ arch: "arm64", fileExists: (path) => path === "/lib/ld-musl-aarch64.so.1" }))).toEqual({ + libc: "musl", + version: "unknown", + }); + }); + + it("prefers the glibc runtime over a musl loader file when both look present", () => { + // A glibc process on Debian/Ubuntu with the musl package installed + // still reports glibc: the runtime header sees the actual linkage. + const result = detectLibc( + probe({ fileExists: (path) => path.includes("ld-musl"), glibcVersionRuntime: () => "2.39" }), + ); + expect(result).toEqual({ libc: "glibc", version: "2.39" }); + }); + + it("reports glibc with the runtime version when the report header exposes it", () => { + expect(detectLibc(probe({ glibcVersionRuntime: () => "2.39" }))).toEqual({ + libc: "glibc", + version: "2.39", + }); + }); + + it("falls back to the glibc loader when no runtime version is exposed", () => { + expect(detectLibc(probe({ fileExists: (path) => path === "/lib64/ld-linux-x86-64.so.1" }))).toEqual({ + libc: "glibc", + version: "unknown", + }); + }); + + it("falls back to the 32-bit glibc loader path on ia32", () => { + expect(detectLibc(probe({ arch: "ia32", fileExists: (path) => path === "/lib/ld-linux.so.2" }))).toEqual({ + libc: "glibc", + version: "unknown", + }); + }); + + it("reports none off Linux and unknown when nothing matches", () => { + expect(detectLibc(probe({ platform: "darwin", arch: "arm64" }))).toEqual({ + libc: "none", + version: "unknown", + }); + expect(detectLibc(probe({ platform: "win32" })).libc).toBe("none"); + expect(detectLibc(probe()).libc).toBe("unknown"); + }); + + it("truncates an absurd glibc version instead of shipping it whole", () => { + const result = detectLibc(probe({ glibcVersionRuntime: () => "2.".padEnd(500, "9") })); + expect(result.libc).toBe("glibc"); + expect(result.version).toHaveLength(64); + }); +}); + +describe("detectCpuBaseline", () => { + it("reads AVX2 out of /proc/cpuinfo on linux-x64", () => { + expect(detectCpuBaseline(probe({ readTextFile: () => AVX2_CPUINFO }))).toBe("avx2"); + expect(detectCpuBaseline(probe({ readTextFile: () => NO_AVX2_CPUINFO }))).toBe("no_avx2"); + }); + + it("does not confuse avx512 or avx for avx2", () => { + const avx512 = "flags\t\t: sse2 avx avx512f avx512dq\n"; + expect(detectCpuBaseline(probe({ readTextFile: () => avx512 }))).toBe("no_avx2"); + }); + + it("matches avx2 at the end of the flags line", () => { + expect(detectCpuBaseline(probe({ readTextFile: () => "flags\t\t: sse2 avx avx2" }))).toBe("avx2"); + }); + + it("returns unknown when /proc/cpuinfo is unreadable or has no flags line", () => { + expect(detectCpuBaseline(probe())).toBe("unknown"); + expect(detectCpuBaseline(probe({ readTextFile: () => "processor\t: 0\n" }))).toBe("unknown"); + }); + + it("reports not_applicable for non-x64 architectures", () => { + expect(detectCpuBaseline(probe({ arch: "arm64" }))).toBe("not_applicable"); + expect(detectCpuBaseline(probe({ platform: "darwin", arch: "arm64" }))).toBe("not_applicable"); + }); + + it("marks darwin x64 as assumed rather than measured", () => { + expect(detectCpuBaseline(probe({ platform: "darwin", arch: "x64" }))).toBe("avx2_assumed"); + }); + + it("returns unknown on x64 platforms with no probe", () => { + expect(detectCpuBaseline(probe({ platform: "win32", arch: "x64" }))).toBe("unknown"); + }); +}); + +describe("detectOsProductVersion", () => { + it("parses the macOS product version from SystemVersion.plist", () => { + expect(detectOsProductVersion(probe({ platform: "darwin", readTextFile: () => SYSTEM_VERSION_PLIST }))).toBe( + "15.6", + ); + }); + + it("returns unknown off darwin and when the plist is missing or malformed", () => { + expect(detectOsProductVersion(probe({ readTextFile: () => SYSTEM_VERSION_PLIST }))).toBe("unknown"); + expect(detectOsProductVersion(probe({ platform: "darwin" }))).toBe("unknown"); + expect(detectOsProductVersion(probe({ platform: "darwin", readTextFile: () => "" }))).toBe("unknown"); + }); +}); + +describe("detectPlatformFidelity", () => { + it("describes an Alpine linux-x64 host without AVX2", () => { + expect( + detectPlatformFidelity( + probe({ + fileExists: (path) => path === "/lib/ld-musl-x86_64.so.1", + readTextFile: () => NO_AVX2_CPUINFO, + release: () => "5.15.0-alpine", + }), + ), + ).toEqual({ + libc: "musl", + libc_version: "unknown", + cpu_baseline: "no_avx2", + os_release: "5.15.0-alpine", + os_product_version: "unknown", + }); + }); + + it("describes a glibc host that merely has the musl package installed", () => { + expect( + detectPlatformFidelity( + probe({ + fileExists: (path) => path === "/lib/ld-musl-x86_64.so.1", + readTextFile: () => AVX2_CPUINFO, + glibcVersionRuntime: () => "2.39", + release: () => "6.8.0-45-generic", + }), + ), + ).toEqual({ + libc: "glibc", + libc_version: "2.39", + cpu_baseline: "avx2", + os_release: "6.8.0-45-generic", + os_product_version: "unknown", + }); + }); + + it("describes a glibc linux-arm64 host", () => { + expect(detectPlatformFidelity(probe({ arch: "arm64", glibcVersionRuntime: () => "2.35" }))).toEqual({ + libc: "glibc", + libc_version: "2.35", + cpu_baseline: "not_applicable", + os_release: "6.8.0-45-generic", + os_product_version: "unknown", + }); + }); + + it("never throws when every probe fails", () => { + const broken: PlatformProbe = { + platform: "linux", + arch: "x64", + release: () => { + throw new Error("release failed"); + }, + fileExists: () => { + throw new Error("stat failed"); + }, + readTextFile: () => { + throw new Error("read failed"); + }, + glibcVersionRuntime: () => { + throw new Error("report failed"); + }, + }; + expect(detectPlatformFidelity(broken)).toEqual({ + libc: "unknown", + libc_version: "unknown", + cpu_baseline: "unknown", + os_release: "unknown", + os_product_version: "unknown", + }); + }); +}); + +describe("platformFidelity", () => { + it("memoises the result and returns values from the live host probe", () => { + resetPlatformFidelityCacheForTests(); + const first = platformFidelity(); + expect(platformFidelity()).toBe(first); + expect(["glibc", "musl", "none", "unknown"]).toContain(first.libc); + expect(["avx2", "no_avx2", "avx2_assumed", "not_applicable", "unknown"]).toContain(first.cpu_baseline); + expect(first.os_release).not.toBe(""); + }); + + it("builds a usable default probe for the current host", () => { + const live = createDefaultPlatformProbe(); + expect(live.platform).toBe(process.platform); + expect(live.arch).toBe(process.arch); + expect(live.fileExists("/definitely/not/here")).toBe(false); + expect(live.readTextFile("/definitely/not/here", 16)).toBeUndefined(); + }); +}); diff --git a/packages/coding-agent/test/public-command.test.ts b/packages/coding-agent/test/public-command.test.ts index 5fc4ae676..1aa057195 100644 --- a/packages/coding-agent/test/public-command.test.ts +++ b/packages/coding-agent/test/public-command.test.ts @@ -120,6 +120,182 @@ describe("public command routing", () => { expect(mocks.mcpCommands).toEqual([["add", "local", "--", "node", "server file.js", "--stdio"]]); }); + it("runs a management command written after global flags", async () => { + await expect(handlePublicCommand(["--offline", "model", "list", "sonnet"])).resolves.toMatchObject({ + handled: false, + args: [INTERNAL_RUNTIME_COMMAND_MARKER, "--list-models", "sonnet", "--offline"], + }); + await expect(handlePublicCommand(["--offline", "list", "--json"])).resolves.toMatchObject({ handled: true }); + expect(mocks.daemonCommands).toEqual([["daemon", "list", "--json", "--offline"]]); + }); + + it("keeps rotated global flags ahead of a -- separator", async () => { + await expect( + handlePublicCommand(["--offline", "mcp", "add", "local", "--", "node", "server file.js", "--stdio"]), + ).resolves.toMatchObject({ handled: true }); + expect(mocks.mcpCommands).toEqual([["add", "local", "--offline", "--", "node", "server file.js", "--stdio"]]); + + await expect( + handlePublicCommand([ + "--daemon-socket", + "/tmp/prime.sock", + "schedule", + "add", + "worker", + "0 9 * * 1-5", + "--", + "Check open work", + ]), + ).resolves.toMatchObject({ handled: true }); + expect(mocks.daemonCommands.at(-1)).toEqual([ + "daemon", + "cron", + "add", + "worker", + "0 9 * * 1-5", + "--daemon-socket", + "/tmp/prime.sock", + "--", + "Check open work", + ]); + }); + + it("rejects a rotated global flag the command does not accept instead of chatting", async () => { + await expect( + handlePublicCommand(["--daemon-socket", "/tmp/prime.sock", "status", "--json"]), + ).resolves.toMatchObject({ handled: true }); + expect(mocks.psCalls).toEqual([]); + expect(process.exitCode).toBe(1); + expect(console.error).toHaveBeenCalledWith(expect.stringContaining("Unknown option for status: --daemon-socket")); + }); + + it("forwards a custom daemon socket written before stop", async () => { + await expect( + handlePublicCommand(["--daemon-socket", "/tmp/custom-daemon.sock", "stop", "worker"]), + ).resolves.toMatchObject({ handled: true }); + expect(mocks.daemonCommands).toEqual([ + ["daemon", "kill", "worker", "--daemon-socket", "/tmp/custom-daemon.sock"], + ]); + }); + + it("keeps a command word after -- as message text", async () => { + await expect(handlePublicCommand(["--", "status"])).resolves.toEqual({ + handled: false, + args: ["--", "status"], + explicitAgentsView: false, + }); + expect(mocks.psCalls).toEqual([]); + }); + + it("keeps a positional after a prompt-value flag on the message path", async () => { + for (const flag of ["--system-prompt", "--append-system-prompt"]) { + const args = [flag, "--offline", "status"]; + await expect(handlePublicCommand(args)).resolves.toEqual({ + handled: false, + args, + explicitAgentsView: false, + }); + } + }); + + it("keeps a version request ahead of command routing instead of rotating it", async () => { + const args = ["--version", "status"]; + await expect(handlePublicCommand(args)).resolves.toEqual({ + handled: false, + args, + explicitAgentsView: false, + }); + await expect(handlePublicCommand(["-v", "status"])).resolves.toEqual({ + handled: false, + args: ["-v", "status"], + explicitAgentsView: false, + }); + await expect(handlePublicCommand(args)).resolves.toEqual({ + handled: false, + args, + explicitAgentsView: false, + }); + }); + + it("keeps an unknown long option's value out of command routing", async () => { + await expect(handlePublicCommand(["--extension-option", "status"])).resolves.toEqual({ + handled: false, + args: ["--extension-option", "status"], + explicitAgentsView: false, + }); + await expect(handlePublicCommand(["--extension-option", "statuses", "of", "my", "agents"])).resolves.toEqual({ + handled: false, + args: ["--extension-option", "statuses", "of", "my", "agents"], + explicitAgentsView: false, + }); + expect(mocks.psCalls).toEqual([]); + expect(mocks.daemonCommands).toEqual([]); + }); + + it("still rotates a command written after flags that take no value", async () => { + await expect(handlePublicCommand(["--verbose", "model", "list"])).resolves.toMatchObject({ + handled: false, + args: [INTERNAL_RUNTIME_COMMAND_MARKER, "--list-models", "--verbose"], + }); + await expect(handlePublicCommand(["-x", "model", "list"])).resolves.toMatchObject({ + handled: false, + args: [INTERNAL_RUNTIME_COMMAND_MARKER, "--list-models", "-x"], + }); + await expect(handlePublicCommand(["--extension-option=status", "model", "list"])).resolves.toMatchObject({ + handled: false, + args: [INTERNAL_RUNTIME_COMMAND_MARKER, "--list-models", "--extension-option=status"], + }); + }); + + it("keeps text after -- following a value flag on the message path", async () => { + await expect(handlePublicCommand(["--cwd", "--", "status"])).resolves.toEqual({ + handled: false, + args: ["--cwd", "--", "status"], + explicitAgentsView: false, + }); + expect(mocks.psCalls).toEqual([]); + }); + + it("keeps a resume @file reference free instead of consuming it as a selector", async () => { + await expect(handlePublicCommand(["--resume", "@prompt.md", "status"])).resolves.toEqual({ + handled: false, + args: ["--resume", "@prompt.md", "status"], + explicitAgentsView: false, + }); + expect(mocks.psCalls).toEqual([]); + expect(mocks.daemonCommands).toEqual([]); + }); + + it("keeps the positional of a print run as the message", async () => { + await expect(handlePublicCommand(["--print", "status"])).resolves.toEqual({ + handled: false, + args: ["--print", "status"], + explicitAgentsView: false, + }); + await expect(handlePublicCommand(["-p", "--offline", "list"])).resolves.toEqual({ + handled: false, + args: ["-p", "--offline", "list"], + explicitAgentsView: false, + }); + expect(mocks.psCalls).toEqual([]); + expect(mocks.daemonCommands).toEqual([]); + }); + + it("leaves a prompt that only starts like a command alone", async () => { + await expect(handlePublicCommand(["--offline", "statuses", "of", "my", "agents"])).resolves.toEqual({ + handled: false, + args: ["--offline", "statuses", "of", "my", "agents"], + explicitAgentsView: false, + }); + expect(mocks.psCalls).toEqual([]); + }); + + it("rejects a removed command written after global flags", async () => { + await expect(handlePublicCommand(["--offline", "install", "pkg"])).resolves.toMatchObject({ handled: true }); + expect(process.exitCode).toBe(1); + expect(console.error).toHaveBeenCalledWith(expect.stringContaining("Unknown command: install")); + }); + it("routes agent operations through the internal protocol adapter", async () => { await expect(handlePublicCommand(["list", "--all", "--json"])).resolves.toMatchObject({ handled: true }); expect(mocks.daemonCommands).toEqual([["daemon", "list", "--all", "--json"]]); @@ -312,6 +488,65 @@ describe("public command routing", () => { }); }); + it("keeps the help topic when an explicit help flag follows it", async () => { + await expect(handlePublicCommand(["help", "status", "--help"])).resolves.toMatchObject({ handled: true }); + expect(console.log).toHaveBeenCalledWith(expect.stringContaining("prime-agent status [--json]")); + await expect(handlePublicCommand(["--offline", "help", "mcp", "add", "-h"])).resolves.toMatchObject({ + handled: true, + }); + expect(console.log).toHaveBeenCalledWith(expect.stringContaining("prime-agent mcp add ")); + await expect(handlePublicCommand(["help", "--help"])).resolves.toMatchObject({ handled: true }); + expect(console.log).toHaveBeenLastCalledWith(expect.not.stringContaining("prime-agent status [--json]")); + expect(mocks.daemonCommands).toEqual([]); + }); + + it("treats help written after global flags as a help request", async () => { + await expect(handlePublicCommand(["--offline", "help"])).resolves.toMatchObject({ handled: true }); + expect(console.log).toHaveBeenCalledWith(expect.stringContaining("prime-agent - AI coding assistant")); + await expect(handlePublicCommand(["--offline", "help", "status"])).resolves.toMatchObject({ handled: true }); + expect(console.log).toHaveBeenLastCalledWith(expect.stringContaining("prime-agent status [--json]")); + expect(mocks.daemonCommands).toEqual([]); + expect(process.exitCode).toBeUndefined(); + }); + + it("excludes global flags from the help command path instead of forwarding them", async () => { + await expect(handlePublicCommand(["--offline", "help", "mcp", "add"])).resolves.toMatchObject({ + handled: true, + }); + expect(console.log).toHaveBeenCalledWith(expect.stringContaining("prime-agent mcp add ")); + + await expect(handlePublicCommand(["help", "--verbose"])).resolves.toMatchObject({ handled: true }); + expect(console.log).toHaveBeenLastCalledWith(expect.stringContaining("prime-agent - AI coding assistant")); + }); + + it("excludes parseArgs-consumed flag values from the help command path", async () => { + await expect(handlePublicCommand(["help", "--resume", "status"])).resolves.toMatchObject({ + handled: true, + }); + expect(console.log).toHaveBeenLastCalledWith(expect.stringContaining("prime-agent - AI coding assistant")); + await expect(handlePublicCommand(["help", "-r", "status"])).resolves.toMatchObject({ handled: true }); + expect(console.log).toHaveBeenLastCalledWith(expect.stringContaining("prime-agent - AI coding assistant")); + await expect(handlePublicCommand(["help", "--print", "status"])).resolves.toMatchObject({ + handled: true, + }); + expect(console.log).toHaveBeenLastCalledWith(expect.stringContaining("prime-agent - AI coding assistant")); + + // The print flag must not swallow the help flag itself: an explicit + // --help still defers to the per-command help block. + await expect(handlePublicCommand(["help", "--print", "--help"])).resolves.toMatchObject({ + handled: true, + }); + expect(console.log).toHaveBeenLastCalledWith(expect.not.stringContaining("prime-agent status [--json]")); + }); + + it("keeps a -- after the help command on the message path", async () => { + await expect(handlePublicCommand(["--offline", "help", "--", "status"])).resolves.toEqual({ + handled: false, + args: ["help", "--offline", "--", "status"], + explicitAgentsView: false, + }); + }); + it("rejects invalid paths below a known help command", async () => { await handlePublicCommand(["help", "schedule", "nonsense"]); diff --git a/packages/coding-agent/test/suite/regressions/4603-worker-recovery.test.ts b/packages/coding-agent/test/suite/regressions/4603-worker-recovery.test.ts index 04a86323e..3182f7e2f 100644 --- a/packages/coding-agent/test/suite/regressions/4603-worker-recovery.test.ts +++ b/packages/coding-agent/test/suite/regressions/4603-worker-recovery.test.ts @@ -94,6 +94,12 @@ const fixturePath = resolve(__dirname, "../../fixtures/eng-4600-supervisor-fixtu const fauxExtensionPath = resolve(__dirname, "../../fixtures/eng-4600-faux-extension.ts"); const cliPath = resolve(__dirname, "../../../src/cli.ts"); const tsxPath = resolve(__dirname, "../../../../../node_modules/tsx/dist/cli.mjs"); +// The fixture must run in the spawned process itself: the tsx CLI wrapper forks, +// which would leave the spawned pid owning only tsx IPC pipes while the real +// supervisor (and its daemon socket) hides in an untracked child pid — invisible +// to the OS socket sweep this regression exercises. `--import` with tsx's ESM +// loader runs the TypeScript fixture in-process. +const tsxLoaderPath = resolve(__dirname, "../../../../../node_modules/tsx/dist/esm/index.mjs"); const tsconfigPath = resolve(__dirname, "../../../../../tsconfig.json"); const supervisorRegistryDirEnv = "PRIME_AGENT_INTERNAL_DAEMON_SUPERVISOR_REGISTRY_DIR"; const handles = new Set(); @@ -147,7 +153,7 @@ async function createPaths(): Promise { function spawnSupervisor(paths: TestPaths): ProcessHandle { return trackProcess( - spawn(paths.executablePath, [tsxPath, fixturePath], { + spawn(paths.executablePath, ["--import", tsxLoaderPath, fixturePath], { cwd: paths.agentDir, env: { ...process.env, diff --git a/packages/coding-agent/test/telemetry.test.ts b/packages/coding-agent/test/telemetry.test.ts index 49c2ba426..d451e5b9f 100644 --- a/packages/coding-agent/test/telemetry.test.ts +++ b/packages/coding-agent/test/telemetry.test.ts @@ -328,6 +328,39 @@ describe("agent telemetry aggregation", () => { expect(telemetryAuthCategory("models_json_command")).toBe("models_json"); }); + it("includes platform fidelity properties on every event", async () => { + vi.stubEnv("PRIME_AGENT_TELEMETRY", "1"); + const sink = new FakeTelemetrySink(); + + await captureAgentCommandUsed({ + agentDir: "/not-used", + settingsManager: SettingsManager.inMemory(), + commandName: "model", + sink, + }); + + const properties = sink.events[0]?.properties ?? {}; + expect(properties.libc).toMatch(/^(glibc|musl|none|unknown)$/); + expect(properties.cpu_baseline).toMatch(/^(avx2|no_avx2|avx2_assumed|not_applicable|unknown)$/); + expect(typeof properties.libc_version).toBe("string"); + expect(typeof properties.os_release).toBe("string"); + expect(typeof properties.os_product_version).toBe("string"); + // Non-identifying: no hostname, username, path, or hardware id fields. + expect(Object.keys(properties).sort()).toEqual([ + "architecture", + "command_name", + "cpu_baseline", + "execution_mode", + "install_method", + "libc", + "libc_version", + "os_family", + "os_product_version", + "os_release", + "version", + ]); + }); + it("emits aggregate metrics without message or tool content", () => { vi.stubEnv("PRIME_AGENT_TELEMETRY", "1"); let timestamp = 1_000; diff --git a/packages/coding-agent/test/version-check.test.ts b/packages/coding-agent/test/version-check.test.ts index 28d6b46fd..e41ddb6cd 100644 --- a/packages/coding-agent/test/version-check.test.ts +++ b/packages/coding-agent/test/version-check.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { checkForNewPiVersion, comparePackageVersions, @@ -9,25 +9,21 @@ import { isReleaseUpdateCandidate, resolveUpdateChannel, } from "../src/utils/version-check.js"; +import { clearAmbientRuntimeEnv } from "./ambient-env.js"; const defaultPrimeAgentDownloadBaseUrl = "https://pub-728493de92a943e2a9b2d17b4719f318.r2.dev"; -const originalSkipVersionCheck = process.env.PI_SKIP_VERSION_CHECK; -const originalOffline = process.env.PI_OFFLINE; -const originalPrimeAgentDownloadBaseUrl = process.env.PRIME_AGENT_DOWNLOAD_BASE_URL; - -function restoreEnv(name: string, value: string | undefined): void { - if (value === undefined) { - delete process.env[name]; - return; - } - process.env[name] = value; -} + +// These checks read the environment, so each test starts from a cleared one and the +// host shell cannot decide the outcome. Tests that need a variable set it themselves. +let restoreAmbientRuntimeEnv: () => void; + +beforeEach(() => { + restoreAmbientRuntimeEnv = clearAmbientRuntimeEnv(); +}); afterEach(() => { vi.unstubAllGlobals(); - restoreEnv("PI_SKIP_VERSION_CHECK", originalSkipVersionCheck); - restoreEnv("PI_OFFLINE", originalOffline); - restoreEnv("PRIME_AGENT_DOWNLOAD_BASE_URL", originalPrimeAgentDownloadBaseUrl); + restoreAmbientRuntimeEnv(); }); describe("version checks", () => { @@ -108,8 +104,6 @@ describe("update channel preference", () => { }); it("follows a preferred nightly channel from a stable installation", async () => { - delete process.env.PI_SKIP_VERSION_CHECK; - delete process.env.PI_OFFLINE; const fetchMock = vi.fn(async () => Response.json({ version: "v1.2.5-beta.130.1.abcdef0" })); vi.stubGlobal("fetch", fetchMock); await expect(getLatestPiVersion("1.2.4", { channel: "nightly" })).resolves.toBe("1.2.5-beta.130.1.abcdef0"); @@ -117,8 +111,6 @@ describe("update channel preference", () => { }); it("follows a preferred stable channel from a beta installation", async () => { - delete process.env.PI_SKIP_VERSION_CHECK; - delete process.env.PI_OFFLINE; const fetchMock = vi.fn(async () => Response.json({ version: "v1.2.4" })); vi.stubGlobal("fetch", fetchMock); await expect(getLatestPiVersion("1.2.4-beta.123.1.1234567", { channel: "stable" })).resolves.toBe("1.2.4"); @@ -153,8 +145,6 @@ describe("update channel preference", () => { }); it("reports the current beta build from a stable installation once nightly is preferred", async () => { - delete process.env.PI_SKIP_VERSION_CHECK; - delete process.env.PI_OFFLINE; vi.stubGlobal( "fetch", vi.fn(async () => Response.json({ version: "v1.2.3-beta.5.1.abcdef0" })), @@ -164,8 +154,6 @@ describe("update channel preference", () => { }); it("reports a newer beta build when nightly is preferred", async () => { - delete process.env.PI_SKIP_VERSION_CHECK; - delete process.env.PI_OFFLINE; vi.stubGlobal( "fetch", vi.fn(async () => Response.json({ version: "v1.2.5-beta.1.1.abcdef0" })),