Skip to content

Commit cbb1005

Browse files
Fix infinite terminal:attach loop caused by unstable callback refs
The Terminal and TaskTerminal components were sending terminal:attach messages in an infinite loop because callback props (onReady, onContainerReady, attachXterm, etc.) were changing identity on every render. When these callbacks changed, React effects would re-run their cleanup (which reset attachment state) and then re-run the effect body (sending another attach message). Fixed by storing callbacks in refs and keeping them updated via separate effects, then using the refs inside effects that shouldn't re-run when callback identity changes. This pattern was already in use for onResize.
1 parent fcfb3ba commit cbb1005

2 files changed

Lines changed: 44 additions & 16 deletions

File tree

src/components/terminal/task-terminal.tsx

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,15 @@ export function TaskTerminal({ taskId, taskName, cwd, className, aiMode, descrip
4343
writeToTerminal,
4444
} = useTerminalWS()
4545

46+
// Store callbacks in refs to avoid effect re-runs when they change
47+
const attachXtermRef = useRef(attachXterm)
48+
const setupImagePasteRef = useRef(setupImagePaste)
49+
const writeToTerminalRef = useRef(writeToTerminal)
50+
51+
useEffect(() => { attachXtermRef.current = attachXterm }, [attachXterm])
52+
useEffect(() => { setupImagePasteRef.current = setupImagePaste }, [setupImagePaste])
53+
useEffect(() => { writeToTerminalRef.current = writeToTerminal }, [writeToTerminal])
54+
4655
// Get the current terminal's status
4756
const currentTerminal = terminalId ? terminals.find((t) => t.id === terminalId) : null
4857
const terminalStatus = currentTerminal?.status
@@ -225,6 +234,7 @@ export function TaskTerminal({ taskId, taskName, cwd, className, aiMode, descrip
225234
}, [terminals, cwd, terminalId])
226235

227236
// Attach xterm to terminal once we have both
237+
// Use refs for callbacks to avoid effect re-runs when callbacks change identity
228238
useEffect(() => {
229239
if (!terminalId || !termRef.current || !containerRef.current || attachedRef.current) return
230240

@@ -234,6 +244,14 @@ export function TaskTerminal({ taskId, taskName, cwd, className, aiMode, descrip
234244
newTerminalIds.delete(terminalId)
235245
}
236246

247+
// Capture current values for use in callbacks
248+
const currentTerminalId = terminalId
249+
const currentStartupScript = startupScript
250+
const currentAiMode = aiMode
251+
const currentDescription = description
252+
const currentTaskName = taskName
253+
const currentTaskId = taskId
254+
237255
// Callback when terminal is fully attached (buffer received from server)
238256
const onAttached = () => {
239257
// Trigger a resize after attaching
@@ -242,10 +260,10 @@ export function TaskTerminal({ taskId, taskName, cwd, className, aiMode, descrip
242260
// Run startup commands only if this is a newly created terminal (not restored from persistence)
243261
if (isNewTerminal) {
244262
// 1. Run startup script first (e.g., mise trust, mkdir .vibora, export VIBORA_DIR)
245-
if (startupScript) {
263+
if (currentStartupScript) {
246264
setTimeout(() => {
247265
// Write the script as-is - newlines act as Enter presses in terminals
248-
writeToTerminal(terminalId, startupScript + '\r')
266+
writeToTerminalRef.current(currentTerminalId, currentStartupScript + '\r')
249267
}, 100)
250268
}
251269

@@ -254,38 +272,38 @@ export function TaskTerminal({ taskId, taskName, cwd, className, aiMode, descrip
254272
'When you finish working and need user input, run: vibora current-task review. ' +
255273
'When linking a PR: vibora current-task pr <url>. ' +
256274
'For notifications: vibora notify "Title" "Message".'
257-
const taskInfo = description ? `${taskName}: ${description}` : taskName
275+
const taskInfo = currentDescription ? `${currentTaskName}: ${currentDescription}` : currentTaskName
258276
const prompt = taskInfo.replace(/"/g, '\\"')
259277
const escapedSystemPrompt = systemPrompt.replace(/"/g, '\\"')
260278

261-
const taskCommand = aiMode === 'plan'
262-
? `claude "${prompt}" --append-system-prompt "${escapedSystemPrompt}" --session-id "${taskId}" --allow-dangerously-skip-permissions --permission-mode plan`
263-
: `claude "${prompt}" --append-system-prompt "${escapedSystemPrompt}" --session-id "${taskId}" --dangerously-skip-permissions`
279+
const taskCommand = currentAiMode === 'plan'
280+
? `claude "${prompt}" --append-system-prompt "${escapedSystemPrompt}" --session-id "${currentTaskId}" --allow-dangerously-skip-permissions --permission-mode plan`
281+
: `claude "${prompt}" --append-system-prompt "${escapedSystemPrompt}" --session-id "${currentTaskId}" --dangerously-skip-permissions`
264282

265283
setTimeout(() => {
266-
writeToTerminal(terminalId, taskCommand + '\r')
267-
}, startupScript ? 300 : 100)
284+
writeToTerminalRef.current(currentTerminalId, taskCommand + '\r')
285+
}, currentStartupScript ? 300 : 100)
268286
}
269287
}
270288

271-
const cleanup = attachXterm(terminalId, termRef.current, { onAttached })
289+
const cleanup = attachXtermRef.current(terminalId, termRef.current, { onAttached })
272290
// Set up image paste handler
273-
const cleanupPaste = setupImagePaste(containerRef.current, terminalId)
291+
const cleanupPaste = setupImagePasteRef.current(containerRef.current, terminalId)
274292
attachedRef.current = true
275293

276294
return () => {
277295
cleanup()
278296
cleanupPaste()
279297
attachedRef.current = false
280298
}
281-
}, [terminalId, attachXterm, setupImagePaste, cwd, doFit, writeToTerminal, aiMode, description, taskName, startupScript, newTerminalIds])
299+
}, [terminalId, doFit, newTerminalIds, startupScript, aiMode, description, taskName, taskId])
282300

283301
// Callback for mobile terminal controls
284302
const handleMobileSend = useCallback((data: string) => {
285303
if (terminalId) {
286-
writeToTerminal(terminalId, data)
304+
writeToTerminalRef.current(terminalId, data)
287305
}
288-
}, [terminalId, writeToTerminal])
306+
}, [terminalId])
289307

290308
if (!cwd) {
291309
return (

src/components/terminal/terminal.tsx

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ export function Terminal({ className, onReady, onResize, onContainerReady, termi
2626
const fitAddonRef = useRef<FitAddon | null>(null)
2727
const onResizeRef = useRef(onResize)
2828
const onFocusRef = useRef(onFocus)
29+
const onReadyRef = useRef(onReady)
30+
const onContainerReadyRef = useRef(onContainerReady)
2931
const { setTerminalFocused } = useKeyboardContext()
3032

3133
// Keep refs updated
@@ -37,6 +39,14 @@ export function Terminal({ className, onReady, onResize, onContainerReady, termi
3739
onFocusRef.current = onFocus
3840
}, [onFocus])
3941

42+
useEffect(() => {
43+
onReadyRef.current = onReady
44+
}, [onReady])
45+
46+
useEffect(() => {
47+
onContainerReadyRef.current = onContainerReady
48+
}, [onContainerReady])
49+
4050
const doFit = useCallback(() => {
4151
if (!fitAddonRef.current || !termRef.current) return
4252

@@ -91,9 +101,9 @@ export function Terminal({ className, onReady, onResize, onContainerReady, termi
91101
// Initial fit after container is sized, with delayed refit to catch layout stabilization
92102
requestAnimationFrame(() => {
93103
doFit()
94-
onReady?.(term)
104+
onReadyRef.current?.(term)
95105
if (containerRef.current) {
96-
onContainerReady?.(containerRef.current)
106+
onContainerReadyRef.current?.(containerRef.current)
97107
}
98108
})
99109

@@ -166,7 +176,7 @@ export function Terminal({ className, onReady, onResize, onContainerReady, termi
166176
termRef.current = null
167177
fitAddonRef.current = null
168178
}
169-
}, [doFit, onReady, onContainerReady, setTerminalFocused])
179+
}, [doFit, setTerminalFocused])
170180

171181
// Set up image paste when terminalId is available
172182
useEffect(() => {

0 commit comments

Comments
 (0)