sammorrowdrums-explicit-protocol-annotations
134 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 kerobbi
|
b463b647ce | centralise lockdown checks and default fail closed on empty author | ||
|
Tommaso Moro
|
439ec711fa |
Add fields param to six more list/search tools (#2810)
* Add fields param to six more list/search tools Extend the optional `fields` response-filtering parameter (gated behind the `fields_param` feature flag, with adoption/savings telemetry) to six more read tools, following the dual-variant pattern already used by search_code and get_file_contents: - list_issues, list_pull_requests, list_commits, list_releases - search_issues, search_pull_requests For each tool, `X` is the flag-enabled variant that advertises `fields` and filters each result item to the requested subset, while `LegacyX` exposes the original schema and never filters, acting as a kill switch when the flag is off. Exactly one variant survives inventory filtering for any flag state via mutually exclusive FeatureFlagEnable / FeatureFlagDisable annotations. Wrapped responses (list_issues, search_issues, search_pull_requests) preserve their count / pagination envelope and only filter the item list; bare-array responses keep their array shape. Filtering reuses the shared filterEachField helper. A new fieldsSchemaProperty helper builds the `fields` schema (search_code and get_file_contents now use it too), and a shared recordFieldsUsageFor helper centralizes the telemetry full-size computation. Each field enum lists only the JSON fields the specific tool actually emits: list_commits omits stats/files (requested without per-file detail) and list_issues lists only the fields its GraphQL fragment populates. Adds per-tool field-filtering, telemetry, and Legacy definition tests, extends the mutual-exclusivity gating test to all eight gated tools, and regenerates the `_ff_fields_param` toolsnaps and feature-flag docs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Remove duplicate tool doc comments Address review: drop the leftover one-line doc comment above each dual-variant tool constructor (list_issues, list_pull_requests, list_commits, list_releases, search_issues, search_pull_requests); the detailed variant comment remains. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
Tommaso Moro
|
778f5bb6a3 |
chore(deps): bump go-github v87 → v89 and resolve breaking changes (#2840)
* chore(deps): bump go-github v87 -> v89 and resolve breaking changes Bumps google/go-github from v87 to v89 across the module and fixes the resulting breaking changes. No tool or behavior changes. - Rewrite all import paths go-github/v87 -> go-github/v89. - gists.go: Gists.Create now takes CreateGistRequest by value and Gists.Edit is renamed to Gists.Update taking UpdateGistRequest. - repositories_test.go: adapt to RepositoryRelease fields that became value types in v89 (ID, TagName, Draft). - Regenerate third-party license files for the new module path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Don't clear gist description on update when omitted update_gist always sent Description as a pointer to the OptionalParam zero value (""), so omitting description would overwrite an existing gist description with an empty string. Only set UpdateGistRequest.Description when the caller actually provided the argument; an explicit empty string still clears it. Adds a test asserting the description key is absent from the PATCH body when omitted and present when set. This addresses a pre-existing behavior surfaced while migrating to the v89 gist request types. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
Bryan Zwicker
|
b7bc3dccee |
Enrich issue_read get with hierarchy relationship signals (#2764)
* Enrich issue_read get with hierarchy relationship signals The default issue_read `get` payload surfaced no hierarchy data, forcing agents to drop to raw REST (parent_issue_url) or scan sibling sub_issues to discover relationships. Enrich `get` with a layered, zero-extra-round-trip relationship signal derived from a single combined GraphQL query: - has_parent / has_children: cheap, always-emitted routing booleans (addresses Sam Morrow's #2726 review note). - parent: compact ref (number/title/state/url/repository) mirroring the existing get_parent payload keys; omitted when there is no parent. - sub_issues_summary: native subIssuesSummary counts (total/completed/ percent_completed); omitted when there are no sub-issues. The single-issue field-values GraphQL call in GetIssue is replaced by one combined query (fetchIssueReadEnrichment) returning field values + parent + subIssuesSummary, so `get` adds no round-trips. Enrichment is best-effort: a query failure still returns the base issue and never fails `get`. Parent titles are sanitized (parent may be cross-repo) and redacted under lockdown mode unless the parent content can be verified as safe; numeric/structural fields and counts stay intact. get_parent / sub_issue_write behavior is unchanged; tool descriptions clarify hierarchy is read here but written via sub_issue_write (no writable parent field). Refs github/planning-tracking#3306 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Tighten hierarchy tool wording Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Omit unverified parent under lockdown instead of redacting title Align issue_read get parent enrichment with the codebase's existing lockdown patterns: rather than introducing a third, redaction-with-sentinel behavior, omit the whole parent reference when its (possibly cross-repo) content cannot be verified safe. This mirrors how unsafe comments, sub-issues, and PR reviews are filtered out. has_parent stays true so an agent can still route to get_parent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Restore explicit issue read query matcher Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Omit hierarchy flags when enrichment fails Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address hierarchy review nits Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Roberto Nacu <kerobbi@github.com> |
||
|
Bryan Zwicker
|
6592847357 |
Sanitize and lockdown-gate issue_read get_parent title (#2780)
* Sanitize and lockdown-gate issue_read get_parent title GetIssueParent returned the parent issue title raw and ungated by lockdown mode, so an agent could read an unverified, possibly cross-repo parent title even with lockdown enabled. Always sanitize the parent title and, under lockdown mode, only return the parent when its author has push access to the parent repository, failing closed otherwise. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Prove sanitization in get_parent test; trim comments Embed a U+202E BiDi control char in the mocked parent title so the happy-path assertion fails if Sanitize is removed, and tighten the GetIssueParent comments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Roberto Nacu <kerobbi@github.com> |
||
|
Tim Rogers
|
8cd03c0185 |
Add reaction tools for issues and pull requests (#2732)
Docker / build (push) Has been cancelled
CodeQL / Analyze (go) (push) Has been cancelled
CodeQL / Analyze (actions) (push) Has been cancelled
CodeQL / Analyze (javascript) (push) Has been cancelled
Build and Test Go Project / build (macos-latest) (push) Has been cancelled
Build and Test Go Project / build (ubuntu-latest) (push) Has been cancelled
Build and Test Go Project / build (windows-latest) (push) Has been cancelled
GoReleaser Release / release (push) Has been cancelled
MCP Server Diff / mcp-diff (push) Has been cancelled
MCP Server Diff / mcp-diff-http (push) Has been cancelled
Publish to MCP Registry / publish (push) Has been cancelled
* Add reaction tools for issues and pull requests Implement three granular-only tools for adding emoji reactions: - add_issue_reaction: Add reaction to an issue - add_issue_comment_reaction: Add reaction to an issue comment - add_pull_request_review_comment_reaction: Add reaction to a PR review comment All tools are feature-flagged with FeatureFlagEnable set to enable them only when clients request granular toolsets. Tools use go-github's Reactions service and return minimal ID response on success (HTTP 201). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Make reaction tools available in both granular and non-granular modes Remove feature flag gates from reaction tools so they're available to all clients regardless of granular toolset preference. Reaction tools are naturally atomic operations and work equally well in both modes. Updates test expectations to exclude reaction tools from granular-only test assertions, since they're now always available. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update toolsnaps with aligned reaction tool descriptions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Align reaction tool descriptions with codebase style Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Improve reaction tool responses and wording Return reaction URLs in minimal responses and clarify issue tools apply to pull requests where applicable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Expose reactions through default comment tools Keep the standalone reaction tools behind granular feature flags to avoid expanding the default tool count. Add optional reaction support to the existing issue comment and pull request comment reply tools, requiring at least one of body or reaction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clarify PR review comment IDs for reactions Document that PR review comment reaction inputs require the numeric review comment ID, not the GraphQL review thread node ID returned by review thread APIs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Support issue comment reactions in add_issue_comment Add optional comment_id support so the default add_issue_comment tool can react to a specific issue or pull request comment without exposing a separate default reaction tool. Keep body creation tied to issue_number and require reaction targets to provide either issue_number or comment_id. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Harden combined comment reaction tools Apply reactions before creating comments or replies so retrying a failed combined call cannot duplicate the non-idempotent comment operation. Also reject issue comment IDs without a reaction target to avoid silently ignoring the field. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Group reaction tools with granular registrations Keep standalone reaction tool registrations next to the granular issue and pull request tools they belong with, so the default compound tool sections stay focused. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Align granular reaction tool constructors Rename the standalone reaction tool constructors to match the granular tool naming convention and clarify issue comment reaction IDs for pull request comments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Require issue number for comment reactions Make issue_number required on the consolidated add_issue_comment tool even when reacting to a specific issue comment, keeping the default tool input shape explicit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address issue comment feedback Return structured GitHub API errors for issue comment creation failures and assert MCP error results in tests. Also reject comment_id with body on the consolidated comment tool to avoid ambiguous comment-plus-comment-reaction requests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Validate issue comment reaction target Use the required issue_number to verify issue comment reaction targets before creating the reaction, and remove overlapping add_issue_comment test coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Validate positive comment IDs Reject non-positive issue and pull request comment IDs in handlers and add the missing schema minimum for pull request review comment IDs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Sam Morrow <sammorrowdrums@github.com> Co-authored-by: Sam Morrow <info@sam-morrow.com> |
||
|
Owen Niblock
|
29634dad00 |
Fix delete:true on issue fields by calling deleteIssueFieldValue mutation (#2755)
* Fall back to REST DELETE when the PATCH can't carry an issue field clear
The dotcom REST update endpoint uses set semantics for issue_field_values:
sending {"issue_field_values": [...]} overwrites the whole list, and
anything not in the new list is treated as a deletion. UpdateIssue
already exploits this via merge-and-filter — delete:true on a field
removes it from the kept list and the server clears it as a side effect.
That breaks for one specific case: when the kept list ends up empty
(e.g. you're deleting the only remaining field value, or every field in
one call), go-github's omitempty tag on
`IssueRequest.IssueFieldValues` strips the empty slice from the JSON
body. The dotcom REST handler's top-level
`if data.include?(ISSUE_FIELD_VALUES)` guard then short-circuits — the
key isn't in the payload, so the whole block is skipped and the field
keeps its old value. The MCP tool returns success regardless, so a
coding agent would happily report the field as cleared.
Detect this case in UpdateIssue and fall back to the dedicated REST
DELETE endpoint per field: DELETE /repos/{owner}/{repo}/issues/{number}/
issue-field-values/{field_id}. The endpoint is idempotent, takes the
integer field ID (no GraphQL node ID needed), and is the same one the
public OpenAPI documents.
Empirical confirmation on github/github-mcp-server#2756:
- Set Priority=P1 via REST
- Before the fix: MCP issue_write delete:true returns success, PATCH body
on the wire is literally {}, Priority remains P1 (silent no-op).
- After the fix: MCP issue_write delete:true returns success, PATCH body
is still {}, but the follow-up DELETE clears the field. Priority is
cleared as expected.
Three new tests in issues_delete_test.go cover:
- The omitempty contract (so we know if go-github ever drops the tag)
- The 1-of-1 fallback path (PATCH body has no issue_field_values, DELETE
fires)
- The N-1 set-semantics path (kept list is non-empty, PATCH alone
handles it, no DELETE call)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address review: handle delete-absent-field and DELETE partial failures
Two issues surfaced in code review of the original delete fix:
1. Asking to delete a field that isn't set on the issue used to be a
silent no-op (PATCH body is {} after omitempty stripping, server
skips the issue_field_values block, tool returns success). The fix
turned it into a hard error because the fallback DELETE loop fires
unconditionally for every field ID in fieldIDsToDelete, and the
dotcom DELETE endpoint returns 404 when the field has no value to
delete. This breaks idempotent 'ensure field X is cleared' callers
that may re-run the same delete on a field that's already been
cleared.
Fix: before queueing the fallback DELETEs, filter
fallbackDeleteFieldIDs down to IDs that actually appeared in the
existing field values. This preserves the pre-fix silent-no-op
behaviour and avoids a guaranteed 404.
2. The DELETE loop returned on the first error. If a caller asks to
clear three fields and the second fails (transient 5xx, rate limit,
etc.), the first is gone, the third is never attempted, and the
user gets a generic error with no indication of which field failed
or which had already succeeded. This is unrecoverable from the
caller side.
Fix: continue on per-field errors, accumulate the failed and
succeeded IDs, and return a single aggregated error naming both
sets so callers can retry the right fields.
Tests:
- Test_UpdateIssue_DeleteAbsentFieldIsNoOp: existing field values is
empty, fieldIDsToDelete=[101]. Asserts no DELETE call fires (the
mock returns 404 if it does, which would fail the test) and the
tool returns success.
- Test_UpdateIssue_DeleteFallbackContinuesOnPartialFailure: three
fields exist, all three are deleted. Mock returns 500 for the
middle DELETE, 204 for the others. Asserts all three DELETEs
fired (the middle failure didn't short-circuit the third) and the
error result names failed=[202] and cleared=[101 303].
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Slim down code comments in delete-fix region
Trim the doc comments on UpdateIssue's omitempty-trap region + test
file to the bare minimum needed to understand the non-obvious behaviour
(why the DELETE fallback exists, why we filter to existing IDs, why
errors are aggregated). Drop restated context and step-by-step
explanations.
No behaviour change.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
||
|
Sam Morrow
|
0503f2f2c5 |
fix(mcp-apps): reconcile the show/defer contract — render results, remove show_ui (#2774)
* fix(ui): render success view when an MCP App tool executed up-front The create_pull_request / issue_write / update_pull_request Views decided form-vs-success from in-app submit state only, ignoring the tool-result the host delivers on render. Per the MCP Apps 2026-01-26 spec the host renders a View whenever the tool carries _meta.ui.resourceUri — independent of whether the server deferred or executed. So when the server executed up-front (e.g. show_ui=false, or parameters the form can't represent) the View still showed its "Create pull request" input form over an already-created PR, which reads as a bug (it even shows a PR number). Drive the Views off the result instead: a new shared completedToolResult() helper returns parsed data only for a genuine completed success, and returns null for the awaiting_user_submission deferral sentinel, errors, or no result. Each write View now shows its success card when that completed result is present, so the form is only ever shown while the action is genuinely deferred. Reconciles the show/defer state machine at the View (decision layer that the host result feeds). See github/copilot-mcp-core#1864. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(ui): scope tool-result to the current invocation Address review feedback: the write Views derive their success card from `toolResult`, but it wasn't cleared when a new invocation arrived (only the in-app `successPR`/`successIssue` was reset on `toolInput` change). A completed result from a previous invocation could briefly render a stale success card over the next, still-deferred form. Clear `toolResult` whenever a new `tool-input` notification arrives. The spec guarantees `tool-input` precedes that invocation's `tool-result`, so this scopes the result to the current invocation centrally in the hook — fixing all three Views without per-app invocation keys. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor(mcp-apps): remove show_ui — it can't suppress app rendering show_ui promised "skip the form and execute directly", but it can't deliver: the host renders an MCP App for any tool that carries _meta.ui.resourceUri, and the 2026-01-26 MCP Apps spec has no per-call/per-result way to opt out of rendering. show_ui only flipped the server's defer decision, so show_ui=false created the PR/issue up-front yet the host still rendered the app — exactly the contradiction this work set out to fix. And show_ui is only ever exposed to clients that support UI, i.e. precisely the clients that always render the app. Remove it entirely: - Drop the show_ui schema property, the form-param allowlist entry, and the showUI term from the defer predicate in create_pull_request and issue_write. The gate is now FF && clientSupportsUI && !_ui_submitted && !hasNonFormParams. - Delete the now-unused UI-only schema-property strip machinery in pkg/inventory (uiOnlySchemaProperties, stripUIOnlySchemaProperties, stripSchemaProperties) and the exported ConditionalSchemaPropertyDescriptions, which existed solely to surface show_ui to UI-capable clients. _meta.ui stripping is untouched. - Drop the conditional-property annotation from the docs generator. - Update toolsnaps, generated docs, and tests. With the up-front-execution Views now rendering the result (success card), the remaining contract is simple: when MCP Apps are enabled the form is the path, and the form is only shown while the action is genuinely deferred. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor(mcp-apps): centralize the show/defer decision (single source of truth) The defer-to-form predicate was triplicated across create_pull_request, update_pull_request, and issue_write, each with its own near-identical *HasNonFormParams function. As more MCP App tools are added this duplication would grow and the copies could silently drift. Extract one shared gate in ui_capability.go: - shouldDeferToForm(ctx, deps, req, args, formParams) — the single show/defer decision (MCP Apps enabled, client supports UI, not a form submission, and no non-form params). - hasNonFormParams(args, formParams) — one generic helper replacing the three per-tool functions. - uiSubmitted(args) — small shared predicate. Each handler is now a one-line `if shouldDeferToForm(...) { return awaiting }`. The per-tool form-parameter allowlists and the user-facing messages stay per-tool (that is the genuine per-tool config). Pure refactor — behavior unchanged; existing tests now exercise the generic helper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Matt Holloway
|
3c4749d234 |
Show issue_write MCP App form for labels/assignees/milestone/type (#2767)
The issue_write form-gating logic uses issueWriteFormParams to decide whether to render the MCP App form or execute directly: a call carrying any parameter outside that allowlist bypasses the form. The form prefills and re-submits labels, assignees, milestone and type, but those four were absent from the allowlist, so passing any of them skipped the confirmation form even though the form fully supports them. Add labels, assignees, milestone and type to issueWriteFormParams so the form is shown when they are present. The allowlist now covers every input-schema property, leaving issueWriteHasNonFormParams as a forward-compatibility safety net for properties added without form support. Also correct the now-inaccurate show_ui guidance on issue_write and create_pull_request: both descriptions claimed the form "does not collect" fields it actually collects (labels/assignees/milestone/type/issue_fields, and reviewers respectively). Update unit tests and regenerate toolsnaps and docs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Bryan Zwicker
|
9430064a2b |
Add get_parent method to issue_read (#2726)
Add an upward parent read to issue_read, the counterpart to the existing downward get_sub_issues. Uses the GraphQL Issue.parent field and returns a null parent when the issue is not a sub-issue. Kept always-on (not feature gated) to mirror get_sub_issues; the dependency tools remain flag-gated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Sam Morrow <info@sam-morrow.com> |
||
|
Renee Xu
|
a37837027f |
Promote issue fields and deprecate legacy issue write tool (#2696)
* Promote issue fields FF and remove legacy issue write methods * Remove legacy issue_write methods * Remove feature flag * Update tests * Update tool snaps * Update docs * Remove legacy issue write from issues test * Update tool snap * Fix docs * Include issue_fields in doc * Remove issue fields FF check in ui tools * Update UI tools test * update docs * Regenerate doc |
||
|
Kelsey Myers
|
4f73cfd1db |
Add repo-scoped support to list_issue_types tool (#2692)
CodeQL / Analyze (go) (push) Has been cancelled
CodeQL / Analyze (actions) (push) Has been cancelled
CodeQL / Analyze (javascript) (push) Has been cancelled
Docker / build (push) Has been cancelled
Build and Test Go Project / build (macos-latest) (push) Has been cancelled
Build and Test Go Project / build (ubuntu-latest) (push) Has been cancelled
Build and Test Go Project / build (windows-latest) (push) Has been cancelled
GoReleaser Release / release (push) Has been cancelled
MCP Server Diff / mcp-diff (push) Has been cancelled
MCP Server Diff / mcp-diff-http (push) Has been cancelled
Publish to MCP Registry / publish (push) Has been cancelled
* Add repo-scoped support to list_issue_types tool * Render multi-scope tools as "any of" in generated docs * Clarify issue type field description for repo-scoped list_issue_types |
||
|
Ross Tarrant
|
667bd3e803 | Fix IFC private repository labels (#2695) | ||
|
Matt Holloway
|
4e8eb81dac |
MCP Apps with extra functionality (#1974)
* PoC full flow (hello world example) * add avatar resource domain * add postmessage logic and richer UI * add create issue ui * update ui for issue creatioon * fix * ignore banner * update docs after rebase * update toolsnap for get_me * new UI changes * update docs * update workflows that need ui build * add UI diff * fix build ui step for windows runners to use git bash * fix UI diff * refactor issue creation UI * add AvatarWithFallback component and update UserCard to use it; enhance CreateIssueApp to manage existing issue data * fix formatting of button labels * add create pull request functionality with UI support and insiders * update docs * add test for insiders mode handling in ServerTool schema * remove `show_ui` param for now * make insiders mode metadata stripping generic * remove ui diff * fix CI * remove redundant mention of old app name * add node types to fix ide issues for ts code * remove unused TriangleDownIcon import * update @primer/behaviors and electron-to-chromium versions in package-lock.json * add check to ensure base and head are not the same when creating a new PR * remove old show_ui * fix gitignore for dist so builds dont break * add tests for insiders mode handling and metadata stripping in ServerTool * remove unused state and components from CreatePRApp * fix ui build * update docker build to fix npm issue * remove reference to show_ui * allow insiders to work for non-ui features * formalise insiders inventory support * update docs * fix overflow issues and replace pull request dropdown with matching UI from dotcom * fix createpullrequest test * consolidate fetching tools under `ui_get` tool to remove toolset deps * fix issue data prefill in issue_write form * fix link component when updating issue * fix avatar URL * fix broken issue update logic * remove dbg * fix for new GetFlags * revert to original required fields for create_pull_request * fix for UI form submission * Simplify MCP App UIs for basic branch Remove advanced features to be kept in mcp-ui-apps-advanced: - Strip labels, assignees, milestones, issue types, repo picker from issue-write - Strip repo picker, branch selectors from pr-write - Delete ui_get tool (ui_tools.go, ui_tools_test.go, ui_get.snap) - Remove UIGet registration from tools.go Basic forms retain: title, body, submit with _ui_submitted, draft/regular split button (PR), MarkdownEditor, and SuccessView. * Fix header spacing in issue-write and pr-write UIs Add proper spacing between icon, title text, and repo name in the header bar for both issue-write and create-pull-request forms. * fix UI spacing * Revert "Simplify MCP App UIs for basic branch" This reverts commit 24174b91e222e45b47913ff6b760db809f48660b. * Undo dependency downgrades in ui/package-lock.json * Update ui/src/apps/pr-write/App.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update ui/src/apps/issue-write/App.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Implement pagination for uiGetBranches (#2012) * Initial plan * Implement pagination for uiGetBranches function Co-authored-by: mattdholloway <918573+mattdholloway@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mattdholloway <918573+mattdholloway@users.noreply.github.com> * update to new insiders feature flag func * ensure transient state is reset on successive tool calls * Mark ui_get as app-only visibility ui_get backs only the MCP App views and has no business in the agent's tool list. Per the MCP Apps 2026-01-26 spec, omitting _meta.ui.visibility defaults to ["model","app"], which exposes the tool to the model. Declare visibility ["app"] so the host hides it from tools/list while the views can still invoke it via tools/call. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update ui_get toolsnap for app-only visibility Regenerated via UPDATE_TOOLSNAPS to capture the new _meta.ui.visibility. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Assert ui_get declares app-only visibility Locks in the _meta.ui.visibility ["app"] contract so a future edit can't silently re-expose the UI data tool to the model. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add ui_get to insiders feature docs Regenerated docs/feature-flags.md and docs/insiders-features.md to include the ui_get tool entry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address ui_get review feedback - Paginate the labels GraphQL query (cursor-based) so repos with more than 100 labels return a complete list instead of silently truncating. - Emit an empty due_on for milestones without a due date instead of formatting the zero time as "0001-01-01". - Use NewGitHubAPIErrorResponse in uiGetIssueTypes to preserve GitHub response context, matching the other REST-backed methods. - Extend tests to cover the labels (GraphQL), milestones (including the no-due-date case) and issue_types methods, plus the issue_types error path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix repo reset and stale base-branch in MCP App views - Re-initialize selectedRepo from toolInput inside the reset-on-invocation effect instead of a separate effect. The two effects both depended on toolInput and ran in declaration order, so the reset wiped the just- initialized repo and the picker never reflected the invocation's owner/repo. - Set the default base branch with a functional update in pr-write so a base prefilled from toolInput.base (or chosen by the user) isn't overwritten by a stale baseBranch value captured before the branches request resolved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Fix issue-write repo owner mapping and clear stale UI state on reset - issue-write: derive owner/name from full_name since search_repositories minimal output omits the owner object (mirrors pr-write) - pr-write/issue-write: clear available branch/label/assignee/milestone/type lists and filters in the toolInput reset effect so prefill effects can't match against the previous repo's stale data Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Merge remote-tracking branch 'origin/main' into mcp-ui-apps-advanced * feat: add pull request editing functionality with reviewers support * feat: implement interactive form handling for issue and pull request creation and updates * Close response body per page in ui_get pagination loops Avoids leaking HTTP connections when paging through assignees, milestones, branches, collaborators, and teams. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Cache pr-edit.html in build-ui action The build-ui cache only saved get-me/issue-write/pr-write HTML, so once a cache entry was stored it restored an incomplete ui_dist on later runs and skipped the rebuild, leaving pr-edit.html absent and panicking the tests. Add pr-edit.html to the cached paths and bump the cache key to v2 to evict the incomplete entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: tommaso-moro <tommaso-moro@github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Matt Holloway
|
d27540ff67 |
Add explicit show_ui parameter to UI-enabled write tools (#2601)
* Add explicit show_ui parameter to UI-enabled write tools
Today the server decides whether to route issue_write and create_pull_request
through the MCP App form using two implicit signals: _ui_submitted (set by
the form on submit) and a heuristic that bypasses the form when the call
carries any parameter the form cannot represent (labels, assignees,
issue_fields, state, reviewers, etc.). The model had no first-class,
documented way to say "execute directly, do not show a form".
Add a show_ui boolean parameter to the input schema of IssueWrite,
LegacyIssueWrite, and CreatePullRequest. It defaults to true and is
visible only to clients that advertise MCP App UI support: the strip
happens per-request in inventory.ToolsForRegistration via a new
stripUIOnlySchemaProperties helper, gated by the same predicate that
already strips _meta.ui (shouldStripMCPAppsMetadata). The two strips share
one decision so the schema and metadata stay in lock-step.
Form-routing predicate becomes:
MCPApps FF on && client supports UI &&
!_ui_submitted && show_ui && !hasNonFormParams
show_ui=false is a new explicit way for the model to opt out. The existing
non-form-param auto-bypass stays as a safety net, and the React forms keep
sending _ui_submitted=true on submit unchanged. get_me is out of scope
because its UI is pure client-side card rendering with no server-side
gating to replace.
The current strip gate ("strip when FF is off OR capability explicitly
absent") mirrors today's _meta.ui behavior exactly, including the
"capability unknown" case. For stdio that means UI-capable schemas are
exposed to any FF-enabled client. The handler-side clientSupportsUI check
still gates form execution at call time, so it is functionally a no-op for
non-UI stdio clients. A separate follow-up will tighten the gate to
"strip on unknown too" and wire an InitializedHandler in stdio to
re-register the un-stripped surface only after a UI-capable client has
advertised; the two changes must ship together to avoid breaking stdio.
docs/feature-flags.md and docs/insiders-features.md include an unrelated
"reviewers" description update picked up by script/generate-docs from
commit
|
||
|
JoannaaKL
|
fb7cbc8b85 |
Annotate read tools with ifc labels (#2671)
* Annotate read tools with ifc labels * Dont automatically enable IFCLabels in insiders mode * ifc: don't label unpublished repo advisories as public Repository security advisory listings can include draft/triage/closed advisories (via the state filter), which are not world-readable even on a public repository. Deriving confidentiality from repo visibility alone under-classified those results as public. LabelRepositorySecurityAdvisory now takes an allPublished flag and only returns a public label when the repo is public AND every returned advisory is published; otherwise it is private. list_repository_security_advisories computes allPublished from the response state; the org-wide listing stays private-untrusted. Adds unit + handler regression tests covering the draft-advisory-on-public-repo case. Addresses PR review feedback. * ifc: fix confidentiality under-classification in releases, collaborators, get_me Audit for the same bug class as the repo-advisory fix (confidentiality derived from a coarse signal that misses access-restricted items) found three more under-classifications: - Releases (list_releases, get_latest_release, get_release_by_tag): draft releases are visible only to push-access users and are not world-readable even on a public repo. New LabelRelease(isPrivate, hasDraft) returns public only for a non-draft release on a public repo; handlers compute hasDraft from the response (Draft flag / per-item scan). - list_repository_collaborators: a collaborator roster requires push access to list, so it is never world-readable, not even on a public repo. New LabelCollaboratorRoster() is always PrivateTrusted (mirrors LabelTeam), replacing the repo-visibility-derived label. - get_me: the result includes private_gists / total_private_repos / owned_private_repos, which are not part of the public profile. LabelGetMe is now PrivateTrusted instead of PublicTrusted. Verified the remaining public-capable labels are sound: Actions logs are world-readable on public repos; branches/tags are public metadata; gist, project, search, and starred-repo labels read per-item visibility and join. Adds ifc unit tests for the new/changed labels and a get_release_by_tag handler regression test (draft on public repo -> private); updates the get_me handler test to assert private. * ifc: document why list results use one joined label, not per-item Explain on LabelSearchIssues (and cross-ref from LabelGistList) that a tool result is delivered as one opaque payload and the IFC engine makes one allow/deny decision per flow at egress, so the only sound bound for a list is the meet of every item's label. Per-item labels would only be load-bearing if the engine could partition a result and route items to different sinks; until then they would invite unsafe declassification of a public item that arrived alongside private data. Doc-only change. |
||
|
Tim Rogers
|
8bbd902b9e |
Update title annotations for issue_write and add_issue_comment tools to reflect that they also work with pull requesta (#2664)
* Clarify issue tool titles for PR context Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix issue_write_ff_remote_mcp_issue_fields snap title Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Re-generate docs after upstream merge Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Ross Tarrant
|
33849e98eb |
fix: Empty assignees array should clear assignees (#2600)
* fix: Empty assignees array should clear assignees |
||
|
Matt Holloway
|
04c8dcbc8f |
Skip MCP App form when issue/PR write carries non-form params (#2589)
* Skip MCP App form when issue/PR write carries non-form params When MCP Apps are enabled and the client supports UI, issue_write and create_pull_request route the call to an interactive form. The form only collects a subset of fields and rebuilds the submit payload from scratch, so any parameter it cannot represent was silently dropped — e.g. labels, assignees, milestone, type, state and issue_fields (priority) for issue_write. Skip the form and execute directly whenever the call carries a parameter outside the set the form collects and re-sends. This generalizes the previous state-only guard and is robust to future parameter additions (an unrecognized param now bypasses the form rather than being lost). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Forward original tool params on MCP App form submit The issue-write and pr-write forms rebuilt their submit payload from scratch, so any parameter the form does not render was dropped on submit. Spread the original toolInput first and override only the edited fields, so unsupported params (e.g. issue_fields, labels, state) are preserved when the user submits the form. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Sam Morrow
|
51a7383fdd |
fix: make mergeIssueFieldValues output order deterministic
mergeIssueFieldValues built the merged slice by iterating a Go map, which produces non-deterministic ordering and caused a flake in Test_UpdateIssue/partial_update_with_issue_fields_reconciled_by_names (introduced in #2551). Switch to an order-preserving merge: emit incoming entries first in their original order, then any existing entries (in their original order) whose field IDs weren't seen in incoming. Semantics (incoming wins, existing preserved) unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Sam Morrow
|
7159cbf185 |
refactor: inline IssueWrite/LegacyIssueWrite as full duplicates
Replace the shared buildIssueWrite(includeIssueFields) helper with two fully duplicated tool definitions. When the FeatureFlagIssueFields flag is retired, LegacyIssueWrite can be deleted as a single function with no merge thinking required. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Sam Morrow
|
014fd17fa3 |
feat: gate issue_write and get_issue behind remote_mcp_issue_fields flag
Ports the gating from PR #2553 onto main (the original merge landed on a stack base that did not make it to main). Changes: - pkg/inventory: FeatureFlagDisable becomes []string (any-listed-on → hide). FeatureFlagEnable stays as a single string. This avoids the AND-of-enable semantics from the earlier proposal, which encoded dependencies rather than rollout knobs and had no real call site. Disable-OR is the case that does need the slice (LegacyIssueWrite below). - pkg/github/issues.go: split IssueWrite into IssueWrite (flag-enabled, exposes issue_fields) and LegacyIssueWrite (flag-disabled, omits it). Both register as 'issue_write'; mutually exclusive flag annotations pick exactly one at runtime. Refactored into a shared buildIssueWrite helper instead of duplicating the ~250-line tool definition. - pkg/github/issues.go: GetIssue field_values enrichment now requires the flag at runtime. The verbose REST IssueFieldValues is always cleared from the response. - Existing single-flag Disable call sites converted to slices. - New toolsnap variant issue_write_ff_remote_mcp_issue_fields.snap; the canonical issue_write.snap is owned by LegacyIssueWrite. - README + flag docs regenerated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Iulia B
|
2b7807bcb7 | add delete support and merge logic | ||
|
Kelsey Myers
|
6e19842c61 | schema: strict additionalProperties and typed value in issue_fields items | ||
|
Kelsey Myers
|
8b5c025669 | refactor(issue-fields): unexport IssueWriteFieldInput, fix issue_fields schema with oneOf | ||
|
Kelsey Myers
|
389ddcce59 | Fix field_option_name to pass option name to REST API, not DB ID - return minimal fields | ||
|
Iulia B
|
b91c3b5b3a |
feat(issue-fields): support issue_fields in issue_write using fullDatabaseId
- Expose fullDatabaseId (BigInt) in list_issue_fields - Add issue_fields parameter to issue_write for setting field values - Support single-select fields via field_option_name resolution - Add REST API field value extraction in get_issue responses - Update minimal types with IssueFieldValue for REST responses Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Sam Morrow
|
f5e26a8540 |
feat(issues): gate issue-fields features behind remote_mcp_issue_fields flag (#2520)
* feat(issues): gate issue-fields features behind remote_mcp_issue_fields flag Gates the recently merged issue-fields work (list_issue_fields tool, field_values enrichment on list_issues/search_issues, and field_filters input on list_issues) behind a new feature flag, also enabled in insiders mode. - list_issues splits into two same-named registrations: the field-aware variant requires the flag, while LegacyListIssues (FeatureFlagDisable) preserves the prior schema and GraphQL selection set so disabled callers don't pay the extra wire/server cost. - search_issues skips the field-values lookup when the flag is off. - list_issue_fields requires the flag to be registered at all. - Adopts <tool>_ff_<flag>.snap naming for flagged toolsnap variants so same-named duplicates each get a distinct snapshot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address PR review on issue-fields gating - docs generator: install a no-flags feature checker so README reflects the default user experience (tools enabled with no special flags), fixing duplicate `list_issues` and removing granular/flagged-only tools that were never meant to appear in the default docs. - csv_output: drop the FeatureFlagEnable/Disable exclusion in isCSVOutputTool. Wrapping happens before the per-request flag filter picks the live variant, so flag-gated list_* tools wrap safely; this restores CSV conversion for `list_issues` and enables it for `list_issue_fields` when both flags are on. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Ross Tarrant
|
f929c58c6b |
feat: Add CSV output format for default list tools under insiders mode (#2450)
* Add CSV output for list tools under insiders mode * fix: resolve rebase feature flag conflicts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Simplify feature-flag handling: collapse CSV dual-variant + skip filtering when no checker (#2516) * refactor: generic toolset+name sort, clarify feature flag intent Address review feedback on #2450: - Collapse the three near-identical sort helpers in pkg/inventory/filters.go into a generic sortByToolsetThenName so adding new inventory item types doesn't require copying the comparator. - Expand the doc comments on the three *WithoutFeatureFiltering helpers to spell out why they exist: HTTP mode builds a static (process-wide) inventory as an upper bound, but per-request feature flags from headers (X-MCP-Features, X-MCP-Insiders) are evaluated later, so feature-flagged variants must be preserved here. - Strengthen the doc comment on ResolveFeatureFlags to make the contract explicit: user-supplied flags are validated against AllowedFeatureFlags, but insiders expansion deliberately is not — InsidersFeatureFlags may include server-controlled flags that are not user-toggleable. CORS comments are intentionally left for the PR author. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(feature-flags): clarify allowed and insiders sets are independent Also add tests covering: - a user-toggleable flag (FeatureFlagIssuesGranular) that insiders does not turn on automatically - insiders mode not turning on user-only allowed flags Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor(inventory): collapse three *WithoutFeatureFiltering helpers into StaticUpperBound The three parallel methods (AvailableToolsWithoutFeatureFiltering, AvailableResourceTemplatesWithoutFeatureFiltering, AvailablePromptsWithoutFeatureFiltering) were always called as a triple in exactly two places: HTTP buildStaticInventory and its test mirror. They exist because the dual-variant pattern (sibling tools with mirrored FeatureFlagEnable / FeatureFlagDisable on the same name, e.g. CSV output) makes feature filtering at static-build time impossible — both variants must be kept and resolved per-request. Replace the three with one method, Inventory.StaticUpperBound(ctx), that returns (tools, resources, prompts) and carries the rationale in its doc comment. Reduces API surface, eliminates the triplication, and makes the single "skip feature filtering" concept obvious to readers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: simplify feature-flag handling Two related simplifications, both about treating insiders as a meta flag that expands once at startup and then stops mattering: - Collapse CSV's dual-variant pattern into a single tool whose handler performs a runtime feature-flag check via deps.IsFeatureEnabled. CSV is a pure response-format toggle, not a schema change, so it does not need the dual-name pattern that genuine schema variants (granular issues/PRs) still use. - When no feature checker is installed, skip feature-flag filtering and return the full upper bound. The static HTTP inventory now uses plain AvailableTools/Resources/Prompts; the per-request inventory always installs a checker, so MCP registration (which serves a tool name once) always sees a deduplicated set. The bespoke StaticUpperBound helper and the isToolEnabledWithFeatureFlags split go away. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci(mcp-diff): add insiders + per-feature configs The mcp-diff matrix now includes: - --insiders (and --insiders --read-only) - one config per github.AllowedFeatureFlags entry, generated by script/print-mcp-diff-configs so new user-controllable flags get diffed automatically without editing the workflow Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(insiders): explain feature-flag resolution for contributors Adds a 'How feature flags are resolved' section covering: - Insiders is a meta flag, like 'all'/'default' for toolsets - User input -> allowlist filter -> insiders expansion -> server-side fallback (remote only) - AllowedFeatureFlags vs InsidersFeatureFlags are independent - How to add a new feature flag, including the TestGitHubPackageDoesNotReadInsidersMode guard Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor(inventory): make feature-flag gating a regular ToolFilter Move tool feature-flag evaluation out of isToolEnabled and into a ToolFilter installed at the head of the pipeline by Build() when WithFeatureChecker received a non-nil checker. The 'no checker = no filtering' contract is now expressed structurally (the filter isn't installed) instead of by a runtime nil check inside the helper. Resources and prompts have no filter pipeline, so they call the now-pure featureFlagAllowed helper behind an explicit r.featureChecker != nil guard at the iteration site. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * perf(inventory): cache extracted toolset IDs in sort comparator Avoid evaluating the extractor closures up to three times per comparison. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: correct MCP features header in cors * docs: regenerate README for CSV output toolset Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: remove duplicate MCPFeaturesHeader from CORS headers * ci(mcp-diff): add streamable-http job with header-based configs Adds a sibling mcp-diff-http job that exercises the streamable-http transport against a shared HTTP server, with per-config settings supplied via X-MCP-* request headers — mirroring how the remote server is invoked in production (server-side defaults + per-user header overrides). The config generator gains a -transport flag: - stdio (default, unchanged behaviour) - http-headers (emits headers-only configs targeting a shared server) Two new combined entries layer multiple headers together as a smoke test for header-merging regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: regenerate after merging main Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Sam Morrow <info@sam-morrow.com> Co-authored-by: sammorrowdrums <sammorrowdrums@github.com> |
||
|
Tim Rogers
|
f39f758d6f |
Remove trailing periods from tool title annotations (#2518)
Tool title annotations should be consistent with the rest of the tool catalog and render cleanly in agent UIs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Sam Morrow <info@sam-morrow.com> |
||
|
Kelsey Myers
|
13e7f10fc8 |
Add custom field filtering to list_issues (#2480)
* Add custom field filtering to list_issues * Flatten schema * add repo fields flag * test fix --------- Co-authored-by: Sam Morrow <sammorrowdrums@github.com> |
||
|
Kelsey Myers
|
8f4680b90a |
Add field_values to search_issues results (#2474)
* Add field_values to search_issues results * remove dupe keys * Fix advanced search not enabled for fields --------- Co-authored-by: Sam Morrow <info@sam-morrow.com> |
||
|
Kelsey Myers
|
cc2a95725a |
Include custom issue field values in list_issues response (#2466)
* Include custom issue field values in list_issues response
Adds Issues 2.0 custom field values to each issue returned by the
list_issues GraphQL query, exposed on MinimalIssue as field_values:
[{field, value}]. Filtering by field is a separate concern (needs the
GraphQL IssueFilters input updated upstream) and is not included here.
shurcooL/graphql's response decoder walks every inline fragment of a
union regardless of __typename, so IssueFieldNumberValue.value is
aliased to valueNumber to avoid a Float-vs-String type clash when the
runtime variant is, e.g., a SingleSelectValue.
* Extend list_issues tests to cover Date/Number/Text field value variants
---------
Co-authored-by: Sam Morrow <info@sam-morrow.com>
|
||
|
Iulia Bejan
|
1861a351f8 |
Upgrade go-github from v82 to v87 (#2452)
Breaking changes addressed: - raw.NewClient: Use WithHTTPClient/WithEnterpriseURLs options, pass ctx to NewRequest, return (*Client, error) - internal/ghmcp/server.go: Use functional options for REST client creation, replace UserAgent field mutation with UserAgentTransport wrapper, add restUATransp field to githubClients struct - pkg/github/dependencies.go: Use functional options for REST client creation, handle raw.NewClient error return - pkg/github/actions.go: Handle new WorkflowDispatchRunDetails return value from CreateWorkflowDispatchEventByID/ByFileName - pkg/github/issues.go: Replace IssueListOptions with ListOptions for SubIssue.ListByIssue - pkg/github/notifications.go: MarkThreadDone now takes string instead of int64; remove ParseInt and strconv import - pkg/github/projects.go: Remove pointer indirection from ListProjectsPaginationOptions and ListProjectsOptions fields - pkg/github/issues_granular.go: Pass ctx to NewRequest, remove ctx from Do - Test files: Add mustNewGHClient helper, replace all NewClient calls, fix stubClientFnFromHTTP signature, fix lockdown_test.go BaseURL handling, fix raw_test.go, remove invalid threadID test case Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Gökhan Arkan
|
39d86b80af |
Replace ingress IFC reader list with private marker (#2478)
* Replace ingress IFC reader list with private marker Switches the ingress IFC labels from emitting a per-repo collaborator list to a single 'private' marker. The CLI engine now fetches readers from the GitHub endpoint on demand at egress decision time (P-F check), with pagination + caching, which removes a wire-bloat ceiling for repos with thousands of collaborators. Drops the per-call FetchRepoCollaborators from list_issues, issue_read, get_file_contents, search_issues, and search_repositories. The shared LabelSearchIssues helper collapses to a single []bool argument; the intersection logic and length-mismatch failure mode go away. This is a breaking wire-format change for _meta.ifc consumers — coordinate with the CLI cut-over. Refs github/copilot-mcp-core#1389. * format * Update FetchRepoCollaborators doc comment for marker-only ingress Addresses Copilot review on #2478. The helper is no longer called by the server itself; ingress emits a 'private' marker and the client engine resolves readers on demand. Kept exported per the library-consumer convention; updated the comment to reflect the new role. * Address review: drop FetchRepoCollaborators and make confidentiality a scalar Per Joanna's review on #2478: - Remove FetchRepoCollaborators entirely (no callers left after the marker switch). Drops the GetReposCollaboratorsByOwnerByRepo mock route too. - Change SecurityLabel.Confidentiality from []Confidentiality to a scalar Confidentiality. Wire format is now {integrity, confidentiality} where confidentiality is a single 'public' or 'private' string. Updated all tests and the LabelSearchIssues helper accordingly. |
||
|
Gökhan Arkan
|
883f58d979 |
Add ifc label for issue_read tool (#2457)
* Add ifc label for search_issues tool Emits an IFC SecurityLabel on the search_issues tool result when the InsidersMode flag is enabled, mirroring the pattern landed for get_me in #2432, list_issues in #2453, and get_file_contents in #2454. Search results may span multiple repositories, so the label is the IFC join of the per-repository labels: - Integrity is always untrusted (issues are user-authored). - If any matched repository is public, the joined readers are ["public"] (the public side dominates the lub). - Otherwise the joined readers are the intersection of the collaborator sets across all matched private repositories. - Empty result sets are labelled public-untrusted (no data leaked). The shared searchHandler in search_utils.go gains an additive variadic 'searchOption' hook so SearchIssues can attach _meta.ifc without duplicating the search call. SearchPullRequests is unaffected; it does not pass any options. If any per-repository visibility or collaborators lookup fails the label is omitted entirely, consistent with get_file_contents, to avoid misclassifying the result. Refs github/copilot-mcp-core#1623, github/copilot-mcp-core#1389. Note: this PR is chained on #2454 (gokhanarkan/fides-get-file-contents) because it depends on the FetchRepoIsPrivate and FetchRepoCollaborators helpers introduced there. GitHub will retarget the base to main once #2454 merges. * search_issues: address Copilot review findings - LabelSearchIssues now returns (SecurityLabel, bool); the bool is false when len(repoVisibilities) != len(readerSets), so callers can omit the label rather than emit one computed from inconsistent inputs. - searchIssuesIFCPostProcess no longer substitutes [owner] when the collaborators API returns an empty list. The substitution was inconsistent with the cross-repo intersection semantics: the owner could appear in another matched private repo's collaborator list and thereby widen the joined reader set incorrectly. Empty collaborator sets are now passed through unchanged. - Add a subtest exercising the collaborators-failure branch (500 on /repos/{owner}/{repo}/collaborators), asserting the tool still succeeds and result.Meta["ifc"] is absent. - Extend the LabelSearchIssues table tests with the slice-length mismatch case. Addresses the three Copilot findings on #2456. * search_issues: flip IFC join to intersection (private wins) Address Joanna's review feedback on #2456: a reader of a multi-repo result must be authorised to read every matched private repository, so the IFC join is the meet (intersection over private repos) rather than the join. Public matches contribute the universe set and drop out of the intersection without shrinking it. - LabelSearchIssues: collect only the private reader sets, then intersect. Empty result and all-public remain public-untrusted. - TestLabelSearchIssues: flip the mixed public+private expectation and add a 'two private + one public' case to lock in the new semantics. - Test_SearchIssues_IFC_InsidersMode: mixed subtest now expects the private repo's reader set instead of public. * Add ifc label for issue_read tool Emits an IFC SecurityLabel on the issue_read tool result when the InsidersMode flag is enabled, mirroring the pattern landed for get_me in #2432, list_issues in #2453, get_file_contents in #2454, and search_issues in #2456. issue_read operates on a single issue in a single repository so the label has the same per-repo semantics as list_issues; the helper ifc.LabelListIssues is reused directly. Integrity is always untrusted (issue contents, comments, and label descriptions are user-authored). Public repos are labelled PublicUntrusted; private repos are labelled PrivateUntrusted with the repository's collaborator logins, falling back to [owner] when the collaborators lookup fails. The IssueRead handler dispatches to four sub-functions (GetIssue, GetIssueComments, GetSubIssues, GetIssueLabels). The IFC label is attached at the dispatch site via a single attachIFC closure, so all four method branches emit the label without changes to the underlying helpers. Visibility-lookup failures cause the label to be omitted entirely (consistent with get_file_contents and search_issues). A future cleanup PR can extract attachIFC into a shared helper now that get_file_contents, search_issues, and issue_read use near-identical closures; intentionally not bundled here to keep the diff minimal. Refs github/copilot-mcp-core#1623, github/copilot-mcp-core#1389. Note: chained on #2456 (gokhanarkan/fides-search-issues), which is in turn chained on #2454. GitHub will retarget the base to main once those merge. * issue_read: simplify attachIFC by dropping unused lazy-cache Address Joanna's review feedback on #2457: the dispatch switch returns on exactly one branch, so attachIFC runs at most once per request. The ifcLabelKnown / ifcIsPrivate / ifcReaders cache variables were never reused across calls and only added complexity. Inline the visibility and collaborators lookups directly into the closure and drop the cache. Behaviour is identical; a follow-up can add real per-request caching across handlers if needed. |
||
|
Gökhan Arkan
|
9ad99c52c8 |
Add ifc label for search_issues tool (#2456)
* Add ifc label for search_issues tool Emits an IFC SecurityLabel on the search_issues tool result when the InsidersMode flag is enabled, mirroring the pattern landed for get_me in #2432, list_issues in #2453, and get_file_contents in #2454. Search results may span multiple repositories, so the label is the IFC join of the per-repository labels: - Integrity is always untrusted (issues are user-authored). - If any matched repository is public, the joined readers are ["public"] (the public side dominates the lub). - Otherwise the joined readers are the intersection of the collaborator sets across all matched private repositories. - Empty result sets are labelled public-untrusted (no data leaked). The shared searchHandler in search_utils.go gains an additive variadic 'searchOption' hook so SearchIssues can attach _meta.ifc without duplicating the search call. SearchPullRequests is unaffected; it does not pass any options. If any per-repository visibility or collaborators lookup fails the label is omitted entirely, consistent with get_file_contents, to avoid misclassifying the result. Refs github/copilot-mcp-core#1623, github/copilot-mcp-core#1389. Note: this PR is chained on #2454 (gokhanarkan/fides-get-file-contents) because it depends on the FetchRepoIsPrivate and FetchRepoCollaborators helpers introduced there. GitHub will retarget the base to main once #2454 merges. * search_issues: address Copilot review findings - LabelSearchIssues now returns (SecurityLabel, bool); the bool is false when len(repoVisibilities) != len(readerSets), so callers can omit the label rather than emit one computed from inconsistent inputs. - searchIssuesIFCPostProcess no longer substitutes [owner] when the collaborators API returns an empty list. The substitution was inconsistent with the cross-repo intersection semantics: the owner could appear in another matched private repo's collaborator list and thereby widen the joined reader set incorrectly. Empty collaborator sets are now passed through unchanged. - Add a subtest exercising the collaborators-failure branch (500 on /repos/{owner}/{repo}/collaborators), asserting the tool still succeeds and result.Meta["ifc"] is absent. - Extend the LabelSearchIssues table tests with the slice-length mismatch case. Addresses the three Copilot findings on #2456. * search_issues: flip IFC join to intersection (private wins) Address Joanna's review feedback on #2456: a reader of a multi-repo result must be authorised to read every matched private repository, so the IFC join is the meet (intersection over private repos) rather than the join. Public matches contribute the universe set and drop out of the intersection without shrinking it. - LabelSearchIssues: collect only the private reader sets, then intersect. Empty result and all-public remain public-untrusted. - TestLabelSearchIssues: flip the mixed public+private expectation and add a 'two private + one public' case to lock in the new semantics. - Test_SearchIssues_IFC_InsidersMode: mixed subtest now expects the private repo's reader set instead of public. |
||
|
Gökhan Arkan
|
525951397d |
Add ifc label for list_issues tool (#2453)
* Add ifc label for list_issues tool Emits an IFC SecurityLabel on the list_issues tool result when the InsidersMode flag is enabled, mirroring the pattern landed for get_me in #2432. Public repositories are labelled PublicUntrusted; private repositories are labelled PrivateUntrusted with the repository owner as a placeholder reader (full collaborator enumeration is intentionally deferred to a follow-up shared helper). A new IsPrivate field is added to the ListIssues GraphQL query types so visibility is available without a second round trip. Refs github/copilot-mcp-core#1623, github/copilot-mcp-core#1389. * list_issues: populate readers with repo collaborators Addresses Joanna's review feedback: for private repositories, populate the IFC confidentiality reader set with the repository's collaborator logins instead of the [owner] placeholder. Adds an exported FetchRepoCollaborators helper in pkg/github/repositories.go that paginates through Repositories.ListCollaborators. Mirrors the helper in github-mcp-server-remote (without the cache for now; cache can land in a follow-up). The lookup is invoked only for private repos under InsidersMode; if it fails we fall back to [owner] so the reader set is never empty for a private repo. |
||
|
Matt Holloway
|
62266f804b |
OSS granular PRs and issues toolsets (#2306)
Build and Test Go Project / build (macos-latest) (push) Has been cancelled
Docker / build (push) Has been cancelled
Build and Test Go Project / build (ubuntu-latest) (push) Has been cancelled
Build and Test Go Project / build (windows-latest) (push) Has been cancelled
MCP Server Diff / mcp-diff (push) Has been cancelled
CodeQL / Analyze (go) (push) Has been cancelled
CodeQL / Analyze (actions) (push) Has been cancelled
CodeQL / Analyze (javascript) (push) Has been cancelled
GoReleaser Release / release (push) Has been cancelled
Publish to MCP Registry / publish (push) Has been cancelled
* initial OSS granular PRs and issues toolsets * update docs * refactor: reuse existing helpers in granular toolsets Refactor granular issue and PR tools to delegate to existing tested helper functions instead of reimplementing logic from scratch: - Sub-issue tools (add/remove/reprioritize) now delegate to existing REST-based AddSubIssue, RemoveSubIssue, ReprioritizeSubIssue helpers - PR review tools (create/submit/delete) now delegate to existing CreatePullRequestReview, SubmitPendingPullRequestReview, DeletePendingPullRequestReview helpers (fixes viewer filtering bug) - Review comment tool now uses viewer-safe pattern from AddCommentToPendingReview (query viewer, filter by author, validate PENDING state, pass PullRequestReviewID) - Fix milestone param to use RequiredInt instead of float64 cast - Fix line/startLine params to use OptionalIntParam - Draft state tool uses typed GraphQL inputs matching existing patterns - Remove duplicate GraphQL types and helper functions - Add toolsnap tests for all 20 granular tools - Update generated docs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: use feature flags instead of separate granular toolsets Place granular tools in existing issues/pull_requests toolsets with FeatureFlagEnable, instead of creating separate issues_granular and pull_requests_granular toolsets. This is simpler and uses the existing feature flag infrastructure to switch between consolidated and granular tool variants at runtime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address review feedback on granular toolsets - Fix REST response handling: capture resp, close body, use ghErrors helpers in issueUpdateTool, prUpdateTool, GranularCreateIssue, and GranularRequestPullRequestReviewers - Add FeatureFlagDisable on consolidated tools (IssueWrite, SubIssueWrite, UpdatePullRequest, PullRequestReviewWrite, AddCommentToPendingReview) so they are hidden when granular variants are active - Use OptionalStringArrayParam for assignees, labels, reviewers instead of manual loop that silently dropped non-string elements - Fix side/startSide empty string leak: pass nil pointer when absent instead of pointer to empty string in GraphQL mutations - Fix milestone minimum from 0 to 1 to match RequiredInt rejection of 0 - Return MinimalResponse {id, url} instead of full JSON objects - Fix RequiredParam[bool] rejecting draft=false by using presence check - Add handler tests for update_pull_request_draft_state (draft + ready) and add_pull_request_review_comment with full GraphQL mocking Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address review feedback on granular toolsets - Fix translation keys to use ALL_CAPS convention (strings.ToUpper) - Fix assignees/labels clearing: check key presence instead of len==0 - Extract AddCommentToPendingReviewCall helper to deduplicate GraphQL logic between consolidated and granular tools - Add missing granular tools: resolve_review_thread, unresolve_review_thread (were in pull_request_review_write but had no granular replacements) - Add handler tests for new resolve/unresolve tools Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Sam Morrow <info@sam-morrow.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
copilot-swe-agent[bot]
|
f09dd5e774 |
Use key presence check instead of OptionalParam in UI gate
Build and Test Go Project / build (macos-latest) (push) Has been cancelled
CodeQL / Analyze (go) (push) Has been cancelled
CodeQL / Analyze (actions) (push) Has been cancelled
CodeQL / Analyze (javascript) (push) Has been cancelled
Docker / build (push) Has been cancelled
Build and Test Go Project / build (ubuntu-latest) (push) Has been cancelled
Build and Test Go Project / build (windows-latest) (push) Has been cancelled
GoReleaser Release / release (push) Has been cancelled
MCP Server Diff / mcp-diff (push) Has been cancelled
Publish to MCP Registry / publish (push) Has been cancelled
Check for the "state" key directly in the args map rather than using OptionalParam and ignoring its error. This ensures that a wrongly-typed state value bypasses the UI form (falling through to the normal validation path) instead of silently showing the form. Co-authored-by: mattdholloway <918573+mattdholloway@users.noreply.github.com> |
||
|
copilot-swe-agent[bot]
|
50a04616ea |
Skip MCP Apps UI form when update includes a state change
When using issue_write with method "update" and a state parameter (e.g. "closed"), the MCP Apps UI form was incorrectly shown. The form only handles title/body editing and would lose the state transition. Now when a state change is requested, the UI form is skipped and the update executes directly. Fixes github/github-mcp-server#798 Co-authored-by: mattdholloway <918573+mattdholloway@users.noreply.github.com> |
||
|
Ksenia Bobrova
|
f58208394a | Correctly wrap GraphQl error (#2149) | ||
|
Roberto Nacu
|
bf6467855c |
Reduce context usage for list_issues (#2098)
* reduce context usage for list_issues * address copilot feedback, align pagination tags to camelCase |
||
|
Matt Holloway
|
81f4c87a31 | make ui submit message prior to click even more insistent (#2096) | ||
|
Matt Holloway
|
584d0c9163 | clarify user confirmation requirement in issue and pull request creation messages (#2094) | ||
|
Matt Holloway
|
a94f95b43f |
Enhance client support checks for MCP Apps UI rendering (#2051)
* enhance client support checks for MCP Apps UI rendering * update dependencies and enhance MCP Apps UI support handling * chore: regenerate license files Auto-generated by license-check workflow * retrigger CI * update test * introduce constants for client names and remove wrong ide name for mcp apps support --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> |
||
|
Tommaso Moro
|
3ffc06b71d | reduce context for add_issue_comments using minimal types (#2063) | ||
|
Sam Morrow
|
16ff74a0eb |
feat: move copilot tools to default copilot toolset (#2039)
* feat: move copilot tools to default copilot toolset Move AssignCopilotToIssue and RequestCopilotReview from the issues and pull_requests toolsets respectively into a new default-enabled copilot toolset. This groups all copilot-related tools together and makes them available by default. - Set Default: true on ToolsetMetadataCopilot - Remove copilot from RemoteOnlyToolsets() - Update AllTools() grouping and tests - Regenerate docs Refs: github/copilot-mcp-core#1180 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: move copilot tools to dedicated copilot.go Extract AssignCopilotToIssue, RequestCopilotReview, AssignCodingAgentPrompt, and all supporting types/helpers from issues.go and pullrequests.go into copilot.go and copilot_test.go. This follows the existing convention where each domain (actions, dependabot, discussions, gists, etc.) has its own file, and keeps the copilot toolset implementation cohesive in one place. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Tommaso Moro
|
543a1fa019 | use minimal types for issue comments to optimize context usage (#2024) | ||
|
Tommaso Moro
|
dc7e789dc4 |
Optimize context usage for getting an issue using the issue_read tool (#2022)
* minimize context usage using minimal types for get issue comments * preserver reactions field * add back author association |