refactor(desktop): address a conversation by its session id alone - #888
refactor(desktop): address a conversation by its session id alone#888tonyfettes wants to merge 1 commit into
Conversation
Session ids are unique, so nothing needs a store beside one to say which
record it means. Every mechanism that existed to keep same-id records in
different stores apart is gone:
Wire — `session.load`, `session.load_archived`, and `session.delete_archived`
take `{session}`; the host locates the record across the global store and each
registered workspace, exactly as the id-only ops (`session.archive`,
`session.unarchive`) always did. `session.event` and `session.changed` drop
their store half. `agent.started`/`agent.connected` keep `session_root`: that
answers which project a run belongs to, not which of several records it is.
Host — `ArchivedStoreSelection` and the `selected` path through
`claim_session_family_move` are gone, along with the live-twin refusal and the
detach revalidation that only existed for a client-chosen store.
`SessionFamilyMove` no longer carries a protocol store token, so every
`session.changed` callback is `(session) -> Unit`.
Client — `ArchiveRecordKey` collapses to the id: archive overrides,
tombstones, the delete confirmation, and the archived/live list reconciles all
key by session. `SessionTreeKey` and the `root` half of a sidebar row's
component id go with it. Row keys are `(source, channel, conversation)`.
Tests asserting the old semantics are removed rather than rewritten — they
described states that can no longer occur.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6601faf3c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// The read-only record ops — `session.load`, `session.load_archived` — and | ||
| /// permanent deletion address a conversation by its session id alone. Ids are | ||
| /// unique across every store, so the host locates the record itself instead of | ||
| /// being told where to look. | ||
| pub(all) struct SessionRecordPayload { | ||
| session : String |
There was a problem hiding this comment.
When an existing or externally created session ID occurs in more than one attached store, session.delete_archived now sends only that ID, and archived_record_root selects the first matching store (global first) rather than the sidebar row the user confirmed. Deleting a project copy can therefore permanently delete an unrelated Scratch copy, while loads and notifications can likewise target the wrong history. The code does not enforce the asserted global uniqueness when accepting client-supplied session IDs, so retain a store qualifier or explicitly reject ambiguous IDs before any destructive operation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
“When an existing or externally created session ID occurs in more than one attached store” How's that possible?!
1 participant
Session ids are unique, so nothing needs a store beside one to say which record it means. This removes every mechanism that existed to keep same-id records in different stores apart.
mainhad accumulated that machinery across #878 and the unmerged #886 line of work. This branch starts frommaininstead of continuing it.Wire
session.load{session, workspace?}{session}session.load_archived{session, workspace?}{session}session.delete_archived{session, workspace}{session}session.event{session, sequence, session_root, event}{session, sequence, event}session.changed{change, session, workspace}{change, session}The host locates a record across the global store and each registered workspace — exactly what the id-only ops (
session.archive,session.unarchive) always did.agent.startedandagent.connectedkeepsession_root. That field answers which project a run belongs to (which drives the checkout), not which of several same-id records this is, so it is not a uniqueness backstop.LoadSessionPayloadandArchivedSessionPayloadmerge into oneSessionRecordPayload.Host
ArchivedStoreSelectionand theselectedpath throughclaim_session_family_moveare gone, along with the live-twin refusal and the detach revalidation that only existed to police a client-chosen store.SessionFamilyMoveno longer carries a protocol store token, so everysession.changedcallback is(session) -> Unit.load_session/load_archived_sessiondrop the workspace hint and always resolve through the existing by-id store search.Client
ArchiveRecordKeycollapses to the session id, takingmatches,from_protocol, andworkspace_payloadwith it. Archive overrides, delete tombstones, the delete-confirmation dialog, and both list reconciles key by id.SessionTreeKeycollapses to the id.roothalf of a sidebar row's component id is gone: row keys are(source, channel, conversation).open_session/refresh_session/load_session_cmdno longer thread a workspace to the request.Tests
Tests asserting the old semantics are deleted rather than rewritten — they describe states that can no longer occur:
a commit for an unlisted store refreshes past a same-id rowa commit for an unlisted store refreshes past a same-id watermarka rooted commit places its conversation without waiting for the listswitching same-id archived stores fences the first transcript loadarchived deletion keeps same-id rows in other storesdeleted broadcast rotates the active conversation's exact storearchive facts rotate the open conversation's exact storesame-id records in Scratch and a project store keep distinct row keyssession trees never fold children across storespermanent deletion uses the selected archived store,permanent deletion refuses a live twin in the selected store,selected archived store is revalidated after detachment,load rejects a detached workspace hint instead of probing globalTests about cross-channel same-id conversations are kept — that axis is unaffected.
Compatibility
This is a breaking wire change on those five ops: a client and host of different vintages will no longer interoperate on them.
docs/remote-protocol.mdalready states the desktop frontend and host ship in lockstep, so the old-host compatibility paragraph is removed rather than given a fallback.Verification
moon check+moon fmtclean on--target jsand--target nativemoon test --target js— 2926/2926moon test --target native— 3451/3451Not exercised live: archive / unarchive / permanent delete against a running desktop build. Worth an E2E pass on those three before merge, plus opening an archived row from the sidebar.
🤖 Generated with Claude Code