fix: preserve accepted evidence during Deep scan reduction - #442
mldangelo-oai wants to merge 31 commits into
Conversation
|
@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. |
![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: 5f90b68330
ℹ️ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54aff1afa6
ℹ️ 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 |
|
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a792d90968
ℹ️ 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 |
|
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b1ed1f46b
ℹ️ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4a54c5401
ℹ️ 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 Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: d4a54c5401
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.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ffd6d3f7f
ℹ️ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20b485b953
ℹ️ 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 Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 20b485b953
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.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be0a338634
ℹ️ 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 |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 440c2bbf50
ℹ️ 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 Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 440c2bbf50
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.
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 5ff2fb0c81
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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ff2fb0c81
ℹ️ 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 |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dca1d9ae6e
ℹ️ 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 Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: dca1d9ae6e
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.
|
@codex review |
|
@codex security review |
|
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". |
|
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fab35ea2e6
ℹ️ 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".
| }, | ||
| }); | ||
|
|
||
| for (const locations of [[], first.locations]) { |
There was a problem hiding this comment.
When a same-identity finding has multiple accepted locations and the reducer returns them in reverse order, reconciliation preserves the reducer's order rather than the source sequence; I reproduced [primary, secondary] becoming [secondary, primary]. This is observable because scan-history-renderer.ts displays locations[0], so a completed Deep scan can show a secondary site as the finding's primary location. Extend this case to preserve each accepted location sequence, as is already done for ordered attack-path evidence.
AGENTS.md reference: sdk/typescript/AGENTS.md:L23-L23
Useful? React with 👍 / 👎.
| [first, second], | ||
| [second, first], | ||
| ]) { | ||
| for (const supplied of [first.attackPath.steps, second.attackPath.steps]) { |
There was a problem hiding this comment.
When two same-identity workers report opposite but individually valid step orders, such as [read input, reach sink] and [reach sink, read input], every reducer result fails reconciliation: neither accepted sequence can contain the other as a subsequence, and duplicating either step exceeds the accepted occurrence maximum. I reproduced both original worker sequences throwing Deep reduction changed accepted finding.attackPath.steps evidence, so Deep mode can exhaust all retries solely because workers disagree on ordering. Unlike the previously fixed heuristic misses, no combined trace exists here; select one complete accepted trace while retaining the alternative in provenance rather than requiring an impossible supersequence.
AGENTS.md reference: sdk/typescript/AGENTS.md:L23-L23
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 355b2213d4
ℹ️ 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".
| expect(reduced.findings[0]!.summary).toBe(result.findings[0]!.summary); | ||
| }); | ||
|
|
||
| test("preserves the accepted finding producer in canonical provenance", async () => { |
There was a problem hiding this comment.
When same-identity workers disagree on classification and provenance, these checks validate the two tuples independently, allowing the reducer to combine worker A's title/taxonomy/extensions with worker B's source, candidateId, and provider. I reproduced the target bundle accepting an injection finding with candidate A's extension while attributing it to imported candidate B; because findingCandidateId prioritizes the provenance candidate, later history reconciliation can associate the canonical finding with the wrong candidate. Fresh evidence beyond the prior provenance and classification fixes is this cross-boundary combination: every individual field is accepted, but no worker supplied the resulting record. Bind canonical provenance and source-owned classification fields to the same accepted finding.
AGENTS.md reference: sdk/typescript/AGENTS.md:L23-L23
Useful? React with 👍 / 👎.
| threatModel: { | ||
| summary: "Worker boundary.", | ||
| assets: ["workers"], |
There was a problem hiding this comment.
When two accepted workers use the schema-valid threatModel extension fields with different scalar values, no reducer output can pass: omitting the field reports discarded evidence, selecting either accepted value fails the all-sources equality check, and combining the values is unsupported. I reproduced deployment: "public cloud" and deployment: "private datacenter" exhausting every possible scalar representation against the target bundle. Unlike the summary and array fields exercised here, arbitrary accepted threat-model properties are not reconciled, so a Deep scan can exhaust its reducer retries solely because workers supplied different valid deployment facts.
AGENTS.md reference: sdk/typescript/AGENTS.md:L23-L23
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d4c7e1cb0
ℹ️ 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".
| const { reconcileDeepReduction } = bundledReducer(await loadBundledRuntime()); | ||
| const original = reducerFinding("accepted-assessments"); | ||
|
|
||
| for (const field of ["impact", "likelihood"]) { |
There was a problem hiding this comment.
When same-identity workers provide the same categorical attack-path tuple but distinct schema-valid counterevidence, severity_rationale, or change_conditions strings, this coverage only treats impact/likelihood explanations as mergeable narratives. I reproduced the bundled reducer accepting either worker's complete attack path unchanged and dropping the other worker's counterevidence and assessment rationale from the canonical attackPath, leaving it only in nested provenance. Fresh evidence beyond the existing why/rationale coverage is that these text fields are defined by the bundled candidate attack-path schema but are still classified as conflicting scalars; retain every compatible complete claim as done for the covered explanations.
AGENTS.md reference: sdk/typescript/AGENTS.md:L23-L23
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d4c7e1cb0
ℹ️ 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".
| } | ||
| }); | ||
|
|
||
| test("binds related structured finding evidence to one accepted source", async () => { |
There was a problem hiding this comment.
When same-identity workers report { method: "dynamic", assertions: ["Dynamic assertion."] } and { method: "static", assertions: ["Static assertion."] }, a reducer that retains the second source's assertions but omits the optional method is rejected: tuple selection ignores arrays, chooses the first worker by order, restores dynamic, and then reports the source-backed static assertion as changed evidence. Unlike the earlier partially overlapping scalar case, the fresh evidence here is that an exact accepted array identifies a valid source tuple; use supplied array values when selecting the compatible record so this otherwise valid reduction does not consume retries or fail.
AGENTS.md reference: sdk/typescript/AGENTS.md:L23-L23
Useful? React with 👍 / 👎.
2 participants
Summary
Keep accepted worker evidence intact when Deep scans deduplicate findings or recover a saved reduction.
Changes
Testing
Affected reducer, custom-validation, runtime, workbench, timestamp, shutdown and parent-denial suites: 244 passed, 12 platform-dependent skips.
Types/generated-model checks, formatting and build passed.
Decoded runtime syntax, deterministic Brotli round-trip and unchanged code outside the reducer module verified.
Built npm package validation (282 entries) and the full installed-package smoke test passed.
Full repository suite, native Windows and live model-quality evaluation were not rerun. CI is left for a separate pass.
Final main refresh (
01bd062): 41 reducer-recovery tests and 44 patch-risk/CLI tests passed; types/models, formatting and build passed again. Both real cache upgrades matched all 118 installed plugin files and preserved credentials. The decoded reducer payload is unchanged by this final merge.Final main refresh (
fd98a90): package 0.1.21 includes the MCP launcher-permission fix; feature source and bundled payload are unchanged. Types/model generation, formatting, build, 28 focused package/report/launcher tests, static artifact verification and full installed-package smoke passed, including MCP initialization. CI was not awaited.Risk and rollout
No new CLI or SDK API. Conflicting or unsupported evidence still fails validation; all accepted originals remain available in provenance. The bundle version change refreshes cached installations. Repeated-trace reconciliation retains its existing fast paths and bounded search.
Public disclosure review
Existing commit contact metadata and account-specific automated review links prevent the second attestation. This update uses synthetic fixtures and a GitHub noreply commit identity; historical metadata and other authors' comments are unchanged.