[codex] gate offline host workspaces#4672
Conversation
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
Ready to review this PR? Stage has broken it down into 3 individual chapters for you:
Chapters generated by Stage for commit 2ef0f21 on May 17, 2026 5:49pm UTC. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR adds offline host detection and UI display to the v2 workspace layer. It extends ChangesOffline Host Detection and Rendering
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Greptile SummaryThis PR adds an offline-host gate to the v2 workspace detail route. When the synced host row has
Confidence Score: 4/5Safe to merge — the offline gate is correctly inserted before WorkspaceProvider mounts, preventing relay calls to downed hosts, and the new component follows established patterns in the codebase. The core logic is sound: The hook file (useRemoteHostStatus.ts) has the most behavioural surface area — the
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/hooks/useRemoteHostStatus/useRemoteHostStatus.ts | Adds "offline" status variant; disables infoQuery when host is offline and also gates it behind isReady, which is a behavioural change worth noting |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx | Adds offline guard rendering WorkspaceHostOfflineState before WorkspaceProvider is mounted; guard ordering is safe since hook returns only one status at a time |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/components/WorkspaceHostOfflineState/WorkspaceHostOfflineState.tsx | New offline-state UI component; clean implementation with host name/ID display and action links to host settings and workspace list |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/components/WorkspaceHostOfflineState/index.ts | Barrel export for WorkspaceHostOfflineState; no issues |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[V2WorkspaceLayout mounts] --> B{workspaceId / isReady / workspaces?}
B -- no --> C[Empty loading div]
B -- yes --> D{workspace row?}
D -- no, creating --> E[WorkspaceCreatingState]
D -- no, error --> F[WorkspaceCreateErrorState]
D -- no --> G[WorkspaceNotFoundState]
D -- yes --> H[useRemoteHostStatus]
H --> I{workspace?}
I -- no --> J[status: loading]
I -- yes --> K{isLocal?}
K -- yes --> L[status: skip]
K -- no --> M{isReady?}
M -- no --> N[status: loading]
M -- yes --> O{hostRow?.isOnline === false?}
O -- yes --> P[status: offline]
O -- no --> Q{infoQuery.isSuccess?}
Q -- yes, version too old --> R[status: incompatible]
Q -- otherwise --> S[status: ready]
P --> T[WorkspaceHostOfflineState]
R --> U[WorkspaceHostIncompatibleState]
S --> V[WorkspaceProvider + Outlet]
J --> W[Empty loading div]
N --> W
L --> V
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/hooks/useRemoteHostStatus/useRemoteHostStatus.ts:69-70
**`isReady` added to `infoQuery.enabled` — intentional but worth documenting**
`isReady` was not previously part of the enabled guard, so this PR changes when the relay request fires: the host-info query now waits for the live query to hydrate before dispatching. This is the correct behavior (we need `hostRow?.isOnline` before deciding whether to hit the network), but it serialises what was a parallel fetch. In practice the delay is negligible since `!isReady` already returns `"loading"`, but it's a subtle side-effect of the offline gate worth a brief comment in the code.
### Issue 2 of 2
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/hooks/useRemoteHostStatus/useRemoteHostStatus.ts:78-84
**Missing host row is not treated as offline**
The offline gate triggers only when `hostRow?.isOnline === false` (strict boolean). If there is no host row at all (`hostRow === null`, e.g. the host was deleted or has not synced yet), `isOnline` is `undefined`, the check is `false`, and the query proceeds. This asymmetry means a workspace whose host row was deleted will attempt a relay connection instead of showing the offline screen, ultimately falling through to `{ status: "ready" }` on query failure. This is pre-existing behaviour rather than a regression introduced here, but the new offline gate handles only half the "host unreachable" space — worth a comment or, at minimum, a `TODO`.
Reviews (1): Last reviewed commit: "gate offline host workspaces" | Re-trigger Greptile
| enabled: | ||
| workspace != null && !isLocal && isReady && hostRow?.isOnline !== false, |
There was a problem hiding this comment.
isReady added to infoQuery.enabled — intentional but worth documenting
isReady was not previously part of the enabled guard, so this PR changes when the relay request fires: the host-info query now waits for the live query to hydrate before dispatching. This is the correct behavior (we need hostRow?.isOnline before deciding whether to hit the network), but it serialises what was a parallel fetch. In practice the delay is negligible since !isReady already returns "loading", but it's a subtle side-effect of the offline gate worth a brief comment in the code.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/hooks/useRemoteHostStatus/useRemoteHostStatus.ts
Line: 69-70
Comment:
**`isReady` added to `infoQuery.enabled` — intentional but worth documenting**
`isReady` was not previously part of the enabled guard, so this PR changes when the relay request fires: the host-info query now waits for the live query to hydrate before dispatching. This is the correct behavior (we need `hostRow?.isOnline` before deciding whether to hit the network), but it serialises what was a parallel fetch. In practice the delay is negligible since `!isReady` already returns `"loading"`, but it's a subtle side-effect of the offline gate worth a brief comment in the code.
How can I resolve this? If you propose a fix, please make it concise.| if (hostRow?.isOnline === false) { | ||
| return { | ||
| status: "offline", | ||
| hostId, | ||
| hostName: hostRow.name, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Missing host row is not treated as offline
The offline gate triggers only when hostRow?.isOnline === false (strict boolean). If there is no host row at all (hostRow === null, e.g. the host was deleted or has not synced yet), isOnline is undefined, the check is false, and the query proceeds. This asymmetry means a workspace whose host row was deleted will attempt a relay connection instead of showing the offline screen, ultimately falling through to { status: "ready" } on query failure. This is pre-existing behaviour rather than a regression introduced here, but the new offline gate handles only half the "host unreachable" space — worth a comment or, at minimum, a TODO.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/hooks/useRemoteHostStatus/useRemoteHostStatus.ts
Line: 78-84
Comment:
**Missing host row is not treated as offline**
The offline gate triggers only when `hostRow?.isOnline === false` (strict boolean). If there is no host row at all (`hostRow === null`, e.g. the host was deleted or has not synced yet), `isOnline` is `undefined`, the check is `false`, and the query proceeds. This asymmetry means a workspace whose host row was deleted will attempt a relay connection instead of showing the offline screen, ultimately falling through to `{ status: "ready" }` on query failure. This is pre-existing behaviour rather than a regression introduced here, but the new offline gate handles only half the "host unreachable" space — worth a comment or, at minimum, a `TODO`.
How can I resolve this? If you propose a fix, please make it concise.
Summary
Why
Remote workspaces already depend on host availability. When a host is offline, the workspace detail route should behave like the existing missing-worktree gate: explain why terminals and file actions are unavailable and avoid downstream host-service queries.
Validation
bunx biome check --write --unsafe apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/hooks/useRemoteHostStatus/useRemoteHostStatus.ts apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/components/WorkspaceHostOfflineState/WorkspaceHostOfflineState.tsx apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/components/WorkspaceHostOfflineState/index.tsbun run --cwd apps/desktop typecheckbun run lint -- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/hooks/useRemoteHostStatus/useRemoteHostStatus.ts apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/components/WorkspaceHostOfflineState/WorkspaceHostOfflineState.tsx apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/components/WorkspaceHostOfflineState/index.tsgit diff --checkSummary by cubic
Gates V2 workspace routes when the host is offline and shows a clear offline state. This avoids unreachable host calls and explains why terminals and file actions are unavailable.
isOnlineand return anofflinestatus; skip remotehost.infoqueries when offline.Written for commit 2ef0f21. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Release Notes