Skip to content

[finding] deliveredInbox returns SHORT on timeout, so a slow run is asserted as a wrong value — and in one call site that makes the test PASS for the wrong reason #10106

Description

@os-elon

Observation-class finding, filed unassigned. No user is affected — this is about a test helper's failure mode, not shipped behaviour.

Surfaced as a candidate lead during PM review of PR #10093 (#9974), where Test Core (3/3) failed once on @objectstack/example-showcase and passed on re-run at the identical commit. ⛔ This card does NOT claim to be that failure's cause — that was never reproduced, and the #9974 dev deliberately declined to file a flaky-test card for exactly that reason, which was the right call. What is filed here is a structural property readable from the source, which stands whether or not it ever caused a red.

The measurement

examples/app-showcase/test/approval-resume-relation-expand.test.ts:

asyncfunctiondeliveredInbox(booted: Booted,titleFragment: string,expected: number): Promise<any[]>{// :146constdeadline=Date.now()+5_000;// :147if(Date.now()>deadline)returnhits;// :152

On timeout the helper returns what it has so far — it does not throw, and it does not signal that it gave up. The callers then assert on the length of that short return:

lineassertionwhat a timeout produces
:259expect(delivered, 'the Notify: Cleared inbox message was never delivered').toHaveLength(1)fails, with a message that misdiagnoses it
:409expect(delivered, 'the subflow notified nobody — the project hop resolved to nothing').toHaveLength(1)same
:362expect(await deliveredInbox(booted, 'Invoice cleared', 0)).toHaveLength(0)passes — for the wrong reason

Why it is worth a card rather than a shrug

1. The failure message is a wrong diagnosis baked into the test. A 5-second wall-clock deadline exceeded on a loaded runner prints "the Notify: Cleared inbox message was never delivered". It was delivered; the poll gave up. Whoever reads that CI log is told a false fact by the assertion itself, and will go looking for a delivery bug that does not exist. That is strictly worse than a bare toHaveLength failure, because the annotation is confident and wrong.

2. ⭐ The same helper can produce a false PASS, and that half is not covered by "make the timeout louder".:362 asserts the inbox stays empty. A timeout returning short satisfies that assertion, so the case that is supposed to prove nothing was delivered also passes when the poll never waited long enough to see anything. A green there is currently consistent with the helper being broken. Whatever the fix is, it has to distinguish "waited the full window and saw nothing" from "gave up early", or :362 remains a test that cannot fail for its own reason.

3. The prevailing condition is exactly when it bites.Test Core runs the monorepo with turbo concurrency against a booted showcase stack; a fixed 5 s wall-clock budget is a thin margin under that load, and it is the same margin for every call site.

What was already tested and did NOT confirm

The #9974 dev tried to reproduce this as the cause and could not: the suite ran green 10/10 — standalone ×3, under CI's own env (TZ=UTC, NODE_OPTIONS='--report-on-signal --report-signal=SIGUSR2'), serial --maxWorkers=1, ×5 repeat — and specifically did not fail at 3× CPU oversubscription. The CI-equivalent shard set (turbo ls --affected + partition-test-shards.mjs --shard 3/3 --exclude @objectstack/dogfood, which does contain example-showcase) ran 74/74.

⇒ So the load hypothesis is named and unconfirmed. Recorded here so the next reader does not re-run those same probes expecting a different answer.

What this card should establish

  1. Make the timeout distinguishable from a real absence — throw, or return a discriminated result — so :259 and :409 report "the poll timed out" instead of "nobody was notified".
  2. Fix the :362 direction specifically: a "stays empty" assertion must be unable to pass on a short return. This is the load-bearing half and the easy one to miss.
  3. ⛔ Do not simply raise the 5 s deadline. That trades a wrong-message failure for a slower wrong-message failure and leaves the false-pass at :362 untouched.
  4. ⛔ Do not skip, quarantine, or .skip any of the three call sites.

Priority

Low. Maintainer, standing: 「目前也没有相关的真实用户」 · 「objectstack cloud 还没有正式上线,简化开发」. This is test-harness honesty, not product behaviour. It earns a card because a test that misreports why it failed costs the next reader a wrong investigation, and one that can pass for the wrong reason is not evidence at all.

Refs: PR #10093 / #9974 (where it surfaced) · #9350 (the open card on non-deterministic driver-sql live-DB failures — a different suite and a different mechanism; ⛔ not the same finding, do not merge them).

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions