feat: support model-native Deep Scan workers - #270
Conversation
|
@codex review Please review the exact current head |
![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: 205fb5d2c1
ℹ️ 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".
| Some remediation patches have `kind = "host_setting"`. Present those as host-level setup guidance, not as edits to persistent Codex config. | ||
|
|
||
| Deep Security Scan uses MCP-owned SDK sessions rather than the parent thread's worker pool. Its preflight requires native V2 but does not derive outer worker concurrency from the parent session cap or require parent delegation depth. When migrating a legacy V1 configuration for Deep Security Scan, remediation removes `agents.max_threads`, sets `features.multi_agent_v2.enabled = true`, and sets `features.multi_agent_v2.max_concurrent_threads_per_session = 4`. The coordinator overrides each child session's cap from the per-user deep-scan configuration. Codex rejects the legacy V1 thread setting and explicit V2 mode together. | ||
| Deep Security Scan uses MCP-owned SDK sessions rather than the parent thread's worker pool. Its preflight does not require a particular parent delegation runtime, ownership, capacity, or depth. Discovery workers inherit the scan's model and run under the verified read-only worker sandbox. |
There was a problem hiding this comment.
When the active CODEX_HOME contains legacy agents.max_threads, this profile now returns ready, but the bundled CodexSdkWorkerExecutor.run still inherits that environment and explicitly passes features.multi_agent_v2.enabled=true to every discovery child. Codex rejects that V1/V2 combination, as the unchanged preflight documentation itself notes, so the newly admitted V1 configuration fails only after start_codex_security_deep_scan launches. Keep the config_absent check/remediation, or isolate the child configuration before declaring this parent setup compatible.
Useful? React with 👍 / 👎.
| - `$codex-security:attack-path-analysis` | ||
|
|
||
| The discovery tool launches Codex workers that may use Subagents v2. The active configuration must satisfy the deep profile's native-v2 requirement. The worker count is configured separately from this thread's subagent allowance. | ||
| The discovery tool manages its own workers independently of this thread's delegation runtime and subagent allowance. |
There was a problem hiding this comment.
Worker-pool independence does not remove the parent workflow's phase-skill dependencies: this same section still requires security-scan, threat-model, finding-discovery, validation, and attack-path-analysis, and the parent invokes them after discovery. In a runtime that exposes the Deep Scan entry skill but omits any of those selectively loaded skills, removing deep_scan_phase_skills lets preflight report ready; the expensive discovery can then finish before the mandatory centralized tail becomes impossible. Restore this skill-availability requirement independently of the delegation-runtime checks.
Useful? React with 👍 / 👎.
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…/codex/model-native-deep-workers
|
@codex review Please review the exact updated head |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! 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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…/codex/model-native-deep-workers # Conflicts: # sdk/typescript/_bundled_plugin/mcp/server.mjs.br.part-000 # sdk/typescript/_bundled_plugin/mcp/server.mjs.br.part-001
|
@codex review Please review the exact updated head |
|
Codex Review: Didn't find any major issues. 🚀 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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…/codex/model-native-deep-workers # Conflicts: # sdk/typescript/_bundled_plugin/mcp/server.mjs.br.part-000 # sdk/typescript/_bundled_plugin/mcp/server.mjs.br.part-001
|
@codex review Please review the exact updated head |
|
Codex Review: Didn't find any major issues. Hooray! 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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…/codex/model-native-deep-workers
|
@codex review Please review the exact updated head |
|
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
1 participant
Summary
Stack
mdangelo/codex/preserve-url-scan-contextso this review contains only its atomic upstream change.Verification