Repository navigation
Wait for editor ownership before listing remote Markdown - #26590
Conversation
acd0044 to
2bf93f3
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📥 CommitsReviewing files that changed from the base of the PR and between ffb449690229dbcf099df7dcc19f64e05744f3d7 and 2e95086. 📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe Markdown document hook now checks restored workspace ownership and runtime availability before requesting document listings. New tests cover ownership resolution for retained editors, SSH repositories, and folder workspaces. A Docker-gated E2E test checks Markdown listing and link navigation after conversion and session retention. CI source routing, job selection, and workflow configuration now select and run this test. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The ownership transitions inspected provide a retry for Markdown listing, and relevant editor changes select the dedicated conversion E2E job. No concrete current-head regression remains to block merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 9 files. (1 skipped: 1 unsupported.)
✨ 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:
3671e1f1-f241-41cb-a63f-b5ea9da96ae9
📒 Files selected for processing (11)
.github/workflows/e2e.ymlconfig/scripts/ci-e2e-job-selection.mjsconfig/scripts/ci-e2e-job-selection.test.mjsconfig/scripts/pr-e2e-docker-lane-contract.test.mjsconfig/scripts/pr-e2e-gate-contract.test.mjsconfig/scripts/pr-e2e-source-routing.mjssrc/renderer/src/components/editor/restored-editor-workspace-runtime-owner.tssrc/renderer/src/components/editor/useMarkdownDocuments.navigation.test.tssrc/renderer/src/components/editor/useMarkdownDocuments.ownership.test.tsxsrc/renderer/src/components/editor/useMarkdownDocuments.tstests/e2e/ssh-orcad-markdown-conversion.spec.ts
💤 Files with no reviewable changes (1)
- config/scripts/pr-e2e-gate-contract.test.mjs
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
| const state = useAppStore.getState() | ||
| const settings = settingsForRuntimeOwner(state.settings, activeFile.runtimeEnvironmentId) | ||
| // The content loader reowns retained tabs before metadata may use their new host. | ||
| if ( | ||
| findRestoredEditorWorkspaceRuntimeOwner( | ||
| state, | ||
| { | ||
| worktreeId, | ||
| filePath: activeFile.filePath, | ||
| externalSshTargetId: activeFile.externalSshTargetId, | ||
| operationProvenance: activeFile.operationProvenance, | ||
| runtimeEnvironmentId: activeFile.runtimeEnvironmentId | ||
| }, | ||
| worktreeId | ||
| ) || | ||
| (connectionId === undefined && !settings?.activeRuntimeEnvironmentId?.trim()) | ||
| ) { | ||
| return | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
rg -nP -C3 'findRestoredEditorWorkspaceRuntimeOwner\s*\(' src/renderer
rg -nP -C5 'function getExplicitRuntimeEnvironmentIdForWorktree' src/rendererRepository: stablyai/orca
Length of output: 3455
🏁 Script executed:
set -u
printf '%s\n' '--- useMarkdownDocuments ---'
nl -ba src/renderer/src/components/editor/useMarkdownDocuments.ts | sed -n '1,180p'
printf '%s\n' '--- restored owner helper ---'
nl -ba src/renderer/src/components/editor/restored-editor-workspace-runtime-owner.ts | sed -n '1,180p'
printf '%s\n' '--- runtime owner resolver ---'
nl -ba src/renderer/src/lib/worktree-runtime-owner.ts | sed -n '1,180p'
printf '%s\n' '--- related hook and tests ---'
rg -n -F --glob '*.{ts,tsx}' -- 'useMarkdownDocuments' src/renderer
rg -n -F --glob '*.{ts,tsx}' -- 'runtimeEnvironmentId:' src/renderer/src/components/editor src/renderer/src | head -120Repository: stablyai/orca
Length of output: 36534
🏁 Script executed:
nl -ba src/renderer/src/components/editor/useMarkdownDocuments.ts | sed -n '1,180p'
nl -ba src/renderer/src/components/editor/restored-editor-workspace-runtime-owner.ts | sed -n '1,180p'
nl -ba src/renderer/src/lib/worktree-runtime-owner.ts | sed -n '1,180p'
rg -n -F --glob '*.{ts,tsx}' -- 'useMarkdownDocuments' src/rendererRepository: stablyai/orca
Length of output: 21385
🏁 Script executed:
set -u
printf '%s\n' '--- remaining useMarkdownDocuments ---'
nl -ba src/renderer/src/components/editor/useMarkdownDocuments.ts | sed -n '178,280p'
printf '%s\n' '--- connection context ---'
rg -n -F --glob '*.ts' -- 'function getConnectionIdFromState' src/renderer/src
rg -n -F --glob '*.ts' -- 'getConnectionIdFromState' src/renderer/src/lib src/renderer/src/store | head -80
printf '%s\n' '--- active runtime setting updates ---'
rg -n -P --glob '*.{ts,tsx}' -C4 'activeRuntimeEnvironmentId\s*:' src/renderer/src | head -240
printf '%s\n' '--- ownership test ---'
nl -ba src/renderer/src/components/editor/useMarkdownDocuments.ownership.test.tsx | sed -n '1,230p'Repository: stablyai/orca
Length of output: 38510
🏁 Script executed:
nl -ba src/renderer/src/components/editor/useMarkdownDocuments.ts | sed -n '178,280p'
rg -n -F --glob '*.ts' -- 'function getConnectionIdFromState' src/renderer/src
rg -n -F --glob '*.ts' -- 'getConnectionIdFromState' src/renderer/src/lib src/renderer/src/store | head -80
rg -n -P --glob '*.{ts,tsx}' -C4 'activeRuntimeEnvironmentId\s*:' src/renderer/src | head -240
nl -ba src/renderer/src/components/editor/useMarkdownDocuments.ownership.test.tsx | sed -n '1,230p'Repository: stablyai/orca
Length of output: 35796
🏁 Script executed:
nl -ba src/renderer/src/lib/connection-owner-resolution.ts | sed -n '1,145p'
printf '%s\n' '--- active workspace route ---'
rg -n -F --glob '*.ts' -- 'function resolveActiveWorkspaceRoute' src/renderer/src/lib
rg -n -F --glob '*.ts' -- 'resolveActiveWorkspaceRoute' src/renderer/src/lib/worktree-operation-route.ts src/renderer/src/lib | head -80
printf '%s\n' '--- editor owner restamping ---'
rg -n -P --glob '*.{ts,tsx}' -C5 'runtimeEnvironmentId\s*=\s*.*activeRuntimeEnvironmentId|runtimeEnvironmentId:\s*.*activeRuntimeEnvironmentId|set.*runtimeEnvironmentId|update.*runtimeEnvironmentId' src/renderer/src/components/editor src/renderer/src/store | head -240Repository: stablyai/orca
Length of output: 32767
🏁 Script executed:
nl -ba src/renderer/src/lib/connection-owner-resolution.ts | sed -n '1,145p'
rg -n -F --glob '*.ts' -- 'function resolveActiveWorkspaceRoute' src/renderer/src/lib
rg -n -F --glob '*.ts' -- 'resolveActiveWorkspaceRoute' src/renderer/src/lib/worktree-operation-route.ts src/renderer/src/lib | head -80
rg -n -P --glob '*.{ts,tsx}' -C5 'runtimeEnvironmentId\s*=\s*.*activeRuntimeEnvironmentId|runtimeEnvironmentId:\s*.*activeRuntimeEnvironmentId|set.*runtimeEnvironmentId|update.*runtimeEnvironmentId' src/renderer/src/components/editor src/renderer/src/store | head -240Repository: stablyai/orca
Length of output: 32508
🏁 Script executed:
nl -ba src/renderer/src/components/editor/useEditorPanelFileContentLoader.ts | sed -n '80,210p'
rg -n -P --glob '*.{ts,tsx}' -C5 'migrateRestored|reown|restored.*owner|runtimeEnvironmentId.*openFile|updateOpenFile|setOpenFile' src/renderer/src/components/editor src/renderer/src/store/slices/editor* src/renderer/src/store | head -260Repository: stablyai/orca
Length of output: 35643
Wake Markdown listing when the focused runtime owner changes.
refreshMarkdownDocuments does not depend on settings.activeRuntimeEnvironmentId or the explicit workspace owner. Focusing the workspace’s owning runtime can clear findRestoredEditorWorkspaceRuntimeOwner without changing activeFile.runtimeEnvironmentId or connectionId, so the effect does not retry and the document list can remain empty.
Subscribe to the focused runtime and owner-route state, or otherwise trigger refreshMarkdownDocuments when those values change. Do not rely on the content loader to restamp this case: it intentionally skips reownership when the restored owner is the focused runtime.
2bf93f3 to
ffb4496
Compare
ffb4496 to
2e95086
Compare
ELI5
While an old SSH editor tab moved onto its managed server, Orca briefly asked this computer to list the remote Markdown files. This computer rejected the remote path. Markdown now waits until the editor has taken its new host, then asks that host.
What Changed
Markdown document listing reuses the content loader’s existing restored-editor ownership check and waits when the workspace owner is still missing. It subscribes to the existing connection-owner selector so a catalog update resumes listing even when the workspace path stays the same. The ownership check accepts only the file fields it reads; its behavior is unchanged. Callback dependencies stay scoped to routing fields, so ordinary editor updates do not restart scans.
A real conversion regression is routed to the CI job that builds its server template. The existing Docker lane contract is extracted unchanged and recognizes that job.
Why
File content already waited for ownership migration, but its document-link metadata started independently from the old tab’s saved owner. Reusing that decision keeps both reads on the same host without a second routing policy, polling, or treating missing ownership as local.
Linked Issue
Found while validating #26483. Independent fix against
OrcaWin/phase35.Visual Proof
Baseline conversion context at the failing desktop metadata request. The attached baseline log records
fs:listMarkdownDocumentsrejecting the remote path; this screenshot captures the timing of that request.Corrected flow: the retained README loads from the host, and its preview link opens the host-only sibling document.
Baseline failure: markdown-conversion-runtime-red.log
Corrected run: markdown-conversion-runtime-green.log
Testing
Hidden macOS Electron client and real Linux Docker SSH host with the exact installed
1.4.223-adhoc.20261008080519server template. Repeated baseline conversions fail the wrong-host request assertion; corrected flow passes in 10.1 seconds, including positive host document listing and rendered preview-link navigation. No Windows runtime claim.93 tests across 10 files pass: managed and folder-workspace migration, missing-owner hydration without a path change, explicit remote ownership before catalog hydration, existing navigation, request deduplication, selector scaling, and CI routing. Full
pnpm tc,pnpm lint, changed quality, fresh Electron E2E build, final affected lint, and commit hooks pass.Review
The existing content loader completes the owner change; Markdown waits for it and resumes from the same store. File saving, server publication, and transport behavior are unchanged.
Agent skill upstream boundary
Notes
The runtime logs also show pre-existing hosted-review reconnect errors and duplicate same-ID editor entries. A rich-editor document link can remain visually marked missing even though its preview link works. These are separate investigations, not fixed or suppressed here. A superseded listing error may be logged without publishing a toast; this PR does not claim every baseline conversion shows a visible error message.
No wire or platform-specific changes. Missing ownership delays a read; it does not authorize a desktop fallback. Existing contact-loss verdicts are preserved.
Checklist