spinner-waiter: honor explicit action timeouts by mmkal · Pull Request #34 · iterate/middlewright

mmkal · 2026-08-14T12:00:43Z

Follow-up to #36 (popup-overlay) — its popup auto-wrap exposed the explicit-timeout bug fixed here.

1. Explicit timeouts are the author’s budget — stop overriding them with the 1ms fast-fail

iterate/iterate’s preview e2e went fully red on the first run with #33’s build: every mobile spec’s OAuth popup action failed with TimeoutError: locator.click: Timeout 1ms exceeded. Those specs pass { timeout: 15_000 } on popup actions precisely because the auth pages render no spinner-visible loading UI — and popups used to be raw pages where that timeout was honored. Once popups auto-wrapped, spinner-waiter’s no-spinner fast-fail replaced the author’s 15s budget with 1ms:

await popup.getByRole("button", { name: "Allow access" }).click({ timeout: 15_000 });
// → TimeoutError: locator.click: Timeout 1ms exceeded.

Now an explicitly passed timeout passes straight through — the per-action equivalent of settings.run({ disabled: true }).

2. Leave disappearance waits to Playwright

waitFor({ state: "detached" | "hidden" }) drives toward the target leaving the page. Spinner-waiter’s appear-oriented model does not apply, so these waits now pass through unchanged: satisfied waits resolve normally and failures use Playwright’s configured action timeout rather than the 1ms fast-fail.

Risk map

Explicit-timeout actions skip all spinner-waiter behavior. That is deliberate: the author owns the supplied budget.
Disappearance waits use vanilla Playwright behavior rather than spinner detection.
Review order: src/plugins/spinner-waiter.ts, then the three regression specs in spec/spinner-waiter.spec.ts.

Spinner-waiter suite green locally (13 passed).

Video

video-rendered.webm

🤖 Generated with Claude Code

Session id: 7b51d95e-5871-4a8f-b4c9-996a84e1beea

Note

Medium Risk
Middleware pass-through changes when explicit timeouts are set skip all spinner extension for those actions; disappearance waits bypass fast-fail—behavioral change in a widely used plugin path but aligned with author intent and downstream iterate usage.

Overview
Spinner-waiter no longer overrides actions when the test author sets an explicit { timeout } or uses waitFor({ state: "hidden" | "detached" }).

For explicit timeouts, middleware now passes straight to Playwright instead of applying the no-spinner 1ms fast-fail. That restores per-action budgets (e.g. OAuth popup clicks with { timeout: 15_000 }) that broke once popup auto-wrap routed those actions through spinner-waiter.

For disappearance waits, the appear-oriented spinner logic is skipped so those calls get normal Playwright timeouts—satisfied waits resolve, failures use the configured action timeout rather than 1ms.

spec/spinner-waiter.spec.ts adds three tests covering honored explicit timeout, exceeded explicit timeout messaging, and disappearance wait pass-through.

^{Reviewed by Cursor Bugbot for commit be26f5a. Bugbot is set up for automated code reviews on this repo. Configure here.}

…act gap Investigation findings: middleware plugins already support popups via a second addPlugins call (per-page dispatch with fall-through). videoMode needs work: a reused instance wipes the main timeline on beforeTest, and a fresh instance per popup collides on fixed artifact filenames in testInfo.outputDir. Spec for the intended behavior comes next. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Two tests against an auth-popup demo app. The first passes today: wrapping the popup with a second addPlugins call runs its actions through its own plugins, with timelines isolated in-memory. The second is an intended-behavior spec that fails today: after both pages finalize, each videoMode instance should own its artifacts, but both resolve the same fixed filenames in testInfo.outputDir so the last finalize clobbers the first. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Each videoMode() registered within a test now gets its own artifact namespace: the first instance keeps the legacy unsuffixed filenames, later ones get -2, -3, ... on metadata, raw/rendered video, player HTML, highlight/pan/fill images, dialog/final frames, .ass files, and attachment names. This lets an auth popup carry its own fresh videoMode instance without clobbering the main page's artifacts - Playwright screencasts each page separately, so separate videos per page is the natural model. Reusing one active instance on a second page now throws a clear error instead of silently wiping the first page's timeline in beforeTest. Specs: popup.spec.ts (artifact isolation now green, plus the reuse guard), popup-video.spec.ts (video: 'on' end to end - popup gets its own raw/rendered webm), shared demo app in auth-demo-app.ts. README gains a Popups section. Task moved to complete. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Used by the gitignored popup-demo spec that records the PR demo videos; the popup specs assert the same roles/text either way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Design settled interactively (plannotator grill): parent-owned composition, default-on auto-wrap with a forPopup plugin hook, unified piece timeline with source tags, 90%-fit dimmed overlay, double-wrap error instead of an escape hatch. Full decision log in the task file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Wrapped pages listen for the popup event and wrap the popup automatically (recursively for nested popups). Plugins may declare forPopup(ctx) to produce the popup's plugin - videoMode returns null for now (its parent-bound child recorder is phase 2) - and plugins without the hook are re-registered as-is, which is safe for the stateless ones. Opt out with popups: false; children dispose before the parent finalizes. addPlugins on an already-wrapped page now throws instead of silently replacing plugin state mid-flight - this also guards accidental double wrapping of main pages. Manual popup wrapping (the #32 pattern) becomes the popups: false path; specs migrated accordingly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

videoMode.forPopup now returns a parent-bound child recorder: popup actions record highlights on the parent clock (so child->parent time mapping is trivial), waits merge into the parent's unified dead air, and at finalize the popup recorder settles, closes the popup, calibrates recordingEndedAt, and copies the popup's raw screencast to video-raw-popup-N.webm. Facts land in a new children array in video-mode.json (schemaVersion 2). Grandchild popups recurse into the same flat children list. Rendering is untouched - the composite overlay is phase 3. The action middleware is extracted into videoModeActionMiddleware, shared between an instance (recording onto its own state) and its child recorders (recording onto a child state that shares the parent's clock and dead-air spans). Popup dialogs are not annotated yet - noted in the task file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Two-pass render when a test recorded popups. Pass A composites each popup screencast onto the parent's raw footage: dimmed backdrop (40% black), scaled to fit 90% of the frame (never upscaled), centered, alpha-faded in/out over 200ms, windowed to the popup's open/close span, stacked newest-on-top. The composite shares the parent raw timeline exactly, so pass B - the existing piece/hold/dead-air/cursor machinery - runs on it unchanged; holds freeze the composite, so it never matters which source triggered them. Popup highlights project into composite coordinates and join the render plan as plain box/pointer highlights (child-frame pixel treatments - pans, fill reveals, screenshot stills - drop away). Child raw time maps to the parent timeline via the settled-recorder calibration when the popup was closed by the recorder, or the screencast start approximation when the popup closed itself; the auth demo popup now self-closes after Approve like a real OAuth popup. Frame-sampling spec verifies the rendered output: frames with a dimmed page background and the popup's bright card centered above it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…spec Three compositing fixes found by eyeballing the rendered demo: - Static pages emit sparse screencast frames, and overlay emits output only at primary-input frame times. Resample BOTH chains to a continuous fps: without it the child frame that passed fade mid-ramp ghosts at partial alpha for the whole window, and the popup window can contain zero composite frames entirely (showing post-close footage). - Run the exit fade AFTER close, using the screencast's padded final frame: a self-closing popup otherwise puts its own Approve click - and the click hold's freeze frame - inside the fade by construction. - An instant click's source slice is a few ms wide, often between frame ticks, so the hold trim came up empty and bled the next piece's footage. Anchor the slice back from close and widen it to two frames. spec/popup-overlay-demo.spec.ts replaces the gitignored demo spec: full watchable treatment (pointer, captions, overlay), light assertions on the child span and rendered output. Overlay frame assertions calibrated against measured downscale blends. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The popup overlay now slides up from the bottom edge on enter (300ms, quadratic ease-out) and slides back down after close, alongside the alpha fade. Piece planning gains keepSpans: the overlapping-hold skip used to jump source footage straight across the popup's enter/exit animations, hard-cutting them out of the output; skips that would cross a popup transition are cancelled. Parent highlights that start inside a popup's exit window shift past it, so their holds can't freeze a mid-fade ghost (the flash after the popup disappeared). Fill reveals now work inside overlays: the typed-text reveal renders over a frozen composite frame (popup risen, field empty, backdrop intact) with the child screenshot's content rect scaled and positioned through the overlay transform, synced to cursor arrival like main-page fills. highlightCursorPoint projects fillReveal.initialRect through the overlay transform too - the I-beam used to land offset from the field. Child raw footage anchoring: a self-closing popup's screencast t=0 is its first captured frame, which lags the popup event by the initial paint, so footage played early (fields filled before the reveal). The raw video's padded end is the better anchor: closedAt + 1s minimum final-frame padding - duration, floored at openedAt. Demo: the popup is now a realistic sign-in form (username/password + Sign in) on indigo, over a teal app page so the dimmed backdrop reads; MIDDLEWRIGHT_DEBUG_PIECES=1 dumps the render piece plan. Includes previously staged demo/styling work reviewed via the local video (not the GitHub PR). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The reveal base predates the fill, so the field typed with no focus ring, and the ring then flashed in from live footage after the hold. At reveal start the field's ring region from the post-fill screenshot overlays the base, its text immediately covered by the pre-fill screenshot's empty content box, and the reveal bands type over that - ring appears when the cursor lands, letters arrive inside it, and the post-piece footage continues the ring seamlessly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Password inputs render one bullet per character, and the reveal only ever shows the screenshot's dots - so there was no reason to fall back to an instant fill. Measure bullet glyphs instead of the value's graphemes for the reveal stops. The reveal base now rewinds a single frame (a longer rewind could cross the previous fill's completion and wipe its value from the frozen frame), and the pre-fill empty content box covers the field from t=0 so anchor imprecision can't leak early-typed text. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…isappearance waits An explicitly passed { timeout } is the test author's owned budget — the per-action equivalent of settings.run({ disabled: true }). The middleware now passes such actions straight through instead of overriding them with the 1ms no-spinner fast-fail, which turned deliberate long waits into guaranteed failures the moment popup auto-wrap put previously-raw popup actions (timeouts and all) behind the middleware. Also upstreams the readiness-goal fix that iterate has carried as a dist patch (patches/middlewright.patch): waitFor({ state: 'detached' | 'hidden' }) drives toward the target LEAVING the page, so an already-gone target is the success-adjacent state — without this, a satisfied disappearance wait takes the 1ms fast-fail path. Includes the spinner-to-target handoff bridge from the same patch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

pkg-pr-new · 2026-08-14T12:05:20Z

Open in StackBlitz

pnpm add https://pkg.pr.new/middlewright@34

commit: be26f5a

Two spec-only changes to `specs/mobile/chat-titles.spec.ts` ([#2490](#2490)); no product changes. ### 1. Drop the picker tap the app already skips The spec failed every preview e2e run it appeared in (seen twice + retries on [#2498](#2498)): ``` strict mode violation: getByText('mobile-chat-titles-…') resolved to 2 elements: 1) <h1 …>mobile-chat-titles-…</h1> ← chat list header 2) <div …>→ /notes in mobile-chat-titles-…</div> ← note composer caption ``` Test identities auto-continue project selection and the app auto-opens the account's only project, so the spec's `page.getByText(projectSlug).click()` picker tap ran when the chat list was already up — where the slug appears twice. Deterministic, not flake. Fix: align the signup helper with `notes.spec.ts`'s proven flow — consent's "Allow access", then wait for "New chat". ### 2. middlewright#33: popups auto-wrap — popup timeout hacks removed [iterate/middlewright#33](iterate/middlewright#33) auto-wraps popups opened by wrapped pages, so the spinner/hydration waiters now cover the OAuth popup with zero test wiring. The spec drops its two popup-action `{ timeout: 15_000 }` escapes (which existed only because the popup was unwrapped). Timeouts justified by the auth app's missing loading UI are unchanged. **Note for merge:** `middlewright` is sha-pinned to the PR build (`https://pkg.pr.new/middlewright@eb34bdc`). Swap to the npm release once middlewright#33 lands — or merge as-is and bump later; the sha URL is immutable. Verified locally: 2/2 passes of the spec against a local dev server with the wrapped popup. ### 3. The real fix for the popup slowness: auth app loading states The popup timeouts all pointed at one product gap — the auth app goes loading-UI-less exactly where it's slowest. On a cold preview, "Allow access" is >1s away while the consent skeleton shows unmarked gray boxes, and the redirect transitions flip their pending labels ("Authorizing...", "Signing in...") back to idle text mid-navigation. Fixed in the auth app: - Both loading skeletons (consent, project-access) are now `role="status" aria-label="Loading"` — announced to assistive tech, and the e2e spinner-waiter extends its budget while they show. - `redirectAndStayPending` is now shared: consent and OTP sign-in keep their pending labels showing until the browser actually leaves ("Get started" already did this). - `email-otp-signup.ts` drops every manual timeout and its disabled-spinner-waiter block — the signup flow rides real product loading UI end to end. middlewright is repinned to the [#34](iterate/middlewright#34) build (`pkg.pr.new/middlewright@2f47229`, stacked on #33): explicit action timeouts pass through spinner-waiter (a genuine middlewright gap, kept), and the readiness-goal fix carried as `patches/middlewright.patch` is upstreamed — the patch is deleted here. ### Risk map - Riskiest: `redirectAndStayPending` in consent/OTP sign-in — a never-resolving promise held until unload. Same pattern project-access has shipped for a while; the mutation can no longer "succeed" visibly, which is the point. - Behavior change on merge: sign-in flows show persistent pending labels through redirects (previously flickered back to idle); loading skeletons are announced as status to screen readers. - Review order: `apps/auth/src/routes/_auth/consent.tsx` + `login.tsx` (redirect semantics), `project-access.tsx` (skeleton mark + helper extraction), `specs/test-support/email-otp-signup.ts` (timeout escapes deleted), `chat-titles.spec.ts`, generated/lockfile churn last. ### Video The spec end-to-end — including middlewright#33's popup overlay compositing (the OAuth popup renders as an overlay in the main page's video): https://github.com/user-attachments/assets/36135e5c-3a96-466b-83b9-0a426705910c 🤖 Generated with [Claude Code](https://claude.com/claude-code) Session id: `7b51d95e-5871-4a8f-b4c9-996a84e1beea`  --- > [!NOTE] > **Medium Risk** > Changes OAuth consent/login/project flows and pins middlewright to a PR tarball; behavior is UX-only but on the authentication path and affects CI e2e timing. > > **Overview** > Keeps auth OAuth/login flows visibly loading until navigation actually starts, and updates Playwright helpers to rely on **middlewright** instead of manual timeouts. > > **Auth app:** Introduces shared `redirectAndStayPending` so react-query mutations stay pending after `window.location` is set—buttons keep **"Authorizing..."**, **"Signing in..."**, and **"Creating..."** and stay disabled through slow server round trips (consent, login OTP success, project-access continue/create paths). Loading skeletons on consent and project-access now use `role="status"` and `aria-label="Loading"` so assistive tech and the e2e spinner-waiter can extend waits. > > **Tooling:** Replaces the local `middlewright` patch with `https://pkg.pr.new/middlewright@2f47229` and removes `patches/middlewright.patch` (upstream spinner-waiter fixes). Specs drop redundant popup timeouts and the disabled spinner-waiter block in email OTP signup; **chat-titles** stops tapping the project slug on the chat list (aligns with auto-open + avoids strict-mode duplicate slug matches). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 68f46b3. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>   ## Environment Config Lease [Login ↗](https://auth.iterate-preview-8.com/test-login?email=pr2500%2Btest%40nustom.com&project=pr2500&return_to=https%3A%2F%2Fos.iterate-preview-8.com%2Fapi%2Fiterate-auth%2Flogin)    <details> <summary>Slot: preview-8 | Doppler config: preview_8</summary> | app | status | commit | preview | size (gzip) | deploy duration | test duration | retries | cleanup duration | workflow run | updated | summary | | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | | Auth | deployed | `68f46b3` | [https://auth.iterate-preview-8.com](https://auth.iterate-preview-8.com) | 817.4 KiB | 24.9s | 18.2s | | | [Workflow run](https://github.com/iterate/iterate/actions/runs/274798385393991) | 2026-08-14T12:17:43.196Z | | | Docs | deployed | `68f46b3` | [https://docs-preview-8.iterate-dev-preview.workers.dev](https://docs-preview-8.iterate-dev-preview.workers.dev) | 866.2 KiB | 13.2s | 2.2s | | | [Workflow run](https://github.com/iterate/iterate/actions/runs/274798385393991) | 2026-08-14T12:17:27.225Z | | | Dummy Petshop | deployed | `68f46b3` | [https://dummy-petshop.iterate-preview-8.com](https://dummy-petshop.iterate-preview-8.com) | 198.3 KiB | 6.0s | 49.0s | | | [Workflow run](https://github.com/iterate/iterate/actions/runs/274798385393991) | 2026-08-14T12:18:14.007Z | | | OS | deployed | `68f46b3` | [https://os.iterate-preview-8.com](https://os.iterate-preview-8.com) | 5.11 MiB (-10.8 KiB vs main) | 60.9s | 235.5s | | | [Workflow run](https://github.com/iterate/iterate/actions/runs/274798385393991) | 2026-08-14T12:21:20.488Z | | | Semaphore | deployed | `68f46b3` | [https://semaphore.iterate-preview-8.com](https://semaphore.iterate-preview-8.com) | 441.3 KiB | 16.1s | 61.4s | | | [Workflow run](https://github.com/iterate/iterate/actions/runs/274798385393991) | 2026-08-14T12:18:26.478Z | | | Streams Example App | deployed | `68f46b3` | [https://streams.iterate-preview-8.com](https://streams.iterate-preview-8.com) | 1.53 MiB | 37.6s | 92.8s | | | [Workflow run](https://github.com/iterate/iterate/actions/runs/274798385393991) | 2026-08-14T12:18:57.867Z | | </details>   | Group | Lines | Significant | | --- | ---: | ---: | | Product | +39 -20 | +17 -11 🟩⬜⬜⬜⬜ | | Tests | +5 -4 | +2 -2 🟩⬜⬜⬜⬜ | | Config | +1 -2 | +1 -2 🟥⬜⬜⬜⬜ | | Other | +13 -241 | +7 -228 🟥🟥🟥🟥⬜ | | Generated | +56 -280 | +56 -264 🟩🟥🟥🟥🟥 | | **Total** | **+114 -547** | **+83 -507** 🟩🟥🟥🟥🟥 | <sub>Lines counts every changed line; Significant ignores blank lines, JS comments, and TypeScript lines with no runtime output. Between d90becf and 68f46b3, bucketed first-match-wins into the groups defined in `scripts/ci/loc-report.ts`.</sub>  --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Per review: waitFor({ state: detached|hidden }) waits for the target to leave, and spinner-waiter's appear-oriented model turns nonsensical when inverted for that (the visible spinner may be the very thing that's disappearing). Those waits are lint-discouraged in favor of positive waits; where one exists it now gets vanilla Playwright behavior via a plain pass-through instead of the inverted readiness goal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…it-timeouts

mmkal and others added 14 commits August 13, 2026 12:34

Style the auth demo app for watchable popup videos

7fe19b3

Used by the gitignored popup-demo spec that records the PR demo videos; the popup specs assert the same roles/text either way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Phase 4: popups README section, task completion

a05ce83

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

mmkal commented Aug 14, 2026

View reviewed changes

Comment thread src/plugins/spinner-waiter.ts Outdated

mmkal mentioned this pull request Aug 14, 2026

Fix chat-titles spec: drop the picker tap the app already skips iterate/iterate#2500

Merged

mmkal changed the base branch from popup-overlay to main August 14, 2026 13:42

mmkal changed the title ~~spinner-waiter: honor explicit action timeouts (stacked on #33)~~ spinner-waiter: honor explicit action timeouts Aug 14, 2026

mmkal added 2 commits August 14, 2026 14:43

Merge remote-tracking branch 'origin/main' into spinner-waiter-explic…

e5bb82e

…it-timeouts

Remove speculative spinner handoff grace

be26f5a

mmkal marked this pull request as ready for review August 14, 2026 13:49

mmkal merged commit d7594cf into main Aug 14, 2026
3 checks passed

Navigation Menu

Uh oh!

spinner-waiter: honor explicit action timeouts - #34

spinner-waiter: honor explicit action timeouts#34
mmkal merged 17 commits into
mainfrom
spinner-waiter-explicit-timeouts

mmkal commented Aug 14, 2026 •

edited

Loading

Uh oh!

pkg-pr-new Bot commented Aug 14, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

mmkal commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. Explicit timeouts are the author’s budget — stop overriding them with the 1ms fast-fail

2. Leave disappearance waits to Playwright

Risk map

Video

Uh oh!

pkg-pr-new Bot commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mmkal commented Aug 14, 2026 •

edited

Loading

pkg-pr-new Bot commented Aug 14, 2026 •

edited

Loading