fix: use plural channel_ids in query_incidents/query_changes#43
Merged
Conversation
Backend /incident/list and /change/list expect channel_ids ([]int64), not channel_id (int). The singular field was silently dropped, causing the channel filter to leak incidents/changes from every channel. Renames the MCP tool parameter channel_id → channel_ids (comma-separated string), parsed via the existing parseCommaSeparatedInts helper, and supports filtering by multiple channels in one call. BREAKING: callers passing channel_id: <n> must switch to channel_ids: "<n>". Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
5 tasks
4 tasks
ysyneu
added a commit
that referenced
this pull request
Apr 14, 2026
… tests (#44) * fix: send channel_ids array to backend in query_incidents/query_changes Backend /incident/list and /change/list expect channel_ids ([]int64), not channel_id (int). The singular field was silently dropped, causing the channel filter to leak incidents/changes from every channel. Renames the MCP tool parameter channel_id → channel_ids (comma-separated string), parsed via the existing parseCommaSeparatedInts helper, and supports filtering by multiple channels in one call. BREAKING: callers passing channel_id: <n> must switch to channel_ids: "<n>". Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: align tool descriptions with glossary and document supported transports Replace "collaboration space" with "channel" in tool descriptions, parameter descriptions, and READMEs to match the canonical Flashduty glossary (flashduty-docs/glossary.md). User-facing strings only — wire-level identifiers, JSON tags, and MCP parameter names are unchanged. Also add a "Supported Transports" table to README.md / README_zh.md clarifying: - stdio: supported - Streamable HTTP (/mcp, /flashduty): supported - Standalone SSE (legacy GET /sse): not supported (server returns 405 by design) Adds TEST_PLAN_param_audit.md capturing the wider audit sweep that surfaced this glossary drift. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: send 'query' instead of member_name/team_name to /member/list and /team/list Backend input structs (fc-pgy/cmd/server/controller/member/member.go:38-45 and fc-pgy/cmd/server/controller/team/team.go:311-318) declare the search keyword field as 'query', not 'member_name'/'team_name'. Our previous payload was silently dropped, so name-based search returned all members/teams unfiltered. Wire-only fix; the MCP-side 'name' parameter and tool descriptions are unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: drop TEST_PLAN_param_audit.md — audit complete The audit surfaced two real bugs (channel_ids in #43, query field in #45) and a glossary drift fix (this PR). With those landing, the planning artifact has served its purpose and shouldn't live in the repo. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(e2e): add live-API regression tests for channel and name filters Three new tests under the existing e2e build tag exercise the bugs fixed in this PR end-to-end against a real Flashduty cluster: - TestQueryIncidentsChannelFilter — picks the channel with the most recent incidents in an unfiltered query, then re-queries with channel_ids=that and asserts every returned incident matches. Catches the channel_id (singular) → channel_ids (array) wire-format bug. - TestQueryMembersNameFilter — pulls a substring from a real member's name (rune-aware so non-ASCII names work), filters with name=substring, asserts the result narrowed and each member matches. Catches the member_name → query bug. - TestQueryTeamsNameFilter — same shape as the members test against /team/list. The existing TestQueryIncidents/Members/Teams in the suite call the tools without filter args, so they could not have caught these bugs. Skips gracefully when there isn't enough data (no incidents in window, ≤1 member/team). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
query_incidentsandquery_changessentchannel_id(singularint) to the backend, but/incident/listand/change/listexpectchannel_ids([]int64). The singular field was silently dropped, leaking results from every channel.channel_id→channel_ids(comma-separated string), parsed via the existingparseCommaSeparatedIntshelper.Root cause
Backend input structs:
fc-event/cmd/server/controller/incident/info.go:384—ChannelIDs []int64 \json:"channel_ids"``fc-event/cmd/server/controller/change/info.go:27—ChannelIDs []int64 \json:"channel_ids"``Our MCP request body was
{"channel_id": N}, which the backend silently ignored.Breaking change
Existing callers passing
channel_id: <n>must switch tochannel_ids: "<n>". Without the switch, they continue to get unfiltered results (same bug as before the fix — no regression, but the call won't start filtering either).Test plan
query_incidentswithchannel_ids: "<single-id>"— verify only incidents from that channel are returned.query_incidentswithchannel_ids: "<id1>,<id2>"— verify incidents from both channels.query_incidentswith nochannel_ids— verify no regression (all-channel results).query_changes."channel_ids":[...](plural, array), not"channel_id":N.