Repository navigation
check-submodules: the release blocks a build tree that is not the pinned tree, and warns when there is nothing to compare (#2129) - #2177
bong-water-water-bong wants to merge 3 commits into
Conversation
…g checked the checkout (engine#2129) #2129 corrected its own title: mtmd cannot block an engine build, the engine does not build the fork, and the pin itself is healthy. What it did establish is narrower and worse, and this adds the check that would have said so. Measured on this box while triaging it -- 7 of 7 submodules disagreed with the commit the tree pins: third_party/hrx-system pin 7f4236924 head 51b1739ae dirty 0 third_party/llama.cpp pin 2293f95ea head c16424f56 dirty 301 <- the issue's "301 files short of its own HEAD" third_party/xdna-driver pin 526c60819 head 5d302c998 dirty 32 third_party/laya pin b8a0f0991 head 593a159 dirty 4 third_party/tokenizers pin 301277072 head 88a4498ad dirty 20 third_party/lemonade pin 248f818b4 head 593a159 dirty 4 third_party/ggmlc pin df08d1b06 head 593a159 dirty 4 CI checks the PINS -- "submodule pins only move forward", "hrx-system gitlink agrees with the hold file" -- and those are checks on the repository. A build reads the tree on the box. "The pin is healthy" and "the checkout is the pin" are two different claims and nothing made the second one, which is how a tree can build 301 uncommitted files and report success. scripts/check-submodules.sh checks the second claim: per submodule, HEAD against the gitlink, and the worktree against HEAD. Dirty is a failure even when HEAD matches -- a checkout whose worktree differs builds sources the pin does not name, the same defect with a smaller diff -- and it is reported with its count so a reader can tell a stray file from a reset. Exit 0 clean, 1 on any disagreement, 2 when there is nothing to inspect: an unchecked tree is the state this exists to end, so "no .gitmodules" is not a pass. The ctest row is the --self-test only, and it is hermetic: two throwaway local repos and a real `submodule add` over file protocol, so no network, no submodule of this tree, no build. The six arms are a clean checkout at its pin passing, a checkout ahead of its pin failing and naming both commits, a dirty checkout at the right pin failing and saying the worktree differs, and no .gitmodules exiting 2 rather than passing. The --root mode is deliberately NOT a ctest row: CI checks out at the pin by construction, so as a row it would test the runner, not the tree. It is for the box, before a build: scripts/check-submodules.sh --root . -> 1, naming all seven, today Registered as cmake/tests-submodules.cmake, a NEW file, so this touches no line of CMakeLists.txt -- the glob at CMakeLists.txt:1979 picks it up, which is the mechanism engine#1979 wants and why this PR has no rebase cost at 19 open PRs on that file. `ci_targets_check.py --where submodules_self` -> cmake/tests-submodules.cmake:13.
PR Reviewer Guide 🔍(Review updated until commit 7d898a4)Here are some key observations to aid the review process:
✅ Resolved findingsscripts/check-submodules.sh:1-155Submodule Integrity Check The script checks that each submodule's checkout matches its pinned commit and is clean, which is critical for ensuring that builds use the intended versions of dependencies. This is especially important for the scripts/weekly.sh:669-682Preflight Integration The preflight function scripts/check-submodules.sh:1-133Submodule Integrity Check The script checks that each submodule's checkout matches its pinned commit and is clean, which is critical for ensuring that builds use the intended versions of dependencies. This is especially important for the cmake/tests-submodules.cmake:1-13CI Integration The new CMake test file integrates the submodule integrity check into CI. However, it's important to note that this check only verifies the checkout state and does not address the core issue of CMake registration hunks going stale, which is a separate concern raised in issue #1979. |
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT at 5f6c830: CHANGES
Reviewed head 5f6c830 (refs #2129). This is the first verdict.
CI is red on this PR's own file. build's failed step is "Copyright notice on every file": missing copyright notice: cmake/tests-submodules.cmake. The new family file needs the full Apache notice, like cmake/tests-submodule-pin.cmake.
This is a second checker for what #2141 already merged. tools/ci/submodule_pin_check.py (#2141, on main, registered from cmake/tests-submodule-pin.cmake) already fails when a submodule's checkout is not the pinned commit. It reads git submodule status --recursive and has --require-initialised for the release form. I reviewed it on this box: it caught my own checkout's third_party/llama.cpp off its pin. So the "checkout ahead of its pin" row of the table here is #2141's.
What is genuinely new here is the dirty worktree at the right pin. git submodule status cannot see it (+ is a HEAD mismatch only), and the 301 modified files of #2129 are exactly that shape. That is worth having. But as a second tool, in shell, with its own exit codes (0/1/2, while #2141 uses 0/1/2 with different meanings for "nothing to inspect"), the two will drift.
Suggested: add the dirty check to submodule_pin_check.py. For each initialised submodule, git -C <path> status --porcelain non-empty fails, with the count, as here. Keep this PR's hermetic fixture (two local repos, a real submodule add over file protocol) as that checker's self-test arm. Then there is one place that answers "does the build's tree match the pin".
…tree, and only warns when there is nothing to compare (engine#2129)
The script in the previous commit answers a question nobody was asking yet. This makes the release ask it: the `bumps` and
`build` stages build the tree in front of them, so `preflight` runs the check there and stops the run on a checkout that
disagrees with the pin -- the build would not be of the pinned sources, and until now that was silent. Exit 3 (submodules not
checked out at all, which is what a fresh clone looks like) only WARNS, because a fresh box must still be able to build; the
warning says the check inspected nothing rather than implying it passed, which is the exit-3 discipline of engine#2094.
`check-submodules.sh` therefore grew that third state, and its self-test earned its keep immediately by catching a real bug in
the first version: `git -C` inside an unchecked-out submodule directory is not an error, it walks UP and reports the PARENT's
HEAD -- so a fresh clone was being reported as "checked out at the wrong commit". A checkout now has to be its own repository
root (`rev-parse --show-toplevel` equal to the path) before its HEAD means anything.
states: 0 every checkout is the pin and clean | 1 a checkout disagrees (blocks) | 3 nothing checked out (warns) | 2 no
.gitmodules (warns, cannot happen here but "cannot tell" is the honest answer)
Tests, all through the real code:
scripts/check-submodules.sh --self-test 7/7 arms (clean at pin, ahead of pin naming both commits, dirty at the right
pin, fresh clone = 3, no .gitmodules = 2, and unrelated repo ignored)
tests/weekly_submodule_preflight_test.sh 7/7 rows, the function cut out of weekly.sh the way
tests/weekly_runner_preflight_test.sh cuts preflight_runner_units, run
against stub checkers: a disagreeing tree returns 1, exit 2 and 3 return 0
with a WARNING, a missing checker returns 0
`tests_referenced_check.py`: 83/83 test scripts referenced, both rows resolve --
`ci_targets_check.py --where submodules_self` -> cmake/tests-submodules.cmake:13, `--where weekly_submodule_preflight` -> :18.
Still no CMakeLists.txt line: the glob at CMakeLists.txt:1979 registers the family file, which is what engine#1979 wants.
Measured on this box, which is why this is worth blocking on: 7 of 7 submodules disagree with the pin (llama.cpp 2293f95ea vs
c16424f56, 301 files dirty). Repairing those checkouts is fleet state and stays with the owner; this is the part that makes the
next build say so instead of building 301 uncommitted files and reporting success.
bong-water-water-bong
changed the title
|
Persistent review updated to latest commit 4791221 |
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT at 4791221: CHANGES
Re-review of 4791221 (one commit, force-pushed over 5f6c830). Both points from my CHANGES at 5f6c830 are still open.
1. CI build is red, for the same cause as before. The Copyright notice on every file step fails with missing copyright notice: cmake/tests-submodules.cmake. The file carries only the two SPDX lines. tools/copyright.py --check requires the full Apache notice, which is lines 2-15 in every other cmake/tests-*.cmake. python3 tools/copyright.py --fix writes it.
2. This duplicates tools/ci/submodule_pin_check.py (#2141, already on main). That checker answers "is each checkout its pin" from git submodule status. The new 155-line script answers the same question again. The one thing it adds is the dirty-worktree check (a checkout at the right commit with uncommitted files), and that part is useful. Please add it to the existing checker, and have weekly.sh's preflight call that one, so the box has a single answer to the question. Otherwise, say in the body why two checkers are needed. Right now the body does not mention #2141.
What works at this head (run from a git archive of 4791221):
scripts/check-submodules.sh --self-test: ok, including the fix where--show-toplevelmust equal the path for an unchecked-out directory.tests/weekly_submodule_preflight_test.sh: ok (exit 1 blocks, exit 2/3 warn).- The family file is picked up by the
cmake/tests-*.cmakeglob (CMakeLists.txt:1979), so CI does register both rows.
…wo-liner CI's "Copyright notice on every file" step failed on this PR: tools/copyright.py only accepts the SPDX pair in a file that also carries the license block, and a .cmake file starts with the block. Reproduced locally in a worktree (the checker needs a real index, so it cannot be run from an archive) and fixed with the repository's own `--fix`. Worth recording, because it is a trap rather than a mistake: `--fix` prepends the block and LEAVES an existing two-liner, so the file it just fixed fails its own `--check` with "duplicated inside the header window". Neither mode says that the state it produced is invalid; only a second `--check` does. The duplicate was removed by hand here; a follow-up can make `--fix` drop an in-window notice instead of adding a second one. Local, on the tree that is pushed: copyright --check clean, shellcheck -S warning clean on both new shell files, tests_referenced_check 83/83.
|
Persistent review updated to latest commit 7d898a4 |
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
Review at 7d898a4 — no verdict token (author's lane). I ran both tests rather than reading them, and the design decision that matters here — "cannot tell" is a third state, not a pass — is implemented all the way through.
Ran:
check-submodules.sh --self-test -> ok
ok a dirty checkout fails even when HEAD is the pin
ok and it says the worktree differs from its HEAD
ok an unchecked-out submodule is exit 3 (cannot tell), not 1 and not 0
ok no .gitmodules is 'cannot tell' (2), not a pass
weekly_submodule_preflight_test.sh -> ok
ok no .gitmodules only warns -> return 0
ok exit 2 is not a block ok exit 3 is not a block
- The dirty arm is the one that earns this check. "A dirty checkout fails even when HEAD is the pin" is precisely the
llama.cpprow in your table (pin2293f95ea, 301 dirty files): HEAD matching would have declared that tree pinned, and the build would have compiled 301 files the pin does not contain. (That hazard is live in the shared clone too —~/enginecarries ~220 lines of another lane's uncommitted work innpu/pack_import*, so this check would catch it there as well.) - The tri-state is real, not documented: 0 pass, 1 block, 2 no-
.gitmodules/no-git("nothing to inspect, which is not a pass"), 3 nothing-to-compare. And the preflight honours it in the direction that matters — it blocks only on 1 and lets 2/3 through as a warning, so a can't-tell week does not fake a red or a green. - The warning text is honest. "WARNING, not blocking: no checkout to compare against the pin, so the build follows the tree rather than this check" — it says the build will not be of the pinned sources, which is the sentence that stops a release note claiming otherwise.
- Registered through the glob-included family file (
cmake/tests-submodules.cmake:submodules_self,weekly_submodule_preflight), which is the mechanism that keeps a test from being written and never run.
One gap, in the can't-tell path only. When the preflight warns (exit 2/3), the release proceeds and the build is of some tree — but nothing records which. The check's own table shows 7/7 submodules disagreeing on this box, so "which tree was built" is exactly the question a release artifact should answer after the fact. Since the checker already computes pin/head/dirty per submodule, writing that table into the build log (or beside the artifact) on the warn path would make a can't-tell release auditable: "built c16424f56 + 301 dirty, pin 2293f95ea" instead of "build followed the tree". One echo of a summary line, on the path that already exists.
Nothing else: the block path is right, the wiring into weekly.sh accumulates into its failure rather than swallowing it, and the test stubs the checker so it cannot pass by accident.
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT at 7d898a4: CHANGES
Re-review of 7d898a4. It adds 7d898a4 (the family file now carries the full Apache header).
Fixed: tools/copyright.py --check gives rc 0 at this head, and CI build is a success.
Still open, and now measured: this is a second checker for a question main already answers. On the same working tree (my review checkout, where third_party/llama.cpp sits at 7cc3187b5 against a pin of 204493d40):
| rc | finding | |
|---|---|---|
main's tools/ci/submodule_pin_check.py --check --root . (#2141) |
1 | third_party/llama.cpp is checked out at 7cc3187b5f4e, which is NOT the commit this tree pins; the uninitialised ones are "not compared" |
this PR's scripts/check-submodules.sh --root . |
1 | third_party/llama.cpp is not the pin: tree 204493d40, checkout 7cc3187b5; the uninitialised ones are warnings |
Same verdict, same submodule, same three states (pass, mismatch fail, "not initialised, not compared"), and both cite #2129. What this PR adds that main does not have:
- (a) failing a checkout that is at the pin but has a dirty worktree;
- (b) the
weekly.shpreflight that calls it.
Both are worth having. Taking them as a second 155-line script means two implementations that will drift. Please put (a) into submodule_pin_check.py (one git -C <path> status --porcelain per initialised submodule, plus a self-test arm), and have preflight_submodules() call python3 tools/ci/submodule_pin_check.py --check --root "$ENGINE_DIR". If there is a reason the box needs a separate shell checker, say it in the body. Neither earlier review got a reply on this point.
Works at this head (from a git archive of 7d898a4): check-submodules.sh --self-test ok, and weekly_submodule_preflight_test.sh ok.
…e is already a two-liner Met while fixing a new file's header in #2177, and it is the kind of defect this tool exists to prevent, in the tool: $ printf '# Copyright 2026 bong-water-bong\n# SPDX-License-Identifier: Apache-2.0\n\nx = 1\n' > twoliner.py $ python3 tools/copyright.py --check run: python3 tools/copyright.py --fix missing copyright notice: twoliner.py $ python3 tools/copyright.py --fix added: twoliner.py $ python3 tools/copyright.py --check run: python3 tools/copyright.py --fix copyright notice duplicated inside the header window (delete the second copy): twoliner.py `--fix` reported success and left an invalid file. Neither mode says the state it produced is invalid; only a second `--check` does, and the instruction it then prints is the one thing `--fix` cannot do. The cause is a gap between two predicates that each look right alone. `has_notice()` wants the licence block, so a file carrying only the SPDX pair is "missing". `near_miss_holder()` filters what `add_notice()` drops before prepending the block, and it returns False for an *exact* holder **by design** -- an exact holder beside the canonical block is the *repaired* state (engine#1606). So the exact pair survived the prepend and became the second copy. The pair is not a second claim: the block `add_notice()` inserts contains both lines. Now `add_notice()` drops an in-window line that IS one of the canonical notice lines, so the merge is a merge. Two arms in tests/copyright_gate_test.sh, and they bite: against main's `tools/copyright.py` the second one fails with "window: 2 SPDX line(s)", which is exactly the duplicate. The first arm alone passes on old code -- the second `--check` there still finds a duplicate, so what actually fails to be one notice is that count, and that is what is asserted. Verified: tests/copyright_gate_test.sh PASS with the patch, FAIL (2 SPDX lines) without it; `tools/copyright.py --check` clean on this tree. The first push of this branch also carried four files staged for #2177's header check -- `git add` is needed for `tools/copyright.py` to see a new file, and the following `git commit` then took the whole index. Rebuilt from main with the two files this PR is about.
|
Withdrawn — superseded by #2202, and by main. What was genuinely additive is in #2202: (1) the state Three things from this PR worth keeping in the record even though the code goes:
|
|
ORCHESTRATOR decision (posted by cl-review on the orchestrator's instruction): there is one submodule checker, |
…e is already a two-liner (#2192) * copyright: --fix leaves a file its own --check rejects when the notice is already a two-liner Met while fixing a new file's header in #2177, and it is the kind of defect this tool exists to prevent, in the tool: $ printf '# Copyright 2026 bong-water-bong\n# SPDX-License-Identifier: Apache-2.0\n\nx = 1\n' > twoliner.py $ python3 tools/copyright.py --check run: python3 tools/copyright.py --fix missing copyright notice: twoliner.py $ python3 tools/copyright.py --fix added: twoliner.py $ python3 tools/copyright.py --check run: python3 tools/copyright.py --fix copyright notice duplicated inside the header window (delete the second copy): twoliner.py `--fix` reported success and left an invalid file. Neither mode says the state it produced is invalid; only a second `--check` does, and the instruction it then prints is the one thing `--fix` cannot do. The cause is a gap between two predicates that each look right alone. `has_notice()` wants the licence block, so a file carrying only the SPDX pair is "missing". `near_miss_holder()` filters what `add_notice()` drops before prepending the block, and it returns False for an *exact* holder **by design** -- an exact holder beside the canonical block is the *repaired* state (engine#1606). So the exact pair survived the prepend and became the second copy. The pair is not a second claim: the block `add_notice()` inserts contains both lines. Now `add_notice()` drops an in-window line that IS one of the canonical notice lines, so the merge is a merge. Two arms in tests/copyright_gate_test.sh, and they bite: against main's `tools/copyright.py` the second one fails with "window: 2 SPDX line(s)", which is exactly the duplicate. The first arm alone passes on old code -- the second `--check` there still finds a duplicate, so what actually fails to be one notice is that count, and that is what is asserted. Verified: tests/copyright_gate_test.sh PASS with the patch, FAIL (2 SPDX lines) without it; `tools/copyright.py --check` clean on this tree. The first push of this branch also carried four files staged for #2177's header check -- `git add` is needed for `tools/copyright.py` to see a new file, and the following `git commit` then took the whole index. Rebuilt from main with the two files this PR is about. * copyright: --fix also repairs the block pasted twice, which it used to report as `repaired:` and leave (engine#2192) The two-liner case in the previous commit was one of two states in which `--fix` printed success and left a file its own `--check` rejects. The other is the block pasted twice: $ python3 copyright.py --check copyright notice duplicated inside the header window (delete the second copy): doubled.py $ python3 copyright.py --fix repaired: doubled.py $ python3 copyright.py --check copyright notice duplicated inside the header window (delete the second copy): doubled.py `repair_notice()` is the path `--fix` takes for a file that already has the canonical notice, and it existed for the near-miss-beside-canonical shape (engine#1606): it drops a typo'd holder and the SPDX line beneath it, keeps everything else, and falls back to `add_notice()` when the result no longer has a notice. A *second canonical* notice is neither of those things, so it passed through untouched -- and `has_notice()` was still true, so the fallback never ran either. It now keeps the first occurrence of each canonical notice line and drops later copies, which is what `--check`'s own instruction ("delete the second copy") asks for and what the word `repaired:` has always claimed. Dual attribution is untouched: `Copyright 2026 The 1bit-MONSTER Authors.` is not a canonical line, so it survives beside the block exactly as before. Two arms, and they bite: against the previous `repair_notice()` the SPDX count in the window is 2 and the arm fails ("...and exactly one SPDX line survives the repair"), so the whole gate test is FAIL without the fix and PASS with it. The first arm (that `--check` stops reporting a duplicate) passes on old code, which is worth knowing: it means the *count* is the assertion that has teeth here, not the absence of the word "duplicated" -- the same asymmetry as the two-liner case in the previous commit. --------- Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
…s 'at their pinned commit' (#2129) (#2202) * submodule_pin: a submodule at its pin with a dirty work tree passes as "at their pinned commit" engine#2129 is already fixed on main by tools/ci/submodule_pin_check.py -- another lane's, registered in cmake/tests-submodule-pin.cmake as a ctest row -- and it reads `git submodule status`, which reports four states. This adds the fifth, which that command cannot report, and the wiring that runs the check before a build instead of only in CI. **The missing state: at the pin, not clean.** `git submodule status` prints ' ' for "the checkout IS the pinned commit" whatever the submodule's own work tree holds, and the build compiles that work tree. Measured on the box that motivated #2129: on top of the checkouts being off-pin, `third_party/llama.cpp` had 301 uncommitted paths, `xdna-driver` 32, `tokenizers` 20, and four others 4 each. Every one of those would read as "at their pinned commit" to the existing check. Reported always, with its count, and a failure only under the new `--require-clean`: a deliberate local patch in a submodule is a real thing a box does, and moving the pin is not always the caller's to do, so the default is to say it rather than to stop. Five self-test arms, including that a dirty '+' row is not double-counted (the commit is the failure there). **The wiring: nothing ran it before a build.** A ctest row runs in CI; the drift was measured on the box, where the release and the weekly build actually run. `scripts/weekly.sh`'s preflight now runs the check for the `bumps` and `build` stages through `preflight_submodules()` (a function of its own, testable against stub checkers like `preflight_runner_units`): exit 1 **blocks** the run -- the build would not be of the pinned sources -- and exit 2 (no submodule initialised, i.e. a fresh clone) only **warns**, saying that the check inspected nothing rather than implying a pass. tools/ci/submodule_pin_check.py --self-test 19 arms, 5 of them new tests/weekly_submodule_preflight_test.sh 6/6, the function cut out of weekly.sh, stub checkers tools/copyright.py --check clean tools/ci/tests_referenced_check.py 83/83 referenced `--require-clean` is deliberately not passed in the preflight: a dirty submodule is reported there and does not stop a build, while a wrong commit does. This supersedes #2177, which grew a parallel `scripts/check-submodules.sh` before main's tool landed. Two checks for one property is the failure mode the standing rules name, so that script is withdrawn rather than maintained beside it. * weekly: the submodule gate runs in build(), not preflight() -- where it was a no-op that reported success Found by checking my own wiring against where the variables are set rather than against whether the tests pass. `SRC=$W/src` is created by `checkout`, which the `bumps` and `build` stages call, and `preflight()` runs before any stage -- so `[ -f "$SRC/tools/ci/submodule_pin_check.py" ]` was false on every real run, the function returned 0, and the gate inspected nothing while reporting success. A preflight row that cannot see the tree it is meant to inspect is worse than no row: it reads as verified. The gate now runs at the top of `build()`, immediately after `checkout` has updated `$SRC`'s submodules and before the first compile -- the tree that is about to be compiled, stopped before the compile rather than after it. `build()` returning non-zero fails the stage. And the missing-checker path now says so. An older ref does not carry the tool, so that build's submodule drift is not checked; it prints a WARNING naming the ref's reason instead of returning 0 silently. That was the same defect one level up: the function's first version treated "no file to run" as "fine", which is the reporting shape this whole check exists to remove. tests/weekly_submodule_preflight_test.sh 6/6, including that a missing checker warns rather than passing quietly tools/ci/submodule_pin_check.py --self-test 19 arms tools/copyright.py --check clean * weekly_submodule_preflight_test: pin WHERE the gate runs, not merely that it exists (engine#2129) The previous commit moved the submodule gate from preflight() to build() because in preflight() it was a no-op that reported success: preflight runs before `checkout` creates $SRC, so the checker file was not there, the function returned 0, and a green run meant nothing had been inspected. The test it shipped with could not catch that regression. It asserted grep -q 'preflight_submodules "' scripts/weekly.sh -- "present anywhere in the file", which is true in exactly the broken state. The test and the bug agreed. It now extracts the two functions and asserts the call is inside build() and NOT inside preflight(), so moving it back fails with the reason rather than passing quietly: FAIL scripts/weekly.sh's build() no longer runs the submodule gate (a call elsewhere is the no-op this test exists for) FAIL the submodule gate is inside preflight(), which runs before $SRC exists: it would inspect nothing and report success Verified by making that move: with the call restored to preflight() both assertions fail and the row exits 1; with the call in build() the row passes. That is the negative control this test never had. --------- Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
Refs #2129— not a close: the issue's own correction (the engine does not build the fork,mtmdcannot block an engine build, the pin is healthy) leaves its real content intact, and this is the check that content needed — plus the wiring that makes the release ask it.The measurement
7 of 7 submodules on this box disagree with the commit the tree pins:
third_party/hrx-system7f423692451b1739aethird_party/llama.cpp2293f95eac16424f56third_party/xdna-driver526c608195d302c998third_party/layab8a0f0991593a1596fthird_party/tokenizers30127707288a4498adthird_party/lemonade248f818b4593a1596fthird_party/ggmlcdf08d1b06593a1596fWhy CI could not have caught it
CI checks the pins — "submodule pins only move forward", "hrx-system gitlink agrees with the hold file" — and those are checks on the repository. A build reads the tree on the box. "The pin is healthy" and "the checkout is the pin" are two claims, and nothing made the second one. That is how a tree can build 301 uncommitted files and report success.
What this adds
scripts/check-submodules.sh— HEAD against the gitlink and the worktree against HEAD, with four honest states:.gitmodules(cannot happen here)Dirty fails even at the right pin: a worktree differing from its HEAD builds sources the pin does not name, the same defect with a smaller diff.
The self-test earned its keep immediately by catching a real bug in my first version:
git -Cinside an unchecked-out submodule directory is not an error — it walks up and reports the parent's HEAD, so a fresh clone was reported as "checked out at the wrong commit". A checkout now has to be its own repo root (--show-toplevelequal to the path) before its HEAD means anything.Wiring: the release now asks the question
scripts/weekly.sh'spreflightruns it for thebumps/buildstages, throughpreflight_submodules()— a function of its own so it is testable against stubs, exactly likepreflight_runner_units(#1504). Exit 1 stops the run before the first stage; 2/3 are warnings that say the check inspected nothing rather than implying it passed (the exit-3 discipline of #2094).Verification
No
CMakeLists.txtline: the glob at:1979registers the family file — the mechanism #1979 wants, at a moment when 19 open PRs are editing that file.Not closing #2129: this reports the drift and stops a build from ignoring it; repairing the seven checkouts is shared fleet state other lanes may be building from right now, so it stays with the owner.