Skip to content

fix(native-chat): never drop an agent process end Orca could not confirm - #24507

Open
brennanb2025 wants to merge 29 commits into
mainfrom
brennanb2025/native-chat-lifecycle-phase1
Open

brennanb2025 wants to merge 29 commits into
mainfrom
brennanb2025/native-chat-lifecycle-phase1

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 9 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1111 $\color{#cf222e}{\Huge{\mathbf{−}}}$​3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1108
Prod 18 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​381 $\color{#cf222e}{\Huge{\mathbf{−}}}$​73 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​308

ELI5

Sometimes Orca has to end a Claude or Codex process behind a chat because of a problem on Orca's side, for example when it can't save the chat's history. After ending it, Orca checks that the process really stopped. If that check can't finish in time (a busy machine, say), Orca used to say nothing at all. The chat went on believing the old process was alive, so every later message was aimed at a process that wasn't listening: each one showed "Message delivery is unconfirmed." and never reached the agent. Even when the old process did exit later, Orca didn't hear it, and the turn just stopped with no explanation.

With this change, Orca never drops an end it couldn't confirm. Messages are refused clearly while the old process might still be running, the old process's exit is always heard, and the turn says what went wrong.

What Changed

The problem

  • Orca ends an agent process itself in three cases:
    • the chat's history can't be saved;
    • Codex sends something Orca can't record;
    • Orca's own bookkeeping for the chat fails.
  • After ending it, Orca waits for proof that the process exited. When that proof didn't come in time, four places said nothing:
    • Claude's handling of the fault (claude-structured-session-exit-lifecycle.ts);
    • Codex's forced close (codex-structured-session-teardown.ts);
    • the recovery after a failed history write (structured-agent-session-event-recovery.ts);
    • and, for Claude, the old process's later exit. Once Claude's adapter (the per-provider code that runs the process) had let the chat go, that exit was ignored.
  • The "host" (the Orca process that runs the agent: the local app, or Orca on another machine for a chat over SSH) kept treating the process as alive. Every later message failed inside Orca, was marked "Message delivery is unconfirmed." (Orca's state for a message it can't tell whether the agent got), and was never retried. The turn kept reading "Working…".
  • fix(native-chat): the agent's exit ends its record, and an unconfirmed stop is joined instead of held #24862 (now on main) makes every process exit reach one handler, keeps an unconfirmed stop on the process's record, and makes later sends and Stops retry it. But nothing ever starts that stop for these faults, so on main with fix(native-chat): the agent's exit ends its record, and an unconfirmed stop is joined instead of held #24862 alone the bug still reproduces. The regression test below fails there with that unconfirmed state.

What is different for the user

  • While the old process might still be running:
  • When the old process exits: Orca hears it at once.
    • The turn ends with "Orca ran into a problem, so this didn't go through. Try again.", the same row a fault with a confirmed exit already writes.
    • The next message goes to a fresh process.
  • Stop or close after one of these faults:
  • A message, or /compact, sent in the moment after the adapter let the old process go:
    • It is rejected with the real reason instead of "Message delivery is unconfirmed.": for example "Claude stopped before it finished starting. Send your message to try again." for a start that died, or the "Orca ran into a problem…" sentence for a fault.
    • Codex sends and /compact behave the same way. Codex is said to have stopped only when its exit was actually seen. For a connection that broke while the process may still be running, the rejection reads "Orca couldn't hand this message to the agent, so it was not sent."
    • A /compact rejected this way keeps that reason on its row, instead of reading only "Compaction failed."
  • After a failed start whose close couldn't be confirmed: the messages waiting on that start are rejected with the start's real failure.

The mechanism

  1. Adapters report every end.
  2. The host records it (structured-agent-session-unproven-end.ts, -event-recovery.ts).
  3. The exit ends the record as the fault it was (-child-exit.ts).
  4. Sending to a process the adapter has let go of returns "rejected" with the end's reason. Before, it threw, and the host could only record a throw as unconfirmed. This covers both adapters, for messages and /compact. Claude keeps a failed start's wording.
  5. end-unproven is handled on its chat's own queue, off the exit queue all chats share.

The event stays inside the host. Nothing Orca sends to other windows, the mobile app or remote clients changes, and no saved data changes. The record of the reported fault lives in memory only.

Why

This is part of the rework of how Orca owns agent processes. #24862 owns the stop and close half: one exit handler, and an unconfirmed stop that later sends and Stops join. This PR owns the other half: no end is ever dropped, and a send to a process that's gone is answered rather than lost.

The fix is made where the ends were being dropped:

Alternatives considered:

How this compares with the common pattern (every process end reaches the one owner exactly once; a send to a process the owner no longer holds fails visibly instead of hanging):

  • Intended: an end Orca can't confirm keeps the chat from starting a second process on the same conversation. Messages are refused readably until the exit is confirmed. This is Orca's stricter "confirmed exited" rule, the same one its SSH/remote design uses, where losing contact is never treated as proof a process died.
  • Temporary: there is no time limit on how long an unconfirmed process can block new starts. In practice it's seconds, because the kill sequence ends the process and its exit is now always heard; the limit matters only for a process the OS can't kill. The next phase of this agent-process lifecycle rework adds a bound. No ticket exists for it yet.
  • Temporary: a refused or rejected message is never retried automatically; the user sends it again. The common pattern retries a message that never reached the agent automatically, after 15 seconds, 1 minute and 5 minutes. fix(native-chat): a failed start is shown on its message and retried, and later messages go ahead #24340's retry doesn't cover these: it retries only messages Orca had not yet recorded as handed over to the agent, and these were recorded as handed over, though never written to the process. No follow-up is filed yet.
  • Temporary: an exit Orca didn't ask for still goes through the single exit queue shared by all chats. Only end-unproven and the end of a close the host asked for run on the chat's own queue. Per-chat queues for all exits land in the next phase of this rework. No ticket exists for it yet.

How these rejections are shown (checked against #24710, now on main): every rejection this PR records is a host-recorded rejection with a typed reason ("Orca ran into a problem…", "Claude stopped before it finished starting…", "Couldn't stop Claude from before.", "Orca couldn't hand this message to the agent…", or closed with the chat). On desktop, #24710 draws such a message as "Not sent" with that reason, where it was rejected, with no Retry; the user sends it again. None of these reasons is a Stop withdrawal or a command, so none is hidden. A rejected /compact stays hidden as #24710 decided, and the command's own row carries the reason, which this PR keeps (the round-3 compaction fix). No conflict found; no change was needed.

Open check: #25028 (client-only, still a draft) will show host-recorded rejections through its new "sent from somewhere else" state (undeliveredSentElsewhere). Once it lands, check that each rejection here is shown once, with its own sentence. Not verified yet.

Linked Issue

None (maintainer change; no pre-existing issue). Builds on #24862 (merged).

Visual Proof

N/A. No new UI. The refusal, the rejection reasons and the "Orca ran into a problem…" row are existing chat rows that now appear where a message used to end as "Message delivery is unconfirmed." On desktop, a rejected message is drawn by #24710's existing "Not sent" row with its reason. Verified through host-level tests that drive the real Claude and Codex adapters and the real host against scripted processes. No app run.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

Head f5e48b6f2c5 targets main and includes main up to 4de9f9f85a8. Its diff against main is this PR's own 27 files (+1492/−76). The main merges needed hand resolution in four places. A Codex adapter test, where main had replaced one assertion. The Stop-event line in the stop function, where main now resolves with null. The Codex lifecycle routing, which moved with main's new agent registrations into structured-agent-runtime-registrations.ts. A compaction test file, where both sides added a test. Three of this PR's test files were also updated to main's new host construction (agents from a registry) and provider-handle shape. Rows marked "earlier head" below were run before the latest merge.

New and rewritten tests:

  • structured-agent-session-claude-unproven-fault-send.test.ts, the main regression test:
    • After a history-write fault whose close can't be confirmed, a message is refused readably and never written to the old process. Stop retries the kill. When the old process exits, the record ends with the fault's row and the next message goes to a new process and is accepted.
    • A message sent while the first close is still running is rejected with its reason.
    • A failed history write whose stop can't be confirmed leaves the process on record, closing, with the fault.
    • A turn cut short reads the same (turn "interrupted", one fault row) whether the exit was confirmed at once or only later.
    • A user's Stop, or a chat close, before the late exit: the turn still reads "interrupted" (never the user's cancellation) with one fault row, and the end keeps the user's cause.
  • structured-agent-session-codex-forced-close-unproven.test.ts: the same for a Codex forced close, including a chat close or an Orca eviction before the late exit (no Stop event written, turn "interrupted", one fault row), and a stale exit from an old process being ignored.
  • Claude and Codex takeover tests (added in 96416f316aa): a message accepted while a fault's close is unproven, then a chat close that takes that close over (with the close's own clearing of the queue failing), then the exit's proof. The child's end is stamped where the close was asked, so the message is rejected as closed with the chat and no new agent starts. If the takeover keeps the fault's old position instead, both tests fail: the message is not closed and a new agent is started for it.
  • structured-agent-session-unproven-end.test.ts: an open approval left after a finished turn is not interrupted work; a running turn is.
  • claude-structured-startup-fault-is-not-an-exit.test.ts: a failed start with an unconfirmed close rejects the waiting messages with the start's real failure.
  • structured-conversation-compaction.test.ts: a /compact whose agent was already gone keeps that reason on its row.
  • Adapter tests:
    • a send or /compact to a Claude process the adapter let go of is rejected, not thrown;
    • Codex sends to an ended session or a broken connection are rejected with the right reason.
  • structured-agent-session-lifecycle-delivery.test.ts: end-unproven does not wait behind other chats' exits.

Results:

AI Disclosure

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Security: no new inputs or permissions.
  • Cross-platform: the change is platform-neutral; the kill sequences are unchanged.
  • SSH and remote: the new event never leaves the machine running the agent, and an end that can't be confirmed is never treated as an exit.
  • Mobile and backwards compatibility: nothing sent between Orca processes changes and no saved data changes; clients see only existing rejection reasons and chat rows.
  • Performance: one extra comparison per adapter event, and nothing per message frame.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

…se, and bookkeeping after it never reads as unproven

- Both connections report the root process's exit once, with `expected` set when a close had
  begun. A close that came back unproven and whose root exits later is finished by the adapter,
  and its end reaches the host like any other.
- A Claude close whose resume-point write fails after the exit was proven, and a Codex close whose
  terminal row is refused, now end the session and report the failure, instead of keeping a dead
  child indexed as if its exit were unproven.
- A Codex close whose forced tree kill can't prove the descendants gone but saw the root exit
  reports the descendants and counts the root exit.
- Every child exit with an identity, expected or not, is forwarded to the host.
…p is the child's own close, which everyone joins

- The host keeps no stored "stop still owed" record any more. A stop begins the child's close
  (`child.close`), which lives on the child and ends with it. A second Stop, the idle reaper,
  quit, a send and an option/answer/goal/rewind all join that close instead of retrying a
  separate obligation.
- A caller waits on the close only as long as the step deadline; the close itself is never
  abandoned. A proof that lands after every caller stopped waiting reaches the host as the
  adapter's report of that exit, which ends the record through the same handler.
- Once the exit is proven, draining, settling, the lease release and the adapter's
  acknowledgement are each attempted and reported on failure; none keeps the child on record.
  A start, and the handle's close, write a release that failed from this host's proof of that
  exit, so a failed write never refuses a send.
- A start that meets a close still unverifiable is refused with `previousExitUnverifiable`, so the
  queued message is rejected with a send-again reason; nothing is held and nothing starts beside
  the old process.
- The idle sweep goes back to idle reaping only.
- Removes #24333's retry entry points, the wait row and its hold rule, the ask/failure cursors on
  the stored record, and the stop's own wake.

Tests replace the #24333 unproven-stop test: a send joining an unproven close and an in-flight
one, a late proof past the caller's bound, a root exiting after its close gave up, a proven exit
whose resume-point write and lease release both failed, an unverifiable close rejecting the send
and refusing an option change, a surviving descendant, quit and the idle reaper; and Codex's
unverifiable, late-exit and joined-close cases.
… unverifiable says so, and to send again

The start failure for a refusal with reason `previousExitUnverifiable` reads "Orca couldn't
confirm Claude's previous process ended. Send your message to try again." instead of "Claude
couldn't restart." The status-row kind and the refusal reason stay in the shared lists for rows
and hosts that still carry them; the catalogs keep one sentence for both.
…t follows it is logged

- A Claude close resolves as soon as the root's exit is proven: the session ends and its `ended`
  report goes out then. Saving the resume point runs afterwards and a failure is logged, so a slow
  or hung write never reads as an unproven exit or keeps a dead child on record.
- A root that exits after its close came back unproven finishes that close through the same path
  as any close, so the session's child work is published as ended (background tasks and subagents
  no longer stay shown running for a dead agent), and a failure there is logged.
- Codex logs a refused final row, and reports a root exit whose forced tree kill could not prove
  the rest of the tree gone the way Claude does, so the host logs it and blocks nothing.
- Both adapters take the host's logger for this bookkeeping.
…ts on the adapter's own close

- One exit handler (`structured-agent-session-child-exit`) ends a child's record for an exit
  expected or not. `expected` only changes what the chat is told: the stop's cause, its end at the
  stop's ask, the settlement id, and no crash outcome row. The lease release keeps the exit's
  evidence; the handoff guard, lifecycle barrier, sink release and adapter acknowledgement apply to
  both. A Claude journal-sink failure ends in the same step as its stop, as Orca's own fault.
- Joining a close is asking the adapter, whose close is memoized while it runs and bounded by its
  own kill escalation; the host keeps no attempt of its own and no 10 s caller bound. An ask after
  a close came back unproven runs the stop again.
- A close's end is stamped where its stop was asked for (a repeated ask moves it), so the closed
  chat and failed start checks order a message accepted meanwhile after it.
- A start refused because the old exit is unverifiable rejects what was queued in the same step.
- The end of a close the host asked for no longer waits on the cross-session recovery chain.
- The kill no longer waits for the stop event's write; the journal writes rows in order.
…f its reason

A crash's reason can carry kilobytes of the provider's stderr, and a lease whose death detail is
over 512 characters fails the store's own check. The exit handler cut it, but the release a start
or the chat handle's close re-derives did not, so after a crash whose own release failed every
message was refused as not resumable until restart. The record's builder now cuts the detail to
the record's bound, so no writer can hand it one too long.
…dy wrote

Once the root's exit is proven, the close still waited for the SDK's output reader to end. Something
outside the process tree that holds the output open would keep that close, and every send, Stop
and quit joining it, waiting with no bound. The wait is now bounded; past it the close resolves as
proven and the open output is logged.
…a closed it for

The connection reports the app-server's exit inside the close that ends it, so that report ended
every Codex close and replaced the close's own reason (for example, a provider frame that could not
be recorded) with the connection's stderr text in the ended record and the lease's exit evidence.
The session now records Orca's close with its reason, and the exit it ends keeps that reason. The
test connection reports its exit inside close the way the real one does.
Every exit now wakes delivery, and teardown drained exit recovery before it stopped delivery, so an
exit settled in that window could start a fresh agent that teardown then killed. Teardown stops
delivery first; queued messages wait for the next launch.
…ller's wait

The caller's bounded wait was removed; the comment describes the close as it is now.
…ext ask kills again

When a close's kill leaves the agent's root running, the host now logs it. Tests pin what a later
ask does: each connection runs its whole stop again (Codex sends SIGKILL a second time), refuses
input meanwhile, and proves the exit once the kill takes.
…dn't stop it

The host reaches an unverifiable verdict only after its own kill left the agent's root running, on
the machine that runs the agent, so the sentence now says that: "Orca couldn't stop {agent}'s
previous process." The refusal reason, failure kind and wire shapes are unchanged. The host test
also checks the failed kill is logged.
…ved the kill

The host's close runs where the agent runs, so lost contact never yields this verdict; the comment no longer says it does.
…er met it

The log added at the close fired beside a Stop's own failure report for the same event. A stop
still reports it through its failure; a send or option change refused over it now logs it at the
refusal, the only place it is otherwise invisible.
…p-exit-ends-record

# Conflicts:
#	src/main/codex/codex-app-server-connection.test.ts
…p-exit-ends-record

# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-agent-session-claude-unproven-stop-send.test.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts
…p-exit-ends-record

# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-agent-session-claude-unproven-stop-send.test.ts
#	src/renderer/src/i18n/en-runtime-required.json
#	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-failure-copy.ts
…inly

The rejection now reads "Couldn't stop {{agent}} from before. Send your message again to try once more."
This kind has its own send-again step; every other failure keeps "Send your message to try again."
Built on the one-exit-handler, close-on-the-child model of #24862.

- Claude: an Orca fault whose close cannot prove the child gone reports
  `end-unproven` instead of staying silent, and the child's later exit (now
  reported inside that close) settles the exit the adapter kept.
- Codex: a forced close that cannot prove the exit reports `end-unproven`
  for the window before the exit.
- Sink failure: an unproven or failing stop reports the end instead of
  returning silently.
- The host keeps such a child on record, closing, with the reported
  reason, failure and whether work was in flight; every later send, start
  or Stop joins that close. The exit that ends the record settles as that
  end (cause exit, the fault's row when work was interrupted); a Stop or
  close asked for since keeps its own cause and the fault's row.
- Dispatch and /compact to a session the adapter no longer holds answer
  `rejected` (never a throw that reads as delivery unknown), keeping a
  failed start's wording; Codex says the agent stopped only when its end
  was seen.
- `end-unproven` is handled on its chat's own lane, off the exit chain.
@brennanb2025
brennanb2025 force-pushed the brennanb2025/native-chat-lifecycle-phase1 branch from 1bbe8c3 to baf85fc Compare October 5, 2026 00:06
@brennanb2025
brennanb2025 changed the base branch from main to brennanb2025/owed-stop-exit-ends-record October 5, 2026 00:07
…op event; review round 3

- A Stop, chat close or quit that takes over the close an unproven fault
  began keeps its own cause but writes no Stop event, like any stop that
  joins a close: the fault ended the turn, so it reads interrupted with the
  fault's row, never the user's cancellation.
- A /compact rejected because its agent was already gone or never started
  keeps that reason on its row instead of reading only "Compaction failed."
…lose ends the child

A message accepted while a fault's close is unproven, then a chat close
that takes that close over and whose own rejection of the queue fails,
then the exit's proof: the end stands where the close was asked, so the
message is closed with the chat and no new agent starts (Claude and Codex).
…hat-lifecycle-phase1

# Conflicts:
#	src/main/claude/claude-structured-session-adapter.ts
#	src/main/codex/codex-structured-session-adapter-lifecycle.test.ts
#	src/main/codex/codex-structured-session-teardown.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-event-recovery.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts
#	src/main/runtime/structured-agent-session-lifecycle-delivery.test.ts
#	src/main/runtime/structured-agent-session-lifecycle-delivery.ts
#	src/main/runtime/structured-agent-session-runtime.ts
@brennanb2025
brennanb2025 changed the base branch from brennanb2025/owed-stop-exit-ends-record to main October 5, 2026 19:32
@brennanb2025
brennanb2025 marked this pull request as ready for review October 5, 2026 22:15
@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

Walkthrough

Claude and Codex adapters now report unproven session ends and return classified rejections for dispatch or compaction on detached sessions. The host records the reported failure and interruption state, and reconciles it with later stop and exit events. Runtime delivery routes unproven-end events, and Codex journal and command handling process the related outcomes. Tests cover provider exits, host recovery, interruption state, and close races.

Priority: ➖ Normal

Merge Risk: 🔵 Low · up to f5e48

Run the required real Claude CLI tests on this head before merging to validate the changed session behavior.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 4453a

The change blocks further work while a previous process may still be running and preserves the failure for later resolution. Reviewed paths retain process-ownership checks. No introduced security weakness was established, but restart recovery and some dependents remain incompletely verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected lifecycle mutation is scoped to the named session and matching provider child. Its immediate effects are work admission, close retries, ownership release, and journal settlement for that child. Export and dependent counts alone do not establish a broader attacker-reachable boundary.

Trust Boundaries and Controls

  • observed — Both acquisition generation and fence must match before a lifecycle report can mutate the current child. Claude additionally matches the retained connection before processing a late exit. These controls prevent an old child's report from settling a replacement child.

Resilience and Maintainability Implications

  • observed — Unproven-end delivery queues on its session's serialized path rather than waiting behind unrelated exit recovery. Pending handlers remain tracked by drain, preserving teardown's ability to await lifecycle work. The inspected test specifically covers cross-session progress and drain waiting.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.59% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 54 functions across 28 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly summarizes the main change: preserving and reporting agent-process ends that Orca could not confirm.
Description check ✅ Passed The description covers the required explanation, changes, rationale, issue context, visual proof, and testing. It also documents limitations and cross-platform considerations. The maintainer-specific …
  • Fix all pre-merge checks with AI
✨ 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.

@pullfrog pullfrog 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.

✅ No new issues found.

Reviewed changes

This PR makes an agent-process end that Orca could not confirm impossible to drop, building on #24862's close record. I read the full 27-file diff, traced both adapter paths into the host, and ran the changed suites (82 tests, all green).

  • New internal end-unproven event (structured-agent-session-adapter.ts) — emitted by Claude's reportClaudeUnprovenEnd and Codex's forceCloseUnexpected when a stop cannot prove the root gone; it stays inside main and never reaches clients, other windows, or saved data.
  • Host records the unproven end (structured-agent-session-unproven-end.ts, wired in -event-recovery.ts) — keeps the child on record, closing, writes no Stop event, precomputes interruptedWork under the same rule an observed exit uses, and republishes status.
  • The late exit settles as the reported end (-child-exit.ts) — expected yields to reported, the fault's failure is preserved, endedAt stands where the close was asked, and a Stop/close that took over keeps its own cause while the fault's row is still written.
  • Stop takeover (-host-lifetime.ts) — the first Stop/chat-close/quit after a fault adopts the fault's reported details and writes no Stop event.
  • Sends/compaction answered instead of thrown — Claude withClaudeSessionOrRejected, Codex detachedRejection, and structured-agent-session-command-turn.ts keep the agent-gone reason rather than reading as "Compaction failed."
  • Delivery routing — end-unproven takes the session's own lane and is tracked for drain; Codex routing extracted to structured-codex-lifecycle-event.ts.

Two independent specialist passes found no re-entrant-lane deadlock (all producers fire-and-forget onEvent; the one in-lane path uses the …UnderSerialize variant) and no dropped fault failure/row. The documented open items (no time bound on the blocking process, no auto-retry of refused messages, single shared exit queue, #25028 rendering check) are pre-existing/sequenced trade-offs already stated in the PR body.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

…hat-lifecycle-phase1

# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-conversation-compaction.test.ts
#	src/main/runtime/structured-agent-session-runtime.ts
…main now does

main's host takes its agents from a registry and the provider handle is now
transport/agent/nativeId; the three new test files follow both.

@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: 867c1c89-ffe7-4071-988d-50cdd332e8a4
📥 Commits

Reviewing files that changed from the base of the PR and between 4453a86 and f5e48b6.

📒 Files selected for processing (13)
  • src/main/claude/claude-structured-session-adapter.ts
  • src/main/codex/codex-structured-session-adapter-lifecycle.test.ts
  • src/main/codex/codex-structured-session-adapter.test.ts
  • src/main/codex/codex-structured-session-state.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-claude-unproven-fault-send.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-codex-forced-close-unproven.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-unproven-end.test.ts
  • src/main/native-chat/agent-session-wire/structured-conversation-compaction.test.ts
  • src/main/runtime/structured-agent-runtime-registrations.ts
  • src/main/runtime/structured-claude-runtime-adapter.ts

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

Comment on lines 67 to +99
@@ -83,7 +81,7 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda
supportsLocation = supportsClaudeStructuredLocation

acquire = (input: StructuredAgentSessionAcquireInput): Promise<AgentSessionAcquisition> => {
this.settledExitErrors.delete(input.identity.sessionId)
this.settledExits.delete(input.identity.sessionId)
return acquireClaudeSession({
input,
deps: this.deps,
@@ -96,13 +94,9 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda
handleExit: (sessionId, attempt, error) =>
observeClaudeSessionExit(this.exitLifecycle, sessionId, attempt, error),
finishClose: (sessionId, attempt) =>
finishClaudeCloseAfterExit({
sessions: this.sessions,
sessionId,
connection: attempt.connection,
deps: this.deps,
afterClose: (close) => this.afterClose(sessionId, close)
}),
finishClaudeExitedClose(this.exitLifecycle, sessionId, attempt.connection, (close) =>
this.afterClose(sessionId, close)
),

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.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

git diff --name-only baf85fcc958c9cf108f9d76bec45c1e69be1c87d f5e48b6f2c539e5a0f5b6d4b0e9fee782c21ebd8 -- 'src/main/claude/claude-structured-*'
rg -n 'Real Claude CLI|ORCA_REAL_CLAUDE_CLI_TEST|Verifying Changes' AGENTS.md src/main/claude package.json 2>/dev/null | head -65

Repository: stablyai/orca

Length of output: 3035


Run the required real Claude CLI tests on this head.

The reported 8/8 pass predates changes to Claude structured-session code, and the PR says the tests were not rerun. The project requires this run when that code changes:

ORCA_REAL_CLAUDE_CLI_TEST=1 pnpm test src/main/claude/claude-structured-real-cli.test.ts src/main/claude/claude-structured-real-cli-fold.test.ts

This branch has not been deployed

No deployments
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