Repository navigation
js/net's widened max-age re-drop test ends cleanly, not with a reset - #8307
Conversation
Quest-Claim: 48b7b9d01691a62d
The "keeping the budget" lite-07 test finished the track right after its priority-only update, racing the publisher's reply that re-drops the stale groups. When the end won, the reply was reset with Cancel and the reader failed with it. The test now waits until the reader holds the re-drop of groups 1 and 2 under the new update, then finishes the track, so the end is a FIN. The wait is event-driven, so a missing re-drop still fails, and the 500 ms wall-clock assert it replaces is gone. Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 4 minutes. View limit details
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. |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed ba992ae. No actionable findings. The test now subscribes before updating, observes stale coverage clear and return for both groups, and only then finishes the producer. This addresses the previously identified test ordering race without changing the publisher's intentional cancellation behavior or relying on the removed elapsed-time assertion.
Verification: static review of the complete diff, subscriber update/drop handling, track hooks, and signal notification implementation through GitHub. No tests were run and CI flake elimination was not independently reproduced.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — one test-fidelity suggestion inline.
Reviewed changes
- Re-drop wait: the failing lite-07 test now subscribes to the drops signal before
reader.update, waits for the update's clear and then for the publisher's re-drop of groups 1 and 2 to land, and only then finishes the track, so the end is a FIN rather than aCancelfromresponse(). - Wall-clock assert removed: the
now()import and the 500 ms elapsed check are gone; a missing re-drop now fails through the test timeout instead of the grace. - Quest closed:
quest/m0/test-flakes-2/widened-max-age-reset.mdand its epic README link are removed.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏
…2/widened-max-age-reset
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Your OpenRouter API key was rejected. Issue a new key with the provider, then update the copy Pullfrog uses, in Pullfrog or in this repo's GitHub Actions secrets. The provider's responsePullfrog provider settings → · GitHub Actions secrets →
|
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head 6a28e24 incrementally from ba992ae, accounting for base changes. No new actionable findings.
The regression now explicitly resends delay.max = 1500 ms with the priority update, matching Subscriber.update's replacement/defaulting semantics and preserving the budget the test intends to exercise. It still subscribes to re-drop notifications before the update and waits for their arrival before finishing the producer. No production behavior changes.
Verification limits: GitHub-only static source/diff review. No tests or merge-queue flake reproduction executed.
Claims quest/m0/test-flakes-2/widened-max-age-reset.md, which this PR finishes.
Summary
js/net's "an update keeping the budget drops the stale groups again on lite-07" finished the track right after its priority-only update, racing the publisher's reply that re-drops groups 1 and 2. When the end won, the reply was reset withCanceland the reader failed with it.The test now waits until the reader holds the re-drop of groups 1 and 2 under the new update, then finishes the track, so the end is a FIN. The publisher is unchanged.
Choices made without asking
hooks.givenUp(...).changedandhooks.staleHeld(...)fromjs/net/src/internal.ts, which other tests already import. The update clears the stale verdict, then the re-drop sets it again. The listener is registered beforeupdate(), so the clear is never missed. No public or wire API changes.quest rmdeletedquest/m0/test-flakes-2/widened-max-age-reset.mdand its link in the epic README.Models
Size: XS
Implementer: Claude, claude-haiku-5-5
Verification
bun test src/lite/widened_max_age.test.ts --rerun-each 100: 500 pass, 0 fail.quest gate just check: exit 0 (biome, js check, js tests).quest check: 586 documents ok.Waiting
None. The quest is complete, and no decision is left for the maintainer.
Note: moq-dev/moq#8287 also edits
quest/m0/test-flakes-2/README.md. If it lands first, mergemainhere; the conflict is only the link list.🤖 Generated with Claude Code
(Written by Claude Haiku 5.5)
Review
Rewritten by
quest reviewafter each round.01a1bf43-c74c-7200-850d-94ef67d864bb, round 1The test waits for the re-drop event before finishing the producer and preserves the intended delay budget; source review found no actionable defects, though tests were not run as instructed.