Conversation
Exposes the Pi coding-agent API as iii functions and streams, nothing else. pi::run executes one headless Pi turn in a chosen working directory and returns the result, token usage, and cost; pi::start runs it in the background. Raw AgentSession events mirror verbatim onto pi::events, and a translated AgentEvent view lands on agent::events, so the console and acp worker render a Pi run like any native harness turn. pi::steer and pi::follow_up inject instructions into a live run via Pi's steering and follow-up queues. Sessions resume by session file, keyed by iii session_id in engine state. Turns carry the iii runtime context by default so the agent discovers and calls registered functions through the iii CLI. Embeds @earendil-works/pi-coding-agent in-process (no CLI subprocess). Bundle worker on the configuration-worker pattern: config.yaml is the seed, the live value hot-reloads. 68 tests, typecheck and lint clean, single-file bundle builds.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 34 minutes and 52 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR introduces a new ChangesPi Worker Package
Discovery Prompt Refinements
Sequence Diagram(s)sequenceDiagram
participant Caller
participant pi_run as pi::run handler
participant state as iii state (pi_sessions)
participant buildSession as buildSession
participant PiAgent as AgentSession
participant rawStream as raw_events_stream
participant agentStream as agent::events stream
Caller->>pi_run: {prompt, model, session_id, ...}
pi_run->>state: loadSession(session_id)
state-->>pi_run: SessionRecord | null
pi_run->>buildSession: {cwd, model, thinkingLevel, resumeFile}
buildSession-->>pi_run: AgentSession
pi_run->>state: saveSession(status: working)
pi_run->>PiAgent: subscribe(listener)
pi_run->>PiAgent: prompt(text)
loop Pi emits events
PiAgent-->>pi_run: tool_start / tool_end / assistant_message / end
pi_run->>rawStream: emit(raw event)
pi_run->>agentStream: emit(function_execution_start/end or message_complete)
end
pi_run->>agentStream: emit(turn_end)
pi_run->>agentStream: emit(agent_end)
pi_run->>state: saveSession(status: done | error)
pi_run->>PiAgent: dispose()
pi_run-->>Caller: {session_id, pi_session_id, result, stop_reason, usage, total_cost_usd}
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
skill-check — worker0 verified, 25 skipped (no docs/).
Four for four. Nicely done. |
Sync the iii runtime context with the harness system-prompt enrichment (workers#309): teach worker::list (installed + daemon-managed builtins) alongside engine::workers::list (WS-connected only) so the agent can confirm a worker is RUNNING by merging the two, and add the trust-a-probe -over-an-empty-list rule so lag is not mistaken for absence. Harness-only guidance (coder-for-files, web::fetch, agent_trigger mechanics) is deliberately not ported: this agent has its own native tools.
…ility Backport the provider-neutral half of the harness system-prompt enrichment (workers#309) into the claude-code and codex iii runtime context: teach worker::list (installed + daemon-managed builtins) alongside engine::workers::list (WS-connected only) so the agent confirms a worker is RUNNING by merging the two, and add the trust-a-probe-over-an -empty-list rule. Harness-only guidance (coder-for-files, web::fetch, agent_trigger payload mechanics) is intentionally not ported: both agents drive iii through the CLI and own native file/shell/web tools.
Halve the single-file bundle (14.7MB -> 7.4MB) by minifying the esbuild output, and declare esbuild as the only approved build dependency so a clean pnpm install does not prompt to approve build scripts. The minified bundle boots and registers all functions unchanged.
Show a real pi::run turn (pong with usage and cost), the pi::run --help request-schema table, pi::sessions::list output, and a live iii discovery turn enumerating every connected worker.
rohitg00
marked this pull request as ready for review
June 23, 2026 12:28
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
pi/src/events.ts (1)
9-16: 🩺 Stability & Availability | 🔵 Trivial | ⚖️ Poor tradeoffPotential memory leak: unbounded session sequence tracking.
The
seqBySessionMap grows indefinitely as new sessions are added. Completed or abandoned sessions are never removed, which could cause memory accumulation in long-running workers with many sessions.Consider adding cleanup logic:
- Remove entries after a session completes (agent_end event)
- Periodically prune entries older than a reasonable threshold
- Use an LRU cache with a maximum size
Potential cleanup approach
One option is to add a cleanup function called after emitting
agent_end:export function makeEmitter(iii: ISdk, streamName: string) { return { emit: async function (session_id: string, event: unknown): Promise<void> { const seq = seqBySession.get(session_id) ?? 0; seqBySession.set(session_id, seq + 1); const item_id = `${session_id}-${PROCESS_EPOCH}-${seq.toString().padStart(8, '0')}`; try { await iii.trigger({ function_id: 'stream::set', payload: { stream_name: streamName, group_id: session_id, item_id, data: event }, }); } catch (err) { console.warn(`stream::set failed for ${session_id}: ${String(err)}`); } // Clean up after terminal events if ((event as { type?: string })?.type === 'agent_end') { seqBySession.delete(session_id); } }, }; }Note: This would require updating call sites to use
emitter.emit()instead ofemitter().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pi/src/events.ts` around lines 9 - 16, The seqBySession Map grows indefinitely as new sessions are added without ever being cleaned up, causing potential memory leaks in long-running workers. Add cleanup logic to the emit function within makeEmitter that checks if the event being emitted has a type property equal to 'agent_end', and if so, delete the corresponding session_id entry from the seqBySession Map. This ensures that completed sessions no longer accumulate in memory.pi/tests/_helpers/fake-iii.ts (1)
36-39: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winClone
state::get/state::listoutputs to match bus semantics.Inbound payloads are cloned, but Line 36 and Line 38 return map-backed objects by reference. That can let tests mutate persisted state without calling
state::set, reducing test fidelity.Proposed patch
- if (req.function_id === 'state::get') return state.get(`${scope}/${key}`) ?? null; + if (req.function_id === 'state::get') { + return structuredClone(state.get(`${scope}/${key}`) ?? null); + } if (req.function_id === 'state::list') { - return [...state.entries()].filter(([k]) => k.startsWith(`${scope}/`)).map(([, v]) => v); + return [...state.entries()] + .filter(([k]) => k.startsWith(`${scope}/`)) + .map(([, v]) => structuredClone(v)); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pi/tests/_helpers/fake-iii.ts` around lines 36 - 39, The state::get and state::list operations are returning references to objects from the state map instead of clones. To match bus semantics and prevent tests from mutating persisted state without calling state::set, clone the returned values in both cases. For state::get, wrap the returned value with a deep clone utility before returning. For state::list, ensure each value in the returned array is cloned so mutations don't affect the original persisted state. This will improve test fidelity by preventing unintended state mutations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pi/iii.worker.yaml`:
- Line 12: The start script command in pi/iii.worker.yaml currently references
./index.mjs, but the build:bundle script outputs the bundled file to
dist/bundle/index.mjs. Update the start command to point to the correct output
path by changing it to node ./dist/bundle/index.mjs so that the worker uses the
actual bundled output location.
In `@pi/src/index.ts`:
- Line 57: The console.log statement at line 57 logs the full URL which may
contain sensitive credentials or token-like query parameters, causing potential
credential leaks in logs. Create a helper function or inline logic to extract
and log only the safe parts of the URL (scheme, hostname, and port) while
excluding the query string and credentials. Update the console.log statement to
use this sanitized version of the url variable instead of logging the complete
URL directly.
In `@pi/src/run.ts`:
- Around line 324-336: The issue is that the pi::start registered function
discards the return value of executeRun using void and always returns {
session_id, started: true }, even when executeRun would return { busy: true } to
indicate the session is already active. To fix this, remove the void keyword and
await the executeRun call instead of discarding it, then merge and return the
actual result from executeRun (which could indicate busy status) along with the
session_id, rather than always reporting started: true.
In `@pi/src/session.ts`:
- Around line 30-35: The resolveModel function silently returns undefined when
the model string is malformed (when the '/' separator is not found or is at
position 0), which masks configuration errors and makes it hard to detect
misconfiguration. Instead of silently returning undefined in the condition where
sep <= 0, throw an error with a descriptive message that explains the expected
model format (provider/modelId) to help users identify and fix configuration
issues immediately.
In `@pi/tests/events.test.ts`:
- Around line 39-44: The spy created with vi.spyOn(console, 'warn') is only
restored after all assertions if they pass, but if any expect statement fails
before line 44, the mockRestore() call will not execute and the spy will remain
active for subsequent tests. Wrap the test logic (the await expect and
subsequent expect calls) in a try-finally block so that warn.mockRestore() is
called in the finally clause, guaranteeing cleanup regardless of whether
assertions pass or fail.
---
Nitpick comments:
In `@pi/src/events.ts`:
- Around line 9-16: The seqBySession Map grows indefinitely as new sessions are
added without ever being cleaned up, causing potential memory leaks in
long-running workers. Add cleanup logic to the emit function within makeEmitter
that checks if the event being emitted has a type property equal to 'agent_end',
and if so, delete the corresponding session_id entry from the seqBySession Map.
This ensures that completed sessions no longer accumulate in memory.
In `@pi/tests/_helpers/fake-iii.ts`:
- Around line 36-39: The state::get and state::list operations are returning
references to objects from the state map instead of clones. To match bus
semantics and prevent tests from mutating persisted state without calling
state::set, clone the returned values in both cases. For state::get, wrap the
returned value with a deep clone utility before returning. For state::list,
ensure each value in the returned array is cloned so mutations don't affect the
original persisted state. This will improve test fidelity by preventing
unintended state mutations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6040636-87b3-4bb0-b6cb-1ac79ab554e9
⛔ Files ignored due to path filters (5)
pi/assets/cli-discovery.pngis excluded by!**/*.pngpi/assets/cli-help.pngis excluded by!**/*.pngpi/assets/cli-run.pngis excluded by!**/*.pngpi/assets/cli-sessions.pngis excluded by!**/*.pngpi/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (33)
claude-code/src/iii-prompt.tscodex/src/iii_prompt.rspi/.gitignorepi/README.mdpi/biome.jsonpi/config.yamlpi/iii-permissions.yamlpi/iii.worker.yamlpi/package.jsonpi/scripts/build-bundle.mjspi/skills/SKILL.mdpi/src/config.tspi/src/configuration.tspi/src/events.tspi/src/iii-prompt.tspi/src/index.tspi/src/map.tspi/src/run.tspi/src/session.tspi/src/state.tspi/src/types.tspi/tests/_helpers/fake-iii.tspi/tests/_helpers/fake-session.tspi/tests/config.test.tspi/tests/configuration.test.tspi/tests/events.test.tspi/tests/map.test.tspi/tests/register.test.tspi/tests/run-payload.test.tspi/tests/run.test.tspi/tests/state.test.tspi/tsconfig.jsonpi/vitest.config.ts
pi::start reserved the live slot but always returned started: true, so a
start against an already-running session reported success while the run
was silently dropped. Check the live slot synchronously (executeRun sets
it before any await) and return { busy: true, started: false } instead;
start still returns immediately rather than awaiting the turn.
resolveModel now warns when the configured model is not in provider/modelId
form instead of silently falling back, so a misconfigured model id is
visible in the logs.
Add pi to the release flow and repo index (new-worker SOP §3, §6): the pi/v* tag trigger in release.yml, pi in the create-tag.yml worker choices, and a pi row in the root README Modules table — matching the claude-code and codex agent workers.
This was referenced Jul 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Exposes the Pi coding-agent API as iii functions and streams, nothing else. pi::run executes one headless Pi turn in a chosen working directory and returns the result, token usage, and cost; pi::start runs it in the background. Raw AgentSession events mirror verbatim onto pi::events, and a translated AgentEvent view lands on agent::events, so the console and acp worker render a Pi run like any native harness turn.
pi::steer and pi::follow_up inject instructions into a live run via Pi's steering and follow-up queues. Sessions resume by session file, keyed by iii session_id in engine state. Turns carry the iii runtime context by default so the agent discovers and calls registered functions through the iii CLI.
Embeds @earendil-works/pi-coding-agent in-process (no CLI subprocess). Bundle worker on the configuration-worker pattern: config.yaml is the seed, the live value hot-reloads. 68 tests, typecheck and lint clean, single-file bundle builds.
Summary by CodeRabbit
New Features
Documentation
Chores