fix(pi): bridge MCP tools into pi.registerTool() so the LLM can call them (#426)#472
Conversation
…them (mksglu#426) Pi 0.73.x has no native MCP support — its README is explicit: > No MCP. Build CLI tools with READMEs (see Skills), or build an > extension that adds MCP support. Without a bridge inside the context-mode Pi extension, the routing block tells the LLM to call `ctx_execute` / `ctx_search` / etc. but those tools never enter Pi's tool list and the LLM cannot reach them. The reporter measured 18 sessions over 2 days: ~2,500 tokens of system-prompt overhead per window, 0 actual ctx_* calls, 447 events recorded but never retrieved. Net ROI on Pi was negative. This adds a stdio JSON-RPC client (`MCPStdioClient`) plus a thin bootstrap (`bootstrapMCPTools`) that: - spawns `server.bundle.mjs` as a long-lived MCP child, - performs the standard MCP handshake (initialize → notifications/initialized), - lists tools once via `tools/list`, and - registers each tool through `pi.registerTool({ name, label, description, parameters, execute })` so the LLM sees the canonical bare names (matching what hooks/core/tool-naming.mjs emits for Pi). Each Pi `execute()` callback forwards into the MCP child via `tools/call`. Errors are translated to `throw` (Pi's contract for "tool failed") so the LLM sees the MCP server's diagnostic text. Lifecycle: - Bridge bootstrap is fire-and-forget at extension load — the rest of the extension (session capture, hooks, slash commands) is not blocked by spawn / handshake latency. - `session_shutdown` terminates the child via SIGTERM. - A missing `server.bundle.mjs` or any spawn / handshake error is surfaced once on stderr, then the extension keeps running with only the existing hooks + commands. Defense-in-depth so the bridge can never break Pi sessions for users with broken installs. ## Why a JSON Schema parameters object instead of TypeBox MCP `tools/list` returns JSON Schema. Pi's parameter validator accepts JSON Schema directly (TypeBox just produces JSON Schema with extra Symbol metadata for type inference). Passing the schema through unchanged avoids a runtime translation pass and keeps the bridge a true thin layer over the MCP protocol — what works in Claude Code, Gemini CLI, and the other adapters now also works in Pi. ## No new runtime dependencies Pure `node:child_process` + `node:path`. The `@earendil-works/pi-*` packages are NOT pulled in as build deps — `pi` is typed structurally as `any` (matching the existing src/pi-extension.ts style) and the bridge only touches the documented `pi.registerTool()` shape. ## Tests Added two new `describe` blocks in `tests/pi-extension.test.ts`: 1. `MCPStdioClient` (5 tests) — wire-protocol contract pinned with fake stdio servers: id-matched responses, concurrent in-flight requests with out-of-order delivery, child-exit cancellation, timeout, non-JSON noise tolerance. 2. `bootstrapMCPTools` (2 integration tests) — spawn the real `start.mjs` MCP server, assert that the canonical ctx_* set (`ctx_execute`, `ctx_execute_file`, `ctx_search`, `ctx_index`, `ctx_batch_execute`, `ctx_fetch_and_index`, `ctx_doctor`, `ctx_stats`, `ctx_purge`) is registered, and round-trip `ctx_index` through `tools/call` to confirm execute() forwards args and returns text. ## Test plan - [x] `npm run build` - [x] `npm run typecheck` clean - [x] `npm test` — 73 files, 2406 pass / 25 skipped / 0 fail - [x] `npx vitest run tests/pi-extension.test.ts` — 44 pass (37 pre-existing + 7 new for the bridge) - [x] see-real-bug repro: `pi.registerTool` count = 0 in installed binary (`/home/$USER/.nvm/.../context-mode/build/pi-extension.js`) on `next` @ 1f70bee, plus Pi README's "No MCP" stance, plus the issue reporter's 18-session measurements — all three agree. - [-] Live LLM tool-call probe in Pi: blocked — free-tier Gemini quota was exhausted on every available key during the fix session. The integration test exercises the same code path (real MCP server + the Pi-facing registerTool surface), so the regression contract is enforced from CI. ## Out of scope - Removing the MCP server stanza from the Pi install README. Once this lands, the `~/.pi/agent/mcp.json` step is still harmless but no longer load-bearing. Cleanup left to a docs-only follow-up. - In-process refactor of server.ts handlers. The subprocess bridge is the same model used by every other adapter; a refactor that inlines the handlers is its own scope. Co-Authored-By: Ora Studio <noreply@oratelecom.net>
Self-review (mode: empirical) — PR #472Head SHA: Phase A (bug reproduces without fix): PASS — reverted Phase B (test passes with fix): PASS — 7/7 green on head. Quality lenses:
Findings[HIGH] No test asserts the wiring in Fix: add a test that calls [LOW] Dead getter [LOW] Path resolution duplicated across the two integration tests. VerdictREQUEST_CHANGES — only for the C1 wiring-test gap. C2/C3 are nits, ship-or-not at maintainer discretion. |
Three follow-up changes from the empirical self-review on PR mksglu#472: 1. **C1 HIGH — wiring not test-covered.** Phase A of the empirical review only failed because removing `src/pi-mcp-bridge.ts` produced an import error, not because the bug reproduced behaviorally. If a future refactor dropped the `bootstrapMCPTools(pi, …)` call from `src/pi-extension.ts` while keeping the bridge module intact, every existing bridge test stayed green and the bug silently re-entered. Fix: export `_mcpBridgeReady: Promise<void>` from `src/pi-extension.ts`. Bootstrap is still fire-and-forget (so spawn / handshake latency does not block session_start), but the promise gives tests a deterministic await point. Reset to a fresh promise on every `piExtension(pi)` call so multiple registrations in one process do not see a stale resolution. New test in `tests/pi-extension.test.ts` ("pi-extension.ts wiring (mksglu#426 regression guard)"): calls `registerPiExtension(api)`, awaits `_mcpBridgeReady`, asserts `api.registerTool.mock.calls` includes at least the canonical `ctx_execute` / `ctx_search` / `ctx_index` / `ctx_batch_execute` / `ctx_fetch_and_index` set. Verified red-on-revert: with the bridge module intact but the `bootstrapMCPTools(...)` call reverted to next, this test fails with `registeredNames: []`. Pre-fix it would have stayed green. 2. **C2 LOW — duplicated path resolution in the integration tests.** `tests/pi-extension.test.ts` had `path.dirname(...) + path.resolve(here, "..", "start.mjs")` recomputed in each `it()`. Lifted to a single `mcpEntry` const at the top of the `bootstrapMCPTools — registers every ctx_* tool with Pi` describe block, plus a shared `mcpEnv` for the `CONTEXT_MODE_DISABLE_VERSION_CHECK` override. One place to update if `start.mjs` ever moves. 3. **C3 LOW — dead `running` getter on MCPStdioClient.** Exported in the original commit but had zero callers anywhere in `src/` or `tests/`. Dropped — five lines, no behavioral impact. ## Test plan - [x] `npm run build` - [x] `npm run typecheck` — clean - [x] `npm test` — 73/73 files, 2407 pass / 25 skipped / 0 fail - [x] `npx vitest run tests/pi-extension.test.ts -t "MCP bridge|wiring"` — 8 pass - [x] Phase A re-validation: revert ONLY the wiring in `src/pi-extension.ts` (keep `src/pi-mcp-bridge.ts` intact), run the wiring test → fails with `registeredNames: []`. Restore and the test goes green again. Co-Authored-By: Ora Studio <noreply@oratelecom.net>
|
Self-review findings addressed in [HIGH C1] Wiring test added. Exported [LOW C2] Path resolution lifted. [LOW C3] Dead Suite: 73 files, 2407 pass / 25 skipped / 0 fail. Bridge + wiring tests: 8/8. |
…claw/
Pre-fix layout split OpenClaw across two locations:
- src/adapters/openclaw/ — config, hooks, index, session-db (standard
adapter pattern matching every other platform)
- src/openclaw/ — mcp-tools, workspace-router (rogue location)
The split predates the adapter pattern: workspace-router.ts was added
first by Pedro Almeida (#aa8d93c), then mcp-tools.ts by the maintainer
(#ff0a9a2 v1.0.107), while the adapter dir was bootstrapped later by
the copilot-swe-agent (#5fd6a9e). Source-of-truth for every platform
should live under src/adapters/<platform>/, so we move the two
stragglers in.
## Changes
- git mv src/openclaw/mcp-tools.ts → src/adapters/openclaw/mcp-tools.ts
- git mv src/openclaw/workspace-router.ts → src/adapters/openclaw/workspace-router.ts
- rmdir src/openclaw
- src/openclaw-plugin.ts: 3 import-path updates
- tests/plugins/openclaw.test.ts: 1 import-path update
- tests/core/cli.test.ts: 1 readFileSync source-grep path update
(the existing PR mksglu#183 path-traversal regression test reads the
workspace-router source file directly to grep for safe-regex
patterns; pin updated to the new location)
## Test plan
- [x] npm run typecheck — clean
- [x] npx vitest run tests/plugins/openclaw.test.ts tests/core/cli.test.ts
→ 225/225 pass
- [x] npm test — 73 files, 2405+ pass / 25 skipped / 0 fail
Co-Authored-By: Ora Studio <noreply@oratelecom.net>
The src/concurrency/ directory held a single file. A whole directory for one module is structural noise — flatten it to src/runPool.ts. ## Changes - git mv src/concurrency/runPool.ts → src/runPool.ts - rmdir src/concurrency - src/server.ts: import path updated - tests/core/server.test.ts: import path updated ## Test plan - [x] npm run typecheck — clean - [x] npx vitest run tests/core/server.test.ts -t "runPool" — pass Co-Authored-By: Ora Studio <noreply@oratelecom.net>
Pre-fix layout had three platform plugin entry files at the src/ root: src/pi-extension.ts — Pi Coding Agent extension src/pi-mcp-bridge.ts — Pi MCP bridge (added in mksglu#426) src/openclaw-plugin.ts — OpenClaw gateway plugin src/opencode-plugin.ts — OpenCode plugin Every other platform follows the src/adapters/<name>/ pattern (config, hooks, index, …). The four root-level files were the last hold-outs: inconsistent layout, plus they made adapter discovery harder for new contributors. ## Changes (file moves) - git mv src/pi-extension.ts → src/adapters/pi/extension.ts - git mv src/pi-mcp-bridge.ts → src/adapters/pi/mcp-bridge.ts - git mv src/openclaw-plugin.ts → src/adapters/openclaw/plugin.ts - git mv src/opencode-plugin.ts → src/adapters/opencode/plugin.ts ## Internal import-path updates inside the moved files - ./session/db.js → ../../session/db.js (depth +2) - ./types.js → ../../types.js - ./adapters/X/Y.js → ./Y.js (now sibling) - ./adapters/types.js → ../types.js (now parent) - ./pi-mcp-bridge.js → ./mcp-bridge.js (renamed + sibling) ## Runtime path-resolution updates The plugins read sibling resources (hooks/, package.json, etc.) via `resolve(buildDir, "..")`. After the move buildDir lives 2 dirs deeper, so every `..` is now `../../..`: - resolve(buildDir, "..") → resolve(buildDir, "..", "..", "..") - resolve(buildDir, "..", "hooks", "core", "routing.mjs") → resolve(buildDir, "..", "..", "..", "hooks", "core", "routing.mjs") - (and similar for routing-block / tool-naming / auto-injection) For opencode/plugin.ts the version-from-package.json walker prepends `../../../package.json` to its search list (keeps the legacy `../package.json` and `./package.json` entries as fall-backs so unbundled or old-layout dev environments still resolve). ## Build-output paths in package.json tsc preserves src/ structure under build/, so: ./build/pi-extension.js → ./build/adapters/pi/extension.js ./build/openclaw-plugin.js → ./build/adapters/openclaw/plugin.js ./build/opencode-plugin.js → ./build/adapters/opencode/plugin.js Updated: - package.json: pi.extensions[0], openclaw.extensions[0], main, exports["."], exports["./plugin"], exports["./openclaw"] - .pi/extensions/context-mode/index.ts: re-export delegate path - .openclaw-plugin/index.ts: re-export delegate path + JSDoc ## Test-side updates - tests/pi-extension.test.ts: dynamic-import paths updated - tests/opencode-plugin.test.ts: dynamic-import paths updated - tests/plugins/openclaw.test.ts: dynamic-import paths updated - tests/core/cli.test.ts: 4 dynamic-import paths + 1 `readFileSync(src/openclaw-plugin.ts)` source-grep updated to the new location - src/adapters/detect.ts: comment-line ref updated - tests/adapters/detect.test.ts: comment-line ref updated ## Test plan - [x] npm run build clean - [x] npm run typecheck clean - [x] npm test 73 files, 2407 pass / 25 skipped / 0 fail - [x] npx vitest run tests/opencode-plugin.test.ts 33/33 pass (regression: marker test that needed package.json walker fix) - [x] npx vitest run tests/plugins/openclaw.test.ts 225/225 pass - [x] npx vitest run tests/pi-extension.test.ts 45/45 pass (incl. the wiring guard added in the previous commit) - [x] npx vitest run tests/core/cli.test.ts -t "openclaw-plugin.ts doctor/upgrade" passes against the new src/adapters/openclaw/plugin.ts location - [x] Manual sanity: every old root-level path (build/pi-extension.js, src/opencode-plugin.ts, etc.) is gone from the repo — grep confirms zero stale refs in src/ + tests/ + package.json + the .pi/.openclaw-plugin/ thin wrappers. Co-Authored-By: Ora Studio <noreply@oratelecom.net>
|
Pushed structure refactor on top of the bridge fix per Mert's feedback. Three atomic commits, each with its own message + test plan: 3243caf
5bb1b0f
4911c07
Test plan (full suite re-run after each commit):
No force-push — the 3 refactor commits sit cleanly on top of the bridge fix so each step is reviewable on its own. |
The structural refactor in 4911c07 (src/openclaw-plugin.ts → src/adapters/ openclaw/plugin.ts) moved the build output from build/openclaw-plugin.js to build/adapters/openclaw/plugin.js. Three scripts still pointed at the legacy path and broke on next-CI. ## OpenClaw E2E (failing on ubuntu-latest + macos-latest) scripts/test-openclaw-e2e.sh:34 join(process.cwd(), "build", "openclaw-plugin.js") The Phase 1 plugin-load check failed at "❌ build/openclaw-plugin.js exists" → exit 1. Updated to look for the new path first, fall back to the legacy one for transition safety: build/adapters/openclaw/plugin.js → fall back → build/openclaw-plugin.js Loaded-tag tracks which path actually resolved. ## OpenClaw global install (would have broken at user-install time) scripts/install-openclaw-plugin.sh:49 (auto-generated index.ts stub) Updated the absolute re-export path written into the generated stub plus the jiti cache-clear glob (now matches both `build-adapters-openclaw-plugin.*.cjs` and the legacy `build-openclaw-plugin.*.cjs` filenames). ## Bonus: security.js path was wrong post-refactor The opencode + openclaw plugins called `routing.initSecurity(buildDir)` where buildDir = build/adapters/<platform>/. That made initSecurity look for build/adapters/<platform>/security.js — which never exists. The security module lives at build/security.js (top-level). The fix-open fallback meant tests still passed but every plugin load emitted a spurious WARNING about deny-policy enforcement being off. - opencode/plugin.ts: pass `resolve(buildDir, "..", "..")` (= build/) - openclaw/plugin.ts: same Verified locally: `bash scripts/test-openclaw-e2e.sh` → 39/39 pass, no security warning. ## Test plan - [x] npm run build clean - [x] npm run typecheck clean - [x] npm test 73 files, 2405+ pass / 25 skipped / 0 fail (2 pre-existing flake worker-pool timeouts on kiro-hooks + insight-cors; both pass when run in isolation) - [x] bash scripts/test-openclaw-e2e.sh → "Results: 39 passed 0 warned 0 failed" + "✅ E2E test PASSED" Co-Authored-By: Ora Studio <noreply@oratelecom.net>
… refactor Independent PR review on mksglu#472 caught 3 stale path strings in `docs/adapters/openclaw.md` that the structural refactor (4911c07) missed: - src/openclaw-plugin.ts → src/adapters/openclaw/plugin.ts - src/openclaw/workspace-router.ts → src/adapters/openclaw/workspace-router.ts (×2) Doc-only — no code paths reference these strings. Co-Authored-By: Ora Studio <noreply@oratelecom.net>
CI fix + independent reviewCI fix (3b0bfd5)
Independent review (general-purpose subagent, no prior context)Empirical Phase A on the wiring guard test commit (b2baec9) was the most interesting:
Quality lenses:
Refactor sanity:
Verdict: APPROVE — bug reproduces empirically, fix asserts the regression behaviorally, refactor is import-clean across src/, tests/, scripts/, package.json, thin wrappers, and runtime resolve depths. |
…them (mksglu#426) (mksglu#472) * fix(pi): bridge MCP tools into pi.registerTool() so the LLM can call them (mksglu#426) Pi 0.73.x has no native MCP support — its README is explicit: > No MCP. Build CLI tools with READMEs (see Skills), or build an > extension that adds MCP support. Without a bridge inside the context-mode Pi extension, the routing block tells the LLM to call `ctx_execute` / `ctx_search` / etc. but those tools never enter Pi's tool list and the LLM cannot reach them. The reporter measured 18 sessions over 2 days: ~2,500 tokens of system-prompt overhead per window, 0 actual ctx_* calls, 447 events recorded but never retrieved. Net ROI on Pi was negative. This adds a stdio JSON-RPC client (`MCPStdioClient`) plus a thin bootstrap (`bootstrapMCPTools`) that: - spawns `server.bundle.mjs` as a long-lived MCP child, - performs the standard MCP handshake (initialize → notifications/initialized), - lists tools once via `tools/list`, and - registers each tool through `pi.registerTool({ name, label, description, parameters, execute })` so the LLM sees the canonical bare names (matching what hooks/core/tool-naming.mjs emits for Pi). Each Pi `execute()` callback forwards into the MCP child via `tools/call`. Errors are translated to `throw` (Pi's contract for "tool failed") so the LLM sees the MCP server's diagnostic text. Lifecycle: - Bridge bootstrap is fire-and-forget at extension load — the rest of the extension (session capture, hooks, slash commands) is not blocked by spawn / handshake latency. - `session_shutdown` terminates the child via SIGTERM. - A missing `server.bundle.mjs` or any spawn / handshake error is surfaced once on stderr, then the extension keeps running with only the existing hooks + commands. Defense-in-depth so the bridge can never break Pi sessions for users with broken installs. ## Why a JSON Schema parameters object instead of TypeBox MCP `tools/list` returns JSON Schema. Pi's parameter validator accepts JSON Schema directly (TypeBox just produces JSON Schema with extra Symbol metadata for type inference). Passing the schema through unchanged avoids a runtime translation pass and keeps the bridge a true thin layer over the MCP protocol — what works in Claude Code, Gemini CLI, and the other adapters now also works in Pi. ## No new runtime dependencies Pure `node:child_process` + `node:path`. The `@earendil-works/pi-*` packages are NOT pulled in as build deps — `pi` is typed structurally as `any` (matching the existing src/pi-extension.ts style) and the bridge only touches the documented `pi.registerTool()` shape. ## Tests Added two new `describe` blocks in `tests/pi-extension.test.ts`: 1. `MCPStdioClient` (5 tests) — wire-protocol contract pinned with fake stdio servers: id-matched responses, concurrent in-flight requests with out-of-order delivery, child-exit cancellation, timeout, non-JSON noise tolerance. 2. `bootstrapMCPTools` (2 integration tests) — spawn the real `start.mjs` MCP server, assert that the canonical ctx_* set (`ctx_execute`, `ctx_execute_file`, `ctx_search`, `ctx_index`, `ctx_batch_execute`, `ctx_fetch_and_index`, `ctx_doctor`, `ctx_stats`, `ctx_purge`) is registered, and round-trip `ctx_index` through `tools/call` to confirm execute() forwards args and returns text. ## Test plan - [x] `npm run build` - [x] `npm run typecheck` clean - [x] `npm test` — 73 files, 2406 pass / 25 skipped / 0 fail - [x] `npx vitest run tests/pi-extension.test.ts` — 44 pass (37 pre-existing + 7 new for the bridge) - [x] see-real-bug repro: `pi.registerTool` count = 0 in installed binary (`/home/$USER/.nvm/.../context-mode/build/pi-extension.js`) on `next` @ 1f70bee, plus Pi README's "No MCP" stance, plus the issue reporter's 18-session measurements — all three agree. - [-] Live LLM tool-call probe in Pi: blocked — free-tier Gemini quota was exhausted on every available key during the fix session. The integration test exercises the same code path (real MCP server + the Pi-facing registerTool surface), so the regression contract is enforced from CI. ## Out of scope - Removing the MCP server stanza from the Pi install README. Once this lands, the `~/.pi/agent/mcp.json` step is still harmless but no longer load-bearing. Cleanup left to a docs-only follow-up. - In-process refactor of server.ts handlers. The subprocess bridge is the same model used by every other adapter; a refactor that inlines the handlers is its own scope. Co-Authored-By: Ora Studio <noreply@oratelecom.net> * fix(pi): address self-review findings on the MCP bridge (mksglu#426) Three follow-up changes from the empirical self-review on PR mksglu#472: 1. **C1 HIGH — wiring not test-covered.** Phase A of the empirical review only failed because removing `src/pi-mcp-bridge.ts` produced an import error, not because the bug reproduced behaviorally. If a future refactor dropped the `bootstrapMCPTools(pi, …)` call from `src/pi-extension.ts` while keeping the bridge module intact, every existing bridge test stayed green and the bug silently re-entered. Fix: export `_mcpBridgeReady: Promise<void>` from `src/pi-extension.ts`. Bootstrap is still fire-and-forget (so spawn / handshake latency does not block session_start), but the promise gives tests a deterministic await point. Reset to a fresh promise on every `piExtension(pi)` call so multiple registrations in one process do not see a stale resolution. New test in `tests/pi-extension.test.ts` ("pi-extension.ts wiring (mksglu#426 regression guard)"): calls `registerPiExtension(api)`, awaits `_mcpBridgeReady`, asserts `api.registerTool.mock.calls` includes at least the canonical `ctx_execute` / `ctx_search` / `ctx_index` / `ctx_batch_execute` / `ctx_fetch_and_index` set. Verified red-on-revert: with the bridge module intact but the `bootstrapMCPTools(...)` call reverted to next, this test fails with `registeredNames: []`. Pre-fix it would have stayed green. 2. **C2 LOW — duplicated path resolution in the integration tests.** `tests/pi-extension.test.ts` had `path.dirname(...) + path.resolve(here, "..", "start.mjs")` recomputed in each `it()`. Lifted to a single `mcpEntry` const at the top of the `bootstrapMCPTools — registers every ctx_* tool with Pi` describe block, plus a shared `mcpEnv` for the `CONTEXT_MODE_DISABLE_VERSION_CHECK` override. One place to update if `start.mjs` ever moves. 3. **C3 LOW — dead `running` getter on MCPStdioClient.** Exported in the original commit but had zero callers anywhere in `src/` or `tests/`. Dropped — five lines, no behavioral impact. ## Test plan - [x] `npm run build` - [x] `npm run typecheck` — clean - [x] `npm test` — 73/73 files, 2407 pass / 25 skipped / 0 fail - [x] `npx vitest run tests/pi-extension.test.ts -t "MCP bridge|wiring"` — 8 pass - [x] Phase A re-validation: revert ONLY the wiring in `src/pi-extension.ts` (keep `src/pi-mcp-bridge.ts` intact), run the wiring test → fails with `registeredNames: []`. Restore and the test goes green again. Co-Authored-By: Ora Studio <noreply@oratelecom.net> * refactor(openclaw): consolidate src/openclaw/* into src/adapters/openclaw/ Pre-fix layout split OpenClaw across two locations: - src/adapters/openclaw/ — config, hooks, index, session-db (standard adapter pattern matching every other platform) - src/openclaw/ — mcp-tools, workspace-router (rogue location) The split predates the adapter pattern: workspace-router.ts was added first by Pedro Almeida (#aa8d93c), then mcp-tools.ts by the maintainer (#ff0a9a2 v1.0.107), while the adapter dir was bootstrapped later by the copilot-swe-agent (#5fd6a9e). Source-of-truth for every platform should live under src/adapters/<platform>/, so we move the two stragglers in. ## Changes - git mv src/openclaw/mcp-tools.ts → src/adapters/openclaw/mcp-tools.ts - git mv src/openclaw/workspace-router.ts → src/adapters/openclaw/workspace-router.ts - rmdir src/openclaw - src/openclaw-plugin.ts: 3 import-path updates - tests/plugins/openclaw.test.ts: 1 import-path update - tests/core/cli.test.ts: 1 readFileSync source-grep path update (the existing PR mksglu#183 path-traversal regression test reads the workspace-router source file directly to grep for safe-regex patterns; pin updated to the new location) ## Test plan - [x] npm run typecheck — clean - [x] npx vitest run tests/plugins/openclaw.test.ts tests/core/cli.test.ts → 225/225 pass - [x] npm test — 73 files, 2405+ pass / 25 skipped / 0 fail Co-Authored-By: Ora Studio <noreply@oratelecom.net> * refactor: flatten src/concurrency/runPool.ts → src/runPool.ts The src/concurrency/ directory held a single file. A whole directory for one module is structural noise — flatten it to src/runPool.ts. ## Changes - git mv src/concurrency/runPool.ts → src/runPool.ts - rmdir src/concurrency - src/server.ts: import path updated - tests/core/server.test.ts: import path updated ## Test plan - [x] npm run typecheck — clean - [x] npx vitest run tests/core/server.test.ts -t "runPool" — pass Co-Authored-By: Ora Studio <noreply@oratelecom.net> * refactor: relocate plugin entry files into src/adapters/<platform>/ Pre-fix layout had three platform plugin entry files at the src/ root: src/pi-extension.ts — Pi Coding Agent extension src/pi-mcp-bridge.ts — Pi MCP bridge (added in mksglu#426) src/openclaw-plugin.ts — OpenClaw gateway plugin src/opencode-plugin.ts — OpenCode plugin Every other platform follows the src/adapters/<name>/ pattern (config, hooks, index, …). The four root-level files were the last hold-outs: inconsistent layout, plus they made adapter discovery harder for new contributors. ## Changes (file moves) - git mv src/pi-extension.ts → src/adapters/pi/extension.ts - git mv src/pi-mcp-bridge.ts → src/adapters/pi/mcp-bridge.ts - git mv src/openclaw-plugin.ts → src/adapters/openclaw/plugin.ts - git mv src/opencode-plugin.ts → src/adapters/opencode/plugin.ts ## Internal import-path updates inside the moved files - ./session/db.js → ../../session/db.js (depth +2) - ./types.js → ../../types.js - ./adapters/X/Y.js → ./Y.js (now sibling) - ./adapters/types.js → ../types.js (now parent) - ./pi-mcp-bridge.js → ./mcp-bridge.js (renamed + sibling) ## Runtime path-resolution updates The plugins read sibling resources (hooks/, package.json, etc.) via `resolve(buildDir, "..")`. After the move buildDir lives 2 dirs deeper, so every `..` is now `../../..`: - resolve(buildDir, "..") → resolve(buildDir, "..", "..", "..") - resolve(buildDir, "..", "hooks", "core", "routing.mjs") → resolve(buildDir, "..", "..", "..", "hooks", "core", "routing.mjs") - (and similar for routing-block / tool-naming / auto-injection) For opencode/plugin.ts the version-from-package.json walker prepends `../../../package.json` to its search list (keeps the legacy `../package.json` and `./package.json` entries as fall-backs so unbundled or old-layout dev environments still resolve). ## Build-output paths in package.json tsc preserves src/ structure under build/, so: ./build/pi-extension.js → ./build/adapters/pi/extension.js ./build/openclaw-plugin.js → ./build/adapters/openclaw/plugin.js ./build/opencode-plugin.js → ./build/adapters/opencode/plugin.js Updated: - package.json: pi.extensions[0], openclaw.extensions[0], main, exports["."], exports["./plugin"], exports["./openclaw"] - .pi/extensions/context-mode/index.ts: re-export delegate path - .openclaw-plugin/index.ts: re-export delegate path + JSDoc ## Test-side updates - tests/pi-extension.test.ts: dynamic-import paths updated - tests/opencode-plugin.test.ts: dynamic-import paths updated - tests/plugins/openclaw.test.ts: dynamic-import paths updated - tests/core/cli.test.ts: 4 dynamic-import paths + 1 `readFileSync(src/openclaw-plugin.ts)` source-grep updated to the new location - src/adapters/detect.ts: comment-line ref updated - tests/adapters/detect.test.ts: comment-line ref updated ## Test plan - [x] npm run build clean - [x] npm run typecheck clean - [x] npm test 73 files, 2407 pass / 25 skipped / 0 fail - [x] npx vitest run tests/opencode-plugin.test.ts 33/33 pass (regression: marker test that needed package.json walker fix) - [x] npx vitest run tests/plugins/openclaw.test.ts 225/225 pass - [x] npx vitest run tests/pi-extension.test.ts 45/45 pass (incl. the wiring guard added in the previous commit) - [x] npx vitest run tests/core/cli.test.ts -t "openclaw-plugin.ts doctor/upgrade" passes against the new src/adapters/openclaw/plugin.ts location - [x] Manual sanity: every old root-level path (build/pi-extension.js, src/opencode-plugin.ts, etc.) is gone from the repo — grep confirms zero stale refs in src/ + tests/ + package.json + the .pi/.openclaw-plugin/ thin wrappers. Co-Authored-By: Ora Studio <noreply@oratelecom.net> * fix(ci): update E2E + install scripts for relocated openclaw plugin path The structural refactor in 4911c07 (src/openclaw-plugin.ts → src/adapters/ openclaw/plugin.ts) moved the build output from build/openclaw-plugin.js to build/adapters/openclaw/plugin.js. Three scripts still pointed at the legacy path and broke on next-CI. ## OpenClaw E2E (failing on ubuntu-latest + macos-latest) scripts/test-openclaw-e2e.sh:34 join(process.cwd(), "build", "openclaw-plugin.js") The Phase 1 plugin-load check failed at "❌ build/openclaw-plugin.js exists" → exit 1. Updated to look for the new path first, fall back to the legacy one for transition safety: build/adapters/openclaw/plugin.js → fall back → build/openclaw-plugin.js Loaded-tag tracks which path actually resolved. ## OpenClaw global install (would have broken at user-install time) scripts/install-openclaw-plugin.sh:49 (auto-generated index.ts stub) Updated the absolute re-export path written into the generated stub plus the jiti cache-clear glob (now matches both `build-adapters-openclaw-plugin.*.cjs` and the legacy `build-openclaw-plugin.*.cjs` filenames). ## Bonus: security.js path was wrong post-refactor The opencode + openclaw plugins called `routing.initSecurity(buildDir)` where buildDir = build/adapters/<platform>/. That made initSecurity look for build/adapters/<platform>/security.js — which never exists. The security module lives at build/security.js (top-level). The fix-open fallback meant tests still passed but every plugin load emitted a spurious WARNING about deny-policy enforcement being off. - opencode/plugin.ts: pass `resolve(buildDir, "..", "..")` (= build/) - openclaw/plugin.ts: same Verified locally: `bash scripts/test-openclaw-e2e.sh` → 39/39 pass, no security warning. ## Test plan - [x] npm run build clean - [x] npm run typecheck clean - [x] npm test 73 files, 2405+ pass / 25 skipped / 0 fail (2 pre-existing flake worker-pool timeouts on kiro-hooks + insight-cors; both pass when run in isolation) - [x] bash scripts/test-openclaw-e2e.sh → "Results: 39 passed 0 warned 0 failed" + "✅ E2E test PASSED" Co-Authored-By: Ora Studio <noreply@oratelecom.net> * docs(openclaw): update Key Files paths after src/adapters/<platform>/ refactor Independent PR review on mksglu#472 caught 3 stale path strings in `docs/adapters/openclaw.md` that the structural refactor (4911c07) missed: - src/openclaw-plugin.ts → src/adapters/openclaw/plugin.ts - src/openclaw/workspace-router.ts → src/adapters/openclaw/workspace-router.ts (×2) Doc-only — no code paths reference these strings. Co-Authored-By: Ora Studio <noreply@oratelecom.net> --------- Co-authored-by: Ora Studio <noreply@oratelecom.net>
|
After testing I found this issue. Pi subagents may miss Each subagent starts a fresh This does not look like a frontmatter allowlist issue; the agents already include the ctx tool names. context-mode likely needs to await bridge setup, or block first agent start until |
* fix(pi): await MCP bridge bootstrap in before_agent_start Pi subagents (`pi --mode json -p --no-session`) spawn a fresh process that loads the context-mode extension and immediately fires `before_agent_start` to dispatch the LLM call. The MCP bridge bootstrap (spawn server.bundle.mjs → initialize → tools/list → pi.registerTool × N) was fire-and-forget via `_mcpBridgeReady`, so the LLM call went out with an empty ctx_* tool registry and the routing block (~2.5K tokens) became dead weight — the LLM was told to call `ctx_execute` / `ctx_search` / etc. but Pi had not yet registered them. Awaiting `_mcpBridgeReady` at the top of the `before_agent_start` handler closes the race: by the time the handler resolves, the bridge has settled (success or failure — failures are still logged to stderr but never propagated, matching the original best-effort contract) and the registry contains the ctx_* tools. Reported in #472 (comment 4412197109). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: raise hookTimeout to 30s to absorb Windows better-sqlite3 cleanup flake The default 10s vitest hookTimeout is exhausted on Windows runners by files whose afterAll loops over many better-sqlite3 handles — tests/session/session-pipeline.test.ts is the canonical example. Local runs finish in ~500ms but Windows fork-pool contention plus native addon cleanup can stretch past 10s, surfacing as FAIL tests/session/session-pipeline.test.ts Error: Hook timed out in 10000ms. Match the 30s testTimeout already in this config so the cleanup window matches the work window — same envelope better-sqlite3 needs for tests themselves. No change to local-dev wall time (only fires when a hook exceeds 10s, which is a flake-level event). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
round-4) Two #472 round-3 resilience tests pin behaviors that depend on POSIX signal semantics: - "escalates to SIGKILL when SIGTERM is ignored" relies on the fake server's process.on('SIGTERM', ...) handler keeping the child alive past the bridge's first kill so the 5s SIGKILL fallback fires. On Windows there are no POSIX signals: child.kill('SIGTERM') and child.kill('SIGKILL') both invoke TerminateProcess, which is unblockable and dies on the first call. The fake handler never runs, so the SIGKILL-escalation contract this test pins is fundamentally unobservable on win32. - "session_shutdown awaits bridge bootstrap so child is not orphaned" reads ChildProcess.killed/exitCode/signalCode after a win32 TerminateProcess race. The transitions on Windows for a tsx-spawned child differ from POSIX in CI in a way that produces a false-negative for the dead-child assertion, even when the underlying `await _mcpBridgeReady` contract is honored. Skip both on win32 with `it.skipIf`. POSIX coverage on ubuntu-latest and macos-latest exercises the resilience guarantee.
…APPDATA fallback Two correctness bugs from the original commit (1dba9b3): 1. PRICE_PER_TOKEN module-load freeze ------------------------------------- Original: export const PRICE_PER_TOKEN = resolvePricePerToken(); Pi sets PI_CONTEXT_MODE_PRICE_OUTPUT_PER_TOKEN at bridge spawn time — AFTER the MCP server bundle has already been imported. The module-load const captures process.env at import (when the var is unset) and freezes to the $15/1M Opus fallback for the life of the process. The PR body advertises env-driven dynamic pricing that silently does not work; a Pi host running on a cheaper model overstates savings until the user restarts the MCP server. Fix: convert to a function pricePerToken() that re-reads the env on every call. Five call sites updated (server.ts persistStats x2, analytics.ts renderCostExample, tokensToUsd, plus the import). Kept OPUS_INPUT_PRICE_PER_TOKEN as a deprecated literal alias so any third-party consumer importing the old name still resolves to the same fallback rate — preserves the P1.1 single-source-of-truth invariant from PR mksglu#401 architect review. 2. Pi-on-Windows silent PI_CONFIG_DIR rescue regression ------------------------------------------------------- Original (mcp-bridge.ts:467-473): const home = childEnv.HOME ?? childEnv.USERPROFILE ?? childEnv.HOMEPATH; if (home) { const piConfig = join(home, ".pi"); if (existsSync(piConfig)) { childEnv.PI_CONFIG_DIR = piConfig; } } Pi on Windows installs to %APPDATA%\.pi (XDG-on-Windows pattern), NOT %USERPROFILE%\.pi. The HOME-rooted probe is false on every Windows Pi install, PI_CONFIG_DIR never gets set, detectPlatform() falls through to the Claude Code default, and Pi sessions write into ~/.claude/ instead of Pi's own dir — silently. CI's windows-latest green status is misleading because there is no Pi-on-Windows assertion in the test suite. Fix: probe both %USERPROFILE%\.pi and %APPDATA%\.pi (and trust PI_CONFIG_DIR verbatim when the parent process already set it — Pi's launcher may pin a non-default location for CI / corporate setups). The POSIX path is unchanged (only HOME is set; APPDATA is undefined). Archaeology: - 1dba9b3 — original PR commit that introduced both bugs. - 4ef1c78 (mksglu#561) — foreign env scrub regression the original PR was trying to rescue; without the cross-OS fallback, Windows Pi users remain broken even with the rescue logic in place. - b1dd553 (mksglu#472) — bundle SIGKILL precedent for cross-OS robustness. - 401 — pricing constant dedup invariant preserved via the deprecated alias.
What
Fixes #426.
Pi 0.73.x has no native MCP support. From pi-mono README:
Without a bridge in the context-mode Pi extension, the routing block tells the LLM to call
ctx_execute/ctx_search/ etc. but those tools never enter Pi's tool list and the LLM cannot reach them. The reporter measured 18 sessions over 2 days: ~2,500 tokens of system-prompt overhead per window, 0 actual ctx_ calls*, 447 events recorded but never retrieved. ROI on Pi was net negative.Fix
A new
src/pi-mcp-bridge.tsmodule:server.bundle.mjsas a long-lived MCP child via stdio.initialize→notifications/initialized).tools/listonce, then registers each returned tool throughpi.registerTool({ name, label, description, parameters, execute }).execute()forwards into the child viatools/call. Errors are thrown — Pi's contract for "tool failed" — so the LLM sees the MCP server's diagnostic text.src/pi-extension.tsadds the bootstrap call (fire-and-forget, so spawn / handshake latency does NOT block session start) and asession_shutdowncleanup that SIGTERMs the child.Why JSON Schema instead of TypeBox
MCP
tools/listreturns JSON Schema. Pi's parameter validator accepts JSON Schema directly — TypeBox just adds Symbol metadata to JSON Schema for type inference. Passing the schema through unchanged avoids a runtime translation pass and keeps the bridge a true thin layer over the MCP protocol. Same code path Claude Code, Gemini CLI and the other adapters use.No new runtime dependencies
Pure
node:child_process+node:path. The@earendil-works/pi-*packages are NOT pulled in as build deps —piis typed structurally (matching the existingsrc/pi-extension.tsstyle ofpi: any) and the bridge only touches the documentedpi.registerTool()shape.Defense-in-depth
A missing
server.bundle.mjsor any spawn / handshake error is surfaced once on stderr, then the extension keeps running with the existing hooks + slash commands. The bridge cannot break a Pi session for users with broken installs.Coverage added (
tests/pi-extension.test.ts)Two new
describeblocks (no new test files — added to the existing domain file per CONTRIBUTING):MCPStdioClient— 5 unit tests pinning the wire protocol with fake stdio servers:\n-delimited JSONbootstrapMCPTools— 2 integration tests against the real MCP server (start.mjs):ctx_*set is registered:ctx_execute,ctx_execute_file,ctx_search,ctx_index,ctx_batch_execute,ctx_fetch_and_index,ctx_doctor,ctx_stats,ctx_purgectx_indexthroughtools/call(server returns "Indexed N sections … from: pi-bridge-smoke" — pin proves args make it across the bridge)Test plan
npm run buildnpm run typecheck— cleannpm test— 73 files, 2406 pass / 25 skipped / 0 failnpx vitest run tests/pi-extension.test.ts— 44 pass (37 pre-existing + 7 new for the bridge)next@1f70bee:pi.registerToolcount = 0 in installedbuild/pi-extension.js(onlypi.registerCommand("ctx-stats")andpi.registerCommand("ctx-doctor"))~/.pi/agent/mcp.jsonfor context-mode purposesregisterToolsurface).Out of scope
~/.pi/agent/mcp.jsonstep in the Pi install README. Once this lands the step is harmless but no longer load-bearing — cleanup deferred to a docs-only follow-up.server.tshandlers. The subprocess bridge matches every other adapter; an in-process inline is its own scope.