Skip to content

Fix fork PR detection for same-name upstreams - #6446

Merged
OrcaWin merged 1 commit into
stablyai:mainfrom
brennanb2025:brennanb2025/fix-fork-pr-detection
Jun 26, 2026
Merged

OrcaWin merged 1 commit into
stablyai:mainfrom
brennanb2025:brennanb2025/fix-fork-pr-detection

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Keep tracked upstream metadata when the remote branch name matches the local branch name.
  • Retry GitHub PR discovery with the tracked upstream remote owner when the initial origin-head lookup misses.
  • Avoid duplicate same-name retries when the tracked upstream resolves to the same head repo.
  • Add local and SSH regression coverage for same-name fork upstream branches.

Screenshots

Not applicable; GitHub PR lookup behavior only.

AI Review Report

CodeRabbit reviewed the PR and left one maintainability nit about the adjacent Why comment. Addressed in the latest commit.

Security Audit

No new inputs, permissions, credential handling, or shell command surfaces. The change reuses existing git/gh execution paths and only changes which already-configured Git remote owner is used for a GitHub PR lookup retry.

Validation

  • pnpm test -- src/main/github/client.test.ts - 89 passed
  • pnpm test -- src/main/github/gh-utils.test.ts - 35 passed
  • pnpm run lint - passed; existing useGitStatusPolling.ts exhaustive-deps warning remains
  • pnpm run typecheck - passed
  • git diff --check HEAD~1..HEAD - clean

Electron validation

Launched the PR Electron app from C:/Users/neil/orca/workspaces/orca/fix-fork-pr-detection via Orca CLI terminal. Verified identity over CDP:

  • name: Orca: brennanb2025/fix-fork-pr-detection
  • devRepoRoot: C:\Users\neil\orca\workspaces\orca\fix-fork-pr-detection
  • renderer: http://localhost:5173/

Imported C:/Users/neil/source/orca into that dev app and invoked the same gh.prForBranch path for brennanb2025/worktree-remove-fix. Temporary trace showed Electron resolved the tracked upstream as brennan/brennanb2025/worktree-remove-fix, retried with headRepo: brennanb2025/orca, and found PR #6433 in stablyai/orca. The public result is now null only because #6433 is already MERGED and Orca intentionally hides implicit merged branch matches without a linked PR hint.

Notes

This is intentionally GitHub-only. Other hosted review providers continue through their existing provider-specific lookup paths.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

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: 066823c4-b51a-4a7d-8422-9b0e8d790e43

📥 Commits

Reviewing files that changed from the base of the PR and between 185c5c4fa13e8361d22a241ce808bd59442e77ec and 3b48117.

📒 Files selected for processing (2)
  • src/main/github/client.test.ts
  • src/main/github/client.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/main/github/client.test.ts
  • src/main/github/client.ts

📝 Walkthrough

Walkthrough

Tracked-upstream branch parsing was split from retry decision logic. parseTrackedUpstreamRef now parses upstream refs directly, and new owner/repo key helpers compare upstream and current PR identities. getPRForBranchOutcome only retries upstream PR lookup when shouldRetryTrackedUpstreamBranch returns true. Tests were added for same-name upstream owner resolution, SSH-based owner lookup, and avoiding repeated retries after an initial miss.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: fixing fork PR detection for same-name upstreams.
Description check ✅ Passed The description covers summary, screenshots, validation, AI review, security, and notes, but the Testing template is not followed exactly and cross-platform review isn't explicit.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/main/github/client.ts (1)

2828-2831: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Update the adjacent Why comment for same-name fork retries.

The guarded retry now covers same-name upstream branches from a different fork owner, but the nearby comment only mentions differently named remote heads.

Suggested comment update
-        // Why: worktrees can have a short local branch tracking a differently
-        // named remote PR head; after the local miss, try that configured head.
+        // Why: the tracked upstream can identify the real PR head by
+        // branch name or by fork owner even when branch names match locally.

As per coding guidelines, “When writing or modifying code driven by a design doc or non-obvious constraint, add a comment explaining why the code behaves the way it does.”

Source: Coding guidelines


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6687bb8b-1faa-4b52-ab9f-0fe33e48a4a9

📥 Commits

Reviewing files that changed from the base of the PR and between d188e7d and 185c5c4fa13e8361d22a241ce808bd59442e77ec.

📒 Files selected for processing (2)
  • src/main/github/client.test.ts
  • src/main/github/client.ts

@brennanb2025
brennanb2025 force-pushed the brennanb2025/fix-fork-pr-detection branch from 185c5c4 to 3b48117 Compare June 26, 2026 21:02
@OrcaWin
OrcaWin merged commit 85d626b into stablyai:main Jun 26, 2026
1 check passed
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 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.

3 participants