diff --git a/rust/crates/caos-cli/TUI.md b/rust/crates/caos-cli/TUI.md index bfd6f29b..a921ee6d 100644 --- a/rust/crates/caos-cli/TUI.md +++ b/rust/crates/caos-cli/TUI.md @@ -45,11 +45,13 @@ asks for one — paste the key, or enter the path to a file that holds it — an writes the canonical secret entry, trimmed, with fresh cache-isolation entropy already included (what `caos secrets` would add). It ensures git ignores `.caos-secrets/` (adding the rule to `.git/info/exclude` when nothing else -covers it), re-loads the store through the normal loader, and continues -straight into the UI — no relaunch. A pasted key is erased from the screen the -moment it is submitted. A store that exists but fails to load is reported as -the error it is rather than prompting, so an existing broken configuration is -never overwritten. +covers it), re-reads the entry through the same store parser every turn starts +from, and continues straight into the UI — no relaunch. A pasted key is erased +from the screen the moment it is submitted. A store that exists but fails to +read is reported as the error it is rather than prompting, so an existing +broken configuration is never overwritten. The check is parse-only: the +secret's `reader=` expressions are resolved by the first turn, behind the +TUI's progress display, never while the terminal is still blank at startup. ```text caos tui continue the most recent conversation diff --git a/rust/crates/caos-cli/src/bin/tui/mod.rs b/rust/crates/caos-cli/src/bin/tui/mod.rs index cdc07575..5678a0e5 100644 --- a/rust/crates/caos-cli/src/bin/tui/mod.rs +++ b/rust/crates/caos-cli/src/bin/tui/mod.rs @@ -250,8 +250,9 @@ pub(crate) fn run(raw: &[String]) -> Result<(), String> { let transport = GitTransport::from_cwd()?; transport.ensure_server_reachable()?; // Missing model credential? Ask for one and install it right here, while - // the shell still has the terminal (setup::ensure_model_secret). - setup::ensure_model_secret(&transport)?; + // the shell still has the terminal (setup::ensure_model_secret). The check + // is parse-only: nothing before the first draw may evaluate the workspace. + setup::ensure_model_secret()?; let mut app = App::new(args)?; enable_raw_mode().map_err(|error| format!("enabling terminal raw mode: {error}"))?; diff --git a/rust/crates/caos-cli/src/bin/tui/setup.rs b/rust/crates/caos-cli/src/bin/tui/setup.rs index 68c06ef3..24f95216 100644 --- a/rust/crates/caos-cli/src/bin/tui/setup.rs +++ b/rust/crates/caos-cli/src/bin/tui/setup.rs @@ -2,18 +2,21 @@ //! secret is missing, ask for the key — pasted, or a path to a file holding //! one — before the alternate screen takes the terminal, write the canonical //! `.caos-secrets` entry with fresh entropy included, make sure git ignores -//! the store, prove it loads through the same loader every turn uses, and -//! continue straight into the UI — no relaunch. +//! the store, prove it reads back through the same store parser every turn +//! starts from, and continue straight into the UI — no relaunch. //! -//! A store that exists but fails to LOAD is not handled here on purpose: that +//! A store that exists but fails to READ is not handled here on purpose: that //! is an existing configuration broken, and a setup prompt would hide the -//! actual error (see [`caos_cli::model_secret_missing`]). +//! actual error (see [`caos_cli::model_secret_missing`]). Reader expressions +//! are deliberately NOT resolved anywhere in this preflight — resolution +//! evaluates the workspace, and the first turn does it behind the tui's own +//! progress UI instead of a blank terminal. use std::io::{self, BufRead, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; -use caos::{fresh_entropy, GitTransport, SECRETS_DIR}; +use caos::{fresh_entropy, SECRETS_DIR}; use caos_cli::{ ensure_conversation_secret, model_secret_manual_setup, model_secret_missing, MODEL_API_SECRET, MODEL_API_SECRET_READERS, MODEL_API_SECRET_VALUE_FILE, @@ -29,8 +32,8 @@ const MIN_KEY_CHARS: usize = 12; /// when none is configured, ask for one and install it instead of exiting /// with instructions. The caller has already verified stdin/stdout are a /// terminal and the server is reachable. -pub(crate) fn ensure_model_secret(transport: &GitTransport) -> Result<(), String> { - if !model_secret_missing(transport)? { +pub(crate) fn ensure_model_secret() -> Result<(), String> { + if !model_secret_missing()? { return Ok(()); } let cols = terminal_size().map(|(cols, _)| cols).unwrap_or(80); @@ -47,10 +50,12 @@ pub(crate) fn ensure_model_secret(transport: &GitTransport) -> Result<(), String for line in install_model_secret(&root, &key)? { println!("{line}"); } - // Prove the new entry through the loader every turn uses — value read, - // readers resolved, credential present — while failures are still - // readable at the shell prompt. - ensure_conversation_secret(transport)?; + // Prove the new entry through the store parser every turn starts from — + // spec parsed, value read, credential present — while failures are still + // readable at the shell prompt. Readers stay unresolved here too: the + // written readers are constants, and resolving them evaluates the + // workspace, which the first turn does behind its own progress UI. + ensure_conversation_secret()?; println!("{MODEL_API_SECRET} configured; starting the tui"); Ok(()) } diff --git a/rust/crates/caos-cli/src/lib.rs b/rust/crates/caos-cli/src/lib.rs index 9f574f73..b091eb69 100644 --- a/rust/crates/caos-cli/src/lib.rs +++ b/rust/crates/caos-cli/src/lib.rs @@ -17,7 +17,7 @@ use serde_json::{json, Value}; use caos::{ build_secret_store, compute_client_request_with_store, curry_client_object, - eval_workspace_dep_with_store, prepare_client_request_with_store, + eval_workspace_dep_with_store, local_secret_names, prepare_client_request_with_store, run_client_request_with_store, ClientSecret, GitTransport, Transport, CAOS_REMOTE, }; #[cfg(test)] @@ -1102,8 +1102,8 @@ fn resolve_llm( curry_client_object(t, &llm_base, &config).map(|hash| hash.to_string()) } -fn require_model_secret(store: &[ClientSecret]) -> Result<(), String> { - if store.iter().any(|secret| secret.name() == MODEL_API_SECRET) { +fn require_model_secret<'a>(mut names: impl Iterator) -> Result<(), String> { + if names.any(|name| name == MODEL_API_SECRET) { return Ok(()); } Err(format!( @@ -1151,26 +1151,37 @@ pub fn model_secret_manual_setup() -> String { } /// Whether the conversation model credential is absent from an otherwise -/// loadable store — the "offer to set one up" case. Distinct from a store that -/// fails to load at all (unreadable value file, unresolvable reader): that is -/// someone's existing configuration broken, reported as the error it is rather -/// than papered over with a setup prompt. -pub fn model_secret_missing(t: &GitTransport) -> Result { - Ok(build_secret_store(t)? +/// readable store — the "offer to set one up" case. Distinct from a store +/// whose files fail to read at all (a malformed spec, an unreadable value): +/// that is someone's existing configuration broken, reported as the error it +/// is rather than papered over with a setup prompt. +/// +/// Parse-only deliberately ([`local_secret_names`]): resolving a reader runs +/// eval-path over the workspace, which dispatches real computation — the +/// deepening run, and worker builds for `DEEP-DEPS/llm-step`'s inputs — and +/// that took tens of seconds of blank terminal after any build that touched +/// them. The turn that sends a request resolves the full store behind its own +/// progress UI ([`conversation_secret_store`]), where a broken reader is +/// reported just as readably. +pub fn model_secret_missing() -> Result { + Ok(local_secret_names()? .iter() - .all(|secret| secret.name() != MODEL_API_SECRET)) + .all(|name| name != MODEL_API_SECRET)) } fn conversation_secret_store(t: &GitTransport) -> Result, String> { let store = build_secret_store(t)?; - require_model_secret(&store)?; + require_model_secret(store.iter().map(ClientSecret::name))?; Ok(store) } /// Check the model credential before an interactive client takes over the -/// terminal, so setup failures remain readable at the shell prompt. -pub fn ensure_conversation_secret(t: &GitTransport) -> Result<(), String> { - conversation_secret_store(t).map(drop) +/// terminal, so setup failures remain readable at the shell prompt. Parse-only, +/// like [`model_secret_missing`] and for the same reason: the declared names +/// answer "is a key configured?" without evaluating the workspace. +pub fn ensure_conversation_secret() -> Result<(), String> { + let names = local_secret_names()?; + require_model_secret(names.iter().map(String::as_str)) } fn request_is_active(status: &str) -> bool { diff --git a/rust/crates/caos/src/lib.rs b/rust/crates/caos/src/lib.rs index feb120bf..f9fc910d 100644 --- a/rust/crates/caos/src/lib.rs +++ b/rust/crates/caos/src/lib.rs @@ -4269,24 +4269,71 @@ pub fn build_secret_store(t: &dyn Transport) -> Result, String } let pinned = secrets_pinned_tree(t, dir)?; let mut store = Vec::new(); - for (file_name, path) in local_secret_files(dir)? { - let text = std::fs::read_to_string(&path) - .map_err(|e| format!("reading secret {file_name}: {e}"))?; - let spec = parse_local_secret_spec(&file_name, &text)?; - let value = resolve_local_secret_value(&file_name, &path, spec.value)?; - let readers = spec + for secret in load_local_secrets(dir)? { + let readers = secret .readers .iter() .map(|r| resolve_reader_client(t, &pinned, r)) .collect::>()?; store.push(ClientSecret { + name: secret.name, + value: secret.value, + entropy: secret.entropy, + readers, + }); + } + Ok(store) +} + +/// A local secret parsed and value-resolved, its readers still the declared +/// expressions: the transport-free half of [`build_secret_store`]. +struct LocalSecret { + name: String, + value: String, + entropy: String, + readers: Vec, +} + +/// Read every secret file under `dir` — specs parsed, values loaded — without +/// touching a reader. One reading path for [`build_secret_store`] and the +/// parse-only [`local_secret_names`], so the two can never disagree on what +/// the store declares. +fn load_local_secrets(dir: &Path) -> Result, String> { + let mut secrets = Vec::new(); + for (file_name, path) in local_secret_files(dir)? { + let text = std::fs::read_to_string(&path) + .map_err(|e| format!("reading secret {file_name}: {e}"))?; + let spec = parse_local_secret_spec(&file_name, &text)?; + let value = resolve_local_secret_value(&file_name, &path, spec.value)?; + secrets.push(LocalSecret { name: spec.name, value, entropy: spec.entropy.unwrap_or_default(), - readers, + readers: spec.readers, }); } - Ok(store) + Ok(secrets) +} + +/// The names the local store declares, WITHOUT resolving readers: spec files +/// are parsed and their values read (a malformed or half-written store still +/// fails loudly), but no workspace tree is ingested and no reader expression +/// is evaluated. Resolving a reader runs eval-path over the workspace, which +/// dispatches real computation — the deepening run, and worker builds for a +/// reader like `DEEP-DEPS/llm-step` whose entry compiles — so a presence +/// check that resolved would stall an interactive client's startup for as +/// long as those take (tens of seconds after a build that touches their +/// inputs). [`build_secret_store`] remains the loader for anything that sends +/// a request. +pub fn local_secret_names() -> Result, String> { + let dir = Path::new(SECRETS_DIR); + if !dir.is_dir() { + return Ok(Vec::new()); + } + Ok(load_local_secrets(dir)? + .into_iter() + .map(|secret| secret.name) + .collect()) } /// Serialize the store for the `X-Caos-Secrets` header — a JSON array of @@ -4936,7 +4983,71 @@ mod git_transport_tests { #[cfg(test)] mod local_secret_tests { - use super::{parse_local_secret_spec, LocalSecretValue}; + use super::{load_local_secrets, parse_local_secret_spec, LocalSecretValue}; + + fn scratch_store(name: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!( + "caos-local-secrets-{name}-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + std::fs::create_dir_all(&dir).unwrap(); + dir + } + + /// The parse-only load reads specs and values but leaves readers as the + /// declared expressions — nothing here has a transport to resolve with, + /// which is the point: an interactive preflight must know what the store + /// DECLARES without evaluating the workspace. + #[test] + fn loading_reads_specs_and_values_but_never_resolves_readers() { + let dir = scratch_store("load"); + std::fs::write(dir.join(".key-value"), "hunter2").unwrap(); + std::fs::write( + dir.join("api-key"), + "name=anthropic-api-key\nvalue:@=.key-value\nreader=DEEP-DEPS/llm-step\n", + ) + .unwrap(); + std::fs::write(dir.join("inline"), "value=plain\n").unwrap(); + + let secrets = load_local_secrets(&dir).unwrap(); + // Sorted by file name, dotfiles (the value file) skipped as metadata. + assert_eq!(secrets.len(), 2); + assert_eq!(secrets[0].name, "anthropic-api-key"); + assert_eq!(secrets[0].value, "hunter2"); + assert_eq!(secrets[0].entropy, ""); + assert_eq!(secrets[0].readers, ["DEEP-DEPS/llm-step"]); + assert_eq!(secrets[1].name, "inline"); + assert_eq!(secrets[1].value, "plain"); + assert!(secrets[1].readers.is_empty()); + + std::fs::remove_dir_all(dir).unwrap(); + } + + /// A half-written store (spec present, value file missing) is broken + /// configuration, and the parse-only load reports it — presence checks + /// built on it must surface the error, not offer to set up a new key. + #[test] + fn loading_fails_on_an_unreadable_value_file() { + let dir = scratch_store("missing-value"); + std::fs::write( + dir.join("api-key"), + "name=anthropic-api-key\nvalue:@=.absent\n", + ) + .unwrap(); + + // `.err()`, not `unwrap_err`: LocalSecret carries a secret value, so + // it derives no Debug — the same choice ClientSecret makes. + let error = load_local_secrets(&dir) + .err() + .expect("a spec without its value file was accepted"); + assert!(error.contains("value:@=.absent"), "{error}"); + + std::fs::remove_dir_all(dir).unwrap(); + } #[test] fn one_parser_serves_entropy_maintenance_and_runtime_loading() {