[brennanb2025]

What changes

PR details now load in forked repositories. Previously, opening a PR in a repo where origin points to a fork and upstream points to the canonical repo showed "Unable to load details for this GitHub item."

Root cause: getOwnerRepo() resolved only the origin remote, so PR lookups queried the fork (where the PR does not exist) instead of upstream (where fork-contribution PRs live). Issue lookups already resolved upstream-first, which is why issues worked but PRs did not.

This builds on and hardens community PR #7332 (by @fsdwen) — that PR is left open and unmodified; this one carries its commit plus review fixes so it can be considered as a reviewed, test-hardened unit.

Approach

• getOwnerRepo() now resolves upstream-first and falls back to origin, matching the existing getIssueOwnerRepo() behavior. All ~20 PR-detail call sites that use getOwnerRepo() benefit automatically.
• New getOriginOwnerRepo() for the callers that genuinely need the fork's origin: createGitHubPullRequest (head repo is the fork), getRepoUpstream (fork detection), and resolvePrWorkItemSource (raw origin candidate).
• getIssueOwnerRepo() retained as a backwards-compatible alias.

Takeover review — what was wrong and what this fixes

The original PR switched only two callers to the origin-specific resolver and missed one, plus left stale comments and thin test coverage. This PR adds:

1. Regression fix (renderer): resolvePrWorkItemSource resolved its originCandidate via the now-upstream-first getOwnerRepo. For a fork that collapsed originCandidate === upstreamCandidate, and the renderer's fork detection (!sameGitHubOwnerRepo(originCandidate, upstreamCandidate)) then hid the issue-source (origin/upstream) selector on fork repos. Now resolved via getOriginOwnerRepo, preserving the type contract that originCandidate is the raw origin remote.
2. Comment accuracy: refreshed two comments (work-item-details.ts, client.ts) that still claimed PR lookups target origin.
3. Test coverage: gave getOriginOwnerRepo a dedicated mock (distinct from getOwnerRepo) plus a regression test that fails if the originCandidate fix is reverted; distinct origin/upstream values in the cache-separation test; realistic missing-remote error wording so the negative-cache path is exercised and the git-spawn-count guard restored; and getOriginOwnerRepoMock resets.

Completeness & headline behavior

• Headline behavior (fork PR details load): implemented at the resolution seam that the details dialog (gh:workItemDetails) actually uses.
• Broad consistency: PR detail, participants, checks, and list-source paths all now resolve fork-aware; the renderer fork toggle is preserved (regression Linux support #1).

Known tradeoff (conscious decision for the reviewer)

The upstream-first flip is a global heuristic. It fixes the common fork-contribution case (PR lives on upstream) but introduces a mirror-image edge case: because the details dialog re-derives a PR's repo by number, a fork-internal PR whose number also exists on upstream would now resolve to the upstream item. The root-cause-correct fix is to thread the already-known PR owner/repo through the details IPC + runtime RPC + detail helpers so PR details never re-guess by number — a cross-cutting change beyond this bug fix. Net effect here is still positive (common case fixed, rare collision regressed). Flagging so this is merged as a conscious decision; the prRepo-threading follow-up can be tracked separately.

Validation

• npx vitest run src/main/github → 296 passed (22 files; +1 regression test).
• npx tsgo --noEmit -p config/tsconfig.node.json → clean.
• Two-round-clean adversarial code review; performance checked — the added upstream probe is bounded by the existing per-runtime/path/remote owner-repo cache (30s positive / 5min negative TTL) with in-flight coalescing, so non-fork repos add negligible git churn.
• SSH/remote and WSL paths flow through the same connectionId/localGitOptions plumbing (unchanged); GitLab/other providers are untouched (GitHub-remote-resolution only).

Not merged.

Fixes #7331

🤖 Generated with Claude Code

Made with Orca 🐋

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 5046edf8-63d5-4661-b0ac-c95fc3c65525

📥 Commits

Reviewing files that changed from the base of the PR and between ecff854 and 385d50e.

📒 Files selected for processing (9)

• src/main/github/client-create-pr.test.ts
• src/main/github/client-issue-source.test.ts
• src/main/github/client-work-items.test.ts
• src/main/github/client.test.ts
• src/main/github/client.ts
• src/main/github/gh-utils.test.ts
• src/main/github/gh-utils.ts
• src/main/github/github-repository-identity.ts
• src/main/github/work-item-details.ts

---

📝 Walkthrough

Walkthrough

This PR introduces a new getOriginOwnerRepo helper that resolves GitHub owner/repo strictly from the origin remote, while getOwnerRepo is changed to probe upstream first and fall back to origin. getIssueOwnerRepo now delegates to getOwnerRepo. client.ts updates call sites in resolvePrWorkItemSource, getRepoUpstream, and createGitHubPullRequest to use getOriginOwnerRepo, plus a comment update. gh-utils.ts re-exports the new helper. Associated test files are updated to mock and verify the new upstream-first and origin-only resolution behaviors.

Changes

Area	Change
github-repository-identity.ts	Added getOriginOwnerRepo; getOwnerRepo now upstream-first with origin fallback; getIssueOwnerRepo delegates to getOwnerRepo
gh-utils.ts	Re-exports getOriginOwnerRepo
client.ts	Uses getOriginOwnerRepo in PR source resolution, repo upstream lookup, and PR creation; comment update
work-item-details.ts	Comment update clarifying upstream resolution for both issues and PRs
Test files	Mock and assert getOriginOwnerRepo behavior across client.test.ts, client-create-pr.test.ts, client-issue-source.test.ts, client-work-items.test.ts, gh-utils.test.ts

Related PRs: None identified.

Suggested labels: area:github, refactor, tests

Suggested reviewers: Reviewer with GitHub client and remote-resolution expertise.

A rabbit hops from origin's gate,
then checks upstream, won't hesitate.
Two paths now clear,
resolved with cheer —
owner and repo, disambiguate!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description covers the change, validation, and tradeoffs, but it does not follow the required template sections.	Rewrite the PR description to use the repo template, including Summary, Screenshots, Testing, AI Review Report, Security Audit, and Notes.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: loading PR details correctly in forked repositories.
Linked Issues check	✅ Passed	The code change matches #7331 by making getOwnerRepo upstream-first so PR details load in forked repositories.
Out of Scope Changes check	✅ Passed	The added helper, comment updates, and regression tests all support the forked-PR fix and are not unrelated scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[nwparker]

Thanks @brennanb2025 for hardening the community fix with the extra test coverage. The upstream-first change landed on main in #9384 (commit 4930c87), rebased against current main with the call-site audit (one behavioral difference from this branch: getRepoSlug stays origin-based so renderer display/icon autodetect keep the checkout's own identity). You're credited as a commit co-author on the merge. Closing as superseded — appreciated!