fix: restore the active top-level view across renderer reload#8265
Conversation
📝 WalkthroughWalkthroughPersisted UI state now includes the active top-level view, defaulting to 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/e2e/active-view-restart-restore.spec.ts (1)
101-109: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard each cleanup step in the
finallyblock to prevent leaked resources.If
session.close(secondApp)throws,firstAppcleanup andsession.dispose()are skipped, potentially leaving Electron processes and temp directories lingering in CI. Wrapping each close in its own try/catch ensures all cleanup steps run.♻️ Proposed refactor
} finally { - if (secondApp) { - await session.close(secondApp) - } - if (firstApp) { - await session.close(firstApp) - } + for (const app of [secondApp, firstApp]) { + if (app) { + try { + await session.close(app) + } catch (error) { + console.error('Failed to close Electron app during cleanup:', error) + } + } + } await session.dispose() }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 964f2d1e-0f30-425b-8b09-8af5b56cb731
📒 Files selected for processing (7)
src/renderer/src/App.tsxsrc/renderer/src/lib/startup-ui-hydration.tssrc/renderer/src/store/slices/ui.test.tssrc/renderer/src/store/slices/ui.tssrc/shared/constants.tssrc/shared/types.tstests/e2e/active-view-restart-restore.spec.ts
b90b2ff to
2245593
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/lib/startup-ui-hydration.test.ts (1)
36-48: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd
activeViewcoverage to the fallback assertion. The fallback test only checks width/group/sort and sleeping-workspace flags, so the newactiveView: 'terminal'default isn’t exercised yet.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: eb5e3f58-e74e-49b2-8660-e8d6537b91ba
📒 Files selected for processing (10)
src/renderer/src/App.tsxsrc/renderer/src/components/automations/AutomationsPage.tsxsrc/renderer/src/hooks/useIpcEvents.tssrc/renderer/src/lib/startup-ui-hydration.test.tssrc/renderer/src/lib/startup-ui-hydration.tssrc/renderer/src/store/slices/ui.test.tssrc/renderer/src/store/slices/ui.tssrc/shared/constants.tssrc/shared/types.tstests/e2e/active-view-restart-restore.spec.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/e2e/active-view-restart-restore.spec.ts
- src/shared/constants.ts
- src/shared/types.ts
- src/renderer/src/store/slices/ui.ts
- src/renderer/src/store/slices/ui.test.ts
2245593 to
f8c9f38
Compare
|
Thanks for the PR! |
Summary / description
The active top-level view (Tasks, Automations, Mobile, Settings, Skills, Space, or the enabled Activity view) now survives a renderer reload or app relaunch instead of always resetting to the terminal.
activeViewnow uses Orca's existing persisted UI pipeline. Startup hydration restores a validated value once, while laterui:stateChangedbroadcasts continue syncing preferences without unexpectedly navigating an already-open window. Unknown values and a disabled experimental Activity view safely fall back to the terminal.Closes #8264
Evidence of fix
activeViewreached persisted main-process state, relaunches, and verifies the Tasks DOM is visible while the terminal DOM is hidden.main:1 passed (8.9s); the restore scenario itself completed in8.2s.151 passedacrossui.test.tsandstartup-ui-hydration.test.ts.ELI5
Orca remembered the details inside each screen, but it forgot which screen you were looking at. This change saves that screen name too. When Orca starts again, it checks that the saved name is still valid and opens that screen; background settings updates are not allowed to pull you away from the screen you are currently using.
Trade-offs / expected user regressions
No user regression is expected. The value uses the existing profile-level UI state, so the last desktop view written is the one restored on the next launch; a live window is never navigated by a cross-window/mobile sync. Like the surrounding UI preferences, writes are debounced by 150 ms, so a reload triggered almost immediately after navigation could still restore the prior view.
Screenshots
No visual change. The E2E test provides render-layer evidence for both the restored Tasks page and hidden terminal surface.
Testing
pnpm lint(focused Oxlint and the max-lines ratchet passed; the full repository lint suite was not run)pnpm typecheckpnpm test(focused suites passed: 151 tests)pnpm build(the E2E-mode Electron build passed; the full release build was not run)Commands run:
AI Review Report
Final review checked startup ordering, legacy/corrupt values, experimental-view gating, startup failure fallback, cross-window/mobile rebroadcast behavior, the debounced writer, and the full restart/render path. It identified that the contributor branch was behind
main; the branch was merged with currentmain, then all focused checks were repeated successfully.Cross-platform review covered macOS, Linux, and Windows. This change adds no shortcuts, shortcut labels, shell commands, platform-specific paths, or OS-specific Electron behavior. It uses the same typed IPC and persistence path on every platform. The SSH/remote use case is preserved because the restored view is app UI state and does not assume a local repository or filesystem path.
Security Audit
The persisted value is treated as untrusted: unknown, missing, legacy, or unavailable view values fall back to
terminal. The change adds no command execution, path handling, authentication, secrets, dependencies, or new IPC channel. It only extends the existing trusted partial UI-state payload with a validated navigation value. No security follow-up is needed.Notes
The original contributor PR was selected because it is the only linked candidate and already covered the important persistence and synchronization edge cases. It has been updated to current
mainand retained as the final PR so the contributor keeps authorship and discussion history.