diff --git a/CHANGELOG.md b/CHANGELOG.md index ae2b357b..a1940694 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ breaking entries are marked **BREAKING**. `examples_section`, `operations_line`, `command_aliases_line`, and `subcommand_roster`. `help` and `kaish-tools` both render from these, so a second surface cannot drift from the first by omission. +- **Nested verbs for wrapped commands** — `Verb::verb()` declares child verbs, + so `git worktree list` is now declarable. Flags, positionals, tail, and + stdin stay leaf-only, refused at `build()` on a node. See + `docs/wrapped_command.md` for the full grammar. ### Fixed - **`help ` and `kaish-tools ` only rendered one level of diff --git a/crates/kaish-kernel/src/tools/wrapped.rs b/crates/kaish-kernel/src/tools/wrapped.rs index d72fbfed..e2f9392b 100644 --- a/crates/kaish-kernel/src/tools/wrapped.rs +++ b/crates/kaish-kernel/src/tools/wrapped.rs @@ -85,8 +85,12 @@ pub struct PathCheck { #[derive(Debug, Clone, PartialEq, Eq)] #[non_exhaustive] pub struct RenderedCall { - /// The selected verb's declared name; `None` for the root verb. + /// The selected verb's own declared name; `None` for the root verb. + /// Never the full path — `git worktree list` still reports `list` here. pub verb: Option, + /// The full path to the selected verb: `git worktree list`, or the + /// command name alone for the root verb. + pub scope: String, /// The child's argv, without the executable. pub argv: Vec, /// What the child's standard input is connected to. @@ -179,14 +183,16 @@ impl WrappedTool { } } + /// True when every leaf the tool can run declares `json_output()`. A + /// node is never callable, so its own `json_output` (if set) does not + /// count — only the leaves under it decide this. fn every_verb_is_json(&self) -> bool { - let mut verbs = self - .declaration - .root - .iter() - .chain(self.declaration.verbs.iter()) - .peekable(); - verbs.peek().is_some() && verbs.all(|verb| verb.json_output) + let mut any = false; + let mut all = true; + for verb in self.declaration.root.iter().chain(self.declaration.verbs.iter()) { + walk_leaf_json(verb, &mut any, &mut all); + } + any && all } /// Plan a call: parse it, check every constraint, and render the argv. @@ -228,6 +234,7 @@ impl WrappedTool { Ok(RenderedCall { verb: verb.name.clone(), + scope: self.declaration.scope_of_path(&call.verb_path), argv, stdin: verb.stdin, json_output: verb.json_output, @@ -351,10 +358,7 @@ impl WrappedTool { Ok(call) => call, Err(error) => return ExecResult::failure(error.exit_code(), error.to_string()), }; - let label = match &call.verb { - Some(verb) => format!("{} {verb}", self.declaration.name), - None => self.declaration.name.clone(), - }; + let label = call.scope.clone(); // A virtual cwd has no location to spawn in. The same refusal an // external command gets, named for this command. @@ -507,7 +511,22 @@ fn issue(error: &WrappedError, uncertain: bool) -> ValidationIssue { } } -/// The schema for one named verb. +/// Visit every leaf under `verb` (a node's own `json_output`, if set, does +/// not count — only a leaf runs), tracking whether at least one leaf exists +/// and whether every leaf visited so far declares JSON output. +fn walk_leaf_json(verb: &Verb, any: &mut bool, all: &mut bool) { + if verb.verbs.is_empty() { + *any = true; + *all = *all && verb.json_output; + } else { + for child in &verb.verbs { + walk_leaf_json(child, any, all); + } + } +} + +/// The schema for one verb, recursing into its children so a node's schema +/// carries its own leaves as nested subcommands. fn verb_schema(verb: &Verb) -> ToolSchema { let description = match verb.tail { Tail::Forward => append_clause(&verb.about, "forwards undeclared flags"), @@ -523,6 +542,9 @@ fn verb_schema(verb: &Verb) -> ToolSchema { if verb.json_output { schema = schema.with_typed_substitution(); } + for child in &verb.verbs { + schema = schema.subcommand(verb_schema(child)); + } schema } diff --git a/crates/kaish-kernel/src/tools/wrapped/constraint.rs b/crates/kaish-kernel/src/tools/wrapped/constraint.rs index 2f5a3388..15ce0807 100644 --- a/crates/kaish-kernel/src/tools/wrapped/constraint.rs +++ b/crates/kaish-kernel/src/tools/wrapped/constraint.rs @@ -25,7 +25,7 @@ pub(crate) fn check( verb: &Verb, call: &Call, ) -> Vec { - let scope = declaration.scope_of(verb); + let scope = declaration.scope_of_path(&call.verb_path); let mut errors = Vec::new(); for use_ in &call.flags { diff --git a/crates/kaish-kernel/src/tools/wrapped/declaration.rs b/crates/kaish-kernel/src/tools/wrapped/declaration.rs index c882e3ff..eda900d1 100644 --- a/crates/kaish-kernel/src/tools/wrapped/declaration.rs +++ b/crates/kaish-kernel/src/tools/wrapped/declaration.rs @@ -278,6 +278,11 @@ pub struct Verb { /// Usage examples, published through the tool schema. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub(crate) examples: Vec<(String, String)>, + /// Child verbs. A verb with children is a node: it selects among them + /// and is never itself callable. Empty for a leaf, which is the verb + /// that actually runs. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub(crate) verbs: Vec, } impl Verb { @@ -350,10 +355,26 @@ impl Verb { self } + /// Declare a child verb, making this verb a node: `git worktree list` + /// declares `list` as a child of `worktree`. A node selects among its + /// children and is never itself callable — `build()` refuses one that + /// also declares flags, positionals, a tail, or a stdin posture, since + /// those belong on the leaf that actually runs. + pub fn verb(mut self, verb: Verb) -> Self { + self.verbs.push(verb); + self + } + /// The verb's name, or the empty string for the root. pub(crate) fn name_or_root(&self) -> &str { self.name.as_deref().unwrap_or("") } + + /// True when this verb has children — it selects among them and is + /// never itself callable. + pub(crate) fn is_node(&self) -> bool { + !self.verbs.is_empty() + } } /// One executable: pinned path, fixed lead argv, env pins, verbs. @@ -462,43 +483,101 @@ impl WrappedCommand { self.name ); } - if let Some(root) = &self.root - && root.name.is_some() - { - bail!( - "wrapped command '{}' passed a named verb to root(); use Verb::root()", - self.name - ); + if let Some(root) = &self.root { + if root.name.is_some() { + bail!( + "wrapped command '{}' passed a named verb to root(); use Verb::root()", + self.name + ); + } + if root.is_node() { + bail!( + "wrapped command '{}' declares children on the root verb; nest with \ + named verb() calls instead of under root()", + self.name + ); + } } + self.check_verb_list(&self.name, &self.verbs)?; + if let Some(root) = &self.root { + self.check_verb(&self.name, root)?; + } + Ok(()) + } + + /// Check one sibling list under `scope` — the command name at the top + /// level, or a node's full scope (`git worktree`) one level down: every + /// verb is named and none share a name, then each verb's own grammar and + /// its own children, recursively. + fn check_verb_list(&self, scope: &str, verbs: &[Verb]) -> Result<()> { let mut seen: Vec<&str> = Vec::new(); - for verb in &self.verbs { + for verb in verbs { let Some(name) = verb.name.as_deref() else { + if scope == self.name { + bail!( + "wrapped command '{}' passed an unnamed verb to verb(); use root()", + self.name + ); + } bail!( - "wrapped command '{}' passed an unnamed verb to verb(); use root()", + "wrapped command '{}' passed an unnamed verb to '{scope}''s verb(); a \ + nested verb must be named", self.name ); }; if name.is_empty() { - bail!("wrapped command '{}' declares a verb with no name", self.name); + bail!("'{scope}' declares a verb with no name"); } if seen.contains(&name) { - bail!( - "wrapped command '{}' declares the verb '{name}' twice", - self.name - ); + bail!("'{scope}' declares the verb '{name}' twice"); } seen.push(name); } - - for verb in self.root.iter().chain(self.verbs.iter()) { - self.check_verb(verb)?; + for verb in verbs { + self.check_verb(scope, verb)?; } Ok(()) } - fn check_verb(&self, verb: &Verb) -> Result<()> { - let scope = self.scope_of(verb); + fn check_verb(&self, parent_scope: &str, verb: &Verb) -> Result<()> { + let scope = match &verb.name { + Some(name) => format!("{parent_scope} {name}"), + None => parent_scope.to_string(), + }; + + // A node selects among its children and never runs itself, so a + // flag, positional, tail, or stdin posture on it would describe a + // call that never happens. Refuse loudly rather than binding it + // silently to whichever leaf the agent picks. + if verb.is_node() { + if let Some(flag) = verb.flags.first() { + bail!( + "'{scope}' declares the flag '{}' and child verbs; a node cannot run, \ + so a flag belongs on the leaf verb that does", + flag.written_name() + ); + } + if let Some(positional) = verb.positionals.first() { + bail!( + "'{scope}' declares the positional '{}' and child verbs; a node cannot \ + run, so a positional belongs on the leaf verb that does", + positional.name + ); + } + if verb.tail != Tail::default() { + bail!( + "'{scope}' declares a tail and child verbs; a node cannot run, so tail \ + belongs on the leaf verb that does" + ); + } + if verb.stdin != Stdin::default() { + bail!( + "'{scope}' declares a stdin posture and child verbs; a node cannot run, \ + so stdin belongs on the leaf verb that does" + ); + } + } let mut spellings: Vec = Vec::new(); for flag in &verb.flags { @@ -567,6 +646,10 @@ impl WrappedCommand { ); } } + + if verb.is_node() { + self.check_verb_list(&scope, &verb.verbs)?; + } Ok(()) } @@ -617,6 +700,35 @@ impl WrappedCommand { None => self.name.clone(), } } + + /// The full scope of a resolved verb path: `git worktree list` for a + /// two-deep call, `git` for the root or for an empty path. Every level + /// on the way down contributes its name, mirroring [`scope_of`] for a + /// path more than one verb deep. + pub(crate) fn scope_of_path(&self, path: &[usize]) -> String { + let mut scope = self.name.clone(); + let Some((&first, rest)) = path.split_first() else { + return scope; + }; + let Some(mut verb) = self.verbs.get(first) else { + return scope; + }; + if let Some(name) = &verb.name { + scope.push(' '); + scope.push_str(name); + } + for &index in rest { + let Some(child) = verb.verbs.get(index) else { + break; + }; + verb = child; + if let Some(name) = &verb.name { + scope.push(' '); + scope.push_str(name); + } + } + scope + } } /// The name as the child sees it: an explicitly dashed spelling verbatim, diff --git a/crates/kaish-kernel/src/tools/wrapped/error.rs b/crates/kaish-kernel/src/tools/wrapped/error.rs index e03249dc..68b66eac 100644 --- a/crates/kaish-kernel/src/tools/wrapped/error.rs +++ b/crates/kaish-kernel/src/tools/wrapped/error.rs @@ -31,6 +31,31 @@ pub enum WrappedError { allowed: Vec, }, + /// A node (a verb with children) was called with no leaf to run it. A + /// node only selects among its children; it is never itself callable. + #[error("{command}: '{scope}' needs a verb. Allowed: {}", allowed_list(allowed))] + BareNode { + /// The wrapped command's name. + command: String, + /// The node's full path, e.g. `git worktree`. + scope: String, + /// The node's own children, sorted. + allowed: Vec, + }, + + /// The word after a node names none of its children. + #[error("{command}: unknown verb '{word}' for '{scope}'. Allowed: {}", allowed_list(allowed))] + UnknownChildVerb { + /// The wrapped command's name. + command: String, + /// The node's full path, e.g. `git worktree`. + scope: String, + /// The word that named no child. + word: String, + /// The node's own children, sorted — never the top level's. + allowed: Vec, + }, + /// A word in flag position names no declared flag or alias. #[error("{command}: unknown flag '{word}' for '{scope}'. Allowed: {}", allowed_list(allowed))] UnknownFlag { @@ -233,6 +258,8 @@ impl WrappedError { match self { WrappedError::UnknownVerb { command, .. } | WrappedError::MissingVerb { command, .. } + | WrappedError::BareNode { command, .. } + | WrappedError::UnknownChildVerb { command, .. } | WrappedError::UnknownFlag { command, .. } | WrappedError::ClusteredShort { command, .. } | WrappedError::GluedShortValue { command, .. } diff --git a/crates/kaish-kernel/src/tools/wrapped/parse.rs b/crates/kaish-kernel/src/tools/wrapped/parse.rs index a071a647..433f279f 100644 --- a/crates/kaish-kernel/src/tools/wrapped/parse.rs +++ b/crates/kaish-kernel/src/tools/wrapped/parse.rs @@ -106,8 +106,12 @@ pub(crate) enum Item { /// A parsed call, ready to render. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct Call { - /// Index into the declaration's `verbs`; `None` selects the root verb. - pub(crate) verb_index: Option, + /// The path from the top down to the selected leaf: each entry is an + /// index into the previous level's `verbs` (the first into + /// `declaration.verbs`, every one after into the previous verb's own + /// `verbs`). Empty selects the root verb. The root can never be a node + /// (`build()` refuses that), so an empty path always names a leaf. + pub(crate) verb_path: Vec, /// Declared flag occurrences, in source order. `items` points into this. pub(crate) flags: Vec, /// Every word after the verb, in source order. @@ -121,7 +125,7 @@ impl Call { /// A call the parser declines to describe: the verb itself was opaque. fn unjudgeable() -> Self { Self { - verb_index: None, + verb_path: Vec::new(), flags: Vec::new(), items: Vec::new(), uncertain: true, @@ -130,11 +134,21 @@ impl Call { /// The selected verb. pub(crate) fn verb<'d>(&self, declaration: &'d WrappedCommand) -> Option<&'d Verb> { - match self.verb_index { - Some(index) => declaration.verbs.get(index), - None => declaration.root.as_ref(), - } + resolve_verb(declaration, &self.verb_path) + } +} + +/// Walk `path` from the declaration's top-level `verbs` down through each +/// verb's own `verbs`. An empty path selects the root verb. +fn resolve_verb<'d>(declaration: &'d WrappedCommand, path: &[usize]) -> Option<&'d Verb> { + let Some((&first, rest)) = path.split_first() else { + return declaration.root.as_ref(); + }; + let mut verb = declaration.verbs.get(first)?; + for &index in rest { + verb = verb.verbs.get(index)?; } + Some(verb) } /// A refusal, plus whether an unjudgeable word preceded it. @@ -149,19 +163,16 @@ pub(crate) struct ParseError { /// Parse `words` against `declaration`. pub(crate) fn parse(declaration: &WrappedCommand, words: &[Word]) -> Result { - let Some((verb_index, mut index)) = select_verb(declaration, words)? else { + let Some((verb_path, mut index)) = select_verb(declaration, words)? else { return Ok(Call::unjudgeable()); }; - let Some(verb) = (match verb_index { - Some(i) => declaration.verbs.get(i), - None => declaration.root.as_ref(), - }) else { + let Some(verb) = resolve_verb(declaration, &verb_path) else { return Ok(Call::unjudgeable()); }; - let scope = declaration.scope_of(verb); + let scope = declaration.scope_of_path(&verb_path); let mut call = Call { - verb_index, + verb_path, flags: Vec::new(), items: Vec::new(), uncertain: false, @@ -258,12 +269,17 @@ pub(crate) fn parse(declaration: &WrappedCommand, words: &[Word]) -> Result Result, usize)>, ParseError> { +) -> Result, usize)>, ParseError> { if declaration.verbs.is_empty() { - return Ok(Some((None, 0))); + return Ok(Some((Vec::new(), 0))); } let allowed = allowed_verbs(declaration); let missing = |uncertain: bool| ParseError { @@ -276,7 +292,7 @@ fn select_verb( let Some(first) = words.first() else { return match declaration.root { - Some(_) => Ok(Some((None, 0))), + Some(_) => Ok(Some((Vec::new(), 0))), None => Err(missing(false)), }; }; @@ -288,11 +304,15 @@ fn select_verb( .iter() .position(|v| v.name_or_root() == first.text) { - Some(index) => Ok(Some((Some(index), 1))), + Some(index) => { + let verb = &declaration.verbs[index]; + let scope = declaration.scope_of(verb); + descend(declaration, scope, verb, vec![index], words, 1) + } // A declaration with both a root and named verbs falls // through: `python etl.py` runs the root, `python json-tool` // runs the verb. - None if declaration.root.is_some() => Ok(Some((None, 0))), + None if declaration.root.is_some() => Ok(Some((Vec::new(), 0))), None => Err(ParseError { error: WrappedError::UnknownVerb { command: declaration.name.clone(), @@ -306,18 +326,80 @@ fn select_verb( // A flag or `--` where a verb belongs: the root absorbs it if there // is one, otherwise no verb was named. Known::Literal => match declaration.root { - Some(_) => Ok(Some((None, 0))), + Some(_) => Ok(Some((Vec::new(), 0))), None => Err(missing(false)), }, // The verb itself came from an expansion. Which verb's flags apply is // unknowable, so the parser describes nothing. Known::Opaque | Known::OpaqueValue => match declaration.root { - Some(_) => Ok(Some((None, 0))), + Some(_) => Ok(Some((Vec::new(), 0))), None => Ok(None), }, } } +/// After `verb` (at `path`, having consumed `consumed` words) is selected, +/// keep consuming while it has children. A node is never itself callable: +/// the word at `words[consumed]` must select one of them, or the call +/// refuses (nothing there, or a flag/`--` where a child belongs) or — for an +/// opaque word — describes nothing, the same rule a flat declaration already +/// applies at the top level. +fn descend( + declaration: &WrappedCommand, + scope: String, + verb: &Verb, + path: Vec, + words: &[Word], + consumed: usize, +) -> Result, usize)>, ParseError> { + if verb.verbs.is_empty() { + return Ok(Some((path, consumed))); + } + let allowed = allowed_verb_names(&verb.verbs); + let bare_node = || ParseError { + error: WrappedError::BareNode { + command: declaration.name.clone(), + scope: scope.clone(), + allowed: allowed.clone(), + }, + uncertain: false, + }; + + let Some(next) = words.get(consumed) else { + return Err(bare_node()); + }; + + match next.known { + Known::Literal if !next.text.starts_with('-') => { + match verb.verbs.iter().position(|v| v.name_or_root() == next.text) { + Some(index) => { + let child = &verb.verbs[index]; + let child_scope = format!("{scope} {}", next.text); + let mut child_path = path; + child_path.push(index); + descend(declaration, child_scope, child, child_path, words, consumed + 1) + } + None => Err(ParseError { + error: WrappedError::UnknownChildVerb { + command: declaration.name.clone(), + scope, + word: next.text.clone(), + allowed, + }, + uncertain: false, + }), + } + } + // A flag or `--` where a child verb belongs: a node has no root to + // fall through to, so this is the same refusal as no word at all. + Known::Literal => Err(bare_node()), + // The child came from an expansion. Which child's grammar applies is + // unknowable, so the parser describes nothing — the node-depth + // reading of the rule `select_verb` already applies at the top. + Known::Opaque | Known::OpaqueValue => Ok(None), + } +} + /// Bind one word in flag position. fn bind_flag( declaration: &WrappedCommand, @@ -524,13 +606,15 @@ fn fill_positional( } } -/// Every declared verb name, sorted. +/// Every declared top-level verb name, sorted. pub(crate) fn allowed_verbs(declaration: &WrappedCommand) -> Vec { - let mut allowed: Vec = declaration - .verbs - .iter() - .map(|verb| verb.name_or_root().to_string()) - .collect(); + allowed_verb_names(&declaration.verbs) +} + +/// Every verb name in `verbs`, sorted — the siblings at one level, never the +/// top level unless `verbs` is `declaration.verbs` itself. +fn allowed_verb_names(verbs: &[Verb]) -> Vec { + let mut allowed: Vec = verbs.iter().map(|verb| verb.name_or_root().to_string()).collect(); allowed.sort(); allowed } diff --git a/crates/kaish-kernel/src/tools/wrapped/render.rs b/crates/kaish-kernel/src/tools/wrapped/render.rs index 5e260730..7be07782 100644 --- a/crates/kaish-kernel/src/tools/wrapped/render.rs +++ b/crates/kaish-kernel/src/tools/wrapped/render.rs @@ -3,10 +3,14 @@ //! ```text //! argv = command.lead… //! , verb.name (omitted for the root verb, and for omit_name) -//! , verb.lead… +//! , verb.lead… , repeated for every node on the path, then the leaf //! , every word the agent wrote, in source order //! ``` //! +//! A node's name and lead render exactly like a leaf's, in path order: `git +//! worktree list` renders `worktree`, then `list`, with each level's `lead` +//! spliced in right after its own name. +//! //! The executable itself is not in the rendered argv — the spawn site owns //! argv[0]. //! @@ -40,12 +44,14 @@ pub(crate) struct Rendered { pub(crate) fn render(declaration: &WrappedCommand, verb: &Verb, call: &Call) -> Rendered { let mut argv: Vec = Vec::new(); argv.extend(declaration.lead.iter().cloned()); - if !verb.omit_name - && let Some(name) = &verb.name - { - argv.push(name.clone()); + for step in verb_chain(declaration, &call.verb_path) { + if !step.omit_name + && let Some(name) = &step.name + { + argv.push(name.clone()); + } + argv.extend(step.lead.iter().cloned()); } - argv.extend(verb.lead.iter().cloned()); let mut item_argv_index = Vec::with_capacity(call.items.len()); for item in &call.items { @@ -73,6 +79,25 @@ pub(crate) fn render(declaration: &WrappedCommand, verb: &Verb, call: &Call) -> } } +/// The verb at every step from the top down to the leaf `path` selects. +/// Empty for the root (`render` then renders nothing but `declaration.lead` +/// before the agent's own words). Every index in `path` was produced by +/// `parse::select_verb`'s own walk over these same lists, so it is always +/// in range. +fn verb_chain<'d>(declaration: &'d WrappedCommand, path: &[usize]) -> Vec<&'d Verb> { + let Some((&first, rest)) = path.split_first() else { + return declaration.root.iter().collect(); + }; + let mut chain = Vec::with_capacity(rest.len() + 1); + let mut verb = &declaration.verbs[first]; + chain.push(verb); + for &index in rest { + verb = &verb.verbs[index]; + chain.push(verb); + } + chain +} + /// Render one flag occurrence under its declared name, whichever alias the /// agent wrote. fn render_flag(flag: &Flag, value: Option<&str>, argv: &mut Vec) { diff --git a/crates/kaish-kernel/src/tools/wrapped/tests.rs b/crates/kaish-kernel/src/tools/wrapped/tests.rs index 48b263aa..fc26f3a8 100644 --- a/crates/kaish-kernel/src/tools/wrapped/tests.rs +++ b/crates/kaish-kernel/src/tools/wrapped/tests.rs @@ -881,3 +881,148 @@ async fn a_variable_is_opaque_to_validation_and_refused_at_execution() { "git: unknown flag '--output' for 'git log'. Allowed: -n/--max-count, --oneline, --since" ); } + +// ── Nested verbs: build()-time node restrictions ─────────────────────── + +#[test] +fn build_refuses_a_flag_declared_on_a_node() { + let dir = tempfile::tempdir().unwrap(); + let path = executable_at(dir.path(), "probe"); + let error = WrappedCommand::new("git") + .executable(&path) + .verb( + Verb::new("worktree") + .flag(Flag::switch("bad")) + .verb(Verb::new("list")), + ) + .build() + .expect_err("a node cannot run, so it cannot declare a flag"); + let message = error.to_string(); + assert!(message.contains("git worktree"), "{message}"); + assert!(message.contains("--bad"), "{message}"); +} + +#[test] +fn build_refuses_a_positional_declared_on_a_node() { + let dir = tempfile::tempdir().unwrap(); + let path = executable_at(dir.path(), "probe"); + let error = WrappedCommand::new("git") + .executable(&path) + .verb( + Verb::new("worktree") + .positional(Positional::one("path")) + .verb(Verb::new("list")), + ) + .build() + .expect_err("a node cannot run, so it cannot declare a positional"); + let message = error.to_string(); + assert!(message.contains("git worktree"), "{message}"); + assert!(message.contains("path"), "{message}"); +} + +#[test] +fn build_refuses_a_tail_declared_on_a_node() { + let dir = tempfile::tempdir().unwrap(); + let path = executable_at(dir.path(), "probe"); + let error = WrappedCommand::new("git") + .executable(&path) + .verb( + Verb::new("worktree") + .tail(Tail::Forward) + .verb(Verb::new("list")), + ) + .build() + .expect_err("a node cannot run, so it cannot declare a tail"); + assert!(error.to_string().contains("git worktree"), "{error}"); +} + +#[test] +fn build_refuses_a_stdin_posture_declared_on_a_node() { + let dir = tempfile::tempdir().unwrap(); + let path = executable_at(dir.path(), "probe"); + let error = WrappedCommand::new("git") + .executable(&path) + .verb( + Verb::new("worktree") + .stdin(Stdin::Pipe) + .verb(Verb::new("list")), + ) + .build() + .expect_err("a node cannot run, so it cannot declare a stdin posture"); + assert!(error.to_string().contains("git worktree"), "{error}"); +} + +#[test] +fn build_refuses_children_declared_on_the_root_verb() { + let dir = tempfile::tempdir().unwrap(); + let path = executable_at(dir.path(), "probe"); + let error = WrappedCommand::new("probe") + .executable(&path) + .root(Verb::root().verb(Verb::new("list"))) + .build() + .expect_err("root nesting is not supported; use named verb() calls"); + assert!(error.to_string().contains("root"), "{error}"); +} + +#[test] +fn build_refuses_a_duplicate_name_among_a_nodes_children() { + let dir = tempfile::tempdir().unwrap(); + let path = executable_at(dir.path(), "probe"); + let error = WrappedCommand::new("git") + .executable(&path) + .verb( + Verb::new("worktree") + .verb(Verb::new("list")) + .verb(Verb::new("list")), + ) + .build() + .expect_err("two children cannot share a name"); + assert!(error.to_string().contains("'list' twice"), "{error}"); +} + +// ── Nested verbs: the schema recurses ─────────────────────────────────── + +#[test] +fn the_schema_recurses_nested_verbs_as_nested_subcommands() { + let schema = tool( + WrappedCommand::new("git").verb( + Verb::new("worktree") + .verb(Verb::new("add")) + .verb(Verb::new("list")), + ), + ) + .schema(); + let worktree = &schema.subcommands[0]; + assert_eq!(worktree.name, "worktree"); + let names: Vec<&str> = worktree.subcommands.iter().map(|s| s.name.as_str()).collect(); + assert_eq!(names, vec!["add", "list"]); +} + +#[test] +fn typed_substitution_walks_through_a_node_to_its_leaves() { + let all_json = tool(WrappedCommand::new("git").verb(Verb::new("worktree").verb(Verb::new("list").json_output()))) + .schema(); + assert!(all_json.typed_substitution, "every leaf under the node is JSON"); + + let mixed = tool( + WrappedCommand::new("git").verb( + Verb::new("worktree") + .verb(Verb::new("list").json_output()) + .verb(Verb::new("add")), + ), + ) + .schema(); + assert!(!mixed.typed_substitution, "a text leaf keeps the root off"); +} + +// ── Nested verbs: plan_call ───────────────────────────────────────────── + +#[test] +fn plan_call_reports_only_the_leafs_own_name() { + let declaration = WrappedCommand::new("git").verb(Verb::new("worktree").verb(Verb::new("list"))); + let planned = tool(declaration) + .plan_call(&args(&["worktree", "list"])) + .expect("a two-level call should plan"); + assert_eq!(planned.verb.as_deref(), Some("list")); + assert_eq!(planned.scope, "git worktree list"); +} diff --git a/crates/kaish-kernel/tests/wrapped_command_exec_tests.rs b/crates/kaish-kernel/tests/wrapped_command_exec_tests.rs index 1fb02f36..1fe8e9a9 100644 --- a/crates/kaish-kernel/tests/wrapped_command_exec_tests.rs +++ b/crates/kaish-kernel/tests/wrapped_command_exec_tests.rs @@ -739,3 +739,48 @@ async fn a_relative_path_under_value_reaches_the_child_canonical() { "the refusal names the value: {refused}" ); } + +// ── Nested verbs ──────────────────────────────────────────────────────── + +#[tokio::test] +async fn a_nested_verb_spawns_with_the_full_path_and_leads_in_argv() { + let dir = tempfile::tempdir().unwrap(); + let tool = WrappedCommand::new("wgit2") + .executable("/bin/echo") + .lead(["--no-pager"]) + .verb(Verb::new("worktree").verb(Verb::new("list").flag(Flag::switch("porcelain")))) + .build() + .expect("the nested declaration builds"); + let kernel = kernel_with(dir.path(), vec![tool]); + + let result = run(&kernel, "wgit2 worktree list --porcelain").await; + assert_eq!(result.code, 0, "wgit2 worktree list failed: {}", result.err); + assert_eq!( + result.text_out().trim(), + "--no-pager worktree list --porcelain", + "the full path, with every level's lead, must reach the child" + ); +} + +#[tokio::test] +async fn a_bare_node_refuses_before_anything_spawns() { + let dir = tempfile::tempdir().unwrap(); + let tool = WrappedCommand::new("wgit3") + .executable("/bin/echo") + .verb( + Verb::new("worktree") + .verb(Verb::new("add")) + .verb(Verb::new("list")), + ) + .build() + .expect("the nested declaration builds"); + let kernel = kernel_with(dir.path(), vec![tool]); + + // A literal bare node is caught at validation, before the script runs — + // the same lane an unknown verb is refused from. + let refusal = attempt(&kernel, "wgit3 worktree") + .await + .expect_err("a bare node cannot run and never spawns"); + assert!(refusal.contains("needs a verb"), "{refusal}"); + assert!(refusal.contains("add, list"), "{refusal}"); +} diff --git a/crates/kaish-kernel/tests/wrapped_command_parse_tests.rs b/crates/kaish-kernel/tests/wrapped_command_parse_tests.rs index 7b73bac4..983a0817 100644 --- a/crates/kaish-kernel/tests/wrapped_command_parse_tests.rs +++ b/crates/kaish-kernel/tests/wrapped_command_parse_tests.rs @@ -804,3 +804,150 @@ fn a_forwarding_verb_is_marked_in_the_published_schema() { .expect("the verb is published"); assert_eq!(clippy.description, "forwards undeclared flags"); } + +// ── 10. Nested verbs ──────────────────────────────────────────────────── +// +// `git worktree` is undeclarable with one flat level of verbs: `worktree` +// selects among `add`, `list`, `lock`, `prune`, `remove`, and only those +// leaves run. `status` sits alongside `worktree` at the top level so a +// node-scoped refusal has a real top-level name to leak, if it were going +// to leak one. + +fn git_nested() -> Fixture { + build(|path| { + WrappedCommand::new("git") + .executable(path) + .lead(["--no-pager"]) + .verb(Verb::new("status")) + .verb( + Verb::new("worktree") + .verb(Verb::new("add").positional(Positional::one("path").required())) + .verb(Verb::new("list").flag(Flag::switch("porcelain"))) + .verb(Verb::new("lock").positional(Positional::one("path").required())) + .verb(Verb::new("prune")) + .verb(Verb::new("remove").positional(Positional::one("path").required())), + ) + }) +} + +#[test] +fn git_worktree_list_renders_the_full_path_in_order() { + assert_eq!( + git_nested().argv(&["worktree", "list", "--porcelain"]), + vec!["--no-pager", "worktree", "list", "--porcelain"] + ); +} + +#[test] +fn lead_concatenates_down_the_path_in_path_order() { + let fixture = build(|path| { + WrappedCommand::new("probe") + .executable(path) + .lead(["--cmd-lead"]) + .verb( + Verb::new("outer").lead(["--outer-lead"]).verb( + Verb::new("inner") + .lead(["--inner-lead"]) + .positional(Positional::many("rest")), + ), + ) + }); + assert_eq!( + fixture.argv(&["outer", "inner", "x"]), + vec!["--cmd-lead", "outer", "--outer-lead", "inner", "--inner-lead", "x"] + ); +} + +#[test] +fn a_bare_node_refuses_exit_two_and_names_its_children() { + let fixture = git_nested(); + let message = fixture.refuse(&["worktree"]); + assert_eq!( + message, + "git: 'git worktree' needs a verb. Allowed: add, list, lock, prune, remove" + ); + + let mut args = ToolArgs::new(); + args.positional.push(Value::String("worktree".to_string())); + let error = fixture.tool.plan_call(&args).expect_err("a node cannot run bare"); + assert_eq!(error.exit_code(), 2); +} + +#[test] +fn an_unknown_leaf_under_a_node_names_the_nodes_children_not_the_top_level() { + let message = git_nested().refuse(&["worktree", "frobnicate"]); + assert_eq!( + message, + "git: unknown verb 'frobnicate' for 'git worktree'. Allowed: add, list, lock, prune, remove" + ); + // The assertion that actually discriminates: a node-scoped refusal must + // not leak the top level's own verb names. + assert!( + !message.contains("status"), + "the top-level verb 'status' must not appear in a node-scoped error: {message}" + ); +} + +#[test] +fn an_unknown_leaf_at_the_top_level_still_names_the_top_level() { + // `git frobnicate` (no node involved) keeps the original, unscoped shape. + assert_eq!( + git_nested().refuse(&["frobnicate"]), + "git: unknown verb 'frobnicate'. Allowed: status, worktree" + ); +} + +#[test] +fn three_levels_deep_renders_and_scopes_correctly() { + let fixture = build(|path| { + WrappedCommand::new("tool").executable(path).verb( + Verb::new("a").verb( + Verb::new("b").verb( + Verb::new("c") + .flag(Flag::switch("x")) + .positional(Positional::many("rest")), + ), + ), + ) + }); + + assert_eq!( + fixture.argv(&["a", "b", "c", "-x", "y"]), + vec!["a", "b", "c", "-x", "y"] + ); + assert_eq!( + fixture.refuse(&["a", "b", "zzz"]), + "tool: unknown verb 'zzz' for 'tool a b'. Allowed: c" + ); + assert_eq!( + fixture.refuse(&["a", "b"]), + "tool: 'tool a b' needs a verb. Allowed: c" + ); +} + +#[test] +fn a_leaf_under_a_node_still_enforces_its_own_grammar() { + let fixture = git_nested(); + assert_eq!( + fixture.refuse(&["worktree", "add"]), + "git: required argument 'path' not given for 'git worktree add'." + ); + assert_eq!( + fixture.argv(&["worktree", "add", "/tmp/wt"]), + vec!["--no-pager", "worktree", "add", "/tmp/wt"] + ); +} + +#[test] +fn one_level_declarations_are_unaffected_by_nesting_support() { + // `git`'s own flat declaration renders and refuses exactly as it always + // has; nesting is opt-in per verb. + assert_eq!( + git().argv(&["log", "-n", "5"]), + vec!["--no-pager", "log", "--max-count=5"] + ); + assert_eq!( + git().refuse(&["comit", "-m", "x"]), + "git: unknown verb 'comit'. Allowed: commit, diff, log, push, status" + ); +} diff --git a/docs/wrapped_command.md b/docs/wrapped_command.md index 47a521c9..41de8276 100644 --- a/docs/wrapped_command.md +++ b/docs/wrapped_command.md @@ -42,7 +42,7 @@ Five nouns and two enums: | Noun | Meaning | |---|---| | `WrappedCommand` | One executable: pinned path, fixed lead argv, env pins, verbs. | -| `Verb` | A subcommand, or the root for a verb-less program like `python`. | +| `Verb` | A subcommand, or the root for a verb-less program like `python`. A verb may itself declare child verbs (`git worktree list`); see "Nested verbs" below. | | `Flag` | A switch or a value flag, with its render style. | | `Positional` | One or many positional arguments, with optional constraints. | | `Stdin` | `Closed` (default) or `Pipe`. Per verb. | @@ -229,6 +229,57 @@ the verb: `clippy — forwards undeclared flags`. A value that expands to a flag reaches the child under `Forward`; that is the override's cost, and the declaration shows where it was paid. +### Nested verbs: a subcommand group + +`git worktree list`, `docker container ls`, `kubectl get pods` — a subcommand +group is the ordinary shape of a real program. `Verb::verb()` declares a +child, the same builder `WrappedCommand::verb()` uses at the top level: + +```rust +let git = WrappedCommand::new("git") + .executable("/usr/bin/git") + .lead(["--no-pager"]) + .verb( + Verb::new("worktree") + .verb(Verb::new("add").positional(Positional::one("path").required())) + .verb(Verb::new("list").flag(Flag::switch("porcelain"))) + .verb(Verb::new("lock").positional(Positional::one("path").required())) + .verb(Verb::new("prune")) + .verb(Verb::new("remove").positional(Positional::one("path").required())), + ) + .build()?; +``` + +A verb with children is a **node**. A node selects among its children and is +never itself callable — `git worktree` alone is refused, exit 2, before +anything spawns: + +``` +$ git worktree +git: 'git worktree' needs a verb. Allowed: add, list, lock, prune, remove + +$ git worktree frobnicate +git: unknown verb 'frobnicate' for 'git worktree'. Allowed: add, list, lock, prune, remove + +$ git worktree list --porcelain + argv: ["/usr/bin/git", "--no-pager", "worktree", "list", "--porcelain"] +``` + +The allowed set in a node-scoped refusal names that node's own children, +never the top level's — `git worktree frobnicate` never lists `commit`, +`log`, or `push`. That is the property nesting exists to give an agent: the +next word it should try is always drawn from the set it actually chose into. + +`lead` and `omit_name` are meaningful at every level and concatenate down the +path: the command's `lead`, then each node's `name` (unless `omit_name`) and +`lead`, then the leaf's, in that order. Every other property — `flags`, +`positionals`, `tail`, `stdin`, `json_output` — belongs on the leaf that +actually runs. `build()` refuses a node that declares a flag, a positional, a +tail, or a stdin posture, naming the node and the property to move onto a +child. The restriction is deliberate and narrow, kept so it can relax later +without a declaration having worked around its absence in the meantime. The +root verb may not declare children — nest with named `verb()` calls instead. + ### The same declaration as data ```toml @@ -260,7 +311,10 @@ Rules, in order: 1. The first word selects the verb. A `WrappedCommand` with a `root` verb and no named verbs skips this step. A word that names no verb fails: `unknown - verb 'X'. Allowed: …`. Verb names match exactly; no prefix matching. + verb 'X'. Allowed: …`. Verb names match exactly; no prefix matching. When + the selected verb is a node, the next word repeats this step against that + node's own children, and keeps repeating for as long as the selected verb + has children — see "Nested verbs" above. 2. Before `--`, a word that starts with `-` is a flag. It must match a declared name or alias exactly. `--flag=value` and `--flag value` both bind a value flag; `-f value` binds a short alias. Clustered shorts (`-sv`) and glued @@ -296,7 +350,8 @@ with `x = "--output=f"` is an unknown flag, not a positional. To pass a value that starts with `-` as a positional, write `--` before it, as in `sh`. Every parse failure exits 2 and names the verb, the offending word, and the -allowed set. +allowed set. Under a node, the allowed set is that node's own children — +never the top level's, and never a sibling node's. ## Rendering @@ -304,7 +359,7 @@ allowed set. argv = executable , command.lead… , verb.name (omitted for the root verb, and for omit_name) - , verb.lead… + , verb.lead… (repeated for every node on the path, then the leaf) , every word the agent wrote, in source order ``` @@ -422,8 +477,13 @@ and the two names that collide: - No `root` and no `verbs` — the declaration can accept no call. - An empty command name, verb name, flag name, or positional name. - A named verb passed to `root()`, or an unnamed verb passed to `verb()`. -- The same verb name, flag spelling, or positional name declared twice. An +- The same verb name, flag spelling, or positional name declared twice at + one level — a nested level checks only against its own siblings. An alias that shadows another flag's name counts. +- A node (a verb with children) that also declares a flag, a positional, a + tail, or a stdin posture — those belong on the leaf that runs. +- Children declared on the root verb — nest with named `verb()` calls + instead. - `choices([…])` or `int()` on a switch — a switch binds no value. - A `many` positional that is not the last slot. - A `required` positional after an optional one — no call could fill the @@ -489,6 +549,11 @@ kernel in `crates/kaish-kernel/tests/wrapped_command_exec_tests.rs`. 1. Parse and render. Every flag form; `--` placement; each `Tail` mode; repeatable, choices, required, int; unknown verb, verb prefix, flag prefix, clustered and glued shorts; case sensitivity; the root-verb program. + Nested verbs: a leaf's argv with every level's `lead` concatenated in + path order; a bare node's refusal, exit 2, naming its own children; an + unknown leaf under a node naming that node's children and never the top + level's; three levels deep; a node's flag/positional/tail/stdin refused + at `build()`. 2. Injection corpus. Values that look like flags, via literal and via variable; `--` inside a value; empty string; `=` inside a value; unicode; NUL.