feat(storage): opt-in retention for archived tasks

[liugddx]

Closes #5899 (step 2 of #5776). It adds an opt-in, per-Host setting, delete archived tasks after N days, which is off by default. The Host decides and executes; Desktop exposes the setting and shows what happened.

Rules (as specified in #5899)

• Opt-in, per Host. Nothing happens until the user enables the setting on that Host.
• Clock start. A task's clock starts at max(archivedAt ?? enabledAt, enabledAt), and the boundary is strict. Enabling the setting never deletes a backlog at once, and legacy tasks without archivedAt count from enablement.
• Settings changes restart the clock. Enabling the setting or changing N re-stamps enabledAt on the Host clock, so it is never earlier than the latest time the Host has observed. Disabling clears it.
• Restore and re-archive. Restoring a task cancels its deadline; archiving it again starts a fresh one.
• Pinned families. If any member of a revision family is pinned, the whole family is kept.
• Candidate rows. Only rows shown on Settings › Archived tasks are candidates: archived roots and orphaned archived subtasks. One shared SQL predicate now backs both the catalog page and retention. A test asserts that the candidate set equals archivedTaskRows for a mixed fixture: revisions, live-parent subtasks, orphans, a graph operator, preparing rows, and rows with a missing projection.
• Needs review. Families whose removal would reclaim a subagent worktree, or archive still-active subtasks, are skipped and reported as "needs review". The worktree count uses the same expression as the manual delete preview.
• Recheck inside the removal admission. The settings revision, archive and pin state, and the deadline are rechecked under the same admission as manual session.remove, together with every existing busy guard. Manual removal behaves exactly as before.
• Clock regression pauses. If the wall clock is behind the Host's high-water mark, or behind the newest timestamp in session metadata, the sweep pauses and deletes nothing.

How it works

Setting document. archive-retention.json in the State Root holds { version, revision, enabled, days, enabledAt?, latest? }.

• Writes: atomic (temp file, fsync, rename), with a revision CAS. The only writer is storage.retention.set. It is not part of the runtime policy, agent settings patches, or config export/import.
• Invalid documents count as disabled.

Sweep lane in HostStorageMaintenance. It starts after Ready.

• Cadence: every 15 min when idle; while work remains, every 1 s, with at most 8 families per tick.
• Before the first deadline (enabledAt + days), it runs no SQL at all.
• Faults: an undecodable row is counted as failed and skipped. Drain stops the loop between families.
• Turning the setting off waits for the family already in flight, so set returns only once nothing more can be deleted.

Results are latest-only:

• lastSweep { at, deleted, skippedBusy, needsReview, failed, paused? }
• lastDeletion { at, count, bytes? }, where bytes is measured before deletion with the same helper as the batch preview.

The document is written only when a sweep changes something. Logs carry counts only.

Protocol (epoch 202 → 203)

• storage.retention.query {} returns { revision, enabled, days, enabledAt?, preview: { count, eligibleAt? }, lastSweep?, lastDeletion? }.
  • The preview is a single aggregate query.
  • The count is a lower bound: deleting a root can orphan archived subtasks, which then become candidates.
• storage.retention.set { expectedRevision, enabled, days } returns committed or revision_conflict.
• Both operations are added to the renderer passthrough allowlist and to the remote-owner grants, matching session.remove.

Desktop: Settings › Archived tasks

• New section: an enable switch, a choice of 30, 60 or 90 days, the Host preview, "Last automatic cleanup: …", and a needs-review count.
  • A banner appears if the last sweep paused.
  • The section names the Host it applies to, because the list below spans all Hosts. The page's settings scope is now mixed for that reason.
• Confirmation: enabling the setting, or changing N while it is on, asks for confirmation first. The prompt refetches the Host count and says "at least N tasks, no earlier than ~", that pinned tasks are kept, and that deletion is permanent. Disabling needs no confirmation.

Verification

• Tests use an injected clock throughout. They cover:
  • every rule above, the cadence and the per-tick cap, the CAS, and the write frequency;
  • the disable-while-a-deletion-is-in-flight window;
  • drain, bad rows, and clock-regression pauses;
  • the candidate/page parity test;
  • the codecs;
  • the Desktop confirm and rendering.
• Mutation testing: 36 mutations of key production lines, applied in dist, are all caught.
• Checks: build:test, Biome, format:check, lint, Knip (desktop and ui), protocol-epoch-check (202 → 203), check-renderer-architecture --strict-base, locale hygiene, the Astryx inventory, the Windows inventory, app-shell hooks, ASF headers, git diff --check, and the desktop typecheck.
• Existing full-suite failures: the remaining failures in the full suites (Windows EBUSY/EPERM) match main.
• Running app: I ran dev:worktree on a disposable copy of a real workspace.
  1. I archived four tasks and pinned one. Enabling the setting showed "at least 3 tasks, no earlier than <now + 30 days>". The pinned task was excluded, and archive-retention.json recorded enabledAt.
  2. To simulate time passing, I moved enabledAt back 31 days and the archive times of three tasks back 40 days in the copy, then restarted.
  3. About a second after Ready, the sweep deleted the two eligible unpinned tasks. The pinned 40-day task and the just-archived task were kept.
  4. The page showed "Last automatic cleanup: deleted 2 tasks (about 6.2 MB)". That matches the two rows' sizes, and the DB confirmed it.

Size: production code is +2171/−28, of which about 145 lines are copy and about 57 are the test-only memory store. Tests are +1868.

AI use

Implemented and reviewed with Claude Code; the commits carry a Co-Authored-By trailer.

🤖 Generated with Claude Code

[Astro-Han]

Independent review at b237ed1.

What changed: This PR adds an opt-in, per-Host retention policy that deletes archived tasks after 30/60/90 days. It adds storage.retention.query/set (epoch 202 -> 203) and a State Root document archive-retention.json. It also adds a maintenance lane: 8 families/tick, 1s active, 15 min idle. Each family is deleted through session.remove's path with a guard that runs under the removal admission. A Settings > Archived tasks section is added.

Scope checked, found sound:

• Opt-in: an absent or invalid document is treated as disabled. There is no SQLite migration (the schema-41 archived_at is reused).
• Retention clock: it starts at max(archivedAt ?? enabledAt, enabledAt), so pre-schema-41 rows count from enablement. Any enable or period change restamps enabledAt to at least the newest recorded time.
• Restore race: the guard runs inside #withStableRemovalPlan -> #admission.runMany, the same gate unarchive takes. It rechecks archive state, pin state, setting revision and archived_at. The commit is CAS-versioned.
• archived_at maintenance: unarchive clears it, re-archive restamps it, and the archive-on-remove branch stamps it.
• Excluded from deletion: pinned families, Agent Graph operators, and plans that would archive active subtasks or reclaim worktrees (held as needs_review).
• Clock going backwards: the sweep pauses.
• Setting changes: a change waits for the in-flight tick, and the guard holds while a change is pending.
• Merges cleanly into main. There were no prior reviews.

Tests: runtime-host retention/retirement/maintenance/protocol tests pass (72/72), as do the storage archive-retention-store tests (5/5).

Findings: P3 epoch collision; P3 no guard against the wall clock jumping forward before an unattended, permanent deletion; P3 preview wording.

Not exercised:

• The desktop UI was not run.
• Not verified: legacy importSession rows that are archived but have NULL archived_at would count from enabledAt and could become eligible right after import. Worth a look.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

[Astro-Han]

P3: This PR claims compatibility epoch 203, but open PRs #5753 (head 9f0eeb5) and #5495 (head cae4a93) also claim 203. Whichever merges second must move to 204 and re-word its rationale comment, or two incompatible protocol shapes will share one epoch. Please coordinate the merge order.

[Astro-Han]

P3: Only backward clock motion is detected (wentBack, newest metadata time). Nothing bounds a forward jump: a bad RTC or NTP, a VM restore, or a manual date change. If now jumps at least days past enabledAt, the whole backlog becomes eligible at once, because pre-schema-41 rows count from enabledAt. The sweep then deletes 8 families per second. The deletion is permanent, and the user never had the real retention window to notice or restore. This is the one unattended path where a misread clock destroys data. Two possible guards:

• Compare the wall-clock delta since the last observation with the monotonic delta inside a running process. If wall time leaps far ahead, pause (as for backward motion) until the user re-confirms.
• At minimum, cap how far a single sweep may advance past the last recorded lastSweep.at.

Once such a jump has persisted lastSweep.at, the pause correctly blocks later backward motion, so the gap is only on the forward side.

[Astro-Han]

P3: "Covers at least N archived tasks" (and "At least N tasks are covered" in the confirm) is presented as a lower bound. But countArchiveRetentionCandidates counts families that the sweep will hold: those that would archive an active subtask or reclaim a worktree (needs_review), those whose removal plan contains a pinned subtask, and busy ones. So the count can overstate what will actually be deleted. Suggest neutral wording ("N archived tasks are subject to cleanup") or excluding held families from the count.

[liugddx]

Thanks @Astro-Han. All four points are addressed in fefa3dc, which merges cleanly with current main.

P3: forward clock jump. Agreed, this was the one unattended path where a misread clock destroys data. Neither suggested guard fits a local Host, though:

• Capping advancement by uptime or the last sweep is the credited clock that feat(storage): opt-in retention for archived tasks #5899 (decision 4) rejected. A local Host runs only while the app is open, so for occasional users retention would barely progress.
• Comparing wall-clock and monotonic deltas misfires on laptops: the monotonic clock stops during system sleep on macOS and Linux, so a lid closed for a few days reads as a jump.

What I did instead is a gap hold. When the wall clock has moved more than min(days, 7) days past the last observed time, the sweep deletes nothing and records latest.hold = { since, detectedAt, until = detectedAt + 24h }.

• What counts as last observed. While the Host is running, that is the in-process high-water mark. After a restart it is the persisted floor (enabledAt, lastSweep.at, a previous hold) together with the newest session metadata time. A Host that was in recent use therefore doesn't hold after a restart.
• During a hold: deletions resume at until, a further jump re-arms the hold, and any settings change clears it.
• Desktop banner: "Automatic cleanup resumes after because the system clock moved ahead by about N days since this Host last ran. Check your system time; if it is wrong, turn cleanup off."

The trade-off is that someone genuinely away for more than 7 days sees the banner and a one-day delay. A bad clock, on the other hand, can no longer delete a backlog in the minute it is noticed.

Tests run on an injected clock and cover:

• a jump that holds and then resumes at until;
• a jump during a hold, which re-arms it;
• an advance of exactly 7 days, which never holds;
• a restart after 20 days offline with a 30-day window, which holds once for 24 h and then deletes;
• a restart with recent DB writes, which doesn't hold;
• a settings change, which clears the hold.

Each of these was mutation-checked.

Your import note was a real gap, and it's fixed. importSessionBundleState copies session_metadata with INSERT … SELECT *, so an imported archived task kept the source machine's old archived_at, or NULL, which counts from a past enabledAt. Either way it could be deleted right after import.

• Fix: in the same transaction, archived rows from the bundle now have archived_at set to the import time.
• Test: a new case exports archived sessions with archived_at NULL and with an old value, then asserts that both are still archived after import and are stamped within the import window.
• SessionStore.importSession (external sessions) always writes isArchived: false, so it is unaffected.

P3: preview wording. You're right: the count includes families the sweep will hold, and it also misses subtasks orphaned later, so it is neither a lower nor an upper bound. The copy is now neutral, "N archived tasks are subject to automatic cleanup", in the section and in the confirm, in all three locales.

P3: epoch. main is still at 202, so this keeps 203. #5753 and #5495 also claim 203; whichever lands second re-pins. I'll do it here if this one is second.

Checks on fefa3dc: Biome, format:check, lint, both Knip runs, the epoch check (202 → 203), renderer architecture --strict-base, locale hygiene, the Astryx and Windows inventories, app-shell hooks, ASF headers, git diff --check and the desktop typecheck all pass. Focused tests pass: storage 86, runtime-host 72, desktop 9. The remaining local failures are Windows symlink EPERM, and they are identical on the previous head.

[Astro-Han]

Follow-up review at fefa3dc (delta since b237ed1: one commit, "hold retention after a forward clock jump").

Earlier points:

• Forward clock jump: addressed by the gap hold. The in-run detection, re-arming, the setting-change clear and the post-deadline restart check (persisted floor plus the newest metadata time) look correct. There is one regression in the restart path before the deadline; see the inline comment.
• Import: fixed. mergeAttachedBundle restamps archived_at on archived bundle rows inside the import transaction, and there is a test. External importSession always writes non-archived headers, so it is unaffected.
• Preview wording: fixed in all three locales and in the confirm text.
• Epoch: unchanged. main is still at 202, and this PR claims 203. #5753 and #5495 also claim 203 (#5826 now claims 204). The re-pin plan in the thread is fine; I am just flagging that the collision is still open.

Non-blocking design note: the hold defers deletions by only 24h. For a clock that really is wrong, the safety net is that the user opens Settings > Storage within a day. A clock that is wrong and unnoticed for more than a day still deletes the backlog. This is an acceptable trade-off for a local Host, but consider making the notice visible outside the Storage settings page (for example, a one-time notification) if that is cheap.

Tests: the runtime-host retention coordinator, retention protocol and retirement tests pass (68/68), including a temporary probe that I later reverted. The storage archive-retention-store and session-bundle-policy tests pass (8/8).

CI: test and windows_recovery pass. audit and Build immutable tarball fail on dependency advisories in shipped dependencies (axios, fast-uri). This PR changes no lockfile, so those failures are unrelated. The PR merges cleanly.

Verdict: one P2 (a false clock-jump hold, with a banner that stays up, after any restart before the deadline) and one P3 (epoch coordination). No data-loss paths found in the delta.

Automated review by Claude (Anthropic), posted on behalf of the maintainer. It is not an independent human review.

[Astro-Han]

P2: a restart before the deadline records a false clock-jump hold, and its banner stays up until the deadline.

When a process is fresh (observedThisRun === false) and now <= deadline, previous is only the persisted floor. Before the deadline that floor is effectively enabledAt, because no lastSweep or hold is written yet. The newest metadata time is never consulted on this arm. With the allowed periods of 30, 60 and 90 days, the gap is always 7 days. So any Host restart more than 7 days after enabling, and before the deadline, records a hold, and the user sees "the system clock moved ahead by about N days ... check your system time". That happens even if the app was in use a minute earlier.

I reproduced it with the rig:

1. set(true, 30).
2. Advance 9 days with in-run sweeps (no hold).
3. restart(), then wait 1h with newest = now - 1min.
4. sweep(): the hold is {since: enabledAt, detectedAt: enabledAt+9.04d}.
5. Five days later (past until, still before the deadline), query().hold is still present.

The hold persists because the clear at L203-210 only runs after the deadline and L413 returns the hold regardless of until. It also re-arms on every restart that comes more than 7 days after the previous detectedAt.

Fix: on this arm, measure from Math.max(previous, await readLatestSessionMetadataTime()). It is a single MAX query, so the "no SQL before the deadline" property can give way here. Also drop or ignore an expired hold independently of the deadline: clear it when now >= hold.until, or omit it from storage.retention.query once expired. Please add a test for a restart at day 9 of a 30-day policy with recent metadata, asserting no hold.

[liugddx]

Thanks @Astro-Han, good catch: your repro reproduces exactly. The fix is in 5000b16, and the branch is rebased onto main@0b25078 (the only conflict was the generated Astryx inventory, which I regenerated). That rebase also picks up #5906, so audit and the tarball build should be green now.

P2: false hold on a fresh process before the deadline.

• The gap is now measured from the right point. A fresh process measures the gap from max(persisted floor, newest metadata time), both before and after the deadline. That costs one MAX query per process; a running Host keeps using its in-memory high-water mark. Only the candidate query is still skipped before the deadline, and the test that used to assert "no SQL" now asserts "no candidate read".
• Holds expire at until, independently of the deadline. A sweep clears an expired hold with a single write, and storage.retention.query never returns an expired hold, even before that sweep has run.
• New tests:
  • your case: a restart on day 9 of a 30-day policy, with metadata written a minute earlier, records no hold;
  • a fresh process still holds after a real jump measured from old metadata;
  • a hold that expires before the deadline stops being reported and is cleared with exactly one write, and nothing is deleted.
• Mutation checks: each of these four mutations is caught by the new tests:
  • the fresh-process path ignoring recent metadata;
  • the fresh-process path never holding;
  • an expired hold never being cleared;
  • the query reporting an expired hold.

Notice outside Settings. Agreed that this would make the 24 h window more useful. It is the optional follow-up already listed in #5899 (a one-time notice for new automatic deletions), and I'll fold the hold into that follow-up rather than widen this PR.

Epoch. main is still at 202, so this PR keeps 203. If #5753 or #5495 lands first, I'll re-pin.

Checks after the rebase:

• Pass: Biome, format:check, lint, both Knip runs, the epoch check (202 → 203), renderer architecture --strict-base, locale hygiene, the Astryx and Windows inventories, app-shell hooks, ASF headers, git diff --check and the desktop typecheck.
• Focused tests: storage 86 and desktop 24 pass. In runtime-host, 150 pass and 2 fail; both failures are the Windows symlink EPERM cases, which also fail on main.

[liugddx]

One more fix, found while designing the next step (idle archiving): fd9c3f3 makes the retention candidate query linear.

What was wrong. The pinned-family exclusion was a correlated NOT EXISTS over session_metadata. Its comment, and the PR description, claimed the (is_flagged, is_archived, …) index bounds the scan, but migration 26 dropped that index. So every candidate rescanned the table. The cost was quadratic, and it ran synchronously on the Host thread for both the sweep page and the Settings preview count.

What changed. The exclusion is now an uncorrelated NOT IN over the pinned family roots, which SQLite evaluates once per statement. A family root is COALESCE(revision_root_session_id, session_id), which can never be NULL, so NOT IN is safe here. The parent check in the row predicate stays correlated; it is a primary-key lookup.

Synthetic timings, same result sets:

Sessions	Before	After
2,000	47 ms	2 ms
10,000	1.1 s	4 ms
40,000	20 s	21 ms

Tests: storage retention 13/13, the candidate/page parity test, and the runtime-host retention coordinator 26/26. Biome is clean.

The "Candidate rows" paragraph in the description still holds. Only its index claim was wrong.

[Astro-Han]

Re-review at 5000b16 (rebased onto main 0b25078; the only new PR commit is 5000b16).

Prior findings

• P2, a false clock-jump hold when a fresh process restarts before the deadline: fixed. A fresh process now measures the gap from max(persisted floor, newest metadata time) (archive-retention-coordinator.ts:197-200). A hold is cleared once now >= until, whatever the deadline (L188-192). storage.retention.query no longer reports a hold that has expired (L417-420). The new tests reproduce my earlier probe exactly (a restart on day 9 with metadata written a minute earlier records no hold), and they also check that a real jump still holds and that an expired hold is cleared with a single write. Before the deadline the candidate list is still never read.
• P3, epoch 203: still open, coordination only. main is at 202. #5709, #5495 and #5753 also claim 203, and #5826 claims 204. Whichever PR lands second needs to re-pin, as the author has already said.

Remaining (P3): see the inline comment. A fresh process can only tell when the Host last ran from Session metadata writes. A Host that kept running with no task activity for more than 7 days and is then restarted still shows the "system clock moved ahead" banner. It now lasts 24h instead of weeks, so this is cosmetic.

Verification: I built core, storage, mcp, runtime and runtime-host. archive-retention-coordinator, storage-retention-protocol and the session-retirement* tests pass 76/76. I also ran a temporary probe test, since reverted, for the idle-restart case below.

CI: audit and Build immutable tarball now pass because the rebase picked up #5906. windows_recovery passes. test was still pending when I reviewed. Mergeable.

Update for head fd9c3f36 (pushed after this review was drafted at 5000b160): the only new commit rewrites the pinned-family exclusion in archiveRetentionCandidatePredicate from a correlated NOT EXISTS to an uncorrelated NOT IN. Both sides are COALESCE(revision_root_session_id, session_id), so no NULL can reach the NOT IN list and the semantics are unchanged; the findings above still apply at this head.

---

This is an automated review by Claude (Anthropic), run on behalf of the maintainer.

[Astro-Han]

P3: on a fresh process, the only signal for when the Host last ran is MAX(committed_at, archived_at) from session_metadata. The in-memory high-water mark is never persisted. lastSweep is not written before the deadline, and it is not written on unchanged passes either. So a Host that ran continuously with no task activity for more than 7 days still records a hold when it restarts, and the banner tells the user to "check your system time".

I confirmed this with a temporary test (reverted): set(true, 30), newest = enabledAt, nine daily in-run sweeps with no hold, then restart() and one minute later → hold {since: enabledAt, ...}.

The banner now lasts only 24h, so this is minor. One fix: persist a coarse heartbeat, e.g. write latest.observedAt from #tick at most once a day, and fold it into the #read floor. Alternatively, reword the banner so it also covers "this Host was not running for a while".

[Astro-Han]

Re-review at ffaa68e7. The branch was rebased onto main c7fa6bb6. git range-diff shows the five earlier PR commits unchanged (=), so the only new code is ffaa68e7 "persist idle retention heartbeat".

Prior findings

• P3, a Host that stays up with no task activity for more than 7 days and then restarts shows a false "clock moved ahead" hold: fixed. Sweeps now persist latest.observedAt at most once a day (#heartbeatDue/#heartbeat, archive-retention-coordinator.ts:336-353). The heartbeat is also carried on #record, #pause and #clearHold, and #load folds it into the floor (L579-584). It is written only after the gap check passes (L202-207), so a real forward jump is never laundered into the floor. The storage decoder accepts the new key and rejects non-integers, and storage.retention.query does not expose it. I checked this with temporary probe tests, since reverted: 22 days idle past the deadline with sweeps every 12h, then a restart a minute later, records no hold; 9 days idle before the deadline, then a restart, records no hold (this was my earlier repro); and a real 8-day jump after heartbeats still holds, with since set to the last heartbeat.
• P3, compatibility epoch 203: still open, coordination only. main is still at 202, and #5866, #5709, #5495 and #5753 are all still open and also claim 203.

New: P2, the test job will fail. See the inline comment. Because of the heartbeat, #finishPass now writes an unchanged pass once a day, so lastSweep is no longer undefined after a pass that deleted nothing. session-retirement-coordinator.test.ts:1561 still asserts that it is undefined, and it fails locally, deterministically, at this head. The other retirement and retention tests pass.

Verification: I built core, storage, mcp, runtime, acp, antigravity and runtime-host (tsc OK). Results: archive-retention-coordinator, storage-retention-protocol, session-retirement* and storage-maintenance pass 81/82 (the one failure is the test above); storage archive-retention-store and session-bundle-policy pass 8/8.

CI: audit, Build immutable tarball, the direct-peer addons, the installed-CLI validations and windows_recovery pass. test is still pending, and I expect it to fail on the test above. Mergeable; blocked only on review.

---

This is an automated review by Claude (Anthropic), run on behalf of the maintainer.

[Astro-Han]

P2: this fails at ffaa68e7, so test will go red. enabledAt + 31 * DAY is more than a day after enabledAt, so #heartbeatDue is true, and #finishPass (archive-retention-coordinator.ts:390) now records the unchanged pass with lastSweep: { at, deleted: 0, ... } instead of returning early. The point of this test is that a task removed by hand is not counted as a deletion, which lastDeletion === undefined and deleted === 0 already show. Suggested fix: replace this line with assert.equal(result.lastSweep?.deleted, 0);. Alternatively, if lastSweep should keep its old meaning ("last time the result changed"), let the heartbeat-only path write just observedAt and leave lastSweep alone.