feat(pi): add first-class Pi provider support - #5688
Conversation
Adds the Pi RPC provider across server, web, desktop, and mobile, while preserving Pi's own configuration and session state.\n\nBuilt with GPT-5.4 in Pi.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the new Pi provider modules (PiDriver, PiAdapter, PiProvider, PiRpcClient, PiSessionFile, PiTextGeneration) against the Effect service conventions. Dependency acquisition, namespace imports, and layer/factory shapes look consistent with the existing provider modules. Three error-modeling/handling violations are noted inline.
Posted via Macroscope — Effect Service Conventions
| Effect.catchTag( | ||
| "TimeoutError", | ||
| () => | ||
| new TextGenerationError({ | ||
| operation: input.operation, | ||
| detail: "Pi text generation timed out.", | ||
| }), | ||
| ), |
There was a problem hiding this comment.
Statically known tagged failures should be recovered with Effect.catchTags({ ... }), including when only one tag is handled; Effect.catchTag is not used by this convention.
| Effect.catchTag( | |
| "TimeoutError", | |
| () => | |
| new TextGenerationError({ | |
| operation: input.operation, | |
| detail: "Pi text generation timed out.", | |
| }), | |
| ), | |
| Effect.catchTags({ | |
| TimeoutError: () => | |
| new TextGenerationError({ | |
| operation: input.operation, | |
| detail: "Pi text generation timed out.", | |
| }), | |
| }), |
Posted via Macroscope — Effect Service Conventions
| new TextGenerationError({ | ||
| operation: input.operation, | ||
| detail: "Failed to start Pi RPC text generation.", | ||
| cause: String(cause), |
There was a problem hiding this comment.
String(cause) erases the underlying PiRpcError (tag, fields, stack) so the error chain is no longer inspectable. TextGenerationError.cause is Schema.Defect(), so the failure itself can be preserved directly — consider passing it through as sibling text-generation modules do (OpenCode2TextGeneration uses cause).
| cause: String(cause), | |
| cause, |
Posted via Macroscope — Effect Service Conventions
| onFailure: (cause) => | ||
| end(new PiRpcProcessExitedError({ detail: `Pi RPC stdout failed: ${String(cause)}` })), |
There was a problem hiding this comment.
This stringifies the stdout failure into detail and drops the failure itself: PiRpcProcessExitedError (lines 40-43) has no cause field, so the immediate underlying error and its stack are lost, and the caller-visible message becomes arbitrary defect text. Consider adding cause: Schema.optional(Schema.Defect()) to the error class and constructing it with a stable detail (for example "Pi RPC stdout failed") plus cause, keeping the exact value only in cause.
Posted via Macroscope — Effect Service Conventions
| cause: info.type, | ||
| }); | ||
| yield* fs.access(sessionFile, { readable: true }); | ||
| const firstLine = (yield* fs.readFileString(sessionFile)).split(/\r?\n/, 1)[0] ?? ""; |
There was a problem hiding this comment.
🟠 High pi/PiSessionFile.ts:79
validatePiResumeSessionFile calls fs.readFileString(sessionFile) and only then takes the first line, so resuming a long-running session loads the entire JSONL transcript into memory even though only the header is needed. A large session file can cause a major memory spike or OOM. Consider reading a bounded prefix of the file (for example via a stream or a fixed-size read) and parsing the first line from that instead.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/pi/PiSessionFile.ts around line 79:
`validatePiResumeSessionFile` calls `fs.readFileString(sessionFile)` and only then takes the first line, so resuming a long-running session loads the entire JSONL transcript into memory even though only the header is needed. A large session file can cause a major memory spike or OOM. Consider reading a bounded prefix of the file (for example via a stream or a fixed-size read) and parsing the first line from that instead.
| | { readonly confirmed: boolean } | ||
| | { readonly cancelled: true }, | ||
| ) { | ||
| if (ctx.pendingUserInputs.get(requestId) !== pending) return false; |
There was a problem hiding this comment.
🟠 High Layers/PiAdapter.ts:341
resolveExtensionInput deletes the pending request from ctx.pendingUserInputs and emits user-input.resolved before calling ctx.client.respondToExtensionUi. If that RPC call fails (e.g., the Pi process exited), the error propagates to the caller, but the UI has already been told the request was resolved and the map entry is gone — so the user cannot retry and Pi never receives the answer. The pending entry should be retained (or restored) and the resolved event published only after respondToExtensionUi succeeds.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/PiAdapter.ts around line 341:
`resolveExtensionInput` deletes the pending request from `ctx.pendingUserInputs` and emits `user-input.resolved` *before* calling `ctx.client.respondToExtensionUi`. If that RPC call fails (e.g., the Pi process exited), the error propagates to the caller, but the UI has already been told the request was resolved and the map entry is gone — so the user cannot retry and Pi never receives the answer. The pending entry should be retained (or restored) and the resolved event published only after `respondToExtensionUi` succeeds.
| !available.models.some((m) => m.provider === parsed.provider && m.id === parsed.modelId) | ||
| ) | ||
| return yield* validation("sendTurn", "Selected Pi model is not currently available."); | ||
| yield* ctx.client |
There was a problem hiding this comment.
🟡 Medium Layers/PiAdapter.ts:1354
After setModel succeeds, ctx.session.model is never updated, so listSessions() keeps returning the model from startSession even when a later turn selects a different one. This means the advertised in-session model switch is not reflected in session state. Consider assigning ctx.session.model (and any related fields) from the parsed selection after the setModel call, as other in-session adapters do.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/PiAdapter.ts around line 1354:
After `setModel` succeeds, `ctx.session.model` is never updated, so `listSessions()` keeps returning the model from `startSession` even when a later turn selects a different one. This means the advertised in-session model switch is not reflected in session state. Consider assigning `ctx.session.model` (and any related fields) from the parsed selection after the `setModel` call, as other in-session adapters do.
| } | ||
| if (state.value.isStreaming === true) return; | ||
| const terminalEvents: ProviderRuntimeEvent[] = []; | ||
| if (turn.assistantText.trim().length > 0) { |
There was a problem hiding this comment.
🟡 Medium Layers/PiAdapter.ts:1022
When a Pi response contains only whitespace deltas, the assistant message item is left permanently inProgress even after the turn completes. turn.assistantStarted is set to true on the first delta, but the settlement logic at agent_settled only emits item.completed when turn.assistantText.trim().length > 0, so a whitespace-only response never closes the item. The reasoning branch correctly uses turn.reasoningStarted for this decision — the assistant branch should use turn.assistantStarted instead of re-checking the trimmed text length.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/PiAdapter.ts around line 1022:
When a Pi response contains only whitespace deltas, the assistant message item is left permanently `inProgress` even after the turn completes. `turn.assistantStarted` is set to `true` on the first delta, but the settlement logic at `agent_settled` only emits `item.completed` when `turn.assistantText.trim().length > 0`, so a whitespace-only response never closes the item. The reasoning branch correctly uses `turn.reasoningStarted` for this decision — the assistant branch should use `turn.assistantStarted` instead of re-checking the trimmed text length.
| Effect.map((value) => `t3-pi-${String(value)}`), | ||
| ); | ||
| const waiter = yield* Deferred.make<PiRpcResponse, PiRpcError>(); | ||
| yield* Ref.update(pending, (current) => new Map(current).set(id, waiter)); |
There was a problem hiding this comment.
🟠 High pi/PiRpcClient.ts:257
After the transport has ended (stdout exits or client.close() runs), new request calls still register a waiter and block until the 120-second timeout instead of failing immediately with PiRpcProcessExitedError. The end function clears the pending map once, so any request added afterward is stranded with no response or future end to complete it.
This happens because request checks neither the closed ref nor whether the transport is still alive before inserting into pending. Consider failing the request up front when closed is true, or having end also reject late additions via a shared flag.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/pi/PiRpcClient.ts around line 257:
After the transport has ended (stdout exits or `client.close()` runs), new `request` calls still register a waiter and block until the 120-second timeout instead of failing immediately with `PiRpcProcessExitedError`. The `end` function clears the pending map once, so any request added afterward is stranded with no response or future `end` to complete it.
This happens because `request` checks neither the `closed` ref nor whether the transport is still alive before inserting into `pending`. Consider failing the request up front when `closed` is `true`, or having `end` also reject late additions via a shared flag.
| ); | ||
| }; | ||
|
|
||
| const interruptTurn: ProviderAdapterShape<ProviderAdapterError>["interruptTurn"] = ( |
There was a problem hiding this comment.
🟠 High Layers/PiAdapter.ts:1432
interruptTurn calls withThreadLock, but sendTurn already holds that lock while ctx.client.prompt can be blocked waiting for extension UI input. The interrupt request deadlocks on the lock and the abort RPC is never sent, so the turn cannot be cancelled while it is waiting on user input. The same deadlock affects stopSession (and stopAll during finalization): it blocks on the lock held by sendTurn and hangs indefinitely when the extension UI has no timeout. Consider not serializing interruptTurn and stopSession behind the same lock, or having sendTurn release the lock while awaiting extension UI, so abort/close can proceed.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/PiAdapter.ts around line 1432:
`interruptTurn` calls `withThreadLock`, but `sendTurn` already holds that lock while `ctx.client.prompt` can be blocked waiting for extension UI input. The interrupt request deadlocks on the lock and the abort RPC is never sent, so the turn cannot be cancelled while it is waiting on user input. The same deadlock affects `stopSession` (and `stopAll` during finalization): it blocks on the lock held by `sendTurn` and hangs indefinitely when the extension UI has no timeout. Consider not serializing `interruptTurn` and `stopSession` behind the same lock, or having `sendTurn` release the lock while awaiting extension UI, so abort/close can proceed.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f3eb5d0. Configure here.
| if (Exit.isFailure(state) || !piStateMatchesCursor(state.value, ctx.cursor)) { | ||
| yield* failActive(ctx, "Pi session identity drifted during settlement.", native); | ||
| yield* close(ctx); | ||
| return; |
There was a problem hiding this comment.
Settlement errors misread as drift
Medium Severity
During agent_settled handling, a failed get_state Exit is treated the same as a cursor mismatch. That fails the turn with an identity-drift message and closes the session, so a transient RPC/get_state error tears down an otherwise healthy Pi session.
Reviewed by Cursor Bugbot for commit f3eb5d0. Configure here.
| }, | ||
| raw: raw(native), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Whitespace leaves assistant item open
Medium Severity
Assistant text_delta events start an assistant item whenever the delta is non-empty, including whitespace. Settlement only emits item.completed when trimmed assistant text is non-empty, so whitespace-only replies leave an in-progress assistant item after turn.completed.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f3eb5d0. Configure here.
![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: f3eb5d0f67
ℹ️ 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".
| if (type === "message_end") { | ||
| yield* handleSubagentResultMessage(ctx, turn, event, native); | ||
| return; |
There was a problem hiding this comment.
When Pi ends an assistant message with stopReason: "error" or "aborted", this branch ignores that status and the later agent_settled handler publishes a successful turn.completed. API, authentication, and model failures can therefore appear to users as successful blank turns; handle terminal assistant-message stop reasons here, as PiTextGeneration already does.
AGENTS.md reference: AGENTS.md:L139-L142
Useful? React with 👍 / 👎.
| const client = yield* makeRpcClient({ | ||
| command: settings.binaryPath, | ||
| args: DETERMINISTIC_ARGS, | ||
| env: environment, | ||
| }); |
There was a problem hiding this comment.
The health probe launches Pi without a cwd, so getCommands() loads commands, extensions, and skills relative to the T3 server process rather than the user's project. For projects with project-scoped Pi configuration, those commands will be absent from the composer catalog, while commands from the server's own directory may be advertised instead; command discovery needs project/session context or must avoid presenting this process-global snapshot as the project catalog.
Useful? React with 👍 / 👎.
| cause: info.type, | ||
| }); | ||
| yield* fs.access(sessionFile, { readable: true }); | ||
| const firstLine = (yield* fs.readFileString(sessionFile)).split(/\r?\n/, 1)[0] ?? ""; |
There was a problem hiding this comment.
For every resumed Pi thread, this reads the entire append-only JSONL session merely to inspect its first line. Long-running sessions can contain large tool outputs and attachments, so resume cost and peak memory grow with the complete conversation and can stall or exhaust the server; stream or bound the read to the first newline instead.
AGENTS.md reference: AGENTS.md:L15-L17
Useful? React with 👍 / 👎.
| const parsed = selection ? decodePiModelSlug(selection.model) : undefined; | ||
| if (!parsed) | ||
| return yield* validation("sendTurn", "A valid Pi model selection is required."); |
There was a problem hiding this comment.
ThreadTurnStartCommand.modelSelection is optional, and the orchestration layer deliberately passes it through as absent for in-session-switching providers, but Pi rejects every such turn even when ctx.session.model already identifies the active model. Compatible clients or API callers that omit the redundant selection on a follow-up therefore fail with “A valid Pi model selection is required”; use the stored session model and keep it updated after model switches.
AGENTS.md reference: AGENTS.md:L67-L74
Useful? React with 👍 / 👎.
ApprovabilityVerdict: Needs human review 6 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
1 participant
Closes #402.
What Changed
Adds Pi as a first-class provider through Pi's RPC mode. T3 now discovers the installed Pi binary and its real models, starts and resumes Pi session files, maps Pi output and tools into the existing provider event model, and exposes Pi on web, desktop, and mobile.
The provider also reads Pi's command and skill catalog, bridges RPC-safe extension questions into T3's user-input panel, and projects Pi subagents and workflows into the existing Agents panel. Pi keeps ownership of its models, accounts, tools, extensions, skills, config, and session files.
This does not add an Executor or MCP path.
Why
#402 asks for a provider that fits T3's current provider and orchestration layers without a Pi-only client flow. This implementation keeps the process and protocol details at the Pi adapter boundary and reuses the current model picker, composer, task lifecycle, Agents panel, and provider settings.
The full vertical slice is large because a provider needs contracts, health checks, model discovery, session lifecycle, event mapping, clients, and tests to work safely. Splitting those pieces would leave branches that register a provider which cannot complete a turn.
UI Changes
No new UI system or provider-only screen was added. Pi uses the current provider picker, model and reasoning controls, composer command and skill menus, user-input panel, and Agents panel. Before this change Pi is absent from those existing surfaces; after it is shown when its configured binary passes the health check.
Verification
/extension commands and$skillsChecklist
Built with GPT-5.4 in Pi.
Note
Medium Risk
Large new provider path with child-process RPC, durable session files, and complex turn/settlement concurrency; mistakes could break threads or leak sessions, but it does not change auth or billing.
Overview
Pi is registered as a built-in provider driver that spawns the Pi CLI in RPC mode, probes models/commands/skills for snapshots, and wires adapter + text generation like other agents.
The Pi adapter owns durable session files (allocate, resume cursor validation, per-file leasing), full-access-only threads, and maps Pi RPC stream events into T3 turns, tool items (bash/edit/agents), user-input for extension UI, and task lifecycle for subagents and workflows—including steer prompts, settlement on
agent_settled, and interrupt/close edge cases.Supporting pieces include an NDJSON RPC client, URL-encoded model slugs with thinking-level options, PiTextGeneration for commit/PR/branch/title helpers, plus a Pi icon and "Pi" display label on mobile.
Reviewed by Cursor Bugbot for commit f3eb5d0. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add first-class Pi provider support with RPC client, session management, and text generation
PiDriver,PiAdapter, andmakePiTextGenerationto register Pi as a full built-in provider with adapter, text generation, and managed snapshot capabilities.makePiRpcTransportandmakePiRpcClientin PiRpcClient.ts for NDJSON-framed RPC communication with the Pi binary, including request correlation, timeouts, and graceful shutdown.PiSessionFileutilities for secure per-instance session file allocation with strict containment, permission, and symlink validation.checkPiProviderStatusin PiProvider.ts to probe the Pi binary, discover models and commands, and build provider snapshots distinguishing missing-binary from other errors.PiSettings/PiSettingsPatch), the session provider picker (marked 'new'), and web/mobile UI with the Pi icon and display label.📊 Macroscope summarized f3eb5d0. 20 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.