fix(mcp): provision bridge runtime during setup

[Cesar-M-Diaz]

• Stage-install the shared mcp-remote runtime during setup; publish only validated roots. npm is resolved near the running node, never from PATH or the project .bin; non-npm npm_execpath values (pnpm/yarn) are ignored.
• MCP wrapper uses only the stable runtime and fails fast with a per-harness repair command.
• Doctor reports bridge runtime health (required only for plugin-owned harnesses).
• Keep the Codex MCP startup timeout at 60s.
• Preserve third-party MCP server fields (command/args/tools) when rewriting the codex TOML config.
• Consolidate duplicated harness sets into types.ts with a generator/core sync guard; drop the middle-man wrapper generators.
• Add a typecheck pre-commit gate; allocate auth test ports dynamically.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0d45863f-2f26-4e0f-912e-527b7a3524f6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}

[ns-control-tower]

Walkthrough

This PR provisions a shared, exact-pinned mcp-remote runtime during nsolid-plugin setup so the generated MCP wrappers never invoke npm/npx/shell during harness startup. setup stage-installs into a randomized staging dir, validates the dependency closure statically, and publishes via an atomic rename with race handling that never degrades a valid runtime. The wrapper now takes an explicit --harness argument, resolves mcp-remote only from the stable runtime (or a version-matched dev checkout), and fails fast with a per-harness repair command. Collateral changes: the codex TOML writer preserves third-party stdio server fields, the Codex MCP startup timeout is pinned at 60s, duplicated harness sets are consolidated into types.ts with a generator/core sync guard, a typecheck pre-commit gate is added, and auth tests allocate ports dynamically to avoid cross-file contention.

Changes

File(s)	Summary
packages/core/src/mcp/mcp-remote-runtime.ts (new)	Runtime manager: inspect, stage-install, validate closure, atomic publish, guarded cleanup, npm resolution that avoids PATH/project .bin
scripts/mcp-wrapper.js, scripts/plugin-generators.mjs	Wrapper resolves mcp-remote only from the stable runtime/dev checkout; harness arg; generator consolidates to a single generateMcpWrapper() and syncs version/harness constants
packages/core/src/index.ts	setup provisions the runtime before per-harness install; doctor reports bridge health (required only for plugin-owned harnesses)
packages/core/src/types.ts	Consolidated PLUGIN_OWNED_HARNESSES / NATIVE_PLUGIN_HARNESSES; DoctorReport.bridge shape
packages/core/src/mcp/mcp-config-writer.ts	TOML writer preserves full server objects ({ ...srv }) instead of rebuilding from a url/headers whitelist
packages/core/src/utils/format.ts, src/cli.ts, scripts/setup.mjs	Bridge line in doctor output; setup/CLI messaging
.claude-mcp.json, .mcp.json, mcp_config.json	Pass harness arg through wrapper; add startup_timeout_sec: 60 for codex
packages/core/test/**	New runtime tests (491 lines), wrapper contract tests, config-writer TOML regression tests, dynamic auth test ports
.husky/pre-commit, package.json, packages/core/package.json, eslint.config.js	typecheck gate; **/dist/** ignore glob
README.md	Docs for the bridge runtime, repair flow, and doctor output

Assessment

• No blocking findings. The security model is strong: the wrapper never invokes a shell, npx, or cmd.exe; npm resolution is confined to the node directory and a validated npm_execpath (pnpm/yarn rejected); safeRemove asserts paths stay inside the runtime parent before any rmSync; staging is validated (package name, version, dist/proxy.js, and a static dependency-closure walk) before the atomic rename; --ignore-scripts is used for the install.
• Validation run in sandbox (fresh clone at head b54b3c3): node --check on generated scripts ✓; tsc --noEmit ✓; eslint on changed core sources ✓; 105 unit tests pass ✓; 91 integration tests pass ✓; materialize-github-marketplace --check ✓; sync-plugin-assets --check ✓.
• Two non-blocking notes inline: (1) 🛠️ the wrapper's runtime path and core's getMcpRemoteRuntimeRoot() are independent sources of truth that agree today but aren't cross-tested; (2) 🧹 the CODEX_MCP_STARTUP_TIMEOUT_SEC constant isn't guarded against the committed .mcp.json the way the version/harness constants are.
• 🚩 This change touches the MCP runtime supply chain (what mcp-remote code runs at harness startup) and the codex config writer. The install uses --ignore-scripts and pins an exact version with closure validation, which is the right posture, but a human reviewer should confirm the pinned mcp-remote@0.1.38 and its transitive deps are acceptable to run with user credentials in scope.

Verdict: APPROVE — clean pass with two non-blocking refactor/nit notes; advisory and does not replace required human review.

[ns-control-tower]

🛠️ Refactor suggestion

The runtime root path is assembled twice with slightly different sources of truth: core uses getAgentsDir() (which resolves os.homedir()/.agents), while the generated wrapper hardcodes path.join(os.homedir(), '.agents', 'nsolid-plugin', 'runtime', 'mcp-remote', MCP_REMOTE_VERSION). They agree today, but a future change to getAgentsDir() (e.g. honoring NSOLID_HOME or XDG_CONFIG_HOME) would silently desync the wrapper from the provisioning code, and the existing sync tests only compare the wrapper against the generator, not against core's getMcpRemoteRuntimeRoot().

Consider adding a unit test that asserts the wrapper's resolved runtime path equals getMcpRemoteRuntimeRoot() (or at minimum that getAgentsDir() continues to mean os.homedir()/.agents), so a divergence is caught at test time rather than at a user's harness startup.

[ns-control-tower]

🧹 Nitpick

CODEX_MCP_STARTUP_TIMEOUT_SEC = 60 lives only here in the generator, while the same 60s value is written as startup_timeout_sec: 60 literals in .mcp.json and is referenced in the PR description ("Keep the Codex MCP startup timeout at 60s"). The value is now consistent across the generated artifacts, but there's no test guarding the generator constant against the committed .mcp.json the way the mcp-remote version and harness lists are guarded. Not blocking since the value is stable and the --check materializer validates generated output byte-for-byte, but a future bump would need to update the constant and the committed JSON in lockstep.

[ns-control-tower]

Walkthrough

Re-review on synchronize (head b54b3c3 → 0b3c48e, a force-push that squashed to a single commit with identical logic). The PR provisions a shared, exact-pinned mcp-remote runtime during nsolid-plugin setup so the generated MCP wrappers never invoke npm/npx/shell during harness startup. setup stage-installs into a randomized staging dir, validates the dependency closure statically, and publishes via an atomic rename with race handling that never degrades a valid runtime. The wrapper resolves mcp-remote only from the stable runtime (or a version-matched dev checkout) and fails fast with a per-harness repair command. Collateral changes: the codex TOML writer preserves third-party stdio server fields, the Codex MCP startup timeout is pinned at 60s, duplicated harness sets are consolidated into types.ts with a generator/core sync guard, a typecheck pre-commit gate is added, and auth tests allocate ports dynamically to avoid cross-file contention.

Changes

File(s)	Summary
packages/core/src/mcp/mcp-remote-runtime.ts (new)	Runtime manager: inspect, stage-install, validate closure, atomic publish, guarded cleanup, npm resolution that avoids PATH/project .bin
scripts/mcp-wrapper.js, scripts/plugin-generators.mjs	Wrapper resolves mcp-remote only from the stable runtime/dev checkout; harness arg; generator consolidates to a single generateMcpWrapper() and syncs version/harness constants
packages/core/src/index.ts	setup provisions the runtime before per-harness install; doctor reports bridge health (required only for plugin-owned harnesses)
packages/core/src/types.ts	Consolidated PLUGIN_OWNED_HARNESSES / NATIVE_PLUGIN_HARNESSES; DoctorReport.bridge shape
packages/core/src/mcp/mcp-config-writer.ts	TOML writer preserves full server objects ({ ...srv }) instead of rebuilding from a url/headers whitelist
packages/core/src/utils/format.ts, src/cli.ts, scripts/setup.mjs	Bridge line in doctor output; setup/CLI messaging
.claude-mcp.json, .mcp.json, mcp_config.json	Pass harness arg through wrapper; add startup_timeout_sec: 60 for codex
packages/core/test/**	New runtime tests (491 lines), wrapper contract tests, config-writer TOML regression tests, dynamic auth test ports
.husky/pre-commit, package.json, packages/core/package.json, eslint.config.js, .gitattributes	typecheck gate; **/dist/** ignore glob; LF-eol enforcement for byte-for-byte wrapper sync
README.md, packages/core/README.md	Docs for the bridge runtime, repair flow, and doctor output

Assessment

• Re-review confirms no blocking findings at the new head. The security model is unchanged and strong: the wrapper never invokes a shell, npx, or cmd.exe; npm resolution is confined to the node directory and a validated npm_execpath (pnpm/yarn rejected); safeRemove asserts paths stay inside the runtime parent before any rmSync; staging is validated (package name, version, dist/proxy.js, and a static dependency-closure walk) before the atomic rename; --ignore-scripts is used for the install.
• Validation run in sandbox (fresh clone at head 0b3c48e): node --check on mcp-wrapper.js, plugin-generators.mjs, materialize-github-marketplace.mjs ✓; tsc --noEmit (core) ✓; eslint on changed core sources ✓; 106 unit tests pass (mcp-remote-runtime, mcp-wrapper, mcp-config-writer, format) ✓; sync-plugin-assets --check ✓; materialize-github-marketplace --check ✓.
• Two previously raised non-blocking notes remain open and are still non-blocking at this head:
  • 🛠️ scripts/mcp-wrapper.js:119 — the wrapper's runtime path (path.join(os.homedir(), '.agents', ...)) and core's getMcpRemoteRuntimeRoot() (via getAgentsDir()) are independent sources of truth that agree today but aren't cross-tested. The version and harness lists now have sync tests, but no path-equality test guards a future getAgentsDir() change (e.g. honoring XDG_CONFIG_HOME) from silently desyncing the wrapper from provisioning. Optional hardening, not blocking.
  • 🧹 scripts/plugin-generators.mjs:36 — CODEX_MCP_STARTUP_TIMEOUT_SEC = 60 is not guarded against the committed .mcp.json literals the way the version/harness constants are. The --check materializer validates generated output byte-for-byte, so a future bump would be caught at sync time; a dedicated constant test would catch it at unit-test time. Optional, not blocking.
• 🚩 This change touches the MCP runtime supply chain (what mcp-remote code runs at harness startup) and the codex config writer. The install uses --ignore-scripts and pins an exact version with closure validation, which is the right posture, but a human reviewer should confirm the pinned mcp-remote@0.1.38 and its transitive deps are acceptable to run with user credentials in scope.

Verdict: APPROVE — re-review confirms the squashed head carries the same logic as the previously approved commit; no blocking findings. Advisory and does not replace required human review.