fix(audit-git-checkouts): audits should never lose commits that only a submodule holds #78
Loading…
Reference in a new issue
No description provided.
Delete branch "docs/audit-mode-submodule-writes"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
A default audit no longer destroys commits that live only inside a submodule. Before this, a submodule move could strand a detached commit, a configured-tag fetch could overwrite a local tag, and removing a linked worktree deleted its submodules' Git directories. The reviews on this PR reproduced all three.
Before a fast-forward or selector move, the driver walks every populated submodule at any depth. It finds them from the Gitlinks, so
.gitmodulesedits andignoresettings can't hide one. A checkout with a submodule on a commit nothing else holds is left alone and reported as a decision (newstrandedSubmodulesfield). Removal keeps a worktree whose submodules hold uncommitted files, stashes, or commits that no remote-tracking ref holds. The new outcome isjudgment/submodule-local-work. A submodule the driver cannot read or query blocks updates and removal, so it is never assumed empty.The report schema moves to version 6. The mode table now lists every write a default audit makes, and a condition sends agents to
--no-fetch --no-removewhen submodules must stay put.Deferred from #77 review round 4.
🤖 Generated with Claude Code
Review
01M3BJ3S1CY1F1S8BW4PD0P9FC— head0cdf0815fc1f001c6c097c960f3b7803512d4081Review — j4k-oss/agent-skills @
d581670616Scope: diff against base tree
d1e95321d499Status: dispatched — coverage complete (3/3 slots terminal)
Facts: current review-wide projection
Computed under:
Findings (15)
medium — "moves branch- or tag-tracked submodules when nothing fast-forwards" understates when selector moves run: they also run right after a successful fast-forward
01M3BJAY8E33N13G2MRZKFY5X3skills/audit-git-checkouts/SKILL.md(snippet)medium — The stranded-submodule rule is one 57-word sentence whose "found from the Gitlinks rather than
.gitmodules" aside drops the consequence that makes it worth stating01M3BJBFWMJRZTY6TQRHTN69G0skills/audit-git-checkouts/references/checkout-updates.md(snippet)medium — "nested submodules follow their recorded commits" names a checkout the selector path never performs, and reads as a guarantee
01M3BJCEANJFWKDH83C7BJ4JFVskills/audit-git-checkouts/references/checkout-updates.md(snippet)medium —
--helpstill advertises "schema 5" after the same change bumped the report to schemaVersion 601M3BJAD5HST20NAJ8TXEPNNJRskills/audit-git-checkouts/scripts/audit-checkouts.sh(snippet)01M3BJXKD6FR4GJBVEJ0FVG22D(general-bug)medium — The update-mode submodule walk discards its own diagnostics, so a failed walk blocks every fast-forward with an unactionable "submodule check failed" row
01M3BJVR540DX0TTTF5985H5RHskills/audit-git-checkouts/scripts/audit-checkouts.sh(snippet)medium — git worktree remove --force is gated on a .gitmodules file, but Git refuses on populated Gitlinks, so a merged worktree with a Gitlink and no .gitmodules can never be removed
01M3BJX1D6ARVSHY1Y8V11MZT2skills/audit-git-checkouts/scripts/audit-checkouts.sh(snippet)medium — The new submodule removal-gate test stages uncommitted submodule work but only asserts the status-query-failure path, so the uncommitted-files and stash gates are unprotected
01M3BK8QQK8CCEE9262AAYWE8Askills/audit-git-checkouts/scripts/audit-checkouts.test.mjs(snippet)medium — stepFailure returns "submodule check failed; checkout not updated" for every worktree, hiding a linked worktree's real removal outcome and dropping it from all other report sections
01M3BJWCTMM2QXQWEPKZA7TSHJskills/audit-git-checkouts/scripts/render-audit-report.ts(snippet)medium — The stranded-submodule row overrides primaryDecision for every primary checkout, so detached and off-default primaries lose their real reason and are compared against the wrong line
01M3BJZHXK440NSNFW2AJ720B6skills/audit-git-checkouts/scripts/render-audit-report.ts(snippet)low — The mode table's "What changes" cell now restates the submodule behavior that the bullet three lines below and the reference already give
01M3BJD43THCCSVM63RWPDHTDZskills/audit-git-checkouts/SKILL.md(snippet)low — The reference's failure list omits the new "submodule check failed; checkout not updated" outcome and sends its reader to repair a state the driver never touched
01M3BJFJHNJM26BM2SDNQ4X9VXskills/audit-git-checkouts/references/checkout-updates.md(snippet)low — "rerun once ... they authorize discarding it" sends the agent back into the same gate: authorization alone does not change what the walk sees
01M3BJGHNSVNR7A66RVMKR11BXskills/audit-git-checkouts/references/removal-gates.md(snippet)low — The new help text says a ref "keeps" a commit where every other file says "holds", and "work no remote-tracking ref keeps is" garden-paths the reader
01M3BJDYR88CZ3HMK9ZDZWETFHskills/audit-git-checkouts/scripts/audit-checkouts.sh(snippet)low — A git ls-files failure in list_submodules_with_local_work cannot be detected, so the submodule safety gate fails open instead of closed
01M3BJYBGZFFQGBD05BS5YHCYMskills/audit-git-checkouts/scripts/audit-checkouts.sh(snippet)low — The tag-selector test never exercises the "another ref holds it" half of update_submodule_tag, so that allowance can be deleted with the suite green
01M3BKCTPMEA51K2Z53GH578NAskills/audit-git-checkouts/scripts/audit-checkouts.test.mjs(snippet)Other claims
01M3BJF1C8Y7SDRTS25DV3DSQQlow — Test comment "Nor must a submodule whose status query fails" is a predicate-less fragment whose negation states the opposite of the assertion below it01M3BJXKD6FR4GJBVEJ0FVG22Dlow — --help still advertises "schema 5" after the report was bumped to schemaVersion 6 →01M3BJAD5HST20NAJ8TXEPNNJRCoverage
Coverage pass: 01M3BJ3S4H2RMJ1PD1ZWC9K7FR
Accounting: complete
Slot health: healthy
@ -16,3 +16,3 @@| Request | Flags | What changes || --- | --- | --- || Audit, clean up, "which are behind" | none | Fetch and prune `origin`; fast-forward eligible default checkouts; prune stale worktree registrations; remove proven-merged linked worktrees inside the root. || Audit, clean up, "which are behind" | none | Fetch and prune `origin`; fast-forward eligible default checkouts and sync their submodules; in first-party default checkouts, move submodules to their configured branch or tag, leaving the Gitlink change unstaged; prune stale worktree registrations; remove proven-merged linked worktrees inside the root. |high — Default selector updates abandon unreferenced submodule commits
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39S02MAT0TTV614JNDM127Eof review01M39RTBF4DNVQ28DF97Z9ZE21high — Configured-tag updates overwrite local submodule tags
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39S1NH64NYPDQ30RW6K760Sof review01M39RTBF4DNVQ28DF97Z9ZE21medium — The mode table hides submodule initialization behind “sync”
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39RYNHTH5TR2P6BDC96B9N0of review01M39RTBF4DNVQ28DF97Z9ZE21Fixed in
85dfb5b. Reproduced first: a detached submodule commit no ref held went to the reflog. A selector-managed Gitlink change now counts as deferred maintenance only when some ref holds the submodule's checked-out commit, so neither the fast-forward nor the selector move runs; the move also checks this itself. Covered by the new test "a submodule commit no ref holds blocks the fast-forward and the selector move".Fixed in
85dfb5b. Reproduced first: the forced tag fetch replaced a localv1holding a unique commit. The tag is now fetched intoFETCH_HEAD, and a differing local tag is replaced only when another ref still holds its commit; otherwise the selector update fails and names the tag. An upstream re-tag is still followed. Covered by the new test "a configured tag follows origin unless the local tag holds a commit no other ref holds".Fixed in
85dfb5b. Confirmedsynchronize_submodulesrunsgit submodule update --init --recursive --checkout. The table now says the audit initializes submodules and checks them out at the recorded commits after a fast-forward;references/checkout-updates.mdand the driver's--helpsay the same.@ -23,3 +23,4 @@- A repository whose work integrates somewhere other than `origin/<default>`, such as a fork with an `upstream`: `--no-remove`. Removal proves containment against `origin/<default>` only.- A repository with a custom `origin` fetch refspec: `--no-fetch --no-remove`. The fetch prunes every `refs/remotes/origin/*` ref that no server branch supplies (see [references/checkout-updates.md](references/checkout-updates.md)).- A first-party checkout whose submodules must stay where they are: `--no-fetch --no-remove`. Selector moves happen even without a fast-forward, and `update = none` does not stop the post-fast-forward sync.low — “Selector moves” says the configuration changes when the checkout moves
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39RZCXAEDY17MKCNN4C1BZHof review01M39RTBF4DNVQ28DF97Z9ZE21Fixed in
85dfb5b. Confirmed the selector itself never changes andupdate = noneskips only the selector move. The bullet now reads: the driver moves branch- or tag-tracked submodules even when nothing fast-forwards, andupdate = nonestops only that move, not the checkout at recorded commits after a fast-forward.docs(audit-git-checkouts): name the submodule writes a default audit makesto fix(audit-git-checkouts): audits should not strand submodule commits or local tags that no other ref holds@ -23,3 +23,4 @@- A repository whose work integrates somewhere other than `origin/<default>`, such as a fork with an `upstream`: `--no-remove`. Removal proves containment against `origin/<default>` only.- A repository with a custom `origin` fetch refspec: `--no-fetch --no-remove`. The fetch prunes every `refs/remotes/origin/*` ref that no server branch supplies (see [references/checkout-updates.md](references/checkout-updates.md)).- A first-party checkout whose submodules must stay where they are: `--no-fetch --no-remove`. The driver moves branch- or tag-tracked submodules even when nothing fast-forwards, and `update = none` stops only that move, not the checkout at recorded commits after a fast-forward.low — "the checkout at recorded commits" reuses
checkout, the skill's word for a working copy, to mean the act of checking outlens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39T9CDN90M09PQTBECZSE3Pof review01M39T0ST0ZAZJTHPM4AJGVRA3superseded by review
01M39W1HVW5VE4BHR8WJKRSQ29for headfeb2194b3542c215cc150cbce403698ba16a51c7Fixed in
feb2194. Both places now use a verb: "after a fast-forward the driver still checks each submodule out at its recorded commit".@ -5,3 +5,3 @@## What the driver already updatesThe driver fast-forwards a default-branch checkout only when its comparison is fresh, it has no local commits, and it is strictly behind `origin/<default>`. A dirty checkout qualifies only when every changed path is deferred guidance (`AGENTS.md`, `.agents/**`, at any depth) or first-party `.gitmodules`/Gitlink maintenance, and upstream did not touch those paths. The merge runs `--ff-only` with `merge.autostash=false`, because autostash would round-trip the tree through a stash and silently unstage staged guidance. After a fast-forward it synchronizes committed submodules to the recorded Gitlinks.The driver fast-forwards a default-branch checkout only when its comparison is fresh, it has no local commits, and it is strictly behind `origin/<default>`. A dirty checkout qualifies only when every changed path is deferred guidance (`AGENTS.md`, `.agents/**`, at any depth) or first-party `.gitmodules`/Gitlink maintenance whose submodule commit some ref holds, and upstream did not touch those paths. The merge runs `--ff-only` with `merge.autostash=false`, because autostash would round-trip the tree through a stash and silently unstage staged guidance. After a fast-forward it initializes committed submodules and checks them out at the recorded Gitlinks.low — The fast-forward eligibility gate's new qualifier attaches to
.gitmodulestoo and omits the selector requirement it actually depends onlens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39T8QXN3T44RXQHE57QGB18of review01M39T0ST0ZAZJTHPM4AJGVRA3superseded by review
01M39W1HVW5VE4BHR8WJKRSQ29for headfeb2194b3542c215cc150cbce403698ba16a51c7Fixed in
feb2194. The sentence now lists the cases separately: a first-party.gitmodulesedit, or a first-party Gitlink change on a submodule with abranchortagselector. The held-commit rule moved to its own sentence, where it covers submodules at any depth.@ -18,3 +18,3 @@- **Under a `third-party/` path component:** `.gitmodules` and Gitlinks belong to upstream. Never add or change selectors, URLs, paths, update policies, or Gitlinks; only synchronize what upstream committed.- **Everywhere else (first-party):** each submodule needs exactly one selector, `branch = <name>` for a moving line or `tag = <name>` for an exact release. On fresh eligible default checkouts the driver fetches that branch or tag, checks the submodule out detached, and leaves the Gitlink change unstaged for the owner's next commit.- **Everywhere else (first-party):** each submodule needs exactly one selector, `branch = <name>` for a moving line or `tag = <name>` for an exact release. On fresh eligible default checkouts the driver fetches that branch or tag, checks the submodule out detached, and leaves the Gitlink change unstaged for the owner's next commit. A submodule checked out at a commit that is neither the recorded Gitlink nor held by any ref stays where it is, and its checkout is reported for a decision. A local tag whose commit no other ref holds is never replaced; the selector update fails and says so.medium — checkout-updates.md promises a stranded submodule is "reported for a decision", but the driver emits a failure row or no submodule signal at all
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39T84EZ238357RHHNS50JZ9of review01M39T0ST0ZAZJTHPM4AJGVRA3superseded by review
01M39W1HVW5VE4BHR8WJKRSQ29for headfeb2194b3542c215cc150cbce403698ba16a51c7Fixed in
feb2194. The stranded case is now a real decision row: the worktree record carriesstrandedSubmodules, and the report lists the checkout under "Needs your decision" assubmodule on a commit no ref holds: <path>; not updated. The reference says that, and says a refused tag fails the selector update and names the tag.@ -478,0 +504,5 @@fetched_object=$(git -C "$submodule_path" rev-parse --verify --quiet FETCH_HEAD 2>>"$error_path") || return 1fetched_commit=$(git -C "$submodule_path" rev-parse --verify --quiet "FETCH_HEAD^{commit}" 2>>"$error_path") || return 1local_commit=$(git -C "$submodule_path" rev-parse --verify --quiet "refs/tags/$tag^{commit}" 2>/dev/null || true)if [ -n "$local_commit" ] && [ "$local_commit" != "$fetched_commit" ] \&& [ -z "$(git -C "$submodule_path" for-each-ref --format='%(refname)' --contains "$local_commit" 2>/dev/null | grep -v -x -F "refs/tags/$tag")" ]; thenhigh — A tag selector self-locks after its first advance: update_submodule_tag refuses every later upstream tag move, and the run regresses the submodule to the stale Gitlink
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39TP8DZD5VYQ90N1XTBSVSEof review01M39T0ST0ZAZJTHPM4AJGVRA3superseded by review
01M39W1HVW5VE4BHR8WJKRSQ29for headfeb2194b3542c215cc150cbce403698ba16a51c7Fixed in
feb2194. Reproduced the second-advance refusal. A differing local tag is now also replaced when the fetched commit contains it, so an upstream tag that moves forward keeps being followed. A refusal no longer resets the submodule to the old Gitlink (initialized submodules are not re-checked-out first), and it skips only that submodule. The tag test now advances the tag twice, then checks the refusal leaves HEAD in place.@ -1123,0 +1212,7 @@git(upstreamSuperprojectPath, "commit", "--quiet", "-m", "non-overlapping upstream change");git(superprojectPath, "fetch", "--quiet", upstreamSuperprojectPath, "HEAD:refs/remotes/origin/main");const result = audit();assert.equal(result.fastForward.attempted, false);assert.equal(result.trackingUpdate.attempted, false);assert.equal(git(dependencyPath, "rev-parse", "HEAD").trim(), localCommit);medium — The new "blocks the fast-forward and the selector move" test never reaches the selector-move guard; deleting that guard leaves the whole suite green
lens
test-trimming· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39TB9X48FTAPRV48DNV18ETof review01M39T0ST0ZAZJTHPM4AJGVRA3superseded by review
01M39W1HVW5VE4BHR8WJKRSQ29for headfeb2194b3542c215cc150cbce403698ba16a51c7Fixed in
feb2194. The per-submodule guard you mutated is gone; one recursive check now gates both the fast-forward and the selector move. Tests cover a visible Gitlink change,ignore = all, and a nested submodule, and all three fail when the gate is removed.@ -16,3 +16,3 @@| Request | Flags | What changes || --- | --- | --- || Audit, clean up, "which are behind" | none | Fetch and prune `origin`; fast-forward eligible default checkouts; prune stale worktree registrations; remove proven-merged linked worktrees inside the root. || Audit, clean up, "which are behind" | none | Fetch and prune `origin`; fast-forward eligible default checkouts, then initialize their submodules and check them out at the recorded commits; in first-party default checkouts, check submodules out at their configured branch or tag, leaving the Gitlink change unstaged; prune stale worktree registrations; remove proven-merged linked worktrees inside the root. |medium — Scope selector updates to direct submodules in the mode table
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39W8KJ9N12T4ASA3Z17ZMZTof review01M39W1HVW5VE4BHR8WJKRSQ29Fixed in
29216b3. The mode table now says selector moves apply to each submodule listed in the checkout's own.gitmodules.checkout-updates.mdadds that nested submodules follow their recorded commits.@ -23,3 +23,4 @@- A repository whose work integrates somewhere other than `origin/<default>`, such as a fork with an `upstream`: `--no-remove`. Removal proves containment against `origin/<default>` only.- A repository with a custom `origin` fetch refspec: `--no-fetch --no-remove`. The fetch prunes every `refs/remotes/origin/*` ref that no server branch supplies (see [references/checkout-updates.md](references/checkout-updates.md)).- A first-party checkout whose submodules must stay where they are: `--no-fetch --no-remove`. The driver moves branch- or tag-tracked submodules even when nothing fast-forwards, and `update = none` stops only that move; after a fast-forward the driver still checks each submodule out at its recorded commit.medium — Apply the frozen-submodule warning to third-party checkouts too
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39WA0ADQAJAVPFGY8FZCD1Eof review01M39W1HVW5VE4BHR8WJKRSQ29Fixed in
29216b3. Confirmed thatsynchronize_submodulesruns for third-party checkouts too. The condition now reads "A checkout whose submodules must stay where they are" and says the post-fast-forward checkout covers third-party submodules.@ -22,8 +22,11 @@ usage() {echo " git fetch --prune origin; git worktree prune (repository-wide, including outside root);"critical — Forced worktree removal can delete an unreferenced submodule commit
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39WD94EGN0R98F393FGJN48of review01M39W1HVW5VE4BHR8WJKRSQ29Fixed in
29216b3. Reproduced withignore = alland a detached local commit: the removal went through. Right before the final status check, removal now walks every populated submodule, live. It keeps the worktree asjudgment/submodule-local-workwhen a submodule has uncommitted files, a stash, a branch commit no remote-tracking ref holds, or an unrecorded HEAD no remote-tracking ref holds. Only remote-tracking refs count, because the submodule's local refs are deleted with the worktree.removal-gates.mddocuments the gate and says ignored files inside submodules are still not scanned.@ -478,0 +503,7 @@fetched_object=$(git -C "$submodule_path" rev-parse --verify --quiet FETCH_HEAD 2>>"$error_path") || return 1fetched_commit=$(git -C "$submodule_path" rev-parse --verify --quiet "FETCH_HEAD^{commit}" 2>>"$error_path") || return 1local_commit=$(git -C "$submodule_path" rev-parse --verify --quiet "refs/tags/$tag^{commit}" 2>/dev/null || true)if [ -n "$local_commit" ] && [ "$local_commit" != "$fetched_commit" ] \&& ! git -C "$submodule_path" merge-base --is-ancestor "$local_commit" "$fetched_commit" 2>/dev/null \&& [ -z "$(git -C "$submodule_path" for-each-ref --format='%(refname)' --contains "$local_commit" 2>/dev/null | grep -v -x -F "refs/tags/$tag")" ]; thenhigh — A local tag on a non-commit object is silently overwritten
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39WAEFSWWWYD9ARFQ9YDDRYof review01M39W1HVW5VE4BHR8WJKRSQ29Fixed in
29216b3. Reproduced with an annotated localv1on a blob.update_submodule_tagnow checks whether the tag ref exists separately from whether it peels to a commit. A tag that does not peel to a commit is kept and the update fails naming it. A local tag already on the fetched commit is left as it is. New test: "a local selector tag that does not point at a commit is kept".@ -509,5 +548,7 @@[ "$update_mode" != none ] || continueif ! git -C "$checkout_path" submodule update --init --checkout -- "$configured_path" >>"$output_path" 2>>"$error_path"; thensubmodule_path="$checkout_path/$configured_path"if [ ! -e "$submodule_path/.git" ] \&& ! git -C "$checkout_path" submodule update --init --checkout -- "$configured_path" >>"$output_path" 2>>"$error_path"; thenreturn 1fimedium — An extra .gitmodules entry can move an unrelated nested repository
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39W9TK28DR3W6EA1VDTDZDZof review01M39W1HVW5VE4BHR8WJKRSQ29Fixed in
29216b3. Reproduced: a stray clone at a.gitmodulespath with no Gitlink was moved. The selector loop now skips any entry whose path is not a Gitlink in HEAD. New test: "a .gitmodules entry without a Gitlink never moves the repository at its path".@ -1414,11 +1460,20 @@ audit_worktree() {write_submodule_metadata "$worktree_path" "$submodule_metadata_path"submodule_metadata_ok=$(jq -r '.ok' "$submodule_metadata_path")if [ "$outside_root" = false ] && [ -f "$worktree_path/.gitmodules" ]; thenif stranded_output=$(list_stranded_submodule_commits "$worktree_path" 2>/dev/null); thencritical — Deleting .gitmodules bypasses the unique submodule commit guard
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39WCSXYV6VTH081YC1MZXDRof review01M39W1HVW5VE4BHR8WJKRSQ29Fixed in
29216b3. Reproduced: with.gitmodulesdeleted andignore = allin local config, the fast-forward stranded the commit. The walk now reads Gitlinks from HEAD and the index, not.gitmodules, and runs on every checkout. New test: "a deleted .gitmodules does not hide a submodule commit no ref holds".@ -310,6 +312,12 @@ export function formatAuditReport(report: AuditReport): string {const isMain = worktree.registration?.isMain === true;const onDefault = worktree.status?.branch.isDetached === false && worktree.status.branch.current === defaultBranch;if ((isMain || onDefault) && worktree.strandedSubmodules?.length !== 0) {const stranded = worktree.strandedSubmodules;const why = stranded === null ? "submodule check failed" : `submodule on a commit no ref holds: ${listPaths(stranded)}`;high — Renderer crashes on saved schema-5 reports lacking strandedSubmodules
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39WEH2JS8JNKF2XVF3CWAGAof review01M39W1HVW5VE4BHR8WJKRSQ29medium — Report a failed submodule check as a failure, not a user decision
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39W6HVRYAA5F505Z0GTWG2Dof review01M39W1HVW5VE4BHR8WJKRSQ29Fixed in
29216b3by bumping the report schema to 6 in the driver and the renderer. A saved version-5 report is now refused withexpected report schemaVersion 6, got 5instead of crashing. That follows the repo's rule against compatibility shims: rerun the audit to get a current report.Fixed in
29216b3. A nullstrandedSubmodulesnow renders as the failuresubmodule check failed; checkout not updated. Only a non-empty list of submodule paths goes under "Needs your decision".The round-2 report (review
01M39T0ST0ZAZJTHPM4AJGVRA3, head85dfb5b) had one finding with no inline thread. Recording its outcome here, because that report has since been rewritten.feb2194. Reproduced both routes you described: a nested submodule, andignore = all. The per-path guard is gone. Before any fast-forward or selector move, the driver now walks every populated submodule withgit submodule foreach --recursive. If any HEAD is neither the commit its superproject records nor held by a ref, that checkout gets no fast-forward and no selector move, and the report lists it as a decision. The tests cover both routes.fix(audit-git-checkouts): audits should not strand submodule commits or local tags that no other ref holdsto fix(audit-git-checkouts): audits should never lose commits that only a submodule holds@ -16,3 +16,3 @@| Request | Flags | What changes || --- | --- | --- || Audit, clean up, "which are behind" | none | Fetch and prune `origin`; fast-forward eligible default checkouts; prune stale worktree registrations; remove proven-merged linked worktrees inside the root. || Audit, clean up, "which are behind" | none | Fetch and prune `origin`; fast-forward eligible default checkouts, then initialize their submodules and check them out at the recorded commits; in first-party default checkouts, check each submodule listed in the checkout's own `.gitmodules` out at its configured branch or tag, leaving the Gitlink change unstaged; prune stale worktree registrations; remove proven-merged linked worktrees inside the root. |low — Mode table cell restates submodule mechanics already carried by the bullet below it and by checkout-updates.md
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3A0590N60944F6QWN1CRD3Gof review01M39ZTYXRMDP1E05ME1J23DF1@ -23,3 +23,4 @@- A repository whose work integrates somewhere other than `origin/<default>`, such as a fork with an `upstream`: `--no-remove`. Removal proves containment against `origin/<default>` only.- A repository with a custom `origin` fetch refspec: `--no-fetch --no-remove`. The fetch prunes every `refs/remotes/origin/*` ref that no server branch supplies (see [references/checkout-updates.md](references/checkout-updates.md)).- A checkout whose submodules must stay where they are: `--no-fetch --no-remove`. After a fast-forward the driver checks every submodule, third-party ones included, out at its recorded commit. In first-party checkouts it also moves branch- or tag-tracked submodules when nothing fast-forwards, and `update = none` stops only that move.medium — SKILL.md says selector moves happen "when nothing fast-forwards", but they also run after a successful fast-forward
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M39ZZZYMYDQRPQ3RYHRT65DRof review01M39ZTYXRMDP1E05ME1J23DF1@ -18,3 +18,3 @@- **Under a `third-party/` path component:** `.gitmodules` and Gitlinks belong to upstream. Never add or change selectors, URLs, paths, update policies, or Gitlinks; only synchronize what upstream committed.- **Everywhere else (first-party):** each submodule needs exactly one selector, `branch = <name>` for a moving line or `tag = <name>` for an exact release. On fresh eligible default checkouts the driver fetches that branch or tag, checks the submodule out detached, and leaves the Gitlink change unstaged for the owner's next commit.- **Everywhere else (first-party):** each submodule needs exactly one selector, `branch = <name>` for a moving line or `tag = <name>` for an exact release. On fresh eligible default checkouts the driver fetches that branch or tag, checks the submodule out detached, and leaves the Gitlink change unstaged for the owner's next commit. Only submodules listed in the checkout's own `.gitmodules` that are Gitlinks in HEAD move; nested submodules follow their recorded commits. A local tag of the configured name on the same commit is kept as it is. One on another commit is replaced only when the fetched tag's commit contains that commit or another ref holds it. Otherwise, or when the local tag does not point at a commit, that submodule stays where it is, the local tag is kept, and the selector update fails naming the tag.low — checkout-updates.md claims nested submodules follow their recorded commits, but the selector move checks out without --recurse-submodules
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3A07TG1XVJNNE29C5SVFME2of review01M39ZTYXRMDP1E05ME1J23DF1@ -27,0 +26,5 @@echo " submodule selectors advanced outside third-party/ paths; removal of in-root linked"echo " worktrees that pass every removal gate. A checkout with a submodule, at any depth, on a"echo " commit neither recorded nor held by a ref gets no fast-forward or selector move; a local"echo " selector tag is replaced only when the fetched tag or another ref keeps its commit; a"echo " worktree whose submodules hold work no remote-tracking ref keeps is not removed."low — New --help lines say a ref "keeps" a commit, a third verb for the skill's "holds", in a clause that garden-paths
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3A02CWGVN82CZM0HJRW6KZYof review01M39ZTYXRMDP1E05ME1J23DF1@ -478,0 +496,7 @@mode=$2prefix=${3:-}gitlink_paths=$({git -C "$superproject_path" ls-files --stage -zgit -C "$superproject_path" ls-tree -r -z --full-tree HEAD 2>/dev/null} | tr '\0' '\n' | awk -F '\t' '$1 ~ /^160000 / {print $2}' | sort -u) || return 1medium — An unborn-HEAD checkout fails the new submodule walk, so every commit-less repository is reported as "submodule check failed"
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3A08NB6MVVT98N5F4PBSKRHof review01M39ZTYXRMDP1E05ME1J23DF1Fixed in
e9f38b7. Reproduced: pipefail carriedls-tree HEAD's exit 128 out of the walk. It is now|| true, so an unborn repository falls back to the index and yields nothing. New test: "a repository with no commits has no submodules to check". This was a regression this PR introduced into every commit-less repository, so I fixed it despite the round-4 gate.@ -478,0 +524,9 @@[ "$head" != "$recorded" ] && [ -z "$(git -C "$submodule_path" for-each-ref --count=1 --contains "$head")" ]returnfiif [ "$head" != "$recorded" ] && [ -z "$(git -C "$submodule_path" for-each-ref --count=1 --contains "$head" refs/remotes)" ]; thenreturn 0fi[ -n "$(git -C "$submodule_path" status --porcelain --untracked-files=normal --ignore-submodules=all)" ] && return 0git -C "$submodule_path" rev-parse --verify --quiet refs/stash >/dev/null && return 0[ -n "$(git -C "$submodule_path" rev-list -n 1 --branches --not --remotes)" ]high — Removal gate treats a recorded Gitlink as proof of survival, destroying unpushed submodule commits and leaving an unresolvable Gitlink on the default branch
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3A0C1NTK178K7H5JFY9Z7Z0of review01M39ZTYXRMDP1E05ME1J23DF1medium — Removal gate checks only branches, so a submodule commit held only by a local tag is deleted with the worktree
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3A0EVG61DHR3PHC417ZZ96Fof review01M39ZTYXRMDP1E05ME1J23DF1Fixed in
e9f38b7. Reproduced with a local submodule commit recorded in a merged branch: the worktree was removed. The removal gate now requires a remote-tracking ref for the submodule HEAD even when it equals the recorded Gitlink, because a Gitlink names the commit without keeping it. The removal test now covers this and fails with the old exemption restored.Fixed in
e9f38b7. Reproduced: the tag-only commit went with the worktree. The last clause is nowrev-list --branches --tags --not --remotes, and the removal test covers a tag-only commit.@ -312,1 +320,4 @@const onDefault = worktree.status?.branch.isDetached === false && worktree.status.branch.current === defaultBranch;const stranded = worktree.strandedSubmodules ?? [];if ((isMain || onDefault) && stranded.length > 0) {const why = `submodule on a commit no ref holds: ${listPaths(stranded)}; not updated`;low — Decision-row note says "submodule on a commit no ref holds" but prints a list of paths
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3A040D2C8J5T211YSFAAMWYof review01M39ZTYXRMDP1E05ME1J23DF1Round-4 outcomes for review
01M39ZTYXRMDP1E05ME1J23DF1(head29216b3). This PR is now in round 4, so only clear, reproduced data-loss bugs and one regression were fixed, ine9f38b7. The other items below are real but change only wording, so they are acknowledged and deferred to a follow-up PR with the exact fix.Fixed in
e9f38b7:Deferred, acknowledged (real, documentation or wording only):
--helpstill says "schema 5" (no inline thread). Inscripts/audit-checkouts.shusage(), change "Writes one JSON report (schema 5)" to "(schema 6)".SKILL.md, the "A merged worktree kept by a gate (…)" bullet should add "submodule work" to the list.SKILL.md's stricter-mode bullet, say "In first-party checkouts it also moves branch- or tag-tracked submodules, with or without a fast-forward".SKILL.md, shorten the cell to "check submodules out at their recorded commits or configured selectors (see below)".references/checkout-updates.md, say "The selector move does not recurse; nested submodules move only with the post-fast-forward checkout, to their recorded commits."--helpuses "keeps" (87700). Inscripts/audit-checkouts.shusage(), use "holds" and split the tag clause into its own sentence.scripts/render-audit-report.ts, render "submodules on a commit no ref holds: ".@ -16,3 +16,3 @@| Request | Flags | What changes || --- | --- | --- || Audit, clean up, "which are behind" | none | Fetch and prune `origin`; fast-forward eligible default checkouts; prune stale worktree registrations; remove proven-merged linked worktrees inside the root. || Audit, clean up, "which are behind" | none | Fetch and prune `origin`; fast-forward eligible default checkouts, then initialize their submodules and check them out at the recorded commits; in first-party default checkouts, check each submodule listed in the checkout's own `.gitmodules` out at its configured branch or tag, leaving the Gitlink change unstaged; prune stale worktree registrations; remove proven-merged linked worktrees inside the root. |low — Mode-selection table cell tripled in length and buries a "check ... out" split across ten words
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3A1NZF0PFD14A40XTQAMG5Fof review01M3A1CRCCTK8AGTYWGXAJDQPF@ -23,3 +23,4 @@- A repository whose work integrates somewhere other than `origin/<default>`, such as a fork with an `upstream`: `--no-remove`. Removal proves containment against `origin/<default>` only.- A repository with a custom `origin` fetch refspec: `--no-fetch --no-remove`. The fetch prunes every `refs/remotes/origin/*` ref that no server branch supplies (see [references/checkout-updates.md](references/checkout-updates.md)).- A checkout whose submodules must stay where they are: `--no-fetch --no-remove`. After a fast-forward the driver checks every submodule, third-party ones included, out at its recorded commit. In first-party checkouts it also moves branch- or tag-tracked submodules when nothing fast-forwards, and `update = none` stops only that move.medium — SKILL.md mode bullet says selector moves happen "when nothing fast-forwards", but they also run after a successful fast-forward
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3A1KZV50Q8KGG3GWKSFDW1Gof review01M3A1CRCCTK8AGTYWGXAJDQPF@ -5,3 +5,3 @@## What the driver already updatesThe driver fast-forwards a default-branch checkout only when its comparison is fresh, it has no local commits, and it is strictly behind `origin/<default>`. A dirty checkout qualifies only when every changed path is deferred guidance (`AGENTS.md`, `.agents/**`, at any depth) or first-party `.gitmodules`/Gitlink maintenance, and upstream did not touch those paths. The merge runs `--ff-only` with `merge.autostash=false`, because autostash would round-trip the tree through a stash and silently unstage staged guidance. After a fast-forward it synchronizes committed submodules to the recorded Gitlinks.The driver fast-forwards a default-branch checkout only when its comparison is fresh, it has no local commits, and it is strictly behind `origin/<default>`. A dirty checkout qualifies only when every changed path is deferred guidance (`AGENTS.md`, `.agents/**`, at any depth), a first-party `.gitmodules` edit, or a first-party Gitlink change on a submodule with a `branch` or `tag` selector, and upstream did not touch those paths. The merge runs `--ff-only` with `merge.autostash=false`, because autostash would round-trip the tree through a stash and silently unstage staged guidance. After a fast-forward it initializes committed submodules and checks them out at the recorded Gitlinks. When any populated submodule, at any depth, found from the Gitlinks rather than `.gitmodules`, is checked out at a commit that its superproject does not record and no ref holds, the driver skips the fast-forward and every selector move for that checkout, and the report lists it under "Needs your decision" with the submodule's path.medium — The stranded-submodule rule says "no ref holds" without naming the repository, and the refs searched are the submodule's, not the superproject's
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3A1MRYKK826KYZJJDN3HZ2Gof review01M3A1CRCCTK8AGTYWGXAJDQPF@ -27,0 +26,5 @@echo " submodule selectors advanced outside third-party/ paths; removal of in-root linked"echo " worktrees that pass every removal gate. A checkout with a submodule, at any depth, on a"echo " commit neither recorded nor held by a ref gets no fast-forward or selector move; a local"echo " selector tag is replaced only when the fetched tag or another ref keeps its commit; a"echo " worktree whose submodules hold work no remote-tracking ref keeps is not removed."low —
--helpalternates "held by", "keeps", and "holds" for one reachability relation, and its last clause stacks three bare verbslens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3A1PSQ4NCBQKYC6B64XXAAXof review01M3A1CRCCTK8AGTYWGXAJDQPF@ -478,0 +505,4 @@[ -n "$gitlink_path" ] || continuesubmodule_path="$superproject_path/$gitlink_path"[ -e "$submodule_path/.git" ] || continuehead=$(git -C "$submodule_path" rev-parse --verify --quiet HEAD) || continuehigh — A submodule whose HEAD cannot be read is silently reported as holding no local work, so the removal gate deletes it
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3A2DSD56MWBG1DRWY1NS6KMof review01M3A1CRCCTK8AGTYWGXAJDQPFFixed in
7f0da38. Reproduced withignore = all, a localwipbranch in the submodule, and a brokencore.worktree: the walk returned 0 with no output. It now printscannot read submodule <path>and fails. Removal becomesoperational/gate-check-failed, and the update check becomes a failure row. The removal test now covers an unreadable submodule and fails againste9f38b7.@ -1123,0 +1281,5 @@assert.equal(refused.trackingUpdate.attempted, true);assert.equal(refused.trackingUpdate.ok, false);assert.match(refused.trackingUpdate.error, /local tag v1 .* no other ref holds/);assert.equal(git(dependencyPath, "rev-parse", "v1^{commit}").trim(), localCommit);assert.equal(git(dependencyPath, "rev-parse", "HEAD").trim(), thirdCommit);low — The refused-tag case asserts the submodule HEAD equals the commit a successful move would also produce, so "stays where it is" is unprotected
lens
test-trimming· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3A1TNK4DSZ5Y0F097EAY7S6of review01M3A1CRCCTK8AGTYWGXAJDQPFRound-5 outcomes for review
01M3A1CRCCTK8AGTYWGXAJDQPF(heade9f38b7). From round 5, a new push goes only to clear, severe bugs.Fixed in
7f0da38:Acknowledged and deferred to a follow-up PR. These change only wording or test precision:
--helpstill says "schema 5" (no inline thread; same as round 4). Inscripts/audit-checkouts.shusage(), drop the number: "Writes one JSON report to stdout".SKILL.md, use "even when no fast-forward runs".references/checkout-updates.mdandusage(), write "no ref in the submodule holds".SKILL.md, shorten it to "fast-forward eligible default checkouts and move their submodules".--helpuses "held by", "keeps" and "holds" (87750; same as 87700). Use "holds" throughout, and split the removal clause as the finding proposes.scripts/audit-checkouts.test.mjs, park the submodule onsecondCommitbefore the refused run and assert HEAD stays there.@ -16,3 +16,3 @@| Request | Flags | What changes || --- | --- | --- || Audit, clean up, "which are behind" | none | Fetch and prune `origin`; fast-forward eligible default checkouts; prune stale worktree registrations; remove proven-merged linked worktrees inside the root. || Audit, clean up, "which are behind" | none | Fetch and prune `origin`; fast-forward eligible default checkouts, then initialize their submodules and check them out at the recorded commits; in first-party default checkouts, check each submodule listed in the checkout's own `.gitmodules` out at its configured branch or tag, leaving the Gitlink change unstaged; prune stale worktree registrations; remove proven-merged linked worktrees inside the root. |low — Mode-table cell grew to a 70-word semicolon chain that splits "check … out" across nine words and restates the reference
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BGV3J1HRMKZ1EBQ61CS1FVof review01M3BGFRYK9R81EWETEHWA9X9E@ -23,3 +23,4 @@- A repository whose work integrates somewhere other than `origin/<default>`, such as a fork with an `upstream`: `--no-remove`. Removal proves containment against `origin/<default>` only.- A repository with a custom `origin` fetch refspec: `--no-fetch --no-remove`. The fetch prunes every `refs/remotes/origin/*` ref that no server branch supplies (see [references/checkout-updates.md](references/checkout-updates.md)).- A checkout whose submodules must stay where they are: `--no-fetch --no-remove`. After a fast-forward the driver checks every submodule, third-party ones included, out at its recorded commit. In first-party checkouts it also moves branch- or tag-tracked submodules when nothing fast-forwards, and `update = none` stops only that move.medium — SKILL.md restricts first-party selector moves to checkouts where "nothing fast-forwards", but they also run after a successful fast-forward
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BGP9SEGPG8CRH5HFRCE6FVof review01M3BGFRYK9R81EWETEHWA9X9E@ -5,3 +5,3 @@## What the driver already updatesThe driver fast-forwards a default-branch checkout only when its comparison is fresh, it has no local commits, and it is strictly behind `origin/<default>`. A dirty checkout qualifies only when every changed path is deferred guidance (`AGENTS.md`, `.agents/**`, at any depth) or first-party `.gitmodules`/Gitlink maintenance, and upstream did not touch those paths. The merge runs `--ff-only` with `merge.autostash=false`, because autostash would round-trip the tree through a stash and silently unstage staged guidance. After a fast-forward it synchronizes committed submodules to the recorded Gitlinks.The driver fast-forwards a default-branch checkout only when its comparison is fresh, it has no local commits, and it is strictly behind `origin/<default>`. A dirty checkout qualifies only when every changed path is deferred guidance (`AGENTS.md`, `.agents/**`, at any depth), a first-party `.gitmodules` edit, or a first-party Gitlink change on a submodule with a `branch` or `tag` selector, and upstream did not touch those paths. The merge runs `--ff-only` with `merge.autostash=false`, because autostash would round-trip the tree through a stash and silently unstage staged guidance. After a fast-forward it initializes committed submodules and checks them out at the recorded Gitlinks. When any populated submodule, at any depth, found from the Gitlinks rather than `.gitmodules`, is checked out at a commit that its superproject does not record and no ref holds, the driver skips the fast-forward and every selector move for that checkout, and the report lists it under "Needs your decision" with the submodule's path.low — The stranded-submodule rule is a 57-word sentence that stacks three modifiers before its subject reaches a verb
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BGSMFG15D1KAC28GVZRRW5of review01M3BGFRYK9R81EWETEHWA9X9E@ -18,3 +18,3 @@- **Under a `third-party/` path component:** `.gitmodules` and Gitlinks belong to upstream. Never add or change selectors, URLs, paths, update policies, or Gitlinks; only synchronize what upstream committed.- **Everywhere else (first-party):** each submodule needs exactly one selector, `branch = <name>` for a moving line or `tag = <name>` for an exact release. On fresh eligible default checkouts the driver fetches that branch or tag, checks the submodule out detached, and leaves the Gitlink change unstaged for the owner's next commit.- **Everywhere else (first-party):** each submodule needs exactly one selector, `branch = <name>` for a moving line or `tag = <name>` for an exact release. On fresh eligible default checkouts the driver fetches that branch or tag, checks the submodule out detached, and leaves the Gitlink change unstaged for the owner's next commit. Only submodules listed in the checkout's own `.gitmodules` that are Gitlinks in HEAD move; nested submodules follow their recorded commits. A local tag of the configured name on the same commit is kept as it is. One on another commit is replaced only when the fetched tag's commit contains that commit or another ref holds it. Otherwise, or when the local tag does not point at a commit, that submodule stays where it is, the local tag is kept, and the selector update fails naming the tag.medium — The new submodule prose uses "contains", "holds", and "keeps" interchangeably for ref reachability, twice inside one sentence
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BGQTHFHS7SX6CGGV98VD1Yof review01M3BGFRYK9R81EWETEHWA9X9Emedium — "nested submodules follow their recorded commits" promises an update the selector path never performs
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BGRV25RCM405CYVBKKMHTDof review01M3BGFRYK9R81EWETEHWA9X9E@ -18,3 +19,3 @@A worktree lock is an owner pin that expires seven days after its `locked` file's mtime. `git worktree lock` refuses an already locked worktree, so the mtime dates from the original lock; to renew a pin, unlock and lock again. A future-dated lock counts as young. The driver unlocks an expired lock only after every other gate passes, immediately before removal. It reads the lock once, at the gate: a pin renewed during the containment proof and ignored scan that follow is unlocked anyway. If unlock fails, the outcome is `operational/removal-failed`. If removal then fails, the worktree stays unlocked and the next run evaluates it from scratch. Expiry deliberately overrides pins agents leave behind.Worktrees containing `.gitmodules` are removed with `--force`, because Git otherwise refuses any worktree with submodules; the repeated status check replaces the check `--force` disables. Submodules get no audit or veto of their own: superproject containment says nothing about a submodule's history or ignored files.Worktrees containing `.gitmodules` are removed with `--force`, because Git otherwise refuses any worktree with submodules; the repeated status check replaces the check `--force` disables. Removal also deletes each submodule's Git directory, local branches and stash included, so right before the status check the driver walks every populated submodule at any depth. A submodule keeps the worktree (`judgment/submodule-local-work`, paths in `removal.error`) when it has uncommitted files, a stash, a branch or tag commit no remote-tracking ref holds, or a HEAD no remote-tracking ref holds. A Gitlink names a commit without keeping it, so a recorded HEAD needs a remote-tracking ref too. Ignored files inside submodules are not scanned.low — "right before the status check" is ambiguous in a document that defines two status-check gates, and points at the wrong one
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BGW03X9ZGQ1QSM494RBYF5of review01M3BGFRYK9R81EWETEHWA9X9E@ -26,1 +25,6 @@echo " third-party/ paths; removal of in-root linked worktrees that pass every removal gate."echo " submodule init and checkout at recorded Gitlinks after a fast-forward; branch/tag"echo " submodule selectors advanced outside third-party/ paths; removal of in-root linked"echo " worktrees that pass every removal gate. A checkout with a submodule, at any depth, on a"echo " commit neither recorded nor held by a ref gets no fast-forward or selector move; a local"echo " selector tag is replaced only when the fetched tag or another ref keeps its commit; a"echo " worktree whose submodules hold work no remote-tracking ref keeps is not removed."medium —
--help's "Writes in a default run" section now ends with three refusals, one of them a garden-path clauselens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BGQ09SVGZABQEBS1M2V004of review01M3BGFRYK9R81EWETEHWA9X9E@ -478,0 +500,4 @@gitlink_paths=$({git -C "$superproject_path" ls-files --stage -zgit -C "$superproject_path" ls-tree -r -z --full-tree HEAD 2>/dev/null || true} | tr '\0' '\n' | awk -F '\t' '$1 ~ /^160000 / {print $2}' | sort -u) || return 1low — The
|| return 1guard on the Gitlink enumeration can never fire, so an unreadable index silently degrades the submodule walk to HEAD-onlylens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BHE917P1HSFWK0RCQYEHN1of review01M3BGFRYK9R81EWETEHWA9X9E@ -478,0 +530,4 @@returnfi[ -z "$(git -C "$submodule_path" for-each-ref --count=1 --contains "$head" refs/remotes)" ] && return 0[ -n "$(git -C "$submodule_path" status --porcelain --untracked-files=normal --ignore-submodules=all)" ] && return 0high — submodule_holds_local_work judges a submodule clean when its
git statusfails, so a forced worktree removal silently deletes uncommitted submodule worklens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BHBYWXC3Z8BY113BPM6MZNof review01M3BGFRYK9R81EWETEHWA9X9EFixed in
0cdf081. Reproduced with a corrupt submodule index andignore = all: the walk returned nothing. Every submodule query now fails closed.submodule_holds_local_workreturns 2 on a failedfor-each-ref,statusorrev-list, and the walk turns that intocannot inspect submodule <path>, so removal becomesoperational/gate-check-failed. The removal test covers a corrupt index with uncommitted work and fails against7f0da38.@ -478,0 +532,4 @@[ -z "$(git -C "$submodule_path" for-each-ref --count=1 --contains "$head" refs/remotes)" ] && return 0[ -n "$(git -C "$submodule_path" status --porcelain --untracked-files=normal --ignore-submodules=all)" ] && return 0git -C "$submodule_path" rev-parse --verify --quiet refs/stash >/dev/null && return 0[ -n "$(git -C "$submodule_path" rev-list -n 1 --branches --tags --not --remotes)" ]medium — Removal gate counts upstream tags inside a submodule as local work, permanently blocking removal of merged worktrees
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BH2BM6K1SSQ81M73VNE1GDof review01M3BGFRYK9R81EWETEHWA9X9E@ -1415,2 +1527,4 @@submodule_metadata_ok=$(jq -r '.ok' "$submodule_metadata_path")if [ "$outside_root" = false ]; thenif stranded_output=$(list_submodules_with_local_work "$worktree_path" update 2>/dev/null); thenmedium — The update-mode submodule walk discards its only diagnostic, so a checkout whose submodule cannot be read stops being updated with no reportable reason
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BH809Z5NDAKJ8VPKSNTS1Qof review01M3BGFRYK9R81EWETEHWA9X9E@ -312,1 +320,4 @@const onDefault = worktree.status?.branch.isDetached === false && worktree.status.branch.current === defaultBranch;const stranded = worktree.strandedSubmodules ?? [];if ((isMain || onDefault) && stranded.length > 0) {const why = `submodule on a commit no ref holds: ${listPaths(stranded)}; not updated`;low — Report cell says "submodule on a commit no ref holds" in the singular while printing a comma-separated list of submodules
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BGTKESPB052Q5S1Y8DMECTof review01M3BGFRYK9R81EWETEHWA9X9ERound-6 outcomes for review
01M3BGFRYK9R81EWETEHWA9X9E(head7f0da38). From round 6 only data loss, data corruption, or a security hole gets a new push.Fixed in
0cdf081:git statusreads as clean (87929): agreed; it is data loss. See the inline thread.Real bug left unfixed (87934, medium). The removal gate counts tags inside a submodule as local work unless a remote-tracking branch holds their commit. An upstream tag on a commit that no remote branch contains therefore keeps a merged worktree forever. It errs toward keeping work, so nothing is lost, but such worktrees need manual removal until the follow-up lands. Fix, in
scripts/audit-checkouts.shsubmodule_holds_local_work: keep--branchesin the reachability test, and count a local tag only whengit ls-remote --tags originlacks it or names a different object.Acknowledged and deferred to a follow-up PR:
audit_worktree, redirect the walk's stderr to astranded-submodules.errorfile and carry it asstrandedSubmodulesError.stepFailureshould append its first line.ls-filesfailure is swallowed. Inlist_submodules_with_local_work, capturels-files --stage -zseparately with|| return 1before combining it withls-tree. An unreadable superproject index already fails the superproject status gate, so removal is not exposed.references/checkout-updates.md, use the finding's wording: a nested selector is ignored, and a nested submodule stays where the last Gitlink sync put it.references/removal-gates.md, say "immediately before the second status check".--help. These are the wording items deferred in rounds 4 and 5: "even when no fast-forward runs"; "holds" throughout; a shorter--helpsection and mode-table cell; splitting the 57-word stranded-submodule sentence; the plural "submodules on a commit no ref holds"; and removing the schema number fromusage().@ -16,3 +16,3 @@| Request | Flags | What changes || --- | --- | --- || Audit, clean up, "which are behind" | none | Fetch and prune `origin`; fast-forward eligible default checkouts; prune stale worktree registrations; remove proven-merged linked worktrees inside the root. || Audit, clean up, "which are behind" | none | Fetch and prune `origin`; fast-forward eligible default checkouts, then initialize their submodules and check them out at the recorded commits; in first-party default checkouts, check each submodule listed in the checkout's own `.gitmodules` out at its configured branch or tag, leaving the Gitlink change unstaged; prune stale worktree registrations; remove proven-merged linked worktrees inside the root. |low — The mode table's "What changes" cell now restates the submodule behavior that the bullet three lines below and the reference already give
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BJD43THCCSVM63RWPDHTDZof review01M3BJ3S1CY1F1S8BW4PD0P9FC@ -23,3 +23,4 @@- A repository whose work integrates somewhere other than `origin/<default>`, such as a fork with an `upstream`: `--no-remove`. Removal proves containment against `origin/<default>` only.- A repository with a custom `origin` fetch refspec: `--no-fetch --no-remove`. The fetch prunes every `refs/remotes/origin/*` ref that no server branch supplies (see [references/checkout-updates.md](references/checkout-updates.md)).- A checkout whose submodules must stay where they are: `--no-fetch --no-remove`. After a fast-forward the driver checks every submodule, third-party ones included, out at its recorded commit. In first-party checkouts it also moves branch- or tag-tracked submodules when nothing fast-forwards, and `update = none` stops only that move.medium — "moves branch- or tag-tracked submodules when nothing fast-forwards" understates when selector moves run: they also run right after a successful fast-forward
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BJAY8E33N13G2MRZKFY5X3of review01M3BJ3S1CY1F1S8BW4PD0P9FC@ -5,3 +5,3 @@## What the driver already updatesThe driver fast-forwards a default-branch checkout only when its comparison is fresh, it has no local commits, and it is strictly behind `origin/<default>`. A dirty checkout qualifies only when every changed path is deferred guidance (`AGENTS.md`, `.agents/**`, at any depth) or first-party `.gitmodules`/Gitlink maintenance, and upstream did not touch those paths. The merge runs `--ff-only` with `merge.autostash=false`, because autostash would round-trip the tree through a stash and silently unstage staged guidance. After a fast-forward it synchronizes committed submodules to the recorded Gitlinks.The driver fast-forwards a default-branch checkout only when its comparison is fresh, it has no local commits, and it is strictly behind `origin/<default>`. A dirty checkout qualifies only when every changed path is deferred guidance (`AGENTS.md`, `.agents/**`, at any depth), a first-party `.gitmodules` edit, or a first-party Gitlink change on a submodule with a `branch` or `tag` selector, and upstream did not touch those paths. The merge runs `--ff-only` with `merge.autostash=false`, because autostash would round-trip the tree through a stash and silently unstage staged guidance. After a fast-forward it initializes committed submodules and checks them out at the recorded Gitlinks. When any populated submodule, at any depth, found from the Gitlinks rather than `.gitmodules`, is checked out at a commit that its superproject does not record and no ref holds, the driver skips the fast-forward and every selector move for that checkout, and the report lists it under "Needs your decision" with the submodule's path.medium — The stranded-submodule rule is one 57-word sentence whose "found from the Gitlinks rather than
.gitmodules" aside drops the consequence that makes it worth statinglens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BJBFWMJRZTY6TQRHTN69G0of review01M3BJ3S1CY1F1S8BW4PD0P9FC@ -7,3 +7,3 @@The driver fast-forwards a default-branch checkout only when its comparison is fresh, it has no local commits, and it is strictly behind `origin/<default>`. A dirty checkout qualifies only when every changed path is deferred guidance (`AGENTS.md`, `.agents/**`, at any depth) or first-party `.gitmodules`/Gitlink maintenance, and upstream did not touch those paths. The merge runs `--ff-only` with `merge.autostash=false`, because autostash would round-trip the tree through a stash and silently unstage staged guidance. After a fast-forward it synchronizes committed submodules to the recorded Gitlinks.The driver fast-forwards a default-branch checkout only when its comparison is fresh, it has no local commits, and it is strictly behind `origin/<default>`. A dirty checkout qualifies only when every changed path is deferred guidance (`AGENTS.md`, `.agents/**`, at any depth), a first-party `.gitmodules` edit, or a first-party Gitlink change on a submodule with a `branch` or `tag` selector, and upstream did not touch those paths. The merge runs `--ff-only` with `merge.autostash=false`, because autostash would round-trip the tree through a stash and silently unstage staged guidance. After a fast-forward it initializes committed submodules and checks them out at the recorded Gitlinks. When any populated submodule, at any depth, found from the Gitlinks rather than `.gitmodules`, is checked out at a commit that its superproject does not record and no ref holds, the driver skips the fast-forward and every selector move for that checkout, and the report lists it under "Needs your decision" with the submodule's path.A fast-forward, submodule sync, or selector update can fail after an earlier step succeeded. Inspect the actual HEAD, index, and submodule state before repairing; never describe a failed multi-step update as atomic, and never force a refused merge.low — The reference's failure list omits the new "submodule check failed; checkout not updated" outcome and sends its reader to repair a state the driver never touched
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BJFJHNJM26BM2SDNQ4X9VXof review01M3BJ3S1CY1F1S8BW4PD0P9FC@ -18,3 +18,3 @@- **Under a `third-party/` path component:** `.gitmodules` and Gitlinks belong to upstream. Never add or change selectors, URLs, paths, update policies, or Gitlinks; only synchronize what upstream committed.- **Everywhere else (first-party):** each submodule needs exactly one selector, `branch = <name>` for a moving line or `tag = <name>` for an exact release. On fresh eligible default checkouts the driver fetches that branch or tag, checks the submodule out detached, and leaves the Gitlink change unstaged for the owner's next commit.- **Everywhere else (first-party):** each submodule needs exactly one selector, `branch = <name>` for a moving line or `tag = <name>` for an exact release. On fresh eligible default checkouts the driver fetches that branch or tag, checks the submodule out detached, and leaves the Gitlink change unstaged for the owner's next commit. Only submodules listed in the checkout's own `.gitmodules` that are Gitlinks in HEAD move; nested submodules follow their recorded commits. A local tag of the configured name on the same commit is kept as it is. One on another commit is replaced only when the fetched tag's commit contains that commit or another ref holds it. Otherwise, or when the local tag does not point at a commit, that submodule stays where it is, the local tag is kept, and the selector update fails naming the tag.medium — "nested submodules follow their recorded commits" names a checkout the selector path never performs, and reads as a guarantee
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BJCEANJFWKDH83C7BJ4JFVof review01M3BJ3S1CY1F1S8BW4PD0P9FC@ -44,6 +45,7 @@ The outcome names the first blocker only; later gates may never have run. After| `judgment/hidden-index-flags` | Have the owner clear the flags they set, prove the revealed tree clean, then rerun. In `git ls-files -v`, a lowercase letter is assume-unchanged (clear it with `--no-assume-unchanged`); an uppercase `S` outside a sparse cone is a hand-set skip-worktree bit (clear it with `--no-skip-worktree`). Leave a sparse cone's `S` entries alone. || `judgment/sparse-checkout` | Keep the cone's `skip-worktree` bits: clearing them turns absent files into apparent deletions. Confirm the cone with `git sparse-checkout list`, verify no file outside it is materialized, then remove manually once every other gate holds. || `judgment/precious-ignored-files` | Leave `preciousPaths` in place until the owner disposes of them or authorizes their deletion, then rerun. || `judgment/submodule-local-work` | The listed submodules hold work only this worktree has. Show the owner what each holds; rerun once it is pushed or they authorize discarding it. |low — "rerun once ... they authorize discarding it" sends the agent back into the same gate: authorization alone does not change what the walk sees
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BJGHNSVNR7A66RVMKR11BXof review01M3BJ3S1CY1F1S8BW4PD0P9FC@ -27,0 +26,5 @@echo " submodule selectors advanced outside third-party/ paths; removal of in-root linked"echo " worktrees that pass every removal gate. A checkout with a submodule, at any depth, on a"echo " commit neither recorded nor held by a ref gets no fast-forward or selector move; a local"echo " selector tag is replaced only when the fetched tag or another ref keeps its commit; a"echo " worktree whose submodules hold work no remote-tracking ref keeps is not removed."low — The new help text says a ref "keeps" a commit where every other file says "holds", and "work no remote-tracking ref keeps is" garden-paths the reader
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BJDYR88CZ3HMK9ZDZWETFHof review01M3BJ3S1CY1F1S8BW4PD0P9FC@ -478,0 +497,7 @@mode=$2prefix=${3:-}gitlink_paths=$({git -C "$superproject_path" ls-files --stage -zgit -C "$superproject_path" ls-tree -r -z --full-tree HEAD 2>/dev/null || true} | tr '\0' '\n' | awk -F '\t' '$1 ~ /^160000 / {print $2}' | sort -u) || return 1low — A git ls-files failure in list_submodules_with_local_work cannot be detected, so the submodule safety gate fails open instead of closed
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BJYBGZFFQGBD05BS5YHCYMof review01M3BJ3S1CY1F1S8BW4PD0P9FC@ -1415,2 +1541,4 @@submodule_metadata_ok=$(jq -r '.ok' "$submodule_metadata_path")if [ "$outside_root" = false ]; thenif stranded_output=$(list_submodules_with_local_work "$worktree_path" update 2>/dev/null); thenmedium — The update-mode submodule walk discards its own diagnostics, so a failed walk blocks every fast-forward with an unactionable "submodule check failed" row
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BJVR540DX0TTTF5985H5RHof review01M3BJ3S1CY1F1S8BW4PD0P9FC@ -1123,0 +1277,7 @@git(dependencyPath, "tag", "--no-sign", "--force", "v1");git(dependencyPath, "checkout", "--quiet", "--detach", thirdCommit);const refused = audit();assert.equal(refused.trackingUpdate.attempted, true);assert.equal(refused.trackingUpdate.ok, false);assert.match(refused.trackingUpdate.error, /local tag v1 .* no other ref holds/);low — The tag-selector test never exercises the "another ref holds it" half of update_submodule_tag, so that allowance can be deleted with the suite green
lens
test-trimming· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BKCTPMEA51K2Z53GH578NAof review01M3BJ3S1CY1F1S8BW4PD0P9FC@ -1123,0 +1405,8 @@assert.match(unreadable.removal.error, /cannot read submodule dependency/);execFileSync("git", ["config", "--file", join(dependencyGitDir, "config"), "--unset", "core.worktree"]);// Nor must a submodule whose status query fails, even with uncommitted work the superproject cannot see.writeFileSync(join(dependencyPath, "README.md"), "uncommitted\n");const dependencyIndex = join(dependencyGitDir, "index");const healthyIndex = readFileSync(dependencyIndex);writeFileSync(dependencyIndex, "not an index");medium — The new submodule removal-gate test stages uncommitted submodule work but only asserts the status-query-failure path, so the uncommitted-files and stash gates are unprotected
lens
test-trimming· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BK8QQK8CCEE9262AAYWE8Aof review01M3BJ3S1CY1F1S8BW4PD0P9FC@ -143,6 +146,7 @@ function branchLabel(worktree: Worktree): string {function stepFailure(worktree: Worktree, fetched: boolean): string | null {if (worktree.statusError !== null) return `status query failed: ${firstLine(worktree.statusError)}`;if (worktree.strandedSubmodules === null) return "submodule check failed; checkout not updated";medium — stepFailure returns "submodule check failed; checkout not updated" for every worktree, hiding a linked worktree's real removal outcome and dropping it from all other report sections
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BJWCTMM2QXQWEPKZA7TSHJof review01M3BJ3S1CY1F1S8BW4PD0P9FC@ -311,2 +319,8 @@const isMain = worktree.registration?.isMain === true;const onDefault = worktree.status?.branch.isDetached === false && worktree.status.branch.current === defaultBranch;const stranded = worktree.strandedSubmodules ?? [];if ((isMain || onDefault) && stranded.length > 0) {const why = `submodule on a commit no ref holds: ${listPaths(stranded)}; not updated`;decisions.push([path, branch, formatAheadBehind(worktree.defaultComparison), why]);continue;}medium — The stranded-submodule row overrides primaryDecision for every primary checkout, so detached and off-default primaries lose their real reason and are compared against the wrong line
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3BJZHXK440NSNFW2AJ720B6of review01M3BJ3S1CY1F1S8BW4PD0P9FCRound-7 outcomes for review
01M3BJ3S1CY1F1S8BW4PD0P9FC(head0cdf081). From round 6, only data loss, data corruption, or a security hole gets a new push. None of these findings is in that class, so all are acknowledged, and nothing is pushed for this round.Real report and usability bugs left unfixed, for a follow-up PR. None of them loses data. Each either keeps a worktree or mislabels a row.
strandedSubmodules: null) turns any worktree into a failure row, including a linked worktree, whose removal outcome then disappears. Inrender-audit-report.tsstepFailure, apply the null check only to primary and default-branch checkouts.primaryDecision, so a detached or off-default primary loses its own reason. InformatAuditReport, run the stranded check only on thecurrent-default,default-needs-attentionandmanual-reviewclassifications, or append its note to theprimaryDecisionreason.--forceneeds a.gitmodulesfile (summary only): a merged worktree with a populated Gitlink but no.gitmodulesis refused by Git on every run. Indecide_removal_outcome, choose--forcewhen the walk found any populated Gitlink, not when.gitmodulesexists.Checked for data loss and ruled out:
ls-filesfailure. An unreadable superproject index also fails the final superproject status check, so removal stops asoperational/gate-check-failed.Deferred wording and test items, fixes as in the round-4 to 6 replies: 88009, 88010, 88011, 88016, 88017, 88018, 88019, 88013, 88021, and "schema 5" in
--help. For 88013, the removal test should add a case with uncommitted files and one with a stash, each on a readable submodule. For 88021, a tag-selector case should cover a local tag whose commit another ref holds.