release: 17.0.0 — a bound whose own NaN made its guard read false, in 129 places - #378
Merged
Conversation
… 129 places One sweep, five slices, in tier order: #364 (tiers 0–1), #370 (2–3), #374 (4), #375 (5), #377 (the blind spots). `bun run finite-bounds` goes from 129 sites to 4, and both survivors are AUDITED pins carrying the sentence saying why screening them would be worse, not unexamined debt. THE DEFECT CLASS. `??` guards nullish and `NaN` is not nullish, so `Number(process.env.X)` on an unset variable, a parseInt of a typo and an untyped config value all walk past the default and land on the bound intact. `Math.max`, `Math.min` and `Math.floor` are not validators either — all three PROPAGATE NaN, and this repo was relying on all three as guards. What that produced, each measured rather than reasoned about: a NaN token estimate did not bypass the AI budget, it POISONED it — after one such call a 5,000,000-token request passed a 1,000-token ceiling; `awaitActionable({ timeoutMs: NaN })` ran 835,462 polls in 3 seconds against a real browser, past ctx.signal, past the watchdog, past the job timeout; `syncAuthenticator({ ttlMs: NaN })` held a revoked session across a full year of clock advance; `randomToken(NaN)` returned "" — the framework's secret generator, producing no secret and reporting success; `generateRecoveryCodes(Infinity)` wedged the process on the enrolment path, and `NaN` enrolled a user with zero recovery codes; `configureLifecycle({ deadlineMs: NaN })` made a deploy drop in-flight requests and abandon close hooks on the first tick; an ISR page with a non-finite TTL was never fresh, so it regenerated on EVERY request; `chunk({ size })` and `embedBatched` were synchronous infinite loops. THREE BREAKING ENTRIES, all the same shape: a numeric option that used to accept NaN refuses it, at boot or at the call boundary rather than mid-request. An app passing real numbers is unaffected. An app passing NaN was not working — the bound it declared was not being enforced, and nothing said so. `0` stays legal everywhere it means something: port 0 asks the OS for a free port, timeout 0 is one look, seed 0 is a seed, maxAgeSeconds 0 is "revalidate every time". THE RATCHET SHIPPED WITH THE SWEEP AND WAS WIDENED THREE TIMES BY DEFECTS THAT WALKED PAST IT — an optional chain on the object, a default read out of a table of numbers, and bare parameter defaults. All three were found the same way: by TESTING the guard rather than reading it. One clause added along the way measured inert and was deleted rather than left reading as a rule holding a line it was not holding. Its non-vacuity guard also broke because the tree got better: `total > 10` was calibrated at 129 sites, so it failed as the count approached zero. It now asserts that every PINNED package still reports exactly its pinned count. Docs made true rather than restated: CLAUDE.md said `@ultimat3/notify` had never been published and owed a hand publish before the next release run. True when written; the 16.0.0 run published it, and the audit answers 31/31 attested — so following that paragraph would have produced an E403. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
On-demand reviews are free for the next 25 days. After that, they cost $0.25 per reviewed file. Or wait 2 minutes for your next included review. View limit detailsLimit details: You’ve used the included review currently available. Your 76 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (48)
Comment |
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.
One sweep, five slices, in tier order: #364 (tiers 0–1), #370 (2–3), #374 (4), #375 (5), #377 (the blind spots). This is the release commit.
bun run finite-boundsgoes from 129 sites to 4, and both survivors are audited pins carrying the sentence saying why screening them would be worse —@ultimat3/ui'sTextarea rowsand@ultimat3/admin'slogo.width, each with the one-line edit that overturns the decision. Not unexamined debt.The defect class
??guards nullish, andNaNis not nullish. SoNumber(process.env.X)on an unset variable, aparseIntof a typo and an untyped config value all walk past the default and land on the bound intact.Math.max,Math.minandMath.floorare not validators either — all three propagateNaN, and this repo was relying on all three as guards.What it produced — each measured, not reasoned about
NaNtoken estimateawaitActionable({ timeoutMs: NaN })ctx.signal, past the watchdog, past the job timeoutsyncAuthenticator({ ttlMs: NaN })sweepGrantsanswered{revoked: 0}and the book still held the socketrandomToken(NaN)""— the framework's secret generator, producing no secret and reporting successgenerateRecoveryCodes(Infinity)NaNenrolled a user with zero recovery codesconfigureLifecycle({ deadlineMs: NaN })chunk({ size }),embedBatchedAbortSignalThree breaking entries, all the same shape
A numeric option that used to accept
NaNrefuses it — at boot or at the call boundary, never mid-request, so onebun testor onex verifysurfaces every one at once and eachfix:carries the edit.An app passing real numbers is unaffected. An app passing
NaNwas not working: the bound it declared was not being enforced, and nothing said so.0stays legal everywhere it means something —port: 0asks the OS for a free port,timeout: 0is one look,seed: 0is a seed,maxAgeSeconds: 0is "revalidate every time",concurrency: 0is one worker. Every floor is pinned by a mutation in both directions, and four floors were chosen by intuition and corrected by a grep —kdf.maxConcurrent: 0is a deliberate zero-width gate,retentionPlan(deploys, 0)andSkeleton lines: 0are shipped tests.wiki/Upgrading.mdgains its16.x → 17.0.0walkthrough, one row per entry.The ratchet shipped with the sweep, and was widened three times by defects that walked past it
An optional chain on the object; a default read out of a table of numbers; and bare parameter defaults. All three found the same way — by testing the guard rather than reading it, never by reading it.
Two more corrections in the same spirit:
total > 10, calibrated at 129 sites, so it failed as the count approached zero. A count is the wrong instrument once the count is meant to reach zero. It now asserts every pinned package still reports exactly its pinned count — those sites are deliberately unrepaired, so they are the one thing a working scanner is guaranteed to find. Blinding the scanner turns 9 tests red.And one rule I proposed was measured and declined: a second matcher for the
!== undefinedassignment form would find ~1 site per two packages (24 occurrences acrosscore+auth, exactly one numeric).Docs made true, not restated
CLAUDE.mdsaid@ultimat3/notify"has never been published" and owed step 1 ofPUBLISHING.mdbefore the next release run. That was true when written, and the 16.0.0 run published it —bun run scripts/registry-audit.ts --jsonanswers31/31 publishable packages are on npm at 16.0.0, every one attested. Following that paragraph would have producedE403 … cannot publish over the previously published versions. Corrected to point at the audit instead, which is what the whole table on that page exists for.Verification
bun run verify→ 14 of 20 passed, 6 skipped.bun run scripts/release.ts --check 17.0.0→ 31 packages stamped at 17.0.0.bun run scripts/reference-app-gate.ts→ every pin holds.bun install --frozen-lockfile→ no changes.The first release-gate run was red on 2 steps, both post-bump bookkeeping with executable fixes: 232 stale workspace ranges in
bun.lockand av16.0.0stamp inwiki/_Footer.md. Notebun installalone would not have fixed the lockfile — Bun refreshes a workspace block only when that workspace's own manifest changed, and--frozen-lockfileaccepts every stale range, which is why this repo has a dedicatedlockfile:fix.After merge
Annotated tag (
git tag -a— a lightweight tag is not a release trigger and--follow-tagswill not push one), then the GitHub Release, which is what firesrelease.yml. The workflow then reacheswaitingon thenpm-publishenvironment gate — that reads aswaiting, not a failure, and is the last point at which an irreversible publish can be stopped.Filed during the sweep, not fixed by it
#376 — a third
toMs, inpackages/jobs/src/clock.ts:18, with no screen at all and a comment reading "Numbers pass through so callers may stay explicit". It is the onestep.sleep's wake time, the retry curve'sbase/maxand every job timeout go through, sostep.sleep(Number(process.env.X))is still a sleep that never ends.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.