diff --git a/packages/api/src/agents/handlers.spec.ts b/packages/api/src/agents/handlers.spec.ts index 22fdf7ef6db..441297c3712 100644 --- a/packages/api/src/agents/handlers.spec.ts +++ b/packages/api/src/agents/handlers.spec.ts @@ -5121,6 +5121,200 @@ describe('createToolExecuteHandler', () => { expect(updateSkill).not.toHaveBeenCalled(); }); + function makeMatchingHandler(content: string) { + const saveSkillFileContent = jest.fn(async () => ({ + bytes: content.length, + relativePath: 'references/a.md', + })); + const handler = makeAuthoringHandler({ + getSkillByName: jest.fn(async () => ({ + _id: SKILL_ID, + name: 'matching-skill', + body: '# Existing', + fileCount: 1, + version: 1, + })), + getSkillFileByPath: jest.fn(async () => ({ + content, + isBinary: false, + mimeType: 'text/markdown', + bytes: Buffer.byteLength(content), + filepath: '/tmp/a.md', + file_id: 'revision-1', + source: 'local', + relativePath: 'references/a.md', + })), + saveSkillFileContent, + }); + return { handler, saveSkillFileContent }; + } + + it.each([ + { + label: 'exact precedence over tolerant duplicates', + content: 'alpha\nbeta\nalpha \nbeta \n', + oldText: 'alpha\nbeta', + expected: 'changed\nalpha \nbeta \n', + strategy: 'exact', + }, + { + label: 'trimmed precedence over whitespace-normalized duplicates', + content: 'alpha \nbeta\t\nalpha\tbeta\n', + oldText: 'alpha\nbeta', + expected: 'changed\nalpha\tbeta\n', + strategy: 'line-trimmed', + }, + { + label: 'whitespace-normalized precedence over indentation', + content: 'header\n a\n b\nfooter\n', + oldText: 'a\n b', + expected: 'header\n changed\nfooter\n', + strategy: 'whitespace-normalized', + }, + { + label: 'repeated-prefix fallback in the line sequence', + content: 'a \nb \na \nb \na \nc \n', + oldText: 'a\nb\na\nc', + expected: 'a \nb \nchanged\n', + strategy: 'line-trimmed', + }, + { + label: 'UTF-16 offsets and CRLF line endings', + content: '😀 keep\r\nfoo \r\nbar\t\r\nlast', + oldText: 'foo\nbar', + expected: '😀 keep\r\nchanged\nlast', + strategy: 'line-trimmed', + }, + { + label: 'whitespace-only lines at EOF', + content: 'pre\n\t\n ', + oldText: '\t \n\t ', + expected: 'pre\nchanged', + strategy: 'line-trimmed', + }, + { + label: 'window-local indentation with shorter blank lines', + content: 'pre\n \n\t token\n\t\npost', + oldText: '\n token\n', + expected: 'pre\nchanged\npost', + strategy: 'indentation-flexible', + }, + { + label: 'residual whitespace and zero needle indentation', + content: 'pre\n\n token\t\n ', + oldText: '\ntoken\t\n ', + expected: 'pre\nchanged', + strategy: 'indentation-flexible', + }, + { + label: 'overlapping trimmed replace_all ranges', + content: 'a \na \na \n', + oldText: 'a\na', + expected: 'changed\na \n', + strategy: 'line-trimmed x1', + replaceAll: true, + }, + { + label: 'multiple local indentation levels', + content: 'pre\n\n token\n\nmid\n\n\ttoken\n\npost', + oldText: '\n token\n', + expected: 'pre\nchanged\nmid\nchanged\npost', + strategy: 'indentation-flexible x2', + replaceAll: true, + }, + ])( + 'preserves $label in edit_file', + async ({ content, oldText, expected, strategy, ...flags }) => { + const { handler, saveSkillFileContent } = makeMatchingHandler(content); + const [result] = await invokeHandler(handler, [ + { + id: 'call_matching', + name: 'edit_file', + args: { + path: 'skills/matching-skill/references/a.md', + old_text: oldText, + new_text: 'changed', + replace_all: flags.replaceAll === true, + }, + }, + ]); + expect(result.errorMessage).toBeUndefined(); + expect(result.status).toBe('success'); + expect(result.artifact).toMatchObject({ strategies: [strategy] }); + expect(saveSkillFileContent).toHaveBeenCalledWith( + expect.objectContaining({ content: expected }), + ); + }, + ); + + it.each([ + ['a \na \na \n', 'a\na', 'matched 2 locations with line-trimmed', false], + [ + '\n token\n\n\ttoken\n', + '\n token\n', + 'matched 2 locations with indentation-flexible', + false, + ], + ['pre\n \n token\n \npost', ' \t \n token\n \t ', 'did not match', false], + ['a\nb', 'a\nb\nc', 'did not match', false], + ['\nx\n'.repeat(10_001), '\n x\n', 'limited to 10000 locations', true], + ])( + 'refuses unmatched or ambiguous line windows (%#)', + async (content, oldText, error, replaceAll) => { + const { handler, saveSkillFileContent } = makeMatchingHandler(content); + const [result] = await invokeHandler(handler, [ + { + id: 'call_matching_error', + name: 'edit_file', + args: { + path: 'skills/matching-skill/references/a.md', + old_text: oldText, + new_text: 'changed', + replace_all: replaceAll, + }, + }, + ]); + expect(result.status).toBe('error'); + expect(result.errorMessage).toContain(error); + expect(saveSkillFileContent).not.toHaveBeenCalled(); + }, + ); + + it('normalizes each line once on a large multiline miss', async () => { + const content = ' '.repeat(19).concat('\n').repeat(20_000); + const oldText = '\t'.repeat(19).concat('\n').repeat(800) + 'missing'; + const { handler, saveSkillFileContent } = makeMatchingHandler(content); + const trimEnd = String.prototype.trimEnd; + const budget = 20_001 + 801; + let normalizations = 0; + const spy = jest.spyOn(String.prototype, 'trimEnd').mockImplementation(function ( + this: string, + ) { + if (++normalizations > budget) throw new Error('Repeated window normalization'); + return trimEnd.call(this); + }); + let result: ToolExecuteResult; + try { + [result] = await invokeHandler(handler, [ + { + id: 'call_matching_large_miss', + name: 'edit_file', + args: { + path: 'skills/matching-skill/references/a.md', + old_text: oldText, + new_text: 'changed', + }, + }, + ]); + } finally { + spy.mockRestore(); + } + expect(normalizations).toBe(budget); + expect(result.status).toBe('error'); + expect(result.errorMessage).toContain('old_text did not match'); + expect(saveSkillFileContent).not.toHaveBeenCalled(); + }); + it('fails loudly when edit_file old_text is ambiguous', async () => { const saveSkillFileContent = jest.fn(); const handler = makeAuthoringHandler({ diff --git a/packages/api/src/agents/handlers.ts b/packages/api/src/agents/handlers.ts index 8c4132f6442..5b34cf83c17 100644 --- a/packages/api/src/agents/handlers.ts +++ b/packages/api/src/agents/handlers.ts @@ -1762,29 +1762,85 @@ function findLineWindowMatch( } const starts = lineStarts(content); - const normalizedNeedle = - strategy === 'line-trimmed' - ? needleLines.map((line) => line.trimEnd()).join('\n') - : stripCommonIndent(needle); const matches: Array<{ index: number; length: number }> = []; - - for ( - let i = 0; - i <= contentLines.length - needleLines.length && matches.length <= MAX_EDIT_MATCHES; - i++ - ) { - const windowLines = contentLines.slice(i, i + needleLines.length); - const candidate = - strategy === 'line-trimmed' - ? windowLines.map((line) => line.trimEnd()).join('\n') - : stripCommonIndent(windowLines.join('\n')); - if (candidate !== normalizedNeedle) { - continue; - } - const index = starts[i]; - const endLine = i + needleLines.length; + const addMatch = (startLine: number) => { + const index = starts[startLine]; + const endLine = startLine + needleLines.length; const end = endLine < starts.length ? starts[endLine] - 1 : content.length; matches.push({ index, length: end - index }); + }; + + if (strategy === 'line-trimmed') { + // Intern normalized lines so KMP compares integer IDs, not overlapping strings. + const ids = new Map(); + const pattern = needleLines.map((line) => { + const normalized = line.trimEnd(); + let id = ids.get(normalized); + if (id == null) { + id = ids.size; + ids.set(normalized, id); + } + return id; + }); + const prefixes = new Uint32Array(pattern.length); + let matched = 0; + for (let i = 1; i < pattern.length; i++) { + while (matched > 0 && pattern[i] !== pattern[matched]) { + matched = prefixes[matched - 1]; + } + if (pattern[i] === pattern[matched]) matched++; + prefixes[i] = matched; + } + + matched = 0; + for (let i = 0; i < contentLines.length && matches.length <= MAX_EDIT_MATCHES; i++) { + const id = ids.get(contentLines[i].trimEnd()); + while (matched > 0 && id !== pattern[matched]) { + matched = prefixes[matched - 1]; + } + if (id === pattern[matched]) matched++; + if (matched === pattern.length) { + addMatch(i - pattern.length + 1); + // Keep overlapping occurrences for ambiguity detection and replace_all. + matched = prefixes[matched - 1]; + } + } + } else { + // This fallback runs only after line-trimmed and whitespace-normalized miss. + // Two nonblank needle lines imply at least two tokens: any indentation match + // would already have matched whitespace-normalized. An all-blank needle would + // already have matched line-trimmed. Only a single nonblank line remains. + let anchor = -1; + for (let i = 0; i < needleLines.length; i++) { + if (needleLines[i].trim().length === 0) continue; + if (anchor !== -1) return { status: 'none' }; + anchor = i; + } + if (anchor === -1) return { status: 'none' }; + + const normalizedNeedle = stripCommonIndent(needle).split('\n'); + const nonblank = new Uint32Array(contentLines.length + 1); + for (let i = 0; i < contentLines.length; i++) { + nonblank[i + 1] = nonblank[i] + Number(contentLines[i].trim().length > 0); + } + for (let i = anchor; i < contentLines.length && matches.length <= MAX_EDIT_MATCHES; i++) { + if (nonblank[i + 1] === nonblank[i]) continue; + const start = i - anchor; + const end = start + needleLines.length; + if (end > contentLines.length) break; + if (nonblank[end] - nonblank[start] !== 1) continue; + const indent = contentLines[i].length - contentLines[i].trimStart().length; + let matchesNeedle = true; + // Eligible windows have one nonblank line at a fixed offset. Each file line + // can belong to at most two of them, so these comparisons stay linear too. + for (let j = 0; j < needleLines.length; j++) { + if (contentLines[start + j].slice(indent) !== normalizedNeedle[j]) { + matchesNeedle = false; + break; + } + } + if (matchesNeedle) addMatch(start); + } } if (matches.length === 1) {