Repository navigation
refactor(mcp): derive Deep Scan contracts from existing schemas - #1584
mldangelo-oai wants to merge 20 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. |
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. 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.
🔵 Needs a closer look
The validation-sensitive contract refactor is broad and explicitly requires #1577 to be reviewed and merged first.
0 open findings
What changed in this PR
Consolidates MCP and Deep Scan contracts around existing Zod schemas and producer types while preserving validation, persistence, diagnostics, and cancellation behavior. This PR depends on #1577.
Changes:
- Derives progress, persistence, worker, artifact, and candidate types from authoritative schemas.
- Centralizes response parsing and retry policy.
- Forwards MCP cancellation through the SDK callback context.
| File | Description |
|---|---|
sdk/typescript/src/worker-progress.ts |
Derives progress contracts from Zod schemas. |
sdk/typescript/src/deep-progress.ts |
Consolidates Deep Scan progress validation. |
plugins/codex-security/mcp-app/tests/test_deep_scan_store.ts |
Expands persistence normalization tests. |
plugins/codex-security/mcp-app/src/server/compact-artifact-tools.ts |
Uses typed MCP callback cancellation. |
plugins/codex-security/mcp-app/src/helpers/normalize-candidates.ts |
Derives candidate fields and types. |
plugins/codex-security/mcp-app/src/deep-scan/worker-runner.ts |
Reuses failure and cancellation outcomes. |
plugins/codex-security/mcp-app/src/deep-scan/types.ts |
Derives durable state types from schemas. |
plugins/codex-security/mcp-app/src/deep-scan/store.ts |
Centralizes parsing and retry selection. |
plugins/codex-security/mcp-app/src/deep-scan/response.ts |
Adds schema-based response normalization. |
plugins/codex-security/mcp-app/src/deep-scan/registry.ts |
Reuses coordinator constructor input type. |
plugins/codex-security/mcp-app/src/deep-scan/artifacts.ts |
Infers artifact layout type. |
plugins/codex-security/mcp-app/src/artifact-io.ts |
Uses native Zod row results and errors. |
plugins/codex-security/mcp-app/server.ts |
Shares equivalent schema primitives. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! 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". |
…-finding-normalization
…elo/codex/shared-workbench-messages
…/codex/deep-scan-remote-validation # Conflicts: # plugins/codex-security/mcp-app/src/deep-scan/store.ts
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! 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/shared-finding-normalization
…elo/codex/shared-workbench-messages
…/codex/deep-scan-remote-validation
|
@codex review |
|
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". |
mldangelo-oai
changed the title
zcrab-oai
left a comment
zcrab-oai
left a comment
There was a problem hiding this comment.
Reviewed bf93c30c2a90 in two source-review passes, including relevant callers and contracts.
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 MCP tests passed: test_deep_scan_store.ts, test_deep_scan_store_integration.ts. 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 63bb0d2e271e327ba002dd9a397fedb4eaa38814 in three independent passes, including the changed tests and documentation. The shared argument builder retains repeated flags, ordering, empty strings, and zero values; the response schemas retain the existing optional-field defaults and validation behavior. Worker cancellation, serialized writes, and terminal publication still follow the existing lifecycle.
No blocking findings. Static review only; I did not rerun the suites or generators.
Summary
Derive repeated MCP and Deep Scan fields from their existing schemas and constructors. Merge the reviewed
mainsnapshot; the prerequisite workbench refactor is already merged.Changes
Testing
Risk and rollout
No CLI surface, persisted-format, accepted-status, runtime-setting, default, or diagnostic-policy changes. No migration is required. Public SDK progress interfaces remain augmentable. Registered MCP callbacks retain their request context and cancellation signal.
Public disclosure review