diff --git a/src/gate/test-presence.test.ts b/src/gate/test-presence.test.ts index 8578a76..2dfefd6 100644 --- a/src/gate/test-presence.test.ts +++ b/src/gate/test-presence.test.ts @@ -40,6 +40,83 @@ describe("patchAddsQueryCode", () => { expect(patchAddsQueryCode(addPatch("const total = items.length + 1;"))).toBe(false); }); + test("ignores DDL keywords in a prose string literal", () => { + // The Site#3539 false positive: an MCP tool description mentioning the + // fix it returns. Prose, not a statement — no ON clause, no column list. + expect( + patchAddsQueryCode( + addPatch( + '"each with its suggested CREATE INDEX fix, cost delta, and current triage status, ranked by impact.",', + ), + ), + ).toBe(false); + }); + + test("still detects statement-shaped DDL", () => { + expect( + patchAddsQueryCode(addPatch('"CREATE INDEX plans_project_id_idx ON plans (project_id)"')), + ).toBe(true); + expect( + patchAddsQueryCode(addPatch("const ddl = `CREATE TABLE widgets (id serial primary key)`;")), + ).toBe(true); + expect(patchAddsQueryCode(addPatch('"ALTER TABLE users ADD COLUMN age int"'))).toBe(true); + expect(patchAddsQueryCode(addPatch('"DROP TABLE IF EXISTS old_stuff"'))).toBe(true); + }); + + // The DDL recall the statement-shape rewrite must hold, shape by shape. Two + // variants are newly caught: the old adjacency pattern missed + // `CREATE UNIQUE INDEX CONCURRENTLY` and `CREATE OR REPLACE VIEW` (a word + // between the keywords broke it); the optional groups here cover them. + test("detects each DDL variant, including two the old pattern missed", () => { + const ddl = [ + "CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS ix_email ON users (email);", + 'CREATE INDEX "ix quoted" ON "user events" (occurred_at DESC);', + "CREATE TABLE IF NOT EXISTS widgets (id serial PRIMARY KEY, name text);", + "CREATE OR REPLACE VIEW active_users AS SELECT 1;", + "CREATE MATERIALIZED VIEW daily_stats AS SELECT 1;", + "ALTER TABLE users ADD COLUMN age int;", + 'ALTER TABLE "users" DROP COLUMN age;', + "ALTER TABLE IF EXISTS users RENAME TO members;", + "DROP TABLE old_stuff;", + "DROP INDEX CONCURRENTLY ix_email;", + "DROP VIEW IF EXISTS active_users;", + "DROP MATERIALIZED VIEW daily_stats;", + ]; + for (const stmt of ddl) { + expect(patchAddsQueryCode(addPatch(stmt)), stmt).toBe(true); + } + }); + + test("detects multi-line DDL through the query-call shapes", () => { + // The DDL patterns are same-line; DDL wrapped across lines still registers + // via the call that runs it (`.execute(` / sql``) on its opening line. + const patch = + "@@ -1,0 +1,3 @@\n" + + "+await db.execute(`\n" + + "+ CREATE INDEX idx_users_email\n" + + "+ ON users (email)`);"; + expect(patchAddsQueryCode(patch)).toBe(true); + }); + + test("detects CREATE TABLE ... AS SELECT via the select shape", () => { + expect( + patchAddsQueryCode(addPatch("CREATE TABLE archive AS SELECT id FROM users;")), + ).toBe(true); + }); + + test("accepts the known miss: a bare multi-line DDL string with no query call", () => { + // Deliberate: the keyword line alone carries no statement grammar, and the + // gate is warn-only with an under-fire bias — a false positive costs trust, + // a miss here is covered by the capture rungs (#3502/#3503). This test pins + // the trade-off so a future change to it is a decision, not an accident. + const patch = + "@@ -1,0 +1,3 @@\n" + + "+const DDL =\n" + + "+ 'CREATE INDEX idx_users_email' +\n" + + "+ ' ON users (email)';"; + expect(patchAddsQueryCode(patch)).toBe(false); + }); + test("only looks at added lines, not removed ones", () => { const patch = "@@ -1,1 +1,1 @@\n-await db.select().from(users);\n+const x = 1;"; expect(patchAddsQueryCode(patch)).toBe(false); @@ -191,6 +268,30 @@ describe("evaluateTestPresence", () => { expect(verdict).toBeNull(); }); + test("does not fire on the tool-rename PR from Site#3539", () => { + // A rename PR rewrote a tool-registration line whose description string + // says "suggested CREATE INDEX fix" — prose, not a query. The touched spec + // files aren't data-layer tests and a rename captures no new hashes, so + // only the content heuristic can keep this PR clean. + const verdict = evaluateTestPresence( + [ + changed( + "packages/mcp-server/src/tools/index.ts", + '"Review a CI run in one call: each with its suggested CREATE INDEX fix, cost delta, and current triage status.",', + ), + changed("packages/mcp-server/src/server.spec.ts", '"review_ci_run",'), + changed( + "packages/mcp-server/src/tools/ci/review-ci-run.spec.ts", + 'const result = await runReviewCiRun({ runId: "run_1" }, client);', + "renamed", + ), + ], + undefined, + { newQueryHashes: [] }, + ); + expect(verdict).toBeNull(); + }); + test("passes when the PR changes no query code", () => { const verdict = evaluateTestPresence([ changed("apps/app/src/components/Button.tsx", "const label = props.label;"), diff --git a/src/gate/test-presence.ts b/src/gate/test-presence.ts index f695a8a..fdfd237 100644 --- a/src/gate/test-presence.ts +++ b/src/gate/test-presence.ts @@ -77,7 +77,17 @@ export const DEFAULT_TEST_PRESENCE_CONFIG: TestPresenceConfig = { /\bdelete\s+from\b/i, /\bupdate\b[^\n]{0,80}\bset\b/i, /\bselect\b[\s\S]{0,300}?\bfrom\b/i, - /\b(create|alter|drop)\s+(table|index|view|materialized\s+view)\b/i, + // DDL is matched by statement shape (ON clause, column list, target + // identifier), not bare keyword adjacency: prose in a string literal — + // "its suggested CREATE INDEX fix" in an MCP tool description (Site#3539) + // — must not read as a query. Stripping string literals instead would + // blind the raw-SQL shapes above, since raw SQL lives in strings; the + // statement's own grammar is the discriminator. + /\bcreate\s+(unique\s+)?index\b[^\n]{0,120}?\bon\b/i, + /\bcreate\s+table\b[^\n]{0,80}?\(/i, + /\bcreate\s+(or\s+replace\s+)?(materialized\s+)?view\b[^\n]{0,80}?\bas\b/i, + /\balter\s+table\s+(if\s+exists\s+)?["\w]/i, + /\bdrop\s+(table|index|view|materialized\s+view)\s+(if\s+exists\s+|concurrently\s+)?["\w]/i, ], dataAccessPathPatterns: [ /(^|\/)[^/]*repositor(y|ies)[^/]*\.[cm]?[jt]s$/i,