fix(core,auth,http,time): randomToken(NaN) was the empty string, and a deploy abandoned its own shutdown - #377
Merged
Merged
Conversation
…a deploy abandoned its own shutdown
The defects the finite-bounds ratchet cannot see. It matches `a.b ?? <number>`
and nothing else; these are bare PARAMETER defaults and a `!== undefined`
assignment, which is why five merged slices walked past them.
`randomToken(NaN)` returned `""` — the framework's secret generator, producing
no secret and reporting success. `nanoid` and `randomHex` did the same, and
`randomHex` is what traceId(), spanId() and uuid() mint from. A negative length
threw a bare, uncoded RangeError.
`generateRecoveryCodes(Infinity)` WEDGED THE PROCESS, synchronously, on the
enrolment path, from a public export: `for (index = 0; index < Infinity; …)`
has no exit. Measured, timeout 15 → exit 124, and the mutation proof for that
case is that the test run never terminates. `NaN` enrolled a user with zero
recovery codes in a well-formed RecoveryCodeSet; `2.5` silently gave three.
The second hang in @ultimat3/auth after `mfa.drift`.
`configureLifecycle({ deadlineMs: NaN })` made a deploy abandon its own
shutdown — in-flight requests dropped and close hooks ABANDONED on the first
tick. Measured: drain() returned in 16ms with inflight still 1, and Bun printed
TimeoutNaNWarning. `packages/cli/src/hold.ts` carried the identical defect and
is fixed transitively, so a reader of that file will see no diff.
THE X_SHUTDOWN_TIMEOUT MESSAGE IS DELIBERATELY UNCHANGED. It said `after NaNms`
with a fix: telling the operator to raise the budget, which cannot help when the
value is not a budget. With the bound screened that state is unreachable, so a
branch for it would be a test that cannot fail — in the one repo whose gate has
a ratchet by that name. The screen's own refusal is the message, and it arrives
at boot rather than at the deploy that drops the requests.
A route's `cache` hint is now refused where it is DECLARED, at createRouter,
naming the route file and the key — the layered form: refuse where the value is
written, be total where it is used. `maxAgeSeconds: 0` stays legal; it is
"revalidate every time" and three framework defaults declare it.
@ultimat3/time's `toMs` passed a non-finite number through, so time and notify
gave opposite answers to one input after the tier-4 slice screened the copy and
not the original. It gets finiteOption and NOT finiteCount: `toSeconds(-3000)
=== -3` is shipped, tested behaviour, so negatives and fractions are unchanged.
TWO OF MY OWN PREMISES WERE WRONG, and the code disproved both. `notify`'s
`toDurationMs` is NOT a duplicate — the tier-4 slice made it a screened
narrowing, and collapsing it would have LOST the option name that slice added
on purpose, so the public export stays and there is no breaking deletion. And a
second `finite-bounds` matcher for the `!== undefined` form is not worth
building: measured, 24 occurrences across core+auth and exactly ONE is numeric.
MAX_PROXY_HOPS is exported from @ultimat3/http and the CLI imports it instead of
restating 64. The behavioural test comparing the two ceilings is KEPT, not
deleted: it compares what each screen accepts, so it still catches them
diverging for a reason a shared constant cannot fix.
`release-facts` now reads a stat strip. The deployed demo rendered "29 packages
published in lockstep" against a tree that publishes 31, and widening the globs
to JSON would NOT have caught it — the count and the words it counts are on two
different lines under different JSON keys, and every pattern here matches a
number ADJACENT to its subject. A {value,label} pair is folded onto one line,
the label line blanked so line numbers stay true. The demo's other two
self-contradictions go with it: an empty-state string claiming a signed-out
reader sees nothing (the seed writes two public posts), and a post whose own
body said it must never appear while correctly appearing.
Fixes #369, #371, #372, #373.
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 14 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 (28)
Comment |
This was referenced Aug 26, 2026
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.
The defects the finite-bounds ratchet cannot see. It matches
a.b ?? <number>and nothing else; these are bare parameter defaults and one!== undefinedassignment, which is why five merged slices (#364, #370, #374, #375) walked past them while driving that rule from 129 sites to 4.All three were found the same way the ratchet's own two blind spots were found: by testing the guard rather than reading it.
The three that matter
randomToken(NaN)returned"". The framework's secret generator, producing no secret and reporting success.nanoidandrandomHexdid the same — andrandomHexis whattraceId(),spanId()anduuid()mint from. A negative length threw a bare, uncodedRangeError.generateRecoveryCodes(Infinity)wedged the process. Synchronously, on the enrolment path, from a public barrel export:for (index = 0; index < Infinity; …)has no exit. Measured —timeout 15→ exit 124 — and the mutation proof for that case is that the test run never terminates.NaNenrolled a user with zero recovery codes in a well-formedRecoveryCodeSet;2.5silently gave three. The second hang in@ultimat3/authaftermfa.drift.configureLifecycle({ deadlineMs: NaN })made a deploy abandon its own shutdown. In-flight requests dropped, close hooks ABANDONED on the first tick. Measured:drain()returned in 16 ms withinflightstill1, and Bun printedTimeoutNaNWarning.packages/cli/src/hold.tscarried the identical defect and is fixed transitively, so a reader of that file will see no diff.The
X_SHUTDOWN_TIMEOUTmessage is deliberately unchanged, and that is the answerIt said
1 in-flight operations still running after NaNms, with afix:telling the operator to raise the budget — advice that cannot help, because the value is not a budget that is too small.With
deadlineMsscreened,deadlineMshas exactly two writers anddeadlineAtis finite by construction, soNaNmsis unreachable. Writing a "the budget is not a number" branch would be a branch no test can reach — in the one repo whose gate has a ratchet named a test that cannot fail. The screen's own refusal is the message for that case, and it arrives at boot, at the declaration, not at the deploy that drops the requests.Two more
cachehint was only caught on the response path, once per request, forever. Now refused where it is declared, atcreateRouter, naming the route file andmeta.cache.<field>. The layered form the whole sweep uses: refuse where the value is written, be total where it is used.maxAgeSeconds: 0stays legal — it is "revalidate every time", three framework defaults declare it, and a floor of 1 refuses the framework at its own boot.@ultimat3/time'stoMspassed a non-finite number straight through, sotimeandnotifygave opposite answers to one input after the tier-4 slice screened the copy and not the original. It getsfiniteOption, notfiniteCount:toSeconds(-3000) === -3andtoSeconds('-1500ms') === -2are shipped, tested behaviour, so negatives and fractions are unchanged.Two of my own premises were wrong, and the code disproved both
notify'stoDurationMsis not a duplicate. notify,time: toDurationMs in notify is a second copy of time's toMs, and the two now DISAGREE — one screens a NaN, the other passes it through #372 called it byte-for-byte identical totoMs. The tier-4 slice had already turned it into a screened narrowing with a named subject, and that wrapper is exactly what makes it not a copy — routing it throughtoMswould fire a generic refusal first and lose the option name (deliver[email].digest.window) that slice added on purpose. So no public export is deleted and there is no breaking removal.finite-boundsmatcher for the!== undefinedform is not worth building. I suggested it in auth,core: a bare parameter default is the sweep's fourth blind spot — randomToken(NaN) is the empty string, and generateRecoveryCodes(NaN) enrols a user with zero codes #371. Measured instead: 24 occurrences acrosscore+auth, and exactly one is numeric. The productive scan was bare parameter defaults, which found five exported functions — the three named in the issue, plusrandomHex(fixed here) andstartMetricExport(already screened by an earlier slice).A third correction: one worker caught itself mid-draft writing
Number.isSafeInteger(v) && v >= 0into two files — the exact duplication this sweep exists to close — and collapsed it into one exported predicate before finishing.Also here
MAX_PROXY_HOPSis exported from@ultimat3/httpandpackages/cli/src/dev-roles.tsimports it instead of restating64. The behavioural test comparing the two ceilings is kept, not deleted: it compares what each screen accepts, so it still catches them diverging for a reason a shared constant cannot fix — one side gaining a range check the other lacks.release-factsreads a stat strip now, closing social-media-clone: the deployed demo contradicts itself on the feed, and release-facts cannot see the string that does it #369's fourth item. The deployed demo rendered29 packages published in lockstepagainst a tree that publishes 31 — and widening the globs to JSON would not have caught it. The count and the words it counts are on two different lines under different JSON keys, and every pattern in that file matches a number adjacent to its subject. A{value, label}pair is folded onto one line, the label line blanked so line numbers stay true and a finding cites the line a human edits. Mutation-proven by restoring the shipped29.Verification
bun run verify→ 14 of 20 passed, 6 skipped.bun run scripts/reference-app-gate.ts→ every pin holds;examples/dummy18/20,dummy/social-media-clone18/20, 2 pinned each.The app gate caught a real regression the framework gate cannot see. The
seed.tscopy fix lives underdummy/social-media-clone/packages/db/src/, and the drift hash covers both the entity registry and every non-test file in that directory — deliberately, so a seed moving under the schema is caught.x db genansweredno migration needed — schema hash re-recorded, which is the designed path; no empty migration was generated. It was a regression against a passing step, not a pinned one, and the ratchet held me to it.Every behavioural fix is mutation-proven, including every floor flipped in both directions.
configureLifecycle's floor is0and not1, checked rather than assumed:@ultimat3/httpscreensdrainTimeoutMswith a floor of 0 and hands it straight toconfigureLifecycle, so a floor of 1 would refuse at boot what http accepts in every process that serves web.No new error code, so no
wiki/Error-Codes.mdrow and no manifest change —X_INVARIANTandX_CONFIG_INVALIDare both already documented.Fixes #369, #371, #372, #373.
Filed, not fixed here
#376 — a THIRD
toMs, and the worst-placed one.packages/jobs/src/clock.ts:18, whose comment reads "Numbers pass through so callers may stay explicit", with no screen at all. It is the onestep.sleep's wake time, the retry curve'sbase/max, and every job timeout go through — sostep.sleep(Number(process.env.X))is still a sleep that never ends. Two of its callers wrap the result infiniteOption, which makes the file read as screened.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.