Skip to content

fix(github): route runtime repo paths by execution host - #33194

Closed
Ethereal49 wants to merge 3 commits into
stablyai:mainfrom
Ethereal49:fix/repo-path-host-routing
Closed

Ethereal49 wants to merge 3 commits into
stablyai:mainfrom
Ethereal49:fix/repo-path-host-routing

Conversation

@Ethereal49

Copy link
Copy Markdown
Contributor

ELI5

Orca can know the same repository under two different registrations: one over SSH and one on a paired runtime. When both use the same folder path, the GitHub client could pick the SSH registration and send its local repo ID to the runtime. The runtime does not know that ID, so the PR status never loaded. This change makes the client prefer the registration that belongs to the runtime it is asking.

What Changed

When a GitHub request is routed to a runtime environment, the shared repo resolver now:

  1. looks for a repo with the requested path whose execution host is that runtime; and
  2. falls back to the previous first-path match when no runtime-owned row exists.

This covers PR refresh and the shared work-item request path, including pagination, counts, prefetch, issue/PR comments, review threads, and check details. Explicit GitHub task-source contexts already used their own repoId and keep that behavior.

The general path lookup is unchanged, so callers that only know a path keep their existing first-registration semantics.

Why

findByPath() is first-registration-wins. A legacy SSH row for the same path can therefore win over a runtime row and produce a repo ID that the runtime rejects with repo_not_found.

Passing the candidate repoId everywhere would require wider changes and does not help callers that genuinely only have a path. Filtering by the target runtime's execution host at the shared resolver fixes both affected surfaces with one small change and preserves the old fallback.

Linked Issue

Fixes #32997

Visual Proof

N/A: this only changes which repo row is used for GitHub runtime RPCs. There is no UI or interaction change to capture.

Testing

  • Automated tests added/updated
  • I manually tested these changes locally

Commands run:

ORCA_BACKGROUND_LAUNCH=1 /opt/homebrew/opt/node@24/bin/node \
  node_modules/vitest/vitest.mjs run --config config/vitest.config.ts \
  src/renderer/src/store/slices/github-pr-refresh-owner-routing.test.ts \
  src/renderer/src/store/slices/github-work-items-runtime-routing.test.ts

Result after merging the latest origin/main@35157974b99d: 2 files passed, 21 tests passed.

The new PR case previously sent repo: 'repo-ssh' to github.prForBranch; it now sends repo: 'repo-runtime'. The new work-item case previously sent repo: 'ssh-repo-id'; it now sends repo: 'runtime-repo-id'. A third assertion keeps the existing first-path fallback when the target runtime has no matching row.

Also passed:

  • tsc --noEmit -p config/tsconfig.web.json --composite false
  • node config/scripts/check-changed-code-quality.mjs
  • oxfmt --check on the three changed files
  • git diff --check

Not run: a real paired runtime plus legacy SSH registration, full pnpm lint / pnpm test / pnpm build, and upstream CI. I did not claim manual end-to-end verification.

AI Disclosure

Codex harness; model deepseek-v4.1-flash; reasoning max. AI assisted with code analysis, implementation, tests.

Review

Please focus on the runtime-owner filter/fallback in getRuntimeRepoTarget() and confirm that explicit GitHub source contexts and local/SSH routing remain unchanged.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Cross-platform: host routing is not OS-specific.
  • Remote: this is the reported SSH + paired-runtime case.
  • Local: local and direct SSH rows do not match a runtime host, so the existing path fallback remains.
  • Mobile / agent integrations / git providers: no behavior change.
  • Security and performance: no new external input, IPC, credentials, or network surface; one linear scan of the in-memory repo list only when a request targets a runtime.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (targeted web typecheck, tests, and changed-code quality passed; full local gates not run)

When a repo path is registered on both SSH and a paired runtime, path-only lookup could send the SSH-side repo ID to the runtime. Prefer the row owned by the target runtime, then retain the original first-path fallback.

AI-Assisted: OpenAI Codex

Signed-off-by: ethereal49 <hongyeluo49@gmail.com>
@coderabbitai

coderabbitai Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 845a19fd-9966-4ef4-b540-ad4a370918f5
Commits

Reviewing files that changed from the base of the PR and between 41d25c0 and 8da969f.

Files selected for processing: 3
  • src/renderer/src/store/github/repository-routing.ts
  • src/renderer/src/store/slices/github-pr-refresh-owner-routing.test.ts
  • src/renderer/src/store/slices/github-work-items-runtime-routing.test.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.

Walkthrough

getRuntimeRepoTarget now prefers a repository that matches both the requested path and the target runtime host. If no such repository exists, it retains the path-only lookup fallback. Tests cover PR refresh and work-item routing when SSH and runtime repositories share a path, and verify the fallback when no repository matches the runtime.

Priority: Normal

Severity of issue fixed: Medium

Merge Risk: Minimal · up to 8da96

The routing change appears ready to merge after normal checks; no concrete issue remains from this review.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: routing runtime repository paths by execution host.
Description check Passed The description is complete and relevant. It explains the problem, implementation, rationale, affected behavior, linked issue, visual proof, tests, limitations, and compatibility considerations.
Linked Issues check Passed Issue [#32997] requires runtime-owned repository resolution when the same path has SSH and runtime rows, plus a regression test for the runtime repository ID. getRuntimeRepoTarget() now matches the …
Out of Scope Changes check Passed The whole-PR diff changes one shared runtime resolver and adds focused tests for PR refresh and work-item routing. The fallback, local/SSH routing, and explicit source-context behavior remain covered.…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files.
Full details: Linked Issues check

Explanation

Issue [#32997] requires runtime-owned repository resolution when the same path has SSH and runtime rows, plus a regression test for the runtime repository ID. getRuntimeRepoTarget() now matches the requested path and runtime:&lt;environmentId&gt; host before the existing path lookup. The PR tests cover PR refresh, shared work-item routing, and the first-path fallback. The existing explicit source-context path remains unchanged in the reviewed test evidence.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff changes one shared runtime resolver and adds focused tests for PR refresh and work-item routing. The fallback, local/SSH routing, and explicit source-context behavior remain covered. The changes support issue [#32997] and do not show unrelated product behavior.

  • 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nwparker

Copy link
Copy Markdown
Contributor

Thank you @Ethereal49 for this fix and for the clear write-up. Your diagnosis was right: when one folder path has both an SSH row and a paired-server row, the GitHub request to the server could carry the SSH row's repo id, so PR status never loaded. We shipped your commit as-is, with your authorship kept, in PR #33257. On top of it we added two things. The lookup now prefers the repo id the caller already has, and it uses the store's repo index instead of scanning the list. Current main has a test that limits repo reads during a refresh sweep, and the list scan went over that limit. We also dropped the fallback test, because it pinned sending another host's repo id to the server. Your two regression tests are in the PR and fail on main without the fix. Thanks again!

@nwparker nwparker closed this Oct 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: PR status is missing when a repo path is registered on both an SSH host and a paired runtime

2 participants