Skip to content

Commit 21f3979

Browse files
Haofeiclaude
andcommitted
Add markdown review render: rss pkg review --markdown (review #9)
A reviewer (or PR bot) needs more than JSON. `rss pkg review --markdown` now renders a PR-facing report: risk + reasons, a Capabilities-by-risk table (with provider and author-declared/unknown flags), the native boundary, and a diagnostics/"not valid for gating" note. - format_package_review_markdown (exported); --markdown flag on `pkg review`. - also drop the now-unused serde default on CiGateOutput.valid_for_gating (the struct is serialize-only). - test: the markdown lists capabilities high-risk first. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3d41544 commit 21f3979

5 files changed

Lines changed: 127 additions & 13 deletions

File tree

reir/src/format.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,6 @@ pub struct CiGateOutput {
186186
pub status: CiGateStatus,
187187
/// False when the evidence was derived from invalid source (error
188188
/// diagnostics): the bundle must not be trusted as a gate input.
189-
#[serde(default = "default_true")]
190189
pub valid_for_gating: bool,
191190
#[serde(default, skip_serializing_if = "Option::is_none")]
192191
pub gating_reason: Option<String>,
@@ -301,10 +300,6 @@ impl Default for CiGatePolicy {
301300
}
302301
}
303302

304-
fn default_true() -> bool {
305-
true
306-
}
307-
308303
/// Whether a fact's acquisition mode is an author declaration (the package author
309304
/// asserted it) rather than independently established evidence.
310305
fn is_author_declared(fact: &Fact) -> bool {

src/cli/package.rs

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ use rsscript::{
88
format_package_lock_json, format_package_lock_reir_json, format_package_lock_toml,
99
format_package_metadata_human, format_package_metadata_json, format_package_metadata_reir_json,
1010
format_package_publish_human, format_package_publish_json, format_package_review_human,
11-
format_package_review_json, format_package_tree_human, format_package_tree_json,
11+
format_package_review_json, format_package_review_markdown, format_package_tree_human,
12+
format_package_tree_json,
1213
format_package_tree_reir_json, format_package_vendor_human, format_package_vendor_json,
1314
format_package_vendor_reir_json, lock_package_dir, package_metadata, package_metadata_verify,
1415
package_tree, publish_package_dry_run_with_registry, review_package_dir, vendor_package_dir,
@@ -28,7 +29,11 @@ pub(crate) fn run_package(args: &[String]) -> ExitCode {
2829
match command {
2930
PackageCommand::Ci { json, path } => run_package_check(json, path),
3031
PackageCommand::Check { json, path } => run_package_check(json, path),
31-
PackageCommand::Review { json, path } => run_package_review(json, path),
32+
PackageCommand::Review {
33+
json,
34+
markdown,
35+
path,
36+
} => run_package_review(json, markdown, path),
3237
PackageCommand::Diff {
3338
json,
3439
old_path,
@@ -67,6 +72,7 @@ pub(crate) enum PackageCommand<'a> {
6772
},
6873
Review {
6974
json: bool,
75+
markdown: bool,
7076
path: &'a str,
7177
},
7278
Diff {
@@ -114,6 +120,7 @@ pub(crate) enum PackageCommand<'a> {
114120

115121
fn parse_package_args(args: &[String]) -> Result<PackageCommand<'_>, String> {
116122
let mut json = false;
123+
let mut markdown = false;
117124
let mut reir = false;
118125
let mut verify = false;
119126
let mut dry_run = false;
@@ -125,6 +132,8 @@ fn parse_package_args(args: &[String]) -> Result<PackageCommand<'_>, String> {
125132
while let Some(arg) = args.get(index) {
126133
if arg == "--json" {
127134
json = true;
135+
} else if arg == "--markdown" {
136+
markdown = true;
128137
} else if arg == "--reir" {
129138
reir = true;
130139
} else if arg == "--verify" {
@@ -183,8 +192,16 @@ fn parse_package_args(args: &[String]) -> Result<PackageCommand<'_>, String> {
183192
match (words.as_slice(), paths.as_slice()) {
184193
([], []) => Ok(PackageCommand::Check { json, path: "." }),
185194
([], [path]) => Ok(PackageCommand::Check { json, path }),
186-
(["review"], []) => Ok(PackageCommand::Review { json, path: "." }),
187-
(["review"], [path]) => Ok(PackageCommand::Review { json, path }),
195+
(["review"], []) => Ok(PackageCommand::Review {
196+
json,
197+
markdown,
198+
path: ".",
199+
}),
200+
(["review"], [path]) => Ok(PackageCommand::Review {
201+
json,
202+
markdown,
203+
path,
204+
}),
188205
(["diff"], [old_path, new_path]) => Ok(PackageCommand::Diff {
189206
json,
190207
old_path,
@@ -333,7 +350,7 @@ pub(crate) fn run_package_check(json: bool, path: &str) -> ExitCode {
333350
}
334351
}
335352

336-
fn run_package_review(json: bool, path: &str) -> ExitCode {
353+
fn run_package_review(json: bool, markdown: bool, path: &str) -> ExitCode {
337354
let review = match review_package_dir(Path::new(path)) {
338355
Ok(review) => review,
339356
Err(error) => {
@@ -342,7 +359,9 @@ fn run_package_review(json: bool, path: &str) -> ExitCode {
342359
}
343360
};
344361

345-
if json {
362+
if markdown {
363+
print!("{}", format_package_review_markdown(&review));
364+
} else if json {
346365
println!("{}", format_package_review_json(&review));
347366
} else {
348367
print!("{}", format_package_review_human(&review));
@@ -664,7 +683,7 @@ mod tests {
664683
let values = args(&["review", "--json", "package"]);
665684
let command = super::parse_package_args(&values).expect("review should parse");
666685
match command {
667-
super::PackageCommand::Review { json, path } => {
686+
super::PackageCommand::Review { json, path, .. } => {
668687
assert!(json);
669688
assert_eq!(path, "package");
670689
}

src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ pub use package::{
7777
format_package_lock_reir_json_with_path, format_package_lock_toml,
7878
format_package_metadata_human, format_package_metadata_json, format_package_metadata_reir_json,
7979
format_package_publish_human, format_package_publish_json, format_package_publish_reir_json,
80-
format_package_review_human, format_package_review_json, format_package_review_reir_diff_json,
80+
format_package_review_human, format_package_review_json, format_package_review_markdown,
81+
format_package_review_reir_diff_json,
8182
format_package_review_reir_json, format_package_tree_human, format_package_tree_json,
8283
format_package_tree_reir_json, format_package_vendor_human, format_package_vendor_json,
8384
format_package_vendor_reir_json, lock_package_dir, package_lowering_input, package_metadata,

src/package/format.rs

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,88 @@ pub fn format_package_review_human(review: &PackageReview) -> String {
222222
output
223223
}
224224

225+
/// Render a package review as a PR-facing Markdown report: the powers it needs
226+
/// (ranked by risk, with provider and author-declared/unknown flags), native
227+
/// risk, reviewable boundaries, and diagnostics.
228+
pub fn format_package_review_markdown(review: &PackageReview) -> String {
229+
use std::fmt::Write;
230+
let mut out = String::new();
231+
let _ = writeln!(
232+
out,
233+
"## RSScript review: `{}` {} — risk **{}**",
234+
review.package.name,
235+
review.package.version,
236+
package_risk_label(review.risk)
237+
);
238+
if !review.reasons.is_empty() {
239+
let _ = writeln!(out, "\n**Why:**");
240+
for reason in &review.reasons {
241+
let _ = writeln!(out, "- {reason}");
242+
}
243+
}
244+
245+
let mut capabilities = review.capabilities.clone();
246+
capabilities.sort_by_key(|capability| match capability.risk {
247+
crate::CapabilityRisk::High => 0u8,
248+
crate::CapabilityRisk::Medium => 1,
249+
crate::CapabilityRisk::Low => 2,
250+
});
251+
let mut seen = std::collections::BTreeSet::new();
252+
let mut rows = Vec::new();
253+
for capability in &capabilities {
254+
if !seen.insert((capability.category.clone(), capability.binding_symbol.clone())) {
255+
continue;
256+
}
257+
let risk = match capability.risk {
258+
crate::CapabilityRisk::High => "high",
259+
crate::CapabilityRisk::Medium => "medium",
260+
crate::CapabilityRisk::Low => "low",
261+
};
262+
let note = capability
263+
.unknown_reason
264+
.as_deref()
265+
.map(|reason| format!(" ⚠️ {reason}"))
266+
.unwrap_or_default();
267+
rows.push(format!(
268+
"| {} | {} | {} | `{}`{} |",
269+
risk,
270+
capability.category,
271+
capability.provider.as_deref().unwrap_or("—"),
272+
capability.binding_symbol,
273+
note
274+
));
275+
}
276+
if rows.is_empty() {
277+
let _ = writeln!(out, "\nNo declared capabilities.");
278+
} else {
279+
let _ = writeln!(out, "\n### Capabilities (by risk)\n");
280+
let _ = writeln!(out, "| risk | capability | provider | via |");
281+
let _ = writeln!(out, "|------|------------|----------|-----|");
282+
for row in rows {
283+
let _ = writeln!(out, "{row}");
284+
}
285+
}
286+
287+
if let Some(native) = &review.native_rust {
288+
let _ = writeln!(out, "\n### Native boundary\n");
289+
let _ = writeln!(out, "- crate: `{}`", native.crate_name.as_deref().unwrap_or("?"));
290+
let _ = writeln!(out, "- path: `{}`", native.path);
291+
}
292+
293+
let errors = review
294+
.diagnostics
295+
.iter()
296+
.filter(|d| d.severity.is_error())
297+
.count();
298+
if errors > 0 {
299+
let _ = writeln!(
300+
out,
301+
"\n### Diagnostics\n\n**{errors} error(s)** — this review is not valid for gating."
302+
);
303+
}
304+
out
305+
}
306+
225307
/// Distinct capabilities the package requires, ranked high-risk first, so a
226308
/// reviewer sees the powers (and any unrecognized ones) at a glance.
227309
fn format_package_review_capabilities_human(

tests/checker_package.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8424,3 +8424,20 @@ fn package_metadata_fails_closed_on_error_diagnostics() {
84248424
report.reasons
84258425
);
84268426
}
8427+
8428+
#[test]
8429+
fn package_review_markdown_lists_capabilities_by_risk() {
8430+
let review = review_package_dir(std::path::Path::new(
8431+
"examples/capability-review-demo/after",
8432+
))
8433+
.expect("demo review should succeed");
8434+
let markdown = rsscript::format_package_review_markdown(&review);
8435+
assert!(markdown.contains("## RSScript review:"));
8436+
assert!(markdown.contains("### Capabilities (by risk)"));
8437+
assert!(markdown.contains("network.client"));
8438+
assert!(markdown.contains("database.read"));
8439+
// high-risk capability is listed before medium.
8440+
let high = markdown.find("network.client").unwrap();
8441+
let medium = markdown.find("database.read").unwrap();
8442+
assert!(high < medium, "high-risk capability should sort first");
8443+
}

0 commit comments

Comments
 (0)