Skip to content

fix(sessions): keep new sessions visible in sidebar - #725

Open
beruro wants to merge 5 commits into
developfrom
junyu/fix-new-session-sidebar-roster
Open

fix(sessions): keep new sessions visible in sidebar#725
beruro wants to merge 5 commits into
developfrom
junyu/fix-new-session-sidebar-roster

Conversation

@beruro

@beruro beruro commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

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_history shape, 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 registerCreatedSession as the single client creation boundary and add a durable client-created session registry with explicit ownership:

  • native entries 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.
  • local entries cover JSON snapshots and collaboration replay imports. They retain their Session metadata and explicit standalone_agent roster projection until deletion or bounded eviction.
  • the registry is zod-validated, stored under a versioned localStorage key, capped at 200 entries, and migrates the previous guest-share registry;
  • synchronous startup hydration restores both entity records and roster projections, while persisted-cache hydration is not mistaken for backend acknowledgement;
  • deletion removes entity state, roster membership, registry state, and existing per-session runtime caches through the existing mutation chokepoint;
  • native first-page, load-more, recent-refresh, and flat-list paths reconcile the same ownership model.

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:v1 local persistence format and migrates orgii: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

  • Architecture: covered all 10 layers. Layers 1-7 cover compilation, one registry owner, naming, explicit native versus local semantics, 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.
  • Systematic sweep: all registerCreatedSession call sites and all roster acknowledgement paths were inspected. Client-only creation paths now pass explicit ownership/projection; backend-owned paths retain the native default.
  • React Effects: none added or modified.
  • UI evidence: styling and layout are unchanged. The rendered Sidebar integration test confirms a registered creation produces a real row before roster refresh; screenshots would not add visual evidence.
Area Verdict Evidence Change or reason kept Verification
Background work keep no new timer, poll, subscription, worker, or IPC call registry work is synchronous and demand-driven at creation/load/delete boundaries lifecycle unit/integration tests
Memory/storage fix prior overlay was unbounded and volatile 200-entry validated registry with deterministic eviction and deletion cleanup bound, corruption, migration, and deletion tests
Scope/isolation keep records remain in the existing app data-home/localStorage boundary and retain org/source metadata no cross-instance global or module-level growing cache added restart and replacement tests
Rendering/hot path keep matcher reads the already-hydrated pagination projection no localStorage read was added to render matching rendered Sidebar regression

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.
  • Rust checks and real Tauri runtime measurement were not run because no Rust, database, IPC, network, timer, subscription, or worker path changed.

beruro and others added 2 commits August 6, 2026 22:29

@Harry19081 Harry19081 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes before merge.

  1. Fix roster ownership for non-native creations.
  • Collaboration imports are created with category: "external_history" and an imported-session-... ID. sidebarCategoryForSession cannot assign that shape to a roster category, so registerCreatedSessionWithNativeRoster returns 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 localSessionIds overlay 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.
  1. 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 loadSessions replacement;
  • 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.

  1. 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.

@beruro

beruro commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the requested changes:

  1. Collaboration imports now carry an explicit durable standalone_agent roster projection. JSON imports are locally owned; collaboration forks are native-pending. The shared registry survives restart/flat replacement, is capped at 200, and is cleared on deletion or real native acknowledgement.
  2. Added coverage for collaboration import after authoritative load, JSON/fork restart and replacement, a page response that started before creation, cache hydration versus backend acknowledgement, bounds, migration, corruption, and deletion. The focused suite is 211/211 green; full TypeScript and changed-file lint pass.
  3. The ResizeObserver change is absent from the current diff; the equivalent fix already exists on current develop.

Please re-review when convenient.

@beruro
beruro requested a review from Harry19081 August 8, 2026 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants