Skip to content
Open
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
42 changes: 37 additions & 5 deletions sdk/typescript/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,18 @@ import {
constants,
existsSync,
lstatSync,
readdirSync,
realpathSync,
writeSync,
} from "node:fs";
import { mkdir, readFile, realpath, stat, writeFile } from "node:fs/promises";
import {
mkdir,
readFile,
readdir,
realpath,
stat,
writeFile,
} from "node:fs/promises";
import {
basename,
dirname,
Expand Down Expand Up @@ -2539,6 +2547,24 @@ function isOutsidePath(path: string): boolean {
return path === ".." || path.startsWith(`..${sep}`) || isAbsolute(path);
}

async function hasPartialOutput(path: string): Promise<boolean> {
try {
return (await readdir(path)).length > 0;
} catch {
// Keep the path in the diagnostic when it disappeared or cannot be read.
// Only suppress the message when emptiness was confirmed.
return true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle ENOENT as no retained output

Both this helper and hasPartialOutputSync turn every readdir failure into true, including ENOENT. If the path is removed after onOutputDirReady reports it, ordinary failure, SIGINT, and SIGTERM still print Partial output was kept at <missing path>. That is the same false claim this PR is meant to suppress, and #191 asks to show it only when the directory is non-empty.

At exact head 79e9fe1a, and again after a clean merge onto main 216212b7, the same fixture produced {exit: 2, claimed: true}, {signal: SIGINT, exit: 130, claimed: true}, and {signal: SIGTERM, exit: 143, claimed: true}. Returning false for ENOENT in both helpers makes all three cases pass while preserving the PR's empty/non-empty test and the static checks. Please handle the missing-path case and add ordinary plus signal-path regression coverage.

}
}

function hasPartialOutputSync(path: string): boolean {
try {
return readdirSync(path).length > 0;
} catch {
return true;
}
}

async function runExport(
arguments_: ExportArguments,
output: Writable,
Expand Down Expand Up @@ -3150,19 +3176,25 @@ async function runScan(
}

if (requestedSignal !== null) {
const partialOutput = scanDir !== null && hasPartialOutputSync(scanDir);
diagnostic("scan.interrupted", {
signal: requestedSignal,
partial_output: scanDir !== null,
partial_output: partialOutput,
});
return {
exitCode: interruptedExit(requestedSignal, scanDir, errorOutput),
exitCode: interruptedExit(
requestedSignal,
partialOutput ? scanDir : null,
errorOutput,
),
error:
requestedSignal === "SIGINT"
? "Scan canceled by Ctrl-C."
: "Scan terminated by SIGTERM.",
};
}
if (failed) {
const partialOutput = scanDir !== null && (await hasPartialOutput(scanDir));
const costLimitFailure =
failure instanceof ScanCostLimitExceededError ? failure : undefined;
const message =
Expand All @@ -3176,15 +3208,15 @@ async function runScan(
: isLocalScanFailure(failure)
? "local"
: classifyConnectionFailure(failure),
partial_output: scanDir !== null,
partial_output: partialOutput,
max_cost_usd: costLimitFailure?.maxCostUsd,
estimated_usd: costLimitFailure?.cost.estimatedUsd,
});
errorOutput.write(`${message}\n`);
if (failure instanceof ScanInterruptedError) {
return { exitCode: 2, error: message };
}
if (scanDir !== null) {
if (partialOutput && scanDir !== null) {
errorOutput.write(
`Partial output was kept at ${errorMessage(scanDir)}.\n`,
);
Expand Down
41 changes: 41 additions & 0 deletions sdk/typescript/tests-ts/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4876,6 +4876,47 @@ describe("CLI", () => {
expect(stderr.text()).not.toContain("codex-security:");
});

test("does not claim partial output when the output directory is empty", async () => {
const root = await mkdtemp(join(tmpdir(), "codex-security-empty-output-"));
try {
for (const [name, populate, expectsPartial] of [
["empty", false, false],
["partial", true, true],
] as const) {
const scanDir = join(root, name);
await mkdir(scanDir);
if (populate)
await writeFile(join(scanDir, "progress.log"), "partial\n");

const stdout = capture();
const stderr = capture();
const failing = dependencies();
failing.createSecurity = () => ({
run: async (_repository, options) => {
options?.onOutputDirReady?.(scanDir);
throw new Error("SYNTHETIC_SCAN_FAILURE");
},
close: async () => {},
preflight: async () => fakePreflight(),
});

expect(
await main(["scan", "."], stdout.stream, stderr.stream, failing),
).toBe(2);
expect(stderr.text()).toContain("SYNTHETIC_SCAN_FAILURE");
if (expectsPartial) {
expect(stderr.text()).toContain(
`Partial output was kept at ${scanDir}.`,
);
} else {
expect(stderr.text()).not.toContain("Partial output was kept");
}
}
} finally {
await rm(root, { recursive: true, force: true });
}
});

test("preserves complete protected-root diagnostics", async () => {
const stdout = capture();
const stderr = capture();
Expand Down