Repository navigation
fix(github-mcp): stop hiding gh failures and validate tool input earlier - #13
Merged
Martin Bens (SpiGAndromeda) merged 14 commits intoOct 2, 2026
Merged
Martin Bens (SpiGAndromeda) merged 14 commits into
Martin Bens (SpiGAndromeda) merged 14 commits into
Conversation
The writing-code, writing-tests, and writing-docs skills ran on their generic defaults in this repository, without the wrapper helpers, test runners, or documentation surface map they need to apply their rules here. The three files under .claude/extensions/software-writer/ register the helpers in mcp-server-gh/lib/common.sh, hooks/scripts/lib/common.sh, and pi/gate.ts that new code must call instead of raw gh, jq, or child_process calls; the BATS and node:test runners with their shared fixtures and the rule that tests stay correct under bats --jobs; the ShellCheck, tsc, and ESLint gates CI runs; and the owner of each fact across the README, AGENTS.md, REFERENCE.md, SETUP.md, and CHANGELOG surfaces. Claude Code loads the files through the software-writer plugin. Codex reads them through the new root AGENTS.override.md, which first sends it to AGENTS.md because the override replaces that file rather than adding to it. .gitignore now excepts .claude/extensions/ from the .claude/* rule so the files are tracked. Co-Authored-By: Claude <noreply@anthropic.com>
The 31 read and 25 write tool counts appeared in README.md, plugins/github-mcp/AGENTS.md, and plugins/github-mcp/REFERENCE.md as well as in plugins/github-mcp/README.md §Tools Reference, so adding or removing a tool meant updating 11 lines in four files. The counts now stay only in §Tools Reference. The other lines name the read and write tools without a number. Co-Authored-By: Claude <noreply@anthropic.com>
…eaders The libraries mixed two header styles: short `Args:`/`Sets:` comments and `#####`-bounded Google Shell Style blocks. Every function in hooks/scripts/lib/common.sh and mcp-server-gh/lib/*.sh that used the short form now has a block with Globals, Arguments, Outputs, and Returns, matching the blocks already there. Four headers contradicted their code and are corrected: load_mcp_config and block_tool gave php-tooling examples that do not exist in this plugin, _gh_resolve_owner_repo left the bare repo step out of its precedence list, and _gh_validate_path described a '..' segment check where the code refuses '..' anywhere in the path. The hook scripts also switch from #!/bin/bash to #!/usr/bin/env bash, like the server scripts. The hooks run as `bash <script>`, so this changes no behaviour. Co-Authored-By: Claude <noreply@anthropic.com>
…uilder Eleven tool suites each carried a copy of the gh stub tail that writes GH_STUB_STDERR, prints GH_STUB_OUTPUT, and exits with GH_STUB_EXIT, and three server suites formatted JSON-RPC requests by hand, one of them splicing the tool name into a JSON string. gh_stub_respond and reset_gh_stub in plugin-tests/github-mcp/test_helper/common_setup.bash now carry the stub contract, and jsonrpc_request builds request lines with jq. Each suite keeps its own gh() for what its tests record. write_tools_issue_schema.bats moves to the shared stub too. Its old stub printed nothing once GH_STUB_EXIT was set, so its suppress_errors test now arranges a failing gh the way the real one fails: an error on stderr and no body. The writing-tests project extension names the new helpers. Co-Authored-By: Claude <noreply@anthropic.com>
_gh_post_process ran grep with `|| true`, so an invalid grep_pattern (exit 2) succeeded and returned grep's error message as the tool's output. Exit 1 still means no match and an empty result. Exit 2 and above now fails the call with "Error: grep_pattern failed on output". _gh_resolve_org dropped the stderr of its gh repo view fallback, so an authentication or network failure read as "org is required". The lookup now writes stderr to a file in MCP_CALL_TMPDIR, keeps it out of the org value on success, and appends it to the error on failure unless suppress_errors is set. _gh_validate_jq_filter now rejects a filter only when jq exits 3 and reports "compile error", instead of matching the error text alone. halt_error(3) also exits 3 and stays accepted. REFERENCE.md documents the grep_pattern error and the issue_schema org error. Co-Authored-By: Claude <noreply@anthropic.com>
Both tools passed the resolved repo to _gh_resolve_issue_node_id without validating it, and that helper splits on the first and last slash, so acme/app/extra was read as owner acme and repository extra. The tool schema puts no pattern on repo, and a malformed default from .mcp-gh-tooling.json takes the same path, so both tools now call _gh_validate_repo on the effective repo before any GitHub call. Co-Authored-By: Claude <noreply@anthropic.com>
The standard execution block discarded gh's stderr under suppress_errors but still returned gh's stdout on failure, and gh api prints an HTTP error's JSON body there, so every gh api tool returned the error as its result. Every failure branch now prints nothing under suppress_errors, as REFERENCE.md already stated. The project and issue-schema resolvers take suppress_errors as an argument and drop gh's message under it, while their not-found errors about the caller's input stay. repo_file's download branch follows the same rule. Several captures folded gh's stderr into a value parsed from stdout: the node ID lookups in sub_issue_add and sub_issue_remove, the head SHA lookup in pr_review_submit, the project and status listings, and the organization's issue types and fields. A gh warning on a successful call corrupted the value. _gh_capture_split captures the two streams apart through a file in MCP_CALL_TMPDIR, and _gh_resolve_org uses it too. read_tools_issue_schema.bats answers its gh repo view branch through gh_stub_respond. Co-Authored-By: Claude <noreply@anthropic.com>
_gh_validate_jq_filter ran the filter against null input, so until(.done; .next) hung the call and a valid filter halting with exit status 3 and the text "compile error" was rejected. The filter is now compiled behind `empty |`, which never runs it, and any non-zero exit is a compile error. Newlines around the filter keep a trailing comment from swallowing the closing parenthesis. Co-Authored-By: Claude <noreply@anthropic.com>
An invalid grep_pattern was caught only in _gh_post_process, after run_logs and job_logs had downloaded the log and search_code had spent a search request. _gh_validate_grep_pattern runs grep -E against empty input, where a valid pattern exits 1 and an invalid one 2, and pr_diff, run_logs, job_logs, search_code, and repo_file call it next to their other argument checks. The check in _gh_post_process stays behind it. Co-Authored-By: Claude <noreply@anthropic.com>
project_item_add and project_status_set built the item URL from the resolved repo without validating it, so acme/app/extra produced a URL for a repository that does not exist. They now call _gh_validate_repo, like the sub-issue tools. The check sits in each tool rather than inside _gh_resolve_repo, whose callers capture its stdout, where an error message would become the repo value. Co-Authored-By: Claude <noreply@anthropic.com>
The not-found errors joined the available names with paste -sd ', ', which takes each character as the next delimiter in turn, so the list came out as "Roadmap,Sprint Board Backlog". jq's join(", ") builds it now.
Co-Authored-By: Claude <noreply@anthropic.com>
_gh_parse_github_url split https://github.com/acme into owner acme and repo acme, because stripping up to the first slash leaves a string without one unchanged. It now returns 1 when the path after the host has no slash. Co-Authored-By: Claude <noreply@anthropic.com>
When fetching one run's jobs failed, workflow_jobs logged a warning and returned the other runs' jobs, and a jq failure on gh's output fell back to [] or to the unfiltered list. Each case read as a complete result. Every step now fails the call, or returns fallback when set, and the filters run as one jq program. The run list and each run's jobs are captured with _gh_capture_split. --paginate prints one JSON object per page and only the first was read, so jobs past a run's first page were missing. Every page is read now. The test that pinned the skip behavior is replaced. Co-Authored-By: Claude <noreply@anthropic.com>
_gh_download_file did not name the globals and the EXIT trap it sets through _gh_probe_allow_escape_flag and _gh_partial_create, and load_mcp_config did not name the PROJECT_DIR and HOOK_HOST it reads through find_mcp_config. Co-Authored-By: Claude <noreply@anthropic.com>
Martin Bens (SpiGAndromeda)
changed the title
Martin Bens (SpiGAndromeda)
deleted the
refactor/shell-conventions-and-test-helpers
branch
October 2, 2026 21:59
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes
suppress_errors: truenow returns empty output on failure for every tool, asREFERENCE.mdalready stated. The standard execution block discarded gh's stderr but still returned its stdout, wheregh apiprints an HTTP error's JSON body. The project and issue-schema resolvers take the flag too. Their not-found errors are about the caller's input and stay visible.pr_review_submitfetches, the project and status listings, the organization's issue types and fields, and theworkflow_jobsrun list and jobs._gh_capture_splitcaptures the two streams apart through a file inMCP_CALL_TMPDIR._gh_resolve_orguses it, soissue_schemacalled without an org reports whygh repo viewcould not supply one.jq_filteris compiled behindempty |and never run. A filter such asuntil(.done; .next)hung the call.grep_patternfails the call beforeghruns, onpr_diff,run_logs,job_logs,search_code, andrepo_file. Before, grep's exit status was discarded and its error message came back as the tool's output.workflow_jobsfails, or returnsfallback, when the jobs of any run cannot be fetched or read. Before, it left that run out and returned the rest as if complete. It also reads every--paginatepage; only the first reached the result.sub_issue_add,sub_issue_remove,project_item_add, andproject_status_setreject arepothat is not inowner/repoform before any GitHub call. The check sits in each tool rather than in_gh_resolve_repo, whose callers capture its stdout.urlnaming only an owner is rejected instead of resolving to owner and repositoryacme.,.paste -sd ', 'alternated between the two characters.fallbackdoes not apply when org resolution fails: like the existing "fallback does not mask an unresolvable org" case, a missing org is an input error.Cleanup
hooks/scripts/lib/common.shandmcp-server-gh/lib/*.shthat used shortArgs:/Sets:comments has a Google-style header. Headers that contradicted their code are corrected. The hook scripts use#!/usr/bin/env bash.gh_stub_respondandreset_gh_stubreplace the gh stub tail copied into eleven suites, andjsonrpc_requestreplaces the hand-formatted JSON-RPC requests in three server suites.