Skip to content

feat(issues): add protocol-gated typed consolidated issue tools - #3393

Open
SamMorrowDrums wants to merge 9 commits into
sammorrowdrums-typed-repository-toolsfrom
sammorrowdrums-typed-consolidated-issue-tools
Open

SamMorrowDrums wants to merge 9 commits into
sammorrowdrums-typed-repository-toolsfrom
sammorrowdrums-typed-consolidated-issue-tools

Conversation

@SamMorrowDrums

@SamMorrowDrums SamMorrowDrums commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Add concrete Go input/output contracts to IssueRead, IssueWrite, and SubIssueWrite, exposing accurate output schemas and structured content for protocol 2026-07-28 and newer while preserving legacy text and exported raw-result helper signatures. Preserve explicit MCP App awaiting statuses for modern clients without exposing SDK-generated error fallbacks.

Why

Next file-focused typed MCP stack layer; no linked issue. Targets sammorrowdrums-typed-repository-tools; current PR base is c9e4171c59909d4425b14325ce9cee957cdc22b9.

What changed

  • Typed all five issue-read methods (get, get_comments, get_sub_issues, get_parent, get_labels), issue create/update including atomic parent creation, and sub-issue add/remove/reprioritize. Compact method-discriminated DTOs provide accurate modern schemas; legacy JSON text ordering is retained.
  • Added typed custom-field scalars, mutation optionality coverage, modern/legacy/unknown wire tests, strict schema conformance checks, enriched/lockdown output checks, and scoped tool snapshots.
  • Retained the shared inventory distinction between explicitly supplied structured statuses and generated output, with protocol-gating and validation coverage. Updated error-handling documentation.

MCP impact

  • No tool or API changes
  • Tool schema or behavior changed
  • New tool added

Modern clients receive concrete output schemas and matching structured content; older/unknown protocols retain the original text without output schemas or structured content. Existing exported helper APIs, scopes, feature gates, sanitization, IFC labeling, and lockdown filtering are preserved.

Intentional execution-error change, approved by the user: issue_read get for issue #3385 returning HTTP 404 changes from frozen main 71ef8266's JSON-RPC error code 0 to a successful JSON-RPC response containing CallToolResult{isError:true} with the same error message, in both legacy and modern protocols. This makes the execution failure visible to the model and follows the typed SDK's tool-error behavior; it is not a successful tool execution. Pi, Codex, and Inspector captures on integrated top 2c6d0826 independently confirmed this exact direction. Explicit awaiting statuses and ordinary error text remain intact; no generated structured success is exposed. Other ordinary handler execution errors converted in this same RPC-error-to-tool-error direction are also intentional; the reverse isError:true to JSON-RPC error direction is not accepted.

Prompts tested (tool changes only)

Mocked MCP wire equivalents:

  • "Get issue 1, its comments, sub-issues, parent, and labels."
  • "Create an issue under parent issue 7."
  • "Update an issue, clear its labels/assignees/type, and set its text, number, date, and single-select fields."
  • "Clear the last custom field on an issue."
  • "Reopen an issue, close it as completed/not planned, or close it as a duplicate."
  • "Add or remove a sub-issue, or reorder it before/after another sub-issue."
  • "Open the issue form, wait for submission, then execute the submitted create/update."

Separately, the stack harness exercised the real GitHub 404 for issue_read get on github/github-mcp-server#3385 with Pi, Codex, and Inspector; this was error-envelope evidence, not a full live e2e suite pass.

Security / limits

  • No security or limits impact
  • Auth / permissions considered
  • Data exposure, filtering, or token/size limits considered

Existing scope requirements and feature rules remain unchanged. Structured responses use the same sanitized, lockdown-filtered Go responses as the text path; unsafe hierarchy/closing references remain omitted. API/validation errors do not expose generated success-shaped output; explicit awaiting statuses retain their stop signal and reason.

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

Tool names and aliases 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

Recorded validation on local/published candidate 84358808a2e3d20e38d6f99b7349e890f5787375 (not a claim about later PR head 1973eeeaeba472089373bc7fc31fabd0803ac6b1):

  • UPDATE_TOOLSNAPS=true go test ./... — passed; snapshots regenerated, unrelated terminal-newline drift excluded.
  • script/lint — passed, 0 issues.
  • script/test — passed (go test -race ./...).
  • script/generate-docs — passed; generated documentation remained unchanged.
  • git diff --check — passed.

Live PAT-backed repository e2e suite was not run by this layer. Contract tests use in-memory MCP transports and mocked REST/GraphQL responses, including every scoped method, state variants, empty/null responses, errors, mutation optionality, custom fields, awaiting forms, and lockdown-filtered enrichment. The separately reported Pi/Codex/Inspector real-404 evidence supports the intentional error-envelope change above, not all-tool parity or a full suite pass. A temporary RPC-parity fixture and wrapper experiment were canceled per user decision and removed without changing the branch head.

Docs

  • Not needed
  • Updated (README / docs / examples)

Updated docs/error-handling.md with protocol-gated typed output, explicit awaiting statuses, and unchanged multi-round-trip semantics. Generated docs were refreshed and required no changes.

@SamMorrowDrums
SamMorrowDrums added this pull request to stack #3385 October 2, 2026 13:27
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-consolidated-issue-tools branch from c3f5817 to 99cd27e 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.

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

Explicit structured output can bypass schema validation, and null wire-presence coverage is incomplete.

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

Open (2)
What changed in this PR

Adds protocol-gated typed contracts and structured outputs for consolidated issue tools while preserving legacy responses.

Changes:

  • Adds typed issue read/write and sub-issue schemas, normalization, and outputs.
  • Preserves explicit awaiting-form statuses and expands protocol/schema tests.
  • Updates snapshots, error-handling documentation, and license metadata.
File Description
third-party-licenses.windows.md Updates Windows architecture license groups.
pkg/​inventory/​typed_output.go Preserves explicit structured error outputs.
pkg/​inventory/​typed_output_test.go Tests protocol gating and input requests.
pkg/​github/​typed_consolidated_issue_outputs_test.go Tests issue tool contracts and outputs.
pkg/​github/​typed_consolidated_issue_enrichment_test.go Tests enrichment and lockdown filtering.
pkg/​github/​issues.go Migrates consolidated issue tools to typed handlers.
pkg/​github/​consolidated_issue_types.go Defines typed contracts and schemas.
pkg/​github/​__toolsnaps__/​issue_write.snap Updates the issue-write schema snapshot.
pkg/​github/​__toolsnaps__/​issue_write_typed.snap Adds the modern typed snapshot.
docs/​error-handling.md Documents typed output and status behavior.

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

Comment thread pkg/inventory/typed_output.go Outdated
Comment on lines +137 to +140
if metadata.explicitOutput != nil {
// The SDK may replace a handler's explicit status with
// its serialized typed output, including an error zero.
resultCopy.StructuredContent = metadata.explicitOutput
Comment on lines +83 to +84
// A JSON null is a real success for get_sub_issues, not an error.
assert.JSONEq(t, text, mustMarshalJSON(t, result.StructuredContent))
@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
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-consolidated-issue-tools branch from a652b26 to 8435880 Compare October 5, 2026 11:59
SamMorrowDrums and others added 9 commits October 6, 2026 18:12
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>
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
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-typed-consolidated-issue-tools branch from 1973eee to 9dbc47b Compare October 6, 2026 16:12
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