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(
|
pub async fn terminal_spawn(
|
||||||
working_dir: String,
|
working_dir: String,
|
||||||
initial_command: Option<String>,
|
initial_command: Option<String>,
|
||||||
|
terminal_id: Option<String>,
|
||||||
manager: State<'_, TerminalManager>,
|
manager: State<'_, TerminalManager>,
|
||||||
app_handle: tauri::AppHandle,
|
app_handle: tauri::AppHandle,
|
||||||
window: tauri::WebviewWindow,
|
window: tauri::WebviewWindow,
|
||||||
) -> Result<String, TerminalError> {
|
) -> 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
|
let app_data_dir = app_handle
|
||||||
.path()
|
.path()
|
||||||
|
|||||||
@@ -162,6 +162,17 @@ impl TerminalManager {
|
|||||||
opts: SpawnOptions,
|
opts: SpawnOptions,
|
||||||
emitter: EventEmitter,
|
emitter: EventEmitter,
|
||||||
) -> Result<String, TerminalError> {
|
) -> 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 pty_system = native_pty_system();
|
||||||
|
|
||||||
let pair = pty_system
|
let pair = pty_system
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ use crate::terminal::types::TerminalInfo;
|
|||||||
pub struct TerminalSpawnParams {
|
pub struct TerminalSpawnParams {
|
||||||
pub working_dir: String,
|
pub working_dir: String,
|
||||||
pub initial_command: Option<String>,
|
pub initial_command: Option<String>,
|
||||||
|
pub terminal_id: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Deserialize)]
|
#[derive(Deserialize)]
|
||||||
@@ -50,7 +51,10 @@ pub async fn terminal_spawn(
|
|||||||
Json(params): Json<TerminalSpawnParams>,
|
Json(params): Json<TerminalSpawnParams>,
|
||||||
) -> Result<Json<String>, AppCommandError> {
|
) -> Result<Json<String>, AppCommandError> {
|
||||||
let manager = &state.terminal_manager;
|
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);
|
let extra_env = prepare_credential_env(&state.data_dir);
|
||||||
|
|
||||||
|
|||||||
@@ -18,6 +18,8 @@ export function TerminalPanel() {
|
|||||||
<TerminalView
|
<TerminalView
|
||||||
key={tab.id}
|
key={tab.id}
|
||||||
terminalId={tab.id}
|
terminalId={tab.id}
|
||||||
|
workingDir={tab.workingDir}
|
||||||
|
initialCommand={tab.initialCommand}
|
||||||
isActive={tab.id === activeTabId}
|
isActive={tab.id === activeTabId}
|
||||||
isVisible={isOpen}
|
isVisible={isOpen}
|
||||||
/>
|
/>
|
||||||
|
|||||||
@@ -1,8 +1,13 @@
|
|||||||
"use client"
|
"use client"
|
||||||
|
|
||||||
import { useEffect, useRef } from "react"
|
import { useEffect, useRef, useState } from "react"
|
||||||
import { subscribe } from "@/lib/platform"
|
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 { TerminalEvent } from "@/lib/types"
|
||||||
import type { ITheme } from "@xterm/xterm"
|
import type { ITheme } from "@xterm/xterm"
|
||||||
|
|
||||||
@@ -86,12 +91,16 @@ function getTerminalTheme(container: HTMLDivElement | null): ITheme {
|
|||||||
|
|
||||||
interface TerminalViewProps {
|
interface TerminalViewProps {
|
||||||
terminalId: string
|
terminalId: string
|
||||||
|
workingDir: string
|
||||||
|
initialCommand?: string
|
||||||
isActive: boolean
|
isActive: boolean
|
||||||
isVisible: boolean
|
isVisible: boolean
|
||||||
}
|
}
|
||||||
|
|
||||||
export function TerminalView({
|
export function TerminalView({
|
||||||
terminalId,
|
terminalId,
|
||||||
|
workingDir,
|
||||||
|
initialCommand,
|
||||||
isActive,
|
isActive,
|
||||||
isVisible,
|
isVisible,
|
||||||
}: TerminalViewProps) {
|
}: TerminalViewProps) {
|
||||||
@@ -101,6 +110,7 @@ export function TerminalView({
|
|||||||
const lastResizeRef = useRef<{ cols: number; rows: number } | null>(null)
|
const lastResizeRef = useRef<{ cols: number; rows: number } | null>(null)
|
||||||
const isActiveRef = useRef(isActive)
|
const isActiveRef = useRef(isActive)
|
||||||
const isVisibleRef = useRef(isVisible)
|
const isVisibleRef = useRef(isVisible)
|
||||||
|
const [loading, setLoading] = useState(true)
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
isActiveRef.current = isActive
|
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>(
|
const unlisten = await subscribe<TerminalEvent>(
|
||||||
`terminal://output/${terminalId}`,
|
`terminal://output/${terminalId}`,
|
||||||
(payload) => {
|
(payload) => {
|
||||||
@@ -192,6 +202,27 @@ export function TerminalView({
|
|||||||
return
|
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 fitIfReady = () => {
|
||||||
const el = containerRef.current
|
const el = containerRef.current
|
||||||
if (!el) return
|
if (!el) return
|
||||||
@@ -237,7 +268,7 @@ export function TerminalView({
|
|||||||
cancelled = true
|
cancelled = true
|
||||||
cleanup?.()
|
cleanup?.()
|
||||||
}
|
}
|
||||||
}, [terminalId])
|
}, [terminalId, workingDir, initialCommand])
|
||||||
|
|
||||||
// Refit and focus when becoming active or panel becomes visible
|
// Refit and focus when becoming active or panel becomes visible
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
@@ -262,6 +293,32 @@ export function TerminalView({
|
|||||||
aria-hidden={!isActive}
|
aria-hidden={!isActive}
|
||||||
>
|
>
|
||||||
<div ref={containerRef} className="h-full w-full" />
|
<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>
|
</div>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -10,7 +10,7 @@ import {
|
|||||||
useState,
|
useState,
|
||||||
type ReactNode,
|
type ReactNode,
|
||||||
} from "react"
|
} from "react"
|
||||||
import { terminalSpawn, terminalKill } from "@/lib/api"
|
import { terminalKill } from "@/lib/api"
|
||||||
import { useFolderContext } from "@/contexts/folder-context"
|
import { useFolderContext } from "@/contexts/folder-context"
|
||||||
import { useShortcutSettings } from "@/hooks/use-shortcut-settings"
|
import { useShortcutSettings } from "@/hooks/use-shortcut-settings"
|
||||||
import { matchShortcutEvent } from "@/lib/keyboard-shortcuts"
|
import { matchShortcutEvent } from "@/lib/keyboard-shortcuts"
|
||||||
@@ -18,6 +18,8 @@ import { matchShortcutEvent } from "@/lib/keyboard-shortcuts"
|
|||||||
export interface TerminalTab {
|
export interface TerminalTab {
|
||||||
id: string
|
id: string
|
||||||
title: string
|
title: string
|
||||||
|
workingDir: string
|
||||||
|
initialCommand?: string
|
||||||
}
|
}
|
||||||
|
|
||||||
const DEFAULT_HEIGHT = 300
|
const DEFAULT_HEIGHT = 300
|
||||||
@@ -67,12 +69,12 @@ export function TerminalProvider({ children }: { children: ReactNode }) {
|
|||||||
const [tabs, setTabs] = useState<TerminalTab[]>([])
|
const [tabs, setTabs] = useState<TerminalTab[]>([])
|
||||||
const [activeTabId, setActiveTabId] = useState<string | null>(null)
|
const [activeTabId, setActiveTabId] = useState<string | null>(null)
|
||||||
const tabCounterRef = useRef(0)
|
const tabCounterRef = useRef(0)
|
||||||
const spawningRef = useRef(false)
|
|
||||||
const suppressAutoCreateRef = useRef(false)
|
|
||||||
const lastMouseActivityInTerminalRef = useRef(false)
|
const lastMouseActivityInTerminalRef = useRef(false)
|
||||||
// Keep a ref of tabs for cleanup on unmount (effect [] captures stale state)
|
// Keep a ref of tabs for cleanup on unmount (effect [] captures stale state)
|
||||||
const tabsRef = useRef(tabs)
|
const tabsRef = useRef(tabs)
|
||||||
tabsRef.current = tabs
|
useEffect(() => {
|
||||||
|
tabsRef.current = tabs
|
||||||
|
}, [tabs])
|
||||||
|
|
||||||
const folderPath = folder?.path ?? ""
|
const folderPath = folder?.path ?? ""
|
||||||
|
|
||||||
@@ -83,54 +85,66 @@ export function TerminalProvider({ children }: { children: ReactNode }) {
|
|||||||
}, [])
|
}, [])
|
||||||
|
|
||||||
const toggle = useCallback(() => {
|
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(
|
const createTerminalWithCommand = useCallback(
|
||||||
async (title: string, command: string) => {
|
async (title: string, command: string) => {
|
||||||
if (!folderPath) return null
|
if (!folderPath) return null
|
||||||
|
|
||||||
suppressAutoCreateRef.current = true
|
|
||||||
setIsOpen(true)
|
setIsOpen(true)
|
||||||
|
|
||||||
try {
|
const id = crypto.randomUUID()
|
||||||
const id = await terminalSpawn(folderPath, command)
|
tabCounterRef.current += 1
|
||||||
tabCounterRef.current += 1
|
setTabs((prev) => [
|
||||||
setTabs((prev) => [...prev, { id, title }])
|
...prev,
|
||||||
setActiveTabId(id)
|
{ id, title, workingDir: folderPath, initialCommand: command },
|
||||||
return id
|
])
|
||||||
} catch (err) {
|
setActiveTabId(id)
|
||||||
console.error("Failed to spawn terminal for command:", err)
|
|
||||||
return null
|
return id
|
||||||
} finally {
|
|
||||||
suppressAutoCreateRef.current = false
|
|
||||||
}
|
|
||||||
},
|
},
|
||||||
[folderPath]
|
[folderPath]
|
||||||
)
|
)
|
||||||
|
|
||||||
const createTerminalInDirectory = useCallback(
|
const createTerminalInDirectory = useCallback(
|
||||||
async (workingDir: string, title?: string) => {
|
async (workingDir: string, title?: string) => {
|
||||||
if (!workingDir || spawningRef.current) return null
|
if (!workingDir) return null
|
||||||
|
|
||||||
suppressAutoCreateRef.current = true
|
|
||||||
setIsOpen(true)
|
setIsOpen(true)
|
||||||
spawningRef.current = true
|
|
||||||
|
|
||||||
try {
|
const id = crypto.randomUUID()
|
||||||
const id = await terminalSpawn(workingDir)
|
tabCounterRef.current += 1
|
||||||
tabCounterRef.current += 1
|
const defaultTitle = `Terminal ${tabCounterRef.current}`
|
||||||
const defaultTitle = `Terminal ${tabCounterRef.current}`
|
setTabs((prev) => [
|
||||||
setTabs((prev) => [...prev, { id, title: title ?? defaultTitle }])
|
...prev,
|
||||||
setActiveTabId(id)
|
{ id, title: title ?? defaultTitle, workingDir },
|
||||||
return id
|
])
|
||||||
} catch (err) {
|
setActiveTabId(id)
|
||||||
console.error("Failed to spawn terminal in directory:", err)
|
|
||||||
return null
|
return id
|
||||||
} finally {
|
|
||||||
spawningRef.current = false
|
|
||||||
suppressAutoCreateRef.current = false
|
|
||||||
}
|
|
||||||
},
|
},
|
||||||
[]
|
[]
|
||||||
)
|
)
|
||||||
@@ -140,18 +154,10 @@ export function TerminalProvider({ children }: { children: ReactNode }) {
|
|||||||
await createTerminalInDirectory(folderPath)
|
await createTerminalInDirectory(folderPath)
|
||||||
}, [folderPath, createTerminalInDirectory])
|
}, [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) => {
|
const setHeight = useCallback((h: number) => {
|
||||||
setHeightState(Math.max(MIN_HEIGHT, Math.min(MAX_HEIGHT, h)))
|
setHeightState(Math.max(MIN_HEIGHT, Math.min(MAX_HEIGHT, h)))
|
||||||
}, [])
|
}, [])
|
||||||
|
|
||||||
// No stale closure — reads current activeTabId via updater
|
|
||||||
const closeTerminal = useCallback((id: string) => {
|
const closeTerminal = useCallback((id: string) => {
|
||||||
terminalKill(id).catch(() => {})
|
terminalKill(id).catch(() => {})
|
||||||
setTabs((prev) => {
|
setTabs((prev) => {
|
||||||
@@ -160,19 +166,15 @@ export function TerminalProvider({ children }: { children: ReactNode }) {
|
|||||||
tabCounterRef.current = 0
|
tabCounterRef.current = 0
|
||||||
setIsOpen(false)
|
setIsOpen(false)
|
||||||
setActiveTabId(null)
|
setActiveTabId(null)
|
||||||
|
} else {
|
||||||
|
setActiveTabId((prevActive) =>
|
||||||
|
prevActive === id ? next[next.length - 1].id : prevActive
|
||||||
|
)
|
||||||
}
|
}
|
||||||
return next
|
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(
|
const closeOtherTerminals = useCallback(
|
||||||
(id: string) => {
|
(id: string) => {
|
||||||
setTabs((prev) => {
|
setTabs((prev) => {
|
||||||
|
|||||||
@@ -1265,11 +1265,13 @@ export async function gitCommitBranches(
|
|||||||
|
|
||||||
export async function terminalSpawn(
|
export async function terminalSpawn(
|
||||||
workingDir: string,
|
workingDir: string,
|
||||||
initialCommand?: string
|
initialCommand?: string,
|
||||||
|
terminalId?: string
|
||||||
): Promise<string> {
|
): Promise<string> {
|
||||||
return getTransport().call("terminal_spawn", {
|
return getTransport().call("terminal_spawn", {
|
||||||
workingDir,
|
workingDir,
|
||||||
initialCommand: initialCommand ?? null,
|
initialCommand: initialCommand ?? null,
|
||||||
|
terminalId: terminalId ?? null,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1098,11 +1098,13 @@ export async function gitCommitBranches(
|
|||||||
|
|
||||||
export async function terminalSpawn(
|
export async function terminalSpawn(
|
||||||
workingDir: string,
|
workingDir: string,
|
||||||
initialCommand?: string
|
initialCommand?: string,
|
||||||
|
terminalId?: string
|
||||||
): Promise<string> {
|
): Promise<string> {
|
||||||
return invoke("terminal_spawn", {
|
return invoke("terminal_spawn", {
|
||||||
workingDir,
|
workingDir,
|
||||||
initialCommand: initialCommand ?? null,
|
initialCommand: initialCommand ?? null,
|
||||||
|
terminalId: terminalId ?? null,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user