Skip to content

feat(build): Add auto-detection of PR number from GitHub Actions #2722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Sep 9, 2025
Merged

feat(build): Add auto-detection of PR number from GitHub Actions #2722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
54682d0
feat(build): Add auto-detection of PR number from GitHub Actions
runningcode Sep 1, 2025
c56e32e
fix: Format code and update test snapshots for PR auto-detection
runningcode Sep 2, 2025
4046833
fix: Remove redundant closure in PR number detection
runningcode Sep 2, 2025
b9fd298
Address PR feedback: improve get_github_pr_number function
runningcode Sep 3, 2025
5509fd9
Update integration tests for help text changes
runningcode Sep 3, 2025
1a4e171
Address review feedback: add debug logging and simplify PR parsing
runningcode Sep 8, 2025
c4f4bfd
Update integration tests for help text changes
runningcode Sep 8, 2025
9dabcbb
Format code and update test snapshots for PR auto-detection
runningcode Sep 8, 2025
3a42364
fix: Update integration tests for help text changes
runningcode Sep 8, 2025
b2f7057
fix: Update not-macos integration test for help text changes
runningcode Sep 8, 2025
54f47bd
Address remaining review feedback from szokeasaurusrex
runningcode Sep 8, 2025
69fafe2
Format code
runningcode Sep 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions src/commands/build/upload.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ use crate::utils::fs::TempDir;
use crate::utils::fs::TempFile;
use crate::utils::progress::ProgressBar;
use crate::utils::vcs::{
self, get_provider_from_remote, get_repo_from_remote, git_repo_base_ref, git_repo_head_ref,
git_repo_remote_url,
self, get_github_pr_number, get_provider_from_remote, get_repo_from_remote, git_repo_base_ref,
git_repo_head_ref, git_repo_remote_url,
};

pub fn make_command(command: Command) -> Command {
Expand Down Expand Up @@ -86,7 +86,9 @@ pub fn make_command(command: Command) -> Command {
Arg::new("pr_number")
.long("pr-number")
.value_parser(clap::value_parser!(u32))
.help("The pull request number to use for the upload. If not provided, the current pull request number will be used.")
.help("The pull request number to use for the upload. If not provided and running \
in a pull_request-triggered GitHub Actions workflow, the PR number will be automatically \
detected from GitHub Actions environment variables.")
)
.arg(
Arg::new("build_configuration")
Expand Down Expand Up @@ -194,7 +196,10 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {

let base_repo_name = matches.get_one("base_repo_name").map(String::as_str);
let base_sha = matches.get_one("base_sha").map(String::as_str);
let pr_number = matches.get_one::<u32>("pr_number");
let pr_number = matches
.get_one("pr_number")
.copied()
.or_else(get_github_pr_number);

let build_configuration = matches.get_one("build_configuration").map(String::as_str);
let release_notes = matches.get_one("release_notes").map(String::as_str);
Expand Down Expand Up @@ -259,7 +264,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
base_repo_name,
head_ref: head_ref.as_deref(),
base_ref: base_ref.as_deref(),
pr_number,
pr_number: pr_number.as_ref(),
};
match upload_file(
&authenticated_api,
Expand Down
44 changes: 44 additions & 0 deletions src/utils/vcs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,28 @@ fn find_merge_base_ref(
Ok(merge_base_sha)
}

/// Attempts to get the PR number from GitHub Actions environment variables.
/// Returns the PR number if running in a GitHub Actions pull request environment.
pub fn get_github_pr_number() -> Option<u32> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this function, I didn't log a warning if we failed to fetch here because i figure there are so many different CI providers we'd be logging a warning every time. Until we cover a broader range of CI providers it doesn't make sense to log something. WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug logging should be fine, I'd recommend adding some logs around failure points, particularly in the like 258 block and any other early return points just to make debugging easier.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I second this, as debug logs are hidden by default. Default log level is WARN. However, when folks report bugs, we ask them to set a log level of DEBUG, as the debug logs, though verbose, often help identify the problem.

let github_ref = std::env::var("GITHUB_REF").ok()?;
let event_name = std::env::var("GITHUB_EVENT_NAME").ok()?;

if event_name != "pull_request" {
debug!("Not running in pull_request event, got: {}", event_name);
return None;
}

let pr_number_str = github_ref.strip_prefix("refs/pull/")?;
debug!("Extracted PR reference: {}", pr_number_str);

let pr_number_str = pr_number_str.split('/').next()?;
debug!("Parsing PR number from: {}", pr_number_str);

let pr_number = pr_number_str.parse().ok()?;
debug!("Auto-detected PR number from GitHub Actions: {}", pr_number);
Some(pr_number)
}

fn find_reference_url(repo: &str, repos: &[Repo]) -> Result<Option<String>> {
let mut non_git = false;
for configured_repo in repos {
Expand Down Expand Up @@ -1278,3 +1300,25 @@ fn test_git_repo_head_ref() {
"HEAD is detached - no branch reference available"
);
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_get_github_pr_number() {
std::env::set_var("GITHUB_EVENT_NAME", "pull_request");
std::env::set_var("GITHUB_REF", "refs/pull/123/merge");
let pr_number = get_github_pr_number();
assert_eq!(pr_number, Some(123));
std::env::set_var("GITHUB_EVENT_NAME", "push");
let pr_number = get_github_pr_number();
assert_eq!(pr_number, None);
std::env::set_var("GITHUB_EVENT_NAME", "pull_request");
std::env::set_var("GITHUB_REF", "refs/heads/main");
let pr_number = get_github_pr_number();
assert_eq!(pr_number, None);
std::env::remove_var("GITHUB_EVENT_NAME");
std::env::remove_var("GITHUB_REF");
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Test Pollution from Global Environment Modifications

The test_get_github_pr_number function modifies global environment variables (GITHUB_EVENT_NAME, GITHUB_REF) without isolation. This creates shared state, which can lead to race conditions and flaky test results when tests run in parallel. If the test panics, these variables may also remain set, affecting other tests.

Fix in Cursor Fix in Web

5 changes: 3 additions & 2 deletions tests/integration/_cases/build/build-upload-help-macos.trycmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ Options:
The base reference (branch) to use for the upload. If not provided, the merge-base with
the remote tracking branch will be used.
--pr-number <pr_number>
The pull request number to use for the upload. If not provided, the current pull request
number will be used.
The pull request number to use for the upload. If not provided and running in a
pull_request-triggered GitHub Actions workflow, the PR number will be automatically
detected from GitHub Actions environment variables.
--build-configuration <build_configuration>
The build configuration to use for the upload. If not provided, the current version will
be used.
Expand Down
5 changes: 3 additions & 2 deletions tests/integration/_cases/build/build-upload-help-not-macos.trycmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ Options:
The base reference (branch) to use for the upload. If not provided, the merge-base with
the remote tracking branch will be used.
--pr-number <pr_number>
The pull request number to use for the upload. If not provided, the current pull request
number will be used.
The pull request number to use for the upload. If not provided and running in a
pull_request-triggered GitHub Actions workflow, the PR number will be automatically
detected from GitHub Actions environment variables.
--build-configuration <build_configuration>
The build configuration to use for the upload. If not provided, the current version will
be used.
Expand Down