feat(desktop): clean up archived tasks by age and project - #5896
Conversation
Settings > Archived tasks could only search by name and delete the
whole matched set, and its confirm stated a count and fixed copy about
subtasks without asking the Host what the delete would take with it.
Add "archived more than 7/30/90 days" and project filters ("All
projects", each project with archived tasks, "No project"), combined
with the search box. The age filter reads archivedAt only: tasks with
an unknown archive time are left out while it is on, and the page says
how many. The bulk delete always acts on exactly the rows on screen and
reads "Delete N shown" whenever anything narrows the list; unnarrowed
it keeps "Clear all".
The confirm opens at once with the count and fills in a Host preview:
child tasks deleted with them (Agent Graph operators), subagent
worktrees retired, linked subtasks kept and archived, and an estimate
of the bytes stored, measured with M1's per-Session footprint reader
over every Session the removals delete. Delete waits for the preview;
if it fails the dialog still names the count and can be cancelled or
confirmed. The ids it deletes are the ones it previewed, frozen at the
click. Execution is unchanged: one session.remove per task with
requireArchived, so a task restored meanwhile is kept.
session.remove.preview now takes a list of 1-25 unique sessionIds (the
storage.usage.sessions.query cap) and unions the same removal plans
session.remove executes, adding removedSubtaskCount, worktreeCount and
bytes. The preload pages a selection per Host into bounded requests
and rejects rather than report a partial sum. Protocol epoch 201 ->
202; there is no compatible-change declaration, so an older Host fails
admission.
The filter state and the confirm live in a new archived-task-cleanup
feature slice; the legacy Settings page loses its own search and purge
state (two useState and useMountedRef) and renders the rows it is
handed.
Refs apache#5825
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Address the review of the scoped cleanup of archived tasks. The bulk delete now follows the single-delete pattern instead of a confirmation state machine: the button shows loading, the rail's new purgeArchived row action awaits the batch preview and opens toast.confirm with it, falling back to the existing uncertain subtask note when the preview fails. The ids are frozen at the call, and a preview that settles after the page closed or the scope changed opens no dialog. The outcome report moved there from the page. The archived-task-cleanup slice, its port, context and platform adapter are gone. previewRemovals sits beside previewRemoval on the session-navigation port, and the filter model and ArchivedTaskScope wrapper live in that feature. Settings mounts the wrapper around the page, which receives the visible rows and controls as a prop and keeps its list unindented. Project filters use the rail's Host-scoped identity, runtimeHostProjectKey with alias resolution, so equal ids on two Hosts stay apart and aliases land on their project; ArchivedTasks now carries the Task Entry project scopes and the row-action ref. session.remove.preview takes optional measureBytes and requireArchived. Single delete does not measure; the batch purge measures archived targets only. bytes is optional and is omitted when not requested or when measurement fails, so the counts still return. The Host reads listHeaders once per request and derives every plan from that snapshot, and sizes removed Sessions in pages of at most 25 with a yield between. SESSION_REMOVE_PREVIEW_MAX_ITEMS is its own constant. session.remove takes requireArchivedForMs. The Host keeps a task unless its archivedAt is known and older than the threshold by the Host's clock, answering too_recent, which Desktop counts as kept. An age filter passes its threshold; the Desktop filter is display only. The preload paging for storage usage and removal previews shares one group-by-Host helper: Hosts run side by side, one page in flight per Host, and each caller keeps its own failure policy. Protocol epoch stays at 202 against main's 201. Refs apache#5825 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The scoped cleanup moved the last public consumers of `deriveSessionRail` and `SessionPurgeOutcome` inside the slice, so Knip reports both entry exports as unused. Tests keep importing `deriveSessionRail` through `testing.ts`. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed commit 800143f. This adds archived-task search, age and project filters, and a confirmed bulk deletion flow. The renderer freezes the displayed IDs and accounts for removed, restored, too-recent, and unresolved tasks (archived-task-scope.ts:91,159; session-row-actions.ts:324,406,462). The preload groups bounded preview requests per Host (session-removal-preview.ts:51; host-session-pages.ts:57), while the Host previews a union of removal plans and rechecks the archive-age condition under its removal admission gate (session-retirement-coordinator.ts:348,422). The protocol advances to epoch 202.
I found no substantiated P0–P3 issue in the inspected paths. Node 24 build:test and 85 focused Desktop/Runtime Host tests passed. The current-head hosted test is successful; fresh main c838e1f is mergeable and diff-check clean. I did not run a packaged Electron app, test real multi-Host disconnects, or run the full test suite. This is a code review, not merge approval.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed exact head 800143f46a47b944f84885e54e520db00d6ddd19 as an independent second pass focused on the bulk-delete safety boundary.
I found no path that removes a task outside the confirmed set. The renderer freezes the displayed IDs at confirmation, so a task archived later never joins the batch. Each removal re-reads the target, and the Host compares the revision inside the removal admission gate, so a restore cannot slip between check and delete. The Host also rechecks archive age under that gate (session-retirement-coordinator.ts:348-354). Unarchiving clears archivedAt, so a restored-then-re-archived task is kept as too recent. Tasks with no recorded archive time (archived before the schema that added it) are excluded under an age filter on both sides and only included under "Any time". Both sides compare plain epoch milliseconds, so time zones do not matter. The project filter is enforced only through the frozen IDs, not re-checked on the Host; that is sound, but worth a sentence in the code.
The protocol moves to epoch 202. Main is at 201, and other open PRs also claim 202, so whichever lands later must renumber.
Findings are P3 only, inline. The most user-visible one is the silent project-filter fallback to all projects after a scoped purge. Missing tests: restore then re-archive between preview and remove, and the project-filter fallback.
Locally the workspace build passed, along with 37 focused Runtime Host tests (retirement coordinator, protocol) and 57 Desktop tests (archived-task scope, removal preview, purge, client). I did not run a packaged Electron app or a real multi-Host disconnect.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
| return project === null ? copy.noProject : project?.label; | ||
| }; | ||
| const projectOptions = useMemo(() => archivedProjectOptions(rows, projectOf), [projectOf, rows]); | ||
| const effective = { ...scope, project: availableProjectFilter(scope.project, projectOptions) }; |
There was a problem hiding this comment.
P3: availableProjectFilter silently widens a chosen project to "All projects" once that project has no archived rows left. That happens right after a project-scoped purge, or when its Host or project drops out of projectScopes. The button then reads "Clear all", and the confirm title is the only remaining cue. Consider keeping the selection with an empty state, or resetting it visibly, so a second click right after a scoped purge cannot quietly target every project.
| // The caller's age filter ran on its own clock and may be stale: a | ||
| // task restored and archived again meanwhile carries a new time. | ||
| const { archivedAt } = (await this.#stores.readCatalogRecord(input.sessionId)).summary; | ||
| if (archivedAt === undefined || this.#now() - archivedAt <= input.requireArchivedForMs) { |
There was a problem hiding this comment.
P3: an unknown archivedAt is reported as too_recent, so the toast tells the user these tasks were "archived too recently". That isn't true for pre-schema tasks with no recorded archive time. A distinct outcome, or wording that covers both cases, would avoid the confusion.
|
|
||
| /** Only the figures the Host stated, each clause only when it is not zero. */ | ||
| function purgePreviewNotes(preview: SessionRemovePreviewResult | undefined): string[] { | ||
| if (!preview) return [tasksCopy.purgeSubtaskNote]; |
There was a problem hiding this comment.
P3: one offline Host fails the whole multi-Host preview. When the preview fails, the fallback confirm no longer mentions that Agent Graph subtasks and worktrees will also be deleted, so the user confirms a larger removal than the dialog describes.
| preview = undefined; | ||
| } | ||
| if (!request.isCurrent()) return; | ||
| const count = sessionIds.length; |
There was a problem hiding this comment.
P3: "Clear all" sends one sequential preview request per 25 tasks. Each request does a full header read and byte measurement, with no progress or cancel, so a large archive may sit silently for a while before the confirm appears.
| * destructive answer. It repeats the shape check its sibling does instead of | ||
| * relying on the caller running that one first. | ||
| */ | ||
| function archiveAgeGuard(options: unknown): { requireArchivedForMs?: number } { |
There was a problem hiding this comment.
P3: archiveAgeGuard was inserted between requiresArchivedSession's doc comment and that function, so the comment now sits on the wrong function.
| ); | ||
| // A Host-workspace task is labelled by its Host, as it always was here. | ||
| const projectLabelOf = (session: T): string | undefined => { | ||
| if (runtimeHostProfileUsesHostWorkspace(session.profileKind)) return session.profileName; |
There was a problem hiding this comment.
P3: Host-workspace rows are labelled with the Host name but filtered through projectOf, so they can show up under "No project" while displaying a Host label. The filter and the label should agree.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
LGTM. Some P3s but non-blocking.
3 participants
Part of #5825 (second M3 PR, built on the
archivedAtfrom #5884). It lets users narrow Settings › Archived tasks by archive age and by project, then delete exactly that scope after seeing what will go.What changes
Desktop: Settings › Archived tasks
Filters. Two filters sit next to the search and combine with it:
Unknown archive times. With an age filter on, tasks whose archive time is unknown (archived before feat(sessions): record when a task was archived #5884) are left out, and the page says how many.
Project entries. They use the sidebar's identity,
runtimeHostProjectKey(hostId, projectId)plus aliases. Two Hosts that share a project id stay separate, and an alias id resolves to its project.Bulk delete. It always acts on the visible rows. The button reads "Clear all" when nothing is narrowed, and "Delete N shown" otherwise. The ids are frozen when the button is clicked.
Confirmation. It follows the existing single-delete pattern: await the Host preview, then
toast.confirm. It shows:Zero-valued clauses are omitted. If the preview fails, the dialog falls back to the existing uncertain copy and still allows cancel or delete.
Runtime Host (epoch 201 → 202)
session.remove.previewnow takes{ sessionIds (1–25, unique), measureBytes?, requireArchived? }and returns{ archivableSubtaskCount, removedSubtaskCount, worktreeCount, bytes? }.bytes; the counts still come back.requireArchived. It skips targets that are no longer archived, so the preview matches what the delete will actually remove.session.removeacceptsrequireArchivedForMs. When an age filter is on, the Host keeps a task unless its archive time is known and, measured on the Host's own clock, older than the threshold. A kept task answerstoo_recent. This covers Host/Desktop clock skew, and a task that was restored and re-archived while the dialog was open. Desktop counts kept tasks as kept, not failed ("1 task was archived too recently and was kept").session.removeruns once per task, withrequireArchived. No Host batch delete is added; that is still an open question on tracking(storage): task storage visibility, reclamation and scoped cleanup #5825.Preload. One shared helper, "group by Host and page ≤ N", now serves both the M1 size reader and the removal preview. Each keeps its own failure policy: sizes skip a failing Host with a cooldown, while the preview is all-or-nothing.
Size. Production code is +1292/−387, and about 150 of those lines move existing code (row derivation, the query matcher, the purge result toast). Tests are +1263/−60. A first version was cut down after review: a second confirmation state machine, a duplicate port, and duplicated paging were removed.
Verification
measureBytes, stale result, fallback copy,too_recentcounted as kept, threshold only with an age filter), the visible-set delete, and the paging helper.dist, are caught.build:test, Biome,format:check, ASF headers,protocol-epoch-check(201 → 202),check-renderer-architecture --strict-base(legacy allowances only go down), locale hygiene, the Astryx inventory, the desktop typecheck andgit diff --check.dev:worktreeon a disposable copy of a real workspace, with four archived tasks: just now, 10 days, unknown and 40 days; the older and unknown times were set in the DB copy):Not covered: a DOM test that drives the Astryx Selector under linkedom. The age rule is covered by the model and purge-flow tests instead.
Coordination: #5394 also claims epoch 202, so whichever lands second re-pins.
AI use
Implemented and reviewed with Claude Code; the commits carry a
Co-Authored-Bytrailer.🤖 Generated with Claude Code