Repository navigation
fix(git-submodules): refuse a non-positive stale_after, and never report an unreadable history as neglect - #44
Conversation
…ort an unreadable history as neglect 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 #42, #43 Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)
Review — BLOCKWhat the diff doesThe Go change is coherent and, on its own, looks correct:
The file count and diffstat in the body ( There are no comments to disposition (0 comment threads), so no maintainer sign-off exists and the change must stand on its own evidence. Blocks
Secondary concern (not independently blocking)
DispositionThe code and unit arms are directionally sound, and the README is consistent with them. But the "How tested" section — the only evidence offered for the two fixes and for R7 — is internally contradictory in both differential proofs, so the change cannot stand on its own evidence as submitted. Verdict: BLOCK ollama-cloud/deepseek-v4.1-flash — action-review. |
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 -- <path>` — 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)
|
(This comment was first posted as a bare path reference — STATUS — the validator's BLOCK is fixed in the control that produced it; the round's fix is committed, the body is rewritten, and the RCA is below. Head. Done this round. Each stale arm now has its own fixture, its own task name and its own bound, so every arm's output names the arm that produced it. The unreadable arm now demonstrates its own brokenness: the very read the verb performs must FAIL on the unreadable fixture and SUCCEED on the healthy fixture, or the arm fails loudly. The evidence in the body is extracted from the logs by the arm's header, and each block carries that header. Both differentials were re-run against the strengthened gate and both discriminate: fix 1 reverted shows the Next. The validator's verdict on this head. Nothing is blocked: the consumer leg opencharly/charly#1048 waits on this producer leg landing and its tag being written, and starts on that. Issue #42 stays open until #44 lands, and is closed with its RESOLVED record then. RCA — the BLOCK on this PR was a defect in my own evidence, and the root fix went into the control that produced it. What the validator found. Both differential pastes in the "How tested" section could not have come from the arms they claimed to describe: the fix-1 paste carried the 168h bound of the unrelated Mechanism. I extracted each evidence block from the differential log by the recorded SHA of the pin under test. That key is not unique to an arm: every stale fixture in one run pins the same twin commit (the run's shared fixture source, Missed control. My own verification of each paste checked that the text existed somewhere in the differential log ("verbatim in body = True") and stopped there. It never checked the text was the output of the arm the claim was about. That is a presence check standing in for a provenance check — the check was weaker than the claim it was backing, so a wrong-arm paste passed it. The pre-push self-audit reviewed the body for completeness and for lint, and neither looks at whether a pasted block belongs to the claim it sits under. Blast radius. The PR body only. No shipped behaviour was wrong: the two fixes and their unit arms were judged directionally sound, and the README matches the code. But the body was the only R7 proof offered for both fixes, so both proofs were unsupported — the change could not stand on its own evidence, which is exactly what the verdict said. Root cause. Evidence was selected by an ambiguous key (a SHA shared by several arms) instead of by the arm's own identity. Root fix — in the control, not only in the body.
The secondary concern is answered. The two The rewritten body pastes each arm's own block, keyed by its own header, with the demonstration line included, and the two differentials re-run against the strengthened gate. Agent: charly Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated) |
Review — PASSWhat the diff doesThe two code fixes are present and correct, and they match the body's claims. Fix 1 — refuse a non-positive bound. Fix 2 — three-outcome history read. The old Tests. README states both enforced rules and matches the code. Gate script. Evidence consistency (re-derived against current state)
Comments dispositioned
No designated maintainer sign-off is present, so the change stands on its own evidence — which is now internally consistent. Non-blocking observations
BlocksNone. Verdict: PASS ollama-cloud/deepseek-v4.1-flash — action-review. |
Summary
verb:git-submodulesinmode: currencygained the opt-instale_afterknob in#43 (merged as
131b7df, taggedv2026.296.2124). The validator'sreview of that PR carried two non-blocking observations. R1 is explicit that a finding
raised on your own PR is fixed, or rebutted on the thread with evidence — never filed away.
Both are fixed here, in a follow-up PR, because #43 is merged and tagged and its branch can
no longer carry a fix.
Head
51f89a1af81694ead6d834b85b6e4a8bd5978a11, onfeat/pin-staleness-hardening, branched offorigin/mainatc908275. This is the successor of #43 and carries the same scope(#42); the consumer leg is opencharly/charly#1048.
The validator's BLOCK on this PR is also fixed here, not filed away. The BLOCK was a
defect in the evidence this PR offered, not in the code: the two differential pastes were
selected from their logs by a key that several arms share, so the paste carried the wrong
arm's output. The root fix went into the control that produced the paste, not only into the
body — see "The evidence control, fixed" below and the RCA posted on this thread.
Finding 1 — a non-positive bound was honoured instead of refused
Expected. 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.
Actual.
stale_after: "0s"parsed and was applied. The live gate reports1 STALE pin(s) (neglected for more than 0h)— the permanently-red verdict, reachedthrough the operator's own hand.
Mechanism. The parse block rejected only text
time.ParseDurationcould not read. Anyduration it could read was accepted, and the positivity of the bound was never tested.
Missed control. The knob's contract is enforced at the reading side (the age is
compared against the bound) and never at the bound side (whether the bound is a bound at
all). Every arm in the unit test and in the live gate exercised a positive bound, so no arm
could reach the gap.
Blast radius. Only a repo that declared a zero or negative bound. The knob is opt-in and
no repo declares it yet, so nothing in the org is affected today.
Root cause. The value was validated as a duration but not as a bound.
Root fix. A non-positive duration is refused in the verb — the canonical place — with a
message that names the reason. Covered by a unit arm (three values:
0s,0,-1h) and bya live arm that asserts both the refusal and the absence of any staleness judgement.
Finding 2 — a failed history read was reported as neglect
Expected. A pin whose age cannot be measured is not reported as neglected. An unknown
age is not a claim that the owner neglected the pin.
Actual. An unreadable history produced
it has never been advanced hereand a STALEverdict — a claim about the owner's conduct, backed by no evidence.
Mechanism.
lastAdvancedAtreturned(time.Time{}, false)for every failure mode ofgit log, and the caller read the singlefalseas "never advanced". But two differentthings arrive at that same exit status: a repo with no history (the path is in no commit
— a FACT, and it is neglected) and a repo whose history cannot be read (a broken object
store, a transient git failure — the age is UNKNOWN).
Missed control. git reports those two conditions identically, and nothing in the verb
told them apart. Measured before choosing a discriminator:
git logexits 128 for an unbornHEAD ("does not have any commits yet") and 128 for a broken object store ("fatal: bad object
HEAD");
git rev-parse --verify -q HEAD— the first discriminator reached for — exits 1 forBOTH an unborn HEAD and a broken HEAD ref, so it cannot separate them either. The
discriminator that works is
git rev-list --all --count: zero commits means there is nothinghere that could have been advanced (the fact), a count that cannot be produced means the
history cannot be read (the failure).
Blast radius. Any repo whose object store is temporarily unreadable. The verdict would
name a pin STALE on no evidence, and the owner would be told to do work that is not the
cause.
Root cause. One absence — no age — carried two meanings, and one of them was a claim the
evidence did not support.
Root fix. The read now has three outcomes, and only one of them is a claim about the pin:
a measured age, no-history (the path is in no commit of this repo's history, so it
has never been advanced here — 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 read-failure case is routed to the structural class
precisely so that a transient git error can never manufacture a stale verdict. Covered by a
unit arm (a removed HEAD commit object) and by a live arm that asserts the verdict names the
read failure and carries no STALE pin.
How tested
Every command below was run on the committed tree above, in the session worktree
opencharly/plugin-task, with the working tree clean (git status --shortprints nothing).The diff itself:
The Go gates, the repo's own CI set:
The configured linter set, under the toolchain
candy/plugin-task/go.moddeclares(
go 1.26.4) — the same toolchain CI selects throughgo-version-file(the linter isnot on this host's
PATH; that condition is recorded under## Filed issues):The structural check over the repo's own manifest, with the worktree-local charly build
(R9 — not a shared install):
The evidence control, fixed
The previous round of this PR pasted its differential evidence 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 that SHA appears in more than one arm's verdict, and the extraction
returned the wrong arm's line. The fix is in the control, not only in the body:
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.
the verb performs —
git log -1 --format=%ct -- <path>— fails on the unreadablefixture 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.
|convention, so thepasted block is the arm's own output, under the arm's own header.
not by hand.
The live gate — this repo's own proof that the verbs work against a real
charlybinary and real git fixtures. It is the ONLY live execution of these verbs, and it runs
every one of the five maintenance verbs in the same run:
The differential proof (R7). Each fix was reverted in place, in the working tree, and
the arm was re-run against the same fixtures; the file was then restored byte-for-byte
from the committed copy. The arms discriminate — each one goes red with its own fix
reverted. Each block below is that arm's own output, extracted by its own header.
Fix 1 reverted (fix 2 in place), so the run reaches the
currency-nonpositivearm andstops there:
The arm that must REFUSE the bound instead reports the permanently-red verdict it exists
to prevent —
neglected for more than 0h,bound 0h— on thecurrency-nonpositivetask, and the gate's assertion fails on it. That is the discrimination the previous round
failed to show.
Fix 2 reverted (fix 1 in place), so the run reaches the
currency-unreadablearm:The arm that must report an unreadable history as unreadable instead reports it as
neglect — the false claim this fix removes. The same difference in the unit test, with the
same fix reverted:
Both live blocks and the unit block now say the same thing about the same fixture: with
fix 2 reverted the age is not measured, and the verdict is a stale claim.
The
firstLinehelper is pre-existing (the validator's secondary concern). It is notadded by this diff:
and its callers — the two
cli.gosites and the comment above them are pre-existing, thetwo
verbs.gosites are this change's:The sweep (R5). The old two-outcome helper is gone, replaced in place by the
three-outcome form; the "never advanced" claim now has exactly one producer, and it is
reachable only from the no-history case:
The README's
stale_afterblock now states both rules the code enforces — the bound mustbe positive, and an unreadable history is never reported as neglect.
Rulebook compliance
/charly-internals:git-workflow(and its
references/pre-validator-self-audit.md),/charly-internals:plugin,/charly-internals:root-cause-analyzer, and this repo's ownAGENTS.md./charly-image:layeris N/A — no candy entity field and no plan step changed; the diffis Go source, the README, and the gate script.
stale_after#43 are fixed here, and each carries afresh RCA in this body (expected, actual, mechanism, missed control, blast radius, root
cause, root fix). The validator's BLOCK on this PR was an anomaly, and it got its
own fresh RCA: the mechanism, the missed control, the blast radius, the root cause and
the root fix are in "The evidence control, fixed" above and on the thread. Every finding
raised on this PR is fixed here — none is filed away.
read-failure split replaces the two-outcome helper in place rather than adding a second
helper alongside it. The sweep above shows one definition site and one claim producer.
or a suppressed failure; there is no sleep, no retry, and no ad-hoc shell.
stale_afterblock is updated in the samechange, and it states the two rules the code now enforces.
sweep pasted above.
main. The branch is fresh offorigin/mainatc908275; the pre-merge branchfeat/pin-stalenessis left untouched, and no other session's work was touched.arm was re-run against the same fixtures, with the arm's own output pasted above.
charlybinary with thiscandy compiled in and drives it against real git fixtures, and no service is mocked.
tree and the run above is that binary's output;
charly box validateused theworktree-local
opencharly/charly/charly/bin/charly, not a shared install.charlymain into a freshtemporary directory, builds a fresh binary there, and every fixture is created and
destroyed inside it.
Change classification
Plugin library, runtime behaviour (a fix to an opt-in parameter). The change class's
gate is this repo's full bed —
gofmt,go build,go vet,go test,golangci-lint,charly box validateandscripts/gate-task.sh— all run on the committed tree above, withzero warnings. The umbrella's R10 rule that a plugin-library change runs the full assembled
bed is satisfied by this repo's own assembled bed: the plugin is compiled into the binary
the live gate builds, and every one of the five maintenance verbs is executed live in that
same run.
Producer-first. This is the producer leg for opencharly/charly#1048: the verb ships
here, and the owner declares the bound and advances its twin in the consumer repo
afterwards.
Filed issues
PATH, and thehost's Go (1.27.3) is newer than the analysis side the pinned linter was built against,
so a run under the host's default toolchain dies with
export data version 5 is greater than maximum supported version 4. The run pasted above uses the toolchaincandy/plugin-task/go.moddeclares (go 1.26.4), which is the toolchain CI selectsthrough
go-version-file, and it lints 0 issues. The condition is already recorded onthat issue, with this evidence added; no duplicate was filed.
Closes #42
Agent: charly
Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)