WIP: cmd_doctor returns Result<i32> but never fails

adr.md

@@ -0,0 +1,58 @@
+# ADR-0534: Add #[must_use] to render_sensor_report()
+
+## Status
+Proposed
+
+## Context
+
+The `render_sensor_report()` function in `crates/diffguard-core/src/sensor.rs` returns a `SensorReport` containing critical sensor data (findings, verdict, run metadata, and artifacts). This function is part of the R2 Library Contract for Cockpit ecosystem integration — it is exported publicly from `diffguard-core` and used as the primary data contract for the sensor system.
+
+Unlike other render functions that return `String` (where discarding the result produces visible empty output), `render_sensor_report()` returns a structured `SensorReport`. If a caller discards this value, the entire sensor report is silently lost with no compiler warning. In a governance/sensor system where findings, verdict status, and run metadata are critical, silent data loss is a serious failure mode.
+
+The codebase already uses `#[must_use]` extensively on similar functions:
+- `diffguard-diff/src/unified.rs` — `is_submodule()`, `is_added()`, etc.
+- `diffguard-domain/src/suppression.rs` — `suppress()` variants
+- `diffguard-domain/src/overrides.rs` — `override_finding()`
+- `diffguard-types/src/lib.rs` — various type methods
+- `diffguard-analytics/src/lib.rs` — multiple functions
+
+## Decision
+
+Add `#[must_use]` attribute to the `render_sensor_report()` function in `crates/diffguard-core/src/sensor.rs` at line 44.
+
+```rust
+/// Renders a CheckReceipt as a SensorReport.
+#[must_use]
+pub fn render_sensor_report(receipt: &CheckReceipt, ctx: &SensorReportContext) -> SensorReport {
+```
+
+## Consequences
+
+### Benefits
+- **Compile-time enforcement against silent data loss**: Any caller that discards the `SensorReport` will now receive a compiler warning, making the bug immediately visible rather than silently failing.
+- **Consistency with codebase conventions**: The attribute aligns with the established pattern of `#[must_use]` usage across the monorepo.
+- **Strengthens the R2 Library Contract**: The `SensorReport` is explicitly the stable integration surface for Cockpit/BusyBox consumers. Making it a compile-time error to discard the result reinforces this contract.
+- **Purely additive change**: No runtime behavior change. No existing correct code is affected.
+
+### Tradeoffs/Risks
+- **Potential warnings in existing code**: If any existing code (inside or outside `diffguard-core`) calls `render_sensor_report()` and discards the result, it will now produce a compiler warning. However, this only exposes pre-existing bugs — such code was already silently losing sensor data.
+- **CI lint gates**: If other work items or CI pipelines have `#[deny(warnings)]`, newly warned code could cause failures. However, this is the intended behavior — those callers need to be fixed.
+
+### No impact on:
+- `render_sensor_json()` — returns `Result<String, serde_json::Error>`, which has implicit `#[must_use]` on `Result` in Rust 2018+.
+- Runtime behavior — `#[must_use]` is purely compile-time.
+- Backward compatibility — only affects code that was already incorrect.
+
+## Alternatives Considered
+
+### 1. Do nothing (leave without #[must_use])
+- **Rejected because**: Silent data loss remains a latent defect. As Cockpit ecosystem integration grows, more callers might emerge that accidentally discard the result, and the bug would only be discovered when sensor dashboards go blank or integration tests silently pass with no findings.
+
+### 2. Add #[must_use] to multiple render_* functions
+- **Rejected because**: The issue specifically calls out `render_sensor_report()`. Other render functions (`render_csv_for_receipt()`, `render_junit_for_receipt()`) return `String`, which produces visible empty output if discarded. `render_sensor_report()` returns a structured `SensorReport` with no visible indication when discarded — a qualitatively different failure mode.
+
+### 3. Document the requirement in code comments only
+- **Rejected because**: Documentation can be ignored. The `#[must_use]` attribute provides compile-time enforcement that comments cannot.
+
+## Dependencies
+- None. The change is self-contained and does not require any other work items or external dependencies.

crates/diffguard-domain/src/evaluate.rs

@@ -574,12 +574,28 @@ fn trim_snippet(s: &str) -> String {
     out
 }
 
+/// Extracts a substring from `s` in the range `[start, end)`, with bounds clamping.
+///
+/// `end` is first clamped to `s.len()`, then `start` is clamped to the
+/// adjusted `end`. This guarantees `start <= end <= s.len()`, making the
+/// range always valid for direct indexing.
+///
+/// Returns the substring as a new `String`.
 fn safe_slice(s: &str, start: usize, end: usize) -> String {
+    // Clamp end first, then clamp start to the adjusted end.
+    // After these two lines: start <= end <= s.len(), so the range is always valid.
     let end = end.min(s.len());
     let start = start.min(end);
     s.get(start..end).unwrap_or("").to_string()
 }
 
+/// Converts a byte index to a 1-based column number (character count).
+///
+/// Returns `None` if `byte_idx` exceeds the string length, otherwise returns
+/// the number of characters in `s[..byte_idx]` plus one (to get 1-based column).
+///
+/// Uses direct slicing `s[..byte_idx]` because the guard on line 590 guarantees
+/// `byte_idx <= s.len()`, making the range always valid.
 fn byte_to_column(s: &str, byte_idx: usize) -> Option<usize> {
     if byte_idx > s.len() {
         return None;

crates/diffguard/tests/green_tests_work_d4a75f70.rs

@@ -0,0 +1,289 @@
+//! Green tests for work-d4a75f70: Document `tags` and `test_cases` in diffguard.toml.example
+//!
+//! These tests verify that `diffguard.toml.example` demonstrates the `tags` and `test_cases`
+//! features that exist in the codebase but are missing from the example file.
+//!
+//! These green tests CORRECT the logical flaw in the red tests where
+//! `rust_no_unwrap_has_negative_test_case` incorrectly checked the entire rule block
+//! for `.unwrap()` absence instead of just checking the negative test case's input.
+//!
+//! The path to diffguard.toml.example is computed at compile time using CARGO_MANIFEST_DIR.
+//! For tests in crates/diffguard/tests/, CARGO_MANIFEST_DIR = crates/diffguard
+//! We need to go up 2 levels to reach the repo root: crates/diffguard -> crates -> repo root
+
+/// The content of diffguard.toml.example embedded at compile time.
+const DIFFGUARD_EXAMPLE_CONTENT: &str = include_str!(concat!(
+    env!("CARGO_MANIFEST_DIR"),
+    "/../../diffguard.toml.example"
+));
+
+/// Find the bounds of the `rust.no_unwrap` rule block in the TOML.
+/// Returns the start and end line indices (0-based).
+fn find_rust_no_unwrap_block(lines: &[&str]) -> Option<(usize, usize)> {
+    let mut rule_start: Option<usize> = None;
+    let mut in_rust_no_unwrap = false;
+
+    for (i, line) in lines.iter().enumerate() {
+        let trimmed = line.trim();
+
+        // Check for end of rust.no_unwrap block BEFORE we process new [[rule]]
+        if in_rust_no_unwrap && trimmed == "[[rule]]" {
+            return Some((rule_start.unwrap(), i - 1));
+        }
+
+        // Start of a new rule block
+        if trimmed == "[[rule]]" {
+            rule_start = Some(i);
+            in_rust_no_unwrap = false;
+        } else if let Some(_start) = rule_start {
+            // Check if this is the rust.no_unwrap rule
+            if trimmed.starts_with("id = ") && trimmed.contains("rust.no_unwrap") {
+                in_rust_no_unwrap = true;
+            }
+        }
+    }
+
+    if in_rust_no_unwrap {
+        rule_start.map(|s| (s, lines.len() - 1))
+    } else {
+        None
+    }
+}
+
+/// Extract all [[rule.test_cases]] blocks from the rule block.
+/// Returns a vector of (description, input, should_match) tuples.
+fn extract_test_cases(rule_block: &str) -> Vec<(Option<&str>, &str, bool)> {
+    let mut test_cases = Vec::new();
+    let lines: Vec<&str> = rule_block.lines().collect();
+    let mut i = 0;
+
+    while i < lines.len() {
+        let trimmed = lines[i].trim();
+        if trimmed == "[[rule.test_cases]]" {
+            let mut description = None;
+            let mut input = None;
+            let mut should_match = None;
+
+            // Look ahead for the fields in this test case block
+            let mut j = i + 1;
+            while j < lines.len() && !lines[j].trim().is_empty() {
+                let field_trimmed = lines[j].trim();
+                if field_trimmed == "[[rule]]" || field_trimmed.starts_with("id = ") {
+                    break;
+                }
+                if field_trimmed.starts_with("description = ") {
+                    description = Some(
+                        field_trimmed
+                            .trim_start_matches("description = ")
+                            .trim_matches('"'),
+                    );
+                }
+                if field_trimmed.starts_with("input = ") {
+                    input = Some(
+                        field_trimmed
+                            .trim_start_matches("input = ")
+                            .trim_matches('"'),
+                    );
+                }
+                if field_trimmed.starts_with("should_match = ") {
+                    let val = field_trimmed.trim_start_matches("should_match = ");
+                    should_match = Some(val == "true");
+                }
+                j += 1;
+            }
+
+            if let (Some(inp), Some(sm)) = (input, should_match) {
+                test_cases.push((description, inp, sm));
+            }
+            i = j;
+        } else {
+            i += 1;
+        }
+    }
+
+    test_cases
+}
+
+/// Test that `rust.no_unwrap` rule has `tags = ["safety"]` field.
+///
+/// This verifies that users can discover the `tags` feature from the example file.
+/// The value should match built_in.json which uses `tags: ["safety"]` for this rule.
+#[test]
+fn rust_no_unwrap_rule_has_tags_safety() {
+    let lines: Vec<&str> = DIFFGUARD_EXAMPLE_CONTENT.lines().collect();
+
+    let (start, end) = find_rust_no_unwrap_block(&lines)
+        .expect("rust.no_unwrap rule block not found in diffguard.toml.example");
+
+    // Extract the rust.no_unwrap rule block
+    let rule_block: String = lines[start..=end].join("\n");
+
+    // Check for tags field with "safety" value
+    assert!(
+        rule_block.contains("tags = [\\\"safety\\\"]"),
+        "diffguard.toml.example rust.no_unwrap rule is MISSING `tags = [\\\"safety\\\"]`.\n\n\
+        Expected: The rust.no_unwrap rule block should contain `tags = [\\\"safety\\\"]`\n        to demonstrate the tags feature and be consistent with built_in.json (line 30).\n\n\
+        Actual: The rust.no_unwrap rule block does not contain `tags = [\\\"safety\\\"]`.",
+    );
+}
+
+/// Test that `rust.no_unwrap` rule has at least one `[[rule.test_cases]]` block.
+///
+/// This verifies that users can discover the `test_cases` feature from the example file.
+/// The `[[rule.test_cases]]` syntax is TOML's array of tables notation for appending
+/// elements to an array.
+#[test]
+fn rust_no_unwrap_rule_has_test_cases_blocks() {
+    let lines: Vec<&str> = DIFFGUARD_EXAMPLE_CONTENT.lines().collect();
+
+    let (start, end) = find_rust_no_unwrap_block(&lines)
+        .expect("rust.no_unwrap rule block not found in diffguard.toml.example");
+
+    // Extract the rust.no_unwrap rule block
+    let rule_block: String = lines[start..=end].join("\n");
+
+    // Check for [[rule.test_cases]] syntax (TOML array of tables)
+    assert!(
+        rule_block.contains("[[rule.test_cases]]"),
+        "diffguard.toml.example rust.no_unwrap rule is MISSING `[[rule.test_cases]]` blocks.\n\n\
+        Expected: The rust.no_unwrap rule should contain at least one `[[rule.test_cases]]`\n        block to demonstrate the test_cases feature for `diff test` command."
+    );
+}
+
+/// Test that `rust.no_unwrap` rule has a positive test case with `should_match = true`.
+///
+/// A positive test case verifies that the rule matches inputs that should be flagged.
+/// The example input should contain `.unwrap()` or `.expect()` which are the patterns
+/// that rust.no_unwrap detects.
+#[test]
+fn rust_no_unwrap_has_positive_test_case() {
+    let lines: Vec<&str> = DIFFGUARD_EXAMPLE_CONTENT.lines().collect();
+
+    let (start, end) = find_rust_no_unwrap_block(&lines)
+        .expect("rust.no_unwrap rule block not found in diffguard.toml.example");
+
+    // Extract the rust.no_unwrap rule block
+    let rule_block: String = lines[start..=end].join("\n");
+
+    // Extract test cases
+    let test_cases = extract_test_cases(&rule_block);
+
+    // Find a positive test case (should_match = true and input contains .unwrap() or .expect())
+    let has_positive_case = test_cases.iter().any(|(_desc, input, should_match)| {
+        *should_match && (input.contains(".unwrap()") || input.contains(".expect()"))
+    });
+
+    assert!(
+        has_positive_case,
+        "diffguard.toml.example rust.no_unwrap rule is MISSING a positive test case.\n\n\
+        Expected: At least one `[[rule.test_cases]]` block with `should_match = true`\n        where the `input` contains `.unwrap()` or `.expect()` (patterns the rule matches).\n\n\
+        Found test cases: {:?}",
+        test_cases
+    );
+}
+
+/// Test that `rust.no_unwrap` rule has a negative test case with `should_match = false`.
+///
+/// A negative test case verifies that the rule does NOT match safe inputs.
+/// This test correctly checks ONLY the negative test case's input, not the entire rule block.
+/// This is the CORRECTED version of the flawed red test that incorrectly checked the entire block.
+#[test]
+fn rust_no_unwrap_has_negative_test_case() {
+    let lines: Vec<&str> = DIFFGUARD_EXAMPLE_CONTENT.lines().collect();
+
+    let (start, end) = find_rust_no_unwrap_block(&lines)
+        .expect("rust.no_unwrap rule block not found in diffguard.toml.example");
+
+    // Extract the rust.no_unwrap rule block
+    let rule_block: String = lines[start..=end].join("\n");
+
+    // Extract test cases
+    let test_cases = extract_test_cases(&rule_block);
+
+    // Find a negative test case (should_match = false and input does NOT contain .unwrap() or .expect())
+    // CORRECTION: We check ONLY the negative test case's input, not the entire block!
+    let has_negative_case = test_cases.iter().any(|(_desc, input, should_match)| {
+        !*should_match && !input.contains(".unwrap()") && !input.contains(".expect()")
+    });
+
+    assert!(
+        has_negative_case,
+        "diffguard.toml.example rust.no_unwrap rule is MISSING a negative test case.\n\n\
+        Expected: At least one `[[rule.test_cases]]` block with `should_match = false`\n        where the `input` does NOT contain `.unwrap()` or `.expect()` (safe code).\n\n\
+        Found test cases: {:?}",
+        test_cases
+    );
+}
+
+/// Test that tags appears before [[rule.test_cases]] in the rust.no_unwrap rule.
+///
+/// Per the acceptance criteria, `tags` should appear after existing fields and
+/// `[[rule.test_cases]]` blocks should appear after `tags`.
+#[test]
+fn tags_appears_before_test_cases_in_rust_no_unwrap() {
+    let lines: Vec<&str> = DIFFGUARD_EXAMPLE_CONTENT.lines().collect();
+
+    let (start, end) = find_rust_no_unwrap_block(&lines)
+        .expect("rust.no_unwrap rule block not found in diffguard.toml.example");
+
+    let rule_block: String = lines[start..=end].join("\n");
+
+    let tags_pos = rule_block.find("tags = [\"safety\"]");
+    let test_cases_pos = rule_block.find("[[rule.test_cases]]");
+
+    if tags_pos.is_none() {
+        panic!(
+            "tags = [\"safety\"] not found in rust.no_unwrap rule block.\n\
+            This test requires tags to be present before checking ordering."
+        );
+    }
+
+    if test_cases_pos.is_none() {
+        panic!(
+            "[[rule.test_cases]] not found in rust.no_unwrap rule block.\n\
+            This test requires test_cases to be present before checking ordering."
+        );
+    }
+
+    let tags_idx = tags_pos.unwrap();
+    let test_cases_idx = test_cases_pos.unwrap();
+
+    assert!(
+        tags_idx < test_cases_idx,
+        "tags should appear BEFORE [[rule.test_cases]] in the rust.no_unwrap rule.\n\n\
+        Expected: tags = [\"safety\"] at position {}, [[rule.test_cases]] at position {}\n\
+        Actual: tags appears after [[rule.test_cases]]",
+        tags_idx,
+        test_cases_idx
+    );
+}
+
+/// Test that the TOML file parses correctly.
+#[test]
+fn toml_parses_correctly() {
+    // This is a simple smoke test that the TOML is valid
+    let content = DIFFGUARD_EXAMPLE_CONTENT;
+
+    // If this parsing doesn't panic, the TOML is valid
+    let _parsed: toml::Table =
+        toml::from_str(content).expect("diffguard.toml.example should be valid TOML");
+
+    // If we get here, the TOML is valid
+}
+
+/// Edge case: Test that test_cases with both .unwrap() and .expect() patterns are handled.
+#[test]
+fn test_cases_cover_both_patterns() {
+    let lines: Vec<&str> = DIFFGUARD_EXAMPLE_CONTENT.lines().collect();
+
+    let (start, end) = find_rust_no_unwrap_block(&lines)
+        .expect("rust.no_unwrap rule block not found in diffguard.toml.example");
+
+    let rule_block: String = lines[start..=end].join("\n");
+
+    // The patterns are ["\\.unwrap\\(", "\\.expect\\("] - check both are represented
+    assert!(
+        rule_block.contains(".unwrap()") || rule_block.contains(".expect()"),
+        "rust.no_unwrap should have test cases covering both .unwrap() and .expect() patterns"
+    );
+}