fix(mcp): never marshal nil citations slice as JSON null

[sourcehawk]

What

Citation-bearing MCP tool outputs were failing with an opaque output-schema error instead of returning their result:

validating tool output: validating root: validating /properties/citations:
type: <invalid reflect.Value> has type "null", want "array"

Affected tools: analyze_change, correlate_with_findings (git), draft_pr (git), summarize_thread, analyze_channel (slack).

Root cause

The go-sdk auto-generates each tool's output schema from its typed Out struct. A field like Citations []citations.Citation (json "citations", no omitempty) becomes a non-nullable array. A nil Go slice marshals to JSON null, which the SDK's server-side output validation rejects — replacing the real result (even an errorResult) with the error above.

Two nil sources fed this:

1. citations.Run left Citations nil on its soft-fail returns (runner.go:69, :85). When a sub-agent can't produce a valid citations block after the corrective retry, the graceful soft-fail outcome carried a nil slice. This is the path that surfaces in normal use.
2. Handler early-return paths ("ref and question are required", clone failure, "channel_id is required", …) built output structs with a nil Citations. The SDK marshals + validates the typed Out whenever the handler's Go error return is nil (mcp/server.go ~L330-372), and these handlers report failures via errorResult with a nil Go error — so the early returns were validated too.

The existing soft-fail test asserted with assert.Empty, which passes for both nil and [], so the nil-vs-empty distinction was never pinned. Handler unit tests also call the method directly, bypassing the SDK's validation entirely.

Fix

Enforce one invariant: a citation-bearing Out never marshals a nil slice.

• pkg/mcp/citations/runner.go — both soft-fail returns set Citations: []Citation{}. Shared chokepoint: fixes the soft-fail path for all five tools.
• git tools_subagent.go / tools_draft_pr.go, slack tool_analyze.go / tool_summarize.go — early-return literals initialize Citations too.

Tests

• New in-memory wire tests (NewInMemoryTransports + CallTool) exercise the real SDK output validation that handler unit tests bypass. They reproduce the exact transcript error pre-fix.
• Strengthened the citations.Run soft-fail tests to assert NotNil (not just Empty).
• Verified RED→GREEN by stashing only the production .go changes: all 5 tests failed with the literal ... has type "null", want "array" error, then passed once restored.
• go test -race ./... clean; golangci-lint 0 issues. Frontend untouched.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Fixes MCP tool failures where citations was being marshaled as JSON null (from a nil Go slice) even though the auto-generated output schema requires a non-nullable array, causing the SDK to replace real tool results with an opaque output-schema validation error. This PR enforces the invariant that citation-bearing outputs always marshal citations as [].

Changes:

• Ensure pkg/mcp/citations.Run soft-fail outcomes always return Citations: []Citation{} (never nil).
• Initialize Citations to a non-nil empty slice on early-return/error-result paths in git + slack citation-bearing tools.
• Add in-memory “wire” tests that exercise the go-sdk’s real output-schema validation, and strengthen runner soft-fail tests to assert Citations is non-nil.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
pkg/mcp/citations/runner.go	Sets Citations to a non-nil empty slice in soft-fail outcomes to prevent JSON null.
pkg/mcp/citations/runner_test.go	Strengthens soft-fail assertions to pin nil-vs-empty behavior (NotNil + Empty).
pkg/mcp/git/tools_subagent.go	Initializes Citations on early-return error-result paths for citation-bearing git tools.
pkg/mcp/git/tools_draft_pr.go	Initializes draftPR output Citations up front so early returns never marshal null.
pkg/mcp/git/tools_subagent_wire_test.go	Adds in-memory transport tests to reproduce/guard the SDK output-schema validation failure.
pkg/mcp/slack/tool_summarize.go	Initializes Citations on early-return/error-result paths to avoid schema failures.
pkg/mcp/slack/tool_analyze.go	Same as above for analyze_channel.
pkg/mcp/slack/tools_subagent_wire_test.go	Adds in-memory transport test covering slack tools’ early-return paths through SDK validation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.