Files
software-mansion__argent/packages/argent-cli/test/run-required-flags.test.ts
filip131311 d08c3a3cfc fix(cli): name the missing flag instead of printing a Zod dump (#648)
Fixes #645.

## Before

```
$ argent run gesture-tap
[
  { "expected": "string", "code": "invalid_type", "path": ["udid"],
    "message": "Invalid input: expected string, received undefined" },
  ... two more
]
```

## After

```
$ argent run gesture-tap
Error: missing required flags --udid, --x, --y

argent run gesture-tap [flags]
...full per-flag help...
```

## Scope

The issue names the missing-flag case. Reproducing it showed the same
raw dump has a second
trigger: any constraint the tool itself rejects, which the CLI cannot
know in advance.

| invocation | before | after |
|---|---|---|
| `gesture-tap` (no flags) | raw issue array | `missing required flags
--udid, --x, --y` |
| `screenshot --scale 99` | raw issue array | `--scale Too big: expected
number to be <=1` |
| `gesture-scroll --udid X --x .5 --y .5` (zero deltas) | raw issue
array | `Pass a non-zero deltaX and/or deltaY …` |
| `run-sequence --udid X` | raw issue array | `missing required flag
--steps-json` |
| `gesture-tap --x abc` | already good | unchanged |
| runtime failure | plain message, exit 1 | unchanged |

Fixing only the first row would have left the CLI's error voice less
predictable than before, so
both go through the same renderer.

## How it works

Two sources, one output. Missing flags are found locally from the tool's
published schema — a
**presence** check only, never a type or range judgement, which stays
exclusively the server's.
That is sound because the schema generator excludes defaulted fields
from `required`
(`debugger-connect` requires `device_id`, not `port`), so it cannot
reject a call the server would
have accepted. Everything else is read off the rejection the tool-server
already returns.

Both produce one report rendered by one function and reported through
one call site, so the
wording, the output channel and the exit code cannot drift between them.

Recognition of a rejection is **structural** — the shape of the issue
list plus the tool's own
schema — never the wording of a message, which is not ours to depend on
and changes between Zod
releases. Four gates must all hold, and any failure leaves the existing
handling untouched.

Details worth a reviewer's attention:

- **A rule spanning the whole payload reports no field.**
`gesture-scroll` and `gesture-rotate`
validate at the object level, and `screenshot-diff` is `.strict()`.
Treating those as
field-scoped would print `Error: --undefined …`; they are stated on
their own instead.
- **A rule firing on an *optional* field is not called a missing
required flag.**
`await-ui-element` refines on `expectedText`, which it does not require
— reporting that as
missing would contradict the help block printed directly underneath,
where the field carries no
  `(required)` marker.
- **The check runs after `--args` is merged**, so `argent run
gesture-tap --args '{...}'` is not
falsely reported as missing everything. Verified live: it reaches the
device and taps.
- Presence uses own-key semantics, so a required field named e.g.
`constructor` is not treated as
  supplied by the prototype chain.

## Behaviour changes a reviewer should weigh

- **A tool-rejected value now exits 2 instead of 1.** `run.ts` already
uses 2 for bad input and 1
for a failed run; a Zod 400 means the tool never ran. No in-repo
consumer tests for a specific
code — `scripts/e2e-full` only checks zero/non-zero. Note this does
**not** make 2 universal:
  an unknown tool name still exits 1, which is untouched here.
- **`--json` gains a defined failure shape** (it had none — the blob
printed regardless). One
object on stderr, nothing on stdout, so `--json | jq` on a failed run
reads an empty stream and
  a non-zero status:
  ```json
  { "error": "missing required flags --udid, --x, --y",
    "missing": ["--udid", "--x", "--y"],
    "issues": [] }
  ```
`issues` carries the tool's list verbatim, which is the raw data the
issue asked to keep.
- **One adjacent fix**, called out because it is not the reported bug:
the global-option parse ran
*outside* the error handler, so `argent run screenshot --out` printed a
raw stack trace. Same
  bad-input-to-ugly-output defect, one line above the fix.

## Not done

- The unknown-flag hole (`argent run gesture-tap --bogusflag 1` is
silently accepted and the tool
runs). The pass-through is deliberate — see the comment in
`flag-parser.ts` — and belongs with
  #586, the same CLI-argument family.
- `argent flow` builds its own payload and has its own required check;
it cannot reach this path.
- The server still returns the issue list verbatim, so MCP and other
clients are unaffected. All
  prettifying is CLI-side.

## Verification

Live against the built CLI for every row of the table above, plus:
`debugger-connect` asks only
for `--device_id` and never `--port`; `--args '{...}'` reaches the
device and taps; a thrown
runtime error still prints its bare message with no `Error:` prefix, no
help block, exit 1.

43 new tests. `run-telemetry.test.ts` passes **unedited** — it already
asserted the non-Zod
passthrough, which makes it the regression guard for the untouched path.
Full CLI suite 320
passed, registry 87 passed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---

> **#699 is stacked on this branch** — it conflicts with this one in
`flag-parser.ts`, so it was rebased on top rather than left to collide
at merge time. Merge this first.
2026-08-03 11:42:26 +02:00

238 lines
8.1 KiB
TypeScript

import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { run } from "../src/run.js";
const toolsClientMock = vi.hoisted(() => ({
fetchTool: vi.fn(),
callTool: vi.fn(),
baseUrl: vi.fn(async () => ({ url: "http://127.0.0.1:3001", token: "t" })),
}));
const telemetryMock = vi.hoisted(() => ({
init: vi.fn(),
shutdown: vi.fn(async () => undefined),
track: vi.fn(),
}));
vi.mock("@argent/tools-client", async (importOriginal) => {
const actual = await importOriginal<Record<string, unknown>>();
return {
...actual,
createToolsClient: vi.fn(() => toolsClientMock),
materializeArtifacts: vi.fn(async (result: unknown) => ({ result, images: [] })),
};
});
vi.mock("@argent/telemetry", () => telemetryMock);
const gestureTap = {
name: "gesture-tap",
description: "Tap the screen",
inputSchema: {
type: "object",
properties: {
udid: { type: "string" },
x: { type: "number" },
y: { type: "number" },
clickCount: { type: "integer" },
},
required: ["udid", "x", "y"],
},
};
const runSequence = {
name: "run-sequence",
description: "Run steps",
inputSchema: {
type: "object",
properties: {
udid: { type: "string" },
steps: { type: "array", items: { type: "object" } },
},
required: ["udid", "steps"],
},
};
const invoke = (argv: string[]) => run(argv, { paths: {} as never });
function stderr(): string {
return (console.error as unknown as { mock: { calls: unknown[][] } }).mock.calls
.map((c) => String(c[0]))
.join("\n");
}
function stdout(): string {
return (console.log as unknown as { mock: { calls: unknown[][] } }).mock.calls
.map((c) => String(c[0]))
.join("\n");
}
describe("argent run input validation", () => {
let exitSpy: ReturnType<typeof vi.spyOn>;
let errorSpy: ReturnType<typeof vi.spyOn>;
let logSpy: ReturnType<typeof vi.spyOn>;
beforeEach(() => {
vi.clearAllMocks();
toolsClientMock.fetchTool.mockResolvedValue(gestureTap);
toolsClientMock.callTool.mockResolvedValue({ data: { tapped: true } });
exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => {
throw new Error(`process.exit:${code}`);
}) as typeof process.exit);
errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined);
logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined);
});
afterEach(() => {
exitSpy.mockRestore();
errorSpy.mockRestore();
logSpy.mockRestore();
});
it("names the missing flags and shows the tool's help without calling the tool", async () => {
await expect(invoke(["gesture-tap"])).rejects.toThrow("process.exit:2");
expect(toolsClientMock.callTool).not.toHaveBeenCalled();
expect(stderr()).toContain("Error: missing required flags --udid, --x, --y");
expect(stdout()).toContain("argent run gesture-tap [flags]");
expect(stdout()).toContain("--udid <value>");
expect(telemetryMock.track).toHaveBeenCalledWith(
"cli:run_fail",
expect.objectContaining({
tool: "gesture-tap",
error_code: "CLI_RUN_INPUT_VALIDATION_FAILED",
failure_stage: "cli_run_required_flags",
failure_area: "cli",
error_kind: "validation",
})
);
});
it("never prints a raw issue list", async () => {
await expect(invoke(["gesture-tap"])).rejects.toThrow("process.exit:2");
expect(stderr()).not.toContain("invalid_type");
expect(stderr()).not.toContain('"path"');
});
it("accepts required fields supplied through --args", async () => {
await invoke(["gesture-tap", "--args", '{"udid":"X","x":0.5,"y":0.5}']);
expect(toolsClientMock.callTool).toHaveBeenCalledWith("gesture-tap", {
udid: "X",
x: 0.5,
y: 0.5,
});
});
it("accepts required fields split across --args and flags", async () => {
await invoke(["gesture-tap", "--udid", "X", "--args", '{"x":0.5,"y":0.5}']);
expect(toolsClientMock.callTool).toHaveBeenCalledTimes(1);
});
it("reports only the fields still missing after --args is merged", async () => {
await expect(invoke(["gesture-tap", "--args", '{"x":0.5}'])).rejects.toThrow("process.exit:2");
expect(stderr()).toContain("missing required flags --udid, --y");
});
it("names a JSON-only field by its -json flag", async () => {
toolsClientMock.fetchTool.mockResolvedValue(runSequence);
await expect(invoke(["run-sequence", "--udid", "X"])).rejects.toThrow("process.exit:2");
expect(stderr()).toContain("Error: missing required flag --steps-json");
});
it("accepts a JSON-only field passed as --field-json", async () => {
toolsClientMock.fetchTool.mockResolvedValue(runSequence);
await invoke(["run-sequence", "--udid", "X", "--steps-json", '[{"tool":"button"}]']);
expect(toolsClientMock.callTool).toHaveBeenCalledTimes(1);
});
it("still prints help for --help without demanding flags", async () => {
await invoke(["gesture-tap", "--help"]);
expect(toolsClientMock.callTool).not.toHaveBeenCalled();
expect(stdout()).toContain("argent run gesture-tap [flags]");
expect(stderr()).not.toContain("missing required");
});
describe("a value the tool rejects", () => {
beforeEach(() => {
toolsClientMock.callTool.mockRejectedValue(
new Error(
JSON.stringify([{ code: "too_big", path: ["x"], message: "Too big: expected <=1" }])
)
);
});
it("is reported against its flag instead of as a raw issue list", async () => {
await expect(
invoke(["gesture-tap", "--udid", "X", "--x", "99", "--y", "0.5"])
).rejects.toThrow("process.exit:2");
expect(stderr()).toContain("Error: --x Too big: expected <=1");
expect(stderr()).not.toContain('"code"');
expect(stdout()).toContain("argent run gesture-tap [flags]");
expect(telemetryMock.track).toHaveBeenCalledWith(
"cli:run_fail",
expect.objectContaining({
error_code: "CLI_RUN_INPUT_VALIDATION_FAILED",
failure_stage: "cli_run_server_validation",
error_kind: "validation",
})
);
});
});
it("leaves an ordinary runtime failure exactly as it was", async () => {
toolsClientMock.callTool.mockRejectedValue(new Error("Simulator not booted"));
await expect(
invoke(["gesture-tap", "--udid", "X", "--x", "0.5", "--y", "0.5"])
).rejects.toThrow("process.exit:1");
expect(console.error).toHaveBeenCalledWith("Simulator not booted");
expect(stderr()).not.toContain("Error: ");
expect(stdout()).not.toContain("argent run gesture-tap [flags]");
expect(telemetryMock.track).toHaveBeenCalledWith(
"cli:run_fail",
expect.objectContaining({ error_code: "CLI_RUN_TOOL_CALL_FAILED", error_kind: "unknown" })
);
});
describe("--json", () => {
it("emits one machine-readable object on stderr and nothing on stdout", async () => {
await expect(invoke(["gesture-tap", "--json"])).rejects.toThrow("process.exit:2");
expect(stdout()).toBe("");
const envelope = JSON.parse(stderr());
expect(envelope).toEqual({
error: "missing required flags --udid, --x, --y",
missing: ["--udid", "--x", "--y"],
issues: [],
});
});
it("carries the tool's own issue list when the tool rejected a value", async () => {
const issues = [{ code: "too_big", path: ["x"], message: "Too big: expected <=1" }];
toolsClientMock.callTool.mockRejectedValue(new Error(JSON.stringify(issues)));
await expect(
invoke(["gesture-tap", "--json", "--udid", "X", "--x", "99", "--y", "0.5"])
).rejects.toThrow("process.exit:2");
const envelope = JSON.parse(stderr());
expect(envelope.issues).toEqual(issues);
expect(envelope.missing).toEqual([]);
expect(envelope.error).toBe("--x Too big: expected <=1");
expect(stdout()).toBe("");
});
});
it("reports a missing value for a global option instead of a raw stack", async () => {
await expect(invoke(["gesture-tap", "--out"])).rejects.toThrow("process.exit:2");
expect(stderr()).toContain("Error: --out requires a path");
expect(stderr()).toContain("argent run gesture-tap --help");
});
});