fix(tier 5): 835,462 polls in three seconds, and a session that never expired - #375
Merged
Conversation
… expired
Tier 5 of the 17.0.0 sweep, last of the code slices. The campaign closes at
4 sites from 129, and both survivors are AUDITED pins with written reasons
rather than unexamined debt.
`awaitActionable({ timeoutMs: NaN })` was a busy-loop against a real browser.
Measured with the screens removed: 835,462 polls in 3 seconds — 278,487/s —
still looping, each one a CDP round trip, past ctx.signal, past the wedge
watchdog and past the job timeout. `for (;;)` with a NaN budget has no exit.
`syncAuthenticator({ ttlMs: NaN })` reopened the hole that file exists to
close. `expiresAt = now + NaN` is NaN and `expired()` asks `expiresAt <= now`,
false forever. Measured: session revoked, clock advanced a FULL YEAR,
`sweepGrants` answered `{refreshed: 0, revoked: 0, failed: 0}`, and the book
still held the socket.
A saved scrape session whose `savedAt` does not parse was restored at any age —
the stored value is only checked to BE a string. The comparison is fail-closed
now: `!(age <= limit)` is identical for every finite age and opposite for NaN.
`installDeterminism({ now: 'yesterday' })` broke `Date.now()` for the WHOLE
test process, because `bun test` is one process and the preload installs the
clock globally. And a NaN seed did not break determinism — measured,
`seed >>> 0` maps NaN, ±Infinity, 0.5, -1 and 2**32 all to the seed-zero
sequence, so the run reproduced fine and the RECORD of which seed produced it
was false, in the package whose whole promise is reproducibility.
TWO SCREENS GOVERNED ONE SETTING WITH TWO CEILINGS. `dev-roles.ts` judged the
env string and capped hops at 16; `@ultimat3/http` judges the config number and
caps at 64. Both screens stay — an operator who set an env var must not get a
`fix:` naming a code edit they cannot make — but the NUMBER was the axiom-1
violation. The CLI moved, because widening breaks no shipped configuration
while tightening http would narrow a public API in an already-released tier.
A test probes both screens for the highest value each accepts and fails naming
both numbers the instant they diverge: behaviour compared, not constants.
THE RATCHET'S OWN NON-VACUITY GUARD BROKE BECAUSE THE TREE GOT BETTER. It
asserted `total > 10`, a threshold calibrated at 129 sites, and the sweep walked
through it. A count is the wrong instrument once the count is meant to approach
zero — lowering it repeatedly ends at `> 0`, which proves nothing. It now
asserts that 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.
No new error code: every refusal is X_INVARIANT from core's finiteOption /
finiteCount, so no wiki row and no manifest change.
Found and NOT fixed here, added to #371: `configureLifecycle({ deadlineMs })`
has no screen, so a deploy drops in-flight requests and abandons shutdown hooks
on the first tick — reproduced, `drain()` returned in 111ms with work still in
flight. It is an `!== undefined` assignment, not a `??` default, so the ratchet
cannot see it, which means packages/core's ABSENCE from the pin table currently
claims more than it has proved.
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 47 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 selected for processing (43)
Comment |
sebyx07
added a commit
that referenced
this pull request
Aug 26, 2026
… 129 places (#378) 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>
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.
Tier 5 of the 17.0.0 sweep — the last code slice. Tiers 0–1 in #364, 2–3 in #370, 4 in #374.
The campaign closes at 4 sites, from 129. Both survivors are audited pins with written reasons, not unexamined debt —
@ultimat3/ui'sTextarea rowsand@ultimat3/admin'slogo.width, each carrying the sentence saying why screening it would be worse and the one-line edit that overturns the decision.The two that would page someone
A
NaNscrape timeout is a busy-loop against a real browser. Measured with the screens removed:Each one a CDP round trip. Past
ctx.signal, past the wedge watchdog, past the job timeout.for (;;)with aNaNbudget has no exit.syncAuthenticator({ ttlMs: NaN })reopened the exact hole that file exists to close.expiresAt = now + NaNisNaN, andexpired()asksexpiresAt <= now— false forever. Measured: session revoked, clock advanced a full year,sweepGrantsanswered{refreshed: 0, revoked: 0, failed: 0}, and the book still held the socket.Four more worth naming
savedAtdoes not parseageisNaNandage > maxAgeis false. Now written fail-closed:!(age <= limit), identical for every finite age and opposite forNaNinstallDeterminism({ now: 'yesterday' })Date.now()for the whole test process —bun testis one process and the preload installs the clock globally, so every test file sawNaN/Invalid Date, and no lateradvanceClockcould undo itNaNseedseed >>> 0mapsNaN,±Infinity,0.5,-1and2**32all to the seed-zero sequence — so the run reproduced fine and the record of which seed produced it was false, in the package whose whole promise is reproducibilityx devtrace limitwhile (byTrace.size > NaN)never runs, so eviction stops existing and a dev session holds every span of every request foreverTwo screens, one setting, two ceilings
dev-roles.tsjudged the env string and capped proxy hops at 16;@ultimat3/httpjudges the config number and caps at 64.Both screens are legitimate and both stay — an operator who set an env var must not receive a
fix:naming a code edit they cannot make. Only the number was the axiom-1 violation. The CLI moved, to 64, because widening breaks no shipped configuration while tightening@ultimat3/httpwould narrow a public API in an already-released tier.The constant is module-private in
http, so the permanent repair (export it, import it downward) was out of the worker's file set. Instead the duplication is enforced rather than documented: a test probes both screens for the highest value each accepts and fails naming both numbers the instant they diverge — comparing behaviour, not constants, and guarded against the vacuous pass where a broken probe makes both agree at 0. That guard fired immediately in development, which is how we know it works.The ratchet's own non-vacuity guard broke because the tree got better
It asserted
total > 10— a threshold calibrated when there were 129 sites — and the sweep walked straight through it. A count is the wrong instrument once the count is meant to approach zero: it fails when the tree improves, and the "fix" is to keep lowering a magic number until it reaches> 0and proves nothing.It now asserts that every pinned package still reports exactly its pinned count. Those sites are audited and deliberately unrepaired, so they are the one thing a working scanner is guaranteed to find, and a broken one reports zero for them. Mutation-checked: blinding the scanner turns 9 tests red.
Premises of the brief that turned out false
scrape-run.ts:72 ratewas already screened one file away (scrape.ts:137, pinned by a shipped test). Not the open defect the site list implied — that is the ratchet's own blind spot 3.determinism.ts:85 seedwas not the interesting line;:84was.new RealDate(options.now ?? DEFAULT_NOW)has a string default, so it is invisible to the scanner — and it is the one that broke the whole process.maxBytessites were not unguarded — they were guarded too late and in the wrong voice. Core'sreadWithinLimitdoes assert, but inrobots-fetch.tsthat throw lands insidecatch { return undefined }, which the gate reads as no robots restrictions; and inhttp.tsit arrived after the request had already been sent, with afix:naming a framework internal instead of themaxBytesthe caller wrote.Verification
bun run verify→ 14 of 20 passed, 6 skipped — green on the first run this time.33 mutations, 33 red, including every floor flipped both ways. Two are worth noting: removing the
watchdog.idleMsscreen makes the suite hang (killed at 45 s —idleMs: Infinitynever fires, which in production is an incident with nothing armed), andport: 0/minBytes: 0are shipped values that a floor of 1 would have refused, each caught by an existing test.Both workers typechecked without touching shared build state (
noEmit,composite: false, tsbuildinfo redirected), which is why this slice did not repeat #374's red typecheck run.Found here, fixed in #371, not in this PR
configureLifecycle({ deadlineMs })has no screen. A deploy drops in-flight requests and abandons shutdown hooks on the first tick — reproduced:drain()returned in 111 ms with work still in flight and the close hook never finished. It emitsX_SHUTDOWN_TIMEOUT, so it is not silent — but the cause renders asNaNmsand thefix:says raise the budget, which cannot help, because the value is not a budget that is too small.It is an
!== undefinedassignment rather than a??default, so the ratchet cannot see it — blind spot 1 in that file's own header, sitting inpackages/core, a package whose absence from the pin table is that file's claim that its slice closed everything. So that claim is currently overstated, and #371 owns fixing both the defect and the claim.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.