test(signals): deflake the spawn retention test

[kixelated]

Fixes the intermittent spawn retention > an effect that never reruns drops settled tasks failure on the arm64 Test job (received ~8191 instead of < 100).

Root cause

Not a leak in Effect.spawn. Spawning 10000 tasks synchronously grows #async (a Set) through several rehashes. JSC's OrderedHashTable rehash marks the old storage obsolete but leaves its keys in place (copyImpl only overwrites the header with deleted-entry indices). Those obsolete tables are garbage, unless a stale word on the stack or in a register still points at one; JSC scans both conservatively. When that happens, the old tables keep ~8k settled promises alive.

Reproduced locally by pinning the set's storage with an iterator taken mid-loop: the delta is exactly 8191 for any pin point during the add phase, vs 3 without. Whether a stale pointer survives depends on JIT tier and register allocation, hence arm64-only and run-to-run variance.

Fix

Settle each task before spawning the next, which is also how the real per-group caller behaves. The set never grows past one entry, so there are no large obsolete tables to retain. The test still fails without the fix (removing the tasks.delete line gives 10003).

API / wire impact

None (test only).

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-25T12:35:17.914994Z	eb48f4b	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[kixelated]

MERGE

Positive improvement: this is a real CI flake, not a leak in Effect.spawn. Spawning 10k tasks into #async in one go forces several Set rehashes; JSC can keep an obsolete table alive via a stale conservative stack/register word, which shows up as ~8191 retained promises on arm64. Settling between spawns matches how a never-rerunning per-group caller actually uses spawn, keeps the set at one entry, and still fails hard if tasks.delete is removed (10003 retained). Test-only, no API or wire impact, complexity is a few lines and a useful comment.

Worth it. A bulk-spawn-plus-stronger-GC approach would stay engine-fragile; shrinking the count would only paper over the rehash window. This is the right shape.

This is an automated review, not the maintainer's decision
(Written by Grok)

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: c0de0304-f095-4e4a-a58a-05e9f61ca03b

📥 Commits

Reviewing files that changed from the base of the PR and between f822924 and eb48f4b.

📒 Files selected for processing (1)

• js/signals/src/index.test.ts

Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review.

---

Walkthrough

The spawn-retention test now spawns and settles each of 10,000 tasks before starting the next. The promise-count assertion remains unchanged.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to eb48f

The test continues to check for settled-promise retention while avoiding the flaky concurrent-spawn setup. No material merge risk is evident.

Architecture Summary

Architecture risk: 🔵 Low · up to eb48f

The change affects 1 system.

Changed systems: js

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — js (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in js/signals/src/index.test.ts: The spawn-retention test now settles each of 10,000 spawned tasks before proceeding, instead of spawning all tasks before a single settle.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the test change to prevent intermittent spawn-retention failures.
Description check	✅ Passed	The description explains the intermittent failure, its reported JSC retention cause, and the test-only fix. It is directly related to the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

⚔️ Resolve merge conflicts 💡

• Resolve merge conflict in branch claude/signals-spawn-retention-flake

✨ Simplify code

• Commit to this branch
• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[kixelated]

Closing this. #4128 is already on main and rewrote an effect that never reruns drops settled tasks to a bookkeeping assertion (tasks.size). This PR only changes the old heap-count version of that test, so replaying it would undo #4128. The heap flake cannot happen on the new assertion.

(Written by Grok 4.7)