Skip to content

fix(cli): allow scan options during interactive bulk discovery - #175

Open
dipeshbabu wants to merge 2 commits into
openai:mainfrom
dipeshbabu:agent/allow-interactive-bulk-options
Open

fix(cli): allow scan options during interactive bulk discovery#175
dipeshbabu wants to merge 2 commits into
openai:mainfrom
dipeshbabu:agent/allow-interactive-bulk-options

Conversation

@dipeshbabu

@dipeshbabu dipeshbabu commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • allow --workers, --mode, --max-attempts, --plugin-path, and --python when starting interactive bulk-scan discovery
  • preserve the raw invocation guard while extending it for current provider, knowledge-base, and scan options
  • retain a focused error for --output-dir, which belongs to CSV-driven bulk scans because the discovery wizard selects its own destination
  • keep globally prefixed interactive invocations blocked so customer-container discovery safeguards remain effective
  • add regression coverage for split and --option=value forms

Root cause

The command schema parsed all bulk-scan options correctly, and the scan runner consumed them after discovery. A second raw-argument guard did not recognize the concurrency, mode, retry, plugin, or Python options, so those valid invocations failed before the wizard started.

Impact

Direct interactive discovery can now use the same concurrency, mode, retry, plugin, and Python settings as CSV-driven bulk scans. CSV behavior is unchanged, and prefixed customer-container invocations remain blocked by the CLI-side safeguard.

Related work

PR #197 includes the original contributor change together with the broader #45 Docker entrypoint update. This branch remains focused on the interactive option gate while preserving its existing container guard.

Validation

  • pnpm dlx bun test --timeout 30000 ./tests-ts/cli.test.ts — 129 passed
  • pnpm exec tsc --noEmit
  • pnpm exec prettier --check src/cli.ts tests-ts/cli.test.ts
  • git diff --check

@mldangelo-oai mldangelo-oai changed the title Allow scan options during bulk discovery fix(cli): allow scan options during interactive bulk discovery Aug 1, 2026
@mldangelo-oai

Copy link
Copy Markdown
Collaborator

@codex review Please review the exact current head a8e97d5, focusing on typed interactive bulk-scan options, CSV output-directory validation, and customer-container discovery safeguards.

@github-actions github-actions Bot added the bug Something isn't working label Aug 1, 2026
@dipeshbabu
dipeshbabu force-pushed the agent/allow-interactive-bulk-options branch from a8e97d5 to 0ba7351 Compare August 1, 2026 13:32

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a8e97d5014

ℹ️ 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".

Comment thread sdk/typescript/src/cli.ts
Comment on lines 1294 to +1295
if (args.input === undefined) {
let optionIndex = 1;
while (optionIndex < argv.length) {
const argument = argv[optionIndex]!;
if (
argument === "--model" ||
argument === "--effort" ||
argument === "--codex"
) {
optionIndex += 2;
} else if (
argument.startsWith("--model=") ||
argument.startsWith("--effort=") ||
argument.startsWith("--codex=")
) {
optionIndex += 1;
} else {
break;
}
}
if (argv[0] !== "bulk-scan" || optionIndex !== argv.length) {
if (options.outputDir !== undefined) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep prefixed bulk scans behind the container guard

When the customer image is invoked with a global Incur option before the command, such as docker run -it … --format toon bulk-scan --workers 8, docker/entrypoint.sh:5-10 misses the command because it only checks whether $1 is bulk-scan. This replacement validation only rejects --output-dir, so the invocation now reaches runBulkScanWizard; the removed argv[0] check previously rejected it. With a gh executable/auth made available to the container, this enables the interactive discovery flow that the entrypoint explicitly prohibits. Either locate bulk-scan after supported global options in the entrypoint or retain an equivalent CLI-side container guard.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: a8e97d5014

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@mldangelo-oai

Copy link
Copy Markdown
Collaborator

Thank you for the interactive bulk-discovery improvement. #197 includes your exact contributor-authored commit and additionally fixes #45 while preserving the container's command/sandbox safeguards. That follow-up is the more complete implementation; this PR remains open so the original contribution and discussion stay visible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working duplicate This issue or pull request already exists

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants