fix(plugin): pin Deep Scan worker executable - #764
Open
soyeon-oai wants to merge 2 commits into
Open
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Keep every worker in a live Deep Scan coordinator on the Codex executable selected for its first worker. Stop safely with retained partial results if that executable changes or disappears.
Changes
Testing
Risk and rollout
The executor now fails closed when the selected executable changes during a scan. Completed work remains available as retained partial results. Existing scans that keep a stable executable are unchanged. CI should provide the Windows coverage for executable identity checks.
Public disclosure review
Change impact
A live Deep Scan reuses one CLI path and stops before launching a worker if that file changes.
flowchart LR subgraph column_0["Scan coordinator"] direction TB node_0["One executor per live scan<br/><code>startOrJoinDeepScanCoordinator</code>"] end subgraph column_1["Executable checks"] direction TB node_1["Pin path and file identity<br/><code>CodexSdkWorkerExecutor</code>"] node_2["Check permissions on pinned CLI<br/><code>preflightDeepScanWorkerPermissionProfile</code>"] node_3["Recheck at worker launch<br/><code>runStreamed</code>"] end subgraph column_2["Worker outcome"] direction TB node_4["Run one CLI or stop safely"] node_5["Keep shutdown coverage isolated<br/><code>deep-scan-worker-shutdown.test.ts</code>"] end node_0 -->|"owns"| node_1 node_1 -->|"supplies path"| node_2 node_2 -->|"then rechecks"| node_3 node_3 -->|"runs or stops"| node_4 node_3 -->|"stays testable"| node_5 class node_0 context class node_1 changed class node_2 affected class node_3 changed class node_4 affected class node_5 changed classDef changed fill:#d7f5e5,stroke:#237a4b,color:#111 classDef affected fill:#e6f0ff,stroke:#3569a8,color:#111 classDef context fill:#f2f3f5,stroke:#6e7781,color:#111Source evidence (6)
plugins/codex-security/mcp-app/server.ts:L658-L673— The scan coordinator receives one CodexSdkWorkerExecutor instance with its scan context.plugins/codex-security/mcp-app/src/deep-scan/executor.ts:L53-L85— The executor caches the first selected executable and gives the same path to permission preflight and the SDK.plugins/codex-security/mcp-app/src/deep-scan/executor.ts:L125-L129— The executor verifies the pinned file identity immediately before runStreamed starts the worker.plugins/codex-security/mcp-app/src/deep-scan/executor.ts:L396-L462— Identity changes or missing files produce a non-retryable error that preserves completed partial-scan work.plugins/codex-security/mcp-app/tests/test_deep_scan_executor.mjs:L540-L654— The tests prove path pinning and fail-closed behavior for replacements between workers and after preflight.sdk/typescript/tests-ts/deep-scan-worker-shutdown.test.ts:L51-L84— The isolated shutdown tests receive stubs for the two new executable-binding dependencies.