Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
f67c8fb
docs: add implementation plan for terminal-attach-leak
codex Jul 25, 2026
d66192c
docs(plan): harden terminal-attach-leak plan with load-bearing valida…
codex Jul 25, 2026
b5b7707
docs(plan): fix two blocking fresheyes findings in terminal-attach-le…
codex Jul 25, 2026
ba34636
docs(plan): fix four blocking fresheyes findings in terminal-attach-l…
codex Jul 25, 2026
f1e27d0
feat(client): add collectAllTerminalIds layout helper
codex Jul 25, 2026
2e2795d
feat(client): add terminal release-marks module with global test reset
codex Jul 25, 2026
0fe5acf
feat(client): add sendTerminalKill helper that marks terminals released
codex Jul 25, 2026
43ddcfd
feat(client): add terminalDetachMiddleware releasing dropped terminal…
codex Jul 25, 2026
5e66320
feat(client): register terminalDetachMiddleware in production store
codex Jul 25, 2026
cf6efd1
fix(client): route TabBar close detach through terminalDetachMiddleware
codex Jul 25, 2026
bb505f3
fix(client): route pane-close detach through terminalDetachMiddleware
codex Jul 25, 2026
4654d06
fix(client): route context-menu detach through terminalDetachMiddlewa…
codex Jul 25, 2026
ba83b1f
refactor(client): route remaining terminal.kill sends through sendTer…
codex Jul 25, 2026
1da8b0a
test(e2e): assert replace-pane detach from the middleware instead of …
codex Jul 25, 2026
9b18328
fix(client): gate terminal attach on layout membership to close creat…
codex Jul 25, 2026
378204e
test(client): align remaining suites with middleware-based terminal d…
codex Jul 25, 2026
2f43ec6
docs(client): fix stale comments flagged by final review
codex Jul 25, 2026
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
1,416 changes: 1,416 additions & 0 deletions docs/plans/2026-07-25-terminal-attach-leak.md

Large diffs are not rendered by default.

7 changes: 3 additions & 4 deletions src/components/BackgroundSessions.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useCallback, useEffect, useMemo } from 'react'
import { useCallback, useEffect } from 'react'
import { nanoid } from 'nanoid'
import { getWsClient } from '@/lib/ws-client'
import { sendTerminalKill } from '@/lib/terminal-kill'
import { Button } from '@/components/ui/button'
import { Badge } from '@/components/ui/badge'
import { useAppDispatch, useAppSelector } from '@/store/hooks'
Expand All @@ -22,7 +22,6 @@ function formatAge(ms: number): string {

export default function BackgroundSessions() {
const dispatch = useAppDispatch()
const ws = useMemo(() => getWsClient(), [])
const terminals = useAppSelector((state) => (
(state as any).terminalDirectory?.windows?.background?.items ?? EMPTY_TERMINALS
)) as BackgroundTerminal[]
Expand Down Expand Up @@ -117,7 +116,7 @@ export default function BackgroundSessions() {
<Button
size="sm"
variant="destructive"
onClick={() => ws.send({ type: 'terminal.kill', terminalId: t.terminalId })}
onClick={() => sendTerminalKill(t.terminalId)}
>
Kill
</Button>
Expand Down
15 changes: 4 additions & 11 deletions src/components/TabBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { cn } from '@/lib/utils'
import { useAppDispatch, useAppSelector } from '@/store/hooks'
import { addTab, closeTab, setActiveTab, reorderTabs, clearTabRenameRequest } from '@/store/tabsSlice'
import { dismissTabGreen } from '@/store/turnCompletionAttention'
import { getWsClient } from '@/lib/ws-client'
import { getTabDisplayTitle } from '@/lib/tab-title'
import { sendTerminalKill } from '@/lib/terminal-kill'
import { collectPaneEntries, collectTerminalIds } from '@/lib/pane-utils'
import { getBusyPaneIdsForTab } from '@/lib/pane-activity'
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
Expand Down Expand Up @@ -179,8 +179,6 @@ export default function TabBar({ sidebarCollapsed, onToggleSidebar }: TabBarProp
const multirowTabs = useAppSelector((s) => s.settings?.settings?.panes?.multirowTabs ?? false)
const extensions = useAppSelector((s) => s.extensions?.entries)

const ws = useMemo(() => getWsClient(), [])

// Compute display title for a single tab
// Priority: user-set title > programmatically-set title (e.g., from Claude) > derived name
const getDisplayTitle = useCallback(
Expand Down Expand Up @@ -310,14 +308,10 @@ export default function TabBar({ sidebarCollapsed, onToggleSidebar }: TabBarProp
}
}}
onClose={(e) => {
const terminalIds = getTerminalIdsForTab(tab)
if (terminalIds.length > 0) {
const messageType = e.shiftKey ? 'terminal.kill' : 'terminal.detach'
if (e.shiftKey) {
const terminalIds = getTerminalIdsForTab(tab)
for (const terminalId of terminalIds) {
ws.send({
type: messageType,
terminalId,
})
sendTerminalKill(terminalId)
}
}
dispatch(closeTab(tab.id))
Expand Down Expand Up @@ -350,7 +344,6 @@ export default function TabBar({ sidebarCollapsed, onToggleSidebar }: TabBarProp
renameValue,
renamingId,
tabAttentionStyle,
ws,
])

useEffect(() => {
Expand Down
13 changes: 11 additions & 2 deletions src/components/TerminalView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { focusNextTerminalSearchMatch, focusPreviousTerminalSearchMatch, loadTer
import { isFatalConnectionErrorCode } from '@/store/connectionSlice'
import { flushPersistedLayoutNow } from '@/store/persistControl'
import { getWsClient } from '@/lib/ws-client'
import { sendTerminalKill } from '@/lib/terminal-kill'
import { getTerminalTheme } from '@/lib/terminal-themes'
import {
buildCodexIdentityMismatchRepairContent,
Expand Down Expand Up @@ -65,7 +66,7 @@ import {
type DeferredAttachReason,
type TerminalAttachPriority,
} from '@/lib/terminal-attach-policy'
import { paneRefreshTargetMatchesContent } from '@/lib/pane-utils'
import { collectAllTerminalIds, paneRefreshTargetMatchesContent } from '@/lib/pane-utils'
import { getInstalledPerfAuditBridge } from '@/lib/perf-audit-bridge'
import {
beginAttach,
Expand Down Expand Up @@ -2433,6 +2434,13 @@ function TerminalView({ tabId, paneId, paneContent, hidden }: TerminalViewProps)
opts?: AttachTerminalOptions,
) => {
if (suppressNetworkEffects) return
// Never attach a terminal the layouts no longer reference: the layout-diff
// middleware can only release subscriptions it saw acquired. Covers the
// close-during-create race and stale deferred re-attach timers.
const layouts = appStore.getState().panes.layouts
if (!collectAllTerminalIds(layouts).has(tid)) {
return
}
const term = termRef.current
if (!term) return
const runtime = runtimeRef.current
Expand Down Expand Up @@ -2580,6 +2588,7 @@ function TerminalView({ tabId, paneId, paneContent, hidden }: TerminalViewProps)
}, [
suppressNetworkEffects,
ws,
appStore,
applySeqState,
buildCheckpointReplayInput,
clearQuarantineRepair,
Expand Down Expand Up @@ -2907,7 +2916,7 @@ function TerminalView({ tabId, paneId, paneContent, hidden }: TerminalViewProps)
lastSentViewportRef.current = null
applySeqState(createAttachSeqState())
writeLocalXtermNotice(term, '\r\n[Restarting OpenCode session because the saved terminal replay is no longer available]\r\n')
ws.send({ type: 'terminal.kill', terminalId })
sendTerminalKill(terminalId)
return true
}

Expand Down
23 changes: 4 additions & 19 deletions src/components/context-menu/ContextMenuProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
} from '@/store/panesSlice'
import { setProjectExpanded } from '@/store/sessionsSlice'
import { getWsClient } from '@/lib/ws-client'
import { sendTerminalKill } from '@/lib/terminal-kill'
import { api, setSessionMetadata } from '@/lib/api'
import { refreshActiveSessionWindow } from '@/store/sessionsThunks'
import { getAuthToken } from '@/lib/auth'
Expand Down Expand Up @@ -295,24 +296,12 @@ export function ContextMenuProvider({
}, [dispatch])

const replacePaneAction = useCallback((tabId: string, paneId: string) => {
if (!panes[tabId]) return
const content = findPaneContent(panes[tabId], paneId)
if (content?.kind === 'terminal' && content.terminalId) {
ws.send({ type: 'terminal.detach', terminalId: content.terminalId })
}
dispatch(replacePane({ tabId, paneId }))
}, [dispatch, panes, ws])
}, [dispatch])

const closeTabById = useCallback((tabId: string) => {
const layout = panes[tabId]
if (layout) {
const terminalIds = collectTerminalIds(layout)
for (const terminalId of terminalIds) {
ws.send({ type: 'terminal.detach', terminalId })
}
}
dispatch(closeTab(tabId))
}, [dispatch, panes, ws])
}, [dispatch])

const reopenClosedTabAction = useCallback(() => {
dispatch(reopenClosedTab())
Expand Down Expand Up @@ -927,7 +916,7 @@ export function ContextMenuProvider({
)

if (latest.content.kind === 'terminal' && latest.content.terminalId) {
ws.send({ type: 'terminal.kill', terminalId: latest.content.terminalId })
sendTerminalKill(latest.content.terminalId)
} else if (latest.content.kind === 'fresh-agent' && latest.content.sessionId) {
const cwd = getFreshOpenCodeRouteCwd(
latest.content,
Expand Down Expand Up @@ -1221,10 +1210,6 @@ export function ContextMenuProvider({
resetSplit: (tabId, splitId) => dispatch(resetSplit({ tabId, splitId })),
swapSplit: (tabId, splitId) => dispatch(swapSplit({ tabId, splitId })),
closePane: (tabId, paneId) => {
const content = panes[tabId] ? findPaneContent(panes[tabId], paneId) : null
if (content?.kind === 'terminal' && content.terminalId) {
ws.send({ type: 'terminal.detach', terminalId: content.terminalId })
}
dispatch(closePaneWithCleanup({ tabId, paneId }))
},
getTerminalActions: getTerminalActions,
Expand Down
9 changes: 2 additions & 7 deletions src/components/panes/PaneContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -335,13 +335,8 @@ export default function PaneContainer({ tabId, node, hidden }: PaneContainerProp
}, [])

const handleClose = useCallback((paneId: string, content: PaneContent) => {
// Clean up terminal process if this pane has one
if (content.kind === 'terminal' && content.terminalId) {
ws.send({
type: 'terminal.detach',
terminalId: content.terminalId,
})
}
// Terminal detach is handled by terminalDetachMiddleware, which reconciles
// dropped terminal references on the resulting layout change.
if (content.kind === 'fresh-agent') {
clearDraft(paneId)
const pendingCreate = freshAgentPendingCreates[content.createRequestId]
Expand Down
18 changes: 18 additions & 0 deletions src/lib/pane-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,24 @@ export function collectTerminalIds(node: PaneNode): string[] {
]
}

/**
* Union of every terminalId referenced by any pane in any tab layout.
* This is the client's complete "terminals I currently reference" set —
* the primitive the detach middleware diffs to spot dropped references.
*/
export function collectAllTerminalIds(
layouts: Record<string, PaneNode | undefined>
): Set<string> {
const ids = new Set<string>()
for (const layout of Object.values(layouts)) {
if (!layout) continue
for (const terminalId of collectTerminalIds(layout)) {
ids.add(terminalId)
}
}
return ids
}

export function collectPaneContents(node: PaneNode): PaneContent[] {
if (node.type === 'leaf') {
return [node.content]
Expand Down
15 changes: 15 additions & 0 deletions src/lib/terminal-kill.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { getWsClient } from './ws-client'
import { markTerminalReleased } from './terminal-release-marks'

/**
* Send terminal.kill for a terminal, marking it released first so the
* detach middleware does not follow up with a redundant terminal.detach
* when the pane reference disappears from the layouts.
*
* Every production terminal.kill send in the client goes through here.
* (Test harness escape hatch sendWsMessage in App.tsx can bypass it.)
*/
export function sendTerminalKill(terminalId: string): void {
markTerminalReleased(terminalId)
getWsClient().send({ type: 'terminal.kill', terminalId })
}
26 changes: 26 additions & 0 deletions src/lib/terminal-release-marks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* Terminal ids whose server-side subscription has already been (or is about
* to be) released by an explicit send — currently terminal.kill. The detach
* middleware consumes a mark instead of sending a redundant terminal.detach
* for a terminal the server just removed (the server replies with an error
* for detach on a non-existent terminal).
*
* Terminal ids are server-generated and never reused, so a stale mark can
* only ever suppress a detach for a terminal that no longer needs one.
* Marks for terminals never referenced by any layout (e.g. background session
* kills) are intentionally left unconsumed — benign because terminal ids are
* never reused.
*/
const releasedTerminalIds = new Set<string>()

export function markTerminalReleased(terminalId: string): void {
releasedTerminalIds.add(terminalId)
}

export function consumeTerminalReleaseMark(terminalId: string): boolean {
return releasedTerminalIds.delete(terminalId)
}

export function resetTerminalReleaseMarks(): void {
releasedTerminalIds.clear()
}
2 changes: 2 additions & 0 deletions src/store/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { sessionActivityPersistMiddleware } from './sessionActivityPersistence'
import { browserPreferencesPersistenceMiddleware } from './browserPreferencesPersistence'
import { createLogger } from '@/lib/client-logger'
import { layoutMirrorMiddleware } from './layoutMirrorMiddleware'
import { terminalDetachMiddleware } from './terminalDetachMiddleware'
import { serverSettingsSaveStateMiddleware } from './settingsThunks'
import { tabFallbackIdentityMiddleware } from './tabFallbackIdentityMiddleware'
import {
Expand Down Expand Up @@ -73,6 +74,7 @@ export const store = configureStore({
serverSettingsSaveStateMiddleware,
browserPreferencesPersistenceMiddleware,
layoutMirrorMiddleware,
terminalDetachMiddleware,
sessionActivityPersistMiddleware,
),
})
Expand Down
54 changes: 54 additions & 0 deletions src/store/terminalDetachMiddleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import type { Middleware } from '@reduxjs/toolkit'
import { getWsClient } from '@/lib/ws-client'
import { collectAllTerminalIds } from '@/lib/pane-utils'
import { consumeTerminalReleaseMark } from '@/lib/terminal-release-marks'
import { clearDeadTerminals, clearTerminalLiveHandles } from './panesSlice'
import type { PaneNode } from './paneTypes'

type PanesStateSlice = { panes: { layouts: Record<string, PaneNode | undefined> } }

/**
* These actions strip terminalIds ONLY for terminals the server itself
* reported dead or unrecoverable (terminal.inventory / terminals.changed).
* There is no live subscription to release, and the server replies with an
* error for terminal.detach on a non-existent terminal — so skip them.
*/
const skipDetachActionTypes = new Set<string>([
clearDeadTerminals.type,
clearTerminalLiveHandles.type,
])

/**
* Detach reconciler: whenever an action makes a terminalId disappear from
* ALL pane layouts, the client no longer references that terminal and must
* release its server-side attach subscription — otherwise the server sees
* hasClients=true forever and its idle reaper can never collect the
* terminal. The set diff over every layout is what guards the multi-pane
* case: a terminal referenced by two panes only detaches when the LAST
* reference goes away.
*
* Stateless by design (derives everything from getState) — safe under the
* test suite's per-test ws-client reset.
*/
export const terminalDetachMiddleware: Middleware = (store) => (next) => (action) => {
const beforeLayouts = (store.getState() as PanesStateSlice).panes.layouts
const result = next(action)
const afterLayouts = (store.getState() as PanesStateSlice).panes.layouts
if (afterLayouts === beforeLayouts) return result

const actionType = (action as { type?: unknown }).type
if (typeof actionType === 'string' && skipDetachActionTypes.has(actionType)) {
return result
}

const before = collectAllTerminalIds(beforeLayouts)
if (before.size === 0) return result
const after = collectAllTerminalIds(afterLayouts)

for (const terminalId of before) {
if (after.has(terminalId)) continue
if (consumeTerminalReleaseMark(terminalId)) continue
getWsClient().send({ type: 'terminal.detach', terminalId })
}
return result
}
Loading
Loading