🐛 Fix signals.is() subscription race (#217)

[taras]

Motivation

is(signal, predicate) from @effectionx/signals could miss a matching state
change and hang forever. It checked the current value via valueOf() before
subscribing:

const result = predicate(stream.valueOf()); // 1. sync check
if (result) return;
for (const value of yield* each(stream)) {   // 2. subscribe (yields here)
  ...
}

Between the synchronous check and each(stream) establishing the subscription,
the operation yields. A producer that calls signal.set() to a matching value
in that gap sends to zero subscribers — the event is lost and is() waits
forever for a second update. This is a time-of-check/time-of-use race
(issue #217), and it forced producers to yield* sleep(0) before publishing,
contradicting the repo's no-sleep-test-sync policy that recommends is() as a
deterministic state waiter.

Approach

Establish the subscription before the decisive current-state check. is is
now a stateless function returning an Operation whose [Symbol.iterator]
subscribes first, then takes the valueOf() snapshot. With no yield between
subscribe and snapshot, a matching change during subscription setup is either
reflected in the snapshot or buffered in the subscription and consumed via
subscription.next(). Stream-close-before-match still returns cleanly.

• signals/helpers.ts — subscribe-first is(). Public is() signature and the
  ValueSignal interface are unchanged; no replay added, so the fix stays
  independent of and compatible with the replay-ownership work in ♻️ Move replay semantics from signals to stream-helpers #213.
• signals/helpers.test.ts — behavior coverage (already-matching;
  non-matching-then-matching) plus a is() can hang when publish lands between valueOf() check and each() subscription #217 regression that spawns a producer
  mutating the signal with no sleep, bounded by @effectionx/timebox purely
  as a diagnostic deadline.
• signals/README.md and .policies/no-sleep-test-sync.md — describe is() as
  a deterministic state waiter that needs no producer sleep()/yield.
• @effectionx/signals bumped 0.5.3 → 0.5.4; @effectionx/timebox added as
  a test-only dependency.

Verification

• pnpm check, biome lint/format, pnpm sync — clean.
• Signals suite 22/22 on Effection 4.0.2 (peer-range max-stable);
  reverting helpers.ts fails 3/4 (the race manifests on v4), proving the
  regression bites.
• The is() behaviors also pass against Effection 3.0.0 (peer-range
  minimum). Note the race is v4-specific — v3 was already correct — so the fix
  corrects v4 without regressing v3, satisfying the declared ^3 || ^4 range.

Summary by CodeRabbit

• New Features

  • Improved deterministic waiting: the helper now reliably observes signal state changes without requiring sleep/yield delays.

• Documentation

  • Clarified that the helper establishes subscriptions before checking current state so updates aren’t missed; examples updated to remove unnecessary sleeps.

• Tests

  • Added thorough tests covering immediate matches, subsequent updates, and subscription-timing edge cases.

• Chores

  • Package version bumps: signals → 0.5.4; stream-helpers → 0.8.3; worker → 0.5.3.

[coderabbitai]

Warning

Review limit reached

@taras, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 34 minutes and 57 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9a5f443e-4a13-4ec4-bf31-210d2cff6f25

📥 Commits

Reviewing files that changed from the base of the PR and between 20bb576 and e6ce60c.

⛔ Files ignored due to path filters (1)

• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (8)

• .policies/no-sleep-test-sync.md
• signals/README.md
• signals/helpers.test.ts
• signals/helpers.ts
• signals/package.json
• signals/tsconfig.json
• stream-helpers/package.json
• worker/package.json

📝 Walkthrough

Walkthrough

This PR fixes a race condition in the is() signal helper where matching updates could be lost during subscription initialization. The implementation now establishes a subscription before checking the current value, guaranteeing deterministic behavior without requiring sleep() calls in tests or examples.

Changes

is() helper deterministic synchronization

Layer / File(s)	Summary
Core is() implementation rewrite signals/helpers.ts	is() changed from generator function to Operation<void> with explicit iterator. New implementation subscribes before checking current value, then iterates updates until predicate matches, eliminating TOCTOU race condition. Imports adjusted to remove each helper dependency.
Test coverage and deterministic validation signals/helpers.test.ts	Existing test updated to remove sleep(0) delay. Three new test cases added: immediate completion when current value matches, completion after first matching update following non-matching updates, and regression test verifying matching updates during subscription setup are not lost (with timebox timeout guarantee).
Documentation and policy updates signals/README.md, .policies/no-sleep-test-sync.md	README now documents subscription-before-check behavior and guarantees matching updates are never lost. Policy documentation examples updated to remove sleep() calls and explain deterministic synchronization.
Package version and timebox dependency setup signals/package.json, signals/tsconfig.json	Version bumped 0.5.3 → 0.5.4. Added @effectionx/timebox workspace dev dependency and TypeScript project reference to support timeout-based regression test.

Sequence Diagram

sequenceDiagram
  participant Caller
  participant Operation as is() Operation
  participant Stream as ValueSignal
  participant Subscription
  Caller->>Operation: call is(stream, predicate)
  Operation->>Stream: yield* stream (subscribe)
  Stream-->>Subscription: subscription iterator
  Operation->>Stream: stream.valueOf() (check current)
  alt Current matches predicate
    Operation-->>Caller: return immediately
  else Current does not match
    loop Until match or completion
      Operation->>Subscription: subscription.next()
      Subscription-->>Operation: next value
      Operation->>Operation: check predicate(value)
    end
    alt Match found
      Operation-->>Caller: return
    else Stream completed
      Operation-->>Caller: return
    end
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• #217: Directly addresses the TOCTOU race condition hang in is() by rewriting the implementation to subscribe before checking current value, ensuring matching updates are never lost during startup.

Suggested reviewers

• cowboyd

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically identifies the main fix—resolving a subscription race condition in signals.is() referenced as issue #217—which directly corresponds to the core change in the PR.
Description check	✅ Passed	The description fully addresses both required sections: Motivation clearly explains the race condition with code examples, and Approach provides a comprehensive summary of all changes made across multiple files and testing verification.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
Policy Compliance	✅ Passed	PR complies with all policies: no AI agent marketing in title/description; package.json has required description and keywords; version bumped 0.5.3→0.5.4 matches bug fix.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/signals-is-subscription-race

---

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 and usage tips.}

[cowboyd]

🎉

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@effectionx/signals@218

npm i https://pkg.pr.new/@effectionx/stream-helpers@218

npm i https://pkg.pr.new/@effectionx/worker@218

commit: e6ce60c