Skip to content

fix(pi): bridge MCP tools into pi.registerTool() so the LLM can call them (#426)#472

Merged
mksglu merged 7 commits into
mksglu:nextfrom
ousamabenyounes:fix/issue-426-pi-register-tools
May 7, 2026
Merged

fix(pi): bridge MCP tools into pi.registerTool() so the LLM can call them (#426)#472
mksglu merged 7 commits into
mksglu:nextfrom
ousamabenyounes:fix/issue-426-pi-register-tools

Conversation

@ousamabenyounes

Copy link
Copy Markdown
Collaborator

What

Fixes #426.

Pi 0.73.x has no native MCP support. From pi-mono README:

No MCP. Build CLI tools with READMEs (see Skills), or build an extension that adds MCP support.

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.ts module:

  • Spawns server.bundle.mjs as a long-lived MCP child via stdio.
  • Performs the standard MCP handshake (initializenotifications/initialized).
  • Calls tools/list once, then registers each returned tool through pi.registerTool({ name, label, description, parameters, execute }).
  • execute() forwards into the child via tools/call. Errors are thrown — Pi's contract for "tool failed" — so the LLM sees the MCP server's diagnostic text.

src/pi-extension.ts adds the bootstrap call (fire-and-forget, so spawn / handshake latency does NOT block session start) and a session_shutdown cleanup that SIGTERMs the child.

Why JSON Schema instead of TypeBox

MCP tools/list returns 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 — pi is typed structurally (matching the existing src/pi-extension.ts style of pi: any) and the bridge only touches the documented pi.registerTool() shape.

Defense-in-depth

A missing server.bundle.mjs or 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 describe blocks (no new test files — added to the existing domain file per CONTRIBUTING):

  1. MCPStdioClient — 5 unit tests pinning the wire protocol with fake stdio servers:

    • Id-matched responses over \n-delimited JSON
    • Concurrent in-flight requests with out-of-order delivery (id-map dispatch)
    • Child-exit cancellation rejects every pending request
    • Timeout instead of hanging on a silent server
    • Non-JSON noise tolerance (banner lines, garbage)
  2. bootstrapMCPTools — 2 integration tests against the real MCP server (start.mjs):

    • Asserts the canonical 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_purge
    • Round-trips ctx_index through tools/call (server returns "Indexed N sections … from: pi-bridge-smoke" — pin proves args make it across the bridge)

Test plan

  • npm run build
  • npm run typecheck — clean
  • npm test — 73 files, 2406 pass / 25 skipped / 0 fail
  • npx vitest run tests/pi-extension.test.ts — 44 pass (37 pre-existing + 7 new for the bridge)
  • see-real-bug repro — three independent confirmations on next @ 1f70bee:
    1. Static: pi.registerTool count = 0 in installed build/pi-extension.js (only pi.registerCommand("ctx-stats") and pi.registerCommand("ctx-doctor"))
    2. Pi documentation: explicit "No MCP" stance — Pi 0.73.x ignores ~/.pi/agent/mcp.json for context-mode purposes
    3. Issue reporter's 18-session measurements: 0 ctx_* calls, 447 events captured but never retrieved
  • [-] Live Pi LLM probe: free-tier Gemini quota was exhausted on every available key during the fix session, so the LLM-side regression contract is enforced via the integration test (which spawns the same MCP server Pi would, against the same Pi-facing registerTool surface).

Out of scope

  • The ~/.pi/agent/mcp.json step 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.
  • In-process refactor of server.ts handlers. The subprocess bridge matches every other adapter; an in-process inline is its own scope.

…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>
@ousamabenyounes

Copy link
Copy Markdown
Collaborator Author

Self-review (mode: empirical) — PR #472

Head SHA: 1734c787 · Base: next

Phase A (bug reproduces without fix): PASS — reverted src/pi-extension.ts and removed src/pi-mcp-bridge.ts, the 7 new tests all fail. Caveat: the failure mode is import-not-found, not a behavior assertion. See C1 below.

Phase B (test passes with fix): PASS — 7/7 green on head.

Quality lenses:

  • C1 Coverage strength: 1 HIGH
  • C2 Copy-paste: 1 LOW
  • C3 Dead code: 1 LOW
  • C4 Clarity: PASS
  • C5 Performance: skipped (spawn-once-per-session, no hot path)
  • C6 Security: PASS (env passthrough to same-package child, trust boundary unchanged)

Findings

[HIGH] No test asserts the wiring in src/pi-extension.ts actually runs at load time. Phase A passes only because removing src/pi-mcp-bridge.ts produces an import error, not because the bug reproduces behaviorally. If a future refactor drops the bootstrapMCPTools(pi, serverBundle).then(...) line from src/pi-extension.ts while keeping the bridge module intact, every one of the 7 new bridge tests stays green and the bug silently re-enters. Of the 36 existing registerPiExtension(api) tests in tests/pi-extension.test.ts, none assert that any ctx_* tool is registered through pi.registerTool() after the extension's default export runs.

Fix: add a test that calls await registerPiExtension(api), waits for the fire-and-forget bootstrap to settle (export _mcpBridgeReady as a Promise from pi-extension.ts for tests), then asserts api.registerTool.mock.calls includes at least one entry whose name matches /^ctx_/. The mock already tracks calls — api.registerTool is vi.fn() in createMockPiApi().

[LOW] Dead getter running on MCPStdioClient. src/pi-mcp-bridge.ts:189 exports get running(): boolean but no caller exists in src/ or tests/. Either drop it or wire it into one of the unit tests as a sanity probe.

[LOW] Path resolution duplicated across the two integration tests. tests/pi-extension.test.ts:887-892 and :931-934 both compute mcpEntry = path.resolve(here, "..", "start.mjs") from scratch. Lift to a const at the top of the bootstrapMCPTools — registers every ctx_* tool with Pi describe block.

Verdict

REQUEST_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>
@ousamabenyounes

Copy link
Copy Markdown
Collaborator Author

Self-review findings addressed in b2baec9:

[HIGH C1] Wiring test added. Exported _mcpBridgeReady: Promise<void> from src/pi-extension.ts; new test pi-extension.ts wiring (#426 regression guard) calls registerPiExtension(api), awaits _mcpBridgeReady, asserts api.registerTool.mock.calls includes at least ctx_execute / ctx_search / ctx_index / ctx_batch_execute / ctx_fetch_and_index. Verified red-on-revert: revert ONLY the bootstrapMCPTools(pi, ...) call (keep the bridge module intact) → registeredNames: [], test fails. Restore → green.

[LOW C2] Path resolution lifted. mcpEntry + mcpEnv consts at top of the bootstrapMCPTools — ... describe, used by both integration tests.

[LOW C3] Dead running getter dropped. 5-line removal on MCPStdioClient.

Suite: 73 files, 2407 pass / 25 skipped / 0 fail. Bridge + wiring tests: 8/8.

ousamabenyounes and others added 3 commits May 7, 2026 22:35
…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>
@ousamabenyounes

Copy link
Copy Markdown
Collaborator Author

Pushed structure refactor on top of the bridge fix per Mert's feedback. Three atomic commits, each with its own message + test plan:

3243caf refactor(openclaw): consolidate src/openclaw/* into src/adapters/openclaw/

  • git mv src/openclaw/{mcp-tools,workspace-router}.ts → src/adapters/openclaw/
  • Updates the 3 imports in the openclaw plugin + 2 test refs
  • Deletes the now-empty src/openclaw/ dir

5bb1b0f refactor: flatten src/concurrency/runPool.ts → src/runPool.ts

  • One-file directory was structural noise; flattened
  • 2 import-path updates (server.ts, server.test.ts)

4911c07 refactor: relocate plugin entry files into src/adapters/<platform>/

  • src/pi-extension.ts → src/adapters/pi/extension.ts
  • src/pi-mcp-bridge.ts → src/adapters/pi/mcp-bridge.ts
  • src/openclaw-plugin.ts → src/adapters/openclaw/plugin.ts
  • src/opencode-plugin.ts → src/adapters/opencode/plugin.ts
  • Internal import paths updated (depth +2)
  • Runtime path resolution updated (resolve(buildDir, "..")resolve(buildDir, "..", "..", ".."))
  • package.json: pi.extensions, openclaw.extensions, main, exports.* paths follow the build-output relocation
  • .pi/extensions/context-mode/index.ts + .openclaw-plugin/index.ts thin wrappers updated
  • 4 test-side dynamic-import path updates + 1 source-grep path update in tests/core/cli.test.ts

Test plan (full suite re-run after each commit):

  • npm run typecheck — clean
  • npm test — 73/73 files, 2407 pass / 25 skipped / 0 fail
  • No stale src/pi-extension, src/openclaw-plugin, src/opencode-plugin, build/pi-extension.js, build/openclaw-plugin.js, build/opencode-plugin.js refs anywhere in src/ tests/ package.json or the thin-wrapper dirs (grep clean).

No force-push — the 3 refactor commits sit cleanly on top of the bridge fix so each step is reviewable on its own.

ousamabenyounes and others added 2 commits May 7, 2026 22:57
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>
@ousamabenyounes

Copy link
Copy Markdown
Collaborator Author

CI fix + independent review

CI fix (3b0bfd5)

  • OpenClaw E2E was failing because scripts/test-openclaw-e2e.sh and scripts/install-openclaw-plugin.sh still pointed at build/openclaw-plugin.js — the structural refactor moved the build output to build/adapters/openclaw/plugin.js.
  • Updated both scripts (with a fall-back to the legacy path for transition safety).
  • Bonus: the opencode + openclaw plugins were calling routing.initSecurity(buildDir) with the adapter sub-dir instead of the build root, so every plugin load emitted a noisy "security.js not found" WARNING. Fixed — now passes resolve(buildDir, "..", "..").
  • Local: bash scripts/test-openclaw-e2e.sh → 39/39 pass.
  • Latest CI run: OpenClaw E2E success ✅ (1m10s on 3b0bfd5).

Independent review (general-purpose subagent, no prior context)

Empirical Phase A on the wiring guard test commit (b2baec9) was the most interesting:

  • Reverted ONLY the bootstrapMCPTools(pi, serverBundle).then(...) call line in src/adapters/pi/extension.ts while keeping src/adapters/pi/mcp-bridge.ts and the import statement intact.
  • Result: exactly 1 test fails — the wiring guard, with registeredNames: [] instead of arrayContaining(["ctx_execute", ...]). The other 7 bridge tests stay green. Confirms the wiring guard catches the precise regression class no other test catches.

Quality lenses:

  • C1 Coverage strength: PASS — every prod path is exercised behaviorally; arrayContaining is additive-friendly while still pinning the canonical names.
  • C2 / C3 / C4 / C6: PASS.
  • C5 Performance: SKIPPED (spawn-once-per-session, no hot path).

Refactor sanity:

  • Stale path strings, package.json paths, thin wrappers, runtime resolve depths, initSecurity arg: all PASS.
  • One LOW finding — 3 stale paths in docs/adapters/openclaw.md. Fixed in fa60719.
  • One INFO — let _mcpBridgeReady export binding semantics; existing docstring already covers it.

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.

@mksglu mksglu merged commit c805410 into mksglu:next May 7, 2026
5 checks passed
ousamabenyounes added a commit to ousamabenyounes/context-mode that referenced this pull request May 8, 2026
…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>
@mystilleef

Copy link
Copy Markdown
Contributor

After testing I found this issue. Pi subagents may miss ctx_* tools due to an async registration race.

Each subagent starts a fresh pi --mode json -p --no-session process. context-mode’s Pi adapter registers ctx_* tools through the MCP bridge, but that bootstrap runs fire-and-forget via _mcpBridgeReady. The child process can start the prompt before registration finishes, so the initial tool registry may not include ctx_execute, ctx_batch_execute, etc.

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 _mcpBridgeReady settles.

mksglu pushed a commit that referenced this pull request May 9, 2026
* 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>
mksglu added a commit that referenced this pull request May 10, 2026
 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.
mksglu added a commit to jalexandre0/context-mode that referenced this pull request May 31, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants