fix(TASK-046): make inspection flags look-don't-touch and pin the contracts

- Skip the builtins bootstrap, function-binary build, and tool-scope
  rebuild for inspection-only runs (--info/--list-*/--mcp-list/
  --list-secrets without --agent/--role): a pristine COYOTE_CONFIG_DIR
  now stays byte-empty (was 269 files written before dispatch)
- Tolerate a model-less default config on the inspection surface via
  AppConfig::from_config_lenient + Model::default() in bootstrap;
  strict 'No available model' behavior unchanged for the REPL path and
  all mutating mcp/vault flags
- --list-secrets degrades to an rc=0 informational message in sandbox
  mode (mutating vault flags still fail there), pinned by test
- Add the 041.7-4 regression test: a malformed COYOTE_<NAME>_GLOBAL_TOOLS
  env override logs the debug message and leaves global_tools untouched
- Adopt scripts/usage-probe-inspection.sh (10 flags: rc=0 + zero writes,
  output required only for --info) and add binary-level
  tests/inspection_flags.rs, incl. the agent+--info still-bootstraps pin
This commit is contained in:
Dark-Alex-17 committed 2026-09-16 15:07:21 -06:00
1 parent 6514588e05
commit 9da0b91673
9 files changed
+382 -24

No files matched your search

+92
View File
@@ -0,0 +1,92 @@
#!/usr/bin/env bash
# Black-box usage-pattern probe for inspection flags on a pristine config dir.
#
# Contract under test (first-run wizard bypass for look-don't-touch flags):
# With COYOTE_CONFIG_DIR pointing at an EMPTY directory, every inspection
# flag (--info, --list-* , --mcp-list, vault list) must:
# 1) exit without prompting (no tty read, no first-run wizard),
# 2) exit 0 (Config::default() fallback); only --info guarantees output
# on stdout — list flags legitimately emit an empty listing on a
# pristine dir,
# 3) write NOTHING into the config dir (no config.yaml, no builtins).
# The plain interactive path (`coyote` on a tty, no flags) must still reach
# the first-run wizard prompt; that is covered manually / by the REPL recipe
# and is intentionally not asserted here (needs a pty).
#
# Usage: scripts/usage-probe-inspection.sh
# Requires: a `coyote` binary on PATH or at ./target/debug/coyote or
# ./target/release/coyote (build with `cargo build --bin coyote` first).
set -euo pipefail
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
BIN="${COYOTE_BIN:-}"
if [ -z "$BIN" ]; then
if [ -x "$REPO_ROOT/target/debug/coyote" ]; then
BIN="$REPO_ROOT/target/debug/coyote"
elif [ -x "$REPO_ROOT/target/release/coyote" ]; then
BIN="$REPO_ROOT/target/release/coyote"
else
echo "No coyote binary found; run 'cargo build --bin coyote' first." >&2
exit 2
fi
fi
WORKDIR="$(mktemp -d)"
trap 'rm -rf "$WORKDIR"' EXIT
failures=0
fail() {
echo "FAIL: $1" >&2
failures=$((failures + 1))
}
# Each flag gets a fresh, empty config dir. stdin is closed so any attempt to
# prompt fails fast; timeout guards against a hang instead of a read error.
probe_flag() {
# $1 = "require-output" or "allow-empty" (stdout expectation),
# $2... = the flag (and any args) to pass
local output_mode="$1"; shift
local desc="$*"
local cfg out rc nwritten
cfg="$(mktemp -d "$WORKDIR/cfg.XXXXXX")"
out="$WORKDIR/out.txt"
rc=0
COYOTE_CONFIG_DIR="$cfg" timeout 30 "$BIN" "$@" </dev/null >"$out" 2>&1 || rc=$?
if [ "$rc" = "124" ]; then
fail "$desc: hung (likely waiting on a prompt) on an empty config dir"
return
fi
if [ "$rc" != "0" ]; then
fail "$desc: expected clean exit 0 on an empty config dir, got rc=$rc, output: $(head -c 200 "$out")"
fi
if [ "$output_mode" = "require-output" ] && [ ! -s "$out" ]; then
fail "$desc: expected some output, got none"
fi
nwritten="$(find "$cfg" -mindepth 1 | wc -l | tr -d ' ')"
if [ "$nwritten" != "0" ]; then
fail "$desc: expected NOTHING written into the empty config dir, found $nwritten entries: $(find "$cfg" -mindepth 1 -maxdepth 1 | tr '\n' ' ')"
fi
}
echo "== Inspection flags on a pristine COYOTE_CONFIG_DIR: no wizard, clean exit, zero writes =="
probe_flag require-output --info
probe_flag allow-empty --list-models
probe_flag allow-empty --list-roles
probe_flag allow-empty --list-sessions
probe_flag allow-empty --list-agents
probe_flag allow-empty --list-macros
probe_flag allow-empty --list-skills
probe_flag allow-empty --list-bundles
probe_flag allow-empty --list-secrets
probe_flag allow-empty --mcp-list
if [ "$failures" != "0" ]; then
echo
echo "$failures inspection-flag scenario(s) FAILED" >&2
exit 1
fi
echo
echo "ALL SCENARIOS PASSED"
+27
View File
@@ -1475,6 +1475,33 @@ global_hooks:
}
}
#[test]
#[serial_test::serial]
fn load_envs_ignores_malformed_global_tools_and_logs_debug() {
crate::testing::install_log_collector();
let name = "gt-malformed-envtest";
let yaml = format!("name: {name}\ninstructions: hi\nglobal_tools:\n - keep_tool.sh\n");
let mut config: AgentConfig = serde_yaml::from_str(&yaml).unwrap();
let env_name = normalize_env_name(&format!("{name}_global_tools"));
let _guard = crate::testing::EnvVarGuard::set(&env_name, "{not-json");
config.load_envs(&AppConfig::default());
assert_eq!(
config.global_tools,
vec!["keep_tool.sh"],
"a malformed override must leave the existing value untouched"
);
let debugs = crate::testing::debug_snapshot();
assert!(
debugs.iter().any(|message| {
message.contains("Ignoring malformed global_tools env override")
&& message.contains(name)
}),
"expected a debug log for the malformed override: {debugs:?}"
);
}
#[test]
fn from_graph_carries_hooks_and_global_hooks() {
let yaml = "\
+30
View File
@@ -210,6 +210,18 @@ impl Default for AppConfig {
impl AppConfig {
pub fn from_config(config: super::Config) -> Result<Self> {
Self::from_config_impl(config, true)
}
/// Like [`Self::from_config`], but tolerates a config with no resolvable
/// model: `model_id` stays empty instead of failing with "No available
/// model". For inspection readouts (--info, --list-*, --mcp-list,
/// --list-secrets), which render model fields as absent/default.
pub fn from_config_lenient(config: super::Config) -> Result<Self> {
Self::from_config_impl(config, false)
}
fn from_config_impl(config: super::Config, require_model: bool) -> Result<Self> {
let mut app_config = Self {
model_id: config.model_id,
temperature: config.temperature,
@@ -306,7 +318,13 @@ impl AppConfig {
}
app_config.setup_document_loaders();
app_config.setup_user_agent();
if require_model {
app_config.resolve_model()?;
} else {
// Model-less is fine here: model_id stays empty and inspection
// callers substitute Model::default().
let _ = app_config.resolve_model();
}
app_config.validate_reasoning_effort()?;
Ok(app_config)
}
@@ -1269,6 +1287,18 @@ mod tests {
assert!(result.is_err());
}
#[test]
fn from_config_errors_when_no_model_available() {
let result = AppConfig::from_config(Config::default());
assert!(result.is_err());
}
#[test]
fn from_config_lenient_tolerates_missing_model() {
let app = AppConfig::from_config_lenient(Config::default()).unwrap();
assert!(app.model_id.is_empty());
}
#[test]
fn resolve_model_keeps_explicit_model_id() {
let mut app = AppConfig {
+8 -1
View File
@@ -43,6 +43,7 @@ impl AppState {
log_path: Option<PathBuf>,
start_mcp_servers: bool,
lenient_vault: bool,
inspection: bool,
abort_signal: AbortSignal,
) -> Result<Self> {
let vault = if lenient_vault {
@@ -74,7 +75,13 @@ impl AppState {
}
}
let mut functions = Functions::init(config.visible_tools.as_deref())?;
// Inspection-only runs are look-don't-touch readouts: skip the
// function-binary prune/build, which writes into the config dir.
let mut functions = if inspection {
Functions::default()
} else {
Functions::init(config.visible_tools.as_deref())?
};
if !mcp_registry.is_empty() && config.mcp_server_support {
functions.append_mcp_meta_functions(mcp_registry.server_features());
}
+7 -1
View File
@@ -417,7 +417,13 @@ impl RequestContext {
working_mode: WorkingMode,
info_flag: bool,
) -> Result<Self> {
let model = Model::retrieve_model(&app.config, &app.config.model_id, ModelType::Chat)?;
// A lenient AppConfig (inspection-only run, no resolvable model)
// leaves model_id empty; render those readouts with a default model.
let model = if info_flag && app.config.model_id.is_empty() {
Model::default()
} else {
Model::retrieve_model(&app.config, &app.config.model_id, ModelType::Chat)?
};
let mut functions = app.functions.clone();
if working_mode.is_repl() {
+45 -6
View File
@@ -140,7 +140,16 @@ async fn main() -> Result<()> {
return config::list_installed_bundles();
}
// Inspection flags are look-don't-touch readouts: skip the builtins
// bootstrap so a pristine config dir stays empty. On such a dir the
// list flags list nothing because nothing is installed, by design.
// Agent/role runs still bootstrap: they load builtin tool definitions.
let inspection_only = (info_flag || cli.list_secrets || cli.mcp_list)
&& cli.agent.is_none()
&& cli.role.is_none();
if !inspection_only {
install_builtins()?;
}
if let Some(category) = cli.install_builtins {
return config::install_assets(category);
@@ -226,7 +235,15 @@ async fn main() -> Result<()> {
cli.mcp_list || cli.mcp_get.is_some() || cli.mcp_remove.is_some() || cli.mcp_add.is_some();
if mcp_action {
let cfg = Config::load_with_interpolation(true).await?;
let app_config = AppConfig::from_config(cfg)?;
let app_config = if cli.mcp_list
&& cli.mcp_get.is_none()
&& cli.mcp_remove.is_none()
&& cli.mcp_add.is_none()
{
AppConfig::from_config_lenient(cfg)?
} else {
AppConfig::from_config(cfg)?
};
let vault = Vault::init(&app_config)?;
mcp::manage::handle(&cli, &vault)?;
@@ -236,7 +253,16 @@ async fn main() -> Result<()> {
if vault_flags {
let cfg = Config::load_with_interpolation(true).await?;
let app_config = AppConfig::from_config(cfg)?;
let app_config = if cli.list_secrets
&& cli.add_secret.is_none()
&& cli.get_secret.is_none()
&& cli.update_secret.is_none()
&& cli.delete_secret.is_none()
{
AppConfig::from_config_lenient(cfg)?
} else {
AppConfig::from_config(cfg)?
};
let vault = Vault::init(&app_config)?;
return Vault::handle_vault_flags(cli, &vault);
}
@@ -244,7 +270,11 @@ async fn main() -> Result<()> {
let abort_signal = create_abort_signal();
let start_mcp_servers = cli.agent.is_none() && cli.role.is_none();
let cfg = Config::load_with_interpolation(info_flag).await?;
let mut app_config = AppConfig::from_config(cfg)?;
let mut app_config = if info_flag {
AppConfig::from_config_lenient(cfg)?
} else {
AppConfig::from_config(cfg)?
};
if cli.no_workspace_mcp {
app_config.no_workspace_mcp = true;
}
@@ -258,14 +288,20 @@ async fn main() -> Result<()> {
log_path,
start_mcp_servers,
info_flag,
inspection_only,
abort_signal.clone(),
)
.await?,
);
let mut ctx = RequestContext::bootstrap(app_state, working_mode, info_flag)?;
let app_config = Arc::clone(&ctx.app.config);
// Rebuilding the tool scope prunes and builds function binaries;
// inspection-only runs are readouts that never execute tools, so skip
// it there too.
if !inspection_only {
ctx.bootstrap_tools(&app_config, start_mcp_servers, abort_signal.clone())
.await?;
}
{
let app = &*ctx.app.config;
@@ -306,9 +342,12 @@ async fn run(
}
if cli.list_models {
for model in list_models(ctx.app.config.as_ref(), ModelType::Chat) {
println!("{}", model.id());
}
let models = list_models(ctx.app.config.as_ref(), ModelType::Chat)
.iter()
.map(|model| model.id())
.collect::<Vec<_>>()
.join("\n");
println!("{models}");
return Ok(());
}
if cli.list_roles {
+22 -16
View File
@@ -9,25 +9,30 @@ struct TestLogCollector;
static WARN_MESSAGES: OnceLock<Mutex<Vec<String>>> = OnceLock::new();
static DEBUG_MESSAGES: OnceLock<Mutex<Vec<String>>> = OnceLock::new();
/// Debug capture is restricted to the hooks module so the buffer is not
/// flooded by debug output from the rest of the crate. Matching is
/// module-boundary-aware: only this exact module or its `::` submodules
/// Debug capture is restricted to the modules whose tests assert on debug
/// output so the buffer is not flooded by the rest of the crate. Matching is
/// module-boundary-aware: only these exact modules or their `::` submodules
/// qualify, so a sibling like `hooks_registry` would not.
const DEBUG_TARGET_PREFIX: &str = concat!(env!("CARGO_CRATE_NAME"), "::hooks");
const DEBUG_TARGET_PREFIXES: [&str; 2] = [
concat!(env!("CARGO_CRATE_NAME"), "::hooks"),
concat!(env!("CARGO_CRATE_NAME"), "::config::agent"),
];
fn captures_warn(metadata: &Metadata) -> bool {
metadata.level() <= Level::Warn
}
fn captures_hooks_debug(metadata: &Metadata) -> bool {
fn captures_module_debug(metadata: &Metadata) -> bool {
if metadata.level() != Level::Debug {
return false;
}
let target = metadata.target();
target == DEBUG_TARGET_PREFIX
DEBUG_TARGET_PREFIXES.iter().any(|prefix| {
target == *prefix
|| target
.strip_prefix(DEBUG_TARGET_PREFIX)
.strip_prefix(prefix)
.is_some_and(|rest| rest.starts_with("::"))
})
}
/// Every warn- or error-level message captured since the collector was
@@ -38,9 +43,9 @@ pub(crate) fn warn_messages() -> &'static Mutex<Vec<String>> {
WARN_MESSAGES.get_or_init(Mutex::default)
}
/// Every debug-level message from the hooks module captured since the
/// collector was installed. Same marker-filtering discipline as
/// [`warn_messages`].
/// Every debug-level message from the [`DEBUG_TARGET_PREFIXES`] modules
/// captured since the collector was installed. Same marker-filtering
/// discipline as [`warn_messages`].
pub(crate) fn debug_messages() -> &'static Mutex<Vec<String>> {
DEBUG_MESSAGES.get_or_init(Mutex::default)
}
@@ -66,7 +71,7 @@ fn snapshot_of(buffer: &Mutex<Vec<String>>) -> Vec<String> {
impl Log for TestLogCollector {
fn enabled(&self, metadata: &Metadata) -> bool {
captures_warn(metadata) || captures_hooks_debug(metadata)
captures_warn(metadata) || captures_module_debug(metadata)
}
// A test that panics while holding a buffer poisons its mutex; the logger
@@ -77,7 +82,7 @@ impl Log for TestLogCollector {
.lock()
.unwrap_or_else(|poisoned| poisoned.into_inner())
.push(record.args().to_string());
} else if captures_hooks_debug(record.metadata()) {
} else if captures_module_debug(record.metadata()) {
debug_messages()
.lock()
.unwrap_or_else(|poisoned| poisoned.into_inner())
@@ -89,10 +94,11 @@ impl Log for TestLogCollector {
}
/// Installs the process-wide log collector capturing warn-level messages and
/// hooks-module debug messages. `log::set_logger` accepts one logger per
/// process, so every test that captures either stream must install through
/// this shared entry point. The max level is Debug so the hooks debug capture
/// sees its records; warn capture is unaffected.
/// debug messages from the [`DEBUG_TARGET_PREFIXES`] modules.
/// `log::set_logger` accepts one logger per process, so every test that
/// captures either stream must install through this shared entry point. The
/// max level is Debug so the debug capture sees its records; warn capture is
/// unaffected.
pub(crate) fn install_log_collector() {
static INSTALL: Once = Once::new();
INSTALL.call_once(|| {
+27
View File
@@ -332,8 +332,16 @@ impl Vault {
}
if cli.list_secrets {
// Listing is an inspection readout: report sandbox unavailability
// instead of failing like the mutating vault flags do.
if vault.sandbox_mode {
println!(
"The vault is unavailable in sandbox mode. Use `coyote --list-secrets` on your host."
);
} else {
vault.list_secrets(true)?;
}
}
Ok(())
}
@@ -410,4 +418,23 @@ mod tests {
}
}
}
#[test]
fn list_secrets_flag_ok_in_sandbox() {
use clap::Parser;
// Built directly rather than via IS_SANDBOX so this test cannot race
// the env juggling in vault_disabled_in_sandbox.
let vault = Vault {
sandbox_mode: true,
..Vault::default()
};
let cli = Cli::try_parse_from(["coyote", "--list-secrets"]).unwrap();
// Pins the --list-secrets sandbox contract: rc=0 with an informational
// readout instead of the hard failure the mutating flags keep. The
// message goes straight to stdout, so only Ok is asserted here.
Vault::handle_vault_flags(cli, &vault)
.expect("--list-secrets must succeed in sandbox mode");
}
}
+124
View File
@@ -0,0 +1,124 @@
//! Binary-level contract for inspection flags on a pristine config dir:
//! exit 0 and NOTHING written into the config dir (no first-run wizard,
//! no builtins bootstrap). Only --info guarantees output on stdout; the
//! list flags legitimately emit an empty listing on a pristine dir, so
//! their contract is exit 0 + zero writes. Vault-backed flags like
//! --list-secrets depend on host state and are covered by
//! scripts/usage-probe-inspection.sh instead. Agent runs, by contrast,
//! must still bootstrap builtins even when combined with --info — see
//! agent_info_on_empty_config_dir_bootstraps_builtins.
use std::env;
use std::fs;
use std::path::PathBuf;
use std::process::{Command, Stdio};
use std::time::{SystemTime, UNIX_EPOCH};
fn fresh_config_dir(label: &str) -> PathBuf {
let unique = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.as_nanos();
let tmp_dir = env::temp_dir().join(format!("coyote-inspection-{label}-{unique}"));
fs::create_dir_all(&tmp_dir).unwrap();
tmp_dir
}
fn probe_inspection_flag(flag: &str, require_stdout: bool) {
let tmp_dir = fresh_config_dir(flag.trim_start_matches("--"));
let output = Command::new(env!("CARGO_BIN_EXE_coyote"))
.arg(flag)
.env("COYOTE_CONFIG_DIR", &tmp_dir)
.stdin(Stdio::null())
.output()
.unwrap();
let leftover: Vec<_> = fs::read_dir(&tmp_dir)
.unwrap()
.map(|entry| entry.unwrap().file_name())
.collect();
let _ = fs::remove_dir_all(&tmp_dir);
assert!(
output.status.success(),
"{flag}: expected exit 0 on an empty config dir, got {:?}\nstdout: {}\nstderr: {}",
output.status,
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
if require_stdout {
assert!(
!output.stdout.is_empty(),
"{flag}: expected some output on stdout"
);
}
assert!(
leftover.is_empty(),
"{flag}: inspection flags must write nothing into the config dir: {leftover:?}"
);
}
#[test]
fn info_on_empty_config_dir_writes_nothing() {
probe_inspection_flag("--info", true);
}
// List flags on a pristine dir correctly print an empty listing, so the
// contract is exit 0 + zero writes only (no stdout assertion).
#[test]
fn list_models_on_empty_config_dir_writes_nothing() {
probe_inspection_flag("--list-models", false);
}
#[test]
fn list_agents_on_empty_config_dir_writes_nothing() {
probe_inspection_flag("--list-agents", false);
}
#[test]
fn mcp_list_on_empty_config_dir_writes_nothing() {
probe_inspection_flag("--mcp-list", false);
}
// The wizard bypass must not swallow the builtins bootstrap for agent runs:
// `--agent <builtin> --info` on a totally fresh config dir has to install
// the bundled definitions before it can render the agent readout.
#[test]
fn agent_info_on_empty_config_dir_bootstraps_builtins() {
let tmp_dir = fresh_config_dir("agent-info");
let output = Command::new(env!("CARGO_BIN_EXE_coyote"))
.args(["--agent", "adversary", "--info"])
.env("COYOTE_CONFIG_DIR", &tmp_dir)
.stdin(Stdio::null())
.output()
.unwrap();
let agents_dir = tmp_dir.join("agents");
let builtins_installed =
agents_dir.is_dir() && fs::read_dir(&agents_dir).unwrap().next().is_some();
let _ = fs::remove_dir_all(&tmp_dir);
assert!(
output.status.success(),
"--agent adversary --info: expected exit 0 on an empty config dir, got {:?}\nstdout: {}\nstderr: {}",
output.status,
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
assert!(
builtins_installed,
"--agent adversary --info: expected builtin agents installed into {}",
agents_dir.display()
);
// The --info readout echoes the loaded agent definition including its
// global_tools, so a builtin tool name there proves the definitions were
// loaded into the agent's tool scope, not just written to disk.
let stdout = String::from_utf8_lossy(&output.stdout);
assert!(
stdout.contains("fs_cat.sh"),
"--agent adversary --info: expected builtin tool fs_cat.sh in the readout\nstdout: {stdout}"
);
}