Skip to content

test(cursor): stabilize stream-health watchdog timing - #3930

Closed
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:agent/cursor-stream-health-budget-20260908
Closed

test(cursor): stabilize stream-health watchdog timing#3930
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:agent/cursor-stream-health-budget-20260908

Conversation

@luvs01

@luvs01 luvs01 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

The macOS control hit the 400ms silence watchdog in two Cursor stream-health fixtures, while both cases passed in another macOS lane on the same head. The positive progress fixture also ended after about 900ms against a 10-second heartbeat-only limit, so it could not detect a missing progress-clock refresh.

Use one load-adjusted scale S for the silence deadline, 2S for heartbeat-only failure, and at least 3S of meaningful progress after the client receives its first text. Scaling once preserves the ordering when the existing helper applies its CI floor. Keep frequent heartbeat traffic and give each producer a bounded completion path, including write failure cleanup.

The change is confined to one existing test file and its private drain helper.

Verification

Current head 141077f7270e2f2a0564fb036d091f0cf793b784, rebased onto dev 2abf071e0a3e765195ea2997b962171c87151f06. Tree: cedc011fa2e59476077219022b97f5bd7b6f0831. The local results below were obtained on 4b24e3c228bd56c0ababf2bdc3e0884654e0d7e3; the rebase leaves the fixture, production transport, package manifest and lockfile unchanged.

  • bun run test:changed --timeout 60000: 5 passed across the selected test file with Bun 1.4.0 on Windows. Its full-suite profile exercised 16-second heartbeat-only and 24-second progress intervals; the selection completed in 43.8 seconds.
  • The original positive fixture still passed when progress-clock refresh was removed. The revised fixture failed when inbound refresh was removed (silence at about 1 second) and when progress refresh was removed (heartbeat-only at about 2 seconds).
  • Making heartbeat traffic count as meaningful progress also failed the heartbeat-only fixture, which ended at its bounded fixture limit instead of leaving the producer running.
  • Each ablation restored the production transport and verified its SHA-256. The final diff contains only the test file.
  • bun run typecheck, bun run privacy:scan, and git diff --check: passed.
  • Independent read-only review checked deadline ordering, first-text receipt, and producer cleanup; its cleanup finding was addressed and re-reviewed.

Original-head full CI completed successfully with 26/26 jobs. Full CI on the rebased head also completed successfully with 26/26 jobs. CodeRabbit reported no actionable findings on the original head; the PR file is unchanged by the rebase and there are no unresolved review threads. This does not claim a second completed CodeRabbit run on the new commit.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

  • All CI tests are green on my local testing.
  • I pushed my PR to the latest dev commit.
  • I resolved all correct Codex and CodeRabbit findings.
  • My PR is ready for review.

Summary by CodeRabbit

  • Tests
    • Improved automated coverage for stream health and heartbeat behavior.
    • Added progress tracking after the first text response.
    • Added safeguards for streams that exceed configured fixture limits.
    • Updated timing checks to adapt reliably across continuous integration environments.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 2453f533-ee66-42d9-8983-c8f8572c8c4a

📥 Commits

Reviewing files that changed from the base of the PR and between 09f669a and 4b24e3c.

📒 Files selected for processing (1)
  • tests/providers/cursor/cursor-stream-health.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The Cursor stream health tests now derive deadlines from CI watchdog helpers. They add first-text observation, fixture-limit cleanup, scaled heartbeat timing, and progress-span assertions.

Changes

Cursor stream health tests

Layer / File(s) Summary
Timing and progress helpers
tests/providers/cursor/cursor-stream-health.test.ts:15, tests/providers/cursor/cursor-stream-health.test.ts:103-145
The test imports CI watchdog helpers, derives scaled timing constants, and adds a one-time onFirstText callback to drain.
Stream health scenarios
tests/providers/cursor/cursor-stream-health.test.ts:164-232
The heartbeat-only test uses a fixture limit and scaled watchdog values. The meaningful-frames test tracks first text, waits for a progress span, and asserts text, done, and completed progress.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 4b24e

Cursor stream-health tests now use CI-scaled deadlines, bounded fixture completion, and first-text progress tracking to reduce timing flakiness without changing production behavior.

Suggested reviewers: lidge-j

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: stabilizing Cursor stream-health watchdog timing tests.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

@luvs01

luvs01 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@luvs01
luvs01 force-pushed the agent/cursor-stream-health-budget-20260908 branch from 4b24e3c to 141077f Compare September 7, 2026 17:20
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 52 / 80

이 PR은 제품 코드가 아니라 Cursor 스트림 헬스 워치독 테스트가 로드된 macOS 러너에서 침묵/heartbeat-only 시계가 서로 이겨 버리던 flake를 고칩니다. 지금 dev HEAD는 942c02873이고, HEAD의 tests/providers/cursor/cursor-stream-health.test.ts는 침묵 400ms·heartbeat-only 900ms 같은 고정 짧은 값을 쓰고 있었습니다. 본문이 가리킨 macOS control 잡에서는 침묵 워치독이 먼저 터져 “heartbeat-only” 기대를 깨고, 진행 프레임 fixture도 전체 길이가 heartbeat-only 한도보다 짧아 progress-clock 갱신을 제대로 못 볼 수 있었습니다. 이번 변경은 tests/helpers/ci-watchdog.tsisolationBudgetMs/watchdogMs로 한 번만 스케일한 S를 쓰고, 침묵=S·heartbeat-only=2S·의미 있는 진행≥3S·fixture 한도=4S+2s로 순서를 유지합니다. CI 바닥값이 있어도 비율이 무너지지 않게 한 점이 핵심입니다. drain에 첫 text 수신 콜백을 두어 “클라이언트가 받은 뒤”부터 진행 구간을 재고, heartbeat fixture에는 종료 타이머·write 실패 정리도 넣었습니다. 파일 하나·테스트만이라 types/config 분할과 무관합니다.

로컬에서는 isolationBudgetMs(1000)이 그대로 1000이라 스위트가 갑자기 길어지지 않고, CI/OCX_TEST_FULL_SUITE에서는 바닥(5s/8s)으로 올라갑니다. 그래서 “로드된 러너에서만” 터지던 클래스와 같은 헬퍼 계약을 씁니다. 제품 전송 경로(createLiveCursorTransport 등)는 건드리지 않습니다. 베이스는 #3927 tip(2abf071e0)이라 #3931 이후보다 한 스텝 뒤지만 MERGEABLE이고 충돌면이 테스트뿐입니다. draft·chore 라벨이며, enforce-target/hygiene는 통과 기록이 있고 일부 label/resolve는 재실행·취소가 섞여 있습니다. 릴리스 트레인(#3932 preview·#3933 main 로스터 보정)과는 무관한 CI 안정화입니다.

라인 tests/providers/cursor/cursor-stream-health.test.ts silenceMs/heartbeatOnlyMs/progressDurationMs - 시계를 한 번의 isolationBudgetMs에서 파생해 CI floor에도 S < 2S < 3S 순서를 지킨다.
라인 drain onFirstText - 첫 text를 받은 시각부터 진행 구간을 재서, 서버가 일찍 써도 클라이언트 수신이 늦은 경우를 커버한다.
경로 tests/helpers/ci-watchdog.ts isolationBudgetMs / watchdogMs - 이미 HEAD에 있는 로드 스케일 헬퍼. 이 PR은 import만 추가한다.
경로 heartbeat-only fixture - 40ms ping 유지·fixtureLimitMs로 스트림을 닫아, 침묵 시계만 리셋되고 heartbeat-only는 만료되게 한다.
경로 PR draft / 범위 - 제품 코드 0. 머지해도 런타임 위험이 없고, flake 재발만 보면 된다.

메인테이너의 판단이 필요한 지점

  • CI에서 fixture가 수 초~십수 초로 길어지는 trade-off를 macOS control lane 예산 안에서 받아들일지
  • chore/test-only를 2.47 컷 전에 넣을지, open-dev 2.48.0 구간에 넣을지
  • draft ready를 초점 파일 초록만으로 허용할지

너의 추천
제품 리스크 없는 CI 안정화라 점수는 중간. Cross-platform/macOS 관련 체크가 이 head에서 초록이면 ready 후 머지. 릴리스 블로커는 아니고, #3931과 겹치지 않으니 그냥 랜딩해도 된다. close-don't-rebase 대상 아님.

이 댓글은 grok-bot이 작성했습니다

@github-actions
github-actions Bot marked this pull request as ready for review September 7, 2026 18:16
lidge-jun added a commit that referenced this pull request Sep 7, 2026
…or stream-health watchdog (#3940)

Preserve captured stdout and stderr when a test lane times out. Bound
post-exit pipe draining to one second and report incomplete capture as a
failure when the child otherwise exited successfully.

Scale the Cursor stream-health fixture deadlines from one load-adjusted
budget and verify meaningful progress continues for the required interval
after the client receives its first text.

Carries #3924 and #3930 without changes to their files. The branch commits
retain the original author and cherry-pick provenance from:
e241632
141077f

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Landed on dev as 221617b80756f1be13a5db942cd80a3d8f79ab01, through #3940.

Your commit 141077f7270e2f2a0564fb036d091f0cf793b784 was carried unchanged with git cherry-pick -x, so the branch commit kept your authorship and its provenance line. I verified tests/providers/cursor/cursor-stream-health.test.ts on dev byte-for-byte against this pull request's head. The squash commit carries Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>.

It landed as the tip of a two-layer stack over #3924, so one CI run covered both: run 34152136978, all 16 applicable jobs green, including both macOS lanes that surfaced the original flake. Your own contributor run on 141077f7 had already passed all 26 jobs.

Closing as superseded by that verified landing. The part worth noting for later: scaling the three budgets from one value keeps their ordering intact under the CI floor, and asserting the progress span actually completed is what turns this from a longer timeout into a real check.

@lidge-jun lidge-jun closed this Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants