From 984d51ee620e35551bf8ffd0acaff71e977cf2c9 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 30 Jul 2026 15:09:13 -0400 Subject: [PATCH] fix tracked Codex initial prompt delivery --- .../src/main/services/pty/ptyService.test.ts | 166 +++++++++++++++++- .../src/main/services/pty/ptyService.ts | 95 ++++++++-- .../pty-and-sessions.md | 24 ++- 3 files changed, 257 insertions(+), 28 deletions(-) diff --git a/apps/desktop/src/main/services/pty/ptyService.test.ts b/apps/desktop/src/main/services/pty/ptyService.test.ts index 4451ac950..205000766 100644 --- a/apps/desktop/src/main/services/pty/ptyService.test.ts +++ b/apps/desktop/src/main/services/pty/ptyService.test.ts @@ -1402,7 +1402,7 @@ describe("ptyService", () => { } }); - it("does not send Codex initialInput into the update prompt", async () => { + it("preserves Codex initialInput after a readiness timeout without sending it into the update prompt", async () => { vi.useFakeTimers(); try { const { service, mockPty, logger, sessionService } = createHarness(); @@ -1417,6 +1417,7 @@ describe("ptyService", () => { args: ["--no-alt-screen"], startupCommand: "codex --no-alt-screen", initialInput: "please keep going", + initialInputReadyTimeoutMs: 20_000, }); mockPty._emitter.emit("data", [ @@ -1433,12 +1434,8 @@ describe("ptyService", () => { expect.objectContaining({ provider: "codex" }), ); expect(logger.warn).toHaveBeenCalledWith( - "pty.initial_input_skipped_not_ready", - expect.objectContaining({ provider: "codex" }), - ); - expect(logger.warn).toHaveBeenCalledWith( - "pty.initial_input_launch_failed", - expect.objectContaining({ toolType: "codex" }), + "pty.initial_input_retrying_not_ready", + expect.objectContaining({ provider: "codex", timeoutMs: 20_000 }), ); expect(mockPty.kill).not.toHaveBeenCalled(); expect(sessionService.end).not.toHaveBeenCalledWith(expect.objectContaining({ @@ -1449,6 +1446,160 @@ describe("ptyService", () => { } }); + it("delivers preserved Codex initialInput when the composer appears after a readiness timeout", async () => { + vi.useFakeTimers(); + try { + const { service, mockPty, logger } = createHarness(); + + await service.create({ + laneId: "lane-1", + title: "Codex CLI", + cols: 80, + rows: 24, + toolType: "codex", + command: "codex", + args: ["--no-alt-screen"], + startupCommand: "codex --no-alt-screen", + initialInput: "ADE_INITIAL_PROMPT_RETRY_MARKER", + initialInputReadyTimeoutMs: 20_000, + }); + + mockPty._emitter.emit("data", "Starting MCP servers (unityMCP)\n"); + await vi.advanceTimersByTimeAsync(20_100); + expect(mockPty.write).not.toHaveBeenCalled(); + expect(logger.warn).toHaveBeenCalledWith( + "pty.initial_input_retrying_not_ready", + expect.objectContaining({ provider: "codex" }), + ); + + mockPty._emitter.emit( + "data", + "\x1b[2J\x1b[HOpenAI Codex\nmodel: gpt-5.6-terra\nMCP startup incomplete (failed: unityMCP)\n› ", + ); + await vi.advanceTimersByTimeAsync(600); + await vi.advanceTimersByTimeAsync(25); + await vi.advanceTimersByTimeAsync(25); + + expect(mockPty.write).toHaveBeenCalledWith( + "\x1b[200~ADE_INITIAL_PROMPT_RETRY_MARKER\x1b[201~", + ); + } finally { + vi.useRealTimers(); + } + }); + + it("cancels preserved Codex initialInput when the user takes control before late readiness", async () => { + vi.useFakeTimers(); + try { + const { service, mockPty, logger } = createHarness(); + + const created = await service.create({ + laneId: "lane-1", + title: "Codex CLI", + cols: 80, + rows: 24, + toolType: "codex", + command: "codex", + args: ["--no-alt-screen"], + startupCommand: "codex --no-alt-screen", + initialInput: "ADE_STALE_INITIAL_PROMPT", + initialInputReadyTimeoutMs: 20_000, + }); + + mockPty._emitter.emit("data", "Starting MCP servers (unityMCP)\n"); + await vi.advanceTimersByTimeAsync(20_100); + service.write({ ptyId: created.ptyId, data: "user draft" }); + mockPty._emitter.emit( + "data", + "\x1b[2J\x1b[HOpenAI Codex\nmodel: gpt-5.6-terra\nMCP startup incomplete (failed: unityMCP)\n› user draft", + ); + await vi.advanceTimersByTimeAsync(700); + + expect(mockPty.write).toHaveBeenCalledTimes(1); + expect(mockPty.write).toHaveBeenCalledWith("user draft"); + expect(mockPty.write).not.toHaveBeenCalledWith(expect.stringContaining("ADE_STALE_INITIAL_PROMPT")); + expect(logger.info).toHaveBeenCalledWith( + "pty.initial_input_cancelled_user_takeover", + expect.objectContaining({ provider: "codex" }), + ); + } finally { + vi.useRealTimers(); + } + }); + + it("cancels Codex initialInput when the user takes control during the initial delay", async () => { + vi.useFakeTimers(); + try { + const { service, mockPty, logger } = createHarness(); + + const created = await service.create({ + laneId: "lane-1", + title: "Codex CLI", + cols: 80, + rows: 24, + toolType: "codex", + command: "codex", + args: ["--no-alt-screen"], + startupCommand: "codex --no-alt-screen", + initialInput: "ADE_DELAYED_STALE_PROMPT", + initialInputDelayMs: 750, + }); + + service.write({ ptyId: created.ptyId, data: "user draft" }); + mockPty._emitter.emit( + "data", + "\x1b[2J\x1b[HOpenAI Codex\nmodel: gpt-5.6-terra\nMCP startup incomplete (failed: unityMCP)\n› user draft", + ); + await vi.advanceTimersByTimeAsync(1_500); + + expect(mockPty.write).toHaveBeenCalledTimes(1); + expect(mockPty.write).toHaveBeenCalledWith("user draft"); + expect(mockPty.write).not.toHaveBeenCalledWith(expect.stringContaining("ADE_DELAYED_STALE_PROMPT")); + expect(logger.info).toHaveBeenCalledWith( + "pty.initial_input_cancelled_user_takeover", + expect.objectContaining({ provider: "codex" }), + ); + } finally { + vi.useRealTimers(); + } + }); + + it("accepts a stable Codex composer while unrelated PTY redraws continue after failed MCP startup", async () => { + vi.useFakeTimers(); + try { + const { service, mockPty } = createHarness(); + + await service.create({ + laneId: "lane-1", + title: "Codex CLI", + cols: 80, + rows: 24, + toolType: "codex", + command: "codex", + args: ["--no-alt-screen"], + startupCommand: "codex --no-alt-screen", + initialInput: "ADE_STABLE_COMPOSER_MARKER", + }); + + mockPty._emitter.emit( + "data", + "\x1b[2J\x1b[HOpenAI Codex\nmodel: gpt-5.6-terra\nMCP startup incomplete (failed: unityMCP)\n› ", + ); + for (let elapsed = 0; elapsed < 700; elapsed += 100) { + mockPty._emitter.emit("data", `\x1b]0;Codex startup ${elapsed}\x07`); + await vi.advanceTimersByTimeAsync(100); + } + await vi.advanceTimersByTimeAsync(25); + await vi.advanceTimersByTimeAsync(25); + + expect(mockPty.write).toHaveBeenCalledWith( + "\x1b[200~ADE_STABLE_COMPOSER_MARKER\x1b[201~", + ); + } finally { + vi.useRealTimers(); + } + }); + it("moves node_modules bins behind user paths for Codex CLI launches", async () => { const previousPath = process.env.PATH; process.env.PATH = [ @@ -1556,6 +1707,7 @@ describe("ptyService", () => { startupCommand: "codex --no-alt-screen", initialInput: "please keep going", awaitInitialInput: true, + initialInputReadyTimeoutMs: 20_000, }).then( () => null, (error: unknown) => error, diff --git a/apps/desktop/src/main/services/pty/ptyService.ts b/apps/desktop/src/main/services/pty/ptyService.ts index 96fa6ef80..e541840a5 100644 --- a/apps/desktop/src/main/services/pty/ptyService.ts +++ b/apps/desktop/src/main/services/pty/ptyService.ts @@ -161,6 +161,7 @@ const AGENT_CLI_SUBMIT_DELAY_MS = 25; const CODEX_CLI_PASTE_SUBMIT_DELAY_MS = 180; const CURSOR_CLI_PASTE_SUBMIT_DELAY_MS = 500; const AGENT_CLI_READY_TIMEOUT_MS = 20_000; +const CODEX_CLI_READY_TIMEOUT_MS = 60_000; const AGENT_CLI_READY_POLL_MS = 100; const AGENT_CLI_READY_QUIET_MS = 600; const PTY_PROCESS_TREE_KILL_DELAY_MS = 1500; @@ -611,6 +612,8 @@ type PtyEntry = { processOutputData: ((data: string) => void) | null; /** Epoch ms of the last user write; shortens the data batch window. */ lastUserInputAt: number; + /** Monotonic generation used to detect user takeover of deferred input. */ + userInputGeneration: number; terminalSnapshot: TerminalSnapshotMirror | null; recentOutputTail: string; runtimeWindowTitleScanBuffer: string; @@ -3778,21 +3781,37 @@ export function createPtyService({ timeoutMs = AGENT_CLI_READY_TIMEOUT_MS, ): Promise => { const deadline = Date.now() + timeoutMs; + let stableReadyText = ""; + let stableReadySince = 0; while (Date.now() < deadline) { - if (agentCliInputReadyNow(sessionId, provider)) return true; - if (!liveEntryBySessionId(sessionId)) return false; + const readiness = agentCliInputReadiness(sessionId, provider); + if (!readiness) return false; + if (readiness.readyNow) return true; + if (provider === "codex" && readiness.markerVisible) { + if (readiness.text === stableReadyText) { + if (stableReadySince > 0 && Date.now() - stableReadySince >= AGENT_CLI_READY_QUIET_MS) { + return true; + } + } else { + stableReadyText = readiness.text; + stableReadySince = Date.now(); + } + } else { + stableReadyText = ""; + stableReadySince = 0; + } await delay(AGENT_CLI_READY_POLL_MS); } logger.warn("pty.agent_cli_ready_wait_timeout", { sessionId, provider, timeoutMs }); return false; }; - const agentCliInputReadyNow = ( + const agentCliInputReadiness = ( sessionId: string, provider: TerminalResumeProvider, - ): boolean => { + ): { markerVisible: boolean; readyNow: boolean; text: string } | null => { const live = liveEntryBySessionId(sessionId); - if (!live || live[1].disposed) return false; + if (!live || live[1].disposed) return null; const entry = live[1]; const outputTail = stripAnsi(entry.recentOutputTail).replace(/\r/g, "\n"); const visibleText = entry.terminalSnapshot @@ -3803,8 +3822,19 @@ export function createPtyService({ const readinessText = visibleText.trim().length > 0 ? visibleText : outputTail; const runtime = runtimeStates.get(sessionId); const quietForMs = runtime ? Date.now() - runtime.lastActivityAt : 0; - return providerReadyMarkerVisible(provider, readinessText) - && quietForMs >= AGENT_CLI_READY_QUIET_MS; + const markerVisible = providerReadyMarkerVisible(provider, readinessText); + return { + markerVisible, + readyNow: markerVisible && quietForMs >= AGENT_CLI_READY_QUIET_MS, + text: readinessText, + }; + }; + + const agentCliInputReadyNow = ( + sessionId: string, + provider: TerminalResumeProvider, + ): boolean => { + return agentCliInputReadiness(sessionId, provider)?.readyNow ?? false; }; const writeAgentCliInput = async ( @@ -4101,6 +4131,7 @@ export function createPtyService({ const markPtyUserInput = (entry: PtyEntry): void => { entry.lastUserInputAt = Date.now(); + entry.userInputGeneration += 1; if (entry.tracked && isTrackedAgentCliToolType(entry.toolTypeHint)) { clearTrackedCliTurnStartMarkers(entry.sessionId); entry.attentionRequested = false; @@ -4606,6 +4637,7 @@ export function createPtyService({ pendingOutputHighSurrogate: "", processOutputData: null, lastUserInputAt: 0, + userInputGeneration: 0, terminalSnapshot: tracked ? createTerminalSnapshotMirror(cols, rows) : null, recentOutputTail: "", runtimeWindowTitleScanBuffer: "", @@ -4732,9 +4764,13 @@ export function createPtyService({ if (requestedInitialInput.length > 0) { const normalizedInitialInput = requestedInitialInput.replace(/\r\n?/g, "\n"); + const provider = providerFromTool(toolTypeHint); + const defaultInitialInputReadyTimeoutMs = provider === "codex" + ? CODEX_CLI_READY_TIMEOUT_MS + : AGENT_CLI_READY_TIMEOUT_MS; const requestedInitialInputReadyTimeoutMs = args.initialInputReadyTimeoutMs; const parsedInitialInputReadyTimeoutMs = Math.floor( - Number(requestedInitialInputReadyTimeoutMs ?? AGENT_CLI_READY_TIMEOUT_MS) || 0, + Number(requestedInitialInputReadyTimeoutMs ?? defaultInitialInputReadyTimeoutMs) || 0, ); const initialInputReadyTimeoutMs = Math.max( AGENT_CLI_READY_TIMEOUT_MS, @@ -4756,24 +4792,57 @@ export function createPtyService({ maxTimeoutMs: 300_000, }); } + const initialInputUserGeneration = entry.userInputGeneration; const writeInitialInput = async (): Promise => { entry.initialInputTimer = null; if (entry.disposed) throw new Error("Terminal session closed before initial input could be sent."); - const provider = providerFromTool(toolTypeHint); + const userTookControl = (): boolean => entry.userInputGeneration !== initialInputUserGeneration; try { if (provider) { - const ready = await waitForAgentCliInputReady(sessionId, provider, initialInputReadyTimeoutMs); - if (!ready) { - logger.warn("pty.initial_input_skipped_not_ready", { + while (!await waitForAgentCliInputReady(sessionId, provider, initialInputReadyTimeoutMs)) { + if (entry.disposed || !liveEntryBySessionId(sessionId)) { + throw new Error("Terminal session closed before initial input could be sent."); + } + if (userTookControl()) { + logger.info("pty.initial_input_cancelled_user_takeover", { + ptyId, + sessionId, + cwd, + toolType: toolTypeHint, + provider, + }); + return; + } + if (args.awaitInitialInput || provider !== "codex") { + logger.warn("pty.initial_input_skipped_not_ready", { + ptyId, + sessionId, + cwd, + toolType: toolTypeHint, + provider, + }); + throw new Error(`${provider} CLI did not become ready; initial input was not sent.`); + } + logger.warn("pty.initial_input_retrying_not_ready", { ptyId, sessionId, cwd, toolType: toolTypeHint, provider, + timeoutMs: initialInputReadyTimeoutMs, }); - throw new Error(`${provider} CLI did not become ready; initial input was not sent.`); } if (entry.disposed) throw new Error("Terminal session closed before initial input could be sent."); + if (userTookControl()) { + logger.info("pty.initial_input_cancelled_user_takeover", { + ptyId, + sessionId, + cwd, + toolType: toolTypeHint, + provider, + }); + return; + } } if (provider) { const submittedInitialInput = normalizedInitialInput.trim(); diff --git a/docs/features/terminals-and-sessions/pty-and-sessions.md b/docs/features/terminals-and-sessions/pty-and-sessions.md index d4b6f701d..e6cf02dd6 100644 --- a/docs/features/terminals-and-sessions/pty-and-sessions.md +++ b/docs/features/terminals-and-sessions/pty-and-sessions.md @@ -159,7 +159,9 @@ Each live PTY has an entry in the `ptys` map keyed by `ptyId` with: rows). Flushed on PTY exit, on resize, and on every `terminal.preview` call. - initial input: `initialInputTimer` — deferred initial-input write for - callers that pass `args.initialInput` with an `initialInputDelayMs` + callers that pass `args.initialInput` with an `initialInputDelayMs`; + `userInputGeneration` advances on every user write so a deferred launch + prompt can be cancelled if the user takes control first - live session resync: `lastSessionResyncCheckAt` — last time the PTY entry re-synced its session row to keep the DB in step with the in-memory state @@ -238,13 +240,19 @@ Each live PTY has an entry in the `ptys` map keyed by `ptyId` with: the older pattern where callers embedded the prompt in the provider argv or typed it as a post-create PTY write. The timer is cleared on `closeEntry` / `dispose` and the callback bails out if the PTY - was disposed in the meantime. When `awaitInitialInput` is false, a - readiness/write failure is logged and the PTY is preserved; ADE no - longer kills or ends the session just because the first input could - not be delivered. When a caller explicitly sets `awaitInitialInput`, - readiness/write failure is treated as startup failure: the process - tree is terminated and the session is ended as `failed`. Returns - `{ ptyId, sessionId, pid }`. + was disposed in the meantime. Readiness requires the provider's visible + composer and 600 ms of stability. Codex gets a 60-second default readiness + deadline (other agent CLIs use 20 seconds), and a stable visible Codex + composer can satisfy the stability check even while unrelated terminal + title/redraw output continues after an MCP startup failure. If a + fire-and-forget Codex launch reaches that deadline, ADE preserves the + initial prompt and repeats the bounded readiness wait instead of dropping + it; the PTY remains open throughout. Any user write during the initial + delay or a retry cancels the preserved prompt so ADE cannot overwrite or + submit behind a user-authored draft. A caller that explicitly sets + `awaitInitialInput` retains fail-fast startup semantics: a readiness/write + failure terminates the process tree and ends the session as `failed`. + Returns `{ ptyId, sessionId, pid }`. The launch env is built layer by layer: `process.env`, the lane runtime env (from `getLaneRuntimeEnv`), the caller's `args.env`, then