fix(act): support directory moves for archive actions

The archive actions in the acting epic move whole skill/agent directories,
which the framework could not handle: snapshotFile used copyFile (EISDIR on
a directory) and the afterHash pass used readFile. Snapshots now branch on
lstat, copying directory trees with fs.cp recursive. Directories get an
empty afterHash ('' means no content hash) and drift detection skips the
hash comparison for them; the occupied-original-path check still applies and
a missing movedTo still falls back to the backup. Backup restore likewise
branches: a directory snapshot replaces the target (rm then cp recursive).

Apply-side rename cannot replace a directory destination (ENOTEMPTY), so a
move retries once after clearing the already-snapshotted destination,
rethrowing other codes before any destination damage.

Tests: archive a directory tree and restore it byte-identical, move a
directory onto an existing destination directory (destBackup taken, both
trees restored), and dir-move undo with an occupied original path refusing
without --force and overwriting with it.
This commit is contained in:
AgentSeal 2026-07-03 10:14:48 +02:00
parent d756eacae5
commit 2eec2fdc31
4 changed files with 104 additions and 8 deletions

View file

@ -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,

View file

@ -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<boolean> {
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<boolean> {
// overwrote an existing file and an edit of a missing file restore correctly.
export async function revertChange(actionsDir: string, change: FileChange): Promise<void> {
const restore = async (backup: string, to: string): Promise<void> => {
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!)) {

View file

@ -32,6 +32,7 @@ async function driftedFiles(record: ActionRecord): Promise<string[]> {
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)

View file

@ -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 {