Repository navigation
fix(sidebar): keep remote-runtime agent sessions visible while parked - #15443
PannenetsF wants to merge 4 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 8043b5f and b0fd15943ccf99bac1bf4355ae16a586f20cebcf. 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughParked remote-runtime PTY watchers now register and retain renderer-owned agent-status claims across pane lifecycle changes. They route valid agent-status updates, mark renderer-owned writes, and store status payloads. Watcher disposal releases claims. PTY disposal retains claims for remote-runtime PTYs and releases them for local and SSH PTYs. Actual PTY exit releases retained claims. Tests cover routing, claim persistence, disposal, and non-remote PTYs. Merge Risk: ⚪ Minimal · up to This localized change keeps remote-runtime agent sessions visible while parked and includes targeted regression coverage; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/renderer/src/components/terminal-pane/parked-terminal-renderer-owned-agent-status.test.ts (1)
49-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReduce the test rationale comments.
Lines 49-51 and Lines 75-79 use multi-line implementation explanations. Condense each comment to one brief, non-obvious rationale.
As per coding guidelines, “Comments must be concise, non-obvious, and brief—prefer one line; do not explain obvious behavior or walk through code.”
Also applies to: 75-79
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d66499a1-b705-446f-b978-0572dca4a65e
📥 Commits
Reviewing files that changed from the base of the PR and between 953fc2d0d311a37aa7bd580332047afd54c71335 and e42700ee770d39e7e7e7147559b0b09ed60efc09.
📒 Files selected for processing (1)
src/renderer/src/components/terminal-pane/parked-terminal-renderer-owned-agent-status.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
9f78224 to
b0fd159
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
For remote-runtime workspaces the renderer owns agent status (it parses OSC 9999 directly from PTY bytes; main never sees the stream). When the tab parked, the pane dispose released the renderer-owned claim and the parked watcher did not ingest agent-status facts. The next mirrored snapshot then deleted the entry because the host surface carries no agentStatus for remote-runtime surfaces, so the session vanished from the sidebar until the user refocused the workspace. - Skip releasing the renderer-owned claim on pane dispose for remote-runtime PTYs; the parked watcher re-registers it (preserving hasClientWrite) and releases it on PTY exit. - Have the parked watcher process agent-status facts for remote-runtime PTYs so the store row stays fresh while parked.
Regression coverage for stablyai#15442: the parked watcher registers the renderer-owned agent-status claim for remote-runtime PTYs, proves it on each routed agent-status fact so the mirrored host snapshot can't delete the row, leaves the claim unproven when routing rejects the fact, and releases it on dispose. Non-remote parked PTYs register no claim. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per review feedback, collapse two multi-line comment blocks into single brief rationale lines covering only the non-obvious why.
Per review: seed a proven renderer-owned claim before startWatcher so the test asserts hasClientWrite survives the mounted-pane-to-parked-watcher handoff, and dispose without dispatching a fact so a release path gated on fact handling would fail.
b0fd159 to
196edbe
Compare
|
Adding Windows/paired-headless coverage relevant to this PR: Windows client 1.4.224, native Linux headless servers 1.4.224, with OMP CLI 18.10.4 installed. The servers are registered as Remote Orca Servers; the WSL server uses a direct loopback WebSocket connection, and the VPS uses an SSH tunnel. They are not ordinary Windows-owned SSH PTYs. In normal use, an OMP row can disappear until its project/workspace is selected, matching #15442. I have not captured a deterministic park/reveal reproduction or inspected the live client-owned claim transition, and I have not tested this PR branch. There is a separate verified host-side problem: an isolated fresh OMP turn loses accepted hook state after a pane split, while later tool/turn completion posts receive HTTP 204 without restoring that state (#24436/#26933; proposed fixes #24732/#24196). A terminal-derived row can survive that loss temporarily. Please validate both properties independently: park/reveal must retain the client row, and a repaint must not invalidate the host's managed-agent authority. Keeping a client claim alive cannot by itself recover suppressed host hooks. Could you clarify whether this parked-watcher change is still needed alongside the currently shipped hydration fixes, and which client release will include it? Detailed isolated reproduction: comment on #24436. AI assistance disclosure: prepared with an OpenAI coding assistant via OMP, using my reported behavior and local runtime diagnostics/source inspection. Reproduction and validation limits are stated above. |
This branch has not been deployed
2 participants
ELI5
When you park a workspace that runs an agent on a remote machine, Orca used to lose track of whether that agent was still working, so the session disappeared from the sidebar until you clicked back into it. This keeps it visible.
What Changed
For remote-runtime workspaces the renderer owns agent status (it parses OSC 9999 directly from PTY bytes; main never sees the stream). When the tab parked, the pane dispose released the renderer-owned claim and the parked watcher did not ingest agent-status facts. The next mirrored snapshot then deleted the entry because the host surface carries no
agentStatusfor remote-runtime surfaces, so the session vanished from the sidebar until the user refocused the workspace.disposefor remote-runtime PTYs; the parked watcher re-registers it (preservinghasClientWrite) and releases it on PTY exit.Why
Without this, remote-runtime agent sessions silently disappear from the sidebar whenever their workspace is parked, even though the agent is still running. Ownership must stay with the renderer across the park/reveal handoff because the host never sees these PTY bytes and therefore has no
agentStatusto mirror.Linked Issue
Fixes #15442
Visual Proof
N/A — no visual or layout change. The fix restores an existing sidebar entry that was incorrectly being deleted; there is no new or changed UI surface. Behavior is covered by automated tests (see Testing).
Testing
Added regression coverage in
parked-terminal-byte-watcher.test.ts:hasClientWriteentry).Ran on macOS:
pnpm vitest run src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.test.ts, pluspnpm lintandpnpm typecheckon the changed files.AI Disclosure
N/A — internal contributor.
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Ensure no issues in: Security, Cross-platform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)