Skip to content

fix(hosts): preserve complete status changes in the renderer - #29033

Merged
brennanb2025 merged 3 commits into
mainfrom
brennanb2025/wslchat-host-status-dedupe
Oct 20, 2026
Merged

brennanb2025 merged 3 commits into
mainfrom
brennanb2025/wslchat-host-status-dedupe

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​96 $\color{#cf222e}{\Huge{\mathbf{−}}}$​23 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​73
Prod 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​36 $\color{#cf222e}{\Huge{\mathbf{−}}}$​35 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1

ELI5

An already-connected WSL host could defer a server update, but Settings → WSL hosts hid the reason and Update/Force actions until a renderer reload. The renderer discarded the new status because it compared only the connection state, detail and distro name.

What Changed

The renderer now compares complete WSL and SSH status snapshots using the existing structural comparator. Changes to update details reach the existing host row immediately, and adding status fields cannot silently exclude them from comparison. Runtime environment status already uses this mechanism and needs no change.

Why

Main publishes full WSL snapshots without deduplicating them, so the renderer still needs to suppress repeated equivalent snapshots. Reusing the comparator already used for runtime environments removes the incomplete field lists instead of adding one more field to them. This follows the common pattern of consuming complete published state; the existing desktop event transport is an intended difference because it preserves Orca's host ownership and needs no protocol change.

Linked Issue

Follow-up to #27934 (live Windows QA finding F-1).

Ownership: Neil (@nwparker) owns the WSL host UI; this touches his src/renderer/src/store/slices/wsl-hosts.ts slice and adds wsl-hosts.test.ts. No mover or main-process WSL modules change.

Visual Proof

Windows recheck handed to the coordinator for the next WSL host run: keep Settings open on a ready host, defer a server update while work is live, and verify its reason and Update/Force actions appear without reload and clear after the update. No layout or copy changes.

Testing

  • I manually tested these changes locally
  • Automated regression added: a deferral-only status change reaches the store for an already-ready WSL host.
  • Static analysis/typecheck, all five unit shards, relay integration, and Windows packaging passed on 8d4269f6461573de47550a570b738b5a15b84274 in PR Checks run 38693703882.
  • The full .github/workflows/pr.yml workflow, including Linux packaging and the aggregate PR checks passed gate, passed on attempt 2 for that exact head.
  • CI artifacts explicitly record wsl-hosts.test.ts as passed in shard 5, and the existing SSH store tests as passed in shard 1; no checks ran on the memory-constrained Mac.
  • The first Linux package attempt hit an unrelated frame-capture timeout in browser-popup-native-compatibility.electron.test.ts → preserves native Shift POST context, title and policy ordering: local (failed job and log). One failed-job retry passed the full Linux package job without code changes.
  • Windows UI recheck is pending with the coordinator.

Review

Compared WSL, SSH and runtime environment status storage. SSH had the same field-list comparison and now uses the same full-value comparator; runtime status retains its existing exclusion of the unread probe timestamp. No wire format, host execution, path or workspace ownership changes.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@brennanb2025

brennanb2025 commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor Author

Review status: done. Review, Windows QA and CI are complete on the final head

The problem: in Settings → WSL hosts, a host that was already connected could defer a server update (because a terminal was still running), but its row didn't show the "Update to … deferred." line or the Update / Force update buttons until a reload. After the update went through, the row could keep showing the stale deferral. Cause: the app compared WSL host status using a hand-picked list of fields that left out the deferred update, so a status change that only touched that field was thrown away. SSH host status used the same kind of hand-picked list.

What changes for the user: the WSL host row now shows a deferred update as soon as it happens and clears it as soon as the update goes through, with no reload. The app now compares the whole status, so a field added later can't be dropped the same way.

Review: one round at 8d4269f6461. A fresh correctness reviewer and a fresh architecture reviewer found no confirmed problems and recommended keeping the design. The SSH part fixes no current SSH bug, because the old SSH list already covered every field; it prevents the same drift in future. No fixes were needed.

Final head d2a4c4af8a70: the reviewed 8d4269f6461 plus one merge of current main.

  • The merge had no conflicts, and main had not changed any of this PR's three files.
  • The repo's pre-commit formatter rewrapped lines in 11 files that came in from main, mostly in src/main/native-chat/. I checked each one: the changes are whitespace and line breaks only, with no code changes.

Review path: the light review path. The change is 2 production files and 24 lines, renderer-only, with no saved data, wire, process or lifecycle change. The only commits on these files in the last 30 days are the original features, not earlier fixes for this symptom.

CI: the full PR workflow passed on d2a4c4af8a70, including the PR checks passed gate (run 39210112956).

Windows QA: run on the final head on Windows high spec, with a throwaway imported WSL distro and isolated homes. Codex drove the app through the debugging protocol only; details are in the QA comment.

  1. The WSL host was connected and ready on server build N.
  2. One terminal was left open in a workspace on that host.
  3. Requesting build N+1 showed "Update to … deferred." with Update and Force update right away, with no reload. The status stayed "ready" and only the deferred update changed, which is the exact case that was broken.
  4. Force update cleared the row in about 4 seconds; the host stayed ready and the terminal stayed open.
  5. Build N+2 deferred again. Closing the terminal and clicking Update once cleared it in about 4 seconds.
  6. The same steps on the PR's base (current main's code) reproduced the bug: the row showed no deferral until a reload, and after Force update it kept the stale line and buttons.
  7. The real agent configs on the box (Claude and Codex hook files) were unchanged. The two other config files that changed were written by other sessions on the box, not by the test copy.

Not verified: SSH host rows (no live SSH check; the existing SSH tests pass in CI).

For Brennan's review

Branch, right after a deferral and then after clearing it:

29033-02-deferred.png
29033-03-cleared.png
29033-05-cleared-by-update.png

Main, with the same steps: no deferral shown after connecting, then a stale deferral still showing after Force update:

main-29033-02-after-connect.png
main-29033-06-after-force-renavigate.png

Places where your judgment matters:

  • wsl-hosts.ts compares the whole status instead of a field list. It reuses the comparison runtime environment status already uses.
  • QA found a copy mismatch that this PR doesn't change. The row says terminals are "still running. Close them and try again", but Force update doesn't stop them. The row also shows no progress during the ~16 s connect. Both are outside this PR's scope.

Update (10-19): main moved again and conflicted on one test file this PR doesn't own (it was only in the diff because the pre-commit formatter had rewrapped it). Final head 5624698 merges main again and takes main's version of that file; no code in the PR's own files changed since QA. The formatter rewrapped 6 more of main's files on this merge (formatting only). Full PR checks green on 5624698.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 19, 2026 06:28
@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 11 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2c41e1e0-f3e4-4285-b51c-1a65e88b875a
📥 Commits

Reviewing files that changed from the base of the PR and between d2a4c4a and 5624698.

📒 Files selected for processing: 9
  • src/main/agent-hooks/server/server-status-application.ts
  • src/main/agent-hooks/server/server-status-update.ts
  • src/main/daemon/pty-subprocess-spawn-file-foreground.test.ts
  • src/main/daemon/terminal-host-process-inspection.test.ts
  • src/main/providers/local-pty-child-process-verdict.test.ts
  • src/main/providers/local-pty-provider-foreground-process.test.ts
  • src/renderer/src/store/slices/ssh-connection-state-equality.ts
  • src/renderer/src/store/slices/wsl-hosts.test.ts
  • src/renderer/src/store/slices/wsl-hosts.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8c7e85fc-dfef-419c-bb8c-90fb16935ef3
📥 Commits

Reviewing files that changed from the base of the PR and between 75b6fa2 and d2a4c4a.

📒 Files selected for processing: 14
  • src/main/grok/grok-transcript-import.ts
  • src/main/native-chat/agent-session-journal/adopted-transcript-source.ts
  • src/main/native-chat/agent-session-journal/journal-legacy-import.ts
  • src/main/native-chat/codex-adoption-preparation.ts
  • src/main/native-chat/session-file-resolver.ts
  • src/main/native-chat/structured-agent-cli-conversations.ts
  • src/main/native-chat/transcript-opencode-sqlite-query.test.ts
  • src/main/omp/omp-session-cwd-lookup.test.ts
  • src/main/runtime/rpc/methods/structured-agent-session-adoption-replay.test.ts
  • src/renderer/src/store/slices/ssh-connection-state-equality.ts
  • src/renderer/src/store/slices/wsl-hosts.test.ts
  • src/renderer/src/store/slices/wsl-hosts.ts
  • src/shared/structured-agent-session-create.test.ts
  • src/shared/structured-agent-session-mutation.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.

📝 Walkthrough

The changes update SSH connection and WSL host equality checks to use persistedUIValuesEqual. A WSL host test covers applying a deferred-update status to an already-ready host. Other edits reformat transcript and session calls, types, imports, test fixtures, and assertions without changing their described behavior.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to d2a4c

The renderer can reflect deferred WSL status changes without reloading, and SSH state uses structural equality. The supplied change evidence establishes no concrete user-impacting regression.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 14 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the main change: preserving complete host status changes in the renderer.
Description check Passed The description includes all required sections and clearly explains the WSL and SSH status comparison changes, rationale, testing, and scope. Visual proof is not attached, and the Windows UI recheck r…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description includes all required sections and clearly explains the WSL and SSH status comparison changes, rationale, testing, and scope. Visual proof is not attached, and the Windows UI recheck remains pending, but these omissions do not make the description incomplete.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@OrcaWin

OrcaWin commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

QA on Windows: WSL host row shows and clears "Update … deferred" without a reload (PR head d2a4c4a)

Result: pass. On this branch the Settings → WSL hosts row picked up a deferral-only status change both ways (deferral appears, deferral clears) with no reload. On the PR's base commit (current origin/main has the same two files) the same steps reproduce the original bug.

Problem being checked

When an already-ready WSL host's status changed only in deferredUpdate (a server update held back because a terminal is running), the renderer dropped the update because it compared only state/detail/distro. The row then failed to show "Update to deferred." with Update / Force update, or kept showing it after the update had cleared, until a renderer reload. The PR compares the complete status snapshot instead (src/renderer/src/store/slices/wsl-hosts.ts).

Setup

  • Windows 11 (10.0.26200), WSL 2. The test distro OrcaQa29033 was imported from a slim, read-only tar of an Ubuntu 24.04 distro and unregistered at the end.
  • The orcad template was built inside the test distro from a depth-1 fetch of d2a4c4a (build:orcad-prebuilds, --require-slots linux-x64-glibc, build:orcad, build-orcad-template.mjs --targets linux-x64-glibc; node-pty compiled from source). Template N served orcad 0.1.0+9e35a8537fde. N+1, N+2 (and N+3 for the comparison) were made the same way as nextTemplate() in wsl-orcad-real-distro-lane.test.ts: append a comment to orcad.js and recompute its commonSha256. The app was launched with ORCA_ORCAD_TEMPLATE_PATH pointing at a directory junction, which was repointed between connects.
  • Desktop: pnpm build:desktop (only the expected mobile-web step failed), run as a hidden electron-vite preview with an isolated profile and homes, ORCA_BACKGROUND_LAUNCH=1 and ORCA_INTERNAL_WSL_HOSTS=1. The window was never shown or focused.
  • QA driver: Codex (codex-cli 0.159.2) under a separate QA CODEX_HOME, driving the hidden renderer only through CDP. The project was added with the test copy's orca repo add --path … --environment wsl:orcaqa29033; nothing clicked Browse folder.

Steps on this branch (expected vs observed)

  1. Turn on "WSL distros as hosts" in Settings → Experimental, add OrcaQa29033, wait for ready. Expected: row ready on template N. Observed: ready (deferredUpdate: null), guest running orcad-0.1.0+9e35a8537fde.
    29033-01-ready.png
  2. Open a workspace for /home/neil/qa29033-repo on the host and keep one terminal open. Observed: uname -a showed the WSL2 kernel; the terminal stayed open.
  3. Switch to template N+1. With Settings → WSL hosts open, call window.api.runtimeEnvironments.wslHosts.connect({environmentId:'wsl:orcaqa29033', force:false}) over CDP. Expected: "Update to deferred." with Update and Force update, no reload. Observed: connect took 16 s. The row then showed "Update to 0.1.0+b9e8bf865467 deferred. Not done: terminals on this server are still running. Close them and try again." with Update and Force update, without a reload. The renderer received a ready→ready event that differed only in deferredUpdate (code orcad_update_terminals_running, forceable).
    29033-02-deferred.png
  4. Click Force update in the row. Expected: row stays ready; the deferral line and both buttons disappear without a reload. Observed: cleared in about 4 s; status ready with deferredUpdate: null; guest now runs orcad-0.1.0+b9e8bf865467. The terminal stayed alive, because the daemon is preserved.
    29033-03-cleared.png
  5. Switch to template N+2 and connect again the same way. Expected: deferral shown again. Observed: "Update to 0.1.0+f693273c00a7 deferred. …" with Update / Force update, no reload. (This screenshot is dark because the app follows the dark Windows theme. The other screenshots are light only because the CDP attach forces a light colour scheme.)
    29033-04-deferred-again.png
  6. Close the terminal tab, return to Settings → WSL hosts, click Update (not Force). Expected: row stays ready and the deferral clears without a reload. Observed: one click cleared it in about 4 s; status ready with deferredUpdate: null; guest runs orcad-0.1.0+f693273c00a7.
    29033-05-cleared-by-update.png

Same steps on the base commit 75b6fa2 (old behavior, for comparison)

Fresh profile, same distro, one live terminal, template switched to N+3, same connect() call:

  • After connect, getStatuses() reported deferredUpdate for 0.1.0+c1b97e7c0bdb (orcad_update_terminals_running), but the row showed no deferral line and no Update / Force buttons for the full 60 s. Leaving Settings and coming back without a reload made no difference.
    main-29033-02-after-connect.png
  • The line and buttons appeared only after a renderer reload.
    main-29033-04-after-reload.png
  • After clicking Force update, getStatuses() went back to deferredUpdate: null, but the row kept showing the stale "Update to … deferred." line with both buttons for 60 s and after leaving and reopening Settings.
    main-29033-06-after-force-renavigate.png

Things a user might not expect (outside this diff)

  • The deferral text contradicts the server's reason and what Force does. The row says "Not done: terminals on this server are still running. Close them and try again." The server's own reason says "The restart would not kill them — the daemon is preserved — but the host would run orcad against a daemon forked from until they all exit." Force update did keep the terminal alive. A user reading only the row would likely assume Force update kills their terminals, and the row never shows the fuller reason.
  • No progress indicator while a connect is running from outside the row. connect() took about 16 s each time. When started outside the row's own buttons, the row looks unchanged until the result arrives. Clicking Update / Force only disables the buttons; the row never shows a "connecting" state.

Config files (sha256 before → after)

  • Source distro ~/.claude/settings.json, ~/.claude.json, ~/.codex/hooks.json, ~/.codex/config.toml: unchanged.
  • Windows .claude\settings.json, .codex\hooks.json: unchanged.
  • Windows .claude.json: changed. It is rewritten continuously by the other agent sessions on this machine that use the real Windows home; no test copy or driver used it.
  • Windows .codex\config.toml: changed at 00:43:37. Another session's interactive agent CLI started at 00:43:38 and added a project trust entry for a different worktree. The QA driver used its own CODEX_HOME and ran codex exec.

Cleanup

All test copies and helper processes were stopped by their recorded PIDs. OrcaQa29033 was unregistered; the distro list matches the original.

…host-status-dedupe

# Conflicts:
#	src/main/runtime/rpc/methods/structured-agent-session-adoption-replay.test.ts
@brennanb2025
brennanb2025 merged commit 907694d into main Oct 20, 2026
40 checks passed
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