Re: [PR] feat(desktop): clean up archived tasks by age and project [maka]

["Astro-Han (via GitHub)" <gi...@apache.org>]

Astro-Han commented on code in PR #5896:
URL: https://github.com/apache/maka/pull/5896#discussion_r4150886054


##########
packages/runtime-host/src/server/session-retirement-coordinator.ts:
##########
@@ -331,6 +345,14 @@ export class HostSessionRetirementCoordinator {
             actualRevision: target.revision,
           });
         }
+        if (input.requireArchivedForMs !== undefined) {
+          // 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) {

Review 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.



##########
apps/desktop/src/renderer/features/session-navigation/controller/session-row-actions.ts:
##########
@@ -363,11 +397,96 @@ export function createSessionNavigationRowActions(deps: {
       archivedSubtasks,
       remaining,
       restored,
+      tooRecent,
       verified: true,
       firstFailure,
     };
   }
 
+  async function purgeArchived(request: ArchivedPurgeRequest): Promise<void> {
+    // Frozen here: what the confirm counts is exactly what the sweep deletes.
+    const sessionIds = [...request.sessionIds];
+    // Measured, and only archived targets, as the sweep will find them. A
+    // failed preview is not silence: the confirm falls back to the note every
+    // bulk delete carries, as a single delete does.
+    let preview: SessionRemovePreviewResult | undefined;
+    try {
+      preview = await service.previewRemovals(sessionIds, {
+        measureBytes: true,
+        requireArchived: true,
+      });
+    } catch {
+      preview = undefined;
+    }
+    if (!request.isCurrent()) return;
+    const count = sessionIds.length;
+    const ok = await toastApi.confirm({
+      title: request.narrowed
+        ? tasksCopy.purgeShownConfirmTitle(count)
+        : tasksCopy.purgeAllConfirmTitle(count),
+      description: [tasksCopy.purgeConfirmBody, ...purgePreviewNotes(preview)].join(' '),
+      confirmLabel: tasksCopy.purgeConfirmAction,
+      cancelLabel: getSettingsSharedCopy(uiLocale).cancel,
+      destructive: true,
+    });
+    if (!ok) return;
+    const outcome = await purgeSessions(
+      sessionIds,
+      request.requireArchivedForMs === undefined
+        ? {}
+        : { requireArchivedForMs: request.requireArchivedForMs },
+    );
+    reportPurge(outcome);
+  }
+
+  /** 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];

Review 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.



##########
apps/desktop/src/renderer/features/session-navigation/controller/session-row-actions.ts:
##########
@@ -363,11 +397,96 @@ export function createSessionNavigationRowActions(deps: {
       archivedSubtasks,
       remaining,
       restored,
+      tooRecent,
       verified: true,
       firstFailure,
     };
   }
 
+  async function purgeArchived(request: ArchivedPurgeRequest): Promise<void> {
+    // Frozen here: what the confirm counts is exactly what the sweep deletes.
+    const sessionIds = [...request.sessionIds];
+    // Measured, and only archived targets, as the sweep will find them. A
+    // failed preview is not silence: the confirm falls back to the note every
+    // bulk delete carries, as a single delete does.
+    let preview: SessionRemovePreviewResult | undefined;
+    try {
+      preview = await service.previewRemovals(sessionIds, {
+        measureBytes: true,
+        requireArchived: true,
+      });
+    } catch {
+      preview = undefined;
+    }
+    if (!request.isCurrent()) return;
+    const count = sessionIds.length;

Review 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.



##########
apps/desktop/src/renderer/features/session-navigation/ui/archived-task-scope.tsx:
##########
@@ -0,0 +1,214 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { type ReactNode, useMemo, useRef, useState } from 'react';
+import { runtimeHostProfileUsesHostWorkspace } from '@maka/runtime-host/profile-kind';
+import { Button, useMountedRef, useUiLocale } from '@maka/ui';
+import { Search } from '@maka/ui/icons';
+import { HStack, StackItem, VStack } from '@astryxdesign/core';
+import { Selector } from '@astryxdesign/core/Selector';
+import { Text } from '@astryxdesign/core/Text';
+import { TextInput } from '@astryxdesign/core/TextInput';
+import { getSettingsTasksCopy } from '../../../locales/settings-tasks-copy.js';
+import type { SessionNavigationRowActions } from '../controller/session-row-actions.js';
+import {
+  ARCHIVED_AGE_DAYS,
+  type ArchivedProjectFilter,
+  type ArchivedTaskProjectScope,
+  type ArchivedTaskScope as Scope,
+  archivedAgeThresholdMs,
+  archivedProjectOptions,
+  archivedTaskProjectResolver,
+  archivedTaskRows,
+  availableProjectFilter,
+  isArchivedTaskScopeNarrowed,
+  scopeArchivedTasks,
+  UNSCOPED_ARCHIVED_TASKS,
+} from '../model/archived-task-scope.js';
+import type { SessionNavigationSession } from '../ports.js';
+
+const ANY_AGE = 'any';
+const ALL_PROJECTS = '*all';
+const NO_PROJECT = '*none';
+const PROJECT_PREFIX = '=';
+
+export interface ArchivedTaskScopeView<T> {
+  /** Every archived task row, before any filter. */
+  readonly rows: readonly T[];
+  /** The rows on screen — exactly the set the bulk delete removes. */
+  readonly visible: readonly T[];
+  readonly purging: boolean;
+  /** Search, filters and the bulk delete, for the page to place above its list. */
+  readonly controls: ReactNode;
+  /** What a row shows as its project, which is also what search matches. */
+  projectLabelOf(session: T): string | undefined;
+}
+
+/**
+ * Settings › Archived tasks' scope: search, the age and project filters, and
+ * the one bulk delete, which always acts on exactly the rows on screen. It
+ * owns that state so the page, a legacy Settings surface, holds none; the page
+ * renders the rows this hands it.
+ *
+ * The delete itself is the rail's `purgeArchived` flow — preview, confirm,
+ * sweep, report — run on the ids frozen at the click. A preview that settles
+ * after the page closed or the scope changed opens no dialog.
+ */
+export function ArchivedTaskScope<T extends SessionNavigationSession>(props: {
+  readonly sessions: readonly T[];
+  readonly projectScopes: readonly ArchivedTaskProjectScope[];
+  readonly commands: { readonly current: SessionNavigationRowActions | null };
+  readonly children: (view: ArchivedTaskScopeView<T>) => ReactNode;
+}) {
+  const copy = getSettingsTasksCopy(useUiLocale());
+  const mountedRef = useMountedRef();
+  const [scope, setScope] = useState<Scope>(UNSCOPED_ARCHIVED_TASKS);
+  const [purging, setPurging] = useState(false);
+  // The click a pending confirm belongs to; any scope change retires it.
+  const pendingPurgeRef = useRef<object | null>(null);
+
+  const rows = useMemo(() => archivedTaskRows(props.sessions), [props.sessions]);
+  const projectOf = useMemo(
+    () => archivedTaskProjectResolver(props.projectScopes),
+    [props.projectScopes],
+  );
+  // 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;

Review 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.



##########
apps/desktop/src/main/runtime-host-session-catalog-ipc-main.ts:
##########
@@ -331,6 +335,11 @@ async function moveSessionToProject(
  * 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 } {

Review Comment:
   **P3:** `archiveAgeGuard` was inserted between `requiresArchivedSession`'s doc comment and that function, so the comment now sits on the wrong function.



##########
apps/desktop/src/renderer/features/session-navigation/ui/archived-task-scope.tsx:
##########
@@ -0,0 +1,214 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { type ReactNode, useMemo, useRef, useState } from 'react';
+import { runtimeHostProfileUsesHostWorkspace } from '@maka/runtime-host/profile-kind';
+import { Button, useMountedRef, useUiLocale } from '@maka/ui';
+import { Search } from '@maka/ui/icons';
+import { HStack, StackItem, VStack } from '@astryxdesign/core';
+import { Selector } from '@astryxdesign/core/Selector';
+import { Text } from '@astryxdesign/core/Text';
+import { TextInput } from '@astryxdesign/core/TextInput';
+import { getSettingsTasksCopy } from '../../../locales/settings-tasks-copy.js';
+import type { SessionNavigationRowActions } from '../controller/session-row-actions.js';
+import {
+  ARCHIVED_AGE_DAYS,
+  type ArchivedProjectFilter,
+  type ArchivedTaskProjectScope,
+  type ArchivedTaskScope as Scope,
+  archivedAgeThresholdMs,
+  archivedProjectOptions,
+  archivedTaskProjectResolver,
+  archivedTaskRows,
+  availableProjectFilter,
+  isArchivedTaskScopeNarrowed,
+  scopeArchivedTasks,
+  UNSCOPED_ARCHIVED_TASKS,
+} from '../model/archived-task-scope.js';
+import type { SessionNavigationSession } from '../ports.js';
+
+const ANY_AGE = 'any';
+const ALL_PROJECTS = '*all';
+const NO_PROJECT = '*none';
+const PROJECT_PREFIX = '=';
+
+export interface ArchivedTaskScopeView<T> {
+  /** Every archived task row, before any filter. */
+  readonly rows: readonly T[];
+  /** The rows on screen — exactly the set the bulk delete removes. */
+  readonly visible: readonly T[];
+  readonly purging: boolean;
+  /** Search, filters and the bulk delete, for the page to place above its list. */
+  readonly controls: ReactNode;
+  /** What a row shows as its project, which is also what search matches. */
+  projectLabelOf(session: T): string | undefined;
+}
+
+/**
+ * Settings › Archived tasks' scope: search, the age and project filters, and
+ * the one bulk delete, which always acts on exactly the rows on screen. It
+ * owns that state so the page, a legacy Settings surface, holds none; the page
+ * renders the rows this hands it.
+ *
+ * The delete itself is the rail's `purgeArchived` flow — preview, confirm,
+ * sweep, report — run on the ids frozen at the click. A preview that settles
+ * after the page closed or the scope changed opens no dialog.
+ */
+export function ArchivedTaskScope<T extends SessionNavigationSession>(props: {
+  readonly sessions: readonly T[];
+  readonly projectScopes: readonly ArchivedTaskProjectScope[];
+  readonly commands: { readonly current: SessionNavigationRowActions | null };
+  readonly children: (view: ArchivedTaskScopeView<T>) => ReactNode;
+}) {
+  const copy = getSettingsTasksCopy(useUiLocale());
+  const mountedRef = useMountedRef();
+  const [scope, setScope] = useState<Scope>(UNSCOPED_ARCHIVED_TASKS);
+  const [purging, setPurging] = useState(false);
+  // The click a pending confirm belongs to; any scope change retires it.
+  const pendingPurgeRef = useRef<object | null>(null);
+
+  const rows = useMemo(() => archivedTaskRows(props.sessions), [props.sessions]);
+  const projectOf = useMemo(
+    () => archivedTaskProjectResolver(props.projectScopes),
+    [props.projectScopes],
+  );
+  // 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;
+    const project = projectOf(session);
+    return project === null ? copy.noProject : project?.label;
+  };
+  const projectOptions = useMemo(() => archivedProjectOptions(rows, projectOf), [projectOf, rows]);
+  const effective = { ...scope, project: availableProjectFilter(scope.project, projectOptions) };

Review 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.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org