diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 1a5678e..675cb53 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -96,16 +96,19 @@ acquire diff --> parse supported lockfiles --> parse + index --> bounded evidenc resources, values, versions, paths, and identifiers. One receipt binds those queries to the immutable reviewed head and reports `complete`, `unavailable`, or `exhausted`. Only a complete receipt with no positive counterexample supports a universal claim. - Incomplete repository evidence cannot confirm it, so the original finding remains open. + Incomplete repository evidence cannot confirm a fresh claim, so the claim is suppressed; + a prior-ledger claim remains open until exact-head adjudication resolves it. Every surviving generated candidate, plus applicable baseline candidates during a full rereview, enters one bounded adjudication operation before scoring and publication. The operation admits the complete candidate set or fails closed before provider contact. Its direct-source receipt hashes and scans the complete diff, records deterministic citation occurrence counts, and carries only bounded evidence windows to the model. Adjudication validates exact candidate identities, result completeness, evidence, publication text, - and duplicate primaries. Later and cross-file evidence can refute stale claims, while - unresolved claims remain open. Semantic duplicates collapse across files and kinds - only when one established defect remains; distinct defects sharing a line remain separate. + and duplicate primaries. Later and cross-file evidence can refute stale claims. Fresh + unresolved repository claims are suppressed, ordinary grounded unresolved findings and + prior-ledger claims remain open, and provider or contract failure preserves every candidate. + Semantic duplicates collapse across files and kinds only when one established defect remains; + distinct defects sharing a line remain separate. - `forge/`: trait + GitHub, GitLab, Bitbucket Cloud, and Azure DevOps implementations, with self-managed base URLs where the same API contract applies. Paginated forge metadata has aggregate byte and changed-file bounds. Source responses stream to @@ -266,7 +269,9 @@ additions (violations are `kind: contentPolicy`). Content policy is on by defaul requires the named target and compared value in one searched evidence unit. Query, request, object, tree-depth, byte, deadline, and detailed-match bounds are explicit in the receipt outcome. Public findings state the repository construct and correction without describing - evidence retrieval boundaries or delegating evidence collection to the author. + evidence retrieval boundaries or delegating evidence collection to the author. Fresh + unresolved repository claims are suppressed; prior-ledger claims remain open until an + exact-head adjudication explicitly resolves them. 15. Finding adjudication performs exactly one logical provider operation over every admitted candidate. Candidate identities bind the exact snapshot and semantic finding fields. Adjudication input, output, attempts, deadline, and projected cost are bounded, and results diff --git a/Cargo.lock b/Cargo.lock index 960f736..9108fb7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1104,7 +1104,7 @@ checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" [[package]] name = "postil-cli" -version = "0.8.16" +version = "0.8.17" dependencies = [ "aho-corasick", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index a49ac65..aeddbcb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "postil-cli" -version = "0.8.16" +version = "0.8.17" edition = "2024" description = "Postil: a low-noise AI review gate. Silent on clean PRs, hard gate on real risk." license = "Apache-2.0" diff --git a/src/adjudication.rs b/src/adjudication.rs index b51d537..1724ae8 100644 --- a/src/adjudication.rs +++ b/src/adjudication.rs @@ -115,6 +115,7 @@ pub(crate) struct AdjudicationCandidate<'a> { pub(crate) struct AdjudicationApplication { pub kept: Vec, pub kept_indices: Vec, + pub unresolved_indices: Vec, pub resolved_indices: Vec, pub suppressed: Vec, } @@ -1288,6 +1289,7 @@ pub(crate) fn apply_results( .collect::>(); let mut kept = Vec::new(); let mut kept_indices = Vec::new(); + let mut unresolved_indices = Vec::new(); let mut resolved_indices = Vec::new(); let mut suppressed = Vec::new(); for (index, (mut finding, id)) in findings.into_iter().zip(candidate_ids).enumerate() { @@ -1308,6 +1310,7 @@ pub(crate) fn apply_results( } (AdjudicationProvenance::Model, AdjudicationDisposition::PreserveUnresolved) => { kept_indices.push(index); + unresolved_indices.push(index); kept.push(finding); } ( @@ -1315,6 +1318,7 @@ pub(crate) fn apply_results( AdjudicationDisposition::PreserveUnresolved, ) => { kept_indices.push(index); + unresolved_indices.push(index); kept.push(finding); } (AdjudicationProvenance::Model, AdjudicationDisposition::SuppressRefuted) => { @@ -1337,6 +1341,7 @@ pub(crate) fn apply_results( Ok(AdjudicationApplication { kept, kept_indices, + unresolved_indices, resolved_indices, suppressed, }) @@ -2658,6 +2663,7 @@ mod tests { assert_eq!(kept.evidence, original.evidence); } assert_eq!(applied.kept_indices, vec![0, 1]); + assert_eq!(applied.unresolved_indices, vec![0, 1]); assert!(applied.resolved_indices.is_empty()); assert!(applied.suppressed.is_empty()); } @@ -2725,6 +2731,7 @@ mod tests { assert_eq!(applied.kept[1].body, unresolved.body); assert_eq!(applied.kept[1].evidence, unresolved.evidence); assert_eq!(applied.kept_indices, vec![0, 2]); + assert_eq!(applied.unresolved_indices, vec![2]); assert_eq!(applied.resolved_indices, vec![1]); assert_eq!( applied.suppressed[0].reason, diff --git a/src/cli.rs b/src/cli.rs index 7155fde..3014326 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -56,6 +56,12 @@ pub enum ForgeArg { #[derive(Subcommand)] #[allow(clippy::large_enum_variant)] // Review carries the full flag set by design. pub enum Command { + /// Probe machine-readable CLI capabilities without external access. + Capabilities { + /// Require and print an exact publication-plan contract identifier. + #[arg(long, value_name = "IDENTIFIER")] + publication_plan_contract: String, + }, /// Print immutable qualification metadata embedded in this binary. #[command(hide = true)] QualificationMetadata, @@ -141,6 +147,44 @@ pub enum Command { /// Leave the merge-gate check pending for a controlling service to complete. #[arg(long, requires = "publish")] defer_gate_check: bool, + /// Write an immutable GitHub publication plan without mutating the forge. + #[arg( + long, + hide = true, + value_name = "PATH", + requires_all = [ + "repo", + "pr", + "sha", + "base_sha", + "publication_generation", + "publication_input_identity" + ], + conflicts_with_all = [ + "publish", + "no_post", + "check_run_id", + "gate_check_run_id", + "defer_gate_check" + ] + )] + publication_plan_output: Option, + /// Service-owned generation identity for publication planning. + #[arg( + long, + hide = true, + value_name = "IDENTITY", + requires = "publication_plan_output" + )] + publication_generation: Option, + /// Service-supplied immutable input identity for publication planning. + #[arg( + long, + hide = true, + value_name = "SHA256", + requires = "publication_plan_output" + )] + publication_input_identity: Option, }, /// Reply to an @postil mention on a pull request or issue (interactive bot). Respond { @@ -214,6 +258,15 @@ pub enum Command { Logout, } +pub fn publication_plan_contract_capability(required: &str) -> anyhow::Result<&'static str> { + anyhow::ensure!( + required == crate::forge::GITHUB_PUBLICATION_PLAN_CONTRACT, + "unsupported publication-plan contract {required:?}; supported contract: {}", + crate::forge::GITHUB_PUBLICATION_PLAN_CONTRACT + ); + Ok(crate::forge::GITHUB_PUBLICATION_PLAN_CONTRACT) +} + #[derive(Subcommand)] pub enum HookAction { /// Install a pre-push hook that reviews outgoing commits. @@ -227,7 +280,33 @@ pub enum HookAction { mod tests { use clap::{CommandFactory, Parser}; - use super::{Cli, Command}; + use super::{Cli, Command, publication_plan_contract_capability}; + + const PUBLICATION_INPUT_IDENTITY: &str = + "sha256:1111111111111111111111111111111111111111111111111111111111111111"; + + #[test] + fn publication_plan_capability_requires_the_exact_contract() { + let parsed = Cli::try_parse_from([ + "postil", + "capabilities", + "--publication-plan-contract", + "github-publication-v1", + ]) + .unwrap(); + let Command::Capabilities { + publication_plan_contract, + } = parsed.command + else { + panic!("expected capabilities command"); + }; + assert_eq!(publication_plan_contract, "github-publication-v1"); + assert_eq!( + publication_plan_contract_capability(&publication_plan_contract).unwrap(), + "github-publication-v1" + ); + assert!(publication_plan_contract_capability("github-publication-v2").is_err()); + } #[test] fn review_bounded_is_an_explicit_action_flag_with_concise_help() { @@ -356,6 +435,174 @@ mod tests { ); } + #[test] + fn publication_plan_is_hidden_snapshot_bound_and_separate_from_mutation() { + let parsed = Cli::try_parse_from([ + "postil", + "review", + "--repo", + "postil-dev/postil", + "--pr", + "1", + "--sha", + "aaaaaaaa", + "--base-sha", + "bbbbbbbb", + "--publication-plan-output", + "publication-plan.json", + "--publication-generation", + "1", + "--publication-input-identity", + PUBLICATION_INPUT_IDENTITY, + ]) + .unwrap(); + let Command::Review { + publication_plan_output, + publication_generation, + publication_input_identity, + publish, + .. + } = parsed.command + else { + panic!("expected review command"); + }; + assert_eq!( + publication_plan_output.as_deref(), + Some(std::path::Path::new("publication-plan.json")) + ); + assert_eq!(publication_generation.as_deref(), Some("1")); + assert_eq!( + publication_input_identity.as_deref(), + Some(PUBLICATION_INPUT_IDENTITY) + ); + assert!(!publish); + + let help = Cli::command() + .find_subcommand_mut("review") + .expect("review subcommand") + .render_long_help() + .to_string(); + assert!(!help.contains("publication-plan-output")); + + for extra in [ + vec!["--publish"], + vec!["--no-post"], + vec!["--check-run-id", "901"], + vec!["--gate-check-run-id", "902"], + ] { + let mut arguments = vec![ + "postil", + "review", + "--repo", + "postil-dev/postil", + "--pr", + "1", + "--sha", + "aaaaaaaa", + "--base-sha", + "bbbbbbbb", + "--publication-plan-output", + "publication-plan.json", + "--publication-generation", + "1", + "--publication-input-identity", + PUBLICATION_INPUT_IDENTITY, + ]; + arguments.extend(extra); + assert!(Cli::try_parse_from(arguments).is_err()); + } + + assert!( + Cli::try_parse_from([ + "postil", + "review", + "--repo", + "postil-dev/postil", + "--pr", + "1", + "--sha", + "aaaaaaaa", + "--publication-plan-output", + "publication-plan.json", + "--publication-generation", + "1", + "--publication-input-identity", + PUBLICATION_INPUT_IDENTITY, + ]) + .is_err() + ); + assert!( + Cli::try_parse_from([ + "postil", + "review", + "--repo", + "postil-dev/postil", + "--pr", + "1", + "--sha", + "aaaaaaaa", + "--base-sha", + "bbbbbbbb", + "--publication-plan-output", + "publication-plan.json", + "--publication-generation", + "1", + ]) + .is_err() + ); + assert!( + Cli::try_parse_from([ + "postil", + "review", + "--repo", + "postil-dev/postil", + "--pr", + "1", + "--sha", + "aaaaaaaa", + "--base-sha", + "bbbbbbbb", + "--publication-plan-output", + "publication-plan.json", + "--publication-input-identity", + PUBLICATION_INPUT_IDENTITY, + ]) + .is_err() + ); + assert!( + Cli::try_parse_from([ + "postil", + "review", + "--publication-input-identity", + PUBLICATION_INPUT_IDENTITY, + ]) + .is_err() + ); + assert!( + Cli::try_parse_from([ + "postil", + "review", + "--repo", + "postil-dev/postil", + "--pr", + "1", + "--sha", + "aaaaaaaa", + "--base-sha", + "bbbbbbbb", + "--publication-plan-output", + "publication-plan.json", + "--publication-generation", + "1", + "--publication-input-identity", + PUBLICATION_INPUT_IDENTITY, + "--publication-input-identity", + PUBLICATION_INPUT_IDENTITY, + ]) + .is_err() + ); + } + #[test] fn login_accepts_an_optional_org_hint() { let parsed = Cli::try_parse_from(["postil", "login"]).unwrap(); diff --git a/src/diff.rs b/src/diff.rs index 5860973..d5bcaed 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -969,6 +969,30 @@ impl DiffIndex { .is_some_and(|rs| rs.iter().any(|r| r.contains(&line))) } + /// Return the closest groundable new-side line in `path` to `requested`. + /// + /// Each hunk contributes only its clamped endpoint or `requested` itself, + /// so lookup remains proportional to the number of indexed ranges rather + /// than the number of lines they cover. Equal-distance candidates resolve + /// to the lower line. + pub fn nearest_new_side_line(&self, path: &str, requested: u32) -> Option { + if !self.complete { + return None; + } + self.ranges.get(path)?.iter().fold(None, |nearest, range| { + let candidate = requested.max(*range.start()).min(*range.end()); + match nearest { + Some(line) + if (line.abs_diff(requested), line) + <= (candidate.abs_diff(requested), candidate) => + { + Some(line) + } + _ => Some(candidate), + } + }) + } + /// True when both endpoints are on the new side of one diff hunk. Forge /// APIs reject multiline comments that cross hunk boundaries even when the /// individual line numbers both appear elsewhere in the file diff. @@ -6075,6 +6099,20 @@ deleted file mode 100644 -bye diff --git a/img.png b/img.png Binary files a/img.png and b/img.png differ +"; + + const MULTI_HUNK_SAMPLE: &str = "\ +diff --git a/src/multi.rs b/src/multi.rs +--- a/src/multi.rs ++++ b/src/multi.rs +@@ -10 +10,3 @@ + line ten ++added eleven ++added twelve +@@ -20 +20,3 @@ + line twenty ++added twenty-one ++added twenty-two "; #[test] @@ -6353,6 +6391,43 @@ Binary files a/img.png and b/img.png differ assert!(!idx.touches("src/lib.rs", 1, 9)); } + #[test] + fn nearest_new_side_line_returns_the_requested_line_in_range() { + let index = DiffIndex::build(&parse(MULTI_HUNK_SAMPLE)); + + assert_eq!(index.nearest_new_side_line("src/multi.rs", 11), Some(11)); + } + + #[test] + fn nearest_new_side_line_clamps_before_and_after_ranges() { + let index = DiffIndex::build(&parse(MULTI_HUNK_SAMPLE)); + + assert_eq!(index.nearest_new_side_line("src/multi.rs", 1), Some(10)); + assert_eq!(index.nearest_new_side_line("src/multi.rs", 99), Some(22)); + } + + #[test] + fn nearest_new_side_line_breaks_between_range_ties_toward_the_lower_line() { + let index = DiffIndex::build(&parse(MULTI_HUNK_SAMPLE)); + + assert_eq!(index.nearest_new_side_line("src/multi.rs", 16), Some(12)); + } + + #[test] + fn nearest_new_side_line_returns_none_for_a_missing_path() { + let index = DiffIndex::build(&parse(MULTI_HUNK_SAMPLE)); + + assert_eq!(index.nearest_new_side_line("src/missing.rs", 11), None); + } + + #[test] + fn nearest_new_side_line_returns_none_for_deleted_and_binary_files() { + let index = DiffIndex::build(&parse(SAMPLE)); + + assert_eq!(index.nearest_new_side_line("gone.txt", 1), None); + assert_eq!(index.nearest_new_side_line("img.png", 1), None); + } + #[test] fn annotated_render_numbers_new_lines() { let d = parse(SAMPLE); diff --git a/src/forge/azure.rs b/src/forge/azure.rs index a6b488b..eb973eb 100644 --- a/src/forge/azure.rs +++ b/src/forge/azure.rs @@ -465,6 +465,7 @@ impl Forge for Azure { &self, envelope: &Envelope, snapshot: &PrMeta, + _publication_diff: Option<&crate::diff::Diff>, ) -> Result { let findings = &envelope.findings; let receipt = untracked_review_publication_receipt("azure", envelope, &snapshot.head_sha); diff --git a/src/forge/bitbucket.rs b/src/forge/bitbucket.rs index fc9c2ae..37c13b5 100644 --- a/src/forge/bitbucket.rs +++ b/src/forge/bitbucket.rs @@ -510,6 +510,7 @@ impl Forge for Bitbucket { &self, envelope: &Envelope, snapshot: &PrMeta, + _publication_diff: Option<&crate::diff::Diff>, ) -> Result { let findings = &envelope.findings; let receipt = diff --git a/src/forge/github.rs b/src/forge/github.rs index 92c880c..fa65113 100644 --- a/src/forge/github.rs +++ b/src/forge/github.rs @@ -11,16 +11,31 @@ use std::io::Write; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use super::{ - CheckRunIds, CheckState, FindingPublicationOutcome, FindingPublicationReceipt, Forge, PrMeta, - ReviewPublicationReceipt, ReviewPublicationSummary, SummaryContext, ThreadKind, check_summary, - check_title, only_operational_findings, valid_details_url, + CheckRunIds, CheckState, FindingPublicationOutcome, FindingPublicationReceipt, Forge, + GitHubPublicationPlan, GitHubPublicationPlanIdentity, GitHubPublicationPlanRequest, PrMeta, + PublicationPlanActivationCondition, PublicationPlanCheckAnnotation, + PublicationPlanCheckConclusion, PublicationPlanCheckStatus, PublicationPlanDuplicateProvenance, + PublicationPlanFileComment, PublicationPlanFinding, PublicationPlanFindingFallback, + PublicationPlanFindingReconciliation, PublicationPlanGateAnalysis, + PublicationPlanGateOwnership, PublicationPlanLifecycleReceipt, + PublicationPlanMarkerAbsenceGuard, PublicationPlanOperation, + PublicationPlanOperationActivation, PublicationPlanOperationKind, + PublicationPlanOperationReconciliation, PublicationPlanOperationResultField, + PublicationPlanOperationResultReference, PublicationPlanPlacementClassification, + PublicationPlanRepository, PublicationPlanReviewAttemptKind, PublicationPlanReviewComment, + PublicationPlanReviewCreateOutcome, PublicationPlanReviewCreatePayload, + PublicationPlanReviewSummaryCase, PublicationPlanSnapshot, PublicationPlanTerminalOperation, + PublicationPlanTerminalOutcome, ReviewPublicationReceipt, ReviewPublicationSummary, + SummaryContext, ThreadKind, check_summary, check_title, only_operational_findings, + valid_details_url, }; -use crate::diff::{DiffSnapshot, DiffSpool, WorkspaceBudget}; +use crate::diff::{Diff, DiffIndex, DiffSnapshot, DiffSpool, WorkspaceBudget}; use crate::envelope::{Envelope, Finding, Severity}; use crate::filter; pub const EXPECTED_REPOSITORY_ID_ENV: &str = "POSTIL_EXPECTED_GITHUB_REPO_ID"; const GITHUB_MAX_ANNOTATIONS_PER_REQUEST: usize = 50; +const FILE_LEVEL_COMMENT_MARKER: &str = ""; // Filtering caps visible findings before forge publication, so one completed // check update always fits GitHub's annotation request limit. Raising the @@ -116,11 +131,34 @@ impl GitHub { ) } + fn review_summary_with_unplaced_findings( + &self, + envelope: &Envelope, + receipt: &ReviewPublicationReceipt, + unplaced: &[&Finding], + ) -> String { + let mut summary = self.review_summary_for_receipt(envelope, receipt); + for finding in unplaced { + let (finding_id, _) = finding_receipt_id(finding); + summary.push_str(&format!( + "\n\nLocation: `{}:{}`\n\n{}", + super::safe_code_text(&finding.path), + finding.line, + append_marker( + &super::finding_comment_body(finding, true), + &finding_marker(&finding_id), + ), + )); + } + summary + } + async fn finalize_review_summary( &self, envelope: &Envelope, receipt: &ReviewPublicationReceipt, marker: &str, + snapshot: &PrMeta, ) -> Result<()> { let review_id = receipt .review_id @@ -131,13 +169,14 @@ impl GitHub { let summary = self.review_summary_for_receipt(envelope, receipt); let body = bounded_review_body(&summary, marker, self.details_url.as_deref()); let response = self - .send_write_retryable( + .send_snapshot_write_retryable( self.request( reqwest::Method::PUT, self.url(&format!("/pulls/{}/reviews/{review_id}", self.pr)), ) .json(&json!({ "body": body })), "review summary update", + snapshot, ) .await?; Self::check_ok(response, "review summary update").await?; @@ -149,9 +188,10 @@ impl GitHub { envelope: &Envelope, receipt: &ReviewPublicationReceipt, marker: &str, + snapshot: &PrMeta, ) { if self - .finalize_review_summary(envelope, receipt, marker) + .finalize_review_summary(envelope, receipt, marker, snapshot) .await .is_err() { @@ -224,6 +264,56 @@ impl GitHub { Ok(()) } + async fn publication_plan_repository_identity(&self) -> Result { + let response = self + .send_retryable( + self.request(reqwest::Method::GET, self.url("")), + "publication-plan repository identity", + ) + .await?; + let identity: RepositoryIdentity = super::bounded_response_json( + Self::check_ok(response, "publication-plan repository identity").await?, + "GitHub publication-plan repository identity", + ) + .await?; + let expected_name = format!("{}/{}", self.owner, self.repo); + ensure!( + identity.id > 0 && identity.full_name.eq_ignore_ascii_case(&expected_name), + "GitHub repository identity changed; refusing publication planning" + ); + if let Some(expected_id) = self.expected_repository_id { + ensure!( + identity.id == expected_id, + "GitHub repository identity changed; refusing publication planning" + ); + } + Ok(identity) + } + + async fn reconcile_published_finding_markers( + &self, + receipt: &mut ReviewPublicationReceipt, + envelope: &Envelope, + head_sha: &str, + ) -> std::collections::HashMap { + let published = self.published_finding_comments(head_sha).await; + for publication in &mut receipt.findings { + let Ok(finding) = publication_plan_finding(envelope, &publication.finding_id) else { + continue; + }; + let Some(comment) = finding_marker_candidates(finding) + .iter() + .find_map(|marker| published.get(marker)) + else { + continue; + }; + publication.initial_outcome = FindingPublicationOutcome::Carried; + publication.inline_rejected = false; + publication.comment_id = Some(comment.id.to_string()); + } + published + } + async fn fetch_pr_state(&self) -> Result { let response = self .send_retryable( @@ -259,7 +349,7 @@ impl GitHub { request: reqwest::RequestBuilder, what: &str, ) -> Result { - self.send_retryable_inner(request, what, false).await + self.send_retryable_inner(request, what, false, None).await } async fn send_write_retryable( @@ -267,7 +357,17 @@ impl GitHub { request: reqwest::RequestBuilder, what: &str, ) -> Result { - self.send_retryable_inner(request, what, true).await + self.send_retryable_inner(request, what, true, None).await + } + + async fn send_snapshot_write_retryable( + &self, + request: reqwest::RequestBuilder, + what: &str, + snapshot: &PrMeta, + ) -> Result { + self.send_retryable_inner(request, what, true, Some(snapshot)) + .await } async fn send_retryable_inner( @@ -275,11 +375,19 @@ impl GitHub { request: reqwest::RequestBuilder, what: &str, fence_each_attempt: bool, + expected_snapshot: Option<&PrMeta>, ) -> Result { const RETRIES: u32 = 2; const TOTAL_BUDGET: Duration = Duration::from_secs(55); let operation_started_at = std::time::Instant::now(); for retry in 0..=RETRIES { + if let Some(expected_snapshot) = expected_snapshot + && !Box::pin(self.snapshot_is_current(expected_snapshot)).await? + { + return Err(anyhow!( + "GitHub {what} skipped because the PR snapshot changed" + )); + } if fence_each_attempt { self.verify_repository_identity_before_write().await?; } @@ -443,7 +551,12 @@ impl GitHub { unreachable!("bounded check-run create loop always returns") } - async fn find_review(&self, marker: &str, head_sha: &str) -> Result> { + async fn find_review( + &self, + markers: &[String], + head_sha: &str, + allow_correlated_legacy_marker: bool, + ) -> Result> { const PAGE_SIZE: usize = 100; const MAX_PAGES: usize = 20; for page in 1..=MAX_PAGES { @@ -467,10 +580,13 @@ impl GitHub { let page_len = reviews.len(); if let Some(review) = reviews.into_iter().find(|review| { review.commit_id.as_deref() == Some(head_sha) - && review - .body - .as_deref() - .is_some_and(|body| body.contains(marker)) + && review.body.as_deref().is_some_and(|body| { + markers.iter().any(|marker| body.contains(marker)) + || (allow_correlated_legacy_marker + && review_marker_in(body).is_some_and(|marker| { + marker.starts_with("", + crate::repository_search::hex_digest(hash) + ) +} + +fn legacy_finding_marker(finding_id: &str) -> String { let hash = Sha256::digest(finding_id.as_bytes()); format!( "", @@ -2191,20 +4190,52 @@ fn finding_marker(finding_id: &str) -> String { ) } +fn finding_marker_candidates(finding: &Finding) -> Vec { + let (finding_id, _) = finding_receipt_id(finding); + let legacy_id = legacy_finding_receipt_id(finding); + let mut markers = vec![ + finding_marker(&finding_id), + legacy_finding_marker(&finding_id), + ]; + if legacy_id != finding_id { + markers.push(legacy_finding_marker(&legacy_id)); + } + markers +} + fn review_marker(receipt_id: &str) -> String { let mut digest = Sha256::new(); digest.update(receipt_id.as_bytes()); let hash = digest.finalize(); + format!( + "", + crate::repository_search::hex_digest(hash) + ) +} + +fn legacy_review_marker(receipt_id: &str) -> String { + let hash = Sha256::digest(receipt_id.as_bytes()); format!( "", hash[0], hash[1], hash[2], hash[3], hash[4], hash[5] ) } +fn review_marker_in(body: &str) -> Option { + let start = ["")? + start + "-->".len(); + Some(body[start..end].to_string()) +} + /// The finding marker a published comment body ends with, if any. fn finding_marker_in(body: &str) -> Option { - const OPEN: &str = "")? + start + "-->".len(); Some(body[start..end].to_string()) } @@ -2217,6 +4248,28 @@ fn append_marker(body: &str, marker: &str) -> String { } } +fn without_finding_marker(body: &str) -> &str { + let Some(marker) = finding_marker_in(body) else { + return body.trim_end(); + }; + body.strip_suffix(&marker) + .map(str::trim_end) + .unwrap_or_else(|| body.trim_end()) +} + +fn publication_plan_body_digest(body: &str) -> String { + publication_plan_text_digest(body) +} + +fn published_comment_for_finding<'a>( + published: &'a std::collections::HashMap, + finding: &Finding, +) -> Option<&'a PublishedReviewComment> { + finding_marker_candidates(finding) + .iter() + .find_map(|marker| published.get(marker)) +} + const MAX_REVIEW_BODY_BYTES: usize = 60_000; const OVERSIZED_REVIEW_MESSAGE: &str = "Review summary omitted because it exceeds GitHub's size limit."; @@ -2238,6 +4291,15 @@ fn bounded_review_body(body: &str, marker: &str, details_url: Option<&str>) -> S append_marker(OVERSIZED_REVIEW_MESSAGE, marker) } +fn required_review_body(body: &str, marker: &str) -> Result { + let marked = append_marker(body, marker); + ensure!( + marked.len() <= MAX_REVIEW_BODY_BYTES, + "GitHub review summary cannot represent every unplaced finding within its size limit" + ); + Ok(marked) +} + fn comment_marker(number: u64, body: &str) -> String { let mut digest = Sha256::new(); digest.update(number.to_be_bytes()); @@ -2260,7 +4322,12 @@ mod tests { use crate::envelope::{ Envelope, Finding, Gate, Kind, Severity, SuppressedFinding, SuppressionReason, Usage, }; - use crate::forge::{CheckRunIds, CheckState, FindingPublicationOutcome, Forge, PrMeta}; + use crate::forge::{ + CheckRunIds, CheckState, FindingPublicationOutcome, Forge, PrMeta, + PublicationPlanActivationCondition, PublicationPlanCheckStatus, PublicationPlanOperation, + PublicationPlanOperationActivation, PublicationPlanOperationKind, + PublicationPlanOperationReconciliation, + }; use reqwest::header::{HeaderMap, HeaderValue}; use std::sync::{ Arc, @@ -2270,6 +4337,24 @@ mod tests { use wiremock::matchers::{method, path, path_regex, query_param}; use wiremock::{Mock, MockServer, ResponseTemplate}; + const PUBLICATION_INPUT_IDENTITY: &str = + "sha256:1111111111111111111111111111111111111111111111111111111111111111"; + + fn publication_plan_key_scope<'a>( + head_sha: &'a str, + input_identity: &'a str, + review_output_digest: &'a str, + ) -> super::PublicationPlanKeyScope<'a> { + super::PublicationPlanKeyScope { + repository_id: "42", + pull_request_number: "7", + head_sha, + controller_generation: "1", + input_identity, + review_output_digest, + } + } + fn delivery_envelope(head_sha: &str, base_sha: &str) -> Envelope { Envelope { version: 1, @@ -2319,875 +4404,2439 @@ mod tests { envelope } - fn publication_finding(id: &str, path: &str, body: &str) -> Finding { - Finding { - path: path.into(), - line: 7, - end_line: None, - severity: Severity::Warn, - kind: Kind::Risk, - confidence: 0.9, - generator_confidence: None, - scorer_confidence: None, - generator_kind: None, - scorer_kind: None, - scorer_reason: None, - repository_claim: None, - title: format!("Finding {id}"), - body: body.into(), - evidence: Some("let value = risky();".into()), - id: Some(id.into()), + fn publication_finding(id: &str, path: &str, body: &str) -> Finding { + Finding { + path: path.into(), + line: 7, + end_line: None, + severity: Severity::Warn, + kind: Kind::Risk, + confidence: 0.9, + generator_confidence: None, + scorer_confidence: None, + generator_kind: None, + scorer_kind: None, + scorer_reason: None, + repository_claim: None, + title: format!("Finding {id}"), + body: body.into(), + evidence: Some("let value = risky();".into()), + id: Some(id.into()), + } + } + + fn placement_diff() -> crate::diff::Diff { + crate::diff::parse( + "diff --git a/src/lib.rs b/src/lib.rs\n\ + --- a/src/lib.rs\n\ + +++ b/src/lib.rs\n\ + @@ -10 +10,3 @@\n\ + context\n\ + +added one\n\ + +added two\n", + ) + } + + fn file_only_placement_diff() -> crate::diff::Diff { + crate::diff::parse( + "diff --git a/src/lib.rs b/src/lib.rs\n\ + index 9daeafb..0f15a6e 100644\n\ + Binary files a/src/lib.rs and b/src/lib.rs differ\n", + ) + } + + fn unrelated_placement_diff() -> crate::diff::Diff { + crate::diff::parse( + "diff --git a/src/other.rs b/src/other.rs\n\ + --- a/src/other.rs\n\ + +++ b/src/other.rs\n\ + @@ -1 +1 @@\n\ + -old();\n\ + +new();\n", + ) + } + + fn two_file_placement_diff() -> crate::diff::Diff { + crate::diff::parse( + "diff --git a/src/first.bin b/src/first.bin\n\ + index 9daeafb..0f15a6e 100644\n\ + Binary files a/src/first.bin and b/src/first.bin differ\n\ + diff --git a/src/second.bin b/src/second.bin\n\ + index 9daeafb..0f15a6e 100644\n\ + Binary files a/src/second.bin and b/src/second.bin differ\n", + ) + } + + fn repository_search_terms() -> Vec { + use crate::envelope::{RepositoryClaim, RepositoryClaimKind}; + crate::repository_search::search_terms(std::iter::once(&RepositoryClaim { + kind: RepositoryClaimKind::Absence, + resources: vec![], + values: vec![], + versions: vec![], + paths: vec![], + identifiers: vec!["clusterVersion".into()], + })) + .unwrap() + } + + #[test] + fn hosted_repository_identity_environment_contract_is_stable() { + assert_eq!(EXPECTED_REPOSITORY_ID_ENV, "POSTIL_EXPECTED_GITHUB_REPO_ID"); + } + + #[test] + fn review_summary_body_is_bounded_before_github_publication() { + let marker = ""; + assert_eq!( + super::bounded_review_body("summary", marker, None), + format!("summary\n\n{marker}") + ); + let fallback = super::bounded_review_body( + &"x".repeat(super::MAX_REVIEW_BODY_BYTES), + marker, + Some("https://postil.dev/runs/1"), + ); + assert!(fallback.contains(super::OVERSIZED_REVIEW_MESSAGE)); + assert!(fallback.contains("[Review details](https://postil.dev/runs/1)")); + assert!(fallback.ends_with(marker)); + assert!(fallback.len() <= super::MAX_REVIEW_BODY_BYTES); + + let oversized_url = format!("https://postil.dev/{}", "x".repeat(60_000)); + let without_link = super::bounded_review_body( + &"x".repeat(super::MAX_REVIEW_BODY_BYTES), + marker, + Some(&oversized_url), + ); + assert!(!without_link.contains("Review details")); + assert!(without_link.len() <= super::MAX_REVIEW_BODY_BYTES); + } + + fn delivery_snapshot(head_sha: &str, target_sha: &str, merge_base_sha: &str) -> PrMeta { + PrMeta { + title: "t".into(), + body: "b".into(), + head_sha: head_sha.into(), + base_sha: merge_base_sha.into(), + target_sha: Some(target_sha.into()), + changed_files: Some(1), + } + } + + #[test] + fn github_retry_delays_honor_headers_with_a_hard_cap() { + let mut headers = HeaderMap::new(); + headers.insert("retry-after", HeaderValue::from_static("999999")); + assert_eq!( + github_retry_delay_at(reqwest::StatusCode::TOO_MANY_REQUESTS, &headers, 0, 1_000), + Duration::from_secs(10) + ); + + headers.remove("retry-after"); + headers.insert("x-ratelimit-reset", HeaderValue::from_static("999999")); + assert_eq!( + github_retry_delay_at(reqwest::StatusCode::FORBIDDEN, &headers, 0, 1_000), + Duration::from_secs(10) + ); + assert!(github_retryable_response( + reqwest::StatusCode::TOO_MANY_REQUESTS, + &headers + )); + assert_eq!(github_transport_retry_delay(0), Duration::from_millis(100)); + assert_eq!(github_transport_retry_delay(1), Duration::from_millis(200)); + } + + #[tokio::test] + async fn github_safe_requests_retry_transport_errors() { + let github = GitHub { + http: reqwest::Client::builder() + .timeout(Duration::from_millis(100)) + .build() + .unwrap(), + api_base: "http://127.0.0.1:9".into(), + details_url: None, + token: "test-token".into(), + owner: "owner".into(), + repo: "repo".into(), + pr: 1, + expected_repository_id: None, + }; + let started_at = std::time::Instant::now(); + let error = github + .send_retryable( + github.request(reqwest::Method::GET, github.url("/transport-failure")), + "transport test", + ) + .await + .unwrap_err(); + + assert!(error.to_string().contains("request failed")); + assert!(started_at.elapsed() >= Duration::from_millis(300)); + assert!(started_at.elapsed() < Duration::from_secs(2)); + } + + #[tokio::test] + async fn github_check_create_reconciles_before_retrying_uncertain_post() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/repos/owner/repo/check-runs")) + .respond_with(ResponseTemplate::new(500)) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/repos/owner/repo/commits/abcdef12/check-runs")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "check_runs": [{"id": 77, "external_id": "postil:postil/review:abcdef12"}] + }))) + .expect(1) + .mount(&server) + .await; + let github = test_github(&server); + let external_id = "postil:postil/review:abcdef12"; + let body = serde_json::json!({ + "name": "postil/review", + "head_sha": "abcdef12", + "status": "in_progress", + "external_id": external_id + }); + + let run = github + .create_check_run(&body, "abcdef12", "postil/review", external_id) + .await + .unwrap(); + + assert_eq!(run.id, 77); + } + + #[tokio::test] + async fn github_review_reconciles_before_retrying_uncertain_post() { + let server = MockServer::start().await; + mount_current_delivery_snapshot(&server).await; + Mock::given(method("POST")) + .and(path("/repos/owner/repo/pulls/1/reviews")) + .respond_with(ResponseTemplate::new(500)) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/repos/owner/repo/pulls/1/reviews")) + .respond_with( + ResponseTemplate::new(200).set_body_json(serde_json::json!([{ + "body": "summary\n\n", + "commit_id": "aaaaaaaaaaaa" + }])), + ) + .expect(1) + .mount(&server) + .await; + let github = test_github(&server); + let body = serde_json::json!({ + "body": "summary\n\n", + "commit_id": "aaaaaaaaaaaa", + "event": "COMMENT" + }); + let snapshot = delivery_snapshot("aaaaaaaaaaaa", "bbbbbbbbbbbb", "cccccccccccc"); + + let response = github + .send_review_reconciled( + &body, + "", + &snapshot, + "review post", + ) + .await + .unwrap(); + + assert!(matches!(response, super::ReviewDelivery::Reconciled(_))); + } + + #[tokio::test] + async fn github_comment_reconciles_before_retrying_uncertain_post() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/repos/owner/repo/issues/9/comments")) + .respond_with(ResponseTemplate::new(500)) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/repos/owner/repo/issues/9/comments")) + .respond_with( + ResponseTemplate::new(200).set_body_json(serde_json::json!([{ + "body": "reply\n\n" + }])), + ) + .expect(1) + .mount(&server) + .await; + let github = test_github(&server); + + github + .post_comment_reconciled( + 9, + "reply\n\n", + "", + ) + .await + .unwrap(); + } + + fn test_github(server: &MockServer) -> GitHub { + GitHub { + http: reqwest::Client::new(), + api_base: server.uri(), + details_url: None, + token: "test-token".into(), + owner: "owner".into(), + repo: "repo".into(), + pr: 1, + expected_repository_id: None, + } + } + + #[tokio::test] + async fn fetch_repository_file_at_revision_returns_text_and_rejects_binary() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/repos/owner/repo/contents/config/review.toml")) + .and(query_param("ref", "abc123")) + .respond_with(ResponseTemplate::new(200).set_body_string("enabled = true\n")) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/repos/owner/repo/contents/assets/data.bin")) + .and(query_param("ref", "def456")) + .respond_with(ResponseTemplate::new(200).set_body_bytes(vec![0xff, 0xfe])) + .expect(1) + .mount(&server) + .await; + let github = test_github(&server); + + let text = github + .fetch_repository_file_at_revision("abc123", "config/review.toml") + .await + .unwrap(); + assert_eq!(text, "enabled = true\n"); + + let error = github + .fetch_repository_file_at_revision("def456", "assets/data.bin") + .await + .unwrap_err(); + assert!(error.to_string().contains("not valid UTF-8")); + } + + #[tokio::test] + async fn fetch_repository_file_if_present_never_reads_a_base_revision() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/repos/owner/repo/contents/config/review.toml")) + .and(query_param("ref", "head123")) + .respond_with(ResponseTemplate::new(404)) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/repos/owner/repo/contents/config/review.toml")) + .and(query_param("ref", "base123")) + .respond_with(ResponseTemplate::new(200).set_body_string("enabled = true\n")) + .expect(0) + .mount(&server) + .await; + let github = test_github(&server); + let content = github + .fetch_repository_file_if_present("head123", "config/review.toml") + .await + .unwrap(); + + assert_eq!(content, None); + } + + #[tokio::test] + async fn fetch_repository_file_if_present_does_not_mask_head_failures() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/repos/owner/repo/contents/config/review.toml")) + .and(query_param("ref", "head123")) + .respond_with(ResponseTemplate::new(500)) + .expect(3) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/repos/owner/repo/contents/config/review.toml")) + .and(query_param("ref", "base123")) + .respond_with(ResponseTemplate::new(200).set_body_string("stale = true\n")) + .expect(0) + .mount(&server) + .await; + + let error = test_github(&server) + .fetch_repository_file_if_present("head123", "config/review.toml") + .await + .unwrap_err(); + + assert!(error.to_string().contains("500")); + } + + #[tokio::test] + async fn repository_search_walks_nested_trees_and_every_blob_at_the_pinned_head() { + use crate::envelope::{ + RepositoryClaim, RepositoryClaimKind, RepositorySearchQueryKind, RepositorySearchState, + }; + + let server = MockServer::start().await; + let head = "a".repeat(40); + let readme = b"CephCluster supports stable releases.\n"; + let generated = b"clusterVersion: 19.2.5\nimage: ceph:19.2.5\n"; + let symlink = b"../outside"; + let readme_blob = crate::repository_search::git_blob_sha1(readme); + let generated_blob = crate::repository_search::git_blob_sha1(generated); + let symlink_blob = crate::repository_search::git_blob_sha1(symlink); + let submodule = "1".repeat(40); + let manifest_tree = crate::repository_search::git_tree_sha1([( + "generated.yaml", + "100644", + generated_blob.as_str(), + )]); + let root_tree = crate::repository_search::git_tree_sha1([ + ("README.md", "100644", readme_blob.as_str()), + ("manifests", "040000", manifest_tree.as_str()), + ("outside-link", "120000", symlink_blob.as_str()), + ("vendor", "160000", submodule.as_str()), + ]); + + Mock::given(method("GET")) + .and(path(format!("/repos/owner/repo/git/commits/{head}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "sha": head, + "tree": {"sha": root_tree} + }))) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path(format!("/repos/owner/repo/git/trees/{root_tree}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "sha": root_tree, + "truncated": false, + "tree": [ + {"path": "README.md", "mode": "100644", "type": "blob", "sha": readme_blob, "size": readme.len()}, + {"path": "manifests", "mode": "040000", "type": "tree", "sha": manifest_tree}, + {"path": "outside-link", "mode": "120000", "type": "blob", "sha": symlink_blob, "size": symlink.len()}, + {"path": "vendor", "mode": "160000", "type": "commit", "sha": submodule} + ] + }))) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path(format!( + "/repos/owner/repo/git/trees/{manifest_tree}" + ))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "sha": manifest_tree, + "truncated": false, + "tree": [ + {"path": "generated.yaml", "mode": "100644", "type": "blob", "sha": generated_blob, "size": generated.len()} + ] + }))) + .expect(1) + .mount(&server) + .await; + for (blob, body) in [ + (readme_blob.as_str(), readme.as_slice()), + (generated_blob.as_str(), generated.as_slice()), + (symlink_blob.as_str(), symlink.as_slice()), + ] { + Mock::given(method("GET")) + .and(path(format!("/repos/owner/repo/git/blobs/{blob}"))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(body)) + .expect(1) + .mount(&server) + .await; + } + + let claim = RepositoryClaim { + kind: RepositoryClaimKind::Mismatch, + resources: vec!["CephCluster".into()], + values: vec!["outside-secret-term".into()], + versions: vec!["19.2.5".into()], + paths: vec!["manifests/generated.yaml".into()], + identifiers: vec!["clusterVersion".into()], + }; + let terms = crate::repository_search::search_terms(std::iter::once(&claim)).unwrap(); + let receipt = test_github(&server) + .search_repository_at_head(&head, terms) + .await; + + assert_eq!(receipt.head_sha.as_deref(), Some(head.as_str())); + assert_eq!(receipt.state, RepositorySearchState::Unavailable); + assert!(receipt.tree_sha256.is_some()); + assert_eq!(receipt.searched_blobs, 3); + assert_eq!( + receipt.searched_bytes, + (readme.len() + generated.len() + symlink.len()) as u64 + ); + assert!(receipt.queries.iter().any(|query| { + query.kind == RepositorySearchQueryKind::Path + && receipt.matched_query_sha256.contains(&query.query_sha256) + })); + assert!(receipt.matches.iter().any(|matched| { + matched.path == "manifests/generated.yaml" && matched.occurrences == 2 + })); + let outside = receipt + .queries + .iter() + .find(|query| query.kind == RepositorySearchQueryKind::Value) + .unwrap(); + assert!(!receipt.matched_query_sha256.contains(&outside.query_sha256)); + } + + #[tokio::test] + async fn repository_search_rejects_same_size_blob_substitution() { + use crate::envelope::RepositorySearchState; + + let server = MockServer::start().await; + let head = "a".repeat(40); + let expected = b"required-construct=true\n"; + let substituted = b"required-construct=fals\n"; + assert_eq!(expected.len(), substituted.len()); + let blob = crate::repository_search::git_blob_sha1(expected); + let tree = + crate::repository_search::git_tree_sha1([("config.txt", "100644", blob.as_str())]); + + Mock::given(method("GET")) + .and(path(format!("/repos/owner/repo/git/commits/{head}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "sha": head, + "tree": {"sha": tree} + }))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path(format!("/repos/owner/repo/git/trees/{tree}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "sha": tree, + "truncated": false, + "tree": [{ + "path": "config.txt", "mode": "100644", "type": "blob", + "sha": blob, "size": expected.len() + }] + }))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path(format!("/repos/owner/repo/git/blobs/{blob}"))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(substituted)) + .mount(&server) + .await; + + let receipt = test_github(&server) + .search_repository_at_head(&head, repository_search_terms()) + .await; + + assert_eq!(receipt.state, RepositorySearchState::Unavailable); + assert_eq!(receipt.searched_blobs, 0); + } + + #[tokio::test] + async fn repository_search_rejects_tree_entry_substitution() { + use crate::envelope::RepositorySearchState; + + let server = MockServer::start().await; + let head = "a".repeat(40); + let expected_blob = crate::repository_search::git_blob_sha1(b"expected\n"); + let substituted_blob = crate::repository_search::git_blob_sha1(b"attacker\n"); + let tree = crate::repository_search::git_tree_sha1([( + "config.txt", + "100644", + expected_blob.as_str(), + )]); + + Mock::given(method("GET")) + .and(path(format!("/repos/owner/repo/git/commits/{head}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "sha": head, + "tree": {"sha": tree} + }))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path(format!("/repos/owner/repo/git/trees/{tree}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "sha": tree, + "truncated": false, + "tree": [{ + "path": "config.txt", "mode": "100644", "type": "blob", + "sha": substituted_blob, "size": 9 + }] + }))) + .mount(&server) + .await; + + let receipt = test_github(&server) + .search_repository_at_head(&head, repository_search_terms()) + .await; + + assert_eq!(receipt.state, RepositorySearchState::Unavailable); + assert_eq!(receipt.searched_blobs, 0); + } + + #[tokio::test] + async fn repository_search_rejects_head_mutation_before_reading_a_tree() { + use crate::envelope::{RepositoryClaim, RepositoryClaimKind, RepositorySearchState}; + + let server = MockServer::start().await; + let head = "a".repeat(40); + let changed_head = "b".repeat(40); + let tree = "c".repeat(40); + Mock::given(method("GET")) + .and(path(format!("/repos/owner/repo/git/commits/{head}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "sha": changed_head, + "tree": {"sha": tree} + }))) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path(format!("/repos/owner/repo/git/trees/{tree}"))) + .respond_with(ResponseTemplate::new(200)) + .expect(0) + .mount(&server) + .await; + let claim = RepositoryClaim { + kind: RepositoryClaimKind::Absence, + resources: vec![], + values: vec![], + versions: vec![], + paths: vec![], + identifiers: vec!["identifier".into()], + }; + let terms = crate::repository_search::search_terms(std::iter::once(&claim)).unwrap(); + + let receipt = test_github(&server) + .search_repository_at_head(&head, terms) + .await; + assert_eq!(receipt.state, RepositorySearchState::Unavailable); + assert_eq!(receipt.queries.len(), 1); + } + + #[tokio::test] + async fn repository_search_reports_exhaustion_before_fetching_oversized_blobs() { + use crate::envelope::RepositorySearchState; + + let server = MockServer::start().await; + let head = "a".repeat(40); + let blob = "c".repeat(40); + let tree = crate::repository_search::git_tree_sha1([("huge.bin", "100644", blob.as_str())]); + Mock::given(method("GET")) + .and(path(format!("/repos/owner/repo/git/commits/{head}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "sha": head, + "tree": {"sha": tree} + }))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path(format!("/repos/owner/repo/git/trees/{tree}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "sha": tree, + "truncated": false, + "tree": [{ + "path": "huge.bin", "mode": "100644", "type": "blob", "sha": blob, + "size": crate::repository_search::search_byte_cap() + 1 + }] + }))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path(format!("/repos/owner/repo/git/blobs/{blob}"))) + .respond_with(ResponseTemplate::new(200)) + .expect(0) + .mount(&server) + .await; + + let receipt = test_github(&server) + .search_repository_at_head(&head, repository_search_terms()) + .await; + assert_eq!(receipt.state, RepositorySearchState::Exhausted); + } + + #[tokio::test] + async fn repository_search_rejects_hostile_tree_paths_and_truncated_trees() { + use crate::envelope::RepositorySearchState; + + for (entry_path, truncated) in [("../escape", false), ("safe", true)] { + let server = MockServer::start().await; + let head = "a".repeat(40); + let tree = "b".repeat(40); + let blob = "c".repeat(40); + Mock::given(method("GET")) + .and(path(format!("/repos/owner/repo/git/commits/{head}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "sha": head, + "tree": {"sha": tree} + }))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path(format!("/repos/owner/repo/git/trees/{tree}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "sha": tree, + "truncated": truncated, + "tree": [{ + "path": entry_path, "mode": "100644", "type": "blob", "sha": blob, + "size": 1 + }] + }))) + .mount(&server) + .await; + + let receipt = test_github(&server) + .search_repository_at_head(&head, repository_search_terms()) + .await; + assert_eq!(receipt.state, RepositorySearchState::Unavailable); + } + } + + #[tokio::test] + async fn repository_search_with_zero_terms_makes_no_github_requests() { + use crate::envelope::RepositorySearchState; + + let server = MockServer::start().await; + let receipt = test_github(&server) + .search_repository_at_head(&"a".repeat(40), vec![]) + .await; + + assert_eq!(receipt.state, RepositorySearchState::Unavailable); + assert!(server.received_requests().await.unwrap().is_empty()); + } + + #[test] + fn repository_search_budget_enforces_request_object_and_deadline_caps() { + let mut requests = RepositorySearchBudget::new(); + for _ in 0..crate::repository_search::github_request_cap() { + requests.charge_request().unwrap(); + } + assert!(requests.charge_request().is_err()); + + let mut objects = RepositorySearchBudget::new(); + objects + .charge_objects(crate::repository_search::github_object_cap()) + .unwrap(); + assert!(objects.charge_objects(1).is_err()); + + let mut expired = RepositorySearchBudget::new(); + expired.started_at = Instant::now() - crate::repository_search::github_aggregate_deadline(); + assert!(expired.remaining().is_err()); + } + + #[tokio::test] + async fn repository_search_reports_rate_limit_risk_as_exhausted() { + use crate::envelope::RepositorySearchState; + + let server = MockServer::start().await; + let head = "a".repeat(40); + Mock::given(method("GET")) + .and(path(format!("/repos/owner/repo/git/commits/{head}"))) + .respond_with(ResponseTemplate::new(403).insert_header("x-ratelimit-remaining", "0")) + .expect(1) + .mount(&server) + .await; + + let receipt = test_github(&server) + .search_repository_at_head(&head, repository_search_terms()) + .await; + assert_eq!(receipt.state, RepositorySearchState::Exhausted); + } + + #[test] + fn repository_rate_limit_risk_distinguishes_permission_failures() { + assert!(!github_repository_rate_limit_risk( + reqwest::StatusCode::FORBIDDEN, + &HeaderMap::new(), + )); + let mut limited = HeaderMap::new(); + limited.insert("x-ratelimit-remaining", HeaderValue::from_static("0")); + assert!(github_repository_rate_limit_risk( + reqwest::StatusCode::OK, + &limited, + )); + } + + #[test] + fn publication_plan_operation_keys_match_every_key_safe_contract_shape() { + let expected = [ + ( + "initial-review-create", + "05d5806e72114f105b5b1e2809be8651811a3e527d89f7455c20dedbf32f24ce", + ), + ( + "relocated-review-create", + "178636b4f224fdfb105833cde04ca2f5b1cf74bf407b9ab2dd23938198467058", + ), + ( + "summary-review-create", + "7ca44380841305baacfac07805bc41c4b308f95acd0278b3e5ed3bfca4f7d8a0", + ), + ( + "file-comment-fallback", + "6e61767bbe33ef6d754950c5bcce236fbeb92df235005db907731caa96e047dc", + ), + ( + "finding-comment-update", + "875e393c65ef65394d4e73148f3369bad46f1d23d064cb906b3a8a69feb38d2f", + ), + ( + "review-summary-update", + "e575a792bbe3c702f35bd4218263a9cf160b841c1e898c8045802530d56cac5f", + ), + ( + "advisory-check-create", + "3be1458de77b6d90c70ead018e3897dddf217dc3bb5c0a68ba51b3f4129556d4", + ), + ( + "advisory-check-complete", + "2c9ec6990843d0012a5489aa99a2fec170326e1a36f48a38fc51a73b42af33d7", + ), + ]; + for (kind, (expected_kind, expected_digest)) in super::PublicationPlanOperationKeyKind::ALL + .into_iter() + .zip(expected) + { + assert_eq!(kind.as_str(), expected_kind); + let finding_id = matches!( + kind, + super::PublicationPlanOperationKeyKind::FileCommentFallback + | super::PublicationPlanOperationKeyKind::FindingCommentUpdate + ) + .then_some("finding-1"); + let key = super::publication_plan_operation_key( + publication_plan_key_scope( + "head-1", + PUBLICATION_INPUT_IDENTITY, + "sha256:2222222222222222222222222222222222222222222222222222222222222222", + ), + kind, + finding_id, + ); + assert_eq!( + key, + format!("github-publication-v1:{expected_kind}:sha256:{expected_digest}") + ); + assert!(!key.contains('/')); } } - fn repository_search_terms() -> Vec { - use crate::envelope::{RepositoryClaim, RepositoryClaimKind}; - crate::repository_search::search_terms(std::iter::once(&RepositoryClaim { - kind: RepositoryClaimKind::Absence, - resources: vec![], - values: vec![], - versions: vec![], - paths: vec![], - identifiers: vec!["clusterVersion".into()], - })) - .unwrap() + #[test] + fn advisory_check_completion_depends_on_creation_and_terminal_review_work() { + let operation = |operation_key: &str| { + PublicationPlanOperation::new( + 0, + operation_key.into(), + vec![], + PublicationPlanOperationActivation { + any_of: vec![PublicationPlanActivationCondition::Always], + }, + PublicationPlanOperationReconciliation { + logical_identity: operation_key.into(), + markers: vec![], + observed_remote_id: None, + exclusive: true, + }, + PublicationPlanOperationKind::AdvisoryCheckCreate { + name: "postil/review".into(), + head_sha: "aaaaaaaaaaaa".into(), + status: PublicationPlanCheckStatus::InProgress, + external_id: "postil:postil/review:aaaaaaaaaaaa".into(), + details_url: None, + }, + ) + .unwrap() + }; + let preceding = vec![ + operation("advisory-create"), + operation("finding-update"), + operation("file-fallback"), + ]; + + assert_eq!( + super::publication_plan_advisory_completion_dependencies( + &preceding, + "advisory-create", + None, + ), + vec!["advisory-create", "finding-update", "file-fallback"] + ); + assert_eq!( + super::publication_plan_advisory_completion_dependencies( + &preceding, + "advisory-create", + Some("review-summary-update"), + ), + vec!["advisory-create", "review-summary-update"] + ); } #[test] - fn hosted_repository_identity_environment_contract_is_stable() { - assert_eq!(EXPECTED_REPOSITORY_ID_ENV, "POSTIL_EXPECTED_GITHUB_REPO_ID"); + fn same_head_rereviews_bind_remote_identity_to_generation_and_content() { + let envelope = delivery_envelope_with_findings( + "aaaaaaaaaaaa", + "cccccccccccc", + vec![publication_finding( + "finding-1", + "src/lib.rs", + "Original finding body.", + )], + ); + let snapshot = delivery_snapshot("aaaaaaaaaaaa", "bbbbbbbbbbbb", "cccccccccccc"); + let receipt = super::planned_review_receipt(&envelope, &snapshot.head_sha); + let identity = |generation: &str, snapshot: &PrMeta, envelope: &Envelope, gate| { + super::publication_plan_review_output_digest(super::PublicationPlanReviewOutputInput { + controller_generation: generation, + input_identity: PUBLICATION_INPUT_IDENTITY, + repository_id: "42", + pull_request_number: "7", + snapshot, + envelope, + receipt: &receipt, + should_comment: true, + duplicate_of_baseline: false, + annotate_findings: false, + advisory: CheckState::Success, + gate, + details_url: None, + }) + .unwrap() + }; + let baseline = identity("1", &snapshot, &envelope, CheckState::Success); + let mut changed_title = snapshot.clone(); + changed_title.title = "Changed title".into(); + let mut changed_body = snapshot.clone(); + changed_body.body = "Changed pull request body".into(); + let mut changed_finding = envelope.clone(); + changed_finding.findings[0].body = "Changed finding body.".into(); + let mut changed_confidence = envelope.clone(); + changed_confidence.findings[0].confidence = 0.73; + let variants = [ + identity("2", &snapshot, &envelope, CheckState::Success), + identity("1", &changed_title, &envelope, CheckState::Success), + identity("1", &changed_body, &envelope, CheckState::Success), + identity("1", &snapshot, &changed_finding, CheckState::Success), + identity("1", &snapshot, &changed_confidence, CheckState::Success), + identity("1", &snapshot, &envelope, CheckState::Failure), + ]; + for variant in variants { + assert_ne!(variant, baseline); + let variant_scope = publication_plan_key_scope( + &snapshot.head_sha, + PUBLICATION_INPUT_IDENTITY, + &variant, + ); + let baseline_scope = publication_plan_key_scope( + &snapshot.head_sha, + PUBLICATION_INPUT_IDENTITY, + &baseline, + ); + assert_ne!( + super::publication_plan_logical_review_identity(variant_scope), + super::publication_plan_logical_review_identity(baseline_scope) + ); + assert_ne!( + super::publication_plan_operation_key( + variant_scope, + super::PublicationPlanOperationKeyKind::InitialReviewCreate, + None, + ), + super::publication_plan_operation_key( + baseline_scope, + super::PublicationPlanOperationKeyKind::InitialReviewCreate, + None, + ) + ); + } + + let changed_input_identity = + "sha256:3333333333333333333333333333333333333333333333333333333333333333"; + let changed_input_output = + super::publication_plan_review_output_digest(super::PublicationPlanReviewOutputInput { + controller_generation: "1", + input_identity: changed_input_identity, + repository_id: "42", + pull_request_number: "7", + snapshot: &snapshot, + envelope: &envelope, + receipt: &receipt, + should_comment: true, + duplicate_of_baseline: false, + annotate_findings: false, + advisory: CheckState::Success, + gate: CheckState::Success, + details_url: None, + }) + .unwrap(); + assert_ne!(changed_input_output, baseline); + assert_ne!( + super::publication_plan_operation_key( + publication_plan_key_scope( + &snapshot.head_sha, + changed_input_identity, + &changed_input_output, + ), + super::PublicationPlanOperationKeyKind::InitialReviewCreate, + None, + ), + super::publication_plan_operation_key( + publication_plan_key_scope( + &snapshot.head_sha, + PUBLICATION_INPUT_IDENTITY, + &baseline, + ), + super::PublicationPlanOperationKeyKind::InitialReviewCreate, + None, + ) + ); } #[test] - fn review_summary_body_is_bounded_before_github_publication() { - let marker = ""; + fn durable_markers_are_strong_and_accept_released_compatibility_shapes() { + let finding = publication_finding("finding-1", "src/lib.rs", "Body."); + let finding_markers = super::finding_marker_candidates(&finding); + assert_eq!(finding_markers.len(), 2); + assert!(finding_markers[0].starts_with("".len() + 64 ); - let fallback = super::bounded_review_body( - &"x".repeat(super::MAX_REVIEW_BODY_BYTES), - marker, - Some("https://postil.dev/runs/1"), + assert!(finding_markers[1].starts_with("".len() + 12 ); - assert!(fallback.contains(super::OVERSIZED_REVIEW_MESSAGE)); - assert!(fallback.contains("[Review details](https://postil.dev/runs/1)")); - assert!(fallback.ends_with(marker)); - assert!(fallback.len() <= super::MAX_REVIEW_BODY_BYTES); - let oversized_url = format!("https://postil.dev/{}", "x".repeat(60_000)); - let without_link = super::bounded_review_body( - &"x".repeat(super::MAX_REVIEW_BODY_BYTES), - marker, - Some(&oversized_url), + let current_receipt = super::planned_review_receipt( + &delivery_envelope_with_findings("aaaaaaaaaaaa", "cccccccccccc", vec![finding]), + "aaaaaaaaaaaa", + ); + let current_review_marker = super::review_marker(¤t_receipt.receipt_id); + assert!(current_review_marker.starts_with("".len() + 64 + ); + let released_marker = super::legacy_review_marker("github-review-v2:0123456789ab"); + assert!(released_marker.starts_with("".len() + 12 ); - assert!(!without_link.contains("Review details")); - assert!(without_link.len() <= super::MAX_REVIEW_BODY_BYTES); } - fn delivery_snapshot(head_sha: &str, target_sha: &str, merge_base_sha: &str) -> PrMeta { - PrMeta { - title: "t".into(), - body: "b".into(), - head_sha: head_sha.into(), - base_sha: merge_base_sha.into(), - target_sha: Some(target_sha.into()), - changed_files: Some(1), - } + async fn mount_current_delivery_snapshot(server: &MockServer) { + Mock::given(method("GET")) + .and(path("/repos/owner/repo/pulls/1")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "title": "t", "body": "b", "state": "open", "merged": false, + "head": {"sha": "aaaaaaaaaaaa"}, + "base": {"sha": "bbbbbbbbbbbb"}, + "changed_files": 1 + }))) + .mount(server) + .await; + Mock::given(method("GET")) + .and(path( + "/repos/owner/repo/compare/bbbbbbbbbbbb...aaaaaaaaaaaa", + )) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "merge_base_commit": {"sha": "cccccccccccc"}, + "files": [] + }))) + .mount(server) + .await; } - #[test] - fn github_retry_delays_honor_headers_with_a_hard_cap() { - let mut headers = HeaderMap::new(); - headers.insert("retry-after", HeaderValue::from_static("999999")); + async fn mount_no_existing_review(server: &MockServer) { + Mock::given(method("GET")) + .and(path("/repos/owner/repo/pulls/1/reviews")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!([]))) + .mount(server) + .await; + } + + #[tokio::test] + async fn publication_plan_reuses_review_checks_markers_and_the_placement_ladder() { + let server = MockServer::start().await; + mount_current_delivery_snapshot(&server).await; + mount_no_existing_review(&server).await; + Mock::given(method("GET")) + .and(path("/repos/owner/repo")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "id": 42, + "full_name": "owner/repo" + }))) + .expect(2) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/repos/owner/repo/pulls/1/comments")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!([]))) + .expect(2) + .mount(&server) + .await; + + let mut active = publication_finding( + "active-1", + "src/lib.rs", + "The unchecked value reaches the protected operation.", + ); + active.severity = Severity::Error; + let mut carried = publication_finding( + "carried-1", + "src/carried.rs", + "The carried finding remains open.", + ); + carried.body = format!( + "{} The carried finding remains open.", + crate::filter::CARRIED_MARKER + ); + let summary_only = publication_finding( + "summary-1", + crate::envelope::CHANGE_METADATA_PATH, + "The metadata finding belongs in the summary.", + ); + let resolved = publication_finding( + "resolved-1", + "src/resolved.rs", + "The resolved finding remains identifiable.", + ); + let suppressed = publication_finding( + "suppressed-1", + "src/suppressed.rs", + "The suppressed finding remains identifiable.", + ); + let mut envelope = delivery_envelope_with_findings( + "aaaaaaaaaaaa", + "cccccccccccc", + vec![active, carried, summary_only], + ); + envelope.resolved.push(resolved); + envelope.suppressed_findings.push(SuppressedFinding { + finding: suppressed, + reason: SuppressionReason::BelowConfidence, + }); + let snapshot = delivery_snapshot("aaaaaaaaaaaa", "bbbbbbbbbbbb", "cccccccccccc"); + let planner = || { + let mut github = test_github(&server); + github.details_url = Some("https://postil.dev/orgs/acme/runs/run-1".into()); + github + }; + let plan = planner() + .build_publication_plan(crate::forge::GitHubPublicationPlanRequest { + controller_generation: "1", + input_identity: PUBLICATION_INPUT_IDENTITY, + envelope: &envelope, + snapshot: &snapshot, + publication_diff: Some(&placement_diff()), + should_comment: true, + duplicate_of_baseline: false, + annotate_findings: false, + advisory: CheckState::Success, + gate: CheckState::Failure, + }) + .await + .unwrap(); + let mut reordered_envelope = envelope.clone(); + reordered_envelope.findings.reverse(); + reordered_envelope.resolved.reverse(); + reordered_envelope.suppressed_findings.reverse(); + let reordered_plan = planner() + .build_publication_plan(crate::forge::GitHubPublicationPlanRequest { + controller_generation: "1", + input_identity: PUBLICATION_INPUT_IDENTITY, + envelope: &reordered_envelope, + snapshot: &snapshot, + publication_diff: Some(&placement_diff()), + should_comment: true, + duplicate_of_baseline: false, + annotate_findings: false, + advisory: CheckState::Success, + gate: CheckState::Failure, + }) + .await + .unwrap(); assert_eq!( - github_retry_delay_at(reqwest::StatusCode::TOO_MANY_REQUESTS, &headers, 0, 1_000), - Duration::from_secs(10) + serde_json::to_vec(&plan).unwrap(), + serde_json::to_vec(&reordered_plan).unwrap(), + "provider finding order must not affect publication intent" ); - - headers.remove("retry-after"); - headers.insert("x-ratelimit-reset", HeaderValue::from_static("999999")); + let serialized = serde_json::to_value(&plan).unwrap(); + let operations = serialized["operations"].as_array().unwrap(); assert_eq!( - github_retry_delay_at(reqwest::StatusCode::FORBIDDEN, &headers, 0, 1_000), - Duration::from_secs(10) + operations + .iter() + .map(|operation| operation["kind"].as_str().unwrap()) + .collect::>(), + vec![ + "advisoryCheckCreate", + "reviewCreate", + "reviewCreate", + "reviewCreate", + "fileCommentFallback", + "reviewSummaryUpdate", + "advisoryCheckComplete", + ] ); - assert!(github_retryable_response( - reqwest::StatusCode::TOO_MANY_REQUESTS, - &headers - )); - assert_eq!(github_transport_retry_delay(0), Duration::from_millis(100)); - assert_eq!(github_transport_retry_delay(1), Duration::from_millis(200)); - } + assert_eq!(operations[0]["ordinal"], 1); + assert_eq!(operations[1]["ordinal"], 2); + assert_eq!(operations[2]["ordinal"], 3); + assert_eq!(operations[0]["name"], "postil/review"); + assert_eq!(operations[0]["headSha"], "aaaaaaaaaaaa"); + assert_eq!(operations[0]["status"], "in_progress"); + assert_eq!( + operations[0]["externalId"], + "postil:run-1:postil/review:aaaaaaaaaaaa" + ); + assert_eq!( + operations[0]["detailsUrl"], + "https://postil.dev/orgs/acme/runs/run-1" + ); + assert_eq!(operations[1]["attempt"], "initial"); + assert_eq!(operations[2]["attempt"], "relocatedInline"); + assert_eq!(operations[3]["attempt"], "summaryOnly"); + assert_eq!(operations[1]["payload"]["comments"][0]["line"], 7); + assert_eq!( + operations[2]["payload"]["comments"][0]["path"], + "src/lib.rs" + ); + assert!(operations[3]["payload"]["body"].is_string()); + let logical_review_identity = operations[1]["logicalReviewIdentity"].as_str().unwrap(); + for operation in &operations[1..4] { + assert_eq!(operation["logicalReviewIdentity"], logical_review_identity); + assert_eq!( + operation["reconciliation"]["logicalIdentity"], + logical_review_identity + ); + assert_eq!(operation["reconciliation"]["exclusive"], true); + assert_eq!( + operation["reconciliation"]["markers"] + .as_array() + .unwrap() + .len(), + 2 + ); + } + assert_eq!( + operations[2]["activation"]["anyOf"][0]["condition"], + "semanticPlacementRejected" + ); + assert_eq!(operations[2]["activation"]["anyOf"][0]["httpStatus"], 422); + assert_eq!( + operations[2]["activation"]["anyOf"][0]["classification"], + "invalidReviewCommentPlacement" + ); + assert_eq!( + operations[2]["activation"]["anyOf"][0]["markerAbsence"], + operations[1]["activation"]["anyOf"][0]["guard"] + ); + assert_eq!( + operations[3]["activation"]["anyOf"][0]["markerAbsence"], + operations[1]["activation"]["anyOf"][0]["guard"] + ); + let findings = serialized["lifecycleReceipt"]["findings"] + .as_array() + .unwrap(); + assert_eq!( + findings + .iter() + .map(|finding| finding["findingId"].as_str().unwrap()) + .collect::>(), + vec![ + "active-1", + "carried-1", + "resolved-1", + "summary-1", + "suppressed-1" + ] + ); + assert_eq!(findings[0]["initialOutcome"], "inline"); + assert_eq!( + findings[0]["fallbackIntent"], + serde_json::json!(["relocatedInline", "fileComment"]) + ); + assert_eq!(findings[1]["initialOutcome"], "carried"); + assert_eq!(findings[2]["initialOutcome"], "resolved"); + assert_eq!(findings[3]["initialOutcome"], "summaryOnly"); + assert_eq!(findings[4]["initialOutcome"], "suppressed"); + assert_eq!(findings[4]["suppressionReason"], "belowConfidence"); + assert!( + serialized["lifecycleReceipt"]["digest"] + .as_str() + .unwrap() + .starts_with("sha256:") + ); + assert_eq!(operations[4]["findingId"], "active-1"); + assert_eq!( + operations[4]["activation"]["anyOf"][0]["condition"], + "semanticPlacementRejected" + ); + assert_eq!( + operations[4]["activation"]["anyOf"][1]["condition"], + "partialReviewObserved" + ); + assert_eq!( + operations[4]["activation"]["anyOf"][1]["findingMarkerAbsence"]["markers"], + operations[4]["reconciliation"]["markers"] + ); + assert_eq!( + operations[5]["terminalOperations"][0]["findingId"], + "active-1" + ); + assert_eq!( + operations[5]["terminalOperations"][0]["requiresRemoteId"], + true + ); + assert_eq!(operations[5]["cases"][0]["fileCommentCount"], 0); + assert_eq!(operations[5]["cases"][1]["fileCommentCount"], 1); + assert_eq!( + operations[5]["cases"][1]["selectedReviewOutcomes"], + serde_json::json!(["partialObserved"]) + ); + assert_eq!(operations[6]["name"], "postil/review"); + assert_eq!(operations[6]["conclusion"], "success"); + assert_eq!( + operations[6]["dependencies"], + serde_json::json!([ + operations[0]["operationKey"].clone(), + operations[5]["operationKey"].clone() + ]) + ); + assert_eq!( + operations[6]["createdCheck"]["dependencyOperationKey"], + operations[0]["operationKey"] + ); + assert_eq!(operations[6]["createdCheck"]["resultField"], "remoteId"); + assert_eq!(serialized["repository"]["id"], "42"); + assert_eq!(serialized["pullRequestNumber"], "1"); + assert_eq!(serialized["gateAnalysis"]["ownership"], "service"); + assert_eq!(serialized["gateAnalysis"]["authoritative"], false); + assert_eq!( + serialized["gateAnalysis"]["organizationGateModeRequired"], + true + ); + assert_eq!(serialized["gateAnalysis"]["name"], "postil/gate"); + assert_eq!(serialized["gateAnalysis"]["analyzedConclusion"], "failure"); + assert!(operations.iter().all(|operation| { + operation["kind"] != "gateCheck" && operation["name"] != "postil/gate" + })); + assert_eq!(plan.recompute_intent_digest().unwrap(), plan.intent_digest); - #[tokio::test] - async fn github_safe_requests_retry_transport_errors() { - let github = GitHub { - http: reqwest::Client::builder() - .timeout(Duration::from_millis(100)) - .build() - .unwrap(), - api_base: "http://127.0.0.1:9".into(), - details_url: None, - token: "test-token".into(), - owner: "owner".into(), - repo: "repo".into(), - pr: 1, - expected_repository_id: None, - }; - let started_at = std::time::Instant::now(); - let error = github - .send_retryable( - github.request(reqwest::Method::GET, github.url("/transport-failure")), - "transport test", + let requests = server.received_requests().await.unwrap(); + assert!(requests.iter().all(|request| { + !matches!( + request.method, + wiremock::http::Method::POST + | wiremock::http::Method::PATCH + | wiremock::http::Method::PUT + | wiremock::http::Method::DELETE ) - .await - .unwrap_err(); - - assert!(error.to_string().contains("request failed")); - assert!(started_at.elapsed() >= Duration::from_millis(300)); - assert!(started_at.elapsed() < Duration::from_secs(2)); + })); } #[tokio::test] - async fn github_check_create_reconciles_before_retrying_uncertain_post() { + async fn publication_plan_recovers_a_partial_review_without_an_alternative_create() { let server = MockServer::start().await; - Mock::given(method("POST")) - .and(path("/repos/owner/repo/check-runs")) - .respond_with(ResponseTemplate::new(500)) - .expect(1) - .mount(&server) - .await; + mount_current_delivery_snapshot(&server).await; + let first = publication_finding( + "first", + "src/lib.rs", + "The first finding was observed in the partial review.", + ); + let second = publication_finding( + "second", + "src/lib.rs", + "The second finding is missing from the partial review.", + ); + let envelope = delivery_envelope_with_findings( + "aaaaaaaaaaaa", + "cccccccccccc", + vec![second, first.clone()], + ); + let legacy_review_marker = "".to_string(); + let first_body = super::initial_review_comment(&first)["body"] + .as_str() + .unwrap() + .to_string(); Mock::given(method("GET")) - .and(path("/repos/owner/repo/commits/abcdef12/check-runs")) + .and(path("/repos/owner/repo")) .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ - "check_runs": [{"id": 77, "external_id": "postil:postil/review:abcdef12"}] + "id": 42, + "full_name": "owner/repo" }))) - .expect(1) - .mount(&server) - .await; - let github = test_github(&server); - let external_id = "postil:postil/review:abcdef12"; - let body = serde_json::json!({ - "name": "postil/review", - "head_sha": "abcdef12", - "status": "in_progress", - "external_id": external_id - }); - - let run = github - .create_check_run(&body, "abcdef12", "postil/review", external_id) - .await - .unwrap(); - - assert_eq!(run.id, 77); - } - - #[tokio::test] - async fn github_review_reconciles_before_retrying_uncertain_post() { - let server = MockServer::start().await; - Mock::given(method("POST")) - .and(path("/repos/owner/repo/pulls/1/reviews")) - .respond_with(ResponseTemplate::new(500)) - .expect(1) + .expect(2) .mount(&server) .await; Mock::given(method("GET")) - .and(path("/repos/owner/repo/pulls/1/reviews")) + .and(path("/repos/owner/repo/pulls/1/comments")) .respond_with( ResponseTemplate::new(200).set_body_json(serde_json::json!([{ - "body": "summary\n\n", - "commit_id": "abcdef12" + "id": 501, + "body": first_body, + "commit_id": "aaaaaaaaaaaa" }])), ) - .expect(1) - .mount(&server) - .await; - let github = test_github(&server); - let body = serde_json::json!({ - "body": "summary\n\n", - "commit_id": "abcdef12", - "event": "COMMENT" - }); - - let response = github - .send_review_reconciled( - &body, - "", - "abcdef12", - "review post", - ) - .await - .unwrap(); - - assert!(matches!(response, super::ReviewDelivery::Reconciled(_))); - } - - #[tokio::test] - async fn github_comment_reconciles_before_retrying_uncertain_post() { - let server = MockServer::start().await; - Mock::given(method("POST")) - .and(path("/repos/owner/repo/issues/9/comments")) - .respond_with(ResponseTemplate::new(500)) - .expect(1) + .expect(2) .mount(&server) .await; Mock::given(method("GET")) - .and(path("/repos/owner/repo/issues/9/comments")) + .and(path("/repos/owner/repo/pulls/1/reviews")) .respond_with( ResponseTemplate::new(200).set_body_json(serde_json::json!([{ - "body": "reply\n\n" + "id": 77, + "body": legacy_review_marker.clone(), + "commit_id": "aaaaaaaaaaaa" }])), ) - .expect(1) + .expect(2) .mount(&server) .await; - let github = test_github(&server); - - github - .post_comment_reconciled( - 9, - "reply\n\n", - "", - ) - .await - .unwrap(); - } - fn test_github(server: &MockServer) -> GitHub { - GitHub { - http: reqwest::Client::new(), - api_base: server.uri(), - details_url: None, - token: "test-token".into(), - owner: "owner".into(), - repo: "repo".into(), - pr: 1, - expected_repository_id: None, + let snapshot = delivery_snapshot("aaaaaaaaaaaa", "bbbbbbbbbbbb", "cccccccccccc"); + let build = || async { + test_github(&server) + .build_publication_plan(crate::forge::GitHubPublicationPlanRequest { + controller_generation: "3", + input_identity: PUBLICATION_INPUT_IDENTITY, + envelope: &envelope, + snapshot: &snapshot, + publication_diff: Some(&placement_diff()), + should_comment: true, + duplicate_of_baseline: false, + annotate_findings: false, + advisory: CheckState::Success, + gate: CheckState::Success, + }) + .await + .unwrap() + }; + let first_plan = build().await; + let second_plan = build().await; + assert_eq!( + serde_json::to_vec(&first_plan).unwrap(), + serde_json::to_vec(&second_plan).unwrap() + ); + let plan = serde_json::to_value(first_plan).unwrap(); + let findings = plan["lifecycleReceipt"]["findings"].as_array().unwrap(); + assert_eq!(findings[0]["findingId"], "first"); + assert_eq!(findings[0]["observedCommentId"], "501"); + assert_eq!(findings[0]["observedOutcome"], "inline"); + assert_eq!(findings[0]["reconciliation"], "retain"); + assert_eq!(findings[1]["findingId"], "second"); + assert!(findings[1].get("observedCommentId").is_none()); + assert_eq!(plan["lifecycleReceipt"]["observedReviewId"], "77"); + + let operations = plan["operations"].as_array().unwrap(); + let review_operations = &operations[1..4]; + let logical_identity = review_operations[0]["logicalReviewIdentity"].clone(); + let shared_markers = review_operations[0]["reconciliation"]["markers"].clone(); + assert!( + shared_markers + .as_array() + .unwrap() + .contains(&serde_json::json!(legacy_review_marker)) + ); + let exact_marker_guard = review_operations[0]["activation"]["anyOf"][0]["guard"].clone(); + for operation in review_operations { + assert_eq!(operation["logicalReviewIdentity"], logical_identity); + assert_eq!(operation["reconciliation"]["markers"], shared_markers); + assert_eq!(operation["reconciliation"]["observedRemoteId"], "77"); + assert_eq!(operation["reconciliation"]["exclusive"], true); } - } + assert_eq!( + review_operations[1]["activation"]["anyOf"][0]["markerAbsence"], + exact_marker_guard + ); + assert_eq!( + review_operations[2]["activation"]["anyOf"][0]["markerAbsence"], + exact_marker_guard + ); - #[tokio::test] - async fn fetch_repository_file_at_revision_returns_text_and_rejects_binary() { - let server = MockServer::start().await; - Mock::given(method("GET")) - .and(path("/repos/owner/repo/contents/config/review.toml")) - .and(query_param("ref", "abc123")) - .respond_with(ResponseTemplate::new(200).set_body_string("enabled = true\n")) - .expect(1) - .mount(&server) - .await; - Mock::given(method("GET")) - .and(path("/repos/owner/repo/contents/assets/data.bin")) - .and(query_param("ref", "def456")) - .respond_with(ResponseTemplate::new(200).set_body_bytes(vec![0xff, 0xfe])) - .expect(1) - .mount(&server) - .await; - let github = test_github(&server); + let file_fallback = operations + .iter() + .find(|operation| operation["kind"] == "fileCommentFallback") + .unwrap(); + assert_eq!(file_fallback["findingId"], "second"); + let partial_conditions = file_fallback["activation"]["anyOf"] + .as_array() + .unwrap() + .iter() + .filter(|condition| condition["condition"] == "partialReviewObserved") + .collect::>(); + assert_eq!(partial_conditions.len(), 2); + for condition in partial_conditions { + assert_eq!(condition["reviewMarkers"], shared_markers); + assert_eq!( + condition["findingMarkerAbsence"]["markers"], + file_fallback["reconciliation"]["markers"] + ); + assert_eq!(condition["findingMarkerAbsence"]["required"], true); + } - let text = github - .fetch_repository_file_at_revision("abc123", "config/review.toml") - .await + let final_summary = operations + .iter() + .find(|operation| operation["kind"] == "reviewSummaryUpdate") .unwrap(); - assert_eq!(text, "enabled = true\n"); + assert!( + final_summary["dependencies"] + .as_array() + .unwrap() + .contains(&file_fallback["operationKey"]) + ); + assert_eq!( + final_summary["terminalOperations"] + .as_array() + .unwrap() + .len(), + 1 + ); + assert_eq!( + final_summary["terminalOperations"][0]["findingId"], + "second" + ); + let partial_case = final_summary["cases"] + .as_array() + .unwrap() + .iter() + .find(|case| { + case["selectedReviewOperationKey"] == review_operations[0]["operationKey"] + && case["selectedReviewOutcomes"] == serde_json::json!(["partialObserved"]) + && case["fileCommentCount"] == 1 + }) + .expect("partial observation has a truthful final summary case"); + assert!( + partial_case["body"] + .as_str() + .unwrap() + .contains("file-level") + ); - let error = github - .fetch_repository_file_at_revision("def456", "assets/data.bin") - .await - .unwrap_err(); - assert!(error.to_string().contains("not valid UTF-8")); + let requests = server.received_requests().await.unwrap(); + assert!(requests.iter().all(|request| { + !matches!( + request.method, + wiremock::http::Method::POST + | wiremock::http::Method::PATCH + | wiremock::http::Method::PUT + | wiremock::http::Method::DELETE + ) + })); } #[tokio::test] - async fn fetch_repository_file_if_present_never_reads_a_base_revision() { + async fn publication_plan_updates_stale_same_head_finding_content() { let server = MockServer::start().await; + mount_current_delivery_snapshot(&server).await; + let finding = publication_finding( + "stable-finding", + "src/lib.rs", + "The desired finding body is current.", + ); + let envelope = + delivery_envelope_with_findings("aaaaaaaaaaaa", "cccccccccccc", vec![finding.clone()]); + let receipt = super::planned_review_receipt(&envelope, "aaaaaaaaaaaa"); + let review_marker = super::review_marker(&receipt.receipt_id); + let stale_body = super::append_marker( + "**Stale finding title**\n\nThe old finding prose is obsolete.", + &super::finding_marker("stable-finding"), + ); Mock::given(method("GET")) - .and(path("/repos/owner/repo/contents/config/review.toml")) - .and(query_param("ref", "head123")) - .respond_with(ResponseTemplate::new(404)) - .expect(1) + .and(path("/repos/owner/repo")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "id": 42, + "full_name": "owner/repo" + }))) + .expect(2) .mount(&server) .await; Mock::given(method("GET")) - .and(path("/repos/owner/repo/contents/config/review.toml")) - .and(query_param("ref", "base123")) - .respond_with(ResponseTemplate::new(200).set_body_string("enabled = true\n")) - .expect(0) + .and(path("/repos/owner/repo/pulls/1/comments")) + .respond_with( + ResponseTemplate::new(200).set_body_json(serde_json::json!([{ + "id": 501, + "body": stale_body, + "commit_id": "aaaaaaaaaaaa" + }])), + ) + .expect(2) .mount(&server) .await; - - let github = test_github(&server); - let content = github - .fetch_repository_file_if_present("head123", "config/review.toml") + Mock::given(method("GET")) + .and(path("/repos/owner/repo/pulls/1/reviews")) + .respond_with( + ResponseTemplate::new(200).set_body_json(serde_json::json!([{ + "id": 77, + "body": review_marker, + "commit_id": "aaaaaaaaaaaa" + }])), + ) + .expect(2) + .mount(&server) + .await; + let snapshot = delivery_snapshot("aaaaaaaaaaaa", "bbbbbbbbbbbb", "cccccccccccc"); + let first_plan = test_github(&server) + .build_publication_plan(crate::forge::GitHubPublicationPlanRequest { + controller_generation: "4", + input_identity: PUBLICATION_INPUT_IDENTITY, + envelope: &envelope, + snapshot: &snapshot, + publication_diff: Some(&placement_diff()), + should_comment: true, + duplicate_of_baseline: false, + annotate_findings: false, + advisory: CheckState::Success, + gate: CheckState::Success, + }) + .await + .unwrap(); + let mut changed_envelope = envelope.clone(); + changed_envelope.findings[0].body = "A later same-head finding body is desired.".into(); + let changed_plan = test_github(&server) + .build_publication_plan(crate::forge::GitHubPublicationPlanRequest { + controller_generation: "4", + input_identity: PUBLICATION_INPUT_IDENTITY, + envelope: &changed_envelope, + snapshot: &snapshot, + publication_diff: Some(&placement_diff()), + should_comment: true, + duplicate_of_baseline: false, + annotate_findings: false, + advisory: CheckState::Success, + gate: CheckState::Success, + }) .await .unwrap(); - assert_eq!(content, None); + let first = serde_json::to_value(&first_plan).unwrap(); + let changed = serde_json::to_value(&changed_plan).unwrap(); + let finding_receipt = &first["lifecycleReceipt"]["findings"][0]; + assert_eq!(finding_receipt["findingId"], "stable-finding"); + assert_eq!(finding_receipt["observedCommentId"], "501"); + assert_eq!(finding_receipt["reconciliation"], "replace"); + assert_ne!( + finding_receipt["desiredBodySha256"], + finding_receipt["observedBodySha256"] + ); + let update = first["operations"] + .as_array() + .unwrap() + .iter() + .find(|operation| operation["kind"] == "findingCommentUpdate") + .unwrap(); + assert_eq!(update["findingId"], "stable-finding"); + assert_eq!(update["observedCommentId"], "501"); + assert_eq!(update["reconciliation"]["observedRemoteId"], "501"); + assert_eq!(update["expectedMarkers"].as_array().unwrap().len(), 2); + let final_summary = first["operations"] + .as_array() + .unwrap() + .iter() + .find(|operation| operation["kind"] == "reviewSummaryUpdate") + .unwrap(); + assert!( + final_summary["dependencies"] + .as_array() + .unwrap() + .contains(&update["operationKey"]) + ); + + let changed_update = changed["operations"] + .as_array() + .unwrap() + .iter() + .find(|operation| operation["kind"] == "findingCommentUpdate") + .unwrap(); + assert_eq!(first["inputIdentity"], changed["inputIdentity"]); + assert_ne!(first["reviewOutputDigest"], changed["reviewOutputDigest"]); + assert_ne!(update["operationKey"], changed_update["operationKey"]); + assert_ne!(update["desiredDigest"], changed_update["desiredDigest"]); + assert_ne!(first["intentDigest"], changed["intentDigest"]); + + let requests = server.received_requests().await.unwrap(); + assert!(requests.iter().all(|request| { + !matches!( + request.method, + wiremock::http::Method::POST + | wiremock::http::Method::PATCH + | wiremock::http::Method::PUT + | wiremock::http::Method::DELETE + ) + })); } #[tokio::test] - async fn fetch_repository_file_if_present_does_not_mask_head_failures() { + async fn publication_plan_routes_check_annotation_presentation_without_review_delivery() { let server = MockServer::start().await; + mount_current_delivery_snapshot(&server).await; Mock::given(method("GET")) - .and(path("/repos/owner/repo/contents/config/review.toml")) - .and(query_param("ref", "head123")) - .respond_with(ResponseTemplate::new(500)) - .expect(3) - .mount(&server) - .await; - Mock::given(method("GET")) - .and(path("/repos/owner/repo/contents/config/review.toml")) - .and(query_param("ref", "base123")) - .respond_with(ResponseTemplate::new(200).set_body_string("stale = true\n")) - .expect(0) + .and(path("/repos/owner/repo")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "id": 42, + "full_name": "owner/repo" + }))) + .expect(1) .mount(&server) .await; - - let error = test_github(&server) - .fetch_repository_file_if_present("head123", "config/review.toml") + let mut finding = publication_finding( + "annotation-1", + "src/lib.rs", + "The unchecked value reaches the protected operation.", + ); + finding.severity = Severity::Error; + let envelope = + delivery_envelope_with_findings("aaaaaaaaaaaa", "cccccccccccc", vec![finding]); + let snapshot = delivery_snapshot("aaaaaaaaaaaa", "bbbbbbbbbbbb", "cccccccccccc"); + let plan = test_github(&server) + .build_publication_plan(crate::forge::GitHubPublicationPlanRequest { + controller_generation: "1", + input_identity: PUBLICATION_INPUT_IDENTITY, + envelope: &envelope, + snapshot: &snapshot, + publication_diff: Some(&placement_diff()), + should_comment: true, + duplicate_of_baseline: false, + annotate_findings: true, + advisory: CheckState::Success, + gate: CheckState::Failure, + }) .await - .unwrap_err(); + .unwrap(); + let operations = serde_json::to_value(plan).unwrap()["operations"] + .as_array() + .unwrap() + .clone(); + assert_eq!(operations.len(), 2); + assert_eq!(operations[0]["kind"], "advisoryCheckCreate"); + assert_eq!(operations[1]["kind"], "advisoryCheckComplete"); + assert_eq!(operations[1]["annotations"][0]["path"], "src/lib.rs"); + assert_eq!(operations[1]["annotations"][0]["startLine"], 7); + assert_eq!( + operations[1]["dependencies"], + serde_json::json!([operations[0]["operationKey"].clone()]) + ); + assert!( + operations + .iter() + .all(|operation| operation["kind"] != "gateCheck") + ); - assert!(error.to_string().contains("500")); + let requests = server.received_requests().await.unwrap(); + assert!(requests.iter().all(|request| { + !matches!( + request.method, + wiremock::http::Method::POST + | wiremock::http::Method::PATCH + | wiremock::http::Method::PUT + | wiremock::http::Method::DELETE + ) + })); } #[tokio::test] - async fn repository_search_walks_nested_trees_and_every_blob_at_the_pinned_head() { - use crate::envelope::{ - RepositoryClaim, RepositoryClaimKind, RepositorySearchQueryKind, RepositorySearchState, - }; - + async fn github_receipt_separates_inline_summary_carried_resolved_and_suppressed() { let server = MockServer::start().await; - let head = "a".repeat(40); - let readme = b"CephCluster supports stable releases.\n"; - let generated = b"clusterVersion: 19.2.5\nimage: ceph:19.2.5\n"; - let symlink = b"../outside"; - let readme_blob = crate::repository_search::git_blob_sha1(readme); - let generated_blob = crate::repository_search::git_blob_sha1(generated); - let symlink_blob = crate::repository_search::git_blob_sha1(symlink); - let submodule = "1".repeat(40); - let manifest_tree = crate::repository_search::git_tree_sha1([( - "generated.yaml", - "100644", - generated_blob.as_str(), - )]); - let root_tree = crate::repository_search::git_tree_sha1([ - ("README.md", "100644", readme_blob.as_str()), - ("manifests", "040000", manifest_tree.as_str()), - ("outside-link", "120000", symlink_blob.as_str()), - ("vendor", "160000", submodule.as_str()), - ]); + mount_current_delivery_snapshot(&server).await; + let inline = publication_finding( + "inline-1", + "src/lib.rs", + "The expression `time() - kube_pod_start_time > 60d` measures pod age.", + ); + let synthetic = publication_finding( + "summary-1", + crate::envelope::PR_DESCRIPTION_PATH, + "The pull request description contradicts the changed code.", + ); + let carried = publication_finding( + "carried-1", + "src/old.rs", + "[carried from previous review]\n\nAn earlier issue remains.", + ); + let resolved = publication_finding("resolved-1", "src/fixed.rs", "Resolved issue."); + let suppressed = publication_finding("suppressed-1", "src/noise.rs", "Suppressed issue."); + let mut envelope = delivery_envelope_with_findings( + "aaaaaaaaaaaa", + "cccccccccccc", + vec![inline, synthetic, carried], + ); + envelope.resolved = vec![resolved]; + envelope.suppressed_findings = vec![SuppressedFinding { + finding: suppressed, + reason: SuppressionReason::BelowConfidence, + }]; - Mock::given(method("GET")) - .and(path(format!("/repos/owner/repo/git/commits/{head}"))) - .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ - "sha": head, - "tree": {"sha": root_tree} - }))) - .expect(1) - .mount(&server) - .await; - Mock::given(method("GET")) - .and(path(format!("/repos/owner/repo/git/trees/{root_tree}"))) + Mock::given(method("POST")) + .and(path("/repos/owner/repo/pulls/1/reviews")) .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ - "sha": root_tree, - "truncated": false, - "tree": [ - {"path": "README.md", "mode": "100644", "type": "blob", "sha": readme_blob, "size": readme.len()}, - {"path": "manifests", "mode": "040000", "type": "tree", "sha": manifest_tree}, - {"path": "outside-link", "mode": "120000", "type": "blob", "sha": symlink_blob, "size": symlink.len()}, - {"path": "vendor", "mode": "160000", "type": "commit", "sha": submodule} - ] + "id": 77, + "commit_id": "aaaaaaaaaaaa", + "comments": [{ + "id": 501, + "body": format!("finding\n\n{}", super::finding_marker("inline-1")) + }] }))) .expect(1) .mount(&server) .await; - Mock::given(method("GET")) - .and(path(format!( - "/repos/owner/repo/git/trees/{manifest_tree}" - ))) - .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ - "sha": manifest_tree, - "truncated": false, - "tree": [ - {"path": "generated.yaml", "mode": "100644", "type": "blob", "sha": generated_blob, "size": generated.len()} - ] - }))) + Mock::given(method("PUT")) + .and(path("/repos/owner/repo/pulls/1/reviews/77")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({}))) .expect(1) .mount(&server) .await; - for (blob, body) in [ - (readme_blob.as_str(), readme.as_slice()), - (generated_blob.as_str(), generated.as_slice()), - (symlink_blob.as_str(), symlink.as_slice()), - ] { - Mock::given(method("GET")) - .and(path(format!("/repos/owner/repo/git/blobs/{blob}"))) - .respond_with(ResponseTemplate::new(200).set_body_bytes(body)) - .expect(1) - .mount(&server) - .await; - } - let claim = RepositoryClaim { - kind: RepositoryClaimKind::Mismatch, - resources: vec!["CephCluster".into()], - values: vec!["outside-secret-term".into()], - versions: vec!["19.2.5".into()], - paths: vec!["manifests/generated.yaml".into()], - identifiers: vec!["clusterVersion".into()], - }; - let terms = crate::repository_search::search_terms(std::iter::once(&claim)).unwrap(); let receipt = test_github(&server) - .search_repository_at_head(&head, terms) - .await; + .post_review( + &envelope, + &delivery_snapshot("aaaaaaaaaaaa", "bbbbbbbbbbbb", "cccccccccccc"), + None, + ) + .await + .unwrap(); - assert_eq!(receipt.head_sha.as_deref(), Some(head.as_str())); - assert_eq!(receipt.state, RepositorySearchState::Unavailable); - assert!(receipt.tree_sha256.is_some()); - assert_eq!(receipt.searched_blobs, 3); + assert_eq!(receipt.version, 2); assert_eq!( - receipt.searched_bytes, - (readme.len() + generated.len() + symlink.len()) as u64 + receipt.channel, + super::super::ReviewPublicationChannel::ReviewComments ); - assert!(receipt.queries.iter().any(|query| { - query.kind == RepositorySearchQueryKind::Path - && receipt.matched_query_sha256.contains(&query.query_sha256) - })); - assert!(receipt.matches.iter().any(|matched| { - matched.path == "manifests/generated.yaml" && matched.occurrences == 2 - })); - let outside = receipt - .queries - .iter() - .find(|query| query.kind == RepositorySearchQueryKind::Value) - .unwrap(); - assert!(!receipt.matched_query_sha256.contains(&outside.query_sha256)); + assert!(receipt.receipt_id.starts_with("github-review-v2:")); + assert_eq!(receipt.review_id.as_deref(), Some("77")); + let outcome = |id: &str| { + receipt + .findings + .iter() + .find(|finding| finding.finding_id == id) + .unwrap() + }; + assert_eq!( + outcome("inline-1").initial_outcome, + FindingPublicationOutcome::Inline + ); + assert_eq!(outcome("inline-1").comment_id.as_deref(), Some("501")); + assert_eq!( + outcome("summary-1").initial_outcome, + FindingPublicationOutcome::SummaryOnly + ); + assert_eq!( + outcome("carried-1").initial_outcome, + FindingPublicationOutcome::Carried + ); + assert_eq!( + outcome("resolved-1").initial_outcome, + FindingPublicationOutcome::Resolved + ); + assert_eq!( + outcome("suppressed-1").initial_outcome, + FindingPublicationOutcome::Suppressed + ); + + let requests = server.received_requests().await.unwrap(); + let review: serde_json::Value = serde_json::from_slice( + &requests + .iter() + .find(|request| request.method == reqwest::Method::POST) + .unwrap() + .body, + ) + .unwrap(); + let initial_summary = review["body"].as_str().unwrap(); + assert!(!initial_summary.contains("posted inline")); + let inline_body = review["comments"][0]["body"].as_str().unwrap(); + assert!(inline_body.contains("`time() - kube_pod_start_time > 60d`")); + assert!(!inline_body.contains("> 60d`")); + let update: serde_json::Value = serde_json::from_slice( + &requests + .iter() + .find(|request| request.method == reqwest::Method::PUT) + .unwrap() + .body, + ) + .unwrap(); + let final_summary = update["body"].as_str().unwrap(); + assert!(final_summary.contains("1 finding posted inline")); + assert!(final_summary.contains("1 finding in review details")); + assert!(final_summary.contains("3 advisory findings")); + assert!(final_summary.contains("1 resolved finding")); } #[tokio::test] - async fn repository_search_rejects_same_size_blob_substitution() { - use crate::envelope::RepositorySearchState; - + async fn github_summary_update_failure_preserves_truthful_review_and_receipt() { let server = MockServer::start().await; - let head = "a".repeat(40); - let expected = b"required-construct=true\n"; - let substituted = b"required-construct=fals\n"; - assert_eq!(expected.len(), substituted.len()); - let blob = crate::repository_search::git_blob_sha1(expected); - let tree = - crate::repository_search::git_tree_sha1([("config.txt", "100644", blob.as_str())]); - - Mock::given(method("GET")) - .and(path(format!("/repos/owner/repo/git/commits/{head}"))) - .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ - "sha": head, - "tree": {"sha": tree} - }))) - .mount(&server) - .await; - Mock::given(method("GET")) - .and(path(format!("/repos/owner/repo/git/trees/{tree}"))) + mount_current_delivery_snapshot(&server).await; + let envelope = delivery_envelope_with_findings( + "aaaaaaaaaaaa", + "cccccccccccc", + vec![publication_finding( + "inline-1", + "src/lib.rs", + "A concrete issue.", + )], + ); + Mock::given(method("POST")) + .and(path("/repos/owner/repo/pulls/1/reviews")) .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ - "sha": tree, - "truncated": false, - "tree": [{ - "path": "config.txt", "mode": "100644", "type": "blob", - "sha": blob, "size": expected.len() + "id": 79, + "commit_id": "aaaaaaaaaaaa", + "comments": [{ + "id": 502, + "body": format!("finding\n\n{}", super::finding_marker("inline-1")) }] }))) + .expect(1) .mount(&server) .await; - Mock::given(method("GET")) - .and(path(format!("/repos/owner/repo/git/blobs/{blob}"))) - .respond_with(ResponseTemplate::new(200).set_body_bytes(substituted)) + Mock::given(method("PUT")) + .and(path("/repos/owner/repo/pulls/1/reviews/79")) + .respond_with(ResponseTemplate::new(422)) + .expect(1) .mount(&server) .await; let receipt = test_github(&server) - .search_repository_at_head(&head, repository_search_terms()) - .await; + .post_review( + &envelope, + &delivery_snapshot("aaaaaaaaaaaa", "bbbbbbbbbbbb", "cccccccccccc"), + None, + ) + .await + .unwrap(); - assert_eq!(receipt.state, RepositorySearchState::Unavailable); - assert_eq!(receipt.searched_blobs, 0); + assert_eq!(receipt.review_id.as_deref(), Some("79")); + assert_eq!(receipt.findings[0].comment_id.as_deref(), Some("502")); + let requests = server.received_requests().await.unwrap(); + let initial: serde_json::Value = serde_json::from_slice( + &requests + .iter() + .find(|request| request.method == reqwest::Method::POST) + .unwrap() + .body, + ) + .unwrap(); + assert!(!initial["body"].as_str().unwrap().contains("posted inline")); } #[tokio::test] - async fn repository_search_rejects_tree_entry_substitution() { - use crate::envelope::RepositorySearchState; - + async fn github_oversized_summary_falls_back_without_losing_inline_publication() { let server = MockServer::start().await; - let head = "a".repeat(40); - let expected_blob = crate::repository_search::git_blob_sha1(b"expected\n"); - let substituted_blob = crate::repository_search::git_blob_sha1(b"attacker\n"); - let tree = crate::repository_search::git_tree_sha1([( - "config.txt", - "100644", - expected_blob.as_str(), - )]); - - Mock::given(method("GET")) - .and(path(format!("/repos/owner/repo/git/commits/{head}"))) + mount_current_delivery_snapshot(&server).await; + let envelope = delivery_envelope_with_findings( + "aaaaaaaaaaaa", + "cccccccccccc", + vec![publication_finding( + "inline-1", + "src/lib.rs", + "A concrete issue.", + )], + ); + Mock::given(method("POST")) + .and(path("/repos/owner/repo/pulls/1/reviews")) .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ - "sha": head, - "tree": {"sha": tree} + "id": 80, + "commit_id": "aaaaaaaaaaaa", + "comments": [{ + "id": 503, + "body": format!("finding\n\n{}", super::finding_marker("inline-1")) + }] }))) + .expect(1) .mount(&server) .await; - Mock::given(method("GET")) - .and(path(format!("/repos/owner/repo/git/trees/{tree}"))) - .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ - "sha": tree, - "truncated": false, - "tree": [{ - "path": "config.txt", "mode": "100644", "type": "blob", - "sha": substituted_blob, "size": 9 - }] - }))) + Mock::given(method("PUT")) + .and(path("/repos/owner/repo/pulls/1/reviews/80")) + .respond_with(ResponseTemplate::new(200)) + .expect(1) .mount(&server) .await; + let mut github = test_github(&server); + github.details_url = Some(format!("https://postil.dev/{}", "x".repeat(60_000))); - let receipt = test_github(&server) - .search_repository_at_head(&head, repository_search_terms()) - .await; + let receipt = github + .post_review( + &envelope, + &delivery_snapshot("aaaaaaaaaaaa", "bbbbbbbbbbbb", "cccccccccccc"), + None, + ) + .await + .unwrap(); - assert_eq!(receipt.state, RepositorySearchState::Unavailable); - assert_eq!(receipt.searched_blobs, 0); + assert_eq!(receipt.findings[0].comment_id.as_deref(), Some("503")); + let requests = server.received_requests().await.unwrap(); + let initial: serde_json::Value = serde_json::from_slice( + &requests + .iter() + .find(|request| request.method == reqwest::Method::POST) + .unwrap() + .body, + ) + .unwrap(); + assert_eq!(initial["comments"].as_array().unwrap().len(), 1); + let body = initial["body"].as_str().unwrap(); + assert!(body.contains(super::OVERSIZED_REVIEW_MESSAGE)); + assert!(!body.contains("Review details")); + assert!(body.contains("".into(), + compatible_markers: vec![], + desired_body: "Finding body".into(), + desired_body_sha256: "sha256:body".into(), + observed_comment_id: None, + observed_body_sha256: None, + observed_outcome: None, + reconciliation: PublicationPlanFindingReconciliation::Create, + suppression_reason: None, + duplicate_provenance: PublicationPlanDuplicateProvenance::None, + }; + let lifecycle_receipt = PublicationPlanLifecycleReceipt::new( + format!("sha256:{}", "1".repeat(64)), + ReviewPublicationChannel::ReviewComments, + "github-review-v2:receipt".into(), + vec![], + None, + false, + vec![finding.clone()], + ) + .unwrap(); + let review_operation = PublicationPlanOperation::new( + 1, + "review-key".into(), + vec![], + PublicationPlanOperationActivation { + any_of: vec![PublicationPlanActivationCondition::Always], + }, + PublicationPlanOperationReconciliation { + logical_identity: "review-identity".into(), + markers: vec!["".into()], + observed_remote_id: None, + exclusive: true, + }, + PublicationPlanOperationKind::ReviewCreate { + attempt: PublicationPlanReviewAttemptKind::Initial, + logical_review_identity: "review-identity".into(), + payload: PublicationPlanReviewCreatePayload { + commit_id: head_sha.into(), + event: "COMMENT".into(), + body: review_body.into(), + comments: vec![], + }, + }, + ) + .unwrap(); + GitHubPublicationPlan::new( + GitHubPublicationPlanIdentity { + controller_generation: "1".into(), + input_identity: format!("sha256:{}", "1".repeat(64)), + review_output_digest: format!("sha256:{}", "2".repeat(64)), + repository: PublicationPlanRepository { + id: "42".into(), + full_name: "acme/api".into(), + }, + pull_request_number: "7".into(), + reviewed_snapshot: PublicationPlanSnapshot { + head_sha: head_sha.into(), + merge_base_sha: "bbbbbbbb".into(), + target_sha: "cccccccc".into(), + pull_request_title_sha256: "sha256:title".into(), + pull_request_body_sha256: "sha256:body".into(), + }, + }, + lifecycle_receipt, + vec![review_operation], + PublicationPlanGateAnalysis { + ownership: PublicationPlanGateOwnership::Service, + authoritative: false, + organization_gate_mode_required: true, + name: "postil/gate".into(), + head_sha: head_sha.into(), + analyzed_conclusion: gate_conclusion, + title: "Merge gate".into(), + summary: "Gate summary".into(), + details_url: None, + }, + ) + .unwrap() + } + + fn rebuild_publication_plan( + template: &GitHubPublicationPlan, + operations: Vec, + ) -> Result { + GitHubPublicationPlan::new( + GitHubPublicationPlanIdentity { + controller_generation: template.controller_generation.clone(), + input_identity: template.input_identity.clone(), + review_output_digest: template.review_output_digest.clone(), + repository: template.repository.clone(), + pull_request_number: template.pull_request_number.clone(), + reviewed_snapshot: template.reviewed_snapshot.clone(), + }, + template.lifecycle_receipt.clone(), + operations, + template.gate_analysis.clone(), + ) + } + + fn dependent_publication_operation( + template: &PublicationPlanOperation, + operation_key: &str, + dependency_operation_key: &str, + ) -> PublicationPlanOperation { + let mut operation = template.clone(); + operation.ordinal = 2; + operation.operation_key = operation_key.into(); + operation.dependencies = vec![dependency_operation_key.into()]; + operation.activation = PublicationPlanOperationActivation { + any_of: vec![ + PublicationPlanActivationCondition::ReviewSelectionTerminal { + selected_review_operation_keys: vec![dependency_operation_key.into()], + }, + ], + }; + operation.reconciliation.logical_identity = format!("{operation_key}-identity"); + operation + } + + fn bounded_publication_operations( + template: &PublicationPlanOperation, + operation_count: usize, + dependency_edge_count: usize, + ) -> Vec { + let mut remaining_edges = dependency_edge_count; + let mut operations = Vec::with_capacity(operation_count); + for index in 0..operation_count { + let operation_key = format!("operation-{index}"); + let edge_count = remaining_edges.min(index); + let dependencies = (0..edge_count) + .map(|dependency_index| format!("operation-{dependency_index}")) + .collect(); + remaining_edges -= edge_count; + + let mut operation = template.clone(); + operation.ordinal = u32::try_from(index + 1).unwrap(); + operation.operation_key = operation_key.clone(); + operation.dependencies = dependencies; + operation.activation = PublicationPlanOperationActivation { + any_of: vec![PublicationPlanActivationCondition::Always], + }; + operation.reconciliation.logical_identity = format!("{operation_key}-identity"); + operations.push(operation); + } + assert_eq!(remaining_edges, 0, "fixture has enough dependency capacity"); + operations + } + + #[test] + fn publication_plan_is_private_atomic_deterministic_and_intent_bound() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("publication-plan.json"); + let plan = sample_publication_plan( + "aaaaaaaa", + "Review body", + "finding-1", + PublicationPlanCheckConclusion::Failure, + ); + write_github_publication_plan(&path, &plan).unwrap(); + let first = std::fs::read(&path).unwrap(); + write_github_publication_plan(&path, &plan).unwrap(); + let second = std::fs::read(&path).unwrap(); + assert_eq!(first, second); + let mut piped = Vec::new(); + write_github_publication_plan_to_writer(&mut piped, &plan).unwrap(); + assert_eq!(piped, first); + assert_eq!(piped.last(), Some(&b'\n')); + assert_eq!( + serde_json::from_slice::(&first).unwrap(), + plan + ); + let serialized: serde_json::Value = serde_json::from_slice(&first).unwrap(); + assert_eq!(serialized["repository"]["id"], "42"); + assert_eq!(serialized["pullRequestNumber"], "7"); + assert_eq!( + serialized["inputIdentity"], + format!("sha256:{}", "1".repeat(64)) + ); + assert_eq!( + serialized["reviewOutputDigest"], + format!("sha256:{}", "2".repeat(64)) + ); + assert_eq!(serialized["operationCount"], 1); + assert_eq!(plan.operation_count, plan.operations.len() as u32); + assert_eq!( + plan.recompute_operation_manifest_digest().unwrap(), + plan.operation_manifest_digest + ); + assert!(plan.intent_digest.starts_with("sha256:")); + assert_eq!(plan.intent_digest.len(), "sha256:".len() + 64); + assert_eq!(plan.recompute_intent_digest().unwrap(), plan.intent_digest); + assert_eq!( + plan.lifecycle_receipt.recompute_digest().unwrap(), + plan.lifecycle_receipt.digest + ); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!( + std::fs::metadata(&path).unwrap().permissions().mode() & 0o777, + 0o600 + ); + } + + for changed in [ + sample_publication_plan( + "dddddddd", + "Review body", + "finding-1", + PublicationPlanCheckConclusion::Failure, + ), + sample_publication_plan( + "aaaaaaaa", + "Changed body", + "finding-1", + PublicationPlanCheckConclusion::Failure, + ), + sample_publication_plan( + "aaaaaaaa", + "Review body", + "finding-2", + PublicationPlanCheckConclusion::Failure, + ), + sample_publication_plan( + "aaaaaaaa", + "Review body", + "finding-1", + PublicationPlanCheckConclusion::Success, + ), + ] { + assert_ne!(changed.intent_digest, plan.intent_digest); + } + + let mut stored_digest_only = plan.clone(); + stored_digest_only.intent_digest = "sha256:untrusted".into(); + assert_eq!( + stored_digest_only.recompute_intent_digest().unwrap(), + plan.intent_digest, + "the stored intent digest is outside its own canonical boundary" + ); + let mut repository_changed = plan.clone(); + repository_changed.repository.id = "43".into(); + assert_ne!( + repository_changed.recompute_intent_digest().unwrap(), + plan.intent_digest + ); + let mut pull_request_changed = plan.clone(); + pull_request_changed.pull_request_number = "8".into(); + assert_ne!( + pull_request_changed.recompute_intent_digest().unwrap(), + plan.intent_digest + ); + let mut lifecycle_changed = plan.clone(); + lifecycle_changed.lifecycle_receipt.findings[0].desired_body = "Changed finding".into(); + assert_ne!( + lifecycle_changed + .lifecycle_receipt + .recompute_digest() + .unwrap(), + plan.lifecycle_receipt.digest + ); + assert_ne!( + lifecycle_changed.recompute_intent_digest().unwrap(), + plan.intent_digest + ); + let mut operation_changed = plan.clone(); + let PublicationPlanOperationKind::ReviewCreate { payload, .. } = + &mut operation_changed.operations[0].desired + else { + panic!("sample plan must contain a review create"); + }; + payload.body = "Changed operation payload".into(); + assert_ne!( + operation_changed + .recompute_operation_manifest_digest() + .unwrap(), + plan.operation_manifest_digest + ); + let mut stored_manifest_only = plan.clone(); + stored_manifest_only.operation_manifest_digest = "sha256:untrusted".into(); + assert_eq!( + stored_manifest_only + .recompute_operation_manifest_digest() + .unwrap(), + plan.operation_manifest_digest, + "the stored manifest digest is outside its own canonical boundary" + ); + assert_ne!( + stored_manifest_only.recompute_intent_digest().unwrap(), + plan.intent_digest, + "the manifest seal is inside the plan intent boundary" + ); + } + + #[test] + fn publication_plan_rejects_noncanonical_numeric_identifiers() { + for repository_id in ["", "0", "01", "9223372036854775808", "18446744073709551615"] { + assert!(ensure_publication_decimal_identifier("repository id", repository_id).is_err()); + } + for pull_request_number in ["", "0", "07", "-7"] { + assert!( + ensure_publication_decimal_identifier("pull request number", pull_request_number) + .is_err() + ); + } + assert!( + ensure_publication_decimal_identifier("repository id", "9223372036854775807").is_ok() + ); + assert!( + ensure_publication_sha256_identity( + "input identity", + &format!("sha256:{}", "a".repeat(64)), + ) + .is_ok() + ); + for identity in [ + "", + "sha256:", + &format!("sha256:{}", "a".repeat(63)), + &format!("sha256:{}", "A".repeat(64)), + &format!("sha256:{}", "g".repeat(64)), + ] { + assert!(ensure_publication_sha256_identity("input identity", identity).is_err()); + } + + let plan = sample_publication_plan( + "aaaaaaaa", + "Review body", + "finding-1", + PublicationPlanCheckConclusion::Failure, + ); + assert!( + GitHubPublicationPlan::new( + GitHubPublicationPlanIdentity { + controller_generation: "generation-1".into(), + input_identity: plan.input_identity.clone(), + review_output_digest: plan.review_output_digest.clone(), + repository: plan.repository.clone(), + pull_request_number: plan.pull_request_number.clone(), + reviewed_snapshot: plan.reviewed_snapshot.clone(), + }, + plan.lifecycle_receipt.clone(), + plan.operations.clone(), + plan.gate_analysis.clone(), + ) + .is_err() + ); + + assert!( + PublicationPlanLifecycleReceipt::new( + plan.input_identity.clone(), + plan.lifecycle_receipt.channel, + plan.lifecycle_receipt.receipt_id.clone(), + plan.lifecycle_receipt.compatible_receipt_ids.clone(), + Some("9223372036854775808".into()), + plan.lifecycle_receipt.duplicate_of_baseline, + plan.lifecycle_receipt.findings.clone(), + ) + .is_err() + ); + let mut finding_with_oversized_id = plan.lifecycle_receipt.findings[0].clone(); + finding_with_oversized_id.observed_comment_id = Some("9223372036854775808".into()); + assert!( + PublicationPlanLifecycleReceipt::new( + plan.input_identity.clone(), + plan.lifecycle_receipt.channel, + plan.lifecycle_receipt.receipt_id.clone(), + vec![], + None, + false, + vec![finding_with_oversized_id], + ) + .is_err() + ); + } + + #[test] + fn publication_plan_constructor_accepts_only_a_sealed_topological_operation_graph() { + let template = sample_publication_plan( + "aaaaaaaa", + "Review body", + "finding-1", + PublicationPlanCheckConclusion::Success, + ); + let first = template.operations[0].clone(); + let second = dependent_publication_operation(&first, "summary-key", "review-key"); + let valid = rebuild_publication_plan(&template, vec![first.clone(), second.clone()]) + .expect("a contiguous topological operation graph is valid"); + assert_eq!(valid.operation_count, 2); + assert_eq!( + valid.recompute_operation_manifest_digest().unwrap(), + valid.operation_manifest_digest + ); + assert_eq!( + valid.recompute_intent_digest().unwrap(), + valid.intent_digest + ); + + let mut duplicate = second.clone(); + duplicate.operation_key = first.operation_key.clone(); + assert!( + rebuild_publication_plan(&template, vec![first.clone(), duplicate]) + .unwrap_err() + .to_string() + .contains("keys must be unique") + ); + + let mut missing = second.clone(); + missing.dependencies = vec!["missing-key".into()]; + missing.activation = PublicationPlanOperationActivation { + any_of: vec![PublicationPlanActivationCondition::Always], + }; + assert!( + rebuild_publication_plan(&template, vec![first.clone(), missing]) + .unwrap_err() + .to_string() + .contains("depends on missing operation") + ); + + let mut forward_first = first.clone(); + forward_first.dependencies = vec![second.operation_key.clone()]; + assert!( + rebuild_publication_plan(&template, vec![forward_first, second.clone()]) + .unwrap_err() + .to_string() + .contains("forward, self, or cyclic dependency") + ); + + let mut cyclic_first = first.clone(); + cyclic_first.dependencies = vec![second.operation_key.clone()]; + let mut cyclic_second = second.clone(); + cyclic_second.dependencies = vec![first.operation_key.clone()]; + assert!( + rebuild_publication_plan(&template, vec![cyclic_first, cyclic_second]) + .unwrap_err() + .to_string() + .contains("forward, self, or cyclic dependency") + ); + + let mut self_dependent = first.clone(); + self_dependent.dependencies = vec![first.operation_key.clone()]; + assert!( + rebuild_publication_plan(&template, vec![self_dependent]) + .unwrap_err() + .to_string() + .contains("forward, self, or cyclic dependency") + ); + + let mut undeclared_activation = second.clone(); + undeclared_activation.dependencies.clear(); + assert!( + rebuild_publication_plan(&template, vec![first.clone(), undeclared_activation]) + .unwrap_err() + .to_string() + .contains("references undeclared dependency") + ); + + let mut empty_activation = first.clone(); + empty_activation.activation.any_of.clear(); + assert!( + rebuild_publication_plan(&template, vec![empty_activation]) + .unwrap_err() + .to_string() + .contains("require at least one activation condition") + ); + + let mut stale_desired_digest = first.clone(); + let PublicationPlanOperationKind::ReviewCreate { payload, .. } = + &mut stale_desired_digest.desired + else { + panic!("sample plan must contain a review create"); + }; + payload.body = "Changed without resealing".into(); + assert!( + rebuild_publication_plan(&template, vec![stale_desired_digest]) + .unwrap_err() + .to_string() + .contains("desired digest does not match") + ); + + let advisory_completion = PublicationPlanOperation::new( + 2, + "advisory-complete".into(), + vec![], + PublicationPlanOperationActivation { + any_of: vec![PublicationPlanActivationCondition::Always], + }, + PublicationPlanOperationReconciliation { + logical_identity: "advisory-complete".into(), + markers: vec![], + observed_remote_id: None, + exclusive: true, + }, + PublicationPlanOperationKind::AdvisoryCheckComplete { + name: "postil/review".into(), + head_sha: "aaaaaaaa".into(), + created_check: PublicationPlanOperationResultReference { + dependency_operation_key: first.operation_key.clone(), + result_field: PublicationPlanOperationResultField::RemoteId, + }, + conclusion: PublicationPlanCheckConclusion::Success, + title: "Review".into(), + summary: "Review complete.".into(), + annotations: vec![], + details_url: None, + }, + ) + .unwrap(); + assert!( + rebuild_publication_plan(&template, vec![first.clone(), advisory_completion]) + .unwrap_err() + .to_string() + .contains("undeclared create dependency") + ); + + let mut ordinal_zero = first.clone(); + ordinal_zero.ordinal = 0; + assert!( + rebuild_publication_plan(&template, vec![ordinal_zero]) + .unwrap_err() + .to_string() + .contains("contiguous and one-based") + ); + + let mut ordinal_gap = second; + ordinal_gap.ordinal = 3; + assert!( + rebuild_publication_plan(&template, vec![first, ordinal_gap]) + .unwrap_err() + .to_string() + .contains("contiguous and one-based") + ); + } + + #[test] + fn publication_plan_enforces_operation_and_dependency_edge_bounds() { + let template = sample_publication_plan( + "aaaaaaaa", + "Review body", + "finding-1", + PublicationPlanCheckConclusion::Success, + ); + let operation_template = &template.operations[0]; + + let maximum_operations = bounded_publication_operations( + operation_template, + GitHubPublicationPlan::MAX_OPERATIONS, + 0, + ); + assert_eq!( + rebuild_publication_plan(&template, maximum_operations) + .unwrap() + .operation_count, + 126 + ); + let too_many_operations = bounded_publication_operations( + operation_template, + GitHubPublicationPlan::MAX_OPERATIONS + 1, + 0, + ); + assert!( + rebuild_publication_plan(&template, too_many_operations) + .unwrap_err() + .to_string() + .contains("operation count must not exceed 126") + ); + + let maximum_edges = bounded_publication_operations( + operation_template, + GitHubPublicationPlan::MAX_OPERATIONS, + GitHubPublicationPlan::MAX_DEPENDENCY_EDGES, + ); + assert!(rebuild_publication_plan(&template, maximum_edges).is_ok()); + let too_many_edges = bounded_publication_operations( + operation_template, + GitHubPublicationPlan::MAX_OPERATIONS, + GitHubPublicationPlan::MAX_DEPENDENCY_EDGES + 1, + ); + assert!( + rebuild_publication_plan(&template, too_many_edges) + .unwrap_err() + .to_string() + .contains("dependency edge count must not exceed 1024") + ); + } + + #[test] + fn publication_plan_check_wire_values_are_closed_enums() { + assert_eq!( + serde_json::to_value(PublicationPlanCheckStatus::InProgress).unwrap(), + serde_json::json!("in_progress") + ); + assert_eq!( + serde_json::to_value(PublicationPlanCheckConclusion::Success).unwrap(), + serde_json::json!("success") + ); + assert_eq!( + serde_json::to_value(PublicationPlanCheckConclusion::Failure).unwrap(), + serde_json::json!("failure") + ); + assert_eq!( + serde_json::to_value(PublicationPlanCheckConclusion::Neutral).unwrap(), + serde_json::json!("neutral") + ); + assert!(serde_json::from_str::("\"queued\"").is_err()); + assert!(serde_json::from_str::("\"cancelled\"").is_err()); + } + + #[test] + fn publication_operation_manifest_seals_complete_ordered_records() { + let plan = sample_publication_plan( + "aaaaaaaa", + "Review body", + "finding-1", + PublicationPlanCheckConclusion::Failure, + ); + let baseline = plan.operation_manifest_digest.clone(); + let assert_changed = |changed: &GitHubPublicationPlan| { + assert_ne!( + changed.recompute_operation_manifest_digest().unwrap(), + baseline + ); + }; + + let mut changed = plan.clone(); + changed.operations[0].ordinal = 2; + assert_changed(&changed); + + let mut changed = plan.clone(); + changed.operations[0].operation_key.push_str("-changed"); + assert_changed(&changed); + + let mut changed = plan.clone(); + changed.operations[0] + .dependencies + .push("dependency-key".into()); + assert_changed(&changed); + + let mut changed = plan.clone(); + changed.operations[0].activation.any_of.clear(); + assert_changed(&changed); + + let mut changed = plan.clone(); + changed.operations[0] + .reconciliation + .logical_identity + .push_str("-changed"); + assert_changed(&changed); + + let mut changed = plan.clone(); + changed.operations[0].desired_digest = "sha256:changed".into(); + assert_changed(&changed); + + let mut changed = plan.clone(); + let PublicationPlanOperationKind::ReviewCreate { + attempt, + logical_review_identity, + .. + } = &mut changed.operations[0].desired + else { + panic!("sample plan must contain a review create"); + }; + *attempt = PublicationPlanReviewAttemptKind::SummaryOnly; + logical_review_identity.push_str("-changed"); + assert_changed(&changed); + } + + #[cfg(unix)] + #[test] + fn publication_plan_refuses_symlink_and_directory_targets() { + use std::os::unix::fs::symlink; + + let directory = tempfile::tempdir().unwrap(); + let plan = sample_publication_plan( + "aaaaaaaa", + "Review body", + "finding-1", + PublicationPlanCheckConclusion::Failure, + ); + let destination = directory.path().join("destination.json"); + std::fs::write(&destination, "unchanged").unwrap(); + let link = directory.path().join("publication-plan.json"); + symlink(&destination, &link).unwrap(); + let error = write_github_publication_plan(&link, &plan).unwrap_err(); + assert!(error.to_string().contains("must not be a symlink")); + assert_eq!(std::fs::read_to_string(destination).unwrap(), "unchanged"); + + let target_directory = directory.path().join("target-directory"); + std::fs::create_dir(&target_directory).unwrap(); + assert!(write_github_publication_plan(&target_directory, &plan).is_err()); + } + #[tokio::test] async fn source_snapshot_streams_above_page_limit_with_complete_length() { let server = MockServer::start().await; @@ -1733,6 +3314,99 @@ mod tests { assert!(!without_details.contains("")); } + #[test] + fn publication_summary_explains_unplaced_findings_without_duplicate_counts() { + let env = envelope_with_findings(vec![finding(), finding()]); + let unplaced_only = check_summary( + &env, + true, + SummaryContext { + details_url: Some("https://postil.dev/orgs/acme/runs/run-1".into()), + publication: Some(ReviewPublicationSummary { + summary_only: 2, + rejected_inline: 2, + ..Default::default() + }), + ..Default::default() + }, + ); + assert!( + unplaced_only.contains( + "2 findings could not be placed on the changed lines; see review details" + ) + ); + assert!(!unplaced_only.contains("inline placement unavailable")); + assert!(!unplaced_only.contains("2 findings in review details")); + + let unplaced_without_details_link = check_summary( + &env, + true, + SummaryContext { + publication: Some(ReviewPublicationSummary { + summary_only: 2, + rejected_inline: 2, + ..Default::default() + }), + ..Default::default() + }, + ); + assert!( + unplaced_without_details_link + .contains("2 findings could not be placed on the changed lines") + ); + assert!(!unplaced_without_details_link.contains("see review details")); + + let mixed = check_summary( + &env, + true, + SummaryContext { + details_url: Some("https://postil.dev/orgs/acme/runs/run-1".into()), + publication: Some(ReviewPublicationSummary { + active_inline: 1, + summary_only: 2, + rejected_inline: 1, + ..Default::default() + }), + ..Default::default() + }, + ); + assert!(mixed.contains( + "2 findings in review details, including 1 that could not be placed on the changed lines" + )); + assert!(mixed.contains("1 finding posted inline")); + assert!(!mixed.contains("inline placement unavailable")); + + let mixed_without_details_link = check_summary( + &env, + true, + SummaryContext { + publication: Some(ReviewPublicationSummary { + summary_only: 2, + rejected_inline: 1, + ..Default::default() + }), + ..Default::default() + }, + ); + assert!(mixed_without_details_link.contains( + "2 findings were not posted inline, including 1 that could not be placed on the changed lines" + )); + assert!(!mixed_without_details_link.contains("review details")); + + let file_level = check_summary( + &env, + true, + SummaryContext { + publication: Some(ReviewPublicationSummary { + file_comments: 2, + ..Default::default() + }), + ..Default::default() + }, + ); + assert!(file_level.contains("2 findings posted as file-level review comments")); + } + #[test] fn review_coverage_stays_out_of_compact_pull_request_summaries() { let mut env = envelope_with_findings(vec![finding()]); diff --git a/src/main.rs b/src/main.rs index f17db4e..9354f36 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,7 +2,9 @@ use clap::Parser; #[cfg(feature = "qualification-candidate")] use postil_cli::attribution; -use postil_cli::cli::{Cli, Command, ForgeArg, HookAction, publication_enabled}; +use postil_cli::cli::{ + Cli, Command, ForgeArg, HookAction, publication_enabled, publication_plan_contract_capability, +}; use postil_cli::config::{Config, qualification_metadata, starter_config}; use postil_cli::review::{ForgeKind, ReviewArgs}; use postil_cli::{doctor, hook, login, plan, respond, review}; @@ -22,6 +24,15 @@ async fn main() { async fn dispatch(cli: Cli) -> anyhow::Result { match cli.command { + Command::Capabilities { + publication_plan_contract, + } => { + println!( + "{}", + publication_plan_contract_capability(&publication_plan_contract)? + ); + Ok(0) + } Command::QualificationMetadata => { println!("{}", serde_json::to_string(&qualification_metadata())?); Ok(0) @@ -54,6 +65,9 @@ async fn dispatch(cli: Cli) -> anyhow::Result { publish, no_post, defer_gate_check, + publication_plan_output, + publication_generation, + publication_input_identity, } => { let local_mode = staged || base.is_some() || diff_file.is_some(); let kind = match forge { @@ -89,6 +103,9 @@ async fn dispatch(cli: Cli) -> anyhow::Result { bounded, no_post: !publication_enabled(publish, no_post)?, defer_gate_check, + publication_plan_output, + publication_generation, + publication_input_identity, }) .await } diff --git a/src/review.rs b/src/review.rs index c6ce31f..bc07d7c 100644 --- a/src/review.rs +++ b/src/review.rs @@ -391,6 +391,9 @@ pub struct ReviewArgs { pub bounded: bool, pub no_post: bool, pub defer_gate_check: bool, + pub publication_plan_output: Option, + pub publication_generation: Option, + pub publication_input_identity: Option, } impl ReviewArgs { @@ -422,6 +425,19 @@ struct RemoteReviewInput<'a> { repository_source: RepositorySource<'a>, } +struct RemoteReviewResult { + envelope: Envelope, + /// Complete pull-request diff retained for forge placement fallback. An + /// incremental review cannot establish the complete PR file surface. + publication_diff: Option, +} + +#[derive(Clone, Copy)] +struct PublicationContext<'a> { + snapshot: &'a PrMeta, + diff: Option<&'a diff::Diff>, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum ReviewFailureKind { Provider, @@ -474,6 +490,47 @@ pub async fn run(args: ReviewArgs) -> Result { if args.output_file.is_some() && args.resolved_output_format().is_none() { return Err(anyhow!("--output-file requires --output or --output-json")); } + if args.publication_plan_output.is_some() { + anyhow::ensure!( + args.forge == ForgeKind::GitHub + && args.repo.is_some() + && args.pr.is_some() + && args.sha.is_some() + && args.base_sha.is_some() + && !args.staged + && args.base.is_none() + && args.diff_file.is_none(), + "--publication-plan-output requires a remote GitHub pull request with explicit --repo, --pr, --sha, and --base-sha" + ); + anyhow::ensure!( + args.no_post + && args.check_run_id.is_none() + && args.gate_check_run_id.is_none() + && !args.defer_gate_check, + "--publication-plan-output cannot be combined with forge mutation options" + ); + let generation = args + .publication_generation + .as_deref() + .context("--publication-plan-output requires --publication-generation")?; + crate::forge::ensure_publication_decimal_identifier("controller generation", generation) + .context("invalid --publication-generation")?; + let input_identity = args + .publication_input_identity + .as_deref() + .context("--publication-plan-output requires --publication-input-identity")?; + crate::forge::ensure_publication_sha256_identity("input identity", input_identity) + .context("invalid --publication-input-identity")?; + } else { + anyhow::ensure!( + args.publication_generation.is_none(), + "--publication-generation requires --publication-plan-output" + ); + anyhow::ensure!( + args.publication_input_identity.is_none(), + "--publication-input-identity requires --publication-plan-output" + ); + } let mut cfg = Config::load(&cwd, args.config.as_deref())?; if let Some(m) = &args.model { cfg.model = m.clone(); @@ -678,25 +735,13 @@ async fn run_remote( ) .await; match result { - Ok(envelope) => { + Ok(review) => { + let RemoteReviewResult { + envelope, + publication_diff, + } = review; + let (review_state, gate_state) = remote_check_states(&envelope); let check_completion = if let Some((a, g)) = &checks { - let gate_state = if envelope.gate.failing { - CheckState::Failure - } else { - CheckState::Success - }; - // `postil/review` reports whether a review verdict exists. It - // fails when inference is operationally incomplete, independent - // of whether `postil/gate` is configured to stand aside. - let operational = envelope.findings.iter().any(|f| { - f.path == crate::envelope::OPERATIONAL_PATH - || f.path == crate::envelope::PROVIDER_PATH - }); - let review_state = if operational { - CheckState::Failure - } else { - CheckState::Success - }; complete_remote_checks( forge, a, @@ -723,7 +768,10 @@ async fn run_remote( envelope, Some(forge), Some(review_started), - Some(&meta), + Some(PublicationContext { + snapshot: &meta, + diff: publication_diff.as_ref(), + }), strict_publication, ) .await; @@ -748,17 +796,13 @@ async fn run_remote( Some(&meta), review_started.elapsed().as_millis() as u64, ); + let (review_state, gate_state) = remote_check_states(&envelope); let check_completion = if let Some((a, g)) = &checks { - let gate_state = if envelope.gate.failing { - CheckState::Failure - } else { - CheckState::Success - }; complete_remote_checks( forge, a, g, - CheckState::Failure, + review_state, (!args.defer_gate_check).then_some(gate_state), &envelope, &meta, @@ -784,7 +828,10 @@ async fn run_remote( envelope, Some(forge), Some(review_started), - Some(&meta), + Some(PublicationContext { + snapshot: &meta, + diff: None, + }), strict_publication, ) .await; @@ -811,6 +858,24 @@ fn strict_hosted_github_publication(args: &ReviewArgs) -> bool { crate::config::hosted_mode() && args.forge == ForgeKind::GitHub && !args.no_post } +fn remote_check_states(envelope: &Envelope) -> (CheckState, CheckState) { + let operational = envelope.findings.iter().any(|finding| { + finding.path == crate::envelope::OPERATIONAL_PATH + || finding.path == crate::envelope::PROVIDER_PATH + }); + let advisory = if operational { + CheckState::Failure + } else { + CheckState::Success + }; + let gate = if envelope.gate.failing { + CheckState::Failure + } else { + CheckState::Success + }; + (advisory, gate) +} + async fn require_current_snapshot( forge: &F, expected: &PrMeta, @@ -897,7 +962,7 @@ async fn remote_review( forge: &F, repo: &str, input: RemoteReviewInput<'_>, -) -> Result { +) -> Result { let RemoteReviewInput { meta, review_started, @@ -976,7 +1041,9 @@ async fn remote_review( } else { (diff_snapshot, scope, force_model) }; - review_diff( + let publication_diff = matches!(scope, filter::ReconcileScope::Full { .. }) + .then(|| diff::parse(diff_snapshot.as_str())); + let envelope = review_diff( cfg, args, ReviewInput { @@ -992,7 +1059,11 @@ async fn remote_review( repository_source, }, ) - .await + .await?; + Ok(RemoteReviewResult { + envelope, + publication_diff, + }) } fn load_baseline(args: &ReviewArgs) -> Result> { @@ -2323,6 +2394,10 @@ async fn review_diff_at( preserve_unadjudicated_findings(all_adjudication_candidates) } }; + suppress_fresh_unresolved_repository_claims( + &mut application, + fresh_candidate_count, + ); for (candidate_index, finding) in application .kept_indices .iter() @@ -2735,9 +2810,11 @@ async fn finish( envelope: Envelope, forge: Option<&F>, hosted_budget_started_at: Option, - expected_snapshot: Option<&PrMeta>, + publication: Option>, strict_publication: bool, ) -> Result { + let publication_plan_to_stdout = + args.publication_plan_output.as_deref() == Some(Path::new("-")); // Persist artifacts before any forge I/O: a posting hiccup must not // discard the completed review's SARIF or envelope output. if let Some(path) = &args.sarif { @@ -2746,18 +2823,70 @@ async fn finish( .with_context(|| format!("writing SARIF to {}", path.display()))?; } - if let Some(format) = args.resolved_output_format() { + if let Some(format) = args.resolved_output_format() + && (args.output_file.is_some() || !publication_plan_to_stdout) + { output::write_envelope(&envelope, format, args.output_file.as_deref())?; } - if args.resolved_output_format().is_none() || args.output_file.is_some() { + if !publication_plan_to_stdout + && (args.resolved_output_format().is_none() || args.output_file.is_some()) + { output::print_pretty(&envelope); } + let duplicate_of_baseline = load_baseline(args) + .ok() + .is_some_and(|baseline| visible_finding_sets_equal(&baseline, &envelope.findings)); + let intentional_no_comment = crate::forge::only_operational_findings(&envelope.findings) + || (!envelope.findings.is_empty() && envelope.findings.iter().all(filter::is_carried)); + let should_comment = (!envelope.silent + || matches!(cfg.on_clean, crate::config::OnClean::Comment)) + && !duplicate_of_baseline + && !intentional_no_comment; + + if let Some(path) = &args.publication_plan_output { + let forge = forge.context("publication planning requires a GitHub forge")?; + let PublicationContext { + snapshot: expected_snapshot, + diff: publication_diff, + } = publication.context("publication planning is missing its immutable PR snapshot")?; + let (advisory, gate) = remote_check_states(&envelope); + let plan = forge + .build_publication_plan(crate::forge::GitHubPublicationPlanRequest { + controller_generation: args + .publication_generation + .as_deref() + .context("publication planning is missing its generation identity")?, + input_identity: args + .publication_input_identity + .as_deref() + .context("publication planning is missing its input identity")?, + envelope: &envelope, + snapshot: expected_snapshot, + publication_diff, + should_comment, + duplicate_of_baseline, + annotate_findings: cfg.finding_presentation + == FindingPresentation::CheckAnnotations, + advisory, + gate, + }) + .await?; + if publication_plan_to_stdout { + crate::forge::write_github_publication_plan_to_writer(std::io::stdout().lock(), &plan)?; + } else { + crate::forge::write_github_publication_plan(path, &plan)?; + } + return Ok(if envelope.gate.failing { 1 } else { 0 }); + } + if let Some(forge) = forge && !args.no_post { - let expected_snapshot = - expected_snapshot.context("remote publication is missing its immutable PR snapshot")?; + let PublicationContext { + snapshot: expected_snapshot, + diff: publication_diff, + } = publication.context("remote publication is missing its immutable PR snapshot")?; if cfg.finding_presentation == FindingPresentation::CheckAnnotations { let mut receipt = forge.plan_review_publication(&envelope, expected_snapshot); receipt.channel = crate::forge::ReviewPublicationChannel::CheckAnnotations; @@ -2772,15 +2901,6 @@ async fn finish( crate::forge::write_review_publication_receipt_from_env(&receipt)?; return Ok(if envelope.gate.failing { 1 } else { 0 }); } - let duplicate_of_baseline = load_baseline(args) - .ok() - .is_some_and(|baseline| visible_finding_sets_equal(&baseline, &envelope.findings)); - let intentional_no_comment = crate::forge::only_operational_findings(&envelope.findings) - || (!envelope.findings.is_empty() && envelope.findings.iter().all(filter::is_carried)); - let should_comment = (!envelope.silent - || matches!(cfg.on_clean, crate::config::OnClean::Comment)) - && !duplicate_of_baseline - && !intentional_no_comment; if !should_comment { let mut receipt = forge.plan_review_publication(&envelope, expected_snapshot); if duplicate_of_baseline { @@ -2788,6 +2908,7 @@ async fn finish( if matches!( finding.initial_outcome, crate::forge::FindingPublicationOutcome::Inline + | crate::forge::FindingPublicationOutcome::FileComment | crate::forge::FindingPublicationOutcome::SummaryOnly ) { finding.initial_outcome = crate::forge::FindingPublicationOutcome::Carried; @@ -2832,7 +2953,7 @@ async fn finish( let posted = run_with_hosted_budget( hosted_budget_started_at, REVIEW_POST_TIMEOUT_SECS, - forge.post_review(&envelope, expected_snapshot), + forge.post_review(&envelope, expected_snapshot, publication_diff), "posting review comment", ) .await; @@ -3004,11 +3125,43 @@ fn preserve_unadjudicated_findings( crate::adjudication::AdjudicationApplication { kept_indices: (0..findings.len()).collect(), kept: findings, + unresolved_indices: Vec::new(), resolved_indices: Vec::new(), suppressed: Vec::new(), } } +fn suppress_fresh_unresolved_repository_claims( + application: &mut crate::adjudication::AdjudicationApplication, + fresh_candidate_count: usize, +) { + let mut kept = Vec::with_capacity(application.kept.len()); + let mut kept_indices = Vec::with_capacity(application.kept_indices.len()); + for (candidate_index, finding) in application + .kept_indices + .drain(..) + .zip(application.kept.drain(..)) + { + let unsupported = candidate_index < fresh_candidate_count + && application.unresolved_indices.contains(&candidate_index) + && finding.repository_claim.is_some(); + if unsupported { + application.suppressed.push(SuppressedFinding { + finding, + reason: SuppressionReason::RepositoryClaimUnsupported, + }); + } else { + kept_indices.push(candidate_index); + kept.push(finding); + } + } + application.kept = kept; + application.kept_indices = kept_indices; + application + .unresolved_indices + .retain(|index| application.kept_indices.contains(index)); +} + fn error_envelope( cfg: &Config, err: &anyhow::Error, @@ -3747,6 +3900,57 @@ mod tests { ])); } + #[test] + fn only_fresh_unresolved_repository_claims_are_suppressed() { + let claim = RepositoryClaim { + kind: RepositoryClaimKind::Absence, + resources: vec!["widget".into()], + values: vec![], + versions: vec![], + paths: vec![], + identifiers: vec![], + }; + let mut fresh_repository_claim = finding("src/fresh.rs", 1, "widget is absent"); + fresh_repository_claim.repository_claim = Some(claim.clone()); + let fresh_local_finding = finding("src/local.rs", 2, "authorization is bypassed"); + let mut baseline_repository_claim = finding("src/baseline.rs", 3, "widget is absent"); + baseline_repository_claim.repository_claim = Some(claim); + let mut application = crate::adjudication::AdjudicationApplication { + kept: vec![ + fresh_repository_claim.clone(), + fresh_local_finding.clone(), + baseline_repository_claim.clone(), + ], + kept_indices: vec![0, 1, 2], + unresolved_indices: vec![0, 1, 2], + resolved_indices: vec![], + suppressed: vec![], + }; + + suppress_fresh_unresolved_repository_claims(&mut application, 2); + + assert_eq!(application.kept_indices, vec![1, 2]); + assert_eq!(application.unresolved_indices, vec![1, 2]); + assert_eq!(application.kept.len(), 2); + assert!(same_visible_finding( + &application.kept[0], + &fresh_local_finding + )); + assert!(same_visible_finding( + &application.kept[1], + &baseline_repository_claim + )); + assert_eq!(application.suppressed.len(), 1); + assert!(same_visible_finding( + &application.suppressed[0].finding, + &fresh_repository_claim + )); + assert_eq!( + application.suppressed[0].reason, + SuppressionReason::RepositoryClaimUnsupported + ); + } + #[test] fn adjudication_preserved_baseline_survives_a_deterministic_refutation_receipt() { let snapshot_id = "a".repeat(40); diff --git a/tests/e2e.rs b/tests/e2e.rs index b1cbc9b..ccb6388 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -11,6 +11,9 @@ use serde_json::{Value, json}; use wiremock::matchers::{body_string_contains, header, method, path, path_regex, query_param}; use wiremock::{Mock, MockServer as WireMockServer, Request, Respond, ResponseTemplate}; +const PUBLICATION_INPUT_IDENTITY: &str = + "sha256:1111111111111111111111111111111111111111111111111111111111111111"; + struct MockServer(WireMockServer); impl std::ops::Deref for MockServer { @@ -121,6 +124,43 @@ impl Respond for DefaultAdjudicator { } } +#[derive(Clone, Copy)] +struct RefuteFromReceiptAdjudicator; + +impl Respond for RefuteFromReceiptAdjudicator { + fn respond(&self, request: &Request) -> ResponseTemplate { + let request_body: Value = request.body_json().unwrap(); + let payload: Value = serde_json::from_str( + request_body["messages"] + .as_array() + .and_then(|messages| messages.last()) + .and_then(|message| message["content"].as_str()) + .unwrap(), + ) + .unwrap(); + let candidate = payload["candidates"].as_array().unwrap().first().unwrap(); + let candidate_id = candidate["candidateId"].as_str().unwrap(); + let evidence = payload["diffCorpusReceipt"]["candidateCitations"] + .as_array() + .unwrap() + .iter() + .find(|receipt| receipt["candidateId"] == candidate_id) + .and_then(|receipt| receipt["refutationEvidence"]["source"].as_str()) + .expect("repository claim fixture must expose exact replacement evidence"); + ResponseTemplate::new(200).set_body_json(scorer_text( + &json!([{ + "candidateId": candidate_id, + "status": "refuted", + "revisedTitle": "", + "revisedBody": "", + "evidence": evidence, + "duplicateOf": null + }]) + .to_string(), + )) + } +} + #[derive(Clone, Copy)] struct AllUnresolvedAdjudicator; @@ -640,7 +680,14 @@ struct ReconciledReviewListResponder { } #[derive(Clone)] -struct PublishedReviewResponder; +struct PublishedReviewResponder { + comments: Arc>>, +} + +#[derive(Clone)] +struct PublishedReviewCommentsResponder { + comments: Arc>>, +} struct OutputBudgetResponder; @@ -848,17 +895,34 @@ impl Respond for PublishedReviewResponder { json!({ "id": 500 + index, "body": comment["body"], + "commit_id": request_body["commit_id"], }) }) .collect::>(); + *self.comments.lock().unwrap() = comments; ResponseTemplate::new(200).set_body_json(json!({ "id": 77, "commit_id": request_body["commit_id"], - "comments": comments, })) } } +impl Respond for PublishedReviewCommentsResponder { + fn respond(&self, _request: &Request) -> ResponseTemplate { + ResponseTemplate::new(200).set_body_json(self.comments.lock().unwrap().clone()) + } +} + +fn published_review_responders() -> (PublishedReviewResponder, PublishedReviewCommentsResponder) { + let comments = Arc::new(Mutex::new(Vec::new())); + ( + PublishedReviewResponder { + comments: Arc::clone(&comments), + }, + PublishedReviewCommentsResponder { comments }, + ) +} + async fn mount_github_complete_diff(server: &MockServer, pr: u64) { Mock::given(method("GET")) .and(path_regex(r"^/repos/acme/api/compare/b+\.\.\.a+$")) @@ -1172,6 +1236,349 @@ async fn remote_review_without_publish_never_mutates_github_even_in_hosted_ci() ); } +#[tokio::test] +async fn github_publication_plan_is_byte_stable_private_and_never_mutates_the_forge() { + let server = MockServer::start().await; + mount_github_complete_diff(&server, 7).await; + mount_static_github_pr(&server).await; + Mock::given(method("GET")) + .and(path("/repos/acme/api/pulls/7/comments")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!([]))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/repos/acme/api/pulls/7/reviews")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!([]))) + .mount(&server) + .await; + let directory = tempfile::tempdir().unwrap(); + disable_review_for_hosted_publication(directory.path(), true); + let first_plan_name = "publication-plan-1.json"; + let first_plan = directory.path().join(first_plan_name); + let second_plan = directory.path().join("publication-plan-2.json"); + let second_envelope = directory.path().join("publication-plan-2.envelope.json"); + + for (plan_path, envelope_path) in [ + ( + std::path::Path::new(first_plan_name), + std::path::Path::new("publication-plan-1.envelope.json"), + ), + (second_plan.as_path(), second_envelope.as_path()), + ] { + postil() + .current_dir(directory.path()) + .env("GITHUB_API_URL", server.uri()) + .env("GITHUB_TOKEN", "gh-test-token") + .env("POSTIL_EXPECTED_GITHUB_REPO_ID", "42") + .args([ + "review", + "--repo", + "acme/api", + "--pr", + "7", + "--sha", + "aaaaaaaa", + "--base-sha", + "bbbbbbbb", + "--publication-plan-output", + ]) + .arg(plan_path) + .args(["--publication-generation", "1"]) + .args(["--publication-input-identity", PUBLICATION_INPUT_IDENTITY]) + .args(["--output", "json", "--output-file"]) + .arg(envelope_path) + .assert() + .code(0); + } + + let first = std::fs::read(&first_plan).unwrap(); + let second = std::fs::read(&second_plan).unwrap(); + assert_eq!(first, second); + let piped = postil() + .current_dir(directory.path()) + .env("GITHUB_API_URL", server.uri()) + .env("GITHUB_TOKEN", "gh-test-token") + .env("POSTIL_EXPECTED_GITHUB_REPO_ID", "42") + .args([ + "review", + "--repo", + "acme/api", + "--pr", + "7", + "--sha", + "aaaaaaaa", + "--base-sha", + "bbbbbbbb", + "--publication-plan-output", + "-", + "--publication-generation", + "1", + "--publication-input-identity", + PUBLICATION_INPUT_IDENTITY, + "--output", + "json", + ]) + .assert() + .code(0) + .get_output() + .stdout + .clone(); + assert_eq!(piped, first); + assert_eq!(piped.last(), Some(&b'\n')); + assert!(!directory.path().join("-").exists()); + for envelope_path in [ + directory.path().join("publication-plan-1.envelope.json"), + second_envelope, + ] { + assert!( + !std::fs::read_to_string(envelope_path) + .unwrap() + .contains(PUBLICATION_INPUT_IDENTITY), + "the service-supplied input identity must remain private to the plan artifact" + ); + } + assert!(!std::fs::read_dir(directory.path()).unwrap().any(|entry| { + let name = entry.unwrap().file_name(); + let name = name.to_string_lossy(); + name.starts_with(".publication-plan-1.json.") && name.ends_with(".tmp") + })); + let plan: Value = serde_json::from_slice(&first).unwrap(); + assert_eq!(plan["version"], 1); + assert_eq!(plan["forge"], "github"); + assert_eq!(plan["controllerGeneration"], "1"); + assert_eq!(plan["inputIdentity"], PUBLICATION_INPUT_IDENTITY); + assert_eq!( + plan["lifecycleReceipt"]["inputIdentity"], + PUBLICATION_INPUT_IDENTITY + ); + assert!( + plan["reviewOutputDigest"] + .as_str() + .is_some_and(|digest| digest.starts_with("sha256:") && digest.len() == 71) + ); + assert_eq!(plan["repository"]["id"], "42"); + assert_eq!(plan["repository"]["fullName"], "acme/api"); + assert_eq!(plan["pullRequestNumber"], "7"); + assert_eq!(plan["reviewedSnapshot"]["headSha"], "aaaaaaaa"); + assert_eq!(plan["reviewedSnapshot"]["mergeBaseSha"], "bbbbbbbb"); + assert_eq!(plan["reviewedSnapshot"]["targetSha"], "bbbbbbbb"); + assert!( + plan["intentDigest"] + .as_str() + .is_some_and(|digest| digest.starts_with("sha256:") && digest.len() == 71) + ); + assert_eq!( + plan["operationCount"], + plan["operations"].as_array().unwrap().len() + ); + assert!( + plan["operationManifestDigest"] + .as_str() + .is_some_and(|digest| digest.starts_with("sha256:") && digest.len() == 71) + ); + assert_eq!(plan["operations"][0]["kind"], "advisoryCheckCreate"); + assert_eq!(plan["operations"][0]["name"], "postil/review"); + assert_eq!(plan["operations"][1]["kind"], "reviewCreate"); + assert_eq!(plan["operations"][1]["attempt"], "initial"); + assert_eq!(plan["operations"][2]["kind"], "reviewSummaryUpdate"); + assert_eq!(plan["operations"][3]["kind"], "advisoryCheckComplete"); + assert_eq!(plan["operations"][3]["name"], "postil/review"); + assert_eq!(plan["operations"][3]["conclusion"], "success"); + assert_eq!( + plan["operations"][3]["createdCheck"]["dependencyOperationKey"], + plan["operations"][0]["operationKey"] + ); + assert_eq!( + plan["operations"][3]["dependencies"], + json!([ + plan["operations"][0]["operationKey"].clone(), + plan["operations"][2]["operationKey"].clone() + ]) + ); + assert!( + plan["operations"] + .as_array() + .unwrap() + .iter() + .all(|operation| { + operation["kind"] != "gateCheck" && operation["name"] != "postil/gate" + }) + ); + assert_eq!(plan["gateAnalysis"]["ownership"], "service"); + assert_eq!(plan["gateAnalysis"]["authoritative"], false); + assert_eq!(plan["gateAnalysis"]["organizationGateModeRequired"], true); + assert_eq!(plan["gateAnalysis"]["name"], "postil/gate"); + assert_eq!(plan["gateAnalysis"]["analyzedConclusion"], "success"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!( + std::fs::metadata(&first_plan).unwrap().permissions().mode() & 0o777, + 0o600 + ); + } + + let planning_requests = server.received_requests().await.unwrap(); + assert!( + planning_requests.iter().all(|request| { + !matches!( + request.method, + wiremock::http::Method::POST + | wiremock::http::Method::PATCH + | wiremock::http::Method::PUT + | wiremock::http::Method::DELETE + ) + }), + "publication planning attempted a GitHub mutation: {planning_requests:?}" + ); + + mount_successful_hosted_check_patches(&server).await; + Mock::given(method("POST")) + .and(path("/repos/acme/api/pulls/7/reviews")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "id": 77, + "commit_id": "aaaaaaaa" + }))) + .expect(1) + .mount(&server) + .await; + postil() + .current_dir(directory.path()) + .env("GITHUB_API_URL", server.uri()) + .env("GITHUB_TOKEN", "gh-test-token") + .args([ + "review", + "--publish", + "--repo", + "acme/api", + "--pr", + "7", + "--sha", + "aaaaaaaa", + "--base-sha", + "bbbbbbbb", + "--check-run-id", + "901", + "--gate-check-run-id", + "902", + "--output", + "json", + "--output-file", + ]) + .arg(directory.path().join("published-envelope.json")) + .assert() + .code(0); + let all_requests = server.received_requests().await.unwrap(); + assert!(all_requests.iter().any(|request| { + matches!( + request.method, + wiremock::http::Method::POST + | wiremock::http::Method::PATCH + | wiremock::http::Method::PUT + | wiremock::http::Method::DELETE + ) + })); +} + +#[test] +fn publication_plan_capability_probe_is_exact_and_external_io_free() { + let directory = tempfile::tempdir().unwrap(); + postil() + .current_dir(directory.path()) + .env("POSTIL_API_BASE", "http://127.0.0.1:1") + .env("GITHUB_API_URL", "http://127.0.0.1:1") + .args([ + "capabilities", + "--publication-plan-contract", + "github-publication-v1", + ]) + .assert() + .code(0) + .stdout("github-publication-v1\n") + .stderr(""); + assert_eq!(std::fs::read_dir(directory.path()).unwrap().count(), 0); + + postil() + .current_dir(directory.path()) + .env("POSTIL_API_BASE", "http://127.0.0.1:1") + .env("GITHUB_API_URL", "http://127.0.0.1:1") + .args([ + "capabilities", + "--publication-plan-contract", + "github-publication-v2", + ]) + .assert() + .code(2) + .stdout("") + .stderr(predicates::str::contains( + "supported contract: github-publication-v1", + )); + assert_eq!(std::fs::read_dir(directory.path()).unwrap().count(), 0); +} + +#[test] +fn publication_plan_rejects_non_github_and_local_review_modes_before_io() { + let directory = tempfile::tempdir().unwrap(); + for extra in [vec!["--forge", "gitlab"], vec!["--staged"]] { + let mut command = postil(); + command.current_dir(directory.path()).args([ + "review", + "--repo", + "acme/api", + "--pr", + "7", + "--sha", + "aaaaaaaa", + "--base-sha", + "bbbbbbbb", + "--publication-plan-output", + "publication-plan.json", + "--publication-generation", + "1", + "--publication-input-identity", + PUBLICATION_INPUT_IDENTITY, + ]); + command + .args(extra) + .assert() + .code(2) + .stderr(predicates::str::contains( + "requires a remote GitHub pull request", + )); + } + + for invalid_identity in [ + "sha256:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", + "sha256:1", + "not-a-digest", + ] { + postil() + .current_dir(directory.path()) + .args([ + "review", + "--repo", + "acme/api", + "--pr", + "7", + "--sha", + "aaaaaaaa", + "--base-sha", + "bbbbbbbb", + "--publication-plan-output", + "publication-plan.json", + "--publication-generation", + "1", + "--publication-input-identity", + invalid_identity, + ]) + .assert() + .code(2) + .stderr(predicates::str::contains( + "invalid --publication-input-identity", + )); + } +} + #[test] fn publication_looking_environment_variables_are_rejected() { for variable in ["POSTIL_PUBLISH", "POSTIL_NO_POST"] { @@ -2689,9 +3096,10 @@ async fn adjudication_provider_failure_preserves_findings_and_baseline_blocker() &server, json!([{ "path": "src/auth.rs", "line": 42, "severity": "warn", "kind": "risk", - "confidence": 0.99, "title": "Validate query input", - "body": "The query executes attacker-controlled input without validation.", - "evidence": "exec_query(&token);" + "confidence": 0.99, "title": "Authorization validator is absent", + "body": "The repository does not contain `validate_query_input`.", + "evidence": "exec_query(&token);", + "repositoryContext": {"claim": "absence", "identifiers": ["validate_query_input"]} }]), ) .await; @@ -2754,7 +3162,7 @@ async fn adjudication_provider_failure_preserves_findings_and_baseline_blocker() .as_array() .unwrap() .iter() - .any(|finding| { finding["title"] == "Validate query input" }) + .any(|finding| { finding["title"] == "Authorization validator is absent" }) ); assert!( envelope["findings"] @@ -2803,9 +3211,10 @@ async fn malformed_adjudication_output_blocks_under_advisory_provider_policy() { &server, json!([{ "path": "src/auth.rs", "line": 42, "severity": "warn", "kind": "risk", - "confidence": 0.99, "title": "Validate query input", - "body": "The query executes attacker-controlled input without validation.", - "evidence": "exec_query(&token);" + "confidence": 0.99, "title": "Authorization validator is absent", + "body": "The repository does not contain `validate_query_input`.", + "evidence": "exec_query(&token);", + "repositoryContext": {"claim": "absence", "identifiers": ["validate_query_input"]} }]), ) .await; @@ -2834,7 +3243,8 @@ async fn malformed_adjudication_output_blocks_under_advisory_provider_policy() { .unwrap() .iter() .any(|finding| { - finding["path"] == "src/auth.rs" && finding["title"] == "Validate query input" + finding["path"] == "src/auth.rs" + && finding["title"] == "Authorization validator is absent" }) ); assert!( @@ -10270,7 +10680,7 @@ async fn hosted_silent_review_does_not_require_a_later_comment_freshness_check() } #[tokio::test] -async fn github_unresolved_inline_line_falls_back_once_to_summary_only() { +async fn github_unresolved_inline_line_retries_on_a_changed_line() { let server = MockServer::start().await; mount_github_complete_diff(&server, 7).await; Mock::given(method("POST")) @@ -10316,19 +10726,47 @@ async fn github_unresolved_inline_line_falls_back_once_to_summary_only() { .with_priority(1) .mount(&server) .await; + Mock::given(method("GET")) + .and(path("/repos/acme/api/pulls/7/reviews")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!([]))) + .expect(1) + .mount(&server) + .await; + let (published_review, published_comments) = published_review_responders(); Mock::given(method("POST")) .and(path("/repos/acme/api/pulls/7/reviews")) - .respond_with(ResponseTemplate::new(200).set_body_json(json!({}))) + .respond_with(published_review) .with_priority(2) .mount(&server) .await; + Mock::given(method("GET")) + .and(path("/repos/acme/api/pulls/7/reviews/77/comments")) + .and(query_param("per_page", "100")) + .and(query_param("page", "1")) + .respond_with(published_comments) + .expect(1) + .mount(&server) + .await; + Mock::given(method("PUT")) + .and(path("/repos/acme/api/pulls/7/reviews/77")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({}))) + .expect(1) + .mount(&server) + .await; let dir = tempfile::tempdir().unwrap(); + let receipt_name = "publication-receipt.json"; + let receipt_path = dir.path().join(receipt_name); let out = postil() .current_dir(dir.path()) .env("POSTIL_API_BASE", server.uri()) .env("GITHUB_API_URL", server.uri()) .env("GITHUB_TOKEN", "gh-test-token") + .env( + "POSTIL_DETAILS_URL", + "https://postil.dev/orgs/acme/runs/run-1", + ) + .env("POSTIL_PUBLICATION_RECEIPT_PATH", receipt_name) .args([ "review", "--publish", @@ -10343,7 +10781,7 @@ async fn github_unresolved_inline_line_falls_back_once_to_summary_only() { let stderr = String::from_utf8_lossy(&out.get_output().stderr); assert!(stderr.contains("category=unresolved-line")); - assert!(stderr.contains("recovery=summary-only")); + assert!(stderr.contains("recovery=placement-ladder")); let requests = server.received_requests().await.unwrap(); let review_bodies = requests .iter() @@ -10355,7 +10793,41 @@ async fn github_unresolved_inline_line_falls_back_once_to_summary_only() { .collect::>(); assert_eq!(review_bodies.len(), 2); assert!(review_bodies[0].get("comments").is_some()); - assert!(review_bodies[1].get("comments").is_none()); + let fallback_comments = review_bodies[1]["comments"].as_array().unwrap(); + assert_eq!(fallback_comments.len(), 1); + assert_eq!(fallback_comments[0]["path"], "src/auth.rs"); + assert_eq!(fallback_comments[0]["line"], 41); + assert_eq!(fallback_comments[0]["side"], "RIGHT"); + assert!(fallback_comments[0].get("start_line").is_none()); + assert!( + !review_bodies[1]["body"] + .as_str() + .unwrap() + .contains("inline placement unavailable") + ); + let receipt: Value = serde_json::from_slice(&std::fs::read(&receipt_path).unwrap()).unwrap(); + assert_eq!(receipt["version"], 2); + assert_eq!(receipt["channel"], "reviewComments"); + assert_eq!(receipt["reviewId"], "77"); + assert_eq!(receipt["findings"][0]["initialOutcome"], "inline"); + assert_eq!(receipt["findings"][0]["commentId"], "500"); + assert!(!std::fs::read_dir(dir.path()).unwrap().any(|entry| { + let name = entry.unwrap().file_name(); + let name = name.to_string_lossy(); + name.starts_with(".publication-receipt.json.") && name.ends_with(".tmp") + })); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!( + std::fs::metadata(&receipt_path) + .unwrap() + .permissions() + .mode() + & 0o777, + 0o600 + ); + } } #[tokio::test] @@ -10522,9 +10994,18 @@ async fn github_flow_posts_review_and_completes_both_checks() { .respond_with(ResponseTemplate::new(200).set_body_json(json!({}))) .mount(&server) .await; + let (published_review, published_comments) = published_review_responders(); Mock::given(method("POST")) .and(path("/repos/acme/api/pulls/7/reviews")) - .respond_with(PublishedReviewResponder) + .respond_with(published_review) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/repos/acme/api/pulls/7/reviews/77/comments")) + .and(query_param("per_page", "100")) + .and(query_param("page", "1")) + .respond_with(published_comments) + .expect(2) .mount(&server) .await; Mock::given(method("PUT")) @@ -11339,7 +11820,198 @@ async fn full_rereview_resolves_false_absence_from_unchanged_repository_source() } #[tokio::test] -async fn fresh_repository_claims_remain_open_for_unavailable_and_exhausted_receipts() { +async fn same_hunk_groups_replacement_refutes_false_deletion_claim() { + const PATH: &str = "ansible/group_vars/atlas.yml"; + const OLD_LINE: &str = "groups: [atlas_packages_base]"; + const REPLACEMENT: &str = "groups: [atlas_packages_base, atlas_packages_test]"; + + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/chat/completions")) + .and(body_string_contains("single finding adjudicator")) + .respond_with(RefuteFromReceiptAdjudicator) + .with_priority(1) + .mount(&server) + .await; + mock_review( + &server, + json!([{ + "path": PATH, + "line": 120, + "severity": "warn", + "kind": "uncertainty", + "confidence": 0.91, + "title": "The groups entry was deleted", + "body": "The `groups` entry was deleted without its replacement `atlas_packages_test`.", + "evidence": "policy: atlas", + "repositoryContext": { + "claim": "mismatch", + "resources": ["groups"], + "values": ["atlas_packages_test"], + "versions": [], + "paths": [], + "identifiers": [] + } + }]), + ) + .await; + + let directory = tempfile::tempdir().unwrap(); + let diff = directory.path().join("groups-replacement.diff"); + std::fs::write( + &diff, + format!( + "diff --git a/{PATH} b/{PATH}\n--- a/{PATH}\n+++ b/{PATH}\n@@ -120,4 +120,4 @@\n-policy: legacy\n+policy: atlas\n stable: true\n owner: atlas\n-{OLD_LINE}\n+{REPLACEMENT}\n" + ), + ) + .unwrap(); + + let output = postil() + .current_dir(directory.path()) + .env("POSTIL_API_BASE", server.uri()) + .env("POSTIL_DISABLE_SCORER", "1") + .args(["review", "--diff-file"]) + .arg(&diff) + .args(["--output", "json"]) + .assert() + .success(); + let envelope: Value = serde_json::from_slice(&output.get_output().stdout).unwrap(); + + assert_eq!(envelope["findings"], json!([])); + assert_eq!(envelope["suppressedFindings"][0]["finding"]["path"], PATH); + assert_eq!( + envelope["suppressedFindings"][0]["finding"]["title"], + "The groups entry was deleted" + ); + assert_eq!(envelope["suppressedFindings"][0]["reason"], "nonActionable"); + assert_eq!(envelope["counts"]["warn"], 0); + assert_eq!(envelope["counts"]["suppressed"], 1); + assert_eq!(envelope["gate"]["failing"], false); + + let requests = server.received_requests().await.unwrap(); + let adjudication: Value = requests + .iter() + .find(|request| request_system_contains(request, "single finding adjudicator")) + .unwrap() + .body_json() + .unwrap(); + let payload: Value = serde_json::from_str( + adjudication["messages"].as_array().unwrap().last().unwrap()["content"] + .as_str() + .unwrap(), + ) + .unwrap(); + assert_eq!( + payload["diffCorpusReceipt"]["candidateCitations"][0]["refutationEvidenceComplete"], + true + ); + assert_eq!( + payload["diffCorpusReceipt"]["candidateCitations"][0]["refutationEvidence"]["source"], + REPLACEMENT + ); +} + +#[tokio::test] +async fn cross_file_package_existence_refutes_false_absence_claim() { + const CLAIM_PATH: &str = "ci/package-policy.yml"; + const PACKAGE_PATH: &str = "packages/atlas/manifest.yml"; + const REPLACEMENT: &str = "name: atlas_packages_base"; + + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/chat/completions")) + .and(body_string_contains("single finding adjudicator")) + .respond_with(RefuteFromReceiptAdjudicator) + .with_priority(1) + .mount(&server) + .await; + mock_review( + &server, + json!([{ + "path": CLAIM_PATH, + "line": 24, + "severity": "warn", + "kind": "uncertainty", + "confidence": 0.88, + "title": "The atlas package is absent", + "body": "The repository does not contain `atlas_packages_base`.", + "evidence": "package: atlas_packages_candidate", + "repositoryContext": { + "claim": "absence", + "resources": ["atlas package"], + "values": ["atlas_packages_base"], + "versions": [], + "paths": [], + "identifiers": [] + } + }]), + ) + .await; + + let directory = tempfile::tempdir().unwrap(); + let diff = directory.path().join("package-existence.diff"); + std::fs::write( + &diff, + format!( + "diff --git a/{CLAIM_PATH} b/{CLAIM_PATH}\n--- a/{CLAIM_PATH}\n+++ b/{CLAIM_PATH}\n@@ -24,1 +24,1 @@\n-package: atlas_packages_legacy\n+package: atlas_packages_candidate\ndiff --git a/{PACKAGE_PATH} b/{PACKAGE_PATH}\n--- a/{PACKAGE_PATH}\n+++ b/{PACKAGE_PATH}\n@@ -1,2 +1,2 @@\n-type: legacy package\n-name: atlas_packages_legacy\n+type: atlas package\n+{REPLACEMENT}\n" + ), + ) + .unwrap(); + + let output = postil() + .current_dir(directory.path()) + .env("POSTIL_API_BASE", server.uri()) + .env("POSTIL_DISABLE_SCORER", "1") + .args(["review", "--diff-file"]) + .arg(&diff) + .args(["--output", "json"]) + .assert() + .success(); + let envelope: Value = serde_json::from_slice(&output.get_output().stdout).unwrap(); + + assert_eq!(envelope["findings"], json!([])); + assert_eq!( + envelope["suppressedFindings"][0]["finding"]["path"], + CLAIM_PATH + ); + assert_eq!( + envelope["suppressedFindings"][0]["finding"]["title"], + "The atlas package is absent" + ); + assert_eq!(envelope["suppressedFindings"][0]["reason"], "nonActionable"); + assert_eq!(envelope["counts"]["warn"], 0); + assert_eq!(envelope["counts"]["suppressed"], 1); + assert_eq!(envelope["gate"]["failing"], false); + + let requests = server.received_requests().await.unwrap(); + let adjudication: Value = requests + .iter() + .find(|request| request_system_contains(request, "single finding adjudicator")) + .unwrap() + .body_json() + .unwrap(); + let payload: Value = serde_json::from_str( + adjudication["messages"].as_array().unwrap().last().unwrap()["content"] + .as_str() + .unwrap(), + ) + .unwrap(); + assert_eq!( + payload["diffCorpusReceipt"]["candidateCitations"][0]["refutationEvidenceComplete"], + true + ); + assert_eq!( + payload["diffCorpusReceipt"]["candidateCitations"][0]["refutationEvidence"]["path"], + PACKAGE_PATH + ); + assert_eq!( + payload["diffCorpusReceipt"]["candidateCitations"][0]["refutationEvidence"]["source"], + REPLACEMENT + ); +} + +#[tokio::test] +async fn fresh_unresolved_repository_claims_are_suppressed() { for (name, repository, resources, state) in [ ( "unavailable", @@ -11378,15 +12050,12 @@ async fn fresh_repository_claims_remain_open_for_unavailable_and_exhausted_recei command.arg("--diff-file").arg(&diff); } command.args(["--output", "json"]); - let out = command.assert().code(1); + let out = command.assert().code(0); let envelope: Value = serde_json::from_slice(&out.get_output().stdout).unwrap(); assert_eq!(envelope["repositorySearch"]["state"], state, "{name}"); - assert_eq!(envelope["counts"]["suppressed"], 0, "{name}"); - assert_eq!( - envelope["findings"][0]["title"], "Widget dependency is absent", - "{name}" - ); - assert_eq!(envelope["gate"]["failing"], true, "{name}"); + assert_eq!(envelope["counts"]["suppressed"], 1, "{name}"); + assert_eq!(envelope["findings"], json!([]), "{name}"); + assert_eq!(envelope["gate"]["failing"], false, "{name}"); } }