From 5e4b7230737b9f57c07c70ac375a46b547f1dd27 Mon Sep 17 00:00:00 2001 From: Andreas Trawoeger Date: Fri, 23 Oct 2026 23:38:55 +0200 Subject: [PATCH 1/2] fix(git-submodules): refuse a non-positive stale_after, and never report an unreadable history as neglect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings the validator raised on #43 as non-blocking observations are fixed here. Neither is filed away: a finding raised on your own PR is fixed, or rebutted on the thread with evidence. A bound of zero or less is not a bound. `stale_after: "0s"` parsed and was honoured, marking EVERY not-current pin stale — the permanently-red gate the knob exists to avoid, arrived at by the operator's own hand. A non-positive bound is now refused as the operator error it is, and the refusal names why. A failed history read is not neglect. `lastAdvancedAt` treated any `git log` failure as "never advanced here" and flagged the pin stale, so a transient git error could manufacture a false stale verdict — a claim the evidence does not support. The read now has three outcomes: a measured age, "no history" (the path is in no commit of this repo's history — a FACT, and it IS neglected), and "read failed" (the age could not be measured — the workflow being unable to JUDGE the pin, which is a structural-class failure naming the read error). The two are told apart by COUNTING the repo's commits, because `git log` refuses identically on an unborn HEAD and on a broken object store, and only the first of those is the "never advanced" fact. Proven differentially, in the unit test and in the live gate: with the refusal reverted, `stale_after: "0s"` reports `1 STALE pin(s) ... (bound 0h)`; with the read-failure split reverted, an unreadable history is reported as `has never been advanced here` and as STALE. Both arms are green with both fixes in. Refs opencharly/plugin-task#42, opencharly/plugin-task#43 Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated) --- README.md | 8 ++++ candy/plugin-task/currency_test.go | 69 ++++++++++++++++++++++++++++++ candy/plugin-task/verbs.go | 67 +++++++++++++++++++++++------ scripts/gate-task.sh | 41 ++++++++++++++++++ 4 files changed, 173 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 1c0375e..8eece17 100644 --- a/README.md +++ b/README.md @@ -75,6 +75,14 @@ currency: # red on a healthy tree, which trains every reader to ignore it. Leave it unset and # the verdict is the structural one alone. # +# The bound must be a POSITIVE duration: zero (or less) is refused rather than +# honoured, because it would mark EVERY not-current pin stale — the permanently-red +# gate this knob exists to avoid. +# +# The age is read from this repo's OWN history, and an UNREADABLE history is never +# reported as neglect: it is a structural failure that names the read error, because +# an unknown age is not a claim that the owner neglected the pin. +# # git-submodules: {mode: currency, stale_after: "168h"} # Assert the harness config files are identical to charly's twins. diff --git a/candy/plugin-task/currency_test.go b/candy/plugin-task/currency_test.go index 69bfc08..d9a06d0 100644 --- a/candy/plugin-task/currency_test.go +++ b/candy/plugin-task/currency_test.go @@ -511,4 +511,73 @@ func TestGitSubmodules_CurrencyStalenessBoundsNeglect(t *testing.T) { if st != spec.StatusFail || !strings.Contains(msg, "not a duration") { t.Fatalf("a malformed bound must fail loud; got %s: %s", st, msg) } + + // Arm 6 — a NON-POSITIVE bound is refused. Zero (or less) is not a bound: it marks + // EVERY not-current pin stale, which is precisely the permanently-red gate this knob + // exists to avoid, so it is an operator error the verb refuses rather than honours. + // The validator's round-1 observation named it. + for _, bad := range []string{"0s", "0", "-1h"} { + st, msg = runMaintenanceVerbIn(oldDir, "git-submodules", map[string]any{ + "mode": "currency", "stale_after": bad, + }) + if st != spec.StatusFail || !strings.Contains(msg, "not a POSITIVE duration") { + t.Fatalf("a non-positive bound must be refused; got %s: %s", st, msg) + } + if strings.Contains(msg, "STALE pin(s)") { + t.Fatalf("a refused bound must not judge staleness; got:\n%s", msg) + } + } + + // Arm 7 — a FAILED READ is not neglect. When the repo's history cannot be read, the + // age of the last advance is UNKNOWN, and an unknown is not a claim that the owner + // neglected the pin. It is reported as a structural failure that names the read + // error — the workflow being unable to JUDGE the pin — and never as a stale verdict. + // + // The corruption is real: the superproject's HEAD commit object is removed, so the + // history genuinely cannot be read. The tree is otherwise the same neglect fixture, so + // the ONLY difference from Arm 2 is the readability of the evidence. + brokenDir, _, _ := curStale(t, tmp, "broken", true, 240*time.Hour) + brokenSHA := curGit(t, "-C", brokenDir, "rev-parse", "HEAD") + brokenObj := filepath.Join(brokenDir, ".git", "objects", brokenSHA[:2], brokenSHA[2:]) + if err := os.Remove(brokenObj); err != nil { + t.Fatalf("remove the HEAD commit object: %v", err) + } + st, msg = runMaintenanceVerbIn(brokenDir, "git-submodules", map[string]any{ + "mode": "currency", "stale_after": bound, + }) + if st != spec.StatusFail { + t.Fatalf("an unreadable history must fail, not pass; got %s: %s", st, msg) + } + if strings.Contains(msg, "STALE pin(s)") { + t.Fatalf("an unreadable history must never be reported as neglect; got:\n%s", msg) + } + if !strings.Contains(msg, "could not be read") { + t.Fatalf("an unreadable history must name the read failure; got:\n%s", msg) + } + + // Arm 8 — a pin in NO commit of this repo's history. The path is in the index and in + // .gitmodules, but this repo has never committed an advance of it, so its age is + // unbounded and it IS neglected. This is the third outcome of the read, and it is + // the opposite of Arm 7: the history reads FINE here (the repo has commits), it simply + // holds no advance of THIS path. A read that returns nothing is a fact about the + // bookkeeping, not a failure to read it. + noHistDir, run, c1, c2, _ := curSuper(t, filepath.Join(tmp, "nohist")) + curRecord(t, noHistDir, "d", c2) // current: isolates the arm under test + curRecord(t, noHistDir, "src", c1) + // Commit ONLY .gitmodules: the gitlinks stay in the index, so the pins exist for the + // currency mode (which reads the INDEX) while no commit of this repo ever touched + // the path `src`. + run("commit", "-qm", "the modules only", "--", ".gitmodules") + st, msg = runMaintenanceVerbIn(noHistDir, "git-submodules", map[string]any{ + "mode": "currency", "stale_after": bound, + }) + if st != spec.StatusFail { + t.Fatalf("a pin this repo has never advanced must fail; got %s: %s", st, msg) + } + if !strings.Contains(msg, "has never been advanced here") { + t.Fatalf("a pin in no commit must be reported as never advanced here; got:\n%s", msg) + } + if strings.Contains(msg, "could not be read") { + t.Fatalf("a readable history must not be reported as unreadable; got:\n%s", msg) + } } diff --git a/candy/plugin-task/verbs.go b/candy/plugin-task/verbs.go index 5833bff..11fd9b1 100644 --- a/candy/plugin-task/verbs.go +++ b/candy/plugin-task/verbs.go @@ -878,6 +878,14 @@ func runGitSubmodules(projDir string, in params.GitSubmodulesInput) (spec.Status return spec.StatusFail, fmt.Sprintf( "git-submodules: stale_after %q is not a duration this verb can parse (e.g. \"168h\")", raw) } + // A bound of zero or less is not a bound: it marks EVERY not-current pin + // stale, which is the permanently-red gate this knob exists to avoid. The + // validator's round-1 observation named it, and it is refused here rather + // than left to the operator's discretion. + if d <= 0 { + return spec.StatusFail, fmt.Sprintf( + "git-submodules: stale_after %q is not a POSITIVE duration — a bound of zero or less marks every not-current pin stale, which is the permanently-red gate this bound exists to avoid", raw) + } staleAfter, staleBoundSet = d, true } type pin struct { @@ -985,25 +993,50 @@ func runGitSubmodules(projDir string, in params.GitSubmodulesInput) (spec.Status // lastAdvancedAt reads the AGE of this repo's OWN bookkeeping for a pin: the // timestamp of the last commit that touched the pin's path. This is the owner's own // record of when it last advanced the pin — never the pin's distance from the served - // ref, which is churn. An ABSENT record (the path is in no commit of this repo's - // history) is reported as "never advanced here" rather than as a fresh pin, because - // a path with no bookkeeping has not been advanced for as long as the repo has - // existed. - lastAdvancedAt := func(p string) (time.Time, bool) { - out, _, exit := hostCapture(context.Background(), projDir, + // ref, which is churn. + // + // The THREE outcomes are different and must not be conflated, because they mean + // different things and only ONE of them is a claim about the pin: + // * a TIMESTAMP — the age, measured; + // * an EMPTY result — the path is in no commit of this repo's history, so this + // repo has never advanced it: its age is unbounded and it IS neglected; + // * a FAILING read — the age could not be measured at all (a broken object + // store, a transient git failure). That is the workflow being unable to JUDGE + // the pin, and it is reported as such — never as neglect, which would be a + // claim the evidence does not support. + // The validator's round-1 observation named the conflation; the outcomes are now + // separate. + // + // A repo with NO COMMIT AT ALL is the third case, and it is a FACT rather than a + // failure: `git log` refuses on an unborn HEAD, but the refusal means "there is + // no history here", which is exactly the "never advanced" claim. It is told apart + // from a genuine read failure by COUNTING the repo's commits: zero commits means + // there is nothing here to have advanced anything (a fact), while a count that + // cannot be produced, or a history that exists but whose age for this path cannot + // be read, is a failure to JUDGE. + lastAdvancedAt := func(p string) (at time.Time, how, detail string) { + out, errOut, exit := hostCapture(context.Background(), projDir, fmt.Sprintf("git log -1 --format=%%ct -- %s", shellQuote(p))) if exit != 0 { - return time.Time{}, false + cOut, cErr, cExit := hostCapture(context.Background(), projDir, "git rev-list --all --count") + if cExit != 0 { + return time.Time{}, "read-failed", firstLine(cErr) + } + if n, perr := strconv.Atoi(strings.TrimSpace(cOut)); perr == nil && n == 0 { + // This repo has no commit at all: nothing was ever advanced here. + return time.Time{}, "no-history", "" + } + return time.Time{}, "read-failed", firstLine(errOut) } ts := strings.TrimSpace(out) if ts == "" { - return time.Time{}, false + return time.Time{}, "no-history", "" } secs, err := strconv.ParseInt(ts, 10, 64) if err != nil { - return time.Time{}, false + return time.Time{}, "read-failed", "unparsable timestamp " + strconv.Quote(ts) } - return time.Unix(secs, 0), true + return time.Unix(secs, 0), "ok", "" } // THE GRAPH, classified once for EVERY path, then the depth rule walked over EVERY @@ -1219,8 +1252,18 @@ func runGitSubmodules(projDir string, in params.GitSubmodulesInput) (spec.Status if !r.leaf || !staleEligible(r.state) { continue } - at, known := lastAdvancedAt(r.path) - if !known { + at, how, detail := lastAdvancedAt(r.path) + switch how { + case "read-failed": + // The age could not be measured. That is the workflow being unable to + // JUDGE the pin — a structural-class failure that names the read + // error — and NOT a claim that the pin is neglected. A transient git + // error must never manufacture a stale verdict. + structural = append(structural, fmt.Sprintf( + "%s: the age of this repo's own last advance of it could not be read (%s) — the staleness of this pin cannot be judged", + r.path, detail)) + continue + case "no-history": stale = append(stale, fmt.Sprintf( "%s: %s, and this repo's own history records no advance of it — it has never been advanced here", r.path, r.state)) diff --git a/scripts/gate-task.sh b/scripts/gate-task.sh index 3c40ef4..870fddd 100755 --- a/scripts/gate-task.sh +++ b/scripts/gate-task.sh @@ -572,6 +572,47 @@ printf '%s\n' "$fresh_out" | grep -q '1 of 1 pin(s) NOT current' || { echo "FAIL: the not-current count must still be reported; got: $fresh_out" >&2; exit 1; } echo " the same behind pin, advanced recently, passed the bound" +echo "== verb:git-submodules currency (stale_after: a NON-POSITIVE bound must be REFUSED) ==" +# Zero (or less) is not a bound: it would mark EVERY not-current pin stale, which is +# the permanently-red gate the knob exists to avoid. The verb refuses it rather than +# honouring an operator error that would silently disarm the control in the other +# direction. +cat > "$WORK/stale-old/charly.yml" <<'YML' +currency-nonpositive: + task: + description: a bound of zero is refused, not honoured + dir: . + plan: + - check: no pin is neglected past the bound + git-submodules: {mode: currency, stale_after: "0s"} + context: [runtime] +YML +if nonpos_out="$(cd "$WORK/stale-old" && "$CH" task currency-nonpositive 2>&1)"; then + echo "FAIL: a non-positive bound must be refused" >&2; printf '%s\n' "$nonpos_out" >&2; exit 1 +fi +printf '%s\n' "$nonpos_out" | grep -q 'not a POSITIVE duration' || { + echo "FAIL: the refusal must name the reason; got: $nonpos_out" >&2; exit 1; } +printf '%s\n' "$nonpos_out" | grep -q 'STALE pin(s)' && { + echo "FAIL: a refused bound must not judge staleness; got: $nonpos_out" >&2; exit 1; } +echo " a bound of zero was refused, and no staleness was judged" + +echo "== verb:git-submodules currency (an UNREADABLE history is not neglect) ==" +# The same neglect fixture, with its HEAD commit object removed, so the history +# genuinely cannot be read. The age of the last advance is then UNKNOWN — and an +# unknown is not a claim that the owner neglected the pin. The verdict names the read +# failure and never says STALE. +stale_fixture "$WORK/stale-unreadable" "2020-01-01T00:00:00Z" +unread_sha="$(cd "$WORK/stale-unreadable" && git rev-parse HEAD)" +rm -f "$WORK/stale-unreadable/.git/objects/${unread_sha:0:2}/${unread_sha:2}" +if unread_out="$(cd "$WORK/stale-unreadable" && "$CH" task currency-stale 2>&1)"; then + echo "FAIL: an unreadable history must fail, not pass" >&2; printf '%s\n' "$unread_out" >&2; exit 1 +fi +printf '%s\n' "$unread_out" | grep -q 'could not be read' || { + echo "FAIL: the verdict must name the read failure; got: $unread_out" >&2; exit 1; } +printf '%s\n' "$unread_out" | grep -q 'STALE pin(s)' && { + echo "FAIL: an unreadable history must never be reported as neglect; got: $unread_out" >&2; exit 1; } +echo " an unreadable history was reported as unreadable, not as neglect" + echo "== verb:file-parity (identical must PASS, drift must FAIL) ==" par_out="$("$CH" task parity)"; echo "$par_out" | grep -q '0 failed' || { echo "FAIL: identical pair must pass" >&2; exit 1; } echo drifted >> r.copy From 51f89a1af81694ead6d834b85b6e4a8bd5978a11 Mon Sep 17 00:00:00 2001 From: Andreas Trawoeger Date: Fri, 23 Oct 2026 23:49:44 +0200 Subject: [PATCH 2/2] test(gate-task): make each stale arm's evidence self-identifying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The validator's BLOCK on #44 was a defect in this PR's own EVIDENCE, not in the code: the differential pastes were extracted from their logs by a key that several arms share (the recorded SHA of the pin under test — every stale fixture in one run pins the same twin commit), so the paste carried the wrong arm's output. The root fix belongs in the control that produced the paste, not only in the PR body. * Each stale arm gets its OWN fixture (so its own pin and its own HEAD), its OWN task name and its OWN bound. The task name appears in the run's own output, so an arm's evidence block names the arm that produced it, and no two arms can produce the same text. * The unreadable arm now DEMONSTRATES its own brokenness: it asserts that the very read the verb performs — `git log -1 --format=%ct -- ` — FAILS on the unreadable fixture and SUCCEEDS on the healthy fixture, and it fails loudly if either does not hold. The arm can no longer pass for the wrong reason (a no-op object removal would have left a readable history). * Each arm's captured output is printed with the gate's own " | " convention, so the pasted block is the arm's own output, under the arm's own header. The verb, the unit test and the README are unchanged by this commit: the two fixes under review are the same ones, proven by the same arms, which are now self-identifying. Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated) --- scripts/gate-task.sh | 51 +++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/scripts/gate-task.sh b/scripts/gate-task.sh index 870fddd..e4425be 100755 --- a/scripts/gate-task.sh +++ b/scripts/gate-task.sh @@ -524,7 +524,13 @@ echo "$exp_out" | grep -q 'which is ITSELF an edge' || { echo "FAIL: the refusal # --------------------------------------------------------------------------- . "$FIX/shas" stale_fixture() { - local dir="$1" stamp="$2" + # The task name and the bound are parameters because every arm's output must be + # identifiable by ITSELF. Two arms used to share one fixture — and therefore one + # recorded SHA — so an evidence paste keyed only by the SHA was ambiguous: the + # same key selected the healthy arm's verdict. Now each arm has its OWN fixture + # (so its own SHA) and its OWN task name (which appears in its own output), and + # an evidence paste is keyed by the arm, not by a SHA. + local dir="$1" stamp="$2" task="${3:-currency-stale}" bound="${4:-168h}" mkdir -p "$dir" ( cd "$dir" && git init -q -b main @@ -537,14 +543,14 @@ stale_fixture() { # not merely "differs (no checkout to judge ancestry)". git -c protocol.file.allow=always submodule update --init -q gap ) - cat > "$dir/charly.yml" <<'YML' -currency-stale: + cat > "$dir/charly.yml" <&1)" || { +stale_fixture "$WORK/stale-fresh" "$(date -u +%Y-%m-%dT%H:%M:%SZ)" currency-fresh +fresh_out="$(cd "$WORK/stale-fresh" && "$CH" task currency-fresh 2>&1)" || { echo "FAIL: a pin advanced just now is churn, not neglect" >&2; printf '%s\n' "$fresh_out" >&2; exit 1; } printf '%s\n' "$fresh_out" | grep -q '0 failed' || { echo "FAIL: the run must pass; got: $fresh_out" >&2; exit 1; } @@ -577,19 +583,14 @@ echo "== verb:git-submodules currency (stale_after: a NON-POSITIVE bound must be # the permanently-red gate the knob exists to avoid. The verb refuses it rather than # honouring an operator error that would silently disarm the control in the other # direction. -cat > "$WORK/stale-old/charly.yml" <<'YML' -currency-nonpositive: - task: - description: a bound of zero is refused, not honoured - dir: . - plan: - - check: no pin is neglected past the bound - git-submodules: {mode: currency, stale_after: "0s"} - context: [runtime] -YML -if nonpos_out="$(cd "$WORK/stale-old" && "$CH" task currency-nonpositive 2>&1)"; then +# +# This arm has its OWN fixture, so its recorded SHA is its own and cannot be confused +# with the healthy arm's SHA; and its own task name, which appears in its own output. +stale_fixture "$WORK/stale-nonpositive" "2020-01-01T00:00:00Z" currency-nonpositive 0s +if nonpos_out="$(cd "$WORK/stale-nonpositive" && "$CH" task currency-nonpositive 2>&1)"; then echo "FAIL: a non-positive bound must be refused" >&2; printf '%s\n' "$nonpos_out" >&2; exit 1 fi +printf '%s\n' "$nonpos_out" | sed 's/^/ | /' printf '%s\n' "$nonpos_out" | grep -q 'not a POSITIVE duration' || { echo "FAIL: the refusal must name the reason; got: $nonpos_out" >&2; exit 1; } printf '%s\n' "$nonpos_out" | grep -q 'STALE pin(s)' && { @@ -601,12 +602,24 @@ echo "== verb:git-submodules currency (an UNREADABLE history is not neglect) ==" # genuinely cannot be read. The age of the last advance is then UNKNOWN — and an # unknown is not a claim that the owner neglected the pin. The verdict names the read # failure and never says STALE. -stale_fixture "$WORK/stale-unreadable" "2020-01-01T00:00:00Z" +stale_fixture "$WORK/stale-unreadable" "2020-01-01T00:00:00Z" currency-unreadable unread_sha="$(cd "$WORK/stale-unreadable" && git rev-parse HEAD)" rm -f "$WORK/stale-unreadable/.git/objects/${unread_sha:0:2}/${unread_sha:2}" -if unread_out="$(cd "$WORK/stale-unreadable" && "$CH" task currency-stale 2>&1)"; then +# DEMONSTRATE that the removal genuinely broke the read the verb performs, so this +# arm cannot pass for the wrong reason (a no-op removal would leave a readable +# history and the arm would prove nothing). The very same read must FAIL here and +# SUCCEED on the healthy fixture. +if (cd "$WORK/stale-unreadable" && git log -1 --format=%ct -- gap) >/dev/null 2>&1; then + echo "FAIL: the unreadable fixture's history read must FAIL, or this arm proves nothing" >&2; exit 1 +fi +if ! (cd "$WORK/stale-old" && git log -1 --format=%ct -- gap) >/dev/null 2>&1; then + echo "FAIL: the healthy fixture's history read must SUCCEED, or the contrast proves nothing" >&2; exit 1 +fi +echo " the fixture's history read genuinely fails, and the healthy fixture's succeeds" +if unread_out="$(cd "$WORK/stale-unreadable" && "$CH" task currency-unreadable 2>&1)"; then echo "FAIL: an unreadable history must fail, not pass" >&2; printf '%s\n' "$unread_out" >&2; exit 1 fi +printf '%s\n' "$unread_out" | sed 's/^/ | /' printf '%s\n' "$unread_out" | grep -q 'could not be read' || { echo "FAIL: the verdict must name the read failure; got: $unread_out" >&2; exit 1; } printf '%s\n' "$unread_out" | grep -q 'STALE pin(s)' && {