Skip to content

fix(worktree): never delete or misreport what meta didn't check; remove snapshot - #33

Merged
gold-silver-copper merged 3 commits into
mainfrom
fix/meta-git-safety
Oct 25, 2026
Merged

gold-silver-copper merged 3 commits into
mainfrom
fix/meta-git-safety

Conversation

@gold-silver-copper

@gold-silver-copper gold-silver-copper commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

CLI half of the meta-git safety fixes. Merge together with the PRs linked below (same branch name, so CI builds them together).

What changed in meta_git_cli

  • worktree remove: requires a store record for the exact path, runs the safety check, refuses repositories that aren't linked worktrees even with --force, lists unsaved work (needs --force, which prints what it discards), and removes exactly the checked members.
  • worktree prune: . and nested aliases count as existing; each candidate is checked without force and deleted with delete_checked_set; unsafe ones are kept and listed (JSON kept); a missing directory only loses its record; removal failures are listed and exit 1.
  • worktree create: --from-pr with no matching repo or a failed fetch is an error; --from-ref that matches no repo fails; the set directory is claimed atomically and any failure rolls back what the call created (worktrees and new branches); failing to record the worktree is an error.
  • Worktree command errors print their full cause chain ({e:#}), so git's own message reaches the user.
  • snapshot: removed (src/snapshot.rs, the routes in lib.rs, the help text and the plugin's command list in main.rs, the dialoguer dependency it alone used, and its test). meta git snapshot … now passes through to git and fails.
  • git update / git clone: failed clones are printed to stderr with the last lines of git's output (kept in a bounded buffer while the pipe is drained) and the command exits 1.
  • tests/safety.rs: end-to-end tests through the plugin protocol.

Note for the other meta tabs: this touches commands/worktree/mod.rs (one line), clone_worker.rs, update.rs, clone.rs, and deletes snapshot.rs; whoever merges second may need a small rebase.

Linked PRs (same branch fix/meta-git-safety)

Companions that don't depend on the code above (can merge on their own):

Why

A fuzz run on 2026-10-24 (fresh install in debian:stable) found meta-git commands that delete data they never checked, report failures as success, or panic on bad input. I re-ran every repro on current main before changing anything; all nine still reproduced. Incidents: incidents/fuzz-2026-10-24-{06,07,13,14,16,17,22,23,32}; task [[tasks/harmony-2754]].

Finding Before After
06 worktree remove deleted any dir under the worktree root, a primary clone (via worktrees_dir: "." or META_WORKTREES), and uncommitted work in nested repos (only top-level repos were dirty-checked) needs a store record for that exact path; walks the whole set (nested and hidden repos, files outside any repo); a repo that isn't a linked worktree (.git dir, bare repo, --separate-git-dir, submodule, vanished repo) a checkout git can't inspect, or another recorded set inside it, is refused even with --force; unsaved work is listed and needs --force, which prints what it discards; deletion re-checks the set first (complete git status, commit ids, staged blob ids, and size and mtime of every listed file, not just the summary) and refuses if anything changed, then removes only what was checked (git unregisters each worktree, stopping at the first failure, then the listed loose files if forced, then empty dirs); a symlinked set root is refused
07 worktree prune treated a root-only (--repo .) worktree, or root plus removed children, as orphaned and force-deleted it with its work, in every project (global store); force-deleted when the source project moved . always exists (a set holding it is never orphaned), nested aliases resolve through child .metas; every candidate goes through the same check without force; unsafe ones are kept and listed (JSON kept); a record whose directory is gone only loses the record (only a path that is really gone counts as missing: one that reappears, a dangling link, or one prune can't read is kept); removal failures are listed and exit 1; a record is removed only if it's still the one prune read
16 --ttl 18446744073709551615 wrapped to -1 → expired at once → prune deleted it rejected at parse ("too large"); TTL math saturates for hand-edited stores
17 hooks stderr piped and never read: a hook writing more than the pipe buffer hung worktree create forever hook stderr goes to meta's stderr, as git does for its hooks
23 --from-pr / --from-ref org/api#5 matched org/api-legacy (url.contains); no match or fetch failure was only a warning; a ref no repo has printed "✓ Created" for an empty worktree exact owner/repo match on the remote URL; no match or fetch failure is an error with git's text; a ref no repo has is an error and nothing is left behind; only a repo that lacks the ref is skipped (any other failure, including a repo git can't read, fails the create and rolls it back); upstream tracking for --from-ref origin/x is kept
13, 22, 32 meta git snapshot snapshot delete ../package deleted package.json; restore moved branches back with checkout -B, stranding later commits silently, said "stashed" for empty stashes and exited 0 on failure; a short SHA panicked Removed. The meta owner decided on 2026-10-25 that nobody uses it, so it goes instead of being fixed: the command, meta_git_lib::snapshot, the meta_snapshot_* MCP tools, help text, docs, skills and tests. A leftover meta git snapshot … now passes through to git, which fails with "'snapshot' is not a git command" and writes nothing
14 git update / git clone failed clones counted as cloned, exit 0, git's stderr only in a progress bar (hidden off a terminal) each failure printed to stderr with git's last lines; exit 1 ("N of M repositories failed to clone: …")

Things a reviewer should look at

  • What counts as "work removal would lose": uncommitted and untracked changes (nested checkouts excluded from a parent's untracked list, they're checked themselves), any file flagged assume-unchanged/skip-worktree that is on disk (git doesn't check those, so meta doesn't guess; sparse-checkout files are absent and don't count), an operation in progress (rebase, merge, cherry-pick, revert, bisect: their state such as an autostash lives in the worktree's git dir), commits not on any branch, tag or remote (meta never deletes branches, so branch commits survive removal), and files outside every repo. Ignored files count as disposable, the same as git worktree remove. Counting them would make plain remove refuse on almost every worktree (target/, node_modules/), which trains --force, and --force also discards real uncommitted work. Two helper reviewers raised this; I kept git's semantics on purpose.
  • Hook stderr is now visible. Before, it was silently swallowed, even when a hook failed. Now it shows, like git hooks. stdout stays /dev/null, so --json output is unaffected.
  • worktree remove of a set whose source repo was moved or deleted now refuses even with --force (meta can't attribute the checkout to a repo); the message says so, and the user can delete the directory themselves. The same applies to a set holding a submodule checkout or any repository inside an ignored directory (e.g. a .git under .venv/src/): meta refuses rather than guess.
  • worktree create is all-or-nothing now. Because remove only deletes recorded worktrees, a create that fails partway rolls back what it made: it claims the set directory with an atomic mkdir first (so it can never roll back another create's set), deletes the set only if the safety check is clear, and deletes only the branches its own git worktree add -b created (git refuses -b for an existing branch, so success proves ownership), and only while they still point at the commit they were created at (a post-checkout hook may have committed on one) and git would let them go (not checked out, rebased or bisected anywhere); the deletion is an atomic update-ref -d <ref> <expected>. Since that commit came from another ref, a deleted branch never strands commits. A branch that appeared although the add failed is kept and named. Failing to write the store record is an error instead of a warning.
  • Remaining race, stated plainly: meta doesn't lock out other processes. Deletion re-checks right before it starts, but a process writing into a set while git removes it can still lose that write. On a machine with a stuck store lock (finding 15, fixed in a sibling PR) create now fails loudly instead of leaving an unrecorded worktree.
  • prune --strict exits 1 when it keeps any worktree, consistent with strict mode elsewhere.
  • created_branch (the "(new)" label and the store field) now means "this create made the local branch", which includes a tracking branch made from origin/<branch>.
  • Merge all PRs in this set together (listed above). Root CI builds the children from this branch name, so main of gitkb/meta is only consistent once all of them are in. The harmony, loop_cli and agent PRs don't depend on the code above and can merge on their own.
  • meta git snapshot is removed, not fixed (owner's decision). The guard's denial messages and the destructive-commands rule now say to commit or stash first (a backup branch keeps only commits) and recover commits with git reflog. Not changed: agent score still grades "snapshot safety" (25% of the score, counting meta git snapshot create before destructive commands); that metric needs its own decision in gitkb/agent. Copies of the old rule and skills checked into gitkb/gitkb and ttyeye (from an earlier meta init) still mention snapshot; re-running meta init there after release refreshes them.

How it was verified

  • Rebased onto meta-shell-args (root meta 47eeca3: commands run as argv, --include/--exclude name whole repos). The child rebases were clean; the one conflict was main's edit to .context/tasks/meta-snapshot.md, which this set deletes. Main's new --include refusal in meta_git_cli and a README line and bats test still named git snapshot; they no longer do. The meta_exec examples in docs/agent_workflows.md now pass argv lists (["sh", "-c", ...] where a shell is needed), matching main's MCP schema. Everything below was re-run after the rebase.

  • Every fuzz repro re-run (adapted to an isolated HOME under ~/.cache/claude-tmp/, never a real meta or harmony checkout), on main and on this branch. On main all nine findings reproduce; on this branch each worktree, clone and TTL finding is refused, reported or rejected as described above, and the three snapshot findings are gone with the command (meta git snapshot create x exits 1 with git's "not a git command" and writes nothing). For 17, this machine's default pipe buffer is 256 KB (the fuzz container's was 64 KB), so the repro used 2 MB.

  • End-to-end tests in meta_git_cli/tests/safety.rs drive the meta-git binary through the plugin protocol: 15 tests, one or more per remaining finding. With main's sources and these tests, all of them fail (the hook test hits its 120 s timeout); on this branch all pass. Unit tests cover the rest (safety check, duration overflow, owner/repo matching, TTL saturation, branch in use, clone failure tracking).

  • cargo test for meta_git_lib (167), meta_git_cli (76 + 15 + 4), meta-mcp (22) and meta pass, and cargo clippy --all-targets -D warnings is clean. Locally two meta registry tests (*_fails_outside_workspace) fail only because my temp dir is under a $HOME that has ~/.meta; this PR doesn't touch that code, and CI runs them. Root bats git, help, worktree and worktree_edge_cases: 143/143 with an isolated HOME. gitkb/agent: cargo test (126) and tests/guard.bats pass.

  • Reviewed by two read-only helper reviewers over several rounds, then repeated final passes by four fresh read-only reviewers until all four said yes on green CI; the snapshot removal was reviewed the same way.

Try it in a scratch meta repo:

cd ~/code/gitkb-dev/harmony/.worktrees/meta-git-safety/open-source/meta && cargo test -p meta_git_cli --test safety

@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 253f40c9-21ba-48db-b3b1-7f0035b883e1
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@mateodelnorte

Copy link
Copy Markdown
Contributor

I would honestly lobby for removing the snapshot command entirely. I don't think anyone uses it.

@gold-silver-copper
gold-silver-copper force-pushed the fix/meta-git-safety branch 6 times, most recently from a8e162c to eb02b9f Compare October 25, 2026 17:38
@gold-silver-copper gold-silver-copper changed the title fix(worktree): never delete or misreport what meta didn't check fix(worktree): never delete or misreport what meta didn't check; remove snapshot Oct 25, 2026
Drops the snapshot command, its help text and its test. A leftover
`meta git snapshot ...` call now passes through to git, which fails
with "'snapshot' is not a git command" and writes nothing.

Refs [[tasks/harmony-2754]]
@gold-silver-copper
gold-silver-copper merged commit 79af652 into main Oct 25, 2026
7 checks passed
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.

2 participants