From e396cd2160be4161c15796d6e66c3ca3e5a6aae3 Mon Sep 17 00:00:00 2001 From: HughhhhCoder Date: Thu, 27 Aug 2026 14:17:37 +0800 Subject: [PATCH 1/4] Preserve caller cancellation during cloud publication --- sdk/typescript/src/cloud-publish.ts | 6 ++- sdk/typescript/tests-ts/cloud-publish.test.ts | 52 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/sdk/typescript/src/cloud-publish.ts b/sdk/typescript/src/cloud-publish.ts index 3f0a6ce63..8f5a1b3ee 100644 --- a/sdk/typescript/src/cloud-publish.ts +++ b/sdk/typescript/src/cloud-publish.ts @@ -455,6 +455,7 @@ async function publishCloudPayload( signal, }); } catch { + dependencies.signal?.throwIfAborted(); // A lost response does not establish whether the server accepted the POST. throw new CodexSecurityError( "Cloud publication was not confirmed. The request was not retried; check whether it was accepted before submitting again.", @@ -475,7 +476,10 @@ async function publishCloudPayload( ); } const receipt = receiptSchema.safeParse( - await response.json().catch(() => undefined), + await response.json().catch(() => { + dependencies.signal?.throwIfAborted(); + return undefined; + }), ); // Cloud assigns opaque IDs in request order, so they cannot be compared to // local finding IDs. The authenticated response must still preserve the diff --git a/sdk/typescript/tests-ts/cloud-publish.test.ts b/sdk/typescript/tests-ts/cloud-publish.test.ts index 200b97613..a5c39f86d 100644 --- a/sdk/typescript/tests-ts/cloud-publish.test.ts +++ b/sdk/typescript/tests-ts/cloud-publish.test.ts @@ -684,6 +684,58 @@ describe("Cloud publication", () => { expect(requests).toBe(1); }); + test("preserves caller cancellation during the publication request", async () => { + const { scan, environment } = await fixture(); + const controller = new AbortController(); + const cancellation = new Error("publication cancelled"); + + await expect( + publishScanToCloud(scan, { + environment, + signal: controller.signal, + fetch: async (_url, options) => { + const signal = options.signal as AbortSignal; + return await new Promise((_resolve, reject) => { + signal.addEventListener( + "abort", + () => reject(signal.reason), + { once: true }, + ); + controller.abort(cancellation); + }); + }, + }), + ).rejects.toBe(cancellation); + }); + + test("preserves caller cancellation while parsing the acceptance receipt", async () => { + const { scan, environment } = await fixture(); + const controller = new AbortController(); + const cancellation = new Error("receipt parsing cancelled"); + const response = { + ok: true, + status: 201, + body: null, + json: async () => + await new Promise((_resolve, reject) => { + controller.signal.addEventListener( + "abort", + () => reject(controller.signal.reason), + { once: true }, + ); + controller.abort(cancellation); + }), + } as unknown as Response; + + await expect( + publishScanToCloud(scan, { + environment, + signal: controller.signal, + fetch: async () => response, + }), + ).rejects.toBe(cancellation); + }); + test("requires a complete acceptance receipt instead of treating any 2xx as success", async () => { const { scan, environment } = await fixture(); for (const body of [ From 8e28127ab7a1afef5c05dd284b8287b1b5c3fa9f Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Thu, 27 Aug 2026 06:59:20 -0400 Subject: [PATCH 2/4] fix(publish): retain recovery guidance on canceled uploads --- sdk/typescript/src/cli.ts | 13 +- .../tests-ts/cli-cloud-publish.test.ts | 164 +++++++++++++++++- sdk/typescript/tests-ts/cloud-publish.test.ts | 8 +- 3 files changed, 176 insertions(+), 9 deletions(-) diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index 5656c5862..ae312a07c 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -2107,6 +2107,13 @@ export async function main( output: z.record(z.string(), z.unknown()).optional(), async run({ args, format, formatExplicit, options }) { const controller = new AbortController(); + const publishingToCloud = options.to === "cloud" && !options.dryRun; + const publicationErrorMessage = (error: unknown): string => + publishingToCloud && + controller.signal.aborted && + error === controller.signal.reason + ? "Any upload already in flight may have been accepted. Check Cloud before retrying." + : safeErrorMessage(error); let presentation: PublicationProgressPresenter | undefined; let firstSignalAt = 0; let observingSignals = false; @@ -2150,9 +2157,9 @@ export async function main( ? "Publication canceled by Ctrl-C." : "Publication terminated by SIGTERM."; const recovery = - error === undefined || error === signal + error === undefined || (error === signal && !publishingToCloud) ? "" - : ` ${diagnosticValue(safeErrorMessage(error))}`; + : ` ${diagnosticValue(publicationErrorMessage(error))}`; errorOutput.write(`codex-security: ${reason}${recovery}\n`); exitCode = signal === "SIGINT" ? 130 : 143; return true; @@ -2492,7 +2499,7 @@ export async function main( }); cloudBatch.results.push({ scanDir: directory, ...result }); } catch (error) { - const message = safeErrorMessage(error); + const message = publicationErrorMessage(error); cloudBatch.failed.push({ scanDir: directory, ...(scanId === undefined ? {} : { scanId }), diff --git a/sdk/typescript/tests-ts/cli-cloud-publish.test.ts b/sdk/typescript/tests-ts/cli-cloud-publish.test.ts index 803c5ad93..6d21c7472 100644 --- a/sdk/typescript/tests-ts/cli-cloud-publish.test.ts +++ b/sdk/typescript/tests-ts/cli-cloud-publish.test.ts @@ -1,8 +1,18 @@ -import { mkdir, mkdtemp, realpath, rm, symlink } from "node:fs/promises"; +import { + chmod, + cp, + mkdir, + mkdtemp, + realpath, + rm, + symlink, + writeFile, +} from "node:fs/promises"; import { homedir, tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { afterEach, describe, expect, test } from "bun:test"; import { main } from "../src/cli.js"; +import { publishScanToCloud } from "../src/cloud-publish.js"; import type { JsonObject } from "../src/index.js"; import { capture, @@ -10,6 +20,7 @@ import { FakeSignals, SYNTHETIC_CREDENTIALS, } from "./cli-fixtures.js"; +import { PLUGIN_ROOT } from "./plugin-root.js"; const receipt = { scanId: "scan-1", @@ -631,6 +642,136 @@ describe("publish scan to Cloud", () => { }, ); + test.each([ + ["single request", "request", false], + ["single receipt", "receipt", false], + ["batch request", "request", true], + ["batch receipt", "receipt", true], + ] as const)( + "keeps recovery guidance from the real Cloud publisher: %s", + async (_scenario, stage, batch) => { + for (const [signal, code] of [ + ["SIGINT", 130], + ["SIGTERM", 143], + ] as const) { + const root = await realpath( + await mkdtemp(join(tmpdir(), "cloud-publication-cancel-")), + ); + temporaryDirectories.push(root); + const credentialHome = join(root, "credentials"); + await mkdir(credentialHome, { mode: 0o700 }); + await writeFile( + join(credentialHome, "auth.json"), + JSON.stringify({ + auth_mode: "chatgpt", + tokens: { + access_token: "synthetic-access-token", + account_id: "synthetic-account", + }, + }), + { mode: 0o600 }, + ); + await writeFile( + join(credentialHome, "config.toml"), + 'cli_auth_credentials_store = "file"\n', + ); + const scanDirectories = await Promise.all( + ["scan-one", "scan-two"].map(async (name) => { + const directory = join(root, name); + await cp( + join(PLUGIN_ROOT, "examples", "completed-scan"), + directory, + { + recursive: true, + }, + ); + if (process.platform !== "win32") await chmod(directory, 0o700); + return directory; + }), + ); + const directories = batch + ? [...scanDirectories, join(root, "not-attempted")] + : [scanDirectories[0]!]; + const signals = new FakeSignals(); + const deps = dependencies({ + signals, + currentDirectory: root, + environment: { + CODEX_HOME: credentialHome, + CODEX_SECURITY_STATE_DIR: join(root, "state"), + }, + }); + let requests = 0; + deps.publishScanToCloud = (directory, options) => + publishScanToCloud(directory, { + ...options, + fetch: async (_url, request) => { + requests++; + if (batch && requests === 1) { + return Response.json({ + status: "accepted", + finding_ids: ["accepted-finding"], + finding_count: 1, + }); + } + expect(request.signal).toBeInstanceOf(AbortSignal); + if (stage === "request") { + signals.emit(signal); + throw request.signal!.reason; + } + return new Response( + new ReadableStream({ + pull(body) { + signals.emit(signal); + body.error(request.signal!.reason); + }, + }), + { status: 201 }, + ); + }, + }); + const stdout = capture(); + const stderr = capture(); + expect( + await main( + [ + "publish", + "scan", + ...directories.flatMap((directory) => ["--scan-dir", directory]), + "--to", + "cloud", + "--json", + ], + stdout.stream, + stderr.stream, + deps, + ), + ).toBe(code); + expect(requests).toBe(batch ? 2 : 1); + if (batch) { + expect(JSON.parse(stdout.text())).toEqual({ + results: [ + expect.objectContaining({ + scanDir: directories[0], + findingIds: ["accepted-finding"], + }), + ], + failed: [ + { + scanDir: directories[1], + error: expect.stringMatching(/accepted.*check.*retry/i), + }, + ], + notAttempted: [directories[2]], + }); + } else { + expect(stdout.text()).toBe(""); + } + expect(stderr.text()).toMatch(/accepted.*check.*retry/i); + } + }, + ); + test("rejects multiple scans for Linear before publishing any findings", async () => { const deps = dependencies(); let calls = 0; @@ -1034,6 +1175,27 @@ describe("publish scan to Cloud", () => { ).toBe(true); }); + test("does not suggest retrying an upload when a Cloud dry run is canceled", async () => { + const signals = new FakeSignals(); + const deps = dependencies({ signals }); + deps.publishScanToCloud = async (_directory, options) => { + signals.emit("SIGINT"); + throw options!.signal!.reason; + }; + const stdout = capture(); + const stderr = capture(); + expect( + await main( + ["publish", "scan", "completed-scan", "--to", "cloud", "--dry-run"], + stdout.stream, + stderr.stream, + deps, + ), + ).toBe(130); + expect(stdout.text()).toBe(""); + expect(stderr.text()).not.toMatch(/accepted|retry/i); + }); + test("aborts Cloud publication without activating Linear recovery signal handling", async () => { for (const [signal, code] of [ ["SIGINT", 130], diff --git a/sdk/typescript/tests-ts/cloud-publish.test.ts b/sdk/typescript/tests-ts/cloud-publish.test.ts index a5c39f86d..9cd96f6ca 100644 --- a/sdk/typescript/tests-ts/cloud-publish.test.ts +++ b/sdk/typescript/tests-ts/cloud-publish.test.ts @@ -696,11 +696,9 @@ describe("Cloud publication", () => { fetch: async (_url, options) => { const signal = options.signal as AbortSignal; return await new Promise((_resolve, reject) => { - signal.addEventListener( - "abort", - () => reject(signal.reason), - { once: true }, - ); + signal.addEventListener("abort", () => reject(signal.reason), { + once: true, + }); controller.abort(cancellation); }); }, From 3b04e0425e615a881b2fcc782cba403531d6769b Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Thu, 27 Aug 2026 12:34:19 -0400 Subject: [PATCH 3/4] fix(publish): omit upload warnings before publication starts --- sdk/typescript/src/cli.ts | 4 +++- sdk/typescript/tests-ts/cli-cloud-publish.test.ts | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index ae312a07c..27ef46fea 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -2107,7 +2107,7 @@ export async function main( output: z.record(z.string(), z.unknown()).optional(), async run({ args, format, formatExplicit, options }) { const controller = new AbortController(); - const publishingToCloud = options.to === "cloud" && !options.dryRun; + let publishingToCloud = false; const publicationErrorMessage = (error: unknown): string => publishingToCloud && controller.signal.aborted && @@ -2244,6 +2244,7 @@ export async function main( observingSignals = true; } if (csvPath !== undefined) { + publishingToCloud = !options.dryRun; const result = await ( dependencies.publishFindingsCsvToCloud ?? publishFindingsCsvToCloud )(csvPath, { @@ -2472,6 +2473,7 @@ export async function main( } scanDir = selectedScans[0]!.scanDir; controller.signal.throwIfAborted(); + publishingToCloud = !options.dryRun; if (selectedScans.length > 1) { cloudBatch = { results: [], diff --git a/sdk/typescript/tests-ts/cli-cloud-publish.test.ts b/sdk/typescript/tests-ts/cli-cloud-publish.test.ts index 6d21c7472..fa88708b0 100644 --- a/sdk/typescript/tests-ts/cli-cloud-publish.test.ts +++ b/sdk/typescript/tests-ts/cli-cloud-publish.test.ts @@ -353,15 +353,18 @@ describe("publish scan to Cloud", () => { uploads++; return receipt; }; + const stderr = capture(); expect( await main( ["publish", "scan", "--to", "cloud"], capture().stream, - capture().stream, + stderr.stream, deps, ), ).toBe(130); expect(uploads).toBe(0); + expect(stderr.text()).toContain("Publication canceled"); + expect(stderr.text()).not.toMatch(/accepted|retry/i); expect( [...signals.listeners.values()].every( (listeners) => listeners.size === 0, From 994cf636c1d2c044f2e2d48b3c2571be328de9e1 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Thu, 27 Aug 2026 12:50:26 -0400 Subject: [PATCH 4/4] fix(publish): track cancellation at the Cloud request boundary --- sdk/typescript/src/cli.ts | 21 ++- .../tests-ts/cli-cloud-publish.test.ts | 124 +++++++++++++----- 2 files changed, 108 insertions(+), 37 deletions(-) diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index 27ef46fea..6ebcd3f6d 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -1120,6 +1120,7 @@ interface CliDependencies { publishScan?: typeof publishScan; publishFindingsCsvToCloud?: typeof publishFindingsCsvToCloud; publishScanToCloud?: typeof publishScanToCloud; + cloudFetch?: (url: string, options: RequestInit) => Promise; confirmPatchReview?: (question: string) => Promise; patchEditor?: ( repository: string, @@ -2107,9 +2108,17 @@ export async function main( output: z.record(z.string(), z.unknown()).optional(), async run({ args, format, formatExplicit, options }) { const controller = new AbortController(); - let publishingToCloud = false; + let cloudRequestStarted = false; + const cloudFetch = ( + url: string, + options: RequestInit, + ): Promise => { + options.signal?.throwIfAborted(); + cloudRequestStarted = true; + return (dependencies.cloudFetch ?? globalThis.fetch)(url, options); + }; const publicationErrorMessage = (error: unknown): string => - publishingToCloud && + cloudRequestStarted && controller.signal.aborted && error === controller.signal.reason ? "Any upload already in flight may have been accepted. Check Cloud before retrying." @@ -2157,7 +2166,7 @@ export async function main( ? "Publication canceled by Ctrl-C." : "Publication terminated by SIGTERM."; const recovery = - error === undefined || (error === signal && !publishingToCloud) + error === undefined || (error === signal && !cloudRequestStarted) ? "" : ` ${diagnosticValue(publicationErrorMessage(error))}`; errorOutput.write(`codex-security: ${reason}${recovery}\n`); @@ -2244,13 +2253,13 @@ export async function main( observingSignals = true; } if (csvPath !== undefined) { - publishingToCloud = !options.dryRun; const result = await ( dependencies.publishFindingsCsvToCloud ?? publishFindingsCsvToCloud )(csvPath, { environment: dependencies.environment, dryRun: options.dryRun, signal: controller.signal, + fetch: cloudFetch, }); return { ...result }; } @@ -2473,7 +2482,6 @@ export async function main( } scanDir = selectedScans[0]!.scanDir; controller.signal.throwIfAborted(); - publishingToCloud = !options.dryRun; if (selectedScans.length > 1) { cloudBatch = { results: [], @@ -2490,6 +2498,7 @@ export async function main( break; } cloudBatch.notAttempted.shift(); + cloudRequestStarted = false; try { const result = await ( dependencies.publishScanToCloud ?? publishScanToCloud @@ -2497,6 +2506,7 @@ export async function main( environment: dependencies.environment, dryRun: options.dryRun, signal: controller.signal, + fetch: cloudFetch, ...(scanId === undefined ? {} : { expectedScanId: scanId }), }); cloudBatch.results.push({ scanDir: directory, ...result }); @@ -2522,6 +2532,7 @@ export async function main( environment: dependencies.environment, dryRun: options.dryRun, signal: controller.signal, + fetch: cloudFetch, ...(selectedScans[0]?.scanId === undefined ? {} : { expectedScanId: selectedScans[0].scanId }), diff --git a/sdk/typescript/tests-ts/cli-cloud-publish.test.ts b/sdk/typescript/tests-ts/cli-cloud-publish.test.ts index fa88708b0..c021114d6 100644 --- a/sdk/typescript/tests-ts/cli-cloud-publish.test.ts +++ b/sdk/typescript/tests-ts/cli-cloud-publish.test.ts @@ -12,7 +12,10 @@ import { homedir, tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { afterEach, describe, expect, test } from "bun:test"; import { main } from "../src/cli.js"; -import { publishScanToCloud } from "../src/cloud-publish.js"; +import { + publishFindingsCsvToCloud, + publishScanToCloud, +} from "../src/cloud-publish.js"; import type { JsonObject } from "../src/index.js"; import { capture, @@ -488,6 +491,7 @@ describe("publish scan to Cloud", () => { environment: deps.environment, dryRun, signal: expect.any(AbortSignal), + fetch: expect.any(Function), }); const { scanDir: _, ...result } = results[calls.length]!; calls.push(directory); @@ -646,12 +650,14 @@ describe("publish scan to Cloud", () => { ); test.each([ + ["single preflight", "preflight", false], ["single request", "request", false], ["single receipt", "receipt", false], + ["batch preflight", "preflight", true], ["batch request", "request", true], ["batch receipt", "receipt", true], ] as const)( - "keeps recovery guidance from the real Cloud publisher: %s", + "handles cancellation from the real Cloud publisher: %s", async (_scenario, stage, batch) => { for (const [signal, code] of [ ["SIGINT", 130], @@ -705,34 +711,39 @@ describe("publish scan to Cloud", () => { }, }); let requests = 0; - deps.publishScanToCloud = (directory, options) => - publishScanToCloud(directory, { - ...options, - fetch: async (_url, request) => { - requests++; - if (batch && requests === 1) { - return Response.json({ - status: "accepted", - finding_ids: ["accepted-finding"], - finding_count: 1, - }); - } - expect(request.signal).toBeInstanceOf(AbortSignal); - if (stage === "request") { + deps.cloudFetch = async (_url, request) => { + requests++; + if (batch && requests === 1) { + return Response.json({ + status: "accepted", + finding_ids: ["accepted-finding"], + finding_count: 1, + }); + } + expect(request.signal).toBeInstanceOf(AbortSignal); + if (stage === "request") { + signals.emit(signal); + throw request.signal!.reason; + } + return new Response( + new ReadableStream({ + pull(body) { signals.emit(signal); - throw request.signal!.reason; - } - return new Response( - new ReadableStream({ - pull(body) { - signals.emit(signal); - body.error(request.signal!.reason); - }, - }), - { status: 201 }, - ); - }, - }); + body.error(request.signal!.reason); + }, + }), + { status: 201 }, + ); + }; + let publications = 0; + deps.publishScanToCloud = (directory, options) => { + const result = publishScanToCloud(directory, options); + publications++; + if (stage === "preflight" && publications === (batch ? 2 : 1)) { + signals.emit(signal); + } + return result; + }; const stdout = capture(); const stderr = capture(); expect( @@ -750,7 +761,9 @@ describe("publish scan to Cloud", () => { deps, ), ).toBe(code); - expect(requests).toBe(batch ? 2 : 1); + expect(requests).toBe( + (batch ? 1 : 0) + (stage === "preflight" ? 0 : 1), + ); if (batch) { expect(JSON.parse(stdout.text())).toEqual({ results: [ @@ -762,7 +775,10 @@ describe("publish scan to Cloud", () => { failed: [ { scanDir: directories[1], - error: expect.stringMatching(/accepted.*check.*retry/i), + error: + stage === "preflight" + ? signal + : expect.stringMatching(/accepted.*check.*retry/i), }, ], notAttempted: [directories[2]], @@ -770,11 +786,54 @@ describe("publish scan to Cloud", () => { } else { expect(stdout.text()).toBe(""); } - expect(stderr.text()).toMatch(/accepted.*check.*retry/i); + if (stage === "preflight") { + expect(stderr.text()).not.toMatch(/accepted|retry/i); + } else { + expect(stderr.text()).toMatch(/accepted.*check.*retry/i); + } } }, ); + test("cancels during CSV reading without suggesting an upload was accepted", async () => { + const root = await mkdtemp(join(tmpdir(), "cloud-csv-cancel-")); + temporaryDirectories.push(root); + const csv = join(root, "findings.csv"); + await writeFile(csv, ""); + const signals = new FakeSignals(); + const deps = dependencies({ + signals, + currentDirectory: root, + environment: { + CODEX_HOME: join(root, "credentials"), + CODEX_SECURITY_STATE_DIR: join(root, "state"), + }, + }); + let requests = 0; + deps.cloudFetch = async () => { + requests++; + throw new Error("unexpected upload"); + }; + deps.publishFindingsCsvToCloud = (path, options) => { + const result = publishFindingsCsvToCloud(path, options); + signals.emit("SIGINT"); + return result; + }; + const stdout = capture(); + const stderr = capture(); + expect( + await main( + ["publish", "scan", "--csv", csv, "--to", "cloud"], + stdout.stream, + stderr.stream, + deps, + ), + ).toBe(130); + expect(requests).toBe(0); + expect(stdout.text()).toBe(""); + expect(stderr.text()).not.toMatch(/accepted|retry/i); + }); + test("rejects multiple scans for Linear before publishing any findings", async () => { const deps = dependencies(); let calls = 0; @@ -843,6 +902,7 @@ describe("publish scan to Cloud", () => { environment: deps.environment, dryRun, signal: expect.any(AbortSignal), + fetch: expect.any(Function), }); return result; };