Skip to content

Commit d699b62

Browse files
authored
feat(mcp): add custom comment minimize tool (#45)
* feat(mcp): add custom comment minimize tool * fix(mcp): harden custom review tool boundaries * fix(mcp): enforce local write tool gate * fix(mcp): normalize bot identity in minimize ownership check, make GraphQL testable The ownership gate compared GraphQL viewer.login to the comment author.login with strict equality. Empirically (verified against api.github.com with a real installation token): viewer for an App installation token is '<app-slug>[bot]' while Bot-authored comments carry the bare '<app-slug>' author login — the check could never pass for the App's own comments, i.e. the only comments the tool is allowed to minimize. The feature was unusable and untested because the GraphQL endpoint was a hard-coded const. - normalize_actor() strips the '[bot]' suffix on both sides of the comparison - execute_graphql/handle_minimize_comment take the GraphQL URL as a parameter (production callsite passes the api.github.com const) - handler-level tests against a mock GraphQL server: - App-bot-authored comment succeeds (viewer 'oab-ghpool[bot]' vs author 'oab-ghpool' — the exact live API shape), mutation payload asserted - human-authored comment refused, zero mutations - node in a different repository than the policy-checked owner/repo refused (403), zero mutations - GraphQL soft errors (HTTP 200 + errors[]) fail closed; unknown classifier rejected before any network call - drop the contradictory required+default classifier in the tool schema (classifier is required) Tests: ghpool 112 passed, clippy -D warnings clean. --------- Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
1 parent a61de55 commit d699b62

3 files changed

Lines changed: 638 additions & 7 deletions

File tree

README.md

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ path = "/var/lib/ghpool/mcp-audit.jsonl"
361361
[[mcp.agents]] # writes are only for authenticated agents
362362
id = "openab-bot"
363363
key = "aws:secretsmanager:ghpool/mcp-keys:openab"
364-
tools = ["issue_read", "create_issue", "add_issue_comment"]
364+
tools = ["issue_read", "create_issue", "add_issue_comment", "ghpool_review_minimize_comment"]
365365
repos = ["openabdev/ghpool", "openabdev/openab"]
366366
```
367367

@@ -378,10 +378,28 @@ Required GitHub App permissions (grant only what your agents' tools need):
378378
| Tools | App permission |
379379
|-------|----------------|
380380
| `issue_read`, `list_issues`, `create_issue`, `add_issue_comment` | Issues: read / write |
381+
| `ghpool_review_minimize_comment` | Issues: write and Pull requests: write |
381382
| `pull_request_read`, `create_pull_request`, `merge_pull_request` | Pull requests: read / write |
382383
| `get_file_contents`, `create_or_update_file`, `push_files` | Contents: read / write |
383384
| `list_workflows`, `run_workflow` | Actions: read / write |
384385

386+
387+
#### ghpool-owned review tools (issue #44 MVP)
388+
389+
When writes are enabled and an authenticated agent explicitly includes the tool in its allowlist, `tools/list` also advertises it. For example:
390+
391+
```toml
392+
[[mcp.agents]]
393+
id = "review-bot"
394+
key = "env:GHPOOL_REVIEW_KEY"
395+
tools = ["issue_read", "list_issues", "ghpool_review_minimize_comment"]
396+
repos = ["openabdev/ghpool"]
397+
```
398+
399+
The tool accepts `owner`, `repo`, `node_id`, and a `classifier` (`ABUSE`, `DUPLICATE`, `OFF_TOPIC`, `OUTDATED`, `RESOLVED`, or `SPAM`). It supports issue and pull-request comments authored by the current GitHub App bot identity; it does not minimize human-authored comments or unsupported node types. Before mutating, it verifies both the App-bot author and the exact repository owner/name against the policy arguments, then executes GitHub's `minimizeComment` GraphQL mutation locally through ghpool's scoped GitHub App credential. The operation is not forwarded to the upstream MCP server. The call uses the same repository policy, write gate, in-flight limit, and fail-closed audit as upstream write tools. Agents that do not explicitly allowlist the name, or that have writes disabled, neither see it in `tools/list` nor can call it.
400+
401+
This is intentionally a narrow MVP: ghpool does not expose arbitrary GraphQL, and upstream GitHub MCP tools remain unchanged. Any future ghpool-owned tool must preserve the same explicit allowlist, repository binding, write gate, and audit requirements.
402+
385403
The tool surface returned by `tools/list` shrinks to match the App's actual permissions (verified in the [#22 spike](https://github.com/openabdev/ghpool/issues/22)) — grant conservatively and expand as agents need more.
386404

387405
#### Multi-installation routing (one key, many orgs)

docs/DESIGN.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,11 @@ against GitHub directly.
2424
ghpool is a **credential-swapping reverse proxy with a default-deny policy
2525
engine**, sitting between agents and two GitHub surfaces:
2626

27-
- **MCP** (`/mcp`) → GitHub's hosted MCP server (`api.githubcopilot.com/mcp/`).
28-
ghpool proxies the official tool schemas verbatim — it defines no tools of
29-
its own (the Revision 1→2 pivot). Zero schema maintenance; new upstream
30-
tools appear automatically but are **denied by default** until granted.
27+
- **MCP** (`/mcp`) -> GitHub's hosted MCP server (`api.githubcopilot.com/mcp/`), with a
28+
narrowly scoped ghpool-owned review-tool exception. Upstream schemas remain
29+
proxied verbatim; `ghpool_*` tools are explicit, default-deny, App-backed,
30+
repository-bound, and fail-closed audited. ghpool does not expose arbitrary
31+
GraphQL or a general custom-tool registry.
3132
- **REST/GraphQL** (`/{path}`, `/graphql`) → `api.github.com`, with PAT
3233
pooling (budget-aware selection) and in-memory read caching. GraphQL
3334
mutations pass through with the client's own token (full attribution).
@@ -149,3 +150,4 @@ caller.
149150
| Buffer + parse write responses | #17 review | `isError` inside HTTP 200; audit must record tool outcome |
150151
| Scoped installation tokens per policy envelope | #17 review | GitHub enforces the repo boundary, not just our parser |
151152
| Writes: App + audit + agents required in code | #17 review | Hard rules, not documented hopes |
153+
| `ghpool_*` review tools as a narrow MCP exception | #44 / PR #45 | Fill upstream capability gaps without arbitrary GraphQL; preserve default-deny, repo binding, App credentials, and audit |

0 commit comments

Comments
 (0)