diff --git a/src/act/apply.ts b/src/act/apply.ts index 56bbf68..8f4b507 100644 --- a/src/act/apply.ts +++ b/src/act/apply.ts @@ -1,4 +1,4 @@ -import { mkdir, rename, rm, writeFile } from 'fs/promises' +import { lstat, mkdir, rename, rm, writeFile } from 'fs/promises' import { dirname, join } from 'path' import { randomUUID } from 'crypto' import type { ActionPlan, ActionRecord, FileChange } from './types.js' @@ -45,7 +45,17 @@ export async function runAction(plan: ActionPlan, actionsDir: string = defaultAc const pc = plan.changes[i]! if (pc.op === 'move') { await mkdir(dirname(pc.movedTo), { recursive: true }) - await rename(pc.path, pc.movedTo) + try { + await rename(pc.path, pc.movedTo) + } catch (err) { + // rename cannot replace a directory destination. It is already + // snapshotted (destBackup), so clear it and retry. Other codes + // (e.g. a missing source) rethrow before any destination damage. + const code = (err as NodeJS.ErrnoException).code + if (code !== 'ENOTEMPTY' && code !== 'EEXIST' && code !== 'EISDIR' && code !== 'ENOTDIR') throw err + await rm(pc.movedTo, { recursive: true, force: true }) + await rename(pc.path, pc.movedTo) + } } else { await mkdir(dirname(pc.path), { recursive: true }) await writeFile(pc.path, pc.content) @@ -53,8 +63,11 @@ export async function runAction(plan: ActionPlan, actionsDir: string = defaultAc done.push(i) } // Hash after ALL mutations so overlapping changes carry the final state. + // Directories get '' (no content hash); drift detection skips them. for (const change of changes) { - change.afterHash = (await sha256File(change.op === 'move' ? change.movedTo! : change.path)) ?? '' + const p = change.op === 'move' ? change.movedTo! : change.path + const st = await lstat(p).catch(() => null) + change.afterHash = st && !st.isDirectory() ? (await sha256File(p)) ?? '' : '' } const record: ActionRecord = { id, diff --git a/src/act/backup.ts b/src/act/backup.ts index 6c8709f..740ad99 100644 --- a/src/act/backup.ts +++ b/src/act/backup.ts @@ -1,4 +1,4 @@ -import { copyFile, lstat, mkdir, readFile, rename, rm } from 'fs/promises' +import { copyFile, cp, lstat, mkdir, readFile, rename, rm } from 'fs/promises' import { createHash } from 'crypto' import { dirname, join } from 'path' import type { FileChange } from './types.js' @@ -11,11 +11,13 @@ export function relBackupPath(id: string, index: number): string { return `backups/${id}/${index}.bak` } -// Copy src to dest if src exists; return whether it existed so the caller can -// record backup: null for a create. +// Snapshot src (file or directory tree) to dest if it exists; return whether +// it existed so the caller can record backup: null for a create. export async function snapshotFile(src: string, dest: string): Promise { try { - await copyFile(src, dest) + const st = await lstat(src) + if (st.isDirectory()) await cp(src, dest, { recursive: true }) + else await copyFile(src, dest) return true } catch (err) { if ((err as NodeJS.ErrnoException).code === 'ENOENT') return false @@ -50,8 +52,14 @@ export async function pathExists(path: string): Promise { // overwrote an existing file and an edit of a missing file restore correctly. export async function revertChange(actionsDir: string, change: FileChange): Promise { const restore = async (backup: string, to: string): Promise => { + const src = join(actionsDir, backup) await mkdir(dirname(to), { recursive: true }) - await copyFile(join(actionsDir, backup), to) + if ((await lstat(src)).isDirectory()) { + await rm(to, { recursive: true, force: true }) + await cp(src, to, { recursive: true }) + } else { + await copyFile(src, to) + } } if (change.op === 'move') { if (await pathExists(change.movedTo!)) { diff --git a/src/act/undo.ts b/src/act/undo.ts index aa7ca90..570ca1e 100644 --- a/src/act/undo.ts +++ b/src/act/undo.ts @@ -32,6 +32,7 @@ async function driftedFiles(record: ActionRecord): Promise { if (change.op === 'move' && await pathExists(change.path)) { drifted.push(`${change.path} (occupied, undo would overwrite it)`) } + if (change.afterHash === '') continue // no content hash (directories) const p = currentPath(change) try { if ((await sha256File(p)) !== change.afterHash) drifted.push(p) diff --git a/tests/act-undo.test.ts b/tests/act-undo.test.ts index d0890a9..3d37012 100644 --- a/tests/act-undo.test.ts +++ b/tests/act-undo.test.ts @@ -218,6 +218,80 @@ describe('undoAction', () => { await expect(undoAction({ id: 'aaaaaaaa' }, { actionsDir })).rejects.toThrow(/matches 2 actions/) }) + + it('archives a directory and undo restores the tree with nested content byte-identical', async () => { + const { actionsDir, files } = await makeRoot() + const dir = join(files, 'skill') + const dest = join(files, '.archived', 'skill') + const nestedBytes = Buffer.from([1, 2, 3, 250, 0]) + await mkdir(join(dir, 'nested'), { recursive: true }) + await writeFile(join(dir, 'SKILL.md'), 'skill body') + await writeFile(join(dir, 'nested', 'data.bin'), nestedBytes) + + const rec = await runAction({ + kind: 'archive-skill', + description: 'archive dir', + changes: [{ op: 'move', path: dir, movedTo: dest }], + }, actionsDir) + expect(rec.changes[0]!.afterHash).toBe('') + expect(rec.changes[0]!.backup).not.toBeNull() + expect(existsSync(dir)).toBe(false) + expect(await readFile(join(dest, 'SKILL.md'), 'utf-8')).toBe('skill body') + + await undoAction({ id: rec.id }, { actionsDir }) + expect(existsSync(dest)).toBe(false) + expect(await readFile(join(dir, 'SKILL.md'), 'utf-8')).toBe('skill body') + expect(Buffer.compare(await readFile(join(dir, 'nested', 'data.bin')), nestedBytes)).toBe(0) + }) + + it('moves a directory onto an existing destination directory and undo restores both trees', async () => { + const { actionsDir, files } = await makeRoot() + const src = join(files, 'agent') + const dest = join(files, 'agent-archived') + await mkdir(src, { recursive: true }) + await mkdir(dest, { recursive: true }) + await writeFile(join(src, 'agent.md'), 'src tree') + await writeFile(join(dest, 'old.md'), 'dest tree') + + const rec = await runAction({ + kind: 'archive-agent', + description: 'dir onto dir', + changes: [{ op: 'move', path: src, movedTo: dest }], + }, actionsDir) + expect(rec.changes[0]!.destBackup).not.toBeNull() + expect(await readFile(join(dest, 'agent.md'), 'utf-8')).toBe('src tree') + expect(existsSync(join(dest, 'old.md'))).toBe(false) + + await undoAction({ id: rec.id }, { actionsDir }) + expect(await readFile(join(src, 'agent.md'), 'utf-8')).toBe('src tree') + expect(await readFile(join(dest, 'old.md'), 'utf-8')).toBe('dest tree') + expect(existsSync(join(dest, 'agent.md'))).toBe(false) + }) + + it('refuses dir-move undo when the original path is occupied, then --force overwrites', async () => { + const { actionsDir, files } = await makeRoot() + const src = join(files, 'cmd') + const dest = join(files, 'cmd-archived') + await mkdir(src, { recursive: true }) + await writeFile(join(src, 'cmd.md'), 'body') + const rec = await runAction({ + kind: 'archive-command', + description: 'occupied dir', + changes: [{ op: 'move', path: src, movedTo: dest }], + }, actionsDir) + + await mkdir(src, { recursive: true }) + await writeFile(join(src, 'squatter.md'), 'squatter') + + const err = await undoAction({ id: rec.id }, { actionsDir }).catch(e => e) + expect(err).toBeInstanceOf(DriftError) + expect((err as DriftError).drifted.some(d => d.includes(src))).toBe(true) + + await undoAction({ id: rec.id }, { actionsDir, force: true }) + expect(await readFile(join(src, 'cmd.md'), 'utf-8')).toBe('body') + expect(existsSync(join(src, 'squatter.md'))).toBe(false) + expect(existsSync(dest)).toBe(false) + }) }) function bareRecord(id: string, description: string): ActionRecord {