optimize: terminal spawn lifecycle to eliminate output race condition
Move PTY spawn from context layer to view layer so event subscription happens before spawn, preventing loss of initial terminal output. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -69,11 +69,14 @@ pub(crate) fn prepare_credential_env(
|
||||
pub async fn terminal_spawn(
|
||||
working_dir: String,
|
||||
initial_command: Option<String>,
|
||||
terminal_id: Option<String>,
|
||||
manager: State<'_, TerminalManager>,
|
||||
app_handle: tauri::AppHandle,
|
||||
window: tauri::WebviewWindow,
|
||||
) -> Result<String, TerminalError> {
|
||||
let terminal_id = uuid::Uuid::new_v4().to_string();
|
||||
let terminal_id = terminal_id
|
||||
.filter(|id| !id.is_empty() && id.len() <= 256)
|
||||
.unwrap_or_else(|| uuid::Uuid::new_v4().to_string());
|
||||
|
||||
let app_data_dir = app_handle
|
||||
.path()
|
||||
|
||||
@@ -162,6 +162,17 @@ impl TerminalManager {
|
||||
opts: SpawnOptions,
|
||||
emitter: EventEmitter,
|
||||
) -> Result<String, TerminalError> {
|
||||
// Reject duplicate IDs to prevent orphaning an existing PTY process.
|
||||
{
|
||||
let terminals = self.terminals.lock().unwrap();
|
||||
if terminals.contains_key(&opts.terminal_id) {
|
||||
return Err(TerminalError::SpawnFailed(format!(
|
||||
"terminal id '{}' already exists",
|
||||
opts.terminal_id
|
||||
)));
|
||||
}
|
||||
}
|
||||
|
||||
let pty_system = native_pty_system();
|
||||
|
||||
let pair = pty_system
|
||||
|
||||
@@ -18,6 +18,7 @@ use crate::terminal::types::TerminalInfo;
|
||||
pub struct TerminalSpawnParams {
|
||||
pub working_dir: String,
|
||||
pub initial_command: Option<String>,
|
||||
pub terminal_id: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
@@ -50,7 +51,10 @@ pub async fn terminal_spawn(
|
||||
Json(params): Json<TerminalSpawnParams>,
|
||||
) -> Result<Json<String>, AppCommandError> {
|
||||
let manager = &state.terminal_manager;
|
||||
let terminal_id = uuid::Uuid::new_v4().to_string();
|
||||
let terminal_id = params
|
||||
.terminal_id
|
||||
.filter(|id| !id.is_empty() && id.len() <= 256)
|
||||
.unwrap_or_else(|| uuid::Uuid::new_v4().to_string());
|
||||
|
||||
let extra_env = prepare_credential_env(&state.data_dir);
|
||||
|
||||
|
||||
@@ -18,6 +18,8 @@ export function TerminalPanel() {
|
||||
<TerminalView
|
||||
key={tab.id}
|
||||
terminalId={tab.id}
|
||||
workingDir={tab.workingDir}
|
||||
initialCommand={tab.initialCommand}
|
||||
isActive={tab.id === activeTabId}
|
||||
isVisible={isOpen}
|
||||
/>
|
||||
|
||||
@@ -1,8 +1,13 @@
|
||||
"use client"
|
||||
|
||||
import { useEffect, useRef } from "react"
|
||||
import { useEffect, useRef, useState } from "react"
|
||||
import { subscribe } from "@/lib/platform"
|
||||
import { terminalWrite, terminalResize } from "@/lib/api"
|
||||
import {
|
||||
terminalSpawn,
|
||||
terminalWrite,
|
||||
terminalResize,
|
||||
terminalKill,
|
||||
} from "@/lib/api"
|
||||
import type { TerminalEvent } from "@/lib/types"
|
||||
import type { ITheme } from "@xterm/xterm"
|
||||
|
||||
@@ -86,12 +91,16 @@ function getTerminalTheme(container: HTMLDivElement | null): ITheme {
|
||||
|
||||
interface TerminalViewProps {
|
||||
terminalId: string
|
||||
workingDir: string
|
||||
initialCommand?: string
|
||||
isActive: boolean
|
||||
isVisible: boolean
|
||||
}
|
||||
|
||||
export function TerminalView({
|
||||
terminalId,
|
||||
workingDir,
|
||||
initialCommand,
|
||||
isActive,
|
||||
isVisible,
|
||||
}: TerminalViewProps) {
|
||||
@@ -101,6 +110,7 @@ export function TerminalView({
|
||||
const lastResizeRef = useRef<{ cols: number; rows: number } | null>(null)
|
||||
const isActiveRef = useRef(isActive)
|
||||
const isVisibleRef = useRef(isVisible)
|
||||
const [loading, setLoading] = useState(true)
|
||||
|
||||
useEffect(() => {
|
||||
isActiveRef.current = isActive
|
||||
@@ -167,7 +177,7 @@ export function TerminalView({
|
||||
}
|
||||
)
|
||||
|
||||
// Set up event listeners BEFORE fit so initial output is captured
|
||||
// Subscribe to events BEFORE spawning so no initial output is lost
|
||||
const unlisten = await subscribe<TerminalEvent>(
|
||||
`terminal://output/${terminalId}`,
|
||||
(payload) => {
|
||||
@@ -192,6 +202,27 @@ export function TerminalView({
|
||||
return
|
||||
}
|
||||
|
||||
// Spawn the terminal AFTER subscribing to events
|
||||
try {
|
||||
await terminalSpawn(workingDir, initialCommand, terminalId)
|
||||
} catch (err) {
|
||||
term.write(`\r\n\x1b[31m[Failed to start terminal: ${err}]\x1b[0m\r\n`)
|
||||
} finally {
|
||||
if (!cancelled) setLoading(false)
|
||||
}
|
||||
|
||||
// If unmounted while spawn was in flight, clean up the spawned PTY
|
||||
if (cancelled) {
|
||||
terminalKill(terminalId).catch(() => {})
|
||||
themeObserver.disconnect()
|
||||
onDataDisposable.dispose()
|
||||
onResizeDisposable.dispose()
|
||||
unlisten()
|
||||
unlistenExit()
|
||||
term.dispose()
|
||||
return
|
||||
}
|
||||
|
||||
const fitIfReady = () => {
|
||||
const el = containerRef.current
|
||||
if (!el) return
|
||||
@@ -237,7 +268,7 @@ export function TerminalView({
|
||||
cancelled = true
|
||||
cleanup?.()
|
||||
}
|
||||
}, [terminalId])
|
||||
}, [terminalId, workingDir, initialCommand])
|
||||
|
||||
// Refit and focus when becoming active or panel becomes visible
|
||||
useEffect(() => {
|
||||
@@ -262,6 +293,32 @@ export function TerminalView({
|
||||
aria-hidden={!isActive}
|
||||
>
|
||||
<div ref={containerRef} className="h-full w-full" />
|
||||
{loading && isActive && (
|
||||
<div className="absolute inset-0 flex items-center justify-center bg-background/80">
|
||||
<div className="flex items-center gap-2 text-sm text-muted-foreground">
|
||||
<svg
|
||||
className="h-4 w-4 animate-spin"
|
||||
viewBox="0 0 24 24"
|
||||
fill="none"
|
||||
>
|
||||
<circle
|
||||
className="opacity-25"
|
||||
cx="12"
|
||||
cy="12"
|
||||
r="10"
|
||||
stroke="currentColor"
|
||||
strokeWidth="4"
|
||||
/>
|
||||
<path
|
||||
className="opacity-75"
|
||||
fill="currentColor"
|
||||
d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4z"
|
||||
/>
|
||||
</svg>
|
||||
<span>Starting terminal...</span>
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -10,7 +10,7 @@ import {
|
||||
useState,
|
||||
type ReactNode,
|
||||
} from "react"
|
||||
import { terminalSpawn, terminalKill } from "@/lib/api"
|
||||
import { terminalKill } from "@/lib/api"
|
||||
import { useFolderContext } from "@/contexts/folder-context"
|
||||
import { useShortcutSettings } from "@/hooks/use-shortcut-settings"
|
||||
import { matchShortcutEvent } from "@/lib/keyboard-shortcuts"
|
||||
@@ -18,6 +18,8 @@ import { matchShortcutEvent } from "@/lib/keyboard-shortcuts"
|
||||
export interface TerminalTab {
|
||||
id: string
|
||||
title: string
|
||||
workingDir: string
|
||||
initialCommand?: string
|
||||
}
|
||||
|
||||
const DEFAULT_HEIGHT = 300
|
||||
@@ -67,12 +69,12 @@ export function TerminalProvider({ children }: { children: ReactNode }) {
|
||||
const [tabs, setTabs] = useState<TerminalTab[]>([])
|
||||
const [activeTabId, setActiveTabId] = useState<string | null>(null)
|
||||
const tabCounterRef = useRef(0)
|
||||
const spawningRef = useRef(false)
|
||||
const suppressAutoCreateRef = useRef(false)
|
||||
const lastMouseActivityInTerminalRef = useRef(false)
|
||||
// Keep a ref of tabs for cleanup on unmount (effect [] captures stale state)
|
||||
const tabsRef = useRef(tabs)
|
||||
tabsRef.current = tabs
|
||||
useEffect(() => {
|
||||
tabsRef.current = tabs
|
||||
}, [tabs])
|
||||
|
||||
const folderPath = folder?.path ?? ""
|
||||
|
||||
@@ -83,54 +85,66 @@ export function TerminalProvider({ children }: { children: ReactNode }) {
|
||||
}, [])
|
||||
|
||||
const toggle = useCallback(() => {
|
||||
setIsOpen((prev) => !prev)
|
||||
}, [])
|
||||
const autoId = crypto.randomUUID()
|
||||
const nextCounter = tabCounterRef.current + 1
|
||||
|
||||
setIsOpen((wasOpen) => !wasOpen)
|
||||
|
||||
// Auto-create first terminal when opening with no tabs
|
||||
setTabs((currentTabs) => {
|
||||
if (currentTabs.length > 0 || !folderPath) return currentTabs
|
||||
tabCounterRef.current = nextCounter
|
||||
return [
|
||||
{
|
||||
id: autoId,
|
||||
title: `Terminal ${nextCounter}`,
|
||||
workingDir: folderPath,
|
||||
},
|
||||
]
|
||||
})
|
||||
|
||||
setActiveTabId((prev) => {
|
||||
if (prev !== null) return prev
|
||||
if (!folderPath) return null
|
||||
return autoId
|
||||
})
|
||||
}, [folderPath])
|
||||
|
||||
const createTerminalWithCommand = useCallback(
|
||||
async (title: string, command: string) => {
|
||||
if (!folderPath) return null
|
||||
|
||||
suppressAutoCreateRef.current = true
|
||||
setIsOpen(true)
|
||||
|
||||
try {
|
||||
const id = await terminalSpawn(folderPath, command)
|
||||
tabCounterRef.current += 1
|
||||
setTabs((prev) => [...prev, { id, title }])
|
||||
setActiveTabId(id)
|
||||
return id
|
||||
} catch (err) {
|
||||
console.error("Failed to spawn terminal for command:", err)
|
||||
return null
|
||||
} finally {
|
||||
suppressAutoCreateRef.current = false
|
||||
}
|
||||
const id = crypto.randomUUID()
|
||||
tabCounterRef.current += 1
|
||||
setTabs((prev) => [
|
||||
...prev,
|
||||
{ id, title, workingDir: folderPath, initialCommand: command },
|
||||
])
|
||||
setActiveTabId(id)
|
||||
|
||||
return id
|
||||
},
|
||||
[folderPath]
|
||||
)
|
||||
|
||||
const createTerminalInDirectory = useCallback(
|
||||
async (workingDir: string, title?: string) => {
|
||||
if (!workingDir || spawningRef.current) return null
|
||||
if (!workingDir) return null
|
||||
|
||||
suppressAutoCreateRef.current = true
|
||||
setIsOpen(true)
|
||||
spawningRef.current = true
|
||||
|
||||
try {
|
||||
const id = await terminalSpawn(workingDir)
|
||||
tabCounterRef.current += 1
|
||||
const defaultTitle = `Terminal ${tabCounterRef.current}`
|
||||
setTabs((prev) => [...prev, { id, title: title ?? defaultTitle }])
|
||||
setActiveTabId(id)
|
||||
return id
|
||||
} catch (err) {
|
||||
console.error("Failed to spawn terminal in directory:", err)
|
||||
return null
|
||||
} finally {
|
||||
spawningRef.current = false
|
||||
suppressAutoCreateRef.current = false
|
||||
}
|
||||
const id = crypto.randomUUID()
|
||||
tabCounterRef.current += 1
|
||||
const defaultTitle = `Terminal ${tabCounterRef.current}`
|
||||
setTabs((prev) => [
|
||||
...prev,
|
||||
{ id, title: title ?? defaultTitle, workingDir },
|
||||
])
|
||||
setActiveTabId(id)
|
||||
|
||||
return id
|
||||
},
|
||||
[]
|
||||
)
|
||||
@@ -140,18 +154,10 @@ export function TerminalProvider({ children }: { children: ReactNode }) {
|
||||
await createTerminalInDirectory(folderPath)
|
||||
}, [folderPath, createTerminalInDirectory])
|
||||
|
||||
// Auto-create first terminal when panel opens with no tabs
|
||||
useEffect(() => {
|
||||
if (isOpen && tabs.length === 0 && !suppressAutoCreateRef.current) {
|
||||
createTerminal()
|
||||
}
|
||||
}, [isOpen, tabs.length, createTerminal])
|
||||
|
||||
const setHeight = useCallback((h: number) => {
|
||||
setHeightState(Math.max(MIN_HEIGHT, Math.min(MAX_HEIGHT, h)))
|
||||
}, [])
|
||||
|
||||
// No stale closure — reads current activeTabId via updater
|
||||
const closeTerminal = useCallback((id: string) => {
|
||||
terminalKill(id).catch(() => {})
|
||||
setTabs((prev) => {
|
||||
@@ -160,19 +166,15 @@ export function TerminalProvider({ children }: { children: ReactNode }) {
|
||||
tabCounterRef.current = 0
|
||||
setIsOpen(false)
|
||||
setActiveTabId(null)
|
||||
} else {
|
||||
setActiveTabId((prevActive) =>
|
||||
prevActive === id ? next[next.length - 1].id : prevActive
|
||||
)
|
||||
}
|
||||
return next
|
||||
})
|
||||
setActiveTabId((prev) => (prev === id ? null : prev))
|
||||
}, [])
|
||||
|
||||
// Auto-select last tab when active tab is removed
|
||||
useEffect(() => {
|
||||
if (activeTabId === null && tabs.length > 0) {
|
||||
setActiveTabId(tabs[tabs.length - 1].id)
|
||||
}
|
||||
}, [activeTabId, tabs])
|
||||
|
||||
const closeOtherTerminals = useCallback(
|
||||
(id: string) => {
|
||||
setTabs((prev) => {
|
||||
|
||||
@@ -1265,11 +1265,13 @@ export async function gitCommitBranches(
|
||||
|
||||
export async function terminalSpawn(
|
||||
workingDir: string,
|
||||
initialCommand?: string
|
||||
initialCommand?: string,
|
||||
terminalId?: string
|
||||
): Promise<string> {
|
||||
return getTransport().call("terminal_spawn", {
|
||||
workingDir,
|
||||
initialCommand: initialCommand ?? null,
|
||||
terminalId: terminalId ?? null,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -1098,11 +1098,13 @@ export async function gitCommitBranches(
|
||||
|
||||
export async function terminalSpawn(
|
||||
workingDir: string,
|
||||
initialCommand?: string
|
||||
initialCommand?: string,
|
||||
terminalId?: string
|
||||
): Promise<string> {
|
||||
return invoke("terminal_spawn", {
|
||||
workingDir,
|
||||
initialCommand: initialCommand ?? null,
|
||||
terminalId: terminalId ?? null,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user