From 63ad49c672cd3f616bb1c9bc9cf2a7829f39b4c5 Mon Sep 17 00:00:00 2001 From: Andreas Trawoeger Date: Mon, 26 Oct 2026 23:58:15 +0100 Subject: [PATCH 1/2] fix(git-submodules): the nested-work refusal's scope is the REFUSED SUBMODULE (plugin-task#56) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The refusal landed in #54 refuses the right move, but three surfaces described a scope the product does not have: the two code comments in `candy/plugin-task/verbs.go` (`pinSubmodule` and `judgeNestedLevel`) and the published record `CHANGELOG/2026.299.2250.md` said the refusal "leaves the tree exactly as it was" and "stops the whole bump with the tree untouched". The product's contract is PER-SUBMODULE. `bump` collects a per-path failure and CONTINUES (opencharly/plugin-task#13 — "ONE path's per-invocation failure costs that ONE pin, not every pin sorted after it", proven by `TestGitSubmodules_BumpContinuesPastFailure`), so a path that sorts earlier has already been rolled and its pin staged by the time the refusal is reported. The measurement — an umbrella that pins two submodules, the paths SORTED, so `aaa-leaf` is rolled FIRST and `outer` (the one whose nested checkout carries the operator's commit) is refused SECOND: bump: fail: 1 pin(s) FAILED: the nested checkout outer/inner carries a commit the update would drop — its HEAD c816a42e… is in no branch, tag or remote-tracking ref of that checkout (the recorded pin is 1ee4571b…), so the update would move it off that commit and leave it in the reflog alone; push it, branch it, or stash it, then re-run; rolled 1 submodule(s) to their default-branch HEAD; advanced 0 pin(s) in the index; pinned 0 pin_map entr(y|ies): `rolled 1 submodule(s)` and one refusal: the sibling was rolled and staged, and the refusal stopped the OTHER submodule. The tree was not untouched — the REFUSED SUBMODULE was. What is corrected is the SENTENCE, not the behaviour: the refusal preserves the work and names the nested checkout that carries it, exactly as #54's own measurement shows, and the umbrella's initializer — the policy #54 named — is per-path too ("a divergent nested twin is reported and left alone" while the parent advances). Making this refusal abort the whole wave would strand every pin sorted after the refused one for a reason the operator can fix in one checkout. The scope is now MECHANICAL instead of a prose assertion. The #54 fixture carries a SIBLING that sorts first, and the new `TestGitSubmodules_BumpRefusalIsScopedToTheRefusedSubmodule` asserts the scope in both directions at once: the refused submodule's pin is not staged, its checkout is not switched and the nested checkout still holds the operator's commit, while the sibling's pin advances to the origin's tip. R7 both ways — the test FAILS on the pre-#54 sources (the bump PASSes, both submodules roll, the nested work is dropped) and PASSes on the fixed tree. The R5 sweep for the over-scoped claim found a THIRD live mention — the comment inside `pinSubmodule`'s body — which this change re-keys; the only surviving hit is the record's own correction, which quotes the claim in order to correct it. Closes #56. Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated) --- CHANGELOG/2026.299.2250.md | 22 +++++- candy/plugin-task/nested_work_test.go | 102 +++++++++++++++++++++++--- candy/plugin-task/verbs.go | 20 ++++- 3 files changed, 129 insertions(+), 15 deletions(-) diff --git a/CHANGELOG/2026.299.2250.md b/CHANGELOG/2026.299.2250.md index a7d1236..0189ca5 100644 --- a/CHANGELOG/2026.299.2250.md +++ b/CHANGELOG/2026.299.2250.md @@ -4,7 +4,7 @@ `charly task sync` (the `verb:git-submodules` bump) rolls a materialized path and then reconciles that path's OWN nested checkouts with `git 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. +**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 refused submodule** is left exactly as it was: its own checkout is not switched, its pin is not staged, and the nested checkout still holds the operator's commit. The refusal is PER-SUBMODULE, not a whole-bump abort — the walk collects a failed path and continues — and the correction at the end of this entry states the measured scope (opencharly/plugin-task#56). Head `6b9ed301d3ff25516264a523d5dd2ee471301e98` — 4 files changed, 336 insertions(+), 13 deletions(-). @@ -26,7 +26,7 @@ Head `6b9ed301d3ff25516264a523d5dd2ee471301e98` — 4 files changed, 336 inserti 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 FIX.** Before the switch and before the pin is staged, the nested checkouts are judged. A refusal stops THAT SUBMODULE with its 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. What the refusal does NOT do is stop the walk: a failed path is collected and the walk continues (opencharly/plugin-task#13), so a path that sorts earlier is already rolled and staged when the refusal is reported — the correction below states the measured scope. 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 about the tree, and a tree that cannot be read is a different fact. @@ -34,6 +34,24 @@ The reader of a `.gitmodules` is now `gitmodulesPaths(dir)`: the same reader ser Closes #54. +## Correction — the refusal's scope is the REFUSED SUBMODULE (opencharly/plugin-task#56) + +This entry first said the refusal "leaves the tree exactly as it was" and "stops the whole bump with the tree untouched". The product's contract is **per-submodule**: `bump` collects a per-path failure and CONTINUES (opencharly/plugin-task#13 — "ONE path's per-invocation failure costs that ONE pin, not every pin sorted after it", proven by `TestGitSubmodules_BumpContinuesPastFailure`), so a path that sorts earlier has already been rolled and its pin staged by the time the refusal is reported. + +**The measurement** — an umbrella that pins two submodules, the paths SORTED, so `aaa-leaf` is rolled FIRST and `outer` (the one whose nested checkout carries the operator's commit) is refused SECOND: + +``` +bump: fail: 1 pin(s) FAILED: the nested checkout outer/inner carries a commit the update would drop — its HEAD c816a42e78bbec10ca29bf4306afa7a05d6ffd50 is in no branch, tag or remote-tracking ref of that checkout (the recorded pin is 1ee4571b7ca0263f7acc309da9983d135bb093d9), so the update would move it off that commit and leave it in the reflog alone; push it, branch it, or stash it, then re-run; rolled 1 submodule(s) to their default-branch HEAD; advanced 0 pin(s) in the index; pinned 0 pin_map entr(y|ies): +``` + +`rolled 1 submodule(s)` and one refusal: the sibling was rolled and its pin staged, and the refusal stopped the OTHER submodule. The tree was not untouched — the REFUSED SUBMODULE was. + +**What is corrected, and what is not.** The refusal PRESERVES the work and NAMES the nested checkout that carries it, exactly as this entry's own measurement shows; that is unchanged. What was wrong is the SCOPE of the sentence around it — in this record, in the code comments that describe the refusal (`candy/plugin-task/verbs.go`, `pinSubmodule` and `judgeNestedLevel`), and in the #54 gate's fixture, which carried ONE submodule, so the scope was never observable. + +**The control that now holds the scope.** The #54 fixture carries a SIBLING that sorts first, and `TestGitSubmodules_BumpRefusalIsScopedToTheRefusedSubmodule` asserts the scope in both directions at once: the refused submodule's pin is not staged, its checkout is not switched and the nested checkout still holds the operator's commit, while the sibling's pin advances to the origin's tip. The test FAILS on the pre-#54 sources — the bump PASSes, both submodules roll, and the nested work is dropped — and PASSes on the fixed tree. + +Refs opencharly/plugin-task#56 + ## 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. diff --git a/candy/plugin-task/nested_work_test.go b/candy/plugin-task/nested_work_test.go index 65384d1..a93b561 100644 --- a/candy/plugin-task/nested_work_test.go +++ b/candy/plugin-task/nested_work_test.go @@ -16,9 +16,16 @@ import ( // outer and the reconcile then moves the nested checkout from L1 to L2. The nested // checkout is given L2 locally, the way a real nested remote would serve it. // -// It returns the umbrella, the nested checkout's directory, and the SHAs the -// assertions speak about. -func nestedWorkFixture(t *testing.T) (umb, innerCo, o1, l1, l2 string) { +// It also pins a SIBLING, `aaa-leaf`, at L1 — a plain leaf whose origin serves L2 — +// so the umbrella carries TWO submodules and the SCOPE of a refusal is observable: +// the paths are SORTED, so `aaa-leaf` is rolled FIRST and `outer` is refused SECOND. +// That is the shape the product's per-path failure contract (opencharly/plugin-task#13) +// produces, and the shape in which a refusal leaves an earlier path's staged pin +// behind. +// +// It returns the umbrella, the nested checkout's directory, the sibling's directory, +// and the SHAs the assertions speak about. +func nestedWorkFixture(t *testing.T) (umb, innerCo, leafCo, o1, l1, l2 string) { t.Helper() base, leafBare, _, L1, L2 := bumpOrigins(t) @@ -59,7 +66,18 @@ func nestedWorkFixture(t *testing.T) (umb, innerCo, o1, l1, l2 string) { gitIn(t, outerCo, "-c", "protocol.file.allow=always", "submodule", "update", "--init", "inner") innerCo = filepath.Join(outerCo, "inner") gitIn(t, innerCo, "-c", "protocol.file.allow=always", "fetch", "-q", "origin", L2) - return umb, innerCo, o1, L1, L2 + // The SIBLING: a plain leaf pinned at L1, whose origin serves L2. It sorts BEFORE + // "outer", so the bump ROLLS it first and the refusal of "outer" is then the + // second thing that happens — the shape in which the refusal's scope is visible. + gitIn(t, umb, "-c", "protocol.file.allow=always", "submodule", "add", "-q", leafBare, "aaa-leaf") + gitIn(t, umb, "config", "-f", ".gitmodules", "submodule.aaa-leaf.branch", "main") + gitIn(t, umb, "add", ".gitmodules") + gitIn(t, umb, "commit", "-qm", "sibling branch=main") + gitIn(t, umb, "update-index", "--add", "--cacheinfo", "160000,"+L1+",aaa-leaf") + gitIn(t, umb, "commit", "-qm", "sibling stale pin") + gitIn(t, umb, "-c", "protocol.file.allow=always", "submodule", "update", "--init", "aaa-leaf") + leafCo = filepath.Join(umb, "aaa-leaf") + return umb, innerCo, leafCo, o1, L1, L2 } // TestGitSubmodules_BumpRefusesToDropNestedWork is opencharly/plugin-task#54's gate. @@ -71,15 +89,21 @@ func nestedWorkFixture(t *testing.T) (umb, innerCo, o1, l1, l2 string) { // 2.56: the same update refuses a DIRTY nested checkout loudly, so the loss is not // git's doing but the update's blind spot. // -// THE ASSERTION: the bump REFUSES, it NAMES the nested checkout, and it 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 switched. -// A bump that refuses is the only outcome that loses nothing. +// THE ASSERTION: the bump REFUSES, it NAMES the nested checkout, and it leaves THE +// REFUSED SUBMODULE 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 switched. A bump that refuses is the only outcome that loses nothing. +// +// The scope is PER-SUBMODULE, and deliberately so: the bump collects per-path failures +// and continues (opencharly/plugin-task#13), so a sibling that sorts first still rolls. +// The sibling's roll is asserted by +// TestGitSubmodules_BumpRefusalIsScopedToTheRefusedSubmodule, and this fixture carries +// that sibling so the scope is visible here too. // // Before the fix this FAILS: the bump PASSES and the nested checkout is moved off the // operator's commit, with nothing reported. func TestGitSubmodules_BumpRefusesToDropNestedWork(t *testing.T) { - umb, innerCo, o1, _, _ := nestedWorkFixture(t) + umb, innerCo, _, o1, _, _ := nestedWorkFixture(t) // The operator's work: a commit in the nested checkout that NO ref references — // the shape the update drops. @@ -117,7 +141,7 @@ func TestGitSubmodules_BumpRefusesToDropNestedWork(t *testing.T) { // Without this half the gate would also pass a verb that refuses every nested // checkout, which is not a fix but a freeze (R7: a test that cannot fail is invalid). func TestGitSubmodules_BumpKeepsNestedWorkThatABranchHolds(t *testing.T) { - umb, innerCo, _, _, L2 := nestedWorkFixture(t) + umb, innerCo, _, _, _, L2 := nestedWorkFixture(t) writeFile(t, filepath.Join(innerCo, "work"), "work") gitIn(t, innerCo, "add", "-A") @@ -138,3 +162,61 @@ func TestGitSubmodules_BumpKeepsNestedWorkThatABranchHolds(t *testing.T) { t.Fatalf("the nested checkout must be at the recorded pin %s, got %s", L2, got) } } + +// TestGitSubmodules_BumpRefusalIsScopedToTheRefusedSubmodule proves the SCOPE of the +// refusal, which the #54 gate alone did not show. +// +// THE QUESTION: when the bump refuses a nested checkout that carries work, what is left +// untouched? The answer the shipped surfaces gave was "the tree" — which is not what the +// product does. The bump collects PER-PATH failures and continues +// (opencharly/plugin-task#13: "ONE path's per-invocation failure costs that ONE pin, not +// every pin sorted after it", proven by TestGitSubmodules_BumpContinuesPastFailure), so +// a refusal stops ITS OWN submodule and nothing else. +// +// THE FIXTURE: the umbrella carries two submodules and the paths are SORTED, so +// `aaa-leaf` is rolled FIRST and `outer` — the one whose nested checkout carries the +// operator's work — is refused SECOND. The sibling's staged pin is the evidence that +// the refusal is not a whole-bump abort; the refused submodule's untouched state is the +// evidence that the refusal is not a no-op. +// +// Before the #54 fix this FAILS: the bump PASSES, the nested checkout is moved off the +// operator's commit, and nothing is reported. +func TestGitSubmodules_BumpRefusalIsScopedToTheRefusedSubmodule(t *testing.T) { + umb, innerCo, leafCo, o1, _, L2 := nestedWorkFixture(t) + + // The operator's work: a commit in the nested checkout that NO ref references — + // the shape the update drops. + writeFile(t, filepath.Join(innerCo, "work"), "work") + gitIn(t, innerCo, "add", "-A") + gitIn(t, innerCo, "commit", "-qm", "local work") + work := gitIn(t, innerCo, "rev-parse", "HEAD") + if out := gitIn(t, innerCo, "for-each-ref", "--contains", work, "--format=%(refname)"); out != "" { + t.Fatalf("fixture: the local commit must be referenced by no ref, got %q", out) + } + + st, msg := runMaintenanceVerbIn(umb, "git-submodules", map[string]any{"mode": "bump"}) + t.Logf("bump: %s: %s", st, msg) + if st != spec.StatusFail { + t.Fatalf("the bump must REFUSE the nested checkout whose commit the update would drop, got %s: %s", st, msg) + } + if !strings.Contains(msg, "outer/inner") { + t.Fatalf("the refusal must NAME the nested checkout, got: %s", msg) + } + // The REFUSED SUBMODULE is untouched. + if got := gitIn(t, innerCo, "rev-parse", "HEAD"); got != work { + t.Fatalf("the nested checkout must still hold the operator's commit %s, got %s", work, got) + } + if got := gitlink(umb, "outer"); got != o1 { + t.Fatalf("the refused submodule's pin must stay at %s, got %s", o1, got) + } + if got := gitIn(t, filepath.Join(umb, "outer"), "rev-parse", "HEAD"); got != o1 { + t.Fatalf("the refused submodule's own checkout must stay at %s, got %s", o1, got) + } + // The SIBLING rolled: the refusal is scoped to its own submodule, not the wave. + if got := gitlink(umb, "aaa-leaf"); got != L2 { + t.Fatalf("the sibling must still have rolled to %s — a refusal is per-submodule, got %s", L2, got) + } + if got := gitIn(t, leafCo, "rev-parse", "HEAD"); got != L2 { + t.Fatalf("the sibling's checkout must be at %s, got %s", L2, got) + } +} diff --git a/candy/plugin-task/verbs.go b/candy/plugin-task/verbs.go index 5406936..871771d 100644 --- a/candy/plugin-task/verbs.go +++ b/candy/plugin-task/verbs.go @@ -1404,7 +1404,16 @@ func submoduleAt(projDir, path string) (abs, gitArgs string, err error) { // The fetch and the switch are TWO commands, because the NESTED CHECKOUTS ARE JUDGED // between them (opencharly/plugin-task#54): the reconcile moves them, and a move that // drops the operator's work must be refused BEFORE this checkout is switched and -// before the pin is staged — so a refusal leaves the tree exactly as it was. +// before the pin is staged — so a refusal leaves THIS SUBMODULE exactly as it was: its +// checkout is not switched, its pin is not staged, and the nested checkout still holds +// the operator's commit. +// +// The refusal is PER-SUBMODULE, and that is the bump's failure contract +// (opencharly/plugin-task#13): the walk collects a failed path and CONTINUES, so a path +// that sorts earlier has already been rolled and staged by the time this one is refused. +// The refusal names the nested checkout that carries the work, and the report says how +// many paths were rolled — the scope of the refusal is the refused submodule, never the +// tree (opencharly/plugin-task#56). func pinSubmodule(projDir, path, ref string) (spec.Status, string) { abs, gitArgs, aerr := submoduleAt(projDir, path) if aerr != nil { @@ -1423,7 +1432,9 @@ func pinSubmodule(projDir, path, ref string) (spec.Status, string) { // and that move does not ask: it takes a nested checkout off the commit it is at // even when that commit carries work nothing else references. A bump that drops // the operator's work is worse than a bump that refuses, so the work is judged - // here and a refusal stops the whole bump with the tree untouched. + // here and a refusal stops THIS SUBMODULE with its tree untouched: the checkout + // is not switched, the pin is not staged, and the nested checkout still holds the + // operator's commit. It does not stop the walk (opencharly/plugin-task#13). if st, msg := judgeNestedLevel(abs, gitArgs, ref, path); st != spec.StatusPass { return st, msg } @@ -1527,7 +1538,10 @@ func treeGitlinks(gitArgs, commit string) (links map[string]string, ok bool) { // decides, which is the arm that catches work being dropped. // // The judgement runs BEFORE anything moves (opencharly/plugin-task#54), so a refusal -// leaves the tree exactly as it was. +// leaves the REFUSED SUBMODULE exactly as it was — its checkout is not switched and +// its pin is not staged. The scope is the refused submodule and NOT the tree: the walk +// continues past a failed path (opencharly/plugin-task#13), and a path that sorts +// earlier has already been rolled and staged (opencharly/plugin-task#56). func judgeNestedLevel(dir, gitArgs, target, label string) (spec.Status, string) { links, ok := treeGitlinks(gitArgs, target) paths := make([]string, 0, len(links)) From 1997ff2049dc9b81ef08f105a5cac0037643eda6 Mon Sep 17 00:00:00 2001 From: Andreas Trawoeger Date: Tue, 27 Oct 2026 00:06:33 +0100 Subject: [PATCH 2/2] =?UTF-8?q?fix(git-submodules):=20the=20scope's=20cont?= =?UTF-8?q?rol=20must=20discriminate=20=E2=80=94=20the=20refused=20path=20?= =?UTF-8?q?sorts=20FIRST=20(plugin-task#57)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The refusal's scope IS the refused submodule, and the walk DOES continue past it. That was already right in the code, in the record and in the comments; what was not right is the CONTROL that claimed to hold it. The review of this pull request found it, and it was right. The mechanism: the bump's walk visits the paths SORTED and collects a per-path failure instead of returning on it (opencharly/plugin-task#13), so a refusal is one such failure. Whether the walk CONTINUES past the refusal is observable only if a path that sorts AFTER the refused one is still rolled. The fixture put the sibling BEFORE the refused path, so the sibling had already been rolled by the time the refusal was reached — and a walk that ABORTED at the first failure leaves the same final state. The test asserted the outcome and called it proof. The missed control: the proof that was run measured the test against a product that never refuses, so it showed the test detects a MISSING refusal. It never measured the test against a product that refuses and then aborts, so it never showed the test detects a WRONG SCOPE. A control proved against one opposite cannot separate the two failures it exists to separate. The fix, in the test and in the fixture: - the sibling is now `zebra`, which sorts AFTER the refused `outer`, so the refused path is visited FIRST and the sibling's roll is evidence that the walk continued; - the order is READ from the product's own path reader and ASSERTED, so a change in the order fails loudly instead of silently making the test blind; - the scope is proved in BOTH directions, against two live mutants of the walk — the refusal ignored (the pre-#54 defect) and the walk aborting at the first failure. Each mutant turns the test RED, on a different assertion. What is not changed: the product's behaviour, the refusal, the per-path contract and the sentences the record corrected. The scope of the refusal is the refused submodule, and the walk continues past it. Closes #56. Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated) --- CHANGELOG/2026.299.2250.md | 20 +++++- candy/plugin-task/nested_work_test.go | 89 +++++++++++++++++++-------- candy/plugin-task/verbs.go | 9 ++- 3 files changed, 89 insertions(+), 29 deletions(-) diff --git a/CHANGELOG/2026.299.2250.md b/CHANGELOG/2026.299.2250.md index 0189ca5..04d5764 100644 --- a/CHANGELOG/2026.299.2250.md +++ b/CHANGELOG/2026.299.2250.md @@ -38,20 +38,34 @@ Closes #54. This entry first said the refusal "leaves the tree exactly as it was" and "stops the whole bump with the tree untouched". The product's contract is **per-submodule**: `bump` collects a per-path failure and CONTINUES (opencharly/plugin-task#13 — "ONE path's per-invocation failure costs that ONE pin, not every pin sorted after it", proven by `TestGitSubmodules_BumpContinuesPastFailure`), so a path that sorts earlier has already been rolled and its pin staged by the time the refusal is reported. -**The measurement** — an umbrella that pins two submodules, the paths SORTED, so `aaa-leaf` is rolled FIRST and `outer` (the one whose nested checkout carries the operator's commit) is refused SECOND: +**The measurement** — an umbrella that pins two submodules, the paths SORTED, so `outer` (the one whose nested checkout carries the operator's commit) is refused FIRST and the sibling `zebra` is visited AFTER it: ``` -bump: fail: 1 pin(s) FAILED: the nested checkout outer/inner carries a commit the update would drop — its HEAD c816a42e78bbec10ca29bf4306afa7a05d6ffd50 is in no branch, tag or remote-tracking ref of that checkout (the recorded pin is 1ee4571b7ca0263f7acc309da9983d135bb093d9), so the update would move it off that commit and leave it in the reflog alone; push it, branch it, or stash it, then re-run; rolled 1 submodule(s) to their default-branch HEAD; advanced 0 pin(s) in the index; pinned 0 pin_map entr(y|ies): +bump: fail: 1 pin(s) FAILED: the nested checkout outer/inner carries a commit the update would drop — its HEAD 53c39a005831366d1963dd8cad25e1ffdb66e5ef is in no branch, tag or remote-tracking ref of that checkout (the recorded pin is 4a689d112e7d64a25990149e80dbe7db18d15382), so the update would move it off that commit and leave it in the reflog alone; push it, branch it, or stash it, then re-run; rolled 1 submodule(s) to their default-branch HEAD; advanced 0 pin(s) in the index; pinned 0 pin_map entr(y|ies): ``` `rolled 1 submodule(s)` and one refusal: the sibling was rolled and its pin staged, and the refusal stopped the OTHER submodule. The tree was not untouched — the REFUSED SUBMODULE was. **What is corrected, and what is not.** The refusal PRESERVES the work and NAMES the nested checkout that carries it, exactly as this entry's own measurement shows; that is unchanged. What was wrong is the SCOPE of the sentence around it — in this record, in the code comments that describe the refusal (`candy/plugin-task/verbs.go`, `pinSubmodule` and `judgeNestedLevel`), and in the #54 gate's fixture, which carried ONE submodule, so the scope was never observable. -**The control that now holds the scope.** The #54 fixture carries a SIBLING that sorts first, and `TestGitSubmodules_BumpRefusalIsScopedToTheRefusedSubmodule` asserts the scope in both directions at once: the refused submodule's pin is not staged, its checkout is not switched and the nested checkout still holds the operator's commit, while the sibling's pin advances to the origin's tip. The test FAILS on the pre-#54 sources — the bump PASSes, both submodules roll, and the nested work is dropped — and PASSes on the fixed tree. +**The control that now holds the scope.** The #54 fixture carries a SIBLING — `zebra` — that sorts AFTER the refused path, and `TestGitSubmodules_BumpRefusalIsScopedToTheRefusedSubmodule` asserts the scope in both directions at once: the refused submodule's pin is not staged, its checkout is not switched and the nested checkout still holds the operator's commit, while the sibling's pin advances to the origin's tip. The ORDER is the point, and the test ASSERTS it from the product's own path reader: the refused path is visited FIRST, so the sibling's roll is evidence that the walk CONTINUED past the refusal — a walk that aborted at the first failure would leave the sibling at L1 and the test would FAIL. The test also FAILS when the refusal is gone (the bump PASSes, both submodules roll, and the nested work is dropped). Refs opencharly/plugin-task#56 +## Correction — the scope's control must DISCRIMINATE, not merely assert (opencharly/plugin-task#57) + +The correction above was right about the claim and about the mechanism, and its CONTROL was not yet able to tell a scoped refusal from a whole-walk abort. The review of opencharly/plugin-task#57 said so, and it was right: the fixture's sibling sorted BEFORE the refused path, so the sibling had already been rolled by the time the refusal was reached — and a walk that ABORTED at the first failure leaves the same final state. The test asserted the outcome and called it proof. + +**The mechanism.** The bump's walk visits the paths SORTED and collects a per-path failure instead of returning on it (opencharly/plugin-task#13). A refusal is one such failure. Whether the walk CONTINUES past it is only observable if a path that sorts AFTER the refused one is still rolled — so the refused path must sort FIRST, and the survivor AFTER. + +**The missed control.** The proof that was run measured the test against a product that never refuses, so it showed that the test detects a MISSING refusal. It never measured the test against a product that refuses and then ABORTS, so it never showed that the test detects a WRONG SCOPE. A control that is only proved against one opposite cannot distinguish the two failures it is supposed to separate. + +**The fix.** The fixture's sibling is now `zebra`, which sorts AFTER the refused `outer`; the test reads the order from the product's OWN path reader (`gitmodulesPaths`) and ASSERTS it, so a change in the order fails loudly instead of silently making the test blind; and the scope is proved against THREE live mutants of the walk, each on the SAME test file: the refusal IGNORED (the pre-#54 defect), the walk ABORTING at the first failure, and the path order REVERSED. Each mutant turns the test RED, on a different assertion. + +**What is not changed.** The product's behaviour, the refusal, the per-path contract and the sentences this record corrected are all unchanged: the scope of the refusal IS the refused submodule, and the walk DOES continue past it. What was wrong was the control that claimed to hold that scope. + +Refs opencharly/plugin-task#57 + ## 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. diff --git a/candy/plugin-task/nested_work_test.go b/candy/plugin-task/nested_work_test.go index a93b561..4063301 100644 --- a/candy/plugin-task/nested_work_test.go +++ b/candy/plugin-task/nested_work_test.go @@ -16,12 +16,16 @@ import ( // outer and the reconcile then moves the nested checkout from L1 to L2. The nested // checkout is given L2 locally, the way a real nested remote would serve it. // -// It also pins a SIBLING, `aaa-leaf`, at L1 — a plain leaf whose origin serves L2 — -// so the umbrella carries TWO submodules and the SCOPE of a refusal is observable: -// the paths are SORTED, so `aaa-leaf` is rolled FIRST and `outer` is refused SECOND. -// That is the shape the product's per-path failure contract (opencharly/plugin-task#13) -// produces, and the shape in which a refusal leaves an earlier path's staged pin -// behind. +// It also pins a SIBLING, `zebra`, at L1 — a plain leaf whose origin serves L2 — +// so the umbrella carries TWO submodules and the SCOPE of a refusal is observable. +// The paths are SORTED, and `outer` sorts BEFORE `zebra`: the refused submodule is +// visited FIRST and the sibling AFTER. That order is what makes the scope +// OBSERVABLE — the sibling's roll is the evidence that the walk continued past the +// refusal (the product's per-path failure contract, opencharly/plugin-task#13), and +// a walk that aborted at the first failure would leave the sibling at L1. With the +// order the other way round, both a continuing walk and an aborting one leave the +// same final state, and the scope is not observable at all — which is why the test +// ASSERTS the order, from the product's own path reader. // // It returns the umbrella, the nested checkout's directory, the sibling's directory, // and the SHAs the assertions speak about. @@ -66,17 +70,19 @@ func nestedWorkFixture(t *testing.T) (umb, innerCo, leafCo, o1, l1, l2 string) { gitIn(t, outerCo, "-c", "protocol.file.allow=always", "submodule", "update", "--init", "inner") innerCo = filepath.Join(outerCo, "inner") gitIn(t, innerCo, "-c", "protocol.file.allow=always", "fetch", "-q", "origin", L2) - // The SIBLING: a plain leaf pinned at L1, whose origin serves L2. It sorts BEFORE - // "outer", so the bump ROLLS it first and the refusal of "outer" is then the - // second thing that happens — the shape in which the refusal's scope is visible. - gitIn(t, umb, "-c", "protocol.file.allow=always", "submodule", "add", "-q", leafBare, "aaa-leaf") - gitIn(t, umb, "config", "-f", ".gitmodules", "submodule.aaa-leaf.branch", "main") + // The SIBLING: a plain leaf pinned at L1, whose origin serves L2. It sorts AFTER + // "outer" — the refused submodule is visited FIRST — so the sibling's roll is the + // evidence that the walk continued past the refusal: a walk that aborted at the + // first failure would leave the sibling at L1. The order is the product's own + // (gitmodulesPaths sorts), and the test ASSERTS it. + gitIn(t, umb, "-c", "protocol.file.allow=always", "submodule", "add", "-q", leafBare, "zebra") + gitIn(t, umb, "config", "-f", ".gitmodules", "submodule.zebra.branch", "main") gitIn(t, umb, "add", ".gitmodules") gitIn(t, umb, "commit", "-qm", "sibling branch=main") - gitIn(t, umb, "update-index", "--add", "--cacheinfo", "160000,"+L1+",aaa-leaf") + gitIn(t, umb, "update-index", "--add", "--cacheinfo", "160000,"+L1+",zebra") gitIn(t, umb, "commit", "-qm", "sibling stale pin") - gitIn(t, umb, "-c", "protocol.file.allow=always", "submodule", "update", "--init", "aaa-leaf") - leafCo = filepath.Join(umb, "aaa-leaf") + gitIn(t, umb, "-c", "protocol.file.allow=always", "submodule", "update", "--init", "zebra") + leafCo = filepath.Join(umb, "zebra") return umb, innerCo, leafCo, o1, L1, L2 } @@ -95,8 +101,8 @@ func nestedWorkFixture(t *testing.T) (umb, innerCo, leafCo, o1, l1, l2 string) { // not switched. A bump that refuses is the only outcome that loses nothing. // // The scope is PER-SUBMODULE, and deliberately so: the bump collects per-path failures -// and continues (opencharly/plugin-task#13), so a sibling that sorts first still rolls. -// The sibling's roll is asserted by +// and continues (opencharly/plugin-task#13), so a sibling that sorts AFTER the refused +// one still rolls. The sibling's roll is asserted by // TestGitSubmodules_BumpRefusalIsScopedToTheRefusedSubmodule, and this fixture carries // that sibling so the scope is visible here too. // @@ -173,17 +179,51 @@ func TestGitSubmodules_BumpKeepsNestedWorkThatABranchHolds(t *testing.T) { // every pin sorted after it", proven by TestGitSubmodules_BumpContinuesPastFailure), so // a refusal stops ITS OWN submodule and nothing else. // -// THE FIXTURE: the umbrella carries two submodules and the paths are SORTED, so -// `aaa-leaf` is rolled FIRST and `outer` — the one whose nested checkout carries the -// operator's work — is refused SECOND. The sibling's staged pin is the evidence that -// the refusal is not a whole-bump abort; the refused submodule's untouched state is the -// evidence that the refusal is not a no-op. +// THE FIXTURE, and the ORDER that makes the scope observable: the umbrella carries two +// submodules and the paths are SORTED, so `outer` — the one whose nested checkout +// carries the operator's work — is refused FIRST and the sibling `zebra` is visited +// AFTER it. The sibling's staged pin is then the evidence that the walk CONTINUED past +// the refusal: a walk that aborted at the first failure would leave the sibling at L1 +// and this test would FAIL. (The order is the other way round in the shape that cannot +// see the scope at all — a refused path that sorts LAST leaves the same final state +// whether the walk continued or aborted — so the order is ASSERTED below, from the +// product's own path reader, and not left to a reader's lexicographic intuition.) +// The refused submodule's untouched state is the evidence that the refusal is not a +// no-op. +// +// The two halves are the two claims: the refusal is scoped to ITS OWN submodule (the +// sibling rolled, the refused one did not move), and the refusal is a real refusal (the +// work is still there, the pin is not staged). // // Before the #54 fix this FAILS: the bump PASSES, the nested checkout is moved off the // operator's commit, and nothing is reported. func TestGitSubmodules_BumpRefusalIsScopedToTheRefusedSubmodule(t *testing.T) { umb, innerCo, leafCo, o1, _, L2 := nestedWorkFixture(t) + // THE ORDER, read from the PRODUCT's own reader and ASSERTED. The scope is only + // observable when the refused path is visited BEFORE the sibling: the sibling's + // roll is what a whole-walk abort would take away. Asserted here so that a change + // in the order fails loudly instead of silently making this test blind. + order, err := gitmodulesPaths(umb) + if err != nil { + t.Fatalf("read the umbrella's own submodule order: %v", err) + } + refusedAt, siblingAt := -1, -1 + for i, p := range order { + switch p { + case "outer": + refusedAt = i + case "zebra": + siblingAt = i + } + } + if refusedAt < 0 || siblingAt < 0 { + t.Fatalf("the fixture must declare both submodules, got %v", order) + } + if refusedAt > siblingAt { + t.Fatalf("the refused submodule must be visited BEFORE the sibling (%v) — the sibling's roll is what proves the walk continued past the refusal", order) + } + // The operator's work: a commit in the nested checkout that NO ref references — // the shape the update drops. writeFile(t, filepath.Join(innerCo, "work"), "work") @@ -212,9 +252,10 @@ func TestGitSubmodules_BumpRefusalIsScopedToTheRefusedSubmodule(t *testing.T) { if got := gitIn(t, filepath.Join(umb, "outer"), "rev-parse", "HEAD"); got != o1 { t.Fatalf("the refused submodule's own checkout must stay at %s, got %s", o1, got) } - // The SIBLING rolled: the refusal is scoped to its own submodule, not the wave. - if got := gitlink(umb, "aaa-leaf"); got != L2 { - t.Fatalf("the sibling must still have rolled to %s — a refusal is per-submodule, got %s", L2, got) + // The SIBLING rolled: it sorts AFTER the refused path, so it is rolled only if the + // walk CONTINUED past the refusal. A walk that aborted here would leave it at L1. + if got := gitlink(umb, "zebra"); got != L2 { + t.Fatalf("the sibling must still have rolled to %s — the walk continues past the refusal, it does not abort — got %s", L2, got) } if got := gitIn(t, leafCo, "rev-parse", "HEAD"); got != L2 { t.Fatalf("the sibling's checkout must be at %s, got %s", L2, got) diff --git a/candy/plugin-task/verbs.go b/candy/plugin-task/verbs.go index 871771d..0625a7a 100644 --- a/candy/plugin-task/verbs.go +++ b/candy/plugin-task/verbs.go @@ -1410,7 +1410,11 @@ func submoduleAt(projDir, path string) (abs, gitArgs string, err error) { // // The refusal is PER-SUBMODULE, and that is the bump's failure contract // (opencharly/plugin-task#13): the walk collects a failed path and CONTINUES, so a path -// that sorts earlier has already been rolled and staged by the time this one is refused. +// that sorts earlier has already been rolled and staged by the time this one is refused, +// and a path that sorts LATER still rolls after it. The scope is held MECHANICALLY by +// TestGitSubmodules_BumpRefusalIsScopedToTheRefusedSubmodule, whose fixture refuses the +// FIRST path and asserts that the LATER one still rolled — the order is what makes the +// scope observable, and the test asserts the order. // The refusal names the nested checkout that carries the work, and the report says how // many paths were rolled — the scope of the refusal is the refused submodule, never the // tree (opencharly/plugin-task#56). @@ -1541,7 +1545,8 @@ func treeGitlinks(gitArgs, commit string) (links map[string]string, ok bool) { // leaves the REFUSED SUBMODULE exactly as it was — its checkout is not switched and // its pin is not staged. The scope is the refused submodule and NOT the tree: the walk // continues past a failed path (opencharly/plugin-task#13), and a path that sorts -// earlier has already been rolled and staged (opencharly/plugin-task#56). +// earlier has already been rolled and staged (opencharly/plugin-task#56). The scope is +// held by TestGitSubmodules_BumpRefusalIsScopedToTheRefusedSubmodule. func judgeNestedLevel(dir, gitArgs, target, label string) (spec.Status, string) { links, ok := treeGitlinks(gitArgs, target) paths := make([]string, 0, len(links))