Skip to content

feat: Resolve issues and PRs by full GitHub URL#2094

Open
charlesvien wants to merge 2 commits into05-06-track_individual_subscription_start_and_cancel_eventsfrom
05-07-resolve_issues_and_prs_by_full_github_url
Open

feat: Resolve issues and PRs by full GitHub URL#2094
charlesvien wants to merge 2 commits into05-06-track_individual_subscription_start_and_cancel_eventsfrom
05-07-resolve_issues_and_prs_by_full_github_url

Conversation

@charlesvien
Copy link
Copy Markdown
Member

@charlesvien charlesvien commented May 7, 2026

Problem

Pasting a full GitHub issue or PR URL into the message editor's issue picker fell through to a title-only text search and returned "No issues or pull requests found."

Changes

  1. Add parseGithubRefUrl covering both /issues/N and /pull/N
  2. Short-circuit searchGithubRefs to a direct gh view on URL match
  3. Resolve against the URL's repo so cross-repo references work
  4. Read pathname rather than full_name (git-url-parse drops the issue suffix)
  5. Add parser tests for issue and PR URL shapes

How did you test this?

Manually

Publish to changelog?

@charlesvien charlesvien changed the title Resolve issues and PRs by full GitHub URL feat: Resolve issues and PRs by full GitHub URL May 7, 2026
Copy link
Copy Markdown
Member Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 7, 2026

Comments Outside Diff (1)

  1. packages/git/src/utils.ts, line 163-179 (link)

    P2 parsePrUrl is now a subset of parseGithubRefUrl — both functions duplicate the same gitUrlParse call, github.com source check, and number validation. Per the OnceAndOnlyOnce rule, parsePrUrl could become a thin wrapper: const ref = parseGithubRefUrl(url); if (!ref || ref.kind !== 'pr') return null; return { owner: ref.owner, repo: ref.repo, number: ref.number };. This would also bring parsePrUrl onto the pathname-based parsing path as a bonus fix.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: packages/git/src/utils.ts
    Line: 163-179
    
    Comment:
    `parsePrUrl` is now a subset of `parseGithubRefUrl` — both functions duplicate the same `gitUrlParse` call, `github.com` source check, and number validation. Per the OnceAndOnlyOnce rule, `parsePrUrl` could become a thin wrapper: `const ref = parseGithubRefUrl(url); if (!ref || ref.kind !== 'pr') return null; return { owner: ref.owner, repo: ref.repo, number: ref.number };`. This would also bring `parsePrUrl` onto the `pathname`-based parsing path as a bonus fix.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
apps/code/src/main/services/git/service.ts:1555-1565
The ternary `urlRef.kind === "pr" ? "pr" : "issue"` is superfluous — `urlRef.kind` is already typed as `"issue" | "pr"`, and both string values map directly to the `gh` CLI sub-command. The ternary says the same thing twice (violating the OnceAndOnlyOnce rule).

```suggestion
      return this.fetchGhRefs(
        [
          urlRef.kind,
          "view",
          String(urlRef.number),
          "--repo",
          repoSlug,
        ],
        repoSlug,
        urlRef.kind,
      );
```

### Issue 2 of 2
packages/git/src/utils.ts:163-179
`parsePrUrl` is now a subset of `parseGithubRefUrl` — both functions duplicate the same `gitUrlParse` call, `github.com` source check, and number validation. Per the OnceAndOnlyOnce rule, `parsePrUrl` could become a thin wrapper: `const ref = parseGithubRefUrl(url); if (!ref || ref.kind !== 'pr') return null; return { owner: ref.owner, repo: ref.repo, number: ref.number };`. This would also bring `parsePrUrl` onto the `pathname`-based parsing path as a bonus fix.

Reviews (1): Last reviewed commit: "Resolve issues and PRs by full GitHub UR..." | Re-trigger Greptile

Comment thread apps/code/src/main/services/git/service.ts
Copy link
Copy Markdown

@arthurdedeus arthurdedeus left a comment

Choose a reason for hiding this comment

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

lgtm!

@charlesvien charlesvien force-pushed the 05-07-resolve_issues_and_prs_by_full_github_url branch from e71fc4d to f57eddc Compare May 7, 2026 21:11
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 7, 2026

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
packages/git/src/utils.test.ts:237-252
**Asymmetric negative coverage for `pull` rejects**

The `pr` rejects block is missing test cases for `pull/-1` and `pull/42.5` that are explicitly covered in the `issue` rejects block. The implementation shares the same validation path so the behaviour is correct, but the test asymmetry means a future refactor could silently regress negative-number or fractional-number rejection for PR URLs without a test catching it.

Reviews (2): Last reviewed commit: "unify github url parsing into one functi..." | Re-trigger Greptile

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.

2 participants