diff --git a/apps/server/src/project/ProjectSetupScriptRunner.test.ts b/apps/server/src/project/ProjectSetupScriptRunner.test.ts index 864b88f2d4..35f961f2be 100644 --- a/apps/server/src/project/ProjectSetupScriptRunner.test.ts +++ b/apps/server/src/project/ProjectSetupScriptRunner.test.ts @@ -58,7 +58,7 @@ const makeProjectionSnapshotQueryLayer = (project: OrchestrationProject) => }); type TerminalOverrides = Pick & - Partial>; + Partial>; const makeTerminalManagerLayer = (overrides: TerminalOverrides) => Layer.succeed(TerminalManager.TerminalManager, { @@ -67,6 +67,7 @@ const makeTerminalManagerLayer = (overrides: TerminalOverrides) => clear: () => Effect.void, restart: () => Effect.die(new Error("unused")), close: () => Effect.void, + closeCompletedSetup: () => Effect.void, subscribe: () => Effect.succeed(() => undefined), subscribeMetadata: () => Effect.succeed(() => undefined), ...overrides, @@ -262,6 +263,7 @@ describe("ProjectSetupScriptRunner", () => { listener = null; }); }); + const closeCompletedSetup = vi.fn(() => Effect.void); const project = makeProject([ { id: "setup", @@ -329,14 +331,86 @@ describe("ProjectSetupScriptRunner", () => { ]); // The subscription is torn down once the sentinel arrives. expect(listener).toBeNull(); + // A failed run keeps its shell open for a look. + expect(closeCompletedSetup).not.toHaveBeenCalled(); }).pipe( - Effect.provide(testLayer(project, { open, write, subscribe })), + Effect.provide(testLayer(project, { open, write, subscribe, closeCompletedSetup })), Effect.provideService(HostProcessPlatform, "linux"), Effect.provideService(HostProcessEnvironment, { SHELL: "/bin/zsh" }), ); }, ); + it.effect("closes the idle setup shell after a clean exit", () => { + const open = vi.fn(() => + Effect.succeed({ + threadId: "thread-1", + terminalId: "setup-setup", + cwd: "/repo/worktrees/a", + worktreePath: "/repo/worktrees/a", + status: "running" as const, + pid: 123, + history: "", + exitCode: null, + exitSignal: null, + label: "setup-setup", + updatedAt: "2026-01-01T00:00:00.000Z", + }), + ); + let written = ""; + const write = vi.fn((input: { data: string }) => + Effect.sync(() => void (written = input.data)), + ); + let listener: ((event: TerminalEvent) => Effect.Effect) | null = null; + const subscribe = vi.fn((next: (event: TerminalEvent) => Effect.Effect) => { + listener = next; + return Effect.succeed(() => { + listener = null; + }); + }); + const closeCompletedSetup = vi.fn(() => Effect.void); + const project = makeProject([ + { + id: "setup", + name: "Setup", + command: "bun install", + icon: "configure", + runOnWorktreeCreate: true, + }, + ]); + + return Effect.gen(function* () { + const runner = yield* ProjectSetupScriptRunner.ProjectSetupScriptRunner; + const result = yield* runner.runForThread({ + threadId: "thread-1", + projectCwd: "/repo/project", + worktreePath: "/repo/worktrees/a", + observeCompletion: {}, + }); + if (result.status !== "started" || !result.completion) { + return yield* Effect.die("expected an observed setup run"); + } + const sentinel = /__T3_SETUP_DONE___[0-9a-f]{32}:/.exec(written)?.[0]; + yield* listener!({ + threadId: "thread-1", + terminalId: "setup-setup", + type: "output", + data: `${sentinel}0\r\n`, + }); + + expect((yield* result.completion).exitCode).toBe(0); + expect(closeCompletedSetup).toHaveBeenCalledWith({ + threadId: "thread-1", + terminalId: "setup-setup", + expectedInputCount: 1, + }); + }).pipe( + Effect.provide(testLayer(project, { open, write, subscribe, closeCompletedSetup })), + Effect.provideService(HostProcessPlatform, "linux"), + Effect.provideService(HostProcessEnvironment, { SHELL: "/bin/zsh" }), + ); + }); + it.effect("unsubscribes from terminal output when the command cannot be written", () => { const open = vi.fn(() => Effect.succeed({ diff --git a/apps/server/src/project/ProjectSetupScriptRunner.ts b/apps/server/src/project/ProjectSetupScriptRunner.ts index 16cbfaa594..a287c09dce 100644 --- a/apps/server/src/project/ProjectSetupScriptRunner.ts +++ b/apps/server/src/project/ProjectSetupScriptRunner.ts @@ -36,6 +36,7 @@ export interface ProjectSetupScriptRunnerResultStarted { * Resolves when the script's shell prints the completion sentinel. The * exit code is null when the terminal exited or was closed before the * sentinel arrived. Only present when `observeCompletion` was requested. + * An exit code of 0 closes the setup shell if it has nothing left running. */ readonly completion?: Effect.Effect; } @@ -412,6 +413,20 @@ export const make = Effect.gen(function* () { Effect.tapError(() => Effect.sync(() => observed?.unsubscribe())), ); + // A clean run leaves only an idle prompt behind; its output stays in the + // terminal history. A failed run keeps its shell open for a look. + const completion = observed?.completion.pipe( + Effect.tap(({ exitCode }) => + exitCode === 0 + ? terminalManager.closeCompletedSetup({ + threadId: input.threadId, + terminalId, + expectedInputCount: 1, + }) + : Effect.void, + ), + ); + return { status: "started", scriptId: script.id, @@ -420,7 +435,7 @@ export const make = Effect.gen(function* () { terminalId, cwd, async: script.async !== false, - ...(observed ? { completion: observed.completion } : {}), + ...(completion ? { completion } : {}), } as const; }); diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index 0d10d7d5a0..5319bb2c39 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -1084,9 +1084,13 @@ it.layer( const runCalls: Array<{ command: string; args: ReadonlyArray }> = []; // FakePtyAdapter assigns pids starting at 9000, so the two terminals // opened below run as pids 9000 and 9001. - const psStdout = [" 100 9000 vim", " 101 100 git", " 200 9001 /usr/bin/python3"].join( - "\n", - ); + const psStdout = [ + " 9000 1 zsh", + " 9001 1 zsh", + " 100 9000 vim", + " 101 100 git", + " 200 9001 /usr/bin/python3", + ].join("\n"); const processRunner: ProcessRunner.ProcessRunner["Service"] = { run: (input) => Effect.sync(() => { @@ -1153,7 +1157,7 @@ it.layer( Effect.sync(() => { if (failSnapshots) failedCalls += 1; return { - stdout: failSnapshots ? "" : " 100 9000 vim", + stdout: failSnapshots ? "" : " 9000 1 zsh\n 100 9000 vim", stderr: "", code: ChildProcessSpawner.ExitCode(failSnapshots ? 1 : 0), timedOut: false, @@ -1214,8 +1218,8 @@ it.layer( return { stdout: platform === "win32" - ? "100|9000|vim.exe\n101|9001|ping.exe" - : "100 9000 vim\n101 9001 ping", + ? "9000|1|pwsh.exe\n9001|1|pwsh.exe\n100|9000|vim.exe\n101|9001|ping.exe" + : "9000 1 zsh\n9001 1 zsh\n100 9000 vim\n101 9001 ping", stderr: "", code: ChildProcessSpawner.ExitCode(0), timedOut: false, @@ -1237,6 +1241,8 @@ it.layer( reason: "test sidecar unavailable", }); return [ + { pid: 9000, ppid: 1, name: "zsh" }, + { pid: 9001, ppid: 1, name: "zsh" }, { pid: 100, ppid: 9000, name: "vim" }, { pid: 101, ppid: 9001, name: "ping" }, ]; @@ -1273,6 +1279,172 @@ it.layer( }).pipe(Effect.provide(TestClock.layer())), ); + it.effect("closes a completed setup shell but keeps shells with child processes", () => + Effect.gen(function* () { + // FakePtyAdapter assigns pids from 9000 in open order. + const { manager, ptyAdapter } = yield* createManager(5, { + shellResolver: () => "/bin/zsh", + processTable: Effect.succeed([ + { pid: 9000, ppid: 1, name: "zsh" }, + { pid: 9001, ppid: 1, name: "zsh" }, + // A same-name child may be running a builtin, so keep it. + { pid: 100, ppid: 9001, name: "zsh" }, + { pid: 9002, ppid: 1, name: "zsh" }, + { pid: 300, ppid: 9002, name: "node" }, + { pid: 9003, ppid: 1, name: "zsh" }, + ]), + }).pipe(Effect.provide(withHostPlatform("linux"))); + yield* manager.open(openInput({ terminalId: "idle" })); + yield* manager.open(openInput({ terminalId: "builtin-subshell" })); + yield* manager.open(openInput({ terminalId: "dev-server" })); + yield* manager.open(openInput({ threadId: "thread-2" })); + yield* manager.write({ threadId: "thread-1", terminalId: "idle", data: "setup\r" }); + + yield* manager.closeCompletedSetup({ + threadId: "thread-1", + terminalId: "idle", + expectedInputCount: 1, + }); + yield* manager.write({ + threadId: "thread-1", + terminalId: "builtin-subshell", + data: "setup\r", + }); + yield* manager.closeCompletedSetup({ + threadId: "thread-1", + terminalId: "builtin-subshell", + expectedInputCount: 1, + }); + yield* manager.write({ threadId: "thread-1", terminalId: "dev-server", data: "setup\r" }); + yield* manager.closeCompletedSetup({ + threadId: "thread-1", + terminalId: "dev-server", + expectedInputCount: 1, + }); + + expect(ptyAdapter.processes.map((process) => process.killed)).toEqual([ + true, + false, + false, + false, + ]); + }), + ); + + it.effect("keeps a command that replaced its shell at the PTY PID", () => + Effect.gen(function* () { + const { manager, ptyAdapter } = yield* createManager(5, { + shellResolver: () => "/bin/zsh", + processTable: Effect.succeed([ + { pid: 9000, ppid: 1, name: "sleep" }, + // A missing PID cannot prove that a shell is idle. + { pid: 9002, ppid: 1, name: "zsh" }, + { pid: 9003, ppid: 1, name: "zsh" }, + ]), + }).pipe(Effect.provide(withHostPlatform("linux"))); + yield* manager.open(openInput({ terminalId: "exec-command" })); + yield* manager.open(openInput({ terminalId: "unknown-process" })); + yield* manager.open(openInput({ terminalId: "idle" })); + yield* manager.open(openInput({ terminalId: "reused" })); + for (const terminalId of ["exec-command", "unknown-process", "idle"]) { + yield* manager.write({ threadId: "thread-1", terminalId, data: "setup\r" }); + yield* manager.closeCompletedSetup({ + threadId: "thread-1", + terminalId, + expectedInputCount: 1, + }); + } + yield* manager.write({ threadId: "thread-1", terminalId: "reused", data: "setup\r" }); + yield* manager.write({ threadId: "thread-1", terminalId: "reused", data: "read\r" }); + yield* manager.closeCompletedSetup({ + threadId: "thread-1", + terminalId: "reused", + expectedInputCount: 1, + }); + + expect(ptyAdapter.processes.map((process) => process.killed)).toEqual([ + false, + false, + true, + false, + ]); + }), + ); + + it.effect("keeps a terminal that emits output during completion inspection", () => + Effect.gen(function* () { + const ptyAdapter = new FakePtyAdapter(); + let duringCheck: (pid: number) => Effect.Effect = () => Effect.void; + const { manager, getEvents } = yield* createManager(5, { + ptyAdapter, + subprocessPollIntervalMs: 60_000, + subprocessInspector: (pid) => + duringCheck(pid).pipe( + Effect.as({ hasRunningSubprocess: false, childCommand: null, processIds: [] }), + ), + }); + yield* manager.open(openInput({ terminalId: "echoed" })); + yield* manager.write({ threadId: "thread-1", terminalId: "echoed", data: "setup\r" }); + const [echoed] = ptyAdapter.processes; + duringCheck = () => + Effect.gen(function* () { + echoed!.emitData("make build\r\n"); + yield* waitFor( + Effect.map(getEvents, (events) => events.some((event) => event.type === "output")), + ); + }).pipe(Effect.orDie); + + yield* manager.closeCompletedSetup({ + threadId: "thread-1", + terminalId: "echoed", + expectedInputCount: 1, + }); + + expect(echoed?.killed).toBe(false); + }), + ); + + it.effect("rejects input queued while a completed setup shell closes", () => + Effect.gen(function* () { + const inspectionStarted = yield* Deferred.make(); + const releaseInspection = yield* Deferred.make(); + const { manager, ptyAdapter } = yield* createManager(5, { + subprocessPollIntervalMs: 60_000, + subprocessInspector: () => + Deferred.succeed(inspectionStarted, undefined).pipe( + Effect.andThen(Deferred.await(releaseInspection)), + Effect.as({ hasRunningSubprocess: false, childCommand: null, processIds: [] }), + ), + }); + yield* manager.open(openInput({ terminalId: "setup" })); + yield* manager.write({ threadId: "thread-1", terminalId: "setup", data: "setup\r" }); + const closeFiber = yield* manager + .closeCompletedSetup({ threadId: "thread-1", terminalId: "setup", expectedInputCount: 1 }) + .pipe(Effect.forkScoped); + yield* Deferred.await(inspectionStarted); + const writeStarted = yield* Deferred.make(); + const writeFiber = yield* Deferred.succeed(writeStarted, undefined).pipe( + Effect.andThen( + manager.write({ threadId: "thread-1", terminalId: "setup", data: "read\r" }), + ), + Effect.forkScoped, + ); + yield* Deferred.await(writeStarted); + yield* Effect.yieldNow; + expect(ptyAdapter.processes[0]?.writes).toEqual(["setup\r"]); + yield* Deferred.succeed(releaseInspection, undefined); + yield* Fiber.join(closeFiber); + const writeError = yield* Effect.flip(Fiber.join(writeFiber)); + expect(writeError).toMatchObject({ + _tag: "TerminalSessionLookupError", + threadId: "thread-1", + terminalId: "setup", + }); + expect(ptyAdapter.processes[0]?.writes).toEqual(["setup\r"]); + expect(ptyAdapter.processes[0]?.killed).toBe(true); + }), + ); + it.effect("caps persisted history to configured line limit", () => Effect.gen(function* () { const { manager, ptyAdapter } = yield* createManager(3); diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index 6a7ddf3327..1b7cb3b238 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -197,6 +197,15 @@ export class TerminalManager extends Context.Service< */ readonly close: (input: TerminalCloseInput) => Effect.Effect; + /** Close a setup shell after its caller observed successful completion. + * The expected write count excludes terminals reused or written by a user. + */ + readonly closeCompletedSetup: (input: { + readonly threadId: string; + readonly terminalId: string; + readonly expectedInputCount: number; + }) => Effect.Effect; + /** * Subscribe to terminal runtime events with a direct callback. * @@ -226,6 +235,7 @@ interface TerminalSubprocessInspectResult { interface TerminalSubprocessInspector { ( terminalPid: number, + launchedShellName?: string | null, ): Effect.Effect; } @@ -274,6 +284,10 @@ interface TerminalSessionState { exitSignal: number | null; updatedAt: string; eventSequence: number; + /** Counts writes, so closeCompletedSetup can see input that has not echoed yet. */ + inputCount: number; + /** Executable originally spawned at the PTY PID; `exec` can replace it. */ + launchedShellName: string | null; cols: number; rows: number; process: PtyAdapter.PtyProcess | null; @@ -687,7 +701,26 @@ function deriveSubprocessInspectResult( snapshot: TerminalProcessTableSnapshot, terminalPid: number, platform: NodeJS.Platform, + launchedShellName?: string | null, ): TerminalSubprocessInspectResult { + const commandName = (pid: number) => + normalizeChildCommandName(snapshot.commandById.get(pid) ?? "", platform); + const shellName = commandName(terminalPid); + // A shell can `exec` a command at its own PID, with no child to discover. + // Missing or unrecognized PID identity is also not proof of an idle shell. + if ( + launchedShellName !== undefined && + (shellName === null || + launchedShellName === null || + shellName.toLowerCase() !== launchedShellName.toLowerCase()) + ) { + return { + hasRunningSubprocess: true, + childCommand: shellName ? truncateTerminalWireLabel(shellName) : null, + processIds: [terminalPid], + }; + } + // Even a same-name shell child may be running an in-process builtin. const childPid = (snapshot.childrenByParent.get(terminalPid) ?? [])[0]; if (childPid === undefined) { return { hasRunningSubprocess: false, childCommand: null, processIds: [] }; @@ -703,7 +736,7 @@ function deriveSubprocessInspectResult( pending.push(pid); } } - const normalized = normalizeChildCommandName(snapshot.commandById.get(childPid) ?? "", platform); + const normalized = commandName(childPid); return { hasRunningSubprocess: true, childCommand: normalized ? truncateTerminalWireLabel(normalized) : null, @@ -1497,8 +1530,10 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func readonly inspector: TerminalSubprocessInspector; readonly snapshotSucceeded: boolean; } => ({ - inspector: (terminalPid) => - Effect.succeed(deriveSubprocessInspectResult(snapshot, terminalPid, platform)), + inspector: (terminalPid, launchedShellName) => + Effect.succeed( + deriveSubprocessInspectResult(snapshot, terminalPid, platform, launchedShellName), + ), snapshotSucceeded, }), ); @@ -2125,7 +2160,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func index = 0, lastError: PtyAdapter.PtySpawnError | null = null, ): Effect.fn.Return< - { process: PtyAdapter.PtyProcess; shellLabel: string }, + { process: PtyAdapter.PtyProcess; shellLabel: string; shellName: string | null }, PtyAdapter.PtySpawnError > { if (index >= shellCandidates.length) { @@ -2162,6 +2197,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func return { process: attempt.success, shellLabel: formatShellCandidate(candidate), + shellName: normalizeChildCommandName(candidate.shell, platform), }; } @@ -2197,6 +2233,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func session.exitSignal = null; session.hasRunningSubprocess = false; session.childCommandLabel = null; + session.launchedShellName = null; session.pendingProcessEvents = []; session.pendingProcessEventIndex = 0; session.processEventDrainRunning = false; @@ -2216,6 +2253,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func const spawnResult = yield* trySpawn(shellCandidates, terminalEnv, session); ptyProcess = spawnResult.process; startedShell = spawnResult.shellLabel; + session.launchedShellName = spawnResult.shellName; const processPid = ptyProcess.pid; const unsubscribeData = ptyProcess.onData((data) => { @@ -2533,6 +2571,8 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func exitSignal: null, updatedAt: yield* nowIso, eventSequence: 0, + inputCount: 0, + launchedShellName: null, cols, rows, process: null, @@ -2856,7 +2896,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func ); }; - const write: TerminalManager["Service"]["write"] = Effect.fn("terminal.write")(function* (input) { + const writeLocked = Effect.fn("terminal.write")(function* (input: TerminalWriteInput) { const terminalId = input.terminalId; const session = yield* requireSession(input.threadId, terminalId); const process = session.process; @@ -2867,6 +2907,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func terminalId, }); } + session.inputCount += 1; yield* Effect.try({ try: () => process.write(input.data), catch: (cause) => @@ -2879,6 +2920,9 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func }); }); + const write: TerminalManager["Service"]["write"] = (input) => + withThreadLock(input.threadId, writeLocked(input)); + const resizeLocked = Effect.fn("terminal.resize")(function* (input: TerminalResizeInput) { const session = yield* getSession(input.threadId, input.terminalId); // ResizeObserver traffic can already be in flight when the UI closes the session. @@ -2948,6 +2992,8 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func exitSignal: null, updatedAt: yield* nowIso, eventSequence: 0, + inputCount: 0, + launchedShellName: null, cols, rows, process: null, @@ -3025,6 +3071,43 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func }), ); + const closeCompletedSetup: TerminalManager["Service"]["closeCompletedSetup"] = (input) => + withThreadLock( + input.threadId, + Effect.gen(function* () { + const candidate = yield* getSession(input.threadId, input.terminalId); + if (Option.isNone(candidate)) return; + const session = candidate.value; + if ( + session.status !== "running" || + !Number.isInteger(session.pid) || + session.pid === null || + session.inputCount !== input.expectedInputCount + ) { + return; + } + // Writes share this thread lock, while PTY output can still arrive + // during inspection. Keep the activity check for that output. + const activityMark = (session: TerminalSessionState) => + session.eventSequence + session.inputCount; + const mark = activityMark(session); + // Inspect now instead of trusting the last poll, so a command started + // since then keeps its terminal. + const { inspector } = yield* acquireSubprocessInspector; + const result = yield* inspector(session.pid, session.launchedShellName); + if (result.hasRunningSubprocess || activityMark(session) !== mark) return; + yield* closeSession(input.threadId, session.terminalId, false); + }), + ).pipe( + // A failed process check cannot prove this setup terminal is safe to close. + Effect.catch((error) => + Effect.logWarning("failed to close completed setup terminal", { + threadId: input.threadId, + error: error.message, + }), + ), + ); + return TerminalManager.of({ open, attachStream, @@ -3033,6 +3116,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func clear, restart, close, + closeCompletedSetup, subscribe, subscribeMetadata, }); diff --git a/docs/user/thread-sidebar.md b/docs/user/thread-sidebar.md index 5d72158159..854bd40f81 100644 --- a/docs/user/thread-sidebar.md +++ b/docs/user/thread-sidebar.md @@ -33,7 +33,9 @@ If the server restarts during setup, the task is marked interrupted so you can r Project setup scripts normally run in the background. Enable **Wait for it to finish before the agent starts** for a script when the agent needs its results before starting. A failed required script prevents the first turn -from starting. +from starting. After a successful waited setup script, Pylon closes its terminal when no command +remains active; its output stays in setup history. Failed scripts and terminals with active commands +stay open. ## Pin and arrange threads