Skip to content

fix(poll): pace long-poll reads the server answers without holding - #581

Merged
zeshi-du merged 2 commits into
TestSprite:mainfrom
cipoklean:fix/poll-pace-early-long-poll
Nov 9, 2026
Merged

fix(poll): pace long-poll reads the server answers without holding#581
zeshi-du merged 2 commits into
TestSprite:mainfrom
cipoklean:fix/poll-pace-early-long-poll

Conversation

@cipoklean

@cipoklean cipoklean commented Oct 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What does this PR do?

The --wait run poll re-issues a ?waitSeconds read immediately after every non-terminal answer, assuming the server held it. If a proxy answers early, the loop becomes back-to-back GETs for the whole --timeout. This adds a client-side floor: each long-poll read's held time is measured, and a non-terminal answer that returns faster than the 2s fallback pace sleeps only the remainder, clamped to the --timeout deadline. Held reads behave exactly as today.

  • src/lib/poll.ts long-poll branch only; plan-poll.ts and test.ts call sites untouched.
  • No extra requests or dependencies; no changes to exit codes, retryAfterSeconds or the VALIDATION_ERROR fallback.

Related issue

Closes #481

Season 4 CLI Improvement Bonus

  • I'm entering this PR for the Season 4 CLI Improvement Bonus

Discord username: Cipoklean

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that changes existing behavior)
  • Documentation only
  • Build / CI / chore

Checklist

  • PR targets the main branch.
  • Commits follow Conventional Commits.
  • npm run lint and npm run format:check pass.
  • npm run typecheck passes.
  • npm test passes and coverage stays at or above the 80% gate.
  • New behavior is covered by unit tests (mock-based; no network or credentials required).
  • No secrets, API keys, internal endpoints, or personal data are included.
  • User-facing changes are reflected in README.md / DOCUMENTATION.md where relevant.

Notes for reviewers

Three tests in src/lib/poll.spec.ts with a fake clock that the sleep spy advances: an instant-return client with a bounded read count (fails on main), a held read gets no added sleep, and the pacing sleep is clamped to the deadline (fails on main). Full suite green locally.

Summary by CodeRabbit

  • Bug Fixes
    • Improved long-polling behavior to prevent rapid repeated requests when the server responds sooner than expected.
    • Preserved immediate retries when the server holds a request for the expected duration.
    • Ensured polling delays respect the remaining request timeout, preventing unnecessary waits or extended polling beyond the deadline.
    • Added safeguards for servers that do not honor requested wait durations.

@cipoklean
fix(poll): pace long-poll reads the server answers without holding
a3d01c3 
The --wait run poll re-issues a `?waitSeconds` read immediately after
every non-terminal answer, on the assumption that the server held it for
up to 25s. When a proxy in front of the service answers at once instead,
the loop turns into back-to-back GETs for the whole --timeout.

Measure how long each long-poll read was held. A read held for at least
the fallback pace (2s) is still re-issued at once; a faster one sleeps
only the remainder, clamped to the --timeout deadline. No extra
requests, and no change to exit codes, retryAfterSeconds or the
VALIDATION_ERROR fallback.

Closes TestSprite#481
@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •
edited
Loading

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration

Commits

Reviewing files that changed from the base of the PR and between a3d01c3 and 31d990c.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

Long-poll reads now enforce a minimum two-second interval when a non-terminal response returns sooner. The delay is capped by the remaining timeout. Tests cover immediate continuation after a 25-second held read, rapid responses, and timeout-clamped pacing.

Changes

Long-Poll Pacing

Layer / File(s) Summary
Enforce minimum read interval
src/lib/poll.ts, src/lib/poll.spec.ts		 The polling loop measures request duration and waits for the remainder of the two-second interval after an early non-terminal response. The wait is capped by the remaining timeout. Tests cover held reads, rapid responses, and timeout-clamped pacing.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: Minimal · previous review a3d01

The long-poll pacing change is mergeable after normal checks. Pacing for test-plan generation remains separate work.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Linked Issues check Warning Issue #481 requires pacing for both pollRunUntilTerminal and the analogous test plan generate polling path. This PR implements and tests the src/lib/poll.ts long-poll pacing: it measures the rea… Implement the same held-time pacing and deadline clamp in the src/lib/plan-poll.ts long-poll active-status path, and add deterministic unit tests for fast responses, held responses, and deadline clamping. Keep the existing fallback and er…
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title identifies the main change: pacing long-poll reads when the server does not hold the request. It is specific and related to the changeset, despite minor grammatical awkwardness.
Out of Scope Changes check Passed The changes are limited to src/lib/poll.ts and src/lib/poll.spec.ts. The new exported interval, implementation logic, and tests directly support the run-poll pacing requirement in issue #481. No u…
Full details: Out of Scope Changes check

Explanation

The changes are limited to src/lib/poll.ts and src/lib/poll.spec.ts. The new exported interval, implementation logic, and tests directly support the run-poll pacing requirement in issue #481. No unrelated production behavior, dependency, or call-site changes are present.

Full details: Linked Issues check

Explanation

Issue #481 requires pacing for both pollRunUntilTerminal and the analogous test plan generate polling path. This PR implements and tests the src/lib/poll.ts long-poll pacing: it measures the read duration, sleeps for the remaining 2-second interval, and clamps the sleep to the deadline. The diff does not modify src/lib/plan-poll.ts, so the plan-generation path remains unmet.

Resolution

Implement the same held-time pacing and deadline clamp in the src/lib/plan-poll.ts long-poll active-status path, and add deterministic unit tests for fast responses, held responses, and deadline clamping. Keep the existing fallback and error behavior unchanged.

Finishing Touches

Fix failing CI checks

  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

ℹ️ Recent review info

⚙️ Run configuration

  • Configuration used: Repository: TestSprite/testsprite-cli/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 72607088-b63b-429b-a1e0-1eeaadd5e539
📥 Commits

Reviewing files that changed from the base of the PR and between d61e6df and a3d01c3.

📒 Files selected for processing: 2

  • src/lib/poll.spec.ts
  • src/lib/poll.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

@Andy00L Andy00L mentioned this pull request Oct 20, 2026
Merged
10 of 14 tasks
@github-actions

github-actions Bot commented Oct 22, 2026

Copy link
Copy Markdown

🏆 Season 4 CLI Improvement Bonus: entry recorded. Thanks, @cipoklean!

To be counted, this PR needs the following by Oct 29, 11:59 PM PT:

  • ✅ A linked issue assigned to you.
  • ✅ Out of draft.
  • ✅ A Discord username in the description.

Rewards go to merged PRs judged a genuine improvement, and up to three bonus-eligible PRs count per person. Untick the box to withdraw the entry. This comment refreshes when you edit the PR. Full rules.

@github-actions github-actions Bot added the s4-cli-bonus Entered for the Season 4 CLI Improvement Bonus label Oct 22, 2026
@zeshi-du
zeshi-du merged commit 2ff4960 into TestSprite:main Nov 9, 2026
24 checks passed
@zeshi-du zeshi-du mentioned this pull request Nov 9, 2026
Merged
10 of 14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zeshi-du zeshi-du zeshi-du approved these changes
@ruili-testsprite ruili-testsprite Awaiting requested review from ruili-testsprite ruili-testsprite is a code owner

Assignees

No one assigned

Labels

s4-cli-bonus Entered for the Season 4 CLI Improvement Bonus

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Hackathon] --wait polling turns into a tight request loop when a long-poll read comes back without being held (run poll + plan generation)

2 participants

@cipoklean @zeshi-du