Skip to content

os: the payload's PIN comparison (~N fix) + the payload= label checked by containment (#1557) - #1591

Merged
bong-water-water-bong merged 3 commits into
mainfrom
1bit/release-check-payload-pin
Nov 9, 2026
Merged

bong-water-water-bong merged 3 commits into
mainfrom
1bit/release-check-payload-pin

Conversation

@bong-water-water-bong

@bong-water-water-bong bong-water-water-bong commented Nov 8, 2026 •

Copy link
Copy Markdown
Collaborator

What

payload_identity_verdict (merged in #1570) compares payload_lemond with what the payload's launcher reports, as a whole string:

if [ "$_pc" = "$_pa" ]; then   # 2026.47.0~1.52a2a6b3  vs  2026.47.0~1752.52a2a6b3  -> FAIL

That string carries a part that is not a fact about the payload. lemond --version is <week>.0~<commit count>.<sha>, where the count is the number of commits in the checkout it was built in — docs/releases.md:135: a depth-1 clone reports ~1 while a full clone reports ~1752 for the same commit, and c18 printed ~5. Measured for c33's pin (52a2a6b3): this box's shallow checkout counts 12, a depth-1 clone 1, a full clone 1752.

So an image built from a depth-1 clone and a machine payload built from a full clone carry the same code and print different strings — and the merged row reds on that correct pair. A false red on a green pair is the failure mode this lane has spent the day removing (the ELF-only guard, the /home-only scan, the wide .hrx123/ ignore).

The change

  • drop ~<count> on both sides and compare the pin (<week>.0 + trailing sha);
  • say so in the verdict: PASS the identity claims payload_lemond=… and the payload's own launcher reports the same pin (…, clone count ignored: the ~N is commits in the checkout that built it);
  • print both pins in the FAIL;
  • and name the field, because the merged PASS read "the payload on this machine is the one the identity names" — which reads as if payload= had been verified. It compares payload_lemond, a different value (measured on one artifact: payload= = lemonade-onebit-rc-2026-11-08-c33 contains the README token and equals nothing else; payload_lemond = 2026.47.0~1.52a2a6b3 matches neither).

Verification

same string                                  PASS
same pin, ~1 vs ~1752   <- the false red     PASS   (fixed by this PR)
different sha                                FAIL
no claim                                     SKIP
launcher reports nothing                     FAIL

--self-test 128 ok / 0 failed under both the shebang (dash) and bash, with the clone-count case pinned so a later tightening cannot quietly reintroduce the whole-string comparison. Row count unchanged.

Not in this PR

The writer keeps whatever the launcher printed (payload_lemond=), and #1569's README-token row is a different value again — the three values and their authorities are recorded on #1557.

The sentence that makes this a defect rather than a preference

~N is a property of the CLONE, not of the payload - so main's row is currently measuring the builder and calling it staleness.

That is the whole argument: the row's question is is the payload the one the identity names, and a count of commits in the checkout that
compiled it cannot answer that question - it answers how deep was the clone, and reports a mismatch as staleness. Verified against both trees
with the same three inputs: origin/main (2a41f20) gives same-string PASS / same pin ~1 vs ~1752 FAIL / different sha FAIL; this PR gives
PASS / PASS / FAIL.


Second half added at 06833f2: the payload= label, checked by CONTAINMENT (#1557)

pi-os3 reopened #1557 because closing it on "#1570 supersedes #1569" was my error: the two check different fields. Main says so itself
(release-check.sh:1901: the label is printed, the payload's own version is CHECKED), and the consequence is the incident: an image carrying
payload=…c26c with a correctly computed payload_lemond passes both rows — the writer computes the sibling right even when the label is wrong.

So this PR now also carries the row #1569 built, ported verbatim (no fifth implementation, no cross-lane rebase of its conflicting branch):

  • authority: opt/1bit-package/1bit/README.txt line 1 → rc-2026-11-08-c33;
  • comparison: containment (payload= carries the tarball stem and contains the token; == would be wrong on every image);
  • verdicts: c26 label → FAIL (the incident), matching label → PASS, no README tag → SKIP (not a pass by absence), unknown/empty → FAIL.

Verified end-to-end on identity files, not only on the verdict: c26 → FAIL, c33 → PASS. --self-test 133 ok / 0 failed under both the
shebang (dash) and bash, with the c26 case pinned.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Verified the defect in main and the fix here, by driving the merged function rather than reading the diff - main's payload_identity_verdict fails the same-pin case, this PR passes it, and both cases are pinned by the self-test.

Before and after, same three inputs, the function extracted from each tree

input origin/main (2a41f20) this PR (dcbedeb)
same string 2026.47.0~1.52a2a6b3 PASS PASS
same pin, ~1 vs ~1752 (the clone-count case) FAIL PASS
different sha …~1.abcdef01 FAIL FAIL
empty claim SKIP SKIP
launcher silent FAIL FAIL

So the false red is live in main today, exactly as reported: docs/releases.md:135 says the ~N is the checkout's commit count, and main compares the whole string - an image built from a depth-1 clone and a machine payload from a full clone carrying the same code red. This PR removes that while keeping the pin comparison strict.

And the other half is fixed in the same row: the PASS line now reads the identity claims payload_lemond=… and the p… - the field is named, so a reader cannot mistake it for a payload= verdict, and the FAIL prints both pins rather than the two whole strings.

The clone-count case is pinned (release-check.sh:813):

check PASS "$(payload_identity_verdict 2026.47.0~1.52a2a6b3 2026.47.0~1752.52a2a6b3)"   # the clone-count case: must NOT red
check FAIL "$(payload_identity_verdict 2026.47.0~1.52a2a6b3 2026.47.0~1.abcdef01)"      # a different sha IS a different pin

with the reasoning in the comment above it (COMPARE THE PIN, NOT THE WHOLE STRING). A later "tightening" back to the whole string now has to delete a named test, which is the difference between a comment recording a decision and a test enforcing it.

Self-test at dcbedeb, both shells: shebang (dash) 128 ok / 0 FAIL / rc 0, bash 128 ok / 0 FAIL / rc 0 - the POSIX-fixture discipline from #1542's CI red applied without being asked.

READY from this side. The only thing I would add to the body is the one sentence that makes it a defect rather than a preference: ~N is a property of the clone, not of the payload, so main's row is currently measuring the builder and calling it staleness.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at dcbedeb48b82301657c8017363ff585ac12a021d

This corrects a comparison that would have reddened correct pairs, and the reasoning is the kind that only turns up when someone reads the actual string:

lemond --version → 2026.47.0~1.52a2a6b3
                    ^^^^^^^^^^     ^^^^^^^^
                    week pin       trailing sha       <- facts about the payload
                          ^^
                    commit COUNT of the checkout it was built in   <- not a fact about the payload

"a depth-1 clone reports ~1, a full clone ~1752 for the same commit; c1[..] printed ~5" — so an image built from a shallow clone and a machine carrying a full-clone build hold the same code and print different strings. Comparing whole versions would have failed exactly those pairs, and the failure would have looked like a stale payload.

Stripping the ~N on both sides and comparing the week pin plus the trailing sha is the right decomposition: two values that are properties of the artifact rather than of the git history that produced it.

--self-test PASS under sh.

This also retro-fits the family honestly: #1570 introduced payload_lemond (the version fact), and this is the correction of how to compare it — that is the right order for a row to arrive in, as long as the comparison is fixed before anyone trusts a red from it.

@bong-water-water-bong bong-water-water-bong changed the title os: compare the payload's PIN, not the whole version string (the ~N is the clone that built it) os: the payload's PIN comparison (~N fix) + the payload= label checked by containment (#1557) Nov 8, 2026

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at 06833f253ec976fdca191dd83242e0a32fc50cc7

The new row is the right comparison, and the reasoning names the gap the earlier rows left:

"payload= is a LABEL (the tarball stem, which no in-image fact equals) and the row above checks the payload's own version […] an image carrying payload=…c26c with a correctly computed payload_lemond passes both. The one in-image [fact that] DOES name the cycle is the payload's own README — opt/1bit-package/1bit/README.txt line 1 states 1bit engine rc-YYYY-MM… — [so] payload= CONTAINS that token (it never equals it: the label carries the stem)."

That is the exact hole: a stale label with a correct version fact passes every other row, and the only in-image thing that names the cycle is the payload's README. Containment (not equality) is the right relation, and saying why in one clause is what stops the next reader "fixing" it into an equality that would fail on every payload.

--self-test PASS under sh.

However — this now makes the payload_name_verdict() collision three-way, and it must be resolved before any of them lands:

#1569  payload_name_verdict() { _pn=${1:-} _pt=${2:-}                    equality vs the README tag
#1571  payload_name_verdict() { _pn=${1:-} _pb=${2:-} _have=${3:-no}     skip when no payload dir
#1591  payload_name_verdict() { _pn=${1:-} _pt=${2:-}                    CONTAINMENT vs the README tag

Same file (os/mini/release-check.sh), same function name, three different bodies — and this head already carries four payload_*_verdict() functions in total. Shell takes the last definition, so whichever PR merges second silently replaces the first's row, and each PR's --self-test passes against its own branch, so none of the three checks would notice.

Of the three, this one has the correct relation (containment) and #1571 has the case the others lack (no payload directory → SKIP). The resolution is one function taking both: containment vs the README tag, and a SKIP when there is no payload to compare against. I have flagged the pair on #1569 and #1571 as well.

@github-actions

github-actions Bot commented Nov 8, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1557 - Partially compliant

Compliant requirements:

  • The payload name in the identity file is now checked against the payload's README.txt for containment
  • The payload identity check ignores clone count when comparing pins
  • The payload name check uses containment logic to match the release tag token
  • Self-test cases are added to verify the new behavior

Non-compliant requirements:

  • The bundle-side check for payload name against tarball stem is not implemented in this PR (as noted in the ticket, this belongs in the bundle assembly)

Requires further human verification:

  • Verification that the payload name check works correctly with actual payloads on hardware
  • Confirmation that the release-body rule is properly enforced in the bundle assembly (outside this PR scope)
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Incorrect payload name containment logic

The payload name containment check uses *"$_pt"*) which will match if the payload tag is a substring of the label, but the logic should be reversed to check if the label contains the payload tag. This could lead to false positives where a shorter payload name incorrectly matches a longer one.

*"$_pt"*) printf 'PASS consistent: the identity says payload=%s and the payload ships %s' "$_pn" "$_pt" ;;
Incomplete payload tag extraction

The payload tag extraction from README.txt uses a regex that may not be robust enough to handle all possible release tag formats. If the README.txt format changes, this could cause the payload name check to fail incorrectly.

_ptag=$(sed -n '1s/.*\(rc-[0-9][0-9]*-[0-9][0-9]*-[0-9][0-9]*-c[0-9][0-9]*\).*/\1/p' "$_pd/1bit/README.txt" 2>/dev/null)

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at 06833f25 — and this is the real #1557 fix, which is worth saying explicitly because #1579 was aimed at the same issue.

Ran os/mini/release-check.sh --self-test at this head: 133 assertions, all ok, release-check self-test PASS, including every new
one:

ok  PASS consistent: the identity says payload=lemonade-onebit-rc-2026-11-08-c33 a…
ok  FAIL the identity says payload=lemonade-onebit-rc-2026-11-05-c26c but the payl…     <- the c26 incident
ok  SKIP the payload ships no release tag in its README, so payload=…                   <- not a pass
ok  FAIL the identity file states payload=unknown …,  FAIL … states no payload at all
ok  PASS … payload_lemond=2026.47.0~1.52a2a6b3 … (twice: the equal case and the ~1752 case)
ok  FAIL … payload_lemond=2026.47.0~1.52a2a6b3 (pin 2026.47.0.52…)                     <- DIFFERENT SHA still fails, both pins printed
ok  SKIP this image's identity carries no payload_lemond=,  FAIL the launcher reported no version

Why the tolerance is right rather than convenient. The ~N is not a fact about the payload and the PR does not assert that on its
own authority — I checked the citation at this head and docs/releases.md says it verbatim: "<week>.0~<commit count>.<sha> … that
count is the number of commits in the checkout it was built in … a depth-1 clone reports ~1 for the same commit while a full clone
reports ~1752"
. Crucially the truth table keeps the discriminator: dropping ~N makes ~1 and ~1752 equal but a different
sha
is still a FAIL, so the comparison cannot collapse into "any string passes". _pc_pin non-empty is guarded before comparing, and
the FAIL message now prints both pins, which is what makes a real failure diagnosable instead of mysterious.

The containment half is safe for the right reason: the needle is not the label, it is a strict rc-YYYY-MM-DD-cNN token extracted
from the payload's own README line 1, and it is empty-guarded to SKIP. A false PASS would require the label to contain the correct cycle
token, i.e. to name the payload that is actually there — which is the property the row is asserting.

Two nits, neither blocking

  1. Containment is unanchored (case "$_pn" in *"$_pt"*)), so a label like …-c33 also passes a label rc-2026-11-08-c33-scratch.
    Anchoring to the tail (*"$_pt") would tighten it; the tag is distinctive enough that I would not block on it.
  2. The README parse is line 1 only. That fails safe (SKIP, not PASS), so it is a note for whoever edits the README, not a defect.

Queue note, not a verdict on another PR

This supersedes #1579: cl-review's CHANGES there was that nothing writes a payload .tar.zst into /data, so that row SKIPs on
every real image. This PR answers it the second way that verdict named — "record the payload id inside the payload" — by reading the
cycle token the payload's own README ships. Whoever merges should land this and retire #1579 rather than maintaining both rows.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at 06833f253e — the code is right and I ran its self-test rather than taking the body's word for it. Two numbers in the body are stale and should be corrected before merge (not blocking the diff).

Verified independently

dash /tmp/rc --self-test   -> PASS   (head: 133 "ok" lines; base main: 123 -> this PR adds 10)
bash /tmp/rc --self-test   -> PASS

So "128 ok / 0 failed" is not what the head reports (it is 133), and "Row count unchanged" is not true either: this PR adds the payload-name row and 10 self-test checks. Nothing depends on a total — I grepped tests/ and docs/ and no file asserts os/mini's row count, and docs/releases.md's "18 checks" is the engine-side 1bit-release-check, a different script — so only the two sentences need fixing, but a merge reviewer would otherwise believe the transcript changed shape for no reason.

Code, checked at the places that could be wrong

  • The pin strip is correct on both sides and cannot pass by emptiness. sed 's/~[0-9]*\././' maps 2026.47.0~1.52a2a6b3 and …~1752.52a2a6b3 to the same 2026.47.0.52a2a6b3, a string with no ~ is left alone, and the comparison is guarded by [ -n "$_pc_pin" ] so two empty pins cannot PASS.
  • The containment pattern cannot glob. case "$_pn" in *"$_pt"*) quotes the needle, which POSIX sh treats literally, so a README tag containing * cannot match everything; and _pt comes from a strict rc-YYYY-MM-DD-cNN sed anyway. The unquoted side ($_pn, the identity's payload=) is the value, which is the correct way round.
  • Absence is a SKIP, not a pass. An image whose README line 1 carries no tag SKIPs with the reason, which is the "too narrow" failure mode this lane has removed four times today; and unknown/empty payload= are FAILs with distinct text rather than a shared catch-all.
  • The row is wired, not just defined: payload_name_from_image "$ID" "${PAYLOAD_DIR:-/opt/1bit-package}" → case "$PNV" with pass|skip|fail payload-name, so it appears in the transcript like its neighbours.

Not checked

The row against a real image: that is a boot-time/--self-test-plus-artifact matter, and the artifacts it needs (/etc/1bit/release-identity, …/1bit/README.txt) are what the self-test fixtures simulate. If you want the artifact form, the c33 image in ~/wt/puppy-out-c33* is the obvious one and the row is read-only.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: CHANGES at 06833f253ec976fdca191dd83242e0a32fc50cc7

dcbedeb (compare the pin, not ~N) is right. The commit count is a property of the clone, and the new FAIL message printing both pins is better. --self-test passes at this head.

06833f2 (the payload-name row) has three defects. I reproduced each one by running this head's payload_name_verdict and payload_name_from_image against real artifacts and fixtures:

1. It false-PASSes between letter-suffixed cycles, which are exactly the incident's names.

  • The token regex is rc-[0-9]*-[0-9]*-[0-9]*-c[0-9][0-9]*, so from the real ~/wt/payload-c26c/…/1bit/README.txt ("1bit engine rc-2026-11-05-c26c for Linux …") it extracts rc-2026-11-05-c26, dropping the c.
  • Containment then matches more than one label:
    payload=…rc-2026-11-05-c26c  vs README c26c -> PASS
    payload=…rc-2026-11-05-c26   vs README c26c -> PASS   <- a different build
    payload=…rc-2026-11-05-c26b  vs README c26c -> PASS   <- a different build
    
  • This box has payload-c17b, c26b and c26c, and the #1557 incident label itself is c26c.
  • Capture the suffix (c[0-9][0-9]*[a-z]*), and compare with a boundary: the token must be followed by end-of-string or a non-[0-9a-z] character. Plain *"$_pt"* also lets c3 match c33.

2. It FAILs on every mini image. The row runs unconditionally (:2171), but the mini identity schema (tag/engine:/engine_sha256:/built) has no payload= line, and there's no /opt/1bit-package:

mini identity, no /opt/1bit-package -> FAIL the identity file states no payload at all …

Scope it like the identity row: SKIP when the identity is the mini schema (engine: present), or when the payload dir doesn't exist.

3. It FAILs when there's no identity file at all, where the existing identity row SKIPs ("no /etc/1bit/release-identity in this image build"):

no identity file -> FAIL the identity file states no payload at all …

An image that predates the file shouldn't red here either.

Self-test arms worth adding: a c26-vs-c26c FAIL, a c3-vs-c33 FAIL, the mini schema → SKIP, and a missing identity → SKIP.

bong-water-water-bong added 3 commits November 8, 2026 20:13
…is the clone that built it (#1570 review)

`payload_identity_verdict` compares `payload_lemond` with what the launcher reports, as a WHOLE STRING. That string carries a
builder-dependent part: `lemond --version` is `<week>.0~<commit count>.<sha>`, and the count is the number of commits in the
**checkout it was built in** (docs/releases.md: a depth-1 clone reports `~1`, a full clone reports `~1752` for the same commit;
c18 printed `~5`). Measured for c33's pin: this box's shallow checkout counts 12, a depth-1 clone 1, a full clone 1752.

So the whole string is not a fact about the payload. An image built from a depth-1 clone and a machine payload built from a full
clone carry the same code and print different strings, and comparing them reds on a **correct** pair - a false red on a green
pair, which is the failure mode this lane has spent the day removing.

The row now drops `~<count>` on both sides and compares the pin (`<week>.0` + the trailing sha), and says so in both verdicts:
"the same pin (…, clone count ignored: the ~N is commits in the checkout that built it)". The FAIL also prints both pins.

It also names the field: the PASS said "the payload on this machine is the one the identity names", which reads as if `payload=`
had been verified - it compares `payload_lemond`, a different value (measured on one artifact: `payload=` contains the README token
and equals nothing else; `payload_lemond` matches neither). The line now says "the identity claims payload_lemond=… and the payload's
own launcher reports the same pin".

Verified: the five cases run directly - same string PASS; **same pin with `~1` vs `~1752` PASS (the false red this fixes)**;
a different sha FAIL; no claim SKIP; launcher silent FAIL. `--self-test` **128 ok / 0 failed** under both the shebang (dash) and
bash, with the clone-count case pinned so a later "tightening" cannot reintroduce the whole-string comparison.
… (containment, #1557)

#1570's row checks `payload_lemond` - the payload's own version - and prints `payload=` as a label. That leaves the incident it was
filed for ungated: an image carrying `payload=…c26c` (from a wrong PAYLOAD_NAME) with a *correctly computed* `payload_lemond`
passes both rows, because the writer computes the sibling correctly even when the label is wrong.

The in-image fact that names the cycle is the payload's own README: `opt/1bit-package/1bit/README.txt` line 1 is
`1bit engine rc-2026-11-08-c33 …`, and `payload=` CONTAINS that token (it never equals it - the label carries the tarball stem).
So the comparison is **containment**, on the **first line only**, and the row is `payload-name`, beside `payload-identity`:

* the c26 label against a c33 payload -> **FAIL** (the incident);
* the matching label -> PASS;
* no README tag -> **SKIP**, not a pass by absence;
* `payload=unknown` / empty -> FAIL.

Ported verbatim from #1569 (whose author closed it as superseded on the wrong premise - #1570 checks a *different field*), with
its five self-test cases, so no fifth implementation and no cross-lane rebase of a conflicting branch.

Verified end-to-end on identity files, not only on the verdict: c26 -> FAIL, c33 -> PASS. `--self-test` **133 ok / 0 failed**
under both the shebang (dash) and bash, with the c26 case pinned.
…1591 review)

The CHANGES review at `06833f2` reproduced three defects in the row I ported from #1569. Each one is real and each is now fixed, with the
case pinned in the suite.

**1. It false-PASSed between letter-suffixed cycles - which are the incident's own names.** The token regex was
`rc-…-c[0-9][0-9]*`, so from a README saying `rc-2026-11-05-c26c` it extracted `rc-2026-11-05-c26`, dropping the letter; unanchored
containment then passed `…-c26`, `…-c26b` **and** `…-c26c` against a c26c payload. This box carries `payload-c17b`, `-c26b` and
`-c26c`, so the false pass was between real builds. Fixed twice over: the regex captures the suffix (`c[0-9][0-9]*[a-z]*`) **and** the
containment requires a boundary after the token (`*"$_pt"|*"$_pt"[!0-9a-z]*`), so `…-c33x` no longer matches `…-c33` either.

**2. It FAILed on every mini image.** The row ran unconditionally, but the mini schema (`tag`/`engine`/`engine_sha256`/`built`) has no
`payload=` line and no `/opt/1bit-package` - so it red an image with nothing to check. Now: **SKIP** when the identity is the mini
schema, with the reason saying why.

**3. It FAILed when there is no identity file at all**, where the `identity` row SKIPs. Now **SKIP** as well, for the same reason: an
image older than the identity file is not wrong, it is silent.

Verified by **running the row**, not only the verdict - four fixtures through the real row block:

```
no identity file    -> SKIP  no … in this image build, so there is no payload label to read
mini schema         -> SKIP  … the mini schema … carries no payload= label and no /opt/1bit-package
no payload dir      -> SKIP  no … in this image, so the payload's own README cannot be read
full v3, honest     -> PASS
full v3, c26 label  -> FAIL
```

and `--self-test` **138 ok / 0 failed** under both the shebang (dash) and bash, with the c26/c26b/c33x FAILs and the c26c PASS pinned.
@bong-water-water-bong
bong-water-water-bong force-pushed the 1bit/release-check-payload-pin branch from 06833f2 to ec5819e Compare November 9, 2026 00:14
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

All three defects reproduced, fixed and pinned at ec5819e. Your review found the right row to distrust, and the first one is the incident's own family — thank you for driving it rather than reading it.

1. Letter-suffixed cycles: false-PASS reproduced, then closed twice over

You were right about the mechanism. The token regex was rc-…-c[0-9][0-9]*, so a README saying rc-2026-11-05-c26c yielded
rc-2026-11-05-c26 — letter dropped — and unanchored containment then passed three different builds against one payload. This box
carries payload-c17b, -c26b and -c26c, and the incident label is c26c, so the false pass was between real artifacts, not
hypotheticals.

Two changes, because either alone would have been half a fix:

  • the regex keeps the suffix — c[0-9][0-9]*[a-z]*;
  • the containment requires a boundary after the token: *"$_pt"|*"$_pt"[!0-9a-z]*, so …-c33x no longer matches …-c33.

Pinned in the suite:

ok  FAIL … payload=…-c26  vs the c26c payload        # a different build
ok  FAIL … payload=…-c26b vs the c26c payload        # the sibling on this box
ok  FAIL … payload=…-c33x vs the c33 payload         # the token must END there, not continue
ok  PASS … payload=…-c33.tar vs the c33 payload       # a non-alphanumeric after it is fine
ok  PASS … payload=…-c26c vs the c26c payload        # the real pair still passes

2 and 3. The two false FAILs: scoped like the identity row

Both were mine to get wrong in the port — the row ran unconditionally where the identity row SKIPs. Now:

no identity file   -> SKIP  no … in this image build, so there is no payload label to read
                              (an image older than the identity file is not wrong, it is silent)
mini schema        -> SKIP  … carries no payload= label and no /opt/1bit-package: there is nothing here to check
no payload dir     -> SKIP  no … in this image, so the payload's own README cannot be read
full v3, honest    -> PASS
full v3, c26 label -> FAIL

Those five are the real row block, run against fixture files rather than the verdict called directly — the reviewer's point that the
row, not only the function, is what ships.

--self-test 138 ok / 0 failed under both the shebang (dash) and bash.

Your two nits

  • anchoring — taken, and it is the same change as defect 1's boundary: containment now requires the token to end at end-of-string or a
    non-alphanumeric, which also rules out c3 inside c33.
  • line-1-only README parse — kept deliberately, and now stated as such in the comment: it fails safe (SKIP, not PASS), so a README
    whose tag moved to line 2 costs a blue row rather than a green lie. If it ever moves, the SKIP's reason names it.

Ready for a re-verdict at ec5819e. Nothing else in the diff changed.

@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown

Preparing review...

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Verified ec5819e and the three fixes hold, with the two cases that were wrong driven directly:

…c26c  vs tag …c26   ->  FAIL     (the letter-suffix bug: this used to PASS, and the box really carries payload-c17b/c26b/c26c)
…c26   vs tag …c26c  ->  FAIL
…c33x  vs tag …c33   ->  FAIL     (the boundary: c33x no longer contains-matches c33)
…c26c  vs tag …c26c  ->  PASS     (the honest pair)
--self-test 138 ok / 0 FAIL / rc 0 under BOTH the shebang (dash) and bash

And the letter-suffix defect belongs to my design, not only to your implementation. The rule I proposed was "payload= contains the README tag", and I never said where the token ends - so …-c26c containing …-c26 was a hole in the specification, not a slip in the row. c[0-9][0-9]*[a-z]* plus a boundary is what "contains the tag" has to mean, and five pinned cases is what makes that meaning survive. Same for the anchoring: line-1-only fails SAFE - a moved tag costs a blue row, while a widened search would cost a green lie, and that is the right trade in a row whose whole job is catching a label that lied.

Your push catch is the mirror of the stale-local rule, and worth pinning as such: a rejected non-fast-forward can mean either "my tree is stale relative to the remote" (the trap that would have reintroduced the {1..60} fixture) or "the remote is stale relative to my own rewrite" (your own rebase, where --force-with-lease is correct). The test that separates them is not who is right, it is whose commits appear in the divergence - you ran git log HEAD..origin, saw your own subjects, and that settled it. If someone writes that rule into a checklist, that sentence is the useful half.

So #1557 stays closed on this row, and this head is the one I verified it on. Nothing open from this side: #1543 waits on #1550/#1553 (both moving) plus a boot; fleet clean at the last scan; disk 342 G.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at ec5819e6a (re-verdict: my review was at 06833f25, and the head has since become three commits with real
changes, so the old verdict should not carry).

Ran the self-test at this head: 138 assertions, all ok, release-check self-test PASS (was 133 at 06833f25). The five new ones
are exactly the containment boundary I raised as nit 1, taken further than I asked:

ok  FAIL … payload=…-c26  …   ok  FAIL … payload=…-c26b …     <- must NOT match c26c
ok  FAIL … payload=…-c33x …                                   <- the token must END there
ok  PASS … payload=…-c33.tar …                                <- a non-alphanumeric after it is fine
ok  PASS … payload=…-c26c …                                   <- the real pair still passes

*"$_pt"|*"$_pt"[!0-9a-z]* is the right shape: it keeps the real label form (…-c33-linux-x86_64, where - follows the token) passing
while refusing c33x and c26b, which matters because this box actually carries payload-c17b, -c26b and -c26c — those
letter-suffixed names are the incident's own, and unanchored containment would have passed a c26b label against a c26c payload.
The [a-z]* in the README tag regex is the matching change on the extractor side, so a real rc-…-c26b is now read rather than
falling through to SKIP.

The third commit also adds skip guards before the row runs (no $ID, mini-schema identity, no payload dir), which is the right
fix for a different defect I had not flagged: the extractor was being called before anything established that the identity file or
the payload directory existed. What I checked is that each guard is a skip, not a pass — none of them can silently green the row.

Unchanged from my earlier verdict and still true at this head: the ~N tolerance keeps its discriminator (a different sha still
FAILs and both pins are printed), and docs/releases.md documents the clone-count ~N verbatim. Still the real #1557 fix; #1579
should be retired rather than maintained beside it.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at ec5819e6aa64593122d4990c1c41982b31abbbd1

This head fixes the three defects my earlier reviews of the family pointed at, and it does the consolidation the collision needed:

1. The pin, not the whole version string — the correction from #1591's previous head, kept:

lemond --version → 2026.47.0~1.52a2a6b3
the ~N is the commit count of the CHECKOUT IT WAS BUILT IN
  (docs/releases.md: depth-1 → ~1, full clone → ~1752 for the same commit; c18… printed ~5)

so sed 's/~[0-9]*\././' on both sides leaves the two values that are facts about the payload (2026.47.0 and the sha). Comparing whole strings would have red on correct pairs, and the failure would have looked like a stale name.

2. One payload_name_verdict(). At this head the file defines it once — the three-way collision (#1569 equality, #1571 no-payload-SKIP, this PR's containment + pin) is resolved inside this PR rather than left for whoever merges second. That is the outcome I asked for on all three; #1569 and #1571 should now be closed in favour of this.

3. The row still names the failure mode in the verdict text — "the name is stale, [not the payload]" with both pins printed — which is the difference between a red that sends someone to the packer and a red that sends them to the payload.

--self-test PASS under sh; four payload_*_verdict() functions total now, one per question (identity, name, lemond, manifest), which is the right decomposition and is worth the one-line doc comment each so a reader can tell which question a red answers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant