Repository navigation
Thank you for contributing to GitHub CLI! - #1
Merged
Merged
Conversation
|
Warning
PR Compliance Guide 🔍(Compliance updated until commit 0274077)Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label Previous compliance checksCompliance check up to commit 4e65637
|
||||||||||||||||||||||||||||||||||||||||||
Summary of ChangesHello @HIHACK1911, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant enhancements to the GitHub CLI, focusing on integrating the GitHub Copilot CLI, improving interactive workflows for managing pull request reviewers and assignees, and refining various core commands. It also includes comprehensive updates to development dependencies and build configurations, ensuring better code quality and release processes. The changes aim to streamline developer experience and expand the CLI's capabilities. Highlights
Changelog
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||||||||||
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This impressive pull request introduces significant new features, including Copilot CLI integration and dynamic reviewer search, alongside numerous enhancements, bug fixes, and dependency updates. The migration to GoReleaser v2 and expanded linters enhance maintainability. The code is well-structured and thoroughly tested. No security vulnerabilities were identified in the audited files, and there is one minor suggestion to improve code consistency.
| return fmt.Sprintf("%s/%s", r.Organization.Login, r.Slug) | ||
| } | ||
| if r.TypeName == botTypeName && r.Login == CopilotReviewerLogin { | ||
| return "Copilot (AI)" |
There was a problem hiding this comment.
For consistency with ReviewerBot.DisplayName(), it would be better to construct this display name using the CopilotActorName constant rather than hardcoding the string "Copilot (AI)". This improves maintainability in case the actor name for Copilot ever changes.
Suggested change
| return "Copilot (AI)" | |
| return fmt.Sprintf("%s (AI)", CopilotActorName) |
HIHACK1911
marked this pull request as draft
March 24, 2026 15:05
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+116
to
+117
| if !features.ProjectItemQuery { | ||
| return fmt.Errorf("the `--query` flag is not supported on this GitHub host") |
There was a problem hiding this comment.
2. Missing todo above projectitemquery 📘 Rule violation ⚙ Maintainability
A new feature-detection conditional (if !features.ProjectItemQuery) was added without the required immediately-preceding // TODO <cleanupIdentifier> comment. This violates the mandated pattern for feature-detection fallbacks and may fail lint expectations.
Agent Prompt
## Issue description
A feature-detection `if` conditional was added without the required `// TODO <cleanupIdentifier>` comment directly above it.
## Issue Context
The code branches on host capabilities (`features.ProjectItemQuery`), which is tracked as a temporary fallback and must be marked for cleanup.
## Fix Focus Areas
- pkg/cmd/project/item-list/item_list.go[116-117]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+818
to
+826
| reg := &httpmock.Registry{} | ||
| if tt.httpStubs != nil { | ||
| tt.httpStubs(reg) | ||
| } | ||
| httpClient := &http.Client{} | ||
| httpmock.ReplaceTripper(httpClient, reg) | ||
|
|
||
| detector := NewDetector(httpClient, tt.hostname) | ||
|
|
There was a problem hiding this comment.
3. reg.verify(t) missing in test 📘 Rule violation ☼ Reliability
A test creates an httpmock.Registry and registers responders but never calls reg.Verify(t), so unused stubs would not be detected. This can allow tests to pass without exercising expected HTTP interactions.
Agent Prompt
## Issue description
A test uses `httpmock.Registry` without verifying that all registered stubs were called.
## Issue Context
The compliance checklist requires `defer reg.Verify(t)` for tests using `httpmock.Registry` to ensure responders are exercised.
## Fix Focus Areas
- internal/featuredetection/feature_detection_test.go[818-824]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+645
to
+650
| if tt.wantErr { | ||
| assert.Error(t, err) | ||
| return | ||
| } | ||
| assert.NoError(t, err) | ||
| assert.Equal(t, tt.wantFeatures, gotFeatures) |
There was a problem hiding this comment.
4. Uses assert.noerror for errors 📘 Rule violation ☼ Reliability
New tests use assert.NoError/assert.Error for error checks, which can allow the test to continue in an invalid state after a failure. Error checks must use require to fail fast.
Agent Prompt
## Issue description
Tests use `assert.*` for error checks instead of `require.*`, which should halt the test immediately when an error expectation fails.
## Issue Context
The repo’s test conventions require `require.NoError` / `require.Error` for error assertions.
## Fix Focus Areas
- internal/featuredetection/feature_detection_test.go[645-650]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+97
to
+101
| type PullRequestReviewStatus struct { | ||
| ChangesRequested bool | ||
| Approved bool | ||
| ReviewRequired bool | ||
| } |
There was a problem hiding this comment.
5. pullrequestreviewstatus lacks godoc 📘 Rule violation ⚙ Maintainability
A new exported type was introduced without a preceding godoc comment, reducing API documentation quality and potentially failing linting rules. Exported identifiers should have intent-focused comments.
Agent Prompt
## Issue description
New exported identifiers were added without godoc comments.
## Issue Context
Go linting and project conventions require exported types/functions/constants to have doc comments that start with the identifier name.
## Fix Focus Areas
- api/queries_pr_review.go[97-114]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+179
to
+184
| if err := runExternalCmdFunc(externalCmd); err != nil { | ||
| if exitErr, ok := err.(*exec.ExitError); ok { | ||
| // We terminate with os.Exit here, preserving the exit code from Copilot CLI, | ||
| // and also preventing stdio writes by callers up the stack. | ||
| os.Exit(exitErr.ExitCode()) | ||
| } |
There was a problem hiding this comment.
6. Os.exit bypasses telemetry flush 🐞 Bug ☼ Reliability
runCopilot calls os.Exit on exec.ExitError, which skips deferred cleanup in internal/ghcmd (including telemetryService.Flush and updateCancel). This can drop telemetry and bypass the standard external-command exit-code propagation path.
Agent Prompt
## Issue description
`pkg/cmd/copilot/runCopilot` calls `os.Exit(exitErr.ExitCode())` when the Copilot subprocess exits non-zero. This exits the process immediately, skipping deferred cleanups in `internal/ghcmd` (notably `telemetryService.Flush()`), and bypassing the existing centralized exit-code propagation mechanism used for extensions/aliases.
## Issue Context
`internal/ghcmd/cmd.go` uses `defer telemetryService.Flush()` before executing the root command. Because `os.Exit` does not run defers, non-zero exits from Copilot will prevent telemetry from flushing.
## Fix Focus Areas
- pkg/cmd/copilot/copilot.go[179-184]
- internal/ghcmd/cmd.go[130-145]
- internal/ghcmd/cmd.go[194-223]
- pkg/cmd/root/extension.go[43-50]
## Implementation notes
- Avoid `os.Exit` inside command code.
- Instead, return an error that carries an exit code and is handled centrally.
- Option A (recommended): update `internal/ghcmd/cmd.go` to detect `interface{ ExitCode() int }` (or a shared cmdutil error type) and return `exitCode(...)` for it.
- Option B: refactor/move the existing `root.ExternalCommandExitError` (or a new shared type) to a package that `copilot` can import without cycles, and use it here.
- Ensure the error path remains silent (no extra printing) while preserving Copilot’s exit code.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+353
to
+360
| fields := strings.Fields(line) | ||
| if len(fields) >= 2 { | ||
| checksum := fields[0] | ||
| filename := fields[1] | ||
| if filename == archiveName { | ||
| return checksum, nil | ||
| } | ||
| } |
There was a problem hiding this comment.
7. Checksum filename prefix mismatch 🐞 Bug ≡ Correctness
fetchExpectedChecksum compares archiveName to the raw second token in each SHA256SUMS line, so common formats like "<sum> *<filename>" won't match and installation fails with "checksum not found". This makes Copilot installation fragile even when checksums are correct.
Agent Prompt
## Issue description
`fetchExpectedChecksum` parses SHA256SUMS lines using `strings.Fields` and compares `fields[1]` directly to `archiveName`. If the checksum file uses a common convention like `*filename` (binary-mode marker), the comparison fails and the installer errors out with "checksum not found".
## Issue Context
Copilot installation depends on this lookup to proceed. A mismatch here prevents installation even though the downloaded archive checksum may be correct.
## Fix Focus Areas
- pkg/cmd/copilot/copilot.go[347-366]
## Implementation notes
- Normalize the parsed filename before comparing, e.g.:
- `filename = strings.TrimPrefix(filename, "*")`
- optionally also trim surrounding whitespace (already handled) and handle potential leading `./` if needed.
- Keep returning the checksum string exactly as parsed (first field).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
1 similar comment
|
…er/goreleaser-action-7.2.3 chore(deps): bump goreleaser/goreleaser-action from 7.2.2 to 7.2.3
…ples docs(search): add examples for multiple qualifiers
Introduce a dry_run input (default true) to the deployment workflow so artifacts are built and signed but no GitHub Release is created and no site is published. Rename DO_PUBLISH to DO_SIGN_ARTIFACTS for the Windows signing path so signing remains driven solely by environment, independent of publish gating, and set it on the MSI signing step. Co-authored-by: Copilot <[email protected]>
Rework the macOS deployment signing to use App Store Connect API key based notarization and a Developer ID certificate identifier. - Use a single consistent keychain (build.keychain) for signing and notarization; the notarization credentials previously targeted a keychain that was never created. - Gate the App Store Connect API key and notarization credential steps on the production environment so non-production builds (with empty secrets) no longer fail. Co-authored-by: Copilot <[email protected]>
Convert the deployment workflow from reusable `workflow_call` to manual `workflow_dispatch`, removing the extra `repository` input and forwarded secret declarations. Drop per-job deploy app token generation and custom checkout token/repository overrides, relying on the default checkout context instead. Update release publishing to use `secrets.GITHUB_TOKEN` for GH CLI authentication.
Net-new draft agentic issue-triage labeller for cli/cli built with gh-aw. On a newly opened issue (or manual workflow_dispatch) an agent reads the issue and, only when confident, applies a small allowlist of existing cli/cli labels and posts one short rationale comment via native issue-intents safe outputs. Conservative starting point for discussion, not a final triage configuration. Inert until issue_intents is enabled for the org/repo and a repo admin sets the GH_AW_RUNTIME_FEATURES=issue_intents repository variable. Co-authored-by: Copilot App <[email protected]>
Rework the draft workflow to follow gh-cli-and-desktop triage-process.md: the objective is to drive an issue toward removing needs-triage by landing the minimal correct end-state labels, surfaced for maintainer approval via issue-intents rationale and confidence rather than silent auto-apply. - Prompt now follows the documented issues decision tree (closeable / duplicate / spam / abuse / off-topic / no-help-wanted-issue, then bug + priority or unable-to-reproduce, then enhancement or more-info-needed). - add-labels allowlist switched to the real cli/cli triage taxonomy: bug, priority-1/2/3, enhancement, more-info-needed, unable-to-reproduce, off-topic, no-help-wanted-issue, invalid, suspected-spam, duplicate. needs-triage is intentionally excluded (the agent must not add it). - max labels 3, add-comment max 1; engine copilot, strict:false, permissions contents:read + issues:read, github toolset [issues], triggers issues:opened + workflow_dispatch, timeout 10m, intents wiring unchanged. Co-authored-by: Copilot App <[email protected]>
Helps debug 'no identity found' failures by printing the codesigning identities available in the build keychain, so DEVELOPER_ID_CERT_IDENTIFIER can be verified against the actual imported certificate. Co-authored-by: Copilot <[email protected]>
Reflects the reworked macOS code-signing/notarization flow (dedicated build.keychain, App Store Connect API key, notarytool keychain profile, DEVELOPER_ID_CERT_IDENTIFIER/MAC_APP_SIGNING_IDENTITY), documents the new dry_run input and its publishing gates, the ref input and per-job timeouts, the Windows DO_SIGN_ARTIFACTS flag, and the GitHub App site deploy token replacing SITE_DEPLOY_PAT. Co-authored-by: Copilot <[email protected]>
The deployment workflow's dry_run input defaults to true so manual UI dispatches are safe by default. script/release, however, is used to initiate real releases, so it now passes -f dry_run=false and exposes an opt-in --dry-run flag. Without this, releases triggered via script/release silently became no-ops (no attestation, no GitHub Release, no site push). Co-authored-by: Copilot <[email protected]>
The evidence rule told the agent that a dependency in the first `require` block is direct. That is a `go mod tidy` formatting convention, not the semantics. What actually marks a requirement indirect is the trailing `// indirect` comment on its own line, and Go's parser reads it that way regardless of block: put a commented and an uncommented require in the same block and `go mod edit -json` still reports Indirect true and false respectively. The two agree in this repository today, so nothing was misclassified. But the rule would break on a reorganised or hand-edited file, and misreporting a direct dependency as indirect is precisely the error the required-evidence section exists to prevent. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 190d246d-0b1f-4ce6-9aa6-dee10d3d4cf8
Adding a custom pre-agent `steps:` block made the compiler wrap it in a DIFC proxy, because a guard policy is configured. That proxy applies `min-integrity` but not `trusted-users`, which are resolved at runtime after it starts. The pre-flight finds its dedup marker by reading back its own `cli-triage[bot]` comments, and those are precisely what `min-integrity: approved` filters out - the app posts with author_association NONE, which is why `trusted-users` exists here at all. So the marker was never found, every open Dependabot PR looked unassessed on every run, and the workflow would have re-triaged and re-commented on all of them hourly: the exact failure this design was written to prevent, moved from the agent to a place with no model to notice it. Turning the proxy off does not widen the injection surface. The pre-flight hands nothing it reads to the model - it extracts PR numbers, head SHAs and CI states - and it matches the marker only inside comments already narrowed to the app's own login. That login check, not integrity, is what stops a third party forging a marker. The agent still runs under the full policy via the MCP gateway. Verified against cli/cli: all seven open Dependabot PRs are correctly recognised as already assessed at their current head, and a run with the login filter pointed at a non-existent bot correctly reports them as needing assessment. Three smaller corrections ride along, all fallout from the same review: - Silence no-op issue reporting. gh-aw posts a comment to a shared "no-op runs" issue on every noop, and noop is now the routine idle outcome of an hourly reconciler, so that would have been roughly 24 comments a day forever. The run log already records why a run did nothing. - Drop CI state from the required-evidence count. The pre-flight now guarantees terminal CI, so the agent never gathers it and it could never be the missing item that caps confidence. Counting it made the gate for High confidence four items dressed up as five. - Log which PRs the terminal-CI gate excluded. A check that never reports would otherwise keep a PR out of triage permanently and silently. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 18bc01f9-9498-4bd8-9fd0-70308491b695
`statusCheckRollup` contexts are CheckRun and StatusContext objects, which sit behind the `checks` and `statuses` scopes. The workflow token had neither, so the field would have come back unreadable at runtime even though it reads fine with a developer token, which is what I tested with. The dangerous part was not the missing permission but how the gate reacted to it. `[.statusCheckRollup[]? | select(pending)] | length == 0` cannot tell "this PR has no checks" from "I could not read this PR's checks", so an unreadable rollup counted as terminal CI and the PR would have been assessed while its CI was still running. Silently wrong beats loudly broken only from the outside. So the classification now treats a null rollup as pending and names it in the skip log, and the permissions are granted. The gate fails safe if either is ever dropped again. Also warn when the listing hits the 100-PR cap. gh truncates silently and the ordering is stable, so PRs past the cap would never be reached on a later run either. Paginating for a case that far outside anything this repository has seen, and well above the 20-comment safe-output cap, is not worth the extra requests, but the condition should not be invisible. Reported by Copilot review on #14079. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 18bc01f9-9498-4bd8-9fd0-70308491b695
Make Dependabot triage cheaper and more decisive
…igstore/sigstore-go-1.3.0 chore(deps): bump github.com/sigstore/sigstore-go from 1.2.2 to 1.3.0
Bumps [github.com/google/go-containerregistry](https://github.com/google/go-containerregistry) from 0.21.7 to 0.21.8. - [Release notes](https://github.com/google/go-containerregistry/releases) - [Commits](google/go-containerregistry@v0.21.7...v0.21.8) --- updated-dependencies: - dependency-name: github.com/google/go-containerregistry dependency-version: 0.21.8 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.82.1 to 1.83.0. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.82.1...v1.83.0) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-version: 1.83.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
…ctions-432e0d4de6 chore(deps): bump the codeql-actions group across 1 directory with 3 updates
…g.org/grpc-1.83.0 chore(deps): bump google.golang.org/grpc from 1.82.1 to 1.83.0
…oogle/go-containerregistry-0.21.8 chore(deps): bump github.com/google/go-containerregistry from 0.21.7 to 0.21.8
Co-authored-by: Copilot App <[email protected]> Copilot-Session: cd6661ad-7e77-46d0-b0e7-0a296f4e252b
Co-authored-by: Copilot App <[email protected]> Copilot-Session: 18bc01f9-9498-4bd8-9fd0-70308491b695
Copilot-Session: e00d0ec3-c3d6-4ee7-9100-a7f294fe2402
…attest-4.2.1 chore(deps): bump actions/attest from 4.2.0 to 4.2.1
Co-authored-by: Copilot <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> Copilot-Session: 7db06537-cb57-48b9-b09c-e0ea393c3734 Copilot-Session: 6dba8ad8-2a85-4cda-a057-cf91b58a158e
Co-authored-by: Copilot App <[email protected]> Copilot-Session: db48c8cb-9607-4607-8000-121b01833106
Agents were filling "How did you test this change?" with unit test output, which duplicates CI and tells reviewers nothing about whether the change actually works for a user. Spell out the acceptable forms instead: screenshots/GIFs, Given/When/Then, or a plain walkthrough. Co-authored-by: Copilot App <[email protected]> Copilot-Session: af0c0477-c30c-4c0f-b198-42ce0cd450f3
Co-authored-by: Copilot App <[email protected]> Copilot-Session: db48c8cb-9607-4607-8000-121b01833106
Co-authored-by: Copilot App <[email protected]>
Co-authored-by: Copilot App <[email protected]>
Clarify what belongs in the PR template's testing section
Bump golangci-lint in CI to v2.12.2
|
HIHACK1911
marked this pull request as ready for review
August 8, 2026 23:00
HIHACK1911
had a problem deploying
to
cli-automation
August 8, 2026 23:00 — with 
GitHub Actions
Failure
GitHub Actions
Failure
PR Summary by QodoAdd
AI Description
Diagram
High-Level Assessment
Files changed (22)
|
Code Review by Qodo
1. Symlink overwrite on extract
|
Comment on lines
+454
to
+456
| func extractFile(target string, mode os.FileMode, r io.Reader) (err error) { | ||
| out, err := os.OpenFile(target, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, mode) | ||
| if err != nil { |
There was a problem hiding this comment.
1. Symlink overwrite on extract 🐞 Bug ⛨ Security
Copilot tarball extraction validates only lexical traversal via safepaths.Join but then writes files with os.OpenFile(O_TRUNC), which follows any pre-existing symlink under the install directory and can overwrite files outside it. Because downloadCopilot reuses an existing installDir (MkdirAll without cleanup), a local attacker who can plant symlinks in that directory can redirect writes during gh copilot install.
Agent Prompt
### Issue description
Copilot tar.gz extraction prevents `../` traversal but does not defend against symlink traversal. Writing with `os.OpenFile(..., O_TRUNC, ...)` can follow a pre-existing symlink and overwrite arbitrary files outside the install directory.
### Issue Context
`safepaths.Absolute.Join` explicitly does not handle symlinks, and `downloadCopilot` extracts into an existing directory without first removing/atomically replacing it.
### Fix Focus Areas
- pkg/cmd/copilot/copilot.go[319-330]
- pkg/cmd/copilot/copilot.go[413-468]
- internal/safepaths/absolute.go[35-57]
### Implementation guidance
- Extract into a new empty temp directory under `config.DataDir()` and atomically `Rename` into place (after removing/renaming any prior install dir), OR delete `installDir` before extraction.
- Additionally, harden file creation:
- On Unix, consider `O_NOFOLLOW` (via `unix.Open`) for the final path component.
- Reject symlinks in any path component (walk parents with `Lstat`), or write only into a directory tree that you just created.
- Consider using `O_EXCL` semantics when possible to avoid clobbering existing filesystem objects.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+56
to
+59
| if io.ExperimentalPrompterEnabled() { | ||
| return &huhPrompter{ | ||
| stdin: io.In, | ||
| stdout: io.Out, |
There was a problem hiding this comment.
2. Accessible prompter ignored 🐞 Bug ≡ Correctness
internal/prompter.New prioritizes ExperimentalPrompterEnabled over AccessiblePrompterEnabled, so when both GH_EXPERIMENTAL_PROMPTER and GH_ACCESSIBLE_PROMPTER are enabled the non-accessible huhPrompter is selected and accessibility mode is silently bypassed. This PR also wires both env vars independently, making the conflict reachable.
Agent Prompt
### Issue description
When both accessibility and experimental prompter flags are enabled, `prompter.New` returns `huhPrompter` first, bypassing `accessiblePrompter` and losing `WithAccessible(true)` behavior.
### Issue Context
This PR introduces `GH_EXPERIMENTAL_PROMPTER` wiring alongside `GH_ACCESSIBLE_PROMPTER`, but does not define precedence or prevent both from being enabled simultaneously.
### Fix Focus Areas
- internal/prompter/prompter.go[55-72]
- internal/ghcmd/cmd.go[360-374]
- internal/prompter/huh_prompter.go[22-27]
### Implementation guidance
Choose one of:
1) Give accessibility precedence:
- In `prompter.New`, check `io.AccessiblePrompterEnabled()` before `io.ExperimentalPrompterEnabled()`.
2) Enforce mutual exclusivity:
- In `newIOStreams` (or earlier), if both env vars/configs are enabled, either:
- disable experimental automatically when accessible is enabled, or
- emit a clear warning/error and pick a deterministic default.
(1) is safest for users relying on accessibility features.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+173
to
+176
| func isSameRef(refs creationRefs) bool { | ||
| if strings.Contains(refs.QualifiedHeadRef(), ":") { | ||
| return false | ||
| } |
There was a problem hiding this comment.
3. Same-branch check bypass 🐞 Bug ≡ Correctness
isSameRef treats any QualifiedHeadRef containing ':' as cross-repo and returns false, but QualifiedHeadRef parsing allows <owner>:<branch> without validating that owner differs from the base repo owner. As a result, --head <baseOwner>:<baseBranch> can bypass the new same-branch prevention and proceed to an avoidable PR-creation failure later.
Agent Prompt
### Issue description
`isSameRef` assumes any `:` in the qualified head ref implies a fork, and therefore skips same-branch detection. However, qualified refs can legally include an owner even when referring to the same repository.
### Issue Context
`ParseQualifiedHeadRef` explicitly accepts both `<branch>` and `<owner>:<branch>` forms without checking repo relationships, and `QualifiedHeadRef.String()` preserves the owner prefix when present.
### Fix Focus Areas
- pkg/cmd/pr/create/create.go[170-178]
- pkg/cmd/pr/shared/find_refs_resolution.go[46-75]
### Implementation guidance
- Replace the `strings.Contains(..., ":")` heuristic with proper parsing:
- Parse the qualified head ref into `(owner?, branch)`.
- If branch != baseRef: return false.
- If owner is absent: treat as same-repo and return true.
- If owner is present: only treat as cross-repo when `owner != baseRepoOwner`; if `owner == baseRepoOwner`, treat it as same-repo and return true.
- (Optional) If you can access the resolved head repo, compare repositories instead of only owner strings.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This branch had an error being deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
PR Type
Enhancement, Tests, Bug fix
Description
Implemented GitHub Copilot CLI integration with download, extraction, checksum validation, and removal functionality supporting multiple platforms and archive formats
Added dynamic search functionality for PR reviewers and assignees with GraphQL mutations and queries for suggestions, including support for users, bots, and teams
Introduced multi-select with search capability to the prompter interface with persistent options and cascading quota system for reviewer suggestions
Added
--actionsflag to browse command for navigating to repository actions with mutual exclusivity validationImplemented git remote name resolution in
set-defaultcommand to accept remote names as argumentsAdded validation to prevent PR creation with identical head and base branches
Separated Copilot logins for assignees and reviewers with distinct constants and replacer implementations
Fixed null project item handling in GraphQL queries for issues and pull requests
Improved error messages in reference parsing with better context information
Allowed
--refflag to work with--allin cache deletion commandMigrated GoReleaser configuration to v2 format with updated deployment workflow and signing configuration
Updated Go version to 1.25.7 and expanded golangci-lint linter configuration
Removed unused Google Cloud dependencies and updated third-party licenses
Enhanced test coverage across multiple commands including PR edit, Copilot, cache deletion, browse, and extension registration
Diagram Walkthrough
File Walkthrough
15 files
edit_test.go
Update PR edit tests for new reviewer mutation and metadata handlingpkg/cmd/pr/edit/edit_test.go
shared.EditableReviewerswrapper typeinstead of direct
shared.EditableSlicefor reviewer fieldsmockRequestReviewsByLoginmutationinstead of
mockPullRequestAddReviewersfor non-interactive reviewerupdates
mutation" to verify empty slices properly clear all reviewer types
operations don't require reviewer metadata fetching
both assignees and reviewers
search function behavior
copilot_test.go
Add comprehensive test suite for Copilot CLI commandpkg/cmd/copilot/copilot_test.go
cases covering argument parsing, removal, and download functionality
createTarGzBufferandcreateZipBufferforcreating test archives
validation
missing binary detection
support
queries_pr_test.go
Add tests for suggested reviewer actors GraphQL queryapi/queries_pr_test.go
mockReviewerResponse()helper function to generate GraphQLresponses for reviewer suggestion tests
TestSuggestedReviewerActors()with 9 test cases coveringcascading quota logic, deduplication, and edge cases
without organization teams, and error handling
accessible_prompter_test.go
Add tests for MultiSelectWithSearch prompter functionalityinternal/prompter/accessible_prompter_test.go
MultiSelectWithSearchfunctionalitycovering basic flow, default pre-selection, persistence between
searches, and error propagation
handling of search results
prompter_mock.go
Add mock support for multi-select search methodinternal/prompter/prompter_mock.go
MultiSelectWithSearchFuncfield toPrompterMockfor mockingsearch functionality
MultiSelectWithSearchmethod with allparameters
delete_test.go
Add tests for cache deletion with ref parameterpkg/cmd/cache/delete/delete_test.go
--all --refcombination
success-on-no-caches scenarios
create_test.go
Add tests for same head/base branch validationpkg/cmd/pr/create/create_test.go
produce errors
Test_isSameReffunction with comprehensive test cases for refcomparison logic
PRs, and fork scenarios
setdefault_test.go
Add tests for git remote name resolutionpkg/cmd/repo/setdefault/setdefault_test.go
remotesfield to test struct for mocking git remotesmatching remote name
Remotesfunction to factoryextension_registration_test.go
Add tests for extension command conflict detectionpkg/cmd/root/extension_registration_test.go
skipped
scenarios
browse_test.go
Add tests for browse actions flagpkg/cmd/browse/browse_test.go
--actionsflag--actions --wikicombination--actionsflagActionsFlagfieldtest.go
Add mock support for multi-select search in testsinternal/prompter/test.go
multiSelectWithSearchStubsfield toMockPrompterfor test stubsMultiSelectWithSearchmethod for mock prompterRegisterMultiSelectWithSearchmethod for test setupqueries_projects_v2_test.go
Add tests for null project item handlingapi/queries_projects_v2_test.go
queries
params_test.go
Add tests for Copilot reviewer replacerpkg/cmd/pr/shared/params_test.go
TestCopilotReviewerReplacer_ReplaceSlicetest function@copilotwith reviewer loginfinder_test.go
Add test for large number parsing errorpkg/cmd/pr/shared/finder_test.go
range
details
zip_test.go
Update tests for zip package renameinternal/zip/zip_test.go
downloadtozipextractZiptoExtractZip17 files
queries_pr.go
Add GraphQL mutations and queries for reviewer/assignee suggestionsapi/queries_pr.go
DisplayName()method toRequestedReviewerfor user-friendlyreviewer names, with special handling for Copilot bot
DisplayNames()method toReviewRequeststo return display namesfor all reviewers
extractTeamSlugs()helper to extract slug portion from"org/slug" team identifiers
toGitHubV4Strings()helper to convert string slices toGraphQL string types with optional suffix
RequestReviewsByLogin()GraphQL mutation function for settingreviewers on github.com with support for users, bots, and teams
SuggestedAssignableActors()query function to fetch suggestedassignees for an issue or PR
ReviewerCandidate,ReviewerUser,ReviewerBot,ReviewerTeam) with display name methodsSuggestedReviewerActors()query with cascading quotasystem to fetch up to 15 suggested reviewers from multiple sources
AddPullRequestReviews()andRemovePullRequestReviews()to useextractTeamSlugs()for proper team slug handlingcopilot.go
Implement GitHub Copilot CLI integration commandpkg/cmd/copilot/copilot.go
copilotcommand with support for downloading andrunning GitHub Copilot CLI
CopilotOptionsstruct with configuration for HTTP client,prompter, and command arguments
downloadCopilot()function with checksum validation andplatform-specific archive extraction
extractTarGz()andextractZip()functions with path traversalsecurity checks using
safepathsfetchExpectedChecksum()to parse SHA256SUMS.txt files fromrelease downloads
findCopilotBinary()to locate Copilot in PATH or gh's datadirectory
removeCopilot()to uninstall downloaded Copilot CLI--removeflag and interactive installation promptsedit.go
Add dynamic search functions and GraphQL mutation for reviewerspkg/cmd/pr/edit/edit.go
assigneeSearchFunc()to provide dynamic assignee search withmetadata population for ID resolution
reviewerSearchFunc()to provide dynamic reviewer searchsupporting users, bots, and teams
@copilotsupport foradd/remove operations
updatePullRequestReviews()to use GraphQL mutation ongithub.com and REST API on GHES
updatePullRequestReviewsGraphQL()using newRequestReviewsByLoginmutationupdatePullRequestReviewsREST()for legacy GHEScompatibility
partitionUsersAndTeams()topartitionReviewersByType()toseparately identify bots and teams
@copilotreplacement logic for non-interactive revieweroperations
ActorIsAssignablefeature is enableddelete.go
Allow --ref flag to work with --all for cache deletionpkg/cmd/cache/delete/delete.go
--alland--refflags toallow using them together
--refwith--allfor deleting all cachesfor a specific ref
GetCaches()call to passRefoption whenDeleteAllis truecombined usage
editable.go
Add dynamic search support for reviewers and assigneespkg/cmd/pr/shared/editable.go
ReviewerSearchFuncandAssigneeSearchFuncfields toEditablestruct for dynamic search functionality
EditableReviewerstype withDefaultLoginsfield similar toEditableAssigneesMultiSelectWithSearchmethod toEditPrompterinterfaceEditFieldsSurveyto use search-based selection when searchfunctions are available
FetchOptionsto conditionally fetch reviewers/assignees basedon search function availability
prompter.go
Implement multi-select with dynamic search functionalityinternal/prompter/prompter.go
MultiSelectWithSearchmethod toPrompterinterface withcomprehensive documentation
multiSelectWithSearchfunction supporting dynamic searchresults with persistent options
MultiSelectSearchResultstruct to return search results withkeys, labels, and more results count
accessiblePrompterandsurveyPromptertypesprompter.go
Add preview command support for multi-select searchpkg/cmd/preview/prompter/prompter.go
multiSelectWithSearchPromptconstant and corresponding handlerfunction
runMultiSelectWithSearchdemonstration function withsearch callback
type
root.go
Add copilot command and extension conflict detectionpkg/cmd/root/root.go
copilotCmdpackagestandalone functionality
copilotCmd.NewCmdCopilotto root commandoverriding core commands
browse.go
Add actions flag to browse commandpkg/cmd/browse/browse.go
ActionsFlagfield toBrowseOptionsstruct--actionsflag definition with short form-aActionsFlagactionssection handling inparseSectionfunctionsetdefault.go
Support git remote names in set-default commandpkg/cmd/repo/setdefault/setdefault.go
fails
name
params.go
Support different Copilot logins for assignees and reviewerspkg/cmd/pr/shared/params.go
copilotLoginfield toCopilotReplacerstructNewCopilotReviewerReplacerfunction for reviewer-specificCopilot handling
NewCopilotReplacerto useapi.CopilotAssigneeLoginconstantreplacemethod to use instance-specificcopilotLoginfieldsurvey.go
Add multi-select search to prompt interfacepkg/cmd/pr/shared/survey.go
MultiSelectWithSearchmethod toPromptinterfaceprompterpackagecreate.go
Add validation to prevent same head/base branch PRspkg/cmd/pr/create/create.go
isSameReffunction to check if head and base refs point to samelocation
createRunto prevent PR creation with identicalhead and base branches
qualified ref
queries_repo.go
Separate Copilot logins for assignees and reviewersapi/queries_repo.go
CopilotActorLoginconstant toCopilotAssigneeLoginCopilotReviewerLoginconstant for pull request reviewersDisplayNamemethod to useCopilotAssigneeLoginfor comparisoncontexts
http.go
Use public zip extraction functionpkg/cmd/run/download/http.go
ghzippackage aliasextractZipfunction call toghzip.ExtractZipzip.go
Rename zip package and export extraction functioninternal/zip/zip.go
downloadtozipextractZipfunction toExtractZip(exported)query_builder.go
Add Bot support to pull request review requests queryapi/query_builder.go
...on Bot{login}toprReviewRequestsquery to support Copilot asreviewer
namefield to...on Userfragment in review requestsGHES compatibility
3 files
queries_projects_v2.go
Handle null project items in GraphQL queriesapi/queries_projects_v2.go
results
results
edit_test.go
Fix mock data login casepkg/cmd/issue/edit/edit_test.go
finder.go
Improve error message in reference parsingpkg/cmd/pr/shared/finder.go
ParseFullReferenceto include the originalstring and wrapped error
sin error format3 files
go.mod
Update Go version and dependenciesgo.mod
third-party-licenses.windows.md
Update third-party licenses and remove unused dependenciesthird-party-licenses.windows.md
cloud.google.com/go packages, GoogleCloudPlatform packages)
(charmbracelet/bubbletea, charmbracelet/x/ansi, cli/oauth,
gdamore/tcell/v2, go-openapi packages, sigstore packages, etc.)
Unknown (cyberphone/json-canonicalization, in-toto packages)
packages, fsnotify, go-jose, go-chi, googleapis packages, and others)
third-party-licenses.linux.md
Update third-party licenses and remove unused dependenciesthird-party-licenses.linux.md
cloud.google.com/go packages, GoogleCloudPlatform packages)
(charmbracelet/bubbletea, charmbracelet/x/ansi, cli/oauth,
gdamore/tcell/v2, go-openapi packages, sigstore packages, etc.)
Unknown (cyberphone/json-canonicalization, in-toto packages)
packages, fsnotify, go-jose, go-chi, googleapis packages, and others)
4 files
deployment.yml
Update deployment workflow for GoReleaser v2 and signing.github/workflows/deployment.yml
v1.0.95
.goreleaser.yml
Migrate GoReleaser configuration to v2 format.goreleaser.yml
version: 2directive for GoReleaser v2 formatgoarchvalues to strings (e.g., "386" instead of 386)buildstoidsfieldformattoformatsarray syntaxrlcpoption (deprecated in v2)idsfield.golangci.yml
Expand golangci-lint linter configuration.golangci.yml
exptostd, fatcontext, and others
lint.yml
Update lint workflow dependencies.github/workflows/lint.yml
6 files
LICENSE
Add license for yaml v3 packagethird-party/go.yaml.in/yaml/v3/LICENSE
NOTICE
Add notice file for jsonpointer packagethird-party/github.com/go-openapi/jsonpointer/NOTICE
NOTICE
Add notice file for jsonreference packagethird-party/github.com/go-openapi/jsonreference/NOTICE
CONTRIBUTING.md
Update contribution guidelines.github/CONTRIBUTING.md
help wantedlabeled issuessource.md
Improve documentation formattingdocs/source.md
instructions
install_linux.md
Update Debian package documentation linkdocs/install_linux.md
101 files