feat: preserve cross-scan finding relationships - #574
Conversation
|
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". |
|
@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. 🎉 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 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. |
|
@codex review Please review the current head, |
|
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". |
|
@codex review Please review the current head, |
|
Codex Review: Didn't find any major issues. Delightful! 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". |
Keep legacy repositories separate when preparing matching inputs, simplify finding alias bookkeeping, and retain the shared workbench stdin API. Use platform-aware Python probes and exercise comparison payloads larger than command-line limits.
|
@codex review Please review exact head |
|
Codex Review: Didn't find any major issues. Another round soon, please! 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: Something went wrong. Try again later by commenting “@codex review”. ℹ️ 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. 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". |
|
@codex review Please review current head |
![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: d7bca7f46e
ℹ️ 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".
| await dependencies.matchFindings({ | ||
| before: input.before, | ||
| after: input.after, | ||
| }), |
There was a problem hiding this comment.
When compare-scans supplies the newly added knownFindingGroups, this call explicitly reconstructs the input using only before and after; the built-in matcher also still emits only matches and uncertain. Thus ordinary scans match/scans compare executions can neither use confirmed cross-scan identities nor generate the new related pairs, leaving the added persistence and display path unreachable and allowing later matching to contradict saved history. Pass the group data through and extend the matcher input/output schema, including the --all path.
AGENTS.md reference: sdk/typescript/AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
kmbroai
left a comment
kmbroai
left a comment
There was a problem hiding this comment.
Critical review
Reviewed head d7bca7f46e0ff0a8bf1f32080000364d927aef17, with #575 treated as its explicit matcher follow-up.
Recommendation: keep the storage/invariant work, but fix bundle upgrade behavior before shipping this stack. Preserving confirmed identity separately from uncertainty and relatedness is necessary; collapsing those concepts can make an unresolved finding appear fixed.
[P2] Existing installs keep the old bundled workbench
This PR changes bundled database/history/schema helpers but leaves both plugin version markers at 0.1.22. bootstrapPlugin only restages when the version changes.
I reproduced this with the real local Codex installer in a fresh temporary home: bootstrap the current-main 0.1.22 bundle, then bootstrap this PR's bundle. The second operation still reported 0.1.22; both the staged and installed workbench_scan_history.py hashes remained identical to main, while this PR's source hash differed. Fresh-source CLI tests do not cover that installed-payload path.
Assign one fresh synchronized bundle version to the final #574/#575 payload and add a predecessor-cache upgrade regression. A new content-hash cache mechanism is unnecessary; the existing version contract is sufficient if advanced correctly.
Necessity and simplification
The database validates the complete comparison before replacing saved rows, keeps related pairs distinct from confirmed groups, and checks coverage across every prior grouped occurrence before calling an issue resolved. Batched relation lookups and indexed traversal of selected finding components are preferable to per-finding database scans. The migration adds indexes without rewriting history.
The current CLI still strips knownFindingGroups when invoking the old matcher, and that matcher does not produce related. This is a real limitation of this base by itself, but #575 explicitly wires the full path. Describe this PR as storage preparation, not the complete end-user matching feature; do not force matcher implementation into the storage review merely to resolve the older comment. Conversely, do not advertise the whole feature if only the base lands.
Verification
Ran history, CLI-workbench, and publication-store suites: 42 passed, 0 failed, plus the real local cache-upgrade reproduction. Linux, Bun 1.3.14 / Node 22.13.1 with cached dependencies. No model call, live publication, or native Windows run was performed.
3 participants
Summary
Preserve confirmed, uncertain, and related relationships between findings in the SQLite workbench so later comparisons can reuse known identities without collapsing independent findings. #575 remains stacked on this storage work.
No user-facing CLI commands, flags, accepted values, or defaults change, and this PR adds no model calls.
Changes
origin/mainatfa9dfa491a9ed464fbfca9481dc493784a8ef1fc.Testing
At
d7bca7f46e0ff0a8bf1f32080000364d927aef17:bun test --timeout 30000 --seed 12345 ./tests-ts: 1,603 passed, 28 platform-specific skips, 0 failed.pnpm run types,pnpm run format,pnpm run build, Python syntax parsing for the changed workbench modules, andgit diff --check: passed.Risk and rollout
Migration 31 builds indexes under SQLite's normal write lock but does not rewrite saved comparisons or completed scan artifacts.
Older custom plugins keep confirmed and uncertain results, but cannot preserve related links and remain subject to operating-system argument limits for large comparison payloads.
Merge this PR before #575.
Public disclosure review