Repository navigation
feat(inbox): rename to Activity and simplify the filter bar - #2543
Open
tulsi-builder wants to merge 1 commit into
Open
tulsi-builder wants to merge 1 commit into
tulsi-builder wants to merge 1 commit into
Conversation
Rename the Inbox page to Activity and pare the toolbar down to one left-aligned row: type tabs, a "From" menu (Anyone/People/Agents) that fills when it narrows the list, and the Unread switch right after it. The first tab lines up with the title-bar bell. Remove Archive and Drafts from the page. The row check now marks a thread read; with Unread on it leaves the list with an Undo toast. Focus moves to the next row only while the list still owns focus, so a pending read never pulls the cursor out of the composer. The sidebar hover card is renamed to Activity too and no longer hides conversations archived before, so it lists what the page lists. Links made with the retired Archived or Drafts route still open the page with its saved filters instead of failing. Plugin id, page id, and storage keys are unchanged. Co-authored-by: BasilBot <399581b6165ab01343e3a1324774b1024d76942aea563468f93c975e5ee5193c@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: tulsi <tulsi@block.xyz>
tulsi-builder
force-pushed
the
basil/inbox-filter-clear
branch
from
November 5, 2026 00:54
334e685 to
c1dcf44
Compare
tulsi-builder
marked this pull request as ready for review
November 5, 2026 01:41
kalvinnchau
reviewed
Nov 5, 2026
kalvinnchau
left a comment
kalvinnchau
left a comment
Contributor
There was a problem hiding this comment.
🤖 Two non-blocking maintainability and test-contract findings.
| const focused = document.activeElement; | ||
| if (!focused || focused === document.body) return true; | ||
| return !!workspace.current | ||
| ?.querySelector('ul[aria-label="Activity"]') |
Contributor
There was a problem hiding this comment.
🤖 [P3] Decouple focus ownership from accessible copy
The list's accessible name is now also its lookup key. A later label change without the corresponding selector update makes listOwnsFocus() return false while a row is focused, preventing Mark as read from moving focus to the next row or toolbar. This change itself renames that label. Attach a ref to the list and use listRef.current?.contains(focused) so copy changes cannot silently change focus behavior. This is a maintenance risk, not a reproduced failure at the current head.
| expect(follows(fromTrigger(), unread)).toBe(true); | ||
| }); | ||
|
|
||
| it("loads a saved Drafts view, from before Drafts left Activity, as All", async () => { |
Contributor
There was a problem hiding this comment.
🤖 [P3] Remove the unsupported saved-Drafts migration claim
This case seeds activity: "drafts", but Drafts was component-local state at the base and toolbar persistence explicitly skipped view === "drafts"; persisted activities excluded Drafts. The same pattern dates to #770. The test therefore repeats the invalid-value fallback already covered by activity: "bogus", while implying a historical storage contract that did not exist. Remove this redundant case and the matching "or Drafts" saved-filter claim in docs/inbox.md:24. Keep the separate Drafts route compatibility, since routes did carry { drafts: true }.
matt2e
added a commit
that referenced
this pull request
Nov 5, 2026
…2566) ## Summary Six browser journeys each acted before a boundary they depended on: a debounced search landing, a background recovery read (in two journeys), a message reveal taking focus, a cold page load that fetched the fixture module by module, and a pause-then-seek in the same tick. Each commit adds the missing barrier, or removes the race at its source. All six changes are test-only: no assertions are loosened, no retries, sleeps or timeouts change, no browser cases are added or removed, and no product code changes. ### test(channels): let the member search land before the final pointer expansion **Problem** WebKit's `channel-members-focus.spec.mjs:826` "final invitation expansion continues keyboard focus at the first new row" failed 5 times between 2026-11-02 23:00Z and 2026-11-04 11:00Z, on 4 unrelated branches including main. Every failure was in the pointer iteration at :853: "Your other agents" held 8 "Add Helper" rows, never 10. - main, run 41662691884: https://github.com/block/buzz-app/actions/runs/41662691884/job/126010368608 - sol/aida-2436-recipient-search: https://github.com/block/buzz-app/actions/runs/41717923671/job/126203332188 - kylez_NEXUS-4155_solo-model-effort-per-chat: https://github.com/block/buzz-app/actions/runs/41748955704/job/126309380524 and https://github.com/block/buzz-app/actions/runs/41752221861/job/126320136617 (attempt 2 passed) - kylez_NEXUS-4165_agent-create-thinking: https://github.com/block/buzz-app/actions/runs/41799304245/job/126471226006 **Cause** Typing "Helper" shows the local agent rows at once. The relay search runs after `SEARCH_DEBOUNCE_MS` (300 ms) and adds a "People not in this channel" group above "Your other agents". In all five traces, the pointer click started 185–260 ms after the fill, while the list was still `aria-busy`. The click passed Playwright's hit-target check inside Show more's box (y 523–547). The People group then landed and moved Show more to y 543–567, so the press hit Show more and the release hit the row below it, and no click reached the button. **Fix** After filling the search, wait for the Member list's `data-refreshing="false"`, as this file's large-search test already does. The search keeps `loading` true from the moment the text changes until that text's results arrive, so the barrier can't pass early. **Validation** - Adversarial ordering, with a fixture gate that held the relay search until `pointerdown` on Show more and the debounce lowered to 100 ms: without the fix 20/20 failed at :853 with "Received: 8", and with the fix 20/20 passed. - Natural contention, with 4 CPUs shared with 4 busy loops: without the fix 1/24 failed with the CI signature, and with the fix 48/48 passed. - Both engines, `--repeat-each=30`: 60/60 passed. The full file, both engines: 36/36 passed. ### test(channels): deny the failed preference read so recovery can't race Retry **Problem** WebKit's `sidebar-section-creation.spec.mjs:9` "Channels offers empty-section creation with keyboard focus and confirmed persistence" failed at :167 clicking "Retry preferences". The call log said "element is not enabled", then "element was detached from the DOM". - clay/bestie-empty-state, run 41547288213: https://github.com/block/buzz-app/actions/runs/41547288213/job/125626193138 - kylez_NEXUS-4082_channel_unread_boundary, run 41553877476 attempt 1: https://github.com/block/buzz-app/actions/runs/41553877476/job/125648251816 (attempt 2 passed) **Cause** The journey failed the channel-sections writer read with a 502. A 502 counts as transient, so relay recovery reads again after 1 s. That read disables Retry preferences, and its success removes the button. In the trace, WebKit's first actionability check for the Retry click returned 100 ms after recovery's read reached the route. Any click that takes about a second to land loses the Retry to recovery. **Fix** Inject a 403 instead. A denial isn't transient, so recovery leaves it alone, and the manual Retry is the only way out, which is the path this journey exists to exercise. The failure is recorded under `app.report.relayFailures`. The check that the alert doesn't show the status now looks for "403". **Note on main's #2150** #2150 merged to main after this commit was written (as `21f6b97a99`). It makes the same 403 and `relayFailures` change in this hunk, with its own one-line comment, so the cherry-pick conflicted there. The conflict was resolved by keeping this commit's side. On this branch, the commit therefore only replaces that comment with the longer explanation and changes the alert check from "502" to "403". On main that check passes no matter what, because the injected status is already 403. The commit body still refers to #2150 as an open PR. No other adjustment was needed. **Validation** - Adversarial ordering, with the Retry click delayed 1.2 s and the first read after the failure held for 600 ms: - without the fix: 10/10 failed with the CI signature - with the fix: 10/10 passed, and 6/6 passed with a 3.5 s delay - Both engines, `--repeat-each=20`: 40/40 passed. The full file, both engines: 6/6 passed. ### test(inbox): wait for the entry reveal before filling an opened conversation **Problem** WebKit's `inbox-archive.spec.mjs:658` "reply and fresh mention reopen their filters without replacing composer draft" failed on attempt 1, and the same SHA passed on rerun. The composer was empty: `Expected: "Unsent mention retention draft"`, `Received: ""`. - peon/people-relationships-cache, run 41752428902: https://github.com/block/buzz-app/actions/runs/41752428902/job/126320899490 **Cause** Opening a conversation reveals its entry reply. `useMessageReveal` focuses the row two frames after the thread target is ready. The journey filled the composer as soon as it appeared, so on a slow runner the reveal landed during the fill. Playwright focuses the editor and selects its contents, and then inserts text at the selection. In WebKit, the row taking focus clears that selection, so nothing is inserted. A probe that focused the row between the two steps reproduced it in WebKit but not in Chromium. The trace shows the only DOM change during the fill was the entry row gaining `tabindex=-1`. Three more journeys in the file share the race. The existing in-view wait in one of them isn't a barrier, because the row is in view before the reveal runs. **Fix** Add a `revealed()` helper. It waits for the conversation list's `aria-busy` to be false, for the entry row to be focused, and then two frames. All four journeys that fill after Open now call it. The product's focus policy is addressed separately in #2563. **Validation** - Adversarial ordering, with the reveal held 800 ms and run as soon as a fill placed its selection: without the fix 17/24 failed, and with the fix 24/24 passed. - The four journeys, both engines, `--repeat-each=10`: 80/80 passed. The full file, both engines, `--repeat-each=2`: 40/40 passed. ### test(voice-notes): serve the fixture as a build so WebKit's first load fits **Problem** The "WebKit recording (macOS)" lane failed in the first case of `voice-notes.spec.mjs` (:138), in the shared beforeEach. `page.goto(.../tests/fixtures/voice-notes.html)` exceeded the 15 s navigation timeout. - renovate/all-minor-patch, run 41755043277: https://github.com/block/buzz-app/actions/runs/41755043277/job/126329063633 - peon/dnd-stable-transform, run 41757728323 attempt 1: https://github.com/block/buzz-app/actions/runs/41757728323/job/126337712196 (attempt 2 passed) **Cause** The fixture runs the real PluginRuntime, ConversationService, relay session and Voice Notes plugin, and the Vite dev server served that graph module by module. The traces show about 816–818 requests. Vite's median wait was about 50 ms, but the median receive was about 1.0–1.14 s, and the first request to the last response took 13.3–14.5 s. On a cold 3-core macOS runner, WebKit's per-module round trips alone nearly fill the budget. #1971 only warmed the server's transforms. **Fix** - Build the fixture once in `beforeAll` and serve it with `build` and `preview` from `vite-server.mjs`, as four other specs already do. The page now loads in 4 requests instead of 838. - The build keeps development React (`import.meta.env.DEV` and `process.env.NODE_ENV` are defined as `build.mjs` does), so StrictMode's double effects still apply. - The last case used to import `/src/features/messages/attachment-draft.ts` from the dev server. The fixture now exposes `attachmentDraft` bound to its session instead. **Validation** Cold `page.goto` to load, WebKit (WPE), one new browser per run: | CPUs, busy loops | dev server | build | | --- | --- | --- | | 3, none | 1.15–1.23 s | 0.53–0.65 s | | 3, 3 | 2.0–2.85 s | 0.44–1.21 s | | 2, 8 | 3.7–6.8 s | 1.1–2.55 s | | 1, 3 | 5.1–6.1 s | 2.0–2.3 s | - CI config, Chromium (all five cases) and Linux WebKit (the untagged case), `--repeat-each=10`: 60/60 passed. Pinned to 3 CPUs with 3 busy loops, `--repeat-each=5`: 30/30 passed. - **The macOS lane couldn't be validated here.** Its selection (`--project webkit --grep @media-recorder`) can't pass on Linux, because Playwright's Linux WebKit has no MediaRecorder. Before and after this change, those cases fail at the same step locally. Hosted macOS CI is the first real check of the fix. - `tsc --noEmit` is clean, and the flake-check listing test in `tests/integration/flake-check.test.mjs` that reads this spec passes. ### test(github): seek the fullscreen PR video without pausing it in the same tick **Problem** WebKit's `github-body.spec.mjs:35` "PR media plays under the packaged media policy in a narrow GitHub panel" failed at :189. The fullscreen video's `seeked` fired, but `currentTime` was about 0.5 instead of 15. - peon/federated-session-bundle, run 41567557720 attempt 1 (Received 0.533400327): https://github.com/block/buzz-app/actions/runs/41567557720/job/125692945945 (the rerun passed) - renovate/all-minor-patch, run 41683322444 (Received 0.48949321): https://github.com/block/buzz-app/actions/runs/41683322444/job/126083327417 **Cause** The fullscreen `VideoPlayer` autoplays and, on `loadedmetadata`, seeks to the inline video's position. The journey then paused the playing video and set `currentTime` in the same evaluate. There were two candidate mechanisms: - (a) the restore lands after the journey's seek and overwrites it - (b) WebKit completes a seek issued in the same tick as `pause()` on a playing video without updating `currentTime` Local probes support (b). About 0.2% of reloaded samples were unmoved: `seeked` fired, but `currentTime` didn't change. In those samples the restore always came before the journey's seek, and `requestVideoFrameCallback` showed a frame at 15.2 s while `currentTime` still read under 0.2. Across 1,800 reopen samples, `loadedmetadata` never landed after the journey's seek. **Fix** Before the journey opens the viewer, a MutationObserver pauses the fullscreen video as soon as it mounts, so it never plays. The observer also resolves on the video's first `seeked`, which is the restore's. The journey waits for that before its own seek, and no longer calls `pause()` in the seek evaluate. This covers both candidate mechanisms: the video has never played (b), and the restore has already landed (a). The assertion stays exact (`toBeCloseTo(…, 1)`), and the two inline videos still cover playback. **Validation** - Reload A/B, WebKit, 30 repeats × 50 samples per mode, 4 CPUs shared with 2 busy loops: - old: 3/1,500 unmoved, all with the CI signature - new: 0/1,500 unmoved That fits (b), but it isn't statistically conclusive (one-sided p ≈ 0.12). - Reopen A/B: 0/900 unmoved in both modes, and the restore never landed late (0/1,800). - "PR media plays", both engines, `--repeat-each=10`: 20/20 passed. WebKit pinned to 4 CPUs with 4 busy loops, `--repeat-each=16`: 16/16 passed. ### test(channels): deny the retried preference read so recovery can't race the final Retry **Problem** Found in code review, following up the sidebar-section-creation commit above. `navigation-groups.spec.mjs` "Create new retains its draft when preferences fail before submission and recovers in place" injects 502s on `sidebar-preferences` and clicks "Retry preferences" twice, the second time right after `page.unroute`. Recovery's next background read can succeed and remove the alert before that click lands. No CI failure of this journey was looked up. The race was found by reading the code and confirmed with a forced delay. **Cause** - The first 502 starts recovery, which retries after 1 s and again about 2 s later, and the route fails both. - The dialog's Retry calls `preferences.reload`. That cancels recovery's timer but keeps its attempt count, so the next background read is scheduled 2 s or 4 s after the manual Retry fails. - After the unroute, that background read succeeds, and the alert unmounts with its button before a slow final click lands. **Fix** Keep the first failure a 502, so recovery still owns it and shows the "Reconnecting to relay…" notice the journey asserts. From the first manual Retry on, the route answers 403, so recovery stays idle and the final click is the only thing that can recover preferences. The 403s go under `app.report.relayFailures`, and the 502 stays under `sidebarPreferenceFailures`. **Validation** - A forced wait between `page.unroute` and the final click, both engines: - without the fix, 4.5 s: 4/4 failed with the click timing out - without the fix, 1.5 s: 6/6 passed, consistent with the 2–4 s window - with the fix, 4.5 s: 6/6 passed - Both engines, `--repeat-each=10`: 20/20 passed. The full file, both engines: 20/20 passed. - Rebased on main `f4805605f5`: `tsc --noEmit`, `biome check --error-on-warnings` on the seven changed files and `git diff --check` are clean. Biome's one info-level `useLiteralKeys` note is on an untouched line that is the same on main. `node --test tests/integration/flake-check.test.mjs` passes 10/10. In WebKit and Chromium, `--workers=4 --retries=0`, `channel-members-focus`, `sidebar-section-creation`, `inbox-archive`, `github-body` and `navigation-groups` in full passed 84/84. `voice-notes.spec.mjs` under `playwright.ci.config.mjs`, Chromium and Linux WebKit: 6/6 passed. The macOS recording lane was not run, as noted above. A green run doesn't prove the flakes are gone. Hosted CI covers broad validation. ## Coordination - Draft #2532 adds the same `data-refreshing` wait in `channel-members-focus.spec.mjs` and conflicts textually with this branch. It duplicates the first commit, so only one of the two should land. - #2543 (open, head `c1dcf443`) deletes `inbox-archive.spec.mjs` and moves its draft journeys into a new `inbox-mark-read.spec.mjs`. There it works around the same reveal-vs-fill race with a `typeDraft` helper that refills until the text lands (`expect(...).toPass()`). Git reports a modify/delete conflict with this branch. A resolver who just takes the delete drops the `revealed()` fix. Porting the helper into `inbox-mark-read.spec.mjs` would replace the retry loop with a real barrier. - #2150 is merged. Its overlap in `sidebar-section-creation.spec.mjs` was resolved as described in the second commit's section. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: mtoohey <mtoohey@squareup.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
People said Inbox's filter bar was crowded and Archive didn't make sense. They liked the tabs and the Unread toggle, and they wanted the page called Activity, like Slack's. This PR keeps those parts and removes the rest, so the toolbar fits on one row for as long as possible.
What
buzz.inbox, page idinboxand storage keys are unchanged.showanddrafts. Older links that still carry them open the page with its saved filters instead of failing:showis ignored, and{ drafts: true }alone opens the saved filters.Not in this PR
src/features/relay/session.tsandinbox-feed.ts. Those files are shared relay code, so the strings are unchanged.archive.tsno longer has a caller in the page or the card. Removing it is a follow-up, so this PR doesn't touch the files fix(inbox): show Solo responses under All, read live replies in the open conversation, describe toolbar badges #2275 edited.Testing
vitest run src/bundled/inbox src/app src/features/pages src/pluginspasses, 1184 tests.showordrafts.tests/browser/inbox*andsidenav-polishpass, 72 tests.inbox-mark-read.spec.mjscovers the one-row toolbar layout, bell alignment, Mark as read with Undo, and All caught up.tsc,lintanddesign:checkare clean.🤖 Generated with Claude Code