Skip to content

Fix GitHub Enterprise URL parsing in review crate (Vibe Kanban) #1784

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

Open
LSRCT wants to merge 4 commits into
base: main
Choose a base branch
from
+36 −105
Open

Fix GitHub Enterprise URL parsing in review crate (Vibe Kanban) #1784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2269faf
All tasks are complete. Let me provide a summary of the changes made:
LSRCT Jan 5, 2026
b7b65fc
Cleanup script changes for workspace 4afe95a8-1a94-4f23-b641-b4fa1480…
LSRCT Jan 5, 2026
eef849d
Done. I've removed all the requested doc comments:
LSRCT Jan 5, 2026
3f8055a
Fixed. The issue was that `gh` returns `headRepository` and `headRepo…
LSRCT Jan 5, 2026
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
3 changes: 0 additions & 3 deletions crates/review/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ pub enum ReviewError {
#[error("GitHub CLI is not authenticated. Run 'gh auth login' first.")]
GhNotAuthenticated,

#[error("Invalid GitHub PR URL format. Expected: https://github.com/owner/repo/pull/123")]
InvalidPrUrl,

#[error("Failed to get PR information: {0}")]
PrInfoFailed(String),

Expand Down
100 changes: 18 additions & 82 deletions crates/review/src/github.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,16 @@ pub struct PrInfo {
pub head_ref_name: String,
}

/// Response from `gh pr view --json`
#[derive(Debug, Deserialize)]
struct GhRepoOwner {
login: String,
}

#[derive(Debug, Deserialize)]
struct GhRepo {
name: String,
}

#[derive(Debug, Deserialize)]
#[serde(rename_all = "camelCase")]
struct GhPrView {
Expand All @@ -26,47 +35,8 @@ struct GhPrView {
base_ref_oid: String,
head_ref_oid: String,
head_ref_name: String,
}

/// Parse a GitHub PR URL to extract owner, repo, and PR number
///
/// Expected format: https://github.com/owner/repo/pull/123
pub fn parse_pr_url(url: &str) -> Result<(String, String, i64), ReviewError> {
let url = url.trim();

// Remove trailing slashes
let url = url.trim_end_matches('/');

// Try to parse as URL
let parts: Vec<&str> = url.split('/').collect();

// Find the index of "github.com" and then extract owner/repo/pull/number
let github_idx = parts
.iter()
.position(|&p| p == "github.com")
.ok_or(ReviewError::InvalidPrUrl)?;

// We need at least: github.com / owner / repo / pull / number
if parts.len() < github_idx + 5 {
return Err(ReviewError::InvalidPrUrl);
}

let owner = parts[github_idx + 1].to_string();
let repo = parts[github_idx + 2].to_string();

if parts[github_idx + 3] != "pull" {
return Err(ReviewError::InvalidPrUrl);
}

let pr_number: i64 = parts[github_idx + 4]
.parse()
.map_err(|_| ReviewError::InvalidPrUrl)?;

if owner.is_empty() || repo.is_empty() || pr_number <= 0 {
return Err(ReviewError::InvalidPrUrl);
}

Ok((owner, repo, pr_number))
head_repository: GhRepo,
head_repository_owner: GhRepoOwner,
}

/// Check if the GitHub CLI is installed
Expand All @@ -83,21 +53,18 @@ fn ensure_gh_available() -> Result<(), ReviewError> {
Ok(())
}

/// Get PR information using `gh pr view`
pub fn get_pr_info(owner: &str, repo: &str, pr_number: i64) -> Result<PrInfo, ReviewError> {
pub fn get_pr_info(pr_url: &str) -> Result<PrInfo, ReviewError> {
ensure_gh_available()?;

debug!("Fetching PR info for {owner}/{repo}#{pr_number}");
debug!("Fetching PR info for {pr_url}");

let output = Command::new("gh")
.args([
"pr",
"view",
&pr_number.to_string(),
"--repo",
&format!("{owner}/{repo}"),
pr_url,
"--json",
"title,body,baseRefOid,headRefOid,headRefName",
"title,body,baseRefOid,headRefOid,headRefName,headRepository,headRepositoryOwner",
])
.output()
.map_err(|e| ReviewError::PrInfoFailed(e.to_string()))?;
Expand All @@ -121,8 +88,8 @@ pub fn get_pr_info(owner: &str, repo: &str, pr_number: i64) -> Result<PrInfo, Re
serde_json::from_str(&stdout).map_err(|e| ReviewError::PrInfoFailed(e.to_string()))?;

Ok(PrInfo {
owner: owner.to_string(),
repo: repo.to_string(),
owner: pr_view.head_repository_owner.login,
repo: pr_view.head_repository.name,
title: pr_view.title,
description: pr_view.body,
base_commit: pr_view.base_ref_oid,
Expand Down Expand Up @@ -196,34 +163,3 @@ pub fn checkout_commit(commit_sha: &str, repo_dir: &Path) -> Result<(), ReviewEr

Ok(())
}

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

#[test]
fn test_parse_pr_url_valid() {
let (owner, repo, pr) = parse_pr_url("https://github.com/anthropics/claude-code/pull/123")
.expect("Should parse valid URL");
assert_eq!(owner, "anthropics");
assert_eq!(repo, "claude-code");
assert_eq!(pr, 123);
}

#[test]
fn test_parse_pr_url_with_trailing_slash() {
let (owner, repo, pr) =
parse_pr_url("https://github.com/owner/repo/pull/456/").expect("Should parse");
assert_eq!(owner, "owner");
assert_eq!(repo, "repo");
assert_eq!(pr, 456);
}

#[test]
fn test_parse_pr_url_invalid_format() {
assert!(parse_pr_url("https://github.com/owner/repo").is_err());
assert!(parse_pr_url("https://github.com/owner/repo/issues/123").is_err());
assert!(parse_pr_url("https://gitlab.com/owner/repo/pull/123").is_err());
assert!(parse_pr_url("not a url").is_err());
}
}
38 changes: 18 additions & 20 deletions crates/review/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use anyhow::Result;
use api::{ReviewApiClient, ReviewStatus, StartRequest};
use clap::Parser;
use error::ReviewError;
use github::{checkout_commit, clone_repo, get_pr_info, parse_pr_url};
use github::{checkout_commit, clone_repo, get_pr_info};
use indicatif::{ProgressBar, ProgressStyle};
use tempfile::TempDir;
use tracing::debug;
Expand Down Expand Up @@ -130,17 +130,15 @@ async fn run(args: Args) -> Result<(), ReviewError> {
let mut config = config::Config::load();
let email = prompt_email(&mut config);

// 2. Parse PR URL
let spinner = create_spinner("Parsing PR URL...");
let (owner, repo, pr_number) = parse_pr_url(&args.pr_url)?;
spinner.finish_with_message(format!("PR: {owner}/{repo}#{pr_number}"));

// 3. Get PR info
// 2. Get PR info (also extracts owner/repo from the URL via gh CLI)
let spinner = create_spinner("Fetching PR information...");
let pr_info = get_pr_info(&owner, &repo, pr_number)?;
spinner.finish_with_message(format!("PR: {}", pr_info.title));
let pr_info = get_pr_info(&args.pr_url)?;
spinner.finish_with_message(format!(
"PR: {}/{} - {}",
pr_info.owner, pr_info.repo, pr_info.title
));

// 4. Select Claude Code session (optional)
// 3. Select Claude Code session (optional)
let session_files = match session_selector::select_session(&pr_info.head_ref_name) {
Ok(session_selector::SessionSelection::Selected(files)) => {
println!(" Selected {} session file(s)", files.len());
Expand All @@ -157,20 +155,20 @@ async fn run(args: Args) -> Result<(), ReviewError> {
}
};

// 5. Clone repository to temp directory
// 4. Clone repository to temp directory
let temp_dir = TempDir::new().map_err(|e| ReviewError::CloneFailed(e.to_string()))?;
let repo_dir = temp_dir.path().join(&repo);
let repo_dir = temp_dir.path().join(&pr_info.repo);

let spinner = create_spinner("Cloning repository...");
clone_repo(&owner, &repo, &repo_dir)?;
clone_repo(&pr_info.owner, &pr_info.repo, &repo_dir)?;
spinner.finish_with_message("Repository cloned");

// 6. Checkout PR head commit
// 5. Checkout PR head commit
let spinner = create_spinner("Checking out PR...");
checkout_commit(&pr_info.head_commit, &repo_dir)?;
spinner.finish_with_message("PR checked out");

// 7. Create tarball (with optional session data)
// 6. Create tarball (with optional session data)
let spinner = create_spinner("Creating archive...");

// If sessions were selected, write .agent-messages.json to repo root
Expand All @@ -185,18 +183,18 @@ async fn run(args: Args) -> Result<(), ReviewError> {
let size_mb = payload.len() as f64 / 1_048_576.0;
spinner.finish_with_message(format!("Archive created ({size_mb:.2} MB)"));

// 8. Initialize review
// 7. Initialize review
let client = ReviewApiClient::new(args.api_url.clone());
let spinner = create_spinner("Initializing review...");
let init_response = client.init(&args.pr_url, &email, &pr_info.title).await?;
spinner.finish_with_message(format!("Review ID: {}", init_response.review_id));

// 9. Upload archive
// 8. Upload archive
let spinner = create_spinner("Uploading archive...");
client.upload(&init_response.upload_url, payload).await?;
spinner.finish_with_message("Upload complete");

// 10. Start review
// 9. Start review
let spinner = create_spinner("Starting review...");
let codebase_url = format!("r2://{}", init_response.object_key);
client
Expand All @@ -212,7 +210,7 @@ async fn run(args: Args) -> Result<(), ReviewError> {
.await?;
spinner.finish_with_message(format!("Review started, we'll send you an email at {} when the review is ready. This can take a few minutes, you may now close the terminal", email));

// 11. Poll for completion
// 10. Poll for completion
let spinner = create_spinner("Review in progress...");
let start_time = std::time::Instant::now();

Expand Down Expand Up @@ -246,7 +244,7 @@ async fn run(args: Args) -> Result<(), ReviewError> {
}
}

// 12. Print result URL
// 11. Print result URL
let review_url = client.review_url(&init_response.review_id.to_string());
println!("\nReview available at:");
println!(" {review_url}");
Expand Down