feat: migrate session metadata storage from frontend overlay to backend#8769
feat: migrate session metadata storage from frontend overlay to backend#8769
Conversation
![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: cbeabc220a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const acpSessions = await acpListSessions(); | ||
| const mergedAcpSessions = sortByUpdatedAtDesc( | ||
| acpSessions.map((session) => | ||
| mergeAcpSessionWithOverlay(session, overlays.get(session.sessionId)), | ||
| ), | ||
| const sessions = sortByUpdatedAtDesc( | ||
| acpSessions.map(acpSessionToChatSession), | ||
| ); |
There was a problem hiding this comment.
This reload path now replaces the entire store with acpListSessions() results, but the backend list endpoint still returns only unarchived threads (on_list_sessions calls list_threads(false) in crates/goose/src/acp/server.rs). Since this commit also starts persisting archive actions to backend state, archived sessions disappear after the next reload/startup and can no longer be surfaced for unarchive in the UI. Fetch archived sessions as well (or merge them separately) before replacing sessions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d59f5cfc0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const acpSessions = await acpListSessions(); | ||
| const mergedAcpSessions = sortByUpdatedAtDesc( | ||
| acpSessions.map((session) => | ||
| mergeAcpSessionWithOverlay(session, overlays.get(session.sessionId)), | ||
| ), | ||
| const sessions = sortByUpdatedAtDesc( | ||
| acpSessions.map(acpSessionToChatSession), | ||
| ); |
There was a problem hiding this comment.
loadSessions now replaces the store directly from acpListSessions() and no longer reads the old local overlay/legacy caches. That means users upgrading from the previous build lose metadata that only existed in goose:acp-session-metadata / goose:chat-sessions (for example previously renamed titles, project/provider/persona/model selections) on the first reload, because nothing backfills those values into backend state before this overwrite. Add a one-time migration path that reads legacy records and persists them via backend session APIs before switching fully to ACP-only hydration.
Useful? React with 👍 / 👎.
| self.thread_manager | ||
| .update_thread(&req.session_id, Some(req.title), Some(true), None) | ||
| .await |
There was a problem hiding this comment.
The rename RPC updates only the thread record, but other backend flows still read the internal session record (for example on_export_session goes through session_manager.export_session). After a rename, list sessions can show the new title while exported/imported session payloads still carry the old one, creating backend state drift introduced by this endpoint. Update the linked internal session name/user-set flag alongside thread_manager.update_thread so both stores stay consistent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12ac0a5f36
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| acpArchiveSession(session.acpSessionId).catch((err: unknown) => | ||
| console.error("Failed to archive session in backend:", err), | ||
| ); |
There was a problem hiding this comment.
archiveSession/unarchiveSession are declared async and are awaited by callers, but the backend write is launched fire-and-forget via .catch(...) instead of being awaited. In practice, UI flows proceed as if persistence succeeded (or users can close the app immediately) while the RPC may still be in flight or fail, leaving backend archive state out of sync until a later reload.
Useful? React with 👍 / 👎.
| } catch (error) { | ||
| console.error("Failed to load sessions from ACP:", error); |
There was a problem hiding this comment.
The error path now only logs and skips setting any fallback session data, so a transient acpListSessions() failure leaves the store empty while startup still considers hydration complete; this can trigger downstream "no sessions" flows and create new sessions even though existing backend sessions were never loaded. Keeping prior sessions (or deferring hydrated state on failure) avoids this false-empty state.
Useful? React with 👍 / 👎.
The field actually stores the model ID (e.g. claude-sonnet-4-5-20250514), not the display name. Rename it to reflect its true semantics and expose it as modelId (instead of modelName) in the ACP _meta response. The frontend already resolves display names from its model inventory, so it only needs the ID from the backend. A serde alias preserves backward compatibility with existing database rows serialized under model_name. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Accept personaId from newSession meta, persist it in ThreadMetadata, and expose it in the _meta response. The frontend now forwards personaId on session creation and prefers the backend value over the local overlay when loading sessions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
agentId was written to ChatSession and the localStorage overlay but never read back for any logic or display. The frontend already uses providerId everywhere it needs agent identity. The backend has no corresponding field. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add createdAt, archivedAt, and userSetName to the _meta map returned by thread_session_meta() so the frontend can read all session metadata from the backend instead of maintaining a local overlay. Add a _goose/session/rename endpoint that updates the thread name and sets user_set_name=true via the existing thread_manager.update_thread(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Expand AcpSessionInfo with createdAt, archivedAt, and userSetName fields now exposed by the backend. Replace mergeAcpSessionWithOverlay with a direct acpSessionToChatSession mapping that reads all metadata from the backend response. Wire archiveSession, unarchiveSession, and rename (via updateSession with userSetName) to call the corresponding backend endpoints instead of persisting to the localStorage overlay. Remove all overlay read/write calls from the session store: no more loadSessionMetadataOverlay, syncOverlaySnapshots, buildOverlayRecord, upsertSessionMetadataOverlayRecord, or overlayToFallbackSession. Remove the persistOverlay option from updateSession since the overlay is no longer written to. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The overlay module is no longer imported anywhere — all session metadata now comes from the backend. Delete the file and remove legacy migration test cases that relied on localStorage hydration. Replace the "falls back to cached sessions on error" test with a simpler "keeps empty sessions list on error" test since there is no longer a localStorage fallback. Add a test that verifies all metadata fields are read directly from the backend response. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace `(info._meta?.userSetName as boolean) ?? false` with `info._meta?.userSetName === true` in both listSessions and forkSession. The strict equality check is more defensive against unexpected types (e.g. string "true") and avoids the no-op `as` cast followed by a nullish coalescing that could never trigger (since `false ?? false` is `false`, not a fallback). Resolves code review comment on acpApi.ts lines 59-61. Signed-off-by: Matt Toohey <contact@matttoohey.com>
The thread_session_meta() changes added createdAt and userSetName to every session's _meta map. Update the test to strip the dynamic createdAt timestamp before comparison and include userSetName: false in the expected output. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collapse multi-line `expected_meta.insert` call for `userSetName` onto a single line to satisfy `cargo fmt --check`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When the user switches providers without explicitly selecting a model, update_provider() only updated provider_id in ThreadMetadata, leaving a stale model_id from the previous provider. On session reload, this produced a cross-provider mismatch (e.g. modelId "gpt-4" paired with provider "anthropic"). Clear model_id to None on provider switch to match the frontend behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The frontend already passes personaId into acpSendMessage, but it was only used for session tracker lookup — never forwarded to the backend. After a mid-session persona switch and page reload, the session list showed the stale persona from session creation. Forward personaId in the prompt request's _meta field and extract it in on_prompt to update the thread's persona_id metadata, mirroring how model_id and provider_id are already kept in sync. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
matt2e
force-pushed
the
frontend-storage
branch
from
12ac0a5 to
4d4b961
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d4b961df6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "title" in patch && | ||
| "userSetName" in patch && | ||
| patch.userSetName && |
There was a problem hiding this comment.
The new rename persistence gate requires userSetName to be present and true, so any updateSession call that only patches title stays local-only. Those local titles are then lost on the next loadSessions()/app restart when backend data overwrites the store, which regresses flows that set transient titles without marking them as user-set. Either persist non-user title updates to backend metadata as well, or avoid local title mutations that cannot survive rehydration.
Useful? React with 👍 / 👎.
2 participants
Summary
created_at,archived_at,user_set_name,persona_id,model_id) in the_metaobject returned by the backend session list endpoint, and adds a_goose/session/renameRPC endpointsessionMetadataOverlaymodule that was duplicating session metadata in localStorage, replacing all reads/writes with backend API callschatSessionStoreby removing overlay sync logic, rename-via-overlay, and persona/model tracking that now lives server-sideuserSetNameand removes deadagentIdfield from session typesTest plan
🤖 Generated with Claude Code