diff --git a/README.md b/README.md index de352e2..98013ab 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ At launch, an animated wordmark and indeterminate loading indicator wait for the ### Read the dashboard - **Header:** repository, trunk, and stacked or single mode; the checked-out worktree branch and age of the last GitHub refresh; and, on larger terminals, a pie chart of row statuses beside two lines: `Next to merge:` names the lowest open PR layer by its PR title, or its branch name when the title is empty (stacks merge bottom-up), followed by one bullet per issue, colored red when it blocks and yellow when it is only waiting (or `ready to merge`), and `Blocked by:` counts rows by issue category, including issues that are waiting rather than blocking. A no-PR row is never selected as the next PR to merge. On smaller terminals the header shows nonzero counts for each verdict, including merged, queued, and closed PRs (`⊘`). A spinner means data is being fetched; an overdue refresh is highlighted. -- **Table:** rows hang off the trunk on a git-tree style spine; each PR layer is two lines with its colored verdict glyph as the node. The first line leads with the PR title in bold, then the PR number, state (draft is a PR flag layered over open, alongside merged, queued, and closed), checks, reviews, and local reports; the branch name sits dimmed on the second line. A no-PR row keeps the branch as its headline with a dimmed `no PR yet` hint, without inventing PR cells or a ready-to-merge verdict. The animated, pulsing row is the selected branch shown in the detail pane; `*` in the left column marks the branch checked out in this worktree. A muted suffix reports only that branch's local sync: `↑` has unpushed commits, `↓` has remote commits to incorporate, `⇅` has diverged, and `?` has no remote-tracking branch. Local sync does not change readiness or verdict colors. For a PR, the verdict glyph and headline color answer "is this ready?": `✔` means ready, not a draft, CI passing, no changes requested or review outstanding, no rebase needed, and declared local checks have no failed, stale, or missing result; `◷` means nothing is blocking but evidence is outstanding (running CI, pending review, missing local result); `✖` means a draft, failed CI, requested changes, rebase needed, or a failed or stale local result. Merged and closed layers are dimmed in every column; queued layers are green, and a draft state is red. The Checks cell shows `✔` (green) when everything passed and `✖` (red) when something failed; while any check is queued or running, an animated spinner replaces that icon and keeps the color of the worst state, so it spins yellow normally and red when something has already failed, and the icon returns when the checks finish. It gives passed/total for checks that ran, excluding those skipped by the workflow or by [repository configuration](#configuration). `—` means no result. The Threads column counts unresolved review threads: green `✔ 0` when all are resolved, yellow `◷ N` when some are open, red `✖ N` when open threads are configured to block (see [configuration](#configuration)). The Rebase column appears only when a PR layer needs a rebase: a red `↻` means the layer is blocked because it, or a layer under it, must be rebased (the detail pane names which; a rebase moves every layer above it, so each of them is marked), and `↻ `, for example `↻ main`, means the first unmerged layer is behind the trunk (gray unless `rebase.onto_trunk` makes it blocking). The Local column appears only when local checks are declared or reported for the view. The Reviews cell is red for changes requested, yellow when GitHub requires a review that is still outstanding (`required` when nobody has been asked, `pending` when someone has), and green when approved; when reviews are not required it stays gray and does not block the ready verdict. +- **Table:** rows hang off the trunk on a git-tree style spine; each PR layer is two lines with its colored verdict glyph as the node. The first line leads with the PR title in bold, then the PR number, state (draft is a PR flag layered over open, alongside merged, queued, and closed), checks, reviews, and local reports; the branch name sits dimmed on the second line. A no-PR row keeps the branch as its headline with a dimmed `no PR yet` hint, without inventing PR cells or a ready-to-merge verdict. The animated, pulsing row is the selected branch shown in the detail pane; `*` in the left column marks the branch checked out in this worktree. A muted suffix reports only that branch's local sync: `↑` has unpushed commits, `↓` has remote commits to incorporate, `⇅` has diverged, and `?` has no remote-tracking branch. Local sync does not change readiness or verdict colors. For a PR, the verdict glyph and headline color answer "is this ready?": `✔` means ready, not a draft, CI passing, no changes requested or review outstanding, no rebase needed, and declared local checks have no failed, stale, or missing result; `◷` means nothing is blocking but evidence is outstanding (running CI, pending review, missing local result); `✖` means a draft, failed CI, requested changes, rebase needed, or a failed or stale local result. Merged and closed layers are dimmed in every column; queued layers are green, and a draft state is red. The Checks cell shows `✔` (green) when everything passed and `✖` (red) when something failed; while any check is queued or running, an animated spinner replaces that icon and keeps the color of the worst state, so it spins yellow normally and red when something has already failed, and the icon returns when the checks finish. It gives passed/total for checks that ran, excluding those skipped by the workflow or by [repository configuration](#configuration). `—` means no result. The Threads column counts unresolved review threads: green `✔ 0` when all are resolved, yellow `◷ N` when some are open, red `✖ N` when open threads are configured to block (see [configuration](#configuration)). The Rebase column appears only when a PR layer needs a rebase: a red `↻` means the layer is blocked because it, or a layer under it, must be rebased (the detail pane names which; a rebase moves every layer above it, so each of them is marked), and `↻ `, for example `↻ main`, means the first unmerged layer is behind the trunk (gray unless `rebase.onto_trunk` makes it blocking). The Local column appears only when local checks are declared or reported for the view. The Reviews cell is red for changes requested, yellow when GitHub requires a review that is still outstanding (`required` when nobody has been asked, `pending` when someone has), and green when approved; GitHub's review decision is authoritative, so an approval counts even while other reviewers are still requested, and one given only by a bot hidden by `show_bots` reads `approved (bot)`. When reviews are not required it stays gray and does not block the ready verdict. - **Detail pane:** always open for the selected branch and starts on the worktree's checked-out branch. Its header names that branch's local sync next to `current worktree`, then shows the PR title in bold with the branch name dimmed beneath it (a branch without a PR title keeps the branch name as the headline); checks are listed failing first, then running, passed, and skipped, with durations; reviews show a headline and each visible reviewer, then local reports and whether the branch needs a rebase. When the details are longer than the pane, a scrollbar on its right edge shows the position; scroll with Page Up/Down or the mouse wheel. - **Footer:** key hints and operation results. Actions that change GitHub or the worktree open a confirmation modal listing the exact commands; press `Enter` or `y` to run them, `Esc` or `n` to cancel. Opening a PR in the browser runs immediately. @@ -97,7 +97,7 @@ test = { description = "cargo test --workspace --locked --no-fail-fast" } [reviews] bots = ["ci-bot"] -show_bots = false +show_bots = true gate_on_unresolved_threads = false [rebase] diff --git a/crates/stack-stat-core/src/config.rs b/crates/stack-stat-core/src/config.rs index 40c053d..9d963ec 100644 --- a/crates/stack-stat-core/src/config.rs +++ b/crates/stack-stat-core/src/config.rs @@ -55,17 +55,27 @@ pub struct LocalCheckConfig { } /// Known automated reviewers and initial visibility. -#[derive(Clone, Debug, Default, Deserialize, PartialEq, Eq)] +#[derive(Clone, Debug, Deserialize, PartialEq, Eq)] #[serde(default, deny_unknown_fields)] pub struct ReviewsConfig { /// Review logins the repository identifies as bots. pub bots: Vec, /// Whether unresolved review threads block readiness. pub gate_on_unresolved_threads: bool, - /// Whether bot reviews are visible before toggling them. + /// Whether bot reviews are visible before toggling them. Defaults to `true`. pub show_bots: bool, } +impl Default for ReviewsConfig { + fn default() -> Self { + Self { + bots: Vec::new(), + gate_on_unresolved_threads: false, + show_bots: true, + } + } +} + /// Repository policy for whether trunk drift blocks readiness. #[derive(Clone, Debug, Default, Deserialize, PartialEq, Eq)] #[serde(default, deny_unknown_fields)] diff --git a/crates/stack-stat-core/tests/config.rs b/crates/stack-stat-core/tests/config.rs index 5bbe23f..cfa0ab3 100644 --- a/crates/stack-stat-core/tests/config.rs +++ b/crates/stack-stat-core/tests/config.rs @@ -15,7 +15,7 @@ fn absent_files_use_defaults() { assert_eq!(config.poll.github, 30); assert_eq!(config.poll.github_active, 15); assert_eq!(config.poll.store, 2); - assert!(!config.reviews.show_bots); + assert!(config.reviews.show_bots); assert!(!config.reviews.gate_on_unresolved_threads); assert!(!config.rebase.onto_trunk); assert_eq!(config.display.reviews_default, ReviewsDefault::Collapsed); diff --git a/crates/stack-stat-tui/src/ui/table/labels.rs b/crates/stack-stat-tui/src/ui/table/labels.rs index 860bea9..15eb577 100644 --- a/crates/stack-stat-tui/src/ui/table/labels.rs +++ b/crates/stack-stat-tui/src/ui/table/labels.rs @@ -96,17 +96,19 @@ pub(super) fn review_label( let Some(pr) = &branch.pr else { return (missing_pr_label(branch), Health::Muted); }; - let summary = pr.review_summary(show_bots); - let summary = if pr.reviews.is_empty() { - match pr.review_decision { - ReviewDecision::Approved => ReviewSummary::Approved, - ReviewDecision::ChangesRequested => ReviewSummary::ChangesRequested, - ReviewDecision::ReviewRequired => ReviewSummary::Pending, - ReviewDecision::None => summary, + let visible = pr.review_summary(show_bots); + // GitHub's decision is authoritative for approval: pending reviewer requests + // and the bot display filter must not mask it. + let summary = match pr.review_decision { + ReviewDecision::Approved if visible != ReviewSummary::ChangesRequested => { + ReviewSummary::Approved } - } else { - summary + ReviewDecision::ChangesRequested => ReviewSummary::ChangesRequested, + ReviewDecision::ReviewRequired if pr.reviews.is_empty() => ReviewSummary::Pending, + _ => visible, }; + let approved_by_hidden_bot = + summary == ReviewSummary::Approved && visible != ReviewSummary::Approved; let requested = pr .reviews .iter() @@ -121,6 +123,9 @@ pub(super) fn review_label( ) } else { match summary { + ReviewSummary::Approved if approved_by_hidden_bot => { + ("✔", "approved (bot)", Health::Success) + } ReviewSummary::Approved => ("✔", "approved", Health::Success), ReviewSummary::ChangesRequested => ("↺", "changes", Health::Failure), ReviewSummary::Pending => ("◷", "pending", Health::Running), @@ -130,7 +135,7 @@ pub(super) fn review_label( if !full { return (icon.into(), health); } - if expanded { + if expanded && !approved_by_hidden_bot { let names: Vec<_> = pr .reviews .iter() diff --git a/crates/stack-stat-tui/tests/keymap.rs b/crates/stack-stat-tui/tests/keymap.rs index d326dfc..01f0d99 100644 --- a/crates/stack-stat-tui/tests/keymap.rs +++ b/crates/stack-stat-tui/tests/keymap.rs @@ -85,8 +85,9 @@ async fn navigation_refresh_and_unbound_enter_tab_have_distinct_actions() { )))] ); assert!(handles.github.recorded_actions().is_empty()); + assert!(app.show_bots(), "bot reviews are visible by default"); app.handle_key(key('b')); - assert!(app.show_bots()); + assert!(!app.show_bots()); app.handle_key(key('q')); } @@ -230,7 +231,7 @@ async fn every_normal_mode_binding_has_its_documented_effect() { Action::Refresh => assert_eq!(effects, [Effect::Refresh(Source::All)]), Action::Filter => assert_eq!(app.mode(), &Mode::Filter), Action::Sort => assert_eq!(app.sort(), stack_stat_tui::app::SortOrder::PrNumber), - Action::ToggleBots => assert!(app.show_bots()), + Action::ToggleBots => assert!(!app.show_bots(), "toggles away from the default"), Action::Help => assert_eq!(app.mode(), &Mode::Help), Action::Quit => assert_eq!(effects, [Effect::Quit]), } diff --git a/crates/stack-stat-tui/tests/render.rs b/crates/stack-stat-tui/tests/render.rs index d74e483..5263397 100644 --- a/crates/stack-stat-tui/tests/render.rs +++ b/crates/stack-stat-tui/tests/render.rs @@ -336,11 +336,11 @@ async fn bot_review_visibility_toggles_in_detail_pane() { .build() .into_app(); app.poll_once(Source::All).await; - assert!(find_row(&draw(&app, 120, 28), "dependabot[bot]").is_none()); + let visible = draw(&app, 120, 28); + assert!(find_row(&visible, "dependabot[bot]").is_some()); + assert!(find_row(&visible, "(bot)").is_some()); app.handle_key(key('b')); - let buf = draw(&app, 120, 28); - assert!(find_row(&buf, "dependabot[bot]").is_some()); - assert!(find_row(&buf, "(bot)").is_some()); + assert!(find_row(&draw(&app, 120, 28), "dependabot[bot]").is_none()); } #[tokio::test] diff --git a/crates/stack-stat-tui/tests/reviews.rs b/crates/stack-stat-tui/tests/reviews.rs index e48984c..426d32e 100644 --- a/crates/stack-stat-tui/tests/reviews.rs +++ b/crates/stack-stat-tui/tests/reviews.rs @@ -6,7 +6,7 @@ use stack_stat_tui::{ theme::{ColorDepth, Theme}, ui, }; -use support::{PrBuilder, TestScenario, assert_row_contains, find_body_row}; +use support::{ConfigBuilder, PrBuilder, TestScenario, assert_row_contains, find_body_row}; #[tokio::test] async fn review_column_is_yellow_text_only_when_a_review_is_required() { @@ -49,3 +49,32 @@ async fn review_column_is_yellow_text_only_when_a_review_is_required() { assert_eq!(colored(requested, "◷ pending"), theme.warning); assert_ne!(colored(none, "—"), theme.warning); } + +#[tokio::test] +async fn github_approval_by_a_hidden_bot_is_not_masked_by_pending_requests() { + let (mut app, _) = TestScenario::builder() + .stack("main", &["botok"]) + .pr( + "botok", + PrBuilder::new(1) + .ready() + .approved() + .bot_review("copilot-pull-request-reviewer") + .pending_review(), + ) + .config(ConfigBuilder::new().show_bots(false)) + .current_branch("botok") + .build() + .into_app(); + app.poll_once(Source::All).await; + + let theme = Theme::for_depth(ColorDepth::TrueColor); + let mut terminal = Terminal::new(TestBackend::new(120, 40)).unwrap(); + terminal + .draw(|frame| ui::render(frame, &app, &theme)) + .unwrap(); + let buffer = terminal.backend().buffer(); + + let row = find_body_row(buffer, "#1").unwrap(); + assert_row_contains(buffer, row, &["#1", "✔ approved (bot)"]); +} diff --git a/crates/stack-stat-tui/tests/support/mod.rs b/crates/stack-stat-tui/tests/support/mod.rs index 958f47b..94c6f27 100644 --- a/crates/stack-stat-tui/tests/support/mod.rs +++ b/crates/stack-stat-tui/tests/support/mod.rs @@ -37,6 +37,10 @@ impl ConfigBuilder { self.0.reviews.bots.push(name.into()); self } + pub fn show_bots(mut self, show: bool) -> Self { + self.0.reviews.show_bots = show; + self + } pub fn gate_on_unresolved_threads(mut self, gate: bool) -> Self { self.0.reviews.gate_on_unresolved_threads = gate; self