Repository navigation
BUZZ-615: Separate viewer membership from peer rosters - #2700
Open
bradseiler wants to merge 7 commits into
Open
bradseiler wants to merge 7 commits into
bradseiler wants to merge 7 commits into
Conversation
bradseiler
force-pushed
the
honor/buzz-615-client-authority
branch
from
November 6, 2026 22:47
0e52115 to
4acd6e4
Compare
bradseiler
marked this pull request as ready for review
November 6, 2026 23:32
Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Hold the next browse page while asserting retained cached rows, then release it after remount. This preserves the request and isolation assertions without racing the automatic paging timer. Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Use the viewer projection for draft access and owner role fallback. Keep explicit role and read-only policies, and cover denial with retained peers plus joined proof without a roster. Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
bradseiler
force-pushed
the
honor/buzz-615-client-authority
branch
from
November 7, 2026 00:53
4acd6e4 to
6b92e20
Compare
Contributor
|
🤖 Design review at Evidence: viewer membership, draft recovery, lifecycle gate. Unchecked: no screenshots supplied or live app/tests run; themes, narrow/wide layouts, enlarged text, long content, many-agent states, keyboard/focus, native behavior and performance remain unverified. Current-head Chromium 3/9 and WebKit 4/14 browser jobs are failing; those failures were not triaged here. The described browser workflow run covers an older head. |
Contributor
|
🤖 Design follow-up: no design review needed. This supersedes the design classification in my earlier review. Any engineering findings there remain with code review. |
kazemek21
added a commit
that referenced
this pull request
Nov 9, 2026
## TL;DR Removes the six owner-session read tools that let a Buzz-hosted agent read with its owner's own access: channel, member and DM lists, channel details, messages and canvases. An agent's own reads and the channel archive, unarchive and delete changes are unchanged. The tools are gone from discovery on every harness, and a cached or direct call to one is refused before anything is read with the owner's session. Fixes NEXUS-4348 · Initiative: https://blockcell.sqprod.co/sites/kylez-buzz-mcp-initiative/ ## Scope Kyle's request was to remove "the channel/member/dm list/get" endpoints and leave the archive/delete channel changes for now. Removing `owner_read` and the two Canvas tools as well is the initiative lead's interpretation: they read through the same owner session and handle, and leaving them would keep the owner-read capability this removes. If that is wrong, the #2881 half can be restored on its own. ### Removed From #2745 (owner channels and members): - `owner_channel_list`, `owner_channel_get`, `owner_member_list`, their hint text and discovery on Claude Code, Codex, Pi, the session RPC and the harness kit. - `src/features/agents/owner-access.ts`, the run handle's `ownerAccess` field and the runner's host hook that built it for bundled-revision runs. - The `BUZZ_OWNER_MCP_COMMUNITIES` build setting (`vite.config.ts`). A distributor build that still sets it now gets nothing from it; `docs/configuration.md` lists it with the other retired inputs. - The owner-tool sections in `docs/agents.md`, `docs/configuration.md`, `docs/buzz-help.md` and `src/buzz-mcp/AGENTS.md`. From #2881 (owner messages and canvases): - `owner_read`, `owner_canvas_get`, `owner_canvas_history`, `src/buzz-mcp/owner.ts` and the `Owner*` client types. - The `docs/relay-queries.md` paragraph, and the exports of `tools.ts` helpers that only `owner.ts` used. `edited()` keeps its author-only edit selection, minus the owner parameter. ### Kept - From #2745: the Stop and cancellation fences in the Claude Code, Goose and ACP sessions (`workLifetime`, `current()`), which every Buzz tool call now depends on, and the channel-lifecycle changes (`channel-lifecycle.ts`, including its `records` parameter). - `addTool` still refuses plugin tool names starting with `owner_`, so a stale call to a retired name can never reach plugin code. There is no other owner-prefixed restriction and no new permission layer. - Ordinary agent-scoped reads, Codex per-thread/turn tool validation, plugin and external isolation. ## Alternate routes checked Nothing else reached these tools: no Rust, IPC, sidecar or RPC path, and Mesh and external agents use the same run handle that no longer carries owner access. Codex thread tool keys no longer carry the `:owner` suffix, so threads created with owner tools are replaced instead of reused with a stale tool list. ## Tests The new tests list the six names as a fixed fixture rather than reading them from the catalog: - `session-tools.test.ts`: `sessionTools()` and `sessionHint()`, which every harness lists from, name none of the six. - `service.test.ts`: a real bundled run handle has no `ownerAccess`. With the owner's session stubbed to return an `OWNER_ONLY` message for an owner-only channel, each retired name is refused with `No tool <name>` before any relay query. An ordinary `read` of that channel then runs with the agent's own key, filtered to that channel, and returns `No messages.`. The same client's `read` of the agent's own channel returns its message, and no read through the owner's session ever names the owner-only channel. External agents see none of the names and are refused the same way. - `codex/runtime.test.ts`: `dynamicTools` lists none of the names, each call fails Codex's validation, a planted owner-access spy is never called, and the thread's own `read` works. - `app-client.test.ts`: a handle that still carries an owner capability doesn't pass it to the tool client. - The Claude Code and Goose Stop/timer tests now hold an ordinary `mem_get` read instead of an owner tool. On a Blox box with these commits applied to main: `tsc` passes, and the 12 affected Vitest files pass (claude, codex, goose, pi runtimes, app-client, tools, session-tools, service, kit, ACP client, channel-lifecycle). Full CI covers the rest. `git push --no-verify` skipped the pre-push lanes (types-and-related-unit-tests, design-system, rust-clippy); CI's `pnpm check`, sharded Vitest and clippy jobs repeat all three. Fail-before check: the removal tests in `service.test.ts` and `codex/runtime.test.ts` were run against the pre-removal source (0a5c190). Three of them fail on real assertions: - Codex `dynamicTools` contains `owner_channel_list`. - An external agent's call returns "Owner tools are not available to this agent here; nothing was read." instead of `No tool owner_channel_list`. - The bundled handle has `ownerAccess`. All three pass on this branch. Vitest strips types, so this check doesn't cover `tsc`. The `session-tools.test.ts` discovery test also passes on the old source, because that source only lists the tools when the owner setting is on, so it doesn't count as a fail-before check. ## Not covered - No native app run or live relay check. The removal is exercised through the service, runner and harness tests above. - A Buzz process already running a build with these tools keeps them until it updates. This doesn't revoke anything remotely. ## Related open PRs - Human work in overlapping files keeps its behavior but may need a rebase: #2700, #2974, #3081, #2972 (cancel added tools on native Stop), #2935, #1634, #2622, #3095, #2882. - #3004, #3005, #3006, #3008 and #2980 build on the removed owner access and are not part of this change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) (claude-opus-5-5) on behalf of kylez --------- Signed-off-by: Kyle Zemek <kylez@squareup.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
matt2e
added a commit
that referenced
this pull request
Nov 9, 2026
…l refuses the alert (#3066) ## Summary A reminder set on a message in a channel you later leave (or are removed from) never alerted. At the due time the plugin submitted only the message target, `notificationAuthorized` refused it because the viewer is no longer a listed member, and the reminder was already marked shown, so it went silent for good. The Reminders page listed it under Overdue with no channel name and no explanation. This PR keeps the message target as the first attempt and, when the notifications service refuses it, resubmits the same alert (same `sourceKey`, title and body) with the Reminders page of that community as the target. The `shown` guard is still set synchronously before the first submit, so nothing is retried later and the existing no-replay behaviour on relaunch and session swap is unchanged. **Behaviour change:** a due reminder in a channel you aren't in now alerts on time with "Reminder due" and the saved preview, and clicking the banner opens Reminders in the reminder's community. Two matrix rows in `index.test.ts` ("missing membership", "an empty roster") flip from 0 to 1 banner accordingly. A due reminder in a channel you are in still opens the message. ## Why it is safe Every other refusal path in `NotificationsService.submit` refuses the page target too: closed service, missing viewer, stale category, viewer mismatch, the `seen` dedupe (its key ignores target kind), the pending cap, the dev pause, alerts off, the Reminders category off, permission denied, and the Focus gate (which refuses page targets). So the page alert can only appear when the message was refused for channel access. A future gate that allowed pages but blocked conversations would be routed around; `docs/plugin-architecture.md` now says so in the gate paragraph. ## Commits 1. `fix(reminders): alert on the Reminders page when the message's channel refuses the alert` — the 10-line plugin change, README Details bullet, new test and the two flipped rows. Every banner-producing row now clicks the banner and asserts what opens. 2. `test(reminders): pin that the page fallback respects gates and membership` — comment stating the refusal invariant, the plugin-architecture sentence, two more matrix rows ("missing membership, Focus on" through the real `soloConversation` gate expects 0 banners; "a listed channel without membership" expects 1 banner opening the page), and a `docs/buzz-help.md` bullet. 3. `docs(reminders): say the alert opens Reminders when you aren't in the channel` — help wording covering channels you were never in. ## Verification - `TZ=UTC vitest run src/bundled/reminders src/features/notifications src/bundled/focus src/bundled/readmes.test.ts`: 24 files, 322 tests pass. `tsc --noEmit` and Biome clean. `src/buzz-mcp/tools.test.ts` (reads buzz-help.md) passes. - On main with only the tests applied, the new test and the two flipped rows fail. - Mutation checks: making `notificationAuthorized` admit every conversation target fails the two flipped rows on the click assertion; removing only the `members` clause fails exactly "a listed channel without membership"; making `soloConversation` admit pages fails exactly the Focus row. - Reproduced in the real desktop app (Linux dev build under Xvfb with dunst banners, two local relays, throwaway keys). macOS banners were not exercised; the change is renderer-side and platform-independent. ## Demo Control on the fixed build (still in the channel), then the viewer leaves the channel with main on the left and this branch on the right, then the Reminders row close-up. The banner is the existing OS notification; only where a refused alert goes has changed. https://github.com/user-attachments/assets/fafd7a57-c3cc-4104-b558-489b2b751b05 Shorter side-by-side of just the left-channel flow, main left and fix right, aligned on the due time: https://github.com/user-attachments/assets/c84c8583-f0c8-49a5-aad2-339291c8087e     ## Notes for reviewers - Already-missed reminders are not recovered on upgrade: the plugin checks only reminders due after its saved check time, so an old silent miss stays under Overdue without an alert. - A preview from a private channel the viewer was removed from now reaches the OS notification surface. The same text was already on the viewer's Reminders page. - Follow-ups, not in this PR: explain "you're no longer in this channel" on the Reminders row; a host-side fallback target used only when `authorized` fails would keep this policy in the host. - Overlaps: #2285 and #2075 have trivial textual conflicts in the same submit/mock lines; #2700's `viewerMember` change is compatible, the fallback follows whatever the access check decides. 🤖 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 branch has not been deployed
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
Channel consumers use peer rosters to decide whether the viewer has joined. A denied channel can retain its old roster while a fresh public preview restores reading. That stale peer list must not restore joined behavior.
What
Add viewer-bound membership evidence to channel summaries and use it for viewer-only gates. Explicit membership loss takes precedence over retained peers; joined evidence works without a loaded roster.
How
DiscoveryState owns the projection, and store equality includes it. The shared predicate checks the viewer pubkey. Legacy summaries without the projection remain compatible. Each consumer keeps its existing Guest, cache, archive and preview rules.
The conversion includes lifecycle, reactions, agent setup, draft/sent previews and owner-access role fallback. Agent setup follows live membership changes and retains its separate peer eligibility check. Explicit viewer roles and read-only Sent behavior keep their precedence.
This slice still derives membership from legacy discovery. Peer demand and compact startup follow in later slices. A separate test-only commit controls background pagination before asserting restored pages; production paging and timeouts are unchanged.
Risk
Retained peers no longer grant joined behavior after denial. Startup keeps its existing reads, relay permissions remain authoritative, and feature flags and rollout remain unchanged. Roll back by reverting this slice.
Testing
Published child:
6b92e207097c0225b47e507cb1a3ec0b47fb2b51, above parent52fbf9bcc7eeecf0f3e6b007f888718d2c3836ad, on main22e4efd2e4d5e0c1d413869da322a1d82729748b. All seven child patches replay unchanged. Mandatory hooks passed with one worker: types, related unit tests, design checks and Rust Clippy. Independent review found no blockers; 682 targeted tests passed. Full local validation and CI run 42320998609 remain pending. Cancelled duplicate run 42320998268 has a failed aggregate; assess the active run separately. The first publication attempt failed with 28 test failures and a worker-start error; all 265 tests in the 14 affected files passed serially without edits. This does not establish the cause of those failures. Earlier full-suite evidence (19,368 passes, 22 skips) and CI run 42307295237 apply only to old head4acd6e44f.Independent review caught all five individual draft/owner gate reversions. The new live setup regressions catch stale-prop and roster-authority reversions. Denied-preview regressions exercise real DiscoveryState denial, complete-read omission and deny-all transitions.
Historical comparison: pre-existing timeout, reproduced on base; no higher failure rate at the child. With two workers and a sign-in companion, cold Settings timed out in 3/20 runs at
8b908c832and 0/20 at1f2e96d15. The companion passed 40/40. This does not establish equal or better performance, explain the timing mechanism, or validate the restacked candidate. The original 209-failure hook run remains unexplained; failed runs remain recorded as failures.Historical agent app workflow at old head
4acd6e44f: all 28 existing channel-permissions and channel-lifecycle journeys passed in Chromium and WebKit, using the built app and production browser broker with an isolated relay fixture. This covers public previews, explicit join/leave, Guest read-only access and lifecycle controls. Native behavior and performance are outside this browser run. Explicit human testing remains pending. Zero-roster startup and throttled native/browser acceptance belong to the later startup slice.Bigger picture
Part 2a of BUZZ-615, stacked on #2661. Next add bounded peer-roster demand, then compact startup with lifecycle fencing and cache isolation.
Generated with Codex