Uh oh!
There was an error while loading. Please reload this page.
[codex] Add Pi provider integration - #2831
Conversation
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
| readonly resetTimeoutOnActivity?: boolean | undefined; | ||
| }): Promise<PiRpcRunResult> { | ||
| return await new Promise((resolve, reject) => { | ||
| const child = spawn(input.binaryPath, ["--mode", "rpc", ...(input.args ?? [])], { | ||
| cwd: input.cwd, | ||
| env: input.environment ?? process.env, | ||
| shell: process.platform === "win32", | ||
| stdio: ["pipe", "pipe", "pipe"], | ||
| }); | ||
| const responses = new Map<string, PiRpcLine>(); | ||
| const pending = new Set(input.commands.map((command) => command.id)); | ||
| const events: PiRpcLine[] = []; | ||
| let stderr = ""; | ||
| let stdoutBuffer = ""; | ||
| let agentEnded = false; | ||
| let requestedClose = false; | ||
| let settled = false; | ||
| const finishIfReady = () => { | ||
| if (pending.size > 0) return; | ||
| if (input.waitForAgentEnd === true && !agentEnded) return; | ||
| if (requestedClose) return; | ||
| requestedClose = true; | ||
| child.stdin.end(); | ||
| }; | ||
| const rejectOnce = (error: Error) => { | ||
| if (settled) return; | ||
| settled = true; | ||
| clearTimeout(timer); | ||
| child.kill("SIGTERM"); | ||
| reject(error); | ||
| }; | ||
| const timeoutMs = input.timeoutMs ?? 120_000; | ||
| let timer: ReturnType<typeof setTimeout>; | ||
| const resetTimer = () => { | ||
| clearTimeout(timer); | ||
| // @effect-diagnostics-next-line globalTimers:off | ||
| timer = setTimeout(() => { | ||
| rejectOnce(new Error(`Timed out waiting for Pi RPC. Stderr: ${stderr.trim()}`)); | ||
| }, timeoutMs); | ||
| }; | ||
| // @effect-diagnostics-next-line globalTimers:off | ||
| timer = setTimeout(() => { | ||
| rejectOnce(new Error(`Timed out waiting for Pi RPC. Stderr: ${stderr.trim()}`)); | ||
| }, timeoutMs); | ||
| input.signal?.addEventListener("abort", () => { | ||
| rejectOnce(new Error("Pi RPC request was aborted.")); | ||
| }); |
There was a problem hiding this comment.
🟢 LowLayers/PiRpc.ts:149
When an already-aborted AbortSignal is passed, the abort listener is never triggered, so the function spawns the child process and runs to completion or timeout instead of immediately rejecting. Consider checking input.signal?.aborted before spawning and rejecting early if already aborted.
+ if (input.signal?.aborted) {+ rejectOnce(new Error("Pi RPC request was aborted."));+ return;+ }+
input.signal?.addEventListener("abort", () => {
rejectOnce(new Error("Pi RPC request was aborted."));
});🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/provider/Layers/PiRpc.ts around lines 149-201:
When an already-aborted `AbortSignal` is passed, the `abort` listener is never triggered, so the function spawns the child process and runs to completion or timeout instead of immediately rejecting. Consider checking `input.signal?.aborted` before spawning and rejecting early if already aborted.
Evidence trail:
apps/server/src/provider/Layers/PiRpc.ts lines 139-201 at REVIEWED_COMMIT: no `signal.aborted` check before `spawn()` on line 152; `addEventListener('abort', ...)` on line 199 won't fire for already-aborted signals per Node.js EventTarget spec. Node.js v22+ docs confirm `addEventListener` does not retroactively fire: https://nodejs.org/api/events.html (events.addAbortListener section).
| operation: "startSession", | ||
| issue: `Expected provider '${PROVIDER}' but received '${input.provider}'.`, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🟡 MediumLayers/PiAdapter.ts:285
When startSession replaces an existing session at lines 286-290, it aborts the old session and deletes it from sessions but never emits a session.exited event. This differs from stopSession which emits the event, so downstream consumers cannot track the lifecycle transition when sessions are replaced.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/provider/Layers/PiAdapter.ts around line 285:
When `startSession` replaces an existing session at lines 286-290, it aborts the old session and deletes it from `sessions` but never emits a `session.exited` event. This differs from `stopSession` which emits the event, so downstream consumers cannot track the lifecycle transition when sessions are replaced.
Evidence trail:
apps/server/src/provider/Layers/PiAdapter.ts lines 286-290 (startSession replaces existing session without emitting event), lines 686-700 (stopSession emits session.exited event after similar abort+delete), lines 759-761 (streamEvents exposes runtimeEvents queue to consumers).
Summary
Adds Pi as a selectable provider in T3 Code using Pi's local CLI/RPC integration rather than treating it as a generic API provider. The adapter discovers and uses the existing local Pi configuration, surfaces configured Pi models in the selector, supports Pi slash-command metadata in provider discovery, and routes prompts through Pi sessions.
Follow-up fixes in this branch also make Pi prompt timeouts activity-based for long tool-heavy turns and map Pi tool execution events into T3 provider runtime tool lifecycle events so the Work Log can show progress while Pi is using tools.
Implementation Notes
~/.pi/agentconfiguration.Validation
bun installbun fmtbun lintbun typecheckbun run testbunx vitest run src/provider/Layers/PiRpc.test.ts~/.pi/agentconfigNotes / Risks
PI_PARITY.md; commands that are inherently Pi-native are surfaced/documented rather than reimplemented as separate T3 UI features.Note
Add Pi provider integration with session management, streaming, and text generation
piprovider driver kind in contracts, settings, and the built-in driver registry, removing it from the "Coming soon" list in the settings UI.ProviderAdapterShapefor Pi: session lifecycle, streaming assistant text and tool events, attachment handling, turn interruption, and in-memory session state.pi --versionand Pi RPC inventory, and slash command surfacing.PI_CODING_AGENT_DIRor~/.pi/agent; misconfiguration or binary absence will surface as probe errors at status-check time.📊 Macroscope summarized 9c1c57e. 17 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.