Skip to content

Comments

Update attachment_pdf_view_prompt.yml#3982

Closed
mrobertsonris wants to merge 3 commits intosublime-security:mainfrom
mrobertsonris:patch-6
Closed

Update attachment_pdf_view_prompt.yml#3982
mrobertsonris wants to merge 3 commits intosublime-security:mainfrom
mrobertsonris:patch-6

Conversation

@mrobertsonris
Copy link
Contributor

100% TP across all our Orgs.

100% TP across all our Orgs.
@mrobertsonris mrobertsonris requested a review from a team February 7, 2026 00:06
@mrobertsonris mrobertsonris requested a review from a team as a code owner February 7, 2026 00:06
@github-actions github-actions bot added the review-needed Indicates that a PR is waiting for review label Feb 7, 2026
github-actions bot added a commit that referenced this pull request Feb 7, 2026
@github-actions
Copy link

github-actions bot commented Feb 7, 2026

Test Rules Sync - Action Required

This PR was not automatically synced to test-rules because the author is not a member of the sublime-security organization.

To enable syncing, an organization member can comment /update-test-rules on this PR.

Once triggered, the rules will be synced on the next scheduled run (every 10 minutes).

eric-sim-sublime added a commit that referenced this pull request Feb 9, 2026
The previous logic used git merge-base to find a comparison point, but this
breaks when the PR contains merge commits from main. When comparing files at
HEAD (a merge commit) vs the calculated merge-base, it would incorrectly
detect all of main's recent changes as PR changes.

This fix uses git's three-dot diff syntax (origin/main...HEAD) which
automatically finds the correct merge-base and compares only the changes
introduced by the PR branch, regardless of merge commits.

Changes:
- Use 'git diff origin/main...HEAD --find-renames' to find changed files
- For each file, search for the old file with same rule ID (handles renames)
- Compare source fields using 'git show' directly
- Remove unnecessary sr-main checkout step (no longer needed)
- Handles all merge commit scenarios (HEAD, middle of history, multiple merges)

Benefits:
- Correctly detects only PR changes even with merge commits
- Handles file renames by searching for matching rule IDs
- More efficient (no need to checkout entire base branch)
- More robust (uses git's built-in three-dot diff)

Fixes issue where PRs with merge commits incorrectly triggered tests for
hundreds of unchanged rules (e.g., PR #3982 detected 774 rules when 0 changed).

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@zoomequipd
Copy link
Member

@mrobertsonris due to some technical difficulties, we've had to replicate this change in a different PR: #3987. I'm going to close this PR and get the other merged.

Thanks for the feedback on this rule!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-needed Indicates that a PR is waiting for review test-rules:excluded:author_membership

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants