From b4eabd3993d93bd35870c2cc5c107dbfe9ecd189 Mon Sep 17 00:00:00 2001 From: Cedric Karungu Date: Sun, 4 Oct 2026 15:27:54 +0200 Subject: [PATCH] fix(core): keep a milestone before every command that destroys work --- .../checkpoint-before-every-destroyer.md | 10 ++++ packages/core/src/recovery/destructive.ts | 60 ++++++++++++++++++- packages/core/test/checkpoint.test.ts | 22 +++++++ 3 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 .changeset/checkpoint-before-every-destroyer.md diff --git a/.changeset/checkpoint-before-every-destroyer.md b/.changeset/checkpoint-before-every-destroyer.md new file mode 100644 index 00000000..ff7fcf09 --- /dev/null +++ b/.changeset/checkpoint-before-every-destroyer.md @@ -0,0 +1,10 @@ +--- +'@memnox/core': patch +'@memnox/proxy': patch +'@memnox/interceptors': patch +'memnox': patch +--- + +A milestone is kept before every command that destroys work, not only `rm`, `mv` and four git subcommands. `shred`, `unlink` and `rmdir` were already destructive to the seam that rules on them and invisible to the one that keeps a tree first, so an agent was asked about them and then ran them with nothing to rewind to. `truncate` to a size that is not an extension, `git checkout --force`, `git switch --discard-changes`, `git switch -f`, `git stash drop` and `git stash clear` join them, and `--force` is now accepted wherever `-f` already was. + +A dropped stash is not in the tree a milestone keeps, so that row marks when the work went rather than offering to bring it back. diff --git a/packages/core/src/recovery/destructive.ts b/packages/core/src/recovery/destructive.ts index 210e882b..1535c253 100644 --- a/packages/core/src/recovery/destructive.ts +++ b/packages/core/src/recovery/destructive.ts @@ -31,6 +31,22 @@ const WORKTREE_FLAGS: readonly string[] = ['-W', '--worktree']; const RECURSIVE_FLAG = /^-[a-zA-Z]*[rR]/; +/** Removers that `rm` is not, which delete what they are given just the same. */ +const REMOVERS: readonly string[] = ['rmdir', 'unlink', 'shred']; + +const SIZE_FLAGS: readonly string[] = ['-s', '--size']; + +/** `+n` only extends a file, so nothing already written is lost. */ +const EXTENDS_FILE = /^\+/; + +const FORCE_FLAGS: readonly string[] = ['-f', '--force']; + +/** `switch` spells the same discard three ways. */ +const DISCARD_FLAGS: readonly string[] = [...FORCE_FLAGS, '--discard-changes']; + +/** The stash subcommands that throw an entry away rather than applying it. */ +const STASH_DESTROYS: readonly string[] = ['drop', 'clear']; + function operandsOf(args: readonly string[]): string[] { const at = args.indexOf('--'); const before = at === -1 ? args : args.slice(0, at); @@ -45,6 +61,33 @@ function removal(args: readonly string[]): DestructiveCommand | null { return { note: `before ${recursive ? 'rm -r' : 'rm'} ${operands.join(' ')}`, operands }; } +/** `rmdir`, `unlink` and `shred`, which `intercept/writers.ts` already calls destructive. */ +function removedBy(binary: string, args: readonly string[]): DestructiveCommand | null { + const operands = operandsOf(args); + if (operands.length === 0) return null; + return { note: `before ${binary} ${operands.join(' ')}`, operands }; +} + +/** The size asked for, however it was spelled, or null when none was. */ +function sizeAsked(args: readonly string[]): string | null { + const glued = args.find((arg) => arg.startsWith('--size=') || /^-s./.test(arg)); + if (glued !== undefined) return glued.replace(/^(--size=|-s)/, ''); + const at = args.findIndex((arg) => SIZE_FLAGS.includes(arg)); + return at === -1 ? null : (args[at + 1] ?? null); +} + +function truncation(args: readonly string[]): DestructiveCommand | null { + const size = sizeAsked(args); + // No size shrinks nothing, and an extension leaves what is there alone. + if (size === null || EXTENDS_FILE.test(size)) return null; + const at = args.findIndex((arg) => SIZE_FLAGS.includes(arg)); + // The size is a value rather than a path, so it is not one of the files named. + const named = at === -1 ? args : [...args.slice(0, at + 1), ...args.slice(at + 2)]; + const operands = operandsOf(named); + if (operands.length === 0) return null; + return { note: `before truncate ${operands.join(' ')}`, operands }; +} + function massMove(args: readonly string[]): DestructiveCommand | null { const operands = operandsOf(args); const sources = operands.slice(0, -1); @@ -67,7 +110,11 @@ function gitSubcommand(args: readonly string[]): readonly string[] { /** Every path, `.` included, is a tree-wide discard once `--` or a bare `.` names it. */ function discardsFiles(rest: readonly string[]): boolean { - return rest.includes('--') || rest.includes('.') || rest.includes('-f'); + return ( + rest.includes('--') || + rest.includes('.') || + rest.some((arg) => FORCE_FLAGS.includes(arg)) + ); } function gitDestroys(args: readonly string[]): DestructiveCommand | null { @@ -85,6 +132,11 @@ function gitDestroys(args: readonly string[]): DestructiveCommand | null { rest.includes('--staged') && !rest.some((arg) => WORKTREE_FLAGS.includes(arg)); if (subcommand === 'restore' && !indexOnly) return { note: `before git restore ${rest.join(' ')}`.trim(), ...tree }; + if (subcommand === 'switch' && rest.some((arg) => DISCARD_FLAGS.includes(arg))) + return { note: `before git switch ${rest.join(' ')}`.trim(), ...tree }; + // The stash is not in the tree a milestone keeps, so this marks when rather than undoes it. + if (subcommand === 'stash' && STASH_DESTROYS.includes(rest[0] ?? '')) + return { note: `before git stash ${rest[0] ?? ''}`.trim(), ...tree }; return null; } @@ -92,9 +144,13 @@ function gitDestroys(args: readonly string[]): DestructiveCommand | null { export function destructiveCommand(argv: readonly string[]): DestructiveCommand | null { const [binary, ...args] = argv; if (binary === undefined) return null; - switch (basename(binary)) { + const name = basename(binary); + if (REMOVERS.includes(name)) return removedBy(name, args); + switch (name) { case 'rm': return removal(args); + case 'truncate': + return truncation(args); case 'mv': return massMove(args); case 'git': diff --git a/packages/core/test/checkpoint.test.ts b/packages/core/test/checkpoint.test.ts index 06dd3144..b012dd1e 100644 --- a/packages/core/test/checkpoint.test.ts +++ b/packages/core/test/checkpoint.test.ts @@ -48,6 +48,21 @@ describe('which commands destroy work', () => { [['git', 'restore', '.'], 'before git restore .'], [['mv', 'a', 'b', 'c', 'dest/'], 'before mv of 3 path(s)'], [['mv', 'src/*', 'old/'], 'before mv of 1 path(s)'], + // `intercept/writers.ts` already calls these destructive, so recovery was the half that disagreed. + [['shred', '-u', 'secrets.txt'], 'before shred secrets.txt'], + [['unlink', 'notes.md'], 'before unlink notes.md'], + [['rmdir', 'build'], 'before rmdir build'], + [['truncate', '-s', '0', 'app.log'], 'before truncate app.log'], + [['truncate', '--size=0', 'app.log'], 'before truncate app.log'], + [['truncate', '-s0', 'app.log'], 'before truncate app.log'], + [['git', 'checkout', '--force'], 'before git checkout --force'], + [ + ['git', 'switch', '--discard-changes', 'main'], + 'before git switch --discard-changes main', + ], + [['git', 'switch', '-f', 'main'], 'before git switch -f main'], + [['git', 'stash', 'drop'], 'before git stash drop'], + [['git', 'stash', 'clear'], 'before git stash clear'], ])('%j is kept before', (argv, note) => { expect(destructiveCommand(argv)?.note).toBe(note); }); @@ -60,6 +75,13 @@ describe('which commands destroy work', () => { [['git', 'reset', 'HEAD']], [['mv', 'a', 'b']], [['ls', '-la']], + // An extension leaves every byte already written where it was. + [['truncate', '-s', '+1M', 'app.log']], + [['truncate', 'app.log']], + [['git', 'switch', 'main']], + [['git', 'stash']], + [['git', 'stash', 'pop']], + [['shred']], ])('%j destroys nothing a milestone could keep', (argv) => { expect(destructiveCommand(argv)).toBeNull(); });