Repository navigation
Retry failed catalog reads when SSH hosts move to Remote - #26531
Conversation
fab5d3b to
515aabf
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughManaged-server catalog fetches can now propagate errors to the load tracker. Loads are tracked by identity and checked during catalog and worktree refreshes. Tests cover retrying failed catalog requests after reconnect, stopping stale loads after a relay transition, and recovering from a failed catalog refresh during conversion. Priority: ⬇️ Low Merge Risk: 🔵 Low · up to A catalog response started before a host returns to relay may leave obsolete repository rows visible. The remaining risk is bounded, but the catalog-fencing fix should be made or explicitly accepted before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
2050493e-85dc-4966-826f-5300c0b6b26b
📒 Files selected for processing (7)
src/renderer/src/hooks/ipc-events/ssh-managed-server-converted-catalogs.test.tssrc/renderer/src/hooks/ipc-events/ssh-managed-server-state-effects.tssrc/renderer/src/store/folder-workspaces/folder-workspace-catalog-actions.tssrc/renderer/src/store/project-groups/project-group-catalog-actions.tssrc/renderer/src/store/repos/repo-catalog-actions.tssrc/renderer/src/store/repos/repo-state.tstests/e2e/ssh-orcad-auto-convert.spec.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
| // Starts and wakes keep the same owner; only a new server or a failed load needs a refresh. | ||
| if (catalogLoadByTarget.get(targetId)?.environmentId !== next.environmentId) { | ||
| const load = { environmentId: next.environmentId } | ||
| catalogLoadByTarget.set(targetId, load) | ||
| void loadManagedServerCatalogs(targetId, load).catch((error: unknown) => { | ||
| if (catalogLoadByTarget.get(targetId) === load) { | ||
| catalogLoadByTarget.delete(targetId) | ||
| } | ||
| console.warn('[ssh] Could not load the managed server catalogs:', error) | ||
| ) | ||
| }) | ||
| } | ||
| return | ||
| } | ||
| if (next?.kind === 'relay') { | ||
| loadedEnvironmentByTarget.delete(targetId) | ||
| catalogLoadByTarget.delete(targetId) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Fence catalog commits when the target enters relay.
When a managed-server catalog load is pending, the relay transition deletes catalogLoadByTarget, but it does not invalidate the repository catalog action. A pending fetchRepos can therefore pass its unchanged host-specific generation check and merge the old server's repositories into repos. The later load-identity check only prevents converted-row cleanup and workspace rehoming; it does not undo that repository commit.
Invalidate the pending catalog actions at this transition, or pass the load identity into their commit guards so obsolete results cannot commit after the target enters relay.
2 participants
ELI5
Moving a machine to Remote updates Orca's address book. If reading that address book fails once, Orca currently remembers it as successfully loaded. Reconnecting then leaves the machine labeled with an opaque ID. Let failed reads retry when the machine reconnects.
What Changed
Clear a failed conversion-catalog load from the per-host cache. Keep successful and in-progress loads deduplicated, and ignore an old load when that host returns to the relay or changes servers. Conversion requests now ask the existing repository, project-group and folder-workspace loaders to propagate errors, so the renderer removes its old workspace rows only after those catalog reads succeed. Ordinary refresh callers retain their existing error handling.
Why
Retrying the existing conversion refresh fixes the stale host list without adding another poller or requiring an app restart. Keeping each load's identity also prevents a late failure from invalidating a newer load. This changes renderer coordination only; no remote wire fields, Git commands, persisted migration state or process-liveness decisions change.
Linked Issue
Follow-up to #26483, based on
OrcaWin/phase35at544fffe9214.Visual Proof
Same journey against a real Linux SSH container, with the local environment-list IPC made temporarily unavailable during conversion, then restored before reconnect. The SSH server, migration and subsequent catalog reads are real. This is controlled fault injection, not a claim that a physical network outage was reproduced.
Before: reconnect leaves the managed host labeled
4f49f65…/Orca server.After: reconnect restores the machine's name and SSH connection detail.
The folder shown under Local Mac is a separate finding recorded in the local consolidation tracker and being investigated separately.
Testing
macOS arm64 client, hidden Electron renderer, isolated profiles, real Debian arm64 OpenSSH container, packaged Phase 3.5 Linux server template. Every launch used
ORCA_BACKGROUND_LAUNCH=1.pnpm tc,pnpm lint, changed-code quality againstorigin/OrcaWin/phase35, andelectron-vite build --mode e2epass.typecheck:e2ereports existing errors in unrelated fixtures; no new-test errors were reported. Windows client/host execution was not run locally.Reproduction command after providing a Linux orcad template:
ORCA_BACKGROUND_LAUNCH=1 SKIP_BUILD=1 ORCA_E2E_SSH_DOCKER=1 \ ORCA_E2E_ORCAD_CONVERT_HOST=docker \ ORCA_E2E_ORCAD_CONVERT_TEMPLATE=/absolute/path/to/orcad-template \ pnpm run test:e2e tests/e2e/ssh-orcad-auto-convert.spec.ts \ --workers=1 -g 'reconnect restores the managed host name'The regression is inside the existing conversion spec, so the dedicated Linux conversion CI job runs it.
Unfixed failure log · All six fixed real-host tests
AI Disclosure
Review
Self-reviewed error propagation, per-host isolation, cancellation and unchanged ordinary refresh behavior.
Agent skill upstream boundary
Notes
No disk data is deleted by this change. Existing migration retention and rollback remain host-owned. Folder workspaces are explicitly covered. No provider-specific or OS-specific production code was added.
Checklist