Skip to content

test: assert the matrix controller records the installed library (#961) - #981

Merged
jdatcmd merged 2 commits into
commandprompt:mainfrom
OffgridwithJD:fix/961-the-controller-records-the-binary
Sep 11, 2026
Merged

test: assert the matrix controller records the installed library (#961)#981
jdatcmd merged 2 commits into
commandprompt:mainfrom
OffgridwithJD:fix/961-the-controller-records-the-binary

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Closes #961. The matrix controller's stamp write is now asserted, because the failure if it stops is a green matrix-wide downgrade.

run_all_versions.sh builds once per major and runs every child with PGC_SKIP_BUILD=1, so each child reads the controller's stamp to learn whether the binary it measures came from this tree. If the controller's stamp write loses its third argument, every child reaches source-only and passes — because source-only is also the state of every stamp written before #959, so it cannot be a failure. The whole matrix degrades to UNVERIFIED with a green rollup on both majors.

@jdatcmd asked for this while approving #960: "the arm that would catch a silent matrix-wide downgrade is worth more than most of what either of us wrote today, precisely because the failure it catches is green."

The constraint that shapes it

The defect is a dropped argument at a call site. An arm that calls pgc_write_source_stamp itself proves nothing — the function would be correct and the caller wrong. I named that trap in the issue and it is the reason this part extracts the controller's stamp block and runs it, so what executes is the real call site's own text.

It costs no build. The block reads the installed library and fingerprints a tree; it does not compile. So it runs against a copy of the tree with builddir and pgc set the way the controller sets them — 24 MB at 31 ms a copy, against minutes for a per-major build.

Proven by mutating the real file

Not the extracted copy — test/run_all_versions.sh itself, dropping the third argument in a way that still parses, which is what a careless edit looks like:

installed-digest argument in the controller block:  before=1  after=0
bash -n test/run_all_versions.sh                    still parses

FAIL  the controller's stamp carries BOTH fields (#961): got [1] want [2]
FAIL  so a child suite under PGC_SKIP_BUILD reaches verified, not source-only (#961): got [source-only] want [verified]
FAIL  every caller records the installed library's digest (#961): got [2] want [3]

Three arms, and the control arm correctly keeps passing — it is a control, not an arm.

The part carries its own control

An arm asserting verified passes for any reason that makes the claim verified, including a driver that never ran the block. So the part removes the argument from the extracted block and asserts the same driver reaches source-only:

PASS  control: without that argument the same driver degrades to source-only (#961)
PASS  control: and the stamp carries one field rather than two

Without those, the two arms above could both be vacuous and nothing would say so.

The other two call sites

pgc_setup and devloop.sh write the same stamp, and driving either costs a build, so they get a static sweep. It joins line continuations first, because all three calls are written across four lines and a per-line grep finds the function name on a line carrying no arguments at all — a guard that passes because it read half a statement.

What it cannot see

That the controller reaches that line. The make install guard above it could start failing closed and this part would not notice. It asserts what the line does, not that control flow arrives there. Stated here because the whole issue is about a failure mode nobody was watching, and a new guard with an unstated blind spot is how the next one gets built.

One of my arms reported got [] first

It read $PG_CONFIG, and no selftest part sets that — the harness passes PGC_SELFTEST_PG_CONFIG. Under set -u that aborted the command substitution the driver runs in, and both arms reported empty.

Fixed two ways, not one: the input is asserted before use, and the driver answers driver-could-not-run rather than nothing — because an empty result reads identically for "the controller is broken" and "this part misspelled a variable". Same shape as the vacuous arm in #972, and I wrote it again four hours later.

My teeth test was also wrong the first time: I over-escaped a backslash, the mutation did not apply, and the script reported three PASSes that meant nothing. The assertion fired and my script carried on past it. Every number above is from the corrected run, which refuses to continue if the before/after count does not move.

Gate

harness_selftest   rc=0  815 checks  0 FAIL   (804 on main; +11 are these arms)
docs_style         rc=0  9 checks    0 FAIL
shellcheck -S warning   clean
bash -n            parses
ledger             11 rows, census DERIVED 847 -> 858, gate rc=0
  census stated 858, ledger holds 858: they agree
  coverage registered=252 | covered=2, not covered=250, ceiling=250

Sequencing: this is THIRD in line, and the census here is not final

#975 and #978 both move checks_never_observed_red on the same line. Whichever lands last regenerates against the tree the others produced. I will do that for this PR — rebase, fresh log from the rebased tree, guard it, merge, then derive — so review the part and treat the census number as provisional.

Three PRs contending on one line is itself a measurement for #432. The budget is committed precisely so a change to it is a diff a reviewer sees, which is the right call and is why the contention exists. With two covered suites it is an inconvenience; with 240 it is a queue, and the order of merges decides who pays. That is not an argument against the budget being tracked — it is an argument that the key shape wants settling before the covered set grows.

Closes #961.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto f69ed1e3 and the census re-derived. New head a2eca556. This is the second time this PR has paid the ledger tax, and the reason is worth recording.

#975 merged while this was open. The census I had derived against 847 was therefore stale before anyone reviewed it — not wrong when written, wrong by the time it mattered. That is the failure mode the "derive, never add" rule exists for, and it arrived faster than I expected: within the same session, on the PR whose body already said it would.

1  drop the stale ledger commit        it derived against 847; main is now 859
2  rebase onto f69ed1e3                one CHANGELOG conflict with #975's entry
3  re-run harness_selftest             on the REBASED tree, 827 checks, 0 FAIL
4  guard the log                       rc=0, numeric total above a floor, 0 FAILs, PASSED
5  merge it
6  DERIVE the census                   rows 859 -> 870, census 859 -> 870
7  CI
ledger census: rows=870 | never observed red=870, ever red=0, new this run=0
  census stated 870, ledger holds 870: they agree
ledger coverage: registered=252 | covered=2, not covered=250, ceiling=250
gate rc=0

870 equals 859 + 11 and is not set that way. Writing it by addition would have been right today and wrong the first time a merge order surprises me — which has now happened to this exact PR, once, in the space of an hour.

The CHANGELOG conflicted with #975's entry. Both kept, in landing order, each asserted to appear exactly once in the result rather than eyeballed.

Unchanged by the rebase

test/selftest/460-..., the manifest entry, and all eleven arms are identical to 4d41f2d6. Only the base, the CHANGELOG position, and the census value moved.

Two small things from doing it

My force-push was correctly refused, and the reason is a trap worth naming beside the push convention. I built the lease from the short sha by padding it:

fabricated:  63e1b63e1a5a5f7e5c59c8b79a77a3ae5e3e9a1e
actual:      63e1b63e635b29ff7b5d7d4f45765eafb0531060

They share only the eight characters I had. --force-with-lease=<ref>:<sha> wants the real sha, read from git ls-remote, and padding a short one invents a commit that has never existed. The lease rejected it with stale info, which is the mechanism working — but stale info reads like a race with someone else's push, and it was not one.

A two-dot diff nearly made me report a defect in someone else's PR. Checking whether #978 had regenerated, I ran git diff --stat upstream/main..pr978d -- test/check_ledger.tsv and got 12 ------------. Twelve rows deleted. Those are #975's twelve rows arriving on main, shown as deletions by a branch that lacks them. Against the merge-base, #978 changes the ledger not at all. I have a memory for exactly this and still ran the two-dot form first; what caught it was printing the merge-base beside the number.

Sequencing

#978 is still open and also moves this line. If it lands first this rebases a third time, which I will do.

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved. I drove the teeth test myself by mutating run_all_versions.sh — the real controller, not the extracted copy — and it is the cleanest removal proof I have seen in this repository.

Three arms redden, and exactly the right three

premise: the matrix controller is present and parses                     PASS
premise: this part was given an executable pg_config                     PASS
premise: the controller's stamp block was extracted exactly once         PASS
premise: and it holds exactly one stamp write                            PASS
the controller's stamp carries BOTH fields (#961)                        FAIL
so a child suite under PGC_SKIP_BUILD reaches verified, not source-only   FAIL
premise: the mutation removed the installed-digest argument              PASS
control: without that argument the same driver degrades to source-only   PASS
control: and the stamp carries one field rather than two                 PASS
premise: all three stamp call sites were found with their arguments joined PASS
every caller records the installed library's digest (#961)               FAIL

Total FAILs in the whole suite: 3, all here.

The split is what makes it convincing. The three claims about the controller go red. Every premise and both internal controls stay green — because they test the driver, and the driver is not what I broke. An arm set that reddened wholesale would have told me only that something was wrong.

Why the design is right, and not merely adequate

#961's defect is a dropped argument at a call site. An arm calling pgc_write_source_stamp would be green with the function correct and the caller wrong — which is the entire failure mode, and it is green by construction because source-only cannot be a failure: it is the state of every stamp written before #959.

So the block has to run, and what runs has to be the call site's own text. Extraction plus eval is the only shape that gets there without a per-major build.

And the internal control is the part I would probably not have written. An arm asserting verified passes for any reason that makes the claim verified — including a driver that silently never ran the block. Removing the argument from the extracted copy and requiring the same driver to reach source-only is what turns two green arms into evidence. Without it both could be vacuous and nothing would say so.

The sentinel, which is the same lesson from a different direction

driver-could-not-run rather than empty output. An empty result reads identically for "the controller is broken" and "this part misspelled a variable" — and that is not hypothetical, it is the fault you hit on $PG_CONFIG and fixed twice over, asserting the input and making the driver answer distinguishably.

Two faults of my own while verifying this

My first mutation script used an unquoted heredoc, so the shell expanded the $(...) before Python saw the pattern. The mutation did not apply:

pgc_installed_library_digest: before=1 after=1
MUTATION DID NOT APPLY -- refusing to report a result

That is your rule, adopted an hour ago, earning its keep on first use. Without it I would have run the suite against an unmutated controller, seen eleven passes, and reported that the teeth test confirmed the arms. A perfect and meaningless green.

And my baseline run showed 15 FAILs that were entirely mine — I copied the tree with --exclude=.git, so every git-dependent part reported no-repo. None were part 460. Same instrument fault I hit this morning, in the same session, after writing it down.

The stated gap is the right one to state

it cannot see that the controller REACHES that line

Correct, and worth leaving. The make install guard above it could start failing closed and this part would not notice. What it asserts is what the line does. Saying so is better than an arm that implies otherwise.

Not blocking, for whenever this area is next touched

The extraction anchors on \tif ( and \t); then. A second if ( block at one tab inside the controller would extend the range, and the exit on first match means it would stop at the wrong terminator. Your two premises catch it — the block would hold zero or two stamp writes — so it fails closed rather than silently. Worth knowing it is the premises doing that work rather than the anchor.

Merging once you have rebased onto whatever main holds when #978 lands. I am second in the queue and you are third, which you already have in hand.

@jdatcmd

jdatcmd commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Disclosure on my own approval: I drove the teeth test on 63e1b63e and the approval landed on a2eca556. You rebased while I was running it. Checked rather than assumed, and it carries — but the check is the point, not the outcome.

test/selftest/460-...sh at 63e1b63e   35310e4714e1
test/selftest/460-...sh at a2eca556   35310e4714e1   identical

Everything else in that delta is #975's twelve rows and the re-derived census arriving via the rebase:

-checks_never_observed_red 858
+checks_never_observed_red 870

So the part I mutated the controller against is byte-identical to the part I approved, and the three arms that reddened are the three in this head.

I am saying it out loud because #969 taught me the opposite lesson four hours ago: GitHub kept showing APPROVED while that PR grew from two files to eleven. An approval state survives a head moving and says nothing about it. Mine would have been technically at the right sha and I would not have known which one I had driven.

And your 870 is derived, which I verified rather than took

rows in the committed ledger   870
verdict == never               870
verdict != never                 0
never + everred == total       yes
budget states                  870
ceiling                        250, unchanged

859 + 11 happens to equal it. The value being derivable is not the same as the value being derived, and the only reason anyone can tell is that the file and the number agree when counted independently — which is what I just did.

That is the second time this PR has paid the tax, and the second time the earlier census was stale before anyone reviewed it. Third and fourth cycles today, on one line, between us.

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Your anchor observation measured, and I am deliberately not acting on it in this PR.

You are right about the mechanism. I checked how much room it has today:

one-tab `if (` blocks in run_all_versions.sh:   exactly 1, at line 729
one-tab `); then`:                              exactly 1
the stamp write:                                line 731, inside it

So the extraction is currently unambiguous — there is one candidate and it is the right one. Your point stands for the future: a second one-tab if ( added before line 729 would shift the range, and because the exit fires on the first terminator it would stop at the wrong one. And you are right about what saves it: the premise "and it holds exactly one stamp write" would read 0, so it fails closed with a message naming the extraction rather than the controller.

Worth restating your framing exactly, because it is the useful part: it is the premises doing that work, not the anchor. The anchor is positional and the guard against the anchor being wrong is content-based.

Why I am not changing it here

A content-anchored extraction is strictly better — find the stamp line, walk back to the enclosing if (, stop at the first terminator at or after it — and it is about six lines. I am not putting it in this PR because you approved what you drove, and you drove this text. Changing the extraction after approval would mean either you re-verify a teeth proof you have already run, or an approval rides through a change to the exact mechanism it was granted for. I have spent today arguing against the second, so I am not going to do it to you.

Two ways forward, your pick:

  1. Merge as approved and I open the content-anchored version as a small follow-up, with its own teeth run.
  2. Say the word and I fold it into the rebase that ci: a red nightly opens an issue, and a green one closes it (#973) #978 forces anyway, push, and you re-drive the one mutation — it is the same script you already have.

I have no preference. (1) keeps your approval meaning what it says; (2) costs one extra run and lands it sooner.

On your two faults while verifying

The unquoted heredoc eating $(...) before Python saw the pattern is the same fault I made two hours earlier on the docs probe, and the refusal catching it on first use is worth more than the fix it was protecting. "Without it I would have run the suite against an unmutated controller, seen eleven passes, and reported that the teeth test confirmed the arms" — a perfect and meaningless green is exactly the thing, and it is what my own first attempt produced before I printed the before/after count.

The --exclude=.git baseline with 15 git-dependent parts reporting no-repo is the second instance of that one today and you had already written it down. For what it is worth: none of those were part 460, and the fact that you checked which parts failed rather than reading "15 FAILs" as a verdict is what kept it from becoming a finding against this PR.

OffgridwithJD and others added 2 commits September 11, 2026 22:16
…mandprompt#961)

run_all_versions.sh builds once per major and runs every child with
PGC_SKIP_BUILD=1, so each child reads the controller's stamp to learn whether
the binary it measures came from this tree. If the controller's stamp write
loses its third argument, every child reaches source-only and PASSES -- because
source-only is also the state of every stamp written before commandprompt#959, so it cannot
be a failure. The matrix degrades to UNVERIFIED with a green rollup on both
majors and nothing says so.

jdatcmd asked for this while approving commandprompt#960: an arm catching a green failure is
worth more than most arms.

THE DEFECT IS A DROPPED ARGUMENT AT A CALL SITE, so an arm calling
pgc_write_source_stamp would prove nothing -- the function correct, the caller
wrong. This extracts the controller's stamp block and RUNS it, so what executes
is the real call site's own text.

No build: the block reads the installed library and fingerprints a tree, it does
not compile. It runs against a copy of the tree with builddir and pgc set the
way the controller sets them. 24MB at 31ms a copy, against minutes for a build.

Proven by mutating run_all_versions.sh itself, dropping the argument in a way
that still parses:

    the controller's stamp carries BOTH fields          got [1] want [2]
    a child reaches verified, not source-only    got [source-only] want [verified]
    every caller records the installed library's digest got [2] want [3]

The part carries its own control: it removes the argument from the extracted
block and asserts the same driver reaches source-only, so the arms cannot pass
for a reason about the driver rather than the controller.

The static sweep covers pgc_setup and devloop.sh, where driving either costs a
build. It JOINS LINE CONTINUATIONS first: all three calls span four lines and a
per-line grep finds the function name on a line carrying no arguments at all.

What it cannot see, stated because the gap is the point: that the controller
REACHES that line. It asserts what the line does, not that flow arrives there.

One arm reported 'got []' when first written, reading $PG_CONFIG where the
harness passes PGC_SELFTEST_PG_CONFIG; under set -u that aborted the command
substitution. The input is asserted before use now and the driver answers
driver-could-not-run rather than nothing, because an empty result reads the same
for 'the controller is broken' and 'this part misspelled a variable'.

harness_selftest 815 checks 0 FAIL, docs_style 9/9, shellcheck clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
…ommandprompt#961)

THIRD pass for this PR. commandprompt#975 moved the census while it was open, then commandprompt#978 moved
it again, so each earlier derivation was stale before anyone could review it.
That is not a defect in the PRs -- it is what a committed census costs when more
than one change is in flight, and it is the measurement commandprompt#432 needed.

Done in the agreed order: drop the stale ledger commit, rebase onto main, FRESH
log from the rebased tree, guard it, merge, then DERIVE from the file.

The guard now also asserts RESULT records == checks run, which catches a
truncated log -- @linuxhikerpm's addition, and the one my earlier guards missed.

No expected census value was written down before the count ran. Knowing the
target does not bias the count; it biases whether a DISAGREEMENT is investigated
or dismissed as a mis-run, and that is where an expected answer wins.

The selftest part is byte-identical to the approved revision: a rebase that
altered it would be an approval riding through a change to the mechanism it was
granted for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
@OffgridwithJD
OffgridwithJD force-pushed the fix/961-the-controller-records-the-binary branch from a2eca55 to 36f58dd Compare September 11, 2026 22:17
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto d1a00a39 and the census re-derived. New head 36f58dd3. Third pass for this PR, and the two independent counts agree.

#978 merged, so the census derived against 859 was stale — the second time this PR's census expired before it could be reviewed. That is not a defect in anyone's change; it is what a committed census costs when more than one thing is in flight, and it is the measurement #432 needed.

fresh run on the REBASED tree   rc=0  checks run=857  RESULT records=857  FAILs=0  PASSED
merge                           rows 889 -> 900
derive                          never=900  not-never=0  partition closes
budget written from the count   900
suites_not_covered              250, unchanged
gate                            census stated 900, ledger holds 900: they agree; rc=0

Two independent counts, and they match

@jdatcmd rebased the same commits onto the same main on a local branch they did not push, ran their own fresh selftest, and reported 857 checks / 857 records on PG16. Mine is 857 / 857 on PG18.

Two trees, two majors, same total. And they deliberately did not say what census that implies — "I am not putting a number in your head four hours after you corrected me for doing exactly that" — which is the rule being applied harder than I stated it.

The guard gained the check my earlier ones missed

rc=0 | checks run numeric and above a floor | zero FAILs | suite's own PASSED verdict
RESULT records == checks run        <- new, and the only one a half-finished run cannot satisfy

That last is theirs. A truncated log can satisfy every other condition: the total line is present, no FAIL appears in what survived, and the verdict was printed before the truncation. Records-equals-total is the one that catches it. My previous two regenerations did not have it.

No expected value was written down before the count ran

Not in the script, not in a comment, not in this PR body before the numbers. Knowing the target does not bias the count — it biases whether a disagreement gets investigated or dismissed as a mis-run, and the re-run is where an expected answer wins.

The part is byte-identical to the approved revision

460-the-controller-must-record-the-binary.sh   md5 35310e4714e1  before the rebase
                                               md5 35310e4714e1  after

Verified two ways by two people: an empty diff here, and the same hash computed independently by the reviewer before and after their own rebase. A rebase that altered it would be an approval riding through a change to the mechanism it was granted for.

The two conflicts, resolved the same way independently

test/selftest/parts.manifest   both lines kept, 450 then 460, numeric order;
                               asserted each appears exactly once and the file stays sorted
the stale ledger commit        dropped entirely before the rebase

The reviewer hit both on their own branch and skipped the ledger commit rather than resolving it — different route, same result. Their reason is the better statement of it: resolving a conflict in a file you are about to recompute is how a stale row survives a regeneration. I dropped it before starting; the ledger at my rebased head was main's 889 rows, with nothing of mine left in it.

On the near-collision

We both started this rebase. They offered to take it, I was already inside it, and I told them to stand down rather than letting us discover it through a rejected lease. They pushed nothing, deleted their local branch, and sent their count as a second opinion. Worth recording because the alternative — two force-pushes racing a lease — was one message away.

@jdatcmd

jdatcmd commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Verified at 36f58dd3 before merging, because the head moved after my approval — three checks, and the one that matters is the part I actually drove.

test/selftest/460-...sh at a2eca556   35310e4714e1   the head I teeth-tested and approved
test/selftest/460-...sh at 36f58dd3   35310e4714e1   the head being merged

Identical, so the teeth test carries: mutating the real run_all_versions.sh reddens exactly three arms out of eleven, and three in the whole suite, while every premise and both internal controls stay green.

That hash is now the third independent check on one artifact. @OffgridwithJD had an empty diff and then computed the hash; I computed it before and after my own discarded rebase, and again here. Three checks, two people, two methods. Their line about it is the one to keep: "that is what byte-identical should cost when an approval depends on it, and I had been using the phrase as a claim about my intent."

The ledger, derived here rather than taken

rows           900
never          900
not-never        0
partition      closes
budget         900   matches the derived count
ceiling        250   unchanged
rows added      11   all in 460-the-controller-must-record-the-binary
rows removed     0

And the two rebases agreed

@OffgridwithJD rebased on PG18; I rebased the same commits onto the same main on PG16 and threw mine away. Both fresh logs: 857 checks, 857 records.

Two independent rebases of the same commits producing the same count is the check that says both trees are what we think they are. I sent the 857 without the census it implied, deliberately — they were about to derive one, and putting the expected number in their head is the thing they had corrected me for four hours earlier.

One guard added between the two regenerations

records == checks run. It is the only one of the five a half-finished run cannot satisfy: a truncated log still carries the total line, still shows no FAIL in what survived, and still printed its verdict before the cut. Their two earlier regenerations did not have it.

The conflicts, same place by different routes

Both of us hit parts.manifest (two appends) and the stale ledger commit. They dropped the ledger commit before starting; I skipped it during. Resolving a conflict in a file you are about to recompute is how a stale row survives a regeneration — and the consequence was checked rather than assumed: the ledger at the rebased head was main's 889 with nothing of the PR's in it.

Merging on green.

@jdatcmd
jdatcmd merged commit bf325b3 into commandprompt:main Sep 11, 2026
13 checks passed
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.

Nothing asserts the matrix controller records the library digest, and the failure is a green matrix-wide downgrade to UNVERIFIED

2 participants