fix(sessions): keep new sessions visible in sidebar - #725
Open
beruro wants to merge 5 commits into
Open
Conversation
Pre-commit hook ran. Total eslint: 0, total circular: 0
Harry19081
requested changes
Aug 7, 2026
Harry19081
left a comment
Member
There was a problem hiding this comment.
Requesting changes before merge.
- Fix roster ownership for non-native creations.
- Collaboration imports are created with
category: "external_history"and animported-session-...ID.sidebarCategoryForSessioncannot assign that shape to a roster category, soregisterCreatedSessionWithNativeRosterreturns unchanged state. Please give collaboration replays an explicit authoritative roster projection and test it after an authoritative roster page has loaded. - JSON imports and collaboration forks are client-created rows, not native-roster rows. They therefore may never be acknowledged by a native page. The current volatile
localSessionIdsoverlay can lose visibility after restart or a flat-list replacement, and it has no growth bound for never-acknowledged rows. Please either persist these rows through the native owning boundary or introduce a durable, bounded local-roster registry with explicit deletion/eviction semantics.
- Add regression coverage for the actual lifecycle boundaries:
- collaboration import visibility after an authoritative roster load;
- JSON import and collaboration fork visibility across restart/rehydration and
loadSessionsreplacement; - stale in-flight page protection;
- overlay/registry bounds and cleanup on deletion.
The current rendered test covers only a warm, same-process native sdeagent-* creation and does not exercise these paths.
- Split commit
339f491d6(fix(ui): tolerate missing ResizeObserver) into a separate PR. It is unrelated to the session-roster invariant and violates this repository single-responsibility PR contract.
CI is green, but these owning-boundary and lifecycle gaps should be resolved before merge.
Collaborator
Author
|
Addressed the requested changes:
Please re-review when convenient. |
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
A newly created session could be present in the entity cache and open tab while disappearing from the Sidebar after an authoritative roster page or flat-list replacement. Creation paths wrote
sessionsAtom, but Sidebar membership is owned separately by paginated roster state.The first implementation covered warm-process native launches but left three lifecycle gaps: collaboration replay IDs cannot be classified from their
external_historyshape, JSON imports have no backend roster owner, and collaboration forks can be replaced before their backend row is observed. The volatile overlay also did not survive restart and had no bound for never-acknowledged rows.Solution
Keep
registerCreatedSessionas the single client creation boundary and add a durable client-created session registry with explicit ownership:nativeentries cover backend-created launches and collaboration forks. They survive restart, flat-list replacement, and stale page responses, then are removed only when an actual backend roster/list response confirms the ID.localentries cover JSON snapshots and collaboration replay imports. They retain their Session metadata and explicitstandalone_agentroster projection until deletion or bounded eviction.All known launch/import/fork/E2E creation paths use the boundary. The unrelated ResizeObserver commit is no longer present in the PR diff because the equivalent fix already exists on current
develop.Potential risks
The registry uses localStorage. If storage is unavailable or quota-limited, the current-process roster still updates, but cross-restart recovery falls back to the native backend, the ordinary persisted session cache, or re-importing/reopening the local snapshot.
The registry intentionally caps retained client-created rows at 200 and evicts the least recently registered entries first. Eviction removes only roster metadata; it does not delete transcript/event data. A native session is recoverable from the backend, a collaboration replay can be pulled again, and a JSON snapshot can be re-imported.
This adds the versioned
orgii:clientCreatedSessions:v1local persistence format and migratesorgii:guestShareImports:v1. It does not change a database schema, IPC/API payload, cloud protocol, timer, subscription, worker, or polling cadence. Rollback is a code revert; if rollback occurs after migration, guest/local rows may need to be reopened or re-imported because the older build does not read the new key. No transcript recovery or database migration is required.Audit
nativeversuslocalsemantics, default ownership, domain boundaries, and creation-call-site clarity. Layer 8 inspected the versioned serialized localStorage shape and legacy migration; no external wire payload changed. Layer 9 covers standard launch, service launch, ADE, collaboration import/fork, JSON import, and E2E seed entry points plus cold-start/flat/page hydration. Layer 10 was reviewed and is not applicable because no multi-field resolver or fallback chain changed.registerCreatedSessioncall sites and all roster acknowledgement paths were inspected. Client-only creation paths now pass explicit ownership/projection; backend-owned paths retain the native default.Performance verdict: pass for the applicable non-background invariants. A real Tauri CPU/RSS run was not performed because this change adds no active/idle/hidden background resource or Rust runtime path.
Verification
npx vitest run src/store/session/sessionAtom/createdSessionRegistry.test.ts src/store/session/sessionAtom/guestImportRegistry.test.ts src/store/session/sessionAtom/__tests__/paginationAtoms.test.ts src/store/session/sessionAtom/__tests__/sidebarRoster.test.ts src/store/session/sessionAtom/__tests__/sidebarLoaders.test.ts src/store/session/sessionAtom/__tests__/mutations.test.ts src/store/session/sessionAtom/__tests__/persistence.test.ts src/store/session/__tests__/sessionManagerAtom.test.ts src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/__tests__/createdSessionVisibility.test.ts src/engines/SessionCore/services/SessionService.create.test.ts src/engines/SessionCore/hooks/session/__tests__/launchPayload.test.ts src/scaffold/GlobalSpotlight/palettes/AgentControlPalette/utils.test.ts src/features/TeamCollaboration/forkSession.test.ts src/features/TeamCollaboration/engine/collabSyncEngineHelpers.test.ts— 14 files, 211 tests passed.npx tsc --noEmit --pretty false— passed.npx eslint <18 changed TypeScript files>— passed.git diff --check origin/develop...HEAD— passed.