Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@
"testing:rendering": "npm run testing:rendering:build && electron testing/rendering/out/main/main.js",
"testing:rendering:build": "electron-vite build --config testing/rendering/electron.vite.config.ts",
"testing:rendering:preview": "electron testing/rendering/out/main/main.js",
"test:review-fixes": "npm run test:settings-coerce && npm run test:prompt-templates",
"test:review-fixes": "npm run test:settings-coerce && npm run test:prompt-templates && npm run test:session-ownership",
"test:settings-coerce": "tsx --tsconfig tsconfig.web.json scripts/test-settings-coerce.ts",
"test:prompt-templates": "tsx --tsconfig tsconfig.web.json scripts/test-prompt-templates.ts",
"test:work-context": "tsx --tsconfig tsconfig.web.json scripts/test-work-context.ts",
"test:session-ownership": "tsx --tsconfig tsconfig.web.json scripts/test-session-ownership.ts",
"test:worktree-activity": "tsx --tsconfig tsconfig.node.json scripts/test-worktree-activity-index.ts",
"test:ghost-fallback": "tsx --tsconfig tsconfig.web.json scripts/test-ghost-fallback.ts",
"test:dictation": "tsx --tsconfig tsconfig.node.json scripts/test-dictation-imports.ts",
Expand Down
144 changes: 144 additions & 0 deletions scripts/test-session-ownership.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import assert from 'node:assert/strict'

import {
collectOwnedSessionIds,
collectUnownedSessionIds,
pickOwnedSessions,
pruneSessionOwnership,
} from '../src/renderer/src/workspace/sessionOwnership'
import type {
BuriedPaneRecord,
DetachedSessionRecord,
SessionMeta,
Tab,
} from '../src/renderer/src/workspace/types'

const meta = (cwd: string, kind: SessionMeta['kind'] = 'codex'): SessionMeta => ({
cwd,
kind,
providerSessionId: `${cwd}-provider`,
})

const tab: Tab = {
id: 'tab-a',
title: 'Project A',
focusedSessionId: 'visible-a',
root: {
type: 'split',
direction: 'vertical',
ratio: 0.5,
a: { type: 'leaf', sessionId: 'visible-a' },
b: { type: 'leaf', sessionId: 'visible-b' },
},
}

const detached: DetachedSessionRecord = {
sessionId: 'detached-a',
surface: 'dispatch',
projectTabId: 'tab-a',
projectTabTitle: 'Project A',
projectTabIndex: 0,
detachedAt: 1,
}

const buried: BuriedPaneRecord = {
id: 'buried-a',
sessionId: 'buried-a',
sessionMeta: meta('/buried'),
buriedAt: 2,
sourceTabId: 'tab-a',
sourceTabTitle: 'Project A',
sourceTabIndex: 0,
}

const sessions: Record<string, SessionMeta> = {
'visible-a': meta('/visible-a'),
'visible-b': meta('/visible-b'),
'detached-a': meta('/detached-a'),
'buried-a': meta('/buried-a'),
'stale-a': meta('/stale-a'),
}

{
const owned = collectOwnedSessionIds({
tabs: [tab],
sessions,
detachedSessions: { 'old-key': detached },
buried: [buried],
})

assert.deepEqual(
[...owned].sort(),
['buried-a', 'detached-a', 'visible-a', 'visible-b'],
)
}

{
assert.deepEqual(
collectUnownedSessionIds({
tabs: [tab],
sessions,
detachedSessions: { 'detached-a': detached },
buried: [buried],
}),
['stale-a'],
)
}

{
const owned = new Set(['visible-a', 'buried-a'])
assert.deepEqual(Object.keys(pickOwnedSessions(sessions, owned)).sort(), [
'buried-a',
'visible-a',
])
}

{
const pruned = pruneSessionOwnership({
tabs: [tab],
sessions: {
...sessions,
'missing-detached-meta': meta('/missing-detached-meta'),
},
detachedSessions: {
'stale-key': detached,
'missing-owner': {
...detached,
sessionId: 'owner-without-meta',
},
},
buried: [
buried,
{
...buried,
id: 'buried-without-meta',
sessionId: 'buried-without-meta',
},
],
dispatchMode: {
scope: 'project',
focusedSessionId: 'stale-a',
},
})

// WHY this assertion matters:
//
// The regression was not "visible panes disappear"; it was the opposite:
// invisible metadata survived and became real processes. This fixture keeps
// all legitimate hidden owners (detached + buried), normalizes detached keys
// to their session id, and proves that focus alone does not keep stale-a
// alive.
assert.deepEqual(Object.keys(pruned.sessions).sort(), [
'buried-a',
'detached-a',
'visible-a',
'visible-b',
])
assert.deepEqual(Object.keys(pruned.detachedSessions), ['detached-a'])
assert.deepEqual(pruned.buried.map(entry => entry.sessionId), ['buried-a'])
assert.equal(pruned.dispatchMode?.focusedSessionId, undefined)
assert.deepEqual(pruned.droppedSessionIds.sort(), [
'missing-detached-meta',
'stale-a',
])
}
28 changes: 26 additions & 2 deletions src/renderer/src/workspace/hook/actions/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ import type {
import type { WorkspaceRefs } from '@renderer/workspace/hook/refs'
import { loadInitialHistoryForSession } from '@renderer/workspace/hook/actions/initialHistory'
import { commandTargetSessionIdForState } from '@renderer/workspace/hook/selectors/commandTargetSessionId'
import {
collectOwnedSessionIds,
collectUnownedSessionIds,
pickOwnedSessions,
} from '@renderer/workspace/sessionOwnership'

// -----------------------------------------------------------------------------
// Session lifecycle actions.
Expand Down Expand Up @@ -426,7 +431,23 @@ export function useSessionActions(
const reloadAgentSessions = useCallback(
async (dangerousMode = refs.dangerousAgentsRef.current) => {
const current = refs.stateRef.current
const agentEntries = Object.entries(current.sessions).filter(([, meta]) => {
const ownedIds = collectOwnedSessionIds(current)
const staleIds = collectUnownedSessionIds(current)
if (staleIds.length > 0) {
// WHY reload prunes but does not kill unowned ids directly:
//
// `reloadAgentSessions` is a user-visible policy change ("restart the
// owned agents with the current dangerous-mode setting"), not a
// workspace garbage collector. If an unowned runtime process exists,
// it has already escaped the UI ownership model; respawning it here is
// the bug. Dropping the metadata prevents the stale row from surviving
// the reload/autosave cycle, while process cleanup can stay best-effort
// at the explicit kill/session-manager layer.
// eslint-disable-next-line no-console
console.warn('[workspace] dropping unowned sessions during agent reload:', staleIds)
}
const agentEntries = Object.entries(current.sessions).filter(([id, meta]) => {
if (!ownedIds.has(id)) return false
const kind = meta.kind ?? 'claude'
return kind === 'claude' || kind === 'codex'
})
Expand Down Expand Up @@ -509,7 +530,10 @@ export function useSessionActions(
})

setState(prev => {
const nextSessions = { ...prev.sessions }
const nextSessions = pickOwnedSessions(
prev.sessions,
collectOwnedSessionIds(prev),
)
for (const [oldId] of agentEntries) delete nextSessions[oldId]
for (const [newId, meta] of Object.entries(freshSessions)) {
nextSessions[newId] = meta
Expand Down
110 changes: 66 additions & 44 deletions src/renderer/src/workspace/hook/persistence/rehydrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import type {
import { collectLeaves } from '@renderer/workspace/tile-tree/treeOps'
import { sanitizeTileTabsState } from '@renderer/workspace/layout/helpers'
import type { PersistedWorkspace } from '@renderer/workspace/persistence'
import {
collectOwnedSessionIds,
collectUnownedSessionIds,
} from '@renderer/workspace/sessionOwnership'

import type {
WorkspaceSetRuntimes,
Expand Down Expand Up @@ -64,6 +68,22 @@ export async function rehydrateWorkspace(
})
const idMap = new Map<SessionId, SessionId>()
const freshSessions: Record<SessionId, SessionMeta> = {}
const ownedIds = collectOwnedSessionIds(persisted)
const staleIds = collectUnownedSessionIds(persisted)

if (staleIds.length > 0) {
// WHY log and drop instead of trying to repair by providerSessionId:
//
// Local SessionIds are the workspace ownership keys. providerSessionId is
// provider history identity and can legitimately be duplicated by clone,
// rewind, or failed restore paths. Using it as a repair key risks attaching
// a hidden stale row to the wrong visible pane. The only safe restore set is
// the ids already owned by tab leaves, detached sessions, or buried panes.
// Dropping stale metadata here prevents invisible persisted rows from
// becoming real backend processes/proxies during startup.
// eslint-disable-next-line no-console
console.warn('[workspace] dropping unowned persisted sessions during rehydrate:', staleIds)
}

const remapNode = (n: TileNode): TileNode => {
if (n.type === 'leaf') {
Expand Down Expand Up @@ -290,53 +310,55 @@ export async function rehydrateWorkspace(
// slow respawn must not block the entire tab strip from coming
// back.
await Promise.all(
Object.entries(persisted.sessions).map(async ([oldId, meta]) => {
const restoreSpan = perf.span('workspace.rehydrate.session', {
oldId,
kind: meta.kind ?? 'claude',
hasProviderSessionId: Boolean(meta.providerSessionId),
hasTmuxName: Boolean(meta.tmuxName),
})
try {
const kind: SessionKind = meta.kind ?? 'claude'
// For terminal sessions with a persisted tmuxName, pass it
// as recoverTmuxName so main re-attaches the alive tmux
// session (or falls back to fresh spawn if it died). Agents
// ignore recoverTmuxName at the main side; safe to omit.
const { sessionId: newId, tmuxName: nextTmuxName } = await window.api.spawnSession({
kind,
cwd: meta.cwd,
resumeSessionId: kind !== 'terminal' ? meta.providerSessionId : undefined,
dangerousMode: kind !== 'terminal' ? refs.dangerousAgentsRef.current : undefined,
useProxy: kind !== 'terminal' ? refs.useProxyStreamingRef.current : undefined,
recoverTmuxName: kind === 'terminal' ? meta.tmuxName : undefined,
Object.entries(persisted.sessions)
.filter(([oldId]) => ownedIds.has(oldId))
.map(async ([oldId, meta]) => {
const restoreSpan = perf.span('workspace.rehydrate.session', {
oldId,
kind: meta.kind ?? 'claude',
hasProviderSessionId: Boolean(meta.providerSessionId),
hasTmuxName: Boolean(meta.tmuxName),
})
idMap.set(oldId, newId)
// Carry the full meta forward — kind + providerSessionId +
// tmuxName — so the next save cycle doesn't drop these and
// cause the session to degrade on the NEXT reload.
// tmuxName is replaced with whatever main reported
// (recovered name when alive, fresh name when respawned).
freshSessions[newId] = {
...meta,
...(nextTmuxName ? { tmuxName: nextTmuxName } : {}),
}
commitRehydratedState()
if (kind !== 'terminal' && freshSessions[newId].providerSessionId) {
void loadInitialHistoryForSession({
sessionId: newId,
meta: freshSessions[newId],
refs,
setRuntimes,
try {
const kind: SessionKind = meta.kind ?? 'claude'
// For terminal sessions with a persisted tmuxName, pass it
// as recoverTmuxName so main re-attaches the alive tmux
// session (or falls back to fresh spawn if it died). Agents
// ignore recoverTmuxName at the main side; safe to omit.
const { sessionId: newId, tmuxName: nextTmuxName } = await window.api.spawnSession({
kind,
cwd: meta.cwd,
resumeSessionId: kind !== 'terminal' ? meta.providerSessionId : undefined,
dangerousMode: kind !== 'terminal' ? refs.dangerousAgentsRef.current : undefined,
useProxy: kind !== 'terminal' ? refs.useProxyStreamingRef.current : undefined,
recoverTmuxName: kind === 'terminal' ? meta.tmuxName : undefined,
})
idMap.set(oldId, newId)
// Carry the full meta forward — kind + providerSessionId +
// tmuxName — so the next save cycle doesn't drop these and
// cause the session to degrade on the NEXT reload.
// tmuxName is replaced with whatever main reported
// (recovered name when alive, fresh name when respawned).
freshSessions[newId] = {
...meta,
...(nextTmuxName ? { tmuxName: nextTmuxName } : {}),
}
commitRehydratedState()
if (kind !== 'terminal' && freshSessions[newId].providerSessionId) {
void loadInitialHistoryForSession({
sessionId: newId,
meta: freshSessions[newId],
refs,
setRuntimes,
})
}
restoreSpan.end({ newId })
} catch (err) {
restoreSpan.fail(err)
// eslint-disable-next-line no-console
console.warn(`[workspace] failed to respawn ${meta.cwd}:`, err)
}
restoreSpan.end({ newId })
} catch (err) {
restoreSpan.fail(err)
// eslint-disable-next-line no-console
console.warn(`[workspace] failed to respawn ${meta.cwd}:`, err)
}
}),
}),
)

if (!commitRehydratedState()) {
Expand Down
24 changes: 19 additions & 5 deletions src/renderer/src/workspace/hook/persistence/useAutoSave.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useCallback, useEffect } from 'react'

import type { PersistedWorkspace } from '@renderer/workspace/persistence'
import type { SessionId, WorkspaceState } from '@renderer/workspace/types'
import { pruneSessionOwnership } from '@renderer/workspace/sessionOwnership'

import type { WorkspaceRefs } from '@renderer/workspace/hook/refs'
import * as perf from '@renderer/performance/client'
Expand Down Expand Up @@ -38,10 +39,23 @@ export function useAutoSave(
const flushSave = useCallback(() => {
const saveSpan = perf.span('workspace.autosave.flush')
const s = refs.latestStateRef.current
const pruned = pruneSessionOwnership(s)
if (pruned.droppedSessionIds.length > 0) {
// WHY autosave prunes instead of faithfully serializing runtime state:
//
// Autosave is the durability boundary. If an action accidentally leaves
// an unowned row in `state.sessions`, writing it to workspace.json turns a
// transient invariant violation into a startup respawn on every future
// launch. Pruning here is the last line of defense: owned hidden sessions
// (detached/buried) still persist, but metadata with no owner cannot make
// itself durable.
// eslint-disable-next-line no-console
console.warn('[workspace] dropping unowned sessions during autosave:', pruned.droppedSessionIds)
}
// Collect non-empty drafts so in-progress prompts survive crashes.
const drafts: Record<SessionId, string> = {}
for (const [id, rt] of Object.entries(refs.latestRuntimesRef.current)) {
if (rt.draftInput) drafts[id] = rt.draftInput
if (pruned.sessions[id] && rt.draftInput) drafts[id] = rt.draftInput
}
const persisted: PersistedWorkspace = {
tabs: s.tabs.map(t => ({
Expand All @@ -51,10 +65,10 @@ export function useAutoSave(
root: t.root,
})),
activeTabId: s.activeTabId,
dispatchMode: s.dispatchMode,
sessions: s.sessions,
detachedSessions: s.detachedSessions,
buried: s.buried,
dispatchMode: pruned.dispatchMode,
sessions: pruned.sessions,
detachedSessions: pruned.detachedSessions,
buried: pruned.buried,
tileTabs: refs.latestTileTabsRef.current,
drafts: Object.keys(drafts).length > 0 ? drafts : undefined,
}
Expand Down
Loading