Repository navigation
feat(sessions): record when a task was archived - #5884
Conversation
Sessions had an archive flag but no archive time: committed_at moves on every metadata write, so nothing durable said when a task was put away. Storage: schema 41 adds session_metadata.archived_at. Only the archive lifecycle writer (setArchivedSync, reached from setArchivedVersioned and from removeVersioned when a delete moves linked subtasks to the archive) writes it: archiving stamps it, restoring clears it, re-archiving stamps a fresh time, and archiving an already archived Session is a no-op that keeps the first time. One lifecycle write stamps every Session in it with one instant, so a revision family or the subtasks moved by a delete share a time. Generic metadata writes leave the column alone. Rows archived before the column existed keep NULL; no time is derived from committed_at or activity. Protocol: Session catalog projections may carry an optional archivedAt, present only on an archived Session; absent means unknown. Epoch 200 -> 201, since epoch-200 decoders reject the unknown key. Desktop: Settings > Archived tasks shows "Archived <date>" or "Archive time unknown" on each row, and lists the most recently archived first, unknown last, ties in the existing store order. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Address review findings on the archive time: - Test that projectSessionCatalogSummary carries archivedAt into the summary Desktop and the CLI read, and adds no key when it is absent. - Pin that Sessions archived by a delete share the retirement's deleted_at, read from the tombstone row. - Project archivedAt from the Host only for an archived Session, so a stray time on an active row cannot fail the Host's own decoder and hide the task as an unsupported record. - Stamp, keep and clear the archive time in the in-memory test store the way SQLite does, and check both through the provider conformance suite. - Document on SessionSummary.archivedAt that isArchived alone is the archive state; absence of the time never means "not archived". - Inline the row label into the archived tasks page and drop the helper and its copy-table test. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed the current head. Schema 41 adds nullable archived_at without inventing a time for previously archived Sessions. Lifecycle archive writes stamp a revision family once, repeated archive preserves the first time, restore clears it, and ordinary header updates leave it unchanged. The catalog and Desktop projection distinguish an unknown archive time from active state and sort known times newest-first. I found no substantiated P0–P3 issue in the changed path. Node 24 clean npm ci and build:test, 314 focused Storage/Host/Desktop tests, diff-check, and static merge against current main pass. Current-head hosted test and label checks pass. I did not repeat the author’s real-workspace migration or packaged Desktop smoke test. This review is tied only to this head and current main; protocol epoch 201 will need re-evaluation if another pending epoch-201 PR lands first.\n\n> 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
There was a problem hiding this comment.
Reviewed at 0f6318ae, focused on the migration and wire compatibility. No P0–P3 found.
- Schema 41: adds a nullable column with no backfill, guarded by
hasColumnso replays are no-ops; existing archived rows stayNULLwithout touching their version/commit time. Older Hosts refuse the newer schema as with every prior migration, and bundle import still requires identical schemas. - Single writer: only the archive/unarchive path sets
archived_at— stamped only on an actual state change (re-archiving keeps the first time), cleared on unarchive, one timestamp per bulk archive, delete-time for children archived during delete; all other writes preserve the column, and nothing creates an already-archived Session, so forks/revisions/imports have nothing to copy. - Protocol:
archivedAtis optional, rejected by the decoder on non-archived rows and dropped Host-side rather than failing; the 200→201 bump is required since epoch-200 peers reject unknown keys, no earlier compatible-change declarations were rewritten, and the epoch guard passes againstmain. - Ordering/UI: server paging still keys on activity + id, so cursors are unaffected; the archive-time sort is Desktop-only (newest first, unknown last, stable ties), and unknown times render as "Archive time unknown" instead of borrowing another timestamp.
Storage (206) and runtime-host catalog/protocol (125) tests pass locally. Not verified: the Desktop task-catalog-rows test (local UI build drift unrelated to this PR) and the settings page visually.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
|
Thanks @liugddx — a clean, well-scoped foundation for the archive-age cleanup: a single writer for the timestamp, an honest NULL for unknown history, and a correctly versioned wire field. Merging now. |
Astro-Han
left a comment
There was a problem hiding this comment.
Approved at @Astro-Han's explicit request: two independent automated reviews of this head found no issues, and CI is green.
Resolve the protocol epoch collision: upstream landed epoch 201 first (apache#5884, session catalog projections may carry `archivedAt`), so this branch's callKinds usage filter moves to 202. The 201 comment chain entry from main is kept verbatim; a new 202 entry documents the callKinds filter. Re-pin the two compatible-change declarations that this branch had pinned to 201 (mechanical-candidate-sweep.json, turn-snapshot-optional-fields.json) to 202 per the guard README; both reasons still hold against the protocol as it now stands. Generated-by: GLM-5.3-Flash (ZCode)
Fourth structural catch-up for apache#5274: carries apache#5868 (Composer staging moved into a persistent ComposerStagingProvider owner) and apache#5884 (task archive timestamps). Conflict resolution ports the revision staged-context semantics onto the new owner structure: ComposerStagingSubmission gains optional owner-key reads/clears (snapshot for captured submissions, live plate for the revision re-key) and ComposerStagingCommands gains stagedContext(); the frozen shell forwards the one composerStaging handle and the revision assembler derives the gate probe and plate reads from it.
Review follow-ups (#5274 review): - prepareRevisionSend re-keyed the edit-start snapshot, so quotes the user removed or re-annotated during the edit reappeared on the branch and could be sent. The re-key now lands the plate's current quotes, and the send reads them live (the before-send snapshot retires — the re-key covers its case). - Drops the stale-base footer copy block and a dead regenerate key: no production code on current main references the old footer shape that survived in this branch's base. The no-op gate keeps counting the model-facing comment. Generated-by: GLM-5.3-Flash (ZCode) Merge upstream/main into fix/desktop-revision-structured-context fix(desktop,ui): preserve the quote plate across the in-flight revision send The revision send awaited prepareRevisionSend before reading the plate, and the lifecycle re-keys it mid-send: the plate's current quotes are copied onto the branch child and the source bucket is emptied in place. The still-running sendWithAttachments invocation holds quotesForSend from the render that started the send, so it read the emptied source bucket, got undefined, and submitted the edited replacement without its quotes or their edited annotations (#5274 review). Read the plate through the re-keyed owner explicitly: quotesForSend now resolves the bucket by owner key (defaulting to the live draft key for the non-revision send paths), and the revision send passes the prepared draft's session id. The resumed send therefore reads the child bucket the lifecycle just wrote — removals and re-annotations included. The regression test drives beginEditUserMessage → an in-render quotesForSend capture → prepareRevisionSend and asserts the re-keyed plate with its edited annotation still reaches the send; it fails on the previous head with `actual: undefined`. Generated-by: GLM-5.3-Flash (ZCode) fix(desktop,ui): keep quotes added during a cancelled edit Cancelling a revision draft cleared both draft keys wholesale, so quotes the user staged during the edit were lost together with the edit's own restaged set (#5274 review: "Cancelling an edit deletes quotes added during it" — on main they stayed in the composer, and adding quotes during an edit is supported). clearQuotes now returns the entries it removed, and clearRevisionStagedContext re-stages whatever lies beyond the edit's beginEdit snapshot (matched per QuoteRef field set, counted as a multiset) onto the source Session the cancel returns to — whether the user added them before or after the branch child was prepared. The edit's own items are still dropped wherever the commit left them. The regression tests drive a cancelled edit with a user-added quote on each side of the prepare step and assert the addition survives on the source key while the composer text rolls back; both fail on the previous head, the first with the edit's restaged quote returned instead of the user's own, the second with no plate restore at all. Generated-by: GLM-5.3-Flash (ZCode) fix(ui): treat negative owned counts as fully user-owned clearRevisionStagedContext drops the edit's own quotes by per-key count: the first N entries matching the beginEdit snapshot are the edit's, and everything past that count is the user's own staging. The kept check compared the remaining count with `=== 0`, so once the counter ran negative the surplus stopped matching: when the user added a second copy of a quote the edit had also staged, the third identical plate entry was dropped despite being the user's. Treat any non-positive count as fully user-owned (`<= 0`), so every entry past the edit's own count survives the cancel. Fixes a P3 raised inline on #5274. Generated-by: GLM-5.3-Flash (ZCode) Merge upstream/main (8648258) Brings in the composer-submit extraction (#5815) and the allow-unchanged edit-and-resend decision, merged into the revision staged-context feature: - app-shell.tsx keeps the upstream extraction; the staged-context wiring and the #5274 re-keyed quotesForSend/clearQuotes semantics move into features/conversation/controller/composer-submit.ts (owner-keyed ports). - app-shell-revision-actions.ts keeps the @maka/ui delegation; TurnRevisionDraftBase drops originalText and the unchanged gate (revisionStagedContextUnchanged) so unchanged resends pass through per upstream #5815, while the mixed-context conflict gate stays. - conversation-copy.ts keeps both new locale keys (revisionDraftQuoteConflict, revisionMixedContextUnsupported) on top of upstream's revisionUnchanged removal and #5815 copy updates. - renderer-architecture.json regenerated via check-renderer-architecture --write; app-shell.tsx budget drops 12091 -> 11154. Tests: upstream app-shell-revision-resend gains a stagedContext fake; the ui revision suite expectations updated to the allow-unchanged behavior. Generated-by: GLM-5.3-Flash (ZCode) fix(desktop): assemble the revision staged context outside the frozen shell Root cause: upstream #5546 extracted app-shell helpers and drove the frozen shell's nonTriviaTokens baseline down to 11129 with zero headroom, so the merge that re-attached this PR's stagedContext wiring (a 7-line factory in the createAppShellRevisionActions ports object, +25 tokens) broke the renderer architecture ratchet: 11129 -> 11154. Fix: assemble the RevisionStagedContext closure inside the desktop useComposerAttachments controller (a feature file outside the debt ledger), right beside the hooks that own the quote/attachment buckets. The frozen shell now forwards one member (stagedContext,) instead of building the factory inline. The restoreAttachments entry the shell only carried for that factory is dropped from its destructure: its sole consumer was the deleted block (RevisionStagedContext declares no such member and nothing in @maka/ui reads it from the staged context). Verification: - check:renderer-architecture --base 6e21e61 --strict-base: red (11129 -> 11154) before, green (11129 == base) after; ledger diff is the single app-shell.tsx nonTriviaTokens line, no other entry moved. - desktop focused suites (app-shell-revision-actions, use-composer-quotes-revision-send, app-shell-revision-resend, new-task-staged-content): 18/18 pass. - packages/ui full suite: 710/710 pass. - typecheck green: ui tsconfig + desktop preload/main/renderer/storybook. - biome lint clean on both touched source files; check:asf-headers green. Generated-by: GLM-5.3-Flash (ZCode) Merge upstream/main (28cc4e6) Resolve the renderer-architecture.json conflict by re-pricing the ledger from the merged tree via check-renderer-architecture.mjs --write (all entries at real merged-tree values, within baseline); drop the trailing blank line at EOF in conversation-copy.ts flagged by diff-check. Merge upstream/main (0aa2707) Resolve renderer-architecture.json by re-pricing from the merged tree via check-renderer-architecture.mjs --write. Merge upstream/main (d6876d7) Resolve the renderer-architecture.json conflict by taking main's ledger (new #5851 file entries) as the base and re-pricing every entry from the merged tree via check-renderer-architecture.mjs --write; the check passes against the ratchet baseline. Merge upstream/main (d7dffca) Merge upstream/main (c838e1f) Fourth structural catch-up for #5274: carries #5868 (Composer staging moved into a persistent ComposerStagingProvider owner) and #5884 (task archive timestamps). Conflict resolution ports the revision staged-context semantics onto the new owner structure: ComposerStagingSubmission gains optional owner-key reads/clears (snapshot for captured submissions, live plate for the revision re-key) and ComposerStagingCommands gains stagedContext(); the frozen shell forwards the one composerStaging handle and the revision assembler derives the gate probe and plate reads from it. Merge upstream/main (1e80e3b) into fix/desktop-revision-structured-context # Conflicts: # apps/desktop/renderer-architecture.json Merge upstream/main (c7fa6bb) into fix/desktop-revision-structured-context # Conflicts: # apps/desktop/renderer-architecture.json Merge upstream/main (255ae23) into fix/desktop-revision-structured-context Merge upstream/main (229e1b4) into fix/desktop-revision-structured-context Merge upstream/main (f7633c3) into fix/desktop-revision-structured-context Merge upstream/main (7c90bac) into fix/desktop-revision-structured-context Merge remote-tracking branch 'upstream/main' into fix/desktop-revision-structured-context # Conflicts: # apps/desktop/renderer-architecture.json fix(desktop): drop the stale regenerate footer copy #5468 removed the Regenerate feature (87ff279), deleting the footer's `regenerate` label and the `regenerateRunning` / `regenerateAgain` / `regenerate` / `requestRegenerate` keys from the conversation copy. This branch's stale base still carried the block, so the merge-base diff re-adds those keys across the contract type and all three locales (including the zh-TW 本輪迴答 wording) while nothing on current main references them — merging would resurrect dead strings. Drop the block from the type and the zh-CN / zh-TW / en entries, matching the upstream shape exactly; the `lineage.regenerated*` keys are a different, live feature and stay. Generated-by: GLM-5.3-Flash (ZCode)
Part of #5825 (first M3 PR). It adds a durable archive time that the later scoped cleanup ("archived more than N days") and retention steps from #5776 build on, and it shows that time to users now.
Why
Sessions have
isArchivedbut no record of when they were archived.committed_atchanges on every metadata update, so it can't stand in for one. Without a durable time there is no honest "archived more than N days" filter, and no retention clock.What changes
Storage (session metadata schema 40 → 41)
session_metadata.archived_at(ms). There is no backfill. Rows archived before this change keepNULL, which means "unknown". The old payload time was stripped by migration 27 (refactor: make isArchived the sole session archive authority #3074), so there is nothing to recover.deleted_at.SessionHeaderpayload, which existing tests keep free ofarchivedAt.Protocol (epoch 200 → 201)
SessionCatalogProjection.archivedAtis optional; an absent value means unknown.archivedAton a session that isn't archived. The Host projection emits the field only when the session is archived, so a stray value degrades to "no time" rather than dropping the task.Desktop
Docs.
SessionSummary.archivedAtis documented as metadata only.isArchivedstays the single archive state: unlike Projects,archivedAt === undefineddoes not mean "not archived".Test doubles. The memory store now tracks the time too.
Verification
deleted_at), and v40 → v41 with legacy rows leftNULLand unrewritten;dist, and each break fails its test.protocol-epoch-check(200 → 201),check-renderer-architecture --strict-base, locale hygiene, the Astryx inventory, the desktop typecheck andgit diff --check.archived_atin the DB. The screenshot shows the three states next to the M1 sizes:The "3 days ago" and "unknown" rows were set directly in the DB copy to stand in for older and legacy data.
Not in this PR
"Archived more than N days" and project filters, batch delete preview, and retention. Those come next in #5825 and #5776.
AI use
Implemented and reviewed with Claude Code; the commits carry a
Co-Authored-Bytrailer.🤖 Generated with Claude Code