diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 4645910093..ac79098d01 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -125,7 +125,7 @@ import { MAX_FOCUS_MANIFEST_BYTES } from "../signals/focus-manifest"; import { loadPublicRepoFocusManifest, loadRepoFocusManifest } from "../signals/focus-manifest-loader"; import { buildPredictedGateVerdict } from "../rules/predicted-gate"; import { buildIssueSlopAssessment, buildSlopAssessment } from "../signals/slop"; -import { buildBoundaryTestGenerationFinding, buildBoundaryTestGenerationSpec, detectBoundaryTouches } from "../signals/boundary-test-generation"; +import { buildBoundaryTestGenerationFinding, buildBoundaryTestGenerationSpec } from "../signals/boundary-test-generation"; import { buildRepoDataQuality } from "../signals/data-quality"; import { PREFLIGHT_LIMITS } from "../signals/preflight-limits"; import { SCENARIO_MAX_BRANCH_REF_CHARS, SCENARIO_MAX_LINKED_ISSUE_NUMBERS, SCENARIO_MAX_REPO_FULL_NAME_CHARS } from "../scenarios/input-model"; @@ -791,13 +791,22 @@ const checkIssueSlopShape = { const checkIssueSlopOutputSchema = checkSlopRiskOutputSchema; // Boundary-safe test-generation suggestion (#1972): pure local-metadata, like checkSlopRisk — the agent -// supplies changed-file paths + patch text (never full file content) plus any test evidence it already has. -// Advisory-only; this tool never blocks or writes anything — it only returns criteria/hints for the caller's -// OWN agent to scaffold tests from (mirrors the local-write-tools.ts no-cloud-write boundary). +// supplies changed-file paths plus precomputed boundary-touch metadata from its local diff scan. The remote MCP +// boundary never accepts patch/source text. Advisory-only; this tool never blocks or writes anything — it only +// returns criteria/hints for the caller's OWN agent to scaffold tests from. const suggestBoundaryTestsShape = { - changedFiles: z - .array(z.object({ path: z.string().min(1).max(400), patch: z.string().max(20000).optional() })) - .max(500), + changedFiles: z.array(z.object({ path: z.string().min(1).max(400) }).strict()).max(500), + boundaryTouches: z + .array( + z + .object({ + path: z.string().min(1).max(400), + kind: z.enum(["array_index_bounds", "null_or_undefined_branch", "empty_collection_check"]), + }) + .strict(), + ) + .max(20) + .optional(), tests: z.array(z.string().max(400)).max(2000).optional(), testFiles: z.array(z.string().max(400)).max(2000).optional(), }; @@ -1277,7 +1286,7 @@ export class GittensoryMcp { "gittensory_suggest_boundary_tests", { description: - "Boundary-safe test-generation suggestion (#1972): scan changed-file patches for a small, precise set of boundary-condition patterns (off-by-one array/index bounds, null/undefined branches, empty-collection checks) with no test evidence in the diff, and return a LOCAL-execution action spec (criteria/hints only — never generated test code) for your OWN agent to scaffold tests with. Advisory-only; never blocks, never writes.", + "Boundary-safe test-generation suggestion (#1972): evaluate locally precomputed boundary-touch metadata (path + pattern kind only; no patch/source text) with no test evidence in the diff, and return a LOCAL-execution action spec (criteria/hints only — never generated test code) for your OWN agent to scaffold tests with. Advisory-only; never blocks, never writes.", inputSchema: suggestBoundaryTestsShape, outputSchema: suggestBoundaryTestsOutputSchema, }, @@ -2294,8 +2303,9 @@ export class GittensoryMcp { } private suggestBoundaryTests(input: z.infer>): ToolPayload { - const touches = detectBoundaryTouches(input.changedFiles); - const finding = buildBoundaryTestGenerationFinding({ files: input.changedFiles, tests: input.tests, testFiles: input.testFiles }); + const changedPaths = new Set(input.changedFiles.map((file) => file.path)); + const touches = (input.boundaryTouches ?? []).filter((touch) => changedPaths.has(touch.path)); + const finding = buildBoundaryTestGenerationFinding({ touches, tests: input.tests, testFiles: input.testFiles }); const spec = finding ? buildBoundaryTestGenerationSpec(touches) : null; return { summary: finding ? "Boundary-condition code changed without test evidence." : "No boundary-condition gap detected.", diff --git a/src/signals/boundary-test-generation.ts b/src/signals/boundary-test-generation.ts index 18290bddee..9958d92b35 100644 --- a/src/signals/boundary-test-generation.ts +++ b/src/signals/boundary-test-generation.ts @@ -26,8 +26,6 @@ export type BoundaryPatternKind = "array_index_bounds" | "null_or_undefined_bran export type BoundaryTouch = { path: string; kind: BoundaryPatternKind; - /** The matched added line, trimmed, capped for display (never the full patch). */ - snippet: string; }; // Kept deliberately SMALL and PRECISE (per #1972's scope note: false positives are worse than a narrow @@ -44,7 +42,6 @@ const BOUNDARY_PATTERNS: ReadonlyArray<{ kind: BoundaryPatternKind; pattern: Reg { kind: "empty_collection_check", pattern: EMPTY_COLLECTION_CHECK_PATTERN }, ]; -const MAX_SNIPPET_LENGTH = 160; const MAX_TOUCHES = 20; /** Added-line prefix in a unified diff: a single leading `+` not followed by another `+` (which would be the @@ -71,7 +68,7 @@ export function detectBoundaryTouches(files: BoundaryPatchInput[]): BoundaryTouc for (const line of addedLines(patch)) { for (const { kind, pattern } of BOUNDARY_PATTERNS) { if (!pattern.test(line)) continue; - touches.push({ path: file.path, kind, snippet: line.slice(0, MAX_SNIPPET_LENGTH) }); + touches.push({ path: file.path, kind }); if (touches.length >= MAX_TOUCHES) return touches; break; // one match per line is enough signal; avoid double-counting the same line across patterns } @@ -96,11 +93,12 @@ const PATTERN_LABELS: Record = { * or evidence-covered repo sees byte-identical behavior. */ export function buildBoundaryTestGenerationFinding(input: { - files: BoundaryPatchInput[]; + files?: BoundaryPatchInput[] | undefined; + touches?: BoundaryTouch[] | undefined; tests?: string[] | undefined; testFiles?: string[] | undefined; }): AdvisoryFinding | null { - const touches = detectBoundaryTouches(input.files); + const touches = input.touches ?? detectBoundaryTouches(input.files ?? []); if (touches.length === 0) return null; if (hasLocalTestEvidence({ tests: input.tests, testFiles: input.testFiles })) return null; @@ -120,8 +118,7 @@ export function buildBoundaryTestGenerationFinding(input: { export type BoundaryTestGenerationSpec = { action: "scaffold_boundary_tests"; description: string; - /** The boundary touches this spec was generated from — criteria only, no source content beyond the already- - * public per-line snippet the diff itself carries. */ + /** The boundary touches this spec was generated from — path + pattern kind only, never source text. */ touches: BoundaryTouch[]; /** Natural-language hints the contributor's own agent uses to scaffold tests in the repo's own framework and * conventions — content supplied by gittensory, execution stays on the contributor's machine. */ diff --git a/test/unit/boundary-test-generation.test.ts b/test/unit/boundary-test-generation.test.ts index f7d6bc3b86..eda286d219 100644 --- a/test/unit/boundary-test-generation.test.ts +++ b/test/unit/boundary-test-generation.test.ts @@ -83,10 +83,10 @@ describe("detectBoundaryTouches", () => { expect(touches.length).toBe(20); }); - it("truncates an oversized snippet to the display cap", () => { - const longLine = `+if (x === null) { ${"a".repeat(300)} }`; - const touches = detectBoundaryTouches([{ path: "src/long.ts", patch: longLine }]); - expect(touches[0]?.snippet.length).toBeLessThanOrEqual(160); + it("returns only path and kind metadata, never matched source snippets", () => { + const touches = detectBoundaryTouches([{ path: "src/long.ts", patch: "+if (x === null) return secretValue;" }]); + expect(touches[0]).toEqual({ path: "src/long.ts", kind: "null_or_undefined_branch" }); + expect(touches[0] as Record).not.toHaveProperty("snippet"); }); it("scans multiple files and aggregates touches across them", () => { @@ -150,6 +150,14 @@ describe("buildBoundaryTestGenerationFinding", () => { expect(finding?.detail).toContain("…"); }); + it("builds from precomputed source-free touches when supplied", () => { + const finding = buildBoundaryTestGenerationFinding({ + touches: [{ path: "src/list.ts", kind: "array_index_bounds" }], + }); + expect(finding?.detail).toContain("array/index bounds"); + expect(finding?.detail).toContain("src/list.ts"); + }); + it("handles undefined tests/testFiles (absent input) the same as an empty list", () => { const finding = buildBoundaryTestGenerationFinding({ files: [{ path: "src/list.ts", patch: "+if (items.length === 0) return null;\n" }], @@ -167,8 +175,8 @@ describe("buildBoundaryTestGenerationSpec", () => { it("builds a scaffold-tests spec with one hint per distinct pattern kind", () => { const spec = buildBoundaryTestGenerationSpec([ - { path: "src/list.ts", kind: "array_index_bounds", snippet: "list[list.length - 1]" }, - { path: "src/user.ts", kind: "null_or_undefined_branch", snippet: "user === null" }, + { path: "src/list.ts", kind: "array_index_bounds" }, + { path: "src/user.ts", kind: "null_or_undefined_branch" }, ]); expect(spec).not.toBeNull(); expect(spec?.action).toBe("scaffold_boundary_tests"); @@ -179,8 +187,8 @@ describe("buildBoundaryTestGenerationSpec", () => { it("deduplicates hints when multiple touches share the same pattern kind", () => { const spec = buildBoundaryTestGenerationSpec([ - { path: "src/a.ts", kind: "empty_collection_check", snippet: "a.length === 0" }, - { path: "src/b.ts", kind: "empty_collection_check", snippet: "b.length === 0" }, + { path: "src/a.ts", kind: "empty_collection_check" }, + { path: "src/b.ts", kind: "empty_collection_check" }, ]); expect(spec?.hints).toHaveLength(1); expect(spec?.touches).toHaveLength(2); diff --git a/test/unit/mcp-suggest-boundary-tests.test.ts b/test/unit/mcp-suggest-boundary-tests.test.ts index de83568ccb..bcd78c423e 100644 --- a/test/unit/mcp-suggest-boundary-tests.test.ts +++ b/test/unit/mcp-suggest-boundary-tests.test.ts @@ -19,13 +19,15 @@ describe("MCP gittensory_suggest_boundary_tests (#1972)", () => { const result = await client.callTool({ name: "gittensory_suggest_boundary_tests", arguments: { - changedFiles: [{ path: "src/list.ts", patch: "+if (items.length === 0) return null;\n" }], + changedFiles: [{ path: "src/list.ts" }], + boundaryTouches: [{ path: "src/list.ts", kind: "empty_collection_check" }], }, }); expect(result.isError).toBeFalsy(); - const data = result.structuredContent as { finding: { code: string } | null; spec: { action: string } | null }; + const data = result.structuredContent as { finding: { code: string } | null; spec: { action: string; touches: unknown[] } | null }; expect(data.finding?.code).toBe("boundary_test_generation_available"); expect(data.spec?.action).toBe("scaffold_boundary_tests"); + expect(data.spec?.touches).toEqual([{ path: "src/list.ts", kind: "empty_collection_check" }]); expect(JSON.stringify(data)).not.toMatch(/wallet|hotkey|reward|payout|trust score/i); }); @@ -34,7 +36,8 @@ describe("MCP gittensory_suggest_boundary_tests (#1972)", () => { const result = await client.callTool({ name: "gittensory_suggest_boundary_tests", arguments: { - changedFiles: [{ path: "src/list.ts", patch: "+if (items.length === 0) return null;\n" }], + changedFiles: [{ path: "src/list.ts" }], + boundaryTouches: [{ path: "src/list.ts", kind: "empty_collection_check" }], testFiles: ["test/unit/list.test.ts"], }, }); @@ -49,11 +52,37 @@ describe("MCP gittensory_suggest_boundary_tests (#1972)", () => { const result = await client.callTool({ name: "gittensory_suggest_boundary_tests", arguments: { - changedFiles: [{ path: "src/util.ts", patch: "+export const greeting = 'hello';\n" }], + changedFiles: [{ path: "src/util.ts" }], }, }); expect(result.isError).toBeFalsy(); const data = result.structuredContent as { finding: unknown }; expect(data.finding).toBeNull(); }); + + it("rejects raw patch input at the MCP boundary", async () => { + const client = await connect(); + const result = await client.callTool({ + name: "gittensory_suggest_boundary_tests", + arguments: { + changedFiles: [{ path: "src/list.ts", patch: "+if (items.length === 0) return null;\n" }], + }, + }); + expect(result.isError).toBe(true); + }); + + it("ignores boundary touches for files not listed in changedFiles", async () => { + const client = await connect(); + const result = await client.callTool({ + name: "gittensory_suggest_boundary_tests", + arguments: { + changedFiles: [{ path: "src/list.ts" }], + boundaryTouches: [{ path: "src/other.ts", kind: "empty_collection_check" }], + }, + }); + expect(result.isError).toBeFalsy(); + const data = result.structuredContent as { finding: unknown; spec: unknown }; + expect(data.finding).toBeNull(); + expect(data.spec).toBeNull(); + }); });