Repository navigation
refactor(deep-scan)!: compose ordinary scans and simplify recovery - #939
mldangelo-oai wants to merge 235 commits into
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57984f86c6
ℹ️ 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".
| const inheritedEnvironment = await snapshotNativeEnvironment(); | ||
| const codex = await resolveTrustedCodex( | ||
| inheritedEnvironment, | ||
| (await gitMarkerRoot(input.scan.targetPath, signal, "outermost")) ?? | ||
| input.scan.targetPath, | ||
| ); |
There was a problem hiding this comment.
When the host stops after artifacts are sealed but before complete-scan commits the parent, the next invocation still enters prepareNativeScan because the database status remains running. This eagerly requires a trusted Codex executable before publicationOnly is detected below, so if CODEX_CLI_PATH or PATH changed during the restart—for example, while the CLI was upgraded—the already-completed scan cannot be published even though the sealed-result branch in api.ts performs no model turn. Detect sealed publication before resolving Codex, or otherwise avoid requiring an executable on this recovery-only path.
Useful? React with 👍 / 👎.
| if ( | ||
| record.parentScanId !== input.scanId || | ||
| record.targetPath !== input.repository | ||
| ) |
There was a problem hiding this comment.
When the repository changes between Deep Scan batches, a newly registered Standard child captures the new revision, but this acceptance check verifies only its parent ID and repository path. The projection helper then validates that child against its own receipt, not the parent's original revision, so the reducer can combine findings from multiple code states while the parent manifest remains bound to the original target revision; a final target-change warning does not make those findings internally consistent. Compare each child's saved target revision or diff snapshot with the parent's registered target identity before accepting it.
AGENTS.md reference: sdk/typescript/AGENTS.md:L23-L24
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ff5df0a17
ℹ️ 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".
| externalStop.abort(error); | ||
| if (discoverySignal.aborted) throw error; | ||
| if (attempt >= retries.length) { | ||
| await failPass(error); |
There was a problem hiding this comment.
When projectChild keeps failing after client.run has already sealed the ordinary child, the final retry calls failPass on that completed scan. The workbench rejects that transition (a completed scan cannot be marked failed), so this secondary error escapes before exhaustPassRetries and replaces the actual projection or artifact-integrity failure. Only mark a still-running child failed, or retain and rethrow the original projection error when failure recording is rejected.
Useful? React with 👍 / 👎.
|
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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13084da10d
ℹ️ 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".
| knowledgeBasePaths: | ||
| recipe.knowledgeBasePaths ?? | ||
| (environment.CODEX_SECURITY_KNOWLEDGE_BASE | ||
| ? [environment.CODEX_SECURITY_KNOWLEDGE_BASE] | ||
| : undefined), |
There was a problem hiding this comment.
When a native scan was created without a knowledge base, its recipe omits knowledgeBasePaths; if CODEX_SECURITY_KNOWLEDGE_BASE is set before a later rejoin, this fallback treats that new path as part of the saved scan. The resume branch then calls restoreScanKnowledge, but the original scanInputs records a null knowledge base, so rejoining—including sealed-result publication—fails with “no saved knowledge-base snapshot” instead of preserving the scan’s original inputs. Apply the environment fallback only for fresh scans; resumed scans should use only the recipe.
AGENTS.md reference: AGENTS.md:L31-L35
Useful? React with 👍 / 👎.
| with completion_lock(scan_id): | ||
| with pytest.raises(RuntimeError, match="nested failure"), completion_lock(scan_id): | ||
| raise RuntimeError("nested failure") | ||
| assert acquire.call_count == 1 |
| ): | ||
| assert acquire.call_count == 2 | ||
| assert release.call_count == 1 | ||
| with completion_lock(scan_id): |
| release_completion_file_lock=mock.Mock(), | ||
| ), | ||
| ): | ||
| acquire = lock_globals["acquire_completion_file_lock"] |
| ), | ||
| ): | ||
| acquire = lock_globals["acquire_completion_file_lock"] | ||
| release = lock_globals["release_completion_file_lock"] |
| @@ -10,6 +10,7 @@ | |||
| import sys | |||
| import tempfile | |||
| import threading | |||
| import unittest | |||
Summary
Deep Scan repeatedly reviews a repository to find additional issues and combines the results. This change runs those reviews as batches of ordinary Standard scans, sharing their execution, authentication, permissions, and artifact lifecycle. A merge model groups duplicates and selects representative originals; host code retains the findings, evidence, coverage, and costs. Batches with no findings need no merge-model turn.
Changes
Testing
build:ci, source compatibility, and the source-compatibility test suite.13084da10d5passed 158 focused Python tests on Python 3.13.14, with 11 Windows-only tests skipped locally, and six affected SDK resume cases. All five portable plugin checks were repeated successfully on this follow-up.2ff5df0a177passed 4,867 tests with 66 skipped and no failures. The later fixed-seed and randomized runs atf38509061c9completed with one and eight 30-second timeouts respectively; those exact cases passed hosted Ubuntu baseline and parallel runs at0e8ad8c0592. The original local failures remain recorded.Tests use synthetic local artifacts, credentials, and model responses; these checks do not assess live model quality.
Risk and rollout
Ship the plugin and bundled SDK together. Interrupted v1 scans and unsealed v1 worker fragments require the prior release; completed historical reports remain readable. Historical checkpoints without a pending checkpoint index are no longer imported automatically into unfinished scans. Finish those scans with the prior release or start a fresh scan.
get-cli-scan-resume --migrateand implicit npm, managed-package, and desktop-cache executable discovery are removed. ConfigureCODEX_CLI_PATHor expose the real executable onPATH. Unreleased database snapshots that inferred child membership from artifact paths are not backfilled; start fresh scans for those snapshots. Schema migrations remain append-only.The selected original supplies each merged finding's current narrative; earlier accepted narratives and every source finding remain in provenance. This does not claim improved precision, recall, or latency. Credential, unsafe-path, target/claim, and artifact-seal protections remain. If a limit stops discovery before any child starts, the empty result retains partial coverage and may have a null thread ID.
Public disclosure review