fix: harden Deep Scan worker startup and finalization - #274
fix: harden Deep Scan worker startup and finalization#274mldangelo-oai wants to merge 6 commits into
Conversation
|
@codex review Please review the exact current head |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…odex/harden-deep-worker-finalization
There was a problem hiding this comment.
💡 Codex Review
When the host reports the valid permissionProfile.type: "disabled" with an unrestricted filesystem—as in Code mode and the supplied environment—the decompressed runtime's resolveDeepWorkerParentSandbox rejects it solely because the type is not managed. This happens before deepScanStore.begin, so every start_codex_security_deep_scan call returns a non-retryable tool error even though the worker itself is explicitly launched with sandboxMode: "read-only". Treat a disabled/unrestricted parent as having no restrictions to preserve, or otherwise support this advertised headless environment.
When two record_codex_security_scan_draft calls for the same scan overlap—for example, when an idempotent client retry starts before the original call returns—the decompressed runtime replaces findings.json, coverage.json, and scan-manifest.json sequentially under three independent per-file locks. The calls can therefore interleave and both report success while leaving one call's findings combined with the other's coverage or manifest, and later completion can seal that internally inconsistent result. Hold one scan-wide lock or stage and commit the three-document set as a single operation.
For a running non-diff Git scan migrated from before target_snapshot_digest was recorded, the workbench intentionally exposes allowedKinds as ["git_worktree", "git_revision"]; however, the new buildTarget always selects the first value and has no digest to attach. It therefore writes a git_worktree target without the snapshotDigest required by scan-manifest.schema.json, so finalization cannot complete that otherwise recoverable legacy scan. Select git_revision when no authoritative snapshot digest exists, or derive the kind from the available coordinates rather than array order.
ℹ️ 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".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…odex/harden-deep-worker-finalization # Conflicts: # sdk/typescript/_bundled_plugin/mcp/mcp-app.html.br # sdk/typescript/_bundled_plugin/mcp/server.mjs.br.part-000 # sdk/typescript/_bundled_plugin/mcp/server.mjs.br.part-001
|
@codex review Please review the exact updated head |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…odex/harden-deep-worker-finalization # Conflicts: # sdk/typescript/_bundled_plugin/mcp/server.mjs.br.part-000 # sdk/typescript/_bundled_plugin/mcp/server.mjs.br.part-001
|
@codex review Please review the exact updated head |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…odex/harden-deep-worker-finalization # Conflicts: # sdk/typescript/_bundled_plugin/mcp/server.mjs.br.part-000 # sdk/typescript/_bundled_plugin/mcp/server.mjs.br.part-001
|
@codex review Please review the exact updated head |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…odex/harden-deep-worker-finalization
|
@codex review Please review the exact updated head |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Summary
Stack
mdangelo/codex/fail-stale-scan-targetsso this review contains only its atomic upstream change.Verification