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
2 changes: 1 addition & 1 deletion docs/plans/2026-09-25-pty-attach-survives-wake.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
The attach count and restore size describe the renderer's VIEW, not the process. Process cleanup leaves them alone, and the view's own `detachAgentPty` (on unmount, including pane close) remains the only release.

## Residuals (not in this PR)
- Plain terminals: `terminalAttached` has no detach call at all and is only cleared by cleanup. Keeping it would leak an id per closed terminal, so the terminal half needs a renderer detach first. Recorded on #1281.
- Plain terminals: `terminalAttached` has no detach call at all and is only cleared by cleanup. Keeping it would leak an id per closed terminal, so the terminal half needs a renderer detach first. Recorded on #1281. Done in `2026-09-26-terminal-attach-survives-respawn.md`.
- A renderer reload or crash never runs the leaf cleanup (#1283 item 3). That predates this change, but it now also survives a process exit.

## Test
Expand Down
25 changes: 25 additions & 0 deletions docs/plans/2026-09-26-terminal-attach-survives-respawn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# A plain terminal view survives a same-id respawn (#1281 residual, #1283 item 3 for terminals)

## Problem
#1281 is two halves. The agent raw-PTY half shipped in `2ab60d45`, and #1311's review (`6cf6f054`) made each renderer page own its agent-PTY attaches, which covers #1283 item 3 for agents. The plain-terminal half is still open, and its plan (`2026-09-25-pty-attach-survives-wake.md`) recorded it as a residual.
- `SessionManager.terminalAttached` is a Set that `cleanupSessionState` deletes when the shell exits.
- `TerminalLeaf` attaches once per sessionId (effect deps `[sessionId]`), after `ensureSessionLive`.
- A same-id respawn under a still-mounted leaf buffers the new shell's bytes but never forwards them: the xterm stays on the dead shell until a remount. The respawn happens through `recover` with `recoverTmuxName`, from a delivery, a control call, or a wake.
- There is no terminal detach at all, so the flag could not be kept past exit without leaking one id per closed terminal.
- `session:terminal-attach` ignores the sender, so a reloaded or crashed renderer could never release a reference either (#1283 item 3 for terminals).

## Evidence
- Source: `sessionManager.ts` (`terminalAttached` add in `attachTerminal`; delete in `cleanupSessionState`; data forwarding gated on `has`), `ipc/session.ts` (`session:terminal-attach` handler), `TerminalLeaf.tsx` (attach in the `[sessionId]` effect, no detach in its cleanup).
- The agent-half fix and its tests (`sessionManager.ptyAttachWake.test.ts`, `ipc/session.test.ts` "raw PTY attach ownership (#1311)") are the recorded model this copies.

## Decisions (defaults)
- **The attach describes the VIEW, not the process.** `terminalAttached` becomes a count, which process cleanup no longer touches. A new `detachTerminal` releases one reference.
- **A missing session still takes a reference.** A leaf can mount while its shell is down, and the respawn then forwards. A kind mismatch (an agent id) takes none, and the IPC layer records only references main took.
- **Renderer ownership:** the IPC ownership table for agent-PTY attaches becomes one generic per-page table, keyed by view type and session. Terminal attach and detach use it with the page document, like agent PTYs. A reload or destroy releases a page's terminal references too; a stale page's late detach is ignored.
- **TerminalLeaf detaches in its effect cleanup** when its attach resolved; a late attach after unmount detaches itself, as `AgentInlineTerminal` does.
- **No terminal resize restore:** plain terminals have none today, so none is added.

## Tests
- **Manager:** a still-attached terminal view keeps forwarding across exit and a same-id respawn, and its detach ends forwarding. Red on main.
- **IPC:** terminal attaches are released on reload and destroy; a stale page's detach is ignored; a kind-mismatch attach takes no reference.
- **Renderer:** TerminalLeaf detaches on unmount, and a late attach after unmount releases itself.
52 changes: 52 additions & 0 deletions src/main/ipc/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,58 @@ describe('raw PTY attach ownership (#1311)', () => {
})
})

// #1281 / #1283 item 3 (terminal half): a plain terminal's view reference now
// outlives the shell too, so it gets the same page ownership as the agent PTY
// above. Before, the manager flag had no release at all, and a reloaded or
// crashed renderer could never give one back.
describe('terminal attach ownership (#1281)', () => {
function register(id: number) {
const detached: string[] = []
const manager = Object.assign(new EventEmitter(), {
attachTerminal: (sessionId: string) => (sessionId === 'agent-pane' ? null : 'replay'),
detachTerminal: (sessionId: string) => { detached.push(sessionId) },
detachAgentPty: vi.fn(),
})
registerSessionIpc(manager as never, {} as never, new SessionFeedTap(manager as never))
const sender = Object.assign(new EventEmitter(), { id })
return {
detached,
sender,
attach: (sessionId: string, document: string) => harness.handlers.get('session:terminal-attach')!({ sender }, sessionId, document),
detach: (sessionId: string, document: string) => harness.handlers.get('session:terminal-detach')!({ sender }, sessionId, document),
announce: (document: string) => harness.handlers.get('session:screen-document')!({ sender }, document),
}
}

it('releases a page\'s terminal views when it reloads or dies, and ignores its late detach', () => {
const page = register(6161)
page.announce('doc-1')
expect(page.attach('shell', 'doc-1')).toBe('replay')
// Not a terminal: main took no reference, so the reload releases none.
expect(page.attach('agent-pane', 'doc-1')).toBe('')
page.announce('doc-2')
expect(page.detached).toEqual(['shell'])
// The dead page's queued detach must not take the new page's reference.
page.attach('shell', 'doc-2')
page.detach('shell', 'doc-1')
expect(page.detached).toEqual(['shell'])
page.sender.emit('destroyed')
expect(page.detached).toEqual(['shell', 'shell'])
})

it('passes a leaf\'s detach through once per attach, and a stale page gets the replay without a reference', () => {
const page = register(6262)
page.announce('doc')
page.attach('shell', 'doc')
page.detach('shell', 'doc')
page.detach('shell', 'doc')
expect(page.detached).toEqual(['shell'])
// A page that already reloaded away takes nothing that could leak.
expect(page.attach('shell', 'old-doc')).toBe('')
page.sender.emit('destroyed')
expect(page.detached).toEqual(['shell'])
})
})

// #1267 (steering q22 at the source): session:spawn relayed the raw provider
// exception over IPC, where every renderer surface had to remember not to
Expand Down
145 changes: 93 additions & 52 deletions src/main/ipc/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,15 +221,6 @@ export function registerSessionIpc(
return manager.getSessionKind(sessionId)
})

// Terminal attach/replay. Called once by TerminalLeaf on mount.
// Returns the full buffered output of the session so far AND flips
// the manager's "attached" flag so subsequent PTY data events
// broadcast live. See SessionManager.terminalBuffers for the race
// being fixed.
ipcMain.handle('session:terminal-attach', (_evt, sessionId: string) => {
return manager.attachTerminal(sessionId)
})

// Snapshot for a renderer that restored after the last change event. The
// monitor emits on change only, so without this a reload would show every
// busy shell idle until its foreground moved again. Filtered to the caller's
Expand All @@ -243,30 +234,57 @@ export function registerSessionIpc(
)
})

// Agent PTY attach/replay. DebugPanel uses this for Claude
// and Codex panes when the user asks to see the raw underlying TUI
// as an xterm terminal. Kept separate from terminal-attach because
// plain terminal panes and agent panes have different primary
// renderers and different live IPC channels.
// Raw views (agent PTY xterms and plain-terminal leaves) are OWNED by the
// calling renderer page (#1311 review for agents, #1283 item 3 for
// terminals), the same lifecycle as the screen leases below. Main's attach
// counts outlive the process (a same-id wake keeps a mounted view live,
// #1281), so a renderer that reloads or crashes without running its leaf
// cleanup would otherwise pin a count forever: bytes forwarded to nobody,
// and for agents a restore size applied to some later process. Each
// renderer's outstanding attaches are released when it is destroyed or
// loads a new document.
//
// Attaches are OWNED by the calling renderer page (#1311 review), the same
// lifecycle as the screen leases below. The attach count now outlives the
// provider process (a same-id wake keeps a mounted view live), so a
// renderer that reloads or crashes without running its leaf cleanup would
// otherwise pin a count forever: raw PTY bytes forwarded to nobody, and a
// restore size applied to some later process. Each renderer's outstanding
// attaches are released when it is destroyed or loads a new document.
const agentPtyAttaches = new Map<number, Map<string, number>>()
// Each renderer's current page document, so only a real reload releases.
const agentPtyDocuments = new Map<number, string>()
const releaseAgentPtyAttaches = (owner: number): void => {
const owned = agentPtyAttaches.get(owner)
if (!owned) return
agentPtyAttaches.delete(owner)
for (const [sessionId, count] of owned) {
for (let i = 0; i < count; i += 1) manager.detachAgentPty(sessionId)
// WHY one table per view type from one factory, rather than the agent-only
// table this used to be plus a copy for terminals: the terminal half of
// #1281 needs exactly the same take / give-back-only-what-you-hold /
// release-on-reload rules, and two hand-written copies are how one of them
// drifts. `detach` is the manager call one reference is worth.
const rawViewOwnership = (detach: (sessionId: string) => void) => {
const byOwner = new Map<number, Map<string, number>>()
return {
take(owner: number, sessionId: string): void {
const owned = byOwner.get(owner) ?? new Map<string, number>()
owned.set(sessionId, (owned.get(sessionId) ?? 0) + 1)
byOwner.set(owner, owned)
},
/** Give back one reference this owner holds; one it does not hold is ignored. */
give(owner: number, sessionId: string): void {
const owned = byOwner.get(owner)
const count = owned?.get(sessionId) ?? 0
if (count === 0) return
if (count === 1) owned!.delete(sessionId)
else owned!.set(sessionId, count - 1)
if (owned!.size === 0) byOwner.delete(owner)
detach(sessionId)
},
releaseOwner(owner: number): void {
const owned = byOwner.get(owner)
if (!owned) return
byOwner.delete(owner)
for (const [sessionId, count] of owned) {
for (let i = 0; i < count; i += 1) detach(sessionId)
}
},
}
}
const agentPtyViews = rawViewOwnership(sessionId => manager.detachAgentPty(sessionId))
const terminalViews = rawViewOwnership(sessionId => manager.detachTerminal(sessionId))
// Each renderer's current page document, so only a real reload releases.
const rawViewDocuments = new Map<number, string>()
const releaseRawViews = (owner: number): void => {
agentPtyViews.releaseOwner(owner)
terminalViews.releaseOwner(owner)
}
// Is this call from the renderer's CURRENT page? #1311 round 2 (review A):
// a reload can keep the webContents id, so the sender alone cannot tell
// the dead page's queued detach from the live page's; without the page
Expand All @@ -275,39 +293,61 @@ export function registerSessionIpc(
// once per load, the same token the screen leases carry). A renderer that
// has not announced yet adopts the caller's document, which its own
// announcement then confirms.
const isCurrentAgentPtyDocument = (owner: number, document: string): boolean => {
const current = agentPtyDocuments.get(owner)
const isCurrentRawViewDocument = (owner: number, document: string): boolean => {
const current = rawViewDocuments.get(owner)
if (current === undefined) {
agentPtyDocuments.set(owner, document)
rawViewDocuments.set(owner, document)
return true
}
return current === document
}

// Agent PTY attach/replay. DebugPanel uses this for Claude
// and Codex panes when the user asks to see the raw underlying TUI
// as an xterm terminal. Kept separate from terminal-attach because
// plain terminal panes and agent panes have different primary
// renderers and different live IPC channels.
ipcMain.handle('session:agent-pty-attach', (evt, sessionId: string, document: string) => {
const owner = watchLeaseOwner(evt.sender)
// A dead page's attach must take no reference: nothing would release it,
// because that page's release already ran.
if (!isCurrentAgentPtyDocument(owner, document)) return null
if (!isCurrentRawViewDocument(owner, document)) return null
const buffer = manager.attachAgentPty(sessionId)
// null = no backend, and main took no reference; nothing to own.
if (buffer === null) return buffer
const owned = agentPtyAttaches.get(owner) ?? new Map<string, number>()
owned.set(sessionId, (owned.get(sessionId) ?? 0) + 1)
agentPtyAttaches.set(owner, owned)
agentPtyViews.take(owner, sessionId)
return buffer
})

ipcMain.handle('session:agent-pty-detach', (evt, sessionId: string, document: string) => {
// Only a reference the CURRENT page holds. After a reload released the
// old page's references, its late detach must not take the new page's.
if (agentPtyDocuments.get(evt.sender.id) !== document) return
const owned = agentPtyAttaches.get(evt.sender.id)
const count = owned?.get(sessionId) ?? 0
if (count === 0) return
if (count === 1) owned!.delete(sessionId)
else owned!.set(sessionId, count - 1)
if (owned!.size === 0) agentPtyAttaches.delete(evt.sender.id)
manager.detachAgentPty(sessionId)
if (rawViewDocuments.get(evt.sender.id) !== document) return
agentPtyViews.give(evt.sender.id, sessionId)
})

// Terminal attach/replay. Called once by TerminalLeaf per session id.
// Returns the full buffered output of the session so far AND takes a view
// reference so subsequent PTY data events broadcast live. See
// SessionManager.terminalBuffers for the race being fixed. Same page
// ownership as the agent PTY above (#1283 item 3): the reference used to be
// a manager flag no renderer could release, cleared only by the shell's
// exit, which is what froze a mounted leaf across a same-id respawn (#1281).
ipcMain.handle('session:terminal-attach', (evt, sessionId: string, document: string) => {
const owner = watchLeaseOwner(evt.sender)
// A dead page gets the replay for nothing: no reference, since nothing
// would release it. '' keeps the renderer contract a plain string.
if (!isCurrentRawViewDocument(owner, document)) return ''
const buffer = manager.attachTerminal(sessionId)
// null = not a terminal: main took no reference, so there is none to own.
if (buffer === null) return ''
terminalViews.take(owner, sessionId)
return buffer
})

ipcMain.handle('session:terminal-detach', (evt, sessionId: string, document: string) => {
if (rawViewDocuments.get(evt.sender.id) !== document) return
terminalViews.give(evt.sender.id, sessionId)
})

// #762. Live `session:screen` frames are forwarded only while a renderer
Expand All @@ -327,8 +367,8 @@ export function registerSessionIpc(
leaseOwnersWatched.add(owner)
sender.once('destroyed', () => {
screenInterest.dropOwner(owner)
releaseAgentPtyAttaches(owner)
agentPtyDocuments.delete(owner)
releaseRawViews(owner)
rawViewDocuments.delete(owner)
leaseOwnersWatched.delete(owner)
})
}
Expand All @@ -339,11 +379,12 @@ export function registerSessionIpc(
ipcMain.handle('session:screen-document', (evt, document: string): void => {
const owner = watchLeaseOwner(evt.sender)
screenInterest.enterDocument(owner, document)
// A NEW document is a page load: the previous page's raw PTY attaches
// died with it. A re-announce of the live document changes nothing.
const previous = agentPtyDocuments.get(owner)
agentPtyDocuments.set(owner, document)
if (previous !== undefined && previous !== document) releaseAgentPtyAttaches(owner)
// A NEW document is a page load: the previous page's raw views (agent
// PTY and terminal attaches) died with it. A re-announce of the live
// document changes nothing.
const previous = rawViewDocuments.get(owner)
rawViewDocuments.set(owner, document)
if (previous !== undefined && previous !== document) releaseRawViews(owner)
})
ipcMain.handle('session:screen-lease', (evt, sessionId: string, document: string): void => {
screenInterest.acquire(watchLeaseOwner(evt.sender), sessionId, document)
Expand Down
Loading
Loading