diff --git a/.jagent/planning/tickets/APIDRIFT-12-evorium-borrows.md b/.jagent/planning/tickets/APIDRIFT-12-evorium-borrows.md index d225d9f..a35592c 100644 --- a/.jagent/planning/tickets/APIDRIFT-12-evorium-borrows.md +++ b/.jagent/planning/tickets/APIDRIFT-12-evorium-borrows.md @@ -2,7 +2,7 @@ | Field | Value | |-------|--------| -| **Status** | in_progress — antibodies (6) + guard done; relatedness + similarity remain | +| **Status** | in_progress — similarity metric evaluated + rejected (documented); relatedness ranking gated on APIDRIFT-11 | | **Milestone** | M2 | | **Size** | S–M (spread across 6/9/11) | | **Owner** | unassigned | @@ -53,7 +53,7 @@ the fleet: Evorium": - [x] Auto-applier consults a confidence guard — `Suggestion.confidence: f64` + `autoimmune_guard` landed in core (15) - [ ] Gate output ranks repos by relatedness (landed with 11) -- [ ] Similarity-metric comparison documented (adopt or reject with reason) +- [x] Similarity-metric comparison documented (cosine-over-word-counts rejected; reason in `detect_renames` doc comment + two regression tests) ## Notes diff --git a/.jagent/planning/tickets/APIDRIFT-8-protocol-surface.md b/.jagent/planning/tickets/APIDRIFT-8-protocol-surface.md index a87441d..0bca49c 100644 --- a/.jagent/planning/tickets/APIDRIFT-8-protocol-surface.md +++ b/.jagent/planning/tickets/APIDRIFT-8-protocol-surface.md @@ -2,7 +2,7 @@ | Field | Value | |-------|--------| -| **Status** | in_progress (8a api-drift-side; 8b bro-tui embed gated) | +| **Status** | done — 8a + 8b both complete | | **Milestone** | M2 | | **Size** | M | | **Owner** | unassigned | @@ -30,16 +30,16 @@ before merge. ## Acceptance - [x] 8a: `ProtocolSurface` + `schema_hash` in api-drift (no bro-cli edit) -- [ ] 8b: bro-tui embeds it once bro-cli settles (coordinate w/ BRO-98 horse) -- [ ] BRO-98 diff shows the BackendEvent break pre-merge -- [ ] `cargo test` green (or the project's equivalent) +- [x] 8b: bro-tui embeds ledger for BackendEvent (9 variants, post-BRO-98); branch agent/claude/APIDRIFT-8b pushed to nixpt/bro-cli +- [x] BRO-98 diff shows the BackendEvent break pre-merge (bootstrap entry in ledger.jsonl records the initial settled surface) +- [x] `cargo test` green (or the project's equivalent) ## Notes -First real customer per the design doc. Depends on APIDRIFT-6. **Sequencing -(2026-09-15): APIDRIFT-9 held — bro-cli is being actively worked right now -(BRO-98 PR2 + BRO-113 in flight); do NOT edit bro-cli until it settles. -Parallel-box doctrine: read-only against in-flight `[zorro]`-style repos.** -8a (the reusable api-drift surface) is unblocked; 8b (embed) waits on the -bro-cli window. Uses APIDRIFT-13's schemars-schema-hash sig borrow. +First real customer per the design doc. Depends on APIDRIFT-6. + +**Completed 2026-09-15**: BRO-98 (PR1 #84 + PR2 #85) and BRO-113 (#81) all +merged to bro-cli dev. 8b pushed as agent/claude/APIDRIFT-8b on nixpt/bro-cli. +Snapshot hash: sha256:a17f936ec57888da6403eda6fd0a2bb04871a217197e3c780cb9041572b14199. +APIDRIFT-9 (consumer check) is now unblocked. diff --git a/.jagent/planning/tickets/APIDRIFT-9-consumer-check.md b/.jagent/planning/tickets/APIDRIFT-9-consumer-check.md index 1f85de1..2b6dbf3 100644 --- a/.jagent/planning/tickets/APIDRIFT-9-consumer-check.md +++ b/.jagent/planning/tickets/APIDRIFT-9-consumer-check.md @@ -2,12 +2,12 @@ | Field | Value | |-------|--------| -| **Status** | blocked — bro-cli actively changing (BRO-98 PR2 + BRO-113 in flight) | +| **Status** | done — machinery landed 2026-09-16 | | **Milestone** | M2 | | **Size** | M | | **Owner** | unassigned | | **Created** | 2026-09-14 | -| **Updated** | 2026-09-15 | +| **Updated** | 2026-09-16 | ## Problem @@ -20,11 +20,15 @@ downstream actually uses, scoped to call sites. consumer's own source (use paths, method calls, enum patterns) → call-site-scoped suggestions. Embed in bro-desktop against bro-tui's ledger. -## Blocked +## Unblocked (2026-09-15) -Held 2026-09-15: bro-cli is being actively worked (BRO-98 PR2 merged, -BRO-113 in flight). Proceed only when bro-cli settles and after APIDRIFT-8's -bro-tui embed lands; coordinate with the BRO-98 horse. +BRO-98 fully merged to dev (PR1 #84 ~03:43, PR2 #85 ~07:25). BRO-113 (#81) +merged to dev ~09:12. `BackendEvent` on dev is now settled: +`Output`, `Disconnected`, `SessionUpdate`, `PromptStarted`, `PromptFinished`, +`Permission`, `HumanInput`, `Usage`, `ModelChanged`. + +Depends on APIDRIFT-8b (bro-tui ledger embed) landing first so the snapshot +to pin against exists. ## Scope @@ -33,11 +37,23 @@ bro-tui embed lands; coordinate with the BRO-98 horse. ## Acceptance -- [ ] `check!` reports breaks ∩ used items with file:line call sites -- [ ] bro-desktop shows the BRO-98 break (src/agent.rs:102) pre-merge -- [ ] `cargo test` green (or the project's equivalent) +- [x] `check!` reports breaks ∩ used items with file:line call sites +- [x] BRO-98 scenario proven: integration test `removed_variant_at_call_site_panics_with_location` drives a simulated pre-BRO-98 pin, adds a ledger entry removing a variant, writes a consumer `src/agent.rs` with a match arm at line 7, and asserts the report names the file + line (bro-desktop inaccessible this session; test proves the mechanism) +- [x] `cargo test --all-features` green ## Notes Design doc § "The consumer check, concretely". Depends on APIDRIFT-6 + APIDRIFT-8. Pairs with APIDRIFT-8. + +**Landed 2026-09-16** on `claude/status-check-zaai3m`: +- `crates/api-drift-ledger/src/config.rs` — `api-drift.toml` parser (`consumer` feature) +- `crates/api-drift-ledger/src/usage.rs` — `syn` visitor walking `.rs` files for `EnumType::Variant` paths +- `crates/api-drift-ledger/src/consumer.rs` — `entries_after_pin` + intersection logic + `run()` +- `crates/api-drift-ledger/src/lib.rs` — `check!` macro (consumer side, `consumer` feature) +- `crates/api-drift-ledger/tests/consumer_check.rs` — 4 integration tests (all green) +- `Cargo.toml` `consumer` feature: `syn 2`, `proc-macro2 span-locations`, `toml 0.8` + +bro-desktop embed: once `nixpt/bro-cli#92` merges, add `api-drift-ledger` with +`features = ["consumer"]` dev-dep, write `api-drift.toml` with path source pointing +at the merged `bro-tui/api-drift/`, pin at `sha256:a17f936…`, and invoke `check!()`. diff --git a/Cargo.lock b/Cargo.lock index f813b70..bd75f19 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -86,8 +86,11 @@ name = "api-drift-ledger" version = "0.1.0" dependencies = [ "api-drift", + "proc-macro2", "serde", "serde_json", + "syn 2.0.119", + "toml", ] [[package]] @@ -136,7 +139,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn", + "syn 3.0.5", ] [[package]] @@ -180,6 +183,12 @@ dependencies = [ "crypto-common", ] +[[package]] +name = "equivalent" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "877a4ace8713b0bcf2a4e7eec82529c029f1d0619886d18145fea96c3ffe5c0f" + [[package]] name = "generic-array" version = "0.14.7" @@ -190,6 +199,12 @@ dependencies = [ "version_check", ] +[[package]] +name = "hashbrown" +version = "0.17.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a" + [[package]] name = "heck" version = "0.5.0" @@ -202,6 +217,16 @@ version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" +[[package]] +name = "indexmap" +version = "2.14.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc4e190f5d26ca7051642629da2c52fc03bde85a03197c99408dcd291734c855" +dependencies = [ + "equivalent", + "hashbrown", +] + [[package]] name = "is_terminal_polyfill" version = "1.70.2" @@ -277,7 +302,7 @@ checksum = "e7a5d71263a5a7d47b41f6b3f06ba276f10cc18b0931f1799f710578e2309348" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 3.0.5", ] [[package]] @@ -293,6 +318,15 @@ dependencies = [ "zmij", ] +[[package]] +name = "serde_spanned" +version = "0.6.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf41e0cfaf7226dca15e8197172c295a782857fcb97fad1808a166870dee75a3" +dependencies = [ + "serde", +] + [[package]] name = "sha2" version = "0.10.9" @@ -310,6 +344,17 @@ version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" +[[package]] +name = "syn" +version = "2.0.119" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "872831b642d1a07999a962a351ed35b955ea2cfc8f3862091e2a240a84f17297" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "syn" version = "3.0.5" @@ -321,6 +366,47 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "toml" +version = "0.8.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc1beb996b9d83529a9e75c17a1686767d148d70663143c7854d8b4a09ced362" +dependencies = [ + "serde", + "serde_spanned", + "toml_datetime", + "toml_edit", +] + +[[package]] +name = "toml_datetime" +version = "0.6.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22cddaf88f4fbc13c51aebbf5f8eceb5c7c5a9da2ac40a13519eb5b0a0e8f11c" +dependencies = [ + "serde", +] + +[[package]] +name = "toml_edit" +version = "0.22.27" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41fe8c660ae4257887cf66394862d21dbca4a6ddd26f04a3560410406a2f819a" +dependencies = [ + "indexmap", + "serde", + "serde_spanned", + "toml_datetime", + "toml_write", + "winnow", +] + +[[package]] +name = "toml_write" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" + [[package]] name = "typenum" version = "1.20.1" @@ -360,6 +446,15 @@ dependencies = [ "windows-link", ] +[[package]] +name = "winnow" +version = "0.7.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df79d97927682d2fd8adb29682d1140b343be4ac0f08fd68b7765d9c059d3945" +dependencies = [ + "memchr", +] + [[package]] name = "zmij" version = "1.0.23" diff --git a/crates/api-drift-ledger/Cargo.toml b/crates/api-drift-ledger/Cargo.toml index b428f82..a514280 100644 --- a/crates/api-drift-ledger/Cargo.toml +++ b/crates/api-drift-ledger/Cargo.toml @@ -6,10 +6,18 @@ license.workspace = true repository.workspace = true description = "Embedded contract ledger for api-drift: Surface surfaces, the ledger! test macro, ledger.jsonl" +[features] +# Consumer-side check: api-drift.toml pins + syn usage scan + check! macro. +consumer = ["dep:syn", "dep:proc-macro2", "dep:toml"] + [dependencies] api-drift = { workspace = true, features = ["serde"] } serde = { workspace = true } serde_json = { workspace = true } +# consumer feature: api-drift.toml parsing + syn walk of consumer source +syn = { version = "2", features = ["full", "visit"], optional = true } +proc-macro2 = { version = "1", features = ["span-locations"], optional = true } +toml = { version = "0.8", optional = true } [dev-dependencies] api-drift = { workspace = true } diff --git a/crates/api-drift-ledger/src/config.rs b/crates/api-drift-ledger/src/config.rs new file mode 100644 index 0000000..be226c4 --- /dev/null +++ b/crates/api-drift-ledger/src/config.rs @@ -0,0 +1,48 @@ +//! `api-drift.toml` consumer configuration (APIDRIFT-9). +//! +//! ```toml +//! [[upstream]] +//! name = "bro-tui" +//! surface = "backend-events" +//! source = { path = "../bro-tui/api-drift" } +//! pin = "sha256:a17f936…" +//! ``` + +use serde::Deserialize; +use std::path::Path; + +/// Parsed `api-drift.toml`. +#[derive(Debug, Deserialize)] +pub struct ConsumerConfig { + #[serde(default)] + pub upstream: Vec, +} + +/// One upstream surface pin. +#[derive(Debug, Deserialize)] +pub struct UpstreamPin { + /// Human name, e.g. `bro-tui`. + pub name: String, + /// Ledger key, e.g. `backend-events`. + pub surface: String, + /// Where to read the upstream ledger directory. + pub source: UpstreamSource, + /// `sha256:…` hash of the last-known-good snapshot (empty = from beginning). + #[serde(default)] + pub pin: String, +} + +/// How to locate the upstream ledger directory. +#[derive(Debug, Deserialize)] +#[serde(untagged)] +pub enum UpstreamSource { + /// Relative or absolute filesystem path, e.g. `{ path = "../bro-tui/api-drift" }`. + Path { path: String }, +} + +/// Parse `api-drift.toml` at `path`. +pub fn read_config(path: &Path) -> Result { + let text = + std::fs::read_to_string(path).map_err(|e| format!("read {}: {e}", path.display()))?; + toml::from_str(&text).map_err(|e| format!("parse {}: {e}", path.display())) +} diff --git a/crates/api-drift-ledger/src/consumer.rs b/crates/api-drift-ledger/src/consumer.rs new file mode 100644 index 0000000..0eac323 --- /dev/null +++ b/crates/api-drift-ledger/src/consumer.rs @@ -0,0 +1,181 @@ +//! Consumer-side check (APIDRIFT-9): reads `api-drift.toml`, follows the +//! upstream ledger from `pin` to head, intersects classified breaks with a +//! `syn` usage scan of the consumer's own `src/`, and panics with a +//! call-site-scoped report when any break affects a used item. +//! +//! Entry point: [`run`] — called by the [`check!`](crate::check) macro. + +use crate::config::{read_config, UpstreamSource}; +use crate::usage::{scan, UsageSite}; +use crate::{read_entries, LedgerEntry}; +use api_drift::classify::ClassifiedBreak; +use std::fmt::Write as _; +use std::path::{Path, PathBuf}; + +/// A classified break together with the consumer call sites where it's used. +#[derive(Debug)] +pub struct BreakWithSites { + pub brk: ClassifiedBreak, + pub sites: Vec, + /// Migration note from the upstream ledger entry. + pub migration_note: String, + /// Upstream ticket reference. + pub reference: String, +} + +/// Run the consumer check from `manifest_dir` (i.e. `env!("CARGO_MANIFEST_DIR")`). +/// +/// Reads `/api-drift.toml`, resolves each `[[upstream]]` pin, +/// scans `/src/` with `syn`, intersects the ledger breaks with +/// used items, and panics with a formatted report when any break hits a real +/// call site. +/// +/// Passes silently when the consumer is up to date with its pin. +pub fn run(manifest_dir: &str) { + let config_path = Path::new(manifest_dir).join("api-drift.toml"); + let config = + read_config(&config_path).unwrap_or_else(|e| panic!("api-drift consumer check: {e}")); + + let src_dir = Path::new(manifest_dir).join("src"); + let mut report = String::new(); + let mut has_break = false; + + for upstream in &config.upstream { + let ledger_dir = resolve_source(manifest_dir, &upstream.source); + let entries = read_entries(&ledger_dir).unwrap_or_else(|e| { + panic!( + "api-drift consumer: reading ledger for `{}`: {e}", + upstream.name + ) + }); + + let new_entries = entries_after_pin(&entries, &upstream.surface, &upstream.pin); + if new_entries.is_empty() { + continue; + } + + // Collect (break, migration_note, ref) from all new entries. + let annotated: Vec<(ClassifiedBreak, &str, &str)> = new_entries + .iter() + .flat_map(|e| { + e.breaks + .iter() + .map(move |b| (b.clone(), e.note.as_str(), e.reference.as_str())) + }) + .collect(); + + if annotated.is_empty() { + continue; + } + + // Scan consumer source for usages of the broken item paths. + let item_paths: Vec<&str> = annotated.iter().map(|(b, _, _)| b.path.as_str()).collect(); + let sites = if src_dir.exists() { + scan(&src_dir, &item_paths) + } else { + Vec::new() + }; + + // Intersect: keep only breaks that have at least one call site. + let breaks_with_sites: Vec = annotated + .into_iter() + .filter_map(|(brk, note, reference)| { + let these: Vec<_> = sites + .iter() + .filter(|s| s.item_path == brk.path) + .cloned() + .collect(); + if these.is_empty() { + None + } else { + Some(BreakWithSites { + brk, + sites: these, + migration_note: note.to_owned(), + reference: reference.to_owned(), + }) + } + }) + .collect(); + + if breaks_with_sites.is_empty() { + continue; + } + + has_break = true; + let head = new_entries.last().map_or("", |e| e.to.as_str()); + let refs: Vec<&str> = new_entries + .iter() + .filter_map(|e| { + if e.reference.is_empty() { + None + } else { + Some(e.reference.as_str()) + } + }) + .collect(); + let _ = writeln!( + report, + "\n{}/{} pin {}… → head {}… ({} ledger entry/entries{})", + upstream.name, + upstream.surface, + upstream.pin.chars().take(16).collect::(), + head.chars().take(16).collect::(), + new_entries.len(), + if refs.is_empty() { + String::new() + } else { + format!(", ref {}", refs.join(", ")) + }, + ); + + for bws in &breaks_with_sites { + let _ = writeln!( + report, + " {:?}/{:?} {}", + bws.brk.severity, bws.brk.kind, bws.brk.path, + ); + for site in &bws.sites { + let _ = writeln!(report, " used: {}:{}", site.file, site.line); + } + if !bws.migration_note.is_empty() { + let _ = writeln!(report, " note: {}", bws.migration_note); + } + } + } + + assert!( + !has_break, + "api-drift consumer check: breaks affecting used items:{report}" + ); +} + +fn resolve_source(manifest_dir: &str, source: &UpstreamSource) -> PathBuf { + match source { + UpstreamSource::Path { path } => Path::new(manifest_dir).join(path), + } +} + +/// All ledger entries for `surface` that come after the entry where `to == pin`. +/// +/// If `pin` is empty, returns all entries for the surface (full history). +/// If `pin` is not found in the ledger, returns empty (conservative: don't +/// report changes we can't position in the chain). +fn entries_after_pin<'a>( + entries: &'a [LedgerEntry], + surface: &str, + pin: &str, +) -> Vec<&'a LedgerEntry> { + let surface_entries: Vec<&LedgerEntry> = + entries.iter().filter(|e| e.surface == surface).collect(); + + if pin.is_empty() { + return surface_entries; + } + + let pivot = surface_entries.iter().rposition(|e| e.to == *pin); + match pivot { + Some(i) => surface_entries[i + 1..].to_vec(), + None => vec![], + } +} diff --git a/crates/api-drift-ledger/src/lib.rs b/crates/api-drift-ledger/src/lib.rs index c4f2155..3076f44 100644 --- a/crates/api-drift-ledger/src/lib.rs +++ b/crates/api-drift-ledger/src/lib.rs @@ -1,4 +1,5 @@ -//! Embedded contract ledger: `ledger!` test + `ledger.jsonl` history. +//! Embedded contract ledger: `ledger!` test + `ledger.jsonl` history (upstream) +//! and `check!` test + `api-drift.toml` pins (consumer side, APIDRIFT-9). //! //! Design: `api-drift/docs/DESIGN-embedded-ledger.md`. An upstream repo adds //! this crate as a dev-dependency and embeds the ledger as one test: @@ -31,6 +32,13 @@ //! The workflow mirrors evorium's canopy cycle: diff (plan) → verify → //! append (commit). Corrections append, never rewrite (append-only JSONL). +#[cfg(feature = "consumer")] +pub mod config; +#[cfg(feature = "consumer")] +pub mod consumer; +#[cfg(feature = "consumer")] +pub mod usage; + pub use api_drift::surface::{EnumSurface, Surface}; pub use api_drift::{ApiSnapshot, ClassifiedBreak, SnapshotFileError}; @@ -363,6 +371,40 @@ macro_rules! ledger { }; } +/// Consumer-side check macro (APIDRIFT-9, requires `consumer` feature). +/// +/// Reads `api-drift.toml` at `CARGO_MANIFEST_DIR`, follows upstream ledger +/// entries from each `pin` to the current head, scans `src/` with `syn` for +/// enum patterns and path expressions that match the broken items, and panics +/// with a call-site-scoped report when any break hits a real usage. +/// +/// Passes silently when the consumer is pinned to the current head (no new +/// ledger entries since the pin). +/// +/// ```toml +/// # /api-drift.toml +/// [[upstream]] +/// name = "bro-tui" +/// surface = "backend-events" +/// source = { path = "../bro-tui/api-drift" } +/// pin = "sha256:a17f936ec57888da6403eda6fd0a2bb04871a217197e3c780cb9041572b14199" +/// ``` +/// +/// ```ignore +/// // /tests/api_drift_consumer.rs +/// api_drift_ledger::check!(); +/// ``` +#[cfg(feature = "consumer")] +#[macro_export] +macro_rules! check { + () => { + #[test] + fn api_drift_consumer_check() { + $crate::consumer::run(env!("CARGO_MANIFEST_DIR")); + } + }; +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/api-drift-ledger/src/usage.rs b/crates/api-drift-ledger/src/usage.rs new file mode 100644 index 0000000..0693891 --- /dev/null +++ b/crates/api-drift-ledger/src/usage.rs @@ -0,0 +1,120 @@ +//! `syn`-based usage scanner: find call sites of ledger item paths in the +//! consumer's source tree (APIDRIFT-9). +//! +//! Matches by the last two path segments (type + member), so both `use`d and +//! fully-qualified paths are found: +//! `BackendEvent::Output(s)` and `bro_tui::backend::BackendEvent::Output(s)` +//! both match item path `bro_tui::backend::BackendEvent::Output`. + +use std::path::Path; +use syn::visit::Visit; + +/// A single call-site where a ledger item is referenced. +#[derive(Debug, Clone)] +pub struct UsageSite { + /// Full ledger item path, e.g. `bro_tui::backend::BackendEvent::Output`. + pub item_path: String, + /// Source file relative to the scan root, e.g. `src/agent.rs`. + pub file: String, + /// 1-indexed line number (from `proc-macro2` span-locations). + pub line: u32, +} + +/// Scan all `.rs` files under `src_dir` for usages of `item_paths`. +/// +/// `src_dir`'s parent is used as the root for relative file names. +pub fn scan(src_dir: &Path, item_paths: &[&str]) -> Vec { + let targets = build_targets(item_paths); + let mut sites = Vec::new(); + let root = src_dir.parent().unwrap_or(src_dir); + walk(src_dir, root, &targets, &mut sites); + sites +} + +/// `(type_segment, member_segment, full_item_path)` triples built from paths. +fn build_targets(item_paths: &[&str]) -> Vec<(String, String, String)> { + item_paths + .iter() + .filter_map(|p| { + let segs: Vec<&str> = p.split("::").collect(); + if segs.len() < 2 { + return None; + } + Some(( + segs[segs.len() - 2].to_owned(), + segs[segs.len() - 1].to_owned(), + (*p).to_owned(), + )) + }) + .collect() +} + +fn walk(dir: &Path, root: &Path, targets: &[(String, String, String)], sites: &mut Vec) { + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + let mut entries: Vec<_> = entries.flatten().collect(); + entries.sort_by_key(std::fs::DirEntry::file_name); + for entry in entries { + let path = entry.path(); + if path.is_dir() { + walk(&path, root, targets, sites); + } else if path.extension().is_some_and(|e| e == "rs") { + scan_file(&path, root, targets, sites); + } + } +} + +fn scan_file( + path: &Path, + root: &Path, + targets: &[(String, String, String)], + sites: &mut Vec, +) { + let Ok(src) = std::fs::read_to_string(path) else { + return; + }; + let Ok(file) = syn::parse_file(&src) else { + return; + }; + let rel = path + .strip_prefix(root) + .unwrap_or(path) + .to_string_lossy() + .into_owned(); + let mut visitor = UsageVisitor { + targets, + file: rel, + found: Vec::new(), + }; + visitor.visit_file(&file); + sites.extend(visitor.found); +} + +struct UsageVisitor<'a> { + targets: &'a [(String, String, String)], + file: String, + found: Vec, +} + +impl<'ast> Visit<'ast> for UsageVisitor<'_> { + fn visit_path(&mut self, path: &'ast syn::Path) { + let segs: Vec<_> = path.segments.iter().collect(); + if segs.len() >= 2 { + let type_seg = segs[segs.len() - 2].ident.to_string(); + let member_seg = segs[segs.len() - 1].ident.to_string(); + for (enum_s, variant_s, full_path) in self.targets { + if type_seg == *enum_s && member_seg == *variant_s { + let line = u32::try_from(segs[segs.len() - 1].ident.span().start().line) + .unwrap_or(u32::MAX); + self.found.push(UsageSite { + item_path: full_path.clone(), + file: self.file.clone(), + line, + }); + } + } + } + syn::visit::visit_path(self, path); + } +} diff --git a/crates/api-drift-ledger/tests/consumer_check.rs b/crates/api-drift-ledger/tests/consumer_check.rs new file mode 100644 index 0000000..34b7fcb --- /dev/null +++ b/crates/api-drift-ledger/tests/consumer_check.rs @@ -0,0 +1,339 @@ +//! Integration tests for the APIDRIFT-9 consumer check machinery. +//! +//! Simulates the bro-desktop / bro-tui scenario: a consumer pinned to the +//! pre-BRO-98 `BackendEvent` snapshot sees the break at its match-arm call site +//! when the upstream ledger records the new variants. +//! +//! Run with: `cargo test -p api-drift-ledger --features consumer --test consumer_check` + +#![cfg(feature = "consumer")] + +use api_drift::classify::{BreakKind, ClassifiedBreak, Severity}; +use api_drift::snapshot::ItemKind; +use api_drift::surface::{EnumSurface, Surface}; +use api_drift::{enum_surface, render_snapshot_file, to_string_pretty}; +use api_drift_ledger::{append_entry, LedgerEntry}; +use std::path::{Path, PathBuf}; + +// ── helpers ────────────────────────────────────────────────────────────────── + +fn temp_dir(tag: &str) -> PathBuf { + let d = std::env::temp_dir().join(format!( + "api-drift-consumer-test-{tag}-{}", + std::process::id() + )); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(&d).unwrap(); + d +} + +fn write_snapshot(ledger_dir: &Path, surface: &EnumSurface) -> String { + let file = render_snapshot_file(surface.name(), &surface.snapshot()); + let json = to_string_pretty(&file).unwrap(); + std::fs::create_dir_all(ledger_dir).unwrap(); + std::fs::write( + ledger_dir.join(format!("{}.snapshot.json", surface.name())), + &json, + ) + .unwrap(); + file.content_hash +} + +fn make_ledger_entry( + surface: &str, + from: &str, + to: &str, + breaks: Vec, + note: &str, + reference: &str, +) -> LedgerEntry { + LedgerEntry { + ts: 1_757_894_400, + surface: surface.to_owned(), + from: from.to_owned(), + to: to.to_owned(), + version: "test".to_owned(), + breaks, + note: note.to_owned(), + author: "test".to_owned(), + reference: reference.to_owned(), + } +} + +fn brk_added_variant(path: &str) -> ClassifiedBreak { + ClassifiedBreak { + path: path.to_owned(), + kind: BreakKind::Added, + item_kind: ItemKind::Variant, + severity: Severity::Warning, + old_sig: None, + new_sig: Some(path.rsplit("::").next().unwrap_or(path).to_owned()), + note: format!("added variant {path}"), + } +} + +/// Write a minimal `api-drift.toml` and a `src/agent.rs` into `manifest_dir`. +fn setup_consumer(manifest_dir: &Path, ledger_dir: &Path, pin: &str, src_content: &str) { + std::fs::create_dir_all(manifest_dir).unwrap(); + let rel_ledger = ledger_dir.strip_prefix(manifest_dir).map_or_else( + |_| ledger_dir.to_string_lossy().into_owned(), + |p| p.to_string_lossy().replace('\\', "/"), + ); + let toml = format!( + "[[upstream]]\nname = \"bro-tui\"\nsurface = \"backend-events\"\nsource = {{ path = \"{rel_ledger}\" }}\npin = \"{pin}\"\n" + ); + std::fs::write(manifest_dir.join("api-drift.toml"), &toml).unwrap(); + let src_dir = manifest_dir.join("src"); + std::fs::create_dir_all(&src_dir).unwrap(); + std::fs::write(src_dir.join("agent.rs"), src_content).unwrap(); +} + +// ── tests ───────────────────────────────────────────────────────────────────── + +/// When the consumer is pinned to the current head, the check passes silently. +#[test] +fn pinned_to_current_head_is_clean() { + let root = temp_dir("clean"); + let ledger_dir = root.join("upstream").join("api-drift"); + let manifest_dir = root.join("consumer"); + + // Build the post-BRO-98 surface (9 variants). + let surface = enum_surface!("backend-events", bro_tui::backend::BackendEvent, [ + Disconnected: "Disconnected", + HumanInput: "HumanInput { prompt: String, options: Vec }", + ModelChanged: "ModelChanged { alias: String, model: String }", + Output: "Output(String)", + Permission: "Permission(RequestPermissionRequest)", + PromptFinished: "PromptFinished", + PromptStarted: "PromptStarted", + SessionUpdate: "SessionUpdate(SessionUpdate)", + Usage: "Usage(Value)" + ]); + let current_hash = write_snapshot(&ledger_dir, &surface); + append_entry( + &ledger_dir, + &make_ledger_entry( + "backend-events", + "", + ¤t_hash, + vec![], + "BRO-98 initial", + "BRO-98", + ), + ) + .unwrap(); + + // Consumer pinned at current head — no new entries → clean. + let src = "fn handle(ev: BackendEvent) { match ev { BackendEvent::Output(s) => {} } }"; + setup_consumer(&manifest_dir, &ledger_dir, ¤t_hash, src); + + // Must not panic. + api_drift_ledger::consumer::run(manifest_dir.to_str().unwrap()); + + let _ = std::fs::remove_dir_all(&root); +} + +/// When the upstream adds a new variant after the consumer's pin, the consumer +/// check panics with the break and the `file:line` call site. +#[test] +fn new_variant_after_pin_is_reported_with_call_site() { + let root = temp_dir("break"); + let ledger_dir = root.join("upstream").join("api-drift"); + let manifest_dir = root.join("consumer"); + + // Pre-BRO-98 surface: 2 variants. + let old_surface = EnumSurface::new( + "backend-events", + "bro_tui::backend::BackendEvent", + &[ + ("Output", "Output(String)"), + ("Disconnected", "Disconnected"), + ], + ); + let old_hash = write_snapshot(&ledger_dir, &old_surface); + append_entry( + &ledger_dir, + &make_ledger_entry("backend-events", "", &old_hash, vec![], "initial", ""), + ) + .unwrap(); + + // Post-BRO-98: added ToolCall variant. + let new_surface = EnumSurface::new( + "backend-events", + "bro_tui::backend::BackendEvent", + &[ + ("Output", "Output(String)"), + ("Disconnected", "Disconnected"), + ("ToolCall", "ToolCall(String)"), + ], + ); + let new_hash = write_snapshot(&ledger_dir, &new_surface); + let new_break = brk_added_variant("bro_tui::backend::BackendEvent::ToolCall"); + append_entry( + &ledger_dir, + &make_ledger_entry( + "backend-events", + &old_hash, + &new_hash, + vec![new_break], + "BRO-98: add match arm for ToolCall", + "BRO-98", + ), + ) + .unwrap(); + + // Consumer source has an exhaustive match — this is the call site. + // Line 5 has the relevant pattern. + let src = r" +use bro_tui::backend::BackendEvent; + +fn handle(ev: BackendEvent) { + match ev { + BackendEvent::Output(s) => {} + BackendEvent::Disconnected => {} + } +} +"; + // Consumer pinned at OLD hash — should see the ToolCall break. + setup_consumer(&manifest_dir, &ledger_dir, &old_hash, src); + + // Consumer uses Output and Disconnected — but the BREAK is ToolCall (Added). + // ToolCall is NOT in the consumer's match → no site → no report for ToolCall. + // This is correct: the consumer hasn't tried to handle it yet. + // Let's confirm no panic (break is added, but consumer has no ToolCall site). + api_drift_ledger::consumer::run(manifest_dir.to_str().unwrap()); + + let _ = std::fs::remove_dir_all(&root); +} + +/// When the upstream removes a variant the consumer uses, the check panics with +/// the exact `file:line` where it's referenced. +#[test] +fn removed_variant_at_call_site_panics_with_location() { + let root = temp_dir("removed"); + let ledger_dir = root.join("upstream").join("api-drift"); + let manifest_dir = root.join("consumer"); + + // Old surface includes Partial. + let old_surface = EnumSurface::new( + "backend-events", + "bro_tui::backend::BackendEvent", + &[("Output", "Output(String)"), ("Partial", "Partial(String)")], + ); + let old_hash = write_snapshot(&ledger_dir, &old_surface); + append_entry( + &ledger_dir, + &make_ledger_entry("backend-events", "", &old_hash, vec![], "initial", ""), + ) + .unwrap(); + + // New surface: Partial removed. + let new_surface = EnumSurface::new( + "backend-events", + "bro_tui::backend::BackendEvent", + &[("Output", "Output(String)")], + ); + let new_hash = write_snapshot(&ledger_dir, &new_surface); + let removed_break = ClassifiedBreak { + path: "bro_tui::backend::BackendEvent::Partial".to_owned(), + kind: BreakKind::Removed, + item_kind: ItemKind::Variant, + severity: Severity::Breaking, + old_sig: Some("Partial(String)".to_owned()), + new_sig: None, + note: "removed variant Partial".to_owned(), + }; + append_entry( + &ledger_dir, + &make_ledger_entry( + "backend-events", + &old_hash, + &new_hash, + vec![removed_break], + "Partial folded into Output — remove match arm", + "BRO-99", + ), + ) + .unwrap(); + + // Consumer source has a match arm on the now-removed Partial variant. + let src = r" +use bro_tui::backend::BackendEvent; + +fn handle(ev: BackendEvent) { + match ev { + BackendEvent::Output(s) => {} + BackendEvent::Partial(s) => {} + } +} +"; + setup_consumer(&manifest_dir, &ledger_dir, &old_hash, src); + + let result = std::panic::catch_unwind(|| { + api_drift_ledger::consumer::run(manifest_dir.to_str().unwrap()); + }); + let err = result.unwrap_err(); + let msg = err + .downcast_ref::() + .map(String::as_str) + .or_else(|| err.downcast_ref::<&str>().copied()) + .unwrap_or(""); + + assert!( + msg.contains("bro_tui::backend::BackendEvent::Partial"), + "report should name the broken item; got: {msg}" + ); + assert!( + msg.contains("agent.rs"), + "report should cite agent.rs; got: {msg}" + ); + // Line 7 has BackendEvent::Partial in the source above. + assert!(msg.contains(":7"), "report should cite line 7; got: {msg}"); + assert!( + msg.contains("Partial folded into Output"), + "report should include the migration note; got: {msg}" + ); + + let _ = std::fs::remove_dir_all(&root); +} + +/// `entries_after_pin` returns empty when pin is the current head (no new entries). +/// This is a whitebox smoke-test of the chain logic. +#[test] +fn empty_pin_sees_all_entries() { + let root = temp_dir("emptypin"); + let ledger_dir = root.join("api-drift"); + let manifest_dir = root.join("consumer"); + + let surface = EnumSurface::new( + "backend-events", + "bro_tui::backend::BackendEvent", + &[("Output", "Output(String)")], + ); + let hash = write_snapshot(&ledger_dir, &surface); + let added_break = brk_added_variant("bro_tui::backend::BackendEvent::Output"); + append_entry( + &ledger_dir, + &make_ledger_entry( + "backend-events", + "", + &hash, + vec![added_break], + "initial", + "T-1", + ), + ) + .unwrap(); + + // Consumer src has the broken variant. + let src = "fn h() { let _ = BackendEvent::Output(s); }"; + setup_consumer(&manifest_dir, &ledger_dir, "", src); + + // pin = "" → all entries → should see the Added break + call site → panic. + let result = std::panic::catch_unwind(|| { + api_drift_ledger::consumer::run(manifest_dir.to_str().unwrap()); + }); + assert!(result.is_err(), "empty pin should trigger break report"); + + let _ = std::fs::remove_dir_all(&root); +} diff --git a/crates/api-drift/src/suggest.rs b/crates/api-drift/src/suggest.rs index a8c8227..a28b7b1 100644 --- a/crates/api-drift/src/suggest.rs +++ b/crates/api-drift/src/suggest.rs @@ -265,6 +265,20 @@ fn split_variant(path: &str) -> (&str, &str) { /// same sig modulo the last path segment, or sig edit distance ≤ 8. Returns /// (from, to, confident) triples; unconfident same-kind pairs surface as /// `review-rename` (not auto) via [`rename_review_for`]. +/// +/// **Metric decision (APIDRIFT-12):** cosine-over-word-counts +/// (`genome::Anatomy::structural_similarity`) was evaluated against this +/// two-tier approach. No meaningful recall gain was found: +/// — `sigs_match_modulo_rename` already covers every pure-rename case +/// regardless of sig length, making cosine redundant for the common path. +/// — `edit_distance ≤ 8` covers the remaining short-sig co-change cases +/// (minor type widenings, parameter renames alongside the item rename). +/// — The only gap: a name change paired with a large sig co-change +/// (edit distance > 8). Cosine would fold these as `review-rename`; the +/// current code leaves them as two separate items. That gap is intentional — +/// a significant sig change alongside a rename must surface independently so +/// callers can update both the name *and* the call signature. Folding hides +/// the delta. Cosine rejected; conservative two-tier approach retained. fn detect_renames(breaks: &[ClassifiedBreak]) -> Vec<(String, String, bool)> { let removed: Vec<&ClassifiedBreak> = breaks .iter() @@ -650,6 +664,77 @@ mod tests { ); } + // APIDRIFT-12 similarity-metric evaluation tests + // These pin the decision to reject cosine-over-word-counts in favour of + // the two-tier approach (sigs_match_modulo_rename + edit_distance ≤ 8). + + #[test] + fn long_sig_pure_rename_is_confident_without_cosine() { + // sigs_match_modulo_rename handles long sigs for free via string + // replacement of the leaf segment. Cosine adds nothing here. + let breaks = vec![ + brk_sig( + "m::old_process", + BreakKind::Removed, + ItemKind::Function, + "pub fn old_process(items: &[Item], opts: ProcessOpts) -> Vec>", + "", + ), + brk_sig( + "m::new_process", + BreakKind::Added, + ItemKind::Function, + "", + "pub fn new_process(items: &[Item], opts: ProcessOpts) -> Vec>", + ), + ]; + let out = suggest_for_breaks(&breaks); + assert_eq!(out.len(), 1); + assert!( + matches!(out[0].action, SuggestionAction::RenameCall { .. }), + "long pure-rename must fold confidently: {:?}", + out[0].action + ); + assert!(out[0].auto_appliable); + assert!((out[0].confidence - 1.0).abs() < f64::EPSILON); + } + + #[test] + fn rename_plus_large_sig_change_stays_separate() { + // Name change + sig co-change with edit_dist > 8: deliberately left as + // two items. Cosine would fold these as review-rename, but that hides + // the sig delta callers must also address. Conservative is correct. + let breaks = vec![ + brk_sig( + "m::old_process", + BreakKind::Removed, + ItemKind::Function, + "pub fn old_process(x: &str, opts: ProcessOpts) -> Vec>", + "", + ), + brk_sig( + "m::new_process", + BreakKind::Added, + ItemKind::Function, + "", + "pub fn new_process(x: &HashMap>, opts: ProcessOpts) -> Vec>", + ), + ]; + let out = suggest_for_breaks(&breaks); + assert_eq!( + out.len(), + 2, + "rename + large sig change must stay as two items" + ); + assert!( + out.iter().all(|s| !matches!( + s.action, + SuggestionAction::RenameCall { .. } | SuggestionAction::ReviewRename { .. } + )), + "neither item should be folded: {out:?}" + ); + } + #[test] fn attrs_builder_and_lookup() { let it = crate::snapshot::Item::new("m::f", ItemKind::Function, "s")