Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions sdk/typescript/src/deduplication/codex-review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { createInterface } from "node:readline";
import {
comparisonEnvironment,
disabledMcpServers,
environmentEntry,
} from "../scan-comparison.js";
import {
codexSecurityCredentialHome,
Expand Down Expand Up @@ -81,19 +82,23 @@ export class CodexReviewRunner {
environment,
{ workingDirectory: this.workingDirectory, signal: this.signal },
);
const apiKey =
environment["OPENAI_API_KEY"] ?? environment["CODEX_API_KEY"];
const apiKey = [
environmentEntry(environment, "OPENAI_API_KEY"),
environmentEntry(environment, "CODEX_API_KEY"),
].find((value) => value?.trim());
const args = ["app-server", "--stdio", "--disable", "plugins"];
const stateDatabase = join(
codexSecurityStateDirectory(environment),
"workbench.sqlite3",
);
const privatePaths = new Set(
[
environment["CODEX_HOME"] ?? join(homedir(), ".codex"),
environmentEntry(environment, "CODEX_HOME") ||
join(homedir(), ".codex"),
codexSecurityCredentialHome(environment),
join(homedir(), ".ssh"),
environment["GH_CONFIG_DIR"] ?? join(homedir(), ".config", "gh"),
environmentEntry(environment, "GH_CONFIG_DIR") ||
join(homedir(), ".config", "gh"),
stateDatabase,
`${stateDatabase}-wal`,
`${stateDatabase}-shm`,
Expand Down
2 changes: 1 addition & 1 deletion sdk/typescript/src/scan-comparison.ts
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ export async function comparisonEnvironment(
return environment;
}

function environmentEntry(
export function environmentEntry(
environment: Record<string, string>,
requested: string,
): string | undefined {
Expand Down
192 changes: 171 additions & 21 deletions sdk/typescript/tests-ts/codex-review.test.ts
Original file line number Diff line number Diff line change
@@ -1,44 +1,87 @@
import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process";
import { existsSync } from "node:fs";
import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { homedir, tmpdir } from "node:os";
import { join, resolve } from "node:path";
import { fileURLToPath } from "node:url";
import { expect, test } from "bun:test";
import { expect, mock, test } from "bun:test";
import { CodexReviewRunner } from "../src/deduplication/codex-review.js";
import { environmentEntry } from "../src/scan-comparison.js";
import { runTestInSubprocess } from "./support/test-subprocess.js";

const fixture = fileURLToPath(
new URL("fixtures/codex-review.mjs", import.meta.url),
);

for (const scenario of [
"correction",
"text-only",
"failed-turn",
"exit",
"cancel",
]) {
test(`Codex review transport: ${scenario}`, async () => {
const transportCases: {
scenario: string;
name?: string;
environmentNames?: readonly [string, string, string];
extraEnvironment?: Record<string, string>;
windowsOnly?: boolean;
}[] = [
...["correction", "text-only", "failed-turn", "exit", "cancel"].map(
(scenario) => ({ scenario }),
),
{
scenario: "correction",
name: "lowercase Windows environment",
environmentNames: ["codex_home", "openai_api_key", "gh_config_dir"],
windowsOnly: true,
},
{
scenario: "correction",
name: "mixed-case Windows environment",
environmentNames: ["Codex_Home", "Codex_Api_Key", "Gh_Config_Dir"],
windowsOnly: true,
},
...["", " \t"].map((value) => ({
scenario: "correction",
name: `${value === "" ? "empty" : "blank"} OpenAI key uses Codex key`,
environmentNames: ["CODEX_HOME", "CODEX_API_KEY", "GH_CONFIG_DIR"] as const,
extraEnvironment: { OPENAI_API_KEY: value },
})),
{
scenario: "correction",
name: "empty Windows OpenAI alias uses Codex key",
environmentNames: ["Codex_Home", "Codex_Api_Key", "Gh_Config_Dir"],
extraEnvironment: { openai_api_key: "" },
windowsOnly: true,
},
];

for (const {
scenario,
name = scenario,
environmentNames = ["CODEX_HOME", "OPENAI_API_KEY", "GH_CONFIG_DIR"],
extraEnvironment,
windowsOnly = false,
} of transportCases) {
const runCase = test.skipIf(windowsOnly && process.platform !== "win32");
runCase(`Codex review transport: ${name}`, async () => {
const modelHome = await mkdtemp(join(tmpdir(), "codex-review-test-"));
const checkout = await mkdtemp(join(tmpdir(), "codex-review-source-"));
const ghConfig = await mkdtemp(join(tmpdir(), "codex-review-gh-"));
const transcript = join(modelHome, "messages.jsonl");
let child: ChildProcessWithoutNullStreams | undefined;
let directory: string | undefined;
let args: readonly string[] = [];
const controller = new AbortController();
try {
await writeFile(
join(modelHome, "config.toml"),
'[mcp_servers.synthetic]\ncommand = "synthetic-unused-command"\n',
);
const configuration =
'[mcp_servers.synthetic]\ncommand = "synthetic-unused-command"\n';
await writeFile(join(modelHome, "config.toml"), configuration);
const [homeName, keyName, ghName] = environmentNames;
const runner = new CodexReviewRunner(
{
PATH: process.env["PATH"],
SystemRoot: process.env["SystemRoot"],
TEMP: process.env["TEMP"],
TMP: process.env["TMP"],
CODEX_HOME: modelHome,
OPENAI_API_KEY: "synthetic-review-key",
[homeName]: modelHome,
[keyName]: "synthetic-review-key",
[ghName]: ghConfig,
...extraEnvironment,
},
(_command, commandArgs, options) => {
args = commandArgs;
Expand Down Expand Up @@ -95,17 +138,124 @@ for (const scenario of [
}
expect(args).toContain('cli_auth_credentials_store="ephemeral"');
expect(args.join(" ")).not.toContain("synthetic-review-key");
if (scenario !== "cancel")
expect(await readFile(transcript, "utf8")).toContain(
'"method":"account/login/start"',
);
const permissions = args.find((argument) =>
argument.startsWith("permissions.codex_security_review="),
);
expect(permissions).toContain(
`${JSON.stringify(resolve(modelHome))}="deny"`,
);
expect(permissions).toContain(
`${JSON.stringify(resolve(ghConfig))}="deny"`,
);
if (scenario !== "cancel") {
const loginRequest = (await readFile(transcript, "utf8"))
.trim()
.split("\n")
.map(
(line) =>
JSON.parse(line) as {
method?: string;
params?: { apiKey?: string };
},
)
.find((message) => message.method === "account/login/start");
expect(loginRequest?.params?.apiKey).toBe("synthetic-review-key");
}
expect(existsSync(join(modelHome, "auth.json"))).toBe(false);
expect(child!.exitCode !== null || child!.signalCode !== null).toBe(true);
expect(existsSync(directory!)).toBe(false);
expect(existsSync(checkout)).toBe(true);
} finally {
await rm(modelHome, { recursive: true, force: true });
await rm(checkout, { recursive: true, force: true });
await rm(ghConfig, { recursive: true, force: true });
}
});
}

test("empty credential paths use default directories without denying cwd", async () => {
if (
runTestInSubprocess(
import.meta.path,
"empty credential paths use default directories without denying cwd",
)
) {
return;
}
const comparison = { ...(await import("../src/scan-comparison.js")) };
const root = await mkdtemp(join(tmpdir(), "codex-review-empty-paths-"));
const checkout = await mkdtemp(join(tmpdir(), "codex-review-source-"));
// An empty CODEX_HOME makes native Codex use the real user profile.
mock.module("../src/scan-comparison.js", () => ({
...comparison,
disabledMcpServers: async () => ({}),
}));
try {
const names: [string, string][] = [["CODEX_HOME", "GH_CONFIG_DIR"]];
if (process.platform === "win32")
names.push(["codex_home", "Gh_Config_Dir"]);
for (const [homeName, ghName] of names) {
let args: readonly string[] = [];
const runner = new CodexReviewRunner(
{
CODEX_CLI_PATH: process.execPath,
CODEX_SECURITY_STATE_DIR: join(root, "state"),
OPENAI_API_KEY: "synthetic-review-key",
[homeName]: "",
[ghName]: "",
},
(_command, commandArgs) => {
args = commandArgs;
throw new Error("Synthetic stop after permission configuration");
},
undefined,
checkout,
);
await expect(
runner.run({
model: "gpt-5.6-sol",
effort: "ultra",
prompt: "Review the supplied synthetic reports.",
schema: {},
validate: (value) => value,
}),
).rejects.toThrow(
"Codex did not complete a validated deduplication review",
);
const permissions = args.find((argument) =>
argument.startsWith("permissions.codex_security_review="),
);
for (const path of [
join(homedir(), ".codex"),
join(homedir(), ".config", "gh"),
]) {
expect(permissions).toContain(
`${JSON.stringify(resolve(path))}="deny"`,
);
}
expect(permissions).not.toContain(
`${JSON.stringify(resolve(""))}="deny"`,
);
}
} finally {
mock.module("../src/scan-comparison.js", () => comparison);
await rm(root, { recursive: true, force: true });
await rm(checkout, { recursive: true, force: true });
}
});

test("environment lookups preserve platform case rules and exact-key precedence", () => {
const aliases = { codex_home: "synthetic-alias" };
expect(environmentEntry(aliases, "CODEX_HOME")).toBe(
process.platform === "win32" ? "synthetic-alias" : undefined,
);
expect(
environmentEntry(
{ ...aliases, CODEX_HOME: "synthetic-exact" },
"CODEX_HOME",
),
).toBe("synthetic-exact");
expect(environmentEntry({ ...aliases, CODEX_HOME: "" }, "CODEX_HOME")).toBe(
"",
);
});
Loading