Repository navigation
fix(tools): scripts that moved out of www/ still resolve their output from it - #525
Merged
Merged
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
aadhar-sh | dd18360 | Aug 24 2026, 01:07 AM |
Contributor
Wire-size diff vs merge base
Brotli q11, the quality the edge serves. Every number here is deterministic: Client assetsNo change. 22 files, 77.74 KiB Brotli total. PagesNo change. 50 files, 531.29 KiB Brotli total. Worker bundleNo change. 237.91 KiB gzip. Dictionary deltas (
|
… from it Eight sites across six photo and asset scripts find their output with `$SCRIPT_DIR/..`. That was the served tree when these lived in www/scripts/; it has been tools/ since #452 and #453, so extract-photo-metadata.sh was writing tools/images/metadata.json, hash-thumbnails.sh was hashing into tools/i/, and add-car-photo.sh was aiming at tools/cars. add-photos.sh passed `--root "$PROJECT_DIR/www"`, and www/ stopped existing on 2026-08-18. A rename sweep greps for the old string, and there is no old string here, only a relative expression whose meaning is a function of where the file sits. Two things then kept it silent: `zenc histogram` prints "cannot read" and exits 0, and both callers pipe it to `tail -1`, so the bake skips without failing. And the two scripts resolving through $PROJECT_DIR were correctly updated by the split, which left the pipeline looking half-migrated rather than broken. No photo has been added since the move, so nobody ran into any of it. The same rename left build.mjs's taste exemption anchored on /^www\/(garage| lwe)\//, matching nothing, so the demo pages it exists to exempt lost the exemption and the build printed 14 warnings on every run since 2026-08-18. A warn block nobody can clear is how a warn block gets ignored. Re-anchored on the SERVED_SOURCES roots. Controls both ways: an injected @font-face url() on a garage page still hard-fails, and a 99px radius there is silent again. lens-seed.mjs still spawned execFileSync("pnpm", ["exec", "wrangler", ...]), which refuses outright on a bun tree. The contract test that bans package manager spawns carried lens-seed.mjs as its one RECORDED exception, justified as driving a package script rather than using a manager to reach another binary. That described no code in the file. So the single genuine offender was the single exempted file and the suite reported green. The spawn goes through wranglerCommand() now and the allowlist is gone; a control confirms the guard catches what it used to exempt. Also dead or wrong, removed or corrected: - explorerChrome(), exported and called by nothing anywhere, including its own file - an isCallable doc comment orphaned fifteen lines above its function - build.mjs told you to run node public/scripts/build-histogram-index.mjs, which is not a path - check-wrangler.mjs told you to run `pnpm install`. Its transitive-copy scan reads node_modules/.pnpm and so reports 0 forever on a bun tree; the logic is unchanged and the blind spot is now named at the code - MAINTENANCE.md documented `zenc histogram --root www` and two .ts modules as .js CLAUDE.md counts re-derived from the tree: 36 to 37 documents, 46 to 49 test files, 117 to 147 relative specifiers, 474 to 632 hashed tiers (the 200px tier from #441 was never written down), 2645/48 to 2921/50 link refs. Gotcha 16 named contract-tests.mjs under `node --test`; both runners were measured, and the rule holds while both of its symptoms changed, so the documented error string now finds nothing and the blast radius is one file rather than the run. Gotcha 39 records the $SCRIPT_DIR/.. class so the next rename has something to check against. 353 tests, typecheck, lint, build, pages, photos and the 142-route oracle all green.
oddharsh
force-pushed
the
claude/stale-files-code-cleanup-215deb
branch
from
August 24, 2026 01:06
92e3f13 to
dd18360
Compare
oddharsh
added a commit
that referenced
this pull request
Aug 24, 2026
…uarded ENFORCE_PAGE_HASHES was pinned true on 2026-08-16 and nothing has flipped it in either direction since, so its false arm has been dead code for a week. It was not one line: it cost a second header name, a second tail constant, a `tail` parameter threaded through both policy builders to feed that constant, an entries loop in withSecurityHeaders whose only job was carrying a second header past a bail, and two branches apiece in the contract test and the twin-fact tripwire. The emitted policy is byte-identical, checked both ways. CSP_TAIL now holds the string CSP_TAIL_REPORTABLE plus "; upgrade-insecure-requests" used to compose, and a build of each side produces the same _headers line. cspHeadersFor returns exactly ONE header now. The contract test counts the keys rather than probing one name, since a twin under any other spelling is the same regression, and the upgrade-insecure-requests assertion inverts with the change: it used to check the twin OMITTED the directive, and with one policy left the only way to get it wrong is to lose it, so it now checks presence on both arms. gen-md-twins no longer reads the flag. It reads the header set cspHeadersFor actually emits, which is the better check for the reason the flag era proved: a flag states an intention, and #249's bug was the enforcing half never being applied to a single document while the flag said it was. /security claims 'unsafe-inline' left script-src, and that claim goes false the moment a report-only twin returns, so the deploy fails until one of the two is put right. Controls, both run: reintroducing a report-only twin fails the contract test by name (it reports the two keys it found) and fails the twin-fact tripwire by name. Reverting makes both green again. The rollout story stays. What the DevTools sweep found on /garage/horizon, and why the enforcing half had never been applied, are gotcha 17 in CLAUDE.md, which is where it belongs now that the mechanism is gone. Rolling back is `git revert` rather than a flag, and `bun run csp:sweep` is the signal that would call for one; its header says so instead of describing a flip that already happened. 353 tests, typecheck, lint, build, pages, photos and the 142-route oracle green. The build's taste warnings on this branch are the ones #525 fixes, not new.
oddharsh
added a commit
that referenced
this pull request
Aug 24, 2026
…uarded ENFORCE_PAGE_HASHES was pinned true on 2026-08-16 and nothing has flipped it in either direction since, so its false arm has been dead code for a week. It was not one line: it cost a second header name, a second tail constant, a `tail` parameter threaded through both policy builders to feed that constant, an entries loop in withSecurityHeaders whose only job was carrying a second header past a bail, and two branches apiece in the contract test and the twin-fact tripwire. The emitted policy is byte-identical, checked both ways. CSP_TAIL now holds the string CSP_TAIL_REPORTABLE plus "; upgrade-insecure-requests" used to compose, and a build of each side produces the same _headers line. cspHeadersFor returns exactly ONE header now. The contract test counts the keys rather than probing one name, since a twin under any other spelling is the same regression, and the upgrade-insecure-requests assertion inverts with the change: it used to check the twin OMITTED the directive, and with one policy left the only way to get it wrong is to lose it, so it now checks presence on both arms. gen-md-twins no longer reads the flag. It reads the header set cspHeadersFor actually emits, which is the better check for the reason the flag era proved: a flag states an intention, and #249's bug was the enforcing half never being applied to a single document while the flag said it was. /security claims 'unsafe-inline' left script-src, and that claim goes false the moment a report-only twin returns, so the deploy fails until one of the two is put right. Controls, both run: reintroducing a report-only twin fails the contract test by name (it reports the two keys it found) and fails the twin-fact tripwire by name. Reverting makes both green again. The rollout story stays. What the DevTools sweep found on /garage/horizon, and why the enforcing half had never been applied, are gotcha 17 in CLAUDE.md, which is where it belongs now that the mechanism is gone. Rolling back is `git revert` rather than a flag, and `bun run csp:sweep` is the signal that would call for one; its header says so instead of describing a flip that already happened. 353 tests, typecheck, lint, build, pages, photos and the 142-route oracle green. The build's taste warnings on this branch are the ones #525 fixes, not new.
oddharsh
added a commit
that referenced
this pull request
Aug 24, 2026
…uarded (#527) ENFORCE_PAGE_HASHES was pinned true on 2026-08-16 and nothing has flipped it in either direction since, so its false arm has been dead code for a week. It was not one line: it cost a second header name, a second tail constant, a `tail` parameter threaded through both policy builders to feed that constant, an entries loop in withSecurityHeaders whose only job was carrying a second header past a bail, and two branches apiece in the contract test and the twin-fact tripwire. The emitted policy is byte-identical, checked both ways. CSP_TAIL now holds the string CSP_TAIL_REPORTABLE plus "; upgrade-insecure-requests" used to compose, and a build of each side produces the same _headers line. cspHeadersFor returns exactly ONE header now. The contract test counts the keys rather than probing one name, since a twin under any other spelling is the same regression, and the upgrade-insecure-requests assertion inverts with the change: it used to check the twin OMITTED the directive, and with one policy left the only way to get it wrong is to lose it, so it now checks presence on both arms. gen-md-twins no longer reads the flag. It reads the header set cspHeadersFor actually emits, which is the better check for the reason the flag era proved: a flag states an intention, and #249's bug was the enforcing half never being applied to a single document while the flag said it was. /security claims 'unsafe-inline' left script-src, and that claim goes false the moment a report-only twin returns, so the deploy fails until one of the two is put right. Controls, both run: reintroducing a report-only twin fails the contract test by name (it reports the two keys it found) and fails the twin-fact tripwire by name. Reverting makes both green again. The rollout story stays. What the DevTools sweep found on /garage/horizon, and why the enforcing half had never been applied, are gotcha 17 in CLAUDE.md, which is where it belongs now that the mechanism is gone. Rolling back is `git revert` rather than a flag, and `bun run csp:sweep` is the signal that would call for one; its header says so instead of describing a flip that already happened. 353 tests, typecheck, lint, build, pages, photos and the 142-route oracle green. The build's taste warnings on this branch are the ones #525 fixes, not new. Co-authored-by: Aadharsh Pannirselvam <19518661+oddharsh@users.noreply.github.com>
This was referenced Aug 24, 2026
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
1 participant
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.
A stale-code passthrough. What it found is that most of the stale LoC here is not dead, it is broken: code left pointing at directories that three renames moved out from under it. Whole-file deadness is nearly zero (one unused function in ~23k lines of Worker code, no commented-out blocks, no unused deps, no orphaned
/itiers, dictionaries at their KEEP=3 retention).The photo pipeline has been writing to directories that do not exist
Eight sites across six scripts find their output with
$SCRIPT_DIR/... That was the served tree when these lived inwww/scripts/; it has beentools/since #452 and #453.extract-photo-metadata.shOUT, META_DIR,--roottools/images/…public/images/…hash-thumbnails.shSRC_DIR, OUT_DIRtools/images,tools/ipublic/images,public/ihash-thumbnails.shfingerprints calltools/scripts/…tools/photos/…add-car-photo.shDESTtools/carspublic/carsgen-encoding-{grids,samples}.shDESTtools/garage/encpublic/garage/encadd-photos.shhistogram--root<repo>/www<repo>/publicEvery one was correct when written. A rename sweep greps for the old string, and there is no old string here, only a relative expression whose meaning is a function of where the file sits.
Two things kept it silent.
zenc histogramprintscannot read …/images/hashes.jsonand exits 0, and both callers pipe it totail -1, so the bake skips without failing. Andadd-photos.sh/reencode-thumbnails.shresolve through$PROJECT_DIRrather than$SCRIPT_DIR/.., so those two were updated by the split, which left the pipeline looking half-migrated and plausibly fine. No photo has been added since the move, so nobody ran into it.Measured:
--root <repo>/wwwanswerscannot read;--root <repo>/publicreads all 158 stems.The build has printed 14 warnings a run since 2026-08-18
The same rename left
build.mjs's taste exemption anchored on/^www\/(garage|lwe)\//, matching nothing, so the/garageand/lwedemo pages it exists to exempt lost the exemption. Every one of the 14 warnings was on a demo page. A warn block nobody can clear is how a warn block gets ignored.Re-anchored on the
SERVED_SOURCESroots. Controls both directions, since this change re-enables acontinue: an injected@font-face url()on/garage/tooltipsstill hard-fails the deploy, and a 99px radius on that same page is silent again. The two hard rules run before the exemption, so nothing lost enforcement.A guard whose one exemption hid the only offender
lens-seed.mjsstill spawnedexecFileSync("pnpm", ["exec", "wrangler", …]). pnpm is gone from this repo (no lockfile, no workspace file,packageManageris bun) and refuses outright on a bun tree, so every seed run has been dying.contract-the-typescript-quarantinebans exactly this. It carriedlens-seed.mjsas its single RECORDED exception, justified as "drives a package script as the subject of the browser recording rather than using a manager to reach another binary". That described no code in the file. The single genuine offender was the single exempted file, and the suite reported green.The spawn goes through
wranglerCommand()now and the allowlist is deleted. Control: re-adding apnpmspawn fails the test by name, which it could not do before.Dead or wrong, removed
explorerChrome(), exported and called by nothing anywhere including its own fileisCallabledoc comment orphaned fifteen lines above its functionbuild.mjstold you to runnode public/scripts/build-histogram-index.mjs, which is not a pathcheck-wrangler.mjstold you to runpnpm install. Its transitive-copy scan readsnode_modules/.pnpm, so it reports 0 forever on a bun tree; logic unchanged, blind spot now named at the codeMAINTENANCE.mddocumentedzenc histogram --root wwwand two.tsmodules as.jsCLAUDE.md counts, re-derived from the tree
36→37 documents, 46→49 test files, 117→147 relative specifiers, 474→632 hashed tiers (the 200px tier from #441 was never written down), 2645/48→2921/50 link refs.
Gotcha 16 named
contract-tests.mjsundernode --test. Both runners measured: the rule holds and both symptoms changed, so the documented error string finds nothing and the blast radius is one file rather than the whole run.node --testERR_UNSUPPORTED_ESM_URL_SCHEMEbun testCannot find package 'cloudflare:workers'Gotcha 39 records the
$SCRIPT_DIR/..class so the next rename has something to check against.Verification
353 tests, typecheck, lint, build (0 warnings),
pages:check,photos:check, and the 142-route oracle all green, re-run after rebasing onto thetools/libTypeScript migration (#522).Left alone, deliberately
tools/verify-baseline.aadhar.sh.json(480 lines) is written byverify-routes.mjsand read by nothing. Clearest stale file in the repo, but a diff on it after a prod run does show route drift, so deleting it may throw away a real if manual use. Delete, or wire it into an actual comparison?ENFORCE_PAGE_HASHEShas been stucktruesince 2026-08-16, leaving a dead report-only branch plusCSP_TAIL_REPORTABLEand dead arms ingen-md-twins.mjs. Roughly 50 lines, and also the one-line rollback if a page breaks under enforcement.zenc histogram --checkreports drift on 158/158 photos against the corrected root, whilephotos:checkpasses, so the committed artifacts are self-consistent. Reads like a zenjpeg decoder bump rather than data loss. Deliberately not re-baked, since that would rewrite every photo's histograms from a possibly-different decoder. Worth its own look.