Skip to content

fix(core): keep a milestone before every command that destroys work - #126

Open
Cedric921 wants to merge 1 commit into
Memnox:mainfrom
Cedric921:fix/checkpoint-before-every-destroyer
Open

Cedric921 wants to merge 1 commit into
Memnox:mainfrom
Cedric921:fix/checkpoint-before-every-destroyer

Conversation

@Cedric921

Copy link
Copy Markdown
Contributor

Closes #49.

What this changes

The two halves disagreed about what destroys work. intercept/writers.ts:30 has had rmdir, unlink and shred in REMOVERS as destructive all along, while recovery/destructive.ts only knew rm, mv and four git subcommands. So an agent was asked about shred secrets.txt, said yes, and ran it with no milestone to rewind to:

shred -u secrets.txt      recovery=null   writers=destructive
unlink notes.md           recovery=null   writers=destructive
rmdir build               recovery=null   writers=destructive
truncate -s 0 app.log     recovery=null   writers=write
git checkout --force      recovery=null
git switch --discard-changes main   recovery=null
git stash drop            recovery=null

Each of those now keeps a tree first. --force is also accepted wherever -f already was, which is what git checkout --force was falling through.

Commands that now keep a milestone: rmdir, unlink, shred, truncate to a size that is not an extension, git checkout --force, git switch --discard-changes, git switch -f, git stash drop, git stash clear.

Nothing moved the other way. truncate -s +1M keeps none, because an extension leaves every byte already written where it was; git switch main, git stash and git stash pop keep none either.

One thing to decide

A dropped stash is not in the tree a milestone keeps. refs/memnox/milestones/<id> is tracked plus untracked-not-ignored, and a stash entry is neither, so a milestone before git stash drop records when the work went rather than offering to bring it back. I put it in because the issue asks for it and the mark is still the truthful answer to "what did the tree look like before this", but the note deliberately does not promise a recovery it cannot make. Say the word if you would rather those two rows came out.

How it was verified

Eleven rows added to the table at checkpoint.test.ts:52-64, as the issue asks, plus six to the list that keeps nothing. All eleven fail on main and pass with the change.

Test Files  286 passed (286)
     Tests  8469 passed (8469)

pnpm format && pnpm typecheck && pnpm test && pnpm deadcode all clean, on Node 24.

Checklist

  • pnpm format && pnpm typecheck && pnpm test && pnpm deadcode all pass
  • Behaviour change ships with a test
  • No any, no magic values, no console.* outside cli-output.ts
  • If this touches the decision path: n/a — this is what is kept before a decision is acted on, not the verdict
  • If this changes a verb table: n/a, no table is touched; the classes named above are recovery's own list
  • If this changes a command, flag or file it writes: it writes more milestones under refs/memnox/milestones/, which the changeset names; no command, flag or path moves

@Cedric921

Copy link
Copy Markdown
Contributor Author

The red Dependency audit here is not this change — all twelve test legs pass, and the audit fails the same way on a clean main.

It is GHSA-vfj7-8cjw-p6xm on braces, which the advisory says is patched in >=3.0.4. That version was never published: pnpm view braces dist-tags answers { latest: '3.0.3' }, and 3.0.3 is the vulnerable one. So there is nothing to override to, and pnpm audit --audit-level high cannot pass on any branch right now. braces is a devDependency only and reaches nothing the four packages publish.

I tried bumping the dependency that pulls it before reporting. @changesets/cli@3 does drop the chain, but knip reaches the same micromatch@4.0.8 on its own, so the advisory stays:

before:  braces <- micromatch <- fast-glob <- globby <- @manypkg/get-packages <- @changesets/cli
after:   braces <- micromatch <- fast-glob <- knip

I reverted that rather than push a major bump of the tool pnpm ship runs for a fix that is not one.

Written up with the evidence in #129, where the remaining choices are policy ones — an ignore entry, a qualified gate, or merging past it — which seemed yours to make rather than mine to guess at. This PR needs nothing.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

recovery: no checkpoint before shred, truncate, unlink, git stash clear or git checkout --force

1 participant