Skip to content

Add Pi structured native chat through its RPC mode - #25851

Merged
brennanb2025 merged 433 commits into
mainfrom
brennanb2025/native-chat-pi-rpc
Oct 8, 2026
Merged

brennanb2025 merged 433 commits into
mainfrom
brennanb2025/native-chat-pi-rpc

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 42 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​4121 $\color{#cf222e}{\Huge{\mathbf{−}}}$​11 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​4110
Prod 73 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3871 $\color{#cf222e}{\Huge{\mathbf{−}}}$​309 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3562

ELI5

Pi chats currently use a terminal. With “Use updated structured native chat” enabled in Settings > Chat UI, supported Pi installations now open native chat: answers stream into messages, tools show their results, extension questions can be answered, and Stop ends the work. Turning the setting off keeps the existing terminal chats, including the OpenCode and OMP transcript readers.

Supported: stable @earendil-works/pi-coding-agent from 0.84.0 (0.84 and later 0.x releases, and 1.x), the releases that report when a turn has fully settled; recordings and live QA used 1.0.4. Older Pi, including @mariozechner/pi-coding-agent 0.73.x, keeps the terminal chat. Orca asks the execution host's own Pi binary for its version before creating a structured chat and again right before starting it, through the same version check and Command setting OpenCode's structured chat uses, so your Command setting picks the Pi that is checked and run.

What Changed

Pi uses its own --mode rpc interface. A reusable JSON-lines transport owns one supervised process and builds the existing chat history. Input, queued writes, pending prompts and final history writes have finite limits. Prompt delivery continues while an extension asks a question; steering and follow-ups travel with the prompt. A turn finishes after Pi reports settlement and an idle check confirms that streaming, compaction and queued messages have finished.

Pi’s session file is the resume handle. A native conversation can continue in Pi’s terminal interface and then resume natively from the same file. Resuming in another folder forks that file through Pi. Pi entries that originated only in terminal history still resume in the terminal; native chat renders Orca’s existing history and does not import prior terminal messages. Model and thinking choices, image prompts, usage, retry failures and extension dialogs use the existing chat surfaces. Stop sends abort, requests a bounded checkpoint, then terminates through the shared supervisor.

An extension input dialog the user answers with nothing (Pi accepts an empty answer) now shows "Empty answer" on its resolved card. Before, the card read "Selected answer unavailable", the wording for an answer Orca cannot read, because the card treated an empty text as no answer. An answer that names neither text nor a known option still reads as unavailable.

Built on #25845 (merged). Pi's version check and binary lookup now use the shared ones #25845 added (agent-cli-version-probe.ts, structured-agent-command-resolution.ts); Pi's own copies (rpc-version.ts, rpc-create-support.ts) are deleted, and Pi's registration lives in src/main/pi/rpc-runtime-registration.ts.

The existing registered-agent routing and setting gate are reused. Clients that cannot answer Pi dialogs are not offered Pi structured sessions; cleanup stays available. The current mobile client does not advertise Pi. Windows launches require the same process identity proof as other supervised providers. Folder workspaces use the host’s existing path resolver.

Why

The native RPC interface follows the common pattern and avoids an extra protocol adapter. The transport and history lane can support another agent later; this integration adds Pi, with OMP left out. It builds on the Grok/ACP integration from #25225, which is now on main; this branch carries only Pi's change on top of it.

Differences from the common pattern:

  • Intended: Pi runs with full access only. No supervised permission mode or approval-policy extension is injected. Pi extension questions still use existing question and approval cards.
  • Intended: settlement plus an idle check protects against compaction, retries and queued input outliving an earlier end event.
  • Intended: Pi’s own session store enables terminal/native context resume; native history remains in Orca’s existing chat store. Existing cards offer editable editor prefill and explicit empty answers through optional metadata.
  • Intended: an oversized protocol record closes the connection, so a dropped reply or completion cannot silently strand the session. The shared supervisor owns group/tree termination and preserves uncertainty about descendants separately from observed root exit.
  • Intended: Pi and the Agent Client Protocol (Grok) now share one stdin write queue. A request that times out while still waiting in the queue is dropped and only that request fails, as before. A request that times out while its line is already being written now closes the connection (previously Grok's queue left it in place): the line cannot be taken back, and leaving it would hold every later write, Stop's cancel included, behind an agent that stopped reading.
  • Intended: current mobile and older paired clients are withheld Pi until they support its launch and dialog contract. Direct WSL structured launch remains unsupported; the existing terminal route remains available.
  • Intended: Pi older than 0.84.0 keeps the terminal chat. The common pattern also requires a minimum Pi release (0.84.0 is accepted by it), because only newer Pi reports when a turn has fully settled.

Linked Issue

Internal coordinated native-chat integration; no new issue opened.

Visual Proof

Live QA in a hidden Orca build of this branch on macOS, with a private Pi 1.0.4 (@earendil-works/pi-coding-agent) signed in through a Codex account. Every launch ran with its own home, profile and agent folders, and the real ~/.codex, ~/.claude, ~/.orca, ~/.grok and ~/.pi files were hashed before and after every check: no change came from the test app.

Check Result Screenshot
Setting off: Pi opens the existing terminal chat Pass P1
Setting on: structured Pi chat, streamed reply, the shell command's row (folded into the finished turn) Pass P2
Stop while a command runs: interrupted, the command's file never written, the next prompt continues the same Pi session Pass P3
A second message sent while Pi works is queued and delivered once Pass P4
Extension dialogs: confirm Yes / No, select, an empty input (now reads "Empty answer"), and an edited editor text that Pi received as edited Pass P5
Restart Orca, reopen the chat: Pi still remembers the conversation and keeps writing to the same Pi session file Pass P6

Not verified live: an older Pi (0.73.x) keeping the terminal chat (no older Pi was installed on the test machine; the version check is unit-tested), Windows, Linux, SSH and mobile. Checks 1-4 ran at e54c521, checks 5-6 at 4c25ad0; the commits between them are a main merge and the empty-answer fix. Not re-run live after the merge with #25845 and the move to the shared version check; Pi 0.84.x was not run live (unit-tested).

Testing

  • I manually tested these changes locally.
  • Automated tests added/updated.

Scrubbed recordings of the exact supported Pi 1.0.4 package were replayed through the existing chat history implementation: prompts/tools, delayed prompt replies with and without request IDs, steering/follow-ups, Stop, provider errors, signed-out state, extension dialogs and compaction. All assigned Pi capture cells are complete.

From live QA: an explicitly empty (or whitespace-only) input answer now reads "Empty answer" (NativeChatResolutionReceipt.empty-answer.test.tsx, fails with the old receipt); the editor dialog submits the text the user edited rather than its prefill (NativeChatStructuredQuestionCard.test.tsx); a chat reopened after an Orca restart, with a fresh record store reading the journal back, launches Pi with --session on the stored session file (rpc-launch-resolution.test.ts).

Focused checks passed: 62 tests in four version/launch files, 20 tests in three location/adapter/registry files, 120 tests in twelve files covering the earlier main merge and last messages under history-write pressure, and 68 tests in eight files covering the latest reasoning/settlement merge, Pi capture replay and unchanged provider contracts. Earlier full integration and protocol review snapshots passed 230 tests in 22 files and 104 tests in twelve files respectively; these are separate runs. Scoped lint, formatting and changed-code/design-system checks passed.

Final head f3be80a4f9c (origin/main with #25845 merged in): both typechecks pass locally; 74 test files (516 tests) covering the changed and dependent code pass, plus the Pi version tests for the new 0.84.0 minimum; lint and the changed-code quality gate pass. Not run locally: the 4 mobile test files (mobile config; no mobile code changed in the merge). Earlier head 4c25ad02e4c passed all 19 applicable CI checks.

Review

Fresh protocol and integration reviewers, an architecture challenger and narrow follow-up reviewers checked the change. Findings were fixed and tracked in a private ledger. Final readiness was reviewed against the complete diff without the counsel step, as requested.

An existing shared shutdown path waits for chat-history writes before releasing ownership. If a write stalls, bookkeeping can delay opening the chat again. That pre-existing cross-provider issue is deferred to a separate fix; this PR adds no new ownership-release wait.

Agent skill upstream boundary

  • Not applicable; no skill-installer source, tests, fixtures, registry or path tables copied.

Notes

The execution host owns the session file, account environment, process and status. No OMP integration, new transcript store, global extension install, custom tool injection, permission hook, rewind or thread-goal feature is included. The optional extension-specific OpenRouter token-budget hook is absent because this lane injects no extension.

Author (X): @BrennanKB5.

Checklist

  • This PR is focused on Pi structured chat and its required shared transport/lifecycle support.
  • I explained the user-facing before/after, mechanism and approach.
  • Before/after visual evidence attached (see Visual Proof).
  • Self-reviewed correctness, security and performance.
  • Cross-platform, SSH/remote, mobile compatibility and path impact considered.
  • Final CI typechecks, tests and builds pass; CI is the authority.

…embler' into brennanb2025/acp-c5-tool-interrupted
A call still running when its turn ends takes the state of that turn's
row: a row another writer settled first (a person's Stop) stands, so its
calls read interrupted whatever the provider's later end reports. The
no-ending path that settled calls from the Stop row is gone, since a Stop
now leaves running calls to the provider. Adds the two Spanish strings.
A failed Grok turn ended with no reason on screen: the translator dropped
every copy of Grok's message. The failed turn now gets one status row in
Orca's existing "provider did not accept this message" words with Grok's
reason, read from whichever copy arrives first (the given-up retry, the
turn's end, the prompt's completion notice, or the prompt's error answer);
later copies only fill a reason the row still lacks.

A running background command no longer reads "Background task <id>
started": a task's summary is mapped only once it has settled. A monitor
stays a monitor when the agent reads its output: a frame that names no
kind keeps the known one, and a "[monitor" command is a monitor.

A prompt's turn is marked started, so a late frame for an ended prompt
neither reopens it nor becomes the active turn. A tool's turn is held in
one place at a time.
A turn that started and then failed was told "The provider did not accept
this message", Orca's sentence for a message refused before its turn. The
row now reads as a Codex turn-ending error does: an error status row with the
provider's own words. With no words, the dialect names the failure ("Grok
ended this turn with an error." / "Grok usage limit reached."), else the
agent's display name does.
…start too

The restart sweep ended every running call by the death evidence alone, so after
a person's Stop with no proof the child died the call read failed under a turn
that read interrupted. The sweep and the live dead-generation settlement now ask
the same rule the assembler does: a call in a turn already settled ends as that
row ended; only a turn still running leaves its calls to the evidence.
…mit what they claim

A StructuredAgentRegistry, built once from the {definition, adapter}
registrations, is now a required host dependency and the router routes with
it. The adapter interface loses its optional router-only capabilities?() and
definition?(); every reader (options at rest, thread goal, rewind, the
/compact handover) asks the registry. A live session still narrows rewind
through the adapter, and the host combines declared and narrowed in one
helper. The registry refuses a registration that declares compact, a thread
goal or rewind without the adapter method behind it.

/compact is admitted by the declared capability, so an agent that declares
compact:false gets the commandRefused fact instead of a thrown error.

Also: the cut-turn notice names the agent from the catalog, create-support
builds the account home through agentSessionAccountHome, and the turn
status text older clients read names the session's agent instead of
defaulting to Codex (Claude/Codex text unchanged).
…otocol client's own types

The translator now reads a permission request with the client's lenient reader, a session update
with its session-event reader, and takes only the agent's own error answer as a failed prompt's
reason, so an Orca-side error never reads as the provider's words. Tests cover protocol values
newer than this build.
The router holds the registry, so its rewindSupport answers the owner's
declared rewind narrowed by the adapter, in one place no reader can bypass.
The host-side combining helper is gone; readers ask the adapter they hold.
…ry' into brennanb2025/acp-a3-wire

# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts
#	src/main/runtime/orca-runtime-get-structured-agent-session-create-support.ts
#	src/main/runtime/structured-agent-session-runtime.ts
The adoption replay test builds its host over this build's registered agents
instead of an adapter definition method, and the stored-form test passes the
stored agents the record guard now requires.
…he desktop

The desktop renderer calls the restart methods as a paired runtime client
without the registered-agents capability, so it was handed a Claude/Codex
audience and its "dismiss all" took the scoped path: no dismissal fence and
no clearing of unwritten teardown witnesses, so a late teardown write could
bring a dismissed offer back.

The audience is now derived against this host's registry: a client that can
show every agent the host registered gets none, and dismisses exactly as
before (one fence for every offer). A client that truly cannot show some
agent still gets a final dismissal for what it sees: each cleared session is
fenced on its own (bounded, superseded by a dismiss-all fence) and only the
witnesses of agents it sees are dropped.

One predicate now answers which agents a client renders, for both tab
projection and restart offers; the Claude capability rule is folded in.
…'s chats

A record was readable only if every handle used the transport its agent's
definition declares today, and its account variable was declared by some
registered agent. A later build that drives the same agent over another
protocol, or under a renamed variable, would have set every existing chat of
that agent aside: no tab, no history, no error.

Readability now asks only that the record agree with itself: its agent is
registered, every handle shares one namespace owned by that agent, and the
account variable is a well-formed name. Whether this build can drive it (the
chain's transport is the one the agent speaks, and the account variable is
that agent's own, since it becomes the child's environment) is decided in
performAttach, the one admission every start of every agent passes through,
and refused there as hostUnsupported. A record pinning another agent's
variable is refused the same way rather than passing because some agent
declares it.
… and which account it pins

createSupport, create and the model catalog's account read still chose a
location rule and an account resolver by name (Claude, Codex, else none),
so a new agent would have needed a third branch beside the registration
list that already decides storage, routing and publication.

Each runtime registration now carries supportsLocation(location) and
resolveAccountHomePath({ launchEnv, location, purpose, workspacePath }),
and Claude's and Codex's rules move into their entries unchanged: a read
still syncs no home and starts no bridge, a Codex launch still trusts the
workspace first, and WSL locations resolve as before. The list exists
before the host is built, so none of these reads installs the host, and an
agent the list does not hold is answered no without opening the journal.
…f the capsule state

A spread input already carries the registry, so the explicit key was
overwritten (TS2783). The session-fence helper now returns only the fence,
not the capsule state it was given.
The per-session dismissal fence added a forever-persisted capsule field
with an arbitrary cap that served no caller: only the local desktop calls
the restart methods, and on a host whose agents it all shows it gets no
audience and takes the unscoped, fenced dismissal. A scoped dismiss-all
(a caller that cannot show some registered agent) keeps that agent's
offers, clears only its audience's unwritten witnesses, and writes no
fence; a late write from this process is serialized behind it. The field
never left this unreleased branch.
… own

The start check judges the record's agent while the router starts the adapter of params.agent, and the attach wire let the two differ. Admission now refuses agent !== provider as requestMalformed, so the agent checked is the agent started. Every host-built attach already sets them equal.
…ccept it

The restart offer, a failure's retryable flag and the pre-send check asked only whether the adapter runs the chat's location, while the start also refuses a record this build cannot drive; such a chat was offered Resume and its Retry failed forever. One host predicate, hostCanStartRecord (location support and agentDrivesSession), now answers all four; adapterSupportsRecord stays the reader gate, and an undrivable chat's offer is kept, not retired.
…nly if this build drives it

A record-scoped catalog read started the agent's lister under the record's account path whatever variable the record pinned. It now uses that path only when agentDrivesSession holds, and otherwise resolves the account as for a read with no record.
…say where transport is checked

The store only ever asked whether an agent is registered, so its admission list is now the registered ids; the definition is the one home of an agent's transport and account variable. Comments that said the record store checks transport, and one that cited a nonexistent function, now point at agentDrivesSession and the attach admission. The isPersistedAgentSessionRecord(value, agents) call shape and the test fixture the cross-version probe imports are kept.
…hat-pi-rpc

# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-agent-session-dead-generation-settlement.ts
#	src/main/runtime/orca-runtime-get-worktree-ps.ts
#	src/main/runtime/structured-agent-session-runtime.ts
@brennanb2025

brennanb2025 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor Author

Pi chats previously ran through the terminal. With the updated structured native-chat setting on, supported Pi 1.x now streams messages and tools into native chat, answers extension dialogs through existing cards, and stops through the shared process supervisor. The setting off and older/unknown Pi versions keep terminal chat; Pi runs with full access only.

Fresh reviews and an architecture challenge led to fixes in bounded writes, message-queue ordering and retries, crash reporting, dialogs, cancellation and last history writes under pressure. Pi audience checks cover content, mutations, status and completion events; older clients and current mobile are withheld Pi. Native-origin Pi files can continue in the terminal and then resume natively with that context. Terminal-only history entries retain terminal resume; arbitrary history adoption and OMP are outside this work. The branch includes the approved Grok/ACP dependency from #25225.

The floor is stable @earendil-works/pi-coding-agent 1.x, recorded at 1.0.4. Temporary: older Pi line (agent_end-only) support awaits a captured follow-up with a guarded fallback. An existing shared shutdown path can let stalled chat-history bookkeeping delay opening the chat again; that pre-existing P2 is deferred to a separate cross-provider fix. This change adds no ownership-release wait.

Verification: recorded supported-version flows replay through the existing history implementation. 62 version/launch tests, 20 support/lifecycle tests, 120 earlier-main overlap/admission/provider-contract tests and 68 newest-main reasoning/settlement/Pi overlap tests passed in separate focused runs. Fresh version review found no proven defect; final readiness was completed without counsel as requested. Current head 4c25ad02e4ca487b99648abc2f35743dadf23a8c (empty input answers read "Empty answer"; origin/main merged) passed all 19 applicable checks (16 skipped), including typechecks, builds, all five unit-test shards, cross-version wire checks, mobile checks and macOS/Linux/Windows ownership contracts: CI run, first attempt. Earlier c410 CI required retries after an unchanged terminal-history timer assertion failed and a subsequent runner shut down; no unrelated test or production code was changed.

Live app QA is BLOCKED: no authorized UI driver is available, and the coordinator has held further launches. The earlier hidden M4Air app startup wrote managed Codex hooks into real ~/.codex outside its test profile; the driver stopped only its owned process group and restored nothing. No Pi user flow or screenshot was completed. Rendered desktop/mobile and real Pi on Windows/WSL/SSH remain unverified. All assigned Pi captures are delivered, with no capture blocker. This remains a draft pending that evidence.

… RPC

Main's squash of the Grok ACP change is authoritative; this branch keeps only Pi's own change on top of it, including the provider-neutral start tracking.
Main added prompt alerts to the turn-completion stream; the merge extended Pi's audience filter to them.
… being written

A line still waiting in the queue is dropped and only its request fails, as before. A line already handed to the stream cannot be taken back and would hold every later write, a cancel included, so that timeout closes the connection; tests now cover both for the shared queue and for ACP.
Its host id is a branded type; the literal only checks under the wire type, as the completion fixture beside it does.
It opens a real SQLite record store, which main's new boundary check now requires to run on Node.
…n file

A fresh record store reading the journal back from disk must hand the launch the session file the chat proved before the restart, and the spawn must carry it as --session.
…able

Live QA: answering a Pi input dialog with nothing (which Pi accepts) left the resolved card reading "Selected answer unavailable", the wording for an answer Orca cannot read. The receipt joined the answer's labels and text and turned an empty result into "no answer". It now keeps an answer whose text is present but empty (or whitespace) as an empty answer, shown as "Empty answer"; an answer naming neither text nor a known option still reads as unavailable.

Also pins that the editor dialog submits the text the user edited, not its prefill.
Pi's runtime registration moves to its own module in main's registration
shape (resolveAccountHome, supportsLaunch), so createSupport's Pi version
check runs through main's create-support composition instead of a Pi branch
in the runtime class. agentSession.agents reads main's static registrations
and still hides Pi from clients without the Pi dialog capability. Pi's
question card forwards main's answer-input ref.
…n probe and command resolver

Pi's own `--version` runner and PATH lookup are gone. Create support and
every launch now resolve the binary through resolveStructuredAgentCommand,
so the Pi Command setting picks the binary both check and spawn, and an
unrunnable Command is refused at launch with agentCommandNotRunnable as for
the other agents. The version check runs through probeAgentCliVersion with
the same rule as before: stable 1.x of the current package; the older 0.x
package keeps the terminal chat. Pi's launch reads the shared per-agent
environment overlay and Command settings instead of Pi-only runtime deps.
@brennanb2025
brennanb2025 marked this pull request as ready for review October 8, 2026 00:03
@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

Walkthrough

This change adds Pi structured-session support through a new JSONL-RPC transport, Pi launch and session adapters, runtime registration, and client capability gating. It also adds shared provider-process queue and acquisition helpers, updates ACP to use them, and adds timeline final-tail handling. In parallel, it extends question schemas with freeTextInput metadata and uses it in shared helpers, mobile UI, renderer UI, and receipt rendering so prompts can preserve empty or whitespace answers when allowed and can carry initial text, placeholders, and multiline settings.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: 🟡 Moderate · up to f3be8

Closing a Pi native chat session normally can fail with an error because the close result is misread. This can break chat cleanup and reopening. The fix is a one-line condition change plus a test, and it should be made before merging.

🚥 Pre-merge checks | ✅ 4 | ❓ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ❓ Inconclusive Docstring coverage is 19.05% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 42 functions across 50 files. (65 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: adding Pi structured native chat through RPC mode.
Description check ✅ Passed The description is detailed and covers the required sections, including user impact, implementation, rationale, visual proof, testing, compatibility, review notes, and checklist status. It states that…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 19.05% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 42 functions across 50 files. (65 skipped: 14 unsupported, 51 over the file limit.)

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fdb6dc11-0ea3-4878-a95f-e01f5a1283ff
📥 Commits

Reviewing files that changed from the base of the PR and between a551aa5 and f3be80a.

📒 Files selected for processing (115)
  • config/scripts/check-runtime-electron-ratchet.mjs
  • config/scripts/check-runtime-electron-ratchet.test.mjs
  • config/scripts/vitest-sqlite-runtime-files.mjs
  • mobile/src/session/MobileNativeChatQuestion.test.tsx
  • mobile/src/session/MobileNativeChatQuestion.tsx
  • mobile/src/session/mobile-native-chat-question.test.ts
  • mobile/src/session/mobile-native-chat-question.ts
  • mobile/src/session/mobile-structured-agent-prompts-free-text.test.ts
  • mobile/src/session/mobile-structured-agent-prompts.ts
  • mobile/src/session/mobile-structured-grouped-question.test.ts
  • mobile/src/session/mobile-structured-grouped-question.ts
  • src/main/acp/acp-json-rpc-peer.test.ts
  • src/main/acp/acp-stdio-error-boundary.ts
  • src/main/acp/acp-structured-session-adapter.ts
  • src/main/acp/acp-write-queue.ts
  • src/main/agent-launch/agent-launch-executor.test.ts
  • src/main/ipc/desktop-renderer-runtime-capabilities.ts
  • src/main/jsonl-rpc/agent-connection.test.ts
  • src/main/jsonl-rpc/agent-connection.ts
  • src/main/jsonl-rpc/peer-limits.ts
  • src/main/jsonl-rpc/peer.test.ts
  • src/main/jsonl-rpc/peer.ts
  • src/main/jsonl-rpc/timeline-final-tail.test.ts
  • src/main/jsonl-rpc/timeline-lane.test.ts
  • src/main/jsonl-rpc/timeline-lane.ts
  • src/main/native-chat/agent-session-journal/journal-prompt-body-bounds.ts
  • src/main/native-chat/agent-session-timeline/provider-timeline-assembler.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-event-sink-queue.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-final-tail-reservation.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-final-tail-reservation.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-transition.ts
  • src/main/pi/__fixtures__/command-only.jsonl
  • src/main/pi/__fixtures__/compaction.jsonl
  • src/main/pi/__fixtures__/dialogs.jsonl
  • src/main/pi/__fixtures__/provider-fetch-error.jsonl
  • src/main/pi/__fixtures__/provider-socket-error.jsonl
  • src/main/pi/__fixtures__/signed-out.jsonl
  • src/main/pi/__fixtures__/steer-followup.jsonl
  • src/main/pi/__fixtures__/stop-mid-tool.jsonl
  • src/main/pi/rpc-agent-definition.ts
  • src/main/pi/rpc-capture-replay.test.ts
  • src/main/pi/rpc-compaction.ts
  • src/main/pi/rpc-context-usage.ts
  • src/main/pi/rpc-dialog-callbacks.ts
  • src/main/pi/rpc-extension-dialogs.ts
  • src/main/pi/rpc-idle-check.ts
  • src/main/pi/rpc-launch-resolution.test.ts
  • src/main/pi/rpc-launch-resolution.ts
  • src/main/pi/rpc-launch.test.ts
  • src/main/pi/rpc-launch.ts
  • src/main/pi/rpc-location.test.ts
  • src/main/pi/rpc-message-dialog-shapes.test.ts
  • src/main/pi/rpc-messages.ts
  • src/main/pi/rpc-options.test.ts
  • src/main/pi/rpc-options.ts
  • src/main/pi/rpc-prompt-delivery.test.ts
  • src/main/pi/rpc-prompt-delivery.ts
  • src/main/pi/rpc-prompt.test.ts
  • src/main/pi/rpc-prompt.ts
  • src/main/pi/rpc-protocol.ts
  • src/main/pi/rpc-retry-activity.ts
  • src/main/pi/rpc-runtime-registration.test.ts
  • src/main/pi/rpc-runtime-registration.ts
  • src/main/pi/rpc-session-adapter.test.ts
  • src/main/pi/rpc-session-adapter.ts
  • src/main/pi/rpc-session.ts
  • src/main/pi/rpc-turn-races.test.ts
  • src/main/pi/rpc-turns.ts
  • src/main/provider-process/provider-acquisition-starts.test.ts
  • src/main/provider-process/provider-acquisition-starts.ts
  • src/main/provider-process/provider-stdio-error-boundary.ts
  • src/main/provider-process/provider-stdio-write-queue.test.ts
  • src/main/provider-process/provider-stdio-write-queue.ts
  • src/main/runtime/rpc/methods/session-tab-registered-agent-projection.test.ts
  • src/main/runtime/rpc/methods/structured-agent-session-agents.ts
  • src/main/runtime/rpc/methods/structured-agent-session-conversation-outline.ts
  • src/main/runtime/rpc/methods/structured-agent-session-gate.ts
  • src/main/runtime/rpc/methods/structured-agent-session-hold.ts
  • src/main/runtime/rpc/methods/structured-agent-session-options-read.ts
  • src/main/runtime/rpc/methods/structured-agent-session-pi-dialog-capability.test.ts
  • src/main/runtime/rpc/methods/structured-agent-session-policy.test.ts
  • src/main/runtime/rpc/methods/structured-agent-session-policy.ts
  • src/main/runtime/rpc/methods/structured-agent-session-queued-methods.ts
  • src/main/runtime/rpc/methods/structured-agent-session-reveal.ts
  • src/main/runtime/rpc/methods/structured-agent-session-send-compatibility.ts
  • src/main/runtime/rpc/methods/structured-agent-session-status-stream.ts
  • src/main/runtime/rpc/methods/structured-agent-session-thread-goal.ts
  • src/main/runtime/rpc/methods/structured-agent-session-turn-completion-stream.ts
  • src/main/runtime/rpc/methods/structured-agent-session.ts
  • src/main/runtime/structured-agent-runtime-registrations-acp.test.ts
  • src/main/runtime/structured-agent-runtime-registrations.ts
  • src/main/runtime/structured-agent-session-runtime.ts
  • src/renderer/src/components/native-chat/NativeChatQuestionCard.tsx
  • src/renderer/src/components/native-chat/NativeChatResolutionReceipt.empty-answer.test.tsx
  • src/renderer/src/components/native-chat/NativeChatResolutionReceipt.tsx
  • src/renderer/src/components/native-chat/NativeChatStructuredQuestionCard.test.tsx
  • src/renderer/src/components/native-chat/NativeChatStructuredQuestionCard.tsx
  • src/renderer/src/components/native-chat/NativeChatStructuredSession.newer-approval.test.tsx
  • src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx
  • src/renderer/src/components/native-chat/native-chat-resolution-receipt.ts
  • src/renderer/src/components/native-chat/use-structured-agent-session.ts
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/zh.json
  • src/shared/agent-session-journal-schemas.ts
  • src/shared/agent-session-journal-types.ts
  • src/shared/agent-session-question-answer.ts
  • src/shared/agent-session-question-free-text.test.ts
  • src/shared/electron-remote-runtime-client-capabilities.ts
  • src/shared/protocol-version.ts
  • src/shared/structured-agent-session-surface-capabilities.ts
  • src/shared/structured-native-chat-launch-route.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Comment on lines +243 to +248
if (result.root !== 'exited') {
return false
}
if (result.tree !== 'exited') {
throw new AgentSessionAcquisitionRootExitObservedError(new Error('Pi root exit observed'))
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
# How does the ACP connection derive processTreeUnproven from a close result's tree verdict?
rg -nP -C6 '\bprocessTreeUnproven\b' src/main/acp src/main/provider-process
# How does the managed process produce tree: null on a graceful root exit?
rg -nP -C4 'tree:\s*null' src/main/provider-process

Repository: stablyai/orca

Length of output: 13385


Do not treat tree: null as an observed descendant exit.

ManagedProviderProcess returns { root: 'exited', tree: null } when the root exits after stdin closes. The current closeSession check throws for that result, so a normal Pi close can reject. The ACP adapter treats null as no tree observation and only reports live or unverifiable as unproven.

🐛 Suggested fix
--- "a/src/main/pi/rpc-session-adapter.ts"
+++ "b/src/main/pi/rpc-session-adapter.ts"
@@ -240,12 +240,12 @@
       return true
     }
     const result = await session.close(requested)
     if (result.root !== 'exited') {
       return false
     }
-    if (result.tree !== 'exited') {
+    if (result.tree !== null && result.tree !== 'exited') {
       throw new AgentSessionAcquisitionRootExitObservedError(new Error('Pi root exit observed'))
     }
     return true
   }
   releaseAcquisition(input: { sessionId: string }): Promise<boolean> {

Add an adapter test with closeResult = { root: 'exited', tree: null }.

📝 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.

Suggested change
if (result.root !== 'exited') {
return false
}
if (result.tree !== 'exited') {
throw new AgentSessionAcquisitionRootExitObservedError(new Error('Pi root exit observed'))
}
if (result.root !== 'exited') {
return false
}
if (result.tree !== null && result.tree !== 'exited') {
throw new AgentSessionAcquisitionRootExitObservedError(new Error('Pi root exit observed'))
}

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