diff --git a/crates/op-host-services/src/cli_model_discovery.rs b/crates/op-host-services/src/cli_model_discovery.rs index b88f37eea..8c36fb232 100644 --- a/crates/op-host-services/src/cli_model_discovery.rs +++ b/crates/op-host-services/src/cli_model_discovery.rs @@ -20,14 +20,29 @@ use crate::cli_probe_error::CliProbeError; use crate::cli_probe_support::{bounded_cli_output, diagnose_timeout, BoundedProbe}; use crate::model_discovery::resolve_cli; +/// `agy models` parsing. Split off at the 800-line cap; re-exported below +/// so `cli_model_discovery::parse_antigravity_models` stays the import path. +#[path = "cli_model_discovery_antigravity.rs"] +mod antigravity; + +pub use antigravity::{note_antigravity_catalog_version, parse_antigravity_models}; + +#[cfg(test)] +use antigravity::{catalog_format_code, catalog_shape_change}; + /// Matches `cli_provider_probe::MODELS_PROBE_TIMEOUT`: every query in this /// module is a `models` call, and a `models` call is a network round trip. /// Ten seconds cut off `agy`'s own error (11.04 s without a proxy) a beat /// before it arrived. const MODEL_QUERY_TIMEOUT: Duration = Duration::from_secs(20); -/// Query the installed Antigravity catalog. `agy models` may print display -/// names or kebab-case model IDs; both are accepted verbatim by `--model`. +/// Query the installed Antigravity catalog. `agy models` has printed three +/// different shapes across releases — display names, one column of slugs, +/// and today's `iddisplay name` — all of which +/// [`parse_antigravity_models`] accepts while keeping the id column apart +/// from the label. Feeding a whole two-column row back as `--model` is +/// exactly the "model … is not recognized" failure this module exists to +/// avoid. pub fn query_antigravity_models() -> Result, CliProbeError> { let exe = resolve_cli("agy").ok_or(CliProbeError::NotFound { provider: "Antigravity", @@ -92,106 +107,8 @@ pub fn antigravity_default_model() -> Vec { )] } -/// Parse `agy models`. Accept display names, kebab-case IDs, and JSON catalogs -/// so the integration survives CLI output-format changes. -pub fn parse_antigravity_models(raw: &str) -> Vec { - let mut names = BTreeSet::new(); - if let Ok(value) = serde_json::from_str::(raw) { - // A catalog is either a top-level array or lives under one of the - // documented catalog wrapper fields. Do not mine arbitrary top-level - // `name` / `id` diagnostics for model-looking strings. - collect_antigravity_names(&value, &mut names, value.is_array()); - } - - let mut in_catalog = false; - let mut catalog_ended = false; - let mut saw_catalog_entry = false; - for line in raw.lines() { - let clean = strip_ansi(line); - let clean = clean.trim(); - if clean.is_empty() { - if in_catalog && saw_catalog_entry { - in_catalog = false; - catalog_ended = true; - } - continue; - } - let lower = clean.to_ascii_lowercase(); - if lower.contains("available models") || lower == "models:" { - in_catalog = true; - catalog_ended = false; - saw_catalog_entry = false; - continue; - } - let (candidate, was_bullet) = trim_catalog_bullet(clean); - if candidate.is_empty() || is_catalog_diagnostic(candidate) { - if in_catalog && saw_catalog_entry { - in_catalog = false; - catalog_ended = true; - } - continue; - } - let unheaded_bullet = was_bullet && !catalog_ended; - let unheaded_model = !catalog_ended && looks_like_antigravity_model(candidate); - if (in_catalog || unheaded_bullet || unheaded_model) - && looks_like_antigravity_model(candidate) - { - names.insert(candidate.to_string()); - if in_catalog || unheaded_bullet { - saw_catalog_entry = true; - } - } else if in_catalog && saw_catalog_entry { - in_catalog = false; - catalog_ended = true; - } - } - - names - .into_iter() - .map(|name| ModelEntry::new(AgentProvider::Antigravity, name.clone(), name)) - .collect() -} - -fn collect_antigravity_names( - value: &serde_json::Value, - out: &mut BTreeSet, - catalog_context: bool, -) { - match value { - serde_json::Value::String(name) - if catalog_context && looks_like_antigravity_model(name) => - { - out.insert(name.trim().to_string()); - } - serde_json::Value::Array(values) => { - for value in values { - collect_antigravity_names(value, out, true); - } - } - serde_json::Value::Object(map) => { - for (key, value) in map { - if catalog_context - && matches!( - key.as_str(), - "id" | "model" | "name" | "displayName" | "display_name" - ) - { - if let Some(name) = value - .as_str() - .map(str::trim) - .filter(|name| looks_like_antigravity_model(name)) - { - out.insert(name.to_string()); - } - } else if matches!(key.as_str(), "models" | "data" | "result" | "catalog") { - collect_antigravity_names(value, out, true); - } - } - } - _ => {} - } -} - +/// Shared by both catalogs: a leading list bullet is decoration, not part +/// of the value behind it. fn trim_catalog_bullet(line: &str) -> (&str, bool) { let trimmed = line.trim_start(); for bullet in ["* ", "- ", "• ", "● "] { @@ -202,54 +119,6 @@ fn trim_catalog_bullet(line: &str) -> (&str, bool) { (trimmed.trim(), false) } -fn looks_like_antigravity_model(value: &str) -> bool { - let lower = value.trim().to_ascii_lowercase(); - !is_catalog_diagnostic(&lower) - && [ - "gemini ", - "gemini-", - "claude ", - "claude-", - "gpt-", - "gpt ", - "gemma ", - "deepseek ", - "grok ", - "qwen ", - ] - .iter() - .any(|prefix| lower.starts_with(prefix)) -} - -fn is_catalog_diagnostic(value: &str) -> bool { - let lower = value.trim().to_ascii_lowercase(); - [ - "sign in", - "signin", - "log in", - "login", - "authenticate", - "authentication", - "unauthorized", - "credential", - "api key", - "required", - "unavailable", - "failed", - "failure", - "error:", - "no models", - "loading", - "checking", - "timed out", - "troubleshoot", - "documentation", - "release notes", - ] - .iter() - .any(|marker| lower.contains(marker)) -} - /// Query the real Grok Build model catalog. The command is bounded because /// model discovery runs during startup and must never strand its worker. pub fn query_grok_models() -> Result, CliProbeError> { @@ -477,6 +346,14 @@ fn first_grok_catalog_column(row: &str) -> Option<&str> { .map(str::trim) .find(|column| !column.is_empty()); } + // A tab is always a column boundary. `grok models` prints bullet rows + // today (verified: `Available models:` / ` * grok-4.5 (default)`), so + // this is defensive — but it is the shape that broke the Antigravity + // parser, and here the single-whitespace scan below would silently drop + // the whole row rather than mis-read it. + if let Some(gap) = row.find('\t') { + return Some(row[..gap].trim()); + } let bytes = row.as_bytes(); let mut index = 0; diff --git a/crates/op-host-services/src/cli_model_discovery_antigravity.rs b/crates/op-host-services/src/cli_model_discovery_antigravity.rs new file mode 100644 index 000000000..f9e0ec27f --- /dev/null +++ b/crates/op-host-services/src/cli_model_discovery_antigravity.rs @@ -0,0 +1,416 @@ +//! `agy models` catalog parsing. +//! +//! Split out of `cli_model_discovery.rs` to keep that file under the +//! repository's 800-line cap. The Antigravity half is the larger one +//! because upstream has changed the output format three times and each +//! generation is still a live input; the spine keeps the query/discover +//! surface and the helpers Grok shares (`trim_catalog_bullet`, +//! `strip_ansi`). + +use std::collections::BTreeMap; +use std::sync::{LazyLock, Mutex}; + +use op_ai::agent_settings_state::AgentProvider; +use op_ai::chat_models::ModelEntry; + +use super::{strip_ansi, trim_catalog_bullet}; + +/// Parse `agy models`. +/// +/// **Upstream has changed this format three times, so all three are live +/// inputs** (see the version-named fixtures in the test sibling): +/// +/// | `agy` | shape | +/// | ---------- | ------------------------------ | +/// | pre-1.1.5 | display names | +/// | 1.1.5 | one column of kebab-case slugs | +/// | 1.1.11 | `iddisplay name` | +/// +/// JSON catalogs are accepted too. The returned `ModelEntry.value` is what +/// gets handed to `agy --model`, so a row's id column must never carry its +/// display column along: `agy` rejects +/// `"gemini-3.6-flash-high\tGemini 3.6 Flash (High)"` with +/// `invalid model selection`, and the failure reaches the user as a bare +/// `CLI exited with status 1`. +pub fn parse_antigravity_models(raw: &str) -> Vec { + // A rejected `--model` prints its own `Available models:` block. That + // block is a DIAGNOSTIC, not a catalog query — it lists display names + // with no id column and describes a run that failed. Catalogs come + // from `agy models` stdout and nowhere else, so refuse to mine one out + // of an error. (Both callers already pass only `agy models` stdout and + // short-circuit on a non-zero exit; this keeps that true by content as + // well as by call site.) + if looks_like_model_rejection(raw) { + return Vec::new(); + } + remember_catalog_format(catalog_format_code(raw)); + // id -> display label. Keyed by id because that is what identifies a + // model to `--model`; two rows sharing an id are the same model. + let mut names: BTreeMap = BTreeMap::new(); + if let Ok(value) = serde_json::from_str::(raw) { + // A catalog is either a top-level array or lives under one of the + // documented catalog wrapper fields. Do not mine arbitrary top-level + // `name` / `id` diagnostics for model-looking strings. + collect_antigravity_names(&value, &mut names, value.is_array()); + } + + let mut in_catalog = false; + let mut catalog_ended = false; + let mut saw_catalog_entry = false; + for line in raw.lines() { + let clean = strip_ansi(line); + let clean = clean.trim(); + if clean.is_empty() { + if in_catalog && saw_catalog_entry { + in_catalog = false; + catalog_ended = true; + } + continue; + } + let lower = clean.to_ascii_lowercase(); + if lower.contains("available models") || lower == "models:" { + in_catalog = true; + catalog_ended = false; + saw_catalog_entry = false; + continue; + } + let (candidate, was_bullet) = trim_catalog_bullet(clean); + if candidate.is_empty() || is_catalog_diagnostic(candidate) { + if in_catalog && saw_catalog_entry { + in_catalog = false; + catalog_ended = true; + } + continue; + } + let (id, label) = split_antigravity_row(candidate); + let unheaded_bullet = was_bullet && !catalog_ended; + let unheaded_model = !catalog_ended && looks_like_antigravity_model(id); + if (in_catalog || unheaded_bullet || unheaded_model) && looks_like_antigravity_model(id) { + names.insert(id.to_string(), label.to_string()); + if in_catalog || unheaded_bullet { + saw_catalog_entry = true; + } + } else if in_catalog && saw_catalog_entry { + in_catalog = false; + catalog_ended = true; + } + } + + let total = names.len(); + let models: Vec = names + .into_iter() + .filter(|(id, _)| is_usable_model_id(id)) + .map(|(id, label)| ModelEntry::new(AgentProvider::Antigravity, id, label)) + .collect(); + warn_on_dropped_ids("Antigravity", "agy models", total, models.len()); + models +} + +/// Which of the known `agy models` layouts this output is, as a stable code +/// for the log. Coarse on purpose — it answers "did the shape change?", not +/// "is it valid?", which is what [`is_usable_model_id`] is for. +pub(super) fn catalog_format_code(raw: &str) -> &'static str { + if raw.trim().is_empty() { + return "empty"; + } + if serde_json::from_str::(raw).is_ok() { + return "json"; + } + let rows: Vec<&str> = raw + .lines() + .map(str::trim) + .filter(|row| !row.is_empty()) + .collect(); + if rows.iter().any(|row| row.contains('\t')) { + // 1.1.11 + "two-column-tsv" + } else if rows.iter().all(|row| is_model_slug(row)) { + // 1.1.5 + "slug-column" + } else { + // pre-1.1.5 + "display-names" + } +} + +/// The layout the last parse saw, so the version probe can name it without +/// re-reading the catalog. Written on every parse; read only when a connect +/// probe reports a version. +static LAST_PARSED_FORMAT: LazyLock>> = + LazyLock::new(|| Mutex::new(None)); + +/// The `(version, format)` pair already written to the log, so a steady +/// state stays silent and only a CHANGE prints. +static LAST_LOGGED_SHAPE: LazyLock>> = + LazyLock::new(|| Mutex::new(None)); + +fn remember_catalog_format(format: &'static str) { + *lock(&LAST_PARSED_FORMAT) = Some(format); +} + +/// Record which `agy` version produced the catalog we just parsed, and log +/// the pair when it differs from the last pair logged. +/// +/// Motivation, from the 1.1.11 incident: reconstructing "the binary updated +/// at 11:03, the first user report came at 16:19" took reading the +/// executable's mtime and guessing from git history. That line belongs in +/// the log. +/// +/// `version` is the string the connect probe ALREADY fetched — this spends +/// no subprocess of its own — and the whole call is informational: it +/// returns nothing and blocks nothing. +pub fn note_antigravity_catalog_version(version: &str) { + let Some(format) = *lock(&LAST_PARSED_FORMAT) else { + return; + }; + note_catalog_shape(version, format); +} + +/// Emit the breadcrumb if there is one to emit. +fn note_catalog_shape(version: &str, format: &'static str) { + if let Some(line) = catalog_shape_change(version, format) { + eprintln!("{line}"); + } +} + +/// The change detector: returns the line to log when `(version, format)` +/// differs from the pair already logged, and `None` when it does not. +/// +/// Returning the line instead of printing it is what makes "stays quiet on +/// an unchanged pair" observable — asserting on the stored pair cannot see +/// the difference, because a redundant write stores the same value. +/// +/// The layout is passed in rather than read from the parse global so a test +/// can drive this without racing every sibling that calls +/// [`parse_antigravity_models`]. +pub(super) fn catalog_shape_change(version: &str, format: &'static str) -> Option { + let seen = (version.trim().to_string(), format); + let mut logged = lock(&LAST_LOGGED_SHAPE); + if logged.as_ref() == Some(&seen) { + return None; + } + let line = match logged.as_ref() { + Some((was_version, was_format)) => format!( + "[agents] Antigravity: `agy {}` prints a {} model catalog \ + (was `agy {was_version}` / {was_format})", + seen.0, seen.1 + ), + None => format!( + "[agents] Antigravity: `agy {}` prints a {} model catalog", + seen.0, seen.1 + ), + }; + *logged = Some(seen); + Some(line) +} + +/// Lock helper that ignores poisoning: a panicking sibling must not turn +/// this bookkeeping into a second failure. +fn lock(cell: &LazyLock>) -> std::sync::MutexGuard<'_, T> { + cell.lock().unwrap_or_else(|poison| poison.into_inner()) +} + +/// Whether the raw text is `agy` complaining about a `--model` value rather +/// than answering a catalog query. Both markers come from the real message: +/// `Error: invalid model selection (--model "…" --effort ""): model … is not +/// recognized as a known model or custom model in settings`. +fn looks_like_model_rejection(raw: &str) -> bool { + let lower = raw.to_ascii_lowercase(); + lower.contains("invalid model selection") + || lower.contains("is not recognized as a known model") +} + +/// Shape self-check for a value about to be handed to `--model`. +/// +/// This is the tripwire for the NEXT format change, and it is deliberately +/// not "contains no whitespace": pre-1.1.5 `agy` listed display names, and +/// `agy --model "Gemini 3.6 Flash (High)"` still answers normally, so a +/// space is legitimate. What is never legitimate is a column separator or a +/// control character inside a single wire value — that only happens when a +/// row was consumed whole instead of being split, which is exactly how the +/// 1.1.11 change broke us. +fn is_usable_model_id(id: &str) -> bool { + !id.is_empty() + && id.len() <= 128 + && !id.chars().any(|c| c == '\t' || c.is_control()) + && id.trim() == id +} + +/// Report ids the shape check refused, so an unrecognised format leaves a +/// breadcrumb at probe time instead of waiting for a user to hit +/// `CLI exited with status 1` on a real generation. +/// +/// Dropping every id collapses to the existing empty-catalog path +/// (`UnrecognizedCatalog` → the provider's default model), so the fallback +/// is the one already in place rather than a new one. +fn warn_on_dropped_ids(provider: &str, command: &str, total: usize, kept: usize) { + if kept < total { + eprintln!( + "[agents] {provider}: dropped {} of {total} model id(s) from `{command}` — \ + an id carried a column separator or control character, which means the \ + CLI's output format changed and the parser needs updating", + total - kept + ); + } +} + +/// Split one catalog row into `(id, display label)`. +/// +/// `agy models` (verified against 1.1.x with `od -c`) prints +/// `gemini-3.6-flash-high\tGemini 3.6 Flash (High)`. Treating the whole row +/// as the id is what made every generation fail: `agy` answers +/// `model … is not recognized as a known model or custom model in settings` +/// and exits 1. +/// +/// Rows with no id column — the bare-slug catalog older builds printed, and +/// the display-name list `agy` prints when it rejects a `--model` value — +/// degrade to `(row, row)`. That is correct rather than a second bug: +/// `agy --model "Gemini 3.6 Flash (High)"` runs normally (verified by +/// running it), so a display name is a usable `--model` value. +fn split_antigravity_row(row: &str) -> (&str, &str) { + if let Some((id, label)) = row.split_once('\t') { + let (id, label) = (id.trim(), label.trim()); + if !id.is_empty() && !label.is_empty() { + return (id, label); + } + } + // Space-aligned columns, gated on the left cell being slug-shaped: a + // display name padded out to a column width must stay one value, or + // `Gemini 3.5 Flash (Medium)` would lose its effort suffix. + if let Some((id, label)) = split_on_column_gap(row) { + if is_model_slug(id) && !label.is_empty() { + return (id, label); + } + } + (row, row) +} + +/// Split at the first run of two or more spaces — the conventional +/// column gap in a human-formatted table. +fn split_on_column_gap(row: &str) -> Option<(&str, &str)> { + let bytes = row.as_bytes(); + let gap = (0..bytes.len().saturating_sub(1)) + .find(|index| bytes[*index] == b' ' && bytes[index + 1] == b' ')?; + let (left, right) = row.split_at(gap); + Some((left.trim(), right.trim())) +} + +/// Whether a string is shaped like a wire model id rather than a human +/// label: no whitespace, and only the punctuation model ids use. +fn is_model_slug(value: &str) -> bool { + !value.is_empty() + && value + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '-' | '_' | '.' | '/' | ':')) +} + +fn collect_antigravity_names( + value: &serde_json::Value, + out: &mut BTreeMap, + catalog_context: bool, +) { + match value { + serde_json::Value::String(name) + if catalog_context && looks_like_antigravity_model(name) => + { + let name = name.trim().to_string(); + out.insert(name.clone(), name); + } + serde_json::Value::Array(values) => { + for value in values { + collect_antigravity_names(value, out, true); + } + } + serde_json::Value::Object(map) => { + // One object is one model, so its id and its display name are + // read together. Reading each key independently used to emit + // `{"id": …, "displayName": …}` as two separate picker rows. + if catalog_context { + if let Some((id, label)) = antigravity_object_entry(map) { + out.insert(id, label); + } + } + for (key, value) in map { + if matches!(key.as_str(), "models" | "data" | "result" | "catalog") { + collect_antigravity_names(value, out, true); + } + } + } + _ => {} + } +} + +/// Read one catalog object's `(id, display label)` pair. An object carrying +/// only a display name yields it as both, matching the text path. +fn antigravity_object_entry( + map: &serde_json::Map, +) -> Option<(String, String)> { + let field = |keys: &[&str]| -> Option { + keys.iter() + .filter_map(|key| map.get(*key)) + .filter_map(serde_json::Value::as_str) + .map(str::trim) + .find(|value| !value.is_empty()) + .map(str::to_string) + }; + let label = field(&["displayName", "display_name", "name"]) + .filter(|label| looks_like_antigravity_model(label)); + let id = field(&["id", "model"]).filter(|id| looks_like_antigravity_model(id)); + // An id that is not model-shaped falls back to the label rather than + // discarding the object, which is what the per-key walk this replaced + // did for `{"id": "x1", "displayName": "Gemini …"}`. + match (id, label) { + (Some(id), Some(label)) => Some((id, label)), + (Some(id), None) => Some((id.clone(), id)), + (None, Some(label)) => Some((label.clone(), label)), + (None, None) => None, + } +} + +fn looks_like_antigravity_model(value: &str) -> bool { + let lower = value.trim().to_ascii_lowercase(); + !is_catalog_diagnostic(&lower) + && [ + "gemini ", + "gemini-", + "claude ", + "claude-", + "gpt-", + "gpt ", + "gemma ", + "deepseek ", + "grok ", + "qwen ", + ] + .iter() + .any(|prefix| lower.starts_with(prefix)) +} + +fn is_catalog_diagnostic(value: &str) -> bool { + let lower = value.trim().to_ascii_lowercase(); + [ + "sign in", + "signin", + "log in", + "login", + "authenticate", + "authentication", + "unauthorized", + "credential", + "api key", + "required", + "unavailable", + "failed", + "failure", + "error:", + "no models", + "loading", + "checking", + "timed out", + "troubleshoot", + "documentation", + "release notes", + ] + .iter() + .any(|marker| lower.contains(marker)) +} diff --git a/crates/op-host-services/src/cli_model_discovery_tests.rs b/crates/op-host-services/src/cli_model_discovery_tests.rs index baeed6591..1844150f7 100644 --- a/crates/op-host-services/src/cli_model_discovery_tests.rs +++ b/crates/op-host-services/src/cli_model_discovery_tests.rs @@ -94,6 +94,243 @@ fn parses_antigravity_display_names_without_losing_effort_suffixes() { assert!(models.iter().all(|model| model.value == model.display_name)); } +// --------------------------------------------------------------------- +// `agy models` output-format archive. +// +// Upstream has changed this format three times, and each change silently +// broke the parser until a user hit it. One version-named test per format, +// each against a REAL captured fixture: +// +// pre-1.1.5 display names `parses_antigravity_display_names_…` +// 1.1.5 one column of slugs `parses_antigravity_v1_1_5_slug_catalog` +// 1.1.11 `iddisplay` `parses_antigravity_v1_1_11_two_column_catalog` +// +// All three are still live inputs — users run whatever `agy` auto-updated +// them to. When it changes again: capture the real output, add the next +// version-named test, and leave the older ones alone. +// --------------------------------------------------------------------- + +/// The real `agy models` stdout, captured 2026-08-07 from `agy` 1.1.11 and +/// verified byte-for-byte against the live command: one row per model, +/// `iddisplay name`, no heading. The tabs ARE the fixture — the +/// pre-existing samples in this file were hand-written without them, which +/// is why every test here passed while the shipped parser handed `agy` a +/// `--model` value with a tab and a display name embedded in it. +const AGY_MODELS_TSV: &str = concat!( + "gemini-3.6-flash-high\tGemini 3.6 Flash (High)\n", + "gemini-3.6-flash-medium\tGemini 3.6 Flash (Medium)\n", + "gemini-3.6-flash-low\tGemini 3.6 Flash (Low)\n", + "gemini-3.5-flash-high\tGemini 3.5 Flash (High)\n", + "gemini-3.5-flash-medium\tGemini 3.5 Flash (Medium)\n", + "gemini-3.5-flash-low\tGemini 3.5 Flash (Low)\n", + "gemini-3.1-pro-high\tGemini 3.1 Pro (High)\n", + "gemini-3.1-pro-low\tGemini 3.1 Pro (Low)\n", + "claude-sonnet-4-6\tClaude Sonnet 4.6 (Thinking)\n", + "claude-opus-4-6-thinking\tClaude Opus 4.6 (Thinking)\n", + "gpt-oss-120b-medium\tGPT-OSS 120B (Medium)\n", +); + +/// The block `agy` prints on stderr when it rejects a `--model` value, +/// captured the same day by running `agy --model definitely-not-a-model`. +/// It lists DISPLAY NAMES and no id column at all. +const AGY_REJECTED_MODEL_BLOCK: &str = concat!( + "Error: invalid model selection (--model \"definitely-not-a-model\" --effort \"\"): ", + "model definitely-not-a-model is not recognized as a known model or custom model in settings\n", + "Available models:\n", + " Gemini 3.6 Flash (High)\n", + " Gemini 3.5 Flash (Medium)\n", + " Claude Opus 4.6 (Thinking)\n", + " GPT-OSS 120B (Medium)\n", +); + +/// `agy` 1.1.11 (auto-updated 2026-08-07, ~5 hours before the first user +/// report): two columns. The 1.1.5 adaptation kept whole rows, so `--model` +/// received `"gemini-3.6-flash-high\tGemini 3.6 Flash (High)"`. +#[test] +fn parses_antigravity_v1_1_11_two_column_catalog() { + let models = parse_antigravity_models(AGY_MODELS_TSV); + assert_eq!(models.len(), 11, "{models:#?}"); + + // The exact defect: a `--model` value that carries its own label. + for model in &models { + assert!( + !model.value.contains('\t') && !model.value.contains(' '), + "id absorbed the display column: {:?}", + model.value + ); + } + + let pairs: Vec<(&str, &str)> = models + .iter() + .map(|model| (model.value.as_str(), model.display_name.as_str())) + .collect(); + assert!( + pairs.contains(&("gemini-3.6-flash-high", "Gemini 3.6 Flash (High)")), + "{pairs:#?}" + ); + assert!( + pairs.contains(&("claude-opus-4-6-thinking", "Claude Opus 4.6 (Thinking)")), + "{pairs:#?}" + ); + assert!( + pairs.contains(&("gpt-oss-120b-medium", "GPT-OSS 120B (Medium)")), + "{pairs:#?}" + ); +} + +/// A rejected `--model` prints its own `Available models:` block. It +/// describes a run that FAILED, so it is a diagnostic and never a catalog — +/// even though its rows would parse, and even though those display names +/// happen to be usable `--model` values (`agy --model "Gemini 3.6 Flash +/// (High)"` answers normally; measured). +/// +/// This is why "no id column ⇒ not a model id" would be the wrong rule: +/// pre-1.1.5 `agy models` legitimately printed display names and nothing +/// else. The distinction that holds is the SOURCE, not the shape. +#[test] +fn model_rejection_block_is_a_diagnostic_not_a_catalog() { + assert!( + parse_antigravity_models(AGY_REJECTED_MODEL_BLOCK).is_empty(), + "an error block must not populate the model picker" + ); + // …and a display-name catalog with no error in it still parses, so the + // pre-1.1.5 format is not collateral damage. (Covered in full by + // `parses_antigravity_display_names_without_losing_effort_suffixes`.) + assert_eq!( + parse_antigravity_models("Available models:\n Gemini 3.6 Flash (High)").len(), + 1 + ); +} + +/// Reaching this block means `agy models` succeeded but printed something +/// we could not split, so the honest outcome is the existing +/// unrecognized-catalog fallback rather than ids that will fail later. +#[test] +fn a_rejection_block_on_stdout_reports_an_unrecognized_catalog() { + let error = require_antigravity_models(AGY_REJECTED_MODEL_BLOCK, "") + .unwrap_err() + .to_string(); + assert!(error.contains("unrecognized model catalog"), "{error}"); +} + +/// The tripwire for the NEXT format change: an id that could not possibly +/// be a single wire value is dropped instead of being handed to `--model`. +/// Reachable through the JSON path, which takes its id verbatim. +#[test] +fn ids_carrying_a_column_separator_are_dropped_rather_than_handed_to_the_cli() { + let models = parse_antigravity_models( + r#"{"models":[{"id":"gemini-3.6-flash-high\tGemini 3.6 Flash (High)"}, + {"id":"gemini-3.5-flash-low"}]}"#, + ); + assert_eq!( + models + .iter() + .map(|model| model.value.as_str()) + .collect::>(), + ["gemini-3.5-flash-low"], + "a spliced id must never survive to the wire" + ); + + // Dropping every id degrades to the catalog error the callers already + // handle by falling back to the provider default. + assert!(parse_antigravity_models( + "{\"models\":[{\"id\":\"gemini-3.6-flash-high\\tGemini 3.6 Flash (High)\"}]}" + ) + .is_empty()); +} + +#[test] +fn space_aligned_columns_split_only_when_the_left_cell_is_a_slug() { + let models = parse_antigravity_models("gemini-3.1-pro-high Gemini 3.1 Pro (High)"); + assert_eq!(models.len(), 1, "{models:#?}"); + assert_eq!(models[0].value, "gemini-3.1-pro-high"); + assert_eq!(models[0].display_name, "Gemini 3.1 Pro (High)"); + + // A padded display name is one value, not an id plus its effort suffix. + let models = parse_antigravity_models("Available models:\n* Gemini 3.5 Flash (Medium)"); + assert_eq!(models.len(), 1, "{models:#?}"); + assert_eq!(models[0].value, "Gemini 3.5 Flash (Medium)"); +} + +#[test] +fn json_object_carrying_both_columns_is_one_model_not_two() { + let models = parse_antigravity_models( + r#"{"models":[{"id":"gemini-3.6-flash-high","displayName":"Gemini 3.6 Flash (High)"}]}"#, + ); + assert_eq!(models.len(), 1, "{models:#?}"); + assert_eq!(models[0].value, "gemini-3.6-flash-high"); + assert_eq!(models[0].display_name, "Gemini 3.6 Flash (High)"); +} + +/// The three live layouts must classify apart, or the version/format +/// breadcrumb cannot report a change. Pure function, no globals touched. +#[test] +fn each_known_catalog_layout_gets_its_own_format_code() { + assert_eq!(catalog_format_code(AGY_MODELS_TSV), "two-column-tsv"); + assert_eq!( + catalog_format_code("gemini-3.6-flash-high\ngemini-3.1-pro-low"), + "slug-column" + ); + assert_eq!( + catalog_format_code("Available models:\n Gemini 3.6 Flash (High)"), + "display-names" + ); + assert_eq!(catalog_format_code(r#"{"models":[]}"#), "json"); + assert_eq!(catalog_format_code(" \n\n"), "empty"); +} + +/// The breadcrumb is a CHANGE log, not a per-startup line: the same +/// version + layout prints once and then stays quiet. +/// +/// Driven through `catalog_shape_change`, which RETURNS the line it would +/// log: asserting on the stored pair instead would not catch a detector +/// that logs unconditionally, because a redundant write stores the same +/// value. (Measured — an always-log injection passed that weaker test.) +/// `LAST_LOGGED_SHAPE` is touched by nothing else in this binary. +#[test] +fn the_version_breadcrumb_only_reports_a_change() { + let first = catalog_shape_change("1.1.11", "two-column-tsv").expect("first sighting must log"); + assert!(first.contains("1.1.11"), "{first}"); + assert!(first.contains("two-column-tsv"), "{first}"); + + assert!( + catalog_shape_change("1.1.11", "two-column-tsv").is_none(), + "an unchanged version + layout must stay silent" + ); + + let bumped = catalog_shape_change("1.1.12", "two-column-tsv") + .expect("a version bump must log even when the layout held"); + // The line carries both sides, so the log dates the change by itself. + assert!( + bumped.contains("1.1.12") && bumped.contains("1.1.11"), + "{bumped}" + ); + + let reshaped = catalog_shape_change("1.1.12", "slug-column") + .expect("a layout change must log even when the version held"); + assert!( + reshaped.contains("slug-column") && reshaped.contains("two-column-tsv"), + "{reshaped}" + ); +} + +/// `grok models` prints bullet rows today, so this pins the defensive +/// branch: were it ever to switch to the tab layout `agy` uses, the rows +/// must yield ids rather than being dropped whole. +#[test] +fn grok_tab_separated_rows_keep_only_the_id_column() { + let models = parse_grok_models( + "Available models:\ngrok-4.5\tGrok 4.5\ngrok-code-fast-1\tGrok Code Fast 1", + ); + assert_eq!( + models + .iter() + .map(|model| model.value.as_str()) + .collect::>(), + ["grok-4.5", "grok-code-fast-1"] + ); +} + #[test] fn parses_antigravity_v1_1_5_slug_catalog() { let text = "gemini-3.6-flash-high\ngemini-3.6-flash-medium\ngemini-3.6-flash-low\ngemini-3.5-flash-high\ngemini-3.5-flash-medium\ngemini-3.5-flash-low\ngemini-3.1-pro-high\ngemini-3.1-pro-low\nclaude-sonnet-4-6\nclaude-opus-4-6-thinking\ngpt-oss-120b-medium"; diff --git a/crates/op-host-services/src/cli_provider_probe.rs b/crates/op-host-services/src/cli_provider_probe.rs index 51e43d55b..5ec2e06ee 100644 --- a/crates/op-host-services/src/cli_provider_probe.rs +++ b/crates/op-host-services/src/cli_provider_probe.rs @@ -84,6 +84,11 @@ pub fn connect_antigravity_localized(locale: Locale) -> ProbeOutcome { Ok(models) => models, Err(error) => return failed(error.to_string()), }; + // Informational only: pairs the version we just probed with the catalog + // layout the parse above saw, and logs it when that pair changes. `agy` + // has changed its `models` format three times; without this, dating a + // format change means reading the binary's mtime. + crate::cli_model_discovery::note_antigravity_catalog_version(&version); ProbeOutcome { connected: true, models,