From ef9e2ab03da7adb57b35cbd0f85d4690ec1f1b76 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Sat, 12 Sep 2026 08:13:43 +0000 Subject: [PATCH 01/23] fix(plugin): fence stale Deep publication and preserve replay --- plugins/codex-security/mcp-app/server.ts | 4 +- .../mcp-app/src/artifact-scan-draft.ts | 12 +- .../mcp-app/src/deep-scan/coordinator.ts | 10 +- .../tests/deep_scan_publication_cases.mjs | 12 +- .../tests/test_artifact_scan_draft.mjs | 8 + .../test_deep_scan_store_integration.mjs | 5 +- .../codex-security/scripts/workbench_db.py | 4 +- .../scripts/workbench_saved_results.py | 50 ++++-- .../test_deep_scan_publication_authority.py | 84 ++++++++++ .../test_deep_scan_publication_replay.py | 146 ++++++++++++++++++ .../test_deep_scan_successful_publication.py | 31 +++- 11 files changed, 348 insertions(+), 18 deletions(-) create mode 100644 plugins/codex-security/tests/test_deep_scan_publication_authority.py create mode 100644 plugins/codex-security/tests/test_deep_scan_publication_replay.py diff --git a/plugins/codex-security/mcp-app/server.ts b/plugins/codex-security/mcp-app/server.ts index a21fbb6270..2c35f3c7d9 100644 --- a/plugins/codex-security/mcp-app/server.ts +++ b/plugins/codex-security/mcp-app/server.ts @@ -765,7 +765,7 @@ export function createCodexSecurityServer(): McpServer { log: logDeepScanEvent, handoffClaimToken, threadId, - onComplete: async (draft, signal) => { + onComplete: async (draft, signal, publication) => { const context = await createScanArtifactContext( begun.run.scanId, runWorkbench, @@ -779,7 +779,7 @@ export function createCodexSecurityServer(): McpServer { await recordCodexSecurityScanDraftViaWorkbench(context, { ...draft, ...(handoffClaimToken === undefined ? {} : { handoffClaimToken }) - }, runWorkbench, signal); + }, runWorkbench, signal, publication); }, onStopped: async (run) => { await runWorkbench([ 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 4c560822cd..125a02ce9e 100644 --- a/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts +++ b/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts @@ -55,6 +55,12 @@ interface PreparedScanDraft { coverage: JsonObject; } +/** Host-selected Deep aggregate, separate from model-authored draft fields. */ +export interface DeepScanPublication { + coordinatorGeneration?: number; + resultPath: string | null; +} + type PublishScanDraft = ( draft: PreparedScanDraft, expectedDigest: string | undefined, @@ -165,6 +171,7 @@ export async function recordCodexSecurityScanDraftViaWorkbench( input: ScanDraftInput, runWorkbench: RunArtifactWorkbench, signal?: AbortSignal, + publication?: DeepScanPublication, ): Promise { return recordCodexSecurityScanDraft( context, @@ -184,7 +191,10 @@ export async function recordCodexSecurityScanDraftViaWorkbench( const { handoffClaimToken: _claim, ...snapshot } = checkpoint; await Promise.all([ replaceArtifactJson(checkpointPath, snapshot), - replaceArtifactJson(draftPath, draft), + replaceArtifactJson(draftPath, { + ...draft, + ...(publication === undefined ? {} : { deepScanPublication: publication }), + }), ]); const arguments_ = [ "write-scan-draft", 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 905c0c9a87..e88b473b42 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/coordinator.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/coordinator.ts @@ -8,6 +8,7 @@ import { import { validateDiscoveryArtifacts, validateReducerArtifacts, type DeepReductionInput } from "./artifact-validation.js"; import { scanDraftInputSchema, + type DeepScanPublication, type ScanDraftInput } from "../artifact-scan-draft.js"; import type { DeepScanArtifacts } from "./artifacts.js"; @@ -58,6 +59,7 @@ interface SchedulerResult { mergedWorkerIds: string[]; reducers: AcceptedReducer[]; result?: DeepReductionInput; + resultPath?: string; } type CoordinatorPhase = "setup" | "discovery" | "terminal"; @@ -86,7 +88,7 @@ export interface CoordinatorOptions { threadId?: string; heartbeatIntervalMs?: number; observeReplacement?: (run: DeepScanRunState) => Promise; - onComplete?: (draft: ScanDraftInput, signal: AbortSignal) => Promise; + onComplete?: (draft: ScanDraftInput, signal: AbortSignal, publication: DeepScanPublication) => Promise; onStopped?: (run: DeepScanRunState) => Promise; } @@ -319,7 +321,10 @@ export class DeepScanCoordinator { if (draft.scanId !== this.state.scanId) { throw new Error("Deep Scan aggregate does not match its authoritative scan identity."); } - await this.options.onComplete?.(draft, this.publicationAbortController.signal); + await this.options.onComplete?.(draft, this.publicationAbortController.signal, { + coordinatorGeneration: this.state.coordinatorGeneration, + resultPath: schedulerResult.resultPath ?? null, + }); if (this.canceled || this.externallyFailed) return; this.state = await this.finishWithReplay(schedulerResult); if (this.canceled || this.externallyFailed) return; @@ -966,6 +971,7 @@ export class DeepScanCoordinator { mergedWorkerIds: unique(mergedDiscoveries.map((worker) => worker.id)), reducers: reducerOutcomes, result: latestResult, + resultPath: previousReducerResultPath, }; } 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 5e44d1796a..4221c95ee4 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 @@ -150,6 +150,7 @@ export async function testDeepScanPublication({ async function testPublicationUsesAcceptedReducerSnapshot() { const fixture = await fixtureRun({ workers: 1, subagents: 0, stopAfterNoNew: 1, maxDiscoveryRuns: 1 }); + fixture.run.coordinatorGeneration = 3; const store = new FakeStore(fixture.run); const commitDedup = store.commitDedup.bind(store); store.commitDedup = async (commit) => { @@ -163,17 +164,26 @@ export async function testDeepScanPublication({ return structuredClone(store.run); }; const completed = []; + const published = []; const coordinator = new DeepScanCoordinator({ run: fixture.run, store, executor: new FakeExecutor({ discoveryCandidateId: "accepted-finding" }), pluginRoot: fixture.pluginRoot, clock: immediateClock, - onComplete: async (draft) => completed.push(structuredClone(draft)), + onComplete: async (draft, _signal, publication) => { + completed.push(structuredClone(draft)); + published.push(publication); + }, }); coordinator.start(); const terminal = await coordinator.wait(undefined, 5_000); assert.equal(terminal?.status, "succeeded", terminal?.error); assert.equal(completed[0].findings[0].provenance.candidateId, "accepted-finding"); assert.equal(completed[0].coverage.completeness, "complete"); + const reducer = [...store.workers.values()].find((worker) => worker.kind === "dedup"); + assert.deepEqual(published, [{ + coordinatorGeneration: 3, + resultPath: reducer.resultManifestPath, + }]); } await testSaturationOmitsWorkerAcceptedDuringCancellation(); 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 72f815fc30..aa5859ad23 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 @@ -591,6 +591,10 @@ try { const obsoleteCheckpointPath = path.join(deepParentRoot, "checkpoints", "obsolete.json"); await writeFile(obsoleteCheckpointPath, "{malformed obsolete checkpoint\n"); let deepWorkbenchWrites = 0; + const deepPublication = { + coordinatorGeneration: 3, + resultPath: path.join(deepParentRoot, "workers", "reducer", "result.json"), + }; await recordCodexSecurityScanDraftViaWorkbench( deepParentContext, acceptedDeepDraft, @@ -603,11 +607,15 @@ try { const checkpointPath = arguments_[arguments_.indexOf("--checkpoint-path") + 1]; const staged = JSON.parse(await readFile(draftPath, "utf8")); const stagedCheckpoint = JSON.parse(await readFile(checkpointPath, "utf8")); + assert.deepEqual(staged.deepScanPublication, deepPublication); + assert.equal(stagedCheckpoint.deepScanPublication, undefined); assert.deepEqual(staged.findings, acceptedDeepFindings); assert.deepEqual(staged.coverage, acceptedDeepCoverage); assert.deepEqual(stagedCheckpoint.findings, acceptedDeepDraft.findings); assert.equal(stagedCheckpoint.handoffClaimToken, undefined); }, + undefined, + deepPublication, ); assert.equal(deepWorkbenchWrites, 1, "terminal Deep drafts still publish through the workbench lock despite obsolete malformed checkpoints"); assert.deepEqual(await readdir(path.join(deepParentRoot, "drafts")), []); diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_store_integration.mjs b/plugins/codex-security/mcp-app/tests/test_deep_scan_store_integration.mjs index a933849194..589f7874fa 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_store_integration.mjs +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_store_integration.mjs @@ -91,7 +91,10 @@ async function testRecoveredPublicationRejectsLateFailure() { paths: ["fixture.py"], }], }, - }, runWorkbench); + }, runWorkbench, undefined, { + coordinatorGeneration: claim.run.coordinatorGeneration, + resultPath: null, + }); await runWorkbench([ "cancel-scan", "--scan-id", run.scanId, "--thread-id", "publication-failure-owner", diff --git a/plugins/codex-security/scripts/workbench_db.py b/plugins/codex-security/scripts/workbench_db.py index 161756ee0c..b53a506ecf 100644 --- a/plugins/codex-security/scripts/workbench_db.py +++ b/plugins/codex-security/scripts/workbench_db.py @@ -1547,8 +1547,10 @@ def add_warning() -> None: wrote = True manifest, findings, _ = _write_prepared_scan_finalization(prepared) except ContractError as exc: - if wrote or ( + # Replay a validated Deep aggregate after an output write fails. + if (wrote and scan["mode"] != "deep") or ( scan["mode"] == "deep" + and not wrote and not already_sealed and not isinstance(exc, RecoverableContractError) ): diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index a32b965132..0c72a2e0d6 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -1383,6 +1383,41 @@ def save_scan_artifact(db: Any, connection: Any, args: Any) -> dict[str, Any]: return {"scanId": scan_id, "path": str(scan_dir / output)} +def _read_staged_scan_draft(scan_dir: Path, draft_path: str) -> dict[str, Any]: + try: + relative = Path(draft_path).relative_to(scan_dir).as_posix() + except ValueError as exc: + raise SystemExit("Scan draft must be inside the registered scan drafts directory.") from exc + if not re.fullmatch(r"drafts/[0-9a-fA-F-]+\.json", relative): + raise SystemExit("Scan draft must be inside the registered scan drafts directory.") + return _read_scan_local_json(scan_dir, relative, "Staged scan draft") + + +def _require_current_deep_publication( + db: Any, connection: Any, scan_id: str, draft: dict[str, Any] +) -> None: + publication = draft.get("deepScanPublication") + run = db.deep_scan.require_deep_scan_run(connection, scan_id) + db.deep_scan.require_current_coordinator( + run, + argparse.Namespace( + coordinator_generation=publication.get("coordinatorGeneration") if publication else None + ), + ) + # Generation-one runs predate host publication metadata. Keep their existing + # draft path; adopted coordinators must carry their generation and selection. + if publication is None: + return + reducer = _latest_successful_reducer( + connection.execute( + "SELECT * FROM deep_scan_workers WHERE scan_id = ?", (scan_id,) + ).fetchall() + ) + selected_result = reducer["result_manifest_path"] if reducer is not None else None + if publication["resultPath"] != selected_result: + raise SystemExit("Deep Scan aggregate belongs to a superseded publication selection.") + + def write_scan_draft(db: Any, connection: Any, args: Any) -> dict[str, Any]: scan_id = db.require_uuid(args.scan_id, "scan-id") with db.scan_completion_lock(scan_id): @@ -1395,6 +1430,10 @@ def write_scan_draft(db: Any, connection: Any, args: Any) -> dict[str, Any]: "The scan stopped; its saved checkpoint was retained without replacing sealed results." ) scan_dir = db.require_canonical_scan_directory(Path(scan["scan_dir"])) + draft = None + if scan["mode"] == "deep": + draft = _read_staged_scan_draft(scan_dir, args.draft_path) + _require_current_deep_publication(db, connection, scan_id, draft) if args.checkpoint_path is not None: try: checkpoint_relative = Path(args.checkpoint_path).relative_to(scan_dir).as_posix() @@ -1424,15 +1463,8 @@ def write_scan_draft(db: Any, connection: Any, args: Any) -> dict[str, Any]: raise SystemExit( "scan_draft_conflict: canonical scan results changed; reconcile the saved checkpoint again." ) - try: - relative = Path(args.draft_path).relative_to(scan_dir).as_posix() - except ValueError as exc: - raise SystemExit( - "Scan draft must be inside the registered scan drafts directory." - ) from exc - if not re.fullmatch(r"drafts/[0-9a-fA-F-]+\.json", relative): - raise SystemExit("Scan draft must be inside the registered scan drafts directory.") - draft = _read_scan_local_json(scan_dir, relative, "Staged scan draft") + if draft is None: + draft = _read_staged_scan_draft(scan_dir, args.draft_path) manifest, findings, coverage = draft["manifest"], draft["findings"], draft["coverage"] binding = db.workbench_completion_binding(scan, db.now()) # Validate on copies: saved canonical documents remain ordinary unsealed drafts. diff --git a/plugins/codex-security/tests/test_deep_scan_publication_authority.py b/plugins/codex-security/tests/test_deep_scan_publication_authority.py new file mode 100644 index 0000000000..b4ff3a0bdb --- /dev/null +++ b/plugins/codex-security/tests/test_deep_scan_publication_authority.py @@ -0,0 +1,84 @@ +from __future__ import annotations + +import copy +import json +import uuid +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 + + +def stage_publication(scan, *, generation, result_path, title): + draft_dir = scan.scan_dir / "drafts" + draft_dir.mkdir(exist_ok=True) + draft_path = draft_dir / f"{uuid.uuid4()}.json" + checkpoint_path = draft_dir / f"{uuid.uuid4()}.checkpoint.json" + findings = copy.deepcopy(scan.findings) + findings[0]["title"] = title + draft = { + "manifest": json.loads((scan.scan_dir / "scan-manifest.json").read_text()), + "findings": {"findings": findings}, + "coverage": scan.coverage, + } + if generation is not None: + draft["deepScanPublication"] = { + "coordinatorGeneration": generation, + "resultPath": str(result_path), + } + draft_path.write_text(json.dumps(draft)) + checkpoint_path.write_text( + json.dumps({"scanId": scan.scan_id, "findings": findings, "coverage": scan.coverage}) + ) + return Namespace( + scan_id=scan.scan_id, + claim_token=None, + draft_path=str(draft_path), + checkpoint_path=str(checkpoint_path), + expected_draft_digest=None, + ) + + +@pytest.mark.parametrize("stale", ["generation", "aggregate", "unfenced"]) +def test_stale_coordinator_cannot_replace_newer_canonical_publication( + workbench_api, workbench_db, publication_scan, stale +): + scan = publication_scan() + old_result = add_worker(workbench_db, scan) + new_result = add_worker(workbench_db, scan) + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_runs SET coordinator_generation = 3 WHERE scan_id = ?", + (scan.scan_id,), + ) + for result, completed_at in ((old_result, "2026-01-01"), (new_result, "2026-01-02")): + workbench_db.execute( + "UPDATE deep_scan_workers SET kind = 'dedup', merge_state = 'none', completed_at = ? " + "WHERE result_manifest_path = ?", + (completed_at, str(result)), + ) + current = stage_publication( + scan, generation=3, result_path=new_result, title="Current accepted aggregate" + ) + workbench_api["write_scan_draft"](workbench_db, current) + saved = { + path: path.read_bytes() + for path in scan.scan_dir.rglob("*.json") + if "drafts" not in path.parts + } + old = stage_publication( + scan, + generation=None if stale == "unfenced" else 2 if stale == "generation" else 3, + result_path=old_result if stale == "aggregate" else new_result, + title="Superseded aggregate", + ) + + with pytest.raises(SystemExit, match="coordinator|aggregate"): + workbench_api["write_scan_draft"](workbench_db, old) + + assert { + path: path.read_bytes() + for path in scan.scan_dir.rglob("*.json") + if "drafts" not in path.parts + } == saved diff --git a/plugins/codex-security/tests/test_deep_scan_publication_replay.py b/plugins/codex-security/tests/test_deep_scan_publication_replay.py new file mode 100644 index 0000000000..af7a629bee --- /dev/null +++ b/plugins/codex-security/tests/test_deep_scan_publication_replay.py @@ -0,0 +1,146 @@ +from __future__ import annotations + +import json +import sqlite3 +import subprocess +import sys +from argparse import Namespace +from pathlib import Path + +import pytest +from test_deep_scan_publication_authority import stage_publication +from test_deep_scan_successful_publication import add_worker +from test_deep_scan_successful_publication import publication_scan as publication_scan + +_CRASH_PUBLICATION = """ +import json, os, runpy, sqlite3, sys +from argparse import Namespace + +api = runpy.run_path(sys.argv[1], run_name="publication_crash_test") +args = Namespace(**json.loads(sys.argv[3])) +boundary = sys.argv[4] + +class CrashConnection(sqlite3.Connection): + def commit(self): + completing = self.execute( + "SELECT status FROM scans WHERE id = ?", (args.scan_id,) + ).fetchone()[0] == "complete" + if completing and boundary == "sqlite-before": + os._exit(72) + super().commit() + if completing and boundary == "sqlite-after": + os._exit(73) + +connection = sqlite3.connect(sys.argv[2], factory=CrashConnection) +connection.row_factory = sqlite3.Row +connection.execute("PRAGMA foreign_keys = ON") +if boundary.startswith("sqlite-"): + api["complete_scan"](connection, Namespace( + scan_id=args.scan_id, claim_token=None, cost_json=None + )) +else: + saved = api["saved_results"] + original_write = saved.write_scan_local_bytes + def crash_after_write(root, relative, contents): + original_write(root, relative, contents) + if relative == boundary: + os._exit(71) + saved.write_scan_local_bytes = crash_after_write + api["write_scan_draft"](connection, args) +raise AssertionError("publication never reached the requested crash boundary") +""" + + +@pytest.mark.parametrize( + "boundary", + ["findings.json", "coverage.json", "scan-manifest.json", "sqlite-before", "sqlite-after"], +) +def test_publication_crash_replays_selected_input_without_stale_overwrite( + workbench_api, workbench_db, publication_scan, tmp_path, boundary +): + scan = publication_scan() + result_path = add_worker(workbench_db, scan) + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_runs SET coordinator_generation = 3 WHERE scan_id = ?", + (scan.scan_id,), + ) + workbench_db.execute( + "UPDATE deep_scan_workers SET kind = 'dedup', merge_state = 'none' WHERE scan_id = ?", + (scan.scan_id,), + ) + current = stage_publication( + scan, generation=3, result_path=result_path, title="Selected aggregate" + ) + stale = stage_publication( + scan, generation=2, result_path=result_path, title="Obsolete coordinator draft" + ) + database_path = tmp_path / "publication.sqlite3" + with sqlite3.connect(database_path) as connection: + workbench_db.backup(connection) + connection.row_factory = sqlite3.Row + if boundary.startswith("sqlite-"): + workbench_api["write_scan_draft"](connection, current) + + child = subprocess.run( + [ + sys.executable, + "-c", + _CRASH_PUBLICATION, + str(Path(__file__).resolve().parents[1] / "scripts" / "workbench_db.py"), + str(database_path), + json.dumps(vars(current)), + boundary, + ], + capture_output=True, + text=True, + ) + assert child.returncode == {"sqlite-before": 72, "sqlite-after": 73}.get(boundary, 71), ( + child.stdout, + child.stderr, + ) + interrupted = { + path: path.read_bytes() + for path in scan.scan_dir.rglob("*") + if path.is_file() and "drafts" not in path.parts + } + with sqlite3.connect(database_path) as connection: + connection.row_factory = sqlite3.Row + connection.execute("PRAGMA foreign_keys = ON") + row = connection.execute("SELECT * FROM scans WHERE id = ?", (scan.scan_id,)).fetchone() + assert row["status"] == ("complete" if boundary == "sqlite-after" else "running") + assert bool(row["seal_manifest_digest"]) == (boundary == "sqlite-after") + run_before = dict(connection.execute("SELECT * FROM deep_scan_runs").fetchone()) + workers_before = [ + dict(row) for row in connection.execute("SELECT * FROM deep_scan_workers") + ] + + with pytest.raises(SystemExit, match="coordinator|stopped"): + workbench_api["write_scan_draft"](connection, stale) + assert all(path.read_bytes() == contents for path, contents in interrupted.items()) + + if not boundary.startswith("sqlite-"): + workbench_api["write_scan_draft"](connection, current) + completed = workbench_api["complete_scan"]( + connection, Namespace(scan_id=scan.scan_id, claim_token=None, cost_json=None) + )["scan"] + assert completed["progress"]["status"] == "complete" + assert completed["findingCount"] == 1 + assert dict(connection.execute("SELECT * FROM deep_scan_runs").fetchone()) == run_before + assert [ + dict(row) for row in connection.execute("SELECT * FROM deep_scan_workers") + ] == workers_before + assert connection.execute("SELECT COUNT(*) FROM finding_occurrences").fetchone()[0] == 1 + published = { + path: path.read_bytes() + for path in scan.scan_dir.rglob("*") + if path.is_file() and "drafts" not in path.parts + } + workbench_api["complete_scan"]( + connection, Namespace(scan_id=scan.scan_id, claim_token=None, cost_json=None) + ) + assert all(path.read_bytes() == contents for path, contents in published.items()) + if boundary.startswith("sqlite-"): + assert published == interrupted + findings = json.loads((scan.scan_dir / "findings.json").read_text())["findings"] + assert findings[0]["title"] == "Selected aggregate" diff --git a/plugins/codex-security/tests/test_deep_scan_successful_publication.py b/plugins/codex-security/tests/test_deep_scan_successful_publication.py index 8a144ece48..95c799dc86 100644 --- a/plugins/codex-security/tests/test_deep_scan_successful_publication.py +++ b/plugins/codex-security/tests/test_deep_scan_successful_publication.py @@ -72,7 +72,7 @@ def create(*, mode="deep", scope="."): "INSERT INTO deep_scan_runs (scan_id, schema_version, workflow_version, " "status, phase, workers, subagents, stop_after_no_new, max_discovery_runs, " "manifest_path, terminal_reason, created_at, updated_at, completed_at) " - "VALUES (?, 1, 'publication-test', 'succeeded', 'terminal', 1, 0, 1, 1, " + "VALUES (?, 1, 'deep-security-scan/v1', 'succeeded', 'terminal', 1, 0, 1, 1, " "?, 'saturated', ?, ?, ?)", ( scan_id, @@ -460,3 +460,32 @@ def test_standard_publication_preserves_deliberately_partial_coverage( complete(workbench_api, workbench_db, scan) assert_published_aggregate(scan) + + +def test_deep_publication_write_failure_keeps_original_terminal_cause( + workbench_api, workbench_db, publication_scan, monkeypatch +): + scan = publication_scan() + finalizer_globals = workbench_api["_write_prepared_scan_finalization"].__globals__ + write_bytes = finalizer_globals["write_scan_local_bytes"] + + def fail_report(scan_dir, relative_path, payload, **kwargs): + if relative_path == "report.md": + raise finalizer_globals["ContractError"]("Synthetic report write interruption") + return write_bytes(scan_dir, relative_path, payload, **kwargs) + + with monkeypatch.context() as patch: + patch.setitem(finalizer_globals, "write_scan_local_bytes", fail_report) + with pytest.raises(SystemExit, match="Synthetic report write interruption"): + complete(workbench_api, workbench_db, scan) + + assert ( + workbench_db.execute("SELECT status FROM scans WHERE id = ?", (scan.scan_id,)).fetchone()[0] + == "running" + ) + run = workbench_db.execute( + "SELECT status, terminal_reason FROM deep_scan_runs WHERE scan_id = ?", (scan.scan_id,) + ).fetchone() + assert tuple(run) == ("succeeded", "saturated") + assert complete(workbench_api, workbench_db, scan)["progress"]["status"] == "complete" + assert_published_aggregate(scan) From 0e623ac2ffc7d4d535268235144e6e9346424988 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 16 Sep 2026 06:45:50 +0000 Subject: [PATCH 02/23] test(plugin): preserve legacy Deep publication replay --- .../test_deep_scan_publication_authority.py | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/plugins/codex-security/tests/test_deep_scan_publication_authority.py b/plugins/codex-security/tests/test_deep_scan_publication_authority.py index b4ff3a0bdb..99865d82d2 100644 --- a/plugins/codex-security/tests/test_deep_scan_publication_authority.py +++ b/plugins/codex-security/tests/test_deep_scan_publication_authority.py @@ -82,3 +82,45 @@ def test_stale_coordinator_cannot_replace_newer_canonical_publication( for path in scan.scan_dir.rglob("*.json") if "drafts" not in path.parts } == saved + + +@pytest.mark.parametrize("generation", [None, 3], ids=["legacy-generation-one", "current-lease"]) +def test_current_publication_replays_without_changing_checkpoint_or_worker_state( + workbench_api, workbench_db, publication_scan, generation +): + scan = publication_scan() + result = add_worker(workbench_db, scan) + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_runs SET coordinator_generation = ? WHERE scan_id = ?", + (generation or 1, scan.scan_id), + ) + workbench_db.execute( + "UPDATE deep_scan_workers SET kind = 'dedup', merge_state = 'none' WHERE scan_id = ?", + (scan.scan_id,), + ) + draft = stage_publication( + scan, generation=generation, result_path=result, title="Accepted aggregate" + ) + run_before = dict(workbench_db.execute("SELECT * FROM deep_scan_runs").fetchone()) + worker_before = dict(workbench_db.execute("SELECT * FROM deep_scan_workers").fetchone()) + + workbench_api["write_scan_draft"](workbench_db, draft) + published = { + path: path.read_bytes() + for path in scan.scan_dir.rglob("*.json") + if "drafts" not in path.parts + } + replay = workbench_api["write_scan_draft"](workbench_db, draft) + + assert replay == {"scanId": scan.scan_id, "status": "draft_written"} + assert { + path: path.read_bytes() + for path in scan.scan_dir.rglob("*.json") + if "drafts" not in path.parts + } == published + assert len(list((scan.scan_dir / "checkpoints").glob("*.json"))) == 1 + assert dict(workbench_db.execute("SELECT * FROM deep_scan_runs").fetchone()) == run_before + assert dict(workbench_db.execute("SELECT * FROM deep_scan_workers").fetchone()) == worker_before + findings = json.loads((scan.scan_dir / "findings.json").read_text())["findings"] + assert findings[0]["title"] == "Accepted aggregate" From fed714002416bc2f73bf0cdf77fe72a4c4cba866 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 16 Sep 2026 08:29:25 +0000 Subject: [PATCH 03/23] Preserve Deep parent drafts and reducer publication order --- .../mcp-app/tests/clock_workbench.py | 14 + ...test_deep_scan_publication_integration.mjs | 279 ++++++++++++++++++ .../scripts/workbench_saved_results.py | 24 +- .../test_deep_scan_publication_authority.py | 29 +- 4 files changed, 336 insertions(+), 10 deletions(-) create mode 100644 plugins/codex-security/mcp-app/tests/clock_workbench.py create mode 100644 plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.mjs diff --git a/plugins/codex-security/mcp-app/tests/clock_workbench.py b/plugins/codex-security/mcp-app/tests/clock_workbench.py new file mode 100644 index 0000000000..22d2c52b62 --- /dev/null +++ b/plugins/codex-security/mcp-app/tests/clock_workbench.py @@ -0,0 +1,14 @@ +"""Control the workbench clock while retaining its real commands and database writes.""" + +import os +import runpy +import sys +from pathlib import Path + +source = Path(sys.argv[1]) +sys.argv = [str(source), *sys.argv[2:]] +sys.path.insert(0, str(source.parent)) +api = runpy.run_path(str(source), run_name="test_workbench") +instant = os.environ["TEST_WORKBENCH_NOW"] +api["main"].__globals__["now"] = lambda: instant +api["main"]() diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.mjs b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.mjs new file mode 100644 index 0000000000..270b9466bb --- /dev/null +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.mjs @@ -0,0 +1,279 @@ +import assert from "node:assert/strict"; +import { execFile } from "node:child_process"; +import { mkdir, mkdtemp, readFile, readdir, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { after, test } from "node:test"; +import { fileURLToPath } from "node:url"; +import { promisify } from "node:util"; +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import { build } from "esbuild"; + +const execFileAsync = promisify(execFile); +const testsRoot = path.dirname(fileURLToPath(import.meta.url)); +const mcpAppRoot = path.resolve(testsRoot, ".."); +const pluginRoot = path.resolve(mcpAppRoot, ".."); +const workbenchPath = path.join(pluginRoot, "scripts", "workbench_db.py"); +const python = process.env.PYTHON?.trim() || "python3"; +const owner = "publication-test-owner"; +const coverage = { completeness: "complete", surfaces: [], explicitExclusions: [], deferred: [] }; +const highId = "ffffffff-ffff-4fff-8fff-ffffffffffff"; +const lowId = "00000000-0000-4000-8000-000000000001"; +const bundleRoot = await mkdtemp(path.join(tmpdir(), "deep-publication-bundle-")); +after(() => rm(bundleRoot, { recursive: true, force: true })); +const serverPath = path.join(bundleRoot, "server.cjs"); +await build({ + bundle: true, + define: { __dirname: JSON.stringify(mcpAppRoot), "import.meta.url": "__filename" }, + entryPoints: [path.join(mcpAppRoot, "main.ts")], + external: ["fsevents"], + format: "cjs", + loader: { ".md": "text" }, + logLevel: "silent", + outfile: serverPath, + platform: "node", + target: "node20", +}); +const hostBundle = await build({ + bundle: true, + stdin: { + contents: [ + 'export { WorkbenchDeepScanStore } from "./src/deep-scan/store.ts";', + 'export { DeepScanCoordinator } from "./src/deep-scan/coordinator.ts";', + 'export { createScanArtifactContext } from "./src/artifact-context.ts";', + 'export { recordCodexSecurityScanDraftViaWorkbench } from "./src/artifact-scan-draft.ts";', + ].join("\n"), + resolveDir: mcpAppRoot, + }, + format: "esm", + loader: { ".md": "text" }, + platform: "node", + write: false, +}); +const { + WorkbenchDeepScanStore, DeepScanCoordinator, createScanArtifactContext, + recordCodexSecurityScanDraftViaWorkbench, +} = await import(`data:text/javascript;base64,${Buffer.from(hostBundle.outputFiles[0].contents).toString("base64")}`); + +test("public partial drafts retain checkpoints after coordinator adoption", async (t) => { + const fixture = await createFixture(t); + const { run, store, call, runWorkbench } = fixture; + assertSuccess(await call("record_codex_security_scan_draft", partial(run, "generation-one"))); + const first = await checkpoints(run); + assert.equal(Object.keys(first).length, 1); + const claim = await store.claimCoordinator({ scanId: run.scanId, threadId: owner }); + assert.equal(claim.acquired, true); + assert.equal(claim.run.coordinatorGeneration, 2); + + assertSuccess(await call("record_codex_security_scan_draft", partial(run, "generation-two"))); + const retained = await checkpoints(run); + assert.equal(Object.keys(retained).length, 2); + for (const [name, bytes] of Object.entries(first)) assert.equal(retained[name], bytes); + assert.deepEqual(new Set(Object.values(retained).flatMap((bytes) => ( + JSON.parse(bytes).coverage.deferred.map((item) => item.candidateId) + ))), new Set(["generation-one", "generation-two"])); + + const before = await snapshot(run); + assertToolError(await call("record_codex_security_scan_draft", { + ...partial(run, "unfenced-final"), complete: true, + }), /current coordinator lease/); + const context = await createScanArtifactContext(run.scanId, runWorkbench, { requireRunning: true }); + await assert.rejects(recordCodexSecurityScanDraftViaWorkbench( + context, partial(run, "stale-generation"), runWorkbench, undefined, + { coordinatorGeneration: 1, resultPath: null }, + ), /newer generation/); + assertToolError(await call("complete_codex_security_scan", { scanId: run.scanId }), /deep/i); + assert.deepEqual(await snapshot(run), before); + assert.deepEqual(await readdir(path.join(run.scanDir, "drafts")), []); +}); + +for (const [label, offsets, ids] of [ + ["increasing timestamps", [1, 2], [highId, lowId]], + ["equal timestamps and descending UUIDs", [1, 1], [highId, lowId]], + ["decreasing timestamps", [2, 1], [lowId, highId]], + ["equal timestamps and ascending UUIDs", [1, 1], [lowId, highId]], +]) { + test(`selected reducer survives recovery and public completion: ${label}`, async (t) => { + const fixture = await createFixture(t); + const { run, store, call, runWorkbench, instant } = fixture; + assertSuccess(await call("record_codex_security_scan_draft", partial(run, "parent-checkpoint-only"))); + const parentCheckpoints = await checkpoints(run); + assert.equal(Object.keys(parentCheckpoints).length, 1); + const claimed = await store.claimCoordinator({ scanId: run.scanId, threadId: owner }); + assert.equal(claimed.run.coordinatorGeneration, 2); + assert.equal(claimed.run.config.stopAfterNoNew, 4); + const results = await commitReducers(fixture, offsets, ids); + const persisted = await store.get(run.scanId, owner); + assert.equal(persisted.noNewStreak, 4); + assert.deepEqual(persisted.persistedWorkers.filter((worker) => worker.kind === "discovery") + .map((worker) => worker.mergeState), Array(4).fill("merged")); + const context = await createScanArtifactContext(run.scanId, runWorkbench, { requireRunning: true }); + const publish = async (index, coordinatorGeneration = 2) => ( + recordCodexSecurityScanDraftViaWorkbench(context, { + ...JSON.parse(await readFile(results[index], "utf8")), coverage, + }, runWorkbench, undefined, { coordinatorGeneration, resultPath: results[index] }) + ); + + await publish(1); + const selected = await snapshot(run); + await assert.rejects(publish(0), /superseded publication selection/); + await assert.rejects(publish(1, 1), /newer generation/); + assert.deepEqual(await snapshot(run), selected); + + const publications = []; + const coordinator = new DeepScanCoordinator({ + run: persisted, store, pluginRoot, + executor: { run() { throw new Error("Recovery must reuse the committed workers."); } }, + clock: { now: () => Date.parse(instant), sleep: async () => {} }, + onComplete: async (draft, signal, publication) => { + publications.push(publication); + await recordCodexSecurityScanDraftViaWorkbench(context, draft, runWorkbench, signal, publication); + }, + }); + coordinator.start(); + const terminal = await coordinator.wait(undefined, 30_000); + assert.equal(terminal?.status, "succeeded", terminal?.error); + assert.deepEqual(publications, [{ coordinatorGeneration: 2, resultPath: results[1] }]); + + const beforeCompletion = await snapshot(run); + const lateDraft = partial(run, "late-parent-checkpoint"); + assertToolError(await call("record_codex_security_scan_draft", lateDraft)); + assert.deepEqual(await snapshot(run), beforeCompletion); + assertSuccess(await call("complete_codex_security_scan", { scanId: run.scanId })); + const sealed = await snapshot(run); + const manifest = JSON.parse(sealed.files["scan-manifest.json"]); + assert.equal(manifest.scan.threatModel.summary, "dedup-0002"); + assert.ok(manifest.scan.sealedAt); + assert.deepEqual(JSON.parse(sealed.files["findings.json"]).findings, []); + assert.deepEqual(JSON.parse(sealed.files["coverage.json"]).deferred, []); + assert.ok(sealed.files["report.md"]); + for (const [name, bytes] of Object.entries(parentCheckpoints)) { + assert.equal(sealed.checkpoints[name], bytes, "completion retains the immutable parent checkpoint"); + } + assert.deepEqual(sealed.checkpoints, beforeCompletion.checkpoints); + assertSuccess(await call("complete_codex_security_scan", { scanId: run.scanId })); + assert.deepEqual(await snapshot(run), sealed, "completion replay is byte-stable"); + assertToolError(await call("record_codex_security_scan_draft", lateDraft)); + assert.deepEqual(await snapshot(run), sealed, "late writes cannot replace sealed results"); + }); +} + +async function createFixture(t) { + const root = await mkdtemp(path.join(tmpdir(), "deep-publication-")); + let client; + t.after(async () => { + await client?.close(); + await rm(root, { recursive: true, force: true }); + }); + const targetPath = path.join(root, "target"); + await mkdir(targetPath); + await writeFile(path.join(targetPath, "fixture.py"), "# Synthetic publication fixture\n"); + const environment = { + ...process.env, + CODEX_HOME: path.join(root, "home"), + CODEX_SECURITY_STATE_DIR: path.join(root, "state"), + CODEX_SECURITY_SCAN_ROOT: path.join(root, "scans"), + }; + delete environment.CODEX_SECURITY_DEEP_SCAN_CONFIG_PATH; + const instant = new Date().toISOString(); + let now = instant; + const runWorkbench = async (args, input) => { + const child = execFileAsync(python, [path.join(testsRoot, "clock_workbench.py"), workbenchPath, ...args], { + cwd: pluginRoot, env: { ...environment, TEST_WORKBENCH_NOW: now }, + timeout: 30_000, maxBuffer: 4 * 1024 * 1024, + }); + child.child.stdin.end(input); + return JSON.parse((await child).stdout); + }; + const store = new WorkbenchDeepScanStore(runWorkbench); + const { run } = await store.begin({ targetPath, scope: ".", threadId: owner, scanRoot: environment.CODEX_SECURITY_SCAN_ROOT }); + client = new Client({ name: "publication-integration", version: "1.0.0" }); + const transport = new StdioClientTransport({ + command: process.execPath, args: [serverPath, "--stdio"], cwd: mcpAppRoot, + env: environment, stderr: "pipe", + }); + transport.stderr?.resume(); + await client.connect(transport); + return { + run, store, runWorkbench, instant, + setTime(offset) { now = new Date(Date.parse(instant) + offset * 1_000).toISOString(); }, + call(name, args) { + return client.callTool({ name, arguments: args, _meta: { "openai/threadId": owner } }); + }, + }; +} + +async function commitReducers(fixture, offsets, ids) { + const { run, store } = fixture; + const results = []; + for (let batch = 0; batch < 2; batch++) { + const workerIds = []; + for (let index = batch * 2 + 1; index <= batch * 2 + 2; index++) { + const id = `10000000-0000-4000-8000-${String(index).padStart(12, "0")}`; + const label = `discovery-${String(index).padStart(4, "0")}`; + const artifact = await workerArtifact(run, "workers", label); + const update = { id, scanId: run.scanId, kind: "discovery", status: "running", attempt: 1, ...artifact }; + await store.updateWorker(update); + const resultManifestPath = path.join(artifact.artifactDir, "result.json"); + await writeFile(resultManifestPath, JSON.stringify({ scanId: run.scanId, findings: [], coverage })); + await store.updateWorker({ ...update, status: "succeeded", resultManifestPath }); + workerIds.push(id); + } + const label = `dedup-${String(batch + 1).padStart(4, "0")}`; + const artifact = await workerArtifact(run, "dedup", label); + const id = ids[batch]; + await store.claimDedup({ id, scanId: run.scanId, workerIds, ...artifact }); + await store.updateWorker({ id, scanId: run.scanId, kind: "dedup", status: "running", attempt: 1, ...artifact }); + const resultManifestPath = path.join(artifact.artifactDir, "result.json"); + await writeFile(resultManifestPath, JSON.stringify({ scanId: run.scanId, findings: [], threatModel: { summary: label } })); + fixture.setTime(offsets[batch]); + await store.commitDedup({ id, scanId: run.scanId, resultManifestPath, newFindings: 0 }); + fixture.setTime(0); + results.push(resultManifestPath); + } + return results; +} + +async function workerArtifact(run, kind, label) { + const artifactDir = path.join(run.scanDir, "artifacts", "deep_discovery", kind, label, "output"); + const promptPath = path.join(path.dirname(artifactDir), "prompt.md"); + await mkdir(artifactDir, { recursive: true }); + await writeFile(promptPath, `Synthetic ${label}\n`); + return { artifactDir, promptPath }; +} + +function partial(run, candidateId) { + return { + scanId: run.scanId, complete: false, findings: [], + coverage: { + ...coverage, completeness: "partial", + deferred: [{ candidateId, reason: "Synthetic review remains pending.", paths: ["fixture.py"] }], + }, + }; +} + +async function checkpoints(run) { + const directory = path.join(run.scanDir, "checkpoints"); + const files = {}; + for (const name of await readdir(directory)) files[name] = await readFile(path.join(directory, name), "utf8"); + return files; +} + +async function snapshot(run) { + const files = {}; + for (const name of ["scan-manifest.json", "findings.json", "coverage.json", "report.md"]) { + try { files[name] = await readFile(path.join(run.scanDir, name), "utf8"); } + catch (error) { if (error.code !== "ENOENT") throw error; } + } + return { files, checkpoints: await checkpoints(run) }; +} + +function assertSuccess(result) { + assert.notEqual(result.isError, true, JSON.stringify(result)); +} + +function assertToolError(result, pattern) { + assert.equal(result.isError, true, JSON.stringify(result)); + if (pattern) assert.match(JSON.stringify(result), pattern); +} diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index 6cbc7ad54a..f51ba74a5d 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -1396,6 +1396,8 @@ def _require_current_deep_publication( db: Any, connection: Any, scan_id: str, draft: dict[str, Any] ) -> None: publication = draft.get("deepScanPublication") + if publication is None and draft["manifest"]["scan"].get("complete") is False: + return run = db.deep_scan.require_deep_scan_run(connection, scan_id) db.deep_scan.require_current_coordinator( run, @@ -1407,10 +1409,24 @@ def _require_current_deep_publication( # draft path; adopted coordinators must carry their generation and selection. if publication is None: return - reducer = _latest_successful_reducer( - connection.execute( - "SELECT * FROM deep_scan_workers WHERE scan_id = ?", (scan_id,) - ).fetchall() + + # Match the durable reducer sequence used by coordinator recovery. + def reducer_order(worker: Any) -> tuple[int, str]: + match = re.search(r"dedup-(\d+)", worker["prompt_path"]) + return (int(match[1]) if match else 0, worker["id"]) + + reducer = max( + ( + worker + for worker in connection.execute( + "SELECT * FROM deep_scan_workers WHERE scan_id = ?", (scan_id,) + ) + if worker["kind"] == "dedup" + and worker["status"] == "succeeded" + and worker["result_manifest_path"] + ), + key=reducer_order, + default=None, ) selected_result = reducer["result_manifest_path"] if reducer is not None else None if publication["resultPath"] != selected_result: diff --git a/plugins/codex-security/tests/test_deep_scan_publication_authority.py b/plugins/codex-security/tests/test_deep_scan_publication_authority.py index 99865d82d2..1d17ca8be7 100644 --- a/plugins/codex-security/tests/test_deep_scan_publication_authority.py +++ b/plugins/codex-security/tests/test_deep_scan_publication_authority.py @@ -10,7 +10,7 @@ from test_deep_scan_successful_publication import publication_scan as publication_scan -def stage_publication(scan, *, generation, result_path, title): +def stage_publication(scan, *, generation, result_path, title, complete=True): draft_dir = scan.scan_dir / "drafts" draft_dir.mkdir(exist_ok=True) draft_path = draft_dir / f"{uuid.uuid4()}.json" @@ -22,6 +22,8 @@ def stage_publication(scan, *, generation, result_path, title): "findings": {"findings": findings}, "coverage": scan.coverage, } + if not complete: + draft["manifest"]["scan"]["complete"] = False if generation is not None: draft["deepScanPublication"] = { "coordinatorGeneration": generation, @@ -40,9 +42,18 @@ def stage_publication(scan, *, generation, result_path, title): ) -@pytest.mark.parametrize("stale", ["generation", "aggregate", "unfenced"]) +@pytest.mark.parametrize( + ("stale", "complete"), + [ + ("generation", True), + ("generation", False), + ("aggregate", True), + ("aggregate", False), + ("unfenced", True), + ], +) def test_stale_coordinator_cannot_replace_newer_canonical_publication( - workbench_api, workbench_db, publication_scan, stale + workbench_api, workbench_db, publication_scan, stale, complete ): scan = publication_scan() old_result = add_worker(workbench_db, scan) @@ -52,11 +63,16 @@ def test_stale_coordinator_cannot_replace_newer_canonical_publication( "UPDATE deep_scan_runs SET coordinator_generation = 3 WHERE scan_id = ?", (scan.scan_id,), ) - for result, completed_at in ((old_result, "2026-01-01"), (new_result, "2026-01-02")): + for sequence, result in enumerate((old_result, new_result), start=1): workbench_db.execute( - "UPDATE deep_scan_workers SET kind = 'dedup', merge_state = 'none', completed_at = ? " + "UPDATE deep_scan_workers SET kind = 'dedup', merge_state = 'none', " + "prompt_path = ?, completed_at = ? " "WHERE result_manifest_path = ?", - (completed_at, str(result)), + ( + str(scan.scan_dir / f"dedup-{sequence:04d}" / "prompt.md"), + f"2026-01-0{sequence}", + str(result), + ), ) current = stage_publication( scan, generation=3, result_path=new_result, title="Current accepted aggregate" @@ -72,6 +88,7 @@ def test_stale_coordinator_cannot_replace_newer_canonical_publication( generation=None if stale == "unfenced" else 2 if stale == "generation" else 3, result_path=old_result if stale == "aggregate" else new_result, title="Superseded aggregate", + complete=complete, ) with pytest.raises(SystemExit, match="coordinator|aggregate"): From 4173b55f7f3286b1aa9f15c4731e554a715442fc Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 16 Sep 2026 08:58:37 +0000 Subject: [PATCH 04/23] fix(deep): recover accepted publication response loss --- .../mcp-app/src/artifact-scan-draft.ts | 26 ++- ...test_deep_scan_publication_integration.mjs | 208 +++++++++++++++++- .../scripts/workbench_saved_results.py | 22 ++ .../test_deep_scan_publication_authority.py | 42 ++++ 4 files changed, 290 insertions(+), 8 deletions(-) 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 125a02ce9e..32132b51e0 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,7 @@ 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 { isDeepStrictEqual } from "node:util"; 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"; @@ -187,14 +188,16 @@ export async function recordCodexSecurityScanDraftViaWorkbench( ["drafts", `${randomUUID()}.json`], "staged scan draft", ); + const acceptanceName = `${basename(draftPath, ".json")}.accepted.json`; try { const { handoffClaimToken: _claim, ...snapshot } = checkpoint; + const stagedDraft = { + ...draft, + ...(publication === undefined ? {} : { deepScanPublication: publication }), + }; await Promise.all([ replaceArtifactJson(checkpointPath, snapshot), - replaceArtifactJson(draftPath, { - ...draft, - ...(publication === undefined ? {} : { deepScanPublication: publication }), - }), + replaceArtifactJson(draftPath, stagedDraft), ]); const arguments_ = [ "write-scan-draft", @@ -214,6 +217,18 @@ export async function recordCodexSecurityScanDraftViaWorkbench( try { await runWorkbench(arguments_); } catch (error) { + if (context.mode === "deep" && publication !== undefined) { + const accepted = await readArtifactJsonObject( + context, ["drafts", acceptanceName], "accepted scan draft", + ).catch(() => undefined); + if (isDeepStrictEqual(accepted, JSON.parse(JSON.stringify({ + status: "draft_written", input: { ...stagedDraft, checkpoint: snapshot }, + })))) { + signal?.throwIfAborted(); + await runWorkbench(arguments_); + return; + } + } if (!workbenchScanDraftConflict(error)) throw error; throw Object.assign( new Error( @@ -226,6 +241,7 @@ export async function recordCodexSecurityScanDraftViaWorkbench( await Promise.all([ fs.rm(checkpointPath, { force: true }), fs.rm(draftPath, { force: true }), + fs.rm(join(dirname(draftPath), acceptanceName), { force: true }), ]); } }, diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.mjs b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.mjs index 270b9466bb..4e8478b2c6 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.mjs +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.mjs @@ -196,7 +196,7 @@ async function createFixture(t) { transport.stderr?.resume(); await client.connect(transport); return { - run, store, runWorkbench, instant, + run, store, runWorkbench, instant, environment, setTime(offset) { now = new Date(Date.parse(instant) + offset * 1_000).toISOString(); }, call(name, args) { return client.callTool({ name, arguments: args, _meta: { "openai/threadId": owner } }); @@ -213,7 +213,8 @@ async function commitReducers(fixture, offsets, ids) { const id = `10000000-0000-4000-8000-${String(index).padStart(12, "0")}`; const label = `discovery-${String(index).padStart(4, "0")}`; const artifact = await workerArtifact(run, "workers", label); - const update = { id, scanId: run.scanId, kind: "discovery", status: "running", attempt: 1, ...artifact }; + const update = { id, scanId: run.scanId, kind: "discovery", status: "running", attempt: 1, + threadId: `session-${label}`, ...artifact }; await store.updateWorker(update); const resultManifestPath = path.join(artifact.artifactDir, "result.json"); await writeFile(resultManifestPath, JSON.stringify({ scanId: run.scanId, findings: [], coverage })); @@ -224,7 +225,8 @@ async function commitReducers(fixture, offsets, ids) { const artifact = await workerArtifact(run, "dedup", label); const id = ids[batch]; await store.claimDedup({ id, scanId: run.scanId, workerIds, ...artifact }); - await store.updateWorker({ id, scanId: run.scanId, kind: "dedup", status: "running", attempt: 1, ...artifact }); + await store.updateWorker({ id, scanId: run.scanId, kind: "dedup", status: "running", attempt: 1, + threadId: `session-${label}`, ...artifact }); const resultManifestPath = path.join(artifact.artifactDir, "result.json"); await writeFile(resultManifestPath, JSON.stringify({ scanId: run.scanId, findings: [], threatModel: { summary: label } })); fixture.setTime(offsets[batch]); @@ -277,3 +279,203 @@ function assertToolError(result, pattern) { assert.equal(result.isError, true, JSON.stringify(result)); if (pattern) assert.match(JSON.stringify(result), pattern); } + +async function publicationFixture(t, workflow = "deep-scan-mcp/v1") { + const fixture = await createFixture(t); + const { run, store, runWorkbench, environment } = fixture; + const database = path.join(environment.CODEX_SECURITY_STATE_DIR, "workbench.sqlite3"); + const sql = async (script, ...args) => (await execFileAsync(python, + ["-c", script, database, run.scanId, ...args], { env: environment })).stdout; + await sql(`import sqlite3, sys +with sqlite3.connect(sys.argv[1]) as c: + c.execute("UPDATE deep_scan_runs SET workflow_version = ? WHERE scan_id = ?", (sys.argv[3], sys.argv[2])) +`, workflow); + const claim = await store.claimCoordinator({ scanId: run.scanId, threadId: owner }); + assert.equal(claim.run.coordinatorGeneration, 2); + const results = await commitReducers(fixture, [1, 2], [highId, lowId]); + await mkdir(path.join(run.scanDir, "checkpoints"), { recursive: true }); + const draft = { ...JSON.parse(await readFile(results[1], "utf8")), coverage }; + const context = await createScanArtifactContext(run.scanId, runWorkbench, { requireRunning: true }); + return { + ...fixture, sql, results, draft, + publish: (runner = runWorkbench) => recordCodexSecurityScanDraftViaWorkbench( + context, draft, runner, undefined, { coordinatorGeneration: 2, resultPath: results[1] }, + ), + workers: async () => JSON.parse(await sql(`import json, sqlite3, sys +c = sqlite3.connect(sys.argv[1]); c.row_factory = sqlite3.Row +print(json.dumps({ + "workflow": c.execute("SELECT workflow_version FROM deep_scan_runs WHERE scan_id = ?", (sys.argv[2],)).fetchone()[0], + "owner": dict(c.execute("SELECT workspace_id, deep_scan_owner_thread_id, continuation_thread_id, handoff_claim_token FROM scans WHERE id = ?", (sys.argv[2],)).fetchone()), + "workers": [dict(r) for r in c.execute("SELECT * FROM deep_scan_workers WHERE scan_id = ? ORDER BY id", (sys.argv[2],))], + "inputs": [dict(r) for r in c.execute("SELECT * FROM deep_scan_dedup_inputs WHERE dedup_worker_id IN (SELECT id FROM deep_scan_workers WHERE scan_id = ?) ORDER BY dedup_worker_id, discovery_worker_id", (sys.argv[2],))], +}))`)), + }; +} + +function publicationRunner(fixture, { before, after } = {}) { + const requests = []; + let writes = 0; + const run = async (args, input) => { + if (args[0] !== "write-scan-draft") return fixture.runWorkbench(args, input); + const draftPath = args[args.indexOf("--draft-path") + 1]; + const checkpointPath = args[args.indexOf("--checkpoint-path") + 1]; + requests.push({ args: [...args], input, + draft: await readFile(draftPath, "utf8"), checkpoint: await readFile(checkpointPath, "utf8") }); + await before?.(requests.length, { draftPath, checkpointPath }); + const result = await fixture.runWorkbench(args, input); + writes++; + await after?.(requests.length, { draftPath, checkpointPath }); + return result; + }; + return { run, requests, writes: () => writes }; +} + +for (const workflow of ["deep-scan-mcp/v1", "deep-security-scan/v1"]) { + for (const loseResponse of [false, true]) { + test(`${workflow} finishes the same call after accepted publication (lost=${loseResponse})`, async (t) => { + const f = await publicationFixture(t, workflow); + const before = await f.workers(); + const sources = await Promise.all(f.results.map((file) => readFile(file))); + let accepted; + const runner = publicationRunner(f, { after: async (attempt) => { + if (attempt === 1) accepted = await snapshot(f.run); + else assert.deepEqual(await snapshot(f.run), accepted); + if (loseResponse && attempt === 1) throw new Error("Synthetic accepted publication response loss"); + } }); + let publications = 0; + const coordinator = new DeepScanCoordinator({ + run: await f.store.get(f.run.scanId, owner), store: f.store, pluginRoot, + executor: { run() { assert.fail("Publication recovery cannot dispatch workers"); } }, + clock: { now: () => Date.parse(f.instant), sleep: async () => {} }, + onComplete: async (draft, signal, publication) => { + publications++; + assert.deepEqual(draft, f.draft); + assert.deepEqual(publication, { coordinatorGeneration: 2, resultPath: f.results[1] }); + const context = await createScanArtifactContext(f.run.scanId, runner.run, { requireRunning: true }); + await recordCodexSecurityScanDraftViaWorkbench(context, draft, runner.run, signal, publication); + }, + }); + coordinator.start(); + const terminal = await coordinator.settled(); + assert.equal(terminal.status, "succeeded", terminal.error); + assert.equal(terminal.terminalReason, "saturated"); + assert.equal((await f.workers()).workflow, workflow); + assert.equal(terminal.coordinatorGeneration, 2); + assert.equal(publications, 1); + assert.equal(runner.requests.length, loseResponse ? 2 : 1); + assert.equal(runner.writes(), loseResponse ? 2 : 1); + if (loseResponse) assert.deepEqual(runner.requests[1], runner.requests[0]); + assert.deepEqual(await f.workers(), before); + assert.deepEqual(await Promise.all(f.results.map((file) => readFile(file))), sources); + const parent = await f.runWorkbench(["complete-scan", "--scan-id", f.run.scanId]); + assert.equal(parent.scan.progress.status, "complete"); + assert.deepEqual(await f.workers(), before); + assert.deepEqual(await readdir(path.join(f.run.scanDir, "drafts")), []); + }); + } +} + +for (const code of ["EACCES", "ECONNRESET"]) { + for (const matching of [false, true]) { + test(`publication ${code} before acceptance is one attempt (matching=${matching})`, async (t) => { + const f = await publicationFixture(t); + if (matching) await f.publish(); + const before = await snapshot(f.run); + const workers = await f.workers(); + const original = Object.assign(new Error(`Synthetic precommit ${code}`), { code }); + const runner = publicationRunner(f, { before() { throw original; } }); + await assert.rejects(f.publish(runner.run), (error) => error === original); + assert.equal(runner.requests.length, 1); + assert.equal(runner.writes(), 0); + assert.deepEqual(await snapshot(f.run), before); + assert.deepEqual(await f.workers(), workers); + assert.deepEqual(await readdir(path.join(f.run.scanDir, "drafts")), []); + }); + } +} + +for (const takeover of ["coordinator", "continuation"]) { + test(`accepted publication replay rejects a replacement ${takeover}`, async (t) => { + const f = await publicationFixture(t); + const workers = await f.workers(); + let accepted; + const runner = publicationRunner(f, { after: async (attempt) => { + assert.equal(attempt, 1, "A stale replay must not commit"); + accepted = await snapshot(f.run); + if (takeover === "coordinator") { + f.setTime(120); + const replacement = new WorkbenchDeepScanStore(f.runWorkbench); + const claim = await replacement.claimCoordinator({ scanId: f.run.scanId, threadId: owner }); + assert.equal(claim.acquired, true); + assert.equal(claim.run.coordinatorGeneration, 3); + } else { + await f.sql(`import sqlite3, sys +with sqlite3.connect(sys.argv[1]) as c: + c.execute("UPDATE scans SET handoff_claim_token = ?, continuation_thread_id = ? WHERE id = ?", ("00000000-0000-4000-8000-000000000099", "new-owner", sys.argv[2])) +`); + workers.owner.handoff_claim_token = "00000000-0000-4000-8000-000000000099"; + workers.owner.continuation_thread_id = "new-owner"; + } + throw new Error("Synthetic response loss after takeover"); + } }); + await assert.rejects(f.publish(runner.run), takeover === "coordinator" ? /newer generation/ : /another continuation/); + assert.equal(runner.requests.length, 2); + assert.equal(runner.writes(), 1); + assert.deepEqual(runner.requests[1], runner.requests[0]); + assert.deepEqual(await snapshot(f.run), accepted); + assert.deepEqual(await f.workers(), workers); + assert.deepEqual(await readdir(path.join(f.run.scanDir, "drafts")), []); + }); +} + +test("a failed accepted publication replay has no third attempt", async (t) => { + const f = await publicationFixture(t); + let accepted; + const failure = new Error("Synthetic replay failure"); + const runner = publicationRunner(f, { + before(attempt) { if (attempt === 2) throw failure; }, + async after() { accepted = await snapshot(f.run); throw new Error("Synthetic accepted response loss"); }, + }); + await assert.rejects(f.publish(runner.run), (error) => error === failure); + assert.equal(runner.requests.length, 2); + assert.equal(runner.writes(), 1); + assert.deepEqual(runner.requests[1], runner.requests[0]); + assert.deepEqual(await snapshot(f.run), accepted); + assert.deepEqual(await readdir(path.join(f.run.scanDir, "drafts")), []); +}); + +test("raw publication repeats the same staged paths without changing their bytes", async (t) => { + const f = await publicationFixture(t); + const workers = await f.workers(); + await f.publish(async (args, input) => { + const stagedPaths = ["--draft-path", "--checkpoint-path"].map((flag) => args[args.indexOf(flag) + 1]); + const staged = await Promise.all(stagedPaths.map((file) => readFile(file))); + const first = await f.runWorkbench(args, input); + const accepted = await snapshot(f.run); + assert.deepEqual(await Promise.all(stagedPaths.map((file) => readFile(file))), staged); + assert.deepEqual(await f.runWorkbench(args, input), first); + assert.deepEqual(await Promise.all(stagedPaths.map((file) => readFile(file))), staged); + assert.deepEqual(await snapshot(f.run), accepted); + }); + assert.deepEqual(await f.workers(), workers); + assert.deepEqual(await readdir(path.join(f.run.scanDir, "drafts")), []); +}); + +for (const changed of ["draftPath", "checkpointPath"]) { + test(`publication acknowledgment must match the original ${changed}`, async (t) => { + const f = await publicationFixture(t); + const lost = new Error("Synthetic response loss after different staged input"); + const runner = publicationRunner(f, { + async before(attempt, paths) { + const document = JSON.parse(await readFile(paths[changed], "utf8")); + document.threatModel = { summary: "Different staged input" }; + await writeFile(paths[changed], JSON.stringify(document)); + }, + after() { throw lost; }, + }); + await assert.rejects(f.publish(runner.run), (error) => error === lost); + assert.equal(runner.requests.length, 1); + assert.equal(runner.writes(), 1); + assert.deepEqual(await readdir(path.join(f.run.scanDir, "drafts")), []); + }); +} diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index f51ba74a5d..2ab6886583 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -1501,6 +1501,28 @@ def write_scan_draft(db: Any, connection: Any, args: Any) -> dict[str, Any]: filename, (json.dumps(document, allow_nan=False, indent=2) + "\n").encode(), ) + if ( + scan["mode"] == "deep" + and draft.get("deepScanPublication") is not None + and db.deep_scan.require_deep_scan_run(connection, scan_id)["workflow_version"] + in {"deep-scan-mcp/v1", "deep-security-scan/v1"} + ): + # Acknowledge this staged operation only after validation and all canonical writes. + acceptance = { + "status": "draft_written", + "input": { + **draft, + "checkpoint": checkpoint if args.checkpoint_path is not None else None, + }, + } + write_scan_local_bytes( + scan_dir, + Path(args.draft_path) + .with_suffix(".accepted.json") + .relative_to(scan_dir) + .as_posix(), + (json.dumps(acceptance, allow_nan=False, indent=2) + "\n").encode(), + ) # Accepted Standard drafts are evidence of review or report assembly, # even when the parent omitted its explicit progress call. if scan["mode"] == "standard": diff --git a/plugins/codex-security/tests/test_deep_scan_publication_authority.py b/plugins/codex-security/tests/test_deep_scan_publication_authority.py index 1d17ca8be7..b0eb00787c 100644 --- a/plugins/codex-security/tests/test_deep_scan_publication_authority.py +++ b/plugins/codex-security/tests/test_deep_scan_publication_authority.py @@ -141,3 +141,45 @@ def test_current_publication_replays_without_changing_checkpoint_or_worker_state assert dict(workbench_db.execute("SELECT * FROM deep_scan_workers").fetchone()) == worker_before findings = json.loads((scan.scan_dir / "findings.json").read_text())["findings"] assert findings[0]["title"] == "Accepted aggregate" + + +@pytest.mark.parametrize( + "failure", ["validation", "findings.json", "coverage.json", "scan-manifest.json"] +) +def test_failed_publication_does_not_acknowledge_staged_input( + workbench_api, workbench_db, publication_scan, monkeypatch, failure +): + scan = publication_scan() + result = add_worker(workbench_db, scan) + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_runs SET workflow_version = 'deep-scan-mcp/v1', " + "coordinator_generation = 2 WHERE scan_id = ?", + (scan.scan_id,), + ) + workbench_db.execute( + "UPDATE deep_scan_workers SET kind = 'dedup', merge_state = 'none' WHERE scan_id = ?", + (scan.scan_id,), + ) + args = stage_publication(scan, generation=2, result_path=result, title="Accepted aggregate") + staged = {path: path.read_bytes() for path in (scan.scan_dir / "drafts").iterdir()} + saved_results = workbench_api["saved_results"] + original_write = saved_results.write_scan_local_bytes + + def fail_validation(*args): + raise OSError("Synthetic validation failure") + + def fail_write(scan_dir, filename, contents): + if filename == failure: + raise OSError("Synthetic canonical write failure") + original_write(scan_dir, filename, contents) + + if failure == "validation": + monkeypatch.setattr(saved_results, "_validate_completion_binding", fail_validation) + else: + monkeypatch.setattr(saved_results, "write_scan_local_bytes", fail_write) + + with pytest.raises(OSError, match="Synthetic"): + workbench_api["write_scan_draft"](workbench_db, args) + + assert {path: path.read_bytes() for path in (scan.scan_dir / "drafts").iterdir()} == staged From 4a5983f7aac613131d2ee8eec12a6b5170857136 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 16 Sep 2026 09:23:03 +0000 Subject: [PATCH 05/23] Read reducer sequence from its worker directory --- .../mcp-app/src/deep-scan/coordinator.ts | 2 +- ...test_deep_scan_publication_integration.mjs | 26 ++++++++++++------- .../scripts/workbench_saved_results.py | 2 +- 3 files changed, 18 insertions(+), 12 deletions(-) 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 efb8b48dad..4e9dd08098 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/coordinator.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/coordinator.ts @@ -1184,7 +1184,7 @@ function compareCompletionSequence(left: AcceptedDiscovery, right: AcceptedDisco } function workerLabelSequence(worker: PersistedDeepScanWorker, kind: "discovery" | "dedup"): number { - const match = worker.promptPath.match(new RegExp(`${kind}-(\\d+)`)); + const match = basename(dirname(worker.promptPath)).match(new RegExp(`${kind}-(\\d+)`)); return match ? Number(match[1]) : 0; } diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.mjs b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.mjs index 270b9466bb..93bf5498db 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.mjs +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.mjs @@ -88,14 +88,18 @@ test("public partial drafts retain checkpoints after coordinator adoption", asyn assert.deepEqual(await readdir(path.join(run.scanDir, "drafts")), []); }); -for (const [label, offsets, ids] of [ +for (const [label, offsets, ids, paths, recoveryOnly] of [ ["increasing timestamps", [1, 2], [highId, lowId]], ["equal timestamps and descending UUIDs", [1, 1], [highId, lowId]], ["decreasing timestamps", [2, 1], [lowId, highId]], ["equal timestamps and ascending UUIDs", [1, 1], [lowId, highId]], + ["scan root collision, live publication", [1, 1], [highId, lowId], { scanRoot: "dedup-123/scans" }], + ["scan root collision, recovery", [1, 1], [highId, lowId], { scanRoot: "dedup-123/scans" }, true], + ["target name collision, live publication", [1, 1], [highId, lowId], { target: "dedup-456-target" }], + ["target name collision, recovery", [1, 1], [highId, lowId], { target: "dedup-456-target" }, true], ]) { test(`selected reducer survives recovery and public completion: ${label}`, async (t) => { - const fixture = await createFixture(t); + const fixture = await createFixture(t, paths); const { run, store, call, runWorkbench, instant } = fixture; assertSuccess(await call("record_codex_security_scan_draft", partial(run, "parent-checkpoint-only"))); const parentCheckpoints = await checkpoints(run); @@ -115,11 +119,13 @@ for (const [label, offsets, ids] of [ }, runWorkbench, undefined, { coordinatorGeneration, resultPath: results[index] }) ); - await publish(1); - const selected = await snapshot(run); - await assert.rejects(publish(0), /superseded publication selection/); - await assert.rejects(publish(1, 1), /newer generation/); - assert.deepEqual(await snapshot(run), selected); + if (!recoveryOnly) { + await publish(1); + const selected = await snapshot(run); + await assert.rejects(publish(0), /superseded publication selection/); + await assert.rejects(publish(1, 1), /newer generation/); + assert.deepEqual(await snapshot(run), selected); + } const publications = []; const coordinator = new DeepScanCoordinator({ @@ -159,21 +165,21 @@ for (const [label, offsets, ids] of [ }); } -async function createFixture(t) { +async function createFixture(t, paths = {}) { const root = await mkdtemp(path.join(tmpdir(), "deep-publication-")); let client; t.after(async () => { await client?.close(); await rm(root, { recursive: true, force: true }); }); - const targetPath = path.join(root, "target"); + const targetPath = path.join(root, paths.target ?? "target"); await mkdir(targetPath); await writeFile(path.join(targetPath, "fixture.py"), "# Synthetic publication fixture\n"); const environment = { ...process.env, CODEX_HOME: path.join(root, "home"), CODEX_SECURITY_STATE_DIR: path.join(root, "state"), - CODEX_SECURITY_SCAN_ROOT: path.join(root, "scans"), + CODEX_SECURITY_SCAN_ROOT: path.join(root, paths.scanRoot ?? "scans"), }; delete environment.CODEX_SECURITY_DEEP_SCAN_CONFIG_PATH; const instant = new Date().toISOString(); diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index f51ba74a5d..abb428519a 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -1412,7 +1412,7 @@ def _require_current_deep_publication( # Match the durable reducer sequence used by coordinator recovery. def reducer_order(worker: Any) -> tuple[int, str]: - match = re.search(r"dedup-(\d+)", worker["prompt_path"]) + match = re.search(r"dedup-(\d+)", Path(worker["prompt_path"]).parent.name) return (int(match[1]) if match else 0, worker["id"]) reducer = max( From 2cb364554bb11f38a5d64f7dcb1f832476f803a2 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 16 Sep 2026 13:11:07 +0000 Subject: [PATCH 06/23] Keep accepted publication successful when receipt I/O fails --- .../mcp-app/tests/clock_workbench.py | 10 +++ ...test_deep_scan_publication_integration.mjs | 29 ++++++++- .../scripts/workbench_saved_results.py | 20 +++--- .../test_deep_scan_publication_authority.py | 61 +++++++++++++++++++ 4 files changed, 109 insertions(+), 11 deletions(-) diff --git a/plugins/codex-security/mcp-app/tests/clock_workbench.py b/plugins/codex-security/mcp-app/tests/clock_workbench.py index 22d2c52b62..aa58a3a638 100644 --- a/plugins/codex-security/mcp-app/tests/clock_workbench.py +++ b/plugins/codex-security/mcp-app/tests/clock_workbench.py @@ -1,5 +1,6 @@ """Control the workbench clock while retaining its real commands and database writes.""" +import errno import os import runpy import sys @@ -11,4 +12,13 @@ api = runpy.run_path(str(source), run_name="test_workbench") instant = os.environ["TEST_WORKBENCH_NOW"] api["main"].__globals__["now"] = lambda: instant +if os.environ.get("TEST_WORKBENCH_RECEIPT_IO_FAILURE"): + original_replace = os.replace + + def replace(source, destination, *args, **kwargs): + if str(destination).endswith(".accepted.json"): + raise OSError(errno.ENOSPC, "Synthetic receipt I/O failure", destination) + return original_replace(source, destination, *args, **kwargs) + + os.replace = replace api["main"]() diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.mjs b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.mjs index 115980228e..09e8c71981 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.mjs +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.mjs @@ -337,15 +337,19 @@ function publicationRunner(fixture, { before, after } = {}) { } for (const workflow of ["deep-scan-mcp/v1", "deep-security-scan/v1"]) { - for (const loseResponse of [false, true]) { - test(`${workflow} finishes the same call after accepted publication (lost=${loseResponse})`, async (t) => { + for (const [loseResponse, receiptIoFailure] of [[false, false], [true, false], [false, true]]) { + test(`${workflow} finishes accepted publication (lost=${loseResponse}, receipt I/O failure=${receiptIoFailure})`, async (t) => { const f = await publicationFixture(t, workflow); + if (receiptIoFailure) f.environment.TEST_WORKBENCH_RECEIPT_IO_FAILURE = "1"; const before = await f.workers(); const sources = await Promise.all(f.results.map((file) => readFile(file))); let accepted; - const runner = publicationRunner(f, { after: async (attempt) => { + const runner = publicationRunner(f, { after: async (attempt, { draftPath }) => { if (attempt === 1) accepted = await snapshot(f.run); else assert.deepEqual(await snapshot(f.run), accepted); + if (receiptIoFailure) { + await assert.rejects(readFile(draftPath.replace(/\.json$/, ".accepted.json")), { code: "ENOENT" }); + } if (loseResponse && attempt === 1) throw new Error("Synthetic accepted publication response loss"); } }); let publications = 0; @@ -381,6 +385,25 @@ for (const workflow of ["deep-scan-mcp/v1", "deep-security-scan/v1"]) { } } +test("receipt I/O failure cannot authorize replay after response loss", async (t) => { + const f = await publicationFixture(t); + f.environment.TEST_WORKBENCH_RECEIPT_IO_FAILURE = "1"; + const workers = await f.workers(); + const lost = new Error("Synthetic accepted publication response loss"); + let accepted; + const runner = publicationRunner(f, { after: async (_attempt, { draftPath }) => { + accepted = await snapshot(f.run); + await assert.rejects(readFile(draftPath.replace(/\.json$/, ".accepted.json")), { code: "ENOENT" }); + throw lost; + } }); + await assert.rejects(f.publish(runner.run), (error) => error === lost); + assert.equal(runner.requests.length, 1); + assert.equal(runner.writes(), 1); + assert.deepEqual(await snapshot(f.run), accepted); + assert.deepEqual(await f.workers(), workers); + assert.deepEqual(await readdir(path.join(f.run.scanDir, "drafts")), []); +}); + for (const code of ["EACCES", "ECONNRESET"]) { for (const matching of [false, true]) { test(`publication ${code} before acceptance is one attempt (matching=${matching})`, async (t) => { diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index 47ecf160be..de10502d25 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -1515,14 +1515,18 @@ def write_scan_draft(db: Any, connection: Any, args: Any) -> dict[str, Any]: "checkpoint": checkpoint if args.checkpoint_path is not None else None, }, } - write_scan_local_bytes( - scan_dir, - Path(args.draft_path) - .with_suffix(".accepted.json") - .relative_to(scan_dir) - .as_posix(), - (json.dumps(acceptance, allow_nan=False, indent=2) + "\n").encode(), - ) + try: + write_scan_local_bytes( + scan_dir, + Path(args.draft_path) + .with_suffix(".accepted.json") + .relative_to(scan_dir) + .as_posix(), + (json.dumps(acceptance, allow_nan=False, indent=2) + "\n").encode(), + ) + except OSError: + # Publication succeeded; a missing receipt still prevents lost-response replay. + pass # Accepted Standard drafts are evidence of review or report assembly, # even when the parent omitted its explicit progress call. if scan["mode"] == "standard": diff --git a/plugins/codex-security/tests/test_deep_scan_publication_authority.py b/plugins/codex-security/tests/test_deep_scan_publication_authority.py index b0eb00787c..266d504f9d 100644 --- a/plugins/codex-security/tests/test_deep_scan_publication_authority.py +++ b/plugins/codex-security/tests/test_deep_scan_publication_authority.py @@ -1,9 +1,12 @@ from __future__ import annotations import copy +import errno import json +import os import uuid from argparse import Namespace +from pathlib import Path import pytest from test_deep_scan_successful_publication import add_worker @@ -183,3 +186,61 @@ def fail_write(scan_dir, filename, contents): workbench_api["write_scan_draft"](workbench_db, args) assert {path: path.read_bytes() for path in (scan.scan_dir / "drafts").iterdir()} == staged + + +@pytest.mark.parametrize("workflow", ["deep-scan-mcp/v1", "deep-security-scan/v1"]) +@pytest.mark.parametrize("failure", ["write", "rename"]) +def test_receipt_io_failure_preserves_successful_publication( + workbench_api, workbench_db, publication_scan, monkeypatch, workflow, failure +): + scan = publication_scan() + result = add_worker(workbench_db, scan) + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_runs SET workflow_version = ?, coordinator_generation = 2 " + "WHERE scan_id = ?", + (workflow, scan.scan_id), + ) + workbench_db.execute( + "UPDATE deep_scan_workers SET kind = 'dedup', merge_state = 'none' WHERE scan_id = ?", + (scan.scan_id,), + ) + args = stage_publication(scan, generation=2, result_path=result, title="Accepted aggregate") + draft = json.loads(Path(args.draft_path).read_text()) + staged = {path: path.read_bytes() for path in (scan.scan_dir / "drafts").iterdir()} + saved_results = workbench_api["saved_results"] + original_write = saved_results.write_scan_local_bytes + original_replace = os.replace + failures = [] + + def fail_receipt(filename): + failures.append(filename) + # The fault occurs only after all three canonical files have been published. + for name, document in ( + ("findings.json", draft["findings"]), + ("coverage.json", draft["coverage"]), + ("scan-manifest.json", draft["manifest"]), + ): + assert json.loads((scan.scan_dir / name).read_text()) == document + raise OSError(errno.ENOSPC, "Synthetic receipt I/O failure", filename) + + def write_file(scan_dir, filename, contents): + if failure == "write" and filename.endswith(".accepted.json"): + fail_receipt(filename) + return original_write(scan_dir, filename, contents) + + def replace_file(source, destination, *args, **kwargs): + if failure == "rename" and str(destination).endswith(".accepted.json"): + fail_receipt(destination) + return original_replace(source, destination, *args, **kwargs) + + monkeypatch.setattr(saved_results, "write_scan_local_bytes", write_file) + monkeypatch.setattr(os, "replace", replace_file) + + assert workbench_api["write_scan_draft"](workbench_db, args) == { + "scanId": scan.scan_id, + "status": "draft_written", + } + assert len(failures) == 1 + assert {path: path.read_bytes() for path in (scan.scan_dir / "drafts").iterdir()} == staged + assert len(list((scan.scan_dir / "checkpoints").glob("*.json"))) == 1 From 91515fad38556d651d8387e6ecc13edde66062a6 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 16 Sep 2026 13:46:36 +0000 Subject: [PATCH 07/23] Distinguish Windows receipt storage errors from path validation --- .../mcp-app/tests/clock_workbench.py | 13 +- .../scripts/finalize_scan_contract.py | 11 +- .../scripts/windows_scan_local_files.py | 8 +- .../scripts/workbench_saved_results.py | 3 +- .../test_deep_scan_publication_authority.py | 140 +++++++++++++++++- 5 files changed, 161 insertions(+), 14 deletions(-) diff --git a/plugins/codex-security/mcp-app/tests/clock_workbench.py b/plugins/codex-security/mcp-app/tests/clock_workbench.py index aa58a3a638..4bf9823890 100644 --- a/plugins/codex-security/mcp-app/tests/clock_workbench.py +++ b/plugins/codex-security/mcp-app/tests/clock_workbench.py @@ -13,12 +13,21 @@ instant = os.environ["TEST_WORKBENCH_NOW"] api["main"].__globals__["now"] = lambda: instant if os.environ.get("TEST_WORKBENCH_RECEIPT_IO_FAILURE"): - original_replace = os.replace + if os.name == "nt": + from finalize_scan_contract import _windows_scan_local_files + + backend = _windows_scan_local_files() + original_replace = backend._rename_handle + else: + original_replace = os.replace def replace(source, destination, *args, **kwargs): if str(destination).endswith(".accepted.json"): raise OSError(errno.ENOSPC, "Synthetic receipt I/O failure", destination) return original_replace(source, destination, *args, **kwargs) - os.replace = replace + if os.name == "nt": + backend._rename_handle = replace + else: + os.replace = replace api["main"]() diff --git a/plugins/codex-security/scripts/finalize_scan_contract.py b/plugins/codex-security/scripts/finalize_scan_contract.py index 58cac8f1f8..fa6557a819 100644 --- a/plugins/codex-security/scripts/finalize_scan_contract.py +++ b/plugins/codex-security/scripts/finalize_scan_contract.py @@ -83,6 +83,10 @@ class ContractError(ValueError): """Raised when a completed scan does not satisfy the additive contract.""" +class ScanLocalIOError(ContractError): + """Raised when scan-local storage I/O fails, distinct from path validation.""" + + class RecoverableContractError(ContractError): """Raised when report projection can safely be retried before publication.""" @@ -536,15 +540,18 @@ def write_scan_local_bytes( if not _descriptor_relative_writes_available(): if not _is_windows(): raise ContractError("scan-local output requires descriptor-relative file operations") + backend = _windows_scan_local_files() try: - _windows_scan_local_files().atomic_write( + backend.atomic_write( scan_dir, relative_path, payload, expected_root_identity=expected_root_identity, ) - except OSError as exc: + except backend.WindowsScanLocalPathError as exc: raise ContractError(f"{relative_path}: {exc}") from exc + except OSError as exc: + raise ScanLocalIOError(f"{relative_path}: {exc}") from exc return root_fd: int | None = None parent_fd: int | None = None diff --git a/plugins/codex-security/scripts/windows_scan_local_files.py b/plugins/codex-security/scripts/windows_scan_local_files.py index c67c86eb60..3997fd2cb0 100644 --- a/plugins/codex-security/scripts/windows_scan_local_files.py +++ b/plugins/codex-security/scripts/windows_scan_local_files.py @@ -37,6 +37,10 @@ class WindowsScanLocalFileError(OSError): """Raised when a scan-local operation cannot be completed securely.""" +class WindowsScanLocalPathError(WindowsScanLocalFileError): + """Raised when a scan-local path fails an integrity check.""" + + # CreateFile access and sharing flags. _DELETE = 0x00010000 _FILE_READ_ATTRIBUTES = 0x00000080 @@ -224,8 +228,8 @@ def _raise_last_error(operation: str, path: Path | None = None) -> None: raise WindowsScanLocalFileError(error, f"{operation}{target}: {detail}", str(path or "")) -def _invalid_path(path: Path | str, reason: str) -> WindowsScanLocalFileError: - return WindowsScanLocalFileError(errno.EINVAL, reason, str(path)) +def _invalid_path(path: Path | str, reason: str) -> WindowsScanLocalPathError: + return WindowsScanLocalPathError(errno.EINVAL, reason, str(path)) def _validated_parts(relative_path: str) -> tuple[str, ...]: diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index de10502d25..6322236d45 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -21,6 +21,7 @@ sys.path.insert(0, str(Path(__file__).resolve().parent)) from finalize_scan_contract import ( ContractError, + ScanLocalIOError, _finding_strength, _populate_unsealed_artifact_envelope, _populate_unsealed_manifest_envelope, @@ -1524,7 +1525,7 @@ def write_scan_draft(db: Any, connection: Any, args: Any) -> dict[str, Any]: .as_posix(), (json.dumps(acceptance, allow_nan=False, indent=2) + "\n").encode(), ) - except OSError: + except (OSError, ScanLocalIOError): # Publication succeeded; a missing receipt still prevents lost-response replay. pass # Accepted Standard drafts are evidence of review or report assembly, diff --git a/plugins/codex-security/tests/test_deep_scan_publication_authority.py b/plugins/codex-security/tests/test_deep_scan_publication_authority.py index 266d504f9d..e588b5ea0e 100644 --- a/plugins/codex-security/tests/test_deep_scan_publication_authority.py +++ b/plugins/codex-security/tests/test_deep_scan_publication_authority.py @@ -4,9 +4,12 @@ import errno import json import os +import sys import uuid from argparse import Namespace +from contextlib import contextmanager from pathlib import Path +from types import SimpleNamespace import pytest from test_deep_scan_successful_publication import add_worker @@ -147,7 +150,12 @@ def test_current_publication_replays_without_changing_checkpoint_or_worker_state @pytest.mark.parametrize( - "failure", ["validation", "findings.json", "coverage.json", "scan-manifest.json"] + "failure", + ["validation", "findings.json", "coverage.json", "scan-manifest.json"] + + [ + f"windows-emulated:{name}" + for name in ("findings.json", "coverage.json", "scan-manifest.json") + ], ) def test_failed_publication_does_not_acknowledge_staged_input( workbench_api, workbench_db, publication_scan, monkeypatch, failure @@ -175,6 +183,12 @@ def fail_validation(*args): def fail_write(scan_dir, filename, contents): if filename == failure: raise OSError("Synthetic canonical write failure") + if failure == f"windows-emulated:{filename}": + with monkeypatch.context() as patch: + finalizer = sys.modules[original_write.__module__] + backend = emulate_windows_atomic_write(patch, finalizer) + patch.setattr(backend, "_rename_handle", fail_validation) + return original_write(scan_dir, filename, contents) original_write(scan_dir, filename, contents) if failure == "validation": @@ -182,16 +196,65 @@ def fail_write(scan_dir, filename, contents): else: monkeypatch.setattr(saved_results, "write_scan_local_bytes", fail_write) - with pytest.raises(OSError, match="Synthetic"): + expected_error = ( + saved_results.ContractError if failure.startswith("windows-emulated:") else OSError + ) + with pytest.raises(expected_error, match="Synthetic"): workbench_api["write_scan_draft"](workbench_db, args) assert {path: path.read_bytes() for path in (scan.scan_dir / "drafts").iterdir()} == staged +def emulate_windows_atomic_write(monkeypatch, finalizer, *, reparse_point=False): + """Run the real Windows atomic writer with emulated handles, not native Win32 I/O.""" + backend = finalizer._windows_scan_local_files() + paths = {} + + @contextmanager + def locked_parent(scan_dir, relative_path, **kwargs): + parts = backend._validated_parts(relative_path) + yield scan_dir.joinpath(*parts[:-1]), parts[-1] + + def create_file(path, **kwargs): + descriptor = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + paths[descriptor] = path + return backend._OwnedHandle(descriptor) + + def rename_handle(handle, destination): + os.replace(paths[handle], destination) + paths[handle] = destination + + monkeypatch.setattr(finalizer, "_descriptor_relative_writes_available", lambda: False) + monkeypatch.setattr(finalizer, "_is_windows", lambda: True) + monkeypatch.setattr(backend, "_locked_parent", locked_parent) + monkeypatch.setattr(backend, "_validate_existing_output", lambda path: None) + monkeypatch.setattr(backend, "_create_file", create_file) + monkeypatch.setattr(backend, "_close_handle", os.close) + monkeypatch.setattr( + backend, + "_attributes", + lambda handle: SimpleNamespace( + FileAttributes=backend._FILE_ATTRIBUTE_REPARSE_POINT if reparse_point else 0 + ), + ) + monkeypatch.setattr( + backend, "_GetFileType", lambda handle: backend._FILE_TYPE_DISK, raising=False + ) + monkeypatch.setattr(backend, "_verify_handle_path", lambda *args: None) + monkeypatch.setattr(backend, "_write_all", os.write) + monkeypatch.setattr(backend, "_rename_handle", rename_handle) + monkeypatch.setattr(backend, "_mark_handle_for_deletion", lambda handle: paths[handle].unlink()) + return backend + + @pytest.mark.parametrize("workflow", ["deep-scan-mcp/v1", "deep-security-scan/v1"]) -@pytest.mark.parametrize("failure", ["write", "rename"]) +@pytest.mark.parametrize( + ("backend_kind", "failure"), + [("host", "write"), ("host", "rename")] + + [("windows-emulated", failure) for failure in ("create", "write", "rename")], +) def test_receipt_io_failure_preserves_successful_publication( - workbench_api, workbench_db, publication_scan, monkeypatch, workflow, failure + workbench_api, workbench_db, publication_scan, monkeypatch, workflow, backend_kind, failure ): scan = publication_scan() result = add_worker(workbench_db, scan) @@ -225,9 +288,29 @@ def fail_receipt(filename): raise OSError(errno.ENOSPC, "Synthetic receipt I/O failure", filename) def write_file(scan_dir, filename, contents): - if failure == "write" and filename.endswith(".accepted.json"): - fail_receipt(filename) - return original_write(scan_dir, filename, contents) + if not filename.endswith(".accepted.json"): + return original_write(scan_dir, filename, contents) + with monkeypatch.context() as patch: + finalizer = sys.modules[original_write.__module__] + if backend_kind == "windows-emulated": + backend = emulate_windows_atomic_write(patch, finalizer) + elif os.name == "nt": + backend = finalizer._windows_scan_local_files() + else: + if failure == "write": + fail_receipt(filename) + return original_write(scan_dir, filename, contents) + + def fail_backend(*args, **kwargs): + fail_receipt(filename) + + operation = { + "create": "_create_file", + "write": "_write_all", + "rename": "_rename_handle", + } + patch.setattr(backend, operation[failure], fail_backend) + return original_write(scan_dir, filename, contents) def replace_file(source, destination, *args, **kwargs): if failure == "rename" and str(destination).endswith(".accepted.json"): @@ -244,3 +327,46 @@ def replace_file(source, destination, *args, **kwargs): assert len(failures) == 1 assert {path: path.read_bytes() for path in (scan.scan_dir / "drafts").iterdir()} == staged assert len(list((scan.scan_dir / "checkpoints").glob("*.json"))) == 1 + + +@pytest.mark.parametrize("failure", ["validation", "unsafe-path", "windows-reparse-emulated"]) +def test_receipt_contract_errors_still_reject_publication( + workbench_api, workbench_db, publication_scan, monkeypatch, failure +): + scan = publication_scan() + result = add_worker(workbench_db, scan) + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_runs SET workflow_version = 'deep-scan-mcp/v1', " + "coordinator_generation = 2 WHERE scan_id = ?", + (scan.scan_id,), + ) + workbench_db.execute( + "UPDATE deep_scan_workers SET kind = 'dedup', merge_state = 'none' WHERE scan_id = ?", + (scan.scan_id,), + ) + args = stage_publication(scan, generation=2, result_path=result, title="Accepted aggregate") + staged = {path: path.read_bytes() for path in (scan.scan_dir / "drafts").iterdir()} + saved_results = workbench_api["saved_results"] + original_write = saved_results.write_scan_local_bytes + finalizer = sys.modules[original_write.__module__] + attempts = [] + + def write_file(scan_dir, filename, contents): + if not filename.endswith(".accepted.json"): + return original_write(scan_dir, filename, contents) + attempts.append(filename) + if failure == "validation": + raise finalizer.ContractError("Synthetic receipt validation failure") + if failure == "unsafe-path": + return original_write(scan_dir, "../outside.accepted.json", contents) + with monkeypatch.context() as patch: + emulate_windows_atomic_write(patch, finalizer, reparse_point=True) + return original_write(scan_dir, filename, contents) + + monkeypatch.setattr(saved_results, "write_scan_local_bytes", write_file) + with pytest.raises(finalizer.ContractError, match="validation|safe|reparse"): + workbench_api["write_scan_draft"](workbench_db, args) + assert len(attempts) == 1 + assert {path: path.read_bytes() for path in (scan.scan_dir / "drafts").iterdir()} == staged + assert not (scan.scan_dir.parent / "outside.accepted.json").exists() From 774b4d5690af94fcaefd155c869e5e600ca8f8a1 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 16 Sep 2026 14:00:07 +0000 Subject: [PATCH 08/23] Emulate Windows temporary-file deletion on handle close --- .../tests/test_deep_scan_publication_authority.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/plugins/codex-security/tests/test_deep_scan_publication_authority.py b/plugins/codex-security/tests/test_deep_scan_publication_authority.py index e588b5ea0e..0e176f78d2 100644 --- a/plugins/codex-security/tests/test_deep_scan_publication_authority.py +++ b/plugins/codex-security/tests/test_deep_scan_publication_authority.py @@ -209,6 +209,7 @@ def emulate_windows_atomic_write(monkeypatch, finalizer, *, reparse_point=False) """Run the real Windows atomic writer with emulated handles, not native Win32 I/O.""" backend = finalizer._windows_scan_local_files() paths = {} + pending_deletions = set() @contextmanager def locked_parent(scan_dir, relative_path, **kwargs): @@ -224,12 +225,17 @@ def rename_handle(handle, destination): os.replace(paths[handle], destination) paths[handle] = destination + def close_handle(handle): + os.close(handle) + if handle in pending_deletions: + paths[handle].unlink() + monkeypatch.setattr(finalizer, "_descriptor_relative_writes_available", lambda: False) monkeypatch.setattr(finalizer, "_is_windows", lambda: True) monkeypatch.setattr(backend, "_locked_parent", locked_parent) monkeypatch.setattr(backend, "_validate_existing_output", lambda path: None) monkeypatch.setattr(backend, "_create_file", create_file) - monkeypatch.setattr(backend, "_close_handle", os.close) + monkeypatch.setattr(backend, "_close_handle", close_handle) monkeypatch.setattr( backend, "_attributes", @@ -243,7 +249,7 @@ def rename_handle(handle, destination): monkeypatch.setattr(backend, "_verify_handle_path", lambda *args: None) monkeypatch.setattr(backend, "_write_all", os.write) monkeypatch.setattr(backend, "_rename_handle", rename_handle) - monkeypatch.setattr(backend, "_mark_handle_for_deletion", lambda handle: paths[handle].unlink()) + monkeypatch.setattr(backend, "_mark_handle_for_deletion", pending_deletions.add) return backend From 5e89f16169bdc79981ea006b3aee1a8419cccf9a Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Tue, 6 Oct 2026 09:54:49 +0000 Subject: [PATCH 09/23] fix(deep): preserve terminal aggregates during completion replay --- .../test_deep_scan_publication_integration.ts | 74 +++++++++++++++++++ .../scripts/workbench_saved_results.py | 28 +++---- 2 files changed, 89 insertions(+), 13 deletions(-) diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts index b736b6ab1f..982b9de345 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts @@ -328,6 +328,80 @@ for (const [label, offsets, ids, paths, recoveryOnly] of scenarios) { }); } +test("partial drafts cannot replace a terminal aggregate awaiting completion replay", async (t) => { + const fixture = await createFixture(t); + const { run, store, call, runWorkbench, instant } = fixture; + const claimed = await store.claimCoordinator({ + scanId: run.scanId, + threadId: owner, + }); + const context = await createScanArtifactContext(run.scanId, runWorkbench, { + requireRunning: true, + }); + const elapsedSeconds = ((run.config.maxTimeHours ?? 1) + 1) * 3_600; + fixture.setTime(elapsedSeconds); + const deadline = Date.parse(instant) + elapsedSeconds * 1_000; + const coordinator = new DeepScanCoordinator({ + run: claimed.run, + store, + pluginRoot, + executor: { + run() { + throw new Error("Expired coordinator must not start a worker."); + }, + }, + clock: { now: () => deadline, sleep: async () => {} }, + onComplete: (draft, signal, publication) => + recordCodexSecurityScanDraftViaWorkbench( + context, + draft, + runWorkbench, + signal, + publication, + ).then(() => {}), + }); + coordinator.start(); + const terminal = await coordinator.wait(undefined, 30_000); + assert.equal( + terminal?.status, + "succeeded", + terminal?.error ?? "Coordinator did not succeed", + ); + assert.equal(terminal?.terminalReason, "capped"); + + const report = path.join(run.scanDir, "report.html"); + await mkdir(report); + assertToolError( + await call("complete_codex_security_scan", { scanId: run.scanId }), + /report\.html/, + ); + await rm(report, { recursive: true }); + const interrupted = await snapshot(run); + assert.equal( + JSON.parse(interrupted.files["coverage.json"]).completeness, + "partial", + ); + assertToolError( + await call( + "record_codex_security_scan_draft", + partial(run, "late-progress"), + ), + /current coordinator lease/, + ); + assert.deepEqual(await snapshot(run), interrupted); + assertSuccess( + await call("complete_codex_security_scan", { scanId: run.scanId }), + ); + const completed = await snapshot(run); + assert.ok(JSON.parse(completed.files["scan-manifest.json"]).scan.sealedAt); + assert.deepEqual( + JSON.parse(completed.files["coverage.json"]), + JSON.parse(interrupted.files["coverage.json"]), + ); + assert.deepEqual(completed.checkpoints, interrupted.checkpoints); + assert.equal((await store.get(run.scanId, owner)).terminalReason, "capped"); +}); + async function createFixture(t: TestContext, paths: FixturePaths = {}) { const root = await mkdtemp(path.join(tmpdir(), "deep-publication-")); let client: Client | undefined; diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index b295737da2..ac40cafe2c 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -2386,23 +2386,17 @@ def save_scan_artifact(db: Any, connection: Any, args: Any) -> dict[str, Any]: return {"scanId": scan_id, "path": str(scan_dir / output)} -def _read_staged_scan_draft(scan_dir: Path, draft_path: str) -> dict[str, Any]: - try: - relative = Path(draft_path).relative_to(scan_dir).as_posix() - except ValueError as exc: - raise SystemExit("Scan draft must be inside the registered scan drafts directory.") from exc - if not re.fullmatch(r"drafts/[0-9a-fA-F-]+\.json", relative): - raise SystemExit("Scan draft must be inside the registered scan drafts directory.") - return _read_scan_local_json(scan_dir, relative, "Staged scan draft") - - def _require_current_deep_publication( db: Any, connection: Any, scan_id: str, draft: dict[str, Any] ) -> None: + run = db.deep_scan.require_deep_scan_run(connection, scan_id) publication = draft.get("deepScanPublication") - if publication is None and draft["manifest"]["scan"].get("complete") is False: + if ( + publication is None + and draft["manifest"]["scan"].get("complete") is False + and run["status"] == "running" + ): return - run = db.deep_scan.require_deep_scan_run(connection, scan_id) db.deep_scan.require_current_coordinator( run, argparse.Namespace( @@ -2456,7 +2450,15 @@ def write_scan_draft(db: Any, connection: Any, args: Any) -> dict[str, Any]: raise SystemExit( "scan_draft_conflict: canonical scan results changed; reconcile the saved checkpoint again." ) - draft = _read_staged_scan_draft(scan_dir, args.draft_path) + try: + relative = Path(args.draft_path).relative_to(scan_dir).as_posix() + except ValueError as exc: + raise SystemExit( + "Scan draft must be inside the registered scan drafts directory." + ) from exc + if not re.fullmatch(r"drafts/[0-9a-fA-F-]+\.json", relative): + raise SystemExit("Scan draft must be inside the registered scan drafts directory.") + draft = _read_scan_local_json(scan_dir, relative, "Staged scan draft") if scan["mode"] == "deep": _require_current_deep_publication(db, connection, scan_id, draft) manifest, findings, coverage = draft["manifest"], draft["findings"], draft["coverage"] From dabab962104e665ef7f48597346cc260a38b74b7 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Tue, 6 Oct 2026 11:46:02 +0000 Subject: [PATCH 10/23] fix(deep): preserve accepted publication results during recovery --- .../mcp-app/src/artifact-scan-draft.ts | 12 +- .../test_deep_scan_publication_integration.ts | 77 +++++++++++- .../scripts/finalize_scan_contract.py | 22 +++- .../scripts/workbench_saved_results.py | 114 +++++++++++------- .../test_deep_scan_publication_authority.py | 90 +++++++++++++- 5 files changed, 255 insertions(+), 60 deletions(-) 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 40b715b382..0f80ddc826 100644 --- a/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts +++ b/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts @@ -275,13 +275,11 @@ export async function recordCodexSecurityScanDraftViaWorkbench( "accepted scan draft", ).catch(() => undefined); if ( + accepted?.status === "draft_written" && isDeepStrictEqual( - accepted, + accepted.input, JSON.parse( - JSON.stringify({ - status: "draft_written", - input: { ...stagedDraft, checkpoint: snapshot }, - }), + JSON.stringify({ ...stagedDraft, checkpoint: snapshot }), ), ) ) { @@ -301,7 +299,9 @@ export async function recordCodexSecurityScanDraftViaWorkbench( await Promise.all([ fs.rm(checkpointPath, { force: true }), fs.rm(draftPath, { force: true }), - fs.rm(join(dirname(draftPath), acceptanceName), { force: true }), + fs + .rm(join(dirname(draftPath), acceptanceName), { force: true }) + .catch(() => {}), ]); } }, diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts index c51c4403fa..8d57666c29 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts @@ -601,6 +601,7 @@ async function snapshot(run: DeepScanRunState) { "findings.json", "coverage.json", "report.md", + "checkpoint-head.json", ]) { try { files[name] = await readFile(path.join(run.scanDir, name), "utf8"); @@ -765,8 +766,10 @@ for (const workflow of ["deep-scan-mcp/v1", "deep-security-scan/v1"]) { { code: "ENOENT" }, ); } - if (loseResponse && attempt === 1) + if (loseResponse && attempt === 1) { + f.setTime(10); throw new Error("Synthetic accepted publication response loss"); + } }, }); let publications = 0; @@ -836,6 +839,71 @@ for (const workflow of ["deep-scan-mcp/v1", "deep-security-scan/v1"]) { } } +test("accepted response recovery preserves an omitted threat model and the accepted bytes", async (t) => { + const f = await publicationFixture(t); + await f.publish(); + f.setTime(5); + delete f.draft.threatModel; + let accepted: Awaited> | undefined; + const runner = publicationRunner(f, { + async after(attempt) { + if (attempt === 1) { + accepted = await snapshot(f.run); + f.setTime(10); + throw new Error("Synthetic accepted publication response loss"); + } + }, + }); + assert.equal((await f.publish(runner.run)).status, "draft_written"); + assert.ok(runner.requests[0].args.includes("--expected-draft-digest")); + assert.equal(runner.requests.length, 2); + assert.deepEqual(runner.requests[1], runner.requests[0]); + assert.deepEqual(await snapshot(f.run), accepted); +}); + +test("accepted response recovery cannot replace a later publication from the same coordinator", async (t) => { + const f = await publicationFixture(t); + let newer: Awaited> | undefined; + const runner = publicationRunner(f, { + async after(attempt) { + assert.equal(attempt, 1, "The superseded replay cannot publish"); + f.setTime(10); + f.draft.threatModel = { summary: "Later accepted model" }; + await f.publish(); + newer = await snapshot(f.run); + throw new Error("Synthetic accepted publication response loss"); + }, + }); + await assert.rejects(f.publish(runner.run), /canonical scan results changed/); + assert.equal(runner.requests.length, 2); + assert.deepEqual(await snapshot(f.run), newer); +}); + +for (const code of ["EPERM", "EACCES", "EIO"]) { + test(`optional receipt cleanup ${code} preserves publication success`, async (t) => { + const f = await publicationFixture(t); + const filesystem = await import("node:fs"); + const original = filesystem.promises.rm; + t.mock.method( + filesystem.promises, + "rm", + async (...[file, options]: Parameters) => { + if (String(file).endsWith(".accepted.json")) + throw Object.assign(new Error(`Synthetic receipt cleanup ${code}`), { + code, + }); + return original(file, options); + }, + ); + assert.equal((await f.publish()).status, "draft_written"); + assert.ok( + JSON.parse((await snapshot(f.run)).files["scan-manifest.json"]).scan + .completedAt, + ); + t.mock.restoreAll(); + }); +} + test("receipt I/O failure cannot authorize replay after response loss", async (t) => { const f = await publicationFixture(t); f.environment.TEST_WORKBENCH_RECEIPT_IO_FAILURE = "1"; @@ -955,7 +1023,13 @@ test("a failed accepted publication replay has no third attempt", async (t) => { test("raw publication repeats the same staged paths without changing their bytes", async (t) => { const f = await publicationFixture(t); const workers = await f.workers(); + let calls = 0; await f.publish(async (args, input) => { + assert.equal( + ++calls, + 1, + "Raw replay assertions must not trigger host recovery", + ); const stagedPaths = ["--draft-path", "--checkpoint-path"].map( (flag) => args[args.indexOf(flag) + 1], ); @@ -966,6 +1040,7 @@ test("raw publication repeats the same staged paths without changing their bytes await Promise.all(stagedPaths.map((file) => readFile(file))), staged, ); + f.setTime(10); assert.deepEqual(await f.runWorkbench(args, input), first); assert.deepEqual( await Promise.all(stagedPaths.map((file) => readFile(file))), diff --git a/plugins/codex-security/scripts/finalize_scan_contract.py b/plugins/codex-security/scripts/finalize_scan_contract.py index f23132b2cf..ca3933f646 100644 --- a/plugins/codex-security/scripts/finalize_scan_contract.py +++ b/plugins/codex-security/scripts/finalize_scan_contract.py @@ -467,18 +467,28 @@ def _windows_unsafe_path_component(value: str) -> bool: return WINDOWS_UNSAFE_PATH_COMPONENT_RE.search(value) is not None +def _scan_local_directory_error(message: str, error: OSError) -> ContractError: + if error.errno in {errno.ENOENT, errno.ENOTDIR, errno.ELOOP}: + return ContractError(message) + return ScanLocalIOError(message) + + def _require_scan_directory(scan_dir: Path) -> Path: scan_dir = scan_dir.absolute() try: metadata = scan_dir.lstat() except OSError as exc: - raise ContractError("scan directory: expected an existing non-symlink directory") from exc + raise _scan_local_directory_error( + "scan directory: expected an existing non-symlink directory", exc + ) from exc if not stat.S_ISDIR(metadata.st_mode): raise ContractError("scan directory: expected an existing non-symlink directory") try: resolved = scan_dir.resolve(strict=True) except OSError as exc: - raise ContractError("scan directory: expected an existing non-symlink directory") from exc + raise _scan_local_directory_error( + "scan directory: expected an existing non-symlink directory", exc + ) from exc if os.path.normcase(resolved) != os.path.normcase(scan_dir): raise ContractError("scan directory: expected a canonical non-symlink directory") return resolved @@ -544,7 +554,9 @@ def _open_verified_scan_directory( os.O_RDONLY | getattr(os, "O_DIRECTORY", 0) | getattr(os, "O_NOFOLLOW", 0), ) except OSError as exc: - raise ContractError("scan directory: expected an existing non-symlink directory") from exc + raise _scan_local_directory_error( + "scan directory: expected an existing non-symlink directory", exc + ) from exc opened = os.fstat(descriptor) opened_identity = (opened.st_dev, opened.st_ino) if opened_identity != observed_identity or ( @@ -789,8 +801,8 @@ def write_scan_local_bytes( try: parent_fd = _open_scan_local_directory(root_fd, parts[:-1], create=True) except OSError as exc: - raise ContractError( - f"{relative_path}: expected a path inside the scan directory" + raise _scan_local_directory_error( + f"{relative_path}: expected a path inside the scan directory", exc ) from exc # The held descriptor is the authority for the validated parent. A # concurrent rename cannot redirect later operations through a diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index 607d586aa6..ceb63bf298 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -2444,13 +2444,6 @@ def write_scan_draft(db: Any, connection: Any, args: Any) -> dict[str, Any]: "The scan stopped; its saved checkpoint was retained without replacing sealed results." ) scan_dir = db.require_canonical_scan_directory(Path(scan["scan_dir"])) - if ( - args.expected_draft_digest is not None - and args.expected_draft_digest != _scan_draft_digest(scan_dir) - ): - raise SystemExit( - "scan_draft_conflict: canonical scan results changed; reconcile the saved checkpoint again." - ) try: relative = Path(args.draft_path).relative_to(scan_dir).as_posix() except ValueError as exc: @@ -2487,6 +2480,37 @@ def write_scan_draft(db: Any, connection: Any, args: Any) -> dict[str, Any]: accepted_checkpoint = checkpoint if checkpoint.get("scanId") != scan_id: raise SystemExit("Staged scan checkpoint belongs to another scan.") + acceptance_input = {**accepted_input, "checkpoint": accepted_checkpoint} + acceptance_relative = Path(relative).with_suffix(".accepted.json").as_posix() + acknowledge = ( + scan["mode"] == "deep" + and draft.get("deepScanPublication") is not None + and db.deep_scan.require_deep_scan_run(connection, scan_id)["workflow_version"] + in {"deep-scan-mcp/v1", "deep-security-scan/v1"} + ) + if acknowledge and (scan_dir / acceptance_relative).exists(): + accepted = _read_scan_local_json(scan_dir, acceptance_relative, "Accepted scan draft") + if ( + accepted.get("status") == "draft_written" + and accepted.get("input") == acceptance_input + ): + if accepted.get("draftDigest") != _scan_draft_digest(scan_dir): + raise SystemExit( + "scan_draft_conflict: canonical scan results changed after publication." + ) + return { + "scanId": scan_id, + "status": "draft_written", + **({"warnings": accepted["warnings"]} if accepted.get("warnings") else {}), + } + if ( + args.expected_draft_digest is not None + and args.expected_draft_digest != _scan_draft_digest(scan_dir) + ): + raise SystemExit( + "scan_draft_conflict: canonical scan results changed; reconcile the saved checkpoint again." + ) + if args.checkpoint_path is not None: checkpoint_digest = hashlib.sha256(checkpoint_contents).hexdigest() write_scan_local_bytes( scan_dir, @@ -2502,43 +2526,30 @@ def write_scan_draft(db: Any, connection: Any, args: Any) -> dict[str, Any]: write_scan_local_bytes( scan_dir, "checkpoint-head.json", _encoded({"checkpoint": checkpoint_name}) ) - for filename, document in ( - ("findings.json", findings), - ("coverage.json", coverage), - ("scan-manifest.json", manifest), - ): - write_scan_local_bytes( - scan_dir, - filename, - (json.dumps(document, allow_nan=False, indent=2) + "\n").encode(), + documents = { + filename: (json.dumps(document, allow_nan=False, indent=2) + "\n").encode() + for filename, document in ( + ("findings.json", findings), + ("coverage.json", coverage), + ("scan-manifest.json", manifest), ) - if ( - scan["mode"] == "deep" - and draft.get("deepScanPublication") is not None - and db.deep_scan.require_deep_scan_run(connection, scan_id)["workflow_version"] - in {"deep-scan-mcp/v1", "deep-security-scan/v1"} - ): - # Acknowledge this staged operation only after validation and all canonical writes. - acceptance = { - "status": "draft_written", - "input": { - **accepted_input, - "checkpoint": accepted_checkpoint, - }, - } + } + for filename, contents in documents.items(): + write_scan_local_bytes(scan_dir, filename, contents) + model_warning = write_threat_model_projection_if_possible(scan_dir, manifest) + if acknowledge: + # Optional recovery evidence is written only after the publication succeeds. try: - write_scan_local_bytes( - scan_dir, - Path(args.draft_path) - .with_suffix(".accepted.json") - .relative_to(scan_dir) - .as_posix(), - (json.dumps(acceptance, allow_nan=False, indent=2) + "\n").encode(), - ) + acceptance = { + "status": "draft_written", + "input": acceptance_input, + "draftDigest": _scan_draft_contents_digest(documents), + **({"warnings": [model_warning]} if model_warning else {}), + } + write_scan_local_bytes(scan_dir, acceptance_relative, _encoded(acceptance)) except (OSError, ScanLocalIOError): - # Publication succeeded; a missing receipt still prevents lost-response replay. + # A missing receipt prevents recovery but does not undo accepted results. pass - model_warning = write_threat_model_projection_if_possible(scan_dir, manifest) # Accepted Standard drafts are evidence of review or report assembly, # even when the parent omitted its explicit progress call. model_only_checkpoint = ( @@ -2577,20 +2588,31 @@ def write_scan_draft(db: Any, connection: Any, args: Any) -> dict[str, Any]: def _scan_draft_digest(scan_dir: Path) -> str: - digest = hashlib.sha256() + documents: dict[str, bytes | None] = {} for filename in ("scan-manifest.json", "findings.json", "coverage.json"): - digest.update(filename.encode()) - digest.update(b"\0") try: (scan_dir / filename).lstat() except FileNotFoundError: - digest.update(b"missing\0") + documents[filename] = None continue - digest.update(b"present\0") descriptor = open_scan_local_file_descriptor(scan_dir, filename, filename) with os.fdopen(descriptor, "rb") as handle: - digest.update(handle.read()) + documents[filename] = handle.read() + return _scan_draft_contents_digest(documents) + + +def _scan_draft_contents_digest(documents: dict[str, bytes | None]) -> str: + digest = hashlib.sha256() + for filename in ("scan-manifest.json", "findings.json", "coverage.json"): + digest.update(filename.encode()) digest.update(b"\0") + contents = documents[filename] + if contents is None: + digest.update(b"missing\0") + else: + digest.update(b"present\0") + digest.update(contents) + digest.update(b"\0") return digest.hexdigest() diff --git a/plugins/codex-security/tests/test_deep_scan_publication_authority.py b/plugins/codex-security/tests/test_deep_scan_publication_authority.py index 4aa996265c..fdc0b3d81b 100644 --- a/plugins/codex-security/tests/test_deep_scan_publication_authority.py +++ b/plugins/codex-security/tests/test_deep_scan_publication_authority.py @@ -270,12 +270,15 @@ def close_handle(handle): @pytest.mark.parametrize("workflow", ["deep-scan-mcp/v1", "deep-security-scan/v1"]) @pytest.mark.parametrize( ("backend_kind", "failure"), - [("host", "write"), ("host", "rename")] + [("host", failure) for failure in ("write", "rename")] + + [("posix", failure) for failure in ("root-stat", "root-resolve", "root-open", "parent-open")] + [("windows-emulated", failure) for failure in ("create", "write", "rename")], ) def test_receipt_io_failure_preserves_successful_publication( workbench_api, workbench_db, publication_scan, monkeypatch, workflow, backend_kind, failure ): + if backend_kind == "posix" and os.name == "nt": + pytest.skip("POSIX descriptor-relative directory errors") scan = publication_scan() result = add_worker(workbench_db, scan) with workbench_db: @@ -316,13 +319,47 @@ def fail_receipt(filename): ("scan-manifest.json", draft["manifest"]), ): assert json.loads((scan.scan_dir / name).read_text()) == document - raise OSError(errno.ENOSPC, "Synthetic receipt I/O failure", filename) + raise OSError( + errno.EIO if backend_kind == "posix" else errno.ENOSPC, + "Synthetic receipt I/O failure", + filename, + ) def write_file(scan_dir, filename, contents): if not filename.endswith(".accepted.json"): return original_write(scan_dir, filename, contents) with monkeypatch.context() as patch: finalizer = sys.modules[original_write.__module__] + if backend_kind == "posix": + original_open = os.open + original_stat = Path.lstat + original_resolve = Path.resolve + + def open_directory(file, flags, *args, **kwargs): + if (failure == "root-open" and Path(file) == scan_dir) or ( + failure == "parent-open" and file == "drafts" + ): + fail_receipt(filename) + return original_open(file, flags, *args, **kwargs) + + def stat_root(path, *args, **kwargs): + if path == scan_dir: + fail_receipt(filename) + return original_stat(path, *args, **kwargs) + + def resolve_root(path, *args, **kwargs): + if path == scan_dir: + fail_receipt(filename) + return original_resolve(path, *args, **kwargs) + + if failure == "root-stat": + patch.setattr(Path, "lstat", stat_root) + elif failure == "root-resolve": + patch.setattr(Path, "resolve", resolve_root) + else: + patch.setattr(os, "open", open_directory) + patch.setattr(os, "supports_dir_fd", {*os.supports_dir_fd, open_directory}) + return original_write(scan_dir, filename, contents) if backend_kind == "windows-emulated": backend = emulate_windows_atomic_write(patch, finalizer) elif os.name == "nt": @@ -405,3 +442,52 @@ def write_file(scan_dir, filename, contents): assert len(attempts) == 1 assert {path: path.read_bytes() for path in (scan.scan_dir / "drafts").iterdir()} == staged assert not (scan.scan_dir.parent / "outside.accepted.json").exists() + + +def test_accepted_publication_replay_retains_original_warning_and_bytes( + workbench_api, workbench_db, publication_scan, monkeypatch +): + scan = publication_scan() + result = add_worker(workbench_db, scan) + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_runs SET coordinator_generation = 2 WHERE scan_id = ?", + (scan.scan_id,), + ) + workbench_db.execute( + "UPDATE deep_scan_workers SET kind = 'dedup', merge_state = 'none' WHERE scan_id = ?", + (scan.scan_id,), + ) + args = stage_publication(scan, generation=2, result_path=result, title="Accepted aggregate") + saved = workbench_api["saved_results"] + args.expected_draft_digest = saved._scan_draft_digest(scan.scan_dir) + warnings = iter(["Original projection warning", "Replay must not reproject"]) + monkeypatch.setattr( + saved, "write_threat_model_projection_if_possible", lambda *_: next(warnings) + ) + first = saved.write_scan_draft(workbench_api["_WORKBENCH_DB_CONTEXT"], workbench_db, args) + published = { + path: path.read_bytes() + for path in scan.scan_dir.rglob("*.json") + if "drafts" not in path.parts + } + assert args.expected_draft_digest != saved._scan_draft_digest(scan.scan_dir) + monkeypatch.setattr( + workbench_api["_WORKBENCH_DB_CONTEXT"], "now", lambda: "2099-01-01T00:00:00Z" + ) + + assert ( + saved.write_scan_draft(workbench_api["_WORKBENCH_DB_CONTEXT"], workbench_db, args) + == first + == { + "scanId": scan.scan_id, + "status": "draft_written", + "warnings": ["Original projection warning"], + } + ) + assert { + path: path.read_bytes() + for path in scan.scan_dir.rglob("*.json") + if "drafts" not in path.parts + } == published + assert next(warnings) == "Replay must not reproject" From 02a7c3913b66ed3fb90b2cb502beea796b9cfc75 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Tue, 6 Oct 2026 12:16:08 +0000 Subject: [PATCH 11/23] fix(deep): retain Windows receipt probe I/O errors --- .../scripts/windows_scan_local_files.py | 2 + .../test_deep_scan_publication_authority.py | 136 +++++++++++++++--- 2 files changed, 121 insertions(+), 17 deletions(-) diff --git a/plugins/codex-security/scripts/windows_scan_local_files.py b/plugins/codex-security/scripts/windows_scan_local_files.py index 005daece5c..ae9558eb73 100644 --- a/plugins/codex-security/scripts/windows_scan_local_files.py +++ b/plugins/codex-security/scripts/windows_scan_local_files.py @@ -368,6 +368,8 @@ def _canonical_scan_directory(scan_dir: Path) -> tuple[Path, tuple[int, int]]: expected = absolute.lstat() canonical = absolute.resolve(strict=True) except OSError as exc: + if exc.errno not in {errno.ENOENT, errno.ENOTDIR, errno.ELOOP}: + raise raise _invalid_path(scan_dir, "expected an existing scan directory") from exc if _normalized_windows_path(absolute) != _normalized_windows_path(canonical): raise _invalid_path(scan_dir, "scan directory must be canonical and non-reparse") diff --git a/plugins/codex-security/tests/test_deep_scan_publication_authority.py b/plugins/codex-security/tests/test_deep_scan_publication_authority.py index fdc0b3d81b..ce69e6b282 100644 --- a/plugins/codex-security/tests/test_deep_scan_publication_authority.py +++ b/plugins/codex-security/tests/test_deep_scan_publication_authority.py @@ -7,7 +7,6 @@ import sys import uuid from argparse import Namespace -from contextlib import contextmanager from pathlib import Path from types import SimpleNamespace @@ -224,14 +223,17 @@ def emulate_windows_atomic_write(monkeypatch, finalizer, *, reparse_point=False) backend = finalizer._windows_scan_local_files() paths = {} pending_deletions = set() - - @contextmanager - def locked_parent(scan_dir, relative_path, **kwargs): - parts = backend._validated_parts(relative_path) - yield scan_dir.joinpath(*parts[:-1]), parts[-1] + next_directory_handle = -1 def create_file(path, **kwargs): - descriptor = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + nonlocal next_directory_handle + if kwargs["disposition"] == backend._OPEN_EXISTING: + if kwargs.get("missing_ok") and not path.exists(): + return None + descriptor = next_directory_handle + next_directory_handle -= 1 + else: + descriptor = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) paths[descriptor] = path return backend._OwnedHandle(descriptor) @@ -240,27 +242,29 @@ def rename_handle(handle, destination): paths[handle] = destination def close_handle(handle): - os.close(handle) + if handle >= 0: + os.close(handle) if handle in pending_deletions: paths[handle].unlink() + def attributes(handle): + path = paths[handle] + flags = backend._FILE_ATTRIBUTE_DIRECTORY if path.is_dir() else 0 + if path.is_symlink() or (reparse_point and not path.is_dir()): + flags |= backend._FILE_ATTRIBUTE_REPARSE_POINT + return SimpleNamespace(FileAttributes=flags) + monkeypatch.setattr(finalizer, "_descriptor_relative_writes_available", lambda: False) monkeypatch.setattr(finalizer, "_is_windows", lambda: True) - monkeypatch.setattr(backend, "_locked_parent", locked_parent) + monkeypatch.setattr(backend, "_require_windows", lambda: None) monkeypatch.setattr(backend, "_validate_existing_output", lambda path: None) monkeypatch.setattr(backend, "_create_file", create_file) monkeypatch.setattr(backend, "_close_handle", close_handle) - monkeypatch.setattr( - backend, - "_attributes", - lambda handle: SimpleNamespace( - FileAttributes=backend._FILE_ATTRIBUTE_REPARSE_POINT if reparse_point else 0 - ), - ) + monkeypatch.setattr(backend, "_attributes", attributes) monkeypatch.setattr( backend, "_GetFileType", lambda handle: backend._FILE_TYPE_DISK, raising=False ) - monkeypatch.setattr(backend, "_verify_handle_path", lambda *args: None) + monkeypatch.setattr(backend, "_final_path", lambda handle, **kwargs: str(paths[handle])) monkeypatch.setattr(backend, "_write_all", os.write) monkeypatch.setattr(backend, "_rename_handle", rename_handle) monkeypatch.setattr(backend, "_mark_handle_for_deletion", pending_deletions.add) @@ -399,6 +403,104 @@ def replace_file(source, destination, *args, **kwargs): assert len(list((scan.scan_dir / "checkpoints").glob("*.json"))) == 2 +@pytest.mark.parametrize("workflow", ["deep-scan-mcp/v1", "deep-security-scan/v1"]) +@pytest.mark.parametrize("operation", ["lstat", "resolve"]) +@pytest.mark.parametrize("failure", ["io", "missing", "not-directory", "loop"]) +def test_windows_receipt_root_probe_classifies_storage_and_path_failures( + workbench_api, + workbench_db, + publication_scan, + monkeypatch, + tmp_path, + workflow, + operation, + failure, +): + scan = publication_scan() + result = add_worker(workbench_db, scan) + with workbench_db: + workbench_db.execute( + "UPDATE deep_scan_runs SET workflow_version = ?, coordinator_generation = 2 " + "WHERE scan_id = ?", + (workflow, scan.scan_id), + ) + workbench_db.execute( + "UPDATE deep_scan_workers SET kind = 'dedup', merge_state = 'none' WHERE scan_id = ?", + (scan.scan_id,), + ) + args = stage_publication(scan, generation=2, result_path=result, title="Accepted aggregate") + bad_root = tmp_path / "invalid-root" + if failure == "not-directory": + bad_root.write_text("ordinary file") + bad_root /= "child" + elif failure == "loop": + try: + bad_root.symlink_to(bad_root, target_is_directory=True) + except OSError as exc: + pytest.skip(f"creating a synthetic symlink loop requires host support: {exc}") + bad_root /= "child" + saved = workbench_api["saved_results"] + original_write = saved.write_scan_local_bytes + finalizer = sys.modules[original_write.__module__] + published = {} + probes = [] + + def write_file(scan_dir, filename, contents): + if not filename.endswith(".accepted.json"): + return original_write(scan_dir, filename, contents) + published.update( + (path, path.read_bytes()) + for path in scan_dir.rglob("*.json") + if "drafts" not in path.parts + ) + assert json.loads(published[scan_dir / "findings.json"])["findings"][0]["title"] == ( + "Accepted aggregate" + ) + with monkeypatch.context() as patch: + backend = emulate_windows_atomic_write(patch, finalizer) + original_probe = backend._canonical_scan_directory + original_operation = getattr(Path, operation) + + def fail_root(path, *args, **kwargs): + if path == scan_dir: + probes.append(operation) + if failure == "io": + raise OSError(errno.EIO, "Synthetic receipt root I/O failure", str(path)) + # Exercise actual missing, non-directory, and loop errors without + # changing the already accepted canonical artifacts. + return original_operation(bad_root, *args, **kwargs) + return original_operation(path, *args, **kwargs) + + def probe_root(path): + # The common finalizer root check has succeeded. Keep the real + # Windows locked-parent and canonical probe in the call path. + with monkeypatch.context() as probe_patch: + probe_patch.setattr(Path, operation, fail_root) + return original_probe(path) + + patch.setattr(backend, "_canonical_scan_directory", probe_root) + return original_write(scan_dir, filename, contents) + + monkeypatch.setattr(saved, "write_scan_local_bytes", write_file) + if failure == "io": + assert saved.write_scan_draft( + workbench_api["_WORKBENCH_DB_CONTEXT"], workbench_db, args + ) == {"scanId": scan.scan_id, "status": "draft_written"} + else: + # Python versions before 3.13 report resolve loops as RuntimeError. + expected_errors = ( + (finalizer.ContractError, RuntimeError) + if failure == "loop" and operation == "resolve" + else finalizer.ContractError + ) + with pytest.raises(expected_errors, match="existing scan directory|Symlink loop"): + saved.write_scan_draft(workbench_api["_WORKBENCH_DB_CONTEXT"], workbench_db, args) + assert probes == [operation] + assert published + assert {path: path.read_bytes() for path in published} == published + assert not list((scan.scan_dir / "drafts").glob("*.accepted.json")) + + @pytest.mark.parametrize("failure", ["validation", "unsafe-path", "windows-reparse-emulated"]) def test_receipt_contract_errors_still_reject_publication( workbench_api, workbench_db, publication_scan, monkeypatch, failure From d7a0591e7b0c9b4ec91ad6eeb2c4a7b6538cb160 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Tue, 6 Oct 2026 15:54:31 +0000 Subject: [PATCH 12/23] Preserve partial Deep setup drafts before orchestration --- .../scripts/workbench_saved_results.py | 8 +- .../tests/test_workbench_draft_publication.py | 76 +++++++++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index ac40cafe2c..b848af3a80 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -2389,14 +2389,18 @@ def save_scan_artifact(db: Any, connection: Any, args: Any) -> dict[str, Any]: def _require_current_deep_publication( db: Any, connection: Any, scan_id: str, draft: dict[str, Any] ) -> None: - run = db.deep_scan.require_deep_scan_run(connection, scan_id) + run = connection.execute( + "SELECT * FROM deep_scan_runs WHERE scan_id = ?", (scan_id,) + ).fetchone() publication = draft.get("deepScanPublication") if ( publication is None and draft["manifest"]["scan"].get("complete") is False - and run["status"] == "running" + and (run is None or run["status"] == "running") ): return + if run is None: + run = db.deep_scan.require_deep_scan_run(connection, scan_id) db.deep_scan.require_current_coordinator( run, argparse.Namespace( diff --git a/plugins/codex-security/tests/test_workbench_draft_publication.py b/plugins/codex-security/tests/test_workbench_draft_publication.py index 47a4bae1cd..733d78008e 100644 --- a/plugins/codex-security/tests/test_workbench_draft_publication.py +++ b/plugins/codex-security/tests/test_workbench_draft_publication.py @@ -91,3 +91,79 @@ def test_terminal_deep_draft_compares_previous_bytes_without_parsing_review_docu checkpoint = json.loads((scan_dir / "checkpoints" / head["checkpoint"]).read_text()) assert checkpoint["threatModel"] == model assert (scan_dir / "threatmodel.md").read_text().startswith(model["content"]) + + +@pytest.mark.parametrize("initialized", [False, True]) +@pytest.mark.parametrize("complete", [False, True]) +def test_deep_setup_draft_before_orchestration( + tmp_path: Path, initialized: bool, complete: bool +) -> None: + if initialized: + state_dir, codex_home, target, scan_dir, scan_id = deep_scan_fixture(tmp_path) + else: + state_dir, codex_home = tmp_path / "state", tmp_path / "codex-home" + target, scan_dir = tmp_path / "target", tmp_path / "scan" + target.mkdir() + (target / "app.py").write_text("value = request.args['value']\n") + scan_dir.mkdir(mode=0o700) + registered = run_workbench( + state_dir, + "register-cli-scan", + "--scan-dir", + str(scan_dir), + "--repository", + str(target), + "--recipe-json", + json.dumps( + { + "config": {}, + "mode": "deep", + "repository": str(target), + "target": {"kind": "repository", "paths": []}, + } + ), + ) + scan_id = str(registered["scanId"]) + contract_dir = tmp_path / "contract" + contract_dir.mkdir() + write_completed_contract( + contract_dir, scan_id, target, relative_path="app.py", coverage_mode="deep_repository" + ) + documents = { + key: json.loads((contract_dir / name).read_text()) + for key, name in ( + ("manifest", "scan-manifest.json"), + ("findings", "findings.json"), + ("coverage", "coverage.json"), + ) + } + model = {"format": "markdown", "content": "# Supplied setup model\n"} + documents["manifest"] = { + "scan": { + **{key: documents["manifest"]["scan"][key] for key in ("target", "scope")}, + "threatModel": model, + **({} if complete else {"complete": False}), + } + } + documents["findings"]["findings"] = [] + documents["coverage"].update(completeness="complete", surfaces=[], deferred=[]) + drafts = scan_dir / "drafts" + drafts.mkdir(exist_ok=True) + staged = drafts / f"{uuid.uuid4()}.json" + staged.write_text(json.dumps(documents)) + args = ("write-scan-draft", "--scan-id", scan_id, "--draft-path", str(staged)) + environment = {"CODEX_HOME": str(codex_home)} + if complete and not initialized: + rejected = run_workbench(state_dir, *args, environment=environment, check=False) + assert rejected["returncode"] != 0 + assert "orchestration state not found" in str(rejected["stderr"]) + assert not (scan_dir / "checkpoint-head.json").exists() + return + result = run_workbench(state_dir, *args, environment=environment) + assert result["status"] == "draft_written" + head = json.loads((scan_dir / "checkpoint-head.json").read_text()) + checkpoint = json.loads((scan_dir / "checkpoints" / head["checkpoint"]).read_text()) + assert checkpoint["threatModel"] == model + assert (checkpoint.get("complete") is not False) is complete + assert json.loads((scan_dir / "scan-manifest.json").read_text())["scan"]["threatModel"] == model + assert (scan_dir / "threatmodel.md").read_text().startswith(model["content"]) From 5dceb5e661d80ac574c31c2140ca3f97e9cc2689 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 7 Oct 2026 04:04:01 +0000 Subject: [PATCH 13/23] fix(deep-scan): retain terminal legacy publication during replay --- .../scripts/workbench_saved_results.py | 4 ++ .../test_deep_scan_publication_authority.py | 61 ++++++++++++++++++- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index 76cd6b8e8b..d257938f3f 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -2410,6 +2410,10 @@ def _require_current_deep_publication( # Generation-one runs predate host publication metadata. Keep their existing # draft path; adopted coordinators must carry their generation and selection. if publication is None: + if draft["manifest"]["scan"].get("complete") is False: + raise SystemExit( + "Deep Scan is terminal; incomplete progress cannot replace its publication." + ) return # Match the durable reducer sequence used by coordinator recovery. diff --git a/plugins/codex-security/tests/test_deep_scan_publication_authority.py b/plugins/codex-security/tests/test_deep_scan_publication_authority.py index c8f42e7c78..993edd9118 100644 --- a/plugins/codex-security/tests/test_deep_scan_publication_authority.py +++ b/plugins/codex-security/tests/test_deep_scan_publication_authority.py @@ -6,7 +6,7 @@ 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 @@ -153,3 +153,62 @@ def test_current_publication_replays_without_changing_checkpoint_or_worker_state assert dict(workbench_db.execute("SELECT * FROM deep_scan_workers").fetchone()) == worker_before findings = json.loads((scan.scan_dir / "findings.json").read_text())["findings"] assert findings[0]["title"] == "Accepted aggregate" + + +def test_legacy_terminal_aggregate_survives_a_late_partial_draft_after_write_failure( + workbench_api, workbench_db, publication_scan, monkeypatch +): + scan = publication_scan() + scan.coverage["completeness"] = "partial" + scan.coverage["deferred"] = [ + {"id": "remaining-review", "reason": "The bounded scan retained unfinished review."} + ] + (scan.scan_dir / "coverage.json").write_text(json.dumps(scan.coverage)) + finalizer_globals = workbench_api["_write_prepared_scan_finalization"].__globals__ + write_bytes = finalizer_globals["write_scan_local_bytes"] + + def fail_report(scan_dir, relative_path, payload, **kwargs): + if relative_path == "report.md": + raise finalizer_globals["ContractError"]("Synthetic report write interruption") + return write_bytes(scan_dir, relative_path, payload, **kwargs) + + with monkeypatch.context() as patch: + patch.setitem(finalizer_globals, "write_scan_local_bytes", fail_report) + with pytest.raises(SystemExit, match="Synthetic report write interruption"): + complete(workbench_api, workbench_db, scan) + + run = workbench_db.execute( + "SELECT status, coordinator_generation FROM deep_scan_runs WHERE scan_id = ?", + (scan.scan_id,), + ).fetchone() + assert tuple(run) == ("succeeded", 1) + assert ( + workbench_db.execute( + "SELECT status, seal_manifest_digest FROM scans WHERE id = ?", (scan.scan_id,) + ).fetchone()[0] + == "running" + ) + saved = { + path: path.read_bytes() + for path in scan.scan_dir.rglob("*.json") + if "drafts" not in path.parts + } + late = stage_publication( + scan, generation=None, result_path=None, title="Late incomplete progress", complete=False + ) + with pytest.raises(SystemExit, match="terminal|publication"): + workbench_api["saved_results"].write_scan_draft( + workbench_api["_WORKBENCH_DB_CONTEXT"], workbench_db, late + ) + assert { + path: path.read_bytes() + for path in scan.scan_dir.rglob("*.json") + if "drafts" not in path.parts + } == saved + assert complete(workbench_api, workbench_db, scan)["progress"]["status"] == "complete" + findings = json.loads((scan.scan_dir / "findings.json").read_text())["findings"] + assert [finding["title"] for finding in findings] == [scan.findings[0]["title"]] + assert ( + json.loads((scan.scan_dir / "coverage.json").read_text())["deferred"] + == scan.coverage["deferred"] + ) From 1665efea78b395017a64017ae66342e80c443fce Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 7 Oct 2026 04:15:41 +0000 Subject: [PATCH 14/23] test(deep-scan): retain actual SQLite replay crash boundaries --- .../tests/test_deep_scan_publication_replay.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/plugins/codex-security/tests/test_deep_scan_publication_replay.py b/plugins/codex-security/tests/test_deep_scan_publication_replay.py index ae9da2c01d..4060d1e2a5 100644 --- a/plugins/codex-security/tests/test_deep_scan_publication_replay.py +++ b/plugins/codex-security/tests/test_deep_scan_publication_replay.py @@ -21,15 +21,16 @@ boundary = sys.argv[4] class CrashConnection(sqlite3.Connection): - def commit(self): - completing = self.execute( + def __exit__(self, exc_type, exc_value, traceback): + completing = exc_type is None and self.execute( "SELECT status FROM scans WHERE id = ?", (args.scan_id,) ).fetchone()[0] == "complete" if completing and boundary == "sqlite-before": os._exit(72) - super().commit() + result = super().__exit__(exc_type, exc_value, traceback) if completing and boundary == "sqlite-after": os._exit(73) + return result connection = sqlite3.connect(sys.argv[2], factory=CrashConnection) connection.row_factory = sqlite3.Row From 1cf7c2a3f03b7ef51292315045367e23d2a27acb Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 7 Oct 2026 13:05:29 +0000 Subject: [PATCH 15/23] fix(deep): preserve selected legacy publications during completion replay --- .../mcp-app/tests/clock_workbench.py | 1 + .../test_deep_scan_publication_integration.ts | 103 ++++++++++++++++++ .../scripts/workbench_saved_results.py | 18 +-- .../test_deep_scan_publication_authority.py | 3 +- 4 files changed, 116 insertions(+), 9 deletions(-) diff --git a/plugins/codex-security/mcp-app/tests/clock_workbench.py b/plugins/codex-security/mcp-app/tests/clock_workbench.py index 22d2c52b62..54c6e44758 100644 --- a/plugins/codex-security/mcp-app/tests/clock_workbench.py +++ b/plugins/codex-security/mcp-app/tests/clock_workbench.py @@ -11,4 +11,5 @@ api = runpy.run_path(str(source), run_name="test_workbench") instant = os.environ["TEST_WORKBENCH_NOW"] api["main"].__globals__["now"] = lambda: instant +api["_WORKBENCH_DB_CONTEXT"].now = lambda: instant api["main"]() diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts index 982b9de345..cf2265c7b3 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts @@ -328,6 +328,109 @@ for (const [label, offsets, ids, paths, recoveryOnly] of scenarios) { }); } +test("legacy discovery completion permits the first parent draft", async (t) => { + const fixture = await createFixture(t); + const { run, store, call } = fixture; + await commitReducers(fixture, [1, 2], [highId, lowId]); + const discovery = path.join(run.scanDir, "artifacts", "02_discovery"); + await mkdir(discovery, { recursive: true }); + await writeFile(path.join(discovery, "in_scope_files.txt"), "fixture.py\n"); + await writeFile(path.join(discovery, "candidate_ledger.jsonl"), ""); + const manifestPath = path.join(run.scanDir, "coordinator-manifest.json"); + await writeFile(manifestPath, "{}\n"); + const terminal = await store.finish({ + scanId: run.scanId, + reason: "saturated", + manifestPath, + omittedWorkerIds: [], + }); + assert.equal(terminal.status, "succeeded"); + assert.equal(terminal.coordinatorGeneration, 1); + assert.equal(terminal.manifestPath, manifestPath); + assertSuccess( + await call("record_codex_security_scan_draft", { + scanId: run.scanId, + complete: true, + findings: [], + coverage, + }), + ); + assertSuccess( + await call("complete_codex_security_scan", { scanId: run.scanId }), + ); +}); + +for (const complete of [false, true]) { + test(`legacy terminal aggregate rejects late public drafts with complete=${complete}`, async (t) => { + const fixture = await createFixture(t); + const { run, store, call, runWorkbench } = fixture; + await commitReducers(fixture, [1, 2], [highId, lowId]); + const context = await createScanArtifactContext(run.scanId, runWorkbench, { + requireRunning: true, + }); + const draft: ScanDraftInput = { + scanId: run.scanId, + complete: true, + findings: [], + coverage, + threatModel: { summary: "Selected aggregate" }, + }; + await assert.rejects( + recordCodexSecurityScanDraftViaWorkbench(context, draft, async (args) => { + await runWorkbench(args); + throw new Error("Synthetic lost publication response"); + }), + /lost publication response/, + ); + const published = await snapshot(run); + await recordCodexSecurityScanDraftViaWorkbench( + context, + draft, + runWorkbench, + ); + assert.deepEqual(await snapshot(run), published); + const terminal = await store.finish({ + scanId: run.scanId, + reason: "saturated", + manifestPath: path.join(run.scanDir, "scan-manifest.json"), + omittedWorkerIds: [], + }); + assert.equal(terminal.status, "succeeded"); + assert.equal(terminal.coordinatorGeneration, 1); + const report = path.join(run.scanDir, "report.html"); + await mkdir(report); + assertToolError( + await call("complete_codex_security_scan", { scanId: run.scanId }), + /report\.html/, + ); + await rm(report, { recursive: true }); + const interrupted = await snapshot(run); + assertToolError( + await call("record_codex_security_scan_draft", { + ...partial(run, "late-progress"), + complete, + threatModel: { summary: "Late replacement" }, + }), + /terminal|publication/, + ); + assert.deepEqual(await snapshot(run), interrupted); + assertSuccess( + await call("complete_codex_security_scan", { scanId: run.scanId }), + ); + const completed = await snapshot(run); + assert.deepEqual( + JSON.parse(completed.files["coverage.json"]), + JSON.parse(interrupted.files["coverage.json"]), + ); + assert.equal( + JSON.parse(completed.files["scan-manifest.json"]).scan.threatModel + .summary, + "Selected aggregate", + ); + assert.deepEqual(completed.checkpoints, interrupted.checkpoints); + }); +} + test("partial drafts cannot replace a terminal aggregate awaiting completion replay", async (t) => { const fixture = await createFixture(t); const { run, store, call, runWorkbench, instant } = fixture; diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index d257938f3f..10a7ebe22d 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -2387,8 +2387,9 @@ def save_scan_artifact(db: Any, connection: Any, args: Any) -> dict[str, Any]: def _require_current_deep_publication( - db: Any, connection: Any, scan_id: str, draft: dict[str, Any] + db: Any, connection: Any, scan: Any, draft: dict[str, Any] ) -> None: + scan_id = scan["id"] run = connection.execute( "SELECT * FROM deep_scan_runs WHERE scan_id = ?", (scan_id,) ).fetchone() @@ -2407,13 +2408,14 @@ def _require_current_deep_publication( coordinator_generation=publication.get("coordinatorGeneration") if publication else None ), ) - # Generation-one runs predate host publication metadata. Keep their existing - # draft path; adopted coordinators must carry their generation and selection. + # Legacy discovery-only runs still need a parent draft. Once finish selects + # the canonical parent, only completion may replay it. if publication is None: - if draft["manifest"]["scan"].get("complete") is False: - raise SystemExit( - "Deep Scan is terminal; incomplete progress cannot replace its publication." - ) + if draft["manifest"]["scan"].get("complete") is False or ( + run["status"] != "running" + and run["manifest_path"] == str(Path(scan["scan_dir"]) / "scan-manifest.json") + ): + raise SystemExit("Deep Scan is terminal; drafts cannot replace its publication.") return # Match the durable reducer sequence used by coordinator recovery. @@ -2468,7 +2470,7 @@ def write_scan_draft(db: Any, connection: Any, args: Any) -> dict[str, Any]: raise SystemExit("Scan draft must be inside the registered scan drafts directory.") draft = _read_scan_local_json(scan_dir, relative, "Staged scan draft") if scan["mode"] == "deep": - _require_current_deep_publication(db, connection, scan_id, draft) + _require_current_deep_publication(db, connection, scan, draft) manifest, findings, coverage = draft["manifest"], draft["findings"], draft["coverage"] binding = db.workbench_completion_binding(scan, db.now()) # Save scan IDs without sealing the draft. diff --git a/plugins/codex-security/tests/test_deep_scan_publication_authority.py b/plugins/codex-security/tests/test_deep_scan_publication_authority.py index 993edd9118..e30b9cc24a 100644 --- a/plugins/codex-security/tests/test_deep_scan_publication_authority.py +++ b/plugins/codex-security/tests/test_deep_scan_publication_authority.py @@ -116,7 +116,8 @@ def test_current_publication_replays_without_changing_checkpoint_or_worker_state result = add_worker(workbench_db, scan) with workbench_db: workbench_db.execute( - "UPDATE deep_scan_runs SET coordinator_generation = ? WHERE scan_id = ?", + "UPDATE deep_scan_runs SET coordinator_generation = ?, status = 'running' " + "WHERE scan_id = ?", (generation or 1, scan.scan_id), ) workbench_db.execute( From cf3f24dd36e146a9d6f11f5146379cb51abef558 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 7 Oct 2026 14:00:26 +0000 Subject: [PATCH 16/23] fix(deep): preserve legacy progress and selected-result replay --- .../test_deep_scan_publication_integration.ts | 105 +++++++++++++----- .../codex-security/scripts/workbench_db.py | 9 +- .../scripts/workbench_saved_results.py | 5 +- 3 files changed, 84 insertions(+), 35 deletions(-) diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts index cf2265c7b3..a650fdecd0 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts @@ -328,37 +328,82 @@ for (const [label, offsets, ids, paths, recoveryOnly] of scenarios) { }); } -test("legacy discovery completion permits the first parent draft", async (t) => { - const fixture = await createFixture(t); - const { run, store, call } = fixture; - await commitReducers(fixture, [1, 2], [highId, lowId]); - const discovery = path.join(run.scanDir, "artifacts", "02_discovery"); - await mkdir(discovery, { recursive: true }); - await writeFile(path.join(discovery, "in_scope_files.txt"), "fixture.py\n"); - await writeFile(path.join(discovery, "candidate_ledger.jsonl"), ""); - const manifestPath = path.join(run.scanDir, "coordinator-manifest.json"); - await writeFile(manifestPath, "{}\n"); - const terminal = await store.finish({ - scanId: run.scanId, - reason: "saturated", - manifestPath, - omittedWorkerIds: [], - }); - assert.equal(terminal.status, "succeeded"); - assert.equal(terminal.coordinatorGeneration, 1); - assert.equal(terminal.manifestPath, manifestPath); - assertSuccess( - await call("record_codex_security_scan_draft", { +for (const outputFailure of [false, true]) { + test(`legacy discovery parent ${outputFailure ? "retains failure after an output write error" : "retains progress before first completion"}`, async (t) => { + const fixture = await createFixture(t); + const { run, store, call, runWorkbench } = fixture; + await commitReducers(fixture, [1, 2], [highId, lowId]); + const discovery = path.join(run.scanDir, "artifacts", "02_discovery"); + await mkdir(discovery, { recursive: true }); + await writeFile(path.join(discovery, "in_scope_files.txt"), "fixture.py\n"); + await writeFile(path.join(discovery, "candidate_ledger.jsonl"), ""); + const manifestPath = path.join(run.scanDir, "coordinator-manifest.json"); + await writeFile(manifestPath, "{}\n"); + const terminal = await store.finish({ scanId: run.scanId, - complete: true, - findings: [], - coverage, - }), - ); - assertSuccess( - await call("complete_codex_security_scan", { scanId: run.scanId }), - ); -}); + reason: "saturated", + manifestPath, + omittedWorkerIds: [], + }); + assert.equal(terminal.status, "succeeded"); + assert.equal(terminal.coordinatorGeneration, 1); + assert.equal(terminal.manifestPath, manifestPath); + if (!outputFailure) { + assertSuccess( + await call( + "record_codex_security_scan_draft", + partial(run, "legacy-progress"), + ), + ); + const progress = await checkpoints(run); + assert.ok(Object.keys(progress).length > 0); + assertToolError( + await call("complete_codex_security_scan", { scanId: run.scanId }), + /incomplete/, + ); + assert.deepEqual(await checkpoints(run), progress); + } + assertSuccess( + await call("record_codex_security_scan_draft", { + scanId: run.scanId, + complete: true, + findings: [], + coverage, + }), + ); + if (outputFailure) { + const report = path.join(run.scanDir, "report.html"); + await mkdir(report); + assertToolError( + await call("complete_codex_security_scan", { scanId: run.scanId }), + /report\.html/, + ); + await rm(report, { recursive: true }); + const context = await runWorkbench(["get-scan", "--scan-id", run.scanId]); + assert.equal( + (context.scan as { progress: { status: string } }).progress.status, + "failed", + ); + const interrupted = await snapshot(run); + assertToolError( + await call("record_codex_security_scan_draft", { + ...partial(run, "late-legacy-draft"), + complete: true, + threatModel: { summary: "Late replacement" }, + }), + ); + assert.deepEqual(await snapshot(run), interrupted); + assertToolError( + await call("complete_codex_security_scan", { scanId: run.scanId }), + /running/, + ); + } else { + assertSuccess( + await call("complete_codex_security_scan", { scanId: run.scanId }), + ); + } + }); +} for (const complete of [false, true]) { test(`legacy terminal aggregate rejects late public drafts with complete=${complete}`, async (t) => { diff --git a/plugins/codex-security/scripts/workbench_db.py b/plugins/codex-security/scripts/workbench_db.py index 9d1f741eb4..146361c7a2 100644 --- a/plugins/codex-security/scripts/workbench_db.py +++ b/plugins/codex-security/scripts/workbench_db.py @@ -1513,8 +1513,13 @@ def add_warning() -> None: prepared, projection_warnings=warnings ) except ContractError as exc: - # Replay a validated Deep aggregate after an output write fails. - if (wrote and scan["mode"] != "deep") or ( + # Only a coordinator-selected parent is fenced for output-write replay. + selected_deep_parent = ( + scan["mode"] == "deep" + and deep_scan.require_deep_scan_run(connection, scan_id)["manifest_path"] + == str(scan_dir / ARTIFACTS["manifest"]) + ) + if (wrote and not selected_deep_parent) or ( scan["mode"] == "deep" and not wrote and not already_sealed diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index 10a7ebe22d..cfb2ba4cd1 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -2411,9 +2411,8 @@ def _require_current_deep_publication( # Legacy discovery-only runs still need a parent draft. Once finish selects # the canonical parent, only completion may replay it. if publication is None: - if draft["manifest"]["scan"].get("complete") is False or ( - run["status"] != "running" - and run["manifest_path"] == str(Path(scan["scan_dir"]) / "scan-manifest.json") + if run["status"] != "running" and run["manifest_path"] == str( + Path(scan["scan_dir"]) / "scan-manifest.json" ): raise SystemExit("Deep Scan is terminal; drafts cannot replace its publication.") return From 0118c1a9bf21edcbc10db5b71ed1df2a3e520404 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 7 Oct 2026 14:09:56 +0000 Subject: [PATCH 17/23] style(deep): format selected publication check --- plugins/codex-security/scripts/workbench_db.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/plugins/codex-security/scripts/workbench_db.py b/plugins/codex-security/scripts/workbench_db.py index 146361c7a2..485f3d6fb7 100644 --- a/plugins/codex-security/scripts/workbench_db.py +++ b/plugins/codex-security/scripts/workbench_db.py @@ -1514,11 +1514,9 @@ def add_warning() -> None: ) except ContractError as exc: # Only a coordinator-selected parent is fenced for output-write replay. - selected_deep_parent = ( - scan["mode"] == "deep" - and deep_scan.require_deep_scan_run(connection, scan_id)["manifest_path"] - == str(scan_dir / ARTIFACTS["manifest"]) - ) + selected_deep_parent = scan["mode"] == "deep" and deep_scan.require_deep_scan_run( + connection, scan_id + )["manifest_path"] == str(scan_dir / ARTIFACTS["manifest"]) if (wrote and not selected_deep_parent) or ( scan["mode"] == "deep" and not wrote From e21526429ef359ac73133a0ac3d6ac508f1b3942 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Wed, 7 Oct 2026 16:35:53 +0000 Subject: [PATCH 18/23] fix(deep): retain parent publication after legacy lease claims --- .../test_deep_scan_publication_integration.ts | 19 ++++++++++++++++--- .../scripts/workbench_saved_results.py | 10 ++++++++-- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts index a650fdecd0..01e85ac730 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts @@ -328,10 +328,23 @@ for (const [label, offsets, ids, paths, recoveryOnly] of scenarios) { }); } -for (const outputFailure of [false, true]) { - test(`legacy discovery parent ${outputFailure ? "retains failure after an output write error" : "retains progress before first completion"}`, async (t) => { +for (const [claimed, outputFailure] of [ + [false, false], + [false, true], + [true, false], + [true, true], +]) { + test(`legacy discovery parent with claimed=${claimed} ${outputFailure ? "retains failure after an output write error" : "retains progress before first completion"}`, async (t) => { const fixture = await createFixture(t); const { run, store, call, runWorkbench } = fixture; + if (claimed) { + const claim = await store.claimCoordinator({ + scanId: run.scanId, + threadId: owner, + }); + assert.equal(claim.acquired, true); + assert.equal(claim.run.coordinatorGeneration, 2); + } await commitReducers(fixture, [1, 2], [highId, lowId]); const discovery = path.join(run.scanDir, "artifacts", "02_discovery"); await mkdir(discovery, { recursive: true }); @@ -346,7 +359,7 @@ for (const outputFailure of [false, true]) { omittedWorkerIds: [], }); assert.equal(terminal.status, "succeeded"); - assert.equal(terminal.coordinatorGeneration, 1); + assert.equal(terminal.coordinatorGeneration, claimed ? 2 : 1); assert.equal(terminal.manifestPath, manifestPath); if (!outputFailure) { assertSuccess( diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index cfb2ba4cd1..28a1aabfdf 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -2402,14 +2402,20 @@ def _require_current_deep_publication( return if run is None: run = db.deep_scan.require_deep_scan_run(connection, scan_id) + # Finished discovery-only runs hand remaining publication back to the parent. + if ( + publication is None + and run["status"] != "running" + and run["manifest_path"] != str(Path(scan["scan_dir"]) / "scan-manifest.json") + ): + return db.deep_scan.require_current_coordinator( run, argparse.Namespace( coordinator_generation=publication.get("coordinatorGeneration") if publication else None ), ) - # Legacy discovery-only runs still need a parent draft. Once finish selects - # the canonical parent, only completion may replay it. + # Once finish selects the canonical parent, only completion may replay it. if publication is None: if run["status"] != "running" and run["manifest_path"] == str( Path(scan["scan_dir"]) / "scan-manifest.json" From 21eb50f868f56f7efd0312d0180b5a4452e5390d Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 8 Oct 2026 01:55:08 +0000 Subject: [PATCH 19/23] fix(deep): align publication with recovered reducer selection --- .../test_deep_scan_publication_integration.ts | 37 +++++++++++++++++-- .../scripts/workbench_saved_results.py | 2 +- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts index 01e85ac730..b6a90cb862 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts @@ -55,6 +55,7 @@ type Scenario = readonly [ ids: readonly [string, string], paths?: FixturePaths, recoveryOnly?: boolean, + reducerLabels?: readonly [string, string], ]; type PublicationFixture = Awaited>; @@ -178,6 +179,22 @@ const scenarios: Scenario[] = [ ["equal timestamps and descending UUIDs", [1, 1], [highId, lowId]], ["decreasing timestamps", [2, 1], [lowId, highId]], ["equal timestamps and ascending UUIDs", [1, 1], [lowId, highId]], + [ + "legacy reducer label", + [1, 1], + [highId, lowId], + undefined, + false, + ["dedup-0002-legacy", "dedup-0001"], + ], + [ + "large reducer sequence", + [1, 1], + [highId, lowId], + undefined, + false, + ["dedup-9007199254740992", "dedup-9007199254740993"], + ], [ "scan root collision, live publication", [1, 1], @@ -205,7 +222,14 @@ const scenarios: Scenario[] = [ true, ], ]; -for (const [label, offsets, ids, paths, recoveryOnly] of scenarios) { +for (const [ + label, + offsets, + ids, + paths, + recoveryOnly, + reducerLabels, +] of scenarios) { test(`selected reducer survives recovery and public completion: ${label}`, async (t) => { const fixture = await createFixture(t, paths); const { run, store, call, runWorkbench, instant } = fixture; @@ -223,7 +247,7 @@ for (const [label, offsets, ids, paths, recoveryOnly] of scenarios) { }); assert.equal(claimed.run.coordinatorGeneration, 2); assert.equal(claimed.run.config.stopAfterNoNew, 4); - const results = await commitReducers(fixture, offsets, ids); + const results = await commitReducers(fixture, offsets, ids, reducerLabels); const persisted = await store.get(run.scanId, owner); assert.equal(persisted.noNewStreak, 4); assert.ok(persisted.persistedWorkers); @@ -298,7 +322,10 @@ for (const [label, offsets, ids, paths, recoveryOnly] of scenarios) { ); const sealed = await snapshot(run); const manifest = JSON.parse(sealed.files["scan-manifest.json"]); - assert.equal(manifest.scan.threatModel.summary, "dedup-0002"); + assert.equal( + manifest.scan.threatModel.summary, + reducerLabels?.[1] ?? "dedup-0002", + ); assert.ok(manifest.scan.sealedAt); assert.deepEqual(JSON.parse(sealed.files["findings.json"]).findings, []); assert.deepEqual(JSON.parse(sealed.files["coverage.json"]).deferred, []); @@ -642,6 +669,7 @@ async function commitReducers( fixture: PublicationFixture, offsets: readonly [number, number], ids: readonly [string, string], + reducerLabels?: readonly [string, string], ) { const { run, store } = fixture; const results = []; @@ -672,7 +700,8 @@ async function commitReducers( }); workerIds.push(id); } - const label = `dedup-${String(batch + 1).padStart(4, "0")}`; + const label = + reducerLabels?.[batch] ?? `dedup-${String(batch + 1).padStart(4, "0")}`; const artifact = await workerArtifact(run, "dedup", label); const id = ids[batch]; await store.claimDedup({ id, scanId: run.scanId, workerIds, ...artifact }); diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index 28a1aabfdf..6c02c1f950 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -2425,7 +2425,7 @@ def _require_current_deep_publication( # Match the durable reducer sequence used by coordinator recovery. def reducer_order(worker: Any) -> tuple[int, str]: - match = re.search(r"dedup-(\d+)", Path(worker["prompt_path"]).parent.name) + match = re.fullmatch(r"dedup-([0-9]+)", Path(worker["prompt_path"]).parent.name) return (int(match[1]) if match else 0, worker["id"]) reducer = max( From 39f06e6967612c048ea6f0f0a736b11f98df74a8 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 8 Oct 2026 03:34:43 +0000 Subject: [PATCH 20/23] test(workbench): track the current migration history --- .../mcp-app/tests/test_workbench_database.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/codex-security/mcp-app/tests/test_workbench_database.ts b/plugins/codex-security/mcp-app/tests/test_workbench_database.ts index 6f7b777184..c76c3f19c8 100644 --- a/plugins/codex-security/mcp-app/tests/test_workbench_database.ts +++ b/plugins/codex-security/mcp-app/tests/test_workbench_database.ts @@ -99,7 +99,7 @@ test("opens a private WAL database at the configured state path", async () => { assert.equal( database.prepare("SELECT COUNT(*) AS count FROM schema_migrations").get() ?.count, - 42, + migrations.length, ); assert.deepEqual(database.prepare("PRAGMA foreign_key_check").all(), []); if (process.platform !== "win32") { @@ -129,7 +129,7 @@ test("every released schema upgrades to the same current schema and remains idem const current = memory(t); applyMigrations(current); const expected = schema(current); - for (let version = 0; version <= 42; version++) { + for (const version of [0, ...migrations.map(({ version }) => version)]) { const database = memory(t, version); applyMigrations(database); assert.deepEqual( @@ -243,7 +243,7 @@ test( database .prepare("SELECT MAX(version) AS version FROM schema_migrations") .get()?.version, - 42, + migrations.at(-1)!.version, ); } finally { database.close(); @@ -721,7 +721,7 @@ test("retries an upgrade when another process holds the write lock beyond the bu database .prepare("SELECT COUNT(*) AS count FROM schema_migrations") .get()?.count, - 42, + migrations.length, ); assert.equal( database.prepare("SELECT COUNT(*) AS count FROM security_targets").get() From bc7a120b60a61c71975265c01b72993be03e3881 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 8 Oct 2026 06:06:27 +0000 Subject: [PATCH 21/23] fix(deep): retain progress after publication selection --- .../test_deep_scan_publication_integration.ts | 225 +++++++++++++----- .../scripts/deep_scan_workbench.py | 2 + .../scripts/workbench_saved_results.py | 30 ++- .../tests/test_workbench_draft_publication.py | 25 ++ 4 files changed, 215 insertions(+), 67 deletions(-) diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts index b6a90cb862..d2804ddb9e 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts @@ -300,6 +300,7 @@ for (const [ signal, publication, ); + await assertInterleavedProgressRetained(fixture); }, }); coordinator.start(); @@ -474,6 +475,7 @@ for (const complete of [false, true]) { runWorkbench, ); assert.deepEqual(await snapshot(run), published); + await assertInterleavedProgressRetained(fixture); const terminal = await store.finish({ scanId: run.scanId, reason: "saturated", @@ -516,79 +518,170 @@ for (const complete of [false, true]) { }); } -test("partial drafts cannot replace a terminal aggregate awaiting completion replay", async (t) => { - const fixture = await createFixture(t); - const { run, store, call, runWorkbench, instant } = fixture; - const claimed = await store.claimCoordinator({ - scanId: run.scanId, - threadId: owner, - }); - const context = await createScanArtifactContext(run.scanId, runWorkbench, { - requireRunning: true, - }); - const elapsedSeconds = ((run.config.maxTimeHours ?? 1) + 1) * 3_600; - fixture.setTime(elapsedSeconds); - const deadline = Date.parse(instant) + elapsedSeconds * 1_000; - const coordinator = new DeepScanCoordinator({ - run: claimed.run, - store, - pluginRoot, - executor: { - run() { - throw new Error("Expired coordinator must not start a worker."); +for (const claimed of [false, true]) { + test(`partial drafts retain a capped publication with claimed=${claimed}`, async (t) => { + const fixture = await createFixture(t); + const { run, store, call, runWorkbench, instant } = fixture; + const selectedRun = claimed + ? (await store.claimCoordinator({ scanId: run.scanId, threadId: owner })) + .run + : run; + const context = await createScanArtifactContext(run.scanId, runWorkbench, { + requireRunning: true, + }); + const elapsedSeconds = ((run.config.maxTimeHours ?? 1) + 1) * 3_600; + fixture.setTime(elapsedSeconds); + const deadline = Date.parse(instant) + elapsedSeconds * 1_000; + const coordinator = new DeepScanCoordinator({ + run: selectedRun, + store, + pluginRoot, + executor: { + run() { + throw new Error("Expired coordinator must not start a worker."); + }, }, - }, - clock: { now: () => deadline, sleep: async () => {} }, - onComplete: (draft, signal, publication) => + clock: { now: () => deadline, sleep: async () => {} }, + onComplete: async (draft, signal, publication) => { + await recordCodexSecurityScanDraftViaWorkbench( + context, + draft, + runWorkbench, + signal, + claimed ? publication : undefined, + ); + await assertInterleavedProgressRetained(fixture); + }, + }); + coordinator.start(); + const terminal = await coordinator.wait(undefined, 30_000); + assert.equal( + terminal?.status, + "succeeded", + terminal?.error ?? "Coordinator did not succeed", + ); + assert.equal(terminal?.terminalReason, "capped"); + + const report = path.join(run.scanDir, "report.html"); + await mkdir(report); + assertToolError( + await call("complete_codex_security_scan", { scanId: run.scanId }), + /report\.html/, + ); + await rm(report, { recursive: true }); + const interrupted = await snapshot(run); + assert.equal( + JSON.parse(interrupted.files["coverage.json"]).completeness, + "partial", + ); + assertToolError( + await call( + "record_codex_security_scan_draft", + partial(run, "late-progress"), + ), + /coordinator lease|terminal/, + ); + assert.deepEqual(await snapshot(run), interrupted); + assertSuccess( + await call("complete_codex_security_scan", { scanId: run.scanId }), + ); + const completed = await snapshot(run); + assert.ok(JSON.parse(completed.files["scan-manifest.json"]).scan.sealedAt); + assert.deepEqual( + JSON.parse(completed.files["coverage.json"]), + JSON.parse(interrupted.files["coverage.json"]), + ); + assert.deepEqual(completed.checkpoints, interrupted.checkpoints); + assert.equal((await store.get(run.scanId, owner)).terminalReason, "capped"); + }); +} + +for (const status of ["failed", "interrupted"] as const) { + test(`selected publication retains ${status} and exact failure replay`, async (t) => { + const { run, store, runWorkbench } = await createFixture(t); + await store.claimCoordinator({ scanId: run.scanId, threadId: owner }); + const context = await createScanArtifactContext(run.scanId, runWorkbench, { + requireRunning: true, + }); + const draft: ScanDraftInput = { + scanId: run.scanId, + findings: [], + coverage, + }; + const publication = { coordinatorGeneration: 2, resultPath: null }; + await assert.rejects( recordCodexSecurityScanDraftViaWorkbench( context, draft, - runWorkbench, - signal, + async (args) => { + await runWorkbench(args); + throw new Error("Synthetic lost publication response"); + }, + undefined, publication, - ).then(() => {}), + ), + /lost publication response/, + ); + await recordCodexSecurityScanDraftViaWorkbench( + context, + draft, + runWorkbench, + undefined, + publication, + ); + const message = "Synthetic failure after publication"; + const failed = await store.fail(run.scanId, message, status); + assert.equal(failed.status, status); + assert.equal( + failed.manifestPath, + path.join(run.scanDir, "scan-manifest.json"), + ); + assert.deepEqual(await store.fail(run.scanId, message, status), failed); + await assert.rejects( + store.fail(run.scanId, message + " changed", status), + /immutable/, + ); + const differentManifest = path.join(run.scanDir, "different-manifest.json"); + await writeFile(differentManifest, "{}\n"); + await assert.rejects( + store.fail(run.scanId, message, status, differentManifest), + /immutable/, + ); }); - coordinator.start(); - const terminal = await coordinator.wait(undefined, 30_000); - assert.equal( - terminal?.status, - "succeeded", - terminal?.error ?? "Coordinator did not succeed", - ); - assert.equal(terminal?.terminalReason, "capped"); +} - const report = path.join(run.scanDir, "report.html"); - await mkdir(report); - assertToolError( - await call("complete_codex_security_scan", { scanId: run.scanId }), - /report\.html/, - ); - await rm(report, { recursive: true }); - const interrupted = await snapshot(run); - assert.equal( - JSON.parse(interrupted.files["coverage.json"]).completeness, - "partial", - ); - assertToolError( - await call( - "record_codex_security_scan_draft", - partial(run, "late-progress"), - ), - /current coordinator lease/, - ); - assert.deepEqual(await snapshot(run), interrupted); - assertSuccess( - await call("complete_codex_security_scan", { scanId: run.scanId }), - ); - const completed = await snapshot(run); - assert.ok(JSON.parse(completed.files["scan-manifest.json"]).scan.sealedAt); - assert.deepEqual( - JSON.parse(completed.files["coverage.json"]), - JSON.parse(interrupted.files["coverage.json"]), +async function assertInterleavedProgressRetained(fixture: PublicationFixture) { + const { run, call } = fixture; + const before = await snapshot(run); + const progress = { + scanId: run.scanId, + complete: false, + findings: [], + coverage: { + ...coverage, + completeness: "partial", + deferred: [ + { + id: "interleaved-progress", + candidateId: "interleaved-progress", + reason: "Synthetic review remains pending.", + paths: ["fixture.py"], + }, + ], + }, + threatModel: { summary: "Unselected progress model" }, + }; + assertSuccess(await call("record_codex_security_scan_draft", progress)); + const after = await snapshot(run); + assert.deepEqual(after.files, before.files); + for (const [name, contents] of Object.entries(before.checkpoints)) + assert.equal(after.checkpoints[name], contents); + const added = Object.keys(after.checkpoints).filter( + (name) => !(name in before.checkpoints), ); - assert.deepEqual(completed.checkpoints, interrupted.checkpoints); - assert.equal((await store.get(run.scanId, owner)).terminalReason, "capped"); -}); + assert.equal(added.length, 1); + assert.deepEqual(JSON.parse(after.checkpoints[added[0]]), progress); +} async function createFixture(t: TestContext, paths: FixturePaths = {}) { const root = await mkdtemp(path.join(tmpdir(), "deep-publication-")); @@ -788,6 +881,8 @@ async function snapshot(run: DeepScanRunState) { "findings.json", "coverage.json", "report.md", + "checkpoint-head.json", + "threatmodel.md", ]) { try { files[name] = await readFile(path.join(run.scanDir, name), "utf8"); diff --git a/plugins/codex-security/scripts/deep_scan_workbench.py b/plugins/codex-security/scripts/deep_scan_workbench.py index d1d0b2dca0..1942eb32ae 100644 --- a/plugins/codex-security/scripts/deep_scan_workbench.py +++ b/plugins/codex-security/scripts/deep_scan_workbench.py @@ -1865,6 +1865,8 @@ def fail_deep_scan_locked( kind="file", ) ) + elif run["manifest_path"] == str(Path(scan["scan_dir"]) / "scan-manifest.json"): + manifest_path = run["manifest_path"] if run["status"] in {"failed", "interrupted"} or scan["status"] == "failed": if ( run["status"] == args.deep_status diff --git a/plugins/codex-security/scripts/workbench_saved_results.py b/plugins/codex-security/scripts/workbench_saved_results.py index 6c02c1f950..cfd1722bd5 100644 --- a/plugins/codex-security/scripts/workbench_saved_results.py +++ b/plugins/codex-security/scripts/workbench_saved_results.py @@ -2474,14 +2474,14 @@ def write_scan_draft(db: Any, connection: Any, args: Any) -> dict[str, Any]: if not re.fullmatch(r"drafts/[0-9a-fA-F-]+\.json", relative): raise SystemExit("Scan draft must be inside the registered scan drafts directory.") draft = _read_scan_local_json(scan_dir, relative, "Staged scan draft") - if scan["mode"] == "deep": - _require_current_deep_publication(db, connection, scan, draft) manifest, findings, coverage = draft["manifest"], draft["findings"], draft["coverage"] binding = db.workbench_completion_binding(scan, db.now()) # Save scan IDs without sealing the draft. _populate_unsealed_manifest_envelope(manifest, manifest["scan"], binding) _populate_unsealed_artifact_envelope(manifest, findings, coverage, binding) _validate_completion_binding(manifest, findings, coverage, binding) + checkpoint = manifest["scan"] + checkpoint_contents = None if args.checkpoint_path is not None: try: checkpoint_relative = Path(args.checkpoint_path).relative_to(scan_dir).as_posix() @@ -2498,18 +2498,36 @@ def write_scan_draft(db: Any, connection: Any, args: Any) -> dict[str, Any]: ) if checkpoint.get("scanId") != scan_id: raise SystemExit("Staged scan checkpoint belongs to another scan.") + checkpoint_only = ( + scan["mode"] == "deep" + and draft.get("deepScanPublication") is None + and checkpoint.get("complete") is False + and connection.execute( + "SELECT 1 FROM deep_scan_runs WHERE scan_id = ? " + "AND status = 'running' AND manifest_path = ?", + (scan_id, str(scan_dir / "scan-manifest.json")), + ).fetchone() + is not None + ) + if scan["mode"] == "deep" and not checkpoint_only: + _require_current_deep_publication(db, connection, scan, draft) + if checkpoint_contents is not None: checkpoint_digest = hashlib.sha256(checkpoint_contents).hexdigest() write_scan_local_bytes( scan_dir, f"checkpoints/{checkpoint_digest}.json", checkpoint_contents, ) + if checkpoint_only: + return {"scanId": scan_id, "status": "draft_written"} checkpoint = _parent_scan_draft(scan_id, manifest["scan"], findings, coverage) checkpoint_contents = _encoded(checkpoint) checkpoint_name = f"{hashlib.sha256(checkpoint_contents).hexdigest()}.json" checkpoint_relative = f"checkpoints/{checkpoint_name}" if not (scan_dir / checkpoint_relative).exists(): write_scan_local_bytes(scan_dir, checkpoint_relative, checkpoint_contents) + if checkpoint_only: + return {"scanId": scan_id, "status": "draft_written"} write_scan_local_bytes( scan_dir, "checkpoint-head.json", _encoded({"checkpoint": checkpoint_name}) ) @@ -2523,6 +2541,14 @@ def write_scan_draft(db: Any, connection: Any, args: Any) -> dict[str, Any]: filename, (json.dumps(document, allow_nan=False, indent=2) + "\n").encode(), ) + if scan["mode"] == "deep" and manifest["scan"].get("complete") is not False: + # Select the accepted final publication before releasing the write lock. + with connection: + connection.execute( + "UPDATE deep_scan_runs SET manifest_path = ? " + "WHERE scan_id = ? AND status = 'running'", + (str(scan_dir / "scan-manifest.json"), scan_id), + ) model_warning = write_threat_model_projection_if_possible(scan_dir, manifest) # Accepted Standard drafts are evidence of review or report assembly, # even when the parent omitted its explicit progress call. diff --git a/plugins/codex-security/tests/test_workbench_draft_publication.py b/plugins/codex-security/tests/test_workbench_draft_publication.py index 733d78008e..0b731d380e 100644 --- a/plugins/codex-security/tests/test_workbench_draft_publication.py +++ b/plugins/codex-security/tests/test_workbench_draft_publication.py @@ -167,3 +167,28 @@ def test_deep_setup_draft_before_orchestration( assert (checkpoint.get("complete") is not False) is complete assert json.loads((scan_dir / "scan-manifest.json").read_text())["scan"]["threatModel"] == model assert (scan_dir / "threatmodel.md").read_text().startswith(model["content"]) + if initialized and complete: + selected = { + name: (scan_dir / name).read_bytes() + for name in ( + "scan-manifest.json", + "findings.json", + "coverage.json", + "checkpoint-head.json", + "threatmodel.md", + ) + } + checkpoints = set((scan_dir / "checkpoints").iterdir()) + documents["manifest"]["scan"].update( + complete=False, threatModel={"summary": "Unselected progress"} + ) + staged.write_text(json.dumps(documents)) + assert run_workbench(state_dir, *args, environment=environment)["status"] == "draft_written" + assert all( + (scan_dir / name).read_bytes() == contents for name, contents in selected.items() + ) + added = set((scan_dir / "checkpoints").iterdir()) - checkpoints + assert len(added) == 1 + retained = json.loads(added.pop().read_text()) + assert retained["complete"] is False + assert retained["threatModel"] == {"summary": "Unselected progress"} From 812c72acc2a77e12f3a0cb2d762b51b94c35d965 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Sat, 10 Oct 2026 07:37:12 +0000 Subject: [PATCH 22/23] style(deep-scan): format accepted publication comparison --- plugins/codex-security/mcp-app/src/artifact-scan-draft.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 0eef70d762..4b2937e366 100644 --- a/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts +++ b/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts @@ -211,7 +211,9 @@ export async function recordCodexSecurityScanDraftViaWorkbench( accepted?.status === "draft_written" && isDeepStrictEqual( accepted.input, - JSON.parse(JSON.stringify({ ...stagedDraft, checkpoint: snapshot })), + JSON.parse( + JSON.stringify({ ...stagedDraft, checkpoint: snapshot }), + ), ) ) { signal?.throwIfAborted(); From 95f1160845c5ee4673bb4e2ae5e2be1f0d372ca3 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Mon, 12 Oct 2026 02:50:21 +0000 Subject: [PATCH 23/23] fix(publication): keep late Windows path failures fatal --- .../scripts/windows_scan_local_files.py | 31 ++++++--- .../test_deep_scan_publication_authority.py | 66 ++++++++++++++++--- .../tests/test_windows_scan_local_files.py | 41 ++++++++++++ 3 files changed, 121 insertions(+), 17 deletions(-) diff --git a/plugins/codex-security/scripts/windows_scan_local_files.py b/plugins/codex-security/scripts/windows_scan_local_files.py index 7cadfb948a..c884ba5cb8 100644 --- a/plugins/codex-security/scripts/windows_scan_local_files.py +++ b/plugins/codex-security/scripts/windows_scan_local_files.py @@ -76,6 +76,7 @@ class WindowsScanLocalPathError(WindowsScanLocalFileError): _ERROR_LOCK_VIOLATION = 33 _ERROR_FILE_EXISTS = 80 _ERROR_ALREADY_EXISTS = 183 +_ERROR_DIRECTORY = 267 _MISSING_ERRORS = {_ERROR_FILE_NOT_FOUND, _ERROR_PATH_NOT_FOUND} _COLLISION_ERRORS = {_ERROR_FILE_EXISTS, _ERROR_ALREADY_EXISTS} _INVALID_HANDLE_VALUE = ctypes.c_void_p(-1).value @@ -421,14 +422,19 @@ def _canonical_scan_directory(scan_dir: Path) -> tuple[Path, tuple[int, int]]: def _open_directory(path: Path, *, missing_ok: bool = False) -> _OwnedHandle | None: - handle = _create_file( - path, - access=_FILE_READ_ATTRIBUTES, - share=_FILE_SHARE_READ | _FILE_SHARE_WRITE, - disposition=_OPEN_EXISTING, - flags=_FILE_FLAG_BACKUP_SEMANTICS | _FILE_FLAG_OPEN_REPARSE_POINT, - missing_ok=missing_ok, - ) + try: + handle = _create_file( + path, + access=_FILE_READ_ATTRIBUTES, + share=_FILE_SHARE_READ | _FILE_SHARE_WRITE, + disposition=_OPEN_EXISTING, + flags=_FILE_FLAG_BACKUP_SEMANTICS | _FILE_FLAG_OPEN_REPARSE_POINT, + missing_ok=missing_ok, + ) + except WindowsScanLocalFileError as exc: + if exc.errno not in {*_MISSING_ERRORS, _ERROR_DIRECTORY}: + raise + raise _invalid_path(path, f"could not open scan-local directory: {exc}") from exc if handle is not None: try: assert handle.value is not None @@ -471,7 +477,14 @@ def _locked_parent( directory_handle = _open_directory(directory_path) assert directory_handle is not None handles.append(directory_handle) - current_root = root_path.lstat() + try: + current_root = root_path.lstat() + except OSError as exc: + if exc.errno not in {errno.ENOENT, errno.ENOTDIR, errno.ELOOP}: + raise + raise _invalid_path( + scan_dir, f"scan directory changed while it was being opened: {exc}" + ) from exc current_root_identity = (current_root.st_dev, current_root.st_ino) if current_root_identity != observed_root_identity or ( expected_root_identity is not None and current_root_identity != expected_root_identity diff --git a/plugins/codex-security/tests/test_deep_scan_publication_authority.py b/plugins/codex-security/tests/test_deep_scan_publication_authority.py index 6861615209..cc59a625d2 100644 --- a/plugins/codex-security/tests/test_deep_scan_publication_authority.py +++ b/plugins/codex-security/tests/test_deep_scan_publication_authority.py @@ -464,8 +464,19 @@ def replace_file(source, destination, *args, **kwargs): @pytest.mark.parametrize("workflow", ["deep-scan-mcp/v1", "deep-security-scan/v1"]) -@pytest.mark.parametrize("operation", ["lstat", "resolve"]) -@pytest.mark.parametrize("failure", ["io", "missing", "not-directory", "loop"]) +@pytest.mark.parametrize( + ("operation", "failure"), + [ + (operation, failure) + for operation in ("lstat", "resolve", "locked-lstat") + for failure in ("io", "handles", "denied", "missing", "not-directory", "loop") + ] + + [ + (operation, failure) + for operation in ("ancestor-open", "root-open") + for failure in ("io", "handles", "denied", "missing", "missing-parent", "not-directory") + ], +) def test_windows_receipt_root_probe_classifies_storage_and_path_failures( workbench_api, workbench_db, @@ -519,13 +530,21 @@ def write_file(scan_dir, filename, contents): with monkeypatch.context() as patch: backend = emulate_windows_atomic_write(patch, finalizer) original_probe = backend._canonical_scan_directory - original_operation = getattr(Path, operation) + path_operation = "lstat" if operation == "locked-lstat" else operation + original_operation = getattr(Path, path_operation, None) + original_create = backend._create_file + original_directory = backend._open_directory def fail_root(path, *args, **kwargs): if path == scan_dir: probes.append(operation) - if failure == "io": - raise OSError(errno.EIO, "Synthetic receipt root I/O failure", str(path)) + if operation == "locked-lstat": + patch.setattr(Path, "lstat", original_operation) + if failure in {"io", "handles", "denied"}: + code = {"io": errno.EIO, "handles": errno.EMFILE, "denied": errno.EACCES}[ + failure + ] + raise OSError(code, "Synthetic receipt root I/O failure", str(path)) # Exercise actual missing, non-directory, and loop errors without # changing the already accepted canonical artifacts. return original_operation(bad_root, *args, **kwargs) @@ -538,11 +557,39 @@ def probe_root(path): probe_patch.setattr(Path, operation, fail_root) return original_probe(path) - patch.setattr(backend, "_canonical_scan_directory", probe_root) + def open_directory(path, **kwargs): + selected = scan_dir.parent if operation == "ancestor-open" else scan_dir + if path == selected and kwargs["disposition"] == backend._OPEN_EXISTING: + probes.append(operation) + code = { + "io": 1117, + "handles": 4, + "denied": 5, + "missing": 2, + "missing-parent": 3, + "not-directory": 267, + }[failure] + raise backend.WindowsScanLocalFileError( + code, "Synthetic late directory failure", str(path) + ) + return original_create(path, **kwargs) + + def open_root_before_stat(path, **kwargs): + handle = original_directory(path, **kwargs) + if path == scan_dir: + patch.setattr(Path, "lstat", fail_root) + return handle + + if operation == "locked-lstat": + patch.setattr(backend, "_open_directory", open_root_before_stat) + elif operation.endswith("-open"): + patch.setattr(backend, "_create_file", open_directory) + else: + patch.setattr(backend, "_canonical_scan_directory", probe_root) return original_write(scan_dir, filename, contents) monkeypatch.setattr(saved, "write_scan_local_bytes", write_file) - if failure == "io": + if failure in {"io", "handles", "denied"}: assert saved.write_scan_draft( workbench_api["_WORKBENCH_DB_CONTEXT"], workbench_db, args ) == {"scanId": scan.scan_id, "status": "draft_written"} @@ -553,8 +600,11 @@ def probe_root(path): if failure == "loop" and operation == "resolve" else finalizer.ContractError ) - with pytest.raises(expected_errors, match="existing scan directory|Symlink loop"): + with pytest.raises(expected_errors) as rejected: saved.write_scan_draft(workbench_api["_WORKBENCH_DB_CONTEXT"], workbench_db, args) + assert not isinstance(rejected.value, finalizer.ScanLocalIOError) + if operation.endswith("-open"): + assert "Synthetic late directory failure" in str(rejected.value) assert probes == [operation] assert published assert {path: path.read_bytes() for path in published} == published diff --git a/plugins/codex-security/tests/test_windows_scan_local_files.py b/plugins/codex-security/tests/test_windows_scan_local_files.py index 25f4c8af34..05c8b3a9e3 100644 --- a/plugins/codex-security/tests/test_windows_scan_local_files.py +++ b/plugins/codex-security/tests/test_windows_scan_local_files.py @@ -121,3 +121,44 @@ def test_native_copy_junction_rejects_nonempty_destination(tmp_path: Path) -> No WINDOWS_FILES.copy_directory_junction(source, destination) assert (destination / "keep.txt").read_text() == "placeholder contents\n" assert not (target / "keep.txt").exists() + + +@pytest.mark.parametrize("changed", ["root", "ancestor"]) +@pytest.mark.parametrize("replacement", ["missing", "file"]) +def test_native_directory_change_after_probe_remains_a_path_error( + tmp_path: Path, monkeypatch, changed: str, replacement: str +) -> None: + finalizer = load_script("finalize_scan_contract") + ancestor = tmp_path / "ancestor" + scan_dir = ancestor / "scan" + scan_dir.mkdir(parents=True) + (scan_dir / "keep.txt").write_bytes(b"Accepted content") + finalizer.write_scan_local_bytes(scan_dir, "new-parent/receipt.json", b"Ordinary creation") + assert (scan_dir / "new-parent/receipt.json").read_bytes() == b"Ordinary creation" + moved = tmp_path / "retained" + target = scan_dir if changed == "root" else ancestor + retained_scan = moved if changed == "root" else moved / "scan" + mutated = False + # Windows exercises the actual CreateFileW boundary; POSIX is a native + # control at its existing directory-validation boundary. + owner = finalizer._windows_scan_local_files() if os.name == "nt" else finalizer + name = "_canonical_scan_directory" if os.name == "nt" else "_require_scan_directory" + original = getattr(owner, name) + + def replace_after_probe(path): + nonlocal mutated + result = original(path) + if path == scan_dir and not mutated: + mutated = True + target.rename(moved) + if replacement == "file": + target.write_bytes(b"Replacement file") + return result + + monkeypatch.setattr(owner, name, replace_after_probe) + with pytest.raises(finalizer.ContractError) as rejected: + finalizer.write_scan_local_bytes(scan_dir, "drafts/receipt.json", b"receipt") + assert not isinstance(rejected.value, finalizer.ScanLocalIOError) + assert mutated + assert (retained_scan / "keep.txt").read_bytes() == b"Accepted content" + assert not (retained_scan / "drafts/receipt.json").exists()