diff --git a/server/routes/__tests__/revertAgentChanges.test.js b/server/routes/__tests__/revertAgentChanges.test.js new file mode 100644 index 00000000..b8109faf --- /dev/null +++ b/server/routes/__tests__/revertAgentChanges.test.js @@ -0,0 +1,231 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import path from 'path'; +import fs from 'fs'; +import os from 'os'; +import { execSync } from 'child_process'; +import { revertFilesAtProjectPath } from '../git.js'; + +let REPO; + +function run(cmd, args, opts = {}) { + execSync(`${cmd} ${args.map(a => JSON.stringify(a)).join(' ')}`, { + cwd: REPO, + stdio: 'pipe', + ...opts, + }); +} + +beforeEach(() => { + // Resolve symlinks (e.g. macOS /var -> /private/var) so git paths match. + REPO = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'revert-test-'))); + run('git', ['init', '-q', '-b', 'main']); + run('git', ['config', 'user.email', 'test@example.com']); + run('git', ['config', 'user.name', 'Test']); + run('git', ['config', 'commit.gpgsign', 'false']); + + fs.writeFileSync(path.join(REPO, 'a.txt'), 'original-a\n'); + fs.writeFileSync(path.join(REPO, 'b.txt'), 'original-b\n'); + run('git', ['add', '.']); + run('git', ['commit', '-q', '-m', 'initial']); +}); + +afterEach(() => { + fs.rmSync(REPO, { recursive: true, force: true }); +}); + +describe('revertFilesAtProjectPath', () => { + it('restores a modified tracked file', async () => { + fs.writeFileSync(path.join(REPO, 'a.txt'), 'agent-modified\n'); + + const result = await revertFilesAtProjectPath(REPO, ['a.txt']); + + expect(result.success).toBe(true); + expect(result.reverted).toEqual(['a.txt']); + expect(result.errors).toEqual([]); + expect(fs.readFileSync(path.join(REPO, 'a.txt'), 'utf8')).toBe('original-a\n'); + }); + + it('deletes an untracked file (agent created new)', async () => { + fs.writeFileSync(path.join(REPO, 'new.txt'), 'agent-new\n'); + + const result = await revertFilesAtProjectPath(REPO, ['new.txt']); + + expect(result.success).toBe(true); + expect(result.reverted).toEqual(['new.txt']); + expect(fs.existsSync(path.join(REPO, 'new.txt'))).toBe(false); + }); + + it('unstages and removes a staged-added file', async () => { + fs.writeFileSync(path.join(REPO, 'staged.txt'), 'staged-add\n'); + run('git', ['add', 'staged.txt']); + + const result = await revertFilesAtProjectPath(REPO, ['staged.txt']); + + expect(result.success).toBe(true); + expect(result.reverted).toEqual(['staged.txt']); + expect(fs.existsSync(path.join(REPO, 'staged.txt'))).toBe(false); + }); + + it('leaves untouched files alone (preserves user manual edits)', async () => { + // Agent modifies a.txt; user separately modifies b.txt. + fs.writeFileSync(path.join(REPO, 'a.txt'), 'agent-edit\n'); + fs.writeFileSync(path.join(REPO, 'b.txt'), 'user-edit\n'); + + const result = await revertFilesAtProjectPath(REPO, ['a.txt']); + + expect(result.success).toBe(true); + expect(result.reverted).toEqual(['a.txt']); + // User's manual edit to b.txt must be preserved. + expect(fs.readFileSync(path.join(REPO, 'b.txt'), 'utf8')).toBe('user-edit\n'); + }); + + it('skips files with no changes', async () => { + const result = await revertFilesAtProjectPath(REPO, ['a.txt']); + + expect(result.success).toBe(true); + expect(result.reverted).toEqual([]); + expect(result.skipped).toEqual(['a.txt']); + }); + + it('handles multiple files with mixed states in one call', async () => { + fs.writeFileSync(path.join(REPO, 'a.txt'), 'mod-a\n'); + fs.writeFileSync(path.join(REPO, 'created.txt'), 'new\n'); + // b.txt has no changes + + const result = await revertFilesAtProjectPath(REPO, ['a.txt', 'created.txt', 'b.txt']); + + expect(result.reverted.sort()).toEqual(['a.txt', 'created.txt']); + expect(result.skipped).toEqual(['b.txt']); + expect(result.errors).toEqual([]); + }); + + it('rejects paths that escape the project root', async () => { + const result = await revertFilesAtProjectPath(REPO, ['../outside.txt']); + + expect(result.success).toBe(false); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].file).toBe('../outside.txt'); + // safePath throws with "Path traversal blocked" + expect(result.errors[0].reason).toMatch(/traversal/i); + }); + + it('deduplicates repeated file entries', async () => { + fs.writeFileSync(path.join(REPO, 'a.txt'), 'mod\n'); + + const result = await revertFilesAtProjectPath(REPO, ['a.txt', 'a.txt', 'a.txt']); + + expect(result.reverted).toEqual(['a.txt']); + }); + + it('returns empty result for empty files array', async () => { + const result = await revertFilesAtProjectPath(REPO, []); + expect(result).toEqual({ success: true, reverted: [], skipped: [], errors: [] }); + }); + + it('restores a tracked file that the agent deleted', async () => { + fs.unlinkSync(path.join(REPO, 'a.txt')); + + const result = await revertFilesAtProjectPath(REPO, ['a.txt']); + + expect(result.success).toBe(true); + expect(result.reverted).toEqual(['a.txt']); + expect(fs.readFileSync(path.join(REPO, 'a.txt'), 'utf8')).toBe('original-a\n'); + }); + + it('refuses to recursively delete an untracked directory', async () => { + const dirPath = path.join(REPO, 'agent-dir'); + fs.mkdirSync(dirPath); + fs.writeFileSync(path.join(dirPath, 'inner.txt'), 'inner\n'); + + const result = await revertFilesAtProjectPath(REPO, ['agent-dir']); + + expect(result.success).toBe(false); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].reason).toMatch(/directory/i); + // Directory must still exist + expect(fs.existsSync(dirPath)).toBe(true); + }); + + it('deletes a symlink itself rather than following it out of the repo', async () => { + // Create an external target the symlink would point to + const externalFile = fs.mkdtempSync(path.join(os.tmpdir(), 'external-target-')); + const externalPath = path.join(externalFile, 'outside.txt'); + fs.writeFileSync(externalPath, 'do-not-touch\n'); + + // Symlink inside the repo pointing out + const linkPath = path.join(REPO, 'agent-link'); + fs.symlinkSync(externalPath, linkPath); + + const result = await revertFilesAtProjectPath(REPO, ['agent-link']); + + expect(result.success).toBe(true); + expect(result.reverted).toEqual(['agent-link']); + // Symlink removed + expect(fs.existsSync(linkPath)).toBe(false); + // External file untouched — critical safety guarantee + expect(fs.existsSync(externalPath)).toBe(true); + expect(fs.readFileSync(externalPath, 'utf8')).toBe('do-not-touch\n'); + + fs.rmSync(externalFile, { recursive: true, force: true }); + }); + + it('handles renamed files by skipping (explicitly not auto-guessed)', async () => { + run('git', ['mv', 'a.txt', 'a-renamed.txt']); + + const result = await revertFilesAtProjectPath(REPO, ['a-renamed.txt']); + + expect(result.success).toBe(true); + expect(result.skipped).toEqual(['a-renamed.txt']); + expect(result.reverted).toEqual([]); + // Rename must be left intact — user can resolve manually. + expect(fs.existsSync(path.join(REPO, 'a-renamed.txt'))).toBe(true); + }); + + it('skips rename-with-modification (RM status)', async () => { + run('git', ['mv', 'a.txt', 'a-renamed.txt']); + // Now modify the renamed file so git status shows RM + fs.writeFileSync(path.join(REPO, 'a-renamed.txt'), 'renamed-and-modified\n'); + + const result = await revertFilesAtProjectPath(REPO, ['a-renamed.txt']); + + expect(result.skipped).toEqual(['a-renamed.txt']); + expect(result.reverted).toEqual([]); + // Both sides of the rename remain intact. + expect(fs.readFileSync(path.join(REPO, 'a-renamed.txt'), 'utf8')).toBe('renamed-and-modified\n'); + }); + + it('handles filenames containing a literal " -> " sequence', async () => { + // NUL-delimited porcelain parsing should not confuse this with rename output. + const weirdName = 'weird -> name.txt'; + fs.writeFileSync(path.join(REPO, weirdName), 'original\n'); + run('git', ['add', '.']); + run('git', ['commit', '-q', '-m', 'add weird']); + + fs.writeFileSync(path.join(REPO, weirdName), 'agent-edit\n'); + + const result = await revertFilesAtProjectPath(REPO, [weirdName]); + + expect(result.success).toBe(true); + expect(result.reverted).toEqual([weirdName]); + expect(fs.readFileSync(path.join(REPO, weirdName), 'utf8')).toBe('original\n'); + }); + + it('fully restores a file with both staged and worktree modifications (MM)', async () => { + // First edit staged + fs.writeFileSync(path.join(REPO, 'a.txt'), 'staged-edit\n'); + run('git', ['add', 'a.txt']); + // Second edit unstaged — yields MM status + fs.writeFileSync(path.join(REPO, 'a.txt'), 'worktree-edit\n'); + + const result = await revertFilesAtProjectPath(REPO, ['a.txt']); + + expect(result.success).toBe(true); + expect(result.reverted).toEqual(['a.txt']); + // Both staged and worktree must be back at HEAD. + expect(fs.readFileSync(path.join(REPO, 'a.txt'), 'utf8')).toBe('original-a\n'); + const diff = execSync('git diff HEAD -- a.txt', { cwd: REPO, encoding: 'utf8' }); + expect(diff).toBe(''); + const staged = execSync('git diff --cached -- a.txt', { cwd: REPO, encoding: 'utf8' }); + expect(staged).toBe(''); + }); +}); diff --git a/server/routes/__tests__/revertAgentChangesRoute.test.js b/server/routes/__tests__/revertAgentChangesRoute.test.js new file mode 100644 index 00000000..b4c2cd4e --- /dev/null +++ b/server/routes/__tests__/revertAgentChangesRoute.test.js @@ -0,0 +1,145 @@ +/** + * Route-level smoke test for POST /api/git/revert-agent-changes. + * + * Uses an in-process Express app + the node http module so we don't need + * to pull in supertest as a new devDependency. Auth middleware is not + * mounted here — that layer has its own tests in the auth route suite. + */ +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import express from 'express'; +import http from 'http'; +import path from 'path'; +import fs from 'fs'; +import os from 'os'; +import { execSync } from 'child_process'; + +// Path to the per-test temp git repo (assigned in beforeEach). +let REPO; + +// Mock extractProjectDirectory so the route treats our temp repo as a project. +// vi.mock is hoisted, so REPO is read lazily inside the mock factory via ref. +vi.mock('../../projects.js', () => ({ + extractProjectDirectory: async () => REPO, +})); +// Avoid pulling in Claude SDK / cursor-cli side effects. +vi.mock('../../claude-sdk.js', () => ({ queryClaudeSDK: async () => ({}) })); +vi.mock('../../cursor-cli.js', () => ({ spawnCursor: async () => ({}) })); + +const { default: gitRouter } = await import('../git.js'); + +let server; +let baseUrl; + +function run(cmd, args) { + execSync(`${cmd} ${args.map(a => JSON.stringify(a)).join(' ')}`, { + cwd: REPO, + stdio: 'pipe', + }); +} + +function request(method, pathname, body) { + return new Promise((resolve, reject) => { + const data = body ? JSON.stringify(body) : ''; + const req = http.request( + { + method, + hostname: '127.0.0.1', + port: new URL(baseUrl).port, + path: pathname, + headers: { + 'Content-Type': 'application/json', + 'Content-Length': Buffer.byteLength(data), + }, + }, + (res) => { + let out = ''; + res.on('data', (chunk) => { out += chunk; }); + res.on('end', () => { + let parsed; + try { parsed = JSON.parse(out); } catch { parsed = out; } + resolve({ status: res.statusCode, body: parsed }); + }); + }, + ); + req.on('error', reject); + if (data) req.write(data); + req.end(); + }); +} + +beforeAll(async () => { + const app = express(); + app.use(express.json()); + app.use('/api/git', gitRouter); + await new Promise((resolve) => { + server = app.listen(0, '127.0.0.1', resolve); + }); + const { port } = server.address(); + baseUrl = `http://127.0.0.1:${port}`; +}); + +afterAll(async () => { + await new Promise((resolve) => server.close(resolve)); +}); + +beforeEach(() => { + REPO = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'revert-route-'))); + run('git', ['init', '-q', '-b', 'main']); + run('git', ['config', 'user.email', 'test@example.com']); + run('git', ['config', 'user.name', 'Test']); + run('git', ['config', 'commit.gpgsign', 'false']); + fs.writeFileSync(path.join(REPO, 'a.txt'), 'original\n'); + run('git', ['add', '.']); + run('git', ['commit', '-q', '-m', 'initial']); +}); + +afterEach(() => { + fs.rmSync(REPO, { recursive: true, force: true }); +}); + +describe('POST /api/git/revert-agent-changes', () => { + it('returns 400 when project is missing', async () => { + const res = await request('POST', '/api/git/revert-agent-changes', { files: ['a.txt'] }); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/project/i); + }); + + it('returns 400 when files is not an array', async () => { + const res = await request('POST', '/api/git/revert-agent-changes', { project: 'demo', files: 'a.txt' }); + expect(res.status).toBe(400); + }); + + it('returns success with empty arrays for empty file list', async () => { + const res = await request('POST', '/api/git/revert-agent-changes', { project: 'demo', files: [] }); + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ success: true, reverted: [], skipped: [], errors: [] }); + }); + + it('reverts a modified file end-to-end over HTTP', async () => { + fs.writeFileSync(path.join(REPO, 'a.txt'), 'agent-edit\n'); + + const res = await request('POST', '/api/git/revert-agent-changes', { + project: 'demo', + files: ['a.txt'], + }); + + expect(res.status).toBe(200); + expect(res.body.success).toBe(true); + expect(res.body.reverted).toEqual(['a.txt']); + expect(fs.readFileSync(path.join(REPO, 'a.txt'), 'utf8')).toBe('original\n'); + }); + + it('reports partial success when some files error', async () => { + fs.writeFileSync(path.join(REPO, 'a.txt'), 'agent-edit\n'); + + const res = await request('POST', '/api/git/revert-agent-changes', { + project: 'demo', + files: ['a.txt', '../outside.txt'], + }); + + expect(res.status).toBe(200); + expect(res.body.success).toBe(false); + expect(res.body.reverted).toEqual(['a.txt']); + expect(res.body.errors).toHaveLength(1); + }); +}); diff --git a/server/routes/git.js b/server/routes/git.js index 98cae6b9..74d63dad 100755 --- a/server/routes/git.js +++ b/server/routes/git.js @@ -5,6 +5,7 @@ import { promises as fs } from 'fs'; import { extractProjectDirectory } from '../projects.js'; import { queryClaudeSDK } from '../claude-sdk.js'; import { spawnCursor } from '../cursor-cli.js'; +import { safePath } from '../utils/safePath.js'; const router = express.Router(); @@ -1343,4 +1344,217 @@ router.post('/delete-untracked', async (req, res) => { } }); +/** + * Revert a list of files in a validated git project back to their last + * committed (HEAD) state, file-by-file. + * + * Semantics — intentionally conservative: + * - Each file is handled independently; errors on one do not affect others. + * - Untracked regular files the agent created are deleted. Directories + * and symlinks are *not* recursively removed even if listed — the caller + * would have to opt in explicitly (not currently exposed), because + * recursive deletion of an agent-chosen path is too dangerous to trigger + * from a single UI click. + * - Tracked modifications are restored with `git restore -- `. This + * overwrites the working-tree copy with HEAD; any user edits the agent + * did NOT trigger will be lost if they touched one of these same files. + * The UI must warn the user before calling this endpoint. + * - Rename (`R`) and copy (`C`) entries are currently skipped rather than + * guessed at — git's porcelain output would require additional parsing. + * + * Path traversal is blocked via `safePath()`; symlinked paths inside the + * project are additionally re-validated with `realpath()` so a symlink that + * points outside the repo cannot be used to unlink an external file. + * + * @returns {Promise<{success: boolean, reverted: string[], skipped: string[], errors: Array<{file: string, reason: string}>}>} + */ +export async function revertFilesAtProjectPath(projectPath, files) { + if (!Array.isArray(files) || files.length === 0) { + return { success: true, reverted: [], skipped: [], errors: [] }; + } + + const uniqueFiles = Array.from(new Set(files.filter(f => typeof f === 'string' && f.length > 0))); + const reverted = []; + const skipped = []; + const errors = []; + const normalizedRoot = path.resolve(projectPath); + + // Single porcelain snapshot of the whole repo so rename detection and + // per-file status are taken from the same instant. Uses -z so NUL + // delimits records and rename pairs — this means a filename containing + // a literal " -> " cannot confuse the parser. + const statusByPath = new Map(); // path -> 2-char status (XY) + const renamedPaths = new Set(); + let snapshotError = null; + try { + const { stdout: fullStatus } = await spawnAsync( + 'git', + ['status', '--porcelain', '-z'], + { cwd: projectPath }, + ); + + // In -z output, each record is "XY path\0", and for rename/copy entries + // the SOURCE path comes as an additional NUL-terminated record immediately + // after: "R. new\0old\0". We have to stream through the tokens instead of + // splitting once. + const tokens = fullStatus.split('\0'); + for (let i = 0; i < tokens.length; i++) { + const record = tokens[i]; + if (!record) continue; + if (record.length < 4) continue; // "XY " + path + const xy = record.slice(0, 2); + const newPath = record.slice(3).replace(/\/+$/, ''); + if ((xy[0] === 'R' || xy[0] === 'C') && i + 1 < tokens.length) { + const oldPath = (tokens[i + 1] || '').replace(/\/+$/, ''); + if (oldPath) renamedPaths.add(oldPath); + if (newPath) { + renamedPaths.add(newPath); + statusByPath.set(newPath, xy); + } + i += 1; // consume the source path token + continue; + } + if (newPath) statusByPath.set(newPath, xy); + } + } catch (err) { + snapshotError = err; + } + + // If the repo-wide snapshot failed, don't silently downgrade every + // input to "skipped" — that hides real breakage like a corrupt repo. + // Surface a single error per file so the caller can show a useful + // message. Individual file safety checks still run afterwards for + // the traversal-protection path so malicious inputs still get rejected. + if (snapshotError) { + for (const file of uniqueFiles) { + try { + safePath(file, projectPath); + } catch (safeErr) { + errors.push({ file, reason: safeErr.message || String(safeErr) }); + continue; + } + errors.push({ file, reason: `git status failed: ${getGitErrorText(snapshotError).trim()}` }); + } + return { success: false, reverted, skipped, errors }; + } + + for (const file of uniqueFiles) { + try { + const resolved = safePath(file, projectPath); + const relFile = path.relative(projectPath, resolved); + if (!relFile || relFile.startsWith('..')) { + errors.push({ file, reason: 'Path outside project root' }); + continue; + } + + if (renamedPaths.has(relFile)) { + // Part of a rename/copy pair. Reverting one side of a detected + // rename would leave the other side inconsistent; bail instead. + skipped.push(file); + continue; + } + + const xy = statusByPath.get(relFile) ?? ''; + if (!xy) { + skipped.push(file); + continue; + } + + const status = xy; + + if (status === '??') { + // Untracked — agent created this path. Use lstat so we see the + // symlink itself rather than its target, then revalidate after + // realpath for any regular files we intend to unlink. + let lstats; + try { + lstats = await fs.lstat(resolved); + } catch (statError) { + if (statError.code === 'ENOENT') { + skipped.push(file); + continue; + } + throw statError; + } + + if (lstats.isDirectory()) { + errors.push({ file, reason: 'Refusing to delete untracked directory via revert' }); + continue; + } + if (lstats.isSymbolicLink()) { + // Delete the symlink itself; do not follow it. The safePath + // check already bound the link path to the project root. + await fs.unlink(resolved); + } else { + // Regular file — re-check realpath points inside the repo in + // case the filesystem has cross-boundary symlinks we missed. + try { + const real = await fs.realpath(resolved); + if (real !== normalizedRoot && !real.startsWith(normalizedRoot + path.sep)) { + errors.push({ file, reason: 'Resolved path escapes project root via symlink' }); + continue; + } + } catch (realpathError) { + if (realpathError.code !== 'ENOENT') throw realpathError; + } + await fs.unlink(resolved); + } + } else { + const indexChar = status[0]; + const worktreeChar = status[1]; + + if (indexChar === 'A' && worktreeChar !== 'D') { + // Agent staged a new file (may or may not be further modified in + // worktree). Unstage the add and remove the working-tree copy. + await spawnAsync('git', ['reset', 'HEAD', '--', relFile], { cwd: projectPath }); + try { + await fs.unlink(resolved); + } catch (unlinkError) { + if (unlinkError.code !== 'ENOENT') throw unlinkError; + } + } else if ('MDT'.includes(indexChar) || 'MDT'.includes(worktreeChar) || status === 'AD') { + // Anything that existed at HEAD and has staged and/or worktree + // modifications, deletions, or type changes. --staged --worktree + // together restore BOTH the index and the working tree to HEAD + // so mixed states like MM / AM / AD / MD / DM all land back + // at the committed state. + await spawnAsync('git', ['restore', '--source=HEAD', '--staged', '--worktree', '--', relFile], { + cwd: projectPath, + }); + } else { + // Unrecognized porcelain code — skip rather than guess. + skipped.push(file); + continue; + } + } + + reverted.push(file); + } catch (fileError) { + errors.push({ file, reason: fileError.message || String(fileError) }); + } + } + + return { success: errors.length === 0, reverted, skipped, errors }; +} + +// Revert multiple files modified by a specific agent interaction. +router.post('/revert-agent-changes', async (req, res) => { + const { project, files } = req.body; + + if (!project || !Array.isArray(files)) { + return res.status(400).json({ error: 'Project name and files array are required' }); + } + + try { + const projectPath = await getActualProjectPath(project); + await validateGitRepository(projectPath); + + const result = await revertFilesAtProjectPath(projectPath, files); + res.json(result); + } catch (error) { + console.error('Git revert-agent-changes error:', error); + res.status(500).json({ error: error.message }); + } +}); + export default router; diff --git a/src/components/chat/utils/__tests__/agentModifiedFiles.test.ts b/src/components/chat/utils/__tests__/agentModifiedFiles.test.ts new file mode 100644 index 00000000..45f4f524 --- /dev/null +++ b/src/components/chat/utils/__tests__/agentModifiedFiles.test.ts @@ -0,0 +1,225 @@ +import { describe, it, expect } from 'vitest'; +import { getAgentModifiedFiles } from '../agentModifiedFiles'; +import type { AgentTurnItem } from '../groupAgentTurns'; +import type { ChatMessage } from '../../types/types'; + +function makeTurn(messages: ChatMessage[]): AgentTurnItem { + return { + kind: 'agent-turn', + textMessages: [], + intermediateMessages: [], + allMessages: messages, + toolCount: 0, + toolNames: [], + isActivelyStreaming: false, + }; +} + +/** Successful tool call — has a non-error toolResult (required to count). */ +function toolMsg( + toolName: string, + toolInput: unknown, + opts: { isError?: boolean; omitResult?: boolean } = {}, +): ChatMessage { + return { + type: 'assistant', + timestamp: new Date(), + isToolUse: true, + toolName, + toolInput, + toolResult: opts.omitResult ? null : opts.isError ? { isError: true } : { content: 'ok' }, + }; +} + +describe('getAgentModifiedFiles', () => { + it('returns empty list for a turn with no tool calls', () => { + const turn = makeTurn([ + { type: 'assistant', timestamp: new Date(), content: 'just text' }, + ]); + expect(getAgentModifiedFiles(turn)).toEqual([]); + }); + + it('collects file paths from Edit/Write/ApplyPatch object toolInput', () => { + const turn = makeTurn([ + toolMsg('Edit', { file_path: 'src/a.ts' }), + toolMsg('Write', { file_path: 'src/b.ts' }), + toolMsg('ApplyPatch', { file_path: 'src/c.ts' }), + ]); + expect(getAgentModifiedFiles(turn)).toEqual(['src/a.ts', 'src/b.ts', 'src/c.ts']); + }); + + it('parses stringified JSON toolInput (production shape from messageTransforms)', () => { + const turn = makeTurn([ + toolMsg('Edit', JSON.stringify({ file_path: 'src/a.ts', old_string: 'x', new_string: 'y' })), + ]); + expect(getAgentModifiedFiles(turn)).toEqual(['src/a.ts']); + }); + + it('collects multi-file file_paths array (ApplyPatch pattern)', () => { + const turn = makeTurn([ + toolMsg('ApplyPatch', { file_paths: ['src/a.ts', 'src/b.ts', 'src/c.ts'] }), + ]); + expect(getAgentModifiedFiles(turn)).toEqual(['src/a.ts', 'src/b.ts', 'src/c.ts']); + }); + + it('collects paths from toolResult.toolUseResult.changes (patch result)', () => { + const msg: ChatMessage = { + type: 'assistant', + timestamp: new Date(), + isToolUse: true, + toolName: 'ApplyPatch', + toolInput: {}, + toolResult: { + toolUseResult: { + changes: [ + { file_path: 'src/a.ts' }, + { file_path: 'src/b.ts' }, + ], + }, + }, + }; + const turn = makeTurn([msg]); + expect(getAgentModifiedFiles(turn)).toEqual(['src/a.ts', 'src/b.ts']); + }); + + it('deduplicates repeated file paths while preserving first-seen order', () => { + const turn = makeTurn([ + toolMsg('Edit', { file_path: 'src/a.ts' }), + toolMsg('Edit', { file_path: 'src/b.ts' }), + toolMsg('Edit', { file_path: 'src/a.ts' }), // duplicate + ]); + expect(getAgentModifiedFiles(turn)).toEqual(['src/a.ts', 'src/b.ts']); + }); + + it('ignores non-file-modifying tools', () => { + const turn = makeTurn([ + toolMsg('Read', { file_path: 'src/a.ts' }), + toolMsg('Bash', { command: 'ls' }), + toolMsg('Grep', { pattern: 'x', path: 'src/b.ts' }), + toolMsg('Edit', { file_path: 'src/c.ts' }), + ]); + expect(getAgentModifiedFiles(turn)).toEqual(['src/c.ts']); + }); + + it('skips tool calls that returned an error', () => { + const turn = makeTurn([ + toolMsg('Edit', { file_path: 'src/a.ts' }, { isError: true }), + toolMsg('Edit', { file_path: 'src/b.ts' }), + ]); + expect(getAgentModifiedFiles(turn)).toEqual(['src/b.ts']); + }); + + it('skips tool calls without a result (still streaming / aborted)', () => { + const turn = makeTurn([ + toolMsg('Edit', { file_path: 'src/pending.ts' }, { omitResult: true }), + toolMsg('Edit', { file_path: 'src/done.ts' }), + ]); + expect(getAgentModifiedFiles(turn)).toEqual(['src/done.ts']); + }); + + it('skips tool calls without a file_path', () => { + const turn = makeTurn([ + toolMsg('Edit', {}), + toolMsg('Write', { file_path: 'src/a.ts' }), + ]); + expect(getAgentModifiedFiles(turn)).toEqual(['src/a.ts']); + }); + + it('accepts aliases (MultiEdit, write_file, replace)', () => { + const turn = makeTurn([ + toolMsg('MultiEdit', { file_path: 'src/a.ts' }), + toolMsg('write_file', { file_path: 'src/b.ts' }), + toolMsg('replace', { file_path: 'src/c.ts' }), + ]); + expect(getAgentModifiedFiles(turn)).toEqual(['src/a.ts', 'src/b.ts', 'src/c.ts']); + }); + + it('also extracts from subagent child tools', () => { + const container: ChatMessage = { + type: 'assistant', + timestamp: new Date(), + isSubagentContainer: true, + subagentState: { + childTools: [ + { + toolId: 't1', + toolName: 'Edit', + toolInput: { file_path: 'src/sub.ts' }, + toolResult: { content: 'ok' }, + timestamp: new Date(), + }, + { + toolId: 't2', + toolName: 'Write', + toolInput: { file_path: 'src/other.ts' }, + toolResult: { isError: true }, + timestamp: new Date(), + }, + ], + currentToolIndex: 0, + isComplete: true, + }, + }; + const turn = makeTurn([container]); + expect(getAgentModifiedFiles(turn)).toEqual(['src/sub.ts']); + }); + + it('handles malformed stringified JSON gracefully', () => { + const turn = makeTurn([ + toolMsg('Edit', '{ this is not: "valid json" '), + toolMsg('Edit', { file_path: 'src/ok.ts' }), + ]); + expect(getAgentModifiedFiles(turn)).toEqual(['src/ok.ts']); + }); + + it('parses Codex FileChanges toolInput as newline-delimited "kind: path"', () => { + const turn = makeTurn([ + toolMsg('FileChanges', 'update: src/a.ts\nadd: src/b.ts\ndelete: src/c.ts'), + ]); + expect(getAgentModifiedFiles(turn)).toEqual(['src/a.ts', 'src/b.ts', 'src/c.ts']); + }); + + it('extracts paths from object-map changes (Codex patch-apply shape)', () => { + const msg: ChatMessage = { + type: 'assistant', + timestamp: new Date(), + isToolUse: true, + toolName: 'FileChanges', + toolInput: '', + toolResult: { + toolUseResult: { + changes: { + 'src/a.ts': { type: 'update' }, + 'src/b.ts': { type: 'add' }, + }, + }, + }, + }; + const turn = makeTurn([msg]); + expect(getAgentModifiedFiles(turn).sort()).toEqual(['src/a.ts', 'src/b.ts']); + }); + + it('ignores FileChanges with malformed lines', () => { + const turn = makeTurn([ + toolMsg('FileChanges', 'malformed line with no colon\nupdate: src/a.ts\n '), + ]); + expect(getAgentModifiedFiles(turn)).toEqual(['src/a.ts']); + }); + + it('accepts changes as a plain string[] (edge shape)', () => { + const msg: ChatMessage = { + type: 'assistant', + timestamp: new Date(), + isToolUse: true, + toolName: 'ApplyPatch', + toolInput: {}, + toolResult: { + toolUseResult: { + changes: ['src/a.ts', 'src/b.ts'], + }, + }, + }; + const turn = makeTurn([msg]); + expect(getAgentModifiedFiles(turn)).toEqual(['src/a.ts', 'src/b.ts']); + }); +}); diff --git a/src/components/chat/utils/agentModifiedFiles.ts b/src/components/chat/utils/agentModifiedFiles.ts new file mode 100644 index 00000000..3614c011 --- /dev/null +++ b/src/components/chat/utils/agentModifiedFiles.ts @@ -0,0 +1,166 @@ +/** + * Extract the set of files an agent turn modified via Edit/Write/ApplyPatch + * tool calls. Used by the "Revert agent changes" button so the backend only + * touches files this specific interaction created or edited. + * + * `message.toolInput` is often a JSON string (see messageTransforms.ts + * normalizeToolInput) rather than an object, and patch tools may carry + * multi-file `file_paths` / `paths` arrays — the extractor handles both. + */ + +import type { ChatMessage } from '../types/types'; +import type { AgentTurnItem } from './groupAgentTurns'; + +const FILE_MODIFYING_TOOLS = new Set([ + 'Edit', + 'Write', + 'ApplyPatch', + 'MultiEdit', + 'write_file', + 'replace', + // Codex normalizes patch-apply events into a synthetic 'FileChanges' tool + // whose toolInput is a newline-separated string of "kind: path" lines + // (see server/utils/codexSessionMessages.js toFileChangesToolInput). + 'FileChanges', +]); + +interface ToolResultLike { + isError?: boolean; + content?: unknown; + toolUseResult?: unknown; + [key: string]: unknown; +} + +function parseJson(value: unknown): any { + if (value === null || value === undefined) return null; + if (typeof value === 'object') return value; + if (typeof value !== 'string') return null; + const trimmed = value.trim(); + if (!trimmed || (!trimmed.startsWith('{') && !trimmed.startsWith('['))) return null; + try { + return JSON.parse(trimmed); + } catch { + return null; + } +} + +function addPath(raw: unknown, seen: Set, out: string[]): void { + if (typeof raw !== 'string') return; + const trimmed = raw.trim(); + if (!trimmed || seen.has(trimmed)) return; + seen.add(trimmed); + out.push(trimmed); +} + +/** Parse Codex's newline-delimited "kind: path" FileChanges payload. */ +function parseFileChangesString(raw: string, seen: Set, out: string[]): void { + for (const line of raw.split('\n')) { + const trimmed = line.trim(); + if (!trimmed) continue; + const sep = trimmed.indexOf(':'); + if (sep === -1) continue; + addPath(trimmed.slice(sep + 1).trim(), seen, out); + } +} + +function collectPathsFromValue(value: unknown, seen: Set, out: string[]): void { + if (value === null || value === undefined) return; + + // Handle Codex's raw "kind: path\nkind: path" string shape. + if (typeof value === 'string') { + const trimmed = value.trim(); + if (!trimmed) return; + // Try JSON first; fall back to kind:path line parsing. + if (trimmed.startsWith('{') || trimmed.startsWith('[')) { + const parsed = parseJson(trimmed); + if (parsed) collectPathsFromValue(parsed, seen, out); + return; + } + parseFileChangesString(trimmed, seen, out); + return; + } + + if (typeof value !== 'object') return; + const record = value as Record; + + const scalarKeys = ['file_path', 'path', 'filePath', 'absolutePath', 'relativePath']; + for (const key of scalarKeys) { + addPath(record[key], seen, out); + } + + const arrayKeys = ['file_paths', 'paths']; + for (const key of arrayKeys) { + const list = record[key]; + if (Array.isArray(list)) { + for (const v of list) addPath(v, seen, out); + } + } + + // `changes` / `FileChanges` / `fileChanges` can be either: + // - an array of { file_path: ... } entries (Claude-style) + // - an object map { "src/a.ts": {type: "update"}, "src/b.ts": {...} } + // (Codex patch-apply payload — keys are the paths) + const changeKeys = ['changes', 'FileChanges', 'fileChanges']; + for (const key of changeKeys) { + const sub = record[key]; + if (Array.isArray(sub)) { + for (const entry of sub) { + if (entry && typeof entry === 'object') { + collectPathsFromValue(entry, seen, out); + } else if (typeof entry === 'string') { + addPath(entry, seen, out); + } + } + } else if (sub && typeof sub === 'object') { + for (const pathKey of Object.keys(sub)) { + addPath(pathKey, seen, out); + } + } + } +} + +function isToolCallReverted(result?: ToolResultLike | null): boolean { + if (!result) return true; // no result yet = not committed, skip + if (result.isError) return true; + return false; +} + +/** + * Return the deduplicated list of file paths a turn modified, in the order + * they first appear. Tool calls that errored out or have no result are + * skipped — they never actually changed the filesystem. + */ +export function getAgentModifiedFiles(turn: AgentTurnItem): string[] { + const seen = new Set(); + const files: string[] = []; + + const collect = ( + toolName: string | undefined, + toolInput: unknown, + toolResult?: ToolResultLike | null, + ) => { + if (!toolName || !FILE_MODIFYING_TOOLS.has(toolName)) return; + if (isToolCallReverted(toolResult)) return; + + collectPathsFromValue(toolInput, seen, files); + + // Some patch flows only populate file paths in the result payload. + if (toolResult) { + collectPathsFromValue(toolResult.content, seen, files); + collectPathsFromValue(toolResult.toolUseResult, seen, files); + } + }; + + for (const message of turn.allMessages) { + if (message.isToolUse) { + collect(message.toolName, message.toolInput, message.toolResult); + } + if (message.isSubagentContainer && message.subagentState?.childTools) { + for (const child of message.subagentState.childTools) { + collect(child.toolName, child.toolInput, child.toolResult); + } + } + } + + return files; +} diff --git a/src/components/chat/view/subcomponents/AgentTurnContainer.tsx b/src/components/chat/view/subcomponents/AgentTurnContainer.tsx index fd8050b6..48609624 100644 --- a/src/components/chat/view/subcomponents/AgentTurnContainer.tsx +++ b/src/components/chat/view/subcomponents/AgentTurnContainer.tsx @@ -1,5 +1,7 @@ import { useTranslation } from 'react-i18next'; import MessageComponent from './MessageComponent'; +import RevertAgentChangesButton from './RevertAgentChangesButton'; +import { getAgentModifiedFiles } from '../../utils/agentModifiedFiles'; import type { AgentTurnItem } from '../../utils/groupAgentTurns'; import type { ChatMessage } from '../../types/types'; import type { Project } from '../../../../types/app'; @@ -108,6 +110,10 @@ export default function AgentTurnContainer({ const hasIntermediate = turn.intermediateMessages.length > 0; + // Files the agent modified in this turn — used to offer one-click revert. + // Only shown when the turn has finished streaming to avoid racing the agent. + const modifiedFiles = getAgentModifiedFiles(turn); + // Build summary text const summaryParts: string[] = []; @@ -171,6 +177,11 @@ export default function AgentTurnContainer({ {turn.textMessages.map((msg, i) => renderMessage(msg, i, i > 0 ? turn.textMessages[i - 1] : null, false) )} + {modifiedFiles.length > 0 && ( +
+ +
+ )} ); } diff --git a/src/components/chat/view/subcomponents/RevertAgentChangesButton.tsx b/src/components/chat/view/subcomponents/RevertAgentChangesButton.tsx new file mode 100644 index 00000000..dc6df9c8 --- /dev/null +++ b/src/components/chat/view/subcomponents/RevertAgentChangesButton.tsx @@ -0,0 +1,197 @@ +import { useEffect, useRef, useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import { authenticatedFetch } from '../../../../utils/api'; +import type { Project } from '../../../../types/app'; + +interface RevertAgentChangesButtonProps { + files: string[]; + project: Project; +} + +interface RevertResponse { + success: boolean; + reverted?: string[]; + skipped?: string[]; + errors?: Array<{ file: string; reason: string }>; + error?: string; +} + +type RevertState = 'idle' | 'confirming' | 'reverting' | 'done' | 'error'; + +export default function RevertAgentChangesButton({ files, project }: RevertAgentChangesButtonProps) { + const { t } = useTranslation('chat'); + const [state, setState] = useState('idle'); + const [result, setResult] = useState(null); + const confirmButtonRef = useRef(null); + const idleButtonRef = useRef(null); + + // When opening the confirmation, move focus to the destructive action so + // keyboard users can see it's the focused control; Escape cancels. + useEffect(() => { + if (state === 'confirming' && confirmButtonRef.current) { + confirmButtonRef.current.focus(); + } + }, [state]); + + useEffect(() => { + if (state !== 'confirming') return; + const onKey = (e: KeyboardEvent) => { + if (e.key === 'Escape') { + e.preventDefault(); + setState('idle'); + // restore focus to the original trigger + requestAnimationFrame(() => idleButtonRef.current?.focus()); + } + }; + window.addEventListener('keydown', onKey); + return () => window.removeEventListener('keydown', onKey); + }, [state]); + + if (files.length === 0) return null; + + const handleRevert = async () => { + setState('reverting'); + try { + const response = await authenticatedFetch('/api/git/revert-agent-changes', { + method: 'POST', + body: JSON.stringify({ + project: project.name, + files, + }), + }); + const data: RevertResponse = await response.json(); + if (!response.ok) { + setResult({ success: false, error: data?.error || `HTTP ${response.status}` }); + setState('error'); + return; + } + setResult(data); + setState(data.success ? 'done' : 'error'); + } catch (error) { + setResult({ + success: false, + error: error instanceof Error ? error.message : String(error), + }); + setState('error'); + } + }; + + const fileCount = files.length; + const buttonLabel = t('revertAgent.button', { defaultValue: 'Revert changes' }); + // Explicit about semantics: we restore to HEAD (last commit), which can + // also undo manual edits to the same files. The issue asked for + // preserving edits to *other* files, which we do. + const confirmLabel = t('revertAgent.confirmLabel', { + count: fileCount, + defaultValue: fileCount === 1 + ? 'Restore 1 file to its last committed state? Any manual edits to this file will also be lost.' + : `Restore ${fileCount} files to their last committed state? Any manual edits to these files will also be lost.`, + }); + + return ( +
+ {state === 'idle' && ( + + )} + + {state === 'confirming' && ( + // Non-modal inline confirmation: role=group keeps the semantics honest + // (no focus trap, no backdrop), while aria-labelledby points screen + // readers at the warning text and focus moves to the destructive + // action so keyboard users see what pressing Enter will do. +
+ {confirmLabel} + + +
+ )} + + {state === 'reverting' && ( + + + {t('revertAgent.inProgress', { defaultValue: 'Reverting...' })} + + )} + + {state === 'done' && result && ( + + + {t('revertAgent.done', { + count: result.reverted?.length ?? 0, + defaultValue: `Reverted ${result.reverted?.length ?? 0} file(s)`, + })} + {(result.skipped?.length ?? 0) > 0 && ( + + · {t('revertAgent.skipped', { + count: result.skipped!.length, + defaultValue: `${result.skipped!.length} unchanged`, + })} + + )} + + )} + + {state === 'error' && result && ( + `${e.file}: ${e.reason}`).join('\n')} + > + + {t('revertAgent.error', { defaultValue: 'Revert failed' })} + {(result.errors?.length ?? 0) > 0 && ( + ({result.errors!.length}) + )} + + )} +
+ ); +} diff --git a/src/i18n/locales/en/chat.json b/src/i18n/locales/en/chat.json index e441abbe..50e6f964 100644 --- a/src/i18n/locales/en/chat.json +++ b/src/i18n/locales/en/chat.json @@ -79,6 +79,20 @@ "steps": "Agent steps", "streaming": "Streaming..." }, + "revertAgent": { + "button": "Revert changes", + "tooltip": "Discard changes the agent made to {{count}} file(s)", + "confirmLabel_one": "Restore 1 file to its last committed state? Any manual edits to this file will also be lost.", + "confirmLabel_other": "Restore {{count}} files to their last committed state? Any manual edits to these files will also be lost.", + "confirm": "Revert", + "cancel": "Cancel", + "inProgress": "Reverting...", + "done_one": "Reverted 1 file", + "done_other": "Reverted {{count}} files", + "skipped_one": "1 unchanged", + "skipped_other": "{{count}} unchanged", + "error": "Revert failed" + }, "skill": { "contentLoaded": "Skill Content Loaded" }, diff --git a/src/i18n/locales/ko/chat.json b/src/i18n/locales/ko/chat.json index 301d8ed0..35f7edd0 100644 --- a/src/i18n/locales/ko/chat.json +++ b/src/i18n/locales/ko/chat.json @@ -79,6 +79,20 @@ "steps": "에이전트 단계", "streaming": "스트리밍 중..." }, + "revertAgent": { + "button": "변경 사항 되돌리기", + "tooltip": "에이전트가 {{count}}개 파일에 적용한 변경을 취소", + "confirmLabel_one": "1개 파일을 마지막 커밋 상태로 되돌릴까요? 해당 파일에 대한 수동 편집도 함께 손실됩니다.", + "confirmLabel_other": "{{count}}개 파일을 마지막 커밋 상태로 되돌릴까요? 해당 파일들에 대한 수동 편집도 함께 손실됩니다.", + "confirm": "되돌리기", + "cancel": "취소", + "inProgress": "되돌리는 중...", + "done_one": "1개 파일 되돌림", + "done_other": "{{count}}개 파일 되돌림", + "skipped_one": "1개 변경 없음", + "skipped_other": "{{count}}개 변경 없음", + "error": "되돌리기 실패" + }, "skill": { "contentLoaded": "스킬 콘텐츠 로드됨" }, diff --git a/src/i18n/locales/zh-CN/chat.json b/src/i18n/locales/zh-CN/chat.json index 9c557ea2..f12ad43a 100644 --- a/src/i18n/locales/zh-CN/chat.json +++ b/src/i18n/locales/zh-CN/chat.json @@ -79,6 +79,20 @@ "steps": "执行步骤", "streaming": "传输中..." }, + "revertAgent": { + "button": "回滚更改", + "tooltip": "丢弃代理对 {{count}} 个文件的修改", + "confirmLabel_one": "将 1 个文件恢复到最近一次提交的状态？对该文件的手动修改也会丢失。", + "confirmLabel_other": "将 {{count}} 个文件恢复到最近一次提交的状态？对这些文件的手动修改也会丢失。", + "confirm": "回滚", + "cancel": "取消", + "inProgress": "正在回滚...", + "done_one": "已回滚 1 个文件", + "done_other": "已回滚 {{count}} 个文件", + "skipped_one": "1 个无变动", + "skipped_other": "{{count}} 个无变动", + "error": "回滚失败" + }, "skill": { "contentLoaded": "技能内容已加载" },