git-ents.gitmain
⌘K
foforge
commit 50d6d0c
feat: render check results and role rules through the Render trait

Config.roles had no rendering path at all - RepoMeta’s projection dropped it and the structural Render default was never called - so the Settings page had no way to show ref-push gating by role. Check results had the opposite problem: their presentation was hand-rolled across pages.rs and asciidoc.rs instead of going through render.rs like every other meta-ref type, and the checks-list row was a bare status word with no color and no exit code.

feat: add a Roles card to Settings, rendering Config.roles via Render feat: add a colored status badge shared by the checks-list row and the full recording page feat: show exit code and duration on the check-recording page feat: add a Download asciicast button and its download route fix: show the no-output notice instead of 404 for a settled check with no recording refactor: consolidate check-result and live-fragment rendering into web/render.rs Assisted-by: Claude:claude-sonnet-5

Joseph D. Carpinelli · 1 month ago

Reviews

No reviews of this commit yet — record a verdict below.

Start a review

verdict

crates/git-ents-server/src/web/mod.rs @@ -427,6 +427,9 @@ Some((&"checks", &[commit, name, "live"])) => { pages::check_live_fragment(repo, commit, name, live_runs).await } + Some((&"checks", &[commit, name, "download"])) => { + pages::check_recording_download(repo, commit, name).await + } Some((&"issues", &[])) => pages::issues_page(repo, &meta).await.into_response(), Some((&"settings", &[])) => { let auth = resolve_auth(repo, session).await;
crates/git-ents-server/src/web/pages.rs @@ -955,29 +955,15 @@ ) -> Markup { let outcome = head_run.and_then(|run| run.results.iter().find(|result| result.name == check.name)); + let href = format!("/{rel}/checks/{head}/{}", check.name); html! { div.card-row.signer-row { code.key { (check.name) } - @match outcome { - None => span.muted { "no run yet" } - Some(outcome) if outcome.recording.is_some() || is_in_progress(outcome.status) => { - a href={ "/" (rel) "/checks/" (head) "/" (check.name) } { (outcome.status.to_string()) } - } - Some(outcome) => span.muted { (outcome.status.to_string()) } - } + (super::render::check_list_row(outcome, &href)) } } } -/// Whether `status` is still on its way to a terminal outcome — the check has -/// no recording yet, but its run page has a live view worth linking to. -fn is_in_progress(status: git_ents::checks::Status) -> bool { - matches!( - status, - git_ents::checks::Status::Queued | git_ents::checks::Status::Running - ) -} - /// Find `name`'s outcome in `commit`'s latest recorded run, or `None` when /// `commit` has no run, or no result under that name. async fn latest_outcome( @@ -1017,10 +1003,11 @@ let short_commit = commit.get(..8).unwrap_or(commit); let rel = &meta.rel; - let body = if is_in_progress(outcome.status) { + let body = if super::render::is_in_progress(outcome.status) { let key = (repo.to_owned(), commit_oid, name.to_owned()); let fragment_url = format!("/{rel}/checks/{commit}/{name}/live"); - let initial = live_fragment_body(crate::checks::live_snapshot(live_runs, &key)); + let initial = + super::render::live_fragment_body(crate::checks::live_snapshot(live_runs, &key)); html! { p.shell-note { "This check is still " (outcome.status.to_string()) "; the view below updates live." @@ -1028,20 +1015,9 @@ style { (PreEscaped(crate::asciidoc::TERMINAL_VIEW_CSS)) } div #live-terminal data-live-check=(fragment_url) { (initial) } } - } else if let Some(recording) = &outcome.recording { - if crate::asciidoc::recording_has_no_output(recording) { - html! { (no_output_notice(&outcome)) } - } else { - let Some(player) = crate::asciidoc::render_recording(recording) else { - return not_found().into_response(); - }; - html! { - style { (PreEscaped(crate::asciidoc::TERMINAL_VIEW_CSS)) } - (PreEscaped(player)) - } - } } else { - return not_found().into_response(); + let download_href = format!("/{rel}/checks/{commit}/{name}/download"); + super::render::check_result_view(&outcome, &download_href) }; repo_shell( meta, @@ -1057,33 +1033,6 @@ .into_response() } -/// The best-possible-UX fallback for a settled check that produced no -/// terminal output: its exit code when the command actually ran, or just its -/// status when it didn't (a composite, or an infra failure before any command -/// started). -fn no_output_notice(outcome: &git_ents::checks::RunOutcome) -> Markup { - html! { - @match outcome.exit_code { - Some(code) => p.muted { "Check finished with exit code " code { (code) } " without output." } - None => p.muted { "This check produced no terminal output." } - } - } -} - -/// The live-terminal container's inner markup for one poll: the check's -/// current screen, rendered as a static snapshot (see -/// [`asciidoc::render_live`](crate::asciidoc::render_live)), or a placeholder -/// while output has yet to arrive. -fn live_fragment_body(recording: Option<String>) -> Markup { - let rendered = recording - .filter(|recording| !crate::asciidoc::recording_has_no_output(recording)) - .and_then(|recording| crate::asciidoc::render_live(&recording)); - match rendered { - Some(player) => html! { (PreEscaped(player)) }, - None => html! { p.muted { "Waiting for output…" } }, - } -} - /// One poll of a running check's live output — the fragment [`LIVE_SCRIPT`] /// swaps into the run page's `#live-terminal` container. Signals completion /// (the check no longer has a live buffer: it settled, or was never queued) @@ -1104,11 +1053,58 @@ let key = (repo.to_owned(), commit_oid, name.to_owned()); let recording = crate::checks::live_snapshot(live_runs, &key); let done = recording.is_none(); - let body = live_fragment_body(recording).into_string(); + let body = super::render::live_fragment_body(recording).into_string(); let header = if done { "done" } else { "running" }; ([("x-check-live", header)], body).into_response() } +/// Download a check's raw asciicast recording, for replaying outside the +/// browser (`asciinema play <file>`) or archiving. 404s under the same +/// conditions as [`check_recording_page`] (no run recorded, or none for +/// `name`), and also when the settled run has no recording to hand out. +pub(super) async fn check_recording_download(repo: &Path, commit: &str, name: &str) -> Response { + let Some(commit_oid) = ObjectId::from_hex(commit.as_bytes()).ok() else { + return not_found().into_response(); + }; + let Some(recording) = latest_outcome(repo, commit_oid, name) + .await + .and_then(|outcome| outcome.recording) + else { + return not_found().into_response(); + }; + let short_commit = commit.get(..8).unwrap_or(commit); + let filename = format!( + "{}-{}.cast", + sanitize_filename(name), + sanitize_filename(short_commit) + ); + ( + [ + ("content-type", "application/x-asciicast".to_owned()), + ( + "content-disposition", + format!("attachment; filename=\"{filename}\""), + ), + ], + recording, + ) + .into_response() +} + +/// Keep only characters safe for a `Content-Disposition` filename, so a check +/// name can't inject header syntax into the download response. +fn sanitize_filename(s: &str) -> String { + s.chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c == '-' || c == '_' { + c + } else { + '_' + } + }) + .collect() +} + /// Load the configured check set off the async runtime, since `checks::load` /// shells out to git and reads the object database synchronously. async fn load_checks(repo: &Path) -> Result<Vec<git_ents::checks::Check>, String> { @@ -1203,6 +1199,7 @@ ) -> Markup { let members = load_members(repo).await; let checks = load_checks(repo).await; + let config = load_repo_config(repo).await; repo_shell( meta, Tab::Settings, @@ -1275,11 +1272,37 @@ } } } + + div.card { + div.card-header { "Roles" } + p.shell-note { + "Ref-push gating by member role, on " code { "refs/meta/config" } + " — members join a role with " code { "git ents members add --role" } "." + } + @match &config { + Err(err) => div.card-row.muted { "Could not read config: " (err) } + Ok(config) if config.roles.is_empty() => { + div.card-row.muted { + "No roles configured — every member may push any ref." + } + } + Ok(config) => (config.render()) + } + } } }, ) } +/// Load `refs/meta/config` off the async runtime, like [`load_checks`]. +async fn load_repo_config(repo: &Path) -> Result<git_ents::config::Config, String> { + let repo = repo.to_owned(); + tokio::task::spawn_blocking(move || git_ents::config::load(&repo)) + .await + .map_err(|err| err.to_string())? + .map_err(|err| err.to_string()) +} + /// Load the member set off the async runtime, since `members::load_all` shells /// out to git and reads the object database synchronously. async fn load_members(repo: &Path) -> Result<Vec<git_ents::members::Member>, String> {
crates/git-ents-server/src/web/render.rs @@ -9,13 +9,15 @@ //! run's one-line summary — override [`Render::render`] instead. use facet::{Def, Facet, Peek, Type, UserType}; -use maud::{Markup, html}; +use maud::{Markup, PreEscaped, html}; -use git_ents::checks::{Check, Run}; -use git_ents::config::Config; +use git_ents::checks::{Check, Run, RunOutcome, Status}; +use git_ents::config::{Config, RoleRules}; use git_ents::issues::Issue; use git_ents::members::Member; +use crate::asciidoc; + /// HTML rendering for a meta-ref value. The default walks the value's [`Facet`] /// shape structurally; a type overrides [`render`](Render::render) when its /// presentation needs more than the structure carries. @@ -46,8 +48,36 @@ } } -/// Config renders structurally: each field becomes a keyed row. -impl Render for Config {} +/// Config's editable fields (description, homepage, topics) get their own +/// edit-form treatment in the settings page, so the only piece left to render +/// here is `roles` — one row per role rather than the raw map the structural +/// default would otherwise print. +impl Render for Config { + fn render(&self) -> Markup { + html! { + @for (role, rules) in &self.roles { + (role_row(role, rules)) + } + } + } +} + +/// One role's ref-push gating: its `allow`/`deny` glob lists joined for +/// display, or "no rules" for a role entry with neither (matches every ref, +/// same as no entry at all). +fn role_row(role: &str, rules: &RoleRules) -> Markup { + let mut parts = Vec::new(); + if !rules.allow.is_empty() { + parts.push(format!("allow {}", rules.allow.join(", "))); + } + if !rules.deny.is_empty() { + parts.push(format!("deny {}", rules.deny.join(", "))); + } + if parts.is_empty() { + parts.push("no rules".to_owned()); + } + row(role, &parts.join(" · ")) +} /// An issue's title leads the row, its labels render as chips beside it rather /// than the raw ` · `-joined list the structural walk would print. @@ -172,3 +202,104 @@ .collect::<Vec<_>>() .join(" · ") } + +/// Whether `status` is still on its way to a terminal outcome — the check has +/// no recording yet, but its run page has a live view worth linking to. +pub(super) fn is_in_progress(status: Status) -> bool { + matches!(status, Status::Queued | Status::Running) +} + +/// A colored status word, shared by the checks-list row and the full +/// recording page so the two agree on how a status reads: green for a pass, +/// red for a failure, muted for anything still settling or skipped. +fn status_badge(status: Status) -> Markup { + let class = match status { + Status::Pass => "status-pass", + Status::Fail | Status::Error => "status-fail", + Status::Queued | Status::Running | Status::Skipped => "status-pending", + }; + html! { span class=(class) { (status.to_string()) } } +} + +/// One check's row on the "Checks on HEAD" card: a status badge, linked to +/// `href` when there's a live view or a recording behind it, or "no run yet" +/// when `outcome` is absent (just added, or its run has not landed). +pub(super) fn check_list_row(outcome: Option<&RunOutcome>, href: &str) -> Markup { + html! { + @match outcome { + None => span.muted { "no run yet" } + Some(outcome) if outcome.recording.is_some() || is_in_progress(outcome.status) => { + a href=(href) { (status_badge(outcome.status)) } + } + Some(outcome) => (status_badge(outcome.status)) + } + } +} + +/// The full recording-page body for a settled check: a summary line (status, +/// exit code, duration) plus the terminal — a replay player, or a no-output +/// notice when there is nothing worth replaying — and, when there's a +/// recording, a link to download the raw asciicast. +pub(super) fn check_result_view(outcome: &RunOutcome, download_href: &str) -> Markup { + html! { + div.check-summary { + (status_badge(outcome.status)) + @if let Some(code) = outcome.exit_code { + span.muted { "exit code " code { (code) } } + } + @if let Some(secs) = outcome.duration_secs { + span.muted { (secs) "s" } + } + @if outcome.recording.is_some() { + a.btn-quiet href=(download_href) download { "Download asciicast" } + } + } + (settled_terminal(outcome)) + } +} + +/// The terminal for a settled check: the replay player when the recording has +/// visible output, or the exit-code notice when it doesn't (including when +/// there's no recording at all, or acdc fails to render one) — acdc's replay +/// player renders a bare empty box with no explanation otherwise. +fn settled_terminal(outcome: &RunOutcome) -> Markup { + let Some(recording) = &outcome.recording else { + return no_output_notice(outcome); + }; + if asciidoc::recording_has_no_output(recording) { + return no_output_notice(outcome); + } + match asciidoc::render_recording(recording) { + Some(player) => html! { + style { (PreEscaped(asciidoc::TERMINAL_VIEW_CSS)) } + (PreEscaped(player)) + }, + None => no_output_notice(outcome), + } +} + +/// The best-possible-UX fallback for a settled check with nothing to replay: +/// its exit code when the command actually ran, or just its status when it +/// didn't (a composite, or an infra failure before any command started). +fn no_output_notice(outcome: &RunOutcome) -> Markup { + html! { + @match outcome.exit_code { + Some(code) => p.muted { "Check finished with exit code " code { (code) } " without output." } + None => p.muted { "This check produced no terminal output." } + } + } +} + +/// The live-terminal container's inner markup for one poll: the check's +/// current screen, rendered as a static snapshot (see +/// [`asciidoc::render_live`]), or a placeholder while output has yet to +/// arrive. +pub(super) fn live_fragment_body(recording: Option<String>) -> Markup { + let rendered = recording + .filter(|recording| !asciidoc::recording_has_no_output(recording)) + .and_then(|recording| asciidoc::render_live(&recording)); + match rendered { + Some(player) => html! { (PreEscaped(player)) }, + None => html! { p.muted { "Waiting for output…" } }, + } +}
crates/git-ents-server/src/web/style.css @@ -346,6 +346,12 @@ .feature-status { flex-shrink: 0; font-family: var(--font-mono); font-size: .76rem; color: var(--color-text-muted); background: var(--color-surface); border: 1px solid var(--color-border); border-radius: var(--radius-pill); padding: .2rem .7rem; } .feature-status.on { color: var(--color-bg); background: var(--color-accent); border-color: var(--color-accent); } +.status-pass { color: var(--s-func); font-weight: 600; } +.status-fail { color: var(--s-keyword); font-weight: 600; } +.status-pending { color: var(--color-text-muted); } +.check-summary { display: flex; align-items: center; gap: .85rem; margin-bottom: 1.25rem; font-family: var(--font-mono); font-size: .9rem; } +.check-summary .btn-quiet { margin-left: auto; } + .site-footer { border-top: 1px solid var(--color-border); color: var(--color-text-muted); font-size: .8rem; margin-top: auto; } .footer-inner { max-width: var(--max-width); margin: 0 auto; padding: 2rem 1.5rem; text-align: center; } .footer-inner a { color: var(--color-text-muted); text-decoration: none; }