This commit is contained in:
Eason Goodale
2025-04-30 11:45:29 -07:00
parent 41ef530683
commit 5481bf0cb4
10 changed files with 174 additions and 41 deletions

View File

@@ -92,6 +92,14 @@ export default function TerminalChatInput({
const [selectedSuggestion, setSelectedSuggestion] = useState<number>(0);
const [input, setInput] = useState("");
const [attachedImages, setAttachedImages] = useState<Array<string>>([]);
// 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<Array<string>>([]);
useEffect(() => {
attachedImagesRef.current = attachedImages;
}, [attachedImages]);
// Image picker state null when closed, else current directory
const [pickerCwd, setPickerCwd] = useState<string | null>(null);
const [pickerRoot, setPickerRoot] = useState<string | null>(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
// higherlevel `useInput` hook does *not* emit a callback for this
// control sequence when running under the inktestinglibrary, 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;
})() && (
<Box flexDirection="column" paddingX={2} marginBottom={1}>

View File

@@ -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");

View File

@@ -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",
);
}

View File

@@ -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-"));

View File

@@ -48,6 +48,8 @@ function props() {
interruptAgent: () => {},
active: true,
onCompact: () => {},
openDiffOverlay: () => {},
thinkingSeconds: 0,
};
}

View File

@@ -44,6 +44,8 @@ function props() {
interruptAgent: () => {},
active: true,
onCompact: () => {},
openDiffOverlay: () => {},
thinkingSeconds: 0,
};
}

View File

@@ -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();

View File

@@ -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 Vitests 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 its 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<typeof vi.fn>;
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>): 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<Array<unknown>> = createInputItemMock.mock.calls as any;
const lastCall = calls[calls.length - 1] as Array<unknown>;
expect(lastCall?.[1 as number]).toEqual(["dropped.png"]);
cleanup();
process.chdir(orig);

View File

@@ -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");
});

View File

@@ -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-"));