Repository navigation
fix(coverage): clarify requested scope and deferred work - #586
mldangelo-oai wants to merge 118 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. |
![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: f7ca16c408
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d384888a9a
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 855e95acd6
ℹ️ 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".
mldangelo-oai
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a377d56f9
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8f2326ac9
ℹ️ 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".
zcrab-oai
left a comment
zcrab-oai
left a comment
There was a problem hiding this comment.
Approved at a8f2326ac90d after independent source review and validation.
[P3] Consideration: valid legacy scans can lose their history coverage summary. The null seal_manifest_digest guard excludes completed legacy records even when their artifacts remain valid. Finding-detail backfill returns before pinning when there are no missing details, including zero-finding scans. Verify supported legacy artifacts with existing seal/binding checks and pin the digest before reading coverage. Guard. A focused real-workbench probe confirmed coverage disappears after setting the legacy null digest while the sealed manifest and completed status remain unchanged.
Validation: Fresh plugin build passed on Node 22.13. 35 SDK tests passed. Local checks used macOS ARM64; platform-specific cases and the full CI matrix were not rerun locally.
CI consideration: 19 checks have not passed (excluding skipped and neutral checks). See the checks.
|
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.
🔵 Needs a closer look
It changes cross-language coverage semantics, artifact verification, reporting, and SARIF behavior across many public surfaces.
0 open findings
What changed in this PR
Separates scan execution completion from requested-scope coverage across the SDK, history, reports, and SARIF.
Changes:
- Adds canonical scope formatting and clearer completion terminology.
- Loads verified coverage from sealed history artifacts.
- Distinguishes blocking work from optional follow-up and updates SARIF success semantics.
| File | Description |
|---|---|
sdk/typescript/tests-ts/workbench-scan-history.test.ts |
Tests scope-aware finding resolution. |
sdk/typescript/tests-ts/scan-recovery.test.ts |
Tests sealed coverage recovery and exports. |
sdk/typescript/tests-ts/scan-history-renderer.test.ts |
Tests execution and coverage rendering. |
sdk/typescript/tests-ts/runtime.test.ts |
Verifies bundled plugin refreshes. |
sdk/typescript/tests-ts/prompt-only-start-timeout.test.ts |
Tests timeouts and large responses. |
sdk/typescript/tests-ts/plugin-finding-detail-contract.test.ts |
Verifies examples and schema descriptions. |
sdk/typescript/tests-ts/coverage-presentation.test.ts |
Tests exact, safe path formatting. |
sdk/typescript/tests-ts/contract.test.ts |
Tests complete scoped coverage contracts. |
sdk/typescript/tests-ts/cli.test.ts |
Tests revised CLI summaries and outcomes. |
sdk/typescript/tests-ts/cli-export.test.ts |
Tests SARIF coverage semantics. |
sdk/typescript/src/scan-history-renderer.ts |
Separates execution, scope, and coverage. |
sdk/typescript/src/models.ts |
Clarifies coverage field semantics. |
sdk/typescript/src/custom-validation-prompt.ts |
Updates workflow integrity hashes. |
sdk/typescript/src/coverage-presentation.ts |
Adds canonical scope presentation helpers. |
sdk/typescript/src/cli.ts |
Reports finished execution and requested coverage. |
sdk/typescript/scripts/package-dist-files.mjs |
Packages the new presentation module. |
sdk/typescript/README.md |
Documents requested-scope completion. |
sdk/typescript/docs/cli.md |
Documents scoped coverage behavior. |
plugins/codex-security/tests/test_workbench_standard_deep_results.py |
Tests stopped-scan coverage and SARIF. |
plugins/codex-security/tests/test_workbench_scan_history.py |
Tests read-only legacy coverage recovery. |
plugins/codex-security/tests/test_workbench_completion_binding.py |
Tests partial report and SARIF output. |
plugins/codex-security/tests/test_report_projection.py |
Tests report sections and path fidelity. |
plugins/codex-security/tests/test_deep_scan_successful_publication.py |
Updates scope-report assertions. |
plugins/codex-security/skills/security-scan/SKILL.md |
Clarifies scoped completion guidance. |
plugins/codex-security/scripts/workbench_scan_history.py |
Reads verified sealed coverage summaries. |
plugins/codex-security/scripts/workbench_db.py |
Attaches coverage to terminal history records. |
plugins/codex-security/scripts/report_projection.py |
Separates blockers from optional questions. |
plugins/codex-security/scripts/finalize_scan_contract.py |
Exports scope and coverage into SARIF. |
plugins/codex-security/schemas/tools/scan-draft.schema.json |
Clarifies draft coverage semantics. |
plugins/codex-security/schemas/coverage.schema.json |
Clarifies canonical coverage semantics. |
plugins/codex-security/references/sarif-adapter.md |
Documents revised SARIF success behavior. |
plugins/codex-security/references/final-report.md |
Documents blocker and follow-up reporting. |
plugins/codex-security/references/core-scan.md |
Defines requested-scope completion rules. |
plugins/codex-security/mcp-app/src/workbench-client.ts |
Allows large workbench responses. |
plugins/codex-security/examples/completed-scan/report.md |
Updates the canonical example report. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
mldangelo-oai
changed the title
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 567c3095cb5f and prior review evidence. Unchanged portions of the large branch were not exhaustively re-audited.
Approved. No new actionable findings in the reviewed scope.
Validation: CI-helper TypeScript compilation and plugin bundling passed locally; native binaries came from CI with matching compiled source inputs. Focused Python tests passed: test_workbench_standard_deep_results.py, test_workbench_completion_binding.py (235 passed in 133.94s (0:02:13)). The full cross-platform suite was not run locally; Windows execution remains unverified here.
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Reviewed the actual diff at 4b1bb00d22f1e52b07fdffeacabaf062469d898d in three independent passes, including the changed behavioral tests and documentation. Completion consistently refers to the requested scope; optional follow-ups remain distinct from blocking work. Legacy sealed-history reads preserve stored artifacts, and the path-rendering changes retain exact path identity while making ambiguous displays visible.
No blocking findings. The SARIF execution-status behavior is an explicit documented change. Static review only; Unicode/platform behavior and the suites were not rerun.
mldangelo-oai
changed the title
Summary
Show requested scan scope clearly in CLI summaries and saved history, and separate deferred requested work from optional report follow-up.
Changes
mainsnapshot while preserving its source-scan execution outcomes, exit codes, schemas, and SARIF success semantics.coverage.deferredseparately from optionalcoverage.openQuestionsin reports. Keep ordinary path quoting and terminal-control escaping without adding a Unicode normalization policy.Testing
Risk and rollout
The change is limited to scope presentation and historical scope retrieval. It adds no commands, flags, accepted values, public environment variables, defaults, migrations, or schema versions.
The older branch's partial/unknown source-completion and unsuccessful-SARIF changes are intentionally removed to retain current
mainbehavior. Deferred work remains visible without changing execution status. History summaries require verified artifacts, but missing unrelated files do not hide already saved findings.Public disclosure review
Reviewed the candidate diff, branch/title/body, all available commit messages and review/conversation comments, and the current published head's six CI log archives, 29 artifacts (including package contents and native payloads), and four CodeQL SARIF reports. No sensitive additions were identified. No attached screenshots or user-uploaded media were found.
The prior description records unavailable historical artifacts and log archives; those unavailable bytes are not represented as reviewed. Fresh CI artifacts must be checked after publication.