feat(sessions): archive worker sessions from the sidebar row#180
feat(sessions): archive worker sessions from the sidebar row#180ashish921998 wants to merge 4 commits into
Conversation
Terminated workers can now be archived to declutter the sidebar without
destroying the row. Right-clicking a terminated worker offers Archive
next to Restore; archived workers move behind a collapsed "Archived (n)"
disclosure at the end of the project's worker list, where Unarchive
brings them back.
Daemon side, sessions gain a nullable archived_at (migration 0011,
mirroring the projects soft-delete pattern) with POST
/sessions/{id}/archive and /unarchive routes and an `archived` list
filter. Archiving requires a terminated session (409
SESSION_NOT_TERMINATED otherwise) and restore clears the flag, so a
running agent can never be hidden. The sessions CDC update trigger is
recreated so archive flips fan out session_updated events; the full
UpdateSession write deliberately leaves archived_at untouched so
lifecycle writes can't clobber user intent.
Also removes a stray aria-hidden on the context-menu overlay that hid
every menu item (kill/restore included) from the accessibility tree.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e archive UI to the new shell The base branch now carries main's agent-orchestrator-clone shell, which deleted App.tsx/SideRail.tsx and rewrote the Sidebar this feature had wired its archive UI into. The backend is untouched by the redesign; the UI re-lands on the new surfaces: - Sidebar worker rows get a right-click context menu (the ui/context-menu component survived the redesign): Archive on terminated workers, Unarchive on archived ones, no menu on running workers. - Archived workers move behind a per-project "Archived (n)" disclosure at the end of the session sub-list, dimmed, still navigable. - useWorkspaceQuery splits archivedSessions out of sessions so the kanban board, sidebar counts, and every other default surface ignore archived workers; SessionView's lookup includes them so their rows still open. - archive/unarchive mutations live in the _shell route beside createProject/createTask and invalidate the workspace query. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Greptile SummaryThis PR adds archive/unarchive for terminated worker sessions — a soft-hide feature that moves workers behind a collapsible "Archived (n)" disclosure in the sidebar without destroying the row. The daemon gains a nullable
Confidence Score: 4/5Safe to merge with one path worth verifying before shipping: the auto-unarchive on restore depends on the real session manager returning a SessionRecord with ArchivedAt populated, which the unit test covers via an injected fake but the real manager implementation should be confirmed. The archive feature is well-structured with atomic SQL guards, a consistent service-layer pattern, and solid test coverage across store, service, controller, and frontend layers. The one concern is that Service.Restore conditionally calls UnarchiveSession only when rec.ArchivedAt returned by manager.Restore is non-zero — if the real manager builds its returned record without reading archived_at from the DB, an archived-and-restored session will silently remain archived in the DB and hidden from default lists while running. The rest of the changes (migration, CDC trigger, sidebar UX, accessibility fix) are clean and low risk. The real implementation of manager.Restore (not in this diff) should be checked to confirm it reads or forwards the full archived_at column so Service.Restore's auto-unarchive actually fires. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Sidebar
participant Shell
participant API
participant Service
participant Store
participant CDC
User->>Sidebar: right-click terminated worker row
Sidebar->>Sidebar: sessionMenuItems() → [Archive worker]
User->>Sidebar: click "Archive worker"
Sidebar->>Shell: onArchiveSession(sessionId)
Shell->>API: "POST /api/v1/sessions/{id}/archive"
API->>Service: Archive(ctx, id)
Service->>Store: GetSession(ctx, id)
Store-->>Service: "rec (IsTerminated=true, ArchivedAt=zero)"
Service->>Store: "ArchiveSession WHERE is_terminated=1 AND archived_at IS NULL"
Store->>CDC: trigger session_updated event
Store-->>Service: (rows, nil)
Service-->>API: nil
API-->>Shell: "200 OK {ok:true, sessionId}"
Shell->>Shell: invalidateQueries(workspaceQueryKey)
Shell->>Sidebar: workspaces with archivedSessions split out
User->>Sidebar: click "Archived (n)" disclosure → expand
User->>Sidebar: right-click archived row
Sidebar->>Sidebar: sessionMenuItems() → [Unarchive worker]
User->>Sidebar: click "Unarchive worker"
Sidebar->>Shell: onUnarchiveSession(sessionId)
Shell->>API: "POST /api/v1/sessions/{id}/unarchive"
API->>Service: Unarchive(ctx, id)
Service->>Store: GetSession(ctx, id)
Store-->>Service: "rec (ok=true)"
Service->>Store: UnarchiveSession WHERE archived_at IS NOT NULL
Store->>CDC: trigger session_updated event
Store-->>Service: (rows, nil)
Service-->>API: nil
API-->>Shell: "200 OK {ok:true, sessionId}"
Shell->>Shell: invalidateQueries(workspaceQueryKey)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if !rec.ArchivedAt.IsZero() { | ||
| if _, err := s.store.UnarchiveSession(ctx, id, s.now().UTC()); err != nil { | ||
| return domain.Session{}, fmt.Errorf("unarchive restored %s: %w", id, err) | ||
| } | ||
| rec.ArchivedAt = time.Time{} | ||
| } |
There was a problem hiding this comment.
manager.Restore populating ArchivedAt
rec here comes from s.manager.Restore, not from a direct store read. If the underlying manager implementation constructs its returned SessionRecord from in-memory or partial state (rather than re-reading the full row from the DB), rec.ArchivedAt will always be zero and the auto-unarchive block is silently skipped — leaving a now-running session still hidden from every default surface.
The unit test covers this path correctly by injecting a record with ArchivedAt set via archivedRestoreCommander, but that test doesn't prove the real manager returns a populated ArchivedAt. Since the manager code is out of scope for this PR, it's worth verifying that manager.Restore reads the full session row (now including archived_at after migration 0011) or explicitly sets ArchivedAt on its returned record, or alternatively reading the session via s.store.GetSession after the restore to obtain the authoritative value.
| INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) | ||
| VALUES (NEW.project_id, NEW.id, 'session_updated', | ||
| json_object('id', NEW.id, 'activity', NEW.activity_state, 'isTerminated', json(CASE WHEN NEW.is_terminated THEN 'true' ELSE 'false' END)), | ||
| NEW.updated_at); |
There was a problem hiding this comment.
isArchived
The trigger fires correctly when archived_at changes, but the json_object payload only carries id, activity, and isTerminated. A session_updated event triggered by an archive or unarchive flip gives downstream consumers no way to derive the new archive state from the event alone — they must issue a separate GET /sessions/{id} to learn it. The frontend currently invalidates and re-fetches on every archive action, so this doesn't break the current UI. Future CDC consumers that expect the payload to be a self-contained diff (as isTerminated already is) will silently receive stale data on archive flips. Adding isArchived: CASE WHEN NEW.archived_at IS NOT NULL THEN json('true') ELSE json('false') END to the payload would make archive flips consistent with the existing terminated-state field.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| /** | ||
| * Archived workers, split out so every default surface (sidebar list, side | ||
| * rail, counts) ignores them; the sidebar shows them behind an | ||
| * "Archived (n)" disclosure. | ||
| */ | ||
| archivedSessions?: WorkspaceSession[]; |
There was a problem hiding this comment.
archivedSessions is typed as optional (archivedSessions?) but useWorkspaceQuery always populates it unconditionally. Every call site that accesses it already uses ?? [] to guard the optional, adding noise. Making the field required avoids scattered defensive fallbacks and keeps the type honest with how it is actually built.
| /** | |
| * Archived workers, split out so every default surface (sidebar list, side | |
| * rail, counts) ignores them; the sidebar shows them behind an | |
| * "Archived (n)" disclosure. | |
| */ | |
| archivedSessions?: WorkspaceSession[]; | |
| /** | |
| * Archived workers, split out so every default surface (sidebar list, side | |
| * rail, counts) ignores them; the sidebar shows them behind an | |
| * "Archived (n)" disclosure. | |
| */ | |
| archivedSessions: WorkspaceSession[]; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| onSessionContextMenu: (event: React.MouseEvent, session: WorkspaceSession) => void; | ||
| }) { | ||
| const projectActive = selection.activeProjectId === workspace.id && !selection.activeSessionId; | ||
| const archivedSessions = workspace.archivedSessions ?? []; |
There was a problem hiding this comment.
archivedSessions always present on the workspace (built unconditionally in useWorkspaceQuery), the ?? [] fallback is a dead branch. If the type is made required (see workspace.ts comment), this can be simplified to remove the unnecessary fallback.
| const archivedSessions = workspace.archivedSessions ?? []; | |
| const archivedSessions = workspace.archivedSessions; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
I don't think we want to bring back archiving worker sessions. We can kill a session and restore it anyways so there is no need of archiving. |
2 participants
What
Terminated workers can now be archived to declutter the sidebar without destroying the row, and unarchived to bring them back.
archived_at(migration 0011, mirroring the projects soft-delete pattern);POST /sessions/{id}/archiveand/unarchiveroutes; anarchivedfilter onGET /sessions;isArchivedon the Session read model.SESSION_NOT_TERMINATED) and restore clears the flag, so an active agent can never be hidden. The fullUpdateSessionwrite never touchesarchived_at, so lifecycle writes can't clobber user intent. The sessions CDC update trigger is recreated so archive flips fan outsession_updatedevents.useWorkspaceQuerysplitsarchivedSessionsout ofsessions, so the kanban board, sidebar counts, and every other default surface ignore archived workers automatically;SessionViewstill resolves them so archived rows open normally.Notes
feat/wire-core-workflows); retarget tomainonce that merges. The UI was originally built on the old emdash shell and re-landed on the new agent-orchestrator-clone shell in the merge commit.ui/context-menucomponent (from feat: wire the core workflows — daemon status, orchestrator attach, git rail, lifecycle #178) hadaria-hiddenon its overlay, hiding every menu item from the accessibility tree — fixed here; the new tests query menu items by role.ao session list) shows archived sessions with no marker; a CLI flag/column is a possible follow-up.Verified
npm run lint(backend tests + golangci) green,go test -raceon touched packages green, spec drift/route parity green,npm run apiproduces no diff🤖 Generated with Claude Code