diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index e078e3094..a2ac81371 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -27,6 +27,7 @@ - Fixed session transcript entries being lost on process crash: file-backed JSONL writers now write each completed entry to the OS page cache on append instead of microtask-batching; concurrent appends during an in-place atomic rewrite supersede the paused publish with a synchronous full-body rewrite so fenced events are durable before return; concurrent appends during `/move` write a full body to the live relocation path (source pre-rename, destination post-rename) so a crash mid-move no longer drops completed events; and the first write failure latches `#diskFailure` synchronously via `appendSync` so a later flush/close reports it instead of silently succeeding after a discarded rejected Promise ([#7444](https://github.com/can1357/oh-my-pi/pull/7444) by [@olegpulatov](https://github.com/olegpulatov)). - Fixed `/mcp reauth` sending literal `${VAR}` placeholders instead of env-expanded OAuth client credentials during the token exchange, and `MCPOAuthFlow.exchangeToken()` accepting an HTTP-200 token response with no `access_token` (e.g. a Slack `{ ok: false, error }` body), which stored an empty access token and only surfaced `invalid_token` on a later MCP request ([#7440](https://github.com/can1357/oh-my-pi/issues/7440)). +- Fixed fuzzy replace-all edits re-matching replacement text indefinitely, which could freeze the TUI and exhaust memory ([#7432](https://github.com/can1357/oh-my-pi/issues/7432)). - Fixed template argument substitution (`substituteArgs`) executing recursive placeholder expansion when positional argument values contain literal `$@` or `$ARGUMENTS` tokens. - Fixed focused-agent status bar dimming darkening Powerline end caps. - Fixed the browser relay creating duplicate "omp" tab groups: the bridge now keeps at most one group RPC in flight (a queued drain replaces fire-and-forget per-tab requests), so concurrent requests can no longer race the extension's non-atomic query→create→set-title sequence in the same window. Also fixed an extension reconnect (relay daemon restart, service-worker recycle) being misread as the user dragging every tab out of the omp group — grouping state is reset when the extension socket closes, so tabs regroup on the next hello instead of being permanently opted out. diff --git a/packages/coding-agent/src/edit/diff.ts b/packages/coding-agent/src/edit/diff.ts index b071ef77c..964ece0e4 100644 --- a/packages/coding-agent/src/edit/diff.ts +++ b/packages/coding-agent/src/edit/diff.ts @@ -855,7 +855,6 @@ export function replaceText(content: string, oldText: string, newText: string, o let normalizedContent = normalizeToLF(content); const normalizedOldText = normalizeToLF(oldText); const normalizedNewText = normalizeToLF(newText); - let count = 0; if (options.all) { // Check for exact matches first @@ -867,11 +866,13 @@ export function replaceText(content: string, oldText: string, newText: string, o }; } - // No exact matches - try fuzzy matching iteratively + // Match against the immutable source so inserted replacement text cannot become a later candidate. + const replacements: Array<{ startIndex: number; endIndex: number; text: string }> = []; while (true) { const matchOutcome = findMatch(normalizedContent, normalizedOldText, { allowFuzzy: options.fuzzy, threshold, + excludedRanges: replacements, }); const shouldUseClosest = @@ -888,14 +889,22 @@ export function replaceText(content: string, oldText: string, newText: string, o if (adjustedNewText === match.actualText) { break; } - normalizedContent = - normalizedContent.substring(0, match.startIndex) + - adjustedNewText + - normalizedContent.substring(match.startIndex + match.actualText.length); - count++; + replacements.push({ + startIndex: match.startIndex, + endIndex: match.startIndex + Math.max(match.actualText.length, 1), + text: adjustedNewText, + }); } - return { content: normalizedContent, count }; + replacements.sort((a, b) => a.startIndex - b.startIndex); + const parts: string[] = []; + let sourceIndex = 0; + for (const replacement of replacements) { + parts.push(normalizedContent.substring(sourceIndex, replacement.startIndex), replacement.text); + sourceIndex = replacement.endIndex; + } + parts.push(normalizedContent.substring(sourceIndex)); + return { content: parts.join(""), count: replacements.length }; } // Single replacement mode diff --git a/packages/coding-agent/src/edit/modes/replace.ts b/packages/coding-agent/src/edit/modes/replace.ts index e2db7e3eb..5fdb28859 100644 --- a/packages/coding-agent/src/edit/modes/replace.ts +++ b/packages/coding-agent/src/edit/modes/replace.ts @@ -258,23 +258,41 @@ function formatPreviewWindow(lines: string[], centerIndex: number, options: Prev .join("\n"); } -function findExactMatchOutcome(content: string, target: string): MatchOutcome | undefined { - const exactIndex = content.indexOf(target); - if (exactIndex === -1) { +function findExactMatchOutcome( + content: string, + target: string, + excludedRanges: readonly { startIndex: number; endIndex: number }[], +): MatchOutcome | undefined { + let firstIndex: number | undefined; + let occurrences = 0; + const recordedIndices: number[] = []; + let searchStart = 0; + + while (searchStart <= content.length - target.length) { + const index = content.indexOf(target, searchStart); + if (index === -1) break; + + const endIndex = index + target.length; + if (!excludedRanges.some(range => index < range.endIndex && endIndex > range.startIndex)) { + firstIndex ??= index; + occurrences++; + if (recordedIndices.length < MAX_RECORDED_MATCHES) { + recordedIndices.push(index); + } + } + searchStart = endIndex; + } + + if (firstIndex === undefined) { return undefined; } - const occurrences = content.split(target).length - 1; if (occurrences > 1) { const contentLines = content.split("\n"); const occurrenceLines: number[] = []; const occurrencePreviews: string[] = []; - let searchStart = 0; - - for (let i = 0; i < MAX_RECORDED_MATCHES; i++) { - const idx = content.indexOf(target, searchStart); - if (idx === -1) break; - const lineNumber = content.slice(0, idx).split("\n").length; + for (const index of recordedIndices) { + const lineNumber = content.slice(0, index).split("\n").length; occurrenceLines.push(lineNumber); occurrencePreviews.push( formatPreviewWindow(contentLines, lineNumber - 1, { @@ -282,17 +300,16 @@ function findExactMatchOutcome(content: string, target: string): MatchOutcome | maxLen: OCCURRENCE_PREVIEW_MAX_LEN, }), ); - searchStart = idx + 1; } return { occurrences, occurrenceLines, occurrencePreviews }; } - const startLine = content.slice(0, exactIndex).split("\n").length; + const startLine = content.slice(0, firstIndex).split("\n").length; return { match: { actualText: target, - startIndex: exactIndex, + startIndex: firstIndex, startLine, confidence: 1, }, @@ -408,6 +425,7 @@ function findBestFuzzyMatchCore( offsets: number[], threshold: number, includeDepth: boolean, + excludedRanges: readonly { startIndex: number; endIndex: number }[], ): BestFuzzyMatchResult { const targetNormalized = normalizeLines(targetLines, includeDepth); @@ -417,6 +435,12 @@ function findBestFuzzyMatchCore( let aboveThresholdCount = 0; for (let start = 0; start <= contentLines.length - targetLines.length; start++) { + const startIndex = offsets[start]; + const endLine = start + targetLines.length - 1; + const endIndex = Math.max(startIndex + 1, offsets[endLine] + contentLines[endLine].length); + if (excludedRanges.some(range => startIndex < range.endIndex && endIndex > range.startIndex)) { + continue; + } const windowLines = contentLines.slice(start, start + targetLines.length); const windowNormalized = normalizeLines(windowLines, includeDepth); let score = 0; @@ -434,7 +458,7 @@ function findBestFuzzyMatchCore( bestScore = score; best = { actualText: windowLines.join("\n"), - startIndex: offsets[start], + startIndex, startLine: start + 1, confidence: score, }; @@ -446,7 +470,12 @@ function findBestFuzzyMatchCore( return { best, aboveThresholdCount, secondBestScore }; } -function findBestFuzzyMatch(content: string, target: string, threshold: number): BestFuzzyMatchResult { +function findBestFuzzyMatch( + content: string, + target: string, + threshold: number, + excludedRanges: readonly { startIndex: number; endIndex: number }[], +): BestFuzzyMatchResult { const contentLines = content.split("\n"); const targetLines = target.split("\n"); @@ -458,11 +487,18 @@ function findBestFuzzyMatch(content: string, target: string, threshold: number): } const offsets = computeLineOffsets(contentLines); - let result = findBestFuzzyMatchCore(contentLines, targetLines, offsets, threshold, true); + let result = findBestFuzzyMatchCore(contentLines, targetLines, offsets, threshold, true, excludedRanges); // Retry without indent depth if match is close but below threshold if (result.best && result.best.confidence < threshold && result.best.confidence >= FALLBACK_THRESHOLD) { - const noDepthResult = findBestFuzzyMatchCore(contentLines, targetLines, offsets, threshold, false); + const noDepthResult = findBestFuzzyMatchCore( + contentLines, + targetLines, + offsets, + threshold, + false, + excludedRanges, + ); if (noDepthResult.best && noDepthResult.best.confidence > result.best.confidence) { result = noDepthResult; } @@ -473,25 +509,35 @@ function findBestFuzzyMatch(content: string, target: string, threshold: number): /** * Find a match for target text within content. - * Used primarily for replace-mode edits. + * Used primarily for replace-mode edits; excluded ranges remain invisible to exact and fuzzy matching. */ export function findMatch( content: string, target: string, - options: { allowFuzzy: boolean; threshold?: number }, + options: { + allowFuzzy: boolean; + threshold?: number; + excludedRanges?: readonly { startIndex: number; endIndex: number }[]; + }, ): MatchOutcome { if (target.length === 0) { return {}; } + const excludedRanges = options.excludedRanges ?? []; - const exactMatch = findExactMatchOutcome(content, target); + const exactMatch = findExactMatchOutcome(content, target, excludedRanges); if (exactMatch) { return exactMatch; } // Try fuzzy match const threshold = options.threshold ?? DEFAULT_FUZZY_THRESHOLD; - const { best, aboveThresholdCount, secondBestScore } = findBestFuzzyMatch(content, target, threshold); + const { best, aboveThresholdCount, secondBestScore } = findBestFuzzyMatch( + content, + target, + threshold, + excludedRanges, + ); if (!best) { return {}; diff --git a/packages/coding-agent/test/edit-diff.test.ts b/packages/coding-agent/test/edit-diff.test.ts index 91bb573ff..97d4aa3ad 100644 --- a/packages/coding-agent/test/edit-diff.test.ts +++ b/packages/coding-agent/test/edit-diff.test.ts @@ -14,6 +14,7 @@ import { computeHashlineDiff, DEFAULT_FUZZY_THRESHOLD, findMatch, + replaceText, } from "@oh-my-pi/pi-coding-agent/edit"; import { removeWithRetries } from "@oh-my-pi/pi-utils"; @@ -224,6 +225,26 @@ describe("adjustIndentation", () => { }); }); +describe("replaceText", () => { + test("does not re-match inserted text while replacing all fuzzy source matches", () => { + const oldText = "a".repeat(50); + const firstActual = `${"a".repeat(49)}b`; + const secondActual = `${"a".repeat(44)}cccccc`; + const newText = `${oldText}\nexpanded`; + + expect( + replaceText(`${firstActual}\n${secondActual}`, oldText, newText, { + all: true, + fuzzy: true, + threshold: 0.8, + }), + ).toEqual({ + content: `${newText}\n${newText}`, + count: 2, + }); + }); +}); + describe("computeHashlineDiff", () => { let tempDir = "";