refactor!: simplify saved scans and remove retired recovery - #1092
mldangelo-oai merged 10 commits into
Conversation
mldangelo-oai
changed the title
|
@codex review Please review the current cleanup head, including sealed-result completion, historical registration, and the documented compatibility removals. |
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: Didn't find any major issues. Chef's kiss. 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". |
|
@codex review Please review the updated cleanup head |
![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: f5737c43ae
ℹ️ 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".
| migrate_finding_workflow_columns(connection) | ||
| elif version == 39: | ||
| migrate_finding_workflow_review_columns(connection) |
There was a problem hiding this comment.
When upgrading a version-42 database containing Deep Scan pass records, migration 43 adds parent_scan_role as nullable but this path no longer marks the existing children as deep_pass. Those rows are then omitted by composition_children() in workbench_composition.py, so saved-result recovery, child cancellation, and usage accounting can ignore already-created passes and lose their findings or costs. Restore the migration-time backfill before recording version 43 as applied.
Useful? React with 👍 / 👎.
e367e17
into
mdangelo/codex/simplify-deep-scan
1 participant
Summary
Stacked on #939. Remove retired resume/recovery compatibility, duplicated publication work, and redundant test setup: 3,183 net lines removed across 83 files. SDK sealed results can be finalized from their saved state without preparing Codex authentication or a model session.
Changes
Remove active v1 conversion and
get-cli-scan-resume --migrate, unsealed legacy worker-fragment salvage, obsolete draft writers, and an unshipped migration backfill. Keep ordinary stopped-result recovery, sealed historical reports, source-integrity checks, and append-only migration DDL.Share saved accounting, coverage/provenance helpers, publication fixtures, configuration copying, timeout selection, and build setup. Read authoritative completion metadata after fresh native success instead of completing twice.
Require explicit provenance, simplify terminal-resume rejection, and replace the broad composition test matrix with focused boundary scenarios. Keep fresh/resumed child-process auth, environment, executable, and permission checks.
Remove npm-layout executable discovery and unused dependencies; bundle one Zod runtime. Native executable selection uses
CODEX_CLI_PATHor an executable onPATH.Share report metadata and validation-list rendering without changing Markdown output; use a pure finding builder instead of writing unused contract fixtures.
Update stopped-result fixtures for canonical parent checkpoints and normalize the Windows protocol fixture executable. Integrate the current base and preserve historical cost attribution through the shared saved-result reader.
This implements the maintenance portion of the audit. Broader scheduler, reducer, and evidence-storage redesigns remain outside this PR. The smaller integration matrix deliberately covers fewer unusual failure combinations.
Testing
Passed on the updated source:
build:ci.rgand date-time validation dependencies; all 159 tests in the affected files passed after correcting the isolated test tools. Every Python case is covered across these runs.The prior complete SDK runs at
241becbwere not fully green locally: four tool/user-environment cases passed corrected reruns, and two sandbox integration cases require nested user namespaces unavailable in the local container. No ownership or permission protections were relaxed. GitHub CI supplies the platform coverage, including the corrected Windows protocol fixture.Risk and rollout
This is a breaking compatibility cleanup. Interrupted v1 scans and unsealed v1 worker fragments require the prior release; completed historical reports remain readable. Implicit npm-managed executable layouts are no longer discovered: configure
CODEX_CLI_PATHor expose the actual executable onPATH. Databases created by unreleased builds that inferred child membership from artifact paths are not backfilled; start fresh scans for those snapshots. Schema migrations remain append-only. Credential, unsafe-path, target/claim, artifact-seal, and per-scan isolation protections remain.Public disclosure review