From 9439370183b588dfda6c56f560ecc0bee15d241f Mon Sep 17 00:00:00 2001 From: A Tobey Date: Tue, 1 Sep 2026 09:16:39 -0400 Subject: [PATCH 1/5] fix: help recurses into nested subcommands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem: kaish-extras reported that `help git` printed the worktree node and stopped, never naming `list` as its verb or showing its flags. The loop in `tool_help` (crates/kaish-help/src/topic.rs:126) walked `schema.subcommands` one level and rendered each `sub.name` without ever looking at `sub.subcommands`. Evidence: `ToolSchema.subcommands` is a recursive `Vec` carried over serde, so `tools --json` already expresses arbitrarily deep grammars. The renderer disagreed with the structured surface it was supposed to describe, and every OTHER tool renders fully, which made the gap look like a complete document rather than a bug. This matters most for wrapped commands (kaish_kernel::tools::wrapped), 0.17.0's headline feature, which can declare grammars deeper than one level. Wrote a failing test first: a two-level `ToolSchema` (git -> worktree -> list, with a parameter on the leaf). It failed exactly as predicted — content stopped at "worktree — Work with the repository's working trees" and never named `list` or its parameter. Decision: replace the one-level loop with `push_subcommand_roster`, a recursive helper that renders one line per subcommand at ANY depth, each carrying its FULL PATH ("worktree list — ...") rather than adding an indent level per depth. kaish-extras parses this roster structurally: lines at exactly two spaces, split on the " — " separator (space, em-dash, space). A deeper indent or a different separator would have broken their parser even while fixing the missing verb. Added a three-level test (kj context session list) to confirm depth is not capped at two, and a control test that a flat tool (no subcommands) renders unchanged. Rule now in force: a subcommand roster line is always the full path from the tool name to that node, at a fixed two-space indent, joined to its description with " — ", regardless of how deep the grammar nests. Surveyed the other two tool-listing surfaces named in review: format_tool_list in crates/kaish-help/src/topic.rs (`help builtins`) and format_tool_list in crates/kaish-kernel/src/tools/builtin/introspect.rs (`kaish-tools` with no argument) are both flat one-line-per-tool rosters that never attempted to walk subcommands, so they don't share this defect. format_tool_detail in introspect.rs (`kaish-tools `) is a related but more severe gap: it doesn't render subcommands at all, at any depth, not just one level. Left unchanged pending a decision on scope. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 5 ++ crates/kaish-help/src/topic.rs | 137 ++++++++++++++++++++++++++++++--- 2 files changed, 133 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb1469a7..f782bb0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ breaking entries are marked **BREAKING**. ## [Unreleased] +### Fixed +- **`help ` only rendered one level of subcommands**, hiding a nested + verb (`worktree list`) and its flags. It now recurses to any depth, still + one flat two-space line per full path (`worktree list — …`). + ## [0.17.0] - 2026-08-31 ### Changed diff --git a/crates/kaish-help/src/topic.rs b/crates/kaish-help/src/topic.rs index 7a7846a0..fbc8a07c 100644 --- a/crates/kaish-help/src/topic.rs +++ b/crates/kaish-help/src/topic.rs @@ -120,19 +120,12 @@ pub fn tool_help(name: &str, schemas: &[ToolSchema]) -> Option { } // A subcommand-aware tool (`kj`, every wrapped command) keeps its real - // grammar here, one level down. Without this the whole allowlist a + // grammar here, at any depth. Without this the whole allowlist a // wrapped command publishes — its verbs, their flags, and the // constraints in their descriptions — was invisible to `help`. if !schema.subcommands.is_empty() { output.push_str("\nSubcommands:\n"); - for sub in &schema.subcommands { - if sub.description.is_empty() { - output.push_str(&format!(" {}\n", sub.name)); - } else { - output.push_str(&format!(" {} — {}\n", sub.name, sub.description)); - } - push_params(&mut output, &sub.params, " "); - } + push_subcommand_roster(&mut output, "", &schema.subcommands); } if !schema.examples.is_empty() { @@ -166,6 +159,33 @@ fn push_params(output: &mut String, params: &[kaish_types::ParamSchema], indent: } } +/// One flat roster line per subcommand at any depth, plus its parameters. +/// +/// `ToolSchema::subcommands` is recursive — a node (`worktree`) can hold a +/// leaf (`list`) that holds another node — but the roster stays flat: every +/// line renders the full path (`worktree list`) at the same two-space +/// indent, never a deeper indent per level. kaish-extras parses this roster +/// by column: exactly two spaces, then the ` — ` (space, em-dash, space) +/// separator. A nested indent or a different separator breaks that reader. +fn push_subcommand_roster(output: &mut String, prefix: &str, subs: &[ToolSchema]) { + for sub in subs { + let path = if prefix.is_empty() { + sub.name.clone() + } else { + format!("{prefix} {}", sub.name) + }; + if sub.description.is_empty() { + output.push_str(&format!(" {path}\n")); + } else { + output.push_str(&format!(" {path} — {}\n", sub.description)); + } + push_params(output, &sub.params, " "); + if !sub.subcommands.is_empty() { + push_subcommand_roster(output, &path, &sub.subcommands); + } + } +} + /// Format help for a single tool. fn format_tool_help(name: &str, schemas: &[ToolSchema]) -> String { tool_help(name, schemas).unwrap_or_else(|| { @@ -220,6 +240,105 @@ pub fn list_topics() -> Vec<(&'static str, &'static str)> { #[cfg(test)] mod tests { use super::*; + use kaish_types::ParamSchema; + + /// A two-level grammar (`git worktree list --porcelain`) — the node + /// (`worktree`) has no params of its own, the leaf (`list`) does. + fn nested_tool_schema() -> ToolSchema { + let leaf = ToolSchema::new("list", "List the repository's working trees").param( + ParamSchema::optional( + "porcelain", + "bool", + kaish_types::Value::Bool(false), + "Machine-readable output", + ), + ); + let node = ToolSchema::new("worktree", "Work with the repository's working trees").subcommand(leaf); + ToolSchema::new("git", "Git plumbing and porcelain").subcommand(node) + } + + #[test] + fn test_tool_help_recurses_into_nested_subcommands() { + let schema = nested_tool_schema(); + let content = tool_help("git", std::slice::from_ref(&schema)).expect("git is registered"); + + // The leaf's full path names the actual verb, not just the node. + assert!( + content.contains("worktree list — List the repository's working trees"), + "expected full-path leaf line, got:\n{content}" + ); + // The leaf's parameter renders too. + assert!( + content.contains("porcelain"), + "expected leaf parameter to render, got:\n{content}" + ); + assert!( + content.contains("Machine-readable output"), + "expected leaf parameter description to render, got:\n{content}" + ); + + // Flat-roster contract: every roster line is exactly two spaces of + // indent, path and description joined by " — " (space, em-dash, + // space) — kaish-extras parses this shape. + let roster_start = content.find("Subcommands:\n").expect("Subcommands section") + "Subcommands:\n".len(); + for line in content[roster_start..].lines() { + if line.is_empty() || line.starts_with(" ") || line.starts_with("Examples:") { + continue; // param line, or past the roster + } + assert!( + line.starts_with(" ") && !line.starts_with(" "), + "roster line must start with exactly two spaces: {line:?}" + ); + assert!( + line.contains(" — "), + "roster line must use the ' — ' separator: {line:?}" + ); + } + } + + #[test] + fn test_tool_help_recurses_three_levels() { + // A wrapped command can declare grammar deeper than two levels + // (`kj context session list --active`) — depth must not cap at 2. + let leaf = ToolSchema::new("list", "List sessions in this context").param( + ParamSchema::optional( + "active", + "bool", + kaish_types::Value::Bool(false), + "Only running sessions", + ), + ); + let session = ToolSchema::new("session", "Session operations").subcommand(leaf); + let context = ToolSchema::new("context", "Context operations").subcommand(session); + let schema = ToolSchema::new("kj", "kaijutsu control").subcommand(context); + + let content = tool_help("kj", std::slice::from_ref(&schema)).expect("kj is registered"); + assert!( + content.contains("context session list — List sessions in this context"), + "expected three-level full-path leaf line, got:\n{content}" + ); + assert!(content.contains("active"), "expected leaf parameter to render, got:\n{content}"); + + let roster_start = content.find("Subcommands:\n").expect("Subcommands section") + "Subcommands:\n".len(); + for line in content[roster_start..].lines() { + if line.contains(" — ") { + assert!( + line.starts_with(" ") && !line.starts_with(" "), + "roster line must stay at exactly two spaces regardless of depth: {line:?}" + ); + } + } + } + + #[test] + fn test_tool_help_flat_tool_unchanged() { + // Control: a tool with no subcommands renders exactly as before — + // no "Subcommands:" section at all. + let schema = ToolSchema::new("cat", "Read and output file contents") + .param(ParamSchema::required("path", "string", "File path to read")); + let content = tool_help("cat", std::slice::from_ref(&schema)).expect("cat is registered"); + assert!(!content.contains("Subcommands:")); + } #[test] fn test_topic_parsing() { From 5d470c9f3dc1e55e9c1f7f6b666a1308849c1db0 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Tue, 1 Sep 2026 09:21:57 -0400 Subject: [PATCH 2/5] fix: kaish-tools recurses into nested subcommands too Problem: while fixing help 's one-level subcommand gap, the coordinator asked whether format_tool_detail in crates/kaish-kernel/src/tools/builtin/introspect.rs shared the defect. It is worse: that function has zero occurrences of `subcommands` anywhere in the file. `kaish-tools ` renders a tool's params and operations and never names a subcommand at any depth, not even the top level `help` used to manage before this patch series. This matters more than the `help` gap because kaish-tools is the introspection surface an agent calls programmatically, not prose it reads. Wrote failing tests first, in the existing tools::builtin::introspect::tests module: a two-level grammar (git -> worktree -> list, parameter on the leaf) and a three-level one (kj -> context -> session -> list). Both failed exactly as predicted -- `kaish-tools git` returned only "git\nGit plumbing and porcelain\n\n", naming neither worktree nor list. Added a control test asserting a flat tool's output is byte-identical to today's, to guard against the fix touching output shape for tools that were already correct. Decision: rather than writing a second recursive walker, made push_subcommand_roster in crates/kaish-help/src/topic.rs `pub` and called it directly from format_tool_detail. kaish-kernel already depends on kaish-help (crate::help re-exports tool_help from it), so the crate boundary was already open; reusing the same function means `help ` and `kaish-tools ` cannot drift into naming a tool's grammar two different ways. Kept format_tool_detail's own header/params/operations formatting untouched -- the control test pins that shape -- and inserted the shared roster between params and operations. Rule now in force: one subcommand renderer (push_subcommand_roster) backs every kaish surface that names a tool's subcommands, so a grammar change is visible to `help` and `kaish-tools` in lockstep. Surveyed further per the coordinator's question -- does format_tool_detail drop anything else tool_help renders. It does: schema.examples never renders (tool_help has an "Examples:" section), and a parameter's aliases (`-n` for `--max-count`) never render (tool_help's push_params adds "(also: ...)", format_tool_detail's inline loop does not). The disagreement also runs the other way: tool_help never renders schema.operations, which format_tool_detail does. Neither surface renders schema.aliases (command-level aliases like `ls` for `list`). Left all of these unchanged pending a scope decision -- this commit is subcommands only. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 7 +- crates/kaish-help/src/topic.rs | 6 +- .../src/tools/builtin/introspect.rs | 103 +++++++++++++++++- 3 files changed, 111 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f782bb0f..0c9e5e11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,9 +11,10 @@ breaking entries are marked **BREAKING**. ## [Unreleased] ### Fixed -- **`help ` only rendered one level of subcommands**, hiding a nested - verb (`worktree list`) and its flags. It now recurses to any depth, still - one flat two-space line per full path (`worktree list — …`). +- **`help ` and `kaish-tools ` only rendered one level of + subcommands** (`kaish-tools` rendered none), hiding a nested verb + (`worktree list`) and its flags. Both now recurse to any depth, one flat + two-space line per full path (`worktree list — …`). ## [0.17.0] - 2026-08-31 diff --git a/crates/kaish-help/src/topic.rs b/crates/kaish-help/src/topic.rs index fbc8a07c..56151e11 100644 --- a/crates/kaish-help/src/topic.rs +++ b/crates/kaish-help/src/topic.rs @@ -167,7 +167,11 @@ fn push_params(output: &mut String, params: &[kaish_types::ParamSchema], indent: /// indent, never a deeper indent per level. kaish-extras parses this roster /// by column: exactly two spaces, then the ` — ` (space, em-dash, space) /// separator. A nested indent or a different separator breaks that reader. -fn push_subcommand_roster(output: &mut String, prefix: &str, subs: &[ToolSchema]) { +/// +/// `pub` so `kaish-tools ` (`kaish-kernel`'s +/// `tools::builtin::introspect::format_tool_detail`) renders the same +/// roster as `help ` instead of a second, drifting implementation. +pub fn push_subcommand_roster(output: &mut String, prefix: &str, subs: &[ToolSchema]) { for sub in subs { let path = if prefix.is_empty() { sub.name.clone() diff --git a/crates/kaish-kernel/src/tools/builtin/introspect.rs b/crates/kaish-kernel/src/tools/builtin/introspect.rs index 5839e1aa..72638fb4 100644 --- a/crates/kaish-kernel/src/tools/builtin/introspect.rs +++ b/crates/kaish-kernel/src/tools/builtin/introspect.rs @@ -113,6 +113,14 @@ fn format_tool_detail(schemas: &[ToolSchema], name: &str) -> ExecResult { } } + // Same recursive renderer `help ` uses (kaish_help::topic) + // — the two introspection surfaces must not disagree about a + // tool's grammar. + if !s.subcommands.is_empty() { + output.push_str("Subcommands:\n"); + kaish_help::topic::push_subcommand_roster(&mut output, "", &s.subcommands); + } + if !s.operations.is_empty() { output.push_str(&format!("Operations: {}\n", s.operations.join(", "))); } @@ -272,7 +280,7 @@ mod tests { use super::*; use crate::ast::Value; use crate::interpreter::{apply_output_format, OutputFormat}; - use crate::tools::ToolSchema as TS; + use crate::tools::{ParamSchema, ToolSchema as TS}; use crate::vfs::{MemoryFs, VfsRouter}; use std::sync::Arc; @@ -344,6 +352,99 @@ mod tests { assert!(result.err.contains("tool not found")); } + #[tokio::test] + async fn test_tools_detail_recurses_into_nested_subcommands() { + // Same two-level grammar as help's regression test: a node + // (`worktree`) with no params of its own, a leaf (`list`) with one. + let leaf = TS::new("list", "List the repository's working trees").param( + ParamSchema::optional("porcelain", "bool", Value::Bool(false), "Machine-readable output"), + ); + let node = TS::new("worktree", "Work with the repository's working trees").subcommand(leaf); + let git = TS::new("git", "Git plumbing and porcelain").subcommand(node); + + let mut ctx = make_ctx(); + ctx.set_tool_schemas(vec![git]); + let mut args = ToolArgs::new(); + args.positional.push(Value::String("git".into())); + + let result = Tools.execute(args, &mut ctx).await; + assert!(result.ok()); + let text = result.text_out(); + + assert!( + text.contains("worktree list — List the repository's working trees"), + "expected full-path leaf line, got:\n{text}" + ); + assert!(text.contains("porcelain"), "expected leaf parameter to render, got:\n{text}"); + assert!( + text.contains("Machine-readable output"), + "expected leaf parameter description to render, got:\n{text}" + ); + + // Same flat-roster contract as `help `: exactly two spaces of + // indent, path and description joined by " — ". + let roster_start = text.find("Subcommands:\n").expect("Subcommands section") + "Subcommands:\n".len(); + for line in text[roster_start..].lines() { + if line.is_empty() || line.starts_with(" ") || line.starts_with("Operations:") { + continue; // param line, or past the roster + } + assert!( + line.starts_with(" ") && !line.starts_with(" "), + "roster line must start with exactly two spaces: {line:?}" + ); + assert!(line.contains(" — "), "roster line must use the ' — ' separator: {line:?}"); + } + } + + #[tokio::test] + async fn test_tools_detail_recurses_three_levels() { + let leaf = TS::new("list", "List sessions in this context").param(ParamSchema::optional( + "active", + "bool", + Value::Bool(false), + "Only running sessions", + )); + let session = TS::new("session", "Session operations").subcommand(leaf); + let context = TS::new("context", "Context operations").subcommand(session); + let kj = TS::new("kj", "kaijutsu control").subcommand(context); + + let mut ctx = make_ctx(); + ctx.set_tool_schemas(vec![kj]); + let mut args = ToolArgs::new(); + args.positional.push(Value::String("kj".into())); + + let result = Tools.execute(args, &mut ctx).await; + assert!(result.ok()); + let text = result.text_out(); + + assert!( + text.contains("context session list — List sessions in this context"), + "expected three-level full-path leaf line, got:\n{text}" + ); + assert!(text.contains("active"), "expected leaf parameter to render, got:\n{text}"); + } + + #[tokio::test] + async fn test_tools_detail_flat_tool_byte_identical() { + // Control: a tool with no subcommands must render exactly as it did + // before recursion was added — same header, params, operations. + let mut cat = TS::new("cat", "Concatenate files") + .param(ParamSchema::required("path", "string", "File path to read")); + cat.operations = vec!["fs.read".to_string()]; + + let mut ctx = make_ctx(); + ctx.set_tool_schemas(vec![cat]); + let mut args = ToolArgs::new(); + args.positional.push(Value::String("cat".into())); + + let result = Tools.execute(args, &mut ctx).await; + assert!(result.ok()); + assert_eq!( + result.text_out(), + "cat\nConcatenate files\n\nParameters:\n path : string (required)\n File path to read\nOperations: fs.read\n" + ); + } + // ============================ // mounts tests // ============================ From d39e728a7f8e144c66d7cf07ab5031e5834bfd55 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Tue, 1 Sep 2026 09:31:00 -0400 Subject: [PATCH 3/5] refactor(help): the shared roster gets a seam a caller can hold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the previous two commits. Sharing one recursion between `help ` and `kaish-tools ` is right -- two renderers would drift into two spellings of a tool's grammar -- but the shared function was published in the shape the recursion happened to have: push_subcommand_roster(&mut output, "", &s.subcommands) Both call sites passed `""`. `prefix` is the path accumulated during the walk; there is no prefix a caller could correctly supply other than the empty one, so the parameter existed only because the recursion needed it. kaish-help is on crates.io, so that signature would have been the published API from 0.17.1 onward. The public surface is now `subcommand_roster(subs) -> String`, which names what a caller wants rather than how the walk carries state, and the recursion stays private behind it. The caller still writes its own `Subcommands:` header, because the two surfaces place it differently. No behavior change: same roster, same two-space indent, same ` — ` separator at every depth, same tests. --- crates/kaish-help/src/topic.rs | 21 +++++++++++++------ .../src/tools/builtin/introspect.rs | 2 +- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/crates/kaish-help/src/topic.rs b/crates/kaish-help/src/topic.rs index 56151e11..76d9072d 100644 --- a/crates/kaish-help/src/topic.rs +++ b/crates/kaish-help/src/topic.rs @@ -125,7 +125,7 @@ pub fn tool_help(name: &str, schemas: &[ToolSchema]) -> Option { // constraints in their descriptions — was invisible to `help`. if !schema.subcommands.is_empty() { output.push_str("\nSubcommands:\n"); - push_subcommand_roster(&mut output, "", &schema.subcommands); + output.push_str(&subcommand_roster(&schema.subcommands)); } if !schema.examples.is_empty() { @@ -159,7 +159,8 @@ fn push_params(output: &mut String, params: &[kaish_types::ParamSchema], indent: } } -/// One flat roster line per subcommand at any depth, plus its parameters. +/// The roster lines naming every subcommand at any depth, plus each one's +/// parameters. The caller writes its own `Subcommands:` header. /// /// `ToolSchema::subcommands` is recursive — a node (`worktree`) can hold a /// leaf (`list`) that holds another node — but the roster stays flat: every @@ -168,10 +169,18 @@ fn push_params(output: &mut String, params: &[kaish_types::ParamSchema], indent: /// by column: exactly two spaces, then the ` — ` (space, em-dash, space) /// separator. A nested indent or a different separator breaks that reader. /// -/// `pub` so `kaish-tools ` (`kaish-kernel`'s -/// `tools::builtin::introspect::format_tool_detail`) renders the same -/// roster as `help ` instead of a second, drifting implementation. -pub fn push_subcommand_roster(output: &mut String, prefix: &str, subs: &[ToolSchema]) { +/// Public so `help ` and `kaish-tools ` render one roster from +/// one implementation instead of drifting into two spellings of a tool's +/// grammar. +pub fn subcommand_roster(subs: &[ToolSchema]) -> String { + let mut output = String::new(); + push_subcommand_roster(&mut output, "", subs); + output +} + +/// The recursion behind [`subcommand_roster`]. `prefix` is the path accumulated +/// so far and is a detail of the walk, which is why callers never supply it. +fn push_subcommand_roster(output: &mut String, prefix: &str, subs: &[ToolSchema]) { for sub in subs { let path = if prefix.is_empty() { sub.name.clone() diff --git a/crates/kaish-kernel/src/tools/builtin/introspect.rs b/crates/kaish-kernel/src/tools/builtin/introspect.rs index 72638fb4..9ba3f420 100644 --- a/crates/kaish-kernel/src/tools/builtin/introspect.rs +++ b/crates/kaish-kernel/src/tools/builtin/introspect.rs @@ -118,7 +118,7 @@ fn format_tool_detail(schemas: &[ToolSchema], name: &str) -> ExecResult { // tool's grammar. if !s.subcommands.is_empty() { output.push_str("Subcommands:\n"); - kaish_help::topic::push_subcommand_roster(&mut output, "", &s.subcommands); + output.push_str(&kaish_help::topic::subcommand_roster(&s.subcommands)); } if !s.operations.is_empty() { From 9e47ecd6df1a929b2f2298626634a4c849e215ac Mon Sep 17 00:00:00 2001 From: A Tobey Date: Tue, 1 Sep 2026 09:46:16 -0400 Subject: [PATCH 4/5] fix: close the rest of the help/kaish-tools description gap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem: after fixing the subcommand-recursion gap, the coordinator asked whether kaish-tools dropped anything else that help renders. It did, in both directions. tool_help (help ) has an Examples: section and names a parameter's aliases (-n for --max-count); format_tool_detail (kaish-tools ) had neither. format_tool_detail names a tool's declared effects (Operations: ...); tool_help never rendered schema.operations at all. Neither surface named a tool's own command-level aliases (schema.aliases, e.g. ls for list) -- real, shipped data: schema_from_clap reflects it straight from clap, and kj's context/list subcommands already carry it. Wrote failing tests first. Two in crates/kaish-help/src/topic.rs (tool_help never named an operation or a command alias): rm — Remove files No parameters. and list — List sessions No parameters. Three more in crates/kaish-kernel/src/tools/builtin/introspect.rs (format_tool_detail never named an example, a parameter's alias, or a command alias) -- e.g. `kaish-tools head` on a param with alias -n rendered "lines : int (optional)\n Line count\n", no "(also: -n)". Decision: one implementation per element, called from both surfaces, same principle as the subcommand roster. Added four pub functions to kaish-help's topic module -- param_lines, examples_section, operations_line, command_aliases_line -- each named for the concept a caller wants (an owned string to append) rather than shaped by an internal walk. param_lines wraps the existing push_params so kaish- kernel can call it without holding the accumulator; push_params itself stays private and unchanged for tool_help's own use. Both format_tool_detail and tool_help now call all four, plus the already- shared subcommand_roster, so a field named on one surface cannot go unnamed on the other by omission. Kept the byte-identical control test passing on both surfaces for a flat tool (no subcommands, aliases, examples, or operations) -- switching format_tool_detail's inline parameter loop to param_lines turned out to produce the identical bytes for a required parameter with no alias, so the existing control needed no change; kaish-help's control was strengthened from a substring check to an exact-string assertion. Left the two surfaces' framing different on purpose: tool_help's header is "name — desc", format_tool_detail's is "name\ndesc" on two lines; tool_help blank-lines before each optional section, format_ tool_detail stays tight. Neither is an accident -- they're prose for an agent to read versus a fixed low-noise introspection format -- so only the *content* of each section was unified, not the spacing or header style around it. One judgment call, not fixed here: a subcommand's own command-level alias (kj's context list subcommand carries "ls") still doesn't appear in the subcommand roster line -- only a tool's top-level aliases were in scope, matching what was actually compared and reported. Adding alias text into a roster line's path column also risks the kaish-extras parser, which treats everything before " — " as the invocable path; that needs its own decision, not a silent addition here. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 4 + crates/kaish-help/src/topic.rs | 97 +++++++++++++++++-- .../src/tools/builtin/introspect.rs | 83 +++++++++++++--- 3 files changed, 162 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c9e5e11..016a55a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ breaking entries are marked **BREAKING**. subcommands** (`kaish-tools` rendered none), hiding a nested verb (`worktree list`) and its flags. Both now recurse to any depth, one flat two-space line per full path (`worktree list — …`). +- **The same two surfaces also disagreed on a tool's examples, parameter + aliases, declared effects, and command-level aliases** — each rendered on + one side and silently dropped on the other. Both now render every field + from one shared implementation. ## [0.17.0] - 2026-08-31 diff --git a/crates/kaish-help/src/topic.rs b/crates/kaish-help/src/topic.rs index 76d9072d..0b8e93af 100644 --- a/crates/kaish-help/src/topic.rs +++ b/crates/kaish-help/src/topic.rs @@ -5,7 +5,7 @@ //! tool registry. //! Behavior here is intentionally byte-stable — frontends and tests depend on it. -use kaish_types::ToolSchema; +use kaish_types::{Example, ParamSchema, ToolSchema}; use crate::compose::render_syntax_section; use crate::content::{IGNORE, LIMITS, OUTPUT_LIMIT, OVERLAY, OVERVIEW, SCATTER, SYNTAX, VFS}; @@ -110,7 +110,9 @@ pub fn tool_help(name: &str, schemas: &[ToolSchema]) -> Option { let schema = schemas.iter().find(|s| s.name == name)?; let mut output = String::new(); - output.push_str(&format!("{} — {}\n\n", schema.name, schema.description)); + output.push_str(&format!("{} — {}\n", schema.name, schema.description)); + output.push_str(&command_aliases_line(&schema.aliases)); + output.push('\n'); if schema.params.is_empty() { output.push_str("No parameters.\n"); @@ -130,10 +132,12 @@ pub fn tool_help(name: &str, schemas: &[ToolSchema]) -> Option { if !schema.examples.is_empty() { output.push_str("\nExamples:\n"); - for example in &schema.examples { - output.push_str(&format!(" # {}\n", example.description)); - output.push_str(&format!(" {}\n\n", example.code)); - } + output.push_str(&examples_section(&schema.examples)); + } + + if !schema.operations.is_empty() { + output.push('\n'); + output.push_str(&operations_line(&schema.operations)); } Some(output) @@ -144,7 +148,7 @@ pub fn tool_help(name: &str, schemas: &[ToolSchema]) -> Option { /// Aliases are named here because they are the spelling agents actually /// write: a declaration that publishes `-n` for `--max-count` was telling /// `help` something it then dropped. -fn push_params(output: &mut String, params: &[kaish_types::ParamSchema], indent: &str) { +fn push_params(output: &mut String, params: &[ParamSchema], indent: &str) { for param in params { let req = if param.required { " (required)" } else { "" }; let aliases = if param.aliases.is_empty() { @@ -159,6 +163,50 @@ fn push_params(output: &mut String, params: &[kaish_types::ParamSchema], indent: } } +/// `push_params` as an owned string, for a caller across the crate +/// boundary that cannot hold the buffer `push_params` writes into +/// (`kaish-tools ` in `kaish-kernel`). `indent` is real per-caller +/// state — `" "` for a tool's own parameters, `" "` for a subcommand's +/// — unlike the accumulator, so it stays a parameter here. +pub fn param_lines(params: &[ParamSchema], indent: &str) -> String { + let mut output = String::new(); + push_params(&mut output, params, indent); + output +} + +/// The example lines for a tool: a `#`-comment naming what it demonstrates, +/// then the command, blank-line separated. The caller writes its own +/// `Examples:` header — the two surfaces place it differently. +pub fn examples_section(examples: &[Example]) -> String { + let mut output = String::new(); + for example in examples { + output.push_str(&format!(" # {}\n", example.description)); + output.push_str(&format!(" {}\n\n", example.code)); + } + output +} + +/// A tool's declared effect ids (`fs.remove`, `fs.overwrite`, …) as one +/// line, or empty when it declares none. See `ToolSchema.operations`. +pub fn operations_line(operations: &[String]) -> String { + if operations.is_empty() { + String::new() + } else { + format!("Operations: {}\n", operations.join(", ")) + } +} + +/// A one-line note naming a tool's command-level aliases (`ls` for +/// `list`), distinct from a parameter's own aliases (see `push_params`). +/// Empty when the tool declares none. +pub fn command_aliases_line(aliases: &[String]) -> String { + if aliases.is_empty() { + String::new() + } else { + format!("Aliases: {}\n", aliases.join(", ")) + } +} + /// The roster lines naming every subcommand at any depth, plus each one's /// parameters. The caller writes its own `Subcommands:` header. /// @@ -345,12 +393,41 @@ mod tests { #[test] fn test_tool_help_flat_tool_unchanged() { - // Control: a tool with no subcommands renders exactly as before — - // no "Subcommands:" section at all. + // Control: a tool with no subcommands, aliases, examples, or + // operations renders byte-identical to before this change. let schema = ToolSchema::new("cat", "Read and output file contents") .param(ParamSchema::required("path", "string", "File path to read")); let content = tool_help("cat", std::slice::from_ref(&schema)).expect("cat is registered"); - assert!(!content.contains("Subcommands:")); + assert_eq!( + content, + "cat — Read and output file contents\n\nParameters:\n path : string (required)\n File path to read\n" + ); + } + + #[test] + fn test_tool_help_renders_operations() { + // `kaish-tools ` already named a tool's declared effects; + // `help ` silently dropped them. + let mut schema = ToolSchema::new("rm", "Remove files"); + schema.operations = vec!["fs.remove".to_string()]; + let content = tool_help("rm", std::slice::from_ref(&schema)).expect("rm is registered"); + assert!( + content.contains("Operations: fs.remove"), + "expected declared effects to render, got:\n{content}" + ); + } + + #[test] + fn test_tool_help_renders_command_aliases() { + // Command-level aliases (`ls` for `list`) are real, shipped data — + // schema_from_clap reflects them from clap — but neither `help + // ` nor `kaish-tools ` named them. + let schema = ToolSchema::new("list", "List sessions").with_command_aliases(["ls"]); + let content = tool_help("list", std::slice::from_ref(&schema)).expect("list is registered"); + assert!( + content.contains("Aliases: ls"), + "expected command alias to render, got:\n{content}" + ); } #[test] diff --git a/crates/kaish-kernel/src/tools/builtin/introspect.rs b/crates/kaish-kernel/src/tools/builtin/introspect.rs index 9ba3f420..e1d14b80 100644 --- a/crates/kaish-kernel/src/tools/builtin/introspect.rs +++ b/crates/kaish-kernel/src/tools/builtin/introspect.rs @@ -100,29 +100,31 @@ fn format_tool_detail(schemas: &[ToolSchema], name: &str) -> ExecResult { match schema { Some(s) => { - let mut output = format!("{}\n{}\n\n", s.name, s.description); + // Every section below calls the same kaish_help::topic renderer + // `help ` uses, so the two introspection surfaces cannot + // drift into two spellings of a tool's params, subcommands, + // examples, operations, or aliases. + let mut output = format!("{}\n{}\n", s.name, s.description); + output.push_str(&kaish_help::topic::command_aliases_line(&s.aliases)); + output.push('\n'); if !s.params.is_empty() { output.push_str("Parameters:\n"); - for p in &s.params { - let required = if p.required { "(required)" } else { "(optional)" }; - output.push_str(&format!( - " {} : {} {}\n {}\n", - p.name, p.param_type, required, p.description - )); - } + output.push_str(&kaish_help::topic::param_lines(&s.params, " ")); } - // Same recursive renderer `help ` uses (kaish_help::topic) - // — the two introspection surfaces must not disagree about a - // tool's grammar. if !s.subcommands.is_empty() { output.push_str("Subcommands:\n"); output.push_str(&kaish_help::topic::subcommand_roster(&s.subcommands)); } + if !s.examples.is_empty() { + output.push_str("Examples:\n"); + output.push_str(&kaish_help::topic::examples_section(&s.examples)); + } + if !s.operations.is_empty() { - output.push_str(&format!("Operations: {}\n", s.operations.join(", "))); + output.push_str(&kaish_help::topic::operations_line(&s.operations)); } ExecResult::with_output(OutputData::text(output)) @@ -445,6 +447,63 @@ mod tests { ); } + #[tokio::test] + async fn test_tools_detail_renders_examples() { + // `help ` already named a tool's examples; `kaish-tools + // ` silently dropped them. + let mut ctx = make_ctx(); + let echo = TS::new("echo", "Print arguments") + .example("Print a literal string", "echo hello"); + ctx.set_tool_schemas(vec![echo]); + let mut args = ToolArgs::new(); + args.positional.push(Value::String("echo".into())); + + let result = Tools.execute(args, &mut ctx).await; + assert!(result.ok()); + let text = result.text_out(); + assert!( + text.contains("Print a literal string") && text.contains("echo hello"), + "expected the example to render, got:\n{text}" + ); + } + + #[tokio::test] + async fn test_tools_detail_renders_parameter_aliases() { + // `help `'s push_params names a flag's aliases (`-n` for + // `--max-count`); `kaish-tools `'s inline loop dropped them. + let mut ctx = make_ctx(); + let head = TS::new("head", "Print the first lines") + .param(ParamSchema::optional("lines", "int", Value::Int(10), "Line count").with_aliases(["-n"])); + ctx.set_tool_schemas(vec![head]); + let mut args = ToolArgs::new(); + args.positional.push(Value::String("head".into())); + + let result = Tools.execute(args, &mut ctx).await; + assert!(result.ok()); + let text = result.text_out(); + assert!( + text.contains("(also: -n)"), + "expected the parameter's alias to render, got:\n{text}" + ); + } + + #[tokio::test] + async fn test_tools_detail_renders_command_aliases() { + let mut ctx = make_ctx(); + let list = TS::new("list", "List sessions").with_command_aliases(["ls"]); + ctx.set_tool_schemas(vec![list]); + let mut args = ToolArgs::new(); + args.positional.push(Value::String("list".into())); + + let result = Tools.execute(args, &mut ctx).await; + assert!(result.ok()); + let text = result.text_out(); + assert!( + text.contains("Aliases: ls"), + "expected the command alias to render, got:\n{text}" + ); + } + // ============================ // mounts tests // ============================ From c77c825d9d2805ce1014afef6255b6752c6505d1 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Tue, 1 Sep 2026 09:52:49 -0400 Subject: [PATCH 5/5] changelog: kaish-help gained public API, and the entry only said Fixed The parity work added five public functions to kaish-help -- param_lines, examples_section, operations_line, command_aliases_line, subcommand_roster. kaish-help is on crates.io, so that is a surface an embedder can call and we now maintain, and the changelog filed the whole change under Fixed. The behavior entries were accurate; they described what a reader of `help` sees. They did not describe what a reader of the crate gets. An embedder scanning Fixed for a patch release has no reason to read further, which is exactly the reader who needed to know. Public is forced by the crate split rather than chosen -- kaish-kernel is across a crate boundary, and Rust has no visibility between "this crate" and "everyone." The granularity is chosen, though: the two surfaces frame a header and their section spacing differently on purpose, so one render-everything call could not serve both. --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 016a55a2..9cb97065 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ breaking entries are marked **BREAKING**. ## [Unreleased] +### Added +- **`kaish-help` publishes the pieces of a tool description** — `param_lines`, + `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. + ### Fixed - **`help ` and `kaish-tools ` only rendered one level of subcommands** (`kaish-tools` rendered none), hiding a nested verb