Skip to content

fix(native-chat): start a new chat after an earlier start failed - #24917

Merged
brennanb2025 merged 21 commits into
mainfrom
brennanb2025/chat-new-launch-after-failed-start
Oct 5, 2026
Merged

brennanb2025 merged 21 commits into
mainfrom
brennanb2025/chat-new-launch-after-failed-start

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 45 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2855 $\color{#cf222e}{\Huge{\mathbf{−}}}$​282 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2573
Prod 62 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​895 $\color{#cf222e}{\Huge{\mathbf{−}}}$​186 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​709

ELI5

When a native (structured) chat fails to start, for example because the host refused to create it, Orca kept treating that failed chat as "still starting". Two things went wrong because of that:

  1. In the + menu and the new-tab search, that agent (Claude or Codex) stayed greyed out with a spinner. You could not open a new chat with that agent in that workspace until you closed the failed chat or reloaded the app.
  2. Some places that start a new chat with text already filled in, such as Send notes to > New agent > Claude, source-control agent actions ("Fix with AI") and browser annotations, did not open a new chat. They joined the old chat instead and quietly dropped the text you sent. This also happened when the old chat's start could not be confirmed (its answer was lost), and while an old chat's Retry was running. "Fix with AI" even said "Started an AI agent for this check" while your instructions went nowhere.

After this change, a failed or unconfirmed chat stays where it is, with its own Retry button. Asking for a new chat always opens a new one and sends your text to it, even while an earlier chat for the same agent is still starting. Each click, menu pick or send now carries its own request id, so only a repeat of that very action (a double click, or the same action delivered twice) still makes one chat; and an empty chat that is still starting takes the first text sent to it.

What Changed

Before / after, as the user sees it

Situation Before After
A Codex chat failed to start in this workspace. Open the + menu or the new-tab search. Codex is greyed out with a spinner until the failed tab is closed or the app reloads. Codex can be picked. Picking it opens a new chat.
Same, then Send notes to > New agent > Codex, or a source-control action that starts a new agent with text. The old failed chat is focused and restarted. The notes or action text are never sent, and nothing says so. A new Codex chat opens and the notes are sent to it.
Click Retry in the failed chat. Retries that chat. Same. Retry still works on the failed chat, including after a reload, and leaves any newer chat alone.
An earlier chat's start could not be confirmed (its answer was lost), or an old failed chat is being retried. Then "Fix with AI", "Send notes to > New agent", or the + menu. The request joins that old chat: the + menu re-checks it; notes and "Fix with AI" text are dropped (after a reload they can land as an unsent draft in the old chat), yet "Fix with AI" reports that it started an agent. A new chat opens with your text. The unconfirmed chat keeps its own "Chat connection could not be confirmed." line with Retry.
A chat is still starting or still sending its opening text, and a DIFFERENT request arrives (another "Fix with AI", notes sent to a new agent, a + pick after a typed request). The different request joins the first chat; its text is added there. It opens its own new chat with its own text.
The SAME action twice (a double click, or the same action delivered twice). One chat. Unchanged: one chat, and the text is sent once.
A + menu or new-tab pick (no text) while another chat with that agent is still starting. That agent was greyed out with a spinner until the first chat had started. The agent can be picked. If the split you picked in already has an empty chat with that agent (starting or idle, nothing sent or typed in it), the pick brings it to the front instead of opening another; otherwise it opens a new chat there.
A request with text (notes, "Fix with AI", a quick command) — Always its own chat, except that it may take over an empty chat that is still starting in the same split.
You send notes (or browser annotations) to an agent, add another note, and send again before the first send has arrived. The second send included the first notes again, so they reached the agent twice. Notes that are on their way are left out of the next send; it carries only the new notes. If the first send fails, those notes come back. While every note is on its way, the menus offer no empty send.
A BLANK chat is still starting (a + pick, or the empty workspace's default chat), and a request with text arrives (for example notes sent to a new agent). The text joins that chat. Unchanged in effect: if that chat is still empty (you have not sent or typed anything in it), it takes that text as its opening message (sent, not left as a draft) instead of a second chat opening; if you already sent or typed something there, the request opens its own new chat. After that, only a repeat of the same text joins it; anything else opens a new chat.

Mechanism

Orca stops two requests for the same agent in the same workspace from creating two chats when they race, for example a double click or two callers firing at once. It does this with a slot keyed by agent + workspace (plus the conversation id when resuming). A later request joins whatever holds the slot. Before this change, the registry stored that slot in a map (pendingStructuredLaunchesByIdentity), and a failed chat kept its entry there indefinitely. So:

  • getStructuredAgentLaunchStatus counted the failed chat as starting, which is why the menus disabled the agent.
  • structuredAgentLaunchState found the failed chat in the slot, restarted it, and threw away the new caller's text, because it assumed the caller was retrying the same text. That is how the notes were lost.

Now the registry keeps every launch only by session id, and the slot holder is derived instead of stored. A new chat's launch holds the slot only during its own first attempt: while its create is running, or once created while it is still sending the text it started with. That is exactly the double-click window. It never holds the slot while unconfirmed, while being retried, or after failing or being cancelled, so a new request opens a new chat. A launch that resumes an earlier conversation keeps holding its slot through every in-flight state, including unconfirmed and retries, because the host refuses a second chat adopting the same conversation. Whether an attempt is the first one or a retry is recorded on that attempt's caller group (attempt: 'first' | 'retry'), which a new attempt replaces, so the fact never outlives its attempt. A blank chat that is still starting and still empty (nothing sent into it, nothing typed in its composer) can be claimed once by the first request that carries text, which matches the common pattern of reusing an existing thread only while it is empty: that text becomes its opening message and the recorded request becomes that request. The holder rules live in structured-agent-session-launch-holders.ts. Every new-chat launch now carries a request id, minted once at the user's gesture (lib/agent-launch-request-id.ts): in the event handler of a click, menu pick or send, or once at the entry of an action that runs once per gesture ("Fix with AI", source-control starts, quick commands, the dashboard, a fork). Programmatic starts mint once at their entry (the empty workspace's default chat, onboarding), and a worktree-create chat uses its create's id, so a retry of that same create carries the same id. The id is a required field on the launch request and options, so every caller must pass one. The first attempt records its id (attempt: { kind: 'first', requestId, … }, in structured-agent-session-launch-request.ts); only an equal id joins it, and a repeat never stages text of its own: it waits on what the first delivery staged, so the text goes out once and every caller is told it was delivered. The id is not saved with the launch record: after a reload a launch is reached only by its session id (its Retry), so nothing would read it. Because every pick is now a new request, the + menu and the new-tab search no longer disable an agent while one of its chats is starting. A pick with no text first looks for an existing empty chat with that agent in that workspace: a starting one is joined in the launch registry (structured-agent-session-launch-holders.ts), an idle published one is found by structured-agent-session-idle-empty-chat.ts and brought to the front through the existing tab focus path, and the caller is told which session it got. In code (structured-agent-session-launch-registry.ts):

  • getJoinableStructuredLaunchState(identity, request) (in structured-agent-session-launch-holders.ts) replaces getStructuredLaunchState(identity). It returns the launch this request may join, a repeat of its own request or an empty starting chat it may claim, or nothing.
  • getStructuredAgentLaunchStatus (moved into structured-agent-session-launch-status.ts to stay under the file-length limit) counts only launches that hold the slot.
  • setStructuredLaunchState, deleteStructuredLaunchStateIfCurrent, and the persistence loop now key on session id. Before, a retry of an old failed chat after a reload could overwrite a newer chat's slot entry, and that chat's cleanup could then delete it. That can no longer happen.

In structured-agent-session-launch.ts, a new start records its attempt as first with its opening request, and a retry as retry. The join itself moved into joinStructuredLaunchState, which stages the joiner's text before changing anything (a repeat reuses the first click's staged message; a claim of an empty chat stages the claimer's text and adopts its delivery mode). Joining restarts a launch only when it is an unconfirmed resume; a failed launch, or an unconfirmed or retried new chat, can no longer be found by a join.

A failed chat stays reachable by its session id. Its Retry line, retryStructuredAgentSessionLaunch, the relaunch that runs when a message is queued on it, its saved launch record, and closing it all work as before.

Existing tests changed (the behavior they pinned has changed)

  • structured-agent-session-launch.test.ts "retries a definitively refused launch with the same session and a new operation" retried the failed chat by starting the same agent again; that route now opens a new chat, so it retries through retryStructuredAgentSessionLaunch(worktreeId, sessionId) (what the chat's Retry button calls) with its original assertions.
  • "keeps one launch identity per worktree while the outcome is unknown" pinned the text drop itself; it now asserts the reverse: a new start opens a new chat and its text is staged on it.
  • "keeps an unresolved identity reserved…" now uses a resume launch, where the reservation still holds.
  • "reuses the queued prompt without a second delivery after unknown recovery" now re-checks through the chat's own Retry.
  • launch-agent-structured-chat-guard.test.ts "does not create a sibling when post-create visibility proof is unknown": its second call is now the chat's Retry (renamed accordingly).
  • Removed "does not stage the preserved launch prompt again on retry" (the retry path cannot stage by construction, so it could no longer fail) and the after-failure test "keeps an unconfirmed launch coalescing" (reversed; the new file covers the reversal and the resume case).

Notes and annotations on their way. Notes sent from the notes shelf, and browser annotations, used to stay in their list until the agent received them, so a second send in that window sent them again. With "new is always new" that second send opens its own chat, which would put the first notes in two chats. Now notes handed to a send leave the next send at once. For a new agent, the new chat's own saved message owns them: they are cleared when any send of that message is accepted (including a Retry of a failed chat, or the automatic re-check), and they come back to the shelf when that chat is closed. Sends to a running agent or the sidebar agent row hold them until that send's own result arrives. While every note is on its way, the trigger reads "Sending…" and offers no empty send. Every route reports its result through one callback (a new agent, a running agent, the sidebar agent row, browser annotations). The notes menu builds each send's text from the notes not on their way. Nothing is stored on disk: the links are in memory and end with the message or the send. A Stop that pulls a queued message back into its chat's composer counts as delivered, because the text now sits in that chat (Intended). A renderer reload drops those links (Temporary, see "Known follow-up").

Scope note: this part changes the notes and annotation send code (about 20 files), beyond the launch registry. It is the same user-facing rule (a new request carries its own text, exactly once), and without it the launch fix would turn an existing duplicate into a duplicate across two chats.

Why

Disabling the agent after a failure meant a bookkeeping record was blocking the user. The narrower fix, simply reading a failed launch as "not starting", was tried and reviewed on #24904 and rejected. It enables the menu, but a pick still joins and restarts the failed chat and drops the notes. The drop is not specific to the menu. It already happens on main from launchers that start a chat with text and never check the status (source-control agent actions).

The common pattern is that "new" always creates a new thread and a failed thread keeps its own retry. Nothing ever joins a new request onto a thread whose start failed. This change adopts that for failed starts; the two cases where Orca still joins a request onto an existing chat are listed below.

The alternatives were weaker:

  • Keep the stored slot and release it when the launch fails, then reclaim it on Retry. That spreads writes over every place a launch can fail or be restored, and it is the kind of second copy of state that drifts. Deriving the holder from the launches themselves has nothing to keep in sync.
  • Keep joining failed launches but stage the joiner's text on the old chat. That would deliver notes to a chat the user did not pick, and it still differs from the common pattern.

Differences from the common pattern

  • How "the same request" is recognised: matches the common pattern. The common pattern mints a request's id at the user's gesture and passes it down, dedupes a repeat of that id, and never lets two different requests share a thread. Orca now does the same: one id per gesture, joined only on an equal id. (An earlier version of this PR matched requests by their text; that could split one action whose text was re-fetched between deliveries, or merge two different actions with identical text, so it was replaced.)
  • Matches the common pattern: an empty chat is reused. A request with no text (a + menu or new-tab pick, the new-agent shortcut, the dashboard) reuses an existing empty chat for that agent in the same split (the tab group the pick was made in, or the workspace's active group), still starting or idle, instead of leaving two empty chats. A pick in another split opens a chat there, so focus never jumps across splits. "Empty" means nothing was sent into it (for a chat that already started, the host reports no request at all, and only while it is connected) and nothing is typed, attached or queued in its composer. Failed, unconfirmed and resumed chats are never reused; a chat with anything in it stays, and a new one opens.
  • Intended: the same button that carries text ("Fix with AI", a source-control action), pressed again after its first start has begun, is a new action and opens a second chat. Each such button already blocks repeat presses while it is starting, and the new chat comes to the front right away, so pressing again means asking again.
  • Intended: a request to resume an earlier conversation still joins that conversation's launch while it is in flight, whatever its id, because the host refuses a second chat adopting the same conversation.

Known follow-up

Linked Issue

Follow-up split out of #24904 (review item F3). Merge this before #24904 (or together): #24904's automatic re-check of unconfirmed starts relies on this PR's rule that a new request never joins a failed or unconfirmed chat. Both add one field to the per-attempt caller group, so whichever merges second keeps both. Related: #24904 (failed-start mark on the tab and workspace row), #24588 (structured launch failure surfaces). Both also edit structured-agent-session-launch.ts, and this PR's edits there are not small (about 65 added and 36 removed lines: the join moved into joinStructuredLaunchState, the attempt kinds, the re-export of the status module), so whoever merges second should merge carefully and re-run the launch tests.

Visual Proof

Added: macOS (M4Air), on this PR's final commit. Hidden, isolated test builds on a Mac. Before: 3284b4c70c9 (this branch's base on main). After: 60d7e010e59 (this PR). A stand-in Claude program was used, never the real one. These cover the unconfirmed-start cases and the notes changes that the Windows run below could not reach.
How the states were made: the test build's request to start a chat was held, lost, or refused on its way to the app's back end. "Unconfirmed" means the start request got no usable answer, so the chat shows "Chat connection could not be confirmed." with Retry.

An unconfirmed chat, then + > Claude. Before: no new tab opens; the pick re-checks the old chat, which turns into a normal empty chat. After: a new chat opens, and the old chat keeps its own "Chat connection could not be confirmed." line with Retry (right).

Before After After: the older chat
Before: still one Claude Chat tab after the pick; the old chat became a normal empty chat After: a second Claude Chat tab is open and active, a normal empty chat After: the older tab still shows Chat connection could not be confirmed. with Retry

An unconfirmed chat, then Send notes to > New agent > Claude. Before: no new tab opens. The note is never sent: the old chat is re-checked and stays empty, and the shelf still says "Notes 1". After: a new chat opens with the note as its first message, and the stand-in received it once.

Before After
Before: same tabs, empty chat, Notes 1 still in the shelf After: a new Claude Chat with the note as its first message and the stand-in reply

Notes on their way (after only). Note A was sent to a new agent while that chat's start was held. While A was the only note and on its way, the send button was disabled and its tooltip read "Sending…" (left). Note B was then added and sent: it opened a second chat. After the first start was released, the first chat had only A (middle) and the second only B (right). The stand-in's log shows A received once and B once, by different sessions.

A on its way: "Sending…" First chat: only A Second chat: only B
After: notes send button tooltip reads Sending… while note A is on its way After: the first chat holds only QA-NOTE-A and the reply After: the second chat holds only QA-NOTE-B and the reply

Notes to a new agent whose start fails (after only). The new chat shows "Chat could not be started." with Retry, and the note stays held: the button is disabled with "Sending…" (left). Closing the failed chat puts the note back in the shelf ("Notes 1"), and the send button works again (right).

Failed chat, note held After closing it
After: Chat could not be started. with Retry; notes button tooltip Sending… After: failed chat closed; Notes 1 back in the shelf with the send button enabled

How this failure was made on macOS: on a Mac, Claude starts under a small supervisor process. A Claude program that cannot be run therefore does not refuse the start; it gives "Claude stopped before it finished starting." instead. So the start was refused at the back end, the way a host without that request answers. The chat lands in the same failed state as the Windows "cannot be run" case.

Found, not caused by this PR (same in both builds). Suppose notes are sent to a new agent whose Claude quits while starting. The chat shows "Your message was not sent." and "Claude stopped before it finished starting. Send your message to try again." The notes leave the shelf, and closing that chat does not bring them back. Before and after behave the same.

Before After
Before: note sent to a chat whose Claude quit while starting; no notes left in the shelf After: same; no notes left in the shelf

No new pop-up, toast or banner appeared in the after build. The "Enjoying Orca?" card seen in some after captures is Orca's existing star prompt, shown after the first finished chat turn; the before runs never finished a turn.
Not captured on macOS: Codex, a source-control "Fix with AI" start, and a renderer reload during a notes send.

Earlier: Windows captures (still hold).

Windows, in hidden, isolated test builds. Before: de77c4b065a (this branch's base on main). After: c4751e2d995 (this PR). A stand-in Claude program was used, never the real one.
How the failure was made: the Claude program the app finds first cannot be run, so the chat's start fails before any process exists. Both builds show "Chat could not be started." with Retry. The cause was then removed, so a new start can succeed.

The + menu after a failed start. Before: Claude is greyed out with a spinner. After: Claude can be picked.

Before After
Before: + menu with Claude greyed out and spinning, failed chat behind it After: + menu with Claude enabled, failed chat behind it

Picking Claude (after only). A new "Claude Chat" tab opens and starts normally (left). The failed tab stays, with its own "Chat could not be started." and Retry (right).

New chat The earlier failed chat, still there
After: second Claude Chat tab active, normal empty chat with Opus (stand-in) After: first Claude Chat tab still shows Chat could not be started. with Retry

Send notes to > New agent > Claude, from the Source Control notes shelf, with one note on README.md and the failed chat present.

  • Before: the Claude item under "New agent" was greyed out with a spinner and could not be clicked. No chat opened and the note was not sent.
  • After: a new chat opened, the note arrived in it as the first message ("File: README.md, Line: 1, User comment: ..."), and the stand-in answered.
  • In this source the before build blocked the pick outright, so the "old failed chat restarted, notes dropped" path was not reached from here. That path is reached by launchers that do not check the menu state, such as source-control agent actions, which were not exercised.
Before After
Before: after trying Send notes to New agent Claude, still one failed Claude Chat tab, no notes sent After: a third Claude Chat tab holding the note as its first message and the stand-in's reply

Repeated picks while the start keeps failing (after only). With the failure put back, two more picks opened two more tabs. Each shows its own "Chat could not be started.", one failed tab per pick, as expected. The "Enjoying Orca?" card in this shot is Orca's existing star prompt, shown after the first finished chat turn; it is not part of this change.

After: five Claude Chat tabs; the newest shows Chat could not be started.

Not captured:

  • Retry after a reload while a newer chat is starting.
  • An unconfirmed launch (the create's answer lost).
  • Codex.
  • macOS/Linux; the change has no platform code.

Testing

  • structured-agent-session-launch-after-failure.test.ts (new): after a refused start the status is idle and a new start with notes text opens a new session that receives it, while the failed chat keeps its own prompt; retrying the failed chat while a new launch is in flight, then closing it; after a reload, retrying the restored failed chat leaves a newer launch alone; a failed resume, then resuming again.

  • structured-agent-session-launch-unconfirmed-or-retried.test.ts (new): "Fix with AI" through the real runSourceControlAgentActionStart while an earlier chat is unconfirmed reaches a new chat; notes are delivered to a new chat; after a reload, while a restored failed chat's Retry runs, a new start with text opens a new chat instead of becoming a draft in the old one; a + pick while unconfirmed opens a new chat; an unconfirmed resume is re-checked, not created twice.

  • structured-agent-session-launch-different-request.test.ts (new): two actions with identical text open two chats; the same action delivered twice makes one chat and sends the first delivery's text once, even if the re-delivery carries other text; a click after a failed start opens a new chat; a different request while a first chat's create runs, or while it is still sending its opening text, opens its own chat with only its own text; a double click makes one chat; an empty starting chat takes the first notes as its opening message, then a different request or a blank pick opens a new chat; a chat the user already sent into, or typed into, is never taken; a claim whose text cannot be saved is not recorded and the failure shows.

  • QuickLaunchButton.launch-status.test.tsx (new): every agent stays launchable while chats start, fail or retry, and each pick gets its own request id, so a second pick opens its own chat.

  • Ablations: every rule above was removed in turn (reverting to the previous head; ignoring the first/retry distinction; letting an unconfirmed new chat be joined; dropping the resume exception; ignoring the request when joining; restaging a repeat; dropping the empty-chat claim or either half of the emptiness check; recording a claim whose text failed to save), and each makes its tests fail.

  • new-agent-prompt-outcome.test.ts (new, real launch and outbox): notes sent to a new agent leave the shelf on delivery; after a failed start they stay held while the failed chat has them; a Retry that delivers clears them; closing the failed chat, or a chat still starting, brings them back; the paired route follows the failed chat once its start settles. NotesSendMenu, annotation, sidebar-send and quick-launch tests cover the in-flight hold and the "nothing left to send" state.

  • 118 targeted test files (1217 tests: everything that imports or mocks the launch modules, the outbox, the notes and annotation send code, the tab-bar menus, source-control starts and the startup-terminal tests) pass. Web typecheck from scratch, oxlint (also on the tree merged with fix(native-chat): mark a chat whose start failed on its tab and workspace row #24904, which stays under the file-length limit), oxfmt, the changed-code quality gate, React Doctor and the localization checks are clean.

  • Startup default terminal: an unconfirmed or failed chat always has its own chat tab, which counts as an open tab, so no terminal is seeded beside it (code read).

  • SSH / remote / folder workspaces: renderer-only change keyed by workspace and session id. No wire change.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

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

No security, platform, mobile, or performance impact. Looking up the slot holder scans the in-memory launches, which number at most a few per workspace. Backwards compatible: saved launch records are unchanged, and the slot never was persisted.

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)

A chat whose start the host refused stayed registered under its agent and
workspace, so the + menu and new-tab search kept that agent disabled with a
spinner, and any later "new chat" for that agent (for example "Send notes to
> New agent") restarted the failed chat instead and dropped its own prompt.

Only launches in flight now hold the agent/workspace slot that later starts
join. The registry keeps every launch by session id and derives the holder,
so a failed chat keeps its own Retry while a new start makes a new chat.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review summary

The problem. When a native chat's start failed (its create was refused), Orca kept treating that failed chat as "still starting". So the + menu and the new-tab search kept that agent greyed out with a spinner until you closed the chat or reloaded. Worse, anything that asked for a new chat with that agent in that workspace was joined onto the old failed chat instead. The old chat was restarted, and the new request's text was thrown away with no message. That covers "Send notes to > New agent", browser annotations, and source-control agent actions. This came out of the review of #24904, which first re-enabled the menu and would have exposed the drop more widely.

What changes for you. After a failed start, the agent is selectable again. Picking it opens a new chat, and any text you sent goes to that new chat. The failed chat stays in its tab with its own Retry, which still works, including after a reload.

How. The launch registry no longer keeps a separate "who holds this agent+workspace slot" map. Launches are kept by session id, and the slot holder is worked out from each launch's state. Only a launch that is still starting can be joined, and a failed one never can. This also fixes an older leak, where a reload restore could overwrite a newer launch's entry.

What the review changed. No code changes. The architecture challenge sketched the same design independently and recommended keeping it and not splitting it. The PR description was corrected:

  • the "Intended" note now says exactly when Orca still joins a request onto a chat: while that chat's create is running or it is still sending its first text, which is what turns a double click into one chat;
  • the "Temporary" note names the two remaining windows (below) and a follow-up that does not contradict the PR's own reasoning.

Deferred.

  • Pre-existing on main, not changed here; your call: two cases still join a new request onto an existing chat and drop its text.

    • (1) A chat whose create answer was lost (unconfirmed). The + menu shows the agent as pickable then, and a pick re-checks that chat.
    • (2) The short window while only an old chat's Retry is in flight, for launchers that don't check the menu state.

    The follow-up would make any new start that isn't a resume stop joining those, so it always opens a new chat. It reverses three older tests that pin today's behavior, so it is kept out of this PR. Folding it in is a small change if you want it here.

  • Seen in QA, pre-existing and unchanged here: the notes menu lists a failed chat as a send target labelled "Idle".

  • P3s:

    • one changed test can no longer fail and should be repointed;
    • a test comment and fixture name are misleading;
    • with many failed chats open, each save rewrites the stored launch list once per chat;
    • the launch module now re-exports its status helpers with export *.

What was verified.

  • New tests: notes go to a new chat after a failure; retrying or closing the old chat while a new one starts; a reload with a saved failed chat plus a new launch; resume launches; unconfirmed launches still joining; the + menu enabled for a failed agent and disabled for one still starting.
  • Each part was removed in turn to show its test fails, including the narrower fix that was rejected on fix(native-chat): mark a chat whose start failed on its tab and workspace row #24904.
  • 56 test files (499 tests) pass; web typecheck, lint, code-quality gate and React Doctor are clean; CI is green.
  • Review round 1 (correctness + architecture) and a readiness check found no P0–P2 issues caused by the PR.
  • Real app on Windows (isolated hidden test builds, stand-in agent, before = base, after = this PR; screenshots in the PR body): after a failed start, before shows Claude greyed out in the + menu and in "Send notes to > New agent"; after, Claude can be picked, a new chat opens, the failed chat keeps its Retry, and notes sent to a new agent arrive in the new chat. Repeated picks while the start keeps failing open one failed tab per pick, as intended. The "old failed chat restarted, notes dropped" path itself was not reproduced in the app: the before build blocks the menu pick, and only launchers that skip the menu check (such as source-control actions) reach it; tests cover it.

Not verified. The startup check that keeps a default terminal out of a workspace was confirmed by reading the code, not by a test: a failed chat always has its tab, and that tab counts as open. Native chat runs only in local Git worktrees and folder workspaces, so SSH and paired remote workspaces are not affected.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 3, 2026 02:19

@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

  • Retire the stored identity slot — structured-agent-session-launch-registry.ts drops pendingStructuredLaunchesByIdentity; only structuredLaunchesBySessionId remains, and the coalescing holder is derived via holdsLaunchIdentity / structuredLaunchesHoldingIdentity / getJoinableStructuredLaunchState. Failed and cancelled launches no longer hold an identity, so a new start opens a new chat.
  • Status stops counting failed starts — new structured-agent-session-launch-status.ts owns getStructuredAgentLaunchStatus, which scans only identities that still hold (blank identity or :resume: prefix), excluding failed/cancelled. Re-exported from structured-agent-session-launch.ts.
  • Join seam — structuredAgentLaunchState now uses getJoinableStructuredLaunchState; retrying is just existing.visibilityUnknown, correctly making the old outcome === 'failed' branch dead.
  • Tests — new structured-agent-session-launch-after-failure.test.ts (new-chat-with-own-prompt, retry-beside-in-flight, reload-does-not-unregister, failed-resume, unconfirmed-still-joins) and QuickLaunchButton.launch-status.test.tsx; the two structured-agent-session-launch.test.ts retry tests now route through retryStructuredAgentSessionLaunch, preserving their same-session/new-operation/no-double-stage assertions.

I read the full diff and the surrounding registry/launch/status/recovery modules, checked the three removed symbols for remaining callers or doc references (none), and confirmed retry, cancellation, persistence, and the startup-terminal gate still key off session id. Targeted vitest for the seven affected files passes (46 tests).

One non-blocking observation carried forward from the PR description: a new (non-resume) start still joins an unconfirmed or retried launch and drops its text. The author explicitly scopes that out as a follow-up with no ticket filed yet — noting it only so the deferral is visible on the PR record.

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

@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Structured agent launches now carry request IDs that distinguish repeated delivery of one action from separate actions. Launch-state joining and retry handling use request identity, attempt data, launch visibility, and chat contents. Note-send flows hold handed-off notes while delivery is pending and make them eligible again when delivery settles without success. Send components pass delivery promises to tracking callbacks and prevent empty-prompt sends. Tests cover launch outcomes, retries, note delivery, and disabled actions.

Priority: ➖ Normal

Merge Risk: 🔵 Low · up to d70c7

A focused regression test would improve confidence in this launch edge case. No confirmed production failure from it is established here.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to d70c7

A failure-recovery path can confuse independent sends containing identical text, risking incorrect delivery tracking and note removal. The normal launch paths retain ownership checks, but the new recovery lifecycle needs stronger action-level isolation.

Retained concerns

  • Medium · reliability · inferred: Failed delivery recovery selects a staged prompt across all registered launches using only matching text, without binding it to the originating request, workspace, agent, or host. Independent equal-text launches can therefore attach a note hold to another chat's lifecycle. Spending that unrelated entry can mark the original notes delivered; retaining it can keep those notes unavailable after their intended send failed. This weakens failure containment and action-level state ownership. The selection is observed in source; the resulting incorrect removal or hold is inferred, not execution-verified.
Security review details

Security Blast Radius

  • inferred — The delivery-recovery correlation problem is bounded to launch states registered in the same renderer. Because the fallback does not filter workspace, agent, host, or request identity, independent launches within that renderer can influence each other's note-delivery bookkeeping. Remote exploitation, cross-user access, and increased execution privileges were not established.

Trust Boundaries and Controls

  • observed — The inspected paired route resolves an environment-backed host target and requires a worktree and supported provider. Paired refusal uses an explicit terminal fallback plan; the recursive renderer call retains the original caller arguments rather than replanning the request into structured chat. Remote-side permission enforcement remains outside the inspected evidence.

Resilience and Maintainability Implications

  • observed — Session-bound outcome tracking and the inspected single-launch tests provide counterevidence: failed starts retain notes, successful retry spends them, and cancellation releases them without delivery. These tests were inspected, not executed, and do not resolve the independent equal-text launch collision in global fallback recovery.

Hardening Proposals

  • proposed — Carry the originating action's stable identity into delivery recovery and bind it to the resulting session and outbox entry. Treat an ambiguous text-only match as unresolved rather than allowing another launch's settlement to authorize note removal.
🚥 Pre-merge checks | ✅ 4 | ❓ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ❓ Inconclusive Docstring coverage is 26.60% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 94 functions across 78 files. (22 skipped… 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: opening a new chat after an earlier start fails.
Description check ✅ Passed The description covers the required sections with detailed user impact, mechanism, rationale, linked issue, visual proof, and testing. Automated tests are checked and reported; manual testing and the …
Full details: Docstring Coverage

Explanation

Docstring coverage is 26.60% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 94 functions across 78 files. (22 skipped: 22 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: f5faddfa-1942-418d-9524-585c7594a041
📥 Commits

Reviewing files that changed from the base of the PR and between e80b550 and c4751e2.

📒 Files selected for processing (6)
  • src/renderer/src/components/tab-bar/QuickLaunchButton.launch-status.test.tsx
  • src/renderer/src/lib/structured-agent-session-launch-after-failure.test.ts
  • src/renderer/src/lib/structured-agent-session-launch-registry.ts
  • src/renderer/src/lib/structured-agent-session-launch-status.ts
  • src/renderer/src/lib/structured-agent-session-launch.test.ts
  • src/renderer/src/lib/structured-agent-session-launch.ts

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

Comment on lines 95 to 99
export function getJoinableStructuredLaunchState(
identity: string
): StructuredLaunchState | undefined {
return structuredLaunchesHoldingIdentity((candidate) => candidate === identity).at(-1)
}

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

sed -n '75,115p' src/renderer/src/lib/structured-agent-session-launch-registry.ts
sed -n '155,240p' src/renderer/src/lib/structured-agent-session-launch.ts
sed -n '290,330p' src/renderer/src/lib/structured-agent-session-launch.ts
sed -n '190,245p' src/renderer/src/lib/structured-agent-session-launch-after-failure.test.ts

Repository: stablyai/orca

Length of output: 9746


🏁 Script executed:

sed -n '1,155p' src/renderer/src/lib/structured-agent-session-launch-registry.ts
sed -n '1,95p' src/renderer/src/lib/structured-agent-session-launch.ts
sed -n '175,260p' src/renderer/src/lib/structured-agent-session-launch.ts
sed -n '285,325p' src/renderer/src/lib/structured-agent-session-launch.ts
sed -n '1,120p' src/renderer/src/lib/structured-agent-session-launch-after-failure.test.ts
sed -n '180,245p' src/renderer/src/lib/structured-agent-session-launch-after-failure.test.ts
rg -n "enqueueStructuredAgentSessionLaunchPrompt|promptDelivery|retryStructuredAgentSessionLaunch\\(|startStructuredAgentLaunch\\(" src/renderer/src/lib/structured-agent-session-launch*

Repository: stablyai/orca

Length of output: 41150


🏁 Script executed:

rg -n "retryStructuredAgentSessionLaunch|startStructuredAgentLaunch\\(" src/renderer/src -g '*.{ts,tsx}'
sed -n '1,120p' src/renderer/src/lib/structured-agent-session-launch-recovery.ts
sed -n '1,100p' src/renderer/src/lib/structured-agent-session-launch-reload.ts
sed -n '1,130p' src/renderer/src/lib/structured-agent-session-launch-callers.ts
sed -n '145,220p' src/renderer/src/lib/structured-agent-session-launch-prompt.ts
sed -n '1,120p' src/renderer/src/components/native-chat/structured-agent-session-outbox-storage.ts
rg -n "createdAt|startedAt|sequence|timestamp|intent.*sessionId" src/renderer/src/lib/structured-agent-session-launch{,-recovery,-reload,-registry}.ts

Repository: stablyai/orca

Length of output: 30655


Preserve original launch order when restoring joinable states.

After a registry reset, a new same-identity launch is registered before the older failed session is restored for retry. While both are pending, .at(-1) selects the restored session. A subsequent startStructuredAgentLaunch returns that older session ID and can stage a non-draft prompt in its outbox. Select by original session launch order, so an in-flight retry remains joinable when no newer matching launch exists.

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

Important

The origin/main merge left two of this PR's new test files on the pre-merge intent shape, so pnpm run typecheck:web and the branch's tests are currently red.

Reviewed changes

This run re-reviewed the delta since the prior pullfrog review at c4751e2:

  • Merged origin/main — cb64b5e0 folds in main's paired-host work, which added required executionHostId/target fields to StructuredAgentSessionLaunchIntent and changed structuredAgentLaunchRecordFor to read intent.target.kind.
  • Repointed a test import — 8ca0065 moves structured-agent-session-paired-admission.test.ts's getStructuredAgentLaunchStatus import from the registry to the new structured-agent-session-launch-status module.

The PR's own production changes (registry slot refactor, status module, join seam) are unchanged by these commits, and the remaining affected suites pass (structured-agent-session-launch.test.ts 26, paired-admission 7, resume-identity 6, initial-terminal-structured-launch). The two failures are the stale fixtures noted inline.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@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 run re-reviewed the delta since the prior pullfrog review at 8ca0065 (one commit, 04ebe48f):

  • Repaired the two stale intent fixtures — structured-agent-session-launch-after-failure.test.ts launchIntent() and QuickLaunchButton.launch-status.test.tsx registerLaunch() now pass executionHostId: 'local' and target: { kind: 'local' }, matching the required StructuredAgentSessionLaunchIntent shape introduced by the origin/main merge. This clears the prior review's typecheck/runtime blocker.

Verified in this run: pnpm run typecheck:web exits 0, and the two suites pass 6/6. Production code is unchanged since the initial review and had no concerns.

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

@brennanb2025
brennanb2025 marked this pull request as draft October 4, 2026 05:50
A new start for an agent joined a chat whose create answer was lost, or a
failed chat whose Retry was in flight, and dropped its own text: the
source-control "Fix with AI" action reported success with nothing sent,
"Send notes to > New agent" sent nothing, and after a reload the text became
an unsent draft in the old chat.

Only a new start's own first create now coalesces later starts (double click,
two callers at once). Each attempt records whether it is that first create or
a Retry/re-check of an existing chat; an unconfirmed or retried blank chat
keeps its own Retry and a new start opens a new chat with its text. A resume
still re-checks or joins its conversation's launch, since the host refuses a
second adoption.
A new start used to join any blank chat still in its first create or still
sending its opening text, so a second "Fix with AI" for another check, or
notes sent to a new agent, landed in that first chat. Each first attempt now
keeps the request it was started with (its text and whether it is sent or
drafted). Only a repeat of that request (a double click, a retried call)
joins it, and it shares the text already staged, so the text is sent once.
Any other request opens a new chat with its own text. The + menu, new-tab
search and send-notes menus disable an agent only when their own pick would
join. Resume launches are unchanged.
…ent to it

A blank chat that is still starting (a + pick, the empty-workspace default
chat) is empty, so the first request with text, such as notes sent to a new
agent, now goes into it instead of opening a second chat beside it. That
request becomes the chat's own request: an identical repeat joins and is sent
once, and any other request opens a new chat.

Move the logic that decides which launch a start joins out of the launch
registry into its own module, so the registry stays under the line limit
once #24904 lands beside it. Pin that a repeat arriving while the opening
text is still sending is sent once.
… request's text

A blank chat that is still starting was claimable by the first request with
text even after its user had sent a message into it or typed into its
composer, so notes or a Fix with AI prompt could land in a conversation the
user had already started. Emptiness is now read from what the chat holds:
nothing in its outbox and no text in its composer draft. A chat its user has
used stays theirs, and the request opens a new chat.

If the claiming text cannot be saved, the claim is not recorded: the request
falls through to a new launch, whose save failure shows on that chat as for
any new launch, instead of reporting a failure nothing showed.
Notes sent to an agent stayed in the notes shelf until their chat delivered
them, so a second "Send notes" made while the first new chat was still
starting collected the first notes again. With every different request now
opening its own chat, those notes reached two chats.

When notes or browser annotations are handed to a send (a new agent, a
running agent from the menu, or a sidebar agent row), they are held in
memory against that send's own delivery result and left out of the next
send. Delivered notes are removed as before; a failed, refused or
undelivered send releases the hold, so they come back for the next send.
The notes menu now builds each scope's prompt from the notes it will send.
When every note or annotation in a send is held by an earlier send still in
flight, the built prompt is empty. The notes menu already disabled its
trigger then, but its New agent rows still started an agent with no text,
the annotation Send buttons still opened, and a sidebar agent row could be
sent an empty prompt. The New agent rows, the annotation Send buttons and the
sidebar send now offer nothing when there is nothing to send.
…he shelf

Notes sent to a new agent were released back to the shelf as soon as the
chat's start failed, while that failed chat kept the same text staged for its
own Retry. Re-sending and pressing Retry put the notes in two chats, and Retry
alone left delivered notes listed as unsent.

For a new chat, the notes now follow the prompt it staged: held while that
message is in the chat's outbox, cleared from the shelf when any dispatch
(Retry or re-check included) sends it on, and back on the shelf when the
chat is closed and its outbox thrown away. A paired server's chat is found
once its start settles. Running-agent sends keep their own result.

While notes are only on their way, the send button reads "Sending…" rather
than "All notes sent".
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Follow-up: "new is always new"

The problem. Beyond failed starts, a request for a new chat could still land in an existing chat and lose its text. This happened when an earlier chat's start couldn't be confirmed, while an old chat's Retry was running, and even while a first chat was still starting or sending its opening text. "Fix with AI" would report "Started an AI agent" while its instructions went nowhere. Notes sent twice in quick succession reached the agent twice.

What changes for you.

  • A new request always opens its own chat with its own text. A double click (the very same request) still makes one chat, and the text is sent once.
  • A blank chat that is still starting, with nothing sent or typed in it yet, takes the first text sent to it, so sending notes right after opening a blank chat doesn't open a second one. A chat you have already used is never taken over.
  • Notes and browser annotations you have sent leave the next send immediately. If that send fails or you close the chat, they come back. While all of them are on their way, the button says "Sending…" and offers no empty send.

How. Each launch attempt records whether it is the first or a retry, and what its opening request was. Only a repeat of that request (or a claim of an empty starting chat) joins it. A new chat's saved opening message owns the notes it carries until that message is accepted (Retry included) or discarded. Nothing new is stored on disk.

Scope. This PR now also changes the notes and annotation send code (about 20 files). It is the same rule — a new request carries its own text, exactly once — and without it the launch fix would turn an existing duplicate into a duplicate across two chats.

Merge order. Merge this before #24904 (or together). Merged together locally: lint, 126 test files (1,286 tests) and the web typecheck pass. The only conflict is one type, where both new fields are kept.

Review. Rounds 2–4, narrow checks of each fix, and a final readiness check: no P0–P2 issues remain. Deferred P3s:

  • with annotations all on their way, their Send button is greyed out without a "Sending…" reason;
  • send counts still include notes on their way;
  • a tab closed while its host can't be named keeps its notes "Sending…" until a reload;
  • a few test-coverage gaps.

Disclosed limits (labelled in the body):

  • a reload drops the in-memory note links, so notes fail open back to the shelf;
  • where "Fix with AI" text differs between clicks (re-fetched check logs), two separate clicks make two chats.

Verified / not verified. Captured on macOS (isolated test builds, stand-in Claude, before = base, after = 60d7e010e59; screenshots in the body):

  • with an unconfirmed chat, + > Claude and "Send notes > New agent" re-check the old chat and drop the note in the before build, and open a new chat with the note in the after build;
  • notes on their way show "Sending…" and a second note opens its own chat, with each note delivered once;
  • notes sent into a failed start stay held and come back when the chat is closed.

Found, not caused by this PR (same in both builds): if Claude quits during startup after its chat was created, notes sent to it leave the shelf; the chat shows "Your message was not sent." and they don't return to the shelf when the chat is closed. Not captured: Codex, a source-control "Fix with AI" start, and a reload during a notes send. The earlier Windows captures still hold.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 4, 2026 10:28
@brennanb2025
brennanb2025 marked this pull request as draft October 4, 2026 10:36

@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 run read the complete raw diff at 60d7e010e59 — the PR is now 46 files (+2493/−225), far beyond the earlier 7-file launch-registry revision that the prior pullfrog approvals covered, so this is a fresh review of the whole change.

  • Retire the stored identity slot — structured-agent-session-launch-registry.ts drops pendingStructuredLaunchesByIdentity; only structuredLaunchesBySessionId remains. setStructuredLaunchState, deleteStructuredLaunchStateIfCurrent, structuredLaunchStates and notifyStructuredLaunchListeners key on session id.
  • Derive the slot holder — new structured-agent-session-launch-holders.ts: holdsLaunchIdentity (false for failed/cancelled; true for any resume; else a non-unknown first attempt whose request the caller repeats), getJoinableStructuredLaunchState, and claimableStructuredLaunchAttempt for a still-empty blank chat.
  • Request/attempt model — new structured-agent-session-launch-request.ts (StructuredLaunchRequest, StructuredLaunchAttempt first/retry, repeatedStructuredLaunchAttempt).
  • Join seam — structured-agent-session-launch.ts moves joining into joinStructuredLaunchState: a claim stages the claimer's text before committing (an unstaged claim falls through to a new chat), a repeat reuses the first click's staged entry, an unknown/retry join carries no text. New starts/retries record the attempt.
  • Status — new structured-agent-session-launch-status.ts owns getStructuredAgentLaunchStatus(worktreeId, agent, request?); failed/unconfirmed no longer read as starting. Re-exported from structured-agent-session-launch.ts.
  • Menus pass their own request — QuickLaunchButton.tsx and TabBarCreateEntry.tsx feed structuredLaunchRequest/BLANK_STRUCTURED_LAUNCH_REQUEST into the status hook.
  • Notes/annotations in-flight hold — new notes-send-in-flight.ts, new-agent-prompt-outcome.ts, and structured-agent-session-outbox-entry-watch.ts; NotesSendMenu, ReviewNotesSendMenuContent, the diff/markdown scopes, the browser annotation tray/banners, and sendPromptToSidebarAgentTarget now hold notes until their own send settles and offer no empty send while every note is on its way.

I traced the coalescing/holder logic, the in-flight hold lifecycle (stuck/duplicated/lost notes), and the outbox watch equality against every outbox mutation (stage, refusal requeue with id rotation, reconcile, discard). The new tests assert exact sessions, sessions/text pairs, and delivery results rather than loose truths, and the targeted suites exercise the real launch/outbox paths.

Scope notes (no action needed on this branch)

  • holdsLaunchIdentity's resume branch returns true for any request, so a resume start carrying text while that resume is unconfirmed would join it and drop the text. I confirmed no production caller passes resumeFrom together with a prompt today (the AI-vault resume passes resumeFrom only; fork passes a prompt only), and the PR body documents this exact case as intended.
  • newAgentPromptOutcome never invokes the unsubscribe from watchStructuredAgentSessionOutboxEntry, so a failed new-agent chat held indefinitely keeps one watch and one hold entry. Bounded and escapable (Retry, close, or reload all settle it).

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

… send across a reload

The hold that keeps notes sent to a new agent out of the next send lived only
in memory. A reload while that chat had not yet sent them put the notes back
on the shelf while the chat's saved message still carried their text, so a
second "Send notes" sent them twice.

The staged message now saves the send keys of the notes it was built from.
The shelf treats a note as on its way while any saved message carries its key,
read from the saved outboxes on first use and kept current by the outbox's one
write funnel. When a message carrying notes is sent on (its own start, a Retry
or the re-check), those notes are cleared from their shelf; when it is thrown
away with its chat, they come back. Browser annotations sent to a new chat use
the same keys while the app runs. Running-agent sends keep their in-memory hold.

This replaces the per-message watch and outcome promise from the previous
change.
…eb client too

Two gaps in keeping notes out of another send while a saved chat message
carries them:

- The web client never installed the clearing that removes notes once a
  chat sends them on, so after a Retry there the notes stayed listed. It is
  now installed, once per renderer, by the background services both the
  desktop and the web client load with App.
- A saved message carrying notes was thrown away only by this window's own
  close. A chat closed from the phone, another window or while Orca was off,
  or closed here without a known host, kept its notes held for good. A chat
  the host stops listing (affirmed, and not a launch still starting or
  failed) now has its queued messages thrown away once that sync lands, as
  does a close that can name no host, so the notes come back.
… another send across a reload"

This reverts commit b88d0dc.
brennanb2025 added a commit that referenced this pull request Oct 4, 2026
… send is on its way) into C2

Both add a hook where the outbox commits an entry's end; both are kept here and folded into one next.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Follow-up: back to the in-memory notes hold

An attempt to keep "notes on their way" across a reload was reverted. It saved which notes a chat's message carried, then released or discarded them when a chat looked gone. A final check showed that the "gone" signal can't be trusted when the same checkout is on this machine and on a paired server: a sync from one host could have deleted the other host's queued messages. The PR is back to the reviewed in-memory hold. The head a3c1d7c8a0e has the same tree as 60d7e010e59, which is the commit the macOS screenshots were taken on.

What that means for you: everything in the earlier comment still holds, except one case, now labelled Temporary in the body. If Orca reloads while a send of notes to a new agent is still unanswered, the notes reappear in the shelf while that chat can still send them, so sending them again would deliver them twice. Main has the same problem; this PR narrows it to the reload case. #25207 owns the chat outbox and will derive "on their way" from the saved message itself, without deleting anything on inference.

Checks on the reverted head: 118 test files (1,217 tests) pass, the web typecheck from scratch is clean, and CI is green. One CI shard failed once on an unrelated terminal-notification test, which passes locally and on re-run. Merge order is unchanged: this PR before #24904.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 4, 2026 11:59

@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 run reviews head a3c1d7c8a0, whose tree is byte-identical to the previously reviewed 60d7e010e59: the two commits on top are a fix followed by its revert, twice (b88d0dc2c↔a3c1d7c8a0, ae0b9f3fea↔19cabc72b), so git diff 60d7e01 a3c1d7c is empty. The change set is the full 46-file revision:

  • Retire the stored identity slot — structured-agent-session-launch-registry.ts drops pendingStructuredLaunchesByIdentity; only structuredLaunchesBySessionId remains, and every read/write (setStructuredLaunchState, deleteStructuredLaunchStateIfCurrent, structuredLaunchStates, notifyStructuredLaunchListeners) keys on session id.
  • Derive the slot holder — new structured-agent-session-launch-holders.ts: holdsLaunchIdentity (false for failed/cancelled; true for any resume; else a non-unknown first attempt whose request the caller repeats), getJoinableStructuredLaunchState, and claimableStructuredLaunchAttempt for a still-empty blank chat.
  • Request/attempt model — new structured-agent-session-launch-request.ts (StructuredLaunchRequest, StructuredLaunchAttempt first/retry, repeatedStructuredLaunchAttempt).
  • Join seam — structured-agent-session-launch.ts moves joining into joinStructuredLaunchState: a claim stages the claimer's text before committing (an unstaged claim returns undefined → new chat), a repeat reuses the first click's staged entry, an unknown/retry join carries no text.
  • Status — new structured-agent-session-launch-status.ts owns getStructuredAgentLaunchStatus(worktreeId, agent, request?); failed/unconfirmed no longer read as starting.
  • Menus pass their own request — QuickLaunchButton.tsx and TabBarCreateEntry.tsx feed structuredLaunchRequest/BLANK_STRUCTURED_LAUNCH_REQUEST into the status hook.
  • Notes/annotations in-flight hold — new notes-send-in-flight.ts, new-agent-prompt-outcome.ts, and structured-agent-session-outbox-entry-watch.ts; the notes/annotation send menus hold a note until its own send settles and offer no empty send while every note is on its way.

I re-verified the holder derivation, the request/attempt model, the join seam (structured-agent-session-launch.ts:159-205), the empty-chat claim (structured-agent-session-launch-empty-chat.ts), the in-flight hold ref-counting, the outbox watch equality, and the sidebar/annotation/new-agent send callbacks. The one unresolved review thread (CodeRabbit #PRRT_kwDORpCz1s6ojHm9, outdated) claims .at(-1) in getJoinableStructuredLaunchState can select a restored failed session over a newer launch; that is not reachable — structured-agent-session-launch-reload.ts:46 restores callers as {kind:'retry'}, so holdsLaunchIdentity is false for them and they are never candidates. Two non-blocking, already-documented scope notes stand (resume branch returns true for any request; newAgentPromptOutcome does not unsubscribe a watch for an indefinitely-held failed chat).

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

…r action

"The same request" was recognised by its content (agent, workspace, trimmed
text, sent or drafted), which split one action whose text changes between
deliveries ("Fix with AI" re-fetches logs) and merged two different actions
that happen to carry the same text.

Each user action now mints one request id where it is handled (the click,
menu pick, notes send, shortcut, Fix with AI press, quick command; a worktree
create uses its creation id; programmatic starts mint once at their entry)
and passes it through the launch plan, its verdict and the structured launch
options, where it is required. A new start joins a chat only when its first
attempt carries the same id: a double click or a caller retrying its own call
makes one chat and sends the first delivery's text once; any other action
opens its own chat, whatever its text. An empty chat that is still starting
is still claimed once by the first action with text, and then belongs to that
action's id. Resume launches keep joining their conversation's launch.

The + menu and new-tab search no longer grey out an agent while one of its
chats starts: every pick is a new action and opens its own chat. The id lives
in memory only; nothing reads it after a reload (a restored launch is reached
by its session id through Retry, never joined).
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Follow-up: every new-chat request now carries its own request id

Why. Brennan asked why our callers don't carry a request id. Until now this PR recognised "the same request" by its content: the same agent, workspace and text while the first chat was still starting. That guess could go wrong both ways. It could split one action into two chats ("Fix with AI" re-fetches the check logs, so its text can differ between deliveries), or merge two different actions that happen to have the same text.

What changed. Each click, menu pick or send now mints one request id at the moment you act. Programmatic starts (the empty workspace's default chat, onboarding) mint one at their entry, and a worktree-create chat uses its create's id. The id is a required field on every new-chat launch. A new chat joins an earlier one only when it carries the same id and that chat is still starting; a repeat never sends text of its own, so the text goes out once. That is the common pattern: mint the id at the gesture and dedupe on it. The content matching is gone. The empty-chat rule stays: a blank chat that is still starting, with nothing sent or typed in it, takes the first request that carries text. The id is not saved with the launch record, because after a reload nothing reads it.

What's different for you.

  • Two actions with the same text now open two chats.
  • The same action delivered twice (a double click) still makes one chat.
  • A click after a failed start opens a new chat.
  • New: the + menu and the new-tab search no longer grey out an agent while one of its chats is starting. Every pick is its own request and opens its own chat.
  • Pressing the same button again after its first start has begun opens a second chat. Those buttons still block repeat presses while starting, and the new chat comes to the front at once. This is labelled Intended in the body.

Checks. 150 test files (1,528 tests) pass, and the new tests fail at the previous head. The web typecheck from scratch, lint, the reliability-gate check and CI are green. A narrow review found no P0–P2 issues; its P3s are deferred (a stale test file name, a narrow comment). The code merged with #24904 lints clean, with only the known two-field conflict.

Not re-captured. The macOS screenshots in the body were taken on 60d7e010e59; the flows they show behave the same. The only new visible difference, the agent no longer greyed out while a chat starts, is covered by a component test, not a screenshot. Merge order is unchanged: this PR before #24904.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 4, 2026 22:10
@brennanb2025
brennanb2025 marked this pull request as draft October 4, 2026 22:12

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

🧹 Nitpick comments (1)
src/renderer/src/lib/structured-agent-session-launch.ts (1)

165-195: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add a regression case for an unconfirmed blank launch.

The current unknown-launch test starts with prompt: 'first task', so its attempt is not claimable. In structured-agent-session-launch-unconfirmed-or-retried.test.ts, start a blank launch, leave it visibility-unknown, then submit text with a new request. Assert that the request opens a new session and sends its text there.


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 89081724-80c0-4bfa-b083-eda04aaf405f
📥 Commits

Reviewing files that changed from the base of the PR and between a3c1d7c and d70c7eb.

📒 Files selected for processing (73)
  • src/renderer/src/components/dashboard/launch-dashboard-agent.test.ts
  • src/renderer/src/components/dashboard/launch-dashboard-agent.ts
  • src/renderer/src/components/floating-terminal/FloatingTerminalWindowControls.test.tsx
  • src/renderer/src/components/floating-terminal/FloatingTerminalWindowControls.tsx
  • src/renderer/src/components/native-chat/use-native-chat-provisional-launch.test.tsx
  • src/renderer/src/components/right-sidebar/ai-vault-session-launch-actions.ts
  • src/renderer/src/components/right-sidebar/ai-vault-session-resume-in-chat-launch.test.ts
  • src/renderer/src/components/right-sidebar/ai-vault-session-resume-in-chat-launch.ts
  • src/renderer/src/components/right-sidebar/runSourceControlAgentActionStart.ts
  • src/renderer/src/components/right-sidebar/source-control-launch-agent-args-applicability.test.ts
  • src/renderer/src/components/right-sidebar/source-control-launch-agent-args-applicability.ts
  • src/renderer/src/components/right-sidebar/source-control/ai/recovery-launch.ts
  • src/renderer/src/components/right-sidebar/source-control/sync/use-git-history-commit-actions.ts
  • src/renderer/src/components/sidebar/folder-workspace-composer-submit.ts
  • src/renderer/src/components/tab-bar/QuickLaunchButton.launch-status.test.tsx
  • src/renderer/src/components/tab-bar/QuickLaunchButton.tsx
  • src/renderer/src/components/tab-bar/TabBarCreateEntry.keyboard.test.tsx
  • src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx
  • src/renderer/src/components/tab-bar/use-tab-bar-create-menu-controller.ts
  • src/renderer/src/components/terminal-pane/terminal-agent-session-fork.ts
  • src/renderer/src/components/use-terminal-create-actions.ts
  • src/renderer/src/hooks/composer-state/full-creation-execution.ts
  • src/renderer/src/hooks/composer-state/quick-creation-execution.ts
  • src/renderer/src/lib/agent-launch-caller-permissions.test.ts
  • src/renderer/src/lib/agent-launch-caller-placement.test.ts
  • src/renderer/src/lib/agent-launch-caller-profiles-test-harness.ts
  • src/renderer/src/lib/agent-launch-caller-prompt-transport.test.ts
  • src/renderer/src/lib/agent-launch-caller-routing.test.ts
  • src/renderer/src/lib/agent-launch-request-id.ts
  • src/renderer/src/lib/agent-launch-route-connection-fallback.test.ts
  • src/renderer/src/lib/agent-launch-route-input.test.ts
  • src/renderer/src/lib/agent-session-launch-plan.test.ts
  • src/renderer/src/lib/agent-session-launch-plan.ts
  • src/renderer/src/lib/empty-workspace-default-agent-chat.ts
  • src/renderer/src/lib/fix-checks-agent-launch.ts
  • src/renderer/src/lib/launch-agent-in-new-tab-cwd.test.ts
  • src/renderer/src/lib/launch-agent-in-new-tab-host-resolution.test.ts
  • src/renderer/src/lib/launch-agent-in-new-tab-main-surface.test.ts
  • src/renderer/src/lib/launch-agent-in-new-tab-placement.test.ts
  • src/renderer/src/lib/launch-agent-in-new-tab-structured-route.ts
  • src/renderer/src/lib/launch-agent-in-new-tab-structured.test.ts
  • src/renderer/src/lib/launch-agent-in-new-tab-web-runtime.test.ts
  • src/renderer/src/lib/launch-agent-in-new-tab-windows-quoting.test.ts
  • src/renderer/src/lib/launch-agent-in-new-tab.test.ts
  • src/renderer/src/lib/launch-agent-in-new-tab.ts
  • src/renderer/src/lib/launch-agent-session-continuation.ts
  • src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts
  • src/renderer/src/lib/launch-work-item-direct-agent-routing.test.ts
  • src/renderer/src/lib/launch-work-item-direct-route-preparation.ts
  • src/renderer/src/lib/launch-work-item-direct.ts
  • src/renderer/src/lib/new-agent-prompt-outcome.test.ts
  • src/renderer/src/lib/onboarding-folder-agent-launch.ts
  • src/renderer/src/lib/run-quick-command-in-new-tab.test.ts
  • src/renderer/src/lib/run-quick-command-in-new-tab.ts
  • src/renderer/src/lib/structured-agent-launch-settlement.test.ts
  • src/renderer/src/lib/structured-agent-session-launch-after-failure.test.ts
  • src/renderer/src/lib/structured-agent-session-launch-callers.ts
  • src/renderer/src/lib/structured-agent-session-launch-cancellation.test.ts
  • src/renderer/src/lib/structured-agent-session-launch-close-race.test.ts
  • src/renderer/src/lib/structured-agent-session-launch-different-request.test.ts
  • src/renderer/src/lib/structured-agent-session-launch-exited-owner.test.ts
  • src/renderer/src/lib/structured-agent-session-launch-holders.ts
  • src/renderer/src/lib/structured-agent-session-launch-join-delivery.test.ts
  • src/renderer/src/lib/structured-agent-session-launch-options.test.ts
  • src/renderer/src/lib/structured-agent-session-launch-request.ts
  • src/renderer/src/lib/structured-agent-session-launch-resume-identity.test.ts
  • src/renderer/src/lib/structured-agent-session-launch-status.ts
  • src/renderer/src/lib/structured-agent-session-launch-unconfirmed-or-retried.test.ts
  • src/renderer/src/lib/structured-agent-session-launch.test.ts
  • src/renderer/src/lib/structured-agent-session-launch.ts
  • src/renderer/src/lib/structured-agent-session-paired-admission.test.ts
  • src/renderer/src/lib/structured-agent-session-paired-admission.ts
  • src/renderer/src/lib/worktree-creation-structured-session.ts
💤 Files with no reviewable changes (1)
  • src/renderer/src/components/tab-bar/TabBarCreateEntry.keyboard.test.tsx

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

brennanb2025 added a commit that referenced this pull request Oct 4, 2026
Takes #24917's request ids everywhere a launch is started: a launch joins another only on an equal
id during its first attempt, and every launch call site passes one. C2's notes hold stays the only
one: the staged message carries the notes' keys and the hold derives from the saved outbox, so
#24917's in-memory hold and new-agent-prompt-outcome stay deleted. Its new test case (a chat
still starting is closed without waiting on its create) is ported into notes-carried-by-chat.

@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 run reviews the delta at head d70c7eb61d (73 files, +793/−416) over the previously reviewed a3c1d7c8a0. That commit replaces content-based request matching with a per-user-action request id, and I read the full 101-file PR diff end-to-end plus every production call site.

  • Request ids replace text matching — new agent-launch-request-id.ts; StructuredLaunchRequest is now {id, hasText} and StructuredLaunchAttempt.first records {requestId, blank, stagedEntry}. repeatedStructuredLaunchAttempt compares ids only, so one action's text can be re-fetched between deliveries without splitting the chat, and two actions with identical text no longer merge.
  • Holder derivation — holdsLaunchIdentity(state, requestId?) is unchanged for failed/cancelled/resume and now matches a first attempt by id; getJoinableStructuredLaunchState joins the last state holding the request id, else the last claimable empty first attempt.
  • Join seam — joinStructuredLaunchState suppresses the empty-chat claim on a repeat, stages a repeat's first entry once (stagedEntry: repeat?.stagedEntry), and drops a repeat's own prompt when it staged none.
  • Status hook deleted — useStructuredAgentLaunchStatus is gone; QuickLaunchButton and TabBarCreateEntry no longer disable an agent while one of its chats starts (each pick is its own request). The remaining getStructuredAgentLaunchStatus consumer, default-terminal seeding, is behaviorally unchanged.
  • Route-only helper — resolveAgentSessionLaunchRoute added for callers that branch on the route without planning a settlement (sourceControlLaunchAppliesAgentArgs, quick-creation-execution).
  • Id plumbing — required on the verdict/options; a preflighted plan carries its own id (LaunchAgentInNewTabArgs is a {requestId} / {agentSessionLaunchPlan} union); the worktree-create route reuses creationId so a retry re-delivers the same create.

The notes/annotation in-flight hold (notes-send-in-flight.ts, new-agent-prompt-outcome.ts, the outbox entry watch, and the notes/annotation menus) is unchanged by this commit; I re-read it and it matches the previously approved 60d7e010 revision.

I traced the coalescing state machine (holds/claim/repeat/retrying), the empty-chat claim, the plan → settlement → start threading, and audited every production newAgentLaunchRequestId() call site. Each mints once per user gesture (no loops or render bodies), the id is never lost between planning and the structured start, and the removed status hook left no dangling references. Two focused specialist passes (coalescing transitions; call-site granularity) found no production-reachable defect. The existing stale CodeRabbit thread (getJoinableStructuredLaunchState .at(-1)) remains invalid — restored launches are {kind: 'retry'} and never candidates. The one logical hole at the exported-function boundary (a same-id re-delivery whose first attempt was blank and second carries text is dropped) has no production caller, since plans carry a fixed prompt and per-action callers mint fresh ids.

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

… of stacking another

Two bare "+ > Claude" picks (or new-tab search, the new-agent shortcut, the dashboard) opened two
empty chats. A request with no text now focuses an existing empty chat for that agent in that
workspace: one still starting (joined in the launch, as a re-delivery is) or one that published and
sits idle (its host's journal holds no request, read live from the status feed). Empty also means
nothing queued, no composer text or images, and no launch draft its composer has not taken. Failed,
unconfirmed, resumed and other-agent/workspace chats are never reused. A request with text still
opens its own chat, or claims an empty starting one as before; request-id dedupe is unchanged.
…n the split it was opened from

Pressing + in the right-hand split focused an empty chat sitting in the left-hand split. The reuse
now looks only in the tab group the pick targets (the caller's group, else the workspace's active
group, which is where its tab would open); a pick in another split opens a new chat there. Applies
to both an empty chat still starting and one that published and sits idle.
…hat only in its own split

Notes sent from the right-hand split could land in, and focus, an empty chat still starting in the
left-hand one. A request with text now claims an empty starting chat only in the tab group it opens
in, the same rule a request without text follows; elsewhere it opens its own chat there.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Follow-up: a new chat with no text reuses an empty one

Why. After the request-id change, every pick opened its own chat, so two bare "+ > Claude" picks left two empty chats. The common pattern (checked first-hand) reuses a "new chat" that nothing has been typed or sent into, and only makes another one once that one is in use.

What changes for you.

  • A pick with no text (the + menu, the new-tab search, the new-agent shortcut, the dashboard) brings an existing empty chat for that agent to the front instead of opening another. This works whether that chat is still starting or already idle. It only applies to a chat in the same split you picked in; a pick in another split opens a chat there, so focus never jumps across splits.
  • "Empty" means nothing was sent into it and nothing is typed, attached or queued in its composer. For a chat that has already started, the host must also report no request at all, and the app must be connected to that host. A lost connection is never taken as "empty".
  • Failed, unconfirmed and resumed chats are never reused. A chat with anything in it stays, and a new one opens.
  • A request with text (notes, "Fix with AI", a quick command) is still its own chat. It may take over an empty chat that is still starting, but only in the same split.

Checks. New tests cover reuse while starting and while idle, every "not empty" case, other agents, other workspaces and other splits. Each rule was removed in turn to show its test fails. 181 targeted test files (1,836 tests) pass, plus 434 test files that reach the changed code indirectly. The from-scratch web typecheck, lint, the quality gates and CI are green. A narrow review found no P0–P2 issues. Its P3s are deferred: a duplicate history tab's composer key, model changes not counting as "used" (matches the common pattern), and the per-window draft caches.

Not re-captured. The screenshots in the body predate this change. Reuse is covered by tests that drive the real launch plan and tab-focus path. Merge order is unchanged: this PR before #24904.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 4, 2026 23:34

@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 run re-reviewed the delta since the prior pullfrog review at d70c7eb — three commits that add "an empty chat is reused instead of stacking another" to the structured-launch registry, plus the tests that pin it. The full 107-file change set was approved through d70c7eb; only the delta is described here.

  • Reuse an idle published empty chat — new structured-agent-session-idle-empty-chat.ts (findIdleEmptyStructuredChat): for a launch with no prompt and no resume, beginStructuredAgentLaunchSettlement returns an existing open chat for the same agent whose lifecycle is null/published, whose host feed is live with a null status, whose outbox/composer/attachments are empty, and whose launch draft is unadopted, instead of starting a new one. The reused chat is focused, not cancelled.
  • Reuse a starting empty chat, split-scoped — getJoinableStructuredLaunchState now matches an empty first attempt only in the request's tab group; StructuredLaunchRequest carries groupId; new structured-agent-session-chat-tab-group.ts reads a chat tab's group.
  • Thread the tab group through — targetGroupId on StructuredAgentLaunchOptions/AgentSessionLaunchTarget, resolved in beginLocalProvisionalLaunch (the caller's group, else the workspace's active group), so a pick in one split no longer focuses or opens in another.
  • Attachments count as "not empty" — isStructuredLaunchChatEmpty also requires no composer attachments.
  • Text claims are split-scoped too — the last commit drops the request.hasText bypass, so a request with text also claims an empty starting chat only in its own split.

I traced the reuse/claim paths through the real callers (QuickLaunchButton/QuickLaunchAgentMenuItems, ReviewNotesSendMenuContent, launchAgentInNewTab, beginStructuredAgentSessionProvisionalLaunch), confirmed the status feed that hostHoldsNoRequest depends on is activated for every structured tab (AppBackgroundServices mounts StructuredAgentSessionStatusBridge), and verified resumed, failed, unconfirmed, in-flight, and other-host/other-split chats are excluded. createUnifiedTab validates an unknown targetGroupId and falls back to the active group. The new structured-agent-session-empty-chat-reuse.test.ts (21 tests) asserts exact session ids, tab groups, and outbox contents; the affected launch/guard/provisional suites pass (68 tests), and typecheck:web is clean.

Scope notes (no action needed on this branch)

  • getJoinableStructuredLaunchState's empty-chat branch is group-scoped but not host-scoped (the identity is agent:worktree, and worktree ids repeat across hosts). This predates the delta and I could not reach it — a worktree's owner is normally pinned — so it is a latent hardening gap, not a defect.
  • A few NotesSendMenu callers pass groupId={worktreeId} (RichMarkdownReviewRailActions, MarkdownPreviewAnnotationComposer, CheckRunDetailsPanel). Because createUnifiedTab validates the target group and falls back to the active one, the tab still lands correctly; the only effect is that those flows never match the new same-split reuse/claim. Pre-existing prop misuse, not a regression.

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

brennanb2025 added a commit that referenced this pull request Oct 5, 2026
Takes #24917's round 7 as written: a new chat with no text reuses an empty
chat (still starting, or published and idle) in the tab group it was opened
from, with the group threaded from the provisional tab to the launch request.
C2's requestId, carriedNoteKeys and notes hand-off wiring is unchanged.

- The empty-chat check reads the composer's images from the draft store, not
  the composer hook: the launch path must not load the composer, which closes
  the store import cycle C2 already removed once.
- Two cases pin carried notes beside an empty chat: a notes send (it always has
  text) never reuses an idle chat and stages its one message with its notes,
  in its own chat or in the starting empty chat it takes.
@brennanb2025
brennanb2025 merged commit 0a8c572 into main Oct 5, 2026
55 checks passed
brennanb2025 added a commit that referenced this pull request Oct 5, 2026
…keep both caller-group fields; give the tests' starts their request ids
brennanb2025 added a commit that referenced this pull request Oct 5, 2026
#24917's empty-chat check read the composer draft under the pane key, while
this branch keeps a structured chat's draft under its conversation key, so a
chat holding typed or given-back text, or only an image, read as empty: a new
chat could reuse it and another request's text could go into it. It reads
the conversation's draft now, as C2 adapted it (d4e469b), with the tests
moved to that key.

While the startup load of saved drafts is still running, no chat reads as
empty: one may hold a saved draft memory doesn't have yet. The cost is only
that a new chat opens instead of reusing one during that moment.
brennanb2025 added a commit that referenced this pull request Oct 5, 2026
Main now carries #24917 as landed; C2 already holds that version, so the
files only main changed keep C2's resolution, and #24917's in-memory notes
hold (new-agent-prompt-outcome, the outbox entry watch) stays deleted: the
outbox-derived hold is the only one.

From #24905: the per-scope write confirm, data additions that report whether
they are durable (journaled at once), the load-pending guard on empty chats,
and its paste wording and chat-image test.

C2 moves onto them:
- A hand-back finishes at once when its addition is durable, and otherwise
  when the draft's own write is confirmed (nativeChatComposerDraftWriteSettled),
  re-checking a refused one on the draft's next landed save.
- It runs against the loaded drafts, waiting for the startup load, so the
  suffix rule and the image id check read what was saved (an addition made
  before the load is applied again to the loaded draft without them).
- The suffix rule stays C2's (the end, as its own paragraph); #24905's `once`
  matches any text ending, so it isn't used for it. A whitespace-only draft
  counts as empty.
brennanb2025 added a commit that referenced this pull request Oct 6, 2026
* fix(native-chat): an unsent draft survives a reload or quit

The composer's draft text, its editor document and settled image refs lived
only in module memory, so a reload, quit or crash lost whatever was typed,
including text a Stop had just given back. They are now saved per pane scope
in localStorage: typing is debounced and flushed when the window hides or
closes, text given back to the composer is saved at once, an emptied or sent
draft is removed at once, and only the newest drafts within the composer
caches' existing bound are kept.

* fix(native-chat): save draft images outside the state updater

* fix(native-chat): one store owns each unsent draft

The draft's text and its image chips were saved by two writers that each
rebuilt the stored record from what storage held, so a draft that once went
over the size cap, or one refused write, lost its images on the next save.

One store now holds each composer scope's whole draft in memory and writes
storage from it: typing is coalesced, returned text, chips and clears land at
once, a refused write stays pending for the next flush, and all drafts
together keep to a budget so the outbox can still save a send. Closing a tab
drops its panes' drafts.

* test(native-chat): returned text is stored before its images arrive

* fix(native-chat): keep returned text and drop launch-seed copies across reloads

A message given back from the outbox could be larger than one draft was
allowed to store, so the saved draft was deleted and the returned text was
gone after a reload. One draft may now use the whole draft budget, which
holds the largest message a send accepts.

An adopted launch link is also parked in the agent's input line. Restoring
it after a reload, with no seed left to replace that copy, typed it twice,
so the untouched copy is shown but not saved.

A composer reused for another pane now shows that pane's images, so adding
one never saves the previous pane's over them.

* test(native-chat): a plain 260k given-back message stays saved

* fix(native-chat): saved drafts drop pasted images and die with their worktree

A pasted image lives in a temp file Orca wrote, and the permission to show
it is held in memory only, so after a restart a restored pasted chip showed
a generic icon, and once the temp file was cleaned it looked fine but the
send failed after the composer cleared. Pasted images now stay in the
draft for the current run only; images the user attached from their own
files are still saved.

Removing a worktree now deletes its terminal tabs' drafts, as closing the
tab does.

A paste that finished late in a composer that had already been replaced
wrote that composer's old images back over a newer send. Dropping a
placeholder chip no longer writes the saved draft.

The pane-change reload in the images hook is removed: the composer is
keyed by pane, so a pane change always mounts a new one.

* fix(native-chat): the composer renders the draft store instead of keeping a copy

A composer that had been replaced, for example by a question prompt, could
still finish an attach or a send later and write its own old copy of the
draft over the store, so a sent message or image came back. The composer
now reads text and images from the draft store and changes the draft as it
is now: an added image or file reference joins what is there, and a send
that settles after a round trip, or after its composer was replaced, clears
the draft only if it is still what was sent. Only chips still being written
and an IME composition in progress stay local to the composer.

Closing a tab now deletes drafts by the exact tab id: a second chat for one
session has an id that extends the first one's, so a prefix match deleted
its drafts too.

* fix(native-chat): keep images pasted during a command and on shown composers

An image pasted while /compact or /clear was still on its way to the host
vanished when the command was accepted: the clear that followed also wiped
the chip still being saved. A command that settles now clears only the
draft it sent, and an image pasted meanwhile stays.

A composer on screen could lose its pasted images, and its unsaved launch
text, once 128 other drafts pushed its record out of memory, because those
are never stored. A draft that a composer is showing is no longer evicted.

* fix(native-chat): a draft being written is never evicted from memory

A write to a pane while every other cached draft was on screen could evict
the record it had just written, losing it. Also adds the test that a send
settling after its composer unmounted clears the saved draft, through the
real send hook.

* fix(native-chat): restored images Orca couldn't keep come back to attach again

A pasted screenshot in an unsent draft vanished after a restart with no
notice, and a restored image whose file had been deleted looked fine until
the send failed. Both now come back as a placeholder chip that names the
image and says it wasn't saved with the draft. Send stays disabled, and
the send button says to attach the image again or remove it. Attaching a
file with the same name takes the placeholder's place.

Two browser tabs of the web client on one chat now follow each other: when
one sends or edits the draft, the other drops its copy and shows the new
one, unless it has an edit of its own not yet saved.

* test(native-chat): no send while an image waits to be attached again

* fix(native-chat): pasted images in a draft come back after a restart

A pasted screenshot lived in the OS temp folder with a read permission held
in memory, so a draft could only bring it back as a placeholder. Local
pastes are now written to Orca's own native-chat-pastes folder under the
app's user data, and a draft keeps them as real images. On restore the
composer asks the main process to re-grant each one; main grants a read
only when the file's real path, symlinks and junctions resolved, is inside
that folder, and reports anything else as not kept, which becomes the
placeholder. Pastes older than 30 days are deleted at startup, far past
the host's 24 hour window for a resent message; the sweep never follows a
link and never blocks startup. SSH pastes and older temp-folder pastes
still come back as placeholders.

* test(native-chat): the paste sweep test ages the link itself

* fix(native-chat): type the paste-folder path helper with PlatformPath

* fix(native-chat): type the paste-folder path helper from path.posix

* fix(native-chat): placeholder chips say what happened and only a user attach replaces one

A placeholder now shows the image's name and a short hint on the chip
itself, "Attach again" for a file and "Not kept" for a pasted image, with
the longer explanation in its tooltip. The copy no longer says the image
wasn't saved: it says it couldn't be brought back, and for a pasted image
it asks only for removal, since a new paste can't match it. The send
button now says to remove the image to send.

Only an image the user attaches (picking, dropping or pasting) takes the
place of a placeholder with its file name. An image Stop gives back is
added beside it, so a different file with the same name no longer hides
the reminder.

* fix(native-chat): only composer pastes use the paste folder, and restore grants only real paths

Every local clipboard image save had moved into the paste folder, whose
macOS path has a space, so a screenshot pasted into a terminal or sent to
a terminal-backed agent no longer attached. Only a native-chat composer
paste goes there now, through an optional field on the existing save
call; terminal, editor and phone pastes stay in the system temp folder as
before. An image path sent to an agent's input is escaped the way a
dropped image is.

The restore re-grant now grants only the file's real path, and only when
both the real path and the path as written are inside the paste folder,
which must not itself be a link. The sweep deletes only Orca's paste
files and skips a linked folder.

* fix(native-chat): a restored paste is readable by the path its draft stored

Restore granted only a paste's real path, but the composer reads the
preview by the path its draft stored, so with user data reached through a
link the restored chip showed a generic icon. The stored spelling is
granted too; it is already proven to sit inside the paste folder. A test
also pins that a composer paste asks for the paste folder.

* fix(native-chat): grant a paste's stored spelling only when it names the same file

A grant also covers its path's own real path, so a stored spelling that is
itself a link inside the paste folder could have granted an outside file
while the path as restored reached a real paste. The stored spelling is
now granted only when it resolves to the same file; otherwise only the real
path is, and the preview falls back to the generic icon.

* test(native-chat): check the outside file by its real path too

* fix(native-chat): a structured chat's draft belongs to its conversation

Drafts of a structured chat were saved under the pane (tab id plus a hash
of the session), so the follow-up that keys them by conversation would
have left every draft saved by this build invisible after an upgrade,
never shown and never deleted. They are now keyed by the conversation
(`agent-session:<sessionId>`) from the start: every composer showing the
conversation shares one draft, Stop and a queued card's Edit give text
back to it, closing the tab keeps it, and removing the worktree deletes
it. Terminal-backed chats keep the pane key and lose their draft when the
tab is closed.

A send now leaves whatever was added since it was sent, text typed or
composed and images attached meanwhile, and clears only what was sent;
a draft replaced in the meantime is left alone.

Lifted from #25207 (62ef8e7): the conversation key, the composer's
draftScopeKey, keep-on-close and delete-on-worktree-removal, and the
leave-what-was-added send settle.

* fix(native-chat): every path that drops a worktree's tabs deletes their drafts

A worktree's chat drafts were deleted only by the removal's own renderer
teardown, which looked them up from the worktree's tab lists after the
removal round trip. The host announces the change before it replies, and
the listing refresh that starts can purge those tab lists first, so the
teardown found no tabs and the drafts stayed. The bulk purge now deletes
the drafts of the tabs it drops, and the teardown deletes them before it
closes any tab.

* fix(native-chat): only a user's delete removes a workspace's chat drafts

The listing purge is reconciliation, not a delete: re-pairing a server under
the same id purges its worktrees' tabs, and the host then restores the same
chats under the same draft keys, so deleting drafts there erased them.

Removing a worktree, a project or a folder workspace now reads the draft
keys of the workspace's open chats before the host round trip, and deletes
them only once the host confirms. A listing refresh that drops the tabs
during the round trip no longer hides them, and a refused delete keeps them.

* feat(native-chat): drafts live in IndexedDB with no budget, and a refused save shows

Unsent drafts shared localStorage with the send outbox, so they kept to a
128-draft, 1M-character budget and the oldest quietly did not come back.
They now live in their own IndexedDB store, all loaded once at startup,
with no budget and no eviction.

- Startup waits up to 1.5 s for the load alongside the session read; a
  slower load still fills in every draft not edited meanwhile.
- Each write commits explicitly; on pagehide, beforeunload and hiding,
  anything storage has not confirmed is also journaled synchronously to
  localStorage and replayed by the next run.
- A BroadcastChannel replaces the storage event: another window's save is
  taken unless this window has a change of its own not yet saved.
- A refused save is retried on the next flush and shown: a warning icon
  by Send and on the draft's image chips, only after a refusal.
- Each draft records the workspace and host it was written in, so removing
  a worktree, project or folder workspace also deletes the drafts of chats
  whose tab was closed earlier.

* fix(native-chat): drafts set from the store are never saved back, and early appends merge

- The editor no longer saves a value the field sets from the store (a
  late-loaded or adopted draft): that echo made another window's draft a
  local change here, so a send in one web tab came back from the other.
  A whole draft set on the editor uses its saved document, keeping its
  skill chips. Equal documents no longer count as a change.
- Text or images given back, or attached, before the startup load lands
  are added to the loaded draft instead of replacing it; only typing wins.
- The unload journal leaves out drafts already refused, keeps to 256,000
  characters, and drops a draft's entry once any window confirms a change
  to it, so a replay never brings back a draft sent since.
- A failed load is retried three times with backoff, warned about once,
  then left.
- Adopting another window's write keeps images already checked, so their
  chips don't flash.
- A refused draft shows one icon, by Send, with plainer copy; the chip
  badge and its string are gone.
- A comment that said drafts' bounds retire orphans is corrected.

* test(native-chat): an early append the load already read is not added twice

* test(native-chat): the store-value editor test waits for the editor and checks no document is saved

* fix(native-chat): early appends are read-modify-writes, and the load can't be skipped

- Before the load lands, an append (text or images given back, a paste)
  reads the stored draft and writes it back with the append in one storage
  change, so a load that failed and was retried can never lose the saved
  draft. On landing, only appends that load could not have read (made after
  it began reading, or never written) are applied again, by in-run order
  rather than by comparing clocks.
- The editor saves a value set from the store when this window has a change
  of its own behind it, so a mention accepted next to a skill chip keeps the
  chip; another window's draft or a late load still isn't saved back.
- A refused draft is journaled again; the 256,000-character cap bounds it.
- The draft load starts before the startup chain, and the first write
  starts it in a window whose startup never did; startup still waits for it
  before the session's tabs mount.

* fix(native-chat): text given back is durable before its source is deleted

Draft writes reach IndexedDB asynchronously, but Stop's restore and a queued
card's Edit delete their source (the outbox entry, the card) right after
adding the text to the draft. A crash before the write committed lost it.

An addition (text or images given back or attached) is now journaled to
localStorage synchronously, as data, the moment it is made, and dropped
once storage confirms a change at least as new. The next run replays it onto
the stored draft only when storage doesn't already hold it, and only where
the draft doesn't already end with that text or hold that image, so a crash
either side of the commit gives the text back exactly once and resends
nothing. Only earlier runs' entries are replayed.

The append reports whether the addition is durable. Edit, when it isn't
(the journal full or refused), deletes the card only once the draft is
saved, and keeps it if storage refuses. Additions may grow the journal to
800,000 characters; whole drafts still stop at 256,000.

* test(native-chat): a kept paste is readable by the composer preview through chat-image access, with no grant

* fix(native-chat): a per-scope write confirm, and the paste rule as C2 has it

- nativeChatComposerDraftWriteSettled(scopeKey) settles once every write of
  that scope issued before the call has: true only after IndexedDB completed
  each transaction (an append made before the load landed included), false
  when one was refused or failed, and never on a timeout. A change still
  waiting for its deferred write is issued first.
- A queued card's Edit deletes the card only on true, and keeps it on false.
- A restored paste is kept when its real path is a file inside Orca's paste
  folder, as C2 resolved it. The tests keep every containment case and now
  check that neither an outside file nor the file a stored spelling names
  becomes readable through the preview's chat-image access, which reads only
  image files and needs no grant.

* test(native-chat): the per-scope confirm settles at the IndexedDB transaction's complete, and false on abort, for an append before the load

* fix(native-chat): a journaled removal always fits, and a load replays only what preceded it

- A removal is never capped in the journal (it is tiny, and it is what
  keeps a sent draft from coming back); only whole drafts count against
  their 256,000 characters, and additions keep their own 800,000.
- A load replays the journal as it stood when the load began reading, so an
  addition a live window journals later, whose own write is ordered after
  that read, isn't replayed over its newer typing.
- A lifecycle test no longer says the paste is re-granted.

* fix(native-chat): a chat with a draft is never reused or taken over

#24917's empty-chat check read the composer draft under the pane key, while
this branch keeps a structured chat's draft under its conversation key, so a
chat holding typed or given-back text, or only an image, read as empty: a new
chat could reuse it and another request's text could go into it. It reads
the conversation's draft now, as C2 adapted it (d4e469b), with the tests
moved to that key.

While the startup load of saved drafts is still running, no chat reads as
empty: one may hold a saved draft memory doesn't have yet. The cost is only
that a new chat opens instead of reusing one during that moment.

* fix(native-chat): a returned text counts as already there only as its own paragraph

Replaying an addition after a crash skipped it whenever the draft merely ended with its text, so
"go" given back onto "please go" was lost. It now counts only when the draft equals the text or
ends with it after a blank line. A whitespace-only draft counts as empty when text is added, and
an addition made before the startup load lands is made again on the loaded draft only where that
draft doesn't already hold it.

* fix(native-chat): a hand-back the draft already holds stays safe until that draft is saved

After merging main's shared returned-text rule, a hand-back the draft
already ends with changes nothing, so it wrote no backup entry yet
reported durable. If the draft holding that text was not saved yet, a
crash after the source copy was deleted lost it. Journal the addition
whenever the draft is not confirmed saved.

* fix(lint): import worktree removal names once in the delete failure toast

Main's #25668 imported from shared/worktree/removal twice in this file,
which fails the type-aware lint gate (no-duplicate-imports) for every
branch that merges current main.
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