feat(ui): rename, delete, and toggle sidebar session groups - #101117
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85bcd65a74
ℹ️ 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".
| sessions.list({ activeMinutes: 0, limit: 0 }), | ||
| sessions.list({ activeMinutes: 0, limit: 0, showArchived: true }), |
There was a problem hiding this comment.
Page through all group members before patching
For groups with more than the default sessions.list page, rename/delete only updates the first page of active and archived rows: SessionCapability.list drops nonpositive limit/activeMinutes before sending the RPC, while the protocol requires limit and activeMinutes to be at least 1 and the gateway applies a bounded default. In a group with, for example, 101 active sessions named Research, the 101st session is never returned here, so it keeps the old category after Rename group or Delete group and the sidebar/store ends up split between old and new group names; this needs pagination using hasMore/nextOffset or another real unbounded/bulk path instead of limit: 0.
Useful? React with 👍 / 👎.
|
Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 7:41 PM ET / 23:41 UTC. Summary PR surface: Source +670, Tests +286, Docs 0. Total +956 across 49 files. Reproducibility: yes. Current main source shows the missing group-header controls, empty-group sidebar sections, and grouping-off preference, while the latest PR source and tests exercise those paths; I did not run a browser repro in this read-only review. Review metrics: 1 noteworthy metric.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Land the Control UI implementation after maintainer product acceptance and exact-head checks, preserving paginated active/archive enumeration and the existing gateway category contract. Do we have a high-confidence way to reproduce the issue? Yes. Current main source shows the missing group-header controls, empty-group sidebar sections, and grouping-off preference, while the latest PR source and tests exercise those paths; I did not run a browser repro in this read-only review. Is this the best way to solve the issue? Yes, as a patch shape. Keeping this in Control UI on top of the existing sessions.list pagination and sessions.patch category contract is narrower than adding a gateway bulk API, but maintainers still need to accept the new UX and preference. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c5161f96ce7c. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +670, Tests +286, Docs 0. Total +956 across 49 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (2 earlier review cycles)
|
Sidebar group headers gain a kebab + right-click menu with Rename group, New group, and Delete group. Rename/delete enumerate every member session (active + archived, across agents) via unbounded sessions.list queries and patch category per session; delete keeps sessions and moves them to Ungrouped. Stored-but-empty groups render as sections, and the sidebar sort popover gains a persisted Group by toggle (Custom groups / None).
…fault sessions.list caps an absent limit at SESSIONS_LIST_DEFAULT_LIMIT (100), so the rename/delete member enumeration now walks nextOffset pages explicitly; a silent cap would strand members in the old group on stores >100 sessions.
f071547 to
6b7ccd5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b7ccd51a9
ℹ️ 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".
| members.map((row) => | ||
| sessions.patch(row.key, { category }, { agentId: parseAgentSessionKey(row.key)?.agentId }), |
There was a problem hiding this comment.
Preserve the current session-list scope after bulk patches
When a group contains sessions from multiple agents, each member patch refreshes the shared SessionCapability state with the member's parsed agentId (ui/src/lib/sessions/index.ts:666-688). For example, renaming a group while viewing agent:main:* but with an archived agent:work:* member can leave context.sessions.state.agentId and sessionsResult hydrated for work if that patch finishes last, after which AppSidebar filters that result against the current main session and most recent rows disappear until another refresh. The group mutation should avoid per-member scoped refreshes or restore the original/current list scope once the bulk patch completes.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
…#101117) * feat(ui): rename, delete, and toggle sidebar session groups Sidebar group headers gain a kebab + right-click menu with Rename group, New group, and Delete group. Rename/delete enumerate every member session (active + archived, across agents) via unbounded sessions.list queries and patch category per session; delete keeps sessions and moves them to Ungrouped. Stored-but-empty groups render as sections, and the sidebar sort popover gains a persisted Group by toggle (Custom groups / None). * test(ui): capture sidebar group management UI proof shots * fix(ui): page session-group enumeration past the gateway's 100-row default sessions.list caps an absent limit at SESSIONS_LIST_DEFAULT_LIMIT (100), so the rename/delete member enumeration now walks nextOffset pages explicitly; a silent cap would strand members in the old group on stores >100 sessions. * chore(i18n): restore fallback-key tracking for new sidebar group strings after rebase * chore(i18n): translate new sidebar group strings across locales
Closes #101116
What Problem This Solves
Sidebar session groups (added in #100814/#100648) are write-once: a group can be created and sessions assigned to it, but there is no way to rename a group, delete one, keep an empty group visible, or turn grouping off. Stale group names linger in the move-to-group submenu forever, and reorganizing means re-assigning every member session by hand.
Why This Change Was Made
Group headers get the same management affordance session rows already have: a hover kebab plus right-click menu with Rename group, New group, and Delete group. Rename and delete enumerate every member session with unbounded
sessions.listqueries (active + archived, across agents) and patchcategoryper session — the windowed sidebar list can miss members, and archived sessions must keep the new name so restores land in the right group. Delete keeps the sessions and moves them back to Ungrouped. Stored-but-empty groups now render as sections so a freshly created group is a usable move target. The sidebar sort popover gains a Group by toggle (Custom groups / None) persisted per browser profile; None flattens categories into one list while Pinned stays separate. No gateway/protocol changes: group membership is already per-session server state viasessions.patchcategory.Non-goals: the Sessions page table keeps its existing group-by UI (it reflects renames automatically since categories are server-side), and iOS/Android group-management parity is follow-up work.
User Impact
Sidebar groups are now maintainable: rename a group in place, delete it without losing sessions, create an empty group before filling it, and switch the sidebar to one flat recent list when grouping is unwanted. Group renames carry archived sessions along, so restoring an archived session lands in the renamed group.
Screenshots
Evidence
pnpm test ui/src/lib/sessions/custom-groups.test.ts ui/src/lib/sessions/grouping.test.ts— 19/19 passed (run 28813504291).pnpm test ui/src/e2e/session-management.e2e.test.ts— 4/4 passed, including the newrenames, deletes, and toggles sidebar session groupscase asserting per-membersessions.patchcalls (archived member included) and the flat-list toggle (run 28811188010).pnpm tsgo:core:allandpnpm check:test-typesboth green.pnpm check:changedguard lanes green; its diff-based warning lane aborts on the known Testbox no-merge-base sync quirk, so the type/test lanes above were run explicitly.pnpm ui:i18n:syncregenerated locale bundles +.i18nmetadata for the new strings (English fallbacks recorded infallbackKeys, same as prior sidebar strings);pnpm ui:i18n:checkclean on Testbox (run 28814349995).docs/web/control-ui.mdupdated.