[codex] add bulk archive action for desktop sessions - #39376
Conversation
Review: bulk archive sessionsVerdict: approve — no blockers found. The implementation is correct, well-structured, and follows existing patterns (auto-archive, etc.). Findings (all non-blocking)1. Redundant preserve logic in 2. Dialog stays open on error (empty catch in 3. Dialog description could document grace period 4. Test edge-case gaps
|
47c480b to
41c2d9f
Compare
|
Updated this PR after local desktop validation:
This keeps the safety boundary on explicit preserved IDs from the UI: pinned/current/working sessions remain visible, but old lazy/open-ended rows no longer masquerade as unarchiveable live sessions by default. |
41c2d9f to
78d684a
Compare
|
Follow-up update after rebasing onto current
Verification on rebased head |
|
Live desktop verification refresh: /Applications/Hermes.app is now installed from local combined commit 47c00a1, which layers this PR's bulk archive branch with Omar's local thinking-off defaults and the separate bootstrap/update protection from PR #39192. Bulk-archive-specific evidence from the final live app:
I also found and patched a PR #39192 edge while doing this: plain local stamps without pin metadata were not protected from auto-update. That fix was pushed separately to #39192 as 2b50aebe7 so this bulk archive PR stays scoped. |
|
Final live-install correction/proof after folding the bulk-archive work into the combined local desktop bundle:
The previous unsafe |
Review SummaryOverall: Solid implementation. Addresses the user request well with proper safety rails (pinned/current/running preservation, rollback on failure, confirmation dialog). A few findings below. Blocker: Compression continuation tips not archived (hermes_state.py:1578)collects (= ) for archival, but does . For compression continuation sessions, uid=501(obaradei) gid=20(staff) groups=20(staff),12(everyone),61(localaccounts),79(_appserverusr),80(admin),81(_appserveradm),33(_appstore),98(_lpadmin),100(_lpoperator),204(_developer),250(_analyticsusers),395(com.apple.access_ftp),398(com.apple.access_screensharing),399(com.apple.access_ssh),400(com.apple.access_remote_ae),701(com.apple.sharepoint.group.1) = tip's id, while = root's id. Only the root gets archived; the tip rows retain and will reappear in on the next load. Fix: Either (a) append both and to , or (b) add a secondary UPDATE matching . Concern: may undercount in profile scope (use-session-actions.ts:875)When , the server archives across all profiles. The client sets which only reflects the current profile's store. The subsequent corrects this, but there's a brief flash of incorrect count. Not blocking since it self-corrects. Suggestion: Add test for compression continuation archivalThe existing tests cover preserve_ids, recent live sessions, open-ended rows, and scope. Missing: a test verifying that compression continuation tips are archived alongside their roots. Nit: Dialog count shows which is profile-scoped (sidebar/index.tsx:799)When , the dialog shows the current profile's count but the server archives across all profiles when . Minor UX mismatch. |
Review SummaryOverall: Solid implementation. Addresses the user request well with proper safety rails (pinned/current/running preservation, rollback on failure, confirmation dialog). A few findings below. Blocker: Compression continuation tips not archived (hermes_state.py:1578)
Fix: Either (a) append both Concern:
|
78d684a to
482f949
Compare
482f949 to
a97ee30
Compare
|
Maintainer-ready after refresh. I merged current Verification:
MeshBoard merge dry-run passes with green checks and a fresh base. I attempted the actual merge through |
10fa20a to
1719dc9
Compare
ede6041 to
239cbab
Compare
|
Update on the recent pushes: refreshed the branch onto current upstream/main and fixed the real CI failure ( |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused desktop workflow contribution. The manual bulk action fills a capability current main does not expose, but three correctness issues need resolution.
Problems
hermes_state.py:2879-2900archives only the lineage root through an ID-only update. A projected compression tip remains unarchived; currentset_session_archived()recursively updates the full lineage athermes_state.py:3005-3053.hermes_state.py:2867-2874has no source exclusions. Desktop Recents excludes cron, subagent, tool, and messaging sources inapps/desktop/src/app/session/hooks/use-session-list-actions.ts:34-42, so this action can archive rows outside its visible scope.- The sole control is guarded by
!showAllProfilesatapps/desktop/src/app/chat/sidebar/index.tsx:1283; the documented__all__path cannot be invoked from the unified sidebar.
Suggested changes
- Use a lineage-aware archive operation and test root-plus-tip archival.
- Apply the Recents exclusions and test cron/messaging remain untouched.
- Expose and test the All Profiles action, or narrow the feature to one profile.
Automated hermes-sweeper review.
|
|
||
| archive_ids.append(target_id) | ||
|
|
||
| return self.archive_sessions_by_ids(archive_ids) |
There was a problem hiding this comment.
archive_ids contains the lineage root for a projected compression tip, but archive_sessions_by_ids() only updates WHERE id IN (...). The tip remains unarchived and can resurface. Route this through the same recursive lineage operation as set_session_archived() and add a root-plus-tip regression test.
| archive_ids: List[str] = [] | ||
| seen_targets = set() | ||
|
|
||
| sessions = self.list_sessions_rich( |
There was a problem hiding this comment.
This selection has no source exclusions, so it includes cron, tool/subagent, and messaging rows that the desktop deliberately keeps outside Recents. Please scope this to the same source set as the sidebar and cover those excluded sources in the endpoint test.
| @@ -1247,6 +1279,30 @@ export function ChatSidebar({ | |||
| </div> | |||
| ) : ( | |||
| <div className="flex shrink-0 items-center gap-0.5"> | |||
| <div className="grid size-6 shrink-0 place-items-center"> | |||
| {!showAllProfiles && agentSessions.length > 0 ? ( | |||
There was a problem hiding this comment.
This condition hides the only bulk-archive entry point in All Profiles mode, so the profile="__all__" path is unreachable despite being implemented below the UI. Expose the action in that mode or remove the cross-profile behavior and update the stated scope.
04cd62d to
683c73b
Compare
31a19fa to
e17d5a4
Compare
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Upstream main added a filter-driven SessionDB.archive_sessions( older_than_days, source, **filters) after this branch introduced an ID-list-based method of the same name. Both definitions coexisted in the class after the upstream merge with no textual conflict, and the later (filter-driven) def silently shadowed this branch's method, so archive_surfaced_sessions passed its ID list into older_than_days and list_prune_candidates raised TypeError (float - list) — the CI failure in the three bulk-archive endpoint tests. Rename this branch's method to archive_sessions_by_ids and point archive_surfaced_sessions at it; the upstream filter-driven API keeps the archive_sessions name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
upstream retired apps/desktop/src/app/desktop-controller.tsx (369d0ee, "views as contributions"); the rebase resurrected it from this branch's two-line edit. Drop the file again and re-apply the same intent where the sidebar is now wired: SidebarActions gains onArchiveAllSessions, the latest-actions adapter forwards it, and contrib/wiring.tsx supplies archiveAllSessions().then(refreshSessions) — identical behavior to the retired controller's prop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Marker blocks in use-session-actions, sidebar/index, and hermes.test were committed unresolved. Imports merged as unions; archive-all adapted to upstream's store API (workingSessionIds now lives in session-states, sessionsTotal is gone - the dialog count and the optimistic update now work off the loaded session list). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
e17d5a4 to
5ce5ef7
Compare
Why
The desktop sidebar can show hundreds of normal sessions, and one-at-a-time archive is not viable at that scale. This adds an explicit manual bulk archive action so users can clear the unpinned session list without deleting transcripts.
This is intentionally separate from the old-session auto-archive policy work in #39271. It uses the archive storage/API behavior already present on
mainand only adds the manual archive-all path.What changed
SessionDB.archive_sessions()andarchive_surfaced_sessions()helpers for bounded soft-archiving of surfaced conversations.POST /api/sessions/bulk-archive, registered before/api/sessions/{session_id}routes.active_grace_secondsstill available as an opt-in guard for callers that want recent open-ended rows protected.bulkArchiveSessions()helper plus anarchiveAllSessions()action that preserves pinned IDs, working sessions, the selected chat, and the active runtime session.Behavior
The action soft-archives eligible unpinned sessions in the current sidebar scope; it does not delete messages. Pinned chats, the current chat, and working sessions remain visible because the desktop sends those IDs explicitly. Stale open-ended rows are archived by default instead of masquerading as live sessions forever.
Repair note
Rebased the upstream PR branch onto current
upstream/main(955fa4006287) as a clean one-commit cherry-pick:a97ee306f8b9c02fe8edb206aba23d87f22489a8. The only conflict was the desktop sidebar sessions section; the resolution keeps the current upstream sidebar structure and wires the archive-all button through the existing recents header action.Evidence