Skip to content

refactor(pull-requests): migrate consolidated PR tools to typed inputs and outputs - #3396

Merged
SamMorrowDrums merged 21 commits into
mainfrom
sammorrowdrums-typed-pull-request-tools
Oct 6, 2026
Merged

SamMorrowDrums merged 21 commits into
mainfrom
sammorrowdrums-typed-pull-request-tools

Conversation

@SamMorrowDrums

Copy link
Copy Markdown
Collaborator

Summary

Moves the consolidated pull request tools in pkg/github/pullrequests.go to NewTool[In,Out] with concrete inputs and output DTOs. Stacked on #3395.

Why

Continues the typed-output stack. Modern-protocol clients get schema-valid structuredContent, while legacy and unknown protocols get the same output as before.
Fixes #

What changed

  • Typed tools: pull_request_read, create_pull_request, update_pull_request, merge_pull_request, update_pull_request_branch, pull_request_review_write (both the default and the thread-resolution-reason variants), add_comment_to_pending_review and add_reply_to_pull_request_comment.
  • New consolidated_pull_request_types.go with:
    • Concrete inputs.
    • An explicit oneOf union for the pull_request_read methods, with a diff variant and a null variant.
    • Write, merge, branch and reply output DTOs, including the awaiting-form result and the reply/reaction union.
    • Legacy argument normalizers.
  • The legacy handlers never ran schema validation, so the normalizers repeat each handler's checks in the same order with the same error text. They also keep the legacy coercions:
    • numeric strings and whole floats;
    • review_write keys matched case-insensitively, plus its WeakDecode behaviour;
    • the commentId BigInt and < 1 errors;
    • "unknown method" errors;
    • the create form deferral when title/head/base are missing.
  • Legacy text output is byte-for-byte unchanged. structuredContent is sent only on 2026-07-28 and omitted on legacy and unknown protocols.
  • Tests in typed_consolidated_pull_request_outputs_test.go cover:
    • call and list on modern, legacy and unknown protocols;
    • the feature variant;
    • the awaiting form;
    • legacy error and coercion cases, checked against a probe of the parent c918f4b;
    • API errors;
    • schema resolution and conformance of zero values.
  • Updated toolsnaps, plus new _typed snapshots.

MCP impact

  • No tool or API changes
  • Tool schema or behavior changed — the 8 tools now declare an output schema. Input schemas and legacy text are unchanged.
  • New tool added

Prompts tested (tool changes only)

  • Covered by in-process MCP client tests rather than live prompts. Examples: "Get the diff/files/reviews of PR Port CLI Server #1", "Merge PR Port CLI Server #1 with squash", "Reply to review comment 42 with a heart reaction".

Security / limits

  • No security or limits impact — the same API calls, permissions and content filtering as before.
  • Auth / permissions considered
  • Data exposure, filtering, or token/size limits considered

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 — passed (0 issues)
  • Tested locally with ./script/test — passed

Also run:

  • UPDATE_TOOLSNAPS=true go test ./... — passed
  • script/generate-docs — no doc changes
  • git diff --check — clean

Known CI: an inherited macOS flake from #3395 in typed_granular_issue_outputs_test.go:389 (the int64 overflow message differs on macOS).

Docs

  • Not needed — script/generate-docs produced no changes.
  • Updated (README / docs / examples)

@SamMorrowDrums
SamMorrowDrums added this pull request to stack #3385 October 2, 2026 14:33
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-pull-request-tools branch from 938c5c3 to fe126a7 Compare October 2, 2026 20:50
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-pull-request-tools branch from fe126a7 to 41e1877 Compare October 2, 2026 20:59
@SamMorrowDrums
SamMorrowDrums marked this pull request as ready for review October 5, 2026 10:26
@SamMorrowDrums
SamMorrowDrums requested a review from a team as a code owner October 5, 2026 10:26
Copilot AI balanced review requested due to automatic review settings October 5, 2026 10:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Compatibility normalizers introduce several observable regressions in legacy validation behavior and error ordering.

Review effort: Balanced
Findings: 4 Medium severity

Open (4)
What changed in this PR

Migrates consolidated pull-request tools to typed inputs and structured outputs while retaining legacy text responses.

Changes:

  • Adds typed DTOs, schemas, and compatibility normalizers.
  • Adds protocol, schema, coercion, and API-error tests.
  • Updates tool snapshots and generated license references.
File Description
pkg/​github/​pullrequests.go Registers typed pull-request tools and outputs.
pkg/​github/​consolidated_pull_request_types.go Defines DTOs, schemas, and normalizers.
pkg/​github/​typed_consolidated_pull_request_outputs_test.go Tests protocols, outputs, schemas, and errors.
pkg/​github/​__toolsnaps__/​pull_request_read.snap Updates read-tool snapshot.
pkg/​github/​__toolsnaps__/​pull_request_read_typed.snap Adds typed read-tool snapshot.
pkg/​github/​__toolsnaps__/​create_pull_request.snap Updates create-tool snapshot.
pkg/​github/​__toolsnaps__/​create_pull_request_typed.snap Adds typed create-tool snapshot.
pkg/​github/​__toolsnaps__/​update_pull_request.snap Updates update-tool snapshot.
pkg/​github/​__toolsnaps__/​update_pull_request_typed.snap Adds typed update-tool snapshot.
pkg/​github/​__toolsnaps__/​merge_pull_request.snap Updates merge-tool snapshot.
pkg/​github/​__toolsnaps__/​merge_pull_request_typed.snap Adds typed merge-tool snapshot.
pkg/​github/​__toolsnaps__/​update_pull_request_branch.snap Updates branch-tool snapshot.
pkg/​github/​__toolsnaps__/​update_pull_request_branch_typed.snap Adds typed branch-tool snapshot.
pkg/​github/​__toolsnaps__/​pull_request_review_write.snap Updates review-write snapshot.
pkg/​github/​__toolsnaps__/​pull_request_review_write_typed.snap Adds typed review-write snapshot.
pkg/​github/​__toolsnaps__/​pull_request_review_write_resolution_reason_typed.snap Adds feature-variant snapshot.
pkg/​github/​__toolsnaps__/​add_comment_to_pending_review.snap Updates pending-comment snapshot.
pkg/​github/​__toolsnaps__/​add_comment_to_pending_review_typed.snap Adds typed pending-comment snapshot.
pkg/​github/​__toolsnaps__/​add_reply_to_pull_request_comment.snap Updates reply-tool snapshot.
pkg/​github/​__toolsnaps__/​add_reply_to_pull_request_comment_typed.snap Adds typed reply-tool snapshot.
third-party-licenses.darwin.md Corrects dependency license reference.
third-party-licenses.linux.md Corrects dependency license reference.
third-party-licenses.windows.md Corrects dependency license reference.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pkg/github/consolidated_pull_request_types.go Outdated
Comment thread pkg/github/consolidated_pull_request_types.go
Comment thread pkg/github/consolidated_pull_request_types.go
Comment thread pkg/github/consolidated_pull_request_types.go Outdated
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-pull-request-tools branch from 4064e57 to 10c3679 Compare October 5, 2026 15:35
@SamMorrowDrums
SamMorrowDrums requested a balanced review from Copilot October 5, 2026 15:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The generated read schema incorrectly describes commit messages as operation-result messages.

Review effort: Balanced
Findings: 4 Medium severity · 1 Low severity

Open (5)

Comment thread pkg/github/consolidated_pull_request_types.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The broad protocol-sensitive migration and extensive compatibility normalization warrant final human verification.

Review effort: Balanced
Findings: 3 Medium severity

Open (3)
Resolved since last review (2)

@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-pull-request-tools branch from 3e0f06a to 1311814 Compare October 6, 2026 16:12
kerobbi
kerobbi previously approved these changes Oct 6, 2026
SamMorrowDrums and others added 9 commits October 7, 2026 00:11
Migrate the remaining 17 repository tools to concrete inputs and outputs. Preserve existing content, mutations, scopes, filtering, and protocol gating; add wire and schema conformance coverage.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Replace raw REST output schemas with compact repository DTOs, retain one canonical snapshot per tool, and align modern JSON text with structured output while pinning legacy content.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve raw helper APIs and exact legacy text while exposing concrete output unions for modern protocols. Retain explicit awaiting statuses and real null responses without exposing SDK-generated error fallbacks.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Auto-generated by license-check workflow
Project compact method-discriminated DTOs from API responses while preserving the separate legacy text formatter and explicit app-awaiting status. Cache strict schemas and keep only canonical snapshots. Strengthen null-member presence, schema size, and lockdown projection regressions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve forbidden variant properties as annotated schema objects rather than boolean schemas emitted by jsonschema-go. Cover Inspector's input paths and IFC labels on every issue-read method across protocols.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reject unknown methods, states, field types, opaque values and API URL properties while covering nullable and empty method data.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Pin unchanged legacy profile/avatar fields separately from compact modern user projections and assert identical lockdown filtering and sanitization across protocols.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
SamMorrowDrums and others added 12 commits October 7, 2026 00:11
Remove added custom-field union constraints and sentinel enums from the advertised issue-write input. Preserve runtime exactly-one validation and private strict-schema tests. All three scoped input schemas compare exactly equal to main.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Auto-generated by license-check workflow
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

# Conflicts:
#	pkg/github/__toolsnaps__/add_sub_issue.snap
#	pkg/github/__toolsnaps__/remove_sub_issue.snap
#	pkg/github/__toolsnaps__/reprioritize_sub_issue.snap
#	pkg/github/granular_issue_types.go
#	pkg/github/issues_granular.go
#	pkg/github/typed_granular_issue_outputs_test.go

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# interactive rebase in progress; onto 1973eee
# Last command done (1 command done):
#    pick 5b8f9d19 # refactor(issues): minimize granular structured outputs
# No commands remaining.
# You are currently rebasing.
#
# Changes to be committed:
#	modified:   pkg/github/__toolsnaps__/add_issue_comment_reaction.snap
#	modified:   pkg/github/__toolsnaps__/add_issue_reaction.snap
#	modified:   pkg/github/__toolsnaps__/add_sub_issue.snap
#	modified:   pkg/github/__toolsnaps__/create_issue.snap
#	modified:   pkg/github/__toolsnaps__/hide_issue_comment.snap
#	modified:   pkg/github/__toolsnaps__/remove_issue_comment_reaction.snap
#	modified:   pkg/github/__toolsnaps__/remove_issue_reaction.snap
#	modified:   pkg/github/__toolsnaps__/remove_sub_issue.snap
#	modified:   pkg/github/__toolsnaps__/reprioritize_sub_issue.snap
#	modified:   pkg/github/__toolsnaps__/set_issue_fields.snap
#	modified:   pkg/github/__toolsnaps__/unhide_issue_comment.snap
#	modified:   pkg/github/__toolsnaps__/update_issue_assignees.snap
#	modified:   pkg/github/__toolsnaps__/update_issue_body.snap
#	modified:   pkg/github/__toolsnaps__/update_issue_labels.snap
#	modified:   pkg/github/__toolsnaps__/update_issue_milestone.snap
#	modified:   pkg/github/__toolsnaps__/update_issue_state.snap
#	modified:   pkg/github/__toolsnaps__/update_issue_title.snap
#	modified:   pkg/github/__toolsnaps__/update_issue_type.snap
#	modified:   pkg/github/comment_minimize_test.go
#	new file:   pkg/github/granular_issue_types.go
#	modified:   pkg/github/granular_tools_test.go
#	modified:   pkg/github/issues_granular.go
#	new file:   pkg/github/typed_granular_issue_outputs_test.go
#
Preserve legacy text and search semantics while publishing protocol-gated concrete structured outputs. Cover modern, legacy, and unknown clients, full repository JSON unions, defaults, IFC labels, and errors.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Replace the full REST repository mirror with MinimalRepository-based
structured items plus purpose-built full-mode details. Legacy text remains
unchanged, and canonical snapshots now hold the modern output schema.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove the derivable node_id from compact full repository search output and
publish the visibility enum in the static output schema.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep the modern text assertion aligned with structured content after the shared output wrapper change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…s and outputs

Convert pull_request_read, create_pull_request, update_pull_request,
merge_pull_request, update_pull_request_branch, pull_request_review_write
(both feature variants), add_comment_to_pending_review and
add_reply_to_pull_request_comment to NewTool with concrete inputs and
output DTOs. Input normalizers replay legacy handler checks so legacy
coercions and error text are preserved.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use curated output DTOs for modern JSON text while preserving legacy serialization. Cover successful null responses, empty check runs, and Link-header page traversal.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Replay parameter checks in handler order and scope cursor validation to review-thread reads. Keep commit message schema descriptions distinct from operation messages.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove normalization and test assertions that only pinned which invalid argument wins. Keep per-field validation, accepted coercions, and response-shape behavior.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@SamMorrowDrums
SamMorrowDrums dismissed kerobbi’s stale review October 6, 2026 22:11

The merge-base changed after approval.

@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-pull-request-tools branch from 1311814 to c77a3e2 Compare October 6, 2026 22:11
Base automatically changed from sammorrowdrums-typed-search-tools to main October 6, 2026 22:14
@SamMorrowDrums
SamMorrowDrums merged commit 867131b into main Oct 6, 2026
21 checks passed
@SamMorrowDrums
SamMorrowDrums deleted the sammorrowdrums-typed-pull-request-tools branch October 6, 2026 22:14
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.

3 participants