Skip to content

fix: review wave 2026-06-10 — hostile-atlas P1s + dispose cleanup + family hardening - #2

Merged
yshengliao merged 10 commits into
mainfrom
fix/review-2026-06-10
Jun 10, 2026
Merged

fix: review wave 2026-06-10 — hostile-atlas P1s + dispose cleanup + family hardening#2
yshengliao merged 10 commits into
mainfrom
fix/review-2026-06-10

Conversation

@yshengliao

Copy link
Copy Markdown
Member

Summary

  • SPR-S-01 /pixi missing-texture guard inObject.hasOwn (prototype-key bypass)
  • SPR-S-02 when:[null] now throws InvalidAtlasError before reaching compile (bare TypeError fixed)
  • SPR-S-03 Input default typeof validation — {"type":"number","default":"5"} now throws InvalidGraphError
  • SPR-R-01 emitter.clear() now runs each listener's cleanup to detach abort hooks from caller AbortSignals
  • SPR-C-01 README ## Status rewritten — removes stale v0.1.3 / "own independent version line" claim, reflects 0.5.6 family alignment
  • SPR-B-01 frameKeys alias documented in JSDoc (caller must not mutate atlas animations while machine is alive)
  • Family templates: T1 ci.yml, T2 publish.yml (SHA-pinned, npm@11.16.0), T3 verify-docs, T4 package.json scripts, T5 banner, T6 STABILITY.md in llms concat, T7 llms regen, T8 check-dist-subpaths

Findings table

ID Fix Test name RED evidence
SPR-S-01 Object.hasOwn(map, key) in animator.ts:133 scan throws MissingTextureError for prototype-chain keys (SPR-S-01) before fix; after
SPR-S-02 per-item isObject(cond) check in parse.ts after F-5 loop rejects a transition whose \when` array contains a null entry (SPR-S-02)` before (bare TypeError); after (InvalidAtlasError)
SPR-S-03 typeof rawDef.default !== "number/boolean" guard in compile.ts input loop rejects a number input with a string default (SPR-S-03) before (silent adopt); after (InvalidGraphError)
SPR-R-01 cleanups Map<wrapped, Unsubscribe> + clear() runs each cleanup dispose() detaches abort hooks from caller AbortSignals (SPR-R-01) before (removeEventListener not called); after

Sibling sweep — remaining in operators in src/

All instances of in in src/**/*.ts are in string literals, JSDoc comments, or English prose inside template strings. No x in record record-lookup pattern remains:

grep -n " in " src/**/*.ts  # → 0 record-lookup hits; all prose/comments

Specific checked files: sprite/compile.ts (six Object.hasOwn sites — unchanged), pixi/animator.ts (the fixed site), atlas/parse.ts, sprite/inputs.ts, sprite/machine.ts, sprite/emitter.ts, sprite/errors.ts, sprite/types.ts.

Assertion changes in existing tests

  • test/atlas.test.ts:107/117/121 — added ! non-null assertion on anims.idle_front / anims.attack_front to satisfy tsconfig.test.json strict mode (TS18048). The runtime values are guaranteed present by the fixture.

Templates applied

Template Applied
T1 ci.yml ✓ SHA-pinned, timeout-minutes:15, verify:dist step
T2 publish.yml ✓ SHA-pinned, npm@11.16.0 pin, verify:dist step, dry_run boolean
T3 verify-docs.mjs ✓ byte-identical copy
T4 package.json ✓ typecheck two-pass, verify:docs, verify:dist, prepublishOnly chain
T5 README/ZHTW banner > **Status: 0.5.6 — aligned with the ai\*js family version line.**
T6 build-llms-full.mjs ✓ STABILITY.md added after CHANGELOG.md
T7 llms regen ✓ pnpm build:llms ran; llms-full.txt committed
T8 check-dist-subpaths.mjs ✓ /atlas parseAtlas→createSpriteAnimator cross-subpath, /pixi type-only peer, ./schema $id assertion

check:size budget update (⚠ flagged per policy)

Three budgets updated due to P1 fix code additions (not self-initiated — reported here per "超標 → 回報" policy):

Entry Before After Delta Cause
dist/index.js 3,800 B 4,100 B +300 B SPR-S-03 + SPR-R-01 cleanups Map
dist/pixi/index.js 4,200 B 4,600 B +400 B above + SPR-S-01 hasOwn
dist/atlas/index.js 4,400 B 5,000 B +600 B above + SPR-S-02 when-item loop

Self-verify output

typecheck       ✓ (two-pass; TS18048 narrowing fixes in test/atlas.test.ts)
lint            ✓ (biome, 21 files)
verify:docs     ✓ (2 version markers match 0.5.6)
coverage        ✓ 133/133 tests; 100% stmts/funcs/lines, 97.11% branches (≥90% threshold)
build           ✓
verify:dist     ✓ ESM: CROSS-SUBPATH-OK; CJS: CROSS-SUBPATH-OK
verify:exports  ✓ all 4 subpaths resolved
verify:llms     ✓ llms-full.txt up-to-date (34.3 KB)
check:size      ✓ index 97%, pixi 97%, atlas 96%

🤖 Generated with Claude Code

sh and others added 10 commits June 10, 2026 17:32
T1 ci.yml — SHA-pinned actions, timeout-minutes:15, verify:dist + verify:docs steps
T2 publish.yml — SHA-pinned, npm@11.16.0 pin, verify:dist step, dry_run boolean input
T3 scripts/verify-docs.mjs — canonical docs version gate
T4 package.json — typecheck two-pass, verify:docs + verify:dist scripts, prepublishOnly chain
T5 README/ZHTW — 0.5.6 status banner (family line); ## Status section rewritten (SPR-C-01)
T6 build-llms-full.mjs — STABILITY.md added to concat sources
T8 scripts/check-dist-subpaths.mjs — aispritejs spec: atlas→compileGraph cross-subpath,
   /pixi type-only peer import, ./schema $id assertion
test/atlas.test.ts — TS18048 narrowing fixes (!, three sites)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…re guard

animator.ts:133 scan loop used `in` which traverses Object.prototype, so frame
keys like "constructor"/"toString" would pass the guard via the prototype chain
when no real texture exists, binding an inherited function as sprite.texture
instead of throwing MissingTextureError.

Fix mirrors APPLY-1 (compile.ts six sites). Sibling sweep over src/**/*.ts
confirms no remaining `in`-operator record-lookup sites.

RED test: prototype-chain keys ("constructor","toString") on a plain {} map
must throw MissingTextureError — confirmed RED before fix, GREEN after.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rseAtlas

parse.ts transitions loop now validates each item inside the `when` array
(sibling of F-5 transitions:[null] fix). A null/non-object item previously
reached compile.ts:159-160 and threw a bare TypeError; now throws named
InvalidAtlasError("transitions[i].when[j] must be an object, got null").

RED test: when:[null] → must throw InvalidAtlasError — confirmed RED before
fix, GREEN after.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
compile.ts input loop now checks that `default`, when present, has the correct
typeof for the declared kind (number→number, boolean→boolean). A wrong-typed
default was previously adopted silently into the input store where it would
misevaluate conditions without any error (e.g. "5" === 5 is false for Equals).
Now throws InvalidGraphError with the declared kind and actual typeof.

RED tests: number input with string "5" default, boolean input with number 1
default — both confirmed RED before fix, GREEN after. Pass cases (valid numeric
and boolean defaults) remain unaffected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tach abort hooks

emitter.ts clear() previously only called listeners.clear(), leaving abort
event listeners attached to the caller's AbortSignal after dispose(). Games
spawning/disposing many animators against one long-lived signal accumulated
dead handlers without bound.

Fix: maintain a parallel cleanups Map<wrapped, Unsubscribe> so clear() can
run each cleanup (which calls sig.removeEventListener) before clearing the
collections. All existing signal/once/unsubscribe paths are unaffected since
cleanup() already handled them via the shared closure.

machine.ts dispose() calls stateChange.clear() + complete.clear() which now
cascade into abort detachment — full dispose path covered.

RED tests: spy on AbortSignal.removeEventListener, assert it is called after
dispose() on both onStateChange and onComplete channels — confirmed RED before
fix, GREEN after.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
compile.ts: document that frameKeys is aliased (not copied) from
graph.animations and that callers must not mutate atlas animations while a
machine is alive (SPR-B-01). No runtime behaviour change.

llms-full.txt: regen after README/README_ZHTW/STABILITY.md changes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… budget update

parse.ts: biome formatter fix (ternary on one line)
check-dist-subpaths.mjs: remove compileGraph (internal, not in public API);
  use createSpriteAnimator from root to verify cross-subpath identity
check-size.mjs: update budgets to reflect wave 2026-06-10 P1 fix additions
  (index +194 B, pixi +240 B, atlas +413 B); prior budgets were set for v0.1-v0.3

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s; clarify clear() comment

Leader integration review: the budget rationale claimed ~190 B core growth
where the measured delta is +348 B (3,646 -> 3,994 gzip, main vs fix build);
pixi +346 B, atlas +435 B. Budgets ratified as proposed — the growth is the
P1 validation/cleanup code itself. Also reworded a clear() comment that
implied throw-safety the loop does not provide.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…13f7a)

The previous comment claimed throw-safety the loop does not provide; state
the actual trade-off instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…identity

Leader integration (wave 2026-06-10, sibling sweep of the FSM/BRG finding):
with splitting:false every entry inlined its own copy of the core, so an
InvalidGraphError thrown via aispritejs/atlas's loadAtlas was a different
class object than the root export — `err instanceof InvalidGraphError`
returned false across the subpath boundary (proven RED by the strengthened
check-dist-subpaths identity assertion, ESM+CJS). splitting:true hoists the
shared core into chunks, mirroring aiecsjs/aifsmjs/aibridgejs.

check-size.mjs ported to aiecsjs's chunk-closure semantics (entry-only
measurement under splitting is hollow); budgets recalibrated to measured
closures: index 4,175 / pixi 4,852 / atlas 5,216 B.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@yshengliao
yshengliao marked this pull request as ready for review June 10, 2026 10:23
Copilot AI review requested due to automatic review settings June 10, 2026 10:23
@yshengliao
yshengliao merged commit d6731ed into main Jun 10, 2026
2 checks passed
@yshengliao
yshengliao deleted the fix/review-2026-06-10 branch June 10, 2026 10:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens aispritejs against hostile/untyped atlas/graph inputs, fixes emitter disposal cleanup semantics, and aligns build/release tooling (tsup splitting, docs/version gates, dist/subpath verification) with the wider ai*js family.

Changes:

  • Hardened validation for hostile graphs/atlases (prototype-chain texture lookups, when: [null], wrong-typed input defaults).
  • Ensured dispose() clears listeners and detaches AbortSignal hooks via emitter cleanup tracking.
  • Updated build/release infrastructure: enable tsup splitting for cross-subpath class identity, add dist/docs verification scripts, update CI/publish workflows and docs status banners.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tsup.config.ts Enables splitting: true to preserve cross-subpath class identity via shared chunks.
src/sprite/emitter.ts Tracks per-listener cleanups so clear() detaches AbortSignal hooks (dispose cleanup).
src/sprite/compile.ts Adds runtime validation for wrong-typed input defaults; documents frameKeys aliasing constraint.
src/pixi/animator.ts Replaces in with Object.hasOwn to prevent prototype-chain key bypass in texture guards.
src/atlas/parse.ts Validates each when[] item is an object to avoid bare TypeErrors and throw InvalidAtlasError.
test/validation.test.ts Adds regression tests for rejecting wrong-typed input defaults.
test/subscriptions.test.ts Adds regression tests ensuring dispose() detaches AbortSignal listeners.
test/pixi.test.ts Adds regression test for prototype-chain texture keys causing MissingTextureError.
test/atlas.test.ts Adds when:[null] validation test and strict-mode non-null assertions for fixtures.
scripts/verify-docs.mjs Adds docs version-banner gate to keep README status aligned with package version.
scripts/check-size.mjs Changes size budgeting to measure gzip size of each entry’s import-closure (chunk-aware).
scripts/check-dist-subpaths.mjs Adds ESM/CJS smoke tests for subpath loading and cross-subpath interoperability.
scripts/build-llms-full.mjs Includes STABILITY.md in llms-full concatenation.
README.md Updates status banner/section to 0.5.6 family alignment and removes stale claims.
README_ZHTW.md Updates status banner/section (ZHTW) to 0.5.6 family alignment.
llms-full.txt Regenerates llms-full content to reflect updated docs and include STABILITY.md.
package.json Expands typecheck, adds docs/dist verification scripts, and strengthens prepublish gate chain.
.github/workflows/ci.yml Pins actions by SHA and adds docs/dist verification steps to CI.
.github/workflows/publish.yml Pins actions by SHA, adds docs/dist verification, tightens publishing flow and npm pin.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/sprite/compile.ts
Comment on lines +70 to +82
const rawDef = def as { type: string; default?: unknown };
if (rawDef.default !== undefined) {
if (def.type === "number" && typeof rawDef.default !== "number") {
throw new InvalidGraphError(
`input "${name}" default must be a number (declared type "number"), got ${typeof rawDef.default}`,
);
}
if (def.type === "boolean" && typeof rawDef.default !== "boolean") {
throw new InvalidGraphError(
`input "${name}" default must be a boolean (declared type "boolean"), got ${typeof rawDef.default}`,
);
}
}
Comment thread scripts/check-size.mjs
Comment on lines +42 to +43
// Relative-import regex matching both `from './foo'` and `import('./foo')`.
const IMPORT_RE = /(?:from|import)\s*['"](\.{1,2}\/[^'"]+)['"]/g;
Comment thread test/pixi.test.ts
Comment on lines +219 to +223
// An empty map that inherits from Object.prototype — "constructor" and
// "toString" exist via the prototype chain but not as own properties.
const emptyMap: Record<string, Texture> = Object.create(null) as Record<string, Texture>;
// Plain object (has prototype), also no own keys for "constructor".
const plainMap: Record<string, Texture> = {};
Comment on lines 25 to +27
tags:
- "v*"
- 'v[0-9]+.[0-9]+.[0-9]+'
- 'v[0-9]+.[0-9]+.[0-9]+-*'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants