fix: find CLIs installed by npm's global prefix, not just the PATH one

server/cos-runner/index.js

@@ -21,6 +21,7 @@ import { prepareCliSpawn, killProcessTree, guardChildStdin, deliverChildStdin }
 import { buildCliChildEnv } from '../lib/cliChildEnv.js';
 import { prepareCliPrompt } from '../lib/cliProviderArgs.js';
 import { commandExists } from '../lib/commandExists.js';
+import { adoptNpmGlobalBinDir } from '../lib/npmGlobalBin.js';
 import { findCommandOnPath } from '../lib/processEnv.js';
 import { createCodexStderrFormatter } from '../lib/codexCliOutput.js';
 import { createStreamJsonParser } from './streamJsonParser.js';
@@ -928,6 +929,13 @@ async function cleanupOrphanedAgents() {
 server.listen(PORT, HOST, async () => {
   console.log(`🤖 CoS Agent Runner started on http://${HOST}:${PORT}`);
 
+  // The runner is its own PM2 app with its own environment, and it spawns
+  // provider CLIs by bare name — so it must adopt npm's global bin directory
+  // itself. Without this a CLI the AI Providers page reports as installed
+  // (the main server adopted it there) still 422s here as "not on the CoS
+  // Runner PATH". Fire-and-forget: never blocks accepting work.
+  adoptNpmGlobalBinDir().catch((err) => console.error(`❌ npm global bin adoption failed: ${err.message}`));
+
   // Ensure agents directory exists. try/catch is mandatory: this listener runs
   // outside the request lifecycle, so a rejected await here escapes as an
   // unhandled rejection and takes the runner down at boot (fatal on Node >= 15).

server/lib/README.md

@@ -285,7 +285,9 @@ The barrel `server/lib/index.js` is a machine-checkable enumeration of every pub
 | `repoUrl.js` | `parseRepoUrl(url)` → `{ host, provider, owner, repo }` / `isRepoUrl(url)` / `repoCloneUrl(parsed)` / `repoBrowseUrl(parsed)` / `parseGitHubUrl(url)` / `isGitHubRepoUrl(url)` — authoritative "is this a clonable repo URL?" rule, with per-host behavior (subgroup nesting, clone layout) in the `REPO_HOSTS` table. Mirrored to `client/src/lib/repoUrl.js` (parity pinned by `repoUrl.mirror.test.js`) so the Brain capture boxes reveal the post-clone agent options for exactly the URLs the server will clone. |
 | `glabArgs.js` | `GLAB_JSON_ARGS`, `withGlabJson(args)` — pure `glab` argv conventions. The JSON output flag is `--output json`, NOT `-F json`: on `glab issue list` (and only there) `-F` is `--output-format` (details/ids/urls), so `-F json` is accepted, ignored, and answers with the human table at exit 0. Shared by all three `glab` runners so the spelling has one definition; guarded tree-wide by `services/gitlab.glabFlags.test.js`. |
 | `killWithEscalation.js` | `killWithEscalation(proc, {label, stillRunning, delayMs=8000})` — shared SIGTERM→grace→SIGKILL cancel-escalation for spawn-based media jobs. Sends SIGTERM, then escalates to SIGKILL after `delayMs` only when `stillRunning()` holds and the child hasn't exited (`exitCode===null && signalCode===null`). The timer is unref'd and the callback is try/catch-wrapped (runs outside the request lifecycle). Converges musicVideo/render, videoTimeline, imageGen local+codex, videoGen, loraTraining, and the yt-dlp track import cancel paths. |
-| `processEnv.js` | `stripDebugMallocEnv(env)` / `safeChildProcessEnv(extra)` — drop macOS `Malloc*` debug env vars before spawning a child. `safeChildProcessOptions(options)` adds that sanitized environment plus `windowsHide: true`, preventing detached PM2 subprocesses from opening transient Windows console UI; `buildSafeCliBaseEnv(env, provider)` allowlists runtime/delivery essentials and only the selected CLI's ambient provider auth before an AI CLI inherits the server environment. Route Node→Python and other background CLI spawns through the process helpers. Also `whichFirst(name)` — first-hit `which`/`where` PATH probe (safe options, 5s timeout) returning the absolute binary path or `null`; `whichFirstSync(name)` provides the same contract without the await; `findCommandOnPath(name, { env, cwd })` resolves an executable from the exact child environment without needing `which` itself on that PATH. |
+| `npmGlobalBin.js` | `adoptNpmGlobalBinDir()` — puts the directory `npm install --global` actually writes to onto `process.env.PATH`, from one cached `npm prefix -g`. npm resolves its prefix through a config cascade (cli flags, `npm_config_*`, project/user/global npmrc, a builtin npmrc that interpolates env vars), so guessing it from `%APPDATA%`/`$HOME` reproduces the bug on the next host — only npm can answer. Exists because a host whose npm prefix is NOT the directory its Node installer put on PATH (a machine-wide Windows prefix vs the per-user `%APPDATA%
+pm` default, `NPM_CONFIG_PREFIX`, nvm/Volta) installed `codex` successfully and then reported it unrunnable, advising a restart that inherits the same PATH. Called at boot in BOTH processes that spawn provider CLIs (`services/bootstrap.js`, `cos-runner/index.js`) and again from `providerRuntimeInstaller.js`'s probe, because on a first install the prefix directory did not exist when boot looked. |
+| `processEnv.js` | `stripDebugMallocEnv(env)` / `safeChildProcessEnv(extra)` — drop macOS `Malloc*` debug env vars before spawning a child. `safeChildProcessOptions(options)` adds that sanitized environment plus `windowsHide: true`, preventing detached PM2 subprocesses from opening transient Windows console UI; `buildSafeCliBaseEnv(env, provider)` allowlists runtime/delivery essentials and only the selected CLI's ambient provider auth before an AI CLI inherits the server environment. Route Node→Python and other background CLI spawns through the process helpers. Also `whichFirst(name)` — first-hit `which`/`where` PATH probe (safe options, 5s timeout) returning the absolute binary path or `null`; `whichFirstSync(name)` provides the same contract without the await; `findCommandOnPath(name, { env, cwd })` resolves an executable from the exact child environment without needing `which` itself on that PATH. `adoptPathDirs(dirs)` appends the ones that exist and are not already there to THIS process's `process.env.PATH` (case-insensitively on Windows), which fixes this process AND every child, since the env builders above derive from `process.env` — the only fix that reaches a CLI launched by BARE NAME through node-pty. Shared by `npmGlobalBin.js` and `services/llamaServerManager.js`'s winget-shim adoption. |
 | `branchUpstreamGuard.js` | `enforceSafeBranchUpstream(repo, branch)` / `readBranchUpstream` / `isSafeBranchUpstream` — the agent-branch upstream invariant (#4172): a branch handed to a CoS agent tracks either NOTHING or its OWN ref, never `refs/heads/main`. `git worktree add -b <b> <path> origin/main` does not leave the branch untracked — `branch.autoSetupMerge` records `merge=refs/heads/main` — and `/do:pr` derives its push destination from that config (`git push <remote> HEAD:<merge>`), so the agent's work lands straight on the default branch with no PR. Prevention is `--no-track` at the `worktreeManager.js` creation sites; this is the backstop that also REPAIRS branches created before the fix (drops the bogus upstream, logs it) and throws only if the repair doesn't take. Fails CLOSED: `readBranchUpstream` returns `null` for could-not-read (distinct from `''` = genuinely unset), and an unverifiable upstream is refused rather than waved through — `worktreeManager.js` undoes the `worktree add` on refusal so the throw can't strand a tree. Pairs with `primaryCheckoutGuard.js`, which catches the other way agent work reaches `main`. |
 | `primaryCheckoutGuard.js` | `capturePrimaryCheckoutState(path)` / `detectPrimaryCheckoutDrift(baseline, {agentBranch})` + `PRIMARY_CHECKOUT_MUTATED_REASON`/`_CATEGORY` — the branch-jack detector (#3680): stamps the PRIMARY checkout's branch + HEAD onto a worktree agent's metadata at spawn (`agentLifecycle.js`) and re-reads it in the shared finalize path (`agentFinalization.js`), so a worktree-isolated agent that commits to the primary is recorded as a FAILURE naming the drifted branch, the commit count, and the `git reset --hard` recovery — instead of a silent "completed". Detect-and-report only: the reset discards commits, so it stays a human decision. Non-throwing (runs outside the request lifecycle); an unreadable checkout reports no drift rather than inventing one. |
 | `pythonSetup.js` | Python venv / runner setup helpers. |

server/lib/index.js

@@ -269,6 +269,7 @@ export * from './glabArgs.js';
 export * from './goalFeatureMap.js';
 export * from './interactiveShellResolver.js';
 export * from './killWithEscalation.js';
+export * from './npmGlobalBin.js';
 export * from './openFolder.js';
 export * from './processEnv.js';
 export * from './primaryCheckoutGuard.js';

server/lib/npmGlobalBin.js

@@ -0,0 +1,80 @@
+/**
+ * Where `npm install --global` actually writes its executables.
+ *
+ * The AI Providers page installs a CLI with a fixed `npm install --global` and
+ * then asks whether PortOS can run it. Those two questions have different
+ * answers on any host whose npm prefix is not the directory the platform's Node
+ * installer put on PATH: a machine-wide Windows prefix while PATH still carries
+ * the per-user `%APPDATA%\npm` default, an `NPM_CONFIG_PREFIX` an admin set, an
+ * nvm/Volta switch, a `prefix=` in a user or global npmrc. The install writes a
+ * perfectly good `codex.cmd` and nothing on PATH names the directory holding
+ * it — so the card reported "the installer finished, but PortOS still cannot
+ * run `codex`" and advised a restart, which cannot fix a directory the machine
+ * PATH has never contained.
+ *
+ * npm resolves its prefix through a config cascade (cli flags, `npm_config_*`
+ * env, project/user/global npmrc, and a builtin npmrc that itself interpolates
+ * env vars), so re-deriving it from `%APPDATA%`/`$HOME` guesses would just
+ * reproduce the same class of bug on the next host. The only correct answer is
+ * npm's own, hence one cached `npm prefix -g`.
+ *
+ * `adoptPathDirs` does the rest; see its docstring in `processEnv.js` for why
+ * extending `process.env.PATH` is what a bare-name node-pty launch needs.
+ */
+
+import { existsSync } from 'fs';
+import { join } from 'path';
+import { bufferedSpawn, prepareCliSpawn } from './bufferedSpawn.js';
+import { adoptPathDirs, safeChildProcessEnv } from './processEnv.js';
+
+const IS_WIN = process.platform === 'win32';
+
+// `npm prefix -g` reads config files and can pay a cold Node start, so it gets
+// more room than the 5s PATH probes — but it must never hang a status refresh.
+const PREFIX_TIMEOUT_MS = 15_000;
+
+// npm's answer, held as a promise so concurrent callers share one spawn: a host
+// does not change its global prefix underneath a running server.
+let binDirProbe = null;
+
+/** One `npm prefix -g`, mapped to the platform's global bin directory. */
+async function probeNpmGlobalBinDir(spawnImpl) {
+  const env = safeChildProcessEnv();
+  // `shell: false` + `prepareCliSpawn`, not `bufferedSpawn`'s `needsShell('npm')`
+  // default: that default is `shell: true` with an args array, which node
+  // space-joins WITHOUT escaping (DEP0190). The argv here is fixed, but the
+  // warning would print at every boot in both processes.
+  const { command, args } = prepareCliSpawn('npm', ['prefix', '-g'], env);
+  const { stdout } = await spawnImpl(command, args, { env, shell: false, timeoutMs: PREFIX_TIMEOUT_MS });
+  const prefix = String(stdout || '').trim().split(/\r?\n/)[0]?.trim() || '';
+  // Windows drops binaries straight in the prefix; POSIX uses `<prefix>/bin`.
+  return prefix ? (IS_WIN ? prefix : join(prefix, 'bin')) : null;
+}
+
+/**
+ * Put npm's global bin directory on this process's PATH, so a CLI installed
+ * there is both discoverable and launchable by bare name. Idempotent, and a
+ * no-op when npm cannot be asked or the directory is not there yet.
+ *
+ * The existence check is deliberately NOT cached with npm's answer: the prefix
+ * directory does not exist until the first global install, and making that
+ * install visible without a restart is the whole point.
+ *
+ * @param {{spawnImpl?: Function}} [deps]
+ * @returns {Promise<string|null>} the directory now on PATH, or `null`
+ */
+export async function adoptNpmGlobalBinDir({ spawnImpl = bufferedSpawn } = {}) {
+  // A probe that cannot answer resolves null rather than rejecting: an
+  // unreachable npm means "PortOS keeps the PATH it has", not a failed caller.
+  binDirProbe = binDirProbe || probeNpmGlobalBinDir(spawnImpl).catch(() => null);
+  const dir = await binDirProbe;
+  if (!dir || !existsSync(dir)) return null;
+  const [adopted] = adoptPathDirs([dir]);
+  if (adopted) console.log(`🔗 Adopted npm global bin directory onto PortOS's PATH: ${adopted}`);
+  return dir;
+}
+
+/** Test-only: forget npm's answer so the next read re-probes. */
+export function __resetNpmGlobalBinCache() {
+  binDirProbe = null;
+}

server/lib/npmGlobalBin.test.js

@@ -0,0 +1,74 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { mkdtempSync, mkdirSync, rmSync } from 'fs';
+import { tmpdir } from 'os';
+import { delimiter, join } from 'path';
+import { adoptNpmGlobalBinDir, __resetNpmGlobalBinCache } from './npmGlobalBin.js';
+
+const IS_WIN = process.platform === 'win32';
+
+// npm answers with the PREFIX; the bin directory is the prefix itself on
+// Windows and `<prefix>/bin` everywhere else.
+const binDirFor = (prefix) => (IS_WIN ? prefix : join(prefix, 'bin'));
+const npmAnswering = (prefix) => vi.fn(async () => ({ stdout: `${prefix}\n` }));
+
+describe('adoptNpmGlobalBinDir', () => {
+  let root;
+  let originalPath;
+
+  beforeEach(() => {
+    __resetNpmGlobalBinCache();
+    root = mkdtempSync(join(tmpdir(), 'portos-npm-prefix-'));
+    originalPath = process.env.PATH;
+  });
+
+  afterEach(() => {
+    process.env.PATH = originalPath;
+    rmSync(root, { recursive: true, force: true });
+  });
+
+  it('puts npm\'s own global bin directory on PATH, once, for concurrent callers', async () => {
+    const binDir = binDirFor(root);
+    mkdirSync(binDir, { recursive: true });
+    const spawnImpl = npmAnswering(root);
+
+    const both = await Promise.all([
+      adoptNpmGlobalBinDir({ spawnImpl }),
+      adoptNpmGlobalBinDir({ spawnImpl }),
+    ]);
+
+    expect(both).toEqual([binDir, binDir]);
+    expect(process.env.PATH.split(delimiter).filter((entry) => entry === binDir)).toHaveLength(1);
+    // The Providers page probes every runtime at once, and each one asks.
+    expect(spawnImpl).toHaveBeenCalledTimes(1);
+    // Windows wraps npm's .cmd shim as `cmd.exe /c <path> …` — the fixed
+    // subcommand is the tail either way, and never a shell string.
+    const [, args, options] = spawnImpl.mock.calls[0];
+    expect(args.slice(-2)).toEqual(['prefix', '-g']);
+    expect(options.shell).toBe(false);
+  });
+
+  it('answers null when npm cannot be asked, without falling back to a guess', async () => {
+    const spawnImpl = vi.fn(async () => { throw new Error('ENOENT'); });
+
+    await expect(adoptNpmGlobalBinDir({ spawnImpl })).resolves.toBeNull();
+    expect(process.env.PATH).toBe(originalPath);
+  });
+
+  // The prefix directory does not exist until the first global install, and
+  // making that install visible without a restart is the whole point — so the
+  // on-disk check must NOT be cached alongside npm's answer.
+  it('adopts a prefix that only appears after the first global install', async () => {
+    const binDir = binDirFor(root);
+    rmSync(root, { recursive: true, force: true });
+    const spawnImpl = npmAnswering(root);
+
+    await expect(adoptNpmGlobalBinDir({ spawnImpl })).resolves.toBeNull();
+    expect(process.env.PATH).toBe(originalPath);
+
+    mkdirSync(binDir, { recursive: true });
+    await expect(adoptNpmGlobalBinDir({ spawnImpl })).resolves.toBe(binDir);
+    expect(process.env.PATH.split(delimiter)).toContain(binDir);
+    // Still one probe: only the existence check re-runs.
+    expect(spawnImpl).toHaveBeenCalledTimes(1);
+  });
+});

server/lib/processEnv.js

@@ -12,7 +12,7 @@
 // stripping the prefix is a no-op on Linux/Windows.
 import { execFile, execFileSync } from './childProcess.js';
 import { promisify } from 'util';
-import { accessSync, constants, statSync } from 'fs';
+import { accessSync, constants, existsSync, statSync } from 'fs';
 import { delimiter, isAbsolute, join, resolve } from 'path';
 
 const execFileAsync = promisify(execFile);
@@ -208,3 +208,46 @@ export function findCommandOnPath(name, { env = process.env, cwd = process.cwd()
   }
   return null;
 }
+
+// Windows paths are case-insensitive and PATH mixes casings freely
+// (`C:\ProgramData\npm` vs `c:\programdata\npm`), so a case-sensitive compare
+// would append a duplicate of an entry that is already there.
+const samePathEntry = (a, b) => (IS_WIN ? a.toLowerCase() === b.toLowerCase() : a === b);
+
+/**
+ * Adopt directories onto THIS process's PATH.
+ *
+ * A package manager that installs into a directory the running process's PATH
+ * does not name is the recurring case, and it always looks the same from here:
+ * the install succeeds and the binary reports as "not found". winget adds its
+ * portable-shim Links directory to the USER environment, which an
+ * already-running server never inherited; npm's global prefix need not be the
+ * directory the platform's Node installer put on PATH at all, so no restart
+ * fixes that one.
+ *
+ * Extending `process.env.PATH` fixes both this process's own lookups AND every
+ * child it spawns, since `safeChildProcessEnv` / `buildSafeCliBaseEnv` derive
+ * from it — which is what a CLI launched by BARE NAME through node-pty needs,
+ * because resolving its path is not something that launch can use.
+ *
+ * Only directories that exist are adopted: `findCommandOnPath` stats every PATH
+ * entry against every PATHEXT extension, so a dead entry taxes every later
+ * lookup for the life of the process.
+ *
+ * @param {string[]} dirs
+ * @returns {string[]} the directories newly added
+ */
+export function adoptPathDirs(dirs) {
+  // `process.env` is case-insensitive on Windows, so `.PATH` reads a `Path`
+  // entry too — no second spelling to check. The empty segments a trailing or
+  // doubled delimiter leaves behind are dropped rather than carried along.
+  const entries = String(process.env.PATH || '').split(delimiter).filter(Boolean);
+  const added = [];
+  for (const dir of dirs) {
+    if (!dir || entries.some((entry) => samePathEntry(entry, dir)) || !existsSync(dir)) continue;
+    entries.push(dir);
+    added.push(dir);
+  }
+  if (added.length > 0) process.env.PATH = entries.join(delimiter);
+  return added;
+}