mirror of
https://github.com/morluto/rea.git
synced 2026-11-07 05:56:19 -10:00
fix(cli): honor the selected format for unknown mcp subcommands (#7802)
* fix(cli): honor the selected format for unknown mcp subcommands Incur 0.6.1 renders its unknown mcp subcommand rejection as TOON regardless of --format or --json. Report the same COMMAND_NOT_FOUND envelope in the explicitly selected format before dispatching to Incur, share the pinned mcp subcommand table with completions, and redact explicitly declared sensitive values from the diagnostic. Fixes #7691 * fix(cli): correct only the subcommand in the mcp suggestion The suggested command replaced every token equal to the unknown subcommand, including option values such as --command. Record each command token's argument position in the shared builtin split and replace only the subcommand position. * fix(cli): leave earlier Incur builtins and caller output flags intact Incur serves --llms, --llms-full and its update check before the mcp subcommand branch, so defer those invocations to Incur as well. Build the suggested command from the caller's arguments rather than the arguments adapted for Incur, so a delimited format and its columns remain in the suggestion. * fix(cli): ignore output values spelled like earlier Incur builtins The earlier-builtin check matched raw tokens, so an output flag value such as --columns --llms or --filter-output --llms deferred the rejection to Incur's TOON path. Check the arguments Incur receives and skip positions its builtin pass consumes as output values. * fix(cli): omit mcp suggestions that would not replay caller input Joining the corrected tokens with spaces re-tokenizes a value with whitespace or shell metacharacters when the suggestion is copied, and quoting differs between POSIX shells, cmd and PowerShell. Offer the corrected command only when every token is a plain shell word and no declared sensitive value was redacted; the message still names the suggested subcommand. * fix(cli): preserve safe MCP diagnostic output * fix(cli): omit mcp suggestions with words zsh expands zsh expands a word that starts with = to an executable path by default, so a copied suggestion such as --command =node would replace the caller's literal value. Treat =-leading words as non-replayable. * fix(cli): render mcp diagnostics through the document renderer Markdown output went through Incur's formatter, so caller text such as *oops* rendered as emphasis instead of literal text. Use REA's document renderer for every format, and redact declared sensitive literals from the fixed call-to-action strings as well as the message. * test(cli): keep the mcp correction on the subcommand after timing globals --------- Co-authored-by: morluto <[email protected]>
This commit is contained in:
1 parent
afc9cfe9ff
commit
f1bfa1cba2
5 files changed
+332
-23
No files matched your search
+4
-1
@@ -87,6 +87,7 @@ import {
|
||||
presentCliArgumentErrors,
|
||||
} from "./cli/argumentErrors.js";
|
||||
import { completionHelpNote, createCliCompletions } from "./cli/completions.js";
|
||||
import { reportUnknownMcpSubcommand } from "./cli/mcpBuiltinCommands.js";
|
||||
import {
|
||||
closeCliGuidanceResources,
|
||||
registerGuidanceCommands,
|
||||
@@ -601,7 +602,9 @@ export const createCli = (
|
||||
}
|
||||
const result = serveStrict(normalized, options ?? {}, (tokens, selected) =>
|
||||
serveDeclared(tokens, selected, (declared, declaredOptions) =>
|
||||
serve(adaptCliOutputArgumentsForIncur(declared), declaredOptions),
|
||||
reportUnknownMcpSubcommand(cli.name, declared, declaredOptions)
|
||||
? Promise.resolve()
|
||||
: serve(adaptCliOutputArgumentsForIncur(declared), declaredOptions),
|
||||
),
|
||||
);
|
||||
const note = completionHelpNote(normalized);
|
||||
|
||||
+2
-18
@@ -11,6 +11,7 @@ import {
|
||||
} from "../cliBuiltinArguments.js";
|
||||
import { CLI_OUTPUT_FORMATS, parseCliOutputArguments } from "../cliOutput.js";
|
||||
import { bashCompletionScript } from "./bashCompletionRegistration.js";
|
||||
import { MCP_BUILTIN_SUBCOMMANDS } from "./mcpBuiltinCommands.js";
|
||||
import { MCP_DOCTOR_ALIAS, MCP_DOCTOR_OPTIONS } from "../mcpDoctorOptions.js";
|
||||
|
||||
type Commands = Parameters<typeof Completions.complete>[0];
|
||||
@@ -85,24 +86,7 @@ const completionCommands = (cli: CompletionCli): Commands => {
|
||||
commands.set("mcp", {
|
||||
_group: true,
|
||||
description: "Register as MCP server",
|
||||
commands: new Map([
|
||||
[
|
||||
"add",
|
||||
{
|
||||
description: "Register as MCP server",
|
||||
alias: { command: "c" },
|
||||
options: z.object({
|
||||
agent: z.string().optional(),
|
||||
command: z.string().optional(),
|
||||
noGlobal: z.boolean().optional(),
|
||||
}),
|
||||
},
|
||||
],
|
||||
[
|
||||
"doctor",
|
||||
{ description: "Validate MCP server startup and tool listing" },
|
||||
],
|
||||
]),
|
||||
commands: MCP_BUILTIN_SUBCOMMANDS,
|
||||
});
|
||||
commands.set("completions", {
|
||||
_group: true,
|
||||
|
||||
@@ -0,0 +1,132 @@
|
||||
import { z } from "incur";
|
||||
|
||||
import {
|
||||
adaptCliOutputArgumentsForIncur,
|
||||
declaredSensitiveValues,
|
||||
splitIncurBuiltinArguments,
|
||||
} from "../cliBuiltinArguments.js";
|
||||
import {
|
||||
parseCliOutputArguments,
|
||||
renderCliDocument,
|
||||
type CliOutputFormat,
|
||||
} from "../cliOutput.js";
|
||||
import { redactExplicitText } from "../domain/explicitSensitiveValues.js";
|
||||
import type { CliInstance } from "./types.js";
|
||||
|
||||
type ServeOptions = NonNullable<Parameters<CliInstance["serve"]>[1]>;
|
||||
|
||||
/** Pinned Incur 0.6.1 `mcp` builtin subcommands; Incur does not export them. */
|
||||
export const MCP_BUILTIN_SUBCOMMANDS = new Map([
|
||||
[
|
||||
"add",
|
||||
{
|
||||
description: "Register as MCP server",
|
||||
alias: { command: "c" },
|
||||
options: z.object({
|
||||
agent: z.string().optional(),
|
||||
command: z.string().optional(),
|
||||
noGlobal: z.boolean().optional(),
|
||||
}),
|
||||
},
|
||||
],
|
||||
["doctor", { description: "Validate MCP server startup and tool listing" }],
|
||||
]);
|
||||
|
||||
/** Incur 0.6.1 builtins that it serves before its `mcp` subcommand branch. */
|
||||
const PRECEDING_BUILTIN_FLAGS: ReadonlySet<string> = new Set([
|
||||
"--incur-update-check",
|
||||
"--update",
|
||||
"--llms",
|
||||
"--llms-full",
|
||||
"--mcp",
|
||||
]);
|
||||
|
||||
// zsh expands a word that starts with `=` to an executable path.
|
||||
const PLAIN_SHELL_WORD = /^(?!=)[\w./+=:,-]+$/u;
|
||||
|
||||
/**
|
||||
* Incur 0.6.1 rejects an unknown `mcp` subcommand in TOON regardless of the
|
||||
* selected format. Report the same envelope in the explicitly selected format;
|
||||
* without one, Incur's TOON or human rendering is already the selected output.
|
||||
* The arguments are the caller's normalized ones, before REA adapts output
|
||||
* flags for Incur, so the suggested command keeps the caller's selection.
|
||||
*/
|
||||
export const reportUnknownMcpSubcommand = (
|
||||
name: string,
|
||||
arguments_: readonly string[],
|
||||
options: ServeOptions,
|
||||
): boolean => {
|
||||
const { command, commandIndices } = splitIncurBuiltinArguments(arguments_);
|
||||
const [entry, subcommand] = command;
|
||||
// Incur receives JSON for REA's delimited formats and no `--columns`.
|
||||
const incurArguments = adaptCliOutputArgumentsForIncur(arguments_);
|
||||
const incurOutputValues = new Set(
|
||||
splitIncurBuiltinArguments(incurArguments).outputValues.map(
|
||||
({ index }) => index,
|
||||
),
|
||||
);
|
||||
if (
|
||||
entry !== "mcp" ||
|
||||
// Incur shows group help for an empty token and owns flag-shaped globals.
|
||||
!subcommand ||
|
||||
subcommand.startsWith("-") ||
|
||||
MCP_BUILTIN_SUBCOMMANDS.has(subcommand) ||
|
||||
// Incur serves these builtins before its `mcp` subcommand branch. An
|
||||
// output flag's value is data, even when it is spelled like a builtin.
|
||||
incurArguments.some(
|
||||
(token, index) =>
|
||||
!incurOutputValues.has(index) && PRECEDING_BUILTIN_FLAGS.has(token),
|
||||
)
|
||||
)
|
||||
return false;
|
||||
const output = parseCliOutputArguments(incurArguments);
|
||||
// Invalid output controls keep Incur's own parse diagnostic.
|
||||
if (!output.formatExplicit || !output.validation.ok) return false;
|
||||
const sensitive = declaredSensitiveValues(command);
|
||||
const text = (value: string): string => redactExplicitText(value, sensitive);
|
||||
const suggestion = [...MCP_BUILTIN_SUBCOMMANDS.keys()].find((candidate) =>
|
||||
candidate.startsWith(subcommand.toLowerCase()),
|
||||
);
|
||||
const corrected = arguments_.map((token, index) =>
|
||||
index === commandIndices[1] ? suggestion : token,
|
||||
);
|
||||
// Joined tokens replay only when each one is a plain word in every shell;
|
||||
// quoting is shell-specific, and a redacted value cannot be replayed.
|
||||
const replayable =
|
||||
suggestion !== undefined &&
|
||||
corrected.every(
|
||||
(token) =>
|
||||
token !== undefined &&
|
||||
PLAIN_SHELL_WORD.test(token) &&
|
||||
text(token) === token,
|
||||
);
|
||||
const commands = [
|
||||
...(replayable ? [{ command: `${name} ${corrected.join(" ")}` }] : []),
|
||||
{
|
||||
command: text(`${name} mcp --help`),
|
||||
description: text("see all available commands"),
|
||||
},
|
||||
];
|
||||
const document = {
|
||||
code: "COMMAND_NOT_FOUND",
|
||||
message: text(
|
||||
`'${subcommand}' is not a command for '${name} mcp'.${suggestion === undefined ? "" : ` Did you mean '${suggestion}'?`}`,
|
||||
),
|
||||
cta: {
|
||||
description: text(
|
||||
commands.length === 1 ? "Suggested command:" : "Suggested commands:",
|
||||
),
|
||||
commands,
|
||||
},
|
||||
};
|
||||
(options.stdout ?? ((value: string) => process.stdout.write(value)))(
|
||||
// REA's document renderer owns JSON layout and Markdown escaping.
|
||||
renderCliDocument(
|
||||
document,
|
||||
output.format as CliOutputFormat,
|
||||
process.stdout.isTTY === true,
|
||||
),
|
||||
);
|
||||
(options.exit ?? ((code: number) => process.exit(code)))(1);
|
||||
return true;
|
||||
};
|
||||
@@ -67,6 +67,8 @@ export const splitCliTimingArguments = (
|
||||
export interface IncurBuiltinArgumentSplit {
|
||||
/** Tokens left for command resolution and command option parsing. */
|
||||
readonly command: readonly string[];
|
||||
/** Argument position of each `command` token. */
|
||||
readonly commandIndices: readonly number[];
|
||||
/** Valueless global flags extracted by Incur before command dispatch. */
|
||||
readonly flags: ReadonlySet<string>;
|
||||
/** Positions of nonempty values consumed by valued output flags. */
|
||||
@@ -85,6 +87,7 @@ export const splitIncurBuiltinArguments = (
|
||||
arguments_: readonly string[],
|
||||
): IncurBuiltinArgumentSplit => {
|
||||
const command: string[] = [];
|
||||
const commandIndices: number[] = [];
|
||||
const flags = new Set<string>();
|
||||
const outputValues: { readonly flag: string; readonly index: number }[] = [];
|
||||
for (let index = 0; index < arguments_.length; index += 1) {
|
||||
@@ -104,20 +107,29 @@ export const splitIncurBuiltinArguments = (
|
||||
continue;
|
||||
}
|
||||
command.push(argument);
|
||||
commandIndices.push(index);
|
||||
}
|
||||
// Incur extracts builtin output values first, then parses globals until `--`.
|
||||
// A timing-shaped output value is data; a command option's value is still
|
||||
// subject to this global pass before the registration builtin consumes it.
|
||||
let globalsEnded = false;
|
||||
const remaining = command.filter((token) => {
|
||||
const remaining: string[] = [];
|
||||
const remainingIndices: number[] = [];
|
||||
command.forEach((token, position) => {
|
||||
if (token === "--") globalsEnded = true;
|
||||
if (!globalsEnded && cliTimingFlagValue(token) !== undefined) {
|
||||
flags.add(token);
|
||||
return false;
|
||||
return;
|
||||
}
|
||||
return true;
|
||||
remaining.push(token);
|
||||
remainingIndices.push(commandIndices[position]!);
|
||||
});
|
||||
return { command: remaining, flags, outputValues };
|
||||
return {
|
||||
command: remaining,
|
||||
commandIndices: remainingIndices,
|
||||
flags,
|
||||
outputValues,
|
||||
};
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -883,3 +883,181 @@ describe("version report rendering", () => {
|
||||
).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("unknown MCP subcommand correction", () => {
|
||||
workspaceCliTest(
|
||||
"keeps the correction on the subcommand after timing globals",
|
||||
async ({ cli }) => {
|
||||
for (const timing of ["--timings", "--no-timings", "--timings=false"]) {
|
||||
const timed = await cli.run({
|
||||
arguments: [timing, "mcp", timing, "ad", "--format", "json"],
|
||||
});
|
||||
expect(timed.json).toMatchObject({
|
||||
cta: {
|
||||
commands: [
|
||||
{ command: `rea ${timing} mcp ${timing} add --format json` },
|
||||
{},
|
||||
],
|
||||
},
|
||||
});
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe("unknown MCP subcommand output", () => {
|
||||
workspaceCliTest(
|
||||
"reports the rejection in the explicitly selected format",
|
||||
async ({ cli }) => {
|
||||
const expected = {
|
||||
code: "COMMAND_NOT_FOUND",
|
||||
message: "'bogus' is not a command for 'rea mcp'.",
|
||||
cta: {
|
||||
commands: [
|
||||
{
|
||||
command: "rea mcp --help",
|
||||
description: "see all available commands",
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
for (const selection of [["--format", "json"], ["--json"]]) {
|
||||
const result = await cli.run({
|
||||
arguments: ["mcp", "bogus", ...selection],
|
||||
});
|
||||
expect(result.exitCode).toBe(1);
|
||||
expect(result.json).toMatchObject(expected);
|
||||
expect(result.stdout).toBe(`${JSON.stringify(result.json)}\n`);
|
||||
}
|
||||
const yaml = await cli.run({
|
||||
arguments: ["mcp", "bogus", "--format", "yaml"],
|
||||
});
|
||||
expect(yaml.exitCode).toBe(1);
|
||||
expect(yaml.stdout).toContain(" - command: rea mcp --help\n");
|
||||
const markdown = await cli.run({
|
||||
arguments: ["mcp", "bogus", "--format", "md"],
|
||||
});
|
||||
expect(markdown.exitCode).toBe(1);
|
||||
expect(markdown.stdout).toMatch(/^## code\n\nCOMMAND_NOT_FOUND\n/u);
|
||||
const jsonLines = await cli.run({
|
||||
arguments: ["mcp", "bogus", "--format", "jsonl"],
|
||||
});
|
||||
expect(jsonLines.exitCode).toBe(1);
|
||||
expect(JSON.parse(jsonLines.stdout)).toMatchObject({
|
||||
type: "error",
|
||||
ok: false,
|
||||
result: expected,
|
||||
});
|
||||
const suggested = await cli.run({
|
||||
arguments: ["mcp", "ad", "--format", "json"],
|
||||
});
|
||||
expect(suggested.json).toMatchObject({
|
||||
message: "'ad' is not a command for 'rea mcp'. Did you mean 'add'?",
|
||||
cta: { commands: [{ command: "rea mcp add --format json" }, {}] },
|
||||
});
|
||||
// Only the subcommand is corrected; an equal option value is caller input.
|
||||
const optionValue = await cli.run({
|
||||
arguments: ["mcp", "ad", "--command", "ad", "--json"],
|
||||
});
|
||||
expect(optionValue.json).toMatchObject({
|
||||
cta: { commands: [{ command: "rea mcp add --command ad --json" }, {}] },
|
||||
});
|
||||
// Builtins Incur serves before its mcp branch keep their own output.
|
||||
const manifest = await cli.run({
|
||||
arguments: ["mcp", "bogus", "--llms", "--json"],
|
||||
});
|
||||
expect(manifest.exitCode).toBe(0);
|
||||
expect(manifest.stdout).not.toContain("COMMAND_NOT_FOUND");
|
||||
// An output value spelled like a builtin is data, not that builtin.
|
||||
for (const arguments_ of [
|
||||
[
|
||||
"mcp",
|
||||
"ad",
|
||||
"--format",
|
||||
"csv",
|
||||
"--filter-output",
|
||||
"cta.commands",
|
||||
"--columns",
|
||||
"--llms",
|
||||
],
|
||||
["mcp", "ad", "--json", "--filter-output", "--llms"],
|
||||
]) {
|
||||
const builtinValue = await cli.run({ arguments: arguments_ });
|
||||
expect(builtinValue.exitCode).toBe(1);
|
||||
expect(JSON.parse(builtinValue.stdout)).toMatchObject({
|
||||
code: "COMMAND_NOT_FOUND",
|
||||
});
|
||||
}
|
||||
// The suggestion keeps the caller's delimited output selection.
|
||||
const delimited = await cli.run({
|
||||
arguments: [
|
||||
"mcp",
|
||||
"ad",
|
||||
"--format",
|
||||
"csv",
|
||||
"--filter-output",
|
||||
"cta.commands",
|
||||
"--columns",
|
||||
"command",
|
||||
],
|
||||
});
|
||||
expect(delimited.exitCode).toBe(1);
|
||||
expect(delimited.stdout).toContain(
|
||||
"rea mcp add --format csv --filter-output cta.commands --columns command",
|
||||
);
|
||||
// A suggestion that would not replay the caller's tokens is omitted.
|
||||
const spaced = await cli.run({
|
||||
arguments: ["mcp", "ad", "--command", "/path with spaces", "--json"],
|
||||
});
|
||||
expect(spaced.json).toMatchObject({
|
||||
message: "'ad' is not a command for 'rea mcp'. Did you mean 'add'?",
|
||||
cta: {
|
||||
commands: [
|
||||
{
|
||||
command: "rea mcp --help",
|
||||
description: "see all available commands",
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(spaced.stdout).not.toContain("rea mcp add");
|
||||
for (const shellActive of ["@args", "%PATH%", "=node"]) {
|
||||
const result = await cli.run({
|
||||
arguments: ["mcp", "ad", "--command", shellActive, "--json"],
|
||||
});
|
||||
expect(result.exitCode).toBe(1);
|
||||
expect(result.stdout).not.toContain("rea mcp add");
|
||||
expect(result.json).toMatchObject({
|
||||
cta: { commands: [{ command: "rea mcp --help" }] },
|
||||
});
|
||||
}
|
||||
// Markdown output escapes caller text through REA's document renderer.
|
||||
const emphasis = await cli.run({
|
||||
arguments: ["mcp", "*oops*", "--format", "md"],
|
||||
});
|
||||
expect(emphasis.exitCode).toBe(1);
|
||||
expect(emphasis.stdout).toContain("'\\*oops\\*' is not a command");
|
||||
// A declared literal is redacted from fixed call-to-action text too.
|
||||
const fixedText = await cli.run({
|
||||
arguments: ["mcp", "bogus", "--sensitive-value", "help", "--json"],
|
||||
});
|
||||
expect(fixedText.exitCode).toBe(1);
|
||||
expect(fixedText.stdout).not.toContain("help");
|
||||
const redacted = await cli.run({
|
||||
arguments: ["mcp", "s3cret", "--sensitive-value", "s3cret", "--json"],
|
||||
});
|
||||
expect(redacted.exitCode).toBe(1);
|
||||
expect(redacted.stdout).not.toContain("s3cret");
|
||||
expect(redacted.json).toMatchObject({ code: "COMMAND_NOT_FOUND" });
|
||||
// An empty token keeps Incur's group help.
|
||||
const help = await cli.run({ arguments: ["mcp", "", "--json"] });
|
||||
expect(help.exitCode).toBe(0);
|
||||
expect(help.stdout).toContain("rea mcp");
|
||||
// Without a selected format, Incur's default TOON remains the output.
|
||||
const toon = await cli.run({ arguments: ["mcp", "bogus"] });
|
||||
expect(toon.exitCode).toBe(1);
|
||||
expect(toon.stdout).toMatch(/^code: COMMAND_NOT_FOUND\n/u);
|
||||
},
|
||||
CLI_INTEGRATION_TIMEOUT_MS,
|
||||
);
|
||||
});
|
||||
Reference in new issue
Block a user