From d9029cd11f3883f35d4d5ef92f10eef0a5cad837 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Tue, 24 Mar 2026 18:48:47 +0000 Subject: [PATCH 1/5] fix: use merge state for tray grouping --- src-tauri/src/github.rs | 7 +- src-tauri/src/menu.rs | 159 ++++++++++++++++++++++++++++++++++++++-- src-tauri/src/models.rs | 1 + 3 files changed, 159 insertions(+), 8 deletions(-) diff --git a/src-tauri/src/github.rs b/src-tauri/src/github.rs index ae23d33..0bda817 100644 --- a/src-tauri/src/github.rs +++ b/src-tauri/src/github.rs @@ -53,7 +53,8 @@ const PR_FRAGMENT: &str = r#"id mergeQueueEntry { state position - }"#; + } + mergeStateStatus"#; fn build_pr_query() -> String { let since = chrono::Utc::now() - chrono::Duration::days(14); @@ -166,6 +167,10 @@ fn parse_pr_node(node: &Value) -> Option { .unwrap_or("") .to_string(), is_review_requested: false, + merge_state_status: node + .get("mergeStateStatus") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()), }) } diff --git a/src-tauri/src/menu.rs b/src-tauri/src/menu.rs index b1fa91a..3650f2f 100644 --- a/src-tauri/src/menu.rs +++ b/src-tauri/src/menu.rs @@ -10,6 +10,14 @@ struct PrSection { default_collapsed: bool, } +/// Returns true when GitHub considers the PR actually mergeable (clean or has hooks). +fn is_actually_mergeable(pr: &PullRequest) -> bool { + matches!( + pr.merge_state_status.as_deref(), + Some("CLEAN") | Some("HAS_HOOKS") + ) +} + /// Port of src/lib/stores.ts groupPrs() matching its section ordering. fn group_prs(all_prs: &[PullRequest]) -> Vec { let review_requested: Vec<_> = all_prs @@ -86,11 +94,10 @@ fn group_prs(all_prs: &[PullRequest]) -> Vec { .iter() .filter(|pr| { pr.merge_queue_info.is_none() - && pr.check_status == CheckStatus::Success + && is_actually_mergeable(pr) + && pr.check_status != CheckStatus::Failure + && pr.check_status != CheckStatus::Error && pr.review_decision.as_deref() != Some("CHANGES_REQUESTED") - && pr.review_decision.as_deref() != Some("APPROVED") - && (pr.review_decision.as_deref() == Some("REVIEW_REQUIRED") - || pr.review_decision.is_none()) }) .cloned() .collect(), @@ -116,11 +123,12 @@ fn group_prs(all_prs: &[PullRequest]) -> Vec { .iter() .filter(|pr| { pr.merge_queue_info.is_none() - && pr.check_status == CheckStatus::None + && !is_actually_mergeable(pr) + && pr.check_status != CheckStatus::Failure + && pr.check_status != CheckStatus::Error + && pr.check_status != CheckStatus::Pending && pr.review_decision.as_deref() != Some("CHANGES_REQUESTED") && pr.review_decision.as_deref() != Some("APPROVED") - && (pr.review_decision.as_deref() == Some("REVIEW_REQUIRED") - || pr.review_decision.is_none()) }) .cloned() .collect(), @@ -132,6 +140,7 @@ fn group_prs(all_prs: &[PullRequest]) -> Vec { .iter() .filter(|pr| { pr.merge_queue_info.is_none() + && !is_actually_mergeable(pr) && pr.check_status != CheckStatus::Failure && pr.check_status != CheckStatus::Error && pr.review_decision.as_deref() == Some("APPROVED") @@ -338,3 +347,139 @@ fn truncate(s: &str, max: usize) -> String { truncated } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::models::*; + + fn make_pr() -> PullRequest { + PullRequest { + id: "PR_1".into(), + number: 1, + title: "test".into(), + url: "https://github.com/test/repo/pull/1".into(), + state: PrState::Open, + repository: "repo".into(), + owner: "test".into(), + head_ref: "feature".into(), + base_ref: "main".into(), + check_status: CheckStatus::Success, + is_draft: false, + labels: vec![], + merge_queue_info: None, + created_at: "2025-01-01T00:00:00Z".into(), + updated_at: "2025-01-01T00:00:00Z".into(), + review_decision: None, + additions: 0, + deletions: 0, + comment_count: 0, + author_login: "user".into(), + author_avatar_url: "".into(), + is_review_requested: false, + merge_state_status: None, + } + } + + fn find_section<'a>(sections: &'a [PrSection], title: &str) -> Option<&'a PrSection> { + sections.iter().find(|s| s.title == title) + } + + #[test] + fn blocked_review_required_not_in_mergeable() { + let pr = PullRequest { + review_decision: Some("REVIEW_REQUIRED".into()), + merge_state_status: Some("BLOCKED".into()), + check_status: CheckStatus::Success, + ..make_pr() + }; + let sections = group_prs(&[pr]); + assert!( + find_section(§ions, "Mergeable").is_none() + || find_section(§ions, "Mergeable").unwrap().prs.is_empty(), + "BLOCKED PR should not appear in Mergeable" + ); + let waiting = find_section(§ions, "Waiting for Review") + .expect("should be in Waiting for Review"); + assert_eq!(waiting.prs.len(), 1); + } + + #[test] + fn clean_pr_is_mergeable() { + let pr = PullRequest { + review_decision: None, + merge_state_status: Some("CLEAN".into()), + check_status: CheckStatus::Success, + ..make_pr() + }; + let sections = group_prs(&[pr]); + let mergeable = find_section(§ions, "Mergeable").expect("should be in Mergeable"); + assert_eq!(mergeable.prs.len(), 1); + } + + #[test] + fn has_hooks_pr_is_mergeable() { + let pr = PullRequest { + review_decision: None, + merge_state_status: Some("HAS_HOOKS".into()), + check_status: CheckStatus::Success, + ..make_pr() + }; + let sections = group_prs(&[pr]); + let mergeable = find_section(§ions, "Mergeable").expect("should be in Mergeable"); + assert_eq!(mergeable.prs.len(), 1); + } + + #[test] + fn approved_and_clean_is_mergeable() { + let pr = PullRequest { + review_decision: Some("APPROVED".into()), + merge_state_status: Some("CLEAN".into()), + check_status: CheckStatus::Success, + ..make_pr() + }; + let sections = group_prs(&[pr]); + let mergeable = find_section(§ions, "Mergeable").expect("should be in Mergeable"); + assert_eq!(mergeable.prs.len(), 1); + assert!( + find_section(§ions, "Approved").is_none() + || find_section(§ions, "Approved").unwrap().prs.is_empty() + ); + } + + #[test] + fn approved_but_blocked_stays_in_approved() { + let pr = PullRequest { + review_decision: Some("APPROVED".into()), + merge_state_status: Some("BLOCKED".into()), + check_status: CheckStatus::Success, + ..make_pr() + }; + let sections = group_prs(&[pr]); + let approved = find_section(§ions, "Approved").expect("should be in Approved"); + assert_eq!(approved.prs.len(), 1); + assert!( + find_section(§ions, "Mergeable").is_none() + || find_section(§ions, "Mergeable").unwrap().prs.is_empty() + ); + } + + #[test] + fn unknown_merge_state_not_mergeable() { + let pr = PullRequest { + review_decision: None, + merge_state_status: None, + check_status: CheckStatus::Success, + ..make_pr() + }; + let sections = group_prs(&[pr]); + assert!( + find_section(§ions, "Mergeable").is_none() + || find_section(§ions, "Mergeable").unwrap().prs.is_empty(), + "Unknown merge state should not be Mergeable" + ); + let waiting = find_section(§ions, "Waiting for Review") + .expect("should be in Waiting for Review"); + assert_eq!(waiting.prs.len(), 1); + } +} diff --git a/src-tauri/src/models.rs b/src-tauri/src/models.rs index 6162c5e..8c7be2d 100644 --- a/src-tauri/src/models.rs +++ b/src-tauri/src/models.rs @@ -54,6 +54,7 @@ pub struct PullRequest { pub author_login: String, pub author_avatar_url: String, pub is_review_requested: bool, + pub merge_state_status: Option, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] From 560577f9643976b8a164c894855622645836a6f4 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Tue, 24 Mar 2026 18:48:06 +0000 Subject: [PATCH 2/5] fix: use merge state for panel mergeable grouping --- src/lib/components.test.ts | 45 ++++++++++++++++++++++++++++++++++++++ src/lib/stores.ts | 20 +++++++++++------ src/lib/types.ts | 1 + 3 files changed, 59 insertions(+), 7 deletions(-) diff --git a/src/lib/components.test.ts b/src/lib/components.test.ts index f85e7a6..a31a2a9 100644 --- a/src/lib/components.test.ts +++ b/src/lib/components.test.ts @@ -36,6 +36,7 @@ const mockPr: PullRequest = { author_login: "testuser", author_avatar_url: "https://avatars.githubusercontent.com/u/1?v=4", is_review_requested: false, + merge_state_status: null, }; const mockUser: GitHubUser = { @@ -97,6 +98,50 @@ describe("groupPrs", () => { expect(sections.find(s => s.title === "Needs Your Review")?.prs).toHaveLength(1); expect(sections.find(s => s.title === "Approved")?.prs).toHaveLength(1); }); + + it("does NOT put review-blocked PR with passing checks in Mergeable (bug fix)", () => { + const pr = { ...mockPr, review_decision: "REVIEW_REQUIRED", merge_state_status: "BLOCKED", check_status: "success" as const }; + const sections = groupPrs([pr]); + expect(sections.find(s => s.title === "Mergeable")).toBeUndefined(); + const waiting = sections.find(s => s.title === "Waiting for Review"); + expect(waiting).toBeDefined(); + expect(waiting!.prs).toHaveLength(1); + }); + + it("puts a clean PR in Mergeable", () => { + const pr = { ...mockPr, review_decision: null, merge_state_status: "CLEAN", check_status: "success" as const }; + const sections = groupPrs([pr]); + const mergeable = sections.find(s => s.title === "Mergeable"); + expect(mergeable).toBeDefined(); + expect(mergeable!.prs).toHaveLength(1); + }); + + it("puts a HAS_HOOKS PR in Mergeable", () => { + const pr = { ...mockPr, review_decision: null, merge_state_status: "HAS_HOOKS", check_status: "success" as const }; + const sections = groupPrs([pr]); + expect(sections.find(s => s.title === "Mergeable")!.prs).toHaveLength(1); + }); + + it("promotes approved + clean PR to Mergeable (not Approved)", () => { + const pr = { ...mockPr, review_decision: "APPROVED", merge_state_status: "CLEAN", check_status: "success" as const }; + const sections = groupPrs([pr]); + expect(sections.find(s => s.title === "Mergeable")!.prs).toHaveLength(1); + expect(sections.find(s => s.title === "Approved")).toBeUndefined(); + }); + + it("keeps approved + blocked PR in Approved", () => { + const pr = { ...mockPr, review_decision: "APPROVED", merge_state_status: "BLOCKED", check_status: "success" as const }; + const sections = groupPrs([pr]); + expect(sections.find(s => s.title === "Approved")!.prs).toHaveLength(1); + expect(sections.find(s => s.title === "Mergeable")).toBeUndefined(); + }); + + it("treats unknown merge state as not mergeable", () => { + const pr = { ...mockPr, review_decision: null, merge_state_status: null, check_status: "success" as const }; + const sections = groupPrs([pr]); + expect(sections.find(s => s.title === "Mergeable")).toBeUndefined(); + expect(sections.find(s => s.title === "Waiting for Review")!.prs).toHaveLength(1); + }); }); // --------------------------------------------------------------------------- diff --git a/src/lib/stores.ts b/src/lib/stores.ts index ba5c9a4..09c05aa 100644 --- a/src/lib/stores.ts +++ b/src/lib/stores.ts @@ -18,6 +18,11 @@ export const authenticated = writable(false); export const loading = writable(true); export const lastUpdated = writable(null); +/** Returns true when GitHub considers the PR actually mergeable (clean or has merge hooks). */ +function isActuallyMergeable(pr: PullRequest): boolean { + return pr.merge_state_status === "CLEAN" || pr.merge_state_status === "HAS_HOOKS"; +} + export function groupPrs(allPrs: PullRequest[]): PrSection[] { const reviewRequested = allPrs.filter(pr => pr.is_review_requested); const myPrs = allPrs.filter(pr => !pr.is_review_requested); @@ -60,10 +65,9 @@ export function groupPrs(allPrs: PullRequest[]): PrSection[] { icon: CircleDot, prs: nonDraftOpen.filter(pr => pr.merge_queue_info == null && - pr.check_status === "success" && - pr.review_decision !== "CHANGES_REQUESTED" && - pr.review_decision !== "APPROVED" && - (pr.review_decision === "REVIEW_REQUIRED" || pr.review_decision == null) + isActuallyMergeable(pr) && + pr.check_status !== "failure" && pr.check_status !== "error" && + pr.review_decision !== "CHANGES_REQUESTED" ), }, { @@ -81,10 +85,11 @@ export function groupPrs(allPrs: PullRequest[]): PrSection[] { icon: Eye, prs: nonDraftOpen.filter(pr => pr.merge_queue_info == null && - pr.check_status === "none" && + !isActuallyMergeable(pr) && + pr.check_status !== "failure" && pr.check_status !== "error" && + pr.check_status !== "pending" && pr.review_decision !== "CHANGES_REQUESTED" && - pr.review_decision !== "APPROVED" && - (pr.review_decision === "REVIEW_REQUIRED" || pr.review_decision == null) + pr.review_decision !== "APPROVED" ), }, { @@ -92,6 +97,7 @@ export function groupPrs(allPrs: PullRequest[]): PrSection[] { icon: CheckCircle, prs: nonDraftOpen.filter(pr => pr.merge_queue_info == null && + !isActuallyMergeable(pr) && pr.check_status !== "failure" && pr.check_status !== "error" && pr.review_decision === "APPROVED" ), diff --git a/src/lib/types.ts b/src/lib/types.ts index 69320c7..92b4535 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -36,6 +36,7 @@ export interface PullRequest { author_login: string; author_avatar_url: string; is_review_requested: boolean; + merge_state_status: string | null; } export interface GitHubUser { From 1db4bdbf5a454418d60ad82edf00b8ac8b4ce71e Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Tue, 24 Mar 2026 23:47:00 +0000 Subject: [PATCH 3/5] refactor: tighten mergeable filter and strengthen tray grouping tests --- src-tauri/src/github.rs | 2 +- src-tauri/src/menu.rs | 67 ++++++++++++++++++++++------------------- 2 files changed, 37 insertions(+), 32 deletions(-) diff --git a/src-tauri/src/github.rs b/src-tauri/src/github.rs index 0bda817..d76bfaa 100644 --- a/src-tauri/src/github.rs +++ b/src-tauri/src/github.rs @@ -170,7 +170,7 @@ fn parse_pr_node(node: &Value) -> Option { merge_state_status: node .get("mergeStateStatus") .and_then(|v| v.as_str()) - .map(|s| s.to_string()), + .map(str::to_owned), }) } diff --git a/src-tauri/src/menu.rs b/src-tauri/src/menu.rs index 3650f2f..4ccecdc 100644 --- a/src-tauri/src/menu.rs +++ b/src-tauri/src/menu.rs @@ -10,7 +10,7 @@ struct PrSection { default_collapsed: bool, } -/// Returns true when GitHub considers the PR actually mergeable (clean or has hooks). +/// Returns true when GitHub's merge-state check is satisfied (CLEAN or HAS_HOOKS). fn is_actually_mergeable(pr: &PullRequest) -> bool { matches!( pr.merge_state_status.as_deref(), @@ -95,8 +95,7 @@ fn group_prs(all_prs: &[PullRequest]) -> Vec { .filter(|pr| { pr.merge_queue_info.is_none() && is_actually_mergeable(pr) - && pr.check_status != CheckStatus::Failure - && pr.check_status != CheckStatus::Error + && pr.check_status == CheckStatus::Success && pr.review_decision.as_deref() != Some("CHANGES_REQUESTED") }) .cloned() @@ -385,6 +384,20 @@ mod tests { sections.iter().find(|s| s.title == title) } + /// Assert a PR appears in exactly one section with the given title and no others + /// (ignoring empty sections from group_prs output). + fn assert_only_in_section(sections: &[PrSection], expected_title: &str) { + let non_empty: Vec<_> = sections.iter().filter(|s| !s.prs.is_empty()).collect(); + assert_eq!( + non_empty.len(), + 1, + "Expected PR in exactly one section, but found {} non-empty sections: {:?}", + non_empty.len(), + non_empty.iter().map(|s| &s.title).collect::>() + ); + assert_eq!(non_empty[0].title, expected_title); + } + #[test] fn blocked_review_required_not_in_mergeable() { let pr = PullRequest { @@ -394,14 +407,7 @@ mod tests { ..make_pr() }; let sections = group_prs(&[pr]); - assert!( - find_section(§ions, "Mergeable").is_none() - || find_section(§ions, "Mergeable").unwrap().prs.is_empty(), - "BLOCKED PR should not appear in Mergeable" - ); - let waiting = find_section(§ions, "Waiting for Review") - .expect("should be in Waiting for Review"); - assert_eq!(waiting.prs.len(), 1); + assert_only_in_section(§ions, "Waiting for Review"); } #[test] @@ -415,6 +421,7 @@ mod tests { let sections = group_prs(&[pr]); let mergeable = find_section(§ions, "Mergeable").expect("should be in Mergeable"); assert_eq!(mergeable.prs.len(), 1); + assert_only_in_section(§ions, "Mergeable"); } #[test] @@ -428,6 +435,7 @@ mod tests { let sections = group_prs(&[pr]); let mergeable = find_section(§ions, "Mergeable").expect("should be in Mergeable"); assert_eq!(mergeable.prs.len(), 1); + assert_only_in_section(§ions, "Mergeable"); } #[test] @@ -439,12 +447,7 @@ mod tests { ..make_pr() }; let sections = group_prs(&[pr]); - let mergeable = find_section(§ions, "Mergeable").expect("should be in Mergeable"); - assert_eq!(mergeable.prs.len(), 1); - assert!( - find_section(§ions, "Approved").is_none() - || find_section(§ions, "Approved").unwrap().prs.is_empty() - ); + assert_only_in_section(§ions, "Mergeable"); } #[test] @@ -456,12 +459,7 @@ mod tests { ..make_pr() }; let sections = group_prs(&[pr]); - let approved = find_section(§ions, "Approved").expect("should be in Approved"); - assert_eq!(approved.prs.len(), 1); - assert!( - find_section(§ions, "Mergeable").is_none() - || find_section(§ions, "Mergeable").unwrap().prs.is_empty() - ); + assert_only_in_section(§ions, "Approved"); } #[test] @@ -473,13 +471,20 @@ mod tests { ..make_pr() }; let sections = group_prs(&[pr]); - assert!( - find_section(§ions, "Mergeable").is_none() - || find_section(§ions, "Mergeable").unwrap().prs.is_empty(), - "Unknown merge state should not be Mergeable" - ); - let waiting = find_section(§ions, "Waiting for Review") - .expect("should be in Waiting for Review"); - assert_eq!(waiting.prs.len(), 1); + assert_only_in_section(§ions, "Waiting for Review"); + } + + #[test] + fn review_requested_pr_excluded_from_authored_sections() { + let pr = PullRequest { + is_review_requested: true, + merge_state_status: Some("CLEAN".into()), + check_status: CheckStatus::Success, + review_decision: None, + ..make_pr() + }; + let sections = group_prs(&[pr]); + // Should only appear in "Needs Your Review" (review-requested bucket) + assert_only_in_section(§ions, "Needs Your Review"); } } From 90ed561aa7d4ee444f0f64541a0a743b626f8c82 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Tue, 24 Mar 2026 23:47:18 +0000 Subject: [PATCH 4/5] refactor: tighten mergeable filter, add type safety, and strengthen panel tests --- src/lib/components.test.ts | 50 +++++++++++++++++++++++--------------- src/lib/stores.ts | 17 ++++++++----- src/lib/types.ts | 3 ++- 3 files changed, 44 insertions(+), 26 deletions(-) diff --git a/src/lib/components.test.ts b/src/lib/components.test.ts index a31a2a9..4b12da0 100644 --- a/src/lib/components.test.ts +++ b/src/lib/components.test.ts @@ -100,47 +100,59 @@ describe("groupPrs", () => { }); it("does NOT put review-blocked PR with passing checks in Mergeable (bug fix)", () => { - const pr = { ...mockPr, review_decision: "REVIEW_REQUIRED", merge_state_status: "BLOCKED", check_status: "success" as const }; + const pr = { ...mockPr, review_decision: "REVIEW_REQUIRED", merge_state_status: "BLOCKED" as const, check_status: "success" as const }; const sections = groupPrs([pr]); - expect(sections.find(s => s.title === "Mergeable")).toBeUndefined(); - const waiting = sections.find(s => s.title === "Waiting for Review"); - expect(waiting).toBeDefined(); - expect(waiting!.prs).toHaveLength(1); + const nonEmpty = sections.filter(s => s.prs.length > 0); + expect(nonEmpty).toHaveLength(1); + expect(nonEmpty[0].title).toBe("Waiting for Review"); }); it("puts a clean PR in Mergeable", () => { - const pr = { ...mockPr, review_decision: null, merge_state_status: "CLEAN", check_status: "success" as const }; + const pr = { ...mockPr, review_decision: null, merge_state_status: "CLEAN" as const, check_status: "success" as const }; const sections = groupPrs([pr]); - const mergeable = sections.find(s => s.title === "Mergeable"); - expect(mergeable).toBeDefined(); - expect(mergeable!.prs).toHaveLength(1); + const nonEmpty = sections.filter(s => s.prs.length > 0); + expect(nonEmpty).toHaveLength(1); + expect(nonEmpty[0].title).toBe("Mergeable"); }); it("puts a HAS_HOOKS PR in Mergeable", () => { - const pr = { ...mockPr, review_decision: null, merge_state_status: "HAS_HOOKS", check_status: "success" as const }; + const pr = { ...mockPr, review_decision: null, merge_state_status: "HAS_HOOKS" as const, check_status: "success" as const }; const sections = groupPrs([pr]); - expect(sections.find(s => s.title === "Mergeable")!.prs).toHaveLength(1); + const nonEmpty = sections.filter(s => s.prs.length > 0); + expect(nonEmpty).toHaveLength(1); + expect(nonEmpty[0].title).toBe("Mergeable"); }); it("promotes approved + clean PR to Mergeable (not Approved)", () => { - const pr = { ...mockPr, review_decision: "APPROVED", merge_state_status: "CLEAN", check_status: "success" as const }; + const pr = { ...mockPr, review_decision: "APPROVED", merge_state_status: "CLEAN" as const, check_status: "success" as const }; const sections = groupPrs([pr]); - expect(sections.find(s => s.title === "Mergeable")!.prs).toHaveLength(1); - expect(sections.find(s => s.title === "Approved")).toBeUndefined(); + const nonEmpty = sections.filter(s => s.prs.length > 0); + expect(nonEmpty).toHaveLength(1); + expect(nonEmpty[0].title).toBe("Mergeable"); }); it("keeps approved + blocked PR in Approved", () => { - const pr = { ...mockPr, review_decision: "APPROVED", merge_state_status: "BLOCKED", check_status: "success" as const }; + const pr = { ...mockPr, review_decision: "APPROVED", merge_state_status: "BLOCKED" as const, check_status: "success" as const }; const sections = groupPrs([pr]); - expect(sections.find(s => s.title === "Approved")!.prs).toHaveLength(1); - expect(sections.find(s => s.title === "Mergeable")).toBeUndefined(); + const nonEmpty = sections.filter(s => s.prs.length > 0); + expect(nonEmpty).toHaveLength(1); + expect(nonEmpty[0].title).toBe("Approved"); }); it("treats unknown merge state as not mergeable", () => { const pr = { ...mockPr, review_decision: null, merge_state_status: null, check_status: "success" as const }; const sections = groupPrs([pr]); - expect(sections.find(s => s.title === "Mergeable")).toBeUndefined(); - expect(sections.find(s => s.title === "Waiting for Review")!.prs).toHaveLength(1); + const nonEmpty = sections.filter(s => s.prs.length > 0); + expect(nonEmpty).toHaveLength(1); + expect(nonEmpty[0].title).toBe("Waiting for Review"); + }); + + it("excludes review-requested PRs from authored merge-state sections", () => { + const pr = { ...mockPr, is_review_requested: true, merge_state_status: "CLEAN" as const, check_status: "success" as const, review_decision: null }; + const sections = groupPrs([pr]); + const nonEmpty = sections.filter(s => s.prs.length > 0); + expect(nonEmpty).toHaveLength(1); + expect(nonEmpty[0].title).toBe("Needs Your Review"); }); }); diff --git a/src/lib/stores.ts b/src/lib/stores.ts index 09c05aa..096b910 100644 --- a/src/lib/stores.ts +++ b/src/lib/stores.ts @@ -18,11 +18,16 @@ export const authenticated = writable(false); export const loading = writable(true); export const lastUpdated = writable(null); -/** Returns true when GitHub considers the PR actually mergeable (clean or has merge hooks). */ +/** Returns true when GitHub's merge-state check is satisfied (CLEAN or HAS_HOOKS). */ function isActuallyMergeable(pr: PullRequest): boolean { return pr.merge_state_status === "CLEAN" || pr.merge_state_status === "HAS_HOOKS"; } +/** Returns true when checks have failed or errored. */ +function isCheckFailed(pr: PullRequest): boolean { + return pr.check_status === "failure" || pr.check_status === "error"; +} + export function groupPrs(allPrs: PullRequest[]): PrSection[] { const reviewRequested = allPrs.filter(pr => pr.is_review_requested); const myPrs = allPrs.filter(pr => !pr.is_review_requested); @@ -48,7 +53,7 @@ export function groupPrs(allPrs: PullRequest[]): PrSection[] { icon: XCircle, prs: nonDraftOpen.filter(pr => pr.merge_queue_info == null && - (pr.check_status === "failure" || pr.check_status === "error") + isCheckFailed(pr) ), }, { @@ -56,7 +61,7 @@ export function groupPrs(allPrs: PullRequest[]): PrSection[] { icon: RotateCcw, prs: nonDraftOpen.filter(pr => pr.merge_queue_info == null && - pr.check_status !== "failure" && pr.check_status !== "error" && + !isCheckFailed(pr) && pr.review_decision === "CHANGES_REQUESTED" ), }, @@ -66,7 +71,7 @@ export function groupPrs(allPrs: PullRequest[]): PrSection[] { prs: nonDraftOpen.filter(pr => pr.merge_queue_info == null && isActuallyMergeable(pr) && - pr.check_status !== "failure" && pr.check_status !== "error" && + pr.check_status === "success" && pr.review_decision !== "CHANGES_REQUESTED" ), }, @@ -86,7 +91,7 @@ export function groupPrs(allPrs: PullRequest[]): PrSection[] { prs: nonDraftOpen.filter(pr => pr.merge_queue_info == null && !isActuallyMergeable(pr) && - pr.check_status !== "failure" && pr.check_status !== "error" && + !isCheckFailed(pr) && pr.check_status !== "pending" && pr.review_decision !== "CHANGES_REQUESTED" && pr.review_decision !== "APPROVED" @@ -98,7 +103,7 @@ export function groupPrs(allPrs: PullRequest[]): PrSection[] { prs: nonDraftOpen.filter(pr => pr.merge_queue_info == null && !isActuallyMergeable(pr) && - pr.check_status !== "failure" && pr.check_status !== "error" && + !isCheckFailed(pr) && pr.review_decision === "APPROVED" ), }, diff --git a/src/lib/types.ts b/src/lib/types.ts index 92b4535..f2dc18d 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -2,6 +2,7 @@ import type { Component, ComponentType, SvelteComponent } from "svelte"; export type PrState = "open" | "closed" | "merged"; export type CheckStatus = "pending" | "success" | "failure" | "error" | "none"; +export type MergeStateStatus = "BEHIND" | "BLOCKED" | "CLEAN" | "DIRTY" | "DRAFT" | "HAS_HOOKS" | "UNKNOWN" | "UNSTABLE" | null; export interface MergeQueueInfo { state: string; @@ -36,7 +37,7 @@ export interface PullRequest { author_login: string; author_avatar_url: string; is_review_requested: boolean; - merge_state_status: string | null; + merge_state_status: MergeStateStatus; } export interface GitHubUser { From 121fe845f354a5c30fd0bad74ff8592e153eaeeb Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Wed, 25 Mar 2026 00:02:19 +0000 Subject: [PATCH 5/5] fix: allow CLEAN PRs with no checks into Mergeable section --- src-tauri/src/menu.rs | 16 +++++++++++++++- src/lib/components.test.ts | 8 ++++++++ src/lib/stores.ts | 3 ++- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src-tauri/src/menu.rs b/src-tauri/src/menu.rs index 4ccecdc..7201d1f 100644 --- a/src-tauri/src/menu.rs +++ b/src-tauri/src/menu.rs @@ -95,7 +95,9 @@ fn group_prs(all_prs: &[PullRequest]) -> Vec { .filter(|pr| { pr.merge_queue_info.is_none() && is_actually_mergeable(pr) - && pr.check_status == CheckStatus::Success + && pr.check_status != CheckStatus::Failure + && pr.check_status != CheckStatus::Error + && pr.check_status != CheckStatus::Pending && pr.review_decision.as_deref() != Some("CHANGES_REQUESTED") }) .cloned() @@ -487,4 +489,16 @@ mod tests { // Should only appear in "Needs Your Review" (review-requested bucket) assert_only_in_section(§ions, "Needs Your Review"); } + + #[test] + fn clean_pr_with_no_checks_is_mergeable() { + let pr = PullRequest { + review_decision: None, + merge_state_status: Some("CLEAN".into()), + check_status: CheckStatus::None, + ..make_pr() + }; + let sections = group_prs(&[pr]); + assert_only_in_section(§ions, "Mergeable"); + } } diff --git a/src/lib/components.test.ts b/src/lib/components.test.ts index 4b12da0..83a56ea 100644 --- a/src/lib/components.test.ts +++ b/src/lib/components.test.ts @@ -147,6 +147,14 @@ describe("groupPrs", () => { expect(nonEmpty[0].title).toBe("Waiting for Review"); }); + it("puts a CLEAN PR with no checks in Mergeable (repos without required checks)", () => { + const pr = { ...mockPr, review_decision: null, merge_state_status: "CLEAN" as const, check_status: "none" as const }; + const sections = groupPrs([pr]); + const nonEmpty = sections.filter(s => s.prs.length > 0); + expect(nonEmpty).toHaveLength(1); + expect(nonEmpty[0].title).toBe("Mergeable"); + }); + it("excludes review-requested PRs from authored merge-state sections", () => { const pr = { ...mockPr, is_review_requested: true, merge_state_status: "CLEAN" as const, check_status: "success" as const, review_decision: null }; const sections = groupPrs([pr]); diff --git a/src/lib/stores.ts b/src/lib/stores.ts index 096b910..3e8d332 100644 --- a/src/lib/stores.ts +++ b/src/lib/stores.ts @@ -71,7 +71,8 @@ export function groupPrs(allPrs: PullRequest[]): PrSection[] { prs: nonDraftOpen.filter(pr => pr.merge_queue_info == null && isActuallyMergeable(pr) && - pr.check_status === "success" && + !isCheckFailed(pr) && + pr.check_status !== "pending" && pr.review_decision !== "CHANGES_REQUESTED" ), },