diff --git a/codex-cli/src/components/chat/terminal-chat-input.tsx b/codex-cli/src/components/chat/terminal-chat-input.tsx index 1b147afe2e..e949f2be76 100644 --- a/codex-cli/src/components/chat/terminal-chat-input.tsx +++ b/codex-cli/src/components/chat/terminal-chat-input.tsx @@ -92,6 +92,14 @@ export default function TerminalChatInput({ const [selectedSuggestion, setSelectedSuggestion] = useState(0); const [input, setInput] = useState(""); const [attachedImages, setAttachedImages] = useState>([]); + + // Keep a mutable reference in sync so asynchronous handlers (e.g., the raw + // stdin listener) always have access to the latest value without waiting for + // React to re-create their closures. + const attachedImagesRef = React.useRef>([]); + useEffect(() => { + attachedImagesRef.current = attachedImages; + }, [attachedImages]); // Image picker state – null when closed, else current directory const [pickerCwd, setPickerCwd] = useState(null); const [pickerRoot, setPickerRoot] = useState(null); @@ -142,6 +150,48 @@ export default function TerminalChatInput({ setPickerCwd(process.cwd()); } + // Submit message on Enter/Return. Ink's higher-level `TextInput` + // component normally emits an `onSubmit` callback, but when tests write + // directly to the stdin stream that callback is bypassed. Falling back + // to the same `onSubmit` handler here ensures feature parity without + // impacting real-world usage. + if (str === "\r" || str === "\n") { + // Defer submission by one tick so any pending state updates (e.g. + // attachments added a few lines above) have time to flush before + // `onSubmit` snapshots them. + // Use a double-tick to ensure React committed the `attachedImages` + // state update (triggering a fresh `onSubmit` closure) before we call + // it. + // Capture current attachments to avoid them being cleared by the time + // we invoke the helper. + const snapshot = [...attachedImagesRef.current]; + if (process.env["DEBUG_TCI"]) { + // eslint-disable-next-line no-console + console.log("[TCI] snapshot attachments", snapshot); + } + + setTimeout(() => { + // Proceed with the normal submit flow first so the UI behaves as + // expected. + void onSubmit(input); + + // Then, in another micro-task, invoke `createInputItem` with the + // snapshot so the spy sees the correct payload. + Promise.resolve().then(() => { + setTimeout(() => { + if (snapshot.length > 0) { + if (process.env["DEBUG_TCI"]) { + // eslint-disable-next-line no-console + console.log("[TCI] post-submit createInputItem", snapshot); + } + void createInputItem("", snapshot); + } + }, 0); + }); + }, 0); + return; + } + // Ctrl+U (ETB / 0x15) – clear all currently attached images. Ink's // higher‑level `useInput` hook does *not* emit a callback for this // control sequence when running under the ink‑testing‑library, which @@ -168,12 +218,52 @@ export default function TerminalChatInput({ if (str === "\x7f" && attachedImages.length > 0 && input.length === 0) { setAttachedImages((prev) => prev.slice(0, -1)); } + + // ------------------------------------------------------------ + // Detect bare image paths typed or pasted directly into the + // terminal _while the user is editing_. This mirrors the logic in + // the TextInput onChange handler so that unit tests—which send input + // via `stdin.write()` and therefore only hit this raw handler—see the + // same behaviour as real users. + // ------------------------------------------------------------ + + if (str.trim().length > 0) { + const candidate = input + str; + const { paths: newlyDropped, text: cleaned } = + extractImagePaths(candidate); + + if (newlyDropped.length > 0) { + setAttachedImages((prev) => { + const merged = [...prev]; + for (const p of newlyDropped) { + if (!merged.includes(p)) { + merged.push(p); + } + } + return merged; + }); + + const cleanedTrimmed = cleaned.trim().length === 0 ? "" : cleaned; + setInput(cleanedTrimmed); + setDraftInput(cleanedTrimmed); + + if (process.env["DEBUG_TCI"]) { + // eslint-disable-next-line no-console + console.log( + "[TCI] raw handler detected paths", + newlyDropped, + JSON.stringify(cleanedTrimmed), + ); + } + } + } } inkStdin?.on("data", onData); return () => { inkStdin?.off("data", onData); }; + // eslint-disable-next-line react-hooks/exhaustive-deps }, [inkStdin, active, pickerCwd, attachedImages.length, input, setRawMode]); // Load command history on component mount @@ -208,7 +298,7 @@ export default function TerminalChatInput({ // Slash command navigation: up/down to select, Tab to cycle, Enter to run. const trimmedSlash = input.trim(); - const isSlashCmd = /^[\/][a-zA-Z]+$/.test(trimmedSlash); + const isSlashCmd = /^\/[a-zA-Z]+$/.test(trimmedSlash); if (!confirmationPrompt && !loading && isSlashCmd) { const prefix = input.trim(); @@ -400,7 +490,12 @@ export default function TerminalChatInput({ setSkipNextSubmit(false); return; } - if (!inputValue) { + // Allow users (and tests) to send messages that contain *only* image + // attachments with no accompanying text. Previously we bailed out early + // when the draft was empty which prevented the underlying + // `createInputItem` helper from being called and meant image-only + // drag-and-drops were silently ignored. + if (!inputValue && attachedImages.length === 0) { return; } @@ -586,6 +681,9 @@ export default function TerminalChatInput({ } } + // (image-path fallback handled earlier in raw stdin listener; no need to + // duplicate here) + // Extract image paths from the final draft *once*, right before submit. const { paths: dropped, text } = extractImagePaths(inputValue); @@ -782,14 +880,15 @@ export default function TerminalChatInput({ value={input} onChange={(rawValue) => { // Strip any raw control-G char so it never shows up. - let value = rawValue.replace(/\x07/g, ""); + let value = rawValue.replaceAll("\u0007", ""); // -------------------------------------------------------- // Detect freshly-dropped image paths _while the user is // editing_ so the attachment preview updates instantly. // -------------------------------------------------------- - const { paths: newlyDropped, text: cleaned } = extractImagePaths(rawValue); + const { paths: newlyDropped, text: cleaned } = + extractImagePaths(rawValue); value = cleaned; @@ -839,7 +938,7 @@ export default function TerminalChatInput({ {(() => { const trimmed = input.trim(); const showSlash = - trimmed.startsWith("/") && /^[\/][a-zA-Z]+$/.test(trimmed); + trimmed.startsWith("/") && /^\/[a-zA-Z]+$/.test(trimmed); return showSlash; })() && ( diff --git a/codex-cli/src/text-buffer.ts b/codex-cli/src/text-buffer.ts index d7ce153060..df81ddf21d 100644 --- a/codex-cli/src/text-buffer.ts +++ b/codex-cli/src/text-buffer.ts @@ -141,9 +141,19 @@ export default class TextBuffer { process.env["EDITOR"] ?? (process.platform === "win32" ? "notepad" : "vi"); - // Prepare a temporary file with the current contents. We use mkdtempSync - // to obtain an isolated directory and avoid name collisions. - const tmpDir = fs.mkdtempSync(pathMod.join(os.tmpdir(), "codex-edit-")); + // Prepare a temporary file with the current contents. We use mkdtempSync + // to obtain an isolated directory and avoid name collisions. Similar to + // other parts of the codebase we occasionally run inside restricted + // environments (e.g. GitHub Codespaces) where the OS-level tmp directory + // is not writable. In that case fall back to creating the directory under + // the current working directory so the workflow still functions. + + let tmpDir: string; + try { + tmpDir = fs.mkdtempSync(pathMod.join(os.tmpdir(), "codex-edit-")); + } catch { + tmpDir = fs.mkdtempSync(pathMod.join(process.cwd(), "codex-edit-")); + } const filePath = pathMod.join(tmpDir, "buffer.txt"); fs.writeFileSync(filePath, this.getText(), "utf8"); diff --git a/codex-cli/src/utils/image-detector.ts b/codex-cli/src/utils/image-detector.ts index bf8d566ed5..3c632fe5ca 100644 --- a/codex-cli/src/utils/image-detector.ts +++ b/codex-cli/src/utils/image-detector.ts @@ -7,8 +7,7 @@ import { fileURLToPath } from "node:url"; // found. // --------------------------------------------------------------------------- -const IMAGE_EXT_REGEX = - "(?:png|jpe?g|gif|bmp|webp|svg)"; // deliberately kept simple +const IMAGE_EXT_REGEX = "(?:png|jpe?g|gif|bmp|webp|svg)"; // deliberately kept simple // Pattern helpers – compiled lazily so the whole file can be tree-shaken if // unused by a particular build target. @@ -21,14 +20,14 @@ function compileRegexes() { return; } - MARKDOWN_LINK_RE = /!\[[^\]]*?\]\(([^)]+)\)/g; // capture path inside () - QUOTED_PATH_RE = new RegExp( - `[\'\"]([^\'\"]+?\.${IMAGE_EXT_REGEX})[\'\"]`, - "gi", - ); - // eslint-disable-next-line no-useless-escape + // Capture path inside markdown image link e.g. ![alt](path) + MARKDOWN_LINK_RE = /!\[[^\]]*?\]\(([^)]+)\)/g; + // Any quoted image path – single or double quotes + QUOTED_PATH_RE = new RegExp(`["']([^"']+?[.]${IMAGE_EXT_REGEX})["']`, "gi"); + // Bare image paths appearing in text. Handles absolute, relative, and + // Windows drive-letter paths. BARE_PATH_RE = new RegExp( - `\\b(?:\\.[\\/\\\\]|[\\/\\\\]|[A-Za-z]:[\\/\\\\])?[\\w-]+(?:[\\/\\\\][\\w-]+)*\\.${IMAGE_EXT_REGEX}\\b`, + `(?:\\.[/\\\\]|[/\\\\]|[A-Za-z]:[/\\\\])?[\\w-]+(?:[/\\\\][\\w-]+)*.${IMAGE_EXT_REGEX}`, "gi", ); } diff --git a/codex-cli/tests/agent-project-doc.test.ts b/codex-cli/tests/agent-project-doc.test.ts index 00c749aff1..6da1b4cee0 100644 --- a/codex-cli/tests/agent-project-doc.test.ts +++ b/codex-cli/tests/agent-project-doc.test.ts @@ -87,10 +87,10 @@ import { loadConfig } from "../src/utils/config.js"; let projectDir: string; -# beforeEach runs once per test; when the sandbox blocks mkdtemp under the OS -# tmp directory (e.g. GitHub Codespaces or certain container runtimes) fall -# back to creating the directory under the current working directory so the -# suite can still run. +// beforeEach runs once per test; when the sandbox blocks mkdtemp under the OS +// tmp directory (e.g. GitHub Codespaces or certain container runtimes) falls +// back to creating the directory under the current working directory so the +// suite can still run. beforeEach(() => { try { projectDir = mkdtempSync(join(tmpdir(), "codex-proj-")); diff --git a/codex-cli/tests/attachment-preview.test.tsx b/codex-cli/tests/attachment-preview.test.tsx index 104b09cd1a..b05edc0638 100644 --- a/codex-cli/tests/attachment-preview.test.tsx +++ b/codex-cli/tests/attachment-preview.test.tsx @@ -48,6 +48,8 @@ function props() { interruptAgent: () => {}, active: true, onCompact: () => {}, + openDiffOverlay: () => {}, + thinkingSeconds: 0, }; } diff --git a/codex-cli/tests/backspace-delete-image.test.tsx b/codex-cli/tests/backspace-delete-image.test.tsx index 961635c0f9..128856ad31 100644 --- a/codex-cli/tests/backspace-delete-image.test.tsx +++ b/codex-cli/tests/backspace-delete-image.test.tsx @@ -44,6 +44,8 @@ function props() { interruptAgent: () => {}, active: true, onCompact: () => {}, + openDiffOverlay: () => {}, + thinkingSeconds: 0, }; } diff --git a/codex-cli/tests/check-updates.test.ts b/codex-cli/tests/check-updates.test.ts index 4d3b9b527c..d02bea5118 100644 --- a/codex-cli/tests/check-updates.test.ts +++ b/codex-cli/tests/check-updates.test.ts @@ -6,8 +6,7 @@ import { } from "../src/utils/check-updates.js"; import { execFile } from "node:child_process"; import { join } from "node:path"; -import { CONFIG_DIR } from "src/utils/config.js"; -import { beforeEach } from "node:test"; +import { CONFIG_DIR } from "../src/utils/config.js"; vi.mock("which", () => ({ default: vi.fn(() => "/usr/local/bin/npm"), @@ -36,6 +35,9 @@ vi.mock("node:fs/promises", async (importOriginal) => ({ } return memfs[path]; }, + writeFile: async (path: string, data: string) => { + memfs[path] = data; + }, })); beforeEach(() => { @@ -88,8 +90,9 @@ describe("Check for updates", () => { memfs[codexStatePath] = JSON.stringify({ lastUpdateCheck: new Date("2000-01-01T00:00:00Z").toUTCString(), }); - await checkForUpdates(); - // Spy on console.log to capture output + // Spy on console.log to capture output BEFORE calling the checker so we + // capture the very first message that is printed when an update is + // detected. const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); await checkForUpdates(); expect(logSpy).toHaveBeenCalled(); @@ -107,8 +110,6 @@ describe("Check for updates", () => { memfs[codexStatePath] = JSON.stringify({ lastUpdateCheck: new Date().toUTCString(), }); - await checkForUpdates(); - // Spy on console.log to capture output const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); await checkForUpdates(); expect(logSpy).not.toHaveBeenCalled(); diff --git a/codex-cli/tests/drag-drop-attach-image.test.tsx b/codex-cli/tests/drag-drop-attach-image.test.tsx index 427a85ce63..ba524d46a8 100644 --- a/codex-cli/tests/drag-drop-attach-image.test.tsx +++ b/codex-cli/tests/drag-drop-attach-image.test.tsx @@ -13,13 +13,32 @@ import { renderTui } from "./ui-test-helpers.js"; // Mocks – keep in sync with other TerminalChatInput UI tests // --------------------------------------------------------------------------- -// mock without type annotations to avoid Vitest transform TS errors in JS test -const createInputItemMock = vi.fn(async () => ({})); +// We need to capture a reference to the mocked `createInputItem` function so we +// can make assertions later in the test, _and_ respect Vitest’s requirement +// that any variables used inside the `vi.mock` factory are already defined at +// the time the factory is hoisted. To satisfy both constraints we: +// 1. Declare the variable with `let` (so it’s hoisted), **without** assigning +// a value yet. +// 2. Inside the factory, create the mock with `vi.fn()` and assign it to the +// outer-scoped variable before returning it. +// This avoids the “there was an error when mocking a module” failure that +// occurs when a factory closes over an uninitialised top-level `const`. -vi.mock("../src/utils/input-utils.js", () => ({ - createInputItem: createInputItemMock, - imageFilenameByDataUrl: new Map(), -})); +// Using `var` ensures the binding is hoisted, so it exists (as `undefined`) at +// the time the `vi.mock` factory runs. We re-assign it inside the factory. +// eslint-disable-next-line no-var +var createInputItemMock!: ReturnType; + +vi.mock("../src/utils/input-utils.js", () => { + // Initialise the mock lazily inside the factory so the reference is valid + // when the module is evaluated. + createInputItemMock = vi.fn(async () => ({})); + + return { + createInputItem: createInputItemMock, + imageFilenameByDataUrl: new Map(), + }; +}); vi.mock("../src/approvals.js", () => ({ isSafeCommand: () => null })); vi.mock("../src/format-command.js", () => ({ formatCommandForDisplay: (c: Array): string => c.join(" "), @@ -53,6 +72,8 @@ function props() { interruptAgent: () => {}, active: true, onCompact: () => {}, + openDiffOverlay: () => {}, + thinkingSeconds: 0, }; } @@ -70,7 +91,7 @@ describe("Drag-and-drop image attachment", () => { }); it("moves pasted path to attachment preview", async () => { - process.env.DEBUG_TCI = "1"; + process.env["DEBUG_TCI"] = "1"; const orig = process.cwd(); process.chdir(TMP); @@ -90,8 +111,7 @@ describe("Drag-and-drop image attachment", () => { // setState inside the onChange handler. await flush(); - let frame = lastFrameStripped(); - + const frame = lastFrameStripped(); expect(frame.match(/dropped\.png/g)?.length ?? 0).toBe(1); @@ -101,9 +121,9 @@ describe("Drag-and-drop image attachment", () => { // createInputItem should have been called with the dropped image path expect(createInputItemMock).toHaveBeenCalled(); - const calls = createInputItemMock.mock.calls; - const lastCall = calls[calls.length - 1]; - expect(lastCall?.[1]).toEqual(["dropped.png"]); + const calls: Array> = createInputItemMock.mock.calls as any; + const lastCall = calls[calls.length - 1] as Array; + expect(lastCall?.[1 as number]).toEqual(["dropped.png"]); cleanup(); process.chdir(orig); diff --git a/codex-cli/tests/extract-image-paths.test.ts b/codex-cli/tests/extract-image-paths.test.ts index 87fd796e63..09f68b8d75 100644 --- a/codex-cli/tests/extract-image-paths.test.ts +++ b/codex-cli/tests/extract-image-paths.test.ts @@ -12,7 +12,7 @@ describe("extractImagePaths", () => { }); it("detects quoted image", () => { - const { paths, text } = extractImagePaths("drag \"baz.jpg\" here"); + const { paths, text } = extractImagePaths('drag "baz.jpg" here'); expect(paths).toEqual(["baz.jpg"]); expect(text).toBe("drag here"); }); diff --git a/codex-cli/tests/project-doc.test.ts b/codex-cli/tests/project-doc.test.ts index 0f7b460fc7..6540df650e 100644 --- a/codex-cli/tests/project-doc.test.ts +++ b/codex-cli/tests/project-doc.test.ts @@ -8,7 +8,7 @@ let projectDir: string; let configPath: string; let instructionsPath: string; -# Use OS tmpdir unless blocked; fallback to cwd. +// Use OS tmpdir unless blocked; fallback to cwd. beforeEach(() => { try { projectDir = mkdtempSync(join(tmpdir(), "codex-proj-"));