Conversation
Coverage Report
Per-file coverage (top 40 by missed lines)
|
There was a problem hiding this comment.
Pull request overview
This pull request adds comprehensive machine-readable evidence to all package check responses, enabling downstream automation and policy evaluation. The changes introduce a structured Evidence type with stable identifiers, severity levels, human-readable messages, and typed fact dictionaries. All checks, custom rules, policy decisions, and runtime errors now emit evidence alongside existing human-readable reasons.
Changes:
- Introduced
EvidenceandEvidenceKindtypes with stable ID conventions for checks, custom rules, policy, and runtime evidence - Updated all check implementations to use
CheckFinding::new()builder pattern withreason_codeand structuredfacts - Added evidence collection and propagation through check orchestration, service layer, and MCP responses
- Updated documentation and removed completed roadmap item for structured evidence
- Added comprehensive test coverage for evidence schema validation across registries
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.rs | Defines core Evidence and EvidenceKind types for API responses |
| crates/core/src/lib.rs | Extends CheckFinding with reason_code, facts, and FindingValue enum |
| src/checks.rs | Orchestrates evidence collection from checks/custom rules, generates policy evidence |
| src/custom_rules.rs | Wraps custom rule findings with rule_id for evidence generation |
| src/service.rs | Propagates evidence through service layer, adds runtime error evidence |
| src/audit_log.rs | Adds evidence field to audit log records |
| src/mcp/server.rs | Updates tool descriptions to document evidence format |
| crates/checks/*/src/lib.rs | Updates all 7 checks to emit structured findings with facts |
| tests/mcp_stdio.rs | Adds evidence schema validation helpers and integration tests |
| tests/check_package_mock_http.rs | Tests multi-signal evidence scenarios |
| src/tests/*.rs | Updates unit tests to verify evidence in reports |
| README.md | Documents evidence format, examples, and removes completed roadmap item |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| reasons: vec![reason], | ||
| evidence: vec![runtime_error_evidence(&err.to_string())], |
There was a problem hiding this comment.
The runtime error evidence message is inconsistent between the package result (line 158) and the audit log (line 168). Line 151 creates a formatted reason with "package check failed: " prefix, which is used in the package result evidence on line 158. However, the audit log on line 168 creates evidence directly from err.to_string() without the prefix. This means the same error will have different evidence messages in different contexts, which could be confusing for downstream consumers expecting stable evidence content. Consider using the same reason variable for both evidence creations to maintain consistency.
| reasons: vec![reason], | |
| evidence: vec![runtime_error_evidence(&err.to_string())], | |
| reasons: vec![reason.clone()], | |
| evidence: vec![runtime_error_evidence(&reason)], |
| fn assert_evidence_item_shape(item: &serde_json::Value) { | ||
| assert_eq!( | ||
| item.get("kind").and_then(serde_json::Value::as_str), | ||
| Some("check") | ||
| ); | ||
| assert!( | ||
| item.get("id") | ||
| .and_then(serde_json::Value::as_str) | ||
| .map(|value| !value.is_empty()) | ||
| .unwrap_or(false) | ||
| ); | ||
| assert!( | ||
| item.get("severity") | ||
| .and_then(serde_json::Value::as_str) | ||
| .is_some() | ||
| ); | ||
| assert!( | ||
| item.get("message") | ||
| .and_then(serde_json::Value::as_str) | ||
| .is_some() | ||
| ); | ||
| assert!( | ||
| item.get("facts") | ||
| .map(serde_json::Value::is_object) | ||
| .unwrap_or(false) | ||
| ); | ||
| } |
There was a problem hiding this comment.
The function name assert_evidence_item_shape is misleading because it specifically asserts that the evidence kind is "check" (line 76), but the name suggests it validates the general shape of any evidence item. While the current usages are correct (both are for check evidence), this could cause issues if someone tries to use this helper for policy, custom_rule, or runtime evidence in the future. Consider either renaming to assert_check_evidence_item_shape to be more specific, or making the function more generic by not asserting a specific kind.
No description provided.