mirror of
https://github.com/morluto/rea.git
synced 2026-11-07 05:56:19 -10:00
fix(cli): prevent mcp discovery from registering clients (#7919)
* fix(cli): prevent mcp discovery from registering clients * fix(cli): use shared output parsing for discovery errors
This commit is contained in:
1 parent
33d6975749
commit
9e1a1e700a
4 files changed
+155
-5
No files matched your search
+57
@@ -23,7 +23,13 @@ import { Cli, z } from "incur";
|
||||
import {
|
||||
adaptCliOutputArgumentsForIncur,
|
||||
normalizeCliArguments,
|
||||
splitIncurBuiltinArguments,
|
||||
} from "./cliBuiltinArguments.js";
|
||||
import {
|
||||
parseCliOutputArguments,
|
||||
renderCliDocument,
|
||||
renderCliOutputArgumentError,
|
||||
} from "./cliOutput.js";
|
||||
|
||||
import { createLogger } from "./logger.js";
|
||||
import { parseLogLevel } from "./config/processStart.js";
|
||||
@@ -208,6 +214,57 @@ export const createCli = (
|
||||
cli.serve = (arguments_ = process.argv.slice(2), options) => {
|
||||
if (complete(arguments_, options ?? {})) return Promise.resolve();
|
||||
const normalized = normalizeCliArguments(arguments_);
|
||||
const { command, flags } = splitIncurBuiltinArguments(normalized);
|
||||
const builtin = command[0] === PRODUCT_IDENTITY.cliBinary ? 1 : 0;
|
||||
if (
|
||||
command[builtin] === "mcp" &&
|
||||
command[builtin + 1] === "add" &&
|
||||
!flags.has("--help") &&
|
||||
!flags.has("-h") &&
|
||||
!flags.has("--llms") &&
|
||||
!flags.has("--llms-full")
|
||||
) {
|
||||
const write =
|
||||
options?.stdout ?? ((text: string) => process.stdout.write(text));
|
||||
if (flags.has("--version")) {
|
||||
write(`${PRODUCT_IDENTITY.packageVersion}\n`);
|
||||
return Promise.resolve();
|
||||
}
|
||||
if (flags.has("--schema")) {
|
||||
const output = parseCliOutputArguments(normalized);
|
||||
if (!output.validation.ok)
|
||||
write(renderCliOutputArgumentError(output.validation));
|
||||
else if (output.format === "csv" || output.format === "tsv")
|
||||
write(
|
||||
renderCliOutputArgumentError({
|
||||
ok: false,
|
||||
format: "json",
|
||||
code: "UNSUPPORTED_OUTPUT_COMBINATION",
|
||||
message:
|
||||
"CSV/TSV output cannot represent the mcp add schema discovery error. Use a document format instead.",
|
||||
}),
|
||||
);
|
||||
else
|
||||
write(
|
||||
renderCliDocument(
|
||||
{
|
||||
code: "SCHEMA_UNAVAILABLE",
|
||||
message:
|
||||
"Schema discovery is not supported for mcp add. Use 'rea mcp add --help' for its options.",
|
||||
},
|
||||
output.format,
|
||||
process.stdout.isTTY === true,
|
||||
),
|
||||
);
|
||||
(
|
||||
options?.exit ??
|
||||
((code: number) => {
|
||||
process.exitCode = code;
|
||||
})
|
||||
)(1);
|
||||
return Promise.resolve();
|
||||
}
|
||||
}
|
||||
const result = serveStrict(normalized, options ?? {}, (tokens, selected) =>
|
||||
serveDeclared(tokens, selected, (declared, declaredOptions) =>
|
||||
serve(adaptCliOutputArgumentsForIncur(declared), declaredOptions),
|
||||
|
||||
@@ -35,6 +35,8 @@ export const INCUR_BUILTIN_FLAGS: ReadonlySet<string> = new Set([
|
||||
export interface IncurBuiltinArgumentSplit {
|
||||
/** Tokens left for command resolution and command option parsing. */
|
||||
readonly command: readonly string[];
|
||||
/** Valueless global flags extracted by Incur before command dispatch. */
|
||||
readonly flags: ReadonlySet<string>;
|
||||
/** Positions of nonempty values consumed by valued output flags. */
|
||||
readonly outputValues: readonly {
|
||||
readonly flag: string;
|
||||
@@ -51,6 +53,7 @@ export const splitIncurBuiltinArguments = (
|
||||
arguments_: readonly string[],
|
||||
): IncurBuiltinArgumentSplit => {
|
||||
const command: string[] = [];
|
||||
const flags = new Set<string>();
|
||||
const outputValues: { readonly flag: string; readonly index: number }[] = [];
|
||||
for (let index = 0; index < arguments_.length; index += 1) {
|
||||
const argument = arguments_[index]!;
|
||||
@@ -59,8 +62,10 @@ export const splitIncurBuiltinArguments = (
|
||||
INCUR_BUILTIN_FLAGS.has(argument) ||
|
||||
// A following value belongs to a command-local `--version` option.
|
||||
(argument === "--version" && (next === undefined || next.startsWith("-")))
|
||||
)
|
||||
) {
|
||||
flags.add(argument);
|
||||
continue;
|
||||
}
|
||||
if (VALUED_OUTPUT_FLAGS.has(argument) && next) {
|
||||
outputValues.push({ flag: argument, index: index + 1 });
|
||||
index += 1;
|
||||
@@ -68,7 +73,7 @@ export const splitIncurBuiltinArguments = (
|
||||
}
|
||||
command.push(argument);
|
||||
}
|
||||
return { command, outputValues };
|
||||
return { command, flags, outputValues };
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
+3
-3
@@ -97,7 +97,7 @@ export type CliOutputArgumentValidation =
|
||||
interface CliOutputControls {
|
||||
readonly filterOutput: string | undefined;
|
||||
readonly columns: readonly string[] | undefined;
|
||||
readonly format: string;
|
||||
readonly format: CliOutputFormat;
|
||||
/** Whether `--json` or `--format` selected the format, rather than the default. */
|
||||
readonly formatExplicit: boolean;
|
||||
readonly fullOutput: boolean;
|
||||
@@ -120,7 +120,7 @@ export interface ParsedCliOutputArguments extends CliOutputControls {
|
||||
export const parseCliOutputArguments = (
|
||||
arguments_: readonly string[],
|
||||
): ParsedCliOutputArguments => {
|
||||
let format: string = "toon";
|
||||
let format: CliOutputFormat = "toon";
|
||||
let rootVersion = false;
|
||||
let versionEligible = true;
|
||||
let formatExplicit = false;
|
||||
@@ -198,7 +198,7 @@ export const parseCliOutputArguments = (
|
||||
if (value) {
|
||||
const rejection = cliOutputValueRejection(argument, value);
|
||||
if (rejection !== undefined) invalidValue ??= rejection;
|
||||
else {
|
||||
else if (isCliOutputFormat(value)) {
|
||||
format = value;
|
||||
formatExplicit = true;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
import { access, mkdir, readFile, writeFile } from "node:fs/promises";
|
||||
import { join, resolve } from "node:path";
|
||||
|
||||
import { expect } from "vitest";
|
||||
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
import { cliTest } from "../../support/cli/cliFixture.js";
|
||||
|
||||
cliTest(
|
||||
"mcp add discovery does not create or change Amp settings",
|
||||
async ({ processes }) => {
|
||||
const home = await createTestTempDirectory("rea-mcp-discovery-");
|
||||
const path = join(home, ".config", "amp", "settings.json");
|
||||
const run = (args: string[]) =>
|
||||
processes.run(process.execPath, [resolve("scripts/rea.mjs"), ...args], {
|
||||
env: {
|
||||
...process.env,
|
||||
HOME: home,
|
||||
XDG_CONFIG_HOME: join(home, ".config"),
|
||||
},
|
||||
timeoutMs: 30_000,
|
||||
});
|
||||
|
||||
const schema = await run([
|
||||
"mcp",
|
||||
"add",
|
||||
"--agent",
|
||||
"amp",
|
||||
"--schema",
|
||||
"--json",
|
||||
]);
|
||||
expect(schema.exitCode).toBe(1);
|
||||
expect(JSON.parse(schema.stdout)).toMatchObject({
|
||||
code: "SCHEMA_UNAVAILABLE",
|
||||
});
|
||||
await expect(access(path)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
|
||||
for (const format of ["csv", "tsv"]) {
|
||||
const result = await run([
|
||||
"mcp",
|
||||
"add",
|
||||
"--agent",
|
||||
"amp",
|
||||
"--schema",
|
||||
"--format",
|
||||
format,
|
||||
"--filter-output",
|
||||
"rows",
|
||||
"--columns",
|
||||
"code",
|
||||
]);
|
||||
expect(result.exitCode).toBe(1);
|
||||
expect(JSON.parse(result.stdout)).toMatchObject({
|
||||
error: { code: "UNSUPPORTED_OUTPUT_COMBINATION" },
|
||||
});
|
||||
await expect(access(path)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
}
|
||||
|
||||
// A flag-shaped output value remains data; the CLI rejects the invalid
|
||||
// format before dispatching registration or treating it as discovery.
|
||||
const invalidFormat = await run([
|
||||
"mcp",
|
||||
"add",
|
||||
"--agent",
|
||||
"amp",
|
||||
"--format",
|
||||
"--schema",
|
||||
]);
|
||||
expect(invalidFormat.exitCode).toBe(1);
|
||||
expect(invalidFormat.stdout).not.toContain("SCHEMA_UNAVAILABLE");
|
||||
await expect(access(path)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
|
||||
await mkdir(join(home, ".config", "amp"), { recursive: true });
|
||||
const original = '{"existing":true}\n';
|
||||
await writeFile(path, original);
|
||||
for (const flag of ["--schema", "--version", "--help"]) {
|
||||
const result = await run(["mcp", "add", "--agent", "amp", flag]);
|
||||
expect(result.stdout).not.toContain("Registered rea");
|
||||
expect(result.exitCode).toBe(flag === "--schema" ? 1 : 0);
|
||||
expect(await readFile(path, "utf8")).toBe(original);
|
||||
}
|
||||
const registration = await run(["mcp", "add", "--agent", "amp"]);
|
||||
expect(registration.exitCode).toBe(0);
|
||||
expect(
|
||||
JSON.parse(await readFile(path, "utf8"))["amp.mcpServers"].rea,
|
||||
).toBeDefined();
|
||||
},
|
||||
);
|
||||
Reference in new issue
Block a user