From 26dd7e0bfb9ebd0d18fbdddefafe324bdd82ccee Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 11 Aug 2026 12:38:44 +0000 Subject: [PATCH] fix: correct verify-build diagnostics and close its vacuous passes (closes #180) The both-markers diagnostic claimed the debug branch was still live. It is not: with the __BUILD_DEBUG__ define removed, the emitted bundle carries `typeof __BUILD_DEBUG__<"u"?__BUILD_DEBUG__:!1`, and in extension context the identifier is undeclared, so DEBUG evaluates to false at runtime. The message now states what the check does prove -- DEBUG was not resolved at build time, so the release/debug distinction is no longer enforced and which way the unresolved fallback evaluates is an accident a refactor can flip -- and it remains a hard failure. The other seven failure messages were reviewed and none needed rewording. has_marker no longer swallows grep's exit 2 with 2>/dev/null. Match and no-match are answers about the emitted output; an unreadable file is not, and is now reported as a permissions or I/O fault instead of as "the emitted output changed shape". Both paths still fail hard. The manifest-membership grep gets the same treatment in is_listed: an unreadable manifest is no longer answered as "this file is not listed". The unlisted-bundle scan no longer filters by extension, so the endsWith(".js") test in build.js is the only place that assumption lives. A bundle emitted under another extension previously escaped the manifest and the cross-check at once; it now fails as unlisted. Both sites carry a comment naming the other. That makes the scan the sole guard on build.js's filter, so its walk has to be exhaustive rather than assumed to be. find's exit status was discarded twice over -- the pipeline reported sort's status, and set -e does not fire on an assignment from a successful pipeline -- so a subtree find could not descend printed to stderr and was then silently omitted, and an unlisted marker-carrying bundle inside a chmod 000 directory passed green. The status is now captured and a non-zero find is a hard failure naming the unwalked tree; the sort moved off the status-bearing pipeline. Symlinks are walked as well: a marker-carrying bundle reachable under an unlisted path is a stale manifest whether the path is a link or a file, and a link that cannot be read through fails closed via the exit-2 path. Also: the manifest must be readable and a listed bundle must be non-empty, so a vacuous input fails loudly rather than reaching a marker check that cannot prove anything. --- TODO.md | 6 +++ build.js | 6 +++ script/verify-build | 96 ++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 98 insertions(+), 10 deletions(-) diff --git a/TODO.md b/TODO.md index d202884..4cfa98a 100644 --- a/TODO.md +++ b/TODO.md @@ -44,6 +44,12 @@ undefined identifiers, which is how # Completed Steps +- 2026-08-11: `script/verify-build` diagnostics corrected: the both-markers + message now states what is and is not proven, an unreadable bundle is + diagnosed as an I/O fault rather than as changed output, the `*.js` assumption + lives only in `build.js`, and the unlisted-bundle scan hard-fails when it + cannot enumerate `dist/` + ([#180](https://git.eeqj.de/sneak/AutistMask/issues/180)). - 2026-08-11: Three `README.md` claims corrected against the code — blocklist attribution, token-display rule, navigation model ([#213](https://git.eeqj.de/sneak/AutistMask/issues/213)). diff --git a/build.js b/build.js index 0ecdad4..6a9fe31 100644 --- a/build.js +++ b/build.js @@ -29,6 +29,12 @@ function repoRelative(p) { // reports every input that contributed to an output in the metafile, which is // the authoritative answer to "is constants.js in this bundle" — unlike // searching the minified text, it does not depend on what survived minification. +// +// The ".js" filter below is the only place that assumption lives: +// script/verify-build searches every file and symlink under dist/ for a +// marker, without filtering by extension, and hard-fails if it cannot walk the +// whole tree, so a bundle emitted under some other extension fails there as +// unlisted rather than escaping both checks at once. function outputsContainingAuditedModule(metafile) { return Object.entries(metafile.outputs) .filter(([outFile, info]) => { diff --git a/script/verify-build b/script/verify-build index 23b1b7c..6e6bf75 100755 --- a/script/verify-build +++ b/script/verify-build @@ -34,16 +34,51 @@ fail() { exit 1 } +# Is the literal $1 present in the file $2? Match (grep exit 0) and no-match +# (exit 1) are answers about the emitted output. Anything else (exit 2: the +# file could not be read) is not an answer at all, and must not be reported as +# "no marker" — that would blame the bundle for a permissions or I/O fault. has_marker() { - grep -q -F "$1" "$2" 2>/dev/null + _hm_status=0 + grep -q -F -e "$1" -- "$2" || _hm_status=$? + case "$_hm_status" in + 0) return 0 ;; + 1) return 1 ;; + *) + fail "grep exited $_hm_status reading $2, so the file could not be + searched and its DEBUG state was not checked at all. That is a permissions + or I/O fault on the artifact, not a change in the emitted output. Refusing + to report success." + ;; + esac +} + +# Does the manifest list the path $1, as a whole line? Same discipline as +# has_marker: exit 0 and 1 are answers about the manifest, exit 2 means the +# manifest could not be read and is not an answer at all. Without this, an +# unreadable manifest reads as "this file is not listed" and every emitted +# bundle gets reported as an unlisted one. +is_listed() { + _il_status=0 + grep -q -x -F -e "$1" -- "$MANIFEST" || _il_status=$? + case "$_il_status" in + 0) return 0 ;; + 1) return 1 ;; + *) + fail "grep exited $_il_status reading $MANIFEST, so it could not be + searched and nothing was established about which bundles it lists. That is + a permissions or I/O fault on the manifest, not a stale manifest. Refusing + to report success." + ;; + esac } # Read one bundle's DEBUG state into MARKER. Exactly one marker must be # present. Both means the ternary in constants.js was never folded, which is # what happens when the __BUILD_DEBUG__ define goes missing from build.js: -# DEBUG stops being known at build time and the debug branch is live again. -# Neither means we are reading output we do not understand. Both are hard -# failures; neither is ever treated as absence of a problem. +# DEBUG stops being known at build time. Neither means we are reading output +# we do not understand. Both are hard failures; neither is ever treated as +# absence of a problem. read_marker() { _file="$1" _on=no @@ -52,9 +87,14 @@ read_marker() { if has_marker "$MARKER_OFF" "$_file"; then _off=yes; fi if [ "$_on" = yes ] && [ "$_off" = yes ]; then - fail "$_file carries both debug markers, so the build-time DEBUG value - was never resolved and the debug branch is still live. Check that build.js - still defines __BUILD_DEBUG__." + fail "$_file carries both debug markers, so DEBUG was not resolved at + build time: the ternary in src/shared/constants.js survived into the + emitted output. This does not mean the debug branch is live in this + artifact: an unresolved __BUILD_DEBUG__ is undeclared in extension + context, so DEBUG evaluates to false at runtime. It does mean the + release/debug distinction is no longer enforced at build time, and which + way that fallback happens to evaluate is then an accident a refactor can + flip. Check that build.js still defines __BUILD_DEBUG__." fi if [ "$_on" = no ] && [ "$_off" = no ]; then fail "$_file carries no debug marker, so its DEBUG state cannot be @@ -70,13 +110,43 @@ read_marker() { } # The manifest says which bundles must carry a marker. This says no other -# emitted bundle may carry one, which catches a manifest that has gone stale +# emitted file may carry one, which catches a manifest that has gone stale # or short rather than trusting whatever it happens to list. +# +# Deliberately unfiltered by extension. build.js selects manifest entries with +# an endsWith(".js") test; repeating that literal here would mean a bundle +# emitted under some other extension escaped the manifest AND this check at +# once, which is the correlated blind spot the two-source design exists to +# avoid. Every file under dist/ is searched, so build.js's filter is the only +# place the assumption lives and this check is what catches it being wrong. +# +# That claim only holds if the walk is exhaustive, so two things are enforced +# here rather than assumed: +# +# - find's exit status is checked. A subtree it cannot descend is reported on +# stderr and then simply missing from the listing, so an unchecked status +# turns "could not look" into "nothing was there" — the same conflation +# has_marker exists to prevent. The status cannot be read off a pipeline +# ending in sort, so the sort is a separate step. +# - symlinks are walked too (-type l), not skipped. A marker-carrying bundle +# reachable under an unlisted path in dist/ is a stale manifest whether the +# path is a link or a file, and grep reads through the link. A link that +# cannot be read through — dangling, or pointing at a directory — fails +# hard via has_marker's exit-2 path, which is the fail-closed answer: the +# build emits neither, so their DEBUG state is unproven, not fine. check_unlisted_bundles() { - _listing="$(find dist -type f -name '*.js' | sort)" + _find_status=0 + _listing="$(find dist \( -type f -o -type l \) -print)" || _find_status=$? + [ "$_find_status" -eq 0 ] || + fail "find exited $_find_status enumerating dist/, so part of the tree + was never walked and nothing was established about the files in it. Any + unlisted bundle there went unchecked. That is a permissions or I/O fault on + the artifact, not a stale manifest. Refusing to report success." + _listing="$(printf '%s\n' "$_listing" | sort)" + while read -r _file; do [ -n "$_file" ] || continue - if grep -q -x -F "$_file" "$MANIFEST"; then + if is_listed "$_file"; then continue fi if has_marker "$MARKER_ON" "$_file" || @@ -113,12 +183,18 @@ main() { fail "$MANIFEST is empty, so no emitted bundle was found to contain src/shared/constants.js. That is never correct, so it is a failure and not a pass." + [ -r "$MANIFEST" ] || + fail "$MANIFEST is not readable, so nothing was inspected. That is a + permissions or I/O fault, not a pass." count=0 while read -r file; do [ -n "$file" ] || continue [ -f "$file" ] || fail "$MANIFEST lists $file, which does not exist." + [ -s "$file" ] || + fail "$MANIFEST lists $file, which is empty. An empty bundle + carries no marker and proves nothing, so this is a failure and not a pass." read_marker "$file" [ "$MARKER" = "$expected" ] || fail "$file is $MARKER but this build expects $expected." -- 2.49.1