Fix acceptance test failures: git identity, headRepository JSON, obsolete traversal test#13037
Conversation
The sandbox overrides HOME so git cannot find the user's global config, causing 'Author identity unknown' errors when acceptance test scripts make commits. Write a minimal .gitconfig with user.name and user.email into the sandbox working directory during sharedSetup. Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com>
GitHub's Artifact API now rejects artifact names like '..' server-side with a 400 Bad Request, making it impossible to create artifacts with path traversal names. This means the scenario this test was verifying (that gh run download catches traversal names) can no longer be reproduced through normal artifact creation. The client-side traversal check in gh run download remains in place as a defense-in-depth measure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Commit dd424d8 added NameWithOwner to PRRepository for agent-task listings but didn't update the headRepository GraphQL query to fetch it. This caused gh pr view --json headRepository to emit an empty "nameWithOwner":"" field, breaking the pr-create-respects-simple- pushdefault acceptance test. Fetch nameWithOwner in the query and update the test assertion to expect it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes multiple acceptance test failures by ensuring git commits work inside the testscript sandbox, correcting a GraphQL selection regression for headRepository.nameWithOwner, and removing an acceptance test scenario that can no longer be reproduced due to upstream API validation.
Changes:
- Write a minimal
.gitconfiginto the acceptance test sandbox sogit commitdoesn’t fail due to missing identity. - Fetch
nameWithOwnerforheadRepositoryin the PR GraphQL fragment and update the affected acceptance assertion. - Remove the obsolete
run-download-traversalacceptance test.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
acceptance/acceptance_test.go |
Sets up sandbox git identity via .gitconfig so commit-making scripts succeed. |
api/query_builder.go |
Extends headRepository GraphQL fragment to include nameWithOwner. |
acceptance/testdata/pr/pr-create-respects-simple-pushdefault.txtar |
Updates expected JSON output to include headRepository.nameWithOwner. |
acceptance/testdata/workflow/run-download-traversal.txtar |
Removes an acceptance test that’s no longer viable due to server-side artifact name validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "github.com/cli/cli/v2/internal/ghcmd" | ||
| "github.com/cli/go-internal/testscript" | ||
| "github.com/MakeNowJust/heredoc" | ||
| ) |
There was a problem hiding this comment.
Import block isn’t gofmt-sorted (third-party imports should be lexicographically ordered). As-is, the gofmt formatter (enabled in golangci-lint) will report this file; please run gofmt/goimports to normalize the import order.
There was a problem hiding this comment.
Well that's just not true. Our linter seems happy.
| case "headRepository": | ||
| q = append(q, `headRepository{id,name}`) | ||
| q = append(q, `headRepository{id,name,nameWithOwner}`) | ||
| case "assignees": |
There was a problem hiding this comment.
This change to the headRepository fragment isn’t currently covered by api/query_builder_test.go. Since there are already unit tests asserting the generated fragments for other fields, consider adding a case that asserts IssueGraphQL([]string{"headRepository"}) (or via PullRequestGraphQL) includes nameWithOwner to prevent regressions.
There was a problem hiding this comment.
Not super concerned with this personally.
There was a problem hiding this comment.
Fix 2 here looks like it breaks a lot of other tests, which are skipped by default right now:
- pr-create-push-default-upstream-no-merge-ref-fork.txtar:50
- pr-create-respects-push-destination.txtar:53
- pr-create-remote-ref-with-branch-name-slash.txtar:46
- pr-create-respects-remote-pushdefault.txtar:49
- pr-create-guesses-remote-from-sha.txtar:48
- pr-create-respects-user-colon-branch-syntax.txtar:47
- pr-create-guesses-remote-from-sha-with-branch-name-slash.txtar:50
- pr-create-respects-branch-pushremote.txtar:49
Or at least, maybe they were already broken? In any case, we should fix them.
|
I fixed these failures in 971be97 |
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://github.com/cli/cli) | minor | `v2.88.1` → `v2.89.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.89.0`](https://github.com/cli/cli/releases/tag/v2.89.0): GitHub CLI 2.89.0 [Compare Source](cli/cli@v2.88.1...v2.89.0) ####`gh agent-task` now works on ghe.com tenancies `gh agent-task` commands previously failed with `401 Unauthorized` for users on ghe.com tenancy hosts because the Copilot API URL was hardcoded. The URL is now resolved dynamically per host, so `gh agent-task` works correctly regardless of your GitHub hosting environment. #### Experimental new prompter A new TUI-based prompter powered by [charmbracelet/huh](https://github.com/charmbracelet/huh) is available behind the `GH_EXPERIMENTAL_PROMPTER` environment variable. This is an early preview — try it out and share feedback! ``` export GH_EXPERIMENTAL_PROMPTER=1 ``` #### `gh issue create` and `gh issue transfer` no longer require extra token scopes `gh issue create` and `gh issue transfer` previously fetched repository fields they didn't need, which could require additional token scopes. These commands now fetch only the minimal fields necessary for issue operations. #### What's Changed ##### ✨ Features - `gh pr create`, `gh issue create`, `gh issue edit`: search-based assignee selection and login-based mutation on github.com by [@​BagToad](https://github.com/BagToad) in [#​13009](cli/cli#13009) - Add experimental huh-only prompter gated by `GH_EXPERIMENTAL_PROMPTER` by [@​BagToad](https://github.com/BagToad) in [#​12859](cli/cli#12859) ##### 🐛 Fixes - fix(agent-task): resolve Copilot API URL dynamically for ghe.com tenancies by [@​BagToad](https://github.com/BagToad) in [#​12956](cli/cli#12956) - fix(issue): avoid fetching unnecessary fields in `issue create` and `issue transfer` by [@​babakks](https://github.com/babakks) in [#​12884](cli/cli#12884) - fix: resolve data race in codespaces port forwarder by [@​Lslightly](https://github.com/Lslightly) in [#​13033](cli/cli#13033) ##### 📚 Docs & Chores - Record agentic invocations in User-Agent header by [@​williammartin](https://github.com/williammartin) in [#​13023](cli/cli#13023) - docs: clarify that `gh pr edit --add-reviewer` can re-request reviews by [@​joshjohanning](https://github.com/joshjohanning) in [#​13021](cli/cli#13021) - Add AGENTS.md by [@​williammartin](https://github.com/williammartin) in [#​13024](cli/cli#13024) - Fix typo: remove extra space in README.md link by [@​realMelTuc](https://github.com/realMelTuc) in [#​12725](cli/cli#12725) - Align triage.md with current triage process by [@​tidy-dev](https://github.com/tidy-dev) in [#​13030](cli/cli#13030) - Remove auto-labels from issue templates by [@​tidy-dev](https://github.com/tidy-dev) in [#​12972](cli/cli#12972) - Consolidate actor-mode signals into `ApiActorsSupported` by [@​BagToad](https://github.com/BagToad) in [#​13025](cli/cli#13025) - Fix acceptance test failures: git identity, headRepository JSON, obsolete traversal test by [@​BagToad](https://github.com/BagToad) in [#​13037](cli/cli#13037) #####
Dependencies - chore(deps): bump google.golang.org/grpc from 1.79.2 to 1.79.3 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​12963](cli/cli#12963) - chore(deps): bump github.com/google/go-containerregistry from 0.20.7 to 0.21.3 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​12962](cli/cli#12962) - chore(deps): bump github.com/zalando/go-keyring from 0.2.6 to 0.2.8 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13031](cli/cli#13031) - chore(deps): bump microsoft/setup-msbuild from 2.0.0 to 3.0.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13005](cli/cli#13005) - chore(deps): bump mislav/bump-homebrew-formula-action from 3.6 to 4.1 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13004](cli/cli#13004) - chore(deps): bump azure/login from 2.3.0 to 3.0.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​12951](cli/cli#12951) #### New Contributors - [@​joshjohanning](https://github.com/joshjohanning) made their first contribution in [#​13021](cli/cli#13021) - [@​realMelTuc](https://github.com/realMelTuc) made their first contribution in [#​12725](cli/cli#12725) - [@​Lslightly](https://github.com/Lslightly) made their first contribution in [#​13033](cli/cli#13033) **Full Changelog**: [v2.88.1...v2.89.0](cli/cli@v2.88.1...v2.89.0) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My45MS40IiwidXBkYXRlZEluVmVyIjoiNDMuOTEuNCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90IiwiYXV0b21hdGlvbjpib3QtYXV0aG9yZWQiLCJkZXBlbmRlbmN5LXR5cGU6Om1pbm9yIl19-->
Description
Fixes several acceptance test failures found while running the suite locally. Each fix is in its own commit.
Fix 1: Git identity in testscript sandbox
The testscript sandbox overrides
HOME, so git inside the sandbox can't find the user's globaluser.name/user.email. This causes "Author identity unknown" errors in any acceptance test that makes commits (e.g. extensions, PRs). The fix writes a minimal.gitconfiginto the sandbox duringsharedSetup.Fix 2: Fetch
nameWithOwnerinheadRepositoryGraphQL querydd424d8 (agent-task listings) added
NameWithOwnerto thePRRepositorystruct but didn't update theheadRepositoryGraphQL query to fetch it. Sinceexport_pr.godumps the raw struct,gh pr view --json headRepositorystarted emitting"nameWithOwner":""- breaking thepr-create-respects-simple-pushdefaultacceptance test assertion.The fix adds
nameWithOwnerto the GraphQL query so the field is actually populated, and updates the test assertion to expect it.Fix 3: Remove
run-download-traversalacceptance testGitHub's Artifact API now rejects artifact names like
..server-side (400 Bad Request), making it impossible to create artifacts with path traversal names. The test scenario can no longer be reproduced through normal artifact creation. The client-side traversal check ingh run downloadremains as defense-in-depth.Known failures not addressed
These were observed during the acceptance run but are not bugs in
gh:TestExtensions/extensionTestRepo/repo-list-renameTestRepo/repo-archive-unarchiveTestRulesets/rulesetgh ruleset checkTestSecrets/secret-orgCommits
nameWithOwnerinheadRepositoryGraphQL query to fix regression from dd424d8 that brokepr-create-respects-simple-pushdefaultrun-download-traversaltest since GitHub's Artifact API now prevents the scenario server-side