Skip to content

fix(sessions): live-refresh session list via SSE #346

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

Open
dimakis wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(sessions): live-refresh session list via SSE #346

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
14 changes: 14 additions & 0 deletions frontend/src/hooks/useSessionList.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,19 @@ export function useSessionList(): UseSessionListReturn {
};
document.addEventListener('visibilitychange', onVisible);

// Refetch session list when sessions are created, renamed, or deleted
const unsubChanged = eventBus.on('sessions_changed', () => {
Copy link
Copy Markdown
Owner Author

@dimakis dimakis May 20, 2026

Choose a reason for hiding this comment

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

🔵 unsafe_assumptions: No debounce/coalescing on the sessions_changed handler. If multiple events fire in rapid succession (e.g. auto-rename right after create), each triggers a full /api/sessions fetch. The existing session_activity path coalesces via scheduleBroadcast() (200ms), but sessions_changed fires and is consumed immediately. Consider debouncing the handler or coalescing on the server side. [fixable]

Copy link
Copy Markdown
Owner Author

@dimakis dimakis May 20, 2026

Choose a reason for hiding this comment

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

🔵 unsafe_assumptions: Self-initiated operations (dismissSession, clearAll, handleRename) optimistically update state and then call the server API, which broadcasts sessions_changed back to the same client. This triggers a redundant full refetch that overwrites the already-correct optimistic state. Not harmful, but wasteful — could skip the SSE-triggered refetch for actions the client initiated itself. [fixable]

apiFetch('/api/sessions')
.then((r) => r.json())
.then((data) => {
const { sessions: page, hasMore: more } = parseSessionsResponse(data);
setSessions(page);
setHasMore(more);
nextOffset.current = page.length;
})
.catch(() => {});
});

// Live session dots via SSE — update isActive/isAttached without full refetch
const unsubActivity = eventBus.on('session_activity', (data) => {
const activities = data as SessionActivity[];
Expand All @@ -105,6 +118,7 @@ export function useSessionList(): UseSessionListReturn {

return () => {
document.removeEventListener('visibilitychange', onVisible);
unsubChanged();
unsubActivity();
};
}, []);
Expand Down
5 changes: 5 additions & 0 deletions server/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,8 @@ async function handleSessionCreate(
log.info('headless session started', { sessionId: wtId, prompt: initialPrompt });
}

sseRegistry.broadcast('sessions_changed', {});
Copy link
Copy Markdown
Owner Author

@dimakis dimakis May 20, 2026

Choose a reason for hiding this comment

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

🟡 missing_tests: No tests verify that sseRegistry.broadcast('sessions_changed', {}) is called after session create, delete, delete-all, or rename. The existing route tests in server/__tests__/routes.test.ts (lines 402-431) test these endpoints but don't mock or assert on sseRegistry. Similarly, the new setSessionsChangedCallback wiring in chat.ts (line 1003) and index.ts (line 275) has no test coverage. [fixable]


res.status(initialPrompt ? 201 : 200).json({
sessionId: wtId,
worktrees,
Expand Down Expand Up @@ -1100,6 +1102,7 @@ app.get('/api/sessions/:id/messages', async (req, res) => {

app.delete('/api/sessions/:id', (req, res) => {
hideSession(req.params.id as string);
sseRegistry.broadcast('sessions_changed', {});
res.json({ ok: true });
});

Expand Down Expand Up @@ -1136,6 +1139,7 @@ app.get('/api/sessions/:id/events', (req, res) => {

app.delete('/api/sessions', (_req, res) => {
hideAllSessions();
sseRegistry.broadcast('sessions_changed', {});
res.json({ ok: true });
});

Expand All @@ -1147,6 +1151,7 @@ app.put('/api/sessions/:id/rename', async (req, res) => {
}
try {
await renameSessionById(req.params.id, title.slice(0, 200));
sseRegistry.broadcast('sessions_changed', {});
res.json({ ok: true });
} catch {
res.status(404).json({ error: 'Session not found' });
Expand Down
6 changes: 6 additions & 0 deletions server/chat.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ let _onSessionChange: SessionChangeCallback | null = null;
export function setSessionChangeCallback(cb: SessionChangeCallback): void {
_onSessionChange = cb;
}

let _onSessionsChanged: (() => void) | null = null;
Copy link
Copy Markdown
Owner Author

@dimakis dimakis May 20, 2026

Choose a reason for hiding this comment

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

🔵 style: The new _onSessionsChanged declaration (lines 64–67) sits between module-level code and an import statement on line 68 (import { EventStore }). This matches the existing pattern for _onSessionChange above it, but imports-after-code is unusual and would be flagged by import/first linting rules. Consider moving the callback declarations below all imports if the file is ever restructured. [fixable]

export function setSessionsChangedCallback(cb: () => void): void {
_onSessionsChanged = cb;
}
import { EventStore } from './event-store.js';
import { capturePromptComparison } from './prompt-compare.js';
import { shouldAutoRename, extractRecentPrompts, generateSessionName } from './auto-rename.js';
Expand Down Expand Up @@ -995,6 +1000,7 @@ async function tryAutoRename(sessionId: string, clientId: string): Promise<void>

// Persist to EventStore first — survives SDK rename failures.
eventStore.upsertSession({ sessionId, summary: newName });
_onSessionsChanged?.();

// Update the SDK session name (best-effort, fire-and-forget)
renameSessionById(sessionId, newName, false).catch((err: unknown) => {
Expand Down
5 changes: 5 additions & 0 deletions server/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
getRepoConfig,
setConnectionRegistry,
setSessionChangeCallback,
setSessionsChangedCallback,
reconcileSessionsBackground,
} from './chat.js';
import { cleanupStaleWorktrees, countWorktrees } from './worktree.js';
Expand Down Expand Up @@ -271,6 +272,10 @@ setSessionChangeCallback((clientId, event, sessionId) => {
overviewEmitter.scheduleBroadcast();
});

setSessionsChangedCallback(() => {
sseRegistry.broadcast('sessions_changed', {});
});

server.on('upgrade', async (req, socket, head) => {
const url = new URL(req.url || '', `http://${req.headers.host}`);

Expand Down
Loading