[aitrawog]

Refs opencharly/opencharly#799

Summary

Head f6e3b94dd83fe80e86ec50d3943a4a23dbadf170 — the commit this PR pushes: 5 files changed, 1038 insertions(+), 4 deletions(-).

opencharly/opencharly#799 named the gap: the contract knows what is retired, the repos know what they say, and nothing connects the two. The contract's guard (retired.ScanTree) reads the contract's OWN surfaces — Go sources, CUE schemas, protobuf schemas — because that is where the contract's names live. The org's trees have more surfaces than that: an authored charly.yml, a skill's prose, a script, a check bed. A withdrawn name in one of those is just as live, and a scan that reads three extensions cannot see it. So the contract grows the ORG-WIDE reach, in this package, under the rules its own scan already applies.

One implementation of the rules, and one token matcher. The sweep shares the contract's token rule (containsToken/tokenIndex) and its text rule (scanText) with the schema scan, so the two cannot disagree about what a text surface is, or about which occurrence matched. Nothing here re-implements a rule that already exists in this module.

A hit is not automatically a carrier. The distinction is semantic — a sentence that DENIES a withdrawn name ("There is no engine.build") and a sentence that SETS its value carry the same name and mean opposite things — so no control can guess it. retired.Record makes it committed data: a hit a record covers is a RECORD, a hit no record covers is a CARRIER, and a record that covers no hit is ITSELF a finding, so the table cannot rot into a hole that hides a live use. A record covers only the identifier it names, so a record written about one name can never excuse another.

The records TABLE lives with the org, not here. A record names a file in a repository this module does not own, and this tree carries no records at all — its guard reads its own surfaces directly. Only the RULE for what makes a hit a carrier lives here, once.

Where the executable lives, and why it is here and not in .github. The shape chosen for this leg was "one sweep in .github, run over every repo". The ARM lives there; the RULE and the executable live in the contract, and .github invokes this module's command at a pinned tag. Two reasons, both measured rather than preferred: .github has no Go toolchain of its own, so a Go-structural scan — the only scan that can tell a Go comment from a Go literal — would have to be re-implemented there in another language, which is a second implementation of the same rules (R2); and the org's arm already runs in every TARGET repo's context, so a command that ships with the contract is reached by one line (go run github.com/opencharly/spec/retired/cmd/retired-sweep@<tag>) instead of a per-repo matrix. The three main packages the module already carried (internal/wiregen, internal/schemagen, internal/cuesetup) are developer tools under internal/; this one is deliberately PUBLIC — it is the module's SDK surface, and the org's arm must run it from another repo at a pinned tag.

file	change
retired/sweep.go	the org-wide sweep: every text surface of a tree, under the rules the contract's own scan applies, with the coverage stated rather than claimed
retired/records.go	the rule for what makes a hit a carrier: Record, LoadRecords, ValidateRecords, Classify
retired/cmd/retired-sweep/main.go	the executable: the sweep over a tree, judged against a records table, printing the coverage and the verdict
retired/sweep_test.go	the discrimination cases — one per rule, each of which can fail
retired/scan.go	the shared text rule (scanText) extracted out of scanTextFile, so the schema scan and the org-wide sweep cannot disagree

How tested

The head this body describes

$ git rev-parse HEAD
f6e3b94dd83fe80e86ec50d3943a4a23dbadf170
$ git merge-base origin/main HEAD
f316bef7a1432e3050fa094ec392f95286f81c6d
$ git rev-parse origin/main
f316bef7a1432e3050fa094ec392f95286f81c6d
$ git diff --shortstat f316bef7a1432e3050fa094ec392f95286f81c6d HEAD
 5 files changed, 1038 insertions(+), 4 deletions(-)
$ git status --short --untracked-files=no; echo "exit=$?"
exit=0

The change-class gate — this repo's own committed module CI, on the committed tree. The diff adds Go source to a contract module, so the change class is contract / library module and the gate that owns it is this repo's own .github/workflows/ci.yml — go build ./..., go test ./..., gofmt -l ., golangci-lint run ./... — run here on the committed tree, with the CI's pinned linter version (v2.13.2) and the toolchain that linter can read (go1.26.8).

$ gofmt -l . ; echo "gofmt exit=$?"
gofmt exit=0
$ GOWORK=off go build ./... ; echo "build exit=$?"
build exit=0
$ GOWORK=off go vet ./retired/... ; echo "vet exit=$?"
vet exit=0
$ GOWORK=off go test ./... -count=1 ; echo "test exit=$?"
ok  	github.com/opencharly/spec/cache	0.133s
ok  	github.com/opencharly/spec/calver	0.011s
...
ok  	github.com/opencharly/spec/retired	0.109s
...
ok  	github.com/opencharly/spec/spec	1.197s
...
test exit=0
$ GOTOOLCHAIN=go1.26.8 golangci-lint run ./... ; echo "lint exit=$?"
0 issues.
lint exit=0

The "no bed venue" premise, proved with executed output. The class's gate is the module suite BECAUSE no bed can exist here, and that premise is a negative claim, so it is proved rather than asserted: the anchored grep for a disposable: key finds nothing, and the root manifest declares no box:, deploy: or check: node.

$ git grep -nE '^[[:space:]]*disposable:' -- '*.yml' '*.yaml' ; echo "exit=$?"
exit=1
$ grep -nE '^[a-zA-Z_]+:' charly.yml
21:repo: github.com/opencharly/spec

The changed path executed live, over the org's real trees. The sweep was built from this committed tree and run over 17 real repository checkouts: 3760 files read, every repo's coverage stated, every verdict PASS. Ten hits were found across 6 repos, and all ten are RECORDS — a comment that quotes a pre-rename diagnostic, prose that DENIES the name, and the plugin's own retirement registry, which answers the retired key BY NAME. The table that classifies them is the org's and lives with the arm; this PR ships the RULE that reads it.

$ go build -o /tmp/pb799/retired-sweep-bin ./retired/cmd/retired-sweep
$ for r in charly docs marketplace plugin-box plugin-migrate plugin-settings; do
    /tmp/pb799/retired-sweep-bin --tree "$r" --repo "opencharly/$r" --records /tmp/pb799/retired-records.json
  done
retired-sweep: opencharly/charly: 168 file(s) read (104 go, 64 text); skipped 216 test file(s), 1 history file(s), 1 binary file(s), 0 nested repo(s), 0 symlink(s), 3 dependency dir(s), 0 excluded path(s)
PASS    no withdrawn name is live in the tree
retired-sweep: opencharly/docs: 1779 file(s) read (0 go, 1779 text); skipped 0 test file(s), 1 history file(s), 21 binary file(s), 0 nested repo(s), 0 symlink(s), 1 dependency dir(s), 0 excluded path(s)
PASS    no withdrawn name is live in the tree
retired-sweep: opencharly/marketplace: 537 file(s) read (1 go, 536 text); skipped 0 test file(s), 1 history file(s), 0 binary file(s), 0 nested repo(s), 0 symlink(s), 2 dependency dir(s), 0 excluded path(s)
PASS    no withdrawn name is live in the tree
retired-sweep: opencharly/plugin-box: 34 file(s) read (15 go, 19 text); skipped 20 test file(s), 1 history file(s), 0 binary file(s), 0 nested repo(s), 0 symlink(s), 1 dependency dir(s), 0 excluded path(s)
PASS    no withdrawn name is live in the tree
retired-sweep: opencharly/plugin-migrate: 29 file(s) read (17 go, 12 text); skipped 12 test file(s), 1 history file(s), 0 binary file(s), 0 nested repo(s), 0 symlink(s), 2 dependency dir(s), 0 excluded path(s)
PASS    no withdrawn name is live in the tree
retired-sweep: opencharly/plugin-settings: 15 file(s) read (5 go, 10 text); skipped 3 test file(s), 1 history file(s), 0 binary file(s), 0 nested repo(s), 0 symlink(s), 1 dependency dir(s), 0 excluded path(s)
PASS    no withdrawn name is live in the tree

The other eleven repos — opencharly, sdk, spec, plugin-build, plugin-check, plugin-clean, plugin-deploy-pod, plugin-pod, plugin-substrate, plugin-vm, layer-container-nesting — carry no hit at all and PASS with no table.

The control can go RED — the fail-without instrument. A gate that cannot fail is not a proof, so the carrier case was planted and the sweep run against it: a live use of a withdrawn key in an authored charly.yml is a CARRIER, and the command exits 1.

$ printf 'defaults:\n  podman_jobs: 4\n' > /tmp/pb799/planted/candy/plugin-x/charly.yml
$ /tmp/pb799/retired-sweep-bin --tree /tmp/pb799/planted --repo opencharly/plugin-x ; echo "exit=$?"
retired-sweep: opencharly/plugin-x: 1 file(s) read (0 go, 1 text); skipped 0 test file(s), 0 history file(s), 0 binary file(s), 0 nested repo(s), 0 symlink(s), 1 dependency dir(s), 0 excluded path(s)
CARRIER candy/plugin-x/charly.yml:2: the withdrawn config-key podman_jobs is present (named in the source text)
FAIL    1 carrier(s): a withdrawn name is live in the tree
exit=1

The same tree under the contract's PRE-EXISTING scan is CLEAN — ScanTree reads three extensions and the authored yaml is not one of them — which is why the org-wide reach cannot be built on it.

The discrimination cases execute the new code, and each of them can fail. The suite builds a tree per rule so that one rule decides the verdict: a Go comment is not a surface while a Go literal is; a test file holds the assertions OF ABSENCE; a CHANGELOG is history; a nested repo is a repo of its own; a SYMLINK is another path's content, and a DANGLING one does not fail the run; a scoped name is swept only where its scope is declared; a record covers only the identifier it names, and only while its text is on the hit's line; a record that covers no hit is itself a finding. The case that keeps the suite from being vacuous is TestTheSweepReadsEveryTextSurface: it asserts that the contract's PRE-EXISTING scan finds NOTHING in its tree, so a control that used the old scan would read the tree as clean.

The two findings this change introduced, fixed in it. R1 is not a pass with warnings:

• golangci-lint (errcheck): defer file.Close() ignored the error → defer func() { _ = file.Close() }().
• golangci-lint (prealloc): namesOf grew a slice without its capacity → make([]string, 0, len(findings)).

Both were introduced by this change, so both are fixed in it, and the lint re-run is 0 issues.

One anomaly found by the live run, RCA'd and fixed before this push. The first org-wide run aborted: retired-sweep: sweeping the tree: reading …/.dsh/skills/a11y-tools: open …: no such file or directory. RCA — mechanism: filepath.WalkDir hands a symlink entry to the callback without following it, the callback opened the link, the unreadable path ended the whole run; blast radius: one dangling link in one repo would have failed the arm for EVERY repo, and the umbrella alone carries 361 such links; missed control: the sweep had no rule for what a SYMLINK is. The root fix is the umbrella's own rule, adopted here — a symlink is another path's content, so it is not read, it is counted in the coverage, and it never fails the run. The case that proves it (TestTheSweepSkipsSymlinks) has two halves: the link pointing inside the tree proves the content is not judged, and the dangling link proves the run survives it.

Rulebook compliance

• R0 — every skill the dispatcher selects for this leg was read before the first edit: git-workflow, root-cause-analyzer, strict-policy, go, go-quality, check.
• R1 — the dangling-symlink abort and the two lint findings each got a fresh RCA and were fixed here; the sweep's own findings over the 17 repos are ten RECORDS and zero CARRIERS, and the gate is green with zero warnings.
• R2 — one implementation: the sweep reuses the contract's token rule and its text rule (scanText) and adds no second matcher; the org's arm will call this one command rather than re-implement the rules in another language.
• R3 — no workaround: the dangling link is fixed at the walk, not retried, ignored or suppressed.
• R4 — the product comes first: the rule and the executable land here, and the prose that describes the arm lands with the arm in .github.
• R5 — no legacy path survives: scanTextFile now delegates to the shared scanText, so there is one text rule and no copy of it.
• R6 — git status was read before every action; the commit is on feat/retired-sweep in this session's worktree; no amend, no force-push, no bypass.
• R7 — the coverage fails without the change: TestTheSweepReadsEveryTextSurface asserts the pre-existing scan finds nothing in its tree, and the planted carrier exits 1.
• R8 — nothing here crosses a live service; the changed path ran live over 17 real checkouts.
• R9 — no binary ships: the command was built from the committed tree with the repo's own toolchain.
• R10 — the change-class gate (this repo's committed module CI) is green on the committed tree, with zero warnings.

Part II — the change lands in the repo that OWNS the rule (the contract), never inside a submodule (1); it is committed in this session's worktree (2); the contract's own AGENTS.md was read first (4); the org's records TABLE is not here — it belongs to the org's central repo, beside the arm that reads it (5); the leg is claimed on opencharly/opencharly#799 (6, 11); no harness configuration is touched (5).

Change classification

Contract / library module. The diff adds Go source to github.com/opencharly/spec, a module with no disposable: true entity and therefore no bed venue — proved above with executed output, not asserted.

• Pre-flight: none beyond the suite.
• The R10 gate: this repo's own committed module CI (.github/workflows/ci.yml: go build ./... + go test ./... + gofmt -l . + golangci-lint run ./...) GREEN on the PR head, run above on the committed tree. The suite genuinely EXECUTES the changed paths — go test ./retired/... covers the sweep and the records rule, and every case in it is a discrimination that can fail.
• Explicitly not required: a charly check run bed — none can exist in this module; the consumer boundary bed belongs to the consumer PR (the .github arm), and the org's live reach is exercised by the 17-repo run above.
• Tier: fully tested and validated.

Filed issues

None. Every finding this change surfaced — the dangling-symlink abort and the two lint findings — was introduced by this change and is fixed in it. The remaining half of #799 (the arm that reaches the org's other ~400 repos) is a change to a different repo and lands in opencharly/.github under the same umbrella issue, which stays OPEN.

Agent: charly
Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)

[github-actions]

Review — PASS

I reviewed all five changed files line by line against the body, the commit (f6e3b94, single commit matching head), and the (empty) comment thread. No designated maintainers are configured, so no comment counts as a sign-off; the change is judged on its own evidence.

What holds up

• File counts / shortstat are honest. 211 (main.go) + 186 (records.go) + 12 (scan.go) + 258 (sweep.go) + 371 (sweep_test.go) = 1038 insertions, and scan.go's -4 deletions = 4. Matches the body's 1038 insertions(+), 4 deletions(-) exactly.
• The scan.go refactor is a correct extraction. scanTextFile now delegates to scanText(string(data), rel, ids), and the loop (including the id.Scoped() skip and the "named in the source text" detail) is preserved verbatim inside scanText. The parameter order (text, rel, ids) is used consistently at both call sites (scan.go and sweep.go). One text rule, no duplication — R2/R5 hold.
• The sweep rules are each exercised by a discriminating test. TestTheSweepReadsEveryTextSurface includes the fail-without instrument (ScanTree finds nothing in the yaml-only tree). Go-structural vs. text, test-file skip, history skip, list exclusion, scope honoring, binary/nested skip, and symlink skip (both the intra-tree link and the dangling link) each have a case that fails if the rule regresses.
• The records rule is well-tested. TestTheRecordsClassifyTheHits covers cover/no-cover, wrong-identifier, wrong-Contains, wrong-repo, and stale. ValidateRecords rejects invented names, missing Why, and missing repo. recordCovers correctly gates on repo + file + identifier + line text.
• The two lint fixes claimed in R1 are present: defer func() { _ = file.Close() }() and make([]string, 0, len(findings)).
• The symlink handling is correct and the RCA is reflected in code: symlink entries are counted (SkippedSymlinks) and returned before any open, so a dangling link cannot abort the walk — matching the narrative.
• Security/scope: no secrets, no shell interpolation, no injection surface; the command reads files and prints derived text. Coherent, single-purpose change with no leftover legacy path.

Non-blocking observations (do not gate)

1. Body transcript vs. narrative (retired/cmd/retired-sweep/main.go / "How tested"). The narrative states "Ten hits … all ten are RECORDS," but the pasted 6-repo transcript shows only the coverage line + PASS, with no RECORD lines — which printVerdict would emit for each covered hit. The most likely explanation is that the RECORD lines were trimmed from the paste (a real clean run with 0 hits and a non-empty table would instead show STALE → FAIL, which it does not). The underlying claim is consistent with the code path, but the pasted evidence doesn't demonstrate it. Recommend pasting the full output (or a --json run) so the ten records are reproducible as shown.
2. isNestedRepo only recognizes a .git file (submodule/gitlink), not a .git directory. That matches the org's submodule layout and the test, but the doc comment's "or a checkout inside the tree" overstates it — a full nested clone (.git dir) would be swept. Minor doc/code gap, not exercised by the org's use case.
3. --exclude only filters files, not directories (the dir branch never consults excluded). The documented uses (the published list and the records table) are files, so this is fine today.

None of these affect the correctness of the tested paths, the security posture, or the honesty of the verifiable code claims. The change ships meaningful, fail-without coverage and a single implementation of the rules.

Verdict: PASS

---

ollama-cloud/deepseek-v4.1-flash — action-review.

View action run