Skip to content

test(picker): de-flake both Windows switch-picker PTY tests - #3242

Merged
max-sixty merged 3 commits into
mainfrom
fix/flaky-alt-x-sticky-exit-code
Jun 26, 2026
Merged

test(picker): de-flake both Windows switch-picker PTY tests#3242
max-sixty merged 3 commits into
mainfrom
fix/flaky-alt-x-sticky-exit-code

Conversation

@worktrunk-bot

@worktrunk-bot worktrunk-bot commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

What

De-flakes both Windows switch_picker.rs PTY tests that intermittently blocked #3241's test (windows) job. Attempt 1 of that run failed on test_switch_picker_alt_x_keeps_cursor_sticky; attempt 2 failed on test_switch_picker_worktree_row_comments_tab_shows_thread. Both are addressed here.

Flake 1 — alt_x_keeps_cursor_sticky: racy process exit code

The test drives the interactive skim picker in a PTY, removes a row with alt-x, presses Enter, and asserted the wt process exited 0. On Windows it intermittently failed with exit code 1 even though the switch verifiably succeeded — the captured screen shows both the structured result and the success message:

{"action":"existing","branch":"wt-keep","path":"D:\\tmp\\.tmpgrsMhQ\\repo.wt-keep"}
○ Switched to worktree for wt-keep @ 'D:/tmp/.tmpgrsMhQ/repo.wt-keep'

The cursor-stickiness belief is fully verified by the screen assertions: the --format=json result reaches stdout only after the switch pipeline ran, and it names the sticky landing row. The exit_code == 0 pin coupled that belief to the interactive session's process exit code, a separate racy signal on Windows (this file's assert_valid_abort_exit_code already treats skim PTY exit codes as non-deterministic: 0/1/130). The fix keeps the screen-based success verification and accepts skim's selection/abort codes (0 or 1) instead of pinning to 0.

Flake 2 — comments_tab_shows_thread: cursor reset by async row decoration

The test moved the cursor onto feature with a single blind Down, then opened the HEAD± tab and waited for feature's diff --git. On Windows it timed out after 30s. The captured failing screen shows the > pointer stranded on main, with the preview reading "○ main has no uncommitted changes" — so the awaited diff --git never appeared:

> @ main       ? ^|                                      │○ main has no uncommitted changes
  + feature   !  ↑       +1        ↑1        +1   -1      │

Root cause: feature's row decorates asynchronously — the primed CI status resolves to "has PR" in the background. When that resolution lands it refreshes skim's item list, which resets the cursor to the top, undoing the Down. The preview re-issue nudge from #3238 then re-ran Alt-1 against main forever.

Fix: make Down cursor-confirmed. When a Up/Down arrow step carries expected content, it now names the target row, and the wait re-issues the (idempotent) arrow until the > pointer lands on that row and the screen settles — by which point the list has stopped refreshing. The step becomes ("\x1b[B", Some("feature")). This mirrors the alt-digit preview re-issue and reuses wait_for_cursor_on_row's pointer predicate (factored out as cursor_points_at). Arrow steps that pass None (every other test) are unaffected.

Verification

Both flakes are Windows-specific PTY races I could not reproduce on the Linux CI runner, so the fixes mirror this file's established de-flake patterns rather than root-causing platform exit-code / scheduling behavior. The full switch_picker suite (35 tests) passes locally on Linux, including both changed tests and every consumer of the shared helpers (wait_for_cursor_on_row, send_input_awaiting_content). The Windows CI on this PR exercises both on the affected platform.

Refs #3241

`test_switch_picker_alt_x_keeps_cursor_sticky` drives the interactive
skim picker in a PTY, removes a row with alt-x, presses Enter, and
asserted the process exited 0. On Windows this intermittently failed
with exit code 1 even though the switch verifiably succeeded — the
captured screen shows the `--format=json` result and the
"Switched to worktree for wt-keep" success message.

The test's actual belief — alt-x keeps the cursor on the slid-up row,
so Enter switches there rather than to the current worktree at the top
— is fully verified by the screen assertions (the JSON result is
emitted only after the switch pipeline ran, and it names the sticky
landing row). The exit-code-0 pin coupled that belief to the
interactive session's process exit code, which is a separate, racy
signal on Windows (this file's `assert_valid_abort_exit_code` already
treats skim PTY exit codes as non-deterministic: 0/1/130).

Accept skim's selection/abort codes (0 or 1) rather than pinning to 0.
A gross failure (panic, crash) still fails the screen assertions, which
require the success output to be present.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@max-sixty

Copy link
Copy Markdown
Owner

fix all the flakes please! (sorry for having introduced them)

…n feature

`test_switch_picker_worktree_row_comments_tab_shows_thread` drove the
picker with a single blind `Down` (`("\x1b[B", None)`) to move the cursor
onto `feature`, then Alt-1 and waited for `feature`'s `diff --git`. On
Windows CI it timed out after 30s — the captured screen showed the `>`
pointer stranded on `main` ("○ main has no uncommitted changes" in the
HEAD± preview), so the awaited `diff --git` never appeared.

Root cause: `feature`'s row decorates asynchronously (primed CI status
resolves to "has PR" in the background). When that resolution lands it
refreshes skim's item list, which resets the cursor to the top — undoing
the `Down` and leaving the pointer on the primary worktree. The existing
preview-reissue nudge then re-ran Alt-1 against `main` forever.

Make `Down` cursor-confirmed: when a Up/Down arrow step carries expected
content, treat it as the target row name and re-issue the (idempotent)
arrow via `wait_for_stable_until`'s nudge until the `>` pointer lands on
that row and the screen settles — by which point the list has stopped
refreshing. The comments-tab step becomes `("\x1b[B", Some("feature"))`.

This mirrors the alt-digit preview re-issue added in #3238 and reuses
`wait_for_cursor_on_row`'s pointer predicate (factored out as
`cursor_points_at`). Arrow steps that pass `None` (every other test) are
unaffected — they still fall through to a plain stability wait.

Refs #3241

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@worktrunk-bot worktrunk-bot changed the title test(picker): de-flake alt-x sticky-cursor test on Windows exit code test(picker): de-flake both Windows switch-picker PTY tests Jun 25, 2026
@worktrunk-bot

Copy link
Copy Markdown
Collaborator Author

Done — pushed a fix for the second flake so this PR now covers both Windows picker PTY tests that blocked #3241.

I pulled the failing run's logs to confirm the root causes from the captured screens rather than guess:

  • alt_x_keeps_cursor_sticky (attempt 1) — exited 1 despite a verifiably successful switch; the exit_code == 0 pin coupled the cursor-stickiness belief to skim's racy process exit code. Now accepts 0 or 1, with the screen assertions still proving success.
  • comments_tab_shows_thread (attempt 2) — the > pointer was stranded on main (preview read "○ main has no uncommitted changes"), so feature's diff --git never appeared. The single blind Down was undone when the row's async CI/PR decoration landed and reset skim's cursor to the top. The Down is now cursor-confirmed: it re-issues until the pointer lands on feature and the list settles.

Those were the only two failures across the run's attempts. The full switch_picker suite passes locally on Linux; the Windows leg here will exercise both on the affected platform. Monitoring CI now.

@worktrunk-bot

Copy link
Copy Markdown
Collaborator Author

✅ CI is green on 7fcb90b7 — both Windows picker PTY tests (alt_x_keeps_cursor_sticky and comments_tab_shows_thread) passed, along with the full test (windows/linux/macos) matrix, codecov/patch, and pre-commit.ci. The flakes that blocked #3241 are both addressed.

@worktrunk-bot

Copy link
Copy Markdown
Collaborator Author

This flake just recurred on main — the collect affected coverage (windows) job of run 28186340000 (on 438d33e, the merge commit of #3241) failed on test_switch_picker_worktree_row_comments_tab_shows_thread with the identical signature this PR diagnoses: timed out after 30s waiting for diff --git, with the > pointer stranded on main and the preview reading "○ main has no uncommitted changes" while feature's row decorated asynchronously.

>  @ main       ? ^|                                      │○ main has no uncommitted changes
   + feature   !  ↑       +1        ↑1        +1   -1      │

So this is reproducing on the default branch too, not just on #3241's PR CI — adding a data point for the cursor-reset root cause this PR fixes. No separate ci-fix PR; #3242 already covers it.

@max-sixty

Copy link
Copy Markdown
Owner

@worktrunk-bot resolve conflicts

Resolves the conflict in tests/integration_tests/switch_picker.rs. main
(#3247) added PreviewNotifier, which auto-repaints a late preview when its
background compute lands, and removed the test-side Alt-<digit> keystroke
re-issue that #3238 used to paper over stranded "Loading…" placeholders.

The two de-flakes in this PR are orthogonal to that:

- Flake #1 (alt-x sticky exit code) — a separate region, auto-merged cleanly.
- Flake #2 (comments-tab cursor reset) — kept. PreviewNotifier re-runs only
  the *preview* for the selected row (notify_row_changed); it does not stop
  on_update's request_render from resetting skim's cursor to the top after an
  async row decoration lands. So the cursor-confirmed Down is still needed.

Resolution keeps the cursor-arrow re-issue (restored wait_for_stable_until's
nudge param, now scoped to cursor recovery and renamed PREVIEW_REISSUE_INTERVAL
-> CURSOR_REISSUE_INTERVAL) and drops the obsolete Alt-<digit> re-issue branch
(is_alt_digit_tab deleted) in favor of main's PreviewNotifier path. Full
switch_picker suite passes locally on Linux.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@worktrunk-bot

Copy link
Copy Markdown
Collaborator Author

Resolved the conflict and pushed the merge (6d663db). The branch is now MERGEABLE.

The only conflict was in tests/integration_tests/switch_picker.rs, in send_input_awaiting_content. main's #3247 added PreviewNotifier (auto-repaints a late preview when its background compute lands) and removed the test-side Alt-<digit> keystroke re-issue that #3238 used to un-strand "Loading…" placeholders. The two de-flakes here are orthogonal to that, so I kept both:

  • Flake Add CI workflows and fix cross-platform test failures #1 (alt-x sticky exit code) — a separate region; auto-merged cleanly, no decision needed.
  • Flake Test CI fixes and improvements #2 (comments-tab cursor reset) — kept. PreviewNotifier re-runs only the preview for the selected row (notify_row_changed); it doesn't stop on_update's request_render from resetting skim's cursor to the top when an async row decoration lands. on_update is byte-identical between the merge base and main on that path, so the cursor reset the comments-tab test hits still happens — the cursor-confirmed Down is still needed.

So the resolution keeps the cursor-arrow re-issue (restored wait_for_stable_until's nudge param, now scoped to cursor recovery and renamed PREVIEW_REISSUE_INTERVALCURSOR_REISSUE_INTERVAL) and drops the now-obsolete Alt-<digit> re-issue branch (is_alt_digit_tab deleted), deferring to main's PreviewNotifier for late preview content.

The full switch_picker suite (29 tests under shell-integration-tests) passes locally on Linux, including both changed tests and every consumer of the shared helpers. Monitoring CI on the merge commit now — the Windows leg is where both flakes live.

@worktrunk-bot

Copy link
Copy Markdown
Collaborator Author

✅ CI is green on 6d663db43 (the merge commit) — test (windows) passed (11m56s) with both picker PTY flakes, alongside test (linux) / test (macos), codecov/patch, lint, and the advisory matrix. Branch is MERGEABLE; the conflict is resolved.

@max-sixty
max-sixty merged commit aedbd93 into main Jun 26, 2026
35 checks passed
@max-sixty
max-sixty deleted the fix/flaky-alt-x-sticky-exit-code branch June 26, 2026 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated-fix Automated CI fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants