Skip to content

feat: add support for the pi.dev coding agent - #240

Merged
agent-era-ai merged 4 commits into
mainfrom
pi-dev-agent-support
May 17, 2026
Merged

agent-era-ai merged 4 commits into
mainfrom
pi-dev-agent-support

Conversation

@agent-era-ai

@agent-era-ai agent-era-ai commented May 17, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Registers pi (the pi.dev coding agent, binary pi, npm @earendil-works/pi-coding-agent) as a fourth first-class AI tool alongside claude/codex/gemini.

  • Tool registration — new AI_TOOLS.pi entry (--continue resume, processPatterns: ['pi']) and a CONFIG_SCHEMA.aiToolSettings.pi flags block. The AITool type, availability detection, picker, and launch path are all data-driven off AI_TOOLS, so pi is selectable and launchable with no further wiring.
  • Status detection — PI_WORKING_RE (braille spinner) and PI_WAITING_RE / PI_PERMISSION_RE (pi's select/confirm dialog chrome plus the pi-permission-system extension's gate), with explicit case 'pi' in isWorking/isWaitingForTool. Patterns derived from real captured pi v0.74.1 output, not docs.
  • Detection hardening — detectToolFromArgs's loose .includes() fallback is replaced with whole-word matching, so the 2-char name pi can't mis-tag pip, compile, or paths merely containing pi.
  • Fixtures / tests / docs — real captured tests/fixtures/ai-states/pi/* fixtures, detection-test coverage, the capture-ai-states skill extended for pi, and glossary/status-model docs updated.

Notes

  • pi has no built-in permission gate (it auto-runs bash/edits by default); its only agent-blocked "waiting" state comes from the optional pi-permission-system extension, which renders as a standard pi select dialog. Detection covers both pi's native interactive dialogs and that extension's prompt.

Verification

npm run typecheck, npm test (815/815), npm run build all pass. Verified end-to-end: launched pi via the real bash -c pi --continue || pi path and confirmed detectAllSessionAITools tags the session pi.

🤖 Generated with Claude Code

agent-era-ai and others added 3 commits May 17, 2026 04:32
Register `pi` (pi.dev) as a fourth AI tool: AI_TOOLS entry, config schema
flags block, and idle/working/waiting status detection from real captured
pi v0.74.1 terminal output.

pi's spinner is a braille glyph (`⠙ Working...`) and its waiting states are
select-dialog UIs — detection uses regex (PI_WORKING_RE / PI_WAITING_RE)
like claude rather than plain substrings. Waiting also covers the
pi-permission-system extension's gate, pi's only agent-blocked prompt.

Also harden detectToolFromArgs: the loose `.includes()` fallback is replaced
with whole-word matching so the 2-char name `pi` can't mis-tag `pip`,
`compile`, or paths merely containing `pi`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Match the PI_WORKING_RE / PI_WAITING_RE convention so the pattern isn't
recompiled on every isWaitingForTool call in the ~2s status-refresh loop.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@agent-era-ai agent-era-ai changed the title pi dev agent support feat: add support for the pi.dev coding agent May 17, 2026
@claude

claude Bot commented May 17, 2026

Copy link
Copy Markdown

Code Review — PR #240: Add support for pi.dev coding agent

Overview

Well-executed addition of pi as a fourth first-class AI tool. The PR follows the established codex/gemini pattern throughout, and the detectToolFromArgs hardening (replacing loose .includes() with word-boundary matching) is a genuinely important fix that benefits the entire tool family.


Positives

  • Real captured fixtures — idle/working/waiting states are from live pi v0.74.1 output, not docs guesses. The README explaining how to regenerate waiting.txt (which requires the pi-permission-system extension) is a good practice.
  • Detection hardening is well-motivated — TOOL_WORD_RES + \b boundaries prevent the 2-char name pi from matching pip, compile, paths containing pi, etc. The old test case 'someweirdtoolnameclaudethingembedded' → 'claude' is correctly removed.
  • Regex comments justify the non-obvious constraints — the note about excluding U+2800 (blank braille) in PI_WORKING_RE and the explanation of what PI_WAITING_RE keys on are genuinely warranted.
  • Comprehensive unit test coverage — the new "Pi status detection" describe block covers working, waiting (both patterns), mixed state priority, and idle. The fixture-driven matrix test in ai-tool-detection.test.ts gets pi added cleanly.
  • FakeAIToolService.isAIPaneCommand correctly uses /\bpi\b/ rather than .includes('pi') — consistent with the hardening approach.
  • Data-driven design pays off — AITool type, picker, availability detection all pick up pi with no extra wiring.

Concerns

1. processPatterns: ['pi'] — confirm matching semantics

The constant sets processPatterns: ['pi'] for a 2-char name. The whole motivation for TOOL_WORD_RES in detectToolFromArgs is that a substring match on 'pi' would mis-tag pip, compile, etc. The same risk exists for processPatterns if pattern matching is a substring scan against ps -o args= output. The PR notes the end-to-end test confirmed detection works, but it's worth documenting — or asserting in a test — what the matching logic for processPatterns actually is (substring vs. whole-word vs. exact binary name).

2. Comments are borderline multi-line — AGENTS.md prefers single line

AGENTS.md says: "When a comment is genuinely warranted … prefer a single line." The regex comments in AIToolService.ts run 4–5 lines each. The WHY is genuinely non-obvious (braille glyph ranges, dialog footer text), so these aren't gratuitous — but consider whether they can be tightened, especially the PI_WAITING_RE block.

3. FakeAIToolService has no case 'pi' in status simulation

Only isAIPaneCommand was updated in the fake. The fake's status switch (used by E2E tests to simulate ai_status) has no case 'pi', so pi sessions in integration tests will always report the default branch result. Depending on how the fake's default is implemented, this might silently leave pi sessions stuck at idle in E2E scenarios. Worth a quick check — if it's intentional, a comment noting it is fine.

4. Minor: '─' as idle_prompt

The idle_prompt: ['─'] in the pi statusPatterns entry (a single box-drawing character from the separator line) is used differently from other tools' prompts. The comment explains that real detection is in PI_WORKING_RE/PI_WAITING_RE, not statusPatterns. That's fine, but the comment references the implementation internals in a way that will rot — consider just noting why statusPatterns is minimal here rather than pointing at named symbols.


Summary

Approve with minor follow-up. The core implementation is solid — real fixtures, hardened detection, good tests, data-driven registration. The processPatterns substring-safety question is the only one that could produce a real bug; the rest are style/maintenance nits. If processPatterns matching is already word-bounded or exact, this is good to merge.

- FakeAIToolService: add a `case 'pi'` to the status simulation switch so
  pi is not the only tool falling through to the default branch
- isAIPaneCommand: add a pi test documenting that it matches the pane heuristic
- tighten the PI_WAITING_RE comment to a single rationale line

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented May 17, 2026

Copy link
Copy Markdown

Code Review: feat: add support for the pi.dev coding agent

Overview

This PR registers pi as a fourth first-class AI tool alongside claude/codex/gemini. The implementation is data-driven, follows the established codex/gemini integration pattern, and includes a meaningful improvement to detectToolFromArgs that benefits all short-named tools.


Code Quality ✅

Strengths:

  • Data-driven integration: AITool type, getAvailableTools, detectAvailableAITools, and the picker all pick up pi automatically via AI_TOOLS — no ad-hoc wiring needed.
  • Detection hardening is the right call: replacing argsLower.includes(tool) with TOOL_WORD_RES (\bname\b) in the fallback is a net improvement for all tools, not just pi. The old loose fallback was a latent bug for any 2–4 char tool name.
  • Real captures used for patterns: PI_WORKING_RE and PI_WAITING_RE are derived from actual pi v0.74.1 output rather than documentation guesses. The braille-glyph spinner anchor ([⠁-⣿]) correctly excludes U+2800 (blank braille) to avoid false matches on the literal word "Working".
  • Mixed-state priority test (permission picker on screen wins over the working spinner): good catch — tested explicitly.

Minor comments:

  • src/constants.ts — the pi entry has multi-line inline comments (// pi's working/waiting detection lives in…). Per project conventions, comments should be reserved for non-obvious WHY; the constant itself being named statusPatterns and the downstream code being in AIToolService is self-evident. Consider dropping these comment blocks.
  • tracker/items/pi-dev-agent-support/ — these planning/discovery docs are useful context but are internal tracking artifacts. They're fine to keep if that's a project convention, but they add ~200 lines of non-executable content to the diff.

Potential Issues

1. processPatterns: ['pi'] word-boundary risk

The same substring ambiguity fixed in detectToolFromArgs may exist in wherever processPatterns is matched against ps output. If process detection does a .includes('pi') against args strings like spi, pip, or paths containing api, the 2-char pattern could false-match. Worth checking how processPatterns is consumed (likely in TmuxService or WorktreeCore) — if it uses a substring test, apply the same \b-bounded matching there.

2. PI_WORKING_RE requires an ASCII space/tab after the braille glyph

The regex /^[ \t]*[⠁-⣿][ \t]/m requires [ \t] (space or tab) immediately after the spinner character. The captured fixture (⠙ Working...) confirms a space is present, but pi could theoretically use a Unicode thin space or NBSP in a future version. This is low-risk but worth noting as a maintenance surface.

3. TOOL_TOKEN_RES lookahead (?=\\s|$) doesn't account for a trailing single-quote

The strict pass strips quoted spans before matching, so this is likely already handled — but worth a quick sanity-check with the test case bash -c pi --continue 'fix…' || pi 'fix…' to confirm stripped is what the comment implies.


Performance ✅

All regex constants are compiled at module load time (correct). No changes to the 2s polling hot path. TOOL_NAMES iteration is O(n) over 4 tools — not a concern.


Security ✅

No security concerns. No network calls, no user input passed to shell commands. The regex patterns cannot produce ReDoS since they contain no nested quantifiers.


Test Coverage ✅

The test suite is thorough:

  • Five targeted status-detection unit tests covering all states and the mixed-state edge case.
  • Word-boundary detection cases (pip install, embedded substring, path) explicitly cover the hardening.
  • Fixture-driven matrix test (ai-tool-detection.test.ts) covers idle/working/waiting against real captures.
  • FakeAIToolService correctly uses /\bpi\b/ for isAIPaneCommand (consistent with the hardening).
  • Updating the 'legacy embedded substring' test to assert 'none' is the right call — it documents the intentional behavior change.

Summary

Approve with one follow-up to check: The implementation is solid. The single thing worth verifying before merge is how processPatterns is consumed — if it uses a substring match, it needs the same \b treatment applied to detectToolFromArgs. Everything else looks good.

@agent-era-ai
agent-era-ai merged commit 81ee349 into main May 17, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant