mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
a6f1bef632
Fixes #638. ## The report understated it The issue says `argent mcp --help` "does not hang and does not start the server — it simply prints nothing". Reproducing it showed the opposite: ``` $ argent mcp --help < /dev/null ; echo $? 0 # 0 bytes ← what the reporter saw $ argent mcp --help # stdin open (a pipe, an editor, a CI step) # HANGS — I hit a 300s timeout ``` `--help` **starts the MCP stdio server**, which blocks reading JSON-RPC from stdin. The "prints nothing, exit 0" is just the server getting an immediate EOF from a closed stdin. So this is the same "`--help` runs the real command" bug that was fixed for the installers — `mcp` was missed. `startMcpServer` takes no argv at all (`mcp-server.ts:96` — its only parameter is `{ paths }`), so a help flag was *structurally* unreachable; there was nowhere downstream that could have handled it. ## Fix `mcp` joins the existing guarded set rather than growing a second help path. That set's defining property is "the handler never inspects its argv", and `mcp` qualifies more strongly than the installers do — it isn't merely ignoring `rest`, it is never handed it. **It cannot fire on a real launch.** The guard tests `rest`, which is `[]` for every generated editor config (all four shapes end at `mcp`, and `isArgentManagedEntry` requires exactly that), and `[].some(...)` evaluates no predicate. This is pinned in both directions, because a guard that *did* fire here would print usage onto the JSON-RPC channel and break every editor at handshake — the highest-cost regression in this file, and it was untested. The top-level table's `mcp` row now reads the same metadata as its own help, so the two cannot drift. **No rename.** An earlier draft renamed the module and six identifiers (`installer-help` → `command-help`, etc.) on the grounds that its comments had become false. They hadn't quite: fixing `mcp` *and* `tools` makes "every other subcommand parses `--help` itself" true again — I checked all ten siblings. Since #530 and #568 both edit `cli.ts`, a rename would have forced them through churn for no behavioural gain. Prose-only. ## Second commit: `argent tools --help` The issue mentions it in passing; it's a different mechanism and a genuinely dead branch. `argv.filter((a) => !a.startsWith("--"))` dropped `--help` before the subcommand was read, so it fell through to listing all 77 tools — **contacting the tool-server, starting one if none was running**, to do it. Only `-h` ever reached the existing help branch. Scoped to the subcommand position on purpose: a global check would have broken `argent tools describe <name> --help`, which today prints *that tool's* flags. It still does — there is a test. Separate commit so it can be reverted alone. ## Verification Live, against the built CLI: | | before | after | |---|---|---| | `mcp --help`, stdin open | hangs indefinitely | usage, returns immediately | | `mcp --help`, stdin closed | 0 bytes, exit 0 | 849 bytes of help, exit 0 | | `mcp` (real launch) | works | works — verified with an actual `initialize` handshake, server replies with its capabilities | | `mcp --metro-port 8082` | starts server | starts server (guard keys on help, not on "any argv") | | `tools --help` | 77-line catalogue | usage | | `tools describe gesture-tap --help` | tool's flags | tool's flags | | `argent --help` `mcp` row | — | byte-identical | argent 80 passed, argent-cli 284 passed. ## Note on the help text Every claim in it is sourced (stdio/JSON-RPC transport, stdout ownership, the `~/.argent/mcp-calls.log` default and `ARGENT_MCP_LOG` override, auto-spawn vs `ARGENT_TOOLS_URL`/`argent link`). It deliberately does **not** suggest running `argent mcp` by hand to check an install — that is the reported hang — and instead says it will look like it has hung and why. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
77 lines
2.7 KiB
TypeScript
77 lines
2.7 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { tools } from "../src/tools.js";
|
|
|
|
// `argent tools --help` used to print the whole tool catalogue — and to contact
|
|
// the tool-server (starting one) to get it — because the help flag was filtered
|
|
// out before the subcommand was read. These drive the real entry point with a
|
|
// mocked client so "did it reach the server" is observable.
|
|
|
|
const toolsClientMock = vi.hoisted(() => ({
|
|
fetchTool: vi.fn(),
|
|
fetchTools: vi.fn(),
|
|
}));
|
|
|
|
vi.mock("@argent/tools-client", () => ({
|
|
createToolsClient: vi.fn(() => toolsClientMock),
|
|
}));
|
|
|
|
let logSpy: ReturnType<typeof vi.spyOn>;
|
|
let errSpy: ReturnType<typeof vi.spyOn>;
|
|
|
|
beforeEach(() => {
|
|
vi.clearAllMocks();
|
|
toolsClientMock.fetchTools.mockResolvedValue([
|
|
{ name: "gesture-tap", description: "Tap the screen" },
|
|
]);
|
|
toolsClientMock.fetchTool.mockResolvedValue({
|
|
name: "gesture-tap",
|
|
description: "Tap the screen",
|
|
inputSchema: { type: "object", properties: { udid: { type: "string" } }, required: ["udid"] },
|
|
});
|
|
logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
|
|
errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
|
});
|
|
|
|
afterEach(() => {
|
|
logSpy.mockRestore();
|
|
errSpy.mockRestore();
|
|
});
|
|
|
|
const output = () => (logSpy.mock.calls as unknown[][]).map((c) => String(c[0] ?? "")).join("\n");
|
|
|
|
const invoke = (argv: string[]) => tools(argv, { paths: {} as never });
|
|
|
|
describe("argent tools --help", () => {
|
|
it.each([["--help"], ["-h"], ["--help", "--json"], ["--json", "--help"]])(
|
|
"prints usage for %j without contacting the tool-server",
|
|
async (...argv) => {
|
|
await invoke(argv);
|
|
|
|
expect(output()).toContain("argent tools describe <name>");
|
|
expect(output()).toContain("--help, -h");
|
|
// The defect: help used to start a tool-server just to list tools.
|
|
expect(toolsClientMock.fetchTools).not.toHaveBeenCalled();
|
|
expect(toolsClientMock.fetchTool).not.toHaveBeenCalled();
|
|
}
|
|
);
|
|
|
|
it("still lists tools when no subcommand is given", async () => {
|
|
await invoke([]);
|
|
expect(toolsClientMock.fetchTools).toHaveBeenCalled();
|
|
});
|
|
|
|
it("still describes a named tool", async () => {
|
|
await invoke(["describe", "gesture-tap"]);
|
|
expect(toolsClientMock.fetchTool).toHaveBeenCalledWith("gesture-tap");
|
|
});
|
|
|
|
it("leaves a trailing --help for the tool being described", async () => {
|
|
// `argent tools describe <name> --help` asks for that tool's flags, not for
|
|
// this command's usage.
|
|
await invoke(["describe", "gesture-tap", "--help"]);
|
|
|
|
expect(toolsClientMock.fetchTool).toHaveBeenCalledWith("gesture-tap");
|
|
expect(output()).not.toContain("argent tools describe <name>");
|
|
});
|
|
});
|