Settle the notepad prompt's answer inside act so the kill's removal timer wins - #585
Draft
dormouse-bot wants to merge 2 commits into
Draft
Settle the notepad prompt's answer inside act so the kill's removal timer wins#585dormouse-bot wants to merge 2 commits into
dormouse-bot wants to merge 2 commits into
Conversation
Deploying mouseterm with
|
| Latest commit: |
c1a5ec4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://84f7edd6.mouseterm.pages.dev |
| Branch Preview URL: | https://fix-ci-34088776809.mouseterm.pages.dev |
dormouse-bot
commented
Sep 7, 2026
dormouse-bot
left a comment
Collaborator
Author
There was a problem hiding this comment.
Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.
The helper change itself traces clean: the Close anyway chain to killPaneImmediately is microtask-only (helperRefusal → helperHasWork, then removeSurface/killPaneImmediately), so the async act drains it and the setTimeout(…, lath.exitMs) really is registered before flush() registers its own. All five call sites are awaited, and the shape now matches clickHeaderKill right above it.
One inaccuracy in the new doc comment — inline.
`Keep open` is `onKeepOpen={shiftArchiveFailure}` — a synchronous queue
shift with no `closeSurface` call and no removal timer — so only
`Close anyway` depends on the act/flush ordering the helper enforces.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Wall.test.tsx > Wall on the Lath engine > Close anyway discards the notes and removes the Surface without a batchfailed onmainin run 34088776809:expected <div data-lath-leaf="pane-a" …> to be null. The received element carriedopacity: 0; width: 0px; height: 0px; z-index: 35; pointer-events: none— a leaf still mid-fade in the animator's dying band, not a leaf that failed to close. The same commit's test file is unchanged by #584, and CI on the next commit (cbb1e9d0) went green, so this is an intermittent ordering race, not a regression.The race is between two
setTimeout(0)s.clickButtonclicked inside a synchronousact, so the async closure the answer starts (closeSurface→helperRefusal→killPaneImmediately) only ran while the test was already awaitingflush().flush()registers its ownsetTimeout(0)first; the two-phase kill's deferred removal —setTimeout(…, lath.exitMs), which is0here because the suite forces reduced motion — is registered second, during the microtask drain. Node clamps both to 1 ms and runs them in registration order, so whether the removal fires beforeflush()resolves depends on whether it has expired by the time the timers phase runs. Usually it has; under CI timing it didn't.The helper's other three call sites share the shape —
clickButton('Close anyway'); await flush();at Wall.test.tsx:1979 asserts the same removal onpane-b— and one sibling test already worked around it with a doubledawait flush().Solution
Fix the helper rather than the one assertion:
clickButtonis nowasync, clicks insideawait act(async …), and then flushes. The click's async chain now completes inside the act, so the removal timer is registered beforeflush()registers its own and is guaranteed to land first — the ordering the test relied on by luck is now enforced. Verified with a probe: the leaf is already gone when the act returns.All five call sites become
await clickButton(...), and the now-redundant trailingawait flush()calls — including the doubled pair that was papering over this — are removed.Test-only change; no spec covers it.
Testing
pnpm testinlib/(typecheck + full vitest run): 175 files, 2788 tests passed.vitest run src/components/Wall.test.tsx×3 after the change: 56/56 each.Automated fix for failed run