Visitar URL original
feat(comments): add typed visibility tool inputs and outputs by SamMorrowDrums · Pull Request #3384 · github/github-mcp-server · GitHub
Skip to content

feat(comments): add typed visibility tool inputs and outputs - #3384

Merged
SamMorrowDrums merged 5 commits into
sammorrowdrums-typed-context-tool-schemasfrom
sammorrowdrums-typed-comment-visibility-tools
Oct 6, 2026
Merged

feat(comments): add typed visibility tool inputs and outputs#3384
SamMorrowDrums merged 5 commits into
sammorrowdrums-typed-context-tool-schemasfrom
sammorrowdrums-typed-comment-visibility-tools

Conversation

@SamMorrowDrums

@SamMorrowDrums SamMorrowDrums commented Oct 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

Migrate the shared hide/unhide comment-visibility factory to typed inputs and results for all six issue-comment, pull-request review-comment, and review tools. Supported negotiated MCP protocol 2026-07-28+ receives output schemas and structured results with matching JSON text; unknown protocol versions are treated as legacy, preserving exact legacy text.

Why

Dependent layer above #3377, using the typed registration and legacy normalization foundation from #3371. No issue is closed by this layer; #3360 remains unchanged and open.

What changed

  • Add typed comment visibility inputs and a minimal concrete result while retaining exact target-dependent input metadata, identifiers, numeric minimums, permissions, and feature rules.
  • Normalize legacy numeric-string IDs and classifier casing through the foundation hook before SDK validation; preserve ignored arguments and rejection semantics.
  • Reuse the process-cached output schema for all six constructors through inventory.CachedSchemaFor.
  • Add initialized-session wire tests for hide/unhide targets, legacy byte fixtures, modern text/structured parity, output conformance, errors, and gating; update only six related tool snapshots.

MCP impact

  • No tool or API changes
  • Tool schema or behavior changed
  • New tool added
    Output schemas and typed structured content are added for supported modern clients. Unknown versions remain legacy; runtime support for unrecognized future versions is not expanded. Input schemas and legacy success text remain unchanged; errors never become structured success outputs.

Prompts tested (tool changes only)

  • Mocked wire-call equivalents of “Hide issue comment 1 as off-topic”, “Unhide inline pull request review comment 2”, and “Hide/unhide review 3 on pull request 42”; these are automated protocol tests, not live GitHub prompt runs.

Security / limits

  • No security or limits impact
  • Auth / permissions considered
  • Data exposure, filtering, or token/size limits considered
    Existing repo scope requirements, granular feature flags, read-only filtering, permission descriptions, and API calls are preserved. The concrete result contains only node ID and visibility fields; invalid inputs and failed mutations never yield successful structured output.

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
    All six tool names are unchanged.

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
    On local committed HEAD 0da1567de7d56f4e8e5ae51a5c473a31f34b2888, rebased linearly onto context e493a9b9b33d26d909afaee1f236cbb830da4251, ran in order: script/lint passed (0 issues); script/test passed (full go test -race ./...); UPDATE_TOOLSNAPS=true go test ./... passed (owned snapshots unchanged); script/generate-docs passed (no generated documentation diff). git diff --check passed and final worktree is clean. Discarded only unrelated generator-produced trailing-newline drift in merge_pull_request.snap. Live PAT-dependent e2e tests were not run. This local revision awaits separate push authorization; these are local results, not fresh CI claims.

Docs

  • Not needed
  • Updated (README / docs / examples)
    Generated documentation is unchanged; protocol clarification is recorded here and alongside protocol tests. Six tool snapshots total 246 lines on main and 372 lines in this layer: hide/unhide issue comment 47/33 → 67/53; hide/unhide review comment 47/33 → 67/53; hide/unhide review 53/39 → 73/59. Input metadata is retained; additions describe the output API only.

@SamMorrowDrums
SamMorrowDrums added this pull request to stack #3385 October 2, 2026 10:03
@SamMorrowDrums
SamMorrowDrums marked this pull request as ready for review October 2, 2026 10:05
@SamMorrowDrums
SamMorrowDrums requested a review from a team as a code owner October 2, 2026 10:05
Copilot AI balanced review requested due to automatic review settings October 2, 2026 10:05
Copilot AI reviewed Oct 2, 2026
View reviewed changes

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

🟢 Approval recommended

The typed migration preserves existing contracts and is comprehensively covered across supported protocol versions and failure paths.

Review effort: Balanced
Findings: None

What changed in this PR

Migrates six comment-visibility tools to typed inputs and outputs while preserving legacy behavior and protocol-gating structured results.

Changes:

  • Adds typed visibility inputs, output schemas, and normalized legacy arguments.
  • Preserves feature, scope, read-only, and error behavior.
  • Adds comprehensive protocol and behavior tests with updated snapshots.
File Description
pkg/​github/​comment_minimize.go Implements typed visibility tools and normalization.
pkg/​github/​comment_minimize_test.go Tests schemas, protocols, gates, and errors.
pkg/​github/​__toolsnaps__/​hide_issue_comment.snap Adds hide output schema.
pkg/​github/​__toolsnaps__/​unhide_issue_comment.snap Adds unhide output schema.
pkg/​github/​__toolsnaps__/​hide_pull_request_review_comment.snap Adds hide output schema.
pkg/​github/​__toolsnaps__/​unhide_pull_request_review_comment.snap Adds unhide output schema.
pkg/​github/​__toolsnaps__/​hide_pull_request_review.snap Adds hide output schema.
pkg/​github/​__toolsnaps__/​unhide_pull_request_review.snap Adds unhide output schema.

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

@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-comment-visibility-tools branch from 65bbe67 to 9c35bd3 Compare October 2, 2026 10:18
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-comment-visibility-tools branch from 9c35bd3 to a4e5b24 Compare October 2, 2026 10:24
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-comment-visibility-tools branch from a4e5b24 to d593079 Compare October 2, 2026 10:37
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-comment-visibility-tools branch from 0f312ad to 43f5bcb Compare October 2, 2026 10:59
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-comment-visibility-tools branch 2 times, most recently from 603c895 to 7e9c697 Compare October 2, 2026 20:50
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

⚠️ License files need updating

The license files are out of date. I tried to fix them automatically but don't have permission to push to this branch.

Please run:

script/licenses
git add third-party-licenses.*.md third-party/
git commit -m "chore: regenerate license files"
git push

Alternatively, enable "Allow edits by maintainers" in the PR settings so I can fix it automatically.

@SamMorrowDrums
SamMorrowDrums requested a balanced review from Copilot October 3, 2026 04:28
Copilot AI reviewed Oct 3, 2026
View reviewed changes

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

🟢 Approval recommended

The typed migration preserves the existing contracts and includes comprehensive protocol and validation coverage.

Review effort: Balanced
Findings: None

@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-comment-visibility-tools branch from a9fa559 to 28a1816 Compare October 3, 2026 07:25
@SamMorrowDrums
SamMorrowDrums requested a balanced review from Copilot October 3, 2026 07:28
Copilot AI reviewed Oct 3, 2026
View reviewed changes

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

Schema inference is repeated in request-scoped constructors, and the documented future-protocol behavior conflicts with exact-version gating.

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

Open (2)

Comment thread pkg/github/comment_minimize_test.go
Comment thread pkg/github/comment_minimize.go Outdated
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-comment-visibility-tools branch from 28a1816 to 0da1567 Compare October 3, 2026 12:33
@SamMorrowDrums
SamMorrowDrums requested a balanced review from Copilot October 3, 2026 12:34
Copilot AI reviewed Oct 3, 2026
View reviewed changes

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

🟢 Approval recommended

The typed migration preserves compatibility and is thoroughly covered across all six tools and protocol eras.

Review effort: Balanced
Findings: None

Resolved since last review (2)

@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-comment-visibility-tools branch from 0da1567 to 6ebfc8d Compare October 3, 2026 12:59
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-comment-visibility-tools branch from 6ebfc8d to 10ec2c4 Compare October 5, 2026 09:45
SamMorrowDrums and others added 5 commits October 6, 2026 18:12
@SamMorrowDrums
feat(comments): add typed visibility tool inputs and outputs
478eac7 
Preserve legacy argument normalization and text responses while exposing protocol-gated structured results for six hide/unhide tools.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions
chore: regenerate license files
07f67e2 
Auto-generated by license-check workflow
@github-actions
chore: regenerate license files
a42c12b 
Auto-generated by license-check workflow
@SamMorrowDrums
test(comments): pin protocol output parity and reason metadata
3c939e7 
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@SamMorrowDrums
fix(comments): reuse cached visibility output schema
d8102f0 
Clarify supported negotiated protocol versions without expanding runtime support.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-comment-visibility-tools branch from 10ec2c4 to d8102f0 Compare October 6, 2026 16:12
@SamMorrowDrums
SamMorrowDrums merged commit 1feeae4 into main Oct 6, 2026
21 checks passed
@SamMorrowDrums
SamMorrowDrums deleted the sammorrowdrums-typed-comment-visibility-tools branch October 6, 2026 22:11
@BrewTestBot BrewTestBot mentioned this pull request Oct 7, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@kerobbi kerobbi kerobbi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@SamMorrowDrums @kerobbi