Repository navigation
refactor(deep-scan)!: run scans in batches and resume saved results - #1103
mldangelo-oai wants to merge 141 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
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". |
7804913 to
e29a658
Compare
593fdbb to
31e0f2c
Compare
e29a658 to
f2c3eed
Compare
31e0f2c to
42bd243
Compare
|
@codex review |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The scan-scoped persistence and guarded migration are consistent and adequately tested.
Review effort: Balanced
Findings: None
What changed in this PR
Scopes severity assessments per scan to prevent cross-scan overwrites.
Changes:
- Adds migration 42 and scan-keyed assessment storage.
- Passes scan IDs through save/read paths.
- Adds concurrency and migration coverage.
| File | Description |
|---|---|
sdk/typescript/tests-ts/classify-scan-severity.test.ts |
Tests scan isolation and migration behavior. |
sdk/typescript/src/severity-store.ts |
Includes scan ID when saving assessments. |
plugins/codex-security/tests/test_workbench_setup_and_migrations.py |
Tests migration 42 and guarded backfill. |
plugins/codex-security/tests/test_workbench_deep_scan.py |
Updates expected schema version. |
plugins/codex-security/tests/test_workbench_db.py |
Updates expected schema tables and version. |
plugins/codex-security/scripts/workbench_severity.py |
Reads and writes scan-scoped assessments. |
plugins/codex-security/scripts/workbench_schema.py |
Defines and backfills the new table. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Codex Review: Didn't find any major issues. Delightful! 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". |
mldangelo-oai
changed the title
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Reviewed 42bd243a38d3d3a79c0784eebdf1be870312645a against mdangelo/codex/pr939-stack-01-verification at f2c3eed65b1482497c1997bb53b463160b18fb52, scoped to this PR's diff and touched files.
Reviewed the per-scan schema migration, checkpoint writes/reads, and regression tests. No new actionable findings. Direct SQLite probes using the changed schema and checkpoint code preserved separate assessments for two scans of one finding.
Review included simplification/deletion opportunities. Full repository and platform test suites were not run.
| AND EXISTS ( | ||
| SELECT 1 FROM finding_occurrences AS occurrence | ||
| WHERE occurrence.id = assessment.occurrence_id | ||
| AND occurrence.scan_id = classification.scan_id |
There was a problem hiding this comment.
[P2] Preserve assessments for unindexed scan directories
classify-severity --scan-dir supports directories that have never been indexed: checkpoint(save) inserts the finding but does not create a finding_occurrences row. This EXISTS condition therefore excludes valid saved classifications when any subsequent workbench operation applies migration 42. I reproduced a rubric assessment going from one readable assessment before migration to zero afterward; the legacy row still exists, but the reader now uses only the new table, so publication fails as incomplete. Reclassifying requires another model call and can change the saved decision. The scan association is recoverable from the existing deterministic occurrence ID (scan_id + the stored finding fingerprint), without requiring an occurrence index. Could we backfill using that binding and change the unindexed migration test to require preservation rather than expecting the classification to become incomplete?
There was a problem hiding this comment.
Confirmed that this persists at part 23 (#1095, commit 3065926): both parameterized migration cases pass, but the unindexed case explicitly asserts that no assessment reaches the new table. This verifies the migration omission; the external classify-severity --scan-dir impact was checked statically, not through an end-to-end run. Please preserve the saved classification and make the unindexed case assert retention.
42bd243 to
f27e123
Compare
183b88d to
75d6f5c
Compare
f27e123 to
144ac4f
Compare
faizan-oai
left a comment
faizan-oai
left a comment
There was a problem hiding this comment.
Reviewed the per-scan assessment key, save/read paths, and migration matching. Keeping assessments keyed by both scan and finding addresses the cross-scan overwrite. No issue found in this layer. Local migration checks were run at the integrated stack tip, rather than independently at this commit.
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
This moves severity assessments to per-scan storage and isolates concurrent classifications. Approval is withheld for the existing P2 in discussion_r4146527174: migration 42 still drops valid unindexed classifications from the newly selected table. I reproduced one readable legacy assessment becoming zero per-scan assessments using the exact migration SQL, with the original row retained and no foreign-key errors. The same migration remains at the current stack tip.
Completed three independent high-reasoning reviews and source verification against 75d6f5cb78812d7967ae03d641c4f32922274049 → 144ac4fe6f87ed20376f9069627c44fcb79516fd. No additional finding to duplicate here.
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Re-reviewed after the base PR was merged and this PR was retargeted to main at 688e2aee6290e80b3e71ad132adf3e8071a2afa8. Three fresh independent passes found no additional issues. GitHub test merge 69272724c5a7f3f23da54de5b48b1b70f3b531cc has the expected base/head parents; its seven-file change matches the original severity patch despite the expanded three-dot diff.
Approval remains withheld for the existing P2 migration finding. On that test merge, the real migration still leaves one valid saved assessment with a deterministic occurrence identity as zero entries in the new per-scan table. The original row remains, but the new reader cannot use it. The SDK migration test explicitly expects the saved classification to become incomplete. The later recovery in #1150 fixes this at the final stack tip; that fix is not part of this PR.
Validation on the test merge: 296 Python tests and 14 classification tests passed; plugin build, Ruff lint/format, CI TypeScript build, portable source compatibility and its nine tests, and diff whitespace checks passed. No live model calls.
144ac4f to
fd5b5ba
Compare
|
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". |
![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: fd5b5ba147
ℹ️ 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".
| "assessments": assessments( | ||
| connection, finding_ids, scan_id if has_scan_assessments else None | ||
| ), |
There was a problem hiding this comment.
For a database that already has migration 42 but whose external scan was not indexed when that migration ran, scan_severity_assessments exists but contains no row while the recoverable assessment remains in finding_severity_assessments. Publication calls read-severity-classification through the read-only path, which bypasses connect() and therefore never runs migration 46; this existence check consequently selects the empty new table and reports the classification as incomplete. Read the validated legacy row as a fallback for missing per-scan rows (or otherwise perform recovery before choosing the new table) so an upgraded scan can still be published without first running an unrelated mutating command or reclassification.
Useful? React with 👍 / 👎.
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Re-reviewed fd5b5ba147484f20d64bf0680702ef6c7385631b against be8470aa77f21a8bdad614e994b3982066a21073, with three fresh independent passes and root verification.
Migration 46 fixes the earlier loss during migration: a valid unindexed legacy assessment now survives the upgrade, and repeating the migration preserves the recovered row.
One P2 remains for databases already at migration 42, as described in discussion_r4161619542. I reproduced it through the actual SDK publication-preparation API: one saved legacy assessment plus an empty per-scan table produces the incomplete-classification error on read-only publication. Running classification once applies migration 46; the same assessment and publication then succeed. A read-only caller needs a validated fallback for this recoverable row, or recovery before selecting the new table. No duplicate inline comment added; approval remains withheld for this specific upgrade path.
Validation: 14 SDK classification tests; the three touched Python test files; plugin build; Ruff lint/format; SDK TypeScript check; portable source compatibility and its tests. The synthetic migration and publication probes used no model calls or external publication. Current CI still has queued jobs.
fd5b5ba to
a2ca4d5
Compare
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Re-reviewed the real PR diff at a2ca4d5dd7c68158753f039b574c0822ba0ca6b8. Three independent high-reasoning passes covered the complete changed patch and tests; root source checks and focused reproductions informed this decision.
The read-only classification fallback now recovers legacy assessments before any migration write, preserves existing per-scan rows and does not borrow another scan's assessment. Exact-head regression tests and actual SDK publication preparation pass.
Validation across touched paths at branch tip 06e75cfe: 1,356 SDK tests, 867 Python tests plus 100 subtests, and 108 MCP tests passed; builds, types, portable checks and installed-package checks passed. These are integration checks at the branch tip, not a claim that every intermediate PR or its CI is clean. The branch still descends from be8470aa, while the base ref is 2fd78804. GitHub test merge f1c97e81 has the expected parents and identical contents for all eight touched files; the broad local tip tests do not include the newer main changes.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f028959eb
ℹ️ 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".
| [ | ||
| "analytics", | ||
| "shell_environment_policy", | ||
| "model_instructions_file", | ||
| "model_catalog_json", | ||
| ] |
There was a problem hiding this comment.
When a Deep Scan starts with an explicit override such as --codex developer_instructions="Focus on authentication boundaries" and is later resumed, scanPreflightCodexConfig omits that setting and this new replay allowlist preserves only the four listed keys. The resumed discovery and merge workers therefore silently lose the caller's developer-level instructions, potentially changing the scan's scope and results. The new allowlist now covers file-backed instructions, but fresh evidence shows that the equivalent inline developer_instructions setting is still excluded; include it in the saved replay configuration and cover the resumed child-process boundary.
AGENTS.md reference: AGENTS.md:L31-L35
Useful? React with 👍 / 👎.
| } | ||
| const inheritedPermissions = { | ||
| filesystem: Object.fromEntries([ | ||
| [":workspace_roots", "write"], |
There was a problem hiding this comment.
When a native Deep Scan is invoked from a managed parent that grants root read but no workspace write, resolveNativeParentSandbox accepts that profile (the new permission test explicitly exercises this read-only case), yet this unconditional entry gives every launched discovery and merge process write access to all workspace roots. Because the nested Codex process enforces the profile constructed here, it can modify the scanned repository even though the parent authorization was read-only; preserve the parent's workspace access level or reject parents that cannot authorize the write access required by these workers.
AGENTS.md reference: AGENTS.md:L5-L8
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 963e503c64
ℹ️ 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".
| if (waitOnly) await nativeScans.wait(scanId); | ||
| else await nativeScans.cancel(scanId, message); |
There was a problem hiding this comment.
When fail_codex_security_scan stops a Deep Scan that is active in this MCP process, this passes only the failure message to NativeScanHost.cancel, which wraps it in a plain Error. runDeepScans.stopReason() classifies an ordinary aborted execution as canceled, so the checkpoint persists terminalReason: "canceled" even though the workbench recorded the scan as failed, causing retained state and later recovery diagnostics to contradict the terminal failure. Pass an explicit failed-versus-canceled outcome through the abort path.
Useful? React with 👍 / 👎.
zcrab-oai
left a comment
zcrab-oai
left a comment
There was a problem hiding this comment.
Reviewed the changes since the previous reviewed head in two passes, using exact source at 963e503c64f1 and prior review evidence. Unchanged portions of the large branch were not exhaustively re-audited. This pass focused on runtime, replay, registration, publication and integration paths.
Approved with these considerations still open:
- P2: Fresh native workers receive workspace write access even when parent metadata grants only root read access. Preserve applicable parent write authorization. The actual worker cwd is the scan/merge output directory; repository modification or an OS sandbox escape was not established. plugins/codex-security/mcp-app/src/native-scan.ts:260
- P2: Invocation-only developer_instructions still disappear on CLI resume because both preflight projection and the replay allowlist omit them. This gap also exists in the base. Include the setting in private replay state and test recipe-rebuilt workers. sdk/typescript/src/provider-profile.ts:71
- P2: Native publication-only recovery resolves Codex before testing whether model execution is needed. A sealed pending result becomes unrecoverable when the old executable moves. Detect publication-only recovery first while retaining seal, ownership and claim checks. plugins/codex-security/mcp-app/src/native-scan.ts:224
Validation: CI-helper TypeScript compilation and plugin bundling passed locally; native binaries came from CI with matching compiled source inputs. Focused SDK tests passed: scan-registration.test.ts. The full cross-platform suite was not run locally; Windows execution remains unverified here.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| sealed_producer_version=lambda _: sealed_version, | ||
| ) | ||
| if not args.archive_existing or args.archived_scan_dir is not None: | ||
| scan_dir = require_cli_scan_directory(repository, args) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa07d169aa
ℹ️ 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".
| "--scan-id", | ||
| childId, | ||
| "--message", | ||
| message.slice(0, 2400), |
There was a problem hiding this comment.
When a discovery child fails with a diagnostic longer than 2,400 characters—for example, when the actionable nested cause or tool stderr appears near the end—this permanently stores only the prefix in fail-scan. Workbench history and resumed-scan diagnostics can therefore no longer explain the failure even though the command and database accept the full string; pass message unchanged.
AGENTS.md reference: AGENTS.md:L9-L12
Useful? React with 👍 / 👎.
| "--scan-id", | ||
| scanId, | ||
| "--phase", | ||
| "reporting", |
There was a problem hiding this comment.
[P2] Delay the reporting phase until discovery has stopped
When an early batch requires a model merge and more discovery passes remain, this permanently advances the parent to reporting before the loop starts the next batch. Both update-progress and advance_scan_phase allow only forward transitions, so later discovery checkpoint writes cannot restore discovery; get-scan and resumed progress therefore show consolidation while ordinary scans are still reviewing source. A workbench probe requesting discovery → reporting → discovery retained reporting and rejected the last transition, and saving another composition checkpoint also retained reporting. Keep inter-batch merges within the discovery phase and advance to reporting only after the discovery stop decision.
| includeArchivedSessions: true, | ||
| model, | ||
| repository: repo, | ||
| scanDirectory, |
There was a problem hiding this comment.
[P2] Replay saved child worker observations during Deep resume
Completed discovery passes are not executed again on resume, and this historical tracker restores only usage: it receives neither onWorkerEvent nor the shared worker-number callback. The parent tracker now scopes itself to artifacts/deep-scan/merge, so independent completed child roots and their nested workers are outside its session tree. Consequently onWorkerEvent omits those saved workers despite the documented resume contract. With synthetic child-root/child-worker logs and a later merge-root log, the current parent plus historical tracker recovered 220 child tokens but emitted zero worker observations; a child tracker with the observer emitted the expected event. Forward the observer and shared numbering when hydrating saved child sessions.
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Reviewed the full diff and touched production, test, and documentation files at 2bddd36 against 51d3a4d. I am withholding approval for the new test-import path issue described inline and these two existing P2 issues that remain in this revision:
- Sealed publication still requires an executable: when results are sealed but publication is unfinished, native preparation resolves Codex before detecting publication-only recovery. Removing the configured executable prevents publishing those retained results even with no follow-up model work. The terminal shortcut only handles scans already marked complete, failed, or canceled.
- Saved-worker observations on resume: completed discovery passes are absent from
onWorkerEventreplay. Live child events correctly use the shared worker-number callback.
The early reporting phase is P3: it mislabels later discovery, but scheduling uses separate state and I did not establish lost scan work.
The current code fixes the earlier incomplete-batch saturation, cancellation-retry response, original-config restoration, identity-less finding update, migration-test call, and terminal consolidation issues. I am not retaining the repository-write claim: the worker uses the scan/merge output directory and does not add the scanned repository as a writable workspace. Missing Codex-home creation, MCP test dependency lookup, and compressed-rollout accounting are also fixed. The diagnostic truncation limit and loss of per-run inline developer instructions on a fresh CLI resume were already present in the base.
Validation was static; I did not run the test suites.
The unused-helper and duplicate-test cleanup comments are P3 and do not affect the approval decision.
| import { importSource } from "./import-module.ts"; | ||
|
|
||
| const { candidateSchemaV1 } = await importSource( | ||
| new URL("../src/artifact-candidate.ts", import.meta.url).pathname, |
There was a problem hiding this comment.
[P2] Use a filesystem path for the new schema import
URL.pathname retains URL encoding, so a checkout path containing a space becomes a path containing literal %20 here. importSource forwards this string unchanged to esbuild's filesystem entryPoints, and this top-level import then fails before the discovery tests run. The existing import immediately below already uses path.join(import.meta.dirname, ...); use that pattern or fileURLToPath for this new import as well.
| return identity | ||
|
|
||
|
|
||
| def _generic_surface_updates( |
There was a problem hiding this comment.
[P3] Remove the unused legacy merge helpers
This new module retains _generic_surface_updates, _legacy_latest_successful_reducer, _legacy_worker_candidate_key, _legacy_retained_findings, and _merge_tied_parent_observations, but none has a caller in the changed code. The replacement merger imports a different helper subset. These five unused functions add 225 lines of superseded recovery logic; please remove them while keeping the legacy frozen-source helpers that the current merger actually uses.
| expect((await h.checkpoint()).consecutiveErrors).toBe(2); | ||
| }); | ||
|
|
||
| test("a sibling failure aborts an in-flight projection", async () => { |
There was a problem hiding this comment.
[P3] Keep one copy of the lifecycle regression tests
This projection-abort test and the deadline-recovery test below repeat the cases in deep-scan-composition.test.ts at lines 1697 and 1743. Both versions call runDeepScans with fake workbench state and check the same abort reason, before/during-recovery deadlines, stopped child, and partial publication. Please keep one implementation of each case; the second harness adds maintenance without protecting a different behavior.
| @pytest.mark.parametrize("accepted_membership", ["merged", "represented"]) | ||
| @pytest.mark.parametrize("assessment", ["unchanged", "reassessed"]) | ||
| @pytest.mark.parametrize("action", ["cancel-scan", "fail-scan"]) | ||
| def test_native_stop_retains_accepted_and_later_unmerged_findings( |
There was a problem hiding this comment.
[P3] Remove the cancel-only copy of this parameterized test
With action="cancel-scan" and assessment="unchanged", this matrix repeats both accepted_membership cases of test_native_cancel_retains_accepted_and_later_unmerged_findings at line 1556 and retains every assertion from that test. Keep this matrix, including the new failure and reassessment cases, and remove the earlier 88-line function so the same subprocess-heavy cancellation scenarios do not run twice.
| @@ -749,7 +629,7 @@ const legacyScanCases: Array< | |||
| ], | |||
| ]; | |||
| test.each(legacyScanCases)( | |||
| "checks effective worker compatibility for %s scans", | |||
| "SDK-owned passes preserve provider selection for %s scans", | |||
| async (_scenario, mode, overrides, native) => { | |||
There was a problem hiding this comment.
[P3] Remove the obsolete provider-case distinctions
The new harness only reads native.capability and native.isolatedConfig. The explicit OpenAI, conflicting native provider, and same managed provider rows now supply identical effective inputs; their only differences are the unused reads, inherited, managed, and rejects fields. All three therefore copy the plugin and run the same Deep Scan setup and assertions. Keep one explicit OpenAI row and remove those obsolete fields, while retaining the distinct profile and capability cases.
|
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: 9239537e6c
ℹ️ 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".
| record.progress.status === "failed" || | ||
| record.progress.status === "complete" | ||
| ? [ |
There was a problem hiding this comment.
If a Deep Scan is resumed after a child was canceled while the parent was not running, this recovery list ignores that child because it includes only failed and complete statuses. When the canceled child already consumed the final maxDiscoveryRuns slot and no earlier aggregate exists, consecutiveErrors remains zero, so the all-runs-failed guard is skipped and the parent creates and publishes an empty aggregate as a completed capped scan. Treat recovered canceled children as unsuccessful outcomes, or otherwise reject an exhausted composition with no completed pass.
Useful? React with 👍 / 👎.
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Re-reviewed 9239537e6c80 against dd42caa7a22a, including native provider authentication, its API/CLI callers, and the awaited execution factory calls. The inline finding covers three checkpoint tests whose fixed hash order changed. Existing saved-worker replay, sealed-publication prerequisite, and space-path import findings remain supported by unchanged code and prior reproductions.
Fresh builds, types, formatting, portable checks and 11 direct authentication scenarios pass. The three checkpoint cases fail at the confirmed fixture assertion; all 12 provider-auth integration tests stop at the environment’s trusted-parent guard. Prior validation is reused only for verified unchanged inputs; no duplicate inline comments for existing findings.
I also confirmed the recovery logic in the canceled-child thread: a canceled child can exhaust the discovery cap without recording any successful pass or triggering the all-failed guard, permitting an empty aggregate to be published. The narrow recovery-logic probe included failed and completed child controls; the full persisted fixture remains blocked by the existing ownership guard.
| name: createHash("sha256").update(contents).digest("hex") + ".json", | ||
| }; | ||
| }); | ||
| assert.ok( |
There was a problem hiding this comment.
[P2] Keep the checkpoint fixture’s hash ordering valid
These three cases now fail before exercising checkpoint recovery. Removing completeness from semanticCoverage() changes the fixed hashes: the older accepted checkpoint starts ce3af6…, while the newer rejected checkpoint starts 00c677…, so this assertion fails for legacy, indexed, and pending-payload storage. The prior fixture produced the opposite order. Choose fixture contents that deliberately establish the intended adversarial filename order, or derive that order without relying on these exact bytes. With that fixture-only adjustment, all three cases pass against the current implementation.
Summary
Run Deep Scan in batches of Standard scans and persist accepted child work before export, so publication can resume without repeating completed scans. Preserve child usage when receipts or optional tracking are incomplete, and remove replay profiles only when no saved recipe owns them.
Changes
Testing
The final macOS job-budget change passed 97 workflow, shard, and report tests in each seeded and default order, with one Windows-only skip and 535 assertions per run. Actionlint with ShellCheck, zizmor, and workflow formatting passed. The parsed workflow comparison confirms that only the seven macOS primary job allowances change.
On predecessor
0c303b03ec96, the Windows serial baseline completed successfully in 133 minutes 28 seconds, with 5,307 passes, 194 skips, and no failures. Windows parallel also passed. After one retry of the separately classified crash/timeout jobs, macOS shards 1 and 2 and Windows isolated shard 6 passed. MacOS shard 7 completed all 461 cases (449 passes, 12 skips, no failures) in 1,157.96 seconds, but reached its 20-minute outer job limit before cleanup finished. Its complete JUnit identity/outcome multiset matches the prior successful control, and the repository's report comparator passed. This supports the 30-minute macOS allowance; these hosted results apply to the predecessor head. CI run and retry results.The
0c303b03ec96integration passed both focused SDK orders: 219 tests and 1,420 assertions each, using the matching native payload. All five portable plugin checks, five Python plugin-contract tests, SDK types, and formatting passed. Runtime and native source are unchanged by the final workflow-budget correction.Earlier full local SDK evidence remains qualified to
c7cf4be67bc8: the seeded run passed 5,453 tests with 65 skips and no failures. The default-order run remained incomplete after a composed-resume timeout and a later Bun module-loading stall; isolated passing controls do not make that full run green.Risk and rollout
Breaking continuation change: version 3 composition checkpoints replace the old coordinator and version 2 format. Unsealed older scans, and older sealed scans lacking required cost receipts, need their original release or a new scan. Completed reports remain readable. Batches can pass the no-new threshold before it is checked. This overlaps #939 and needs a single integration plan.
Existing feature-review threads, including sealed-publication executable discovery and replay concerns, need separate disposition before merge. The final CI correction changes an outer job allowance, with test commands, partitions, assertions, and per-test limits preserved.
Public disclosure review
New source, fixtures, branch/title/body, commits and publication material were reviewed. Examples and credentials are synthetic; local diagnostic and qualification logs are not attached.
Existing generated review comments contain restricted report links requiring maintainer remediation. The API does not permit editing those comment bodies. Those links were not accessed, and no comments were deleted or threads resolved; the remaining disclosure attestations stay unchecked. Maintainers should correct those existing automation comments before merge.