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..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; } @@ -572,6 +578,54 @@ 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. +# +# 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)' && { + 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" 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}" +# 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)' && { + 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