Repository navigation
Add Mark as Read and archive/delete to Solo chats, with chat wording - #1311
Conversation
Solo chat rows built their own menu and never got the shared read and lifecycle items the Team row menu has. Reuse ChannelReadMenuItem and ChannelLifecycleMenu; confirmation goes through the shared lifecycle dialog. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Bradley Axen <baxen@squareup.com> Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz>
Archive, delete and leave items, their confirmations and result toasts said "channel" for Solo chats. Derive the noun from the channel type wherever the lifecycle actions render, and label archived Solo search results as chats. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Bradley Axen <baxen@squareup.com> Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9284372d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| handoff.openLifecycle( | ||
| channel, | ||
| action, | ||
| trigger, | ||
| ), |
There was a problem hiding this comment.
When the affected chat is currently open, this call supplies neither the visit origin nor Solo-owned completion callbacks, so ChannelNavigationProvider falls back to the callbacks published by ChannelInventory: archiving leaves the user on the now-archived chat, while deleting navigates to the first Team channel or the empty Channels page. The same operations from ChannelsPage deliberately navigate to a new Solo draft, so the new row entry point should use that Solo lifecycle completion policy rather than Team inventory behavior.
AGENTS.md reference: AGENTS.md:L28-L31
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 Fixed in edea96e: the row menu supplies Solo completion callbacks; leaving the open chat opens a new Solo draft unless the visit has moved on.
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/accessibility) reviews.
8/10 — two recommended fixes: the inline Solo Archive destination defect, and the browser assertion race below. This is a non-blocking COMMENT review; humans decide merge readiness.
[P2] Wait for lifecycle permissions before comparing Solo menus. In tests/browser/solo-chats.spec.mjs:58–69, both menu snapshots follow only popup visibility. This PR adds ChannelLifecycleMenu, whose items appear after an asynchronous permission read on each mount. The first snapshot can include Archive/Delete while the reopened popup is still loading. The exact-head Chromium 7/9 job fails at line 69 with precisely those two items missing. Wait for a deterministic permissions-ready condition on both openings before capturing/comparing the complete action list; retain the menu-parity and focus assertions. This follows the repository’s deterministic-test rule and does not require weakening the assertion.
The two other inline notes are optional: preserve independent read/project error notices and clarify the scope of archiving a shared chat. The declared selected-Delete fallback to Team is recorded as an existing integration limitation, not duplicated as another finding.
Reviewed head d9284372d55ca69bcf1c7536ee6e030ec9ada0ad against base dc305c9e3bff304719cde2d06994a4c163c096db, using immutable source objects. Shared Menu, lifecycle capability/dialog and navigation owners remain reused; permission refresh, cancellation, retry/error paths and menu mounting were traced. The public PR description and both commit messages were inspected; no attached images or disclosure finding.
Validation: source review only; no PR code, tests, builds or app were executed. Diff whitespace check passed. Hosted CI snapshot: CI required failed; Chromium 7/9 failed as described, and WebKit 6/9 failed notifications.spec.mjs:290 (expected zero system notifications, observed one). The latter’s cause remains unresolved. Other reported checks passed except skipped Windows validation. Rendered themes, viewport/enlarged-text behavior, keyboard/screen-reader acceptance, native/live relay behavior and renewed human wording acceptance remain unverified.
| handoff.openLifecycle( | ||
| channel, | ||
| action, | ||
| trigger, | ||
| ), |
There was a problem hiding this comment.
🤖 [P2] Preserve the Solo destination after archiving the selected chat. This call supplies no completion callback, so ChannelNavigationState falls back to ChannelInventory’s generic callback, which explicitly returns without navigating for Archive. When the user archives the open Solo chat from this new row menu, its sidebar row disappears but ChannelsPage retains the archived conversation. The existing header/shortcut opens soloDraftTarget(scope), matching docs/solo/README.md’s “Success returns to Solo” contract. Supply Solo-aware completion when this is still the selected chat, guarded against overwriting a newer visit; archiving a different row should leave the current conversation alone.
There was a problem hiding this comment.
🤖 Fixed in edea96e: archive/delete of the selected chat from its row opens soloDraftTarget(scope), guarded by the visit's signal; other rows leave the current conversation alone. Covered by a new ChannelsPage.solo test (open vs another chat), which fails without the fix.
| closeOnClick | ||
| run={(action) => | ||
| action().then( | ||
| () => setProjectError(""), |
There was a problem hiding this comment.
🤖 Optional — keep read failures separate from project failures. A successful read clears projectError even if another chat’s read or a project deletion has since failed. Both operations use this same state slot, and pending reads can settle out of order. Give read feedback its own attempt-scoped state (or otherwise preserve unrelated errors), so success on chat B does not silently dismiss the failure on chat A or a failed project deletion.
There was a problem hiding this comment.
🤖 Fixed in edea96e: read failures have their own attempt-keyed alert; a success only clears its own chat's failure, and project errors are untouched.
| "Hide this conversation from your sidebar only. Messages and membership are kept; other participants are not removed.", | ||
| chat: { | ||
| archive: | ||
| "Archive this chat and remove it from the sidebar. Messages are kept. Find it in search to unarchive it later.", |
There was a problem hiding this comment.
🤖 Optional — retain the shared-chat scope in Archive confirmation. Solo chats can be shared, and archive still changes relay-wide channel state for every participant. The new copy drops the previous “for everyone” clarification while Delete correctly says “for everyone in it”. Consider “Archive this chat for everyone in it and remove it from the sidebar…” so this cannot be mistaken for a personal hide action.
There was a problem hiding this comment.
🤖 Fixed in edea96e: "Archive this chat for everyone in it and remove it from the sidebar…"
Row-menu archive/delete used the Team inventory's completion, which stayed on an archived chat or jumped to a Team channel. Match the chat header: leaving the open chat opens a new Solo draft unless the user has moved on. Also keep read failures apart from project errors, say archive applies to everyone in a shared chat, and wait for lifecycle permissions before comparing Solo menus. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com>
|
🤖 Review follow-up in edea96e. Browser P2: |
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/accessibility) reviews.
9/10 — no changes requested in this follow-up. The two recommended fixes from the prior review are addressed:
- Solo row lifecycle actions now supply Solo-owned completion, return the still-current selected chat to a new Solo draft, and preserve other/newer destinations through the captured navigation-attempt guard (SoloProjects.tsx:201–226).
- Both menu snapshots wait for the permission-dependent Delete item before comparing the full action list (tests/browser/solo-chats.spec.mjs:57–74). The existing shared permission owner mounts these items together.
The optional read/project error separation and explicit shared-chat archive scope are also addressed. Shared Menu, lifecycle dialog, navigation and unread owners remain reused; cancellation, failure/retry and stale-completion behavior were traced.
Reviewed head edea96ebbda2fc302151e67b18ac6fcea18515f2 against base dc305c9e3bff304719cde2d06994a4c163c096db, focusing on the five-file delta from previously reviewed d9284372d55ca69bcf1c7536ee6e030ec9ada0ad and its callers. Unchanged feature areas were not exhaustively re-reviewed. Public PR description and all three commit messages inspected; no attached images or actionable disclosure finding.
Validation: source review only; no local PR code, tests, builds or app executed. Diff whitespace check passed. Hosted logs at this head show the Solo menu journey passed ten repetitions in each engine: Chromium and WebKit; each selected flake job passed 20 cases total. The single check snapshot still had JavaScript and browser shards running, with Windows skipped. This is not an all-CI-green claim. Rendered themes/viewport/enlarged-text, screen-reader, native/live-relay and renewed human wording acceptance remain unverified. Non-blocking COMMENT only; no merge approval.
Summary
Solo chat rows built their own menu and never got the read and lifecycle items that Team rows have. Lifecycle wording also called Solo chats "channels".
ChannelReadMenuItem) and the lifecycle items (ChannelLifecycleMenu: archive/unarchive, delete, leave, depending on permissions). Confirmation goes through the shared lifecycle dialog (handoff.openLifecycle). Read-save failures get their own Solo sidebar alert. Archiving or deleting the open chat from its row returns to a new Solo draft, like the chat's header menu; other rows leave the current page alone.nounprop (lifecycleNoun(channel)→"chat"forchannelType === "solo") onChannelLifecycleMenu,ChannelLifecycleActionsandChannelLifecycleDialog. It applies to the Solo row, the Solo header's Chat actions, the regular header ⋯, Team rows showing a shared Solo chat, and settings. Dialog copy, pending/result toasts and the "Archived chat" search detail follow it. Channel wording is unchanged.Mute is not added to Solo rows.
Testing
vitest run(full suite): 10103 passed.channel-lifecycle,channel-header-menu,inbox-channel-archive,channel-archive-delete-panein Chromium + WebKit: 38 passed. The Solo lifecycle journey is updated to the chat wording.solo-chats.specwaits for lifecycle permissions on both menu openings before comparing them; 4× repeat in Chromium + WebKit passed.🤖 Generated with Claude Code