Skip to content

Commit 7e67fce

Browse files
fix: validate empty MCP comparison reports (#435)
1 parent 03d1264 commit 7e67fce

2 files changed

Lines changed: 83 additions & 8 deletions

File tree

packages/cli/src/mcp.ts

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -670,18 +670,47 @@ function asFixMapReport(candidate: unknown, label: string): LoadedReport {
670670
};
671671
}
672672

673-
// The array check alone accepted `{ contextFiles: [{}] }`, and every downstream comparison
674-
// then read `undefined` paths as real ones — an agent hand-building a report, or truncating
675-
// one, got a confident diff of nothing. Each entry has to carry the field the comparison is
676-
// keyed on. `rank`, `score` and `confidence` stay optional so a trimmed report still works.
673+
// A contextFiles-only empty object is indistinguishable from a truncated report. A real
674+
// empty plan carries the full report envelope; trimmed non-empty reports remain useful as
675+
// long as every field the comparison reads has the documented type.
677676
const contextFiles = (candidate as FixMapReport).contextFiles;
678-
const invalid = contextFiles.findIndex(
679-
(file) => typeof file !== "object" || file === null || typeof (file as { path?: unknown }).path !== "string"
680-
);
677+
const record = candidate as Record<string, unknown>;
678+
if (
679+
contextFiles.length === 0 &&
680+
!(
681+
typeof record.summary === "string" &&
682+
Array.isArray(record.testRoutes) &&
683+
Array.isArray(record.risks) &&
684+
Array.isArray(record.changedFiles) &&
685+
Array.isArray(record.diagnostics)
686+
)
687+
) {
688+
return {
689+
success: false,
690+
message:
691+
`${label} has no context files and is missing the complete FixMap report envelope ` +
692+
'(summary, testRoutes, risks, changedFiles, and diagnostics).'
693+
};
694+
}
695+
696+
const invalid = contextFiles.findIndex((file) => {
697+
if (typeof file !== "object" || file === null) return true;
698+
const ranked = file as Record<string, unknown>;
699+
if (typeof ranked.path !== "string" || ranked.path.trim().length === 0) return true;
700+
if (ranked.rank !== undefined && (!Number.isSafeInteger(ranked.rank) || (ranked.rank as number) < 1)) return true;
701+
if (ranked.score !== undefined && (typeof ranked.score !== "number" || !Number.isFinite(ranked.score))) return true;
702+
if (
703+
ranked.confidence !== undefined &&
704+
ranked.confidence !== "high" && ranked.confidence !== "medium" && ranked.confidence !== "low"
705+
) return true;
706+
return false;
707+
});
681708
if (invalid !== -1) {
682709
return {
683710
success: false,
684-
message: `${label} has a contextFiles entry at index ${invalid} without a string "path"; every ranked file needs one.`
711+
message:
712+
`${label} has an invalid contextFiles entry at index ${invalid}; each entry needs a non-empty string "path", ` +
713+
'and optional rank, score, and confidence fields must use their documented types.'
685714
};
686715
}
687716

packages/cli/test/mcp.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,52 @@ describe("fixmap mcp server", () => {
173173
expect(comparison.confidenceChanged).toHaveLength(1);
174174
});
175175

176+
it("rejects a contextFiles-only empty object instead of treating it as a report", async () => {
177+
const client = await connectClient();
178+
const result = await client.callTool({
179+
name: "fixmap_compare",
180+
arguments: {
181+
previous: { contextFiles: [] },
182+
current: { contextFiles: [] },
183+
format: "json"
184+
}
185+
});
186+
187+
expect(result.isError).toBe(true);
188+
expect((result.content as Array<{ text: string }>)[0]?.text).toContain("complete FixMap report envelope");
189+
});
190+
191+
it("still accepts complete reports that legitimately have no context files", async () => {
192+
const client = await connectClient();
193+
const empty = { summary: "No matches", contextFiles: [], testRoutes: [], risks: [], changedFiles: [], diagnostics: [] };
194+
const result = await client.callTool({
195+
name: "fixmap_compare",
196+
arguments: { previous: empty, current: empty, format: "json" }
197+
});
198+
199+
expect(result.isError).not.toBe(true);
200+
expect(JSON.parse((result.content as Array<{ text: string }>)[0]!.text).unchanged).toEqual([]);
201+
});
202+
203+
it.each([
204+
[{ path: "" }, "path"],
205+
[{ path: "a.ts", rank: 0 }, "rank"],
206+
[{ path: "a.ts", score: "ten" }, "score"],
207+
[{ path: "a.ts", confidence: "certain" }, "confidence"]
208+
])("rejects malformed optional comparison fields in %j", async (entry, expectedField) => {
209+
const client = await connectClient();
210+
const result = await client.callTool({
211+
name: "fixmap_compare",
212+
arguments: {
213+
previous: { contextFiles: [entry] },
214+
current: { contextFiles: [{ path: "a.ts" }] }
215+
}
216+
});
217+
218+
expect(result.isError).toBe(true);
219+
expect((result.content as Array<{ text: string }>)[0]?.text).toContain(expectedField);
220+
});
221+
176222
it("compares report file paths through MCP like the CLI", async () => {
177223
const client = await connectClient();
178224
const directory = await mkdtemp(join(tmpdir(), "fixmap-mcp-compare-"));

0 commit comments

Comments
 (0)