feat(channels): archive and delete channels from settings - #385
Conversation
a8930ec to
eac924e
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source review (via Wes’s account)
Reviewed head eac924e4a8429c339da3ce1e6b14915c3413f792 against base c6b47a5837fd8912dc84d98bb959714a23d70818.
Two actionable findings: the Settings Delete-permission retry loses keyboard focus (inline), and the public commit metadata exposes an internal deployment identifier (below). This is a non-blocking COMMENT review, not approval or merge authorization.
[P2] Remove internal deployment identity from the public commit metadata
The attribution trailers in all four PR commits (6891ee7, e499ac0, db772f8, and eac924e) contain an agent public-key identity at an internal deployment hostname. This public repository distributes that infrastructure/account association through Git history; editing only the source or PR description would leave it exposed. Have the relevant authors correct the affected attribution using verified, public-safe identities while preserving truthful authorship and valid DCO certifications. The hostname/key are deliberately not repeated here.
Scope and observations
- Traced Settings/sidebar handoff, shared confirmation/navigation, signed permission records, host-bound discovery, viewer/channel/community response binding, optional Delete failures, cancellation/access fences, and pre-sign/pre-publication checks. The old profile-based ownership shortcut is no longer used, and optional Delete failure preserves independently verified Archive/Leave permissions.
- Preserved the stated product decisions: explicit named Delete confirmation without typing, direct-role Archive, member-only eligibility, archived-channel Delete prohibition, and deferred native/direct-signer parity. No request to expand these boundaries.
- Inspected the description, all three screenshot attachments, changed source/docs/tests/fixtures, and all four commit messages. The screenshots show the stated disposable channel fixture; no additional public-material finding in those images or added source lines.
- Optional documentation correction: the description still calls
a8930ecfcurrent, describes profile-based ownership and unresolved draft blockers, and says this remains a draft. Update it to distinguish the new capability implementation from still-deferred live/human acceptance, and identify the relay-capability dependency. Do not carry old-head validation forward as current-head evidence.
Validation limits
Source-only review of 1,686 Git-blob-verified, unchanged extracted files; no dirty checkout inputs, PR-code execution, test runs, installs, app launches, or destructive relay operations. Read the applicable repository/contribution/channel/session guidance and relevant block/buzz vision documents (reference revision 12670bd0f037c66a682272bb81c46c3f254fad74).
The single read-only hosted-check snapshot at the reviewed head showed the required Linux/JS/Rust/browser lanes and DCO/security checks successful; Windows native validation was skipped. These are hosted results, not tests run by this reviewer. Three logical browser journeys are added (six engine cases), none removed; real modal/handoff/navigation boundaries justify those journeys, while permission matrices stay below the browser layer. Typed-name assertions were intentionally replaced for the revised UX. The description explicitly reports no fail-then-pass mutation experiment, and its local run evidence is for older heads. The new retry tests assert recovered controls, not keyboard-focus continuity. Live relay capability deployment/semantics, destructive acceptance, native behavior, and final human acceptance remain unverified.
eac924e to
4b0918d
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source review (via Wes’s account)
Reviewed head 4b0918d09c5c6439d7b92ed3d1b4ef9eea0d098f against base fc825697c97ea0ad5c68473f723b12133addaf5b (merge base 14a2e7ed585b130315629ded39d1b83b05eb2a53).
One unresolved public-material finding; no new production-code defect identified in this revision. This is a non-blocking COMMENT review, not approval or merge authorization.
[P2, still unresolved] Remove internal deployment identity from public commit attribution
The previous metadata finding remains in all five current PR commits: 202fa417, 485928e3, 30265d9b, ed78da24, and 4b0918d0. Their attribution trailers associate an agent public-key identity with an internal deployment hostname. This exposes an infrastructure/account association through public Git history; changing only source or the PR description does not address it. Have the relevant authors correct the affected attribution using verified, public-safe identities while preserving actual authorship and valid DCO certifications. The hostname and key are deliberately not repeated here. This is the existing finding, not a new one.
Prior feedback and source observations
- The prior Delete-specific retry-focus finding is resolved by removing that optional authority query and its retry control. The direct-owner-only boundary is explicit in the updated description and docs; no request to restore owner-agent Delete or add relay/native capabilities.
- Settings uses one fresh permission load and independently gates Leave/Archive/Delete, then hands the selected action and trigger to the existing persistent sidebar owner. Traced named confirmation, cancel-focus return, confirmed navigation and the neutral last-channel destination. Preserved the intentional no-typing Delete confirmation and Archive/Leave styling.
- The lifecycle/session/transport and persistent navigation owners match the base. Fresh signed-state checks still precede signing and publication, and uncertain delivery still prevents blind resubmission. The added archived-state condition also participates in both permission rechecks.
- Assessed error/retry separately from success/cancel. Generic Settings permission retry still clears and unmounts the focused button without an explicit handoff, but that behavior already exists in base
ChannelLeaveButton; it is not the removed Delete-specific regression. Confirmation rejection/uncertain-delivery tests check recovery and resubmission controls, not browser focus continuity. No claim of validated error-focus behavior. - Inspected changed source, docs, tests and fixtures, all five commit messages, the description and all three screenshot attachments. The screenshots contain neutral disposable fixture data and are correctly labeled as older-head captures. No additional public-material finding identified in those inspected surfaces.
Validation limits
Source-only; no PR-code execution, tests, builds, installs, app launches or live channel mutations. Extracted inputs were checked against immutable Git blobs (156 files verified); no dirty checkout inputs were used. Read applicable repository/contribution/channel/session/design guidance and relevant block/buzz vision documents at d051350f693bfb158bbb560299d5764991b2e98b.
The reported 5,158 Vitest tests and 24 Chromium/WebKit cases are author-provided evidence, not reviewer-run checks. Three logical browser journeys are added (six engine cases), with modal/focus, persistent-owner handoff, broker/navigation and reload boundaries; permission/race matrices remain below the browser layer. No browser case is removed; the owner-agent case now tests the deliberately narrower contract. The description reports no fail-then-pass mutation experiment. CI was not assessed in this cycle. Live destructive integration, native behavior and final human acceptance remain unverified.
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord — automated source review via Wes's account
Reviewed head 7f0f985fff4362182f75f6ebd97f644851572727 against base 6b4de53359b852e14f4c5f31afb1bb61793e3dc2. This follow-up checks the restored profile-based eligibility/retry path and the previous findings. Two actionable P2 findings; comment-only, not approval or a merge gate.
- Reintroduced Settings retry focus loss — inline at
ChannelLifecycleActions.tsx:78–80. The new Delete-specific retry unmounts its focused control without a pending/result focus handoff. This is the previously reported Delete-retry defect, restored by the new implementation; the pre-existing generic permissions retry is not being counted as a new regression. - [P2] Prior public commit-metadata exposure remains unresolved. This repository is public. All eight current PR commits (
202fa417,485928e3,30265d9b,ed78da24,4b0918d0,bac95801,71b336ee,7f0f985f) still expose the internal deployment hostname linked to an agent's public-key identity in commit attribution/trailers. I am intentionally not repeating that identifier here. Use public-safe agent attribution consistently across the PR's author/committer metadata and trailers, preserving genuine human authorship and valid DCO certifications; correcting only a new tip does not remove the earlier exposure. Coordinate any history repair with the author. This is a public-material privacy finding, not a claim that a private signing key was exposed.
Scope and evidence
- The documented profile-provenance versus persisted relay-authorization distinction is explicit in the current implementation and description. The viewer still signs the command, fresh checks run before signing/publication, and definitive rejection does not remove the channel. I am not requesting a new relay capability, native parity, typed-name confirmation, or other scope expansion.
- Success/cancel handoff was inspected separately from failure/retry focus. The Settings panel focuses Close only on mount; sidebar restoration is tied to confirmation close/completion, not this permission retry. The new retry tests assert action visibility, not focus recovery.
- Browser accounting is consistent with source: three added logical journeys (six engine cases) versus base, covering Settings Archive/Delete and owner-agent Delete composition; the permission/profile/race matrix remains in lower-layer tests. The owner-agent rejection case checks channel retention and cancel focus, but does not cover optional-profile-check retry focus.
- Inspected the current description and all three attached images; the images show neutral disposable fixture data and are explicitly labeled as older-head captures. No additional publication-surface finding in those materials.
Limits: source-only review of immutable Git blobs; no dirty checkout inputs, PR-code execution, tests, builds, app launches, or live destructive operations. The PR's reported validation was not independently rerun, and CI was not assessed in this cycle. Real browser retry focus, live relay ownership agreement, native behavior, and final human acceptance remain unverified.
Reuse the shared permission check and lifecycle confirmation from channel Settings. Explain archive reversibility and match its non-destructive button style. Cover action-specific access, stale reads, cancellation, pending state, and navigation after completion. Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Verify fresh owner-agent attestations in the shared lifecycle gate before offering Delete and before signing and publishing. Keep ordinary admin, membership, and Archive rules unchanged. Simplify Delete confirmation and use non-destructive styling for Archive and Leave. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Keep independently verified Archive and Leave available when the optional Delete eligibility read fails. Bind the authenticated capability to community, viewer and channel, recheck before publication, and require archived channels to be restored before Delete. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Remove the optional Delete authority probe and its transport, broker and session wiring. Keep Archive and Leave governed by the existing signed channel state, and retain restore-before-Delete with fresh direct-owner checks before signing and publication. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Reuse signed profile provenance to offer owner-agent Delete while keeping the relay's stored ownership authoritative at publication. Preserve Archive/Leave on optional profile failure, expose an explicit retry, and retain restore-before-Delete. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Keep the recovery button mounted and focusable while refreshing permissions, without retaining stale actions. Hand focus to the retry or current result only when the user remains in recovery. Cover held reads and moved focus in component tests and both browser engines. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Use prominent primary actions for Archive, Leave and Hide confirmations. Keep Delete destructive and Cancel and Settings entry points unchanged. Signed-off-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
7f0f985 to
34f3be0
Compare
|
Implemented and pushed at
@wesbillman — ready for recheck of the focus finding. Unarchive is a newly requested follow-up, not part of this head. AI-generated implementation and report by Carl, posted at the author's request. |
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source review (via Wes’s account)
Follow-up reviewed at head 34f3be0b7cf4fb25ccae39c13376f90b30f9cac6, base 3bf5da762a85f3851d1e438c8bbf63ef9a441fe9. This is a non-blocking COMMENT, not approval or merge authorization.
Prior retry-focus finding: addressed in source
The Settings recovery control now stays mounted and focusable while busy, stale permissions are cleared, and the resolved result receives focus only when recovery still owns it (ChannelLifecycleActions.tsx:35–75,107–150; shared Button.tsx:44–55). Abort and channel/capability fences remain. I found no new material regression in this follow-up, including the prominent non-Delete confirmation change.
The regression matrix is colocated in ChannelLifecycleActions.test.tsx:324–426; importantly, tests/browser/channel-archive-delete-pane.spec.mjs:270–384 also holds the real profile request and asserts pending focus, repeated failure, a forbidden Delete result, success, and not stealing moved focus. The browser-specific boundary, four added logical journeys/eight engine cases overall, and this iteration’s one added journey/two cases are documented; none are removed. The PR records pre-fix failure and fixed success in both engines. These are appropriate layers for the change; the reported 5,354 unit tests and 36 browser cases are author-provided evidence, not runs I performed.
P2 — Prior public commit-metadata exposure remains unresolved
The issue from the earlier review remains in all nine current PR commits, including 34f3be0b7cf4: public attribution metadata and/or trailers expose an internal deployment hostname associated with an agent public-key identity. This is infrastructure/account-association exposure, not evidence of a leaked private signing key. I am deliberately not reproducing the identifiers.
Please coordinate a public-safe attribution/history repair across the full PR commit range, preserving truthful authorship and valid DCO certifications, then verify the hosted history and DCO check. Editing only the PR description or latest tip would leave earlier affected commits exposed. This review does not authorize rewriting another contributor’s history.
Scope and limitations
Reviewed immutable, Git-blob-verified source, relevant lifecycle/ownership documentation, the public description, all four attached images, and every PR commit’s attribution. The images show neutral fixture content; the older Settings image is explicitly labeled as layout context. I found no additional actionable disclosure in the changed files, description, or those images. One independent source-only lane was completed and reconciled.
No PR code, tests, builds, installs, app launches, or live operations were executed. No CI snapshot was assessed by this reviewer. The author’s browser and mutation evidence does not establish native/direct-signer behavior, live destructive acceptance, or human tryout; those remain explicitly deferred. Named-channel confirmation without typed-name entry and the deferred restore flow are retained product decisions, not reopened findings.
* origin/main: feat(channels): archive and delete channels from settings (#385) feat(updates): add in-app auto-updates with restart toast (#312) fix(ui): keep background loading from shifting populated views (#418) Improve member and agent identity previews (#412) Fix initial emoji autocomplete selection (#419) Add community membership settings (#348) Keep nested replies compact and place actions above message text (#367) Import an exact inventory identity from its selected source with retry (#288) ci: add gated macOS preview updater feed promotion (#414) Set up incomplete inventory identities through a working Use here dialog (#287) Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz> # Conflicts: # src/app/App.tsx
* origin/main: (58 commits) flake fix: keep restored reading anchor out of bottom follow (WebKit scroll measurement) (#407) Replace fixed browser-test waits with conditions, gates and the clock (#373) feat(updates): show installed version in Software Updates settings (#430) fix(desktop): allow deep-link delivery to the main webview (#432) feat(shell): open your profile from the account menu avatar (#390) Polish top bar and animate contextual sidebar toggle (#360) fix(profiles): preserve nonlocal agent identity in profile fallback (#327) test(agents): pause the status poll around the failed-Stop checks (#431) fix(sidebar): paint channel rows with the scroller contents (#428) feat(channels): archive and delete channels from settings (#385) feat(updates): add in-app auto-updates with restart toast (#312) fix(ui): keep background loading from shifting populated views (#418) Improve member and agent identity previews (#412) Fix initial emoji autocomplete selection (#419) Add community membership settings (#348) Keep nested replies compact and place actions above message text (#367) Import an exact inventory identity from its selected source with retry (#288) ci: add gated macOS preview updater feed promotion (#414) Set up incomplete inventory identities through a working Use here dialog (#287) Show saved local and relay inventory while retaining existing import controls (#286) ...
* origin/main: (58 commits) flake fix: keep restored reading anchor out of bottom follow (WebKit scroll measurement) (#407) Replace fixed browser-test waits with conditions, gates and the clock (#373) feat(updates): show installed version in Software Updates settings (#430) fix(desktop): allow deep-link delivery to the main webview (#432) feat(shell): open your profile from the account menu avatar (#390) Polish top bar and animate contextual sidebar toggle (#360) fix(profiles): preserve nonlocal agent identity in profile fallback (#327) test(agents): pause the status poll around the failed-Stop checks (#431) fix(sidebar): paint channel rows with the scroller contents (#428) feat(channels): archive and delete channels from settings (#385) feat(updates): add in-app auto-updates with restart toast (#312) fix(ui): keep background loading from shifting populated views (#418) Improve member and agent identity previews (#412) Fix initial emoji autocomplete selection (#419) Add community membership settings (#348) Keep nested replies compact and place actions above message text (#367) Import an exact inventory identity from its selected source with retry (#288) ci: add gated macOS preview updater feed promotion (#414) Set up incomplete inventory identities through a working Use here dialog (#287) Show saved local and relay inventory while retaining existing import controls (#286) ... Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Category: improvement
User Impact: Channel Settings offers Archive and Delete alongside Leave, including Delete for channels owned by your agent.
Problem: Archive and Delete were available only from the sidebar, and direct-owner-only gating omitted channels owned by the viewer's agent.
Solution: Reuse the shared lifecycle, confirmation and navigation owner from Settings. Offer owner-agent Delete using verified signed profile evidence, matching the existing desktop approach while keeping the relay as the final authorization authority.
Scope and current status
Head:
34f3be0b7cf4fb25ccae39c13376f90b30f9cac6; rebased onto main3bf5da76, preserving current channel-details, navigation, membership and fixture behavior.File changes
docs/channels.md
Document shared entry points, profile-based eligibility and its authorization limits, independent-action recovery and restore-before-Delete.
src/bundled/channels/ChannelLeaveButton.tsx → ChannelLifecycleActions.tsx
Offer independently permitted Leave, Archive and Delete from the existing permission load, with a focusable busy recovery control during retry. Stale actions are gated until the fresh result; focus moves to recovery or an allowed result only if the user has not moved elsewhere.
src/bundled/channels/ChannelLeaveButton.test.tsx → ChannelLifecycleActions.test.tsx
Retain Leave coverage and add role-specific entry, failure/retry and stale-result tests without introducing a second mutation owner.
src/bundled/channels/ChannelLifecycleDialog.tsx
Keep explicit named-channel confirmation without typing, reserve red for Delete and explain where Archive can be reversed.
src/bundled/channels/ChannelLifecycleDialog.module.css
Remove obsolete typed-confirmation styles.
src/bundled/channels/ChannelLifecycleMenu.tsx
Render optional Delete-check failure/retry alongside permitted sidebar actions.
src/bundled/channels/ChannelLifecycleMenu.test.tsx
Cover independent-action recovery, confirmation wording and button variants while retaining rejection/uncertain-delivery behavior.
src/bundled/channels/ChannelsPage.tsx
Hand the selected Settings action and trigger to the existing persistent confirmation/navigation owner; retain main's details editor integration.
src/features/relay/channel-lifecycle-protocol.ts
Require unarchived state for Delete and represent optional Delete-check unavailability independently of base permissions.
src/features/relay/channel-lifecycle.ts
Read exact channel-owner profiles and reuse the existing NIP-OA verifier for eligibility. Bound optional lookup to five seconds, preserve base actions on failure, and recheck before signing/publication. Keep the existing command, relay-receipt and readback flow.
src/features/relay/channel-lifecycle.test.ts
Cover valid/invalid profile evidence and conditions, membership/role boundaries, direct-owner behavior, optional failure/timeout, cancellation, changed profile evidence, archived-state races and a conflicting relay rejection.
tests/browser/channel-archive-delete-pane.spec.mjs
Exercise Settings Archive/Delete focus, pending state, navigation and reload. Verify owner-agent Delete in both surfaces, a definitive negative publication receipt without removal, then confirmed deletion through the production broker.
tests/browser/channel-leave-pane.spec.mjs
Assert prominent Leave confirmation, unchanged Settings/Cancel styling and initial Cancel focus.
tests/browser/channel-lifecycle.spec.mjs
Replace typed-name assertions with explicit confirmation/no textbox while preserving cancellation, navigation and reload coverage.
tests/browser/fixture.mjs
Provide signed owner-agent evidence, exact-author profile responses and normal production-broker discovery. Retain main's dynamic profiles, public metadata and agent-peer roster branches.
Reproduction steps
Screenshots
Earlier real built-product captures with dark theme and teal workspace accent, neutral disposable signed fixture data. The pictured viewer is the last direct owner, so Leave is absent. These illustrate the retained layout and confirmation contract, not a new capture of this head: Settings/Delete are from
a925bb2d; Archive is froma8930ecf. Mainline typography may differ.Settings
Current confirmations
Real built-product captures at
34f3be0b, dark theme, neutral signed disposable fixture data. Archive and Leave are prominent; Delete remains destructive. These are actual app dialogs, not component showcases. The older Settings capture above is retained only for layout context.Validation and limits
At clean
34f3be0b:git range-diffreports=). The earlier fixture/transport conflicts preserved main's channel-details and agent-peer behavior; final session/transport/broker files match main.Final human tryout and live integration acceptance remain open. No live channel was archived/deleted, no relay branch was changed or deployed, and PR readiness was left unchanged. Native/direct-signer parity, in-app archived browsing/restore and broader Delete audit/other-member acceptance remain outside this change.
Browser coverage accounting
Relative to main, four logical journeys are added (eight engine cases): Settings Archive, Settings Delete, owner-agent Delete across both entry points, and keyboard recovery through optional-profile retry. This iteration adds only that final journey (two engine cases), removes none. Browser-only boundaries include native modal/focus behavior, Base UI busy-control focus and unmount handoff, Settings-to-sidebar composition, publication receipts, navigation and reload. Permission/profile/condition/race matrices remain in unit tests.
Fail-then-pass evidence is recorded above. No mainline assertions were weakened. Native/live destructive acceptance remains deferred. Unarchive has been requested as a follow-up and is not implemented at this head.
Implementation and validation prepared with Carl (AI agent).