diff --git a/openspec/changes/stage-mcp-runtime-during-setup/.openspec.yaml b/openspec/changes/stage-mcp-runtime-during-setup/.openspec.yaml new file mode 100644 index 0000000..e5433c5 --- /dev/null +++ b/openspec/changes/stage-mcp-runtime-during-setup/.openspec.yaml @@ -0,0 +1,2 @@ +schema: ns-workflow +created: 2026-08-18 diff --git a/openspec/changes/stage-mcp-runtime-during-setup/design.md b/openspec/changes/stage-mcp-runtime-during-setup/design.md new file mode 100644 index 0000000..b552828 --- /dev/null +++ b/openspec/changes/stage-mcp-runtime-during-setup/design.md @@ -0,0 +1,483 @@ +# Design + +## Goals / Non-Goals + +Goals: provision `mcp-remote@0.1.38` during `setup` into a shared, versioned, +concurrency-safe runtime whose publication is recoverable; make the wrapper +consume only that runtime (or a version-matched dev checkout copy); make +failures fast and actionable; surface the state in `doctor`; preserve +credentials semantics and the current Accounts OAuth untouched. + +Non-goals: native MCP-transport OAuth (tracked as deferred technical debt); +automatic pruning of old runtime +versions; byte-for-byte reproducible installs (see Limitations); changing the +Codex 60s startup timeout; offline first-run installation; a portable +gap-free atomic directory swap (not exposed by Node; the publication +protocol below constrains its guarantee accordingly). + +## Module Boundaries + +### New module: `packages/core/src/mcp/mcp-remote-runtime.ts` + +Single ac responsibility: manage the shared bridge runtime under +`getAgentsDir()` (never paths derived from `cwd`). Public (internal-package) +contract: + +```ts +export const MCP_REMOTE_VERSION = '0.1.38' + +export interface McpRemoteRuntimeStatus { + status: 'ready' | 'missing' | 'invalid' + version: string // pinned version this plugin expects + root: string // ~/.agents/nsolid-plugin/runtime/mcp-remote/ + proxyPath?: string // set when ready + reason?: string // why invalid +} + +export interface EnsureMcpRemoteRuntimeResult { + installed: boolean // false when an already-valid runtime was reused + version: string + root: string + proxyPath: string +} + +export interface NpmRunnerRunResult { + status: number | null + stderr: string + timedOut?: boolean + /** Set when the npm process could not be spawned at all (ENOENT/EACCES/EPERM). */ + spawnError?: string + /** Set when timeout cancellation could not confirm that the managed tree stopped. */ + terminationError?: string +} + +export interface NpmRunner { + /** + * Runs npm without a shell. On timeout, terminates the managed npm process + * tree and confirms that it stopped before cleanup is allowed. If that + * confirmation fails, returns `terminationError` and the caller leaves + * staging marked retained-live and excluded from publication/cleanup. Spawn + * failures surface as `spawnError`, never as a fake exit status. + */ + run(command: string, args: string[], options: { cwd: string; timeoutMs: number }): + Promise +} + +export interface InternalRuntimeOptions { + /** Test-only runner injection; default spawns npm without a shell. */ + runner?: NpmRunner + /** Override the resolved npm entry point (tests point it at node itself). */ + npmCommand?: { command: string; args: string[] } + /** Setup-time npm timeout (default 5 min). NOT the Codex MCP startup timeout. */ + timeoutMs?: number +} + +export function inspectMcpRemoteRuntime(): McpRemoteRuntimeStatus +export function resolveNpmCommand(): { command: string; args: string[] } +export async function ensureMcpRemoteRuntime(options?: InternalRuntimeOptions): Promise +``` + +`InternalRuntimeOptions` exists solely as the internal testing seam; it is not +re-exported from the package root. `inspectMcpRemoteRuntime()` performs no +mutation, no network and no process spawning. + +### Validation ("ready") + +A runtime at `root` is ready iff: + +Before reading package metadata, the probe canonicalizes the controlled runtime +parent and `root`. The canonical root must remain below the canonical parent by +a path-segment-aware boundary check; replacing the whole version root with a +symlink outside that parent makes the runtime invalid. The `mcp-remote` package +directory, every transitively resolved package directory, each package manifest +and `dist/proxy.js` are resolved with `realpath`; each canonical target must +equal the canonical runtime root or remain below it by the same boundary rule. +Package targets must be directories and manifest/proxy targets regular files. A +missing canonical target, a broken symlink or a symlink whose target escapes its +required boundary makes the runtime invalid. + +1. `root/node_modules/mcp-remote/package.json` parses with + `name === 'mcp-remote'` and `version === MCP_REMOTE_VERSION` (exact); +2. `root/node_modules/mcp-remote/dist/proxy.js` exists and is a file; +3. the dependency closure is complete and internally consistent: for every + (non-optional) `dependencies` entry of `mcp-remote` and, transitively, of + its resolved dependencies: + - the entry resolves via Node-style `node_modules` resolution starting at + the dependent's directory, walking up no further than the runtime root — + resolution is **confined to the runtime root** and can never match a + `package.json` living outside it (e.g. a user's global or home-directory + `node_modules`); + - the resolved `package.json` parses and its `name` **exactly equals** the + requested dependency name (a wrong-named package squatting the slot + fails); + - its `version` **satisfies the declared range** of the dependent's + dependency entry. Range evaluation uses the `semver` package (added as a + dependency of `packages/core`); the supported syntax is the semver range + grammar npm accepts in `dependencies` (exact versions, comparators, + hyphen ranges, x-ranges, `~`/`^`, `||`, `*`). A range that cannot be + parsed fails closed (invalid), as does an unsatisfied one. + +Missing `optionalDependencies` are tolerated; `peerDependencies`/ +`devDependencies` are ignored. + +The closure walk is a static probe: it detects missing, misnamed and +version-incompatible transitives without executing package code during setup +(important because installs run with `--ignore-scripts`, so package code has +not been vetted/executed yet). + +### Install sequence (`ensureMcpRemoteRuntime`) + +1. `inspectMcpRemoteRuntime()` → ready ⇒ return `{ installed: false, ... }` + without invoking npm (idempotence; no network, no lock taken). +2. Create the controlled parent `~/.agents/nsolid-plugin/runtime/mcp-remote/`. +3. Create staging `parent/.staging--/` as a sibling of the versioned + root under the controlled runtime parent. This same-filesystem placement is + required for atomic publication, not an optimization. Write a minimal private `package.json` + (`{ name: "nsolid-plugin-mcp-remote-runtime", private: true }`) so npm does + not walk up into unrelated manifests/workspaces. Before spawning npm, write + an adjacent ownership sidecar for the staging tree with the operation token, + creator pid, creation time and `active` state; record the managed process + group/tree identity immediately after spawn. +4. Resolve npm (see below) and run, without a shell and with separated argv: + `install --omit=dev --ignore-scripts --no-audit --no-fund --save-exact + --no-package-lock mcp-remote@0.1.38`. On timeout the runner terminates the + managed npm process tree and confirms that it stopped before permitting + staging cleanup; spawn and termination-confirmation failures surface as + explicit `spawnError` / `terminationError` results. +5. Timeout budget: 5 minutes default (setup-time, independent of the Codex + MCP startup timeout). Capture only a bounded tail of stderr (≈4 KiB); + never dump the environment. +6. Validate staging with the full readiness probe (steps 1–3 above, including + identity and range checks). +7. Publish under the per-version publication lock (protocol below). +8. Re-inspect the published runtime; on any failure throw a single actionable + error: setup must end `success: false` and tell the user to re-run the same + command (credentials may already be stored and must remain valid). + +Only paths created by the current operation are ever deleted: the staging +directory, the stale-aside directory and the lock file this operation owns. +Every recursive deletion target is asserted to live inside the validated +runtime parent. + +### Publication protocol (staging → root) + +All writes live under `parent = ~/.agents/nsolid-plugin/runtime/mcp-remote`, +with `root = parent/`, `staging = parent/.staging--`, +`stale = parent/.stale-` and `lock = +parent/.publish-.lock`. Each staging/stale tree has an adjacent +ownership sidecar tied to the same unique operation token. Protocol: + +1. **Serialize per version.** Before touching `root`, acquire the publication + lock by exclusive creation (`O_CREAT | O_EXCL`) under the runtime parent. + The file records the operation token, pid and creation time. Waiters retry + with bounded backoff while the recorded holder may still be alive. Only a + successful exclusive create establishes ownership. +2. **Stale-lock handling.** A lock older than 10 minutes (greater than the + 5-minute npm budget) may be broken only when its recorded holder is proven + dead. Contenders race to `rename(lock → lock.steal-)`; the contender + whose rename succeeds deletes that tombstone and returns to step 1 to + acquire a fresh lock with `O_EXCL`. It does **not** own the lock merely by + moving the stale file. A live holder is never evicted based on age alone; + when liveness cannot be established, waiters fail after their bounded wait + instead of creating a second publisher. Breaking a stale lock never deletes + another process's staging/stale trees. +3. **Re-inspect under the lock.** With the lock held, re-inspect `root`: if it + is now ready (a racing setup published a valid runtime), accept the + winner, delete only the loser's own staging, release the lock, return. +4. **Validate staging before touching root.** The full readiness probe must + pass on the staging tree before any rename of `root`. An invalid staging + takes the failure path (delete own staging, release, `success: false`) and + leaves `root` exactly as it was. +5. **Publish.** + - `root` absent: `rename(staging → root)`. Renaming onto a non-existent + destination is atomic on POSIX and Windows: `root` never becomes visible + partially. + - `root` present (necessarily invalid — step 3 returned not-ready): + first write the ownership sidecar for the future stale path, then run the + replacement sequence `rename(root → stale-)`, then + `rename(staging → root)`, then remove the stale tree. If the rename-in + fails with `EEXIST`/`EPERM`/`ENOTEMPTY` (platform refused a + directory-over-directory rename because a destination appeared), loop + back to step 3 — the lock is still held. + - Publication never falls back to copying across filesystems. `EXDEV` or + any equivalent cross-filesystem rename failure fails closed, leaves an + existing runtime untouched and returns the actionable setup error. +6. **Recovery is the normal flow.** An operation that acquires the lock while + `root` is absent (a predecessor died between the two replacement renames) + needs no special case: it publishes its own validated staging through the + root-absent branch of step 5. Every interruption state — `root` absent, + with or without ignored `.staging-*`/`.stale-*` siblings — converges + deterministically on the next locked operation, and until then the wrapper + fails fast with the repair message exactly as it does for any missing + runtime. +7. **Cleanup ownership.** An operation deletes only what it created: its own + staging, its own stale-aside tree, its own lock (released in a `finally`; + unlinked only when its unique owner token still matches). A later operation + may delete an orphan only through the safe-reclamation protocol below. + Readiness probes never consult temporary siblings and nothing ever promotes + them. +8. **Atomicity boundary (exact claim).** + - A *fresh publish* is gap-free: `root` appears only through one atomic + rename of a fully validated tree. + - An *invalid-root replacement* is **not** a single atomic swap: between + the two renames `root` is briefly absent. The guarantee is: at every + instant `root` is either the old (invalid) tree, absent, or a fully + validated tree; a **valid** runtime is never removed (replacement is + only entered when `root` failed validation); and every interruption + state recovers deterministically (step 6). We claim preservation of + valid runtimes and deterministic recovery — not a portable gap-free + directory swap, which Node does not expose. + +### Safe orphan reclamation + +Temporary-tree cleanup is separate from the no-auto-pruning policy for +published, versioned runtimes: + +- If managed-tree termination cannot be confirmed, setup atomically updates the + staging sidecar to `retained-live` before returning `terminationError`. The + staging tree remains potentially mutable and is excluded from publication and + cleanup. If the marker cannot be written, the tree is retained as unclassified + and is never automatically deleted. +- A later setup may scan ownership sidecars only while holding the per-version + publication lock. It may reclaim a staging/stale tree and its sidecar only + after a grace period longer than the npm and termination budgets, when the + metadata parses, the path is inside the canonical runtime parent, the creator + pid is proven dead, no live publication lock carries its operation token and, + for staging with a recorded managed process identity, the platform-specific + check confirms that process group/tree no longer exists. Unknown liveness, + permission errors, missing/malformed metadata or token mismatch means retain. +- A stale-aside tree is reclaimed only after a valid versioned root exists. + Reclamation never restores or promotes an orphan. Tests use a short injected + grace period; production uses a fixed conservative grace period documented + beside the implementation constant. + +### npm resolution (`resolveNpmCommand`) + +npm is resolved exclusively from canonical candidates anchored to the running +Node.js installation. First resolve `process.execPath` with `realpath`; on +Unix its installation prefix is the parent of the canonical `bin` directory, +and on Windows it is the canonical Node executable directory. Every candidate +is checked with `lstat` and `realpath`: the resolved target must be a regular +file inside that canonical installation prefix. A symlink is accepted only +when its canonical target remains inside the prefix. `PATH`, `cwd`, project manifests and +`process.env.npm_execpath` are never consulted: a basename or +`node_modules/npm` substring check cannot prove that an arbitrary absolute +path is npm's own CLI, and `npm_execpath` is attacker-influenceable +environment input (a hostile checkout can export it, ship a fake +`node_modules/npm/bin/npm-cli.js`, or symlink it elsewhere). A legitimate +`npm_execpath` can only ever name one of the anchored candidates anyway, so +trusting it adds no resolving power — it is ignored entirely (fail-closed). +Order: + +1. `/node_modules/npm/bin/npm-cli.js` when it + passes the canonical boundary checks ⇒ + `[process.execPath, ]` (Windows Node.js installer layout; `.cmd` + shims cannot be spawned without a shell). +2. `/../lib/node_modules/npm/bin/npm-cli.js` + when it passes the same checks ⇒ `[process.execPath, ]` (Unix prefix + layouts: nvm, Volta images, Homebrew, macOS installer). +3. `/npm` when it is executable and its + canonical target remains inside the installation prefix (Unix distro + sibling shim, e.g. Debian/Ubuntu) ⇒ `[]`, spawned directly without a + shell. +4. Otherwise an actionable error telling the user to install Node.js with + npm and retry. + +## Setup / Startup sequence + +```mermaid +sequenceDiagram + participant U as User + participant S as nsolid-plugin setup + participant A as NodeSource Accounts + participant N as npm registry/cache + participant R as Stable runtime ~/.agents/... + participant H as Harness + participant W as mcp-wrapper.js + participant M as NodeSource remote MCP + + U->>S: setup --harness codex + S->>A: authenticate only if credentials are missing/expired + S->>R: validate mcp-remote 0.1.38 + alt runtime missing or invalid + S->>N: exact npm install into staging + S->>R: validate and publish under the per-version lock + end + S-->>U: setup complete + U->>H: first startup + H->>W: start ncm / benchmark / console + W->>R: import local dist/proxy.js + W->>M: Streamable HTTP with dynamic headers +``` + +### Onboarding dispatcher precondition + +Two dispatchers onboard harnesses: `packages/core/scripts/setup.mjs` (native +plugin bootstrap; its `install` action selects `setup()` for +claude/codex/antigravity and `install()` for opencode/pi) and the CLI +(`setup`/`install` commands in `packages/core/src/cli.ts`). Every onboarding +path must satisfy the runtime precondition — `ensureMcpRemoteRuntime()` to +readiness — **before** any harness-specific asset installation: + +- Core `setup()` runs `ensureMcpRemoteRuntime()` itself, once, after valid + credentials are available (or immediately when the bundle has no auth + section) and before any per-harness install branch. +- `packages/core/scripts/setup.mjs` and the CLI `install` command — the paths + that delegate to `install()` for OpenCode/Pi and fallback installs — run + `ensureMcpRemoteRuntime()` at dispatcher level immediately before + delegating. The runtime step needs no credentials (the npm download is + anonymous), so OpenCode and Pi cannot bypass provisioning. +- `install()` itself never authenticates, provisions or downloads: the rule + "install does not authenticate and does no unexpected network bootstrap" is + unchanged. + +A failed precondition aborts onboarding with the actionable +`MCP runtime setup failed: …` error. + +### Wrapper contract (generated by `scripts/plugin-generators.mjs`) + +- Signature: `node mcp-wrapper.js `. Claude's + `.claude-mcp.json` passes `claude`; the Codex and Antigravity bootstraps pass + `codex` / `antigravity` when launching the shared wrapper. Both `serverName` + and `harness` are validated against allow-lists before any repair message is + built. +- Resolution order: (1) stable runtime + `~/.agents/nsolid-plugin/runtime/mcp-remote//node_modules/mcp-remote`, + validated immediately before import by canonicalizing the controlled runtime + parent and version root, requiring the canonical root to remain below that + parent, then canonicalizing the package directory, `package.json` and + `dist/proxy.js`, requiring the expected directory/file types and enforcing + segment-aware containment under the canonical managed runtime root; (2) only + when the explicit internal development mode + `NSOLID_MCP_RUNTIME_DEV_FALLBACK=1` is set, development fallback + `createRequire(import.meta.url).resolve('mcp-remote/...')`, accepted only when + it validates against the same pinned version and its canonical manifest and + proxy remain inside the canonical fallback package directory; (3) otherwise fail immediately + (non-zero). Released harness configs never set the development flag. With the + flag absent, a local/project `node_modules` cannot mask a missing or invalid + managed runtime. +- **The wrapper never executes or spawns `npx`, npm, `cmd.exe`, or a shell.** + The repair message may contain an `npx` command as text. The Unix `npx` + fallback, the Windows `npx.cmd`/`cmd.exe` bootstrap/payload machinery and + its resolution helpers are deleted. +- Import-time failure translation: any error thrown or rejection surfaced + while importing or initializing the entry-validated `dist/proxy.js` + (including missing or incompatible transitives) becomes the same + harness-specific repair message instead of a raw module error. Direct + process termination by imported code, including `process.exit(1)` in the + pinned package, cannot be translated by an in-process wrapper; monitoring a + child process is outside this change. Dependency-name and version-range + validation remains the setup-time readiness probe's responsibility; the + wrapper stays dependency-free and does not evaluate semver ranges. +- Version-pinned repair command: the generator embeds `MCP_REMOTE_VERSION` + **and** `PLUGIN_VERSION` (the plugin release that generated the wrapper); + the message reads + `MCP bridge runtime is not ready. Run: npx -y nsolid-plugin@ setup --harness `. + The CLI at exactly that release pins exactly the runtime version the + wrapper validates, so the printed command always recreates a usable + runtime even when a newer CLI exists. `MCP_REMOTE_VERSION` is exported by + the generator and kept in sync with the core module and the root + `package.json` `dependencies['mcp-remote']` entry by one assertion. A separate + assertion checks that `PLUGIN_VERSION` equals the generating plugin release + and the version embedded in the wrapper; neither version domain is compared + to the other. +- URL and headers are handed to the imported proxy as separate + `process.argv` elements. + +### Doctor + +`DoctorReport` gains an optional, backward-compatible `bridge` entry: + +```ts +bridge?: { + status: 'ready' | 'missing' | 'invalid' + version: string + root: string + proxyPath?: string + reason?: string + /** true when this harness's MCP servers are actually served through the wrapper */ + required: boolean +} +``` + +`required` is true only for wrapper-owned configurations: claude, codex and +antigravity **with their native plugin detected** (the plugin's MCP config is +the wrapper). A missing/invalid runtime with `required: true` pushes an error +(`Run: nsolid-plugin setup --harness `), marks the bridge line red +and makes `healthy: false`. For OpenCode/Pi — and for direct/fallback +installs of the plugin-owned harnesses, whose MCP config is native HTTP — the +bridge line is informational and never affects health. "Bridge runtime ready" +is reported separately from the existing MCP-config/endpoint checks: a ready +proxy never implies the remote MCP is reachable. + +Human output gains a `MCP bridge` line; `--json` simply carries the new +optional field (documented in `packages/core/README.md`). + +## Concurrency & Failure Model + +- Two setups racing on a missing runtime: both stage independently; the + first to hold the lock publishes; the loser re-inspects under the lock, + accepts the valid winner and deletes only its own staging. +- Two setups racing to replace an **invalid** runtime: the lock serializes + the replacement; the second operation re-inspects under the lock, finds the + winner's valid runtime and returns — the on-disk state never gets worse. +- A predecessor killed mid-replacement (between `root → stale` and + `staging → root`): `root` is absent, the stale sibling is inert, the + wrapper fails fast with the repair message, and the next locked operation + closes the gap deterministically by publishing its own validated staging + (Publication protocol, step 6). +- npm failure/timeout/spawn error: staging is removed only after the runner + confirms termination of the managed npm tree. Unix uses a detached process + group, SIGTERM → SIGKILL escalation, root-process close, and polling the + process group until it no longer exists; Windows waits for `taskkill /T /F` + and the root process. If termination cannot be confirmed within a bounded + deadline, setup marks staging `retained-live`, excludes it from publication + and cleanup, and returns `terminationError` rather than deleting a directory + a survivor might still mutate. Later runs recognize the adjacent ownership + sidecar and apply the safe-reclamation protocol. Arbitrary + descendants that deliberately escape the managed process group are outside + this portable guarantee; `--ignore-scripts` prevents package lifecycle code + from creating them. Any previously valid runtime is untouched (the + destination is never deleted before staging validates); + `setup` returns `success: false` with `MCP runtime setup failed: …` while + stored credentials remain valid for retry. +- Interrupted install: `root` only ever appears via a single atomic rename of + a fully validated staging tree, so partial runtimes are never published; + leftover `.staging-*` and `.stale-*` siblings are ignored by probes + and reclaimed only when their ownership/liveness proof is safe. + +## Lifecycle / Migration + +- `uninstall --harness X`: removes X's artifacts only; the shared runtime is + untouched (other harnesses may still need it, including older plugin + versions pinned to a different runtime version). +- `logout`: credentials only, as today. +- New plugin version pinning a different `mcp-remote`: installs a sibling + versioned directory; old ones are kept (no auto-pruning; a cleanup policy + can be designed later). +- Existing users: after updating/reinstalling the plugin they run + `npx -y nsolid-plugin@ setup --harness ` (the + version-pinned form the updated wrapper prints) once before opening a new + session. If they skip it, the wrapper fails fast with the repair command + instead of hiding the problem behind an npm download. + +## Security Notes / Limitations + +- The runtime directory contains no tokens, authenticated URLs or headers; + credentials stay exclusively in `~/.agents/.nodesource-auth.json`. +- npm runs with `shell: false`, separated argv, `--ignore-scripts`, and only + inside the staging directory; stderr capture is bounded and never includes + the environment. +- npm is resolved only from canonical candidates anchored to the real Node.js + installation; targets that escape its canonical prefix are rejected. + `PATH`, project `node_modules/.bin` and `npm_execpath` (including fake, + renamed or symlinked `npm-cli.js` values and pnpm/yarn lifecycle values) + are never consulted, so a hostile project cannot substitute the installer. +- Exact-pinning `mcp-remote@0.1.38` does not freeze its transitive + dependencies declared with ranges: two cold installs may differ in + transitives (within the declared ranges). A dedicated lockfile/shrinkwrap + or a bundled artifact would be needed for byte-exact reproducibility; + documented as a known limitation, not solved here. +- First run still needs network (npm registry) — the fix makes the download + explicit and moves it out of harness startup; it does not make setup fully + offline. diff --git a/openspec/changes/stage-mcp-runtime-during-setup/proposal.md b/openspec/changes/stage-mcp-runtime-during-setup/proposal.md new file mode 100644 index 0000000..2e67786 --- /dev/null +++ b/openspec/changes/stage-mcp-runtime-during-setup/proposal.md @@ -0,0 +1,155 @@ +# Proposal + +## Problem Statement + +The shared MCP wrapper (`scripts/mcp-wrapper.js`, generated by +`scripts/plugin-generators.mjs`) bridges STDIO→HTTP for the NodeSource MCP +servers (`ncm`, `ns-benchmark`, `nsolid-console`) using `mcp-remote`. Native +plugin installs (Claude, Codex, Antigravity) stage the wrapper **without** +`node_modules`, so the wrapper cannot resolve `mcp-remote/dist/proxy.js` +locally and falls back to: + +```sh +npx -y mcp-remote@0.1.38 ... +``` + +On a cold install/startup, `npx` may consult or download from the npm +registry. Codex starts three MCP servers in parallel and can exceed even the +extended 60s startup timeout, so users see `ncm`, `ns-benchmark` and +`nsolid-console` fail to initialize on the first session. Subsequent sessions +usually work only because the npm/npx cache is warm — the artifact is never +self-sufficient. Claude uses the same wrapper and has the same latent defect. + +## Proposed Solution + +Move the `mcp-remote` download from **startup** to **setup**: + +- `nsolid-plugin setup --harness ` provisions a shared, versioned, + local runtime at `~/.agents/nsolid-plugin/runtime/mcp-remote/0.1.38/` + (exact-pinned `mcp-remote` with its transitive dependencies) for **all + five** harnesses, keeping the experience uniform. The onboarding + dispatchers (the native-plugin bootstrap and the CLI) satisfy this runtime + precondition before any harness-specific asset installation; `install()` + itself stays free of authentication and dependency bootstrap. +- The wrapper resolves that stable copy (validating name, exact version and + `dist/proxy.js`) before importing it, and translates import-time + dependency-resolution failures into the standard repair message. A + `createRequire(import.meta.url)` resolution remains only as a + development-checkout convenience. +- The automatic `npx` fallback (Unix spawn, Windows `cmd.exe`/`npx.cmd` + bootstrap/payload) is **removed**. The wrapper never executes or spawns + `npx`, npm, `cmd.exe`, or a shell. If the runtime is missing or corrupt, + the wrapper fails within seconds with an actionable, version-pinned repair + command (`npx -y nsolid-plugin@ setup --harness + `) that recreates exactly the runtime version the wrapper + validates. +- Installation is idempotent and never runs npm when a valid runtime already + exists. Publication renames a fully validated staging tree into place + under a per-version lock: a valid runtime is never destroyed, racing + setups converge on one valid runtime, and an interrupted replacement + (root briefly absent between two renames) recovers deterministically on + the next setup. +- npm is resolved only from candidates anchored to the running Node.js + executable — never from `PATH`, the project, or `npm_execpath` — so + neither a hostile project nor another package manager can substitute the + installer. +- `doctor` gains an MCP bridge check: for harnesses whose MCP servers are + served through the wrapper, a missing/invalid runtime makes the report + unhealthy with the repair hint; for OpenCode/Pi it is informational only + (their native HTTP transport does not depend on the bridge). +- `uninstall` and `logout` never delete the shared runtime (it is not + per-harness and contains no secrets); old versions are never pruned + automatically. + +Scope note (kept separate on purpose): the existing NodeSource Accounts OAuth +flow is unchanged — it still produces a service token the wrapper sends as +`X-Nsolid-*` headers. Native MCP-transport OAuth (standard OAuth managed by +each harness over Streamable HTTP) is **out of scope** and recorded as +deferred technical debt. + +## Affected Components and Files + +- `packages/core/src/mcp/mcp-remote-runtime.ts` (new) — runtime + inspect/ensure, publication protocol, trusted npm resolution. +- `packages/core/src/mcp/index.ts` — internal re-exports. +- `packages/core/src/index.ts` — `setup()` provisions the runtime; + `doctor()` reports bridge status. +- `packages/core/scripts/setup.mjs` — satisfies the runtime precondition + before delegating to `install()` for OpenCode/Pi. +- `packages/core/src/cli.ts` — satisfies the runtime precondition before the + fallback `install` command; updated messages. +- `packages/core/package.json` — adds the `semver` dependency used by the + readiness probe. +- `packages/core/src/types.ts` — `DoctorReport.bridge`. +- `packages/core/src/utils/format.ts` — human doctor output. +- `scripts/plugin-generators.mjs` — wrapper/generator rewrite (stable + runtime, harness argument, no `npx`/npm/shell execution, import-error + translation), exported `MCP_REMOTE_VERSION` and embedded `PLUGIN_VERSION`. +- Regenerated root artifacts: `.mcp.json`, `.claude-mcp.json`, + `mcp_config.json`, `scripts/mcp-wrapper.js` (via `pnpm plugin:root`). +- Tests: `packages/core/test/unit/mcp/mcp-remote-runtime.test.ts` (new), + `mcp-wrapper.test.ts` (rewritten expectations), + `packages/core/test/unit/utils/format.test.ts`, + `packages/core/test/integration/installer.test.ts`. +- Docs: `README.md`, `packages/core/README.md`. + +## Success Criteria / Acceptance Tests + +1. `setup` for any of the five harnesses leaves the shared exact runtime + ready (spec: first setup installs it; ready runtime ⇒ npm is not + invoked), through either onboarding dispatcher. +2. Multi-harness setup reuses a single installation (one runtime root). +3. No production wrapper executes or spawns `npx`, npm, `cmd.exe`, or a + shell during startup; a fake `npx` sentinel that exits 97 is never + executed (unit test). The repair message may contain an `npx` command as + text. +4. Missing/corrupt runtime fails fast with the harness-correct, + version-pinned repair command; executing that command provisions exactly + the runtime version the wrapper validates. +5. npm failure, timeout, or spawn error during setup yields + `success: false` and preserves stored credentials. Staging is cleaned only + after termination of the managed npm tree is confirmed; otherwise it is + marked retained-live and excluded from publication/cleanup until conservative + reclamation proves the creator and managed tree are gone. +6. Interrupted installation never publishes a partial runtime; a valid + runtime is never removed by setup; an interrupted replacement leaves a + deterministic recoverable state that the next setup converges on. +7. Concurrent setups converge on one valid runtime, including when replacing + an invalid one. The publication lock serializes replacement; only a stale + lock whose holder is proven dead is broken, and ownership still requires a + fresh exclusive acquisition. +8. URLs/tokens with spaces, quotes, `&`, `%PATH%` never cross a shell + boundary. +9. Runtime readiness verifies each dependency's package name and that its + installed version satisfies the dependent's declared range, with + canonical package, manifest and proxy targets confined to the runtime root + (including symlink boundaries); npm is resolved only from + canonical Node.js-anchored candidates (`npm_execpath`, `PATH` and project `.bin` + are never consulted). +10. `uninstall`/`logout` preserve the shared runtime. +11. `doctor` never reports healthy a wrapper-owned harness with a missing + runtime; OpenCode/Pi runtime status stays informational. +12. The generator/core runtime constant and root `package.json` + `dependencies['mcp-remote']` stay pinned to the same runtime version; a + separate assertion keeps the wrapper's embedded plugin version aligned + with the generating release. +13. Lint, `pnpm test`, `pnpm plugin:check`, `pnpm test:marketplace` and + `openspec validate stage-mcp-runtime-during-setup --strict` pass. + +## Rollback Plan + +Revert the integration commit (runtime module + setup/doctor wiring + wrapper +generator) and temporarily restore the wrapper's `npx` fallback, then +regenerate root artifacts with `pnpm plugin:root`. The Codex 60s +`startup_timeout_sec` stays. Runtimes already created under `~/.agents` are +left in place during rollback: they are inert cache, may be required by an +older plugin version in another harness, and removing them could break that +harness. No credentials or config need restoration — the change stores +nothing new outside the runtime directory. + +## Review Note (workflow compliance) + +`openspec/config.yaml` asks for Plannotator `submit_plan` review. The +`submit_plan` tool is not available to this agent's toolset; limitation +registered here. Proceeding under the explicit user approval already granted +for this scope. diff --git a/openspec/changes/stage-mcp-runtime-during-setup/specs/installation-and-auth/spec.md b/openspec/changes/stage-mcp-runtime-during-setup/specs/installation-and-auth/spec.md new file mode 100644 index 0000000..74f6b73 --- /dev/null +++ b/openspec/changes/stage-mcp-runtime-during-setup/specs/installation-and-auth/spec.md @@ -0,0 +1,384 @@ +# MCP bridge runtime provisioning (delta) + +## ADDED Requirements + +### Requirement: Setup provisions the shared MCP bridge runtime + +`nsolid-plugin setup` SHALL prepare a shared, versioned, local `mcp-remote` +runtime (exact pinned version) under `~/.agents/nsolid-plugin/runtime/` for +every supported harness, so MCP startup never needs npm. + +#### Scenario: First setup installs the runtime + +- **GIVEN** no runtime exists at `~/.agents/nsolid-plugin/runtime/mcp-remote//` +- **WHEN** the user runs `nsolid-plugin setup --harness ` +- **THEN** setup installs `mcp-remote` with its dependencies into that versioned root via npm (no shell, exact version) +- **AND** the installed tree contains `node_modules/mcp-remote/package.json` with the exact pinned version and `node_modules/mcp-remote/dist/proxy.js` +- **AND** setup completes successfully + +#### Scenario: Setup with a valid runtime is idempotent and offline-safe + +- **GIVEN** a valid runtime already exists at the versioned root +- **WHEN** the user reruns `nsolid-plugin setup --harness ` +- **THEN** the runtime is reused without invoking npm +- **AND** setup still completes successfully + +#### Scenario: Multiple harnesses reuse one installation + +- **GIVEN** a valid runtime was provisioned by setup for one harness +- **WHEN** the user runs setup for any other harness (or selects multiple harnesses in one run) +- **THEN** all harnesses converge on the same shared runtime path +- **AND** no additional npm installation is performed + +#### Scenario: Setup prepares the runtime for all five harnesses + +- **GIVEN** any supported harness (claude, codex, opencode, antigravity or pi) +- **WHEN** the user runs `nsolid-plugin setup --harness claude|codex|opencode|antigravity|pi` +- **THEN** the shared runtime ends up ready, regardless of whether that harness consumes the wrapper (claude/codex/antigravity) or native HTTP config (opencode/pi) +- **AND** for opencode/pi setup still writes their own MCP config and skills exactly as before + +#### Scenario: npm failure fails setup but preserves credentials + +- **GIVEN** npm fails, times out, or is unavailable during runtime provisioning +- **WHEN** the user runs `nsolid-plugin setup --harness ` +- **THEN** setup finishes with `success: false` and an actionable `MCP runtime setup failed` error asking the user to rerun the same command +- **AND** any credentials already stored remain valid and untouched +- **AND** the temporary staging directory is cleaned up only after installer termination is confirmed; otherwise it is marked retained-live and excluded from publication and cleanup +- **AND** a previously valid runtime, if any, is left intact + +#### Scenario: A timed-out managed npm process tree is terminated safely + +- **GIVEN** npm spawns child or grandchild processes within its managed process group/tree during runtime provisioning +- **WHEN** the setup-time npm timeout elapses +- **THEN** Unix setup signals the detached process group, awaits the root process and confirms the group no longer exists; Windows setup awaits `taskkill /T /F` and the root process +- **AND** staging is cleaned only after termination is confirmed +- **AND** if confirmation reaches its bounded deadline, setup returns an actionable `terminationError`, records retained-live ownership metadata, and excludes staging from publication and cleanup while a survivor may still mutate it + +#### Scenario: An npm spawn error fails setup cleanly + +- **GIVEN** the resolved npm entry point cannot be spawned at all (missing or broken) +- **WHEN** the user runs setup for any harness +- **THEN** setup finishes with `success: false` and an actionable error distinguishing the spawn failure from an npm exit failure +- **AND** the staging directory is cleaned up and no partial runtime is published +- **AND** stored credentials are preserved and the command can be retried + +#### Scenario: A failed reinstall keeps an invalid runtime untouched + +- **GIVEN** the versioned root exists but fails validation (wrong version, missing `dist/proxy.js`, or incomplete dependency closure) +- **WHEN** setup re-provisions the runtime and the npm install or the staging validation fails +- **THEN** the pre-existing invalid runtime remains in place +- **AND** nothing is deleted before a replacement staging tree has validated + +#### Scenario: Tokens and URLs keep argv boundaries during provisioning and startup + +- **GIVEN** credentials containing spaces, quotes, `&`, `%PATH%` or other hostile characters +- **WHEN** setup provisions the runtime or the wrapper starts the bridge +- **THEN** URLs and header values are passed as separated argv elements (or in-process arguments) and never through a shell +- **AND** no secret appears in logs or error output + +### Requirement: Runtime readiness validates dependency identity and versions + +Runtime readiness SHALL verify, for every dependency in the runtime's +transitive closure, that the resolved package has the requested name, that its +installed version satisfies the dependent's declared range, and that +the canonical package, manifest and proxy targets never escape the canonical +runtime root — so every state called `ready` is loadable by the wrapper. + +#### Scenario: A wrong-named transitive dependency is rejected + +- **GIVEN** a dependency slot of `mcp-remote` (or of one of its transitives) resolves to a package whose `package.json` `name` differs from the requested dependency name +- **WHEN** readiness is evaluated +- **THEN** the runtime is reported invalid with a reason naming the mismatched dependency +- **AND** the invalid runtime is not reused without replacement + +#### Scenario: An incompatible transitive version is rejected + +- **GIVEN** a resolved dependency's installed `version` does not satisfy the range declared by its dependent (using the documented semver range syntax) +- **WHEN** readiness is evaluated +- **THEN** the runtime is reported invalid with a reason naming the unsatisfied range +- **AND** a range that cannot be parsed fails closed as invalid + +#### Scenario: A missing transitive dependency is rejected at setup time + +- **GIVEN** a non-optional dependency of `mcp-remote` or of one of its transitives resolves to no `package.json` inside the runtime +- **WHEN** readiness is evaluated +- **THEN** the runtime is reported invalid with a reason naming the missing dependency +- **AND** the missing package is never silently satisfied from outside the runtime + +#### Scenario: Dependency resolution is confined to the runtime root + +- **GIVEN** a package matching a needed dependency name exists only above the runtime root (for example in a user's global or home-directory `node_modules`) +- **WHEN** readiness walks the dependency closure +- **THEN** resolution stops at the runtime root and the dependency counts as missing +- **AND** no `package.json` outside the runtime root is ever read to satisfy the closure + +#### Scenario: Runtime package symlink escapes are rejected + +- **GIVEN** `mcp-remote`, `dist/proxy.js`, or a transitive dependency appears lexically inside the runtime root but its canonical target is outside the canonical root +- **WHEN** readiness is evaluated +- **THEN** the runtime is reported invalid before package code is imported +- **AND** the same canonical boundary rule applies to package directories, manifests and the proxy file + +#### Scenario: Dependency kinds follow runtime-install semantics + +- **GIVEN** a package declares required `dependencies`, missing `optionalDependencies`, and `peerDependencies` or `devDependencies` +- **WHEN** readiness walks its closure +- **THEN** every missing required dependency makes the runtime invalid +- **AND** missing optional dependencies are tolerated +- **AND** peer and development dependencies are ignored + +### Requirement: Runtime publication is serialized and recoverable + +Publishing a runtime version SHALL be serialized by a per-version lock owned +under the runtime parent, SHALL only ever move fully validated staging trees +into the versioned root, SHALL never remove a valid runtime, and SHALL recover +deterministically from any interruption — including a kill between the two +renames of an invalid-runtime replacement. + +#### Scenario: An invalid runtime is replaced only by a validated staging + +- **GIVEN** the versioned root exists but fails validation +- **WHEN** setup re-provisions the runtime and the staging tree validates +- **THEN** the invalid runtime is replaced under the publication lock by the validated staging tree +- **AND** readiness reports the runtime as ready afterwards + +#### Scenario: Interrupted installation never publishes a partial runtime + +- **GIVEN** runtime provisioning is interrupted (process killed, npm crash) +- **WHEN** the runtime root is inspected afterwards +- **THEN** no partially installed runtime is published at the versioned root +- **AND** leftover staging and stale-aside directories are ignored by readiness checks + +#### Scenario: Interruption between the replacement renames leaves a recoverable state + +- **GIVEN** an invalid runtime exists and a replacement is in progress +- **WHEN** the replacing process dies after `root` has been renamed aside but before the validated staging has been renamed in +- **THEN** the versioned root is absent and the renamed-aside tree is an inert sibling ignored by probes +- **AND** the wrapper fails fast with the repair message instead of starting a broken bridge +- **AND** no valid runtime has been destroyed (replacement only ever targets an invalid root) + +#### Scenario: Retry after an interrupted replacement recovers deterministically + +- **GIVEN** the versioned root is absent after an interrupted replacement (with or without inert stale siblings) +- **WHEN** the user reruns the repair command printed by the wrapper +- **THEN** the next operation publishes its own validated staging through the root-absent branch and ends with exactly one valid runtime at the versioned root +- **AND** the orphaned stale siblings are neither promoted nor required for recovery + +#### Scenario: Potentially live staging is retained until reclamation is safe + +- **GIVEN** managed-tree termination was not confirmed and staging is marked retained-live with its operation and process identity +- **WHEN** a later setup scans temporary trees under the publication lock +- **THEN** that staging is excluded from publication and deletion while its creator or managed process tree may still be alive or liveness is unknown +- **AND** after the grace period it may be reclaimed only when the creator is proven dead, the managed process tree is confirmed absent, and no live lock carries its operation token + +#### Scenario: Orphaned stale trees are reclaimed conservatively + +- **GIVEN** a stale-aside tree has valid ownership metadata from an interrupted publisher +- **WHEN** a later setup holds the publication lock and a valid versioned root exists +- **THEN** the stale tree may be removed only after the creator is proven dead and no live lock carries its operation token +- **AND** missing or malformed metadata, unknown liveness, permission errors or token mismatch retain the tree instead of guessing ownership + +#### Scenario: Concurrent setups converge on a valid runtime + +- **GIVEN** two `setup` processes provision the runtime concurrently +- **WHEN** both attempts finish +- **THEN** exactly one valid runtime exists at the versioned root +- **AND** the loser of the publish race accepts the valid winner and removes only its own staging + +#### Scenario: Concurrent setups replacing an invalid runtime converge under the lock + +- **GIVEN** the versioned root holds an invalid runtime and two `setup` processes concurrently re-provision +- **WHEN** both attempts finish +- **THEN** the publication lock serializes the replacement so exactly one valid runtime exists at the versioned root +- **AND** the second operation re-inspects under the lock, accepts the first operation's valid runtime, and deletes only its own staging +- **AND** a lock older than the stale threshold is broken only when its holder is proven dead; breaking it does not grant ownership until a fresh `O_EXCL` acquisition succeeds +- **AND** a live holder is never evicted based on age alone + +### Requirement: Runtime provisioning resolves a trusted npm + +Runtime provisioning SHALL resolve the npm entry point exclusively from +canonical candidates anchored to the real running Node.js installation, +and SHALL never consult `PATH`, the current working directory/project, or +`npm_execpath` — so neither a hostile project nor a non-npm package manager +can substitute the installer by filename, directory segment, symlink escape or +environment value. + +#### Scenario: npm is never resolved from PATH, the project, or the environment + +- **GIVEN** the current project's `node_modules/.bin` or the `PATH` contains an executable named `npm`, and `npm_execpath` is set +- **WHEN** setup provisions the runtime from that project +- **THEN** none of those values is consulted or executed +- **AND** npm is resolved only from the candidates anchored to the running Node.js executable + +#### Scenario: A fake npm-cli.js is never trusted + +- **GIVEN** a hostile or unrelated directory contains `node_modules/npm/bin/npm-cli.js` and `npm_execpath` points at it +- **WHEN** setup provisions the runtime +- **THEN** that file is never consulted or executed, regardless of its basename or directory segments +- **AND** npm is resolved from the Node.js-anchored candidates instead + +#### Scenario: A renamed npm entry point is ignored + +- **GIVEN** `npm_execpath` points at a renamed copy of npm's CLI entry point outside the running Node.js installation +- **WHEN** setup provisions the runtime +- **THEN** the value is ignored entirely +- **AND** npm is resolved from the Node.js-anchored candidates instead + +#### Scenario: Symlinked or path-escaped npm_execpath values are ignored + +- **GIVEN** `npm_execpath` names a path that resolves through a symlink (or `..` segments) to a file outside the running Node.js installation +- **WHEN** setup provisions the runtime +- **THEN** the value is ignored entirely +- **AND** npm is resolved from the Node.js-anchored candidates instead + +#### Scenario: A non-npm npm_execpath is ignored + +- **GIVEN** `npm_execpath` points at an existing absolute entry point of another package manager (pnpm, yarn), or is non-absolute or missing +- **WHEN** setup provisions the runtime +- **THEN** that entry point is not used +- **AND** npm is resolved from the Node.js-anchored candidates instead + +#### Scenario: Supported Node.js/npm layouts resolve without PATH + +- **GIVEN** the running Node.js installation uses a supported layout — npm's `npm-cli.js` under `node_modules/npm` adjacent to the Node binary (Windows installer), under `../lib/node_modules/npm` relative to the Node binary's directory (Unix prefix layouts such as nvm/Volta/Homebrew/macOS), or an executable `npm` sibling shim of the Node binary (Unix distributions) +- **WHEN** setup provisions the runtime +- **THEN** `process.execPath` and the candidate are canonicalized, the target is a regular file inside the canonical Node installation prefix, and npm runs without a shell +- **AND** when no anchored candidate exists, setup fails with an actionable error telling the user to install Node.js with npm + +#### Scenario: An anchored npm candidate cannot escape through a symlink + +- **GIVEN** a candidate exists at one of the Node.js-anchored locations but its canonical target escapes the canonical installation prefix +- **WHEN** setup resolves npm +- **THEN** the candidate is rejected and never executed +- **AND** setup tries the next trusted candidate or fails with the actionable npm-not-found error + +### Requirement: Onboarding dispatchers satisfy the runtime precondition + +Every onboarding dispatcher SHALL satisfy runtime provisioning — +`ensureMcpRemoteRuntime()` to readiness — before invoking harness-specific +asset installation, so OpenCode and Pi cannot bypass the precondition, while +`install()` itself remains free of authentication and dependency bootstrap. + +#### Scenario: OpenCode onboarding provisions the runtime before assets + +- **GIVEN** the native plugin bootstrap dispatches the OpenCode harness through `install()` (no authentication flow) +- **WHEN** OpenCode onboarding runs +- **THEN** the dispatcher satisfies the runtime precondition before any OpenCode skills/MCP config are installed +- **AND** a runtime-precondition failure aborts onboarding with the actionable error instead of silently skipping provisioning + +#### Scenario: Pi onboarding provisions the runtime before assets + +- **GIVEN** the native plugin bootstrap dispatches the Pi harness through `install()` (package-owned skills, MCP config only) +- **WHEN** Pi onboarding runs +- **THEN** the dispatcher satisfies the runtime precondition before any Pi MCP config is written +- **AND** the precondition needs no credentials (the npm download is anonymous) + +#### Scenario: install() itself never provisions or authenticates + +- **GIVEN** the core `install()` function is invoked directly (fallback direct installer) +- **WHEN** it completes +- **THEN** it never invoked npm, never downloaded dependencies, and never opened an authentication flow +- **AND** the runtime precondition remains the dispatchers' responsibility + +### Requirement: The MCP wrapper uses the stable runtime by default + +The generated MCP wrapper SHALL resolve `mcp-remote` from the stable shared +runtime by default. A version-matched development checkout MAY be used only +when explicit internal development mode is enabled. The wrapper SHALL NOT +execute or spawn `npx`, npm, `cmd.exe`, or a shell during startup. The repair +message may contain an `npx` command as text. + +#### Scenario: Wrapper starts from the stable runtime without npm + +- **GIVEN** a valid runtime provisioned by setup +- **WHEN** the harness starts an MCP server through the wrapper +- **THEN** immediately before import the wrapper canonicalizes the controlled runtime parent, version root, package directory, package manifest and `dist/proxy.js` +- **AND** the canonical version root remains within the canonical controlled parent, while the package directory, manifest and proxy remain within the canonical version root with their required directory/file types +- **AND** replacing the whole version root, package directory, manifest or proxy with a symlink or path that escapes its required boundary fails with the repair message before any package code is imported +- **AND** the wrapper validates the runtime's package name and exact version, imports `dist/proxy.js` locally, and passes URL/headers as separate arguments +- **AND** an `npx` sentinel on PATH is never executed + +#### Scenario: Missing or corrupt runtime fails fast with the version-pinned repair command + +- **GIVEN** the runtime is missing, has the wrong version, or lacks `dist/proxy.js` +- **WHEN** the wrapper starts +- **THEN** it exits non-zero within seconds with a message equivalent to `MCP bridge runtime is not ready. Run: npx -y nsolid-plugin@ setup --harness ` +- **AND** the message names the harness that launched the wrapper (claude, codex or antigravity) +- **AND** the pinned plugin version is the release that generated the wrapper + +#### Scenario: A project dependency cannot bypass the managed runtime + +- **GIVEN** the stable runtime is missing or invalid, a matching `mcp-remote` exists in local/project `node_modules`, and development mode is not enabled +- **WHEN** the wrapper starts +- **THEN** it does not import the project dependency and fails with the version-pinned repair message + +#### Scenario: Explicit development mode permits only the pinned fallback + +- **GIVEN** `NSOLID_MCP_RUNTIME_DEV_FALLBACK=1` is explicitly set for development and a local `mcp-remote` checkout is available +- **WHEN** the wrapper starts without a ready stable runtime +- **THEN** it accepts the fallback only when its package name, exact pinned version and proxy file validate +- **AND** released harness configurations never enable this mode + +#### Scenario: An import-time runtime failure becomes the repair message + +- **GIVEN** the runtime passes the wrapper's light validation but importing or initializing `dist/proxy.js` throws, including a missing or incompatible transitive failure +- **WHEN** the wrapper starts +- **THEN** the import-time failure is translated into the same harness-specific, version-pinned repair message instead of a raw module error +- **AND** the wrapper exits non-zero within seconds + +#### Scenario: The printed repair command recreates the wrapper's exact runtime + +- **GIVEN** a wrapper generated by plugin release X (requiring `mcp-remote@V`) runs on a machine where a newer plugin release exists +- **WHEN** the wrapper prints its repair command and the user executes it +- **THEN** the command pins `nsolid-plugin@X`, and that CLI version provisions exactly runtime version V +- **AND** the wrapper that printed the message validates the freshly provisioned runtime as ready + +### Requirement: Shared runtime lifecycle + +The shared runtime SHALL survive per-harness uninstall and logout. + +#### Scenario: Uninstall and logout preserve the shared runtime + +- **GIVEN** the shared runtime exists +- **WHEN** the user runs `nsolid-plugin uninstall --harness ` or `nsolid-plugin logout` +- **THEN** the runtime directory remains (shared across harnesses, no secrets stored) +- **AND** other harnesses continue to work + +### Requirement: Doctor reports bridge runtime health + +`doctor` SHALL surface the shared bridge runtime state and SHALL mark a +wrapper-owned harness configuration unhealthy when the runtime is missing or +invalid, without ever treating bridge readiness as proof of remote endpoint +reachability. + +#### Scenario: Required wrapper bridge missing is unhealthy + +- **GIVEN** doctor runs for a wrapper-owned harness configuration (native plugin installed for claude, codex or antigravity) +- **WHEN** the runtime is missing or invalid +- **THEN** doctor reports `healthy: false` and recommends `nsolid-plugin setup --harness ` + +#### Scenario: Ready wrapper bridge reports local readiness only + +- **GIVEN** doctor runs for a wrapper-owned harness configuration with a ready runtime +- **WHEN** human or JSON output is produced +- **THEN** bridge status is `ready` and does not make an otherwise unhealthy report healthy +- **AND** the output never claims that the remote MCP endpoint is reachable + +#### Scenario: Non-wrapper bridge state is informational + +- **GIVEN** doctor runs for opencode, pi, a native-HTTP direct install, or claude/codex/antigravity without the native plugin detected +- **WHEN** the runtime is ready, missing or invalid +- **THEN** `bridge.required` is false and bridge state does not affect health +- **AND** human and JSON output distinguish local bridge readiness from remote MCP reachability + +### Requirement: Native MCP OAuth is out of scope + +The plugin SHALL keep the Accounts OAuth flow and the service-token headers +unchanged by this change. + +#### Scenario: No confusion between Accounts OAuth and native MCP OAuth + +- **GIVEN** the current wrapper authenticates MCP traffic with `X-Nsolid-*` headers minted from the NodeSource Accounts OAuth flow +- **WHEN** this change ships +- **THEN** that mechanism is preserved unchanged +- **AND** migrating to native MCP-transport OAuth remains deferred technical debt, not implemented here diff --git a/openspec/changes/stage-mcp-runtime-during-setup/tasks.md b/openspec/changes/stage-mcp-runtime-during-setup/tasks.md new file mode 100644 index 0000000..c33693b --- /dev/null +++ b/openspec/changes/stage-mcp-runtime-during-setup/tasks.md @@ -0,0 +1,204 @@ +# Tasks + +Ordered breakdown; each group is independently testable. References: +proposal (scope/rollback), design (module contract, sequences), specs +(scenarios). + +## 1. OpenSpec change + +- [ ] Create `openspec/changes/stage-mcp-runtime-during-setup/` + (proposal, design, specs delta, tasks) per `ns-workflow`. +- [ ] Register the `submit_plan` (Plannotator) tool limitation in the + proposal; proceed under the user's explicit approval for this scope. +- [ ] Amend proposal, design, specs delta and tasks per the PR #62 review: + recoverable publication, canonical npm resolution, wrapper import + failures, dispatcher precondition, managed-tree timeout cancellation, + dependency identity/version ranges, version-pinned repair command, + execution-scoped "no npx" wording and editorial fixes. +- [ ] `openspec validate stage-mcp-runtime-during-setup --strict` passes. + +## 2. Runtime manager (`packages/core/src/mcp/mcp-remote-runtime.ts`) + +- [ ] `MCP_REMOTE_VERSION = '0.1.38'`; paths via `getAgentsDir()` only. +- [ ] `inspectMcpRemoteRuntime()`: read-only readiness probe (name, exact + version, `dist/proxy.js`, transitive dependency closure with + per-dependency name identity, semver range satisfaction and + canonical runtime root confined to the canonical controlled parent, plus + canonical package/manifest/proxy targets confined to the runtime root); + missing optional dependencies are tolerated, peer/development + dependencies ignored, and missing required dependencies rejected. +- [ ] Add `semver` to `packages/core` dependencies for range evaluation; + unparseable ranges fail closed (documented supported syntax in + `design.md`). +- [ ] `resolveNpmCommand()`: canonical Node.js-anchored candidates only — node-dir + `node_modules/npm/bin/npm-cli.js`, then `../lib/node_modules/npm/bin/ + npm-cli.js`, then node-dir `npm` sibling shim; otherwise actionable + error. Canonicalize `process.execPath` and each candidate; require a + regular-file target inside the canonical installation prefix. Never + consults `PATH`, `cwd`/project `.bin`, or + `npm_execpath` (fake, renamed, symlinked and pnpm/yarn values are + ignored by construction). +- [ ] `ensureMcpRemoteRuntime()`: idempotent check, staging sibling under the + controlled runtime parent (same filesystem as the versioned root) + + private package.json, npm without shell (separated argv and the complete + `--omit=dev`, `--ignore-scripts`, `--no-audit`, `--no-fund`, + `--save-exact`, `--no-package-lock` safety set), bounded stderr tail, + 5-minute timeout, staging + validation, publication under the per-version lock, race + convergence, invalid-runtime replacement via rename-aside, actionable + error. +- [ ] Publication protocol: `O_EXCL` lock file under the runtime parent + keyed by version with unique owner token; bounded-backoff waiting; + break a lock older than 10 minutes only when its holder is proven dead, + then reacquire with a fresh `O_EXCL` create (moving a stale lock does not + grant ownership); never evict a live holder based on age; re-inspect + `root` under the lock and accept a valid winner; validate staging + before touching `root`; root-absent publish = single rename; invalid + root replaced by rename-aside + rename-in + stale removal; recovery + of an absent root is the normal root-absent branch; cleanup strictly + limited to operation-created staging/stale/lock paths, except for the + lock-held safe-reclamation protocol using ownership/liveness metadata; + `EXDEV`/cross-filesystem rename fails closed with no copy fallback. +- [ ] Timeout handling: terminate and confirm the managed npm process tree + stopped (Unix: detached process group, SIGTERM → SIGKILL escalation, + root close and group-disappearance polling; Windows: await + `taskkill /T /F` and root close) before staging cleanup; leave staging + marked retained-live and excluded from publication/cleanup when + confirmation times out; later runs reclaim it only after creator death, + managed-tree absence, lock-token exclusion and the grace period; return + `terminationError`; spawn errors + (`ENOENT`/`EACCES`/`EPERM`) surfaced as an explicit `spawnError` + result, never as exit status. +- [ ] Internal re-export from `packages/core/src/mcp/index.ts`. + +## 3. Setup integration & dispatcher precondition + +- [ ] `setup()` in `packages/core/src/index.ts`: after credentials are valid + (or immediately when no auth), call `ensureMcpRemoteRuntime()` before + any per-harness install branch; progress lines `Preparing MCP bridge + runtime — installed mcp-remote 0.1.38` / `already ready`; failure ⇒ + `MCP runtime setup failed: …`, `success: false`, no "setup complete". +- [ ] `packages/core/scripts/setup.mjs`: satisfy the runtime precondition + (credentials-free `ensureMcpRemoteRuntime()`) before delegating to + `install()` for opencode/pi, so neither harness bypasses + provisioning. +- [ ] `packages/core/src/cli.ts`: satisfy the same precondition before the + fallback `install` command. +- [ ] `install()` still never downloads/authenticates on its own + (regression guard). +- [ ] Update `packages/core/scripts/setup.mjs` and `packages/core/src/cli.ts` + wording (setup = credentials **and** bridge). + +## 4. Wrapper / generators + +- [ ] `scripts/plugin-generators.mjs`: export `MCP_REMOTE_VERSION`; embed + `MCP_REMOTE_VERSION` **and** `PLUGIN_VERSION` (the generating + release) in the wrapper; wrapper takes ` `, + validates both, resolves the stable runtime first, version-matched + `createRequire` dev fallback second only when + `NSOLID_MCP_RUNTIME_DEV_FALLBACK=1` is explicitly set, and never executes or spawns + `npx`, npm, `cmd.exe`, or a shell (the repair message may contain an + `npx` command as text); immediately before import, canonicalizes the + controlled runtime parent, stable root, package directory, manifest and + proxy, rejects a stable root outside the canonical parent and enforces + segment-aware containment under that root, while the explicit dev fallback + confines its manifest/proxy to its canonical package directory; thrown import errors + and surfaced initialization rejections are translated into the repair + message, while direct imported-code `process.exit` is an explicit + in-process limitation (no child-process redesign in this change); repair message is + version-pinned: `npx -y nsolid-plugin@ setup + --harness `; Claude config passes `claude`; Codex and + Antigravity bootstraps pass `codex`/`antigravity`. +- [ ] Regenerate root artifacts via `pnpm plugin:root` + (`.mcp.json`, `.claude-mcp.json`, `mcp_config.json`, + `scripts/mcp-wrapper.js`); keep `startup_timeout_sec: 60`. +- [ ] `pnpm plugin:check` reports no drift. + +## 5. Doctor + +- [ ] `DoctorReport.bridge` (optional) in `packages/core/src/types.ts`. +- [ ] `doctor()` in `packages/core/src/index.ts`: required ⇔ wrapper-owned + (claude/codex/antigravity with native plugin detected); error + unhealthy + when required and not ready; informational otherwise; never implies the + remote MCP is reachable. +- [ ] `formatDoctorReport` human output + `--json` compatibility; update + `packages/core/test/unit/utils/format.test.ts`. + +## 6. Tests + +- [ ] New `packages/core/test/unit/mcp/mcp-remote-runtime.test.ts`: paths with + spaces; initial install via fake runner; idempotence; invalid version; + missing proxy; incomplete transitives; wrong-named transitive; + incompatible transitive version; missing optional dependency tolerated; + peer/dev dependencies ignored; missing required dependency rejected; + dependency resolution confined to the runtime root; symlink escapes for + `mcp-remote`, `dist/proxy.js` and transitives rejected; npm error/timeout cleanup; managed process-group timeout + termination confirmation (grandchild included) and unconfirmed- + termination staging preservation with retained-live metadata; safe + reclamation after proof and retention on unknown/malformed ownership; + npm spawn error; + prior valid runtime survives failed reinstall; publish race; race + replacing an invalid runtime; interruption between the two replacement + renames (root absent, stale inert); retry recovery determinism; + pre-existing invalid runtime (wrong version, missing proxy or incomplete + dependency closure) remains untouched when npm or staging validation + fails; + stale dead-lock break followed by fresh acquisition, live-lock + non-eviction and competing-breaker serialization; `shell: false` argv + separation; staging is a same-parent/same-filesystem sibling and + `EXDEV` fails without a copy fallback; fake and real runner argv assert + the complete `--omit=dev`, `--ignore-scripts`, `--no-audit`, `--no-fund`, + `--save-exact`, `--no-package-lock` safety set; trusted npm resolution (fake `npm-cli.js`, renamed entry + point, anchored-candidate symlink/path escape, pnpm/yarn `npm_execpath` + ignored, supported Node/npm layouts, + `node_modules/.bin` and `PATH` never consulted); no secrets in output. +- [ ] Rewrite `packages/core/test/unit/mcp/mcp-wrapper.test.ts`: stable-runtime + fixture for `source` and `generated` wrappers; hostile URL/token argv + boundaries; `npx` sentinel (exit 97) never executed; direct `npm` + sentinel never executed in stable-runtime and explicit dev-fallback + modes; fast fail when + runtime missing / version mismatched even when local `node_modules` has a + matching package; explicit dev-mode fallback accepts only the pinned + package; post-publication whole-runtime-root, package-directory, manifest and proxy + symlink/replacement escapes are rejected immediately before import for + both source and generated wrappers, with the dev fallback checked against + its own canonical package boundary; thrown import errors and surfaced + initialization rejections translated into the repair message; + direct imported-code process termination is not promised to translate; + harness-correct, + version-pinned repair message; no `cmd.exe`/shell execution paths; + `MCP_REMOTE_VERSION` sync across the core constant, generator export and + root `package.json` `dependencies['mcp-remote']`; separate + `PLUGIN_VERSION` sync with the generating plugin release and wrapper; + old-wrapper/new-CLI repair + (wrapper of release X prints `nsolid-plugin@X`, which provisions + exactly X's pinned runtime version). +- [ ] Update `packages/core/test/integration/installer.test.ts` (seed runtime + / fake `npm_execpath` harness): setup installs runtime without browser; + runtime failure keeps credentials with `success: false`; + five-harness convergence; opencode/pi dispatcher scenarios (runtime + provisioned before assets); `install()` purity regression guards; + uninstall/logout preserve runtime; doctor bridge matrix covering ready + and missing wrapper-owned runtimes, opencode/pi, native-HTTP direct + installs, and claude/codex/antigravity without native plugin detection; + only required-but-unready is unhealthy and no output claims remote MCP + reachability. + +## 7. Documentation & lifecycle + +- [ ] `README.md` and `packages/core/README.md`: setup authenticates **and** + prepares the bridge; first run needs network, later runs idempotent; + troubleshooting entry for "runtime missing/corrupt" (vs expired + token); the repair command is version-pinned; npm is resolved from + the Node.js installation only; dev note "the wrapper never downloads + dependencies during startup". + +## 8. Validation & commit + +- [ ] `openspec validate stage-mcp-runtime-during-setup --strict` passes after + all final spec and task edits. +- [ ] `pnpm --filter nsolid-plugin lint`, `pnpm --filter nsolid-plugin test`, + `pnpm plugin:check`, `pnpm test:marketplace`, `pnpm test`. +- [ ] `git diff --check`, `git status --short` clean of drift. +- [ ] Atomic conventional commit: `fix(mcp): provision bridge runtime during + setup`. No push/PR.