test(chat): the pending-row regression waits for its new connection's own input-ready

[devswha]

Change

scripts/ui-regression.ts raced the page in the "sent before the snapshot" step and failed about one run in five with Timed out: the message sent before the snapshot is a live pending row (CI runs 37631750944, 37675014452).

The step closes the routed socket, holds the reconnect's snapshot, waits for pendingReady(pendingPane), sends, then releases the snapshot. pendingReady read inputStates, which was keyed by pane only. The server sends its snapshot as soon as the socket opens, and that can be before the page has sent its attach. In that order the held-snapshot wait finished while inputStates still held the closed connection's last input-ready, so pendingReady resolved at once. The message then reached the bridge before the new attachment was ready. The server answered input_not_ready, and the composer kept the draft with "Not sent: The pane is not attached and ready". So no pending row appeared.

An instrumented copy of the script (frame log, not committed) caught the failing order:

85771 route: snapshot held
85771 pendingReady resolved {"ready":true,...}      <- the old connection's state
85772 ws#2 -> attach
85807 route: snapshot released
85809 ws#2 -> submit (delivery: queue)
85820 ws#2 <- submit-result ok:false input_not_ready
86051 ws#2 <- input-ready                          <- the new connection is ready only here

In passing runs, attach goes out before the snapshot is held, and pendingReady waits for the new connection's input-ready.

The product behaves as intended: a send before the pane's attachment is ready is refused and the draft stays (never queued while offline). So the fix is in the test. Each input state now records the Playwright WebSocket that reported it, and a pane counts as ready only on the page's current, open connection. After the close, the step also waits for the page to open its new connection. There are no new timeouts or sleeps.

Validation

On a herdr of the run's own (bun run check run bun scripts/ui-regression.ts):

• Before (main at 407c1a1): 2 of 14 normal runs failed with this exact assertion (1 of 6 plain, 1 of 8 with the frame log above). Widened race, 3 of 3 failed with the same assertion. The widening is a scratch copy, not committed: in the page, the reconnect's open handler (which sends the attach) is stalled 300 ms, and the route holds that connection's frames to the server, attach first and in order, for 1 s. That is the failing order above, made certain.
• After (2868996): the same widened race, 3 of 3 passed; 10 of 10 normal runs passed.
• bun run check fast: passed (1699 pass, 4 skip, 0 fail). It does not cover scripts/, which the runs above exercise.
• CI on this PR: Fast checks and Integration and browser passed.

No CHANGELOG entry: the change is test-only.

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

• Configuration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: b9ab363c-31e6-4a7c-97c2-f1973a607a12

📥 Commits

Reviewing files that changed from the base of the PR and between 2868996 and 6d5f24d.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 937d3c27-1cab-4553-b931-bfd241274b3c

📥 Commits

Reviewing files that changed from the base of the PR and between 7d06b8e and 2868996.

📒 Files selected for processing (1)

• scripts/ui-regression.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

---

📝 Summary

Summary by CodeRabbit

• Tests
  • Improved UI regression coverage for reconnects by verifying that pending attachment remains stable only after the latest WebSocket is open and readiness has been reported for at least 500 ms.

Walkthrough

The UI regression script now tracks which WebSocket reported pane readiness. It requires readiness from the latest open socket to remain stable for 500 ms. The reconnect test waits for a replacement socket to open before checking readiness.

Changes

Socket-scoped readiness

Layer / File(s)	Summary
Readiness and reconnect check scripts/ui-regression.ts	Readiness is tied to its reporting socket, and pending readiness requires the latest socket to remain open for 500 ms. The reconnect test waits for a replacement socket to open while the snapshot is held.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Suggested reviewers: wouldu-pres

Merge Risk: ⚪ Minimal · up to 28689

No concrete current-head defect is established that should block this test-only change; proceed with normal checks.

Architecture Summary

Architecture risk: 🔵 Low · up to 28689

The change affects 1 system.

Changed systems: scripts

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — scripts (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in scripts/ui-regression.ts: Readiness state now records its reporting socket. pendingReady requires the pane to be ready on the latest, open socket for at least 500 ms; received readiness updates and sent attach/detach frames record or reset that socket-bound state. Previously, readiness was not tied to a connection and could be satisfied by a stale connection.
• observed — Modified behavior in scripts/ui-regression.ts: The reconnect test now records the socket count before closing the connection and, while the snapshot is held, waits for the page to open a replacement socket. Previously, it proceeded from the held snapshot directly to checking readiness.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the test regression and the fix: waiting for input readiness from the new connection.
Description check	✅ Passed	The description includes complete Change and Validation sections, explains the race, documents the fix, and reports reproducible test results. It does not include the template-requested screenshot or …

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[devswha]

Independent review (merge-own lane).

Classification: test fix for the flaky required browser lane. scripts/ui-regression.ts:604 ("the message sent before the snapshot is a live pending row") failed about one run in five, including on #574 today (run 37675014452), so every merge paid for it.

What I checked:

• The race is real. The server sends snapshot as soon as a socket opens (server/AGENTS.md: "On open every client immediately receives snapshot"), which can come before the page's attach. inputStates was keyed by pane only, so the closed connection's last input-ready satisfied pendingReady, and the send reached the bridge before the new attachment was ready (input_not_ready, draft kept, no row). The PR's frame log shows exactly that order.
• The new wait cannot pass vacuously. A pane counts as ready only when the newest socket (sockets.at(-1)) reported ready: true, is still open, and has held that for at least 500 ms. The app opens one kind of websocket (src/lib/ws.ts:83), so sockets.at(-1) is the page's current connection. If some other socket were newest, the wait would time out loudly rather than pass. The four other pendingReady callers run on a single live connection, so the stricter check holds for them too. The added wait for the page's new connection uses until, so it is bounded; there are no new sleeps.
• No product code changed. A send before the attachment is ready is still refused, and the draft stays (never queued while offline).

Evidence (fix-flake's runs, in the PR body): before, 2 of 14 normal runs failed with this assertion, and the widened race failed 3 of 3. After, the widened race passed 3 of 3 and normal runs 10 of 10. My own run on the merged head 6d5f24d (bun run build, then bun run check run bun scripts/ui-regression.ts): all 80 checks PASS. Updated with main before merging; Fast checks and Integration and browser are green on the merged head. There are no review threads.