feat(sparkctl): add SPARK Evidence Packet v1#3
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the SPARK Evidence Packet v1, introducing a deterministic, reviewable evidence envelope along with CLI commands, comprehensive unit tests, agent skill guidelines, and detailed documentation. The code review feedback focuses on enhancing security and robustness by enforcing strict schema validation (deny_unknown_fields), validating that list elements are not empty or whitespace-only, and improving error handling for missing files. Additionally, the feedback suggests optimizing JSON deserialization to avoid unnecessary cloning, simplifying directory creation, and renaming enum variants to adhere to Rust naming conventions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] | ||
| pub enum PolicyResult { | ||
| #[serde(rename = "ALLOW")] | ||
| ALLOW, | ||
| #[serde(rename = "REVIEW_NEEDED")] | ||
| ReviewNeeded, | ||
| #[serde(rename = "BLOCK")] | ||
| BLOCK, | ||
| } |
There was a problem hiding this comment.
According to the Rust API Guidelines (C-CASE), enum variants should use UpperCamelCase (PascalCase).
Renaming ALLOW and BLOCK to Allow and Block adheres to this standard. Since #[serde(rename = "...")] is already used, this change will not affect the serialized JSON representation.
| #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] | |
| pub enum PolicyResult { | |
| #[serde(rename = "ALLOW")] | |
| ALLOW, | |
| #[serde(rename = "REVIEW_NEEDED")] | |
| ReviewNeeded, | |
| #[serde(rename = "BLOCK")] | |
| BLOCK, | |
| } | |
| #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] | |
| pub enum PolicyResult { | |
| #[serde(rename = "ALLOW")] | |
| Allow, | |
| #[serde(rename = "REVIEW_NEEDED")] | |
| ReviewNeeded, | |
| #[serde(rename = "BLOCK")] | |
| Block, | |
| } |
References
- Rust naming conventions (C-CASE) dictate that enum variants should be in UpperCamelCase (PascalCase). (link)
| #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] | ||
| pub enum ProviderBoundaryStatus { | ||
| #[serde(rename = "DEMO")] | ||
| DEMO, | ||
| #[serde(rename = "UNAVAILABLE")] | ||
| UNAVAILABLE, | ||
| #[serde(rename = "AVAILABLE")] | ||
| AVAILABLE, | ||
| #[serde(rename = "BLOCKED_BY_POLICY")] | ||
| BlockedByPolicy, | ||
| } |
There was a problem hiding this comment.
According to the Rust API Guidelines (C-CASE), enum variants should use UpperCamelCase (PascalCase).
Renaming DEMO, UNAVAILABLE, and AVAILABLE to Demo, Unavailable, and Available adheres to this standard.
| #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] | |
| pub enum ProviderBoundaryStatus { | |
| #[serde(rename = "DEMO")] | |
| DEMO, | |
| #[serde(rename = "UNAVAILABLE")] | |
| UNAVAILABLE, | |
| #[serde(rename = "AVAILABLE")] | |
| AVAILABLE, | |
| #[serde(rename = "BLOCKED_BY_POLICY")] | |
| BlockedByPolicy, | |
| } | |
| #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] | |
| pub enum ProviderBoundaryStatus { | |
| #[serde(rename = "DEMO")] | |
| Demo, | |
| #[serde(rename = "UNAVAILABLE")] | |
| Unavailable, | |
| #[serde(rename = "AVAILABLE")] | |
| Available, | |
| #[serde(rename = "BLOCKED_BY_POLICY")] | |
| BlockedByPolicy, | |
| } |
References
- Rust naming conventions (C-CASE) dictate that enum variants should be in UpperCamelCase (PascalCase). (link)
| #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] | ||
| pub enum HumanReviewDecision { | ||
| #[serde(rename = "PASS")] | ||
| PASS, | ||
| #[serde(rename = "NOTES")] | ||
| NOTES, | ||
| #[serde(rename = "BLOCKED")] | ||
| BLOCKED, | ||
| } |
There was a problem hiding this comment.
According to the Rust API Guidelines (C-CASE), enum variants should use UpperCamelCase (PascalCase).
Renaming PASS, NOTES, and BLOCKED to Pass, Notes, and Blocked adheres to this standard.
| #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] | |
| pub enum HumanReviewDecision { | |
| #[serde(rename = "PASS")] | |
| PASS, | |
| #[serde(rename = "NOTES")] | |
| NOTES, | |
| #[serde(rename = "BLOCKED")] | |
| BLOCKED, | |
| } | |
| #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] | |
| pub enum HumanReviewDecision { | |
| #[serde(rename = "PASS")] | |
| Pass, | |
| #[serde(rename = "NOTES")] | |
| Notes, | |
| #[serde(rename = "BLOCKED")] | |
| Blocked, | |
| } |
References
- Rust naming conventions (C-CASE) dictate that enum variants should be in UpperCamelCase (PascalCase). (link)
| pub fn validate_spark_evidence_packet_value(value: &serde_json::Value) -> anyhow::Result<()> { | ||
| let envelope: SparkEvidencePacketEnvelope = serde_json::from_value(value.clone())?; | ||
| validate_spark_evidence_packet_envelope(&envelope) | ||
| } |
There was a problem hiding this comment.
Instead of cloning the serde_json::Value to deserialize it, you can deserialize directly from the reference value because serde_json::Value implements serde::Deserializer. This avoids unnecessary allocations and cloning of the JSON tree.
| pub fn validate_spark_evidence_packet_value(value: &serde_json::Value) -> anyhow::Result<()> { | |
| let envelope: SparkEvidencePacketEnvelope = serde_json::from_value(value.clone())?; | |
| validate_spark_evidence_packet_envelope(&envelope) | |
| } | |
| pub fn validate_spark_evidence_packet_value(value: &serde_json::Value) -> anyhow::Result<()> { | |
| let envelope = SparkEvidencePacketEnvelope::deserialize(value)?; | |
| validate_spark_evidence_packet_envelope(&envelope) | |
| } |
1 participant
Summary
Adds SPARK Evidence Packet v1 as a local deterministic evidence packet for
sparkctl.This branch introduces a typed packet preimage/envelope model, canonical JSON generation from the preimage, a SHA-256 hash over that canonical JSON, CLI demo and validation commands, docs for artifact/claim boundaries, and committed agent instructions for review-safe work.
What Changed
SparkEvidencePacketPreimageandSparkEvidencePacketEnvelope.canonical_jsonfrom preimage fields only.canonical_hashas SHA-256 overcanonical_json.sparkctl spark-evidence-demo.sparkctl spark-evidence-validate.AGENTS.mdand.agents/skills/**are intentional review/governance instructions for this repository.Boundaries
README.mdwas not modified.Validation
cargo fmt --all --check: PASScargo test: PASScargo clippy --all-targets --all-features -- -D warnings: PASScargo run --bin sparkctl -- spark-evidence-validate --input ../artifacts/spark/evidence_packet_v1.json: PASSCI
Rust Validation CI / validate: SUCCESS on PR head1623a3ae591ed50fd34c3dae38db0375e3dd2ca0.cargo fmt --all --checkcargo testcargo clippy --all-targets --all-features -- -D warningscargo run --bin sparkctl -- spark-evidence-validate --input ../artifacts/spark/evidence_packet_v1.jsonReviewer Checklist
canonical_hashis SHA-256 over canonical JSON..spkgpackage compatibility is preserved.Risks
cargo testcan generate local report churn inreports/latest.jsonandreports/performance_baseline.json; those files are excluded from this branch.../...entries, so reviewers should confirm the intended base path is clear enough for downstream users.