From d18e84a1ebde91be7a9efb0d6986a930f7fbf09d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Beteg=C3=B3n?= Date: Tue, 22 Sep 2026 16:28:23 +0200 Subject: [PATCH 1/2] fix(search): warn once with the final rewritten query sanitizeQuery used to log Running query: after the numeric project rewrite, then again after OR/AND. Combined input like project:123 OR project:456 warned with an intermediate that never ran. Collect notes and emit a single warning after every rewrite succeeds. Co-authored-by: Cursor --- packages/cli/src/lib/search-query.ts | 66 +++++++++------ packages/cli/test/lib/search-query.test.ts | 6 ++ .../cli/test/lib/search-query.warn.test.ts | 81 +++++++++++++++++++ 3 files changed, 130 insertions(+), 23 deletions(-) create mode 100644 packages/cli/test/lib/search-query.warn.test.ts diff --git a/packages/cli/src/lib/search-query.ts b/packages/cli/src/lib/search-query.ts index 9ef6ea85c..63e777071 100644 --- a/packages/cli/src/lib/search-query.ts +++ b/packages/cli/src/lib/search-query.ts @@ -362,19 +362,7 @@ export function sanitizeQuery(query: string | undefined): string | undefined { return withNumericProject; } - if (withNumericProject !== query) { - const notes: string[] = []; - if (normalized !== query) { - notes.push("Auto-repaired search query syntax."); - } - if (withNumericProject !== normalized) { - notes.push( - "`project` is the slug; numeric ids use project_id. Rewrote numeric project: filters." - ); - } - notes.push(`Running query: "${withNumericProject}"`); - log.warn(notes.join(" ")); - } + const notes = preParseRewriteNotes(query, normalized, withNumericProject); // Check for OR inside paren groups first — these are opaque and can't // be rewritten. Must throw even if top-level OR would be rewritable, @@ -395,37 +383,69 @@ export function sanitizeQuery(query: string | undefined): string | undefined { if (hasOr) { // Strip AND nodes before OR rewrite const withoutAnd = hasAnd ? stripAndNodes(nodes) : nodes; - return handleOr(withoutAnd, hasAnd); + const result = handleOr(withoutAnd, hasAnd, notes); + warnRunningQuery(notes, result); + return result; } if (hasAnd) { const sanitized = serializeNodes(stripAndNodes(nodes)); - log.warn( - "Sentry search implicitly ANDs terms — removed explicit AND operator. " + - `Running query: "${sanitized}"` + notes.push( + "Sentry search implicitly ANDs terms — removed explicit AND operator." ); + warnRunningQuery(notes, sanitized); return sanitized; } + warnRunningQuery(notes, withNumericProject); return withNumericProject; } +/** Notes from text-layer rewrites that run before PEG parse. */ +function preParseRewriteNotes( + query: string, + normalized: string, + withNumericProject: string +): string[] { + const notes: string[] = []; + if (normalized !== query) { + notes.push("Auto-repaired search query syntax."); + } + if (withNumericProject !== normalized) { + notes.push( + "`project` is the slug; numeric ids use project_id. Rewrote numeric project: filters." + ); + } + return notes; +} + +/** + * One warning after every successful rewrite, quoting the query that + * will actually be sent. Skip if nothing changed. + */ +function warnRunningQuery(notes: string[], result: string): void { + if (notes.length === 0) { + return; + } + log.warn(`${notes.join(" ")} Running query: "${result}"`); +} + /** * Handle the OR rewrite path — extracted to keep `sanitizeQuery` under * the cognitive complexity limit. */ -function handleOr(nodes: SearchNode[], hasAnd: boolean): string { +function handleOr( + nodes: SearchNode[], + hasAnd: boolean, + notes: string[] +): string { const rewritten = tryRewriteOr(nodes); if (rewritten) { - const result = serializeNodes(rewritten); - const notes: string[] = []; notes.push("Rewrote OR using in-list syntax: key:[val1,val2]."); if (hasAnd) { notes.push("Also removed explicit AND (implicit in Sentry search)."); } - notes.push(`Running query: "${result}"`); - log.warn(notes.join(" ")); - return result; + return serializeNodes(rewritten); } throw new ValidationError( diff --git a/packages/cli/test/lib/search-query.test.ts b/packages/cli/test/lib/search-query.test.ts index e27d3bf5c..6bf9d7abf 100644 --- a/packages/cli/test/lib/search-query.test.ts +++ b/packages/cli/test/lib/search-query.test.ts @@ -153,6 +153,12 @@ describe("sanitizeQuery: numeric project:", () => { "project:[frontend,6442225]" ); }); + + test("rewrites numeric project: then OR in one step", () => { + expect(sanitizeQuery("project:123 OR project:456")).toBe( + "project_id:[123,456]" + ); + }); }); // --------------------------------------------------------------------------- diff --git a/packages/cli/test/lib/search-query.warn.test.ts b/packages/cli/test/lib/search-query.warn.test.ts new file mode 100644 index 000000000..c0519618a --- /dev/null +++ b/packages/cli/test/lib/search-query.warn.test.ts @@ -0,0 +1,81 @@ +/** + * Warning copy for stacked search-query rewrites. + * + * `sanitizeQuery` must emit one `Running query:` line, quoting the + * string that is actually sent — not an intermediate rewrite. + */ + +import { beforeEach, describe, expect, test, vi } from "vitest"; + +const { fakeLog } = vi.hoisted(() => { + const log = { + warn: vi.fn(), + debug: vi.fn(), + info: vi.fn(), + error: vi.fn(), + withTag() { + return log; + }, + }; + return { fakeLog: log }; +}); + +vi.mock("../../src/lib/logger.js", () => ({ + logger: fakeLog, +})); + +const { sanitizeQuery } = await import("../../src/lib/search-query.js"); + +function runningQueries(): string[] { + return fakeLog.warn.mock.calls + .map((call) => String(call[0])) + .filter((msg) => msg.includes("Running query:")); +} + +describe("sanitizeQuery: rewrite warnings", () => { + beforeEach(() => { + fakeLog.warn.mockClear(); + }); + + test("numeric project: plus OR warns once with the final in-list", () => { + expect(sanitizeQuery("project:123 OR project:456")).toBe( + "project_id:[123,456]" + ); + const warns = runningQueries(); + expect(warns).toHaveLength(1); + expect(warns[0]).toContain('Running query: "project_id:[123,456]"'); + expect(warns[0]).not.toContain("project_id:123 OR project_id:456"); + expect(warns[0]).toContain("Rewrote numeric project:"); + expect(warns[0]).toContain("Rewrote OR using in-list syntax"); + }); + + test("numeric project: plus AND warns once with the stripped query", () => { + expect(sanitizeQuery("project:123 AND is:unresolved")).toBe( + "project_id:123 is:unresolved" + ); + const warns = runningQueries(); + expect(warns).toHaveLength(1); + expect(warns[0]).toContain('Running query: "project_id:123 is:unresolved"'); + expect(warns[0]).toContain("removed explicit AND operator"); + expect(warns[0]).not.toContain('Running query: "project:123 AND'); + }); + + test("OR-only still warns once with the in-list", () => { + expect(sanitizeQuery("level:error OR level:warning")).toBe( + "level:[error,warning]" + ); + const warns = runningQueries(); + expect(warns).toHaveLength(1); + expect(warns[0]).toContain('Running query: "level:[error,warning]"'); + }); + + test("does not warn Running query: when OR rewrite fails", () => { + expect(() => sanitizeQuery("level:error OR assigned:me")).toThrow(); + expect(runningQueries()).toHaveLength(0); + }); + + test("does not warn Running query: when numeric rewrite is followed by a failed OR", () => { + expect(() => sanitizeQuery("project:123 OR assigned:me")).toThrow(); + expect(runningQueries()).toHaveLength(0); + }); +}); From 39fcfc7abb4722dcecbc191871c1ce4ae6f98f0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Beteg=C3=B3n?= Date: Tue, 22 Sep 2026 18:09:36 +0200 Subject: [PATCH 2/2] fix(search): put Running query: on its own warning line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rewrite reasons and the query that actually runs are different things to scan. Keep one warn() so consola still prints a single ⚠, but break Running query: onto the next line. Co-authored-by: Cursor --- packages/cli/src/lib/search-query.ts | 7 +++--- .../cli/test/lib/search-query.warn.test.ts | 23 +++++++++++-------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/lib/search-query.ts b/packages/cli/src/lib/search-query.ts index 63e777071..2813f5a43 100644 --- a/packages/cli/src/lib/search-query.ts +++ b/packages/cli/src/lib/search-query.ts @@ -420,14 +420,15 @@ function preParseRewriteNotes( } /** - * One warning after every successful rewrite, quoting the query that - * will actually be sent. Skip if nothing changed. + * One warning after every successful rewrite. Reasons on the first + * line; the query that will actually be sent on the second. Skip if + * nothing changed. */ function warnRunningQuery(notes: string[], result: string): void { if (notes.length === 0) { return; } - log.warn(`${notes.join(" ")} Running query: "${result}"`); + log.warn(`${notes.join(" ")}\nRunning query: "${result}"`); } /** diff --git a/packages/cli/test/lib/search-query.warn.test.ts b/packages/cli/test/lib/search-query.warn.test.ts index c0519618a..26f49cadf 100644 --- a/packages/cli/test/lib/search-query.warn.test.ts +++ b/packages/cli/test/lib/search-query.warn.test.ts @@ -1,8 +1,8 @@ /** * Warning copy for stacked search-query rewrites. * - * `sanitizeQuery` must emit one `Running query:` line, quoting the - * string that is actually sent — not an intermediate rewrite. + * `sanitizeQuery` must emit one warn: reasons, then a newline, then + * `Running query:` quoting the string that is actually sent. */ import { beforeEach, describe, expect, test, vi } from "vitest"; @@ -43,10 +43,11 @@ describe("sanitizeQuery: rewrite warnings", () => { ); const warns = runningQueries(); expect(warns).toHaveLength(1); - expect(warns[0]).toContain('Running query: "project_id:[123,456]"'); + expect(warns[0].split("\n")).toEqual([ + "`project` is the slug; numeric ids use project_id. Rewrote numeric project: filters. Rewrote OR using in-list syntax: key:[val1,val2].", + 'Running query: "project_id:[123,456]"', + ]); expect(warns[0]).not.toContain("project_id:123 OR project_id:456"); - expect(warns[0]).toContain("Rewrote numeric project:"); - expect(warns[0]).toContain("Rewrote OR using in-list syntax"); }); test("numeric project: plus AND warns once with the stripped query", () => { @@ -55,9 +56,10 @@ describe("sanitizeQuery: rewrite warnings", () => { ); const warns = runningQueries(); expect(warns).toHaveLength(1); - expect(warns[0]).toContain('Running query: "project_id:123 is:unresolved"'); - expect(warns[0]).toContain("removed explicit AND operator"); - expect(warns[0]).not.toContain('Running query: "project:123 AND'); + expect(warns[0].split("\n")).toEqual([ + "`project` is the slug; numeric ids use project_id. Rewrote numeric project: filters. Sentry search implicitly ANDs terms — removed explicit AND operator.", + 'Running query: "project_id:123 is:unresolved"', + ]); }); test("OR-only still warns once with the in-list", () => { @@ -66,7 +68,10 @@ describe("sanitizeQuery: rewrite warnings", () => { ); const warns = runningQueries(); expect(warns).toHaveLength(1); - expect(warns[0]).toContain('Running query: "level:[error,warning]"'); + expect(warns[0].split("\n")).toEqual([ + "Rewrote OR using in-list syntax: key:[val1,val2].", + 'Running query: "level:[error,warning]"', + ]); }); test("does not warn Running query: when OR rewrite fails", () => {