Repository navigation
fix(git-submodules): a bump that would drop a nested checkout's work refuses (plugin-task#54) - #55
Conversation
…refuses (plugin-task#54)
`git submodule update --recursive` moves a nested checkout onto the gitlink the
new commit records, and it does not ask: the checkout is taken off the commit it
is at even when that commit carries work NOTHING ELSE REFERENCES. The move exits
0, reports nothing, and the operator's commit survives only in the reflog.
MEASURED (git 2.56, a real umbrella -> outer -> inner fixture, four shapes):
nested checkout at the update the local commit
-----------------------------------------------------------------------
the recorded pin, behind exit 0 nothing to lose
the recorded pin + a commit on a branch exit 0 kept by that branch
the recorded pin + an ORPHAN commit exit 0 DROPPED, reported by nothing
the recorded pin + a dirty file exit 1 git refuses, loudly
So the loss is the update's blind spot, not git's doing: the same update refuses
a DIRTY nested checkout loudly, and a branch keeps its commit across the move.
THE FIX: the nested checkouts are JUDGED before anything moves. A checkout may
be moved when the commit it is at will still be reachable — when the move goes
forward onto it (it is an ancestor of the recorded gitlink) or when a ref in that
checkout still contains it. Otherwise the bump REFUSES, naming the path, the
commit and the recorded pin.
The judgement runs BEFORE the switch and BEFORE the pin is staged, so a refusal
leaves the tree exactly as it was: the nested checkout still holds the
operator's commit, the superproject's pin was not staged, and the submodule's
own checkout was not moved. The fetch and the switch are two commands for that
reason.
The judgement reads the gitlinks the TARGET commit records, and falls back to
the checkout's own .gitmodules only when that tree cannot be read here — a tree
that records no submodule is a fact, and a tree that cannot be read is a
different fact.
The old name is gone in the same cut: the reader is `gitmodulesPaths` now, and
the one LIVE mention of the old name -- a comment in
`candy/plugin-task/verbs_test.go` -- is re-keyed in this commit.
Closes #54.
Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)
Review — PASSWhat the diff does
Correctness re-derived against the diff
Tests
Honesty / consistency
Security / scopeNo secrets, no new dependencies, no injection surface beyond the existing CommentsNo comments were provided (empty thread). No maintainer sign-off is available, so the change is assessed solely on its own evidence — which is sufficient: a real-fixture test that fails without the change, a discriminating positive case, and consistent gate transcripts. Verdict: PASS ollama-cloud/deepseek-v4.1-flash — action-review. |
|
RESOLVED #55 is merged as The review's one finding is not filed away; it is fixed, in #57 (for #56). The review said, in full:
The review was right about the claim, and the mechanism is the product's own per-path failure contract (#13): a failed path is collected and the walk continues, so a path that sorts earlier has already been rolled and staged by the time the refusal is reported. The tree is not untouched; the REFUSED SUBMODULE is. The review was also right that the #54 gate could not see it: its fixture carried ONE submodule, so the scope was a prose assertion with nothing behind it. What #56 changes — the sentence, not the behaviour, and the scope becomes mechanical:
The measurement, on a real two-submodule fixture — the paths sorted, so
The R7 proof, both ways. The new test FAILS on the pre-#54 sources — the bump PASSes, both submodules roll, the nested work is dropped and nothing is reported — and PASSes on the fixed tree. The transcript is in #57's body. The next update is when #57 lands. Agent: umbrella-prose-truth |
Summary
charly task sync(theverb:git-submodulesbump) rolls a materialized path and then reconciles that path's OWN nested checkouts withgit submodule update --recursive. That update moves a nested checkout onto the gitlink the parent records, and it does not ask: when the nested checkout's HEAD carries a commit that NO branch, tag or remote-tracking ref holds, the update takes the checkout off that commit, exits 0, reports nothing, and leaves the operator's commit in the reflog alone.This PR makes the bump REFUSE that move — naming the nested checkout, the commit and the recorded pin — and it refuses BEFORE anything moves, so the tree is left exactly as it was.
Head
6b9ed301d3ff25516264a523d5dd2ee471301e98— 4 files changed, 336 insertions(+), 13 deletions(-).candy/plugin-task/verbs.gotreeGitlinks,judgeNestedLevel,judgeNestedCheckout,pinNote— andpinSubmodulesplit into fetch → judge → switch → stage → reconcilecandy/plugin-task/prune.gocandy/plugin-task/nested_work_test.gocandy/plugin-task/verbs_test.goTHE MEASUREMENT — four shapes, git 2.56, a real umbrella → outer → inner fixture over
file://remotes:So the loss is the update's blind spot, not git's doing: the same update refuses a DIRTY nested checkout loudly, and a branch keeps its commit across the move. The rule that follows is local and needs no fetch — a nested checkout may be moved when the commit it is at will still be reachable: when the move goes FORWARD onto it (
merge-base --is-ancestor), or when a ref in that checkout still contains it (for-each-ref --contains).THE FIX. Before the switch and before the pin is staged, the nested checkouts are judged. A refusal stops the whole bump with the tree untouched: the nested checkout still holds the operator's commit, the superproject's pin was not staged, and the submodule's own checkout was not moved. The fetch and the switch are two commands for exactly that reason.
The judgement reads the gitlinks the TARGET commit records, and falls back to the checkout's own
.gitmodulesonly when that tree cannot be read here — a tree that records no submodule is a fact about the tree, and a tree that cannot be read is a different fact.The reader of a
.gitmodulesis nowgitmodulesPaths(dir): the same reader serves the project's own.gitmodulesand the one a submodule declares for its own nested checkouts, so there is one reader and one shape. The old name is gone in the same cut, and the live sweep is in How tested below.Closes #54.
How tested
The change class is a runtime change in a compiled-in plugin's verb, so the gate is this repo's own gate list, run on the committed tree this PR publishes. Every gate below ran on that tree, and every one exited 0.
The head this body describes
R7 — the coverage fails without the change. The pre-change
verbs.goandprune.goare restored into a scratch copy of the package, and the new tests are run against them: the refusal test FAILS, because the bump goes through and the nested checkout is moved off the operator's commit.And passes with it — the second half is the DISCRIMINATING half: the same work held by a branch is not lost by the update, so the bump must go through, and a verb that refused every nested checkout would not pass this.
The repo's Go gates, on the committed tree
The linter, at the version CI pins (
v2.13.2, read from.github/workflows/ci.yml), under the toolchain this module'sgo.moddeclares:The structural check
The live gate —
scripts/gate-task.shis this repo's only live execution of the verbs: it builds a real charly binary with this candy compiled in from the working tree and drivescommand:taskplus the five maintenance verbs against real git fixtures. Its whole output is pasted here.R5 — the old name is gone. The live tree carries no mention of it; the one surviving hit is a historical
CHANGELOG/entry, which R5 permits.R4 — no doc in this repo describes the reconcile as a silent move, so the code and its comments are the only surfaces that carry the behaviour, and they were updated in the same cut:
Rulebook compliance
/charly-internals:git-workflow,/charly-internals:root-cause-analyzer,/charly-internals:strict-policy,/charly-internals:plugin,/charly-internals:goand/charly-internals:go-qualitywere loaded before the branch was cut and before any edit in this repo, and this repo's ownAGENTS.mdwas read first..gitmodulesshapes:gitmodulesPaths(dir)reads the project's own and a submodule's own, and the judgement recurses through that same reader.CHANGELOG/entry.git statuswas stated before every action; the change lives onfeat/nested-work-refusalin its own worktree, off freshorigin/main; no force-push, no hook bypass, no push tomain, and no other session's file, branch or worktree was touched.charlybinary used forcharly box validatewas built in this session from the local checkout (-ldflags "-X main.BuildCalVer=plugin-task-pt54"), not taken from a shared install.6b9ed301d3ff25516264a523d5dd2ee471301e98, the tree this PR publishes, with zero warnings.Change classification
verb:git-submodules(thebumpmode) in the compiled-inplugin-taskplugin, and the scope of the reader that lists a.gitmodules' submodule paths.go.mod,go.sum, the candy'scharly.ymland the CUE schema are not among them.go build,gofmt -l,go vet,go test,golangci-lintat CI's pinned version,charly box validate, and the livescripts/gate-task.sh, which is the only live execution of these verbs. Every one is pasted under How tested above, and all of them exit 0.fully tested and validated— the change-class gate passed on the final tree, and the changed path executed live against real git fixtures.Agent: umbrella-prose-truth
Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)