Add reproduction for MutableList issue - #6847
Conversation
🦋 Changeset detectedLatest commit: d959a71 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — added two reproduction-only tests for MutableList bugs:
prependon empty list doesn't settail— subsequentappendoverwrites the head bucket, losing the prepended elementtoArrayNmissing negative-bound guard —Math.min(-1, length)passes through tonew Array(-1), throwingRangeError(contrast withtakeN, which already hasif (n <= 0) return [])
Both tests are well-structured, use the correct assertion utilities, follow existing test patterns, and were verified to fail against current code.
DeepSeek Pro (free via Pullfrog for OSS) (Kimi K2 not used — the program covers this model; add its provider key to run your pick) ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No critical issues — one scope observation below.
Reviewed changes — added fixes for the two MutableList bugs reproduced in the prior commit:
prependnow initializestailon empty list — a new guardif (!self.tail) self.tail = self.headensures the prepended node serves as both head and tail, so subsequentappendcalls extend rather than overwrite.toArrayNguards non-positive bounds — the newif (n <= 0) return []guard matches the existing pattern intakeNandtakeNVoid, preventingnew Array(-1)RangeError.
ℹ️ prependAllUnsafe has the same tail-unset gap
The prependAllUnsafe function at MutableList.ts:286 (called by prependAll) shares the same structure as the now-fixed prepend: it sets self.head to a new bucket on an empty list but does not set self.tail. A prependAll on an empty list followed by append will therefore still overwrite the prepended bucket.
Technical details
# `prependAllUnsafe` tail initialization
## Affected sites
- `packages/effect/src/MutableList.ts:286-294` — `prependAllUnsafe` creates a new head bucket without setting `self.tail`, identical to the `prepend` bug before fix
## Required outcome
- Add `if (!self.tail) self.tail = self.head` after `self.head = { ... }` in `prependAllUnsafe`, matching the `prepend` fix
## Open questions for the human
- Widen the changeset description to mention `prependAll` as well, or keep the scope narrow and address in a follow-up?DeepSeek Pro (free via Pullfrog for OSS) (Kimi K2 not used — the program covers this model; add its provider key to run your pick) ｜ 𝕏
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
2 participants
Reproduction only
This PR adds reproduction tests only. No implementation fix is included. CI is expected to fail until the underlying issue is fixed.
Covered audit issues
1.
core-g-r-mutablelist-empty-prepend-tail-invariant: Prepending to an empty list leaves the tail unsetModule:
MutableListExpected contract: Prepending to an empty list creates a sole node that is both head and tail; later append preserves that node and list traversal agrees with length.
Observed result: The intended assertion failed; an independent probe produced values [2, null] with length 2.
Reproduction command:
2.
core-g-r-mutablelist-negative-toarrayn-bound: toArrayN throws for a negative boundModule:
MutableListExpected contract: toArrayN returns up to n values as a bounded prefix; for a nonpositive bound it returns an empty snapshot, consistent with sibling takeN.
Observed result: The intended failure was reproduced with RangeError: Invalid array length.
Reproduction command: