Skip to content

Commit de9bd78

Browse files
therealbradclaude
andauthored
fix(mcp): honor customField value filter in cases_list instead of dropping it (#341)
testplanit_cases_list accepted a customField argument but only filtered by field presence (displayName), and the Zod object silently stripped any `value` key — so `{ name: "Priority", value: "High" }` returned every case that had a Priority set, regardless of value. The published JSON schema even advertised additionalProperties:true, inviting clients to pass `value`. - Add value filtering: `{ name, value }` now resolves through the existing resolveCustomFields helper (the same canonicalization the create/update write path uses), so Dropdown/Multi-Select option names resolve to the option ids actually stored in caseFieldValues.value and the equality check matches. Dropdown/scalar -> { equals }, Multi-Select -> { array_contains }. `{ name }` alone keeps the presence-only behavior. - Make the schema a strictObject so unknown keys are rejected with a validation error rather than silently dropped. - An unknown/ambiguous field name or invalid option now returns a 422 error instead of unfiltered results. Fixes #333. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f1bceba commit de9bd78

2 files changed

Lines changed: 120 additions & 30 deletions

File tree

packages/mcp-server/src/tools/cases/list.test.ts

Lines changed: 77 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -154,27 +154,87 @@ describe("registerCasesList", () => {
154154
});
155155
});
156156

157-
it("BL-02: customField filter rejects unsupported `value` key (additionalProperties)", async () => {
157+
it("filter: customField {name,value} resolves a Dropdown value to an option-id equals filter", async () => {
158+
const priorityDropdown = {
159+
id: 50,
160+
displayName: "Priority",
161+
type: { type: "Dropdown" },
162+
fieldOptions: [
163+
{ fieldOption: { id: 99, name: "High" } },
164+
{ fieldOption: { id: 98, name: "Low" } },
165+
],
166+
};
167+
// First call: resolveCustomFields -> caseFields.findMany. Second: the list.
168+
mockZenstack
169+
.mockResolvedValueOnce([priorityDropdown])
170+
.mockResolvedValueOnce([]);
171+
const { client } = await setupClient();
172+
await client.callTool({
173+
name: "testplanit_cases_list",
174+
arguments: {
175+
projectId: 7,
176+
customField: { name: "Priority", value: "High" },
177+
},
178+
});
179+
const where = getLastCallBody()?.where as Record<string, unknown>;
180+
expect(where.caseFieldValues).toEqual({
181+
some: { fieldId: 50, value: { equals: 99 } },
182+
});
183+
});
184+
185+
it("filter: customField {name,value:[...]} resolves a Multi-Select to an array_contains filter", async () => {
186+
const components = {
187+
id: 60,
188+
displayName: "Components",
189+
type: { type: "Multi-Select" },
190+
fieldOptions: [
191+
{ fieldOption: { id: 11, name: "Auth" } },
192+
{ fieldOption: { id: 12, name: "Billing" } },
193+
],
194+
};
195+
mockZenstack.mockResolvedValueOnce([components]).mockResolvedValueOnce([]);
196+
const { client } = await setupClient();
197+
await client.callTool({
198+
name: "testplanit_cases_list",
199+
arguments: {
200+
projectId: 7,
201+
customField: { name: "Components", value: ["Auth"] },
202+
},
203+
});
204+
const where = getLastCallBody()?.where as Record<string, unknown>;
205+
expect(where.caseFieldValues).toEqual({
206+
some: { fieldId: 60, value: { array_contains: [11] } },
207+
});
208+
});
209+
210+
it("filter: customField {name,value} with an unknown field returns an error, not unfiltered results", async () => {
211+
// resolveCustomFields -> caseFields.findMany finds no match -> throws 422.
158212
mockZenstack.mockResolvedValueOnce([]);
159213
const { client } = await setupClient();
160-
// Zod object schemas are strict by default — passing an unknown
161-
// `value` key surfaces a validation error rather than being silently
162-
// swallowed (the prior bug).
163214
const result = await client.callTool({
164215
name: "testplanit_cases_list",
165-
arguments: { projectId: 7, customField: { name: "Priority", value: "High" } },
166-
});
167-
// The MCP framework returns isError:true with the Zod validation message
168-
// when the input is rejected. Accept either an isError result or a
169-
// successful call where the unknown key was stripped — what we care
170-
// about is that `value` is NOT silently included in the where clause.
171-
if (!result.isError) {
172-
const body = getLastCallBody();
173-
const where = body?.where as Record<string, unknown>;
174-
expect(where.caseFieldValues).toEqual({
175-
some: { field: { displayName: "Priority" } },
176-
});
177-
}
216+
arguments: {
217+
projectId: 7,
218+
customField: { name: "DoesNotExist", value: "x" },
219+
},
220+
});
221+
expect(result.isError).toBeTruthy();
222+
// The list query must never run with a dropped/ignored filter.
223+
expect(mockZenstack).toHaveBeenCalledTimes(1);
224+
});
225+
226+
it("#333: customField rejects unknown keys (strict schema) instead of silently dropping them", async () => {
227+
const { client } = await setupClient();
228+
const result = await client.callTool({
229+
name: "testplanit_cases_list",
230+
arguments: {
231+
projectId: 7,
232+
customField: { name: "Priority", bogus: "High" },
233+
},
234+
});
235+
expect(result.isError).toBeTruthy();
236+
// Strict-object validation fails before the handler runs — no query fired.
237+
expect(mockZenstack).not.toHaveBeenCalled();
178238
});
179239

180240
it("pagination: default limit=25, take=26, no cursor in body", async () => {

packages/mcp-server/src/tools/cases/list.ts

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import * as z from "zod/v4";
44
import { zenstack } from "../../api.js";
55
import type { EnvConfig } from "../../env.js";
66
import { mapHttpErrorToToolResult } from "../../errors.js";
7+
import { resolveCustomFields } from "./customFields.js";
78
import { mapCaseRow } from "./shared.js";
89

910
export interface CasesListDeps {
@@ -82,7 +83,7 @@ export function registerCasesList(server: McpServer, deps: CasesListDeps): void
8283
"testplanit_cases_list",
8384
{
8485
description:
85-
"List test cases scoped to a project. Filters: folderId, tagIds, name (case-insensitive substring), stateId, customField (by display name), issueId (linked Issue numeric id — see issues_list for resolution from external keys). Cursor pagination via the `cursor` returned in `nextCursor`. (per CASE-01 + EXEC-06 chain via D7-03) " +
86+
"List test cases scoped to a project. Filters: folderId, tagIds, name (case-insensitive substring), stateId, customField, issueId (linked Issue numeric id — see issues_list for resolution from external keys). customField takes {name} to match cases that have the field set, or {name, value} to match by value (Dropdown/Multi-Select accept the option name or id; an unknown field name or option returns a validation error rather than unfiltered results). Cursor pagination via the `cursor` returned in `nextCursor`. (per CASE-01 + EXEC-06 chain via D7-03) " +
8687
"Phase-8 maintenance filters: automated (user-controlled flag), source (single or array of RepositoryCaseSource), repositoryId, hasNeverExecuted (no junitResults AND no TestRunResults via TestRunCases), staleSinceUpdate (handler-side post-filter — bounded scan of POST_FILTER_SCAN_CAP=400; surfaces truncated:true when scan cap hit), updatedAfter/updatedBefore (filter via the repositoryCaseVersions relation since RepositoryCases has no updatedAt column). Each row carries lastUpdatedAt and latestResult (union of latest junitResults / TestRunResults). " +
8788
"Creator and date filters: creatorIds (array of user ids — matches any; deliberately array-shaped while runs_list/sessions_list use single-string createdById), from/to (ISO 8601 createdAt range).",
8889
inputSchema: {
@@ -91,17 +92,23 @@ export function registerCasesList(server: McpServer, deps: CasesListDeps): void
9192
tagIds: z.array(z.number().int().positive()).optional(),
9293
name: z.string().min(1).optional(),
9394
stateId: z.number().int().positive().optional(),
94-
// BL-02: only `name` is supported. Value-equality on the Json?
95-
// `caseFieldValues.value` column is unreliable across field types
96-
// (Dropdown stores option IDs, Multi-Select stores arrays, Text
97-
// stores strings). Silently swallowing an agent-supplied `value`
98-
// (the prior behavior) would let the agent act on incorrect data.
99-
// Dropping it from the schema makes the unsupported dimension
100-
// explicit; agents that need value filtering should call
101-
// testplanit_cases_get on candidate IDs and filter locally.
95+
// `{ name }` alone matches cases that have the field set (presence).
96+
// `{ name, value }` filters by value: resolveCustomFields canonicalizes
97+
// the value the same way the write path stores it (Dropdown/Multi-Select
98+
// option name -> option id), so the equality check matches what is
99+
// actually persisted in caseFieldValues.value. strictObject rejects any
100+
// other key with a validation error instead of silently dropping it.
102101
customField: z
103-
.object({
102+
.strictObject({
104103
name: z.string().min(1),
104+
value: z
105+
.union([
106+
z.string(),
107+
z.number(),
108+
z.boolean(),
109+
z.array(z.union([z.string(), z.number()])),
110+
])
111+
.optional(),
105112
})
106113
.optional(),
107114
// D7-03: filter cases linked to a specific issue. Pass the internal
@@ -151,9 +158,32 @@ export function registerCasesList(server: McpServer, deps: CasesListDeps): void
151158
}
152159
if (input.stateId !== undefined) where.stateId = input.stateId;
153160
if (input.customField) {
154-
where.caseFieldValues = {
155-
some: { field: { displayName: input.customField.name } },
156-
};
161+
if (input.customField.value === undefined) {
162+
// Presence filter — cases that have this field set (any value).
163+
where.caseFieldValues = {
164+
some: { field: { displayName: input.customField.name } },
165+
};
166+
} else {
167+
// Value filter — resolve {name, value} to the canonical
168+
// {fieldId, value} the write path persists. resolveCustomFields
169+
// throws 422 for unknown/ambiguous fields and invalid option
170+
// values, so an unmatched filter surfaces an error rather than
171+
// returning unfiltered results. It returns exactly one entry per
172+
// input key or throws, so [resolved] is always defined here.
173+
const [resolved] = await resolveCustomFields(
174+
{ [input.customField.name]: input.customField.value },
175+
deps.env,
176+
);
177+
const canonical = resolved!.value as Prisma.InputJsonValue;
178+
where.caseFieldValues = {
179+
some: {
180+
fieldId: resolved!.fieldId,
181+
value: Array.isArray(resolved!.value)
182+
? { array_contains: canonical }
183+
: { equals: canonical },
184+
},
185+
};
186+
}
157187
}
158188
if (input.issueId !== undefined) {
159189
// D7-03: RepositoryCases.issues is many-to-many to Issue. Filtering

0 commit comments

Comments
 (0)