diff --git a/src/main/index.ts b/src/main/index.ts index 69b367dc..536c8807 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -56,7 +56,7 @@ import { FileContentWatcher } from './file-content-watcher' import { SnoozeTimer } from './snooze-timer' import { getWeeklyStats } from './weekly-stats' import type { TerminalTab, PaneNode, PaneLeaf } from '../shared/state/terminals' -import { getLeaves, mapLeaves } from '../shared/state/terminals' +import { findTabById, getLeaves, mapLeaves } from '../shared/state/terminals' import { listWorktrees, listBranches, continueWorktree, isWorktreeDirty, defaultWorktreeDir, getChangedFiles, getFileDiff, getBranchCommits, getCommitDiff, getCommitMeta, getCommitChangedFiles, getCommitFileDiffSides, getCommitRangeChangedFiles, getCommitRangeFileDiffSides, getMainWorktreeStatus, prepareMainForMerge, mergeWorktreeLocally, getBranchSha, previewMergeConflicts, getBranchDiffStats, listAllFiles, listRecentCommitShas, readWorktreeFile, readWorktreeFileBinary, writeWorktreeFile, getFileDiffSides, getCurrentBranch, renameWorktreeBranch, symlinkClaudeSettings, pruneWorktrees, type MergeStrategy } from './worktree' import { listOpenPRs, getPRByNumber, testToken, starRepo, unstarRepo, isRepoStarred, mergePR, approvePR, getRepoInfo, type GitHubMergeMethod, type MergePRResult, type PRLookupResult } from './github' import { AVAILABLE_EDITORS, DEFAULT_EDITOR_ID, openInEditor } from './editor' @@ -266,6 +266,18 @@ function findShellWorktree(shellId: string): string | null { } return null } + +/** True for a shell tab that was created with a command (`create_shell`) and + * whose process is gone. That command runs exactly once, in createShell's + * eager spawn; afterwards the tab is a transcript of the run, not a live + * shell — which is also how it behaves when the command exits mid-session. + * The renderer still fires pty:create when it mounts such a tab (it can't + * tell a spent shell from a fresh one), so the spawn is dropped here rather + * than handing back an interactive prompt no command shell would ever show. */ +function isSpentCommandShell(id: string): boolean { + const tab = findTabById(store.getSnapshot().state.terminals.panes, id) + return tab?.type === 'shell' && !!tab.command +} // Resolves the harness version from disk so it works in every runtime // (Electron dev/packaged, headless dev, headless tarball). Electron's // `app.getVersion()` would do the job in two of those four, but the @@ -3460,6 +3472,10 @@ function registerIpcHandlers(): void { : agentKind === 'cursor' ? config.cursorEnvVars : undefined const existed = ptyManager.hasTerminal(id) + if (!existed && isSpentCommandShell(id)) { + log('pty', `create id=${id} dropped — command shell already ran`) + return + } ptyManager.create(id, cwd, cmd, args, extraEnv, !isAgent, cols, rows) if (!existed) { // Creator becomes controller immediately so their first keystroke diff --git a/src/main/persistence-migrations.ts b/src/main/persistence-migrations.ts index 6525c8da..e86ff97b 100644 --- a/src/main/persistence-migrations.ts +++ b/src/main/persistence-migrations.ts @@ -24,7 +24,8 @@ export interface PersistedTab { sessionId?: string /** For browser tabs: last URL so we can restore the tab on reload. */ url?: string - /** For shell tabs: command passed via `zsh -ilc ` (agent-spawned). */ + /** For shell tabs: the command the tab was created with (agent-spawned). + * Kept for display only — restored tabs do not re-run it. */ command?: string /** For shell tabs: cwd (absolute or relative to worktree root). */ cwd?: string diff --git a/src/main/pty-manager.test.ts b/src/main/pty-manager.test.ts index da102dbc..8a6dce31 100644 --- a/src/main/pty-manager.test.ts +++ b/src/main/pty-manager.test.ts @@ -36,6 +36,7 @@ vi.mock('./persistence', () => ({ import { PtyManager } from './pty-manager' import * as pty from 'node-pty' +import { loadTerminalHistory } from './persistence' describe('PtyManager.create — eager spawn contract for createShell (#203)', () => { beforeEach(() => { @@ -67,3 +68,43 @@ describe('PtyManager.create — eager spawn contract for createShell (#203)', () expect(pty.spawn).not.toHaveBeenCalled() }) }) + +// On a cold app start the renderer asks for scrollback BEFORE it spawns, so +// getHistory has to reach the persisted file itself — reading only the +// in-memory map left restored tabs blank until something else seeded it. +describe('PtyManager.getHistory — persisted scrollback across an app restart', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('returns the persisted scrollback when nothing is in memory yet', () => { + vi.mocked(loadTerminalHistory).mockReturnValueOnce('previous run output') + const mgr = new PtyManager() + expect(mgr.getHistory('shell-restored-1')).toBe('previous run output') + }) + + it('returns empty string when no history was persisted', () => { + const mgr = new PtyManager() + expect(mgr.getHistory('shell-restored-2')).toBe('') + }) + + it('reads the file once per id — the spawn that follows reuses the buffer', () => { + vi.mocked(loadTerminalHistory).mockReturnValueOnce('previous run output') + const mgr = new PtyManager() + const id = 'shell-restored-3' + mgr.getHistory(id) + mgr.create(id, tmpdir(), '', ['-il'], undefined, true) + expect(loadTerminalHistory).toHaveBeenCalledTimes(1) + expect(mgr.getHistory(id)).toBe('previous run output') + }) + + it('drops both the buffer and the file on forgetHistory so a closed tab does not resurrect', () => { + vi.mocked(loadTerminalHistory).mockReturnValue('previous run output') + const mgr = new PtyManager() + const id = 'shell-restored-4' + expect(mgr.getHistory(id)).toBe('previous run output') + vi.mocked(loadTerminalHistory).mockReturnValue(null) + mgr.forgetHistory(id) + expect(mgr.getHistory(id)).toBe('') + }) +}) diff --git a/src/main/pty-manager.ts b/src/main/pty-manager.ts index 2bd9e4d6..b316f11e 100644 --- a/src/main/pty-manager.ts +++ b/src/main/pty-manager.ts @@ -85,8 +85,8 @@ export class PtyManager { private store: Store | null = null private sendSignal: ((channel: string, ...args: unknown[]) => void) | null = null // Per-terminal raw-byte scrollback owned by main. Populated from disk on - // create() (if a history file exists), appended to from the PTY onData - // stream, and returned on getHistory(id). Persistence to + // first touch by either getHistory(id) or create() (whichever comes first), + // appended to from the PTY onData stream. Persistence to // userData/terminal-history/ happens on a throttled cadence and on // before-quit via flushAllHistory(). private history = new Map() @@ -209,20 +209,15 @@ export class PtyManager { } // Seed the history buffer from disk if a file exists. Renderer calls - // getHistory(id) right after createTerminal and writes the bytes into a - // fresh xterm instance before wiring up live data. - let buf = this.history.get(id) - if (!buf) { - buf = new HistoryBuffer() - const existing = loadTerminalHistory(id) - if (existing) buf.seed(existing) - this.history.set(id, buf) - } + // getHistory(id) right before createTerminal and writes the bytes into a + // fresh xterm instance before wiring up live data, so this usually finds + // the buffer already loaded and skips the read. + const buf = this.ensureHistoryBuffer(id) ptyProcess.onData((data: string) => { // Tee into the history ring buffer before forwarding, so a reload // right after output arrives still sees it. - buf!.append(data) + buf.append(data) this.historyDirty.add(id) this.ensureHistoryFlushTimer() this.perfMonitor?.recordTerminalBytes(id, data.length) @@ -258,9 +253,25 @@ export class PtyManager { } } - /** Raw PTY scrollback for `id`, or empty string if none. */ + /** Raw PTY scrollback for `id`, or empty string if none. Falls back to the + * persisted file when nothing is in memory yet: on a cold app start the + * renderer asks for history BEFORE it spawns, so reading only the in-memory + * map returned '' and the saved scrollback stayed invisible until a renderer + * reload happened to hit the buffer create() had since seeded. */ getHistory(id: string): string { - return this.history.get(id)?.toString() || '' + return this.ensureHistoryBuffer(id).toString() + } + + /** The buffer for `id`, seeded from disk on first touch. Both getHistory and + * create go through here so the file is read at most once per id. */ + private ensureHistoryBuffer(id: string): HistoryBuffer { + const existing = this.history.get(id) + if (existing) return existing + const buf = new HistoryBuffer() + const persisted = loadTerminalHistory(id) + if (persisted) buf.seed(persisted) + this.history.set(id, buf) + return buf } /** Drop the in-memory buffer + delete the persisted file. Called on tab diff --git a/src/renderer/components/WorkspaceView.tsx b/src/renderer/components/WorkspaceView.tsx index d87098c2..187c7f3b 100644 --- a/src/renderer/components/WorkspaceView.tsx +++ b/src/renderer/components/WorkspaceView.tsx @@ -634,7 +634,6 @@ export function WorkspaceView({ initialPrompt={tab.initialPrompt} teleportSessionId={tab.teleportSessionId} modelOverride={tab.type === 'agent' ? tab.model : undefined} - shellCommand={tab.type === 'shell' ? tab.command : undefined} shellCwd={tab.type === 'shell' ? tab.cwd : undefined} onRestartAgent={ tab.type === 'agent' diff --git a/src/renderer/components/XTerminal.tsx b/src/renderer/components/XTerminal.tsx index 45950ccb..c5bbd18b 100644 --- a/src/renderer/components/XTerminal.tsx +++ b/src/renderer/components/XTerminal.tsx @@ -267,9 +267,6 @@ interface XTerminalProps { initialPrompt?: string teleportSessionId?: string modelOverride?: string - /** Shell tabs only: when set, spawn ` -ilc ` instead - * of an interactive login shell. Used for agent-spawned shells. */ - shellCommand?: string /** Shell tabs only: directory to spawn in. Relative paths resolve against * `cwd` (the worktree root); absolute paths are used as-is. */ shellCwd?: string @@ -292,7 +289,7 @@ interface XTerminalProps { onSwitchToChat?: () => void } -export function XTerminal({ terminalId, cwd, type, agentKind, visible, sessionName, sessionId, initialPrompt, teleportSessionId, modelOverride, shellCommand, shellCwd, backgroundVar, preamble, hideRestoreNotice, onRestartAgent, onSwitchToChat }: XTerminalProps): JSX.Element { +export function XTerminal({ terminalId, cwd, type, agentKind, visible, sessionName, sessionId, initialPrompt, teleportSessionId, modelOverride, shellCwd, backgroundVar, preamble, hideRestoreNotice, onRestartAgent, onSwitchToChat }: XTerminalProps): JSX.Element { // Lazy font-cache init — fires once on first XTerminal mount. See // initFontCache() comment for why this is lazy rather than at module // top. @@ -634,12 +631,12 @@ export function XTerminal({ terminalId, cwd, type, agentKind, visible, sessionNa const shell = '' const agentArg = type === 'agent' ? await buildAgentArg() : '' if (disposed) return - const args = - type === 'agent' - ? ['-ilc', agentArg] - : shellCommand - ? ['-ilc', shellCommand] - : ['-il'] + // Shell tabs always come up as a plain interactive shell, even when the + // tab carries a `command`. Executing it belongs to the create_shell + // path in main, which spawns eagerly at creation time; doing it here too + // would re-run the command every time the PTY is gone but the tab isn't + // — i.e. on every app restart, and after the command's shell exits. + const args = type === 'agent' ? ['-ilc', agentArg] : ['-il'] const spawnCwd = shellCwd ? shellCwd.startsWith('/') ? shellCwd @@ -702,11 +699,11 @@ export function XTerminal({ terminalId, cwd, type, agentKind, visible, sessionNa spawnPty() return } - // A non-empty history means main already has a live PTY for this id - // — the agent isn't "starting," we're attaching to a running one. - // Clear the loading overlay so the restored scrollback is visible - // without waiting for new bytes (which may never come if the agent - // is idle at its prompt). + // A non-empty history means this tab has run before — either main + // still holds a live PTY for it (we're attaching to a running one) or + // the scrollback came off disk from a previous app run. Clear the + // loading overlay so it's visible without waiting for new bytes + // (which may never come if the agent is idle at its prompt). setLoading(false) // Replay raw scrollback. Wait for xterm to finish parsing before // attaching onData, otherwise any response sequences xterm generates diff --git a/src/shared/state/terminals.ts b/src/shared/state/terminals.ts index b308ee6f..84b3d5c7 100644 --- a/src/shared/state/terminals.ts +++ b/src/shared/state/terminals.ts @@ -58,8 +58,10 @@ export interface TerminalTab { teleportSessionId?: string /** For browser tabs: the URL currently loaded (restored on reload). */ url?: string - /** For shell tabs: command to run via `zsh -ilc ` instead of - * spawning an interactive login shell. Set by agents via the shell MCP. */ + /** For shell tabs: the command the tab was created with (agents set this via + * the shell MCP). A record of origin, not an instruction — it runs once, in + * `createShell`'s eager spawn. Remounting the tab (app restart, or after the + * command's shell exited) gives a plain interactive shell instead. */ command?: string /** For shell tabs: directory to run in. Relative paths resolve against the * worktree root; absolute paths are used as-is. */