style: Cleaned up unnecessary path prefixes, imports, stray scripts, and some documentation

This commit is contained in:
Dark-Alex-17 committed 2026-09-17 12:00:16 -06:00
1 parent 5bd1174ddd
commit fcdcb303a3
26 files changed
+132 -1286

No files matched your search

+17 -26
View File
@@ -113,7 +113,7 @@ pub(crate) fn install_and_reconcile_agent_hooks(
/// swept, while files absent from the manifest always survive.
/// Best-effort: a failure here must not abort the install.
fn sweep_removed_agent_hooks(bundled_files: &HashMap<String, HashSet<String>>) {
let entries = match std::fs::read_dir(paths::agents_data_dir()) {
let entries = match read_dir(paths::agents_data_dir()) {
Ok(entries) => entries,
Err(err) => {
warn!("Failed to scan agents dir for removed-agent hooks: {err}");
@@ -156,7 +156,7 @@ fn sweep_removed_agent_hooks(bundled_files: &HashMap<String, HashSet<String>>) {
}
// remove_dir only succeeds on an empty directory, so a user file
// left behind keeps the directory in place.
let _ = std::fs::remove_dir(&hooks_dir);
let _ = fs::remove_dir(&hooks_dir);
}
}
@@ -184,9 +184,6 @@ impl Agent {
paths::agents_data_dir().display()
);
// Whole-agent installs run Skip (startup) or Force (reinstall); a
// Prompt caller would still get coherent per-file conflict handling,
// scoped to this run.
let mut sticky = StickyMode::None;
let mut written_hooks: HashMap<String, BTreeSet<String>> = HashMap::new();
for file in AgentAssets::iter() {
@@ -283,15 +280,7 @@ impl Agent {
Ok(())
}
/// Refreshes only the bundled agents' hook scripts, leaving every other
/// agent file alone: the hooks asset category must be able to update all
/// hook locations without reinstalling whole agents.
/// Every bundled agent's hooks/ directory reconciles, even when the
/// current embed ships no hooks for it.
pub fn install_builtin_agent_hooks(mode: InstallMode, sticky: &mut StickyMode) -> Result<()> {
// Every bundled agent gets an entry, hooks or not: an agent whose
// hook assets were all dropped still needs its hooks/ directory
// reconciled so manifest-owned leftovers are removed.
let mut shipped: BTreeMap<String, Vec<(String, String)>> = BTreeMap::new();
for file in AgentAssets::iter() {
let Some((agent, rest)) = file.as_ref().split_once('/') else {
@@ -311,6 +300,7 @@ impl Agent {
for (agent, files) in &shipped {
install_and_reconcile_agent_hooks(agent, files, mode, sticky)?;
}
Ok(())
}
@@ -1496,6 +1486,7 @@ pub fn complete_agent_variables(agent_name: &str) -> Vec<(String, Option<String>
#[cfg(test)]
mod tests {
use super::*;
use crate::testing;
#[test]
fn parse_direct_hook_path_accepts_only_direct_hook_children() {
@@ -1610,12 +1601,12 @@ global_hooks:
#[test]
#[serial_test::serial]
fn load_envs_ignores_malformed_global_tools_and_logs_debug() {
crate::testing::install_log_collector();
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");
let _guard = testing::EnvVarGuard::set(&env_name, "{not-json");
config.load_envs(&AppConfig::default());
@@ -1624,7 +1615,7 @@ global_hooks:
vec!["keep_tool.sh"],
"a malformed override must leave the existing value untouched"
);
let debugs = crate::testing::debug_snapshot();
let debugs = testing::debug_snapshot();
assert!(
debugs.iter().any(|message| {
message.contains("Ignoring malformed global_tools env override")
@@ -2076,9 +2067,9 @@ nodes: {}
let _guard = TestConfigDirGuard::new("agent-hooks-prompt");
let shipped = fixture(&[("a.sh", "new content\n"), ("b.sh", "new content\n")]);
let dir = paths::agents_data_dir().join("probe").join("hooks");
std::fs::create_dir_all(&dir).unwrap();
std::fs::write(dir.join("a.sh"), "local a").unwrap();
std::fs::write(dir.join("b.sh"), "local b").unwrap();
create_dir_all(&dir).unwrap();
fs::write(dir.join("a.sh"), "local a").unwrap();
fs::write(dir.join("b.sh"), "local b").unwrap();
// First conflict answered per-file, second by the sticky replace-all.
let script = prompt_script::install(&["replace-all"]);
@@ -2098,7 +2089,7 @@ nodes: {}
// A sticky mode carried in from an earlier location suppresses all
// prompting: `--install-builtins hooks` shares one sticky scope
// across the global, role, and agent hook locations.
std::fs::write(dir.join("a.sh"), "local again").unwrap();
fs::write(dir.join("a.sh"), "local again").unwrap();
let script = prompt_script::install(&[]);
let mut sticky = StickyMode::KeepAll;
install_and_reconcile_agent_hooks("probe", &shipped, InstallMode::Prompt, &mut sticky)
@@ -2121,7 +2112,7 @@ nodes: {}
&mut StickyMode::None,
)
.unwrap();
std::fs::write(dir.join("user.sh"), "user-owned").unwrap();
fs::write(dir.join("user.sh"), "user-owned").unwrap();
install_and_reconcile_agent_hooks(
"probe",
@@ -2182,7 +2173,7 @@ nodes: {}
&mut StickyMode::None,
)
.unwrap();
std::fs::write(dir.join("user.sh"), "user-owned").unwrap();
fs::write(dir.join("user.sh"), "user-owned").unwrap();
Agent::install_builtin_agent_hooks(InstallMode::Skip, &mut StickyMode::None).unwrap();
@@ -2200,14 +2191,14 @@ nodes: {}
let guard = TestConfigDirGuard::new("agent-sweep-symlink");
let real = guard.path.join("agent-elsewhere");
let hooks = real.join("hooks");
std::fs::create_dir_all(&hooks).unwrap();
std::fs::write(
create_dir_all(&hooks).unwrap();
fs::write(
hooks.join(builtin_manifest::BUILTIN_MANIFEST_FILE),
"shipped.sh\n",
)
.unwrap();
std::fs::write(hooks.join("shipped.sh"), "stale").unwrap();
std::fs::create_dir_all(paths::agents_data_dir()).unwrap();
fs::write(hooks.join("shipped.sh"), "stale").unwrap();
create_dir_all(paths::agents_data_dir()).unwrap();
std::os::unix::fs::symlink(&real, paths::agents_data_dir().join("linked")).unwrap();
sweep_removed_agent_hooks(&HashMap::new());
-2
View File
@@ -75,8 +75,6 @@ impl AppState {
}
}
// 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 {
+1 -1
View File
@@ -24,7 +24,7 @@ pub(crate) fn is_builtin_manifest_name(name: &str) -> bool {
}
/// NTFS 8.3 short names (`BUILTI~1`, `builti~1.sh`) can alias any long
/// filename on the same volume — including the manifest itself — so an entry
/// filename on the same volume, including the manifest itself, so an entry
/// shaped like one is never accepted: it could direct a deletion at a file
/// the manifest never named.
fn is_ntfs_short_name_alias(name: &str) -> bool {
-4
View File
@@ -241,10 +241,6 @@ impl BundleStore {
same_name_other_source: None,
};
// Reserved names are the `--install-builtins` categories. `roles` is
// deliberately NOT among them: builtin roles are not installable that
// way (some are coupled to flag-specific coyote behavior), so the
// asymmetry with InstallFilter is intended.
let reserved = AssetCategory::parse(&base).is_some();
let collision = self.source_of_other_bundle(&base, &canonical);
if reserved || collision.is_some() {
+7 -22
View File
@@ -2,7 +2,7 @@
//! silently clobber locally modified files. The bundle installer and the
//! builtin hook installers all funnel per-file conflicts through
//! [`resolve_conflict`], so the prompt wording and the sticky
//! keep-all/replace-all semantics are identical everywhere — including when
//! keep-all/replace-all semantics are identical everywhere, including when
//! one sticky scope spans several install locations in a single run.
use crate::utils::IS_STDOUT_TERMINAL;
@@ -12,17 +12,10 @@ use inquire::Select;
use std::fs;
use std::path::Path;
/// How an installer treats a destination file that already exists.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum InstallMode {
/// Leave existing files untouched. The only mode startup ever uses:
/// startup runs on every launch — including the REPL, which is a
/// terminal — so it must never prompt.
Skip,
/// Overwrite existing files unconditionally.
Force,
/// Write missing files, silently skip identical ones, and ask per
/// differing file.
Prompt,
}
@@ -42,20 +35,12 @@ pub(crate) enum ConflictAction {
Replace,
}
/// What [`resolve_conflict`] does without a terminal: bundle installs bail
/// (an unattended install must not half-apply a plan silently), builtin hook
/// refreshes keep the local file (a scripted refresh must neither clobber
/// local changes nor fail).
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum NonInteractive {
Bail,
Keep,
}
/// Decides whether an installer should overwrite the existing file at `dst`
/// with `content` under `mode`. Only called for files that already exist;
/// missing files are always written without prompting. In `Prompt` mode an
/// unreadable local file counts as differing, so the user decides.
pub(crate) fn should_replace_existing(
dst: &Path,
content: &str,
@@ -337,9 +322,9 @@ mod tests {
fn should_replace_existing_skips_identical_content_without_prompting() {
let _script = prompt_script::install(&[]);
let dir = crate::utils::temp_file("conflict-identical-", "");
std::fs::create_dir_all(&dir).unwrap();
fs::create_dir_all(&dir).unwrap();
let path = dir.join("hook.sh");
std::fs::write(&path, "same content").unwrap();
fs::write(&path, "same content").unwrap();
let mut sticky = StickyMode::None;
let replace = should_replace_existing(
@@ -351,7 +336,7 @@ mod tests {
)
.unwrap();
let _ = std::fs::remove_dir_all(&dir);
let _ = fs::remove_dir_all(&dir);
assert!(!replace);
assert_eq!(prompt_script::prompts_asked(), 0);
}
@@ -361,9 +346,9 @@ mod tests {
fn should_replace_existing_never_prompts_in_skip_or_force_mode() {
let _script = prompt_script::install(&[]);
let dir = crate::utils::temp_file("conflict-modes-", "");
std::fs::create_dir_all(&dir).unwrap();
fs::create_dir_all(&dir).unwrap();
let path = dir.join("hook.sh");
std::fs::write(&path, "local edit").unwrap();
fs::write(&path, "local edit").unwrap();
let mut sticky = StickyMode::None;
let skip =
@@ -371,7 +356,7 @@ mod tests {
let force =
should_replace_existing(&path, "shipped", "hooks", InstallMode::Force, &mut sticky);
let _ = std::fs::remove_dir_all(&dir);
let _ = fs::remove_dir_all(&dir);
assert!(!skip.unwrap());
assert!(force.unwrap());
assert_eq!(prompt_script::prompts_asked(), 0);
-9
View File
@@ -1860,12 +1860,6 @@ fn print_plan_summary(plan: &InstallPlan) {
/// Bundle-shipped hook scripts execute on the user's machine once wired into
/// config, so every planned file landing in a hook location is called out
/// individually. Visibility only: installing them needs no extra approval.
///
/// Only the locations the engine actually resolves hooks from are announced:
/// the top-level hooks category, direct children of `agents/<name>/hooks/`,
/// and direct children of `roles/hooks/`. A `hooks/` directory anywhere else
/// (e.g. inside a skill or macro subtree) is inert data and announcing it
/// would be noise.
fn hook_script_lines(plan: &InstallPlan) -> Vec<String> {
plan.files
.iter()
@@ -2864,9 +2858,6 @@ mod tests {
file("x/config.yaml", TopCategory::Agents),
file("reviewer.md", TopCategory::Roles),
file("hello.yaml", TopCategory::Macros),
// Only the locations the engine resolves hooks from announce:
// a hooks/ directory nested deeper or under another category
// is inert data.
file("x/hooks/nested/deep.sh", TopCategory::Agents),
file("x/sub/hooks/pre.sh", TopCategory::Agents),
file("sub/hooks/extra.sh", TopCategory::Roles),
-5
View File
@@ -83,11 +83,6 @@ pub async fn macro_execute(
.bootstrap_tools(app.as_ref(), true, abort_signal.clone())
.await?;
// An in-macro `.agent` step opens the top-level agent bracket on this
// fresh context, so it must also close here — on the step-failure path
// too, or the run would leak `agent.started` with no terminal event.
// Signal-aware like the headless dispatch arms: a ctrl-c'd macro
// classifies as interrupted, not failed.
let result = async {
for step in &macro_value.steps {
let command = Macro::interpolate_command(step, &variables);
+25 -36
View File
@@ -419,9 +419,6 @@ impl Default for Config {
}
pub fn install_builtins() -> Result<()> {
// Best-effort housekeeping riding along with the startup installers:
// payload files orphaned by crashed processes accumulate in a shared
// temp dir and nothing else ever revisits them.
hooks::sweep_stale_payload_files();
Functions::install_builtin_global_tools(false)?;
Agent::install_builtin_agents(InstallMode::Skip)?;
@@ -530,9 +527,6 @@ pub fn install_assets(category: AssetCategory) -> Result<()> {
AssetCategory::Functions => Functions::install_builtin_global_tools(true)?,
AssetCategory::Skills => Skill::install_builtin_skills(true)?,
AssetCategory::Hooks => {
// One sticky scope spans all three hook locations, so a
// keep-all/replace-all answer carries through the whole refresh,
// matching bundle-install semantics.
let mut sticky = StickyMode::None;
hooks::install_builtin_hooks(InstallMode::Prompt, &mut sticky)?;
Role::install_builtin_role_hooks(InstallMode::Prompt, &mut sticky)?;
@@ -759,8 +753,6 @@ impl Config {
if env::var_os(SANDBOX_ENV_FLAG).is_some() {
if !config_path.exists() {
// Inspection-only flags are look-don't-touch: no first-run
// config creation, nothing written.
if inspection {
return Ok(Self::default());
}
@@ -778,9 +770,6 @@ impl Config {
{
Some(v) => (Self::load_dynamic(&v)?, String::new()),
None => {
// Same look-don't-touch rule: never run the interactive
// first-run wizard (or write a config) for a flag that
// only inspects state.
if inspection {
return Ok(Self::default());
}
@@ -1655,12 +1644,12 @@ hooks:
use std::fs;
use std::time;
let unique = time::SystemTime::now()
let unique = SystemTime::now()
.duration_since(time::UNIX_EPOCH)
.unwrap()
.as_nanos();
let tmp_dir = std::env::temp_dir().join(format!("coyote-sandbox-cfg-{unique}"));
fs::create_dir_all(&tmp_dir).unwrap();
let tmp_dir = env::temp_dir().join(format!("coyote-sandbox-cfg-{unique}"));
create_dir_all(&tmp_dir).unwrap();
let config_path = tmp_dir.join("config.yaml");
fs::write(
@@ -1670,12 +1659,12 @@ hooks:
.unwrap();
let config_env = get_env_name("config_file");
let prev_config = std::env::var_os(&config_env);
let prev_sandbox = std::env::var_os(crate::sandbox::SANDBOX_ENV_FLAG);
let prev_config = env::var_os(&config_env);
let prev_sandbox = env::var_os(SANDBOX_ENV_FLAG);
unsafe {
std::env::set_var(&config_env, &config_path);
std::env::set_var(crate::sandbox::SANDBOX_ENV_FLAG, "1");
env::set_var(&config_env, &config_path);
env::set_var(SANDBOX_ENV_FLAG, "1");
}
let result = Config::load_with_interpolation(false, false).await;
@@ -1683,12 +1672,12 @@ hooks:
unsafe {
match prev_config {
Some(v) => std::env::set_var(&config_env, v),
None => std::env::remove_var(&config_env),
Some(v) => env::set_var(&config_env, v),
None => env::remove_var(&config_env),
}
match prev_sandbox {
Some(v) => std::env::set_var(crate::sandbox::SANDBOX_ENV_FLAG, v),
None => std::env::remove_var(crate::sandbox::SANDBOX_ENV_FLAG),
Some(v) => env::set_var(SANDBOX_ENV_FLAG, v),
None => env::remove_var(SANDBOX_ENV_FLAG),
}
}
let _ = fs::remove_dir_all(&tmp_dir);
@@ -1713,17 +1702,17 @@ hooks:
create_dir_all(&tmp_dir).unwrap();
let _config_dir = crate::testing::EnvVarGuard::set(get_env_name("config_dir"), &tmp_dir);
let _config_file = crate::testing::EnvVarGuard::unset(get_env_name("config_file"));
let _sandbox = crate::testing::EnvVarGuard::unset(crate::sandbox::SANDBOX_ENV_FLAG);
let _sandbox = crate::testing::EnvVarGuard::unset(SANDBOX_ENV_FLAG);
let _provider = crate::testing::EnvVarGuard::unset(get_env_name("provider"));
let _platform = crate::testing::EnvVarGuard::unset(get_env_name("platform"));
let result = Config::load_with_interpolation(true, true).await;
let leftover: Vec<_> = std::fs::read_dir(&tmp_dir)
let leftover: Vec<_> = read_dir(&tmp_dir)
.unwrap()
.map(|entry| entry.unwrap().file_name())
.collect();
let _ = std::fs::remove_dir_all(&tmp_dir);
let _ = fs::remove_dir_all(&tmp_dir);
result.expect("inspection flags must load a default config without a config file");
assert!(
leftover.is_empty(),
@@ -1743,12 +1732,12 @@ hooks:
let config_path = tmp_dir.join("config.yaml");
let _config_file =
crate::testing::EnvVarGuard::set(get_env_name("config_file"), &config_path);
let _sandbox = crate::testing::EnvVarGuard::set(crate::sandbox::SANDBOX_ENV_FLAG, "1");
let _sandbox = crate::testing::EnvVarGuard::set(SANDBOX_ENV_FLAG, "1");
let result = Config::load_with_interpolation(true, true).await;
let created = config_path.exists();
let _ = std::fs::remove_dir_all(&tmp_dir);
let _ = fs::remove_dir_all(&tmp_dir);
result.expect("inspection flags must load a default config in sandbox mode too");
assert!(
!created,
@@ -1835,7 +1824,7 @@ hooks:
let written = write_models_override(vec![entry()]).unwrap();
assert_eq!(written, path);
let mut tmp_name = path.as_os_str().to_os_string();
tmp_name.push(format!(".{}.tmp", std::process::id()));
tmp_name.push(format!(".{}.tmp", process::id()));
assert!(!PathBuf::from(tmp_name).exists());
let loaded = paths::local_models_override().unwrap();
assert_eq!(loaded.len(), 1);
@@ -1910,16 +1899,16 @@ hooks:
assert_eq!(prompt_script::prompts_asked(), 1);
assert_eq!(sticky, StickyMode::ReplaceAll);
assert!(
fs::read_to_string(&notify)
read_to_string(&notify)
.unwrap()
.starts_with("#!/usr/bin/env bash")
);
assert_eq!(
fs::read_to_string(role_dir.join("hook.sh")).unwrap(),
read_to_string(role_dir.join("hook.sh")).unwrap(),
"shipped\n"
);
assert_eq!(
fs::read_to_string(agent_dir.join("hook.sh")).unwrap(),
read_to_string(agent_dir.join("hook.sh")).unwrap(),
"shipped\n"
);
drop(script);
@@ -1931,13 +1920,13 @@ hooks:
refresh(&mut sticky);
assert_eq!(prompt_script::prompts_asked(), 1);
assert_eq!(sticky, StickyMode::KeepAll);
assert_eq!(fs::read_to_string(&notify).unwrap(), "local global");
assert_eq!(read_to_string(&notify).unwrap(), "local global");
assert_eq!(
fs::read_to_string(role_dir.join("hook.sh")).unwrap(),
read_to_string(role_dir.join("hook.sh")).unwrap(),
"local role"
);
assert_eq!(
fs::read_to_string(agent_dir.join("hook.sh")).unwrap(),
read_to_string(agent_dir.join("hook.sh")).unwrap(),
"local agent"
);
drop(script);
@@ -1958,12 +1947,12 @@ hooks:
let script = prompt_script::install(&[]);
install_builtins().unwrap();
assert_eq!(prompt_script::prompts_asked(), 0);
assert_eq!(fs::read_to_string(&notify).unwrap(), "# local edit");
assert_eq!(read_to_string(&notify).unwrap(), "# local edit");
drop(script);
let _script = prompt_script::install_non_interactive();
install_builtins().unwrap();
assert_eq!(prompt_script::prompts_asked(), 0);
assert_eq!(fs::read_to_string(&notify).unwrap(), "# local edit");
assert_eq!(read_to_string(&notify).unwrap(), "# local edit");
}
}
+1 -12
View File
@@ -417,8 +417,6 @@ impl RequestContext {
working_mode: WorkingMode,
info_flag: bool,
) -> Result<Self> {
// 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 {
@@ -4971,15 +4969,6 @@ impl RequestContext {
return;
};
// The abort signal, never the error text, decides interrupted: a
// ctrl-c'd run is not a failure, so no error env rides along. The
// gate is deliberately signal-first -- a latched ctrl-c means
// interrupted even with no error in hand -- unlike
// SpawnResultHooks::fire, which is error-first: a child that finished
// cleanly stays agent.completed under a latched signal because its
// output is collectible. Blind spot: a mid-stream ctrl-d aborts live
// work via poll_abort_signal yet still classifies as agent.completed,
// because ctrl-d always means a deliberate exit (accepted).
if abort_signal.is_some_and(|signal| signal.aborted_ctrlc()) {
hooks::fire(
HookEvent::AgentInterrupted,
@@ -10449,7 +10438,7 @@ mod tests {
// test deadlines. Every spawned script gets its own throwaway lock path.
fn adversary_script_command(script: &str, state: &serde_json::Value) -> Command {
static STACK_LOCK_SEQ: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR"))
let path = Path::new(env!("CARGO_MANIFEST_DIR"))
.join("assets/agents/adversary/scripts")
.join(script);
let lock_path = env::temp_dir().join(format!(