Skip to content

Find a PR by the name its branch was pushed under - #920

Merged
alexeyzimarev merged 3 commits into
mainfrom
tracked-branch-pr-detection
Sep 12, 2026
Merged

alexeyzimarev merged 3 commits into
mainfrom
tracked-branch-pr-detection

Conversation

@alexeyzimarev

Copy link
Copy Markdown
Member

Closes #896 — AI-2714

What & why

A session whose branch was pushed under another name was never linked to its PR. An argument-free
gh pr view looks for the branch git would push to, and under the default push.default=simple a
branch whose upstream goes by another name has no push destination — so gh queries the local name
and finds nothing, once a minute, for the life of the PR. On a miss, detection now asks for the
tracked remote branch by name, pinned to the session's repository with --repo, and takes only a
same-repository head of exactly that name.

Where to look

The fallback fails closed rather than guess: an upstream on another repository's remote, the
remote's default branch, or a remote whose default is unknown yields nothing, because a branch cut
from origin/main tracks main without being its PR. A branch tracking a long-lived non-default
branch (develop, with an open develop→main PR) is still linked to that PR, which is what gh
itself does under push.default=upstream. GitLab keeps querying by the local branch name.

Verification

gh pr view desktop-remote-control-slice2 --repo github.com/kurrent-io/kcap-cli --json number,headRefName,isCrossRepository returns #874 — the lookup the issue's session needed, and the
command shape the fallback builds. The new tests drive real git repositories with only gh faked,
and assert the precondition gh trips on (git rev-parse --symbolic-full-name '@{push}' exits
non-zero) so they cannot pass vacuously; each guard was watched failing before its code existed.

Check Result
Capacitor.Cli.Tests.Unit 4,073 passed, 0 failed, 19 skipped
dotnet publish -c Release no IL2026/IL3050 warnings
scripts/check-linear-ids.sh exit 0

🤖 Generated with Claude Code

Under the default push.default, gh resolves no push destination for a branch whose upstream goes by another name, so it queries the local name and misses.

The fallback fails closed rather than guess: a default-branch or other-repository upstream yields nothing, since a branch cut from origin/main tracks main without being its PR.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-12T12:41:37.212844Z 1f23a76 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Detect GitHub PRs by tracked remote branch name

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Finds GitHub PRs using tracked remote branch names after local-name lookup misses.
• Rejects unsafe, cross-repository, default-branch, and unverifiable fallback candidates.
• Shares one provider deadline across routing, normal detection, and fallback probes.
Diagram

graph TD
  A["Repository Detection"] --> B["Provider Router"] --> C["Normal gh Lookup"] --> D{"PR Found?"}
  D -- No --> E["Tracked Branch Resolver"] --> F["Git Metadata"] --> G["Pinned gh Lookup"] --> H["PR Info"]
  D -- Yes --> H
Loading
High-Level Assessment

The PR’s fail-closed fallback is the appropriate approach. Direct API or gh pr list --head queries would duplicate filtering and host behavior, while changing Git’s push.default would mutate or reinterpret user configuration. Reusing gh pr view with an explicit branch and repository preserves existing provider behavior while the surrounding validation prevents false matches.

Files changed (9) +478 / -22

Bug fix (4) +136 / -22
CommandArgument.csValidate interpolated command arguments +16/-0

Validate interpolated command arguments

• Introduces a generated-regex gate for values interpolated into Git and GitHub CLI argument strings. It rejects whitespace, quotes, leading options, and other unsafe branch or repository identifiers.

src/Capacitor.Cli/PrDetection/CommandArgument.cs

PrDetector.csAdd repository-pinned GitHub branch lookup +28/-4

Add repository-pinned GitHub branch lookup

• Adds explicit GitHub PR detection by branch and repository. The returned PR must have the exact requested head and originate from the repository itself, while JSON parsing is shared with normal detection.

src/Capacitor.Cli/PrDetection/PrDetector.cs

TrackedBranch.csResolve safe tracked remote branch names +59/-0

Resolve safe tracked remote branch names

• Resolves a differently named upstream from Git configuration and verifies its remote points to the session repository. It rejects unsafe names, same-name branches, foreign remotes, default branches, and unknown remote defaults.

src/Capacitor.Cli/PrDetection/TrackedBranch.cs

RepositoryDetection.csRun tracked-branch fallback within the provider deadline +33/-18

Run tracked-branch fallback within the provider deadline

• Invokes tracked-branch GitHub detection only after the normal lookup misses. Refactors remaining-time calculation so routing, Git probes, and both GitHub lookups share one provider deadline; GitLab behavior remains unchanged.

src/Capacitor.Cli/RepositoryDetection.cs

Tests (4) +330 / -0
GitHubPrDetectorTests.csTest pinned GitHub branch detection safeguards +49/-0

Test pinned GitHub branch detection safeguards

• Verifies the explicit gh command shape, exact-head matching, same-repository requirement, and rejection of unsafe interpolated arguments.

test/Capacitor.Cli.Tests.Unit/PrDetection/GitHubPrDetectorTests.cs

TrackedBranchTests.csTest tracked branch resolution and fail-closed guards +144/-0

Test tracked branch resolution and fail-closed guards

• Exercises resolution against real temporary Git repositories, including renamed upstreams, alternate remotes, forks, detached heads, missing metadata, default branches, unsafe names, and exhausted budgets.

test/Capacitor.Cli.Tests.Unit/PrDetection/TrackedBranchTests.cs

ProviderBudgetSplitTests.csTest fallback deadline sharing +50/-0

Test fallback deadline sharing

• Confirms tracked-branch lookup receives only the time left by normal detection. Also verifies no fallback probes run once the shared provider budget is exhausted.

test/Capacitor.Cli.Tests.Unit/ProviderBudgetSplitTests.cs

RepositoryDetectionTrackedBranchTests.csTest end-to-end renamed-branch PR detection +87/-0

Test end-to-end renamed-branch PR detection

• Uses real Git metadata with a fake GitHub CLI to reproduce the missing push-destination condition and validate fallback detection. It also confirms normal hits skip fallback and later PR discovery triggers a watcher update.

test/Capacitor.Cli.Tests.Unit/RepositoryDetectionTrackedBranchTests.cs

Documentation (1) +12 / -0
CHANGES.mdDocument tracked-branch PR detection fallback +12/-0

Document tracked-branch PR detection fallback

• Explains why argument-free GitHub PR detection misses differently named upstream branches. Documents repository, default-branch, safety, and shared-deadline guardrails.

docs/CHANGES.md

@qodo-code-review

qodo-code-review Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (2) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Git tests bypass the process limit 📘 Rule violation ▣ Testability
Description
TrackedBranchTests creates repositories through GitRepo.CreateWithCommit() but declares the
class without [ParallelLimiter<SubprocessLimit>]. Its setup and several tests execute real git
children, so parallel test execution bypasses the suite-wide subprocess pool.
Code

test/Capacitor.Cli.Tests.Unit/PrDetection/TrackedBranchTests.cs[5]

+public class TrackedBranchTests {
Relevance

●●● Strong

Real subprocess tests require concurrency controls; repository history accepts fixes for
subprocess-related test isolation.

PR-#425
PR-#656

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2808212 requires test classes that run real subprocesses to use the shared limiter. The new
class creates real Git repositories, while GitRepo ultimately launches git with Process.Start,
and the class declaration has no limiting attribute.

Rule 2808212: Use ParallelLimiter<SubprocessLimit> for test classes running real subprocesses
test/Capacitor.Cli.Tests.Unit/PrDetection/TrackedBranchTests.cs[5-20]
test/Capacitor.Tests.Helpers/GitRepo.cs[196-209]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TrackedBranchTests` runs real Git subprocesses without participating in the test suite's shared subprocess limit.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/PrDetection/TrackedBranchTests.cs[5-5]

## Recommended Fix
Add `[ParallelLimiter<SubprocessLimit>]` immediately above the test class so all tests in the fixture draw from the shared subprocess pool.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Numeric branches still miss their PRs ✓ Resolved 🐞 Bug ≡ Correctness
Description
DetectForBranchAsync passes the tracked branch as the ambiguous positional selector to `gh pr
view`, where an all-numeric value selects a pull-request number instead of a branch. When branch
123 belongs to any PR other than #123, the fallback queries the wrong object and either rejects
its head or returns no result.
Code

src/Capacitor.Cli/PrDetection/PrDetector.cs[28]

+        var json = await run("gh", $"pr view {branch} --repo {host}/{owner}/{repo} --json {Fields},isCrossRepository", cwd, cap);
Relevance

●●● Strong

Numeric branch names are ambiguous positional selectors; explicit head lookup is needed for correct
PR resolution.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new validator accepts all-numeric branch names, the fallback passes the resulting value directly
to the positional pr view selector, and its post-query head check only turns a wrong numeric
selection into a miss. Elsewhere, the repository already uses gh pr list --head when it needs an
explicit branch lookup.

src/Capacitor.Cli/PrDetection/CommandArgument.cs[12-15]
src/Capacitor.Cli/PrDetection/TrackedBranch.cs[32-42]
src/Capacitor.Cli/PrDetection/PrDetector.cs[21-34]
src/Capacitor.Cli.Core/PullRequests/Readers/GitHubCli/GitHubCliReaderProvider.cs[82-87]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The tracked-branch lookup passes branch names positionally to `gh pr view`, so numeric branch names are interpreted as pull-request numbers and cannot reliably resolve their PR.

## Fix Focus Areas
- src/Capacitor.Cli/PrDetection/PrDetector.cs[21-34]
- src/Capacitor.Cli.Core/PullRequests/Readers/GitHubCli/GitHubCliReaderProvider.cs[82-87]
- test/Capacitor.Cli.Tests.Unit/PrDetection/GitHubPrDetectorTests.cs[47-94]

## Recommended Fix
Use an unambiguous head-filtered query such as `gh pr list --head <branch> --state open` and parse its array response, retaining exact head-name and same-repository checks. Add coverage where tracked branch `123` resolves to a PR whose number is not 123.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Valid punctuated branches miss PRs ✓ Resolved 🐞 Bug ≡ Correctness
Description
CommandArgument.IsPlain rejects legal branch characters such as %, =, ,, and ! even though
they are neither whitespace, quotes, nor leading option markers. A local or tracked branch
containing one of these safe characters exits the fallback before its Git configuration or pull
request can be queried.
Code

src/Capacitor.Cli/PrDetection/CommandArgument.cs[R14-15]

+    [GeneratedRegex(@"^[A-Za-z0-9_][A-Za-z0-9_./+@-]*\z")]
+    private static partial Regex PlainRegex();
Relevance

●●● Strong

Narrow validation excludes legal branch punctuation and causes deterministic PR lookup misses.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The regex admits only a narrow ASCII subset, and both the local and tracked branch gates return
before querying Git when it fails. The command runner launches the executable directly with
UseShellExecute=false, so safe punctuation is not interpreted by a shell, while the new comment
identifies whitespace, quotes, and leading hyphens—not these punctuation characters—as the argument
hazards.

src/Capacitor.Cli/PrDetection/CommandArgument.cs[6-15]
src/Capacitor.Cli/PrDetection/TrackedBranch.cs[16-33]
src/Capacitor.Cli/RepositoryDetection.cs[297-317]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new argument allowlist excludes valid Git branch characters that do not create whitespace, quoting, or option-injection hazards, causing tracked-branch detection to skip those branches.

## Fix Focus Areas
- src/Capacitor.Cli/PrDetection/CommandArgument.cs[6-15]
- src/Capacitor.Cli/PrDetection/TrackedBranch.cs[16-33]
- src/Capacitor.Cli/PrDetection/PrDetector.cs[21-28]
- test/Capacitor.Cli.Tests.Unit/PrDetection/TrackedBranchTests.cs[109-130]

## Recommended Fix
Pass dynamic values through a command API using separate argument tokens, or implement correct platform-aware argument encoding, rather than restricting them to an incomplete character allowlist. Preserve protection against leading option markers and add real-Git tests for safe punctuated local and tracked branch names.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View medium (1)
4. Repository tests evade the process cap 📘 Rule violation ▣ Testability
Description
RepositoryDetectionTrackedBranchTests declares no [ParallelLimiter<SubprocessLimit>] despite
using GitRepo.CreateWithCommit() and forwarding non-gh commands to
RepositoryDetection.DefaultRunner. Repository setup, the push-destination assertion, and detection
probes therefore launch real git children outside the shared concurrency pool.
Code

test/Capacitor.Cli.Tests.Unit/RepositoryDetectionTrackedBranchTests.cs[12]

+public class RepositoryDetectionTrackedBranchTests {
Relevance

●●● Strong

This fixture launches real Git processes without the mandated shared limiter, matching accepted
subprocess-isolation concerns.

PR-#425
PR-#656

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2808212 requires the shared limiter on subprocess-heavy test classes. The new fixture both
creates repositories through the real Git helper and delegates every non-gh detection command to
the production process runner, but its class declaration has no limiter.

Rule 2808212: Use ParallelLimiter<SubprocessLimit> for test classes running real subprocesses
test/Capacitor.Cli.Tests.Unit/RepositoryDetectionTrackedBranchTests.cs[12-25]
test/Capacitor.Cli.Tests.Unit/RepositoryDetectionTrackedBranchTests.cs[28-45]
src/Capacitor.Cli/RepositoryDetection.cs[297-317]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RepositoryDetectionTrackedBranchTests` executes real Git commands without participating in the test suite's shared subprocess limit.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/RepositoryDetectionTrackedBranchTests.cs[12-12]

## Recommended Fix
Add `[ParallelLimiter<SubprocessLimit>]` immediately above the test class so its repository setup and detection probes use the shared subprocess pool.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
Review mode: ⚖️ Balanced: Downgraded extended -> standard: change is below the extended eligibility bar (hunks 11/18, lines 500/200; both must reach the floor). Router rationale: This is a behavior-changing, security-sensitive command-invocation and PR-detection feature spanning multiple independent logic paths, repository/provider resolution, deadline handling, and extensive edge cases, making redundant review materially valuable.

Grey Divider

Tip of the day
💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo


namespace Capacitor.Cli.Tests.Unit.PrDetection;

public class TrackedBranchTests {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Git tests bypass the process limit 📘 Rule violation ▣ Testability

TrackedBranchTests creates repositories through GitRepo.CreateWithCommit() but declares the
class without [ParallelLimiter<SubprocessLimit>]. Its setup and several tests execute real git
children, so parallel test execution bypasses the suite-wide subprocess pool.
Agent Prompt
## Issue description
`TrackedBranchTests` runs real Git subprocesses without participating in the test suite's shared subprocess limit.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/PrDetection/TrackedBranchTests.cs[5-5]

## Recommended Fix
Add `[ParallelLimiter<SubprocessLimit>]` immediately above the test class so all tests in the fixture draw from the shared subprocess pool.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

/// cannot resolve a push destination and falls back to the local name. Real git answers the
/// branch-config probes; only gh is faked.
/// </summary>
public class RepositoryDetectionTrackedBranchTests {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Repository tests evade the process cap 📘 Rule violation ▣ Testability

RepositoryDetectionTrackedBranchTests declares no [ParallelLimiter<SubprocessLimit>] despite
using GitRepo.CreateWithCommit() and forwarding non-gh commands to
RepositoryDetection.DefaultRunner. Repository setup, the push-destination assertion, and detection
probes therefore launch real git children outside the shared concurrency pool.
Agent Prompt
## Issue description
`RepositoryDetectionTrackedBranchTests` executes real Git commands without participating in the test suite's shared subprocess limit.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/RepositoryDetectionTrackedBranchTests.cs[12-12]

## Recommended Fix
Add `[ParallelLimiter<SubprocessLimit>]` immediately above the test class so its repository setup and detection probes use the shared subprocess pool.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread src/Capacitor.Cli/PrDetection/PrDetector.cs
Comment thread src/Capacitor.Cli/PrDetection/CommandArgument.cs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1f23a7657e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +23 to +24
if (!CommandArgument.IsPlain(host) || !CommandArgument.IsPlain(owner)
|| !CommandArgument.IsPlain(repo) || !CommandArgument.IsPlain(branch)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Allow valid dot-prefixed repository names

For a valid GitHub repository whose name begins with a dot—most notably an organization’s .github repository—GitUrlParser supplies .github here, but IsPlain(repo) rejects it and the fallback never invokes gh, so PRs pushed under another branch name remain undetected. The local gh pr view --help describes --repo [HOST/]OWNER/REPO; because the constructed argument begins with the host, a dot at the start of the repository component cannot be interpreted as an option. Validate the composite repository argument separately rather than applying the branch-leading-character restriction to every component.

Useful? React with 👍 / 👎.

alexeyzimarev and others added 2 commits September 12, 2026 14:43
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
No shell is involved, so the argument gate only has to stop a value splitting or re-quoting: rejecting '%' or a leading dot cost an organisation's .github repository its fallback.

gh reads an all-digit selector as a PR number, so a branch named 123 asked for PR #123 and the head check then refused the answer — a round trip spent to learn nothing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@alexeyzimarev
alexeyzimarev merged commit ff60a49 into main Sep 12, 2026
8 checks passed
@alexeyzimarev
alexeyzimarev deleted the tracked-branch-pr-detection branch September 12, 2026 15:15
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.

Discover PRs when local and tracked remote branch names differ

1 participant