Skip to content

Commit c348fc3

Browse files
Merge branch 'prevent-erroneous-task-terminal-deletion-pmo7'
# Conflicts: # frontend/routes/terminals/index.tsx
2 parents 63608bc + 4d9d0d6 commit c348fc3

2 files changed

Lines changed: 23 additions & 71 deletions

File tree

frontend/routes/terminals/index.tsx

Lines changed: 0 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import { useTerminalStore } from '@/stores'
1919
import type { ITerminal, ITab } from '@/stores'
2020
import { useTasks } from '@/hooks/use-tasks'
2121
import { useRepositories } from '@/hooks/use-repositories'
22-
import { useWorktreeBasePath } from '@/hooks/use-config'
2322
import { useTerminalViewState } from '@/hooks/use-terminal-view-state'
2423
import { useHotkeys } from '@/hooks/use-hotkeys'
2524
import { cn } from '@/lib/utils'
@@ -134,7 +133,6 @@ const TerminalsView = observer(function TerminalsView() {
134133

135134
const { data: tasks = [], status: tasksStatus } = useTasks()
136135
const { data: repositories = [] } = useRepositories()
137-
const { data: worktreeBasePath } = useWorktreeBasePath()
138136
const [repoFilter, setRepoFilter] = useState<string | null>(null)
139137

140138
// Map repository path to repository id for linking
@@ -223,75 +221,6 @@ const TerminalsView = observer(function TerminalsView() {
223221
// Track the number of tabs when we initiated a tab creation to detect new tabs
224222
const tabCountBeforeCreateRef = useRef<number | null>(null)
225223

226-
// Destroy orphaned worktree terminals (terminals in worktrees dir but no matching task)
227-
// This cleanup is intentionally conservative to avoid accidental destruction:
228-
// 1. Only runs when tasks are loaded AND we have at least one task (empty tasks = likely error)
229-
// 2. Only affects terminals without a tabId (task terminals, not regular tab terminals)
230-
// 3. Only affects terminals in the worktrees directory
231-
// 4. Skips terminals created within the last 30 seconds (grace period for React Query sync)
232-
const TERMINAL_GRACE_PERIOD_MS = 30000 // 30 seconds grace period for new terminals
233-
234-
useEffect(() => {
235-
log.terminalsView.debug('Orphan cleanup effect running', {
236-
worktreeBasePath,
237-
tasksStatus,
238-
taskCount: tasks.length,
239-
terminalCount: terminals.length,
240-
allTaskWorktreesSize: allTaskWorktrees.size,
241-
})
242-
243-
// Guard 1: Need worktreeBasePath to know which terminals are in worktrees dir
244-
if (!worktreeBasePath) {
245-
log.terminalsView.debug('Orphan cleanup skipped: no worktreeBasePath')
246-
return
247-
}
248-
249-
// Guard 2: Tasks must be successfully loaded
250-
if (tasksStatus !== 'success') {
251-
log.terminalsView.debug('Orphan cleanup skipped', { reason: `tasksStatus=${tasksStatus}` })
252-
return
253-
}
254-
255-
// Guard 3: If tasks array is empty, this likely indicates an error or the user has no tasks
256-
// In either case, we should NOT destroy terminals - better to be safe
257-
if (tasks.length === 0) {
258-
log.terminalsView.debug('Orphan cleanup skipped: no tasks loaded (likely error or empty state)')
259-
return
260-
}
261-
262-
for (const terminal of terminals) {
263-
// Skip terminals that belong to regular tabs - they're not orphans
264-
if (terminal.tabId) {
265-
continue
266-
}
267-
268-
// Guard 4: Skip terminals created within the grace period
269-
// This prevents destroying terminals before React Query refetches the updated tasks list
270-
const terminalAge = Date.now() - terminal.createdAt
271-
if (terminalAge < TERMINAL_GRACE_PERIOD_MS) {
272-
log.terminalsView.debug('Orphan cleanup skipped: terminal too new', {
273-
terminalId: terminal.id,
274-
name: terminal.name,
275-
ageMs: terminalAge,
276-
gracePeriodMs: TERMINAL_GRACE_PERIOD_MS,
277-
})
278-
continue
279-
}
280-
281-
const isInWorktreesDir = terminal.cwd?.startsWith(worktreeBasePath)
282-
const isKnownTask = terminal.cwd && allTaskWorktrees.has(terminal.cwd)
283-
284-
if (isInWorktreesDir && !isKnownTask) {
285-
log.terminalsView.warn('DESTROYING ORPHAN TERMINAL', {
286-
terminalId: terminal.id,
287-
name: terminal.name,
288-
cwd: terminal.cwd,
289-
})
290-
destroyTerminal(terminal.id)
291-
}
292-
}
293-
}, [terminals, allTaskWorktrees, worktreeBasePath, destroyTerminal, tasksStatus, tasks.length])
294-
295224
// Filter terminals for the active tab and convert to TerminalInfo for component compatibility
296225
const visibleTerminals = useMemo(() => {
297226
if (activeTabId === ALL_TASKS_TAB_ID) {

server/websocket/terminal-ws.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { WSContext, WSEvents } from 'hono/ws'
22
import type { ClientMessage, ServerMessage } from '../types'
33
import { getPTYManager } from '../terminal/pty-instance'
44
import { getTabManager } from '../terminal/tab-manager'
5+
import { getWorktreeBasePath } from '../lib/settings'
56
import { log } from '../lib/logger'
67

78
interface ClientData {
@@ -180,6 +181,28 @@ export const terminalWebSocketHandlers: WSEvents = {
180181
break
181182
}
182183

184+
// Protection: Task terminals (no tabId, in worktrees dir) require force flag
185+
// This prevents accidental deletion from frontend bugs or stale state
186+
const worktreeBasePath = getWorktreeBasePath()
187+
const isTaskTerminal = !terminalInfo?.tabId && terminalInfo?.cwd?.startsWith(worktreeBasePath)
188+
if (isTaskTerminal && !force) {
189+
log.ws.warn('terminal:destroy BLOCKED - task terminal requires force flag', {
190+
terminalId,
191+
cwd: terminalInfo?.cwd,
192+
name: terminalInfo?.name,
193+
clientId: clientData.id,
194+
reason,
195+
})
196+
sendTo(ws, {
197+
type: 'terminal:error',
198+
payload: {
199+
terminalId,
200+
error: 'Task terminals require explicit force flag to destroy',
201+
},
202+
})
203+
break
204+
}
205+
183206
// Audit log: Record all deletions with full context
184207
log.ws.info('terminal:destroy EXECUTING', {
185208
terminalId,

0 commit comments

Comments
 (0)