feat: match repeated findings across scan history - #575
Conversation
|
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 The description now includes the final QA results and merge order. Please review the current head, |
|
Codex Review: Didn't find any major issues. Keep them coming! 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". |
…odex/match-repeated-findings
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: c3d4a5661b
ℹ️ 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".
|
@codex review Please review the current head, |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! 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 Please review the current head, |
|
Codex Review: Didn't find any major issues. You're on a roll. 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". |
…iew-575-f533 # Conflicts: # sdk/typescript/src/cli.ts
|
@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". |
|
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". |
mldangelo-oai
changed the title
|
@codex review Please review exact head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 043fa2a80c
ℹ️ 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".
…/pr-575 # Conflicts: # sdk/typescript/scripts/fixtures/package-consumer.ts
|
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". |
kmbroai
left a comment
kmbroai
left a comment
There was a problem hiding this comment.
Critical review
Reviewed head 4110ecec1822c5c09f516e7fa4b61ca6eb8ae185 against its declared base #574.
Recommendation: the paged matcher is justified; coordinate the base's bundle upgrade before shipping. No new blocking defect found in this child diff. Repeated full-history occurrences should not consume the input budget as if they were independent issues, and large valid evidence should not be silently omitted to fit a request.
Correctness and current feedback
The current matchScanFindingsInternal fixes the earlier review findings: occurrence IDs are checked for nonempty global uniqueness before deterministic reconciliation; native comparison configuration sets project_doc_max_bytes: 0; omitted cards require full evidence even for a final no-match decision. The repository-instruction and occurrence-collision comments referred to an earlier head and should not be reported as still present.
Catalogue/evidence progress is enforced by the host, unknown IDs and invalid offsets are rejected, and Unicode-character pagination matches the documented upstream message boundary rather than imposing a new arbitrary finding cap. Uncertainty crossing a confirmed group remains invalid at the pair projection boundary instead of being silently discarded. The CLI now forwards saved finding groups, and newly confirmed groups are reused by later batches.
Simplification and feature scope
The essential work is a compact catalogue, a bounded-message evidence protocol, and deterministic expansion back to occurrences. Keep those separate from presentation and authentication. The new public SDK export is optional to fixing existing scans match/compare; it commits the project to an imported-finding contract, so retain it only if that API is intended for supported consumers. The added input validation is warranted if it is exported.
For cost-limited automatic matching, the single-turn guard prevents this feature from silently adding pagination turns; it is not itself a dollar-accurate spending guarantee. Keep that wording distinct from the scan cost-accounting contract.
I reproduced #574's unchanged-version cache problem separately. Neither this child's source tests nor its new SDK export fixes the stale installed plugin payload; advance the final stack's bundle version once.
Verification
Ran catalogue, comparison, matching end-to-end, CLI-workbench, and history-renderer suites: 114 passed, 3 skipped, 0 failed. Linux, Bun 1.3.14 / Node 22.13.1 with cached dependencies. Tests use synthetic model replies; I did not assess live model matching quality, run a production history migration, or perform a native Windows run.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aae0256d4e
ℹ️ 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".
|
@codex review Please review exact head a59c713, including canonical nonblank finding identities, upstream-limit Unicode evidence batching, read-only matcher isolation, globally unique occurrence IDs, final relationship reconciliation, component boundaries, explicit wrapped plugin capabilities, and installed public SDK types. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a59c713ebc
ℹ️ 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".
|
@codex review Please review exact head 02abe26, including canonical nonblank finding identities, upstream-limit Unicode evidence batching, read-only matcher isolation, globally unique occurrence IDs, final relationship reconciliation, component boundaries, explicit wrapped plugin capabilities, and installed public SDK types with supported maximum reasoning effort. |
|
Codex Review: Didn't find any major issues. Chef's kiss. 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". |
3 participants
Summary
Match repeated findings across completed scan history with compact, paged catalogues and deterministic confirmed identities. Keep this pull request stacked on #574, reuse existing Codex authentication, and add the intended public SDK matcher without changing the public CLI surface.
Changes
matchScanFindingswith self-contained public input, result, progress, cancellation, and reasoning-effort types, including the existing supported maximum reasoning effort.Testing
Validated on exact proposed head
02abe26a52c32e6a6caf323f9088a26da2f57bc9:pnpm run types,pnpm run format, package build and packing, andgit diff --check: passed.@openai/codex-security@0.1.19, public import and matcher, NodeNext consumer types using maximum reasoning effort, CLI, 113 bundled-plugin files, bundled Codex version, nested worker, current publication exports, and all 273 archive entries passed.Risk and rollout
This pull request depends on #574 and must be landed after its parent. Existing saved comparisons are not rewritten automatically; the existing forced all-scan matching command can recompute prior comparisons when explicitly requested.
Confirmed matches can affect later history comparisons. Deterministic identity reconciliation, complete evidence requirements, globally unique occurrence identifiers, and final comparison validation keep existing relationships internally consistent without collapsing distinct related issues.
The intended
matchScanFindingsSDK export is public API. No CLI commands, flags, accepted values, public environment variables, or defaults are added or changed.Public disclosure review