Skip to content

fix(native-chat): each chat failure says why in plain words, on the thing that failed - #23608

Merged
brennanb2025 merged 26 commits into
mainfrom
brennanb2025/chat-refusal-copy-desktop
Sep 28, 2026
Merged

brennanb2025 merged 26 commits into
mainfrom
brennanb2025/chat-refusal-copy-desktop

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 23 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1925 $\color{#cf222e}{\Huge{\mathbf{−}}}$​94 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1831
Prod 46 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​865 $\color{#cf222e}{\Huge{\mathbf{−}}}$​235 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​630

Follows #23116 (merged). This is the desktop half of the chat failure-copy work; the phone's half is a later PR.

Scope: which chat this affects

  • Structured chat only. This is the experimental structured native chat (experimentalStructuredNativeChat, off by default). The terminal-backed chat's "Not delivered — check the terminal" marker now renders through the same per-row notice, with the same words and no Retry.
  • Desktop app, plus one host change. The host now classifies a chat history that cannot be opened when the chat is read, not only when a message is sent.

ELI5

Some chat errors still showed Orca's internal text, and one error line stood in for several messages. Now a chat whose history can't be opened says so in plain words and stops retrying when it never can load. Each message that didn't go through says why on its own row, and its Retry sends only that message. Every refusal is worded from the typed reason the host sent, never from Orca's log text.

What Changed

Before

  • Opening a chat whose saved history could not be read showed raw storage text in red (for example "file is not a database" or "unable to open database file") and "Orca keeps trying to load it." The pane retried every 750 ms forever, even when the history was damaged and could never load.
  • One "Message was not sent" strip above the composer picked a single message. Other failed messages showed nothing. Pressing its Retry sent only that one message, and the strip stayed on screen for the rest.
  • A refusal the host threw (for Stop, an answer, an option change, a goal, or a refused chat create) was worded from its error code alone ("Orca couldn't confirm what happened."), or showed the host's log text.
  • After an app restart, pressing Retry on an older rejected message could do nothing the first time: it went out under the old id and the host answered with the old rejection.

After

  • History that can't be opened. The host classifies the open failure at the one place every read goes through. Proven damage reads "Unable to load this chat." as the pane's one line, and the pane stops retrying. A failure that can clear (a lock, permissions, too many open files) reads "Orca couldn't open this chat's history right now." and keeps retrying, backing off from 750 ms to at most every 30 s and dropping back to 750 ms once a read succeeds. The generic "Could not load conversation" title shows only when the host gave no reason. Raw storage text goes only to the log, once per chat and failure rather than on every retry.
  • One notice per message. Every message that did not go through shows its own reason under its own row. The single strip is gone. When a start-failure row above already states the cause, each message that start rejected reads just "Your message was not sent." with its Retry. This is decided by matching the message's typed rejection with the start-failure row's typed fact, never by comparing sentences. While the queue is stopped on one message, only that message's row offers Retry. A Retry sends only its own message and never releases a different message the queue is holding.
  • Words beside a Retry leave out the retry step. A notice sitting beside its own Retry says the cause without "Send your message to try again." (the button is that step). Steps you must take first stay, for example "Sign in first." The same rule covers message rows, the chat-create strip and the history pane, which retries by itself.
  • Worded from typed facts, never from strings. A message the host recorded and rejected is worded from the typed fact on its journal row, in the chat's own agent name. Refusals the host threw are worded from the typed reason in the error's data. Whether a write went through is decided exactly as before; only the words change.
  • A rejected message retried after a restart goes out the first time. The retry uses a fresh message id whenever the message's saved state says the host settled it as rejected.
  • A lost connection while the chat is on screen reads "Reconnecting to this chat…" in muted text instead of an error.
  • The idle sweep no longer builds an unused stop sentence in memory; the chat's error row already comes from the typed failure.

Why

  • The mechanism. Surfaces read strings: the read error's message went onto the status line, the create failure's message sat beside Retry, thrown errors reached toasts as error.message, and a message's rejection was shown from the host's sentence. Under fix(native-chat): the host writes chat failures for a person, with a typed fact beside them #23116 the host's message is log text, so any surface reading it can show internal text. This PR removes that at the source. The host classifies at the read door, the client carries the typed refusal or fact to each surface, and every surface words it through the one copy table (agent-session-refusal-notice.ts with agent-session-failure-words.ts).
  • Finality comes from the reason, never the message. Only details.reason === 'journalCorrupt' stops reconnecting. A temporary failure, an older host that sends no reason, or any other refusal keeps the current retry.
  • Per-message notices are derived, not stored. They are built each render from what the desktop already keeps: the outbox's typed last failure for a message the host never recorded, and the journal row's full rejection fact for one it did. They are built in the renderer rather than the shared projection. Nothing new is persisted, and the outbox's saved shape is unchanged.
  • Retry ownership. A Retry used to clear the queue's hold no matter which message it retried, so the rows had to hide Retry to avoid releasing a held message. Now a Retry releases only its own hold, and the rows decide only which Retries are offered, so the other message is never stranded or sent by accident.
  • Alternatives rejected.
    • Storing a fuller copy of each rejection in the desktop's saved outbox: that is a second persisted copy of a fact the journal already holds.
    • Rewording a rejection only when its rebuilt sentence matched the host's sentence exactly: that compares strings and fails silently when names or wording drift.
    • Driving write outcomes (sent, unconfirmed, requeued) from the typed refusal as well as the words: every thrower would first need proving free of side effects, so outcomes stay as they are.
    • A single "keeps trying" pane for every read failure: it retried a damaged history forever.

Differences from the common pattern

  • Damaged versus temporarily unavailable history. The common pattern doesn't tell these apart, because it has no per-chat store that can be damaged. Orca keeps a history database per chat, so damage is final and stops retrying, while anything that can clear keeps retrying.
  • Where a message's failure shows. The common pattern puts a queued message's failure on its queue card, or uses a toast and puts the text back in the draft. Here it shows on the message's own transcript row, and the typed fact is worded at render time rather than stored as a sentence.
  • Codes the client doesn't know. The common pattern falls back to the server's message for an unknown code. Here an unknown code reads generic "not done" copy and the host's message is never shown, because the host's message is log text.
  • A rejection older than the loaded history. When the journal row isn't loaded (older than the loaded page, or past the 256-row cap), the row falls back to the outbox's smaller copy of the fact. For the three kinds whose words need more than that copy keeps (provider rejected, couldn't start, couldn't restart), the row shows the host's person-written sentence. Only in that fallback can a resumable start failure still say "Send your message to try again." beside its Retry.

Linked Issue

No separate issue. Follow-up to #23116; reported from live QA of #22918.

Visual Proof

Live Electron QA ran on a separate Mac against dev builds with isolated profiles. BEFORE is this PR's merge base on main (2f8f4f576dd); AFTER is this PR, at 5e41e506a98 for the first four pairs and the restart shot at 4fd67d7cd59; later commits change only tests. A shim made Claude fail to start on demand; history damage was made by overwriting a chat's journal.db (not a database) or removing its permissions (chmod 000), restored afterwards.

Two messages queued behind a start that fails. Before: one strip for both, neither message marked. After: the start-failure row states the cause once, and each message it rejected reads "Your message was not sent." with its own Retry.

Before After
main-queue608-01-refused.jpg pr-queuer3-01-refused.jpg

Pressing one Retry. Before: the strip's Retry sent the first message and the second stayed stuck under the same strip. After: pressing the second message's Retry sent only the second; the first kept its own notice and Retry.

Before After
main-queue608-after-retry.jpg pr-queuer3-after-retry.jpg

Damaged history. Before: red "file is not a database", "Orca keeps trying to load it.", 40 reconnects in 30 s. After: one line, "Unable to load this chat.", no raw text, 0 reconnects in 30 s.

Before After
main-journal608b-01-damaged.jpg pr-journalr3-01-damaged.jpg

History that can't be opened right now (chmod 000). Before: red "unable to open database file". After: "Orca couldn't open this chat's history right now." as the one title, and it keeps retrying with backoff (two retries 24 s apart in a 60 s window, against 40 per 30 s on main), with the host logging it once instead of about 80 times a minute.

Before After
main-unavail608-00-unreadable.jpg pr-unavailr3-00-unreadable.jpg

A rejected message retried after an app restart. After quitting and relaunching with a rejected message on screen, one press of its Retry sent it (one agent start, reply received).

pr-restartr2-after-retry.jpg

The chat-create strip's wording could not be reproduced live on either build (a refused create needs a host race); it is covered by unit and pane tests.

Testing

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

Platforms: macOS, live (the QA above) and unit. The host change runs wherever the chat's history lives, so it applies unchanged to SSH and other remote hosts; SSH was not exercised live. Windows and Linux were not run; nothing here is platform-dependent.

Each new test was checked against a copy with the change reverted, and each failed for the expected reason:

  • journal-open-failure.test.ts, structured-agent-session-at-rest.test.ts: a read, subscribe or options call on a history that won't open is refused with agent_session_journal_unreadable and journalCorrupt or journalUnavailable, the storage text and path never reach the reply, and a reconnecting reader logs once per failure.
  • structured-agent-session-read-transport.test.ts, NativeChatStructuredSession.read-error.test.tsx, NativeChatEmptyState.test.tsx: damage stops reconnecting and reads "Unable to load this chat." as the one line, with no generic title; a temporary failure keeps going with retries backing off from 750 ms to 30 s (fake timers) and resetting after a successful read; a failure with no reason while messages are shown reads "Reconnecting to this chat…".
  • NativeChatStructuredSession.start-failure-notice.test.tsx, use-structured-agent-session-start-failure-facts.test.ts: messages a failed start rejected read "Your message was not sent." under the start-failure row that states the cause; a different rejection keeps its full words; with no start row loaded the full words stay; the start rows' facts keep one identity while unchanged, so streaming doesn't re-render every row.
  • structured-agent-session-delivery-notices.test.ts, NativeChatStructuredSessionDelivery.test.tsx, NativeChatMessageRow.test.tsx: each failed message gets its own notice; only the held message offers Retry while the queue is stopped; clicking one Retry sends only that message; a recorded rejection is worded from its journal row, whatever sentence the host wrote; the not-loaded fallback keeps the provider's words.
  • use-structured-agent-session-outbox-admission.test.tsx, use-structured-agent-session-outbox-rejection-cause.test.tsx: a Retry never releases another held message, including behind a queued or unconfirmed message; a message rejected before a restart retries under a new id the first time.
  • agent-session-refusal-notice.test.ts: across every code, reason and write, words beside a Retry equal the plain words minus only a retry step; "Sign in first." stays.
  • use-structured-agent-session-write-refusal.test.tsx, NativeChatSessionOptionPickers.test.tsx, structured-agent-session-launch-options.test.ts, NativeChatStructuredSession.launch-lifecycle.test.tsx: Stop, answers, options, goal, the option-picker toast and the chat-create strip are worded from the error's typed refusal, and outcomes are unchanged.
  • structured-agent-session-send-disposition.test.ts, agent-session-write-failure.test.ts, structured-agent-session-reducer.test.ts: the rejection words, the refusal read from both error shapes, and the reducer's typed read failure.
  • The phone's shared calls are unchanged; mobile-structured-send-delivery.test.ts and four related mobile suites pass.
  • Typecheck (node, web, mobile), full oxlint on every changed file, the changed-code quality gate, React Doctor and the localization checks are clean. CI was green at earlier heads of this branch.

Known weaknesses

  • Untranslated failure sentences. Sentences the host writes, and the shared failure sentences used in notices, are English on translated desktop builds. Owner: desktop follow-up.
  • Older apps reading a history that won't open now see the bare code agent_session_journal_unreadable instead of raw storage text (which could include a file path). Other read refusals already reach older apps as bare codes.
  • A damaged chat has no in-pane next step. It reads "Unable to load this chat."; the way forward is a new chat. Owner: desktop follow-up.
  • A message rejected by a start that wrote no failure row can read just "Your message was not sent." when an earlier start in the loaded history failed with the same cause; the reason is then stated further up. Follow-up: the host links each rejection to the start that rejected it. Owner: the refusal-copy follow-up.
  • "Session restore failed" toast. Launching with a chat whose history is damaged or unreadable shows "Session restore failed / Changes won't be saved until restart". This happens on main too and is not changed here. Owner: feat(native-chat): one structured-chat journal database per host, owned by one process #23613.
  • Phone. Per-message notices, the read-failure wording and refusal details on the phone are the mobile follow-up PR.

New strings for native review

  • components.native-chat.state.reconnecting ("Reconnecting to this chat…"): es, fr, ja, ko, zh.

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

Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance

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)

…its reason

History, subscribe, snapshot and options reads reach a chat through one accessor, whose open had no
catch: a journal that would not open reached every client as a runtime error carrying the storage's
own text (a path, "file is not a database"). The accessor, and the options read's own open, now throw
the classified journal refusal: journalCorrupt when SQLite reports damage, journalUnavailable
otherwise. The storage text goes to the log only.

The wire code stays runtime_error and the message becomes the bare code, as for every thrown
refusal; the reason rides in the error's data.
…no hand-written reason

The sweep passed an English sentence as the stop's reason. It lived only in memory and nothing read
it: the delivery loop words the error row and the rejection from the hostStopped fact. Dropped, with
the display-name lookup that built it.
…never its message

A thrown agent-session refusal reaches the client as runtime_error with the bare code as its message
and the typed refusal in error.data. Stop, answers, options and goals, a launch's held option pick,
the option picker's failure toast, and the Retry line of a chat that could not start now word it
from that refusal through the shared notice table. What each caller decides about the outcome is
unchanged: only the words move.

The Retry line of a failed start no longer prints the host's message or a thrown error's text; it
keeps the refusal as a fact and says the cause and step its reason names, or only that the chat
could not be started. The option toast keeps a local option surface's own sentence.
…nd damage stops the retry

The structured chat's read failure showed the host's text on the status line, and the pane always
said Orca keeps trying. The read transport now takes the refusal from the error's data (a stream
payload or a thrown RPC error), the reducer keeps it beside the failure text, and the pane and the
status line word it through the notice table, once: on the pane when the failure took it, else
beside the transcript that stays.

A damaged journal says "Unable to load this chat." and the read stops reconnecting for that run;
reopening the chat reads again. An open that can clear names its cause without "Try again", since
the pane retries on its own. A failure that names no reason keeps today's generic line. Finality
comes from the refusal's reason, never its message, which is the bare code for both.
A message the host recorded and then rejected keeps the host's typed fact beside its reason, but
the Retry words re-read the reason alone. Now the fact decides: a hand-over failure says Orca
couldn't reach the agent, a kind whose reason may be a legacy marker gets its fact's own sentence
(a full queue now says so instead of only "not sent"), and any other kind shows the sentence the
host wrote for it, which carries the agent's name and any words the provider wrote for a person. A
row with no fact reads as before.
…s own row

The structured chat showed one Retry strip under the transcript for whichever single entry it
picked, so a second failed message had no reason and no Retry of its own. The terminal-backed
chat's existing per-row delivery marker now carries a notice and an optional Retry, and the
structured pane derives one per message from the outbox on each render: every rejected message,
and the one the queue stopped on (read through the drain's own rule, so a Retry never names a
message waiting behind it). Each is worded from that message's stored failure. The single strip is
deleted. Nothing new is stored, and the shared message projection is untouched.
… already acts

Three wording rules for the desktop chat:

- A chat that could not start shows Retry beside its reason, so the reason stops at its cause
  where the Retry is the step: a reason whose action is to retry, and a start failure's "send
  your message again". Any other step stays (quit the terminal agent, start a new chat). The
  start-failure sentences take a retryControl context for this; what the host writes is unchanged.
- A history that couldn't open right now still reconnects, so the pane keeps "Orca keeps trying
  to load it" under its cause. Only a damaged history, which no retry reads past, drops it.
- A read failure that names no reason while the transcript is shown is only the pane
  reconnecting: the status line says "Reconnecting to this chat…" in muted text, not an error.
  New key components.native-chat.state.reconnecting, hand-translated for es/fr/ja/ko/zh.
… is moving

Each rejected message's row offered its own Retry even while the queue was stopped on another
message. Any Retry clears the stopped queue, so pressing a rejected message's Retry also sent the
message the queue was holding, which the user had not retried; behind a message whose delivery is
unconfirmed, the retried one instead went back into the queue with no notice and waited there.

While the queue is stopped, only the message it stopped on offers Retry, as the single Retry strip
this replaced did. A rejected message keeps its words on its row and gets its Retry back once the
queue moves.
…n every reconnect

A reader reconnects every 750 ms while a journal open can clear, and each attempt logged the
failure with its full stack. The read door now logs a session's failure once until that session
opens, closes, or fails differently; every attempt is still refused with its reason.
…tice stops at the cause

A rejected message offering Retry read "Claude stopped before it finished starting. Send your
message to try again." beside that button. Its row now takes the rule the launch strip already
follows: beside its own Retry the words leave out sending or trying again, worded from the stored
fact with the chat's agent name. The stored fact keeps less than the host wrote from (a refusal,
the provider's words), so a reason it cannot rebuild exactly is kept as written. A rejected
message without a Retry, while the queue is held, keeps the step. What the host writes and the
phone's notice are unchanged.
…e one the queue is held on

Retry released the queue's refusal hold whichever message it was pressed on. While a queued message waited ahead of a held one, every rejected message offered Retry, and pressing it also sent the held message the person had not retried. Retrying an unconfirmed message ahead of a held one did the same. Retry now releases the hold only for its own message.
…sign in first

Beside a Retry the notice dropped the whole next step, so a chat that could not start because the agent was not signed in read only the cause. Pressing Retry without signing in fails the same way again. The words now keep the sign-in step and leave out only the resend, which the Retry button is.
…ep the same way

A message the queue stopped on worded its refusal with no agent name and with its retry step, beside its own Retry, while a rejected message next to it named the agent and left the step to the button. The launch strip and the history pane each had their own copy of the same rule. One wording context now goes through the one notice table for every surface: a Retry beside the words, or a pane that reconnects on its own, is the step for a reason whose action is to retry, and every other step stays. What the phone and the host write is unchanged.
… fact, never by comparing sentences

A message the host recorded and then rejected kept only the rejection's kind on the message, so its notice was reworded from that smaller copy only when it rebuilt the host's sentence word for word. A different agent name, an older host's wording, or anything the copy dropped (why a start failed, the provider's own words) left the host's sentence in place, beside a Retry that repeated its resend step. The notice now reads the journal's own rejection for that message, found by id, with the pane's agent name and Retry, and shows the provider's words only when they were written for a person. The message's smaller copy words it only when that journal row is not loaded. Nothing new is stored.
… new id the first time

Whether a Retry needed a new message id was remembered in memory for one message, or read from the journal row when it was loaded. After a restart, or for an older message whose row was not loaded, the first Retry resent under the old id, the host answered with the same settled rejection, and nothing visibly happened. The message now says so itself: one the host recorded and rejected always retries under a new id, including after a restart. A refusal that already gave the message a fresh id, and a message whose delivery is unconfirmed or in flight, keep their id as before.
…eps the provider's words

When the journal row that rejected a message is not loaded, the message's own copy of the
fact has no provider detail or start refusal. For the kinds worded from those, the row now
shows the sentence the host wrote for the person instead of a thinner rebuilt one.
…aded journal row

Nothing covered the pane handing the journal's rows to the per-message notices, so a pane that stopped passing them would quietly fall back to the message's smaller copy of the rejection and show the host's sentence, resend step and all. The new case renders the pane with a rejected message whose journal row is loaded and checks that it reads that row's refusal in the chat's own agent name.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review status: ready for maintainer review (still a draft). Head 5c1647d3445, all CI checks green.

Review loops. Six review passes and one separate architecture review ran. The last pass came back clean. What they fixed:

  • A message's Retry could release, and send, a different message the queue was holding. Now a Retry releases only its own message (26f0c993858, 5298d8dc2f9).
  • The host logged an unreadable history with a full stack on every 750 ms retry, about 80 lines a minute. It now logs once per chat and failure (d29a373af3d).
  • A notice sitting beside its own Retry also said "Send your message to try again." One rule now drops the retry step wherever a Retry sits beside the words: message rows, the chat-create strip and the history pane (8ac3d2d6ddf). "Sign in first." stays (9e131b25de0).
  • A rejected message was worded by comparing sentences. It is now worded from the typed fact on its journal row, and the desktop stores nothing new (bac487a63c3). When that row isn't loaded, the provider's own words are kept (4fd67d7cd59).
  • After an app restart, the first Retry on an older rejected message did nothing. It now goes out on the first press (8ff73871960).
  • A missing pane-level test was added (5c1647d3445).

Live QA ran on a separate Mac against dev builds, before (main) and after (this PR). Screenshots are in the description.

  • Two messages failing behind a start: each shows its own notice and Retry, and one Retry sends only that message.
  • Damaged history: "Unable to load this chat." with no retrying (40 retries in 30 s on main, 0 here).
  • History that can't be opened right now: plain words, and it keeps retrying. The host log stays flat.
  • Retry after a restart works on the first press.
  • The chat-create strip could not be reproduced live on either build; tests cover it.

Readiness and compatibility. Readiness reviews at the start and end both passed, with no release blockers. The phone is unaffected: its shared calls produce the same words, and nothing on the wire changed except the one disclosed case below. Typecheck (node, web, mobile), lint and the tests pass.

Known and disclosed (see Known weaknesses):

  • Older apps show the bare code agent_session_journal_unreadable for a history that won't open, where they used to show raw storage text.
  • A damaged chat has no in-pane next step, and its title and message say the same thing.
  • A failed start repeats its sentence in the start-failure rows.
  • The "Session restore failed" toast on a damaged history also happens on main.
  • A temporary read failure still retries every 750 ms with no backoff.
  • Failure sentences are English on translated builds.

Overlaps. #23026 touches the same outbox hook (Retry and the hold). Whichever lands second merges the other.

A read the host refused for a named reason put its sentence under the generic
"Could not load conversation" title, so a damaged history read as two lines
saying the same thing. The pane's own sentence now takes the title's place; a
history that can come back keeps its line saying Orca keeps trying. A failure
that names nothing keeps the generic title.
… was not sent

When an agent stopped before it finished starting, the chat showed the start's
red row ("Claude stopped before it finished starting. Send your message to try
again.") and then repeated that cause under every message the start rejected.
Each of those messages now reads "Your message was not sent." beside its Retry.

The match is made on typed facts, not on the words: the host writes the start's
row and the rejection of its queued messages from the same failure fact, and the
row is keyed by the start. The pane finds the loaded start-failure rows by that
key and shortens a message's notice only when its loaded journal submission was
rejected with the same fact. Any other rejection, or one whose submission or row
is not loaded, keeps its full notice. The row key moves to a shared module so the
host that writes it and the pane that reads it use one definition.
…less often

A read the host kept refusing (its history store could not be opened right now)
reopened every 750 ms for as long as the chat stayed open, about 40 opens every
30 seconds. Each reconnect now waits twice as long as the last, from 750 ms up
to 30 seconds, and never gives up; the first read that delivers anything starts
the wait over at 750 ms. A damaged history still stops reconnecting at once.

Reset happens on a delivered read, not on connect: a local subscribe resolves
before the host's open refuses, so resetting there would keep the 750 ms loop.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Status update: ready for maintainer review (still a draft). Head f7892dc8211. This comment replaces the earlier status for the items below.

Follow-up changes from the owner's review of the live QA. Each change has a test that fails without it, and a focused review of these commits found no issues.

  • A damaged chat reads as one line (0b9a7b06c10). When the host gives a reason, the pane shows that sentence once as its title: "Unable to load this chat." It no longer stacks "Could not load conversation" above it. The generic title stays only when no reason was given.
  • The cause is stated once (e3f23a8a171). When a start-failure row already states why the start failed, each message that start rejected reads "Your message was not sent." with its own Retry. The match compares the message's typed rejection with the row's typed fact, never the sentences. Other rejections keep their full words. The sentence is the existing translated one, so there are no new strings.
  • History that can't be opened right now retries with backoff (c2eb6898ba7). Retries start at 750 ms and double up to a 30 s cap. They reset to 750 ms after a successful read, and they never stop; only damaged history stops retrying. This is contained in the read transport and covered by a fake-timer test.
  • Two test-only fixes: a missed test call site (5e41e506a98), and a duplicate import that failed CI's lint (f7892dc8211).

Live QA round 3, on a separate Mac with the chat tab active in every shot; screenshots are in the description:

  • Two messages rejected by a failed start: each reads "Your message was not sent." with its Retry, and one Retry sends only that message.
  • Damaged history: only "Unable to load this chat.", with 0 retries.
  • History that can't be opened right now: one title plus the keeps-trying line. Retries back off (24 s apart late in the window, against 40 per 30 s before), and the host log stays flat.

Known weaknesses, updated in the description:

  • Removed: the doubled title on a damaged chat, the repeated cause, and the 750 ms retry.
  • The restore-failed toast is now tracked in feat(native-chat): one structured-chat journal database per host, owned by one process #23613.
  • Added: a message rejected by a start that wrote no failure row can read just "Your message was not sent." when an earlier start in the loaded history failed with the same cause. The fix is for the host to link each rejection to the start that rejected it, which is left to a follow-up.

Typecheck (node, web) passed on these commits, and CI is green at f7892dc8211.

@brennanb2025
brennanb2025 marked this pull request as ready for review September 28, 2026 18:41
@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 5ac6340b-b2ea-4584-b820-cb37c555ac12

📥 Commits

Reviewing files that changed from the base of the PR and between f7892dc and 4c03975.

📒 Files selected for processing (4)
  • src/renderer/src/components/native-chat/structured-agent-session-read-transport.test.ts
  • src/renderer/src/components/native-chat/structured-agent-session-read-transport.ts
  • src/renderer/src/runtime/runtime-host-contact-regained.test.ts
  • src/renderer/src/runtime/runtime-host-contact-regained.ts

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


📝 Walkthrough

Walkthrough

The changes add structured refusal handling for journal reads and session launches. The renderer now carries refusal details into read-error state, reconnect decisions, and user-facing notices. Delivery failures now produce per-message notices with retry controls based on outbox state and journal failure facts. Tests cover refusal classification, retry behavior, reconnect timing, and notice wording.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 4c039

No blocking issue is established for the reviewed reconnect changes; this PR is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 4c039

The affected chat is experimental and off by default. The review found no demonstrated new access path or security finding, but the new failure and recovery behavior has enough moving parts to warrant review.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected new recovery path is scoped to an existing structured-session read for a selected environment. The test-classified public ranges do not demonstrate expanded external reachability or new write authority.

Trust Boundaries and Controls

  • observed — The host converts journal-open errors to refusal codes before the desktop derives failure wording. A regained-contact notification requests a read retry through the existing scheduler; it does not override the final-refusal stop condition.

Resilience and Maintainability Implications

  • inferred — A stale legacy status response could affect when the new recovery callback fires, but the inspected path does not show that it can bypass refusal gating or expand access. Concurrent publication behavior remains a coverage gap, not an established finding.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 39.19% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 74 functions across 52 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: native-chat failures now show plain-language explanations at the affected UI element. It is specific and related to the changeset.
Description check ✅ Passed The description is comprehensive and covers the user impact, implementation, rationale, testing, visual proof, known weaknesses, platform scope, and checklist items. It references the related issue th…
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — full 67-file diff at head f7892dc8 against base 8b410b4, with independent passes over the reconnect backoff, the "no host error.message reaches a surface" invariant, and the host-side open classification.

  • Per-message delivery notices — the single NativeChatDeliveryRetry strip is gone; each failed user row renders its own NativeChatDeliveryNotice from the new structured-agent-session-delivery-notices.ts, derived per render and keyed by agentJournalSubmissionKey (which is exactly the projected message id). Retry availability reuses the drain's own admitStructuredAgentSessionOutboxEntry rule, and a Retry now clears the queue's hold only when it is that message's own hold.
  • Read-failure wording and finality — structured-agent-session-read-failure-notice.ts drives the one pane line; journalCorrupt stops reconnecting, journalUnavailable keeps retrying with backoff. I traced the subscribe path for local, remote-environment, and SSH targets and confirmed a refused open emits no subscribe frame, so the reset() in handleEvent cannot pin the backoff — the documented 750 ms → 30 s growth holds in production.
  • Host classification — journal-open-failure.ts classifies at the read door and logs once per session+failure (capped); storage text and paths stay in the log while the wire carries the bare code plus data.refusal.
  • Typed facts over strings — start-failure rows are keyed by a shared identity, the start-failure facts hook holds identity across streaming, and refusals thrown by Stop/answer/option/goal/create are worded from typed data. I traced every renderer .message/String(error) read in the covered paths and found no host refusal that reaches a user-visible string.
  • Restart-safe retry — rotation is decided from the message's own saved rejected state, so a rejected message retried after a restart goes out under a fresh id the first time.

The new tests assert exact copy and exact call arguments (not loose truthiness), and the harness change projects the real outbox so the notice keys and row ids are exercised through the same path production uses.

ℹ️ The open-failure wrap reports more than journal failures

journalOpenReadRefusal / createJournalOpenReadRefusals wrap the whole host.open, so an adapter.historyFilePath throw, an adoptOpened/publish failure, or a thrown defect is reported to the reader as agent_session_journal_unreadable and worded as a history open that can clear. This is not a regression — the write path classifies any openConversation throw the same way today, and the retry behavior is unchanged from before this PR — and I am not recommending a change here, only flagging it so the distinction is a deliberate one.

Technical details
# Non-journal `host.open` failures are worded as journal-open failures

## Affected sites
- `src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts:120-122` — wraps all of `host.open`.
- `src/main/native-chat/agent-session-wire/structured-agent-session-options-read.ts:92-94` — same for `openConversation`.
- `src/main/native-chat/agent-session-journal/journal-open-failure.ts:25-27` — `classifyJournalOpenFailure` falls back to `journalUnavailable`.

## What happens
`host.open` reaches `openStructuredAgentSessionConversation`, which also computes
`adapter.historyFilePath`, opens/imports the journal, and runs `adoptOpened` (publish). Any
throw from those, or from a plain bug, becomes `agent_session_journal_unreadable` with
`journalUnavailable` (or `journalCorrupt` only when the cause chain carries an
`ERR_SQLITE_ERROR` errcode 11/26). A non-journal fault is therefore presented as a transient
history problem that keeps retrying, rather than as the fault it is.

## Note
`AgentSessionRefusalError` raised for any other condition passes through unchanged
(`journal-open-failure.ts:35-37, 51-53`), so other refusals are not mislabelled. Pre-PR, the
same failures surfaced as raw text on the status line and retried anyway, so this changes the
words, not the behavior.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

The read retry doubles its wait up to 30 s during an outage, and nothing
reset it when the remote runtime reconnected, so the transcript could
lag the reconnect by up to 30 s. The read now watches the runtime
status store's contact-regained edges (hostContactEpoch for a
same-runtime return, connectionGeneration for a new runtime session)
and, when one lands, runs a waiting retry immediately with the wait
reset to its base.

@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 — the single commit added since the prior pullfrog review (f7892dc8 → 4c03975). It makes a remote chat's read retry immediately when its host comes back, instead of waiting out the grown backoff.

  • Host-contact-regained subscription — new runtime-host-contact-regained.ts watches the runtime status store and fires when an environment's hostContactEpoch or connectionGeneration moves past its last value, covering both a same-runtime return and a new runtime session; a cleared-then-returning entry still counts as a reconnect.
  • Read transport hooks the edge — structured-agent-session-read-transport.ts subscribes for environment targets only, and on a regain calls retryNow(): it resets the doubling wait to 750 ms and, if a retry is pending, runs it immediately. A local read is unchanged, and the watch is torn down in dispose().
  • Tests — the transport test drives an outage to the 30 s cap, asserts the immediate reconnect on the contact edge and a 750 ms wait after a post-reconnect failure, and that a local read never watches; the new store test covers the epoch/generation edges, the same-runtime case not moving connectionGeneration, and environment isolation.

The backoff growth, journalCorrupt finality, and teardown from the earlier commits are untouched, and the new subscription is cleaned up alongside the reconnect scheduler.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025
brennanb2025 merged commit 9d11a75 into main Sep 28, 2026
42 of 44 checks passed
brennanb2025 added a commit that referenced this pull request Sep 28, 2026
Conflicts with #23608 resolved to keep a crash-doubted or older not-delivered send drawn as sent,
with no barrier or Retry, while keeping its per-message notices, the retryControl wording and the
Retry id rotation read from the message. Its admission test that relied on a live unknown turning
unconfirmed now pins that a live unknown holds nothing and offers no Retry.
brennanb2025 added a commit that referenced this pull request Sep 29, 2026
…3614) into #23059

Conflicts, resolved to keep main's mechanism and #23059's turn facts:
- Codex turn boundaries and translator: main's #23682 wins; the error-driven
  `fail` end and the oversized-notification settler are gone, and only
  `turn/completed` ends a turn, a command's turn included.
- Codex adapter: the compaction tracker stays deleted; #23618's settlement of
  sends in an ended turn is kept, and `codexDispatchRejection` is imported from
  its new home.
- Journal reducer: main's per-item creating fence (#23456) moves into the item
  and submission folds this branch split out of the reducer.
- Stale-turn verdict: main's proof-of-death revision; every turn revision states
  the thread scope.
- Failure words: main's retry-control rules (#23608) with this branch's any-command
  retry step; `commandRefused` drops its retry step beside a Retry, as hostFault does.
- Delivery: both the commit wake and main's death-evidence resettle and dispose.
- Fold row and transcript slots: both the turn-outcome flag and main's `agentId`.
- pr.yml and pnpm-lock.yaml taken from main.

Beyond the conflicts, the merged files needed:
- use-structured-agent-session: both sides added the same `submissions` key; one kept.
- structured-agent-session-projection: the root-item import main still used is unused here.
- The desktop message list and the mobile chat view each grew past the line limit
  from both sides' additions: the turn-membership memo moves into its own hook, and
  the mobile list footer (live status, then waiting rows) into its own helper.
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