-
Notifications
You must be signed in to change notification settings - Fork 430
Compact MCP CLI help to show full command/option names within 20/30 lines #38056
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
20b79f1
a634c23
064789d
a1dd504
c538c02
d099bf7
f2734ea
fa8aaea
6936fd9
f4a7fef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,6 +54,12 @@ const KEEPALIVE_PING_INTERVAL_MS = 10000; | |
| /** Starting JSON-RPC ID for keepalive ping requests */ | ||
| const KEEPALIVE_PING_ID_START = 1000; | ||
|
|
||
| /** Preferred max lines for generated CLI help output */ | ||
| const TOP_HELP_MAX_LINES = 20; | ||
| const TOOL_HELP_MAX_LINES = 30; | ||
| const TOOL_DESC_MAX_LEN = 90; | ||
| const COMPACT_NAME_LINE_TARGET_WIDTH = 110; | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Audit logging | ||
| // --------------------------------------------------------------------------- | ||
|
|
@@ -763,20 +769,18 @@ function loadTools(toolsFile) { | |
| * @param {Array<{name: string, description?: string}>} tools - Tool list | ||
| */ | ||
| function showHelp(serverName, tools) { | ||
| const lines = [`Usage: ${serverName} <command> [options]`, ""]; | ||
| lines.push("Available commands:"); | ||
| const lines = [`Usage: ${serverName} <command> [--param value ...]`, `Tip: ${serverName} <command> --help`, "", `Commands (${tools.length}):`]; | ||
| if (tools.length > 0) { | ||
| // Calculate column width for aligned output | ||
| const maxNameLen = Math.max(...tools.map(t => t.name.length)); | ||
| for (const tool of tools) { | ||
| const padded = tool.name.padEnd(maxNameLen + 2); | ||
| lines.push(` ${padded}${tool.description || "No description"}`); | ||
| } | ||
| const maxCommandLines = Math.max(1, TOP_HELP_MAX_LINES - lines.length); | ||
| lines.push( | ||
| ...formatCompactNameLines( | ||
| tools.map(tool => tool.name), | ||
| maxCommandLines | ||
| ) | ||
| ); | ||
| } else { | ||
| lines.push(" (tool list unavailable)"); | ||
| } | ||
| lines.push(""); | ||
| lines.push(`Run '${serverName} <command> --help' for more information on a command.`); | ||
| process.stdout.write(lines.join("\n") + "\n"); | ||
| } | ||
|
|
||
|
|
@@ -796,40 +800,97 @@ function showToolHelp(serverName, toolName, tools) { | |
| return; | ||
| } | ||
|
|
||
| const lines = [`Command: ${toolName}`, `Description: ${tool.description || "No description"}`]; | ||
| const lines = [ | ||
| `Command: ${toolName}`, | ||
| `Description: ${summarizeHelpText(tool.description || "No description", TOOL_DESC_MAX_LEN)}`, | ||
| `Usage: ${serverName} ${toolName} [--param value ...]`, | ||
| `JSON mode: printf '{"param":"value",...}' | ${serverName} ${toolName} .`, | ||
| ]; | ||
|
|
||
| const props = tool.inputSchema?.properties; | ||
| const required = new Set(tool.inputSchema?.required || []); | ||
| if (props && Object.keys(props).length > 0) { | ||
| lines.push(""); | ||
| lines.push("Options:"); | ||
| const maxKeyLen = Math.max(...Object.keys(props).map(k => k.length)); | ||
| for (const [key, val] of Object.entries(props)) { | ||
| const flagPad = `--${key}`.padEnd(maxKeyLen + 4); | ||
| const parts = [getTypeStr(val.type)]; | ||
| if (required.has(key)) parts.push("(required)"); | ||
| if (val.description) parts.push(val.description); | ||
| lines.push(` ${flagPad}${parts.join(" ")}`); | ||
| lines.push(`Options (${Object.keys(props).length}):`); | ||
| const optionEntries = Object.entries(props); | ||
| const hasRequiredOptions = required.size > 0; | ||
| const maxOptionLines = Math.max(1, TOOL_HELP_MAX_LINES - lines.length - (hasRequiredOptions ? 1 : 0)); | ||
| lines.push( | ||
| ...formatCompactNameLines( | ||
| optionEntries.map(([key]) => `--${key}${required.has(key) ? "*" : ""}`), | ||
| maxOptionLines | ||
| ) | ||
| ); | ||
| if (hasRequiredOptions) { | ||
| lines.push("Required options are marked with *."); | ||
| } | ||
|
|
||
| lines.push(""); | ||
| lines.push(`Usage: ${serverName} ${toolName} [--param value ...]`); | ||
| lines.push(` or: printf '{"param":"value",...}' | ${serverName} ${toolName} .`); | ||
| } | ||
|
|
||
| process.stdout.write(lines.join("\n") + "\n"); | ||
| } | ||
|
|
||
| /** | ||
| * Format a JSON schema type value as a short bracketed string. | ||
| * Collapse whitespace and trim long help text for compact output. | ||
| * | ||
| * @param {string|string[]|undefined} type | ||
| * @param {string} value | ||
| * @param {number} maxLen | ||
| * @returns {string} | ||
| */ | ||
| function getTypeStr(type) { | ||
| if (!type) return "(string)"; | ||
| const types = Array.isArray(type) ? type.filter(t => t !== "null") : [type]; | ||
| return `(${types.length > 0 ? types.join("|") : "null"})`; | ||
| function summarizeHelpText(value, maxLen) { | ||
| const normalized = String(value || "") | ||
| .replace(/\s+/g, " ") | ||
| .trim(); | ||
| if (!Number.isFinite(maxLen) || maxLen <= 0) { | ||
| return normalized; | ||
| } | ||
| if (normalized.length <= maxLen) { | ||
| return normalized; | ||
| } | ||
| return `${normalized.slice(0, maxLen - 1)}…`; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/tdd] When None of the current constants reach 💡 SuggestionEither document the assumption ( // Ensure at least one content character before the ellipsis
if (maxLen < 2) return normalized.slice(0, maxLen); // just truncate, no marker
return `${normalized.slice(0, maxLen - 1)}...`; |
||
| } | ||
|
|
||
| /** | ||
| * Render names as comma-separated compact lines and keep all names visible. | ||
| * Width is a soft target; the final line may exceed it to avoid dropping names. | ||
| * | ||
| * @param {string[]} names | ||
| * @param {number} maxLines - Preferred line budget; non-positive/invalid values force one compact line | ||
| * @returns {string[]} | ||
| */ | ||
| function formatCompactNameLines(names, maxLines) { | ||
| if (!Array.isArray(names) || names.length === 0) { | ||
| // Callers spread the result into help lines, so empty input should contribute no lines. | ||
| return []; | ||
| } | ||
| if (!Number.isFinite(maxLines) || maxLines <= 0) { | ||
| return [` ${names.join(", ")}`]; | ||
| } | ||
| const lines = []; | ||
| let current = " "; | ||
| for (const name of names) { | ||
| const token = current.trim() ? `, ${name}` : name; | ||
| // A single very long name may still exceed the width target; we keep it intact. | ||
| const shouldStartNewLine = current.length + token.length > COMPACT_NAME_LINE_TARGET_WIDTH; | ||
| if (shouldStartNewLine) { | ||
| lines.push(current); | ||
| current = ` ${name}`; | ||
| continue; | ||
| } | ||
| current += token; | ||
| } | ||
| if (current.trim()) { | ||
| lines.push(current); | ||
| } | ||
| if (lines.length > maxLines) { | ||
| // Keep maxLines - 1 full lines and collapse the remaining names into the final allowed line. | ||
| const compactTail = lines | ||
| .slice(maxLines - 1) | ||
| // Trim per-line indentation before rebuilding a single indented tail line. | ||
| .map(line => line.trim()) | ||
| .join(", "); | ||
| return [...lines.slice(0, maxLines - 1), ` ${compactTail}`]; | ||
| } | ||
| return lines; | ||
| } | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
|
|
@@ -1130,6 +1191,8 @@ module.exports = { | |
| extractJSONRPCMessages, | ||
| renderProgressMessages, | ||
| formatResponse, | ||
| showHelp, | ||
| showToolHelp, | ||
| hasStdinJsonPayload, | ||
| readStdinSync, | ||
| main, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; | ||
|
|
||
| import { formatResponse, hasStdinJsonPayload, parseToolArgs, readStdinSync } from "./mcp_cli_bridge.cjs"; | ||
| import { formatResponse, hasStdinJsonPayload, parseToolArgs, readStdinSync, showHelp, showToolHelp } from "./mcp_cli_bridge.cjs"; | ||
|
|
||
| describe("mcp_cli_bridge.cjs", () => { | ||
| let originalCore; | ||
|
|
@@ -212,6 +212,121 @@ describe("mcp_cli_bridge.cjs", () => { | |
| expect(process.exitCode).toBe(0); | ||
| }); | ||
|
|
||
| it("keeps top-level help compact for many commands", () => { | ||
| const tools = Array.from({ length: 25 }, (_, i) => ({ | ||
| name: `tool_${i + 1}`, | ||
| description: `Description for command ${i + 1} that is intentionally verbose for truncation checks.`, | ||
| })); | ||
|
|
||
| showHelp("safeoutputs", tools); | ||
|
|
||
| const outputLines = stdoutChunks.join("").trimEnd().split("\n"); | ||
| const output = outputLines.join("\n"); | ||
| expect(outputLines.length).toBeLessThanOrEqual(20); | ||
| expect(output).not.toMatch(/\.\.\. \+\d+ more command\(s\)/); | ||
| for (const tool of tools) { | ||
| expect(output).toContain(tool.name); | ||
| } | ||
| }); | ||
|
|
||
| it("does not truncate top-level help when commands exactly fit the line budget", () => { | ||
| const tools = Array.from({ length: 14 }, (_, i) => ({ | ||
| name: `tool_${i + 1}`, | ||
| description: `Description for command ${i + 1}.`, | ||
| })); | ||
|
|
||
| showHelp("safeoutputs", tools); | ||
|
|
||
| const outputLines = stdoutChunks.join("").trimEnd().split("\n"); | ||
| const output = outputLines.join("\n"); | ||
| expect(outputLines.length).toBeLessThanOrEqual(20); | ||
| expect(output).not.toMatch(/\.\.\. \+\d+ more command\(s\)/); | ||
| for (const tool of tools) { | ||
| expect(output).toContain(tool.name); | ||
| } | ||
| }); | ||
|
Comment on lines
+215
to
+230
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/tdd] 💡 Suggested test skeletondescribe("summarizeHelpText", () => {
// Need to export it first, or test via a helper that calls it directly.
it("collapses internal whitespace", () => {
expect(summarizeHelpText("foo bar\n baz", 100)).toBe("foo bar baz");
});
it("returns full string when within limit", () => {
expect(summarizeHelpText("hello", 5)).toBe("hello");
});
it("truncates with ellipsis when over limit", () => {
expect(summarizeHelpText("hello world", 8)).toBe("hello w...");
});
it("returns normalized text when maxLen <= 0", () => {
expect(summarizeHelpText("text", 0)).toBe("text");
});
});This would also catch the |
||
|
|
||
| it("keeps command help compact for many options", () => { | ||
| const properties = {}; | ||
| for (let i = 1; i <= 24; i++) { | ||
| properties[`field_${i}`] = { type: "string", description: `Field ${i} description with additional details for truncation.` }; | ||
| } | ||
|
|
||
| showToolHelp("safeoutputs", "create_issue", [ | ||
| { | ||
| name: "create_issue", | ||
| description: "Create an issue with many available fields and optional metadata.", | ||
| inputSchema: { | ||
| properties, | ||
| required: ["field_1", "field_2"], | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/tdd] This test always puts required fields ( 💡 Add a companion testit("omits required note when all required options are truncated", () => {
const properties = {};
for (let i = 1; i <= 24; i++) {
properties[`field_${i}`] = { type: "string", description: `Field ${i}.` };
}
showToolHelp("safeoutputs", "create_issue", [
{
name: "create_issue",
description: "Create an issue.",
inputSchema: {
properties,
// required fields are near the end — will be truncated
required: ["field_20", "field_21"],
},
},
]);
const out = stdoutChunks.join("");
expect(out).not.toContain("Required options are marked");
});This test will fail with the current implementation, confirming the bug. |
||
| }, | ||
| }, | ||
| ]); | ||
|
|
||
| const outputLines = stdoutChunks.join("").trimEnd().split("\n"); | ||
| const output = outputLines.join("\n"); | ||
| expect(outputLines.length).toBeLessThanOrEqual(30); | ||
| expect(output).not.toMatch(/\.\.\. \+\d+ more option\(s\)/); | ||
| expect(output).toContain("Required options are marked with *."); | ||
| for (let i = 1; i <= 24; i++) { | ||
| expect(output).toContain(`--field_${i}`); | ||
| } | ||
| expect(output).toContain("--field_1*"); | ||
| expect(output).toContain("--field_2*"); | ||
| }); | ||
|
|
||
| it("does not truncate command help when options exactly fit the line budget", () => { | ||
| const properties = {}; | ||
| for (let i = 1; i <= 13; i++) { | ||
| properties[`field_${i}`] = { type: "string", description: `Field ${i}.` }; | ||
| } | ||
|
|
||
| showToolHelp("safeoutputs", "create_issue", [ | ||
| { | ||
| name: "create_issue", | ||
| description: "Create an issue.", | ||
| inputSchema: { | ||
| properties, | ||
| required: ["field_1"], | ||
| }, | ||
| }, | ||
| ]); | ||
|
|
||
| const outputLines = stdoutChunks.join("").trimEnd().split("\n"); | ||
| const output = outputLines.join("\n"); | ||
| expect(outputLines.length).toBeLessThanOrEqual(30); | ||
| expect(output).not.toMatch(/\.\.\. \+\d+ more option\(s\)/); | ||
| expect(output).toContain("Required options are marked with *."); | ||
| for (let i = 1; i <= 13; i++) { | ||
| expect(output).toContain(`--field_${i}`); | ||
| } | ||
| }); | ||
|
|
||
| it("keeps required note when required options are in the compact list", () => { | ||
| const properties = {}; | ||
| for (let i = 1; i <= 24; i++) { | ||
| properties[`field_${i}`] = { type: "string", description: `Field ${i}.` }; | ||
| } | ||
|
|
||
| showToolHelp("safeoutputs", "create_issue", [ | ||
| { | ||
| name: "create_issue", | ||
| description: "Create an issue.", | ||
| inputSchema: { | ||
| properties, | ||
| required: ["field_23", "field_24"], | ||
| }, | ||
| }, | ||
| ]); | ||
|
|
||
| const outputLines = stdoutChunks.join("").trimEnd().split("\n"); | ||
| const output = outputLines.join("\n"); | ||
| expect(output).not.toMatch(/\.\.\. \+\d+ more option\(s\)/); | ||
| expect(output).toContain("Required options are marked with *."); | ||
| expect(output).toContain("--field_23*"); | ||
| expect(output).toContain("--field_24*"); | ||
| }); | ||
|
|
||
| describe("stdin placeholder removed — '-' is always a literal value", () => { | ||
| it("passes '--key -' as literal '-' (space-separated form)", () => { | ||
| const schemaProperties = { body: { type: "string" } }; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String.slice()can bisect a surrogate pair: JS string indexing is UTF-16, sonormalized.slice(0, maxLen - 1)can split a surrogate pair (e.g., emoji like🚀, U+1F680) at positionmaxLen - 1, producing a lone high surrogate followed by.... The resulting string is invalid Unicode and may render as?or□in some terminals.💡 Suggested fix
Use
Array.fromto iterate codepoints instead of code units:MCP tool descriptions are mostly ASCII today, so probability is low — but
summarizeHelpTextis a generic utility and the fix is trivial.