Skip to content

compat: test against what the polyrepos actually shipped - #18

Open
Peeja wants to merge 26 commits into
mainfrom
claude/compat-polyrepo
Open

Peeja wants to merge 26 commits into
mainfrom
claude/compat-polyrepo

Conversation

@Peeja

@Peeja Peeja commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Written by Claude.

Checks a reviewer can merge without waiting for

At this head (344c764a): everything except ci / guards and ci / unit smelt.

c0489ce2 was green on e2e 35890887107 and itest 35890887140. Every push since has touched seven files, and smelt/pkg/ is not among them:

$ git diff --name-only c0489ce2 344c764a
.github/scripts/check-compat-baselines.sh
.github/scripts/compat-baselines.sh   (renamed from compat-window.sh)
.github/workflows/ci.yml
.github/workflows/compat-refresh.yml
.github/workflows/compat.yml
AGENTS.md
smelt/tests/compat/compat_test.go

$ git diff --stat c0489ce2 344c764a -- smelt/pkg/
(no output)
  • e2e and all four itest legs reach smelt only through smelt/pkg/{stack,workspace}, which is what has not changed. smelt/tests/compat cannot be in their closure: go list -tags compat -f '{{.GoFiles}}' ./tests/compat/ prints [], so it has no non-test Go files and nothing can import it.
  • images ×8 and the nine non-smelt unit legs never reach smelt at all — GOWORK=off go list -deps -test ./... | grep -c forge/smelt is 0 for every module except smelt itself (24).
  • AGENTS.md is read by nothing. Grepped unrestricted: the only hits outside markdown are five prose mentions inside comments.
  • compat's own two jobs skip on a non-release branch, by design, and do at this head.

What must be waited for: ci / guards, because ci.yml's only change is one step added to that job; and ci / unit smelt, whose vet tagged suites step is the only thing that compiles compat_test.go.

What it costs if that is wrong: the ignored checks still run, and a red one after merge leaves main red.


What this does

ci, itest and e2e boot this commit against itself — a development-time guarantee. Production is not HEAD against HEAD: we run sprue and hilt, third-party operators run piri and ingot on their own schedule. TestRollingUpgrade boots the whole fleet at a baseline set, then replaces one service with HEAD in place.

The images are the polyrepos', ghcr.io/fil-forge/<pkg> — what ships to the network today, which also breaks the circularity that stopped this before: nothing here has to be released first. The baseline set comes from the registry, not git tags, because the two disagree in both directions.

The gate was vacuous, and this is the fix

compat-window.sh set pinnable=true when any service could be pinned. TestRollingUpgrade needed every service pinned. Five of the eight publish no version-tagged image. So the job ran, the test skipped, go test exited 0, and the check went green in 0.126s having booted nothing — two conditions differing by one quantifier, one in YAML and one in Go, with a green check in the gap. Visible on run 35886032906.

A git tag is not a release here — the full state, per service

Three things get called "tagged", and they disagree:

service version.json upstream git tag ghcr image X.Y.Z
piri v0.2.4 v0.2.4 (2026-04-01) 0.2.4 ✔
ingot v0.0.0 v0.0.0 (2026-07-13) 0.0.0 ✔
indexing-service v1.13.4 v1.13.4 1.13.4 ✔
hilt v0.0.0 v0.0.0 (2026-07-10) —
swarf v0.0.0 v0.0.0 (2026-07-16) —
sprue v0.0.6 none —
delegator v0.1.1 none —
piri-signing-service v0.1.0 none —

hilt and swarf are tagged v0.0.0 in git and published nothing under it. Those tags mark the moment the release plumbing landed, not a release: hilt's is on the very commit that added version.json; swarf's is on its root commit, six weeks before it had one. The five without an image publish only main, main-dev, sha-<sha> and sha-<sha>-dev.

Resolving git tag → commit → sha-<short> closes one of the five and no more — hilt's tagged commit has sha-6758d7b, swarf's has nothing, and the other three have no tag to resolve. That is why the fallback is :main.

What the :main fallback is worth, and what it is not

The five now take :main, resolved to its index digest — never per-architecture (AGENTS.md rule 2; all eight are 4-entry OCI indexes). The objection the skip was defending against is answered where it arises: the run still faces whatever :main is when it asks, so the answer stays fresh, and the digest is in the log, the output and the job summary, so a red is reproducible afterwards.

It does not buy skew for those five. :main tracks upstream's main, which these prefixes are resynced against — measured, the tree is 1 commit behind for hilt, 1 for piri, 6 for ingot, 0 for the rest. That is the right outcome rather than a compromise: we deploy those five ourselves and can upgrade them together. The skew this suite is about comes from piri and ingot, and both have real releases.

It is also not this tree, so upstream drift can redden a run. Small today; it grows between resyncs. Said in the warning rather than hidden. It is scaffolding — once every service releases, the fallback is unused and the warning stops firing on its own.

There is now no skip path at all
  • The script produces a complete baseline set or fails.
  • compat.yml's job condition is gone — that was the "any service" half of the mismatch.
  • The Go side fails if every baseline is floating. A release is matched positively on newest_version_from's own pattern, so main, main-dev, latest and sha-abc1234 all count as floating; the complement of "is a digest" counted them as releases, and one hand-set COMPAT_BASELINE_PIRI=main defeated the guard.
  • A workspace that cannot be detected is fatal too. compat.yml asserts go env GOWORK is neither empty nor the literal off before running.

Dropped along the way: pinnable, the windows output and the window dispatch input, all of which lost their last reader when TestPinnedPeer was cut.

Where it runs

On a release pull request and nowhere else on its own, plus on demand. The gate is a job condition, not a branches: filter, because branches: matches the base branch and what selects a release is the head branch.

The answer goes stale without the code changing, because the pinned side is read live. compat-refresh.yml (new) lists the open release/* pull requests daily and dispatches compat.yml at each one's branch — a dispatched run records its check runs against the branch head, which is the pull request's head, so the fresh answer lands where the merge decision is made (measured on run 35760677003, whose 11 check runs all landed on the dispatched commit). Nothing runs while no release pull request is open; deliberate, since the earlier signal had nobody to act on it.

What the run found, and the decision it raises

The gate no longer skips, so it got far enough to fail. Run 35899900855: 207s of actual stack against 0.126s of nothing. Both subtests failed on the release-pinned peer, and neither failure is about wire compatibility:

upgrade_piri   …-upgrade_piri-ingot-1 is unhealthy
               ingot: invalid config: - root_access and root_secret are required
upgrade_ingot  …-upgrade_ingot-piri-0-1 is unhealthy
               FATAL cli/root.go:28  unknown flag: --plc-directory

smelt boots baselines with HEAD's entrypoints and config templates, and that surface has moved. All five floating baselines pulled and booted — the :main fallback is not what failed.

The suite cannot pin a baseline older than smelt's own launch contract, and both releases predate it:

service harness usable from only release gap
piri 2026-07-22 (45063e56) 0.2.4, 2026-04-01 ~16 weeks too old
ingot 2026-09-14 (54bed17f) 0.0.0, 2026-07-13 ~9 weeks too old

Neither failure involves the other service: ingot fails parsing its own config before any peer exists, piri rejects a flag in its own entrypoint. It is not a dependency cycle — it is that smelt at HEAD supplies the launch surface for every service and applies it to whatever binary is pinned. Any launch-config change has this effect, and it recurs: after fresh releases, the next flag added to piri init breaks the new baseline too. The constraint is one the field does not have — an operator upgrading gets the binary and its config together.

So the gate gives a true red and cannot give a meaningful green. A red means do not release, however it went red; the two kinds differ in what to fix, not in whether to proceed. RELEASE.md carries that and how to tell them apart. Nothing is blocked today: no release pull request is open.

What a reviewer should look at

The service name is not the package name, in three of nine — in both directions

Measured at the token endpoint: sprue/indexing-service/piri-signing-service answer 200; upload/indexer/signing-service answer 403. So both the image reference and the COMPAT_BASELINE_<KEY> env key go through workspace.ModuleDir — the same mapping that decides what gets built, so the two cannot disagree. Keying the env on the service name is how compat.yml came to set COMPAT_BASELINE_SPRUE for a test reading COMPAT_BASELINE_UPLOAD.

Nothing hand-maintains a list of services, on either side

Three hand-written lists went stale in this pull request's own lifetime. So: the fleet comes from workspace.Detect(), and a workspace service with no pin-table entry is a t.Fatalf; the script's list comes from publishedImages in smelt/pkg/stack/options.go, and an options.go the derivation does not match is an exit 2, not an empty fleet reported as success; the workflow carries one JSON output rather than one outputs: entry per service; the refresher's list comes from gh pr list. ci.yml's tag list stays hand-written on purpose — a suite is added about never, so a guard deriving it would cost more than it saves.

"No releases" and "could not ask" are different answers, and a guard now proves it

An earlier revision wrapped the registry read in || true, so a network failure printed "publishes no version-tagged image" for every service and the run went green. Only a 403 at the token endpoint is treated as a real answer, and it is fatal too.

Every interesting thing this script does is a refusal, and a healthy registry takes the success branch every time — so none of them ran in CI. check-compat-baselines.sh (new, in guards) drives it against a stub registry, because a guard that reaches the network goes red when GitHub has a bad afternoon and could not reproduce a 500 at the token endpoint even then:

  ok    1 release baseline: hyphenated key, the version and the JSON, no warning
  ok    2 floating baseline: resolves :main to a digest and warns
  ok    3 newest release wins, sorted numerically
  ok    4 no such package: exits 1 naming the service
  ok    5 token endpoint error: exits 1, naming the status
  ok    5b tags/list error: exits 1 rather than demoting to floating
  ok    6 no release and no :main: exits 1
  ok    7 manifest without a digest header: exits 1
  ok    8 truncated digest: exits 1
  ok    9 no derivable service list: exits 2
  ok    10 pagination: follows rel=next and sorts across pages
  ok    11 job summary: the table, the digest in the row, and each closing note
  ok    12 an argument is refused, not ignored
  ok    13a manifest non-200: exits 1, naming the status
  ok    13b token refused at digest_for: exits 1, naming the step

Each negative test matches its own message, because every refusal exits 1 and that is the only thing separating them. Test 5 is why that rule exists: it first matched the downstream ::error:: and so passed with the arm it is named for deleted.

Twenty mutations, each caught by exactly the tests its entry names, control clean. Two are worth naming: a lexicographic sort picks the older release and nothing notices (1.10.0 sorts below 1.2.3 as a string), and tags/list's non-200 check is the one that fails open — a rate-limited service is silently demoted from a release baseline to a floating one, the run stays green, and that partial demotion is invisible to the Go side's all-floating check too.

The header says what the file cannot catch, rather than "each guard": the transport-failure and body-is-not-JSON arms have no fixture, and a | head -n 1 regression on newest_version_from is load-dependent, so a pass proves nothing about it.

The one thing imageFor can get wrong that nothing else catches for forty minutes

A digest joins with @, a version with :. Both arrive through the same variable, and joining a digest with : asks for a tag nothing published — surfacing as a container that never becomes healthy. TestImageFor covers it in six cases, runs before the stack, needs no Docker.

Verification

What was run, and what checked out
shellcheck .github/scripts/{compat-baselines,check-compat-baselines}.sh   clean
all seven .github/scripts/check-*.sh                                      exit 0
gofmt -s -l .                                                             empty
GOWORK=off go vet -tags e2e,compat ./...         (smelt)                  ok
go test -tags compat -run TestImageFor           (6 subtests)             pass
twenty guard mutations                                                    each caught
25 guard runs under six-way CPU load                                      0 failed
all seven workflows parse

The script against the real registry, end to end:

base_PIRI=0.2.4            base_INGOT=0.0.0            base_INDEXING_SERVICE=1.13.4
base_HILT=sha256:a71a8bdc…            base_SPRUE=sha256:5dda5c78…
base_DELEGATOR=sha256:30b17579…       base_PIRI_SIGNING_SERVICE=sha256:b7ef5f0e…
base_SWARF=sha256:b090ceb7…
::warning::Compat baseline is a floating :main for hilt sprue delegator
piri-signing-service swarf. …

Both refusals, in both directions:

$ COMPAT_BASELINE_PIRI=main go test -tags compat -run TestRollingUpgrade
--- FAIL  every baseline is a floating :main … a green here would assert nothing

$ GOWORK=off go test -tags compat -run TestRollingUpgrade
--- FAIL  no active go.work, so there is no fleet to hold old

And the dispatched run booted the stack for 207s: 5 of 8 baselines classified floating, exactly one service built from local source per subtest, everyone else on their resolved image.

Review rounds

Seven on the original shape, then a review found the vacuous gate, which no round had. Six more on the fix: 10, 6, 6, 3, 3, 1. They stopped when one came back with no code defect — a condition stated in the previous round's comment rather than after the result. The rounds are in the thread; the summary:

Rounds 1–3 found real defects, including one in shipping code that nothing else would have caught: newest=$(… | head -n 1) lets head close the pipe under set -o pipefail, so the script aborts with rc=1 and an empty stdout. Never idle; 1–55% of runs under load. It needs more than one version-shaped tag to fire, which no fleet service publishes today — so it has never fired in a real run, and it arrives with the second release of any service.

Rounds 4–6 found no code defect — eleven findings, every one a claim a file made about itself. The pattern is the part worth keeping: in five of the six rounds, the commit fixing the previous round introduced a new false claim, always about a measurement or an observation and never about code. Each was caught by applying the mutation or re-running the measurement; none by reading.

Still open, deliberately

  • No release of piri or ingot can be launched by this harness, so the gate cannot go green on a real baseline set today. Two independent releases clear it — in any order, neither blocking the other.
  • smelt owns the launch surface for every service, at HEAD. That makes any launch-config change break the current baseline, permanently and not just at bootstrap. The seam: topology and environment are genuinely shared and belong to smelt; invocation is per-version and belongs with the artifact. Phase-3-shaped, not this pull request's.
  • The baselines are a rolling set. A manifest of what is actually deployed would be better and is the eventual answer.
  • A fork pull request with a release/* head passes the gate. Not fixed: forks are not this repository's threat model, and a gate against them was removed on purpose in d4e95ea2 after it turned out not to do what its comment claimed.

The release-pull-request gate itself landed separately in #19, which is merged; this is the compat half left out of it.

✴️


🤖 Generated with Claude Code

https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

Recovers the compatibility suite from this repository's pre-fork lineage
(f26f56f, discarded when main was rebuilt from subtrees) and points its
pinned side at the polyrepos.

WHY THE POLYREPOS. They are what ships to the network today, so their images
are what "already deployed" means. Pointing at them also removes the
circularity in the original design: it read versions from THIS repository's
git tags and pulled ghcr.io/fil-forge/forge/<svc>, neither of which exists --
images.yml runs with push: false and there are no service-prefixed tags. As
written it could only run after the release-ownership decision, which is the
decision it was meant to inform.

THE VERSION SET COMES FROM THE REGISTRY, NOT FROM TAGS, because the two
disagree and the disagreement is silent. Measured:

    ghcr.io/fil-forge/piri:0.2.4   HTTP 200
    ghcr.io/fil-forge/piri:v0.2.4  HTTP 404

The polyrepos tag git `v0.2.4` and publish the image `0.2.4`. The original
formed the image reference straight from the git tag, so every pin would have
pulled nothing -- surfacing much later as a container that never became
healthy, not as "no such image". compat-window.sh reads the registry instead,
which is the question the suite actually has: what can I pin against?

WHAT RUNS TODAY, AND WHAT DOES NOT. Measured by running the resolver:

    piri   0.2.4       base + window
    ingot  0.0.0       base + window
    sprue  none        only :main, :main-dev, :sha-*
    hilt   none        only :main, :main-dev, :sha-*

So TestPinnedPeer runs -- and piri and ingot are exactly the two it pins,
being the services third parties run and therefore the ones that can lag.
TestRollingUpgrade needs a released image for all four and will SKIP until
sprue and hilt publish one.

That is a skip rather than a substitution on purpose. :main would satisfy the
type but not the question: it moves, and MAJOR_DECISIONS already records what
a moving peer costs -- a red in itest ingot that meant somebody else merged,
and an investigation to find out.

ALSO PORTED: stack.WithWorkspaceBinariesExcept, without which the workspace
binary is mounted over the pinned image and the whole suite silently becomes
HEAD-against-HEAD and always passes.

WHAT A GREEN RUN IS EVIDENCE FOR. Only the pinned side is an image; the HEAD
side is workspace binaries bind-mounted over a base image. So it speaks to
wire compatibility, not to deployability -- rule 4 still wants images for
that, and this does not provide them.

Verified: actionlint clean, shellcheck clean, seven guard scripts exit 0,
gofmt clean, `go vet -tags compat` clean, smelt builds. The resolver was run
against the live registry and produced the table above.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

@Peeja Peeja left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[From Claude:]

Not satisfied — 6 findings, two of them blocking. The registry facts the design rests on all check out: ghcr.io/fil-forge/piri:0.2.4 answers 200 and :v0.2.4 answers 404, piri and ingot publish version-shaped tags, sprue and hilt publish none. The measured claims are right and the design follows from them. What is wrong is downstream: neither test can boot a stack, and nothing in CI compiles this code, so neither of those would have surfaced on its own.

Blocking: neither test can boot — the required *_IMAGE compose variables are never set, so compose up fails before any assertion runs.

Every in-repo service's compose image: is a required interpolation, added deliberately so the stack can never silently run a published image:

$ grep -rn 'image:.*\${.*:?' smelt/systems/ smelt/pkg/generate/
smelt/systems/swarf/compose.yml:10:    image: ${SWARF_IMAGE:?must be set (stack.WithPublishedImages() for the published image); ...}
smelt/systems/signing-service/compose.yml:8:  image: ${SIGNER_IMAGE:?...}
smelt/systems/hilt/compose.yml:10,74:       image: ${HILT_IMAGE:?...}
smelt/systems/upload/compose.yml:8,50:      image: ${UPLOAD_IMAGE:?...}
smelt/systems/indexing/indexer/compose.yml:25: image: ${INDEXER_IMAGE:?...}
smelt/systems/delegator/compose.yml:8:    image: ${DELEGATOR_IMAGE:?...}
smelt/systems/ingot/compose.yml:14:       image: ${INGOT_IMAGE:?...}
smelt/pkg/generate/compose.go:70:          image := "${PIRI_IMAGE:?...}"

config.buildEnv() (smelt/pkg/stack/options.go:86) omits the variable entirely when the field is empty — if c.hiltImage != "" { env["HILT_IMAGE"] = ... } — so an unset image is an unset variable, and :? errors on unset.

TestPinnedPeer passes three options: WithPiriNodes, WithWorkspaceBinariesExcept(service), and one of WithPiriImage/WithIngotImage. That sets one of the eight. TestRollingUpgrade sets four (piri, ingot, upload, hilt), leaving INDEXER_IMAGE, DELEGATOR_IMAGE, SWARF_IMAGE, SIGNER_IMAGE unset. Neither calls WithPublishedImages() or OptionsFromEnv(), and compat.yml sets only COMPAT_* env vars.

WithWorkspaceBinariesExcept cannot cover this: workspace.RenderOverride emits volumes: only, so a workspace binary is a bind-mount on top of an image — it never supplies one.

Both other suites that boot a stack set all eight explicitly:

$ grep -n "_IMAGE" .github/workflows/e2e.yml .github/workflows/itest.yml
e2e.yml:71-78    PIRI_IMAGE, HILT_IMAGE, INGOT_IMAGE, UPLOAD_IMAGE,
itest.yml:95-102 DELEGATOR_IMAGE, SIGNER_IMAGE, SWARF_IMAGE, INDEXER_IMAGE

The compat tests are the only callers of WithWorkspaceBinaries* anywhere in the tree; the pre-existing WithWorkspaceBinaries is reachable only through OptionsFromEnv() (options.go:513), which reads the *_IMAGE vars in the same pass — which is why this has not bitten before.

Fix is stack.WithPublishedImages() first in both option lists (it never overwrites, so the pins still win). The documented example at options.go:236-240 has the same defect and would fail the same way if pasted:

s := stack.MustNewStack(t,
    stack.WithWorkspaceBinariesExcept("piri"),
    stack.WithPiriImage("ghcr.io/fil-forge/piri:0.2.4"),
)
Blocking: ci.yml has a slot built for exactly this and the PR does not fill it — smelt's tagged_suites is still e2e, so nothing compiles tests/compat.

.github/workflows/ci.yml:163-167, with its own comment stating the failure mode this PR walks into:

# Tag-gated packages are invisible to every step above, so a broken
# import path in them stays green until someone runs the suite by hand.
# Compiling them (not running them — they need Docker and a live stack)
# is what keeps that from happening.
- name: vet tagged suites
  if: matrix.tagged_suites != ''
  run: go vet -tags ${{ matrix.tagged_suites }} ./...

and the matrix entry (ci.yml:97-98) is unchanged:

- service: smelt
  tagged_suites: e2e

Measured — the new package is outside what that step reaches:

$ GOWORK=off go list -tags e2e ./tests/...
github.com/fil-forge/forge/smelt/tests/e2e

$ GOWORK=off go list -tags e2e,compat ./tests/...
github.com/fil-forge/forge/smelt/tests/compat
github.com/fil-forge/forge/smelt/tests/e2e

The one-line fix works today:

$ GOWORK=off go vet -tags e2e,compat ./...   # exit 0

This compounds with the PR's own "cannot be proven to run until it is on main": compat.yml is dispatch-only, so the suite is neither run nor compiled by anything. Finding 1 is what that buys — a suite that cannot boot, merged green.

compat-window.sh reads only GHCR's first page of 100 tags and discards the rel="next" header; ingot already has 111 tags.

GHCR paginates tags/list at 100 and says so:

$ curl -sS -D - -o /dev/null -H "Authorization: Bearer $tok" \
    https://ghcr.io/v2/fil-forge/ingot/tags/list | grep -i '^link'
link: </v2/fil-forge/ingot/tags/list?last=sha-e6de741&n=0>; rel="next"

$ curl -fsS ... 'https://ghcr.io/v2/fil-forge/ingot/tags/list?n=500' | jq '.tags|length'
111

The 11 tags the script never sees are real:

$ curl -fsS ... 'tags/list?last=sha-e6de741&n=100' | jq -c .tags
["sha-e6de741-dev","sha-4611e09-dev","sha-4611e09","sha-e9051d3","sha-e9051d3-dev",
 "sha-ae36fe6","sha-ae36fe6-dev","sha-685a6fd","sha-685a6fd-dev","sha-956fd4b","sha-956fd4b-dev"]

There is no ordering property that keeps version tags on page 1 — GHCR's order is not lexicographic:

$ curl ... 'tags/list?n=500' | python3 -c 'import json,sys; t=json.load(sys.stdin)["tags"]; print(t[:6]); print(t==sorted(t))'
['main-dev', 'sha-6cfb5df-dev', 'main', 'sha-6cfb5df', '0.0.0-amd64', '0.0.0-arm64']
False

Today 0.0.0 lands on page 1 by luck. When ingot cuts a real release it will be the most recently pushed tag, and the failure is the silent kind this script exists to prevent: either base_ingot keeps reporting a stale 0.0.0, or the version tag falls off page 1 entirely and the script says ingot publishes no version-tagged image — which tags_for cannot distinguish from the truth. Following Link: rel="next" until exhausted is the fix.

A registry failure is reported as "publishes no version-tagged image" and exits 0, so the nightly goes green having asserted nothing.

The || true at line 63 swallows every failure mode, not just the missing-package one it documents. Run against an unreachable registry (the script copied with the hostname changed, nothing else):

$ ./cw-broken.sh 2
piri publishes no version-tagged image; anything needing it will skip
base_piri=
ingot publishes no version-tagged image; anything needing it will skip
base_ingot=
sprue publishes no version-tagged image; anything needing it will skip
base_sprue=
hilt publishes no version-tagged image; anything needing it will skip
base_hilt=
pinnable=false
SCRIPT EXIT=0

Every one of those four lines is false, and the script exits 0. compat.yml:69 then evaluates if: needs.window.outputs.pinnable == 'true' to false, the compat job is skipped, and a skipped job leaves the run's conclusion green. A GHCR outage and "nobody has cut a release" produce byte-identical output.

The partial case is worse because it looks like a real run: if only piri's fetch fails, pinnable=true from ingot, COMPAT_PIRI_VERSIONS arrives empty, TestPinnedPeer/piri skips and the job passes on ingot alone.

The two cases are distinguishable at the point of failure — a missing package answers 403 on the token endpoint (measured against fil-forge/doesnotexist123), where a transport error or a 5xx does not. Distinguishing them, or asserting that at least one of piri/ingot resolved, is what separates "no releases exist" from "I could not ask".

Note also that with tags_for invoked inside || true, set -e is disabled for the whole function body, so a failed token fetch does not stop the second curl from running with an empty bearer — it only happens to reach the same empty result.

TestRollingUpgrade does not hold the fleet at the baseline: 5 of the 8 workspace-selected services still come from HEAD, contradicting two comments that say otherwise.

workspace.Detect() selects by go.work use-dir basename against workspace.Services' moduleDir. With this repo's go.work (no libforge, no guppy) that resolves to 8 smelt service names:

delegator, hilt, indexer, ingot, piri, signing-service, swarf, upload

otherThan (compat_test.go:223-232) enumerates only four:

all := []string{"piri", "ingot", "upload", "hilt"}

So for upgrade_piri the exclusion set is {ingot, upload, hilt} and delegator, indexer, signing-service, swarf — plus piri itself — are built from HEAD and mounted over their images. Three services old, five new.

That makes both of these false as written:

  • compat_test.go:166-168 — "boots every in-repo service at a released version, then replaces one with HEAD while the rest stay old"
  • compat_test.go:210 — "Building only that service keeps the rest genuinely old"

indexer matters most: assertUploadRetrieve drives guppy → sprue → piri → indexer, so the indexer is on the asserted path and is new in a test whose premise is that everything but one service is old.

Worth noting the upload/sprue split is handled correctly — otherThan uses the smelt service name upload while imageFor("sprue", …) uses the registry name, which is right, and TestPinnedPeer's WithWorkspaceBinariesExcept(service) with "piri"/"ingot" matches genuine keys. The gap is only the four-name list.

The skippable-checks block contradicts itself on unit smelt, and its grep cannot see the tag setter in ci.yml.

Most of the block re-derives correctly. The changed-file list matches git diff --name-only origin/main...HEAD exactly; compat.yml is schedule + workflow_dispatch with no push/pull_request; compat-window.sh has exactly one reader; guards runs seven named check-*.sh invocations plus gofmt and does not touch it. On the Go closure, GOWORK=off go list -deps ./... per module puts smelt/pkg/stack in smelt alone among the unit matrix entries, and in hilt/itest and ingot/itest under -test -tags itest — neither of which is in the unit matrix, so "unit * beyond smelt" holds and listing itest is right.

Two things do not hold:

The headline and the first bullet give opposite instructions. "Wait for ci / guards. Everything else is skippable." against "smelt/pkg/stack is Go that ships, so unit smelt must run." A reviewer cannot act on both. If the intent is "it will run and it will pass", that is an argument about the outcome, not about whether to wait, and the headline should name unit smelt alongside guards.

The grep offered as proof cannot find the setter it needed to find. The block runs:

$ grep -rn 'tags compat\|tags=compat' .github/ */Makefile Makefile 2>/dev/null

Neither alternative matches ci.yml:167's go vet -tags ${{ matrix.tagged_suites }} ./..., where the value is set 70 lines earlier in the matrix. The conclusion drawn — "nothing sets that tag but compat.yml" — is true only because of finding 2. A grep for tagged_suites alongside it would have turned the claim into the finding.

Verification — what was run, and what checked out.

Registry claims — all confirmed. Manifest HEADs with an anonymous pull token:

piri:0.2.4  -> 200        ingot:0.0.0  -> 200
piri:v0.2.4 -> 404        ingot:v0.0.0 -> 404
piri:latest -> 200 (same digest as 0.2.4)

Version-shaped tags by service: piri → 0.2.4; ingot → 0.0.0 (plus 0.0.0-amd64/0.0.0-arm64); sprue → none; hilt → none. sprue and hilt carry only main, main-dev, sha-*. The per-arch exclusion comment is accurate: ingot:0.0.0 is a manifest.list.v2+json and ingot:0.0.0-amd64 is a single-arch manifest.v2+json. fil-forge/ingot has exactly one git tag, v0.0.0 at 6cfb5df, so 0.0.0 is a genuine release tag rather than a moving one.

Script behaviour. ./.github/scripts/compat-window.sh 2 reproduces the PR body's output verbatim. Arguments: 1, 2, 99, and no argument all exit 0; 0, abc and -1 exit 2 with a usage message. The numeric sort is correct for multi-digit components — ["10.0.0","1.0.0","0.10.1","0.10.0","0.9.0","0.2.10","0.2.4","0.0.0"], so 0.10.0 above 0.9.0. No $GITHUB_OUTPUT corruption is reachable: values are constrained by re.fullmatch(r"[0-9]+\.[0-9]+\.[0-9]+") and joined with paste -sd,, so no newline, = or heredoc delimiter can appear, and keys are from a fixed list. A bad workflow_dispatch input exits 2 and fails the step loudly rather than writing junk.

Tools. shellcheck .github/scripts/compat-window.sh clean. actionlint .github/workflows/compat.yml clean. All seven .github/scripts/check-*.sh exit 0. gofmt -s -l . empty.

Go. GOWORK=off go vet -tags compat ./tests/compat/ exits 0. GOWORK=off go build ./... in smelt exits 0. GOWORK=off go vet -tags e2e ./... and -tags e2e,compat ./... both exit 0.

Go details asked about, all correct. workspaceExclude is nil-safe — the read at stack.go:544 is a nil-map lookup returning false, and the map is allocated only in WithWorkspaceBinariesExcept. The continue skips the right thing: workspace.BuildBinary and the bins[svc] assignment for that service only, leaving other services' builds and the explicit-binary/config passes untouched. imageFor is correct for a version that already lacks a v — strings.TrimPrefix("0.2.4", "v") is "0.2.4" — which is the shape the workflow actually feeds it, since the registry tags carry no v.

Skip paths. Beyond findings 1 and 4: pinnedVersions returns an empty slice without skipping if COMPAT_<SVC>_VERSIONS is set to punctuation only (e.g. "," survives the TrimSpace guard, then every element trims to empty), and the subtest passes having asserted nothing. Not reachable from compat.yml, which emits either a paste-joined list or no line at all, but it is the same silent-pass shape as finding 4.


Generated by Claude Code

Six findings from the first review round on #18. The two blocking ones are
the same failure seen from both ends: a suite that cannot work, and no check
that would have said so.

NEITHER TEST COULD BOOT A STACK. Every service this repository builds is a
required compose interpolation -- `${HILT_IMAGE:?...}` and seven siblings --
and config.buildEnv() omits a variable it has no value for, so compose refuses
to start and names it. TestPinnedPeer set one of eight, TestRollingUpgrade
four; neither called WithPublishedImages(). Workspace binaries do not cover
for it: they are bind mounts over an image, not an image. e2e.yml and itest.yml
both set all eight, which is why nothing had hit this before -- these tests are
the tree's only direct callers of WithWorkspaceBinaries*. The doc comment on
WithWorkspaceBinariesExcept showed the same broken example, so it is fixed too.

NOTHING IN CI COMPILED THE SUITE. ci.yml has a `vet tagged suites` step whose
own comment describes this exact failure, and smelt's matrix entry still read
`tagged_suites: e2e`. Adding `compat` is the fix; the hand-maintained list is
the cause, so the step now carries the command that enumerates the class --
every build tag in every go.work module that unlocks a package an untagged
build does not compile. Run over all twelve modules it reports smelt/e2e and
smelt/compat and nothing else, which is how the two above are now known to be
the whole set rather than the two that were noticed.

TestRollingUpgrade did not hold the fleet old. It pinned four services and
excluded four from the workspace build, but workspace.Detect() yields eight
for this go.work, so delegator, swarf, the signing service and the INDEXER all
came from HEAD -- the indexer being on the path the test asserts over. A fleet
with four unintended HEAD services in it is not a baseline. The fleet is now
whatever the workspace would build, so the list cannot drift again, and the
test needs a released image for every member. That makes the skip honest: six
of the eight publish no version-tagged image today, and it names them.

A REGISTRY OUTAGE READ AS "NO RELEASES". compat-window.sh wrapped the read in
`|| true`, so a network failure printed "publishes no version-tagged image"
four times, exited 0 with pinnable=false, and the compat job's `if:` skipped it
-- a green nightly that asserted nothing. A package the registry declines to
mint a token for still reports none, measured: that refusal is a 403 at the
token endpoint, not a 404 at tags/list. Every other failure is now fatal.

It also read one page. GHCR returns 100 tags by default and a `Link: rel=next`
the script discarded; ingot has 111, of which its first page held exactly one
version-shaped tag. Order is neither lexicographic nor version order, so
nothing keeps the next release on page one. It now follows the link, taking
only `last` from it -- GHCR's own next link carries `n=0`, which is not a page
size this script chose. Verified by forcing the page size to 5: still 111.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

@Peeja Peeja left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[From Claude:]

Round 2: not satisfied. Five findings — two blocking, three not.

Of the six from round one: (a) required image variables, (b) the CI tag list, (e) tag pagination are genuinely fixed and I could not break them. (c) the fleet and (d) the registry-failure distinction are half-fixed, and (c)'s rewrite regressed a mapping the previous revision had right. (f)'s two go list -deps tables reproduce exactly, but the body's summary table carries a derived number that is wrong.

Blocking: TestRollingUpgrade now pins three of the eight fleet members to GHCR packages that do not exist — and this push is what broke it

The fleet is workspace.Detect()'s names; imageFor uses those names as registry package names. They are not the same thing for three of the eight, and the tree already records the correct mapping in publishedImages (smelt/pkg/stack/options.go:456-465) and asserts it in TestWithPublishedImagesFillsUnsetFields.

Simulating both tests' exact option sequences against the real config (a scratch _test.go in pkg/stack, since the fields are unexported) puts the two side by side:

TestPinnedPeer, piri pinned:              TestRollingUpgrade, fleet at 1.0.0:
  INDEXER_IMAGE=…/indexing-service:main     INDEXER_IMAGE=…/indexer:1.0.0
  SIGNER_IMAGE=…/piri-signing-service:main  SIGNER_IMAGE=…/signing-service:1.0.0
  UPLOAD_IMAGE=…/sprue:main                 UPLOAD_IMAGE=…/upload:1.0.0

Those three right-hand references are not packages. Probing the token endpoint the way compat-window.sh does, where 403 is its own "no such package" signal:

sprue                  token HTTP 200     upload             token HTTP 403
indexing-service       token HTTP 200     indexer            token HTTP 403
piri-signing-service   token HTTP 200     signing-service    token HTTP 403

This is a regression, not an omission. The previous revision wrote stack.WithUploadImage(imageFor("sprue", baseline["sprue"])) — the registry name, correctly — and the rewrite to pinFor[svc](imageFor(svc, baseline[svc])) replaced it with "upload", then added indexer and signing-service the same way.

It is invisible today only because the test skips, and this PR's own commit message says how it would present: "a container that never became healthy, not as 'no such image'". pinFor already needs a per-service table; the registry name belongs in the same one.

Blocking: compat.yml still feeds COMPAT_BASELINE_SPRUE, which nothing reads any more, so TestRollingUpgrade can never run

envSuffix derives the variable from the fleet name, and the fleet name is upload:

v := strings.TrimSpace(os.Getenv("COMPAT_BASELINE_" + envSuffix(svc)))   // → COMPAT_BASELINE_UPLOAD

compat.yml:99 exports COMPAT_BASELINE_SPRUE. Under the previous revision (strings.ToUpper("sprue")) that matched; after this push nothing reads it. So the wiring is dead, and it would stay dead on the day sprue cuts a release — the test would still skip naming upload.

Wider than the one name: compat-window.sh:118 resolves piri ingot sprue hilt, and the window job declares four base_* outputs, while the test now requires a baseline for all eight fleet members. delegator, swarf, indexing-service and piri-signing-service are never resolved at all. The test's doc comment says the skip lasts "until they cut releases"; as wired, cutting releases would change nothing.

The fix is the same table finding 1 needs — smelt service name ↔ registry package name — used by the resolver loop and the env: block as well as by imageFor.

compat-window.sh still answers "no such package" for HTTP 401 and 404 at the token endpoint, where only 403 was measured

The header documents one measurement — "a package that does not exist is refused HERE, at the token endpoint, with 403 DENIED" — and the case list acts on three codes:

401|403|404)
  return 10 ;;

Reproduced against a local fake registry, GHCR_API pointed at it:

=== MODE=token403     exit=0     piri: no such package … pinnable=false
=== MODE=token401     exit=0     piri: no such package … pinnable=false
=== MODE=token500     exit=1     piri: token endpoint answered HTTP 500   (fatal, correct)

A 401 from a token endpoint is "authenticate", not "absent" — anonymous rate limiting, a degraded auth service, or these packages being made private all produce it, and every one of them ends pinnable=false, the compat job's if: skipping, and the nightly green having asserted nothing. That is the failure this finding was opened to close, narrowed to two codes rather than removed. A 404 at /token is the endpoint itself being wrong (a mirror or proxy in GHCR_API that does not implement it), which is also not a missing package.

Narrowing the case to 403 alone leaves 401 and 404 on the fatal branch, where the script's own measurement puts them.

"Six of the eight publish no version-tagged image" is five, and the indexer — named as one of them — has a release

The claim appears in the PR body's summary table ("Skips today — six of the eight services publish no version-tagged image"), in the commit message, and in compat_test.go:205-207, which names the indexer explicitly:

sprue and hilt publish no version-tagged images today (only :main and :sha-*),
and neither do the indexer, delegator, signing service or swarf

Running the script's own tags_for | versions_from over the eight fleet packages:

piri                   0.2.4        delegator              <none>
ingot                  0.0.0        swarf                  <none>
indexing-service       1.13.4       piri-signing-service   <none>
sprue                  <none>       hilt                   <none>

ghcr.io/fil-forge/indexing-service:1.13.4 exists and its manifest answers 200. So three of the eight have a release, not two, and five lack one, not six. The indexer is on the wrong side of the sentence — which matters beyond the count, because it is the one service with a release whose name finding 1 gets wrong.

The service under upgrade is never checked to be in the fleet, so a narrowed go.work builds nothing from HEAD and still passes

upgraded is a hand-written []string{"piri", "ingot"} while the fleet is derived. If upgraded is not in the fleet, otherThan(fleet, upgraded) returns the whole fleet, so WithWorkspaceBinariesExcept excludes every service Detect() selected and stack.go:541-547 builds none of them. The "upgraded" service then falls through to WithPublishedImages()'s :main, and the subtest compares released code to released code and passes.

Not reachable with this repository's go.work, where both are present. It is reachable with the narrowed workspace pkg/workspace itself advertises in its error text (go work init ./smelt ./libforge ./piri), which is a supported way to run this machinery.

Same silent-pass shape as the round-one finding, on the other side of the comparison — and the same remedy the fleet loop already uses: a t.Fatalf when upgraded is not in the fleet, rather than a run that quietly asserts nothing.

Verification — what was run, and what checked out

(a) required image variables — fixed. The tree marks eight interpolations required, and publishedImages fills exactly those eight:

$ grep -rno '\${[A-Z0-9_]*:?[^}]*}' --include=*.yml --include=*.go .
PIRI_IMAGE DELEGATOR_IMAGE HILT_IMAGE INDEXER_IMAGE
INGOT_IMAGE SIGNER_IMAGE SWARF_IMAGE UPLOAD_IMAGE

Order traced rather than assumed: NewStack applies options in slice order (stack.go:52-54), and WithPublishedImages writes only into an empty field (options.go:490-498), so first-then-pin is right. Simulated, buildEnv() returns 8 variables for both tests and the pin wins — PIRI_IMAGE=ghcr.io/fil-forge/piri:0.2.4, not :main. GUPPY_IMAGE is correctly absent; it keeps its :- compose default.

(b) the CI tag list — fixed, and the command works pasted. Extracted verbatim from the ci.yml comment and run from the repo root: exit 0, walks all twelve go.work modules, and prints exactly

smelt compat github.com/fil-forge/forge/smelt/tests/compat
smelt e2e    github.com/fil-forge/forge/smelt/tests/e2e

e2e,compat really is the whole set — the tree contains only three distinct constraints (//go:build !codegen ×29, e2e ×3, compat ×2). GOWORK=off go vet -tags e2e,compat ./... in smelt is clean, which is what the step runs. One limit worth knowing when it is next used: the grep's class [a-z0-9_, ]+ cannot match a constraint expression, so a suite tagged //go:build e2e && docker would be missed silently. Nothing in the tree is written that way today.

(c) the fleet — what is right. workspace.Detect() run against this go.work returns the eight claimed (delegator hilt indexer ingot piri signing-service swarf upload); guppy is excluded because no guppy or libforge directory is in the use-list, and were it selected it would not trip the t.Fatalf — pinFor has an entry. With upgraded in the fleet, otherThan plus stack.go:541-547 leaves exactly that one service built from HEAD.

(d) the fatal paths — verified fatal. Against a local fake registry: unreachable host, token HTTP 500, token 200 carrying no token field, tags/list 500, tags/list serving non-JSON, and a truncated body (curl 18) all exit 1 with the ::error:: line. No temp-file leak on either the success or the failure path — the trap … RETURN plus the explicit rm -f "$hdr" cover every branch I could reach. A 200 carrying valid JSON with no tags key reports empty; I judged that acceptable rather than a finding, since it is indistinguishable from GHCR's legitimate "tags": null for a package with no tags, and there "no releases" is the true answer.

(e) pagination — fixed. Forced page size 5 against the live registry: 111 ingot tags at n=5 and 111 at n=1000, loop terminates. The real header is lowercase under HTTP/2 —

link: </v2/fil-forge/ingot/tags/list?last=0.0.0-amd64&n=5>; rel="next"

— and the sed does not anchor on the header name, so that is fine; it extracts 0.0.0-amd64. [^&>]* cannot mis-parse a Docker tag (the charset excludes &, > and anything needing percent-encoding), and the greedy .* backtracks correctly when a rel="prev" link shares the line.

(f) the skippable-checks block — both tables reproduce exactly. 0 for all nine non-smelt modules and 24 for smelt; 6 each for hilt/itest and ingot/itest. images.yml:57 does say "smelt is not: that edge is test-only". The block also enumerates every check that exists: ci has exactly two jobs (guards, unit) and images exactly the eight services named. Nothing it calls skippable is reachable from this diff.

Also run: all seven .github/scripts/check-*.sh exit 0; gofmt -s -l . empty.


Generated by Claude Code

… that do

not exist

Five findings, two blocking, and the worst was mine: round one's fix introduced
it.

THE SERVICE NAME IS NOT THE PACKAGE NAME for three of nine. Deriving the fleet
from workspace.Detect() replaced `imageFor("sprue", ...)` with
`imageFor(svc, ...)`, and svc is smelt's name. Measured at the token endpoint:
fil-forge/upload, fil-forge/indexer and fil-forge/signing-service all answer
403, while fil-forge/sprue, fil-forge/indexing-service and
fil-forge/piri-signing-service answer 200. So the rewrite would have pinned
three services to images that cannot be pulled. workspace.ModuleDir is now
exported and is the single mapping -- the same one that decides what gets
built, so the two cannot disagree -- and everything that names a service
outside smelt goes through it.

The same confusion ran the other way through the environment. envSuffix keyed
on the service name, so compat.yml set COMPAT_BASELINE_SPRUE and the test read
COMPAT_BASELINE_UPLOAD. Both sides key on the package now, which is what the
registry knows and therefore what the workflow can produce. The skip message
names the VARIABLES rather than the services, because naming the services is
what sends a reader to the variable nothing reads.

THE WORKFLOW RESOLVED FOUR SERVICES WHERE THE TEST NEEDS EIGHT, so
TestRollingUpgrade could not have run even after the missing five cut releases.
compat-window.sh derives the list from `publishedImages` in
smelt/pkg/stack/options.go -- the one table the Go side already keys on -- and
resolves all eight. The workflow carries them as two JSON outputs rather than
one `outputs:` entry per service: that hand-written list was wrong in both
available directions at once, naming a key nothing read and omitting four
services outright.

401 AND 404 WERE GUESSES IN A POSITION THAT FAILS OPEN. Only 403 was measured
as "no such package"; 401 and 404 were in the case anyway, and simulated, a 401
gave four "no version-tagged image" lines, exit 0, pinnable=false, the compat
job skipped and the nightly green having asserted nothing -- the exact failure
the `|| true` was removed to close, reintroduced by the fix for it. 403 alone
now.

"SIX OF THE EIGHT PUBLISH NO VERSION-TAGGED IMAGE" IS FIVE, and the comment
named the indexer among them: ghcr.io/fil-forge/indexing-service:1.13.4 exists.
The three that publish are piri 0.2.4, ingot 0.0.0 and the indexer at 1.13.4.

And TestRollingUpgrade never checked that the service it upgrades is in the
fleet. If it is not, otherThan excludes every member, nothing is built from
HEAD, and the subtest passes having compared released against released. Not
reachable with this go.work; reachable with a narrowed one, which
pkg/workspace's own error text tells people to make.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 22, 2026
#19's round 1 found that plan derives the runner from the pull request's own
.goreleaser.yaml, so a fork could book a macos-14 runner. I gated the build job
and wrote a comment claiming nothing else fork-controlled could get through.
Round 2 proved that wrong: plan had no condition at all, and resolve-the-version
interpolated ${{ steps.svc.outputs.dir }} into its shell. Reproduced end to end
with a directory named evil$(touch${IFS}PWNED)x and a branch to match. Fixed
twice over -- dir by env, and a fork resolves no service -- and no ${{ }}
remains inside any run: in that file, checked by parsing.

#18's round 2 found the same shape: round 1's fleet-list fix asked ghcr.io for
upload, indexer and signing-service, which answer 403; the packages are sprue,
indexing-service and piri-signing-service.

Four of the nine findings across the two pull requests are in fixes made by the
round before, which is the argument for the round and the thing worth recording
about the practice.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

@Peeja Peeja left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[From Claude:]

Round three: two findings, neither blocking. All five of round two's items hold up against the tree and against the registry, and I found no new defect in compat-window.sh, compat.yml, compat_test.go or workspace.go — the pattern round two hit (a fix introducing the next round's defect) does not repeat here. What is left is one count that is wrong by one in a doc comment this PR adds, and one fact about the registry that changes what a green TestPinnedPeer/ingot means.

WithWorkspaceBinariesExcept's doc says pinning one service leaves "the other six" with no image — it leaves seven

There are eight required image interpolations, not seven:

$ grep -rhoE '\$\{[A-Z_]+_IMAGE:\?' smelt/ | sort -u
${DELEGATOR_IMAGE:?   ${INDEXER_IMAGE:?   ${PIRI_IMAGE:?     ${SWARF_IMAGE:?
${HILT_IMAGE:?        ${INGOT_IMAGE:?     ${SIGNER_IMAGE:?   ${UPLOAD_IMAGE:?

Seven come from systems/*/compose.yml; PIRI_IMAGE is written by pkg/generate/compose.go:70 into the generated piri-N services, which is why it does not show up in a grep of the checked-in compose files. publishedImages has the same eight. Pin one and seven are left unset.

Not new in this push — smelt/pkg/stack/options.go arrived with the base commit — so this is round two's "six of the eight" class in a file neither round looked at.

Related, and not this PR's to fix: .env.published supplies seven of those eight, missing INDEXER_IMAGE, and smelt/.env has no uncommented lines, so make up fails naming INDEXER_IMAGE. Its header still says "the seven services this repository builds", which is where the "six" above descends from. The Go side is unaffected, and so is this suite: WithPublishedImages() fills all eight from publishedImages.

ingot's baseline 0.0.0 is the never-bumped placeholder, not a release — and the version-shape filter cannot tell the difference

The tag exists and resolves, so the comment's claim is accurate as written. What it means is the thing to look at. Each subtree's version.json against what it published:

piri                  v0.2.4    image 0.2.4      (== :latest)
indexing-service      v1.13.4   image 1.13.4     (== :latest)
ingot                 v0.0.0    image 0.0.0      (no :latest exists)
sprue                 v0.0.6    no version-shaped tag
delegator             v0.1.1    no version-shaped tag
piri-signing-service  v0.1.0    no version-shaped tag
hilt                  v0.0.0    no version-shaped tag
swarf                 v0.0.0    no version-shaped tag

For piri and the indexer the pinned tag is a version somebody cut, and it is what :latest points at. ingot's is the placeholder its version.json has always carried; its only version-shaped tags are 0.0.0, 0.0.0-amd64 and 0.0.0-arm64, out of 111.

Two consequences. TestPinnedPeer/ingot/0.0.0 is not asserting interop with a released ingot — it is asserting it against whatever was published while the version was unstamped. And if ingot ever publishes again with version.json still reading v0.0.0, that tag is rewritten underneath the baseline — the moving-tag ambiguity the suite refuses :main to avoid. versions_from's [0-9]+\.[0-9]+\.[0-9]+ shape test cannot separate the two cases, because the placeholder is version-shaped.

It has not moved so far: ingot:0.0.0 is sha256:658a9027… against :main's sha256:67031155…, and 0.0.0 is a Docker manifest list where :main is an OCI image index — a different builder from the one that publishes :main today. I could not read the images' creation timestamps (the blob CDN is blocked from here), so that rests on the digests and media types rather than on dates. The exposure ends the moment ingot cuts a real version, so this may well be "note it and merge"; it is a decision rather than a defect.

Verification: what was run, and what checked out

(1) Service name vs package name — every path. workspace.Services has nine entries and ModuleDir returns the right directory for all nine; three differ from the service name. Measured at the token endpoint, all nine package names answer 200 (piri, sprue, piri-signing-service, indexing-service, delegator, guppy, hilt, ingot, swarf) and the three service names answer 403 (upload, indexer, signing-service) — the split the fix is built on.

The only places a name reaches the outside world are imageFor and envSuffix, both through ModuleDir; pinnedVersions keys through envSuffix; pinFor and WithWorkspaceBinariesExcept take service names, which is what they want. grep for any other ghcr.io/fil-forge/ + variable in Go finds only imageFor.

(2) The whole chain, run end to end. The script against the live registry:

base_PIRI=0.2.4                    base_SPRUE=
piri=0.2.4                         base_DELEGATOR=
base_HILT=                         base_PIRI_SIGNING_SERVICE=
base_INGOT=0.0.0                   base_SWARF=
ingot=0.0.0                        base_INDEXING_SERVICE=1.13.4
baselines={"DELEGATOR": "", "HILT": "", "INDEXING_SERVICE": "1.13.4", "INGOT": "0.0.0",
           "PIRI": "0.2.4", "PIRI_SIGNING_SERVICE": "", "SPRUE": "", "SWARF": ""}
windows={"INGOT": "0.0.0", "PIRI": "0.2.4"}
pinnable=true

The derivation resolves all eight: sed over options.go yields exactly publishedImages' eight package names, and the doc-comment reference at options.go:240 does not match because it is not :main. The eight keys are exactly the set envSuffix produces for Detect()'s eight fleet services.

The expansion step, extracted from the YAML with a parser and run under bash -e with those two values in the environment, writes ten lines and exits 0:

COMPAT_BASELINE_DELEGATOR=              COMPAT_BASELINE_PIRI_SIGNING_SERVICE=
COMPAT_BASELINE_HILT=                   COMPAT_BASELINE_SPRUE=
COMPAT_BASELINE_INDEXING_SERVICE=1.13.4 COMPAT_BASELINE_SWARF=
COMPAT_BASELINE_INGOT=0.0.0             COMPAT_INGOT_VERSIONS=0.0.0
COMPAT_BASELINE_PIRI=0.2.4              COMPAT_PIRI_VERSIONS=0.2.4

The heredoc survives YAML: the block scalar strips ten spaces from every line, so EOF lands at column 0 for an unindented <<'EOF', and the Python body keeps its relative indentation. The asserts are reachable — no -O — and "\n" reaches Python as an escape, not as two characters. The interpolations are in env:, not inside the run: script, so nothing is re-parsed. A malformed or empty baselines fails the step rather than expanding to nothing: json.loads("") raises. grep '^COMPAT_' under set -e cannot be the empty case here, because the compat job only runs when pinnable=true, which requires a window entry.

Against a real Go run, with exactly those variables, TestRollingUpgrade reaches the baseline gate and skips naming the five that lack one:

unset: COMPAT_BASELINE_DELEGATOR COMPAT_BASELINE_HILT
COMPAT_BASELINE_PIRI_SIGNING_SERVICE COMPAT_BASELINE_SWARF COMPAT_BASELINE_SPRUE

which is the proof the other three keys are read. With nothing set, TestPinnedPeer skips naming COMPAT_PIRI_VERSIONS and COMPAT_INGOT_VERSIONS — the keys the windows JSON produces.

(3) Nothing else reports empty instead of failing. Driven against a fake registry, one mode per row:

ok        exit 0   pinnable=true
notags    exit 0   "publishes no version-tagged image"   (genuine: tags, none version-shaped)
403       exit 0   "no such package -- reporting none"   (the one deliberate case)
401       exit 1   token endpoint answered HTTP 401
404       exit 1   token endpoint answered HTTP 404
500       exit 1   token endpoint answered HTTP 500
badtoken  exit 1   token endpoint returned something that is not a token
tags500   exit 1   tags/list answered HTTP 500
badtags   exit 1   tags/list returned something that is not a tag list

as_json failing would print an empty baselines= with exit 0, but the expansion step then dies on json.loads(""), so that is loud too. The residual fail-open is 403 itself, which also covers "the package went private" — narrowing it further would need a signal the token endpoint does not give.

Sorting is integer, not lexicographic: seeded with 0.9.1 and 0.10.0 the fake run picks 0.10.0 as newest.

(4) Release counts, against the registry. Five of the eight publish no version-shaped tag — sprue, hilt, piri-signing-service, delegator, swarf — and three do: piri 0.2.4, ingot 0.0.0, indexing-service 1.13.4. ghcr.io/fil-forge/indexing-service:1.13.4 exists and is :latest. The comment and the PR body now say this. (See the second finding for what ingot's 0.0.0 is.)

(5) Fleet membership. The slices.Contains(fleet, upgraded) skip sits inside the subtest, before otherThan is called, and fleet is Detect()'s list — correct, and unreachable with this go.work, as the comment says.

Tooling. shellcheck .github/scripts/compat-window.sh clean; actionlint .github/workflows/compat.yml clean, ci.yml has the three pre-existing shellcheck notes at lines 74/132/150, none of them in this diff; all seven .github/scripts/check-*.sh exit 0; gofmt -s -l smelt/ empty; GOWORK=off go vet -tags e2e,compat ./... in smelt exits 0, which is what ci / unit smelt will run; working tree clean at 008335bb.

Residual, not a finding. The baseline key set is now derived twice — by the script from publishedImages, by the test from go.work ∩ workspace.Services — and nothing cross-checks them. They agree today. Divergence in the direction that matters (a fleet service with no publishedImages entry, e.g. guppy if it ever joined go.work) shows up as a skip, not a failure, and a permanent skip is what TestRollingUpgrade already does, so it would be invisible. The other direction is guarded: a fleet service missing from pinFor is a hard t.Fatalf.


Generated by Claude Code

… misses one

Two findings, neither blocking, and both are counting errors of the same kind as
round two's "six of the eight".

THERE ARE EIGHT REQUIRED IMAGE INTERPOLATIONS, not seven. `INDEXER_IMAGE` is the
one that is not at `systems/*/compose.yml` -- it lives a level down, in
`systems/indexing/indexer/` -- so a flat glob reports seven and looks complete.
Pinning one service therefore leaves SEVEN others with no image, not six. Both
places that stated it now say eight, and both say how it was counted, because
the glob is the trap.

THE INGOT BASELINE IS A PLACEHOLDER. `ingot/version.json` reads v0.0.0, the same
value hilt and swarf carry, so ghcr.io/fil-forge/ingot:0.0.0 is what its CI
publishes for an unstamped build and the next publish overwrites that tag unless
the file is bumped first. piri (v0.2.4) and the indexer (v1.13.4) carry
version.json values matching their published tags; ingot does not. So one of the
two baselines this suite pins is a moving tag wearing a version's shape -- the
ambiguity the suite refuses `:main` to avoid, and one a version-shape test
cannot see.

Left as it is, and recorded rather than fixed. The fix is rule 2 applied here --
pin the baseline by digest, which the workflow could resolve in the request it
already makes -- and it changes the shape of what the workflow hands the test.
That wants its own change and its own review round rather than being folded into
one that has had three.

MONOREPO_TODO.md also gains the `make up` failure the count above explains:
`.env.published` supplies seven of the eight required variables and `.env` is
entirely commented out, so `make up` refuses naming INDEXER_IMAGE. Pre-existing
on main and invisible to CI, because every Go path fills all eight through
WithPublishedImages(), whose table does list the indexer. Two lists of the same
eight services that can disagree, and did -- rule 3's shape exactly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 22, 2026
Rounds 1 and 2 on each pull request found defects the previous round's fixes had
introduced -- four of the eleven findings across the two. Round 3 on each found
none of that shape: two counting errors on #18 (there are eight required image
interpolations, and a flat systems/*/compose.yml glob misses INDEXER_IMAGE one
level down), and three claims-about-claims on #19, the sharpest being that the
fork gate does not do what its comment said.

Also corrects this page's own count: five services lack a version-tagged image,
not six. The indexer publishes 1.13.4; this entry and #18's comment both had it
among the missing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

@Peeja Peeja left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[From Claude:]

Round 4 — four findings, none blocking. The headline number is right: there are eight required image interpolations, and round 3's finding (a) was correctly resolved. What this push added beside the number — how it was counted — does not reproduce, in all four places it appears. Finding 1 is the one worth acting on; 2–4 are accuracy notes on the two new MONOREPO_TODO.md entries.

1. Eight is right, but the stated derivation yields seven — the eighth is PIRI_IMAGE, which is not under systems/ at all

The three counts, run in the checkout at a7434b4b:

A) systems/*/compose.yml (flat glob)       6   DELEGATOR HILT INGOT SIGNER SWARF UPLOAD
B) find smelt/systems -name compose.yml    7   A + INDEXER
C) B + smelt/pkg/generate/compose.go       8   B + PIRI

PIRI_IMAGE has no compose.yml anywhere. It is emitted at smelt/pkg/generate/compose.go:70 into generated/compose/piri.yml, which the root smelt/compose.yml includes at line 24. So the eighth variable is missed by both globs, and INDEXER_IMAGE explains only the step from 6 to 7.

Every new statement of the recipe therefore has both numbers wrong and attributes the gap to the wrong variable:

  • smelt/pkg/stack/options.go — "Eight, counted over find smelt/systems -name compose.yml and not over systems/*/compose.yml: INDEXER_IMAGE lives one level down … and a flat glob reports seven." The find reports seven, not eight; the flat glob reports six, not seven.
  • smelt/tests/compat/compat_test.go — "Eight, and counted over every compose.yml under systems/ … INDEXER_IMAGE is one level down, so a flat glob says seven." Same two errors.
  • MONOREPO_TODO.md, the first code block — the pasted transcript shows PIRI_IMAGE in the output of a command that does not emit it. Run verbatim:
$ find smelt/systems -name compose.yml -print0 | xargs -0 grep -ho '${[A-Z_]*_IMAGE:?' \
    | sort -u | tr -d '${:?'
DELEGATOR_IMAGE
HILT_IMAGE
INDEXER_IMAGE
INGOT_IMAGE
SIGNER_IMAGE
SWARF_IMAGE
UPLOAD_IMAGE

Seven lines, no PIRI_IMAGE. The other two blocks in that entry (.env.published grep → seven names, grep -cvE on .env → 0) reproduce exactly.

  • MONOREPO_TODO.md, prose — "it is the one required variable not at systems/*/compose.yml … so a flat glob reports seven of eight and looks complete." PIRI_IMAGE is also not at systems/*/compose.yml, and the flat glob reports six of eight.

The correct derivation is already in the tree, in the same file as the first instance: smelt/pkg/stack/options.go:428 reads "the variable names match the ones the compose files already interpolate (systems/*/compose.yml, and PIRI_IMAGE in pkg/generate)".

A reproducible one-liner, if a replacement is wanted:

{ find smelt/systems -name compose.yml -print0 | xargs -0 grep -ho '${[A-Z_]*_IMAGE:?'
  grep -ho '${[A-Z_]*_IMAGE:?' smelt/pkg/generate/compose.go; } | sort -u | tr -d '${:?'

Unaffected: "${HILT_IMAGE:?…} and seven siblings" and "leaves the other seven" are both correct, as is "EIGHT of them".

2. "Only a human running make up hits it" is narrower than the tree — fourteen targets and the snapshot CLI share those env files; "CI is unaffected" does hold

ENV_FILES := --env-file .env.published --env-file .env is set once at smelt/Makefile:23 and used by $(COMPOSE) (and $(DOCKER) compose $(ENV_FILES) at line 321) in fourteen targets, of which up is one:

build clean debug-upload down fresh logs nuke pull
shell-guppy shell-hilt shell-piri shell-upload status up

smelt/pkg/snapshot/stack.go:26 independently attaches the same pair for its ps / stop / config / down calls, so go run ./cmd/smelt snapshot … against the real project directory reads the same seven variables. (In the Go stack path it is harmless: extract.go runs compose in a temp dir and .env.published is deliberately not embedded, so the files are absent and buildEnv() supplies all eight.)

The conclusion that matters is sound. I could not run docker, so I did not confirm which subcommands abort on a missing :?, but the CI half is checkable and checks out — nothing under .github/ invokes smelt's Makefile, $(COMPOSE), or cmd/smelt snapshot; the only matches for docker compose are docker compose version in e2e.yml:101 and itest.yml:127.

3. The recorded digest fix needs a second registry request per tag — tags/list carries no digests

MONOREPO_TODO.md: "The workflow already resolves each service's newest version against the registry, so it can resolve the digest in the same request and carry it through."

The request compat-window.sh already makes is GET /v2/fil-forge/<pkg>/tags/list?n=1000 (line 100), whose body is {"name":…, "tags":[…]} — tag names only. Getting a digest takes a separate authenticated HEAD /v2/fil-forge/<pkg>/manifests/<tag> with an Accept header, which is how I obtained these:

ingot            main    sha256:6703115554d6ccc1908368592fece7a1b1ca6b26dea634b33c6e5b04e6946d0c
ingot            0.0.0   sha256:658a9027ce396017639e301161891a480409e8a951a751c1643769fdef5113ed
piri             0.2.4   sha256:a36a4135b3d31627097acf964b3f4e678f522bfdb4fc04ddd65bea86a6b35468
indexing-service 1.13.4  sha256:b672fcc7deb199f70845cad8e51b042c8afff60b75b0c88bb103b5ed70f5a57d

Worth noting for whoever picks the fix up, since rule 2 says pin the index digest, never a per-architecture one: the Accept header decides which you get back, and ingot publishes 0.0.0-amd64 / 0.0.0-arm64 alongside 0.0.0.

4. "so those baselines are real" claims immutability the matching version.json does not give

smelt/tests/compat/compat_test.go: "piri (v0.2.4) and the indexer (v1.13.4) carry version.json values matching their published tags, so those baselines are real."

Both halves of the premise check out (see verification). The inference does not. The mechanism the same comment describes — CI publishes version.json's value, and the next publish overwrites that tag unless the file is bumped first — applies identically to piri and the indexer: if either cuts another release without bumping, :0.2.4 and :1.13.4 move exactly as :0.0.0 would. What distinguishes them is that their values are cut versions rather than a placeholder, not that their tags are pinned. The thing that would make any of the three immutable is the digest pin, which is why the recorded fix is the right one for all of them and not only for ingot.

Understating it slightly would be accurate: ingot's is the one where the value is known not to correspond to a release.

Verified, and what checked out

Round 3 (b) — the recorded claims are true. Every version.json in the tree:

ingot            v0.0.0     hilt   v0.0.0     swarf  v0.0.0
piri             v0.2.4     sprue  v0.0.6     delegator v0.1.1
indexing-service v1.13.4    piri-signing-service v0.1.0

Against the live registry, version-shaped tags only (re.fullmatch(r"[0-9]+\.[0-9]+\.[0-9]+"), the suite's own filter):

fil-forge/piri              0.2.4                      (+ latest, main, sha-*)
fil-forge/indexing-service  1.13.4                     (+ latest, main, sha-*)
fil-forge/ingot             0.0.0                      (0.0.0-amd64/-arm64 correctly excluded)

So piri and the indexer match their version.json; ingot's 0.0.0 is the placeholder hilt and swarf also carry. ingot:0.0.0 and ingot:main are currently different digests, consistent with "Nothing has moved so far". The claim about what ingot's CI publishes is not checkable here — the subtrees carry no .github/, and images ship from the polyrepos — but nothing in the tree contradicts it.

The make up / INDEXER_IMAGE claim, part by part. INDEXER_IMAGE is required (smelt/systems/indexing/indexer/compose.yml:25) and reachable from the root project: compose.yml includes systems/indexing/compose.yml, which includes indexer/compose.yml. .env.published carries seven _IMAGE assignments and not that one — its own header still says "the seven services this repository builds". smelt/.env has 0 non-comment non-blank lines. publishedImages (options.go:461) has eight entries and does include ghcr.io/fil-forge/indexing-service:main, so the Go paths are covered. Neither .env.published, .env, nor any compose file is touched by this PR, so "pre-existing on main" holds.

No new defect in the code. This push is comment-only in both Go files; git diff shows no statement changed. gofmt -s -l smelt/ is empty, GOWORK=off go vet ./pkg/stack/... ./pkg/workspace/... is clean, and the actual CI command for this matrix entry — go vet -tags e2e,compat ./... in smelt/ — exits 0. The detached comment block added above pinFor compiles and formats fine.

Other numbers in the diff, all correct. "Three of the nine" differ between service and package name (Services has 9 entries; upload/sprue, indexer/indexing-service, signing-service/piri-signing-service). The TestRollingUpgrade fleet is eight — Detect() over this go.work selects piri, upload, signing-service, indexer, delegator, hilt, ingot, swarf (guppy is not in the use-list) — and "five of the eight publish no version-tagged image … the other three do" matches the registry. compat-window.sh's derived service list yields exactly the eight package names from publishedImages. Its "all 111 of ingot's" is exact: tags/list?n=1000 returns 111 tags today.


Generated by Claude Code

Petra asked why a version tag would ever be overwritten -- the build published
at foo:1.2.3 should be the commit git-tagged v1.2.3, which is typically the
commit that incremented version.json. That is exactly what the polyrepos do,
and the previous commit claimed the opposite without checking.

Measured on the upstream workflows rather than inferred from the tag list:

  releaser.yml          push, paths: [version.json]   -> ipdxco's releaser
                        reads the new version, creates git tag vX.Y.Z at that
                        commit, publishes a GitHub release
  tagpush.yml           push, tags: v*                -> the checker
  release-binaries.yml  release: [published]          -> goreleaser, and this
                        is what pushes ghcr.io/fil-forge/<svc>:X.Y.Z
  publish-ghcr.yml      push, branches: [main]        -> :main, :main-dev,
                        :sha-*, and never a version tag

So a version image tag corresponds to a git tag on the commit that bumped
version.json, and no ordinary push republishes one. A release REQUIRES the bump;
it does not happen in spite of one. fil-forge/ingot carries exactly one tag,
v0.0.0, at 6cfb5df -- so ingot:0.0.0 is that tagged commit's artifact, not an
unstamped build of whatever was on main.

What ingot's 0.0.0 actually says is how OLD the pinned peer is: its first and
only release. That is a statement about coverage, not about mutability, and it
does not call for pinning baselines by digest. Both the comment and the
MONOREPO_TODO entry that recommended it are gone.

Also, from round four: the derivation printed beside "eight required image
interpolations" was wrong in four places, and its two intermediate numbers were
both off by one. The eighth is PIRI_IMAGE, which has no compose.yml at all --
pkg/generate/compose.go emits it -- not INDEXER_IMAGE's nesting.
systems/*/compose.yml finds six; recursing adds INDEXER_IMAGE; the generator
adds PIRI_IMAGE. Either glob alone looks complete, which is the trap, and three
rounds got the number wrong in both directions.

And the env-file entry said "only a human running make up hits it". ENV_FILES
feeds all seventeen $(COMPOSE) calls in smelt's Makefile and the snapshot CLI.
"CI is unaffected" does hold: nothing under .github/ runs any of them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

@Peeja Peeja left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[From Claude:]

Round 5 — 4 findings. Three falsify clauses of the new release-mechanism comment above pinFor in smelt/tests/compat/compat_test.go; one is a derived number in MONOREPO_TODO.md that is off by one. The eight-image count re-derives correctly and all three copies of it agree with each other and with the tree.

1. ingot's v0.0.0 is not on a commit that incremented version.json — the releaser tags the push head, and ingot's version.json was never incremented

The comment's load-bearing sentence is "a version image tag corresponds to a git tag on the commit that incremented version.json", and it then rests ingot's baseline on it. ingot is the counterexample.

$ git log -1 --format='%h %s' 6cfb5df4          # fil-forge/ingot tag v0.0.0
6cfb5df4 test: skip test flakes under shuffle
$ git show --stat 6cfb5df4 | tail -3
 CLAUDE.md             | 11 +++++++++++
 testing/smoke_test.go | 32 +++++++++++++++++++++++++++++++-

$ git log --oneline up-ingot/main -- version.json
eae82996 ci: implement release and container workflows     # ADDS it, at v0.0.0

One commit has ever touched ingot's version.json, and it creates the file at v0.0.0. Nothing has ever incremented it. The tag sits on 6cfb5df4, which touches CLAUDE.md and a test.

How it got there — eae82996 is 6cfb5df4's immediate parent, and both carry committer date 2026-07-13 10:51:46 -0700. One push, two commits, version.json among the changed files, so releaser.yml fired with github.sha = the head of the push. The release is stamped 2026-07-13T17:52:03Z (10:52 PDT), one minute later, author github-actions[bot].

So the ipdxco chain is real, but it tags the push head, not the commit that changed version.json — those coincide only when the bump is the last commit in the push. Two smaller imprecisions in the same sentence: paths: ['version.json'] matches a push in which the file was added or modified, which is not a check that the version value changed; and releaser.yml carries no branches: filter at all.

What survives: ingot:0.0.0 genuinely is a cut release's artifact — GHCR carries 0.0.0-amd64, 0.0.0-arm64 and the 0.0.0 manifest, which is goreleaser's docker_manifests and nothing else — and it is 82 commits behind up-ingot/main. The coverage point is right. The sentence a reader would check it by is false on its own example.

2. release-binaries.yml pushes no image for piri or indexing-service — their goreleaser configs have no docker section at all

The comment says "release-binaries.yml fires on that release and is what pushes ghcr.io/fil-forge/<svc>:X.Y.Z", with <svc> generalising over the polyrepos. That is ingot's arrangement only.

$ git show up-piri/main:.goreleaser.yaml | grep -ci -E 'docker|ghcr|image_templates|manifest'
0
$ git show up-indexing-service/main:.goreleaser.yaml | grep -ci -E 'docker|ghcr|image_templates|manifest'
0

Only ingot's .goreleaser.yaml has dockers: / docker_manifests: naming ghcr.io/fil-forge/ingot:{{ .Version }}, and only ingot's release-binaries.yml has packages: write, QEMU, buildx and docker/login-action. piri's runs on macos-latest with permissions: contents: write and no registry login; indexing-service's has no permissions block and no login. Neither could push to GHCR if it wanted to.

For those two the version image comes from publish-ghcr.yml's publish-release job — on: push: tags: ['v*'], or workflow_run: [Releaser] completed with conclusion == 'success' — via docker/metadata-action type=semver,pattern={{version}}. The registry corroborates:

repo version tags in GHCR latest main-dev per-arch
ingot 0.0.0, 0.0.0-amd64, 0.0.0-arm64 no yes yes
piri 0.2.4 yes no no
indexing-service 1.13.4 yes no no

latest is docker/metadata-action's latest=auto default firing on a semver tag; ingot has none because goreleaser, not metadata-action, published its version tag. The same table kills the :main-dev clause: that is ingot's prod/dev publish matrix, and piri's and indexing-service's publish-ghcr.yml emit :main and :sha-<short> only.

3. "no push republishes one" is false: a v* tag push republishes :X.Y.Z for piri and indexing-service, and workflow_dispatch is on every link of the chain

publish-ghcr.yml in both repos fires on push: tags: ['v*'], and publish-release gates only on startsWith(github.ref, 'refs/tags/v'). A moved tag — git push -f origin v0.2.4 at a different commit — re-runs that job and re-pushes ghcr.io/fil-forge/piri:0.2.4 at new content, with no version.json change and no new GitHub release anywhere in the picture. tagpush.yml's own header names this as an expected path: "Sanity-checks v* tags pushed directly (outside the Releaser flow)."

Three more, in all three repos:

  • release-binaries.yml carries workflow_dispatch. Re-running goreleaser at the tag re-pushes the manifest (ingot). release: mode: keep-existing protects the GitHub release's assets, not the image; org.opencontainers.image.created={{.Date}} and a moved base image both change the digest under the same tag.
  • release-binaries.yml also has workflow_run: workflows: [Releaser], types: [completed] with no conclusion == 'success' guard on the job — unlike publish-ghcr.yml's prepare-checkout/publish-release, which do check it. So it runs on a failed or cancelled Releaser too.
  • releaser.yml itself has workflow_dispatch, which is how finding 1's tag is reachable without a bump.

This matters beyond wording: the previous commit's digest-pinning recommendation was removed on the strength of this sentence. Nothing here says the baselines are wrong today — piri's and indexing-service's tags do sit on version.json bumps (b94c8117 release: v0.2.4, 8aaf51b4 docs(version): update for v1.13.4) — but "no push republishes one" is not a property the chain has.

4. MONOREPO_TODO.md: sixteen $(COMPOSE) calls, not seventeen, and debug-upload contains none of them
$ grep -n '\$(COMPOSE)' smelt/Makefile | wc -l
17
$ grep -n '\$(COMPOSE)' smelt/Makefile | head -1
32:# runs it on one line, then `$(COMPOSE) up` on the next), but make expands a

Line 32 is prose inside the comment above the COMPOSE definition. The real calls are at 178, 185, 207, 227, 241, 252, 253, 273, 287, 291, 295, 297, 301, 305, 309, 313 — sixteen.

And debug-upload, named in the entry's target list, has no $(COMPOSE) in it; it spells the call out:

smelt/Makefile:321:	$(DOCKER) compose $(ENV_FILES) -f compose.yml -f compose.debug.yml up -d --force-recreate upload upload-init

The listed targets account for exactly sixteen: up 1, down 1, clean 1, nuke 1, fresh 3, pull 1, build 1, logs 1, status 2, shell-{guppy,piri,upload,hilt} 4. Seventeen is the right number for a different set — compose invocations that take $(ENV_FILES), i.e. those sixteen plus debug-upload's explicit one — which is also the claim the paragraph is actually making about reach. As written it pairs a count with a name that is not a member of what is counted.

Verification — what checked out

The eight-image derivation, re-run from scratch. All three copies agree with each other and with the tree:

$ grep -ho '${[A-Z_]*_IMAGE:?' smelt/systems/*/compose.yml | sort -u | wc -l
6
$ find smelt/systems -name compose.yml -print0 | xargs -0 grep -ho '${[A-Z_]*_IMAGE:?' | sort -u | wc -l
7      # + INDEXER_IMAGE, from systems/indexing/indexer/compose.yml
$ grep -ho '${[A-Z_]*_IMAGE:?' smelt/pkg/generate/compose.go | tr -d '${:?'
PIRI_IMAGE

Six / seven / eight, and PIRI_IMAGE is smelt/pkg/generate/compose.go:70, written to generated/compose/piri.yml (generate.go:53,69). A whole-tree sweep for ${..._IMAGE:? across *.yml, *.yaml, *.go returns the same eight and nothing more. GUPPY_IMAGE, BLOCKCHAIN_IMAGE, IPNI_IMAGE, MINIO_IMAGE, PLC_IMAGE are all :- defaults, correctly excluded.

The pasted transcript reproduces. Run verbatim from the repo root, the brace-group command emits exactly those eight names (one per line; the block wraps them four to a line, as it did before this push). Exit 0. The other two transcripts also hold: .env.published lists the seven without INDEXER_IMAGE, and grep -cvE '^\s*(#|$)' smelt/.env is 0.

stack.WithPublishedImages(). publishedImages (smelt/pkg/stack/options.go:463-475) has eight entries and does include ghcr.io/fil-forge/indexing-service:main.

"CI is unaffected" holds. No workflow runs smelt's Makefile — the four make hits under .github/workflows/ are all inside comments — and the only docker compose invocations are docker compose version in e2e.yml:101 and itest.yml:127, which take no --env-file. No snapshot CLI invocation anywhere under .github/. ENV_FILES is at smelt/Makefile:23 as cited, and smelt/pkg/snapshot/stack.go:30 does attach the same .env.published / .env pair.

ingot's tag count. fil-forge/ingot carries exactly one tag, v0.0.0 at 6cfb5df4, confirmed against both the GitHub API and git ls-remote --tags up-ingot. That clause is right; what it is used to conclude is finding 1.

The comment changes in options.go and in TestPinnedPeer are correct as written, and match the MONOREPO_TODO.md paragraph. No defect found in the removal of the old "placeholder, not a release" section beyond what findings 1-3 say about its replacement.


Generated by Claude Code

Round five falsified the corrected version, as round three had falsified the
one before it. Verified against upstream rather than taken on report: ingot's
v0.0.0 sits on 6cfb5df, "test: skip test flakes under shuffle", which touches
CLAUDE.md and testing/smoke_test.go. The only commit that has ever touched
ingot/version.json is its parent eae8299, which ADDS the file. Same committer
date, so one push -- and the releaser tagged the push head, not the commit that
introduced the version. Two more clauses were wrong with it: release-binaries
pushes an image only for ingot (piri's and indexing-service's goreleaser
configs have no docker block; their version image comes from publish-ghcr's
release job), and "no push republishes one" ignores that those two publish on
`push: tags: v*`, so a moved tag does.

The conclusion each revision was reaching for has survived all three and is
what the comment now says: upstream publishes <svc>:X.Y.Z only from the git tag,
never from an ordinary push to main, and that is what makes these baselines
usable where :main is not.

The mechanism is gone rather than corrected a third time. It differs per
service, it lives in each polyrepo's .github/workflows, and no detail of it
changes what a reader of this file does -- which is the detail rule, applied to
the paragraph that has broken it hardest.

Also sixteen $(COMPOSE) calls, not seventeen: the seventeenth grep hit is prose
in a comment, and debug-upload spells the command out instead of using the
variable.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 22, 2026
Fourteen behavioural findings in rounds 1-2 across #18 and #19. Zero in rounds
3-5, which produced nineteen findings, all about claims -- a comment, a count, a
PR body -- rather than about what the code does.

The claim-findings sustain themselves: each fix to a comment is a new comment
the next round finds slightly off. The timing number could not converge at all,
since the run that measures the sentence always postdates it, and the
release-mechanism paragraph took three wrong versions before deleting it was the
answer.

The diagnosis is about rule 10 rather than these two pull requests.
release.yml's diff is ~305 lines and mostly commentary, so the rounds reviewed
the commentary; a reviewer told to verify every factual claim in 300 lines of
prose will always find one, which makes the satisfied signal unreachable by
construction on a comment-heavy diff. Possible fix recorded, not taken: scope a
round to what changed since the last round and skip it when that is comments
only. Petra's call.

Both pull requests are now hers to read. CI: #18 24/24 green, #19 green bar one
itest leg still running.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
@Peeja
Peeja marked this pull request as ready for review September 22, 2026 21:27
Peeja pushed a commit that referenced this pull request Sep 22, 2026
…open

Petra decided a PR goes Open as soon as the rounds are done, whoever calls the
stop, so #18 and #19 were un-drafted and #19 merged as 82aaa35. Rule 10 said
the opposite in two places -- its headline and the two-signals bullet -- and
forge#22 corrects both, opened without a round because it is a prose-only change
to a prose file, which is the case the rule it adds says to skip.

Her open question on #18 is recorded with my answer: run compat on the release
pull request, yes, but keep the schedule. The pinned side moves independently of
us -- compat-window.sh reads the registry live -- so an upstream release can
break compat with no pull request of ours firing. The release PR answers whether
our change broke it; only the schedule answers whether theirs did.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
…swer fresh

Petra's reframing, and it is better than what was here: the question worth
asking is "does the release in THIS pull request break compat?", and the answer
to that changes over time without the pull request changing. A schedule on main
answers a different question badly -- main is not deployed, so a red there has
no decision attached and leaves you working out which pending release it
implicates.

So compat.yml loses its schedule and gains `pull_request`, gated on a
`release/*` head. It takes no service: unlike release.yml it tests the whole
stack against the window, so any release pull request gets the same run and the
branch name is only the signal that a release is being proposed. The gate is a
job condition rather than `branches:`, which filters the BASE branch.

The staleness is handled by compat-refresh.yml, which is the interesting half.
A `schedule:` always runs on the default branch, so it cannot BE the compat run
for a pull request -- instead it lists the open release pull requests and
RE-RUNS each one's own compat run. Re-running rather than dispatching against
the branch is the whole point: a re-run stays attached to the pull request and
replaces its check in place, so the fresh answer lands where the merge decision
is made. A dispatch would put it in the Actions tab instead.

It also refuses to be quiet about its own failure, which the first draft did
not: `mapfile < <(gh ...)` discards the subshell's status, so a gh that cannot
reach the API produced an empty list, the "nothing to refresh" early exit and a
green run -- the same shape as the `|| true` removed from compat-window.sh one
file over, reproduced in the file written to complement it. Caught by testing
it: six paths exercised against a stubbed gh, and the three failure modes now
exit 1 rather than 0.

First workflow here to ask for more than contents: read. It wants
pull-requests: read and actions: write, to list and to re-run; it starts nothing
new and reaches nothing this repository does not already run.

What this gives up is stated in the file: nothing runs while no release pull
request is open, so an upstream incompatibility landing in a quiet week is found
when the next release is proposed rather than when it appeared. Deliberate --
the earlier signal had nobody to act on it -- but if the lead time turns out to
matter, the fix is a scheduled run that opens or annotates a release pull
request, not one that tests main.

Five stale "nightly" claims went with it, enumerated rather than spotted: one in
compat.yml, three in compat-window.sh, and the test's own doc comment, which had
the trigger exactly backwards ("nightly and before a release, never on PRs").

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

@Peeja Peeja left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[From Claude:]

Round six, scoped to d18b640b only: compat-refresh.yml, compat.yml's trigger and job condition, and the test's doc comment where it asserts something. Correctness only.

Six findings. One blocks: adding pull_request to a workflow whose concurrency group is the bare workflow name makes every pull request in the repository a member of that group, and GitHub cancels a pending member when a newer one queues — including the refresher's own re-runs. The gate itself is sound (an ordinary PR's window skips, compat skips with it), and the sha match in the refresher is correct — verified against this repository's own run records.

1. BLOCKING — `compat.yml`'s concurrency group is repo-wide, and the new trigger fills it: pending runs get cancelled, including the refresher's own

compat.yml keys concurrency on the workflow name alone, unchanged by this push:

concurrency:
  group: ${{ github.workflow }}
  cancel-in-progress: false

That was right when the only entrants were one scheduled run a day and the occasional dispatch — two members could barely coexist. pull_request changes what is in the group: every pull request event in the repository now creates a compat run, and concurrency is applied at run level, before any job's if is evaluated. A run whose jobs will all skip still joins the queue.

GitHub's rule for a group with cancel-in-progress: false:

When a concurrent job or workflow is queued, if another job or workflow using the same concurrency group in the repository is in progress, the queued job or workflow will be pending. Any previously pending job or workflow in the concurrency group will be canceled.

So, with the compat job capped at 60 minutes:

entrants while a release PR's compat run is in progress result
a second release PR's run (from a push, or from the refresher) pending, up to an hour
any third entrant — a third release PR, or any pull_request event anywhere in the repo the second is cancelled

Two consequences, both against what this push is for:

  • The refresher cancels its own work. It re-runs every open release PR back to back. The first starts, the second goes pending, the third cancels the second. gh run rerun returned 0 for all of them, so the run reports 3 open release pull request(s), 0 with no run to re-run. and exits green while one PR's answer was cancelled, not refreshed. The next day's refresh does re-run it (a cancelled run is re-runnable), so it self-heals in 24h — but the check on the PR reads cancelled in the meantime, and nothing said so.
  • "costs an ordinary pull request nothing" is not quite true (compat.yml line 66). It costs no runner minutes, but the run is created, can sit pending behind an hour-long release run, and can be cancelled outright by a later entrant — a compat check on an ordinary PR that reads cancelled rather than skipped.

Every other workflow in the tree already keys on the ref, and images.yml and e2e.yml carry comments explaining why:

ci.yml:40      group: ${{ github.workflow }}-${{ github.ref }}
e2e.yml:56     group: ${{ github.workflow }}-${{ github.ref }}
images.yml:38  group: ${{ github.workflow }}-${{ github.ref }}
itest.yml:38   group: ${{ github.workflow }}-${{ github.ref }}
release.yml:71 group: ${{ github.workflow }}-${{ inputs.service }}
compat.yml:58  group: ${{ github.workflow }}

group: ${{ github.workflow }}-${{ github.head_ref || github.ref }} keeps one compat run per branch, which is what the serialization was ever for, and no longer makes two release PRs contend.

2. A failed `gh run rerun` aborts the loop: the release PRs after it are silently never refreshed, and the tally never prints

set -euo pipefail plus a bare gh run rerun means the first rejected re-run ends the job. Run against a stub gh with three open release PRs, the first rerun rejected:

::group::#12 (release/piri @ aaaaaaaa)
re-running 5550001
HTTP 403: Unable to retry this workflow run because it was created over a month ago
EXIT=1

#7 and #8 were never looked at, ::endgroup:: never closed (so in the Actions UI the failure is inside a collapsed group), and the summary line never printed. The exit is 1, so the refresher is loud — but "which pull requests did not get refreshed today" is exactly what the log no longer says.

Two ways in, neither exotic:

  • The run is still in progress at 07:00. A run cannot be re-run until it completes; a push to a release PR shortly before the schedule is enough.
  • The run is over 30 days old (finding 3).

Both are conditions about one pull request that stop every other pull request from being refreshed. A || { echo "::warning::…"; failed=1; } around the rerun, continuing the loop and exiting non-zero at the end, keeps the loudness and loses the starvation.

3. Re-running has a 30-day ceiling, so "re-runs it daily on every open release pull request" expires — and then goes green with nothing to do

GitHub's documented limits on the mechanism this file is built on:

You can re-run a workflow run, all failed jobs in a workflow run, or specific jobs in a workflow run up to 30 days after its initial run.

A workflow run can be re-run a maximum of 50 times.

A release pull request that stays open without a push therefore ages through three states:

age of the run refresher behaviour what the PR's check says
0–30 days re-run daily, as advertised fresh
31+ days gh run rerun rejected → red, loop aborts (finding 2) last attempt, stale green
past run retention no run matches the sha → stale++, exit 0 stale green, and the refresher is green too

The third row is the shape this file exists to prevent: a stale green check and a green run that refreshed nothing. Two places assert the unqualified version and would both need the qualifier, or a fallback (a fresh dispatch, or asking for a push) when the re-run is refused:

  • compat-refresh.yml header, and compat.yml lines 24–27: "re-runs THIS one on every open release pull request, daily".
  • smelt/tests/compat/compat_test.go: "compat-refresh.yml re-runs it daily on every open release pull request".
4. `actions: write` reaches further than the comment's bound — it can dispatch `release.yml`, which nothing else in this repository ever runs

The claim, at compat-refresh.yml line 41:

It starts nothing new and can reach nothing this repository does not already run.

True of the script. Not true of the grant, and the sentence sits in the permissions block as the justification for it. actions: write is repo-wide and covers, beyond re-running: cancelling runs, deleting workflow runs, logs, artifacts and caches, and POST /actions/workflows/{id}/dispatches — i.e. starting any workflow that has a workflow_dispatch trigger.

Checked against the tree: release.yml is on: workflow_dispatch: and nothing else ("NOT ARMED, deliberately: no push: tags: trigger"). It is precisely a workflow this repository never runs on its own, and this is the first token in the repository that could start it. Its dry_run input defaults to true and publishing is not armed, so the blast radius today is small — but "can reach nothing this repository does not already run" is the wrong claim to leave next to the grant.

Scope-wise the two permissions are minimal for what the job does: pull-requests: read for gh pr list, and actions: write because no narrower scope re-runs a run (actions: read cannot). contents: read is unused — the job never checks out — but harmless. Accurate replacement: say the job starts nothing new, and that actions: write is repo-wide because GitHub has no finer scope for a re-run.

5. "no compat run at this sha" is reported only inside a collapsed group in a green run
::group::#12 (release/piri @ aaaaaaaa)
no compat run at aaaaaaaaaaaaaaaa yet -- nothing to re-run
::endgroup::

1 open release pull request(s), 1 with no run to re-run.
EXIT=0

The comment says this is "Said out loud rather than passed over", but ::group:: blocks are collapsed by default, so the line is behind a disclosure in a green run, and the tally at the end is the only visible trace. For the transient case (a PR opened minutes ago) that is right. For the permanent ones it is not: a release pull request whose run has aged out of retention, or one opened by automation using GITHUB_TOKEN — which does not raise a pull_request event and so never gets a compat run at all — is reported green, daily, forever. ::warning:: costs nothing here and puts the count on the run summary page where a stale check has somewhere to show up.

6. Minor: a fork pull request with a `release/*` head branch passes the gate and starts the hour-capped job

github.head_ref is the head branch name for fork pull requests too, so startsWith(github.head_ref, 'release/') is satisfied by any fork branch named release/anything. Nothing in the job needs a secret — compat-window.sh mints anonymous ghcr pull tokens ("Anonymous pull tokens. These packages are public") — so the run proceeds normally: up to an hour of runner time, and a member of the repo-wide concurrency group in finding 1, available to anyone who can open a pull request.

If release proposals are always in-repo branches, github.event.pull_request.head.repo.full_name == github.repository in the same condition closes it. If a fork should be able to propose a release, then this is working as intended and only finding 1 applies.

Verification — what checked out, and what I could not check

The gate (brief item 1) — holds. compat.yml has two jobs; compat has needs: window and an if: with no status function, so GitHub applies an implicit success() and a skipped window skips it. Belt and braces, independent of that rule: a skipped job's outputs are empty, so needs.window.outputs.pinnable == 'true' is false regardless. Both agree, so the "both skip together" claim holds either way. github.head_ref is populated for pull_request and empty elsewhere; the github.event_name != 'pull_request' || clause carries dispatch, and startsWith('', 'release/') is false rather than an error, so no event errors out. The on: comment is right that branches: filters the base branch. Release PR shapes that do reach it: drafts (opened fires for drafts, which matters here because rule 10 keeps PRs in draft), reopened, and every push (synchronize). Shapes that reach it and arguably should not: finding 6.

The sha match (brief item 4) — holds, verified against real data, not docs. For pull_request runs in this repository, the run's head_sha is the pull request's head commit, not the merge commit: run 35788998843 on claude/compat-polyrepo records head_sha: d18b640b911bb34fbe920fc9ac8ccb0cf892afa1, head_branch: claude/compat-polyrepo, pull_requests: [18], and PR #18's head.sha is the same d18b640b…. So headSha == headRefOid matches the right run, --branch matches the PR head branch, and a run for code the branch has moved past cannot match.

The refresher's parsing — exercised, not read. Extracted the step body and ran it against a stub gh. Tab splitting (${row%%$'\t'*} / ${rest##*$'\t'}) recovers all three fields; branch names cannot contain tabs, so the last-tab form is safe. mapfile on printf '%s' output keeps the final unterminated line. grep . || true yields an empty array on empty input, count 0, clean early exit. jq: [] | first | .databaseId // empty prints nothing and exits 0 (first on an empty array is null, and null | .databaseId is null, not an error), so "no match" is distinguishable from an error, which gh propagates as a non-zero exit into the if ! guards. --json field names (databaseId, headSha, event, headRefOid) are all valid for their commands, and $sha interpolated into the jq program is a 40-char hex string from the API.

Pagination — not a problem today, worth knowing. gh pr list --limit 100: the repository has 1 open pull request. gh run list --limit 20 is newest-first, so the run at the current head sha is near the top. Neither is load-bearing now; both fail by silently finding nothing if the counts ever change.

Header claims (brief item 6) — checked. "A schedule: trigger always runs on the default branch": correct, scheduled workflows run on the latest commit of the default branch. "The first workflow here to want more than contents: read": correct — ci, e2e, images, itest declare contents: read only, and release.yml declares contents: read at job level twice, with nothing else anywhere in .github/workflows/. "gh workflow run --ref release/piri would also run compat against that branch": plausible — workflow_dispatch is present and the window input has a default, so no -f is needed. compat.yml does now have no schedule: and compat-refresh.yml does.

Item 2, the central claim — verified in part. Confirmed from GitHub's documentation: a re-run reuses the same run, keeps the same GITHUB_SHA and GITHUB_REF as the original event, and runs as a new attempt of it; a re-run of a pull_request run therefore stays bound to the same head sha, which is what puts it on the pull request (run 35788998843 above shows the run/PR association in this repo). That a dispatch would not appear as the PR's check follows from the same binding — a workflow_dispatch run's head sha is the branch tip, and the refresher's own event == "pull_request" filter relies on that distinction. Not verified: I did not execute a re-run, so "replaces the check in place" is documented behaviour here, not observed; and the exact status code for re-running an in-progress run is from community reports, not from a response I made. docs.github.com is blocked by this session's egress proxy, so the documentation quotes above come from search results rather than from pages I fetched.

Out of scope, noted only: compat-window.sh changed in this push too, and the diff is three "nightly" → "run" comment edits with no code change.


Generated by Claude Code

…ated PR

Six findings against the previous push, all verified before acting on.

CONCURRENCY (the blocking one). `compat.yml`'s group was
`${{ github.workflow }}` with no ref, which was harmless while it ran only
on a schedule. With `pull_request:` it is not: GitHub holds one run per
group and cancels any previously pending one when a newer arrives, so a
release pull request's hour-long run made every other pull request's compat
run pend behind it, and the next arrival cancelled the pending one. A
release pull request could therefore lose its compat check to an unrelated
pull request. Keyed by `head_ref || ref` now, matching the five other
workflows here; `compat-refresh.yml` keeps a ref-less group deliberately,
as a singleton, and says so rather than looking like the same oversight.

That also falsifies what the gate's comment claimed. An ordinary pull
request's jobs do skip -- measured on run 35788998286, both `skipped` --
but its run is still created and still takes a queue slot, so "costs an
ordinary pull request nothing" was wrong about the part that mattered.

A FAILED `gh run rerun` ABORTED THE LOOP. Reproduced against a stubbed
`gh` with three pull requests: the first refused, the other two were never
attempted, `::endgroup::` never closed and the tally never printed. Each
pull request is independent, so a failure is now counted and the loop
carries on, exiting non-zero at the end. The same reasoning applies to a
failed `gh run list`, which had the same abort and now behaves the same
way -- fixing one and leaving the other is the partial guard AGENTS.md
rule 5 warns about.

RE-RUNS EXPIRE. GitHub allows a re-run for 30 days after the original run,
and at most 50 times, so "re-runs it daily" was unqualified in three
places. There is no fix inside this design -- a fresh dispatch would put
the answer where nobody is looking, which is the trade this shape already
rejected -- so the refusal is now loud and the ceiling is stated in the
refresher's header, `compat.yml`'s, and the test's doc comment.

The age is reported as diagnosis, never as control: the attempt is still
the only authority on whether a re-run was allowed. A first draft appended
the ceiling to every failure, so a run refused for being already in
progress was reported as one day old and then told the reader about a
30-day limit. A reason offered for the wrong failure is worse than none.

`actions: write` IS WIDER THAN THIS USES. The comment said it "starts
nothing new and can reach nothing this repository does not already run".
False: it is also the scope for `POST /actions/workflows/{id}/dispatches`,
and `workflow_dispatch` is one of the two events exempt from the rule that
GITHUB_TOKEN-triggered events start no run -- so this token could start
`release.yml`. GitHub has no narrower scope; what bounds the job is the
fixed commands it runs, not the size of the permission.

"NO COMPAT RUN AT THIS SHA" WAS INVISIBLE. It was a log line inside a
collapsed `::group::` in a green run, which is the same as not saying it.
`::warning::` now.

Verified: the eight behavioural paths (list failure, empty list, per-branch
list failure, missing run, refused re-run first of three, all-succeed,
refused-and-40-days-old, refused-and-1-day-old) against a stubbed `gh`;
both files parse; `gofmt` clean; all seven guards pass; `go vet -tags
compat` clean.

The sixth finding is not acted on: a fork pull request with a `release/*`
head passes the gate. Forks are not this repository's threat model -- the
only one is our own archived `forge-2` -- and a gate against them was
removed on purpose in d4e95ea. Do not re-add one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 22, 2026
…nother PR

Six findings, one blocking, all verified before acting. The blocking one
generalizes past this PR: a workflow-level concurrency group with no ref in
it is harmless on a schedule and not once `pull_request` is added, because
GitHub cancels the previously pending run in a group when a newer arrives.

Also records a near miss with this wiki itself. I read these pages from a
checkout 31 commits behind, concluded they were stale, and rewrote them --
the non-fast-forward refusal is the only thing that stopped a day of
updates being clobbered. Compounded by a push check piped into `tail`,
which reported success on a failed push. Both worth not repeating.

main is 4611cbc; the page still said 699f929, one merge back.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

@Peeja Peeja left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[From Claude:] Round seven, scoped to d18b640b..2864c295 only. Two findings, both non-blocking. Every behaviour I could execute is correct: all five loop paths complete, ::group::/::endgroup:: balance on all of them, the tally prints, exit status is non-zero exactly when failed > 0, and every age-diagnosis edge case degrades safely. Both findings are claims of fact in compat-refresh.yml comments that are wrong, and in each case the wrong claim is the stated justification for the code beside it. The code needs no change; the reasons do.

1. "it only ever runs from the default branch" is false — workflow_dispatch takes a ref, which inverts the argument for the ref-less group

.github/workflows/compat-refresh.yml:67-70

# DELIBERATELY REF-LESS, unlike every other workflow here. This one is a
# singleton by intent: it only ever runs from the default branch, and two
# refreshers re-running the same pull request's run would race for it.

schedule: does always run on the default branch. workflow_dispatch: does not. The workflow file must be on the default branch to be dispatchable, but the run happens on whatever ref is selected — from content/actions/how-tos/manage-workflow-runs/manually-run-a-workflow.md:

  • l.46 — "Select the Branch dropdown menu and click a branch to run the workflow on."
  • l.74/77 — "To run a workflow on a branch other than the repository's default branch, use the --ref flag." / gh workflow run WORKFLOW --ref BRANCH
  • l.90 — "When using the REST API, you configure the inputs and ref as request body parameters."

Why this is not a nitpick. If the premise were true, github.ref would be refs/heads/main on every run of this workflow, a ref-keyed group would be character-for-character equivalent, and ref-lessness would be a style choice. It is not. A dispatch on a non-default ref is precisely the case where a ref-keyed group would put two refreshers in different groups and let them run concurrently — the race the very same sentence says must not happen. The group is load-bearing, and the comment describes it as merely redundant. A reader who believes the premise has a clean argument for making this file match the other six and reintroducing the race.

2. "the one refusal whose cause the API error never names" is backwards — the age refusal is the one 403 GitHub does name, and gh prints it verbatim

.github/workflows/compat-refresh.yml:166-168

# AGE IS DIAGNOSIS, NOT CONTROL. The attempt above is the only
# authority on whether a re-run was allowed; the age is carried to
# explain the one refusal whose cause the API error never names.

GitHub answers a too-old re-run with 403 {"message":"Unable to retry this workflow run because it was created over a month ago"}, and gh run rerun <run> forwards that message verbatim. cli/cli pkg/cmd/run/rerun/rerun.go on trunk, l.208-210 — the run path, which is the one this script uses:

var httpError api.HTTPError
if errors.As(err, &httpError) && httpError.StatusCode == 403 {
    return fmt.Errorf("run %d cannot be rerun; %s", run.ID, httpError.Message)
}

The --job path at l.230-232 is the one that discards the message (fmt.Errorf("job %d cannot be rerun", job.ID)), and this script never uses it. So the claim is exactly inverted: the age refusal is the 403 whose cause survives, and it is the other 403s that arrive unexplained.

In practice the log already reads, in this order:

run 1003 cannot be rerun; Unable to retry this workflow run because it was created over a month ago
::error::#3 (release/ingot): could not re-run compat run 1003. That run is 45 days old. PAST THE 30-DAY RE-RUN CEILING: ...

The block still earns its place — it converts GitHub's vague "over a month" into the documented 30-day rule, gives the exact day count, and states the consequence for the check, none of which gh says. But it is not supplying a cause that was missing, and the next person to touch it will reason from the reverse of what gh does.

Verified, and what checked out: concurrency semantics, the 30/50 ceilings, the permissions claim, run 35788998286, and the loop executed against a stubbed gh

Concurrency. data/reusables/actions/actions-group-concurrency.md: "Any existing pending job or workflow in the same concurrency group, if it exists, will be canceled and the new queued job or workflow will take its place." That confirms "cancels any PREVIOUSLY PENDING one", and with it the whole round-six scenario — release PR B pending behind release PR A's hour-long run, cancelled outright when an unrelated PR's compat run arrives. The fix is correct.

  • github.head_ref || github.ref is valid in a concurrency group (the github context is permitted there; the docs' own fallback example is github.head_ref || github.run_id). head_ref is populated on pull_request and empty elsewhere, so a dispatch falls through to ref. Each PR does get its own group.
  • Re-runs land back in the same group: re-run-workflows-and-jobs.md — "The workflow will also use the same GITHUB_SHA (commit SHA) and GITHUB_REF (git ref) of the original event that triggered the workflow run."
  • cancel-in-progress: false with a twice-pushed release branch does not let the older result overwrite the newer check. Check runs are keyed to head sha, so the superseded run reports against its own sha and the head's run against the head; the PR's check for the head cannot be clobbered by the earlier one. The cost is latency only — a queued run waits out the in-progress one, up to the 60-minute cap. true would be actively worse here: a push could cancel an in-flight refresher re-run, and since a re-run is a new attempt of the same run, that flips the PR's existing check to cancelled.
  • Residual, and I do not think it is worth changing: a release PR's run can still be cancelled by activity on its own ref (run pending behind an in-progress run, then a third arrival). In every ordering I worked through, the run that gets cancelled is one for a superseded sha, so no check anyone reads is affected.

Ceilings. re-run-workflows-and-jobs.md intro: "up to 30 days after its initial run"; body: "A workflow run can be re-run a maximum of 50 times. This limit includes both full re-runs and re-runs of a subset of jobs." Both numbers are right.

Permissions. data/reusables/actions/actions-do-not-trigger-workflows.md: "workflow_dispatch and repository_dispatch events always create workflow runs." So the exemption claim holds, and release.yml is workflow_dispatch-only, so "this token could start release.yml" holds too. The correction over the previous "starts nothing new" is right.

Run 35788998286. Exact. event: pull_request, head branch claude/compat-polyrepo (not release/*, so an ordinary PR), both jobs conclusion: skipped — "Resolve the published window" and "Version skew" — run conclusion skipped, created 21:50:29Z, updated 21:50:30Z. The one-second lifetime also supports the new "the run is still created and still joins the group" line.

The loop, executed rather than read. Extracted the run: block, stubbed gh so the real --jq filters run through real jq against fixture JSON.

  • All five paths in one run — rerun ok / no run / rerun refused aged-out / run list fails / rerun refused recent: the loop completes over all of them, ::group:: 5 = ::endgroup:: 5, tally prints 5 open release pull request(s): 1 with no run to re-run, 3 that could not be refreshed., exit 1. Nothing aborts early; a non-release/* PR is filtered out.
  • Exit status is non-zero exactly when it should be: all-success → 0, missing-only → 0 (so missing really does not fail the run, as documented), pr list fails → 1, no release PRs → 0.
  • The --jq expression on an empty array emits a bare tab (first is null, and null.databaseId is null, so // "" applies to both), which splits to empty run and empty created → the missing branch, correctly. createdAt: null gives 123\t → age diagnosis skipped. Neither databaseId (integer) nor createdAt (RFC3339) can carry a tab or newline; were one to, the split degrades into an unparseable created, which date then rejects into why="".
  • Age: null and garbage! both leave why empty with no crash; a +02:00 offset parses correctly (40 days); 29d23h59m → "29 days old" with no ceiling; 30d1m → ceiling. Truncating division can only under-report, so the ceiling sentence never fires below 30 days, and at or above 30 days the age refusal is the only one GitHub can give — it cannot be attached to the wrong failure.
  • No set -e escape anywhere: every failing command sits in an if condition or an && test, and failed=$((failed + 1)) is an assignment rather than a bare (( )), so it cannot return 1 on a zero result.
  • set -u: why is re-initialised each iteration, and created_s is read only inside the branch its successful assignment guards, so a stale value from an earlier iteration is unreachable.
  • Confirmed "age is diagnosis, not control": nothing branches on days except the message text. failed increments regardless.

smelt/tests/compat/compat_test.go — the two added lines claim only that re-runs are allowed for 30 days and that a PR open past a month keeps an answer nothing refreshes. Both true.

Both workflows parse as YAML. There is no workflow linter in .github/scripts/, so nothing in guards checks these files.


Generated by Claude Code

Two findings, both verified against primary sources before acting. The code
is right in both cases; the stated reason was wrong in both, and in a way
that would mislead whoever changes it next.

THE REF-LESS CONCURRENCY GROUP said it was safe because "it only ever runs
from the default branch". False, and it argued the group was redundant when
it is load-bearing. Only `schedule:` is default-branch-only; a dispatch runs
on whatever ref is picked -- `gh workflow run --ref`, the UI dropdown, or
`ref` in the REST body -- so a ref-keyed group would put a dispatch on a
branch in a different group from the scheduled run and let both go at once.
That is the collision the group prevents. The comment now says what this job
actually contends for, which is every open release pull request's run rather
than any ref.

THE AGE DIAGNOSIS said it explained "the one refusal whose cause the API
error never names". Backwards. GitHub answers that refusal with "Unable to
retry this workflow run because it was created over a month ago", and
cli/cli's whole-run rerun path forwards the server message verbatim
(`run %d cannot be rerun; %s`); it is the `--job` path that replaces it with
a generic string, and that is a different command. So the reader already
learns why. What the block adds is the exact age, 30 days in place of a
vague "over a month", and the consequence -- that this pull request's compat
check can no longer be refreshed at all. It earns its place for that, not
for the reason given.

Comments only: every changed line is a `#` line, so nothing a check runs can
behave differently. Re-ran the six behavioural paths anyway -- exit status,
group/endgroup balance and the tally are unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 22, 2026
Two findings, both a correct piece of code defended by a false premise --
the refresher's concurrency group ("it only ever runs from the default
branch": only `schedule:` is) and the age diagnosis ("the refusal whose
cause the API error never names": GitHub names it, and gh forwards the
message on the whole-run path). Neither changed behaviour; both would have
misled whoever touched the code next.

The fixing push is comments only, which is rule 10's skip condition as #22
amended it, so seven is the last round. #18 stays Open on 1691b59.

Also records what made rounds six and seven productive after five that had
stopped finding things: each was scoped to a single push rather than the
whole PR, and each push contained shell that was executed rather than read.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 23, 2026
First item off the overnight list. TestPeriodicRotator bets on getting six
1ms ticks inside a 30ms sleep; under a starved scheduler it loses. 0 in 240
at GOMAXPROCS=4, 2 in 480 at GOMAXPROCS=1 across 24 concurrent -race
processes. Fixed with a deadline; 0 in 480 after.

Records the correction I made to my own PR body before anyone read it: I
published "fails every time under -race" by copying it from MONOREPO_TODO
rather than measuring, then measured 1 in 10 and thought I had caught the
note out. Both are true -- the race detector halts the binary at first
detection, so it is every time per process and about 5% per run. Rule 7
applies to premises in notes I wrote myself, not only to review findings.

#18 is green on 28/28 and waiting on a merge.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 23, 2026
Round one replaced a shell guard that compared sets instead of pairs. Round
two found the replacement under-reading its inputs -- the same shape one
layer down, and both were proved with the real client rather than argued.

`${X?msg}` IS ALSO A REQUIRED FORM. `${X:?msg}` errors when X is unset or
empty; `${X?msg}` errors when unset. Compose rejects either identically --
"required variable NEWTHING_IMAGE is missing a value" -- and the pattern
matched only the first, so the other spelling of the same line left this
test green. Now `:?\?`.

THE WALK KEYED ON THE FILENAME compose.yml. The root file pulls its parts in
with `include:`, whose paths it names explicitly, so an included file called
anything else was invisible -- and compose.yaml is compose's own
auto-discovered default. Now every .yml and .yaml under the smelt root.
Over-reading is the safe direction and costs nothing measurable: across every
YAML file under that root -- grafana dashboards and service configs included
-- the pattern matches only compose files.

Verified: all three mutations that previously passed now fail (the no-colon
form in smelt/compose.yml, an included file named newsvc.yml, a
systems/compose.yaml), and the committed tree still passes.

Also: `systems/*/compose.yml` in the PR body was said to match seven. It
matches thirteen files and reaches none of the indexer's -- INDEXER_IMAGE is
at systems/indexing/indexer/compose.yml, a level deeper. That is the same
nested-path miscount #18's body records as having caught three rounds, made
again in the body of the pull request that fixes the nesting. The body now
states the number without the glob that does not produce it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 23, 2026
The Go test that replaced round one's shell guard still recognised less than
compose does: it matched only `${X:?msg}` and not `${X?msg}`, which compose
rejects identically, and it keyed its walk on the filename compose.yml, so an
included file with another name or a compose.yaml was invisible. Both proved
with the real client rather than argued. Now `:?\?` over every .yml and .yaml
under the smelt root.

The body also repeated #18's nested-path miscount -- "seven across
systems/*/compose.yml", a glob that matches thirteen files and reaches none of
the indexer's -- in the pull request that fixes the nesting.

And the round caught Current State: it said #18 was the only open forge pull
request, and a later paragraph still listed #19, which merged and is #23's
base. Both corrected; the open pair is now #18 and #23.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
RELEASE.md's "path a change takes" framed development and release as steps 1
and 2 of one sequence, which reads as though every change needs two pull
requests. They are separate CYCLES: an ordinary change never leaves the
development one, and the release cycle bundles every change merged since that
module's last release. Rewritten that way.

Dropped compat-refresh.yml's paragraph defending the ref-less concurrency
group, and the sentence in compat_test.go explaining that the opposite-
direction test used to exist. Both were explaining rather than saying.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 23, 2026
Petra's call on #18. Per service keeps the release/<service> convention and
per-service semver, and the ordering falls out: when two services must be
released in sequence, only the first is green on compat, so that is the one to
release and releasing it turns the other green.

Also records that git log <service>/vX.Y.Z..main -- <service>/ is enough for
now, and that intra-module dependencies from in-repo libraries may or may not
need more -- separate modules may make it simply iterative. To settle when it
is built.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Comment thread RELEASE.md Outdated
Comment on lines +10 to +11
itest. Agent review rounds run until one comes back with nothing (`AGENTS.md`
rule 10). Merge when it is green. An ordinary change never leaves this cycle.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The agent stuff is just for my process during the initial setup we're in. We can ignore it here, as this will survive. What's meaningful here is that there's a review process; how much is agents and how much is human is not necessary to specify here and subject to change. Just call it a review.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[From Claude:]

Done in c0489ce2 — now just "It is reviewed." The AGENTS.md rule 10 citation went with it.

Good catch on the durability argument: AGENTS.md says of itself that the agent-review practice "lasts only until the rest of the team is reviewing", so citing it from a file meant to outlive the setup was pointing the long-lived document at the short-lived one.


Generated by Claude Code

Comment thread RELEASE.md Outdated
Comment on lines +10 to +11
itest. Agent review rounds run until one comes back with nothing (`AGENTS.md`
rule 10). Merge when it is green. An ordinary change never leaves this cycle.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An ordinary change never leaves this cycle.

This doesn't make sense. No change leaves this cycle, and leaving the cycle isn't a sensical thing to say about a change. All changes go through this cycle, and all changes eventually end up in the larger release cycle. I think we just cut this sentence.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[From Claude:]

Cut in c0489ce2. You're right that it was incoherent — changes don't travel between cycles at all; every change goes through development and every change ends up in some release.

What the sentence was reaching for is that an ordinary change needs no release pull request of its own, and that's already carried by the release cycle being per module and bundling whatever has accumulated. So it's cut rather than rephrased.


Generated by Claude Code

…ense

The agent-review machinery is scaffolding for the current setup; RELEASE.md
outlives it, and how much of a review is agent and how much is human is neither
meaningful here nor stable. Now just "It is reviewed."

"An ordinary change never leaves this cycle" was incoherent, as Petra points
out: no change leaves the development cycle, and changes do not travel between
cycles at all. Every change goes through development and every change ends up
in some release. Cut rather than rephrased -- what it was reaching for (that an
ordinary change needs no release pull request of its own) is already carried by
the release cycle being per module and bundling what has accumulated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
The gate was vacuous. compat-window.sh set pinnable=true when ANY service
could be pinned; TestRollingUpgrade needed EVERY service pinned; five of the
eight publish no version-tagged image. So the job ran, the test skipped, `go
test` exited 0, and the check went green in 0.126s having booted nothing --
two conditions differing by one quantifier, one in YAML and one in Go, with a
green check in the gap.

Those five now take `:main` as their baseline. The objection the skip was
defending against -- ":main moves, and pinning a baseline to something that
moves is how a red becomes ambiguous" -- is answered where it arises instead
of by not running: the script resolves :main to its INDEX digest (AGENTS.md
rule 2; all eight are 4-entry OCI indexes, so a per-arch digest would break
one architecture silently) and emits that. The run still faces whatever :main
is when it asks, so the answer stays fresh; the digest is in the run's log,
output and job summary, so a red is reproducible afterwards.

What the fallback does not buy is skew for those five. `:main` is roughly this
tree -- measured against upstream today: hilt 1 commit, piri 1, ingot 6, the
rest 0 -- so they contribute almost none. That is the right outcome rather
than a compromise: we deploy those five ourselves and can upgrade them
together, so skew between them is a scheduling problem. The skew this suite is
about comes from piri and ingot, which third parties run, and those two have
releases to pin to (0.2.4 and 0.0.0, both ~112 commits back).

A GIT TAG IS NOT A RELEASE HERE, which is worth recording because it looks
like one. hilt and swarf both carry v0.0.0 in git; neither published an image
under it. hilt's is on the commit that ADDED version.json, swarf's is on its
root commit six weeks before it had one, and ingot's v0.0.0 is the only one of
the three that reached the registry. sprue, delegator and piri-signing-service
have no git tag at all, though their version.json files name v0.0.6, v0.1.1
and v0.1.0. The registry is the only thing that answers the question this
suite has.

So there is now no skip path at all, and one place decides: the script
produces a complete baseline set or fails. Dropped with it: `pinnable`, the
`windows` output and the `window` dispatch input, all of which lost their last
reader when TestPinnedPeer was cut. Renamed to compat-baselines.sh, since it
no longer computes a window.

The Go side fails rather than skipping if EVERY baseline is floating -- that
is the one state where the fleet is the same code as HEAD and a green would
mean nothing -- and the message says how to resolve a real set.

check-compat-baselines.sh drives the script against a stub registry. Every
interesting thing it does is a refusal, and a healthy registry takes the
success branch every time, so without this they are prose. Verified both
directions: seven mutations, each detected by the test named in the guard's
header, control clean. TestImageFor covers the one thing imageFor can get
wrong that nothing else would catch until a container timed out forty minutes
later -- a digest joined with `:` instead of `@`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
That commit switched compat-refresh.yml from re-running the pull request's own
compat run to dispatching compat.yml at the release branch; this file's header
was not updated with it, so it still explained why a re-run is better than a
dispatch and still cited the 30-day re-run ceiling as a live limitation. Both
are now false, and the ceiling is the one a reader would act on.

Also rewraps a line this branch's previous commit left at 108 characters, and
two in compat-baselines.sh.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 23, 2026
Petra's reading found what seven review rounds had not: compat.yml ran the
job when ANY service could be pinned, TestRollingUpgrade needed EVERY service
pinned, and five of eight publish no version-tagged image -- so the job ran,
the test skipped, and the check went green in 0.126s having booted nothing.

Records the answer to her v0.0.0 question (hilt and swarf are tagged in git
and published nothing under it; the tags sit on the commit that added
version.json and on swarf's root commit), the fix as pushed, and the one
decision taken on her behalf: the fallback emits the index digest :main
resolves to rather than the bare tag, so a red stays reproducible. What would
flip it, and how to revert, are in the entry.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

Peeja commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

[From Claude:] The dispatched run is in, and the gate is no longer vacuous — it ran for 3m28s instead of 0.126s, and it went red for a real reason.

Run 35899900855 on 44fa2b51. Resolve the baselines green; Version skew red.

Verdict: the :main fallback works. The two services with real releases are the ones that fail, and they fail for the same reason in both directions.

What failed, and it is symmetric
TestRollingUpgrade/upgrade_piri  (157.23s)
  compat_test.go:338: failed to create stack: compose up:
    container smeltery-…-upgrade_piri-ingot-1 is unhealthy

TestRollingUpgrade/upgrade_ingot (49.54s)
  compat_test.go:338: failed to create stack: compose up: dependency failed to start:
    container smeltery-…-upgrade_ingot-piri-0-1 is unhealthy

Exactly two containers were unhealthy across both subtests, and each is the release-pinned peer — ingot:0.0.0 in one, piri:0.2.4 in the other. Their own logs say why:

ingot:0.0.0   ingot: invalid config:
              - root_access and root_secret are required

piri:0.2.4    FATAL cmd cli/root.go:28  unknown flag: --plc-directory
              Error: unknown flag: --plc-directory
              Usage: piri init [flags]

Neither is a wire-compatibility failure. Both are launch-contract failures: smelt boots the baseline images with HEAD's entrypoints and config templates, and that surface has moved.

  • --plc-directory is passed by smelt/systems/piri/entrypoint.sh:76, added 2026-07-22 (45063e56, "feat: integrate Hilt (Resync every subtree to its upstream main #14)"). piri:0.2.4 is from 2026-04-01, three and a half months earlier, and its init has no such flag.
  • root_access / root_secret were removed from ingot on 2026-09-14 (54bed17f, "refactor: remove ingot root creds (#42)"). ingot:0.0.0 is from 2026-07-13 and still requires them; nothing at HEAD supplies them — grep -rn 'root_access\|RootAccess' ingot/ is empty.
What did work, which is the part this pull request is about
  • All five floating baselines pulled and booted. Zero manifest unknown / pull access denied / not found: manifest in the whole log, and none of the five appears in the unhealthy list. <pkg>@sha256:… is a reference the stack handles.
  • The classification is right, logged before anything booted:
    compat: 5 of 8 baselines are a floating :main (delegator hilt signing-service
    swarf upload) -- those services have cut no release, so they contribute little
    skew; the skew under test comes from indexer ingot piri
    
  • TestImageFor passed, all six cases, in 0.00s before the stack.
  • Exactly one service built from local source per subtest, everyone else on their resolved image — the property the whole test rests on.

What this means, and the decision it raises

This is not a regression from this pull request. It is what the vacuous green was hiding: the suite never got far enough to try.

The compat suite cannot pin a baseline older than smelt's own launch contract. The premise — boot old images with the current harness — holds only while flags, env and config keys are stable across the version window. They are not. The usable window opens at the last launch-contract change per service:

service usable from only release gap
piri 2026-07-22 0.2.4, 2026-04-01 ~16 weeks too old
ingot 2026-09-14 0.0.0, 2026-07-13 ~9 weeks too old

So there is no released image of either service that this harness can launch. The gate can produce a true red — it just did — but it cannot produce a meaningful green today. The two shapes available are both wrong: all-:main is vacuous (and the new t.Fatalf refuses it), and the real releases predate the launcher.

Not decided here, because it shapes the gate. Three readings:

  1. Version the launch contract — smelt learns which flags and config each baseline needs. Correct, unbounded, and grows with every window.
  2. Shrink the window to what the harness can launch — baselines become recent sha-* images rather than the two ancient tags. This is what the earlier proof-of-concept did. It gives a real, smaller skew signal and works today, at the cost of "a version-shaped tag is a cut release", which is the property the baseline rule was built on.
  3. Accept that the gate is blocked until fresh releases are cut, which is Phase 1 anyway. Honest, and costs nothing now — compat.yml runs only on release/* heads and on dispatch, and there are no open release pull requests, so nothing is blocked today. It would block the first release pull request.

Recommendation: 3 as the position, with 2 as the interim if the lead time matters. What settles it is whether a release cut next week is meant to be gated by compat at all, or whether compat starts gating from the release after it.


Generated by Claude Code

Peeja pushed a commit that referenced this pull request Sep 23, 2026
…rness

The dispatched run is the first time TestRollingUpgrade has booted anything.
It went red, and not because of this change: smelt starts baseline images with
HEAD's entrypoints and config templates, and both real releases predate that
surface. piri:0.2.4 does not know --plc-directory, which the entrypoint has
passed since 2026-07-22; ingot:0.0.0 still requires root_access/root_secret,
which ingot dropped on 2026-09-14.

The :main fallback is vindicated by the same run: all five floating baselines
pulled and booted, no registry errors, and none of them is in the unhealthy
list. The two failures are both release-pinned peers.

So the gate can produce a true red but not a meaningful green, and the three
ways out are a decision for Petra rather than mine. Recorded with the
recommendation and what would settle it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
… hold

Nine findings from the round on 1cdf10c/44fa2b51, all fixed here. Three are
about this guard being weaker than it says it is, which is the class it exists
to prevent.

TEST 5 WAS PASSING ON A DIFFERENT GUARD THAN THE ONE IT IS NAMED FOR. The
header claimed "the non-403 -> could not read exit -> FAIL (test 5)". Delete
that arm and the guard stays 9/9 green: a 500 body falls through the case into
the token parse, which fails, and the NEXT guard down returns 1 with a
different message -- which test 5's grep for the downstream ::error:: still
matched. It now matches the token endpoint's own message, and both mutations
(delete the arm, and replace it with `return 10`) are caught.

TAGS/LIST'S NON-200 CHECK HAD NO TEST AT ALL, and it is the fail-open half of
the "no releases is not could not ask" defence: a token that succeeds and a
tags/list that rate-limits silently demotes that service from a release
baseline to a floating one, the run stays green, and the only trace is a
::warning:: naming it beside the services that float legitimately. The
partial failure is invisible to the Go side's all-floating check too. Test 5b
covers it.

THE ONLY OUTPUT THE WORKFLOW CONSUMES WAS NEVER ASSERTED: replacing
`baselines=$(as_json ...)` with `baselines={}` left the guard green, because
every assertion read the base_* lines the script's own header calls "nothing
consumes it". Tests 1-3 now assert the JSON. Test 10 covers the pagination
loop, which no stub reached -- and the order is neither lexicographic nor
version order, so a loop that stops after one page picks whatever came first.

A LITERAL `main` BASELINE COUNTED AS A RELEASE. isDigest's complement made
every non-digest a release, including `main` -- which this file assigns two
ways, in the unset fallback and in TestImageFor's floating-tag case. One
hand-set COMPAT_BASELINE_PIRI=main put all eight on :main and still satisfied
the all-floating fatal, which is precisely the vacuous run it refuses.
Classification is now positive, on versions_from's own pattern, which also
closes `sha-abc1234`, `latest` and `main-dev`.

A MISSING WORKSPACE WAS STILL A GREEN SKIP -- the last vacuous path left after
the baseline skip went. Now fatal, and compat.yml asserts GOWORK is on before
running, so the diagnosis names the knob rather than the symptom.

Also: the pagination comment justified taking only `last` with "GHCR's own
next link carries n=0", which is false for the request this script makes.
Measured: GHCR echoes the n you sent (?n=100 -> n=100, ?n=5 -> n=5) and
answers n=0 only when you send none. Behaviour unchanged, reason corrected.

AGENTS.md said CI is four workflows plus a dispatch-only fifth. This branch
makes it seven, and compat.yml triggers on pull_request: -- where both its
jobs skip, which is not free, as compat.yml's own concurrency note says.

And the comment naming the tag kinds those five publish said ":main and
:sha-*"; hilt, sprue and swarf also publish :main-dev and :sha-<sha>-dev.

Verified both directions: eleven mutations, each caught by exactly the test
the header names, control clean. COMPAT_BASELINE_PIRI=main now fatals;
GOWORK=off now fatals; the real baseline set still classifies 5 of 8 floating.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

@Peeja Peeja left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[From Claude:] Review round on 1cdf10c7/44fa2b51: not satisfied — 10 findings, all now fixed in 1e1fe5e9. One blocking (the dispatched run came back red and the body said it was still outstanding), three about this guard being weaker than its own header claims, one a real fail-open path, and one that defeats the guard the whole change exists to add.

The three that matter are all the same shape: a check that passes for a reason other than the one it is named for. That is the class this PR added a guard to prevent, happening to the guard.

1 — BLOCKING: the dispatched run came back red, and the body still said it was queued

Run 35899900855 failed after 3m30s. Both subtests red, and neither for wire skew: smelt boots baselines with HEAD's entrypoints, and both release baselines predate that surface — piri:0.2.4 does not know --plc-directory (passed unconditionally by smelt/systems/piri/entrypoint.sh:76, landed upstream in 16880146, 2026-07-21, ~4 months after the tag); ingot:0.0.0 still requires root_access/root_secret, removed 2026-09-14.

Written up in full here, with the three ways out and a recommendation. Not fixed in code, because it is a decision about what the gate is. The body now says the run came back and came back red.

2 — test 5 was passing on a different guard than the one it is named for

The header claimed the non-403 -> "could not read" exit -> FAIL (test 5). Delete that arm and the guard stays 9/9 green:

### token_for non-403 arm DELETED   →  guard rc=0, 9 ok, 0 FAIL

A 500 body falls through the case into the python3 … ["token"] parse, which fails, and the next guard returns 1 — so test 5's grep for the downstream ::error:: still matched. Test 5 now matches the token endpoint's own message (token endpoint answered HTTP 500) and the downstream one, so both mutations are caught:

token_for non-403 arm DELETED       caught by: 5
token_for non-403 -> return 10      caught by: 5
3 — the fail-open half of the defence had no test at all

tags_for's non-200 check on tags/list. Every stub answered 200, so nothing reached it. Deleting it reproduces the exact || true failure the script exists to close, and the guard stayed green.

The dangerous shape is the partial failure, which the Go side's all-floating check cannot see either: one service's tags/list rate-limiting silently demotes it from a release baseline to a floating one, the run stays green, and the only trace is a ::warning:: naming it beside the five that float legitimately.

Test 5b covers it — a stub whose token succeeds and whose tags/list answers 429.

4 — the only output the workflow consumes was never asserted

Every assertion read a base_* line, which the script's own header describes as "readable form; nothing consumes it". Replacing echo "baselines=$(as_json "$baselines")" with echo "baselines={}" left the guard 9/9 green.

Tests 1–3 now assert the JSON. Test 10 covers the pagination loop, which no stub reached — and since the tag order is neither lexicographic nor version order (ingot's first page of 100 holds one of its 117 version-shaped tags), a loop that stops after one page picks whatever came back first.

5 — a literal main baseline counted as a release, defeating the all-floating guard

isDigest's complement made every non-digest a release — including main, which this same file assigns two ways: the unset fallback, and TestImageFor's floating-tag case.

$ COMPAT_BASELINE_PIRI=main go test -tags compat -run TestRollingUpgrade
compat: 7 of 8 baselines are a floating :main … the skew under test comes from piri
=== RUN TestRollingUpgrade/upgrade_piri     (proceeds)

Every service on :main, and the run proceeds because one of them said so with a bare tag. Classification is now positive, on versions_from's own pattern, which also closes sha-abc1234, latest and main-dev. After:

--- FAIL: TestRollingUpgrade
    every baseline is a floating :main (map[… piri:main …]), so the old half of
    the fleet is the same code as HEAD and a green here would assert nothing.
6 — the pagination comment's justification is false for the request this script makes

It said "GHCR's own next link carries n=0, and replaying that asks for a page size this script did not choose." GHCR echoes the n you send; n=0 appears only when you send none, and this script always sends n=1000:

GET ?n=5    → </v2/fil-forge/ingot/tags/list?last=0.0.0-amd64&n=5>;   rel="next"
GET ?n=100  → </v2/fil-forge/ingot/tags/list?last=sha-e6de741&n=100>; rel="next"
GET <none>  → </v2/fil-forge/ingot/tags/list?last=sha-e6de741&n=0>;   rel="next"

Taking only last is still right. The reason was wrong, and is now what was measured.

7 — the body's own table reported a disagreement that is not there

The indexing-service row showed — for its upstream git tag. It has one:

$ git ls-remote --tags https://github.com/fil-forge/indexing-service
refs/tags/v1.13.4

In a table built to show three columns disagreeing, that is the one row claiming a disagreement that does not exist. Corrected in the body. It does not move the five/three counts.

8 — AGENTS.md's workflow count went stale on this branch

"CI is four workflows … A fifth, release, exists but is dispatch-only." This branch makes it seven, and compat.yml triggers on pull_request: — where both jobs skip, which is not free, as compat.yml's own concurrency note says at length. Rewritten to say what runs, what skips, and what that still costs.

9 — a missing workspace was still a green skip, the last one left
$ GOWORK=off go test -tags compat -run TestRollingUpgrade
    no active go.work, so there is no fleet to hold old: …
--- SKIP: TestRollingUpgrade (0.00s)
PASS    rc=0

The 0.126s green in a different costume. Now fatal, and compat.yml asserts go env GOWORK is non-empty before running, so the diagnosis names the knob rather than the symptom.

10 — two tag kinds named where there are four

"only :main and :sha-*" — hilt, sprue and swarf also publish :main-dev and :sha-<sha>-dev. The body's table already had four; the Go comment was narrower than the data behind it.

Verification, and what checked out unchanged

Both directions, eleven mutations, each caught by exactly the test the header names:

CONTROL (must pass)                          passes
403 -> report none and carry on              caught by: 4
token_for non-403 arm DELETED                caught by: 5
token_for non-403 -> return 10               caught by: 5
tags/list non-200 check DELETED              caught by: 5b
digest_for failure message dropped           caught by: 6
digest presence/shape check dropped          caught by: 7 8
digest length check dropped                  caught by: 8
empty-service-list refusal dropped           caught by: 9
pagination: stop after one page              caught by: 10
lexicographic version sort                   caught by: 3
baselines= emitted as {}                     caught by: 1 2 3

Local checks at 1e1fe5e9: all five check-*.sh run here exit 0; gofmt -s -l . empty; shellcheck clean on both scripts; GOWORK=off go vet -tags e2e,compat ./... clean; all three workflows parse.

Re-derived by the round and found correct (so these are not being re-litigated): piri:0.2.4 200 / :v0.2.4 404; all eight :main manifests 200 as application/vnd.oci.image.index.v1+json with 4 entries; the five-without / three-with split; ingot 117 tags with exactly one version-shaped, and exactly one on its first page of 100; upload/indexer/signing-service → 403; hilt's v0.0.0 on the commit that added version.json, swarf's on its root commit with version.json 42 days later; prefix drift hilt 1, piri 1, ingot 6, rest 0; the 0.126s green (run 35886032906, job 107266375561); run 35760677003's 11 check runs all on the dispatched commit; the sha-* tag counts; go list -deps -test | grep -c forge/smelt = smelt 24 and all others 0; go list -tags compat -f '{{.GoFiles}}' = []; e2e 35890887107 and itest 35890887140 green at c0489ce2; compat-refresh 230→131 at cb542109; both compat jobs still skipped on an ordinary pull request at this head; pinnable/windows/window have no remaining readers anywhere in the tree; no temp-file leak on any error path.

A round on 1e1fe5e9 is open.


Generated by Claude Code

Peeja pushed a commit that referenced this pull request Sep 23, 2026
Three of the ten are the same shape -- a check passing for a reason other
than the one it is named for -- which is the defect this PR adds a guard
against, happening to the guard. One of them, tags/list non-200, fails open:
a rate-limited service is silently demoted from a release baseline to a
floating one and the run stays green.

Head bumped to 1e1fe5e.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
…ed for

`go env GOWORK` prints the literal string `off` when GOWORK is off -- it is
empty only when no go.work was found at all. So `[ -n "$w" ]` was true in the
case the step's own comment is written about, and the step printed `GOWORK=off`
and exited 0. pkg/workspace has always tested both (`gowork == "" || gowork ==
"off"`); this now matches it. Measured against the pinned toolchain: off -> rc 1,
no go.work -> rc 1, active workspace -> rc 0.

That is rule 5's partial guard reading as a total one, added by the previous
commit while fixing a finding about a green skip. Both directions were checked
for the Fatalf it accompanies and not for the assertion itself.

FIVE MORE CLAIMS THE GUARD DID NOT COVER, each now a mutation that fails on a
named test:

- EVERY FIXTURE WAS HYPHEN-FREE, so key_for's `-` -> `_` and the `[a-z0-9-]`
  in the service derivation both survived deletion with the guard fully green.
  Two of the eight real services are hyphenated, and that mapping is what
  lines COMPAT_BASELINE_INDEXING_SERVICE up with envSuffix -- the descendant
  of the COMPAT_BASELINE_SPRUE / _UPLOAD mismatch this apparatus exists to
  stop. The `rel` fixture is now `rel-svc`.
- THE INDEX MEDIA TYPES IN THE Accept WERE UNVERIFIABLE, because the stub
  ignored the header. It now answers a different digest when the list types
  are not asked for, so test 2 pins AGENTS.md rule 2's index-not-per-arch
  requirement rather than asserting it in prose.
- versions_from's fullmatch had no fixture with an arch suffix, though ingot
  really publishes 0.0.0-amd64 and -arm64. Added to the sorting fixture,
  where they sort above every real version.
- The whole $GITHUB_STEP_SUMMARY block was unreached -- notable, since the
  ::warning:: two lines above it points readers at that summary for the
  digests. Test 11 covers both arms.
- The argument refusal was unreached. Test 12.

Three corrections to text rather than code. The header's "THE FIRST TWO OF
THOSE" named the wrong pair (it is the second and third). Test 10's comment
inferred "picks whatever came first" from ingot's tag counts, which does not
follow: ingot has exactly one version-shaped tag and it is on page one, so no
page selection changes its baseline -- the risk the loop covers is a tag on a
later page being MISSED, which is what the fixture demonstrates. And AGENTS.md
said "two more exist without costing one" where there are three, and counted
compat among the free ones two sentences before saying it is not.

The stub is threaded now. One run in ~35 failed while eighteen mutation copies
of this file were running, with the script correctly reporting that it could
not reach the stub. 0/30 before and 0/40 after is not proof of cause; a guard
that reddens under load teaches people to re-run it, and a re-run is not a
diagnosis.

Seventeen mutations, each caught by the test its header names; control clean.
The header now also says which guards in the script have NO fixture -- the
transport-failure and body-is-not-JSON arms -- rather than claiming "each".

(The previous commit message said nine findings where the round reported ten.)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

@Peeja Peeja left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[From Claude:] Round on 1e1fe5e9: not satisfied — 6 findings, all fixed in 93a17f6f. One is the fix from the last round being wrong.

All eleven mutations the last round's header claimed were re-verified and hold, each failing on the test it names and on no other. The findings are elsewhere.

1 — the GOWORK assertion passed on exactly the case it was added for

go env GOWORK prints the literal string off when GOWORK is off; it is empty only when no go.work was found at all. So [ -n "$w" ] was true in precisely the case the step's own comment is written about:

$ GOWORK=off go env GOWORK
off
$ env GOWORK=off bash -c '…; w=$(go env GOWORK); [ -n "$w" ] || exit 1; echo "GOWORK=$w"'
GOWORK=off
rc=0

pkg/workspace has always tested both — workspace.go:87 is if gowork == "" || gowork == "off" — so the assertion caught a strict subset of what the t.Fatalf beside it catches, and missed the one case it existed for. That is rule 5's partial guard reading as a total one, introduced while fixing a finding about a green skip.

Now matched to Detect()'s condition. Measured against the pinned toolchain: off → rc 1, no go.work → rc 1, active workspace → rc 0.

2 — every fixture was hyphen-free, so two mappings survived deletion green
tr 'a-z-' 'A-Z_'   ->  tr 'a-z' 'A-Z'          guard rc=0, all tests ok
\([a-z0-9-]*\)     ->  \([a-z0-9]*\)           guard rc=0, all tests ok

Two of the eight real services are hyphenated, and key_for's -→_ is what lines COMPAT_BASELINE_INDEXING_SERVICE up with envSuffix's own ReplaceAll — the direct descendant of the COMPAT_BASELINE_SPRUE/_UPLOAD mismatch this whole apparatus exists to stop. The second mutation silently shrinks the fleet by two, and nothing downstream notices because the Go side's unset fallback quietly assigns those services main.

The rel fixture is now rel-svc. Both mutations are caught by test 1.

3 — the index media types in the Accept were unverifiable

The stub ignored the header entirely, so swapping the two list media types for …image.manifest.v1+json left the guard green — and the index-not-per-architecture requirement is the one thing AGENTS.md rule 2 cares about here.

The stub now answers a different digest when the list types are not requested, so test 2 pins the behaviour rather than the prose.

4 — three more branches with no fixture at all
  • versions_from's fullmatch had no arch-suffixed tag to exclude, though ingot really publishes 0.0.0-amd64 and -arm64. Added to the sorting fixture, where they sort above every real version; fullmatch→match is now caught by test 3.
  • The whole $GITHUB_STEP_SUMMARY block, both arms. Notable, because the ::warning:: two lines above it points readers at that summary for the digests. Test 11.
  • The argument refusal. Test 12.
5 — two claims in the guard's own text pointed at the wrong things

The header's "THE FIRST TWO OF THOSE are one arm and two mutations" named entries 1 and 2, which are two different arms. It is the second and third.

Test 10's comment inferred "a loop that stops after one request picks a baseline from whatever the registry happened to return first" from ingot's tag counts. That does not follow: ingot has exactly one version-shaped tag and it is on page one, so no page selection can change its baseline. The risk the loop actually covers is a version-shaped tag on a later page being missed entirely, silently demoting that service to floating — which is what the fixture demonstrates. Restated.

6 — AGENTS.md's rewritten paragraph miscounted and contradicted itself

"and two more exist without costing one" — there are three (release, compat, compat-refresh), and it counted compat among the free ones two sentences before saying "that is not free". Also "both its jobs are gated on a release/* head branch" is not the mechanism: only the first job carries an if:, and compat.yml:114 says "NO CONDITION, deliberately" about the second, which skips because its needs: skipped.

Rewritten, and every claim re-derived by parsing all seven workflow files.

One thing that was not a finding, and what I did about it anyway

A single run of the guard failed — once, while eighteen mutation copies of it were running — with the script correctly reporting it could not reach the stub. 0/30 immediately after and 0/40 after hardening; that is not proof of cause.

The stub is ThreadingHTTPServer with a raised backlog now. A guard that reddens when the machine is busy teaches people to re-run it, and AGENTS.md is explicit that a re-run is not a diagnosis — so the one line went in rather than the observation being written off.

Verification

Seventeen mutations, each caught by the test its header names, control clean:

CONTROL (must pass)                            passes
403 -> report none and carry on                caught by: 4
token_for non-403 arm DELETED                  caught by: 5
token_for non-403 -> return 10                 caught by: 5
tags/list non-200 check DELETED                caught by: 5b
digest_for failure message dropped             caught by: 6
digest presence/shape check dropped            caught by: 7 8
digest length check dropped                    caught by: 8
empty-service-list refusal dropped             caught by: 9
pagination: stop after one page                caught by: 10
lexicographic version sort                     caught by: 3
versions_from: fullmatch -> match              caught by: 3
baselines= emitted as {}                       caught by: 1 2 3
key_for drops the - -> _ mapping                caught by: 1
service derivation drops the hyphen            caught by: 1
Accept: drop the index media types             caught by: 2
step summary block dropped                     caught by: 11
argument refusal dropped                       caught by: 12

The header no longer says "each guard". The transport-failure arms (curl itself failing in token_for, tags_for, digest_for) and the two body-is-not-JSON arms have no fixture. An unreachable registry was checked by hand — GHCR_API at a dead port exits 1 — but by hand is not by this file, and "each" would be the overclaim rule 5 is about. The header now names them.

Local at 93a17f6f: guard 13/13; the six other check-*.sh run here exit 0; gofmt -s -l . empty; shellcheck clean on both scripts; GOWORK=off go vet -tags e2e,compat ./... clean; all seven workflows parse; a live run against ghcr.io resolves the full baseline set; 40 consecutive guard runs green.

A round on 93a17f6f is open. The launch-contract blocker from the dispatched run is untouched by any of this and is still the decision that gates the gate.


Generated by Claude Code

Peeja pushed a commit that referenced this pull request Sep 23, 2026
…heck

Round one fixed a green skip by adding a t.Fatalf and a workflow assertion.
The assertion passed on exactly the case it was written for: go env GOWORK
prints the literal string off, so [ -n "$w" ] was true. pkg/workspace has
always tested both.

Worth recording as a pattern rather than three slips -- the class is cheap to
introduce, invisible to reading, and the only thing that has caught any of it
is applying the mutation.

Head bumped to 93a17f6.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
…stub

CORRECTION. The previous commit said a guard flake was contention on the test
stub's accept queue and "fixed" it by making the stub threaded and setting
`srv.request_queue_size = 128`. Every part of that was wrong:

  - the assignment runs AFTER the constructor, and socketserver calls
    `listen(self.request_queue_size)` during it, so the backlog stayed 5;
    measured by instrumenting socket.listen -- [5], not [128];
  - `daemon_threads = True` was already ThreadingHTTPServer's class default;
  - and the stub was never the problem.

THE CAUSE IS IN THE SCRIPT UNDER TEST, and it is a real bug rather than test
infrastructure. `newest=$(… | versions_from | head -n 1)`: `head` closes the
pipe after one line, python's print takes BrokenPipeError, `set -o pipefail`
propagates it and `set -e` aborts the whole script with an empty stdout.
Measured on that pipeline in isolation: 0/150 idle, 21/150 with the box
loaded. It can only fire when more than one version-shaped tag is emitted --
which no fleet service does today, so it has never been seen in a real run,
and it arrives with the second release of any service. versions_from is now
newest_version_from and takes the head itself; 0/150 under the same load.

Enumerated the class rather than fixing the instance (rule 5): two more
`| head -n 1` readers, on the pagination Link and the digest header. Both
have writers far below a pipe buffer so neither could realistically fire,
but "could not realistically fire" is what the other one looked like, and
taking the first line in the shell costs nothing. No `| head` left in the
script.

The stub is plain HTTPServer again. Reverted rather than kept with a
justification that did not hold.

FIVE MORE FROM THE SAME ROUND, all of them the guard claiming more than it
checked:

- TEST 11 CHECKED THE ROW'S EXISTENCE, NOT THE DIGEST IN IT -- and the digest
  is the entire reason compat-baselines.sh's ::warning:: points readers at
  that summary. The floating closing note could also be deleted whole, because
  the `floating` grep was already satisfied by the table row itself. It now
  pins the full row, the table header and separator, and each closing note by
  a phrase unique to it. It also drives its own runs: reading other tests'
  byproducts made it fail as collateral whenever one of those failed for an
  unrelated reason.
- THE Accept TEST PINNED THE PAIR, NOT EACH MEMBER. The stub used
  `any(t in accept)`, so dropping either media type -- including the OCI index
  type the measurement rests on -- left it green. `all` now.
- digest_for's manifest-non-200 and token-mint-failure arms had no fixture;
  test 6 reached the first but matched only the caller's message, so deleting
  it left the empty-digest arm producing the same outer text. Tests 13a and
  13b.
- Mutation 11's rationale was wrong about the mechanism: `fullmatch` ->
  `match` makes the script CRASH (`int("0-amd64")`), it does not mis-pick, and
  the fixture's "sorts ABOVE every real version" property did no work. The
  arch pair now sorts below everything and the comment says what happens.
- The `[a-z0-9-]` entry named one test where two go red. Three entries name
  two tests now, and the header says why.

Twenty mutations, each caught by the tests its entry names; control clean;
25/25 green under six-way CPU load. The header also says what this file
CANNOT catch -- the `| head` regression is load-dependent, so a pass here
proves nothing about it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

@Peeja Peeja left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[From Claude:] Round on 93a17f6f: not satisfied — 6 findings, all fixed in b96d59ed. One is a correction to what I posted in the last round.

All seventeen mutations from the previous header were re-applied and hold. The findings are elsewhere — and the first one means the previous round's flake diagnosis was wrong in every part.

Correction to the previous round's comment

The last round's comment ended with a block saying a single guard failure was contention on the test stub's accept queue, and that the stub had been made threaded with a raised backlog. That was wrong three ways, and the "fix" was inert:

  • srv.request_queue_size = 128 runs after the constructor, and socketserver calls listen(self.request_queue_size) during it. Measured by instrumenting socket.listen: the backlog was [5], not [128].
  • daemon_threads = True was already ThreadingHTTPServer's class default.
  • The stub was never the cause.

The cause is a real bug in the script under test, not in test infrastructure:

newest=$(printf '%s\n' "$raw" | versions_from | head -n 1)

head closes the pipe after one line, python's print takes BrokenPipeError, set -o pipefail propagates it and set -e aborts the whole script with an empty stdout. Measured on that pipeline alone:

idle       :   0 / 150 failed
under load :  21 / 150 failed      BrokenPipeError: [Errno 32] Broken pipe

It can only fire when more than one version-shaped tag is emitted — which no fleet service does today, so it has never appeared in a real run. It arrives with the second release of any service. versions_from is now newest_version_from and takes the head itself: 0/150 under the same load.

I asserted a cause I had not established, and the evidence I cited for it ("0/30 before and 0/40 after") was measured idle, which is the one condition under which the bug cannot appear.

1 — the class, not the instance

Two more | head -n 1 readers in the same script: the pagination Link and the digest header. Both have writers far below a pipe buffer, so neither could realistically fire — but "could not realistically fire" is exactly what the other one looked like, and taking the first line in the shell costs nothing. There is no | head left in compat-baselines.sh.

The stub is plain HTTPServer again — reverted rather than kept with a justification that did not hold.

2 — test 11 checked the row's existence, not the digest in it

Three ways to break the summary while the guard stayed green:

  • replacing the floating row's digest with a literal;
  • deleting the whole floating closing note (the grep -q 'floating' was already satisfied by the table row itself);
  • deleting the markdown table header and separator, so the summary stops rendering as a table.

The digest is the entire reason compat-baselines.sh's ::warning:: points readers at that summary. Test 11 now pins the full row, the header and separator, and each closing note by a phrase unique to it — and drives its own runs, because reading other tests' byproducts made it fail as collateral whenever one of those failed for an unrelated reason.

3 — the Accept test pinned the pair, not each member

The stub decided with any(t in accept for t in LIST_TYPES), so dropping either media type left test 2 green — including application/vnd.oci.image.index.v1+json, which is the one the script's own measurement rests on. Only removing both was caught. A guard over part of a chain reading as a guard over the chain, which is the thing rule 5 names.

all now. Both single-type drops are caught:

accept-drop-oci        caught by: 2 11
accept-drop-docker     caught by: 2 11
4 — two more arms in digest_for with no fixture

The manifest-non-200 arm and the token-mint-failure arm. Test 6 reached the first but matched only the caller's message, so deleting it left the empty-digest arm producing the same outer text — green. The second was unreachable: every service that got as far as digest_for received a 200 from /token.

Tests 13a and 13b, with a stub whose /token succeeds for tags/list and is refused on the manifest call.

5 — mutation 11's rationale described something that does not happen

fullmatch → match fails test 3 because the script crashes — "0.0.1-amd64".split(".") gives ["0","0","1-amd64"] and the sort key does int("1-amd64") — not because a per-architecture tag wins the pick. The fixture's stated design property ("sorts ABOVE every real version") did no work at all. The arch pair now sorts below everything on purpose, and the comment says what actually happens.

6 — an entry naming fewer tests than go red

[a-z0-9-] → [a-z0-9] reddened tests 1 and 11, because test 11 read test 1's summary. With test 11 on its own runs that is fixed, but three other entries genuinely go red on two tests, since test 11 drives the sorting and flo fixtures. The header names both for each rather than rounding to one — an entry naming fewer tests than go red is how a reader concludes the wrong thing about which check is load-bearing.

Verification

Twenty mutations, each caught by the tests its entry names; control clean:

CONTROL                passes (correct)      key_for-hyphen         caught by: 1
403-carry-on           caught by: 4          derivation-hyphen      caught by: 1
tok-non403-del         caught by: 5          accept-drop-oci        caught by: 2 11
tok-non403-10          caught by: 5          accept-drop-docker     caught by: 2 11
tagslist-200-del       caught by: 5b         summary-block          caught by: 11
digestfor-msg          caught by: 6          argrefusal             caught by: 12
digest-presence        caught by: 7 8        manifest-non200        caught by: 13a
digest-length          caught by: 8          digestfor-token        caught by: 13b
empty-svc-list         caught by: 9          lex-sort               caught by: 3 11
pagination             caught by: 10         fullmatch              caught by: 3 11
baselines-empty        caught by: 1 2 3

The header also says what this file cannot catch. Putting | head -n 1 back is load-dependent, so a pass here proves nothing about it; the defence is that the pipeline is gone, not that the guard watches for it. The transport-failure and body-is-not-JSON arms still have no fixture, and are named.

Local at b96d59ed: guard 15/15, and 25/25 green under six-way CPU load — the condition the old version failed about 40% of the time under. Six other check-*.sh exit 0; gofmt -s -l . empty; shellcheck clean on both scripts; GOWORK=off go vet -tags e2e,compat ./... clean; all seven workflows parse; a live run against ghcr.io resolves the full baseline set.

Confirmed correct and not re-litigated: the GOWORK assertion is now complete — go env GOWORK can only print an absolute path, empty, or the literal off; every other $GOWORK value makes go env itself exit 1, and set -e catches that. It matches workspace.Detect() exactly. AGENTS.md's paragraph is correct on every claim, checked against all seven parsed workflow files and against the eight pull_request runs of compat on this branch, every one of which concluded skipped.

A round on b96d59ed is open. The launch-contract blocker from the dispatched run is untouched and is still the decision that gates the gate.


Generated by Claude Code

Peeja pushed a commit that referenced this pull request Sep 23, 2026
I asserted a cause I had not established and cited evidence measured under the
one condition where the bug cannot appear. The real cause is a head -n 1 under
pipefail in compat-baselines.sh: 0/150 idle, 21/150 under load. It can only
fire with more than one version-shaped tag, which no fleet service has today,
so it arrives with the second release of any service.

The wrong version stays visible above the correction rather than being edited
away. Head bumped to b96d59e.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Round four is right that the record was wrong, and the part it could not see
is that the faulty input was mine.

WHAT THE FIRST REVISION WROTE DOWN. It said the observed failure was the
script "reporting could not read sorting's tags", and reasoned from that to
contention on the test stub. That string was never seen. Test 3's failure line
printed `$(out_of t3)` and nothing else at the time, so its stderr was never
read -- the message was inferred from the surrounding code and recorded as an
observation. It is also the wrong path: it comes from tags_for returning
non-zero, two steps before the pipeline in question.

WHAT WAS ACTUALLY OBSERVED. One run went red on test 3 alone, rc=1, with empty
stdout, while eighteen mutation copies of the guard were running. At that
commit `sorting` was the only fixture with more than one version-shaped tag
(`paged` had 1.0.0 and main -- one), so "the only test that can hit a
BrokenPipeError is 3" and "only test 3 went red" agree exactly. Restoring the
old pipeline reproduces it under load, on precisely the tests whose fixtures
have two or more.

So the identification stands, but it stands on the rc and the empty stdout,
not on a message nobody read. Round four took the invented symptom at face
value and concluded the cause was unproven -- which is what a fabricated
detail does to everyone downstream of it, and is worse than the original
misdiagnosis. Both versions are now in the comment.

The guard prints stderr on every failure now. Test 3 was one of two `bad`
lines that dropped it, which is the whole reason there was nothing to go on.

Two facts wrong against the tree, both in comments this branch wrote:

- `int("0-amd64")` was derived from a fixture the same commit had replaced;
  `0.0.1-amd64` raises on `int("1-amd64")`. That is inside the comment written
  to correct a previous wrong mechanism claim.
- Two mutation entries and a cross-reference in compat_test.go still named
  `versions_from`, renamed to `newest_version_from` by the commit before this.

And the stub comment no longer says "the flake was never here at all", which
overstated what is known: the stub was never implicated by anything observed,
which is the claim the revert actually rests on.

Re-measured rather than copied: 0/150 idle, 18/150 under six-way CPU load
here, 77/150 on a busier box. The rate is a property of the load.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

@Peeja Peeja left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[From Claude:] Round on b96d59ed: not satisfied — 3 findings, all fixed in f991dcc5. No defect in the shell or Go that ships; all three are claims the files make about themselves.

The round verified every mechanical property of the previous commit and found it correct: newest_version_from is equivalent to the old form across 15 input shapes including empty, ties and no-version-shaped-tags; the two ${v%%$'\n'*} replacements match head -n 1 across 13 input shapes and 9 realistic header files; TOKFLIP leaks no state; all twenty mutations fail on exactly the tests their entries name; and the guard is 0 failures in 165 runs, including 54 as 18 concurrent copies and 61 under eight-way CPU load.

1 — the symptom I recorded for the flake was inferred, not observed

This is the important one, and the part the round could not see is that the faulty input was mine.

The round's argument was that | head -n 1 cannot be the flake, because the symptom the record names — the script reporting could not read sorting's tags — comes from tags_for returning non-zero, two steps before the pipeline in question. The two are mutually exclusive, so it concluded the cause is unknown.

The premise is right and the conclusion does not follow, because that string was never seen. Test 3's failure line printed $(out_of t3) and nothing else at the time:

$ git show 1e1fe5e9:.github/scripts/check-compat-baselines.sh | grep 'bad "3 numeric'
  bad "3 numeric sort (rc=$(rc_of t3)): $(out_of t3)"

Its stderr was never read. I inferred a plausible message from the surrounding code and wrote it down as an observation.

What was actually observed is one run red on test 3 alone, rc=1, empty stdout, while eighteen mutation copies were running. And at that commit sorting was the only fixture with more than one version-shaped tag — paged had 1.0.0 and main, one:

"sorting": ["1.2.3", "1.10.0", "0.9.9", "main"],      <- three
"paged":   ["1.0.0", "main"],                          <- one

So "the only test that can hit a BrokenPipeError is 3" and "only test 3 went red" agree exactly. The round's own repro of the old pipeline confirms it from the other side: the guard goes red 4/15 and 5/20 under load, on precisely the tests whose fixtures have two or more.

The identification stands — on the rc and the empty stdout, not on a message nobody read. A fabricated detail in a record is worse than the original misdiagnosis, because everyone downstream of it reasons from it, which is exactly what happened here. Both versions are now in the comment, and the guard prints stderr on every failure; test 3 was one of two bad lines that dropped it, which is the whole reason there was nothing to go on.

The stub comment no longer says "the flake was never here at all" either — that overstated what is known. The claim the revert actually rests on is that nothing observed ever implicated the stub.

2 — int("0-amd64") was derived from a fixture the same commit replaced
"0.0.1-amd64".split(".") -> ['0','0','1-amd64']   ValueError: … '1-amd64'
"2.0.0-amd64".split(".") -> ['2','0','0-amd64']   ValueError: … '0-amd64'

The fixture was 2.0.0-amd64 before that commit changed it to 0.0.1-amd64; the comment's value was not re-derived. It sits inside the comment written specifically to correct a previous wrong mechanism claim.

3 — the rename left three dangling references

versions_from became newest_version_from. Two mutation entries in the guard's header and a cross-reference at smelt/tests/compat/compat_test.go:137 still named the old one. grep -rn versions_from over the tree now returns only newest_version_from.

Verification

Re-measured rather than copied — the rate is a property of the load, so the comment now gives the conditions:

old form, idle             :   0 / 150
old form, six-way CPU load :  18 / 150
old form, busier box       :  77 / 150   (measured by the round)
new form, same load        :   0 / 150

Local at f991dcc5: guard 15/15; all six other check-*.sh exit 0; gofmt -s -l . empty; shellcheck clean on both scripts; GOWORK=off go vet -tags e2e,compat ./... clean; all seven workflows parse; a live run against ghcr.io resolves eight baselines.

Confirmed by the round and not re-litigated: the \|\| exit 1 entry is exact under deletion (test 6 only), which is what "by deleting each guard" means — neutering it to \|\| true instead reddens 6, 7, 8, 13a and 13b, because all five take their rc=1 from that line. The un-fixtured set is exactly the five arms the header names, mapped against all 27 branches enumerated independently. The "NOT IN THE LIST" paragraph holds: the regression does not become deterministic with volume either (2, 10, 100, 1000 and 5000 tags all give 0/40 idle).

A numbers-only round on f991dcc5 is open — the diff is comments plus one failure message, and AGENTS.md's brief for that is to check every derived number and factual claim against the tree and report nothing else.


Generated by Claude Code

Peeja pushed a commit that referenced this pull request Sep 23, 2026
I recorded a symptom I never observed -- test 3 printed stdout only, so its
stderr was never read -- and a review then reasoned from it to a wrong
conclusion about a real bug. What was actually observed (test 3 alone, rc=1,
empty stdout) matches the BrokenPipeError exactly, because sorting was the
only fixture with more than one version-shaped tag at that commit.

Both versions stay in the record. Head bumped to f991dcc.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
…elog

Numbers-only round, three findings.

`sorting` WAS NOT THE ONLY FIXTURE WITH MORE THAN ONE version-shaped tag.
`paged` carries 1.0.0 on its first page and 2.0.0 on its second, PAGE2 existed
at 1e1fe5e, and tags_for concatenates every page before the pipeline sees it
-- so test 10 can take the same abort, and does in about one loaded run in
twenty. The previous revision counted the first page only and built a
coincidence argument on it ("the only test that can hit this is 3" and "only
test 3 went red" agree), which does not close. Worse, the very next sentence
said "the tests whose fixtures have two or more", plural, contradicting it.
The identification rests on the signature -- rc=1, empty stdout -- reproducing
under load, not on only one test being able to show it.

"THE GUARD NOW PRINTS STDERR ON EVERY FAILURE" WAS FALSE when written. Two
`bad` lines dropped it and that commit fixed one; test 11's printed neither rc
nor stderr, and test 11 drives the `sorting` fixture itself, so it is one of
the tests that reddens from exactly the bug the sentence documents. Every
`bad` line prints both now.

TWO RATES FOR ONE MEASUREMENT. compat-baselines.sh said "18/150 under six-way
CPU load, 77/150 on a busier box" while check-compat-baselines.sh still said
"21/150 on a busy box". Re-measured here across five load levels: 0/100 idle,
1/100 at two-way, 8/100 at four-way, 18/100 at eight-way -- monotonic on this
box, and a review measured 56-83/150 and non-monotonic on another. So no
single rate reproduces and the non-monotonicity does not either. The comment
now gives the range and the conditions and says not to read a rate off it;
the guard points at that one place rather than carrying its own copy.

AND THE COMMENT WAS CUT ROUGHLY IN HALF. It had grown four paragraphs
narrating which revision was wrong and how -- which is precisely the drift
AGENTS.md names: "the commentary drifts toward narrating how the change got
here instead of explaining what is there now. Write for whoever opens the file
next." A reader of newest_version_from needs the bug, when it fires, and that
there is no single rate. The three wrong diagnoses are in git log, where the
comment now points.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

@Peeja Peeja left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[From Claude:] Round on f991dcc5 (numbers-only brief): not satisfied — 3 findings, all fixed in d2d0652f. No code defect; all three are claims the comment makes about itself, and two of them were introduced by the commit that was fixing the previous round.

1 — the uniqueness argument was built on a miscount

The comment said sorting was the only fixture with more than one version-shaped tag, and used that to argue the observed single red test agreed with the BrokenPipeError explanation. It is not the only one:

sorting   3 version-shaped  ['1.2.3', '1.10.0', '0.9.9']
paged     2 version-shaped  ['1.0.0', '2.0.0']          <- across its two pages

PAGE2 existed at 1e1fe5e9, and tags_for concatenates every page into raw before the pipeline sees it. So test 10 can take the same abort — and does, in about one loaded run in twenty. I counted paged's first page only.

Worse, the next sentence of the same paragraph contradicted it — "on exactly the tests whose fixtures have two or more", plural.

The identification still stands, on the signature (rc=1, empty stdout) reproducing under load on exactly those tests, with test 3 much the likeliest. It was never a uniqueness argument and should not have been written as one.

2 — "the guard now prints stderr on every failure" was false when written

Two bad lines dropped stderr; that commit fixed one.

$ grep -n '^  bad "' …/check-compat-baselines.sh | grep -v err_of    # at f991dcc5
435:  bad "11 job summary: r=$(cat …t11r.summary…) | f=$(cat …t11f.summary…)"

Test 11 printed neither rc nor stderr — and test 11 drives the sorting fixture itself, so it is one of the three tests that reddens from precisely the bug that sentence documents. When it did, the failure line read r=cat: …/t11r.summary: No such file or directory, which is the same "nothing to go on" the sentence claimed to have closed.

Every bad line prints rc and stderr now.

3 — two rates in the tree for one measurement, and neither reproduces

compat-baselines.sh said "18/150 under six-way CPU load, 77/150 on a busier box"; check-compat-baselines.sh still said "21/150 on a busy box". Same pipeline, same fact, two numbers — and the whole point of that commit was that the figure had been re-derived.

Re-measured here across five load levels:

idle     0 / 100
2-way    1 / 100
4-way    8 / 100
8-way   18 / 100

Monotonic on this box; a review measured 56–83/150 and non-monotonic on another. So neither a single rate nor the non-monotonicity reproduces. The comment now gives the range and the conditions and says explicitly not to read a rate off it, and the guard points at that one place instead of carrying a copy.

And the comment was cut roughly in half, which is the repo's own rule

It had grown four paragraphs narrating which revision was wrong and how. AGENTS.md: "the commentary drifts toward narrating how the change got here — which draft was wrong, which round found it — instead of explaining what is there now. Write for whoever opens the file next."

A reader of newest_version_from needs the bug, when it fires, and that there is no single rate. The three wrong diagnoses are in git log, where the comment now points.

Verified and not re-litigated

Checked live against ghcr.io with anonymous pull tokens: piri 1 (0.2.4), ingot 1 (0.0.0), indexing-service 1 (1.13.4), and hilt/sprue/delegator/piri-signing-service/swarf 0 — exactly the claim, with the fleet re-derived from publishedImages. int("1-amd64") correct for the 0.0.1-amd64 fixture. "Only fires with more than one version-shaped tag" measured directly: 0 tags 0/150, one 0/150, two 42/150, three 79/150 under load. Test 3's failure line did print stdout only at both 1e1fe5e9 and 93a17f6f. The could not read <svc>'s tags message does come from tags_for two steps earlier. The stub claims about request_queue_size and daemon_threads hold under Python 3.11. Both renamed mutation entries redden exactly tests 3 and 11. No versions_from left anywhere.

Local at d2d0652f: guard 15/15; six other check-*.sh exit 0; gofmt -s -l . empty; shellcheck clean on both scripts; GOWORK=off go vet -tags e2e,compat ./... clean; all seven workflows parse; live run resolves eight baselines.

Where the rounds stand. Five rounds: 10, 6, 6, 3, 3. The last two found no defect in anything that ships — only claims the files make about themselves, and in four of the five rounds the fixing commit introduced at least one new wrong claim of its own. One more numbers-only round is open on d2d0652f, against a comment that is now about half the size. If it comes back without a code defect, I'll stop there rather than keep trading prose corrections.

The launch-contract blocker is untouched by all of this and is still the decision that gates the gate.


Generated by Claude Code

Peeja pushed a commit that referenced this pull request Sep 23, 2026
Round five, numbers-only, three findings, no code defect -- and two of the
three were introduced by the commit fixing round four. Always a claim about a
measurement, never about code.

Records the stopping rule, stated before the next result rather than after it:
one more numbers-only round, and if it finds no code defect the rounds stop.

Head bumped to d2d0652.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
The abort is not silent. It always writes a BrokenPipeError traceback to the
script's stderr -- measured 9/9 here and 93/93 in review, none silent -- and
that traceback is what the diagnosis two commits ago was built from
("python's print takes BrokenPipeError"). So the sentence denies the existence
of the evidence used to write it.

Same failure as the one the previous commit corrected, inverted: that one
removed an INVENTED stderr symptom, this one asserted the ABSENCE of the real
one. Both times the error was writing a claim about stderr without looking at
stderr.

What is actually true, and is the part worth a reader's attention: the
traceback names `<string>` line 4, so it says a python block died but not
which of the script's several or for which service, and in a CI log it sits
under whatever the caller was doing. That is a weaker complaint than "nothing
says which step failed", and it is the one the evidence supports.

Round six found no code defect, which was the stated condition for stopping.
Rounds stop here: 10, 6, 6, 3, 3, 1.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

@Peeja Peeja left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[From Claude:] Round on d2d0652f: one finding, no code defect. Fixed in 344c764a. Rounds stop here, per the condition stated before the result — 10, 6, 6, 3, 3, 1.

The one finding: "no message" was false, and I had the message in front of me

Two sentences that commit added — "aborts the whole script with an empty stdout and no message" and "the signature is rc=1 with empty stdout, which is what makes it expensive: nothing says which step failed" — are contradicted by every reproduction.

The abort always writes a traceback to the script's stderr:

Traceback (most recent call last):
  File "<string>", line 4, in <module>
BrokenPipeError: [Errno 32] Broken pipe

Measured 9/9 aborts here and 93/93 in review. None silent.

It is self-refuting against the record the comment points at: the diagnosis in b96d59ed is "python's print takes BrokenPipeError" — which is this traceback. The sentence denies the existence of the evidence used to write it.

And it is the same failure the previous commit corrected, inverted. f991dcc5 removed an invented stderr symptom; d2d0652f asserted the absence of the real one. Both times the error was writing a claim about stderr without looking at stderr.

What is actually true, and is the version now in the file: the traceback names <string> line 4, so it says a python block died but not which of the script's several or for which service, and in a CI log it sits under whatever the caller was doing. Weaker than "nothing says which step failed", and it is what the evidence supports.

Everything else in the diff checked out, including the code
  • "between 1% and 55% across two machines and five load levels", new form 0 everywhere. Re-measured: 0/20 red idle; 19/24 red at 4 concurrent guard copies (47% per at-risk pipeline invocation); 41/72 at 18 concurrent (27%). Both loaded figures inside the stated band, idle 0, and non-monotonic there too — which is what the refusal to quote a single rate says. New form: 0/72 guard runs at 18-way, 0/300 isolated at every level.
  • "no fleet service produces more than one version-shaped tag today." Live, fleet re-derived from publishedImages, ?n=1000, no Link on any of the eight: piri 1 (0.2.4), ingot 1 (0.0.0), indexing-service 1 (1.13.4), the other five 0. The precondition holds empirically too — across 96 old-pipeline runs, no fixture with ≤1 ever aborted.
  • "tests 3, 10 and 11 are the ones whose fixtures emit two or more." sorting 3, paged 2 across its pages; driven by t3, t11r and t10. With the old pipeline restored, those three were the only tests that ever reddened across 96 runs.
  • "cannot catch a regression here deterministically." Still true — 0/20 idle against 19/24 and 41/72 loaded.
  • One set of figures in the tree. A tree-wide grep for rate shapes returns only compat-baselines.sh; the guard's old disagreeing 21/150 is gone.
  • All 15 bad lines print rc and stderr, and test 11's new line is correct shell — exercised for real in the load runs, printing rc=1/0, both summaries and t11r's traceback.
  • No code defect. With comments stripped, the only non-comment change between f991dcc5 and d2d0652f is that bad line.

Why the rounds stop here

Six rounds: 10, 6, 6, 3, 3, 1. Rounds 1–3 found real defects, including one in shipping code — a BrokenPipeError race that aborts the script with no useful diagnosis, latent until any service cuts a second release. Rounds 4–6 found none: eleven findings, every one a claim a file made about itself.

The pattern worth keeping, because it is not about this pull request: in five of the six rounds, the commit fixing the previous round introduced a new false claim — and every one was a claim about a measurement or an observation, never about code. The mechanism that caught all of them was applying the mutation or re-running the measurement. None was caught by reading.

The stopping condition was stated in the previous round's comment, before this result: no code defect, rounds stop. That is met.

Still open and unaffected by any of this: no release of piri or ingot can be launched by this harness, so the gate goes red truthfully and cannot go green meaningfully. Three ways out, with a recommendation, in that comment. That decision is the only thing between this and mergeable.


Generated by Claude Code

Peeja pushed a commit that referenced this pull request Sep 23, 2026
10, 6, 6, 3, 3, 1. Rounds 1-3 found real defects including a latent CI flake in
shipping code; rounds 4-6 found no code defect at all. In five of the six, the
commit fixing the previous round introduced a new false claim -- always about a
measurement, never about code, and always caught by re-running rather than by
reading.

Head 344c764. The only thing left on the PR is the launch-contract decision.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
…swer

Petra's decision, 2026-09-24, on the three ways out of the launch-contract
problem: accept that the gate cannot go green until fresh releases exist. Her
reasoning, and it is better than the framing I offered it under: "it's not
really broken, the monorepo's just not done yet, and it would reflect that
correctly."

So the work that decision creates is making the red legible. Whoever opens the
first release pull request will see `Version skew` fail and has no way, from
the check alone, to tell "the monorepo is unfinished" from "this release breaks
the network" -- the failure reads `container ... is unhealthy` and nothing
else.

RELEASE.md now says the check is expected to fail, why (smelt boots baselines
with THIS tree's entrypoints, and both published releases predate that surface
-- piri 0.2.4 does not know --plc-directory, added 2026-07-22; ingot 0.0.0
still demands root_access/root_secret, dropped 2026-09-14), and HOW TO TELL THE
TWO APART: a launch-contract red fails during `compose up` with a config or
flag error in the container's log, before any upload runs; a real
incompatibility gets the stack up and fails inside assertUploadRetrieve. If you
see the second, do not merge.

And what closes it: the first release cut from a recent commit. Nothing here
changes when that happens -- the baseline comes from the registry, so a new
X.Y.Z image is picked up on the next run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 24, 2026
Petra chose option 1: a release PR going red is the honest answer, because the
monorepo is not finished. RELEASE.md now says so and, more usefully, says how
to tell a launch-contract red from a real incompatibility.

Separately, and blocking: no pull_request workflow run anywhere in forge since
2026-09-23 18:03Z. Seven pushes, zero runs, zero checks on the PR. Dispatch
works, so it is not Actions being off or out of minutes. Correcting my own
claim that CI should be green -- I said it without checking.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Petra: "a red release still means releasing it would break things, and we
shouldn't. Is that accurate?" The conclusion is right and the section I wrote
was wrong to say "merge anyway". Releasing on a gate that has not cleared the
release is the vacuous-green failure moved up a layer -- from a check that
passed without looking to a human overriding one that did not pass at all.

Her assumed cause is the reverse of what happens, though, and the difference
is worth having: she expected the release to break the network BECAUSE other
services had not been released. What actually happens is that smelt applies
THIS TREE'S configuration to an old binary, which is backwards from a real
upgrade -- an operator gets the new binary first and the new configuration with
it. Verified both directions: HEAD's ingot has no root_access at all (so old
ingot demands a key HEAD removed), and HEAD's piri has --plc-directory (so old
piri does not know a flag HEAD added). Neither is a break an operator would hit
upgrading.

So a launch-contract red is not evidence the release breaks the network. It is
also not evidence that it does not: the container never becomes healthy and the
suite never reaches a wire question. No answer, not a good one -- which is
still a do-not-release.

The section is restructured around what the two reds mean rather than around
whether to merge: "the gate could not ask" (fix: a baseline the harness can
start, Phase 1) versus "the gate asked and the answer was no" (fix: the code).
How to tell them apart is unchanged and is now in service of that question
instead of a merge decision.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants