Skip to content

Flaky CI: useUnsavedChangesGuard double-proceeds, crashing GeneralTab with 'Invalid blocker state transition: unblocked -> proceeding' #5513

Description

@atomantic

Problem

client/src/components/settings/GeneralTab.test.jsx > GeneralTab unsaved changes > resets the draft when Settings preserves General across a stale tab route intermittently fails in the Client tests and build CI job:

Error handled by React Router default ErrorBoundary: Error: Invalid blocker state transition: unblocked -> proceeding
The above error occurred in the <GeneralTab> component.
TestingLibraryElementError: Unable to find a label with the text of: Timezone (IANA)

The second error is fallout — the router error boundary has already torn the tree down, so the assertion at GeneralTab.test.jsx:171 has nothing to query. Passes reliably locally (cd client && npx vitest run src/components/settings/GeneralTab.test.jsx → 11/11). Observed on PR #5502's CI (run 33354555793, attempt 1); the PR does not touch any client file.

Cause

client/src/hooks/useUnsavedChangesGuard.js has two paths that can both call proceed() for the same parked navigation:

  1. The auto-proceed effectuseEffect(() => { if (!isDirty && blocker.state === 'blocked') blocker.proceed(); }, [isDirty, blocker]), which exists so a Save-while-parked still runs the navigation the user asked for.
  2. The caller's discard handler — resets its draft (which flips isDirty false) and then calls the returned proceed().

The Discard click does both in one commit: the handler proceeds synchronously, and the effect then runs holding the blocker object from the render where state was still 'blocked'. It re-calls proceed() on a blocker react-router has already moved to unblocked, and react-router throws. The optional-call guard on the returned proceed (blocker.proceed?.()) does not help — the stale object still carries a proceed function; only its state is stale. Whether the effect wins that race depends on scheduling, which is why it only shows up under CI load.

Every caller of this hook shares the exposure (SongBookViewer, PipelineManuscriptEditor, writers-room WorkEditor, GeneralTab), so the fix belongs in the hook, not in one call site.

Fix

Make proceeding idempotent per parked navigation by remembering which blocker instance has already been proceeded, and route both paths through it:

const proceededRef = useRef(null);
const proceedOnce = useCallback(() => {
  if (blocker.state !== 'blocked' || proceededRef.current === blocker) return;
  proceededRef.current = blocker;
  blocker.proceed();
}, [blocker]);

useEffect(() => { if (!isDirty) proceedOnce(); }, [isDirty, proceedOnce]);

and return proceedOnce as proceed. React Router hands back a fresh blocker object per state change, so identity is a sound key for "this parked navigation was already released". reset keeps its existing blocker.reset?.() shape.

Verification

  • cd client && npx vitest run src/components/settings/GeneralTab.test.jsx — 11 tests still pass.
  • Add a regression test that clicks Discard and asserts no error boundary is rendered, driving both paths in one commit (reset draft + proceed) so a re-introduced double-proceed fails the suite rather than only flaking.
  • Run the suites for the other callers (SongBookViewer, PipelineManuscriptEditor, writers-room WorkEditor) to confirm the auto-proceed-on-save behavior is unchanged.

Metadata

Metadata

Assignees

Labels

area:uiUI components and stylingbugSomething isn't workingeffort:mediumEffort: mediumin-progressClaimed and being workedmodel:mediumModel size: mediumtestsTest suite / test infrastructure

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions