Repository navigation
refactor: delete the retired worker result protocol - #1169
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. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
Retired protocol code and its consumers are consistently removed while active artifact contracts remain covered.
Review effort: Balanced
Findings: None
What changed in this PR
Removes the retired Deep Scan worker artifact protocol while preserving the active scan artifact workflow.
Changes:
- Deletes legacy worker/reducer tools, schemas, templates, evaluations, and tests.
- Simplifies artifact contexts, inventory access, and atomic replacement.
- Removes obsolete runtime dependencies and updates packaging/tests.
| File | Description |
|---|---|
CONTRIBUTING.md |
Removes obsolete reducer CI reference. |
evals/README.md |
Removes retired reducer evaluation. |
evals/deep-reducer/.gitignore |
Deletes obsolete report ignore. |
evals/deep-reducer/README.md |
Deletes retired evaluation documentation. |
evals/deep-reducer/run.mjs |
Deletes retired evaluation runner. |
plugins/codex-security/plugin-files.json |
Removes retired schemas from packaging. |
plugins/codex-security/schemas/tools/deep-reducer.schema.json |
Deletes reducer schema. |
plugins/codex-security/schemas/tools/review-items.schema.json |
Removes worker-only input schema. |
plugins/codex-security/schemas/tools/worker-threat-model.schema.json |
Deletes worker threat-model schema. |
plugins/codex-security/tests/test_scan_contract_examples.py |
Retains Standard draft contract coverage. |
plugins/codex-security/mcp-app/artifact-writer-main.ts |
Deletes legacy worker MCP entrypoint. |
plugins/codex-security/mcp-app/package.json |
Removes SDK runtime dependency. |
plugins/codex-security/mcp-app/pnpm-lock.yaml |
Updates locked dependency graph. |
plugins/codex-security/mcp-app/tsconfig.json |
Removes deleted entrypoint. |
plugins/codex-security/mcp-app/src/artifact-context.ts |
Removes worker/reducer context fields. |
plugins/codex-security/mcp-app/src/artifact-deep-reducer-pages.ts |
Deletes reducer paging implementation. |
plugins/codex-security/mcp-app/src/artifact-deep-reducer.ts |
Deletes reducer artifact operations. |
plugins/codex-security/mcp-app/src/artifact-inventory.ts |
Removes worker inventory contract. |
plugins/codex-security/mcp-app/src/artifact-io.ts |
Removes append/locking support. |
plugins/codex-security/mcp-app/src/artifact-scan-draft.ts |
Removes legacy re-exports. |
plugins/codex-security/mcp-app/src/artifact-storage.ts |
Removes obsolete layout metadata. |
plugins/codex-security/mcp-app/src/artifact-threat-model.ts |
Deletes worker threat-model writer. |
plugins/codex-security/mcp-app/src/artifact-worker-scan-draft.ts |
Deletes legacy worker draft implementation. |
plugins/codex-security/mcp-app/src/deep-scan/artifact-contracts.ts |
Deletes obsolete artifact contracts. |
plugins/codex-security/mcp-app/src/deep-scan/artifact-validation.ts |
Deletes legacy result validation. |
plugins/codex-security/mcp-app/src/deep-scan/artifacts.ts |
Deletes legacy artifact layout helpers. |
plugins/codex-security/mcp-app/src/deep-scan/templates.ts |
Deletes worker prompt rendering. |
plugins/codex-security/mcp-app/src/server/compact-artifact-tools.ts |
Removes worker/reducer tool registration. |
plugins/codex-security/mcp-app/templates/deep-scan/dedup.md |
Deletes reducer prompt. |
plugins/codex-security/mcp-app/templates/deep-scan/discovery.md |
Deletes discovery-worker prompt. |
plugins/codex-security/mcp-app/tests/test_artifact_deep_reducer.mjs |
Deletes reducer tests. |
plugins/codex-security/mcp-app/tests/test_artifact_deep_reducer_pages.mjs |
Deletes paging tests. |
plugins/codex-security/mcp-app/tests/test_artifact_foundation.mjs |
Updates active artifact I/O coverage. |
plugins/codex-security/mcp-app/tests/test_artifact_inventory.mjs |
Replaces worker cases with scan isolation. |
plugins/codex-security/mcp-app/tests/test_artifact_storage_regressions.mjs |
Aligns contexts and bundling. |
plugins/codex-security/mcp-app/tests/test_artifact_worker_threat_model.mjs |
Deletes retired writer tests. |
plugins/codex-security/mcp-app/tests/test_deep_reducer_paging_eval.mjs |
Deletes paging regression evaluation. |
plugins/codex-security/mcp-app/tests/test_deep_scan_artifact_validation.mjs |
Deletes legacy validation tests. |
plugins/codex-security/mcp-app/tests/test_deep_scan_templates.mjs |
Deletes retired template tests. |
plugins/codex-security/mcp-app/tests/support/reducer-paging/controlled-code-mode.mjs |
Deletes evaluation harness. |
plugins/codex-security/mcp-app/tests/support/reducer-paging/deep-reducer-paging-fixture.mjs |
Deletes paging fixture. |
plugins/codex-security/mcp-app/tests/support/reducer-paging/deep-reducer-paging-server.mjs |
Deletes evaluation server. |
plugins/codex-security/mcp-app/tests/support/reducer-paging/deep-reducer-paging.mjs |
Deletes reducer evaluator. |
Files not reviewed (1)
- plugins/codex-security/mcp-app/pnpm-lock.yaml: Generated file
💡 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. Nice work! 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". |
46213af to
d66eaca
Compare
65fee77 to
f6195ff
Compare
d66eaca to
b49ed19
Compare
mldangelo-oai
changed the title
b49ed19 to
835f0d1
Compare
f6195ff to
50fcfff
Compare
faizan-oai
left a comment
faizan-oai
left a comment
There was a problem hiding this comment.
Reviewed the retired result-protocol removal at 835f0d1, including the remaining artifact writer. The integrated plugin builds; SDK/MCP typechecking, compact artifact server checks, and tracked-source compatibility checks pass. No new issue found in this cleanup increment. This approval does not clear the active runtime/recovery issues tracked earlier in the stack.
835f0d1 to
f888263
Compare
50fcfff to
cdeea3c
Compare
f888263 to
79296c3
Compare
cdeea3c to
18695d3
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 the old worker protocol removal and retained artifact operations. All 27 focused artifact checks passed after rerunning the compact-server test against a stable bundle; its first attempt overlapped my local rebuild and is not a code finding. All 20 schema tests (44 subtests) passed.
Final-head validation also passed: clean SDK/plugin builds, MCP types, Ruff check and format with required 0.16.8, CI TypeScript build, source compatibility and all 9 compatibility tests. The exact-head npm archive passed all 596 entry checks plus installed public imports, NodeNext types, CLI/SDK lifecycle, credential locking, MCP initialization, and shared scan runtime smoke tests. No repository files were changed.
79296c3 to
f4b3eab
Compare
18695d3 to
783458b
Compare
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Three independent re-review passes and root reconciliation are complete for this restacked head.
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.
783458b to
43a2b25
Compare
f4b3eab to
06e75cf
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 06e75cfed4bd36b520ff24322acdcf57befc5946. 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.
06e75cf to
fec36f1
Compare
182b45d to
be950cf
Compare
fec36f1 to
355ec32
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 355ec321d0f4.
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.
355ec32 to
6ef8cbb
Compare
be950cf to
1cf170f
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 6ef8cbb81683.
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.
1cf170f to
f0ed597
Compare
6ef8cbb to
3e0083d
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 3e0083d9d4bf.
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.
d519676
into
mdangelo/codex/pr939-split-45-retired-python-engine
Part 46 of 46. Previous: #1168 · Stack index
Summary
Remove the artifact tools and packaging used by the retired Deep Scan worker and reducer runtime.
Changes
Testing
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
The worker entrypoint was already disconnected by the plugin switch. Active artifact writes also drop the per-file lock while retaining temporary-file-and-rename replacement; unused append helpers are removed.
Public disclosure review