Skip to content

Commit fe4cef3

Browse files
cliffhallclaude
andcommitted
fix(interface-diff): never pass a report without a verdict (#4860)
Copilot round 5 on #4975: - A result with no boolean hasDifferences is an error, not "unchanged". - On a probe failure, report the CLI's contextual message ("Base server probe failed" / "Target probe failed") rather than the bare exception. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: cliffhall <[email protected]>
1 parent b15cf21 commit fe4cef3

2 files changed

Lines changed: 17 additions & 4 deletions

File tree

‎scripts/interface-diff.mjs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,19 @@ export function classify(stdout, stderr = "") {
131131
diffs: [],
132132
};
133133
const diffs = Array.isArray(result.diffs) ? result.diffs : [];
134-
if (result.error)
135-
return { status: "error", error: String(result.error), diffs };
134+
if (result.error) {
135+
// `error` is the bare exception; the entry in `diffs` says which build
136+
// failed ("Base server probe failed: …" / "Target probe failed: …").
137+
const context = diffs.find((d) => d.endpoint === "error")?.diff;
138+
return { status: "error", error: String(context ?? result.error), diffs };
139+
}
140+
// A report without a verdict is not a pass.
141+
if (typeof result.hasDifferences !== "boolean")
142+
return {
143+
status: "error",
144+
error: "mcp-server-diff's report has no hasDifferences verdict",
145+
diffs,
146+
};
136147
return {
137148
status: result.hasDifferences ? "changed" : "unchanged",
138149
diffs,

‎scripts/interface-diff.test.mjs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,19 +77,21 @@ test("a probe failure is an error, though the CLI reports it as a difference", (
7777
const v = classify(
7878
cliJson({
7979
hasDifferences: true,
80-
error: "Target probe failed: boom",
80+
error: "boom",
8181
diffs: [{ endpoint: "error", diff: "Target probe failed: boom" }],
8282
}),
8383
);
8484
assert.equal(v.status, "error");
85-
assert.match(v.error, /boom/);
85+
// The context (which build failed) comes from the diffs entry.
86+
assert.equal(v.error, "Target probe failed: boom");
8687
});
8788

8889
test("output that is not the JSON report is an error, never a pass", () => {
8990
assert.equal(classify("").status, "error");
9091
assert.equal(classify("Fatal error: x").status, "error");
9192
assert.equal(classify("{}").status, "error");
9293
assert.equal(classify(JSON.stringify({ results: [] })).status, "error");
94+
assert.equal(classify(JSON.stringify({ results: [{}] })).status, "error");
9395
});
9496

9597
test("a CLI crash before the report carries its stderr as the reason", () => {

0 commit comments

Comments
 (0)