Skip to content

Commit 0ee6535

Browse files
icecrasher321claude
andcommitted
fix(file): accept open-ended repeats, count characters, classify lock waits
Three defects from review, none of which the tests caught: `{n,}` was rejected. `readQuantifierAt` reports an unbounded maximum as Infinity, and the repeat cap compared it directly, so every open-ended repeat failed as "exceeds 1000" — a form the tool's own documentation offers. Only a stated maximum is measured now, and the minimum always is, since that is what an expansion unrolls. Query bounds and literal runs were measured in UTF-16 units while claiming characters, so two astral characters read as four and slipped a gate written for three. Both now count characters, which is also what pg_trgm indexes. `lock_timeout` was set without classifying what it raises. A wait on conflicting DDL surfaced as an unclassified server error, and folding it in with the timeout arm would have told the caller to fix a pattern that is already correct. It now maps to a distinct error the caller is told to retry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 8cd050d commit 0ee6535

7 files changed

Lines changed: 111 additions & 12 deletions

File tree

apps/sim/lib/workspace-files/application/search-workspace-file-content.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ import {
77
type FileSearchMode,
88
FileSearchPatternError,
99
} from '@/lib/workspace-files/search/pattern'
10-
import { searchWorkspaceFileIndex } from '@/lib/workspace-files/search/repository'
10+
import {
11+
searchWorkspaceFileIndex,
12+
WorkspaceFileSearchUnavailableError,
13+
} from '@/lib/workspace-files/search/repository'
1114

1215
export interface SearchWorkspaceFileContentInput {
1316
workspaceId: string
@@ -46,6 +49,10 @@ export const searchWorkspaceFileContent = defineAuthorizedWorkspaceFileUseCase({
4649
if (error instanceof FileSearchPatternError) {
4750
throw new OrchestrationError('validation', error.message)
4851
}
52+
/** Nothing is wrong with the query, so the caller is told to retry, not to rewrite it. */
53+
if (error instanceof WorkspaceFileSearchUnavailableError) {
54+
throw new OrchestrationError('locked', error.message)
55+
}
4956
throw error
5057
}
5158
},

apps/sim/lib/workspace-files/search/pattern.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,13 @@ describe('compileFileSearchPattern', () => {
1313
expect(() => compileFileSearchPattern('a'.repeat(513), mode)).toThrow(/at most 512/)
1414
expect(() => compileFileSearchPattern('abc\0def', mode)).toThrow(/NUL/)
1515
})
16+
17+
/** Two astral characters occupy four UTF-16 units but are still two characters. */
18+
it.each(['exact', 'regex'] as const)('bounds the query in characters in %s mode', (mode) => {
19+
expect(() => compileFileSearchPattern('🙂🙂', mode)).toThrow(/at least 3 characters/)
20+
expect(() => compileFileSearchPattern('🙂🙂🙂', mode)).not.toThrow()
21+
expect(() => compileFileSearchPattern('🙂'.repeat(513), mode)).toThrow(/at most 512/)
22+
})
1623
})
1724

1825
describe('exact mode', () => {

apps/sim/lib/workspace-files/search/pattern.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,12 +173,17 @@ export function compileFileSearchPattern(
173173
query: string,
174174
mode: FileSearchMode
175175
): CompiledFileSearchPattern {
176-
if (query.length < FILE_SEARCH_MIN_QUERY_LENGTH) {
176+
/**
177+
* Characters, not UTF-16 units: two astral characters occupy four units, and
178+
* measuring those would admit a query shorter than the bound claims to allow.
179+
*/
180+
const queryLength = [...query].length
181+
if (queryLength < FILE_SEARCH_MIN_QUERY_LENGTH) {
177182
throw new FileSearchPatternError(
178183
`Search query must be at least ${FILE_SEARCH_MIN_QUERY_LENGTH} characters`
179184
)
180185
}
181-
if (query.length > FILE_SEARCH_MAX_QUERY_LENGTH) {
186+
if (queryLength > FILE_SEARCH_MAX_QUERY_LENGTH) {
182187
throw new FileSearchPatternError(
183188
`Search query must be at most ${FILE_SEARCH_MAX_QUERY_LENGTH} characters`
184189
)

apps/sim/lib/workspace-files/search/regex.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ describe('analyzeFileSearchRegex', () => {
3333
expect(run('ab{2,4}cd')).toBe(3)
3434
})
3535

36+
it('measures a run in characters, not UTF-16 units', () => {
37+
expect(run('🙂🙂needle')).toBe(8)
38+
expect(run('🙂🙂')).toBe(2)
39+
expect(run('東京都')).toBe(3)
40+
})
41+
3642
it('does not let a bounded repeat expand into a large intermediate', () => {
3743
expect(run('(?:(?:abc){1000}){1000}')).toBeGreaterThanOrEqual(3)
3844
expect(run('(?:(?:abc){1000}){1000}')).toBeLessThanOrEqual(512)
@@ -78,6 +84,7 @@ describe('analyzeFileSearchRegex', () => {
7884
['*foo', /has no character to repeat/],
7985
['foo{2,1}', /counts down/],
8086
['a{1,5000}bcd', /exceeds 1000/],
87+
['a{5000,}bcd', /exceeds 1000/],
8188
['foo{bar}', /Unescaped "\{"/],
8289
])('rejects %s', (source, message) => {
8390
expect(() => analyzeFileSearchRegex(source)).toThrow(FileSearchPatternError)
@@ -100,6 +107,8 @@ describe('analyzeFileSearchRegex', () => {
100107
'status: [0-9]{3} failed',
101108
'cache.*?miss',
102109
'user_id=\\w+ token',
110+
'error a{3,}bcd',
111+
'error a{3,10}bcd',
103112
])('accepts %s', (source) => {
104113
expect(() => analyzeFileSearchRegex(source)).not.toThrow()
105114
})

apps/sim/lib/workspace-files/search/regex.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,15 @@ const POSTGRES_ONLY_ESCAPES: Record<string, string> = {
7575
Z: '$',
7676
}
7777

78+
/**
79+
* A run is measured in characters, because that is what `pg_trgm` indexes. The
80+
* parser walks UTF-16 units, so an astral character arrives as two surrogate
81+
* atoms whose concatenation is one character — counting units would score it two.
82+
*/
83+
function runLength(text: string): number {
84+
return [...text].length
85+
}
86+
7887
function head(text: string): string {
7988
return text.length > FILE_SEARCH_PATTERN_LITERAL_CAP
8089
? text.slice(0, FILE_SEARCH_PATTERN_LITERAL_CAP)
@@ -92,7 +101,7 @@ function literal(character: string): LiteralGuarantee {
92101
exact: character,
93102
prefix: character,
94103
suffix: character,
95-
best: character.length,
104+
best: runLength(character),
96105
zeroWidth: false,
97106
}
98107
}
@@ -108,7 +117,7 @@ function concatenate(left: LiteralGuarantee, right: LiteralGuarantee): LiteralGu
108117
exact: left.exact !== null && right.exact !== null ? head(left.exact + right.exact) : null,
109118
prefix: head(left.exact !== null ? left.exact + right.prefix : left.prefix),
110119
suffix: tail(right.exact !== null ? left.suffix + right.exact : right.suffix),
111-
best: Math.max(left.best, right.best, joined.length),
120+
best: Math.max(left.best, right.best, runLength(joined)),
112121
zeroWidth: left.zeroWidth && right.zeroWidth,
113122
}
114123
}
@@ -447,7 +456,16 @@ class FileSearchRegexParser {
447456
`Unescaped "{" at position ${this.index + 1} — write "\\{" to match a literal brace`
448457
)
449458
}
450-
if (bounded.max > FILE_SEARCH_PATTERN_MAX_REPEAT) {
459+
/**
460+
* `{n,}` has no upper bound to cap — it is `+` with a floor, and both engines
461+
* expand it the same way — so only a stated maximum is measured against the
462+
* cap. The minimum is always measured, since that is what an expansion
463+
* actually unrolls.
464+
*/
465+
if (
466+
bounded.min > FILE_SEARCH_PATTERN_MAX_REPEAT ||
467+
(Number.isFinite(bounded.max) && bounded.max > FILE_SEARCH_PATTERN_MAX_REPEAT)
468+
) {
451469
throw new FileSearchPatternError(
452470
`Repeat count at position ${this.index + 1} exceeds ${FILE_SEARCH_PATTERN_MAX_REPEAT}`
453471
)

apps/sim/lib/workspace-files/search/repository.test.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import {
88
compileFileSearchPattern,
99
FileSearchPatternError,
1010
} from '@/lib/workspace-files/search/pattern'
11-
import { searchWorkspaceFileIndex } from '@/lib/workspace-files/search/repository'
11+
import {
12+
searchWorkspaceFileIndex,
13+
WorkspaceFileSearchUnavailableError,
14+
} from '@/lib/workspace-files/search/repository'
1215

1316
/**
1417
* The shape a failed query really arrives in, captured from PostgreSQL 17
@@ -41,6 +44,20 @@ describe('searchWorkspaceFileIndex fault mapping', () => {
4144
await expect(search).rejects.toThrow(message)
4245
})
4346

47+
it('tells the caller to retry when the lock guard fires, not to fix the query', async () => {
48+
dbChainMockFns.transaction.mockRejectedValueOnce(driverError('55P03'))
49+
50+
const search = searchWorkspaceFileIndex({
51+
workspaceId: 'workspace-1',
52+
pattern: compileFileSearchPattern('error \\d+', 'regex'),
53+
maxResults: 50,
54+
})
55+
56+
await expect(search).rejects.toBeInstanceOf(WorkspaceFileSearchUnavailableError)
57+
await expect(search).rejects.not.toBeInstanceOf(FileSearchPatternError)
58+
await expect(search).rejects.toThrow(/Try again shortly/)
59+
})
60+
4461
it('does not reinterpret an unrelated database fault', async () => {
4562
dbChainMockFns.transaction.mockRejectedValueOnce(driverError('23505'))
4663

@@ -53,6 +70,18 @@ describe('searchWorkspaceFileIndex fault mapping', () => {
5370
).rejects.not.toBeInstanceOf(FileSearchPatternError)
5471
})
5572

73+
it('leaves an unrelated fault unclassified for the surface to generalize', async () => {
74+
dbChainMockFns.transaction.mockRejectedValueOnce(driverError('23505'))
75+
76+
await expect(
77+
searchWorkspaceFileIndex({
78+
workspaceId: 'workspace-1',
79+
pattern: compileFileSearchPattern('needle', 'exact'),
80+
maxResults: 50,
81+
})
82+
).rejects.not.toBeInstanceOf(WorkspaceFileSearchUnavailableError)
83+
})
84+
5685
it('caps how long a search may hold its connection', async () => {
5786
await searchWorkspaceFileIndex({
5887
workspaceId: 'workspace-1',

apps/sim/lib/workspace-files/search/repository.ts

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,21 @@ interface SearchWorkspaceFileIndexInput {
5353
}
5454

5555
const QUERY_CANCELED = '57014'
56+
const LOCK_NOT_AVAILABLE = '55P03'
5657
const INVALID_REGULAR_EXPRESSION = '2201B'
5758

59+
/**
60+
* The search could not run, for a reason the caller did not cause and cannot fix
61+
* by changing the query — distinct from {@link FileSearchPatternError}, so a
62+
* surface reports "try again" rather than blaming the pattern.
63+
*/
64+
export class WorkspaceFileSearchUnavailableError extends Error {
65+
constructor(message: string) {
66+
super(message)
67+
this.name = 'WorkspaceFileSearchUnavailableError'
68+
}
69+
}
70+
5871
/**
5972
* Walks to the driver error. Drizzle wraps a failed query in a `DrizzleQueryError`
6073
* that carries no `code` of its own, so reading the top-level error alone finds
@@ -71,17 +84,23 @@ function sqlStateOf(error: unknown): string | undefined {
7184
}
7285

7386
/**
74-
* Rewrites the two database faults a search pattern can cause into faults the
75-
* caller can act on.
87+
* Rewrites the database faults this read can raise into faults a caller can act
88+
* on, separating the two it causes from the one it merely waits on.
7689
*
7790
* `pg_trgm` only indexes a pattern it can extract trigrams from; a
7891
* punctuation-only, non-ASCII, or too-general one plans as a scan across every
7992
* workspace's segments, so {@link FILE_SEARCH_STATEMENT_TIMEOUT_MS} is what
80-
* stops one search holding a pooled connection. And PostgreSQL is the last of
93+
* stops one search holding a pooled connection. PostgreSQL is also the last of
8194
* the engines a regex passes through, so a construct that slipped the pattern
8295
* analyzer and `RegExp` surfaces here rather than as an unexplained failure.
96+
*
97+
* {@link FILE_SEARCH_LOCK_TIMEOUT_MS} is different in kind: it fires while
98+
* waiting on a conflicting lock — DDL against the segment tables — which no
99+
* query can be rewritten to avoid. Without this arm it would reach the caller as
100+
* an unclassified server error, and folding it in with the two above would tell
101+
* them to fix a pattern that is already correct.
83102
*/
84-
function asFileSearchPatternFault(error: unknown): FileSearchPatternError | null {
103+
function asFileSearchFault(error: unknown): Error | null {
85104
const sqlState = sqlStateOf(error)
86105
if (sqlState === QUERY_CANCELED) {
87106
return new FileSearchPatternError(
@@ -91,6 +110,11 @@ function asFileSearchPatternFault(error: unknown): FileSearchPatternError | null
91110
if (sqlState === INVALID_REGULAR_EXPRESSION) {
92111
return new FileSearchPatternError('Invalid search pattern.')
93112
}
113+
if (sqlState === LOCK_NOT_AVAILABLE) {
114+
return new WorkspaceFileSearchUnavailableError(
115+
'Workspace file search is briefly unavailable while its index is being updated. Try again shortly.'
116+
)
117+
}
94118
return null
95119
}
96120

@@ -345,7 +369,7 @@ export async function searchWorkspaceFileIndex({
345369
}
346370
} catch (error) {
347371
signal?.throwIfAborted()
348-
const fault = asFileSearchPatternFault(error)
372+
const fault = asFileSearchFault(error)
349373
if (fault) throw fault
350374
throw error
351375
}

0 commit comments

Comments
 (0)