CLI: sync fingerprint enrichment, fetch hash-after-resolution, diff gate ordering

sync: endpoint_fingerprint now includes security_schemes, security_required,
tags, and operation_id — catches changes that previously went undetected.
Method comparison uppercased for consistency. New details field in
SyncOutput carries full ChangeDetails alongside the summary. Removed
unused _skipped_from_resume binding. normalize_to_json callers updated
for new (bytes, Value) return type. allow_private_host forwarded to
sync --all HTTP client builder.

fetch: content hash now computed from post-resolution json_bytes instead
of raw_bytes — when --resolve-external-refs is used, the stored content
differs from the original fetch, so the hash must match what is actually
written to the cache.

diff: --fail-on=breaking check moved before any output to avoid the
contradictory pattern of emitting success JSON then returning an error
exit code. Robot mode now emits a proper robot_error envelope on
breaking-change failure.
This commit is contained in:
teernisse
2026-02-12 16:14:01 -05:00
parent a36997982a
commit 75d9344b44
3 changed files with 50 additions and 26 deletions

View File

@@ -79,6 +79,26 @@ pub async fn execute(args: &Args, robot_mode: bool) -> Result<(), SwaggerCliErro
let has_breaking = result.summary.has_breaking; let has_breaking = result.summary.has_breaking;
// CI gate: check breaking changes before any output to avoid
// contradictory success-JSON-then-error in robot mode.
if args.fail_on.as_deref() == Some("breaking") && has_breaking {
if robot_mode {
let err = SwaggerCliError::Usage(
"Breaking changes detected (use --fail-on to control this check)".into(),
);
robot::robot_error(
err.code(),
&err.to_string(),
err.suggestion(),
"diff",
duration,
);
}
return Err(SwaggerCliError::Usage(
"Breaking changes detected (use --fail-on to control this check)".into(),
));
}
if robot_mode { if robot_mode {
let output = DiffOutput { let output = DiffOutput {
left: args.left.clone(), left: args.left.clone(),
@@ -145,13 +165,6 @@ pub async fn execute(args: &Args, robot_mode: bool) -> Result<(), SwaggerCliErro
} }
} }
// CI gate: exit non-zero on breaking changes when requested
if args.fail_on.as_deref() == Some("breaking") && has_breaking {
return Err(SwaggerCliError::Usage(
"Breaking changes detected (use --fail-on to control this check)".into(),
));
}
Ok(()) Ok(())
} }

View File

@@ -290,8 +290,7 @@ async fn fetch_inner(
Format::Yaml => "yaml", Format::Yaml => "yaml",
}; };
let json_bytes = normalize_to_json(&raw_bytes, format)?; let (_json_bytes, mut value) = normalize_to_json(&raw_bytes, format)?;
let mut value: serde_json::Value = serde_json::from_slice(&json_bytes)?;
// External ref resolution (optional) // External ref resolution (optional)
if args.resolve_external_refs { if args.resolve_external_refs {
@@ -322,8 +321,11 @@ async fn fetch_inner(
// Re-serialize the (possibly bundled) value to get the final json_bytes // Re-serialize the (possibly bundled) value to get the final json_bytes
let json_bytes = serde_json::to_vec(&value)?; let json_bytes = serde_json::to_vec(&value)?;
// Compute content hash for indexing // Compute content hash from the final json_bytes (post-resolution), not
let content_hash = compute_hash(&raw_bytes); // the original raw_bytes. When external refs are resolved, the stored
// content differs from the original fetch, so the hash must match what
// is actually written to the cache.
let content_hash = compute_hash(&json_bytes);
// Determine generation: if overwriting, increment previous generation // Determine generation: if overwriting, increment previous generation
let previous_generation = if args.force && cm.alias_exists(&args.alias) { let previous_generation = if args.force && cm.alias_exists(&args.alias) {

View File

@@ -143,6 +143,8 @@ struct AliasSyncResult {
remote_version: Option<String>, remote_version: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
changes: Option<ChangeSummary>, changes: Option<ChangeSummary>,
#[serde(skip_serializing_if = "Option::is_none")]
details: Option<ChangeDetails>,
duration_ms: u64, duration_ms: u64,
} }
@@ -201,13 +203,15 @@ fn remove_checkpoint(cache_path: &std::path::Path) {
// Index diffing // Index diffing
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
/// Build a comparable key for an endpoint: (path, method). /// Build a comparable key for an endpoint: (path, METHOD).
/// Method is uppercased for consistent comparison regardless of indexer casing.
fn endpoint_key(ep: &crate::core::spec::IndexedEndpoint) -> (String, String) { fn endpoint_key(ep: &crate::core::spec::IndexedEndpoint) -> (String, String) {
(ep.path.clone(), ep.method.clone()) (ep.path.clone(), ep.method.to_uppercase())
} }
/// Build a fingerprint of an endpoint for modification detection. /// Build a fingerprint of an endpoint for modification detection.
/// Includes summary, parameters, deprecated status, and request body info. /// Includes all semantically meaningful fields so that changes to security,
/// tags, operation_id, etc. are detected during sync.
fn endpoint_fingerprint(ep: &crate::core::spec::IndexedEndpoint) -> String { fn endpoint_fingerprint(ep: &crate::core::spec::IndexedEndpoint) -> String {
let params: Vec<String> = ep let params: Vec<String> = ep
.parameters .parameters
@@ -216,12 +220,16 @@ fn endpoint_fingerprint(ep: &crate::core::spec::IndexedEndpoint) -> String {
.collect(); .collect();
format!( format!(
"{}|{}|{}|{}|{}", "{}|{}|{}|{}|{}|{}|{}|{}|{}",
ep.summary.as_deref().unwrap_or(""), ep.summary.as_deref().unwrap_or(""),
ep.deprecated, ep.deprecated,
params.join(","), params.join(","),
ep.request_body_required, ep.request_body_required,
ep.request_body_content_types.join(","), ep.request_body_content_types.join(","),
ep.security_schemes.join(","),
ep.security_required,
ep.tags.join(","),
ep.operation_id.as_deref().unwrap_or(""),
) )
} }
@@ -378,6 +386,7 @@ async fn sync_one_alias(
local_version: None, local_version: None,
remote_version: None, remote_version: None,
changes: None, changes: None,
details: None,
duration_ms: start.elapsed().as_millis().min(u64::MAX as u128) as u64, duration_ms: start.elapsed().as_millis().min(u64::MAX as u128) as u64,
}, },
} }
@@ -463,6 +472,7 @@ async fn sync_one_alias_inner(
local_version: Some(meta.spec_version.clone()), local_version: Some(meta.spec_version.clone()),
remote_version: None, remote_version: None,
changes: None, changes: None,
details: None,
duration_ms: elapsed_ms(), duration_ms: elapsed_ms(),
}), }),
ConditionalFetchResult::Modified(result) => { ConditionalFetchResult::Modified(result) => {
@@ -478,6 +488,7 @@ async fn sync_one_alias_inner(
local_version: Some(meta.spec_version.clone()), local_version: Some(meta.spec_version.clone()),
remote_version: None, remote_version: None,
changes: None, changes: None,
details: None,
duration_ms: elapsed_ms(), duration_ms: elapsed_ms(),
}); });
} }
@@ -488,11 +499,10 @@ async fn sync_one_alias_inner(
Format::Yaml => "yaml", Format::Yaml => "yaml",
}; };
let json_bytes = normalize_to_json(&result.bytes, format)?; let (json_bytes, value) = normalize_to_json(&result.bytes, format)?;
let value: serde_json::Value = serde_json::from_slice(&json_bytes)?;
let new_index = build_index(&value, &new_content_hash, meta.generation + 1)?; let new_index = build_index(&value, &new_content_hash, meta.generation + 1)?;
let (summary, _details) = compute_diff(&old_index, &new_index); let (summary, details) = compute_diff(&old_index, &new_index);
let has_changes = summary.endpoints_added > 0 let has_changes = summary.endpoints_added > 0
|| summary.endpoints_removed > 0 || summary.endpoints_removed > 0
@@ -533,6 +543,7 @@ async fn sync_one_alias_inner(
local_version: Some(meta.spec_version.clone()), local_version: Some(meta.spec_version.clone()),
remote_version: Some(new_index.info.version.clone()), remote_version: Some(new_index.info.version.clone()),
changes: if include_details { Some(summary) } else { None }, changes: if include_details { Some(summary) } else { None },
details: if include_details { Some(details) } else { None },
duration_ms: elapsed_ms(), duration_ms: elapsed_ms(),
}) })
} }
@@ -573,6 +584,7 @@ async fn sync_inner(
let cfg = Config::load(&config_path(config_override))?; let cfg = Config::load(&config_path(config_override))?;
let mut builder = AsyncHttpClient::builder() let mut builder = AsyncHttpClient::builder()
.allow_insecure_http(url.starts_with("http://")) .allow_insecure_http(url.starts_with("http://"))
.allowed_private_hosts(args.allow_private_host.clone())
.network_policy(network_policy); .network_policy(network_policy);
if let Some(profile_name) = &args.auth { if let Some(profile_name) = &args.auth {
@@ -637,8 +649,7 @@ async fn sync_inner(
Format::Yaml => "yaml", Format::Yaml => "yaml",
}; };
let json_bytes = normalize_to_json(&result.bytes, format)?; let (json_bytes, value) = normalize_to_json(&result.bytes, format)?;
let value: serde_json::Value = serde_json::from_slice(&json_bytes)?;
let new_index = build_index(&value, &new_content_hash, meta.generation + 1)?; let new_index = build_index(&value, &new_content_hash, meta.generation + 1)?;
// 6. Compute diff // 6. Compute diff
@@ -821,12 +832,6 @@ async fn sync_all_inner(
}; };
let total = aliases.len(); let total = aliases.len();
let _skipped_from_resume = if args.resume {
total - to_sync.len()
} else {
0
};
// Handle empty aliases // Handle empty aliases
if total == 0 { if total == 0 {
let output = SyncAllOutput { let output = SyncAllOutput {
@@ -890,6 +895,7 @@ async fn sync_all_inner(
local_version: None, local_version: None,
remote_version: None, remote_version: None,
changes: None, changes: None,
details: None,
duration_ms: 0, duration_ms: 0,
}; };
} }
@@ -911,6 +917,7 @@ async fn sync_all_inner(
local_version: None, local_version: None,
remote_version: None, remote_version: None,
changes: None, changes: None,
details: None,
duration_ms: 0, duration_ms: 0,
}; };
} }
@@ -972,6 +979,7 @@ async fn sync_all_inner(
local_version: None, local_version: None,
remote_version: None, remote_version: None,
changes: None, changes: None,
details: None,
duration_ms: 0, duration_ms: 0,
}); });
} }
@@ -985,6 +993,7 @@ async fn sync_all_inner(
local_version: None, local_version: None,
remote_version: None, remote_version: None,
changes: None, changes: None,
details: None,
duration_ms: 0, duration_ms: 0,
}); });
} }