test(desktop): stabilize unread and audio release smoke fixtures - #7821
Conversation
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
🔐 Codex Security Review
|
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent reviewed base 312cf674fad5563c24a66ec5350d306d192ed64a through exact head 1b126afbe75c2a2afbdfe13744f0938fa584764e.
The audio scheduler assertions bind the real singleton state and the deterministic fixture ordering does not change production behavior. However, the new exact channel/kind readiness oracle can still false-pass because the bridge loses filter correlation before matching. That directly defeats this PR’s stated smoke-hardening intent and needs correction before merge.
Author action: preserve individual REQ-filter correlation (or equivalent exact channel/kind pairs), require one filter to satisfy both predicates, and add a production-boundary regression for a two-filter REQ where channel and kind 9 occur in different filters. Preserve legacy wildcard behavior explicitly.
Verification: git diff --check passed on a clean exact-head checkout. Full Desktop unit suite passed 6,591/6,591 and E2E build passed in independent exact-head validation. Focused touched-spec validation passed the changed unread and audio hard-cap cases, proving scheduler transitions {active:3, queued:21} → {active:0, queued:0}; the broader focused run had four unrelated voice-note failures without a same-host base comparison, so CI remains the owner of that confidence gap. Required CI was still running at submission time.
| ): boolean { | ||
| for (const subscription of subscriptions) { | ||
| if ( | ||
| (subscription.channelIds.includes(channelId) || |
There was a problem hiding this comment.
[P2] Require the channel and kind to match within one original REQ filter. The bridge currently unions every #h and kinds value across an OR-list of filters before calling this predicate. As a result, [{"#h":["channel"],"kinds":[30078]},{"kinds":[9]}] is flattened to channelIds=["channel"], kinds=[30078,9], and this exact-channel check returns true even though no filter requested channel-scoped kind 9. I reproduced that result at this exact head. Preserve each filter (or correlated channel/kind pairs) through the bridge and add this two-filter case at that boundary; tests that construct only the already-flattened shape cannot catch the information loss.
There was a problem hiding this comment.
Fixed in dac0e8c. The live-REQ storage constructor now retains the original filters, and exact readiness requires channel and kind in the same filter. The regression passes both global-kind and other-channel counterexamples through that actual constructor. Legacy nonexact/wildcard readiness and event-delivery projections remain unchanged.
Validation: all 6,592 desktop unit tests, formatting/static checks and TypeScript passed via the commit/push hooks. A fresh E2E build and all 31 affected smoke tests passed with zero retries before the formatting-only hook changes. Please re-review the corrected boundary.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
…delivery Keep raw REQ filters for exact channel/kind readiness while preserving legacy delivery projections. Gate the Inbox live reaction fixture on its actual channel consumer rather than initial history rendering. Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Reviewed: 312cf674fad5563c24a66ec5350d306d192ed64a...dac0e8cd923d523fb7693c2569902abf1e7d2fd5 (exact head dac0e8cd923d523fb7693c2569902abf1e7d2fd5)
The previous cross-filter correlation defect is fixed in the implementation, but two author-actionable gaps remain in the repaired readiness oracle.
[P2] Explicit empty kinds falsely satisfies exact-kind readiness
desktop/src/testing/e2eBridgeSubscriptions.ts:31-36 uses !filter.kinds?.length, so both omitted kinds and explicit kinds: [] satisfy a requested kind. The new table at desktop/src/testing/e2eBridgeSubscriptions.test.mjs:20-22 codifies that result.
That disagrees with the relay contract: crates/buzz-relay/src/subscription.rs:170-174 treats explicit kinds: [] as matching no kinds. A dead channel subscription can therefore make __BUZZ_E2E_HAS_MOCK_LIVE_SUBSCRIPTION__({ kind: 9, exactChannel: true }) report ready, restoring the false-pass class this PR is intended to remove.
Author action: distinguish absence from explicit emptiness (filter.kinds === undefined || filter.kinds.includes(kind)), make the explicit-empty case expect false, and retain omitted kinds as the wildcard case.
Verification owner: author for the helper regression and affected smoke fixtures; reviewer rechecks the next exact head.
[P2] Bind the correlation regression to the actual REQ/socket bridge
The production boundary parses REQs and constructs stored subscriptions in desktop/src/testing/e2eBridge.ts:10850-10879, but the new correlation tests call createMockSubscription directly (desktop/src/testing/e2eBridgeSubscriptions.test.mjs:8-32). Mutation evidence confirmed the gap: restoring flattening only at the production invocation left the new suite green, 3/3. The test therefore does not prevent the bridge from reintroducing the exact defect being repaired. This conflicts with the production-seam/falsifiability requirements in AGENTS.md:192-196 and TESTING.md:19-29.
Author action: add a regression that sends the two-filter counterexample through the actual REQ/socket bridge and observes exact readiness as false, plus a same-filter positive control. Mutation of the production call site back to flattening must fail that regression behaviorally.
Verification owner: author for the seam-bound regression and mutation receipt; Desktop Core/smoke CI for exact-head gates.
Cleared surfaces
- Per-filter correlation in the current implementation rejects the original OR-filter counterexample.
- Inbox kind-7 readiness is awaited before live reaction injection.
- Scheduler assertions still observe the production-owned singleton and preserve the 3-active/21-queued → 0/0 ownership contract.
- No production UI or accessibility code changed, and no additional product defect was established.
Exact-head validation
git diff --checkon the previous-head delta: pass.- Full Desktop package suite: 6,592/6,592 passed at matching
HEAD. pnpm check: exit 0; reported only pre-existing warnings/information outside this PR's changed files.- GitHub macOS/Windows builds and relay artifact checks were green when sampled; Desktop Core and four smoke shards remained in progress.
Confidence gaps: focused local Playwright execution was blocked by the shared configured port; pending CI owns broader smoke validation. These are not additional author defects.
— :bot: Jude’s code review agent
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent re-reviewed base 312cf674fad5563c24a66ec5350d306d192ed64a through exact head dac0e8cd923d523fb7693c2569902abf1e7d2fd5. The prior cross-filter implementation defect is repaired, but two author-actionable test-oracle defects remain.
- Exact readiness conflates omitted
kinds(wildcard) with explicitkinds: [](matches nothing). The relay contract explicitly rejects the latter atcrates/buzz-relay/src/subscription.rs:170-174; the current helper returns ready and the test deliberately expects that false positive. Distinguish absence from emptiness and invert the empty-list case. - The correlation regression bypasses the production bridge boundary: tests call
createMockSubscriptiondirectly rather than feeding REQ throughsendToMockSocketand observing readiness. Mutating only the real bridge call site back to flattening leaves the new suite green 3/3. Add a socket/REQ-boundary regression with the two-filter counterexample and a same-filter positive control, and mutation-prove it fails if the call site reintroduces flattening.
No additional production UI/accessibility or scheduler-ownership defect was proven. git diff --check, just desktop-check, just desktop-typecheck, pnpm check, and the shipped helper suite passed at this head. One full Desktop Node run had 6,591 pass / 1 unrelated call-order failure that passed alone; focused Playwright was blocked by a shared port. Those are confidence gaps, not additional defects. At submission, Desktop Core and four smoke shards were still running with no red checks.
| (filter) => | ||
| filter["#h"]?.includes(channelId) && | ||
| (kind === undefined || | ||
| !filter.kinds?.length || |
There was a problem hiding this comment.
[P2] Do not treat explicit kinds: [] as wildcard readiness. !filter.kinds?.length is true both when kinds is omitted and when it is an explicit empty list. The relay contract distinguishes them: omitted means wildcard, while kinds: [] matches no events and is not indexed (crates/buzz-relay/src/subscription.rs:170-174). At this exact head, a channel filter with kinds: [] therefore makes the fixture report kind-9 readiness for a dead subscription, and the table test currently blesses it. Use an absence check such as filter.kinds === undefined || filter.kinds.includes(kind), expect false for the empty-list row, and retain the omitted-kinds positive case.
There was a problem hiding this comment.
Fixed in 50b7da2. Exact readiness now distinguishes omitted kinds from an explicit empty list, including when the caller omits the requested kind. Unit coverage includes the empty-filter OR valid-filter case. The socket regression also checks empty versus omitted kinds after an accepted REQ.
Mutation proof: restoring the old empty-list predicate makes the socket assertion fail with emptyKinds: true instead of false. Restoring the fix passes.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
| ownerPubkeys.add(p); | ||
| } | ||
| } | ||
| const subscription = createMockSubscription(filters); |
There was a problem hiding this comment.
[P2] Bind the correlation regression to this REQ/socket boundary. The new tests call createMockSubscription(filters) directly, so they verify the helper but not that parsed REQ filters actually reach it here. Mutation evidence shows replacing only this invocation with the prior flattened projection leaves all three new subscription tests green, including the correlation test. Add a regression that sends the two-filter counterexample through the actual mock socket/REQ path and observes exact readiness false, plus a same-filter positive control; reverting this call site to flattening must make that test fail.
There was a problem hiding this comment.
Fixed in 50b7da2. The new registered smoke spec mock-subscription-readiness.spec.ts invokes plugin:websocket|connect and plugin:websocket|send with serialized REQs, requires EOSE, then reads exposed readiness. Locked boot prevents app-owned subscriptions from satisfying the assertion. Both split-filter counterexamples are false; the same-filter positive control is true; CLOSE removes readiness.
Mutation proof: changed only createMockSubscription(filters) at the real sendToMockSocket call site to pass flattened channel/kind unions. The socket test fails because both splitGlobal and splitOtherChannel become true. The mutation was restored before committing.
Validation: rebuilt after each mutation; restored build and all 32 affected smoke tests passed with zero retries before formatting-only commit-hook changes. Exact committed head passed all 6,592 desktop unit tests and required pre-push checks. Hosted CI is separate and pending.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Explicit empty kinds match nothing; omitted kinds remain wildcard. Exercise serialized REQs through the mock socket, including cross-filter counterexamples, same-filter positives, EOSE acceptance and CLOSE cleanup. Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — APPROVE at exact head 50b7da257c0a231a9c22bd98558b2474100c5f4b (base 312cf674fad5563c24a66ec5350d306d192ed64a).
The two prior blockers are resolved:
- Explicit
kinds: []now matches nothing while omittedkindsremains wildcard-compatible, consistent with the relay contract (desktop/src/testing/e2eBridgeSubscriptions.ts:31-37;crates/buzz-relay/src/subscription.rs:161-174). - The regression now traverses the actual mock websocket command/REQ/storage/readiness seam, covering split-filter negatives, same-filter positive behavior, empty/omitted kinds, global behavior, EOSE, and CLOSE cleanup (
desktop/tests/e2e/mock-subscription-readiness.spec.ts:14-91;desktop/src/testing/e2eBridge.ts:10850-10890). A production-call-site flattening mutation makes the browser regression fail behaviorally, so this is causal rather than helper-only coverage.
Team review found no remaining author-actionable defect. Unread/inbox waits require the exact channel and event kind before injection while retaining visible outcome assertions. Audio coverage observes the real scheduler singleton and proves admission {active: 3, queued: 21} followed by cancellation/drain {active: 0, queued: 0}; no production UI or accessibility path changed.
Exact-head evidence:
- Full Desktop Node package: 6,592/6,592 passed.
pnpm check, desktop check/typecheck, E2E build, helper tests, andgit diff --check: passed.- Isolated-port socket regression: 1/1 passed.
- Isolated-port targeted changed browser coverage: 28 passed, including the changed unread/inbox/audio rows.
- GitHub smoke shards 1–4 are green; macOS/Windows builds and relay/integration checks are green.
Confidence gap, not author action: Desktop Core is still running at review submission. A broader local browser batch also encountered unrelated voice-note timeouts after every PR-changed row passed; no base-relative causality was established. Verification owner: the exact-head Desktop Core/release gate. Fixed-port reviewer contention is infrastructure debt, not grounds to hold sound code hostage.
Author action: none.
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent re-reviewed base 312cf674fad5563c24a66ec5350d306d192ed64a through exact head 50b7da257c0a231a9c22bd98558b2474100c5f4b. The two prior blockers are resolved.
- Exact readiness now distinguishes omitted
kinds(wildcard) from explicitkinds: [](matches nothing), consistent withcrates/buzz-relay/src/subscription.rs:161-174. - Channel/kind correlation remains per-filter. The new Playwright regression traverses the real mock WebSocket connect/send → REQ parser/storage → readiness hook path and covers split filters, same-filter positive, empty/omitted kinds, global-only, and CLOSE cleanup. A causal mutation at the production call site made the regression fail behaviorally.
- Unread and inbox fixtures retain visible user-state assertions; audio assertions observe the real scheduler singleton and prove
{active:3, queued:21}drains to{active:0, queued:0}with native fetch cancellation/release accounting. No production UI/accessibility behavior changed.
Exact-head evidence: full Desktop unit package 6,592/6,592 PASS; just desktop-check PASS; just desktop-typecheck PASS; E2E build PASS; isolated targeted Playwright 28/28 PASS, including the new socket regression and all changed unread/inbox/audio rows; git diff --check PASS. CI smoke shards were green and there were no red checks; Desktop Core remained in progress at submission. A broader isolated run still saw four unrelated voice-note timeouts without established base causality, so CI/release smoke owns that residual confidence gap.
Author action: none. Any new head invalidates this approval.
…n-surface * origin/main: fix(agent): preserve Databricks Opus UC reasoning and tool continuation (#7840) feat(canvas): add version history with atomic restore (#6780) chore(release): release Buzz Desktop version 0.5.24 (#7817) test(desktop): stabilize unread and audio release smoke fixtures (#7821) fix(desktop): remember Inbox unread-only choice (#7672) feat(mobile-infra): support development App Attest (#7744) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…rcement * origin/main: fix(hooks): strip repo-local git env from pre-push test lanes (#7841) fix(mobile-infra): render push grant lifetimes as decimal in chart 0.3.2 (#7820) fix(agent): preserve Databricks Opus UC reasoning and tool continuation (#7840) feat(canvas): add version history with atomic restore (#6780) chore(release): release Buzz Desktop version 0.5.24 (#7817) test(desktop): stabilize unread and audio release smoke fixtures (#7821) fix(desktop): remember Inbox unread-only choice (#7672) feat(mobile-infra): support development App Attest (#7744) docs(nip-fi): clarify federated identity amendments (#7803) fix(desktop): refresh channels after access-revoked closure (#7784) fix(desktop): bound startup request bursts and recover quota refusals (#7790) fix(audit): frame hash inputs with TLV (#7492) fix(admin): allow cold storage worker DB startup (#7770) Signed-off-by: Hayt <211b96e6a2b7f45fd4047988976c7bbbeeda0c15f3ae7b32eec20834b5a55118@buzz.block.builderlab.xyz> # Conflicts: # crates/buzz-relay/src/api/bridge.rs
…c-agent-commit-identity * origin/main: fix(mobile-infra): render push grant lifetimes as decimal in chart 0.3.2 (#7820) fix(agent): preserve Databricks Opus UC reasoning and tool continuation (#7840) feat(canvas): add version history with atomic restore (#6780) chore(release): release Buzz Desktop version 0.5.24 (#7817) test(desktop): stabilize unread and audio release smoke fixtures (#7821) fix(desktop): remember Inbox unread-only choice (#7672) feat(mobile-infra): support development App Attest (#7744) docs(nip-fi): clarify federated identity amendments (#7803) fix(desktop): refresh channels after access-revoked closure (#7784) Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…in-gate * origin/main: (30 commits) feat(agents): humanize uncurated Databricks model ids with a label grammar (#7844) fix: route databricks claude fqns to anthropic messages (#7829) feat(relay): add opt-in newest-first thread windows (#7823) refactor: move agent Git bootstrap into ACP harness (#7819) Use worker snapshots for relay storage metrics (#7845) fix(hooks): strip repo-local git env from pre-push test lanes (#7841) fix(mobile-infra): render push grant lifetimes as decimal in chart 0.3.2 (#7820) fix(agent): preserve Databricks Opus UC reasoning and tool continuation (#7840) feat(canvas): add version history with atomic restore (#6780) chore(release): release Buzz Desktop version 0.5.24 (#7817) test(desktop): stabilize unread and audio release smoke fixtures (#7821) fix(desktop): remember Inbox unread-only choice (#7672) feat(mobile-infra): support development App Attest (#7744) docs(nip-fi): clarify federated identity amendments (#7803) fix(desktop): refresh channels after access-revoked closure (#7784) fix(desktop): bound startup request bursts and recover quota refusals (#7790) fix(audit): frame hash inputs with TLV (#7492) fix(admin): allow cold storage worker DB startup (#7770) feat(relay): add admin HTTP routes for member restriction management (#7302) fix(relay): fire kick live side effects at convergence; persist target; fence re-add race with held lock (#7298) ... Signed-off-by: coder 1 <a93f3b1decd199cec83848f116ff60c20776cdd03861b9bba2610acae7c9eeeb@buzz.block.builderlab.xyz>
* origin/main: feat(agents): humanize uncurated Databricks model ids with a label grammar (#7844) fix: route databricks claude fqns to anthropic messages (#7829) feat(relay): add opt-in newest-first thread windows (#7823) refactor: move agent Git bootstrap into ACP harness (#7819) Use worker snapshots for relay storage metrics (#7845) fix(hooks): strip repo-local git env from pre-push test lanes (#7841) fix(mobile-infra): render push grant lifetimes as decimal in chart 0.3.2 (#7820) fix(agent): preserve Databricks Opus UC reasoning and tool continuation (#7840) feat(canvas): add version history with atomic restore (#6780) chore(release): release Buzz Desktop version 0.5.24 (#7817) test(desktop): stabilize unread and audio release smoke fixtures (#7821) fix(desktop): remember Inbox unread-only choice (#7672) feat(mobile-infra): support development App Attest (#7744) docs(nip-fi): clarify federated identity amendments (#7803) fix(desktop): refresh channels after access-revoked closure (#7784) fix(desktop): bound startup request bursts and recover quota refusals (#7790) fix(audit): frame hash inputs with TLV (#7492) fix(admin): allow cold storage worker DB startup (#7770) Signed-off-by: Michael Neale <michael.neale@gmail.com>
Summary
Repair the mock-backed desktop smoke fixtures blocking release PR #7817. No application unread logic, styling, audio component, scheduler, or release-tool changes.
Validation
channel-activity-popover.spec.ts,channel-mute.spec.ts, andvoice-note.spec.tspassed five repetitions with retries disabled: 150/150. This was the working repair on base312cf674f, before final formatting/cleanup, not the entire smoke suite.1b126afbe75c2a2afbdfe13744f0938fa584764e, normal pre-push hooks passed: 6,591/6,591 desktop unit tests, desktop checks, TypeScript typechecking, differential file-size gate, and branch-skew check. Pre-commit Biome formatting passed; working tree is clean.just ci, native release packaging, and the entire Playwright smoke suite were not run locally. GitHub CI is the remaining broader gate; this PR is not a release certification.Release follow-up
After this repair is merged and required checks pass, regenerate #7817 through the desktop release tooling and validate the new candidate's exact head. This PR does not merge or publish a release.
Originating discussion: buzz://message?channel=3428ec3f-a58b-429b-afbc-dc6a77918ce8&id=3303565b14334696e9bbf433ef3ad1aea4307acc69834f7f685a412e81354493