Repository navigation
refactor: remove unused scan result helpers - #1164
Conversation
|
@codex review |
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". |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The deleted helpers have active replacements or no remaining references, and package expectations were updated consistently.
Review effort: Balanced
Findings: None
What changed in this PR
Removes retired Deep Scan helpers after SDK-backed paths replaced them.
Changes:
- Deletes unused accounting and draft-publication helpers and tests.
- Deletes the obsolete plugin handoff prompt and test.
- Updates packaged SDK file expectations.
| File | Description |
|---|---|
sdk/typescript/src/scan-accounting.ts |
Removes unused accounting helper. |
sdk/typescript/src/scan-draft-publication.ts |
Removes superseded publication implementation. |
sdk/typescript/tests-ts/scan-accounting.test.ts |
Removes obsolete tests. |
sdk/typescript/tests-ts/scan-draft-publication.test.ts |
Removes obsolete tests. |
sdk/typescript/tests-ts/support/scan-publication.ts |
Removes unused test support. |
sdk/typescript/scripts/check-package.mjs |
Removes deleted modules from package expectations. |
plugins/codex-security/mcp-app/src/scan-handoff.ts |
Removes retired handoff prompt builder. |
plugins/codex-security/mcp-app/tests/test_scan_handoff.mjs |
Removes its direct tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
7a4be8e to
cc83a1f
Compare
e3495da to
768c85c
Compare
cc83a1f to
9a5f900
Compare
768c85c to
57ce934
Compare
9a5f900 to
bfa9fd3
Compare
57ce934 to
ef0ec7d
Compare
faizan-oai
left a comment
faizan-oai
left a comment
There was a problem hiding this comment.
Reviewed the unused-helper removal at ef0ec7d and checked the surviving publication/accounting callers. Integrated typechecking and focused SDK checks pass. No new issue found in this cleanup increment; this is not clearance of the upstream stack findings.
ef0ec7d to
ccb5ac7
Compare
0e24db8 to
64ac18f
Compare
ccb5ac7 to
413d90e
Compare
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Three independent passes and root verification found no actionable regression in these removals. The clean SDK build, CI TypeScript build, and plugin build passed. I packed the exact head and verified all 599 package entries, installed public imports and NodeNext types, CLI/SDK lifecycle, credential locking, MCP initialization, and the shared native scan runtime. The first smoke attempt inherited my desktop CODEX_CLI_PATH; the repeat with that override removed passed. No repository files were changed.
413d90e to
42b4e95
Compare
64ac18f to
2472bdc
Compare
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
The previously completed three independent passes carry forward after verifying the patch is identical; root reconciled the current base and discussion.
No serious outstanding finding was identified in this change. The scoped prior review and current diff remain consistent.
Approval is for this change. Integrated-tip validation passed 1,357 SDK tests (31 skipped), 105 MCP tests (2 skipped), portable checks, types/builds and installed-package smoke. Other stack findings and known CI fixture failures still prevent the stack from being mergeable.
42b4e95 to
10ce572
Compare
2472bdc to
01ac366
Compare
There was a problem hiding this comment.
Re-reviewed the real PR diff at 10ce572815a7a45cf4ccfaa44e6b98bef387ab9f. The patch is unchanged from the prior three-pass review. I reconciled the inherited changes in touched files and retained the prior verified findings and tests.
No serious outstanding finding identified in this patch; prior conclusions remain consistent with the unchanged change or the current re-review.
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. Latest workflow runs for this unchanged head were verified successful at 2026-10-02 01:26 UTC. Earlier failed or canceled job records are superseded; this CI status correction does not alter the review decision.
01ac366 to
bf47060
Compare
4a6f0eb to
f30ab3b
Compare
bf47060 to
8f425bc
Compare
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Retained the three independent passes for the identical patch and reconciled inherited touched-file changes at f30ab3b9f076.
No actionable introduced finding survives the independent reviews and root reconciliation of this exact patch, touched-file changes, and current integration checks.
Current tip 355ec321 passed 75 report-projection tests, portable source checks, SDK build:ci and plugin build. The immediately preceding tip fec36f1d passed 1,360 SDK tests, 873 Python tests plus 100 subtests, 109 MCP tests, builds/types and the clean 600-entry installed-package check. Only report_projection.py and its test changed between those tips; SDK/MCP runtime files are byte-identical. Both descend from main 008a8b4d. These are integration checks, separate from own-head probes. No native Windows local run.
No serious outstanding finding introduced by this PR. Code review approval does not establish CI or merge readiness.
8f425bc to
b69fb2b
Compare
f30ab3b to
23fbdf0
Compare
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Retained the three independent passes for the verified identical patch and reconciled inherited touched-file changes at 23fbdf01c362.
No actionable introduced finding survives the independent reviews and root reconciliation of this exact patch and inherited changes in touched files.
Integrated tip 6ef8cbb8, based on 008a8b4d, passed 1,388 SDK tests, 879 Python tests plus 100 subtests, 109 MCP tests, portable checks, builds and the clean 600-entry installed-package check. These are branch-tip integration checks, separate from own-head probes; no local Windows execution was performed.
No serious outstanding finding introduced by this PR. Code approval is separate from CI and merge readiness.
b69fb2b to
3e95db1
Compare
23fbdf0 to
0aa1b39
Compare
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Retained three prior independent passes for the verified identical patch and reconciled inherited touched-file changes at 0aa1b3905284.
No actionable introduced finding survives the independent reviews and root reconciliation of this exact patch and inherited changes in touched files.
Branch tip 3e0083d9, based on 008a8b4d, passed 1,392 SDK tests, 881 Python tests plus 100 subtests, 109 MCP tests, builds, portable source checks and the 600-entry installed-package check. These are tip checks, separate from own-head probes. No local Windows run was performed. The tip excludes the dependency update merged separately in #1184; combined validation remains outstanding.
No serious outstanding finding introduced by this PR. Code approval is separate from CI and merge readiness.
Consolidate approved Deep Scan changes into the parent topic branch.
334b3b2
into
mdangelo/codex/pr939-split-40-plugin-deep-scans
Part 41 of 46. Previous: #1163 · Next: #1165 · Stack index
Summary
Delete result helpers whose SDK and plugin callers have already moved to the shared scan flow.
Changes
Testing
Local checks for the implementation:
Checks against the complete stack passed: SDK and MCP typechecks, formatting, Ruff, the SDK build, portable plugin compatibility and its tests, focused recovery/accounting tests, and local-artifact package smoke using exact cached dependencies. The package check was not a clean registry install. See this PR’s Checks tab for CI results for its current commit.
Risk and rollout
This follows the SDK and plugin switches. It removes unused modules without changing the active execution or checkpoint compatibility policy.
Public disclosure review