Add Pi.dev Agent SDK support and clean up log event names#1100
Add Pi.dev Agent SDK support and clean up log event names#1100dammyammy wants to merge 4 commits intocoleam00:devfrom
Conversation
…ider Agent-Logs-Url: https://github.com/dammyammy/Archon/sessions/10aedf44-074e-4f29-8d9d-87b146927cdd Co-authored-by: dammyammy <3688497+dammyammy@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dammyammy/Archon/sessions/10aedf44-074e-4f29-8d9d-87b146927cdd Co-authored-by: dammyammy <3688497+dammyammy@users.noreply.github.com>
…port feat: add Pi.dev Agent SDK support as a first-class AI assistant provider
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request adds Pi as a third AI assistant provider alongside Claude and Codex. Changes include implementing a new PiClient class that interfaces with the Pi SDK, extending configuration types and schemas to support Pi settings, updating the client factory and workflow execution logic to recognize and route Pi providers, and documenting the integration in the README. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PiClient
participant AuthStorage
participant ModelRegistry
participant SessionManager as SessionManager<br/>(In-Memory)
participant PiSDK as Pi SDK<br/>(Agent Session)
participant CodingTools
participant EventQueue
User->>PiClient: sendQuery(prompt, cwd, options)
activate PiClient
PiClient->>AuthStorage: Get authentication
PiClient->>ModelRegistry: Resolve model if provided
PiClient->>SessionManager: Create in-memory session
PiClient->>CodingTools: Create coding tools for cwd
PiClient->>PiSDK: Create agent session with tools
PiClient->>PiSDK: session.prompt(prompt)
activate PiSDK
PiSDK->>EventQueue: Emit message_update events
PiSDK->>EventQueue: Emit tool start/end events
PiSDK->>EventQueue: Emit agent_end event
deactivate PiSDK
loop Process Events Until agent_end
PiClient->>EventQueue: Get next event
alt message_update (text/thinking)
EventQueue-->>PiClient: Delta chunk
PiClient-->>User: Yield assistant/thinking chunk
else tool event
EventQueue-->>PiClient: Tool invocation
PiClient-->>User: Yield tool chunk
PiClient-->>User: Yield tool_result chunk
else agent_end
PiClient-->>User: Stop yielding
end
end
PiClient->>PiSDK: Unsubscribe from events
PiClient->>SessionManager: Dispose session
PiClient-->>User: Yield result chunk
deactivate PiClient
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 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)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f258a4ad06
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| if (promptError) throw promptError; | ||
|
|
||
| yield { type: 'result' }; |
There was a problem hiding this comment.
This client always returns result without a sessionId and also treats resume IDs as unsupported, which makes Pi conversations effectively stateless across turns. In orchestrator-agent.ts, both stream and batch paths only persist session state when msg.type === 'result' && msg.sessionId, so Pi never stores or resumes context; users will lose conversational memory between messages (and workflow nodes/loops that rely on session continuation will restart fresh each time).
Useful? React with 👍 / 👎.
| "@archon/isolation": "workspace:*", | ||
| "@archon/paths": "workspace:*", | ||
| "@archon/workflows": "workspace:*", | ||
| "@mariozechner/pi-coding-agent": "^0.66.1", |
There was a problem hiding this comment.
A new direct dependency is added here, but neither bun.lock nor package-lock.json is updated in the commit. This breaks reproducible installs in CI because .github/workflows/test.yml installs with bun install --frozen-lockfile, and Bun's help explicitly says --frozen-lockfile disallows lockfile changes; with manifest/lock drift, dependency installation will fail before tests run.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds Pi.dev as a supported AI assistant provider across Archon’s config, workflow engine, and server API surface, and updates documentation to reflect the new option.
Changes:
- Introduce a new
PiClient(Pi.dev Agent SDK wrapper) and wire it into the assistant client factory. - Extend config and schema types to allow
pias an assistant/provider (core config, workflow schemas, server OpenAPI config schemas). - Update workflow validation/execution codepaths to accept
provider: piand document Pi usage in the README.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents Pi.dev setup/config and updates assistant list to include Pi. |
| packages/workflows/src/validator.ts | Adds warnings for Claude-only per-node features when provider is pi (and codex). |
| packages/workflows/src/schemas/workflow.ts | Allows provider: pi at workflow level. |
| packages/workflows/src/schemas/dag-node.ts | Allows provider: pi at node level. |
| packages/workflows/src/model-validation.ts | Treats Pi as model-unrestricted in compatibility checks. |
| packages/workflows/src/loader.ts | Parses pi as a valid provider from YAML. |
| packages/workflows/src/executor.ts | Expands resolved provider type to include pi (provider resolution logic). |
| packages/workflows/src/deps.ts | Extends workflow config/deps types to include pi and assistants.pi. |
| packages/workflows/src/dag-executor.ts | Adds Pi option mapping (model-only) and expands provider unions across execution helpers. |
| packages/server/src/routes/schemas/config.schemas.ts | Extends OpenAPI schemas to include pi assistant config. |
| packages/server/src/routes/api.ts | Accepts pi in assistant config PATCH payload and merges into updates. |
| packages/core/src/services/title-generator.ts | Updates doc comment to include pi as an assistant type. |
| packages/core/src/config/config-types.ts | Adds Pi assistant defaults and extends assistant unions to include pi. |
| packages/core/src/config/config-loader.ts | Adds Pi defaults/merging/env override support; extends safe config projection. |
| packages/core/src/clients/pi.ts | New Pi.dev Agent SDK client implementation. |
| packages/core/src/clients/index.ts | Re-exports PiClient. |
| packages/core/src/clients/factory.ts | Adds pi branch to assistant factory + updates supported-types error message. |
| packages/core/package.json | Adds @mariozechner/pi-coding-agent dependency. |
| packages/cli/src/commands/setup.ts | Updates types to allow pi as a default assistant (setup flow changes incomplete). |
Comments suppressed due to low confidence (1)
packages/cli/src/commands/setup.ts:684
defaultAssistantnow allows'pi', butcollectAIConfig()only ever sets up Claude/Codex and the default-assistant prompt only offers those two options. As written, users cannot select Pi viaarchon setup, and the'pi'union is effectively unreachable. Either add Pi to the assistant selection + default-choice flow (and any needed install/auth instructions) or keep this type limited to the assistants that setup can configure.
let defaultAssistant: 'claude' | 'codex' | 'pi' = 'claude';
if (hasClaude && hasCodex) {
const defaultChoice = await select({
message: 'Which should be the default AI assistant?',
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| claude: { ...current.assistants?.claude, ...updates.assistants.claude }, | ||
| codex: { ...current.assistants?.codex, ...updates.assistants.codex }, | ||
| pi: { ...current.assistants?.pi, ...updates.assistants.pi }, |
There was a problem hiding this comment.
updateGlobalConfig() deep-merge spreads optional assistant updates directly (e.g. ...updates.assistants.pi). If the PATCH body updates only one assistant (common case), the other assistant objects will be undefined and spreading them will throw a TypeError at runtime. Use nullish-coalescing to spread {} (or conditionally merge per-key) so partial updates are safe.
| claude: { ...current.assistants?.claude, ...updates.assistants.claude }, | |
| codex: { ...current.assistants?.codex, ...updates.assistants.codex }, | |
| pi: { ...current.assistants?.pi, ...updates.assistants.pi }, | |
| claude: { ...current.assistants?.claude, ...(updates.assistants.claude ?? {}) }, | |
| codex: { ...current.assistants?.codex, ...(updates.assistants.codex ?? {}) }, | |
| pi: { ...current.assistants?.pi, ...(updates.assistants.pi ?? {}) }, |
| async *sendQuery( | ||
| prompt: string, | ||
| cwd: string, | ||
| resumeSessionId?: string, | ||
| options?: AssistantRequestOptions | ||
| ): AsyncGenerator<MessageChunk> { | ||
| if (resumeSessionId) { | ||
| getLog().warn( | ||
| { resumeSessionId }, | ||
| 'pi.session_resume_unsupported' | ||
| ); | ||
| } | ||
|
|
||
| // Check if already aborted | ||
| if (options?.abortSignal?.aborted) { | ||
| throw new Error('Query aborted'); | ||
| } | ||
|
|
There was a problem hiding this comment.
PiClient does not apply the env-leak gate that ClaudeClient/CodexClient enforce before allowing an assistant to operate on a registered codebase. This creates an inconsistent security posture where selecting pi could bypass the existing protection against leaking sensitive keys from auto-loaded env files. Mirror the same scanPathForSensitiveKeys + allowTargetRepoKeys/allow_env_keys checks used in the other clients before starting a Pi session.
| "@archon/isolation": "workspace:*", | ||
| "@archon/paths": "workspace:*", | ||
| "@archon/workflows": "workspace:*", | ||
| "@mariozechner/pi-coding-agent": "^0.66.1", | ||
| "@openai/codex-sdk": "^0.116.0", | ||
| "pg": "^8.11.0", |
There was a problem hiding this comment.
New dependency was added but the repo’s lockfiles don’t appear to be updated (no pi-coding-agent entry in bun.lock or package-lock.json). This can break reproducible installs/CI. Please regenerate and commit the lockfile updates for the package manager(s) this repo uses.
| case 'pi': | ||
| getLog().debug({ provider: 'pi' }, 'client_selected'); | ||
| return new PiClient(); | ||
| default: | ||
| throw new Error(`Unknown assistant type: ${type}. Supported types: 'claude', 'codex'`); | ||
| throw new Error(`Unknown assistant type: ${type}. Supported types: 'claude', 'codex', 'pi'`); | ||
| } |
There was a problem hiding this comment.
Factory error message and supported-type list changed to include pi, but existing unit tests still assert the old message (“Supported types: 'claude', 'codex'”) and there’s no coverage for the new pi branch. Update/add tests accordingly so CI doesn’t fail and the new provider is covered.
| botName: z.string(), | ||
| assistant: z.enum(['claude', 'codex']), | ||
| assistant: z.enum(['claude', 'codex', 'pi']), | ||
| assistants: z.object({ | ||
| claude: z.object({ model: z.string().optional() }), | ||
| codex: z.object({ |
There was a problem hiding this comment.
The OpenAPI config schema now includes pi in assistant and assistants, which will change /api/openapi.json. The frontend derives types from packages/web/src/lib/api.generated.d.ts; that file should be regenerated (and committed if it’s tracked) to stay in sync with the updated schema.
| // Queue for bridging event callbacks to async generator | ||
| const chunks: MessageChunk[] = []; | ||
| let notifyNext: (() => void) | null = null; | ||
| let done = false; | ||
| let promptError: Error | undefined; | ||
|
|
||
| const notify = (): void => { | ||
| const fn = notifyNext; | ||
| notifyNext = null; | ||
| fn?.(); | ||
| }; | ||
|
|
||
| const enqueue = (chunk: MessageChunk): void => { | ||
| chunks.push(chunk); | ||
| notify(); | ||
| }; | ||
|
|
||
| // Subscribe to Pi SDK events and map them to Archon MessageChunk types | ||
| const unsubscribe = session.subscribe((event: AgentSessionEvent) => { | ||
| if (event.type === 'message_update') { | ||
| const ae = event.assistantMessageEvent; | ||
| if (ae.type === 'text_delta') { | ||
| enqueue({ type: 'assistant', content: ae.delta }); | ||
| } else if (ae.type === 'thinking_delta') { | ||
| enqueue({ type: 'thinking', content: ae.delta }); | ||
| } | ||
| } else if (event.type === 'tool_execution_start') { | ||
| // event.args is typed `any` in pi-agent-core — normalize to Record<string, unknown> | ||
| const toolInput: Record<string, unknown> = | ||
| event.args !== null && typeof event.args === 'object' | ||
| ? (event.args as Record<string, unknown>) | ||
| : {}; | ||
| enqueue({ | ||
| type: 'tool', | ||
| toolName: event.toolName, | ||
| toolInput, | ||
| toolCallId: event.toolCallId, | ||
| }); | ||
| } else if (event.type === 'tool_execution_end') { | ||
| // event.result is typed `any` in pi-agent-core — serialize to string | ||
| const toolOutput = event.isError | ||
| ? `Error: ${serializeToolResult(event.result)}` | ||
| : serializeToolResult(event.result); | ||
| enqueue({ | ||
| type: 'tool_result', | ||
| toolName: event.toolName, | ||
| toolOutput, | ||
| toolCallId: event.toolCallId, | ||
| }); | ||
| } else if (event.type === 'agent_end') { | ||
| done = true; | ||
| notify(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
PiClient introduces non-trivial event-to-MessageChunk mapping and async-generator queueing, but there are no accompanying unit tests (unlike Claude/Codex clients). Add tests that simulate Pi session events to verify streaming order, tool/tool_result pairing, abort behavior, and generator completion/error propagation.
| // Resolve provider and model once (used by all nodes) | ||
| // When workflow sets a model but not a provider, infer provider from the model. | ||
| // e.g. model: sonnet → provider: claude, even if config.assistant is codex. | ||
| let resolvedProvider: 'claude' | 'codex'; | ||
| // Pi workflows should always set provider: pi explicitly (pi models cannot be auto-inferred). | ||
| let resolvedProvider: 'claude' | 'codex' | 'pi'; |
There was a problem hiding this comment.
The comment says Pi workflows should set provider: pi explicitly, but the resolution logic below still infers a provider from workflow.model and will treat any non-Claude model string as codex. That means a Pi-style provider/model-id (e.g. openai/gpt-4o) will currently resolve to codex unless workflow.provider is set, which is likely incorrect. Consider adding explicit detection for Pi model strings (or enforcing provider: pi when a slash-form model is used) so Pi configs don’t accidentally route to Codex.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/workflows/src/dag-executor.ts (1)
380-393:⚠️ Potential issue | 🟠 MajorPreserve
piwhen a node only overridesmodel.These branches still treat every non-Claude explicit
modelas Codex. In a Pi workflow, a node likemodel: "openai/gpt-4o"withoutprovider: pigets rerouted to Codex, then either fails compatibility checks or uses the wrong client.🔧 Minimal fix
- } else if (node.model && isClaudeModel(node.model)) { - provider = 'claude'; - } else if (node.model) { - provider = 'codex'; + } else if (node.model) { + provider = workflowProvider === 'pi' ? 'pi' : isClaudeModel(node.model) ? 'claude' : 'codex'; } else {Apply the same change to the loop-node inference block.
Also applies to: 2609-2623
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/workflows/src/dag-executor.ts` around lines 380 - 393, The provider-choosing logic incorrectly forces any explicit non-Claude node.model to 'codex', breaking Pi workflows; update the provider assignment in the shown block (the logic using node.provider, node.model, isClaudeModel, provider, workflowProvider) and the loop-node inference block (the other occurrence referenced) so that: if node.provider is present use it; else if isClaudeModel(node.model) set provider='claude'; otherwise leave provider as workflowProvider (do not default to 'codex'); keep computing model with node.model ?? (provider === workflowProvider ? workflowModel : config.assistants[provider]?.model). Apply the same change to the loop-node inference block at the other occurrence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/setup.ts`:
- Line 48: The type union for defaultAssistant includes 'pi' but the setup flow
in collectAIConfig only offers 'claude' and 'codex', causing an impossible
selection; either remove 'pi' from the defaultAssistant type or add a
selectable/configuration branch for Pi in the wizard. Locate the
defaultAssistant declaration and the collectAIConfig function (assistant
selection logic) and either (A) revert the union to 'claude' | 'codex' to match
the UI, or (B) add a 'pi' option in the assistant selection prompt and implement
the Pi-specific configuration branch (prompt fields, validation, and returned
config) so the wizard can produce a Pi-first configuration end-to-end.
In `@packages/core/src/clients/pi.ts`:
- Around line 84-89: The code currently ignores resumeSessionId and returns a
terminal chunk without a sessionId; change the logic in the Pi client call
handlers (where resumeSessionId is read — e.g., the block using getLog() and the
surrounding request/response functions) to fail fast: detect a non-empty
resumeSessionId and immediately throw/reject with a clear error (and log it via
getLog().error) stating that "Pi session resume is unsupported" instead of
creating a fresh in-memory session or returning a terminal chunk without
sessionId; apply the same change to the other resumeSessionId checks in this
file (the other occurrences around the 124–129 and 218 locations) so callers
requiring resume support receive an explicit error until IAssistantClient-style
session IDs and streaming are implemented.
- Around line 101-118: The current block that parses options.model logs warnings
via getLog().warn ('pi.model_format_invalid' / 'pi.model_not_found') and leaves
model undefined, letting createAgentSession() auto-select a backend; instead,
validate options.model and fail fast: if options.model lacks a '/' or
modelRegistry.find(provider, modelId) returns falsy, throw a clear Error
(including the provided options.model and the provider/modelId where applicable)
rather than logging and continuing so createAgentSession() cannot proceed with
an implicit default.
- Around line 205-213: The generator's wait logic can lose a wake-up because it
sets notifyNext after checking chunks; change the wait so you create/set
notifyNext (the resolve callback) before awaiting and then re-check the
condition immediately after setting notifyNext to avoid the race. Concretely, in
the async generator loop that uses chunks, notifyNext and done, assign
notifyNext = resolve (or build the pending promise) first, then after awaiting
(or before awaiting return to event loop) re-evaluate chunks.length and done—if
chunks were added or done became true, resolve the promise immediately (or skip
awaiting) so the generator never hangs waiting for a lost notification.
In `@packages/workflows/src/dag-executor.ts`:
- Around line 499-503: The current pi branch silently drops all node options
except model (the provider === 'pi' branch that sets options = model ? { model }
: undefined), which hides unsupported settings like maxBudgetUsd, output_format,
hooks, MCP, skills, and tool restrictions; change this to detect any of these
Pi-incompatible fields on the incoming node (e.g., maxBudgetUsd, output_format,
hooks, mcp, skills, tools) and fail fast by throwing a clear error or at minimum
logging an explicit warning that includes the node identifier and the
unsupported fields, instead of silently stripping them—update the provider ===
'pi' handling to validate the node options before setting options and raise an
error when any unsupported option is present so callers cannot proceed with
silently dropped behavior.
In `@packages/workflows/src/model-validation.ts`:
- Around line 11-15: The Pi branch currently returns true untested; keep the
permissive behavior but add unit tests for isModelCompatible(provider: 'pi',
...) to cover realistic Pi IDs (e.g., "gpt-4o", "gpt-4o:high", "llama3.1:8b",
and an empty string) to assert non-empty strings return true and empty/undefined
return true per existing behavior for missing model but false if you decide to
treat empty string as invalid; update or add tests referencing isModelCompatible
to ensure Pi cases are covered alongside 'claude' and 'codex' cases so future
changes don't regress this behavior.
In `@packages/workflows/src/validator.ts`:
- Around line 338-349: The MCP check currently warns for unsupported providers
but earlier MCP file/path/content validation still pushes errors; update the
validation flow in validator.ts so that when provider === 'codex' or provider
=== 'pi' the MCP file validation is skipped or its errors are downgraded to
warnings. Specifically, in the MCP validation logic that adds error entries to
issues (look for the code that inspects node.mcp/mcpPath and pushes level:
'error'), add a guard referencing provider or reuse the existing provider ===
'codex' || provider === 'pi' condition to either return early or change those
pushes to level: 'warning' (include node.id and field: 'mcp' in the warnings to
match the existing warning block).
In `@README.md`:
- Around line 360-362: The README example uses an invalid provider value
"anthropic"; update the example so the provider field uses a valid
workflow/provider value (one of "claude", "codex", or "pi") and ensure the
accompanying model string in the model field matches that provider (e.g., change
provider to "claude" and adjust model from "anthropic/claude-opus-4-5" to the
corresponding allowed model format) while leaving the prompt field ("prompt")
intact.
---
Outside diff comments:
In `@packages/workflows/src/dag-executor.ts`:
- Around line 380-393: The provider-choosing logic incorrectly forces any
explicit non-Claude node.model to 'codex', breaking Pi workflows; update the
provider assignment in the shown block (the logic using node.provider,
node.model, isClaudeModel, provider, workflowProvider) and the loop-node
inference block (the other occurrence referenced) so that: if node.provider is
present use it; else if isClaudeModel(node.model) set provider='claude';
otherwise leave provider as workflowProvider (do not default to 'codex'); keep
computing model with node.model ?? (provider === workflowProvider ?
workflowModel : config.assistants[provider]?.model). Apply the same change to
the loop-node inference block at the other occurrence.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0f05f3e4-b8d1-4b7c-9661-909d1acf9ad1
📒 Files selected for processing (19)
README.mdpackages/cli/src/commands/setup.tspackages/core/package.jsonpackages/core/src/clients/factory.tspackages/core/src/clients/index.tspackages/core/src/clients/pi.tspackages/core/src/config/config-loader.tspackages/core/src/config/config-types.tspackages/core/src/services/title-generator.tspackages/server/src/routes/api.tspackages/server/src/routes/schemas/config.schemas.tspackages/workflows/src/dag-executor.tspackages/workflows/src/deps.tspackages/workflows/src/executor.tspackages/workflows/src/loader.tspackages/workflows/src/model-validation.tspackages/workflows/src/schemas/dag-node.tspackages/workflows/src/schemas/workflow.tspackages/workflows/src/validator.ts
| codex: boolean; | ||
| codexTokens?: CodexTokens; | ||
| defaultAssistant: 'claude' | 'codex'; | ||
| defaultAssistant: 'claude' | 'codex' | 'pi'; |
There was a problem hiding this comment.
pi is typed as selectable, but the setup wizard cannot actually select/configure it.
defaultAssistant now includes 'pi', but collectAIConfig() only offers Claude/Codex. This leaves setup unable to produce a Pi-first configuration through the intended flow.
Please either add a Pi option/branch in the assistant selection flow, or defer this union widening until the wizard supports it end-to-end.
Also applies to: 680-680
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/setup.ts` at line 48, The type union for
defaultAssistant includes 'pi' but the setup flow in collectAIConfig only offers
'claude' and 'codex', causing an impossible selection; either remove 'pi' from
the defaultAssistant type or add a selectable/configuration branch for Pi in the
wizard. Locate the defaultAssistant declaration and the collectAIConfig function
(assistant selection logic) and either (A) revert the union to 'claude' |
'codex' to match the UI, or (B) add a 'pi' option in the assistant selection
prompt and implement the Pi-specific configuration branch (prompt fields,
validation, and returned config) so the wizard can produce a Pi-first
configuration end-to-end.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| if (resumeSessionId) { | ||
| getLog().warn( | ||
| { resumeSessionId }, | ||
| 'pi.session_resume_unsupported' | ||
| ); | ||
| } |
There was a problem hiding this comment.
Pi session continuity isn't implemented yet.
Each call creates a fresh in-memory session, ignores resumeSessionId, and returns a terminal chunk without a sessionId. The workflow executor uses session continuity for shared-context nodes, retries, and interactive loop resumes, so Pi runs will silently lose context on those paths. At minimum, fail fast when a caller needs resume support until real session IDs are wired through.
Based on learnings, packages/core/src/clients/**/*.{ts,tsx} should implement AI clients using IAssistantClient with session management and streaming support.
Also applies to: 124-129, 218-218
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/clients/pi.ts` around lines 84 - 89, The code currently
ignores resumeSessionId and returns a terminal chunk without a sessionId; change
the logic in the Pi client call handlers (where resumeSessionId is read — e.g.,
the block using getLog() and the surrounding request/response functions) to fail
fast: detect a non-empty resumeSessionId and immediately throw/reject with a
clear error (and log it via getLog().error) stating that "Pi session resume is
unsupported" instead of creating a fresh in-memory session or returning a
terminal chunk without sessionId; apply the same change to the other
resumeSessionId checks in this file (the other occurrences around the 124–129
and 218 locations) so callers requiring resume support receive an explicit error
until IAssistantClient-style session IDs and streaming are implemented.
| if (options?.model) { | ||
| const slashIdx = options.model.indexOf('/'); | ||
| if (slashIdx > 0) { | ||
| const provider = options.model.slice(0, slashIdx); | ||
| const modelId = options.model.slice(slashIdx + 1); | ||
| model = modelRegistry.find(provider, modelId); | ||
| if (!model) { | ||
| getLog().warn( | ||
| { model: options.model }, | ||
| 'pi.model_not_found' | ||
| ); | ||
| } | ||
| } else { | ||
| getLog().warn( | ||
| { model: options.model }, | ||
| 'pi.model_format_invalid' | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/core/src/clients/pi.ts | head -150Repository: coleam00/Archon
Length of output: 6189
🏁 Script executed:
wc -l packages/core/src/clients/pi.tsRepository: coleam00/Archon
Length of output: 94
Throw an error when the model string is malformed or not found in the registry; do not silently fall back to Pi's default.
When a user explicitly provides an invalid model string (missing / separator) or the model cannot be found in the registry, the code currently logs a warning and continues. The model variable remains undefined, which is omitted from createAgentSession() on line 128, causing Pi to auto-select a backend instead. This violates the "Fail Fast + Explicit Errors" coding guideline and makes it difficult to diagnose why the wrong model was used. Throw a clear error in both cases instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/clients/pi.ts` around lines 101 - 118, The current block
that parses options.model logs warnings via getLog().warn
('pi.model_format_invalid' / 'pi.model_not_found') and leaves model undefined,
letting createAgentSession() auto-select a backend; instead, validate
options.model and fail fast: if options.model lacks a '/' or
modelRegistry.find(provider, modelId) returns falsy, throw a clear Error
(including the provided options.model and the provider/modelId where applicable)
rather than logging and continuing so createAgentSession() cannot proceed with
an implicit default.
| try { | ||
| while (!done || chunks.length > 0) { | ||
| if (chunks.length > 0) { | ||
| yield chunks.shift()!; | ||
| } else { | ||
| await new Promise<void>(resolve => { | ||
| notifyNext = resolve; | ||
| }); | ||
| } |
There was a problem hiding this comment.
The async queue can miss a wake-up and hang.
A Pi event can enqueue a chunk after the chunks.length check but before notifyNext = resolve. In that case the notification is lost and this generator can wait forever even though data is already queued or done is already true.
🐛 Minimal fix
if (chunks.length > 0) {
yield chunks.shift()!;
} else {
await new Promise<void>(resolve => {
notifyNext = resolve;
+ if (done || chunks.length > 0) {
+ notify();
+ }
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| while (!done || chunks.length > 0) { | |
| if (chunks.length > 0) { | |
| yield chunks.shift()!; | |
| } else { | |
| await new Promise<void>(resolve => { | |
| notifyNext = resolve; | |
| }); | |
| } | |
| try { | |
| while (!done || chunks.length > 0) { | |
| if (chunks.length > 0) { | |
| yield chunks.shift()!; | |
| } else { | |
| await new Promise<void>(resolve => { | |
| notifyNext = resolve; | |
| if (done || chunks.length > 0) { | |
| notify(); | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/clients/pi.ts` around lines 205 - 213, The generator's wait
logic can lose a wake-up because it sets notifyNext after checking chunks;
change the wait so you create/set notifyNext (the resolve callback) before
awaiting and then re-check the condition immediately after setting notifyNext to
avoid the race. Concretely, in the async generator loop that uses chunks,
notifyNext and done, assign notifyNext = resolve (or build the pending promise)
first, then after awaiting (or before awaiting return to event loop) re-evaluate
chunks.length and done—if chunks were added or done became true, resolve the
promise immediately (or skip awaiting) so the generator never hangs waiting for
a lost notification.
| } else if (provider === 'pi') { | ||
| // Pi supports model (in "provider/model-id" format) and nothing else from the Archon | ||
| // workflow options — features like hooks, MCP, skills, output_format, and env injection | ||
| // are Claude/Codex-specific and are silently skipped for Pi. | ||
| options = model ? { model } : undefined; |
There was a problem hiding this comment.
Don't silently drop Pi-incompatible node options.
This branch strips everything except model, so maxBudgetUsd, output_format, hooks, MCP, skills, and tool restrictions quietly stop working when a node switches to Pi. That is a big behavior change compared to the Codex path above, which at least warns. Please reject unsupported combinations or surface explicit warnings before execution.
As per coding guidelines, "Prefer throwing early with a clear error for unsupported/unsafe states - never silently swallow errors or broaden permissions (Fail Fast + Explicit Errors)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/workflows/src/dag-executor.ts` around lines 499 - 503, The current
pi branch silently drops all node options except model (the provider === 'pi'
branch that sets options = model ? { model } : undefined), which hides
unsupported settings like maxBudgetUsd, output_format, hooks, MCP, skills, and
tool restrictions; change this to detect any of these Pi-incompatible fields on
the incoming node (e.g., maxBudgetUsd, output_format, hooks, mcp, skills, tools)
and fail fast by throwing a clear error or at minimum logging an explicit
warning that includes the node identifier and the unsupported fields, instead of
silently stripping them—update the provider === 'pi' handling to validate the
node options before setting options and raise an error when any unsupported
option is present so callers cannot proceed with silently dropped behavior.
| export function isModelCompatible(provider: 'claude' | 'codex' | 'pi', model?: string): boolean { | ||
| if (!model) return true; | ||
| // Pi supports any model via its multi-provider architecture — no restrictions apply. | ||
| if (provider === 'pi') return true; | ||
| if (provider === 'claude') return isClaudeModel(model); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In @mariozechner/pi-coding-agent, what exact string format is accepted for the modeloption? Isprovider/model-id required, and are there documented exceptions?
💡 Result:
The model option in @mariozechner/pi-coding-agent (CLI flag --model) accepts model patterns or IDs in the format provider/model-id (e.g., openai/gpt-4o) or just a model ID/pattern (e.g., gpt-4o). The provider/model-id format is supported but not strictly required; the tool performs fuzzy matching against available models from the registry (built-in + custom from ~/.pi/agent/models.json). It parses provider prefixes (split by /), strips redundant prefixes when --provider is also specified, and handles optional :thinking-level suffixes (e.g., gpt-4o:high). No documented exceptions beyond standard fuzzy matching rules: exact ID match first, then partial ID/name. Custom models (e.g., Ollama llama3.1:8b) work via models.json and support colons in IDs post-fixes. Use --list-models to verify available models. Examples from docs: - pi --provider openai --model gpt-4o - pi --model openai/gpt-4o - pi --model gpt-4o (infers provider)
Citations:
- 1: https://www.npmjs.com/package/@mariozechner/pi-coding-agent
- 2: https://registry.npmjs.org/%40mariozechner%2Fpi-coding-agent
- 3: https://github.com/badlogic/pi-mono/blob/main/packages/coding-agent/README.md
- 4: https://github.com/badlogic/pi-mono/blob/refs/heads/main/packages/coding-agent/README.md
- 5: https://github.com/badlogic/pi-mono/tree/main/packages/coding-agent
- 6: https://github.com/badlogic/pi-mono/tree/main/packages/coding-agent/docs/models.md
- 7: https://github.com/badlogic/pi-mono/blob/main/packages/coding-agent/docs/models.md
- 8: https://github.com/badlogic/pi-mono/blob/5133697b/packages/coding-agent/src/core/model-resolver.ts
🏁 Script executed:
# Search for Pi model usage and validation patterns
rg "provider.*===.*['\"]pi['\"]" packages/workflows/src/ -A 5 -B 2Repository: coleam00/Archon
Length of output: 4421
🏁 Script executed:
# Check how isModelCompatible is used
rg "isModelCompatible" packages/workflows/src/ -B 2 -A 3Repository: coleam00/Archon
Length of output: 9883
🏁 Script executed:
# Search for any existing Pi model validation or error handling
rg -i "pi.*model\|model.*pi" packages/workflows/src/ --type ts -A 3 -B 1Repository: coleam00/Archon
Length of output: 41
🏁 Script executed:
# Check if there's downstream model validation in executor/loader
rg "model.*validat\|validat.*model" packages/workflows/src/ --type ts -A 2 -B 1Repository: coleam00/Archon
Length of output: 41
🏁 Script executed:
# Check for Pi model test coverage
cat packages/workflows/src/model-validation.test.ts | grep -A 20 "describe('isModelCompatible'"Repository: coleam00/Archon
Length of output: 1041
🏁 Script executed:
# Look for any Pi-specific error handling or documentation
rg "pi.*model\|model.*pi" packages/workflows/src/model-validation.ts --type ts -i -B 3 -A 3Repository: coleam00/Archon
Length of output: 41
🏁 Script executed:
# Check dag-executor comment context more carefully
sed -n '1,100p' packages/workflows/src/dag-executor.ts | head -80Repository: coleam00/Archon
Length of output: 2243
🏁 Script executed:
# Search for any tests or validation of Pi models anywhere in the codebase
rg "provider.*pi\|pi.*provider" --type ts -C 3 | grep -i "test\|spec\|model" | head -30Repository: coleam00/Archon
Length of output: 41
Add Pi model validation tests and enforce a supported format.
The if (provider === 'pi') return true; branch currently bypasses validation entirely. While the principle of early validation is sound, the suggested regex is overly restrictive: the Pi SDK accepts bare model IDs (e.g., gpt-4o), :thinking-level suffixes (e.g., gpt-4o:high), and custom models with colons (e.g., llama3.1:8b)—not just provider/model-id format.
Two approaches:
- Permissive: Keep accepting any non-empty string (Pi SDK performs fuzzy matching and validation), but add test cases for Pi models to ensure coverage matches Claude/Codex.
- Restrictive: Document and enforce Archon's supported Pi format explicitly (e.g.,
provider/model-idper the dag-executor comment), then update the regex accordingly and add tests.
Currently there are zero tests for isModelCompatible('pi', ...), which masks this gap.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/workflows/src/model-validation.ts` around lines 11 - 15, The Pi
branch currently returns true untested; keep the permissive behavior but add
unit tests for isModelCompatible(provider: 'pi', ...) to cover realistic Pi IDs
(e.g., "gpt-4o", "gpt-4o:high", "llama3.1:8b", and an empty string) to assert
non-empty strings return true and empty/undefined return true per existing
behavior for missing model but false if you decide to treat empty string as
invalid; update or add tests referencing isModelCompatible to ensure Pi cases
are covered alongside 'claude' and 'codex' cases so future changes don't regress
this behavior.
| // Warn if using MCP with Codex or Pi | ||
| if (provider === 'codex' || provider === 'pi') { | ||
| issues.push({ | ||
| level: 'warning', | ||
| nodeId: node.id, | ||
| field: 'mcp', | ||
| message: 'MCP servers are Claude-only per-node — this will be ignored on Codex', | ||
| hint: 'For Codex, configure MCP servers globally in ~/.codex/config.toml instead', | ||
| message: `MCP servers are Claude-only per-node — this will be ignored on ${provider}`, | ||
| hint: provider === 'codex' | ||
| ? 'For Codex, configure MCP servers globally in ~/.codex/config.toml instead' | ||
| : 'For Pi, MCP is not supported per-node. Use Pi extensions for equivalent functionality', | ||
| }); | ||
| } |
There was a problem hiding this comment.
MCP is marked “ignored” for Pi/Codex, but this path can still fail validation as an error.
This block warns that MCP is unsupported on ${provider}, yet earlier in the same branch missing/invalid MCP files still produce error issues. For unsupported providers, validation should not hard-fail on MCP file content/path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/workflows/src/validator.ts` around lines 338 - 349, The MCP check
currently warns for unsupported providers but earlier MCP file/path/content
validation still pushes errors; update the validation flow in validator.ts so
that when provider === 'codex' or provider === 'pi' the MCP file validation is
skipped or its errors are downgraded to warnings. Specifically, in the MCP
validation logic that adds error entries to issues (look for the code that
inspects node.mcp/mcpPath and pushes level: 'error'), add a guard referencing
provider or reuse the existing provider === 'codex' || provider === 'pi'
condition to either return early or change those pushes to level: 'warning'
(include node.id and field: 'mcp' in the warnings to match the existing warning
block).
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary
Describe this PR in 2-5 bullets:
PiClientimplementingIAssistantClient, propagated the'pi'provider type across all 19 files (config, workflows, schemas, server, CLI), added validator warnings for Claude-only features, and documented setup inREADME.md.bun.lockandapi.generated.d.tsare intentionally not updated in this PR (see Validation Evidence).UX Journey
Before
After
Architecture Diagram
Before
After
Connection inventory:
config-types.tsconfig-loader.tsPiAssistantDefaults,'pi'to all union typesconfig-loader.tsconfig-types.tspidefaults, env overrideDEFAULT_AI_ASSISTANT=pifactory.tspi.tscase 'pi': return new PiClient()factory.tsclaude.ts/codex.tspi.ts@mariozechner/pi-coding-agentpi.tstypes/index.tsIAssistantClientdeps.tsconfig-types.tsAssistantClientFactoryandWorkflowConfigaccept'pi'dag-executor.tsdeps.tsvalidator.tsschemas/dag-node.tsmodel-validation.tsschemas/dag-node.tsisModelCompatible('pi', _) → trueschemas/dag-node.tsprovider: z.enum([..., 'pi'])schemas/workflow.tsprovider: z.enum([..., 'pi'])config.schemas.ts'pi'api.tsconfig-loader.tsbody.pisetup.ts'pi'title-generator.tsindex.ts(clients)pi.tsPiClientREADME.mdLabel Snapshot
risk: lowsize: Mcore,workflows,server,cli,config,docscore:clients,core:config,workflows:executor,workflows:validator,server:api,cli:setupChange Metadata
featuremultiLinked Issue
Validation Evidence (required)
Commands and result summary:
bun run type-checkbunnot available in sandboxed environment. All type annotations are mechanically consistent (verified by manual code review of all 19 files).bun run lintbun run format:checkbun run testbun run validateWhy all skipped: The PR was authored in a sandboxed environment (GitHub Copilot Workspace) that does not have
buninstalled. Thebun.lockfile was also not updated for the same reason (the@mariozechner/pi-coding-agent@^0.66.1dependency is declared inpackage.jsonbut not yet resolved in the lockfile).Post-merge required steps:
bun install— resolve new dependency and updatebun.lockbun run dev:serverthenbun --filter @archon/web generate:types— regenerateapi.generated.d.tsbun run validate— full validation passEvidence provided:
Security Impact (required)
IAssistantClientinterface with the same tool set (read/bash/edit/write viacreateCodingTools)ANTHROPIC_API_KEY,OPENAI_API_KEY, etc.) or OAuth auth stored at~/.pi/agent/auth.json. Archon does not handle these directly.createCodingTools(cwd)scopes file tools to the samecwdas Claude/CodexRisk and mitigation: The external network calls are inherent to any LLM provider integration. Pi SDK handles authentication and API communication — Archon delegates to it the same way it delegates to the Claude Agent SDK and Codex SDK.
Compatibility / Migration
claudeandcodexconfigurations are unchanged.'pi'is additive.DEFAULT_AI_ASSISTANTenv var now accepts'pi'. Config files acceptdefaultAssistant: piandassistants.pi.model. All optional, no migration required.Human Verification (required)
Verified scenarios:
'claude' | 'codex'type unions existed are now'claude' | 'codex' | 'pi'(grep-verified)PiClient.sendQuery()correctly implements theAsyncGenerator<MessageChunk>interface via queue-based adaptersession.abort()and cleans up event listener infinallyunsubscribe(),session.dispose(), abort listener removal) infinallyblockprovider === 'pi'dag-executor.tsPi branch correctly passes only{ model }(skipping all Claude/Codex-specific options)model-validation.tsreturnstruefor all models whenprovider === 'pi'(Pi supports any provider's models)Edge cases checked:
resumeSessionIdprovided to Pi → logs warning, starts fresh session (Pi uses in-memory sessions)options.modelwithout/separator → logspi.model_format_invalidwarningoptions.modelwith valid format but unknown model → logspi.model_not_foundwarning, proceeds with Pi defaultoptions.abortSignalalready aborted beforesendQuery→ throws'Query aborted'immediatelysession.prompt()throws → error captured, re-thrown after generator drains remaining chunksWhat was not verified:
bunin environment)Side Effects / Blast Radius (required)
Affected subsystems/workflows:
pi: {}to defaults, merge, env override, safe projection)piin request/response schemas)Potential unintended effects:
provider: claudeorprovider: codexare completely unaffectedproviderstill default toclaude(unchanged behavior)provider: piand Claude-only features (hooks, MCP, skills) will run but those features are silently skipped (validator warns at validation time)Guardrails/monitoring for early detection:
client.pinamespace for all Pi-specific eventsRollback Plan (required)
git revert <merge-commit-sha>— single commit revert removes all 19 file changes cleanlydefaultAssistant: piorprovider: pi. No existing user is affected unless they configure Pi.bun installnot run). Config validation errors ifapi.generated.d.tsis not regenerated.Risks and Mitigations
bun.locknot updated —bun installwill fail if Pi SDK has unresolvable dependencies@mariozechner/pi-coding-agentis a published npm package. Runbun installimmediately after merge to verify.api.generated.d.tsout of sync — web client TypeScript types won't include Pibun --filter @archon/web generate:typesafter merge. Failure is a build-time error, not runtime.Summary by CodeRabbit
New Features