diff --git a/.github/ISSUE_TEMPLATE/mcp_issue.yml b/.github/ISSUE_TEMPLATE/mcp_issue.yml index 32a1f2c..15b809d 100644 --- a/.github/ISSUE_TEMPLATE/mcp_issue.yml +++ b/.github/ISSUE_TEMPLATE/mcp_issue.yml @@ -79,7 +79,7 @@ body: - gh-tooling / label and project tools (label_list, project_list, project_view) - gh-tooling / api_read - gh-tooling-write / PR write tools (pr_create, pr_edit, pr_ready, pr_merge, pr_close, pr_reopen) - - gh-tooling-write / review tools (pr_review_submit, pr_comment, pr_review_reply) + - gh-tooling-write / review tools (pr_review_submit, pr_comment, pr_review_reply, comment_edit) - gh-tooling-write / issue write tools (issue_create, issue_edit, issue_close, issue_reopen, issue_comment) - gh-tooling-write / labels, assignees, sub-issues - gh-tooling-write / project writes (project_item_add, project_status_set) diff --git a/plugin-tests/github-mcp/check_api_tools.bats b/plugin-tests/github-mcp/check_api_tools.bats index 792c399..aa1febf 100644 --- a/plugin-tests/github-mcp/check_api_tools.bats +++ b/plugin-tests/github-mcp/check_api_tools.bats @@ -256,6 +256,71 @@ setup_write_blocking() { assert_output --partial "pr_review_reply" } +@test "write api: blocks PATCH issues/comments/ID → suggests comment_edit" { + setup_write_blocking + run_api_hook "$WRITE_TOOL" "repos/shopware/shopware/issues/comments/456" "PATCH" + assert_failure 2 + assert_output --partial "comment_edit" + refute_output --partial "issue_edit" +} + +@test "write api: the PATCH issues/comments/ID block takes the url from issue_view or pr_view comments, not pr_comments" { + setup_write_blocking + run_api_hook "$WRITE_TOOL" "repos/shopware/shopware/issues/comments/456" "PATCH" + assert_failure 2 + assert_output --partial "issue_view or pr_view with fields=comments" + refute_output --partial "pr_comments" +} + +@test "write api: blocks PATCH pulls/comments/ID → suggests comment_edit" { + setup_write_blocking + run_api_hook "$WRITE_TOOL" "repos/shopware/shopware/pulls/comments/456" "PATCH" + assert_failure 2 + assert_output --partial "comment_edit" + assert_output --partial "pr_comments" +} + +@test "write api: blocks PUT pulls/N/reviews/ID → suggests comment_edit" { + setup_write_blocking + run_api_hook "$WRITE_TOOL" "repos/shopware/shopware/pulls/123/reviews/456" "PUT" + assert_failure 2 + assert_output --partial "comment_edit" + refute_output --partial "pr_review_submit" +} + +@test "write api: a comment path only in the query string of another PATCH endpoint is not redirected to comment_edit" { + setup_write_blocking + run_api_hook "$WRITE_TOOL" "repos/shopware/shopware/labels/bug?next=repos/shopware/shopware/issues/comments/456" "PATCH" + assert_success +} + +@test "write api: a review path only in the query string of another PUT endpoint is not redirected to comment_edit" { + setup_write_blocking + run_api_hook "$WRITE_TOOL" "repos/shopware/shopware/pulls/123/requested_reviewers?next=repos/shopware/shopware/pulls/123/reviews/456" "PUT" + assert_success +} + +@test "write api: POST pulls/N/reviews still suggests pr_review_submit, not comment_edit" { + setup_write_blocking + run_api_hook "$WRITE_TOOL" "repos/shopware/shopware/pulls/123/reviews" "POST" + assert_failure 2 + assert_output --partial "pr_review_submit" + refute_output --partial "comment_edit" +} + +@test "write api: DELETE issues/comments/ID is not redirected to comment_edit" { + setup_write_blocking + run_api_hook "$WRITE_TOOL" "repos/shopware/shopware/issues/comments/456" "DELETE" + assert_success +} + +@test "Codex write api: PATCH issues/comments/ID suggests comment_edit" { + setup_codex_config "gh-tooling" '{"block_api_tool_write": true}' + run_api_hook "$CODEX_WRITE_TOOL" "repos/shopware/shopware/issues/comments/456" "PATCH" + assert_failure 2 + assert_output --partial "comment_edit" +} + @test "write api: allows unknown write endpoint" { setup_write_blocking run_api_hook "$WRITE_TOOL" "repos/shopware/shopware/releases" "POST" diff --git a/plugin-tests/github-mcp/gh_tools.bats b/plugin-tests/github-mcp/gh_tools.bats index 9bd189b..107f4fa 100644 --- a/plugin-tests/github-mcp/gh_tools.bats +++ b/plugin-tests/github-mcp/gh_tools.bats @@ -237,6 +237,34 @@ bats_test_function \ "gh api repos/shopware/shopware/pulls/14642/reviews" \ "pr_reviews" +# comment_edit redirects: the method flag before or after the endpoint, and the +# reviews/ID PUT must not fall through to pr_reviews. +gh_api_edit_blocks() { + gh_api_hook_blocks "$1" "comment_edit" + refute_output --partial "pr_reviews" +} + +bats_test_function \ + --description "blocks gh api -X PATCH .../issues/comments/ID → suggests comment_edit" \ + -- gh_api_edit_blocks \ + "gh api -X PATCH repos/shopware/shopware/issues/comments/2001 -f body=x" + +bats_test_function \ + --description "blocks gh api .../pulls/comments/ID --method PATCH → suggests comment_edit" \ + -- gh_api_edit_blocks \ + "gh api repos/shopware/shopware/pulls/comments/1234567 --method PATCH -f body=x" + +bats_test_function \ + --description "blocks gh api -X PUT .../pulls/N/reviews/ID → suggests comment_edit, not pr_reviews" \ + -- gh_api_edit_blocks \ + "gh api -X PUT repos/shopware/shopware/pulls/14642/reviews/777 -f body=x" + +bats_test_function \ + --description "blocks gh api .../pulls/N/reviews/ID -X GET → suggests pr_reviews" \ + -- gh_api_hook_blocks \ + "gh api repos/shopware/shopware/pulls/14642/reviews/777 -X GET" \ + "pr_reviews" + bats_test_function \ --description "blocks gh api .../pulls/N/files → suggests pr_files" \ -- gh_api_hook_blocks \ diff --git a/plugin-tests/github-mcp/tool_schemas.bats b/plugin-tests/github-mcp/tool_schemas.bats index 160d8b0..ee867b8 100644 --- a/plugin-tests/github-mcp/tool_schemas.bats +++ b/plugin-tests/github-mcp/tool_schemas.bats @@ -159,6 +159,24 @@ load_validator_for() { assert_output --partial "Unknown parameter(s): body_file" } +@test "comment_edit refuses fallback, which would report a failed edit as a success" { + load_validator_for "$WRITE_TOOLS" + + run validate_tool_arguments "comment_edit" '{"url": "o/r/issues/1#issuecomment-2", "body": "x", "fallback": "ok"}' + + assert_failure + assert_output --partial "Unknown parameter(s): fallback" +} + +@test "comment_edit refuses an allow_other_author that is not a boolean" { + load_validator_for "$WRITE_TOOLS" + + run validate_tool_arguments "comment_edit" '{"url": "o/r/issues/1#issuecomment-2", "body": "x", "allow_other_author": "true"}' + + assert_failure + assert_output --partial "allow_other_author expected boolean, got string" +} + @test "a paging limit sent as a string is refused, naming the parameter" { # The counterpart to the identifier union: limit and max_lines are # integer-only on purpose, so the string form must fail rather than reach diff --git a/plugin-tests/github-mcp/write_tools_review.bats b/plugin-tests/github-mcp/write_tools_review.bats index 465a892..c5803bc 100644 --- a/plugin-tests/github-mcp/write_tools_review.bats +++ b/plugin-tests/github-mcp/write_tools_review.bats @@ -20,6 +20,17 @@ setup() { # --input calls, and dispatches a canned head.sha for the commit_id fetch. gh() { printf '%s\n' "$*" >> "${GH_ARGS_FILE}" + # One file per call, one NUL-terminated record per argument, so a test + # can tell one argument from several that join to the same text. + local call_n + call_n=$(find "${BATS_TEST_TMPDIR}" -maxdepth 1 -name 'gh_argv.*' | wc -l | tr -d ' ') + printf '%s\0' "$@" > "${BATS_TEST_TMPDIR}/gh_argv.$((call_n + 1))" + # Runaway guard: a looping lookup ends here instead of hanging the suite. + # The page-limit test makes 51 calls. + if [[ "${call_n}" -ge 60 ]]; then + echo "stub: more than 60 gh calls" >&2 + return 99 + fi if [[ "$*" == *"--input"* ]]; then cat > "${GH_STDIN_FILE}" fi @@ -27,9 +38,55 @@ setup() { printf '%s\n' "${GH_STUB_HEAD_SHA:-0123456789abcdef0123456789abcdef01234567}" return 0 fi + # comment_edit's calls, told apart by what the argv names. Each kind has + # its own stdout, stderr, and exit code: GH_STUB__OUTPUT/_STDERR/_EXIT. + # A page fetch of a pending review's comments is keyed by the cursor it + # sends, so each page has its own canned answer: GH_STUB_PAGE__*. + local kind="" get_re='^api repos/[^ ]+/(comments|reviews)/[0-9]+$' + # shellcheck disable=SC2016 # literal GraphQL text ($reviewId), not a shell variable + if [[ "$*" == *"reviews(states: [PENDING]"* ]]; then + kind=LOOKUP + elif [[ "$*" == *"... on Comment { viewerDidAuthor }"* ]]; then + kind=AUTHOR + elif [[ "$*" == *'node(id: $reviewId)'* ]]; then + local arg cursor="" + for arg in "$@"; do + [[ "${arg}" == cursor=* ]] && cursor="${arg#cursor=}" + done + kind="PAGE_${cursor}" + elif [[ "$*" == *"updatePullRequestReviewComment"* ]]; then + kind=MUTATION + elif [[ "$*" == *" -X PATCH "* || "$*" == *" -X PUT "* ]]; then + kind=WRITE + elif [[ "$*" =~ ${get_re} ]]; then + kind=GET + fi + if [[ -n "${kind}" ]]; then + local out_var="GH_STUB_${kind}_OUTPUT" err_var="GH_STUB_${kind}_STDERR" exit_var="GH_STUB_${kind}_EXIT" + [[ -n "${!err_var:-}" ]] && printf '%s\n' "${!err_var}" >&2 + if [[ "${kind}" == WRITE && -n "${GH_STUB_APPLY_JQ:-}" ]]; then + # Behave like real gh: run the --jq filter over the canned response. + local arg filter="" prev="" + for arg in "$@"; do + [[ "${prev}" == "--jq" ]] && filter="${arg}" + prev="${arg}" + done + printf '%s' "${!out_var}" | jq -r "${filter}" + return $? + fi + [[ -n "${!out_var:-}" ]] && printf '%s\n' "${!out_var}" + return "${!exit_var:-0}" + fi gh_stub_respond } reset_gh_stub + local kind + for kind in GET WRITE LOOKUP MUTATION AUTHOR; do + printf -v "GH_STUB_${kind}_OUTPUT" '%s' "" + printf -v "GH_STUB_${kind}_STDERR" '%s' "" + printf -v "GH_STUB_${kind}_EXIT" '%s' 0 + done + GH_STUB_AUTHOR_OUTPUT="${AUTHOR_YES}" GH_STUB_HEAD_SHA="0123456789abcdef0123456789abcdef01234567" } @@ -249,3 +306,1002 @@ _assert_rejects_incomplete_comment() { assert_gh_args_contain "-X POST" assert_gh_args_contain "-f body=Addressed, thanks!" } + + +# ============================================================================ +# comment_edit +# ============================================================================ + +NOT_FOUND_STDERR="gh: Not Found (HTTP 404)" +SERVER_ERROR_STDERR="gh: Internal Server Error (HTTP 500)" +WARNING_STDERR="warning: a new gh release is available" + +PENDING_MATCH='{"data":{"repository":{"pullRequest":{"reviews":{"nodes":[{"comments":{"nodes":[{"id":"PRRC_other","fullDatabaseId":"4000"},{"id":"PRRC_node5551","fullDatabaseId":"5551"}],"pageInfo":{"hasNextPage":false}}}]}}}}}' +PENDING_BIG='{"data":{"repository":{"pullRequest":{"reviews":{"nodes":[{"comments":{"nodes":[{"id":"PRRC_near","fullDatabaseId":"4195291200"},{"id":"PRRC_big","fullDatabaseId":"4195291201"}],"pageInfo":{"hasNextPage":false}}}]}}}}}' +PENDING_PAGED='{"data":{"repository":{"pullRequest":{"reviews":{"nodes":[{"id":"PRR_mine","comments":{"nodes":[{"id":"PRRC_a","fullDatabaseId":"1"}],"pageInfo":{"hasNextPage":true,"endCursor":"CUR1"}}}]}}}}}' +PAGE_MORE='{"data":{"node":{"comments":{"nodes":[{"id":"PRRC_b","fullDatabaseId":"2"}],"pageInfo":{"hasNextPage":true,"endCursor":"CUR2"}}}}}' +PAGE_LAST_MATCH='{"data":{"node":{"comments":{"nodes":[{"id":"PRRC_node5551","fullDatabaseId":"5551"}],"pageInfo":{"hasNextPage":false,"endCursor":"CURLAST"}}}}}' +PAGE_LAST_NO_MATCH='{"data":{"node":{"comments":{"nodes":[{"id":"PRRC_c","fullDatabaseId":"3"}],"pageInfo":{"hasNextPage":false,"endCursor":"CURLAST"}}}}}' +AUTHOR_YES='{"data":{"node":{"viewerDidAuthor":true}}}' +AUTHOR_NO='{"data":{"node":{"viewerDidAuthor":false}}}' +# The author query's argv, up to the node ID that follows it. +# shellcheck disable=SC2016 # literal GraphQL text, not a shell variable +AUTHOR_CALL='api graphql -f query=query($id: ID!) { node(id: $id) { ... on Comment { viewerDidAuthor } } } -f id=' +# A text no failure message may repeat: it stands for a comment body. +BODY_MARKER="SECRET-BODY-TEXT" +MUTATION_OK='{"data":{"updatePullRequestReviewComment":{"pullRequestReviewComment":{"url":"https://github.com/shopware/shopware/pull/100#discussion_r5551"}}}}' + +# The GET answer for a comment that belongs to issue or PR N, written by $2 +# (default "me"; the login only names the author in a refusal) and carrying a +# body, so a test can tell whether a failure repeats the comment. +stub_get_issue() { + GH_STUB_GET_OUTPUT=$(jq -cn --arg u "https://api.github.com/repos/shopware/shopware/issues/$1" --arg l "${2:-me}" --arg b "${BODY_MARKER}" '{id: 1, node_id: "IC_node1", issue_url: $u, user: {login: $l}, body: $b}') +} + +stub_get_pull() { + GH_STUB_GET_OUTPUT=$(jq -cn --arg u "https://api.github.com/repos/shopware/shopware/pulls/$1" --arg l "${2:-me}" --arg b "${BODY_MARKER}" '{id: 1, node_id: "PRRC_node1", pull_request_url: $u, user: {login: $l}, body: $b}') +} + +# The author query was not sent. +assert_no_author_query() { + ! grep -qF -- "... on Comment { viewerDidAuthor }" "${GH_ARGS_FILE}" || fail "Expected no author query, got: $(cat "${GH_ARGS_FILE}")" +} + +# gh must not have run: the rejection happens before any API call. +assert_gh_not_called() { + [[ ! -s "${GH_ARGS_FILE}" ]] || fail "Expected no gh call, got: $(cat "${GH_ARGS_FILE}")" +} + +# The one gh invocation, compared whole: endpoint, method, body field, and jq filter. +assert_gh_args_equal() { + local expected="$1" actual + actual=$(cat "${GH_ARGS_FILE}") + [[ "${actual}" == "${expected}" ]] || fail "Expected gh args '${expected}', got: '${actual}'" +} + +# The last gh invocation, compared whole. +assert_gh_last_args_equal() { + local expected="$1" actual + actual=$(tail -n 1 "${GH_ARGS_FILE}") + [[ "${actual}" == "${expected}" ]] || fail "Expected last gh args '${expected}', got: '${actual}'" +} + +# The Nth gh invocation, compared whole. +assert_gh_call_equal() { + local n="$1" expected="$2" actual + actual=$(sed -n "${n}p" "${GH_ARGS_FILE}") + [[ "${actual}" == "${expected}" ]] || fail "Expected gh call ${n} to be '${expected}', got: '${actual}'" +} + +assert_gh_last_args_contain_all() { + local last needle + last=$(tail -n 1 "${GH_ARGS_FILE}") + for needle in "$@"; do + [[ "${last}" == *"${needle}"* ]] || fail "Expected last gh call to contain '${needle}', got: ${last}" + done +} + +# The Nth gh invocation contains every given text. +assert_gh_call_contains_all() { + local n="$1" call needle + shift + call=$(sed -n "${n}p" "${GH_ARGS_FILE}") + for needle in "$@"; do + [[ "${call}" == *"${needle}"* ]] || fail "Expected gh call ${n} to contain '${needle}', got: ${call}" + done +} + +# Exactly N gh calls ran. +assert_gh_call_count() { + local expected="$1" actual + actual=$(wc -l < "${GH_ARGS_FILE}" | tr -d ' ') + [[ "${actual}" -eq "${expected}" ]] || fail "Expected ${expected} gh call(s), got ${actual}: $(cat "${GH_ARGS_FILE}")" +} + +# No PATCH or PUT was sent. +assert_no_rest_write() { + ! grep -q -- " -X " "${GH_ARGS_FILE}" || fail "Expected no REST write, got: $(cat "${GH_ARGS_FILE}")" +} + +# The edit mutation was not sent. +assert_no_mutation() { + ! grep -q -- "updatePullRequestReviewComment" "${GH_ARGS_FILE}" || fail "Expected no mutation, got: $(cat "${GH_ARGS_FILE}")" +} + +# No GraphQL call other than the author query: no pending-review lookup or mutation. +assert_no_graphql_call() { + ! grep -- "graphql" "${GH_ARGS_FILE}" | grep -qvF -- "... on Comment { viewerDidAuthor }" || fail "Expected no GraphQL call, got: $(cat "${GH_ARGS_FILE}")" +} + +# A failed call reports through an error message that starts with "Error:". +assert_error_output() { + assert_output --regexp '^Error:' +} + +# ---- issue and PR conversation comments ------------------------------------ + +@test "comment_edit reads an issue comment, then patches it when it belongs to the issue in the URL" { + stub_get_issue 7 + GH_STUB_WRITE_OUTPUT="https://github.com/shopware/shopware/issues/7#issuecomment-9001" + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-9001", "body": "New text"}' + assert_success + assert_output "https://github.com/shopware/shopware/issues/7#issuecomment-9001" + assert_gh_call_count 3 + assert_gh_call_equal 1 "api repos/shopware/shopware/issues/comments/9001" + assert_gh_call_equal 2 "${AUTHOR_CALL}IC_node1" + assert_gh_call_equal 3 "api repos/shopware/shopware/issues/comments/9001 -X PATCH -f body=New text --jq .html_url // empty" +} + +@test "comment_edit patches a PR conversation comment through the issues comments endpoint" { + stub_get_issue 100 + GH_STUB_WRITE_OUTPUT="https://example/c" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#issuecomment-9002", "body": "New text"}' + assert_success + assert_gh_call_equal 1 "api repos/shopware/shopware/issues/comments/9002" + assert_gh_call_equal 3 "api repos/shopware/shopware/issues/comments/9002 -X PATCH -f body=New text --jq .html_url // empty" +} + +@test "comment_edit refuses to edit a conversation comment that belongs to another issue or PR" { + stub_get_issue 8 + GH_STUB_WRITE_OUTPUT="https://example/c" + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-9001", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "#8" + assert_output --partial "#7" + assert_gh_call_count 1 + assert_no_rest_write +} + +@test "comment_edit does not treat an issue number that merely ends in the URL's number as a match" { + stub_get_issue 17 + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-9001", "body": "x"}' + assert_failure + assert_error_output + assert_no_rest_write +} + +@test "comment_edit fails with gh's message when the conversation comment does not exist, and does not patch" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-9001", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "Not Found (HTTP 404)" + assert_gh_call_count 1 + assert_no_rest_write + assert_no_graphql_call +} + +@test "comment_edit names the failed read and keeps gh's exit status when the read fails with no output" { + GH_STUB_GET_EXIT=3 + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-9001", "body": "x"}' + assert_failure 3 + assert_error_output + assert_output --partial "GET repos/shopware/shopware/issues/comments/9001 failed: gh failed with exit 3 and no output" + assert_gh_call_count 1 + assert_no_rest_write +} + +@test "comment_edit fails when the read of a conversation comment names no issue, without repeating the comment, and does not patch" { + GH_STUB_GET_OUTPUT=$(jq -cn --arg b "${BODY_MARKER}" '{id: 9001, body: $b}') + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-9001", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "GET repos/shopware/shopware/issues/comments/9001 returned no issue_url" + refute_output --partial "${BODY_MARKER}" + assert_gh_call_count 1 + assert_no_rest_write +} + +# ---- inline review comments: submitted --------------------------------------- + +@test "comment_edit patches an inline review comment from a discussion_r anchor with no GraphQL call" { + stub_get_pull 100 + GH_STUB_WRITE_OUTPUT="https://example/c" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "New text"}' + assert_success + assert_output "https://example/c" + assert_gh_call_count 3 + assert_gh_call_equal 1 "api repos/shopware/shopware/pulls/comments/5551" + assert_gh_call_equal 3 "api repos/shopware/shopware/pulls/comments/5551 -X PATCH -f body=New text --jq .html_url // empty" + assert_no_graphql_call +} + +@test "comment_edit refuses to edit an inline comment that belongs to another PR, with no write and no lookup" { + stub_get_pull 101 + GH_STUB_WRITE_OUTPUT="https://example/c" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "#101" + assert_output --partial "#100" + assert_gh_call_count 1 + assert_no_rest_write + assert_no_graphql_call +} + +@test "comment_edit does not fall back to the pending-review lookup when the PATCH of a submitted comment fails" { + stub_get_pull 100 + GH_STUB_WRITE_EXIT=1 + GH_STUB_WRITE_STDERR="${NOT_FOUND_STDERR}" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "PATCH repos/shopware/shopware/pulls/comments/5551 failed" + assert_gh_call_count 3 + assert_no_graphql_call +} + +@test "comment_edit fails without a lookup or write when reading the inline comment fails with a server error" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${SERVER_ERROR_STDERR}" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "Internal Server Error (HTTP 500)" + assert_gh_call_count 1 + assert_no_rest_write + assert_no_graphql_call +} + +# ---- inline review comments: pending review --------------------------------- + +@test "comment_edit finds a comment in the pending review after a 404 and edits it with the mutation" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="${PENDING_MATCH}" + GH_STUB_MUTATION_OUTPUT="${MUTATION_OK}" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "New text"}' + assert_success + assert_output "https://github.com/shopware/shopware/pull/100#discussion_r5551" + assert_gh_call_count 3 + assert_gh_call_equal 1 "api repos/shopware/shopware/pulls/comments/5551" + assert_gh_call_contains_all 2 "-f owner=shopware -f name=shopware -F number=100" + assert_gh_last_args_contain_all "updatePullRequestReviewComment" "-f id=PRRC_node5551" "-f body=New text" + assert_no_rest_write +} + +@test "comment_edit matches a pending-review comment on a database id above 32 bits, given as a string" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="${PENDING_BIG}" + GH_STUB_MUTATION_OUTPUT="${MUTATION_OK}" + run tool_comment_edit '{"url": "shopware/shopware/pull/100/files#r4195291201", "body": "x"}' + assert_success + assert_gh_call_count 3 + assert_gh_last_args_contain_all "-f id=PRRC_big" + assert_no_rest_write +} + +@test "comment_edit fails when the 404 comment is not in the pending review, with no mutation or write" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="${PENDING_MATCH}" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r9999", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "GET repos/shopware/shopware/pulls/comments/9999 returned 404" + assert_output --partial "pending-review lookup on PR 100: no matching comment" + assert_gh_call_count 2 + assert_no_rest_write + assert_no_mutation +} + +@test "comment_edit finds the comment on the second page of a large pending review and edits it with that node id" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="${PENDING_PAGED}" + GH_STUB_PAGE_CUR1_OUTPUT="${PAGE_LAST_MATCH}" + GH_STUB_MUTATION_OUTPUT="${MUTATION_OK}" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "New text"}' + assert_success + assert_output "https://github.com/shopware/shopware/pull/100#discussion_r5551" + assert_gh_call_count 4 + assert_gh_call_contains_all 3 "-f reviewId=PRR_mine" "-f cursor=CUR1" + assert_gh_last_args_contain_all "updatePullRequestReviewComment" "-f id=PRRC_node5551" "-f body=New text" + assert_no_rest_write +} + +@test "comment_edit follows each page's cursor until the comment turns up on the third page" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="${PENDING_PAGED}" + GH_STUB_PAGE_CUR1_OUTPUT="${PAGE_MORE}" + GH_STUB_PAGE_CUR2_OUTPUT="${PAGE_LAST_MATCH}" + GH_STUB_MUTATION_OUTPUT="${MUTATION_OK}" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_success + assert_gh_call_count 5 + assert_gh_call_contains_all 3 "-f reviewId=PRR_mine" "-f cursor=CUR1" + assert_gh_call_contains_all 4 "-f reviewId=PRR_mine" "-f cursor=CUR2" + assert_gh_last_args_contain_all "-f id=PRRC_node5551" +} + +@test "comment_edit fails with the 404 and no matching comment when no page of a large pending review holds it" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="${PENDING_PAGED}" + GH_STUB_PAGE_CUR1_OUTPUT="${PAGE_LAST_NO_MATCH}" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "returned 404" + assert_output --partial "pending-review lookup on PR 100: no matching comment" + assert_gh_call_count 3 + assert_no_rest_write + assert_no_mutation +} + +@test "comment_edit stops at the page limit when every page reports more, with no mutation" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="${PENDING_PAGED}" + GH_STUB_PAGE_CUR1_OUTPUT='{"data":{"node":{"comments":{"nodes":[{"id":"PRRC_b","fullDatabaseId":"2"}],"pageInfo":{"hasNextPage":true,"endCursor":"CUR1"}}}}}' + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "pending-review lookup on PR 100: reached the limit of 50 comment pages without finding comment 5551" + assert_gh_call_count 51 + assert_no_rest_write + assert_no_mutation +} + +@test "comment_edit fails and sends no mutation when the second page of comments cannot be read" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="${PENDING_PAGED}" + GH_STUB_PAGE_CUR1_EXIT=7 + GH_STUB_PAGE_CUR1_STDERR="gh: Bad Gateway (HTTP 502)" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_failure 7 + assert_error_output + assert_output --partial "returned 404" + assert_output --partial "Bad Gateway (HTTP 502)" + assert_gh_call_count 3 + assert_no_rest_write + assert_no_mutation +} + +@test "comment_edit fails and sends no mutation when the second page lacks the expected data, without repeating the page" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="${PENDING_PAGED}" + local answer + for answer in '{"data":{"node":null}}' \ + '{"data":{"node":{"comments":"SECRET-BODY-TEXT"}}}' \ + '{"data":"SECRET-BODY-TEXT"}'; do + GH_STUB_PAGE_CUR1_OUTPUT="${answer}" + : > "${GH_ARGS_FILE}" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + [[ "${status}" -ne 0 ]] || fail "Expected the answer ${answer} to fail" + assert_error_output + assert_output --partial "pending-review lookup on PR 100: unexpected response from GitHub for comment page 2 of your pending review" + refute_output --partial "${BODY_MARKER}" + assert_gh_call_count 3 + assert_no_mutation + done +} + +@test "comment_edit edits a matching pending-review comment even when the review has further pages" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT='{"data":{"repository":{"pullRequest":{"reviews":{"nodes":[{"comments":{"nodes":[{"id":"PRRC_node5551","fullDatabaseId":"5551"}],"pageInfo":{"hasNextPage":true}}}]}}}}}' + GH_STUB_MUTATION_OUTPUT="${MUTATION_OK}" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_success + assert_gh_call_count 3 +} + +@test "comment_edit searches every returned pending review and edits the match from the second one" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT='{"data":{"repository":{"pullRequest":{"reviews":{"nodes":[{"comments":{"nodes":[{"id":"PRRC_theirs","fullDatabaseId":"4000"}],"pageInfo":{"hasNextPage":false}}},{"comments":{"nodes":[{"id":"PRRC_mine","fullDatabaseId":"5551"}],"pageInfo":{"hasNextPage":false}}}]}}}}}' + GH_STUB_MUTATION_OUTPUT="${MUTATION_OK}" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_success + assert_gh_call_count 3 + assert_gh_last_args_contain_all "-f id=PRRC_mine" +} + +@test "comment_edit fails when the pending-review lookup exits non-zero, with gh's message and no mutation" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_EXIT=1 + GH_STUB_LOOKUP_STDERR="gh: Could not resolve to a Repository" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "GET repos/shopware/shopware/pulls/comments/5551 returned 404 (not found, or no access to the repository)" + assert_output --partial "pending-review lookup on PR 100: gh: Could not resolve to a Repository" + assert_gh_call_count 2 + assert_no_rest_write +} + +@test "comment_edit states the 404 and the GraphQL message when the lookup answers a Could not resolve error" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT='{"errors":[{"type":"NOT_FOUND","message":"Could not resolve to a Repository with the name shopware/shopware."}],"data":{"repository":null}}' + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "returned 404 (not found, or no access to the repository)" + assert_output --partial "Could not resolve to a Repository" + assert_gh_call_count 2 + assert_no_mutation +} + +@test "comment_edit fails when the lookup succeeds with no output" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "pending-review lookup on PR 100: GitHub returned no output" + assert_gh_call_count 2 +} + +@test "comment_edit names the failed call and keeps gh's exit status when the lookup fails with no output" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_EXIT=5 + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_failure 5 + assert_error_output + assert_output --partial "pending-review lookup on PR 100: gh failed with exit 5 and no output" +} + +@test "comment_edit fails when the lookup output is not JSON" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="bad gateway" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "unparseable response" + assert_gh_call_count 2 +} + +@test "comment_edit fails when the lookup response lacks the expected data, without repeating the response" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + local answer + for answer in '{"data":{"repository":{"pullRequest":null}}}' \ + '{"data":{"repository":{"pullRequest":"SECRET-BODY-TEXT"}}}' \ + '{"data":"SECRET-BODY-TEXT"}'; do + GH_STUB_LOOKUP_OUTPUT="${answer}" + : > "${GH_ARGS_FILE}" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + [[ "${status}" -ne 0 ]] || fail "Expected the answer ${answer} to fail" + assert_error_output + assert_output --partial "pending-review lookup on PR 100: unexpected response from GitHub for the pending reviews" + refute_output --partial "${BODY_MARKER}" + assert_gh_call_count 2 + done +} + +@test "comment_edit fails with gh's message when the mutation exits non-zero, with no REST write" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="${PENDING_MATCH}" + GH_STUB_MUTATION_EXIT=1 + GH_STUB_MUTATION_STDERR="gh: Resource not accessible" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "Resource not accessible" + assert_output --partial "GET repos/shopware/shopware/pulls/comments/5551 returned 404 (not found, or no access to the repository); pending-review comment edit failed" + assert_gh_call_count 3 + assert_no_rest_write +} + +@test "comment_edit fails when the mutation response holds no url" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="${PENDING_MATCH}" + local answer + for answer in '{"data":{"updatePullRequestReviewComment":{"pullRequestReviewComment":{"url":null}}}}' \ + '{"data":{"updatePullRequestReviewComment":"oops"}}'; do + GH_STUB_MUTATION_OUTPUT="${answer}" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + [[ "${status}" -ne 0 ]] || fail "Expected the answer ${answer} to fail" + assert_error_output + assert_output --partial "GET repos/shopware/shopware/pulls/comments/5551 returned 404 (not found, or no access to the repository); the pending-review edit was sent and GitHub reported success but returned no URL, so the comment may already hold the new body" + done +} + +@test "comment_edit names the failed call and keeps gh's exit status when the mutation fails with no output" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="${PENDING_MATCH}" + GH_STUB_MUTATION_EXIT=4 + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_failure 4 + assert_error_output + assert_output --partial "pending-review comment edit failed: gh failed with exit 4 and no output" + assert_output --partial "GET repos/shopware/shopware/pulls/comments/5551 returned 404 (not found, or no access to the repository); pending-review comment edit failed" +} + +# ---- review summary body ------------------------------------------------------ + +@test "comment_edit reads a review summary, then puts the body on the PR's reviews endpoint" { + GH_STUB_GET_OUTPUT='{"id":777,"node_id":"PRR_node777","user":{"login":"me"}}' + GH_STUB_WRITE_OUTPUT="https://example/c" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#pullrequestreview-777", "body": "New summary"}' + assert_success + assert_gh_call_count 3 + assert_gh_call_equal 1 "api repos/shopware/shopware/pulls/100/reviews/777" + assert_gh_call_equal 3 "api repos/shopware/shopware/pulls/100/reviews/777 -X PUT -f body=New summary --jq .html_url // empty" +} + +# ---- stderr warnings on successful calls ------------------------------------- + +@test "comment_edit returns the clean URL when a successful read and PATCH each print a warning on stderr" { + stub_get_issue 7 + GH_STUB_GET_STDERR="${WARNING_STDERR}" + GH_STUB_WRITE_STDERR="${WARNING_STDERR}" + GH_STUB_WRITE_OUTPUT="https://github.com/shopware/shopware/issues/7#issuecomment-9001" + run --separate-stderr tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-9001", "body": "x"}' + assert_success + assert_output "https://github.com/shopware/shopware/issues/7#issuecomment-9001" +} + +@test "comment_edit returns the clean URL when a successful review PUT prints a warning on stderr" { + GH_STUB_GET_OUTPUT='{"id":777,"node_id":"PRR_node777","user":{"login":"me"}}' + GH_STUB_WRITE_STDERR="${WARNING_STDERR}" + GH_STUB_WRITE_OUTPUT="https://example/review" + run --separate-stderr tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#pullrequestreview-777", "body": "x"}' + assert_success + assert_output "https://example/review" +} + +@test "comment_edit returns the clean URL when a successful lookup and mutation each print a warning on stderr" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="${PENDING_MATCH}" + GH_STUB_LOOKUP_STDERR="${WARNING_STDERR}" + GH_STUB_MUTATION_OUTPUT="${MUTATION_OK}" + GH_STUB_MUTATION_STDERR="${WARNING_STDERR}" + run --separate-stderr tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_success + assert_output "https://github.com/shopware/shopware/pull/100#discussion_r5551" +} + +# ---- author guard ------------------------------------------------------------- + +@test "comment_edit asks GitHub about the node id of the comment it read, for each kind, before it writes" { + GH_STUB_WRITE_OUTPUT="https://example/c" + local row kind_url node + for row in "shopware/shopware/issues/7#issuecomment-9001|IC_node1" \ + "https://github.com/shopware/shopware/pull/100#discussion_r5551|PRRC_node1" \ + "https://github.com/shopware/shopware/pull/100#pullrequestreview-777|PRR_node777"; do + kind_url="${row%%|*}" + node="${row##*|}" + case "${node}" in + IC_*) stub_get_issue 7 ;; + PRRC_*) stub_get_pull 100 ;; + *) GH_STUB_GET_OUTPUT='{"id":777,"node_id":"PRR_node777","user":{"login":"me"}}' ;; + esac + : > "${GH_ARGS_FILE}" + run tool_comment_edit "{\"url\": \"${kind_url}\", \"body\": \"x\"}" + [[ "${status}" -eq 0 ]] || fail "Expected '${kind_url}' to be edited, got: ${output}" + assert_gh_call_count 3 + assert_gh_call_equal 2 "${AUTHOR_CALL}${node}" + done +} + +@test "comment_edit refuses another author's conversation comment by default, naming the author, with no write" { + stub_get_issue 7 "octocat" + GH_STUB_AUTHOR_OUTPUT="${AUTHOR_NO}" + GH_STUB_WRITE_OUTPUT="https://example/c" + local json + for json in '{"url": "shopware/shopware/issues/7#issuecomment-9001", "body": "x"}' \ + '{"url": "shopware/shopware/issues/7#issuecomment-9001", "body": "x", "allow_other_author": false}'; do + : > "${GH_ARGS_FILE}" + run tool_comment_edit "${json}" + assert_failure + assert_error_output + assert_output --partial "written by octocat" + assert_output --partial "allow_other_author: true" + refute_output --partial "${BODY_MARKER}" + assert_gh_call_count 2 + assert_no_rest_write + done +} + +@test "comment_edit refuses another author's inline comment by default, with no write" { + stub_get_pull 100 "octocat" + GH_STUB_AUTHOR_OUTPUT="${AUTHOR_NO}" + GH_STUB_WRITE_OUTPUT="https://example/c" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "written by octocat" + assert_no_rest_write +} + +@test "comment_edit refuses another author's review summary by default, with no PUT" { + GH_STUB_GET_OUTPUT='{"id":777,"node_id":"PRR_node777","user":{"login":"octocat"}}' + GH_STUB_AUTHOR_OUTPUT="${AUTHOR_NO}" + GH_STUB_WRITE_OUTPUT="https://example/c" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#pullrequestreview-777", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "written by octocat" + assert_output --partial "allow_other_author: true" + assert_gh_call_equal 1 "api repos/shopware/shopware/pulls/100/reviews/777" + assert_gh_call_equal 2 "${AUTHOR_CALL}PRR_node777" + assert_no_rest_write +} + +@test "comment_edit names a deleted user when the comment has no author and GitHub says the caller did not write it" { + GH_STUB_GET_OUTPUT='{"id":1,"node_id":"IC_node1","issue_url":"https://api.github.com/repos/shopware/shopware/issues/7","user":null}' + GH_STUB_AUTHOR_OUTPUT="${AUTHOR_NO}" + GH_STUB_WRITE_OUTPUT="https://example/c" + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-9001", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "written by a deleted user" + assert_output --partial "allow_other_author: true" + assert_no_rest_write +} + +@test "comment_edit edits another author's comment when allow_other_author is true, without asking GitHub who wrote it" { + stub_get_issue 7 "octocat" + GH_STUB_AUTHOR_OUTPUT="${AUTHOR_NO}" + GH_STUB_WRITE_OUTPUT="https://example/c" + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-9001", "body": "x", "allow_other_author": true}' + assert_success + assert_output "https://example/c" + assert_gh_call_count 2 + assert_no_author_query +} + +@test "comment_edit edits another author's review summary when allow_other_author is true" { + GH_STUB_GET_OUTPUT='{"id":777,"user":{"login":"octocat"}}' + GH_STUB_AUTHOR_OUTPUT="${AUTHOR_NO}" + GH_STUB_WRITE_OUTPUT="https://example/c" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#pullrequestreview-777", "body": "x", "allow_other_author": true}' + assert_success + assert_gh_call_count 2 + assert_gh_call_equal 2 "api repos/shopware/shopware/pulls/100/reviews/777 -X PUT -f body=x --jq .html_url // empty" + assert_no_author_query +} + +@test "comment_edit fails when the review summary cannot be read, with no PUT" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#pullrequestreview-777", "body": "x", "allow_other_author": true}' + assert_failure + assert_error_output + assert_output --partial "GET repos/shopware/shopware/pulls/100/reviews/777 failed" + assert_gh_call_count 1 + assert_no_rest_write +} + +@test "comment_edit fails with no write when the author query exits non-zero" { + stub_get_issue 7 + GH_STUB_AUTHOR_EXIT=1 + GH_STUB_AUTHOR_STDERR="gh: Bad credentials (HTTP 401)" + GH_STUB_WRITE_OUTPUT="https://example/c" + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-9001", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "Bad credentials" + assert_gh_call_count 2 + assert_no_rest_write +} + +@test "comment_edit refuses with no write when the author answer is anything but true" { + stub_get_issue 7 + GH_STUB_WRITE_OUTPUT="https://example/c" + local answer + for answer in '{"data":{"node":null}}' \ + '{"data":{"node":{}}}' \ + '{"data":{"node":{"viewerDidAuthor":null}}}' \ + '{"data":{"node":{"viewerDidAuthor":"true"}}}' \ + '{"data":{"node":"oops"}}'; do + GH_STUB_AUTHOR_OUTPUT="${answer}" + : > "${GH_ARGS_FILE}" + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-9001", "body": "x"}' + [[ "${status}" -ne 0 ]] || fail "Expected the answer ${answer} to fail" + assert_error_output + assert_output --partial "written by me, not by you" + assert_output --partial "allow_other_author: true" + assert_gh_call_count 2 + assert_no_rest_write + done +} + +@test "comment_edit fails with no write and no author query when the comment read has no node_id, unless allow_other_author is true" { + GH_STUB_GET_OUTPUT=$(jq -cn --arg b "${BODY_MARKER}" '{id: 1, issue_url: "https://api.github.com/repos/shopware/shopware/issues/7", user: {login: "me"}, body: $b}') + GH_STUB_WRITE_OUTPUT="https://example/c" + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-9001", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "Error: comment_edit: unexpected response from GitHub for GET repos/shopware/shopware/issues/comments/9001" + refute_output --partial "${BODY_MARKER}" + assert_gh_call_count 1 + assert_no_rest_write + + : > "${GH_ARGS_FILE}" + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-9001", "body": "x", "allow_other_author": true}' + assert_success +} + +@test "comment_edit does not ask who wrote a comment in the caller's own pending review" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="${PENDING_MATCH}" + GH_STUB_MUTATION_OUTPUT="${MUTATION_OK}" + run tool_comment_edit '{"url": "https://github.com/shopware/shopware/pull/100#discussion_r5551", "body": "x"}' + assert_success + assert_no_author_query +} + +# ---- URL handling ----------------------------------------------------------------- + +@test "comment_edit takes the repository from the URL, not from the default repo" { + GH_STUB_GET_OUTPUT='{"id":1,"node_id":"IC_node1","issue_url":"https://api.github.com/repos/other-org/other.repo/issues/7","user":{"login":"me"}}' + GH_STUB_WRITE_OUTPUT="https://example/c" + run tool_comment_edit '{"url": "other-org/other.repo/issues/7#issuecomment-1", "body": "x"}' + assert_success + assert_gh_call_equal 1 "api repos/other-org/other.repo/issues/comments/1" + assert_gh_call_equal 3 "api repos/other-org/other.repo/issues/comments/1 -X PATCH -f body=x --jq .html_url // empty" +} + +@test "comment_edit reaches the expected first gh call for each accepted URL form" { + local sha40="0123456789abcdef0123456789abcdef01234567" + GH_STUB_GET_OUTPUT='{"id":1,"node_id":"IC_node1","issue_url":"https://api.github.com/repos/shopware/shopware/issues/100","pull_request_url":"https://api.github.com/repos/shopware/shopware/pulls/100","user":{"login":"me"}}' + GH_STUB_WRITE_OUTPUT="https://example/c" + local row u endpoint + for row in \ + "http://github.com/shopware/shopware/issues/7#issuecomment-3|repos/shopware/shopware/issues/comments/3" \ + "https://www.github.com/shopware/shopware/issues/7#issuecomment-3|repos/shopware/shopware/issues/comments/3" \ + "https://GitHub.com/Shopware/Shopware/issues/7#issuecomment-3|repos/Shopware/Shopware/issues/comments/3" \ + "HTTPS://github.com/Shopware/Shopware/issues/7#issuecomment-3|repos/Shopware/Shopware/issues/comments/3" \ + "http://GITHUB.COM/Shopware/Shopware/issues/7#issuecomment-3|repos/Shopware/Shopware/issues/comments/3" \ + "https://WWW.GitHub.com/Shopware/Shopware/issues/7#issuecomment-3|repos/Shopware/Shopware/issues/comments/3" \ + "https://www.GITHUB.com/Shopware/Shopware/issues/7#issuecomment-3|repos/Shopware/Shopware/issues/comments/3" \ + "shopware/shopware/issues/100?notification_referrer_id=NT_x#issuecomment-3|repos/shopware/shopware/issues/comments/3" \ + "https://github.com/shopware/shopware/pull/100?notification_referrer_id=NT_x#issuecomment-3|repos/shopware/shopware/issues/comments/3" \ + "shopware/shopware/pull/100?notification_referrer_id=NT_x#discussion_r5551|repos/shopware/shopware/pulls/comments/5551" \ + "shopware/shopware/pull/100/files#r5552|repos/shopware/shopware/pulls/comments/5552" \ + "shopware/shopware/pull/100/changes#r5553|repos/shopware/shopware/pulls/comments/5553" \ + "shopware/shopware/pull/100/files?w=1#r5552|repos/shopware/shopware/pulls/comments/5552" \ + "shopware/shopware/pull/100/files/${sha40}..${sha40}#r5553|repos/shopware/shopware/pulls/comments/5553" \ + "shopware/shopware/pull/100/files/abc1234..def5678#r5554|repos/shopware/shopware/pulls/comments/5554" \ + "shopware/shopware/pull/100/files/${sha40}#r5558|repos/shopware/shopware/pulls/comments/5558" \ + "shopware/shopware/pull/100/files/abc1234#r5559|repos/shopware/shopware/pulls/comments/5559" \ + "shopware/shopware/pull/100/changes/${sha40}#r5560|repos/shopware/shopware/pulls/comments/5560" \ + "shopware/shopware/pull/100/changes/abc1234#r5561|repos/shopware/shopware/pulls/comments/5561" \ + "shopware/shopware/pull/100/changes/${sha40}..${sha40}#r5562|repos/shopware/shopware/pulls/comments/5562" \ + "shopware/shopware/pull/100/changes/abc1234..def5678#r5563|repos/shopware/shopware/pulls/comments/5563" \ + "shopware/shopware/pull/100/changes/abc1234..def5678?w=1#r5564|repos/shopware/shopware/pulls/comments/5564" \ + "shopware/shopware/pull/100/commits/${sha40}#r5555|repos/shopware/shopware/pulls/comments/5555" \ + "shopware/shopware/pull/100/commits/abc1234#r5556|repos/shopware/shopware/pulls/comments/5556" \ + "shopware/shopware/pull/100/commits/${sha40}?notification_referrer_id=NT_x#r5557|repos/shopware/shopware/pulls/comments/5557" \ + "shopware/shopware/pull/100?notification_referrer_id=NT_x#pullrequestreview-777|repos/shopware/shopware/pulls/100/reviews/777"; do + u="${row%%|*}" + endpoint="${row##*|}" + : > "${GH_ARGS_FILE}" + run tool_comment_edit "{\"url\": \"${u}\", \"body\": \"x\"}" + assert_gh_call_equal 1 "api ${endpoint}" + done +} + +@test "comment_edit rejects an anchor that does not fit the kind or suffix of the URL, before any gh call" { + local u + for u in "https://github.com/shopware/shopware/pull/100" \ + "https://github.com/shopware/shopware/pull/100#event-5" \ + "https://github.com/shopware/shopware/pull/100#issuecomment-abc" \ + "https://github.com/shopware/shopware/pull/abc#issuecomment-5" \ + "https://github.com/shopware/shopware/pull/100#pullrequestreview-" \ + "https://github.com/shopware/shopware/issues/7#discussion_r5" \ + "https://github.com/shopware/shopware/issues/7/files#r5" \ + "https://github.com/shopware/shopware/issues/7#pullrequestreview-5" \ + "https://github.com/shopware/shopware/pull/100#r5" \ + "https://github.com/shopware/shopware/pull/100/files#discussion_r5" \ + "https://github.com/shopware/shopware/pull/100/files#pullrequestreview-5" \ + "https://github.com/shopware/shopware/pull/100/commits#issuecomment-5"; do + run tool_comment_edit "{\"url\": \"${u}\", \"body\": \"x\"}" + [[ "${status}" -ne 0 ]] || fail "Expected '${u}' to be rejected" + [[ "${output}" == "Error: url must be "* ]] || fail "Expected the form error for '${u}', got: ${output}" + done + assert_gh_not_called +} + +@test "comment_edit rejects a malformed sha segment, a misplaced query string, and a sha form on an issues URL before any gh call" { + local sha41="0123456789abcdef0123456789abcdef012345678" + local u + for u in "shopware/shopware/pull/100/commits/abc123#r5" \ + "shopware/shopware/pull/100/commits/xyz1234#r5" \ + "shopware/shopware/pull/100/commits/${sha41}#r5" \ + "shopware/shopware/pull/100/commits/abc1234..def5678#r5" \ + "shopware/shopware/pull/100/commits/abc1234/files#r5" \ + "shopware/shopware/pull/100/files/abc123#r5" \ + "shopware/shopware/pull/100/files/${sha41}#r5" \ + "shopware/shopware/pull/100/files/abc1234#discussion_r5" \ + "shopware/shopware/pull/100/files/abc1234/files#r5" \ + "shopware/shopware/pull/100/changes/abc123#r5" \ + "shopware/shopware/pull/100/changes/xyz1234#r5" \ + "shopware/shopware/pull/100/changes/${sha41}#r5" \ + "shopware/shopware/pull/100/changes/abc1234..#r5" \ + "shopware/shopware/pull/100/changes/abc1234..def567#r5" \ + "shopware/shopware/pull/100/changes/abc1234...def5678#r5" \ + "shopware/shopware/pull/100/changes/abc1234#issuecomment-5" \ + "shopware/shopware/pull/100/changes/abc1234#pullrequestreview-5" \ + "shopware/shopware/issues/100/changes/abc1234#r5" \ + "shopware/shopware/issues/100/files/abc1234#r5" \ + "shopware/shopware/pull/100/files/abc1234..#r5" \ + "shopware/shopware/pull/100/files/abc1234..def567#r5" \ + "shopware/shopware/pull/100/files/abc1234...def5678#r5" \ + "shopware/shopware/pull/100/files/${sha41}..abc1234#r5" \ + "shopware/shopware/pull/100/commits/abc1234#issuecomment-5" \ + "shopware/shopware/issues/100/commits/abc1234#r5" \ + "shopware/shopware/pull/100/commits/abc1234#r5?x=1" \ + "shopware/shopware/pull/100?x=1"; do + run tool_comment_edit "{\"url\": \"${u}\", \"body\": \"x\"}" + [[ "${status}" -ne 0 ]] || fail "Expected '${u}' to be rejected" + [[ "${output}" == "Error: url must be "* ]] || fail "Expected the form error for '${u}', got: ${output}" + done + assert_gh_not_called +} + +@test "comment_edit rejects a host that only starts with github.com, in any case, before any gh call" { + local prefix + for prefix in "https://GitHub.com.example.org/" "https://WWW.github.com.example.org/" "https://github.community/" "https://gist.github.com/"; do + run tool_comment_edit "{\"url\": \"${prefix}shopware/shopware/issues/7#issuecomment-3\", \"body\": \"x\"}" + [[ "${status}" -ne 0 ]] || fail "Expected '${prefix}' to be rejected" + assert_output --partial "must be a github.com URL" + done + assert_gh_not_called +} + +@test "comment_edit accepts an owner with an underscore and reaches gh" { + GH_STUB_GET_OUTPUT='{"id":1,"node_id":"IC_node1","issue_url":"https://api.github.com/repos/mona_octocorp/repo/issues/7","user":{"login":"me"}}' + GH_STUB_WRITE_OUTPUT="https://example/c" + run tool_comment_edit '{"url": "mona_octocorp/repo/issues/7#issuecomment-1", "body": "x"}' + assert_success + assert_output "https://example/c" + assert_gh_call_equal 1 "api repos/mona_octocorp/repo/issues/comments/1" + assert_gh_call_equal 3 "api repos/mona_octocorp/repo/issues/comments/1 -X PATCH -f body=x --jq .html_url // empty" +} + +@test "comment_edit requires url" { + run tool_comment_edit '{"body": "x"}' + assert_failure + assert_output --partial "url is required" + assert_gh_not_called +} + +@test "comment_edit requires body" { + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-3"}' + assert_failure + assert_output --partial "body is required" + assert_gh_not_called +} + +@test "comment_edit rejects a URL on another host" { + run tool_comment_edit '{"url": "https://ghe.example.com/shopware/shopware/pull/1#issuecomment-5", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "must be a github.com URL" + assert_gh_not_called +} + +@test "comment_edit rejects a bare host that is not github.com" { + run tool_comment_edit '{"url": "ghe.example.com/shopware/pull/1#issuecomment-5", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "url must be" + assert_gh_not_called +} + +@test "comment_edit rejects a repository named .." { + run tool_comment_edit '{"url": "shopware/../issues/7#issuecomment-5", "body": "x"}' + assert_failure + assert_output --partial "url must be" + assert_gh_not_called +} + +# ---- the edit request's result ------------------------------------------------ + +@test "comment_edit fails with the call named and gh's message when the PATCH fails" { + stub_get_issue 7 + GH_STUB_WRITE_EXIT=1 + GH_STUB_WRITE_OUTPUT='{"message":"Validation Failed"}' + GH_STUB_WRITE_STDERR="gh: Validation Failed (HTTP 422)" + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-5", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "PATCH repos/shopware/shopware/issues/comments/5 failed: gh: Validation Failed (HTTP 422)" +} + +@test "comment_edit names the failed call and keeps gh's exit status when the PATCH fails with no output" { + stub_get_issue 7 + GH_STUB_WRITE_EXIT=3 + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-3", "body": "x"}' + assert_failure 3 + assert_error_output + assert_output --partial "PATCH repos/shopware/shopware/issues/comments/3 failed: gh failed with exit 3 and no output" +} + +@test "comment_edit fails with a message when the PATCH succeeds with no output" { + stub_get_issue 7 + GH_STUB_WRITE_OUTPUT="" + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-3", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "was sent and GitHub reported success but returned no URL, so the comment may already hold the new body" +} + +@test "comment_edit fails when the PATCH response has no html_url field" { + stub_get_issue 7 + GH_STUB_APPLY_JQ=1 + GH_STUB_WRITE_OUTPUT='{"id":3,"body":"x"}' + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-3", "body": "x"}' + assert_failure + assert_error_output + assert_output --partial "was sent and GitHub reported success but returned no URL, so the comment may already hold the new body" + refute_output --partial "null" +} + +@test "comment_edit returns html_url from the PATCH response when the stub applies the filter" { + stub_get_issue 7 + GH_STUB_APPLY_JQ=1 + GH_STUB_WRITE_OUTPUT='{"id":3,"html_url":"https://example/c"}' + run tool_comment_edit '{"url": "shopware/shopware/issues/7#issuecomment-3", "body": "x"}' + assert_success + assert_output "https://example/c" +} + +# ---- argv integrity --------------------------------------------------------------- + +# Every argument of the last gh call, one array element each. +read_gh_argv() { + local n + n=$(find "${BATS_TEST_TMPDIR}" -maxdepth 1 -name 'gh_argv.*' | wc -l | tr -d ' ') + GH_ARGV=() + local a + while IFS= read -r -d '' a; do + GH_ARGV+=("$a") + done < "${BATS_TEST_TMPDIR}/gh_argv.${n}" +} + +# Exactly one argument equals the expected text. +assert_one_argv_equal() { + local expected="$1" arg count=0 + for arg in "${GH_ARGV[@]}"; do + [[ "${arg}" == "${expected}" ]] && count=$((count + 1)) + done + [[ ${count} -eq 1 ]] || fail "Expected exactly one argument equal to '${expected}', found ${count} in: $(printf '[%s] ' "${GH_ARGV[@]}")" +} + +TRICKY_BODY=$'-leading dash, two words a=b @file.txt "double" \'single\'\nsecond line' + +@test "comment_edit passes a body with a newline, spaces, =, @, quotes, and a leading dash as exactly one REST argument" { + stub_get_issue 7 + GH_STUB_WRITE_OUTPUT="https://example/c" + local json + json=$(jq -cn --arg b "${TRICKY_BODY}" '{url: "shopware/shopware/issues/7#issuecomment-3", body: $b}') + run tool_comment_edit "${json}" + assert_success + read_gh_argv + assert_one_argv_equal "body=${TRICKY_BODY}" +} + +@test "comment_edit passes the same body as exactly one argument to the GraphQL mutation" { + GH_STUB_GET_EXIT=1 + GH_STUB_GET_STDERR="${NOT_FOUND_STDERR}" + GH_STUB_LOOKUP_OUTPUT="${PENDING_MATCH}" + GH_STUB_MUTATION_OUTPUT="${MUTATION_OK}" + local json + json=$(jq -cn --arg b "${TRICKY_BODY}" '{url: "https://github.com/shopware/shopware/pull/100#discussion_r5551", body: $b}') + run tool_comment_edit "${json}" + assert_success + assert_no_rest_write + read_gh_argv + assert_one_argv_equal "body=${TRICKY_BODY}" +} diff --git a/plugins/github-mcp/AGENTS.md b/plugins/github-mcp/AGENTS.md index 69e7f34..68cf13b 100644 --- a/plugins/github-mcp/AGENTS.md +++ b/plugins/github-mcp/AGENTS.md @@ -54,7 +54,7 @@ plugins/github-mcp/ ├── issue_schema.sh # tool_issue_schema() (org issue types + issue fields, name filters) ├── issue_write.sh # tool_issue_create/edit/close/reopen/comment() ├── issue_schema_write.sh # tool_issue_type_set(), tool_issue_field_set() (name-to-ID resolution, PUT replace) - ├── review_write.sh # tool_pr_review_submit(), tool_pr_comment(), tool_pr_review_reply() + ├── review_write.sh # tool_pr_review_submit(), tool_pr_comment(), tool_pr_review_reply(), tool_comment_edit() ├── run.sh # tool_run_view(), tool_run_list(), tool_run_logs(), tool_workflow_jobs() ├── job.sh # tool_job_view(), tool_job_logs(), tool_job_annotations() ├── commit.sh # tool_commit_pulls() @@ -125,7 +125,7 @@ Claude Code / Codex ← stdout ← JSON-RPC response ← formatted output Tools in `tools-read.json` and `tools-write.json` map to bash functions with `tool_` prefix: - Uses bash arrays (`local -a cmd=("gh" "pr" "view" "${number}")`) for injection-safe argument passing - `_gh_resolve_repo()` falls back to `GH_DEFAULT_REPO` from config -- All tools support `suppress_errors` and `fallback` shared parameters +- Shared parameters (`suppress_errors`, `fallback`): which tools take them is in `REFERENCE.md` §Shared Tool Parameters - Tools with JSON output support `jq_filter` with pre-execution syntax validation - Tools that accept `grep_pattern` validate it with `_gh_validate_grep_pattern()` before calling `gh` - Log/text tools support `max_lines`, `tail_lines`, and grep parameters @@ -254,7 +254,7 @@ own unit tests are Node tests under `plugin-tests/github-mcp/pi/`: | `write_tools_issue_schema.bats` | `issue_type_set` and `issue_field_set` name resolution and value checks | | `write_tools_pr.bats` | `pr_create`/`edit`/`ready`/`merge`/`close`/`reopen` parameter handling | | `write_tools_project.bats` | `project_item_add` and `project_status_set` name-to-ID resolution | -| `write_tools_review.bats` | `pr_review_submit`/`pr_comment`/`pr_review_reply` parameter handling and REST payloads | +| `write_tools_review.bats` | `pr_review_submit`/`pr_comment`/`pr_review_reply`/`comment_edit` parameter handling and REST payloads, including `comment_edit`'s URL forms and rejections | | `pi/gate.test.ts` | `runGate()`/`runScript()`: exit codes, stderr capture, stdin handling, and the timeout that kills a gate's background children | | `pi/index.test.ts` | The extension's event handlers on a fake pi: the `github_mcp` section from session-start output (malformed output and non-zero exits add none), and which `bash` and `api_read` calls the gates block or let through | | `pi/tool-search-ranking.test.ts` | Exact-name ranking with pi's own ranker over the real tool catalogs, server descriptions, and server instructions; and that no server description or instructions term matches a tool-name term | diff --git a/plugins/github-mcp/CHANGELOG.md b/plugins/github-mcp/CHANGELOG.md index 7ee26f2..2ae5603 100644 --- a/plugins/github-mcp/CHANGELOG.md +++ b/plugins/github-mcp/CHANGELOG.md @@ -5,6 +5,25 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Added + +- `comment_edit` write tool. Edits the body of an existing comment, given the comment's URL: an + issue or PR conversation comment, an inline review comment (including one in your own unsubmitted + review), or a review summary. Commit comments and Discussions comments are not supported. The URL + can be written with or without `https://github.com/`, can carry a notification link's `?query`, and + can point at a comment shown in the Files, Changes, or commit view of a PR. `REFERENCE.md` lists + the exact forms. +- `comment_edit` refuses to edit a submitted comment written by someone else unless you pass + `allow_other_author: true`, and refuses a comment that does not belong to the issue or PR named in + the URL. Every failure comes back as a tool error; an edit that GitHub + accepts but answers without a URL is also an error, and says the comment may already hold the new + body. +- The `api` write tool (under `block_api_tool_write`) and raw `gh api` calls (under + `block_api_commands`) that edit a comment now redirect to `comment_edit`, and the block messages + for `gh pr comment` and `gh issue comment` name `comment_edit` for editing an existing comment. + ## [5.0.1] - 2026-10-03 ### Fixed diff --git a/plugins/github-mcp/README.md b/plugins/github-mcp/README.md index 36f48d8..2c1a759 100644 --- a/plugins/github-mcp/README.md +++ b/plugins/github-mcp/README.md @@ -22,7 +22,7 @@ GitHub CLI tools via MCP (Model Context Protocol). Wraps the `gh` CLI for pull r ### Write Server (gh-tooling-write) - **PR lifecycle** via `pr_create`, `pr_edit`, `pr_ready`, `pr_merge`, `pr_close`, `pr_reopen` -- **Reviews** via `pr_review_submit`, `pr_comment`, `pr_review_reply` +- **Reviews** via `pr_review_submit`, `pr_comment`, `pr_review_reply`, `comment_edit` (edits an issue or PR conversation comment, an inline review comment, or a review summary by its URL) - **Issue lifecycle** via `issue_create`, `issue_edit`, `issue_close`, `issue_reopen`, `issue_comment` - **Labels** via `label_add`, `label_remove` - **Assignees** via `assignee_add`, `assignee_remove` @@ -129,7 +129,7 @@ With enforcement disabled: |------------------------|---------|---------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `repo` | string | -- | Default repository in `owner/repo` format. Used when `repo` is not passed to a tool call, and for `search_code` and `search_commits` only when `owner` is not passed either. | | `enforce_mcp_tools` | boolean | `true` | Blocks high-level `gh` subcommands (`gh pr view`, `gh issue view`, `gh run view`, `gh search`, `gh pr create`, `gh label list`, `gh project view`, etc.) and redirects to MCP tools. Set to `false` to disable all gh hook enforcement. | -| `block_api_commands` | boolean | `false` | When `true` (and `enforce_mcp_tools` is also `true`), additionally blocks `gh api` calls for endpoints that have a dedicated MCP tool: `pulls/N/comments`, `pulls/N/reviews`, `pulls/N/files`, `pulls/N/commits`, `actions/jobs/N/logs`, `actions/jobs/N`, `check-runs/N/annotations`, `commits/SHA`, `releases`. Other `gh api` calls remain unblocked. | +| `block_api_commands` | boolean | `false` | When `true` (and `enforce_mcp_tools` is also `true`), additionally blocks `gh api` calls for endpoints that have a dedicated MCP tool: comment and review edits (`PATCH issues/comments/ID`, `PATCH pulls/comments/ID`, `PUT pulls/N/reviews/ID`), `pulls/N/comments`, `pulls/N/reviews`, `pulls/N/files`, `pulls/N/commits`, `actions/jobs/N/logs`, `actions/jobs/N`, `check-runs/N/annotations`, `commits/SHA`, `releases`. Other `gh api` calls remain unblocked. | | `enable_write_server` | boolean | `false` | When `true`, the write MCP server exposes write tools (PR creation, issue editing, reviews, etc.). When `false` (default), the write server returns an empty tools list. | | `block_api_tool_read` | boolean | `false` | When `true`, the read server's `api_read` tool blocks requests to endpoints that have a dedicated read MCP tool, suggesting the dedicated tool instead. | | `block_api_tool_write` | boolean | `false` | When `true`, the write server's `api` tool blocks requests to endpoints that have a dedicated write MCP tool, suggesting the dedicated tool instead. | @@ -158,7 +158,7 @@ Configuration is loaded in the following priority order: ## Tools Reference -31 read tools + 25 write tools organized by category. See [REFERENCE.md](./REFERENCE.md) for full parameter docs and examples. +31 read tools + 26 write tools organized by category. See [REFERENCE.md](./REFERENCE.md) for full parameter docs and examples. ### Read Server (gh-tooling) -- 31 tools @@ -177,12 +177,12 @@ Configuration is loaded in the following priority order: | Projects | `project_list`, `project_view` | | Raw API | `api_read` (GET only) | -### Write Server (gh-tooling-write) -- 25 tools +### Write Server (gh-tooling-write) -- 26 tools | Category | Tools | |--------------|------------------------------------------------------------------------------| | PR lifecycle | `pr_create`, `pr_edit`, `pr_ready`, `pr_merge`, `pr_close`, `pr_reopen` | -| Reviews | `pr_review_submit`, `pr_comment`, `pr_review_reply` | +| Reviews | `pr_review_submit`, `pr_comment`, `pr_review_reply`, `comment_edit` | | Issues | `issue_create`, `issue_edit`, `issue_close`, `issue_reopen`, `issue_comment` | | Issue schema | `issue_type_set`, `issue_field_set` | | Labels | `label_add`, `label_remove` | @@ -295,12 +295,12 @@ To allow direct CLI invocations, set `enforce_mcp_tools` to `false` in your conf | `gh pr close` | `pr_close` | | `gh pr reopen` | `pr_reopen` | | `gh pr review` | `pr_review_submit` | -| `gh pr comment` | `pr_comment` | +| `gh pr comment` | `pr_comment` / `comment_edit` | | `gh issue create` | `issue_create` | | `gh issue edit` | `issue_edit` | | `gh issue close` | `issue_close` | | `gh issue reopen` | `issue_reopen` | -| `gh issue comment` | `issue_comment` | +| `gh issue comment` | `issue_comment` / `comment_edit` | | `gh project item-add` | `project_item_add` | | `gh project item-edit` | `project_status_set` | @@ -310,6 +310,7 @@ With `block_api_commands: true`, additionally blocks `gh api` bash calls for end | Endpoint Pattern | MCP Tool | |----------------------------|---------------------------| +| `PATCH issues/comments/ID`, `PATCH pulls/comments/ID`, `PUT pulls/N/reviews/ID` | `comment_edit` | | `pulls/N/comments` | `pr_comments` | | `pulls/N/reviews` | `pr_reviews` | | `pulls/N/files` | `pr_files` | diff --git a/plugins/github-mcp/REFERENCE.md b/plugins/github-mcp/REFERENCE.md index 8d97963..6bc22ea 100644 --- a/plugins/github-mcp/REFERENCE.md +++ b/plugins/github-mcp/REFERENCE.md @@ -575,7 +575,7 @@ Tools available via the `gh-tooling-write` MCP server. Requires `enable_write_se ### Shared Tool Parameters -All gh-tooling-write MCP tools accept these parameters: +All gh-tooling-write MCP tools except `comment_edit` accept these parameters. `comment_edit` declares neither, so a failed edit always returns an error: | Parameter | Type | Default | Description | |-------------------|---------|---------|-------------------------------------------------------------------------| @@ -733,6 +733,39 @@ Use gh-tooling-write pr_review_reply with number 14642 and comment_id 1234567 an - `body` (string, required): Reply body text. - `repo` (string, optional): Repository in `owner/repo` format. +#### `comment_edit` + +Replace the body of one existing comment. Supported: a conversation comment on an issue or a PR, an inline review comment (including one in your own pending review), and a review's summary body. Commit comments and Discussions comments are not supported. The kind comes from the anchor of `url`, so pass the URL exactly as `issue_comment` or `pr_comment` returned it, as the `url` of a comment from `issue_view` or `pr_view` with `fields=comments`, or as the `html_url` from `pr_comments`, `pr_reviews`, or `pr_review_reply`. The body replaces the whole comment text; send the full new text, not a diff. Returns the updated comment's URL. + +``` +Use gh-tooling-write comment_edit with url "https://github.com/shopware/shopware/pull/14642#issuecomment-2001" and body "CI is green, ready to merge" +Use gh-tooling-write comment_edit with url "shopware/shopware/pull/14642#discussion_r1234567" and body "Fixed in b8f9a8c." +``` + +Accepted `url` forms, each with or without a leading `https://github.com/` (`http://github.com/` and `https://www.github.com/` also work; the scheme and host match without regard to case, so `https://GitHub.com/` works too; any other scheme or host is rejected). A `?query` before the `#`, such as `?notification_referrer_id=NT_x`, is ignored. `OWNER` is letters, digits, `-`, and `_` (an Enterprise Managed User account such as `mona_octocorp` has one). `N` and `ID` are digits only. `SHA` is 7 to 40 hex characters: + +| `url` form | Edits | Request | +|--------------------------------------------------------|---------------------------------|-------------------------------------------------| +| `OWNER/REPO/issues/N#issuecomment-ID` | Issue conversation comment | `PATCH repos/OWNER/REPO/issues/comments/ID` | +| `OWNER/REPO/pull/N#issuecomment-ID` | PR conversation comment | `PATCH repos/OWNER/REPO/issues/comments/ID` | +| `OWNER/REPO/pull/N#discussion_rID` | Inline review comment | `PATCH repos/OWNER/REPO/pulls/comments/ID` (see below) | +| `OWNER/REPO/pull/N/files#rID`, `.../changes#rID`, `.../files/SHA#rID`, `.../files/SHA..SHA#rID`, `.../changes/SHA#rID`, `.../changes/SHA..SHA#rID`, or `.../commits/SHA#rID` | Inline review comment | `PATCH repos/OWNER/REPO/pulls/comments/ID` (see below) | +| `OWNER/REPO/pull/N#pullrequestreview-ID` | Review summary body | `PUT repos/OWNER/REPO/pulls/N/reviews/ID` | + +Before editing, the tool reads the comment (`GET`). For a conversation comment or an inline review comment it checks that the comment belongs to the issue or PR `N` in the URL: the comment's `issue_url` must end in `/issues/N` (PR conversation comments also use `/issues/N`), or its `pull_request_url` in `/pulls/N`. A comment of another issue or PR is an error naming both numbers, and nothing is edited. A review summary is read from `GET repos/OWNER/REPO/pulls/N/reviews/ID`, which carries the PR number in its path. A failed read, including a 404 on a conversation comment or a review summary, is an error with gh's message and no edit. + +For the two inline review comment forms, a 404 on that read means the comment is not a submitted one, or the token has no access to the repository. The comment can still be in your own unsubmitted (pending) review, which the REST endpoint does not return. The tool then reads the pending reviews of PR `N` through GraphQL (the first 100 reviews, each with its first 100 comments) and searches every returned review for the comment's `fullDatabaseId`. When none matches, it reads further comment pages of a review that reports more, up to 50 pages (5,000 comments) in all. It edits a match with the `updatePullRequestReviewComment` mutation and returns the comment's `url`. No author check runs for a match: GitHub refuses to edit another user's comment, and that refusal is the mutation's error. When the lookup finds no match or fails, the error states both facts: `Error: comment_edit: GET repos/OWNER/REPO/pulls/comments/ID returned 404 (not found, or no access to the repository); pending-review lookup on PR N: `. A failed page and reaching the page limit are lookup errors. A mutation that fails is an error that starts with the same 404 statement, followed by `pending-review comment edit failed: ...`; a mutation that succeeds without a URL is the same 404 statement followed by the sent-but-no-URL message described below. A read that fails with anything other than a 404 is an error too. A failed read, lookup page, or mutation makes no further call. + +Author check: unless `allow_other_author` is `true`, the tool takes the `node_id` from the read of the comment or review summary and asks GitHub, in one GraphQL query (`node(id: ID) { ... on Comment { viewerDidAuthor } }`), whether the authenticated user wrote it. Any answer other than `viewerDidAuthor: true` (`false`, or no value) is an error that names the author (the read's `user.login`, or "a deleted user" when the read names no login) and says to pass `allow_other_author: true`, and nothing is edited. A read without a `node_id`, or a failed query, is an error and nothing is edited. With `allow_other_author: true` the tool sends no query and skips the check. A comment found through the pending-review lookup is not checked and sends no author query: the lookup searches every pending review it returns for the comment's ID, and GitHub's mutation refuses another author's comment. + +Every failure is returned as a tool error, and its message names what failed. When the edit request succeeds but GitHub's response holds no URL, the call is an error too: the message says the edit was sent and GitHub reported success but returned no URL, so the comment may already hold the new body. Output that a successful `gh` call writes to stderr, such as a warning, does not fail the edit. +Any other URL fails before the request: no anchor, an unknown anchor, a non-numeric number or ID, `#discussion_rID`, `#rID`, or `#pullrequestreview-ID` on an `issues/N` URL, `#rID` without `/files`, `/changes`, `/files/SHA`, `/files/SHA..SHA`, `/changes/SHA`, `/changes/SHA..SHA`, or `/commits/SHA`, a malformed `SHA` segment, a query string after the `#`, and extra path segments. The repository comes from the URL, so the tool takes no `repo` parameter. + +**Parameters:** +- `url` (string, required): URL of the comment to edit, with its anchor. +- `body` (string, required): New comment text. Replaces the whole existing text. +- `allow_other_author` (boolean, optional, default `false`): Editing a comment written by someone else, for example for moderation, requires `true`. + ### Issue Write Tools #### `issue_create` diff --git a/plugins/github-mcp/SETUP.md b/plugins/github-mcp/SETUP.md index 09c1945..833dfd0 100644 --- a/plugins/github-mcp/SETUP.md +++ b/plugins/github-mcp/SETUP.md @@ -104,11 +104,12 @@ ### PR reviews and comments - **Recommended**: ask - **Optional**: Yes (skip if `enable_write_server` is `false` or the user declined to enable the write server in Phase 3) -- **Description**: Posting PR reviews (with optional inline code comments), top-level PR comments, and threaded replies to review comments. Lower-risk than PR lifecycle changes but still visible to other contributors. +- **Description**: Posting PR reviews (with optional inline code comments), top-level PR comments, threaded replies to review comments, and editing an existing comment. Lower-risk than PR lifecycle changes but still visible to other contributors. - **Patterns**: - `mcp__plugin_github-mcp_gh-tooling-write__pr_review_submit` - `mcp__plugin_github-mcp_gh-tooling-write__pr_comment` - `mcp__plugin_github-mcp_gh-tooling-write__pr_review_reply` + - `mcp__plugin_github-mcp_gh-tooling-write__comment_edit` ### Issue write tools - **Recommended**: ask diff --git a/plugins/github-mcp/hooks/prompts/write-operations-enabled.md b/plugins/github-mcp/hooks/prompts/write-operations-enabled.md index b96217b..33424e5 100644 --- a/plugins/github-mcp/hooks/prompts/write-operations-enabled.md +++ b/plugins/github-mcp/hooks/prompts/write-operations-enabled.md @@ -1,6 +1,6 @@ ## Write (gh-tooling-write) PRs: pr_create, pr_edit, pr_ready, pr_merge, pr_close, pr_reopen -Reviews: pr_review_submit, pr_comment, pr_review_reply +Reviews: pr_review_submit, pr_comment, pr_review_reply, comment_edit (replaces the body of one existing issue or PR conversation comment, inline review comment, or review summary, found by its url; another author's comment needs allow_other_author: true) Issues: issue_create, issue_edit, issue_close, issue_reopen, issue_comment Issue type and fields: issue_type_set, issue_field_set (issue_field_set replaces the issue's whole set of field values; read them first with issue_view and with_field_values true) Labels: label_add, label_remove diff --git a/plugins/github-mcp/hooks/scripts/check-api-tools.sh b/plugins/github-mcp/hooks/scripts/check-api-tools.sh index 85a0b3a..73cee86 100755 --- a/plugins/github-mcp/hooks/scripts/check-api-tools.sh +++ b/plugins/github-mcp/hooks/scripts/check-api-tools.sh @@ -166,6 +166,20 @@ if [[ "$IS_WRITE" == "true" ]]; then block_tool "issue_edit or issue_type_set" "Use issue_edit for title, body, labels, and assignees, or issue_type_set for the issue type." fi + # Edit an existing comment: issue/PR conversation comment, inline review comment, + # review body. The anchored comment URL is the tool's input. + if [[ "$METHOD" == "PATCH" ]] && echo "$ENDPOINT_PATH" | grep -qE '(^|/)repos/[^/]+/[^/]+/issues/comments/[0-9]+/?$'; then + block_tool "comment_edit" "Use comment_edit with the comment's url and the new body. Take the url from the comment's own url field in issue_view or pr_view with fields=comments, or from the one issue_comment or pr_comment returned." + fi + + if [[ "$METHOD" == "PATCH" ]] && echo "$ENDPOINT_PATH" | grep -qE '(^|/)repos/[^/]+/[^/]+/pulls/comments/[0-9]+/?$'; then + block_tool "comment_edit" "Use comment_edit with the inline comment's url (the html_url from pr_comments, or the one pr_review_reply returned) and the new body." + fi + + if [[ "$METHOD" == "PUT" ]] && echo "$ENDPOINT_PATH" | grep -qE '(^|/)repos/[^/]+/[^/]+/pulls/[0-9]+/reviews/[0-9]+/?$'; then + block_tool "comment_edit" "Use comment_edit with the review's html_url (from pr_reviews, ending in #pullrequestreview-ID) and the new body." + fi + # PR review comment thread replies (POST) if [[ "$METHOD" == "POST" ]] && echo "$ENDPOINT" | grep -qE 'pulls/[0-9]+/comments/[0-9]+/replies$'; then block_tool "pr_review_reply" "Use pr_review_reply with number, comment_id, and body." diff --git a/plugins/github-mcp/hooks/scripts/check-gh-tools.sh b/plugins/github-mcp/hooks/scripts/check-gh-tools.sh index 552e76a..28c9b81 100644 --- a/plugins/github-mcp/hooks/scripts/check-gh-tools.sh +++ b/plugins/github-mcp/hooks/scripts/check-gh-tools.sh @@ -145,7 +145,7 @@ fi if echo "$COMMAND" | grep -qE '(^|;|&&|\|)\s*gh\s+pr\s+comment(\s|$)'; then block_tool "mcp__gh-tooling-write__pr_comment" \ - "Use pr_comment with number and body parameters." + "Use pr_comment with number and body parameters. To edit an existing comment (gh pr comment --edit-last), use comment_edit with the comment's url and the new body." fi # ============================================================================ @@ -174,7 +174,7 @@ fi if echo "$COMMAND" | grep -qE '(^|;|&&|\|)\s*gh\s+issue\s+comment(\s|$)'; then block_tool "mcp__gh-tooling-write__issue_comment" \ - "Use issue_comment with number and body parameters." + "Use issue_comment with number and body parameters. To edit an existing comment (gh issue comment --edit-last), use comment_edit with the comment's url and the new body." fi # ============================================================================ @@ -228,6 +228,21 @@ fi if [[ "$BLOCK_API_COMMANDS" == "true" ]]; then + # Editing an existing comment or review body → comment_edit. Checked before + # the reads below, so PUT .../pulls/N/reviews/ID does not land on pr_reviews. + # The method flag may come before or after the endpoint. + if echo "$COMMAND" | grep -qE 'gh\s+api\s.*repos/[^/[:space:]]+/[^/[:space:]]+/(issues|pulls)/comments/[0-9]+([^0-9/]|$)' \ + && echo "$COMMAND" | grep -qE '(-X|--method)(\s+|=)?PATCH([^A-Za-z]|$)'; then + block_tool "mcp__gh-tooling-write__comment_edit" \ + "Use comment_edit with the comment's url and the new body." + fi + + if echo "$COMMAND" | grep -qE 'gh\s+api\s.*repos/[^/[:space:]]+/[^/[:space:]]+/pulls/[0-9]+/reviews/[0-9]+([^0-9/]|$)' \ + && echo "$COMMAND" | grep -qE '(-X|--method)(\s+|=)?PUT([^A-Za-z]|$)'; then + block_tool "mcp__gh-tooling-write__comment_edit" \ + "Use comment_edit with the review's url and the new body." + fi + # PR inline review comments → pr_comments if echo "$COMMAND" | grep -qE 'gh\s+api\s+repos/[^/[:space:]]+/[^/[:space:]]+/pulls/[0-9]+/comments'; then block_tool "mcp__gh-tooling__pr_comments" \ diff --git a/plugins/github-mcp/mcp-server-gh/lib/review_write.sh b/plugins/github-mcp/mcp-server-gh/lib/review_write.sh index 33e0821..c67383f 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/review_write.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/review_write.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # Review write tools for gh-tooling MCP server (write operations) -# Tools: pr_review_submit, pr_comment, pr_review_reply +# Tools: pr_review_submit, pr_comment, pr_review_reply, comment_edit # Submit a review on a pull request, optionally with inline comments. # @@ -230,3 +230,382 @@ tool_pr_review_reply() { fi echo "${__raw}" } + +####################################### +# Run one gh command for comment_edit and hand back stdout, stderr, and the +# exit status. Every gh call of the tool goes through here, so a failed call +# is described one way. stdout and stderr stay apart, so a warning on a +# successful call never joins the value. +# Arguments: +# $1 name of the caller's variable that receives stdout on success and the +# problem text on failure, +# $2 name of the caller's variable that receives stderr, +# $3... the gh command and its arguments. +# Outputs: +# Nothing on stdout; the results go to the variables named by $1 and $2. +# Returns: +# 0 on success; otherwise gh's exit status. The problem text is gh's +# stderr, else its stdout (gh api prints an HTTP error's JSON body there), +# else "gh failed with exit N and no output". +####################################### +_comment_edit_gh() { + local __ceh_var="$1" __ceh_err_var="$2" + shift 2 + local __ceh_out __ceh_err __ceh_exit=0 + + _gh_capture_split __ceh_out __ceh_err "$@" || __ceh_exit=$? + printf -v "${__ceh_err_var}" '%s' "${__ceh_err}" + if [[ ${__ceh_exit} -ne 0 ]]; then + printf -v "${__ceh_var}" '%s' "${__ceh_err:-${__ceh_out:-gh failed with exit ${__ceh_exit} and no output}}" + return "${__ceh_exit}" + fi + printf -v "${__ceh_var}" '%s' "${__ceh_out}" +} + +####################################### +# Run one gh api graphql call for comment_edit and classify the result. On top +# of _comment_edit_gh, an empty stdout, an `errors` field, or stdout that is +# not JSON each count as a failure. stdout alone is parsed; stderr only +# explains a failed call, so a warning on a successful call never fails it. +# Arguments: +# $1 name of the caller's variable that receives the result, +# $2... the gh command and its arguments. +# Outputs: +# Nothing on stdout; the result goes to the variable named by $1. +# Returns: +# 0 with the response JSON in the variable. Otherwise gh's exit status (1 +# when gh exited 0) with the problem text in the variable. +####################################### +_comment_edit_graphql() { + local __ceg_var="$1" + shift + local __ceg_out __ceg_err __ceg_exit=0 __ceg_problem="" + + _comment_edit_gh __ceg_out __ceg_err "$@" || __ceg_exit=$? + if [[ ${__ceg_exit} -ne 0 ]]; then + __ceg_problem="${__ceg_out}" + elif [[ -z "${__ceg_out}" ]]; then + __ceg_problem="GitHub returned no output" + elif ! __ceg_problem=$(printf '%s' "${__ceg_out}" | jq -r 'if (.errors // empty) then "GraphQL errors: \(.errors | tostring)" else empty end' 2>/dev/null); then + __ceg_problem="unparseable response: ${__ceg_out}" + fi + + if [[ -n "${__ceg_problem}" ]]; then + [[ ${__ceg_exit} -ne 0 ]] || __ceg_exit=1 + printf -v "${__ceg_var}" '%s' "${__ceg_problem}" + return "${__ceg_exit}" + fi + printf -v "${__ceg_var}" '%s' "${__ceg_out}" +} + +####################################### +# Find a comment in the pending reviews of a PR and return its GraphQL node ID. +# Every pending review the lookup returns is searched; GitHub refuses to edit +# another user's comment, and that refusal comes back as the mutation's error. +# The first 100 comments of each review come with the lookup. When none of them +# matches, the comments of the first review with more are read page by page, +# up to 50 pages in all (5,000 comments). A page that reports more but gives no +# cursor counts as the last one. fullDatabaseId is a BigInt that GitHub +# serializes as a string. One jq program reads each response and prints +# "found", a tab, and the node ID; or "more", a tab, the cursor, and (lookup +# only) a tab and the review ID; or "end". A response without the nodes the +# program iterates makes jq fail. +# Arguments: +# $1 name of the caller's variable that receives the result, +# $2 repository in owner/repo format, +# $3 PR number, +# $4 database ID of the comment. +# Outputs: +# Nothing on stdout; the result goes to the variable named by $1. +# Returns: +# 0 with the node ID in the variable. Otherwise gh's exit status, or 1 when +# the failure did not come from a gh exit, with the problem text in the +# variable. +####################################### +_comment_edit_find_pending() { + local __cfp_var="$1" __cfp_repo="$2" __cfp_number="$3" __cfp_id="$4" + local __cfp_resp __cfp_row __cfp_exit=0 __cfp_page=1 __cfp_max_pages=50 + local kind value review_id + + # shellcheck disable=SC2016 # jq variable ($id), not a shell var + local lookup_jq=' + .data.repository.pullRequest.reviews.nodes as $reviews + | ([$reviews[] | .comments.nodes[] | select(.fullDatabaseId == $id) | .id][0]) as $found + | ([$reviews[] | select(.comments.pageInfo.hasNextPage == true and .comments.pageInfo.endCursor != null)][0]) as $more + | if $found then "found\t\($found)" + elif $more then "more\t\($more.comments.pageInfo.endCursor)\t\($more.id)" + else "end" end' + # shellcheck disable=SC2016 # jq variable ($id), not a shell var + local page_jq=' + .data.node.comments as $c + | ([$c.nodes[] | select(.fullDatabaseId == $id) | .id][0]) as $found + | if $found then "found\t\($found)" + elif $c.pageInfo.hasNextPage == true and $c.pageInfo.endCursor != null then "more\t\($c.pageInfo.endCursor)" + else "end" end' + + _comment_edit_graphql __cfp_resp gh api graphql \ + -f "query=query(\$owner: String!, \$name: String!, \$number: Int!) { repository(owner: \$owner, name: \$name) { pullRequest(number: \$number) { reviews(states: [PENDING], first: 100) { nodes { id comments(first: 100) { nodes { id fullDatabaseId } pageInfo { hasNextPage endCursor } } } } } } }" \ + -f "owner=${__cfp_repo%%/*}" -f "name=${__cfp_repo##*/}" -F "number=${__cfp_number}" || __cfp_exit=$? + if [[ ${__cfp_exit} -ne 0 ]]; then + printf -v "${__cfp_var}" '%s' "${__cfp_resp}" + return "${__cfp_exit}" + fi + if ! __cfp_row=$(printf '%s' "${__cfp_resp}" | jq -r --arg id "${__cfp_id}" "${lookup_jq}" 2>/dev/null); then + printf -v "${__cfp_var}" '%s' "unexpected response from GitHub for the pending reviews" + return 1 + fi + IFS=$'\t' read -r kind value review_id <<< "${__cfp_row}" + + while [[ "${kind}" == "more" ]]; do + if [[ ${__cfp_page} -ge ${__cfp_max_pages} ]]; then + printf -v "${__cfp_var}" '%s' "reached the limit of ${__cfp_max_pages} comment pages without finding comment ${__cfp_id}" + return 1 + fi + __cfp_page=$((__cfp_page + 1)) + log "INFO" "comment_edit: next page of comments in pending review ${review_id}" + __cfp_exit=0 + _comment_edit_graphql __cfp_resp gh api graphql \ + -f "query=query(\$reviewId: ID!, \$cursor: String!) { node(id: \$reviewId) { ... on PullRequestReview { comments(first: 100, after: \$cursor) { nodes { id fullDatabaseId } pageInfo { hasNextPage endCursor } } } } }" \ + -f "reviewId=${review_id}" -f "cursor=${value}" || __cfp_exit=$? + if [[ ${__cfp_exit} -ne 0 ]]; then + printf -v "${__cfp_var}" '%s' "reading the next comments of your pending review failed: ${__cfp_resp}" + return "${__cfp_exit}" + fi + if ! __cfp_row=$(printf '%s' "${__cfp_resp}" | jq -r --arg id "${__cfp_id}" "${page_jq}" 2>/dev/null); then + printf -v "${__cfp_var}" '%s' "unexpected response from GitHub for comment page ${__cfp_page} of your pending review" + return 1 + fi + IFS=$'\t' read -r kind value <<< "${__cfp_row}" + done + + if [[ "${kind}" != "found" ]]; then + printf -v "${__cfp_var}" '%s' "no matching comment in your pending review" + return 1 + fi + printf -v "${__cfp_var}" '%s' "${value}" +} + +####################################### +# Refuse to edit a comment somebody else wrote. GitHub answers whether the +# authenticated user wrote it: the comment's node_id, from the response of the +# read that precedes the write, goes into one GraphQL query for viewerDidAuthor +# (IssueComment, PullRequestReviewComment, and PullRequestReview all implement +# the Comment interface). The login in the read's response only names the +# author in the refusal. +# Arguments: +# $1 response of the GET of the comment or review summary, +# $2 the GET's endpoint, for the message. +# Outputs: +# An "Error: ..." message on stdout when the node ID is missing, the query +# fails, or the answer is not exactly true. +# Returns: +# 0 when the caller wrote it; otherwise gh's exit status from the query, or 1. +####################################### +_comment_edit_check_author() { + local response="$1" endpoint="$2" + local node_id author author_out author_exit=0 + + if ! node_id=$(printf '%s' "${response}" | jq -r '.node_id // empty' 2>/dev/null) || [[ -z "${node_id}" ]]; then + echo "Error: comment_edit: unexpected response from GitHub for GET ${endpoint}" + return 1 + fi + + log "INFO" "comment_edit: asking whether you wrote ${node_id}" + # shellcheck disable=SC2016 # GraphQL variable ($id), not a shell var + _comment_edit_graphql author_out gh api graphql \ + -f 'query=query($id: ID!) { node(id: $id) { ... on Comment { viewerDidAuthor } } }' \ + -f "id=${node_id}" || author_exit=$? + if [[ ${author_exit} -ne 0 ]]; then + echo "Error: comment_edit: asking whether you wrote the comment failed: ${author_out}; nothing was edited" + return "${author_exit}" + fi + if ! printf '%s' "${author_out}" | jq -e '.data.node.viewerDidAuthor == true' >/dev/null 2>&1; then + author=$(printf '%s' "${response}" | jq -r '.user | objects | .login | strings') + echo "Error: comment_edit: the comment was written by ${author:-a deleted user}, not by you; nothing was edited. Pass allow_other_author: true to edit it" + return 1 + fi +} + +####################################### +# Edit an inline comment that the REST API reported as 404 and that may sit in +# the caller's own pending review: find it there, then edit it through the +# updatePullRequestReviewComment mutation. No author check runs: GitHub refuses +# to edit another user's comment, and that refusal is the mutation's error. +# Every error starts with the 404 statement. +# Arguments: +# $1 repository in owner/repo format, +# $2 PR number, +# $3 database ID of the comment, +# $4 new body, +# $5 the statement that the REST read returned 404. +# Outputs: +# The edited comment's URL on success, otherwise an "Error: ..." message. +# Returns: +# 0 when the comment was edited; otherwise gh's exit status, or 1 when the +# failure did not come from a gh exit. +####################################### +_comment_edit_pending_edit() { + local repo="$1" number="$2" comment_id="$3" body="$4" not_found_note="$5" + local pending_node_id mutation mutation_url __exit=0 + + log "INFO" "comment_edit: pending-review lookup for comment ${comment_id} on PR ${number}" + _comment_edit_find_pending pending_node_id "${repo}" "${number}" "${comment_id}" || __exit=$? + if [[ ${__exit} -ne 0 ]]; then + echo "Error: comment_edit: ${not_found_note}; pending-review lookup on PR ${number}: ${pending_node_id}" + return "${__exit}" + fi + + log "INFO" "comment_edit: pending-review mutation for ${pending_node_id}" + _comment_edit_graphql mutation gh api graphql \ + -f "query=mutation(\$id: ID!, \$body: String!) { updatePullRequestReviewComment(input: {pullRequestReviewCommentId: \$id, body: \$body}) { pullRequestReviewComment { url } } }" \ + -f "id=${pending_node_id}" -f "body=${body}" || __exit=$? + if [[ ${__exit} -ne 0 ]]; then + echo "Error: comment_edit: ${not_found_note}; pending-review comment edit failed: ${mutation}" + return "${__exit}" + fi + mutation_url=$(printf '%s' "${mutation}" | jq -r '.data.updatePullRequestReviewComment.pullRequestReviewComment.url // empty' 2>/dev/null || true) + if [[ -z "${mutation_url}" ]]; then + echo "Error: comment_edit: ${not_found_note}; the pending-review edit was sent and GitHub reported success but returned no URL, so the comment may already hold the new body: ${mutation}" + return 1 + fi + echo "${mutation_url}" +} + +####################################### +# Replace the body of one existing comment: an issue or PR conversation +# comment, an inline review comment (including one in the caller's own pending +# review), or a review summary. Commit comments and Discussions comments are +# not supported. The kind comes from the anchor of the URL; REFERENCE.md lists +# the accepted URL forms. A URL that matches none fails before any gh call. The +# comment is read first. A conversation or inline comment is edited only when +# it belongs to the issue or PR number in the URL. Unless allow_other_author is +# true, the edit also needs the comment's author to be the caller. An inline +# comment the REST API reports as 404 may sit in the caller's own pending +# review, so it is looked up there, page by page, and edited through GraphQL; +# no author check runs there, because GitHub refuses to edit another user's +# comment. When that lookup fails or finds nothing, the error states the 404 +# and the lookup's result. Every other failure ends the call without a write. +# Arguments: +# $1 JSON arguments: url (required), body (required), allow_other_author +# (optional boolean, default false). +# Outputs: +# The edited comment's URL on success, otherwise an "Error: ..." message. +# Returns: +# 0 when the comment was edited; otherwise gh's exit status, or 1 when the +# failure did not come from a gh exit. +####################################### +tool_comment_edit() { + local args="$1" + + local url body allow_other_author + url=$(echo "${args}" | jq -r '.url // empty') + body=$(echo "${args}" | jq -r '.body // empty') + allow_other_author=$(echo "${args}" | jq -r '.allow_other_author // false') + + if [[ -z "${url}" ]]; then + echo "Error: url is required for comment_edit" + return 1 + fi + + if [[ -z "${body}" ]]; then + echo "Error: body is required for comment_edit" + return 1 + fi + + # The scheme and host match without regard to case; the path keeps its case. + local rest="${url}" prefix + for prefix in "https://github.com/" "http://github.com/" "https://www.github.com/"; do + if [[ "${rest,,}" == "${prefix}"* ]]; then + rest="${rest:${#prefix}}" + break + fi + done + + if [[ "${rest}" =~ ^[A-Za-z][A-Za-z0-9+.-]*:// ]]; then + echo "Error: url must be a github.com URL, got: '${url}'" + return 1 + fi + + # Owners may contain "_" (Enterprise Managed User accounts). A "?query" + # before the "#" is dropped. + local sha='[0-9a-fA-F]{7,40}' + local form_error="Error: url must be OWNER/REPO/issues/N#issuecomment-ID, OWNER/REPO/pull/N#issuecomment-ID, OWNER/REPO/pull/N#discussion_rID, OWNER/REPO/pull/N/files#rID, OWNER/REPO/pull/N/changes#rID, OWNER/REPO/pull/N/files/SHA#rID, OWNER/REPO/pull/N/files/SHA..SHA#rID, OWNER/REPO/pull/N/changes/SHA#rID, OWNER/REPO/pull/N/changes/SHA..SHA#rID, OWNER/REPO/pull/N/commits/SHA#rID, or OWNER/REPO/pull/N#pullrequestreview-ID (SHA is 7 to 40 hex characters; optionally with a leading https://github.com/ and a ?query before the #), got: '${url}'" + local url_re="^([A-Za-z0-9_-]+/[A-Za-z0-9_.-]+)/(issues|pull)/([0-9]+)(/files|/changes|/files/${sha}\\.\\.${sha}|/files/${sha}|/changes/${sha}\\.\\.${sha}|/changes/${sha}|/commits/${sha})?(\\?[^#]*)?#([A-Za-z0-9_-]+)\$" + if [[ ! "${rest}" =~ ${url_re} ]]; then + echo "${form_error}" + return 1 + fi + local repo="${BASH_REMATCH[1]}" kind="${BASH_REMATCH[2]}" number="${BASH_REMATCH[3]}" + local suffix="${BASH_REMATCH[4]}" anchor="${BASH_REMATCH[6]}" + # A repo named "." or ".." would let repos/OWNER/../... resolve to another endpoint. + if [[ "${repo##*/}" == "." || "${repo##*/}" == ".." ]]; then + echo "${form_error}" + return 1 + fi + + # owner_field names the field of the GET response that holds the issue or + # PR the comment belongs to; owner_path is how that URL must end. A review + # summary has neither: its endpoint already carries the PR number. + local endpoint method owner_field="" owner_path="" review_comment_id="" + if [[ "${anchor}" =~ ^issuecomment-([0-9]+)$ && -z "${suffix}" ]]; then + endpoint="repos/${repo}/issues/comments/${BASH_REMATCH[1]}" + method="PATCH" + owner_field="issue_url" + owner_path="issues/${number}" + elif [[ "${kind}" == "pull" ]] && { [[ -z "${suffix}" && "${anchor}" =~ ^discussion_r([0-9]+)$ ]] || [[ -n "${suffix}" && "${anchor}" =~ ^r([0-9]+)$ ]]; }; then + endpoint="repos/${repo}/pulls/comments/${BASH_REMATCH[1]}" + method="PATCH" + owner_field="pull_request_url" + owner_path="pulls/${number}" + review_comment_id="${BASH_REMATCH[1]}" + elif [[ "${kind}" == "pull" && -z "${suffix}" && "${anchor}" =~ ^pullrequestreview-([0-9]+)$ ]]; then + endpoint="repos/${repo}/pulls/${number}/reviews/${BASH_REMATCH[1]}" + method="PUT" + else + echo "${form_error}" + return 1 + fi + + local __out __err __exit=0 + + log "INFO" "comment_edit: GET ${endpoint}" + _comment_edit_gh __out __err gh api "${endpoint}" || __exit=$? + if [[ ${__exit} -ne 0 ]]; then + # Only an inline comment can still be edited: REST answers 404 for a + # comment inside the caller's unsubmitted review. + if [[ -n "${review_comment_id}" && "${__err}" == *"(HTTP 404)"* ]]; then + _comment_edit_pending_edit "${repo}" "${number}" "${review_comment_id}" "${body}" \ + "GET ${endpoint} returned 404 (not found, or no access to the repository)" + return $? + fi + echo "Error: comment_edit: GET ${endpoint} failed: ${__out}" + return "${__exit}" + fi + + if [[ -n "${owner_field}" ]]; then + local owner_url + if ! owner_url=$(printf '%s' "${__out}" | jq -r --arg field "${owner_field}" '.[$field] // empty' 2>/dev/null) || [[ -z "${owner_url}" ]]; then + echo "Error: comment_edit: GET ${endpoint} returned no ${owner_field}; nothing was edited" + return 1 + fi + if [[ "${owner_url}" != */"${owner_path}" ]]; then + echo "Error: comment_edit: the comment belongs to #${owner_url##*/}, not to #${number} named in the URL; nothing was edited" + return 1 + fi + fi + if [[ "${allow_other_author}" != "true" ]]; then + _comment_edit_check_author "${__out}" "${endpoint}" || return $? + fi + + log "INFO" "comment_edit: ${method} ${endpoint}" + _comment_edit_gh __out __err gh api "${endpoint}" -X "${method}" -f "body=${body}" --jq ".html_url // empty" || __exit=$? + if [[ ${__exit} -ne 0 ]]; then + echo "Error: comment_edit: ${method} ${endpoint} failed: ${__out}" + return "${__exit}" + fi + if [[ -z "${__out}" ]]; then + echo "Error: comment_edit: ${method} ${endpoint} was sent and GitHub reported success but returned no URL, so the comment may already hold the new body" + return 1 + fi + echo "${__out}" +} diff --git a/plugins/github-mcp/mcp-server-gh/tools-write.json b/plugins/github-mcp/mcp-server-gh/tools-write.json index c1a021a..2afb237 100644 --- a/plugins/github-mcp/mcp-server-gh/tools-write.json +++ b/plugins/github-mcp/mcp-server-gh/tools-write.json @@ -755,6 +755,30 @@ "additionalProperties": false } }, + { + "name": "comment_edit", + "description": "Replace the body of an issue or pull request comment, an inline comment on a diff (including one in your own unsubmitted draft), or a summary of a submitted verdict. The new body replaces the whole text. Accepted url forms, with no prefix or with a leading https://github.com/, http://github.com/, or https://www.github.com/: OWNER/REPO/issues/N#issuecomment-ID, OWNER/REPO/pull/N#issuecomment-ID, OWNER/REPO/pull/N#discussion_rID, OWNER/REPO/pull/N/files#rID (or /changes#rID, /files/SHA#rID, /files/SHA..SHA#rID, /changes/SHA#rID, /changes/SHA..SHA#rID, /commits/SHA#rID), OWNER/REPO/pull/N#pullrequestreview-ID; a ?query before the # is ignored. Any other url, including any other host, is rejected before the request. A comment written by someone else is refused unless allow_other_author is true. Returns the edited comment's URL.", + "inputSchema": { + "type": "object", + "required": ["url", "body"], + "properties": { + "url": { + "type": "string", + "description": "URL of the comment to edit, with its anchor, for example https://github.com/OWNER/REPO/pull/123#discussion_r456." + }, + "body": { + "type": "string", + "description": "New comment text. Replaces the whole existing text." + }, + "allow_other_author": { + "type": "boolean", + "description": "Editing a comment written by someone else, for example for moderation, requires true.", + "default": false + } + }, + "additionalProperties": false + } + }, { "name": "project_item_add", "description": "Add an issue or PR to a GitHub Project by project name. The server resolves the project name to its ID.", diff --git a/plugins/plugin-setup/CHANGELOG.md b/plugins/plugin-setup/CHANGELOG.md index 5f4d36c..f965dad 100644 --- a/plugins/plugin-setup/CHANGELOG.md +++ b/plugins/plugin-setup/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed +- The setup reference (`references/plugin-setup.md`) now includes the `comment_edit` permission + pattern, and the description of its "PR reviews and comments" group now mentions editing an + existing comment. + ## [1.0.0] - 2026-07-13 ### Added diff --git a/plugins/plugin-setup/skills/github-mcp-setting-up/references/plugin-setup.md b/plugins/plugin-setup/skills/github-mcp-setting-up/references/plugin-setup.md index 09c1945..833dfd0 100644 --- a/plugins/plugin-setup/skills/github-mcp-setting-up/references/plugin-setup.md +++ b/plugins/plugin-setup/skills/github-mcp-setting-up/references/plugin-setup.md @@ -104,11 +104,12 @@ ### PR reviews and comments - **Recommended**: ask - **Optional**: Yes (skip if `enable_write_server` is `false` or the user declined to enable the write server in Phase 3) -- **Description**: Posting PR reviews (with optional inline code comments), top-level PR comments, and threaded replies to review comments. Lower-risk than PR lifecycle changes but still visible to other contributors. +- **Description**: Posting PR reviews (with optional inline code comments), top-level PR comments, threaded replies to review comments, and editing an existing comment. Lower-risk than PR lifecycle changes but still visible to other contributors. - **Patterns**: - `mcp__plugin_github-mcp_gh-tooling-write__pr_review_submit` - `mcp__plugin_github-mcp_gh-tooling-write__pr_comment` - `mcp__plugin_github-mcp_gh-tooling-write__pr_review_reply` + - `mcp__plugin_github-mcp_gh-tooling-write__comment_edit` ### Issue write tools - **Recommended**: ask