Skip to content

Commit 2ec7e8c

Browse files
frenchie4111claude
andauthored
Stop restored shell tabs from re-running their command (#292)
## Summary - A shell tab created via `create_shell` persists the command it was launched with. `XTerminal` treated that field as an instruction: any mount without a live PTY spawned `zsh -ilc <command>`. After a restart there's never a live PTY, so opening a previously-created shell tab silently re-ran whatever the agent had launched (`npm run dev`, a build, anything). Same thing mid-session once the command's shell had exited and the tab remounted. - The renderer now always spawns a plain interactive shell. Execution stays where it belongs — the single eager spawn in `createShell` (`src/main/index.ts`) at creation time. - The persisted `command` stays as a record of origin: still reported by `list_shells`, still the tab's default label. Comments on `TerminalTab.command` / `PersistedTab.command` updated to say so, since the field name reads like an instruction. ## Test plan - [x] `npm run typecheck` - [x] `npx electron-vite build` - [ ] Agent calls `create_shell` with a command → command runs immediately in the new tab (unchanged) - [ ] Quit and reopen Ness → focusing that shell tab gives an idle prompt instead of re-running the command - [ ] Let a command-backed shell exit, switch away and back → idle prompt, no re-run - [ ] Plain user-opened shell tabs (`+` → Shell) behave as before 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent ae5b4ec commit 2ec7e8c

7 files changed

Lines changed: 101 additions & 34 deletions

File tree

src/main/index.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ import { FileContentWatcher } from './file-content-watcher'
5656
import { SnoozeTimer } from './snooze-timer'
5757
import { getWeeklyStats } from './weekly-stats'
5858
import type { TerminalTab, PaneNode, PaneLeaf } from '../shared/state/terminals'
59-
import { getLeaves, mapLeaves } from '../shared/state/terminals'
59+
import { findTabById, getLeaves, mapLeaves } from '../shared/state/terminals'
6060
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'
6161
import { listOpenPRs, getPRByNumber, testToken, starRepo, unstarRepo, isRepoStarred, mergePR, approvePR, getRepoInfo, type GitHubMergeMethod, type MergePRResult, type PRLookupResult } from './github'
6262
import { AVAILABLE_EDITORS, DEFAULT_EDITOR_ID, openInEditor } from './editor'
@@ -267,6 +267,18 @@ function findShellWorktree(shellId: string): string | null {
267267
}
268268
return null
269269
}
270+
271+
/** True for a shell tab that was created with a command (`create_shell`) and
272+
* whose process is gone. That command runs exactly once, in createShell's
273+
* eager spawn; afterwards the tab is a transcript of the run, not a live
274+
* shell — which is also how it behaves when the command exits mid-session.
275+
* The renderer still fires pty:create when it mounts such a tab (it can't
276+
* tell a spent shell from a fresh one), so the spawn is dropped here rather
277+
* than handing back an interactive prompt no command shell would ever show. */
278+
function isSpentCommandShell(id: string): boolean {
279+
const tab = findTabById(store.getSnapshot().state.terminals.panes, id)
280+
return tab?.type === 'shell' && !!tab.command
281+
}
270282
// Resolves the harness version from disk so it works in every runtime
271283
// (Electron dev/packaged, headless dev, headless tarball). Electron's
272284
// `app.getVersion()` would do the job in two of those four, but the
@@ -3507,6 +3519,10 @@ function registerIpcHandlers(): void {
35073519
: agentKind === 'cursor' ? config.cursorEnvVars
35083520
: undefined
35093521
const existed = ptyManager.hasTerminal(id)
3522+
if (!existed && isSpentCommandShell(id)) {
3523+
log('pty', `create id=${id} dropped — command shell already ran`)
3524+
return
3525+
}
35103526
ptyManager.create(id, cwd, cmd, args, extraEnv, !isAgent, cols, rows)
35113527
if (!existed) {
35123528
// Creator becomes controller immediately so their first keystroke

src/main/persistence-migrations.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ export interface PersistedTab {
2424
sessionId?: string
2525
/** For browser tabs: last URL so we can restore the tab on reload. */
2626
url?: string
27-
/** For shell tabs: command passed via `zsh -ilc <command>` (agent-spawned). */
27+
/** For shell tabs: the command the tab was created with (agent-spawned).
28+
* Kept for display only — restored tabs do not re-run it. */
2829
command?: string
2930
/** For shell tabs: cwd (absolute or relative to worktree root). */
3031
cwd?: string

src/main/pty-manager.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ vi.mock('./persistence', () => ({
3636

3737
import { PtyManager } from './pty-manager'
3838
import * as pty from 'node-pty'
39+
import { loadTerminalHistory } from './persistence'
3940

4041
describe('PtyManager.create — eager spawn contract for createShell (#203)', () => {
4142
beforeEach(() => {
@@ -67,3 +68,43 @@ describe('PtyManager.create — eager spawn contract for createShell (#203)', ()
6768
expect(pty.spawn).not.toHaveBeenCalled()
6869
})
6970
})
71+
72+
// On a cold app start the renderer asks for scrollback BEFORE it spawns, so
73+
// getHistory has to reach the persisted file itself — reading only the
74+
// in-memory map left restored tabs blank until something else seeded it.
75+
describe('PtyManager.getHistory — persisted scrollback across an app restart', () => {
76+
beforeEach(() => {
77+
vi.clearAllMocks()
78+
})
79+
80+
it('returns the persisted scrollback when nothing is in memory yet', () => {
81+
vi.mocked(loadTerminalHistory).mockReturnValueOnce('previous run output')
82+
const mgr = new PtyManager()
83+
expect(mgr.getHistory('shell-restored-1')).toBe('previous run output')
84+
})
85+
86+
it('returns empty string when no history was persisted', () => {
87+
const mgr = new PtyManager()
88+
expect(mgr.getHistory('shell-restored-2')).toBe('')
89+
})
90+
91+
it('reads the file once per id — the spawn that follows reuses the buffer', () => {
92+
vi.mocked(loadTerminalHistory).mockReturnValueOnce('previous run output')
93+
const mgr = new PtyManager()
94+
const id = 'shell-restored-3'
95+
mgr.getHistory(id)
96+
mgr.create(id, tmpdir(), '', ['-il'], undefined, true)
97+
expect(loadTerminalHistory).toHaveBeenCalledTimes(1)
98+
expect(mgr.getHistory(id)).toBe('previous run output')
99+
})
100+
101+
it('drops both the buffer and the file on forgetHistory so a closed tab does not resurrect', () => {
102+
vi.mocked(loadTerminalHistory).mockReturnValue('previous run output')
103+
const mgr = new PtyManager()
104+
const id = 'shell-restored-4'
105+
expect(mgr.getHistory(id)).toBe('previous run output')
106+
vi.mocked(loadTerminalHistory).mockReturnValue(null)
107+
mgr.forgetHistory(id)
108+
expect(mgr.getHistory(id)).toBe('')
109+
})
110+
})

src/main/pty-manager.ts

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ export class PtyManager {
8585
private store: Store | null = null
8686
private sendSignal: ((channel: string, ...args: unknown[]) => void) | null = null
8787
// Per-terminal raw-byte scrollback owned by main. Populated from disk on
88-
// create() (if a history file exists), appended to from the PTY onData
89-
// stream, and returned on getHistory(id). Persistence to
88+
// first touch by either getHistory(id) or create() (whichever comes first),
89+
// appended to from the PTY onData stream. Persistence to
9090
// userData/terminal-history/<id> happens on a throttled cadence and on
9191
// before-quit via flushAllHistory().
9292
private history = new Map<string, HistoryBuffer>()
@@ -209,20 +209,15 @@ export class PtyManager {
209209
}
210210

211211
// Seed the history buffer from disk if a file exists. Renderer calls
212-
// getHistory(id) right after createTerminal and writes the bytes into a
213-
// fresh xterm instance before wiring up live data.
214-
let buf = this.history.get(id)
215-
if (!buf) {
216-
buf = new HistoryBuffer()
217-
const existing = loadTerminalHistory(id)
218-
if (existing) buf.seed(existing)
219-
this.history.set(id, buf)
220-
}
212+
// getHistory(id) right before createTerminal and writes the bytes into a
213+
// fresh xterm instance before wiring up live data, so this usually finds
214+
// the buffer already loaded and skips the read.
215+
const buf = this.ensureHistoryBuffer(id)
221216

222217
ptyProcess.onData((data: string) => {
223218
// Tee into the history ring buffer before forwarding, so a reload
224219
// right after output arrives still sees it.
225-
buf!.append(data)
220+
buf.append(data)
226221
this.historyDirty.add(id)
227222
this.ensureHistoryFlushTimer()
228223
this.perfMonitor?.recordTerminalBytes(id, data.length)
@@ -258,9 +253,25 @@ export class PtyManager {
258253
}
259254
}
260255

261-
/** Raw PTY scrollback for `id`, or empty string if none. */
256+
/** Raw PTY scrollback for `id`, or empty string if none. Falls back to the
257+
* persisted file when nothing is in memory yet: on a cold app start the
258+
* renderer asks for history BEFORE it spawns, so reading only the in-memory
259+
* map returned '' and the saved scrollback stayed invisible until a renderer
260+
* reload happened to hit the buffer create() had since seeded. */
262261
getHistory(id: string): string {
263-
return this.history.get(id)?.toString() || ''
262+
return this.ensureHistoryBuffer(id).toString()
263+
}
264+
265+
/** The buffer for `id`, seeded from disk on first touch. Both getHistory and
266+
* create go through here so the file is read at most once per id. */
267+
private ensureHistoryBuffer(id: string): HistoryBuffer {
268+
const existing = this.history.get(id)
269+
if (existing) return existing
270+
const buf = new HistoryBuffer()
271+
const persisted = loadTerminalHistory(id)
272+
if (persisted) buf.seed(persisted)
273+
this.history.set(id, buf)
274+
return buf
264275
}
265276

266277
/** Drop the in-memory buffer + delete the persisted file. Called on tab

src/renderer/components/WorkspaceView.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -634,7 +634,6 @@ export function WorkspaceView({
634634
initialPrompt={tab.initialPrompt}
635635
teleportSessionId={tab.teleportSessionId}
636636
modelOverride={tab.type === 'agent' ? tab.model : undefined}
637-
shellCommand={tab.type === 'shell' ? tab.command : undefined}
638637
shellCwd={tab.type === 'shell' ? tab.cwd : undefined}
639638
onRestartAgent={
640639
tab.type === 'agent'

src/renderer/components/XTerminal.tsx

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -267,9 +267,6 @@ interface XTerminalProps {
267267
initialPrompt?: string
268268
teleportSessionId?: string
269269
modelOverride?: string
270-
/** Shell tabs only: when set, spawn `<user-shell> -ilc <command>` instead
271-
* of an interactive login shell. Used for agent-spawned shells. */
272-
shellCommand?: string
273270
/** Shell tabs only: directory to spawn in. Relative paths resolve against
274271
* `cwd` (the worktree root); absolute paths are used as-is. */
275272
shellCwd?: string
@@ -292,7 +289,7 @@ interface XTerminalProps {
292289
onSwitchToChat?: () => void
293290
}
294291

295-
export function XTerminal({ terminalId, cwd, type, agentKind, visible, sessionName, sessionId, initialPrompt, teleportSessionId, modelOverride, shellCommand, shellCwd, backgroundVar, preamble, hideRestoreNotice, onRestartAgent, onSwitchToChat }: XTerminalProps): JSX.Element {
292+
export function XTerminal({ terminalId, cwd, type, agentKind, visible, sessionName, sessionId, initialPrompt, teleportSessionId, modelOverride, shellCwd, backgroundVar, preamble, hideRestoreNotice, onRestartAgent, onSwitchToChat }: XTerminalProps): JSX.Element {
296293
// Lazy font-cache init — fires once on first XTerminal mount. See
297294
// initFontCache() comment for why this is lazy rather than at module
298295
// top.
@@ -644,12 +641,12 @@ export function XTerminal({ terminalId, cwd, type, agentKind, visible, sessionNa
644641
const shell = ''
645642
const agentArg = type === 'agent' ? await buildAgentArg() : ''
646643
if (disposed) return
647-
const args =
648-
type === 'agent'
649-
? ['-ilc', agentArg]
650-
: shellCommand
651-
? ['-ilc', shellCommand]
652-
: ['-il']
644+
// Shell tabs always come up as a plain interactive shell, even when the
645+
// tab carries a `command`. Executing it belongs to the create_shell
646+
// path in main, which spawns eagerly at creation time; doing it here too
647+
// would re-run the command every time the PTY is gone but the tab isn't
648+
// — i.e. on every app restart, and after the command's shell exits.
649+
const args = type === 'agent' ? ['-ilc', agentArg] : ['-il']
653650
const spawnCwd = shellCwd
654651
? shellCwd.startsWith('/')
655652
? shellCwd
@@ -712,11 +709,11 @@ export function XTerminal({ terminalId, cwd, type, agentKind, visible, sessionNa
712709
spawnPty()
713710
return
714711
}
715-
// A non-empty history means main already has a live PTY for this id
716-
// — the agent isn't "starting," we're attaching to a running one.
717-
// Clear the loading overlay so the restored scrollback is visible
718-
// without waiting for new bytes (which may never come if the agent
719-
// is idle at its prompt).
712+
// A non-empty history means this tab has run before — either main
713+
// still holds a live PTY for it (we're attaching to a running one) or
714+
// the scrollback came off disk from a previous app run. Clear the
715+
// loading overlay so it's visible without waiting for new bytes
716+
// (which may never come if the agent is idle at its prompt).
720717
setLoading(false)
721718
// Replay raw scrollback. Wait for xterm to finish parsing before
722719
// attaching onData, otherwise any response sequences xterm generates

src/shared/state/terminals.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,10 @@ export interface TerminalTab {
5858
teleportSessionId?: string
5959
/** For browser tabs: the URL currently loaded (restored on reload). */
6060
url?: string
61-
/** For shell tabs: command to run via `zsh -ilc <command>` instead of
62-
* spawning an interactive login shell. Set by agents via the shell MCP. */
61+
/** For shell tabs: the command the tab was created with (agents set this via
62+
* the shell MCP). A record of origin, not an instruction — it runs once, in
63+
* `createShell`'s eager spawn. Remounting the tab (app restart, or after the
64+
* command's shell exited) gives a plain interactive shell instead. */
6365
command?: string
6466
/** For shell tabs: directory to run in. Relative paths resolve against the
6567
* worktree root; absolute paths are used as-is. */

0 commit comments

Comments
 (0)