diff --git a/Cargo.lock b/Cargo.lock index eda902fe..60ed8f7a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1438,6 +1438,7 @@ dependencies = [ name = "mergify-events" version = "0.0.0" dependencies = [ + "anstyle", "chrono", "mergify-core", "mergify-test-support", diff --git a/crates/mergify-cli/src/main.rs b/crates/mergify-cli/src/main.rs index e160121d..d69aaf22 100644 --- a/crates/mergify-cli/src/main.rs +++ b/crates/mergify-cli/src/main.rs @@ -557,7 +557,7 @@ struct EventsOpts { pr_number: Option, since: Option, event_types: Vec, - limit: Option, + limit: usize, output_json: bool, } @@ -2736,7 +2736,8 @@ enum Subcommands { /// merges, commands, CI Insights, every event type — for the /// whole repository or one pull request (`--pr`). The output /// always states the time window it covers: the last 24 hours by - /// default, up to the 90 days the log retains (`--since 90d`). + /// default, up to the 90 days the log retains (`--since 90d`), and + /// the newest page of events within it (`--limit`). Events(EventsCliArgs), /// Schedule and manage merge freezes. /// @@ -4295,10 +4296,19 @@ struct EventsCliArgs { #[arg(long = "type", value_name = "EVENT_TYPE")] r#type: Vec, - /// Stop after the newest N events instead of fetching the whole - /// window. The output says so when it takes effect. - #[arg(long, value_name = "N")] - limit: Option, + /// Stop after the newest N events. The default is one API page — + /// a busy repository records thousands of events a day, so + /// fetching the whole window takes minutes. The output says when + /// the cap took effect; raise it to read further back. + #[arg( + long, + value_name = "N", + default_value_t = mergify_events::list::DEFAULT_LIMIT, + // Zero would fetch a page and then print none of it, which + // reads exactly like an empty window. + value_parser = clap::builder::RangedU64ValueParser::::new().range(1..), + )] + limit: usize, /// Emit a single JSON document (the raw events, newest first, /// with the queried window echoed) instead of the timeline. @@ -4745,14 +4755,15 @@ mod tests { opts.event_types, vec!["action.queue.leave", "command.queue"], ); - assert_eq!(opts.limit, Some(50)); + assert_eq!(opts.limit, 50); assert!(opts.output_json); } #[test] - fn events_defaults_to_repo_wide_last_24h() { + fn events_defaults_to_repo_wide_last_24h_capped_at_one_page() { // No --pr, no --since: the command covers the repository and - // the run applies (and states) the 24h default itself. + // the run applies (and states) the 24h default itself. The + // limit is not optional — an uncapped default is the bug. let parsed = parse(&["events"]); let Dispatch::Native(NativeCommand::Events(opts)) = dispatch_from_parsed(parsed) else { panic!("events must dispatch to the native Events variant"); @@ -4760,9 +4771,20 @@ mod tests { assert_eq!(opts.pr_number, None); assert_eq!(opts.since, None); assert!(opts.event_types.is_empty()); + assert_eq!(opts.limit, mergify_events::list::DEFAULT_LIMIT); assert!(!opts.output_json); } + #[test] + fn events_rejects_a_zero_limit() { + // `--limit 0` would fetch a page and print none of it, which + // is indistinguishable from an empty window. + let Err(err) = CliRoot::try_parse_from(["mergify", "events", "--limit", "0"]) else { + panic!("a zero limit must be a usage error"); + }; + assert!(err.to_string().contains('0'), "got: {err}"); + } + #[test] fn events_rejects_a_since_past_the_retention_cap() { // The impossible window dies as a usage error carrying the diff --git a/crates/mergify-cli/src/snapshots/mergify__tests__cli_schema_golden.snap b/crates/mergify-cli/src/snapshots/mergify__tests__cli_schema_golden.snap index 32c53d8b..6e1a1ee5 100644 --- a/crates/mergify-cli/src/snapshots/mergify__tests__cli_schema_golden.snap +++ b/crates/mergify-cli/src/snapshots/mergify__tests__cli_schema_golden.snap @@ -1859,14 +1859,14 @@ expression: schema ] }, { - "default": null, + "default": "100", "env": null, "global": false, - "help": "Stop after the newest N events instead of fetching the whole window. The output says so when it takes effect", + "help": "Stop after the newest N events. The default is one API page — a busy repository records thousands of events a day, so fetching the whole window takes minutes. The output says when the cap took effect; raise it to read further back", "id": "limit", "kind": "option", "long": "limit", - "longHelp": "Stop after the newest N events instead of fetching the whole window. The output says so when it takes effect", + "longHelp": "Stop after the newest N events. The default is one API page — a busy repository records thousands of events a day, so fetching the whole window takes minutes. The output says when the cap took effect; raise it to read further back", "numArgs": "1", "possibleValues": [], "required": false, @@ -1894,7 +1894,7 @@ expression: schema } ], "commands": [], - "longAbout": "Browse the events Mergify recorded for the repository.\n\nList the activity log as a timeline — queue enters and leaves, merges, commands, CI Insights, every event type — for the whole repository or one pull request (`--pr`). The output always states the time window it covers: the last 24 hours by default, up to the 90 days the log retains (`--since 90d`).", + "longAbout": "Browse the events Mergify recorded for the repository.\n\nList the activity log as a timeline — queue enters and leaves, merges, commands, CI Insights, every event type — for the whole repository or one pull request (`--pr`). The output always states the time window it covers: the last 24 hours by default, up to the 90 days the log retains (`--since 90d`), and the newest page of events within it (`--limit`).", "name": "events", "path": [ "mergify", diff --git a/crates/mergify-events/Cargo.toml b/crates/mergify-events/Cargo.toml index 4326c181..9143a268 100644 --- a/crates/mergify-events/Cargo.toml +++ b/crates/mergify-events/Cargo.toml @@ -12,6 +12,7 @@ publish = false [dependencies] mergify-core = { path = "../mergify-core" } mergify-tui = { path = "../mergify-tui" } +anstyle = { workspace = true } chrono = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } diff --git a/crates/mergify-events/src/client.rs b/crates/mergify-events/src/client.rs index 844d02fd..d51e6ea9 100644 --- a/crates/mergify-events/src/client.rs +++ b/crates/mergify-events/src/client.rs @@ -34,6 +34,22 @@ struct EventsResponse { events: Vec, } +/// The result of a [`fetch`]: the events, plus whether the limit cut +/// the log short. +/// +/// `truncated` is observed, not inferred: only the fetch knows it +/// stopped on [`Query::limit`] while the window still had more to +/// give. A caller comparing `events.len()` to the limit would call a +/// window holding exactly `limit` events truncated, and say so in an +/// output whose whole job is to state what it did not show. +#[derive(Debug)] +pub struct Log { + /// The window's events, newest first, capped at [`Query::limit`]. + pub events: Vec, + /// The limit stopped the walk with events left in the window. + pub truncated: bool, +} + /// Fetch every event matching `query`, newest first. /// /// Both window bounds are always sent (the API's silent @@ -49,15 +65,14 @@ struct EventsResponse { /// Propagates the API failure ([`CliError::MergifyApi`] for HTTP /// errors). What that means is the caller's call — `queue show` /// treats it as "could not determine", not as a command failure. -pub async fn fetch( - client: &Client, - repository: &str, - query: &Query, -) -> Result, CliError> { +pub async fn fetch(client: &Client, repository: &str, query: &Query) -> Result { let path = format!("/v1/repos/{repository}/logs"); let from = query.window.from().to_rfc3339(); let to = query.window.to().to_rfc3339(); let pull_request = query.pull_request.map(|n| n.to_string()); + // Sizing the page to the limit is what makes a limited fetch a + // single request: the default limit is one page, so the no-flag + // command asks the API once and stops. let per_page = query .limit .map_or(PER_PAGE_MAX, |limit| limit.clamp(1, PER_PAGE_MAX)) @@ -76,6 +91,7 @@ pub async fn fetch( let mut raw_events: Vec = Vec::new(); let mut cursor: Option = None; + let mut truncated = false; loop { let mut pairs = base.clone(); if let Some(cursor) = &cursor { @@ -83,14 +99,23 @@ pub async fn fetch( } let page = client.get_page::(&path, &pairs).await?; raw_events.extend(page.body.events); - if query.limit.is_some_and(|limit| raw_events.len() >= limit) { + // A server bug echoing the cursor we just used would + // otherwise loop forever. + let next = page + .next_cursor + .filter(|next| Some(next) != cursor.as_ref()); + if let Some(limit) = query.limit + && raw_events.len() >= limit + { + // Either this page overshot the limit, or the server says + // another page exists: both mean the window outlives what + // we return. + truncated = raw_events.len() > limit || next.is_some(); break; } - match page.next_cursor { - // A server bug echoing the cursor we just used would - // otherwise loop forever. - Some(next) if Some(&next) != cursor.as_ref() => cursor = Some(next), - _ => break, + match next { + Some(next) => cursor = Some(next), + None => break, } } @@ -101,7 +126,7 @@ pub async fn fetch( if let Some(limit) = query.limit { events.truncate(limit); } - Ok(events) + Ok(Log { events, truncated }) } #[cfg(test)] @@ -226,7 +251,10 @@ mod tests { window: Window::retained(at("2026-07-30T00:00:00Z")), limit: None, }; - let events = fetch(&client(&server), "owner/repo", &query).await.unwrap(); + let events = fetch(&client(&server), "owner/repo", &query) + .await + .unwrap() + .events; assert!(events.is_empty()); } @@ -277,7 +305,10 @@ mod tests { window: Window::retained(at("2026-07-30T00:00:00Z")), limit: None, }; - let events = fetch(&client(&server), "owner/repo", &query).await.unwrap(); + let events = fetch(&client(&server), "owner/repo", &query) + .await + .unwrap() + .events; let ids: Vec = events .iter() .map(|e| e.raw["id"].as_u64().unwrap()) @@ -315,8 +346,97 @@ mod tests { window: Window::retained(at("2026-07-30T00:00:00Z")), limit: Some(2), }; - let events = fetch(&client(&server), "owner/repo", &query).await.unwrap(); - assert_eq!(events.len(), 2); + let log = fetch(&client(&server), "owner/repo", &query).await.unwrap(); + assert_eq!(log.events.len(), 2); + // The server said another page exists, so the window outlives + // what we return — and the caller has to be able to say so. + assert!(log.truncated); + } + + #[tokio::test] + async fn a_window_that_fits_the_limit_exactly_is_not_truncated() { + // The reason `truncated` is observed rather than inferred + // from `events.len() == limit`: a window holding exactly the + // limit is complete, and announcing "newest 2 events" over it + // would be a lie in an output whose job is to state what it + // left out. + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/v1/repos/owner/repo/logs")) + .respond_with(ResponseTemplate::new(200).set_body_json(page_body(&[ + event(2, "2026-07-29T12:00:00Z"), + event(1, "2026-07-29T11:00:00Z"), + ]))) + .expect(1) + .mount(&server) + .await; + + let query = Query { + pull_request: None, + event_types: vec![], + window: Window::retained(at("2026-07-30T00:00:00Z")), + limit: Some(2), + }; + let log = fetch(&client(&server), "owner/repo", &query).await.unwrap(); + assert_eq!(log.events.len(), 2); + assert!(!log.truncated); + } + + #[tokio::test] + async fn a_page_overshooting_the_limit_is_truncated() { + // No `next` link, but the page held more than the caller + // asked for: the extras are dropped, and dropping them is + // exactly what `truncated` reports. + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/v1/repos/owner/repo/logs")) + .respond_with(ResponseTemplate::new(200).set_body_json(page_body(&[ + event(3, "2026-07-29T13:00:00Z"), + event(2, "2026-07-29T12:00:00Z"), + event(1, "2026-07-29T11:00:00Z"), + ]))) + .expect(1) + .mount(&server) + .await; + + let query = Query { + pull_request: None, + event_types: vec![], + window: Window::retained(at("2026-07-30T00:00:00Z")), + limit: Some(2), + }; + let log = fetch(&client(&server), "owner/repo", &query).await.unwrap(); + let ids: Vec = log + .events + .iter() + .map(|e| e.raw["id"].as_u64().unwrap()) + .collect(); + // Newest first, so the limit keeps the newest. + assert_eq!(ids, vec![3, 2]); + assert!(log.truncated); + } + + #[tokio::test] + async fn an_unlimited_fetch_is_never_truncated() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/v1/repos/owner/repo/logs")) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(page_body(&[event(1, "2026-07-29T11:00:00Z")])), + ) + .expect(1) + .mount(&server) + .await; + + let query = Query { + pull_request: None, + event_types: vec![], + window: Window::retained(at("2026-07-30T00:00:00Z")), + limit: None, + }; + let log = fetch(&client(&server), "owner/repo", &query).await.unwrap(); + assert!(!log.truncated); } #[tokio::test] @@ -343,7 +463,10 @@ mod tests { window: Window::retained(at("2026-07-30T00:00:00Z")), limit: None, }; - let events = fetch(&client(&server), "owner/repo", &query).await.unwrap(); + let events = fetch(&client(&server), "owner/repo", &query) + .await + .unwrap() + .events; let ids: Vec = events .iter() .map(|e| e.raw["id"].as_u64().unwrap()) @@ -386,7 +509,10 @@ mod tests { window: Window::retained(at("2026-07-30T00:00:00Z")), limit: None, }; - let events = fetch(&client(&server), "owner/repo", &query).await.unwrap(); + let events = fetch(&client(&server), "owner/repo", &query) + .await + .unwrap() + .events; assert_eq!(events.len(), 2); } @@ -414,7 +540,10 @@ mod tests { window: Window::retained(at("2026-07-30T00:00:00Z")), limit: None, }; - let events = fetch(&client(&server), "owner/repo", &query).await.unwrap(); + let events = fetch(&client(&server), "owner/repo", &query) + .await + .unwrap() + .events; assert_eq!(events[0].raw, raw); } diff --git a/crates/mergify-events/src/list.rs b/crates/mergify-events/src/list.rs index 4a9daa61..20f688bd 100644 --- a/crates/mergify-events/src/list.rs +++ b/crates/mergify-events/src/list.rs @@ -9,19 +9,28 @@ //! that ambiguity is the bug this command removes — and the empty //! case says so explicitly, naming the range and the retention. //! +//! What the default page costs is the other half. This is a +//! low-level query over the API — filtering is `--pr` and `--type`, +//! not a curated feed — so the only thing the default owes the caller +//! is a bound: [`DEFAULT_LIMIT`] events, which is one API page, and a +//! header that says when that bound bit. +//! //! Two output modes: //! //! - Human (default): a header naming the scope, the event count and //! the exact UTC window, then one line per event, **oldest first** //! (a timeline reads down the page), with a dim date row wherever -//! the calendar date changes. +//! the calendar date changes. Each line names the pull request it +//! belongs to, and colors its outcome. //! - `--json`: a single document with the query echoed //! (`repository`, `pull_request`, `received_from`, `received_to`), -//! `size`, and `events` — the raw API events **newest first**, -//! unknown fields intact, matching the client's ordering guarantee. +//! `size`, `truncated`, and `events` — the raw API events **newest +//! first**, unknown fields intact, matching the client's ordering +//! guarantee. use std::io::Write; +use anstyle::Style; use chrono::DateTime; use chrono::TimeDelta; use chrono::Utc; @@ -43,6 +52,18 @@ use crate::window::Window; /// applied, which is the difference that matters. const DEFAULT_SINCE: TimeDelta = TimeDelta::hours(24); +/// How many events the default invocation fetches — exactly one API +/// page. +/// +/// The window alone is not a budget: a busy repository records +/// thousands of events a day, so `--since 24h` with no cap walked +/// every page (100 at a time) before printing a line, and the no-flag +/// command took minutes. One page is one request, and — this is the +/// part that makes a default cap acceptable — the header says it is +/// the *newest* N and names `--limit` as the way past it. Nothing is +/// hidden, only deferred. +pub const DEFAULT_LIMIT: usize = 100; + pub struct ListOptions<'a> { pub repository: Option<&'a str>, pub token: Option<&'a str>, @@ -54,9 +75,9 @@ pub struct ListOptions<'a> { /// `event_type` filters, passed to the API verbatim (repeatable, /// OR semantics); empty selects every type. pub event_types: Vec, - /// Stop after the newest N events instead of fetching the whole - /// window. The header says so when it takes effect. - pub limit: Option, + /// Stop after the newest N events; the CLI defaults it to + /// [`DEFAULT_LIMIT`]. The header says so when it takes effect. + pub limit: usize, pub output_json: bool, } @@ -127,53 +148,91 @@ pub async fn run_at( pull_request: opts.pr_number, event_types: opts.event_types.clone(), window, - limit: opts.limit, + limit: Some(opts.limit), }; - let events = client::fetch(&client, &ctx.repository, &query).await?; + let log = client::fetch(&client, &ctx.repository, &query).await?; if opts.output_json { - return emit_json(output, &ctx.repository, &opts, &window, &events); + return emit_json(output, &ctx.repository, &opts, &window, &log); } let theme = Theme::detect(); - let truncated = opts.limit.is_some_and(|limit| events.len() == limit); - output.emit(&(), &mut |w: &mut dyn Write| { - render(w, &theme, &scope, &window, &events, truncated) - })?; + let timeline = Timeline { + scope: &scope, + window: &window, + events: &log.events, + truncated: log.truncated, + // Repo-wide, the pull request is the only thing telling two + // `action.queue.checks.change` lines apart. Under `--pr` the + // header already named it, so the column would be a stutter. + show_pull_request: opts.pr_number.is_none(), + }; + output.emit(&(), &mut |w: &mut dyn Write| render(w, &theme, &timeline))?; Ok(()) } /// `--json`: the query echoed back plus the raw events, newest /// first. Echoing `received_from`/`received_to` keeps the /// anti-ambiguity contract for machine consumers too — an empty -/// `events` names the window it is empty *over*. +/// `events` names the window it is empty *over*, and `truncated` says +/// whether the window holds more than `events` shows, so a script +/// reading `size` is never quietly reading a default cap. fn emit_json( output: &mut dyn Output, repository: &str, opts: &ListOptions<'_>, window: &Window, - events: &[Event], + log: &client::Log, ) -> Result<(), CliError> { - let raw: Vec<&Value> = events.iter().map(|event| &event.raw).collect(); + let raw: Vec<&Value> = log.events.iter().map(|event| &event.raw).collect(); output.emit_json_value(&serde_json::json!({ "repository": repository, "pull_request": opts.pr_number, "received_from": window.from().to_rfc3339(), "received_to": window.to().to_rfc3339(), - "size": events.len(), + "size": log.events.len(), + "truncated": log.truncated, "events": raw, }))?; Ok(()) } -fn render( +/// One rendered result: what the human renderer needs beyond the +/// events themselves. +struct Timeline<'a> { + /// The repository, or `PR #N` — whatever the header names. + scope: &'a str, + window: &'a Window, + events: &'a [Event], + /// The limit stopped the fetch with events left in the window. + truncated: bool, + /// Name each event's pull request in its own column. + show_pull_request: bool, +} + +/// Whether [`write_header`] already said everything there was to say. +#[derive(PartialEq, Eq)] +enum Header { + WasEmpty, + EventsFollow, +} + +/// The scope, the count, the window, and every caveat attached to +/// them. Each caveat states a cut this page made and the flag that +/// undoes it: the limit at the old end, the retention behind the +/// window. +fn write_header( w: &mut dyn Write, theme: &Theme, - scope: &str, - window: &Window, - events: &[Event], - truncated: bool, -) -> std::io::Result<()> { + timeline: &Timeline<'_>, +) -> std::io::Result
{ + let &Timeline { + scope, + window, + events, + truncated, + .. + } = timeline; let from = format_minute(window.from()); let to = format_minute(window.to()); @@ -190,7 +249,7 @@ fn render( R = theme.reset, )?; } - return Ok(()); + return Ok(Header::WasEmpty); } let count = if truncated { @@ -207,13 +266,47 @@ fn render( R = theme.reset, D = theme.dim, )?; + if truncated { + // The cut is at the *old* end, which — the timeline reading + // down the page — is the top of the list. Saying so where the + // cut is, and naming the flag that undoes it, is what keeps a + // default cap from reading as "that was everything". + writeln!( + w, + "{D}Older events in this window were not fetched — raise --limit.{R}", + D = theme.dim, + R = theme.reset, + )?; + } writeln!(w)?; + Ok(Header::EventsFollow) +} + +fn render(w: &mut dyn Write, theme: &Theme, timeline: &Timeline<'_>) -> std::io::Result<()> { + let &Timeline { + window, + events, + show_pull_request, + .. + } = timeline; + if write_header(w, theme, timeline)? == Header::WasEmpty { + return Ok(()); + } let type_width = events .iter() .map(|event| event.event_type().unwrap_or("?").chars().count()) .max() .unwrap_or(0); + let pr_width = if show_pull_request { + events + .iter() + .filter_map(|event| Some(pr_cell(event)?.chars().count())) + .max() + .unwrap_or(0) + } else { + 0 + }; // Oldest first: a timeline reads down the page. Date rows appear // wherever the calendar date changes — and before the first event @@ -231,30 +324,115 @@ fn render( current_date = Some(date); } let time = stamp.map_or_else(|| "--:--".to_string(), |ts| ts.format("%H:%M").to_string()); + write!(w, " {D}{time}{R}", D = theme.dim, R = theme.reset)?; + if pr_width > 0 { + match pr_cell(event) { + Some(pr) => write!( + w, + " {N}{pr: write!(w, " {blank: writeln!(w, " {D}{summary}{R}", D = theme.dim, R = theme.reset)?, + Some(summary) => writeln!( + w, + " {S}{text}{R}", + S = tone_style(theme, summary.tone), + text = summary.text, + R = theme.reset, + )?, None => writeln!(w)?, } } Ok(()) } +/// The event's pull request as it prints, or `None` for the +/// repository-level events that have none. +fn pr_cell(event: &Event) -> Option { + event.pull_request().map(|number| format!("#{number}")) +} + fn format_minute(ts: DateTime) -> String { ts.format("%Y-%m-%d %H:%M").to_string() } -/// One best-effort hint per event line, read from the metadata -/// fields the queue family is known to carry. Everything here -/// degrades: an unknown type (or a known type missing a field) just -/// renders without a summary — `--json` has the whole payload. -fn summary(event: &Event) -> Option { +/// How a summary reads. The colour *is* the classification — a +/// timeline where `success` and `failure` are the same grey makes +/// the reader parse every word to find the one that matters. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum Tone { + /// A fact with no verdict in it: a queue name, a command author. + Muted, + Good, + /// Worth a look, but not a failure — the merge queue interrupted + /// the checks, or something is still running. + Warn, + Bad, +} + +const fn tone_style(theme: &Theme, tone: Tone) -> Style { + match tone { + Tone::Muted => theme.dim, + Tone::Good => theme.green, + Tone::Warn => theme.yellow, + Tone::Bad => theme.red, + } +} + +/// One event line's trailing cell: the text and what it means. +struct Summary { + text: String, + tone: Tone, +} + +impl Summary { + fn muted(text: String) -> Self { + Self { + text, + tone: Tone::Muted, + } + } + + const fn toned(text: String, tone: Tone) -> Self { + Self { text, tone } + } +} + +/// One best-effort hint per event line, read from the metadata each +/// type is known to carry. Everything here degrades: an unknown type +/// (or a known type missing a field) just renders without a summary — +/// `--json` has the whole payload. +/// +/// Which types earn an arm is measured, not guessed. Over 500 +/// consecutive events on `Mergifyio/monorepo`, 93% were the two queue +/// state-change types below (`action.queue.change` 233, +/// `action.queue.checks.change` 232) — neither carries a pull request, +/// so without their metadata a default page is a wall of the same two +/// lines. The rest of the arms cover everything else that appeared +/// more than once: `action.label`, `action.request_reviews`, +/// `action.comment`. +fn summary(event: &Event) -> Option { + // A type this function knows about can still arrive without the + // field it knows about — from an older engine, or from a payload + // shape that changed. That is a miss, not a reason to print + // nothing: fall back to what every event has. + from_metadata(event).or_else(|| { + outcome_summary(event).or_else(|| event.trigger().map(str::to_owned).map(Summary::muted)) + }) +} + +/// The per-type arm of [`summary`]; `None` means "nothing specific to +/// say", which the caller turns into the generic fallback. +fn from_metadata(event: &Event) -> Option { let meta = event.metadata(); let text = |key: &str| { meta.get(key) @@ -263,9 +441,38 @@ fn summary(event: &Event) -> Option { .map(str::to_owned) }; let flag = |key: &str| meta.get(key).and_then(Value::as_bool).unwrap_or(false); + let count = |key: &str| meta.get(key).and_then(Value::as_u64); + // The queue-state events carry no pull request: the queue they + // moved and the depth they moved it to is all they are. + let queue_state = |unit: &str, key: &str| { + let queue = text("queue_name")?; + let n = count(key)?; + Some(Summary::muted(format!( + "{queue} · {n} {unit}{s}", + s = if n == 1 { "" } else { "s" }, + ))) + }; match event.event_type().unwrap_or_default() { - "action.queue.enter" => text("queue_name"), + "action.queue.change" => queue_state("PR", "size"), + "action.queue.checks.change" => queue_state("check", "running_checks"), + // Which labels, not that labelling happened: the trigger this + // falls back to otherwise ("Rule: label on unresolved") is the + // same string on every line the rule ever fired. + "action.label" => { + let mut parts = list_of(meta.get("added"), "+"); + parts.extend(list_of(meta.get("removed"), "-")); + (!parts.is_empty()).then(|| Summary::muted(parts.join(" "))) + } + "action.request_reviews" => { + let mut parts = list_of(meta.get("reviewers"), "@"); + parts.extend(list_of(meta.get("team_reviewers"), "@")); + (!parts.is_empty()).then(|| Summary::muted(parts.join(" "))) + } + // What was posted, first line only — a comment body is + // multi-line and this is one column of a timeline. + "action.comment" => text("message").map(|message| Summary::muted(first_line(&message))), + "action.queue.enter" => text("queue_name").map(Summary::muted), "action.queue.checks_start" => { let mut parts: Vec = Vec::new(); parts.extend(text("queue_name")); @@ -275,27 +482,83 @@ fn summary(event: &Event) -> Option { { parts.push(format!("draft PR #{draft}")); } - (!parts.is_empty()).then(|| parts.join(" · ")) + (!parts.is_empty()).then(|| Summary::muted(parts.join(" · "))) } // The abort codes riding on checks_end interrupt the checks // while the PR stays queued — naming them here is what keeps - // a reader from mistaking one for a dequeue. - "action.queue.checks_end" if flag("aborted") => text("abort_code"), + // a reader from mistaking one for a dequeue. Yellow, not red, + // carries that same distinction at a glance: red is reserved + // for the leave below, which is the one that ejected the PR. + "action.queue.checks_end" if flag("aborted") => { + text("abort_code").map(|code| Summary::toned(code, Tone::Warn)) + } "action.queue.leave" => { if flag("merged") { - Some("merged".to_string()) + Some(Summary::toned("merged".to_string(), Tone::Good)) } else { - text("dequeue_code") + // Red matches `queue show`, which paints the same + // dequeue the same way. + text("dequeue_code").map(|code| Summary::toned(code, Tone::Bad)) } } - t if t.starts_with("command.") => event.trigger().map(str::to_owned), - _ => event - .outcome() - .map(str::to_owned) - .or_else(|| event.trigger().map(str::to_owned)), + // Who ran it is the fact about a command; its outcome, which + // the fallback would prefer, is about what the command asked + // for and shows up on the action events that follow. + t if t.starts_with("command.") => event.trigger().map(str::to_owned).map(Summary::muted), + _ => None, } } +/// A metadata array of strings, each prefixed — skipping the empty +/// arrays the API sends for the half of a pair that didn't happen +/// (`{"reviewers": [], "team_reviewers": ["devs"]}`). +fn list_of(value: Option<&Value>, prefix: &str) -> Vec { + value + .and_then(Value::as_array) + .map(|items| { + items + .iter() + .filter_map(Value::as_str) + .map(|item| format!("{prefix}{item}")) + .collect() + }) + .unwrap_or_default() +} + +/// The first line, clipped to what a timeline column can hold. Counts +/// characters, not bytes: the messages this renders routinely carry +/// emoji, and a byte slice would panic mid-codepoint. +fn first_line(text: &str) -> String { + const MAX: usize = 72; + let line = text.lines().next().unwrap_or_default().trim(); + if line.chars().count() <= MAX { + return line.to_owned(); + } + let kept: String = line.chars().take(MAX - 1).collect(); + format!("{kept}…") +} + +/// The API's derived outcome, coloured by what it says. +/// +/// `neutral` is dropped rather than printed: it is the API's word +/// for "this event has no verdict", and a repo-wide timeline of +/// `action.queue.checks.change neutral` repeated twenty times is +/// exactly the noise that hid the real events. Falling through to +/// the trigger says something instead. +fn outcome_summary(event: &Event) -> Option { + let outcome = event.outcome()?; + let tone = match outcome { + "success" => Tone::Good, + "failure" | "error" | "cancelled" => Tone::Bad, + "pending" => Tone::Warn, + "neutral" => return None, + // An outcome word from a newer engine still prints, just + // without a claim about what it means. + _ => Tone::Muted, + }; + Some(Summary::toned(outcome.to_owned(), tone)) +} + #[cfg(test)] mod tests { use mergify_core::OutputMode; @@ -306,6 +569,8 @@ mod tests { use wiremock::ResponseTemplate; use wiremock::matchers::method; use wiremock::matchers::path; + use wiremock::matchers::query_param; + use wiremock::matchers::query_param_is_missing; use super::*; @@ -372,13 +637,27 @@ mod tests { .await; } - async fn run_list( - server: &MockServer, - opts_for: impl FnOnce(&str) -> (Option, Option, Vec, Option), - output_json: bool, - ) -> Captured { + /// The knobs the tests vary; everything else is fixed. `limit: + /// None` is the no-flag invocation — the CLI's own default. + #[derive(Default)] + struct Args { + pr_number: Option, + since: Option, + event_types: Vec, + limit: Option, + } + + impl Args { + fn pr(number: u64) -> Self { + Self { + pr_number: Some(number), + ..Self::default() + } + } + } + + async fn run_list(server: &MockServer, args: Args, output_json: bool) -> Captured { let api_url = server.uri(); - let (pr_number, since, event_types, limit) = opts_for(&api_url); let mut cap = if output_json { Captured::new(OutputMode::Json) } else { @@ -389,10 +668,10 @@ mod tests { repository: Some("owner/repo"), token: Some("t"), api_url: Some(&api_url), - pr_number, - since, - event_types, - limit, + pr_number: args.pr_number, + since: args.since, + event_types: args.event_types, + limit: args.limit.unwrap_or(DEFAULT_LIMIT), output_json, }, at("2026-07-30T21:00:00Z"), @@ -408,7 +687,7 @@ mod tests { let server = MockServer::start().await; arrange(&server, queue_lifecycle()).await; - let cap = run_list(&server, |_| (Some(1740), None, vec![], None), false).await; + let cap = run_list(&server, Args::pr(1740), false).await; let stdout = cap.stdout(); // The window is the whole point: an empty-looking day must // never read as an empty history. @@ -423,7 +702,7 @@ mod tests { let server = MockServer::start().await; arrange(&server, queue_lifecycle()).await; - let cap = run_list(&server, |_| (Some(1740), None, vec![], None), false).await; + let cap = run_list(&server, Args::pr(1740), false).await; let stdout = cap.stdout(); let enter = stdout.find("action.queue.enter").unwrap(); let leave = stdout.find("action.queue.leave").unwrap(); @@ -462,7 +741,7 @@ mod tests { ) .await; - let cap = run_list(&server, |_| (None, None, vec![], None), false).await; + let cap = run_list(&server, Args::default(), false).await; let stdout = cap.stdout(); // Both dates appear as rows: the window spans two dates, so a // bare `22:00` would be ambiguous. @@ -475,7 +754,7 @@ mod tests { let server = MockServer::start().await; arrange(&server, queue_lifecycle()).await; - let cap = run_list(&server, |_| (None, None, vec![], None), false).await; + let cap = run_list(&server, Args::default(), false).await; let stdout = cap.stdout(); assert!(stdout.contains("owner/repo · 5 events"), "got: {stdout}"); } @@ -487,7 +766,7 @@ mod tests { let server = MockServer::start().await; arrange(&server, vec![]).await; - let cap = run_list(&server, |_| (Some(1740), None, vec![], None), false).await; + let cap = run_list(&server, Args::pr(1740), false).await; let stdout = cap.stdout(); assert!( stdout.contains( @@ -510,7 +789,10 @@ mod tests { let cap = run_list( &server, - |_| (Some(1740), Some(TimeDelta::days(90)), vec![], None), + Args { + since: Some(TimeDelta::days(90)), + ..Args::pr(1740) + }, false, ) .await; @@ -519,19 +801,250 @@ mod tests { assert!(!stdout.contains("try --since"), "got: {stdout}"); } + /// `n` events, newest first, one a minute back from noon — enough + /// of them to outrun the default cap. + fn many(n: usize) -> Vec { + let newest = at("2026-07-30T12:00:00Z"); + (0..n) + .map(|i| { + let received_at = newest - TimeDelta::minutes(i64::try_from(i).unwrap()); + json!({ + "id": n - i, + "type": "action.merge", + "received_at": received_at.to_rfc3339(), + "pull_request": 1700 + i, + "outcome": "success", + }) + }) + .collect() + } + #[tokio::test] async fn human_header_says_newest_when_the_limit_bites() { // A silent cap would read as "that's everything"; the header // must say the list was cut. let server = MockServer::start().await; - let events: Vec = queue_lifecycle().into_iter().take(2).collect(); - arrange(&server, events).await; + arrange(&server, queue_lifecycle()).await; - let cap = run_list(&server, |_| (Some(1740), None, vec![], Some(2)), false).await; + let cap = run_list( + &server, + Args { + limit: Some(2), + ..Args::pr(1740) + }, + false, + ) + .await; let stdout = cap.stdout(); assert!(stdout.contains("newest 2 events"), "got: {stdout}"); } + #[tokio::test] + async fn human_truncation_names_the_flag_that_undoes_it() { + // Saying "newest 100" without saying how to see the rest + // leaves the reader with a truthful dead end. This is also + // the no-flag path: one page over, one page printed. + let server = MockServer::start().await; + arrange(&server, many(DEFAULT_LIMIT + 5)).await; + + let cap = run_list(&server, Args::default(), false).await; + let stdout = cap.stdout(); + assert!( + stdout.contains(&format!("newest {DEFAULT_LIMIT} events")), + "got: {stdout}", + ); + assert!(stdout.contains("Older events"), "got: {stdout}"); + assert!(stdout.contains("--limit"), "got: {stdout}"); + assert_eq!(stdout.matches("action.merge").count(), DEFAULT_LIMIT); + } + + #[tokio::test] + async fn human_says_nothing_about_truncation_when_the_window_fit() { + // The window held fewer events than the cap: claiming a cut + // would be as wrong as hiding one. + let server = MockServer::start().await; + arrange(&server, queue_lifecycle()).await; + + let cap = run_list(&server, Args::default(), false).await; + let stdout = cap.stdout(); + assert!(stdout.contains("owner/repo · 5 events"), "got: {stdout}"); + assert!(!stdout.contains("Older events"), "got: {stdout}"); + assert!(!stdout.contains("newest"), "got: {stdout}"); + } + + #[tokio::test] + async fn the_default_invocation_asks_for_one_page_and_stops() { + // The bug: no flags meant no limit, so a busy repository's + // 24 hours was walked page by page before anything printed. + // The default is one page — `per_page=100` and `expect(1)` + // are the assertion, whatever window is left unread. + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/v1/repos/owner/repo/logs")) + .and(query_param("per_page", DEFAULT_LIMIT.to_string().as_str())) + .and(query_param_is_missing("cursor")) + .respond_with( + ResponseTemplate::new(200) + .insert_header("link", "; rel=\"next\"") + .set_body_json(json!({ + "size": DEFAULT_LIMIT, + "per_page": DEFAULT_LIMIT, + "events": many(DEFAULT_LIMIT), + })), + ) + .expect(1) + .mount(&server) + .await; + + let cap = run_list(&server, Args::default(), false).await; + assert!( + cap.stdout() + .contains(&format!("newest {DEFAULT_LIMIT} events")), + "got: {}", + cap.stdout(), + ); + } + + #[tokio::test] + async fn a_limit_past_one_page_keeps_walking() { + // `--limit N` is the only way past the default, so it has to + // reach beyond the single page the default stops at: pages + // stay capped at the API's 100, and the walk continues until + // the limit or the window runs out. + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/v1/repos/owner/repo/logs")) + .and(query_param("per_page", "100")) + .and(query_param_is_missing("cursor")) + .respond_with( + ResponseTemplate::new(200) + .insert_header("link", "; rel=\"next\"") + .set_body_json(json!({"size": 1, "per_page": 100, "events": [json!({ + "id": 2, + "type": "action.merge", + "received_at": "2026-07-30T12:00:00Z", + "outcome": "success", + })]})), + ) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/v1/repos/owner/repo/logs")) + .and(query_param("cursor", "c2")) + .respond_with(ResponseTemplate::new(200).set_body_json( + json!({"size": 1, "per_page": 100, "events": [json!({ + "id": 1, + "type": "action.merge", + "received_at": "2026-07-30T11:00:00Z", + "outcome": "success", + })]}), + )) + .expect(1) + .mount(&server) + .await; + + let cap = run_list( + &server, + Args { + limit: Some(DEFAULT_LIMIT * 2), + ..Args::default() + }, + false, + ) + .await; + let stdout = cap.stdout(); + // Both pages walked, and nothing claims a cut. + assert!(stdout.contains("2 events"), "got: {stdout}"); + assert!(!stdout.contains("Older events"), "got: {stdout}"); + } + + #[tokio::test] + async fn human_repo_wide_lines_name_their_pull_request() { + // Twenty `action.label` lines are the same line until each + // says whose it is. + let server = MockServer::start().await; + arrange( + &server, + vec![ + json!({ + "id": 2, + "type": "action.label", + "received_at": "2026-07-30T12:00:00Z", + "pull_request": 1801, + }), + json!({ + "id": 1, + "type": "action.label", + "received_at": "2026-07-30T11:00:00Z", + "pull_request": 1740, + }), + ], + ) + .await; + + let cap = run_list(&server, Args::default(), false).await; + let stdout = cap.stdout(); + assert!(stdout.contains("#1740"), "got: {stdout}"); + assert!(stdout.contains("#1801"), "got: {stdout}"); + } + + #[tokio::test] + async fn human_pr_scope_leaves_the_pull_request_column_out() { + // The header already said `PR #1740`; a column repeating it + // on every line is noise. + let server = MockServer::start().await; + arrange(&server, queue_lifecycle()).await; + + let cap = run_list(&server, Args::pr(1740), false).await; + let stdout = cap.stdout(); + assert!(stdout.contains("PR #1740 · 5 events"), "got: {stdout}"); + let body = stdout.split_once("UTC\n").unwrap().1; + assert!(!body.contains("#1740"), "got: {stdout}"); + } + + #[tokio::test] + async fn human_holds_the_column_for_a_repository_level_event() { + // A freeze belongs to no pull request: the column stays, + // empty, so the ones that have a PR still line up. + let server = MockServer::start().await; + arrange( + &server, + vec![ + json!({ + "id": 2, + "type": "action.queue.enter", + "received_at": "2026-07-30T12:00:00Z", + "pull_request": 1740, + "metadata": {"queue_name": "default"}, + }), + json!({ + "id": 1, + "type": "queue.freeze.create", + "received_at": "2026-07-30T11:00:00Z", + "metadata": {}, + }), + ], + ) + .await; + + let cap = run_list(&server, Args::default(), false).await; + let stdout = cap.stdout(); + let freeze = stdout + .lines() + .find(|line| line.contains("queue.freeze.create")) + .unwrap(); + let enter = stdout + .lines() + .find(|line| line.contains("action.queue.enter")) + .unwrap(); + assert_eq!( + freeze.find("queue.freeze.create"), + enter.find("action.queue.enter"), + "the type column must start at the same offset: {stdout}", + ); + } + #[tokio::test] async fn human_survives_an_unknown_event_type() { let server = MockServer::start().await; @@ -546,7 +1059,7 @@ mod tests { ) .await; - let cap = run_list(&server, |_| (None, None, vec![], None), false).await; + let cap = run_list(&server, Args::default(), false).await; let stdout = cap.stdout(); assert!(stdout.contains("something.from.2027"), "got: {stdout}"); assert!(stdout.contains("14:00"), "got: {stdout}"); @@ -558,7 +1071,7 @@ mod tests { let events = queue_lifecycle(); arrange(&server, events.clone()).await; - let cap = run_list(&server, |_| (Some(1740), None, vec![], None), true).await; + let cap = run_list(&server, Args::pr(1740), true).await; let parsed: Value = serde_json::from_str(&cap.stdout()).unwrap(); assert_eq!( parsed, @@ -568,11 +1081,24 @@ mod tests { "received_from": "2026-07-29T21:00:00+00:00", "received_to": "2026-07-30T21:00:00+00:00", "size": 5, + "truncated": false, "events": events, }), ); } + #[tokio::test] + async fn json_says_when_the_default_limit_cut_the_window() { + // A script reading `size` must be able to tell "that was the + // window" from "that was the cap". + let server = MockServer::start().await; + arrange(&server, many(DEFAULT_LIMIT + 5)).await; + + let cap = run_list(&server, Args::default(), true).await; + let parsed: Value = serde_json::from_str(&cap.stdout()).unwrap(); + assert_eq!(parsed["truncated"], json!(true)); + } + #[tokio::test] async fn json_keeps_unknown_fields_intact() { let server = MockServer::start().await; @@ -584,7 +1110,7 @@ mod tests { }); arrange(&server, vec![raw.clone()]).await; - let cap = run_list(&server, |_| (None, None, vec![], None), true).await; + let cap = run_list(&server, Args::default(), true).await; let parsed: Value = serde_json::from_str(&cap.stdout()).unwrap(); assert_eq!(parsed["events"][0], raw); } @@ -594,7 +1120,7 @@ mod tests { let server = MockServer::start().await; arrange(&server, vec![]).await; - let cap = run_list(&server, |_| (None, None, vec![], None), true).await; + let cap = run_list(&server, Args::default(), true).await; let parsed: Value = serde_json::from_str(&cap.stdout()).unwrap(); assert_eq!(parsed["size"], json!(0)); assert_eq!(parsed["events"], json!([])); @@ -602,6 +1128,248 @@ mod tests { assert_eq!(parsed["pull_request"], json!(null)); } + /// `render` with colors forced on. `Theme::detect` reports + /// disabled until the CLI entry point records a `--color` choice, + /// which no test does, so the styled branch needs driving + /// explicitly — and what it emits is the whole point of this + /// change. + fn render_in_color(events: Vec) -> String { + let events: Vec = events.into_iter().map(Event::from_raw).collect(); + let window = Window::last(DEFAULT_SINCE, at("2026-07-30T21:00:00Z")).unwrap(); + let mut out: Vec = Vec::new(); + render( + &mut out, + &Theme::new(true), + &Timeline { + scope: "owner/repo", + window: &window, + events: &events, + truncated: false, + show_pull_request: true, + }, + ) + .unwrap(); + String::from_utf8(out).unwrap() + } + + /// The SGR sequence `style` opens with, as it lands in the output. + fn code(style: Style) -> String { + format!("{style}") + } + + #[test] + fn success_and_failure_do_not_look_alike() { + // The complaint: everything grey, so the one word that + // matters reads like all the others. + let theme = Theme::new(true); + let stdout = render_in_color(vec![ + json!({ + "id": 2, + "type": "action.merge", + "received_at": "2026-07-30T12:00:00Z", + "pull_request": 1740, + "outcome": "success", + }), + json!({ + "id": 1, + "type": "action.queue.checks_end", + "received_at": "2026-07-30T11:00:00Z", + "pull_request": 1740, + "outcome": "failure", + "metadata": {"aborted": false}, + }), + ]); + assert!( + stdout.contains(&format!("{}failure", code(theme.red))), + "got: {stdout:?}", + ); + assert!( + stdout.contains(&format!("{}success", code(theme.green))), + "got: {stdout:?}", + ); + } + + #[test] + fn the_queue_verdicts_carry_their_own_colors() { + // The distinction the timeline exists to make readable: an + // abort left the PR queued (yellow), a dequeue ejected it + // (red, as `queue show` paints it), a merge is the good end. + let theme = Theme::new(true); + let stdout = render_in_color(vec![ + json!({ + "id": 3, + "type": "action.queue.leave", + "received_at": "2026-07-30T13:00:00Z", + "pull_request": 1740, + "metadata": {"merged": true}, + }), + json!({ + "id": 2, + "type": "action.queue.leave", + "received_at": "2026-07-30T12:00:00Z", + "pull_request": 1741, + "metadata": {"merged": false, "dequeue_code": "CHECKS_FAILED"}, + }), + json!({ + "id": 1, + "type": "action.queue.checks_end", + "received_at": "2026-07-30T11:00:00Z", + "pull_request": 1742, + "metadata": {"aborted": true, "abort_code": "PR_AHEAD_DEQUEUED"}, + }), + ]); + assert!( + stdout.contains(&format!("{}PR_AHEAD_DEQUEUED", code(theme.yellow))), + "got: {stdout:?}", + ); + assert!( + stdout.contains(&format!("{}CHECKS_FAILED", code(theme.red))), + "got: {stdout:?}", + ); + assert!( + stdout.contains(&format!("{}merged", code(theme.green))), + "got: {stdout:?}", + ); + } + + #[test] + fn the_pull_request_column_reads_as_an_identifier() { + // Cyan `#N` is what `queue status` already uses for a pull + // request; the timeline must not invent a second convention. + let theme = Theme::new(true); + let stdout = render_in_color(vec![json!({ + "id": 1, + "type": "action.queue.enter", + "received_at": "2026-07-30T12:00:00Z", + "pull_request": 1740, + "metadata": {"queue_name": "default"}, + })]); + assert!( + stdout.contains(&format!("{}#1740", code(theme.cyan))), + "got: {stdout:?}", + ); + } + + #[test] + fn a_neutral_outcome_yields_to_the_trigger() { + // `neutral` is the API saying "no verdict here". Printing it + // is what turned the repo-wide view into a column of the same + // word; the trigger at least says who did it. + let event = Event::from_raw(json!({ + "type": "action.label", + "outcome": "neutral", + "trigger": "Rule: automatic label", + })); + let cell = summary(&event).unwrap(); + assert_eq!(cell.text, "Rule: automatic label"); + assert_eq!(cell.tone, Tone::Muted); + + // Nothing to fall back to: no summary beats a bare `neutral`. + let bare = Event::from_raw(json!({"type": "action.label", "outcome": "neutral"})); + assert!(summary(&bare).is_none()); + } + + #[test] + fn the_queue_state_events_report_the_queue_they_moved() { + // 93% of a busy repository's default page: no pull request, + // `neutral` outcome, and the same trigger on every one. The + // depth is the only thing that changes between them, so it is + // the only thing worth printing. + let change = Event::from_raw(json!({ + "type": "action.queue.change", + "outcome": "neutral", + "trigger": "merge queue internal", + "metadata": {"queue_name": "default", "size": 3}, + })); + assert_eq!(summary(&change).unwrap().text, "default · 3 PRs"); + + let checks = Event::from_raw(json!({ + "type": "action.queue.checks.change", + "outcome": "neutral", + "trigger": "merge queue internal", + "metadata": {"queue_name": "default", "running_checks": 1}, + })); + assert_eq!(summary(&checks).unwrap().text, "default · 1 check"); + } + + #[test] + fn a_label_event_names_the_labels() { + // "Rule: label on unresolved" is the same string on every + // line the rule ever fired; the label is what differs. + let event = Event::from_raw(json!({ + "type": "action.label", + "outcome": "neutral", + "trigger": "Rule: label on unresolved", + "metadata": {"added": ["conflict"], "removed": ["review threads unresolved"]}, + })); + assert_eq!( + summary(&event).unwrap().text, + "+conflict -review threads unresolved", + ); + } + + #[test] + fn a_review_request_names_the_reviewers() { + // The API sends the empty half of the pair, and an empty list + // must not print as a stray separator. + let event = Event::from_raw(json!({ + "type": "action.request_reviews", + "outcome": "neutral", + "metadata": {"reviewers": [], "team_reviewers": ["devs"]}, + })); + assert_eq!(summary(&event).unwrap().text, "@devs"); + } + + #[test] + fn a_comment_event_quotes_its_first_line() { + let event = Event::from_raw(json!({ + "type": "action.comment", + "outcome": "neutral", + "metadata": {"message": "@kozlek this pull request is now in conflict 😩\n\nsecond line"}, + })); + assert_eq!( + summary(&event).unwrap().text, + "@kozlek this pull request is now in conflict 😩", + ); + } + + #[test] + fn a_long_comment_is_clipped_on_a_character_boundary() { + // Real comment bodies carry emoji: clipping by bytes would + // panic mid-codepoint, and a timeline column has to end + // somewhere. + let event = Event::from_raw(json!({ + "type": "action.comment", + "metadata": {"message": "😩".repeat(200)}, + })); + let text = summary(&event).unwrap().text; + assert_eq!(text.chars().count(), 72); + assert!(text.ends_with('…'), "got: {text}"); + } + + #[test] + fn a_known_type_missing_its_metadata_still_falls_back() { + // An older engine, or a payload shape that moved: a taught + // type with nothing to read must degrade to what every event + // has, not print an empty column. + let event = Event::from_raw(json!({ + "type": "action.label", + "outcome": "neutral", + "trigger": "Rule: automatic label", + })); + assert_eq!(summary(&event).unwrap().text, "Rule: automatic label"); + } + + #[test] + fn an_unknown_outcome_still_prints() { + // A verdict word from a newer engine renders, uncolored — + // this CLI does not get to claim it knows what it means. + let event = Event::from_raw(json!({"type": "action.new", "outcome": "quarantined"})); + let cell = summary(&event).unwrap(); + assert_eq!(cell.text, "quarantined"); + assert_eq!(cell.tone, Tone::Muted); + } + #[test] fn parse_since_reads_the_documented_units() { assert_eq!(parse_since("45s").unwrap(), TimeDelta::seconds(45)); diff --git a/crates/mergify-events/src/queue_leave.rs b/crates/mergify-events/src/queue_leave.rs index a798b050..3e05d657 100644 --- a/crates/mergify-events/src/queue_leave.rs +++ b/crates/mergify-events/src/queue_leave.rs @@ -180,8 +180,8 @@ pub async fn fetch_last( // PR's latest exit. limit: Some(1), }; - let events = crate::client::fetch(client, repository, &query).await?; - match events.into_iter().next() { + let log = crate::client::fetch(client, repository, &query).await?; + match log.events.into_iter().next() { Some(event) => LastLeave::from_event(event).map(Some), None => Ok(None), }