Skip to content

feat(repos): add typed commit tool outputs - #3389

Open
SamMorrowDrums wants to merge 7 commits into
sammorrowdrums-typed-discussion-notification-outputsfrom
sammorrowdrums-type-repository-tools
Open

SamMorrowDrums wants to merge 7 commits into
sammorrowdrums-typed-discussion-notification-outputsfrom
sammorrowdrums-type-repository-tools

Conversation

@SamMorrowDrums

@SamMorrowDrums SamMorrowDrums commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Migrates get_commit and list_commits to concrete typed MCP inputs and outputs, including full, field-projected, and empty commit lists. Modern clients receive schema-conformant structured output; legacy JSON text remains byte-exact.

Why

Continues the staged all-tools typed output migration for repository tools, building on discussion/notification PR #3388.
Fixes #

What changed

  • Added concrete commit input DTOs, MinimalCommit output for get_commit, and optional-field ListCommitOutput DTOs and selector for list_commits.
  • Restored original pagination defaults: omitted, explicit numeric zero, and string zero resolve to page=1 and perPage=30 for both tools, with exact REST query assertions for direct handlers and both protocol versions.
  • Preserved advertised input schemas, legacy text serialization, projections, sanitization, permissions, and IFC labels.
  • Added modern/legacy protocol and schema-conformance coverage for full lists, fields=[sha], nested fields=[commit], and empty full/projected lists, with byte-exact legacy text assertions.

MCP impact

  • No tool or API changes
  • Tool schema or behavior changed
    Modern clients receive output schemas and structured content for full, projected, and empty results. Legacy clients retain unchanged text bytes and input schemas; numeric/string zero pagination retains the original defaults.
  • New tool added

Prompts tested (tool changes only)

  • "Get commit details for abc123 in owner/repo" - equivalent mocked tool calls validated typed output and byte-exact legacy text.
  • "List commits in owner/repo and return only the SHA" - equivalent mocked tool calls validated projected structured output and legacy text.
  • "List commits and return the nested commit metadata" - equivalent mocked tool calls validated fields=[commit] and schema conformance.
  • "List commits using page zero and perPage zero" - equivalent mocked tool calls validated numeric/string-zero defaults and exact GitHub query parameters.

Security / limits

  • No security or limits impact
  • Auth / permissions considered
    Existing repository read scope and commit-content IFC labeling are retained.
  • Data exposure, filtering, or token/size limits considered
    Structured output honors the same field selection as legacy text, including nested commit fields; projected calls do not expose unselected fields.

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
    script/lint passed with 0 issues on final head 4b6686b5fc361a702e50d7e37a55870e49da6900.
  • Tested locally with ./script/test
    script/test passed all packages with race detection on the same final head.

Additional final-head validation:

  • go test ./pkg/github -run 'TestTypedRepositoryCommit|Test_GetCommit|Test_ListCommits' -count=1 - passed.
  • script/generate-docs - passed; generated documentation unchanged.
  • git diff --check - passed.

Docs

  • Not needed
    Tool names, descriptions, and advertised input schemas are unchanged; generated documentation and existing toolsnaps require no changes.
  • Updated (README / docs / examples)

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 Linux license report contains an inconsistent architecture-specific license path.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Migrates repository commit tools to typed MCP inputs and outputs while preserving legacy text responses and pagination defaults.

Changes:

  • Adds typed outputs and field projection for commit tools.
  • Adds protocol, schema, projection, and pagination tests.
  • Updates shared argument normalization and a generated license report.
File Description
pkg/​github/​repositories.go Implements typed commit inputs and outputs.
pkg/​github/​typed_read_normalizers.go Adjusts pagination normalization.
pkg/​github/​typed_repository_commit_outputs_test.go Tests typed and legacy commit behavior.
third-party-licenses.linux.md Splits Linux architecture license listings.

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

Comment thread third-party-licenses.linux.md Outdated
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-type-repository-tools branch from 6c5f60b to 02f6053 Compare October 5, 2026 09:22
@SamMorrowDrums
SamMorrowDrums marked this pull request as ready for review October 5, 2026 09:22
@SamMorrowDrums
SamMorrowDrums requested a review from a team as a code owner October 5, 2026 09:22
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-type-repository-tools branch from 02f6053 to d3fa25e Compare October 5, 2026 09:58
SamMorrowDrums and others added 7 commits October 5, 2026 13:17
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Normalize omitted and explicit zero pagination to page 1 and perPage 30 for get_commit and list_commits. Cover legacy and modern wire calls and direct handlers.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use an optional-field commit DTO for full and projected modern outputs while retaining exact legacy text. Cover SHA and nested commit projections, empty lists, and output schema conformance.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Auto-generated by license-check workflow
Capture canonical modern schemas, exercise null lists and error suppression with IFC labels, and drop unrelated generated Linux license drift noted by Copilot review.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep exact-main byte assertions for the legacy protocol and verify modern text represents the same typed DTO as structured content.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-type-repository-tools branch from d3fa25e to 733153c Compare October 5, 2026 11:21
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.

2 participants