Fix query object state mutation during pagination #11244
Merged
+229
−3
Conversation
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
Signed-off-by: Babak K. Shandiz <[email protected]>
Signed-off-by: Babak K. Shandiz <[email protected]>
Signed-off-by: Babak K. Shandiz <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR fixes mutation of the original query slice during pagination and adds tests to ensure quoted multi-word keywords are handled correctly across search endpoints and URL generation.
- Refactor formatKeywords to return a new slice instead of mutating the input
- Add pagination test cases for multi-word queries in code, commits, repositories, and issues
- Add a URL-encoding test for quoted multi-word keywords
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/search/query.go | Changed formatKeywords to allocate a new result slice rather than mutating the input |
| pkg/search/searcher_test.go | Added tests for paginating with quoted multi-word queries and URL encoding |
andyfeller
approved these changes
Jul 7, 2025
Comment on lines
151
to
+161
| func formatKeywords(ks []string) []string { | ||
| result := make([]string, len(ks)) | ||
| for i, k := range ks { | ||
| before, after, found := strings.Cut(k, ":") | ||
| if !found { | ||
| ks[i] = quote(k) | ||
| result[i] = quote(k) | ||
| } else { | ||
| ks[i] = fmt.Sprintf("%s:%s", before, quote(after)) | ||
| result[i] = fmt.Sprintf("%s:%s", before, quote(after)) | ||
| } | ||
| } | ||
| return ks | ||
| return result |
There was a problem hiding this comment.
note: just to clarify, this function had a side effect of modifying Query.Keywords field which is used else where. Refactoring this logic to not modify the input avoids the redundant formatting issue.
tmeijn
pushed a commit
to tmeijn/dotfiles
that referenced
this pull request
Jul 27, 2025
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://github.com/cli/cli) | minor | `v2.74.2` -> `v2.76.1` | 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.76.1`](https://github.com/cli/cli/releases/tag/v2.76.1): GitHub CLI 2.76.1 [Compare Source](cli/cli@v2.76.0...v2.76.1) #### `gh pr create` regression fix This release fixes a regression introduced in `v2.76.0` where organization teams were retrieved outside of intentional use cases. This caused problems for GitHub Enterprise Server users using the GitHub Actions automatic token that does not have access to organization teams. For more information, see cli/cli#11360 #### What's Changed ##### 🐛 Fixes - Fix: `gh pr create`, only fetch teams when reviewers contain a team by [@​BagToad](https://github.com/BagToad) in cli/cli#11361 ##### 📚 Docs & Chores - add tenancy aware for san matcher by [@​ejahnGithub](https://github.com/ejahnGithub) in cli/cli#11261 - Run Lint and Tests on `push` to `trunk` branch by [@​andyfeller](https://github.com/andyfeller) in cli/cli#11325 - update ownership of pkg/cmd/release/shared/ by [@​ejahnGithub](https://github.com/ejahnGithub) in cli/cli#11326 - Automate spam issue detection by [@​babakks](https://github.com/babakks) in cli/cli#11316 - Improve `api` `--preview` docs by [@​jsoref](https://github.com/jsoref) in cli/cli#11274 - Incorporate govulncheck into workflows by [@​andyfeller](https://github.com/andyfeller) in cli/cli#11332 - chore(deps): bump advanced-security/filter-sarif from 1.0.0 to 1.0.1 by [@​dependabot](https://github.com/dependabot)\[bot] in cli/cli#11298 - chore(deps): bump github.com/sigstore/sigstore-go from 1.0.0 to 1.1.0 by [@​dependabot](https://github.com/dependabot)\[bot] in cli/cli#11307 **Full Changelog**: cli/cli@v2.76.0...v2.76.1 ### [`v2.76.0`](https://github.com/cli/cli/releases/tag/v2.76.0): GitHub CLI 2.76.0 [Compare Source](cli/cli@v2.75.1...v2.76.0) ####Copilot Coding Agent Support GitHub Copilot Pro+ and Copilot Enterprise subscribers can now assign issues to GitHub Copilot during issue creation using: - Command-line flag: `gh issue create --assignee @​copilot` - Launching web browser: `gh issue create --assignee @​copilot --web` - Or interactively selecting `Copilot (AI)` as assignee in `gh issue create` metadata For more details, refer to [the full changelog post for Copilot coding agent](https://github.blog/changelog/2025-05-19-github-copilot-coding-agent-in-public-preview/). #### What's Changed ##### ✨ Features - Assign Copilot during `gh issue create` by [@​andyfeller](https://github.com/andyfeller) in cli/cli#11279 - Display immutable field in `release view` command by [@​bdehamer](https://github.com/bdehamer) in cli/cli#11251 ##### 🐛 Fixes - FIX: Do not fetch logs for skipped jobs by [@​babakks](https://github.com/babakks) in cli/cli#11312 - Transform `extension` and `filename` qualifiers into `path` qualifier for web code search by [@​samcoe](https://github.com/samcoe) in cli/cli#11211 ##### 📚 Docs & Chores - FIX: Workflow does not contain permissions by [@​BagToad](https://github.com/BagToad) in cli/cli#11322 - Add automated feature request response workflow by [@​BagToad](https://github.com/BagToad) in cli/cli#11299 **Full Changelog**: cli/cli@v2.75.1...v2.76.0 ### [`v2.75.1`](https://github.com/cli/cli/releases/tag/v2.75.1): GitHub CLI 2.75.1 [Compare Source](cli/cli@v2.75.0...v2.75.1) #### What's Changed ##### 🐛 Fixes - Ensure hostnames are visible in CLI website by [@​andyfeller](https://github.com/andyfeller) in cli/cli#11295 - Revert "Fix: `gh pr create` prioritize `--title` and `--body` over `--fill` when `--web` is present" by [@​andyfeller](https://github.com/andyfeller) in cli/cli#11300 ##### 📚 Docs & Chores - Ensure go directive is always .0 version in bump by [@​williammartin](https://github.com/williammartin) in cli/cli#11259 - Minor (1-word) documentation typo in generated `~/.config/gh/config.yml` by [@​kurahaupo](https://github.com/kurahaupo) in cli/cli#11246 - Automate closing of stale issues by [@​babakks](https://github.com/babakks) in cli/cli#11268 - Filter the `third-party/` folder out of CodeQL results by [@​BagToad](https://github.com/BagToad) in cli/cli#11278 - Exclude `third-party` source from golangci-lint by [@​andyfeller](https://github.com/andyfeller) in cli/cli#11293 #####
Dependencies - Bump Go to 1.24.5 by [@​github-actions](https://github.com/github-actions)\[bot] in cli/cli#11255 - chore(deps): bump github.com/sigstore/protobuf-specs from 0.4.3 to 0.5.0 by [@​dependabot](https://github.com/dependabot)\[bot] in cli/cli#11263 - chore(deps): bump golang.org/x/term from 0.32.0 to 0.33.0 by [@​dependabot](https://github.com/dependabot)\[bot] in cli/cli#11266 - chore(deps): bump golang.org/x/sync from 0.15.0 to 0.16.0 by [@​dependabot](https://github.com/dependabot)\[bot] in cli/cli#11264 - chore(deps): bump golang.org/x/text from 0.26.0 to 0.27.0 by [@​dependabot](https://github.com/dependabot)\[bot] in cli/cli#11265 - chore(deps): bump golang.org/x/crypto from 0.39.0 to 0.40.0 by [@​dependabot](https://github.com/dependabot)\[bot] in cli/cli#11275 #### New Contributors - [@​kurahaupo](https://github.com/kurahaupo) made their first contribution in cli/cli#11246 - [@​github-actions](https://github.com/github-actions)\[bot] made their first contribution in cli/cli#11255 **Full Changelog**: cli/cli@v2.75.0...v2.75.1 ### [`v2.75.0`](https://github.com/cli/cli/releases/tag/v2.75.0): GitHub CLI 2.75.0 [Compare Source](cli/cli@v2.74.2...v2.75.0) #### What's Changed ##### ✨ Features - init release verify subcommands by [@​ejahnGithub](https://github.com/ejahnGithub) in cli/cli#11018 - Embed Windows resources (VERSIONINFO) during build by [@​babakks](https://github.com/babakks) in cli/cli#11048 - Support `--no-repos-selected` on `gh secret set` by [@​williammartin](https://github.com/williammartin) in cli/cli#11217 ##### 🐛 Fixes - Fix: `gh pr create` prioritize `--title` and `--body` over `--fill` when `--web` is present by [@​dankrzeminski32](https://github.com/dankrzeminski32) in cli/cli#10547 - fix: get token for active user instead of blank if possible by [@​anuraaga](https://github.com/anuraaga) in cli/cli#11038 - Use Actions API to retrieve job run logs as a fallback mechanism by [@​babakks](https://github.com/babakks) in cli/cli#11172 - Fix query object state mutation during pagination by [@​babakks](https://github.com/babakks) in cli/cli#11244 - Handle `HTTP 404` when deleting remote branch in `pr merge` by [@​babakks](https://github.com/babakks) in cli/cli#11234 ##### 📚 Docs & Chores - chore: fix function name by [@​jinjingroad](https://github.com/jinjingroad) in cli/cli#11149 - chore: update Go version to 1.24 in devcontainer configuration and docs by [@​tMinamiii](https://github.com/tMinamiii) in cli/cli#11158 - Ensure lint workflow checks whether 3rd party license and code is up to date by [@​andyfeller](https://github.com/andyfeller) in cli/cli#11047 - docs: install\_linux.md: add Solus linux install instructions by [@​chax](https://github.com/chax) in cli/cli#10823 - Fix missing newline in install\_linux.md by [@​BagToad](https://github.com/BagToad) in cli/cli#11160 - Ensure automation uses pinned go-licenses version by [@​andyfeller](https://github.com/andyfeller) in cli/cli#11161 - Add `workflow_dispatch` support to MR Help Wanted check by [@​BagToad](https://github.com/BagToad) in cli/cli#11179 - Remove unused `GH_TOKEN` env variable from workflow by [@​BagToad](https://github.com/BagToad) in cli/cli#11190 - Add workflow to automate go version bumping by [@​williammartin](https://github.com/williammartin) in cli/cli#11189 - Fix inconsistent use of tabs and spaces by [@​Stefan-Heimersheim](https://github.com/Stefan-Heimersheim) in cli/cli#11194 - Decouple arg parsing from MR finder by [@​babakks](https://github.com/babakks) in cli/cli#11192 - docs: consistently use `apt` in installation instructions by [@​tklauser](https://github.com/tklauser) in cli/cli#11216 - Ensure bump go script has git user configured by [@​williammartin](https://github.com/williammartin) in cli/cli#11229 - Inject token into bump-go workflow by [@​williammartin](https://github.com/williammartin) in cli/cli#11233 - Reinstating Primer Style CLI content within `cli/cli` repository by [@​andyfeller](https://github.com/andyfeller) in cli/cli#11060 - Add setup-go to bump-go workflow by [@​williammartin](https://github.com/williammartin) in cli/cli#11237 - Ensure GoReleaser does not break on Mac OS and Linux when skipping Windows `.rsyso` generation script by [@​andyfeller](https://github.com/andyfeller) in cli/cli#11257 #####
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.
Fixes #11228
This PR fixes the issue with mutating original query object during pagination API calls. Test cases were added to assert the expected behaviour.
Verifying A/C
Given I have a query that contains a whitespace character, and my query will result in several pages of pagination
When I run the
gh searchcommand (for any subcommand)Then my query should complete without error
I found a simple use case to verify the expected behaviour. It's looking for the abundant Lorem Ipsum phrase with a grouping of words that forces
ghto quote them:This works fine with the changes in this PR, but if we run it with the latest
gh(or on thetrunkbranch) we'll get the following error: