diff --git a/plugin-tests/github-mcp/mcp_tool_gh.bats b/plugin-tests/github-mcp/mcp_tool_gh.bats index b3a4e7a..3f899b8 100644 --- a/plugin-tests/github-mcp/mcp_tool_gh.bats +++ b/plugin-tests/github-mcp/mcp_tool_gh.bats @@ -922,6 +922,119 @@ bats_test_function --description "pr_checks: fails without repo outside git" -- assert_output --partial "sort must be" } +# ============================================================================= +# search terms: search, search_repos, search_commits +# ============================================================================= + +# Print the terms _gh_split_search_terms produced, one per line. +split_search_terms() { + _gh_split_search_terms "$1" "search" required || return 1 + printf '%s\n' "${_GH_SEARCH_TERMS[@]}" +} + +@test "search terms: words split on any whitespace, a qualifier stays one term" { + run split_search_terms $'is:open timeout\thang\n' + assert_success + assert_output $'is:open\ntimeout\nhang' +} + +@test "search terms: a quoted phrase becomes one term without its quotes" { + run split_search_terms '"exact phrase" crash' + assert_success + assert_output $'exact phrase\ncrash' +} + +@test "search terms: a quoted qualifier value stays attached to its qualifier" { + run split_search_terms 'label:"good first issue" crash' + assert_success + assert_output $'label:good first issue\ncrash' +} + +@test "search terms: whitespace inside a quoted phrase collapses to single spaces" { + run split_search_terms $'"out\tof memory" crash' + assert_success + assert_output $'out of memory\ncrash' +} + +@test "search terms: an expression containing a unit separator is rejected" { + run split_search_terms $'out\x1fof memory' + assert_failure + assert_output --partial "unit separator" +} + +@test "search terms: an expression with no terms is rejected" { + run split_search_terms ' "" ' + assert_failure + assert_output --partial "no search terms" +} + +# Assert the exact argv a search tool hands gh, one argument per line. +# Usage: assert_search_argv +assert_search_argv() { + local fn="$1" args="$2" expected="$3" + gh() { + printf '%s\n' "$@" > "${BATS_TEST_TMPDIR}/gh_args" + echo '[]' + } + run "${fn}" "${args}" + assert_success + run cat "${BATS_TEST_TMPDIR}/gh_args" + assert_output "${expected}" +} + +_test_argv_search() { + assert_search_argv tool_search '{"search":"is:open timeout -label:bug"}' \ + $'search\nprs\n--repo\nshopware/shopware\n--limit\n20\n--\nis:open\ntimeout\n-label:bug' +} +_test_argv_search_repos() { + assert_search_argv tool_search_repos '{"search":"stars:>10 plugin -topic:php","owner":"shopware"}' \ + $'search\nrepos\n--owner\nshopware\n--limit\n20\n--json\nfullName,description,stargazersCount,language,updatedAt,url\n--\nstars:>10\nplugin\n-topic:php' +} +_test_argv_search_commits() { + assert_search_argv tool_search_commits '{"search":"author:mitelg fix cart -committer:bot"}' \ + $'search\ncommits\n--repo\nshopware/shopware\n--limit\n20\n--json\nsha,commit\n--\nauthor:mitelg\nfix\ncart\n-committer:bot' +} + +bats_test_function --description "search: each word is its own argument after --, a negated qualifier included" \ + -- _test_argv_search +bats_test_function --description "search_repos: each word is its own argument after --, a negated qualifier included" \ + -- _test_argv_search_repos +bats_test_function --description "search_commits: each word is its own argument after --, a negated qualifier included" \ + -- _test_argv_search_commits + +@test "search: type=issues passes each word to gh search issues" { + assert_search_argv tool_search '{"search":"timeout hang","type":"issues"}' \ + $'search\nissues\n--repo\nshopware/shopware\n--limit\n20\n--\ntimeout\nhang' +} + +@test "search_repos: a whitespace-only search runs the filter-only search" { + assert_search_argv tool_search_repos '{"search":" ","owner":"shopware"}' \ + $'search\nrepos\n--owner\nshopware\n--limit\n20\n--json\nfullName,description,stargazersCount,language,updatedAt,url' +} + +@test "search_code: the whole search is one argument after --" { + assert_search_argv tool_search_code '{"search":"->getId( foo","repo":"shopware/shopware"}' \ + $'search\ncode\n--repo\nshopware/shopware\n--limit\n30\n--json\nrepository,path,textMatches\n--\n->getId( foo' +} + +# Assert a search tool rejects an unbalanced double quote before calling gh. +# Usage: assert_unbalanced_quote_rejected +assert_unbalanced_quote_rejected() { + local fn="$1" + gh() { touch "${BATS_TEST_TMPDIR}/gh_called"; echo '[]'; } + run "${fn}" '{"search":"label:\"good first issue"}' + assert_failure + assert_output --partial "unbalanced double quote" + assert [ ! -e "${BATS_TEST_TMPDIR}/gh_called" ] +} + +bats_test_function --description "search: an unbalanced double quote is rejected before gh runs" \ + -- assert_unbalanced_quote_rejected tool_search +bats_test_function --description "search_repos: an unbalanced double quote is rejected before gh runs" \ + -- assert_unbalanced_quote_rejected tool_search_repos +bats_test_function --description "search_commits: an unbalanced double quote is rejected before gh runs" \ + -- assert_unbalanced_quote_rejected tool_search_commits + # ============================================================================= # search_discussions # ============================================================================= @@ -2113,21 +2226,6 @@ diff --git a/src/Third.php b/src/Third.php assert_output --partial "type must be" } -@test "search: type=issues uses gh search issues" { - gh() { - echo "$*" > "${BATS_TEST_TMPDIR}/captured_cmd" - echo '[]' - } - run tool_search '{"search":"bug","type":"issues"}' - assert_success - local captured_cmd - captured_cmd=$(cat "${BATS_TEST_TMPDIR}/captured_cmd") - [[ "${captured_cmd}" == *"search issues bug"* ]] || { - echo "Expected 'search issues bug' in command: ${captured_cmd}" - return 1 - } -} - @test "search: repo filter passed to gh" { gh() { echo "$*" > "${BATS_TEST_TMPDIR}/captured_cmd" diff --git a/plugins/github-mcp/CHANGELOG.md b/plugins/github-mcp/CHANGELOG.md index 760a71e..47da62d 100644 --- a/plugins/github-mcp/CHANGELOG.md +++ b/plugins/github-mcp/CHANGELOG.md @@ -9,6 +9,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `search`, `search_repos`, and `search_commits` now pass each word of `search` to `gh` as its own + keyword. Before, the whole expression went to `gh` as one argument, and `gh` turns an argument + with whitespace into one quoted phrase, reading any text before its first `:` as a qualifier name: + `timeout hang` searched for the phrase `"timeout hang"`, and `is:open timeout hang` became + `is:"open timeout hang"`. A double-quoted span such as `"exact phrase"` or + `label:"good first issue"` stays one term, with tabs and newlines inside it turned into spaces. A + `search` with an unbalanced double quote fails before `gh` runs, and so does a `search` without + words for `search` and `search_commits`; `search_repos` then searches by its filters alone. + `REFERENCE.md` lists the forms `gh` cannot pass through, such as a negated phrase. +- `search`, `search_repos`, `search_commits`, and `search_code` now put the search after `--`, so a + search that starts with `-`, such as `-label:bug crash` or the code `->getId(`, is no longer read + as a `gh` flag. `search_code` still sends the whole search as one exact-text phrase. - In pi, `tool_search` now ranks the requested gh-tooling tool first for most exact-name queries, such as `issue_view` (before: 5th). Names that share terms with another tool can still rank 2nd to 5th: `search`, `repo_file`, and with the write server `pr_comments`, `pr_reviews`, and diff --git a/plugins/github-mcp/REFERENCE.md b/plugins/github-mcp/REFERENCE.md index cb29ca5..940ab64 100644 --- a/plugins/github-mcp/REFERENCE.md +++ b/plugins/github-mcp/REFERENCE.md @@ -327,8 +327,11 @@ Search for issues or pull requests. Use gh-tooling search with search "NEXT-3412" and type "prs" Use gh-tooling search with search "custom field translation" and type "issues" and limit 20 Use gh-tooling search with search "attribute entity" and state "closed" +Use gh-tooling search with search "\"custom field\" label:\"good first issue\"" and type "issues" ``` +**Search expressions** (`search`, `search_repos`, `search_commits`): each word is its own keyword, so a result has to match all of them, in any order. Double quotes keep a phrase together (`"custom field"`), also as a qualifier value (`label:"good first issue"`). A `search` with an unmatched `"` fails before `gh` runs. `gh` cannot pass three forms through: a negated phrase (`-"exact phrase"` becomes a phrase that starts with `-`; use `NOT "exact phrase"`), a quoted single word (`"OR"` becomes the operator), and a phrase containing `:`. + ### `search_code` Search for code across GitHub repositories. Uses the legacy code search engine (no regex, no symbol search, no path globs). Rate limit: 10 requests/minute. @@ -341,7 +344,7 @@ Use gh-tooling search_code with search "addClass" and repo "shopware/shopware" a ``` **Parameters:** -- `search` (string, required): Code search expression (exact text match, no regex). +- `search` (string, required): Code search expression, matched as exact text (no regex). Unlike the other search tools, the whole value is one phrase. - `owner` (string, optional): Limit to repositories owned by this user/org. - `repo` (string, optional): Limit to this repository in `owner/repo` format. - `language` (string, optional): Filter by language (e.g. `php`, `typescript`). @@ -363,7 +366,7 @@ Use gh-tooling search_repos with topic "shopware" and limit 10 ``` **Parameters:** -- `search` (string, optional): Search text. +- `search` (string, optional): Search expression; see [Search expressions](#search). A value with no words searches by the filters alone. - `owner` (string, optional): Filter by owner. - `topic` (string, optional): Filter by topic tag. - `language` (string, optional): Filter by language. @@ -382,7 +385,7 @@ Use gh-tooling search_commits with search "fix cart" and author "mitelg" and aut ``` **Parameters:** -- `search` (string, required): Commit message search text. +- `search` (string, required): Commit message search expression; see [Search expressions](#search). - `repo` (string, optional): Limit to this repository in `owner/repo` format. - `owner` (string, optional): Limit to repositories owned by this user/org. - `author` (string, optional): Filter by commit author username. diff --git a/plugins/github-mcp/mcp-server-gh/lib/search.sh b/plugins/github-mcp/mcp-server-gh/lib/search.sh index 227308f..495062f 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/search.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/search.sh @@ -2,9 +2,85 @@ # Search tools for gh-tooling MCP server # Tools: search, search_code, search_repos, search_commits, search_discussions +####################################### +# Split a search expression into the keyword arguments gh search expects. +# gh quotes each argument that contains whitespace, so the whole expression +# passed as one argument would become a single phrase. Splits on spaces, tabs, +# and newlines; a double-quoted span stays in the term it belongs to, with its +# quotes removed and its whitespace collapsed to single spaces, so gh quotes it +# again: '"exact phrase"' -> 'exact phrase', 'label:"good first issue"' -> +# 'label:good first issue'. +# gh re-quotes only a term that contains whitespace, and reads the text before a +# term's first ':' as a qualifier name. So '-"exact phrase"' becomes the phrase +# '-exact phrase' rather than its negation, '"OR"' becomes the operator OR, and +# '"error: timeout"' does not reach GitHub as that phrase; gh has no argument +# form for any of them. +# Callers pass the terms after "--", so a term starting with '-' (-label:bug) +# is not read as a gh flag. +# Built on read's field splitting rather than a per-character loop, which takes +# quadratic time in bash. +# Globals: +# _GH_SEARCH_TERMS (set) +# Arguments: +# $1 search expression, +# $2 tool name, for error messages, +# $3 "required" to fail when no term remains, "optional" to allow none. +# Outputs: +# An error message on stdout on failure. +# Returns: +# 0 on success, 1 on an unbalanced double quote, a unit separator (U+001F) +# in the expression, or no term in required mode. +####################################### +_gh_split_search_terms() { + local search="$1" tool_name="$2" mode="$3" + local ws=$' \t\n' us=$'\x1f' + _GH_SEARCH_TERMS=() + + if [[ "${mode}" != "required" && "${mode}" != "optional" ]]; then + echo "Error: _gh_split_search_terms mode must be 'required' or 'optional', got: '${mode}'" + return 1 + fi + if [[ "${search}" == *"${us}"* ]]; then + echo "Error: search for ${tool_name} contains a unit separator (U+001F): '${search}'" + return 1 + fi + + # Fields at odd indexes were inside quotes. The appended quote terminates the + # last field; the field after it holds only the here-string's newline. + local -a parts words + IFS='"' read -r -d '' -a parts <<< "${search}\"" || true + unset 'parts[${#parts[@]}-1]' + if (( ${#parts[@]} % 2 == 0 )); then + echo "Error: search for ${tool_name} has an unbalanced double quote: '${search}'" + return 1 + fi + + # A quoted span's words are joined with the unit separator, so the split + # below keeps the span in one term; the separator becomes a space after. + local flat="" phrase i + for (( i = 0; i < ${#parts[@]}; i++ )); do + if (( i % 2 == 0 )); then + flat+="${parts[i]}" + continue + fi + IFS="${ws}" read -r -d '' -a words <<< "${parts[i]}" || true + if (( ${#words[@]} > 0 )); then + printf -v phrase "%s${us}" "${words[@]}" + flat+="${phrase%"${us}"}" + fi + done + + IFS="${ws}" read -r -d '' -a _GH_SEARCH_TERMS <<< "${flat}" || true + if (( ${#_GH_SEARCH_TERMS[@]} > 0 )); then + _GH_SEARCH_TERMS=("${_GH_SEARCH_TERMS[@]//"${us}"/ }") + elif [[ "${mode}" == "required" ]]; then + echo "Error: search for ${tool_name} has no search terms: '${search}'" + return 1 + fi +} + # Search for GitHub issues or pull requests using a search expression. -# Maps to: gh search issues|prs [--repo] [--state] [--limit] [--json] -# Also supports the low-level: gh api search/issues -X GET -f q="..." -f per_page=N +# Maps to: gh search issues|prs [--repo] [--state] [--limit] [--json] -- tool_search() { local args="$1" @@ -39,8 +115,9 @@ tool_search() { effective_repo=$(_gh_resolve_repo "${repo}") _gh_validate_number "${limit}" "limit" || return 1 + _gh_split_search_terms "${search}" "search" required || return 1 - local -a cmd=("gh" "search" "${type}" "${search}") + local -a cmd=("gh" "search" "${type}") if [[ -n "${effective_repo}" ]]; then _gh_validate_repo "${effective_repo}" || return 1 @@ -50,6 +127,7 @@ tool_search() { [[ -n "${state}" ]] && cmd+=("--state" "${state}") cmd+=("--limit" "${limit}") [[ -n "${fields}" ]] && cmd+=("--json" "${fields}") + cmd+=("--" "${_GH_SEARCH_TERMS[@]}") log "INFO" "search: ${cmd[*]}" local __raw __exit=0 @@ -68,7 +146,7 @@ tool_search() { # Search for code across GitHub repositories. # Uses the legacy code search engine (no regex, no symbol search, no path globs). # Rate limit: 10 requests/minute (separate bucket from other search endpoints). -# Maps to: gh search code [--repo] [--language] [--extension] [--filename] [--match] [--limit] [--json] +# Maps to: gh search code [--repo] [--owner] [--language] [--extension] [--filename] [--match] [--limit] [--json] -- tool_search_code() { local args="$1" @@ -110,7 +188,7 @@ tool_search_code() { _gh_validate_grep_pattern "${grep_pattern}" || return 1 _gh_validate_number "${limit}" "limit" || return 1 - local -a cmd=("gh" "search" "code" "${search}") + local -a cmd=("gh" "search" "code") # Resolve repo: explicit param > GH_DEFAULT_REPO (consistent with tool_search) local effective_repo @@ -135,6 +213,9 @@ tool_search_code() { local default_fields="repository,path,textMatches" [[ -n "${fields}" ]] && cmd+=("--json" "${fields}") || cmd+=("--json" "${default_fields}") + # One argument, so gh quotes it as the exact text match the tool promises; + # "--" keeps a search such as "->getId(" from being read as a flag. + cmd+=("--" "${search}") log "INFO" "search_code: ${cmd[*]}" local __raw __exit=0 @@ -177,7 +258,7 @@ tool_search_code() { # Search for GitHub repositories. # Query is optional — filters alone (owner, topic, language, stars) suffice. -# Maps to: gh search repos [search] [--owner] [--topic] [--language] [--license] [--stars] [--sort] [--limit] [--json] +# Maps to: gh search repos [--owner] [--topic] [--language] [--license] [--stars] [--sort] [--limit] [--json] [-- ] tool_search_repos() { local args="$1" @@ -209,9 +290,9 @@ tool_search_repos() { _gh_validate_jq_filter "${jq_filter}" || return 1 _gh_validate_number "${limit}" "limit" || return 1 + _gh_split_search_terms "${search}" "search_repos" optional || return 1 local -a cmd=("gh" "search" "repos") - [[ -n "${search}" ]] && cmd+=("${search}") [[ -n "${owner}" ]] && cmd+=("--owner" "${owner}") [[ -n "${topic}" ]] && cmd+=("--topic" "${topic}") [[ -n "${language}" ]] && cmd+=("--language" "${language}") @@ -222,6 +303,7 @@ tool_search_repos() { local default_fields="fullName,description,stargazersCount,language,updatedAt,url" [[ -n "${fields}" ]] && cmd+=("--json" "${fields}") || cmd+=("--json" "${default_fields}") + [[ ${#_GH_SEARCH_TERMS[@]} -gt 0 ]] && cmd+=("--" "${_GH_SEARCH_TERMS[@]}") log "INFO" "search_repos: ${cmd[*]}" local __raw __exit=0 @@ -238,7 +320,7 @@ tool_search_repos() { } # Search for GitHub commits. -# Maps to: gh search commits [--repo] [--owner] [--author] [--committer] [--author-date] [--committer-date] [--hash] [--merge] [--sort] [--limit] [--json] +# Maps to: gh search commits [--repo] [--owner] [--author] [--committer] [--author-date] [--committer-date] [--hash] [--merge] [--sort] [--limit] [--json] -- tool_search_commits() { local args="$1" @@ -277,8 +359,9 @@ tool_search_commits() { _gh_validate_jq_filter "${jq_filter}" || return 1 _gh_validate_number "${limit}" "limit" || return 1 + _gh_split_search_terms "${search}" "search_commits" required || return 1 - local -a cmd=("gh" "search" "commits" "${search}") + local -a cmd=("gh" "search" "commits") # Resolve repo: explicit param > GH_DEFAULT_REPO (consistent with tool_search) local effective_repo @@ -307,6 +390,7 @@ tool_search_commits() { local default_fields="sha,commit" [[ -n "${fields}" ]] && cmd+=("--json" "${fields}") || cmd+=("--json" "${default_fields}") + cmd+=("--" "${_GH_SEARCH_TERMS[@]}") log "INFO" "search_commits: ${cmd[*]}" local __raw __exit=0 diff --git a/plugins/github-mcp/mcp-server-gh/tools-read.json b/plugins/github-mcp/mcp-server-gh/tools-read.json index 6127089..a15f8ad 100644 --- a/plugins/github-mcp/mcp-server-gh/tools-read.json +++ b/plugins/github-mcp/mcp-server-gh/tools-read.json @@ -1091,7 +1091,7 @@ }, "search": { "type": "string", - "description": "Search expression (e.g. 'NEXT-3412', 'attribute entity', 'custom field translated')." + "description": "Search expression. Each word is its own keyword; double quotes keep a phrase together (e.g. 'NEXT-3412', 'attribute entity', '\"custom field\" translated', 'label:\"good first issue\"'). An unmatched double quote fails the call." } }, "additionalProperties": false @@ -1198,7 +1198,7 @@ }, "search": { "type": "string", - "description": "Code search expression (exact text match, no regex). Example: 'addClass', 'extends AbstractController'." + "description": "Code search expression, matched as exact text including spaces (no regex). Example: 'addClass', 'extends AbstractController'." } }, "additionalProperties": false @@ -1269,7 +1269,7 @@ }, "search": { "type": "string", - "description": "Search expression (optional when using filters). Example: 'shopware plugin', 'ecommerce'." + "description": "Search expression (optional when using filters). Each word is its own keyword; double quotes keep a phrase together. Example: 'shopware plugin', '\"headless commerce\"'. An unmatched double quote fails the call." } }, "additionalProperties": false @@ -1348,7 +1348,7 @@ }, "search": { "type": "string", - "description": "Commit message search text (e.g. 'NEXT-1234', 'fix cart calculation')." + "description": "Commit message search expression. Each word is its own keyword; double quotes keep a phrase together (e.g. 'NEXT-1234', 'fix cart', '\"fix cart calculation\"'). An unmatched double quote fails the call." } }, "additionalProperties": false