Skip to content

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

Description

@OffgridwithJD

#960 records the installed library's digest in the source stamp so a suite cannot claim a binary it has not examined. Nothing asserts that the matrix controller records it, and the failure mode if it stops is green.

Why a green rollup cannot see this

run_all_versions.sh builds once per major and then runs every child suite with PGC_SKIP_BUILD=1. If the controller's stamp write loses the digest argument, every child suite reaches source-only and prints

-- source: <hash> matches what this tree last built;
   the installed library was not recorded, so it is UNVERIFIED

and passes. source-only is deliberately not a failure, because it is also the state of every stamp written before #959. So the entire matrix silently degrades to unverified with a green CI rollup on both majors.

Measured during #960's review: the suites job does reach this path (bash test/run_all_versions.sh, children at run_all_versions.sh:753 and :795), so a green job proves the digest is recorded today. It would not have told me the caller change was needed, and it will not tell anyone when it regresses.

What found it was grepping for callers of pgc_write_source_stamp rather than assuming lib.sh was self-contained. That is a method, not a mechanism.

What to assert

That a matrix run leaves a stamp carrying both fields, and that a child suite launched under PGC_SKIP_BUILD=1 reaches verified rather than source-only.

Why it is not a one-line arm

The existing arms in selftest 340 drive lib.sh's functions, which are pure and need no build. This one has to drive the controller far enough to write a stamp, then inspect it — run_all_versions.sh builds and installs per major, which is minutes, and the arm only needs the stamp rather than the suites. So the work is finding a seam that exercises the controller's stamp write without paying for a full matrix.

Two shapes, neither obviously right:

  • a selftest part that runs the controller with a filter reducing it to one trivial suite, then reads the stamp file and asserts two fields;
  • or extracting the controller's stamp write into a named function that a part can call directly — cheaper to test and weaker, because the thing that regresses is the call site and a test of the function would not see an argument dropped at the call.

The second is the trap worth naming: the defect here is a missing argument, so an arm that calls the function itself proves nothing about the caller.

Scope

Not the pytest layer. build_once() has its own tests and #957 covers the equivalent there.

Provenance

@jdatcmd asked for this explicitly 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." Filed separately rather than bolted onto #960, which would have widened that PR past the defect it fixes.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions