Skip to content

refactor(imports): separate report readers from source providers - #1871

Merged
mldangelo-oai merged 27 commits into
mainfrom
mdangelo/codex/import-reader-routing
Oct 23, 2026
Merged

mldangelo-oai merged 27 commits into
mainfrom
mdangelo/codex/import-reader-routing

Conversation

@mldangelo-oai

@mldangelo-oai mldangelo-oai commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

File imports currently use the same provider value to select a report reader and identify stored evidence. Separate those roles so a compatible reader can reuse an existing storage and Cloud provider without changing the normalized import APIs.

Changes

  • Derive CLI reader choices and SDK reader types from one reader-to-provider map.
  • Apply that mapping at the local and Cloud file-import boundaries, preserving Wiz's historical retry projections and Semgrep repository checks.
  • Strengthen existing tests for preview and persisted source identities. Existing readers, CLI values, and output behavior are unchanged.

Testing

Fresh checks on 40ef060071a7:

  • Focused reader-identity and local/Cloud evidence-boundary regressions: 25 passed across four test files.
  • SDK build, plugin build, SDK/MCP types and generated-schema checks, full SDK formatting, and git diff --check passed.
  • All five portable plugin checks passed in order: Ruff check, Ruff format, SDK build:ci, source compatibility, and its nine regression tests.
  • Installed SDK/MCP dependency locks match the tracked locks. Native artifacts were reused only after comparing all 28 tracked native source inputs.
  • Independent review found no unintended changes in the seven-file import refactor.

This head incorporates the reviewed checkpoint-retention repair from #1710 and current main; #1710 must merge first.

The full seeded package run at ba8e23e92028 completed with 6,912 passed, 83 skipped, 41 failed, and one error across 233 files (seed 12345). Its failures were qualified with unchanged-test controls:

  • Forty validation-cache cases inherited native configuration from an ancestor of the test temporary directory, intentionally disabling caching. All 40 passed in a clean runtime temporary directory. The same isolated case failed with the inherited configuration and passed without it on both this branch and the reviewed baseline.
  • One export test exceeded its overall 30-second deadline during host contention. It passed all 21 assertions in 11.18 seconds at lower load. The additional error was that same test resuming after its child was terminated; a disposable reproduction confirmed the identical late assertion.
  • The clean cache cohort initially had two fixture-permission failures from umask 0002; all six related storage cases passed with the package run's 0022 umask. No product code, test assertions, or timeouts changed.

The first default-order attempt was stopped after 946 passing tests because it used the same configuration-inheriting temporary location. The full default-order package run on 40ef060071a7 is now running with the verified clean temporary environment. No complete full-package pass is claimed yet; current hosted CI and the final default-order result remain merge gates.

Earlier focused checks at 3713481feea8 passed 320 importer/publication tests with one Windows-only skip, plus all 23 local evidence-size/Cloud-limit cases.

Risk and rollout

This is a prerequisite refactor for an additional compatible report reader. It changes no wire schema, storage identity, public CLI value, or retry format. No service rollout is required.

Public disclosure review

  • No customer, partner, prospect, or user identities, data, or identifying details are included.
  • No credentials, personal data, private source, scan findings, or nonpublic links or tickets are included.
  • I reviewed the branch name, title, description, commits, changes, comments, logs, screenshots, attachments, and links for public disclosure.

@alandelong-oai alandelong-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for separating reader selection from source identity. No material findings across the seven-file change and its local/Cloud/CLI callers. Exact dependency installs, builds, types, formatting and 18 built-SDK/CLI reader-mapping controls passed. The broader reader run finished with 381 passing and 57 failing cases under existing host directory restrictions; it is not a full integration-suite pass. Current hosted CI is still pending.

@mldangelo-oai
mldangelo-oai marked this pull request as ready for review October 23, 2026 06:21
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-23T18:57:07.784629Z 40ef060 New commits
🔒 Security Review ✅ Completed 2026-10-23T18:57:16.458695Z 40ef060 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Comment thread plugins/codex-security/tests/test_deep_scan_deadline_recovery.py

@alandelong-oai alandelong-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for separating report-reader selection from stored source identity. This owner-level merge-safety rereview found no blocking issues in the complete 21-file diff, including the updated checkpoint recovery integration. Existing reader choices, local/Cloud identities, Wiz retry projections, and Semgrep repository checks remain intact.

Fresh builds, types, formatting, all five portable checks, and 18 built-SDK/CLI reader controls passed. Shared recovery tests and rollback/replay controls were reused after verifying identical source, schemas, tests, native inputs, and Python dependencies. The broader local persistence suite remains limited by filesystem restrictions; previous hosted success is historical, and full current-head hosted qualification is still pending. Merge #1710 first as documented.

@mldangelo-oai mldangelo-oai reopened this Oct 23, 2026
@mldangelo-oai
mldangelo-oai merged commit b3448fc into main Oct 23, 2026
215 of 217 checks passed
@mldangelo-oai
mldangelo-oai deleted the mdangelo/codex/import-reader-routing branch October 23, 2026 20:02
@github-actions github-actions Bot mentioned this pull request Oct 23, 2026
3 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants