Skip to content

test(machine-details): fix flaky "disarms once a save settles cleanly" (PP-78tm) - #2092

Merged
timothyfroehlich merged 2 commits into
mainfrom
claude/vibrant-faraday-t3hvdv
Sep 10, 2026
Merged

test(machine-details): fix flaky "disarms once a save settles cleanly" (PP-78tm)#2092
timothyfroehlich merged 2 commits into
mainfrom
claude/vibrant-faraday-t3hvdv

Conversation

@timothyfroehlich

@timothyfroehlich timothyfroehlich commented Sep 9, 2026

Copy link
Copy Markdown
Owner

What

Fixes the flaky unit test MachineDetailsForm > unsaved-changes navigation guard > disarms once a save settles cleanly (bead PP-78tm).

Why it flaked

The test waited only for the "Saved" note to render, then checked that the beforeunload guard had disarmed in a separate synchronous assertion right after the waitFor:

await waitFor(() => {
  expect(screen.getByTestId("details-dirty-note")).toHaveTextContent("Saved");
});
expect(beforeUnloadWasBlocked()).toBe(false); // sampled once, immediately

The "Saved" note (machine-details-form.tsx:443-447) and the beforeunload listener teardown (:159-168) both derive from the same isDirty false-flip, but the note text lands in the commit phase while the listener teardown lands in the passive-effect phase. On a loaded CI runner the synchronous check could sample the window where the note had settled but the guard had not yet unsubscribed — a false true. It reddened the required CI Gate, including on a main commit (run 33130774346, SHA d139b903, per the bead's 2026-08-29 analysis).

The fix

Assert the disarm inside the same waitFor as the "Saved" note, so the poll retries until both conditions hold together:

await waitFor(() => {
  expect(screen.getByTestId("details-dirty-note")).toHaveTextContent("Saved");
  expect(beforeUnloadWasBlocked()).toBe(false);
});

If the guard ever genuinely fails to disarm, beforeUnloadWasBlocked() stays true, the callback keeps throwing, and waitFor times out and fails — so the test still catches a real regression; it is not weakened. The later, deliberately synchronous queryByText("Discard unsaved changes?") assertion is load-bearing and left untouched.

beforeUnloadWasBlocked() dispatches a fresh cancelable beforeunload each poll; the handler is preventDefault-only, so repeated dispatch is harmless (noted inline).

Scope

One test file, +8/-2. No component change. Comment-only rationale added; it does not over-claim the exact CI mechanism (the bead records that the split-effect cause was never reproduced locally — this is defensive hardening around the correct RTL pattern, robust regardless of the true trigger).

Validation

  • pnpm exec vitest run <file>: 22/22 pass, across four reruns and once with --coverage (the CI differentiator).
  • typecheck / oxlint / prettier: clean (pre-commit hook green).
  • Reviewed adversarially across three lenses (correctness, bead-intent, maintainability); the one substantive finding — the comment over-committing to the unproven root cause — was folded in before this PR.

Opened unattended by the nightly bead session; carries ownerless.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DeLnU9YW1LtvSN9fpE5Y41


Generated by Claude Code

…" (PP-78tm)

The test waited only for the "Saved" note to appear, then checked the
beforeunload guard had disarmed in a separate synchronous assertion. The
note text and the listener teardown settle in different effect passes, so on
a loaded CI runner the check could sample the window where the note had
settled but the guard had not yet unsubscribed. The flake reddened the
required CI Gate, including on a main commit (run 33130774346).

Assert the disarm inside the same waitFor as the "Saved" note so the poll
retries until both hold together. The later synchronous
"Discard unsaved changes?" query is deliberately left synchronous — it is
load-bearing and unaffected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DeLnU9YW1LtvSN9fpE5Y41
@vercel

vercel Bot commented Sep 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
pin-point Ready Ready Preview Sep 9, 2026 12:35pm UTC

Request Review

@timothyfroehlich timothyfroehlich added the ownerless Opened by an unattended agent (routine, Dependabot, Renovate); needs pickup label Sep 9, 2026 — with Claude
@timothyfroehlich timothyfroehlich removed the ownerless Opened by an unattended agent (routine, Dependabot, Renovate); needs pickup label Sep 9, 2026
@timothyfroehlich

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 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-09T12:51:37.278294Z 76bbe44 Manual request
ℹ️ 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.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 76bbe44f31

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@timothyfroehlich timothyfroehlich added the ready-for-review PR passed CI and has no unresolved review comments label Sep 9, 2026
@timothyfroehlich
timothyfroehlich merged commit f54c7d6 into main Sep 10, 2026
22 checks passed
@timothyfroehlich
timothyfroehlich deleted the claude/vibrant-faraday-t3hvdv branch September 10, 2026 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review PR passed CI and has no unresolved review comments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants