Repository navigation
chore(tools): declare the system binaries, and check them - #375
Merged
Merged
Conversation
Contributor
Wire-size diff vs merge base
Brotli q11, the quality the edge serves. Every number here is deterministic: Client assetsNo change. 20 files, 74.02 KiB Brotli total. PagesNo change. 48 files, 496.53 KiB Brotli total. Worker bundleNo change. 284.56 KiB gzip. Dictionary deltas (
|
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
aadhar-sh | f7cfe3d | Aug 14 2026, 05:04 PM |
Four external binaries were required by scripts in this repo and documented nowhere: cwebp, ffmpeg, ssimulacra2 and butteraugli_main. On a clean machine each one arrived as a raw shell error partway through a run. They stayed hidden because most of these preconditions are written `for cmd in sips exiftool`, so the binary name exists only as a loop word and a grep for the name finds nothing. That is gotcha 29's blind spot in different clothes: a command assembled from list elements is invisible to a search for the assembled form. config/tools.json declares all 13, and `pnpm run tools:check` verifies them. It sits outside infra.json because that file declares Cloudflare and GitHub state and diffs it against those APIs, while nothing here is remote. Tiered the way infra:check is. The DECLARATION tier reads source text, needs no binary, and gates in CI: every guard loop, every literal `command -v`, and every `brew install` hint must have an entry, and both CLAUDE.md and MAINTENANCE.md must name it. The PRESENCE tier probes the machine and stays advisory in CI, since a hosted runner carries none of the photo toolchain. Each scanner carries a floor and fails if its match count collapses, because a scanner that matches nothing otherwise reports a pass while checking nothing. Six controls, all measured rather than assumed: an undeclared literal probe fails, an undeclared loop word fails, dropping a tool from both docs fails, a stale required_by path fails, CI with the tools absent passes, and a workstation with them absent fails. Two accuracy fixes fell out of the sweep. ssimulacra2 and butteraugli_main are libjxl tools that `brew install jpeg-xl` does not ship, so the error message in export-for-instagram.sh names the wrong thing. And cjpeg is documented as mozjpeg's while gen-encoding-grids.sh calls whichever copy PATH resolves first.
oddharsh
force-pushed
the
chore/tools-check
branch
from
August 14, 2026 17:03
a1e55bf to
f7cfe3d
Compare
oddharsh
enabled auto-merge (squash)
August 14, 2026 17:07
This was referenced Aug 14, 2026
oddharsh
added a commit
that referenced
this pull request
Aug 14, 2026
docs/DEPENDENCIES.md states version pins in prose and dependabot rewrites those pins on a daily schedule, so the file goes stale on a cadence rather than by accident. It did so twice in two days: #371 corrected Wrangler 4.112.0 -> 4.120.0 and merged, then #377 bumped the pin to 4.120.1 hours later. Both times a human found it. This is the same class as gotcha 29's "one version pin in two files", and check-tools.mjs #375 is the precedent for the answer. The check adds NO third copy. tools.json had to declare what no manifest recorded; every number here is already in package.json or requirements.txt, so this reads those as the source of truth and holds the prose to them. Two directions, because either alone is decoration. Forward: every version the doc states for a known package must match its pin. Reverse: every root dependency must be documented or exempted by name with a reason, so adding a dependency forces a documentation decision instead of landing unmentioned. IT FOUND TWO THINGS ON ITS FIRST RUN, and the second is the one worth having. Wrangler was the expected drift. @noble/post-quantum was not documented at all, and it is the ONLY dependency in this repo that reaches a visitor: workerd ships no post-quantum algorithm in WebCrypto, so AadharshBot's ML-DSA-44 sig2 is pure JavaScript in the Worker bundle. It is now documented as such, with the note that its releases want reading for bundle size as well as correctness, because it is Worker bytes rather than build bytes. The scanner is deliberately not a general "Name 1.2.3" sweep. That would be the fourth naive scanner this repo has caught, and DEPENDENCIES.md breaks it three ways in its own text: `CSS Overflow 5`, `Vite 8`, and `TypeScript 7.0` (two components, discussing the major line rather than the pin) all read as version claims. It matches a known alias followed by a full three-component version, and a test asserts those four real sentences yield zero claims. auditDependencyDocs is pure and takes its policy as parameters, so the negative cases run against two-line fixtures rather than needing the whole manifest reconstructed to keep the other rules quiet. Same reasoning as lib/cron.js. Six negative cases assert the check has teeth, including the floor, because a version of this that only ever agreed with the current tree would be the third check here to pass while asserting nothing. Verified: 274/274 contract tests, lint 0, typecheck 0, build 0, pages:check 0. oxlint flagged a useless spread fallback in the first draft, which is the linter that landed this morning earning its place on the same day. Co-authored-by: Aadharsh Pannirselvam <19518661+oddharsh@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four external binaries were required by scripts in this repo and documented nowhere: cwebp, ffmpeg, ssimulacra2 and butteraugli_main. On a clean machine each one arrived as a raw shell error partway through a run.
They stayed hidden for a specific reason. Most of these preconditions are written
for cmd in sips exiftool, so the binary name exists only as a loop word and a grep for the name finds nothing. That is gotcha 29's blind spot in different clothes: a command assembled from list elements is invisible to a search for the assembled form.What this adds
config/tools.jsondeclares all 13 system binaries with an install hint, a reason, and the scripts that need them.pnpm run tools:checkverifies it.It sits outside
infra.jsonon purpose: that file declares Cloudflare and GitHub state andinfra:checkdiffs it against those APIs, while nothing here is remote and nothing here has an API. One file with two unrelated contracts is worse than two files.Tiered the way infra:check is
The declaration tier asserts that every guard loop, every literal
command -v, and everybrew installhint in the shell scripts has an entry, and that both CLAUDE.md and MAINTENANCE.md name it. The presence tier prints what is missing and passes in CI, because a hosted runner carries none of the photo toolchain and never runs the pipeline that wants it.Each scanner carries a floor and fails if its match count collapses. A scanner that matches nothing otherwise reports a pass while checking nothing, which is how the Markdown-twin test went green for months on the wrong field names.
Controls
Six, measured rather than assumed:
command -vrequired_bypathCI=1, tools absentThe check found its four target bugs on its first run, before the docs were written.
Two accuracy fixes that fell out of the sweep
ssimulacra2andbutteraugli_mainare libjxl tools, built with-DJPEGXL_ENABLE_TOOLS=ON. Homebrew'sjpeg-xlformula does not ship them, soexport-for-instagram.sh's error message names the wrong thing. Documented, script message left alone for a separate change.cjpegis documented as mozjpeg's, butgen-encoding-grids.shcalls it bare and PATH here resolves/opt/homebrew/bin/cjpeg, which is libjpeg-turbo's. The chroma row of/garage/encodingis therefore labelled mozjpeg and may not be. Not fixed here, since it changes published sample bytes.Verification
pnpm run lint(oxlint caught one unused variable, fixed),typecheck,build,photos:check,pnpm test271/271,tools:checkall green locally. No served bytes change: the diff touches onlyscripts/,config/, docs and CI, which per gotcha 35 rebuilds nothing.Rebased onto
main2026-08-14Three PRs landed underneath this one, two of them implementing recommendations from the same audit that produced this branch:
zenc histogram. That deletedphoto-histograms.pyand thepython3 -c "import PIL"guard inadd-photos.sh, so this branch'spython3entry was restated: Pillow is now wanted bygen-pixel-peeper.pyalone. The literal-probe count intools:checkmoved 16 → 15 as a result, which is the scanner correctly seeing one fewer guard.One conflict, in
MAINTENANCE.md, where both sides edited the Pillow install line. Resolved towardmain's wording (Pillow, for gen-pixel-peeper.py only), which is the one made correct by #373, keeping this branch's section header and thejpegtran + cjpegnote.