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
5 changes: 4 additions & 1 deletion scripts/summarize-lifecycle.mts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@ async function readLifecycleEvents(runsDir: string, runId: string): Promise<Jour
function describe(event: JournalEvent): string {
const data = event.data ?? {}
const parts: string[] = []
for (const key of ['caller', 'kind', 'gate', 'reason', 'code', 'disposition', 'status', 'cause']) {
// `unresolvedSessionIds` is here because a restore that never completes is
// the failure this summarizer exists to make readable, and the count alone
// ('3 of 4') does not tell you which pane to go look at.
for (const key of ['caller', 'kind', 'gate', 'reason', 'code', 'disposition', 'status', 'cause', 'unresolvedSessionIds']) {
const value = data[key]
if (value !== undefined && value !== null && value !== '') parts.push(`${key}=${String(value)}`)
}
Expand Down
19 changes: 19 additions & 0 deletions src/renderer/src/workspace/hook/persistence/rehydrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -821,10 +821,29 @@ export async function rehydrateWorkspace(
// provider started. A run whose rehydrate.start has no matching complete is a
// restore that never resolved, which pins autosave off and is invisible today
// apart from a console.warn.
//
// WHY the unresolved ids are named and not just counted: a real workspace sat
// at `expectedCount 4, resolvedCount 3, ok false` on every launch for three
// weeks, and the journal could not say WHICH pane was missing. Diagnosing it
// required diffing tile leaves against the `sessions` map by hand. The count
// alone tells you that restore is stuck; the ids tell you why, which is the
// whole point of recording this event.
const unresolved = [...liveProcessIds].filter(id => !resolvedIds.has(id))
reportLifecycle('rehydrate.complete', undefined, {
expectedCount: expectedSessions,
resolvedCount: resolvedIds.size,
ok: resolvedIds.size === expectedSessions,
// Only on a failure: a healthy boot would otherwise record an empty string
// on every launch, and "the field is absent" already means "none".
//
// Capped at 8 because the journal sanitizer truncates strings over 300
// chars, which at ~37 bytes per comma-joined UUID would slice the last id
// in half — a half-id in a diagnostic is worse than a missing one, since
// it reads like a real id. `expectedCount`/`resolvedCount` still carry the
// true totals.
...(unresolved.length > 0
? { unresolvedSessionIds: unresolved.slice(0, 8).join(',') }
: {}),
durationMs: Date.now() - rehydrateStartedAt,
})
return {
Expand Down
41 changes: 37 additions & 4 deletions src/renderer/src/workspace/hook/persistence/useAutoSave.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +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 { pruneSessionOwnership, repairPersistedTabs } from '@renderer/workspace/sessionOwnership'
import { withNormalizedBuiltInMcpDomains } from '@renderer/workspace/mcpDomains'

import type { WorkspaceRefs } from '@renderer/workspace/hook/refs'
Expand Down Expand Up @@ -53,6 +53,39 @@ export function useAutoSave(
// eslint-disable-next-line no-console
console.warn('[workspace] dropping unowned sessions during autosave:', pruned.droppedSessionIds)
}
// Repair the tile trees BEFORE serializing them. `pruneSessionOwnership`
// above scrubs every pointer that aims at a session, but the trees are
// themselves an ownership surface and used to be written verbatim — so a
// leaf whose metadata had already been removed from `state.sessions` could
// become durable. That shape is not merely untidy: rehydrate counts such a
// leaf as a pane it must restore, can never restore it, and therefore
// reports `partial-restore` and holds autosave off on every subsequent
// launch. Since autosave is the only writer of workspace.json, the file
// then cannot be repaired by the app at all.
//
// WHY `pruned.sessions` and not `s.sessions`: they agree on exactly the
// question being asked. `pruned.sessions` keeps `ownedIds ∩ own keys of
// s.sessions`, and every tile leaf with metadata is owned by construction,
// so a leaf is missing here if and only if it was already an orphan in
// this same snapshot. Nothing `pruneSessionOwnership` drops for an
// unrelated reason (unowned metadata, a detached record whose parent tab
// is gone, a buried pane) can ever be a tile leaf — which is what stops
// this from deleting a live pane. If that ever stops holding, this call
// becomes destructive, so keep the two in step.
const repairedTabs = repairPersistedTabs({
tabs: s.tabs,
sessions: pruned.sessions,
activeTabId: s.activeTabId,
tileTabs: refs.latestTileTabsRef.current,
})
if (repairedTabs.droppedLeafSessionIds.length > 0) {
// eslint-disable-next-line no-console
console.warn(
'[workspace] dropping tile leaves with no session metadata during autosave:',
{ leaves: repairedTabs.droppedLeafSessionIds, tabs: repairedTabs.droppedTabIds },
)
}

// 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)) {
Expand All @@ -69,13 +102,13 @@ export function useAutoSave(
id => pruned.sessions[id] !== undefined,
)
const persisted: PersistedWorkspace = {
tabs: s.tabs.map(t => ({
tabs: repairedTabs.tabs.map(t => ({
id: t.id,
title: t.title,
focusedSessionId: t.focusedSessionId,
root: t.root,
})),
activeTabId: s.activeTabId,
activeTabId: repairedTabs.activeTabId,
dispatchMode: pruned.dispatchMode,
// WHY normalize MCP domains at the persistence boundary:
//
Expand All @@ -96,7 +129,7 @@ export function useAutoSave(
pinnedSessionIds: persistedPinnedSessionIds.length > 0
? persistedPinnedSessionIds
: undefined,
tileTabs: refs.latestTileTabsRef.current,
tileTabs: repairedTabs.tileTabs,
drafts: Object.keys(drafts).length > 0 ? drafts : undefined,
}
let json = ''
Expand Down
226 changes: 224 additions & 2 deletions src/renderer/src/workspace/sessionOwnership.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
import { describe, expect, it } from 'vitest'

import { pruneSessionOwnership } from '@renderer/workspace/sessionOwnership'
import type { TileNode, WorkspaceState } from '@renderer/workspace/types'
import {
collectLiveProcessIds,
collectOwnedSessionIds,
pruneSessionOwnership,
repairPersistedTabs,
} from '@renderer/workspace/sessionOwnership'
import type {
SessionId,
SessionMeta,
TileNode,
WorkspaceState,
} from '@renderer/workspace/types'

function leaf(sessionId: string): TileNode {
return { type: 'leaf', sessionId }
Expand Down Expand Up @@ -158,3 +168,215 @@ describe('pruneSessionOwnership', () => {
expect(result.droppedSessionIds).toHaveLength(83)
})
})


// Regression fixture for the "Autosave off" freeze.
//
// Recorded from a real ~/.config/agent-code/workspace.json: tab "agent-code"
// held a vertical split whose `b` leaf pointed at a session id that had no row
// in `sessions`, and the tab's focusedSessionId pointed at that same dead id.
// Every launch afterwards journalled `expectedCount 4, resolvedCount 3, ok
// false` and refused to autosave, which meant the file could never be
// repaired. The exact shape matters, so it is reproduced rather than
// paraphrased.
function makeOrphanLeafState(): WorkspaceState {
const state = makeState()
state.tabs = [
{
id: 'tabA',
title: 'agent-code',
root: {
type: 'split',
direction: 'vertical',
ratio: 0.5,
a: leaf('live'),
b: leaf('orphan'),
},
focusedSessionId: 'orphan',
},
]
return state
}

describe('collectLiveProcessIds', () => {
it('excludes tile leaves that have no session metadata', () => {
// The gate denominator must only count panes that CAN be restored.
// Counting the orphan is what made restore completion unsatisfiable.
expect([...collectLiveProcessIds(makeOrphanLeafState())]).toEqual(['live'])
})

it('still counts every leaf that does have metadata', () => {
const state = makeOrphanLeafState()
state.sessions.orphan = { cwd: '/work/project-a', kind: 'claude' }

expect([...collectLiveProcessIds(state)].sort()).toEqual(['live', 'orphan'])
})

it('restores the gate invariant: every live id is a key of sessions', () => {
// This is the property whose absence froze the workspace — the gate
// compares |resolvedIds| to |liveProcessIds| while resolvedIds can only
// ever contain keys of `sessions`, so the comparison is satisfiable only
// when liveProcessIds is a subset of those keys. Asserted directly rather
// than restating a fixture's expected size, so it holds for any input.
const state = makeOrphanLeafState()
const live = collectLiveProcessIds(state)

expect([...live].every(id =>
Object.prototype.hasOwnProperty.call(state.sessions, id))).toBe(true)
expect(live.has('orphan')).toBe(false)
})

it('does not read metadata through the prototype chain', () => {
// A leaf id like `toString` resolves to an inherited function under a bare
// index read, which would classify a genuine orphan as healthy and
// reproduce the freeze on a hand-edited workspace.json.
const state = makeOrphanLeafState()
state.tabs[0].root = { type: 'leaf', sessionId: 'toString' }
state.tabs[0].focusedSessionId = 'toString'

expect([...collectLiveProcessIds(state)]).toEqual([])
})
})

describe('collectOwnedSessionIds', () => {
it('keeps tile leaves owned independently of the live-process set', () => {
// Ownership and "needs a process" are different questions. A pane kind
// that deliberately spawns nothing (extension views) narrows the live set;
// if ownership were derived from that narrowed set, autosave would drop the
// pane's SessionMeta and manufacture the very orphan leaf this module
// repairs. Pinning them as separate sources keeps that impossible.
const state = makeOrphanLeafState()
state.sessions.orphan = { cwd: '/work/project-a', kind: 'claude' }

expect([...collectOwnedSessionIds(state)].sort()).toEqual(['live', 'orphan'])
})
})

describe('repairPersistedTabs', () => {
function repair(
state: WorkspaceState,
sessions: Record<SessionId, SessionMeta> = { live: state.sessions.live },
) {
return repairPersistedTabs({
tabs: state.tabs,
sessions,
activeTabId: state.activeTabId,
tileTabs: null,
})
}

it('collapses an orphaned split into its survivor and repoints tab focus', () => {
const result = repair(makeOrphanLeafState())

expect(result.droppedLeafSessionIds).toEqual(['orphan'])
expect(result.droppedTabIds).toEqual([])
expect(result.tabs).toHaveLength(1)
// The split is gone entirely — the survivor is promoted to root, exactly
// as a normal pane close would have left it.
expect(result.tabs[0].root).toEqual({ type: 'leaf', sessionId: 'live' })
expect(result.tabs[0].focusedSessionId).toBe('live')
expect(result.tabs[0].title).toBe('agent-code')
})

it('repoints focus that names a session outside this tab', () => {
// The invariant a tab owes is "focus names a leaf I contain". Testing
// against the sessions map instead would leave this dangling, and rehydrate
// does not repair it either because the id resolves fine.
const state = makeOrphanLeafState()
state.tabs[0].focusedSessionId = 'elsewhere'

const result = repair(state, { live: state.sessions.live, elsewhere: state.sessions.live })

expect(result.tabs[0].focusedSessionId).toBe('live')
})

it('leaves healthy trees untouched', () => {
const state = makeOrphanLeafState()
state.sessions.orphan = { cwd: '/work/project-a', kind: 'claude' }

const result = repair(state, state.sessions)

expect(result.droppedLeafSessionIds).toEqual([])
// Identity, not just equality: a healthy save must not churn the tree.
expect(result.tabs[0]).toBe(state.tabs[0])
expect(result.activeTabId).toBe(state.activeTabId)
})

it('reports one id when the same orphan occupies several leaves', () => {
const state = makeOrphanLeafState()
state.tabs[0].root = {
type: 'split',
direction: 'vertical',
ratio: 0.5,
a: leaf('orphan'),
b: { type: 'split', direction: 'horizontal', ratio: 0.5, a: leaf('orphan'), b: leaf('live') },
}

const result = repair(state)

expect(result.droppedLeafSessionIds).toEqual(['orphan'])
expect(result.tabs[0].root).toEqual({ type: 'leaf', sessionId: 'live' })
})

it('drops a tab whose every leaf is orphaned and repoints activeTabId', () => {
// The one destructive branch in the whole change.
const state = makeOrphanLeafState()
state.tabs = [
{ id: 'tabA', title: 'agent-code', root: leaf('orphan'), focusedSessionId: 'orphan' },
{ id: 'tabB', title: 'other', root: leaf('live'), focusedSessionId: 'live' },
]
state.activeTabId = 'tabA'

const result = repair(state)

expect(result.droppedTabIds).toEqual(['tabA'])
expect(result.tabs.map(t => t.id)).toEqual(['tabB'])
expect(result.activeTabId).toBe('tabB')
})

it('drops a dead tab out of tileTabs rather than persisting a dangling id', () => {
const state = makeOrphanLeafState()
state.tabs = [
{ id: 'tabA', title: 'agent-code', root: leaf('orphan'), focusedSessionId: 'orphan' },
{ id: 'tabB', title: 'b', root: leaf('live'), focusedSessionId: 'live' },
{ id: 'tabC', title: 'c', root: leaf('live'), focusedSessionId: 'live' },
]

const result = repairPersistedTabs({
tabs: state.tabs,
sessions: { live: state.sessions.live },
activeTabId: 'tabB',
tileTabs: {
tabIds: ['tabA', 'tabB', 'tabC'],
focusedTabId: 'tabA',
direction: 'vertical',
ratios: [0.34, 0.33, 0.33],
},
})

expect(result.tileTabs?.tabIds).toEqual(['tabB', 'tabC'])
// Focus pointed at the dropped tab, and ratios must match the new count.
expect(result.tileTabs?.focusedTabId).toBe('tabB')
expect(result.tileTabs?.ratios).toHaveLength(2)
})

it('keeps every tab when nothing was dropped', () => {
const state = makeOrphanLeafState()
const tileTabs = {
tabIds: ['tabA', 'tabB'],
focusedTabId: 'tabA',
direction: 'vertical' as const,
ratios: [0.5, 0.5],
}

const result = repairPersistedTabs({
tabs: state.tabs,
sessions: { live: state.sessions.live },
activeTabId: state.activeTabId,
tileTabs,
})

// Only a dropped TAB can invalidate tileTabs; a collapsed split cannot.
expect(result.tileTabs).toBe(tileTabs)
})
})
Loading