Merged
Conversation
|
@oneilltomhq is attempting to deploy a commit to the plgeek Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new Pi coding agent plugin: implementation and factory, registration and installer mapping, tests, and documentation; includes binary discovery and version probing, non‑interactive JSONL execution and parsing, model/thinking configuration, setup prompts, and sandbox requirements. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Registry as "Agent Registry"
participant PiAgent as "PiAgentPlugin"
participant PiCLI as "pi (binary)"
participant Parser as "JSONL Parser"
User->>Registry: select/run Pi agent
Registry->>PiAgent: instantiate with config (mode, model, thinking, timeout)
PiAgent->>PiCLI: discover binary / probe --version
PiAgent->>PiCLI: run with args (stdin prompt, --print, --mode=json, ...)
PiCLI-->>PiAgent: stream JSONL stdout
PiAgent->>Parser: pass JSONL lines
Parser-->>PiAgent: AgentDisplayEvent(s)
PiAgent-->>User: emit display events / results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/plugins/pi-agent.test.ts (1)
227-229: Consider addingisReady()check beforedispose()in nested afterEach.The outer
afterEach(lines 43-51) defensively checksisReady()before callingdispose(), but this nestedafterEachdoes not. Since some tests in this describe block calldispose()mid-test (e.g., lines 243, 261, 276), theafterEachcould attempt to dispose an already-disposed plugin. Consider applying the same defensive pattern for consistency.♻️ Suggested fix
afterEach(async () => { + try { + if (await testablePlugin.isReady()) { - await testablePlugin.dispose(); + await testablePlugin.dispose(); + } + } catch { + // Ignore errors from already-disposed plugin + } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/plugins/pi-agent.test.ts` around lines 227 - 229, The nested afterEach should defensively check the plugin's readiness before disposing to avoid double-dispose errors: wrap the call to testablePlugin.dispose() in an isReady() check (use testablePlugin.isReady()) inside the nested afterEach so it only calls testablePlugin.dispose() when the plugin is still ready/active, mirroring the outer afterEach behavior.website/content/docs/plugins/agents/pi.mdx (1)
1-4: Consider updating AGENTS.md.Based on learnings, agent implementations should be documented in AGENTS.md with clear descriptions of purpose, capabilities, and usage. Ensure AGENTS.md is also updated with the Pi agent details to maintain it as the single source of truth for agent documentation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/content/docs/plugins/agents/pi.mdx` around lines 1 - 4, Add the Pi Agent to AGENTS.md as the canonical entry: create a section titled "Pi Agent" that mirrors the content in the pi.mdx page (purpose, capabilities, usage instructions, configuration options, and example commands/flags), include a short one-line summary and a "See also" link to the Pi Agent docs (website/content/docs/plugins/agents/pi.mdx), and ensure the format and headings match other agent entries in AGENTS.md so it appears consistently in the single source of truth for agents.src/plugins/agents/builtin/pi.ts (1)
374-409: Potential issue: originalonStdoutcallback is replaced, not wrapped.When
isJsonModeis true and callbacks are provided, the customonStdouthandler parses JSONL and callsoptions.onStdoutwith the processed output. However, if a consumer expects raw stdout data (e.g., for logging), they will only receive the formatted output, not the raw JSONL lines.This may be intentional for cleaner display, but it differs from some other agent plugins that preserve raw output. If raw output is needed for debugging, consider either:
- Documenting this behaviour clearly, or
- Providing a separate callback for raw JSONL data
The current implementation is functional, so this is just a note for awareness.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/agents/builtin/pi.ts` around lines 374 - 409, The current isJsonMode onStdout handler replaces raw stdout delivery; update the handler so it preserves raw data by invoking a new/optional callback if present (e.g., options.onRawStdout or options.onStdoutRaw) with the original data before doing JSONL parsing and event processing, while leaving the existing behavior that calls options.onJsonlMessage, options.onStdoutSegments and options.onStdout (which receives processed output) intact; modify the onStdout branch around isJsonMode and the parsing block to first call options.onRawStdout(data) when present, then continue parsing with parsePiOutputToEvents, JSON.parse lines, processAgentEventsToSegments and processAgentEvents as now.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/content/docs/plugins/agents/pi.mdx`:
- Around line 241-243: The docs show two different install commands (bunx pi-cli
vs npm install -g `@mariozechner/pi-coding-agent`); verify which package name is
correct and make the instructions consistent by updating the troubleshooting
snippet and the prerequisites snippet to use the same package and installer
(e.g., replace `bunx pi-cli` with the canonical install command or replace the
npm global install with the bunx invocation), and ensure any references to the
package name (`pi-cli`, `@mariozechner/pi-coding-agent`) and installer (`bunx`,
`npm install -g`) are identical across the file.
---
Nitpick comments:
In `@src/plugins/agents/builtin/pi.ts`:
- Around line 374-409: The current isJsonMode onStdout handler replaces raw
stdout delivery; update the handler so it preserves raw data by invoking a
new/optional callback if present (e.g., options.onRawStdout or
options.onStdoutRaw) with the original data before doing JSONL parsing and event
processing, while leaving the existing behavior that calls
options.onJsonlMessage, options.onStdoutSegments and options.onStdout (which
receives processed output) intact; modify the onStdout branch around isJsonMode
and the parsing block to first call options.onRawStdout(data) when present, then
continue parsing with parsePiOutputToEvents, JSON.parse lines,
processAgentEventsToSegments and processAgentEvents as now.
In `@tests/plugins/pi-agent.test.ts`:
- Around line 227-229: The nested afterEach should defensively check the
plugin's readiness before disposing to avoid double-dispose errors: wrap the
call to testablePlugin.dispose() in an isReady() check (use
testablePlugin.isReady()) inside the nested afterEach so it only calls
testablePlugin.dispose() when the plugin is still ready/active, mirroring the
outer afterEach behavior.
In `@website/content/docs/plugins/agents/pi.mdx`:
- Around line 1-4: Add the Pi Agent to AGENTS.md as the canonical entry: create
a section titled "Pi Agent" that mirrors the content in the pi.mdx page
(purpose, capabilities, usage instructions, configuration options, and example
commands/flags), include a short one-line summary and a "See also" link to the
Pi Agent docs (website/content/docs/plugins/agents/pi.mdx), and ensure the
format and headings match other agent entries in AGENTS.md so it appears
consistently in the single source of truth for agents.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/plugins/agents/builtin/index.tssrc/plugins/agents/builtin/pi.tssrc/setup/skill-installer.tstests/plugins/pi-agent.test.tswebsite/content/docs/plugins/agents/pi.mdx
oneilltomhq
added a commit
to oneilltomhq/ralph-tui
that referenced
this pull request
Feb 26, 2026
- Add defensive isReady() check in nested afterEach to avoid double-dispose - Fix inconsistent install instructions in troubleshooting section
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/plugins/pi-agent.test.ts (2)
242-256: Consider adding themode: 'text'negative path forbuildArgs.You already verify JSON inclusion; adding the no-tracing text-mode case would better pin expected omission of
--mode json.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/plugins/pi-agent.test.ts` around lines 242 - 256, Add a negative test that verifies buildArgs does not include "--mode json" for the text/no-tracing path by creating a TestablePiPlugin, initializing it with mode: 'text' (use TestablePiPlugin.initialize({ mode: 'text' }) after disposing any prior instance like dispose()), then call testablePlugin.testBuildArgs('test prompt', ...) or testBuildArgs('test prompt') and assert that the returned args do NOT contain '--mode' and do NOT contain 'json'; reference the existing tests that call testBuildArgs, TestablePiPlugin, initialize and dispose to mirror setup/teardown.
326-328: Harden teardown in this block to reduce flaky hook failures.This
afterEachis less defensive than thebuildArgssuite cleanup. Aligning both patterns will make the tests more resilient.Proposed patch
afterEach(async () => { - await testablePlugin.dispose(); + try { + if (await testablePlugin.isReady()) { + await testablePlugin.dispose(); + } + } catch { + // Ignore errors from already-disposed plugin + } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/plugins/pi-agent.test.ts` around lines 326 - 328, The teardown afterEach currently calls await testablePlugin.dispose() directly and should be hardened like the buildArgs suite: guard against a missing plugin, await disposal inside a try/catch to swallow and log non-fatal errors, and null out the reference afterwards; update the afterEach to check if testablePlugin is truthy, run await testablePlugin.dispose() inside a try/catch (e.g., catch and optionally console.warn or rethrow only on unexpected errors), and then set testablePlugin = undefined to ensure subsequent tests start clean.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/plugins/pi-agent.test.ts`:
- Around line 242-256: Add a negative test that verifies buildArgs does not
include "--mode json" for the text/no-tracing path by creating a
TestablePiPlugin, initializing it with mode: 'text' (use
TestablePiPlugin.initialize({ mode: 'text' }) after disposing any prior instance
like dispose()), then call testablePlugin.testBuildArgs('test prompt', ...) or
testBuildArgs('test prompt') and assert that the returned args do NOT contain
'--mode' and do NOT contain 'json'; reference the existing tests that call
testBuildArgs, TestablePiPlugin, initialize and dispose to mirror
setup/teardown.
- Around line 326-328: The teardown afterEach currently calls await
testablePlugin.dispose() directly and should be hardened like the buildArgs
suite: guard against a missing plugin, await disposal inside a try/catch to
swallow and log non-fatal errors, and null out the reference afterwards; update
the afterEach to check if testablePlugin is truthy, run await
testablePlugin.dispose() inside a try/catch (e.g., catch and optionally
console.warn or rethrow only on unexpected errors), and then set testablePlugin
= undefined to ensure subsequent tests start clean.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/plugins/pi-agent.test.tswebsite/content/docs/plugins/agents/pi.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- website/content/docs/plugins/agents/pi.mdx
oneilltomhq
added a commit
to oneilltomhq/ralph-tui
that referenced
this pull request
Feb 26, 2026
- Add defensive isReady() check in nested afterEach to avoid double-dispose - Add negative test case for mode: text (verifies --mode json is omitted) - Harden teardown in getStdinInput block for resilient test cleanup - Fix inconsistent install instructions in troubleshooting section
oneilltomhq
force-pushed
the
feat/add-pi-agent-plugin
branch
from
d41647f to
df0f3d3
Compare
oneilltomhq
added a commit
to oneilltomhq/ralph-tui
that referenced
this pull request
Feb 26, 2026
- Integrates Pi CLI with Ralph TUI for AI-assisted coding - Supports JSONL output for subagent tracing and detailed tool call breakdowns - Includes comprehensive test coverage for all plugin functionality - Resolves PR subsy#344 review feedback (defensive test teardown, negative test cases, doc fixes)
oneilltomhq
force-pushed
the
feat/add-pi-agent-plugin
branch
from
df0f3d3 to
3afe6c8
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/plugins/agents/builtin/pi.ts (1)
119-165: ClearrunVersiontimeout once the process settlesAt Line 160, the timeout is never cleared on normal completion. This leaves a live timer and can call
kill()after exit. Please guard single-settlement and clear the timer in botherrorandclosepaths.Proposed hardening
private runVersion( command: string ): Promise<{ success: boolean; version?: string; error?: string }> { return new Promise((resolve) => { + let settled = false; + const finish = (result: { success: boolean; version?: string; error?: string }) => { + if (settled) return; + settled = true; + clearTimeout(timeoutId); + resolve(result); + }; const useShell = process.platform === 'win32'; const proc = spawn(useShell ? quoteForWindowsShell(command) : command, ['--version'], { stdio: ['ignore', 'pipe', 'pipe'], shell: useShell, }); @@ proc.on('error', (error) => { - resolve({ + finish({ success: false, error: `Failed to execute: ${error.message}`, }); }); proc.on('close', (code) => { if (code === 0) { const versionMatch = stdout.trim().match(/(\d+\.\d+\.\d+)/); - resolve({ + finish({ success: true, version: versionMatch?.[1], }); } else { - resolve({ + finish({ success: false, error: stderr || `Exited with code ${code}`, }); } }); - setTimeout(() => { + const timeoutId = setTimeout(() => { proc.kill(); - resolve({ success: false, error: 'Timeout waiting for --version' }); + finish({ success: false, error: 'Timeout waiting for --version' }); }, 15000); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/agents/builtin/pi.ts` around lines 119 - 165, The promise returned by the version-check routine can resolve multiple times because the 15s timeout is never cleared; update the logic in the function that spawns the process (the block creating proc, its 'error' and 'close' handlers and the setTimeout) to (1) store the timeout id in a variable, (2) use a single "settled" boolean to ensure resolve() is called only once, (3) clearTimeout(timeoutId) in both the proc.on('error', ...) and proc.on('close', ...) paths before resolving, and (4) before the timeout handler calls proc.kill() and resolve(), check the same "settled" flag so it no-ops if the process already finished. Reference the proc variable, the 'error' and 'close' handlers, and the setTimeout/kill logic when making the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugins/agents/builtin/pi.ts`:
- Around line 374-409: The onStdout handler assumes chunks contain whole JSONL
lines and drops partial JSON objects; fix by adding an incremental buffer that
persists across onStdout invocations for the same execute call (e.g., a closure
or instance field like stdoutJsonlBuffer) and update the callback to prepend the
buffer to the incoming data, split by '\n', keep the last partial line in
stdoutJsonlBuffer (instead of parsing it), and only JSON.parse complete lines to
call options.onJsonlMessage; after that feed the reconstructed full data into
parsePiOutputToEvents/processAgentEventsToSegments/processAgentEvents so
segmented and formatted outputs are not lost.
---
Nitpick comments:
In `@src/plugins/agents/builtin/pi.ts`:
- Around line 119-165: The promise returned by the version-check routine can
resolve multiple times because the 15s timeout is never cleared; update the
logic in the function that spawns the process (the block creating proc, its
'error' and 'close' handlers and the setTimeout) to (1) store the timeout id in
a variable, (2) use a single "settled" boolean to ensure resolve() is called
only once, (3) clearTimeout(timeoutId) in both the proc.on('error', ...) and
proc.on('close', ...) paths before resolving, and (4) before the timeout handler
calls proc.kill() and resolve(), check the same "settled" flag so it no-ops if
the process already finished. Reference the proc variable, the 'error' and
'close' handlers, and the setTimeout/kill logic when making the changes.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/plugins/agents/builtin/index.tssrc/plugins/agents/builtin/pi.tssrc/setup/skill-installer.tstests/plugins/pi-agent.test.tswebsite/content/docs/plugins/agents/pi.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/setup/skill-installer.ts
- website/content/docs/plugins/agents/pi.mdx
- Integrates Pi CLI with Ralph TUI for AI-assisted coding - Supports JSONL output for subagent tracing and detailed tool call breakdowns - Includes comprehensive test coverage for all plugin functionality - Resolves PR subsy#344 review feedback (defensive test teardown, negative test cases, doc fixes) Amp-Thread-ID: https://ampcode.com/threads/T-019ca340-073d-7198-94cf-51f1eb646f86 Co-authored-by: Amp <amp@ampcode.com>
oneilltomhq
force-pushed
the
feat/add-pi-agent-plugin
branch
from
3afe6c8 to
4924206
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/plugins/pi-agent.test.ts (2)
115-143: Make the “ignores invalid config” tests assert actual behaviour.At Line 115 onwards, these tests only check
isReady(), so they do not prove invalid values were ignored. Please assert observable outcomes (for example viabuildArgs) so these cases protect against config-regression bugs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/plugins/pi-agent.test.ts` around lines 115 - 143, The tests currently call plugin.initialize(...) and only assert plugin.isReady(), which doesn't verify that invalid config values were ignored; update each test (the ones calling plugin.initialize with invalid mode/thinking/model/timeout) to also call the observable API plugin.buildArgs() (or the equivalent public accessor used in this test suite) and assert its returned args reflect the expected default/previous valid values (e.g., default mode, default thinking level, original model string, default timeout) rather than the invalid input; keep the existing isReady() assertion but add a second assertion comparing plugin.buildArgs() to the expected args for each case so the test actually guards against config regressions.
43-51: Consider extracting cleanup into one helper and avoid blanket error swallowing.The repeated
afterEachblocks are near-identical, and the broadcatchcan hide genuine disposal regressions. A shareddisposeIfReady()helper would reduce duplication and keep failures visible.Also applies to: 227-235, 336-344
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/plugins/pi-agent.test.ts` around lines 43 - 51, Multiple tests repeat an afterEach that silently swallows disposal errors; extract a shared helper (e.g., disposeIfReady) that calls plugin.isReady() and then plugin.dispose(); replace the duplicate afterEach blocks to call disposeIfReady(), and avoid a blanket empty catch—either let errors surface or catch and rethrow/log them (use processLogger or throw) so genuine disposal failures are visible while still handling already-disposed state (check isReady before disposing).src/plugins/agents/builtin/pi.ts (1)
375-385: Parse each JSONL line once and reuse the parsed object.At Line 377 and Line 384, each line is decoded twice. A single parse path (reuse for both
onJsonlMessageand event mapping) would simplify error handling and reduce per-chunk overhead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/agents/builtin/pi.ts` around lines 375 - 385, Parse the JSON line once into a variable (e.g., parsedJson) and reuse it for both callbacks and event mapping: attempt JSON.parse(trimmed) once, if it succeeds call options.onJsonlMessage(parsedJson) and then pass the parsed object into this.parsePiJsonLine; if parse fails, fall back to calling this.parsePiJsonLine(trimmed) so behavior remains unchanged. Update parsePiJsonLine to accept either a string or Record<string, unknown> (or add an overload) and branch inside parsePiJsonLine to handle an already-parsed object versus a raw string to avoid double parsing and reduce per-chunk overhead, keeping existing error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugins/agents/builtin/pi.ts`:
- Around line 295-327: The code currently casts event.assistantMessageEvent and
message blocks without runtime validation; update the handling in the
assistantEvent switch (the assistantEvent variable and its eventType) to first
ensure eventType is a string, then for 'tool_use_start' only push events when
assistantEvent.name is a string and assistantEvent.input is a plain object (or
Record<string, unknown>), and for 'tool_use_end' behave as before; similarly, in
the message_end branch validate that message is an object with content as an
array and that each block is an object with type === 'text' and that block.text
is a string before pushing { type: 'text', content: text }; perform safe type
guards instead of unchecked casts around assistantEvent, event.message,
block.type, and block.text.
---
Nitpick comments:
In `@src/plugins/agents/builtin/pi.ts`:
- Around line 375-385: Parse the JSON line once into a variable (e.g.,
parsedJson) and reuse it for both callbacks and event mapping: attempt
JSON.parse(trimmed) once, if it succeeds call options.onJsonlMessage(parsedJson)
and then pass the parsed object into this.parsePiJsonLine; if parse fails, fall
back to calling this.parsePiJsonLine(trimmed) so behavior remains unchanged.
Update parsePiJsonLine to accept either a string or Record<string, unknown> (or
add an overload) and branch inside parsePiJsonLine to handle an already-parsed
object versus a raw string to avoid double parsing and reduce per-chunk
overhead, keeping existing error handling.
In `@tests/plugins/pi-agent.test.ts`:
- Around line 115-143: The tests currently call plugin.initialize(...) and only
assert plugin.isReady(), which doesn't verify that invalid config values were
ignored; update each test (the ones calling plugin.initialize with invalid
mode/thinking/model/timeout) to also call the observable API plugin.buildArgs()
(or the equivalent public accessor used in this test suite) and assert its
returned args reflect the expected default/previous valid values (e.g., default
mode, default thinking level, original model string, default timeout) rather
than the invalid input; keep the existing isReady() assertion but add a second
assertion comparing plugin.buildArgs() to the expected args for each case so the
test actually guards against config regressions.
- Around line 43-51: Multiple tests repeat an afterEach that silently swallows
disposal errors; extract a shared helper (e.g., disposeIfReady) that calls
plugin.isReady() and then plugin.dispose(); replace the duplicate afterEach
blocks to call disposeIfReady(), and avoid a blanket empty catch—either let
errors surface or catch and rethrow/log them (use processLogger or throw) so
genuine disposal failures are visible while still handling already-disposed
state (check isReady before disposing).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/plugins/agents/builtin/index.tssrc/plugins/agents/builtin/pi.tssrc/setup/skill-installer.tstests/plugins/pi-agent.test.tswebsite/content/docs/plugins/agents/pi.mdx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/setup/skill-installer.ts
- src/plugins/agents/builtin/index.ts
- website/content/docs/plugins/agents/pi.mdx
Extract parsePiJsonLine as a standalone exported function (matching the pattern used by parseKimiJsonLine) and add 20 test cases covering all three JSONL event types: message_update, message_end, turn_end. This closes the biggest test coverage gap identified during review — the parser is pure logic with no external dependencies and was previously untested. Coverage for pi.ts improved from 38% to 55.6%.
Owner
Review + Fixes PushedNice work on this plugin — it follows all the established patterns well. I've pushed one commit to address the main gap I found during review: What I fixedExported The JSONL parser was the riskiest untested code — it handles 3 event types (
What looked good (no changes needed)
All tests pass (65 total, up from 45), typecheck clean, build succeeds. |
added 2 commits
March 13, 2026 14:05
…e tests - Replace unchecked `as` casts in parsePiJsonLine with proper runtime type guards for assistantEvent, eventType, name, input, and content blocks. tool_use_start events are now safely skipped when name is not a string or input is not a plain object. - Accept string | Record<string, unknown> in parsePiJsonLine so callers can pass pre-parsed objects. processLines now parses JSON once and reuses the result for both onJsonlMessage and event mapping. - Extract shared disposeIfReady helper in tests, replacing three duplicate afterEach blocks with blanket empty catches. - Strengthen invalid-config tests to verify defaults are preserved via buildArgs output, not just isReady(). - Add 6 new parser tests for type guard edge cases (non-string name, non-object input, array input, non-string eventType, non-string text, pre-parsed object input). 71 tests pass, 105 assertions. Coverage for pi.ts: 58.6% lines.
Owner
Review feedback addressed (9ce4a57)Merged latest main and pushed fixes for all four findings: 1. Runtime type guards in
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #344 +/- ##
==========================================
+ Coverage 47.63% 47.89% +0.26%
==========================================
Files 104 105 +1
Lines 33923 34358 +435
==========================================
+ Hits 16160 16457 +297
- Misses 17763 17901 +138
🚀 New features to boost your workflow:
|
Pi agent docs existed but were unreachable — not listed in the sidebar navigation alongside the other agent plugins.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
2 participants
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.
Overview
Adds a new agent plugin for Pi Coding Agent, a minimal, extensible terminal coding agent.
Changes
src/plugins/agents/builtin/pi.tsindex.tsandskill-installer.tswebsite/content/docs/plugins/agents/pi.mdxFeatures
--print--model)--thinking)@filesyntaxChecklist
Summary by CodeRabbit
New Features
Documentation
Tests
Chores