feat(providers): add Pi coding agent - #7211
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 Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
Effect service conventions review of the new Pi provider code. Two error-modeling issues found; everything else (namespace subpath imports, Effect.catchTags usage, layer/driver construction acquiring ChildProcessSpawner/FileSystem/IdAllocatorV2/ServerConfig from the environment, Schema.TaggedErrorClass failures) follows the conventions.
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
One consistency finding on the new web provider icon wiring. Everything else in the web scope (providerDriverMeta.ts, ProviderModelsSection.tsx, contextWindow.ts, session-logic.ts, AddProviderInstanceDialog.tsx) is registry/data-only and matches the existing per-driver patterns.
Posted via Macroscope — UI Consistency
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces a substantial Pi provider with a new RPC process, MCP and permission bridge, session lifecycle, text generation, and cross-cutting UI/server changes. Unresolved high- and medium-severity findings include unbounded buffering, text-generation timeouts, and runtime-request lifecycle gaps, warranting human review. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
There was a problem hiding this comment.
One finding on the Pi icon geometry after the tile removal. Everything else in the web scope (provider icon map, driver meta, model placeholder, display-name and picker option entries) follows the existing per-provider patterns.
Posted via Macroscope — UI Consistency
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Effect service conventions: one finding — a raw child-process stderr payload is copied into a log annotation. Elsewhere in this repo process output is reported as lengths only (stdoutLength/stderrLength, lineLength), with explicit tests asserting stderr is not retained in diagnostics.
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
Two log annotations in the new Pi adapter copy Pi's raw wire error text into the observability stream. Everything else in the Pi files (error attributes, cause handling, driver/layer shape, dependency acquisition) matches the repo's adapter conventions.
Posted via Macroscope — Effect Service Conventions
7c18cde to
f8c8600
Compare
Distinguish a nonzero pi child exit from an unexplained stdout close without copying stderr into public error details.
Pi only honors leading /skill:name commands, so hoist every known $skill. Map global location to user scope, pass displayName and shortDescription through, and keep skills that omit a filesystem path.
The default thinking level was advertised with the sentinel id "inherit", so re-picking the Default-tagged choice sent "do not change" and silently kept the previously applied level. Give every choice its real level id and apply it explicitly; Pi's discovered default keeps only the Default tag.
The Pi default model advertises no thinking choices, so an unqualified return kept the effort a previous pick had applied. Re-capture the configured thinking baseline and restore it alongside the baseline model replay, with a high-to-default adapter regression test.
The base branch now carries live context usage on provider turns (tokenUsage, pingdotgg#8144) and the web meter reads it from the projection. Pi kept a second owner for the same data: a contextUsage snapshot on the provider thread, refreshed after terminalization and invalidated by hand on rollback. Read Pi's session stats while the turn settles and attach them to the completed provider turn instead, then drop the provider-thread field and its generation bookkeeping. A turn without usable stats simply carries no report, so the meter keeps the last reported turn without adapter-side retention logic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
7441f73 to
3399752
Compare
| } | ||
|
|
||
| function splitJsonlChunks(buffer: string, chunk: string): readonly [ReadonlyArray<string>, string] { | ||
| const combined = buffer + chunk; |
There was a problem hiding this comment.
🟠 High Adapters/PiRpc.ts:111
splitJsonlChunks grows buffer without bound when stdout never contains \n, so a long malformed stream accumulates memory and repeatedly copies the string until the server becomes unresponsive or OOMs. Enforce a maximum record size and discard the unterminated fragment through the next LF before parsing again.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/PiRpc.ts around line 111:
`splitJsonlChunks` grows `buffer` without bound when stdout never contains `\n`, so a long malformed stream accumulates memory and repeatedly copies the string until the server becomes unresponsive or OOMs. Enforce a maximum record size and discard the unterminated fragment through the next LF before parsing again.
| answers: Record<string, unknown> | undefined, | ||
| ): PiRpcRecord { | ||
| if (pending.method === "confirm") { | ||
| if (decision === "accept" || decision === "acceptForSession") return { confirmed: true }; |
There was a problem hiding this comment.
🟡 Medium Adapters/PiAdapterV2.ts:2549
Selecting acceptForSession only approves the current tool call; every subsequent call still prompts, so the UI's “Always allow this session” action is ineffective. This branch returns the same one-shot { confirmed: true } response as accept, and no approval state is retained for the injected tool_call hook. Persist the session approval and consult it before creating later prompts.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/PiAdapterV2.ts around line 2549:
Selecting `acceptForSession` only approves the current tool call; every subsequent call still prompts, so the UI's “Always allow this session” action is ineffective. This branch returns the same one-shot `{ confirmed: true }` response as `accept`, and no approval state is retained for the injected `tool_call` hook. Persist the session approval and consult it before creating later prompts.
| }); | ||
| } | ||
|
|
||
| yield* connection.request({ type: "prompt", message: prompt }); |
There was a problem hiding this comment.
🟠 High textGeneration/PiTextGeneration.ts:109
connection.request waits for an id-matched response that Pi does not send for prompt, so every ephemeral text-generation operation times out after the 30-second RPC timeout even when the prompt has settled. Use connection.send and continue consuming the id-less acknowledgement from connection.events, as PiAdapterV2 does.
| yield* connection.request({ type: "prompt", message: prompt }); | |
| yield* connection.send({ type: "prompt", message: prompt }); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/textGeneration/PiTextGeneration.ts around line 109:
`connection.request` waits for an id-matched response that Pi does not send for `prompt`, so every ephemeral text-generation operation times out after the 30-second RPC timeout even when the prompt has settled. Use `connection.send` and continue consuming the id-less acknowledgement from `connection.events`, as `PiAdapterV2` does.
| if (token === undefined) continue; | ||
| body = `${body.slice(0, token.start)}${body.slice(token.end)}`; | ||
| } | ||
| body = body.replace(/\s+/g, " ").trim(); |
There was a problem hiding this comment.
🟡 Medium provider/PiCommands.ts:96
expandPiSkillReference destroys formatting in the remaining prompt whenever it removes a valid skill reference, so inputs containing newlines or indentation—such as code blocks—are sent with all whitespace collapsed to single spaces. Replace the whitespace normalization with trimming only, preserving the prompt body after the skill token is removed.
| body = body.replace(/\s+/g, " ").trim(); | |
| body = body.trim(); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/PiCommands.ts around line 96:
`expandPiSkillReference` destroys formatting in the remaining prompt whenever it removes a valid skill reference, so inputs containing newlines or indentation—such as code blocks—are sent with all whitespace collapsed to single spaces. Replace the whitespace normalization with trimming only, preserving the prompt body after the skill token is removed.
| const answer = answers?.[pending.questionId]; | ||
| // An empty string is a valid dialog value per the RPC spec (the extension | ||
| // receives ""), distinct from cancelling (the extension receives undefined). | ||
| if (typeof answer === "string") return { value: answer }; |
There was a problem hiding this comment.
🟡 Medium Adapters/PiAdapterV2.ts:2556
Empty-string answers for Pi input and editor prompts never reach piUiResponse: the shared web and mobile answer builders reject them with if (!answer) return null, leaving the dialog stuck or forcing cancellation even though piUiResponse correctly returns { value: "" }. Update those builders to distinguish undefined from the valid empty string.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/PiAdapterV2.ts around line 2556:
Empty-string answers for Pi `input` and `editor` prompts never reach `piUiResponse`: the shared web and mobile answer builders reject them with `if (!answer) return null`, leaving the dialog stuck or forcing cancellation even though `piUiResponse` correctly returns `{ value: "" }`. Update those builders to distinguish `undefined` from the valid empty string.
| import { HostProcessPlatform } from "@t3tools/shared/hostProcess"; | ||
| import { resolveSpawnCommand } from "@t3tools/shared/shell"; | ||
|
|
||
| export class PiRpcError extends Schema.TaggedErrorClass<PiRpcError>()("PiRpcError", { |
There was a problem hiding this comment.
🟠 High Adapters/PiRpc.ts:33
discoverPiViaRpc never consumes connection.events, while PiRpc offers every valid non-response record to an unbounded queue. User extensions can therefore emit events faster than discovery drains them, growing the heap without limit for up to 15 seconds and potentially causing OOM; disable extensions for this probe or bound/drain the event queue.
Also found in 1 other location(s)
apps/server/src/provider/Layers/PiProvider.ts:130
discoverPiViaRpcstarts Pi with the user's extensions enabled, but never consumesconnection.events.PiRpcstores every non-response JSONL record in an unbounded queue, so an extension that emits progress/status events while discovery is running grows that queue without limit for up to 15 seconds and can exhaust the server heap. Disable extensions for this probe or drain/bound the event queue.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/PiRpc.ts around line 33:
`discoverPiViaRpc` never consumes `connection.events`, while `PiRpc` offers every valid non-response record to an unbounded queue. User extensions can therefore emit events faster than discovery drains them, growing the heap without limit for up to 15 seconds and potentially causing OOM; disable extensions for this probe or bound/drain the event queue.
Also found in 1 other location(s):
- apps/server/src/provider/Layers/PiProvider.ts:130 -- `discoverPiViaRpc` starts Pi with the user's extensions enabled, but never consumes `connection.events`. `PiRpc` stores every non-response JSONL record in an unbounded queue, so an extension that emits progress/status events while discovery is running grows that queue without limit for up to 15 seconds and can exhaust the server heap. Disable extensions for this probe or drain/bound the event queue.
There was a problem hiding this comment.
🟡 Medium
A runless user_input_request remains pending in the transcript when the provider session is released before it is answered, even though its runtime request and node are terminalized. writeReleasedRuntimeRequestEvents only finds approval_request turn items, so include user_input_request in this release cleanup as well.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProviderSessionManager.ts around line 618:
A runless `user_input_request` remains `pending` in the transcript when the provider session is released before it is answered, even though its runtime request and node are terminalized. `writeReleasedRuntimeRequestEvents` only finds `approval_request` turn items, so include `user_input_request` in this release cleanup as well.
| // request UI can answer them and unblock session setup. | ||
| const threadId = sessionScopedRuntimeRequestThreadId(event); | ||
| if (threadId !== undefined) { | ||
| yield* providerEventIngestor |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ProviderSessionManager.ts:1388
A runless event can be ingested after releaseEntry has removed the session and persisted cleanup, recreating an actionable runtime request, node, or item for a provider process that is already gone. The event-pump callback does not revalidate entry.runtime or coordinate with release before calling ingestNormalized; serialize this ingestion with release or reject events from no-longer-live runtimes.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProviderSessionManager.ts around line 1388:
A runless event can be ingested after `releaseEntry` has removed the session and persisted cleanup, recreating an actionable runtime request, node, or item for a provider process that is already gone. The event-pump callback does not revalidate `entry.runtime` or coordinate with release before calling `ingestNormalized`; serialize this ingestion with release or reject events from no-longer-live runtimes.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ 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 3399752. Configure here.
The test helper accepted a settings layer with an unknown error channel, which leaked into every makeTestLayer consumer and failed the Effect diagnostics in CI. The only caller already orDies the layer, so the slot can require never. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Upstream t3code ships five drivers and no Pi one. That is not an oversight waiting to be fixed: 21 community PRs adding Pi have been opened since April and every one was closed unmerged, and no new provider driver has landed on main since Grok on 2026-06-09 -- while the repo merged ~700 PRs in the last 30 days otherwise. Providers specifically are frozen. So pin inputs.t3code-src to the head of pingdotgg/t3code#7211, which adds a real PiDriver + PiRpc + MCP injection and was live-tested against pi 0.84.2 (the version inputs.pi ships). #7211 is stacked on the maintainer branch behind #2829 ("introduce new orchestrator"), ~250 commits ahead of main, so this pin carries orchestrator v2 as well -- flake.nix has the full rationale and the removal condition. packages/t3code/default.nix needed no changes: the pnpm2nix workspace, the node-pty graft, the T3 Connect env bake and the Electron shell all survive the jump to 0.0.35 untouched. Pinning an explicit rev also freezes t3code against `just update`, which is deliberate -- the fork branch gets force-pushed as its author iterates. luna already had pi 0.84.2 on the t3code unit's PATH via the dev bundle, so enabling the provider is the only thing that was missing. Verified: luna's toplevel builds, and its closure carries the same t3code-0.0.35 store path built straight from the PR head, whose server bundle registers "pi" alongside the existing five driver kinds.
6733253 to
886b8db
Compare
Trial layer for pingdotgg#7211 pi-provider. Net of the 18 unique CTM commits at aa38c5d, replayed onto the published trial tip. Adds Pi as an Early Access built-in provider (pi --mode rpc) with native steer, abort-based Stop, official-extension subagent projection, and session-tree rollback. Adapted the replay for this line's older CTM pin: thread fixtures include settledOverrideAt, and Pi select/input/editor questions set multiSelect to false. Shared registry files keep the existing OpenCode 2 entries. Not published to origin/trial/orchestrator-v2.1.
Bring trial Pi spawn discovery in line with the current pingdotgg#7211 pi-provider tip plus StiensWout#35 diagnose-early-pi-rpc-exits git and project package loading.
A first run creates a placeholder provider thread with no native identity, but ensureThread never handed that row to the adapter, so the Pi adapter registered a second row keyed by the session file. Both rows then received updates at turn end, and last-writer-wins on activeProviderThreadId decided at random whether the row carrying contextUsage was active — hiding the composer's context meter on most turns. Pass the run's provider thread through existingProviderThread at both ensureThread call sites so the adapter adopts the orchestration-owned row id (Pi's adopt branch already handles a row without a native ref). The replacement path after a failed resume drops the dead native ref so a fresh session still binds to the same row. OpenCode now only takes its resume path for rows that actually carry a native session, which keeps its behavior unchanged for these new inputs. Reported by @astarktc in pingdotgg#7211. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
discoverPiUserExtensions passed any *.ts/*.js directory entry as an explicit --extension path, including macOS AppleDouble sidecars like "._name.ts". Pi treats a parse failure on an explicit extension path as fatal, so one stale sidecar in ~/.pi/agent/extensions killed every Pi turn with an opaque "pi process closed stdout" error while interactive pi kept working. Skip dot entries, matching pi's own loader. Reported by @astarktc in pingdotgg#7211. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
a26d9ad to
d6ed793
Compare
Merges the head of pingdotgg#7211 (StiensWout:t3code/pi-provider @ a00565f) into upstream/main @ 5e63aea. That PR is stacked on pingdotgg#2829 (t3code/codex-turn-mapping, orchestrator v2), so this single merge brings in both. Conflict resolution lives in this commit rather than in a follow-up, so git rerere can replay it the next time the PR branch is rebased. Resolved 53 conflicts: 32 taken from the integrated tree, 21 orchestration v1 files deleted as part of the v2 migration.
11 participants
Problem
Pi users depend on their existing models, auth, extensions, skills, context files, and native session history. T3 Code did not have a Pi provider, and a shallow CLI wrapper would lose the customization and session behavior that make Pi useful.
Solution
This adds Pi as an Early Access provider on Orchestrator V2:
delegate_tasktool creates durable child threads in the existing subagent UI. User-installed Pisubagentextensions also project progress cards there, without inventing resumable child sessions that Pi does not provide.Pi defaultwhen optional model and command discovery cannot complete. The live interactive session remains authoritative and can handle startup prompts.Pi defaultrestores Pi's own model and thinking level.$skill chips are hoisted to Pi's leading/skill:nameform (all of them, not only the first), and catalog entries without a filesystem path stay listed. Pi's global skill location maps to the personal scope.tokenUsage, the same per-turn report the base branch added for Codex (feat(claude): compact old threads before they burn through usage #8144), so the shared context meter needs no Pi-specific plumbing.agent_settledis not always idle: an extension can start a detached compaction as it fires. The adapter confirms Pi is idle before terminalizing so that recovery stays inside the same turn, and Stop terminates the process once settlement has begun because Pi's generic abort does not cancel detached work.lastErrorso the banner is not blank.The MCP bridge, Pi subagent investigation, and per-model thinking support originated in @mwolson's stacked contribution and retain that authorship. The exit-code diagnostics, explicit thinking levels,
$skill hoisting, retry and compaction work-log items, and idle confirmation before terminalization come from @mwolson's follow-up branch and keep that authorship too. The per-model reasoning-effort memory from that branch is a cross-provider web and mobile change and is left for its own PR.Related work
Closes #397.
Closes #402.
Addresses #6685.
This supersedes the Pi implementations in #2211, #2748, #2800, #2812, #2831, #2856, #3818, #3947, #4355, #4445, #5688, #5882, and #6319.
This incorporates StiensWout#34 by @mwolson.
Validation
unknownerror-channel warnings inProviderSessionManager.test.ts, which the base branch has as well.git diff --check.t3code/codex-turn-mapping(0ae375b).Proof
Provider discovery
End-to-end Pi turn
Context meter persists across turns
The active context stays at 21k/272k while Pi's cumulative session total rises from 90k to 111k after the next completed turn.
Configured thinking default
Pi's effective setting appears as the labeled default while the internal selection continues to inherit from Pi.
Native steering
Checkpoint and Pi session-tree rollback
Clean Stop
Stopping an active Pi turn leaves the expected interrupted terminal and no provider stream error.
Stacked on #2829 (
t3code/codex-turn-mapping) until that branch lands.Built by GPT-5.6 Sol in T3 Code through Codex. Rebase onto the current base, the per-turn usage alignment, and review-feedback integration by Claude Fable 5 in Claude Code.
Note
High Risk
Introduces a full new provider orchestration path (process RPC, MCP, permissions, turn lifecycle) and changes shared ensureThread semantics for OpenCode and Pi thread rows.
Overview
Adds Pi as an orchestration V2 provider (stdio JSONL RPC, session files, MCP bridge, skills, steering, compaction/retry work-log behavior) with a large adapter test suite and mobile Pi icon plus "Pi" catalog labeling.
Thread settings on mobile now derive runtime mode options from the active or pending model’s
supportedRuntimeModes, clamp the displayed mode when the current choice isn’t allowed, and use that filtered list in the Runtime submenu instead of the global three modes.Provider thread identity:
OpenCodeAdapterV2.ensureThreadonly resumes rows that already have anativeThreadRef; placeholder rows without one get the newly created native session bound onto the same id (Pi tests assert the same adoption pattern) soactiveProviderThreadIddoesn’t flap between duplicate rows.Reviewed by Cursor Bugbot for commit a00565f. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add
piprovider driver with RPC adapter, text generation, and UI supportpibuilt-in driver (PiDriver.ts) andpidriver kind in contracts (settings.ts, model.ts), exposing orchestration, text generation, and managed provider snapshots.PiAdapterV2(PiAdapterV2.ts) andPiRpc(PiRpc.ts) for stdio JSONL RPC sessions: thread mapping, event streaming, steering, approvals, forks, snapshots, and token usage aggregation with a minimum Pi version of0.80.5.PiTextGeneration(PiTextGeneration.ts) for ephemeral Pi-backed commit messages, PR content, branch names, and thread titles with a 180s timeout.supportedRuntimeModesfiltering inChatComposer(ChatComposer.tsx) andThreadSettingsSheet(ThreadSettingsSheet.tsx).OpenCodeAdapterV2.ensureThread(OpenCodeAdapterV2.ts) to create native sessions for placeholder rows and adopt existing thread identity, and updatesProviderSessionManager(ProviderSessionManager.ts) to persist runless runtime request events and gracefully end subscribers on provider-initiated stops.ServerProviderschema gains optionalsupportedRuntimeModes;ChatComposernow filters permission modes per provider and falls back to the first supported mode when a thread's stored mode is unsupported.ProviderSessionManagerevent pump now ingests runless events directly viaProviderEventIngestorV2and performs graceful subscriber termination on provider stops.Macroscope summarized a00565f.