From 66260e76f9d81f33117c3ed6afb25aa8795a64e5 Mon Sep 17 00:00:00 2001 From: ajianaz Date: Fri, 16 Oct 2026 11:09:13 +0700 Subject: [PATCH] =?UTF-8?q?feat(cli):=20tole=20doctor=20full=20tier=20?= =?UTF-8?q?=E2=80=94=20chat,=20streaming=20and=20a=20real=20tool-call=20lo?= =?UTF-8?q?op=20(#378,=202/3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 2 +- CONTRIBUTING.md | 11 ++ README.md | 5 +- crates/tole-cli/src/doctor.rs | 323 ++++++++++++++++++++++++++++++++-- crates/tole-cli/src/main.rs | 4 +- 5 files changed, 324 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c55f3e..53961cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added -- `tole doctor` (#378): tiered provider diagnostics — `offline` (wiring) and `catalog` (live `/models`, the model must be served); each blocker carries its fix command, the exit code is non-zero when the requested tier is not reached, and keys / URL credentials are never printed. +- `tole doctor` (#378): tiered provider diagnostics — `offline` (wiring), `catalog` (live `/models`, model must be served) and `full` (chat, streaming and a real tool-call loop) — each blocker carries its fix command, and keys / URL credentials are never printed. Exits non-zero when the requested tier is not reached; `--tier full` is the release-validation gate. ### Changed - Memory loop injection hygiene (#384): the recalled batch is hard-capped at 24 items / 64 KiB and deduplicated; a malformed recall payload (non-array, or a hit without content) now fails the whole batch visibly instead of silently injecting fewer items; `tole serve`/MCP sessions report recall failures on stderr like the CLI does. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 54caf8a..3ee7eb7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -124,6 +124,17 @@ behavior that was already fixed). Before any live validation or mission run: bump: `cp target/release/tole ~/.cargo/bin/tole` — verify with `tole --version` matching the workspace version in `Cargo.toml`. +## Release validation + +Before claiming a build is ready to release, run the provider doctor at the +full tier with the repo checkout binary: + +```bash +cargo build --release -p tole-cli && ./target/release/tole doctor --tier full +``` + +It must exit 0 (chat, streaming and a real tool-call loop all green). + ## Design rules - The core crate (`tole-core`) must stay platform-agnostic: no stdin/stdout, diff --git a/README.md b/README.md index 1fe2fdf..a097c4d 100644 --- a/README.md +++ b/README.md @@ -126,11 +126,12 @@ trust: `internal` / `read_only`), `--skill ` (load a SKILL.md), |----------|--------| | `offline` | config resolves, base URL shape, key present (no network) | | `catalog` (default) | live `GET /models`; the configured model must be served | +| `full` | a chat call, a streamed call and a real tool-call loop (spends a few tokens) | Tiers are cumulative and stop at the first blocker, which is printed with the command that fixes it. The exit code is non-zero when the requested -tier is not reached. API keys and URL credentials never appear in the -output. +tier is not reached, so `tole doctor --tier full` is the gate for "ready to +release" claims. API keys and URL credentials never appear in the output. ## Configuration (project file) diff --git a/crates/tole-cli/src/doctor.rs b/crates/tole-cli/src/doctor.rs index f335d27..cb830c5 100644 --- a/crates/tole-cli/src/doctor.rs +++ b/crates/tole-cli/src/doctor.rs @@ -6,16 +6,34 @@ //! present). No network. //! - **catalog** — live `GET /models`; the configured model must be served //! (when the gateway lists models at all). +//! - **full** — three real calls through the real turn loop: a plain chat +//! completion, a streamed one, and a tool-call loop (the model must call +//! a probe tool and the loop must settle). This is the release gate: a +//! `full` green is the precondition for "ready to release" claims. //! //! Tiers are cumulative and stop at the first blocker, which carries the //! command that fixes it. -use tole_core::openai::{fetch_model_ids, OpenAiConfig, ENV_PREFIXES}; +use serde_json::{json, Value}; +use std::path::PathBuf; +use std::time::Duration; +use tole_core::approval::{Approver, ToolRequest, Verdict}; +use tole_core::openai::{fetch_model_ids, OpenAiConfig, OpenAiProvider, ENV_PREFIXES}; +use tole_core::storage::JsonlStorage; +use tole_core::tool::{Risk, Tool, ToolRegistry}; +use tole_core::turn::{run_turn, TurnOutcome}; + +/// Per-call timeout of the `full` tier. +const FULL_CALL_TIMEOUT: Duration = Duration::from_secs(60); + +/// Marker the probe tool must receive back from the model (not a secret). +const PROBE_MARKER: &str = "doctor-marker-7f3a"; #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] pub enum Tier { Offline, Catalog, + Full, } impl Tier { @@ -23,6 +41,7 @@ impl Tier { match self { Tier::Offline => "offline", Tier::Catalog => "catalog", + Tier::Full => "full", } } @@ -30,6 +49,7 @@ impl Tier { match s { "offline" => Some(Tier::Offline), "catalog" => Some(Tier::Catalog), + "full" => Some(Tier::Full), _ => None, } } @@ -120,6 +140,194 @@ fn base_url_ok(url: &str) -> bool { } } +struct DenyAll; +impl Approver for DenyAll { + fn decide(&self, _req: &ToolRequest<'_>) -> Verdict { + Verdict::Deny + } +} + +/// ReadOnly probe the model must call in the `full` tier. +struct ProbeTool { + calls: std::sync::Arc, + saw_token: std::sync::Arc, +} + +impl Tool for ProbeTool { + fn name(&self) -> &str { + "doctor_probe" + } + fn risk(&self) -> Risk { + Risk::ReadOnly + } + fn summary(&self) -> String { + "Diagnostic probe: echoes the given marker back. Call it exactly once when asked.".into() + } + fn spec(&self) -> Option { + Some(json!({ + "type": "object", + "properties": {"marker": {"type": "string", "description": "The marker to echo."}}, + "required": ["marker"] + })) + } + fn execute(&self, input: Value) -> Result { + use std::sync::atomic::Ordering; + self.calls.fetch_add(1, Ordering::SeqCst); + if input["marker"].as_str() == Some(PROBE_MARKER) { + self.saw_token.store(true, Ordering::SeqCst); + } + Ok(json!({ "echo": input["marker"], "ok": true })) + } +} + +fn scratch_dir(tag: &str) -> PathBuf { + let nanos = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0); + let dir = + std::env::temp_dir().join(format!("tole-doctor-{}-{tag}-{nanos}", std::process::id())); + let _ = std::fs::create_dir_all(&dir); + dir +} + +/// Run one prompt through the real turn loop against the real provider. +fn drive( + cfg: &OpenAiConfig, + streaming: bool, + registry: &ToolRegistry, + prompt: &str, + tag: &str, +) -> Result { + let dir = scratch_dir(tag); + let result = (|| { + let mut storage = JsonlStorage::create(&dir, format!("doctor-{tag}"), None) + .map_err(|e| format!("cannot create the scratch session: {e}"))?; + let mut provider = OpenAiProvider::new(cfg.clone()) + .with_streaming(streaming) + .with_timeout(FULL_CALL_TIMEOUT) + .with_tool_specs(registry.specs()); + run_turn(&mut storage, &mut provider, registry, prompt).map_err(|e| e.to_string()) + })(); + let _ = std::fs::remove_dir_all(&dir); + result +} + +fn describe_outcome(o: &TurnOutcome) -> String { + match o { + TurnOutcome::Final { .. } => "final answer".into(), + TurnOutcome::ProviderFailed { message } => format!("provider failed: {message}"), + TurnOutcome::UnknownTool { name } => format!("model asked for unknown tool {name:?}"), + TurnOutcome::LoopDetected { tool, count } => { + format!("model repeated {tool:?} {count} times") + } + TurnOutcome::BudgetExhausted => "step budget exhausted".into(), + TurnOutcome::ApprovalRequired { name } => format!("tool {name:?} needed approval"), + other => format!("unexpected turn outcome: {other:?}"), + } +} + +/// One plain-text check (`chat` or `stream`): a non-empty final answer. +fn text_check(cfg: &OpenAiConfig, streaming: bool, out: &mut Outcome) -> bool { + let check: &'static str = if streaming { "stream" } else { "chat" }; + let reg = ToolRegistry::with_approver(DenyAll); + match drive( + cfg, + streaming, + ®, + "Reply with the single word PONG.", + check, + ) { + Ok(TurnOutcome::Final { text, .. }) if !text.trim().is_empty() => { + out.pass(format!("{check}: model answered")); + true + } + Ok(TurnOutcome::Final { .. }) => { + out.fail( + Tier::Full, + check, + "the model returned an empty answer".into(), + "tole run \"Reply with the single word PONG.\"".into(), + ); + false + } + Ok(other) => { + out.fail( + Tier::Full, + check, + describe_outcome(&other), + "tole run \"Reply with the single word PONG.\"".into(), + ); + false + } + Err(e) => { + out.fail(Tier::Full, check, e, "tole run \"ping\"".into()); + false + } + } +} + +/// The tool-call loop: the model must call the probe with the token and +/// the loop must settle on a final answer afterwards. +fn tools_check(cfg: &OpenAiConfig, out: &mut Outcome) -> bool { + use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; + use std::sync::Arc; + let calls = Arc::new(AtomicUsize::new(0)); + let saw_token = Arc::new(AtomicBool::new(false)); + let mut reg = ToolRegistry::with_approver(DenyAll); + let probe = ProbeTool { + calls: Arc::clone(&calls), + saw_token: Arc::clone(&saw_token), + }; + if let Err(e) = reg.register(Box::new(probe)) { + out.fail( + Tier::Full, + "tools", + format!("cannot register the probe tool: {e}"), + "report this as a tole bug".into(), + ); + return false; + } + let prompt = format!( + "Call the tool doctor_probe exactly once with {{\"marker\":\"{PROBE_MARKER}\"}}, \ + then reply with the word DONE." + ); + let fix = "pick a model with tool-calling support: export TOLE_MODEL=".to_string(); + match drive(cfg, true, ®, &prompt, "tools") { + Ok(TurnOutcome::Final { .. }) => { + if calls.load(Ordering::SeqCst) == 0 { + out.fail( + Tier::Full, + "tools", + "the model answered without calling the probe tool (tool calling not working)" + .into(), + fix, + ); + false + } else if !saw_token.load(Ordering::SeqCst) { + out.fail( + Tier::Full, + "tools", + "the model called the probe tool with the wrong arguments".into(), + fix, + ); + false + } else { + out.pass("tools: tool-call loop completed".into()); + true + } + } + Ok(other) => { + out.fail(Tier::Full, "tools", describe_outcome(&other), fix); + false + } + Err(e) => { + out.fail(Tier::Full, "tools", e, fix); + false + } + } +} + /// `scheme://host[:port]/path` only: userinfo, query and fragment can carry /// credentials and never belong in output or on disk. pub fn redact_url(url: &str) -> String { @@ -255,6 +463,17 @@ fn diagnose_raw( } } out.reached = Some(Tier::Catalog); + if requested == Tier::Catalog { + return out; + } + + // ---- full ---- + if text_check(cfg, false, &mut out) + && text_check(cfg, true, &mut out) + && tools_check(cfg, &mut out) + { + out.reached = Some(Tier::Full); + } out } @@ -280,11 +499,9 @@ pub fn run_cli( #[cfg(test)] mod tests { use super::*; - use serde_json::{json, Value}; use std::io::{Read, Write}; use std::net::TcpListener; use std::sync::Arc; - use std::time::Duration; fn no_env(_: &str) -> Option { None @@ -341,14 +558,55 @@ mod tests { format!("http://{addr}/v1") } - /// A provider that serves `GET /models` with the given ids. - fn models_provider(models: Vec<&'static str>) -> String { - serve(Arc::new(move |method, path, _| { + fn sse_text(text: &str) -> String { + format!( + "data: {}\n\ndata: {{\"choices\":[{{\"delta\":{{}},\"finish_reason\":\"stop\"}}]}}\n\ndata: [DONE]\n\n", + json!({"choices":[{"delta":{"role":"assistant","content":text}}]}) + ) + } + + fn sse_tool(args: &str) -> String { + format!( + "data: {}\n\ndata: {{\"choices\":[{{\"delta\":{{}},\"finish_reason\":\"tool_calls\"}}]}}\n\ndata: [DONE]\n\n", + json!({"choices":[{"delta":{"tool_calls":[{"index":0,"id":"call_1", + "function":{"name":"doctor_probe","arguments":args}}]}}]}) + ) + } + + /// A well-behaved provider; `calls_tool` toggles tool-calling. + fn good_provider(models: Vec<&'static str>, calls_tool: bool) -> String { + serve(Arc::new(move |method, path, body| { if method == "GET" && path.ends_with("/models") { let data: Vec = models.iter().map(|m| json!({"id": m})).collect(); - (200, "application/json", json!({"data": data}).to_string()) + return (200, "application/json", json!({"data": data}).to_string()); + } + let stream = body["stream"].as_bool() == Some(true); + let has_tools = body["tools"].as_array().is_some_and(|t| !t.is_empty()); + let last_is_tool = body["messages"] + .as_array() + .and_then(|m| m.last()) + .is_some_and(|m| m["role"] == "tool"); + let want_call = has_tools && calls_tool && !last_is_tool; + if stream { + let sse = if want_call { + sse_tool(&json!({"marker": PROBE_MARKER}).to_string()) + } else { + sse_text("PONG") + }; + (200, "text/event-stream", sse) } else { - (404, "application/json", "{}".to_string()) + let msg = if want_call { + json!({"role":"assistant","content":null,"tool_calls":[{"id":"call_1", + "type":"function","function":{"name":"doctor_probe", + "arguments": json!({"marker": PROBE_MARKER}).to_string()}}]}) + } else { + json!({"role":"assistant","content":"PONG"}) + }; + ( + 200, + "application/json", + json!({"choices":[{"message": msg}]}).to_string(), + ) } })) } @@ -407,7 +665,7 @@ mod tests { #[test] fn catalog_flags_an_unserved_model() { - let url = models_provider(vec!["alpha", "beta"]); + let url = good_provider(vec!["alpha", "beta"], true); let o = diagnose(Some(&cfg_for(url, "gamma")), &no_env, Tier::Catalog); let b = o.blocker.unwrap(); assert_eq!((b.tier, b.check), (Tier::Catalog, "model")); @@ -432,6 +690,44 @@ mod tests { assert!(!b.message.contains("sk-test")); } + #[test] + fn full_passes_against_a_well_behaved_provider() { + let url = good_provider(vec!["m"], true); + let o = diagnose(Some(&cfg_for(url, "m")), &no_env, Tier::Full); + assert!(o.blocker.is_none(), "{:?}", o.blocker); + assert_eq!(o.reached, Some(Tier::Full)); + assert!(o.lines.iter().any(|l| l.contains("tools: tool-call loop"))); + } + + #[test] + fn full_fails_when_the_model_never_calls_the_tool() { + let url = good_provider(vec!["m"], false); + let o = diagnose(Some(&cfg_for(url, "m")), &no_env, Tier::Full); + let b = o.blocker.unwrap(); + assert_eq!((b.tier, b.check), (Tier::Full, "tools")); + assert!(b.message.contains("without calling"), "{}", b.message); + assert_eq!(o.reached, Some(Tier::Catalog)); + } + + #[test] + fn full_fails_at_chat_when_completions_are_broken() { + let url = serve(Arc::new(|method, path, _| { + if method == "GET" && path.ends_with("/models") { + ( + 200, + "application/json", + json!({"data":[{"id":"m"}]}).to_string(), + ) + } else { + (500, "application/json", json!({"error":"boom"}).to_string()) + } + })); + let o = diagnose(Some(&cfg_for(url, "m")), &no_env, Tier::Full); + let b = o.blocker.unwrap(); + assert_eq!((b.tier, b.check), (Tier::Full, "chat")); + assert_eq!(o.reached, Some(Tier::Catalog)); + } + #[test] fn urls_are_reduced_to_scheme_host_path() { assert_eq!( @@ -456,15 +752,10 @@ mod tests { #[test] fn tier_parse_roundtrip() { - for t in [Tier::Offline, Tier::Catalog] { + for t in [Tier::Offline, Tier::Catalog, Tier::Full] { assert_eq!(Tier::parse(t.as_str()), Some(t)); } assert_eq!(Tier::parse("nope"), None); - assert_eq!( - Tier::parse("full"), - None, - "the full tier is not available yet" - ); - assert!(Tier::Offline < Tier::Catalog); + assert!(Tier::Offline < Tier::Catalog && Tier::Catalog < Tier::Full); } } diff --git a/crates/tole-cli/src/main.rs b/crates/tole-cli/src/main.rs index a3b5df0..28623f3 100644 --- a/crates/tole-cli/src/main.rs +++ b/crates/tole-cli/src/main.rs @@ -321,7 +321,7 @@ enum Command { /// a real tool-call loop). Exits non-zero when the requested tier is not reached, so `--tier full` /// is the release-validation gate. Doctor { - /// offline | catalog. + /// offline | catalog | full (full spends a few real tokens). #[arg(long, default_value = "catalog")] tier: String, }, @@ -1041,7 +1041,7 @@ fn dispatch(cli: Cli) -> Result<()> { Command::Sessions => sessions_command(&sessions_dir), Command::Doctor { tier } => { let Some(tier) = tole_cli::doctor::Tier::parse(&tier) else { - bail!("--tier must be offline or catalog (got {tier:?})"); + bail!("--tier must be offline, catalog or full (got {tier:?})"); }; let cfg = config_apply::provider_config(); if !tole_cli::doctor::run_cli(cfg.as_ref(), &config_apply::real_env, tier) {