Repository navigation
test(browser): prevent sampled clock pause races - #1955
Conversation
Freeze the captured timestamp during the pause handoff, restore Date advancement, and preserve pending timer deadlines and each caller’s existing virtual offset. Keep current main’s Solo focus and clock fixes. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@feedback.communities.buzz.xyz> Signed-off-by: Taylor Ho <[email protected]>
kalvinnchau
left a comment
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 One integration follow-up: migrate the sampled-clock caller added on main after this branch point.
kalvinnchau
left a comment
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 Approved. The P3 sampled-clock migration follow-up remains nonblocking.
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, integrating Rocket, Mantis and Groot coverage.
No actionable findings in the reviewed diff. The shared clock helper anchors Date during the pause handoff without reinstalling the clock; pending timer deadlines remain on monotonic ticks. Reviewed all 16 migrated call sites across nine existing spec files, including permission/focus, avatar retry, roster recovery, sharing readback and reading-dwell boundaries. The added browser regression checks the stale sample, remaining timer deadline and Date advancement. One browser case added, none removed; real browser clock/protocol behavior justifies this layer.
Reviewed head: 427d488b94211e046449112b291429927ecda390
Base: b0f1719b92ceafa779aa4543eb51cf4ea4f3a605
Validation limits: source-only review; no tests, app or PR code executed by this review team. A one-time snapshot of hosted CI is successful for this head (synthetic merge 13108c9e0b8f87b6b57d0027bac2f1539e80bd8b). Inspected shard-4 advisory artifacts show two passing clock-regression executions per engine with no retries: Chromium 216–280 ms, WebKit 403–628 ms. Those artifacts are a bounded sample, not the full repetition inventory or a before/after cost comparison. Live protocol latency, native behavior and human acceptance were not independently exercised. Public PR description and commit metadata inspected; no attached images or concrete disclosure finding. This COMMENT is not an approval or merge authorization.
Preserve current main behavior and migrate the additional live recovery pause to the shared clock helper without changing its 1000 ms virtual lead or assertions. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@feedback.communities.buzz.xyz> Signed-off-by: Taylor Ho <[email protected]>
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account.
No new actionable findings in the integration at head 5e54e36b4328c53ae189b39da516d6b786c696f1, base 8fd0d34ae55e565e75cbc1fd31bc97897b0385be. The original clock helper and regression remain unchanged from the earlier reviewed revision. The merge adds the helper call to channel recovery at tests/browser/live.spec.mjs:145, before failure injection, while preserving the virtual offset, cooldown steps, retry assertions and subsequent clock resume.
Source-only follow-up: no PR code or tests were executed. The author’s reported eight-case local run was not independently reproduced; current-head CI was incomplete in the inspected snapshot. Run 41144457486 had an earlier failed JavaScript shard 2/3 and WebKit shard 3/14, with their reruns still in progress when inspected. The saved WebKit report shows text movement at message-actions.spec.mjs:101, outside the changed files; its cause and the JavaScript failure remain unresolved. Selected timing artifacts were inspected, but they do not establish a comparable before/after cost result. Runtime focus, full-suite completion, before/after test cost and human acceptance remain unverified. This comment is not approval or merge authorization.
Include the merged MCP help fix while preserving the existing browser clock changes. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@feedback.communities.buzz.xyz> Signed-off-by: Taylor Ho <[email protected]>
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account.
No new actionable findings in this merge-only follow-up. Reviewed head 29b6385e50720d8ba686ab19d38136760d6ed4d1, base fb78d21c52a156e973c0b681b6070ff1cfa56b79. The feature patch is identical to the previously reviewed 5e54e36b patch after ignoring blob IDs and hunk positions. The overlapping Solo file only gains main’s dialog-focus waits in separate tests; the held-sharing readback, recovery ordering, clock helper and regression remain unchanged. This uses the small-follow-up solo exception, rather than claiming fresh team coverage. No browser cases added or removed in this follow-up.
Source-only: no tests, app or PR code executed. Hosted CI run 41150809658 failed JavaScript shard 2/3: StatusEditor.test.tsx could not find the /^Today,/ button in its existing-deadline/custom-date case. That file is outside this feature diff; this review does not establish its cause. The inspected browser checks passed, Windows was skipped. CI’s merge tree was independently confirmed equal to this reviewed head.
Selected advisory shard-3 artifacts contain three passing clock-regression executions per engine without retries. The complete sampled shard reports show Chromium 69 passed, 184.8 s wall / 284.4 s summed test time; WebKit 66 passed, 3 skipped, 270.4 s wall / 420.7 s summed test time. Huddles dominate both samples (slowest tests 13.9 s / 17.9 s); these samples are not a full repetition inventory or comparable before/after cost measurement. Native behavior, live focus/retry acceptance and human testing remain unverified. Public description and commit metadata inspected; no attached images or concrete disclosure finding. This COMMENT is not approval or merge authorization.
Include the StatusEditor test fix from main while preserving the existing browser clock changes. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@feedback.communities.buzz.xyz> Signed-off-by: Taylor Ho <[email protected]>
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account.
No new actionable findings in this merge-only follow-up. Reviewed head 76f734030b87e9973eab69efa2db402d1736727a, base 802bf5aaf28658aa143cc1c914ea8c4e62882d15. The full feature diff is byte-for-byte identical to the previously reviewed 29b6385e patch. The 13-file change since that head exactly matches incoming main; none overlaps the 11 clock-feature files. This qualifies for the limited follow-up solo-review exception. The helper, its callers and regression assertions remain unchanged.
Source-only review; no local tests, builds or app execution. The public description and all four commit messages were inspected; no images are attached. This merge adds no browser cases beyond the feature’s previously reviewed real-browser clock regression.
The one-time CI snapshot was still running. Independently inspected completed advisory shard 3 artifacts: the clock regression passed three repetitions in each engine, without retries. Those shards reported Chromium 69 passed (214.8s wall / 334.5s summed execution) and WebKit 66 passed, 3 skipped (216.1s / 333.7s); huddles was the slowest file, with its slowest case at 15.6s and 13.6s respectively. The tested merge commit’s tree equals the reviewed head. This is sampled hosted evidence, not a completed-suite, before/after cost, flake-elimination or human-acceptance claim.
Picks up the month-boundary StatusEditor test fix (#2064). Co-Authored-By: Claude Opus 5.5 <[email protected]> * 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 <[email protected]>
…isibility * origin/main: (23 commits) feat(profiles): show NIP-FI verified names (BUZZ-433) (#2005) feat(identity): sign in to the organization when Buzz opens (#2086) perf(identity-names): stop rerunning name policy for live member churn (#2092) fix(sidebar): keep the channel drag preview under the pointer (#2019) fix(identity): use xyz.block.buzz.desktop federated-session keychain service (#2094) perf(relay): read each channel's remote access once per accepted batch (#2091) fix(styles): keep page status :has() on each rule's subject (#2081) fix(navigation): show a notice for invalid deep links (#2054) fix(messages): preserve held readers through equal-height resizes (#1963) 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) ... # Conflicts: # src/bundled/channels/ChannelMemberAvatars.tsx
Category: infrastructure
User Impact: Browser checks reliably pause time without intermittent clock errors during otherwise unrelated changes.
Problem: A sampled pause target can fall behind the browser clock before the protocol request arrives, causing
Cannot fast-forward to the past. This affects tests that hold typing, hover, recovery and reading state.Solution: Hold a captured timestamp fixed while pausing, restore Date advancement, and apply each caller’s existing virtual offset. Replace 16 sampled-time targets across eight specs and consolidate the already anchored Solo workspace pause through the same helper, preserving pending timers and existing assertions.
File changes
tests/browser/clock.mjs
Share the anchored pause handoff without increasing timing margins or adding real waits.
tests/browser/clock.spec.mjs
Add a real-browser regression that advances time after sampling and verifies pending timer deadlines and Date progression.
tests/browser/solo-workspace.spec.mjs
Reuse the helper with no virtual offset, preserving main’s existing anchored pause and menu-focus assertion.
tests/browser/solo-chats.spec.mjs
Use the helper while holding a pending sharing readback.
tests/browser/thread-unread.spec.mjs
Replace three sampled pauses while preserving reading and focus handoffs.
tests/browser/typing-dm.spec.mjs
Use the helper for signed typing expiry.
tests/browser/notifications.spec.mjs
Replace two sampled pauses for permission checking and reading dwell.
tests/browser/huddles.spec.mjs
Replace four sampled pauses for speaking, avatar movement and incoming-call behavior.
tests/browser/agent-library.spec.mjs
Replace both sampled pauses around the animated settings footer.
tests/browser/avatar-loading.spec.mjs
Use the helper while holding avatar retry timing.
tests/browser/live.spec.mjs
Use the helper before channel and roster recovery arm their deadlines, including the caller added on current main.
bin/pnpm exec playwright test tests/browser/clock.spec.mjs --config tests/browser/playwright.config.mjs --project chromium --project webkit --no-deps --workers=1.runFor.Validation at
5e54e36b4328c53ae189b39da516d6b786c696f1, integrating main8fd0d34ae55e565e75cbc1fd31bc97897b0385be: both completeclock.spec.mjsandlive.spec.mjsfiles passed 8 cases across Chromium and WebKit in 37.4 seconds, with two workers and no retries. This exercises the newly migrated live-recovery caller while preserving main’s 250 ms clock steps, 3 s notice delay, 6 s cooldown and assertions. The committed tree exactly matches the tested tree. Command:bin/pnpm exec playwright test tests/browser/clock.spec.mjs tests/browser/live.spec.mjs --config tests/browser/playwright.config.mjs --project chromium --project webkit --no-deps --workers=2.Historical validation at
427d488b9: all ten affected files passed 157 cases across both engines, with one existing WebKit microphone skip. Hosted CI for that earlier head passed 65 checks, with two skipped checks. Those results predate this integration; hosted CI for the current head is pending. The delayed-sample regression previously failed with the old pause pattern in both engines and passes with the helper. One browser case added, zero removed; actual browser clock/protocol behavior requires this layer. Self-review of the integration found no blockers. Human test confirmation remains unrecorded.Related: plugin catalog artwork #1944.