Uh oh!
There was an error while loading. Please reload this page.
fix(session): make unarchive work and archived sessions findable - #43919
Open
alohaninja wants to merge 4 commits into
Open
fix(session): make unarchive work and archived sessions findable#43919alohaninja wants to merge 4 commits into
alohaninja wants to merge 4 commits into
Conversation
Archiving was effectively one-way. Two defects meant the clear path could
never work, even with UI wired up, and there was no way to find an archived
session in order to restore it.
Write path:
- projector: `time_archived: info.time.archived ?? null`. Drizzle omits
undefined keys from `.set()`, so clearing the timestamp never emitted a SQL
NULL and the row kept its stale value while the request returned 200.
- session update handler: gate on presence of the `time` object instead of
`time.archived !== undefined`. A client clearing the field sends
`{"time":{}}` because JSON.stringify drops undefined-valued properties, so
the old guard skipped setArchived entirely. A title-only PATCH still leaves
archived untouched.
Restore:
- `useSessionArchive()` gains `unarchive`, alongside the existing `archive`.
- Session dropdown shows Archive or Unarchive based on state, in both the
DropdownMenu and MenuV2 variants.
- New `session.unarchive` command.
- TUI: ctrl+a toggles from the session list, labelled `archive/unarchive` to
match the existing `pin/unpin` convention.
- Event reducer increments sessionTotal when an unarchived session re-enters
the store, mirroring the existing decrement on archive.
Discovery:
- Palette search requests archived sessions through
experimental.session.list({ archived: true }) and groups them under an
"Archived" heading, sorted after active matches. Search-gated, so default
lists are unchanged.
- TUI session list groups archived sessions under "Archived" rather than
interleaving them into date buckets. The category logic is extracted as a
pure exported function and covered by tests.
Verified: create -> archive -> found only with archived=true -> unarchive via
the real client body -> cleared and persisted; title-only PATCH regression
checked. tsc clean across app/core/tui/opencode; 76 tests pass.The footer read a static "archive" even when an archived session was highlighted, so nothing indicated that ctrl+a would restore it. Widening it to "archive/unarchive" fixed the ambiguity but wrapped onto a second line at common terminal widths and pushed the quick-switch hint with it. Let DialogSelect actions derive their title from the highlighted option, the same way `disabled` already can. The session list now shows "archive" or "unarchive" — shorter than either static form, and unambiguous about what the key will do. Existing callers pass plain strings and are unaffected.
…erage
Follow-ups from review of the unarchive change.
sessionTotal could drift upward. The increment fired for any session.updated
whose session was absent from the local store, but a session trimmed out by the
window limit is still counted in sessionTotal, so an unrelated update to one
inflated the total. Only count a session that actually widened the window. A
genuine unarchive while the store is already at its limit now leaves the total
untouched rather than over-counting; the next server fetch reconciles it.
ArchivedTimestamp is Schema.Finite and deliberately accepts 0 and negative
values for legacy compatibility, so truthiness misclassifies those as active.
Test presence instead, matching the existing check in home-session-index.ts.
Affects the TUI category and toggle (where a session archived at 0 would have
been re-archived rather than restored), the dropdown menus, and palette
grouping. Covered by tests for 0 and -1.
Add integration coverage for the two core fixes. Archiving then sending
`{"time":{}}` clears the timestamp and the cleared state survives a re-read,
which exercises the handler guard and the projector's explicit null together —
a unit test on either alone would not have caught the original defect. A second
test asserts a title-only PATCH leaves the timestamp intact.
The command palette no longer offers both Archive and Unarchive; each is gated
on the current session's state, matching the dropdown and TUI.src/i18n/parity.test.ts requires each non-English dictionary to carry every English key, so adding the three unarchive strings to en.ts alone failed CI. Type-level parity was not enough: locale files are typed `satisfies Partial<Record<Keys, string>>` and compiled fine while the parity test failed. Adds common.unarchive, command.session.unarchive, and command.category.session.archived to all 61 locales, each placed next to its existing archive counterpart and worded to match the terminology that locale already uses for "archive".
Enough1122
commented
Aug 22, 2026
Diff too large for automated review — recommend human review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue for this PR
Closes#24153 (also requested in #16000 and #13463)
Type of change
What does this PR do?
Archiving a session is currently one-way. I started out thinking this only needed UI wiring, but the clear path is broken in two places, so a UI-only patch returns 200 and does nothing.
Bug 1 —
packages/core/src/session/projector.tstime_archived: info.time.archivedpassesundefinedwhen the timestamp is cleared. Drizzle omitsundefinedkeys from.set(), which means "leave this column alone", not "set NULL". The row keeps its stale archived value. Fixed with?? nullso an explicit SQL NULL is emitted.Bug 2 — the session update handler
The guard was
if (ctx.payload.time?.archived !== undefined). A client clearing the field sends{"time":{}}, becauseJSON.stringifydrops undefined-valued properties. So the guard short-circuited andsetArchivedwas never called. Now gated on presence of thetimeobject. A title-only PATCH still leavesarchiveduntouched (verified below).Restore action
useSessionArchive()gainsunarchivenext to the existingarchiveDropdownMenuandMenuV2)session.unarchivecommandctrl+atoggles from the session list, labelledarchive/unarchiveto match the existingpin/unpinsessionTotalwhen an unarchived session re-enters the store, mirroring the decrement already there for archiveDiscovery
Without this, the restore action is unreachable — you can't select a session you can't see. Palette search now requests archived sessions via
experimental.session.list({ archived: true })and groups them under an "Archived" heading, sorted after active matches. It's search-gated, so default lists are unchanged. The TUI groups archived sessions under "Archived" instead of mixing them into the date buckets.This is the shape #13463 asked for ("search archived session and un-archive for web ui"), and it also addresses the review feedback on #24154 that there was no way to pick which session to bring back.
Because
packages/desktopmountsAppInterfacefrom@opencode-ai/app, the desktop app picks this up too, which is what several commenters on #24153 were asking for.How did you verify your code works?
tsc --noEmitclean onapp,core,tui,opencode.76 tests pass:
tuisession list (10, including 5 new for the extracted category function),appcompat/helpers/home-index (42),opencodehttpapi-session + global-session-list (24).End-to-end against a local server, using the exact body the client sends rather than a hand-written one, since that distinction is the whole bug:
archived = 1700000000000archived=truearchived=true{"time":{}}archived = None, persisted on re-readarchivedunchangedAlso walked the web UI and the TUI manually: archive, find it under "Archived" in palette search, open it, Unarchive, confirm it returns to the active list.
Screenshots / recordings
Web — palette search showing active and archived results grouped separately, and the dropdown offering Unarchive for an archived session:
Archived
Unarchive
TUI — Pinned / date / Archived groups coexisting. The footer action reads
unarchivehere because an archived session is highlighted; it readsarchiveotherwise:Non-goals
Deliberately out of scope, happy to follow up:
sessionTotal/hasMoreaccounting. Worth its own PR.SHOW_HOME_SESSION_ARCHIVE = false.archivedparam on the V2/api/sessionendpoint. That belongs with the V2 migration; there's an existingTODO(v2)acknowledging it.Discovery here works without touching any of those, and without regenerating the SDK.
Locale strings are English-only, matching the existing pattern where locale files are
Partial(common.archiveis present in 62 of 65).Checklist