diff --git a/docs/tools/subagent.md b/docs/tools/subagent.md index 554458cd..9a729d83 100644 --- a/docs/tools/subagent.md +++ b/docs/tools/subagent.md @@ -71,6 +71,48 @@ after a restart has to re-supply `dir` when the session lives anywhere other than the daemon's own working directory — and a restart is the situation you reach for `continue` in most often. +## Roles + +A `start` may name a **role**: `role: "reviewer"` runs that subagent under +the spawning agent's own `reviewer.md`. Role files live in one directory +per agent, `subagent_roles/` inside that agent's state directory, one +markdown file per role, and the `role` argument is the filename without +its extension. + +One convention directory of named files, because a named set enumerates. +Anything that watches roles — a check for a safety clause that has gone +stale, an operator asking what this agent can dispatch — lists the +directory and has the answer. Per-agent rather than hive-wide: the clauses +a role carries come out of that one agent's own prompt files, and the +daemon only ever spawns on that agent's behalf. + +**The role is the system prompt; the task is the turn — never the other +way round.** The daemon writes the named role's text, alone, into a +per-session file under the harness directory and points +`--append-system-prompt-file` at it — nothing about the task reaches that +file. It reads the task instructions (`prompt_file`) and folds them ahead +of the turn's own prompt instead, the same channel that carries them to +the subagent without a role. A task baked into the system prompt would +re-assert itself as an instruction on every later turn of the session, not +just the one the caller wrote it for — the system prompt is the +subagent's standing identity, not a one-shot channel. The caller's own +task file stays untouched either way. + +### A role with no file refuses the call + +A named role with no file **fails the `start`** and names the roles the +directory does hold. No fallback, no spawn, no reservation — the refusal +lands before the daemon claims the session name, so a caller that fixes +the name may retry it immediately. + +The reason is the state every agent starts in. The directory starts +empty: hyperhive ships no role files, and nothing creates one until an +agent writes its own, so a name with no file behind it counts as the +ordinary first-run answer rather than a rare corruption. Falling back to a spawn +without the role would hand that subagent a prompt missing every clause +the role existed to carry, and would do it quietly. An empty role file +draws the same refusal, for the same reason. + ## Goals, and turns toward them `start` takes an optional `goal`. Without one a session is a single turn, diff --git a/hive-subagent-mcp/src/lib.rs b/hive-subagent-mcp/src/lib.rs index 365eb8cc..1bed0f8f 100644 --- a/hive-subagent-mcp/src/lib.rs +++ b/hive-subagent-mcp/src/lib.rs @@ -19,4 +19,5 @@ pub mod mcp; pub mod mcp_config; pub mod paths; +pub mod role; pub mod session; diff --git a/hive-subagent-mcp/src/mcp.rs b/hive-subagent-mcp/src/mcp.rs index 9d1bf134..21f4d7ee 100644 --- a/hive-subagent-mcp/src/mcp.rs +++ b/hive-subagent-mcp/src/mcp.rs @@ -62,6 +62,16 @@ struct StartArgs { /// file, not an inline string, so a large recipe can't blow past a /// shell argument length limit. prompt_file: String, + /// Which named role this subagent runs as — `reviewer` loads your own + /// `reviewer.md` role file, the short name being the filename without + /// its extension. The role becomes the subagent's system prompt, ahead + /// of the task in `prompt_file`: it says what this subagent *is* and + /// what it must never do, where the task says what to do this once. + /// Roles are per-agent, so the set you can name is your own. Omit it + /// for the task instructions alone. **A role you do not have is an + /// error, not a spawn** — the reply lists the roles you do have. + #[serde(default)] + role: Option, /// Written to the subagent's stdin as its first turn's prompt. Default: /// a generic "carry out your instructions" nudge — the real task detail /// belongs in `prompt_file`, not here. @@ -201,7 +211,9 @@ impl SubagentMcp { configures for it. Pass `goal` to make this a multi-turn run: the daemon re-prompts \ the subagent toward that goal each time a turn ends, up to `max_turns` (default 5), \ stopping early when the subagent reports the goal reached or asks for help. Whichever \ - way it stops, one todo is pushed at the end and `status` says which. See the \ + way it stops, one todo is pushed at the end and `status` says which. Pass `role` to \ + run it as one of your own named roles, whose file becomes its system prompt — a role \ + you do not have refuses the whole call rather than spawning without it. See the \ `base:claude-subagents` skill for when to reach for this." )] fn start(&self, Parameters(args): Parameters) -> String { @@ -212,6 +224,7 @@ impl SubagentMcp { model: args.model, effort: args.effort, prompt_file: args.prompt_file, + role: args.role, trigger: args.trigger, dir: args.dir, goal: args.goal, @@ -546,6 +559,45 @@ mod tests { } } + #[test] + fn a_start_call_that_names_no_role_still_parses() { + // Backwards compatibility at the boundary it actually has to hold + // at: the JSON a caller sends. Every `start` written before roles + // existed names no role, so the argument has to be optional in the + // schema *and* absent-tolerant in the deserializer, which are two + // separate ways this could regress. + let args: StartArgs = serde_json::from_value(serde_json::json!({ + "name": "batch-1", + "prompt_file": "/tmp/prompt.md", + })) + .expect("a pre-role `start` payload still deserializes"); + assert_eq!(args.role, None, "no role named means no role"); + + let schema = + serde_json::to_value(schemars::schema_for!(StartArgs)).expect("a schema serializes"); + let required = schema + .get("required") + .and_then(serde_json::Value::as_array) + .cloned() + .unwrap_or_default(); + assert!( + !required.iter().any(|r| r == "role"), + "`role` must stay optional — a required one breaks every existing call. Required: \ + {required:?}" + ); + } + + #[test] + fn a_start_call_carries_the_role_it_names() { + let args: StartArgs = serde_json::from_value(serde_json::json!({ + "name": "batch-1", + "prompt_file": "/tmp/prompt.md", + "role": "reviewer", + })) + .expect("a payload naming a role deserializes"); + assert_eq!(args.role.as_deref(), Some("reviewer")); + } + #[tokio::test] async fn an_unminted_signal_token_is_a_bare_404() { // No enumeration: a token this daemon never issued gets the same diff --git a/hive-subagent-mcp/src/paths.rs b/hive-subagent-mcp/src/paths.rs index c6140a64..57ef5d71 100644 --- a/hive-subagent-mcp/src/paths.rs +++ b/hive-subagent-mcp/src/paths.rs @@ -30,6 +30,18 @@ pub fn state_dir() -> PathBuf { PathBuf::from(format!("/agents/{label}/state")) } +/// Where this agent's named subagent role files live, one markdown file per +/// role under the agent's own [`state_dir`]. Per-agent rather than +/// hive-wide: the clauses a role carries came out of one agent's own prompt +/// files, and this daemon only ever spawns on behalf of that agent. The +/// directory is a convention, not a registry — it is created by whoever +/// writes a role into it, and its absence simply means this agent ships no +/// roles yet (see `crate::role`). +#[must_use] +pub fn subagent_roles_dir() -> PathBuf { + state_dir().join("subagent_roles") +} + /// Harness-internal scratch dir this daemon writes its own generated /// subagent `--mcp-config` file into (see `crate::mcp_config`). Delegates to /// the shared resolver so this and `hive-agent`'s own harness dir always diff --git a/hive-subagent-mcp/src/role.rs b/hive-subagent-mcp/src/role.rs new file mode 100644 index 00000000..87f000f7 --- /dev/null +++ b/hive-subagent-mcp/src/role.rs @@ -0,0 +1,189 @@ +//! Named roles: the system prompt a subagent runs under, chosen by name at +//! dispatch (`start(role: "reviewer")` loads `reviewer.md`). +//! +//! One markdown file per role in one convention directory +//! ([`crate::paths::subagent_roles_dir`]), because a named set can be +//! listed: enumerability is the reason for the shape, not typing comfort. +//! +//! **A named role that is not there is an error, never a fallback.** No +//! agent ships roles yet, so "named but missing" is the ordinary first-run +//! state, and quietly spawning without the file would hand a subagent a +//! prompt missing every clause the role was there to carry. See +//! `docs/tools/subagent.md`. + +use std::path::{Path, PathBuf}; + +/// Extension every role file carries. Part of the naming contract: the +/// `role` argument is the filename without it. +const ROLE_EXT: &str = "md"; + +/// Load the role named `role` for this agent, from +/// [`crate::paths::subagent_roles_dir`]. +/// +/// # Errors +/// +/// An unusable name, a role with no file, or a file that reads empty — see +/// [`load_from`], which this is the ambient-path wrapper for. +pub fn load(role: &str) -> anyhow::Result { + load_from(&crate::paths::subagent_roles_dir(), role) +} + +/// Load `role` out of `dir`, returning the file's text. +/// +/// # Errors +/// +/// Three refusals, all of them loud on purpose, and all of them before any +/// subagent is spawned: +/// +/// - a name that is not a plain identifier (so `../` never reaches the +/// filesystem, and the argument stays one path segment by construction), +/// - a name with no file in `dir` — the message names what the directory +/// does hold, which is what the one-directory-of-named-files shape buys, +/// - a file that is there but carries nothing, which would put the same +/// clause-free prompt in front of claude as a missing one. +pub fn load_from(dir: &Path, role: &str) -> anyhow::Result { + hive_types::Ident::parse(role) + .map_err(|e| anyhow::anyhow!("invalid role name {role:?}: {e}"))?; + let path = role_path(dir, role); + let body = std::fs::read_to_string(&path).map_err(|e| { + anyhow::anyhow!( + "no role {role:?} for this agent: reading {} failed: {e}. Roles this agent has: {}", + path.display(), + available(dir) + ) + })?; + if body.trim().is_empty() { + anyhow::bail!( + "role {role:?} at {} is empty — refusing to spawn under a role that says nothing", + path.display() + ); + } + Ok(body) +} + +/// Where the file for `role` lives. `role` is a validated identifier by the +/// time this is called, so the join stays inside `dir`. +fn role_path(dir: &Path, role: &str) -> PathBuf { + dir.join(format!("{role}.{ROLE_EXT}")) +} + +/// The role names `dir` holds, comma-separated, for an error message to +/// offer. Says so plainly when there are none — the expected answer until +/// an agent writes its first role file, and far more useful than an empty +/// list that reads like a lookup bug. +fn available(dir: &Path) -> String { + let mut names: Vec = std::fs::read_dir(dir) + .into_iter() + .flatten() + .flatten() + .filter_map(|entry| { + let path = entry.path(); + (path.extension()?.to_str()? == ROLE_EXT) + .then(|| path.file_stem()?.to_str().map(ToOwned::to_owned)) + .flatten() + }) + .collect(); + if names.is_empty() { + return "none — this agent ships no role files".to_owned(); + } + names.sort(); + names.join(", ") +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A roles directory of this test's own, under the usual scratch root. + /// Each test names its own so a parallel run never shares one. + fn roles_dir(tag: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("hive-subagent-roles-test-{tag}")); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("a scratch dir is creatable"); + dir + } + + fn write_role(dir: &Path, role: &str, body: &str) { + std::fs::write(dir.join(format!("{role}.md")), body).expect("a scratch role is writable"); + } + + #[test] + fn a_named_role_loads_its_file_verbatim() { + let dir = roles_dir("loads"); + write_role( + &dir, + "reviewer", + "review the diff, block on anything unsafe", + ); + assert_eq!( + load_from(&dir, "reviewer").expect("a role that exists loads"), + "review the diff, block on anything unsafe" + ); + } + + #[test] + fn an_unknown_role_is_an_error_naming_what_exists() { + // The load-bearing case: with no agent shipping roles yet, a named + // role with no file is the normal first-run state. It must refuse, + // because the alternative is a subagent running under a prompt + // missing every clause the role carried. + let dir = roles_dir("unknown"); + write_role(&dir, "reviewer", "…"); + let err = load_from(&dir, "reviwer") + .expect_err("a misspelled role must not fall back to no role at all") + .to_string(); + assert!( + err.contains("reviwer"), + "the error names what was asked for: {err}" + ); + assert!( + err.contains("reviewer"), + "the error offers the roles that do exist: {err}" + ); + } + + #[test] + fn an_empty_roles_directory_says_so() { + let dir = roles_dir("empty-dir"); + let err = load_from(&dir, "reviewer") + .expect_err("no file, no role") + .to_string(); + assert!( + err.contains("no role files"), + "the first-run state reads as itself, not as a lookup bug: {err}" + ); + } + + #[test] + fn a_role_directory_that_does_not_exist_is_the_same_refusal() { + let dir = roles_dir("absent").join("not-created"); + let err = load_from(&dir, "reviewer") + .expect_err("an absent directory is an absent role") + .to_string(); + assert!(err.contains("reviewer"), "{err}"); + } + + #[test] + fn an_empty_role_file_is_refused_like_a_missing_one() { + let dir = roles_dir("empty-file"); + write_role(&dir, "hollow", " \n\n"); + let err = load_from(&dir, "hollow") + .expect_err("an empty role is no role") + .to_string(); + assert!(err.contains("empty"), "{err}"); + } + + #[test] + fn a_role_name_that_is_not_an_identifier_never_reaches_the_filesystem() { + let dir = roles_dir("traversal"); + for bad in ["../secrets", "reviewer.md", "Reviewer", "a/b", ""] { + let err = load_from(&dir, bad) + .expect_err("only plain identifiers name a role") + .to_string(); + assert!( + err.contains("invalid role name"), + "{bad:?} must be refused as a name, before any read: {err}" + ); + } + } +} diff --git a/hive-subagent-mcp/src/session.rs b/hive-subagent-mcp/src/session.rs index dcb5367b..29fac636 100644 --- a/hive-subagent-mcp/src/session.rs +++ b/hive-subagent-mcp/src/session.rs @@ -76,7 +76,7 @@ use std::collections::HashMap; use std::os::unix::process::ExitStatusExt as _; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex, PoisonError}; use std::time::{Duration, Instant}; @@ -945,6 +945,87 @@ fn build_config( } } +/// Which file `--append-system-prompt-file` is pointed at, and the prompt +/// this turn actually opens with, for one spawn. +/// +/// **The role is the system prompt; the task is the turn — never the +/// other way round.** No role: `prompt_file`, passed through untouched, is +/// the system prompt, and `trigger` is returned as given — the path every +/// `start` took before roles existed, unchanged. A role: its text, alone, +/// becomes the system-prompt file under [`crate::paths::harness_dir`] — +/// nothing about the task reaches it — and the task instructions are read +/// and folded ahead of `trigger` instead, so they still reach the subagent, +/// on the same channel they always have: the turn's own prompt. Per +/// session, like the generated `--mcp-config` beside it, so two concurrent +/// `start`s never share a system-prompt file. +/// +/// # Errors +/// +/// A role naming unreadable task instructions, or a harness dir that +/// cannot be written. Both refuse the spawn rather than dropping either +/// half — see `docs/tools/subagent.md`. +fn system_prompt_and_trigger( + name: &str, + role: Option<&str>, + prompt_file: &str, + trigger: String, +) -> anyhow::Result<(PathBuf, String)> { + // `harness_dir` panics outside a container (`HYPERHIVE_HARNESS_DIR` + // unset) — read lazily, only once a role means it's actually needed, so + // the no-role path never touches it, in tests or otherwise. + let Some(role) = role else { + return Ok((PathBuf::from(prompt_file), trigger)); + }; + compose_prompt( + &crate::paths::harness_dir(), + name, + Some(role), + prompt_file, + trigger, + ) +} + +/// [`system_prompt_and_trigger`] with the harness dir as an argument rather +/// than an ambient container-only environment read — the split a test uses +/// to control where the role-only file lands. +/// +/// # Errors +/// +/// See [`system_prompt_and_trigger`]. +fn compose_prompt( + dir: &Path, + name: &str, + role: Option<&str>, + prompt_file: &str, + trigger: String, +) -> anyhow::Result<(PathBuf, String)> { + let Some(role) = role else { + return Ok((PathBuf::from(prompt_file), trigger)); + }; + let task = std::fs::read_to_string(prompt_file).map_err(|e| { + anyhow::anyhow!("reading the task instructions at {prompt_file} failed: {e}") + })?; + let path = render_role_prompt(dir, name, role)?; + Ok((path, format!("{}\n\n{trigger}", task.trim_end()))) +} + +/// Write `name`'s system-prompt file — `role`'s text, alone — into `dir`, +/// returning the file's path. Split from [`compose_prompt`] so the +/// directory is an argument rather than an ambient container-only +/// environment read. +/// +/// # Errors +/// +/// A `dir` that cannot be created or written. +fn render_role_prompt(dir: &Path, name: &str, role: &str) -> anyhow::Result { + std::fs::create_dir_all(dir) + .map_err(|e| anyhow::anyhow!("creating {} failed: {e}", dir.display()))?; + let path = dir.join(format!("subagent-system-prompt-{name}.md")); + std::fs::write(&path, role) + .map_err(|e| anyhow::anyhow!("writing {} failed: {e}", path.display()))?; + Ok(path) +} + /// The [`SessionStore`] a subagent's turn actually runs against — same /// resolution `hive_claude::Claude` itself uses, so a lookup here can't /// disagree with what the driver does a moment later. @@ -966,6 +1047,11 @@ pub struct StartRequest { pub effort: Option, /// File holding the subagent's task instructions. pub prompt_file: String, + /// Which named role this subagent runs as — a file in this agent's own + /// role directory, named without its extension. `None` is the + /// pre-role shape: the task instructions alone. A name with no file + /// refuses the whole `start` (see [`crate::role`]). + pub role: Option, /// The first turn's prompt. A goal, when given, is appended to it. pub trigger: String, /// Working directory for the session; `None` inherits the daemon's. @@ -1005,6 +1091,10 @@ pub fn start(state: &Arc, req: StartRequest) -> anyhow::Result { let name = req.name.as_str(); validate_name(name)?; check_model(req.model.as_deref(), available_models().as_deref())?; + // Before `reserve`, so a role this agent does not have costs the caller + // an error and nothing else: no name claimed, no session archived, no + // process spawned. + let role = req.role.as_deref().map(crate::role::load).transpose()?; if !state.reserve(name) { anyhow::bail!("subagent `{name}` is already running — use `continue` or `interrupt`"); } @@ -1027,9 +1117,12 @@ pub fn start(state: &Arc, req: StartRequest) -> anyhow::Result { let result = start_reserved( state, name, - req.model, - req.effort, - &req.prompt_file, + SpawnSpec { + model: req.model, + effort: req.effort, + prompt_file: req.prompt_file, + role, + }, trigger, dir.as_deref(), ); @@ -1039,6 +1132,21 @@ pub fn start(state: &Arc, req: StartRequest) -> anyhow::Result { result } +/// How one spawn is shaped, as against which session it is for: the four +/// values `start` has already resolved by the time it commits to running. +/// A struct rather than four more parameters — the role made +/// `start_reserved`'s list long enough to be both unreadable and a lint, +/// the same reason [`StartRequest`] exists. +struct SpawnSpec { + model: Option, + effort: Option, + /// The caller's task-instruction file, as given. + prompt_file: String, + /// The named role's *text*, already loaded — `start` reads it before + /// reserving the name, so an unknown role never gets this far. + role: Option, +} + /// The slow, fallible part of `start`, run only after `reserve` has /// already closed the TOCTOU window — split out so `start` can release the /// reservation on any error path here without duplicating that logic per @@ -1046,18 +1154,18 @@ pub fn start(state: &Arc, req: StartRequest) -> anyhow::Result { fn start_reserved( state: &Arc, name: &str, - model: Option, - effort: Option, - prompt_file: &str, + spec: SpawnSpec, trigger: String, dir: Option<&str>, ) -> anyhow::Result { let signal_url = state.mint_signal_url(name); + let (system_prompt, trigger) = + system_prompt_and_trigger(name, spec.role.as_deref(), &spec.prompt_file, trigger)?; let config = build_config( name, - model, - effort, - Some(prompt_file), + spec.model, + spec.effort, + Some(&system_prompt.to_string_lossy()), dir, Some(&signal_url), ); @@ -1936,6 +2044,7 @@ mod tests { model: None, effort: None, prompt_file: "/tmp/prompt.md".to_owned(), + role: None, trigger: "trigger".to_owned(), dir: None, goal: None, @@ -2032,6 +2141,135 @@ mod tests { ); } + /// A scratch file holding `body`, named after `tag` so parallel tests + /// never share one. + fn scratch_file(tag: &str, body: &str) -> PathBuf { + let path = std::env::temp_dir().join(format!("hive-subagent-session-test-{tag}.md")); + std::fs::write(&path, body).expect("a scratch file is writable"); + path + } + + #[test] + fn without_a_role_the_task_file_is_what_claude_is_pointed_at() { + // The no-role path, pinned: the caller's own path reaches + // `--append-system-prompt-file` unchanged, nothing is rendered on + // the way, and the trigger passes through untouched too — no + // system-prompt file, no extra flag beyond the one every `start` + // written before roles existed already sent. "Unchanged" is the + // whole assertion. + let (path, trigger) = system_prompt_and_trigger("n", None, "/tmp/p.md", "go".to_owned()) + .expect("passing the task file through cannot fail"); + assert_eq!(path, PathBuf::from("/tmp/p.md")); + assert_eq!(trigger, "go", "no role means the trigger is untouched too"); + + let config = build_config("n", None, None, Some(&path.to_string_lossy()), None, None); + let flag = config + .extra_args + .iter() + .position(|a| a == "--append-system-prompt-file") + .expect("the task file is still passed as the appended system prompt"); + assert_eq!( + config.extra_args.get(flag + 1), + Some(&"/tmp/p.md".to_owned()) + ); + } + + #[test] + fn a_role_s_system_prompt_file_holds_the_role_and_never_the_task() { + // A task rendered into the system prompt re-asserts itself as an + // instruction on every later turn of the session, not just the one + // it was written for — the system prompt is the subagent's + // standing identity, not a one-shot channel. Asserting the role is + // present is not enough to catch that regression; a merged file + // would pass that check too. The task's absence from this file is + // the assertion that actually pins the bug. + let task = scratch_file("role-only-task", "rebase the branch and report"); + let dir = std::env::temp_dir().join("hive-subagent-session-test-role-only"); + let (path, _trigger) = compose_prompt( + &dir, + "role-only-run", + Some("you are a reviewer; block on anything unsafe"), + &task.to_string_lossy(), + "go".to_owned(), + ) + .expect("a role and a readable task file compose"); + let body = std::fs::read_to_string(&path).expect("the system-prompt file is readable"); + assert!( + body.contains("you are a reviewer"), + "the role must reach the system prompt: {body:?}" + ); + assert!( + !body.contains("rebase the branch"), + "the task must NOT reach the system-prompt file: {body:?}" + ); + } + + #[test] + fn a_role_still_delivers_the_task_as_the_turns_own_prompt() { + // The other half of the same fix: dropping the task from the + // system prompt must not drop it altogether — it goes back to + // being the turn's prompt, the same channel it reached the + // subagent by before roles existed. + let task = scratch_file("turn-prompt-task", "rebase the branch and report"); + let dir = std::env::temp_dir().join("hive-subagent-session-test-turn-prompt"); + let (_path, trigger) = compose_prompt( + &dir, + "turn-prompt-run", + Some("you are a reviewer"), + &task.to_string_lossy(), + "Carry out the task described in your instructions.".to_owned(), + ) + .expect("a role and a readable task file compose"); + assert!( + trigger.contains("rebase the branch"), + "the task must reach the subagent as the turn's own prompt: {trigger:?}" + ); + } + + #[test] + fn a_role_with_an_unreadable_task_file_refuses_rather_than_dropping_either() { + let dir = std::env::temp_dir().join("hive-subagent-session-test-unreadable"); + let err = compose_prompt( + &dir, + "n", + Some("a role"), + "/tmp/no-such-task-file-here.md", + "go".to_owned(), + ) + .expect_err("an unreadable task file cannot silently become a role-only prompt"); + assert!(err.to_string().contains("task instructions"), "{err}"); + } + + #[test] + fn a_start_naming_an_unknown_role_errors_and_reserves_nothing() { + // The error path end to end: the refusal happens before anything is + // claimed, so an unknown role costs a message and nothing else — + // no reservation to release, no archived prior session, no spawn. + let state = Arc::new(State::new(PathBuf::from("/dev/null"), signal_url())); + let mut req = start_request("unknown-role-run"); + req.role = Some("no-such-role-ships-anywhere".to_owned()); + let err = start(&state, req).expect_err("a role this agent lacks must refuse the start"); + assert!( + err.to_string().contains("no-such-role-ships-anywhere"), + "the refusal names the role that was asked for: {err}" + ); + assert_eq!( + state.occupancy("unknown-role-run"), + None, + "a refused start leaves the name free" + ); + } + + #[test] + fn a_start_naming_an_unusable_role_refuses_before_touching_the_filesystem() { + let state = Arc::new(State::new(PathBuf::from("/dev/null"), signal_url())); + let mut req = start_request("bad-role-name"); + req.role = Some("../../etc/passwd".to_owned()); + let err = start(&state, req).expect_err("a role name that is not an identifier refuses"); + assert!(err.to_string().contains("invalid role name"), "{err}"); + assert_eq!(state.occupancy("bad-role-name"), None); + } + /// The `--tools` value as it reaches the spawned argv. fn spawned_tools(config: &Config) -> &str { let flag = config