Skip to content

fix: limit finding comparison input size - #638

Open
ianw-oai wants to merge 1 commit into
mainfrom
dev/ianw/batch-finding-comparisons
Open

fix: limit finding comparison input size#638
ianw-oai wants to merge 1 commit into
mainfrom
dev/ianw/batch-finding-comparisons

Conversation

@ianw-oai

Copy link
Copy Markdown
Collaborator

Summary

Large finding comparisons can exceed the Codex input limit and leave matching incomplete. Each comparison included all earlier findings in one request. Split large comparisons into smaller requests so the total finding count does not determine the size of a single request.

Changes

  • Compare every earlier/later finding pair in independent batches.
  • Join confirmed groups across batches in code. Keep the existing rules for uncertain pairs.
  • Keep full finding text. Report an error if one pair exceeds the Codex input limit.
  • Add regression tests for large inputs, cross-batch groups, uncertainty, cancellation, and input limits.
  • Clarify that component retries create a separate report.

Testing

  • Full suite in two orders: 1,636 passed, 30 skipped, no failures in each run. Used a 90-second per-test timeout.
    • pnpm run test --seed 12345 --timeout 90000
    • pnpm run test --timeout 90000
  • pnpm run types, pnpm run format, and pnpm run build passed.
  • 89 focused tests and 100 synthetic group-merging checks passed.

Risk and rollout

No CLI options, result formats, or dependencies change. Large comparisons make more model requests. Small comparisons still use one request. Component retry behavior is unchanged. This change does not address process shutdown after a completed summary.

Public disclosure review

  • No customer, partner, prospect, or user identities, data, or identifying details are included.
  • No credentials, personal data, private source, scan findings, or nonpublic links or tickets are included.
  • I reviewed the branch name, title, description, commits, changes, comments, logs, screenshots, attachments, and links for public disclosure.

@github-actions github-actions Bot added the bug Something isn't working label Aug 24, 2026
@ianw-oai
ianw-oai marked this pull request as ready for review August 24, 2026 22:54
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
🔒 Security Review Completed 2026-08-24T22:57:35.379113Z 596a73a Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@mldangelo-oai mldangelo-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed current head 596a73a. The batching and group-merging checks pass, but the shared matcher now starts uncapped post-completion model calls for cost-limited ordinary scans; see the inline comment.

const outputSchema = z.toJSONSchema(comparisonSchema, {
target: "openapi-3.0",
});
for (const batch of comparisonBatches(input)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P1] Keep batched post-scan matching inside the requested cost bound

matchScanFindingsInternal is also used by ordinary CodexSecurity.run() history matching, not just component scans. That path calls matchCompletedScan only after tracker.stop(...) and complete-scan have already persisted the final cost, and it does not pass maxCostUsd or any remaining budget into this loop. With four synthetic 300 KB historical findings and one 5 KB new finding, I reproduced four additional model turns after completion; the README currently allows only one extra call when a cost limit is set. Larger histories can therefore generate many untracked, uncapped turns despite --max-cost, without appearing in the recorded scan cost. Please either keep batching specific to explicitly unbounded component matching or propagate and enforce the remaining scan budget before starting subsequent batches, and update the single-call documentation accordingly.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants