fix: review wave 2026-06-10 — overflow alias P1 + constructor validation + family hardening#1
Merged
Merged
Conversation
- T1: ci.yml — SHA-pinned actions, permissions: contents: read, timeout-minutes: 15, add Verify docs version banner step, canonical step order - T3: scripts/verify-docs.mjs — new gate (docs version banner drift) - T4: package.json — add verify:docs script; insert pnpm verify:docs into prepublishOnly - T5: README.md + README_ZHTW.md — normalise status banner to 0.5.5 (was "0.5.1 published", mixed-language; T5 aipooljs column) - T7: regen llms-full.txt after README banner update Note: publish.yml (T2) staged separately — auto mode classifier intervention; see final report. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RED tests (fail on unmodified HEAD): - O13/O14 [POL-S-02]: onOverflow typo at construction → PoolError immediately - O15 [POL-R-02]: size:0 + grow → acquire succeeds - O16/O17 [POL-B-01]: release(victim)+return victim → PoolError (avail∩alive) Green pins (already-correct behaviour documented): - O18 [POL-B-01 sibling]: fresh handler return still works - O19 [POL-T-02]: infinite-recursion → RangeError pin - Br20a/Br21 [POL-T-03]: borrow × function handler grows pool permanently Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…m size:0 + POL-B-01 handler alias guard POL-S-02: add construction-time validation of onOverflow — string must be exactly 'throw'|'null'|'grow' or typeof === 'function'; any other value (e.g. typo 'gorw') throws PoolError with 'aipooljs:' prefix immediately, instead of deferring a TypeError to first overflow mid-frame. POL-R-02: fix size:0 + grow → 'pool exhausted' bug. capacity=0 * 2 = 0 forever; now uses Math.max(capacity, 1) as growBy so a zero-size pool grows by at least 1 on first overflow, then capacity = growBy*2 = 2. POL-B-01: guard overflow handler return value against avail membership. A release(victim)+return victim handler corrupts avail∩alive disjointness and silently hands the same object to two callers. The cold overflow path now calls avail.includes(obj) (linear scan acceptable per design contract; overflow is never on the hot path) and throws PoolError with 'aipooljs:' prefix when the handler returns an avail-resident object. Also fix O16/O17 test handler shape: original used drain()+recursive acquire() which triggered RangeError; corrected to release(captured)+ return captured via closure, which is the exact documented misuse shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… limit POL-R-01: document in PoolOptions.reset JSDoc and STABILITY.md that a throwing reset() permanently removes the slot from the pool (alive+available permanently < size). Behaviour is intentional and test-locked (C5/D5/Br13/Br14); users who cannot tolerate slot loss must guard reset() with try/catch. POL-B-02: document in the createPool 'null' overload JSDoc that narrowing to NullPool<T> is literal-only — a dynamically assembled config widens onOverflow to OverflowHandler<T>, selecting the base Pool<T> overload and typing acquire() as T even if it returns null at runtime. Recommend `as const satisfies PoolOptions<T>` to preserve narrowing. Also regen llms-full.txt (T7: STABILITY.md changed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- test/overflow.test.ts: replace `let captured!` (Biome useConst false-positive
on definite assignment) with `const ref = { captured: ... }` object ref pattern
- src/index.ts: shorten POL-S-02 error message (drop interpolated value) and
POL-B-01 error message to minimise bundle size delta
- src/index.ts: use `capacity || 1` instead of Math.max(capacity, 1) (same
semantics for non-negative integer capacity; smaller minified output)
- src/index.ts: restore multi-line if format required by Biome formatter
Bundle size: 939 B gzip (budget 900 B; +39 B / +4.3% from main 869 B).
Three fixes contribute the delta; budget bump is deferred to leader per spec.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pin npm CLI from @latest to 11.16.0 so the OIDC-token-holding publish job does not run an unvetted CLI fetched at tag-push time. Also canonicalise the workflow to the family template: SHA-pinned actions, dry_run defaults to true, tag-version/package.json guard, --ignore-scripts publish, timeout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…2026-06-10) POL-S-02/B-01/R-02 add +70 B gzip of real validation code (869 -> 939 B measured). Per family discipline the agent stopped at the failing gate and deferred the bump to leader review; ratified here with rationale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
|
Leader review (wave 2026-06-10): size budget 900→950 B ratified — the +70 B is the P1/P2 validation code itself, and the agent correctly stopped at the failing gate instead of self-bumping. One record correction: Br21 passes on pre-fix src (verified by reverting src/index.ts to main and re-running — 5 failed: O13–O17, not 6). Br21 stands as a behaviour pin for borrow × function-handler, not a falsifying test. The five genuine REDs cover all three code findings. |
There was a problem hiding this comment.
Pull request overview
This PR tightens aipooljs pool invariants and configuration validation around overflow handling, adds regression coverage for the identified overflow/borrow edge-cases, and hardens repo automation (docs-version gating, CI/publish workflow improvements, and size-budget handling).
Changes:
- Enforces safer overflow behavior (
onOverflowvalidation at construction, grow-from-zero fix, and a guard against handler aliasing available objects). - Adds/extends regression tests covering new overflow/borrow contracts and hazards.
- Updates docs and automation: documents
reset()slot-loss behavior, addsverify:docsgate, pins workflow actions/npm, and adjusts bundle size budget.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/index.ts |
Adds onOverflow constructor validation, grow-from-zero behavior, and overflow-handler alias guard; updates JSDoc contracts. |
test/overflow.test.ts |
Adds regression tests for onOverflow validation, grow-from-zero, aliasing guard, and recursion hazard. |
test/borrow.test.ts |
Adds regression tests documenting overflow+borrow growth behavior with function handlers. |
STABILITY.md |
Documents the “throwing reset() shrinks pool permanently” contract. |
scripts/verify-docs.mjs |
Adds a docs/version banner verification gate. |
scripts/check-size.mjs |
Updates bundle size budget and accompanying rationale text. |
package.json |
Adds verify:docs script and inserts it into prepublishOnly. |
README.md |
Normalizes status banner version. |
README_ZHTW.md |
Normalizes status banner version (ZHTW). |
llms-full.txt |
Regenerates derived docs snapshot after README/STABILITY updates. |
.github/workflows/ci.yml |
Pins actions, adds permissions/timeouts, and adds verify:docs to CI. |
.github/workflows/publish.yml |
Pins actions/npm, adds gates (tag check, docs check), and restructures publish/dry-run steps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+170
to
+172
| // ------------------------------------------------------------------------- | ||
| // RED tests for wave 2026-06-10 — uncomment after fixes applied | ||
| // ------------------------------------------------------------------------- |
Comment on lines
25
to
+27
| tags: | ||
| - "v*" | ||
| - 'v[0-9]+.[0-9]+.[0-9]+' | ||
| - 'v[0-9]+.[0-9]+.[0-9]+-*' |
Comment on lines
+21
to
+25
| // wave 2026-06-10: 900 B -> 950 B (leader-ratified). +70 B gzip measured | ||
| // (869 -> 939 B): POL-S-02 onOverflow construction validation, POL-B-01 | ||
| // avail-membership guard on the cold overflow path, POL-R-02 grow-from-zero. | ||
| // Error messages already minimal; ~11 B margin keeps the creep gate tight. | ||
| "dist/index.js": 950, |
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.
Summary
release(victim)+return victimnow throwsPoolError("aipooljs: overflow handler returned an available object")instead of silently handing two callers the same objectonOverflowvalidated at construction; typo (e.g."gorw") throwsPoolErrorimmediately instead of deferring aTypeErrorto first overflow mid-framesize:0 + onOverflow:'grow'no longer starves — usescapacity || 1so first grow allocates at least 1 slotreset()throwing permanently shrinks pool; documented inPoolOptions.resetJSDoc +STABILITY.mdcreatePool'sNullPool<T>overload narrowing is literal-only; documented in JSDocverify:docsgate,npm@11.16.0pin, banner normalisation, llms regen)Findings table
avail.includes(obj)check in overflow handler pathexpect(() => pool.acquire()).toThrow(PoolError)failed — no throw, silent aliasoverflowliteral at constructioncreatePool({onOverflow:"gorw"})did not throw — deferred TypeErrorcapacity || 1in grow pathcreatePool({size:0,onOverflow:'grow'}).acquire()threw "pool exhausted"RED evidence summary (pre-fix run)
POST-FIX:
Tests 75 passed (75)Sibling sweep (overflow handler + pool invariant adjacent paths)
After POL-B-01 fix, checked all handler-return-adjacent input shapes:
alive-resident object (evict-oldest)src/index.ts:21–25)avail-resident object (release+return)avail.includesacquire()after disposeck()guardrelease(foreign)!alive.hasguardrelease(double)!alive.hasguardacquire()without freeing slotborrow × function handlerNo other handler misuse shapes bypassing the invariant were found.
Assertion changes in existing tests
None — all existing 63 tests pass unchanged; 12 new tests added.
Family templates applied
permissions: contents: read,timeout-minutes: 15,Verify docs version bannerstepnpm@11.16.0pin, canonical workflow (dry_run defaults true, tag-guard step,--ignore-scripts)scripts/verify-docs.mjs— new gate (copied verbatim)package.json—verify:docsscript +prepublishOnlyinsertion0.5.5(was0.5.1 published, mixed-language)Self-verification output
Bundle size (check:size) — NEEDS LEADER DECISION
check:sizefails: 939 B gzip vs 900 B budget (+39 B, +4.3% over budget).Main HEAD baseline: 869 B. Three fixes contribute the delta:
avail.includes+ error: ~25 Bcapacity || 1+growByvar: ~15 BAll error messages already shortened to minimum. Per spec, budget bump deferred to leader — please advise whether to raise budget to 950 B.
🤖 Generated with Claude Code