Skip to content

fix(terminal): clean profile/terminal startup under oh-my-posh #37

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
amirlehmam merged 1 commit into from
Jun 20, 2026
Merged

fix(terminal): clean profile/terminal startup under oh-my-posh #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/main/ipc-handlers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
100 changes: 95 additions & 5 deletions src/main/pty-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,15 @@ interface PtyEntry {
writeChain: Promise<void>;
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 {
Expand All @@ -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[<n>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<SurfaceId, PtyEntry>();

Expand All @@ -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();
Expand Down Expand Up @@ -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('<cmd>\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 `<cmd>\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,
Expand Down Expand Up @@ -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);
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions src/preload/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { IPC_CHANNELS } from '../shared/types';

contextBridge.exposeInMainWorld('wmux', {
pty: {
create: (options: { shell: string; cwd: string; env: Record<string, string>; surfaceId?: string }) =>
ipcRenderer.invoke(IPC_CHANNELS.PTY_CREATE, options) as Promise<{ id: string; shell: string }>,
create: (options: { shell: string; cwd: string; env: Record<string, string>; 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) =>
Expand Down
95 changes: 81 additions & 14 deletions src/renderer/hooks/useTerminal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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);

Expand Down Expand Up @@ -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);
});

Expand All @@ -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
// `<cmd>\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(() => {
Expand All @@ -468,29 +499,49 @@ 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) => {
if (exists) {
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`));
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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 };
Expand Down
Loading