main
23 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Sam Morrow
|
acd2afc766 |
refactor(lockdown): trim comments to non-obvious invariants
The cache changes carried explanatory comments that restated the code or narrated what each step did. Drop them and keep only what the code cannot express: that cache2go never reclaims a named table, that its own expiry slides on every read, that createdAt survives entry updates, and that RepoAccessOpts is shared across requests. Exported options keep a short doc comment. Comment-only; no behavior change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
Sam Morrow
|
56c9088c60 |
fix(lockdown): scope repo-access cache per identity via entry keys
Isolating identities by deriving a cache2go table name per token grew a process-wide registry that is never reclaimed: cache2go creates each named table on first use and never evicts it, so every distinct bearer token — including invalid ones, since the table was built before GitHub validated the token — permanently added a table. Keep a single cache table and scope entries instead. WithIdentity stores a SHA-256 digest of the identity and prefixes each entry key with it, so different identities still cannot observe each other's trust decisions, while per-identity state is reclaimed by the table's ordinary TTL cleanup. WithCacheName stays for tenant/test isolation, with docs warning against deriving names from request data. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
Sam Morrow
|
a429a8759f |
fix(lockdown): bound repo-access cache expiry and isolate it per identity
The repo-access cache used by lockdown mode relied on cache2go's sliding expiry: every read extends an entry's life, so a frequently-accessed entry could keep a stale trust decision (e.g. revoked push access) alive indefinitely instead of refreshing after its TTL. Separately, cache2go.Cache(name) returns a process-wide singleton table keyed by name. In HTTP mode, RequestDeps.GetRepoAccessCache built a new RepoAccessCache per request but always reused the same default-named table, so trust decisions computed under one caller's credentials could be served to a different caller for the same owner/repo, without ever validating the second caller's own access. Fixes: - Track each cache entry's original creation time and bound its maximum age from that fixed point, not from last access, so entries are refreshed after a fixed TTL regardless of read frequency. - Add lockdown.CacheNameForIdentity, which derives a stable, hashed cache-table name from a request identity (e.g. auth token). Two calls for the same identity return the same name (reusing a warm cache across a session's repeated requests); different identities always get different names (no shared cache state). - RequestDeps.GetRepoAccessCache now scopes each request's cache to the requesting token's identity via CacheNameForIdentity, closing the cross-identity leak in HTTP/multi-tenant deployments. Stdio mode is unaffected: it constructs a single RepoAccessCache for the whole process lifetime, as before. Tests added: - TestRepoAccessCacheBoundedExpiryIgnoresRepeatedAccess and TestRepoAccessCacheNewUserDoesNotResetEntryAge exercise bounded expiry deterministically via an injectable clock (no sleeps). - TestCacheNameForIdentity and TestRepoAccessCacheIdentityScopedNamesPreventCrossIdentityLeakage cover the naming helper and cross-identity isolation at the lockdown package level. - TestGetRepoAccessCacheIsolatesTrustDecisionsPerIdentity in pkg/github mirrors the HTTP server's exact construction pattern end-to-end and fails without the dependencies.go fix. Fixes #3107 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
Sam Morrow
|
3bad3bc651 | fix(http): make server lockdown mode an upper bound over requests (#3112) | ||
|
Sam Morrow
|
21c5a6f1dd |
fix(auth): scope tokens across GitHub clients
Use exact configured host authorities for every REST, GraphQL, and raw client so redirects cannot reattach credentials to foreign hosts or ports. Add adversarial redirect and lookalike coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
Syed Anas Mohiuddin
|
198017fb54 |
Attach GitHub token only to configured GitHub hosts
BearerAuthTransport re-adds the Authorization header on every hop, which defeats net/http's cross-host redirect stripping. Scope the credential to the configured hosts so a redirect off them travels without the token. An empty AllowedHosts preserves prior behavior; the three production construction sites populate it from the configured REST, upload, GraphQL and raw hosts. |
||
|
Sam Morrow
|
8ec62491c6 |
Add confirmed repository deletion tool (#3076)
* feat(repos): add confirmed repository deletion Add a destructive delete_repository tool that requires an exact owner/repo confirmation through multi-round-trip elicitation. Gate the tool to MCP protocol 2026-07-28 and newer across local and remote transports. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8 * refactor(inventory): generalize tool availability guards Gate protocol-restricted tools on required elicitation capabilities and enforce direct calls inside the registered handler so SDK result finalization remains intact. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8 * feat(http): protect MRTR request state Seal repository deletion targets for self-hosted HTTP with a stable AES-256-GCM key. Hide only delete_repository when no key is configured and expose an optional sealer interface for remote integrators. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8 * fix(repos): expire deletion confirmations Bind sealed repository deletion state to the immutable repository ID and a ten-minute expiry. Re-check identity before deletion so replay cannot affect a recreated repository. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8 * fix(http): preserve tool and scope restrictions Apply static allowlists before removing unavailable tools and fail closed on invalid configured tool names. Model independent OAuth requirements as conjunctive groups so repository deletion requires both delete_repo and repo. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8 * fix(repos): require protected confirmation state Give stdio a process-local request-state sealer and make deletion fail closed without one. Preserve legacy any-of OAuth behavior globally while documenting and enforcing delete_repository's conjunctive delete_repo and repo requirements. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8 * fix(oauth): request repository deletion scope Include delete_repo in the supported OAuth scope set used by stdio login, HTTP protected-resource metadata, and tool filtering. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8 * fix(oauth): require deletion scope opt-in Keep delete_repo in protected-resource discovery for step-up authorization while excluding it from the default stdio OAuth grant. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8 * refactor(oauth): derive scope sets from catalog Generate protected-resource supported scopes and the lower-risk default OAuth grant from one canonical scope definition list. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8 * refactor(scopes): own OAuth scope catalog Move supported and default OAuth scope policy into pkg/scopes so protected-resource metadata and stdio grants derive from the scope domain package. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8 * fix(scopes): require workflow scope opt-in Keep workflow and codespace in protected-resource discovery while excluding both from the default OAuth grant alongside delete_repo. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8 --------- Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8 |
||
|
Tommaso Moro
|
3b8ff504c0 |
Add fields param to search_code and get_file_contents (#2775)
* Add fields param to search_code and get_file_contents Add an optional `fields` array parameter to the `search_code` and `get_file_contents` tools so callers can request only the fields they need, reducing tool response size and context usage. - search_code: filters each result item to the selected fields while preserving the total_count / incomplete_results wrapper. - get_file_contents: filters each directory entry when listing a directory; ignored for single-file responses. Adds shared filterFields / filterEachField helpers and per-tool field enums, plus unit tests and regenerated toolsnaps and docs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Gate fields param behind fields_param flag and add usage telemetry Register search_code and get_file_contents as two mutually exclusive variants gated by the new `fields_param` feature flag, following the existing dual-variant flag pattern: - The flag-enabled variant advertises the optional `fields` parameter and filters each result to the requested subset. It owns the `<tool>_ff_fields_param` toolsnap. - The Legacy* variant exposes the original schema with no `fields` parameter and never filters, acting as a kill switch when the flag is off. It owns the canonical toolsnap. Add best-effort, low-cardinality telemetry at each tool's filter point to measure adoption and realized savings: - `mcp.fields.tool_call` (increment) tagged by tool and whether the response was filtered. - `mcp.fields.bytes_full` / `bytes_sent` / `bytes_saved` (counters) tagged by tool, emitted only when a response was filtered. Tags are limited to `tool` and `filtered` to bound cardinality; repo, owner, user, query, and the requested field list are never tagged. The local server discards these via the noop metrics sink, while hosted deployments inject a real sink. Metrics accessors now fall back to a noop sink when no exporter is configured so emitting telemetry never panics. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Drop mcp.fields.bytes_saved metric Remove the mcp.fields.bytes_saved counter. It is derivable on the dashboard from the two remaining byte counters, since sum(bytes_full) - sum(bytes_sent) equals the total saved at any rollup, so emitting it separately is redundant. Keeping only bytes_full and bytes_sent shrinks the emitted telemetry surface. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <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> |
||
|
kerobbi
|
b5397f6e33 |
lockdown mode: remove RepoAccessCache singleton and isolate viewer state per instance
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
|
||
|
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> |
||
|
Sam Morrow
|
970155ad64 |
refactor: simplify NewServerTool naming (#2510)
- Rename NewServerToolWithRawContextHandler -> NewServerTool. This is the preferred constructor for raw mcp.ToolHandler tools because it avoids creating closures at registration time, which matters for per-request servers that re-register all tools on every request. - Rename deprecated generic NewServerTool[In, Out] -> NewServerToolWithDeps to free up the simpler name and make its closure-based nature explicit. The dynamic tools package is the only legitimate user of this constructor because DynamicToolDependencies differs from the standard ToolDependencies. - Remove deprecated NewServerToolFromHandler. Its only callers can use the new NewServerTool directly via context-injected deps. - Update all call sites in dependencies.go, dynamic_tools.go, and registry_test.go. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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> |
||
|
Roberto Nacu
|
4bded57e02 |
Fix lockdown mode permission check (#2361)
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
Publish to MCP Registry / publish (push) Has been cancelled
* use REST API for permission checks * update tests * skip API call for bots and add github-action[bot] to trusted logins * improve tests * add nil guard to IsSafeContent * add comment clarifying maintain mapping --------- Co-authored-by: Sam Morrow <info@sam-morrow.com> |
||
|
Matt Holloway
|
dd239d8443 |
Initial OSS logging adapter for http (#2008)
* initial logging stack for http * add metrics adapter * fix linter issues * make log fields generic * Update pkg/github/server_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Remove unused SlogMetrics adapter The slog-based metrics adapter was never used — OSS always uses NoopMetrics and the remote server has its own DataDog-backed adapter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update pkg/github/dependencies.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fmt * change to use slog * address feedback * rename noop adapter to noop sink * Update pkg/http/server.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * [WIP] [WIP] Address feedback on OSS logging adapter for http implementation (#2264) * Initial plan * Fix BaseDeps.Logger and BaseDeps.Metrics to return safe defaults when Obsv is nil Agent-Logs-Url: https://github.com/github/github-mcp-server/sessions/53221b0b-abb4-4138-a147-3ce9e13b379a Co-authored-by: mattdholloway <918573+mattdholloway@users.noreply.github.com> * Fix nil metrics in server.go by passing metrics.NewNoopMetrics() to NewExporters Agent-Logs-Url: https://github.com/github/github-mcp-server/sessions/53221b0b-abb4-4138-a147-3ce9e13b379a 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> Co-authored-by: Matt Holloway <mattdholloway@github.com> * replace nil with stubs --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> |
||
|
JoannaaKL
|
266bd9311e |
Gogithub update (#2004)
* Bump google/go-github * chore: regenerate license files Auto-generated by license-check workflow * Fix required block * Go mod vendor and tidy again * Remove unused fatih/color dependency to fix CI (#2005) * Initial plan * Remove unused github.com/fatih/color dependency The fatih/color package was listed in go.mod but not actually imported or used anywhere in the codebase. This caused the CI "go mod tidy -diff" check to fail. Running go mod tidy removed: - github.com/fatih/color v1.18.0 - github.com/mattn/go-colorable v0.1.13 (transitive) - github.com/mattn/go-isatty v0.0.20 (transitive) Fixes the failing ubuntu-latest workflow build. Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> |
||
|
Adam Holt
|
aa302209b0 |
Add Streamable HTTP mode (#1849)
Adds new `http` command supporting Streamable HTTP support, OAuth Metadata handler and Scope filtering. Co-authored-by: kerobbi <kerobbi@github.com> Co-authored-by: Matt Holloway <mattdholloway@github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |
||
|
Tony Truong
|
3af1129c6b |
adding feature flag on dependency level (#1801)
* wip injecting ff function into tool as dep * remove debug * fix linter * add better test * adding compile time check * move experimental to seperate config/ff value * adding test var * fixing test * adding flag and possibility to call feature checker * fixing name |
||
|
Sam Morrow
|
7796c08681 |
Add scope hierarchy and auto-derive accepted scopes
- Add ScopeHierarchy map defining parent-child scope relationships - Add ExpandScopes() function to derive accepted scopes from required scopes - Update NewTool/NewToolFromHandler to take []scopes.Scope and auto-derive AcceptedScopes - Add new scope constants: NoScope, User, ReadUser, UserEmail, ReadPackages, WritePackages - Update all tool files to use new signature with typed scopes - Add comprehensive tests for ExpandScopes The scope hierarchy allows automatic derivation of accepted scopes: - repo → public_repo, security_events - admin:org → write:org → read:org - project → read:project - write:packages → read:packages - user → read:user, user:email This enables the remote server to consume scope info directly from OSS tools. |
||
|
copilot-swe-agent[bot]
|
db179be4c3 |
Simplify scope implementation per feedback
- Replace NewToolWithScopes with updated NewTool that includes scope parameters - All tools now use the same constructor with explicit scope requirements - Documentation only shows accepted scopes when they differ from required - Removed NewToolWithScopes and NewToolFromHandlerWithScopes helper functions Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> |
||
|
copilot-swe-agent[bot]
|
eb6db0fb5d |
Add scopes package and update ServerTool struct with scope fields
- Created pkg/scopes package with OAuth scope constants - Added RequiredScopes and AcceptedScopes fields to ServerTool - Added NewToolWithScopes helpers in dependencies.go - Updated context tools (get_me, get_teams, get_team_members) with scopes Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> |
||
|
Sam Morrow
|
3c453dd9dd |
refactor: inject deps via context instead of closures
This refactor addresses performance issues in per-request server scenarios
where creating ~90 handler closures per request was causing latency.
Changes:
- Add ContextWithDeps, DepsFromContext, MustDepsFromContext to dependencies.go
- Add NewServerToolWithContextHandler, NewServerToolWithRawContextHandler to inventory
- Convert all 89 tool handlers from closure pattern to direct context-based deps
- Update all tests to inject deps into context before calling handlers
- Mark old NewServerTool and NewServerToolFromHandler as deprecated
The new pattern:
- Before: func(deps) handler { return func(ctx, req, args) { use deps } }
- After: func(ctx, deps, req, args) { use deps }
Dependencies are now injected into context once (via ContextWithDeps) and
extracted by NewTool internally before passing to handlers. This eliminates
closure creation on the hot path for remote servers.
|
||
|
Sam Morrow
|
ce2e4f9472 |
refactor: Introduce Inventory pattern with builder, filtering, and per-request optimization (#1589)
* refactor: separate ServerTool into own file with HandlerFunc pattern - Extract ServerTool struct into pkg/toolsets/server_tool.go - Add ToolDependencies struct for passing common dependencies to handlers - HandlerFunc allows lazy handler generation from Tool definitions - NewServerTool for new dependency-based tools - NewServerToolLegacy for backward compatibility with existing handlers - Update toolsets.go to store and pass dependencies - Update all call sites to use NewServerToolLegacy Co-authored-by: Adam Holt <4619+omgitsads@users.noreply.github.com> * Wire ToolDependencies through toolsets - Move ToolDependencies to pkg/github/dependencies.go with proper types - Use 'any' in toolsets package to avoid circular dependencies - Add NewTool/NewToolFromHandler helpers that isolate type assertion - Tool implementations will be fully typed with no assertions scattered - Infrastructure ready for incremental tool migration * refactor(search): migrate search tools to new ServerTool pattern Migrate search.go tools (SearchRepositories, SearchCode, SearchUsers, SearchOrgs) to use the new NewTool helper and ToolDependencies pattern. - Functions now take only TranslationHelperFunc and return ServerTool - Handler generation uses ToolDependencies for typed access to clients - Update tools.go call sites to remove getClient parameter - Update tests to use new Handler(deps) pattern This demonstrates the migration pattern for additional tool files. Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com> * Migrate context_tools to new ServerTool pattern (#1590) * refactor(search): migrate search tools to new ServerTool pattern Migrate search.go tools (SearchRepositories, SearchCode, SearchUsers, SearchOrgs) to use the new NewTool helper and ToolDependencies pattern. - Functions now take only TranslationHelperFunc and return ServerTool - Handler generation uses ToolDependencies for typed access to clients - Update tools.go call sites to remove getClient parameter - Update tests to use new Handler(deps) pattern This demonstrates the migration pattern for additional tool files. Co-authored-by: Adam Holt <oholt@github.com> * Migrate context_tools to new ServerTool pattern Convert GetMe, GetTeams, and GetTeamMembers to use the new typed dependency injection pattern: - Functions now take only translations helper, return toolsets.ServerTool - Handler is generated lazily via deps.GetClient/deps.GetGQLClient - Tests updated to use serverTool.Handler(deps) pattern - Fixed error return pattern to return nil for Go error (via result.IsError) Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com> * refactor(gists): migrate gists.go to NewTool pattern (#1591) * Migrate context_tools to new ServerTool pattern Convert GetMe, GetTeams, and GetTeamMembers to use the new typed dependency injection pattern: - Functions now take only translations helper, return toolsets.ServerTool - Handler is generated lazily via deps.GetClient/deps.GetGQLClient - Tests updated to use serverTool.Handler(deps) pattern - Fixed error return pattern to return nil for Go error (via result.IsError) Co-authored-by: Adam Holt <oholt@github.com> * refactor(gists): migrate gists.go to NewTool pattern Convert all gist tools (ListGists, GetGist, CreateGist, UpdateGist) to use the new NewTool helper with ToolDependencies injection. - Remove getClient parameter from function signatures - Use deps.GetClient(ctx) inside handlers - Standardize error handling with utils.NewToolResultErrorFromErr() - Update all tests to use serverTool.Handler(deps) pattern Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com> --------- Co-authored-by: Adam Holt <oholt@github.com> Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com> --------- Co-authored-by: Adam Holt <oholt@github.com> Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com> * refactor(notifications): migrate notifications.go to NewTool pattern (#1592) * refactor(notifications): migrate notifications.go to NewTool pattern Convert all notification tools to use the new NewTool helper with ToolDependencies injection. Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com> * Refactor repositories.go tools to use NewTool pattern with ToolDependencies Convert all 18 tool functions in repositories.go to use the new NewTool helper pattern with typed ToolDependencies, isolating type assertions to a single location and improving code maintainability. Functions converted: - GetCommit, ListCommits, ListBranches - CreateOrUpdateFile, CreateRepository, GetFileContents - ForkRepository, DeleteFile, CreateBranch, PushFiles - ListTags, GetTag, ListReleases, GetLatestRelease, GetReleaseByTag - ListStarredRepositories, StarRepository, UnstarRepository This is part of a stacked PR series to systematically migrate all tool files to the new pattern. Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com> * refactor(issues): migrate issues.go to NewTool pattern Convert all 8 tool functions in issues.go to use the new NewTool helper pattern which standardizes dependency injection: - IssueRead: GetClient, GetGQLClient, RepoAccessCache, Flags - ListIssueTypes: GetClient - AddIssueComment: GetClient - SubIssueWrite: GetClient - SearchIssues: GetClient - IssueWrite: GetClient, GetGQLClient - ListIssues: GetGQLClient - AssignCopilotToIssue: GetGQLClient Updated tools.go to use direct function calls instead of NewServerToolLegacy wrappers. Updated all tests in issues_test.go to use the new ToolDependencies pattern and Handler() method. Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com> * refactor(pullrequests): convert PR tools to NewTool pattern Convert all 10 pull request tool functions to use the NewTool pattern with ToolDependencies injection: - PullRequestRead - CreatePullRequest - UpdatePullRequest - ListPullRequests - MergePullRequest - SearchPullRequests - UpdatePullRequestBranch - PullRequestReviewWrite - AddCommentToPendingReview - RequestCopilotReview Update tools.go to use direct function calls (removing NewServerToolLegacy wrappers) for PR functions. Update all tests in pullrequests_test.go to use the new handler pattern with deps and 2-value return. Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com> * Refactor actions.go to use NewTool pattern Convert all 14 tool functions in actions.go to use the NewTool pattern with ToolDependencies for dependency injection. This is part of a broader effort to standardize the tool implementation pattern across the codebase. Changes: - ListWorkflows, ListWorkflowRuns, RunWorkflow, GetWorkflowRun - GetWorkflowRunLogs, ListWorkflowJobs, GetJobLogs - RerunWorkflowRun, RerunFailedJobs, CancelWorkflowRun - ListWorkflowRunArtifacts, DownloadWorkflowRunArtifact - DeleteWorkflowRunLogs, GetWorkflowRunUsage The new pattern: - Takes only translations.TranslationHelperFunc as parameter - Returns toolsets.ServerTool with Tool and Handler - Handler receives ToolDependencies for client access - Enables better testability and consistent interface Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com> * refactor(git): migrate GetRepositoryTree to NewTool pattern * refactor(security): migrate code_scanning, secret_scanning, dependabot to NewTool pattern Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com> * refactor(discussions): migrate to NewTool pattern Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com> * Refactor security_advisories tools to use NewTool pattern Convert 4 functions from NewServerToolLegacy wrapper to NewTool: - ListGlobalSecurityAdvisories - GetGlobalSecurityAdvisory - ListRepositorySecurityAdvisories - ListOrgRepositorySecurityAdvisories Update tools.go toolset registration and tests. Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com> * refactor: convert projects, labels, and dynamic_tools to NewTool pattern This PR converts projects.go, labels.go, and dynamic_tools.go from the legacy NewServerToolLegacy wrapper pattern to the new NewTool pattern with proper ToolDependencies. Changes: - projects.go: Convert all 9 project functions to use NewTool with ToolHandlerFor[map[string]any, any] and 3-return-value handlers - projects_test.go: Update tests to use new serverTool.Handler(deps) pattern - labels.go: Convert GetLabel, ListLabels, and LabelWrite to NewTool pattern - labels_test.go: Update tests to use new pattern - dynamic_tools.go: Refactor functions to return ServerTool directly (using NewServerToolLegacy internally since they have special dependencies) - tools.go: Remove NewServerToolLegacy wrappers for dynamic tools registration Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com> * Add --features CLI flag for feature flag support Add CLI flag and config support for feature flags in the local server: - Add --features flag to main.go (StringSlice, comma-separated) - Add EnabledFeatures field to StdioServerConfig and MCPServerConfig - Create createFeatureChecker() that builds a set from enabled features - Wire WithFeatureChecker() into the toolset group filter chain This enables tools/resources/prompts that have FeatureFlagEnable set to a flag name that is passed via --features. The checker uses a simple set membership test for O(1) lookup. Usage: github-mcp-server stdio --features=my_feature,another_feature GITHUB_FEATURES=my_feature github-mcp-server stdio * Add validation tests for tools, resources, and prompts metadata This commit adds comprehensive validation tests to ensure all MCP items have required metadata: - TestAllToolsHaveRequiredMetadata: Validates Toolset.ID and Annotations - TestAllToolsHaveValidToolsetID: Ensures toolsets are in AvailableToolsets() - TestAllResourcesHaveRequiredMetadata: Validates resource metadata - TestAllPromptsHaveRequiredMetadata: Validates prompt metadata - TestToolReadOnlyHintConsistency: Validates IsReadOnly() matches annotation - TestNoDuplicate*Names: Ensures unique names across tools/resources/prompts - TestAllToolsHaveHandlerFunc: Ensures all tools have handlers - TestDefaultToolsetsAreValid: Validates default toolset IDs - TestToolsetMetadataConsistency: Ensures consistent descriptions per toolset Also fixes a bug discovered by these tests: ToolsetMetadataGit was defined but not added to AvailableToolsets(), causing get_repository_tree to have an invalid toolset ID. * Fix default toolsets behavior when not in dynamic mode When no toolsets are specified and dynamic mode is disabled, the server should use the default toolsets. The bug was introduced when adding dynamic toolsets support: 1. CleanToolsets(nil) was converting nil to empty slice 2. Empty slice passed to WithToolsets means 'no toolsets' 3. This resulted in zero tools being registered Fix: Preserve nil for non-dynamic mode (nil = use defaults in WithToolsets) and only set empty slice when dynamic mode is enabled without explicit toolsets. * refactor: address PR review feedback for toolsets - Rename AddDeprecatedToolAliases to WithDeprecatedToolAliases for immutable filter chain consistency (returns new ToolsetGroup) - Remove unused mockGetRawClient from generate_docs.go (use nil instead) - Remove legacy ServerTool functions (NewServerToolLegacy and NewServerToolFromHandlerLegacy) - no usages - Add panic in Handler()/RegisterFunc() when HandlerFunc is nil - Add HasHandler() method for checking if tool has a handler - Add tests for HasHandler and nil handler panic behavior - Update all tests to use new WithDeprecatedToolAliases pattern * refactor: Apply HandlerFunc pattern to resources for stateless NewToolsetGroup This change applies the same HandlerFunc pattern used by tools to resources, allowing NewToolsetGroup to be fully stateless (only requiring translations). Key changes: - Add ResourceHandlerFunc type to toolsets package - Update ServerResourceTemplate to use HandlerFunc instead of direct Handler - Add HasHandler() and Handler(deps) methods to ServerResourceTemplate - Update RegisterResourceTemplates to take deps parameter - Refactor repository resource definitions to use HandlerFunc pattern - Make AllResources(t) stateless (only takes translations) - Make NewToolsetGroup(t) stateless (only takes translations) - Update generate_docs.go - no longer needs mock clients - Update tests to use new patterns This resolves the concern about mixed concerns in doc generation - the toolset metadata and resource templates can now be created without any runtime dependencies, while handlers are generated on-demand when deps are provided during registration. * refactor: simplify ForMCPRequest switch cases * refactor(generate_docs): use strings.Builder and AllTools() iteration - Replace slice joining with strings.Builder for all doc generation - Iterate AllTools() directly instead of ToolsetIDs()/ToolsForToolset() - Removes need for special 'dynamic' toolset handling (no tools = no output) - Context toolset still explicitly handled for custom description - Consistent pattern across generateToolsetsDoc, generateToolsDoc, generateRemoteToolsetsDoc, and generateDeprecatedAliasesTable * feat(toolsets): add AvailableToolsets() with exclude filter - Add AvailableToolsets() method that returns toolsets with actual tools - Support variadic exclude parameter for filtering out specific toolsets - Simplifies doc generation by removing manual skip logic - Naturally excludes empty toolsets (like 'dynamic') without special cases * refactor(generate_docs): hoist success logging to generateAllDocs * refactor: consolidate toolset validation into ToolsetGroup - Add Default field to ToolsetMetadata and derive defaults from metadata - Move toolset validation into WithToolsets (trims whitespace, dedupes, tracks unrecognized) - Add UnrecognizedToolsets() method for warning about typos - Add DefaultToolsetIDs() method to derive defaults from metadata - Remove redundant functions: CleanToolsets, GetValidToolsetIDs, AvailableToolsets, GetDefaultToolsetIDs - Update DynamicTools to take ToolsetGroup for schema enum generation - Add stubTranslator for cases needing ToolsetGroup without translations This eliminates hardcoded toolset lists - everything is now derived from the actual registered tools and their metadata. * refactor: rename toolsets package to registry with builder pattern - Rename pkg/toolsets to pkg/registry (better reflects its purpose) - Split monolithic toolsets.go into focused files: - registry.go: Core Registry struct and MCP methods - builder.go: Builder pattern for creating Registry instances - filters.go: All filtering logic (toolsets, read-only, feature flags) - resources.go: ServerResourceTemplate type - prompts.go: ServerPrompt type - errors.go: Error types - server_tool.go: ServerTool and ToolsetMetadata (existing) - Fix lint: Rename RegistryBuilder to Builder (avoid stuttering) - Update all imports across ~45 files This refactoring improves code organization and makes the registry's purpose clearer. The builder pattern provides a clean API: reg := registry.NewBuilder(). SetTools(tools). WithReadOnly(true). WithToolsets([]string{"repos"}). Build() * fix: remove unnecessary type arguments in helper_test.go * fix: restore correct behavior for --tools and --toolsets flags Two behavioral regressions were fixed in resolveEnabledToolsets(): 1. When --tools=X is used without --toolsets, the server should only register the specified tools, not the default toolsets. Now returns an empty slice instead of nil when EnabledTools is set. 2. When --toolsets=all --dynamic-toolsets is used, the 'all' and 'default' pseudo-toolsets should be removed so only the dynamic management tools are registered. This matches the original pre-refactor behavior. * Move labels tools to issues toolset Labels are closely related to issues - you add labels to issues, search issues by label, etc. Keeping them in a separate toolset required users to explicitly enable 'labels' to get this functionality. Moving to issues toolset makes labels available by default since issues is a default toolset. * Restore labels toolset with get_label in both issues and labels This restores conformance with the original behavior where: - get_label is in issues toolset (read-only label access for issue workflows) - get_label, list_label, label_write are in labels toolset (full management) The duplicate get_label registration is intentional - it was in both toolsets in the original implementation. Added test exception to allow this case. * Fix instruction generation and capability advertisement - Expand nil toolsets to default IDs before GenerateInstructions (nil means 'use defaults' in registry but instructions need actual names) - Remove unconditional HasTools/HasResources/HasPrompts=true in NewServer (let SDK determine capabilities based on registered items, matching main) * Add tests for dynamic toolset management tools Tests cover: - list_available_toolsets: verifies toolsets are listed with enabled status - get_toolset_tools: verifies tools can be retrieved for a toolset - enable_toolset: verifies toolset can be enabled and marked as enabled - enable_toolset invalid: verifies proper error for non-existent toolset - toolsets enum: verifies tools have proper enum values in schema * Advertise all capabilities in dynamic toolsets mode In dynamic mode, explicitly set HasTools/HasResources/HasPrompts=true since toolsets with those capabilities can be enabled at runtime. This ensures clients know the server supports these features even when no tools/resources/prompts are initially registered. * Improve conformance test with dynamic tool calls and JSON normalization - Add dynamic tool call testing (list_available_toolsets, get_toolset_tools, enable_toolset) - Parse and sort embedded JSON in text fields for proper comparison - Separate progress output (stderr) from summary (stdout) for CI - Add test type field to distinguish standard vs dynamic tests * Add conformance-report to .gitignore * Add conformance test CI workflow - Runs on pull requests to main - Compares PR branch against merge-base with origin/main - Outputs full conformance report to GitHub Actions Job Summary - Uploads detailed report as artifact for deeper investigation - Does not fail the build on differences (may be intentional) * Add map indexes for O(1) lookups in Registry Address review feedback to use maps for collections. Added lookup maps (toolsByName, resourcesByURI, promptsByName) while keeping slices for ordered iteration. This provides O(1) lookup for: - FindToolByName - filterToolsByName (used by ForMCPRequest) - filterResourcesByURI - filterPromptsByName Maps are built once during Build() and shared in ForMCPRequest copies. * perf(registry): O(1) HasToolset lookup via pre-computed set Add toolsetIDSet (map[ToolsetID]bool) to Registry for O(1) HasToolset lookups. Previously HasToolset iterated through all tools, resourceTemplates, and prompts to check if any belonged to the given toolset. Now it's a simple map lookup. The set is populated during the single-pass processToolsets() call, which already collected all valid toolset IDs. This adds zero new iteration - just returns the existing validIDs map. processToolsets now returns 6 values: - enabledToolsets, unrecognized, toolsetIDs, toolsetIDSet, defaultToolsetIDs, descriptions * simplify: remove lazy toolsByName map - not needed for actual use cases FindToolByName() is only called once per request at most (to find toolset ID for dynamic enablement). The SDK handles tool dispatch after registration. A simple linear scan over ~90 tools is trivially fast and avoids: - sync.Once complexity - Map allocation - Premature optimization for non-existent 'repeated lookups' The pre-computed maps we keep (toolsetIDSet, etc.) are justified because they're used for filtering logic that runs on every request. * Add generic tool filtering mechanisms to registry package - Add Enabled field to ServerTool for self-filtering based on context - Add ToolFilter type and WithFilter method to Builder for cross-cutting filters - Update isToolEnabled to check Enabled function and builder filters in order: 1. Tool's Enabled function 2. Feature flags (FeatureFlagEnable/FeatureFlagDisable) 3. Read-only filter 4. Builder filters 5. Toolset/additional tools check - Add FilteredTools method to Registry as alias for AvailableTools - Add comprehensive tests for all new functionality - All tests pass and linter is clean Closes #1618 Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> * docs: improve filter evaluation order and FilteredTools documentation - Add numbered filter evaluation order to isToolEnabled function doc - Number inline comments for each filter step (1-5) - Clarify FilteredTools error return is for future extensibility - Document that library consumers may need to surface recoverable errors Addresses review feedback on PR #1620 * Refactor GenerateToolsetsHelp() to use strings.Builder pattern Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --------- Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> * Port functional changes from main to registry pattern Port three functional improvements from main branch: - GraphQL review comments grouped as threads (#1554) - get_file_contents description improvement (#1582) - create_or_update_file SHA validation fix (#1621) Adapted implementations to use the new registry pattern with: - BaseDeps for providing clients via ToolDependencies interface - deps.GetClient(ctx) and deps.GetGQLClient(ctx) patterns - Updated tests to use GraphQL mocks for review comments - Added SHA validation test cases for create_or_update_file * fix(e2e): Fix e2e test compilation and add rate limit handling - Fix DefaultToolsetIDs() type mismatch by using github.GetDefaultToolsetIDs() - Add waitForRateLimit() to check and wait for rate limits before each test - Add skip conditions for Copilot tests when Copilot isn't available - Use multi-line file content in TestPullRequestReviewCommentSubmit for multi-line review comments to work correctly - Improve error messages to include response details * fix(gists): Use proper GitHub API error handling for observability The gists.go file was using NewToolResultErrorFromErr for GitHub API errors, which breaks the error middleware tracking that the remote server uses for observability and incident detection. Changed API errors (client.Gists.List, Get, Create, Edit) to use ghErrors.NewGitHubAPIErrorResponse which properly: - Records errors in the context for middleware access - Preserves the response object for rate limit and status tracking - Maintains consistency with other tools that use this pattern This ensures production observability is maintained for Gist operations. * chore: Update server.json schema to 2025-12-11 - Update schema URL to latest version (2025-12-11) - Remove 'status' field (now managed by registry per 2025-09-29 changelog) * fix(get_file_contents): Restore correct implementation from #1582 The refactor incorrectly restructured the GetFileContents logic: - Move 'if rawOpts.SHA != "" { ref = rawOpts.SHA }' before GetContents call - Always call GetContents first (not conditionally based on path suffix) - Restore matchFiles helper function for proper fallback handling - Use matchFiles when Contents API fails or raw API fails This aligns with the improvements from PR #1582 that was merged into main. * Rename registry to inventory in comments Update remaining references to 'registry' in code comments to use 'inventory' consistently after the package rename. --------- Co-authored-by: Adam Holt <4619+omgitsads@users.noreply.github.com> Co-authored-by: Adam Holt <omgitsads@users.noreply.github.com> Co-authored-by: Adam Holt <oholt@github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> |