From 0f24bb442b97c444ec6aa53ab3ef4d4396e3c53a Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Sat, 12 Sep 2026 19:07:50 +0000 Subject: [PATCH 01/14] Preserve independent review coverage through publication and recovery --- .../mcp-app/src/artifact-deep-reducer.ts | 32 +++- .../codex-security/mcp-app/src/artifact-io.ts | 2 + .../src/deep-scan/artifact-validation.ts | 108 ++++++++++- .../mcp-app/src/deep-scan/coordinator.ts | 34 ++-- .../mcp-app/src/deep-scan/worker-runner.ts | 18 +- .../tests/deep_scan_coverage_fixture.mjs | 143 +++++++++++++++ .../tests/deep_scan_publication_cases.mjs | 20 ++- .../tests/test_artifact_deep_reducer.mjs | 32 +++- .../mcp-app/tests/test_deep_scan_executor.mjs | 38 ++++ .../scripts/report_projection.py | 33 ++++ .../scripts/workbench_saved_results.py | 60 +++++-- .../tests/test_stopped_source_coverage.py | 167 ++++++++++++++++++ .../tests-ts/deep-scan-coverage.test.ts | 114 ++++++++++++ 13 files changed, 750 insertions(+), 51 deletions(-) create mode 100644 plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs create mode 100644 plugins/codex-security/tests/test_stopped_source_coverage.py create mode 100644 sdk/typescript/tests-ts/deep-scan-coverage.test.ts diff --git a/plugins/codex-security/mcp-app/src/artifact-deep-reducer.ts b/plugins/codex-security/mcp-app/src/artifact-deep-reducer.ts index b8e147ed9..06d0efa7f 100644 --- a/plugins/codex-security/mcp-app/src/artifact-deep-reducer.ts +++ b/plugins/codex-security/mcp-app/src/artifact-deep-reducer.ts @@ -1,4 +1,4 @@ -import { join } from "node:path"; +import { dirname, join, relative, sep } from "node:path"; import type { ZodType } from "zod/v4"; import commonSchema from "../../schemas/definitions/artifact-common.schema.json"; import reducerSchema from "../../schemas/tools/deep-reducer.schema.json"; @@ -20,7 +20,9 @@ import { type DeepScanArtifacts } from "./deep-scan/artifacts.js"; import { + deepReductionForPersistence, parseDeepReduction, + projectDiscoveryCoverage, reconcileDeepReduction, type DeepReductionInput, type DeepReductionSources, @@ -54,6 +56,18 @@ interface BoundReducer { /** Read the findings and scan context assigned to this reducer. */ export async function getCodexSecurityDeepReducerInputs( context: ArtifactContext +): Promise { + const inputs = await readDeepReductionSources(context); + const { sourceCoverage: _coverage, ...previous } = inputs.previous ?? {}; + return { + discoveries: inputs.discoveries.map(({ workerId, result }) => ({ workerId, result })), + previous: inputs.previous === null ? null : previous as DeepReductionInput, + }; +} + +/** Capture host coverage alongside the reducer's immutable finding inputs. */ +export async function readDeepReductionSources( + context: ArtifactContext ): Promise { return withLogicalReducerErrors(context, async () => { const bound = bindDeepReducer(context); @@ -73,8 +87,13 @@ export async function getCodexSecurityDeepReducerInputs( sourceFindingIds: [`${worker.id}:${index}`], }, })); - const { coverage: _coverage, ...reduction } = result; - return { workerId: worker.id, result: reduction }; + const { coverage, ...reduction } = result; + return { + workerId: worker.id, + ...(worker.attempt === undefined ? {} : { attempt: worker.attempt }), + coverage: projectDiscoveryCoverage(coverage, worker, relative(bound.artifacts.scanDir, dirname(worker.resultPath)).split(sep).join("/")), + result: reduction, + }; })); const previous = await readPreviousReduction(bound); const scanId = bound.scanId ?? previous?.scanId ?? discoveries[0]?.result.scanId; @@ -107,7 +126,7 @@ export async function recordCodexSecurityDeepReduction( const submitted = deepReductionInputSchema.parse(input); let reduction = parseDeepReduction(submitted); if (reduction.complete === false) throw new Error("Deep reduction is only a checkpoint, not a complete result."); - const inputs = await getCodexSecurityDeepReducerInputs(context); + const inputs = await readDeepReductionSources(context); const expectedScanId = bound.scanId ?? inputs.previous?.scanId ?? inputs.discoveries[0]?.result.scanId; @@ -116,8 +135,9 @@ export async function recordCodexSecurityDeepReduction( } reduction = reconcileDeepReduction(reduction, inputs.discoveries, inputs.previous); - await saveScanDraftCheckpoint(context, reduction); - await writeJsonAtomic(bound.resultPath, reduction); + const persisted = deepReductionForPersistence(reduction, bound.state.persistSourceCoverage); + await saveScanDraftCheckpoint(context, persisted); + await writeJsonAtomic(bound.resultPath, persisted); return { findingCount: reduction.findings.length, consumedWorkerIds: bound.state.claimedWorkers.map((worker) => worker.id) diff --git a/plugins/codex-security/mcp-app/src/artifact-io.ts b/plugins/codex-security/mcp-app/src/artifact-io.ts index 3208f6d8a..0a4299467 100644 --- a/plugins/codex-security/mcp-app/src/artifact-io.ts +++ b/plugins/codex-security/mcp-app/src/artifact-io.ts @@ -5,12 +5,14 @@ import { dirname, isAbsolute, join, resolve, sep } from "node:path"; export interface DeepReducerWorkerContext { id: string; resultPath: string; + attempt?: number; } export interface DeepReducerContext { scanRoot: string; claimedWorkers: DeepReducerWorkerContext[]; previousReducerResultPath?: string; + persistSourceCoverage?: boolean; } /** diff --git a/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts b/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts index 291557a9e..4daaafc99 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts @@ -10,10 +10,13 @@ import { import { readJsonObject, requireRegularFile, writeJsonAtomic } from "./artifacts.js"; import type { DeepScanArtifacts } from "./artifacts.js"; -export type DeepReductionInput = Omit; +export type DeepReductionInput = Omit & { + /** Host projection of accepted source coverage; never supplied by the reducer. */ + sourceCoverage?: ScanDraftInput["coverage"]; +}; export interface DeepReductionSources { - discoveries: { workerId: string; result: DeepReductionInput }[]; + discoveries: { workerId: string; attempt?: number; coverage?: ScanDraftInput["coverage"]; result: DeepReductionInput }[]; previous: DeepReductionInput | null; } @@ -22,6 +25,21 @@ export interface ReducerArtifactValidation { result: DeepReductionInput; } +export function deepReductionToScanDraft(result: DeepReductionInput): ScanDraftInput { + const { sourceCoverage, ...draft } = structuredClone(result); + return { ...draft, coverage: sourceCoverage ?? unknownSourceCoverage() }; +} + +/** Older workflow readers reject the host field; retain their persisted shape. */ +export function deepReductionForPersistence( + result: DeepReductionInput, + persistSourceCoverage = false, +): DeepReductionInput { + if (persistSourceCoverage) return result; + const { sourceCoverage: _coverage, ...legacy } = result; + return legacy; +} + /** * Check reducer findings with the Standard scan validator. * It requires coverage, so add an empty value and remove it after validation. @@ -30,8 +48,9 @@ export function parseDeepReduction( input: Record, persisted = false, ): DeepReductionInput { + const { sourceCoverage, ...submitted } = input; const standard = { - ...input, + ...submitted, coverage: { completeness: "complete", surfaces: [], @@ -42,6 +61,9 @@ export function parseDeepReduction( const { coverage: _coverage, ...parsed } = persisted ? parsePersistedScanDraft(standard) : parseScanDraft(standard as unknown as ScanDraftInput); + if (persisted && sourceCoverage !== undefined) { + return { ...parsed, sourceCoverage: parsePersistedScanDraft({ ...standard, coverage: sourceCoverage }).coverage }; + } return parsed; } @@ -70,6 +92,7 @@ export async function validateReducerArtifacts(input: { reducerId: string; previousReducerResultPath?: string; sources?: DeepReductionSources; + persistSourceCoverage?: boolean; }, expectedScanId?: string): Promise { const { artifacts, @@ -101,8 +124,9 @@ export async function validateReducerArtifacts(input: { if (input.sources) { result = reconcileDeepReduction(result, input.sources.discoveries, input.sources.previous); - await saveScanDraftCheckpoint({ root: artifactDir, repoRoot: artifacts.scanDir, layout: "reducer" }, result); - await writeJsonAtomic(resultPath, result); + const persisted = deepReductionForPersistence(result, input.persistSourceCoverage); + await saveScanDraftCheckpoint({ root: artifactDir, repoRoot: artifacts.scanDir, layout: "reducer" }, persisted); + await writeJsonAtomic(resultPath, persisted); } else { validateRetainedFindings(result, [], previous); } @@ -122,6 +146,7 @@ export function reconcileDeepReduction( previous: DeepReductionInput | null, ): DeepReductionInput { const result = structuredClone(input); + result.sourceCoverage = aggregateSourceCoverage(discoveries, previous); if (result.complete === false) throw new Error("Deep reduction is only a checkpoint, not a complete result."); for (const source of [...discoveries.map((discovery) => discovery.result), ...(previous ? [previous] : [])]) { if (source.scanId !== result.scanId) throw new Error("Deep reduction source belongs to a different scan."); @@ -180,6 +205,79 @@ export function reconcileDeepReduction( return result; } +/** Keep independent reviews separate: matching labels do not resolve another pass's proof gap. */ +export function aggregateSourceCoverage( + discoveries: DeepReductionSources["discoveries"], + previous: DeepReductionInput | null, +): ScanDraftInput["coverage"] { + const sources = [ + ...(previous ? [previous.sourceCoverage ?? unknownSourceCoverage()] : []), + ...discoveries.map((source) => source.coverage ?? unknownSourceCoverage()), + ]; + const result: ScanDraftInput["coverage"] = { + completeness: "complete", surfaces: [], explicitExclusions: [], deferred: [], reviews: [], + }; + for (const field of ["surfaces", "explicitExclusions", "deferred", "openQuestions", "reviews"]) { + const entries = sources.flatMap((source) => (source[field] as unknown[] | undefined) ?? []); + if (entries.length || field !== "openQuestions") result[field] = structuredClone(entries); + } + if (sources.some((source) => source.completeness === "partial") + || (result.deferred as unknown[]).length > 0 + || (result.surfaces as Record[]).some((surface) => surface.disposition === "needs_follow_up")) { + result.completeness = "partial"; + } else if (sources.some((source) => source.completeness === "unknown")) { + result.completeness = "unknown"; + } + return result; +} + +function unknownSourceCoverage(): ScanDraftInput["coverage"] { + return { completeness: "unknown", surfaces: [], explicitExclusions: [], deferred: [] }; +} + +/** Qualify worker-local IDs and receipt paths before combining accepted coverage. */ +export function projectDiscoveryCoverage( + coverage: ScanDraftInput["coverage"], + worker: { id: string; attempt?: number }, + artifactPrefix: string, +): ScanDraftInput["coverage"] { + const provenance = { workerId: worker.id, ...(worker.attempt === undefined ? {} : { attempt: worker.attempt }) }; + const prefix = `${worker.id}-attempt-${worker.attempt ?? "unknown"}`; + const surfaces = coverage.surfaces as Record[]; + const surfaceIds = new Map(surfaces.map((surface, index) => [surface.id, `${prefix}-surface-${index + 1}`])); + const project = (item: Record) => ({ + ...structuredClone(item), + provenance: { + ...provenance, + ...(item.id === undefined ? {} : { sourceId: item.id }), + ...(item.candidateId === undefined ? {} : { candidateId: item.candidateId }), + }, + }); + return { + completeness: coverage.completeness, + reviews: [{ ...provenance, completeness: coverage.completeness }], + surfaces: surfaces.map((surface, index) => ({ + ...project(surface), + id: `${prefix}-surface-${index + 1}`, + receiptRefs: ((surface.receiptRefs as string[] | undefined) ?? []).map((ref) => `${artifactPrefix}/${ref}`), + })), + explicitExclusions: (coverage.explicitExclusions as Record[]).map(project), + deferred: (coverage.deferred as Record[]).map((item, index) => ({ + ...project(item), + id: `${prefix}-deferred-${index + 1}`, + ...(item.candidateId === undefined ? {} : { candidateId: `${prefix}-candidate-${index + 1}` }), + ...(item.surfaceIds === undefined ? {} : { + surfaceIds: (item.surfaceIds as string[]).map((id) => surfaceIds.get(id) ?? id), + }), + })), + ...(coverage.openQuestions === undefined ? {} : { + openQuestions: (coverage.openQuestions as (string | Record)[]).map((question) => ( + project(typeof question === "string" ? { question } : question) + )), + }), + }; +} + function findingSourceIds(finding: Record): string[] { const provenance = finding.provenance as Record; const ids = provenance.sourceFindingIds; diff --git a/plugins/codex-security/mcp-app/src/deep-scan/coordinator.ts b/plugins/codex-security/mcp-app/src/deep-scan/coordinator.ts index 905c0c9a8..e479d60eb 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/coordinator.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/coordinator.ts @@ -5,7 +5,8 @@ import { createDeepScanArtifacts, ensureDeepScanDirectories } from "./artifacts.js"; -import { validateDiscoveryArtifacts, validateReducerArtifacts, type DeepReductionInput } from "./artifact-validation.js"; +import { aggregateSourceCoverage, deepReductionToScanDraft, validateDiscoveryArtifacts, validateReducerArtifacts, type DeepReductionInput } from "./artifact-validation.js"; +import { readDeepReductionSources } from "../artifact-deep-reducer.js"; import { scanDraftInputSchema, type ScanDraftInput @@ -293,17 +294,7 @@ export class DeepScanCoordinator { if (this.canceled || this.externallyFailed) return; this.phase = "terminal"; const draft = schedulerResult.result - ? { - ...structuredClone(schedulerResult.result), - // Readers require coverage.json. The coordinator has accepted this - // result, so mark it complete and leave review notes empty. - coverage: { - completeness: "complete", - surfaces: [], - explicitExclusions: [], - deferred: [] - } - } + ? deepReductionToScanDraft(schedulerResult.result) : scanDraftInputSchema.parse({ scanId: this.state.scanId, findings: [], @@ -809,7 +800,8 @@ export class DeepScanCoordinator { id: randomUUID(), label: `dedup-${String(reducerSequence).padStart(4, "0")}`, consumed, - previousReducerResultPath + previousReducerResultPath, + previousSourceCoverage: latestResult?.sourceCoverage, })); observe(reducer); } @@ -1029,6 +1021,22 @@ export class DeepScanCoordinator { reducerId: worker.id, previousReducerResultPath: outcomes.at(-1)?.resultPath }, this.state.scanId); + if (result.sourceCoverage === undefined) { + const context = { + root: worker.artifactDir, + repoRoot: this.state.targetPath, + scanId: this.state.scanId, + layout: "reducer" as const, + deepReducer: { + scanRoot: this.artifacts.scanDir, + claimedWorkers: accepted.map((source) => ({ + id: source.id, resultPath: source.resultPath, attempt: source.attempt, + })), + }, + }; + const sources = await readDeepReductionSources(context); + result.sourceCoverage = aggregateSourceCoverage(sources.discoveries, latestResult ?? null); + } latestResult = result; noNewStreak = newFindings > 0 ? 0 : noNewStreak + accepted.length; const evidence = await persistedWorkerEvidence(worker); diff --git a/plugins/codex-security/mcp-app/src/deep-scan/worker-runner.ts b/plugins/codex-security/mcp-app/src/deep-scan/worker-runner.ts index 82f119979..7b8eaf5cb 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/worker-runner.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/worker-runner.ts @@ -1,7 +1,7 @@ import { createHash } from "node:crypto"; import { promises as fs } from "node:fs"; import { dirname, join } from "node:path"; -import { getCodexSecurityDeepReducerInputs } from "../artifact-deep-reducer.js"; +import { readDeepReductionSources } from "../artifact-deep-reducer.js"; import { validateDiscoveryArtifacts, validateReducerArtifacts @@ -102,6 +102,7 @@ export interface ReducerRequest { label: string; consumed: AcceptedDiscovery[]; previousReducerResultPath?: string; + previousSourceCoverage?: DeepReductionInput["sourceCoverage"]; } export interface DeepScanWorkerRunnerOptions { @@ -295,7 +296,8 @@ export class DeepScanWorkerRunner { id: reducerId, label: reducerLabel, consumed, - previousReducerResultPath + previousReducerResultPath, + previousSourceCoverage } = request; const { artifacts, run } = this.options; const reducerRoot = join(artifacts.dedupRoot, reducerLabel); @@ -326,6 +328,7 @@ export class DeepScanWorkerRunner { count: consumed.length }); + const persistSourceCoverage = "workflowVersion" in run && run.workflowVersion === "deep-security-scan/v2"; const artifactContext = { root: artifactDir, repoRoot: run.targetPath, @@ -333,13 +336,17 @@ export class DeepScanWorkerRunner { layout: "reducer" as const, deepReducer: { scanRoot: artifacts.scanDir, - claimedWorkers: consumed.map((worker) => ({ id: worker.id, resultPath: worker.resultPath })), + persistSourceCoverage, + claimedWorkers: consumed.map((worker) => ({ id: worker.id, resultPath: worker.resultPath, attempt: worker.attempt })), previousReducerResultPath } }; // Snapshot inputs before execution: direct file output has the same // conservation checks as the MCP writer without rereading consumed sources. - const sources = await getCodexSecurityDeepReducerInputs(artifactContext); + const sources = await readDeepReductionSources(artifactContext); + if (sources.previous && previousSourceCoverage !== undefined) { + sources.previous.sourceCoverage = structuredClone(previousSourceCoverage); + } let reducerValidation: ReducerArtifactValidation | undefined; let outcome = await this.runWorkerWithRetries({ workerId: reducerId, @@ -356,7 +363,8 @@ export class DeepScanWorkerRunner { resultPath, reducerId, previousReducerResultPath, - sources + sources, + persistSourceCoverage }, run.scanId); }, beforeRetry: async (attempt) => { diff --git a/plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs b/plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs new file mode 100644 index 000000000..e5bab7e47 --- /dev/null +++ b/plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs @@ -0,0 +1,143 @@ +import assert from "node:assert/strict"; +import { execFile } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import { mkdir, readFile, readdir, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { promisify } from "node:util"; +import { build } from "esbuild"; + +const pluginRoot = fileURLToPath(new URL("../../", import.meta.url)); +const exec = promisify(execFile); +const bundled = await build({ + bundle: true, + stdin: { + contents: [ + 'export { DeepScanCoordinator } from "./src/deep-scan/coordinator.ts";', + 'export { WorkbenchDeepScanStore } from "./src/deep-scan/store.ts";', + 'export { createScanArtifactContext } from "./src/artifact-context.ts";', + 'export { recordCodexSecurityScanDraftViaWorkbench } from "./src/artifact-scan-draft.ts";', + 'export { recordCodexSecurityDeepReduction } from "./src/artifact-deep-reducer.ts";', + ].join("\n"), + resolveDir: path.join(pluginRoot, "mcp-app"), + }, + format: "esm", platform: "node", loader: { ".md": "text" }, write: false, +}); +export async function publishCoverageFixture(root, completeness, { resume = false, continueAfterResume = false } = {}) { + const runtimePath = path.join(root, "fixture-runtime.mjs"); + await writeFile(runtimePath, bundled.outputFiles[0].contents); + const { DeepScanCoordinator, WorkbenchDeepScanStore, createScanArtifactContext, recordCodexSecurityScanDraftViaWorkbench, recordCodexSecurityDeepReduction } = await import(pathToFileURL(runtimePath).href); + const targetPath = path.join(root, "target"); + const codexHome = path.join(root, "codex-home"); + const scanRoot = path.join(root, "scans"); + const threadId = "coverage-fixture-owner"; + const statuses = completeness === "partial" ? ["partial", "complete", "unknown"] + : completeness === "unknown" ? ["unknown", "complete"] : ["complete"]; + await mkdir(scanRoot, { mode: 0o700 }); + await mkdir(targetPath, { recursive: true }); + await mkdir(path.join(codexHome, "codex-security"), { recursive: true }); + await writeFile(path.join(targetPath, "source.py"), "# Synthetic source\n"); + await writeFile(path.join(codexHome, "codex-security", "config.toml"), + `[deep_scan]\nworkers = 1\nsubagents = 0\nstop_after_no_new = ${statuses.length}\nmax_discovery_runs = ${statuses.length}\n`); + const runWorkbench = async (args) => { + const { stdout } = await exec(process.env.PYTHON || "python3", [path.join(pluginRoot, "scripts", "workbench_db.py"), ...args], { + env: { ...process.env, CODEX_HOME: codexHome, CODEX_SECURITY_STATE_DIR: path.join(root, "state") }, + }); + return JSON.parse(stdout); + }; + const store = new WorkbenchDeepScanStore(runWorkbench); + let { run } = await store.begin({ targetPath, scope: ".", threadId, scanRoot }); + const context = await createScanArtifactContext(run.scanId, runWorkbench, { requireRunning: true }); + const rawSources = new Map(); + const writeDiscovery = async (artifactDir, index) => { + const status = statuses[index]; + const pending = completeness === "partial" && status !== "complete"; + const coverage = { + completeness: status, + surfaces: [{ id: "shared-surface", label: "Archive route", disposition: pending ? "needs_follow_up" : "no_issue_found", receiptRefs: ["artifacts/review.md"] }], + explicitExclusions: [{ pattern: "vendor/", reason: "External dependency." }], + deferred: pending ? [{ id: "same-id", candidateId: "candidate-1", reason: index === 0 ? "Verify entry boundaries." : "Verify symbolic links.", paths: ["source.py"], surfaceIds: ["shared-surface"] }] : [], + openQuestions: pending ? [{ question: `Deployment question ${index + 1}.` }] : [], + }; + await mkdir(path.join(artifactDir, "artifacts"), { recursive: true }); + await writeFile(path.join(artifactDir, "artifacts", "review.md"), "Synthetic review evidence.\n"); + const resultPath = path.join(artifactDir, "result.json"); + const bytes = JSON.stringify({ scanId: run.scanId, complete: true, findings: [], coverage }); + await writeFile(resultPath, bytes); + rawSources.set(resultPath, bytes); + }; + if (resume) { + const workers = []; + const seeded = continueAfterResume ? statuses.slice(0, -1) : statuses; + for (const index of seeded.keys()) { + const workerRoot = path.join(run.scanDir, "artifacts", "deep_discovery", "workers", `discovery-${String(index + 1).padStart(4, "0")}`); + const artifactDir = path.join(workerRoot, "output"); + const worker = { id: randomUUID(), scanId: run.scanId, kind: "discovery", promptPath: path.join(workerRoot, "prompt.md"), artifactDir, attempt: index === 0 ? 2 : 1 }; + await writeDiscovery(artifactDir, index); + await writeFile(worker.promptPath, "Synthetic discovery prompt.\n"); + for (const status of ["queued", "running", "succeeded"]) { + await store.updateWorker({ ...worker, status, ...(status === "succeeded" ? { resultManifestPath: path.join(artifactDir, "result.json") } : {}) }); + } + workers.push(worker); + } + const artifactDir = path.join(run.scanDir, "artifacts", "deep_discovery", "dedup", "dedup-0001", "output"); + const promptPath = path.join(path.dirname(artifactDir), "prompt.md"); + await mkdir(artifactDir, { recursive: true }); + await writeFile(promptPath, "Synthetic reducer prompt.\n"); + const id = randomUUID(); + await store.claimDedup({ id, scanId: run.scanId, workerIds: workers.map((worker) => worker.id), artifactDir, promptPath }); + const resultManifestPath = path.join(artifactDir, "result.json"); + // Legacy accepted reducers omitted coverage entirely. + await writeFile(resultManifestPath, JSON.stringify({ scanId: run.scanId, findings: [] })); + rawSources.set(resultManifestPath, await readFile(resultManifestPath, "utf8")); + await store.commitDedup({ id, scanId: run.scanId, newFindings: 0, resultManifestPath }); + run = await store.get(run.scanId, threadId); + } + let discoveryCalls = 0; + const executor = { + async run(request) { + assert.equal(resume && !continueAfterResume, false, "accepted legacy sources should resume without new model work"); + const thread = request.resumeThreadId ?? randomUUID(); + await request.onThreadStarted?.(thread); + if (request.kind === "discovery") { + discoveryCalls++; + const index = Number(path.basename(path.dirname(request.promptPath)).split("-").at(-1)) - 1; + if (index === 0 && !request.resumeThreadId) return { threadId: thread, finalResponse: "Continue the unfinished audit." }; + await writeDiscovery(request.artifactContext.root, index); + } else { + await recordCodexSecurityDeepReduction({ ...request.artifactContext, repoRoot: targetPath, scanId: run.scanId }, { scanId: run.scanId, findings: [] }); + } + return { threadId: thread, finalResponse: "Audit finished." }; + }, + }; + const coordinator = new DeepScanCoordinator({ + run, store, executor, pluginRoot, retryDelaysMs: [1], + onComplete: async (draft, signal) => { + await recordCodexSecurityScanDraftViaWorkbench(context, draft, runWorkbench, signal); + }, + }); + coordinator.start(); + const terminal = await coordinator.wait(undefined, 30_000); + assert.equal(terminal?.status, "succeeded", terminal?.error); + assert.equal(terminal.noNewStreak, statuses.length, "source coverage must not change stopping policy"); + assert.equal(discoveryCalls, resume ? (continueAfterResume ? 1 : 0) : statuses.length + 1); + const accepted = await store.get(run.scanId, threadId); + for (const worker of accepted.persistedWorkers.filter((worker) => worker.kind === "dedup")) { + const result = JSON.parse(await readFile(worker.resultManifestPath, "utf8")); + assert.equal(Object.hasOwn(result, "sourceCoverage"), false, "v1 reducers remain readable by earlier binaries"); + if (!rawSources.has(worker.resultManifestPath)) { + for (const name of await readdir(path.join(worker.artifactDir, "checkpoints"))) { + const checkpoint = JSON.parse(await readFile(path.join(worker.artifactDir, "checkpoints", name), "utf8")); + assert.equal(Object.hasOwn(checkpoint, "sourceCoverage"), false, "v1 checkpoints remain readable by earlier binaries"); + } + } + } + await runWorkbench(["complete-scan", "--scan-id", run.scanId]); + for (const [file, bytes] of rawSources) assert.equal(await readFile(file, "utf8"), bytes); + return { scanDir: run.scanDir, threadId, terminal }; +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + const result = await publishCoverageFixture(process.argv[2], process.argv[3], { resume: process.argv[4] === "true", continueAfterResume: process.argv[5] === "true" }); + process.stdout.write(JSON.stringify(result)); +} diff --git a/plugins/codex-security/mcp-app/tests/deep_scan_publication_cases.mjs b/plugins/codex-security/mcp-app/tests/deep_scan_publication_cases.mjs index 5e44d1796..8a3325d40 100644 --- a/plugins/codex-security/mcp-app/tests/deep_scan_publication_cases.mjs +++ b/plugins/codex-security/mcp-app/tests/deep_scan_publication_cases.mjs @@ -55,7 +55,7 @@ export async function testDeepScanPublication({ assert.deepEqual(completed[0].findings, [], "late worker findings are not appended to the saturated aggregate"); } - async function testSuccessfulDeepCoverageIgnoresWorkerAndReducerReviewStatus() { + async function testSuccessfulDeepCoveragePreservesWorkerReviewStatus() { const fixture = await fixtureRun({ workers: 2, subagents: 0, stopAfterNoNew: 2, maxDiscoveryRuns: 2 }); const store = new FakeStore(fixture.run); const executor = new FakeExecutor(); @@ -87,9 +87,16 @@ export async function testDeepScanPublication({ const terminal = await coordinator.wait(undefined, 5_000); assert.equal(terminal?.status, "succeeded", terminal?.error); assert.equal(completed.length, 1); - assert.deepEqual(completed[0].coverage, { - completeness: "complete", surfaces: [], explicitExclusions: [], deferred: [], - }); + assert.equal(completed[0].coverage.completeness, "partial"); + assert.equal(completed[0].coverage.deferred.length, 2); + assert.equal(completed[0].coverage.surfaces.length, 4); + assert.equal(completed[0].coverage.reviews.length, 2); + assert.deepEqual(new Set(completed[0].coverage.reviews.map((review) => review.completeness)), + new Set(["partial", "unknown"])); + for (const item of completed[0].coverage.deferred) { + assert.equal(item.provenance.attempt, 1); + assert.ok(store.workers.has(item.provenance.workerId)); + } for (const worker of store.workers.values()) { if (worker.kind !== "discovery") continue; const draft = JSON.parse(await readFile(worker.resultManifestPath, "utf8")); @@ -101,6 +108,7 @@ export async function testDeepScanPublication({ async function testSaturationIgnoresDiscoveryCancellationWriteFailure() { const fixture = await fixtureRun({ workers: 2, subagents: 0, stopAfterNoNew: 2, maxDiscoveryRuns: 6 }); + fixture.run.workflowVersion = "deep-security-scan/v2"; const store = new FakeStore(fixture.run); const executor = new FakeExecutor({ blockDedup: true, blockDiscoveryAfterCalls: 2 }); const updateWorker = store.updateWorker.bind(store); @@ -142,7 +150,7 @@ export async function testDeepScanPublication({ )); const { coverage, ...publishedReduction } = completed[0]; assert.deepEqual( - publishedReduction, + { ...publishedReduction, sourceCoverage: coverage }, JSON.parse(await readFile(acceptedReducer.resultManifestPath, "utf8")), "the accepted aggregate still reaches publication when redundant cancellation writes fail", ); @@ -177,7 +185,7 @@ export async function testDeepScanPublication({ } await testSaturationOmitsWorkerAcceptedDuringCancellation(); - await testSuccessfulDeepCoverageIgnoresWorkerAndReducerReviewStatus(); + await testSuccessfulDeepCoveragePreservesWorkerReviewStatus(); await testSaturationIgnoresDiscoveryCancellationWriteFailure(); await testPublicationUsesAcceptedReducerSnapshot(); } diff --git a/plugins/codex-security/mcp-app/tests/test_artifact_deep_reducer.mjs b/plugins/codex-security/mcp-app/tests/test_artifact_deep_reducer.mjs index 5466999cc..c9cb7533d 100644 --- a/plugins/codex-security/mcp-app/tests/test_artifact_deep_reducer.mjs +++ b/plugins/codex-security/mcp-app/tests/test_artifact_deep_reducer.mjs @@ -14,7 +14,8 @@ const bundled = await build({ const { deepReducerInputsInputSchema, deepReductionInputSchema, - getCodexSecurityDeepReducerInputs, + getCodexSecurityDeepReducerInputs: getModelInputs, + readDeepReductionSources: getCodexSecurityDeepReducerInputs, recordCodexSecurityDeepReduction } = await import( "data:text/javascript;base64," @@ -99,6 +100,7 @@ try { layout: "reducer", deepReducer: { scanRoot, + persistSourceCoverage: true, claimedWorkers: [first, second] } }; @@ -127,13 +129,23 @@ try { /evidenceRefs must refer/, "live reducer submissions reject unknown evidence references instead of silently removing them", ); - assert.deepEqual(inputs, { + assert.deepEqual({ ...inputs, discoveries: inputs.discoveries.map(({ coverage, ...source }) => source) }, { discoveries: [ { workerId: first.id, result: withSourceRefs(first) }, { workerId: second.id, result: withSourceRefs(second) } ], previous: null }); + assert.equal(inputs.discoveries[0].coverage.completeness, "partial"); + assert.equal(inputs.discoveries[1].coverage.completeness, "unknown"); + assert.deepEqual(await getModelInputs(context), { + discoveries: inputs.discoveries.map(({ workerId, result }) => ({ workerId, result })), + previous: null, + }, "coverage accounting does not change reducer model inputs"); + assert.deepEqual(inputs.discoveries[0].coverage.deferred[0].provenance, + { workerId: first.id, candidateId: "candidate-upload" }); + assert.deepEqual(inputs.discoveries[0].coverage.surfaces[0].receiptRefs, + ["artifacts/deep_discovery/workers/discovery-0001/output/artifacts/missing-worker-receipt.md"]); assert.equal(JSON.stringify(inputs).includes(root), false); assert.equal(JSON.stringify(inputs).includes("result.json"), false); @@ -158,6 +170,10 @@ try { const outcome = await recordCodexSecurityDeepReduction(context, merged); const mergedWithSources = { ...merged, + sourceCoverage: { + ...inputs.discoveries[0].coverage, + reviews: [...inputs.discoveries[0].coverage.reviews, ...inputs.discoveries[1].coverage.reviews], + }, findings: [ retainedFinding(shared, [{ id: "worker-001:0", finding: shared }, { id: "worker-002:0", finding: shared }]), retainedFinding(independent, [{ id: "worker-002:1", finding: independent }]), @@ -176,7 +192,7 @@ try { assert.deepEqual( JSON.parse(await readFile(path.join(outputRoot, "checkpoints", checkpointNames[0]), "utf8")), mergedWithSources, - "reducer checkpoints retain the accepted findings and scope without coverage", + "reducer checkpoints retain accepted findings, scope and source coverage", ); assert.deepEqual( @@ -234,12 +250,16 @@ try { layout: "reducer", deepReducer: { scanRoot, + persistSourceCoverage: true, claimedWorkers: [third], previousReducerResultPath: path.join(outputRoot, "result.json") } }; const nextInputs = await getCodexSecurityDeepReducerInputs(nextContext); - assert.deepEqual(nextInputs, { + const { sourceCoverage, ...previousModelInput } = mergedWithSources; + assert.deepEqual((await getModelInputs(nextContext)).previous, previousModelInput, + "host coverage metadata is excluded from the previous model input too"); + assert.deepEqual({ ...nextInputs, discoveries: nextInputs.discoveries.map(({ coverage, ...source }) => source) }, { discoveries: [{ workerId: third.id, result: withSourceRefs(third) }], previous: mergedWithSources }); @@ -255,6 +275,10 @@ try { JSON.parse(await readFile(path.join(nextOutputRoot, "result.json"), "utf8")), { ...mergedWithSources, + sourceCoverage: { + ...mergedWithSources.sourceCoverage, + reviews: [...mergedWithSources.sourceCoverage.reviews, ...nextInputs.discoveries[0].coverage.reviews], + }, findings: [ retainedFinding(shared, [ { id: "worker-003:0", finding: shared }, diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_executor.mjs b/plugins/codex-security/mcp-app/tests/test_deep_scan_executor.mjs index 76112891d..3c6de8723 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_executor.mjs +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_executor.mjs @@ -83,6 +83,7 @@ try { await testSdkInvocationAndThreadCapture(); await testBedrockCredentialsReachWorker(); await testArtifactServerUsesExtendedStartupTimeout(); + await testReducerCoveragePersistenceBinding(); await testZeroSubagentsPreservesHostRestrictions(); await testSdkResumesExistingThread(); await testRetryNotificationDoesNotInterruptTurn(); @@ -922,6 +923,43 @@ async function testArtifactServerUsesExtendedStartupTimeout() { } } +async function testReducerCoveragePersistenceBinding() { + const fixture = await fakeCodexFixture(); + const previousPath = process.env.CODEX_CLI_PATH; + process.env.CODEX_CLI_PATH = fixture.executablePath; + try { + const promptPath = path.join(fixture.root, "prompt.md"); + const workingDirectory = path.join(fixture.root, "artifacts"); + await mkdir(workingDirectory); + await writeFile(promptPath, "fixture reducer prompt\n"); + for (const resume of [false, true]) { + for (const persistSourceCoverage of [false, true]) { + const deepReducer = { + scanRoot: path.join(fixture.root, "scans"), + claimedWorkers: [{ id: "worker-1", resultPath: path.join(fixture.root, "worker", "result.json"), attempt: 2 }], + persistSourceCoverage, + }; + await new CodexSdkWorkerExecutor({ + parentSandbox: trustedParentSandbox, + artifactContext: { pluginRoot: fixture.root, scanRoot: deepReducer.scanRoot, repoRoot: fixture.root, scanId: "fixture-scan-id" }, + }).run({ + kind: "dedup", promptPath, workingDirectory, subagents: 0, + signal: new AbortController().signal, + ...(resume ? { resumeThreadId: "fixture-existing-thread", continuationPrompt: "continue the reducer\n" } : {}), + artifactContext: { root: workingDirectory, layout: "reducer", deepReducer }, + }); + const invocation = JSON.parse(await readFile(fixture.markerPath, "utf8")); + const prefix = "mcp_servers.cs_artifacts.env.CODEX_SECURITY_REDUCER_CONTEXT_JSON="; + const encoded = invocation.argv.find((arg) => arg.startsWith(prefix)); + assert.ok(encoded, "the launched reducer receives its host-bound artifact context"); + assert.deepEqual(JSON.parse(JSON.parse(encoded.slice(prefix.length))), deepReducer); + } + } + } finally { + restoreEnv("CODEX_CLI_PATH", previousPath); + } +} + async function testSdkResumesExistingThread() { const fixture = await fakeCodexFixture(); const previousPath = process.env.CODEX_CLI_PATH; diff --git a/plugins/codex-security/scripts/report_projection.py b/plugins/codex-security/scripts/report_projection.py index b98b052e3..cc404e5f8 100644 --- a/plugins/codex-security/scripts/report_projection.py +++ b/plugins/codex-security/scripts/report_projection.py @@ -514,6 +514,9 @@ def _target_scope_lines(target: dict[str, Any]) -> list[str]: def _surface_notes(surface: dict[str, Any]) -> str: notes = surface.get("notes", "No additional canonical notes were recorded.") + source = _coverage_source(surface) + if source: + notes = f"{source}. {notes}" receipt_refs = surface.get("receiptRefs", []) if not isinstance(receipt_refs, list) or not receipt_refs: return _cell(notes) @@ -523,6 +526,16 @@ def _surface_notes(surface: dict[str, Any]) -> str: return _cell(f"{notes} Evidence: {evidence}") +def _coverage_source(item: dict[str, Any]) -> str: + provenance = item.get("provenance", {}) + if not isinstance(provenance, dict) or not provenance.get("workerId"): + return "" + source = f"Review {provenance['workerId']}" + if provenance.get("attempt") is not None: + source += f", attempt {provenance['attempt']}" + return source + + def _remediation_section(finding: dict[str, Any]) -> list[str]: remediation = _text(finding.get("remediation"), "No canonical remediation was recorded.") lines = ["", "#### Remediation", "", remediation] @@ -1033,6 +1046,22 @@ def build_report_markdown( f"[Open the structural hardening portfolio]({hardening_portfolio_path})", ] ) + reviews = coverage.get("reviews", []) + if reviews: + lines.extend( + [ + "", + "## Source Review Coverage", + "", + "| Review | Attempt | Coverage |", + "| --- | --- | --- |", + ] + ) + for review in reviews: + if isinstance(review, dict): + lines.append( + f"| {_cell(review.get('workerId'))} | {_cell(str(review.get('attempt', 'unknown')))} | {_cell(review.get('completeness'))} |" + ) surfaces = coverage.get("surfaces", []) if surfaces: lines.extend( @@ -1070,6 +1099,7 @@ def build_report_markdown( questions.extend( { "question": item.get("reason", "Deferred review requires follow-up."), + "provenance": item.get("provenance", {}), "followUpPrompt": " ".join( ( f"Review deferred unit {item.get('id', 'unknown')} and close its stated proof gap.", @@ -1091,6 +1121,9 @@ def build_report_markdown( if not isinstance(question, dict): continue lines.append(f"- {_text(question.get('question'), 'Unspecified open question.')}") + source = _coverage_source(question) + if source: + lines.append(f" - {_text(source, '')}.") prompt = _text(question.get("followUpPrompt"), "") if prompt: lines.append(f" - Follow-up prompt: {prompt}") diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index a32b96513..703c77168 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -166,9 +166,12 @@ def _read_saved_result( draft = _read_scan_local_json(scan_dir, relative, "Saved scan checkpoint") if draft.get("scanId") != scan_id: raise ContractError("checkpoint belongs to a different scan") - if not isinstance(draft.get("findings"), list) or not isinstance( - draft.get("coverage", {} if kind == "dedup" else None), dict - ): + coverage = ( + draft.get("sourceCoverage", draft.get("coverage", {})) + if kind == "dedup" + else draft.get("coverage") + ) + if not isinstance(draft.get("findings"), list) or not isinstance(coverage, dict): raise ContractError("checkpoint has no semantic findings or coverage") return draft, _digest(draft) @@ -519,6 +522,7 @@ def merge_saved_results( reducer_paths.add(latest_reducer) except ValueError: warnings.append("Skipped a reducer result outside the scan directory.") + accepted_reducer = latest_reducer def checkpoints(directory: str, worker_id: str | None) -> None: for name in _children(scan_dir, directory): @@ -597,9 +601,14 @@ def reducer_output(directory: str, attempt: int, reducer_worker: Any) -> None: if frozen_source_digests is not None and frozen_source_digests[relative] != digest: raise ContractError("checkpoint changed after the scan stopped") source_digests[relative] = digest - # Recovery expects coverage, but reducer results only contain findings - # and context. Add an empty value after hashing the original result. - sources.append((relative, {"coverage": {}, **draft}, worker_id)) + # The host supplies reducer coverage separately from model output. + # Preserve the digest of the original accepted document. + if relative in reducer_paths: + draft = { + **draft, + "coverage": draft.get("sourceCoverage", draft.get("coverage", {})), + } + sources.append((relative, draft, worker_id)) except (ContractError, OSError, ValueError) as exc: if (scan_dir / relative).exists(): warnings.append(f"Preserved unreadable checkpoint {relative}: {exc}") @@ -608,6 +617,15 @@ def reducer_output(directory: str, attempt: int, reducer_worker: Any) -> None: raise ContractError("Frozen stopped-scan checkpoint set is incomplete.") drafts_by_path = {relative: draft for relative, draft, _ in sources} + if "sourceCoverage" not in drafts_by_path.get(accepted_reducer, {}): + accepted_reducer = None + accepted_coverage = drafts_by_path.get(accepted_reducer, {}).get("coverage", {}) + reviewed_attempts = { + (review.get("workerId"), review.get("attempt")) + for review in accepted_coverage.get("reviews", []) + if isinstance(review, dict) + } + workers_by_id = {worker["id"]: worker for worker in workers} latest_reducer_key = ( (reducer["completed_at"] or "", reducer["id"], int(reducer["attempt"] or 0)) if reducer is not None and latest_reducer in drafts_by_path @@ -722,8 +740,19 @@ def valid_finding(value: Any) -> bool: all_sources = ([("parent", parent, None)] if parent else []) + sources current_drafts = ([(None, parent)] if parent else []) + [ - (worker_id, draft) for relative, draft, worker_id in sources if relative in current_results + (worker_id, draft) + for relative, draft, worker_id in sources + if relative in current_results or relative == accepted_reducer ] + + def coverage_candidate(owner: str | None, item: dict[str, Any]) -> tuple[str | None, Any]: + provenance = item.get("provenance") + if owner is None and isinstance(provenance, dict): + return provenance.get("workerId"), provenance.get( + "candidateId", item.get("candidateId") + ) + return owner, item.get("candidateId") + resolved: dict[tuple[str | None, str], str] = {} for owner, draft in current_drafts: for finding in draft["findings"]: @@ -741,7 +770,7 @@ def valid_finding(value: Any) -> bool: and isinstance(item.get("candidateId"), str) and item.get("disposition") in {"reported", "rejected", "not_applicable"} ): - resolved.setdefault((owner, item["candidateId"]), item["disposition"]) + resolved.setdefault(coverage_candidate(owner, item), item["disposition"]) # Only the current parent may claim that another worker finding was absorbed. # A superseded checkpoint must not suppress a newer independent result. for draft in [parent] if parent else []: @@ -955,11 +984,18 @@ def valid_finding(value: Any) -> bool: continue finding_positions[key] = len(findings) findings.append(finding) - if superseded: + worker = workers_by_id.get(worker_id) + reviewed = ( + worker is not None + and worker["status"] == "succeeded" + and worker["merge_state"] == "merged" + and (worker_id, worker["attempt"]) in reviewed_attempts + ) + if reviewed or (superseded and relative != accepted_reducer): continue - for field in ("surfaces", "explicitExclusions", "deferred", "openQuestions"): + for field in ("surfaces", "explicitExclusions", "deferred", "openQuestions", "reviews"): items = draft["coverage"].get(field, []) - if not isinstance(items, list): + if not isinstance(items, list) or (field == "reviews" and not items): continue output = coverage.setdefault(field, []) if not isinstance(output, list): @@ -990,7 +1026,7 @@ def valid_finding(value: Any) -> bool: history.append(copy.deepcopy(finding)) if ( isinstance(item, dict) - and (worker_id, item.get("candidateId")) in resolved + and coverage_candidate(worker_id, item) in resolved and (field == "deferred" or item.get("disposition") == "needs_follow_up") ): continue diff --git a/plugins/codex-security/tests/test_stopped_source_coverage.py b/plugins/codex-security/tests/test_stopped_source_coverage.py new file mode 100644 index 000000000..57b8e6534 --- /dev/null +++ b/plugins/codex-security/tests/test_stopped_source_coverage.py @@ -0,0 +1,167 @@ +from __future__ import annotations + +import copy +import json +from argparse import Namespace + +import pytest +from test_deep_scan_successful_publication import add_worker +from test_deep_scan_successful_publication import publication_scan as publication_scan + + +@pytest.mark.parametrize("host_coverage", [True, False], ids=["accepted-projection", "legacy"]) +@pytest.mark.parametrize("parent_draft", [True, False], ids=["parent-draft", "no-parent"]) +def test_stopped_recovery_preserves_accepted_coverage_without_worker_id_collisions( + workbench_api, workbench_db, publication_scan, host_coverage, parent_draft +): + scan = publication_scan() + (scan.scan_dir / "findings.json").write_text(json.dumps({"findings": []})) + source_coverage = { + "completeness": "partial", + "surfaces": [], + "explicitExclusions": [], + "deferred": [], + "reviews": [], + } + source_files = [] + for disposition in ("needs_follow_up", "rejected"): + result = add_worker(workbench_db, scan) + worker_id = result.parent.name + surface = { + "id": "surface-1", + "candidateId": "candidate-1", + "label": "Independent review", + "disposition": disposition, + "receiptRefs": [], + } + deferred = {"candidateId": "candidate-1", "reason": "Validation remains unresolved."} + coverage = { + "completeness": "partial" if disposition == "needs_follow_up" else "complete", + "surfaces": [surface], + "explicitExclusions": [], + "deferred": [deferred] if disposition == "needs_follow_up" else [], + } + result.write_text( + json.dumps( + {"scanId": scan.scan_id, "complete": True, "findings": [], "coverage": coverage} + ) + ) + source_files.append(result) + prefix = f"{worker_id}-attempt-1" + provenance = {"workerId": worker_id, "attempt": 1, "candidateId": "candidate-1"} + source_coverage["reviews"].append( + {"workerId": worker_id, "attempt": 1, "completeness": coverage["completeness"]} + ) + source_coverage["surfaces"].append( + { + **surface, + "id": f"{prefix}-surface-1", + "provenance": {**provenance, "sourceId": "surface-1"}, + } + ) + if coverage["deferred"]: + source_coverage["deferred"].append( + { + **deferred, + "id": f"{prefix}-deferred-1", + "candidateId": f"{prefix}-candidate-1", + "provenance": provenance, + } + ) + reducer = add_worker(workbench_db, scan) + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_workers SET kind = 'dedup', merge_state = 'none' " + "WHERE result_manifest_path = ?", + (str(reducer),), + ) + aggregate = {"scanId": scan.scan_id, "complete": True, "findings": []} + if host_coverage: + aggregate["sourceCoverage"] = copy.deepcopy(source_coverage) + reducer.write_text(json.dumps(aggregate)) + source_files.append(reducer) + saved_bytes = {path: path.read_bytes() for path in source_files} + if not parent_draft: + for filename in ("scan-manifest.json", "findings.json", "coverage.json"): + (scan.scan_dir / filename).unlink() + + stopped = workbench_api["fail_scan"]( + workbench_db, + Namespace(scan_id=scan.scan_id, claim_token=None, cost_json=None, message="Audit stopped."), + )["scan"] + + assert stopped["progress"]["status"] == "failed" + coverage = json.loads((scan.scan_dir / "coverage.json").read_text()) + assert coverage["completeness"] == "partial" + assert len(coverage["deferred"]) == 2 + assert coverage["deferred"][-1]["id"] == "scan-stopped" + assert len(coverage["surfaces"]) == 2 + if host_coverage: + for field in ("reviews", "surfaces", "deferred"): + assert ( + coverage[field][:-1] if field == "deferred" else coverage[field] + ) == source_coverage[field] + else: + assert coverage["deferred"][0]["candidateId"] == "candidate-1" + manifest = (scan.scan_dir / "scan-manifest.json").read_bytes() + workbench_api["preserve_scan_results"]( + workbench_db, + Namespace( + scan_id=scan.scan_id, claim_token=None, thread_id=None, coordinator_generation=None + ), + ) + assert (scan.scan_dir / "scan-manifest.json").read_bytes() == manifest + assert all(path.read_bytes() == contents for path, contents in saved_bytes.items()) + + +def test_stopped_recovery_keeps_unmerged_coverage_after_accepted_review( + workbench_api, workbench_db, publication_scan +): + scan = publication_scan() + (scan.scan_dir / "findings.json").write_text(json.dumps({"findings": []})) + accepted = add_worker(workbench_db, scan) + accepted.write_text( + json.dumps( + {"scanId": scan.scan_id, "complete": True, "findings": [], "coverage": scan.coverage} + ) + ) + reducer = add_worker(workbench_db, scan) + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_workers SET kind = 'dedup', merge_state = 'none' " + "WHERE result_manifest_path = ?", + (str(reducer),), + ) + reviews = [{"workerId": accepted.parent.name, "attempt": 1, "completeness": "complete"}] + reducer.write_text( + json.dumps( + { + "scanId": scan.scan_id, + "complete": True, + "findings": [], + "sourceCoverage": {**scan.coverage, "reviews": reviews}, + } + ) + ) + pending = add_worker(workbench_db, scan, status="canceled") + deferred = {"id": "pending-review", "reason": "The independent review remains unresolved."} + pending.write_text( + json.dumps( + { + "scanId": scan.scan_id, + "complete": False, + "findings": [], + "coverage": {**scan.coverage, "completeness": "partial", "deferred": [deferred]}, + } + ) + ) + + workbench_api["fail_scan"]( + workbench_db, + Namespace(scan_id=scan.scan_id, claim_token=None, cost_json=None, message="Audit stopped."), + ) + + coverage = json.loads((scan.scan_dir / "coverage.json").read_text()) + assert coverage["completeness"] == "partial" + assert coverage["reviews"] == reviews + assert deferred in coverage["deferred"] diff --git a/sdk/typescript/tests-ts/deep-scan-coverage.test.ts b/sdk/typescript/tests-ts/deep-scan-coverage.test.ts new file mode 100644 index 000000000..4d4996bad --- /dev/null +++ b/sdk/typescript/tests-ts/deep-scan-coverage.test.ts @@ -0,0 +1,114 @@ +import { mkdir, mkdtemp, readFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { expect, test } from "bun:test"; +import { main } from "../src/cli.js"; +import { loadContract } from "../src/contract.js"; +import { ScanResult } from "../src/result.js"; +import { capture, dependencies } from "./cli-fixtures.js"; + +const fixtureUrl = new URL( + "../../../plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs", + import.meta.url, +); + +test.each([ + ["partial", false, false], + ["unknown", false, false], + ["complete", false, false], + ["partial", true, false], + ["partial", true, true], +] as const)( + "publishes %s source coverage through CLI results (resume: %p, continued: %p)", + async (completeness, resume, continueAfterResume) => { + const root = await mkdtemp(join(tmpdir(), "deep-coverage-publication-")); + try { + await mkdir(join(root, "fixture"), { mode: 0o700 }); + // Keep the real workbench outside other suites' persistent module mocks. + const child = Bun.spawn( + [ + Bun.which("node")!, + fileURLToPath(fixtureUrl), + join(root, "fixture"), + completeness, + String(resume), + String(continueAfterResume), + ], + { stdout: "pipe", stderr: "pipe" }, + ); + const [output, errors, exitCode] = await Promise.all([ + new Response(child.stdout).text(), + new Response(child.stderr).text(), + child.exited, + ]); + expect(exitCode, errors).toBe(0); + const { scanDir, threadId, terminal } = JSON.parse(output); + const contract = await loadContract(scanDir, { + pluginRoot: fileURLToPath( + new URL("../../../plugins/codex-security/", import.meta.url), + ), + }); + const result = new ScanResult({ + ...contract, + scanDir, + threadId, + turnResult: { status: "completed" }, + }); + expect(result.coverage.completeness).toBe(completeness); + const coverage = JSON.parse( + await readFile(join(scanDir, "coverage.json"), "utf8"), + ); + expect(coverage.reviews[0].attempt).toBe(2); + const report = await readFile(join(scanDir, "report.md"), "utf8"); + expect(report).toContain(`| Coverage | ${completeness} |`); + expect(coverage.explicitExclusions).toHaveLength(coverage.reviews.length); + for (const review of coverage.reviews) + expect(report).toContain(review.workerId); + if (completeness === "partial") { + expect( + coverage.deferred.map((item: { reason: string }) => item.reason), + ).toEqual(["Verify entry boundaries.", "Verify symbolic links."]); + expect( + new Set(coverage.deferred.map((item: { id: string }) => item.id)) + .size, + ).toBe(2); + expect( + coverage.reviews.map( + (review: { completeness: string }) => review.completeness, + ), + ).toEqual(["partial", "complete", "unknown"]); + for (const item of coverage.deferred) { + expect(item.provenance.candidateId).toBe("candidate-1"); + expect(report).toContain(item.reason); + expect( + coverage.surfaces.some((surface: { id: string }) => + item.surfaceIds.includes(surface.id), + ), + ).toBe(true); + } + } + for (const surface of coverage.surfaces) { + expect( + await readFile(join(scanDir, surface.receiptRefs[0]), "utf8"), + ).toContain("Synthetic review evidence."); + } + const stdout = capture(); + const stderr = capture(); + expect( + await main( + ["scan", "--mode", "deep", "--json"], + stdout.stream, + stderr.stream, + dependencies({ result, onWorkbench: () => ({ deepScan: terminal }) }), + ), + ).toBe(completeness === "complete" ? 0 : 2); + expect(JSON.parse(stdout.text()).coverage).toEqual(coverage); + if (completeness !== "complete") + expect(stderr.text()).toContain("STOPPED"); + } finally { + await rm(root, { recursive: true, force: true }); + } + }, + 60_000, +); From f3a30dc81b5afce390f1496ca57404737af368ad Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Sat, 12 Sep 2026 20:05:17 +0000 Subject: [PATCH 02/14] Test versioned reducer coverage snapshots --- .../tests/test_deep_scan_artifact_validation.mjs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_artifact_validation.mjs b/plugins/codex-security/mcp-app/tests/test_deep_scan_artifact_validation.mjs index 2b0bd4391..74ad7e755 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_artifact_validation.mjs +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_artifact_validation.mjs @@ -217,8 +217,9 @@ async function testReducerValidation(root) { discoveries: [{ workerId: first.id, result: draft([firstFinding, secondFinding]) }], previous: null, }; - const validateSnapshot = () => validateReducerArtifacts({ + const validateSnapshot = (persistSourceCoverage = false) => validateReducerArtifacts({ artifacts, artifactDir, resultPath, reducerId: "dedup-0001", sources, + persistSourceCoverage, }, scanId); await assert.rejects(validateSnapshot(), /unaccounted source findings/); await writeResult(resultPath, draft([firstFinding, secondFinding])); @@ -226,11 +227,15 @@ async function testReducerValidation(root) { const validatedSnapshot = await validateSnapshot(); assert.equal(validatedSnapshot.newFindings, 2); const admitted = JSON.parse(await readFile(resultPath, "utf8")); + const { sourceCoverage, ...legacySnapshot } = validatedSnapshot.result; + assert.equal(sourceCoverage.completeness, "unknown"); assert.deepEqual( - validatedSnapshot.result, + legacySnapshot, admitted, - "validation returns the same reconciled result that was accepted on disk", + "v1 preserves host coverage in memory while retaining the legacy persisted shape", ); + const versionedSnapshot = await validateSnapshot(true); + assert.deepEqual(versionedSnapshot.result, JSON.parse(await readFile(resultPath, "utf8")), "v2 persists the full host projection"); assert.equal(Object.hasOwn(admitted, "coverage"), false); assert.deepEqual(admitted.findings[1].provenance.sourceFindingIds, ["worker-001:1"]); sources.previous = structuredClone(admitted); From f5adb33ab7d1f86b252d703a8af45d47de7f9486 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 15 Sep 2026 22:21:43 -0700 Subject: [PATCH 03/14] chore(release): 0.1.28 (#880) Update the package version to 0.1.28 and refresh release notes. --- .github/release-notes.md | 70 ++++++++++++++++++++--------------- .github/release-pr-state.json | 6 +-- sdk/typescript/package.json | 2 +- 3 files changed, 44 insertions(+), 34 deletions(-) diff --git a/.github/release-notes.md b/.github/release-notes.md index 8cdc8029b..474eb2725 100644 --- a/.github/release-notes.md +++ b/.github/release-notes.md @@ -1,37 +1,47 @@ - + ## Highlights -- port source and test report checks to TypeScript ([#768](https://github.com/openai/codex-security/pull/768)) -- port custom validation to TypeScript ([#792](https://github.com/openai/codex-security/pull/792)) -- add Python-free Unix OS primitives ([#794](https://github.com/openai/codex-security/pull/794)) -- add Python-free Windows OS primitives ([#795](https://github.com/openai/codex-security/pull/795)) -- add Python-free musl native artifacts ([#796](https://github.com/openai/codex-security/pull/796)) -- bundle verified native runtime artifacts ([#797](https://github.com/openai/codex-security/pull/797)) -- preserve Windows filenames in native helpers ([#798](https://github.com/openai/codex-security/pull/798)) -- port security policy resolution to TypeScript ([#799](https://github.com/openai/codex-security/pull/799)) -- budget package installation and verification ([#834](https://github.com/openai/codex-security/pull/834)) -- open ready pull requests ([#833](https://github.com/openai/codex-security/pull/833)) -- draft SECURITY.md for owner review ([#536](https://github.com/openai/codex-security/pull/536)) -- resolve Windows Node to an absolute executable ([#788](https://github.com/openai/codex-security/pull/788)) -- shard automatic component planning for large repositories ([#845](https://github.com/openai/codex-security/pull/845)) -- import CSV and JSON findings as saved scans ([#850](https://github.com/openai/codex-security/pull/850)) -- add a feedback command ([#854](https://github.com/openai/codex-security/pull/854)) -- improve scan usage and cost reports ([#853](https://github.com/openai/codex-security/pull/853)) -- recover interrupted Deep Scans and bulk campaigns ([#835](https://github.com/openai/codex-security/pull/835)) -- reject owned plugin keys inside Codex profiles ([#861](https://github.com/openai/codex-security/pull/861)) -- include CommonJS and TypeScript module extensions in scan inventories ([#859](https://github.com/openai/codex-security/pull/859)) -- include workflow files in diff inventories ([#820](https://github.com/openai/codex-security/pull/820)) -- accept a UTF-8 byte order mark in imported findings JSON ([#856](https://github.com/openai/codex-security/pull/856)) -- authenticate deep workers with OpenAI API keys ([#870](https://github.com/openai/codex-security/pull/870)) -- drop canonical document size limits that do not exist ([#868](https://github.com/openai/codex-security/pull/868)) -- pin native Rust formatting edition ([#863](https://github.com/openai/codex-security/pull/863)) -- detect a Linear URL contradiction over plain HTTP ([#867](https://github.com/openai/codex-security/pull/867)) -- pipeline dedupe with configurable concurrency ([#852](https://github.com/openai/codex-security/pull/852)) -- disable reasoning summaries by default for Bedrock ([#869](https://github.com/openai/codex-security/pull/869)) -- reuse scan authentication for patch and validation ([#871](https://github.com/openai/codex-security/pull/871)) -- bump smol-toml from 1.6.1 to 1.7.1 in /sdk/typescript ([#873](https://github.com/openai/codex-security/pull/873)) +- use versioned conventional commit titles ([#879](https://github.com/openai/codex-security/pull/879)) +- share scan settings across the CLI and SDK ([#742](https://github.com/openai/codex-security/pull/742)) +- keep parameterized JUnit names unique ([#877](https://github.com/openai/codex-security/pull/877)) +- accept large saved post-scan prompts ([#876](https://github.com/openai/codex-security/pull/876)) +- make cost display optional ([#881](https://github.com/openai/codex-security/pull/881)) +- collect Desktop and worker session logs ([#872](https://github.com/openai/codex-security/pull/872)) +- skip unavailable Daybreak access advisories ([#878](https://github.com/openai/codex-security/pull/878)) +- align Codex 0.154.0 and dependency cooldowns ([#755](https://github.com/openai/codex-security/pull/755)) +- bump docker/setup-buildx-action from 4.2.0 to 4.3.0 ([#891](https://github.com/openai/codex-security/pull/891)) +- bump actions/attest-build-provenance from 4.1.1 to 4.2.2 ([#886](https://github.com/openai/codex-security/pull/886)) +- update setuptools requirement from \>=64.0 to \>=84.0.0 in /plugins/codex-security ([#889](https://github.com/openai/codex-security/pull/889)) +- bump SocketDev/action from 1.3.0 to 1.3.2 ([#884](https://github.com/openai/codex-security/pull/884)) +- bump ruff from 0.16.1 to 0.16.6 in /plugins/codex-security ([#887](https://github.com/openai/codex-security/pull/887)) +- bump typescript from 6.0.3 to 7.0.2 in /plugins/codex-security/mcp-app ([#894](https://github.com/openai/codex-security/pull/894)) +- bump pytest from 9.0.3 to 9.1.1 in /plugins/codex-security ([#885](https://github.com/openai/codex-security/pull/885)) +- bump typescript from 5.7.3 to 7.0.2 in /sdk/typescript ([#898](https://github.com/openai/codex-security/pull/898)) +- simplify fast-uri dependency and update Ajv resolver ([#895](https://github.com/openai/codex-security/pull/895)) +- bump @linear/sdk from 89.0.0 to 93.0.1 in /sdk/typescript ([#896](https://github.com/openai/codex-security/pull/896)) +- bump actions/checkout from 6.0.2 to 7.0.1 ([#890](https://github.com/openai/codex-security/pull/890)) +- bump actions/setup-node from 6.3.0 to 7.0.0 ([#888](https://github.com/openai/codex-security/pull/888)) +- bump @types/node from 22.19.17 to 26.4.1 in /sdk/typescript ([#897](https://github.com/openai/codex-security/pull/897)) +- bump @types/node from 25.9.1 to 26.4.1 in /plugins/codex-security/mcp-app ([#893](https://github.com/openai/codex-security/pull/893)) +- bump the third-party group across 3 directories with 19 updates ([#892](https://github.com/openai/codex-security/pull/892)) +- report patch failures and changed files ([#874](https://github.com/openai/codex-security/pull/874)) +- persist scan artifacts through MCP ([#862](https://github.com/openai/codex-security/pull/862)) +- advance scan progress when saving drafts ([#882](https://github.com/openai/codex-security/pull/882)) +- simplify scan runtime bookkeeping ([#903](https://github.com/openai/codex-security/pull/903)) +- upgrade json-schema-to-typescript to 16.0.0 ([#914](https://github.com/openai/codex-security/pull/914)) +- upgrade eval OpenCode SDK to 1.18.29 ([#911](https://github.com/openai/codex-security/pull/911)) +- upgrade Ink and use complete Escape input in TUI tests ([#913](https://github.com/openai/codex-security/pull/913)) +- upgrade pnpm setup and align the package-manager pin ([#910](https://github.com/openai/codex-security/pull/910)) +- upgrade actions/setup-python to 7.0.0 ([#909](https://github.com/openai/codex-security/pull/909)) +- upgrade Stryker with compatible mutation tooling ([#912](https://github.com/openai/codex-security/pull/912)) +- limit fix-finding to security vulnerabilities ([#923](https://github.com/openai/codex-security/pull/923)) +- accept text knowledge-base files with any extension ([#924](https://github.com/openai/codex-security/pull/924)) +- declare native release workflow permissions ([#925](https://github.com/openai/codex-security/pull/925)) +- report context-aware estimate ranges ([#926](https://github.com/openai/codex-security/pull/926)) +- update vulnerable image and archive dependencies ([#927](https://github.com/openai/codex-security/pull/927)) +- update Inquirer prompts and Node/Bun types ([#929](https://github.com/openai/codex-security/pull/929)) diff --git a/.github/release-pr-state.json b/.github/release-pr-state.json index 38b02e356..be977fc9c 100644 --- a/.github/release-pr-state.json +++ b/.github/release-pr-state.json @@ -1,9 +1,9 @@ { - "baseVersion": "0.1.26", - "baseCommit": "2536d104deef9bca8ced84c6f6263b915418253b", + "baseVersion": "0.1.27", + "baseCommit": "c40d059935592adc1ae04119eb7ad5286d9de554", "sections": { "highlights": { - "generatedHash": "488ce0a2eadd5a3718a22b68227f42dbba6f3f57b1ad9259cc850c397318a977", + "generatedHash": "d0496c2cdec032d1ca5d28e8ae63b650918b93ebe458c76328734d47edc970d4", "humanOwned": false }, "upgrades": { diff --git a/sdk/typescript/package.json b/sdk/typescript/package.json index 40e5c46af..3b34e7201 100644 --- a/sdk/typescript/package.json +++ b/sdk/typescript/package.json @@ -1,6 +1,6 @@ { "name": "@openai/codex-security", - "version": "0.1.27", + "version": "0.1.28", "description": "TypeScript SDK and CLI for Codex Security", "license": "Apache-2.0", "author": "OpenAI", From 480b8a341488039b55b4723213a34ffa0e203191 Mon Sep 17 00:00:00 2001 From: mldangelo-oai Date: Tue, 15 Sep 2026 22:42:02 -0700 Subject: [PATCH 04/14] fix(logs): stream large saved scan JSON output (#932) * fix(logs): stream large saved scan JSON output * test(logs): isolate installed skill state * test(logs): simplify saved log fixtures --- sdk/typescript/scripts/check-package.mjs | 1 + sdk/typescript/src/cli-scan-logs-json.ts | 31 ++ sdk/typescript/src/cli.ts | 41 ++- sdk/typescript/tests-ts/cli-scan-logs.test.ts | 319 ++++++++++++++++++ .../tests-ts/support/cli-logs-large.mts | 92 +++++ 5 files changed, 477 insertions(+), 7 deletions(-) create mode 100644 sdk/typescript/src/cli-scan-logs-json.ts create mode 100644 sdk/typescript/tests-ts/cli-scan-logs.test.ts create mode 100644 sdk/typescript/tests-ts/support/cli-logs-large.mts diff --git a/sdk/typescript/scripts/check-package.mjs b/sdk/typescript/scripts/check-package.mjs index f78271374..04d8c56e2 100644 --- a/sdk/typescript/scripts/check-package.mjs +++ b/sdk/typescript/scripts/check-package.mjs @@ -170,6 +170,7 @@ const distFiles = new Set( "auth", "bulk-scan-discovery", "cli", + "cli-scan-logs-json", "classify-severity", "classify-scan-severity", "severity-store", diff --git a/sdk/typescript/src/cli-scan-logs-json.ts b/sdk/typescript/src/cli-scan-logs-json.ts new file mode 100644 index 000000000..8bceaacc5 --- /dev/null +++ b/sdk/typescript/src/cli-scan-logs-json.ts @@ -0,0 +1,31 @@ +import { Formatter } from "incur"; +import type { readSavedScanLogs } from "./scan-logs.js"; + +// Format records separately: a saved log document can exceed the runtime's +// maximum string length even though each session and event fits comfortably. +export async function* scanLogsJson( + logs: Awaited>, + cta?: unknown, +): AsyncGenerator { + yield Buffer.from( + `{\n "scanId": ${JSON.stringify(logs.scanId)},\n "threadId": ${JSON.stringify(logs.threadId)}`, + ); + for (const key of ["sessions", "events"] as const) { + yield Buffer.from(`,\n "${key}": [`); + let first = true; + for (const record of logs[key]) { + const formatted = Formatter.format(record, "json").replaceAll( + "\n", + "\n ", + ); + yield Buffer.from(`${first ? "\n" : ",\n"} ${formatted}`); + first = false; + } + yield Buffer.from(first ? "]" : "\n ]"); + } + if (cta !== undefined) { + const formatted = Formatter.format(cta, "json").replaceAll("\n", "\n "); + yield Buffer.from(`,\n "cta": ${formatted}`); + } + yield Buffer.from("\n}\n"); +} diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index e5bccc3bf..d369b3ead 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -45,6 +45,7 @@ import { pipeline } from "node:stream/promises"; import { fileURLToPath, pathToFileURL } from "node:url"; import { promisify, stripVTControlCharacters } from "node:util"; import { Cli, z } from "incur"; +import { scanLogsJson } from "./cli-scan-logs-json.js"; import { parse as parseToml } from "smol-toml"; import { classifyConnectionFailure, @@ -1777,6 +1778,7 @@ export async function main( let exitCode = 0; let frameworkExit: number | undefined; let frameworkOutput = ""; + let streamedLogs: Awaited> | undefined; let renderedHistory: string | undefined; let renderedPublication: string | undefined; let renderedPolicy: string | undefined; @@ -2124,18 +2126,33 @@ export async function main( .describe("Scan identifier or unique prefix (default: latest)."), }), output: z.record(z.string(), z.unknown()).optional(), - async run({ args }) { + async run({ args, format }) { const scanId = args.scanId ?? (await latestScans(1, "any"))?.[0]?.scanId; if (scanId === undefined) return; - return await history( + const result = await history( ["get-scan", "--scan-id", scanId], - async (value) => - (await readSavedScanLogs( + async (value) => { + const logs = await readSavedScanLogs( value["scan"] as ScanLogSource, codexSecurityCredentialHome(dependencies.environment), - )) as unknown as JsonObject, + ); + // Incur owns filtering, envelopes and token controls. Keep those + // requests on its formatter; plain JSON needs no aggregate string. + if ( + format === "json" && + !argv.some((argument) => + /^--(?:filter-output|full-output|token-count|token-limit|token-offset)(?:=|$)/u.test( + argument, + ), + ) + ) { + streamedLogs = logs; + } + return logs as unknown as JsonObject; + }, ); + return streamedLogs === undefined ? result : undefined; }, }) .command("resume", { @@ -5686,11 +5703,21 @@ export async function main( return 2; } } - if (frameworkOutput.length === 0) return exitCode; + if (frameworkOutput.length === 0 && streamedLogs === undefined) + return exitCode; try { + // Incur can add a stale-skills CTA after the logs handler returns. + const logOutput = + streamedLogs === undefined + ? undefined + : scanLogsJson( + streamedLogs, + frameworkOutput ? JSON.parse(frameworkOutput).cta : undefined, + ); await writeCliOutput( output, - renderedPolicy ?? + logOutput ?? + renderedPolicy ?? renderedPatch ?? renderedPublication ?? renderedHistory ?? diff --git a/sdk/typescript/tests-ts/cli-scan-logs.test.ts b/sdk/typescript/tests-ts/cli-scan-logs.test.ts new file mode 100644 index 000000000..04f31c44d --- /dev/null +++ b/sdk/typescript/tests-ts/cli-scan-logs.test.ts @@ -0,0 +1,319 @@ +import { execFile } from "node:child_process"; +import { mkdir, mkdtemp, realpath, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { Writable } from "node:stream"; +import { promisify } from "node:util"; +import { pathToFileURL } from "node:url"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { Cli, Formatter, z } from "incur"; +import { main } from "../src/cli.js"; +import { scanLogsJson } from "../src/cli-scan-logs-json.js"; +import { readSavedScanLogs } from "../src/scan-logs.js"; +import { VERSION } from "../src/version.js"; +import { capture, dependencies } from "./cli-fixtures.js"; + +async function fixture() { + const state = await realpath(await mkdtemp(join(tmpdir(), "saved-logs-"))); + const home = join(state, "codex-home"); + await mkdir(join(home, "sessions"), { recursive: true }); + const events = [ + { type: "session_meta", payload: { id: "thread-1" } }, + { + type: "event_msg", + payload: { + message: 'full values: \" \\ \n \u0000 😀', + nested: [null, false, 2], + }, + }, + ]; + await writeFile( + join(home, "sessions", "rollout.jsonl"), + events.map((event) => JSON.stringify(event)).join("\n"), + ); + const scan = { scanId: "scan-1", continuationThreadId: "thread-1" }; + const logs = await readSavedScanLogs(scan, home); + const deps = dependencies({ + environment: { CODEX_SECURITY_STATE_DIR: state }, + onWorkbench: () => ({ scan }), + }); + deps.createSecurity = () => { + throw new Error("Reading logs must not start Codex"); + }; + return { state, logs, deps }; +} + +async function referenceOutput(args: string[], logs: unknown) { + let text = ""; + await Cli.create("codex-security", { version: VERSION, update: false }) + .command( + Cli.create("scans").command("logs", { + args: z.object({ scanId: z.string() }), + run: () => logs, + }), + ) + .serve(["scans", "logs", "scan-1", ...args], { + stdout: (value) => { + text += value; + }, + }); + return text; +} + +function withoutDuration(text: string) { + const value = JSON.parse(text); + value.meta.duration = "duration"; + return value; +} + +describe("saved logs JSON output", () => { + let dataHome: string; + let previousDataHome: string | undefined; + + beforeEach(async () => { + previousDataHome = process.env["XDG_DATA_HOME"]; + dataHome = await mkdtemp(join(tmpdir(), "saved-logs-data-")); + process.env["XDG_DATA_HOME"] = dataHome; + }); + + afterEach(async () => { + if (previousDataHome === undefined) delete process.env["XDG_DATA_HOME"]; + else process.env["XDG_DATA_HOME"] = previousDataHome; + await rm(dataHome, { recursive: true, force: true }); + }); + + test("preserves the stale installed-skills CTA after saved logs", async () => { + const f = await fixture(); + try { + const skillPath = join(f.state, "skills", "codex-security-scans"); + await mkdir(join(dataHome, "incur"), { recursive: true }); + await mkdir(skillPath, { recursive: true }); + await writeFile( + join(skillPath, "SKILL.md"), + "Previously installed skill.", + ); + await writeFile( + join(dataHome, "incur", "codex-security.json"), + JSON.stringify({ + hash: "previous-command-hash", + skills: ["codex-security-scans"], + paths: [skillPath], + }), + ); + for (const args of [ + ["--json"], + ["--format", "json"], + ["--format=json"], + ]) { + const stdout = capture(); + const stderr = capture(); + expect( + await main( + ["scans", "logs", "scan-1", ...args], + stdout.stream, + stderr.stream, + f.deps, + ), + ).toBe(0); + const expected = await referenceOutput(["--json"], f.logs); + expect(Object.keys(JSON.parse(expected))).toEqual([ + "scanId", + "threadId", + "sessions", + "events", + "cta", + ]); + expect(stdout.text()).toBe(expected); + expect(stderr.text()).toBe(""); + } + } finally { + await rm(f.state, { recursive: true, force: true }); + } + }); + + test.each( + [ + [], + ["--json"], + ["--format", "json"], + ["--format=json"], + ["--format", "toon"], + ["--format", "jsonl"], + ["--format", "yaml"], + ["--format", "md"], + ["--json", "--format", "toon"], + ["--format", "toon", "--json"], + ["--json", "--filter-output", "scanId"], + ["--json", "--filter-output", "events[0,1]"], + ["--json", "--filter-output", "missing"], + ["--json", "--filter-output", "scanId,threadId"], + ["--json", "--token-count"], + ["--json", "--token-limit", "4"], + ["--json", "--token-offset", "1", "--token-limit", "4"], + ["--json", "--token-offset", "999999"], + ["--json", "--token-limit", "999999"], + ["--json", "--full-output"], + ["--json", "--full-output", "--filter-output", "scanId"], + ["--json", "--full-output", "--token-limit", "4"], + ["--json", "--full-output", "--token-count"], + ].map((args) => [args]), + )("preserves Incur output for %j", async (args) => { + const f = await fixture(); + try { + const stdout = capture(); + const stderr = capture(); + expect( + await main( + ["scans", "logs", "scan-1", ...args], + stdout.stream, + stderr.stream, + f.deps, + ), + ).toBe(0); + const expected = await referenceOutput( + args.flatMap((arg) => + arg === "--format=json" ? ["--format", "json"] : [arg], + ), + f.logs, + ); + if (args.includes("--full-output") && !args.includes("--token-count")) { + expect(withoutDuration(stdout.text())).toEqual( + withoutDuration(expected), + ); + } else { + expect(stdout.text()).toBe(expected); + } + expect(stderr.text()).toBe(""); + } finally { + await rm(f.state, { recursive: true, force: true }); + } + }); + + test.each( + [ + ["--format", "invalid"], + ["--format=invalid"], + ["--json", "--token-limit", "invalid"], + ["--json", "--token-offset", "invalid"], + ["--json", "--filter-output"], + ].map((args) => [args]), + )("preserves invalid-option failure for %j", async (args) => { + const stdout = capture(); + const stderr = capture(); + expect( + await main( + ["scans", "logs", "scan-1", ...args], + stdout.stream, + stderr.stream, + dependencies(), + ), + ).toBe(2); + expect(stdout.text()).toBe(""); + expect(stderr.text()).not.toBe(""); + }); + + test("formats empty session and event lists", async () => { + const logs = { + scanId: "scan-1", + threadId: "thread-1", + sessions: [], + events: [], + }; + const chunks = []; + for await (const chunk of scanLogsJson(logs)) chunks.push(chunk); + expect(Buffer.concat(chunks).toString()).toBe( + `${Formatter.format(logs, "json")}\n`, + ); + }); + + test("waits for output backpressure and leaves the stream open", async () => { + const f = await fixture(); + try { + const chunks: Buffer[] = []; + const output = new Writable({ + highWaterMark: 1, + write(chunk, _encoding, callback) { + setImmediate(() => { + chunks.push(Buffer.from(chunk)); + callback(); + }); + }, + }); + expect( + await main( + ["scans", "logs", "scan-1", "--json"], + output, + capture().stream, + f.deps, + ), + ).toBe(0); + expect(Buffer.concat(chunks).toString()).toBe( + `${Formatter.format(f.logs, "json")}\n`, + ); + expect(output.writableEnded).toBe(false); + expect(output.listenerCount("error")).toBe(0); + } finally { + await rm(f.state, { recursive: true, force: true }); + } + }); + + test("reports a failed output write", async () => { + const f = await fixture(); + try { + const stderr = capture(); + const output = new Writable({ + write(_chunk, _encoding, callback) { + callback(new Error("synthetic write failure")); + }, + }); + expect( + await main( + ["scans", "logs", "scan-1", "--json"], + output, + stderr.stream, + f.deps, + ), + ).toBe(2); + expect(stderr.text()).toContain("synthetic write failure"); + } finally { + await rm(f.state, { recursive: true, force: true }); + } + }); + + test("writes a complete document larger than Node's maximum string", async () => { + // Keep the bundle beneath the SDK so external dependencies resolve normally. + const bundle = await mkdtemp( + join(import.meta.dir, "..", ".logs-boundary-"), + ); + const state = await mkdtemp(join(tmpdir(), "large-saved-logs-")); + try { + const built = await Bun.build({ + entrypoints: [join(import.meta.dir, "support", "cli-logs-large.mts")], + outdir: bundle, + target: "node", + packages: "external", + }); + expect(built.success).toBe(true); + const result = await promisify(execFile)( + "node", + [ + "--input-type=module", + "--eval", + `await import(${JSON.stringify(pathToFileURL(built.outputs[0]!.path).href)})`, + "synthetic-launcher", + state, + ], + { encoding: "utf8" }, + ); + const proof = JSON.parse(result.stdout); + expect(proof.exitCode).toBe(0); + expect(proof.bytes).toBeGreaterThan(proof.maximumStringLength); + expect(proof.bytes).toBe(proof.expectedBytes); + expect(proof.sha256).toBe(proof.expectedSha256); + expect(result.stderr).toBe(""); + } finally { + await rm(bundle, { recursive: true, force: true }); + await rm(state, { recursive: true, force: true }); + } + }); +}); diff --git a/sdk/typescript/tests-ts/support/cli-logs-large.mts b/sdk/typescript/tests-ts/support/cli-logs-large.mts new file mode 100644 index 000000000..2ac765fbd --- /dev/null +++ b/sdk/typescript/tests-ts/support/cli-logs-large.mts @@ -0,0 +1,92 @@ +import { constants } from "node:buffer"; +import { createHash } from "node:crypto"; +import { mkdir, open, realpath } from "node:fs/promises"; +import { join } from "node:path"; +import { Writable } from "node:stream"; +import { main } from "../../src/cli.js"; +import { dependencies } from "../cli-fixtures.js"; + +const state = await realpath(process.argv[2]!); +const sessions = join(state, "codex-home", "sessions"); +await mkdir(sessions, { recursive: true }); +const path = join(sessions, "rollout.jsonl"); +const metadata = { type: "session_meta", payload: { id: "thread-1" } }; +let payload: unknown = Array(4096).fill(0); +// Pretty indentation expands a small input into a large document economically. +for (let i = 0; i < 32; i++) payload = { nested: payload }; +const event = (sequence: number) => ({ type: "event_msg", sequence, payload }); +const header = { + scanId: "scan-1", + threadId: "thread-1", + sessions: [{ threadId: "thread-1", parentThreadId: null, path }], + events: [{ threadId: "thread-1", event: metadata }], +}; +const first = JSON.stringify(header, null, 2); +const prefix = first.slice(0, first.lastIndexOf("\n ]")); +const suffix = "\n ]\n}\n"; +const record = (sequence: number) => + JSON.stringify( + { events: [{ threadId: "thread-1", event: event(sequence) }] }, + null, + 2, + ).slice('{\n "events": [\n'.length, -"\n ]\n}".length); +const count = Math.ceil(constants.MAX_STRING_LENGTH / record(0).length) + 1; +const expected = createHash("sha256"); +let expectedBytes = 0; +function expectChunk(text: string) { + expected.update(text); + expectedBytes += Buffer.byteLength(text); +} +expectChunk(prefix); +const file = await open(path, "wx"); +try { + await file.write(`${JSON.stringify(metadata)}\n`); + for (let i = 0; i < count; i++) { + await file.write(`${JSON.stringify(event(i))}\n`); + expectChunk(`,\n${record(i)}`); + } +} finally { + await file.close(); +} +expectChunk(suffix); +let bytes = 0; +const actual = createHash("sha256"); +const output = new Writable({ + write(chunk, _encoding, callback) { + bytes += chunk.length; + actual.update(chunk); + callback(); + }, +}); +const deps = dependencies({ + environment: { CODEX_SECURITY_STATE_DIR: state }, + onWorkbench: () => ({ + scan: { scanId: "scan-1", continuationThreadId: "thread-1" }, + }), +}); +deps.createSecurity = () => { + throw new Error("Reading logs must not start Codex"); +}; +try { + const exitCode = await main( + ["scans", "logs", "scan-1", "--json"], + output, + process.stderr, + deps, + ); + console.log( + JSON.stringify({ + exitCode, + bytes, + sha256: actual.digest("hex"), + expectedBytes, + expectedSha256: expected.digest("hex"), + maximumStringLength: constants.MAX_STRING_LENGTH, + events: count + 1, + }), + ); + process.exitCode = exitCode; +} catch (error) { + console.error(error); + process.exitCode = 2; +} From 289f9d2052ad4e3692f791ea72045acfc54c34e4 Mon Sep 17 00:00:00 2001 From: mldangelo-oai Date: Tue, 15 Sep 2026 22:58:29 -0700 Subject: [PATCH 05/14] fix(logs): prefer complete saved rollout copies (#933) * fix(logs): retain complete saved rollout copies * fix(logs): compare copies only for selected sessions * fix(logs): pass the iterator cleanup return value * test: give copied-log boolean cases unique names --- sdk/typescript/src/scan-logs.ts | 40 +++++- sdk/typescript/tests-ts/scan-logs.test.ts | 156 +++++++++++++++++----- 2 files changed, 159 insertions(+), 37 deletions(-) diff --git a/sdk/typescript/src/scan-logs.ts b/sdk/typescript/src/scan-logs.ts index 851a8f2da..a2c3cb4f6 100644 --- a/sdk/typescript/src/scan-logs.ts +++ b/sdk/typescript/src/scan-logs.ts @@ -1,6 +1,7 @@ import { createReadStream } from "node:fs"; import { basename, dirname, join, relative } from "node:path"; import { createInterface } from "node:readline"; +import { isDeepStrictEqual } from "node:util"; import { sessionFiles } from "./cost.js"; import { CodexSecurityError } from "./errors.js"; import type { JsonObject } from "./config.js"; @@ -105,7 +106,7 @@ export async function findScanSession( } export async function readScanLogs(options: ScanLogOptions) { - const logs = new Map(); + const logs = new Map(); const homes = new Set( typeof options.codexHome === "string" ? [options.codexHome] @@ -114,12 +115,14 @@ export async function readScanLogs(options: ScanLogOptions) { for (const directory of ["sessions", "archived_sessions"]) { for (const home of homes) { for await (const session of scanSessions(home, directory)) { - if (!logs.has(session.threadId)) logs.set(session.threadId, session); + const copies = logs.get(session.threadId); + if (copies === undefined) logs.set(session.threadId, [session]); + else copies.push(session); } } } - const root = options.threadId ? logs.get(options.threadId) : undefined; + const root = options.threadId ? logs.get(options.threadId)?.[0] : undefined; if (root === undefined && !options.allowMissingRoot) { throw new CodexSecurityError( `No saved session logs are available for scan ${options.scanId}.`, @@ -136,8 +139,8 @@ export async function readScanLogs(options: ScanLogOptions) { const traversed = new Set(options.executionThreadIds ?? included); const pending = [...traversed]; for (const parentId of pending) { - const parent = logs.get(parentId); - for (const session of logs.values()) { + const parent = logs.get(parentId)?.[0]; + for (const [session] of logs.values()) { if ( !traversed.has(session.threadId) && (session.parentThreadId === parentId || @@ -154,8 +157,13 @@ export async function readScanLogs(options: ScanLogOptions) { } const sessions: SessionLog[] = []; for (const threadId of included) { - const session = logs.get(threadId); - if (session !== undefined) sessions.push(session); + const copies = logs.get(threadId); + if (copies === undefined) continue; + let session = copies[0]; + for (const copy of copies.slice(1)) { + if (await extendsSessionLog(session.path, copy.path)) session = copy; + } + sessions.push(session); } const events: Record[] = []; for (const session of sessions) { @@ -238,6 +246,24 @@ function belongsToScan( return false; } +// Prefer a longer copy only when it preserves every event in the earlier copy. +// Identical or divergent copies keep the existing home/archive precedence. +async function extendsSessionLog( + previousPath: string, + path: string, +): Promise { + const events = sessionEvents(path); + try { + for await (const previous of sessionEvents(previousPath)) { + const next = await events.next(); + if (next.done || !isDeepStrictEqual(previous, next.value)) return false; + } + return !(await events.next()).done; + } finally { + await events.return(undefined); + } +} + async function* sessionEvents( path: string, ): AsyncGenerator> { diff --git a/sdk/typescript/tests-ts/scan-logs.test.ts b/sdk/typescript/tests-ts/scan-logs.test.ts index 059e0e42f..972637477 100644 --- a/sdk/typescript/tests-ts/scan-logs.test.ts +++ b/sdk/typescript/tests-ts/scan-logs.test.ts @@ -82,6 +82,91 @@ function commandEvent(command: string, id: string, timestamp?: string) { } describe("saved scan logs", () => { + test.each([ + ["prefix first", [0], [0, 1, 1, 2], 1, false], + ["complete first", [0, 1, 1, 2], [0], 0, false], + ["identical copies", [0, 1, 1, 2], [0, 1, 1, 2], 0, false], + ["longer divergent copy", [0, 2], [0, 1, 1, 2], 0, false], + ["shorter divergent copy", [0, 1, 1, 2], [0, 2], 0, false], + ["equal-length divergent copy", [0, 1], [0, 2], 0, false], + ["complete archived copy", [0], [0, 1, 1, 2], 1, true], + ["prefix archived copy", [0, 1, 1, 2], [0], 0, true], + ["identical archived copy", [0, 1, 1, 2], [0, 1, 1, 2], 0, true], + ["divergent archived copy", [0, 2], [0, 1, 1, 2], 0, true], + ] as const)( + "retains complete copied rollout events and precedence: %s", + async (_label, first, second, selected, archived) => { + const homes = [await temporaryHome(), await temporaryHome()]; + const activity = [ + commandEvent("first", "first-call", "2026-08-11T12:00:03Z"), + commandEvent("repeated", "repeat-call", "2026-08-11T12:00:01Z"), + commandEvent("last", "last-call", "2026-08-11T12:00:02Z"), + ]; + const copies = [first, second]; + for (const [index, home] of homes.entries()) { + await writeSession(home, "parent", []); + await writeSession( + home, + "worker", + copies[index]!.map((event) => activity[event]!), + "parent", + ); + } + if (archived) { + await rename( + join(homes[1]!, "sessions"), + join(homes[1]!, "archived_sessions"), + ); + } + const result = await readSavedScanLogs( + { + scanId: "scan-1", + continuationThreadId: "parent", + executionThreadIds: ["parent"], + }, + archived ? [...homes].reverse() : homes, + ); + expect(result.sessions.map(({ threadId }) => threadId)).toEqual([ + "parent", + "worker", + ]); + expect(result.sessions[1]!.path).toBe( + join( + homes[selected]!, + archived && selected === 1 ? "archived_sessions" : "sessions", + "2026", + "08", + "11", + "rollout-worker.jsonl", + ), + ); + expect( + result.events + .filter(({ threadId }) => threadId === "worker") + .slice(1) + .map(({ event }) => event), + ).toEqual(copies[selected]!.map((event) => activity[event]!)); + }, + ); + + test("keeps first-copy ownership when a longer copy has different session metadata", async () => { + const first = await temporaryHome(); + const second = await temporaryHome(); + const event = commandEvent("scan work", "scan-call"); + await writeSession(first, "parent", []); + await writeSession(first, "worker", [event], "unrelated"); + await writeSession(second, "worker", [event, event], "parent"); + const result = await readSavedScanLogs( + { + scanId: "scan-1", + continuationThreadId: "parent", + executionThreadIds: ["parent"], + }, + [first, second], + ); + expect(result.sessions.map(({ threadId }) => threadId)).toEqual(["parent"]); + }); + test("collects known desktop and CLI threads across active and archived homes without duplicates", async () => { const desktop = await temporaryHome(); const cli = await temporaryHome(); @@ -523,37 +608,48 @@ describe("saved scan logs", () => { ); }); - test("does not parse event bodies from unrelated saved sessions", async () => { - const home = await temporaryHome(); - await writeSession(home, "parent", [ - commandEvent("included", "parent-call"), - ]); - await writeSession(home, "unrelated", [ - commandEvent("UNRELATED_PRIVATE_EVENT_BODY", "unrelated-call"), - ]); - const originalParse = JSON.parse; - let unrelatedBodies = 0; - const parseSpy = spyOn(JSON, "parse").mockImplementation( - (text, reviver) => { - if (text.includes("UNRELATED_PRIVATE_EVENT_BODY")) unrelatedBodies++; - return originalParse(text, reviver); - }, - ); - - try { - const result = await readScanLogs({ - scanId: "scan-1", - threadId: "parent", - codexHome: home, - }); - expect(result.sessions.map(({ threadId }) => threadId)).toEqual([ - "parent", + test.each([false, true])( + "does not parse event bodies from unrelated saved sessions (copied: %p)", + async (copied) => { + const home = await temporaryHome(); + await writeSession(home, "parent", [ + commandEvent("included", "parent-call"), ]); - expect(unrelatedBodies).toBe(0); - } finally { - parseSpy.mockRestore(); - } - }); + const unrelated = commandEvent( + "UNRELATED_PRIVATE_EVENT_BODY", + "unrelated-call", + ); + await writeSession(home, "unrelated", [unrelated]); + const homes = [home]; + if (copied) { + const copyHome = await temporaryHome(); + await writeSession(copyHome, "unrelated", [unrelated, unrelated]); + homes.push(copyHome); + } + const originalParse = JSON.parse; + let unrelatedBodies = 0; + const parseSpy = spyOn(JSON, "parse").mockImplementation( + (text, reviver) => { + if (text.includes("UNRELATED_PRIVATE_EVENT_BODY")) unrelatedBodies++; + return originalParse(text, reviver); + }, + ); + + try { + const result = await readScanLogs({ + scanId: "scan-1", + threadId: "parent", + codexHome: homes, + }); + expect(result.sessions.map(({ threadId }) => threadId)).toEqual([ + "parent", + ]); + expect(unrelatedBodies).toBe(0); + } finally { + parseSpy.mockRestore(); + } + }, + ); test("preserves large selected events and skips malformed metadata prefixes", async () => { const home = await temporaryHome(); From f40c6a9df5e05c2a6cf99c53af3a7e4c62edc088 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 16 Sep 2026 08:25:44 +0000 Subject: [PATCH 06/14] Preserve projected coverage and meaningful candidate provenance --- .../tests/deep_scan_coverage_fixture.mjs | 10 +- .../tests/test_artifact_scan_draft.mjs | 17 ++ .../tests/test_stopped_source_coverage.mjs | 24 ++ .../scripts/workbench_saved_results.py | 46 +++- .../tests/test_stopped_source_coverage.py | 216 ++++++++++++++++-- 5 files changed, 284 insertions(+), 29 deletions(-) create mode 100644 plugins/codex-security/mcp-app/tests/test_stopped_source_coverage.mjs diff --git a/plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs b/plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs index e5bab7e47..d7c233fa6 100644 --- a/plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs +++ b/plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs @@ -23,7 +23,7 @@ const bundled = await build({ }, format: "esm", platform: "node", loader: { ".md": "text" }, write: false, }); -export async function publishCoverageFixture(root, completeness, { resume = false, continueAfterResume = false } = {}) { +export async function publishCoverageFixture(root, completeness, { resume = false, continueAfterResume = false, stopAfterDraft = false } = {}) { const runtimePath = path.join(root, "fixture-runtime.mjs"); await writeFile(runtimePath, bundled.outputFiles[0].contents); const { DeepScanCoordinator, WorkbenchDeepScanStore, createScanArtifactContext, recordCodexSecurityScanDraftViaWorkbench, recordCodexSecurityDeepReduction } = await import(pathToFileURL(runtimePath).href); @@ -132,7 +132,13 @@ export async function publishCoverageFixture(root, completeness, { resume = fals } } } - await runWorkbench(["complete-scan", "--scan-id", run.scanId]); + if (stopAfterDraft) { + await runWorkbench(["fail-scan", "--scan-id", run.scanId, "--message", "Synthetic stop after parent draft."]); + const recovered = await runWorkbench(["recover-scan-results", "--scan-id", run.scanId]); + assert.equal(recovered.scan.resultsRecoveryNeeded, false); + } else { + await runWorkbench(["complete-scan", "--scan-id", run.scanId]); + } for (const [file, bytes] of rawSources) assert.equal(await readFile(file, "utf8"), bytes); return { scanDir: run.scanDir, threadId, terminal }; } diff --git a/plugins/codex-security/mcp-app/tests/test_artifact_scan_draft.mjs b/plugins/codex-security/mcp-app/tests/test_artifact_scan_draft.mjs index 72f815fc3..df28c6a14 100644 --- a/plugins/codex-security/mcp-app/tests/test_artifact_scan_draft.mjs +++ b/plugins/codex-security/mcp-app/tests/test_artifact_scan_draft.mjs @@ -35,6 +35,7 @@ const module = await import( const { completedScanInputSchema, getCodexSecurityCompletedScan, + parseScanDraft, recordCodexSecurityScanDraft, recordCodexSecurityScanDraftViaWorkbench, recordCodexSecurityWorkerScanDraft, @@ -118,6 +119,22 @@ try { findings: [finding], coverage, }; + for (const provenance of [ + { candidateId: ["candidate-1"] }, + { candidateId: { value: "candidate-1" } }, + { workerId: ["worker-1"], candidateId: "candidate-1" }, + { workerId: { value: "worker-1" }, candidateId: "candidate-1" }, + ]) { + const draft = { + ...input, + coverage: { + ...coverage, + completeness: "partial", + deferred: [{ id: "remaining-review", reason: "Another surface remains.", provenance }], + }, + }; + assert.deepEqual(parseScanDraft(draft).coverage.deferred, draft.coverage.deferred); + } const workerRoot = path.join(root, "worker-output"); await mkdir(workerRoot); diff --git a/plugins/codex-security/mcp-app/tests/test_stopped_source_coverage.mjs b/plugins/codex-security/mcp-app/tests/test_stopped_source_coverage.mjs new file mode 100644 index 000000000..3906ed2ee --- /dev/null +++ b/plugins/codex-security/mcp-app/tests/test_stopped_source_coverage.mjs @@ -0,0 +1,24 @@ +import assert from "node:assert/strict"; +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { publishCoverageFixture } from "./deep_scan_coverage_fixture.mjs"; + +for (const resume of [false, true]) { + const root = await mkdtemp(path.join(tmpdir(), "stopped-source-coverage-")); + try { + const { scanDir } = await publishCoverageFixture(root, "partial", { resume, stopAfterDraft: true }); + const coverage = JSON.parse(await readFile(path.join(scanDir, "coverage.json"), "utf8")); + assert.equal(coverage.reviews.length, 3); + assert.equal(coverage.surfaces.length, 3); + assert.deepEqual(coverage.deferred.filter((item) => item.id !== "scan-stopped").map((item) => item.reason), [ + "Verify entry boundaries.", "Verify symbolic links.", + ]); + for (const surface of coverage.surfaces) { + assert.equal(surface.receiptRefs.length, 1); + assert.equal(await readFile(path.join(scanDir, surface.receiptRefs[0]), "utf8"), "Synthetic review evidence.\n"); + } + } finally { + await rm(root, { recursive: true, force: true }); + } +} diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index bbc0cd867..1ae42e683 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -620,10 +620,27 @@ def reducer_output(directory: str, attempt: int, reducer_worker: Any) -> None: if "sourceCoverage" not in drafts_by_path.get(accepted_reducer, {}): accepted_reducer = None accepted_coverage = drafts_by_path.get(accepted_reducer, {}).get("coverage", {}) + frozen_parent_projections = { + relative: draft["coverage"] + for relative, draft, _ in sources + if frozen_source_digests is not None + and accepted_reducer is None + and relative.startswith("checkpoints/") + and isinstance(draft["coverage"].get("reviews"), list) + } + # V1 persists the review projection in the parent, without reducer sourceCoverage. reviewed_attempts = { (review.get("workerId"), review.get("attempt")) - for review in accepted_coverage.get("reviews", []) + for projected in [ + accepted_coverage, + parent["coverage"] if parent else {}, + *frozen_parent_projections.values(), + ] + if isinstance(reviews := projected.get("reviews"), list) + for review in reviews if isinstance(review, dict) + and isinstance(review.get("workerId"), str) + and isinstance(review.get("attempt"), int) } workers_by_id = {worker["id"]: worker for worker in workers} latest_reducer_key = ( @@ -745,13 +762,21 @@ def valid_finding(value: Any) -> bool: if relative in current_results or relative == accepted_reducer ] - def coverage_candidate(owner: str | None, item: dict[str, Any]) -> tuple[str | None, Any]: + def coverage_candidate( + owner: str | None, item: dict[str, Any] + ) -> tuple[str | None, str] | None: + candidate_id = item.get("candidateId") provenance = item.get("provenance") if owner is None and isinstance(provenance, dict): - return provenance.get("workerId"), provenance.get( - "candidateId", item.get("candidateId") - ) - return owner, item.get("candidateId") + source_owner = provenance.get("workerId") + source_candidate = provenance.get("candidateId", candidate_id) + if (source_owner is None or isinstance(source_owner, str)) and isinstance( + source_candidate, str + ): + return source_owner, source_candidate + if isinstance(candidate_id, str): + return owner, candidate_id + return None resolved: dict[tuple[str | None, str], str] = {} for owner, draft in current_drafts: @@ -769,8 +794,9 @@ def coverage_candidate(owner: str | None, item: dict[str, Any]) -> tuple[str | N isinstance(item, dict) and isinstance(item.get("candidateId"), str) and item.get("disposition") in {"reported", "rejected", "not_applicable"} + and (candidate := coverage_candidate(owner, item)) is not None ): - resolved.setdefault(coverage_candidate(owner, item), item["disposition"]) + resolved.setdefault(candidate, item["disposition"]) # Only the current parent may claim that another worker finding was absorbed. # A superseded checkpoint must not suppress a newer independent result. for draft in [parent] if parent else []: @@ -991,7 +1017,11 @@ def coverage_candidate(owner: str | None, item: dict[str, Any]) -> tuple[str | N and worker["merge_state"] == "merged" and (worker_id, worker["attempt"]) in reviewed_attempts ) - if reviewed or (superseded and relative != accepted_reducer): + if reviewed or ( + superseded + and relative != accepted_reducer + and relative not in frozen_parent_projections + ): continue for field in ("surfaces", "explicitExclusions", "deferred", "openQuestions", "reviews"): items = draft["coverage"].get(field, []) diff --git a/plugins/codex-security/tests/test_stopped_source_coverage.py b/plugins/codex-security/tests/test_stopped_source_coverage.py index 57b8e6534..184df8aa2 100644 --- a/plugins/codex-security/tests/test_stopped_source_coverage.py +++ b/plugins/codex-security/tests/test_stopped_source_coverage.py @@ -5,14 +5,26 @@ from argparse import Namespace import pytest -from test_deep_scan_successful_publication import add_worker +from test_deep_scan_successful_publication import add_worker, complete from test_deep_scan_successful_publication import publication_scan as publication_scan +from workbench_test_support import write_checkpoint @pytest.mark.parametrize("host_coverage", [True, False], ids=["accepted-projection", "legacy"]) -@pytest.mark.parametrize("parent_draft", [True, False], ids=["parent-draft", "no-parent"]) +@pytest.mark.parametrize("retry_publication", [False, True], ids=["publish", "retry-publication"]) +@pytest.mark.parametrize( + "parent_draft", + [True, False, "projected"], + ids=["parent-draft", "no-parent", "projected-parent"], +) def test_stopped_recovery_preserves_accepted_coverage_without_worker_id_collisions( - workbench_api, workbench_db, publication_scan, host_coverage, parent_draft + workbench_api, + workbench_db, + publication_scan, + host_coverage, + parent_draft, + retry_publication, + monkeypatch, ): scan = publication_scan() (scan.scan_dir / "findings.json").write_text(json.dumps({"findings": []})) @@ -27,6 +39,16 @@ def test_stopped_recovery_preserves_accepted_coverage_without_worker_id_collisio for disposition in ("needs_follow_up", "rejected"): result = add_worker(workbench_db, scan) worker_id = result.parent.name + if parent_draft == "projected": + output = scan.scan_dir / "artifacts" / worker_id / "output" + output.mkdir(parents=True) + result = output / "result.json" + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_workers SET artifact_dir = ?, result_manifest_path = ? " + "WHERE id = ?", + (str(output), str(result), worker_id), + ) surface = { "id": "surface-1", "candidateId": "candidate-1", @@ -34,6 +56,11 @@ def test_stopped_recovery_preserves_accepted_coverage_without_worker_id_collisio "disposition": disposition, "receiptRefs": [], } + if parent_draft == "projected": + receipt = result.parent / "artifacts" / "review.txt" + receipt.parent.mkdir() + receipt.write_text("Synthetic review evidence.\n") + surface["receiptRefs"] = ["artifacts/review.txt"] deferred = {"candidateId": "candidate-1", "reason": "Validation remains unresolved."} coverage = { "completeness": "partial" if disposition == "needs_follow_up" else "complete", @@ -56,6 +83,10 @@ def test_stopped_recovery_preserves_accepted_coverage_without_worker_id_collisio { **surface, "id": f"{prefix}-surface-1", + "receiptRefs": [ + f"{result.parent.relative_to(scan.scan_dir).as_posix()}/{ref}" + for ref in surface["receiptRefs"] + ], "provenance": {**provenance, "sourceId": "surface-1"}, } ) @@ -81,14 +112,45 @@ def test_stopped_recovery_preserves_accepted_coverage_without_worker_id_collisio reducer.write_text(json.dumps(aggregate)) source_files.append(reducer) saved_bytes = {path: path.read_bytes() for path in source_files} + if parent_draft == "projected": + (scan.scan_dir / "coverage.json").write_text(json.dumps(source_coverage)) if not parent_draft: for filename in ("scan-manifest.json", "findings.json", "coverage.json"): (scan.scan_dir / filename).unlink() - stopped = workbench_api["fail_scan"]( - workbench_db, - Namespace(scan_id=scan.scan_id, claim_token=None, cost_json=None, message="Audit stopped."), - )["scan"] + with monkeypatch.context() as interrupted: + if retry_publication: + + def fail_publication(*args, **kwargs): + raise OSError("Synthetic publication interruption.") + + interrupted.setattr( + workbench_api["saved_results"], + "_write_prepared_scan_finalization", + fail_publication, + ) + stopped = workbench_api["fail_scan"]( + workbench_db, + Namespace( + scan_id=scan.scan_id, claim_token=None, cost_json=None, message="Audit stopped." + ), + )["scan"] + if retry_publication: + assert stopped["resultsRecoveryNeeded"] is True + frozen = workbench_db.execute( + "SELECT retained_source_digests_json FROM scans WHERE id = ?", (scan.scan_id,) + ).fetchone()[0] + assert frozen is not None + if parent_draft == "projected": + mutable_coverage = copy.deepcopy(source_coverage) + mutable_coverage["deferred"].append( + {"id": "outside-frozen-sources", "reason": "Written after sources were frozen."} + ) + (scan.scan_dir / "coverage.json").write_text(json.dumps(mutable_coverage)) + stopped = workbench_api["recover_scan_results"]( + workbench_db, Namespace(scan_id=scan.scan_id) + )["scan"] + assert stopped["resultsRecoveryNeeded"] is False assert stopped["progress"]["status"] == "failed" coverage = json.loads((scan.scan_dir / "coverage.json").read_text()) @@ -96,11 +158,16 @@ def test_stopped_recovery_preserves_accepted_coverage_without_worker_id_collisio assert len(coverage["deferred"]) == 2 assert coverage["deferred"][-1]["id"] == "scan-stopped" assert len(coverage["surfaces"]) == 2 - if host_coverage: + if host_coverage or parent_draft == "projected": for field in ("reviews", "surfaces", "deferred"): assert ( coverage[field][:-1] if field == "deferred" else coverage[field] ) == source_coverage[field] + assert all( + (scan.scan_dir / ref).is_file() + for surface in coverage["surfaces"] + for ref in surface["receiptRefs"] + ) else: assert coverage["deferred"][0]["candidateId"] == "candidate-1" manifest = (scan.scan_dir / "scan-manifest.json").read_bytes() @@ -114,8 +181,10 @@ def test_stopped_recovery_preserves_accepted_coverage_without_worker_id_collisio assert all(path.read_bytes() == contents for path, contents in saved_bytes.items()) +@pytest.mark.parametrize("review_source", ["reducer", "parent"]) +@pytest.mark.parametrize("pending_state", ["canceled", "unreviewed", "new-attempt", "unmerged"]) def test_stopped_recovery_keeps_unmerged_coverage_after_accepted_review( - workbench_api, workbench_db, publication_scan + workbench_api, workbench_db, publication_scan, review_source, pending_state ): scan = publication_scan() (scan.scan_dir / "findings.json").write_text(json.dumps({"findings": []})) @@ -133,17 +202,30 @@ def test_stopped_recovery_keeps_unmerged_coverage_after_accepted_review( (str(reducer),), ) reviews = [{"workerId": accepted.parent.name, "attempt": 1, "completeness": "complete"}] - reducer.write_text( - json.dumps( - { - "scanId": scan.scan_id, - "complete": True, - "findings": [], - "sourceCoverage": {**scan.coverage, "reviews": reviews}, - } - ) + pending = add_worker( + workbench_db, scan, status="canceled" if pending_state == "canceled" else "succeeded" ) - pending = add_worker(workbench_db, scan, status="canceled") + if pending_state != "unreviewed": + reviews.append({"workerId": pending.parent.name, "attempt": 1, "completeness": "partial"}) + if pending_state in {"new-attempt", "unmerged"}: + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_workers SET attempt = ?, merge_state = ? " + "WHERE result_manifest_path = ?", + ( + 2 if pending_state == "new-attempt" else 1, + "none" if pending_state == "unmerged" else "merged", + str(pending), + ), + ) + aggregate = {"scanId": scan.scan_id, "complete": True, "findings": []} + if review_source == "reducer": + aggregate["sourceCoverage"] = {**scan.coverage, "reviews": reviews} + else: + (scan.scan_dir / "coverage.json").write_text( + json.dumps({**scan.coverage, "reviews": reviews}) + ) + reducer.write_text(json.dumps(aggregate)) deferred = {"id": "pending-review", "reason": "The independent review remains unresolved."} pending.write_text( json.dumps( @@ -165,3 +247,99 @@ def test_stopped_recovery_keeps_unmerged_coverage_after_accepted_review( assert coverage["completeness"] == "partial" assert coverage["reviews"] == reviews assert deferred in coverage["deferred"] + + +@pytest.mark.parametrize("stopped", [False, True], ids=["completion", "recovery"]) +@pytest.mark.parametrize( + "provenance", + [ + {"candidateId": ["candidate-1"]}, + {"candidateId": {"value": "candidate-1"}}, + {"workerId": ["worker-1"], "candidateId": "candidate-1"}, + {"workerId": {"value": "worker-1"}, "candidateId": "candidate-1"}, + {"workerId": 1, "candidateId": 2}, + {"workerId": "worker-1", "candidateId": "candidate-1"}, + ], + ids=[ + "list-candidate", + "object-candidate", + "list-worker", + "object-worker", + "numbers", + "strings", + ], +) +def test_standard_publication_preserves_uninterpreted_coverage_provenance( + workbench_api, workbench_db, publication_scan, stopped, provenance +): + scan = publication_scan(mode="standard") + deferred = { + "id": "remaining-review", + "reason": "Another surface remains.", + "provenance": provenance, + } + scan.coverage.update(completeness="partial", deferred=[deferred]) + (scan.scan_dir / "coverage.json").write_text(json.dumps(scan.coverage)) + + if stopped: + workbench_api["fail_scan"]( + workbench_db, + Namespace( + scan_id=scan.scan_id, claim_token=None, cost_json=None, message="Audit stopped." + ), + ) + published = workbench_api["recover_scan_results"]( + workbench_db, Namespace(scan_id=scan.scan_id) + )["scan"] + else: + published = complete(workbench_api, workbench_db, scan) + + assert published["resultsRecoveryNeeded"] is False + assert published["findingCount"] == 1 + coverage = json.loads((scan.scan_dir / "coverage.json").read_text()) + assert deferred in coverage["deferred"] + assert (scan.scan_dir / "report.md").is_file() + + +@pytest.mark.parametrize( + "provenance, pending", + [ + ({"candidateId": ["candidate-1"]}, False), + ({"workerId": ["worker-1"], "candidateId": "candidate-1"}, False), + ({"workerId": "worker-1", "candidateId": "candidate-1"}, True), + ], + ids=["local-candidate", "local-worker", "independent-worker"], +) +def test_standard_recovery_resolves_local_candidates_with_uninterpreted_provenance( + workbench_api, workbench_db, publication_scan, provenance, pending +): + scan = publication_scan(mode="standard") + manifest_path = scan.scan_dir / "scan-manifest.json" + manifest = json.loads(manifest_path.read_text()) + manifest["scan"]["complete"] = False + manifest_path.write_text(json.dumps(manifest)) + scan.coverage["surfaces"][0]["candidateId"] = "candidate-1" + (scan.scan_dir / "coverage.json").write_text(json.dumps(scan.coverage)) + deferred = { + "id": "older-review", + "candidateId": "candidate-1", + "reason": "Earlier validation remained unresolved.", + "provenance": provenance, + } + write_checkpoint( + scan.scan_dir / "checkpoints", + { + "scanId": scan.scan_id, + "complete": False, + "findings": [], + "coverage": {"completeness": "partial", "surfaces": [], "deferred": [deferred]}, + }, + ) + + workbench_api["fail_scan"]( + workbench_db, + Namespace(scan_id=scan.scan_id, claim_token=None, cost_json=None, message="Audit stopped."), + ) + + coverage = json.loads((scan.scan_dir / "coverage.json").read_text()) + assert (deferred in coverage["deferred"]) is pending From 98fde18c26c8333388960e9ace1ed2e44e166c76 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 16 Sep 2026 10:01:15 +0000 Subject: [PATCH 07/14] fix: retain legacy coverage and archived retry receipts --- .../mcp-app/src/artifact-scan-draft.ts | 17 +++- .../src/deep-scan/artifact-validation.ts | 6 +- .../tests/deep_scan_coverage_fixture.mjs | 55 +++++++++-- .../tests/test_retry_coverage_receipts.mjs | 18 ++++ .../scripts/report_projection.py | 2 +- .../scripts/workbench_saved_results.py | 68 +++++++++++-- .../tests/test_legacy_coverage_reviews.py | 32 +++++++ .../tests/test_stopped_source_coverage.py | 95 ++++++++++++++++++- 8 files changed, 274 insertions(+), 19 deletions(-) create mode 100644 plugins/codex-security/mcp-app/tests/test_retry_coverage_receipts.mjs create mode 100644 plugins/codex-security/tests/test_legacy_coverage_reviews.py diff --git a/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts b/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts index 4c560822c..73f6bac3b 100644 --- a/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts +++ b/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts @@ -1,6 +1,6 @@ import { createHash, randomUUID } from "node:crypto"; import { promises as fs } from "node:fs"; -import { dirname, join, sep } from "node:path"; +import { basename, dirname, join, sep } from "node:path"; import type * as z from "zod/v4"; import commonSchema from "../../schemas/definitions/artifact-common.schema.json"; import scanDraftDocument from "../../schemas/tools/scan-draft.schema.json"; @@ -596,6 +596,7 @@ async function readArchivedWorkerCheckpoints( } const archived: ScanDraftInput[] = []; + const archivePrefix = `artifacts/deep_discovery/workers/${basename(workerRoot)}/attempts/`; const attempts = (await fs.readdir(canonicalAttemptsRoot, { withFileTypes: true })) .filter((entry) => entry.isDirectory() && !entry.isSymbolicLink()) .sort((left, right) => archivedAttemptNumber(right.name) - archivedAttemptNumber(left.name) @@ -704,8 +705,18 @@ async function readArchivedWorkerCheckpoints( drafts.sort((left, right) => right.modifiedMs - left.modifiedMs || Number(right.result) - Number(left.result) || right.name.localeCompare(left.name)); - if (checkpointHead !== undefined) archived.push(checkpointHead); - archived.push(...drafts.map((draft) => draft.input)); + const inputs = [...(checkpointHead ? [checkpointHead] : []), ...drafts.map((draft) => draft.input)]; + for (const input of inputs) { + // The archive moved the receipts with this attempt. Rebase only the + // retained projection; the original checkpoint bytes remain unchanged. + for (const surface of input.coverage.surfaces as JsonObject[]) { + if (!Array.isArray(surface.receiptRefs)) continue; + surface.receiptRefs = (surface.receiptRefs as string[]).map((ref) => ( + ref.startsWith(archivePrefix) ? ref : `${archivePrefix}${attempt.name}/${ref}` + )); + } + archived.push(input); + } } return archived; } diff --git a/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts b/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts index 4daaafc99..0283d7972 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts @@ -1,3 +1,4 @@ +import { posix } from "node:path"; import { isDeepStrictEqual } from "node:util"; import { parsePersistedScanDraft, @@ -241,6 +242,7 @@ export function projectDiscoveryCoverage( worker: { id: string; attempt?: number }, artifactPrefix: string, ): ScanDraftInput["coverage"] { + const archivePrefix = `${posix.dirname(artifactPrefix)}/attempts/`; const provenance = { workerId: worker.id, ...(worker.attempt === undefined ? {} : { attempt: worker.attempt }) }; const prefix = `${worker.id}-attempt-${worker.attempt ?? "unknown"}`; const surfaces = coverage.surfaces as Record[]; @@ -259,7 +261,9 @@ export function projectDiscoveryCoverage( surfaces: surfaces.map((surface, index) => ({ ...project(surface), id: `${prefix}-surface-${index + 1}`, - receiptRefs: ((surface.receiptRefs as string[] | undefined) ?? []).map((ref) => `${artifactPrefix}/${ref}`), + receiptRefs: ((surface.receiptRefs as string[] | undefined) ?? []).map((ref) => ( + ref.startsWith(archivePrefix) ? ref : `${artifactPrefix}/${ref}` + )), })), explicitExclusions: (coverage.explicitExclusions as Record[]).map(project), deferred: (coverage.deferred as Record[]).map((item, index) => ({ diff --git a/plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs b/plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs index d7c233fa6..e2acaada4 100644 --- a/plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs +++ b/plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs @@ -13,20 +13,21 @@ const bundled = await build({ bundle: true, stdin: { contents: [ + 'export { archiveDirectory } from "./src/deep-scan/artifacts.ts";', 'export { DeepScanCoordinator } from "./src/deep-scan/coordinator.ts";', 'export { WorkbenchDeepScanStore } from "./src/deep-scan/store.ts";', 'export { createScanArtifactContext } from "./src/artifact-context.ts";', - 'export { recordCodexSecurityScanDraftViaWorkbench } from "./src/artifact-scan-draft.ts";', + 'export { getCodexSecurityCompletedScan, recordCodexSecurityWorkerScanDraft, recordCodexSecurityScanDraftViaWorkbench } from "./src/artifact-scan-draft.ts";', 'export { recordCodexSecurityDeepReduction } from "./src/artifact-deep-reducer.ts";', ].join("\n"), resolveDir: path.join(pluginRoot, "mcp-app"), }, format: "esm", platform: "node", loader: { ".md": "text" }, write: false, }); -export async function publishCoverageFixture(root, completeness, { resume = false, continueAfterResume = false, stopAfterDraft = false } = {}) { +export async function publishCoverageFixture(root, completeness, { resume = false, continueAfterResume = false, stopAfterDraft = false, receiptRetry = false } = {}) { const runtimePath = path.join(root, "fixture-runtime.mjs"); await writeFile(runtimePath, bundled.outputFiles[0].contents); - const { DeepScanCoordinator, WorkbenchDeepScanStore, createScanArtifactContext, recordCodexSecurityScanDraftViaWorkbench, recordCodexSecurityDeepReduction } = await import(pathToFileURL(runtimePath).href); + const { archiveDirectory, getCodexSecurityCompletedScan, DeepScanCoordinator, WorkbenchDeepScanStore, createScanArtifactContext, recordCodexSecurityScanDraftViaWorkbench, recordCodexSecurityWorkerScanDraft, recordCodexSecurityDeepReduction } = await import(pathToFileURL(runtimePath).href); const targetPath = path.join(root, "target"); const codexHome = path.join(root, "codex-home"); const scanRoot = path.join(root, "scans"); @@ -49,6 +50,14 @@ export async function publishCoverageFixture(root, completeness, { resume = fals let { run } = await store.begin({ targetPath, scope: ".", threadId, scanRoot }); const context = await createScanArtifactContext(run.scanId, runWorkbench, { requireRunning: true }); const rawSources = new Map(); + const writeReceiptAttempt = async (artifactDir) => { + await mkdir(path.join(artifactDir, "artifacts"), { recursive: true }); + await writeFile(path.join(artifactDir, "artifacts", "prior.txt"), "Archived receipt.\n"); + await recordCodexSecurityWorkerScanDraft({ root: artifactDir, layout: "worker", repoRoot: targetPath, scanId: run.scanId }, { + scanId: run.scanId, complete: false, findings: [], + coverage: { completeness: "complete", surfaces: [{ id: "prior", label: "Prior review", disposition: "no_issue_found", receiptRefs: ["artifacts/prior.txt"] }], explicitExclusions: [], deferred: [] }, + }); + }; const writeDiscovery = async (artifactDir, index) => { const status = statuses[index]; const pending = completeness === "partial" && status !== "complete"; @@ -63,8 +72,15 @@ export async function publishCoverageFixture(root, completeness, { resume = fals await writeFile(path.join(artifactDir, "artifacts", "review.md"), "Synthetic review evidence.\n"); const resultPath = path.join(artifactDir, "result.json"); const bytes = JSON.stringify({ scanId: run.scanId, complete: true, findings: [], coverage }); - await writeFile(resultPath, bytes); - rawSources.set(resultPath, bytes); + if (receiptRetry) { + await recordCodexSecurityWorkerScanDraft({ root: artifactDir, repoRoot: targetPath, layout: "worker", scanId: run.scanId }, { + scanId: run.scanId, complete: true, findings: [], + coverage: { completeness: status, surfaces: [{ id: "current", label: "Current review", disposition: "no_issue_found", receiptRefs: ["artifacts/review.md"] }], explicitExclusions: [], deferred: [] }, + }); + } else { + await writeFile(resultPath, bytes); + } + rawSources.set(resultPath, await readFile(resultPath, "utf8")); }; if (resume) { const workers = []; @@ -73,6 +89,10 @@ export async function publishCoverageFixture(root, completeness, { resume = fals const workerRoot = path.join(run.scanDir, "artifacts", "deep_discovery", "workers", `discovery-${String(index + 1).padStart(4, "0")}`); const artifactDir = path.join(workerRoot, "output"); const worker = { id: randomUUID(), scanId: run.scanId, kind: "discovery", promptPath: path.join(workerRoot, "prompt.md"), artifactDir, attempt: index === 0 ? 2 : 1 }; + if (receiptRetry) { + await writeReceiptAttempt(artifactDir); + await archiveDirectory(artifactDir, path.join(workerRoot, "attempts", "attempt-01")); + } await writeDiscovery(artifactDir, index); await writeFile(worker.promptPath, "Synthetic discovery prompt.\n"); for (const status of ["queued", "running", "succeeded"]) { @@ -101,8 +121,13 @@ export async function publishCoverageFixture(root, completeness, { resume = fals await request.onThreadStarted?.(thread); if (request.kind === "discovery") { discoveryCalls++; - const index = Number(path.basename(path.dirname(request.promptPath)).split("-").at(-1)) - 1; - if (index === 0 && !request.resumeThreadId) return { threadId: thread, finalResponse: "Continue the unfinished audit." }; + const index = Number(path.basename(path.dirname(request.artifactContext.root)).split("-").at(-1)) - 1; + if (index === 0 && discoveryCalls === 1) { + if (receiptRetry) { + await writeReceiptAttempt(request.artifactContext.root); + } + return { threadId: thread, finalResponse: "Continue the unfinished audit." }; + } await writeDiscovery(request.artifactContext.root, index); } else { await recordCodexSecurityDeepReduction({ ...request.artifactContext, repoRoot: targetPath, scanId: run.scanId }, { scanId: run.scanId, findings: [] }); @@ -138,8 +163,24 @@ export async function publishCoverageFixture(root, completeness, { resume = fals assert.equal(recovered.scan.resultsRecoveryNeeded, false); } else { await runWorkbench(["complete-scan", "--scan-id", run.scanId]); + const completed = await getCodexSecurityCompletedScan( + await createScanArtifactContext(run.scanId, runWorkbench), { scanId: run.scanId }, + ); + assert.deepEqual(completed.coverage, JSON.parse(await readFile(path.join(run.scanDir, "coverage.json"), "utf8"))); } for (const [file, bytes] of rawSources) assert.equal(await readFile(file, "utf8"), bytes); + if (receiptRetry) { + const coverage = JSON.parse(await readFile(path.join(run.scanDir, "coverage.json"), "utf8")); + const current = coverage.surfaces.filter((surface) => surface.label === "Current review"); + assert.equal(current.length, 1); + assert.match(current[0].receiptRefs[0], /\/output\/artifacts\/review\.md$/); + assert.equal(await readFile(path.join(run.scanDir, current[0].receiptRefs[0]), "utf8"), "Synthetic review evidence.\n"); + const prior = coverage.surfaces.filter((surface) => surface.label === "Prior review"); + assert.equal(prior.length, 1); + assert.equal(prior[0].disposition, "no_issue_found"); + assert.match(prior[0].receiptRefs[0], /\/attempts\/attempt-01\/artifacts\/prior\.txt$/); + assert.equal(await readFile(path.join(run.scanDir, prior[0].receiptRefs[0]), "utf8"), "Archived receipt.\n"); + } return { scanDir: run.scanDir, threadId, terminal }; } diff --git a/plugins/codex-security/mcp-app/tests/test_retry_coverage_receipts.mjs b/plugins/codex-security/mcp-app/tests/test_retry_coverage_receipts.mjs new file mode 100644 index 000000000..7b14f3851 --- /dev/null +++ b/plugins/codex-security/mcp-app/tests/test_retry_coverage_receipts.mjs @@ -0,0 +1,18 @@ +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { publishCoverageFixture } from "./deep_scan_coverage_fixture.mjs"; + +for (const resume of [false, true]) { + for (const stopAfterDraft of [false, true]) { + test(`archived receipt survives ${resume ? "reconstruction" : "live retry"} and ${stopAfterDraft ? "recovery" : "completion"}`, async () => { + const root = await mkdtemp(path.join(tmpdir(), "retry-coverage-")); + try { + await publishCoverageFixture(root, "complete", { receiptRetry: true, stopAfterDraft, resume }); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + } +} diff --git a/plugins/codex-security/scripts/report_projection.py b/plugins/codex-security/scripts/report_projection.py index 84582d970..c3e4569eb 100644 --- a/plugins/codex-security/scripts/report_projection.py +++ b/plugins/codex-security/scripts/report_projection.py @@ -1061,7 +1061,7 @@ def build_report_markdown( ] ) reviews = coverage.get("reviews", []) - if reviews: + if isinstance(reviews, list) and reviews: lines.extend( [ "", diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index 1ae42e683..d59bce733 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -628,20 +628,71 @@ def reducer_output(directory: str, attempt: int, reducer_worker: Any) -> None: and relative.startswith("checkpoints/") and isinstance(draft["coverage"].get("reviews"), list) } + projected_coverages = [ + accepted_coverage, + parent["coverage"] if parent else {}, + *frozen_parent_projections.values(), + ] # V1 persists the review projection in the parent, without reducer sourceCoverage. reviewed_attempts = { (review.get("workerId"), review.get("attempt")) - for projected in [ - accepted_coverage, - parent["coverage"] if parent else {}, - *frozen_parent_projections.values(), - ] + for projected in projected_coverages if isinstance(reviews := projected.get("reviews"), list) for review in reviews if isinstance(review, dict) and isinstance(review.get("workerId"), str) and isinstance(review.get("attempt"), int) } + + def coverage_receipts(item: dict[str, Any], worker: Any, relative: str) -> list[str]: + directory = Path(relative).parent + if directory.name == "checkpoints": + directory = directory.parent + output = Path(worker["artifact_dir"]).relative_to(scan_dir) + worker_root = output.parent if output.name == "output" else output + archive_prefix = (worker_root / "attempts").as_posix() + "/" + return [ + ref if ref.startswith(archive_prefix) else f"{directory.as_posix()}/{ref}" + for ref in item.get("receiptRefs", []) + ] + + def coverage_record_retained(field: str, item: Any, worker: Any, relative: str) -> bool: + if not isinstance(item, dict): + return False + source = {key: value for key, value in item.items() if key != "provenance"} + if field == "surfaces": + source["receiptRefs"] = coverage_receipts(item, worker, relative) + for projection in projected_coverages: + records = projection.get(field, []) + for record in records if isinstance(records, list) else []: + if not isinstance(record, dict): + continue + provenance = record.get("provenance") + if not isinstance(provenance, dict) or ( + provenance.get("workerId"), + provenance.get("attempt"), + ) != (worker["id"], worker["attempt"]): + continue + original = { + key: value for key, value in record.items() if key not in {"id", "provenance"} + } + if "sourceId" in provenance: + original["id"] = provenance["sourceId"] + if "candidateId" in provenance: + original["candidateId"] = provenance["candidateId"] + if field == "deferred" and "surfaceIds" in original: + surface_ids = { + surface.get("id"): surface["provenance"].get("sourceId") + for surface in projection.get("surfaces", []) + if isinstance(surface, dict) and isinstance(surface.get("provenance"), dict) + } + original["surfaceIds"] = [ + surface_ids.get(value, value) for value in original["surfaceIds"] + ] + if original == source: + return True + return False + workers_by_id = {worker["id"]: worker for worker in workers} latest_reducer_key = ( (reducer["completed_at"] or "", reducer["id"], int(reducer["attempt"] or 0)) @@ -1017,7 +1068,7 @@ def coverage_candidate( and worker["merge_state"] == "merged" and (worker_id, worker["attempt"]) in reviewed_attempts ) - if reviewed or ( + if ( superseded and relative != accepted_reducer and relative not in frozen_parent_projections @@ -1035,6 +1086,8 @@ def coverage_candidate( for item in items: if field == "openQuestions" and isinstance(item, str): item = {"question": item.strip()} + if reviewed and coverage_record_retained(field, item, worker, relative): + continue if ( field == "surfaces" and isinstance(item, dict) @@ -1060,6 +1113,9 @@ def coverage_candidate( and (field == "deferred" or item.get("disposition") == "needs_follow_up") ): continue + if field == "surfaces" and worker is not None and isinstance(item, dict): + item = copy.deepcopy(item) + item["receiptRefs"] = coverage_receipts(item, worker, relative) if isinstance(item, dict) and "id" not in item: semantic_item = dict(item) if field == "surfaces": diff --git a/plugins/codex-security/tests/test_legacy_coverage_reviews.py b/plugins/codex-security/tests/test_legacy_coverage_reviews.py new file mode 100644 index 000000000..095fa093b --- /dev/null +++ b/plugins/codex-security/tests/test_legacy_coverage_reviews.py @@ -0,0 +1,32 @@ +from __future__ import annotations + +import json +from argparse import Namespace + +import pytest +from test_deep_scan_successful_publication import complete +from test_deep_scan_successful_publication import publication_scan as publication_scan + + +@pytest.mark.parametrize("reviews", [1, 3]) +@pytest.mark.parametrize("stopped", [False, True], ids=["completion", "recovery"]) +def test_standard_publication_preserves_legacy_review_extension( + workbench_api, workbench_db, publication_scan, reviews, stopped +): + scan = publication_scan(mode="standard") + scan.coverage["reviews"] = reviews + (scan.scan_dir / "coverage.json").write_text(json.dumps(scan.coverage)) + if stopped: + workbench_api["fail_scan"]( + workbench_db, + Namespace(scan_id=scan.scan_id, claim_token=None, cost_json=None, message="Stopped."), + ) + published = workbench_api["recover_scan_results"]( + workbench_db, Namespace(scan_id=scan.scan_id) + )["scan"] + else: + published = complete(workbench_api, workbench_db, scan) + assert published["findingCount"] == 1 + assert published["resultsRecoveryNeeded"] is False + assert json.loads((scan.scan_dir / "coverage.json").read_text())["reviews"] == reviews + assert (scan.scan_dir / "report.md").is_file() diff --git a/plugins/codex-security/tests/test_stopped_source_coverage.py b/plugins/codex-security/tests/test_stopped_source_coverage.py index 184df8aa2..ad1fd0569 100644 --- a/plugins/codex-security/tests/test_stopped_source_coverage.py +++ b/plugins/codex-security/tests/test_stopped_source_coverage.py @@ -182,7 +182,9 @@ def fail_publication(*args, **kwargs): @pytest.mark.parametrize("review_source", ["reducer", "parent"]) -@pytest.mark.parametrize("pending_state", ["canceled", "unreviewed", "new-attempt", "unmerged"]) +@pytest.mark.parametrize( + "pending_state", ["canceled", "unreviewed", "new-attempt", "unmerged", "merged"] +) def test_stopped_recovery_keeps_unmerged_coverage_after_accepted_review( workbench_api, workbench_db, publication_scan, review_source, pending_state ): @@ -343,3 +345,94 @@ def test_standard_recovery_resolves_local_candidates_with_uninterpreted_provenan coverage = json.loads((scan.scan_dir / "coverage.json").read_text()) assert (deferred in coverage["deferred"]) is pending + + +@pytest.mark.parametrize("retry_publication", [False, True]) +def test_partial_parent_projection_keeps_only_missing_worker_records( + workbench_api, workbench_db, publication_scan, monkeypatch, retry_publication +): + scan = publication_scan() + result = add_worker(workbench_db, scan) + worker_id = result.parent.name + output = scan.scan_dir / "artifacts" / worker_id / "output" + output.mkdir(parents=True) + result = output / "result.json" + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_workers SET artifact_dir = ?, result_manifest_path = ? WHERE id = ?", + (str(output), str(result), worker_id), + ) + receipt = output / "artifacts" / "evidence.txt" + receipt.parent.mkdir() + receipt.write_text("Retained source review evidence.\n") + surface = { + "id": "missing-review", + "label": "Missing source projection", + "disposition": "needs_follow_up", + "receiptRefs": ["artifacts/evidence.txt"], + } + pending = [ + {"id": "one", "reason": "First proof remains unresolved."}, + {"id": "two", "reason": "Second proof remains unresolved."}, + ] + result.write_text( + json.dumps( + { + "scanId": scan.scan_id, + "complete": True, + "findings": [], + "coverage": { + **scan.coverage, + "completeness": "partial", + "deferred": pending, + "surfaces": [surface], + }, + } + ) + ) + projected = { + **pending[0], + "id": f"{worker_id}-attempt-1-deferred-1", + "provenance": {"workerId": worker_id, "attempt": 1, "sourceId": "one"}, + } + (scan.scan_dir / "coverage.json").write_text( + json.dumps( + { + **scan.coverage, + "completeness": "partial", + "deferred": [projected], + "reviews": [{"workerId": worker_id, "attempt": 1, "completeness": "partial"}], + } + ) + ) + original = result.read_bytes() + with monkeypatch.context() as interrupted: + if retry_publication: + + def fail_publication(*args, **kwargs): + raise OSError("Synthetic publication interruption.") + + interrupted.setattr( + workbench_api["saved_results"], + "_write_prepared_scan_finalization", + fail_publication, + ) + workbench_api["fail_scan"]( + workbench_db, + Namespace(scan_id=scan.scan_id, claim_token=None, cost_json=None, message="Stopped."), + ) + recovered = workbench_api["recover_scan_results"]( + workbench_db, Namespace(scan_id=scan.scan_id) + )["scan"] + assert recovered["resultsRecoveryNeeded"] is False + coverage = json.loads((scan.scan_dir / "coverage.json").read_text()) + assert coverage["completeness"] == "partial" + assert sorted( + item["reason"] for item in coverage["deferred"] if item["id"] != "scan-stopped" + ) == sorted(item["reason"] for item in pending) + assert result.read_bytes() == original + retained_surface = next( + item for item in coverage["surfaces"] if item["label"] == surface["label"] + ) + assert retained_surface["receiptRefs"] == [receipt.relative_to(scan.scan_dir).as_posix()] + assert receipt.read_text() == "Retained source review evidence.\n" From 247915ba0a0a6b68ea4476c49dca8494d5b100f1 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 16 Sep 2026 10:38:24 +0000 Subject: [PATCH 08/14] fix: preserve ownership and links in restored coverage records --- .../scripts/workbench_saved_results.py | 40 ++++++- .../tests/test_partial_coverage_projection.py | 101 ++++++++++++++++++ .../tests/test_stopped_source_coverage.py | 12 ++- 3 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 plugins/codex-security/tests/test_partial_coverage_projection.py diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index d59bce733..3020e2768 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -693,6 +693,42 @@ def coverage_record_retained(field: str, item: Any, worker: Any, relative: str) return True return False + def project_missing_record( + field: str, item: dict[str, Any], index: int, worker: Any, source: dict[str, Any] + ) -> dict[str, Any]: + # Match projectDiscoveryCoverage so recovered holes retain source ownership. + prefix = f"{worker['id']}-attempt-{worker['attempt']}" + result = copy.deepcopy(item) + result["provenance"] = {"workerId": worker["id"], "attempt": worker["attempt"]} + for key, name in (("id", "sourceId"), ("candidateId", "candidateId")): + if key in item: + result["provenance"][name] = item[key] + if field == "surfaces": + result["id"] = f"{prefix}-surface-{index}" + elif field == "deferred": + result["id"] = f"{prefix}-deferred-{index}" + if "candidateId" in item: + result["candidateId"] = f"{prefix}-candidate-{index}" + if "surfaceIds" in item: + surface_ids = { + surface["id"]: f"{prefix}-surface-{offset}" + for offset, surface in enumerate(source.get("surfaces", []), 1) + } + for projection in projected_coverages: + for surface in projection.get("surfaces", []): + provenance = surface.get("provenance", {}) + if ( + isinstance(provenance, dict) + and provenance.get("workerId") == worker["id"] + and provenance.get("attempt") == worker["attempt"] + and isinstance(provenance.get("sourceId"), str) + ): + surface_ids[provenance["sourceId"]] = surface["id"] + result["surfaceIds"] = [ + surface_ids.get(value, value) for value in item["surfaceIds"] + ] + return result + workers_by_id = {worker["id"]: worker for worker in workers} latest_reducer_key = ( (reducer["completed_at"] or "", reducer["id"], int(reducer["attempt"] or 0)) @@ -1083,7 +1119,7 @@ def coverage_candidate( # Keep malformed canonical collections for the existing finalizer's # recovery and warnings rather than silently changing its contract. continue - for item in items: + for index, item in enumerate(items, 1): if field == "openQuestions" and isinstance(item, str): item = {"question": item.strip()} if reviewed and coverage_record_retained(field, item, worker, relative): @@ -1113,6 +1149,8 @@ def coverage_candidate( and (field == "deferred" or item.get("disposition") == "needs_follow_up") ): continue + if reviewed and isinstance(item, dict) and field != "reviews": + item = project_missing_record(field, item, index, worker, draft["coverage"]) if field == "surfaces" and worker is not None and isinstance(item, dict): item = copy.deepcopy(item) item["receiptRefs"] = coverage_receipts(item, worker, relative) diff --git a/plugins/codex-security/tests/test_partial_coverage_projection.py b/plugins/codex-security/tests/test_partial_coverage_projection.py new file mode 100644 index 000000000..2036b1f59 --- /dev/null +++ b/plugins/codex-security/tests/test_partial_coverage_projection.py @@ -0,0 +1,101 @@ +from __future__ import annotations + +import json +from argparse import Namespace + +import pytest +from test_deep_scan_successful_publication import add_worker +from test_deep_scan_successful_publication import publication_scan as publication_scan + + +@pytest.mark.parametrize("worker_count", [1, 2]) +def test_missing_projection_keeps_surface_links_and_independent_reviews( + workbench_api, workbench_db, publication_scan, worker_count +): + scan = publication_scan() + surface = { + "id": "source-surface", + "label": "Source review", + "disposition": "needs_follow_up", + "receiptRefs": [], + } + deferred = { + "id": "pending-check", + "reason": "Validation remains unresolved.", + "candidateId": "pending-candidate", + "surfaceIds": [surface["id"]], + } + reviews, surfaces, originals = [], [], {} + for _ in range(worker_count): + result = add_worker(workbench_db, scan) + worker_id = result.parent.name + reviews.append({"workerId": worker_id, "attempt": 1, "completeness": "partial"}) + surfaces.append( + { + **surface, + "id": f"{worker_id}-attempt-1-surface-1", + "provenance": {"workerId": worker_id, "attempt": 1, "sourceId": surface["id"]}, + } + ) + result.write_text( + json.dumps( + { + "scanId": scan.scan_id, + "complete": True, + "findings": [], + "coverage": { + **scan.coverage, + "completeness": "partial", + "surfaces": [surface], + "deferred": [deferred], + }, + } + ) + ) + originals[result] = result.read_bytes() + (scan.scan_dir / "coverage.json").write_text( + json.dumps( + { + **scan.coverage, + "completeness": "partial", + "surfaces": surfaces, + "deferred": [], + "reviews": reviews, + } + ) + ) + saved = workbench_api["saved_results"] + context = workbench_api["_WORKBENCH_DB_CONTEXT"] + saved.fail_scan( + context, + workbench_db, + Namespace( + scan_id=scan.scan_id, + claim_token=None, + cost_json=None, + message="Stopped.", + ), + ) + recovered = saved.recover_scan_results(context, workbench_db, Namespace(scan_id=scan.scan_id))[ + "scan" + ] + assert recovered["resultsRecoveryNeeded"] is False + coverage = json.loads((scan.scan_dir / "coverage.json").read_text()) + pending = [item for item in coverage["deferred"] if item.get("reason") == deferred["reason"]] + assert len(pending) == worker_count + by_surface_id = {item["id"]: item for item in coverage["surfaces"]} + for item in pending: + linked = by_surface_id[item["surfaceIds"][0]] + assert linked["provenance"]["workerId"] == item["provenance"]["workerId"] + assert item["provenance"]["attempt"] == 1 + assert item["provenance"]["sourceId"] == deferred["id"] + assert item["provenance"]["candidateId"] == deferred["candidateId"] + assert {item["provenance"]["workerId"] for item in pending} == { + item["workerId"] for item in reviews + } + assert len(by_surface_id) == worker_count + assert coverage["completeness"] == "partial" + assert all(path.read_bytes() == data for path, data in originals.items()) + published = (scan.scan_dir / "coverage.json").read_bytes() + saved.recover_scan_results(context, workbench_db, Namespace(scan_id=scan.scan_id)) + assert (scan.scan_dir / "coverage.json").read_bytes() == published diff --git a/plugins/codex-security/tests/test_stopped_source_coverage.py b/plugins/codex-security/tests/test_stopped_source_coverage.py index ad1fd0569..2d80ebb97 100644 --- a/plugins/codex-security/tests/test_stopped_source_coverage.py +++ b/plugins/codex-security/tests/test_stopped_source_coverage.py @@ -248,7 +248,17 @@ def test_stopped_recovery_keeps_unmerged_coverage_after_accepted_review( coverage = json.loads((scan.scan_dir / "coverage.json").read_text()) assert coverage["completeness"] == "partial" assert coverage["reviews"] == reviews - assert deferred in coverage["deferred"] + if pending_state == "merged": + retained = next( + item for item in coverage["deferred"] if item.get("reason") == deferred["reason"] + ) + assert retained["provenance"] == { + "workerId": pending.parent.name, + "attempt": 1, + "sourceId": deferred["id"], + } + else: + assert deferred in coverage["deferred"] @pytest.mark.parametrize("stopped", [False, True], ids=["completion", "recovery"]) From dedfbe54f0da5d468eb9388b5656800e43f3c883 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 16 Sep 2026 11:06:44 +0000 Subject: [PATCH 09/14] fix: retain malformed coverage recovery during projection --- .../scripts/workbench_saved_results.py | 5 ++++- .../tests/test_partial_coverage_projection.py | 17 ++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index 3020e2768..e5ed4699a 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -715,7 +715,10 @@ def project_missing_record( for offset, surface in enumerate(source.get("surfaces", []), 1) } for projection in projected_coverages: - for surface in projection.get("surfaces", []): + surfaces = projection.get("surfaces", []) + for surface in surfaces if isinstance(surfaces, list) else []: + if not isinstance(surface, dict): + continue provenance = surface.get("provenance", {}) if ( isinstance(provenance, dict) diff --git a/plugins/codex-security/tests/test_partial_coverage_projection.py b/plugins/codex-security/tests/test_partial_coverage_projection.py index 2036b1f59..d16f5e247 100644 --- a/plugins/codex-security/tests/test_partial_coverage_projection.py +++ b/plugins/codex-security/tests/test_partial_coverage_projection.py @@ -9,8 +9,9 @@ @pytest.mark.parametrize("worker_count", [1, 2]) +@pytest.mark.parametrize("missing_parent_surfaces", [False, True]) def test_missing_projection_keeps_surface_links_and_independent_reviews( - workbench_api, workbench_db, publication_scan, worker_count + workbench_api, workbench_db, publication_scan, worker_count, missing_parent_surfaces ): scan = publication_scan() surface = { @@ -58,7 +59,7 @@ def test_missing_projection_keeps_surface_links_and_independent_reviews( { **scan.coverage, "completeness": "partial", - "surfaces": surfaces, + "surfaces": None if missing_parent_surfaces else surfaces, "deferred": [], "reviews": reviews, } @@ -85,15 +86,21 @@ def test_missing_projection_keeps_surface_links_and_independent_reviews( assert len(pending) == worker_count by_surface_id = {item["id"]: item for item in coverage["surfaces"]} for item in pending: - linked = by_surface_id[item["surfaceIds"][0]] - assert linked["provenance"]["workerId"] == item["provenance"]["workerId"] + if not missing_parent_surfaces: + linked = by_surface_id[item["surfaceIds"][0]] + assert linked["provenance"]["workerId"] == item["provenance"]["workerId"] assert item["provenance"]["attempt"] == 1 assert item["provenance"]["sourceId"] == deferred["id"] assert item["provenance"]["candidateId"] == deferred["candidateId"] assert {item["provenance"]["workerId"] for item in pending} == { item["workerId"] for item in reviews } - assert len(by_surface_id) == worker_count + if missing_parent_surfaces: + # The existing finalizer repairs malformed collections and reports a warning. + assert coverage["surfaces"] == [] + assert recovered["warnings"] + else: + assert len(by_surface_id) == worker_count assert coverage["completeness"] == "partial" assert all(path.read_bytes() == data for path, data in originals.items()) published = (scan.scan_dir / "coverage.json").read_bytes() From ac655645339eb1ae5759b6a3e551daceebd670f6 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 16 Sep 2026 11:20:09 +0000 Subject: [PATCH 10/14] fix: compare retained coverage with malformed surface collections --- .../scripts/workbench_saved_results.py | 3 ++- .../tests/test_partial_coverage_projection.py | 26 ++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index e5ed4699a..66d6931c7 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -681,9 +681,10 @@ def coverage_record_retained(field: str, item: Any, worker: Any, relative: str) if "candidateId" in provenance: original["candidateId"] = provenance["candidateId"] if field == "deferred" and "surfaceIds" in original: + surfaces = projection.get("surfaces", []) surface_ids = { surface.get("id"): surface["provenance"].get("sourceId") - for surface in projection.get("surfaces", []) + for surface in (surfaces if isinstance(surfaces, list) else []) if isinstance(surface, dict) and isinstance(surface.get("provenance"), dict) } original["surfaceIds"] = [ diff --git a/plugins/codex-security/tests/test_partial_coverage_projection.py b/plugins/codex-security/tests/test_partial_coverage_projection.py index d16f5e247..82f447446 100644 --- a/plugins/codex-security/tests/test_partial_coverage_projection.py +++ b/plugins/codex-security/tests/test_partial_coverage_projection.py @@ -10,8 +10,14 @@ @pytest.mark.parametrize("worker_count", [1, 2]) @pytest.mark.parametrize("missing_parent_surfaces", [False, True]) +@pytest.mark.parametrize("retained_deferred", [False, True]) def test_missing_projection_keeps_surface_links_and_independent_reviews( - workbench_api, workbench_db, publication_scan, worker_count, missing_parent_surfaces + workbench_api, + workbench_db, + publication_scan, + worker_count, + missing_parent_surfaces, + retained_deferred, ): scan = publication_scan() surface = { @@ -26,7 +32,7 @@ def test_missing_projection_keeps_surface_links_and_independent_reviews( "candidateId": "pending-candidate", "surfaceIds": [surface["id"]], } - reviews, surfaces, originals = [], [], {} + reviews, surfaces, deferred_records, originals = [], [], [], {} for _ in range(worker_count): result = add_worker(workbench_db, scan) worker_id = result.parent.name @@ -38,6 +44,20 @@ def test_missing_projection_keeps_surface_links_and_independent_reviews( "provenance": {"workerId": worker_id, "attempt": 1, "sourceId": surface["id"]}, } ) + deferred_records.append( + { + **deferred, + "id": f"{worker_id}-attempt-1-deferred-1", + "candidateId": f"{worker_id}-attempt-1-candidate-1", + "surfaceIds": [surfaces[-1]["id"]], + "provenance": { + "workerId": worker_id, + "attempt": 1, + "sourceId": deferred["id"], + "candidateId": deferred["candidateId"], + }, + } + ) result.write_text( json.dumps( { @@ -60,7 +80,7 @@ def test_missing_projection_keeps_surface_links_and_independent_reviews( **scan.coverage, "completeness": "partial", "surfaces": None if missing_parent_surfaces else surfaces, - "deferred": [], + "deferred": deferred_records if retained_deferred else [], "reviews": reviews, } ) From 7ca1901c8f60be3a610a89bd7655e079960db87e Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 16 Sep 2026 11:31:14 +0000 Subject: [PATCH 11/14] fix: preserve legacy collection and optional ID projection contracts --- .../scripts/workbench_saved_results.py | 33 ++-- .../test_coverage_projection_collections.py | 168 ++++++++++++++++++ .../tests/test_partial_coverage_projection.py | 21 ++- 3 files changed, 209 insertions(+), 13 deletions(-) create mode 100644 plugins/codex-security/tests/test_coverage_projection_collections.py diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index 66d6931c7..6520ece97 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -644,7 +644,10 @@ def reducer_output(directory: str, attempt: int, reducer_worker: Any) -> None: and isinstance(review.get("attempt"), int) } - def coverage_receipts(item: dict[str, Any], worker: Any, relative: str) -> list[str]: + def coverage_receipts(item: dict[str, Any], worker: Any, relative: str) -> Any: + refs = item.get("receiptRefs", []) + if not isinstance(refs, list): + return refs directory = Path(relative).parent if directory.name == "checkpoints": directory = directory.parent @@ -652,8 +655,10 @@ def coverage_receipts(item: dict[str, Any], worker: Any, relative: str) -> list[ worker_root = output.parent if output.name == "output" else output archive_prefix = (worker_root / "attempts").as_posix() + "/" return [ - ref if ref.startswith(archive_prefix) else f"{directory.as_posix()}/{ref}" - for ref in item.get("receiptRefs", []) + f"{directory.as_posix()}/{ref}" + if isinstance(ref, str) and not ref.startswith(archive_prefix) + else ref + for ref in refs ] def coverage_record_retained(field: str, item: Any, worker: Any, relative: str) -> bool: @@ -680,15 +685,18 @@ def coverage_record_retained(field: str, item: Any, worker: Any, relative: str) original["id"] = provenance["sourceId"] if "candidateId" in provenance: original["candidateId"] = provenance["candidateId"] - if field == "deferred" and "surfaceIds" in original: + if field == "deferred" and isinstance(original.get("surfaceIds"), list): surfaces = projection.get("surfaces", []) surface_ids = { surface.get("id"): surface["provenance"].get("sourceId") for surface in (surfaces if isinstance(surfaces, list) else []) - if isinstance(surface, dict) and isinstance(surface.get("provenance"), dict) + if isinstance(surface, dict) + and isinstance(surface.get("id"), str) + and isinstance(surface.get("provenance"), dict) } original["surfaceIds"] = [ - surface_ids.get(value, value) for value in original["surfaceIds"] + surface_ids.get(value, value) if isinstance(value, str) else value + for value in original["surfaceIds"] ] if original == source: return True @@ -710,15 +718,19 @@ def project_missing_record( result["id"] = f"{prefix}-deferred-{index}" if "candidateId" in item: result["candidateId"] = f"{prefix}-candidate-{index}" - if "surfaceIds" in item: + if isinstance(item.get("surfaceIds"), list): + surfaces = source.get("surfaces", []) surface_ids = { surface["id"]: f"{prefix}-surface-{offset}" - for offset, surface in enumerate(source.get("surfaces", []), 1) + for offset, surface in enumerate( + surfaces if isinstance(surfaces, list) else [], 1 + ) + if isinstance(surface, dict) and isinstance(surface.get("id"), str) } for projection in projected_coverages: surfaces = projection.get("surfaces", []) for surface in surfaces if isinstance(surfaces, list) else []: - if not isinstance(surface, dict): + if not isinstance(surface, dict) or not isinstance(surface.get("id"), str): continue provenance = surface.get("provenance", {}) if ( @@ -729,7 +741,8 @@ def project_missing_record( ): surface_ids[provenance["sourceId"]] = surface["id"] result["surfaceIds"] = [ - surface_ids.get(value, value) for value in item["surfaceIds"] + surface_ids.get(value, value) if isinstance(value, str) else value + for value in item["surfaceIds"] ] return result diff --git a/plugins/codex-security/tests/test_coverage_projection_collections.py b/plugins/codex-security/tests/test_coverage_projection_collections.py new file mode 100644 index 000000000..d4d235e37 --- /dev/null +++ b/plugins/codex-security/tests/test_coverage_projection_collections.py @@ -0,0 +1,168 @@ +from __future__ import annotations + +import json +from argparse import Namespace + +import pytest +from test_deep_scan_successful_publication import add_worker +from test_deep_scan_successful_publication import publication_scan as publication_scan + + +@pytest.mark.parametrize( + "case", + [ + "raw_null", + "raw_null_row", + "raw_idless", + "raw_bad_id", + "parent_null", + "parent_null_row", + "parent_idless", + "parent_bad_id", + "raw_receipts_null", + "raw_receipts_null_row", + "raw_receipts_object", + "raw_links_null", + "raw_links_null_row", + "raw_links_object", + "raw_links_nested", + "retained_links_null", + "retained_links_null_row", + "retained_links_object", + "retained_links_nested", + "retained_parent_null", + "retained_parent_null_row", + "retained_parent_idless", + "retained_parent_bad_id", + ], +) +def test_reviewed_missing_deferred_recovers_malformed_worker_surfaces( + workbench_api, workbench_db, publication_scan, case +): + scan = publication_scan() + worker_count = 1 + surface = { + "id": "source-surface", + "label": "Source review", + "disposition": "needs_follow_up", + "receiptRefs": [], + } + deferred = { + "id": "pending-check", + "reason": "Validation remains unresolved.", + "candidateId": "pending-candidate", + "surfaceIds": [surface["id"]], + } + reviews, surfaces, originals = [], [], {} + for _ in range(worker_count): + result = add_worker(workbench_db, scan) + worker_id = result.parent.name + reviews.append({"workerId": worker_id, "attempt": 1, "completeness": "partial"}) + surfaces.append( + { + **surface, + "id": f"{worker_id}-attempt-1-surface-1", + "provenance": {"workerId": worker_id, "attempt": 1, "sourceId": surface["id"]}, + } + ) + result.write_text( + json.dumps( + { + "scanId": scan.scan_id, + "complete": True, + "findings": [], + "coverage": { + **scan.coverage, + "completeness": "partial", + "surfaces": [surface], + "deferred": [deferred], + }, + } + ) + ) + originals[result] = result.read_bytes() + (scan.scan_dir / "coverage.json").write_text( + json.dumps( + { + **scan.coverage, + "completeness": "partial", + "surfaces": surfaces, + "deferred": [], + "reviews": reviews, + } + ) + ) + raw = json.loads(result.read_text()) + parent_path = scan.scan_dir / "coverage.json" + parent = json.loads(parent_path.read_text()) + retained = { + **deferred, + "id": f"{worker_id}-attempt-1-deferred-1", + "candidateId": f"{worker_id}-attempt-1-candidate-1", + "surfaceIds": [surfaces[0]["id"]], + "provenance": { + "workerId": worker_id, + "attempt": 1, + "sourceId": deferred["id"], + "candidateId": deferred["candidateId"], + }, + } + if case.startswith("retained_"): + parent["deferred"] = [retained] + target = parent if "parent" in case or case.startswith("retained_links") else raw["coverage"] + if "links" in case or "receipts" in case: + values = { + "null": None, + "null_row": [None], + "object": {"legacy": "value"}, + "nested": [["legacy"]], + } + suffix = case.split("_", 2)[2] + field = "surfaceIds" if "links" in case else "receiptRefs" + collection = "deferred" if "links" in case else "surfaces" + target[collection][0][field] = values[suffix] + if case.startswith("retained_links"): + raw["coverage"][collection][0][field] = values[suffix] + elif case.endswith("null_row"): + target["surfaces"] = [None] + elif case.endswith("null"): + target["surfaces"] = None + elif case.endswith("idless"): + target["surfaces"][0].pop("id") + elif case.endswith("bad_id"): + target["surfaces"][0]["id"] = ["legacy"] + result.write_text(json.dumps(raw)) + originals[result] = result.read_bytes() + parent_path.write_text(json.dumps(parent)) + saved = workbench_api["saved_results"] + context = workbench_api["_WORKBENCH_DB_CONTEXT"] + saved.fail_scan( + context, + workbench_db, + Namespace( + scan_id=scan.scan_id, + claim_token=None, + cost_json=None, + message="Stopped.", + ), + ) + recovered = saved.recover_scan_results(context, workbench_db, Namespace(scan_id=scan.scan_id))[ + "scan" + ] + assert recovered["resultsRecoveryNeeded"] is False + coverage = json.loads((scan.scan_dir / "coverage.json").read_text()) + assert isinstance(coverage["surfaces"], list) + assert coverage["completeness"] == "partial" + pending = [item for item in coverage["deferred"] if item.get("reason") == deferred["reason"]] + if "links" in case: + # The existing finalizer drops invalid deferred links and reports them. + assert pending == [] + assert recovered["warnings"] + else: + assert len(pending) == 1 + assert pending[0]["provenance"]["workerId"] == worker_id + assert pending[0]["provenance"]["attempt"] == 1 + assert all(path.read_bytes() == data for path, data in originals.items()) + published = (scan.scan_dir / "coverage.json").read_bytes() + saved.recover_scan_results(context, workbench_db, Namespace(scan_id=scan.scan_id)) + assert (scan.scan_dir / "coverage.json").read_bytes() == published diff --git a/plugins/codex-security/tests/test_partial_coverage_projection.py b/plugins/codex-security/tests/test_partial_coverage_projection.py index 82f447446..e01e458b6 100644 --- a/plugins/codex-security/tests/test_partial_coverage_projection.py +++ b/plugins/codex-security/tests/test_partial_coverage_projection.py @@ -11,6 +11,7 @@ @pytest.mark.parametrize("worker_count", [1, 2]) @pytest.mark.parametrize("missing_parent_surfaces", [False, True]) @pytest.mark.parametrize("retained_deferred", [False, True]) +@pytest.mark.parametrize("idless_surface", [False, True]) def test_missing_projection_keeps_surface_links_and_independent_reviews( workbench_api, workbench_db, @@ -18,6 +19,7 @@ def test_missing_projection_keeps_surface_links_and_independent_reviews( worker_count, missing_parent_surfaces, retained_deferred, + idless_surface, ): scan = publication_scan() surface = { @@ -32,15 +34,28 @@ def test_missing_projection_keeps_surface_links_and_independent_reviews( "candidateId": "pending-candidate", "surfaceIds": [surface["id"]], } + worker_surfaces = ( + [{"label": "Background review", "disposition": "reviewed", "receiptRefs": []}] + if idless_surface + else [] + ) + [surface] reviews, surfaces, deferred_records, originals = [], [], [], {} for _ in range(worker_count): result = add_worker(workbench_db, scan) worker_id = result.parent.name reviews.append({"workerId": worker_id, "attempt": 1, "completeness": "partial"}) + if idless_surface: + surfaces.append( + { + **worker_surfaces[0], + "id": f"{worker_id}-attempt-1-surface-1", + "provenance": {"workerId": worker_id, "attempt": 1}, + } + ) surfaces.append( { **surface, - "id": f"{worker_id}-attempt-1-surface-1", + "id": f"{worker_id}-attempt-1-surface-{len(worker_surfaces)}", "provenance": {"workerId": worker_id, "attempt": 1, "sourceId": surface["id"]}, } ) @@ -67,7 +82,7 @@ def test_missing_projection_keeps_surface_links_and_independent_reviews( "coverage": { **scan.coverage, "completeness": "partial", - "surfaces": [surface], + "surfaces": worker_surfaces, "deferred": [deferred], }, } @@ -120,7 +135,7 @@ def test_missing_projection_keeps_surface_links_and_independent_reviews( assert coverage["surfaces"] == [] assert recovered["warnings"] else: - assert len(by_surface_id) == worker_count + assert len(by_surface_id) == worker_count * len(worker_surfaces) assert coverage["completeness"] == "partial" assert all(path.read_bytes() == data for path, data in originals.items()) published = (scan.scan_dir / "coverage.json").read_bytes() From b572ba6aaab59255136a81c5adfb39a9be84d3f3 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 16 Sep 2026 12:08:38 +0000 Subject: [PATCH 12/14] Preserve exclusion ID extensions during coverage recovery --- .../scripts/workbench_saved_results.py | 2 + .../test_coverage_exclusion_extensions.py | 65 +++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 plugins/codex-security/tests/test_coverage_exclusion_extensions.py diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index 6520ece97..96c5c7d7b 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -1209,6 +1209,8 @@ def coverage_candidate( used.add(item["id"]) continue item.setdefault("id", item.get("candidateId") or f"saved-{_digest(item)[:16]}") + if not isinstance(item["id"], str): + continue if item["id"] in used: item["id"] = f"{item['id']}-{_digest(item)[:16]}" used.add(item["id"]) diff --git a/plugins/codex-security/tests/test_coverage_exclusion_extensions.py b/plugins/codex-security/tests/test_coverage_exclusion_extensions.py new file mode 100644 index 000000000..d2f1940a3 --- /dev/null +++ b/plugins/codex-security/tests/test_coverage_exclusion_extensions.py @@ -0,0 +1,65 @@ +from __future__ import annotations + +import json +from argparse import Namespace + +import pytest +from test_deep_scan_successful_publication import add_worker +from test_deep_scan_successful_publication import publication_scan as publication_scan + + +@pytest.mark.parametrize("retained", [False, True]) +@pytest.mark.parametrize("source_id", ["legacy", ["legacy"]]) +def test_recovery_preserves_exclusion_id_extensions( + workbench_api, workbench_db, publication_scan, retained, source_id +): + scan = publication_scan() + result = add_worker(workbench_db, scan) + worker_id = result.parent.name + exclusion = { + "pattern": "vendor/**", + "reason": "Review dependency separately.", + "id": source_id, + } + projected = { + **exclusion, + "provenance": {"workerId": worker_id, "attempt": 1, "sourceId": source_id}, + } + result.write_text( + json.dumps( + { + "scanId": scan.scan_id, + "complete": True, + "findings": [], + "coverage": {**scan.coverage, "explicitExclusions": [exclusion]}, + } + ) + ) + original = result.read_bytes() + (scan.scan_dir / "coverage.json").write_text( + json.dumps( + { + **scan.coverage, + "explicitExclusions": [projected] if retained else [], + "reviews": [{"workerId": worker_id, "attempt": 1, "completeness": "complete"}], + } + ) + ) + saved = workbench_api["saved_results"] + context = workbench_api["_WORKBENCH_DB_CONTEXT"] + saved.fail_scan( + context, + workbench_db, + Namespace(scan_id=scan.scan_id, claim_token=None, cost_json=None, message="Stopped."), + ) + recovered = saved.recover_scan_results(context, workbench_db, Namespace(scan_id=scan.scan_id))[ + "scan" + ] + assert recovered["resultsRecoveryNeeded"] is False + coverage = json.loads((scan.scan_dir / "coverage.json").read_text()) + assert coverage["explicitExclusions"] == [projected] + assert coverage["completeness"] == "partial" + assert result.read_bytes() == original + published = (scan.scan_dir / "coverage.json").read_bytes() + saved.recover_scan_results(context, workbench_db, Namespace(scan_id=scan.scan_id)) + assert (scan.scan_dir / "coverage.json").read_bytes() == published From 3caa83a72b45feb5953bf617778c67d26f2a31bf Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 16 Sep 2026 13:48:37 +0000 Subject: [PATCH 13/14] Preserve descriptive provenance when recovering missing coverage --- .../scripts/workbench_saved_results.py | 8 +- .../tests/test_coverage_provenance.py | 121 ++++++++++++++++++ 2 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 plugins/codex-security/tests/test_coverage_provenance.py diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index 96c5c7d7b..92d226f18 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -708,7 +708,13 @@ def project_missing_record( # Match projectDiscoveryCoverage so recovered holes retain source ownership. prefix = f"{worker['id']}-attempt-{worker['attempt']}" result = copy.deepcopy(item) - result["provenance"] = {"workerId": worker["id"], "attempt": worker["attempt"]} + provenance = result.get("provenance") + if not isinstance(provenance, dict): + provenance = {} + for key in ("workerId", "attempt", "sourceId", "candidateId"): + provenance.pop(key, None) + provenance.update(workerId=worker["id"], attempt=worker["attempt"]) + result["provenance"] = provenance for key, name in (("id", "sourceId"), ("candidateId", "candidateId")): if key in item: result["provenance"][name] = item[key] diff --git a/plugins/codex-security/tests/test_coverage_provenance.py b/plugins/codex-security/tests/test_coverage_provenance.py new file mode 100644 index 000000000..fffbfe0c4 --- /dev/null +++ b/plugins/codex-security/tests/test_coverage_provenance.py @@ -0,0 +1,121 @@ +from __future__ import annotations + +import copy +import json +from argparse import Namespace + +import pytest +from test_deep_scan_successful_publication import add_worker +from test_deep_scan_successful_publication import publication_scan as publication_scan + + +@pytest.mark.parametrize("retained", [False, True], ids=["missing", "retained"]) +@pytest.mark.parametrize("optional_ids", [False, True], ids=["absent-ids", "present-ids"]) +@pytest.mark.parametrize("field", ["surfaces", "deferred", "explicitExclusions", "openQuestions"]) +def test_recovery_preserves_descriptive_provenance( + workbench_api, workbench_db, publication_scan, retained, optional_ids, field +): + scan = publication_scan() + result = add_worker(workbench_db, scan) + worker_id = result.parent.name + prefix = f"{worker_id}-attempt-1" + descriptions = { + "description": "Synthetic source note.", + "details": {"basis": ["source review"], "resolved": False}, + "optional": None, + } + imported = { + **descriptions, + "workerId": "imported-worker", + "attempt": 999, + "sourceId": "imported-source", + "candidateId": "imported-candidate", + } + records = { + "surfaces": { + "id": "source-surface", + "label": "Synthetic surface", + "disposition": "needs_follow_up", + "receiptRefs": [], + }, + "deferred": { + "id": "source-deferred", + "reason": "Synthetic follow-up remains unresolved.", + "surfaceIds": ["source-surface"], + }, + "explicitExclusions": {"pattern": "vendor/**", "reason": "Review separately."}, + "openQuestions": {"question": "Which deployment controls apply?"}, + } + item = records[field] + if optional_ids: + item.setdefault("id", "source-record") + item["candidateId"] = "source-candidate" + item["provenance"] = copy.deepcopy(imported) + expected = copy.deepcopy(item) + expected["provenance"] = {**descriptions, "workerId": worker_id, "attempt": 1} + if "id" in item: + expected["provenance"]["sourceId"] = item["id"] + if optional_ids: + expected["provenance"]["candidateId"] = "source-candidate" + if field == "surfaces": + expected["id"] = f"{prefix}-surface-1" + elif field == "deferred": + expected["id"] = f"{prefix}-deferred-1" + expected["surfaceIds"] = [f"{prefix}-surface-1"] + if optional_ids: + expected["candidateId"] = f"{prefix}-candidate-1" + source_coverage = {**scan.coverage, field: [item]} + if field == "deferred": + source_coverage["surfaces"] = [records["surfaces"]] + result.write_text( + json.dumps( + { + "scanId": scan.scan_id, + "complete": True, + "findings": [], + "coverage": source_coverage, + } + ) + ) + original = result.read_bytes() + parent = { + **scan.coverage, + field: [expected] if retained else [], + "reviews": [{"workerId": worker_id, "attempt": 1, "completeness": "complete"}], + } + if field == "deferred" and retained: + parent["surfaces"] = [ + { + **records["surfaces"], + "id": f"{prefix}-surface-1", + "provenance": { + "workerId": worker_id, + "attempt": 1, + "sourceId": "source-surface", + }, + } + ] + (scan.scan_dir / "coverage.json").write_text(json.dumps(parent)) + saved = workbench_api["saved_results"] + context = workbench_api["_WORKBENCH_DB_CONTEXT"] + saved.fail_scan( + context, + workbench_db, + Namespace(scan_id=scan.scan_id, claim_token=None, cost_json=None, message="Stopped."), + ) + recovered = saved.recover_scan_results(context, workbench_db, Namespace(scan_id=scan.scan_id)) + assert recovered["scan"]["resultsRecoveryNeeded"] is False + published = (scan.scan_dir / "coverage.json").read_bytes() + coverage = json.loads(published) + actual = coverage[field] + if field == "deferred": + actual = [record for record in actual if record.get("id") != "scan-stopped"] + assert actual[0]["surfaceIds"] == [coverage["surfaces"][0]["id"]] + assert coverage["completeness"] == "partial" + assert result.read_bytes() == original + saved.recover_scan_results(context, workbench_db, Namespace(scan_id=scan.scan_id)) + assert (scan.scan_dir / "coverage.json").read_bytes() == published + assert result.read_bytes() == original + if field == "explicitExclusions" and not retained and not optional_ids: + assert actual[0].pop("id").startswith("saved-") + assert actual == [expected] From 5f36ad8e5ce630463a6769fd76f1237bd0af67b3 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 16 Sep 2026 15:25:12 +0000 Subject: [PATCH 14/14] Preserve coverage descriptions and candidate ownership during recovery --- .../src/deep-scan/artifact-validation.ts | 16 +- .../tests/deep_scan_coverage_fixture.mjs | 12 +- .../tests/test_coverage_provenance.mjs | 34 +++++ .../scripts/workbench_saved_results.py | 8 +- .../tests/test_stopped_source_coverage.py | 138 +++++++++++++++++- 5 files changed, 198 insertions(+), 10 deletions(-) create mode 100644 plugins/codex-security/mcp-app/tests/test_coverage_provenance.mjs diff --git a/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts b/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts index 0283d7972..3d566538c 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts @@ -247,14 +247,20 @@ export function projectDiscoveryCoverage( const prefix = `${worker.id}-attempt-${worker.attempt ?? "unknown"}`; const surfaces = coverage.surfaces as Record[]; const surfaceIds = new Map(surfaces.map((surface, index) => [surface.id, `${prefix}-surface-${index + 1}`])); - const project = (item: Record) => ({ - ...structuredClone(item), - provenance: { + const project = (item: Record) => { + const result = structuredClone(item); + const descriptions = result.provenance; + const projected = typeof descriptions === "object" && descriptions !== null && !Array.isArray(descriptions) + ? descriptions as Record : {}; + for (const key of ["workerId", "attempt", "sourceId", "candidateId"]) delete projected[key]; + result.provenance = { + ...projected, ...provenance, ...(item.id === undefined ? {} : { sourceId: item.id }), ...(item.candidateId === undefined ? {} : { candidateId: item.candidateId }), - }, - }); + }; + return result; + }; return { completeness: coverage.completeness, reviews: [{ ...provenance, completeness: coverage.completeness }], diff --git a/plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs b/plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs index e2acaada4..c3bfe6da5 100644 --- a/plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs +++ b/plugins/codex-security/mcp-app/tests/deep_scan_coverage_fixture.mjs @@ -68,6 +68,12 @@ export async function publishCoverageFixture(root, completeness, { resume = fals deferred: pending ? [{ id: "same-id", candidateId: "candidate-1", reason: index === 0 ? "Verify entry boundaries." : "Verify symbolic links.", paths: ["source.py"], surfaceIds: ["shared-surface"] }] : [], openQuestions: pending ? [{ question: `Deployment question ${index + 1}.` }] : [], }; + for (const field of ["surfaces", "explicitExclusions", "deferred", "openQuestions"]) { + for (const item of coverage[field]) item.provenance = { + description: `Original ${field} context.`, details: { evidence: ["source review"] }, + workerId: "untrusted-worker", attempt: 99, sourceId: "untrusted-source", candidateId: "untrusted-candidate", + }; + } await mkdir(path.join(artifactDir, "artifacts"), { recursive: true }); await writeFile(path.join(artifactDir, "artifacts", "review.md"), "Synthetic review evidence.\n"); const resultPath = path.join(artifactDir, "result.json"); @@ -78,7 +84,11 @@ export async function publishCoverageFixture(root, completeness, { resume = fals coverage: { completeness: status, surfaces: [{ id: "current", label: "Current review", disposition: "no_issue_found", receiptRefs: ["artifacts/review.md"] }], explicitExclusions: [], deferred: [] }, }); } else { - await writeFile(resultPath, bytes); + await recordCodexSecurityWorkerScanDraft({ root: artifactDir, repoRoot: targetPath, layout: "worker", scanId: run.scanId }, JSON.parse(bytes)); + for (const name of await readdir(path.join(artifactDir, "checkpoints"))) { + const checkpointPath = path.join(artifactDir, "checkpoints", name); + rawSources.set(checkpointPath, await readFile(checkpointPath, "utf8")); + } } rawSources.set(resultPath, await readFile(resultPath, "utf8")); }; diff --git a/plugins/codex-security/mcp-app/tests/test_coverage_provenance.mjs b/plugins/codex-security/mcp-app/tests/test_coverage_provenance.mjs new file mode 100644 index 000000000..bd7b936f9 --- /dev/null +++ b/plugins/codex-security/mcp-app/tests/test_coverage_provenance.mjs @@ -0,0 +1,34 @@ +import assert from "node:assert/strict"; +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { test } from "node:test"; +import { publishCoverageFixture } from "./deep_scan_coverage_fixture.mjs"; + +for (const resume of [false, true]) { + for (const stopAfterDraft of [false, true]) { + test(`coverage provenance survives ${resume ? "reconstructed" : "live"} reduction and ${stopAfterDraft ? "recovery" : "completion"}`, async () => { + const root = await mkdtemp(path.join(tmpdir(), "coverage-provenance-")); + try { + const { scanDir } = await publishCoverageFixture(root, "partial", { resume, stopAfterDraft }); + const coverage = JSON.parse(await readFile(path.join(scanDir, "coverage.json"), "utf8")); + for (const field of ["surfaces", "explicitExclusions", "deferred", "openQuestions"]) { + const records = coverage[field].filter((item) => item.id !== "scan-stopped"); + assert.ok(records.length > 0, field); + for (const item of records) { + const review = coverage.reviews.find((review) => review.workerId === item.provenance.workerId); + assert.ok(review, "ownership comes from the accepted worker"); + assert.deepEqual(item.provenance, { + description: `Original ${field} context.`, details: { evidence: ["source review"] }, + workerId: review.workerId, attempt: review.attempt, + ...(field === "surfaces" ? { sourceId: "shared-surface" } : {}), + ...(field === "deferred" ? { sourceId: "same-id", candidateId: "candidate-1" } : {}), + }); + } + } + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + } +} diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index 92d226f18..5b9cb798d 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -880,8 +880,12 @@ def coverage_candidate( if owner is None and isinstance(provenance, dict): source_owner = provenance.get("workerId") source_candidate = provenance.get("candidateId", candidate_id) - if (source_owner is None or isinstance(source_owner, str)) and isinstance( - source_candidate, str + if ( + isinstance(source_owner, str) + and source_owner in workers_by_id + and isinstance(provenance.get("attempt"), int) + and (source_owner, provenance["attempt"]) in reviewed_attempts + and isinstance(source_candidate, str) ): return source_owner, source_candidate if isinstance(candidate_id, str): diff --git a/plugins/codex-security/tests/test_stopped_source_coverage.py b/plugins/codex-security/tests/test_stopped_source_coverage.py index 2d80ebb97..be75a5dd8 100644 --- a/plugins/codex-security/tests/test_stopped_source_coverage.py +++ b/plugins/codex-security/tests/test_stopped_source_coverage.py @@ -318,9 +318,9 @@ def test_standard_publication_preserves_uninterpreted_coverage_provenance( [ ({"candidateId": ["candidate-1"]}, False), ({"workerId": ["worker-1"], "candidateId": "candidate-1"}, False), - ({"workerId": "worker-1", "candidateId": "candidate-1"}, True), + ({"workerId": "worker-1", "candidateId": "candidate-1"}, False), ], - ids=["local-candidate", "local-worker", "independent-worker"], + ids=["local-candidate", "local-worker", "descriptive-worker"], ) def test_standard_recovery_resolves_local_candidates_with_uninterpreted_provenance( workbench_api, workbench_db, publication_scan, provenance, pending @@ -446,3 +446,137 @@ def fail_publication(*args, **kwargs): ) assert retained_surface["receiptRefs"] == [receipt.relative_to(scan.scan_dir).as_posix()] assert receipt.read_text() == "Retained source review evidence.\n" + + +@pytest.mark.parametrize("retry_publication", [False, True]) +def test_standard_recovery_keeps_distinct_candidate_with_descriptive_provenance( + workbench_api, workbench_db, publication_scan, monkeypatch, retry_publication +): + scan = publication_scan(mode="standard") + manifest_path = scan.scan_dir / "scan-manifest.json" + manifest = json.loads(manifest_path.read_text()) + manifest["scan"]["complete"] = False + manifest_path.write_text(json.dumps(manifest)) + scan.coverage["surfaces"][0]["candidateId"] = "candidate-A" + (scan.scan_dir / "coverage.json").write_text(json.dumps(scan.coverage)) + deferred = { + "id": "distinct-review", + "candidateId": "candidate-B", + "reason": "Independent validation remains unresolved.", + "provenance": {"candidateId": "candidate-A", "description": "Related earlier review."}, + } + checkpoint = write_checkpoint( + scan.scan_dir / "checkpoints", + { + "scanId": scan.scan_id, + "complete": False, + "findings": [], + "coverage": { + "completeness": "partial", + "surfaces": [], + "explicitExclusions": [], + "deferred": [deferred], + }, + }, + ) + original = checkpoint.read_bytes() + with monkeypatch.context() as interrupted: + if retry_publication: + + def fail_publication(*args, **kwargs): + raise OSError("Synthetic publication interruption.") + + interrupted.setattr( + workbench_api["saved_results"], + "_write_prepared_scan_finalization", + fail_publication, + ) + stopped = workbench_api["fail_scan"]( + workbench_db, + Namespace( + scan_id=scan.scan_id, claim_token=None, cost_json=None, message="Audit stopped." + ), + )["scan"] + assert stopped["resultsRecoveryNeeded"] is retry_publication + recovered = workbench_api["recover_scan_results"]( + workbench_db, Namespace(scan_id=scan.scan_id) + )["scan"] + assert recovered["resultsRecoveryNeeded"] is False + assert recovered["findingCount"] == 1 + coverage = json.loads((scan.scan_dir / "coverage.json").read_text()) + assert deferred in coverage["deferred"] + assert coverage["completeness"] == "partial" + assert checkpoint.read_bytes() == original + published = { + name: (scan.scan_dir / name).read_bytes() + for name in ("scan-manifest.json", "findings.json", "coverage.json", "report.md") + } + assert deferred["reason"] in published["report.md"].decode() + workbench_api["recover_scan_results"](workbench_db, Namespace(scan_id=scan.scan_id)) + assert all((scan.scan_dir / name).read_bytes() == data for name, data in published.items()) + assert checkpoint.read_bytes() == original + + +def test_deep_recovery_reconciles_recognized_projected_candidates( + workbench_api, workbench_db, publication_scan +): + scan = publication_scan() + (scan.scan_dir / "findings.json").write_text(json.dumps({"findings": []})) + manifest_path = scan.scan_dir / "scan-manifest.json" + manifest = json.loads(manifest_path.read_text()) + manifest["scan"]["complete"] = False + manifest_path.write_text(json.dumps(manifest)) + result = add_worker(workbench_db, scan) + worker_id = result.parent.name + deferred = [ + {"id": candidate, "candidateId": candidate, "reason": f"Review {candidate}."} + for candidate in ("candidate-A", "candidate-B") + ] + result.write_text( + json.dumps( + { + "scanId": scan.scan_id, + "complete": True, + "findings": [], + "coverage": {**scan.coverage, "completeness": "partial", "deferred": deferred}, + } + ) + ) + original = result.read_bytes() + (scan.scan_dir / "coverage.json").write_text( + json.dumps( + { + **scan.coverage, + "completeness": "partial", + "reviews": [{"workerId": worker_id, "attempt": 1, "completeness": "partial"}], + "surfaces": [ + { + "id": f"{worker_id}-attempt-1-surface-1", + "label": "Resolved review", + "candidateId": f"{worker_id}-attempt-1-candidate-1", + "disposition": "rejected", + "receiptRefs": [], + "provenance": { + "workerId": worker_id, + "attempt": 1, + "candidateId": "candidate-A", + }, + } + ], + } + ) + ) + workbench_api["fail_scan"]( + workbench_db, + Namespace(scan_id=scan.scan_id, claim_token=None, cost_json=None, message="Audit stopped."), + ) + recovered = workbench_api["recover_scan_results"]( + workbench_db, Namespace(scan_id=scan.scan_id) + )["scan"] + assert recovered["resultsRecoveryNeeded"] is False + coverage = json.loads((scan.scan_dir / "coverage.json").read_text()) + pending = [item for item in coverage["deferred"] if item["id"] != "scan-stopped"] + assert len(pending) == 1 + assert pending[0]["provenance"]["candidateId"] == "candidate-B" + assert pending[0]["provenance"]["workerId"] == worker_id + assert result.read_bytes() == original