Repository navigation
feat(native-chat): one notice card above the message box for the chat's own errors - #25835
Conversation
374a792 to
c653d41
Compare
e420c5d to
27c4820
Compare
Review and QA summaryThe problem. Orca's own errors in a native chat were loose lines of plain text between the conversation and the message box, from four different places:
What changes for you. All of these are now rows of one bordered card attached above the message box, errors first:
There is no Retry for a message that wasn't sent. Review. One full read-only review of the code against
QA. Two isolated, hidden dev builds (own profile and home folder; a stand-in Not verified in the app: the raw-error block, the paste notice, and the card above an approval card. These are covered by tests. Linux, Windows and SSH-hosted chats weren't run; the change is renderer-only with no platform-specific code. Related. #25959 (desktop sends in memory) shows its "why your text came back" sentence through the same chat error line, so it becomes a row here with no extra wiring. Whichever lands second rebases. #24898's host notice (" is offline / reconnecting") is still its own line; a follow-up makes it a row. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (6)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe composer now accepts multiple typed notices. Notices can include separate error details, actions, and dismissal callbacks. Paste and structured-send failures use shared error conversion helpers. Structured sessions combine launch, session, and composer errors, then display notices in the composer or prompt slot. The status component no longer renders session or composer errors. English, Spanish, French, Japanese, Korean, and Chinese locales add related strings. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to This change adds composer and prompt-slot notices while preserving distinct rows and failed-launch retry access. No actionable merge-blocking risk was identified in the reviewed boundaries. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 53.85% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 26 functions across 25 files. (6 skipped: 6 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
27c4820 to
0ec9968
Compare
…own errors Orca's own errors in a native chat were loose lines between the transcript and the message box: a red "Chat could not be started." line with Retry, a red paragraph for the chat's line and for a send or slash-command error (where the second was hidden whenever the first showed), and a muted attachment notice inside the composer that always used a broken-image icon. A send that threw showed the raw IPC error as the whole message. They are now rows of one card attached above the message box, errors first: each row has an icon, Orca's words, and its action (Retry for a failed start, dismiss for errors the user caused). Error text Orca did not write, such as a host or system error, is shown apart in a monospace block with a copy button, under the words saying what didn't happen. The card also shows where a question or approval card takes the composer's place. Also translates "Paste failed." and the attachments-with-a-command refusal.
0ec9968 to
b6205ad
Compare
Takes main's notice card (#25835) as main has it; the chat's send errors reach it through controller.error. The outbox Stop-withdrawal module stays deleted: #25217 only exported its Retry test for the projection's awaitsRetry flag, and no in-memory send ever waits on a Retry. Main's opening-send test is ported to optimistic messages, minus its two waiting-on-Retry cases. Also: a launch text handed back by a Stop before publish (or a failed start) is guarded like the sender's hand-back, so a draft write that throws is reported and never skips the Stop; the Stop-before-publish cases for a notes launch, an already-sent launch and a throwing hand-back are tested; the legacy outbox comment no longer names a build number.
ELI5
When something Orca itself does goes wrong in a native chat (the chat can't start, a slash command is refused, a send or paste fails), Orca said so with loose lines of plain red text floating between the conversation and the message box. They had no icon, no structure, and sometimes the raw internal error text was the whole message.
Now those messages sit in one tidy card attached to the top of the message box, one row per message. Each row has an icon, Orca's words, and its button: Retry for a chat that failed to start, × to clear an error you caused. If there's a raw system error behind it, that error is shown separately in a small monospace block with a copy button, under a plain sentence saying what didn't happen.
What Changed
The problem, as you saw it
Error invoking remote method 'agentSession:send': Error: connect ECONNREFUSED ….After
/modelwith an unknown model)Mechanism
NativeChatComposerNoticesis the card. It takes a list of notices, each with a kind (errororattachment), Orca's text, optional rawerrorText, and an optional action or dismiss handler.native-chat-structured-session-notices.ts: the failed or unconfirmed start (replacingNativeChatLaunchRetry, including main's rule that a launch-argument problem shows only its cause), the chat's own error line (main's rule for which error shows is kept as is), and the composer's error (held byuseNativeChatComposerError). It passes them to the composer, which adds its own paste and attachment notice (use-native-chat-composer-notice.ts) and renders one card above the input.NativeChatStructuredSessionStatus.setNativeChatPasteFailurekeeps Orca's own "Clipboard text is too large…" sentence as the row text even when it comes back from the main process.nativeChatNoticeFromErrorkeeps a main-process (IPC-wrapped) error apart from Orca's words, with the wrapper stripped. An error this window raised itself, such as the paste size limit, is already Orca's sentence and stays as the row text.onErrorgains an optionalerrorTextargument for the raw part.Why
Differences from the common pattern
Linked Issue
N/A (maintainer).
Visual Proof
Captured in two isolated, hidden dev builds of Orca (their own profile and home folder). A stand-in
codexrefuses to run, so the chat's start fails; no real agent ran. Before =main5b1bb78d14b; after = this PR on that main. The PR has since been rebased onto988f87e743e; the only change in this area is that the error callback now goes through main's new composer-transport hook.A chat that failed to start
A failed start and a refused slash command at once
After pressing × on the refusal: it goes, the failed start with Retry stays, and focus is back in the message box.
Not captured in the app, but covered by tests:
The in-app case above (a failed start plus a refusal) was two separate lines before, so both already showed; the hiding happened between the chat's own error and a composer error.
Testing
NativeChatComposerNotices.test.tsx: card order (errors first, nothing hidden), the live region, raw error in its own block with copy, Retry and dismiss only where given, the raw-vs-Orca-words split, and the paste size-limit sentence coming back from the main process.use-native-chat-composer-notice.test.tsx: the composer's own notice and the chat's errors stay separate rows (distinct keys) and dismiss independently.use-native-chat-structured-composer-send.test.tsx: a send that throws says "Your message was not sent." with the IPC error apart.NativeChatStructuredSession.notice-card.test.tsx: the chat's own error and a composer error both show; a send's raw error is apart and dismissible; the card stays above an approval card.native-chat-structured-session-notices.command-failure.test.tsx(moved from main'sNativeChatLaunchRetry.command-failure.test.tsx): the launch-command failure is still worded the same in its row, with Retry.NativeChatStructuredSession.test-composer.tsx, draws the card, so those tests check what users see.pnpm tc:webpasses.Notes
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)