Skip to content

feat: Persist and restore active session id (resume in last open sess… #714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pedramamini merged 1 commit into from
Apr 4, 2026
Merged

feat: Persist and restore active session id (resume in last open sess… #714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/__tests__/main/ipc/handlers/persistence.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ describe('persistence IPC handlers', () => {
'settings:set',
'settings:getAll',
'sessions:getAll',
'sessions:getActiveSessionId',
'sessions:setActiveSessionId',
'sessions:setAll',
'groups:getAll',
'groups:setAll',
Expand All @@ -154,6 +156,24 @@ describe('persistence IPC handlers', () => {
});
});

describe('sessions:getActiveSessionId', () => {
it('should return empty string when no active session is set', async () => {
mockSessionsStore.get.mockReturnValue('');
const handler = handlers.get('sessions:getActiveSessionId');
const result = await handler!({} as any);
expect(mockSessionsStore.get).toHaveBeenCalledWith('activeSessionId', '');
expect(result).toBe('');
});
});

describe('sessions:setActiveSessionId', () => {
it('should persist and retrieve an active session ID', async () => {
const setHandler = handlers.get('sessions:setActiveSessionId');
await setHandler!({} as any, 'test-session-123');
expect(mockSessionsStore.set).toHaveBeenCalledWith('activeSessionId', 'test-session-123');
});
});

describe('settings:get', () => {
it('should retrieve setting by key', async () => {
mockSettingsStore.get.mockReturnValue('dark');
Expand Down
25 changes: 24 additions & 1 deletion src/__tests__/renderer/hooks/useSessionRestoration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,11 @@ beforeEach(() => {
if (!(window as any).maestro) {
(window as any).maestro = {};
}
(window as any).maestro.sessions = { getAll: mockGetAll };
(window as any).maestro.sessions = {
getAll: mockGetAll,
getActiveSessionId: vi.fn().mockResolvedValue(''),
setActiveSessionId: vi.fn(),
};
(window as any).maestro.groups = { getAll: mockGroupsGetAll };
(window as any).maestro.groupChat = { list: mockGroupChatList };
(window as any).maestro.agents = {
Expand Down Expand Up @@ -980,6 +984,25 @@ describe('Session & Group loading effect', () => {
expect(useSessionStore.getState().activeSessionId).toBe('real-1');
});

it('restores persisted activeSessionId from disk', async () => {
useSessionStore.setState({ activeSessionId: '' } as any);
const session1 = createMockSession({ id: 'sess-1' });
const session2 = createMockSession({ id: 'sess-2' });
mockGetAll.mockResolvedValueOnce([session1, session2]);
// Mock the persisted active session ID to be the second session
(window as any).maestro.sessions.getActiveSessionId = vi.fn().mockResolvedValue('sess-2');

renderHook(() => useSessionRestoration());

await act(async () => {
await new Promise((r) => setTimeout(r, 50));
});

expect(useSessionStore.getState().activeSessionId).toBe('sess-2');
// Reset mock
(window as any).maestro.sessions.getActiveSessionId = vi.fn().mockResolvedValue('');
});

it('keeps activeSessionId when it matches a loaded session', async () => {
useSessionStore.setState({ activeSessionId: 'loaded-1' } as any);
const session = createMockSession({ id: 'loaded-1' });
Expand Down
2 changes: 2 additions & 0 deletions src/__tests__/setup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ const mockMaestro = {
get: vi.fn().mockResolvedValue([]),
save: vi.fn().mockResolvedValue(undefined),
setAll: vi.fn().mockResolvedValue(undefined),
getActiveSessionId: vi.fn().mockResolvedValue(''),
setActiveSessionId: vi.fn().mockResolvedValue(undefined),
},
groups: {
get: vi.fn().mockResolvedValue([]),
Expand Down
8 changes: 8 additions & 0 deletions src/main/ipc/handlers/persistence.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ export function registerPersistenceHandlers(deps: PersistenceHandlerDependencies
return sessions;
});

ipcMain.handle('sessions:getActiveSessionId', async () => {
return sessionsStore.get('activeSessionId', '');
});

ipcMain.handle('sessions:setActiveSessionId', async (_, id: string) => {
sessionsStore.set('activeSessionId', id);
});

ipcMain.handle('sessions:setAll', async (_, sessions: StoredSession[]) => {
// Get previous sessions to detect changes
const previousSessions = sessionsStore.get('sessions', []);
Expand Down
2 changes: 2 additions & 0 deletions src/main/preload/settings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export function createSessionsApi() {
return {
getAll: () => ipcRenderer.invoke('sessions:getAll'),
setAll: (sessions: StoredSession[]) => ipcRenderer.invoke('sessions:setAll', sessions),
getActiveSessionId: () => ipcRenderer.invoke('sessions:getActiveSessionId') as Promise<string>,
setActiveSessionId: (id: string) => ipcRenderer.invoke('sessions:setActiveSessionId', id),
};
}

Expand Down
1 change: 1 addition & 0 deletions src/main/stores/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ export interface MaestroSettings {

export interface SessionsData {
sessions: StoredSession[];
activeSessionId?: string;
}

// ============================================================================
Expand Down
2 changes: 2 additions & 0 deletions src/renderer/global.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,8 @@ interface MaestroAPI {
sessions: {
getAll: () => Promise<any[]>;
setAll: (sessions: any[]) => Promise<boolean>;
getActiveSessionId: () => Promise<string>;
setActiveSessionId: (id: string) => Promise<void>;
};
groups: {
getAll: () => Promise<any[]>;
Expand Down
20 changes: 10 additions & 10 deletions src/renderer/hooks/session/useSessionRestoration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,8 @@ export function useSessionRestoration(): SessionRestorationReturn {
// useCallback/useEffect without appearing in dependency arrays. Zustand
// store actions returned by getState() are stable singletons that never
// change, so the empty deps array is intentional.
const { setSessions, setGroups, setActiveSessionId, setSessionsLoaded } = useMemo(
() => useSessionStore.getState(),
[]
);
const { setSessions, setGroups, setActiveSessionId, hydrateActiveSessionId, setSessionsLoaded } =
useMemo(() => useSessionStore.getState(), []);
const { setGroupChats } = useMemo(() => useGroupChatStore.getState(), []);

// --- initialLoadComplete proxy ref ---
Expand Down Expand Up @@ -430,12 +428,14 @@ export function useSessionRestoration(): SessionRestorationReturn {
const restoredSessions = await Promise.all(savedSessions.map((s) => restoreSession(s)));
setSessions(restoredSessions);

// Set active session to first session if current activeSessionId is invalid
const activeSessionId = useSessionStore.getState().activeSessionId;
if (
restoredSessions.length > 0 &&
!restoredSessions.find((s) => s.id === activeSessionId)
) {
// Restore persisted active session ID, falling back to first session.
const savedActiveSessionId = await window.maestro.sessions.getActiveSessionId();
if (savedActiveSessionId && restoredSessions.find((s) => s.id === savedActiveSessionId)) {
// Saved ID is valid — hydrate locally without writing back to disk
hydrateActiveSessionId(savedActiveSessionId);
} else if (restoredSessions[0]?.id) {
// Saved ID is stale or missing — persist the fallback so it
// doesn't retry the invalid ID on next launch
setActiveSessionId(restoredSessions[0].id);
}
Comment on lines +432 to 440
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Apr 3, 2026

Choose a reason for hiding this comment

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

P2 setActiveSessionId persists during initial load, potentially overwriting a valid saved ID

When the saved activeSessionId isn't found in the restored sessions (e.g., that session was deleted), the code falls back to restoredSessions[0]?.id. setActiveSessionId is then called, which immediately fires window.maestro?.sessions?.setActiveSessionId(id) and overwrites the persisted ID with the fallback.

This is correct behavior for the deletion case, but it also means that if sessions haven't fully migrated yet (unlikely but possible), the fallback ID is permanently stored before the next app run can attempt a re-match. A clarifying comment here would help future maintainers understand this intentional overwrite.


Expand Down
17 changes: 16 additions & 1 deletion src/renderer/stores/sessionStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ export interface SessionStoreActions {
*/
setActiveSessionId: (id: string) => void;

/**
* Set the active session ID from persisted state on startup.
* Updates local state only — does not write back to disk.
*/
hydrateActiveSessionId: (id: string) => void;

/**
* Set the active session ID without resetting cycle position.
* Used internally by session cycling (Cmd+J/K).
Expand Down Expand Up @@ -198,7 +204,16 @@ export const useSessionStore = create<SessionStore>()((set) => ({
}),

// Active session
setActiveSessionId: (id) => set({ activeSessionId: id, cyclePosition: -1 }),
setActiveSessionId: (id) => {
set({ activeSessionId: id, cyclePosition: -1 });
// Fire-and-forget: persist to disk for restore on next launch.
// Not awaited — UI state must update synchronously; if the write
// fails the only consequence is the session won't be pre-selected
// on next launch (falls back to first session).
window.maestro?.sessions?.setActiveSessionId(id);
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 3, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C3 'getActiveSessionId|setActiveSessionId|ipcRenderer\.invoke|ipcMain\.handle' \
  src/renderer/global.d.ts \
  src/main/preload/settings.ts \
  src/main/ipc/handlers/persistence.ts

Repository: RunMaestro/Maestro

Length of output: 7348

🏁 Script executed:

cat -n src/renderer/stores/sessionStore.ts | sed -n '195,220p'

Repository: RunMaestro/Maestro

Length of output: 876

Add explicit await or error handling to these IPC calls, especially at line 209.

window.maestro.sessions.setActiveSessionId() returns Promise<void> (via ipcRenderer.invoke), but both lines 203 and 209 drop the promise without handling it. This creates unhandled rejection risk if persistence fails.

Line 209 is more problematic: the IPC call sits inside a Zustand state setter callback (set((s) => { ... })), which should be synchronous. Any async operation here will not block state updates and will cause the renderer to reflect the new activeSessionId before the main process confirms persistence. Move the IPC call outside the setter and await it before calling setActiveSessionIdInternal, or use a separate effect hook to handle persistence.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/stores/sessionStore.ts` at line 203, The IPC call
window.maestro.sessions.setActiveSessionId returns a Promise and is being
invoked without await or error handling (both where it's called directly and
inside the Zustand setter), so move the async persistence outside the
synchronous set((s) => { ... }) callback: call and await
window.maestro.sessions.setActiveSessionId(id) (or handle errors with try/catch)
before calling setActiveSessionIdInternal (or, alternatively, call
setActiveSessionIdInternal only after the await succeeds), and ensure any
invocation in other places also either awaits the promise or catches/reports
errors to avoid unhandled rejections.

},

hydrateActiveSessionId: (id) => set({ activeSessionId: id, cyclePosition: -1 }),

setActiveSessionIdInternal: (v) =>
set((s) => ({ activeSessionId: resolve(v, s.activeSessionId) })),
Expand Down