Skip to content

fix(reports): omit excerpts for unknown diff target kinds - #515

Merged
mldangelo-oai merged 5 commits into
openai:mainfrom
sylvesterkaczmarek:fix/working-tree-diff-source-excerpt
Oct 25, 2026
Merged

mldangelo-oai merged 5 commits into
openai:mainfrom
sylvesterkaczmarek:fix/working-tree-diff-source-excerpt

Conversation

@sylvesterkaczmarek

@sylvesterkaczmarek sylvesterkaczmarek commented Aug 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #514. A saved diff scan can have a head revision without recording what kind of diff was scanned. In that case, reports could show a line from the commit even when the finding came from different text in the working tree.

Changes

Only use Git objects for a saved diff excerpt when the target kind is commit or range. The check happens before either stored source scopes or the fallback for older scans is used. The regression test exercises both paths and checks that standard scans still show their source.

Testing

  • Reproduced the wrong line on main using a sealed scan in a temporary Git repository. Both get-scan and list-findings returned it for a null target kind and for a simulated unrecognized kind. Both omit it with the fix. Commit and range excerpts still match the committed file; working-tree diffs still omit the excerpt.
  • The regression test failed in the four affected cases on main. All 12 cases passed with the fix, as did all 40 related workbench tests and three SDK fixture scenarios.
  • All five portable plugin checks passed: Ruff lint and format, the SDK CI build, plugin source compatibility, and all nine tests for the compatibility checker.

Risk and rollout

Only the optional source excerpt changes. Findings, locations, and sealed artifacts are untouched. Existing saved scans pick up the fix when read; no migration is needed.

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 17, 2026
@sylvesterkaczmarek
sylvesterkaczmarek force-pushed the fix/working-tree-diff-source-excerpt branch from 983c016 to 99cca6a Compare September 30, 2026 22:12
@sylvesterkaczmarek

Copy link
Copy Markdown
Contributor Author

I resolved this PR against current main while preserving the intended fix and newer upstream behavior. The branch is now mergeable and no longer has a base conflict. Could a maintainer please re-review it when convenient?

@codex review

@sylvesterkaczmarek
sylvesterkaczmarek force-pushed the fix/working-tree-diff-source-excerpt branch from 99cca6a to cf56e9b Compare October 1, 2026 09:34
@mldangelo-oai mldangelo-oai changed the title fix(plugin): avoid stale source excerpts for working-tree diffs fix(reports): avoid stale source excerpts for working-tree diffs Oct 8, 2026
@mldangelo-oai mldangelo-oai added the area:reports Human-readable reports, source excerpts, exports, and SARIF projections. label Oct 8, 2026
@mldangelo-oai mldangelo-oai changed the title fix(reports): avoid stale source excerpts for working-tree diffs fix(reports): omit excerpts for unknown diff target kinds Oct 8, 2026
@mldangelo-oai mldangelo-oai removed the area:reports Human-readable reports, source excerpts, exports, and SARIF projections. label Oct 8, 2026
@sylvesterkaczmarek

Copy link
Copy Markdown
Contributor Author

The current node-ci run is green. Could a maintainer review the remaining unknown-target excerpt guard?

@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.

Thanks for sticking with this one. I checked the same saved scan before and after the fix: both get-scan and list-findings now leave out the misleading excerpt, and commit and range scans still show the right line. Looks good to me.

@mldangelo-oai
mldangelo-oai merged commit af973f9 into openai:main Oct 25, 2026
139 checks passed
@sylvesterkaczmarek

Copy link
Copy Markdown
Contributor Author

Thanks for taking another look and checking the saved scan before and after. Glad this one is in.

@github-actions github-actions Bot mentioned this pull request Oct 25, 2026
3 tasks done
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.

Omit source excerpts for saved diff scans with unknown target kind

2 participants