diff --git a/.claude/extensions/software-writer/writing-code.md b/.claude/extensions/software-writer/writing-code.md new file mode 100644 index 0000000..df59a7a --- /dev/null +++ b/.claude/extensions/software-writer/writing-code.md @@ -0,0 +1,51 @@ +## Named-value assignments + +- `project.stacks` = + - `bash` — `plugins/**/*.sh`, `.github/scripts/*.sh`, `plugin-tests/**/*.{bats,bash,sh}` + - `typescript` — `plugins/github-mcp/pi/*.ts`, `plugin-tests/github-mcp/{pi,fixtures/pi}/*.ts`; executed directly by Node ≥22.19 type stripping, no build step (`tsconfig.json`: `erasableSyntaxOnly`, `allowImportingTsExtensions`; relative imports keep the `.ts` extension) +- `code.primitives` = + | Call shape | Raw primitive | Helper | Invariant carried | + |---|---|---|---| + | Resolve the target repository from tool args | `jq` reads of `repo` / `repository` / `owner` / `url` | `_gh_resolve_owner_repo`, `_gh_resolve_owner_repo_optional` (`mcp-server-gh/lib/common.sh`) | Precedence `url` > `owner`+`repo` > `repository` > `repo` > `GH_DEFAULT_REPO`; a slash in split-form `repo` is rejected | + | Resolve an organization | `gh repo view`, raw `org` arg | `_gh_resolve_org` (`lib/common.sh`) | Result always passes `_gh_validate_org`, so it cannot steer the request to another API path | + | Validate a number, repo, SHA, or repository path argument | Ad hoc `[[ =~ ]]` checks | `_gh_validate_number`, `_gh_validate_repo`, `_gh_validate_sha`, `_gh_validate_path` (`lib/common.sh`) | `_gh_validate_path` rejects a leading `/` and any `..` segment | + | Apply caller-supplied `jq_filter` / `grep_pattern` / `head` / `tail` | Piping output through `jq` or `grep` directly | `_gh_validate_jq_filter`, `_gh_post_process` (`lib/common.sh`) | Filter compile-checked before use; steps run in a fixed jq → grep → head → tail order, each a no-op when unset | + | Emit `gh` output to the model | Raw `gh` stdout | `_gh_strip_ansi` (`lib/common.sh`) | No ESC byte reaches a terminal-rendering host; BSD-`sed`-compatible BRE under `LC_ALL=C` | + | Download file bytes to disk | `gh api … > file`, `mktemp` | `_gh_download_file` with `_gh_partial_create` / `_gh_partial_finish` / `_gh_partial_cleanup` (`lib/common.sh`) | Unique `noclobber` naming, atomic finish, partial file removed on cancellation | + | Locate the project config file | Hand-listed `.claude` / `.codex` / `.pi` candidate paths | `_load_gh_config` (`mcp-server-gh/lib/common.sh`, servers), `find_mcp_config` (`hooks/scripts/lib/common.sh`, hooks), both ordered by `github_mcp_config_dirs` (`shared/config-dirs.sh`) | The active host's directory takes priority, and servers and hooks agree on one host order | + | Read a config key | `jq` on `$GH_TOOLING_CONFIG_FILE` | `_gh_config_value` (`lib/common.sh`) | Returns the default when the file is missing or the key empty | + | Run a hook script from the pi extension | `child_process.spawn` | `runScript`, `runGate` (`plugins/github-mcp/pi/gate.ts`) | Never rejects; timeout kills the whole process group; sets `GITHUB_MCP_HOST=pi` and drops `CLAUDE_PROJECT_DIR`; blocks only on exit 2 | + | Invoke `gh` | A command string | `local -a cmd=(gh …)` then `"${cmd[@]}"` | Arguments stay array elements; never `eval` | +- `code.di_pattern` = TypeScript: the default export `githubMcp(pi: ExtensionAPI)` in `plugins/github-mcp/pi/index.ts` is the composition root; it computes script paths and timeouts and wires `pi.on(...)` handlers. Modules beside it (`gate.ts`) take script path, input, and timeout as parameters and hold no module-level state. Bash: a tool function `tool_()` takes the JSON args string as `$1`; host and project context enter through environment variables set at the server or hook entry point (`GITHUB_MCP_HOST`, `CLAUDE_PROJECT_DIR`, `MCP_GH_TOOLING_CONFIG`, `GH_DEFAULT_REPO`, `PROJECT_ROOT`); `gh` resolves from PATH, so tests substitute a `gh()` function. +- `code.export_conventions` = + - `plugins/github-mcp/pi/index.ts` exports only the default extension factory named in root `package.json` `pi.extensions`; helpers live in sibling modules as named exports. + - Import `@earendil-works/*` packages only with `import type`; the extension carries no runtime dependency on pi (`@typescript-eslint/no-restricted-imports` in `eslint.config.mjs` enforces this for `plugins/github-mcp/pi/**/*.ts`). + - A new MCP tool is a `tool_()` in `mcp-server-gh/lib/.sh` plus its schema entry in `tools-read.json` or `tools-write.json`; a server runs only the tools its list declares. Before adding one, read `plugins/github-mcp/AGENTS.md` §When to Modify What. +- `code.footgun_additions` = + - `plugins/github-mcp/shared/mcpserver_core.sh` is vendored from `shopwareLabs/bash-mcp-sdk`: never edit it; protocol changes go upstream and arrive via `.mcp-sdk.lock` plus `.github/scripts/vendor-mcp-sdk.sh`. + - pi extension `.ts` files run under Node type stripping: no `enum`, `namespace`, parameter properties, or other non-erasable syntax. + - A new runtime file outside the root `package.json` `files` globs is missing from the npm package; only `plugin-tests/github-mcp/pi_e2e.bats`, which runs pi against the packed tarball, surfaces it. + - Host-specific behavior branches only on `GITHUB_MCP_HOST` (servers) or `HOOK_HOST` (hooks); runtime scripts stay shared across Claude Code, Codex, and pi. + - Adding or removing a plugin, MCP server, or hook requires running `.github/scripts/update-issue-templates.sh`; CI fails on stale issue-template dropdowns. +- `comments.preserve_patterns` = + - `# shellcheck disable=…` and `# shellcheck source=…` directives, together with the justification comment beside them + - The `# Tools: ` header line in `plugins/github-mcp/mcp-server-gh/lib/*.sh` +- `domain.terms` = `read server`, `write server`, `gh-tooling`, `gh-tooling-write` (server IDs; distinct from the plugin name `github-mcp`), `host`, `SessionStart directive`, `enforcement`, `escape hatch`, `enforce_mcp_tools`, `enable_write_server`, `block_api_commands`, `block_api_tool_read`, `block_api_tool_write` +- `docs.surfaces` = + | Surface | Owns | Shape | Single-owner | + |---|---|---|---| + | `README.md` | Marketplace pitch, per-host install commands, component summary, pointers into plugin docs | Root README | enforced | + | `AGENTS.md` | Three-host model, runtime-vs-docs file split, repository architecture, development workflow, version bumps, test commands, pre-release checklist, release and distribution | Architecture document | enforced | + | `CLAUDE.md`, `plugins/plugin-setup/CLAUDE.md` | Nothing; a one-line `@AGENTS.md` import. Gitignored and local to each checkout: never the home for a shared fact | LLM pointer file | not applicable | + | `plugins/github-mcp/README.md` | Configuration keys and lookup order, tool catalog and tool counts, write server, label semantics, enforcement layers, blocked commands, pi notes, troubleshooting, dependencies | Module README | enforced, except §Quick Start: exempt duplicate of the root README install commands because this file is the npm package homepage and must stand alone | + | `plugins/github-mcp/AGENTS.md` | Plugin file layout, config-loading internals, protocol flow, tool dispatch convention, standard execution block, task-to-file navigation, suite-to-coverage table | Module architecture and navigation document | enforced | + | `plugins/github-mcp/REFERENCE.md` | Per-tool parameters and examples | Reference | enforced | + | `plugins/github-mcp/SETUP.md` | Interactive setup procedure, permission groups | Guide | exempt: kept byte-identical to `plugins/plugin-setup/skills/github-mcp-setting-up/references/plugin-setup.md` | + | `plugins/plugin-setup/README.md`, `plugins/plugin-setup/AGENTS.md` | Setup-plugin install and usage; skill layout, setup-copy sync procedure, design decisions | Module README; module navigation document | enforced | + | `plugins/github-mcp/CHANGELOG.md`, `plugins/plugin-setup/CHANGELOG.md` | Released changes per version | Changelog | exempt: each entry restates what its release changed | + + Runtime prompt text under `plugins/github-mcp/hooks/prompts/*.md` and `plugins/plugin-setup/skills/*/SKILL.md` are runtime files, not documentation surfaces. + +## Post-Step-6 + +After editing a `.sh` file, run `shellcheck --shell=bash` on it; CI lints all of `plugins/`, `plugin-tests/`, and `.github/scripts/`. After editing a `.ts` file, run `npx tsc --noEmit -p .` and `npx eslint . --max-warnings 0`. diff --git a/.claude/extensions/software-writer/writing-docs.md b/.claude/extensions/software-writer/writing-docs.md new file mode 100644 index 0000000..1d3c091 --- /dev/null +++ b/.claude/extensions/software-writer/writing-docs.md @@ -0,0 +1,25 @@ +## Named-value assignments + +- `docs.surfaces` = + | Surface | Owns | Shape | Single-owner | + |---|---|---|---| + | `README.md` | Marketplace pitch, per-host install commands, component summary, pointers into plugin docs | Root README | enforced | + | `AGENTS.md` | Three-host model, runtime-vs-docs file split, repository architecture, development workflow, version bumps, test commands, pre-release checklist, release and distribution | Architecture document | enforced | + | `CLAUDE.md`, `plugins/plugin-setup/CLAUDE.md` | Nothing; a one-line `@AGENTS.md` import. Gitignored and local to each checkout: never the home for a shared fact | LLM pointer file | not applicable | + | `plugins/github-mcp/README.md` | Configuration keys and lookup order, tool catalog and tool counts, write server, label semantics, enforcement layers, blocked commands, pi notes, troubleshooting, dependencies | Module README | enforced, except §Quick Start: exempt duplicate of the root README install commands because this file is the npm package homepage and must stand alone | + | `plugins/github-mcp/AGENTS.md` | Plugin file layout, config-loading internals, protocol flow, tool dispatch convention, standard execution block, task-to-file navigation, suite-to-coverage table | Module architecture and navigation document | enforced | + | `plugins/github-mcp/REFERENCE.md` | Per-tool parameters and examples | Reference | enforced | + | `plugins/github-mcp/SETUP.md` | Interactive setup procedure, permission groups | Guide | exempt: kept byte-identical to `plugins/plugin-setup/skills/github-mcp-setting-up/references/plugin-setup.md` | + | `plugins/plugin-setup/README.md`, `plugins/plugin-setup/AGENTS.md` | Setup-plugin install and usage; skill layout, setup-copy sync procedure, design decisions | Module README; module navigation document | enforced | + | `plugins/github-mcp/CHANGELOG.md`, `plugins/plugin-setup/CHANGELOG.md` | Released changes per version | Changelog | exempt: each entry restates what its release changed | + + Runtime prompt text under `plugins/github-mcp/hooks/prompts/*.md` and `plugins/plugin-setup/skills/*/SKILL.md` are runtime files, not documentation surfaces. +- `docs.pointer_file` = `CLAUDE.md` — a gitignored, per-checkout single `@AGENTS.md` line; each `AGENTS.md` in turn opens with `@README.md` +- `docs.jargon_home` = `AGENTS.md` +- `docs.changelog` = `plugins/github-mcp/CHANGELOG.md` and `plugins/plugin-setup/CHANGELOG.md`; Keep a Changelog with `## [x.y.z] - YYYY-MM-DD` entries, each version matching its plugin manifest version + +## Post-Step-5 + +After editing `plugins/github-mcp/SETUP.md`, follow the sync procedure in `plugins/plugin-setup/AGENTS.md` §The skill, then confirm the copy with `cmp plugins/github-mcp/SETUP.md plugins/plugin-setup/skills/github-mcp-setting-up/references/plugin-setup.md`. + +When the change documents altered tool behavior, confirm both `plugins/github-mcp/README.md` and `plugins/github-mcp/REFERENCE.md` were updated. diff --git a/.claude/extensions/software-writer/writing-tests.md b/.claude/extensions/software-writer/writing-tests.md new file mode 100644 index 0000000..e090bdb --- /dev/null +++ b/.claude/extensions/software-writer/writing-tests.md @@ -0,0 +1,25 @@ +## Named-value assignments + +- `project.stacks` = + - `bash` — `plugins/**/*.sh`, `.github/scripts/*.sh`, `plugin-tests/**/*.{bats,bash,sh}` + - `typescript` — `plugins/github-mcp/pi/*.ts`, `plugin-tests/github-mcp/{pi,fixtures/pi}/*.ts`; executed directly by Node ≥22.19 type stripping, no build step (`tsconfig.json`: `erasableSyntaxOnly`, `allowImportingTsExtensions`; relative imports keep the `.ts` extension) +- `tests.frameworks` = + - BATS (bats-core + bats-support + bats-assert, vendored into `.bats/` by `.github/scripts/setup-bats.sh`) for every `plugin-tests/**/*.bats`. Run with `.bats/bats-core/bin/bats -r plugin-tests/`. No BATS framework reference ships with the skill: apply the universal rules. Every suite opens with `# bats file_tags=github-mcp,[,]` and `bats_require_minimum_version 1.11.0`; keep both on new suites. + - `node:test` for `plugin-tests/github-mcp/pi/*.test.ts`. Run with `node --test 'plugin-tests/github-mcp/pi/*.test.ts'`; type-checked by `npx tsc --noEmit -p .`. + - End-to-end layer: `plugin-tests/github-mcp/pi_e2e.bats` drives the real `node_modules/.bin/pi` with `--offline` against the scripted faux model in `plugin-tests/github-mcp/fixtures/pi/driver.ts`. Requires `npm ci` and GNU `timeout` or `gtimeout` on PATH. +- `tests.fixture_sources` = + - `plugin-tests/test_helper/common_setup.bash` — `run_hook`, `assert_hook_blocks`, `setup_config`, `setup_codex_config` for hook-script suites. + - `plugin-tests/github-mcp/test_helper/common_setup.bash` — path constants `PLUGIN_DIR`, `SCRIPTS_DIR`, `SESSION_SCRIPT`, `SHARED_DIR`, `GH_SERVER_DIR`, `GH_LIB_DIR`, and the default `setup()`; load it with `load 'test_helper/common_setup'` instead of hardcoding `plugins/github-mcp/` paths. + - Stubbing `gh` in tool-function suites: define a `gh()` function override in `setup()` that records what the tests assert on, then answers through `gh_stub_respond` (`plugin-tests/github-mcp/test_helper/common_setup.bash`: `GH_STUB_STDERR` on stderr, `GH_STUB_OUTPUT` on stdout, exit `GH_STUB_EXIT`); call `reset_gh_stub` in `setup()`. + - JSON-RPC requests to `server-read.sh` / `server-write.sh`: `jsonrpc_request [params-json]` from the same helper, never a hand-formatted JSON string. + - pi end-to-end only: `plugin-tests/github-mcp/fixtures/pi/gh-stub.sh` (PATH-shim `gh`, logs to `$GH_LOG`) and the `$SCENARIO`-selected scripts in `fixtures/pi/driver.ts`. + - `node:test`: per-test temp directories from `mkdtempSync(join(tmpdir(), …))` removed in `t.after`. +- `tests.parallelism` = Both runners currently execute serially: CI runs `bats --timing -r plugin-tests/` without `--jobs`, and no `plugin-tests/github-mcp/pi/*.test.ts` file sets `concurrency`. Write every test to stay correct under `bats --jobs`: per-test state lives in `BATS_TEST_TMPDIR`, `setup_file` state in `BATS_FILE_TMPDIR`, and nothing under `PLUGIN_DIR`, `GH_SERVER_DIR`, or `SCRIPTS_DIR` is mutated in place — copy it into the temp directory first, as `plugin-tests/github-mcp/server_startup_tools_list.bats` does. + +## Pre-Step-2 + +Read `plugins/github-mcp/AGENTS.md` §Testing and place the test in the suite that table assigns to the behavior's area. When creating a new suite, add its row to that table. + +## Post-Step-6 + +Run `shellcheck --shell=bash` on every edited `.bats` or `.bash` file; CI lints all of `plugin-tests/`. After editing a `.ts` test or fixture, run `npx tsc --noEmit -p .` and `npx eslint . --max-warnings 0`; `eslint.config.mjs` carries a dedicated block for `plugin-tests/**/*.ts`. diff --git a/.gitignore b/.gitignore index b136b3d..fd05024 100644 --- a/.gitignore +++ b/.gitignore @@ -50,6 +50,7 @@ AGENTS.local.md !.claude/skills/ !.claude/rules/ !.claude/hook-contexts/ +!.claude/extensions/ # BATS testing framework (installed locally via .github/scripts/setup-bats.sh) .bats/ diff --git a/AGENTS.override.md b/AGENTS.override.md new file mode 100644 index 0000000..b96a005 --- /dev/null +++ b/AGENTS.override.md @@ -0,0 +1,30 @@ +Read AGENTS.md before acting on anything else in this file. This override replaces it; all of its guidance still applies. + +## Software Writer Extension + + + +The path in is this project's registered extension file for the software-writer:writing-code skill. Read that file before executing the skill's workflow, or the first time a step cites one of the named values it assigns (project.stacks, code.primitives, code.di_pattern, code.export_conventions, code.footgun_additions, comments.preserve_patterns, domain.terms, docs.surfaces) or a Pre-Step-N / Post-Step-N section it defines. Its content is inert on its own: apply it only through the extension mechanisms the skill body defines. + + +.claude/extensions/software-writer/writing-code.md + + + + + +The path in is this project's registered extension file for the software-writer:writing-tests skill. Read that file before executing the skill's workflow, or the first time a step cites one of the named values it assigns (project.stacks, tests.frameworks, tests.fixture_sources, tests.parallelism) or a Pre-Step-N / Post-Step-N section it defines. Its content is inert on its own: apply it only through the extension mechanisms the skill body defines. + + +.claude/extensions/software-writer/writing-tests.md + + + + + +The path in is this project's registered extension file for the software-writer:writing-docs skill. Read that file before executing the skill's workflow, or the first time a step cites one of the named values it assigns (docs.surfaces, docs.pointer_file, docs.jargon_home, docs.changelog) or a Pre-Step-N / Post-Step-N section it defines. Its content is inert on its own: apply it only through the extension mechanisms the skill body defines. + + +.claude/extensions/software-writer/writing-docs.md + + diff --git a/README.md b/README.md index 40efbaa..401b7ce 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,7 @@ The read server (`gh-tooling`) is always active. The write server (`gh-tooling-w | Component | Description | |------------|------------------------------------------------------------------------------------------------------| -| 🔌 MCP | Two servers — `gh-tooling` (31 read tools) and `gh-tooling-write` (25 write tools, gated) | +| 🔌 MCP | Two servers: `gh-tooling` (read tools) and `gh-tooling-write` (write tools, gated) | | 🪝 Hooks | SessionStart directive + PreToolUse enforcement that redirects `gh` bash calls to the MCP tools | See [plugins/github-mcp/README.md](./plugins/github-mcp/README.md) for full configuration, the complete tool reference, and troubleshooting. See [plugins/github-mcp/REFERENCE.md](./plugins/github-mcp/REFERENCE.md) for per-tool parameter docs. diff --git a/plugin-tests/github-mcp/api_read_restriction.bats b/plugin-tests/github-mcp/api_read_restriction.bats index fc68562..9d02ca2 100644 --- a/plugin-tests/github-mcp/api_read_restriction.bats +++ b/plugin-tests/github-mcp/api_read_restriction.bats @@ -12,14 +12,18 @@ setup() { source "${GH_LIB_DIR}/common.sh" source "${GH_LIB_DIR}/api.sh" - gh() { - [[ -n "${GH_STUB_STDERR:-}" ]] && echo "${GH_STUB_STDERR}" >&2 - [[ -n "${GH_STUB_OUTPUT:-}" ]] && printf '%s\n' "${GH_STUB_OUTPUT}" - return "${GH_STUB_EXIT:-0}" - } - GH_STUB_OUTPUT="" - GH_STUB_STDERR="" - GH_STUB_EXIT=0 + gh() { gh_stub_respond; } + reset_gh_stub +} + +@test "tool_api_read with suppress_errors returns no error text when the call fails" { + # gh api prints an HTTP error's JSON body on stdout and its summary on stderr. + GH_STUB_OUTPUT='{"message":"Not Found","documentation_url":"https://docs.github.com/rest","status":"404"}' + GH_STUB_STDERR="gh: Not Found (HTTP 404)" + GH_STUB_EXIT=1 + run tool_api_read '{"endpoint": "repos/shopware/shopware/pulls/0", "suppress_errors": true}' + assert_failure + assert_output "" } @test "tool_api_read allows GET method" { diff --git a/plugin-tests/github-mcp/mcp_tool_gh.bats b/plugin-tests/github-mcp/mcp_tool_gh.bats index 8373b14..b3a4e7a 100644 --- a/plugin-tests/github-mcp/mcp_tool_gh.bats +++ b/plugin-tests/github-mcp/mcp_tool_gh.bats @@ -26,17 +26,8 @@ setup() { source "${GH_LIB_DIR}/issue.sh" source "${GH_LIB_DIR}/job.sh" - # Configurable gh stub: control via GH_STUB_OUTPUT / GH_STUB_STDERR / GH_STUB_EXIT - gh() { - [[ -n "${GH_STUB_STDERR:-}" ]] && echo "${GH_STUB_STDERR}" >&2 - [[ -n "${GH_STUB_OUTPUT:-}" ]] && printf '%s\n' "${GH_STUB_OUTPUT}" - return "${GH_STUB_EXIT:-0}" - } - - # Reset stub state between tests - GH_STUB_OUTPUT="" - GH_STUB_STDERR="" - GH_STUB_EXIT=0 + gh() { gh_stub_respond; } + reset_gh_stub } # ============================================================================= @@ -335,6 +326,64 @@ bats_test_function --description "pr_checks: fails without repo outside git" -- assert_success } +@test "_gh_validate_jq_filter: compiles the filter without running it" { + # Run on null input, this filter halts with the exit status and the text of + # a compile error; a filter such as until(.done; .next) would never return. + run _gh_validate_jq_filter '"compile error" | halt_error(3)' + assert_success + assert_output "" +} + +@test "_gh_validate_jq_filter: accepts a filter ending in a comment" { + run _gh_validate_jq_filter '.title # the PR title' + assert_success + assert_output "" +} + +@test "_gh_capture_split: keeps stdout and stderr apart and returns the command's status" { + emit_both() { + printf '%s\n' "shopware/shopware" + printf '%s\n' "A new release of gh is available" >&2 + return 3 + } + local out err rc=0 + _gh_capture_split out err emit_both || rc=$? + assert_equal "${out}" "shopware/shopware" + assert_equal "${err}" "A new release of gh is available" + assert_equal "${rc}" "3" +} + +@test "_gh_validate_grep_pattern: accepts a valid extended regex" { + run _gh_validate_grep_pattern 'FAILED|Error [0-9]+' + assert_success + assert_output "" +} + +@test "_gh_validate_grep_pattern: rejects a pattern grep -E refuses" { + run _gh_validate_grep_pattern '(' + assert_failure + assert_output --partial "Invalid grep_pattern" +} + +@test "an invalid grep_pattern is rejected before gh is called, on every tool that takes one" { + gh() { touch "${BATS_TEST_TMPDIR}/gh_called"; gh_stub_respond; } + local -a calls=( + 'tool_pr_diff {"number": 1, "grep_pattern": "("}' + 'tool_run_logs {"run_id": 1, "grep_pattern": "("}' + 'tool_job_logs {"job_id": 1, "grep_pattern": "("}' + 'tool_search_code {"search": "needle", "grep_pattern": "("}' + 'tool_repo_file {"path": "README.md", "grep_pattern": "("}' + ) + local call + for call in "${calls[@]}"; do + rm -f "${BATS_TEST_TMPDIR}/gh_called" + run "${call%% *}" "${call#* }" + assert_failure + assert_output --partial "Invalid grep_pattern" + [[ ! -f "${BATS_TEST_TMPDIR}/gh_called" ]] || fail "${call%% *} called gh before rejecting the pattern" + done +} + @test "_gh_validate_jq_filter: uses custom field name in error message" { run _gh_validate_jq_filter '{{bad' "my_filter" assert_failure @@ -391,6 +440,13 @@ bats_test_function --description "pr_checks: fails without repo outside git" -- @test "_gh_post_process: grep no matches returns success with empty output" { run _gh_post_process $'line1\nline2' "" "NOMATCH_XYZ" 0 0 false false "" "" assert_success + assert_output "" +} + +@test "_gh_post_process: invalid grep_pattern fails with error message" { + run _gh_post_process $'line1\nline2' "" "(" 0 0 false false "" "" + assert_failure + assert_output --partial "grep_pattern failed" } @test "_gh_post_process: jq filter transforms JSON output" { @@ -626,6 +682,11 @@ bats_test_function --description "pr_checks: fails without repo outside git" -- assert_equal "${_GH_URL_PATH}" "composer.json" } +@test "_gh_parse_github_url: rejects a URL that names only an owner" { + run _gh_parse_github_url "https://github.com/shopware" + assert_failure +} + @test "_gh_parse_github_url: parses repo-only URL" { _gh_parse_github_url "https://github.com/shopware/shopware" assert_equal "${_GH_URL_OWNER}" "shopware" @@ -1057,6 +1118,15 @@ bats_test_function --description "pr_checks: fails without repo outside git" -- assert_output --partial "API error: Not Found" } +@test "repo_file: download_to on failure returns no error text under suppress_errors" { + local dl_path="${BATS_TEST_TMPDIR}/suppressed.json" + GH_STUB_EXIT=1 + GH_STUB_STDERR="API error: Not Found" + run tool_repo_file '{"repository":"shopware/shopware","path":"missing.txt","download_to":"'"${dl_path}"'","suppress_errors":true}' + assert_failure + assert_output "" +} + @test "repo_file: download_to on failure returns fallback when provided" { local dl_path="${BATS_TEST_TMPDIR}/fallback_test.json" GH_STUB_EXIT=1 @@ -1398,33 +1468,50 @@ bats_test_function --description "pr_checks: fails without repo outside git" -- } } -@test "workflow_jobs: partial API failure skips failed run" { +@test "workflow_jobs: fails the call when one run's jobs cannot be fetched" { gh() { if [[ "$1" == "run" && "$2" == "list" ]]; then echo '[{"databaseId":100,"displayTitle":"Run 100","headBranch":"main","status":"completed","conclusion":"failure","createdAt":"2024-01-01T00:00:00Z"},{"databaseId":200,"displayTitle":"Run 200","headBranch":"main","status":"completed","conclusion":"success","createdAt":"2024-01-02T00:00:00Z"}]' return 0 fi - if [[ "$1" == "api" ]]; then - # Run 100 fails, run 200 succeeds - if [[ "$2" == *"/100/"* ]]; then - echo "API error" >&2 - return 1 - fi - if [[ "$2" == *"/200/"* ]]; then - echo '{"jobs":[{"id":2001,"name":"PHPStan","status":"completed","conclusion":"success","html_url":"","started_at":"","completed_at":"","steps":[]}]}' - return 0 - fi + if [[ "$2" == *"/100/"* ]]; then + echo "gh: Server Error (HTTP 502)" >&2 + return 1 fi + echo '{"jobs":[{"id":2001,"name":"PHPStan","status":"completed","conclusion":"success","html_url":"","started_at":"","completed_at":"","steps":[]}]}' } run tool_workflow_jobs '{"workflow":"CI"}' - assert_success - # Should still return jobs from the successful run - local count - count=$(echo "${output}" | jq 'length') - [[ "${count}" == "1" ]] || { - echo "Expected 1 job (from successful run), got: ${count}" + assert_failure + assert_output "Error: could not fetch the jobs of run 100: gh: Server Error (HTTP 502)" +} + +@test "workflow_jobs: returns the fallback when one run's jobs cannot be fetched" { + gh() { + if [[ "$1" == "run" && "$2" == "list" ]]; then + echo '[{"databaseId":100,"displayTitle":"Run 100","headBranch":"main","status":"completed","conclusion":"failure","createdAt":"2024-01-01T00:00:00Z"}]' + return 0 + fi + echo "gh: Server Error (HTTP 502)" >&2 return 1 } + run tool_workflow_jobs '{"workflow":"CI","fallback":"no jobs"}' + assert_success + assert_output "no jobs" +} + +@test "workflow_jobs: reads the jobs on every page of a paginated run" { + gh() { + if [[ "$1" == "run" && "$2" == "list" ]]; then + echo '[{"databaseId":100,"displayTitle":"Run 100","headBranch":"main","status":"completed","conclusion":"failure","createdAt":"2024-01-01T00:00:00Z"}]' + return 0 + fi + # --paginate prints one JSON object per page. + echo '{"total_count":2,"jobs":[{"id":1001,"name":"Unit","status":"completed","conclusion":"success","html_url":"","started_at":"","completed_at":"","steps":[]}]}' + echo '{"total_count":2,"jobs":[{"id":1002,"name":"Lint","status":"completed","conclusion":"failure","html_url":"","started_at":"","completed_at":"","steps":[]}]}' + } + run tool_workflow_jobs '{"workflow":"CI"}' + assert_success + assert_equal "$(printf '%s' "${output}" | jq -c '[.[].id]')" "[1001,1002]" } @test "workflow_jobs: suppress_errors hides run list stderr" { diff --git a/plugin-tests/github-mcp/read_tools_escape_sequences.bats b/plugin-tests/github-mcp/read_tools_escape_sequences.bats index 1da04b8..39d68ce 100644 --- a/plugin-tests/github-mcp/read_tools_escape_sequences.bats +++ b/plugin-tests/github-mcp/read_tools_escape_sequences.bats @@ -29,15 +29,11 @@ setup() { return 0 fi printf '%s\n' "$@" > "${GH_ARGS_FILE}" - [[ -n "${GH_STUB_STDERR:-}" ]] && echo "${GH_STUB_STDERR}" >&2 - [[ -n "${GH_STUB_OUTPUT:-}" ]] && printf '%s\n' "${GH_STUB_OUTPUT}" - return "${GH_STUB_EXIT:-0}" + gh_stub_respond } GH_HELP_OUTPUT=" --allow-escape-sequences Allow printing terminal escape sequences" - GH_STUB_OUTPUT="" - GH_STUB_STDERR="" - GH_STUB_EXIT=0 + reset_gh_stub } # Assert the captured gh argument list contains an exact argument. diff --git a/plugin-tests/github-mcp/read_tools_issue_schema.bats b/plugin-tests/github-mcp/read_tools_issue_schema.bats index d9a5efa..95f0ff4 100644 --- a/plugin-tests/github-mcp/read_tools_issue_schema.bats +++ b/plugin-tests/github-mcp/read_tools_issue_schema.bats @@ -30,11 +30,14 @@ setup() { printf '%s\n' "${GH_STUB_FIELDS}" ;; *) - printf '%s\n' "${GH_STUB_REPO_VIEW:-}" + # gh repo view, the git-remote fallback for the org + gh_stub_respond + return ;; esac return 0 } + reset_gh_stub GH_STUB_TYPES="${TYPES_JSON}" GH_STUB_FIELDS="${FIELDS_JSON}" GH_STUB_TYPES_EXIT="" @@ -157,6 +160,26 @@ setup() { [ ! -f "${GH_ARGS_FILE}" ] } +@test "issue_schema says why the current repository could not supply the org" { + GH_DEFAULT_REPO="" + GH_STUB_STDERR="HTTP 401: Bad credentials (https://api.github.com/graphql)" + GH_STUB_EXIT=1 + run tool_issue_schema '{}' + assert_failure + assert_output --partial "org is required for issue_schema" + assert_output --partial "HTTP 401: Bad credentials" +} + +@test "issue_schema leaves gh's reason out of the org error under suppress_errors" { + GH_DEFAULT_REPO="" + GH_STUB_STDERR="HTTP 401: Bad credentials (https://api.github.com/graphql)" + GH_STUB_EXIT=1 + run tool_issue_schema '{"suppress_errors": true}' + assert_failure + assert_output --partial "org is required for issue_schema" + refute_output --partial "Bad credentials" +} + @test "fallback does not mask an unresolvable org" { GH_DEFAULT_REPO="" run tool_issue_schema '{"org": "!!", "fallback": "ok"}' diff --git a/plugin-tests/github-mcp/read_tools_jq_filter_fields.bats b/plugin-tests/github-mcp/read_tools_jq_filter_fields.bats index 043fff6..2e1d67f 100644 --- a/plugin-tests/github-mcp/read_tools_jq_filter_fields.bats +++ b/plugin-tests/github-mcp/read_tools_jq_filter_fields.bats @@ -22,12 +22,10 @@ setup() { gh() { touch "${GH_CALLED_FILE}" - [[ -n "${GH_STUB_OUTPUT:-}" ]] && printf '%s\n' "${GH_STUB_OUTPUT}" - return "${GH_STUB_EXIT:-0}" + gh_stub_respond } - GH_STUB_OUTPUT="" - GH_STUB_EXIT=0 + reset_gh_stub } # Assert a tool rejects jq_filter without fields, before reaching gh. diff --git a/plugin-tests/github-mcp/read_tools_new.bats b/plugin-tests/github-mcp/read_tools_new.bats index 521ee35..bd69fcc 100644 --- a/plugin-tests/github-mcp/read_tools_new.bats +++ b/plugin-tests/github-mcp/read_tools_new.bats @@ -17,13 +17,9 @@ setup() { gh() { printf '%s\n' "$@" > "${GH_ARGS_FILE}" - [[ -n "${GH_STUB_STDERR:-}" ]] && echo "${GH_STUB_STDERR}" >&2 - [[ -n "${GH_STUB_OUTPUT:-}" ]] && printf '%s\n' "${GH_STUB_OUTPUT}" - return "${GH_STUB_EXIT:-0}" + gh_stub_respond } - GH_STUB_OUTPUT="" - GH_STUB_STDERR="" - GH_STUB_EXIT=0 + reset_gh_stub } # ============================================================================ diff --git a/plugin-tests/github-mcp/server_startup_tools_list.bats b/plugin-tests/github-mcp/server_startup_tools_list.bats index 6aac134..24ad9a1 100644 --- a/plugin-tests/github-mcp/server_startup_tools_list.bats +++ b/plugin-tests/github-mcp/server_startup_tools_list.bats @@ -22,7 +22,7 @@ setup() { # Pipe one initialize request into the copied read server and capture both # streams plus the exit status. run_server() { - printf '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{}}\n' \ + jsonrpc_request "initialize" 1 \ | env PROJECT_ROOT="${PROJECT_DIR}" bash "${PLUGIN_COPY}/mcp-server-gh/server-read.sh" } diff --git a/plugin-tests/github-mcp/test_helper/common_setup.bash b/plugin-tests/github-mcp/test_helper/common_setup.bash index a0083ef..65e65e4 100644 --- a/plugin-tests/github-mcp/test_helper/common_setup.bash +++ b/plugin-tests/github-mcp/test_helper/common_setup.bash @@ -22,6 +22,31 @@ SHARED_DIR="${PLUGIN_DIR}/shared" GH_SERVER_DIR="${PLUGIN_DIR}/mcp-server-gh" GH_LIB_DIR="${GH_SERVER_DIR}/lib" +# Canned gh answer for the tool-function suites. A suite's gh() stub records +# whatever its tests assert on, then answers through this: GH_STUB_STDERR on +# stderr, GH_STUB_OUTPUT on stdout, exit status GH_STUB_EXIT. +gh_stub_respond() { + [[ -n "${GH_STUB_STDERR:-}" ]] && printf '%s\n' "${GH_STUB_STDERR}" >&2 + [[ -n "${GH_STUB_OUTPUT:-}" ]] && printf '%s\n' "${GH_STUB_OUTPUT}" + return "${GH_STUB_EXIT:-0}" +} + +# Clear the canned gh answer; call from setup() so no test inherits another's. +reset_gh_stub() { + GH_STUB_OUTPUT="" + GH_STUB_STDERR="" + GH_STUB_EXIT=0 +} + +# Print one JSON-RPC 2.0 request line for piping into a server script. +# Args: $1=method, $2=id, $3=params JSON (default: {}) +jsonrpc_request() { + local method="$1" id="$2" params="${3:-}" + [[ -n "${params}" ]] || params='{}' + jq -nc --arg method "${method}" --argjson id "${id}" --argjson params "${params}" \ + '{jsonrpc: "2.0", id: $id, method: $method, params: $params}' +} + # Default setup: enforcement enabled, written under the server/config identity. # Override CONFIG_PREFIX in a test file to target a different config file, or # define a custom setup() (tool-function suites do) to replace this entirely. diff --git a/plugin-tests/github-mcp/tool_dispatch_declaration.bats b/plugin-tests/github-mcp/tool_dispatch_declaration.bats index 6829cc7..10758a5 100644 --- a/plugin-tests/github-mcp/tool_dispatch_declaration.bats +++ b/plugin-tests/github-mcp/tool_dispatch_declaration.bats @@ -27,7 +27,7 @@ setup() { # answers "Tool not found"; one that is reports a missing parameter of its own. call_tool() { local server="$1" tool="$2" project_root="$3" - printf '{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"%s","arguments":{}}}\n' "$tool" \ + jsonrpc_request "tools/call" 1 "$(jq -nc --arg name "$tool" '{name: $name, arguments: {}}')" \ | env PROJECT_ROOT="$project_root" bash "${GH_SERVER_COPY_DIR}/${server}" 2>/dev/null \ | tail -1 \ | jq -r '.result.content[0].text // .error.message // "no answer"' @@ -109,7 +109,7 @@ undeclared_tools() { local before after before=$(find "${GH_SERVER_COPY_DIR}" -maxdepth 1 -name 'tools-empty.*.json' | sort) - printf '%s\n' '{"jsonrpc":"2.0","id":1,"method":"tools/list"}' \ + jsonrpc_request "tools/list" 1 \ | env PROJECT_ROOT="${PROJECT_DIR}" bash "${GH_SERVER_COPY_DIR}/server-write.sh" >/dev/null 2>&1 after=$(find "${GH_SERVER_COPY_DIR}" -maxdepth 1 -name 'tools-empty.*.json' | sort) diff --git a/plugin-tests/github-mcp/write_server_gating.bats b/plugin-tests/github-mcp/write_server_gating.bats index 7cd788c..37d95ab 100644 --- a/plugin-tests/github-mcp/write_server_gating.bats +++ b/plugin-tests/github-mcp/write_server_gating.bats @@ -14,14 +14,6 @@ setup() { SERVER_SCRIPT="${PLUGIN_COPY}/mcp-server-gh/server-write.sh" } -# Helper: send a JSON-RPC request -send_jsonrpc() { - local method="$1" - local id="${2:-1}" - local params="${3:-{}}" - printf '{"jsonrpc":"2.0","id":%d,"method":"%s","params":%s}\n' "$id" "$method" "$params" -} - # Helper: run the server with a config and a request, capture last response run_server_request() { local config_json="$1" @@ -34,9 +26,9 @@ run_server_request() { fi local requests - requests=$(send_jsonrpc "initialize" 1) + requests=$(jsonrpc_request "initialize" 1) requests+=$'\n' - requests+=$(send_jsonrpc "${method}" 2) + requests+=$(jsonrpc_request "${method}" 2) run bash -c 'echo "$1" | bash "$2" 2>/dev/null | tail -1' _ "${requests}" "${SERVER_SCRIPT}" } @@ -70,9 +62,9 @@ run_server_request() { mkdir -p "$CLAUDE_PROJECT_DIR" export PROJECT_ROOT="$CLAUDE_PROJECT_DIR" local requests - requests=$(send_jsonrpc "initialize" 1) + requests=$(jsonrpc_request "initialize" 1) requests+=$'\n' - requests+=$(send_jsonrpc "tools/list" 2) + requests+=$(jsonrpc_request "tools/list" 2) run bash -c 'echo "$1" | bash "$2" 2>/dev/null | tail -1' _ "${requests}" "${SERVER_SCRIPT}" assert_success local tool_count @@ -90,9 +82,9 @@ run_host_server_tools_list() { local host="$1" local project_root="$2" local requests - requests=$(send_jsonrpc "initialize" 1) + requests=$(jsonrpc_request "initialize" 1) requests+=$'\n' - requests+=$(send_jsonrpc "tools/list" 2) + requests+=$(jsonrpc_request "tools/list" 2) local -a host_env=(-u GITHUB_MCP_HOST) [[ -n "$host" ]] && host_env=("GITHUB_MCP_HOST=${host}") diff --git a/plugin-tests/github-mcp/write_tools_edit_params.bats b/plugin-tests/github-mcp/write_tools_edit_params.bats index 5c5432e..ecabebf 100644 --- a/plugin-tests/github-mcp/write_tools_edit_params.bats +++ b/plugin-tests/github-mcp/write_tools_edit_params.bats @@ -16,11 +16,9 @@ setup() { GH_ARGS_FILE="${BATS_TEST_TMPDIR}/gh_args" gh() { printf '%s\n' "$*" > "${GH_ARGS_FILE}" - [[ -n "${GH_STUB_OUTPUT:-}" ]] && printf '%s\n' "${GH_STUB_OUTPUT}" - return "${GH_STUB_EXIT:-0}" + gh_stub_respond } - GH_STUB_OUTPUT="" - GH_STUB_EXIT=0 + reset_gh_stub } # Parameterized test helpers diff --git a/plugin-tests/github-mcp/write_tools_graphql.bats b/plugin-tests/github-mcp/write_tools_graphql.bats index c2c9349..70e14e6 100644 --- a/plugin-tests/github-mcp/write_tools_graphql.bats +++ b/plugin-tests/github-mcp/write_tools_graphql.bats @@ -20,11 +20,10 @@ setup() { GH_ARGS_FILE="${BATS_TEST_TMPDIR}/gh_args" gh() { printf '%s\n' "$*" > "${GH_ARGS_FILE}" - [[ -n "${GH_STUB_OUTPUT:-}" ]] && printf '%s\n' "${GH_STUB_OUTPUT}" - return "${GH_STUB_EXIT:-0}" + gh_stub_respond } + reset_gh_stub GH_STUB_OUTPUT='{"data":{"addSubIssue":{"issue":{"number":1,"title":"Parent"},"subIssue":{"number":2,"title":"Child"}}}}' - GH_STUB_EXIT=0 } # ============================================================================ @@ -60,6 +59,23 @@ setup() { assert_output --partial "repo is required" } +@test "sub_issue_add rejects a repo that is not owner/repo before calling GitHub" { + run tool_sub_issue_add '{"issue_number": 1, "sub_issue_number": 2, "repo": "acme/app/extra"}' + assert_failure + assert_output --partial "repo must be in 'owner/repo' format" + [[ ! -f "${GH_ARGS_FILE}" ]] +} + +@test "sub_issue_add with suppress_errors returns no error text when a lookup fails" { + _gh_resolve_issue_node_id() { + printf '%s\n' "Could not resolve to an Issue with the number of 999." >&2 + return 1 + } + run tool_sub_issue_add '{"issue_number": 999, "sub_issue_number": 2, "suppress_errors": true}' + assert_failure + assert_output "" +} + @test "sub_issue_add handles resolution failure" { _gh_resolve_issue_node_id() { printf '%s\n' "not found" >&2 @@ -132,6 +148,23 @@ setup() { assert_output --partial "repo is required" } +@test "sub_issue_remove rejects a repo that is not owner/repo before calling GitHub" { + run tool_sub_issue_remove '{"issue_number": 1, "sub_issue_number": 2, "repo": "acme/app/extra"}' + assert_failure + assert_output --partial "repo must be in 'owner/repo' format" + [[ ! -f "${GH_ARGS_FILE}" ]] +} + +@test "sub_issue_remove with suppress_errors returns no error text when a lookup fails" { + _gh_resolve_issue_node_id() { + printf '%s\n' "Could not resolve to an Issue with the number of 999." >&2 + return 1 + } + run tool_sub_issue_remove '{"issue_number": 999, "sub_issue_number": 2, "suppress_errors": true}' + assert_failure + assert_output "" +} + @test "sub_issue_remove handles resolution failure" { _gh_resolve_issue_node_id() { printf '%s\n' "not found" >&2 diff --git a/plugin-tests/github-mcp/write_tools_issue.bats b/plugin-tests/github-mcp/write_tools_issue.bats index ff40f67..465550d 100644 --- a/plugin-tests/github-mcp/write_tools_issue.bats +++ b/plugin-tests/github-mcp/write_tools_issue.bats @@ -14,13 +14,9 @@ setup() { GH_ARGS_FILE="${BATS_TEST_TMPDIR}/gh_args" gh() { printf '%s\n' "$*" > "${GH_ARGS_FILE}" - [[ -n "${GH_STUB_STDERR:-}" ]] && echo "${GH_STUB_STDERR}" >&2 - [[ -n "${GH_STUB_OUTPUT:-}" ]] && printf '%s\n' "${GH_STUB_OUTPUT}" - return "${GH_STUB_EXIT:-0}" + gh_stub_respond } - GH_STUB_OUTPUT="" - GH_STUB_STDERR="" - GH_STUB_EXIT=0 + reset_gh_stub } # Helper to check gh was called with expected args diff --git a/plugin-tests/github-mcp/write_tools_issue_schema.bats b/plugin-tests/github-mcp/write_tools_issue_schema.bats index 053fe36..6963a4a 100644 --- a/plugin-tests/github-mcp/write_tools_issue_schema.bats +++ b/plugin-tests/github-mcp/write_tools_issue_schema.bats @@ -26,12 +26,10 @@ setup() { esac printf '%s\n' "$@" > "${GH_ARGS_FILE}" cat > "${GH_BODY_FILE}" - [[ -n "${GH_STUB_EXIT:-}" ]] && return "${GH_STUB_EXIT}" - printf '%s\n' "${GH_STUB_OUTPUT}" - return 0 + gh_stub_respond } + reset_gh_stub GH_STUB_OUTPUT='{"number":19952,"type":{"name":"Bug"}}' - GH_STUB_EXIT="" } body() { jq -c "$1" "${GH_BODY_FILE}"; } @@ -58,6 +56,16 @@ body() { jq -c "$1" "${GH_BODY_FILE}"; } assert_equal "$(printf '%s' "${output}" | jq -r '.type')" 'null' } +@test "issue_type_set with suppress_errors returns no error text when listing the types fails" { + gh() { + printf '%s\n' "gh: Not Found (HTTP 404)" >&2 + return 1 + } + run tool_issue_type_set '{"number": 19952, "type": "bug", "suppress_errors": true}' + assert_failure + assert_output "" +} + @test "issue_type_set rejects an unknown type before calling the API" { run tool_issue_type_set '{"number": 19952, "type": "Bogus"}' assert_failure @@ -191,6 +199,9 @@ body() { jq -c "$1" "${GH_BODY_FILE}"; } } @test "issue_field_set with suppress_errors returns no error text" { + # gh api prints an HTTP error's JSON body on stdout and its summary on stderr. + GH_STUB_OUTPUT='{"message":"Not Found","documentation_url":"https://docs.github.com/rest","status":"404"}' + GH_STUB_STDERR="gh: Not Found (HTTP 404)" GH_STUB_EXIT=1 run tool_issue_field_set '{"number": 19952, "values": {}, "suppress_errors": true}' assert_failure diff --git a/plugin-tests/github-mcp/write_tools_pr.bats b/plugin-tests/github-mcp/write_tools_pr.bats index 312fe1e..80560af 100644 --- a/plugin-tests/github-mcp/write_tools_pr.bats +++ b/plugin-tests/github-mcp/write_tools_pr.bats @@ -14,13 +14,9 @@ setup() { GH_ARGS_FILE="${BATS_TEST_TMPDIR}/gh_args" gh() { printf '%s\n' "$*" > "${GH_ARGS_FILE}" - [[ -n "${GH_STUB_STDERR:-}" ]] && echo "${GH_STUB_STDERR}" >&2 - [[ -n "${GH_STUB_OUTPUT:-}" ]] && printf '%s\n' "${GH_STUB_OUTPUT}" - return "${GH_STUB_EXIT:-0}" + gh_stub_respond } - GH_STUB_OUTPUT="" - GH_STUB_STDERR="" - GH_STUB_EXIT=0 + reset_gh_stub } # Helper to check gh was called with expected args diff --git a/plugin-tests/github-mcp/write_tools_project.bats b/plugin-tests/github-mcp/write_tools_project.bats index 1f3f7e0..91e844c 100644 --- a/plugin-tests/github-mcp/write_tools_project.bats +++ b/plugin-tests/github-mcp/write_tools_project.bats @@ -18,11 +18,9 @@ setup() { GH_ARGS_FILE="${BATS_TEST_TMPDIR}/gh_args" gh() { printf '%s\n' "$*" > "${GH_ARGS_FILE}" - [[ -n "${GH_STUB_OUTPUT:-}" ]] && printf '%s\n' "${GH_STUB_OUTPUT}" - return "${GH_STUB_EXIT:-0}" + gh_stub_respond } - GH_STUB_OUTPUT="" - GH_STUB_EXIT=0 + reset_gh_stub } # ============================================================================ @@ -78,6 +76,38 @@ setup() { assert_output --partial "Available projects" } +@test "project_item_add rejects a repo that is not owner/repo before calling GitHub" { + run tool_project_item_add '{"number": 1, "type": "issue", "project": "Board A", "repo": "acme/app/extra"}' + assert_failure + assert_output --partial "repo must be in 'owner/repo' format" + [[ ! -f "${GH_ARGS_FILE}" ]] +} + +@test "project_item_add with suppress_errors returns no error text when listing projects fails" { + source "${GH_LIB_DIR}/project.sh" # the real resolver instead of setup's mock + GH_STUB_STDERR="HTTP 401: Bad credentials (https://api.github.com/graphql)" + GH_STUB_EXIT=1 + run tool_project_item_add '{"number": 1, "type": "issue", "project": "Board A", "suppress_errors": true}' + assert_failure + assert_output "" +} + +@test "an unknown project name lists the available projects comma-separated" { + source "${GH_LIB_DIR}/project.sh" # the real resolver instead of setup's mock + GH_STUB_OUTPUT='{"projects":[{"title":"Roadmap","number":1},{"title":"Sprint Board","number":2},{"title":"Backlog","number":3}]}' + run _gh_resolve_project_number "Missing" "acme" "false" + assert_failure + assert_output "Error: project 'Missing' not found. Available projects: Roadmap, Sprint Board, Backlog" +} + +@test "an unknown status lists the available options comma-separated" { + source "${GH_LIB_DIR}/project.sh" # the real resolver instead of setup's mock + GH_STUB_OUTPUT='{"fields":[{"id":"FIELD_1","name":"Status","type":"ProjectV2SingleSelectField","options":[{"id":"o1","name":"In Progress"},{"id":"o2","name":"Done"},{"id":"o3","name":"Won'"'"'t Do"}]}]}' + run _gh_resolve_status_option 42 "acme" "Missing" "false" + assert_failure + assert_output "Error: status 'Missing' not found in project 42. Available options: In Progress, Done, Won't Do" +} + # ============================================================================ # project_status_set # ============================================================================ @@ -100,6 +130,13 @@ setup() { assert_output --partial "project name is required" } +@test "project_status_set rejects a repo that is not owner/repo before calling GitHub" { + run tool_project_status_set '{"number": 1, "type": "issue", "project": "Board A", "status": "Done", "repo": "acme/app/extra"}' + assert_failure + assert_output --partial "repo must be in 'owner/repo' format" + [[ ! -f "${GH_ARGS_FILE}" ]] +} + @test "project_status_set calls item-edit with resolved IDs" { # Mock item-list to return an item matching the URL gh() { diff --git a/plugin-tests/github-mcp/write_tools_review.bats b/plugin-tests/github-mcp/write_tools_review.bats index 40fb12d..465a892 100644 --- a/plugin-tests/github-mcp/write_tools_review.bats +++ b/plugin-tests/github-mcp/write_tools_review.bats @@ -27,13 +27,9 @@ setup() { printf '%s\n' "${GH_STUB_HEAD_SHA:-0123456789abcdef0123456789abcdef01234567}" return 0 fi - [[ -n "${GH_STUB_STDERR:-}" ]] && echo "${GH_STUB_STDERR}" >&2 - [[ -n "${GH_STUB_OUTPUT:-}" ]] && printf '%s\n' "${GH_STUB_OUTPUT}" - return "${GH_STUB_EXIT:-0}" + gh_stub_respond } - GH_STUB_OUTPUT="" - GH_STUB_STDERR="" - GH_STUB_EXIT=0 + reset_gh_stub GH_STUB_HEAD_SHA="0123456789abcdef0123456789abcdef01234567" } @@ -118,6 +114,20 @@ assert_gh_stdin_contain() { assert_gh_stdin_contain '"line": 42' } +@test "pr_review_submit with suppress_errors returns no error text when the head SHA lookup fails" { + gh() { + printf '%s\n' "gh: Not Found (HTTP 404)" >&2 + return 1 + } + run tool_pr_review_submit '{ + "number": 100, + "comments": [{"path": "x.php", "line": 1, "body": "n"}], + "suppress_errors": true + }' + assert_failure + assert_output "" +} + @test "pr_review_submit with comments auto-fetches commit_id from PR head" { GH_STUB_HEAD_SHA="feedfacefeedfacefeedfacefeedfacefeedface" GH_STUB_OUTPUT='{"id": 99}' diff --git a/plugins/github-mcp/AGENTS.md b/plugins/github-mcp/AGENTS.md index 98d103a..fd49fdf 100644 --- a/plugins/github-mcp/AGENTS.md +++ b/plugins/github-mcp/AGENTS.md @@ -5,7 +5,7 @@ ``` plugins/github-mcp/ ├── README.md # User documentation (usage, configuration, troubleshooting) -├── REFERENCE.md # Full tool parameter docs and examples (31 read + 25 write tools) +├── REFERENCE.md # Full tool parameter docs and examples ├── AGENTS.md # LLM navigation guide (this file) ├── CHANGELOG.md # Version history ├── SETUP.md # Interactive setup procedure (kept byte-identical to plugin-setup's copy) @@ -42,8 +42,8 @@ plugins/github-mcp/ ├── server-write.sh # Write server entry point - gated by enable_write_server config ├── config-read.json # Read server metadata (name="gh-tooling") ├── config-write.json # Write server metadata (name="gh-tooling-write") - ├── tools-read.json # 31 read tools (PR, issue, CI, commit, search, repo, release, label, project, api_read) - ├── tools-write.json # 25 write tools (PR lifecycle, reviews, issues, issue types/fields, labels, assignees, sub-issues, projects, api) + ├── tools-read.json # Read tools (PR, issue, CI, commit, search, repo, release, label, project, api_read) + ├── tools-write.json # Write tools (PR lifecycle, reviews, issues, issue types/fields, labels, assignees, sub-issues, projects, api) ├── tools-empty.json # Tools list the write server reports while enable_write_server is false ├── mcp-gh-tooling.schema.json # JSON Schema for .mcp-gh-tooling.json └── lib/ @@ -73,8 +73,8 @@ plugins/github-mcp/ This plugin provides: - **Two MCP Servers** via `.mcp.json` in Claude Code and inline `mcpServers` in `.codex-plugin/plugin.json` in Codex: - - `gh-tooling` (read) - 31 read-only GitHub tools (PRs, issues, CI, commits, search, repo, releases, labels, projects, read-only API) - - `gh-tooling-write` (write) - 25 write tools (PR lifecycle, reviews, issues, issue types/fields, labels, assignees, sub-issues, projects, full API). Gated by `enable_write_server` config flag. + - `gh-tooling` (read) - read-only GitHub tools (PRs, issues, CI, commits, search, repo, releases, labels, projects, read-only API) + - `gh-tooling-write` (write) - write tools (PR lifecycle, reviews, issues, issue types/fields, labels, assignees, sub-issues, projects, full API). Gated by `enable_write_server` config flag. - **SessionStart Hook** via the shared `hooks/hooks.json`: - Assembles MCP tool directives dynamically from template with conditional write and label sections - Prompt template maintained in `hooks/prompts/mcp-tool-directives.md` @@ -127,11 +127,14 @@ Tools in `tools-read.json` and `tools-write.json` map to bash functions with `to - `_gh_resolve_repo()` falls back to `GH_DEFAULT_REPO` from config - All tools support `suppress_errors` and `fallback` shared 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 ### Standard execution block -Captures `__raw` and `__exit` separately; branches on `suppress_errors` for `2>/dev/null` vs `2>&1`; checks `fallback` before re-echoing error output. Always calls `_gh_post_process()` on success. +Captures `__raw` and `__exit` separately. It branches on `suppress_errors` for `2>/dev/null` vs `2>&1`. On failure it returns `fallback` when set, otherwise re-echoes the error output unless `suppress_errors` is set. Discarding stderr is not enough there: `gh api` prints an HTTP error's JSON body on stdout. Always calls `_gh_post_process()` on success. + +Where gh's stdout is a value the tool parses (an ID, a SHA, a JSON listing) rather than the tool's own output, capture it with `_gh_capture_split` so a gh warning on stderr never becomes part of the value. ## Key Navigation Points diff --git a/plugins/github-mcp/CHANGELOG.md b/plugins/github-mcp/CHANGELOG.md index fec3691..757707f 100644 --- a/plugins/github-mcp/CHANGELOG.md +++ b/plugins/github-mcp/CHANGELOG.md @@ -5,6 +5,39 @@ 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] + +### Fixed + +- An invalid `grep_pattern` now fails the tool call with `Error: Invalid grep_pattern` before `gh` + runs, so `run_logs` and `job_logs` no longer download a log and `search_code` no longer spends a + search request first. Before, grep's exit status was discarded and the call succeeded, returning + grep's error message as the tool's output. +- Checking a `jq_filter` no longer runs it. A filter such as `until(.done; .next)` hung the call, and + a valid filter that halts with exit status 3 and the text `compile error` was rejected. +- `issue_schema` called without `org`, `owner`, or a repository now reports why + `gh repo view` could not supply the organization (for example an authentication failure), + instead of only saying that `org` is required. `suppress_errors: true` leaves gh's message out. +- `sub_issue_add`, `sub_issue_remove`, `project_item_add`, and `project_status_set` now reject a + `repo` that is not in `owner/repo` form before any GitHub call. Before, `acme/app/extra` was read + as owner `acme` and repository `extra`, or built a wrong item URL. +- Under `suppress_errors: true`, a failed call now returns no error text on every tool. Tools built + on `gh api` returned the HTTP error's JSON body, which gh prints on stdout, as the result, and the + project and issue-schema write tools returned gh's message when looking up a project, a status, + or the organization's types or fields failed. +- `workflow_jobs` now fails the call, or returns `fallback`, when fetching any run's jobs fails or + gh's output cannot be read. Before, it left that run out and returned the rest as if complete. + It also reads every page of a run's jobs. Before, only the first page reached the result. +- gh's stderr no longer ends up in a value a tool parses from gh's stdout: the issue node IDs of + `sub_issue_add` and `sub_issue_remove`, the head SHA `pr_review_submit` fetches, the project and + status listings of the project tools, the organization's types and fields for `issue_type_set` and + `issue_field_set`, and the run jobs of `workflow_jobs`. A gh warning on a successful call + corrupted the value. +- A GitHub `url` naming only an owner, such as `https://github.com/acme`, is now rejected. Before, + it resolved to owner `acme` and repository `acme`. +- The project-not-found and status-not-found errors of `project_item_add` and `project_status_set` + now separate the listed names with `, `. Before, the separator alternated between `,` and a space. + ## [5.0.0] - 2026-10-02 ### Added diff --git a/plugins/github-mcp/REFERENCE.md b/plugins/github-mcp/REFERENCE.md index 60deaf1..cb29ca5 100644 --- a/plugins/github-mcp/REFERENCE.md +++ b/plugins/github-mcp/REFERENCE.md @@ -2,7 +2,7 @@ ## Read Server (gh-tooling) -31 tools available via the `gh-tooling` MCP server. Requires `gh` CLI installed and authenticated. +Tools available via the `gh-tooling` MCP server. Requires `gh` CLI installed and authenticated. ### Shared Tool Parameters @@ -29,6 +29,8 @@ Tools with large text output (`run_logs`, `job_logs`, `pr_diff`) additionally ac | `grep_ignore_case` | boolean | Case-insensitive matching (-i) | | `grep_invert` | boolean | Return non-matching lines (-v) | +A `grep_pattern` that `grep -E` rejects, such as an unbalanced `(`, fails the call with `Error: Invalid grep_pattern` before `gh` runs, the same as an invalid `jq_filter`. A pattern that matches nothing is not an error. The result is empty. + `max_lines` and `tail_lines` are also available on `pr_view`, `pr_checks`, `pr_comments`, `pr_reviews`, `issue_view`, `api_read`, `label_list`, `project_list`, and `project_view` for output size control. `job_logs`, `repo_file`, and `api_read` remove terminal escape sequences from the text they return, so `grep_pattern`, `max_lines`, and `tail_lines` work on the words rather than on the colour codes wrapped around them. `repo_file` with `download_to` is the exception: it writes the file's bytes unchanged. @@ -172,7 +174,8 @@ Use gh-tooling issue_list with search "TODO label:component/core" and limit 20 List an organization's issue types and issue fields, including each single-select field's options. The organization comes from `org`, `owner`, a repository parameter, the configured default repo, or the -current clone's remote. +current clone's remote. When none of them supplies one, the error says `org` is required and, when +reading the clone's remote failed, includes gh's message unless `suppress_errors` is set. Types and fields are independent. GitHub lets an organization pin fields to a type, but that pinning only drives the web UI: any organization field can be set on an issue of any type, so this tool @@ -243,7 +246,7 @@ Use gh-tooling run_logs with run_id 22245862281 and tail_lines 100 ### `workflow_jobs` -Aggregate jobs across workflow runs in a single call. Reduces N+1 tool calls (run_list + N x job_view) to one invocation. Fetches runs for a workflow, then retrieves jobs for each run. +Aggregate jobs across workflow runs in a single call. Reduces N+1 tool calls (run_list + N x job_view) to one invocation. Fetches runs for a workflow, then retrieves jobs for each run. If the jobs of any run cannot be fetched, the call fails, or returns `fallback` when set, rather than returning the jobs of the other runs. ``` Use gh-tooling workflow_jobs with workflow "CI" and repo "shopware/shopware" and job "PHPStan" and limit 3 @@ -549,7 +552,7 @@ Use gh-tooling api_read with endpoint "search/issues" and jq_filter ".items[] | ## Write Server (gh-tooling-write) -25 tools available via the `gh-tooling-write` MCP server. Requires `enable_write_server: true` in `.mcp-gh-tooling.json`. +Tools available via the `gh-tooling-write` MCP server. Requires `enable_write_server: true` in `.mcp-gh-tooling.json`. ### Shared Tool Parameters diff --git a/plugins/github-mcp/hooks/scripts/check-api-tools.sh b/plugins/github-mcp/hooks/scripts/check-api-tools.sh index 5e73cc4..cbd3a0e 100755 --- a/plugins/github-mcp/hooks/scripts/check-api-tools.sh +++ b/plugins/github-mcp/hooks/scripts/check-api-tools.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # MCP API Tool Enforcer # ========================================================= # Blocks MCP api/api_read tool calls when a dedicated tool exists. diff --git a/plugins/github-mcp/hooks/scripts/check-gh-tools.sh b/plugins/github-mcp/hooks/scripts/check-gh-tools.sh index 0fa0ab1..552e76a 100644 --- a/plugins/github-mcp/hooks/scripts/check-gh-tools.sh +++ b/plugins/github-mcp/hooks/scripts/check-gh-tools.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # Dev Tooling MCP Enforcer (GitHub CLI) # ========================================================= # Blocks common gh CLI bash commands in favor of gh-tooling MCP tools. diff --git a/plugins/github-mcp/hooks/scripts/lib/common.sh b/plugins/github-mcp/hooks/scripts/lib/common.sh index d3b7ed4..479cd19 100644 --- a/plugins/github-mcp/hooks/scripts/lib/common.sh +++ b/plugins/github-mcp/hooks/scripts/lib/common.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # Shared functions for MCP tool enforcement hooks # ================================================ # This library provides common functionality for the hooks that block bash @@ -7,9 +7,9 @@ # Usage: # source "${SCRIPT_DIR}/lib/common.sh" # parse_hook_input -# load_mcp_config "php-tooling" # or "js-tooling" +# load_mcp_config "gh-tooling" # # ... pattern matching ... -# block_tool "mcp__php-tooling__phpstan_analyze" "Description" +# block_tool "mcp__gh-tooling__pr_view" "Description" # Global variables set by this library: # HOOK_INPUT - Raw hook input read from stdin @@ -22,11 +22,17 @@ source "$(dirname "${BASH_SOURCE[0]}")/../../../shared/config-dirs.sh" +####################################### # Resolve the active host and project directory from hook input. # Claude Code provides CLAUDE_PROJECT_DIR, which selects claude even when # GITHUB_MCP_HOST is exported in the shell. Otherwise GITHUB_MCP_HOST=pi or # codex names the host, and codex is assumed. Codex and pi provide cwd in the # JSON payload. +# Globals: +# CLAUDE_PROJECT_DIR, GITHUB_MCP_HOST (read); HOOK_HOST, PROJECT_DIR (set) +# Arguments: +# $1 raw hook payload JSON. +####################################### resolve_hook_context() { local input="${1:-}" @@ -48,11 +54,15 @@ resolve_hook_context() { fi } +####################################### # Find a project config in the host config directories, in the order # github_mcp_config_dirs gives for the active host, then in the project root. # First match wins. -# Args: $1 = config prefix -# Sets: CONFIG_FILE (global) +# Globals: +# PROJECT_DIR, HOOK_HOST (read); CONFIG_FILE (set, empty when none found) +# Arguments: +# $1 config prefix, e.g. gh-tooling for .mcp-gh-tooling.json. +####################################### find_mcp_config() { local config_prefix="$1" CONFIG_FILE="" @@ -72,9 +82,13 @@ find_mcp_config() { fi } -# Parse hook input from stdin -# Sets: HOOK_INPUT, COMMAND, PROJECT_DIR, HOOK_HOST (globals) -# Exits 0 if command is empty +####################################### +# Read the hook payload from stdin and extract the bash command it carries. +# Globals: +# HOOK_INPUT, COMMAND, PROJECT_DIR, HOOK_HOST (set) +# Returns: +# Exits the hook with 0 when the payload carries no command. +####################################### parse_hook_input() { HOOK_INPUT=$(cat) resolve_hook_context "$HOOK_INPUT" @@ -84,10 +98,16 @@ parse_hook_input() { fi } -# Load MCP config from project directory -# Args: $1 = config prefix (e.g., "php-tooling", "js-tooling") -# Sets: CONFIG_FILE, ENVIRONMENT, ENFORCE_MCP_TOOLS (globals) -# Exits 0 if enforcement is disabled +####################################### +# Load the project config and read its enforcement settings. +# Globals: +# PROJECT_DIR, HOOK_HOST (read, through find_mcp_config); +# CONFIG_FILE, ENVIRONMENT, ENFORCE_MCP_TOOLS (set) +# Arguments: +# $1 config prefix, e.g. gh-tooling. +# Returns: +# Exits the hook with 0 when enforce_mcp_tools is false. +####################################### load_mcp_config() { local config_prefix="$1" ENVIRONMENT="" @@ -111,10 +131,19 @@ load_mcp_config() { fi } -# Block a tool with formatted message -# Args: $1 = full MCP tool name (e.g., "mcp__php-tooling__phpstan_analyze") -# $2 = description of what to use instead -# Outputs to stderr and exits with code 2 +####################################### +# Block the current command and name the MCP tool to use instead, spelled the +# way the active host exposes it. +# Globals: +# HOOK_HOST, COMMAND, ENVIRONMENT (read) +# Arguments: +# $1 MCP tool name in server-ID form, e.g. mcp__gh-tooling__pr_view. +# $2 description of what to use instead. +# Outputs: +# The block message on stderr. +# Returns: +# Exits the hook with 2, which the host reads as a block. +####################################### block_tool() { local tool="$1" local description="$2" diff --git a/plugins/github-mcp/mcp-server-gh/lib/api.sh b/plugins/github-mcp/mcp-server-gh/lib/api.sh index 1b28ae5..3cd32e8 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/api.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/api.sh @@ -86,7 +86,7 @@ tool_api() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi # Strip before post-processing so max_lines and tail_lines count lines of # text rather than lines padded with colour codes. diff --git a/plugins/github-mcp/mcp-server-gh/lib/commit.sh b/plugins/github-mcp/mcp-server-gh/lib/commit.sh index eba0e88..4982020 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/commit.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/commit.sh @@ -40,7 +40,7 @@ tool_commit_pulls() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi _gh_post_process "${__raw}" "${jq_filter}" "" "" "" "" "" } diff --git a/plugins/github-mcp/mcp-server-gh/lib/common.sh b/plugins/github-mcp/mcp-server-gh/lib/common.sh index 0ca5e36..2889ef9 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/common.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/common.sh @@ -4,12 +4,16 @@ source "$(dirname "${BASH_SOURCE[0]}")/../../shared/config-dirs.sh" +####################################### # Locate .mcp-gh-tooling.json. MCP_GH_TOOLING_CONFIG wins outright; otherwise # the last existing file of the list wins: project root, editor directories, # then the host directories of github_mcp_config_dirs in reverse, so the # active host's directory is checked last. -# Args: $1 = project root, $2 = log line when no config is found -# Sets: GH_TOOLING_CONFIG_FILE (global) +# Globals: +# MCP_GH_TOOLING_CONFIG, GITHUB_MCP_HOST (read); GH_TOOLING_CONFIG_FILE (set) +# Arguments: +# $1 project root, $2 log line when no config is found. +####################################### _load_gh_config() { local project_root="$1" local no_config_message="$2" @@ -60,9 +64,15 @@ _load_gh_config() { fi } -# Validate a GitHub number (PR, issue, run, job ID - positive integer) -# Args: $1 = value, $2 = field name for error message -# Outputs error message to stdout and returns 1 on failure +####################################### +# Validate a GitHub number (PR, issue, run, or job ID): digits only, non-empty. +# Arguments: +# $1 value, $2 field name for the error message (default: number). +# Outputs: +# An error message on stdout when the value is invalid. +# Returns: +# 0 when valid, 1 otherwise. +####################################### _gh_validate_number() { local value="$1" local field="${2:-number}" @@ -72,9 +82,16 @@ _gh_validate_number() { fi } -# Validate a GitHub repository in owner/repo format -# Args: $1 = repo string (empty is allowed - means use default) -# Outputs error message to stdout and returns 1 on invalid format +####################################### +# Validate a GitHub repository in owner/repo format. An empty value is valid +# and means the caller falls back to the default repository. +# Arguments: +# $1 repository string. +# Outputs: +# An error message on stdout when the format is invalid. +# Returns: +# 0 when valid or empty, 1 otherwise. +####################################### _gh_validate_repo() { local repo="$1" [[ -z "${repo}" ]] && return 0 @@ -84,8 +101,15 @@ _gh_validate_repo() { fi } -# Validate a git commit SHA (7-40 hex characters) -# Args: $1 = sha string +####################################### +# Validate a git commit SHA: 7 to 40 hex characters. +# Arguments: +# $1 SHA string. +# Outputs: +# An error message on stdout when the SHA is invalid. +# Returns: +# 0 when valid, 1 otherwise. +####################################### _gh_validate_sha() { local sha="$1" if [[ -z "${sha}" ]] || [[ ! "${sha}" =~ ^[0-9a-fA-F]{7,40}$ ]]; then @@ -94,18 +118,30 @@ _gh_validate_sha() { fi } -# Resolve the effective repository to use for an API call. -# Uses the provided repo arg first, then falls back to GH_DEFAULT_REPO. -# Args: $1 = repo from tool arguments (may be empty) -# Outputs: resolved repo string, or empty if none configured +####################################### +# Resolve the effective repository for an API call: the repo argument first, +# then GH_DEFAULT_REPO. +# Globals: +# GH_DEFAULT_REPO (read) +# Arguments: +# $1 repo from the tool arguments, may be empty. +# Outputs: +# The resolved repository on stdout, empty when neither source has one. +####################################### _gh_resolve_repo() { local repo_arg="${1:-}" echo "${repo_arg:-${GH_DEFAULT_REPO:-}}" } -# Assert that a repo is available (either passed or configured as default). -# Outputs error and returns 1 if no repo available. -# Args: $1 = effective repo string (from _gh_resolve_repo) +####################################### +# Require a repository, passed or configured as the default. +# Arguments: +# $1 effective repository, as _gh_resolve_repo returns it. +# Outputs: +# An error message on stdout when the repository is empty. +# Returns: +# 0 when a repository is set, 1 otherwise. +####################################### _gh_require_repo() { local effective_repo="$1" if [[ -z "${effective_repo}" ]]; then @@ -114,11 +150,18 @@ _gh_require_repo() { fi } -# Assert that a repo is available OR the working directory is inside a git repo. +####################################### +# Require a repository, or a working directory inside a git repository. # Tools using gh subcommands (gh pr view, gh issue list) that resolve from local # git context call this instead of _gh_require_repo to preserve the in-repo # "omit repo" workflow while still failing prescriptively in non-git contexts. -# Args: $1 = effective repo string (from _gh_resolve_repo or _gh_resolve_owner_repo) +# Arguments: +# $1 effective repository, from _gh_resolve_repo or _gh_resolve_owner_repo. +# Outputs: +# An error message on stdout when neither is available. +# Returns: +# 0 when a repository or a git working directory is available, 1 otherwise. +####################################### _gh_require_repo_or_git() { local effective_repo="$1" [[ -n "${effective_repo}" ]] && return 0 @@ -145,15 +188,49 @@ _gh_validate_org() { fi } +####################################### +# Run a command with its stdout and stderr captured apart, so a warning on a +# successful run never joins the value and a failed run can still say why. +# stderr goes through a file in MCP_CALL_TMPDIR, which the server removes +# when the call ends, even a cancelled one. +# Globals: +# MCP_CALL_TMPDIR (read) +# Arguments: +# $1 name of the caller's variable that receives stdout, +# $2 name of the caller's variable that receives stderr, +# $3... the command and its arguments. +# Returns: +# The command's exit status, or 1 with a message in the stderr variable when +# the temporary file cannot be created. +####################################### +_gh_capture_split() { + local __gh_cs_out_var="$1" __gh_cs_err_var="$2" + shift 2 + local __gh_cs_file __gh_cs_out __gh_cs_exit=0 + __gh_cs_file=$(mktemp "${MCP_CALL_TMPDIR:-${TMPDIR:-/tmp}}/gh-stderr.XXXXXX") || { + printf -v "${__gh_cs_out_var}" '%s' "" + printf -v "${__gh_cs_err_var}" '%s' "cannot create a temporary file for the command's stderr" + return 1 + } + __gh_cs_out=$("$@" 2>"${__gh_cs_file}") || __gh_cs_exit=$? + printf -v "${__gh_cs_out_var}" '%s' "${__gh_cs_out}" + printf -v "${__gh_cs_err_var}" '%s' "$(<"${__gh_cs_file}")" + rm -f -- "${__gh_cs_file}" + return "${__gh_cs_exit}" +} + ####################################### # Resolve the organization owning org-level resources (issue types, issue fields). # Priority: org > owner > repo-shaped args > GH_DEFAULT_REPO > git remote. # Globals: -# GH_DEFAULT_REPO, _GH_OWNER +# GH_DEFAULT_REPO (read); _GH_OWNER, _GH_REPO, _GH_REF, _GH_PATH (set by +# _gh_resolve_owner_repo_optional) # Arguments: # $1 JSON args string, $2 tool name for the error message. # Outputs: -# Organization login on stdout, or an error message on stdout. +# Organization login on stdout, or an error message on stdout. When the +# git-remote lookup fails, the message carries gh's reason unless the args +# set suppress_errors. # Returns: # 0 when an organization was resolved, 1 otherwise. ####################################### @@ -186,20 +263,36 @@ _gh_resolve_org() { return 0 fi - local name_with_owner - name_with_owner=$(gh repo view --json nameWithOwner -q .nameWithOwner 2>/dev/null) || true - if [[ -n "${name_with_owner}" ]]; then - _gh_validate_org "${name_with_owner%%/*}" "${tool}" || return 1 - printf '%s\n' "${name_with_owner%%/*}" + # A failed lookup must say why: an auth or network failure otherwise reads + # as "no org was given". + local repo_view repo_view_err repo_view_exit=0 + _gh_capture_split repo_view repo_view_err \ + gh repo view --json nameWithOwner -q .nameWithOwner || repo_view_exit=$? + if [[ ${repo_view_exit} -eq 0 && -n "${repo_view}" ]]; then + _gh_validate_org "${repo_view%%/*}" "${tool}" || return 1 + printf '%s\n' "${repo_view%%/*}" return 0 fi - printf '%s\n' "Error: org is required for ${tool}. Pass 'org', 'owner', or a repository ('repository', 'repo', or 'owner'+'repo'), or set 'repo' in .mcp-gh-tooling.json" + local message="Error: org is required for ${tool}. Pass 'org', 'owner', or a repository ('repository', 'repo', or 'owner'+'repo'), or set 'repo' in .mcp-gh-tooling.json" + local suppress_errors + suppress_errors=$(printf '%s\n' "${args}" | jq -r '.suppress_errors // false') + if [[ ${repo_view_exit} -ne 0 && -n "${repo_view_err}" && "${suppress_errors}" != "true" ]]; then + message+=". The current directory's repository could not be read: ${repo_view_err}" + fi + printf '%s\n' "${message}" return 1 } -# Read a value from the gh-tooling config file -# Args: $1 = jq path (e.g. '.repo'), $2 = default value +####################################### +# Read a value from the gh-tooling config file. +# Globals: +# GH_TOOLING_CONFIG_FILE (read) +# Arguments: +# $1 jq path, e.g. '.repo'; $2 value to use when the file or key is absent. +# Outputs: +# The value, or the default, on stdout. +####################################### _gh_config_value() { local path="$1" local default="${2:-}" @@ -209,22 +302,57 @@ _gh_config_value() { [[ -n "${value}" ]] && echo "${value}" || echo "${default}" } -# Validate jq filter syntax before execution. -# Only rejects definitive compile/parse/lexical errors; runtime errors on null are acceptable. -# Args: $1 = filter expression, $2 = field name for error message (default: jq_filter) -# Outputs error message to stdout and returns 1 on compile-time syntax failure. +####################################### +# Reject a jq filter that does not compile, before any gh call is made. +# The filter is compiled behind `empty |`, so it never runs: running it on +# null input would hang on a filter such as until(.done; .next), and a +# halt_error would read as a compile error. The newlines keep a trailing +# `# comment` in the filter from swallowing the closing parenthesis. +# Arguments: +# $1 filter expression, $2 field name for the error message (default: jq_filter). +# Outputs: +# An error message carrying jq's diagnostic on stdout when the filter does +# not compile. +# Returns: +# 0 when the filter is empty or compiles, 1 otherwise. +####################################### _gh_validate_jq_filter() { local filter="$1" local field="${2:-jq_filter}" [[ -z "${filter}" ]] && return 0 - local err - err=$(jq -n "${filter}" 2>&1 1>/dev/null) || true - if [[ -n "${err}" ]] && echo "${err}" | grep -qiE "compile error|unexpected \\\$end|parse error|lexical error"; then + local err jq_exit=0 + err=$(jq -n "empty | ( +${filter} +)" 2>&1 1>/dev/null) || jq_exit=$? + if [[ ${jq_exit} -ne 0 ]]; then echo "Error: Invalid ${field}: ${err}" return 1 fi } +####################################### +# Reject a grep_pattern that grep -E does not accept, before any gh call is +# made, so a bad pattern does not cost a log download or a search request. +# Arguments: +# $1 pattern. +# Outputs: +# An error message carrying grep's diagnostic on stdout when the pattern is +# rejected. +# Returns: +# 0 when the pattern is empty or valid, 1 otherwise. +####################################### +_gh_validate_grep_pattern() { + local pattern="$1" + [[ -z "${pattern}" ]] && return 0 + # Against empty input a valid pattern exits 1 (no match); 2 is an error. + local err grep_exit=0 + err=$(grep -E -- "${pattern}" &1) || grep_exit=$? + if [[ ${grep_exit} -gt 1 ]]; then + echo "Error: Invalid grep_pattern: ${err}" + return 1 + fi +} + ####################################### # Determine whether the installed gh accepts `gh api --allow-escape-sequences` # and record the answer in a global. @@ -292,11 +420,19 @@ _gh_strip_ansi() { -e $'s|\033||g' } +####################################### # Apply optional pipeline post-processing steps in order: jq → grep → head → tail. # Each step is a no-op when its controlling parameter is empty/zero. -# Args: $1=output $2=jq_filter $3=grep_pattern $4=grep_before $5=grep_after -# $6=grep_ignore_case $7=grep_invert $8=max_lines $9=tail_lines -# Outputs processed text to stdout; returns 1 if jq filter fails on the output. +# Arguments: +# $1 output, $2 jq_filter, $3 grep_pattern, $4 grep_before, $5 grep_after, +# $6 grep_ignore_case, $7 grep_invert, $8 max_lines, $9 tail_lines. +# Outputs: +# The processed text on stdout, or an error message on stdout when a step +# fails. +# Returns: +# 0 on success, including a grep that matches nothing; 1 when the jq filter +# fails on the output or grep rejects the pattern. +####################################### _gh_post_process() { local output="$1" local jq_filter="${2:-}" @@ -322,7 +458,15 @@ _gh_post_process() { [[ "${grep_before}" -gt 0 ]] && gcmd+=("-B" "${grep_before}") [[ "${grep_after}" -gt 0 ]] && gcmd+=("-A" "${grep_after}") gcmd+=("--" "${grep_pattern}") - output=$(echo "${output}" | "${gcmd[@]}") || true + # grep exits 1 for "no line matched", which is an empty result; 2 and + # above is an error such as an invalid pattern. + local grep_output grep_exit=0 + grep_output=$(echo "${output}" | "${gcmd[@]}") || grep_exit=$? + if [[ ${grep_exit} -gt 1 ]]; then + echo "Error: grep_pattern failed on output: ${grep_pattern}" + return 1 + fi + output="${grep_output}" fi if [[ -n "${max_lines}" && "${max_lines}" -gt 0 ]]; then @@ -336,11 +480,17 @@ _gh_post_process() { echo "${output}" } +####################################### # Parse a GitHub URL into owner, repo, ref, and path components. # Handles /tree/{ref}/{path} and /blob/{ref}/{path} URLs. -# Sets globals: _GH_URL_OWNER, _GH_URL_REPO, _GH_URL_REF, _GH_URL_PATH -# Returns 1 for non-GitHub URLs or unrecognized formats. # Limitation: refs with slashes (e.g. feature/branch) take only the first segment. +# Globals: +# _GH_URL_OWNER, _GH_URL_REPO, _GH_URL_REF, _GH_URL_PATH (set) +# Arguments: +# $1 URL. +# Returns: +# 0 when parsed, 1 for non-GitHub URLs or unrecognized formats. +####################################### _gh_parse_github_url() { local url="$1" _GH_URL_OWNER="" _GH_URL_REPO="" _GH_URL_REF="" _GH_URL_PATH="" @@ -353,6 +503,10 @@ _gh_parse_github_url() { # Strip scheme and host local path_part="${url#*github.com/}" + # Without a slash, the split below would read the one segment as both + # owner and repo. + [[ "${path_part}" == */* ]] || return 1 + # Extract owner/repo (first two segments) local owner repo remainder owner="${path_part%%/*}" @@ -391,9 +545,17 @@ _gh_parse_github_url() { return 0 } -# Validate a file path (reject traversal and leading slash). -# Empty path is valid (means repo root). -# Args: $1 = path string +####################################### +# Reject a repository path with a leading slash or a '..' anywhere in it, so +# it cannot leave the repository. The substring check also refuses names such +# as a..b.txt. An empty path is valid and means the root. +# Arguments: +# $1 path string. +# Outputs: +# An error message on stdout when the path is rejected. +# Returns: +# 0 when valid or empty, 1 otherwise. +####################################### _gh_validate_path() { local path="$1" [[ -z "${path}" ]] && return 0 @@ -493,8 +655,19 @@ _gh_partial_finish() { _GH_DL_TMP="" } -# Download a file from GitHub to a local path. -# Args: $1=owner, $2=repo, $3=remote_path, $4=local_path, $5=ref (optional) +####################################### +# Download a file from GitHub to a local path, byte for byte. +# Globals: +# _GH_ALLOW_ESCAPE_FLAG (set by _gh_probe_allow_escape_flag); _GH_DL_TMP and +# the shell's EXIT trap (set by _gh_partial_create, which leaves the trap +# installed after the download) +# Arguments: +# $1 owner, $2 repo, $3 remote path, $4 local path, $5 ref (optional). +# Outputs: +# An error message on stdout when the download or the write fails. +# Returns: +# 0 when the file is in place at the local path, 1 otherwise. +####################################### _gh_download_file() { local owner="$1" repo="$2" remote_path="$3" local_path="$4" ref="${5:-}" local -a cmd=("gh" "api" "repos/${owner}/${repo}/contents/${remote_path}") @@ -544,10 +717,18 @@ _gh_download_file() { _gh_partial_finish } +####################################### # Resolve owner/repo from multiple sources with priority: -# url > owner+repo > repository (owner/repo string) > GH_DEFAULT_REPO -# Sets globals: _GH_OWNER, _GH_REPO, _GH_REF, _GH_PATH -# Args: $1=JSON args string +# url > owner+repo > repository (owner/repo) > repo (owner/repo) > GH_DEFAULT_REPO +# Globals: +# GH_DEFAULT_REPO (read); _GH_OWNER, _GH_REPO, _GH_REF, _GH_PATH (set) +# Arguments: +# $1 JSON args string. +# Outputs: +# An error message on stdout when no source resolves or one is malformed. +# Returns: +# 0 when owner and repo are set, 1 otherwise. +####################################### _gh_resolve_owner_repo() { local args="$1" _GH_OWNER="" _GH_REPO="" _GH_REF="" _GH_PATH="" @@ -647,10 +828,19 @@ _gh_require_tools_list() { fi } +####################################### # Like _gh_resolve_owner_repo, but returns success with empty globals when no # repo source is provided. Use for tools that have a valid no-repo fallback # (e.g. gh's own git-context resolution for issue/PR subcommands inside a clone). -# Args: $1 = JSON args string +# Globals: +# GH_DEFAULT_REPO (read); _GH_OWNER, _GH_REPO, _GH_REF, _GH_PATH (set) +# Arguments: +# $1 JSON args string. +# Outputs: +# An error message on stdout when a provided source is malformed. +# Returns: +# 0 when resolved or when no source is provided, 1 otherwise. +####################################### _gh_resolve_owner_repo_optional() { local args="$1" _GH_OWNER="" _GH_REPO="" _GH_REF="" _GH_PATH="" diff --git a/plugins/github-mcp/mcp-server-gh/lib/issue.sh b/plugins/github-mcp/mcp-server-gh/lib/issue.sh index 5c04695..e9d03ff 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/issue.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/issue.sh @@ -74,7 +74,7 @@ tool_issue_view() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi if [[ "${with_field_values}" != "true" ]]; then @@ -128,7 +128,7 @@ _gh_issue_rest_issue() { __raw=$("${cmd[@]}" 2>&1) || __exit=$? fi if [[ ${__exit} -ne 0 ]]; then - echo "${__raw}" + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}" return ${__exit} fi echo "${__raw}" @@ -221,7 +221,7 @@ tool_issue_list() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi _gh_post_process "${__raw}" "${jq_filter}" "" 0 0 false false "" "" || return $? } diff --git a/plugins/github-mcp/mcp-server-gh/lib/issue_schema.sh b/plugins/github-mcp/mcp-server-gh/lib/issue_schema.sh index 5734501..a5fcc86 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/issue_schema.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/issue_schema.sh @@ -100,7 +100,7 @@ _gh_issue_schema_fetch() { __raw=$("${cmd[@]}" 2>&1) || __exit=$? fi if [[ ${__exit} -ne 0 ]]; then - printf '%s\n' "${__raw}" + [[ "${suppress_errors}" == "true" ]] || printf '%s\n' "${__raw}" return ${__exit} fi printf '%s\n' "${__raw}" diff --git a/plugins/github-mcp/mcp-server-gh/lib/issue_schema_write.sh b/plugins/github-mcp/mcp-server-gh/lib/issue_schema_write.sh index 8c4b64e..a058d8a 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/issue_schema_write.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/issue_schema_write.sh @@ -7,34 +7,39 @@ # Arguments: # $1 organization login, $2 endpoint segment ("issue-types" or "issue-fields"). # Outputs: -# The endpoint's JSON array on stdout, or gh's error text on stdout. +# The endpoint's JSON array on stdout; on failure gh's error text (its +# stderr, or the error body it printed when stderr is empty) on stdout. # Returns: # gh's exit status. ####################################### _gh_issue_org_collection() { local org="$1" endpoint="$2" - local __raw __exit=0 - __raw=$(gh api "orgs/${org}/${endpoint}" 2>&1) || __exit=$? - printf '%s\n' "${__raw}" - return ${__exit} + local out err gh_exit=0 + _gh_capture_split out err gh api "orgs/${org}/${endpoint}" || gh_exit=$? + if [[ ${gh_exit} -ne 0 ]]; then + printf '%s\n' "${err:-${out}}" + return ${gh_exit} + fi + printf '%s\n' "${out}" } ####################################### # Resolve an issue type name to the organization's canonical spelling. # Arguments: -# $1 organization login, $2 requested type name. +# $1 organization login, $2 requested type name, $3 suppress_errors flag. # Outputs: # The canonical type name on stdout, or an error listing the available types. +# When gh fails, its error, or nothing under suppress_errors. # Returns: # 0 when the name resolved, 1 otherwise. ####################################### _gh_resolve_issue_type() { - local org="$1" wanted="$2" + local org="$1" wanted="$2" suppress_errors="$3" local types_json types_json=$(_gh_issue_org_collection "${org}" "issue-types") || { - printf '%s\n' "Error: could not list issue types for '${org}': ${types_json}" + [[ "${suppress_errors}" == "true" ]] || printf '%s\n' "Error: could not list issue types for '${org}': ${types_json}" return 1 } @@ -58,18 +63,20 @@ _gh_resolve_issue_type() { # field's data type, so a bad name fails here naming the valid options rather # than reaching GitHub, which reports a wrong option name for an unknown field. # Arguments: -# $1 organization login, $2 values object keyed by field name. +# $1 organization login, $2 values object keyed by field name, +# $3 suppress_errors flag. # Outputs: -# The issue_field_values JSON array on stdout, or an error message. +# The issue_field_values JSON array on stdout, or an error message. When gh +# fails, its error, or nothing under suppress_errors. # Returns: # 0 when every entry resolved, 1 otherwise. ####################################### _gh_resolve_issue_field_values() { - local org="$1" values="$2" + local org="$1" values="$2" suppress_errors="$3" local fields_json fields_json=$(_gh_issue_org_collection "${org}" "issue-fields") || { - printf '%s\n' "Error: could not list issue fields for '${org}': ${fields_json}" + [[ "${suppress_errors}" == "true" ]] || printf '%s\n' "Error: could not list issue fields for '${org}': ${fields_json}" return 1 } @@ -185,8 +192,8 @@ tool_issue_type_set() { fi local canonical - canonical=$(_gh_resolve_issue_type "${effective_repo%%/*}" "${type}") || { - printf '%s\n' "${canonical}" + canonical=$(_gh_resolve_issue_type "${effective_repo%%/*}" "${type}" "${suppress_errors}") || { + [[ -z "${canonical}" ]] || printf '%s\n' "${canonical}" return 1 } body=$(jq -nc --arg type "${canonical}" '{type: $type}') @@ -241,8 +248,8 @@ tool_issue_field_set() { _gh_validate_repo "${effective_repo}" || return 1 local field_values - field_values=$(_gh_resolve_issue_field_values "${effective_repo%%/*}" "${values}") || { - printf '%s\n' "${field_values}" + field_values=$(_gh_resolve_issue_field_values "${effective_repo%%/*}" "${values}" "${suppress_errors}") || { + [[ -z "${field_values}" ]] || printf '%s\n' "${field_values}" return 1 } @@ -266,9 +273,10 @@ tool_issue_field_set() { # $1 HTTP method, $2 endpoint, $3 request body JSON, $4 jq filter for the # response, $5 tool name, $6 suppress_errors, $7 fallback. # Outputs: -# The filtered response on stdout, or gh's error text on stdout. +# The filtered response on stdout; on failure the fallback when one is set, +# otherwise gh's error text, or nothing under suppress_errors. # Returns: -# 0 on success, gh's exit status on failure. +# 0 on success or when the fallback answers, gh's exit status otherwise. ####################################### _gh_issue_schema_write() { local method="$1" endpoint="$2" body="$3" response_filter="$4" @@ -285,7 +293,9 @@ _gh_issue_schema_write() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { printf '%s\n' "${fallback}"; return 0; } - printf '%s\n' "${__raw}" + # gh api prints an HTTP error's JSON body on stdout, so discarding + # stderr alone would still return the error. + [[ "${suppress_errors}" == "true" ]] || printf '%s\n' "${__raw}" return ${__exit} fi diff --git a/plugins/github-mcp/mcp-server-gh/lib/issue_write.sh b/plugins/github-mcp/mcp-server-gh/lib/issue_write.sh index 0edd4e0..992d40a 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/issue_write.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/issue_write.sh @@ -61,7 +61,7 @@ tool_issue_create() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } @@ -126,7 +126,7 @@ tool_issue_edit() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } @@ -177,7 +177,7 @@ tool_issue_close() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } @@ -218,7 +218,7 @@ tool_issue_reopen() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } @@ -265,7 +265,7 @@ tool_issue_comment() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } diff --git a/plugins/github-mcp/mcp-server-gh/lib/job.sh b/plugins/github-mcp/mcp-server-gh/lib/job.sh index 6270e05..0549f81 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/job.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/job.sh @@ -38,7 +38,7 @@ tool_job_view() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } @@ -68,6 +68,7 @@ tool_job_logs() { return 1 fi _gh_validate_number "${job_id}" "job_id" || return 1 + _gh_validate_grep_pattern "${grep_pattern}" || return 1 local effective_repo effective_repo=$(_gh_resolve_repo "${repo}") @@ -89,7 +90,7 @@ tool_job_logs() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi # Strip before post-processing so grep_pattern, max_lines and tail_lines see # clean text rather than colour codes wrapped around the words they match. @@ -134,7 +135,7 @@ tool_job_annotations() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } diff --git a/plugins/github-mcp/mcp-server-gh/lib/label.sh b/plugins/github-mcp/mcp-server-gh/lib/label.sh index 11fe809..7c2cadc 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/label.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/label.sh @@ -40,13 +40,25 @@ tool_label_list() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi _gh_post_process "${__raw}" "${jq_filter}" "" 0 0 false false "${max_lines}" "" || return $? } -# Shared helper for add/remove list operations on PRs/issues. -# Args: $1=args_json, $2=param_name (labels/assignees), $3=gh_flag (--add-label/--remove-label/etc) +####################################### +# Add or remove list values (labels, assignees) on a PR or issue through +# gh edit. +# Arguments: +# $1 JSON args string; $2 list parameter name (labels or assignees); +# $3 gh edit flag per value, e.g. --add-label or --remove-assignee. +# Outputs: +# gh's output on stdout; on failure the fallback text when one is set, +# otherwise gh's error (its stdout only under suppress_errors) or a +# validation error. +# Returns: +# 0 on success or when the fallback answers; 1 on a validation error; +# gh's exit status when gh fails without a fallback. +####################################### _gh_edit_list_param() { local args="$1" param_name="$2" gh_flag="$3" @@ -101,7 +113,7 @@ _gh_edit_list_param() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { printf '%s\n' "${fallback}"; return 0; } - printf '%s\n' "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || printf '%s\n' "${__raw}"; return ${__exit} fi [[ -n "${__raw}" ]] && printf '%s\n' "${__raw}" return 0 diff --git a/plugins/github-mcp/mcp-server-gh/lib/pr.sh b/plugins/github-mcp/mcp-server-gh/lib/pr.sh index 1327ef5..4cbf7a4 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/pr.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/pr.sh @@ -53,7 +53,7 @@ tool_pr_view() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi _gh_post_process "${__raw}" "${jq_filter}" "" 0 0 false false "${max_lines}" "" || return $? } @@ -84,6 +84,7 @@ tool_pr_diff() { return 1 fi _gh_validate_number "${number}" "number" || return 1 + _gh_validate_grep_pattern "${grep_pattern}" || return 1 _gh_resolve_owner_repo_optional "${args}" || return 1 local effective_repo="" @@ -107,7 +108,7 @@ tool_pr_diff() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi # gh pr diff has no native file filter — extract the matching file's @@ -172,7 +173,7 @@ tool_pr_list() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi _gh_post_process "${__raw}" "${jq_filter}" "" 0 0 false false "" "" || return $? } @@ -214,7 +215,7 @@ tool_pr_checks() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi _gh_post_process "${__raw}" "" "" 0 0 false false "${max_lines}" "" || return $? } @@ -259,7 +260,7 @@ tool_pr_comments() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi _gh_post_process "${__raw}" "" "" 0 0 false false "${max_lines}" "" || return $? } @@ -300,7 +301,7 @@ tool_pr_reviews() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi _gh_post_process "${__raw}" "" "" 0 0 false false "${max_lines}" "" || return $? } @@ -339,7 +340,7 @@ tool_pr_files() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } @@ -378,7 +379,7 @@ tool_pr_commits() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } diff --git a/plugins/github-mcp/mcp-server-gh/lib/pr_write.sh b/plugins/github-mcp/mcp-server-gh/lib/pr_write.sh index 56726e7..2da1ced 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/pr_write.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/pr_write.sh @@ -71,7 +71,7 @@ tool_pr_create() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } @@ -138,7 +138,7 @@ tool_pr_edit() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } @@ -179,7 +179,7 @@ tool_pr_ready() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } @@ -242,7 +242,7 @@ tool_pr_merge() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } @@ -286,7 +286,7 @@ tool_pr_close() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } @@ -327,7 +327,7 @@ tool_pr_reopen() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } diff --git a/plugins/github-mcp/mcp-server-gh/lib/project.sh b/plugins/github-mcp/mcp-server-gh/lib/project.sh index d859917..ca422d5 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/project.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/project.sh @@ -48,7 +48,7 @@ tool_project_list() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi _gh_post_process "${__raw}" "${jq_filter}" "" 0 0 false false "${max_lines}" "" || return $? } @@ -91,30 +91,39 @@ tool_project_view() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi _gh_post_process "${__raw}" "${jq_filter}" "" 0 0 false false "${max_lines}" "" || return $? } -# Resolve project name to project number. -# Args: $1=project_name, $2=owner -# On success: prints the project number -# On failure: prints error listing available projects and returns 1 +####################################### +# Resolve a project title to its project number. +# Arguments: +# $1 project title, $2 owner login, $3 suppress_errors flag. +# Outputs: +# The project number on stdout, or an error on stdout that lists the +# owner's project titles. When gh fails, its error, or nothing under +# suppress_errors. +# Returns: +# 0 when the title matched, 1 otherwise. +####################################### _gh_resolve_project_number() { - local project_name="$1" owner="$2" + local project_name="$1" owner="$2" suppress_errors="$3" - local projects_json - projects_json=$(gh project list --owner "${owner}" --format json 2>&1) || { - printf '%s\n' "Error: could not list projects for '${owner}': ${projects_json}" + local projects_json list_err list_exit=0 + _gh_capture_split projects_json list_err \ + gh project list --owner "${owner}" --format json || list_exit=$? + if [[ ${list_exit} -ne 0 ]]; then + [[ "${suppress_errors}" == "true" ]] || printf '%s\n' "Error: could not list projects for '${owner}': ${list_err:-${projects_json}}" return 1 - } + fi local number number=$(printf '%s\n' "${projects_json}" | jq -r --arg name "${project_name}" '.projects[] | select(.title == $name) | .number' 2>/dev/null) if [[ -z "${number}" ]]; then local available - available=$(printf '%s\n' "${projects_json}" | jq -r '.projects[].title' 2>/dev/null | paste -sd ', ' -) + available=$(printf '%s\n' "${projects_json}" | jq -r '[.projects[].title] | join(", ")' 2>/dev/null) printf '%s\n' "Error: project '${project_name}' not found. Available projects: ${available:-}" return 1 fi @@ -122,18 +131,28 @@ _gh_resolve_project_number() { printf '%s\n' "${number}" } -# Resolve status field name to field ID and option ID. -# Args: $1=project_number, $2=owner, $3=status_name -# On success: prints "field_idoption_id" -# On failure: prints error listing available options and returns 1 +####################################### +# Resolve a Status option name to the Status field ID and the option ID. +# Arguments: +# $1 project number, $2 owner login, $3 status option name, +# $4 suppress_errors flag. +# Outputs: +# "field_idoption_id" on stdout with no trailing newline, or an error +# on stdout that lists the available options. When gh fails, its error, or +# nothing under suppress_errors. +# Returns: +# 0 when the option matched, 1 otherwise. +####################################### _gh_resolve_status_option() { - local project_number="$1" owner="$2" status_name="$3" + local project_number="$1" owner="$2" status_name="$3" suppress_errors="$4" - local fields_json - fields_json=$(gh project field-list "${project_number}" --owner "${owner}" --format json 2>&1) || { - printf '%s\n' "Error: could not list fields for project ${project_number}: ${fields_json}" + local fields_json list_err list_exit=0 + _gh_capture_split fields_json list_err \ + gh project field-list "${project_number}" --owner "${owner}" --format json || list_exit=$? + if [[ ${list_exit} -ne 0 ]]; then + [[ "${suppress_errors}" == "true" ]] || printf '%s\n' "Error: could not list fields for project ${project_number}: ${list_err:-${fields_json}}" return 1 - } + fi # Find the Status single-select field local field_id @@ -149,7 +168,7 @@ _gh_resolve_status_option() { if [[ -z "${option_id}" ]]; then local available - available=$(printf '%s\n' "${fields_json}" | jq -r '.fields[] | select(.name == "Status") | .options[].name' 2>/dev/null | paste -sd ', ' -) + available=$(printf '%s\n' "${fields_json}" | jq -r '[.fields[] | select(.name == "Status") | .options[].name] | join(", ")' 2>/dev/null) printf '%s\n' "Error: status '${status_name}' not found in project ${project_number}. Available options: ${available:-}" return 1 fi @@ -185,15 +204,17 @@ tool_project_item_add() { printf '%s\n' "Error: repo is required for project_item_add" return 1 fi + _gh_validate_repo "${effective_repo}" || return 1 local effective_owner effective_owner="${effective_repo%%/*}" # Resolve project name to number local project_number - project_number=$(_gh_resolve_project_number "${project}" "${effective_owner}" 2>&1) || { + project_number=$(_gh_resolve_project_number "${project}" "${effective_owner}" "${suppress_errors}") || { [[ -n "${fallback}" ]] && { printf '%s\n' "${fallback}"; return 0; } - printf '%s\n' "${project_number}"; return 1 + [[ -z "${project_number}" ]] || printf '%s\n' "${project_number}" + return 1 } # Build the item URL @@ -215,7 +236,7 @@ tool_project_item_add() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { printf '%s\n' "${fallback}"; return 0; } - printf '%s\n' "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || printf '%s\n' "${__raw}"; return ${__exit} fi printf '%s\n' "${__raw}" } @@ -250,22 +271,25 @@ tool_project_status_set() { printf '%s\n' "Error: repo is required for project_status_set" return 1 fi + _gh_validate_repo "${effective_repo}" || return 1 local effective_owner effective_owner="${effective_repo%%/*}" # Resolve project name to number local project_number - project_number=$(_gh_resolve_project_number "${project}" "${effective_owner}" 2>&1) || { + project_number=$(_gh_resolve_project_number "${project}" "${effective_owner}" "${suppress_errors}") || { [[ -n "${fallback}" ]] && { printf '%s\n' "${fallback}"; return 0; } - printf '%s\n' "${project_number}"; return 1 + [[ -z "${project_number}" ]] || printf '%s\n' "${project_number}" + return 1 } # Resolve status name to field_id + option_id local status_ids - status_ids=$(_gh_resolve_status_option "${project_number}" "${effective_owner}" "${status}" 2>&1) || { + status_ids=$(_gh_resolve_status_option "${project_number}" "${effective_owner}" "${status}" "${suppress_errors}") || { [[ -n "${fallback}" ]] && { printf '%s\n' "${fallback}"; return 0; } - printf '%s\n' "${status_ids}"; return 1 + [[ -z "${status_ids}" ]] || printf '%s\n' "${status_ids}" + return 1 } local field_id option_id IFS=$'\t' read -r field_id option_id <<< "${status_ids}" @@ -305,7 +329,7 @@ tool_project_status_set() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { printf '%s\n' "${fallback}"; return 0; } - printf '%s\n' "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || printf '%s\n' "${__raw}"; return ${__exit} fi printf '%s\n' "${__raw}" } diff --git a/plugins/github-mcp/mcp-server-gh/lib/release.sh b/plugins/github-mcp/mcp-server-gh/lib/release.sh index 922523f..8f1d845 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/release.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/release.sh @@ -115,7 +115,7 @@ tool_release_list() { if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } [[ "${suppress_errors}" == "true" ]] && continue - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi repo_result=$(echo "${__raw}" | jq -c \ diff --git a/plugins/github-mcp/mcp-server-gh/lib/repo.sh b/plugins/github-mcp/mcp-server-gh/lib/repo.sh index 431065c..404e9ef 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/repo.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/repo.sh @@ -36,7 +36,7 @@ tool_repo_tree() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi # Filter by path prefix if provided, then apply default or custom jq @@ -69,7 +69,7 @@ tool_repo_tree() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi local default_jq='[.[] | {name, type, size, path}]' @@ -114,6 +114,7 @@ tool_repo_file() { fi _gh_validate_path "${path}" || return 1 + _gh_validate_grep_pattern "${grep_pattern}" || return 1 local endpoint="repos/${owner}/${repo}/contents/${path}" [[ -n "${ref}" ]] && endpoint="${endpoint}?ref=${ref}" @@ -150,6 +151,7 @@ tool_repo_file() { rm -f -- "${__tmp_dl}" _gh_partial_finish [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } + [[ "${suppress_errors}" == "true" ]] && return ${__exit} [[ -n "${__dl_err}" ]] && { echo "${__dl_err}"; return ${__exit}; } echo "Error: failed to download ${owner}/${repo}/${path}" return ${__exit} @@ -174,7 +176,7 @@ tool_repo_file() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi # Text mode only — the download_to path above returns before here and must 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 befbcd9..33e0821 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/review_write.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/review_write.sh @@ -66,7 +66,7 @@ tool_pr_review_submit() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" return 0 @@ -88,11 +88,12 @@ tool_pr_review_submit() { # Auto-fetch head SHA if commit_id not provided. if [[ -z "${commit_id}" ]]; then - local fetch_raw fetch_exit=0 - fetch_raw=$(gh api "repos/${effective_repo}/pulls/${number}" --jq '.head.sha' 2>&1) || fetch_exit=$? + local fetch_raw fetch_err fetch_exit=0 + _gh_capture_split fetch_raw fetch_err \ + gh api "repos/${effective_repo}/pulls/${number}" --jq '.head.sha' || fetch_exit=$? if [[ ${fetch_exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "Error: failed to fetch commit_id for PR ${number}: ${fetch_raw}" + [[ "${suppress_errors}" == "true" ]] || echo "Error: failed to fetch commit_id for PR ${number}: ${fetch_err:-${fetch_raw}}" return 1 fi commit_id="${fetch_raw}" @@ -126,7 +127,7 @@ tool_pr_review_submit() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } @@ -173,7 +174,7 @@ tool_pr_comment() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } @@ -225,7 +226,7 @@ tool_pr_review_reply() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi echo "${__raw}" } diff --git a/plugins/github-mcp/mcp-server-gh/lib/run.sh b/plugins/github-mcp/mcp-server-gh/lib/run.sh index 7862263..b3d6469 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/run.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/run.sh @@ -47,7 +47,7 @@ tool_run_view() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi _gh_post_process "${__raw}" "${jq_filter}" "" 0 0 false false "" "" || return $? } @@ -108,7 +108,7 @@ tool_run_list() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi _gh_post_process "${__raw}" "${jq_filter}" "" 0 0 false false "" "" || return $? } @@ -141,6 +141,7 @@ tool_run_logs() { return 1 fi _gh_validate_number "${run_id}" "run_id" || return 1 + _gh_validate_grep_pattern "${grep_pattern}" || return 1 local effective_repo effective_repo=$(_gh_resolve_repo "${repo}") @@ -167,7 +168,7 @@ tool_run_logs() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi _gh_post_process "${__raw}" "" "${grep_pattern}" "${grep_context_before}" "${grep_context_after}" "${grep_ignore_case}" "${grep_invert}" "${max_lines}" "${tail_lines}" || return $? } @@ -212,74 +213,76 @@ tool_workflow_jobs() { [[ -n "${branch}" ]] && list_cmd+=("--branch" "${branch}") [[ -n "${event}" ]] && list_cmd+=("--event" "${event}") + # Every step fails the call rather than returning fewer jobs: a list missing + # a run's jobs reads as a complete one. log "INFO" "workflow_jobs: ${list_cmd[*]}" - local runs_json __exit=0 - if [[ "${suppress_errors}" == "true" ]]; then - runs_json=$("${list_cmd[@]}" 2>/dev/null) || __exit=$? - else - runs_json=$("${list_cmd[@]}" 2>&1) || __exit=$? - fi + local runs_json list_err __exit=0 + _gh_capture_split runs_json list_err "${list_cmd[@]}" || __exit=$? if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${runs_json}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${list_err:-${runs_json}}"; return ${__exit} fi - # Extract run IDs + # Step 2: Extract run IDs local run_ids - run_ids=$(echo "${runs_json}" | jq -r '.[].databaseId // empty' 2>/dev/null) + run_ids=$(printf '%s\n' "${runs_json}" | jq -r '.[].databaseId' 2>/dev/null) || { + echo "Error: workflow_jobs could not read the run list gh returned" + return 1 + } if [[ -z "${run_ids}" ]]; then - local result="[]" - _gh_post_process "${result}" "${jq_filter}" "" 0 0 false false "${max_lines}" "" || return $? + _gh_post_process "[]" "${jq_filter}" "" 0 0 false false "${max_lines}" "" || return $? return 0 fi - # Step 2: Fetch jobs for each run via API + # Step 3: Fetch jobs for each run via API local all_jobs="[]" local run_id while IFS= read -r run_id; do - [[ -z "${run_id}" ]] && continue log "INFO" "workflow_jobs: fetching jobs for run ${run_id}" - local api_out api_exit=0 - api_out=$(gh api "repos/${effective_repo}/actions/runs/${run_id}/jobs" --paginate 2>&1) || api_exit=$? + local api_out api_err api_exit=0 + _gh_capture_split api_out api_err \ + gh api "repos/${effective_repo}/actions/runs/${run_id}/jobs" --paginate || api_exit=$? if [[ ${api_exit} -ne 0 ]]; then - log "WARN" "workflow_jobs: failed to fetch jobs for run ${run_id}: ${api_out}" - continue + [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } + [[ "${suppress_errors}" == "true" ]] || echo "Error: could not fetch the jobs of run ${run_id}: ${api_err:-${api_out}}" + return ${api_exit} fi - # Extract jobs and attach run context via jq -s (slurp) to avoid --argjson quoting issues + # --paginate prints one JSON object per page, so every slurped page is + # read, not only the first. The run context goes in through + # --slurpfile to avoid --argjson quoting issues. local run_jobs run_jobs=$(jq -c -n --arg rid "${run_id}" \ - --slurpfile runs <(echo "${runs_json}") \ - --slurpfile api <(echo "${api_out}") \ - '($runs[0][] | select(.databaseId == ($rid | tonumber))) as $ctx | [$api[0].jobs[] | . + {run: $ctx}]' 2>/dev/null) || run_jobs="[]" - - all_jobs=$(printf '%s\n%s' "${all_jobs}" "${run_jobs}" | jq -s '.[0] + .[1]' 2>/dev/null) || true + --slurpfile runs <(printf '%s\n' "${runs_json}") \ + --slurpfile api <(printf '%s\n' "${api_out}") \ + '($runs[0][] | select(.databaseId == ($rid | tonumber))) as $ctx | [$api[] | .jobs[] | . + {run: $ctx}]' 2>/dev/null) || { + echo "Error: workflow_jobs could not read the jobs gh returned for run ${run_id}" + return 1 + } + + all_jobs=$(printf '%s\n%s' "${all_jobs}" "${run_jobs}" | jq -c -s '.[0] + .[1]') || { + echo "Error: workflow_jobs could not merge the jobs of run ${run_id}" + return 1 + } done <<< "${run_ids}" - # Step 3: Filter jobs by name (case-insensitive substring) - if [[ -n "${job}" ]]; then - all_jobs=$(echo "${all_jobs}" | jq -c --arg name "${job}" \ - '[.[] | select((.name | ascii_downcase) | contains($name | ascii_downcase))]' 2>/dev/null) || all_jobs="[]" - fi - - # Step 4: Filter by conclusion - if [[ -n "${conclusion}" ]]; then - all_jobs=$(echo "${all_jobs}" | jq -c --arg conc "${conclusion}" \ - '[.[] | select((.conclusion // "" | ascii_downcase) == ($conc | ascii_downcase))]' 2>/dev/null) || all_jobs="[]" - fi - - # Step 5: Filter and include steps (only when step filter is set) - if [[ -n "${step}" ]]; then - all_jobs=$(echo "${all_jobs}" | jq -c --arg sname "${step}" \ - '[.[] | .steps = [.steps[]? | select((.name | ascii_downcase) | contains($sname | ascii_downcase))]]' 2>/dev/null) || all_jobs="[]" - else - # Exclude steps by default to reduce output size - all_jobs=$(echo "${all_jobs}" | jq -c '[.[] | del(.steps)]' 2>/dev/null) || true - fi - - # Step 6: Slim down output — keep essential fields only - all_jobs=$(echo "${all_jobs}" | jq -c '[.[] | {id, name, status, conclusion, html_url, started_at, completed_at, run, steps}]' 2>/dev/null) || true + # Step 4: Filter by job name (case-insensitive substring), conclusion, and + # step name; drop steps unless a step filter asks for them; keep the + # essential fields. One program, so a failure cannot leave a partial list. + all_jobs=$(printf '%s\n' "${all_jobs}" | jq -c \ + --arg name "${job}" --arg conc "${conclusion}" --arg sname "${step}" ' + [ .[] + | select($name == "" or ((.name | ascii_downcase) | contains($name | ascii_downcase))) + | select($conc == "" or ((.conclusion // "" | ascii_downcase) == ($conc | ascii_downcase))) + | if $sname == "" then del(.steps) + else .steps = [.steps[]? | select((.name | ascii_downcase) | contains($sname | ascii_downcase))] + end + | {id, name, status, conclusion, html_url, started_at, completed_at, run, steps} + ]') || { + echo "Error: workflow_jobs could not filter the jobs" + return 1 + } _gh_post_process "${all_jobs}" "${jq_filter}" "" 0 0 false false "${max_lines}" "" || return $? } diff --git a/plugins/github-mcp/mcp-server-gh/lib/search.sh b/plugins/github-mcp/mcp-server-gh/lib/search.sh index 4bfec9e..227308f 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/search.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/search.sh @@ -60,7 +60,7 @@ tool_search() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi _gh_post_process "${__raw}" "${jq_filter}" "" 0 0 false false "" "" || return $? } @@ -107,6 +107,7 @@ tool_search_code() { fi _gh_validate_jq_filter "${jq_filter}" || return 1 + _gh_validate_grep_pattern "${grep_pattern}" || return 1 _gh_validate_number "${limit}" "limit" || return 1 local -a cmd=("gh" "search" "code" "${search}") @@ -144,7 +145,7 @@ tool_search_code() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi # download_to mode: save matching files locally @@ -231,7 +232,7 @@ tool_search_repos() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi _gh_post_process "${__raw}" "${jq_filter}" "" 0 0 false false "${max_lines}" "" || return $? } @@ -316,7 +317,7 @@ tool_search_commits() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi _gh_post_process "${__raw}" "${jq_filter}" "" 0 0 false false "" "" || return $? } @@ -408,7 +409,7 @@ GRAPHQL fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { echo "${fallback}"; return 0; } - echo "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || echo "${__raw}"; return ${__exit} fi # Extract just the nodes array for cleaner output diff --git a/plugins/github-mcp/mcp-server-gh/lib/sub_issue_write.sh b/plugins/github-mcp/mcp-server-gh/lib/sub_issue_write.sh index 62c7d24..7998973 100644 --- a/plugins/github-mcp/mcp-server-gh/lib/sub_issue_write.sh +++ b/plugins/github-mcp/mcp-server-gh/lib/sub_issue_write.sh @@ -2,9 +2,15 @@ # Sub-issue tools for gh-tooling MCP server (GraphQL) # Write: sub_issue_add, sub_issue_remove -# Helper: resolve an issue number to its GraphQL node ID. -# Args: $1=repo (owner/repo format), $2=issue_number -# Outputs: node ID string (e.g., "I_kwDOA...") +####################################### +# Resolve an issue number to its GraphQL node ID. +# Arguments: +# $1 repository in owner/repo form, $2 issue number. +# Outputs: +# The node ID on stdout, e.g. I_kwDOA...; gh's error on stderr. +# Returns: +# gh's exit status. +####################################### _gh_resolve_issue_node_id() { local repo="$1" issue_number="$2" local owner="${repo%%/*}" @@ -50,19 +56,25 @@ tool_sub_issue_add() { printf '%s\n' "Error: repo is required for sub_issue_add (no default repo configured)" return 1 fi - - # Resolve both issue numbers to node IDs - local parent_id sub_id - parent_id=$(_gh_resolve_issue_node_id "${effective_repo}" "${issue_number}" 2>&1) || { + _gh_validate_repo "${effective_repo}" || return 1 + + # Resolve both issue numbers to node IDs. stdout and stderr stay apart, so + # a gh warning on a successful lookup never becomes part of an ID. + local parent_id sub_id node_err node_exit=0 + _gh_capture_split parent_id node_err \ + _gh_resolve_issue_node_id "${effective_repo}" "${issue_number}" || node_exit=$? + if [[ ${node_exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { printf '%s\n' "${fallback}"; return 0; } - printf '%s\n' "Error: could not resolve issue #${issue_number}: ${parent_id}" + [[ "${suppress_errors}" == "true" ]] || printf '%s\n' "Error: could not resolve issue #${issue_number}: ${node_err}" return 1 - } - sub_id=$(_gh_resolve_issue_node_id "${effective_repo}" "${sub_issue_number}" 2>&1) || { + fi + _gh_capture_split sub_id node_err \ + _gh_resolve_issue_node_id "${effective_repo}" "${sub_issue_number}" || node_exit=$? + if [[ ${node_exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { printf '%s\n' "${fallback}"; return 0; } - printf '%s\n' "Error: could not resolve issue #${sub_issue_number}: ${sub_id}" + [[ "${suppress_errors}" == "true" ]] || printf '%s\n' "Error: could not resolve issue #${sub_issue_number}: ${node_err}" return 1 - } + fi log "INFO" "sub_issue_add: parent=${parent_id} sub=${sub_id}" @@ -81,7 +93,7 @@ tool_sub_issue_add() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { printf '%s\n' "${fallback}"; return 0; } - printf '%s\n' "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || printf '%s\n' "${__raw}"; return ${__exit} fi printf '%s\n' "${__raw}" } @@ -115,18 +127,23 @@ tool_sub_issue_remove() { printf '%s\n' "Error: repo is required for sub_issue_remove (no default repo configured)" return 1 fi + _gh_validate_repo "${effective_repo}" || return 1 - local parent_id sub_id - parent_id=$(_gh_resolve_issue_node_id "${effective_repo}" "${issue_number}" 2>&1) || { + local parent_id sub_id node_err node_exit=0 + _gh_capture_split parent_id node_err \ + _gh_resolve_issue_node_id "${effective_repo}" "${issue_number}" || node_exit=$? + if [[ ${node_exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { printf '%s\n' "${fallback}"; return 0; } - printf '%s\n' "Error: could not resolve issue #${issue_number}: ${parent_id}" + [[ "${suppress_errors}" == "true" ]] || printf '%s\n' "Error: could not resolve issue #${issue_number}: ${node_err}" return 1 - } - sub_id=$(_gh_resolve_issue_node_id "${effective_repo}" "${sub_issue_number}" 2>&1) || { + fi + _gh_capture_split sub_id node_err \ + _gh_resolve_issue_node_id "${effective_repo}" "${sub_issue_number}" || node_exit=$? + if [[ ${node_exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { printf '%s\n' "${fallback}"; return 0; } - printf '%s\n' "Error: could not resolve issue #${sub_issue_number}: ${sub_id}" + [[ "${suppress_errors}" == "true" ]] || printf '%s\n' "Error: could not resolve issue #${sub_issue_number}: ${node_err}" return 1 - } + fi log "INFO" "sub_issue_remove: parent=${parent_id} sub=${sub_id}" @@ -145,7 +162,7 @@ tool_sub_issue_remove() { fi if [[ ${__exit} -ne 0 ]]; then [[ -n "${fallback}" ]] && { printf '%s\n' "${fallback}"; return 0; } - printf '%s\n' "${__raw}"; return ${__exit} + [[ "${suppress_errors}" == "true" ]] || printf '%s\n' "${__raw}"; return ${__exit} fi printf '%s\n' "${__raw}" }