feat: share scan settings across the CLI and SDK - #742
Conversation
|
@codex review Please review the current head, |
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. |
![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: e934a27a04
ℹ️ 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".
| (isExternalModelProvider(provider) || provider === "amazon-bedrock") && | ||
| !("model" in result) | ||
| !("model" in result) && | ||
| defaults?.["model"] === undefined |
There was a problem hiding this comment.
When a project file selects a profile and supplies its model only through codex.profiles.<name>.model, using --provider amazon-bedrock, openrouter, or fireworks is rejected here because this checks only the root defaults.model. The existing scanModelConfiguration helper resolves the selected profile's model first, so this is a complete configuration that would otherwise run; use that effective-model logic before enforcing the explicit-model requirement.
AGENTS.md reference: sdk/typescript/AGENTS.md:L16-L20
Useful? React with 👍 / 👎.
| // Complete saved settings do not depend on today's ambient configuration. | ||
| if (!DEEP_SCAN_SETTINGS.every(([name]) => explicit[name] !== undefined)) { |
There was a problem hiding this comment.
When all six deep settings are supplied and the runtime destination aliases the ambient source—such as a supported same-home runtime—this branch skips reading the source and leaves resolved.document empty. writeDeepScanConfig then replaces that same TOML file with only [deep_scan], deleting unrelated user sections that the previous implementation preserved. Retain the source document when the destination may alias it, or merge the snapshot without discarding non-deep settings.
AGENTS.md reference: sdk/typescript/AGENTS.md:L20-L23
Useful? React with 👍 / 👎.
mldangelo-oai
changed the title
|
@codex review Please review the current 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". |
Resolve project files and typed configuration into shared SDK scan settings. Keep native Codex configuration separate from scan options and expose public loadProjectConfig and resolveProjectConfig entry points. Reuse prompt-file handling and severity comparison across SDK and CLI paths. Preserve existing flags, file fields, defaults, output, and input protections. Cover equivalent inputs, prompt-backed workflows, and installed package APIs.
mldangelo-oai
changed the title
|
@codex review Please review the current head, |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e33329e85
ℹ️ 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".
| deepScanConfiguration?.settings, | ||
| options.auth, | ||
| ); | ||
| if (options.scanPrompt !== undefined) recipe["requiresScanPrompt"] = true; |
There was a problem hiding this comment.
When an SDK caller supplies scanPrompt: "" or whitespace, resolveScanPrompts preserves that explicit inline value, but the prompt builder ignores it because additionalPrompt?.trim() is empty. This condition nevertheless records requiresScanPrompt, so a later scans rerun rejects a scan that did not actually use additional instructions. Base the marker on the same nonblank check used when constructing the model prompt.
Useful? React with 👍 / 👎.
Only record additional scan instructions as required when the prompt builder actually includes them. Empty and whitespace-only SDK inputs should remain replayable, matching omitted prompts and empty prompt files. Exercise saved SDK recipes through the CLI rerun path while preserving the requirement to resupply real additional instructions.
|
@codex review Please review the current head, |
|
Codex Review: Didn't find any major issues. Keep it up! 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". |
1 participant
Summary
Reusing scan settings means repeating command-line options or translating them into SDK calls. This adds explicit YAML and JSON files that both the CLI and SDK can load, with shared types and validation.
Changes
scan [repository] -c FILEor--config FILE. The option is unset by default, and the CLI doesn't discover project files. The format has noversionfield.schemas/project-config.schema.jsonfrom the input definition. The existing Ajv engine validates files without fetching schema URLs, adding defaults, coercing values, or dropping unknown wrapper keys.loadProjectConfig()andresolveProjectConfig()helpers that return constructor configuration and scan options. AddScanSettings, SDK prompt-file options, andScanResult.hasFindingsAtOrAbove().Example
Save this as
codex-security.yamlat the root of a repository with asrcdirectory:Check the config with a CLI override:
codex-security scan . -c codex-security.yaml --max-cost 5 --dry-run --json--max-cost 5overrides the file's $10 limit for this invocation.--dry-runchecks local inputs without starting a scan.The SDK can load the same file:
Testing
Validation for this follow-up:
12345: 2,175 passed, 39 skipped, zero failures across 117 files.3828761576: the same 2,175 passed, 39 skipped, and zero failures.Python checks used the repository's isolated pytest setup.
Risk and rollout
Deep preflight now resolves and validates applicable legacy TOML before the runtime starts, so invalid settings can fail earlier. Native profiles keep their existing precedence and restrictions.
Project files and SDK options support
stopAfterConsecutiveErrors; this adds no CLI flag for it. Cost limits still apply per scan attempt. Project files cannot select a repository or turn on automatic patching or publication.Existing CLI flags, file fields, defaults, and output remain compatible. SDK inline prompts take precedence over matching files. The SDK records the severity policy; callers decide how to handle
hasFindingsAtOrAbove()results.New recipes save resolved settings without copying source or context contents. Reruns don't reload the project file. Older partial recipes still use legacy defaults for missing settings. If a new recipe required additional scan instructions, reruns refuse to omit them silently.
Local testing covered macOS arm64. It did not include live inference, credential verification, releases, or deployments. Linux and Windows coverage depends on CI.
Public disclosure review