Repository navigation
fix(native-chat): show host outages in the notice card - #26161
Conversation
|
Native chat showed an unreachable host as a separate line beside the card used for other notices. The moved row also used a crossed-out Wi-Fi icon instead of the sidebar’s slashed-server icon. This PR puts the offline/reconnecting message and Reconnect action into the existing card, including approval and question slots. The host now uses ServerOff: red offline, muted while reconnecting. Text colors, sizing, truncation and the existing connection behavior stay unchanged. N1/P2, excessive host wrapping in narrow windows, was fixed at The icon follow-up passed 21 targeted tests and the full The PR body now shows eight fresh macOS captures at Rendered proof uses hidden native Electron panes with injected host/conversation/prompt state and generated-preload fake responses. The new icon smoke verifies glyph, computed colors and unchanged geometry; earlier action runs at The PR remains ready for review. No merge or auto-merge was performed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe structured session now represents host outages as composer notices. The notice hook provides offline or reconnecting text and can expose a reconnect action that connects the runtime host and reloads projects. Notice rows display host-specific styling and honor disabled actions. Tests cover notice ordering, reconnect attempts, prompt interactions, and host recovery. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The host outage notice remains visible across composer and prompt modes, and Reconnect stays disabled during an attempt. No actionable merge-blocking risk remains; the change is ready for normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Place host notices before attachment notices. · NativeChatComposerNotices.tsx:20-22
src/renderer/src/components/native-chat/NativeChatComposerNotices.tsx:20-22
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPlace host notices before attachment notices.
The comparator prioritizes errors only. If an attachment notice precedes a host notice, the list renders error → attachment → host. The changed test now expects that order, but the required order is error → host → attachment. Add a priority for each kind and update the test expectation.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
05c53179-6f7c-44b3-bef9-81f6a7132acb
📒 Files selected for processing (7)
src/renderer/src/components/native-chat/NativeChatComposerNotices.test.tsxsrc/renderer/src/components/native-chat/NativeChatComposerNotices.tsxsrc/renderer/src/components/native-chat/NativeChatHostOutageNotice.tsxsrc/renderer/src/components/native-chat/NativeChatStructuredSession.host-outage.test.tsxsrc/renderer/src/components/native-chat/NativeChatStructuredSession.tsxsrc/renderer/src/components/native-chat/native-chat-composer-notice.tssrc/renderer/src/components/native-chat/use-native-chat-host-outage-notice.ts
💤 Files with no reviewable changes (1)
- src/renderer/src/components/native-chat/NativeChatHostOutageNotice.tsx
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.
…hat-host-notice-row # Conflicts: # src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx
ELI5
When a native chat's host becomes unreachable, its host message and Reconnect action appear outside the notice card used for other chat notices. The moved row also used a crossed-out Wi-Fi icon, so it did not match the sidebar's red slashed-server icon. This PR puts host status in the existing card, including approval and question slots, and uses the same server icon: red when offline, muted while reconnecting.
What Changed
Why
This completes #25835's visual follow-up for the host notice introduced in #24898. Extending the approved card reuses its row, ordering and action presentation; keeping the separate component would leave two notice surfaces. The pending action belongs to the session so replacing the input slot does not reset it.
Intended difference from the common pattern: Orca retains its approved single notice card and existing host-outage policy. This change moves the existing host explanation into that surface; it adds no recovery mechanism.
Linked Issue
Follow-up to #24898 and #25835. Maintainer-requested presentation change; no new issue needed.
Visual Proof
Matched macOS screenshots use the same populated native pane and both sidebars at 1200×850. BEFORE is the reviewed base; AFTER is pushed icon-fix commit
b7a33f1. The unreachable host and conversation data were injected in an isolated rig.The corrected server icon is muted while reconnecting, after the existing two-second grace. Offline text remains amber; only its icon is red.
Earlier Windows evidence at 4331763, before the icon correction
Windows screenshots at
4331763, before the icon correction, use the actual native app with injected host and conversation state. Windows layout QA at that commit completed: the host row stays 36px tall (38px card), with Reconnect inside the card in both themes and narrow layouts. No Windows baseline was captured; the matched before/after comparison above is macOS.The host text retains its single-line truncation; the attachment hint keeps its normal wrapping and dismiss control.
Supported narrow window and prompt slots
At 800×850 with both sidebars open, the chat pane is 169px wide. The baseline host line is 32px tall; the final card row is 36px tall with Reconnect in bounds. Host text is strongly elided, as with the old line. Existing narrow composer clipping is visible in the baseline and is outside this change.
Windows action evidence before the final text-only fix
At
c5b9e38, native Windows captured the populated chat and the actual pending Reconnect action. These earlier action checks remain distinct from the final4331763layout and prompt screenshots above.Testing
Rendered validation used hidden native Electron apps on remote macOS and Windows through Playwright's Chrome DevTools connection. These were isolated synthetic-host checks, not a manual desktop session or a real network outage. The native chat pane and notice components were used directly. The rig injected host status, transcript and prompt data, substituted fake-environment/session RPC responses in generated preload output, and held Reconnect promises. macOS also seeded the existing read-owner registry. No source fixture or screenshot is included in the PR diff.
Earlier Windows QA completed at
4331763, before the icon correction. The configuration-hash stop was an unrelated false alarm, confirmed by the maintainer. Other Codex sessions had added trusted-folder entries on the coordinator's laptop; no QA rig path was present. The saved final screenshots are now attached and their URLs and original file bytes verified; no QA rerun was needed. Windows' own four protected files are unchanged, owned test processes are gone, and private profiles are pruned. Agent hooks were seeded disabled before launch and observed false before chat interactions; no runtime settings snapshot preceded the initial onboarding dismissal. This evidence gap remains explicit.The earlier
c5b9e38rigs exercised pending Reconnect, blocked duplicate clicks, one existing failure toast, retry and restored-host removal; macOS also checked grace and a short blip. Windows checked refusal, older-history suppression, host plus attachment, and pending composer-to-approval handoff. Earlier4331763checks revalidated layout and prompt-row presence after the text-only fix. Fresh macOS captures atb7a33f1check ServerOff in both themes while offline and reconnecting, both narrow offline views, and approval/question slots. The new smoke checks the glyph, computed colors, compact row geometry and truncation; it does not repeat the earlier action checks. Hooks were observed disabled before any interaction in this macOS run; protected files matched after excluding unrelated Codex trusted-folder entries.b7a33f1: two targeted native-chat files and 21 tests passed, including red offline and muted reconnecting ServerOff assertions. Formatting and commit hooks passed. The fullorca-ci-checkspre-push gate passed all 23 checks, including lint, quality checks, localization and full typechecking through the machine queue.4331763and its preceding commits: the full native-chat suite passed 319 files/3,234 tests; after rebase, six affected files/107 tests passed; the narrow-layout fix passed four affected files/49 tests plus scoped web typecheck, formatting and changed-code checks. These results are historical, and the earlier ready workflow reused its exact prior tested source. They are not attributed to the new icon smoke.Verification limits: the corrected icon has not been recaptured on Windows; its attached captures remain earlier layout/action evidence. No real host loss/reconnection, screen-reader speech, normal populated local conversation, Linux rendered UI, simultaneous error+host+attachment card, or pending composer-to-question handoff was exercised. Ordering and prompt handoffs have source/unit coverage. The fixture blocked rendering all three notice kinds together; genuine send failures appeared in their existing message surface. Catalog console errors occurred during fixture runs, including the fresh macOS icon smoke and final Windows layout smoke, so no clean-console or network end-to-end claim is made.
Review
Independent functional and UI reviews, an architecture challenger who sketched its solution before reading the implementation, and final readiness review without counsel used Codex
gpt-6.1-solat xhigh effort. Rendered QA found N1/P2, excessive narrow host wrapping, fixed at4331763and checked by a fresh reviewer. Brennan then reported N2/P2, the wrong host icon. It is fixed atb7a33f1by reusing the sidebar’s ServerOff glyph and offline color; reconnecting remains muted. The focused follow-up source review found no further issues. No findings remain open and no nits were deferred. The card keeps its empty/populated live-region element within each slot; its instance changes across composer/prompt handoff, so speech continuity is unverified.Agent skill upstream boundary
Notes
Renderer presentation only: no persisted data, wire protocol, connection ownership, status store, provider or execution changes. React Native mobile uses separate chat components; existing routing excludes plain SSH and WSL workspace launches from this structured-chat surface, so those moved-row paths are not applicable. Paired-runtime desktop rendering is covered by the injected state checks. Folder workspaces do not require a separate code path here.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)