Skip to content

refactor(pull-requests): type granular MCP tools - #3397

Draft
SamMorrowDrums wants to merge 1 commit into
sammorrowdrums-typed-pull-request-toolsfrom
sammorrowdrums-typed-granular-pr-tools
Draft

SamMorrowDrums wants to merge 1 commit into
sammorrowdrums-typed-pull-request-toolsfrom
sammorrowdrums-typed-granular-pr-tools

Conversation

@SamMorrowDrums

Copy link
Copy Markdown
Collaborator

Summary

Migrate all 13 granular pull request tools, including both resolve_review_thread feature variants, to concrete typed inputs and outputs. Modern 2026-07-28 clients receive output schemas and structuredContent; legacy and unknown protocols retain the exact original text without either.

Why

Stacked directly above #3396 to complete the granular PR surface without changing consolidated helpers or starting another layer. Parent verified before edits and after push: sammorrowdrums-typed-pull-request-tools at 938c5c31b523a48ada472ca1b5a98fc15d487d3d.
Fixes # — N/A; focused typed-output migration.

What changed

  • Concrete mutation DTOs, MinimalResponse/message outputs, standard input normalizers, and 14 modern schema snapshots. Numeric-string coercions, validation order/messages, ignored optional-string type errors, false draft state, reviewer splitting, review semantics and exported helper signatures are preserved.
  • Input schemas describe supported enum values instead of newly enforcing them, and do not introduce minimum constraints the untyped handlers did not enforce. GitHub retains validation of arbitrary strings and negative IDs, matching original wire acceptance.
  • A narrow inventory ToolInputError marker preserves exact user-facing diagnostics when checks move into normalizers; ordinary normalizer errors retain existing formatting.
  • Architecture-independent inherited overflow assertion: require rejection referencing comment_id, not Linux-specific "too large" text. Inspected macOS failures on refactor(pull-requests): migrate consolidated PR tools to typed inputs and outputs #3396 to verify its decoder-overflow diagnostic references the same field.

MCP impact

  • No tool or API changes
  • Tool schema or behavior changed
  • New tool added
    Concrete output schemas and structured content are exposed only on protocol 2026-07-28 and newer. Review mutations expose their actual success message; reference-returning mutations expose the same id/url payload as legacy text. Error results do not expose success-shaped structured content. Existing typed comment/review visibility tools are untouched.

Prompts tested (tool changes only)

  • Automated equivalents of "Change this PR's title/body/state; mark it draft/ready; request octocat and org/team as reviewers."
  • Automated equivalents of "Create a pending review or submit APPROVE/COMMENT/REQUEST_CHANGES; submit/delete the viewer's pending review; add a file or multiline review comment."
  • Automated equivalents of "Resolve/unresolve this thread, optionally with addressed/wont-fix/invalid; add/remove an inline-comment reaction."
  • All 14 definitions covered across modern, legacy and unknown protocol; successful, API-error, mutation-error, client-error, empty and missing-pending-review outcomes; exact legacy validation; schema resolution and result validation.
  • Temporary differential probes used original handlers generated from the exact parent commit across success/error/empty/no-pending outcomes and malformed inputs. go test ./pkg/github -run '^TestGranularPRDifferentialParentProbe$' -count=1 passed; generated fixtures were removed afterward. Permanent parent-behavior expectations remain.

Security / limits

  • No security or limits impact
  • Auth / permissions considered
  • Data exposure, filtering, or token/size limits considered
    Repo scope requirements, feature/GHES variants, destructive annotations and existing IFC/lockdown plumbing remain unchanged. No additional API fields are exposed; visibility tools and exported review helpers are unchanged. Full tests include inventory, IFC, scope and lockdown coverage.

Tool renaming

  • I am renaming tools as part of this PR (e.g. a part of a consolidation effort)
    • I have added the new tool aliases in deprecated_tool_aliases.go
  • I am not renaming tools as part of this PR

Note: if you're renaming tools, you must add the tool aliases. For more information on how to do so, please refer to the official docs.

Lint & tests

  • Linted locally with ./script/lint
  • Tested locally with ./script/test

Passed: UPDATE_TOOLSNAPS=true go test ./..., then in required order script/lint (0 issues), script/test (full race suite), script/generate-docs, git diff --check. Also passed go test ./pkg/github -run '^TestTypedGranularIssueWireErrors$' -count=1 and the exact-parent differential probe above. Initial lint findings were corrected before rerunning the full required sequence. No outstanding local failures. macOS was not available locally; CI must confirm the inherited assertion cleanup there. Live PAT-dependent e2e tests were not run.

Head: 4db5f54f982aa7c99c73321e99892f07f5f6445b; base head: 938c5c31b523a48ada472ca1b5a98fc15d487d3d. Commit includes the requested coauthor trailer; signing succeeded without fallback.

Docs

  • Not needed
  • Updated (README / docs / examples)
    Regenerated all documentation; only relevant granular PR parameter descriptions changed in docs/feature-flags.md. No unrelated generated drift included.

Preserve exact legacy validation and response text while advertising concrete protocol-gated outputs for all granular PR mutations and both thread-resolution variants.

Make the inherited overflow assertion architecture-independent.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@SamMorrowDrums
SamMorrowDrums added this pull request to stack #3385 October 2, 2026 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant