镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

test: wait for dialog focus, reach a reading position, re-sign pair() - #1975

Merged
matt2e merged 1 commit into
mainfrom
flakey-tests-focus-history-unread
Oct 30, 2026
Merged

matt2e merged 1 commit into
mainfrom
flakey-tests-focus-history-unread

Conversation

@matt2e

@matt2e matt2e commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes three test flakes from the 29–30 Oct CI flake sweep:

  • Typing lost in a dialog that opens while a menu is still closing.
  • A history-loading journey that never left the bottom of the timeline.
  • A heavy-tailed signing loop in unread.test.ts.

No product code changes. This is one of six independent PRs split from the flakey-tests branch. Each is cut from main and can merge in any order.

Dialog typing lost after a menu (navigation-groups.spec.mjs:865, solo-chats.spec.mjs:243)

  • CI: main 41003205039 (job 123892389615, WebKit 10/12), where :917 got "" for "Retained launch". [5/5] Add protection to the Goose harness plugin #839 run 40964340233 (job 123765272909, WebKit 8/12), where "Create and move" stayed disabled with an empty name.
  • Cause: fill() focuses the field, then sends insertText as a separate call. If focus moves in between, WebKit drops the text without an error. Both traces show fill() returning with the value still "" on an input that did not remount. At that point the dialog's initial focus (layout effect, microtask, then rAF) is still pending, and NewWorkspaceDialog mounts while both Solo menus are still exiting. The traces do not show what moved focus in each run.
  • Change:
    • navigation-groups waits for toBeFocused() before fill() and checks toHaveValue() after, as navigation-group-icons:238–242 and the sibling test at :720 already do. The :917 check now tests only that the draft survives the failed refresh.
    • The three Solo New workspace flows (solo-chats :105 and :249, solo-rename :348) first wait for page.locator('[role="menu"]') to reach count 0, then toBeFocused(), fill() and toHaveValue(). The locator is CSS because the modal sets aria-hidden on the exiting menus, which hides them from role queries.
  • Verified: a temporary control (not committed) set initialFocus={false} on NewWorkspaceDialog and CreateSidebarSection. With it, every new barrier fails at toBeFocused() in both engines. Without it, all four tests pass in Chromium and WebKit. The WebKit race itself was not reproduced locally.

history-loading.spec.mjs:593 "a live message after a failed older load is new"

  • CI: 40980132056 (job 123818199988, WebKit 2/12) on feat(identity): the first-run Welcome flow replaces the key-setup gate (FOUNDATION: App.tsx) #1820, which touches only identity code and App.tsx. It timed out at :610 waiting for "1 new message".
  • Cause: in the trace, page.mouse.wheel(0, -100000) moved the reader only 41 px. loadNearTop starts an older page at any scrollTop below max(3000, 4 × clientHeight), so the quota-failed load and its "rate-limited" alert still appeared. updateJumpToLatest treats anything within 80 px as the bottom, though, so the latest-messages button never appeared. The test had taken the alert as proof that the reader had left the bottom. Why WebKit stopped at 41 px is not determined.
  • Change: after arming the quota, the test calls upper() from timeline.mjs instead of the hover and raw wheel. upper() requires each bounded gesture to make progress until the reader is more than 400 px above the bottom. The alert wait, settle, publish and assertion are unchanged. The test at :126 already establishes that the extra scrolling does not retry a quota error.
  • Verified: a temporary control (not committed) clamped the first upward wheel gesture to 40 px, as timeline-setup.spec.mjs:17–25 does. The old test then stopped 40 px from the bottom and failed at the latest() assertion in both engines. The new test ended 650 px above the bottom and passed in both. Without the control, it passes in both.

unread.test.ts "inbox describes a row by its newest message within the same second"

  • CI: 41010078970 (job 123913747149, JavaScript 1/3) on feat(github): unfurl PR and issue links inline with their title #1693, which touches no unread files. It failed with "Test timed out in 5000ms", reported after 16,678 ms.
  • Cause: pair() signed done once and re-signed only working until working.id > done.id. With done.id fixed, the chance of needing more than n tries is 1/(n+1), a heavy tail, and the test makes two pairs. The loop is synchronous, so Vitest reports the timeout only once it ends.
  • Change: every try re-signs both events, with ms tags 100+i and 900+i. Both keep created_at 20, and working's ms stays below done's, so Done still follows Working by its ms offset alone. Each try now succeeds with probability 1/2.
  • Verified: the file passes 150/150. A scratch script hashed the test's event shape with getEventHash. Over 1,000 pairs (twice) and 10,000 pairs, the old loop's maximum grew with the sample (818, 18,777 and 242,109 tries). The new loop's maximum stayed at 11–15, with no pair over 20 tries.

Overlaps

Browser coverage

No browser cases were added or removed. Four journeys gain focus and menu-unmount barriers before typing. history-loading:593 now reaches its reading position through the shared upper() helper instead of one raw wheel.

Verification scope

tsc --noEmit and biome check ran on the five changed files. Playwright ran only the five touched tests, one single-worker process at a time: Chromium, then WebKit (5/5 each). The controls ran the same way and were reverted.

Remaining gaps

  • Hosted CI has not run yet.
  • A green run does not show that these flakes are gone.

🤖 Generated with Claude Code

These are units 1-3 of section 3 of the 29-30 Oct CI flake sweep note
(sections 1a, 1b and 1c). Before editing, I re-checked origin/main
(36a283b, two commits past 561265d) and the 175 open PRs. No main
commit since this branch's base touches these files. Eighteen open PRs
touch these specs, unread.test.ts or the Solo dialog and focus code,
but none of their hunks reach these tests or pair(). #1942 edits
ui/finalFocus.ts, but only for an Escape race. #1963
and #1956 edit ChannelTimeline.tsx, but neither touches
updateJumpToLatest or loadNearTop. Nothing was dropped as covered. No
product code changes.

Dialog typing lost after a menu (section 1a)
- CI: navigation-groups.spec.mjs:865 "Create new retains its draft..."
  on main, 41003205039 (job 123892389615, WebKit 10/12, @71668588d),
  where :917 got "" for "Retained launch". solo-chats.spec.mjs:243
  "dragging a chat between a project and Chats..." on #839,
  40964340233 (job 123765272909, WebKit 8/12): "Create and move" stayed
  disabled with an empty name.
- Cause: fill() focuses the field in the page, then sends insertText
  as a separate call. If focus moves in between, WebKit drops the text
  without an error. Both traces show fill() returning with the value
  still "" on an input that did not remount. The dialog's initial focus
  (layout effect, microtask, then rAF) is still pending at that point,
  and NewWorkspaceDialog mounts while both Solo menus are still exiting.
  The traces do not show what moved focus in each run. The Solo menus'
  focus returns cannot move it off the field: the actions menu's
  explicit return goes through useFinalFocusUnlessMoved
  (ui/finalFocus.ts), the Workspace submenu keeps Base UI's default,
  and both skip once focus is outside the closing popups. A return can
  only land before the field is focused, and the dialog's initial focus
  then wins.
- Change: navigation-groups waits for toBeFocused() before fill() and
  checks toHaveValue() after, as navigation-group-icons:238-242 and the
  sibling test at :720 already do. The :917 check now tests only that
  the draft survives the failed refresh. The three Solo New workspace
  flows (solo-chats :105 and :249, solo-rename :348) first wait for
  page.locator('[role="menu"]') to reach count 0, then toBeFocused(),
  fill() and toHaveValue(). The locator is CSS because the modal sets
  aria-hidden on the exiting menus, which hides them from role queries.
- Verified: with initialFocus={false} on NewWorkspaceDialog and
  CreateSidebarSection, a temporary control that is not committed,
  every new barrier fails at toBeFocused() in both engines
  (navigation-groups:902, solo-chats:124 and :294, solo-rename:362). In
  the Solo tests the menu-count barrier passed first. Without the
  control, all four tests pass in Chromium and WebKit. The Solo focus
  barrier held after the menus unmounted, so this run gives no local
  sign of focus leaving the open dialog (the SoloWorkspaces product
  defect the note names as the alternative). The WebKit race itself was
  not reproduced locally.

history-loading.spec.mjs:593 "a live message after a failed older load
is new" (section 1b)
- CI: 40980132056 (job 123818199988, WebKit 2/12) on #1820, which
  touches only identity code and App.tsx. It timed out at :610 waiting
  for "1 new message".
- Cause: in the trace, page.mouse.wheel(0, -100000) moved the reader
  only 41 px. loadNearTop starts an older page at any scrollTop below
  max(3000, 4 x clientHeight), so 41 px still started the quota-failed
  load and its "rate-limited" alert. updateJumpToLatest treats anything
  within 80 px as the bottom, so the latest-messages button never
  appeared. The test took the alert as proof the reader had left the
  bottom. Why WebKit stopped at 41 px is not determined.
- Change: after arming the quota, the test calls upper() from
  timeline.mjs in place of the hover and raw wheel. upper() requires
  each bounded gesture to make progress until the reader is more than
  400 px above the bottom. The alert wait, settle, publish and assertion
  are unchanged. The test at :126 already establishes that the extra
  scrolling does not retry a quota error.
- Verified: a temporary control, not committed, clamped the first upward
  wheel gesture to 40 px, as timeline-setup.spec.mjs:17-25 does. The old
  test then showed the alert, stopped 40 px from the bottom and failed
  at the latest() assertion with "element(s) not found", in both
  engines. The new test took gestures [1, -650 (clamped to -40), -610],
  ended 650 px above the bottom and passed in both. Without the control
  it passes in both.

unread.test.ts "inbox describes a row by its newest message within the
same second" (section 1c)
- CI: 41010078970 (job 123913747149, JavaScript 1/3) on #1693, which
  touches no unread files: "Test timed out in 5000ms", reported after
  16,678 ms.
- Cause: pair() signed done once and re-signed only working until
  working.id > done.id. With done.id fixed, P(more than n tries) =
  1/(n+1), a heavy tail, and the test makes two pairs. The loop is
  synchronous, so Vitest reports the timeout only once it ends.
- Change: every try re-signs both events, with ms tags 100+i and 900+i.
  Both keep created_at 20, and working's ms stays below done's, so
  Done still follows Working by its ms offset alone. Each try then
  succeeds with probability 1/2.
- Verified: the file passes 150/150. A scratch script, not committed,
  hashed the test's event shape with nostr-tools getEventHash. Over
  1,000 pairs (twice) and 10,000 pairs, the old loop's maximum grew
  with the sample (818, 18,777 and 242,109 tries; p99 79-151). The new
  loop's maximum stayed at 11-15 (p99 7), with no pair over 20 tries.

Checks: tsc --noEmit, and biome check on the five changed files.
Playwright ran only the five touched tests, in one single-worker process
at a time: Chromium, then WebKit (5/5 each). The controls ran the same
way and were reverted; the spec and product files match this commit.
Not pushed. Hosted CI is the real check, and a green run will not show
that these flakes are gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e
matt2e requested a review from a team as a code owner October 30, 2026 11:56

@kazemek21 kazemek21 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kyle reviewed this PR and the Nexus review-tracker notes. Approved with no comments.

Posted by Codex (gpt-6.1-sol) on behalf of kylez.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Star Lord’s automated source review via Wes’s account, integrating Rocket, Mantis and Groot coverage.

No actionable findings in the five-file test-only change. The focus barriers retain draft/recovery assertions, the shared upper() helper establishes an above-bottom reading position before the live-message check, and re-signing both events preserves the test’s opposing ID/millisecond ordering. No production code or browser cases were removed.

Reviewed head 037888efe392dfd09a669c8652ea8ec324727a18 against base 36a283b2386473130430890faa3b06c6e45d4a6b, using verified Git blobs. The public description and commit metadata were inspected; there are no attached images and no new concrete publication-surface finding.

Hosted CI run 41051678204 succeeded on merge 8ab1d45dcdd79400d5d535327114b21604317f78, whose tree matches the reviewed head. The changed history case passed ten advisory repetitions in each browser engine; the full unread file passed 150 tests. Counts, elapsed times and slowest-case evidence were inspected, but no equivalent base timing comparison was obtained.

Source-only review: no local tests, builds, app launches or PR-code execution. The author’s temporary failure controls/statistical experiments were not independently reproduced; the original WebKit input cause, elimination of flakes and before/after cost improvement remain unproven. This COMMENT is feedback, not approval or merge authorization.

@matt2e
matt2e merged commit c775bef into main Oct 30, 2026
46 checks passed
@matt2e
matt2e deleted the flakey-tests-focus-history-unread branch October 30, 2026 23:38
shellz-n-stuff added a commit that referenced this pull request Oct 31, 2026
Picks up the month-boundary StatusEditor test fix (#2064).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* origin/main: (56 commits)
  perf(sidebar): subscribe to selected live and list facts (#1966)
  feat(desktop): align the sign-in callback page with web branding (#2038)
  Show linked PRs in Solo headers and move idle huddles to Chat actions (#1968)
  feat(messages): mark edited messages with "(edited)" (#2020)
  feat(desktop): bundle the buzz CLI in every package (#2027)
  feat(plugins): open a plugin page from buzz://plugin links (#2053)
  feat(plugins): managed plugin sources (#2052)
  test(browser): prevent sampled clock pause races (#1955)
  test: fix month-boundary and Sonner mount races in StatusEditor and RelayStaff tests (#2064)
  test: stop tests leaving temp dirs and a flake-check fixture behind (#1977)
  fix(messages): release Virtua's held range when scrolling stops (#2032)
  test(mcp): allow protection checks in bundled help fixture (#2061)
  fix(emoji): recover transient image failures without reopening chats (#2000)
  test: wait for dialog focus, reach a reading position, re-sign pair() (#1975)
  feat(plugins): sign builderlab in through the native fi session in fi builds (#1461)
  feat(agents): offer bundled Buzz help (#2033)
  fix(github): show pull request status for anchors and subpages (#2055)
  fix(dialogs): focus safe controls and task inputs (#2021)
  fix(composer): truncate workspace names at narrow widths (#2043)
  AIDA-2287: Refresh public channel search incrementally (#2023)
  ...

Signed-off-by: Alex Rosenzweig <arosenzweig@squareup.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants