perf(workbench): reuse scan matching inputs - #153
Conversation
`scans match --all` aborted the whole campaign when one batch produced conflicting confirmed and uncertain findings. The throw fires while building `comparisons` for a batch, which is after every earlier batch has already called `save-scan-comparison`, so the run exited nonzero with partially persisted match state and no way to resume. Contain each batch instead. A batch that cannot be matched is counted, warned about, and skipped; the remaining batches still save. When nothing matched at all the first failure is rethrown, so an unwritable workbench or a rejected credential still surfaces as an error rather than a warning attached to a successful no-op. The conflict check itself is unchanged: refusing to persist conflicting confirmed and uncertain findings is deliberate, and its existing test passes unmodified.
runWorkbench only checks that the workbench response parsed as a JSON object, so nothing validates the shape between the Python workbench and the history renderer. The renderer then cast fields unconditionally, so a plugin or database that drifted from the expected shape surfaced as an unhandled TypeError with a stack trace instead of a usable view. Confirmed crashes: a finding whose `severity` is absent or null threw on `severity.level`; a `scans` row without `progress` threw on `progress.status`; a `knownSince` that is not a parsable timestamp threw RangeError out of Intl.DateTimeFormat. Read every field through small optional accessors instead. Missing data now renders as empty or `UNKNOWN`, an unparsable `knownSince` falls back to its raw value, and an unrecognized severity sorts below the known levels rather than above CRITICAL, where indexOf's -1 had placed it. Output for well-formed payloads is unchanged; the existing renderer tests assert exact text and pass unmodified.
`severity.padEnd(8)` is a no-op for INFORMATIONAL, which is thirteen
characters, so informational findings pushed their title five columns to
the right of every other severity and broke the table:
HIGH Short severity row
INFORMATIONAL Long severity row
The badge width and the two indents that line up beneath it were three
independent literals (8, 14, 14), so they only agreed while every label
happened to fit in eight columns.
Derive all of them from the label set and abbreviate INFORMATIONAL to
INFO. Both values are unchanged at 8 and 14, so only the informational
label differs; spelling the word out instead would cost every finding
title five characters, which matters most at the 48-column minimum.
![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: 1ac5da8227
ℹ️ 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".
informational is a valid findings severity and is exported to SARIF/CSV, but report.md filtered it out via REPORTABLE_SEVERITIES, so all- informational scans rendered as "No findings". Include informational in the reportable set and cover it with a projection unit test. Fixes #48
report.md filters findings to REPORTABLE_SEVERITIES, which excludes informational. A scan whose findings are all informational therefore sealed them into findings.json and exported them to SARIF and CSV while the human-facing report stated that no findings survived the discovery, validation and reportability gates. The three artifacts of one scan contradicted each other, and the one a person reads was the one missing data. The exclusion itself is deliberate -- src/cli.ts draws the same line for --fail-on-severity and keeps a separate DISPLAY_SEVERITIES that adds informational -- so the report keeps its reportable-only body. It now records how many findings were held back, with their severity mix, and points at the artifacts that retain them. references/final-report.md described the ordering as critical through low without stating that informational is dropped, so it now says so. Fixes #48
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d6fa32fcf
ℹ️ 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".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4d0ee3e2b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 313768a97b
ℹ️ 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".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 142cc8bb6f
ℹ️ 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".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1bf38a62a
ℹ️ 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".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Hooray! 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". |
|
@codex review |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
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". |
|
@codex review |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Breezy! 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". |
mldangelo-oai
changed the title
There was a problem hiding this comment.
💡 Codex Review
codex-security/sdk/typescript/src/cli.ts
Lines 762 to 764 in ab4710e
Skip model matching when either explicit scan is empty
For an uncached explicit scans match BEFORE AFTER or scans compare BEFORE AFTER, this invokes matchFindings even when either matching-input array is empty. Such a pair is known to have no matches, but the command still requires authentication and performs a paid model call that can fail before the empty comparison is saved; the --all path already short-circuits this case locally.
ℹ️ 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".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. 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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
6 participants
Summary
mainwithout restoring arbitrary pagination, buffering, or result-size limits.Verification
bun test tests-ts/workbench-scan-history.test.ts tests-ts/cli-workbench.test.ts