Skip to content

fix: return non-nil empty completion results - #3665

Open
omgitsads wants to merge 1 commit into
mainfrom
omgitsads-non-nil-completion-results
Open

omgitsads wants to merge 1 commit into
mainfrom
omgitsads-non-nil-completion-results

Conversation

@omgitsads

Copy link
Copy Markdown
Member

Summary

Return non-nil empty completion results for prompt completions and direct non-resource calls to the repository completion handler. Both paths serialize completion values as [], not null.

Why

A successful completion handler returning a typed nil *mcp.CompleteResult can panic during SDK result finalization after request recovery middleware has returned. This fixes the upstream source of that production crash; the remote defense-in-depth wrapper is tracked in github/github-mcp-server-remote#1108.

No upstream issue is associated with this fix. The remote dependency bump is release-dependent and is not included here; no SDK changes are included.

What changed

  • Return an empty CompleteResult with an initialized values slice for ref/prompt in CompletionsHandler.
  • Return the same non-nil empty result for non-resource calls to RepositoryResourceCompletionHandler.
  • Assert non-nil results and JSON values: [] for both paths, and cover unchanged repo:// routing, client errors, unsupported references, and unsupported resource URIs.

MCP impact

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

No tool definitions or schemas change. Empty completion responses now return a valid result instead of a nil pointer; repository resource resolution and error handling are unchanged.

Prompts tested (tool changes only)

  • N/A: no tool changes; completion behavior is covered by unit tests.

Security / limits

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

This prevents a completion-result nil-pointer crash without changing authentication, permissions, data exposure, or limits.

Tool renaming

  • I am renaming tools as part of this PR (e.g. a part of a consolidation effort)
    • I have added the new tool aliases in deprecated_tool_aliases.go
  • I am not renaming tools as part of this PR

No tool names change.

Note: if you're renaming tools, you must add the tool aliases. For more information on how to do so, please refer to the official docs.

Lint & tests

  • Linted locally with ./script/lint
  • Tested locally with ./script/test

Validation results:

  • go test ./pkg/github -run 'Test(CompletionsHandler|RepositoryResourceCompletionHandler)' -count=1 — passed.
  • script/lint — passed, 0 issues. The initial automatic linter download stalled; reran successfully using the already-installed matching golangci-lint v2.14.0 binary in the ignored bin/ directory.
  • script/test — passed (go test -race ./...).
  • go build ./cmd/github-mcp-server — passed.
  • git diff --check — passed.

Docs

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

Internal completion bug fix only; no tool definitions, schemas, toolsets, or generated documentation change.

Return an empty values array for prompt and non-resource completions to prevent nil result finalization panics. Add regression coverage for serialization, resource routing, and errors.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@omgitsads
omgitsads requested a review from a team as a code owner November 3, 2026 20:41
Copilot AI auto review requested due to automatic review settings November 3, 2026 20:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are identified, and the change addresses nil completion results with corresponding coverage for result handling and routing.

No open issues 🎉

What changed in this PR

This PR prevents nil completion results from causing SDK finalization panics by returning non-nil empty results that serialize as values: [] for prompt completions and direct non-resource repository completion calls.

Changes:

  • Return initialized empty CompleteResult values for both completion paths.
  • Preserve existing repository-resource routing and error behavior.
  • Add coverage for empty results, JSON serialization, routing, client errors, and unsupported references or URIs.
File Description
pkg/​github/​server.go Returns a non-nil empty result for prompt completions.
pkg/​github/​server_test.go Tests completion routing, errors, non-nil results, and JSON serialization.
pkg/​github/​repository_resource_completions.go Returns a non-nil empty result for direct non-resource calls.
pkg/​github/​repository_resource_completions_test.go Expands coverage for empty results and serialization.

Review effort: Lite (auto)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants