diff --git a/plugin-tests/github-mcp/mcp_tool_gh.bats b/plugin-tests/github-mcp/mcp_tool_gh.bats index 3f899b8..177dff2 100644 --- a/plugin-tests/github-mcp/mcp_tool_gh.bats +++ b/plugin-tests/github-mcp/mcp_tool_gh.bats @@ -2004,21 +2004,147 @@ diff --git a/src/Third.php b/src/Third.php } } -@test "issue_view: with_comments adds --comments flag" { +# ============================================================================= +# issue_view with_comments / pr_view comments +# Without a TTY, gh's view --comments prints only the comments, so these tools +# make a plain call for the preview and a --comments call for the comments. +# ============================================================================= + +# Stub gh as a view command: the preview without --comments, and VIEW_COMMENTS +# (exit VIEW_COMMENTS_EXIT) with it, the way gh prints them without a TTY. +# Each call's arguments go to gh_calls, one call per line. +_stub_gh_view() { + VIEW_COMMENTS="$1" + VIEW_COMMENTS_EXIT="${2:-0}" gh() { - echo "$*" > "${BATS_TEST_TMPDIR}/captured_cmd" - echo 'issue body' + echo "$*" >> "${BATS_TEST_TMPDIR}/gh_calls" + if [[ " $* " == *" --comments "* ]]; then + if [[ "${VIEW_COMMENTS_EXIT}" -ne 0 ]]; then + echo "comments request failed" >&2 + return "${VIEW_COMMENTS_EXIT}" + fi + [[ -z "${VIEW_COMMENTS}" ]] || printf '%s\n' "${VIEW_COMMENTS}" + return 0 + fi + printf '%s\n' "title: Example" "state: OPEN" "--" "Example body" } - run tool_issue_view '{"number":"42","with_comments":true}' +} + +# Usage: assert_view_without_comments +assert_view_without_comments() { + _stub_gh_view "" + run "$1" "$2" assert_success - local captured_cmd - captured_cmd=$(cat "${BATS_TEST_TMPDIR}/captured_cmd") - [[ "${captured_cmd}" == *"--comments"* ]] || { - echo "Expected --comments in command: ${captured_cmd}" - return 1 + assert_output $'title:\tExample\nstate:\tOPEN\n--\nExample body' +} + +# Usage: assert_view_with_comments +assert_view_with_comments() { + _stub_gh_view $'author:\tmonalisa\n--\nFirst comment' + run "$1" "$2" + assert_success + assert_output "$(printf 'title:\tExample\nstate:\tOPEN\n--\nExample body\n\n%s\nauthor:\tmonalisa\n--\nFirst comment' "$3")" +} + +# Usage: assert_view_comments_failure +assert_view_comments_failure() { + _stub_gh_view "" 1 + run "$1" "$2" + assert_failure + assert_output "comments request failed" +} + +# A gh warning on stderr is not a comment: an item without comments still +# comes back without a separator. +# Usage: assert_view_warning_not_a_comment +assert_view_warning_not_a_comment() { + gh() { + echo "warning: rate limit low" >&2 + [[ " $* " == *" --comments "* ]] || echo "Example body" } + run "$1" "$2" + assert_success + assert_output "Example body" } +# A failed preview call is reported as is, and the comments call never runs. +# Usage: assert_view_preview_failure +assert_view_preview_failure() { + gh() { + echo "$*" >> "${BATS_TEST_TMPDIR}/gh_calls" + echo "could not resolve to an issue" >&2 + return 1 + } + run "$1" "$2" + assert_failure + assert_output "could not resolve to an issue" + run grep -c -- "--comments" "${BATS_TEST_TMPDIR}/gh_calls" + assert_output "0" +} + +# Usage: assert_view_comments_failure_fallback +assert_view_comments_failure_fallback() { + _stub_gh_view "" 1 + run "$1" "$2" + assert_success + assert_output "no data" +} + +# Usage: assert_view_comments_call_args +assert_view_comments_call_args() { + _stub_gh_view "First comment" + run "$1" "$2" + assert_success + run tail -n 1 "${BATS_TEST_TMPDIR}/gh_calls" + assert_output "$3" +} + +# With fields, gh returns JSON and the flag does not apply: one call, no --comments. +# Usage: assert_view_fields_skip_comments +assert_view_fields_skip_comments() { + _stub_gh_view "First comment" + run "$1" "$2" + assert_success + run cat "${BATS_TEST_TMPDIR}/gh_calls" + assert_output "$3" +} + +_test_issue_view_warning() { assert_view_warning_not_a_comment tool_issue_view '{"number":"42","with_comments":true}'; } +_test_pr_view_warning() { assert_view_warning_not_a_comment tool_pr_view '{"number":"123","comments":true}'; } +_test_issue_view_preview_failed() { assert_view_preview_failure tool_issue_view '{"number":"42","with_comments":true}'; } +_test_pr_view_preview_failed() { assert_view_preview_failure tool_pr_view '{"number":"123","comments":true}'; } +_test_issue_view_comments_fallback() { assert_view_comments_failure_fallback tool_issue_view '{"number":"42","with_comments":true,"fallback":"no data"}'; } +_test_pr_view_comments_fallback() { assert_view_comments_failure_fallback tool_pr_view '{"number":"123","comments":true,"fallback":"no data"}'; } +_test_issue_view_comments_args() { assert_view_comments_call_args tool_issue_view '{"number":"42","repo":"acme/app","with_comments":true}' "issue view 42 --repo acme/app --comments"; } +_test_pr_view_comments_args() { assert_view_comments_call_args tool_pr_view '{"number":"123","repo":"acme/app","comments":true}' "pr view 123 --repo acme/app --comments"; } +_test_issue_view_fields_comments() { assert_view_fields_skip_comments tool_issue_view '{"number":"42","repo":"acme/app","fields":"title","with_comments":true}' "issue view 42 --repo acme/app --json title"; } +_test_pr_view_fields_comments() { assert_view_fields_skip_comments tool_pr_view '{"number":"123","repo":"acme/app","fields":"title","comments":true}' "pr view 123 --repo acme/app --json title"; } + +bats_test_function --description "issue_view: with_comments does not count a gh warning as a comment" -- _test_issue_view_warning +bats_test_function --description "pr_view: comments does not count a gh warning as a comment" -- _test_pr_view_warning +bats_test_function --description "issue_view: with_comments reports a failed issue call without fetching comments" -- _test_issue_view_preview_failed +bats_test_function --description "pr_view: comments reports a failed PR call without fetching comments" -- _test_pr_view_preview_failed +bats_test_function --description "issue_view: with_comments returns fallback when the comments call fails" -- _test_issue_view_comments_fallback +bats_test_function --description "pr_view: comments returns fallback when the comments call fails" -- _test_pr_view_comments_fallback +bats_test_function --description "issue_view: the comments call targets the same issue and repo" -- _test_issue_view_comments_args +bats_test_function --description "pr_view: the comments call targets the same PR and repo" -- _test_pr_view_comments_args +bats_test_function --description "issue_view: with_comments does not apply when fields is set" -- _test_issue_view_fields_comments +bats_test_function --description "pr_view: comments does not apply when fields is set" -- _test_pr_view_fields_comments + +_test_issue_view_no_comments() { assert_view_without_comments tool_issue_view '{"number":"42","with_comments":true}'; } +_test_issue_view_comments() { assert_view_with_comments tool_issue_view '{"number":"42","with_comments":true}' "--- comments ---"; } +_test_issue_view_comments_failed() { assert_view_comments_failure tool_issue_view '{"number":"42","with_comments":true}'; } +_test_pr_view_no_comments() { assert_view_without_comments tool_pr_view '{"number":"123","comments":true}'; } +_test_pr_view_comments() { assert_view_with_comments tool_pr_view '{"number":"123","comments":true}' "--- comments and reviews ---"; } +_test_pr_view_comments_failed() { assert_view_comments_failure tool_pr_view '{"number":"123","comments":true}'; } + +bats_test_function --description "issue_view: with_comments on an issue without comments returns the issue" -- _test_issue_view_no_comments +bats_test_function --description "issue_view: with_comments appends the comments after the issue" -- _test_issue_view_comments +bats_test_function --description "issue_view: with_comments fails when the comments call fails" -- _test_issue_view_comments_failed +bats_test_function --description "pr_view: comments on a PR without comments returns the PR" -- _test_pr_view_no_comments +bats_test_function --description "pr_view: comments appends the comments and reviews after the PR" -- _test_pr_view_comments +bats_test_function --description "pr_view: comments fails when the comments call fails" -- _test_pr_view_comments_failed + @test "issue_view: jq_filter applied to output" { GH_STUB_OUTPUT='{"title":"my issue","state":"open"}' run tool_issue_view '{"number":"42","fields":"title,state","jq_filter":".title"}' diff --git a/plugins/github-mcp/CHANGELOG.md b/plugins/github-mcp/CHANGELOG.md index 47da62d..a234d49 100644 --- a/plugins/github-mcp/CHANGELOG.md +++ b/plugins/github-mcp/CHANGELOG.md @@ -21,6 +21,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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. +- `issue_view` with `with_comments: true` and `pr_view` with `comments: true` now return the + issue or PR (title, state, labels, body, and the other fields gh prints), followed by a + `--- comments ---` (issue) or `--- comments and reviews ---` (PR) line and the comments, or only + the issue or PR when it has none. Before, they returned only the comments, and nothing for an item + without comments: run without a terminal, `gh issue view --comments` and `gh pr view --comments` + print the comments alone. The tools now make one call for the item and a second with `--comments`; + a failed second call fails the tool call, or returns `fallback` when set. A warning gh prints on + stderr stays out of the result. `max_lines` now counts the item and its comments together, where + before it applied to the comments alone. `REFERENCE.md` lists the known limits of the two calls. - 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 940ab64..76bedc8 100644 --- a/plugins/github-mcp/REFERENCE.md +++ b/plugins/github-mcp/REFERENCE.md @@ -64,7 +64,7 @@ Use gh-tooling pr_view with number 14642 and comments true - `number` (integer, required): PR number. - Repository selection: see [Repository selection](#repository-selection). - `fields` (string, optional): Comma-separated JSON fields (e.g. `title,body,state,reviews,files`) -- `comments` (boolean, optional): Include PR comments in text output. +- `comments` (boolean, optional): Append the PR's comments and reviews to the text output, after a `--- comments and reviews ---` line. Ignored when `fields` is set. `max_lines` counts the PR and its comments together. The limits listed under [`issue_view`](#issue_view) apply here too. ### `pr_diff` @@ -151,6 +151,22 @@ Use gh-tooling issue_view with number 8498 and fields "title,body,state,labels,c Use gh-tooling issue_view with number 8498 and with_field_values true ``` +`with_comments: true` appends the issue's comments to the text output, after a +`--- comments ---` line. It is ignored when `fields` is set, and rejected together +with `with_field_values`. `max_lines` counts the issue and its comments together, +so a long body can leave the comments out. + +The issue and its comments come from two `gh` calls. If the comments call fails, +the whole call fails, or returns `fallback`; the tool never returns the issue +without its comments. Known limits, which apply to `pr_view` with `comments` too: + +- A comment posted or deleted between the two calls makes the issue's + `comments:` count disagree with the list that follows. +- `gh` leaves hidden (minimized) comments out of the list, so an issue whose only + comments are hidden shows a count but no separator. +- The separator is plain text: a body that contains the same line reads as the + start of the comments. + `with_field_values` returns the issue's type and its field values keyed by field name, which `gh issue view` exposes for neither: diff --git a/plugins/github-mcp/mcp-server-gh/lib/common.sh b/plugins/github-mcp/mcp-server-gh/lib/common.sh index 2889ef9..5622019 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/common.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/common.sh @@ -219,6 +219,47 @@ _gh_capture_split() { return "${__gh_cs_exit}" } +####################################### +# Run a gh issue/pr view command for the item's text preview, then again with +# --comments for its comments. Without a TTY, gh prints only the comments when +# given --comments, so the preview needs a call of its own. Each call's stderr +# is kept out of its output, so a gh warning can never pass for a comment. +# Known limits: the two calls are separate requests, so a comment posted +# between them can make the preview's comment count disagree with the list; +# gh leaves hidden (minimized) comments out of the list. +# Arguments: +# $1 tool name, for the log, +# $2 separator line put between the preview and the comments, +# $3... the view command and its arguments, without --comments. +# Outputs: +# The preview, then a blank line, the separator, and the comments when gh +# printed any. On failure, the failed call's stderr, or its stdout when +# stderr is empty. +# Returns: +# 0 when both calls succeed, otherwise the failed call's exit status. +####################################### +_gh_view_with_comments() { + local tool_name="$1" separator="$2" + shift 2 + local preview comments err exit_code=0 + + _gh_capture_split preview err "$@" || exit_code=$? + if [[ ${exit_code} -ne 0 ]]; then + printf '%s\n' "${err:-${preview}}" + return "${exit_code}" + fi + + log "INFO" "${tool_name}: $* --comments" + _gh_capture_split comments err "$@" --comments || exit_code=$? + if [[ ${exit_code} -ne 0 ]]; then + printf '%s\n' "${err:-${comments}}" + return "${exit_code}" + fi + + printf '%s\n' "${preview}" + [[ -z "${comments}" ]] || printf '\n%s\n%s\n' "${separator}" "${comments}" +} + ####################################### # Resolve the organization owning org-level resources (issue types, issue fields). # Priority: org > owner > repo-shaped args > GH_DEFAULT_REPO > git remote. diff --git a/plugins/github-mcp/mcp-server-gh/lib/issue.sh b/plugins/github-mcp/mcp-server-gh/lib/issue.sh index e9d03ff..494875b 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/issue.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/issue.sh @@ -3,7 +3,8 @@ # Tools: issue_view, issue_list # View a GitHub issue with optional comments, type, and field values. -# Maps to: gh issue view [--repo owner/repo] [--json ] [--comments] +# Maps to: gh issue view [--repo owner/repo] [--json ], +# then gh issue view [--repo owner/repo] --comments for comments, # and gh api repos//issues/ for the type and field values tool_issue_view() { local args="$1" @@ -61,13 +62,13 @@ tool_issue_view() { if [[ -n "${fields}" ]]; then cmd+=("--json" "${fields}") - elif [[ "${with_comments}" == "true" ]]; then - cmd+=("--comments") fi log "INFO" "issue_view: ${cmd[*]}" local __raw __exit=0 - if [[ "${suppress_errors}" == "true" ]]; then + if [[ -z "${fields}" && "${with_comments}" == "true" ]]; then + __raw=$(_gh_view_with_comments "issue_view" "--- comments ---" "${cmd[@]}") || __exit=$? + elif [[ "${suppress_errors}" == "true" ]]; then __raw=$("${cmd[@]}" 2>/dev/null) || __exit=$? else __raw=$("${cmd[@]}" 2>&1) || __exit=$? diff --git a/plugins/github-mcp/mcp-server-gh/lib/pr.sh b/plugins/github-mcp/mcp-server-gh/lib/pr.sh index 4cbf7a4..5fe320d 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/pr.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/pr.sh @@ -3,7 +3,8 @@ # Tools: pr_view, pr_diff, pr_list, pr_checks, pr_comments, pr_reviews, pr_files, pr_commits # View pull request details. -# Maps to: gh pr view [] [--repo owner/repo] [--json ] [--comments] +# Maps to: gh pr view [--repo owner/repo] [--json ], +# then gh pr view [--repo owner/repo] --comments for comments tool_pr_view() { local args="$1" @@ -40,13 +41,13 @@ tool_pr_view() { if [[ -n "${fields}" ]]; then cmd+=("--json" "${fields}") - elif [[ "${comments}" == "true" ]]; then - cmd+=("--comments") fi log "INFO" "pr_view: ${cmd[*]}" local __raw __exit=0 - if [[ "${suppress_errors}" == "true" ]]; then + if [[ -z "${fields}" && "${comments}" == "true" ]]; then + __raw=$(_gh_view_with_comments "pr_view" "--- comments and reviews ---" "${cmd[@]}") || __exit=$? + elif [[ "${suppress_errors}" == "true" ]]; then __raw=$("${cmd[@]}" 2>/dev/null) || __exit=$? else __raw=$("${cmd[@]}" 2>&1) || __exit=$? diff --git a/plugins/github-mcp/mcp-server-gh/tools-read.json b/plugins/github-mcp/mcp-server-gh/tools-read.json index a15f8ad..3ce3c78 100644 --- a/plugins/github-mcp/mcp-server-gh/tools-read.json +++ b/plugins/github-mcp/mcp-server-gh/tools-read.json @@ -26,7 +26,7 @@ }, "comments": { "type": "boolean", - "description": "Include PR comments in output. Only applies when 'fields' is not set.", + "description": "Append the PR comments after the PR text, below a separator line. Only applies when 'fields' is not set. max_lines counts both together.", "default": false }, "jq_filter": { @@ -478,7 +478,7 @@ }, "with_comments": { "type": "boolean", - "description": "Include issue comments in text output. Only applies when 'fields' is not set.", + "description": "Append the issue's comments after the issue text, below a '--- comments ---' line. Only applies when 'fields' is not set; cannot be combined with 'with_field_values'. max_lines counts the issue and its comments together.", "default": false }, "with_field_values": {