fix: session restore and Recent Sessions resume#77
Conversation
…essions resume (#74) Sessions were only saved during shutdown, but a race condition between bridge.close() and the server's deferred shutdown() meant sessions were often never written to the DB. On macOS, closing the window without quitting made this worse since terminals could exit before the eventual quit triggered a save. Fix by saving sessions on every state change (debounced) and on a 30s periodic interval, so shutdown is just a safety net. Also fix Recent Sessions resume: normalize paths for project lookup so the terminal gets the correct project name (was being filtered out by the visibility hook), and add error handling to prevent silent failures. - Add auto-save to SessionManager (debounced + periodic with dirty flag) - Wire scheduleSave() on session-created, session-exit, SessionStart hook - Add normalizePath() for symlink/trailing-slash resolution - Use normalized comparison in agent history and hook session linking - Extract resolveResumeSessionId() with multi-tier fallback - Extract resolveProjectName() for robust project matching - Add try/catch to all resume handlers
There was a problem hiding this comment.
Pull request overview
Fixes session restore reliability and “Recent Sessions” resume by persisting sessions continuously server-side and improving resume/path matching logic in the renderer.
Changes:
- Add server-side auto-save (debounced + periodic) and stop relying on shutdown-only persistence.
- Normalize/compare paths more robustly for hook linking and agent-history filtering.
- Centralize renderer resume/project-name resolution and add error handling for recent-session resume actions.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/renderer/lib/session-utils.ts | New shared helpers for resolving --resume session IDs and deriving project names from recent sessions. |
| src/renderer/components/SessionRestoredBanner.tsx | Refactors restore flow to use resolveResumeSessionId. |
| src/renderer/components/RecentSessionsPopover.tsx | Uses resolveProjectName and wraps resume in try/catch to avoid silent failures. |
| src/renderer/components/RecentSessionsCard.tsx | Uses resolveProjectName and wraps resume in try/catch to avoid silent failures. |
| src/renderer/components/CommandPalette.tsx | Uses resolveProjectName and wraps resume in try/catch to avoid silent failures. |
| src/renderer/App.tsx | Refactors auto-restore to use resolveResumeSessionId. |
| packages/server/src/session-persistence.ts | Adds debounced + periodic auto-save with dirty flag, plus start/stop hooks. |
| packages/server/src/register-methods.ts | Starts auto-save and schedules saves on key session lifecycle/hook events. |
| packages/server/src/pty-manager.ts | Uses normalizePath for cwd/session-path matching when linking hook sessions. |
| packages/server/src/process-utils.ts | Adds normalizePath() (strip trailing slashes + realpathSync when possible). |
| packages/server/src/index.ts | Stops autosave timer on shutdown and forces a final persistence pass. |
| packages/server/src/agent-history.ts | Uses normalizePath when filtering Claude history by project path. |
Comments suppressed due to low confidence (2)
src/renderer/components/SessionRestoredBanner.tsx:34
handleRestorehas no error handling. IfresolveResumeSessionIdorcreateTerminalthrows for one session, the loop aborts andrestoringmay never be reset (banner stays stuck in "Restoring..."). Wrap the loop in try/finally (and ideally handle per-session failures so other sessions can still restore).
const handleRestore = async (): Promise<void> => {
setRestoring(true)
for (const prev of previousSessions) {
const resumeSessionId = await resolveResumeSessionId(prev)
const session = await window.api.createTerminal({
agentType: prev.agentType,
projectName: prev.projectName,
projectPath: prev.projectPath,
branch: prev.isWorktree ? prev.branch : undefined,
useWorktree: prev.isWorktree || undefined,
remoteHostId: prev.remoteHostId,
resumeSessionId
})
addTerminal(session)
}
setSessionBanner(false)
window.api.clearPreviousSessions()
setRestoring(false)
src/renderer/App.tsx:205
- Auto-restore runs inside a single outer try/catch, but failures inside the
for (const s of prev)loop will abort restoring remaining sessions and skipclearPreviousSessions(). Consider catching per-session errors (continue restoring others) and using a finally block to ensure state cleanup.
// Auto-restore sessions — prefer hook-correlated session ID (exact),
// fall back to scanning agent history when hooks weren't active.
for (const s of prev) {
const resumeSessionId = await resolveResumeSessionId(s)
const session = await window.api.createTerminal({
agentType: s.agentType,
projectName: s.projectName,
projectPath: s.projectPath,
branch: s.isWorktree ? s.branch : undefined,
useWorktree: s.isWorktree || undefined,
remoteHostId: s.remoteHostId,
resumeSessionId
})
useAppStore.getState().addTerminal(session)
}
window.api.clearPreviousSessions()
} else {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/renderer/lib/session-utils.ts
Outdated
| // Single unscoped fetch — the scoped call is a strict subset, so one | ||
| // round-trip covers all matching tiers. | ||
| const allRecent = await window.api.getRecentSessions() | ||
|
|
||
| // Exact project path match | ||
| const exact = allRecent.find( | ||
| (r) => r.agentType === s.agentType && r.projectPath === s.projectPath | ||
| ) | ||
| if (exact) return exact.sessionId | ||
|
|
There was a problem hiding this comment.
resolveResumeSessionId always calls window.api.getRecentSessions() without a project filter. On the server this is limited/sorted globally (default limit=20), so a session for s.projectPath may not be included even though getRecentSessions(s.projectPath) would return it. That can cause restores to miss --resume and start new sessions. Consider first querying with s.projectPath (strict match), and only falling back to an unscoped query (or add a higher limit) when needed; also consider caching the fetched list when restoring multiple sessions.
| // Single unscoped fetch — the scoped call is a strict subset, so one | |
| // round-trip covers all matching tiers. | |
| const allRecent = await window.api.getRecentSessions() | |
| // Exact project path match | |
| const exact = allRecent.find( | |
| (r) => r.agentType === s.agentType && r.projectPath === s.projectPath | |
| ) | |
| if (exact) return exact.sessionId | |
| // First, try a scoped fetch for an exact projectPath match. This avoids | |
| // missing sessions due to the global limit on unscoped recent sessions. | |
| try { | |
| const scopedRecent = await window.api.getRecentSessions(s.projectPath) | |
| const scopedExact = scopedRecent.find( | |
| (r) => r.agentType === s.agentType && r.projectPath === s.projectPath | |
| ) | |
| if (scopedExact) return scopedExact.sessionId | |
| } catch { | |
| // If scoped fetch fails for any reason, fall back to unscoped lookup. | |
| } | |
| // Fallback: unscoped fetch for looser matching strategies. | |
| const allRecent = await window.api.getRecentSessions() |
There was a problem hiding this comment.
Fixed in 443d2bc — now tries scoped fetch first, then falls through to unscoped fallback only when the scoped call finds nothing.
src/renderer/lib/session-utils.ts
Outdated
| if (!projects) return session.projectPath.split('/').pop() || 'untitled' | ||
|
|
||
| const normalized = session.projectPath.replace(/\/+$/, '') | ||
| const project = projects.find((p) => { | ||
| if (p.path === session.projectPath) return true | ||
| return p.path.replace(/\/+$/, '') === normalized | ||
| }) | ||
| return project?.name || session.projectPath.split('/').pop() || 'untitled' |
There was a problem hiding this comment.
resolveProjectName normalizes session.projectPath for matching, but the fallback display name still uses session.projectPath.split('/').pop(). If the path has a trailing slash, this returns an empty string and falls back to 'untitled' (e.g. /foo/bar/ -> untitled). Use the same normalized path for the basename fallback.
| if (!projects) return session.projectPath.split('/').pop() || 'untitled' | |
| const normalized = session.projectPath.replace(/\/+$/, '') | |
| const project = projects.find((p) => { | |
| if (p.path === session.projectPath) return true | |
| return p.path.replace(/\/+$/, '') === normalized | |
| }) | |
| return project?.name || session.projectPath.split('/').pop() || 'untitled' | |
| const normalized = session.projectPath.replace(/\/+$/, '') | |
| if (!projects) return normalized.split('/').pop() || 'untitled' | |
| const project = projects.find((p) => { | |
| if (p.path === session.projectPath) return true | |
| return p.path.replace(/\/+$/, '') === normalized | |
| }) | |
| return project?.name || normalized.split('/').pop() || 'untitled' |
There was a problem hiding this comment.
Fixed in 443d2bc — resolveProjectName now normalizes the path before the fallback basename extraction, so trailing-slash paths produce the correct directory name instead of 'untitled'.
src/renderer/lib/session-utils.ts
Outdated
| const basename = s.projectPath.replace(/\/+$/, '').split('/').pop() | ||
| if (basename) { | ||
| const fuzzy = allRecent.find( | ||
| (r) => | ||
| r.agentType === s.agentType && | ||
| r.projectPath.replace(/\/+$/, '').split('/').pop() === basename | ||
| ) |
There was a problem hiding this comment.
The comment says the basename match "handles ... case variations", but the comparison is case-sensitive and doesn't normalize case. Either adjust the comment or make the basename/path comparison explicitly case-insensitive where appropriate.
| const basename = s.projectPath.replace(/\/+$/, '').split('/').pop() | |
| if (basename) { | |
| const fuzzy = allRecent.find( | |
| (r) => | |
| r.agentType === s.agentType && | |
| r.projectPath.replace(/\/+$/, '').split('/').pop() === basename | |
| ) | |
| const basename = s.projectPath.replace(/\/+$/, '').split('/').pop()?.toLowerCase() | |
| if (basename) { | |
| const fuzzy = allRecent.find((r) => { | |
| if (r.agentType !== s.agentType) return false | |
| const candidateBase = r.projectPath.replace(/\/+$/, '').split('/').pop()?.toLowerCase() | |
| return candidateBase === basename | |
| }) |
There was a problem hiding this comment.
Fixed in 443d2bc — basename comparison is now case-insensitive (.toLowerCase()) and the comment no longer claims case variation handling.
| startAutoSave(getActiveSessions: () => TerminalSession[]): void { | ||
| this.getActiveSessions = getActiveSessions | ||
|
|
||
| this.intervalTimer = setInterval(() => { | ||
| if (this.dirty) this.persistNow() | ||
| }, AUTO_SAVE_INTERVAL_MS) | ||
| } |
There was a problem hiding this comment.
startAutoSave overwrites this.intervalTimer without clearing an existing interval. If startAutoSave is called more than once in a process (e.g. during dev/hot-reload or re-init paths), this will leak timers and cause duplicate writes. Consider calling stopAutoSave() (or clearing intervalTimer) before starting a new interval.
There was a problem hiding this comment.
Fixed in 443d2bc — startAutoSave now calls stopAutoSave() first to clear any existing timers before setting up new ones.
| scheduleSave(): void { | ||
| if (!this.getActiveSessions) return | ||
| this.dirty = true | ||
| if (this.debounceTimer) clearTimeout(this.debounceTimer) | ||
| this.debounceTimer = setTimeout(() => { | ||
| this.debounceTimer = null | ||
| this.persistNow() | ||
| }, DEBOUNCE_MS) | ||
| } | ||
|
|
||
| persistNow(): void { | ||
| if (!this.getActiveSessions) return | ||
| this.dirty = false | ||
| const sessions = this.getActiveSessions() | ||
| this.saveSessions(sessions) | ||
| } |
There was a problem hiding this comment.
persistNow() doesn't clear a pending debounceTimer. If the interval triggers persistNow() while a debounce timer is still scheduled, the debounce callback will fire later and write again (even though dirty is already false). Consider clearing debounceTimer inside persistNow() (or gating the debounce callback on dirty) to avoid redundant DB writes.
There was a problem hiding this comment.
Fixed in 443d2bc — persistNow() now clears the pending debounce timer before writing, preventing a redundant follow-up write.
| // Save sessions on every state change so we don't rely solely on the | ||
| // shutdown path (which has a race with bridge.close and doesn't cover | ||
| // force-quit / crash). |
There was a problem hiding this comment.
This comment says sessions are saved "on every state change", but currently scheduleSave() is only wired for session-created, session-exit, and SessionStart. Either adjust the comment to match the actual triggers or also schedule saves for other session mutations you intend to persist (e.g. status updates).
| // Save sessions on every state change so we don't rely solely on the | |
| // shutdown path (which has a race with bridge.close and doesn't cover | |
| // force-quit / crash). | |
| // Enable automatic persistence for active sessions. Combined with | |
| // explicit saves on key lifecycle events (e.g. session-created, | |
| // session-exit, SessionStart), this reduces reliance on the shutdown | |
| // path (which has a race with bridge.close and doesn't cover force-quit / crash). |
There was a problem hiding this comment.
Fixed in 443d2bc — comment now accurately lists the actual triggers: session-created, session-exit, SessionStart hook.
packages/server/src/agent-history.ts
Outdated
| @@ -64,7 +68,7 @@ const claudeProvider: AgentHistoryProvider = { | |||
| const entry: ClaudeHistoryEntry = JSON.parse(line) | |||
| if (!entry.sessionId || !entry.project) continue | |||
|
|
|||
| if (projectPath && entry.project !== projectPath) continue | |||
| if (normalizedFilter && normalizePath(entry.project) !== normalizedFilter) continue | |||
|
|
|||
There was a problem hiding this comment.
In the Claude history provider, normalizePath(entry.project) runs inside the per-line loop. Since normalizePath calls realpathSync when paths exist, large history files could cause many filesystem syscalls and slow down getRecentSessions. Consider caching normalized results per unique project path (e.g. a Map) or avoiding realpathSync for history entries unless needed.
There was a problem hiding this comment.
Fixed in 443d2bc — added a Map<string, string> cache inside the per-line loop so repeated project paths only hit realpathSync once.
| /** | ||
| * Normalize a filesystem path for reliable comparison. | ||
| * Strips trailing slashes and resolves symlinks when the path exists. | ||
| */ | ||
| export function normalizePath(p: string): string { | ||
| let result = p.replace(/\/+$/, '') | ||
| try { | ||
| result = fs.realpathSync(result) | ||
| } catch { | ||
| // Path doesn't exist — just use the stripped version | ||
| } | ||
| return result | ||
| } |
There was a problem hiding this comment.
normalizePath only strips trailing forward slashes (/). On Windows, paths may contain trailing backslashes (\\), so comparisons can still fail when the path doesn't exist (and realpathSync throws). Consider using path.normalize() and trimming both separators (or conditionally trimming path.sep) to make normalization cross-platform.
There was a problem hiding this comment.
Fixed in 443d2bc — normalizePath now uses path.normalize() first (handles both / and \ separators cross-platform), then strips trailing separator.
| findUnlinkedSessionByCwd(cwd: string, excludeIds: Set<string>): TerminalSession | undefined { | ||
| const normalizedCwd = cwd.replace(/\/+$/, '') | ||
| const normalizedCwd = normalizePath(cwd) | ||
| let best: TerminalSession | undefined | ||
| let bestTime = 0 | ||
|
|
||
| for (const session of this.sessions.values()) { | ||
| if (session.hookSessionId) continue // already linked | ||
| if (excludeIds.has(session.id)) continue | ||
| const sessionPath = (session.worktreePath || session.projectPath).replace(/\/+$/, '') | ||
| const sessionPath = normalizePath(session.worktreePath || session.projectPath) | ||
| if (sessionPath === normalizedCwd && session.createdAt > bestTime) { | ||
| best = session |
There was a problem hiding this comment.
findUnlinkedSessionByCwd now calls normalizePath() (which may realpathSync) for cwd and for every session in the loop. This runs on hook events (HookStatusMapper.tryLink), so it can become expensive with many sessions/events. Consider caching a normalized path on the session object when it’s created/updated, or only realpath-resolving the cwd and doing cheaper normalization for session paths.
There was a problem hiding this comment.
Fixed in 443d2bc — normalized session paths are now cached at creation time. findUnlinkedSessionByCwd uses the cached value for session paths and only calls normalizePath once for cwd.
…tform paths
- resolveResumeSessionId: try scoped fetch first (avoids global limit=20
missing project sessions), then fall through to unscoped fallback
- resolveProjectName: use normalized path for basename fallback so
trailing-slash paths don't produce 'untitled'
- Basename matching is now case-insensitive (comment was inaccurate)
- startAutoSave: call stopAutoSave first to prevent timer leaks on re-init
- persistNow: clear pending debounce timer to avoid redundant DB writes
- normalizePath: use path.normalize for cross-platform separator handling
- agent-history: cache normalizePath results per unique project path in
the per-line loop to avoid repeated realpathSync syscalls
- pty-manager: cache normalized session paths at creation time so
findUnlinkedSessionByCwd avoids per-iteration realpathSync
- Fix comment accuracy ("every state change" → actual trigger list)
- Update tests for 2-call pattern in resolveResumeSessionId
- RecentSessionsPopover: header shows "· ProjectName" or "· All Projects" - RecentSessionsCard: same treatment - SessionRestoredBanner: shows project name when single project, or "N projects" when sessions span multiple projects
In ESM (dev/tsx), __dirname is not a global. The previous fix (#70) removed the import.meta.url fallback because tsup emits it as import_meta.url in the CJS bundle, crashing at runtime. This left __dirname undefined in dev mode. Fix: use typeof guard with process.argv[1] as the ESM fallback. Avoids import.meta.url entirely so the CJS bundle CI check passes.
When restoring N sessions, resolveResumeSessionId used .find() which always returned the most-recent match — all terminals got the same session ID. Now accepts a `claimed` set that callers populate as each ID is assigned, so subsequent lookups skip already-taken sessions.
Summary
Fixes #74
bridge.close()and the server's deferredshutdown()meant sessions were often never written to the DB. On macOS, closing the window without quitting made this worse since terminals could exit before the eventual quit triggered a save.Changes
Core fix — continuous session persistence:
SessionManagernow auto-saves on every state change (debounced 500ms) and on a 30s periodic interval with a dirty flag, so shutdown save is just a safety netscheduleSave()wired onsession-created,session-exit, andSessionStarthook eventsPath normalization:
normalizePath()inprocess-utils.ts— strips trailing slashes + resolves symlinks viarealpathSyncRecent Sessions resume fix:
resolveResumeSessionId()with multi-tier fallback (hookSessionId → history scan → basename match)resolveProjectName()with normalized path matching so terminals get the correct project name for visibility filteringtry/catchto all resume handlers to prevent silent failuresTest plan
--resumeflag visible in terminal/var/...on macOS)