Repository navigation
refactor(runtime): runtime git and file callers drop SSH arms - #27501
Conversation
📝 Walkthrough
Merge Risk: 🔵 Low · up to Most SSH host operations now fail cleanly. Two small inconsistencies remain. SSH model discovery shows a different error message than other SSH operations. Branch and ref lookups can treat some repositories as SSH-hosted when they are not, or miss ones that are. Both are quick fixes and fine to address as a follow-up. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 50 files. (16 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Title check The title clearly and concisely identifies the main change: removing obsolete SSH branches from runtime Git and file callers. Description check The description is detailed and covers the required explanation, scope, behavior, rationale, testing, visual proof, review notes, and checklist. The linked issue section states that this is a maintain… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 50 files. (16 skipped: 16 over the file limit.)
✨ 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.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
369c3335-f633-44a5-8d78-eb1a9d26ccd9
📒 Files selected for processing (74)
src/main/git/remote-name-listing.test.tssrc/main/git/remote-name-listing.tssrc/main/ipc/hooks/worktree-hook-execution-host.test.tssrc/main/ports/workspace-port-execution-host.test.tssrc/main/ports/workspace-port-execution-host.tssrc/main/provisioned-root-checkout.test.tssrc/main/provisioned-root-ssh-adoption.test.tssrc/main/provisioned-root-ssh-adoption.tssrc/main/runtime/orca-runtime-files-chunk-reads.test.tssrc/main/runtime/orca-runtime-files-mobile-explorer-reads.test.tssrc/main/runtime/orca-runtime-files-preview-budget.test.tssrc/main/runtime/orca-runtime-files-rename-authority.test.tssrc/main/runtime/orca-runtime-files-search.test.tssrc/main/runtime/orca-runtime-files-ssh-chunk-reads.test.tssrc/main/runtime/orca-runtime-files-terminal-artifact-grants.test.tssrc/main/runtime/orca-runtime-files-terminal-artifact-io.test.tssrc/main/runtime/orca-runtime-files-terminal-link-host-translation.test.tssrc/main/runtime/orca-runtime-files-terminal-path-resolution.test.tssrc/main/runtime/orca-runtime-files-watch-host-scope.test.tssrc/main/runtime/orca-runtime-files-watch.test.tssrc/main/runtime/orca-runtime-git-branch-diff.test.tssrc/main/runtime/orca-runtime-git-diff-budget.test.tssrc/main/runtime/orca-runtime-git.test.tssrc/main/runtime/orca-runtime-list-managed-worktrees.tssrc/main/runtime/orca-runtime-quick-open-capabilities.test.tssrc/main/runtime/orca-runtime-tests/gitlab-and-pr-bases-part-02.spec.tssrc/main/runtime/orca-runtime-tests/gitlab-and-pr-bases-part-03.spec.tssrc/main/runtime/orca-runtime-tests/hooks-and-hosted-review.spec.tssrc/main/runtime/orca-runtime-tests/lineage-and-scan-cache.spec.tssrc/main/runtime/orca-runtime-tests/ssh-worktree-lifecycle-part-02.spec.tssrc/main/runtime/orca-runtime-tests/ssh-worktree-lifecycle-part-03.spec.tssrc/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-02.spec.tssrc/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-03.spec.tssrc/main/runtime/orca-runtime-tests/worktree-selector-resolution.spec.tssrc/main/runtime/orca-runtime-workspace-ports-host.test.tssrc/main/runtime/rpc/methods/git-diff-transport-budget.test.tssrc/main/runtime/runtime-file-command-access.tssrc/main/runtime/runtime-file-command-target.tssrc/main/runtime/runtime-file-commands-assert-remote-terminal-file-grant-path-still-canonical.tssrc/main/runtime/runtime-file-commands-create-file-explorer-dir-no-clobber.tssrc/main/runtime/runtime-file-commands-mobile-file-list-limit.tssrc/main/runtime/runtime-file-commands-read-file-explorer-preview.tssrc/main/runtime/runtime-file-commands-read-mobile-file.tssrc/main/runtime/runtime-file-commands-resolve-allowed-terminal-artifact-path.tssrc/main/runtime/runtime-file-commands-resolve-terminal-path.tssrc/main/runtime/runtime-file-commands-revoke-terminal-file-grants-for-client.tssrc/main/runtime/runtime-file-commands-ssh-file-watcher-rearm.tssrc/main/runtime/runtime-file-commands-write-terminal-artifact-file.tssrc/main/runtime/runtime-file-listing-producer-budgets.test.tssrc/main/runtime/runtime-file-search.tssrc/main/runtime/runtime-file-target-execution-host.test.tssrc/main/runtime/runtime-file-watcher-leases.tssrc/main/runtime/runtime-git-branch-compare-admission.test.tssrc/main/runtime/runtime-git-command-target.test.tssrc/main/runtime/runtime-git-command-target.tssrc/main/runtime/runtime-git-diff-commands.tssrc/main/runtime/runtime-git-generation-admission.test.tssrc/main/runtime/runtime-git-generation-commands.tssrc/main/runtime/runtime-git-generation-context.tssrc/main/runtime/runtime-git-sync-commands.test.tssrc/main/runtime/runtime-git-sync-commands.tssrc/main/runtime/runtime-git-target-execution-host.test.tssrc/main/runtime/runtime-github-worktree-base.tssrc/main/runtime/runtime-gitlab-fork-mr-base.tssrc/main/runtime/runtime-gitlab-issue-source-remote.tssrc/main/runtime/runtime-gitlab-worktree-base.tssrc/main/runtime/runtime-preserved-branch-cleanup.tssrc/main/runtime/runtime-remote-quick-open-search.tssrc/main/runtime/runtime-repository-hooks-commands.tssrc/main/runtime/runtime-repository-issue-command.test.tssrc/main/runtime/runtime-repository-issue-command.tssrc/main/runtime/runtime-repository-ref-queries.tssrc/main/runtime/runtime-terminal-spawn-push-target-materialization.test.tssrc/main/runtime/runtime-terminal-spawn-push-target-materialization.ts
💤 Files with no reviewable changes (8)
- src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-02.spec.ts
- src/main/runtime/orca-runtime-files-ssh-chunk-reads.test.ts
- src/main/runtime/runtime-file-watcher-leases.ts
- src/main/runtime/orca-runtime-files-preview-budget.test.ts
- src/main/runtime/runtime-remote-quick-open-search.ts
- src/main/runtime/orca-runtime-files-search.test.ts
- src/main/runtime/rpc/methods/git-diff-transport-budget.test.ts
- src/main/runtime/runtime-file-commands-ssh-file-watcher-rearm.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
| (plan, cwd, timeoutMs) => provider.executeCommitMessagePlan(plan, cwd, timeoutMs), | ||
| agentCommandOverride | ||
| ) | ||
| return { success: false, error: `No git provider for connection "${route.connectionId}"` } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use SSH_HOST_NOT_SERVED_MESSAGE for SSH model discovery.
Every other SSH path in this class returns SSH_HOST_NOT_SERVED_MESSAGE. Model discovery is the exception. It returns No git provider for connection "<id>" instead. The PR states that SSH error text stays the same. This string shows the user a different message for the same unserved-host state. The UI can also skip the reconnect prompt for this case.
Proposed fix
--- "a/src/main/runtime/runtime-git-generation-commands.ts"
+++ "b/src/main/runtime/runtime-git-generation-commands.ts"
@@ -222,7 +222,7 @@
this.host.getRuntimeSettings().agentCmdOverrides?.[typedAgentId]
const route = runtimeGitRouteForTarget(target)
if (route.kind === 'ssh') {
- return { success: false, error: `No git provider for connection "${route.connectionId}"` }
+ return { success: false, error: SSH_HOST_NOT_SERVED_MESSAGE }
}
const localEnv = await prepareLocalCommitMessageAgentEnv(
typedAgentId,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return { success: false, error: `No git provider for connection "${route.connectionId}"` } | |
| return { success: false, error: SSH_HOST_NOT_SERVED_MESSAGE } |
| // Why: an `ssh:` repo is not served here; it reports no refs and never searches this machine. | ||
| const refDetails = repo.connectionId | ||
| ? await this.searchRemote(repo, query, probeLimit, includeQualifiedRefs) | ||
| ? [] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
rg -nP -C4 'export function hydrateRepo' src/main
rg -nP -C3 'export function getStoredRepoExecutionHostId' src/mainRepository: stablyai/orca
Length of output: 2265
🏁 Script executed:
set -eu
printf '%s\n' '--- src/main/repo-execution-host.ts ---'
nl -ba src/main/repo-execution-host.ts | sed -n '1,180p'
printf '%s\n' '--- src/main/persistence/tracking-repos/repo-hydration.ts ---'
nl -ba src/main/persistence/tracking-repos/repo-hydration.ts | sed -n '1,180p'
printf '%s\n' '--- src/main/runtime/runtime-repository-ref-queries.ts ---'
nl -ba src/main/runtime/runtime-repository-ref-queries.ts | sed -n '1,180p'
printf '%s\n' '--- sibling helper usages ---'
rg -n -F -C5 -- 'getStoredRepoSshConnectionId' src/main/runtime src/main | head -240
printf '%s\n' '--- cited test shapes ---'
rg -n -F -C8 -- 'executionHostId' src/main/ipc/hooks/worktree-hook-execution-host.test.tsRepository: stablyai/orca
Length of output: 33705
Use getStoredRepoSshConnectionId(repo) for the SSH check in search and getDefault.
hydrateRepo does not populate connectionId from executionHostId. It preserves both fields from the stored row. The raw connectionId check therefore misses canonical SSH rows and treats stale connectionId values on local or runtime: rows as SSH.
🐛 Suggested fix
+import { getStoredRepoSshConnectionId } from '../repo-execution-host'
@@
- const refDetails = repo.connectionId
+ const refDetails = getStoredRepoSshConnectionId(repo)
? []
: await searchBaseRefDetails(repo.path, query, probeLimit, includeQualifiedRefs)
@@
- if (repo.connectionId) {
+ if (getStoredRepoSshConnectionId(repo)) {
return { defaultBaseRef: null, remoteCount: 0 }
}
ELI5
Orca used to have a second way to run git and file work on an SSH machine (the "relay"). That relay was removed earlier, but the runtime still had code paths saying "if this is an SSH workspace, ask the relay's git/file helper". That helper is never set up anymore, so those paths always ended in "connection dropped" (or an empty answer). This PR deletes those dead paths from the runtime's git and file commands and makes each one fail (or answer empty) in one consistent, typed way.
What Changed
User-visible behaviour is unchanged. Managed SSH hosts are reached as paired runtimes (
runtime:<env>), which this PR does not touch. Anssh:<target>workspace that is still routed to this process gets the same result as before:SshHostNotServedError, with the same text as before ("Remote connection dropped. Click Reconnect on the SSH target before retrying."): runtime git status/diff/compare/sync/commit/push, file explorer read/write/rename/delete/upload/preview/chunk/watch, mobile file reads, terminal-path stats and artifact grants, GitHub/GitLab PR/MR worktree bases, and force-deleting a preserved branch.success: falsewith the same message), cancel (no-op), model discovery, Quick Open/mobile file listing (empty, capability 0), ref search/default (empty), hooks and issue-command reads (none /status: 'error'), issue-command writes (dropped), remote-name listing (null, i.e. probe anyway), terminal-spawn push-target materialization (skipped), and host port scans (unavailableReason).Mechanism:
RuntimeGitRoute/RuntimeFileRoutekeep ansshvariant but it no longer carries a provider. NewrequireLocalRuntimeGitRoute/requireLocalRuntimeFileRoutethrowSshHostNotServedErrorat the same point the old "provider missing" check did, so anssh:id never reaches a local git/file call.requireSshRuntimeGitProvider/requireSshRuntimeFileProviderare deleted.runtime-file-commands-ssh-file-watcher-rearm.ts) and the remote Quick Open search (runtime-remote-quick-open-search.ts). Remote terminal-artifact grants are gone: a grant was only minted after a successful remote stat, which never happens.getActiveMultiplexersite): anssh:workspace reports unavailable with the same reason text it reported after refactor(providers): an ssh: route carries no provider and throws one typed error #27477.provisioned-root-ssh-adoption.tskeeps its request checks, then refuses with its existing "no longer available" error. The checkout-verification cases it used to cover now testverifyProvisionedRootCheckoutdirectly (newprovisioned-root-checkout.test.ts), so the live runtime adoption path keeps that coverage.isRuntimeWorktreePathMissingand the worktree-removal paths are left for B4/A2.Net lines: production −1,299 (+218 / −1,517), tests −2,664 (+486 / −3,150).
Why
This is Phase 3.99 PR B5 (Lane B, after B2 #27477). Each removed arm reads an SSH git or filesystem provider that has no production registrar:
registerSshGitProvider(/registerSshFilesystemProvider(have only test callers. Deleting the arms removes a "provider or local" branch that new call sites kept copying. We chose to keep thesshroute variant over dropping it, because several commands answer anssh:host with a structured result rather than an error, and resolvingssh:to a managed runtime would be new routing behaviour (D-1, out of scope).Proof that the removed code is dead:
rg -woversrc config tests mobile packages scripts .github, which includes all 168@ts-nocheckfiles, finds 0 references to every removed symbol:requireSshRuntimeGitProvider,requireSshRuntimeFileProvider,RuntimeFileSshRoute,armSshFileExplorerWatchRearm,stopSshFileExplorerWatchRearms,sshFileExplorerWatchRearms,searchRemoteQuickOpenFilePaths,readRemoteMobileFile,statRemoteTerminalPath,resolveAllowedRemoteTerminalArtifactPath,assertRemoteTerminalFileGrantPathStillCanonical,assertRemoteTerminalFileGrantFresh(ForRead),readRemoteTerminalArtifact(File|Preview),terminalArtifactAccessOptions,isRemoteNotFoundErrorMessage,materializeOverSsh,scanSshExecutionHost,getSshMultiplexer,listRemoteMobileFiles.pnpm tc,build:electron-vite,build:cliandbuild:orcadall pass.Linked Issue
Phase 3.99 cleanup (maintainer stack); no issue.
Visual Proof
N/A: no UI change. Paths that already failed keep their error text or their empty result.
Testing
pnpm tc,pnpm lint(all ratchets at or under baseline),pnpm run check:code-quality:changed, oxlint on the changed files.pnpm teston every test undersrc/main/runtime,src/main/git,src/main/portsandsrc/main/ipc/hooks(1,431 files), plus the 282 test files elsewhere that import a changed module. All pass.SSH-provider tests that exercised the dead arms were deleted, or rewritten to assert the refusal or unavailable answer and that no local git/file call is made (
expectRefusedFor(..., 'm4air')keeps the "routes to the host it names" check).I manually tested these changes locally
Automated tests added/updated, or explained why not below
Review
Agent skill upstream boundary
Notes
ssh:arm throws or keeps its unavailable answer, before any local path resolution or spawn.TerminalFileGrant.providerstays'local'on the wire type, which already allowed'ssh'.local.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)