Skip to content

[WRONG BRANCH] x - #3355

Closed
lidge-jun wants to merge 2 commits into
previewfrom
codex/promote-preview-2410-v3
Closed

[WRONG BRANCH] x#3355
lidge-jun wants to merge 2 commits into
previewfrom
codex/promote-preview-2410-v3

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 3, 2026

Copy link
Copy Markdown
Owner

y

Summary by CodeRabbit

  • Tests
    • Improved stream-health test coverage for frequent heartbeat and checkpoint activity.
    • Enhanced graceful-shutdown test diagnostics with clearer startup failure details and captured process output.
    • Adjusted startup timing allowances to better accommodate slower continuous-integration environments.

lidge-jun and others added 2 commits September 3, 2026 19:54
)

* test(shutdown): diagnose and budget the launcher startup wait

This test failed twice on the v2.41.0 promotion, on macOS and on shard 4/4,
both at exactly 20146-20222ms. That is the STARTUP deadline, not the shutdown
path the test is named for: the proxy never answered /healthz, so the signal
forwarding this test exists to guard never ran.

Locally the same spawn is healthy in ~800ms across all three signals, so the
budget was already 25x the observed cost. A shared CI runner building four
shards plus a macOS suite in parallel is a different machine, and it exceeded
it twice in one afternoon.

Two changes, and the first matters more than the second.

Launcher stdout/stderr are captured instead of discarded, and a startup failure
now reports whether the launcher exited, with what code and signal, and what it
printed. The old failure said only expect(up).toBe(true) — no log, no exit
code, no way to tell a slow runner from a real startup regression. That missing
evidence is why this took a local reproduction to classify at all.

The budget then goes to 60s on CI and stays at 20s locally. Raising it cannot
hide the regression the test guards: an orphaned proxy fails at step 4, where
the port never frees, under its own deadline. What a too-short startup budget
does hide is that distinction, by failing before the shutdown path runs.

Verification: bun test tests/shutdown-launcher.test.ts 3 pass 0 fail;
bun run typecheck exit 0.

* test(cursor): give the heartbeat-only watchdog room on a loaded runner

The heartbeat ping ran at 100ms against a 400ms silence threshold, so the test
had four ticks of margin. Miss three in a row and the SILENCE watchdog fires
first — a different error, and a failure report that reads as "the
heartbeat-only watchdog is broken" when the real story is a busy runner.

That is what happened on the v2.41.0 main promotion: the assertion wanted
"heartbeat-only" and got "no inbound frames for 1s before turnEnded". Locally
all five tests in this file pass.

Nothing about the behaviour under test needs a slow ping. The point is that
heartbeats reset the silence clock and do not reset the heartbeat-only clock,
and a 40ms interval exercises both with ten ticks of margin instead of four.

Verification: bun test tests/cursor-stream-health.test.ts 5 pass 0 fail.

---------

Co-authored-by: jun <jun@lidge.dev>
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 3, 2026 10:55
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 3, 2026

Copy link
Copy Markdown

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-03T10:59:02.197902Z 6534e28 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 github-actions Bot added the chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). label Sep 3, 2026
@github-actions github-actions Bot changed the title x [WRONG BRANCH] x Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • wrong target branch (preview); retarget to dev.

What to do

  • Retarget this PR to dev — all contributions go to dev.

Its title has been prefixed with [WRONG BRANCH].
This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.

@github-actions
github-actions Bot marked this pull request as draft September 3, 2026 10:55
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 5aec4c8b-1a24-48df-a71f-2dad9f8a10fb

📥 Commits

Reviewing files that changed from the base of the PR and between 491ffcf and 6534e28.

📒 Files selected for processing (2)
  • tests/cursor-stream-health.test.ts
  • tests/shutdown-launcher.test.ts

📝 Walkthrough

Walkthrough

The tests now use tighter cursor heartbeat timing and provide detailed launcher startup diagnostics. The launcher test captures child output and exit state, uses CI-aware startup budgets, and aligns its timeout with that budget.

Changes

Test Reliability and Diagnostics

Layer / File(s) Summary
Cursor watchdog timing assertions
tests/cursor-stream-health.test.ts
At lines 143–160, the heartbeat/checkpoint interval changes from 100ms to 40ms. Comments explain the timing margin. Lines 165–168 clarify that the "heartbeat-only" assertion identifies the watchdog that fired.
Launcher startup diagnostics
tests/shutdown-launcher.test.ts
Lines 70–84 add CI-aware startup budgets. Lines 110–120 capture stdout and stderr. Lines 133–151 track exit code and signal and report captured output when startup fails. Line 174 aligns the test timeout with the startup budget.

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

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/promote-preview-2410-v3

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.

@lidge-jun

Copy link
Copy Markdown
Owner Author

Duplicate; #3347 already tracks this branch.

@lidge-jun lidge-jun closed this Sep 3, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 54 / 80

이 PR은 제목이 [WRONG BRANCH] x, 본문이 y로 열려 있고, base는 preview, head는 codex/promote-preview-2410-v3입니다. 게이트 봇이 wrong-target으로 draft 처리했고 enforce-target도 실패했습니다. 겉모습만 보면 실수 PR처럼 보입니다. 그런데 들어 있는 diff는 장난이 아닙니다. tests/shutdown-launcher.test.tstests/cursor-stream-health.test.ts 두 파일이고, 내용은 방금 dev에 들어간 #3351(78c630a93)과 같습니다. CI에서 시작 대기가 20초에 걸려 시그널 검증까지 못 가던 문제(진단용 stdout/stderr + CI 60초 예산)와, heartbeat ping 100ms가 silence 400ms 예산에 먼저 먹히던 문제(40ms로 간격 축소)를 그대로 옮깁니다.

지금 dev HEAD에는 이미 #3351이 있습니다. preview 쪽 최신(491ffcf88, #3347로 승격된 promote 브랜치)에는 아직 78c630a93이 없습니다. 그래서 “preview에 flake 수리를 올리는 승격 PR”로 보면 방향은 muse_release_train WP3/WP4와 맞습니다. 반대로 “기여는 전부 dev로”라는 일반 게이트 규칙으로 보면 base=preview가 틀려 보입니다. 제목·본문이 placeholder인 점, 자동 draft·[WRONG BRANCH] 접두가 붙은 점을 보면, 의도적 승격인지 실수로 연 창인지 사람 확인이 필요합니다.

코드 자체는 #3351 리뷰에서 이미 본 것과 같습니다. STARTUP_BUDGET_MS를 CI에서만 키우고, 실패 시 런처 exit/code/signal/output을 Error에 넣고, heartbeat 간격을 줄여 silence watchdog이 먼저 울리지 않게 합니다. 제품 코드는 안 건드립니다. 다만 dev에 이미 랜딩된 커밋을 preview로 옮기는 형태라, dev에 대한 중복 머지 후보가 아니라 채널 승격 후보입니다. types.ts/config.ts 분리와도 무관합니다.

제목 [WRONG BRANCH] x / 본문 y - 리뷰·히스토리·릴리스 노트에 쓸 정보가 없다. 승격 PR이면 #3351 flake 수리를 preview에 올린다는 제목·요약으로 바꿔야 한다.

base preview + head codex/promote-preview-2410-v3 - 직전 #3347과 같은 promote 브랜치 패턴이다. 일반 기여 게이트는 dev를 강제하지만, 릴리스 열차 승격은 preview가 맞을 수 있다. 게이트 실패(enforce-target)는 그 긴장 때문이다.

tests/shutdown-launcher.test.ts STARTUP_BUDGET_MS / stdio pipe - #3351과 동일. process.env.CI truthy 검사 vs 레포 관례 === "true" 차이는 dev에 이미 들어간 상태 그대로다.

tests/cursor-stream-health.test.ts ping 10040 - #3351과 동일. silence 400ms 대비 여유 tick을 늘리는 변경이라 preview 승격에 실어도 제품 동작은 안 바뀐다.

draft + BLOCKED - 지금 상태로는 머지 불가. 승격이 맞다면 undraft·제목/본문 정리·게이트 예외(또는 maintainer override)가 필요하다.

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

  • 이게 v2.41.0 이후 preview 승격(#3347 다음 칸)인지, 아니면 실수로 연 placeholder PR인지
  • 승격이 맞다면 제목/본문을 #3351 요약으로 고치고 draft를 푼 뒤 preview에 랜딩할지
  • 실수라면 닫고, preview 승격은 제대로 된 promote PR로 다시 열지
  • #3354(dev → 2.42.0)와 순서 — 버전 bump는 dev 수리, 이 PR은 preview 채널이라 서로 막지 않음

너의 추천
의도적 preview 승격이면 제목·본문만 고치고 undraft한 뒤 #3351 내용을 preview에 랜딩. placeholder로 연 것이면 바로 닫기. 어느 쪽이든 지금 제목 x/본문 y 상태로는 머지하지 말 것. dev 쪽 flake 수리는 이미 #3351으로 끝났고, 급한 빨간불은 #3354 버전 bump 쪽이다.

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

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).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant