diff --git a/docs/plans/2026-09-27-undo-close-live-children.md b/docs/plans/2026-09-27-undo-close-live-children.md new file mode 100644 index 000000000..3e63a9c71 --- /dev/null +++ b/docs/plans/2026-09-27-undo-close-live-children.md @@ -0,0 +1,21 @@ +# Undo Close: a restored parent's live children follow it (#1373) + +## Problem +Undo Close restores a closed pane under a fresh session id. Its LIVE children (orchestration children and linked panes that stayed open) keep `orchestrationParentId` / `orchestrationRootId` / `linkedParentId` pointing at the dead id. Restore only remaps pointers inside the restored entry (`remapMetaLineage` on the carried rows) and on the undo stack (`remapSingleEntryLineage`). Replace and Reload Agents, by contrast, remap every session (`remapSessionsRelationships`). + +The renderer's orchestration visibility gate compares ids for equality (`orchestrationMcp.ts`), so the restored parent cannot list, read or prompt its own surviving children. They also render as top-level rows. + +## Evidence +- Found by review c of #1369 (`temp/review-1369/report-c.md`, Suspicions). +- `undoClose.ts`: the single-pane commit writes only `sessions[newSessionId]`; the project commit remaps only the carried rows. + +## Decisions (defaults) +- **At both restore commits, map every live session's pointers through the restore's old→new ids with `remapMetaLineage`.** + - WHY not `remapSessionsRelationships`: that one drops a pointer whose target is not in the record. A child of ANOTHER still-closed parent would lose its link, and a later undo of that parent could no longer relink it. `remapMetaLineage` keeps unmapped ids by design ("lineage describes one restore"). + - Rows that no pointer touches keep their object identity, because `remapMetaLineage` returns the same meta. +- Main's orchestration tombstones already follow Undo Close through `carryOrchestrationParents` (#1369). This is the live half. It lands independently: no shared code beyond the file. + +## Tests (fail-first) +- Undo Close of a single pane with a live orchestration child and a live linked child: both now point at the restored id. +- Undo Close of a project whose member has a live child in ANOTHER project: that child follows too. +- A child of a different, still-closed parent keeps its pointer (not dropped). diff --git a/src/renderer/src/workspace/hook/actions/undoClose.ts b/src/renderer/src/workspace/hook/actions/undoClose.ts index a6987a51c..4392e7013 100644 --- a/src/renderer/src/workspace/hook/actions/undoClose.ts +++ b/src/renderer/src/workspace/hook/actions/undoClose.ts @@ -77,6 +77,35 @@ type PublishLineage = (lineage: UndoLineage) => void * has not landed in this snapshot; production `spawn` writes SessionMeta into * workspace state itself, so in the app it is normally present. */ + +/** + * Point every live session's cross-session pointers (orchestration parent and + * root, linked parent) at the restored ids (#1373). + * + * WHY every session and not only the restored rows: a pane's LIVE children + * (orchestration children, linked panes left open) are not part of its undo + * entry, so remapping only the carried rows left them naming the dead id. The + * renderer's orchestration visibility gate compares ids, so the restored + * parent could not list, read or prompt its own children, and they rendered + * as top-level rows. Replace and Reload Agents already remap every session; + * this is the same rule on the undo path. + * + * WHY remapMetaLineage and not remapSessionsRelationships: the latter DROPS a + * pointer whose target is not in the record. A child of a different pane that + * is still closed (still on the undo stack) would lose its link, and undoing + * that pane later could no longer relink it. remapMetaLineage keeps any id the + * map does not name, and returns the same object when nothing changed, so + * untouched rows keep their identity. + */ +function remapLiveLineage( + sessions: Record, + idMap: ReadonlyMap, +): Record { + const out: Record = {} + for (const [sessionId, meta] of Object.entries(sessions)) out[sessionId] = remapMetaLineage(meta, idMap) + return out +} + export function carryDurableMeta(spawned: SessionMeta | undefined, closed: SessionMeta): SessionMeta { // Extension panes skip spawn entirely: all of their metadata is durable UI // identity, including the view ID. The process-specific allowlist below is @@ -261,7 +290,8 @@ export function useUndoCloseAction( // files a session has always done. activeTabId: projectId, sessions: { - ...prev.sessions, + // Live children of the closed pane follow it to its new id (#1373). + ...remapLiveLineage(prev.sessions, new Map([[entry.sessionId, newSessionId]])), // carryDurableMeta restores membership too — `projectId` and, // critically, `joinedAt`, so the row returns to its old position // in the index rather than jumping to the bottom. @@ -388,7 +418,9 @@ export function useUndoCloseAction( const insertIdx = Math.min(entry.tabIndex, prev.tabs.length) const tabs = [...prev.tabs] tabs.splice(insertIdx, 0, restoredTab) - const sessions = { ...prev.sessions } + // Live children of the restored members, wherever they live, follow + // them to their new ids (#1373). + const sessions = remapLiveLineage(prev.sessions, idMap) for (const [newId, closed] of carried) { sessions[newId] = { // Relationship pointers follow the restore: a linked child diff --git a/src/renderer/src/workspace/hook/actions/undoCloseLiveChildren.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/undoCloseLiveChildren.renderer.test.tsx new file mode 100644 index 000000000..8b8fa2bd3 --- /dev/null +++ b/src/renderer/src/workspace/hook/actions/undoCloseLiveChildren.renderer.test.tsx @@ -0,0 +1,81 @@ +import { act, render } from '@testing-library/react' +import { afterEach, expect, it, vi } from 'vitest' + +import { useUndoCloseAction } from '@renderer/workspace/hook/actions/undoClose' +import { makeRefs, sessionActionsWithSpawn, stateWriter } from '@renderer/workspace/hook/actions/testing/paneActionsHarness' +import { freshStage } from '@renderer/workspace/dispatch/gridShape' +import type { SessionMeta, WorkspaceState } from '@renderer/workspace/types' + +// #1373: Undo Close restores a pane under a fresh session id, and its LIVE +// children (orchestration children, linked panes that stayed open) kept +// pointing at the dead id. The orchestration visibility gate compares ids, so +// the restored parent could not list, read or prompt its own children. Replace +// and Reload Agents already remap every session; Undo Close only remapped the +// rows it restored. +const originalApi = window.api +afterEach(() => { window.api = originalApi }) + +const meta = (title: string, extra: Partial = {}): SessionMeta => + ({ cwd: '/projects/agent-code', kind: 'claude', title, projectId: 'tab-live', joinedAt: 1, ...extra }) as SessionMeta + +function mount(entry: unknown, sessions: Record, spawn: ReturnType) { + const state: WorkspaceState = { + tabs: [{ id: 'tab-live', title: 'live' }, { id: 'tab-parent', title: 'agent-code' }], activeTabId: 'tab-live', stage: freshStage(), + sessions, pinnedSessionIds: [], + } + const refs = makeRefs(state) + const writer = stateWriter(state, refs) + refs.undoStackRef.current.push(entry as never) + window.api = { + ...originalApi, carryWorkflowRuns: vi.fn(async () => undefined), carryGoalLoop: vi.fn(async () => null), + controlGoalLoop: vi.fn(async () => null), killOwnedSession: vi.fn(async () => true), + } as never + let actions!: ReturnType + function Harness(): React.JSX.Element { + actions = useUndoCloseAction(state, writer.setState, refs, sessionActionsWithSpawn(spawn), vi.fn()) + return
+ } + const mounted = render() + return { writer, undo: () => actions.undoClose(), unmount: () => mounted.unmount() } +} + +it('points a restored pane\'s live orchestration and linked children at its new id', async () => { + const unrelated = meta('Unrelated') + const harness = mount( + { type: 'session', closedAt: Date.now(), sessionId: 'closed-parent', sessionMeta: meta('Parent', { projectId: 'tab-parent' }) }, + { + worker: meta('Worker', { orchestrationParentId: 'closed-parent', orchestrationRootId: 'closed-parent' }), + linked: meta('Linked', { linkedParentId: 'closed-parent' }), + // A child of ANOTHER closed parent that is still on the stack: a later + // undo of that parent must still find it, so its pointer is kept. + orphan: meta('Orphan', { orchestrationParentId: 'other-closed-parent', orchestrationRootId: 'other-closed-parent' }), + unrelated, + }, + vi.fn().mockResolvedValue('restored-parent'), + ) + await act(async () => { await harness.undo() }) + const sessions = harness.writer.getState().sessions + expect(sessions.worker).toMatchObject({ orchestrationParentId: 'restored-parent', orchestrationRootId: 'restored-parent' }) + expect(sessions.linked).toMatchObject({ linkedParentId: 'restored-parent' }) + expect(sessions.orphan).toMatchObject({ orchestrationParentId: 'other-closed-parent', orchestrationRootId: 'other-closed-parent' }) + // Rows no pointer touches keep their identity (no needless re-render). + expect(sessions.unrelated).toBe(unrelated) + harness.unmount() +}) + +it('points the live children of a restored project\'s members at their new ids', async () => { + const harness = mount( + { + type: 'tab', closedAt: Date.now(), tab: { id: 'closed-tab', title: 'agent-code' }, tabIndex: 1, + sessions: [{ sessionId: 'lead', meta: meta('Lead', { projectId: 'closed-tab' }) }, { sessionId: 'second', meta: meta('Second', { projectId: 'closed-tab' }) }], + }, + { + // Live in ANOTHER project, so the project close did not take it. + worker: meta('Worker', { orchestrationParentId: 'second', orchestrationRootId: 'lead' }), + }, + vi.fn().mockResolvedValueOnce('new-lead').mockResolvedValueOnce('new-second'), + ) + await act(async () => { await harness.undo() }) + expect(harness.writer.getState().sessions.worker).toMatchObject({ orchestrationParentId: 'new-second', orchestrationRootId: 'new-lead' }) + harness.unmount() +})