Skip to content

fix: allow completion when no draft manifest exists yet (#73) - #83

Merged
mldangelo-oai merged 2 commits into
openai:mainfrom
onurtirpan:fix/scan-manifest-required-on-fresh-scan
Jul 31, 2026
Merged

fix: allow completion when no draft manifest exists yet (#73)#83
mldangelo-oai merged 2 commits into
openai:mainfrom
onurtirpan:fix/scan-manifest-required-on-fresh-scan

Conversation

@onurtirpan

@onurtirpan onurtirpan commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

This fixed the issue I was hitting locally (#73). Opening it here mostly as a reference artifact so maintainers have the diff, the test run, and one place to point at. Happy to close it if you'd rather land the change through the canonical mirror.

The bug

Any recipe-driven scan (i.e. anything started via the CLI) fails at completion with:

codex-security: Could not save the Codex Security scan: scan-manifest.json:
  expected a regular file inside the scan directory.

workbench_db.complete_scan reads scan-manifest.json with required=True when scan.recipe_json is not None, but on a first completion of a fresh scan there is no draft on disk yet. The draft gets written later in _write_prepared_scan_finalization. So the read blows up before the write can happen, the scan_dir stays empty, and the model tokens are already spent.

I reproduced this 8 times across Windows Server 2025 and Ubuntu 24.04 in Docker with 3-file, 77-file, and 656-file targets before finding the diff. About $17 of API spend to chase it down. Full details in #73.

What this changes

Read scan-manifest.json only when it already exists. When it does (resumed / re-completed scans) the sealed timestamps are preserved same as before. When it doesn't, skip and let the finalizer write the draft.

     if scan["recipe_json"] is not None:
-        manifest = read_json_object(artifact_path(scan_dir, ARTIFACTS["manifest"], required=True))
-        manifest_scan = manifest.get("scan")
-        if isinstance(manifest_scan, dict) and manifest_scan.get("sealedAt") is not None:
-            completion_binding["startedAt"] = manifest_scan.get("startedAt")
-            completion_binding["completedAt"] = manifest_scan.get("completedAt")
+        manifest_path = artifact_path(scan_dir, ARTIFACTS["manifest"], required=False)
+        if manifest_path is not None:
+            manifest = read_json_object(manifest_path)
+            manifest_scan = manifest.get("scan")
+            if isinstance(manifest_scan, dict) and manifest_scan.get("sealedAt") is not None:
+                completion_binding["startedAt"] = manifest_scan.get("startedAt")
+                completion_binding["completedAt"] = manifest_scan.get("completedAt")

Comment above the guard points at #73 for context.

Tests

Ran the public SDK test suite on main + this patch:

  • pnpm install --frozen-lockfile
  • pnpm run lint — clean
  • pnpm test — 367 pass, 23 expected skips, 0 fail

One thing this does NOT fix

After applying the patch, completion still hits a second read of the same file in _prepare_scan_finalization at finalize_scan_contract.py:2017. That one looks like an upstream problem where the recipe/agent should be writing the draft manifest but isn't — see #73 comment for the trace. Out of scope for this PR.

Contribution policy

Noted from CONTRIBUTING.md that external PRs can't be imported. Not trying to work around that; just leaving the diff + test run somewhere convenient in case it saves the maintainers a few minutes.

Fresh recipe-driven scans fail at completion with:

  Could not save the Codex Security scan: scan-manifest.json:
  expected a regular file inside the scan directory.

`workbench_db.complete_scan` reads `scan-manifest.json` with
`required=True` when `scan.recipe_json is not None`, but on a first
completion of a fresh scan there is no prior draft on disk yet — the
draft is authored later by `_write_prepared_scan_finalization`. The
read therefore always raises before the write can happen, and every
retry still throws before writing anything, so `scan_dir` stays empty
and models are billed with no artifacts produced.

Make the pre-write read optional. When a prior manifest is present
(resumed / re-completed scans), preserve the sealed timestamps as
before; otherwise skip that step and let the downstream finalizer
write the draft.

Verified against `pnpm run lint` (tsc --noEmit) and the full
`bun test` suite on the public SDK: 367 pass, 23 expected skips,
0 fail.

Refs openai#73
@mldangelo-oai

Copy link
Copy Markdown
Collaborator

Thanks for the detailed report and diagnostic PR, @onurtirpan. I preserved your original commits and opened maintainer-owned follow-up #167 with a fail-closed missing-draft diagnostic, full regression coverage, and clearer sandbox/write guidance. It does not fabricate findings or close #73; direct pushes to contributor forks are blocked by organizational policy, so this original PR remains unchanged.

@mldangelo-oai
mldangelo-oai merged commit 2b2d87c into openai:main Jul 31, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants