fix(build): Skip base_sha and base_ref when equal to head_sha during auto-inference (EME-607) #2924
Conversation
runningcode
force-pushed
the
no/eme-607-skip-base-sha-when-equal-to-head
branch
from
2c02e58 to
c5cb821
Compare
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| && !base_ref_from_user | ||
| && base_sha.is_some() | ||
| && head_sha.is_some() | ||
| && base_sha.as_deref() == head_sha.as_deref() | ||
| { | ||
| debug!( | ||
| "Base SHA equals head SHA ({}), and both were auto-inferred. Skipping base_sha and base_ref, but keeping head_sha.", | ||
| base_sha.as_deref().expect("base_sha is Some at this point") | ||
| ); | ||
| base_sha = None; | ||
| base_ref = None; |
There was a problem hiding this comment.
Bug: In GitHub Actions, if base_sha == head_sha, the reliable GITHUB_BASE_REF value is incorrectly cleared because base_ref_from_user only checks CLI args.
Severity: CRITICAL | Confidence: 1.00
🔍 Detailed Analysis
When running in a GitHub Actions pull request context, if base_sha and head_sha are identical, the base_ref value populated from the GITHUB_BASE_REF environment variable is incorrectly cleared to None. This occurs because the base_ref_from_user flag only checks for CLI arguments, causing the clearing logic at src/commands/build/upload.rs:254~265 to treat the reliable GitHub Actions-provided base_ref as an auto-inferred value that should be discarded. This results in the backend receiving base_ref: None instead of the correct base branch name.
💡 Suggested Fix
Adjust the condition for clearing base_ref to distinguish between values derived from git introspection and those reliably provided by the GITHUB_BASE_REF environment variable. Ensure base_ref from GITHUB_BASE_REF is not cleared.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/commands/build/upload.rs#L254-L265
Potential issue: When running in a GitHub Actions pull request context, if `base_sha`
and `head_sha` are identical, the `base_ref` value populated from the `GITHUB_BASE_REF`
environment variable is incorrectly cleared to `None`. This occurs because the
`base_ref_from_user` flag only checks for CLI arguments, causing the clearing logic at
`src/commands/build/upload.rs:254~265` to treat the reliable GitHub Actions-provided
`base_ref` as an auto-inferred value that should be discarded. This results in the
backend receiving `base_ref: None` instead of the correct base branch name.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
This is intentional, we want to clear the GITHUB_BASE_REF if it was auto-inferred and the base-sha and head-sha match.
szokeasaurusrex
left a comment
szokeasaurusrex
left a comment
There was a problem hiding this comment.
Changelog looks good, I'll leave the code changes for the Emerge Tools team to review.
Once they approve, I'll stamp this for the CHANGELOG changes (I am code owner there)
…auto-inference (EME-607) When auto-inferring VCS metadata, if base_sha equals head_sha, we now skip setting both base_sha and base_ref (but still set head_sha) since comparing a commit to itself provides no meaningful baseline. This only applies when both base values are auto-inferred; user-provided values via CLI arguments are always respected. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…auto-inference (EME-607) When auto-inferring VCS metadata, if base_sha equals head_sha, we now skip setting both base_sha and base_ref (but still set head_sha) since comparing a commit to itself provides no meaningful baseline. This only applies when both base values are auto-inferred; user-provided values via CLI arguments are always respected. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
runningcode
force-pushed
the
no/eme-607-skip-base-sha-when-equal-to-head
branch
from
17a9795 to
f1ad020
Compare
Summary
This PR modifies
sentry-clito skip settingbase_shaandbase_refwhen they are auto-inferred andbase_shaequalshead_sha.Changes
base_shaandbase_refbase_sha == head_sha, we now set both toNonebefore sending to the API--base-shaand--base-refare always respected regardless of their valuesRationale
When
base_shaequalshead_sha, there is no meaningful comparison baseline since we'd be comparing a commit against itself. This typically occurs in certain CI scenarios where the auto-inference logic cannot determine a proper base commit. By skipping these values, we allow the backend to handle the comparison more appropriately.Closes EME-607
🤖 Generated with Claude Code