fix(test): preserve lane output after timeouts and stabilize the Cursor stream-health watchdog

[lidge-jun]

Summary

Two test-infrastructure fixes by @luvs01, carried unmodified as a two-layer branch stack so one CI run verifies the cumulative tree.

Layer 1 — the runner loses the output that explains its own failure (#3924). When a captured test lane exceeded its timeout, runTestLane returned { exitCode: 124, output: "" } before awaiting and forwarding the captured output, so a long test:changed run reported 124 with nothing to explain it. The lane now reads stdout and stderr continuously, prints and returns whatever arrived before the timeout, and gives the pipes a bounded one-second drain after the child exits so a descendant holding a pipe open cannot stall the runner. Capture that ends incomplete is reported and turns an otherwise successful child into a failure, so a truncated log can never read as a green run.

Layer 2 — the Cursor stream-health fixtures were timing-fragile and under-tested (#3930). Two fixtures hit the 400 ms silence watchdog on one macOS lane while passing on another at the same head, and the positive fixture finished in about 900 ms against a 10-second heartbeat-only limit, so it could not detect a missing progress-clock refresh. Both now derive from one load-adjusted scale: S for silence, 2S for heartbeat-only, and at least 3S of observed progress measured from the client's first received text. Scaling once keeps the ordering intact when the shared helper applies its CI floor, and the progress fixture now asserts it actually completed that span rather than being cut short by its own safety limit.

Layer 1 is the runner; layer 2 is a fixture that runs under it, which is why they land in that order. The two file sets are disjoint, so each layer's diff stands alone.

Carries #3924 and #3930 unmodified via cherry-pick -x; both commits keep their original author and provenance line.

Co-authored-by: luvs01 27862058+luvs01@users.noreply.github.com

Verification

Hosted CI on this exact head is the verification gate for this change. Local suite, typecheck and build were NOT RUN by maintainer instruction for this delivery, and are not claimed as passing.

Construction evidence, verified locally:

• Ancestry is 942c02873 → ab06523e6 → 8b81676ac; each layer's parent is the commit below it.
• Both commits are authored by luvs01 <27862058+luvs01@users.noreply.github.com> and carry their (cherry picked from commit ...) lines referencing e2416323 and 141077f7.
• The cumulative delta against dev is exactly four files: docs-site/src/content/docs/contributing.md (+6/-0), scripts/test.ts (+81/-8), tests/ci-workflows/test-runner.test.ts (+147/-1), tests/providers/cursor/cursor-stream-health.test.ts (+59/-26).
• git diff --exit-code against each source PR head returns 0 for that PR's files, so nothing was altered in transit.

Contributor-side CI passed all 26 jobs on each source head: #3924 at e2416323 and #3930 at 141077f7. Those results are evidence for the source heads; this cumulative head needs its own run, which is what this pull request is for.

Note on the scope of that run: windows <n>/6 and macos control are workflow_dispatch-only, and npm-global is gated on the packaging path filter, which none of these four files match. Those job families are skipped by the workflow rather than passing, and are not counted as evidence.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed — contributing.md documents the timeout and incomplete-capture behavior.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults — none; the change touches the test runner and two test files.

Summary by CodeRabbit

• Bug Fixes

  • Test runs now preserve and display captured standard output and error output, including when a test times out.
  • Test lanes no longer hang indefinitely when descendant processes keep output pipes open.
  • Incomplete output capture is reported as a failed run, even when the test process exits successfully.
  • Stream health checks now use CI-scaled timing and more reliable progress detection.

• Documentation

  • Updated contributor documentation to describe timeout handling, output draining, and incomplete capture behavior.

[coderabbitai]

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: 07144417-a9d2-4c1f-83e7-6ffe1d1fc9df

📥 Commits

Reviewing files that changed from the base of the PR and between 942c028 and 8b81676.

📒 Files selected for processing (4)

• docs-site/src/content/docs/contributing.md
• scripts/test.ts
• tests/ci-workflows/test-runner.test.ts
• tests/providers/cursor/cursor-stream-health.test.ts

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

---

📝 Walkthrough

Walkthrough

The test runner now captures child output incrementally, bounds post-exit pipe draining, preserves output on timeout, and fails incomplete captures. Cursor stream-health tests use scaled watchdog timing and first-text progress tracking.

Changes

Test runner output capture

Layer / File(s)	Summary
Capture helper and runner contract scripts/test.ts	captureTestOutput continuously decodes stdout and stderr, supports snapshots and cancellation, and reports bounded completion. runTestLane is exported and accepts injectable writers.
Bounded drain and exit handling scripts/test.ts, docs-site/src/content/docs/contributing.md	runTestLane drains output for one second after exit, writes captured streams, returns output on timeout with exit code 124, and changes a clean exit to code 1 when capture is incomplete. The contributing guide documents this behavior.
Capture and lane outcome tests tests/ci-workflows/test-runner.test.ts	Tests cover UTF-8 chunk handling, cancellation, read errors, open pipes, and output delivery for passing, failing, and timed-out lanes.

Cursor stream-health timing

Layer / File(s)	Summary
Scaled stream-health fixtures tests/providers/cursor/cursor-stream-health.test.ts	The tests use scaled isolation and watchdog limits. The meaningful-frames fixture tracks the first text message, emits deltas until the progress span completes, and enforces a fixture limit.

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

Merge Risk: ⚪ Minimal · up to b5218

No actionable merge-blocking risk is established by the current changes.

Sequence Diagram(s)

sequenceDiagram
  participant ChildProcess
  participant runTestLane
  participant captureTestOutput
  participant OutputWriters
  ChildProcess->>captureTestOutput: emit stdout and stderr
  runTestLane->>captureTestOutput: finish with 1-second drain bound
  captureTestOutput-->>runTestLane: captured output and completion status
  runTestLane->>OutputWriters: write stdout and stderr
  runTestLane-->>ChildProcess: return exit code and combined output

Loading

Suggested reviewers: olddonkey

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes both primary changes: preserving test-lane output after timeouts and stabilizing the Cursor stream-health watchdog. It is specific, concise, and suitable for a pull req…
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.

Full details: Docstring Coverage

Explanation

Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/260908-d-group-l2-cursor-watchdog

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[lidge-jun]

리뷰 · 우선순위 66 / 80

이 PR은 기여자 @luvs01의 테스트 인프라 수정 두 층을 손대지 않고 쌓아 dev(HEAD 942c02873, tip #3931 GUI roster/fallback 분리) 위에 올린 랜딩 스택입니다. 레이어 1은 #3924, 레이어 2는 #3930이고, 둘 다 cherry-pick -x로 원작자·provenance 줄이 그대로입니다. 파일 집합이 겹치지 않아서(러너·문서 vs Cursor 스트림 헬스 픽스처) 한 번의 CI로 누적 트리를 검증하려는 구성입니다. types.ts/config.ts 분리 캠페인과는 무관하고, 제품 런타임 경로가 아니라 scripts/test.ts와 픽스처만 건드립니다.

지금 dev의 runTestLane은 capture 모드에서 new Response(child.stdout).text()로 EOF까지 기다립니다. 그런데 레인 타임아웃이 나면 exitCode: 124와 함께 output: ""를 바로 돌려줍니다. 그래서 test:changed처럼 긴 캡처 레인이 124로 죽어도 “왜 죽었는지” 로그가 비어 있습니다. 이 PR은 captureTestOutput을 새로 두고 stdout/stderr를 읽는 동안 계속 모읍니다. 타임아웃이어도 그 시점까지의 스냅샷을 찍고, 자식이 끝난 뒤에는 파이프를 최대 1초만 더 비웁니다. 자손이 파이프를 붙잡고 있어도 러너가 무한 대기에 걸리지 않게 하려는 뜻입니다. 캡처가 끝까지 안 끝나면 그 사실을 명시하고, 자식이 0으로 나왔더라도 실패로 바꿉니다. 잘린 로그가 초록으로 보이는 일을 막는 계약입니다. contributing.md에도 같은 동작을 짧게 적어 두었습니다.

레이어 2는 tests/providers/cursor/cursor-stream-health.test.ts입니다. 지금 HEAD에는 이미 tests/helpers/ci-watchdog.ts의 isolationBudgetMs / watchdogMs가 있고(#3351 계열), 이 픽스처만 예전처럼 고정 400ms / 900ms / 10s를 쓰고 있었습니다. 로드가 센 macOS 레인에서는 침묵 워치독이 먼저 터져 “heartbeat-only”를 기대하는 단언이 깨질 수 있고, 긍정 픽스처는 심박만 있는 한도보다 훨씬 짧게 끝나서 “진행 시계가 실제로 리셋되는지”를 거의 못 봅니다. 이번 변경은 기준 한 번(S = isolationBudgetMs(1000))으로 침묵=S, heartbeat-only=2S, 관측 진행=3S를 잡고, 첫 text를 받은 시각부터 진행 구간을 잽니다. CI에서 isolationBudgetMs가 바닥을 올리면 세 시계가 같이 커지므로 순서가 무너지지 않습니다. 로컬에서는 바닥이 안 올라가서 짧게 유지됩니다.

테스트는 tests/ci-workflows/test-runner.test.ts에 캡처 헬퍼·미완 캡처·UTF-8 청크 분할·타임아웃/실패/성공 레인에서 출력이 한 번만 나가는지까지 새로 잠갔습니다. writers 주입으로 stdout/stderr를 가로채 검증하는 방식이라 실제 프로세스 출력을 더럽히지 않습니다. Cursor 쪽은 completedProgressSpan 단언으로 “안전 한도에 잘려서 끝난 것처럼 보이는 성공”을 거부합니다. 본문대로 이 누적 HEAD의 검증 게이트는 호스티드 CI이고, 소스 PR 헤드(#3924 e2416323, #3930 141077f7)의 26잡 초록은 참고 증거일 뿐입니다.

라인 scripts/test.ts · 예전 early return { exitCode: 124, output: "" } - 버그 재현이 명확합니다. 타임아웃 뒤에 스냅샷을 남기는 쪽은 맞습니다. 다만 타임아웃 경로에서 exitCode === null이면 incomplete 처리가 0→1로 바꾸지 못한 채 결국 124를 돌립니다. 동작은 의도대로지만, 로그에 incomplete 경고가 남아도 종료 코드만 보면 “잘림”과 “순수 타임아웃”을 구분하기 어렵습니다.

라인 scripts/test.ts · captured?.finish(1_000) - 자손 파이프 drain 상한 1초는 문서와 일치합니다. Windows에서 자손 수명이 길면 잘린 로그+실패로 갈 수 있는데, 그건 “초록 위장”보다 낫습니다. 다만 1초가 Windows 레인에서도 충분한지는 CI 초록으로만 확인됩니다(이 PR의 path filter상 windows 샤드는 스킵될 수 있음).

라인 cursor-stream-health.test.ts · silenceMs = isolationBudgetMs(1_000) - CI·풀스위트에서는 POSIX 바닥 5초가 되어 침묵 5s / heartbeat-only 10s / 진행 15s / fixtureLimit 약 22s / test timeout 최소 30s로 커집니다. 플레익을 줄이려는 트레이드오프이고, 로컬은 그대로 짧습니다. “제품 침묵 한도를 늘린다”가 아니라 “픽스처 예산만 로드에 맞춘다”는 점을 리뷰어가 혼동하지 않으면 됩니다.

경로/심볼 · #3924 / #3930 - 원본 PR이 아직 open입니다. 이 스택이 dev에 들어가면 leftover 원본은 Landed via #3940 at <commit> + landed-via-maintainer로 닫는 기존 규칙이 적용됩니다. 따로 리베이스해 올릴 필요는 없습니다.

경로/심볼 · CI - 이 wake 시점에는 resolve-pr / hygiene / label / Cross-platform CI changes / CodeRabbit이 아직 queued·pending이고, windows·macos-control·npm-global은 워크플로 스킵입니다. 머지 전에 이 HEAD의 초록만 보면 됩니다.

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

• 스택 fix(test): preserve lane output after timeouts and stabilize the Cursor stream-health watchdog #3940 한 방으로 넣을지, 아니면 fix(test): preserve captured output after lane timeouts #3924 → #3930을 따로 머지할지(파일 비겹침이라 어느 쪽이든 가능, CI 한 번이면 스택이 유리)
• 머지 후 fix(test): preserve captured output after lane timeouts #3924·#3930을 즉시 landed-via로 닫을지
• drain 1초·incomplete→실패 계약을 Windows에서도 “충분히 관대 / 너무 빡셈”으로 볼지(스킵된 windows 잡 때문에 이 PR만으로는 증거가 약함)
• Cursor 픽스처가 CI에서 30s급으로 길어지는 것을 스위트 예산상 받아들일지

너의 추천
이 PR HEAD CI가 초록이면 dev에 머지하세요. 그다음 #3924와 #3930에 Landed via #3940 at <merge-commit> 코멘트를 남기고 landed-via-maintainer로 닫으세요. 원본을 리베이스하거나 별도 랜딩할 필요는 없습니다. CI가 아직 도는 중이면 초록 전에 머지하지 마세요. types/config 분리와 무관하니 close-don't-rebase 대상이 아닙니다.

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

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-07T18:11:45.358031Z	8b81676	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[lidge-jun]

Maintainer integration record

I, @lidge-jun, exercise maintainer integration under MAINTAINERS.md (lines 59-64) for head b52182c24ddac9cb0608ab3c6df85e6401e53f0b.

Cross-platform CI run 34152136978 passed on this head, constructed from base ca381ea764cfbc63bec978f53eb58e96c00c0c64. All 16 expected jobs succeeded individually: select windows runner, changes, test 1/4 through test 4/4, storage policy, api usage, gates, macos 1/2, macos 2/2, keyring ubuntu, keyring windows, keyring macos, docker smoke, and the aggregate ci.

windows <n>/6, macos control and npm-global <os> were skipped by their workflow conditions — the first two are workflow_dispatch-only and the third is gated on the packaging path filter, which none of these four files match. They are recorded as skipped and are not counted as passing evidence. enforce-target, hygiene, resolve-pr, label and react-doctor are green on this head, CodeRabbit's status is success, the Codex review completed, there are no unresolved review threads, and no maintainer change request is outstanding.

Base advance, reviewed rather than re-run

The destination advanced to b3dec89bf30d0b1016e6cab028ca7963ada5b9d5 while CI ran. I reviewed that delta — #3942's Responses compatibility work and its application-level tests, the relocation of one test into the adapters domain, and #3943's devlog closure. It changes none of the four carried files, and none of the runner, capture, timeout, process-lifetime or Cursor watchdog contracts this patch depends on. I accept the remaining integration risk without another rebase.

The combined destination-plus-PR tree has not been executed before this merge. The run above proves the tested head, not the eventual squash tree. Landing verification will prove that all four carried files equal the tested head and every other path equals the inspected destination. The post-merge push run on dev will be recorded separately; a cancelled or absent run will not be reported as success.

Attribution and local checks

Both branch commits are authored by luvs01 <27862058+luvs01@users.noreply.github.com> and carry their cherry picked from commit lines referencing e2416323 and 141077f7. The squash commit body carries the matching Co-authored-by trailer, because the repository squashes on commit messages rather than the pull-request description.

Local suite, typecheck and build were NOT RUN by owner instruction for this delivery and are not claimed as passing. Head, destination SHA, reviews and permission were refreshed immediately before posting this record.

[lidge-jun]

Post-merge dev run: superseded, not failed

I said I would record the post-merge push run separately and would not report a cancelled run as success, so here it is.

Run 34153261501 on the landed squash 221617b80756f1be13a5db942cd80a3d8f79ab01 reports conclusion: cancelled. Reading the producers rather than the summary:

• Success: changes, select windows runner, test 1/4 through test 4/4, storage policy, api usage, gates, keyring ubuntu, keyring windows, keyring macos, docker smoke.
• Cancelled: macos 1/2, macos 2/2.
• Skipped by workflow: windows <n>/6, macos control, npm-global <os>.

The cause is the concurrency policy, not this change. ci.yml groups on cross-platform-ci-${{ github.ref }} with cancel-in-progress: true, so the next push to dev — #3941 landing as 6188458ae about ten minutes later — cancelled whatever was still running on the previous dev head. Both cancelled jobs are the long macOS lanes, which is exactly what that policy targets. The same thing happened to ca381ea76's push run earlier today for the same reason.

This is a superseded run, not a failure signal, and I am not claiming it as passing evidence either way. The pre-merge evidence stands on its own: run 34152136978 on the exact merged head b52182c24 had all 16 producers green, including both macOS lanes. The next full dev run to complete without being superseded will cover this content as part of the branch.