Repository navigation
feat(verb): pin-closure — the transitive closure of a pin advance, iterated to a fixed point - #63
Conversation
…erated to a fixed point Adopting a pin in a multi-module repo is not one edit: each module that pins the repo also pins its SIBLINGS, so advancing one pin surfaces the next module's breakage, and the next. `pin-closure` answers the whole question in one word: compile the composed set, collect EVERY failing module, classify each failure as a BUMP (a published version already carries the fix) or an EDIT (none does — that module needs its own cutover), and iterate to a fixed point. - `pin_closure.go` — resolve the pair, write a go.work overlay over the anchor module plus any already-composed dirs (they win over the pins), build, parse the failures into module verdicts, classify against the module the FAILING PACKAGE belongs to (these repos tag both the repo and the subdir module, and the repo-root tag can already be fixed while the tag a consumer pins is not), and report an EMPTY closure, a BUMP list, or a REFUSED advance naming the EDITs. - `schema/task.cue` + `params/cue_types_gen.go` — `#PinClosureInput` (anchor, compose, module_prefix); the generated types are REGENERATED from the schema with `charly candy params` (drift gate `--check` clean), which is also how the cue_types_gen conflict against current main was resolved. - `plugin.go`/`verbs.go`/`charly.yml` — declare and route the sixth maintenance verb; the candy's capability probe now asserts all nine (three task classes + six verbs). - `AGENTS.md`/`README.md`/`scripts/gate-task.sh` — the five-to-six arithmetic in the same change (R5: no stale count survives), and the repo's own live gate DRIVES the new verb: a compiling set must report EMPTY, and a module whose package does not compile must be classified EDIT with the advance REFUSED — the shape that must never be silently "adopted". - `pin_closure.go runGo` — a workspace build must be hermetic about `-mod`: workspace mode forbids it outright, and this repo's own gate exports `GOFLAGS=-mod=mod` for its charly build, so the fixture run inherited it and failed with a flag error the operator never set against the verb. Measured by the gate section itself; the sanitizer drops `-mod` and keeps every other flag. Re-cut onto current origin/main (the previous tip was 33 commits behind and main had moved verbs.go), so the tree is gated on the base it will merge into. Gate on this tree: gofmt clean; go build ./... rc=0; go vet ./... rc=0; go test ./... rc=0 (incl. the new TestGoFlagsWithoutMod); charly box validate rc=0 (0 warnings); `charly candy params plugin-task --check` rc=0 (no schema drift); `scripts/gate-task.sh` PASS on a fresh charly build carrying this candy — command:task + all six maintenance verbs executed live. Agent: plugins Assisted-by: DeepSeek Harness deepseek-v4.1-flash (fully tested and validated)
Review — BLOCKHead SHA: Change class: code — new Comment thread: empty — nothing to disposition. Blocks
Verdict: BLOCK ollama-cloud/deepseek-v4.1-flash — action-review. |
…space floor, and prove the iteration (round-1 review fix)
Round 1 found three real defects in the new verb. All three were right, and two were defects in the
product rather than in its proof:
1. The version lookup and the replace key did not agree. `newestModuleVersion(anchor, f.Pkg)` was
handed a PACKAGE path while `replaces[closureModule(f.Pkg)]` was keyed on the 3-segment "repo
root" — so for a subdir module (github.com/opencharly/plugin-box/candy/plugin-box) the lookup
asked the toolchain about a path that is not a module (BUMP unreachable, every failure forced to
EDIT) and the replace, where it was written, targeted the wrong module. `moduleOf` now ASKS the
toolchain (`go list -f {{.Module.Path}}`) and both the lookup and the replace use that one path;
`closureModule` is the documented fallback and no longer claims a resolution it never did.
2. `writeOverlay` hardcoded `go 1.26.4` into every generated workspace, over-constraining every
closure build to one toolchain (a module declaring go 1.24 was built under a workspace demanding
1.26.4) and failing on an older host. The `go` line is now the ANCHOR's own directive
(`workspaceGo`), with a conservative floor only when its go.mod cannot be read.
3. The headline — "iterate to a fixed point" — had no witness: both gate fixtures end at rung 1
with nothing adopted, so the BUMP/adoption/re-compose/second-rung path never ran. It now does,
hermetically: TestRunPinClosure_IteratesToAFixedPointAfterABump builds a real three-module
fixture (a pinned dep whose own package does not compile, and a newest version that does), swaps
only the version-lookup seam, and requires rung 1 to see the breakage, classify BUMP, ADOPT it,
and rung 2 to compile. Reverting fix 1 makes that test fail with the module key it names, which
is the proof it is testing the fix rather than the fixture.
An adopted module is now replaced by its DOWNLOADED DIR when the version lookup resolved one
(adoptionTarget): the source is already on disk, and a version form would make the next build
re-resolve it — needing the network mid-build and, for a module the registry cannot serve,
reintroducing exactly the failure this verb exists to find.
Gate on this tree: gofmt clean; go build ./... rc=0; go vet ./... rc=0; go test ./... rc=0 (incl.
the three new tests); charly candy params plugin-task --check rc=0 (no schema drift); charly box
validate rc=0 (0 warnings); scripts/gate-task.sh PASS on a fresh charly build carrying this candy.
Agent: plugins
Assisted-by: DeepSeek Harness deepseek-v4.1-flash (analysed on a live system)
Review — BLOCKHead SHA: Change class: code — new Comment intake: the single prior comment (6564426242, @github-actions[bot], round-1 BLOCK on SHA Blocks
Non-blocking note (rulebook gap, not a finding)
Verdict: BLOCK ollama-cloud/deepseek-v4.1-flash — action-review. |
…p (round-2 review fix) Round 2 found the dangerous direction this verb exists to refuse, in the verb itself: a module classified BUMP at rung 1 and ADOPTED was skipped at rung 2 by the `verdicts` map, so when its newest version fixed the named symbol but the composed set STILL failed, nothing recorded it — `edits` stayed empty and the report claimed "advance: CLEAN — N module(s) need only a pin bump" about a module that provably does not compile. The re-failing module is now recorded as an EDIT carrying the failure that proves it, so the report refuses the advance instead of blessing it. TestRunPinClosure_AdoptedButStillFailingIsAnEDIT is the guard: a fixture whose "newest version" fixes the named symbol and introduces a different breakage must FAIL the verb, must not print `advance: CLEAN`, and must classify the module EDIT. Reverting the fix makes it fail on exactly those two assertions, so the test bites. Two smaller corrections in the same round: - firstLines' doc comment had been left above workspaceGo (a comment describing a different function than the one it precedes): moved back onto firstLines. - goTmpDir hands the toolchain a shared GOTMPDIR but never created it, so a fresh host could fail its own temp-file creation; the dir is now made, falling back to the toolchain's default when it cannot be. Gate on this tree: gofmt clean; go build ./... rc=0; go vet ./... rc=0; go test ./... rc=0; charly candy params plugin-task --check rc=0; charly box validate rc=0 (0 warnings); scripts/gate-task.sh PASS on a fresh charly build carrying this candy. Agent: plugins Assisted-by: DeepSeek Harness deepseek-v4.1-flash (analysed on a live system)
Review — BLOCKHead SHA: Change class: code — new Security screen: PASS (no T1–T4 finding: no secrets, no egress, no guardrail weakening; the gate change only ADDS a fixture and updates the count). Comment intake:
Blocks
Non-blocking note: the tier Verdict: BLOCK ollama-cloud/deepseek-v4.1-flash — action-review. |
Review — PASSHead SHA: Security screen: PASS — no T1–T4 finding (no secrets/egress, no guardrail weakening; the gate change only adds a fixture and updates the five→six count). Comment intake (all re-derived against the current body/diff):
Checklist: All applicable rules PASS or NA. Placement: CORRECT — a new verb capability lands in Non-blocking notes (rulebook gaps, not findings):
Verdict: PASS ollama-cloud/deepseek-v4.1-flash — action-review. |
…gate I did not run locally (round-4)
The repo's own `go` job was RED on the round-3 head, and it was red on my change, not on the
environment: golangci-lint v2.13.2 reports four issues in pin_closure.go.
- `renderClosureReport` used `b.WriteString(fmt.Sprintf(...))` three times: staticcheck QF1012 wants
`fmt.Fprintf(&b, ...)`. Fixed at all three sites.
- `defer os.RemoveAll(workDir)` left the error unchecked (errcheck). The cleanup is best-effort by
design — the overlay is a temp dir — so the discard is now explicit:
`defer func() { _ = os.RemoveAll(workDir) }()`.
RCA, and the control that missed it: this repo's AGENTS.md lists golangci-lint as part of the local
gate (under the module's own toolchain, pinned in ci.yml), and the pre-push gate I ran was
gofmt + build + vet + test + box validate + the live gate — every one of those is GREEN on the
round-3 head while CI's lint was red, so nothing I ran could have caught it. The control is now in
place and exercised: golangci-lint v2.13.2 (the version ci.yml pins) is installed here and run as
`GOTOOLCHAIN=go$(go list -m -f '{{.GoVersion}}') golangci-lint run --path-prefix=candy/plugin-task`
— 0 issues on this tree.
Gate on this tree: golangci-lint (v2.13.2) 0 issues; gofmt clean; go build ./... rc=0; go vet ./...
rc=0; go test ./... rc=0; charly candy params plugin-task --check rc=0; charly box validate rc=0
(0 warnings); scripts/gate-task.sh PASS on a fresh charly build carrying this candy.
Agent: plugins
Assisted-by: DeepSeek Harness deepseek-v4.1-flash (fully tested and validated)
Review — PASSHead SHA: Security screen: PASS — no T1–T4 finding (no secrets/egress, no guardrail weakening; the gate change only adds a fixture and updates the five→six count). Comment intake (re-derived against current body/diff):
Checklist: All applicable rules PASS or NA. Placement: CORRECT — a new verb capability lands in Verdict: PASS ollama-cloud/deepseek-v4.1-flash — action-review. |
Summary
Adopting a pin in a multi-module repo is never one edit: each module that pins a repo also pins its siblings, so advancing one pin surfaces the next module's breakage, and the next.
verb:pin-closureanswers the whole question in one word — compile the composed set, collect EVERY failing module, classify each failure as a BUMP (a published version already carries the fix) or an EDIT (none does — that module needs its own cutover), and iterate to a fixed point (opencharly/charly#906).What lands, in
candy/plugin-task/:pin_closure.go— resolve the pair, write ago.workoverlay over the anchor module plus any already-composed dirs (they win over the pins), build, parse the failures into per-module verdicts, classify against the module the failing package belongs to, and report an EMPTY closure, a BUMP list, or a REFUSED advance naming the EDITs.schema/task.cue+params/cue_types_gen.go—#PinClosureInput(anchor,compose,module_prefix). The generated types are regenerated from the schema withcharly candy params(drift gate--checkclean) — which is also how thecue_types_gen.goconflict against currentmainwas resolved, rather than hand-merging a generated file.plugin.go/verbs.go/charly.yml— declare and route the sixth maintenance verb; the candy's capability probe now asserts all nine (three task classes + six verbs).AGENTS.md/README.md/scripts/gate-task.sh— the five→six arithmetic corrected in the same change (R5: no stale count survives), and this repo's own live gate now drives the new verb (see How tested).Round 1 review fixes, in this head — all three blocks were right, and two were product defects:
replacekey did not agree.newestModuleVersion(anchor, f.Pkg)was handed a package path whilereplaces[closureModule(f.Pkg)]keyed the replace on the 3-segment "repo root" — so for a subdir module (github.com/opencharly/org/repo/candy/plugin-x) the lookup asked the toolchain about a path that is not a module (BUMP unreachable, every failure forced to EDIT) and any replace it wrote targeted the wrong module.moduleOfnow asks the toolchain (go list -f {{.Module.Path}}) and both the lookup and the replace use that one path;closureModuleis the documented fallback and no longer claims a resolution it never did.writeOverlayhardcodedgo 1.26.4into every generated workspace — over-constraining every closure build to one toolchain (a module declaringgo 1.24was built under a workspace demanding 1.26.4) and failing on an older host. Thegoline is now the anchor's own directive (workspaceGo), with a conservative floor only when itsgo.modcannot be read.TestRunPinClosure_IteratesToAFixedPointAfterABumpbuilds a real three-module fixture (a pinned dependency whose own package does not compile, and a newest version that does), swaps only the version-lookup seam, and requires rung 1 to see the breakage, classify BUMP, adopt it, and rung 2 to compile.Round 2 review fixes, in this head — one real defect in the verb itself:
advance: CLEAN. A module classified BUMP at rung 1 and adopted was skipped at rung 2 by theverdictsmap, so when its newest version fixed the named symbol but the composed set still failed, nothing recorded it:editsstayed empty and the report claimed the pair needed "only a pin bump" about a module that provably does not compile — the false-BUMP directionclassifyClosure's own comment warns against. It is now recorded as an EDIT carrying the failure that proves it, so the report REFUSES the advance.TestRunPinClosure_AdoptedButStillFailingIsAnEDITis the guard, and reverting the fix makes it fail on exactly those two assertions.firstLines' doc comment had been left aboveworkspaceGo(a comment describing a different function than the one it precedes) — moved back ontofirstLines.goTmpDirhanded the toolchain a sharedGOTMPDIRit never created, so a fresh host could fail its own temp-file creation; the dir is now made, falling back to the toolchain's default when it cannot be.An adopted module is now replaced by its downloaded dir when the lookup resolved one (
adoptionTarget): the source is already on disk, and a version form would make the next build re-resolve it — needing the network mid-build and, for a module the registry cannot serve, reintroducing exactly the failure this verb exists to find.Also fixed from the gate's own first run: the workspace build inherited an ambient
GOFLAGS=-mod=modand failed withgo: -mod may only be set to readonly or vendor when in workspace mode—runGonow drops-modfor the workspace build and keeps every other flag (TestGoFlagsWithoutMod).Re-cut onto current
origin/main(the previous tip was 33 commits behind and main had movedverbs.go): the single content commit was re-applied on a fresh branch offorigin/main, nothing was force-pushed, and the old branch is untouched.How tested
The head this body describes:
The repo's own gates on this tree (
candy/plugin-task/, then the repo root):The BUMP / adoption / fixed-point arm, on a check that fails on the old code:
This paste is the run's own log, selected by key — and round 3's block is why it replaced the earlier fence. The previous fence quoted a
-run 'A|B|C|D'alternation and then a fifth PASS line; that alternation does NOT selectTestRunPinClosure_AdoptedButStillFailingIsANEDIT(measured on this toolchain: the alternation selected only some of its alternatives), so the line could not have come from that command, and the evidence was not faithful. The full-suite run above executes everything, so no-runselection can make this paste untrue. Round 3's finding was about the EVIDENCE and no code changed for it: the code and tests it describes were already in the head that round reviewed.Round 4: the repo's own CI
gojob was RED on the round-3 head, on this change. golangci-lint v2.13.2 reported four issues inpin_closure.go; all four are fixed in this head:renderClosureReportnow usesfmt.Fprintf(&b, ...)at all three sites, and the best-effort temp-dir cleanup discards its error explicitly.RCA, and the missed control. This repo's AGENTS.md lists golangci-lint as part of the local gate (under the module's own toolchain, version pinned in
ci.yml), and the pre-push gate I ran was gofmt + build + vet + test +charly box validate+ the live gate — every one GREEN on the round-3 head while CI's lint was red, so nothing I ran could have caught it. The control is now exercised locally with the SAME pinned version:And it tests the FIX, not the fixture: with the module-key fix reverted in
pin_closure.go(the pre-review shape,module := closureModule(f.Pkg)), the same test fails —The repository's LIVE gate, on this tree —
scripts/gate-task.sh, which clones charly atmain, builds a real binary with THIS candy compiled in, and drives the words end to end against real git/Go fixtures:The two gate fixtures drive the compiling set (whole loop → EMPTY closure) and the broken set (classification → EDIT, advance REFUSED, non-zero exit); the BUMP rung is the hermetic test above, because a BUMP needs a published newer version, which a hermetic fixture cannot have.
R7 — coverage that fails without the change. The unit tests name the loop's pieces (
parseClosureFailures,classifyClosure,renderClosureReport,goFlagsWithoutMod,moduleOf,workspaceGo,adoptionTarget), so the pre-change tree cannot build them; the gate section fails on the pre-change tree because the verb does not exist there at all; and the reverted-fix run above shows the BUMP test red on the old key.R8 — every fixture is a real git repo / real Go module set on the host; the closure build runs the real
gotoolchain and the gate's charly build is a real binary. Nothing is mocked.Rulebook compliance
/charly-internals:plugin,/charly-image:layerand/charly-internals:git-workflowloaded as this repo's AGENTS.md requires, before any action.-modflag) which is fixed at the root and pinned. No findings are left unfixed, so## Filed issuesis absent.goFlagsWithoutModis the single place the workspace env is sanitised.closureMaxRungs, the fixtures are real repos, and nothing is hardcoded to an environment (that was the round-1go 1.26.4finding).charly box validateplus the live gate prove the documented surface works with only the charly binary.AGENTS.md,README.mdandscripts/gate-task.sh; nothing is left describing five verbs.git statusclean before the re-cut; no force-push (stale branch untouched, change re-applied on a fresh branch offorigin/main); no--no-verify.-ldflags -X main.BuildCalVer=plugin-task-gate);charly box validateis clean with 0 warnings.Attribution tier
fully tested and validated, on this head's proof: the change-class gate passed on the committed tree (golangci-lint v2.13.2 0 issues, gofmt, build, vet,go test ./...120 PASS / 0 FAIL,charly candy params --check,charly box validate0 warnings, and the repo's livescripts/gate-task.shon a fresh charly build carrying this candy), and the changed path executed live — the verb ran in the gate's own fixtures, and the BUMP/adoption/iteration path ran against realgobuilds and real module trees. One seam is stubbed in the hermetic BUMP test (newestModuleVersionFn); the real version lookup is crossed live by the gate'sclosure-editfixture, which reachesgo list -m -versionsfor real.Change classification
Runtime code + schema + generated types + the repo's own gate/docs, all in the
plugin-taskplugin: a newverb:pin-closureprovider word. Change class = code, so the gate is this repo's declared one (build/vet/gofmt/test +charly box validate+scripts/gate-task.sh, the only live execution of these verbs) — all run on the committed tree and pasted above.Relates to opencharly/charly#906 (the pin-lag closure row this serves).
Assisted-by: DeepSeek Harness deepseek-v4.1-flash (fully tested and validated)