feat: add pi agent support (@codemem/pi-extension) - #1430
Conversation
There was a problem hiding this comment.
Pull request overview
Adds first-class pi coding-agent support to codemem as a new client integration, extending the existing raw-event ingest → sweeper → observer → memory pipeline so pi sessions can ingest events, inject pack context per turn, and use the full memory_* tool surface.
Changes:
- Introduces a new
@codemem/pi-extensionpackage (thin pi extension) with HTTP-first ingest/injection and CLI fallback. - Adds pi ingestion and tooling support across core + viewer-server + CLI (new
/api/pi-hooks, newpi-hook-ingest/pi-hook-inject, pi-aware setup + observer derivation). - Updates UI and docs to include pi in supported client enumerations and user-facing guidance; updates release/version automation to include the new package.
Reviewed changes
Copilot reviewed 52 out of 54 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds the new packages/pi-extension project reference. |
| scripts/release-version.test.mjs | Updates release-version test fixtures and expected changed-file count to include pi-extension. |
| scripts/release-version.mjs | Adds packages/pi-extension/package.json to required markers, version reading, and version bump writes. |
| README.md | Documents pi support, setup flow, ingest/injection/tools behavior, and caveats. |
| pnpm-workspace.yaml | Prevents running @google/genai build script (pi transitive dev-only dependency). |
| packages/viewer-server/src/routes/raw-events.ts | Adds POST /api/pi-hooks ingestion endpoint with server-side source: "pi" assertion. |
| packages/viewer-server/src/index.ts | Wires memoryToolRoutes into the viewer server so memory tools work over HTTP. |
| packages/ui/src/tabs/settings/data/value-helpers.ts | Adds a centralized agent-client source list including pi and a formatter for copy strings. |
| packages/ui/src/tabs/settings/data/value-helpers.test.ts | Tests the pi-inclusive client enumeration + formatted list output. |
| packages/ui/src/tabs/settings/data/model-accessors.ts | Updates observer model description copy to mention pi-derived defaults. |
| packages/ui/src/tabs/settings/components/ObserverPanel.tsx | Updates observer connection copy/tooltips to include pi and pi-derived behavior notes. |
| packages/ui/src/tabs/settings/components/ObserverPanel.test.tsx | Ensures observer panel copy mentions pi and pi-derived settings. |
| packages/ui/src/tabs/health/render/health-overview.ts | Updates health recommendations copy to include pi and updates retry guidance. |
| packages/pi-extension/vite.config.ts | Adds build/test config for the new pi extension package with thin-client bundling constraints. |
| packages/pi-extension/tsconfig.json | TypeScript project config for the new pi extension package. |
| packages/pi-extension/src/viewer.ts | Implements viewer discovery/auto-start plus ingest availability probing/backoff. |
| packages/pi-extension/src/payloads.ts | Defines pi-hook payload builders and injection block formatting (CLI parity). |
| packages/pi-extension/src/payloads.test.ts | Validates payload ↔ core adapter mapping and injection header parity. |
| packages/pi-extension/src/learn.ts | Adds the memory_learn prompt contract payload for the pi tool surface. |
| packages/pi-extension/src/index.ts | Main extension factory: wiring handlers, ingest/inject logic, tool registration, boundary flush behavior. |
| packages/pi-extension/src/config.ts | Loads pi extension config from codemem config + env, with JSONC stripping. |
| packages/pi-extension/src/config.test.ts | Tests config loading + env override behavior. |
| packages/pi-extension/src/client.ts | HTTP-first client with CLI fallback for ingest, pack fetching, and tool calls. |
| packages/pi-extension/README.md | Package-level pi extension documentation (install, config, lifecycle, caveats). |
| packages/pi-extension/package.json | Declares @codemem/pi-extension package metadata, build/test scripts, and peer deps. |
| packages/pi-extension/LICENSE | Adds MIT license file for the new package. |
| packages/core/src/pi-observer-config.ts | Implements derivation of observer settings from pi config (API-key providers only, cheap-first). |
| packages/core/src/pi-hooks.ts | Adds pi payload → AdapterEvent mapping, envelope building, and flush-only boundary signals. |
| packages/core/src/observer-client.ts | Adds pi-derived observer fields and in-memory pi auth fallback into the observer auth cascade. |
| packages/core/src/observer-client.test.ts | Adds tests for pi-derived observer auth and ensures secrets aren’t promoted/persisted. |
| packages/core/src/observer-auth.ts | Extends auth resolution cascade to include a pi token source. |
| packages/core/src/observer-auth.test.ts | Tests the new pi-token position in the auth cascade and precedence. |
| packages/core/src/index.ts | Re-exports pi hooks + pi observer config APIs/types from core. |
| packages/cli/src/index.ts | Registers new pi-hook-ingest and pi-hook-inject commands. |
| packages/cli/src/commands/setup.ts | Adds pi setup target, optional MCP adapter support, and pi-based observer derivation wiring. |
| packages/cli/src/commands/setup-pi.test.ts | Adds comprehensive tests for pi setup behavior, idempotency, and no-secret persistence. |
| packages/cli/src/commands/setup-config.ts | Adds JSON writer with backup support for setup flows. |
| packages/cli/src/commands/pi-hook-inject.ts | Implements pi-hook-inject for formatted system-prompt injection blocks (fail-open). |
| packages/cli/src/commands/pi-hook-inject.test.ts | Tests injection behavior, fallback behavior, framing, truncation, and logging. |
| packages/cli/src/commands/pi-hook-ingest.ts | Implements pi-hook-ingest with HTTP-first, direct DB fallback, spooling, and boundary flush. |
| packages/cli/src/commands/pi-hook-ingest-spool.ts | Adds spool + lock implementation for durable pi ingest with backlog draining. |
| docs/versioning.md | Adds @codemem/pi-extension to the shared versioning policy and release bump list. |
| docs/user-guide.md | Documents pi observer derivation behavior and OAuth-only limitations. |
| docs/plugin-reference.md | Adds pi extension installation/configuration and surface parity documentation. |
| docs/architecture.md | Updates architecture docs and diagrams to include pi extension flow and support tier. |
| .github/workflows/release.yml | Publishes @codemem/pi-extension as part of the release workflow. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const opencodeSessionId = envelope.session_stream_id; | ||
| // Assert/normalize source server-side — never trust a caller-supplied source. | ||
| const source = "pi"; | ||
| const strippedPayload = stripPrivateObj(envelope.payload) as Record<string, unknown>; | ||
|
|
||
| const inserted = store.recordRawEvent({ | ||
| opencodeSessionId, | ||
| source, | ||
| eventId: envelope.event_id, | ||
| eventType: envelope.event_type, | ||
| payload: strippedPayload, | ||
| tsWallMs: envelope.ts_wall_ms, | ||
| }); | ||
|
|
||
| store.updateRawEventSessionMeta({ | ||
| opencodeSessionId, | ||
| source, | ||
| cwd: envelope.cwd, | ||
| project: envelope.project, | ||
| startedAt: envelope.started_at, | ||
| lastSeenTsWallMs: envelope.ts_wall_ms, | ||
| }); | ||
|
|
||
| nudgeSweeper(sweeper, [opencodeSessionId], source); |
There was a problem hiding this comment.
Valid observation, applied: the local binding is now named sessionStreamId and the store API field keeps its opencodeSessionId name explicitly, so the pi route no longer implies an opencode origin. Commit aee5090.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 341b77040a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const objStart = trimmed.lastIndexOf("{"); | ||
| const arrStart = trimmed.lastIndexOf("["); | ||
| const start = Math.max(objStart, arrStart); | ||
| if (start < 0) return trimmed; | ||
| return JSON.parse(trimmed.slice(start)); |
There was a problem hiding this comment.
When the viewer is unreachable and a native pi tool falls back to the CLI, normal JSON outputs that contain nested objects (for example {"items":[{"id":1}]} or [{"id":1}]) fail here: lastIndexOf("{") selects the inner object rather than the start of the top-level JSON value, so JSON.parse(trimmed.slice(start)) throws and the tool returns an error instead of the CLI result. This makes the documented CLI fallback unusable for most memory tools whenever HTTP is down.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed: parseCliJson now scans candidate JSON value starts in document order and returns the first suffix that parses, so nested shapes like the items-with-inner-object example work with viewer down. Added regression tests for the exact nested-object case plus logs-before-JSON. Commit aee5090.
| const body = { | ||
| ...(params as Record<string, unknown>), | ||
| project: | ||
| projectOrClient(params as Record<string, unknown>, client) ?? | ||
| (params as { project?: string }).project, | ||
| }; |
There was a problem hiding this comment.
When the caller sets all_projects: true, this still fills project from the current pi session via projectOrClient, so the HTTP route receives both all_projects and project and rejects the request, and the CLI fallback similarly builds distill -p <project> -A. In the common case where client.project is set, the pi memory_distill_candidates tool cannot actually run across all projects.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Verified against the copy in memory-tools.ts:332, which rejects project combined with all_projects, so this was a real failure mode when client.project was set. buildDistillBody now drops project whenever all_projects is requested, on both the HTTP body and the CLI fallback args. Regression tests cover client.project being set. Commit aee5090.
# Conflicts: # docs/architecture.md # packages/viewer-server/src/index.ts # pnpm-lock.yaml
# Conflicts: # packages/cli/src/index.ts # packages/core/src/index.ts # packages/ui/src/tabs/health/render/health-overview.ts
|
@kunickiaj Could you please take a look at #1430 when you have a moment? It has been open since August 10; latest |
kunickiaj
left a comment
kunickiaj
left a comment
There was a problem hiding this comment.
Thanks for putting substantial work into this. I’m open to supporting pi, especially if you’re willing to remain the primary maintainer for the integration.
I can’t merge this in its current form, though. This is not only about the 12.6k-line size: review found several correctness and credential-handling blockers, detailed inline.
The main blockers are:
- A pi API key can be offered to a different observer provider, potentially sending one provider’s credential to another provider’s endpoint.
- File-context enrichment can replace string tool output instead of appending to it.
- Compaction/shutdown extraction receives a seven-second timeout despite performing retries, spool draining, and an LLM call.
- Injection framing is detected using a substring of untrusted memory content.
- Project resolution uses the cwd basename while ingestion uses the Git root, silently breaking retrieval in monorepos and subdirectories.
- The successful boundary path flushes before draining the spooled backlog.
- The new package version is already out of alignment with the release set.
There are also some important follow-ups:
- The new memory-tool POST routes should reuse the bounded JSON parser and validate filter types.
- The spool/locking implementation should be shared rather than adding a third copy of concurrency-sensitive code.
- MCP/viewer memory access and schema behavior should share core primitives to avoid authorization and contract drift.
- The retrieval-ledger behavior for pi’s preferred HTTP path needs an explicit decision.
I’m also asking that this be converted into a stack rather than continuing as one PR. The existing commits already roughly follow suitable boundaries, so this should preserve the work rather than require starting over. A reasonable stack would be:
- Shared spool/memory-access refactors
- Core pi event adapter
- Viewer routes
- Pi observer configuration/auth
- CLI ingest/inject
- Pi extension lifecycle/client
- Native tools
- Setup, UI, docs, and release wiring
Graphite or GitHub’s native stacked-PR workflow is fine. Each layer should remain independently reviewable and testable.
Local validation at 898ccbdd:
- Build passed
- TypeScript passed
- Biome passed
- Targeted tests passed: 292/292 after generating viewer assets
pnpm run release:version -- checkfailed because@codemem/pi-extensionis0.40.1while the release set is0.40.2
Once the stack is created and the blocking findings are addressed, I’m happy to review it layer by layer.
| // cfg.observerApiKey (that would look "explicit" and could be persisted | ||
| // by callers of toConfig()). Only used as a lower-priority cascade source. | ||
| if (!this._apiKey) { | ||
| this._piApiKey = resolvePiApiKeyForObserver(); |
There was a problem hiding this comment.
Blocking: resolvePiApiKeyForObserver() is not provider-scoped. _piApiKey is subsequently offered to the custom, Anthropic, and OpenAI auth branches. If pi selects an API key for provider A while codemem explicitly configures provider B, provider A’s key can be transmitted to provider B’s endpoint.
Please resolve the credential for the effective observer provider and return it only when the pi provider matches. Add a regression test with an explicit Anthropic/OpenAI observer and an unrelated pi provider key.
| try { | ||
| const block = await fetchFileContextBlock(client, path, ctx.signal); | ||
| if (block) { | ||
| const existing = Array.isArray(event.content) ? [...event.content] : []; |
There was a problem hiding this comment.
Blocking: when event.content is a string, existing becomes [], so the read result is replaced by the memory block. This package’s serializer already handles string content, and pi’s tool result contract permits text content.
Normalize string content into a text item before appending, and add a regression test asserting that the original file contents remain present.
| const { stdout } = await this.execCodemem(["pi-hook-ingest"], { | ||
| stdin: JSON.stringify(payload), | ||
| signal, | ||
| timeoutMs: this.config.httpTimeoutMs + 2000, |
There was a problem hiding this comment.
Blocking: boundary events always use this CLI path, but the default timeout is only seven seconds. The command may perform HTTP retries, lock acquisition, spool draining, and flushRawEvents, including an observer LLM request. It can therefore receive SIGTERM during the extraction that shutdown/compaction is specifically intended to preserve.
Boundary events need a substantially larger or independently configured timeout, plus a test demonstrating that a long-running flush is not killed.
| if (!packOrBlock.trim()) return; | ||
|
|
||
| // pi-hook-inject already returns the full ## block; HTTP pack is bare pack_text. | ||
| const block = packOrBlock.includes("## codemem memories") |
There was a problem hiding this comment.
Blocking: this treats a substring inside recalled memory content as proof that the transport returned a preformatted block. A memory that happens to contain ## codemem memories bypasses formatPiInjectionBlock() and therefore the reference-data safety framing.
Please have fetchPackText() return a discriminated result such as { text, preformatted } instead of inferring transport state from content.
| } | ||
|
|
||
| /** Best-effort project label from cwd basename (viewer accepts explicit project). */ | ||
| static projectFromCwd(cwd: string): string | null { |
There was a problem hiding this comment.
Blocking: ingestion resolves the project from the Git root, but this derives retrieval scope from the cwd basename. Running pi from repo/packages/api can write memories under repo and query them under api, producing silently empty injection and tool results.
Please match core’s project resolution semantics, including walking to the repository root, and add a nested-cwd regression test.
| // 1. Unlocked HTTP attempt — fast path when the viewer is up. | ||
| const httpResult = await httpIngest(payload, opts.host, port); | ||
| if (httpResult.ok) { | ||
| await flushOnBoundaryIfRequested(); |
There was a problem hiding this comment.
Blocking: the successful HTTP path flushes before draining the backlog. For a shutdown/compaction event, spooled earlier events are inserted only after the final extraction, leaving the session tail unprocessed.
Drain the backlog before flushOnBoundaryIfRequested(). The locked retry path already uses the correct ordering.
| @@ -0,0 +1,69 @@ | |||
| { | |||
| "name": "@codemem/pi-extension", | |||
| "version": "0.40.1", | |||
There was a problem hiding this comment.
pnpm run release:version -- check currently fails because this package is 0.40.1 while the managed release set is 0.40.2. Please align it before the release-wiring layer lands.
| const store = getStore(); | ||
| let body: unknown; | ||
| try { | ||
| body = await c.req.json(); |
There was a problem hiding this comment.
These new POST routes use unbounded c.req.json() even though raw-events.ts already has a bounded JSON parser. Please reuse the bounded parser for remember, expand, explain, and distill_candidates, and validate filter value types before passing them to SQLite.
|
@kunickiaj Thank you for the review. I split the work into the eight stacked PRs you asked for, rebased onto current main (0.41). This PR stays open; it is not the merge path. Please start at #1473 (layer 1; mergeable). Layers 2–8 are on the fork for now, because GitHub cannot base a PR on
After #1473 merges I will retarget layer 2 at this repo’s The seven blockers are fixed in the layer that owns them, with tests. I will do what I can on the pi side and fix issues or bugs in this integration. I cannot guarantee I will maintain it forever. Happy to walk through any layer. |
Closes #1429
Summary
First-class support for the pi coding agent, at the same depth as the opencode integration — plus two pi-only capabilities (compaction-boundary extraction, fork-aware streams).
pi has no native MCP or hooks mechanism; its extension API is the hook surface. This PR adds the fourth client integration following the existing AdapterEvent → raw-events → sweeper → ingest pipeline:
@codemem/pi-extension(new npm pi-package): thin client — event ingest, per-turn system-prompt memory injection, all 14memory_*tools registered natively (no third-party MCP adapter required). Zero native deps inside pi's process (dist imports node builtins only).claude-hooks(source: "pi", deterministicpi:<session>:<entry>ids, flush-only compaction signals) + pi-derived observer config (API-key providers only in v1; OAuth explicitly unsupported-with-status; cheap-model-first; credentials resolved in-memory at point of use, never persisted/logged).POST /api/pi-hooks(server-side source assertion) + closes the HTTP route gap so all 14 tools work over HTTP (remember,timeline,expand,schema,search_index,explain,distill_candidates).pi-hook-ingest(+spool, boundary flush) /pi-hook-inject, andcodemem setuppi target (--pi-only,--pi-mcpadapter opt-in,--pi-extension-pathdev mode; idempotent; upgrades stale npm pins without--force).Shared store ⟹ multi-agent users get cross-agent memory (pack filters are project-scoped, never agent-scoped).
Attribution hardening
~20 sites silently default an event's source to
"opencode"; every pi path passessource: "pi"explicitly and the ingestion route asserts it server-side. Tests assert pi-ingested rows attribute to pi and create zero opencode rows (store + route + CLI levels).Test plan
pnpm run lint && pnpm run tsc && pnpm run test && pnpm buildgreentest:plugin122/122 (no regressions for existing clients)source=pi(0 opencode rows), retry dedupe, remember→inject round-trip,## codemem memoriesblockraw-event-sweeper/raw-event-flush/ingest-pipelinetimeout-sensitive tests can fail under heavy load (they pass 179/179 in isolation) — same class exists onmain, untouched by this PRKnown limitations (documented)
unconfigured (oauth-only)with guidance instead of silent 401spi-hook-ingestlock-busy boundary-flush gap in the claude twin is pre-existing upstream behavior; pi's twin handles all paths — the claude fix is left for a follow-up PRcodemem setup's settings.json write is JSON (JSONC comments in pi's file are not preserved;.bakbackup is written first — same behavior as the existing opencode.jsonc handling)