Skip to content

fix(server): confirm an exact signal count instead of resolving on the first fire - #778

Merged
divshekhar merged 3 commits into
reticlehq:mainfrom
Chirag6722:fix/775-signal-count-confirm
Sep 6, 2026
Merged

divshekhar merged 3 commits into
reticlehq:mainfrom
Chirag6722:fix/775-signal-count-confirm

Conversation

@Chirag6722

Copy link
Copy Markdown
Contributor

Closes #775.

One line, and the reason it is the right one

assertsExactCount decides which predicates get the COUNT_CONFIRM_MS hold. Its combinator recursion is already complete; the leaf test was not:

if (PredicateKind.NOT === predicate.kind) return assertsExactCount(predicate.predicate);
return PredicateKind.NET === predicate.kind && predicate.count !== undefined;

signal carries count too, so a signal count resolved on its first match: count: 1 silently meant "at least 1", which is the assertion count exists to avoid.

The channel it was missing on is the one that most needs it. From the signal.count schema:

The double-fire is the defect no state-only oracle can see: a handler wired twice fires the signal twice, the store ends up in the right shape either way, and a presence check is green on both.

A double-wired handler is exactly a React double-effect, which fires within one commit — well inside the 300 ms hold net already had. And evalSignal counts correctly, as evalNet did; the wait was early, not the count. The existing "signal count: exactly-once passes on one fire, and a double-fire is caught" test passes today because it evaluates immediately, which is why the gap survived: the counting was never the broken half.

Because the recursion above was already right, a signal count nested in an allOf was wrong the same way, and the same line fixes it.

What is deliberately untouched

Presence-only signal predicates. No count still means "at least one", which is satisfiable early, and holding those would make every ordinary signal wait pay the confirmation delay for nothing — the same reasoning the net block states. A test pins it.

count: 0. I raised it in #775 and did not take it here. It now gets the hold as a side effect of count !== undefined, which I think is right — an absence claim is the one a late arrival can falsify — but it also means every zero-count signal assertion pays 300 ms, and that is a latency call rather than a correctness one. Say the word if you would rather it were excluded and I will special-case it.

Tests

Four cases mirroring the net block case for case, so the two channels are visibly held to one rule: the double-fire at the measured 59 ms gap fails and reports what it saw, a genuine single fire still passes without burning the timeout, presence-only still resolves on the first match, and a count nested in allOf still holds.

Written RED first — before the fix, exactly two of the four failed with expected true to be false (the direct case and the allOf case), while the two guard cases passed, which is the split that shows the fix does not over-reach.

No duration is asserted as a correctness signal, per the repo's rule on timing assertions: the pass-case bound (< 3000 on a 10 s budget) is the same bound the net block uses to show an honest "exactly one" does not burn the timeout.

Gates

format:check, lint 17/17, typecheck 22/22, and the server suite at 6426 passing (predicate.test.ts 87/87). init/format-generated.test.ts fails identically on a clean main on this Windows box and is unrelated.

This touches the wait path behind act_and_wait and assert, so the e2e battery is the one that matters here; I started a local run and it was still booting when I opened this, so I am leaving that to CI rather than reporting a result I do not have.

Chirag6722 and others added 2 commits September 6, 2026 00:12
…e first fire

`COUNT_CONFIRM_MS` keeps an exact-count wait watching after it first reads true, because a count
only rises while the window is open and "exactly N" cannot be settled early. `assertsExactCount`
decides which predicates get that hold, and its leaf test read `net` alone.

`signal` carries `count` too, so a signal count resolved on its first match and `count: 1` silently
meant "at least 1" — the assertion `count` exists to avoid. A second fire 59ms later, the gap
measured on the net side, arrived after the wait had stopped looking.

The channel this was missing on is the one that most needs it. The `signal.count` schema calls the
double-fire "the defect no state-only oracle can see": a handler wired twice fires the signal twice,
the store ends up in the right shape either way, and a presence check is green on both. A React
double-effect fires within one commit, well inside the 300ms hold that `net` already had. `evalSignal`
counts correctly, as `evalNet` did — the wait was early, not the count.

The combinator recursion above already covered allOf/anyOf/not, so a signal count nested in an allOf
was wrong the same way and is fixed by the same line.

Presence-only signal predicates are untouched: no `count` still means "at least one", which IS
satisfiable early, and holding those would make every ordinary signal wait pay the confirmation
delay for nothing.

Tests mirror the net block case for case, so the two channels are held to one rule, and assert
bounds rather than durations.

Signed-off-by: Chirag <chiraghonnyal6722@gmail.com>
…t the clock

Signed-off-by: Divyanshu Shekhar <imdshekhar@gmail.com>
@divshekhar

Copy link
Copy Markdown
Contributor

Taking this over #777, which makes the same one-line change — this one adds the allOf nesting case and says why the two channels share a rule, so it is the better of the pair.

Verified adversarially before merging: reverting the assertsExactCount change turns both the direct and the nested case red, so the tests fail for the reason they claim.

One maintainer edit pushed on top: the expect(Date.now() - started).toBeLessThan(3000) line is replaced with a per-test timeout (}, 3_000)). Date.now() - t < N is a statement about the machine and fails only under parallel load — CLAUDE.md calls that out under Timing assertions are a bug, and a flake of exactly that shape cost this repo three CI round-trips yesterday (#770). The property you were pinning is preserved: the test still fails if the hold starts burning the whole 10s window. I applied the same change to the pre-existing net case just above it, which is where this pattern was copied from.

@divshekhar
divshekhar merged commit 0138266 into reticlehq:main Sep 6, 2026
19 checks passed
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.

An exact signal count is not confirmed after it first reads true, so a double-fire still passes

2 participants