feat(channels): leave channels from the management pane - #363
Conversation
a36b5cc to
35ffa84
Compare
Reuse the shared lifecycle confirmation and departure navigation from Settings, with permission gating and cancellation focus restoration. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Use UUID channel identities and signed channel type/admin records for unread and profile Settings journeys. Wait for permission resolution before diagnostic alert assertions and keep membership events on the configured channel. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
35ffa84 to
d5bdab2
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source review (via Wes’s account)
No actionable findings in this change at head d5bdab27cecfd64477be43878bd3d4ccfba74a7a, against base 3a19fa43075283423c88a68d4a1362fade28ad3e.
Traced the Settings permission read and retry/abort fencing, exclusions for read-only/DM/session views, session-scoped dialog handoff, cancellation focus return, and confirmed-departure routing to the next sidebar conversation or explicit empty route. The existing lifecycle owner remains the only writer, reauthorizes before publication, and owns confirmed access removal. Reviewed the new component/provider/browser coverage and the UUID/type/admin/membership fixture corrections; the existing diagnostic assertions remain intact.
Evidence and limits: source-only review of pinned, Git-blob/SHA-256-verified inputs; no local code execution, tests, builds, app launch, or live departure. One read-only hosted snapshot of CI run 36484275787 showed CI required, JavaScript, Rust/tool integration, measurements, all six browser shards, security checks and DCO successful; Windows validation was skipped. Browser evidence identifies clean synthetic merge b17cd2aa9184113535cd82767b2543c8d77c2f86, whose parents are the exact base/head above. The two new Leave journeys passed in both Chromium and WebKit (four cases, zero retries).
Inspected shard-2 reports: 137 passing cases per engine, wall times 452.6s/601.0s and summed execution 829.9s/1127.3s (Chromium/WebKit); the Leave cases totaled 10.9s/19.1s. These are hosted observations, not a before/after performance claim; the PR’s local fixture-repair timings were not independently reproduced. Native/direct-signer, live-community and cross-device departure acceptance remain unverified. This COMMENT is not approval or merge authorization.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Reviewed head d5bdab27cecfd64477be43878bd3d4ccfba74a7a against base 3a19fa43075283423c88a68d4a1362fade28ad3e.
Changes required: sanitize the public PR description/media. No blocking production-code defect found in the Settings → shared confirmation → confirmed-departure path.
P2: Remove internal coordination links and workspace identifiers from public PR material
The Overview links to an employee-only issue tracker and an internal Buzz conversation. The three product screenshots expose the actual internal channel name; the Settings screenshot also exposes its UUID, matching the conversation link. This repository is public, so these disclose internal workspace details rather than providing public reproduction material. Existing PR convention does not waive the OSS privacy boundary.
Remove the internal tracker/conversation references and replace the screenshots with sanitized fixture-channel captures (including the channel name and ID). Keep the useful public GitHub validation links. The approved agent attribution email is not a finding. This is the required fix; no production-code rewrite is requested.
Optional, non-blocking
ChannelLeaveButton.tsx:27,42–49: Retry removes its own focused button while loading, leaving keyboard focus on the document body. Keep a disabled/busy retry trigger or restore focus after resolution.tests/browser/fixture.mjs:675–682:lifecycleRolemodels owner/member, but anadminvalue would appear only in membership hints, not the authoritative administrator record. No current caller uses admin; restrict the supported values or model that role if introduced.
Validation and limits
Reviewed the complete diff, PR text and all three images, and integrated independent UI, fixture/test and lifecycle reviews. Traced permission gating, stale/session fencing, failure/uncertainty handling, existing command ownership, access purge, focus recovery, and next/empty navigation with reload. Hosted CI run 36484275787 is successful for this head: 4,942 Vitest tests and both added Leave journeys in Chromium and WebKit passed. CI used synthetic merge b17cd2aa9184113535cd82767b2543c8d77c2f86; no local suites were repeated. No new native, live-community, or cross-device departure acceptance was performed. This review does not approve or authorize merging.
|
AI-generated by Carl, acting for Taylor Ho. @wesbillman The required public-material fix is addressed: internal coordination references are removed, and all three images are replaced with fresh captures of the actual production-built app using disposable fixture channel names/IDs and ephemeral identities—not a recreated UI. Validation at unchanged head The optional retry-focus and unused admin-fixture suggestions are unchanged. There are no inline threads to resolve; please recheck the required description/media correction. No approval or merge is claimed. |
Overview
Category: improvement
User Impact: Members can leave a channel directly from Channel Settings, with the same confirmation and recovery as the sidebar.
Problem: Someone already managing a channel has to return to the sidebar to stop participating.
Solution: Offer Leave channel in Settings when current permissions allow it, and hand it to the existing lifecycle confirmation and departure owner. Cancellation restores focus to the Settings action; only confirmed departure removes access and selects another usable conversation or the neutral Messages page.
Changes
File changes
src/bundled/channels/ChannelLeaveButton.tsx
Read fresh lifecycle permissions while Settings is open, omit forbidden Leave, and expose permission-read recovery. Hand the originating button to the shared confirmation without introducing another command writer.
src/bundled/channels/ChannelLeaveButton.test.tsx
Cover permission loading, forbidden and unavailable states, retry, keyboard handoff, and stale channel/capability/unmount fencing.
src/bundled/channels/ChannelsPage.tsx
Compose Leave into the existing Settings tools slot, excluding DMs, sessions and read-only views.
src/features/channel-navigation/ChannelNavigationState.tsx
Carry one transient lifecycle-dialog intent between Settings and the persistent sidebar, retaining existing session fencing.
src/features/channel-navigation/ChannelNavigationState.test.tsx
Cover both entrances sharing one dialog and session replacement rejecting retired callbacks.
src/features/channel-navigation/ChannelSidebar.tsx
Keep confirmation and completion in the existing owner, restore the management trigger on cancellation, and check the navigation target after confirmed access loss rather than relying on a roster entry that may already be gone.
tests/browser/channel-leave-pane.spec.mjs
Exercise Settings → native modal focus/cancel/pending → confirmed departure, next/empty routing and reload across app owners.
tests/browser/fixture.mjs
Allow the lifecycle fixture to represent a regular member while preserving existing owner defaults. Model signed type/admin records for ordinary roster channels and keep membership events on the configured first channel.
tests/browser/unread.spec.mjs, tests/browser/agent-activity.spec.mjs
Use a UUID for the channel opened in Settings; keep histories, read-state keys and observer records consistent, and wait for permission resolution before diagnostic alert assertions.
docs/channels.md
Document the Settings entrance, permission recovery and shared ownership.
Validation and remaining gates
At
1b4750dba19bdfaf0f86ece79d421eb808b0ed3c, rebased onto7a008c3042b1f4a434a4acc49c7cdc28583cc3f6:git range-diffreports the feature patch unchanged.git diff --checkpassed.bin/pnpm test:browser channel-leave-pane.spec.mjs channel-lifecycle.spec.mjs channel-settings.spec.mjs --project chromium --project webkit --no-deps --workers=2: 18 passed, 35.8s on local macOS. Uses the production app/broker with ephemeral identities and modeled upstream I/O, not live membership changes.d4fa23b0plus the feature passed 4,256 tests with one unchanged broker-test 503; that entire 70-test file passed in isolation. This is historical evidence, not a green full-suite claim at the rebased head. Broad validation belongs to hosted CI.1b4750db: no blockers, 9/10 for minimalness/elegance/correctness. The later fixture-only fix is self-reviewed, not independently re-reviewed. Draft pending hosted checks. No native/direct-signer parity, live cross-device departure acceptance, or merge claimed.CI fixture repair —
35ffa84f96606058f2585bddcbb3c3ef756c5370CI run 36479464803 exposed six failures: two unread cases and the profile-activity case in each engine. Opening Settings correctly rejected the old
alphachannel ID; the fixture also lacked ordinary-channel type/admin records. This follow-up changes only three browser-test files, not production validation or recovery behavior. The original diagnostic alert assertions are preserved; a visible Leave action is now the permission-completion barrier.1b4750db; the repaired tree passed 24/24. No cases added/removed, no retries/timeouts/error allowlists changed. These existing journeys cover app/broker/Settings/read-state/profile wiring; no new browser matrix was added.1b4750dbplus the exact three-file patch subsequently committed; verified byte-for-byte against the commit diff. Final head35ffa84fhas the identical tree. Mandatory pre-push TypeScript, 85 tests in 12 files, design types/guards and diff checks passed at the final head. No full local scan repeated.Local timing evidence (macOS arm64, Node 24.18.0, Chromium 153.0.8010.12 / WebKit 26.6, two workers; not hosted CI measurements):
Failures restart workers and stop the profile journey early; these are diagnostic costs, not a product speedup claim. The fixed slowest test was the now-complete profile journey (17.8s Chromium / 17.1s WebKit).
Historical state before the rebase below: fresh hosted merged-tree CI was pending; no full-suite-green claim. The independent reactions lint repair and shared page-error watcher migration are now included via main. Native, live departure and cross-device checks remain unperformed. PR stays draft.
Latest rebase —
d5bdab27cecfd64477be43878bd3d4ccfba74a7a3a19fa43075283423c88a68d4a1362fade28ad3e, including merged reactions-lint fix fix(browser-tests): restore reaction gallery lint compliance #368 and the shared page-error watcher migration.git range-diffmarks both patches unchanged. The UUID fixture repair and main'sapp.watchPageErrors(survivor)remain together. Both commits retain Taylor's author/sign-off and Carl's co-author credit.git diff --checkpassed; mandatory push hooks passed TypeScript, 85 tests in 12 files, and design types/guards.35ffa84f; remote head verified and worktree clean. No production changes added during rebase.Reproduction Steps
bin/just weband open a joined channel where your membership is allowed to leave. Browser mode uses the real configured identity; it is not a sandbox.Public-media correction — unchanged head
d5bdab27Removed internal coordination references and replaced the previous images with fresh captures of the actual production-built app using disposable fixture data. No source, test, dependency or commit changes.
Screenshots/Demos
Actual production-built app at
d5bdab27cecfd64477be43878bd3d4ccfba74a7a, captured September 28 in Chromium with dark appearance and the default green accent. Synthetic data, real product UI: “Lifecycle channel” and UUID11111111-1111-4111-8111-111111111111are disposable browser fixtures, not an internal workspace. No UI facsimile, image compositing or live membership changes.1. Open management from the channel header
2. Leave channel in the management sidebar
Captured after Cancel returned focus to the same Leave action.

3. Shared Leave confirmation