Skip to content

fix(search): rewrite numeric project: filters to project_id #1621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
betegon wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(search): rewrite numeric project: filters to project_id #1621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9af240d
fix(search): rewrite numeric project: filters to project_id
betegon Sep 22, 2026
7fab5dc
fix(search): warn once with the final rewritten query (#1624)
betegon Sep 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 78 additions & 18 deletions packages/cli/src/lib/search-query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
* - **OR**: Attempted rewrite to in-list syntax (`key:[val1,val2]`)
* when all OR operands share the same qualifier key. Throws a
* {@link ValidationError} when the rewrite is not possible.
* - **`project:<digits>`**: `project` is the slug. Numeric ids belong on
* `project_id`. Agents often paste `project:4511…` (CLI-FA). Rewritten
* with a warning. Slugs, `project_id:…`, and namespaced keys
* (`bolt.project_id`) are left alone.
*
* Parsing uses a pre-compiled PEG parser generated from
* `script/search-query.pegjs` (a simplified version of Sentry's
Expand Down Expand Up @@ -346,22 +350,19 @@ export function sanitizeQuery(query: string | undefined): string | undefined {
// These fix common patterns that agents/users produce, regardless of
// whether the PEG parser would accept them.
const normalized = normalizeQuery(query);
const withNumericProject = rewriteNumericProjectFilters(normalized);

let nodes: SearchNode[];
// biome-ignore lint/plugin: grandfathered silent catch — see #1531; drain by adding log.debug()/log.warn() or re-throwing.
try {
nodes = parse(normalized);
nodes = parse(withNumericProject);
} catch {
// PEG parse still failed after normalization — pass through to the
// API which returns a proper 400 with actionable details.
return normalized;
return withNumericProject;
}

if (normalized !== query) {
log.warn(
`Auto-repaired search query syntax. Running query: "${normalized}"`
);
}
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,
Expand All @@ -382,37 +383,70 @@ 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;
}

return normalized;
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. 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(" ")}\nRunning 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(
Comment on lines 445 to 452

@sentry sentry Bot Sep 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: If a query with a numeric project filter fails to parse, it's rewritten and returned silently without the intended warning, because the warning logic is skipped.
Severity: MEDIUM

Suggested Fix

Move the logic for detecting and noting rewrites to before the try...catch block for parsing. This ensures that any modification to the query is recorded and warned about, even if the subsequent parsing step fails. The warning can then be emitted regardless of the parse outcome.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/cli/src/lib/search-query.ts#L445-L452

Potential issue: In the `sanitizeQuery` function, if a search query contains a numeric
project filter like `project:123` and also has a syntax error that causes the PEG parser
to fail (e.g., an unmatched parenthesis), the query is modified by
`rewriteNumericProjectFilters`. However, because the `parse` function throws an
exception, the `catch` block returns the rewritten query directly. This control flow
bypasses the `preParseRewriteNotes` function call, which is responsible for generating a
warning about the modification. As a result, the user's query is silently changed, which
contradicts the intended behavior of warning users about automatic rewrites.

Expand Down Expand Up @@ -511,6 +545,15 @@ const BALANCED_BRACKET_RE = /\[[^\]]*\]/g;
/** Trailing comma before closing bracket: `,]` */
const TRAILING_LIST_COMMA_RE = /,\s*\]$/;

/**
* `project:<digits>` as its own filter — not `bolt.project`, not `project_id`.
* Issue search treats `project` as a slug and `project_id` as a numeric id.
*/
const PROJECT_NUMERIC_RE = /(^|\s)(!?)project:(\d+)(?=\s|$)/gi;

/** `project:[123,456]` — every list value must be digits. */
const PROJECT_NUMERIC_LIST_RE = /(^|\s)(!?)project:\[(\d+(?:\s*,\s*\d+)*)\]/gi;

/**
* Pattern that splits a query into alternating unquoted / quoted segments.
*
Expand All @@ -519,6 +562,23 @@ const TRAILING_LIST_COMMA_RE = /,\s*\]$/;
*/
const QUOTED_SEGMENT_RE = /"(?:[^"\\]|\\.)*"/g;

/**
* Rewrite `project:<digits>` / `project:[digits,…]` to `project_id`.
*
* `project` is the slug; a numeric value is almost always a pasted Sentry
* project id (CLI-FA). Namespaced keys (`bolt.project:…`) and slugs are
* untouched. Quoted regions are preserved via {@link transformUnquoted}.
*/
function rewriteNumericProjectFilters(query: string): string {
return transformUnquoted(query, (segment) => {
PROJECT_NUMERIC_RE.lastIndex = 0;
PROJECT_NUMERIC_LIST_RE.lastIndex = 0;
return segment
.replace(PROJECT_NUMERIC_RE, "$1$2project_id:$3")
.replace(PROJECT_NUMERIC_LIST_RE, "$1$2project_id:[$3]");
});
}

/**
* Normalize a search query by applying a pipeline of text repairs.
Comment thread
betegon marked this conversation as resolved.
*
Expand Down
63 changes: 63 additions & 0 deletions packages/cli/test/lib/search-query.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,69 @@ describe("sanitizeQuery: AND", () => {
});
});

// ---------------------------------------------------------------------------
// project:<digits> → project_id
// ---------------------------------------------------------------------------

describe("sanitizeQuery: numeric project:", () => {
test("rewrites a numeric project: filter to project_id", () => {
expect(
sanitizeQuery("project:4511730126487632 environment:vercel-production")
).toBe("project_id:4511730126487632 environment:vercel-production");
});

test("rewrites a numeric project: in-list", () => {
expect(
sanitizeQuery("is:unresolved project:[4505521413357568,6442225]")
).toBe("is:unresolved project_id:[4505521413357568,6442225]");
});

test("rewrites a negated numeric project: filter", () => {
expect(sanitizeQuery("!project:1423462 lastSeen:-1h")).toBe(
"!project_id:1423462 lastSeen:-1h"
);
});

test("leaves project slugs alone", () => {
expect(sanitizeQuery("project:frontend is:unresolved")).toBe(
"project:frontend is:unresolved"
);
});

test("leaves project_id numeric filters alone", () => {
expect(sanitizeQuery("project_id:4511730126487632")).toBe(
"project_id:4511730126487632"
);
});

test("leaves namespaced project keys alone", () => {
expect(sanitizeQuery("bolt.project_id:70054175")).toBe(
"bolt.project_id:70054175"
);
expect(sanitizeQuery("bolt.project:70054175")).toBe(
"bolt.project:70054175"
);
});

test("does not rewrite a numeric id inside a quoted value", () => {
expect(sanitizeQuery('message:"project:4511730126487632"')).toBe(
'message:"project:4511730126487632"'
);
});

test("does not rewrite mixed slug/numeric in-lists", () => {
expect(sanitizeQuery("project:[frontend,6442225]")).toBe(
"project:[frontend,6442225]"
);
});

test("rewrites numeric project: then OR in one step", () => {
expect(sanitizeQuery("project:123 OR project:456")).toBe(
"project_id:[123,456]"
);
});
});

// ---------------------------------------------------------------------------
// OR → in-list rewrites (successful)
// ---------------------------------------------------------------------------
Expand Down
86 changes: 86 additions & 0 deletions packages/cli/test/lib/search-query.warn.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/**
* Warning copy for stacked search-query rewrites.
*
* `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";

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].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");
});

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].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", () => {
expect(sanitizeQuery("level:error OR level:warning")).toBe(
"level:[error,warning]"
);
const warns = runningQueries();
expect(warns).toHaveLength(1);
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", () => {
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);
});
});
Loading