feat: persist starred sessions in SQLite database

[userFRM]

Summary

• Adds starred_sessions table to SQLite schema for server-side star persistence
• New REST API endpoints: GET /starred, PUT /sessions/{id}/star, DELETE /sessions/{id}/star, POST /starred/bulk
• Rewrites frontend StarredStore to load from API with optimistic updates
• Automatic migration: existing localStorage stars are bulk-inserted into the DB on first load, then localStorage is cleared
• All existing tests pass; no changes to existing behavior

Closes #93

Changes

Layer	File	What
Schema	internal/db/schema.sql	New starred_sessions table
Schema probe	internal/db/db.go	Detect table for auto-rebuild
DB layer	internal/db/starred.go	StarSession, UnstarSession, ListStarredSessionIDs, BulkStarSessions
Handlers	internal/server/starred.go	Star/unstar/list/bulk handlers
Routes	internal/server/server.go	4 new route registrations
Frontend API	frontend/src/lib/api/client.ts	listStarred, starSession, unstarSession, bulkStarSessions
Frontend store	frontend/src/lib/stores/starred.svelte.ts	API-backed store with optimistic updates + localStorage migration
App init	frontend/src/App.svelte	Call starred.load() on mount

Test plan

• go vet ./... — clean
• go test ./... — all pass
• npm run build — clean
• API verified via curl: star, unstar, list, bulk endpoints all return correct responses
• Manual: star a session → refresh page → star persists (no localStorage)
• Manual: pre-existing localStorage stars migrate to DB on first load

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (32ffb39)

Summary Verdict: The PR successfully implements SQLite persistence for starred sessions, but contains a
high-severity data migration bug and several medium-severity race conditions and error handling issues.

High

Bulk migration fails entirely when any local ID is stale/nonexistent

• Files: internal/db/starred.go (around line 73-74), frontend/src /lib/stores/starred.svelte.ts:27, 46
• Issue: BulkStarSessions runs in one transaction and returns on the first insert error. If localStorage contains a starred session ID that no longer exists in the sessions table (e.g.,
  it was pruned or deleted from disk), the query fails with a foreign key constraint violation (REFERENCES sessions(id)). This aborts the entire migration transaction, causing the frontend to silently retry and fail on every page load, leaving valid IDs unmigrated.
• Suggested Fix: Modify the query to only insert
  if the session actually exists in the parent table: INSERT OR IGNORE INTO starred_sessions (session_id) SELECT ? WHERE EXISTS (SELECT 1 FROM sessions WHERE id = ?) or handle FK violations gracefully within the loop to commit successful inserts.

Medium

**Race condition can persist the wrong final star state
**

• Files: frontend/src/lib/stores/starred.svelte.ts:53, 61, 76
• Issue: star() / unstar() are fire-and-forget optimistic calls with no per-session sequencing. A rapid toggle (star then unstar) can complete out of order on the server, leaving the persisted DB state opposite of the last user intent.
• Suggested Fix: Serialize operations per sessionId (queue/lock) or use request version tokens and ignore stale completions.

Initial load() can clobber
user actions made before fetch resolves

• Files: frontend/src/lib/stores/starred.svelte.ts:11, 15, 61
• Issue: load() replaces this.ids with the server response. If a user stars/unstars before
  the load completes, that optimistic local change can be overwritten and not restored.
• Suggested Fix: Either block toggles until load() completes, or merge fetched IDs with pending local operations instead of a blind replace.

Raw internal errors are returned directly to API clients

• Files: internal /server/starred.go:20, 29, 43, 56, 84
• Issue: Raw internal errors are returned directly to API clients via writeError(..., err.Error()). This can leak DB internals (schema/constraint details) and user
  -controlled values embedded in wrapped errors, increasing information disclosure risk.
• Suggested Fix: Return sanitized/generic client messages (e.g., "internal server error"), and log detailed errors server-side only. Optionally map known DB errors to safe typed responses.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (c9a6b7e)

Verdict: This PR successfully implements persistent SQLite storage for starred sessions with frontend integration and legacy migration, but contains a medium-severity transaction handling bug during bulk migration.

Medium

Bulk migration can
fail entirely on a single stale ID

• Location: internal/db/starred.go:79-80
• Problem: BulkStar Sessions uses INSERT OR IGNORE, but this does not suppress foreign key violations (e.g., if a session ID from localStorage no longer exists in the sessions table). Because this executes inside a transaction, a single invalid ID causes stmt.Exec(id) to error and abort the
  transaction. This leads to the entire migration batch failing silently and the frontend endlessly retrying on every page load.
• Suggested fix: Filter the IDs to ensure they exist in the sessions table before inserting, or use an INSERT ... SELECT ... WHERE EXISTS (...) statement so missing sessions are skipped instead of failing the transaction
  .

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[userFRM]

Addressed remaining roborev feedback in 286a496:

• Bulk migration fails on stale IDs (medium) — BulkStarSessions now uses INSERT ... SELECT ? WHERE EXISTS (SELECT 1 FROM sessions WHERE id = ?) so stale session IDs (pruned/deleted from disk) are silently skipped instead of causing a foreign key violation that aborts the entire migration transaction.

Previously addressed in c9a6b7e:

• Error sanitization (generic messages + server-side logging)
• Load/toggle race condition (merge instead of blind replace, deduplicate concurrent loads)

[roborev-ci]

roborev: Combined Review (286a496)

Summary Verdict: The PR introduces persistent session starring but contains a high-severity schema migration regression and medium-severity frontend state inconsistencies that need addressing.

High

• Destructive schema rebuild triggered for existing users
  File: internal
  /db/db.go:212
  needsSchemaRebuild now probes starred_sessions.session_id. On any pre
  -feature DB, that table is missing, so Open() treats the schema as stale and drops/recreates the entire DB. This is a major regression (forced full resync, potential state loss side effects during the migration window).
  Suggested fix: Do not gate startup on this new table in needs SchemaRebuild; rely on CREATE TABLE IF NOT EXISTS (or add a non-destructive table migration path).

Medium

• Optimistic rollback race can apply stale failure over newer user intent
  File: frontend/src/lib/stores/starred.svelte.ts lines
  83-87 and 97-100
  star()/unstar() fire async calls and unconditionally revert on failure using the current state. If a user toggles quickly, an older failed request can rollback a newer successful action, leaving the UI state inconsistent with the server.

  Suggested fix: Track per-session operation version/token and only rollback if the failed request is still the latest operation for that session, or serialize requests per session.

• Bulk migration can re-add stale IDs to client state even when backend skipped them
  Files: frontend/ src/lib/stores/starred.svelte.ts lines 50-53, internal/db/starred.go lines 74-80
  The backend now intentionally skips nonexistent session IDs during bulk migration, but the frontend still adds all toMigrate IDs into
  this.ids after success. That reintroduces skipped stale IDs into in-memory state (incorrect star count/state until reload).
  Suggested fix: After bulk migration, refresh from listStarred() (canonical source), or make the bulk endpoint return inserted IDs and only add those.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[userFRM]

Addressed in 0522e22:

• Schema rebuild regression (high) — Removed starred_sessions from needsSchemaRebuild probes. Table uses CREATE TABLE IF NOT EXISTS so it's created on startup without triggering a destructive rebuild on pre-feature databases.
• Bulk migration re-adds stale IDs — After bulkStarSessions, frontend now refreshes from listStarred() (canonical source) instead of blindly adding toMigrate IDs.
• Optimistic rollback race — Added per-session version tokens to star()/unstar(). Stale error callbacks check version before reverting, so rapid toggles no longer apply outdated state.

[roborev-ci]

roborev: Combined Review (0522e22)

Summary verdict: The PR implements backend database persistence for starred sessions but introduces a data-clobbering race condition during migration and a state bug that prevents recovery from transient load failures.

Medium

1. Optimistic Updates Clobbered During Migration

• File: frontend/src/
  lib/stores/starred.svelte.ts#L50
• Issue: migrateLocalStorage() performs a hard overwrite
  of the store's state (this.ids = new Set(refreshed.session_ids);) after network calls. If a user stars or unstars a session while the background migration is in flight, those optimistic updates will be clobbered and erased.
• Suggested Fix: Merge the successfully migrated server
  IDs with the current in-memory/pending optimistic state instead of replacing the local set entirely, or serialize the migration so user mutations cannot interleave. Example merge logic:

  const refreshedSet = new Set(refreshed.session_ids);
  const next = new Set(this

.ids);
for (const id of toMigrate) {
if (refreshedSet.has(id)) next.add(id);
}
this.ids = next;

**2. Unrecoverable State on Transient Load Failure**
* **
File:** [frontend/src/lib/stores/starred.svelte.ts#L35](/home/roborev/.roborev/clones/wesm/agentsview/frontend/src/lib/stores/starred.svelte.ts#L35)
* **Issue:** In
the `doLoad()` error path, the store sets `loaded = true` after falling back to localStorage. A transient backend failure at startup will permanently skip server reload/migration for the rest of the application session.
* **Suggested Fix:** Keep `loaded` false on fallback (or schedule a retry/backoff),
so a subsequent `load()` call can successfully recover the server truth.

---
*Synthesized from 3 reviews (agents: codex, gemini | types: default, security)*

[roborev-ci]

roborev: Combined Review (f6734fd)

The database persistence for starred sessions looks good overall, but there are a couple of medium-severity state synchronization issues in the frontend store to address.

Medium

Stale load response can override a newer local unstar action
Files: frontend/src/ lib/stores/starred.svelte.ts:23, frontend/src/lib/stores/starred.svelte.ts:104
doLoad() unions server IDs with current this.ids. If a user stars then quickly unstars while initial load is in flight, a stale
listStarred() response containing that ID can re-add it, and unstar() success does not re-apply state (only failure path reverts).
Suggested fix: Track pending desired state per session (star/unstar) and reapply it after load/migration responses, or ignore load results when
operation version changed after the request started.

Migration can succeed server-side but leave client state stale for the session
Files: frontend/src/lib/stores/starred.svelte.ts:28, frontend/src/lib/stores/starred.svelte.ts:50
loaded is set to true before migration finishes. If bulkStarSessions() succeeds but the follow-up listStarred() fails, migrated IDs may be persisted in DB but not reflected in this.ids, and there is no same-session retry path (load() short-circ
uits when loaded is true).
Suggested fix: Set loaded = true only after migration fully completes, or on migration refresh failure keep a retryable state and preserve local fallback IDs until a successful refresh.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (192c63c)

Summary Verdict: The PR successfully implements backend persistence for starred sessions, but contains a medium-severity race condition in the frontend store's loading logic that needs addressing.

Medium

Regression/race: Stale initial load can drop unrelated existing starred IDs

• Location: [frontend/src/lib/stores/starred.svelte.ts lines 21-32, 79](/home/roborev/.roborev/clones/wesm/
  agentsview/frontend/src/lib/stores/starred.svelte.ts)
• Description: In doLoad(), if a user optimistically toggles any star while the initial load is in flight (this.loadVersion !== vBefore), the fetched server response (res.session_ids) is completely discarded
  . Because this.loaded is still marked as true, preexisting DB stars unrelated to the toggled session are lost in the UI until a full page reload.
• Suggested Fix: Do not completely discard the server response. Reconcile stale loads per-session by merging the fetched res.session _ids with this.ids while preserving local optimistic changes (e.g., ensuring optimistically unstarred items remain removed and new stars remain added), or schedule an immediate follow-up reload before finalizing loaded=true.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (59e6afe)

Summary: The pull request successfully implements SQLite persistence for starred sessions with a migration path, but contains a few Medium severity issues related to frontend state synchronization and testing gaps that need to be addressed.

Medium

1. Stale migration refresh can overwrite newer optimistic user actions

• Files: starred.
  svelte.ts:57, [starred.svelte.ts](/home/roborev/.roborev/clones/wesm/agents
  view/frontend/src/lib/stores/starred.svelte.ts):59
• Details: migrateLocalStorage() fetches listStarred() and then re-adds toMigrate IDs from that response without checking whether the user toggled those IDs while the migration requests were in flight. That
  can reintroduce a star the user just removed, leaving UI/backend temporarily or permanently inconsistent.
• Suggested Fix: Version/snapshot-guard this refresh (same stale-response protection as load), e.g., apply refreshed IDs only if no newer op exists for that ID.

2. opVersions is used as a permanent blocklist during load reconciliation

• Files: starred.svelte.ts:30, starred.svelte
  .ts:78
• Details: doLoad() skips any server ID where opVersions.has(id). Since entries are
  never cleared, any session toggled once can be excluded from later load reconciliation forever (especially after retries/failures), causing drift from DB truth.
• Suggested Fix: Track only in-flight ops (clear on settle), or tie skip logic to a per-request epoch rather than has(id) lifetime.

3. Missing race-focused tests for new starred flow

• Files Impacted: Frontend store and starred API/DB paths
• Details: No tests in this range validate:
  • Toggle during initial load() + migration reconciliation
  • Out-of-order star/un
    star request completion
  • Concurrent delete during PUT /sessions/{id}/star
• Suggested Fix: Add frontend store unit tests for stale response handling and backend handler/DB tests for concurrent edge cases.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (6f7e54f)

Summary Verdict: The PR successfully migrates starred sessions to SQLite with optimistic UI updates, but contains medium-severity race conditions in the frontend state management.

Medium

1. Migrated stars can be lost from in-memory state during concurrent user mutations
Files: starred.svelte. ts lines 48, 55, 63
migrateLocalStorage() clears local storage even when it skips applying refreshed server state
due to a mutationVersion mismatch. If a user toggles stars during migration, migrated IDs can be written to the DB but never merged into ids in this session.
Suggested fix: Only clear local storage after successfully reconciling state, or merge migrated IDs into current ids when the refresh is
discarded.

2. Optimistic rollback can produce incorrect final UI state when multiple ops on same ID fail
Files: [starred.svelte.ts](/home/roborev/.roborev/clones/wesm/agentsview/frontend/src/lib/stores/starred.
svelte.ts) lines 84, 103
With rapid star -> unstar on the same session, if both requests fail, the final rollback path can leave the item starred even though neither request succeeded server-side.
Suggested fix: Track a confirmed base state + pending op queue
(or re-fetch on failure) instead of independent inverse rollbacks.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (5b70acc)

Verdict: The PR implements persistent session starring with a SQLite
backend and frontend integration, but introduces High and Medium severity concurrency and race conditions in frontend state management.

High

• File: [frontend/src/lib/stores/starred.svelte.ts#L76](/home/roborev/.roborev/clones/wesm/agentsview
  /frontend/src/lib/stores/starred.svelte.ts#L76), #L87
  Problem:
  star() and unstar() fire independent async requests without per-session sequencing. Rapid toggles (star then unstar) can complete out of order on the server, so persisted DB state can end up opposite of the user’s last action.
  Suggested fix: Serialize mutations per sessionId (queue/lock) or implement last-write-wins with per-session operation tokens and a post-response reconciliation to the latest desired state.

Medium

• File: [frontend/src/lib/stores/starred.svelte.ts#L98](/home/roborev/.rob
  orev/clones/wesm/agentsview/frontend/src/lib/stores/starred.svelte.ts#L98), [#L100](/home/roborev/.roborev/clones/wesm/agentsview/frontend/src/lib/stores/starred
  .svelte.ts#L100)
  Problem: reconcileAfterError() applies listStarred() wholesale if mutationVersion is unchanged, but that version does not track in-flight request completion. A failed request can trigger reconcile that overwrites optimistic state for another still-in
  -flight request, causing UI/server drift until next reload.
  Suggested fix: Track pending operations per session and merge server results with pending desired state, or defer full reconcile until no pending mutations remain.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (95c9bda)

Summary Verdict: The implementation for backend starred-session persistence is solid, but there
are a couple of medium-severity issues related to state reconciliation races and missing test coverage that should be addressed.

Medium

• Stale refresh can overwrite newer state during migration/reconcile races

  • Files: frontend/src/lib/stores/starred.svelte.ts:47,
    frontend/src/lib/stores/starred.svelte.ts:114
  • Details: migrateLocalStorage() and reconcileIfIdle() can both issue listStarred() calls under the same mutationVersion. If responses return out of order, an older response can overwrite
    a newer one because both pass the same version check.
  • Suggested fix: Add a monotonic refreshRequestId (or equivalent) and only apply the latest response, or route migration refresh through the same "idle reconcile" path so there is a single authoritative refresh pipeline.

• Testing gap for new
  race-sensitive behavior

  • Files: frontend/src/lib/stores/starred.svelte.ts:20, internal/server/starred.go:1, internal/db/starred.go:1
  • Details: The diff adds concurrency-critical client
    logic and new DB/API paths, but no corresponding tests are included in this range.
  • Suggested fix: Add unit/integration tests for:
    1. Load + migration + in-flight toggle ordering (out-of-order listStarred responses).
    2. Per-session
       mutation serialization.
    3. Bulk migration behavior with stale IDs and duplicate IDs.
    4. Handler status/error behavior for star/unstar/list/bulk endpoints.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (dd7df79)

Verdict: The PR successfully implements persistent starred
sessions, but there is a medium-severity issue regarding migration failure handling in the frontend store.

Medium

• File: [starred.svelte.ts#L24](/home/roborev/.roborev/clones/wesm/agentsview/frontend/src/lib/stores/starred.
  svelte.ts#L24), starred.svelte.ts#L42
  doLoad() marks
  loaded = true before/while migration runs, and migrateLocalStorage() swallows failures. If migration fails, legacy local stars can disappear from the UI for the rest of that app session (no further load() because loaded is already true).
  Suggested fix: On migration failure,
  keep/merge local IDs in memory and schedule a retry; or only set loaded = true after the migration path is resolved with a safe fallback.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (15c9655)

Verdict: The PR successfully implements persistent starred sessions, but contains a medium-severity bug in the legacy local data migration logic that could resurrect unstarred sessions.

Medium

Stale migration success can leave local data behind and re-apply old stars later
File:
frontend/src/lib/stores/starred.svelte.ts:50
clearLocalStorage() is only called if the post-migration
listStarred() response passes the mutationVersion/refreshId guard. If bulkStarSessions() succeeds but the refresh response is discarded as stale, legacy local IDs remain. On a later reload, those IDs can be migrated again and re-star sessions the user has since unstarred.
Suggested fix
: Clear localStorage immediately after successful bulkStarSessions(toMigrate) (or persist a one-time migration marker), and keep staleness checks only for applying in-memory ids.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (7edcad7)

Summary Verdict: The pull request successfully migrates starred sessions to a SQLite-backed server API, but introduces a medium-severity race condition during load failures that needs addressing.

Medium

Stale-load fallback can drop legacy local stars during load failure race
File: [/home
/roborev/.roborev/clones/wesm/agentsview/frontend/src/lib/stores/starred.svelte.ts#L35](/home/roborev/.roborev/clones/wesm/agentsview/frontend/src/lib/stores/starred
.svelte.ts#L35)

In doLoad() catch, local fallback is only applied when mutationVersion and refreshId still match (L37). If a user toggles during initial load and listStarred() fails, fallback is skipped and prior local stars are not merged
into memory, so they disappear for that app session.

Suggested fix: When load fails, merge readLocalStorage() into current ids (or otherwise reconcile) instead of fully gating fallback on stale checks.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (b13ec2f)

Summary Verdict: The PR successfully migrates starred sessions to a server-backed SQLite store, but introduces a potential database race condition and lacks test coverage for complex frontend store behaviors.

Medium

Possible incorrect 500 on star due check-then-write race

Locations: internal/server/starred.go:18, [internal/server/starred.go:32](/home/roborev/.robore
v/clones/wesm/agentsview/internal/server/starred.go:32), internal/db/starred.go:12

handleStarSession does GetSession then StarSession in separate DB operations. If the session is deleted between those calls, INSERT can fail with an FK error and return 500 instead of a stable not-found outcome.
Suggested fix: Make the star operation atomic in
SQL (INSERT ... SELECT ... WHERE EXISTS) and branch on affected rows, or map FK constraint failures to 404.

Missing frontend tests for new race-heavy starred store behavior
Locations: [frontend/src/lib/stores/starred.svelte.ts:25](/home/
roborev/.roborev/clones/wesm/agentsview/frontend/src/lib/stores/starred.svelte.ts:25), [frontend/src/lib/stores/starred.svelte.ts:58](/home/roborev/.roborev/clones
/wesm/agentsview/frontend/src/lib/stores/starred.svelte.ts:58), [frontend/src/lib/stores/starred.svelte.ts:125](/home/roborev/.roborev/clones/wesm/agentsview/frontend/
src/lib/stores/starred.svelte.ts:125)
This range adds complex load/migration/reconcile sequencing (mutationVersion, refreshId, per-session queues) but lacks unit coverage in the range.
Suggested fix: Add store tests covering stale load discard, in
-flight toggle ordering, migration success/failure, and reconcile behavior after failed mutations.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (71dd44a)

The PR successfully introduces database persistence for starred sessions, but contains a high-severity idempotency regression in the API and a medium-severity state migration edge case in the frontend.

High Severity

• Idempotency regression in PUT /sessions/{id }/star
  • Locations: /internal/server/starred.go:26, /internal/db/starred.go:16, /internal/db/starred.go:22-23
  • Description: db.StarSession returns
    false when RowsAffected() == 0. Because the query uses INSERT OR IGNORE, zero rows are affected both when a session does not exist AND when a session is already starred. handleStarSession incorrectly translates this false into a 404 Not Found response. This breaks idempot
    ency by causing valid, redundant star requests (e.g., from multiple tabs or UI retries) to return an HTTP 404 error instead of succeeding.
  • Suggested Fix: Update db.StarSession to differentiate between "already starred" and "does not exist" (e
    .g., by executing a SELECT EXISTS on the sessions table when RowsAffected() == 0 to accurately return true if it is already starred) so existing stars map to 204 and missing sessions map to 404.

Medium Severity

• Legacy
  localStorage migration can restore unstarred sessions

  • Locations: /frontend/src/lib/stores/starred.svelte.ts:62, /frontend/src/lib/stores/starred.svelte.ts:67, /frontend/src/lib/stores/starred. svelte.ts:116

  • Description: The migration source reads from localStorage, but unstar() no longer updates it. If a user unstars a session during a failure/offline flow and the migration retries later, stale local IDs can be bulk-starred again.

  • Suggested Fix: While the legacy key exists, mirror user star/unstar mutations into the localStorage set, or maintain a "locally removed" exclusion set to be used by the migration logic.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (0aa80ad)

Verdict: The changes are secure and well-structured, but there is a concurrency issue in the frontend store initialization that requires addressing.

Medium

File: frontend/src/lib/stores/starred.svelte.ts:16, frontend/src/lib/
stores/starred.svelte.ts:33

load() checks loaded before loading, but loaded is set to true before migrateLocalStorage() finishes. A concurrent second load() call can return immediately while the first load is still running, allowing callers to observe a partially-initialized state.

Suggested fix: Check loading first, and/or only set loaded = true after
the migration completes (or consistently return Promise.resolve() when already loaded).

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (359f469)

Summary Verdict: The PR successfully implements persistent starred sessions, but a state-handling bug during migration needs addressing.

Medium

1. Stale IDs can be reintroduced after successful migration if refresh fails
   • File: frontend/src/lib/stores/starred.svelte.ts: 89
   • Problem: In migrateLocalStorage(), bulkStarSessions() intentionally skips non-existent session IDs, but on subsequent listStarred() failure the code merges all toMigrate IDs back into memory. That can resurrect stale/non-existent stars and skew count/filters until a
     later successful refresh.
   • Suggested fix: On refresh failure after successful bulk insert, do not blindly merge toMigrate. Either keep the current state and schedule a retry reconcile, or return applied IDs from the bulk API and merge only those.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[wesm]

I can work on viewing and merging this? let me know when you feel like you're at a stopping point

[roborev-ci]

roborev: Combined Review (3945790)

Summary: The changes successfully implement backend persistence for starred sessions, but contain a medium-severity state management issue.

Medium

1. Post-migration refresh failure can leave UI permanently stale
   Files:

• frontend/src/lib/stores/starred.svelte.ts:82

  • frontend/src/lib/stores/starred.svelte.ts:90
  • frontend/src/lib/stores/starred.svelte.ts:39

  After
  bulkStarSessions succeeds, localStorage is cleared immediately. If the follow-up listStarred() fails, the code intentionally does not merge toMigrate back, and loaded is still set true. With no later star/unstar action, reconcileIfIdle() never runs
  , so migrated stars may stay hidden in memory for the entire session.

  Suggested fix: Trigger an explicit retry/reconcile in that catch path (or defer loaded=true until at least one successful post-migration refresh).

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (5309aab)

Summary Verdict: The PR successfully implements SQLite-backed persistence for starred sessions with
optimistic UI updates, but contains a medium-severity race condition during state migration.

Medium

Stale migration refresh can overwrite in-flight optimistic changes

• Files: /home/roborev/.roborev/clones/wesm/agentsview/frontend/src/lib/ stores/starred.svelte.ts:70, /home/roborev/.roborev/clones/wesm/agentsview/frontend/src/lib/stores/starred.svelte.ts:86
• Details: migrateLocalStorage() snapshots mutationVersion, then later applies
  listStarred() if the version is unchanged. The version changes only when a mutation is queued, not while it is still in-flight. If a star/unstar request is still pending, this refresh can apply pre-mutation server state and temporarily revert the optimistic UI state.
• Suggested Fix: Only apply
  the migration refresh when no mutations are in flight (queues.size === 0), or defer the refresh application to reconcileIfIdle() after the queue drains.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (66227e3)

Summary Verdict: The implementation of SQLite persistence for starred sessions is solid, but there is a data-loss risk during frontend migration that needs to
be addressed.

Medium

• Location: frontend/src/lib/stores/starred.svelte.ts:57, frontend
  /src/lib/stores/starred.svelte.ts:90

  migrateLocalStorage() computes toMigrate from !this .ids.has(id) and clears localStorage when toMigrate.length === 0. This can drop legacy stars if this.ids contains only optimistic values (from an in-flight star()), rather than confirmed server state. If that queued star later fails, the legacy localStorage backup has already been removed, causing potential data loss on reload.

  Suggested fix: Only clear localStorage for IDs that are confirmed persisted (via successful bulk migration or successful per-ID mutation), or defer clearing when any local ID is currently in queues or otherwise only optimistically
  present.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (4dd3b20)

Verdict: The PR successfully implements server-side SQLite persistence for
starred sessions with frontend migration, but contains a medium-severity race condition in the local fallback logic that needs addressing.

Medium

Local fallback load can overwrite optimistic in-flight state
File: [starred.svelte.ts:43](/home/roborev/.roborev/clones/
wesm/agentsview/frontend/src/lib/stores/starred.svelte.ts:43)
In doLoad()’s catch, the replace path only checks mutationVersion/refreshId, not queues.size. If a mutation queue already exists when load() starts,
this.ids = local can clobber optimistic state.
Suggested fix: Gate the replace path with this.queues.size === 0; otherwise use the merge path.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (fe395ac)

Verdict: The pull request successfully migrates starred sessions to SQLite, but introduces a medium-severity state reconciliation issue during migration failure.

Medium

• Stale IDs can be reintroduced after successful bulk migration when refresh fails
  • File
    : [/home/roborev/.roborev/clones/wesm/agentsview/frontend/src/lib/stores/starred.svelte.ts#L115](/home/roborev/.roborev/clones/wesm/agentsview/frontend/src/lib/stores
    /starred.svelte.ts#L115)
  • Problem: BulkStarSessions intentionally skips non-existent session IDs, but on refresh failure the store merges all toMigrate IDs back into memory (for (const id of toMigrate) merged.add(id) ), including IDs that were skipped as stale. This can leave phantom starred IDs in the UI state for the rest of the session.
  • Suggested fix: Make the bulk API return actually-applied IDs and merge only those on refresh failure, or schedule a guaranteed follow-up reconcile instead of merging unknown IDs.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (d20f814)

Verdict: The changes are generally solid, but there is one medium-severity issue regarding retry logic in the starred store reconciliation
.

Medium

• File: starred.svelte.ts (~lines 200-225)
  • Issue:
    reconcileRetries is reset to 0 on any successful listStarred() response, even when that response is discarded as stale (refreshId/mutationVersion mismatch).
  • Why it matters: A stale success can consume the retry chain without applying data, so post-migration reconciliation
    may stop early and leave the UI state outdated until another unrelated refresh/reload.
  • Suggested fix: Reset reconcileRetries only when the response is actually applied. If the response is stale, either keep the retry budget unchanged or schedule another reconcile attempt.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[wesm]

thanks!