Skip to content

fix(sidebar): preserve remote host visibility across stale UI updates - #25737

Merged
nwparker merged 1 commit into
mainfrom
nwparker/m4air-not-showing-up-on-left-sidebar
Oct 6, 2026
Merged

nwparker merged 1 commit into
mainfrom
nwparker/m4air-not-showing-up-on-left-sidebar

Conversation

@nwparker

@nwparker nwparker commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

ELI5

When you reveal a remote computer in the sidebar, its workspaces should stay visible while Orca saves your settings. This fixes a reproducible race where an older settings update could undo that selection.

What Changed

Previously, host visibility saved directly, bypassing the protection used by other sidebar filters. A delayed saved-settings broadcast could restore the previous host selection and remove the newly revealed remote's cards.

Both workspaceHostScope and visibleWorkspaceHostIds now use the existing guarded settings writer. The sidebar still changes immediately; persistence uses its existing 150 ms debounce. Unsaved edits and edits made during an outstanding save survive incoming snapshots, and the final selection is saved after that write completes. Host lists compare by contents, with null retaining its existing meaning of All hosts.

Added coverage for delayed broadcasts, toggling back during an outstanding save, unchanged arrays with fresh identities, and accepting another client's host selection when there is no local edit.

Why

The user's m4air report prompted this investigation. The connected remote had 492 Orca worktrees when inspected. With the current saved filters, four were eligible; ordinary sleeping/CLI filter roundtrips returned to those same four. That did not reproduce the earlier three-card incident.

Controlled tests did reproduce a host-selection rollback. Both a store test and the real settings-writer integration test failed before the fix because the remote host disappeared from the selected list. Reusing the existing save guards fixes the demonstrated race without adding a worktree refresh, a parallel filter cache, or another persistence mechanism.

Linked Issue

User-reported m4air sidebar issue. Related existing report: #15996, which concerns a transient activity gap under Hide sleeping rather than a host-selection rollback. This PR does not claim to close that issue. Prior fixes and open PRs are cross-reviewed below; no exact duplicate of this host-settings fix was found.

Visual Proof

Controlled replay in a separate, hidden Orca instance on macOS, using renderer fixtures with three local cards and four m4air cards. Select Workspace options → Hosts → All hosts, then deliver the previous saved settings through the same sync hydration path used by incoming settings broadcasts. These are reproduction screenshots, not captures of the original live incident.

Before: the old snapshot restores local-only visibility; only three cards remain.

Before: remote cards disappear

After: all seven cards, including m4air, remain visible.

After: remote cards remain

Testing

  • I manually tested these changes locally
  • Automated tests added/updated

On macOS, inspected the running instance and m4air catalog through Orca CLI, then validated the controlled before/after replay with Playwright CDP in a separate background instance. The user's app was not restarted or focused.

Passed:

  • ORCA_BACKGROUND_LAUNCH=1 pnpm test src/renderer/src/store/slices/ui-host-filter-sync.test.ts src/renderer/src/store/slices/persisted-ui-write-baseline.test.ts src/renderer/src/store/slices/ui-hydration-view-layout.test.ts src/renderer/src/app-shell/workspace-view-cross-client-sync.test.tsx — 85 tests.
  • pnpm tc:web.
  • Oxlint on all seven changed files; changed-code quality gate; formatting and diff whitespace checks.
  • Electron Vite main/preload/renderer bundle build and renderer boot-graph check.

pnpm tc is blocked by an existing error in the unchanged src/main/native-chat/agent-session-journal/journal-submission-positions.test.ts:29: missing codexProviderHandle. Full repository lint/test/release packaging were not run.

Review

Self-reviewed the save/hydration interaction, the outstanding-save flip-back case, and all host-selection callers. The writer is mounted in the app shell; existing host normalization and sidebar membership rules are unchanged.

Cross-review of existing reports and fixes

Searched existing issues and open/closed/merged PRs for m4air, host visibility, sidebar filtering, and settings resets; inspected the relevant patches and current source.

Existing work Status Relationship to this fix
#17057 — cross-client preference resets (STA-5781) Merged Introduced the guarded settings writer used here. Its field list omitted both host-visibility fields; this PR extends that protection rather than introducing another writer.
#19032 — remote host disappears until its filter is unchecked/rechecked Merged Closest reported symptom, different cause: a missed pointer release promoted a phantom header drag. Its release guard is present in current source and its three regression tests pass. This PR does not change drag behavior.
#15996 / #16001 — Hide sleeping drops a remote row during a PTY gap Open Delays removal for 1.5 seconds after activity disappears. Does not protect host selections from settings broadcasts. Can coexist with this fix after the integration repair described below.
#13944 / #24363, and #19643 / #19642 — project groups under the wrong host Open Change folder/group placement, with no changed files shared with this PR. Those two PRs overlap with each other; #24363 keeps unstamped groups buffered across host runs, while #19642 assigns them to the default host. Their policy difference requires reconciliation if combined.
#7353 — keep unread sleeping workspaces visible Open Changes which workspaces qualify under Hide sleeping. Its current patch preserves structured-chat activity. Does not change host-settings persistence; the patch applies cleanly alongside this PR.
#18104 / #18417 — remote rows omitted by the CLI limit Closed / merged A CLI pagination problem, not the renderer sidebar. The live inspection used a host/repo-scoped query and the renderer's complete remote catalog.

Integration finding: #16001 needs adaptation before combining with current code. At reviewed head 8d6c16cee93e2c61e09110c60a1eec321012f63d, its hook and tests import hasActiveWorkspaceActivity, but that function is now private in the current activity module. Its visibility-options hunk also needs rebasing. After applying its patch and adapting only that hunk, nine tests failed with hasActiveWorkspaceActivity is not a function (110 passed). Restoring the export temporarily made all 119 tests pass. A rebase must restore an intentional public activity API or use the existing public predicate. This finding concerns integration with current code, not a claim that its original branch fails.

Validation: 104 tests passed on this PR alone across host dragging, host sections, visibility/activity, host sync, and the real settings writer. The temporary combined run added #16001's retention/state-machine/StrictMode tests: 119 passed after the compatibility adaptations above. All temporary code was restored; no changes from other PRs are included here. The open placement and unread PRs were reviewed statically, not fully tested on their own branches. No additional correctness finding remains in this PR from the cross-review; the original incident's cause is still unconfirmed.

Origin and process follow-up

The vulnerable host-selection path was introduced by #5071, Jinwoo Hong (@Jinwoo-H), who is recorded as both author and merger (June 14, 2026 UTC). The merged commit is 36277801e4b2b50867c6a2a09b453a974abd209d. Its actual patch adds both host fields, their direct window.api.ui.set calls, and unconditional hydration of those fields. GitHub blame independently attributes the setters, lines 1820–1853 to that same commit. This is the origin of the demonstrated vulnerable mechanism; it does not conclusively identify the cause of the user's original live incident.

#17057, also authored and merged by @Jinwoo-H (August 28, 2026 UTC), subsequently fixed the same class of stale-settings reset for other preferences. Its new guarded baseline omitted workspaceHostScope and visibleWorkspaceHostIds, leaving their older path unprotected. This was an incomplete coverage boundary in a fix, rather than evidence that #17057 newly broke working host behavior.

Why tests passed: the baseline's compile-time census and test fixture both enumerate only fields already declared writer-owned. Leaving a sidebar preference out of that type and fixture makes both checks pass. Existing host-action tests checked the state update and direct save call, without an intervening stale broadcast or an outstanding-save flip-back.

Recorded review context: #5071 reports 502 changed files and has no GitHub review entries. #17057 records CodeRabbit and author comments, with no independent human approval recorded. Its body describes additional agent reviews; GitHub cannot establish whether offline human review occurred.

Recommended process changes:

  1. Give every durable sidebar preference an explicit write policy, checked against the canonical preference contract, so leaving a control out of the guarded writer is visible to review and automation.
  2. Require real-writer tests for each control: delayed unrelated broadcast, change-back during an outstanding save, and another client's edit with no local dirty state. Include All hosts and multi-host selection.
  3. Split large persistence/host changes into reviewable units and have a second maintainer review the complete control → save → broadcast → hydration path, including controls left outside the diff.

These are follow-up recommendations; this PR includes the host regression tests and the narrow fix, not a new repository-wide policy system.

Agent skill upstream boundary

  • Not applicable; no upstream skill source or resources copied.

Notes

This fixes a demonstrated race consistent with the report; the precise historical m4air event remains unconfirmed. Persistence gains the same small debounce as other filters. No RPC fields or remote execution behavior change, so independently updated clients and hosts keep the existing wire contract. SSH/runtime host IDs and folder workspaces retain their existing filtering rules. No new polling, subprocesses, credentials, paths, shortcuts, or platform-specific behavior. Native Linux/Windows and mobile were not exercised.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • Before/after screenshots attached
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered
  • Full pnpm lint, typecheck, test, and build pass — focused results and the existing typecheck failure are documented above; CI covers the remaining repository checks.

@nwparker
nwparker merged commit 4ccfc16 into main Oct 6, 2026
12 of 13 checks passed
8bitbish pushed a commit to 8bitbish/orca that referenced this pull request Oct 7, 2026
@nolouch

nolouch commented Oct 7, 2026

Copy link
Copy Markdown

Thank you for your work, it's very helpful.

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.

2 participants