Skip to content

Commit 376c3e5

Browse files
ScriptedAlchemyScriptedAlchemy
authored
Merge pull request #240 from ScriptedAlchemy/codex/slow-junit-tests
[codex] Speed up MCP tool CLI coverage test
2 parents a4032ea + f82e3e5 commit 376c3e5

3 files changed

Lines changed: 121 additions & 75 deletions

File tree

‎src/mcp/tools/mod.rs‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ pub(crate) mod render;
1111

1212
use serde::{Deserialize, Serialize};
1313
use serde_json::Value;
14+
use std::fmt::Write as _;
1415

1516
pub use definitions::{
1617
ast_grep_available, ast_grep_diagnostics_json, ast_grep_outline_available, context_description,
@@ -77,6 +78,92 @@ impl ToolResult {
7778
}
7879
}
7980

81+
/// Render the CLI help shown by `tracedecay tool <name> --help`.
82+
///
83+
/// Kept in the library so tests and generated integration surfaces can
84+
/// validate the dynamic tool help without spawning the binary once per tool.
85+
pub fn render_tool_cli_help(def: &ToolDefinition) -> String {
86+
let short = short_tool_name(&def.name);
87+
let mut out = String::new();
88+
let _ = writeln!(out, "tracedecay tool {short}");
89+
let _ = writeln!(out);
90+
let _ = writeln!(out, "{}", def.description);
91+
let _ = writeln!(out);
92+
93+
let props = def
94+
.input_schema
95+
.get("properties")
96+
.and_then(Value::as_object)
97+
.filter(|props| !props.is_empty());
98+
let required: Vec<&str> = def
99+
.input_schema
100+
.get("required")
101+
.and_then(Value::as_array)
102+
.map(|arr| arr.iter().filter_map(Value::as_str).collect())
103+
.unwrap_or_default();
104+
105+
let Some(props) = props else {
106+
let _ = writeln!(out, "(no parameters)");
107+
let _ = writeln!(out);
108+
let _ = writeln!(
109+
out,
110+
"Usage: tracedecay tool {short} [--json] [--project <path>]"
111+
);
112+
return out;
113+
};
114+
115+
let mut usage_params = String::new();
116+
for req in &required {
117+
let _ = write!(usage_params, " --{} <value>", req.replace('_', "-"));
118+
}
119+
let _ = writeln!(
120+
out,
121+
"Usage: tracedecay tool {short}{usage_params} [--key value]... [--json]"
122+
);
123+
let _ = writeln!(out);
124+
125+
let _ = writeln!(out, "Parameters:");
126+
let mut entries: Vec<(&String, &Value)> = props.iter().collect();
127+
entries.sort_by_key(|(k, _)| (*k).clone());
128+
for (key, schema) in entries {
129+
let ty = schema
130+
.get("type")
131+
.and_then(Value::as_str)
132+
.unwrap_or("string");
133+
let req = if required.contains(&key.as_str()) {
134+
"required"
135+
} else {
136+
"optional"
137+
};
138+
let desc = schema
139+
.get("description")
140+
.and_then(Value::as_str)
141+
.unwrap_or("");
142+
let _ = writeln!(
143+
out,
144+
" --{:<26} {:<8} {:<8} {}",
145+
key.replace('_', "-"),
146+
ty,
147+
req,
148+
desc
149+
);
150+
}
151+
let _ = writeln!(out);
152+
let _ = writeln!(
153+
out,
154+
"Reserved flags: --json, --project <path>, --args <json|@file>, -h/--help"
155+
);
156+
let _ = writeln!(
157+
out,
158+
"Any value starting with @ is read from that file (multi-line payloads)."
159+
);
160+
out
161+
}
162+
163+
fn short_tool_name(full: &str) -> &str {
164+
full.strip_prefix("tracedecay_").unwrap_or(full)
165+
}
166+
80167
#[cfg(test)]
81168
mod tests {
82169
use super::*;

‎src/tool_command.rs‎

Lines changed: 2 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use tracedecay::daemon::call_default_tool;
3333
use tracedecay::daemon::DaemonHandshake;
3434
use tracedecay::errors::{Result, TraceDecayError};
3535
use tracedecay::mcp::tools::{
36-
get_tool_definitions, handle_profile_scoped_lcm_tool_call, ToolDefinition,
36+
get_tool_definitions, handle_profile_scoped_lcm_tool_call, render_tool_cli_help, ToolDefinition,
3737
};
3838

3939
/// Old CLI command names that don't match the MCP tool name. Keeps muscle
@@ -719,66 +719,7 @@ fn group_for(def: &ToolDefinition) -> &'static str {
719719

720720
/// Print one tool's description, usage line, and parameter table.
721721
fn print_tool_help(def: &ToolDefinition) {
722-
let short = short_name(&def.name);
723-
println!("tracedecay tool {short}");
724-
println!();
725-
println!("{}", def.description);
726-
println!();
727-
728-
let props = def
729-
.input_schema
730-
.get("properties")
731-
.and_then(Value::as_object)
732-
.filter(|props| !props.is_empty());
733-
let required: Vec<&str> = def
734-
.input_schema
735-
.get("required")
736-
.and_then(Value::as_array)
737-
.map(|arr| arr.iter().filter_map(Value::as_str).collect())
738-
.unwrap_or_default();
739-
740-
let Some(props) = props else {
741-
println!("(no parameters)");
742-
println!();
743-
println!("Usage: tracedecay tool {short} [--json] [--project <path>]");
744-
return;
745-
};
746-
747-
let usage_params: String = required
748-
.iter()
749-
.map(|req| format!(" --{} <value>", req.replace('_', "-")))
750-
.collect();
751-
println!("Usage: tracedecay tool {short}{usage_params} [--key value]... [--json]");
752-
println!();
753-
754-
println!("Parameters:");
755-
let mut entries: Vec<(&String, &Value)> = props.iter().collect();
756-
entries.sort_by_key(|(k, _)| (*k).clone());
757-
for (key, schema) in entries {
758-
let ty = schema
759-
.get("type")
760-
.and_then(Value::as_str)
761-
.unwrap_or("string");
762-
let req = if required.contains(&key.as_str()) {
763-
"required"
764-
} else {
765-
"optional"
766-
};
767-
let desc = schema
768-
.get("description")
769-
.and_then(Value::as_str)
770-
.unwrap_or("");
771-
println!(
772-
" --{:<26} {:<8} {:<8} {}",
773-
key.replace('_', "-"),
774-
ty,
775-
req,
776-
desc
777-
);
778-
}
779-
println!();
780-
println!("Reserved flags: --json, --project <path>, --args <json|@file>, -h/--help");
781-
println!("Any value starting with @ is read from that file (multi-line payloads).");
722+
print!("{}", render_tool_cli_help(def));
782723
}
783724

784725
#[cfg(test)]

‎tests/agent_suite/tool_skill_coverage_test.rs‎

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use std::process::Command;
77

88
use crate::common::tracedecay_command_with_home;
99
use tempfile::TempDir;
10-
use tracedecay::mcp::tools::get_tool_definitions;
10+
use tracedecay::mcp::tools::{get_tool_definitions, render_tool_cli_help};
1111

1212
/// MCP tools intentionally exempt from bundled-skill coverage.
1313
/// Keep empty unless a tool is truly internal.
@@ -47,28 +47,46 @@ fn every_mcp_tool_is_listed_by_the_cli_discovery_command() {
4747
}
4848

4949
#[test]
50-
fn every_mcp_tool_is_invocable_via_the_cli() {
51-
let home = TempDir::new().expect("create isolated TraceDecay home");
50+
fn every_mcp_tool_renders_its_own_cli_help() {
5251
for def in get_tool_definitions() {
5352
let short = short_name(&def.name);
54-
let output = isolated_tracedecay_command(&home)
55-
.args(["tool", short, "--help"])
56-
.output()
57-
.unwrap_or_else(|e| panic!("run `tracedecay tool {short} --help`: {e}"));
58-
let stdout = String::from_utf8_lossy(&output.stdout);
59-
assert!(
60-
output.status.success(),
61-
"`tracedecay tool {short} --help` must succeed so the tool stays \
62-
invocable without an MCP client:\n{}",
63-
String::from_utf8_lossy(&output.stderr)
64-
);
53+
let stdout = render_tool_cli_help(&def);
6554
assert!(
6655
stdout.contains(&format!("tracedecay tool {short}")),
6756
"`tracedecay tool {short} --help` should print the tool's own help, got:\n{stdout}"
6857
);
6958
}
7059
}
7160

61+
/// One real `tracedecay tool <name> --help` invocation, asserting the binary
62+
/// prints exactly what `render_tool_cli_help` renders. Tool-name resolution
63+
/// and help dispatch are shared across tools, so a single spawn keeps the CLI
64+
/// wiring covered end-to-end without paying one process per tool.
65+
#[test]
66+
fn tool_cli_help_matches_rendered_help_end_to_end() {
67+
let home = TempDir::new().expect("create isolated TraceDecay home");
68+
let def = get_tool_definitions()
69+
.into_iter()
70+
.next()
71+
.expect("at least one MCP tool definition");
72+
let short = short_name(&def.name);
73+
let output = isolated_tracedecay_command(&home)
74+
.args(["tool", short, "--help"])
75+
.output()
76+
.unwrap_or_else(|e| panic!("run `tracedecay tool {short} --help`: {e}"));
77+
assert!(
78+
output.status.success(),
79+
"`tracedecay tool {short} --help` must succeed so tools stay invocable \
80+
without an MCP client:\n{}",
81+
String::from_utf8_lossy(&output.stderr)
82+
);
83+
assert_eq!(
84+
String::from_utf8_lossy(&output.stdout),
85+
render_tool_cli_help(&def),
86+
"CLI help output should be exactly the rendered help"
87+
);
88+
}
89+
7290
/// True when `haystack` mentions `tool_name` as a standalone identifier
7391
/// (not as a prefix of a longer tool name such as `tracedecay_lcm_expand`
7492
/// inside `tracedecay_lcm_expand_query`).

0 commit comments

Comments
 (0)