Skip to content

Commit 2ef93c0

Browse files
fix(mcp): honor free-text tests evidence in loopover_check_test_evidence (#6618) (#6657)
`loopover_check_test_evidence` was documented as modeled on `checkSlopRisk`, but its shape only had `changedPaths`/`testFiles` and its handler called `classifyTestCoverage(allPaths)` directly, never consulting `hasLocalTestEvidence`. So a caller whose only evidence is free-text (e.g. "ran `go test ./...` locally, no new file") got an "absent" verdict from this tool, even though `loopover_check_slop_risk` and `loopover_suggest_boundary_tests` correctly credit the exact same evidence via the shared `hasLocalTestEvidence` helper. - Add an optional `tests` field to `checkTestEvidenceShape`, same bounds as the sibling shapes (`z.array(z.string().max(400)).max(2000).optional()`). - Import `hasLocalTestEvidence` and, in the handler, override an otherwise- "absent" classification to "adequate" (with testFileCount >= 1) only when `hasLocalTestEvidence({ tests, testFiles })` is true, plus a distinct guidance line noting the evidence came from the free-text field. - The override applies ONLY above "absent": weak/adequate/strong path-based classifications are returned unchanged, so the tool never becomes more lenient than the path signal once real test-file evidence exists. Adds three test cases to test/unit/mcp-check-test-evidence.test.ts: free-text-only evidence lifts absent→adequate with distinct guidance; an empty `tests: []` stays absent; and a weak path classification stays weak (override does not fire above absent). Closes #6618
1 parent 8b20ad6 commit 2ef93c0

2 files changed

Lines changed: 56 additions & 3 deletions

File tree

src/mcp/server.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ import {
155155
buildTestGenSpec,
156156
type LocalWriteActionSpec,
157157
} from "./local-write-tools";
158-
import { classifyTestCoverage, isCodeFile, isTestPath, TEST_FRAMEWORKS } from "../signals/test-evidence";
158+
import { classifyTestCoverage, hasLocalTestEvidence, isCodeFile, isTestPath, TEST_FRAMEWORKS } from "../signals/test-evidence";
159159
import { applyStepResult, buildPlanDag, nextReadySteps, planProgress, validatePlanDag, type PlanDag } from "../services/plan-dag";
160160
import { buildFocusManifestValidation } from "../services/focus-manifest-validation";
161161
import { isGlobalAgentPause, resolveAgentActionMode, resolveAgentPermissionReadiness } from "../settings/agent-execution";
@@ -1182,6 +1182,7 @@ const checkImprovementPotentialOutputSchema = {
11821182
const checkTestEvidenceShape = {
11831183
changedPaths: z.array(z.string().min(1).max(400)).max(2000),
11841184
testFiles: z.array(z.string().min(1).max(400)).max(2000).optional(),
1185+
tests: z.array(z.string().max(400)).max(2000).optional(),
11851186
};
11861187

11871188
const checkTestEvidenceOutputSchema = {
@@ -3640,12 +3641,24 @@ export class LoopoverMcp {
36403641
private async checkTestEvidence(input: z.infer<z.ZodObject<typeof checkTestEvidenceShape>>): Promise<ToolPayload> {
36413642
await this.enforceToolRateLimit("loopover_check_test_evidence");
36423643
const allPaths = [...input.changedPaths, ...(input.testFiles ?? [])];
3643-
const classification = classifyTestCoverage(allPaths);
36443644
const codeFileCount = input.changedPaths.filter(isCodeFile).length;
3645-
const testFileCount = allPaths.filter(isTestPath).length;
3645+
let classification = classifyTestCoverage(allPaths);
3646+
let testFileCount = allPaths.filter(isTestPath).length;
3647+
// Credit free-text `tests` evidence (e.g. "ran `go test ./...` locally, no new file") the same way the
3648+
// sibling tools loopover_check_slop_risk / loopover_suggest_boundary_tests already do via
3649+
// hasLocalTestEvidence. Only ever LIFT an otherwise-"absent" verdict -- never make this more lenient than
3650+
// the path-based signal once real test-file evidence (weak/adequate/strong) already exists.
3651+
const creditedByFreeTextTests =
3652+
classification === "absent" && hasLocalTestEvidence({ tests: input.tests, testFiles: input.testFiles });
3653+
if (creditedByFreeTextTests) {
3654+
classification = "adequate";
3655+
testFileCount = Math.max(testFileCount, 1);
3656+
}
36463657
const guidance: string[] = [];
36473658
if (codeFileCount === 0) {
36483659
guidance.push("No hand-authored code files changed, so the missing-test-evidence signal does not apply (e.g. a docs- or config-only change).");
3660+
} else if (creditedByFreeTextTests) {
3661+
guidance.push("No test file was detected among the changed paths, but the free-text `tests` evidence you supplied is credited as test evidence (the same way check_slop_risk and suggest_boundary_tests treat it).");
36493662
} else if (classification === "absent") {
36503663
guidance.push("Changed code files carry no test evidence — add or update a test that exercises the change before opening the PR.");
36513664
} else if (classification === "strong") {

test/unit/mcp-check-test-evidence.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,44 @@ describe("MCP loopover_check_test_evidence (#2235)", () => {
6363
expect(data.codeFileCount).toBe(0);
6464
expect(data.guidance.join(" ")).toMatch(/does not apply/i);
6565
});
66+
67+
it("credits free-text tests evidence when no test file is present, lifting absent to adequate (#6618)", async () => {
68+
const client = await connect();
69+
const result = await client.callTool({
70+
name: "loopover_check_test_evidence",
71+
arguments: { changedPaths: ["src/a.ts", "src/b.ts"], tests: ["ran `go test ./internal/entity` locally, no new file"] },
72+
});
73+
const data = result.structuredContent as Result;
74+
expect(data.classification).toBe("adequate"); // lifted from the path-based "absent"
75+
expect(data.testFileCount).toBeGreaterThanOrEqual(1);
76+
expect(data.guidance.join(" ")).toMatch(/free-text `tests`/i); // distinct wording, not the path-derived lines
77+
expect(data.guidance.join(" ")).not.toMatch(/looks strong/i);
78+
});
79+
80+
it("does not credit an empty tests array — classification stays absent (#6618)", async () => {
81+
const client = await connect();
82+
const result = await client.callTool({
83+
name: "loopover_check_test_evidence",
84+
arguments: { changedPaths: ["src/a.ts", "src/b.ts"], tests: [] },
85+
});
86+
const data = result.structuredContent as Result;
87+
expect(data.classification).toBe("absent");
88+
expect(data.testFileCount).toBe(0);
89+
expect(data.guidance.join(" ")).toMatch(/no test evidence/i);
90+
});
91+
92+
it("does not apply the override above absent — a weak path classification stays weak (#6618)", async () => {
93+
const client = await connect();
94+
const result = await client.callTool({
95+
name: "loopover_check_test_evidence",
96+
arguments: {
97+
changedPaths: ["src/a.ts", "src/b.ts", "src/c.ts", "src/d.ts", "src/e.ts"],
98+
testFiles: ["test/a.test.ts"],
99+
tests: ["ran the full suite locally"],
100+
},
101+
});
102+
const data = result.structuredContent as Result;
103+
expect(data.classification).toBe("weak"); // real path evidence already present → override must not fire
104+
expect(data.guidance.join(" ")).toMatch(/weak/i);
105+
});
66106
});

0 commit comments

Comments
 (0)