diff --git a/crates/travsr-cli/src/status.rs b/crates/travsr-cli/src/status.rs index e86c10c0..993c29c9 100644 --- a/crates/travsr-cli/src/status.rs +++ b/crates/travsr-cli/src/status.rs @@ -6,6 +6,10 @@ use anyhow::Context as _; use travsr_mcp::query::{self, StatusPayload}; +// #760: the Phase B warning classes are the daemon's, not this file's. Naming +// them through the enum it writes them from is what lets the guard below prove +// this surface handles every one. +use travsr_plugin_host::phase_b::PhaseBWarningClass as Warn; use crate::daemon_client; use crate::repo::find_git_root; @@ -61,14 +65,14 @@ fn phase_b_state(payload: &StatusPayload) -> String { // on (not trusted / not registered) are their own separate notice, // not a downgrade of the ones that did run. let crashed = crashed_langs(payload); - let not_run: Vec = warned_langs(payload, "skipped_no_analyzer") + let not_run: Vec = warned_langs(payload, Warn::SkippedNoAnalyzer.tag()) .into_iter() - .chain(warned_langs(payload, "needs_consent")) + .chain(warned_langs(payload, Warn::NeedsConsent.tag())) // needs_approval is vestigial (elevated access is auto-granted // now, ADR-017 A5), but a pre-upgrade index can still have it in // stored meta; keep honouring it so status stays honest rather // than reporting a flat "complete" for a language that never ran. - .chain(warned_langs(payload, "needs_approval")) + .chain(warned_langs(payload, Warn::NeedsApproval.tag())) .collect(); if crashed.is_empty() && not_run.is_empty() { "complete".to_string() @@ -94,7 +98,7 @@ fn phase_b_state(payload: &StatusPayload) -> String { /// `semantic:` field from `complete` to `partial (crashed: …)` so it agrees with /// the crash warning and the per-language outcome. fn crashed_langs(payload: &StatusPayload) -> Vec { - warned_langs(payload, "crashed") + warned_langs(payload, Warn::Crashed.tag()) } /// Languages named by a `:` entry in the `phase_b_warnings` meta, for @@ -128,6 +132,151 @@ fn head_at(cwd: &std::path::Path) -> Option { crate::git_bounded::git_stdout_bounded(Some(cwd), ["rev-parse", "--short", "HEAD"]) } +/// The user-facing lines for the `phase_b_warnings` meta, in print order. +/// +/// Split out of `run` (#760) so the guard below can assert that every class the +/// daemon writes is rendered here. Printing straight to stderr from inside `run` +/// left this consumer untestable, which is half of why classes could go missing +/// from it unnoticed. +fn phase_b_warning_lines(warnings: &str) -> Vec { + let mut out: Vec = Vec::new(); + if warnings.is_empty() { + return out; + } + // Trust is per-repo, not per-language: a single `install` enables + // every language at once, so collapse the "not enabled here" notices + // into one line rather than repeating it per language (matches init). + let untrusted: Vec<&str> = warnings + .split(',') + .filter_map(|w| w.strip_prefix(&format!("{}:", Warn::UntrustedCorpus.tag()))) + .collect(); + if !untrusted.is_empty() { + out.push(format!( + "warning: semantic analysis is not enabled for this repository yet ({}); run `travsr lang install ` here to enable", + untrusted.join(", ") + )); + } + for warn in warnings.split(',') { + let parts: Vec<&str> = warn.splitn(2, ':').collect(); + match parts.as_slice() { + // #712: point at the force path. A plain `travsr init + // --semantic` re-runs on top of the existing graph, which a + // no-op Phase A can make look like it did nothing; `--force` + // purges and rebuilds so the retry is unambiguous. + ["crashed", lang] => out.push(format!( + "warning: semantic analyzer for '{lang}' crashed, fix the tool (e.g. `travsr lang install {lang}`), then re-run `travsr init --semantic --force` to rebuild" + )), + ["version_mismatch", rest] => { + let v: Vec<&str> = rest.splitn(3, ':').collect(); + if let [lang, expected, got] = v.as_slice() { + out.push(format!( + "warning: the '{lang}' analyzer is out of date (protocol v{got}, expected v{expected}); run `travsr lang install {lang}`" + )); + } + } + // Windows only: an analyzer that cannot run inside Travsr's + // isolation and has no permission on record. The one-time + // permission is the only thing standing between it and full + // analysis here. + ["needs_consent", lang] => out.push(format!( + "warning: full '{lang}' analysis needs your permission to run; run `travsr lang allow-unsandboxed {lang}`" + )), + // Vestigial: elevated access is auto-granted for local use (ADR-017 + // Amendment A5), so this build never writes it, but a pre-upgrade + // index still holds it in stored meta. `phase_b_state` already + // downgrades to "not run" for it; without this arm the user was told + // the language did not run and never told what to do about it. Found + // by the #760 guard, which is the hole this class of bug lives in. + ["needs_approval", lang] => out.push(format!( + "warning: '{lang}' was skipped by a previous index. Run `travsr lang install {lang}`, then `travsr init --semantic --force` to index it" + )), + // #712: analyzer ran but produced no nodes over the repo's + // source files of this language — a silent zero-node result, + // not a crash. Point at the tool and a rebuild. + // UX-3: the analyzer is installed and active (the zero-node + // warning only fires for a language that ran), so telling the + // user to reinstall misdirects — reinstalling changes nothing. + // The real causes are the sidecar failing to parse/resolve this + // repo's sources (e.g. a sandbox-denied read, a missing SDK, or + // no buildable project). Point at the sidecar's own diagnostics, + // which the host now forwards on stderr. + // #724: definitions arrived, occurrences did not, so no call + // edge can come from this language. The analyzer reported + // success, which is what makes it worth saying out loud. + ["no_references", lang] => { + out.push(format!( + "warning: '{lang}' analysis produced definitions but no references, so no call edges came from it. The analyzer reported success, so this is its output being incomplete rather than a crash. Re-run `RUST_LOG=travsr_plugin_host=debug travsr init --semantic --force` to see its own diagnostics" + )); + } + ["zero_nodes", lang] => { + out.push(format!( + "warning: '{lang}' analysis ran but found no symbols, though the repo has '{lang}' sources. The analyzer is installed, so reinstalling will not help, it usually means the analyzer could not read or build this project's sources (a missing SDK or an unbuildable project). Fix the project setup, then re-run `travsr init --semantic --force`" + )); + // Name the concrete thing to check rather than leaving + // "a missing SDK or an unbuildable project" as the only + // clue — the catalog already knows what this language's + // analyzer needs from the project. + if let Some(entry) = travsr_plugin_host::phase_b::catalog::lookup(lang) { + let prereq = entry.effective_prerequisites(); + if !prereq.is_empty() && prereq != "none" { + out.push(format!(" needs: {prereq}")); + } + } + // #724 Finding 4: the most common cause of a zero-node + // Java run on macOS is scip-java's javac shim crashing + // under the stock bash 3.2. Surface the actionable fix. + if *lang == "java" { + if let Some(hint) = crate::progress::macos_java_bash_hint() { + out.push(format!(" {hint}")); + } + } + } + // #449: languages present in the repo whose Phase B sidecar + // never ran, previously a silent skip that left the user + // with "0 references" and no explanation. + // A language whose analyzer has no build for this OS can never + // reach full analysis here, so pointing at `travsr lang install` + // (which just dead-ends) is misleading — state the honest + // "not available on this platform" instead. + ["skipped_unregistered", lang] if crate::lang::full_analysis_unavailable_here(lang) => { + out.push(format!( + "note: full '{lang}' analysis is not available on this platform, structural analysis still works" + )) + } + ["skipped_unregistered", lang] => out.push(format!( + "warning: '{lang}' sources found but full analysis is not set up. Run `travsr lang install {lang}`" + )), + ["skipped_no_analyzer", lang] if crate::lang::full_analysis_unavailable_here(lang) => { + out.push(format!( + "note: full '{lang}' analysis is not available on this platform, structural analysis still works" + )) + } + // #414 (ADR-017 Rule 3): registered globally but this repo was + // never enabled. Collapsed into one combined line above the + // loop (trust is per-repo, so one install fixes all of them). + ["untrusted_corpus", _] => {} + ["skipped_no_analyzer", lang] => out.push(format!( + "warning: '{lang}' is registered but its analyzer binary is missing. Run `travsr lang install {lang}`" + )), + // L5a: scip-clang (c/cpp) needs a compile_commands.json at the + // repo root — without one it hangs, so it is skipped up front. + ["skipped_no_compdb", lang] => out.push(format!( + "warning: full '{lang}' analysis needs a compile database (compile_commands.json) at the repo root. Generate one (e.g. `bear -- make`, or CMake's CMAKE_EXPORT_COMPILE_COMMANDS)" + )), + // E6: SCIP definitions that did not unify onto their Phase A + // tree-sitter node — their references attribute to an orphaned + // duplicate node instead. `rate` is missed/attempted. Repo-wide, + // not a per-language state, so it is deliberately not a + // `PhaseBWarningClass` variant. + ["scip_unification_misses", rate] => out.push(format!( + "warning: {rate} semantic definitions did not match their parsed symbol, some references may resolve to a duplicate. Re-run `travsr init --semantic` if it persists." + )), + _ => {} + } + } + out +} + pub fn run() -> anyhow::Result<()> { let cwd = std::env::current_dir().context("getting current directory")?; // `head_at` and `find_git_root` are independent, bounded git queries on the @@ -207,131 +356,8 @@ pub fn run() -> anyhow::Result<()> { // H3: surface Phase B warnings so the user knows about crashed/mismatched // analyzers without having to re-read the init output. if let Some(warnings) = &payload.phase_b_warnings { - if !warnings.is_empty() { - // Trust is per-repo, not per-language: a single `install` enables - // every language at once, so collapse the "not enabled here" notices - // into one line rather than repeating it per language (matches init). - let untrusted: Vec<&str> = warnings - .split(',') - .filter_map(|w| w.strip_prefix("untrusted_corpus:")) - .collect(); - if !untrusted.is_empty() { - eprintln!( - "warning: semantic analysis is not enabled for this repository yet ({}); run `travsr lang install ` here to enable", - untrusted.join(", ") - ); - } - for warn in warnings.split(',') { - let parts: Vec<&str> = warn.splitn(2, ':').collect(); - match parts.as_slice() { - // #712: point at the force path. A plain `travsr init - // --semantic` re-runs on top of the existing graph, which a - // no-op Phase A can make look like it did nothing; `--force` - // purges and rebuilds so the retry is unambiguous. - ["crashed", lang] => eprintln!( - "warning: semantic analyzer for '{lang}' crashed, fix the tool (e.g. `travsr lang install {lang}`), then re-run `travsr init --semantic --force` to rebuild" - ), - ["version_mismatch", rest] => { - let v: Vec<&str> = rest.splitn(3, ':').collect(); - if let [lang, expected, got] = v.as_slice() { - eprintln!( - "warning: the '{lang}' analyzer is out of date (protocol v{got}, expected v{expected}); run `travsr lang install {lang}`" - ); - } - } - // Windows only: an analyzer that cannot run inside Travsr's - // isolation and has no permission on record. The one-time - // permission is the only thing standing between it and full - // analysis here. - ["needs_consent", lang] => eprintln!( - "warning: full '{lang}' analysis needs your permission to run; run `travsr lang allow-unsandboxed {lang}`" - ), - // #712: analyzer ran but produced no nodes over the repo's - // source files of this language — a silent zero-node result, - // not a crash. Point at the tool and a rebuild. - // UX-3: the analyzer is installed and active (the zero-node - // warning only fires for a language that ran), so telling the - // user to reinstall misdirects — reinstalling changes nothing. - // The real causes are the sidecar failing to parse/resolve this - // repo's sources (e.g. a sandbox-denied read, a missing SDK, or - // no buildable project). Point at the sidecar's own diagnostics, - // which the host now forwards on stderr. - // #724: definitions arrived, occurrences did not, so no call - // edge can come from this language. The analyzer reported - // success, which is what makes it worth saying out loud. - ["no_references", lang] => { - eprintln!( - "warning: '{lang}' analysis produced definitions but no references, so no call edges came from it. The analyzer reported success, so this is its output being incomplete rather than a crash. Re-run `RUST_LOG=travsr_plugin_host=debug travsr init --semantic --force` to see its own diagnostics" - ); - } - ["zero_nodes", lang] => { - eprintln!( - "warning: '{lang}' analysis ran but found no symbols, though the repo has '{lang}' sources. The analyzer is installed, so reinstalling will not help, it usually means the analyzer could not read or build this project's sources (a missing SDK or an unbuildable project). Fix the project setup, then re-run `travsr init --semantic --force`" - ); - // Name the concrete thing to check rather than leaving - // "a missing SDK or an unbuildable project" as the only - // clue — the catalog already knows what this language's - // analyzer needs from the project. - if let Some(entry) = travsr_plugin_host::phase_b::catalog::lookup(lang) { - let prereq = entry.effective_prerequisites(); - if !prereq.is_empty() && prereq != "none" { - eprintln!(" needs: {prereq}"); - } - } - // #724 Finding 4: the most common cause of a zero-node - // Java run on macOS is scip-java's javac shim crashing - // under the stock bash 3.2. Surface the actionable fix. - if *lang == "java" { - if let Some(hint) = crate::progress::macos_java_bash_hint() { - eprintln!(" {hint}"); - } - } - } - // #449: languages present in the repo whose Phase B sidecar - // never ran, previously a silent skip that left the user - // with "0 references" and no explanation. - // A language whose analyzer has no build for this OS can never - // reach full analysis here, so pointing at `travsr lang install` - // (which just dead-ends) is misleading — state the honest - // "not available on this platform" instead. - ["skipped_unregistered", lang] - if crate::lang::full_analysis_unavailable_here(lang) => - { - eprintln!( - "note: full '{lang}' analysis is not available on this platform, structural analysis still works" - ) - } - ["skipped_unregistered", lang] => eprintln!( - "warning: '{lang}' sources found but full analysis is not set up. Run `travsr lang install {lang}`" - ), - ["skipped_no_analyzer", lang] - if crate::lang::full_analysis_unavailable_here(lang) => - { - eprintln!( - "note: full '{lang}' analysis is not available on this platform, structural analysis still works" - ) - } - // #414 (ADR-017 Rule 3): registered globally but this repo was - // never enabled. Collapsed into one combined line above the - // loop (trust is per-repo, so one install fixes all of them). - ["untrusted_corpus", _] => {} - ["skipped_no_analyzer", lang] => eprintln!( - "warning: '{lang}' is registered but its analyzer binary is missing. Run `travsr lang install {lang}`" - ), - // L5a: scip-clang (c/cpp) needs a compile_commands.json at the - // repo root — without one it hangs, so it is skipped up front. - ["skipped_no_compdb", lang] => eprintln!( - "warning: full '{lang}' analysis needs a compile database (compile_commands.json) at the repo root. Generate one (e.g. `bear -- make`, or CMake's CMAKE_EXPORT_COMPILE_COMMANDS)" - ), - // E6: SCIP definitions that did not unify onto their Phase A - // tree-sitter node — their references attribute to an orphaned - // duplicate node instead. `rate` is missed/attempted. - ["scip_unification_misses", rate] => eprintln!( - "warning: {rate} semantic definitions did not match their parsed symbol, some references may resolve to a duplicate. Re-run `travsr init --semantic` if it persists." - ), - _ => {} - } - } + for line in phase_b_warning_lines(warnings) { + eprintln!("{line}"); } } @@ -531,6 +557,50 @@ mod tests { assert_eq!(phase_b_state(&p), "complete"); } + /// #760: every warning class the daemon can write must produce a line here. + /// + /// The classes are produced in one place and read in three, and until now + /// every reader kept its own hardcoded copy of the names. A class this + /// surface does not handle is dropped in silence: `travsr status` prints + /// nothing about it and the user is left with a language that did not run + /// and no reason why. The guard that was supposed to catch that restated + /// the names in a third hand-written list, so a class missing from the + /// producer's readers AND from the list was invisible. + /// + /// This iterates `PhaseBWarningClass::ALL`, the enum the daemon formats + /// `phase_b_warnings` from, so adding a class there and forgetting this + /// file fails the build. Same treatment `is_native_phase_b` got in #752: + /// assert against the real decision, not against a copy of it. + /// + /// Turning it on immediately found one: `needs_approval` was downgrading + /// the `semantic:` field to "not run" while printing no explanation at all. + #[test] + fn every_phase_b_warning_class_the_daemon_writes_is_rendered() { + for class in Warn::ALL { + let tag = class.tag(); + let lines = phase_b_warning_lines(&class.sample_entry("go")); + assert!( + !lines.is_empty(), + "class {tag:?} is written by the daemon but `travsr status` says nothing \ + about it, so the language is degraded with no visible reason" + ); + for line in &lines { + assert!(!line.is_empty(), "class {tag:?} produced an empty line"); + } + } + } + + /// #760: `untrusted_corpus` is the one class with no arm of its own, because + /// trust is per-repo and the notices collapse into a single line before the + /// loop. Pin that, so the guard above cannot be satisfied by an accidental + /// arm that repeats it per language. + #[test] + fn untrusted_corpus_collapses_into_one_line_for_every_language() { + let lines = phase_b_warning_lines("untrusted_corpus:go,untrusted_corpus:php"); + assert_eq!(lines.len(), 1, "one line for the whole repo: {lines:?}"); + assert!(lines[0].contains("go, php"), "both named: {lines:?}"); + } + #[test] fn phase_b_opt_out_languages_do_not_downgrade_complete() { // Languages the user has not turned on for this repo (not trusted / not diff --git a/crates/travsr-daemon/src/lib.rs b/crates/travsr-daemon/src/lib.rs index e23ab6fd..4f0accfe 100644 --- a/crates/travsr-daemon/src/lib.rs +++ b/crates/travsr-daemon/src/lib.rs @@ -2961,15 +2961,21 @@ fn write_phase_b_results( // H3: stamp phase_b_warnings in the meta table so `travsr status` can surface // actionable issues without the user having to re-read init output. + // + // #760: every class name below comes from `PhaseBWarningClass`, which + // `travsr status` and the MCP `get_index_status` tool both iterate. A class + // added here that a consumer does not handle used to be dropped silently and + // could surface as a terminal `done`; now it fails their guards instead. + use travsr_plugin_host::phase_b::PhaseBWarningClass as Warn; let mut warnings: Vec = Vec::new(); for lang in &pb_outcome.crashed { - warnings.push(format!("crashed:{lang}")); + warnings.push(Warn::Crashed.entry(lang)); } // #712: a language whose analyzer ran but produced no nodes over its source // files. Surfaced so a silent zero-node "success" (e.g. scip-ruby invoked // without an input path) is visible and actionable in `travsr status`. for lang in &pb_outcome.produced_no_nodes { - warnings.push(format!("zero_nodes:{lang}")); + warnings.push(Warn::ZeroNodes.entry(lang)); } // #724: a sidecar that returned definitions and not one occurrence. No call // edge can be derived from it, so the run is a success with nothing @@ -2978,40 +2984,43 @@ fn write_phase_b_results( // `init` path defers Phase B to the daemon, so the java user this exists for // saw nothing anywhere (#752 review). for lang in &pb_outcome.produced_no_references { - warnings.push(format!("no_references:{lang}")); + warnings.push(Warn::NoReferences.entry(lang)); } for (lang, expected, got) in &pb_outcome.version_mismatch { - warnings.push(format!("version_mismatch:{lang}:{expected}:{got}")); + warnings.push(format!( + "{}:{expected}:{got}", + Warn::VersionMismatch.entry(lang) + )); } for lang in &pb_outcome.skipped_needs_approval { - warnings.push(format!("needs_approval:{lang}")); + warnings.push(Warn::NeedsApproval.entry(lang)); } // Windows-only: an analyzer that cannot run isolated here and has no // permission on record. Surface the exact `travsr lang allow-unsandboxed // ` fix rather than leaving the language silently absent. for lang in &pb_outcome.skipped_needs_consent { - warnings.push(format!("needs_consent:{lang}")); + warnings.push(Warn::NeedsConsent.entry(lang)); } // #449: a language present in the repo whose sidecar is not installed or // not registered used to be skipped silently, and the user saw "0 references" // with no hint that Phase B never ran. Surface both skip classes so // `travsr status` can print the exact `travsr lang install ` fix. for lang in &pb_outcome.skipped_unregistered { - warnings.push(format!("skipped_unregistered:{lang}")); + warnings.push(Warn::SkippedUnregistered.entry(lang)); } // #414 (ADR-017 Rule 3): a registered language whose corpus has no trust // grant is skipped before spawn — surface the exact `travsr lang add // --corpus ` fix. for lang in &pb_outcome.skipped_untrusted_corpus { - warnings.push(format!("untrusted_corpus:{lang}")); + warnings.push(Warn::UntrustedCorpus.entry(lang)); } for lang in &pb_outcome.skipped_no_analyzer { - warnings.push(format!("skipped_no_analyzer:{lang}")); + warnings.push(Warn::SkippedNoAnalyzer.entry(lang)); } // L5a: scip-clang (c/cpp) needs a compile_commands.json — surface it the // same way as the other user-actionable skip classes above. for lang in &pb_outcome.skipped_no_compdb { - warnings.push(format!("skipped_no_compdb:{lang}")); + warnings.push(Warn::SkippedNoCompdb.entry(lang)); } // E6: surface SCIP def-unification misses (orphaned twins). Positional // span-containment makes this near-zero; a non-zero rate means Phase A diff --git a/crates/travsr-mcp/src/observability.rs b/crates/travsr-mcp/src/observability.rs index 1ed0734a..5c62ec0b 100644 --- a/crates/travsr-mcp/src/observability.rs +++ b/crates/travsr-mcp/src/observability.rs @@ -304,13 +304,14 @@ fn error_payload(reason: &str) -> serde_json::Value { /// classes (e.g. `scip_unification_misses`, which is not per-language) are /// ignored, they're not a language state. /// -/// The set of per-language classes handled here must stay equal to the set -/// `travsr status` matches on, which is what the shared invariant above -/// actually requires: a class present there and absent here does not merely -/// lose its wording, it silently falls through to the availability ladder and -/// can be reported as a terminal `done` (#636 round-5 review, which is how -/// `untrusted_corpus` was missed). `phase_b_warning_classes_match_the_cli` -/// pins the set, not just one string. +/// Every class the daemon writes must be handled here: one that is not does +/// not merely lose its wording, it silently falls through to the availability +/// ladder and can be reported as a terminal `done` (#636 round-5 review, which +/// is how `untrusted_corpus` was missed). #760 made that enforceable rather +/// than aspirational: the classes are the variants of +/// `travsr_plugin_host::phase_b::PhaseBWarningClass`, the daemon formats from +/// it, and `phase_b_warning_classes_match_the_cli` iterates it, so a class +/// added there and forgotten here fails the build. /// /// `corpus` is the store's `corpus` meta, needed only by the /// `untrusted_corpus` arm, whose remediation names the corpus to trust. @@ -2378,49 +2379,46 @@ mod tests { assert!(!detail.contains('\u{2014}'), "em-dash: {detail}"); } - /// #636 round-5 review: pinning one string's wording was not enough. The - /// invariant that actually matters is that the *set* of per-language - /// warning classes handled here equals the set `travsr status` matches - /// on. A class present there and missing here does not just lose its - /// wording: it falls through to the availability ladder and can surface - /// as a terminal `done`, which is exactly how `untrusted_corpus` was - /// missed. Every class listed here is one the daemon writes. + /// #760: every warning class the daemon can write must be decoded here. + /// + /// #636 round-5 established the invariant: a class the daemon writes and + /// this function does not handle falls through to the availability ladder + /// and can surface as a terminal `done`, telling the user the language + /// succeeded when it did not. That is how `untrusted_corpus` was missed. + /// + /// The guard that followed could not enforce it. It restated the class + /// names in a third hand-written list, so a class missing from BOTH that + /// list and this function was invisible: there was nothing to disagree + /// with. `zero_nodes` and `needs_consent` sat in that hole until they were + /// found by reading the producer against the consumers. + /// + /// So the list is gone. This iterates `PhaseBWarningClass::ALL`, the enum + /// the daemon formats `phase_b_warnings` from, and asserts every variant + /// decodes. Adding a class to the producer and forgetting this consumer now + /// fails here. Same treatment `is_native_phase_b` got in #752: assert + /// against the real decision, not against a copy of it. #[test] fn phase_b_warning_classes_match_the_cli() { - // The per-language classes `travsr status` handles (status.rs). - // `scip_unification_misses` is deliberately absent: it is a repo-wide - // rate, not a per-language state, and neither surface treats it as one. - for class in [ - "crashed", - "version_mismatch", - "needs_approval", - "skipped_unregistered", - "skipped_no_analyzer", - "skipped_no_compdb", - "untrusted_corpus", - "no_references", - "zero_nodes", - "needs_consent", - ] { - // `version_mismatch` carries `lang:expected:got`, the rest `lang`. - let warning = if class == "version_mismatch" { - format!("{class}:go:2:1") - } else { - format!("{class}:go") - }; - let decoded = decode_phase_b_warnings(&warning, "github.com/acme/repo"); + use travsr_plugin_host::phase_b::PhaseBWarningClass; + // `scip_unification_misses` is deliberately not in `ALL`: it is a + // repo-wide rate, not a per-language state, and neither surface treats + // it as one. + for class in PhaseBWarningClass::ALL { + let tag = class.tag(); + let decoded = + decode_phase_b_warnings(&class.sample_entry("go"), "github.com/acme/repo"); let (state, detail) = decoded.get("go").unwrap_or_else(|| { - panic!("class {class:?} is handled by travsr status but falls through here") + panic!( + "class {tag:?} is written by the daemon but falls through here, so a \ + language it describes can still be reported as a terminal `done`" + ) }); assert!( matches!(*state, "failed" | "unavailable"), - "class {class:?} must map to a terminal state, got {state:?}" - ); - assert!(!detail.is_empty(), "class {class:?} must explain itself"); - assert!( - !detail.contains('\u{2014}'), - "em-dash in {class:?}: {detail}" + "class {tag:?} must map to a terminal state, got {state:?}" ); + assert!(!detail.is_empty(), "class {tag:?} must explain itself"); + assert!(!detail.contains('\u{2014}'), "em-dash in {tag:?}: {detail}"); } } diff --git a/crates/travsr-plugin-host/src/phase_b/mod.rs b/crates/travsr-plugin-host/src/phase_b/mod.rs index fc50d105..d0f51d26 100644 --- a/crates/travsr-plugin-host/src/phase_b/mod.rs +++ b/crates/travsr-plugin-host/src/phase_b/mod.rs @@ -1,6 +1,8 @@ pub mod catalog; pub mod platform; pub mod status; +pub mod warning; pub use catalog::{lookup, OutputFormat, PhaseBEntry, SandboxRequirement, CATALOG}; pub use platform::{full_analysis_unavailable_here, unsupported_reason}; pub use status::{capability, os_label, Capability, LangStatus}; +pub use warning::PhaseBWarningClass; diff --git a/crates/travsr-plugin-host/src/phase_b/warning.rs b/crates/travsr-plugin-host/src/phase_b/warning.rs new file mode 100644 index 00000000..7e364ae7 --- /dev/null +++ b/crates/travsr-plugin-host/src/phase_b/warning.rs @@ -0,0 +1,150 @@ +//! #760: the per-language Phase B warning classes, defined once. +//! +//! The daemon stamps a language's Phase B outcome into the `phase_b_warnings` +//! meta key as comma-separated `class:lang[:extra]` entries. Three surfaces read +//! it back: `travsr status` (travsr-cli), the MCP `get_index_status` tool +//! (travsr-mcp), and that tool's guard test. Before this, each of them carried +//! its own hand-written list of the class names and nothing derived from the +//! writer, so a class added to the daemon and forgotten in a consumer was +//! silently dropped: it fell through to the availability ladder and could +//! surface as a terminal `done`, telling the user the language SUCCEEDED when it +//! did not. `zero_nodes` and `needs_consent` sat in exactly that hole, invisible +//! to the guard because the guard was itself a third hand-written list with +//! nothing to disagree with. +//! +//! So this enum is the one source. The daemon formats its entries from +//! [`PhaseBWarningClass::entry`], and each consumer's guard iterates +//! [`PhaseBWarningClass::ALL`] and asserts that consumer handles every variant. +//! Adding a class here without teaching both consumers about it fails the build, +//! and there is no third list to keep in step. +//! +//! It lives in travsr-plugin-host because that is the crate the classes actually +//! come from: every variant below is one field of +//! [`PhaseBOutcome`](crate::PhaseBOutcome), which this crate produces, and it is +//! the only crate all three surfaces already depend on (travsr-daemon depends on +//! travsr-mcp, so the daemon itself cannot hold a definition travsr-mcp reads). +//! The shape follows [`LangStatus::tag`](super::status::LangStatus::tag): a +//! stable machine tag per variant, never reworded. + +/// One per-language Phase B warning class. +/// +/// Repo-wide diagnostics that are not a per-language state are deliberately not +/// here: `scip_unification_misses` is a missed/attempted rate for the whole +/// index, and neither consumer treats it as a language's status, so putting it +/// in [`ALL`](Self::ALL) would force both guards to assert something false. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PhaseBWarningClass { + /// The analyzer was found and spawned but died or errored mid-invoke. + Crashed, + /// #712: the analyzer ran cleanly and produced no graph output at all, even + /// though the language is present in the repo. + ZeroNodes, + /// #724: the analyzer returned definitions and not one occurrence, so no + /// call edge can be derived from it. + NoReferences, + /// The sidecar speaks a different plugin protocol version than expected. + /// The only class that carries extra fields: `lang:expected:got`. + VersionMismatch, + /// Vestigial since elevated access became auto-granted for local use + /// (ADR-017 Amendment A5). This build never writes it, but a pre-upgrade + /// index can still hold it in stored meta, so both consumers still decode it. + NeedsApproval, + /// Windows only: the analyzer cannot run inside Travsr's isolation and the + /// user has not granted permission to run it with their own privileges. + NeedsConsent, + /// #449: the language is present in the repo but not registered in lang.toml. + SkippedUnregistered, + /// #414 (ADR-017 Rule 3): registered globally, but this repository's corpus + /// has no trust grant, so the sidecar was never spawned. + UntrustedCorpus, + /// Registered, but the analyzer binary could not be resolved. + SkippedNoAnalyzer, + /// L5a: scip-clang (c/cpp) needs a `compile_commands.json` at the repo root + /// and there is none. + SkippedNoCompdb, +} + +impl PhaseBWarningClass { + /// Every class the daemon can write. The guards in travsr-cli and travsr-mcp + /// iterate this, so a variant added here must be handled by both. + pub const ALL: [PhaseBWarningClass; 10] = [ + PhaseBWarningClass::Crashed, + PhaseBWarningClass::ZeroNodes, + PhaseBWarningClass::NoReferences, + PhaseBWarningClass::VersionMismatch, + PhaseBWarningClass::NeedsApproval, + PhaseBWarningClass::NeedsConsent, + PhaseBWarningClass::SkippedUnregistered, + PhaseBWarningClass::UntrustedCorpus, + PhaseBWarningClass::SkippedNoAnalyzer, + PhaseBWarningClass::SkippedNoCompdb, + ]; + + /// The stable machine tag written to `phase_b_warnings`. Never reworded: it + /// is persisted in every existing index and read back by `travsr status` and + /// the MCP tool, so it is an API surface, not UI copy. + pub fn tag(&self) -> &'static str { + match self { + PhaseBWarningClass::Crashed => "crashed", + PhaseBWarningClass::ZeroNodes => "zero_nodes", + PhaseBWarningClass::NoReferences => "no_references", + PhaseBWarningClass::VersionMismatch => "version_mismatch", + PhaseBWarningClass::NeedsApproval => "needs_approval", + PhaseBWarningClass::NeedsConsent => "needs_consent", + PhaseBWarningClass::SkippedUnregistered => "skipped_unregistered", + PhaseBWarningClass::UntrustedCorpus => "untrusted_corpus", + PhaseBWarningClass::SkippedNoAnalyzer => "skipped_no_analyzer", + PhaseBWarningClass::SkippedNoCompdb => "skipped_no_compdb", + } + } + + /// The `class:lang` entry the daemon writes for `lang`. + /// [`VersionMismatch`](Self::VersionMismatch) appends `:expected:got` to this. + pub fn entry(&self, lang: &str) -> String { + format!("{}:{lang}", self.tag()) + } + + /// A complete, well-formed entry for `lang`, including whatever extra fields + /// the class carries. Exists so a consumer guard can iterate + /// [`ALL`](Self::ALL) and feed each class a decodable entry without keeping a + /// second list of which classes carry what, which is the drift #760 removed. + pub fn sample_entry(&self, lang: &str) -> String { + match self { + PhaseBWarningClass::VersionMismatch => format!("{}:2:1", self.entry(lang)), + _ => self.entry(lang), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// #760: `ALL` is what both consumer guards iterate, so a duplicated or + /// missing tag would quietly weaken them rather than fail loudly here. + #[test] + fn every_class_has_a_distinct_tag_and_a_decodable_sample() { + let tags: std::collections::HashSet<&str> = + PhaseBWarningClass::ALL.iter().map(|c| c.tag()).collect(); + assert_eq!( + tags.len(), + PhaseBWarningClass::ALL.len(), + "two classes share a tag, so one of them can never be decoded" + ); + for class in PhaseBWarningClass::ALL { + let sample = class.sample_entry("go"); + let (tag, rest) = sample.split_once(':').expect("class:lang at minimum"); + assert_eq!(tag, class.tag()); + assert!( + rest.starts_with("go"), + "the language must follow the tag: {sample}" + ); + } + // The one class with extra fields carries them in the sample, so a guard + // that only knows about `ALL` still hands it something decodable. + assert_eq!( + PhaseBWarningClass::VersionMismatch.sample_entry("go"), + "version_mismatch:go:2:1" + ); + } +} diff --git a/plugin-hashes.lock b/plugin-hashes.lock index 24971455..d42a1792 100644 --- a/plugin-hashes.lock +++ b/plugin-hashes.lock @@ -1,6 +1,6 @@ # Auto-generated by update-plugin-hashes.sh — do not edit manually # Format: crate_name = sha256_of_src_tree -travsr-plugin-host = 1e19ef90398f1e8f73d0cd3431e0c0fae2b56c5bf6debc5fd22f75ef275b6597 +travsr-plugin-host = bd8b4ef18db733bf3128a6b273de8a975495c164ee7ae330031959ac6ff65496 travsr-plugin-protocol = d6f8d6949b3c684aa21d4742a8c9ba04ccec793bea9551d8264f4c7be901d427 travsr-plugin-sdk = 6f20b7313d11d8fa0dfafd5203ef5870be443ee30b2141687c4624b7cc5f9a2b