fix(multiscan): keep scan outcomes when checkout cleanup fails

[rohanpoudel2]

Fixes #211

Problem

The per-repository worker in runCampaign awaited checkout removal in a finally that ran before the attempt receipt was appended:

} catch (error) {
  if (options.signal?.aborted === true) options.signal.throwIfAborted();
  failure = redactedErrorMessage(error);
} finally {
  await rm(checkout, { recursive: true, force: true });
}
const status = failure === undefined ? "completed" : "failed";
await appendReceipt(...);

force: true ignores a checkout that is already gone, but it does not suppress EACCES, EPERM or EBUSY — a still-open handle on Windows, or a parent directory that is no longer writable. When the removal rejected, its filesystem error replaced the outcome the worker had just captured, and execution never reached appendReceipt. Three things went wrong at once: the real scan failure was lost, the campaign surfaced a confusing removal error in its place, and the attempt was never recorded, so resume had no receipt for it.

Change

Capture the removal failure instead of throwing it. The attempt keeps the status its scan earned, always gets a receipt, and reports the removal failure alongside any scan failure, so a leftover checkout is still surfaced rather than silently dropped.

A cleanup failure on a scan that otherwise succeeded therefore produces a completed receipt carrying an error field. That combination is already representable — error is optional on MultiscanReceipt and independent of status — and it is the honest record: the scan did complete, so resume correctly counts it as done rather than re-running billed model work, while the operator still learns that a checkout was left behind. The CLI's existing progress renderer prints it without any change:

codex-security: stubborn completed (attempt 1): Multiscan checkout cleanup failed: EACCES: permission denied, rm '...'

A leftover checkout cannot be mistaken for a fresh one later: every attempt removes the checkout again at the top of the try, so a leftover that outlives this run fails there instead.

Cancellation is unaffected — the signal.throwIfAborted() check stays in the catch, ahead of cleanup.

Verification

Two regression tests in tests-ts/multiscan.test.ts inject a failing removal through mock.module("node:fs/promises", …), the same seam tests-ts/api.test.ts already uses to test cleanup failures. The injection only rejects for the checkout path and only once the scan is over, so the removal at the top of the try still runs normally. That keeps the tests deterministic and cross-platform, rather than depending on icacls or on POSIX permission semantics.

Against the unfixed worker, the injected EACCES escapes runMultiscan entirely, exactly as reported:

EACCES: permission denied, rm '.../results/checkouts/stubborn'
      at runCampaign (src/multiscan.ts:339:29)
      at runMultiscan (src/multiscan.ts:189:15)
(fail) multiscan > keeps a failed scan's outcome when its checkout cannot be removed
(fail) multiscan > keeps a completed scan's outcome when its checkout cannot be removed
 0 pass
 2 fail

The tests assert both halves of the defect: that ORIGINAL_SCAN_FAILURE survives, and that the receipt is written at all. With the fix:

 2 pass
 0 fail

Full suite: 719 pass / 5 skip / 0 fail. pnpm run types and pnpm run format are clean.

Same-class audit

The other finally blocks in multiscan.ts — the supervisor lock release and the client close() path — do not have this shape: they run where a throw is the intended outcome, or already guard their own failures. This was the only cleanup that could silently overwrite a captured result.

[mldangelo-oai]

@codex review exact head 36f0bbd

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 36f0bbd5d3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Security review completed. No security issues were found in this pull request.

Reviewed commit: 36f0bbd5d3

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

• You comment "@codex security review"
• A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.