diff --git a/src/main/ipc-handlers.ts b/src/main/ipc-handlers.ts index bd019f1..980daf7 100644 --- a/src/main/ipc-handlers.ts +++ b/src/main/ipc-handlers.ts @@ -44,6 +44,12 @@ export function registerIpcHandlers(windowManager: WindowManager, cdpProxyInstan }; const created = ptyManager.create(resolvedOptions); const id = created.id; + // Reused PTY (idempotent create — e.g. StrictMode's double create() race): + // the original create already wired data/exit forwarding. Re-wiring here + // would forward every chunk twice and double everything in the renderer. + if (created.reused) { + return created; + } const window = BrowserWindow.fromWebContents(_event.sender); const unsubData = ptyManager.onData(id, (data) => { if (window && !window.isDestroyed()) { diff --git a/src/main/pty-manager.ts b/src/main/pty-manager.ts index d42464e..6f75a78 100644 --- a/src/main/pty-manager.ts +++ b/src/main/pty-manager.ts @@ -95,6 +95,15 @@ interface PtyEntry { writeChain: Promise; pendingChunks: number; alive: boolean; + // Last applied size. Used to drop redundant same-size resizes, which would + // otherwise make the shell (PSReadLine/oh-my-posh) redraw the prompt for no + // reason — a cause of the doubled prompt on startup. + cols: number; + rows: number; + // Resolved shell + whether startup commands were baked in — returned verbatim + // when create() is called again for the same surfaceId (idempotent reuse). + shell: string; + startupConsumed: boolean; } export interface CreateOptions { @@ -106,8 +115,33 @@ export interface CreateOptions { /** When provided, use this as the PTY key instead of generating a new one. * This keeps Surface IDs and PTY IDs in sync for reliable re-attachment. */ surfaceId?: SurfaceId; + /** Quick-launch profile commands (issue #32). When the shell type supports it + * they are baked into the shell's own startup (see `startupCommandsConsumed` + * in the return value) rather than injected later as keystrokes. */ + startupCommands?: string[]; } +// Primary Device Attributes (DA1). oh-my-posh / PSReadLine probe the terminal +// with a DA1 query and block briefly for the reply before drawing the prompt. +// +// xterm answers DA1 too, but its reply travels a slow multi-process round-trip +// (main → renderer → xterm → renderer → main → pty). That latency is the cause +// of three symptoms users saw: the reply arriving after the prompt was drawn and +// leaking onto the command line as `\x1b[?62;4;9;22c`; and, once xterm's reply +// was suppressed to stop that leak, the probe getting no reply at all — so the +// prompt stalled ~3-5s on the probe's timeout and re-rendered (a doubled prompt). +// +// Answering here, in the same process as the PTY, is effectively instant, so the +// probe is satisfied before the prompt draws: one clean prompt, no junk, no +// stall. xterm's own DA1 reply is suppressed in useTerminal so this is the only +// one. The query is `\x1b[c` or `\x1b[c` (no `?`/`>`/`=` prefix — those are +// the reply / DA2 / DA3 forms, which this deliberately does not match). The +// reply advertises the same attributes xterm-with-image did (62=VT220, 4=Sixel, +// 9, 22=ANSI color) so image-capable apps still detect support. +// eslint-disable-next-line no-control-regex -- ESC is intentional: this matches the DA1 query byte-for-byte +const DA1_QUERY = /\x1b\[\d*c/; +const DA1_REPLY = '\x1b[?62;4;9;22c'; + export class PtyManager { private ptys = new Map(); @@ -118,9 +152,27 @@ export class PtyManager { private static readonly CHUNK_THRESHOLD = 1024; private static readonly CHUNK_SIZE = 1024; - create(options: CreateOptions): { id: SurfaceId; shell: string } { + create(options: CreateOptions): { id: SurfaceId; shell: string; startupCommandsConsumed: boolean; reused: boolean } { const id: SurfaceId = options.surfaceId ?? `surf-${uuidv4()}` as SurfaceId; + // Idempotent per surfaceId. React StrictMode (dev) double-mounts the terminal + // component, and the renderer's `pty.has()` check is async — so create() can + // fire twice for the same surface before the first spawn registers. Without + // this guard the second call spawns a SECOND PowerShell process under the + // same id: both stream to the renderer (doubled prompt + every keystroke + // echoed twice) and the first leaks as an orphan. Reuse the live PTY instead. + if (options.surfaceId) { + const existing = this.ptys.get(options.surfaceId); + if (existing && existing.alive) { + return { + id: options.surfaceId, + shell: existing.shell, + startupCommandsConsumed: existing.startupConsumed, + reused: true, + }; + } + } + const shell = resolveShell(options.shell); const shellType = getShellType(shell); const integrationDir = getShellIntegrationPath(); @@ -155,6 +207,30 @@ export class PtyManager { env.WMUX_INTEGRATION = '1'; } + // Quick-launch startup commands (issue #32). Run them as part of the shell's + // own initialization — BEFORE the first interactive prompt — instead of + // injecting them later as keystrokes (`pty.write('\r')`). + // + // The keystroke approach raced the shell's init-time terminal queries: with + // oh-my-posh/PSReadLine, ConPTY answers a Device Attributes query (DA1) by + // writing `\x1b[?62;4;9;22c` onto the shell's stdin. If that response landed + // on the prompt the same instant our injected `\r` arrived, PSReadLine + // merged them into one bogus executed line (e.g. `62;4;9;22ccls`). Baking the + // commands into the integration script (via WMUX_STARTUP_COMMANDS) removes + // the race: they run during init and the first prompt render — the only one + // that triggers the leaky query — happens afterward, exactly as it does for a + // plain terminal that shows no junk. + const startupCommands = (options.startupCommands ?? []).filter( + (cmd): cmd is string => typeof cmd === 'string' && cmd.trim().length > 0, + ); + let startupCommandsConsumed = false; + if (startupCommands.length > 0 && shellType === 'powershell' && env.WMUX_PS1_SCRIPT) { + // Newlines survive the env block; the integration script trims each line + // (so a stray CR is harmless) and runs it via Invoke-Expression. + env.WMUX_STARTUP_COMMANDS = startupCommands.join('\n'); + startupCommandsConsumed = true; + } + const spawnOptions: pty.IWindowsPtyForkOptions = { name: 'xterm-256color', cols: options.cols ?? 80, @@ -183,9 +259,19 @@ export class PtyManager { writeChain: Promise.resolve(), pendingChunks: 0, alive: true, + cols: spawnOptions.cols ?? 80, + rows: spawnOptions.rows ?? 24, + shell, + startupConsumed: startupCommandsConsumed, }; ptyProcess.onData((data) => { + // Answer DA1 probes in-process so the prompt never stalls or leaks the + // reply (see DA1_QUERY note above). Only the escape character is common + // enough to warrant the cheap guard before the regex scan. + if (entry.alive && data.indexOf('\x1b[') !== -1 && DA1_QUERY.test(data)) { + try { ptyProcess.write(DA1_REPLY); } catch { /* pty disposed between events */ } + } for (const listener of entry.dataListeners) { listener(data); } @@ -200,7 +286,7 @@ export class PtyManager { }); this.ptys.set(id, entry); - return { id, shell }; + return { id, shell, startupCommandsConsumed, reused: false }; } write(id: SurfaceId, data: string): void { @@ -253,9 +339,13 @@ export class PtyManager { resize(id: SurfaceId, cols: number, rows: number): void { const entry = this.ptys.get(id); - if (entry) { - entry.pty.resize(cols, rows); - } + if (!entry) return; + // Drop no-op resizes: a same-size resize still makes the shell redraw its + // prompt (doubled-prompt cause). Only forward genuine size changes. + if (cols === entry.cols && rows === entry.rows) return; + entry.cols = cols; + entry.rows = rows; + entry.pty.resize(cols, rows); } kill(id: SurfaceId): void { diff --git a/src/preload/index.ts b/src/preload/index.ts index 95b3541..0b74834 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -3,8 +3,8 @@ import { IPC_CHANNELS } from '../shared/types'; contextBridge.exposeInMainWorld('wmux', { pty: { - create: (options: { shell: string; cwd: string; env: Record; surfaceId?: string }) => - ipcRenderer.invoke(IPC_CHANNELS.PTY_CREATE, options) as Promise<{ id: string; shell: string }>, + create: (options: { shell: string; cwd: string; env: Record; surfaceId?: string; startupCommands?: string[]; cols?: number; rows?: number }) => + ipcRenderer.invoke(IPC_CHANNELS.PTY_CREATE, options) as Promise<{ id: string; shell: string; startupCommandsConsumed?: boolean }>, write: (id: string, data: string) => ipcRenderer.send(IPC_CHANNELS.PTY_WRITE, id, data), resize: (id: string, cols: number, rows: number) => diff --git a/src/renderer/hooks/useTerminal.ts b/src/renderer/hooks/useTerminal.ts index bb1119c..1a2c6fd 100644 --- a/src/renderer/hooks/useTerminal.ts +++ b/src/renderer/hooks/useTerminal.ts @@ -176,6 +176,15 @@ export function useTerminal({ surfaceId, shell, cwd, visible = true, focused = t xtermRef.current = terminal; + // Set true by the cleanup below. React StrictMode (dev) double-invokes + // effects as setup → cleanup → setup, so the terminal can be disposed while + // async work is still in flight (a late `pty.create().then()`, a buffered + // `terminal.write()`, a queued requestAnimationFrame). Touching xterm after + // dispose hits a render service whose renderer is gone and throws + // "Cannot read properties of undefined (reading 'dimensions')" from deep in + // Viewport.syncScrollArea. Every async callback checks this flag first. + let disposed = false; + // Create and load addons const fitAddon = new FitAddon(); const webLinksAddon = new WebLinksAddon((event, uri) => { @@ -196,6 +205,23 @@ export function useTerminal({ surfaceId, shell, cwd, visible = true, focused = t terminal.loadAddon(imageAddon); terminal.unicode.activeVersion = '11'; + // Suppress xterm's automatic Primary Device Attributes (DA1) reply — the + // main process answers DA1 instead (see DA1_QUERY in pty-manager.ts). + // + // xterm would otherwise answer a DA1 query (`\x1b[c` / `\x1b[0c`) by emitting + // a reply through onData that we forward to the PTY. With the image addon + // loaded that reply is `\x1b[?62;4;9;22c`. Because the PTY↔renderer hop is + // multi-process, that reply arrives too late: it lands after the shell has + // drawn its prompt, so oh-my-posh/PSReadLine echo it as a typed line (the + // `[?62;4;9;22c` junk) and re-render. The main process now answers the same + // probe in-process (instant), so we must stop xterm sending its slow + // duplicate — otherwise the late reply leaks again. + // + // Registered AFTER the image addon so it wins precedence: xterm runs CSI + // handlers newest-first and stops at the first returning true, so neither the + // image addon's DA1 override nor xterm's built-in reply runs. + terminal.parser.registerCsiHandler({ final: 'c' }, () => true); + // Open terminal in the DOM terminal.open(terminalRef.current); @@ -429,6 +455,7 @@ export function useTerminal({ surfaceId, shell, cwd, visible = true, focused = t // Wire PTY data → xterm const unsubData = window.wmux.pty.onData(id, (data: string) => { + if (disposed) return; terminal.write(data); }); @@ -447,12 +474,16 @@ export function useTerminal({ surfaceId, shell, cwd, visible = true, focused = t } }; - // Run a quick-launch profile's startup commands once, after the shell has had - // a moment to load its integration script and print a prompt. ConPTY buffers - // input, so the short delay just avoids the commands echoing before the - // banner; it doesn't gate correctness. Only fresh PTYs run these — re-mounting - // a keep-alive tab attaches to an existing PTY and skips this path. - const runStartupCommands = (id: string) => { + // Fallback path for quick-launch startup commands on shells where the main + // process couldn't bake them into the shell's own init (anything other than + // PowerShell — see PtyManager.create). PowerShell runs them via the + // integration script before the first prompt, which avoids a keystroke race + // against the shell's init-time terminal queries (a ConPTY DA1 response + // leaking onto the prompt as `\x1b[?62;4;9;22c` and merging with an injected + // `\r` into a bogus line like `62;4;9;22ccls`). When `consumed` is true + // we MUST NOT also inject, or the commands would run twice. + const runStartupCommands = (id: string, consumed: boolean) => { + if (consumed) return; const cmds = startupCommandsRef.current; if (!cmds || cmds.length === 0) return; setTimeout(() => { @@ -468,6 +499,23 @@ export function useTerminal({ surfaceId, shell, cwd, visible = true, focused = t // Read prefs at spawn time so changing the default later doesn't re-spawn live PTYs. const effectiveShell = shell || useStore.getState().workspacePrefs.defaultShell || ''; + // Spawn the PTY at the already-measured terminal size. Otherwise it starts at + // the 80x24 default and our follow-up resize triggers a window-size-change in + // the shell, which makes PSReadLine/oh-my-posh redraw the prompt — the doubled + // prompt users saw. A hidden/unmeasured tab yields no dims and falls back to + // the default, then resizes correctly when first shown (that redraw isn't + // visible). proposeDimensions needs the element laid out, which it is after + // terminal.open() above. + let initialCols: number | undefined; + let initialRows: number | undefined; + try { + const dims = fitAddon.proposeDimensions(); + if (dims && dims.cols > 0 && dims.rows > 0) { + initialCols = dims.cols; + initialRows = dims.rows; + } + } catch { /* element not measurable yet — fall back to PTY default */ } + // If surfaceId is given AND a PTY already exists for it (agent spawn or re-mount), attach to it if (surfaceId && window.wmux.pty.has) { window.wmux.pty.has(surfaceId).then((exists: boolean) => { @@ -475,22 +523,25 @@ export function useTerminal({ surfaceId, shell, cwd, visible = true, focused = t attachToPty(surfaceId!); } else { // No existing PTY — create a new one, passing surfaceId so PTY ID = Surface ID - window.wmux.pty.create({ shell: effectiveShell, cwd: cwd ?? '', env: {}, surfaceId }) - .then((created: { id: string; shell: string }) => { + window.wmux.pty.create({ shell: effectiveShell, cwd: cwd ?? '', env: {}, surfaceId, startupCommands: startupCommandsRef.current, cols: initialCols, rows: initialRows }) + .then((created: { id: string; shell: string; startupCommandsConsumed?: boolean }) => { + // PTY persists (keep-alive); a remount re-attaches via pty.has. + if (disposed) return; setResolvedShellForSurface(surfaceId, created.shell); attachToPty(created.id); - runStartupCommands(created.id); + runStartupCommands(created.id, !!created.startupCommandsConsumed); }) .catch((err: unknown) => terminal.writeln(`\r\n\x1b[31m[failed to create PTY: ${err}]\x1b[0m`)); } }); } else { // No surfaceId hint — always create new PTY - window.wmux.pty.create({ shell: effectiveShell, cwd: cwd ?? '', env: {} }) - .then((created: { id: string; shell: string }) => { + window.wmux.pty.create({ shell: effectiveShell, cwd: cwd ?? '', env: {}, startupCommands: startupCommandsRef.current, cols: initialCols, rows: initialRows }) + .then((created: { id: string; shell: string; startupCommandsConsumed?: boolean }) => { + if (disposed) return; setResolvedShellForSurface(surfaceId, created.shell); attachToPty(created.id); - runStartupCommands(created.id); + runStartupCommands(created.id, !!created.startupCommandsConsumed); }) .catch((err: unknown) => terminal.writeln(`\r\n\x1b[31m[failed to create PTY: ${err}]\x1b[0m`)); } @@ -520,6 +571,10 @@ export function useTerminal({ surfaceId, shell, cwd, visible = true, focused = t // Cleanup return () => { + // Mark disposed FIRST so any async callback that fires during/after + // teardown (late pty.create().then, buffered write, queued rAF) bails out + // before touching the soon-to-be-disposed terminal. + disposed = true; resizeObserver.disconnect(); if (resizeRaf !== null) cancelAnimationFrame(resizeRaf); dataDisposable.dispose(); @@ -574,6 +629,12 @@ export function useTerminal({ surfaceId, shell, cwd, visible = true, focused = t // sessions keystrokes still target the previously-focused (now hidden) // terminal and the new session looks frozen. useEffect(() => { + // Track the nested rAFs so they can be cancelled if the terminal is hidden + // or unmounted before they fire. Otherwise (notably under StrictMode's + // double-mount) they run fit()/resize/refresh on a disposed terminal and + // throw from Viewport.syncScrollArea ("...reading 'dimensions'"). + let raf1: number | null = null; + let raf2: number | null = null; if (visible && fitAddonRef.current && xtermRef.current) { const term = xtermRef.current; // Attach the GPU renderer on show (WebGL → Canvas → DOM). A Canvas/DOM @@ -582,8 +643,10 @@ export function useTerminal({ surfaceId, shell, cwd, visible = true, focused = t if (!rendererRef.current) { rendererRef.current = attachVisibleRenderer(term); } - requestAnimationFrame(() => { - requestAnimationFrame(() => { + raf1 = requestAnimationFrame(() => { + raf2 = requestAnimationFrame(() => { + // The terminal may have been disposed between scheduling and firing. + if (!xtermRef.current) return; fit(); const dims = fitAddonRef.current?.proposeDimensions(); if (dims && ptyIdRef.current) { @@ -601,6 +664,10 @@ export function useTerminal({ surfaceId, shell, cwd, visible = true, focused = t rendererRef.current.dispose(); rendererRef.current = null; } + return () => { + if (raf1 !== null) cancelAnimationFrame(raf1); + if (raf2 !== null) cancelAnimationFrame(raf2); + }; }, [visible, focused]); return { terminalRef, fit, xtermRef, searchAddonRef }; diff --git a/src/shell-integration/wmux-powershell-integration.ps1 b/src/shell-integration/wmux-powershell-integration.ps1 index d0734c3..1f501d1 100644 --- a/src/shell-integration/wmux-powershell-integration.ps1 +++ b/src/shell-integration/wmux-powershell-integration.ps1 @@ -98,22 +98,51 @@ function prompt { } } -# PR polling background job (every 45 seconds) -$_wmux_pr_job = Start-Job -ScriptBlock { - param($surfaceId, $pipeName) - while ($true) { - Start-Sleep -Seconds 45 - try { - $prJson = gh pr view --json number,state,title 2>$null - if ($LASTEXITCODE -eq 0 -and $prJson) { - $pr = $prJson | ConvertFrom-Json - $pipe = New-Object System.IO.Pipes.NamedPipeClientStream(".", $pipeName, [System.IO.Pipes.PipeDirection]::InOut) - $pipe.Connect(1000) - $writer = New-Object System.IO.StreamWriter($pipe) - $writer.AutoFlush = $true - $writer.WriteLine("report_pr $surfaceId $($pr.number) $($pr.state) $($pr.title)") - $pipe.Close() - } - } catch { } +# PR polling background job (every 45 seconds). +# DEFERRED: Start-Job spins up a whole child PowerShell runspace and costs +# several hundred ms — running it during init delayed the FIRST prompt. We +# instead start it on the shell's first idle (after the prompt is already on +# screen), so it never sits on the startup critical path. A global guard makes it +# fire exactly once; PR data isn't needed in the first 45s anyway. +$global:_wmux_pr_started = $false +$null = Register-EngineEvent -SourceIdentifier ([System.Management.Automation.PSEngineEvent]::OnIdle) -Action { + if ($global:_wmux_pr_started) { return } + $global:_wmux_pr_started = $true + $global:_wmux_pr_job = Start-Job -ScriptBlock { + param($surfaceId, $pipeName) + while ($true) { + Start-Sleep -Seconds 45 + try { + $prJson = gh pr view --json number,state,title 2>$null + if ($LASTEXITCODE -eq 0 -and $prJson) { + $pr = $prJson | ConvertFrom-Json + $pipe = New-Object System.IO.Pipes.NamedPipeClientStream(".", $pipeName, [System.IO.Pipes.PipeDirection]::InOut) + $pipe.Connect(1000) + $writer = New-Object System.IO.StreamWriter($pipe) + $writer.AutoFlush = $true + $writer.WriteLine("report_pr $surfaceId $($pr.number) $($pr.state) $($pr.title)") + $pipe.Close() + } + } catch { } + } + } -ArgumentList $env:WMUX_SURFACE_ID, "wmux" +} + +# Quick-launch profile startup commands (issue #32). +# wmux passes these in WMUX_STARTUP_COMMANDS (newline-separated) so they run as +# part of init — before the first interactive prompt — rather than being injected +# as keystrokes afterward. Keystroke injection raced the shell's init-time +# Device Attributes query (ConPTY answers DA1 with "\e[?62;4;9;22c" on stdin); +# when that response landed on the prompt alongside an injected "\r" the two +# merged into a bogus executed line (e.g. "62;4;9;22ccls"). Running here avoids +# that entirely. Runs last so the prompt override / PSReadLine handlers exist. +if ($env:WMUX_STARTUP_COMMANDS) { + foreach ($_wmux_cmd in ($env:WMUX_STARTUP_COMMANDS -split "`n")) { + $_wmux_cmd = $_wmux_cmd.Trim() + if ($_wmux_cmd) { + try { Invoke-Expression $_wmux_cmd } catch { Write-Error $_ } + } } -} -ArgumentList $env:WMUX_SURFACE_ID, "wmux" + # One-shot: don't let it leak into child shells spawned from this session. + Remove-Item Env:\WMUX_STARTUP_COMMANDS -ErrorAction SilentlyContinue +}