diff --git a/packages/runtime/src/__tests__/builtin-tools.test.ts b/packages/runtime/src/__tests__/builtin-tools.test.ts index 1e2640721c..9824aeb265 100644 --- a/packages/runtime/src/__tests__/builtin-tools.test.ts +++ b/packages/runtime/src/__tests__/builtin-tools.test.ts @@ -2186,6 +2186,24 @@ describe('builtin write tools path containment', () => { expect(await readFile(join(root, 'inside.txt'), 'utf8')).toBe('hello Maka'); }); + test('Edit returns a file diff for a localized change in a large file', async () => { + const root = await mkdtemp(join(tmpdir(), 'maka-edit-large-diff-')); + const content = Array.from({ length: 900 }, (_, index) => `const v${index} = ${index};`).join( + '\n', + ); + await writeFile(join(root, 'large.ts'), `${content}\n`, 'utf8'); + + const result = await runTool( + tool('Edit'), + { path: 'large.ts', old_string: 'const v500 = 500;', new_string: 'const v500 = -1;' }, + root, + ); + + expect(result).toMatchObject({ kind: 'file_diff' }); + assert.match((result as { diff: string }).diff, /-const v500 = 500;/); + assert.match((result as { diff: string }).diff, /\+const v500 = -1;/); + }); + test('file tools stay usable when the session cwd is reached through a symlink', async () => { // The session cwd itself sits under a symlink (macOS hands out `/var/...` // tmpdirs whose realpath is `/private/var/...`, and workspace roots are often diff --git a/packages/runtime/src/__tests__/filesystem-worker.test.ts b/packages/runtime/src/__tests__/filesystem-worker.test.ts index 2f7ca9bd63..d056b7a76d 100644 --- a/packages/runtime/src/__tests__/filesystem-worker.test.ts +++ b/packages/runtime/src/__tests__/filesystem-worker.test.ts @@ -477,7 +477,7 @@ describe('filesystem worker operations', () => { assert.equal(await readFile(target, 'utf8'), 'external'); }); - test('omits the diff when the content is too large to diff cheaply', async () => { + test('returns a localized diff for a small Edit in a large file', async () => { const root = await temporaryDirectory('maka-worker-huge-'); const target = join(root, 'huge.ts'); const before = Array.from({ length: 900 }, (_, i) => `const v${i} = ${i};`).join('\n') + '\n'; @@ -496,10 +496,37 @@ describe('filesystem worker operations', () => { ), ); + assert.ok(response.ok); + assert.equal(response.result.kind, 'edit'); + if (response.result.kind !== 'edit') return; + assert.match(response.result.diff ?? '', /-const v0 = 0;/); + assert.match(response.result.diff ?? '', /\+const v0 = -1;/); + }); + + test('omits an oversized Edit diff while still applying the replacement', async () => { + const root = await temporaryDirectory('maka-worker-large-replacement-'); + const target = join(root, 'large.ts'); + await writeFile(target, 'const value = 1;\n', 'utf8'); + const replacement = Array.from({ length: 900 }, (_, i) => `const value${i} = ${i};`).join('\n'); + + const response = await executeFilesystemWorkerRequest( + await requestFor( + { + kind: 'edit', + cwd: root, + path: target, + oldString: 'const value = 1;', + newString: replacement, + }, + { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file' }, + ), + ); + assert.ok(response.ok); assert.equal(response.result.kind, 'edit'); if (response.result.kind !== 'edit') return; assert.equal(response.result.diff, undefined); + assert.equal(await readFile(target, 'utf8'), `${replacement}\n`); }); test('omits the diff when FormatJson leaves the file unchanged', async () => { diff --git a/packages/runtime/src/filesystem-executor.ts b/packages/runtime/src/filesystem-executor.ts index 4faf52099a..53fd789449 100644 --- a/packages/runtime/src/filesystem-executor.ts +++ b/packages/runtime/src/filesystem-executor.ts @@ -36,7 +36,7 @@ import type { PermissionMode } from '@maka/core/permission'; import type { PermissionProfile } from '@maka/core/permission-profile'; import { ToolOutcomeUnknownError } from '@maka/core/events'; import { computeEditedSource } from './edit-replace.js'; -import { createUnifiedDiff } from './unified-diff.js'; +import { createEditUnifiedDiff, createUnifiedDiff } from './unified-diff.js'; import { classifyFailedMutationOutcome, type FilesystemTargetIdentity, @@ -501,15 +501,17 @@ function createWorkspaceFilesystemExecutor( if (isSupportedImagePath(path)) throw new Error('Edit does not support image files.'); if (workspace.readModifyWrite) { let edited!: ReturnType; - const result = await workspace.readModifyWrite({ + let originalContent = ''; + await workspace.readModifyWrite({ cwd, path, label: 'Edit', scope, approvedIdentity: expectedIdentity, transform: (ctx) => { + originalContent = ctx.content ?? ''; edited = computeEditedSource( - ctx.content ?? '', + originalContent, operation.oldString, operation.newString, operation.path, @@ -517,11 +519,7 @@ function createWorkspaceFilesystemExecutor( return edited.content; }, }); - const diff = createUnifiedDiff( - path, - result.previous === 'unknown' || result.previous === 'new' ? '' : result.previous, - result.finalContent ?? '', - ); + const diff = createEditUnifiedDiff(path, originalContent, edited.content, edited); return { kind: 'edit', ok: true, @@ -542,7 +540,7 @@ function createWorkspaceFilesystemExecutor( operation.path, ); await workspace.writeFile({ cwd, path, content: edited.content }); - const diff = createUnifiedDiff(path, read.content, edited.content); + const diff = createEditUnifiedDiff(path, read.content, edited.content, edited); return { kind: 'edit', ok: true, diff --git a/packages/runtime/src/filesystem-worker/operations.ts b/packages/runtime/src/filesystem-worker/operations.ts index dd567300a4..1c9475c78f 100644 --- a/packages/runtime/src/filesystem-worker/operations.ts +++ b/packages/runtime/src/filesystem-worker/operations.ts @@ -31,7 +31,7 @@ import { } from '../apply-patch-file.js'; import { computeEditedSource } from '../edit-replace.js'; -import { createUnifiedDiff } from '../unified-diff.js'; +import { createEditUnifiedDiff, createUnifiedDiff } from '../unified-diff.js'; import { compareAndDeleteEntry, hostVisibilityAfterWrite, @@ -293,7 +293,7 @@ export async function executeFilesystemOperation( await writeThroughHandle(handle, source.content); const visibility = await hostVisibilityAfterWrite(path, handle); if (visibility) throw visibility; - const diff = createUnifiedDiff(path, content, source.content); + const diff = createEditUnifiedDiff(path, content, source.content, source); return { kind: 'edit', ok: true, diff --git a/packages/runtime/src/unified-diff.ts b/packages/runtime/src/unified-diff.ts index a1d43245c9..3354996cc3 100644 --- a/packages/runtime/src/unified-diff.ts +++ b/packages/runtime/src/unified-diff.ts @@ -27,6 +27,7 @@ const MAX_DIFF_SOURCE_LINES = 800; const MAX_DIFF_SOURCE_BYTES = 32 * 1024; +const MAX_DIFF_OUTPUT_BYTES = 32 * 1024; const CONTEXT_LINES = 3; /** @@ -57,10 +58,56 @@ export function createUnifiedDiff( return [...header, ...formatHunks(ops)].join('\n'); } +/** + * Localized unified diff for Edit's one known replacement span. Unlike the + * generic full-file diff, its cost depends on the changed hunk rather than the + * file size; the final output remains bounded for huge lines or replacements. + */ +export function createEditUnifiedDiff( + path: string, + oldContent: string, + newContent: string, + match: { startLine: number; endLine: number }, +): string | undefined { + if (oldContent.includes('\0') || newContent.includes('\0')) return undefined; + const oldLines = splitLines(oldContent); + const newLines = splitLines(newContent); + const startIndex = match.startLine - 1; + const oldEndIndex = match.endLine - 1; + if (startIndex < 0 || oldEndIndex < startIndex || oldEndIndex >= oldLines.length) { + return undefined; + } + + const windowStart = Math.max(0, startIndex - CONTEXT_LINES); + const oldAfterStart = oldEndIndex + 1; + const newAfterStart = oldAfterStart + newLines.length - oldLines.length; + const oldWindow = oldLines.slice( + windowStart, + Math.min(oldLines.length, oldAfterStart + CONTEXT_LINES), + ); + const newWindow = newLines.slice( + windowStart, + Math.min(newLines.length, Math.max(windowStart, newAfterStart) + CONTEXT_LINES), + ); + if (isDiffWindowTooLarge(oldWindow) || isDiffWindowTooLarge(newWindow)) return undefined; + const ops = diffLines(oldWindow, newWindow); + if (ops.every((op) => op.kind === 'keep')) return undefined; + const diff = [ + `--- a/${path}`, + `+++ b/${path}`, + ...formatHunks(ops, windowStart, windowStart), + ].join('\n'); + return Buffer.byteLength(diff, 'utf8') <= MAX_DIFF_OUTPUT_BYTES ? diff : undefined; +} + function isUndiffable(content: string): boolean { return content.includes('\0') || Buffer.byteLength(content, 'utf8') > MAX_DIFF_SOURCE_BYTES; } +function isDiffWindowTooLarge(lines: string[]): boolean { + return lines.length > MAX_DIFF_SOURCE_LINES || isUndiffable(lines.join('\n')); +} + function splitLines(content: string): string[] { const lines = content.split('\n'); // A trailing newline terminates the last line rather than starting an empty @@ -113,7 +160,7 @@ function diffLines(oldLines: string[], newLines: string[]): DiffOp[] { } /** Group the edit script into hunks with CONTEXT_LINES of surrounding context. */ -function formatHunks(ops: DiffOp[]): string[] { +function formatHunks(ops: DiffOp[], oldLineOffset = 0, newLineOffset = 0): string[] { const changed = ops.flatMap((op, index) => (op.kind === 'keep' ? [] : [index])); if (changed.length === 0) return []; @@ -133,8 +180,8 @@ function formatHunks(ops: DiffOp[]): string[] { const out: string[] = []; for (const group of groups) { const slice = ops.slice(group.start, group.end + 1); - const oldStart = slice[0].oldIndex + 1; - const newStart = slice[0].newIndex + 1; + const oldStart = slice[0].oldIndex + oldLineOffset + 1; + const newStart = slice[0].newIndex + newLineOffset + 1; const oldCount = slice.filter((op) => op.kind !== 'add').length; const newCount = slice.filter((op) => op.kind !== 'del').length; out.push( diff --git a/packages/ui/src/index.ts b/packages/ui/src/index.ts index a51d3b729e..ed46774b29 100644 --- a/packages/ui/src/index.ts +++ b/packages/ui/src/index.ts @@ -75,11 +75,6 @@ export { Card, type CardProps, type CardVariant } from '@astryxdesign/core'; // cross-package consumer — `apps/desktop`'s `artifact-preview.tsx` — which is the // promotion condition the off-barrel convention named, so the export is the rule. export { previewVariants } from './primitives/chat.js'; -// `diffLineKind` rides the same seam for the same reason: it decides the -// `data-line` values those parts are selected by, so a second copy of it is a -// second answer to "what colour is this line". `apps/desktop` had one, and the -// two had already diverged on `diff --git` / `index` headers. -export { diffLineKind } from './tool-activity/tool-result-preview.js'; export { DiffCodePreview } from './tool-activity/diff-code-preview.js'; export { syntaxLanguageForPath } from './tool-activity/diff-syntax.js'; export { MarkdownBody } from './markdown-body.js'; diff --git a/packages/ui/src/tool-activity/tool-result-preview.tsx b/packages/ui/src/tool-activity/tool-result-preview.tsx index e3274e8188..a78e88daba 100644 --- a/packages/ui/src/tool-activity/tool-result-preview.tsx +++ b/packages/ui/src/tool-activity/tool-result-preview.tsx @@ -304,28 +304,6 @@ function PtyControlPreview(props: { ); } -/** - * Which tint a unified-diff line takes. Deliberately shallow: it reads the - * line's first character, not the hunk semantics, which is all the colouring - * needs and all a preview should promise. - * - * `+++`/`---` are file markers, not an addition and a deletion — they have to - * be tested before the single-character cases or every diff opens with one - * green and one red line that mean nothing. - */ -export function diffLineKind(line: string): 'add' | 'del' | 'hunk' | 'meta' | 'ctx' { - // The trailing space is what separates a file marker from content: unified - // diff writes `--- a/path`, never a bare `---`. Without it, deleting a YAML - // document separator or an SQL `--` comment paints the removal as a header — - // the one line the reader most needs to see as red. - if (line.startsWith('--- ') || line.startsWith('+++ ')) return 'meta'; - if (line.startsWith('@@')) return 'hunk'; - if (line.startsWith('+')) return 'add'; - if (line.startsWith('-')) return 'del'; - if (line.startsWith('diff ') || line.startsWith('index ')) return 'meta'; - return 'ctx'; -} - /** * Line-level diff colouring — green additions, red deletions, a tinted hunk * header — in the same surface a command uses, with the changed paths as its