test: replace timing races with explicit lifecycle barriers - #45
Conversation
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Review clear; required CI remains red
No actionable defect found in the nine changed test/instruction files at head 305dc53a4e68e834c16a48da2838102abbd71fe1, compared with base 61274d40e0171cae4ef519021370345e18b884d3. This is a source-review result, not approval or a merge-readiness claim.
The delivery contract is deterministic lifecycle synchronization without changing production behavior or weakening regression coverage. I reviewed all changed files and integrated three independent source-review lanes:
- Signed admission and observer: controlled clocks exercise the actual cooldown/pacing boundary; asynchronous signing remains real. The priority test observes prepared authentication before advancing the queue, checks no dispatch at 499ms, then foreground-before-background ordering. Cancellation still prevents signing/fetch and replacement authentication must have the new exact timestamp. Freshness rejection and unknown-write/no-resend coverage remain.
- Composer and navigation: GIF Enter checks the actual prevented-default/form-submit boundary rather than sleeping; typeahead waits for committed retry UI before keyboard navigation. The activation fixture holds registration explicitly, requires
openingand absent contributions while held, then requires visible contributions andopenedafter release. The gate releases infinally. - Terminal: pending-close entry gates disabled-state checks and releases on failure. Controlled time retains the real renderer and 3000ms splash expiry assertions, plus output, disposal, failure/recovery and re-enabled-state coverage. Production files, dependency versions and CI configuration are unchanged.
Validation boundary and remaining gate
Existing CI run 34766806559 reports this exact head. JavaScript, Rust/tool integration, browser measurements, both Chromium shards and WebKit shard 2 passed. WebKit shard 1 failed at tests/browser/image-scroll.spec.mjs:39, called from line 123: “bounded image navigation reaches its setup”, expected true, received false (failed job). Consequently CI required is failed.
The failing spec, its image-scroll fixture and shared browser/CI configuration are byte-identical between the pinned base and head. I found no causal link to this diff; that does not establish the failure as pre-existing or a confirmed flake. It remains a separate validation gate, not a demonstrated defect introduced by this PR.
Review computation used exact Git objects on Wes’s Mac Studio (BLKD2G9MWNRWY.local), without checkout changes, PR-code execution, builds, tests, runtime probes or CI reruns. This review does not certify all timing races eliminated or repair unrelated image-scroll/startup behavior.
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
* origin/main: fix(messages): prevent native WebKit prepend blanking (#49) feat: add shared browser and standard desktop notifications (#38) Prevent whole-window scroll bounce (#41) test: replace timing races with explicit lifecycle barriers (#45) test(browser): isolate fixture optimizer caches (#46) test(browser): settle startup catch-up before request baselines (#43) feat(messages): show message-carried blurhash before decoded originals (#40) Add a shared full-page surface (#29) fix(messages): prevent image-driven timeline scroll snap-back (#31) Restore grouped channel membership activity (#30) feat: add broker-backed Agent Activity panel (#36) Signed-off-by: Clay Delk <clay.delk@gmail.com> # Conflicts: # src/bundled/channels/ChannelsPage.tsx # src/features/messages/ChannelTimeline.tsx # src/features/messages/MessageRow.tsx # src/features/relay/contracts.ts # src/features/relay/fold.ts
Summary
Replace runner-speed assumptions with explicit lifecycle barriers and controlled clocks. No production behavior changes, retry increases, timeout inflation, relaxed assertions, or error allowlists.
AGENTS.md.Validation
All execution was on Wes's Mac Studio, Apple Silicon macOS, using the repository's pinned tools.
fc467321:just scanpassed repository checks, Node integrations, all 1,117 Vitest tests, plugin-manager Rust tests, and 299/300 browser cases. The remaining case exposed a pre-existing typeahead recovery race: ArrowUp ran before retry options committed.305dc53aadds only the retry-options visibility barrier. Its complete typeahead file passed 30/30 before commit; all four affected browser files passed 92/92 in two repetitions across Chromium and WebKit at the exact clean final head.The complete 300-case browser sweep was not repeated after the last test-only change. Hosted Linux CI still needs to validate this head; local passes do not certify Linux behavior or prove every possible flake is gone.
Scope limits
Remaining work is separate: PR #27 still has its missing WebKit copy event and canceled-stream diagnostic; PR #42 still has its composer-image selector and unsettled anchor-baseline issue. This PR does not resolve either feature PR. Intentional dwell, native lazy-loading, and performance observations remain intact.
Hosted image-navigation follow-up
The first hosted run at
305dc53failed image-scroll setup in Linux WebKit: all eight wheel gestures made progress but did not reach the required reading position. This occurred before the image-anchor assertions; it was not an image-preservation assertion failure.c1b87c17afc0879d26bb7b09a69c79f55c627956replaces the fixed gesture count with condition-driven real-input traversal, requiring net progress after geometry settles. The regression control limits each gesture to 400px and requires more than eight gestures; blocked input still fails on its first gesture. Image-response barriers, anchor/bottom thresholds, remount/request-ledger checks, and the existing test deadline remain unchanged. No direct scroll assignment or production change.Validation on the exact clean final commit, Apple Silicon macOS: complete image-scroll file across Chromium/WebKit, three repetitions, 42/42 passed. Pre-commit formatting/lint and pre-push TypeScript/related-unit gates passed. Independent diff review found no material issue. Full hosted Linux matrix and required human approval remain separate gates; this is not a hosted-green claim.
Related: #46 isolates Vite fixture optimizer caches and is now merged into main. The integration below retains both fixes.
Main integration after #46
Head
fbde4f2100ac453be336813f3d4a17f6ba6776f8merges maina67102aa1201adfa47a03be7d668a62ac748c152without rewriting the existing PR commits. The sole textual conflict was the shared observer-clock setup: retain one mock at the last millisecond of the fixed second, plus deterministic raw payload timestamp construction. Cache isolation/cleanup and the lifecycle/navigation barriers are all retained.At the exact clean combined head on Apple Silicon macOS: 1,117/1,117 Vitest tests, 3/3 fixture-cache integration tests, and 50/50 browser cases across the complete image-scroll, terminal-renderer, and typeahead files in Chromium/WebKit passed. Independent integration review found no material issue. Pre-commit and pre-push hooks passed without bypass. Full hosted CI and required approval remain gates; this update resolves branch conflicts, not a claim of hosted-green status or authorization to merge.