Commit Graph

2 Commits

Author SHA1 Message Date
Matt Toohey
0052b2354e test(desktop): gate flaky E2E specs on the state they assume (#8681)
## Summary

Test-side fixes for ten desktop E2E flake clusters and six long-tail
rows from the 21–28 Oct 2026 CI audit. Each spec now waits for the state
it assumes (a rendered row, a mounted listener, a finished animation, a
held boot read), not a fixed delay or a one-shot probe. Two mock-bridge
seams support this.

The PR changes test code and mock-bridge seams only, apart from one
product constant extracted with its value unchanged. It is one of seven
PRs split out of the `flakey-tests` branch (siblings below).

### Related issue

None found.

- #8622, #8244 and #8386 touch some of these spec files, but not the
tests fixed here.
- §7 mode A (`navigation:472`) is left to open PR #6852, which already
sets the width and measures in one polled evaluate.
- Draft #8279 queues cold-launch nostr-bind links in the product. Its
mock returns null, so the §12 listener seam is still needed.

### Mock-bridge seams

- `__BUZZ_E2E_EVENT_LISTENER_COUNT__(event)` counts live **app**
listeners per Tauri event. It wraps the mocked
`plugin:event|listen`/`unlisten`. The spec-facing
`__TAURI_INTERNALS__.listen` passes a marker target that the wrapper
skips, so a spec's own subscription can't satisfy the wait. Tauri
2.11.1's mock ignores targets, so delivery is unchanged.
- `deferFirstChannelsRead` arms the existing channels-read hold at boot.
`__BUZZ_E2E_RELEASE_CHANNELS_READ__` releases it.
- `CREATE_PROJECT_REFOCUS_DELAY_MS` moves to `createProjectFormFocus.ts`
with its value unchanged (50 ms). Both `CreateProjectFormContent` and
`project-commit-detail.spec` import it.

### Flaky tests fixed

| Test | Failing runs | Cause | Change |
|---|---|---|---|
| §2 `overscroll-boundary.spec.ts:34` (and its sibling) | 39601763602,
40367495044, 40583830861, 40597484657, 40373404267 | The
`message-timeline` test id first lands on the loading skeleton, which
doesn't overflow, so the scroll lock rightly cancels the wheel. | Wait
for a rendered row, and content taller than the viewport, before
dispatching wheels. |
| §4 `project-conversation-load-failure.spec.ts:73` | 40367495044 (all 3
attempts), 40608383847, 40583830861; PRs 39541263507, 40509460168 | The
seeded root and reply were dated `now`. The panel's live REQ (`since =
floor(now)`) still replayed the reply, so the error card never appeared.
| Date both events a minute in the past, and correct the comment. |
| §6 `workflows.spec.ts` `createWorkflow` cluster (:222, :365, :297,
:525, :471, :406, :614, :997) | 14 of 50 main jobs; :222 ×4, e.g.
40353619362 | `AnimatePresence mode="wait"` keeps the trigger body
mounted through its exit, so "Message text" could fill the trigger's
field (:222 and the callers). For :297, `isVisible()` read "closed" one
frame before the dialog opened its own channel list. | Wait for the step
body's "Step details" control before filling. Create dialogs wait for
the list; only the duplicate dialog clicks it. |
| §12 `nostr-bind.spec.ts` (:156, :198, :228, :247, :260, :335, :417,
:447) | 8 main jobs, each test once | The spec emitted
`deep-link-nostr-bind` before the app subscribed, and the mock drops
events that have no listener. | Wait for
`__BUZZ_E2E_EVENT_LISTENER_COUNT__` to report the app listener before
emitting. |
| §15 `profile.spec.ts:1128` (integration) | 7 of 50 main jobs | The 320
ms motion-sampling window started before the click. | Start the window
when `activeTab` reads "runtime". |
| §17 `agent-availability.spec.ts:602`, both shutdown-failure variants
(integration) | 6 jobs (4 disconnected-online, 2 failed-online) | The
"shutdown refused" toast sits over the Delete row and pauses while the
pointer is on it, so the retry click is intercepted. | Assert the toast,
then retry with focus + Enter. The toaster has no close button. |
| §19 `virtualization.spec.ts:247` "08 — cascading older pages…" | 4 of
50 main jobs; PR 40605736093 (all 3 attempts) | The 120 ms and 400 ms
windows were timed from before the wheel bursts, and CDP can deliver
wheels late. | Count the wheels the page receives. Each window opens at
the first wheel and closes only after all have landed, then waits for
still frames. **A second, product-side mode remains** (see below). |
| §20 `message-copy-link.spec.ts:160` | 4 of 50 main jobs (5 failed
attempts), e.g. 40595483966 | The huddle row reused `"d".repeat(64)`,
which is a seeded ordinary `#general` message. | Use ids that no seed
uses, and emit only after `waitForMockChannelHeadReady`. |
| §21 `navigation.spec.ts:687` | 4 of 50 main jobs | The pending chip's
`boundingBox()` was read once, and can be null while the row mounts. |
Poll until the chip has a width. |
| §23 `sidebar-snapshot.spec.ts:358`, `:388`, `:438` | PR 40427822003
(all 3 attempts); never on main | The 500 ms loading-state budget
counted app boot, so a slow runner finished the channels read first. |
Hold the first channels read with `deferFirstChannelsRead`, assert the
loading state, then release it. |
| `project-commit-detail.spec.ts:259` | 1–2 CI failures (audit long
tail) | A 50 ms refocus timer can land mid-fill and type the description
into the name field. | Once the name field is focused, wait out a page
timer of `CREATE_PROJECT_REFOCUS_DELAY_MS`; equal-delay timers fire in
order. |
| `project-pr-review.spec.ts:3520` | 1–2 CI failures | The same timer
pattern in `CreateProjectWorkItemDialog`. | Wait for `toBeFocused()` on
the title, which only that timer can satisfy. |
| `thread-load-failure.spec.ts:79` | 1–2 CI failures |
`openWelcomeThread` branched on a one-shot `count()` and fell back to a
hover-only Reply click. | Wait for the summary row, then click it. |
| `virtualization.spec.ts:201` | 1–2 CI failures | The drop point sat 2
px above the target's bottom edge, after two fixed frames. | Drop at the
centre, after dnd-kit's live region names the target. |
| `channel-composer-overflow.spec.ts:217` | 1–2 CI failures |
Same-second emits sort by id, and the clearance was checked once. |
`createdAt: base + i` and `expect.poll`. |
| `scroll-history.spec.ts:1289` | 1–2 CI failures | A fixed 250 ms wait
before idle sampling. | Poll for coverage first. |

Local before/after counts are from the original `flakey-tests` commits
(e286a6ae0, 816c7c35e, 67185df6a). Each used a forcing that made
`main`'s copy fail with CI's signature:

| Test | Before | After |
|---|---|---|
| §2 | 3/15 | 15/15 |
| §4 | 0/10 | 10/10 |
| §6 :222 | 0/3 | 3/3 |
| §6 :297 | 0/3 | 3/3 |
| §12 | 0/3 | 3/3 |
| §15 | 0/3 | 3/3 |
| §17 | 1/4 | 4/4 |
| §19 late-wheel modes | 0/3 | 3/3 and 2/3 |
| §23 | 2/6 | 6/6 |
| project-commit-detail | 0/3 | 3/3 |
| project-pr-review | 0/3 | 3/3 |

§20, §21, `thread-load-failure:79`, `virtualization:201`,
`channel-composer-overflow:217` and `scroll-history:1289` were not
reproduced; they get the fix that matches their CI signature.

Not fixed (follow-up): `virtualization:247` also has a product-side
mode. In one frame after a prepend, `scrollHeight` can drop 458–639 px
while `scrollTop` moves only 50–114 px, so the anchor row jumps 382–457
px. That is lost anchor compensation, and this test can still fail on
it.

### Verification

This branch was pushed without local re-verification, by the user's
choice: local test runs were overloading the machine and failing for
that reason. **Hosted CI is the check.** It was pushed with `git push
--no-verify`, because the pre-push hook runs typecheck and the unit
suites. #8613's flake-check lane will also repeat the changed specs.

- Every file except `virtualization.spec.ts` is byte-identical to its
final version on `flakey-tests` (67185df6a). That file carries only this
PR's hunks; see "Merge order".
- Checked for this PR: `git diff --check`, biome on the partial spec (no
changes, the same as on `main`), and a conflict-free merge with all six
siblings onto `main`.

### Readiness

Test and mock-bridge change; the one product edit is a constant
extraction with its value unchanged. AGENTS.md's readiness steps (agent
review, then human confirmation) are still outstanding, so
`buzz-review-completed` is not set.

### Merge order

No required order. `desktop/tests/e2e/virtualization.spec.ts` is also
edited by **flakey-tests-desktop-races**, which adds one test after line
929; this PR's hunks are all above line 551. They don't touch, so the
two merge cleanly in either order, as checked with `git merge-tree`.

### Sibling PRs

Split from `flakey-tests` (one PR per area): #8676 (PostgreSQL lease +
lane), #8677 (relay E2E helper + Windows shell), #8678 (mobile), #8679
(Tauri apt cache + Windows Rust cap), #8680 (desktop product races),
#8682 (desktop E2E composer + row actions).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Signed-off-by: Matt Toohey <contact@matttoohey.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Jitter <2b2e6415e748c35180847a37aadae06a11e30dddc76b784e7fd9354c4eb42e7a@buzz.block.builderlab.xyz>
2026-10-30 09:59:40 +11:00
Logan Johnson
d5a73b9f35 fix(desktop): derive agent availability from relay presence (#7127)
🤖

## Requested rebase published — ef40744b

Rebased onto fetched main **47d068e2109d077414cbf2f4f1c927f6d051037a**,
published **ef40744b3aeb4baaf8c81416e1a644fb5b315f91** with the exact
expected-old `df7fad6a` force-with-lease. No merge.

Manual conflicts were additive: preserve main's exact-key identity
documentation alongside the availability contract, and retain both
Bestie props and the shared availability reader in
`UnifiedAgentsSection`. Range-diff confirms unchanged lifecycle policy:
exact-key action-time authority, Unknown versus Offline, rejected
shutdown retains record/memberships, and separate local/provider/owner
gates. Main's exact-key profile routing survives. Both test-only CI
synchronization repairs (natural toast expiry and bounded stderr wait)
are byte-identical to the prior head. All ten original
authors/messages/DCO/material coauthor trailers are preserved;
configured signing policy was not changed.

Fresh checks on the rebased candidate:
- TypeScript, Biome on 26 changed TypeScript files, differential
file-size gate, and diff whitespace: pass.
- Focused production-hook/card/profile units: **58/58**.
- Fresh E2E build, availability/deletion browser: **11/11**, no retries.
- Main exact-key profile cases plus failed-DM send/startup retries:
**6/6**, no retries.

Previously reviewed full Desktop/buzz-agent package and mutation
evidence is reused for unchanged behavior; no ceremonial full suite, new
native/provider test, or `just ci` pass is claimed. Local configs,
dependency links, and historical artifacts are preserved.

Hosted observation: **MERGEABLE**, **BLOCKED / REVIEW_REQUIRED**, no
new-head formal review. [CI
33699735990](https://github.com/block/buzz/actions/runs/33699735990) is
running (including Rust and Desktop lanes), not a completed success. DCO
and required Security aggregate passed at the observation; the separate
Codex advisory review was skipped. No completed failing check or new
inline feedback observed. Historical approvals are not new-head
approvals. No reviewer/security authorization or merge action was
performed.

---

## Feature summary and retained pre-rebase evidence

## Summary

In Buzz Desktop, an agent could look online just because it had been
started or deployed, even when there was no current sign it was
connected. Cards and profiles now show availability from the agent's
relay presence rather than a saved launch record, so you can distinguish
an online agent from one that was merely deployed.

Agents cards and profiles use presence reported through the shared
server (the relay). A successful presence read with no online agent
shows Offline; failed/disconnected evidence shows unknown, rather than
retaining a misleading cached Online state.

Lifecycle actions remain separate. An offline agent may still have a
Shutdown action because the deployment record exists. Shutdown reports a
**request**, not proof the process stopped. Offline does not imply that
starting a duplicate agent is safe, and Online does not promise a
response.

### Related issue

Independent base: `main`; no stack parent or child among the
replacements. Extracted from
[#7114](https://github.com/block/buzz/pull/7114), retained as historical
source (`98fe33ec`).

[Behavior
contract](f4bb2ed44e/docs/agent-availability.md).
Originating [Buzz
discussion](buzz://message?channel=f7a9536a-1738-4bad-a888-b3ea25010ef1&id=7aa1f0ab23dce514bd8a0221441cf005bf428914621171472b79747c50820848)
· channel `f7a9536a-1738-4bad-a888-b3ea25010ef1`.

### Testing

The same saved provider-backed agent, with only authored presence
changing. These are mock-browser states, not a before/after deployment
or live relay transport test; production UI is unchanged by the later
fixture repairs.

**No online presence:** gray dot, existing Shutdown control retained.

![Offline presence does not remove the deployment lifecycle
control](https://raw.githubusercontent.com/block/buzz/ed68b1b4597dde47118b7591b1e8ebf3702447ca/pr-7127--offline-deployment.png)

**Online presence:** green dot, same lifecycle control.

![Authored Online presence changes availability without changing the
deployment](https://raw.githubusercontent.com/block/buzz/ed68b1b4597dde47118b7591b1e8ebf3702447ca/pr-7127--online-deployment.png)

[Capture
details](https://github.com/block/buzz/pull/7127#issuecomment-5482264824).
To check manually, compare runtime-only transitions with presence
updates, then disconnect/fail the presence read and verify it does not
stay Online. A Shutdown request should not immediately claim confirmed
termination.

#### Historical pre-rebase evidence and limitations (df7fad6a)

Lifecycle production source remains
**`f4bb2ed44e5a989d93c5f51e93c0bbd2dca941be`**. Current published head
is **`df7fad6ae65dda78508317186a95522d1bb22ed9`**: the prior browser
synchronization at `b78d093e` plus an additive two-file Rust
test-harness synchronization described below. No production bytes,
dependency/configuration files, or prior commits were changed; no
rebase. Current live main `0dbd036f5bff33e7ade75e7639f3218d424a6e73` has
identical failing-test/toaster/send-flow source; the causal browser
comparison used latest successfully tested main
`04babf02655440b4dfd37f2e2df605ead0a030d8`.

**Lifecycle/deletion correction:** both Agents and actual profile
deletion now pass the shared exact-key availability reader, not raw
cached data. It reads the canonical query state and connection at action
time, including after awaited channel discovery.
Failed/disconnected/pending evidence and unqueried persona siblings are
unknown; successful missing means Offline only for a requested key.
Successful background refetch cache remains usable; settled failure
revokes it. No second cache or per-row polling was added.

Provider record + channel + Online/Away/**unknown** awaits shutdown
submission before local removal; rejection preserves record/membership
for retry. Established Offline preserves intentional no-request removal.
No route preserves warned local removal. Local agents retain native
stop-before-remove, independent of presence. Profile consent now
describes a shutdown **request**, not remote deletion or guaranteed
termination. Existing ownership and force gates are unchanged.

**Verified, reused exact-candidate validation:** the independently
approved eleven-file patch (SHA-256
`2f69fe12ef0420e62dea1fd8db28cfa22cde5eecaf8080e656310a3e60d0cf86`) was
committed without byte changes. Desktop **5,921 passed, 0
failed/skipped**, including **26 new mounted production hook/IPC
regressions**; rebuilt availability browser suite **11/11 passed, no
retries**, including four actual profile Delete journeys. Desktop check
(existing 4 warnings/5 infos), typecheck, production/protected-feature
artifact matrix, differential file-size/policy and diff checks passed.
No blanket rerun or new full-repository `just ci` is claimed for this
frontend correction.

Production regressions cover cached Online **and Offline**
failure/disconnection, genuine missing/Offline, pending, successful
inflight refetch versus settled error, retained reader, error during
awaited channel discovery, unqueried persona sibling, shutdown
rejection/order/cancel, no route and local authority. Browser fixtures
use safe mock IPC and a retained provider receipt, not a real
deployment. Three restored mutation controls fail: unknown → skip
shutdown (**15** regressions), Agents raw-cache reader (**6**), actual
profile raw-cache caller (**1 browser journey**, false removal on failed
cached Offline). Independent review approved the exact frozen bytes and
added **4/4 cached-empty failure/disconnection probes** across both
callers. This is local independent approval, not formal GitHub/A Team
clearance.

The prior native propagation/poll-count defects remain closed ([earlier
response](https://github.com/block/buzz/pull/7127#issuecomment-5512278755)).
The prior hover-popover correction at `b55423f6` remains covered by the
full 11-journey browser run: pending/failed/disconnected means no badge
or accessible status, genuine missing/Offline retains an Offline badge.
Its earlier fallback-restoration mutation failed as expected (badge
count 1 rather than 0); that historical witness is reused, not rerun.

**Reused unchanged native/system boundary:** local `just ci` at
`c59067d8` passed workspace/Tauri fmt/clippy, static/policy checks, Rust
unit recipe, native workspace **3,159 passed / 20 ignored**, Web build
and **2,019 mobile tests**. No native implementation changed in this
lifecycle correction. These are historical boundary results, not
new-head native/live certification. [Parent
CI](https://github.com/block/buzz/actions/runs/33650549130) passed with
**14 retry-recovered browser flakes**, not retry-free. Old-head
CI/reviews are not current-head clearance.

**Hosted gates:**
[CI33662151103](https://github.com/block/buzz/actions/runs/33662151103)
on `f4bb2ed4` **FAILED**: smoke shard1 had 322 pass, one failure, one
retry-recovered flaky, two skipped. The failed first-DM retry test timed
out on all three attempts because the error toast intercepted Send. That
failure is preserved, not waived; the scoped test repair below is
published as `b78d093e`.
[CI33668171165](https://github.com/block/buzz/actions/runs/33668171165)
on `b78d093e` subsequently **FAILED** the Rust unit budget regression
described below. Both original failures remain visible; neither was
retried to green. Exact `f4bb2ed4` and `b78d093e` APPROVED reviews cover
unchanged reviewed bytes, not formal approval of the new head. Fresh
exact-head CI and the established automated technical rereview are the
next gates for `df7fad6a`. Historical deletion responses remain
([5092381800](https://github.com/block/buzz/pull/7127#issuecomment-5513759316),
[5092391193](https://github.com/block/buzz/pull/7127#issuecomment-5513763497)).
No formal review dismissed. The [security
notice](https://github.com/block/buzz/pull/7127#issuecomment-5482231278)
and latest-push maintainer/codeowner policy remain separate actionable
gates: eligible Block organization members own current-range
authorization. No merge/security authority exercised.

**CI causal repair (`b78d093e`, test only):** the error `Message failed
to send: Mock first DM send failed.` is deliberately injected by the
existing fixture. CI screenshot and retry trace show the bottom-right
Sonner notification over the actual enabled Send button. `fill()` leaves
the pointer parked there; Sonner pauses its 4-second lifetime while
hovered. A fast run can click before animation settles (unchanged local
test passed in 2.7s; two actual tested-main CI cases passed first
attempt in 3.3s), which does not disprove the failure. Independent
controlled browser runs on `f4bb2ed4` and tested main `04babf` both
reproduced the same toast hit-test at Send `(1203,627,32,32)`,
persistent hover beyond 4s, and intercepted ordinary click with no
second send. Moving the real pointer to the editor allows natural expiry
and successful ordinary retry, preserving all original
DM-channel/recipient assertions. This same synchronization already
exists in the neighboring agent-startup-failure test.

The one-file correction keeps the visible error assertion, scopes its
toast locator, moves the pointer back to the editor and observes normal
toast removal (bounded 10s) before retry. No forced click, direct toast
dismissal, mocked clock, CSS override, skipped test, production behavior
change, or new backend mock. Six focused browser executions pass
(first-send/startup-failure, three repeats each, no retries); the
held-toast control fails on original bytes at the Send click while the
exact repaired test passes. Biome and diff checks pass. Reuse unchanged
5,921 Desktop / 11 availability browser / four independent probes above;
no semantic production change warrants repeating those suites. Original
failed CI attempt/retries, local fast pass, deliberate failing control
and all traces remain in `WORK_LOGS/AVAILABILITY_CI_B9210A40`. Browser
evidence is mock-IPC Chromium, not native/live-relay certification. The
UI still temporarily overlays Send while a notification is hovered; the
test exercises its real move-away/expiry recovery, not immediate
click-through.

**Rust CI causal repair (`df7fad6a`, test only):** [original Rust / Unit
Tests failure,
job100375291370](https://github.com/block/buzz/actions/runs/33668171165/job/100375291370)
tested GitHub merge `223dee91a396d8cb4ebf18b9b8559e5a54951235`.
`context_recovery_budget_exhaustion_surfaces_the_error` failed at
`regressions.rs:2756` in **0.091s** because its immediate stderr
snapshot lacked `context recovery budget spent`. ACP context-error
assertions had already passed. The captured prefix shows all three
budgets **32768 → 16384 → 8192 bytes**, above the 4096-byte floor, and
ends during the third attempt. This is **not evidence of floor
exhaustion**. The collector is an independent Tokio task; a stdout
response is not a stderr barrier. Recovery, harness and test blobs were
identical across the compared base/head/merge parents; no production
regression was implicated.

The shared test Harness now provides a bounded event/condition wait,
registering for collector notifications before reading the buffer to
avoid lost wakeups. The budget and adjacent terminal floor assertions
wait for their own diagnostic and retain the matching snapshot. The
budget test still requires the provider's ACP context error and exactly
three recovery rungs, now corroborated by **exactly four provider
calls** and no floor diagnostic. Timeout remains a real failure with
captured stderr. No fixed sleep, weaker assertion, skip,
provider-limit/logging change, dependency/config edit, or production
change.

**Deterministic causal control:** the same real agent/HTTP-provider/ACP
scenario holds only stderr collection behind a one-shot gate until after
stdout responds. The old immediate snapshot fails the original budget
assertion (intentional exit101); the repaired wait explicitly remains
Pending while held, then passes after release. No scheduler-speed
assumption or fixed sleep. This reproduces the observation race under
controlled delay, **not the exact historical CI schedule**. A
missing-diagnostic test proves the wait actually times out. Original
failure and deliberate failing-control logs/patch remain in
`WORK_LOGS/RUST_TRIAGE_06E32D2D` and `WORK_LOGS/RUST_SYNC_BC5758B9`.

**Final candidate validation:** focused recovery **9/9**, floor **1/1**,
absent-diagnostic timeout **1/1** pass. One full touched-package run,
`cargo test --locked -p buzz-agent`: **695 passed, 0 failed, 1 existing
ignored**, including all **54 regressions**. Local nextest was
unavailable, so this uses the repository-supported cargo-test fallback,
not a claim of nextest reproduction. `cargo fmt --check`, package-scoped
Clippy all-targets with `-D warnings`, differential file-size/policy and
diff checks pass. Previously reviewed availability production and the
Desktop/browser evidence above are unchanged and reused; no all-native
blanket rerun. The test-only delta was self-reviewed against collector
ordering, timeout and falsification evidence. Existing production
approval remains valid for those bytes; exact-new-head technical/CI
clearance is not assumed. The required **Security aggregate** is
distinct from optional Codex advisory feedback; no security
authorization, human review contact, or merge was requested.

**Remaining policy limits:** shutdown submission is not harness
acceptance or process termination; confirmed Offline/no-route local
removal may leave a remote process; route discovery is best effort,
membership cleanup uses `Promise.allSettled`, and multi-instance
deletion is sequential/non-atomic. No distributed singleton,
provider-health, tenant-switch cancellation, live relay TTL or packaged
WebView/VoiceOver certification is claimed. The pre-existing DM-header
raw-presence fallback (`ChannelScreenHeader`/`useActiveChannelHeader`)
remains outside this repair and uncertified. Screenshots above remain
historical mock-browser illustrations, not new deletion or native
transport evidence.

---------

Signed-off-by: Logan Johnson <loganj@squareup.com>
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
2026-09-02 21:53:15 -04:00