diff --git a/crates/cli/src/cli_report.rs b/crates/cli/src/cli_report.rs index 7b3acadc3..0cae114da 100644 --- a/crates/cli/src/cli_report.rs +++ b/crates/cli/src/cli_report.rs @@ -7,7 +7,9 @@ //! re-rendering from a saved envelope is a recorded follow-up. Dispatch is on //! the envelope's `kind` field, so any envelope produced by `--format json` //! (dead-code, dupes, health, audit, security, or the bare combined run) -//! renders byte-identically to the direct `--format` run. +//! renders byte-identically to the direct `--format` run. The `fallow fix` +//! envelope carries no `kind`; it is detected by its top-level fields and +//! rendered via [`EnvelopeKind::Fix`]. use std::path::Path; use std::process::ExitCode; @@ -76,6 +78,13 @@ fn envelope_kind( output: OutputFormat, ) -> Result { let Some(kind) = envelope.get("kind").and_then(serde_json::Value::as_str) else { + // The `fallow fix --format json` envelope is the only kind-less document + // fallow emits (crates/output/src/fix.rs: no top-level `kind`). Resolve it + // by field detection so `report --from ` renders the fix + // job summary natively; genuinely unrecognized documents keep erroring. + if is_fix_envelope(envelope) { + return Ok(EnvelopeKind::Fix); + } return Err(crate::emit_known_failure( &format!( "{} is not a fallow results envelope (missing top-level `kind`); \ @@ -102,8 +111,8 @@ fn envelope_kind( } /// Map the `--format json` root `kind` onto the renderer dispatch. The fix -/// envelope has no `kind` field, so fix output is not re-renderable here (use -/// `fallow fix --format github-summary` directly). +/// envelope has no `kind` field; it is resolved separately via +/// [`is_fix_envelope`] field detection (see [`envelope_kind`]). fn parse_envelope_kind(kind: &str) -> Option { match kind { "dead-code" => Some(EnvelopeKind::DeadCode), @@ -116,6 +125,20 @@ fn parse_envelope_kind(kind: &str) -> Option { } } +/// Recognize a kind-less `fallow fix --format json` envelope by its stable +/// top-level keys. The fix root always carries both a `fixes` array and a +/// numeric `total_fixed` (see `crates/output/src/fix.rs::FixJsonOutput`); no +/// other fallow envelope is kind-less, so the two keys together are an +/// unambiguous signal. +fn is_fix_envelope(envelope: &serde_json::Value) -> bool { + envelope + .get("fixes") + .is_some_and(serde_json::Value::is_array) + && envelope + .get("total_fixed") + .is_some_and(serde_json::Value::is_number) +} + #[cfg(test)] mod tests { use super::*; @@ -145,4 +168,34 @@ mod tests { assert_eq!(parse_envelope_kind("feature-flags"), None); assert_eq!(parse_envelope_kind(""), None); } + + #[test] + fn is_fix_envelope_detects_kindless_fix_document() { + let fix = serde_json::json!({ + "dry_run": false, + "total_fixed": 3, + "skipped": 0, + "fixes": [{ "type": "remove_export", "applied": true }], + }); + assert!(is_fix_envelope(&fix)); + } + + #[test] + fn is_fix_envelope_rejects_other_kindless_documents() { + // A dead-code envelope stripped of its `kind` must NOT masquerade as + // fix: it has neither `fixes` nor `total_fixed`. + assert!(!is_fix_envelope(&serde_json::json!({ + "total_issues": 4, + "unused_files": [{ "path": "src/a.ts" }], + }))); + // `fixes` alone (no `total_fixed`) is not enough. + assert!(!is_fix_envelope(&serde_json::json!({ "fixes": [] }))); + // `total_fixed` alone (no `fixes` array) is not enough. + assert!(!is_fix_envelope(&serde_json::json!({ "total_fixed": 0 }))); + // A `fixes` value that is not an array is rejected. + assert!(!is_fix_envelope(&serde_json::json!({ + "fixes": "nope", + "total_fixed": 0, + }))); + } } diff --git a/crates/cli/src/report/github_annotations.rs b/crates/cli/src/report/github_annotations.rs index 8b90db5ad..503655f4f 100644 --- a/crates/cli/src/report/github_annotations.rs +++ b/crates/cli/src/report/github_annotations.rs @@ -32,6 +32,10 @@ pub enum EnvelopeKind { Audit, Combined, Security, + /// The `fallow fix --format json` envelope. It carries no top-level `kind` + /// field (see `crates/output/src/fix.rs`), so `fallow report --from` + /// detects it by its stable top-level keys rather than a `kind` string. + Fix, } /// Render and print the annotation stream for one envelope, resolving the @@ -84,6 +88,10 @@ fn collect_annotations( collect_value_section(envelope, "health", &mut out, collect_health); collect_value_section(envelope, "dupes", &mut out, collect_dupes); } + // The bundled action no-ops annotations for the fix command + // (`action/scripts/annotate.sh` skips fix), so the native renderer + // matches by emitting nothing. + EnvelopeKind::Fix => {} } out } diff --git a/crates/cli/src/report/github_summary.rs b/crates/cli/src/report/github_summary.rs index e740fae82..1cbee7449 100644 --- a/crates/cli/src/report/github_summary.rs +++ b/crates/cli/src/report/github_summary.rs @@ -70,8 +70,10 @@ pub fn print_summary(kind: EnvelopeKind, envelope: &Value, root: &Path) -> ExitC ExitCode::SUCCESS } -/// Render the fix envelope's job summary (the fix envelope has no `kind` -/// field, so it does not route through [`EnvelopeKind`]). +/// Render the fix envelope's job summary directly from `fallow fix`. The fix +/// envelope has no `kind` field; `fallow report --from` reaches the same +/// renderer via [`EnvelopeKind::Fix`] (resolved by field detection), while the +/// live `fallow fix` command calls this entry point. pub fn print_fix_summary(envelope: &Value) -> ExitCode { outln!("{}", render_fix_summary(envelope)); ExitCode::SUCCESS @@ -87,6 +89,7 @@ pub fn render_summary(kind: EnvelopeKind, envelope: &Value, links: &LinkContext) EnvelopeKind::Audit => render_audit_summary(envelope), EnvelopeKind::Security => render_security_summary(envelope), EnvelopeKind::Combined => render_combined_summary(envelope, links), + EnvelopeKind::Fix => render_fix_summary(envelope), } } diff --git a/crates/cli/tests/github_format_tests.rs b/crates/cli/tests/github_format_tests.rs index 405b79a7c..7e5c61f20 100644 --- a/crates/cli/tests/github_format_tests.rs +++ b/crates/cli/tests/github_format_tests.rs @@ -545,6 +545,7 @@ fn report_from_round_trip_parity() { (EnvelopeKind::Audit, audit_envelope()), (EnvelopeKind::Security, security_envelope()), (EnvelopeKind::Combined, combined_envelope()), + (EnvelopeKind::Fix, fix_envelope()), ] { let direct = render_annotations(kind, &envelope, &plain_options()); let serialized = serde_json::to_string_pretty(&envelope).expect("serialize"); @@ -650,6 +651,68 @@ fn github_summary_fix_snapshot() { insta::assert_snapshot!("github_summary_fix", rendered); } +/// `report --from --format github-summary` routes the +/// kind-less fix envelope through `EnvelopeKind::Fix`, which must reach the +/// same renderer as the live `fallow fix` command (`render_fix_summary`) and +/// carry `summary-fix.jq`'s sections. +#[test] +fn render_summary_fix_mirrors_render_fix_summary() { + let env = fix_envelope(); + let via_dispatch = render_summary(EnvelopeKind::Fix, &env, &LinkContext::default()); + let direct = fallow_cli::report::github_summary::render_fix_summary(&env); + assert_eq!( + via_dispatch, direct, + "EnvelopeKind::Fix must reach render_fix_summary" + ); + // The summary-fix.jq sections: heading, headline, count table, details. + assert!( + via_dispatch.starts_with("## Fallow - Auto-fix"), + "{via_dispatch}" + ); + assert!( + via_dispatch.contains("**Dry run**: would apply **3 fixes**"), + "{via_dispatch}" + ); + assert!( + via_dispatch.contains("skipped 1 file(s) that changed since analysis"), + "{via_dispatch}" + ); + assert!( + via_dispatch.contains( + "kept exports in 2 file(s) where consumers may be hidden from static analysis" + ), + "{via_dispatch}" + ); + assert!( + via_dispatch.contains("| Export removals | 2 |"), + "{via_dispatch}" + ); + assert!( + via_dispatch.contains("| Dependency removals | 1 |"), + "{via_dispatch}" + ); + assert!( + via_dispatch.contains("View details"), + "{via_dispatch}" + ); + assert!( + via_dispatch.contains("- `src/api/client.ts:42` - `unusedFn`"), + "{via_dispatch}" + ); + assert!( + via_dispatch.contains("- `left-pad` from dependencies in `package.json`"), + "{via_dispatch}" + ); +} + +/// The bundled action no-ops annotations for the fix command, so the native +/// `EnvelopeKind::Fix` annotation renderer must emit nothing. +#[test] +fn render_annotations_fix_is_empty() { + let rendered = render_annotations(EnvelopeKind::Fix, &fix_envelope(), &plain_options()); + assert_eq!(rendered, "", "fix annotations must be empty: {rendered}"); +} + #[test] fn github_summary_combined_snapshot() { // Populated link context exercises the blob-URL branch of the dupes file @@ -674,6 +737,7 @@ fn summary_has_no_em_dashes() { (EnvelopeKind::Audit, audit_envelope()), (EnvelopeKind::Security, security_envelope()), (EnvelopeKind::Combined, combined_envelope()), + (EnvelopeKind::Fix, fix_envelope()), ] { let summary = render_summary(kind, &envelope, &LinkContext::default()); assert!(!summary.contains('\u{2014}'), "em dash in {kind:?} summary");