Repository navigation
fix(deep-scan): wait for the coordinator before ending CLI scans #2007
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
98c3581
0780e59
d3868d1
c6c1d7b
e6a6585
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2856,6 +2856,9 @@ export class CodexSecurity { | |
| runtimePaths, | ||
| options.auth, | ||
| git, | ||
| mode === "deep" && !directDeepScan | ||
| ? deepScanParentConfig(session.sessionConfig) | ||
| : undefined, | ||
| ); | ||
| const threadOptions: ThreadOptions = { | ||
| threadSource: CODEX_SECURITY_THREAD_SOURCES.scan, | ||
|
|
@@ -5810,6 +5813,32 @@ export function classifyConnectionFailure( | |
| return "unknown"; | ||
| } | ||
|
|
||
| function deepScanParentConfig(config: JsonObject): JsonObject { | ||
| const resolved = resolveCodexProfile(config); | ||
| const features = resolved["features"] ?? {}; | ||
| if (!isRecord(features)) return resolved; | ||
| const mode = features["code_mode"]; | ||
| const settings = typeof mode === "boolean" ? { enabled: mode } : (mode ?? {}); | ||
| // Leave malformed user settings for the native config parser to reject. | ||
| if (!isRecord(settings)) return resolved; | ||
| const direct = settings["direct_only_tool_namespaces"] ?? []; | ||
| if (!Array.isArray(direct)) return resolved; | ||
| // The native parent waits for direct tool calls before it can end its turn. | ||
| // A code-mode cell can yield the still-running coordinator to the model. | ||
| return { | ||
| ...resolved, | ||
| features: { | ||
| ...features, | ||
| code_mode: { | ||
| ...settings, | ||
| direct_only_tool_namespaces: [ | ||
| ...new Set([...direct, "mcp__codex_security", "codex_security"]), | ||
|
Comment on lines
+5834
to
+5835
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When another configured or enterprise-managed MCP server normalizes to AGENTS.md reference: AGENTS.md:L14-L19 Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reproduced on the merged code. With two other servers advertising the same tool name, the original coordinator was available only inside a yielding cell. In #2012, the native server-name selector keeps the genuine coordinator direct when a free name is available. The installed CLI completed the original scan in one turn, and neither other server was called. The follow-up is still in draft; an empty peer allowlist and alternate plugin declarations have separate regression coverage in progress.
Comment on lines
+5834
to
+5835
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When AGENTS.md reference: AGENTS.md:L14-L19 Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed with an installed CLI and a machine-wide native tool setting: the merged code removed that existing direct tool from the parent menu. #2012 reads the effective native settings with the scan’s ordered overrides, then extends the existing list. The machine-wide tool and the coordinator were both direct in the native comparison; explicit cell-only policy was preserved on both revisions. The fix is in the follow-up draft. |
||
| ], | ||
| }, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| export function scanRuntimeCodexConfig( | ||
| config: JsonObject, | ||
| protectedCredentialHome?: string, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a parent-driven Deep Scan with
postScanPrompt, this configuration is attached to the thread for its entire lifetime, so the later follow-up atrunPostScanalso sees Codex Security tools as direct-only instead of using the caller's original code-mode settings. That needlessly changes routing and exposes the plugin's full direct schemas during caller-supplied post-scan work; run the follow-up with the original configuration or otherwise limit this override to the coordinator turn.AGENTS.md reference: AGENTS.md:L14-L19
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed that the coordinator-only setting leaked into the post-scan turn. #2012 resumes the original thread with the caller’s original client for a requested post-scan prompt. In the installed comparison, that prompt could retrieve the sealed scan with the caller’s tools. The serial controls passed; two lookup failures in the first concurrent custom-plugin batch remain unexplained. The follow-up is still under review.