feat(channels): confirm archive delete leave and dm hide actions - #92
Conversation
db4efaf to
a455718
Compare
0cbb69a to
67146d8
Compare
e34cbbe to
2f87660
Compare
2f87660 to
cea6444
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
No blocking defects found in the supported browser-broker scope. Reviewed bfec94cd4dcff6e2bbbb6c71fdda5e3ccc17bf7f against 119195ea331de33c8480bab180df0091ca8e9421, including fresh permission checks, command isolation from the outbox, definitive versus uncertain receipts, relay-state confirmation/purge, DM visibility, and menu/dialog focus. The final test-only repair preserves the keyboard and local DM-removal assertions.
- Validation: source-only review with an independent receipt/retry cross-check. Existing CI run 36067046919 passed on synthetic merge
541b45bfcd818fb501793b493ef1cfab93e52ce9, combining this head with the stated base: 309 Vitest files / 3,368 tests, Rust/tool integration, browser measurements, and both Chromium/WebKit journey shards. No reviewer-run repository code or destructive relay writes. - Remaining gaps: browser Leave lacks an end-to-end journey; lifecycle-specific failure classes are tested across layers rather than through one real dialog/broker integration. Native/direct-signer parity and attended native acceptance remain deferred/unverified; Windows native validation was skipped. These are not claims of full-platform acceptance. Live cross-device visibility, unarchive/unhide UI, and delegated/community-admin authority remain outside this slice.
This is a COMMENT review, not approval. The PR body’s “Remains draft” wording is stale relative to GitHub’s draft:false.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested: two P2 integration defects. Head bfec94cd4dcff6e2bbbb6c71fdda5e3ccc17bf7f, base 119195ea331de33c8480bab180df0091ca8e9421.
The inline findings describe the required fixes and regression cases. Preserve the documented dev-broker/direct-role scope, silent permission loading, and exact navigation to hidden DMs.
Validation: CI run 36067046919 passed on merge 541b45bfcd818fb501793b493ef1cfab93e52ce9 (this head/base): 309 Vitest files / 3,368 tests and 606 Chromium/WebKit journeys, including all six lifecycle cases. Local validation was source/contract review plus an isolated Chromium/WebKit modal-Escape reproduction; no broad suite rerun, PR-code execution, or live destructive writes. Browser Leave, attended native behavior, and live cross-device visibility remain unverified, not additional blockers.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Both P2 findings from review 5311166559 are resolved in the author’s published fixes. No remaining blocker from this bounded re-review; this is not a formal approval.
Independently verified the exact current head 4f615de800d91741af40be971bfbf26b006eee9f against base 204ea9d1dea6b7c524d6e09f032395fbde723830: full lifecycle browser file, 12/12 Chromium/WebKit cases passed. This covers last-visible Archive/Hide staying neutral across reload, intentional hidden-DM navigation, and pending narrow-Settings Escape with visible uncertainty recovery and focus restoration. All six new cases failed when run against the original bfec94cd production paths.
The full Vitest suite passed 309 files / 3,368 tests on pre-main-integration fix head 72435bd; that result is not attributed to the later merge. Current hosted DCO is green; broader current-head CI remains outstanding. No live destructive writes or attended native validation. The existing platform/Leave/cross-device limitations remain unchanged.
The PR author published these fixes while my separate implementation was in progress. I stopped before pushing and verified their version instead; no duplicate patch or history rewrite was pushed.
4f615de to
725f53f
Compare
Resolve fresh relay-owned channel permissions, keep lifecycle commands outside the message outbox, and confirm authoritative state before removing rows. Preserve DM membership, fence cancelled sessions, and make ambiguous publication recoverable without blind resubmission. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Accept the relay's four-field membership roster without using hints as administrator authority. Exercise valid and malformed roster shapes, correct browser fixtures, and offer safe retry copy instead of raw protocol errors. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Remove action ellipses and omit forbidden Leave actions instead of displaying a disabled item and ownership guidance. Preserve confirmation and domain authorization, with mounted and browser regressions. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Wait silently for fresh menu permissions while retaining failed-read retry and all authorization checks. Cover pending initial and retry lookups without exposing unchecked actions. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Keep session creation and local DM removal in their existing owners, project only lifecycle actions, and preserve the silent pending-permission menu. Publish lifecycle commands through the current broker socket identity. Retain other members in the Leave fixture and verify viewer-scoped confirmation. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Reuse the shared confirmation input and destructive button recipe instead of overriding control styles and consuming direct palette tokens. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Compose confirmed lifecycle actions after New session in the merged context menu. Preserve local DM removal and persistent sidebar ownership, add shared leading icons, and cover menu composition, focus, and navigation across both browser engines. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Locate local DM removal through the row owner instead of fixed parent depth. Assert the exact New session and unavailable-lifecycle composition while preserving geometry, focus, draft resume, hide, and restoration checks. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Keep existing mute/read, session, focus and lifecycle assertions while checking their combined action groups. Document the menu order after rebasing onto the merged attention controls. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
725f53f to
8af0236
Compare
Overview
Category: new-feature
User Impact: Users can archive, delete or leave supported channels, and hide DMs without removing anyone from the conversation.
Problem: The sidebar lacked confirmed channel lifecycle actions.
Solution: Extend the existing right-click/Shift+F10 context menu with type-appropriate, icon-led actions backed by fresh permissions and explicit confirmation. Remove rows only after relay-owned state confirms the change.
Published snapshot:
8af02363ff880986694c78badf3251c112a0ca40, rebased onto main322381dfcd0fb18faa25fb0b24354dc30d1acfc6: 23 files, +2,676/−47. GitHub currently marks the PR ready for review (not draft); validation gaps below remain, and this update does not change its draft/ready state.Rebase integration: Lifecycle actions follow New session and the mute/read group in the merged menu, not a replacement menu or new ⋮ control. Preserve main’s persistent
ChannelSidebar, session eligibility/navigation, keyboard and pointer menu behavior, and local DM ×. The existing row/item components and their CSS remain unchanged.ChannelsPagenow honors an explicit empty destination so retained archived/hidden membership cannot reopen itself after the last visible row is removed. Includes the requested 14px Phosphor icons throughMenuIcon. The rebase preserves main’s mute/read handlers, retry UI, generation fencing, preference projection and authenticated mute transport, plus its Settings/profile and native session changes. Archive/SignOut reuse main’s exports; Envelope/EnvelopeOpen remain alongside EyeSlash/Trash.Scope: Archive/Delete/Leave and relay-backed DM Hide, with authorization, confirmation, recovery and sidebar integration. No Move/Star/group writer, sorting, mute/read additions, or shared-menu restyling. Relay-backed Hide remains separate from local DM ×.
Source: buzz://message?channel=b9ab2a04-14c4-440d-8c82-aebfbc1caa68&id=2fafae420919217d2b096b02c22f0a045356b19708c16a52d1e65d54d1741d41
Changes
39000/39001/39002checks before signing and again before publication. Membership hints never grant administrator authority; the relay remains final authority.9002/9008/9022/41012commands never enter the message outbox or automatic replay. Definitive rejection permits explicit retry; uncertainty blocks blind resubmission.30622visibility filters sidebar rows without denying exact DM navigation.File changes
dev/relay-broker-api.test.mjs
Exercises the real lifecycle routes, command-shape/signature restrictions, and disconnected versus connected publication.
dev/relay-broker.mjs
Admits narrow lifecycle commands without adding them to ordinary message-writer kinds; retains main’s live-socket publication identity.
docs/channels.md
Documents permissions, confirmation, recovery, retained mainline behavior, and explicit parity limits.
src/bundled/channels/ChannelLifecycleDialog.module.css
Owns dialog layout and semantic surfaces; shared controls own their paint and spacing.
src/bundled/channels/ChannelLifecycleDialog.tsx
Confirms every action, requires the exact channel name for Delete, and prevents blind resubmission after uncertain delivery. Keeps modal Escape from hiding the parent navigation while native cancellation owns dismissal.
src/bundled/channels/ChannelLifecycleMenu.test.tsx
Mounts the real menu/dialog to exercise pending/retry permissions, confirmations, uncertainty, and stale completions.
src/bundled/channels/ChannelLifecycleMenu.tsx
Loads permissions only on menu opening and emits a separator only with lifecycle content when there is preceding content.
src/bundled/channels/ChannelsPage.tsx
Honors the explicit empty lifecycle destination instead of reopening retained membership via saved/default selection.
src/features/channel-navigation/routes.ts
Accepts the version-1 empty Channels destination without treating it as session creation.
src/features/channel-navigation/routes.test.ts
Covers empty-destination parsing alongside existing new-message and session routes.
src/features/channel-navigation/ChannelSidebar.tsx
Extends the persistent sidebar’s existing context menu after New session and attention actions. Owns confirmation/focus handoff, post-removal navigation, and combined local/relay DM visibility.
src/shared/design-system/icons/index.ts
Adds EyeSlash and Trash through the existing gateway; reuses main’s Archive and SignOut exports without duplicate declarations.
src/features/relay/channel-lifecycle-protocol.ts
Defines narrow command shapes and direct role/type/membership capabilities from relay-authored records.
src/features/relay/channel-lifecycle.test.ts
Covers all commands and authority, signing, cancellation, visibility and uncertainty boundaries. Leave retains another member and honors the viewer filter.
src/features/relay/channel-lifecycle.ts
Owns fresh authorization before signing and publishing, relay-state confirmation, uncertain outcomes, and per-viewer DM visibility.
src/features/relay/session.ts
Composes lifecycle into the existing session lifetime and routes confirmed Delete/Leave through access-loss purging.
src/features/relay/transport.ts
Exposes dedicated broker signing/publication and supplies the live publication-identity header.
tests/browser/channel-lifecycle.spec.mjs
Covers real-app Archive, Delete and DM Hide, the composed context menu, persistent sidebar handoff, last-visible-row Archive/Hide plus reload, and pending-modal Escape/uncertain-result recovery on narrow Settings in Chromium/WebKit.
tests/browser/fixture.mjs
Adds opt-in signed lifecycle records, retained archived/hidden membership, and modeled relay effects without live destructive writes.
tests/browser/navigation-sidebar.spec.mjs
Preserves session creation and persistent-sidebar regressions. Disabling Sessions removes New session, not the independent lifecycle menu group; both right-click and Shift+F10 are checked.
tests/browser/policy-relay.mjs
Models the exact capability-query shape used by the production broker fixture.
tests/browser/navigation-session-menu.spec.mjs
Checks exact New session + disabled lifecycle-unavailable composition while preserving all corner-radius, theme/width, draft-resume and focus assertions.
tests/browser/navigation-mute-read.spec.mjs
Checks exact attention + lifecycle-failure composition while preserving mute/read persistence, retry, session disabling, focus and draft-entry assertions. Main’s sidebar-unread test file is retained byte-for-byte.
Reproduction steps
Use the signed fixture-backed app, not live community data, for destructive checks.
41012is published. Main’s local DM × remains separate.Validation and remaining gates
Current head
8af02363, base322381df:dev/relay-broker-api.test.mjsby retaining both independent test blocks. Verified the resulting file is new main byte-for-byte followed by the previous lifecycle test byte-for-byte. No assertion changed or removed.8af02363: 319 Vitest files / 3,503 tests passed, including the combined broker API suite; 20/20 Chromium/WebKit lifecycle and Settings cases passed. Lint of the three overlapping files andgit diff --checkpassed.bin/pnpm exec vitest run bin/pnpm test:browser --project chromium --project webkit --no-deps \ tests/browser/channel-lifecycle.spec.mjs tests/browser/settings.spec.mjsPrior-head evidence only:
725f53f0, base6ae0fe94:3f62aec8plus the test/doc changes committed as725f53f0; the commit hook subsequently only wrapped one assertion for formatting. No production edits followed the browser run.725f53f0, 317 Vitest files / 3,472 tests passed, including lifecycle, session and broker coverage; 98 design tests passed;design:buildpassed (including design typecheck).725f53f0: TypeScript, 183 related Vitest files / 2,229 tests, design typecheck and all design guards. No bypass.git diff --checkpassed. All 11 outgoing commits preserve actual authorship and matching repository-required DCO trailers; hosted DCO Check passed at the new head.725f53f0; GitHub reports conflict-free mergeability. No approval or merge performed; ready/draft state unchanged.Rebase evidence: replayed the ten feature commits onto main’s merged mute/read work. Conflicts retain both sidebar action groups and both relay write capabilities. Shared icons keep main’s Envelope/EnvelopeOpen and Archive/SignOut plus lifecycle’s EyeSlash/Trash. Main’s newer DM-row test locators win over the equivalent branch repair. Range-diff preserves the last-row and modal-Escape fixes unchanged.
Integration fail-then-pass: the first Chromium run at
3f62aec8had two stale composition failures: lifecycle expected no attention group, and mute/read expected no lifecycle group. Updated exact item/separator expectations retain every existing action, focus, persistence and recovery check. The legacybetafixture’s invalid lifecycle ID remains rejected; its retry group does not disable attention actions. The UUID lifecycle fixture proves the successful three-group menu and withheld lifecycle separator while permission loading is explicitly held. No browser cases added or removed by this rebase; no retries, delays or tolerance changes.Review repairs: last-visible-row Archive/Hide now stays neutral despite retained membership, including reload; pending native-modal Escape cannot collapse narrow navigation and strand uncertain-result recovery. Both existing review threads received direct replies and are resolved.
Browser test rationale: six lifecycle scenarios per engine (three added in the review repair, none removed). The three new scenarios prove last-row Archive and Hide across persisted route reload, and native Escape propagation through a narrow navigation disclosure with gated publication and uncertainty. These browser/modal/persistence boundaries are not established by the unit tests. Permission/role/failure matrices remain in lower-layer tests. The pending-state regression uses an explicit held request rather than a delay; no regression-first failure is claimed for these added cases.
Earlier evidence, not current-head validation:
bfec94cpassed its complete hosted run 36067046919 after reproducing and fixing stale menu and DM-remove locator expectations (5 failures before, 16/16 two-file browser cases after). The six screenshots below remain fromcea6444, not the current head; they document the earlier menu/dialog appearance, not the later attention-group composition or route/Escape repairs.Remaining gaps: browser Leave integration and attended native acceptance remain unproved; the browser Leave fixture still removes the entire roster entry, while the unit fixture preserves other members and checks viewer-filter behavior. The full browser suite, Node CLI integration and Rust/native validation were not run locally; the full Vitest package suite was run as listed above. No live destructive writes, live cross-device visibility check, or matched before/after performance claim. Required reviewer/code-owner approval remains outstanding. The earlier independent review applied to pre-rebase
2f87660, not this integration; it is not carried forward as approval.Scope limits: development-broker writes only; native/direct-signer parity and delegated/community-admin authority are deferred. No in-app unarchive or DM reopen/unhide. Visibility refreshes with the roster, not a new live subscription. Confirmation protects the bundled UI, not arbitrary trusted same-origin JavaScript. Cancellation cannot retract an already-sent command.
Screenshots / demos
Historical real production-built app at
cea6444, synthetic signed fixture data, dark theme, narrow crops. These captures predate the merged mute/read group and are not screenshots of the rebased menu. No destructive commands published by the capture journey.Permission lookup pending
New session remains; no unchecked lifecycle actions or dangling divider.
Verified owner actions
The merged context menu keeps New session and adds icon-led Archive/Delete after one separator. Leave is absent for the last owner.
Archive confirmation
Delete confirmation
The shared input is empty, so Delete remains disabled.
DM Hide confirmation
Hide changes per-viewer visibility, not membership.
Permission lookup unavailable
New session remains available; Retry recovers when authority records return.