lane-worktrees: close #170's --yes data-loss and gate holes, self-check every removal, switch --yes back on

[brettheap]

What and why

This is the first of two PRs for #170, stacked on #169 (feat/lane-worktrees-report). It closes every path in #170 that could delete, publish or let #163's gate pass when it should not, and switches lane-worktrees sweep --yes and --expire --yes back on (c550e89). The nine commits after c550e89 take Copilot's first round on this PR (below); none of them brings the switch back. LANE_WORKTREES_ENABLE_YES appears nowhere in the code now; the one mention left is the test asserting the fixture does not set it.

Lane openRepoTools-3 opened this PR. Since 2026-10-06T16:20Z lane openRepoTools-1 is its writer (Brett Heap's word of 2026-10-06: "we can use openRepoTools-1 to help if you can send tasks there too").

--yes has been refused since 5f4bd84 unless LANE_WORKTREES_ENABLE_YES=1 was set. The reviews of #168 and #169 found these data-loss paths:

• a clone's tag-only commits;
• a submodule's unpublished branches and stash;
• a mirror-style fetch config together with --prune;
• scratch directories holding trees;
• bundle-only rescues missing from the ledger;
• edits hidden by skip-worktree or assume-unchanged;
• a clone inside a venv;
• prune dropping the only pointer to detached commits;
• commits that only a reflog names;
• untracked secrets pushed by the WIP rescue;
• --no-verify skipping the git-lfs upload;
• a teammate's open PR branch receiving the lane's commits;
• a register LANDED line overriding a live gh OPEN.

There were also gate-correctness holes. An unreadable lane root, registration list or inventory row read as "nothing there". Other lanes' claims in a nested estate were never read. The --branches counts were off in both directions.

Every item now has a regression case. Each case was proved to fail against the base lane-worktrees (45fa45d) and to pass here.

Each removal also runs a self-check first. It asks git directly whether every commit the removal would take is on origin, in a ref that stays, or in a bundle this run verified. The commits it checks are:

• for a worktree, its HEAD and its admin logs/HEAD, plus the branch reflog when the branch is deleted;
• for a clone, every ref and reflog;
• for a pruned registration, its admin HEAD and reflog.

The check runs git rev-list <those> --not --remotes=origin --branches --tags --glob=refs/stash <verified pushes> <bundle heads>. If anything comes back, the sweep refuses with exit 2 and writes a DISPOSITION.md line. Acts already done stay recorded, and nothing after the refusal is acted on. The commit-level reason for each fix is in its commit body.

What guards each kind of removal, after round 1:

• a worktree, a clone, a pruned registration and, since 702c2df, a --branches delete (its tip and its reflog): the commit-level self-check above;
• scratch, a cache, and an unmarked tmp.* sandbox hold no commit of their own. Right before the rmtree they are checked to hold no repository (any .git); a directory inside that cannot be read counts as one (573a420, 6436450, item 7);
• a suite-marked sandbox (pytest's .lock, run.sh's marker or layout, with its pid gone) is removed on that mark. Its repositories are the suite's fixtures;
• --expire removes only an archive whose ledger is whole and whose every rescue is on origin at its SHA, and never one with a bundle: row;
• a remote branch is deleted only under a lease on the judged SHA, merged by PR evidence, and never while gh answers its PR OPEN.

The two defaults applied (stated for Brett Heap to overrule)

1. Item 1, /tmp/tmp.* sandboxes. A tmp.* directory is removed only on proof, in one of two forms:

   • a suite's mark whose pid is gone: pytest's .lock or the .openrepotools-run marker tests/run.sh now writes, each naming a pid (since 3cdd387 an empty or malformed one is no mark), or run.sh's run-root layout (basetemp/ beside tmp/);
   • no mark at all, but nothing written in it for aging_days (14).

   Any other tmp.* is listed and left alone. A directory a live process stands in, holds open or names in its environment is never touched, as before. Since 6436450 an unmarked tmp.* that holds a repository is listed whatever its age: nothing says its repositories are fixtures. This is stricter than the default, so it stays within it.

   The literal default was "a marker proves it is a suite's, OR its owning pid is gone AND it is older than 14 days". An unmarked mktemp -d directory records no pid, so I read its second half as "unmarked and untouched for 14 days". To overrule: the stricter reading never removes an unmarked tmp.*. That is a one-line change in sandbox_items.

2. Item 2, unreadable /proc entries. Another uid's unreadable entry is ignored, since a writer runs as us. An unreadable entry of our uid is read as unknown liveness: the tree it could be in is kept, and --yes takes it with no other tree. "Could be in" means one of three things:

   • its command line names a path in the tree;
   • its parent's cwd is in the tree;
   • its own status cannot be read either, in which case it could be in any tree.

   On Eagle the always-present unreadable ones are 3 ssh-agent processes, whose command lines name no tree. Treating them as "anywhere" would keep every tree on every sweep. The table also lists every such pid in a note line.

Items, commits and tests

All tests are in tests/test_lane_worktrees.py unless noted. "Fails at base" means the case ran against 45fa45d's lane-worktrees (and lanes-edit.sh) with this branch's tests, in the CI proof runs below.

item	commit	test	fails at base
1 tmp.* only on proof	2eb5216	test_a_tmp_dir_with_no_mark_of_a_suite_is_listed_until_it_is_old	yes
2 unreadable /proc	2eb5216	test_an_unreadable_process_of_this_account_keeps_the_tree_it_names	yes
4 tag-only commits	85365fd	test_a_clones_tag_only_commits_keep_it	yes
5 submodule branches/stash	85365fd	test_a_submodules_unpublished_branch_or_stash_keeps_its_tree[branch,stash]	yes (both)
6 mirror refspec + --prune	040b12b	test_a_mirror_style_fetch_refspec_prunes_no_local_branch	yes
7 scratch holding trees	1e583d1	test_scratch_holding_a_repository_is_kept	yes
8 bundle-only rescue in the ledger	31bbfb1	test_an_archive_whose_bundle_is_the_only_copy_never_expires	yes
A1 register fetched under --yes	4a79c8c	test_yes_reads_the_register_now_or_refuses[origin-unreachable,LANES_NO_FETCH-inherited]	yes (both)
A2 skip-worktree / assume-unchanged	85365fd	test_an_edit_git_status_hides_keeps_the_tree[--skip-worktree,--assume-unchanged]	yes (both)
A2 guard: a sparse checkout is no edit	85365fd	test_a_sparse_checkouts_absent_files_are_no_edit	no, by design (it keeps A2 from over-reaching)
A3 clone inside a venv	85365fd	test_a_clone_inside_a_venv_keeps_its_tree	yes
A4 prune drops detached commits	31bbfb1	test_a_pruned_registrations_detached_commit_is_rescued_first	yes
A5 reflog-only commits	31bbfb1	test_a_commit_only_the_reflog_names_is_bundled_before_removal	yes
A6 ignored file under a build-named ancestor	8ec5a9a	test_an_ignored_file_beneath_a_build_named_directory_is_archived	yes
A7 teammate's open PR branch	d32be5d	test_unreviewed_commits_never_go_onto_somebody_elses_branch[open-pr,another-lanes-trailer,gh-unreadable]	yes (all three)
A8 LANDED vs a live gh OPEN	d32be5d	test_a_landed_line_never_deletes_a_branch_gh_answers_open	yes
A9 untracked files bundle-only	31bbfb1	test_untracked_files_go_to_the_bundle_never_to_origin[True,False], test_push_untracked_pushes_them	yes (all three)
A10 --no-verify vs git-lfs	d4724a2	test_a_push_runs_the_hooks_where_git_lfs_is_configured[True] ([False] is its control)	yes ([True])
A11 dependents anywhere, partial scan	4b65c30, 76011d9, 6ef7c87	test_a_dependent_anywhere_in_the_estate_makes_a_clone_load_bearing, test_a_partial_dependents_scan_removes_no_clone	yes (both)
A11 guard: uv's .git marker file	6ef7c87	test_a_dot_git_file_that_is_no_pointer_leaves_the_scan_whole	not applicable: it guards 4b65c30's own walk, which base does not have
B4 misspelt --live → 64	741b407	test_a_live_path_that_names_no_tree_is_a_usage_error	yes
B5 writer count for own records	741b407	test_trees_this_session_recorded_want_the_writer_count	yes
B6 stop on NOT WRITTEN	741b407	test_the_first_register_line_refused_stops_the_sweep	yes
G1 unreadable lane root	d71569f	test_a_lane_root_that_cannot_be_listed_refuses_the_dry_run	yes
G5 unreadable registrations	d71569f	test_registrations_that_cannot_be_read_refuse_the_dry_run	yes
G6 other lanes' claims in a nested estate	af80118	test_another_lanes_claim_is_read_wherever_its_control_root_is[nested-in-the-estate,recorded-in-the-register]	yes (both)
G2 lane-owned open PR branch counted	c6ab9f8	test_a_lane_owned_open_pr_branch_origin_lacks_is_still_counted	yes
G7 published under its own name	c6ab9f8	test_a_branch_published_under_its_own_name_is_not_unfinished	yes
G9 inventory row with no id	08ad126	test_an_inventory_row_with_no_id_refuses_the_dry_run	yes
G13 pathspec-check fails closed	5ba0c96	tests/test_lane_helpers.sh #162 block: pathspec-check ':(bogus)x' exits 2 and says the read failed; that commit exits 2, says so and commits nothing	yes: the 4 refusal assertions are the shell suite's only failures at 45fa45d (4436 passed, 4 failed). "Commits nothing" holds there too, because base's commit fails later, at the add
self-check before each removal	31bbfb1	test_the_self_check_refuses_a_removal_that_would_lose_a_commit	yes
--yes switched back on	c550e89	test_yes_needs_no_enabling_variable	yes

Some commits only support the items above:

• a6fb64b and 6404f1c are the manual (docs/README-lanes.md).
• 7e1865a holds the existing cases to the new behaviour: the register fetch under --yes, untracked files to the bundle, and B5 for a record this session wrote.
• 76011d9 limits the whole-estate walk to where a removal deletes a directory, meaning a clone the table could take or --include-scratch. On Eagle that walk covers 683,000 directories (397 s cold). A worktree's removal deletes no objects, so the estate's shape is enough there.

Copilot round 1 on this PR, and one gap found auditing the self-check

All eight threads were real defects in this PR's own code; each is fixed with a case. "Red" means the case failed with that commit's lane-worktrees (and lanes-edit.sh) and this branch's tests, in proof run 37500368196 (below).

thread	what it found	commit	test (tests/test_lane_worktrees.py)	red at c550e89	red at 45fa45d
r4194736582 tree_guards	a submodule's commit that only its reflog names went with the tree	64710fb	test_a_submodule_commit_only_its_reflog_names_keeps_the_tree	yes	yes
r4194736644 _tar	an ignored file under a cache- or venv-named ancestor was selected, then dropped from the archive, which still verified	968f29d	test_an_ignored_file_beneath_a_cache_or_venv_named_directory_is_archived[node_modules,.venv]	yes (both)	yes (both)
r4194736696 _caches	--include-caches removed a .venv holding a clone (pip install -e git+)	573a420	test_a_clone_inside_a_venv_keeps_its_tree (now also --include-caches --yes)	yes	yes
r4194736746 _loss_plan	a clone's reflog directory that could not be listed shrank the loss plan in silence	4eb9cbd	test_a_clones_reflogs_that_cannot_be_listed_remove_nothing	yes	yes
r4194736789 _act_item	--branches --yes deleted a branch, and its reflog, with no loss plan, bundle or self-check	702c2df	test_a_branch_deletes_reflog_only_commit_is_bundled_first, test_the_self_check_refuses_a_branch_delete_that_would_lose_a_commit	yes (both)	yes (both)
r4194736836 _wip_rescue	which files were untracked came from the table's status, so a key file arriving later was pushed	8fdb336	test_an_untracked_file_that_arrives_after_the_table_is_never_pushed	yes	yes
r4194736875 lost_tips	an annotated tag object was always "lost", so the self-check refused, and halted the sweep on, every clone with one	4ea93db	test_a_clones_annotated_tag_neither_halts_the_sweep_nor_is_lost[only-in-the-clone,on-origin-too]	yes (both)	only-in-the-clone yes (the tag's message was lost); on-origin-too passes there, as it should: the halt came with this PR's self-check
r4194736925 _wip_rescue	untracked git-lfs content went to its only-copy bundle as a pointer	8fdb336	test_untracked_git_lfs_content_keeps_the_tree[seen-by-the-table,arrived-after-it]	yes (both)	yes (both)
(audit of "a check before every removal") sandbox_items	an unmarked tmp.* aged past 14 days was removed with a repository and its unpushed commit inside	6436450	test_an_old_unmarked_tmp_dir_holding_a_repository_is_listed_never_removed	yes	yes

685cbe3 is the manual and --help for all of these. Fix sketches:

• tag objects: kept where origin holds that very tag object (ls-remote --tags), else bundled with its message;
• untracked files: the pushed snapshot is always tracked-only unless --push-untracked, and what the full snapshot holds beyond it goes to the bundle;
• untracked git-lfs content: keeps its tree. That is the "refuse removal" option, checked both when the table is built and at the act, before anything is pushed.

Copilot round 2 on this PR

There were ten threads. Nine were real defects in this PR's code and are fixed, each with a case. One is a cost trade-off and is filed in #176. "Red" means the case failed with lane-worktrees at 685cbe3 (the head before the round) and this branch's tests, in proof run 37506230403.

thread	what it found	commit	test (tests/test_lane_worktrees.py)	red at 685cbe3
r4198393067 tree_guards	a DEINITIALIZED submodule's repository, kept under <gitdir>/modules/ with a branch no remote held, went with the tree	9eb473a, c538ec1	test_a_deinitialized_submodules_kept_repository_keeps_the_tree	yes
r4198393140 tree_guards	a skip-worktree edit in a submodule read as clean and went with the tree	b266ce2	test_a_submodules_hidden_edit_keeps_the_tree	yes
r4198393195 nested_repos	the walk that proves a removal takes no repository passed over cache-named directories	72970e2	test_a_repository_under_a_cache_named_directory_keeps_the_scratch	yes
r4198393237 gather	the whole-estate walk's .lane-state finds were thrown away, so a claim inside a repository was missed	fed0e5c	test_another_lanes_claim_inside_a_repository_is_read_before_a_clone_goes	yes
r4198393296 judge, _decide	a LANDED line plus gh's OPEN gave "moved", and push+remove pushed onto the open PR's branch	4fba346	test_a_landed_line_never_lets_commits_onto_a_branch_gh_answers_open	yes
r4198393353 _act_prune	a gone registration's submodule repositories (<admin>/modules/) were pruned unread	a03f5df, c538ec1, 499d2ab	test_a_gone_worktrees_kept_submodule_repository_is_never_pruned	yes
r4198393416 _act_tree	a clone's local git-lfs store went when a bundle, which carries pointers only, was a commit's only copy	de45178	test_a_clones_local_lfs_store_keeps_it_when_a_bundle_would_be_the_only_copy	yes
r4198393482 sandbox_provenance	an empty .lock counted as a suite's mark	3cdd387	test_a_suite_mark_that_names_no_pid_is_no_proof[.lock,.openrepotools-run]	yes (both)
r4198393531 module_tags_why	a submodule's annotated tag object on a published commit was in no guard or loss plan	77db871	test_a_submodules_annotated_tag_object_is_never_lost_with_its_tree[only-in-the-submodule,on-its-remote-too]	only-in-the-submodule yes; on-its-remote-too passes, as the control
r4198392951 walk_estate	the dependents walk passes over caches, tool environments and site-packages	filed in #176	n/a	n/a

Notes on the round:

• c538ec1 came out of the proof run itself. A submodule repository whose core.worktree is gone (every gone registration's) failed every git read on "cannot chdir", so it was kept as "unreadable" and never actually read. Those reads now run with the repository's own objects/ as the working tree.
• 499d2ab makes the regression's own final check read that way too.
• de168db is the manual and --help for the round.
• The submodule tag-object check asks remotes, so it runs at the act only. The dry run reads remove; --yes keeps the tree.
• lane-worktrees: the dependents walk passes over caches, tool environments and site-packages (filed from #174 Copilot round 2) #176 is for a person to decide. Walking those subtrees costs about 683,000 directories and 397 s cold on Eagle's estate. Treating them as a partial search would keep every clone. The issue lists the options.

Measurements: a read-only dry run over this lane's real estate on Eagle

These used lane-worktrees sweep openRepoTools-3 --dry-run --porcelain. No --yes on anything real. Every run exits 2 in both builds, because this lane has no #97 snapshot (state NONE) yet 10 trees are its. That refusal is the gate working.

run	before (45fa45d)	after (c550e89)
gate (--dry-run --porcelain)	15.1 s, rc 2; 5 live, 4 foreign, 5 retire	12.7 s, rc 2; identical rows
--include-foreign --branches --include-scratch --include-caches	13.3 s, rc 2; scratch 6 archive+remove, 1 live; branches 1 keep, 2 list; no cache rows	30.6 s, rc 2; scratch 4 archive+remove, 2 keep, 1 live; branches 1 keep (the two review/* branches are published under their own names, G7); 27 cache rows
--include-sandboxes	17.7 s, rc 2; 83 sandboxes remove	13.3 s, rc 2; 81 list, 4 remove (pytest-of-brett with dead .lock pids)

Notes on the "after" column:

• dash-scratch and featlist-scratch are now kept, because repositories lie under them (item 7). dash-scratch/bite has no commits and a staged .gitattributes. The table's earlier archive would have swallowed it.
• The 27 cache rows are the __pycache__ and .pytest_cache directories inside those two kept scratch directories. Before, they rode inside the scratch archive.
• The second run's extra ~17 s is the whole-estate walk. --include-scratch turns it on.
• The table's note lists 3 unreadable ssh-agent pids (item 2).
• The worktree stayed clean through every run (git status --porcelain empty).

Verification

tests/run.sh -k '<selector>' never started within 15 minutes on Eagle; the workstation lock was held by other lanes' suites throughout. CI is the record. I used temporary proof/openRepoTools-3/* branches and dispatched a trimmed tests.yml on Linux. They are deleted now.

• Head, full Linux suite:

  • run 37450303540 on the earlier ordering: 1195 passed, 2 deselected (macos_job), 31 min;
  • run 37452432577 on c550e89, this PR's head: 1197 passed, 2 deselected, 33 min.

• Base: this branch's tests ran against 45fa45d's lane-worktrees:

  • run 37449846462: all 36 new sweep cases failed;
  • run 37452162948: G9 and the reworked A1 and G6 cases failed;
  • run 37452793929, with base lanes-edit.sh too: the shell suite failed only on G13's 4 assertions, and test_yes_needs_no_enabling_variable was refused by the switch.
  • Every other --yes case in that run was also refused by the switch. That is why the per-item base proofs above ran before the switch commit, when the fixture still set the variable.
  • In run 37449846462 the controls test_a_sparse_checkouts_absent_files_are_no_edit and ...git_lfs_is_configured[False] passed at base, as designed.

• Round 1 (lane openRepoTools-1), proof run 37500368196 on a temporary branch, proof/openRepoTools-1/174-round1, deleted once read. It is 685cbe3 with a trimmed workflow, and tests/run.sh on the workstation was starved by other lanes' suites:

  • head (685cbe3): -k lane_worktrees, 101 passed;
  • red-c550e89: the 13 new cases, 13 failed;
  • red-45fa45d, with that base's --yes switch set in the fixture: 12 failed, 1 passed (the on-origin-too tag case, as explained above).

• Round 2 (lane openRepoTools-1). These used temporary branches proof/openRepoTools-1/174-round2, -round2b and -round2c, each deleted once read:

  • run 37504874509, the first: it showed the core.worktree read failure that c538ec1 fixes;
  • run 37505731437: it showed the test's own read, fixed by 499d2ab;
  • run 37506230403, on 499d2ab:
    • head (-k "lane_worktrees or repo_hygiene or openrepotools_command"): 346 passed and 1 failed. The failure is test_the_macos_job_parses_every_bash_file_this_repository_ships, which reads the trimmed proof workflow itself.
    • red-685cbe3: 10 failed and 1 passed (the on-its-remote-too control).

This PR's own CI (Linux and the ready-gated macOS job) is the landing record. I have not applied ready.

Not in this PR

#179 (PR 2), stacked on this one, takes the honesty, portability and test items and says Closes #170:

• C1, C4, C5, C6;
• B8, C2, C3;
• items 3, 9, 10 and 11;
• G3, G4, G8, G10, G11, G12, G14 and G15;
• E1, E2, E4 to E10, and (d).

Items 9 to 11 were not in the brief's list. I am taking them there so that "Closes" is true.

@codex review (it is expected to answer that its quota is spent; that is not a finding).

Part of #170
Refs #162 #168 #169 #163 #176 #179

Lane: openRepoTools-3 (opened), openRepoTools-1 (writer since 2026-10-06T16:20Z)

🤖 Generated with Claude Code

Summary by Sourcery

Harden lane-worktree sweeping against data loss and gate errors, then restore verified --yes operations.

New Features:

• Re-enable --yes execution for worktree sweeping and archive expiration without an environment opt-in.
• Add removal safeguards that verify commits, reflogs, tags, bundles, submodules, LFS data, and dependent repositories before destructive actions.
• Expand sweep discovery and gate validation to fail closed on unreadable state, register fetches, missing inventory data, nested lane claims, and incorrect branch counts.

Bug Fixes:

• Prevent data loss involving unpublished commits, reflog-only and tag-only objects, submodule repositories, hidden edits, untracked files, Git LFS content, scratch directories, bundles, and open pull-request branches.
• Prevent remote branch deletion or publication when pull-request status, lane ownership, or register history is ambiguous.
• Prevent sandbox, cache, scratch, clone, and pruned-registration removal when repositories or unknown liveness may be present.
• Make pathspec and register-write failures refuse or stop sweeps instead of being treated as clean or allowing subsequent actions.

Enhancements:

• Add per-removal self-checks and disposition recording, with exit status 2 when a removal could lose data.
• Improve estate-wide dependent and other-lane claim discovery while preserving conservative behavior for partial scans.
• Refine archive, rescue, bundle, ignored-file, branch, liveness, and sandbox classification rules.

Documentation:

• Update lane sweeping documentation and help text to describe the enabled destructive operations, fail-closed guards, self-checks, rescue handling, and revised exit behavior.

Tests:

• Add comprehensive regression coverage for destructive sweep paths, gate correctness, sandbox provenance, nested repositories, pull-request handling, bundle retention, LFS behavior, and self-check refusals.
• Update test-run roots and fixtures to provide sandbox provenance markers and workspace register data.

[brettheap]

@codex review

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[sonarqubecloud]

Quality Gate passed

Issues
28 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud