Repository navigation
fix(dialogs): dialogs that open by themselves take turns - #25674
brennanb2025 wants to merge 33 commits into
Conversation
After a restart several dialogs could open by themselves at once and stack: the resume-interrupted-chats modal, a feature tip, a contextual tour, the crash report, and the SSH passphrase prompt. Nothing scheduled them. One arbiter in the UI store now decides which automatic prompt is on screen: - automatic prompts (resume offer, crash report, feature tip) request a turn, shown one at a time by priority; the one on screen keeps it until it closes, and an owner that unmounts releases it - at launch they wait (up to 2 s) for this machine's resume read, so the resume offer goes first - user dialogs and dialogs answering something in flight (SSH credentials, confirmations, pinned-tab close, WebAuthn, workspace fallback, onboarding) are never delayed: they register as visible, and a prompt on screen steps aside until they close - a feature tip is marked seen, and a launch crash report acknowledged, only once actually shown - tours read the arbiter instead of one shared boolean that any dialog's close cleared
Review follow-ups to the dialog turn-taking: - the shared dialog primitive counts every open dialog, so a dialog that opens by itself also waits for dialogs outside the app's modal slot (unsaved changes, skill and remote-server updates, notebook setup, skill install); a prompt's own dialog is scoped out of the count - a feature tip is marked seen, and a crash report acknowledged and holding its turn, only once its dialog content has rendered; a tip replaced by the user before it ever showed keeps its turn and opens again afterwards - a tip whose dialog fails to render, or whose owner goes away, gives up its turn, closing only its own modal-slot entry - crash reports raised by themselves queue by identity, each taking its own turn; a second never replaces the first - the launch wait's 2 s bound runs from when this machine's resume read starts, a machine that holds no chats ends it at once, and a boot backstop ends it if neither happens; expiry only changes order, so a resume offer read later is still shown after whatever went first
…s belong to the queued item - A shown automatic prompt stays mounted under a dialog opened over it; its dialog scope hides it (data-stepped-aside on the shared dialog primitive), so typed crash notes, a send in flight and the CLI tip's setup terminal survive. Only the automatic instance is scoped; dialogs a prompt opens inside itself are part of it. - Being shown is a property of the queued request (id + key), so an owner's next item never inherits it and its content's mark is never lost to effect order. - User dialogs are read from what is rendered (presence mirrored into the store), not the modal slot, so a modal that failed to render holds nothing back, including its own crash report. - CommandDialog counts as a dialog; a queued prompt that cannot show does not block a tour inside the dialog holding the screen; the app-level tip owner no longer re-renders on every dialog. - Help > Report Crash drops that report from the automatic queue; error-boundary reports not yet shown coalesce to the newest.
…unt; Help keeps the report on screen - The app-open tip asks for a turn only while the modal slot is free or its own, so it never replaces a user's modal (even one still loading) or answers the hooks trust prompt for them. - Lazily loaded user dialogs count as on screen while their code loads (DialogLoadingSuspense renders the presence marker as its fallback); a failed load still counts as nothing. The tip has its own Suspense so its own loading never holds it back. - Help > Report Crash over the report on screen keeps that same dialog: the surface is keyed by report, and closing a report's dialog drops that report from the queue however it was opened. - When a dialog over a stepped-aside prompt closes and focus fell to the page body, focus returns to where it was in the prompt.
…ansitions; the owner settles crash sends - A user's modal-slot dialog counts as on screen from openModal, before its code or first render, unless a modal surface is showing its error fallback (FailedModalSurfaceMarker, mirrored into the store) or the slot is a handoff that renders no dialog (project-added). Sidebar dialogs get a modal-surface boundary. DialogLoadingSuspense stays only for the SSH prompt, outside the slot. - Automatic prompt dialog content takes no CSS transitions: duration-200 also transitioned visibility, so a prompt stepping aside stayed seen for 200 ms, and on return it was hidden for a frame, when focus was put back. - Crash report sends are owned by CrashReportDialog, keyed by report id: a result settles its report (drops it from the queue, closes a dialog showing it) even after the dialog that started it was replaced; a report being sent shows as sending wherever it is shown. Help pressed again keeps the report its dialog shows.
…d later stack over it Drops the step-aside machinery: a shown resume offer, crash report or feature tip is no longer hidden while another dialog is up. A dialog opened over it stacks on top, as on main. Turn-taking, waiting for not-yet-shown prompts, and seen/acknowledged-on-render are unchanged.
Review status: ready for review (design B)The problem. After Orca restarts (especially after an update), several dialogs that open by themselves could appear at the same time and stack on each other: the "Resume interrupted chats?" offer, the crash report, a feature tip, a product tour. Nothing made them take turns. What changes for the user. These dialogs now appear one at a time, in a fixed order: resume offer, then crash report, then feature tip. A tour starts only when none of them is up or waiting. At launch they wait briefly (up to 2 s) for this computer's resume check, so the resume offer goes first. One that hasn't appeared yet also waits while you have a dialog open (palette, confirmation, SSH passphrase prompt), and appears after you close it. A dialog you open after one is already showing stacks on top of it, as on main. The one underneath keeps everything you typed. Design change in this round. An earlier version hid ("stepped aside") a self-opening dialog that was already showing whenever another dialog opened, then brought it back. Every review round found a new bug in that hiding: focus lost on return, and a visible flash when the covering dialog's code loaded late (the resume offer appeared, then vanished up to a second later). We removed the hiding. That also removed this round's open bugs about the flash, focus and transitions.
Fixed across the review (5 rounds):
Deferred (P3 / no user impact found):
Residual, unproven: a tip whose dialog throws every time is never marked seen, so it would retry each launch. Verified:
Not verified:
QA process note: Grok was skipped as the QA driver on the M4Air, because it reads its own real profile at startup. The run used opencode (DeepSeek V4.1 Flash). Before a hold, the driver opened the new-tab agent menu once and clicked "Claude". That opened a structured chat through the sanctioned stand-in script, inside the isolated profile. No other agent ran. The rig was then rebuilt with the agent menu banned and every agent except Claude disabled. |
…2025/rsc-launch-prompt-turns # Conflicts: # src/renderer/src/components/NativeChatResumeOnRestartModal.test.tsx # src/renderer/src/components/native-chat-resume-on-restart-store.ts # src/renderer/src/components/status-bar/NativeChatResumeStatusSegment.tsx
…main merge The opted-in launch resume is no longer awaited by the launch read, so the existing settle hook marks the launch decided on every path; tests reset the dialog request module themselves.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (5)
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 pull request adds a dialog registry for automatic and user dialogs. Startup hydration coordinates onboarding and feature-tip checks. Crash reports and native-chat resume offers use registry entries and startup discovery. Contextual tours and several UI behaviors now check registered dialog visibility. The app-open feature-tip flow replaces the former feature-tip modal path. The project-added dialog component and modal branch are removed. Priority: ➖ Normal Merge Risk: 🔵 Low · up to A narrow timing race can leave an already-active chat in the resume offer. The PR is mergeable with owner awareness and a follow-up for that stale-offer case. 🚥 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 |
One way back to the resume dialog: the status bar entry, a resume toast's Show and a reconnect toast's Show chats all re-read first and open only over rows, as main's opener does. A server out of contact, or one whose re-read fails, opens on its last listing instead of holding the click back. A paired resume always ends in one toast: the continue call waits as long as a command does for the host's answer, and when that answer is lost the chats the server no longer lists count as unconfirmed instead of vanishing.
…ogs show Replaces the turn-taking layer (presence markers mirrored into the store, hand registrations, modal-slot inference, failed-surface flag, subtree exemption, 2 s / 10 s launch timers) with one window-scoped registry of every dialog: user and response dialogs, the ones the app opens by itself, and a running tour. - The shared Dialog and Command roots register while open; the modal slot, the SSH prompt, onboarding and each self-opening dialog reserve an entry before their code loads and their dialog root adopts it. - Self-opening dialogs are admitted one at a time when nothing else is up, in the order crash report, feature tip, resume offer; each kind also waits until every earlier kind's startup check has answered (ready, none or unavailable), so a fast late check never shows first. No timers. - A dialog the user opens stacks over a shown one, which stays mounted; the next is admitted only once the previous finished closing. - The app-open feature tip renders from its own host, not the modal slot, so a user modal can no longer replace it. - Crash reports queue by report id; a second report never replaces the first. Help > Report Crash over a report on screen keeps that report. - Tours go after every self-opening dialog and run inside the composer, not over dialogs opened from it.
…e and a late load never swaps a taken-over report
… when settings fail A failed settings read left the feature-tip startup check pending all session, so the resume launch offer never showed. Startup now hands the tip check its inputs when the onboarding read resolves (null when settings or onboarding could not be read, or startup failed first), instead of when it later shows onboarding, and the check no longer waits for the CLI probe when no tip is left either way.
…nted An admitted crash report or tip whose code was still loading stayed admitted when a user or SSH dialog opened, then mounted on top of it and took focus. In dev StrictMode the tip host's simulated unmount also let a queued resume offer take the tip's turn. Admission is now decided again on every change until the dialog paints: a dialog opened meanwhile stays on top and the unpainted one goes back to its place; an earlier kind queued meanwhile goes first.
…he slot The app-open tip no longer sets activeModal, so readers that took activeModal !== 'none' as 'a dialog is up' missed it: Escape on the Tasks page closed the page instead of the tip, and the usage-percentage callout showed over it. The readers without a DOM overlay check (Tasks Escape and Cmd+F, the usage callout, hovered-workspace delete, terminal focus on activation) now also ask the registry whether any dialog is up.
…y more
The app-open tip was the only caller of openModal('feature-tips'); it now has
its own host, so the slot wrapper, its lazy mount and the union member go.
…t as it is Taking it over changed nothing on screen; dropping it removes the key aliasing between the Help dialog and the report it took over.
…2025/rsc-launch-prompt-turns
…econnect The resume dialog now takes its place through the app's one dialog registry: a launch request is an automatic entry that waits behind the crash report and the tip, a user request opens at once, and the dialog draws from its entry's phase, open until its exit animation ends. The dialog's owner, and only it, answers this launch's startup discovery from this computer's read; the status entry reads offers without it, and a paired server's read never holds the order. A launch request reaches "shown" when the dialog is actually on screen, which is also when what it lists counts as decided and the restart toasts for it go. A launch request after the user asked is dropped, as the registry's API states. Also: the hook no longer re-checks for the user's own chats here; the store already drops an unseen launch request once none are left. The unused offer-activity watcher #25674 extracted is removed (this branch watches per machine), and #25674's behavioral test reads offers per machine.
|
After restart, crash reports, the feature tip and the interrupted-chat offer could compete for the screen. A user dialog could replace the tip, and another report could replace the one being read. Automatic dialogs now take turns crash reports → feature tip → resume offer. User/response dialogs open immediately above shown content and retain its notes/setup; B stacking stays for now. One content registry replaces before-content reservations and duplicate coordination. Crash reports queue by ID; sends survive closing their surface; acknowledgement/seen writes happen after content appears and never delay close. Review fixed late tip delivery, palette focus through exit, lazy first-run onboarding and enabled recovery reads with missing settings. Removed the dead compatibility dialog, generic tip fallback and separate resume discovery observer/completion flag. The common pattern has no shared lazy pre-content reservation, so the requested option B counts only attached content. Comparable review size: +1393/-280 → +1217/-688, net +1113 → +529; 584 net production lines cut, including 251 in the second cut. Core 578 → 292. Final main integration preserves #25675's decision markers and makes the current-main footprint +1219/-688, net +531. Incoming main files are not counted as savings. Remaining addition buys the shared queue/content authority, distinct crash/send lifetime, tip survival and immediate keyed resume. Existing responsibility splits add25 counted lines and use no file-length exception.
Final pushed e0f4bd1: 176 tests/13 files, fresh gated renderer + node typechecks, 22/22 mandatory static checks including React Doctor, and full overlap-file lint passed. Final CI passed on this exact head: 17 jobs passed, 11 optional jobs skipped, including both Linux/Windows packages and all ten Node24 unit shards. Mobile Checks and PR test LoC also passed; Headless Node server was skipped. No red job or step remained to classify against main. The ready-triggered follow-on CI run also passed on the same head: 5 jobs passed, 22 skipped, including the end-to-end app build, Electron native-cache preparation and changed end-to-end specs. Headless Node passed too. The full end-to-end matrix and optional native WSL/IME/SSH CI jobs were skipped. No code, main merge or push followed the ready transition; no red remained to classify. Fresh full reduced-design review and narrow merge review found no remaining actionable P0–P3. Readiness checklist omitted counsel. No altered automatic launch call/ownership/order/lifetime, so the conditional short live-order rerun was not triggered; live QA remains explicitly at 45319de. Verified live: hidden Windows automatic order with reverse data timing; real virtual-key palette stacks over all3 prompts and retained notes/selection/terminal; crash FIFO/deduplication, failed/hanging acknowledgement/discovery, cold lazy request and immediate manual resume. Real Docker encrypted-key SSH: Escape over crash, credential Cancel over tip, wrong-key error over resume, correct Unlock/reconnect/Add project/selected branch creation; independent host Git confirmed the checkout/branch. Archived verified screenshots are embedded in the PR body. Local wrong-key and settled branch/backing artifacts were inspected; detailed crash/tip SSH DOM and remaining shots are remote. Verified environment guard: actual six pane fields stayed inside the run. Before/after content hashes for C:/Users/neil/.codex/config.toml, C:/Users/neil/.codex/hooks.json and C:/Users/neil/.orca/agent-hooks/codex-hook.cmd, plus empty .codex/hooks inventory, matched. Initial size/mtime omission was disclosed and approved. 60 complete later hash/size/mtime snapshots were independently parsed, unchanged. Snapshots61–69 followed later QA stages, rather than no work after60. Snapshot61 followed the final resource audit;62–63 were read-only transfer checks;64–65 bracketed closing owned control tabs;66–67 bracketed removing the owned QA checkout;68–69 were further read-only transfer checks. Their9 actual local event receipts say unchanged; full later payloads are not transferred. Those later QA stages launched no app/agent, ran no install and made no hooks/auth/config edit or restore. No file contents are published. Pending, not passed: final after-loss real-file fingerprint; checkout-path/branch and cleanup audit; complete cumulative metadata payload; remaining control-tab closure/private stage directory cleanup and lock release; untransferred detailed SSH screenshots. Owned app/container/daemon/temporary keys/profiles cleanup is confirmed; tab closure/checkout removal are acknowledgements rather than a final path audit. Windows lock remains UNVERIFIABLE. No recovery actions on the shared runtime; coordinator is taking recovery to Brennan and explicitly decided these hygiene/evidence items do not hold ready. Not exercised: the full CI end-to-end matrix and optional native WSL/IME/SSH jobs were skipped (changed end-to-end specs passed); native Help-menu input/physical OS or macOS focus, separate macOS/Linux/WSL app GUI, mobile emulator (no changed mobile surface/wire contract). Startup data/discovery faults and external submit/install results were simulated; renderer/terminal/SSH interaction was real, and the displayed installer was never executed. M4Air was unreachable. Cross-host parent refusal selector_not_found/LINEAGE_PARENT_NOT_FOUND is recorded; coordinator approved --no-parent, never C:/orca/orca. Required private Codex/OpenCode chain was unavailable, so permitted scripted CDP fallback was used. Deferred: never-rendered tip can reappear on another launch; closed crash payloads retained for the window; existing sheets/popovers outside the dialog boundary; an overbroad test caption (nit). No unresolved proven severity issue. #25676 is independent; whichever merges second adapts at its final main merge. Marking ready under the coordinator's explicit direction after final CI; never merged and auto-merge remains off. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
89c954a9-5766-48e5-a66b-bbcd64fca51f
📒 Files selected for processing (76)
src/renderer/src/App.tsxsrc/renderer/src/app-shell/AppRootSurfaces.tsxsrc/renderer/src/app-shell/onboarding-contextual-tours.test.tsxsrc/renderer/src/app-shell/self-opening-dialogs.test.tsxsrc/renderer/src/app-shell/startup-tip-check.test.tsxsrc/renderer/src/app-shell/use-app-startup-hydration.tssrc/renderer/src/app-shell/use-onboarding-and-feature-tips.tssrc/renderer/src/components/NativeChatResumeOnRestartModal.test.tsxsrc/renderer/src/components/NativeChatResumeOnRestartModal.tsxsrc/renderer/src/components/NewWorkspaceComposerModal.tsxsrc/renderer/src/components/WorktreeBaseFallbackDialog.test.tsxsrc/renderer/src/components/WorktreeBaseFallbackDialog.tsxsrc/renderer/src/components/browser-webauthn-account-dialog.test.tsxsrc/renderer/src/components/browser-webauthn-account-dialog.tsxsrc/renderer/src/components/confirmation-dialog.test.tsxsrc/renderer/src/components/confirmation-dialog.tsxsrc/renderer/src/components/contextual-tours/ContextualTourOverlay.remeasure.test.tsxsrc/renderer/src/components/contextual-tours/ContextualTourOverlay.tsxsrc/renderer/src/components/contextual-tours/ContextualTourOverlay.visibility.test.tsxsrc/renderer/src/components/contextual-tours/contextual-tour-gate.test.tssrc/renderer/src/components/contextual-tours/contextual-tour-gate.tssrc/renderer/src/components/contextual-tours/use-contextual-tour.tssrc/renderer/src/components/crash-report/CrashReportDialog.test.tsxsrc/renderer/src/components/crash-report/CrashReportDialog.tsxsrc/renderer/src/components/crash-report/CrashReportDialogSurface.overflow.test.tsxsrc/renderer/src/components/crash-report/CrashReportDialogSurface.tsxsrc/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.tsxsrc/renderer/src/components/feature-tips/AppOpenFeatureTip.test.tsxsrc/renderer/src/components/feature-tips/AppOpenFeatureTip.tsxsrc/renderer/src/components/feature-tips/FeatureTipsModal.tsxsrc/renderer/src/components/feature-tips/feature-tip-modal-state.test.tssrc/renderer/src/components/feature-tips/feature-tip-modal-state.tssrc/renderer/src/components/feature-tips/feature-tip-startup-gate.test.tssrc/renderer/src/components/feature-tips/feature-tip-startup-gate.tssrc/renderer/src/components/feature-tips/feature-tip-telemetry.test.tssrc/renderer/src/components/feature-tips/feature-tip-telemetry.tssrc/renderer/src/components/link-routing-preference-dialog.tsxsrc/renderer/src/components/native-chat-resume-offer-activity-watch.tssrc/renderer/src/components/native-chat-resume-on-restart-dialog.tssrc/renderer/src/components/native-chat-resume-on-restart-store.tssrc/renderer/src/components/onboarding/OnboardingFlow.tsxsrc/renderer/src/components/sidebar/ProjectAddedDialog.test.tsxsrc/renderer/src/components/sidebar/ProjectAddedDialog.tsxsrc/renderer/src/components/sidebar/hovered-workspace-delete.tssrc/renderer/src/components/sidebar/index.tsxsrc/renderer/src/components/sidebar/worktree-list/navigation/use-reveal-requests.test.tsxsrc/renderer/src/components/status-bar/UsagePercentageDisplayChangeNotice.test.tsxsrc/renderer/src/components/status-bar/UsagePercentageDisplayChangeNotice.tsxsrc/renderer/src/components/ui/command.tsxsrc/renderer/src/components/ui/dialog.tsxsrc/renderer/src/components/use-task-page-global-effects.dialogs.test.tsxsrc/renderer/src/components/use-task-page-global-effects.tssrc/renderer/src/components/use-task-page-search-actions.tssrc/renderer/src/lazy-modal-mount-state.test.tssrc/renderer/src/lazy-modal-mount-state.tssrc/renderer/src/lib/dialog-registry-entry.test.tsxsrc/renderer/src/lib/dialog-registry-entry.tsxsrc/renderer/src/lib/react-error-boundary-reporting.tssrc/renderer/src/lib/workspace-activation-terminal-focus.test.tssrc/renderer/src/lib/workspace-activation-terminal-focus.tssrc/renderer/src/startup/startup-deferred-catalog-refresh.test.tssrc/renderer/src/startup/startup-deferred-catalog-refresh.tssrc/renderer/src/startup/startup-discovery-read.tssrc/renderer/src/startup/startup-runtime-session-hosts.tssrc/renderer/src/store/dialog-registry-state.test.tssrc/renderer/src/store/dialog-registry-state.tssrc/renderer/src/store/dialog-registry-test-state.tssrc/renderer/src/store/dialog-registry.tssrc/renderer/src/store/slices/ui-contextual-tours.test.tssrc/renderer/src/store/slices/ui-slice-test-harness.tssrc/renderer/src/store/slices/ui/ui-slice-contract-contextual.tssrc/renderer/src/store/slices/ui/ui-slice-modal-actions.tssrc/renderer/src/store/slices/ui/ui-slice-tour-actions.tssrc/shared/usage-percentage-display-change-notice.test.tssrc/shared/usage-percentage-display-change-notice.tstests/e2e/add-project-default-checkout.spec.ts
💤 Files with no reviewable changes (12)
- src/renderer/src/components/link-routing-preference-dialog.tsx
- src/renderer/src/components/sidebar/ProjectAddedDialog.test.tsx
- src/renderer/src/components/feature-tips/feature-tip-modal-state.test.ts
- src/renderer/src/components/confirmation-dialog.test.tsx
- src/renderer/src/components/feature-tips/feature-tip-modal-state.ts
- src/renderer/src/components/sidebar/ProjectAddedDialog.tsx
- src/renderer/src/components/feature-tips/feature-tip-telemetry.test.ts
- src/renderer/src/components/feature-tips/feature-tip-telemetry.ts
- src/renderer/src/components/WorktreeBaseFallbackDialog.tsx
- src/renderer/src/components/browser-webauthn-account-dialog.tsx
- src/renderer/src/components/confirmation-dialog.tsx
- src/renderer/src/components/contextual-tours/contextual-tour-gate.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.
| if (previous !== key) { | ||
| seen.set(sessionId, key) | ||
| // A first sighting is news only if newer than the list; a change to a known chat always is, | ||
| // since the host may have answered the list just before the change was delivered here. | ||
| changed ||= previous !== undefined || summary.updatedAt > offer.listedAt | ||
| } | ||
| } | ||
| if (changed && offeredChatRefresh === null) { | ||
| offeredChatRefresh = setTimeout(() => { | ||
| offeredChatRefresh = null | ||
| refresh() | ||
| }, OFFERED_CHAT_REFRESH_DELAY_MS) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Retain activity that cannot be refreshed during an action.
If the debounce fires while a continue or dismiss action is in flight, refreshAfterOfferedChatActivity() declines the read. The watcher has already updated seen, so it will not retry that activity change. If the action's host answer predates activity in another offered chat, that chat remains in the offer until another event triggers a read. Keep a pending refresh and run it after the action settles.
| } else if (enabled || !holds) { | ||
| const asked = getNativeChatResumeOnRestartDialogRequest() !== null |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
set -eu
head_rev='e0f4bd1586a321e8989ce168cf90c88584b77a64'
base_rev='e9b3efa7d75cf36244e2a1a03bfdd6f27a114f5b'
git cat-file -e "$head_rev^{commit}"
git cat-file -e "$base_rev^{commit}"
printf '%s\n' '--- changed file at reviewed head ---'
git show "$head_rev:src/renderer/src/components/native-chat-resume-on-restart-store.ts" | nl -ba | sed -n '1,460p'
printf '%s\n' '--- registry file at reviewed head ---'
git show "$head_rev:src/renderer/src/store/dialog-registry-state.ts" | nl -ba | sed -n '1,360p'
printf '%s\n' '--- relevant references at reviewed head ---'
git grep -n -E 'useNativeChatRestartOffer|native-chat-resume|ownsStartupDiscovery|readLocalStructuredAgentSessionsHeld|settle.*startup|startup.*settle' "$head_rev" -- 'src/renderer/src' || true
printf '%s\n' '--- PR diff for relevant files ---'
git diff --unified=30 "$base_rev" "$head_rev" -- \
src/renderer/src/components/native-chat-resume-on-restart-store.ts \
src/renderer/src/store/dialog-registry-state.tsRepository: stablyai/orca
Length of output: 43030
🏁 Script executed:
set -eu
h=e0f4bd1586a321e8989ce168cf90c88584b77a64
b=e9b3efa7d75cf36244e2a1a03bfdd6f27a114f5b
git cat-file -e "$h^{commit}"
git cat-file -e "$b^{commit}"
git show "$h:src/renderer/src/components/native-chat-resume-on-restart-store.ts" | nl -ba | sed -n '280,410p'
git show "$h:src/renderer/src/store/dialog-registry-state.ts" | nl -ba | sed -n '1,280p'
git grep -n -E 'useNativeChatRestartOffer|ownsStartupDiscovery|native-chat-resume|readLocalStructuredAgentSessionsHeld' "$h" -- 'src/renderer/src' || true
git diff --unified=20 "$b" "$h" -- src/renderer/src/components/native-chat-resume-on-restart-store.ts src/renderer/src/store/dialog-registry-state.tsRepository: stablyai/orca
Length of output: 42082
🏁 Script executed:
set -eu
h=e0f4bd1586a321e8989ce168cf90c88584b77a64
printf '%s\n' '--- discovery implementation and references ---'
git grep -n -E 'function readStartupDiscovery|const readStartupDiscovery|readStartupDiscovery\\(' "$h" -- 'src/renderer/src' || true
printf '%s\n' '--- relevant startup tests ---'
git show "$h:src/renderer/src/app-shell/self-opening-dialogs.test.tsx" | nl -ba | sed -n '690,845p'
printf '%s\n' '--- registry store and consumers ---'
git grep -n -E 'selectAdmittedDialog|selectDialogPhase|startupSources|settleStartupSource|nextAutomaticDialog' "$h" -- 'src/renderer/src/store' 'src/renderer/src/app-shell' 'src/renderer/src/lib' || true
printf '%s\n' '--- startup discovery files ---'
git ls-tree -r --name-only "$h" | grep -E 'startup-discovery|dialog-registry' || trueRepository: stablyai/orca
Length of output: 20424
🏁 Script executed:
set -eu
h=e0f4bd1586a321e8989ce168cf90c88584b77a64
printf '%s\n' '--- startup discovery helper ---'
git show "$h:src/renderer/src/startup/startup-discovery-read.ts" | nl -ba
printf '%s\n' '--- local structured session helper references ---'
git grep -n -F -- 'readLocalStructuredAgentSessionsHeld' "$h" -- 'src/renderer/src' || true
printf '%s\n' '--- local structured session helper owner candidates ---'
git ls-tree -r --name-only "$h" | grep 'local-structured-chats' || trueRepository: stablyai/orca
Length of output: 1923
Settle a true held-session result.
When enabled is false and readStartupDiscovery(readLocalStructuredAgentSessionsHeld()) resolves to true, the callback skips settleStartupSource(...) because enabled || !holds is false. readStartupDiscovery preserves the resolved boolean; the promise continuation does run, so a separate subscription is not needed.
The source can remain pending, and selectTourBlocked blocks non-forced tours while any startup source is pending. Add an explicit settlement path for holds === true.
…ct toast, one status entry, one dialog grouped by machine When a paired Orca server restarts while chats on it are mid-reply, this desktop now finds out: each verified connection to a server reads its restart offers, a toast names the server and offers Resume or Show chats, the status-bar entry counts the user's own interrupted chats on every machine, and the resume dialog lists every machine with its own row, tri-state box and workspaces. A server that predates the offer capability shows nothing. Rebuilt on #25652 alone: this branch no longer depends on #25674. The dialog opens through main's request/consume/subscribe API; a request carries only the machine it was opened for, and a request while one is open keeps the open dialog (its focus and the user's ticks). The offer-store, actions, toast, status entry, machine rows, Select all, own-only Dismiss, outcome toast, listed dismissal, origin, per-machine fence and the 195 s continue budget are this branch's own work, carried over unchanged.
…the one end-of-review sync) #25652's resume-dialog tests open the dialog as the user and start past the startup checks, like the other resume-dialog tests here.
|
Main merged in once, at the end of review (0d8f16d), after #25652 (resume dialog shows a tree grouped by machine) landed and conflicted with this PR.
|
This branch has not been deployed
1 participant
ELI5
After Orca restarted, a crash report, feature tip and interrupted-chat offer could open together. They used separate checks for whether another dialog was open. A user dialog could replace the tip, and another crash report could replace the report being read.
Automatic dialogs now take turns: crash reports → feature tip → resume offer. A dialog you open yourself appears immediately above shown content and preserves its notes and setup. This stacking stays for now.
What Changed
Why
The common pattern has no shared reservation before lazy content loads. We chose option B: count a dialog once its content is attached to the screen. A request that never renders cannot silently occupy the slot. In a cold-load race, automatic content may appear while requested user/SSH content loads; the requested dialog then stacks above it.
Intended differences are explicit automatic priority and immediate user/response stacking. Actual startup reads have a 15-second observation budget so an unanswered check becomes unavailable; expiry does not cancel recovery or user actions. No display delays are used.
Ignoring whitespace and excluding tests/test-only support, the comparable review footprint changed +1393/-280 → +1217/-688 (net +1113 → +529): 584 net production lines removed, including 251 in the second cut. The coordination core shrank 578 → 292 lines. The final merged head against current main is +1219/-688, net +531; two integration lines preserve incoming launch-decision timing. Unrelated incoming main changes are not counted as savings. Conventional test-file-pattern counts are +1396/-280 before and +1240/-688 after, including 3 → 21 test-support lines.
This remains a net addition. It buys one content queue/authority, separate crash IDs and send lifetime, a tip that survives user dialogs, and immediate keyed resume. Enforced activity-watcher/catalog responsibility boundaries add 25 counted lines without new behavior or wrappers. The per-area table and API were delivered before push.
Linked Issue
Separate interrupted-chat recovery bug fix. #25676 no longer depends on #25674; it is rebuilt on #25652 and main's existing resume-dialog API. Whichever merges second adapts at its final main merge. If #25676 lands first, this PR registers its status-bar and reconnect notification's “Show chats” opens as user dialogs; if this PR lands first, #25676 adopts this PR's API.
The actual dialog/data owner uses
useNativeChatRestartOffer(enabled, { ownsStartupDiscovery: true }); ordinary subscribers keep the default without disposal authority. The separate discovery hook is removed. #25676 was not present in the final fetched main, so no speculative paths were added.Visual Proof
Archived hidden Windows screenshots at
45319de037d: seeded automatic data, genuine renderer actions and Electron virtual key input. Windows remained hidden/unfocused. These compare user stacking and Escape; they are not screenshots of the old bug.Cold lazy user request above crash content, then Escape:
Immediate user-opened resume:
A real encrypted-key SSH error leaves the interrupted-chat offer and its selection intact:
Testing
Final pushed
e0f4bd1586a, merged with maine9b3efa7d75c: fresh gated renderer and node typechecks, 22/22 mandatory static checks (including React Doctor), full overlap-file lint and 176 tests in 13 files passed. Final CI passed on this exact head: 17 jobs passed, 11 optional jobs skipped, including both Linux/Windows packages and all ten Node24 unit shards. Mobile Checks and PR test LoC also passed; Headless Node server was skipped. No red job or step remained to classify against main.The ready-triggered follow-on CI run also passed on the same head: 5 jobs passed, 22 skipped, including the end-to-end app build, Electron native-cache preparation and changed end-to-end specs. Headless Node passed too. The full end-to-end matrix and optional native WSL/IME/SSH CI jobs were skipped. No code, main merge or push followed the ready transition; no red remained to classify.
Fresh narrow source review confirmed that the merge preserves #25675's markers and only changes equivalent Radix imports. It changes no automatic launch call, discovery ownership, priority or content lifetime, so the conditional short live order rerun was not required. Live QA below ran on
45319de037d; it is not a live final-head claim.Hidden Windows CDP passed crash → tip → resume even with resume data ready first; all three palette stacks and retained notes/selection/terminal; crash-ID queue/deduplication; failed/hanging acknowledgement and discovery; cold lazy requests; and immediate manual resume. Help event delivery and one submission after double-click passed; external submission was stubbed.
A real Docker SSH host with a temporary encrypted key passed Escape over crash, credential Cancel over tip, wrong-key error over resume, correct Unlock, reconnect, Add project and selected branch creation. Direct host Git confirmed the created checkout and exact branch. Local wrong-key, settled checkout/backing results and archived screenshots were inspected; detailed crash/tip SSH DOM and remaining SSH screenshots are still remote.
The six actual pane environment fields pointed inside the run. Before/after guards for
C:/Users/neil/.codex/config.toml,C:/Users/neil/.codex/hooks.json,C:/Users/neil/.orca/agent-hooks/codex-hook.cmdand the empty.codex/hooksinventory matched. Initial evidence recorded content hashes/inventory; missing initial size/mtime was disclosed and approved. 60 complete later hash/size/mtime snapshots were independently parsed, all unchanged. Snapshots61–69 followed later QA stages, rather than no work after60. Snapshot61 followed the final resource audit;62–63 were read-only transfer checks;64–65 bracketed closing owned control tabs;66–67 bracketed removing the owned QA checkout;68–69 were further read-only transfer checks. Their nine local event receipts report unchanged; full later payloads have not transferred. Those later QA stages launched no app/agent, ran no install and made no hooks/auth/config edit or restore.Pending after Windows transport loss: final after-loss real-file fingerprint; final checkout-path/branch and cleanup audit; complete cumulative metadata payload; remaining control-tab closure/private stage directory cleanup and lock release; detailed SSH screenshots not yet transferred. The Windows lock is UNVERIFIABLE, not released. Confirmed cleanup includes owned app/container/daemon/temporary keys/profiles; tab closure and checkout removal were acknowledged, not independently audited after loss. Loss of contact is not evidence of process death. No Windows recovery action is being attempted; the coordinator owns the runtime recovery follow-up. These hygiene/evidence gaps do not hold ready under the coordinator's explicit decision.
M4Air was unreachable. The required isolated Codex/OpenCode driver chain was unavailable, so the permitted scripted CDP fallback was used. Discovery/send/install faults were simulated; renderer input, terminal setup and SSH were real. The displayed installation command was not executed. The changed end-to-end specs passed after marking ready; the full end-to-end matrix and native WSL/IME/SSH CI jobs were skipped. Native Help-menu input, physical OS/macOS focus and separate macOS/Linux/WSL app GUI checks were not exercised. No mobile surface or wire contract changed; emulator QA is not applicable.
Review
Fresh full review of the reduced design and narrow final-main review found no remaining actionable P0–P3. Readiness checklist used without counsel; source review covers lifecycle, performance, security, folder/SSH and cross-platform/compatibility impact. No new persisted fields, RPC/wire contracts, dependency or host execution path. Passing tests are regression evidence, not proof of architecture.
Agent skill upstream boundary
Notes
Review and required checks are complete. The coordinator explicitly authorized marking ready with the pending Windows items stated. No merge or auto-merge.
Deferred: never-rendered tips may be offered on a later launch; crash payloads remain for this window; existing sheets/popovers outside the dialog boundary are out of scope; an overbroad test caption is a deferred nit.
Windows placement refused the Mac parent with
selector_not_found/LINEAGE_PARENT_NOT_FOUND. The coordinator approved--no-parentfor the owned QA worktree;C:/orca/orcawas not its parent.Checklist