HOMELAB-1769: feat(mcp-bridge): version-control the LD1.1-validated pi<->sandbox MCP seam #50

Merged
aaron merged 2 commits from plane/HOMELAB-1769-mcp-bridge into main 2026-09-30 21:30:11 +00:00
Owner

What

Lands the pi MCP-bridge extension in Homelab/pi-extensions as the new mcp-bridge/ subdirectory. The extension exposes an MCP server's tools as native typed pi tools (mcp_<name>); against the in-cluster agent-sandbox MCP server that is the 8-tool sandbox lifecycle: create_sandbox, get_sandbox_status, execute_command, upload_file, download_file, list_files, file_exists, delete_sandbox.

The source is a byte-identical copy of the live install at ~/.pi/agent/extensions/pi-mcp-bridge/ (index.ts, schema.ts, package.json, tsconfig.json, package-lock.json), verified with diff against the spike snapshot. No refactor, no behaviour change, no node_modules committed.

Why

The bridge is the durable seam validated by the LD1.1 verdict. It currently lives only under ~/.pi — functional but unversioned, unreviewable, and not fleet-distributable. Per the source-of-truth stack it belongs in Forgejo with pinned deps, house docs, CI, and the normal review/landing flow.

Spike evidence

.rpiv/artifacts/research/homelab-1669/bridge-proof-2026-09-30/ in the homelab repo: summary.md (verdict BRIDGE-WORKS), lifecycle-transcript.jsonl, lifecycle-report.md, lifecycle-stderr.log, tools-list-raw.json. Result: 8 tools registered, 11 mcp_* calls, 0 failures, 0 schema rejections, 0 leftover claims, ~2m15s. Design: .rpiv/artifacts/research/homelab-1747/pi-mcp-bridge-design.md.

Files

File
mcp-bridge/index.ts, schema.ts, package.json, tsconfig.json, package-lock.json extension source + pinned @modelcontextprotocol/sdk 1.31.0
mcp-bridge/README.md authoritative page: config, tools, the four normalisations, ownership/reconnect semantics, limitations, troubleshooting
docs/mcp-bridge.md repo-convention overview page linking to the in-tree README
README.md extensions-table row, in-tree-docs list, config table row, local-npm-deps bullet
.forgejo/workflows/ci.yaml new mcp-bridge job (npm ci + npm run typecheck)

The four server-side normalisations (documented, not code changes)

These are agent-sandbox MCP server behaviours, not bridge behaviour — the bridge forwards arguments verbatim and rewrites nothing. They are documented because the server's own tool descriptions mislead on all four (spike spike-run-2026-09-30/summary.md §3.1–3.4):

  1. File paths are relative to SANDBOX_BASE_DIR (/workspace). Upstream lstrip("/")s then rejoins, so /workspace/x lands at /workspace/workspace/x. All four file tools agree with each other while execute_command cannot see the file — a silent-failure trap.
  2. list_files root is path="". path="." resolves to /list/.. and fails.
  3. execute_command is argv-only (shlex.split + shell=False): no pipes/;/&&/globs. Shell syntax needs an explicit sh -c '…'. Two distinct timeouts are documented: the tool's timeout (default 60 s, server-side) vs the bridge's MCP request timeout (default 600 000 ms).
  4. The post-delete "gone" signal is a tool error, not the advertised SandboxNotFound status: Sandbox claim '…' is not found in namespace '…'. The bridge surfaces it as a thrown pi tool error, which is the correct seam — callers must match the text.

Ownership, timeout, reconnect (all re-verified against the source)

  • MCP session id is the sandbox ownership key (ensure_session_owns server-side); a session can only delete what it created; there is no admin/reaper tool, so an orphan lives until its shutdown_after_seconds TTL. Therefore one client per pi process: created in session_start, closed in session_shutdown.
  • PI_MCP_BRIDGE_TIMEOUT_MS defaults to 600 000 ms and is passed to client.callTool per call (re-read at call time, unlike URL/token which are read at connect time) because a cold-pool claim can block for minutes and the SDK default is 60 s.
  • ensureConnection() is single-flight and only opens a client when none is held. The documented hazard holds: a reconnect is a new MCP session and orphans everything the previous session owned until TTL.
    • One honest deviation found while writing the README: bridge-proof-2026-09-30/summary.md §3.5 claims the lazy reconnect also covers "port-forward started after pi". It does not — if the server is unreachable at session_start, registerMcpTools never runs, so no mcp_* tools exist in that session and there is nothing to call lazily. The code's lazy reconnect only covers a client that died after registration. The README documents the code's actual behaviour; the spike summary is the stale document. The bridge also does not emit a dedicated reconnect warning — each open logs connected to <url> (session <8 hex>…), so a second such line is the tell that prior claims are now orphans.

list_files 403 disposition (minor finding)

With no sandbox claimed in the session, list_files returns isError 403: the upstream path resolves the session's sandbox by a cluster-scope sandboxclaims LIST while the MCP service account holds a namespaced Role (infra-core/core/manifests/agent-sandbox-mcp/rbac.yaml, deliberately not a ClusterRole — see its own rationale comment). In-session calls never hit it; the lifecycle run did not.

Disposition: documented in mcp-bridge/README.md under "Known limitations", no RBAC or manifest change in this PR. Widening the Role is a cluster-config decision owned by sandbox-provisioning-contracts.md §5 in the infra-core repo, and that file explicitly warns against widening without such a decision. A follow-up ticket for the RBAC/upstream-path decision is the right home for a fix.

Verify

  • npm ci in mcp-bridge/ (97 packages, pinned lockfile) and npm run typecheck / npx tsc --noEmit clean locally.
  • Extension loads from the landed tree path: AGENT_SANDBOX_MCP_URL=http://127.0.0.1:19999/mcp pi -p --no-session -ne -e ./mcp-bridge/index.ts … produced [pi-mcp-bridge] connect failed for http://127.0.0.1:19999/mcp: fetch failed and the model still answered — i.e. the copy resolves ./schema.ts and @modelcontextprotocol/sdk from its own node_modules and degrades gracefully, as documented.
  • Post-merge (needs the merged tree): fresh install via the extension install path (git clone/symlink into ~/.pi/agent/extensions) and re-run the LD1.1 lifecycle proof against the agent-sandbox MCP server (create → status → argv exec → sh -c exec → upload/download hash agreement → list/exists → delete → gone-signal), asserting 0 leftover claims.
  • CI: mcp-bridge job (npm ci + typecheck) plus PR-title gate.

Out of scope / follow-ups

  • No test suite. The bridge's proof is the live lifecycle run; the only pure logic is the schema.ts sanitizer, which the pinned server never even exercises (its schemas are flat). A vitest suite for the sanitizer + a fake MCP transport would be a worthwhile follow-up ticket, not this landing.
  • The list_files 403 RBAC decision (above), and the design-noted lazy mcp_connect activation if the bridge ever ships always-on (prompt-bloat risk #4).

Reviewer note: this is a version-control landing of already-proved code, so the review gold is in the README's claims — every claim in it was checked against index.ts/schema.ts and the spike evidence.

## What Lands the pi MCP-bridge extension in `Homelab/pi-extensions` as the new `mcp-bridge/` subdirectory. The extension exposes an MCP server's tools as native typed pi tools (`mcp_<name>`); against the in-cluster agent-sandbox MCP server that is the 8-tool sandbox lifecycle: `create_sandbox`, `get_sandbox_status`, `execute_command`, `upload_file`, `download_file`, `list_files`, `file_exists`, `delete_sandbox`. The source is a **byte-identical copy** of the live install at `~/.pi/agent/extensions/pi-mcp-bridge/` (`index.ts`, `schema.ts`, `package.json`, `tsconfig.json`, `package-lock.json`), verified with `diff` against the spike snapshot. No refactor, no behaviour change, no `node_modules` committed. ## Why The bridge is the durable seam validated by the LD1.1 verdict. It currently lives only under `~/.pi` — functional but unversioned, unreviewable, and not fleet-distributable. Per the source-of-truth stack it belongs in Forgejo with pinned deps, house docs, CI, and the normal review/landing flow. ## Spike evidence `.rpiv/artifacts/research/homelab-1669/bridge-proof-2026-09-30/` in the `homelab` repo: `summary.md` (verdict `BRIDGE-WORKS`), `lifecycle-transcript.jsonl`, `lifecycle-report.md`, `lifecycle-stderr.log`, `tools-list-raw.json`. Result: 8 tools registered, 11 `mcp_*` calls, 0 failures, 0 schema rejections, 0 leftover claims, ~2m15s. Design: `.rpiv/artifacts/research/homelab-1747/pi-mcp-bridge-design.md`. ## Files | File | | |---|---| | `mcp-bridge/index.ts`, `schema.ts`, `package.json`, `tsconfig.json`, `package-lock.json` | extension source + pinned `@modelcontextprotocol/sdk` 1.31.0 | | `mcp-bridge/README.md` | authoritative page: config, tools, the four normalisations, ownership/reconnect semantics, limitations, troubleshooting | | `docs/mcp-bridge.md` | repo-convention overview page linking to the in-tree README | | `README.md` | extensions-table row, in-tree-docs list, config table row, local-npm-deps bullet | | `.forgejo/workflows/ci.yaml` | new `mcp-bridge` job (`npm ci` + `npm run typecheck`) | ## The four server-side normalisations (documented, not code changes) These are **agent-sandbox MCP server** behaviours, not bridge behaviour — the bridge forwards arguments verbatim and rewrites nothing. They are documented because the server's own tool descriptions mislead on all four (spike `spike-run-2026-09-30/summary.md` §3.1–3.4): 1. **File paths are relative to `SANDBOX_BASE_DIR` (`/workspace`).** Upstream `lstrip("/")`s then rejoins, so `/workspace/x` lands at `/workspace/workspace/x`. All four file tools agree with each other while `execute_command` cannot see the file — a silent-failure trap. 2. **`list_files` root is `path=""`.** `path="."` resolves to `/list/..` and fails. 3. **`execute_command` is argv-only** (`shlex.split` + `shell=False`): no pipes/`;`/`&&`/globs. Shell syntax needs an explicit `sh -c '…'`. Two distinct timeouts are documented: the tool's `timeout` (default 60 s, server-side) vs the bridge's MCP request timeout (default 600 000 ms). 4. **The post-delete "gone" signal is a tool error**, not the advertised `SandboxNotFound` status: `Sandbox claim '…' is not found in namespace '…'`. The bridge surfaces it as a thrown pi tool error, which is the correct seam — callers must match the text. ## Ownership, timeout, reconnect (all re-verified against the source) - MCP session id is the sandbox ownership key (`ensure_session_owns` server-side); a session can only delete what it created; there is no admin/reaper tool, so an orphan lives until its `shutdown_after_seconds` TTL. Therefore one client per pi process: created in `session_start`, closed in `session_shutdown`. - `PI_MCP_BRIDGE_TIMEOUT_MS` defaults to 600 000 ms and is passed to `client.callTool` **per call** (re-read at call time, unlike URL/token which are read at connect time) because a cold-pool claim can block for minutes and the SDK default is 60 s. - `ensureConnection()` is single-flight and only opens a client when none is held. The documented hazard holds: a reconnect is a new MCP session and orphans everything the previous session owned until TTL. - **One honest deviation found while writing the README:** `bridge-proof-2026-09-30/summary.md` §3.5 claims the lazy reconnect also covers "port-forward started after pi". It does not — if the server is unreachable at `session_start`, `registerMcpTools` never runs, so no `mcp_*` tools exist in that session and there is nothing to call lazily. The code's lazy reconnect only covers a client that died *after* registration. The README documents the code's actual behaviour; the spike summary is the stale document. The bridge also does **not** emit a dedicated reconnect warning — each open logs `connected to <url> (session <8 hex>…)`, so a second such line is the tell that prior claims are now orphans. ## `list_files` 403 disposition (minor finding) With **no sandbox claimed in the session**, `list_files` returns `isError` 403: the upstream path resolves the session's sandbox by a **cluster-scope** `sandboxclaims` LIST while the MCP service account holds a **namespaced Role** (`infra-core/core/manifests/agent-sandbox-mcp/rbac.yaml`, deliberately not a ClusterRole — see its own rationale comment). In-session calls never hit it; the lifecycle run did not. **Disposition: documented in `mcp-bridge/README.md` under "Known limitations", no RBAC or manifest change in this PR.** Widening the Role is a cluster-config decision owned by `sandbox-provisioning-contracts.md` §5 in the `infra-core` repo, and that file explicitly warns against widening without such a decision. A follow-up ticket for the RBAC/upstream-path decision is the right home for a fix. ## Verify - [x] `npm ci` in `mcp-bridge/` (97 packages, pinned lockfile) and `npm run typecheck` / `npx tsc --noEmit` clean locally. - [x] Extension loads from the **landed tree path**: `AGENT_SANDBOX_MCP_URL=http://127.0.0.1:19999/mcp pi -p --no-session -ne -e ./mcp-bridge/index.ts …` produced `[pi-mcp-bridge] connect failed for http://127.0.0.1:19999/mcp: fetch failed` and the model still answered — i.e. the copy resolves `./schema.ts` and `@modelcontextprotocol/sdk` from its own `node_modules` and degrades gracefully, as documented. - [ ] **Post-merge (needs the merged tree):** fresh install via the extension install path (`git clone`/symlink into `~/.pi/agent/extensions`) and re-run the LD1.1 lifecycle proof against the agent-sandbox MCP server (create → status → argv exec → `sh -c` exec → upload/download hash agreement → list/exists → delete → gone-signal), asserting 0 leftover claims. - [ ] CI: `mcp-bridge` job (`npm ci` + typecheck) plus PR-title gate. ## Out of scope / follow-ups - **No test suite.** The bridge's proof is the live lifecycle run; the only pure logic is the `schema.ts` sanitizer, which the pinned server never even exercises (its schemas are flat). A vitest suite for the sanitizer + a fake MCP transport would be a worthwhile follow-up ticket, not this landing. - The `list_files` 403 RBAC decision (above), and the design-noted lazy `mcp_connect` activation if the bridge ever ships always-on (prompt-bloat risk #4). Reviewer note: this is a version-control landing of already-proved code, so the review gold is in the README's claims — every claim in it was checked against `index.ts`/`schema.ts` and the spike evidence.
HOMELAB-1769: feat(mcp-bridge): version-control the LD1.1-validated pi<->sandbox MCP seam
All checks were successful
CI / pr-title (pull_request) Successful in 1s
CI / pi-otel-telemetry (pull_request) Successful in 40s
CI / ask-user-question (pull_request) Successful in 46s
CI / forgejo-ci (pull_request) Successful in 48s
CI / skill-args (pull_request) Successful in 53s
CI / todo (pull_request) Successful in 57s
CI / mcp-bridge (pull_request) Successful in 18s
CI / pi-swarm-suite (pull_request) Successful in 3m12s
32f2bbaa38
Author
Owner

Landing review — 2 fresh-context reviewers

Reviewers ran as independent pi -p --no-session subprocesses (omniroute/deepseek-flash), no implementer context. Briefs built from primary sources only: git diff origin/main...origin/plane/HOMELAB-1769-mcp-bridge @ 32f2bba, the HOMELAB-1769 acceptance criteria, repo conventions (root README, sibling extension shape, .forgejo/workflows/ci.yaml), the design doc, and the spike evidence.

  • Reviewer A — code · conventions · test adequacy
  • Reviewer B — claim honesty (claim-by-claim against index.ts/schema.ts) · security · lockfile

Verdicts

Reviewer Verdict Blockers Concerns Suggestions
A (code/conventions) CONCERN 0 3 8
B (claims/security) BLOCKER 1 2 3

Independently confirmed on the positive side

  • Byte-identity claim is TRUE — verified twice independently (sha256sum + diff -r): index.ts, schema.ts, package.json, tsconfig.json, package-lock.json are byte-identical to ~/.pi/agent/extensions/pi-mcp-bridge/ and to the spike snapshot. node_modules/ is absent and untracked.
  • Lockfile integrity clean — lockfileVersion 3, 97 packages, all registry.npmjs.org with sha512- integrity, 0 file:/git+/link: deps, 0 hasInstallScript, root deps match package.json exactly. npm ci → 97 packages, npm run typecheck → exit 0 (run in a temp copy).
  • No secrets, no token leakage (token is env-only and never logged), diff scope exact — only the 9 declared files.
  • Conventions match: CI job shape is identical to every sibling job; docs/mcp-bridge.md (112 lines) matches the sibling docs-page shape; the repo-docs pages have no frontmatter convention, so its absence is correct.

BLOCKER (B) / CONCERN (A) — same finding

# Sev Location Finding
1 BLOCKER mcp-bridge/README.md "Reconnect semantics", troubleshooting row; docs/mcp-bridge.md Gotchas The README documents a lazy reconnect the code does not implement. ensureConnection() returns the held connection unconditionally (index.ts:142); connection is assigned once (:149) and cleared only in session_shutdown (:247); execute never resets it. A client that dies mid-session is therefore never reopened — callTool just throws. So these claims are false: "the reconnect path triggers when the held client dies mid-session", "a second connected to line … is the signal that the session id changed", and the troubleshooting row "MCP bridge is not connected… = the client is gone and reopening it failed" (that branch is effectively unreachable after registration). The PR body's "honest deviation" only half-corrects the spike: neither the "port-forward started after pi" case nor the "client died after registration" case is covered.

Adjudication: the code matches the design intent (pi-mcp-bridge-design.md:287 — "hold one client for the process lifetime, never per-call reconnect"), so the README is the stale document. Fix is docs-only — code stays byte-identical to the LD1.1-validated install. A docs correction is in flight on this branch.

CONCERNs (posted, merging allowed)

# Sev Location Finding
2 CONCERN index.ts:42-45,244-249 connection/shuttingDown are process-global, but session_shutdown is per-session. pi-swarm-suite spawns in-process child AgentSessions, so a child's shutdown closes the parent's only MCP client and latches shuttingDown=true, breaking all mcp_* calls for the parent. Siblings gate this — pi-otel-telemetry/index.ts:504,981 explicitly handles "module shared by every in-process session". Code fix required → follow-up ticket (not this landing).
3 CONCERN schema.ts (no test file) schema.ts is the package's only pure logic and has zero coverage, while every gated sibling ships vitest. Follow-up ticket.
4 CONCERN index.ts:117 new URL(url) sits outside the try (:121), so a malformed AGENT_SANDBOX_MCP_URL rejects session_start instead of logging the documented non-fatal warning. Doc now states the actual behaviour; code fix → follow-up.
5 CONCERN index.ts:197-204 details.rawBlocks/structuredContent are returned untruncated while content is capped at 200 000 chars — a chatty server can still push unbounded data into session history. Documented; optional follow-up.

Suggestions (documented, not blocking)

$defs/definitions stripped at every depth (a property literally named definitions is dropped); normalizeToolName doesn't lowercase so CamelCase tools are silently skipped (README only mentions hyphens); local any shim instead of import type { ExtensionAPI } weakens the CI typecheck; shutdown-during-connect leaks the client; module: NodeNext + ./schema.ts specifier diverges from the house Bundler shape; resolveRef only handles $defs/definitions-rooted pointers despite its comment; unbounded errorText(); no explicit connect timeout; install path ~/.pi/agent/extensions/mcp-bridge vs the live pi-mcp-bridge name.

## Landing review — 2 fresh-context reviewers Reviewers ran as independent `pi -p --no-session` subprocesses (`omniroute/deepseek-flash`), **no implementer context**. Briefs built from primary sources only: `git diff origin/main...origin/plane/HOMELAB-1769-mcp-bridge` @ `32f2bba`, the HOMELAB-1769 acceptance criteria, repo conventions (root README, sibling extension shape, `.forgejo/workflows/ci.yaml`), the design doc, and the spike evidence. - **Reviewer A** — code · conventions · test adequacy - **Reviewer B** — claim honesty (claim-by-claim against `index.ts`/`schema.ts`) · security · lockfile ### Verdicts | Reviewer | Verdict | Blockers | Concerns | Suggestions | |---|---|---|---|---| | A (code/conventions) | CONCERN | 0 | 3 | 8 | | B (claims/security) | **BLOCKER** | 1 | 2 | 3 | ### Independently confirmed on the positive side - **Byte-identity claim is TRUE** — verified twice independently (`sha256sum` + `diff -r`): `index.ts`, `schema.ts`, `package.json`, `tsconfig.json`, `package-lock.json` are byte-identical to `~/.pi/agent/extensions/pi-mcp-bridge/` and to the spike snapshot. `node_modules/` is absent and untracked. - **Lockfile integrity clean** — lockfileVersion 3, 97 packages, all `registry.npmjs.org` with `sha512-` integrity, 0 `file:`/`git+`/`link:` deps, 0 `hasInstallScript`, root deps match `package.json` exactly. `npm ci` → 97 packages, `npm run typecheck` → exit 0 (run in a temp copy). - **No secrets**, no token leakage (token is env-only and never logged), **diff scope exact** — only the 9 declared files. - **Conventions match**: CI job shape is identical to every sibling job; `docs/mcp-bridge.md` (112 lines) matches the sibling docs-page shape; the repo-docs pages have no frontmatter convention, so its absence is correct. ### BLOCKER (B) / CONCERN (A) — same finding | # | Sev | Location | Finding | |---|---|---|---| | 1 | **BLOCKER** | `mcp-bridge/README.md` "Reconnect semantics", troubleshooting row; `docs/mcp-bridge.md` Gotchas | The README documents a **lazy reconnect the code does not implement**. `ensureConnection()` returns the held `connection` unconditionally (`index.ts:142`); `connection` is assigned once (`:149`) and cleared only in `session_shutdown` (`:247`); `execute` never resets it. A client that dies mid-session is therefore **never reopened** — `callTool` just throws. So these claims are false: "the reconnect path triggers when the held client dies mid-session", "a **second** `connected to` line … is the signal that the session id changed", and the troubleshooting row "`MCP bridge is not connected…` = the client is gone and reopening it failed" (that branch is effectively unreachable after registration). The PR body's "honest deviation" only half-corrects the spike: **neither** the "port-forward started after pi" case **nor** the "client died after registration" case is covered. | **Adjudication:** the *code* matches the design intent (`pi-mcp-bridge-design.md:287` — "hold one client for the process lifetime, **never per-call reconnect**"), so the README is the stale document. Fix is **docs-only** — code stays byte-identical to the LD1.1-validated install. A docs correction is in flight on this branch. ### CONCERNs (posted, merging allowed) | # | Sev | Location | Finding | |---|---|---|---| | 2 | CONCERN | `index.ts:42-45,244-249` | `connection`/`shuttingDown` are process-global, but `session_shutdown` is per-session. pi-swarm-suite spawns **in-process** child `AgentSession`s, so a child's shutdown closes the parent's only MCP client and latches `shuttingDown=true`, breaking all `mcp_*` calls for the parent. Siblings gate this — `pi-otel-telemetry/index.ts:504,981` explicitly handles "module shared by every in-process session". **Code fix required → follow-up ticket (not this landing).** | | 3 | CONCERN | `schema.ts` (no test file) | `schema.ts` is the package's only pure logic and has zero coverage, while every gated sibling ships vitest. **Follow-up ticket.** | | 4 | CONCERN | `index.ts:117` | `new URL(url)` sits **outside** the `try` (`:121`), so a malformed `AGENT_SANDBOX_MCP_URL` rejects `session_start` instead of logging the documented non-fatal warning. Doc now states the actual behaviour; code fix → follow-up. | | 5 | CONCERN | `index.ts:197-204` | `details.rawBlocks`/`structuredContent` are returned **untruncated** while `content` is capped at 200 000 chars — a chatty server can still push unbounded data into session history. Documented; optional follow-up. | ### Suggestions (documented, not blocking) `$defs`/`definitions` stripped at every depth (a property literally named `definitions` is dropped); `normalizeToolName` doesn't lowercase so CamelCase tools are silently skipped (README only mentions hyphens); local `any` shim instead of `import type { ExtensionAPI }` weakens the CI typecheck; shutdown-during-connect leaks the client; `module: NodeNext` + `./schema.ts` specifier diverges from the house `Bundler` shape; `resolveRef` only handles `$defs`/`definitions`-rooted pointers despite its comment; unbounded `errorText()`; no explicit connect timeout; install path `~/.pi/agent/extensions/mcp-bridge` vs the live `pi-mcp-bridge` name.
HOMELAB-1769: docs(mcp-bridge): correct reconnect semantics to match implemented behaviour
All checks were successful
CI / pr-title (pull_request) Successful in 0s
CI / forgejo-ci (pull_request) Successful in 36s
CI / mcp-bridge (pull_request) Successful in 20s
CI / ask-user-question (pull_request) Successful in 35s
CI / pi-otel-telemetry (pull_request) Successful in 1m5s
CI / skill-args (pull_request) Successful in 53s
CI / todo (pull_request) Successful in 56s
CI / pi-swarm-suite (pull_request) Successful in 3m43s
1901c5994f
Two independent fresh-context reviewers (landing review of #50) found the
README's "Reconnect semantics" section, its troubleshooting rows and the
docs-page Gotchas describing a lazy reconnect the code does not implement.

Ground truth in index.ts at 32f2bba: ensureConnection() returns the held
client unconditionally (:142),  is assigned once (:149) and
cleared only in session_shutdown (:247), and execute() (:182-206) never
resets it. A client that dies mid-session is therefore never reopened; the
call fails with the transport error. The  line is emitted only
inside openConnection (:135), so a second one cannot appear mid-session. This
matches the design intent (pi-mcp-bridge-design.md:284-287 is about a pi
restart, and the client is held for the process lifetime with no per-call
reconnect).

Docs-only: the extension source stays byte-identical to the LD1.1-validated
live install. Also documents two behaviours the reviewers flagged as
code-fix follow-ups: a malformed AGENT_SANDBOX_MCP_URL throws outside the
connect try/catch, and truncation covers content but not details.
aaron merged commit ec706fb489 into main 2026-09-30 21:30:11 +00:00
Author
Owner

Merged — ec706fb

Blocker resolved before merge; docs-only fix pushed as 1901c59 and CI re-run green.

  • CI run #92 on 1901c59 (pre-merge) — 8/8 green.
  • CI run #94 on ec706fb (post-merge main push) — re-validates the merged tree; pr-title correctly skipped on push.
  • Branch plane/HOMELAB-1769-mcp-bridge kept (repo convention: merged plane/* branches stay on origin).
  • Code files are unchanged by the fix: mcp-bridge/index.ts sha256 0898b214… is identical on origin/main and in the live install at ~/.pi/agent/extensions/pi-mcp-bridge/.

What the fix commit changed

mcp-bridge/README.md + docs/mcp-bridge.md only (46+/30−): the "Reconnect semantics" section, the two troubleshooting rows, the "Divergence from the spike write-up" paragraph, the config "read timing" bullet, and the docs-page Gotchas now state that the bridge never reopens a dead client — a dead client surfaces as a transport/SDK error, recovery is a manual pi restart//reload (which is a new MCP session and orphans the previous session's sandboxes until TTL), and the connected to line appears once, at session_start. Two additional behaviours are now documented in "Known limitations": a malformed AGENT_SANDBOX_MCP_URL throws outside the connect try/catch, and truncation covers content but not details.

Follow-ups filed (the concerns/suggestions that are not docs)

Ticket Sev What
HOMELAB-1801 high Gate connection teardown on the owning session — in-process child AgentSessions currently close the parent's client and latch shuttingDown
HOMELAB-1802 medium Connect/result hardening: new URL() outside the try/catch, no connect timeout, untruncated details, unbounded errorText(), any API shim
HOMELAB-1804 medium vitest suite for schema.ts + two demonstrable bugs (definitions property dropped at any depth; CamelCase tool names silently skipped)
HOMELAB-1803 low list_files 403 RBAC decision (namespaced Role vs cluster-scope LIST)

GitOps continuation: none applicable — this repo has no downstream build/rollout pipeline. The live install at ~/.pi/agent/extensions/pi-mcp-bridge/ remains the operational copy and the merged tree is the version-controlled source of truth; no symlink swap or reinstall was performed (operator choice).

## Merged — `ec706fb` Blocker resolved before merge; docs-only fix pushed as `1901c59` and CI re-run green. - **CI run #92** on `1901c59` (pre-merge) — 8/8 green. - **CI run #94** on `ec706fb` (post-merge `main` push) — re-validates the merged tree; `pr-title` correctly skipped on push. - Branch `plane/HOMELAB-1769-mcp-bridge` **kept** (repo convention: merged `plane/*` branches stay on origin). - Code files are unchanged by the fix: `mcp-bridge/index.ts` sha256 `0898b214…` is identical on `origin/main` and in the live install at `~/.pi/agent/extensions/pi-mcp-bridge/`. ### What the fix commit changed `mcp-bridge/README.md` + `docs/mcp-bridge.md` only (46+/30−): the "Reconnect semantics" section, the two troubleshooting rows, the "Divergence from the spike write-up" paragraph, the config "read timing" bullet, and the docs-page Gotchas now state that the bridge **never reopens a dead client** — a dead client surfaces as a transport/SDK error, recovery is a manual pi restart/`/reload` (which is a new MCP session and orphans the previous session's sandboxes until TTL), and the `connected to` line appears once, at `session_start`. Two additional behaviours are now documented in "Known limitations": a malformed `AGENT_SANDBOX_MCP_URL` throws outside the connect `try`/`catch`, and truncation covers `content` but not `details`. ### Follow-ups filed (the concerns/suggestions that are not docs) | Ticket | Sev | What | |---|---|---| | HOMELAB-1801 | high | Gate connection teardown on the owning session — in-process child `AgentSession`s currently close the parent's client and latch `shuttingDown` | | HOMELAB-1802 | medium | Connect/result hardening: `new URL()` outside the try/catch, no connect timeout, untruncated `details`, unbounded `errorText()`, `any` API shim | | HOMELAB-1804 | medium | vitest suite for `schema.ts` + two demonstrable bugs (`definitions` property dropped at any depth; CamelCase tool names silently skipped) | | HOMELAB-1803 | low | `list_files` 403 RBAC decision (namespaced Role vs cluster-scope LIST) | **GitOps continuation:** none applicable — this repo has no downstream build/rollout pipeline. The live install at `~/.pi/agent/extensions/pi-mcp-bridge/` remains the operational copy and the merged tree is the version-controlled source of truth; no symlink swap or reinstall was performed (operator choice).
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Homelab/pi-extensions!50
No description provided.