Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
228 changes: 227 additions & 1 deletion src-tauri/src/git/forge.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::repo::{remote_target, RemoteProvider};
use crate::error::{AppError, AppResult};
use git2::Repository;
use serde::{Deserialize, Serialize};
use std::path::Path;
use std::process::Command;

Expand Down Expand Up @@ -74,9 +75,234 @@ pub fn create_pr(repo: &Repository, title: &str, body: &str, base: &str) -> AppR
Ok(url)
}

/// A pull request (GitHub) / merge request (GitLab), normalized across providers.
#[derive(Serialize)]
pub struct PullRequest {
pub number: u64,
pub title: String,
pub url: String,
/// Source branch (`headRefName` / `source_branch`) - links a PR to a branch.
pub branch: String,
pub draft: bool,
pub author: String,
/// Author avatar URL, or None when the provider doesn't hand one out cheaply.
pub author_avatar: Option<String>,
/// Rolled-up CI: "success" | "failure" | "pending" | "none".
pub checks: String,
/// "approved" | "changes_requested" | "review_required" | "none".
pub review: String,
}

/// Open pull/merge requests for the repo's remote, via the user's `gh`/`glab`
/// CLI. Returns an empty list for non-forge remotes rather than erroring, so the
/// UI just hides the section. GitLab yields a degraded row (no CI/review/avatar -
/// `glab mr list` doesn't include them).
pub fn list_prs(repo: &Repository) -> AppResult<Vec<PullRequest>> {
let Some(target) = remote_target(repo) else {
return Ok(Vec::new());
};
let dir = super::workdir(repo)?;
match target.provider {
RemoteProvider::Github => list_github(dir),
RemoteProvider::Gitlab => list_gitlab(dir),
}
}

#[derive(Deserialize)]
struct GhPr {
number: u64,
title: String,
url: String,
#[serde(rename = "headRefName")]
head_ref_name: String,
#[serde(rename = "isDraft")]
is_draft: bool,
author: GhAuthor,
#[serde(rename = "statusCheckRollup", default)]
status_check_rollup: Vec<GhCheck>,
#[serde(rename = "reviewDecision", default)]
review_decision: Option<String>,
}
#[derive(Deserialize)]
struct GhAuthor {
#[serde(default)]
login: String,
}
/// Heterogeneous rollup entry: a CheckRun (status+conclusion) or a StatusContext
/// (state). All optional so unknown shapes just don't contribute.
#[derive(Deserialize)]
struct GhCheck {
#[serde(default)]
status: Option<String>,
#[serde(default)]
conclusion: Option<String>,
#[serde(default)]
state: Option<String>,
}

fn list_github(dir: &Path) -> AppResult<Vec<PullRequest>> {
let out = run_cli(
dir,
&[
"gh", "pr", "list", "--state", "open", "--limit", "50", "--json",
"number,title,url,headRefName,isDraft,author,statusCheckRollup,reviewDecision",
],
)?;
Comment on lines +143 to +150

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map relevant files and inspect the CLI wrapper plus call sites.
git ls-files | rg '(^|/)src-tauri/src/git/|(^|/)src-tauri/src/.*run_cli|(^|/)src-tauri/src/.*cli|(^|/)src-tauri/src/.*provider'
rg -n "fn run_cli|run_cli\(|list_github\(|list_gitlab\(|refreshPrs\(" src-tauri/src -S

# Read the likely implementation slices.
for f in \
  src-tauri/src/git/forge.rs \
  src-tauri/src/git/mod.rs \
  src-tauri/src/git/*.rs
do
  [ -f "$f" ] || continue
  echo "===== $f ====="
  wc -l "$f"
done

sed -n '1,240p' src-tauri/src/git/forge.rs

Repository: jcardonne/gitchef

Length of output: 11061


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== forge.rs around list_gitlab ==="
sed -n '240,316p' src-tauri/src/git/forge.rs

echo "=== list_prs callers / refreshPrs ==="
rg -n "list_prs\(|refreshPrs|provider change|repo load|load repo|open.*repo|auto.*prs|prs" src-tauri/src -S

echo "=== other run_cli wrappers ==="
rg -n "Command::new\(.*output\(|Command::new|timeout|kill\(" src-tauri/src -S

Repository: jcardonne/gitchef

Length of output: 12686


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== frontend PR refresh flow ==="
rg -n "refreshPrs|list_prs|provider change|repo load|loadRepo|openRepo|selectedRepo|remote provider|GitHub|GitLab" . -S

echo "=== files that mention PR sidebar state ==="
rg -n "PR sidebar|pull request|merge request|prs" src* . -g '!target' -g '!node_modules' -S

echo "=== search for timeout/cancellation around invoke calls ==="
rg -n "AbortController|timeout|setTimeout|clearTimeout|cancel.*invoke|invoke\(" . -S

Repository: jcardonne/gitchef

Length of output: 16938


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== RepoView PR refresh handler ==="
sed -n '230,270p' src/components/RepoView.tsx

echo "=== Tauri list_prs command ==="
sed -n '140,160p' src-tauri/src/lib.rs

echo "=== forge.rs run_cli and list_prs ==="
sed -n '1,170p' src-tauri/src/git/forge.rs

Repository: jcardonne/gitchef

Length of output: 9019


Add a timeout around the PR CLI calls run_cli() still uses Command::output() with no timeout, and both list_github() and list_gitlab() are invoked automatically on repo load/provider change. If gh/glab hangs on auth or a dead network, the PR refresh never settles and the sidebar stays stale.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/src/git/forge.rs` around lines 143 - 150, The PR refresh path can
hang because list_github and list_gitlab rely on run_cli, which currently uses
Command::output() without any timeout. Update run_cli to enforce a bounded
execution time and make sure both list_github and list_gitlab surface a timeout
error instead of waiting indefinitely. Use the existing run_cli symbol as the
central fix point so repo load and provider changes cannot stall the sidebar
refresh.

let prs: Vec<GhPr> = serde_json::from_str(&out)
.map_err(|e| AppError::Msg(format!("could not parse `gh pr list` output: {e}")))?;
Ok(prs
.into_iter()
.map(|p| {
let author_avatar = (!p.login().is_empty())
.then(|| format!("https://github.com/{}.png?size=40", p.author.login));
PullRequest {
number: p.number,
title: p.title,
url: p.url,
branch: p.head_ref_name,
draft: p.is_draft,
author: p.author.login.clone(),
author_avatar,
checks: rollup_checks(&p.status_check_rollup),
review: normalize_review(p.review_decision.as_deref()),
}
})
.collect())
}
Comment on lines +143 to +171

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Silent truncation of PR/MR lists.

gh pr list is capped at --limit 50, and glab mr list uses no --per-page/--page flag, so it defaults to GitLab CLI's built-in page size of 30. Repos with more open PRs/MRs than these caps will silently show an incomplete sidebar list with no indication more exist.

♻️ Proposed fix
     let out = run_cli(dir, &["glab", "mr", "list", "--output", "json"])?;
+    // Without --per-page, glab defaults to 30 results per page.
-    let out = run_cli(dir, &["glab", "mr", "list", "--output", "json"])?;
+    let out = run_cli(dir, &["glab", "mr", "list", "--output", "json", "--per-page", "50"])?;

Also applies to: 240-258

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/src/git/forge.rs` around lines 143 - 171, The PR/MR listing helpers
are silently truncating results because `list_github` hard-caps `gh pr list` at
50 and the GitLab counterpart uses default CLI paging, so the sidebar can miss
items. Update `list_github` and the GitLab list function to fetch all
pages/results (use explicit pagination or loop until exhausted) instead of
relying on fixed defaults, and keep the existing `PullRequest` mapping intact
while aggregating every page into the returned vector.


impl GhPr {
fn login(&self) -> &str {
&self.author.login
}
}

/// Roll a GitHub statusCheckRollup up to one word: any failure wins, then any
/// still-running, then success; empty/all-unknown -> "none".
fn rollup_checks(checks: &[GhCheck]) -> String {
let mut any_pending = false;
let mut any_success = false;
for c in checks {
// CheckRun: not yet COMPLETED means it's still running.
if let Some(s) = &c.status {
if s != "COMPLETED" {
any_pending = true;
continue;
}
}
// Normalized outcome from either a CheckRun conclusion or a StatusContext state.
let outcome = c.conclusion.as_deref().or(c.state.as_deref()).unwrap_or("");
match outcome.to_ascii_uppercase().as_str() {
"FAILURE" | "ERROR" | "CANCELLED" | "TIMED_OUT" | "ACTION_REQUIRED"
| "STARTUP_FAILURE" => return "failure".into(),
"SUCCESS" | "NEUTRAL" | "SKIPPED" => any_success = true,
"PENDING" | "EXPECTED" | "IN_PROGRESS" | "QUEUED" => any_pending = true,
_ => {}
}
}
if any_pending {
"pending".into()
} else if any_success {
"success".into()
} else {
"none".into()
}
}

fn normalize_review(decision: Option<&str>) -> String {
match decision {
Some("APPROVED") => "approved",
Some("CHANGES_REQUESTED") => "changes_requested",
Some("REVIEW_REQUIRED") => "review_required",
_ => "none",
}
.to_string()
}

#[derive(Deserialize)]
struct GlMr {
iid: u64,
title: String,
web_url: String,
source_branch: String,
#[serde(default)]
draft: bool,
#[serde(default)]
work_in_progress: bool,
#[serde(default)]
author: GlAuthor,
}
#[derive(Deserialize, Default)]
struct GlAuthor {
#[serde(default)]
username: String,
}

fn list_gitlab(dir: &Path) -> AppResult<Vec<PullRequest>> {
let out = run_cli(dir, &["glab", "mr", "list", "--output", "json"])?;
let mrs: Vec<GlMr> = serde_json::from_str(&out)
.map_err(|e| AppError::Msg(format!("could not parse `glab mr list` output: {e}")))?;
Ok(mrs
.into_iter()
.map(|m| PullRequest {
number: m.iid,
title: m.title,
url: m.web_url,
branch: m.source_branch,
draft: m.draft || m.work_in_progress,
author: m.author.username,
author_avatar: None, // glab doesn't hand out an avatar URL in list output
checks: "none".into(),
review: "none".into(),
})
.collect())
}

#[cfg(test)]
mod tests {
use super::first_url;
use super::{first_url, normalize_review, rollup_checks, GhCheck};

fn check(status: Option<&str>, conclusion: Option<&str>, state: Option<&str>) -> GhCheck {
GhCheck {
status: status.map(String::from),
conclusion: conclusion.map(String::from),
state: state.map(String::from),
}
}

#[test]
fn rolls_up_checks_failure_wins() {
assert_eq!(rollup_checks(&[]), "none");
assert_eq!(
rollup_checks(&[check(Some("COMPLETED"), Some("SUCCESS"), None)]),
"success"
);
// A still-running check makes the whole thing pending...
assert_eq!(
rollup_checks(&[
check(Some("COMPLETED"), Some("SUCCESS"), None),
check(Some("IN_PROGRESS"), None, None),
]),
"pending"
);
// ...but any failure wins outright.
assert_eq!(
rollup_checks(&[
check(Some("IN_PROGRESS"), None, None),
check(Some("COMPLETED"), Some("FAILURE"), None),
]),
"failure"
);
// StatusContext form (state, no CheckRun status).
assert_eq!(rollup_checks(&[check(None, None, Some("success"))]), "success");
}

#[test]
fn normalizes_review_decision() {
assert_eq!(normalize_review(Some("APPROVED")), "approved");
assert_eq!(normalize_review(Some("CHANGES_REQUESTED")), "changes_requested");
assert_eq!(normalize_review(None), "none");
assert_eq!(normalize_review(Some("weird")), "none");
}

#[test]
fn extracts_the_pr_url_from_cli_output() {
Expand Down
17 changes: 17 additions & 0 deletions src-tauri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,21 @@ fn create_pr(repo: String, title: String, body: String, base: String) -> AppResu
forge::create_pr(&open(&repo)?, &title, &body, &base)
}

#[tauri::command(async)]
fn list_prs(repo: String) -> AppResult<Vec<forge::PullRequest>> {
forge::list_prs(&open(&repo)?)
}

#[tauri::command(async)]
fn open_url(url: String) -> AppResult<()> {
// Only web URLs - never a file:// or an arbitrary scheme, since the URL comes
// from CLI output; the OS opener must not be handed anything else.
if !url.starts_with("https://") && !url.starts_with("http://") {
return Err(error::AppError::Msg("refusing to open a non-web URL".into()));
}
files::open_url(&url)
}

#[tauri::command]
fn commit(repo: String, message: String) -> AppResult<String> {
ops::commit(&open(&repo)?, &message)
Expand Down Expand Up @@ -503,6 +518,8 @@ pub fn run() {
file_history,
file_blame,
create_pr,
list_prs,
open_url,
commit,
commit_amend,
checkout,
Expand Down
5 changes: 5 additions & 0 deletions src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {
FileContent,
FileDiff,
FileHistoryEntry,
PullRequest,
ReflogNode,
RepoInfo,
SequencerState,
Expand Down Expand Up @@ -79,6 +80,10 @@ export const fileBlame = (repo: string, path: string, rev: string | null) =>
/// Create a PR (GitHub) / MR (GitLab) via the gh/glab CLI; resolves to its URL.
export const createPr = (repo: string, title: string, body: string, base: string) =>
invoke<string>("create_pr", { repo, title, body, base });
/// Open pull/merge requests for the repo's remote (empty for non-forge remotes).
export const listPrs = (repo: string) => invoke<PullRequest[]>("list_prs", { repo });
/// Open a web URL in the default browser (backend validates it's http/https).
export const openUrl = (url: string) => invoke<void>("open_url", { url });

export const commit = (repo: string, message: string) =>
invoke<string>("commit", { repo, message });
Expand Down
Loading