diff --git a/packages/argent-cli/src/flow.ts b/packages/argent-cli/src/flow.ts new file mode 100644 index 000000000..1dbf24c94 --- /dev/null +++ b/packages/argent-cli/src/flow.ts @@ -0,0 +1,481 @@ +import * as fsp from "node:fs/promises"; +import * as path from "node:path"; +import { + createToolsClient, + isArtifactHandle, + materializeArtifacts, + type MaterializeContext, + type ToolsServerPaths, +} from "@argent/tools-client"; +import { FlagParseException } from "./flag-parser.js"; + +export interface FlowCommandOptions { + paths: ToolsServerPaths; +} + +export interface StepReport { + index: number; + kind: string; + status: "pass" | "fail" | "skip" | "error"; + reason?: string; + /** + * Legacy: older tool-servers passed a snapshot that adopted a missing + * baseline and annotated it with this caveat (a missing baseline now fails + * the step). Rendered for wire compat with a not-yet-updated server. + */ + warning?: string; + tool?: string; + flow?: string; + message?: string; + /** Human-readable step target (selector / snapshot name), set by the runner. */ + target?: string; + /** Baseline key stem (`__-WxH`) on artifact-bearing snapshot steps. */ + snapshotKey?: string; + /** + * Snapshot-step artifacts keyed by role (baseline/current/diff). The wire + * value is an artifact handle (or a plain path string from a legacy + * tool-server); by render time each has been rewritten to a string — a + * durable local copy for the failed snapshots `--output` exports, otherwise + * the handle's server-side hostPath/filename — or null when a needed + * download failed. + */ + artifacts?: Record; +} + +export interface FlowReport { + flow: string; + device: string; + executionPrerequisite?: string; + ok: boolean; + passed: number; + failed: number; + skipped: number; + errored: number; + steps: StepReport[]; +} + +const STATUS_GLYPH: Record = { + pass: "✓", + fail: "✗", + error: "✗", + skip: "·", +}; + +function printHelp(): void { + console.log(`Usage: argent flow [options] + +Run a saved flow without an LLM in the loop. Flows live in +\`.argent/flows/.yaml\` under the current working directory. A flow that +begins with a \`launch\` step runs its app from scratch; any other flow (a +fragment) runs against the device's current state — handy while authoring one. + +Subcommands: + run Run a flow and report pass/fail (exit code reflects result) + list List flows in .argent/flows + +Options (run): + --device Device id to run against (auto-detected when omitted) + --platform

ios | android | chromium | vega — narrow auto-detection + --update-baselines Write/refresh screenshot baselines instead of diffing + --output

Also write failed snapshot images (baseline/current/diff) + under // — a stable path for CI artifact upload + --json Print the raw JSON report + --help, -h Show this help + +Examples: + argent flow run checkout --platform ios + argent flow run checkout --device --update-baselines + argent flow run checkout --output flow-artifacts --json +`); +} + +export function parseRunArgs(argv: string[]): { + name?: string; + device?: string; + platform?: string; + output?: string; + updateBaselines: boolean; + json: boolean; +} { + const out = { updateBaselines: false, json: false } as ReturnType; + for (let i = 0; i < argv.length; i++) { + const tok = argv[i]!; + if (!tok.startsWith("-")) { + // The first bare token is the flow name; later ones stay ignored. + if (!out.name) out.name = tok; + continue; + } + // Accept `--flag=value` alongside `--flag value`, like the `argent run` / + // `argent tools` parser (flag-parser.ts) does. + const eq = tok.startsWith("--") ? tok.indexOf("=") : -1; + const flag = eq === -1 ? tok : tok.slice(0, eq); + const inline = eq === -1 ? undefined : tok.slice(eq + 1); + // A value-taking flag must consume a real value. A missing one (`--flag=` + // with nothing after the `=`, the flag as the final token, or a next token + // that is itself a flag) would otherwise be dropped silently and the run + // would fall back to device auto-detection — running against whatever + // happens to be booted instead of erroring. + const takeValue = (name: string): string => { + if (inline !== undefined) { + if (inline === "") throw new FlagParseException(`${name} requires a value`); + return inline; + } + const v = argv[i + 1]; + if (v === undefined || v.startsWith("-")) { + throw new FlagParseException(`${name} requires a value`); + } + i += 1; + return v; + }; + const noValue = (name: string): void => { + if (inline !== undefined) throw new FlagParseException(`${name} does not take a value`); + }; + if (flag === "--update-baselines") { + noValue("--update-baselines"); + out.updateBaselines = true; + } else if (flag === "--json") { + noValue("--json"); + out.json = true; + } else if (flag === "--device") out.device = takeValue("--device"); + else if (flag === "--platform") out.platform = takeValue("--platform"); + else if (flag === "--output") out.output = takeValue("--output"); + // Any other flag-shaped token is an error — a typo like --platfrom must + // not silently fall back to device auto-detection. --help/-h never reach + // this parser: flow() intercepts them before calling parseRunArgs. + else throw new FlagParseException(`unknown flag ${tok}`); + } + return out; +} + +export function renderStepLine(s: StepReport, n: number, topFlow: string): string { + const where = s.flow && s.flow !== topFlow ? ` [${s.flow}]` : ""; + const what = s.tool ?? s.target; + const label = what ? `${s.kind} ${what}` : s.kind; + const reason = s.reason ? ` — ${s.reason}` : ""; + const glyph = s.status === "pass" && s.warning ? "⚠" : STATUS_GLYPH[s.status]; + return ` ${glyph} ${String(n).padStart(2)} ${label}${where}${reason}`; +} + +export function renderSummary(report: FlowReport, opts: { withDevice?: boolean } = {}): string { + const warnings = report.steps.filter((s) => s.warning).length; + const warningsNote = warnings ? `, ${warnings} warning${warnings === 1 ? "" : "s"}` : ""; + // The live renderer prints its header before the runner has resolved a + // device, so its summary carries the device instead. + const where = opts.withDevice ? ` on ${report.device}` : ""; + return `${report.ok ? "PASS" : "FAIL"}${where} — ${report.passed} passed, ${report.failed} failed, ${report.errored} errored, ${report.skipped} skipped${warningsNote}`; +} + +/** + * Artifact paths for the live renderer, which prints step lines before any + * path exists (paths are materialized only from the final report). Labeled by + * step number since they no longer sit under their step line. + */ +export function renderArtifactLines(report: FlowReport): string[] { + const lines: string[] = []; + let n = 0; + for (const s of report.steps) { + if (s.kind === "echo") continue; + n++; + if (!s.artifacts || typeof s.artifacts !== "object") continue; + const entries = Object.entries(s.artifacts).filter(([, v]) => typeof v === "string"); + if (entries.length === 0) continue; + lines.push(` ${s.kind} (step ${n}):`); + for (const [k, v] of entries) lines.push(` ${k}: ${v}`); + } + return lines; +} + +/** + * Names spliced into artifact-export destinations. Mirrors the tool-server's + * FLOW_NAME_PATTERN, which every legitimate `report.flow` and `snapshotKey` + * already satisfies (`assertSafeFlowName`'d flow name; `__-WxH` + * key). Re-checked here because the destination root is an operator-chosen + * filesystem path (`--output`) and the values arrive over the wire — a + * malicious or buggy server must not steer the copy outside that directory. + */ +const SAFE_ARTIFACT_NAME = /^[A-Za-z0-9_-]+$/; + +/** + * Copy each failed snapshot's artifacts into a durable, globbable location — + * `//-.png`, where `` is the snapshot's + * baseline key (`name__platform-WxH`), so a run that hits several + * flows/snapshots can't clobber itself. This is the only place the CLI needs + * artifact bytes, so materialization happens here, scoped to each failed + * snapshot's artifacts — a co-located tool-server resolves them in place, a + * remote one downloads just these files. Rewrites each copied role's path in + * the report so the renderers and `--json` print the durable location instead + * of a temp path. Failure-only: a clean pass carries no artifacts, and a + * seeded baseline is already durable under `__baselines__/`. Best-effort per + * file — a copy error warns on stderr and leaves the source path in place; + * artifact export must never change a run's verdict. Server-supplied names + * that fail `SAFE_ARTIFACT_NAME` are skipped the same way — before any + * materialization, so nothing is downloaded for a step that won't be written. + */ +export async function exportFailureArtifacts( + report: FlowReport, + outputDir: string, + ctx: MaterializeContext +): Promise { + if (!SAFE_ARTIFACT_NAME.test(report.flow)) { + console.error( + `warning: skipping artifact export for unsafe flow name ${JSON.stringify(report.flow)}` + ); + return; + } + for (const s of report.steps) { + if (s.kind !== "snapshot" || s.status !== "fail" || !s.artifacts) continue; + // Key first: a legacy tool-server sends plain path strings, and + // keyFromBaselinePath needs that original baseline path, not a rewrite. + // The pattern check also hardens the fallback, whose basename can still + // be ".." for a path ending in "/..". + const key = s.snapshotKey ?? keyFromBaselinePath(s.artifacts); + if (!key || !SAFE_ARTIFACT_NAME.test(key)) continue; + // Materialize only this snapshot's artifacts (local read or remote + // download) — never the whole report. + const { result } = await materializeArtifacts(s.artifacts, ctx); + s.artifacts = result as Record; + const dir = path.join(outputDir, report.flow); + for (const [role, value] of Object.entries(s.artifacts)) { + if (typeof value !== "string") continue; // null = failed materialization + const dest = path.join(dir, `${key}-${role}.png`); + // Same resolved-path check as the server's getFlowPath: even if the + // pattern above is ever weakened, the copy stays inside --output. Also + // covers `role`, the third server-supplied piece of the destination. + const rel = path.relative(outputDir, dest); + if (rel.startsWith("..") || path.isAbsolute(rel)) continue; + try { + await fsp.mkdir(dir, { recursive: true }); + await fsp.copyFile(value, dest); + s.artifacts[role] = dest; + } catch (err) { + console.error( + `warning: could not write ${dest}: ${err instanceof Error ? err.message : String(err)}` + ); + } + } + } +} + +/** + * Fallback for a pre-`snapshotKey` tool-server: the baseline artifact is the + * baseline file itself (or a download named after it), so its basename IS the + * key. + */ +function keyFromBaselinePath(artifacts: Record): string | null { + const baseline = artifacts.baseline; + if (typeof baseline !== "string") return null; + return path.basename(baseline).replace(/\.png$/, ""); +} + +/** + * Rewrite any artifact handle left in the report to a printable string — the + * tool-server's hostPath, or the bare filename — with zero fetches. The CLI + * renders artifact paths as text only (never inline images), so downloading + * the bytes just to print a path would be pure waste against a remote + * tool-server — the same economy the MCP renderer applies to baseline/current. + * The renderers and `--json` expect string values; a raw handle object would + * fail their `typeof v === "string"` filter and vanish from the output. Runs + * after the optional `--output` export, which has already replaced the failed + * snapshots' handles with durable local copies. + */ +export function resolveArtifactDisplayPaths(report: FlowReport): void { + for (const s of report.steps) { + if (!s.artifacts || typeof s.artifacts !== "object") continue; + for (const [role, value] of Object.entries(s.artifacts)) { + if (isArtifactHandle(value)) s.artifacts[role] = value.hostPath ?? value.filename; + } + } +} + +/** + * Flush stdout/stderr, then exit with `code`. + * + * `console.log` is synchronous only when stdout is a file or a TTY. On a pipe + * — every CI capture (`argent flow run … --json | jq`, `$(…)`, `| tee`) — + * writes are asynchronous, and a bare `process.exit()` right after printing a + * large report tears the process down with everything beyond the OS pipe + * buffer (~64KB) still queued inside Node, truncating a big `--json` report + * mid-string. Stream writes complete in FIFO order, so waiting on a + * zero-length sentinel write guarantees every previously queued chunk has + * reached the fd first. + * + * This cannot hang: it waits only on the std streams' own write queues (a + * stalled pipe reader would block `console.log` the same way), never on other + * open handles (tool-server sockets, timers) — and a destroyed/EPIPE'd stream + * still invokes its write callback, so the exit always fires. + */ +export function exitAfterFlush( + code: number, + streams: NodeJS.WritableStream[] = [process.stdout, process.stderr] +): Promise { + return Promise.all( + streams.map((s) => new Promise((resolve) => s.write("", () => resolve()))) + ).then(() => process.exit(code)); +} + +export function renderReport(report: FlowReport): string { + const lines: string[] = []; + lines.push(`Flow "${report.flow}" on ${report.device}`); + // A fragment runs against the device's current state — remind the operator + // what it assumes was already set up. + if (report.executionPrerequisite) { + lines.push(` assumes: ${report.executionPrerequisite}`); + } + // Number only real steps so echo narration doesn't leave gaps in the sequence. + let n = 0; + for (const s of report.steps) { + // Echo is narration, not a pass/fail step — render its message as a plain + // line with no index or status glyph so it reads as a header between steps. + if (s.kind === "echo") { + if (s.message) lines.push(` › ${s.message}`); + continue; + } + n++; + lines.push(renderStepLine(s, n, report.flow)); + if (s.warning) lines.push(` ⚠ ${s.warning}`); + if (s.artifacts && typeof s.artifacts === "object") { + for (const [k, v] of Object.entries(s.artifacts)) { + if (typeof v === "string") lines.push(` ${k}: ${v}`); + } + } + } + lines.push(`\n${renderSummary(report)}`); + return lines.join("\n"); +} + +export async function flow(argv: string[], options: FlowCommandOptions): Promise { + const [sub, ...rest] = argv; + + if (!sub || sub === "--help" || sub === "-h") { + printHelp(); + return; + } + + const { callTool, baseUrl } = createToolsClient({ paths: options.paths }); + + if (sub === "list") { + const dir = path.join(process.cwd(), ".argent", "flows"); + try { + const entries = await fsp.readdir(dir); + const names = entries.filter((f) => f.endsWith(".yaml")).map((f) => f.replace(/\.yaml$/, "")); + if (names.length === 0) console.log("No flows found in .argent/flows"); + else console.log(names.join("\n")); + } catch { + console.log("No .argent/flows directory in the current working directory."); + } + return; + } + + if (sub !== "run") { + console.error(`Unknown flow subcommand "${sub}". Run \`argent flow --help\`.`); + return exitAfterFlush(2); + } + + // Checked before parseRunArgs so --help wins even when it trails a + // value-taking flag (`--device --help` would otherwise throw "requires a + // value" instead of printing help). + if (rest.includes("--help") || rest.includes("-h")) { + printHelp(); + return; + } + + let args: ReturnType; + try { + args = parseRunArgs(rest); + } catch (err) { + if (err instanceof FlagParseException) { + console.error(`Error: ${err.message}\n`); + printHelp(); + return exitAfterFlush(2); + } + throw err; + } + if (!args.name) { + console.error("argent flow run requires a flow name."); + printHelp(); + return exitAfterFlush(2); + } + const flowName = args.name; + + const payload: Record = { + name: flowName, + project_root: process.cwd(), + // Headless runs never block on the LLM prerequisite handshake. + prerequisiteAcknowledged: true, + }; + if (args.device) payload.device = args.device; + if (args.platform) payload.platform = args.platform; + if (args.updateBaselines) payload.updateBaselines = true; + + // Live rendering: with a streaming server each step line prints the moment + // the step completes. A pre-streaming server ignores the request and no + // events fire, so `liveSteps` doubles as the mode detector — zero means the + // buffered renderer below owns the whole report. + let liveSteps = 0; + let liveIndex = 0; + const onStepReport = (event: unknown): void => { + const s = event as StepReport; + if (liveSteps === 0) console.log(`Flow "${flowName}"`); + liveSteps++; + if (s.kind === "echo") { + if (s.message) console.log(` › ${s.message}`); + return; + } + liveIndex++; + console.log(renderStepLine(s, liveIndex, flowName)); + if (s.warning) console.log(` ⚠ ${s.warning}`); + }; + + let report: FlowReport; + try { + const resp = await callTool( + "flow-execute", + payload, + args.json ? undefined : { onProgress: onStepReport } + ); + // Deliberately NOT materialized here: the CLI prints artifact paths as + // text and renders no images (StepReport has no `result` field, so + // tool-step results are never displayed). Deep-walking the report would + // download every tool-step screenshot and all three PNGs of each failed + // snapshot just to show a path. Only the failed-snapshot artifacts that + // --output copies are fetched, below. + report = resp.data as FlowReport; + } catch (err) { + console.error(err instanceof Error ? err.message : String(err)); + return exitAfterFlush(1); + } + + if (!report || !("steps" in report)) { + console.error(`"${flowName}" did not produce a run report.`); + return exitAfterFlush(2); + } + + // Durable diff output: copy failed-snapshot images out of the tool-server's + // cache before any renderer prints paths, so every output mode shows the + // durable location. The only artifact bytes the CLI ever fetches; baseUrl is + // resolved lazily so a run without --output makes no extra round-trip. + if (args.output) { + const { url, token } = await baseUrl(); + await exportFailureArtifacts(report, path.resolve(args.output), { + toolsUrl: url, + authToken: token, + }); + } + // Whatever handles remain (all of them without --output; passing snapshots + // and unexported roles with it) print as server-side paths. + resolveArtifactDisplayPaths(report); + + if (args.json) { + console.log(JSON.stringify(report, null, 2)); + } else if (liveSteps > 0) { + // Steps already printed live — emit only what the final report knows: + // the prerequisite note, materialized artifact paths, and the summary. + if (report.executionPrerequisite) console.log(` assumes: ${report.executionPrerequisite}`); + for (const line of renderArtifactLines(report)) console.log(line); + console.log(`\n${renderSummary(report, { withDevice: true })}`); + } else { + console.log(renderReport(report)); + } + + return exitAfterFlush(report.ok ? 0 : 1); +} diff --git a/packages/argent-cli/src/index.ts b/packages/argent-cli/src/index.ts index 16725e7ab..604481d51 100644 --- a/packages/argent-cli/src/index.ts +++ b/packages/argent-cli/src/index.ts @@ -1,4 +1,5 @@ export { run, type RunCommandOptions } from "./run.js"; +export { flow, type FlowCommandOptions } from "./flow.js"; export { tools, type ToolsCommandOptions } from "./tools.js"; export { server } from "./server.js"; export { lens, type LensCommandOptions } from "./lens.js"; diff --git a/packages/argent-cli/test/flow-output.test.ts b/packages/argent-cli/test/flow-output.test.ts new file mode 100644 index 000000000..1288a6314 --- /dev/null +++ b/packages/argent-cli/test/flow-output.test.ts @@ -0,0 +1,320 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import type { MaterializeContext } from "@argent/tools-client"; +import { exportFailureArtifacts, type FlowReport, type StepReport } from "../src/flow.js"; + +let tmpDir: string; +let outDir: string; + +// Legacy string-path artifacts contain no handles, so materialization walks +// them without touching the network — the URL never resolves. +const ctx: MaterializeContext = { toolsUrl: "http://tools.invalid" }; + +async function writeFile(name: string, content: string): Promise { + const p = path.join(tmpDir, name); + await fs.writeFile(p, content); + return p; +} + +/** + * A wire artifact handle whose hostPath is a real local file, sized/stamped so + * the materializer's co-location gate resolves it in place (no download). + */ +async function writeHandle(name: string, content: string): Promise> { + const p = await writeFile(name, content); + const st = await fs.stat(p); + return { + __argentArtifact: true, + id: `id-${name}`, + filename: name, + mimeType: "image/png", + size: st.size, + mtimeMs: st.mtimeMs, + hostPath: p, + }; +} + +function mkReport(steps: StepReport[]): FlowReport { + return { + flow: "checkout", + device: "UDID-1", + ok: false, + passed: 0, + failed: 1, + skipped: 0, + errored: 0, + steps, + }; +} + +beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "flow-output-")); + outDir = path.join(tmpDir, "out"); +}); +afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); +}); + +describe("exportFailureArtifacts", () => { + it("copies every role of a failed snapshot to //-.png and rewrites the report", async () => { + const baseline = await writeFile("b.png", "baseline-bytes"); + const current = await writeFile("c.png", "current-bytes"); + const diff = await writeFile("d.png", "diff-bytes"); + const step: StepReport = { + index: 0, + kind: "snapshot", + status: "fail", + snapshotKey: "home__ios-390x844", + artifacts: { baseline, current, diff }, + }; + + await exportFailureArtifacts(mkReport([step]), outDir, ctx); + + const dir = path.join(outDir, "checkout"); + for (const [role, content] of [ + ["baseline", "baseline-bytes"], + ["current", "current-bytes"], + ["diff", "diff-bytes"], + ] as const) { + const dest = path.join(dir, `home__ios-390x844-${role}.png`); + expect(step.artifacts?.[role]).toBe(dest); + expect(await fs.readFile(dest, "utf8")).toBe(content); + } + }); + + it("leaves passed and baseline-seeded snapshots alone (failure-only)", async () => { + const baseline = await writeFile("b.png", "baseline-bytes"); + const seeded: StepReport = { + index: 0, + kind: "snapshot", + status: "pass", + warning: "baseline created", + snapshotKey: "home__ios-390x844", + artifacts: { baseline }, + }; + + await exportFailureArtifacts(mkReport([seeded]), outDir, ctx); + + expect(seeded.artifacts?.baseline).toBe(baseline); + await expect(fs.access(outDir)).rejects.toThrow(); + }); + + it("derives the key from the baseline path when the server sent no snapshotKey", async () => { + const baseline = await writeFile("home__android-1080x2400.png", "baseline-bytes"); + const step: StepReport = { + index: 0, + kind: "snapshot", + status: "fail", + artifacts: { baseline }, + }; + + await exportFailureArtifacts(mkReport([step]), outDir, ctx); + + expect(step.artifacts?.baseline).toBe( + path.join(outDir, "checkout", "home__android-1080x2400-baseline.png") + ); + }); + + it("skips unmaterialized (null) roles and steps with no usable key", async () => { + const current = await writeFile("c.png", "current-bytes"); + const withNull: StepReport = { + index: 0, + kind: "snapshot", + status: "fail", + snapshotKey: "home__ios-390x844", + artifacts: { baseline: null, current }, + }; + const keyless: StepReport = { + index: 1, + kind: "snapshot", + status: "fail", + artifacts: { current }, + }; + + await exportFailureArtifacts(mkReport([withNull, keyless]), outDir, ctx); + + expect(withNull.artifacts?.baseline).toBeNull(); + expect(withNull.artifacts?.current).toBe( + path.join(outDir, "checkout", "home__ios-390x844-current.png") + ); + // No snapshotKey and no baseline to derive one from — nothing written. + expect(keyless.artifacts?.current).toBe(current); + }); + + it("materializes a co-located snapshot's handles in place and copies them without fetching", async () => { + const step: StepReport = { + index: 0, + kind: "snapshot", + status: "fail", + snapshotKey: "home__ios-390x844", + artifacts: { + baseline: await writeHandle("b.png", "baseline-bytes"), + current: await writeHandle("c.png", "current-bytes"), + diff: await writeHandle("d.png", "diff-bytes"), + }, + }; + const fetchSpy = vi.fn(async () => { + throw new Error("unexpected network fetch"); + }); + + await exportFailureArtifacts(mkReport([step]), outDir, { + toolsUrl: "http://tools.invalid", + fetchImpl: fetchSpy as unknown as typeof fetch, + }); + + // The handles' hostPaths are on this machine — resolved in place, no wire. + expect(fetchSpy).not.toHaveBeenCalled(); + for (const [role, content] of [ + ["baseline", "baseline-bytes"], + ["current", "current-bytes"], + ["diff", "diff-bytes"], + ] as const) { + const dest = path.join(outDir, "checkout", `home__ios-390x844-${role}.png`); + expect(step.artifacts?.[role]).toBe(dest); + expect(await fs.readFile(dest, "utf8")).toBe(content); + } + }); + + it("downloads a remote handle (no hostPath) and copies it under --output", async () => { + const prevCache = process.env.ARGENT_ARTIFACTS_DIR; + process.env.ARGENT_ARTIFACTS_DIR = path.join(tmpDir, "cache"); + try { + const step: StepReport = { + index: 0, + kind: "snapshot", + status: "fail", + snapshotKey: "home__ios-390x844", + artifacts: { + diff: { + __argentArtifact: true, + id: "diff-1", + filename: "remote-diff.png", + mimeType: "image/png", + size: 10, + }, + }, + }; + const fetchSpy = vi.fn(async () => new Response("diff-bytes")); + + await exportFailureArtifacts(mkReport([step]), outDir, { + toolsUrl: "http://tools.invalid", + authToken: "tok", + fetchImpl: fetchSpy as unknown as typeof fetch, + }); + + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(fetchSpy).toHaveBeenCalledWith("http://tools.invalid/artifacts/diff-1", { + headers: { Authorization: "Bearer tok" }, + }); + const dest = path.join(outDir, "checkout", "home__ios-390x844-diff.png"); + expect(step.artifacts?.diff).toBe(dest); + expect(await fs.readFile(dest, "utf8")).toBe("diff-bytes"); + } finally { + if (prevCache === undefined) delete process.env.ARGENT_ARTIFACTS_DIR; + else process.env.ARGENT_ARTIFACTS_DIR = prevCache; + } + }); + + it("refuses a flow name with path traversal: warns, writes nothing, downloads nothing", async () => { + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + // A remote handle (no hostPath) would force a download — proving the + // guard fires before materialization, not just before the copy. + const handle = { + __argentArtifact: true, + id: "diff-1", + filename: "remote-diff.png", + mimeType: "image/png", + size: 10, + }; + const step: StepReport = { + index: 0, + kind: "snapshot", + status: "fail", + snapshotKey: "home__ios-390x844", + artifacts: { diff: handle }, + }; + const fetchSpy = vi.fn(async () => new Response("diff-bytes")); + + await exportFailureArtifacts({ ...mkReport([step]), flow: "../escape" }, outDir, { + toolsUrl: "http://tools.invalid", + fetchImpl: fetchSpy as unknown as typeof fetch, + }); + + expect(fetchSpy).not.toHaveBeenCalled(); + expect(step.artifacts?.diff).toBe(handle); + expect(errSpy).toHaveBeenCalledWith(expect.stringContaining("unsafe flow name")); + // The would-be destination outside --output must not exist. + await expect(fs.access(path.join(tmpDir, "escape"))).rejects.toThrow(); + await expect(fs.access(outDir)).rejects.toThrow(); + } finally { + errSpy.mockRestore(); + } + }); + + it("skips a step whose snapshotKey contains path traversal, still exporting safe steps", async () => { + const evil = await writeFile("evil.png", "evil-bytes"); + const good = await writeFile("good.png", "good-bytes"); + const badStep: StepReport = { + index: 0, + kind: "snapshot", + status: "fail", + snapshotKey: "../../pwned", + artifacts: { current: evil }, + }; + const goodStep: StepReport = { + index: 1, + kind: "snapshot", + status: "fail", + snapshotKey: "home__ios-390x844", + artifacts: { current: good }, + }; + + await exportFailureArtifacts(mkReport([badStep, goodStep]), outDir, ctx); + + // Untouched — the join would have resolved to /pwned-current.png. + expect(badStep.artifacts?.current).toBe(evil); + await expect(fs.access(path.join(tmpDir, "pwned-current.png"))).rejects.toThrow(); + expect(goodStep.artifacts?.current).toBe( + path.join(outDir, "checkout", "home__ios-390x844-current.png") + ); + }); + + it("skips a step whose baseline-derived key reduces to '..'", async () => { + // path.basename("/..") is ".." — the fallback alone can't contain it. + const step: StepReport = { + index: 0, + kind: "snapshot", + status: "fail", + artifacts: { baseline: `${tmpDir}${path.sep}..` }, + }; + + await exportFailureArtifacts(mkReport([step]), outDir, ctx); + + expect(step.artifacts?.baseline).toBe(`${tmpDir}${path.sep}..`); + await expect(fs.access(outDir)).rejects.toThrow(); + }); + + it("warns and keeps the temp path when a source file is unreadable", async () => { + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + const gone = path.join(tmpDir, "vanished.png"); + const step: StepReport = { + index: 0, + kind: "snapshot", + status: "fail", + snapshotKey: "home__ios-390x844", + artifacts: { diff: gone }, + }; + + await exportFailureArtifacts(mkReport([step]), outDir, ctx); + + expect(step.artifacts?.diff).toBe(gone); + expect(errSpy).toHaveBeenCalledWith(expect.stringContaining("warning: could not write")); + } finally { + errSpy.mockRestore(); + } + }); +}); diff --git a/packages/argent-cli/test/flow-render.test.ts b/packages/argent-cli/test/flow-render.test.ts new file mode 100644 index 000000000..31034d61f --- /dev/null +++ b/packages/argent-cli/test/flow-render.test.ts @@ -0,0 +1,113 @@ +import { describe, it, expect } from "vitest"; +import { + renderReport, + renderStepLine, + renderSummary, + renderArtifactLines, + type FlowReport, + type StepReport, +} from "../src/flow.js"; + +function mkReport(steps: StepReport[], overrides: Partial = {}): FlowReport { + // Mirror the runner's summarize(): echo narration is not a counted step. + const counted = steps.filter((s) => s.kind !== "echo"); + const passed = counted.filter((s) => s.status === "pass").length; + const failed = counted.filter((s) => s.status === "fail").length; + const skipped = counted.filter((s) => s.status === "skip").length; + const errored = counted.filter((s) => s.status === "error").length; + return { + flow: "checkout", + device: "UDID-1", + ok: failed === 0 && errored === 0, + passed, + failed, + skipped, + errored, + steps, + ...overrides, + }; +} + +const STEPS: StepReport[] = [ + { index: 0, kind: "echo", status: "pass", message: "starting" }, + { index: 1, kind: "launch", status: "pass" }, + { index: 2, kind: "tap", status: "pass", flow: "login", target: '"Login"' }, + { + index: 3, + kind: "snapshot", + status: "fail", + reason: "diff 2.10% > 1%", + target: '"home"', + artifacts: { baseline: "/tmp/b.png", diff: "/tmp/d.png" }, + }, + { index: 4, kind: "await", status: "skip", target: 'visible "Done"' }, +]; + +describe("flow report rendering", () => { + it("buffered renderReport keeps its historical shape", () => { + const out = renderReport(mkReport(STEPS)); + expect(out).toBe( + [ + 'Flow "checkout" on UDID-1', + " › starting", + " ✓ 1 launch", + ' ✓ 2 tap "Login" [login]', + ' ✗ 3 snapshot "home" — diff 2.10% > 1%', + " baseline: /tmp/b.png", + " diff: /tmp/d.png", + ' · 4 await visible "Done"', + "", + "FAIL — 2 passed, 1 failed, 0 errored, 1 skipped", + ].join("\n") + ); + }); + + it("live step lines match the buffered renderer's step lines", () => { + const report = mkReport(STEPS); + const buffered = renderReport(report).split("\n"); + + // Reproduce the live loop: number only non-echo steps, same top flow. + const live: string[] = []; + let n = 0; + for (const s of report.steps) { + if (s.kind === "echo") { + if (s.message) live.push(` › ${s.message}`); + continue; + } + n++; + live.push(renderStepLine(s, n, report.flow)); + } + + // Every live line appears verbatim in the buffered output (which adds the + // header, inline artifact paths, and summary around them). + for (const line of live) expect(buffered).toContain(line); + }); + + it("pass with a warning renders the warning glyph", () => { + const step: StepReport = { + index: 0, + kind: "snapshot", + status: "pass", + warning: "baseline seeded", + }; + expect(renderStepLine(step, 1, "checkout")).toBe(" ⚠ 1 snapshot"); + }); + + it("renderSummary carries the device only when asked (live tail)", () => { + const report = mkReport(STEPS); + expect(renderSummary(report)).toBe("FAIL — 2 passed, 1 failed, 0 errored, 1 skipped"); + expect(renderSummary(report, { withDevice: true })).toBe( + "FAIL on UDID-1 — 2 passed, 1 failed, 0 errored, 1 skipped" + ); + }); + + it("renderArtifactLines labels paths by step number, skipping echo steps", () => { + const lines = renderArtifactLines(mkReport(STEPS)); + // The snapshot is the 3rd numbered step (echo carries no number). + expect(lines).toEqual([ + " snapshot (step 3):", + " baseline: /tmp/b.png", + " diff: /tmp/d.png", + ]); + }); +}); diff --git a/packages/argent-cli/test/flow.test.ts b/packages/argent-cli/test/flow.test.ts new file mode 100644 index 000000000..9f2a23279 --- /dev/null +++ b/packages/argent-cli/test/flow.test.ts @@ -0,0 +1,554 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { Writable } from "node:stream"; +import { exitAfterFlush, flow, parseRunArgs } from "../src/flow.js"; +import { FlagParseException } from "../src/flag-parser.js"; + +const toolsClientMock = vi.hoisted(() => ({ + callTool: vi.fn(), + baseUrl: vi.fn(async () => ({ url: "http://127.0.0.1:4141", token: "tok" })), +})); +// Identity materialization; a spy so tests can assert it is only invoked for +// the failed-snapshot artifacts that --output actually copies. +const materializeArtifactsMock = vi.hoisted(() => + vi.fn(async (data: unknown) => ({ result: data, images: [] })) +); + +vi.mock("@argent/tools-client", async (importOriginal) => ({ + // Keep the real isArtifactHandle — the display-path fallback under test + // must recognize genuine wire handles. + ...(await importOriginal()), + createToolsClient: vi.fn(() => toolsClientMock), + materializeArtifacts: materializeArtifactsMock, +})); + +interface StepFixture { + index: number; + kind: string; + status: "pass" | "fail" | "skip" | "error"; + reason?: string; + warning?: string; + tool?: string; + flow?: string; + message?: string; + snapshotKey?: string; + artifacts?: Record; + /** Wire-only tool-step payload; the CLI StepReport type has no such field. */ + result?: unknown; +} + +/** A wire artifact handle as the tool-server emits it (image/png). */ +function handle(hostPath?: string): Record { + return { + __argentArtifact: true, + id: "art-1", + filename: "art.png", + mimeType: "image/png", + size: 4, + ...(hostPath ? { hostPath } : {}), + }; +} + +function report(overrides: Record = {}): Record { + const steps: StepFixture[] = [{ index: 0, kind: "tap", status: "pass" }]; + return { + flow: "checkout", + device: "SIM-1", + executionPrerequisite: "", + ok: true, + passed: 1, + failed: 0, + skipped: 0, + errored: 0, + steps, + ...overrides, + }; +} + +describe("parseRunArgs", () => { + it("returns documented defaults with just a name", () => { + expect(parseRunArgs(["checkout"])).toEqual({ + name: "checkout", + updateBaselines: false, + json: false, + }); + }); + + it("parses every run flag alongside the name", () => { + expect( + parseRunArgs(["checkout", "--device", "SIM-1", "--platform", "ios", "--update-baselines"]) + ).toEqual({ + name: "checkout", + device: "SIM-1", + platform: "ios", + updateBaselines: true, + json: false, + }); + expect(parseRunArgs(["--json", "checkout"]).json).toBe(true); + }); + + it("throws when --device is the final token", () => { + expect(() => parseRunArgs(["checkout", "--device"])).toThrow(FlagParseException); + expect(() => parseRunArgs(["checkout", "--device"])).toThrow("--device requires a value"); + }); + + it("throws when --platform is the final token", () => { + expect(() => parseRunArgs(["checkout", "--platform"])).toThrow("--platform requires a value"); + }); + + it("treats a following flag as a missing value, not as the value", () => { + expect(() => parseRunArgs(["checkout", "--device", "--json"])).toThrow( + "--device requires a value" + ); + expect(() => parseRunArgs(["checkout", "--platform", "--update-baselines"])).toThrow( + "--platform requires a value" + ); + }); + + it("accepts the --flag=value form for every value-taking flag", () => { + expect(parseRunArgs(["checkout", "--device=SIM-1", "--platform=ios", "--output=dir"])).toEqual({ + name: "checkout", + device: "SIM-1", + platform: "ios", + output: "dir", + updateBaselines: false, + json: false, + }); + }); + + it("mixes = and space-separated forms freely", () => { + expect(parseRunArgs(["checkout", "--device=SIM-1", "--platform", "ios"])).toEqual({ + name: "checkout", + device: "SIM-1", + platform: "ios", + updateBaselines: false, + json: false, + }); + }); + + it("does not consume the next token when the value was inline", () => { + // Guards the index bookkeeping: --device=SIM-1 must not swallow --json. + const out = parseRunArgs(["checkout", "--device=SIM-1", "--json"]); + expect(out.device).toBe("SIM-1"); + expect(out.json).toBe(true); + }); + + it("throws when a boolean flag is given an inline value", () => { + expect(() => parseRunArgs(["checkout", "--json=true"])).toThrow(FlagParseException); + expect(() => parseRunArgs(["checkout", "--json=true"])).toThrow("--json does not take a value"); + expect(() => parseRunArgs(["checkout", "--update-baselines=1"])).toThrow( + "--update-baselines does not take a value" + ); + }); + + it("throws when an inline value is empty", () => { + expect(() => parseRunArgs(["checkout", "--device="])).toThrow("--device requires a value"); + }); + + it("rejects unknown flags instead of silently dropping them", () => { + expect(() => parseRunArgs(["checkout", "--verbose"])).toThrow(FlagParseException); + expect(() => parseRunArgs(["checkout", "--verbose"])).toThrow(/unknown flag/); + // A typo'd value flag must not fall back to device auto-detection. + expect(() => parseRunArgs(["checkout", "--platfrom=ios"])).toThrow(/unknown flag/); + }); + + it("still ignores extra bare positionals (only flags are rejected)", () => { + expect(parseRunArgs(["checkout", "extra"])).toEqual({ + name: "checkout", + updateBaselines: false, + json: false, + }); + }); +}); + +describe("argent flow run", () => { + let exitSpy: ReturnType; + let logs: string[]; + let errs: string[]; + let logSpy: ReturnType; + let errSpy: ReturnType; + + const opts = { paths: {} as never }; + + beforeEach(() => { + vi.clearAllMocks(); + toolsClientMock.callTool.mockResolvedValue({ data: report() }); + logs = []; + errs = []; + logSpy = vi.spyOn(console, "log").mockImplementation((...a) => void logs.push(a.join(" "))); + errSpy = vi.spyOn(console, "error").mockImplementation((...a) => void errs.push(a.join(" "))); + exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`process.exit:${code}`); + }) as typeof process.exit); + }); + + afterEach(() => { + exitSpy.mockRestore(); + logSpy.mockRestore(); + errSpy.mockRestore(); + }); + + it("forwards name, device, platform, and updateBaselines to flow-execute and exits 0 on pass", async () => { + await expect( + flow( + ["run", "checkout", "--device", "SIM-1", "--platform", "ios", "--update-baselines"], + opts + ) + ).rejects.toThrow("process.exit:0"); + + expect(toolsClientMock.callTool).toHaveBeenCalledWith( + "flow-execute", + { + name: "checkout", + project_root: process.cwd(), + prerequisiteAcknowledged: true, + device: "SIM-1", + platform: "ios", + updateBaselines: true, + }, + { onProgress: expect.any(Function) } + ); + expect(logs.join("\n")).toContain("PASS — 1 passed, 0 failed, 0 errored, 0 skipped"); + }); + + it("exits 2 without calling the tool when --device is missing its value", async () => { + await expect(flow(["run", "checkout", "--device"], opts)).rejects.toThrow("process.exit:2"); + + expect(toolsClientMock.callTool).not.toHaveBeenCalled(); + expect(errs.join("\n")).toContain("--device requires a value"); + }); + + it("exits 2 without calling the tool when --platform is followed by another flag", async () => { + await expect(flow(["run", "checkout", "--platform", "--json"], opts)).rejects.toThrow( + "process.exit:2" + ); + + expect(toolsClientMock.callTool).not.toHaveBeenCalled(); + expect(errs.join("\n")).toContain("--platform requires a value"); + }); + + it("forwards --flag=value forms to flow-execute like the space-separated ones", async () => { + await expect( + flow(["run", "checkout", "--platform=ios", "--device=SIM-1"], opts) + ).rejects.toThrow("process.exit:0"); + + expect(toolsClientMock.callTool).toHaveBeenCalledWith( + "flow-execute", + { + name: "checkout", + project_root: process.cwd(), + prerequisiteAcknowledged: true, + device: "SIM-1", + platform: "ios", + }, + { onProgress: expect.any(Function) } + ); + }); + + it("exits 2 without calling the tool when a boolean flag is given a value", async () => { + await expect(flow(["run", "checkout", "--json=x"], opts)).rejects.toThrow("process.exit:2"); + + expect(toolsClientMock.callTool).not.toHaveBeenCalled(); + expect(errs.join("\n")).toContain("--json does not take a value"); + }); + + it("exits 2 without calling the tool on a typo'd flag instead of auto-detecting a device", async () => { + await expect(flow(["run", "checkout", "--platfrom=ios"], opts)).rejects.toThrow( + "process.exit:2" + ); + + expect(toolsClientMock.callTool).not.toHaveBeenCalled(); + expect(errs.join("\n")).toContain("unknown flag"); + }); + + it("exits 2 when no flow name is given", async () => { + await expect(flow(["run"], opts)).rejects.toThrow("process.exit:2"); + expect(errs.join("\n")).toContain("requires a flow name"); + expect(toolsClientMock.callTool).not.toHaveBeenCalled(); + }); + + it("renders the report — echo lines unnumbered, real steps numbered, reasons and fragment tags shown — and exits 1 on failure", async () => { + toolsClientMock.callTool.mockResolvedValue({ + data: report({ + executionPrerequisite: "App on the login screen", + ok: false, + passed: 1, + failed: 1, + skipped: 1, + steps: [ + { index: 0, kind: "echo", status: "pass", message: "Opening settings" }, + { index: 1, kind: "tap", status: "pass" }, + { index: 2, kind: "assert", status: "fail", reason: "never visible", flow: "login" }, + { index: 3, kind: "tool", tool: "screenshot", status: "skip" }, + ], + }), + }); + + await expect(flow(["run", "checkout"], opts)).rejects.toThrow("process.exit:1"); + + const out = logs.join("\n"); + expect(out).toContain('Flow "checkout" on SIM-1'); + expect(out).toContain("assumes: App on the login screen"); + // Echo is narration — no index; numbering starts at the first real step. + expect(out).toContain("› Opening settings"); + expect(out).toMatch(/✓ {2}1 tap/); + expect(out).toMatch(/✗ {2}2 assert \[login\] — never visible/); + expect(out).toMatch(/· {2}3 tool screenshot/); + expect(out).toContain("FAIL — 1 passed, 1 failed, 0 errored, 1 skipped"); + }); + + it("renders legacy warnings with the ⚠ glyph and counts them in the summary", async () => { + toolsClientMock.callTool.mockResolvedValue({ + data: report({ + steps: [{ index: 0, kind: "snapshot", status: "pass", warning: "no baseline; adopted" }], + }), + }); + + await expect(flow(["run", "checkout"], opts)).rejects.toThrow("process.exit:0"); + + const out = logs.join("\n"); + expect(out).toMatch(/⚠ {2}1 snapshot/); + expect(out).toContain("⚠ no baseline; adopted"); + expect(out).toContain("1 warning"); + }); + + it("prints the raw report with --json", async () => { + await expect(flow(["run", "checkout", "--json"], opts)).rejects.toThrow("process.exit:0"); + expect(JSON.parse(logs.join("\n"))).toEqual(report()); + }); + + it("renders failed-snapshot handles as server paths without fetching when --output is absent", async () => { + toolsClientMock.callTool.mockResolvedValue({ + data: report({ + ok: false, + passed: 0, + failed: 1, + steps: [ + { + index: 0, + kind: "snapshot", + status: "fail", + reason: "1.2% differs", + snapshotKey: "home__ios-390x844", + artifacts: { + baseline: handle("/srv/base.png"), + current: handle("/srv/cur.png"), + diff: handle("/srv/diff.png"), + }, + }, + ], + }), + }); + + await expect(flow(["run", "checkout"], opts)).rejects.toThrow("process.exit:1"); + + // Nothing to download: paths come straight off the handles, and the + // server URL is never even resolved. + expect(materializeArtifactsMock).not.toHaveBeenCalled(); + expect(toolsClientMock.baseUrl).not.toHaveBeenCalled(); + const out = logs.join("\n"); + expect(out).toContain("baseline: /srv/base.png"); + expect(out).toContain("current: /srv/cur.png"); + expect(out).toContain("diff: /srv/diff.png"); + }); + + it("never materializes tool-step results (the CLI renders no images)", async () => { + toolsClientMock.callTool.mockResolvedValue({ + data: report({ + steps: [ + { + index: 0, + kind: "tool", + tool: "screenshot", + status: "pass", + result: { image: handle("/srv/shot.png") }, + }, + ], + }), + }); + + await expect(flow(["run", "checkout"], opts)).rejects.toThrow("process.exit:0"); + + expect(materializeArtifactsMock).not.toHaveBeenCalled(); + const out = logs.join("\n"); + expect(out).toMatch(/✓ {2}1 tool screenshot/); + expect(out).toContain("PASS — 1 passed"); + }); + + it("materializes only the failed snapshot's artifacts when --output is set", async () => { + const failedArtifacts = { baseline: handle("/srv/base.png") }; + toolsClientMock.callTool.mockResolvedValue({ + data: report({ + ok: false, + failed: 1, + steps: [ + { + index: 0, + kind: "tool", + tool: "screenshot", + status: "pass", + result: { image: handle("/srv/shot.png") }, + }, + { + index: 1, + kind: "snapshot", + status: "fail", + snapshotKey: "home__ios-390x844", + artifacts: failedArtifacts, + }, + ], + }), + }); + + await expect(flow(["run", "checkout", "--output", "flow-artifacts"], opts)).rejects.toThrow( + "process.exit:1" + ); + + // One materialization, scoped to the failed snapshot's artifacts object — + // not the whole report (which would pull the tool-step screenshot too). + expect(toolsClientMock.baseUrl).toHaveBeenCalledTimes(1); + expect(materializeArtifactsMock).toHaveBeenCalledTimes(1); + expect(materializeArtifactsMock).toHaveBeenCalledWith(failedArtifacts, { + toolsUrl: "http://127.0.0.1:4141", + authToken: "tok", + }); + }); + + it("emits string artifact paths in --json without --output (hostPath, or filename)", async () => { + toolsClientMock.callTool.mockResolvedValue({ + data: report({ + ok: false, + passed: 0, + failed: 1, + steps: [ + { + index: 0, + kind: "snapshot", + status: "fail", + snapshotKey: "home__ios-390x844", + artifacts: { baseline: handle("/srv/base.png"), diff: handle() }, + }, + ], + }), + }); + + await expect(flow(["run", "checkout", "--json"], opts)).rejects.toThrow("process.exit:1"); + + expect(materializeArtifactsMock).not.toHaveBeenCalled(); + const parsed = JSON.parse(logs.join("\n")) as { + steps: { artifacts?: Record }[]; + }; + // Strings, not handle objects: hostPath when present, filename otherwise. + expect(parsed.steps[0]?.artifacts).toEqual({ baseline: "/srv/base.png", diff: "art.png" }); + }); + + it("prints legacy string artifact paths as-is (pre-handle tool-server)", async () => { + toolsClientMock.callTool.mockResolvedValue({ + data: report({ + ok: false, + passed: 0, + failed: 1, + steps: [ + { + index: 0, + kind: "snapshot", + status: "fail", + artifacts: { baseline: "/tmp/snaps/home.png", diff: "/tmp/snaps/home-diff.png" }, + }, + ], + }), + }); + + await expect(flow(["run", "checkout"], opts)).rejects.toThrow("process.exit:1"); + + expect(materializeArtifactsMock).not.toHaveBeenCalled(); + const out = logs.join("\n"); + expect(out).toContain("baseline: /tmp/snaps/home.png"); + expect(out).toContain("diff: /tmp/snaps/home-diff.png"); + }); + + it("exits 1 with the error message when the tool call fails", async () => { + toolsClientMock.callTool.mockRejectedValue(new Error("tool-server unreachable")); + + await expect(flow(["run", "checkout"], opts)).rejects.toThrow("process.exit:1"); + expect(errs.join("\n")).toContain("tool-server unreachable"); + }); + + it("exits 2 when the result is not a run report (e.g. a prerequisite notice)", async () => { + toolsClientMock.callTool.mockResolvedValue({ + data: { flow: "checkout", notice: "prerequisite", executionPrerequisite: "logged in" }, + }); + + await expect(flow(["run", "checkout"], opts)).rejects.toThrow("process.exit:2"); + expect(errs.join("\n")).toContain('"checkout" did not produce a run report'); + }); + + it("exits 2 on an unknown subcommand", async () => { + await expect(flow(["frobnicate"], opts)).rejects.toThrow("process.exit:2"); + expect(errs.join("\n")).toContain('Unknown flow subcommand "frobnicate"'); + }); + + it("prints help and returns (no exit) with no subcommand", async () => { + await flow([], opts); + expect(logs.join("\n")).toContain("Usage: argent flow"); + expect(toolsClientMock.callTool).not.toHaveBeenCalled(); + }); + + it("prints help instead of running when --help follows the flow name", async () => { + await flow(["run", "checkout", "--help"], opts); + expect(logs.join("\n")).toContain("Options (run):"); + expect(toolsClientMock.callTool).not.toHaveBeenCalled(); + }); + + it("prints help instead of running when -h trails other run flags", async () => { + await flow(["run", "checkout", "--device", "SIM-1", "-h"], opts); + expect(logs.join("\n")).toContain("Usage: argent flow"); + expect(toolsClientMock.callTool).not.toHaveBeenCalled(); + }); +}); + +describe("exitAfterFlush", () => { + let exitSpy: ReturnType; + + beforeEach(() => { + exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`process.exit:${code}`); + }) as typeof process.exit); + }); + + afterEach(() => { + exitSpy.mockRestore(); + }); + + it("exits only after every queued write has drained (piped stdout is async)", async () => { + // Model a pipe with a slow reader: each chunk sits in the stream's queue + // until _write's deferred callback fires — exactly the state a >64KB + // `--json` report is in when the old bare process.exit() truncated it. + const flushed: string[] = []; + let exitedEarly = false; + const slow = new Writable({ + highWaterMark: 1, + write(chunk: Buffer, _enc, cb) { + setTimeout(() => { + if (exitSpy.mock.calls.length > 0) exitedEarly = true; + flushed.push(chunk.toString()); + cb(); + }, 5); + }, + }); + slow.write("a".repeat(64 * 1024)); + slow.write("b".repeat(64 * 1024)); + + await expect(exitAfterFlush(1, [slow])).rejects.toThrow("process.exit:1"); + + expect(exitedEarly).toBe(false); + expect(flushed.join("")).toContain("a".repeat(64 * 1024)); + expect(flushed.join("")).toContain("b".repeat(64 * 1024)); + expect(exitSpy).toHaveBeenCalledWith(1); + }); + + it("preserves the exit code with nothing queued", async () => { + const idle = new Writable({ write: (_c, _e, cb) => cb() }); + await expect(exitAfterFlush(2, [idle])).rejects.toThrow("process.exit:2"); + expect(exitSpy).toHaveBeenCalledWith(2); + }); +}); diff --git a/packages/argent-mcp/src/content.ts b/packages/argent-mcp/src/content.ts index a07a0134a..7254ee2bd 100644 --- a/packages/argent-mcp/src/content.ts +++ b/packages/argent-mcp/src/content.ts @@ -5,7 +5,11 @@ */ import { readFile } from "node:fs/promises"; -import { materializeArtifacts, type MaterializeContext } from "@argent/tools-client"; +import { + materializeArtifacts, + isArtifactHandle, + type MaterializeContext, +} from "@argent/tools-client"; export type ContentBlock = | { type: "text"; text: string } @@ -81,7 +85,7 @@ export async function toMcpContent( return legacyImageContent(rewritten, suppressImage); } - const blocks: ContentBlock[] = [{ type: "text", text: JSON.stringify(rewritten, null, 2) }]; + const blocks: ContentBlock[] = [{ type: "text", text: stringifyForText(rewritten) }]; // Surface any images that rode along on a non-image result. if (!suppressImage) for (const img of images) blocks.push(imageBlock(img.data, img.mimeType)); return blocks; @@ -91,7 +95,16 @@ export async function toMcpContent( return legacyImageContent(result, suppressImage); } - return [{ type: "text" as const, text: JSON.stringify(result, null, 2) }]; + return [{ type: "text" as const, text: stringifyForText(result) }]; +} + +/** + * JSON.stringify(undefined) returns undefined, which would produce an invalid + * MCP content block ({ type: "text", text: undefined }). Coerce to "null" so a + * result with no value still serializes to a valid text block. + */ +function stringifyForText(value: unknown): string { + return JSON.stringify(value ?? null, null, 2); } /** @@ -192,23 +205,68 @@ export async function screenshotDiffToMcpContent( // ── flow-execute adapter ───────────────────────────────────────────── +export type FlowStepResult = { + index?: number; + kind: string; + status?: "pass" | "fail" | "skip" | "error"; + reason?: string; + /** + * Legacy: older tool-servers passed a snapshot that adopted a missing + * baseline and annotated it with this caveat (a missing baseline now fails + * the step). Rendered for wire compat with a not-yet-updated server. + */ + warning?: string; + tool?: string; + message?: string; + result?: unknown; + outputHint?: string; + args?: unknown; + flow?: string; + /** Human-readable step target (selector / snapshot name), set by the runner. */ + target?: string; + /** + * Snapshot-step artifacts keyed by role (baseline/current/diff). Values are + * artifact handles on current tool-servers; treated as untrusted wire data + * here, so anything else renders as text or is skipped. + */ + artifacts?: Record; + /** Legacy field from pre-report flow-execute results. */ + error?: string; +}; + export type FlowExecuteResult = { flow: string; + device?: string; executionPrerequisite?: string; - steps: { - kind: string; - tool?: string; - message?: string; - result?: unknown; - outputHint?: string; - args?: unknown; - error?: string; - }[]; + ok?: boolean; + passed?: number; + failed?: number; + skipped?: number; + errored?: number; + steps: FlowStepResult[]; +}; + +const STATUS_GLYPH: Record = { + pass: "✓", + fail: "✗", + error: "✗", + skip: "·", }; +function stepLabel(step: FlowStepResult): string { + if (step.kind === "echo") return step.message ?? ""; + if (step.kind === "run") return `run ${step.flow ?? ""}`.trim(); + if (step.tool) return step.tool; + if (step.target) return `${step.kind} ${step.target}`; + return step.kind; +} + /** - * Unpack flow-execute's structured step results into MCP content blocks. - * Each step carries its own outputHint so toMcpContent handles images correctly. + * Unpack flow-execute's structured step report into MCP content blocks. Only + * steps that carry a tool result surface their (image-bearing) content inline; + * directive steps (tap/assert/expect/run/skip) render as a status line. This + * never calls toMcpContent on an undefined result, which would serialize to an + * invalid (text: undefined) content block. */ export async function flowRunToMcpContent( result: FlowExecuteResult, @@ -217,35 +275,91 @@ export async function flowRunToMcpContent( const blocks: ContentBlock[] = []; if (result.executionPrerequisite) { - blocks.push({ - type: "text", - text: `Prerequisite: ${result.executionPrerequisite}`, - }); + blocks.push({ type: "text", text: `Prerequisite: ${result.executionPrerequisite}` }); } blocks.push({ type: "text", - text: `Running flow "${result.flow}" (${result.steps.length} steps)`, + text: `Running flow "${result.flow}"${result.device ? ` on ${result.device}` : ""} (${result.steps.length} steps)`, }); for (let i = 0; i < result.steps.length; i++) { const step = result.steps[i]!; - const num = i + 1; + const num = step.index !== undefined ? step.index + 1 : i + 1; + // Glyph only when a status is present (the new report). Legacy status-less + // results render without one. + const glyph = step.status ? `${STATUS_GLYPH[step.status] ?? "•"} ` : ""; + // `reason` is the new field; `error` is the legacy one. + const reason = step.reason ?? step.error; + const suffix = reason ? ` — ${reason}` : ""; + const warning = step.warning ? ` ⚠ ${step.warning}` : ""; + blocks.push({ type: "text", text: `[${num}] ${glyph}${stepLabel(step)}${suffix}${warning}` }); - if (step.kind === "echo") { - blocks.push({ type: "text", text: `[${num}] ${step.message}` }); - } else if ("error" in step && step.error) { - blocks.push({ - type: "text", - text: `[${num}] ${step.tool} ERROR: ${step.error}`, - }); - } else { - blocks.push({ type: "text", text: `[${num}] ${step.tool}` }); - const stepContent = await toMcpContent(step.result, step.outputHint, ctx, step.args); - blocks.push(...stepContent); + // Surface a step's own content (e.g. a screenshot) only when it actually + // returned one. + if (step.result !== undefined) { + blocks.push(...(await toMcpContent(step.result, step.outputHint, ctx, step.args))); + } + + // Snapshot steps carry artifacts instead of a result — list their paths, + // and inline the annotated diff image when the assertion failed. + if (isRecord(step.artifacts)) { + blocks.push(...(await stepArtifactBlocks(step.artifacts, step.status, ctx))); + } + } + + if (result.ok !== undefined) { + blocks.push({ + type: "text", + text: `${result.ok ? "PASS" : "FAIL"} — ${result.passed ?? 0} passed, ${result.failed ?? 0} failed, ${result.errored ?? 0} errored, ${result.skipped ?? 0} skipped`, + }); + } else { + blocks.push({ type: "text", text: `Flow "${result.flow}" complete.` }); + } + return blocks; +} + +/** + * Render a step's artifacts (snapshot baseline/current/diff): one text block + * listing each artifact, plus the annotated diff image inline when the step + * failed — otherwise the agent has no way to see WHAT differed. Only that + * inlined diff is materialized (local read or remote download); baseline and + * current are full-res PNGs nobody renders, so their handles print as + * tool-server paths (or filenames) without pulling the bytes over the wire — + * the same economy flow-visual.ts applies by omitting artifacts on a clean + * pass. A legacy string[] (pre-handle tool-servers) renders its paths as + * plain text. + */ +async function stepArtifactBlocks( + artifacts: Record, + status: string | undefined, + ctx?: ContentContext +): Promise { + const failed = status === "fail" || status === "error"; + const entries: [string, string][] = []; + let diffImage: ContentBlock | undefined; + + for (const [k, v] of Object.entries(artifacts)) { + if (ctx && failed && k === "diff" && isArtifactHandle(v)) { + // The one artifact rendered inline: materialize it so the image works + // against a remote tool-server too. + const { result, images } = await materializeArtifacts(v, ctx); + // A null means the handle couldn't be fetched; say so rather than + // rendering a dangling reference. + entries.push([k, typeof result === "string" ? result : "(unavailable)"]); + const img = images.find((i) => i.localPath === result); + if (img) diffImage = imageBlock(img.data, img.mimeType); + } else if (isArtifactHandle(v)) { + entries.push([k, v.hostPath ?? v.filename]); + } else if (typeof v === "string") { + entries.push([k, v]); } } - blocks.push({ type: "text", text: `Flow "${result.flow}" complete.` }); + const blocks: ContentBlock[] = + entries.length > 0 + ? [{ type: "text", text: entries.map(([k, v]) => ` ${k}: ${v}`).join("\n") }] + : []; + if (diffImage) blocks.push(diffImage); return blocks; } diff --git a/packages/argent-mcp/test/content.test.ts b/packages/argent-mcp/test/content.test.ts index 25c8d158a..f03c7e690 100644 --- a/packages/argent-mcp/test/content.test.ts +++ b/packages/argent-mcp/test/content.test.ts @@ -304,7 +304,7 @@ describe("flowRunToMcpContent", () => { expect(blocks[1]).toEqual({ type: "text", text: "[1] Hello" }); }); - it("renders tool error steps", async () => { + it("renders legacy tool error steps (status-less)", async () => { const input: FlowExecuteResult = { flow: "f", steps: [{ kind: "tool", tool: "gesture-tap", error: "connection lost" }], @@ -313,10 +313,189 @@ describe("flowRunToMcpContent", () => { expect(blocks[1]).toEqual({ type: "text", - text: "[1] gesture-tap ERROR: connection lost", + text: "[1] gesture-tap — connection lost", }); }); + it("renders the new report shape: status glyphs, reasons, directive kinds, and summary", async () => { + const input: FlowExecuteResult = { + flow: "checkout", + device: "SIM", + ok: false, + passed: 2, + failed: 1, + errored: 0, + skipped: 1, + steps: [ + { index: 0, kind: "tap", status: "pass" }, + { index: 1, kind: "assert", status: "pass" }, + { index: 2, kind: "snapshot", status: "fail", reason: "diff 3.10% > 0.5% (home)" }, + { index: 3, kind: "echo", status: "skip", message: "done" }, + ], + }; + const blocks = await flowRunToMcpContent(input); + const texts = blocks + .filter((b): b is { type: "text"; text: string } => b.type === "text") + .map((b) => b.text); + + expect(texts[0]).toBe('Running flow "checkout" on SIM (4 steps)'); + expect(texts[1]).toBe("[1] ✓ tap"); + expect(texts[2]).toBe("[2] ✓ assert"); + expect(texts[3]).toBe("[3] ✗ snapshot — diff 3.10% > 0.5% (home)"); + expect(texts[4]).toBe("[4] · done"); + expect(texts[texts.length - 1]).toBe("FAIL — 2 passed, 1 failed, 0 errored, 1 skipped"); + // No invalid (text: undefined) blocks even though directive steps carry no result. + expect(blocks.every((b) => b.type !== "text" || typeof b.text === "string")).toBe(true); + }); + + it("surfaces a legacy passed step's warning on its status line (older tool-servers adopted missing baselines)", async () => { + const input: FlowExecuteResult = { + flow: "f", + steps: [ + { + index: 0, + kind: "snapshot", + status: "pass", + reason: "baseline created (home__ios-390x844.png)", + warning: 'no baseline existed for "home" — nothing was compared', + }, + ], + }; + const blocks = await flowRunToMcpContent(input); + + expect(blocks[1]).toEqual({ + type: "text", + text: '[1] ✓ snapshot — baseline created (home__ios-390x844.png) ⚠ no baseline existed for "home" — nothing was compared', + }); + }); + + it("materializes only the diff and inlines it on failure", async () => { + const pngBytes = [...PNG_SIGNATURE, 0x02]; + const fetchImpl = vi.fn(fetchReturning(pngBytes)); + const input: FlowExecuteResult = { + flow: "checkout", + steps: [ + { + index: 0, + kind: "snapshot", + status: "fail", + reason: "diff 3.10% > 0.5% (home)", + artifacts: { + baseline: artifactHandle("b1", "home-baseline.png", "image/png"), + current: artifactHandle("c1", "home-current.png", "image/png"), + diff: artifactHandle("d1", "home-diff.png", "image/png"), + }, + }, + ], + }; + const blocks = await flowRunToMcpContent(input, { + toolsUrl: "http://remote:3001", + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + const artifactText = blocks.find( + (b): b is { type: "text"; text: string } => b.type === "text" && b.text.includes("baseline:") + ); + expect(artifactText?.text).toContain("home-baseline.png"); + expect(artifactText?.text).toContain("home-current.png"); + expect(artifactText?.text).toMatch(/diff: .*home-diff\.png/); + + // Exactly one inline image — the diff, not the full-res baseline/current. + const images = blocks.filter((b) => b.type === "image"); + expect(images).toHaveLength(1); + expect(images[0]).toMatchObject({ data: Buffer.from(pngBytes).toString("base64") }); + + // And exactly one download: baseline/current are referenced by name only, + // never pulled over the wire just to print their paths. + expect(fetchImpl).toHaveBeenCalledTimes(1); + expect(String(fetchImpl.mock.calls[0]?.[0])).toContain("/artifacts/d1"); + }); + + it("lists snapshot artifact paths without fetching anything when the step passed", async () => { + const fetchImpl = vi.fn(fetchReturning([...PNG_SIGNATURE, 0x03])); + const input: FlowExecuteResult = { + flow: "checkout", + steps: [ + { + index: 0, + kind: "snapshot", + status: "pass", + reason: "diff 0.00% ≤ 0.5% (home)", + artifacts: { + baseline: artifactHandle("b1", "home-baseline.png", "image/png"), + current: artifactHandle("c1", "home-current.png", "image/png"), + }, + }, + ], + }; + const blocks = await flowRunToMcpContent(input, { + toolsUrl: "http://remote:3001", + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + expect(blocks.find((b) => b.type === "image")).toBeUndefined(); + const artifactText = blocks.find( + (b): b is { type: "text"; text: string } => b.type === "text" && b.text.includes("baseline:") + ); + expect(artifactText?.text).toContain("home-baseline.png"); + expect(fetchImpl).not.toHaveBeenCalled(); + }); + + it("falls back to artifact host paths when no materialize context is given", async () => { + const input: FlowExecuteResult = { + flow: "checkout", + steps: [ + { + index: 0, + kind: "snapshot", + status: "fail", + reason: "diff 3.10% > 0.5% (home)", + artifacts: { + baseline: { + ...artifactHandle("b1", "base.png", "image/png"), + hostPath: "/srv/base.png", + }, + diff: { ...artifactHandle("d1", "diff.png", "image/png"), hostPath: "/srv/diff.png" }, + }, + }, + ], + }; + const blocks = await flowRunToMcpContent(input); + + expect(blocks.find((b) => b.type === "image")).toBeUndefined(); + const artifactText = blocks.find( + (b): b is { type: "text"; text: string } => b.type === "text" && b.text.includes("baseline:") + ); + expect(artifactText?.text).toContain("baseline: /srv/base.png"); + expect(artifactText?.text).toContain("diff: /srv/diff.png"); + }); + + it("renders legacy string[] artifacts as plain path lines", async () => { + const input: FlowExecuteResult = { + flow: "checkout", + steps: [ + { + index: 0, + kind: "snapshot", + status: "fail", + reason: "diff 3.10% > 0.5% (home)", + artifacts: ["/srv/baseline.png", "/srv/current.png"] as unknown as Record< + string, + unknown + >, + }, + ], + }; + const blocks = await flowRunToMcpContent(input); + + const artifactText = blocks.find( + (b): b is { type: "text"; text: string } => + b.type === "text" && b.text.includes("/srv/baseline.png") + ); + expect(artifactText).toBeDefined(); + expect(blocks.find((b) => b.type === "image")).toBeUndefined(); + }); + it("renders tool success as JSON text", async () => { const input: FlowExecuteResult = { flow: "f", diff --git a/packages/argent-tools-client/src/index.ts b/packages/argent-tools-client/src/index.ts index 6d2231e71..89acff295 100644 --- a/packages/argent-tools-client/src/index.ts +++ b/packages/argent-tools-client/src/index.ts @@ -29,6 +29,7 @@ export { type ToolsClient, type ToolMeta, type ToolInvocationResult, + type CallToolOptions, type CreateToolsClientOptions, } from "./tools-client.js"; diff --git a/packages/argent-tools-client/src/tools-client.ts b/packages/argent-tools-client/src/tools-client.ts index d441fa832..14f6b638a 100644 --- a/packages/argent-tools-client/src/tools-client.ts +++ b/packages/argent-tools-client/src/tools-client.ts @@ -19,6 +19,16 @@ export interface ToolInvocationResult { note?: string; } +export interface CallToolOptions { + /** + * Receive live progress events while the tool runs. Setting this asks the + * server for an NDJSON stream (`Accept: application/x-ndjson`); a server + * that predates streaming ignores the header and replies with plain JSON, + * in which case no events fire and the call behaves exactly as before. + */ + onProgress?: (event: unknown) => void; +} + /** * The CLI and MCP server each instantiate a client bound to the bundled paths * known by the published package. Keeping it as a factory avoids a hidden @@ -27,7 +37,7 @@ export interface ToolInvocationResult { export interface ToolsClient { fetchTools(): Promise; fetchTool(name: string): Promise; - callTool(name: string, args: unknown): Promise; + callTool(name: string, args: unknown, opts?: CallToolOptions): Promise; /** Returns the tool-server base URL + auth token, spawning if needed. */ baseUrl(): Promise; } @@ -41,6 +51,62 @@ function authHeaders(token: string | undefined): Record { return token ? { Authorization: `Bearer ${token}` } : {}; } +/** + * Read an NDJSON tool-invocation stream: each `progress` line fires the + * callback as it arrives, the terminal `result` line becomes the return value, + * and a terminal `error` line throws — mirroring the buffered path's contract. + * A stream that ends with no terminal line means the connection died mid-run. + */ +async function consumeToolStream( + body: ReadableStream, + onProgress: (event: unknown) => void +): Promise { + let final: { data?: unknown; note?: string } | undefined; + const handleLine = (line: string): void => { + if (!line.trim()) return; + const msg = JSON.parse(line) as { + event?: string; + data?: unknown; + note?: string; + error?: string; + }; + if (msg.event === "progress") onProgress(msg.data); + else if (msg.event === "result") final = { data: msg.data, note: msg.note }; + else if (msg.event === "error") throw new Error(msg.error ?? "tool invocation failed"); + }; + + const reader = body.getReader(); + const decoder = new TextDecoder(); + let buffered = ""; + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + buffered += decoder.decode(value, { stream: true }); + let newline: number; + while ((newline = buffered.indexOf("\n")) !== -1) { + const line = buffered.slice(0, newline); + buffered = buffered.slice(newline + 1); + handleLine(line); + } + } + buffered += decoder.decode(); + if (buffered.trim()) handleLine(buffered); + } catch (err) { + // Terminal `error` line or a mid-stream parse/read failure: drop the rest + // of the stream (the server has ended it anyway) and surface the error. + void reader.cancel().catch(() => {}); + throw err; + } + + if (!final) { + throw new Error("tool stream ended without a result — connection lost mid-run?"); + } + // File boundary, inbound: same directive handling as the buffered path. + const { result: data } = await applyClientFileDirectives(final.data); + return { data, note: final.note }; +} + export function createToolsClient(options: CreateToolsClientOptions = {}): ToolsClient { let cached: ToolsServerHandle | null = null; @@ -78,7 +144,11 @@ export function createToolsClient(options: CreateToolsClientOptions = {}): Tools return tools.find((t) => t.name === name) ?? null; } - async function callTool(name: string, args: unknown): Promise { + async function callTool( + name: string, + args: unknown, + opts?: CallToolOptions + ): Promise { const { url, token } = await baseUrl(); // File boundary, outbound: wrap declared file-path args so the server can @@ -96,9 +166,21 @@ export function createToolsClient(options: CreateToolsClientOptions = {}): Tools const res = await fetch(`${url}/tools/${encodeURIComponent(name)}`, { method: "POST", - headers: { "Content-Type": "application/json", ...authHeaders(token) }, + headers: { + "Content-Type": "application/json", + ...(opts?.onProgress ? { Accept: "application/x-ndjson" } : {}), + ...authHeaders(token), + }, body: JSON.stringify(finalArgs ?? {}), }); + // The server only streams when the request asked for it AND every + // pre-invoke gate passed (validation errors stay plain JSON with their + // status codes); a pre-streaming server ignores the Accept header + // entirely. Content-Type is therefore the authoritative mode signal. + const contentType = res.headers.get("content-type") ?? ""; + if (opts?.onProgress && res.ok && res.body && contentType.includes("application/x-ndjson")) { + return consumeToolStream(res.body, opts.onProgress); + } const json = (await res.json().catch(() => ({}))) as { data?: unknown; error?: string; diff --git a/packages/argent-tools-client/test/stream.test.ts b/packages/argent-tools-client/test/stream.test.ts new file mode 100644 index 000000000..8dcfcfe49 --- /dev/null +++ b/packages/argent-tools-client/test/stream.test.ts @@ -0,0 +1,118 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { createServer, type Server, type IncomingMessage, type ServerResponse } from "node:http"; +import type { AddressInfo } from "node:net"; +import { createToolsClient } from "../src/tools-client.js"; + +let server: Server | undefined; + +/** + * Stand up a stub tool-server: GET /tools advertises one tool ("streamy", + * no fileInputs so the file boundary stays out of the way); POST behavior is + * supplied per test. ARGENT_TOOLS_URL routes the client at it. + */ +async function startServer( + onInvoke: (req: IncomingMessage, res: ServerResponse) => void +): Promise { + server = createServer((req, res) => { + if (req.method === "GET" && req.url === "/tools") { + res.setHeader("Content-Type", "application/json"); + res.end(JSON.stringify({ tools: [{ name: "streamy", description: "", inputSchema: {} }] })); + return; + } + if (req.method === "POST" && req.url === "/tools/streamy") { + onInvoke(req, res); + return; + } + res.statusCode = 404; + res.end(JSON.stringify({ error: "not found" })); + }); + await new Promise((resolve) => server!.listen(0, "127.0.0.1", resolve)); + const { port } = server!.address() as AddressInfo; + vi.stubEnv("ARGENT_TOOLS_URL", `http://127.0.0.1:${port}`); +} + +afterEach(async () => { + vi.unstubAllEnvs(); + if (server) { + await new Promise((resolve) => server!.close(() => resolve())); + server = undefined; + } +}); + +describe("callTool progress streaming", () => { + it("fires onProgress per NDJSON line and resolves with the terminal result", async () => { + await startServer((_req, res) => { + res.writeHead(200, { "Content-Type": "application/x-ndjson" }); + res.write(`${JSON.stringify({ event: "progress", data: { index: 0 } })}\n`); + res.write(`${JSON.stringify({ event: "progress", data: { index: 1 } })}\n`); + res.end(`${JSON.stringify({ event: "result", data: { ok: true }, note: "hi" })}\n`); + }); + + const events: unknown[] = []; + const { callTool } = createToolsClient(); + const result = await callTool("streamy", {}, { onProgress: (e) => events.push(e) }); + + expect(events).toEqual([{ index: 0 }, { index: 1 }]); + expect(result.data).toEqual({ ok: true }); + expect(result.note).toBe("hi"); + }); + + it("sends the Accept header only when a progress consumer is attached", async () => { + const accepts: Array = []; + await startServer((req, res) => { + accepts.push(req.headers.accept); + res.setHeader("Content-Type", "application/json"); + res.end(JSON.stringify({ data: { ok: true } })); + }); + + const { callTool } = createToolsClient(); + await callTool("streamy", {}); + await callTool("streamy", {}, { onProgress: () => {} }); + + expect(accepts[0] ?? "").not.toContain("application/x-ndjson"); + expect(accepts[1]).toContain("application/x-ndjson"); + }); + + it("falls back to the buffered path against a pre-streaming server", async () => { + // An old server ignores the Accept header and replies plain JSON. + await startServer((_req, res) => { + res.setHeader("Content-Type", "application/json"); + res.end(JSON.stringify({ data: { ok: true } })); + }); + + const events: unknown[] = []; + const { callTool } = createToolsClient(); + const result = await callTool("streamy", {}, { onProgress: (e) => events.push(e) }); + + expect(events).toEqual([]); + expect(result.data).toEqual({ ok: true }); + }); + + it("rejects with the in-band terminal error", async () => { + await startServer((_req, res) => { + res.writeHead(200, { "Content-Type": "application/x-ndjson" }); + res.write(`${JSON.stringify({ event: "progress", data: { index: 0 } })}\n`); + res.end(`${JSON.stringify({ event: "error", error: "kaput" })}\n`); + }); + + const events: unknown[] = []; + const { callTool } = createToolsClient(); + await expect(callTool("streamy", {}, { onProgress: (e) => events.push(e) })).rejects.toThrow( + "kaput" + ); + expect(events).toEqual([{ index: 0 }]); + }); + + it("rejects when the stream ends without a terminal line (connection lost)", async () => { + await startServer((_req, res) => { + res.writeHead(200, { "Content-Type": "application/x-ndjson" }); + res.write(`${JSON.stringify({ event: "progress", data: { index: 0 } })}\n`); + res.end(); + }); + + const { callTool } = createToolsClient(); + await expect(callTool("streamy", {}, { onProgress: () => {} })).rejects.toThrow( + /without a result/ + ); + }); +}); diff --git a/packages/argent/src/cli.ts b/packages/argent/src/cli.ts index 530964c41..7d94e41a1 100644 --- a/packages/argent/src/cli.ts +++ b/packages/argent/src/cli.ts @@ -76,6 +76,7 @@ Commands: remove Alias for uninstall tools List tools exposed by the tool-server run Invoke a tool by name (use \`argent run --help\` for flags) + flow Run a saved flow (use \`argent flow --help\` for options) server Manage the shared tool-server (start / status / stop / logs) lens Open Argent Lens bound to a fresh coding-agent session (macOS) link Route client requests to a remote tool-server @@ -124,6 +125,8 @@ async function main(): Promise { return (await loadCli()).tools(rest, { paths: BUNDLED_RUNTIME_PATHS }); case "run": return (await loadCli()).run(rest, { paths: BUNDLED_RUNTIME_PATHS }); + case "flow": + return (await loadCli()).flow(rest, { paths: BUNDLED_RUNTIME_PATHS }); case "server": return (await loadCli()).server(rest, { paths: BUNDLED_RUNTIME_PATHS }); case "lens": diff --git a/packages/registry/src/failure-codes.ts b/packages/registry/src/failure-codes.ts index 3be742d26..20542f679 100644 --- a/packages/registry/src/failure-codes.ts +++ b/packages/registry/src/failure-codes.ts @@ -196,6 +196,8 @@ export const FAILURE_CODES = { FLOW_NO_ACTIVE_RECORDING: "FLOW_NO_ACTIVE_RECORDING", FLOW_FILE_INVALID: "FLOW_FILE_INVALID", FLOW_ENTRY_UNRECOGNIZED: "FLOW_ENTRY_UNRECOGNIZED", + FLOW_E2E_HAS_PREREQUISITE: "FLOW_E2E_HAS_PREREQUISITE", + FLOW_DEVICE_RESOLUTION: "FLOW_DEVICE_RESOLUTION", PROFILER_QUERY_MODE_INVALID: "PROFILER_QUERY_MODE_INVALID", PROFILER_QUERY_REQUIRED_PARAM_MISSING: "PROFILER_QUERY_REQUIRED_PARAM_MISSING", PROFILER_DATA_NOT_LOADED: "PROFILER_DATA_NOT_LOADED", diff --git a/packages/registry/src/types.ts b/packages/registry/src/types.ts index a60583cc1..a3f3b3fc3 100644 --- a/packages/registry/src/types.ts +++ b/packages/registry/src/types.ts @@ -117,6 +117,15 @@ export interface InvokeToolOptions { * reads nor validates the recorded metadata. */ recordChildInvocation?: (toolInvocationId: string, childArgs?: unknown) => () => void; + /** + * Fire-and-forget progress events emitted by a long-running tool while it + * executes (e.g. flow-execute streaming one report per completed step). Set + * by transports that can deliver increments — the HTTP layer's NDJSON mode — + * and absent when the caller can only consume a final result. Tools must + * treat it as optional and never behave differently based on its presence; + * the final return value remains the complete, authoritative result. + */ + emitProgress?: (event: unknown) => void; } /** diff --git a/packages/skills/skills/argent-create-flow/SKILL.md b/packages/skills/skills/argent-create-flow/SKILL.md index 40dc8aa46..76f871232 100644 --- a/packages/skills/skills/argent-create-flow/SKILL.md +++ b/packages/skills/skills/argent-create-flow/SKILL.md @@ -3,143 +3,174 @@ name: argent-create-flow description: Record a reusable flow (scripted sequence of MCP tool calls) that can be replayed later with a single command. Use when the user asks to create, record, or build a flow, or to script a sequence of device actions. --- -## 1. Overview +## Overview -A flow is a recorded sequence of MCP tool calls saved to a `.yaml` file in the `.argent/flows/` directory. Each step is **executed live** as you add it, so you verify it works before it becomes part of the flow. Replay a finished flow with `flow-execute`. +A flow is a sequence of steps saved to a `.yaml` file in the `.argent/flows/` directory. Each recorded step is **executed live** as you add it, so you verify it works before it becomes part of the flow. Replay a finished flow with `flow-execute`, or — for an e2e flow — headlessly with `argent flow run `. -## 2. Tools +Flows store **no device id**: the runner binds a device (the single booted one, or pass `device`/`platform`). A recorded coordinate `gesture-tap` is captured as a portable `tap: { selector }` step whenever the tapped element has stable text/identifier. -| Tool | Purpose | -| ------------------------ | -------------------------------------------------------------------------- | -| `flow-start-recording` | Start recording — takes a name and executionPrerequisite, creates the file | -| `flow-add-step` | Execute a tool call live and record it if it succeeds | -| `flow-add-echo` | Add a label/comment that prints during replay | -| `flow-finish-recording` | Stop recording and get a summary | -| `flow-read-prerequisite` | Read a flow's execution prerequisite without running it | -| `flow-execute` | Replay a saved flow by name | +**Two flow types** -## 3. Workflow +- **e2e** — begins with a `launch:` step, which starts that app from scratch (terminate + relaunch), so the flow controls its own start state. No `executionPrerequisite`. May `run:` fragments; cannot itself be a `run:` target. Record one by adding a `restart-app` of the app under test as the **first** step — it is captured as the `launch` step. +- **fragment** — doesn't begin with a launch; runs against the device's current state. May declare an `executionPrerequisite` (a documented entry-state contract). Invoked from other flows via a `run:` step, or directly by you at any time. -### Recording +Both run via `argent flow run ` — a fragment simply runs against whatever is on screen (its prerequisite is printed as a reminder). Only e2e flows are meaningful CI/suite entries, since only they give a deterministic verdict from a clean start. -1. **Start**: Call `flow-start-recording` with a descriptive name, the absolute `project_root`, and an `executionPrerequisite` describing the required app state before running the flow (e.g. "App on home screen after a fresh reload"). `project_root` is stored for the session — you do **not** need to pass it again to subsequent tools. -2. **Build step-by-step**: For each action, call `flow-add-step` with the tool name and args. The tool runs immediately — check the result before moving on. -3. **Add labels**: Use `flow-add-echo` between steps to describe what each section does. -4. **Finish**: Call `flow-finish-recording` to stop recording. It returns the file path where the flow was saved and a summary of all steps. You can edit the `.yaml` file directly afterwards to remove, reorder, or tweak steps. +### Step directives -Every tool during recording returns the current flow file contents so you can track what has been recorded. +Beyond raw `tool:` steps and `echo:`, flows support declarative directives interpreted by the runner (they are **not** agent-callable tools). **Every directive hard-stops the flow on failure**; later steps are reported `skip`. -### Replaying +| Directive | YAML | Meaning | +| ----------- | -------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------- | +| `launch` | `- launch: com.acme.app` or `- launch: { ios: …, android: … }` | start the app from scratch (terminate + relaunch) and wait until ready | +| `tap` | `- tap: Login` or `- tap: { x: 0.5, y: 0.57 }` | tap an element by selector (auto-waits), or a raw normalized point | +| `type` | `- type: { into: email, text: "a@b.com" }` | focus a field, type, then press Enter to submit + dismiss the keyboard | +| `scroll-to` | `- scroll-to: "Order #1234"` (scrolls down) or `- scroll-to: { target: …, direction: right, within: … }` | momentum-free scroll until the target is visible | +| `await` | `- await: { visible: Home }` | wait for a UI condition | +| `wait` | `- wait: 500` | pause for a fixed number of milliseconds (last resort — prefer `await`) | +| `assert` | `- assert: { visible: Welcome }` | check a condition, hard-fail if it never holds | +| `snapshot` | `- snapshot: home` or `- snapshot: { name: home, maxMismatch: 0.5 }` | diff a screenshot against a stored baseline | +| `run` | `- run: login` | execute a fragment's steps inline | -Call `flow-execute` with the flow name. If the flow has an execution prerequisite: +### Selectors -1. The tool returns a **notice** with the prerequisite text instead of running. It asks you to verify the prerequisite is met and call `flow-execute` again with `prerequisiteAcknowledged: true`. -2. You can also call `flow-read-prerequisite` beforehand to inspect the prerequisite without triggering a run. -3. Once you pass `prerequisiteAcknowledged: true`, the flow runs all steps in order and returns every tool call result (including screenshots) merged into a single response. +A **selector** is `{ text?, id?, role? }` (all-must-match; `text`/`role` are case-insensitive substrings, `id` matches the element's testID / accessibilityIdentifier / resource-id exactly, case-insensitive, also accepting the unqualified Android resource-id name — `submit` matches `com.example.app:id/submit`) — the same semantics `await-ui-element` uses, though that tool spells the `id` field `identifier` (flow YAML also accepts `identifier` as an alias for `id`, but `id` is the canonical spelling and what the recorder writes). A bare string is a _loose_ selector: it resolves **identifier-first, then falls back to text** (label/value), so `tap: Login` matches a `testID="Login"` or, failing that, visible text "Login" — no need to know which. Loose fallback applies uniformly to every selector slot (`tap`, `type.into`, `await`, `assert`, `scroll-to`). Use the map form to be strict: `{ id: submit-btn }` (identifier only) or `{ text: Login }` (text only, no fallback). -If the flow has no prerequisite, it runs immediately without needing acknowledgment. +Selectors resolve against the **full native hierarchy** (iOS: the UIView tree; Android: the complete accessibility hierarchy including not-important views) — strictly more than `describe` or the raw `await-ui-element` tool see (both use the trimmed tree), with complete `testID`/`resource-id` coverage. So an `id` selector works even when `describe` collapses or omits the element — don't fall back to coordinate taps just because a testID isn't visible in `describe` output. And when several elements match — a container aggregates its descendants' text, so `text: "Inner"` matches the wrapping containers too — the action directives (`tap`, `type`, `scroll-to`) pick the **most specific** match: an exact text/identifier match beats a substring hit, then the smallest frame wins. -## 4. flow-add-step Usage +**Quote strings YAML would mangle.** An unquoted `#` starts a YAML comment — `tap: Order #1234` silently parses as `tap: Order` — and bare `yes`/`no`/`on`/`off`/numbers coerce to non-strings. When a selector or typed text contains `#`, `:`, quotes, or could read as a boolean/number, wrap it: `tap: "Order #1234"`. -The `command` parameter is the MCP tool name. The `args` parameter is a **JSON string** (not an object): +### `await` and `assert` +The **condition is the key**, and its value is the selector: + +- `{ visible: Home }`, `{ exists: { id: row } }`, `{ hidden: spinner }` +- `{ text: { in: , contains: "Taps:" } }` or `{ text: { in: , equals: "Taps: 0" } }` — `text` locates an element (`in`) and checks its rendered content against exactly one of `contains` (case-insensitive substring) or `equals` (case-insensitive exact match — use it when boundaries matter: `contains: "Taps: 3"` is also satisfied by "Taps: 30"). Reach for `text` only when the locator is an identifier/role; to assert a string is simply on screen, prefer `{ visible: "Taps: 0" }`. +- A container's text aggregates its descendants' text (space-joined), so `text` can assert what a testID wrapper visibly shows even when the string lives in a child node. That also means `equals` against a wrapper must match _everything_ it shows or exactly the wrapper's own label/value — targeting the leaf holding exactly the value (or using `contains`) stays the clearer spelling. + +This condition-as-key form is the only spelling. `await` also accepts an optional `timeout` sibling key in milliseconds — `- await: { visible: Home, timeout: 15000 }` — for a transition that legitimately needs longer than the default budget. **Omit `timeout` by default**: the default budget covers normal transitions, and a habitual generous override just delays failure reporting on every broken step. Add one only after a step demonstrably needs it — it timed out at the default and the wait is legitimately slow (a cold start, a network round-trip, a long animation). `assert` has no timeout override: a check that needs seconds to become true is a wait — spell it `await`. + +For a custom poll interval or bundleId, drop to an explicit `- tool: await-ui-element` step — but the raw tool polls the trimmed `describe` tree, so a testID it reports as not found can still resolve fine as an `await:` directive (see Selectors). Prefer the directive. + +### `type` and `scroll-to` + +`type` presses Enter after typing to commit the value and dismiss the keyboard, so it can't cover later targets. For a chained form whose fields feed one explicit submit — e.g. email then password then a `tap: "Log in"` — set `submit: false` on the intermediate fields so a premature Enter doesn't fire the form early: `type: { into: password, text: "hunter2", submit: false }`. + +`scroll-to` takes an optional `direction` (`up` | `down` | `left` | `right`, default `down` — so the common case is just `- scroll-to: `) and optionally a `within: ` that anchors the scroll inside a specific container — required to drive a **nested** scroller (e.g. a horizontal carousel inside a vertical list), since the device can't be asked which container to scroll. It scrolls in bounded momentum-free increments, re-checks after each, and stops if a scroll reveals nothing new (end of the container). `tap`/`type` do **not** scroll — add a `scroll-to` before any target that may be off-screen. It's a no-op when the target is already visible, so a defensive `scroll-to` costs nothing on replay and keeps the flow working on smaller screens. + +### TV targets (Vega) + +A Vega (Fire TV) device is remote-driven — there is no touch input, so the touch directives (`tap`, `type`, `scroll-to`) fail on it with guidance. Drive focus with `tool: tv-remote` steps and type with `tool: keyboard` instead; everything else (`launch`, `await`, `assert`, `wait`, `snapshot`, `echo`, `run`, selectors) works unchanged — the tree comes from the on-device automation toolkit, which attaches at app launch (the `launch` step waits for it, so a leading `launch` also guarantees selectors resolve). + +```yaml +steps: + - launch: com.example.app.main # the interactive component id from manifest.toml + - await: { visible: Home } + - tool: tv-remote + args: { button: [down, select] } # move focus, then confirm — one step per navigation + - await: { visible: Explore Screen } + - snapshot: explore ``` -command: "launch-app" -args: "{\"udid\": \"\", \"bundleId\": \"com.apple.Preferences\"}" -``` + +Since a `tv-remote` path is positional (like a coordinate tap), gate each navigation with an `await` on the destination screen and echo where focus should be — that is what makes the flow diagnosable when the focus order changes. + +### Standalone runner + +`argent flow run [--device ] [--platform ios|android|chromium|vega] [--update-baselines] [--output ] [--json]` runs a flow with no LLM in the loop and exits non-zero on any failure — suitable for CI (e2e flows; a fragment runs against the current device state, useful while authoring). `snapshot` baselines live in `.argent/flows/__baselines__//`, keyed by platform + resolution; a `snapshot` step **fails** when no baseline exists for the run's device class, so seed baselines with `--update-baselines`, review them, and commit `__baselines__/` — and pin the device class in CI (`--device`/`--platform`, same simulator model) so runs compare against the committed key. The status bar is pinned (iOS `simctl status_bar`, Android demo mode) for the run so it doesn't drive visual diffs. `--output ` writes each failed snapshot's baseline/current/diff images to `//` — a stable path for CI artifact upload. + +## Tools + +| Tool | Purpose | +| ------------------------ | --------------------------------------------------------------------------------------------------------- | +| `flow-start-recording` | Start recording — takes a name and (fragments only) an optional `executionPrerequisite`; creates the file | +| `flow-add-step` | Execute a tool call live and record it if it succeeds | +| `flow-add-echo` | Add a label/comment that prints during replay | +| `flow-finish-recording` | Stop recording and get a summary | +| `flow-read-prerequisite` | Read a flow's execution prerequisite without running it | +| `flow-execute` | Replay a saved flow by name | + +Every tool during recording returns the current flow file contents, so you can track what has been recorded. Rules: + +- **Every step runs live.** You see the real tool result (including screenshots) — verify the step worked before continuing. **Only successful steps are recorded**: a failed call writes nothing to the flow file; fix the issue and try again. +- **Pass `project_root` once.** Give the absolute `project_root` (an error is returned if the path is not absolute) to `flow-start-recording` — it is stored for the session and used by all subsequent flow tools. You do **not** pass a flow name to `flow-add-step`, `flow-add-echo`, or `flow-finish-recording` — the active flow is tracked automatically. +- **Start before adding.** Calling those tools without an active recording returns _"No active flow. Call flow-start-recording first."_ +- **One flow at a time.** `flow-start-recording` while already recording switches to the new flow — the response tells you which flow was abandoned and which is now active; the old flow's file remains on disk. +- **Mistakes can be edited out.** Edit the `.yaml` file directly to remove or reorder steps. + +### flow-add-step arguments + +The `command` parameter is the MCP tool name; `args` is a **JSON string** (not an object), omitted entirely for tools with no arguments: ``` command: "gesture-tap" args: "{\"udid\": \"\", \"x\": 0.5, \"y\": 0.35}" -``` -``` -command: "screenshot" -args: "{\"udid\": \"\"}" -``` - -``` command: "await-ui-element" args: "{\"udid\": \"\", \"condition\": \"visible\", \"selector\": {\"text\": \"Continue\"}}" ``` -Record an `await-ui-element` step to **gate** the next step on a screen transition — it blocks until the element is `visible`/`hidden` (or contains `text`), so the following step runs only once the screen has actually settled. If its condition is not met before the timeout, replay **stops at that step** (the steps after it assume the transition happened). Prefer this over a fixed `delayMs`. See the `await-ui-element` section of `argent-device-interact` for the full condition/selector reference. +Record an `await-ui-element` step to **gate** the next step on a screen transition — it blocks until the element is `visible`/`hidden` (or contains `text`), so the following step runs only once the screen has actually settled; prefer this over a fixed `delayMs`. If its condition is not met before the timeout, replay **stops at that step** (the steps after it assume the transition happened). See the `await-ui-element` section of `argent-device-interact` for the full condition/selector reference. The live call sees only the trimmed `describe` tree — if it can't find an identifier you know exists, gate on visible text to get the step recorded, then retarget the identifier in the `await:` form during polish (the directive resolves the full hierarchy — see Selectors); don't conclude the testID is unusable in the flow. -For tools with no arguments, omit `args` entirely. +## Recording -## 5. Important Rules +1. **Start, then launch as the first step (e2e) or set the stage yourself (fragment).** Call `flow-start-recording` with a descriptive name and the absolute `project_root`. For an **e2e** flow, record a `restart-app` of the app under test as the **first** step — it runs live (resetting the device for the rest of the recording) and is captured as the flow's `launch` step. For a **fragment**, bring the device to the entry state _before_ recording and pass an `executionPrerequisite` describing it (e.g. "App on the login screen") to `flow-start-recording` instead. +2. **Build step-by-step**: for each action, call `flow-add-step` with the tool name and args. The tool runs immediately — check the result before moving on, and gate each navigation with an `await-ui-element` step. +3. **Add labels**: use `flow-add-echo` between steps — echo the expected state, not just the action (see _Making flows resilient_). +4. **Finish**: call `flow-finish-recording`. It returns the file path where the flow was saved and a summary of all steps. +5. **Polish**: **read the saved `.yaml` file** and convert the raw `tool:` steps that have a cleaner directive form (the recorder leaves these as tools): + - `tool: keyboard` typing into a field → `type: { into: "", text: "…" }`, folding in the `tap` that focused the field. + - `tool: await-ui-element` gating a transition → `await: { visible: "…" }` / `{ hidden: … }` / `{ text: { in: …, equals: … } }`, carrying a custom `timeoutMs` over as a `timeout` sibling key. Converting also upgrades the wait from the trimmed `describe` tree to the flow's full-hierarchy tree (see Selectors). Keep the raw `tool: await-ui-element` step only when it sets a custom `pollIntervalMs`/`bundleId` the directive can't express. + - A scroll-to-reach-an-element — a `tool: gesture-swipe` used to bring a specific element on screen before interacting with it (a `tap`, `type`, `assert`, …) → `scroll-to: { target: "", direction: … }`, dropping the swipe. This is far more robust than a fixed-distance swipe: it scrolls momentum-free and stops exactly when the target appears, so it survives layout and content changes. (`tap`/`type` do not scroll, so a raw swipe whose fling lands differently on another device leaves the following tap unresolved — always prefer the `scroll-to` rewrite.) Keep a `gesture-swipe` as a raw `tool:` step when it isn't scrolling toward a specific element — especially a velocity-dependent gesture like swipe-to-dismiss, edge-swipe-back, or swipe-to-reveal a row action, which a momentum-free `scroll-to` would not reproduce. -- **Every step runs live.** You will see the real tool result (including screenshots). Use this to verify the step worked before continuing. -- **Only successful steps are recorded.** If a tool call fails, nothing is written to the flow file — fix the issue and try again. -- **Pass `project_root` only to `flow-start-recording`.** It is stored for the session and automatically used by all subsequent flow tools. An error is returned if the path is not absolute. -- **You do NOT need to pass a flow name** to `flow-add-step`, `flow-add-echo`, or `flow-finish-recording`. The active flow is tracked automatically after `flow-start-recording`. -- **Start before adding.** Calling `flow-add-step`, `flow-add-echo`, or `flow-finish-recording` without an active recording returns an error: _"No active flow. Call flow-start-recording first."_ -- **One flow at a time.** If you call `flow-start-recording` while already recording, the active flow switches to the new one. The response tells you which flow was abandoned and which is now active. The old flow's file remains on disk. -- **Mistakes can be edited out.** If a step was recorded by mistake, edit the `.yaml` file directly to remove or reorder entries. +Every other recorded tool (`gesture-swipe`, `gesture-scroll`, `button`, `screenshot`, …) has no directive form — leave it as a `tool:` step. The recorder already handles the rest: coordinate `gesture-tap`s are captured as portable `tap:` selector steps, a `restart-app` is captured as a `launch:` step, a `flow-execute` of a sibling fragment is captured as a `run: ` composition directive, and device ids are stripped. Captured selectors are emitted in the strict map form (`tap: { text: General }`), never as a loose bare string — the recorder verified the exact element the tap hit, and a bare string would re-parse as loose and route through the identifier-first fallback it was never checked against. After editing, re-run with `flow-execute` to confirm the cleaned flow still passes. -## 6. Example Session +### Example session ``` -flow-start-recording { name: "open-settings", project_root: "/Users/dev/MyApp", executionPrerequisite: "Simulator booted with app installed" } -flow-add-echo { message: "Launch Settings app" } -flow-add-step { command: "launch-app", args: "{\"udid\": \"ABC\", \"bundleId\": \"com.apple.Preferences\"}" } -flow-add-echo { message: "Tap General" } -flow-add-step { command: "gesture-tap", args: "{\"udid\": \"ABC\", \"x\": 0.5, \"y\": 0.35}" } -flow-add-echo { message: "Tap About" } +flow-start-recording { name: "open-about", project_root: "/Users/dev/MyApp" } +flow-add-echo { message: "Start Settings from scratch" } +flow-add-step { command: "restart-app", args: "{\"udid\": \"ABC\", \"bundleId\": \"com.apple.Preferences\"}" } # ⇒ captured as `- launch: com.apple.Preferences` — this is now an e2e flow +flow-add-echo { message: "On the Settings root list, tapping the 'General' row" } +flow-add-step { command: "gesture-tap", args: "{\"udid\": \"ABC\", \"x\": 0.5, \"y\": 0.35}" } # ⇒ captured as `- tap: { text: General }` (portable selector, no udid) +flow-add-step { command: "await-ui-element", args: "{\"udid\": \"ABC\", \"condition\": \"visible\", \"selector\": {\"text\": \"About\"}}" } # gate the transition +flow-add-echo { message: "On Settings > General, tapping 'About'" } flow-add-step { command: "gesture-tap", args: "{\"udid\": \"ABC\", \"x\": 0.5, \"y\": 0.17}" } +flow-add-step { command: "await-ui-element", args: "{\"udid\": \"ABC\", \"condition\": \"visible\", \"selector\": {\"text\": \"Model Name\"}}" } flow-finish-recording {} ``` -## 7. Replay Example +Then polish the saved file: the two `await-ui-element` steps become `await:` directives (see the file below). -``` -flow-execute { name: "open-settings", project_root: "/Users/dev/MyApp" } -→ Returns: notice with executionPrerequisite: "Simulator booted with app installed" - "Verify the prerequisite is met and call flow-execute again with prerequisiteAcknowledged set to true." +## Replaying -flow-execute { name: "open-settings", project_root: "/Users/dev/MyApp", prerequisiteAcknowledged: true } -→ Runs all steps, returns merged results with status and output for every step -``` +Call `flow-execute` with the flow name (and `project_root`, unless a recording this session already stored it). If the flow has an execution prerequisite, the tool returns a **notice** with the prerequisite text instead of running — verify the prerequisite is met (you can also inspect it beforehand with `flow-read-prerequisite`) and call `flow-execute` again with `prerequisiteAcknowledged: true`. A flow without a prerequisite runs immediately. The run executes all steps in order and returns a structured report: `{ ok, passed, failed, skipped, errored, steps }`. + +**What each step reports.** Raw `tool:` steps include the underlying tool's full `result` (screenshots and other outputs render as usual). The directive steps are summarized: `tap`/`type`/`await`/`assert` report only `status` + `reason`, and `snapshot` adds `artifacts` only when there is something to look at — a failed comparison (baseline/current/diff paths), a missing-baseline failure (`current` only), or a baseline write; a clean pass reports just `status` + `reason`. So converting a `tool: gesture-tap` into a `tap:` directive during cleanup drops only that tap's (uninteresting) raw result — output-bearing tools like `screenshot` have no directive form and stay `tool:` steps, so their results keep flowing through. -## 8. Flow File Format +## Flow file format -Flow files use YAML. The top-level is an object with `executionPrerequisite` (describes required state) and `steps` (array of actions): +The top-level is an object with `steps` (array) and — fragments only — `executionPrerequisite` (an e2e flow, one beginning with `launch:`, has none). Besides the directives above: -- `- echo: ` — a label -- `- tool: ` with optional `args:` — a tool call. A tool step may also carry `delayMs: ` to sleep that long before it runs. (`await-ui-element` is an ordinary tool step; see §4 and §10.5 for when to gate a transition with one.) +- `- echo: ` — a label printed during replay +- `- tool: ` with optional `args:` — a raw tool call. A tool step may also carry `delayMs: ` to sleep that long before it runs. (`await-ui-element` is an ordinary tool step; see _flow-add-step arguments_ and _Making flows resilient_ for when to gate a transition with one.) -Example `.yaml` file: +The polished result of the example session above: ```yaml -executionPrerequisite: Simulator booted with app installed steps: - - echo: Launch Settings app - - tool: launch-app - args: - udid: ABC - bundleId: com.apple.Preferences - - echo: Wait for the Settings list to render - - tool: await-ui-element - args: - udid: ABC - condition: visible - selector: - text: General - - echo: Tap General - - tool: gesture-tap - args: - udid: ABC - x: 0.5 - y: 0.35 - - echo: Tap About - - tool: gesture-tap - args: - udid: ABC - x: 0.5 - y: 0.17 + - echo: Start Settings from scratch + - launch: com.apple.Preferences + - echo: On the Settings root list, tapping the 'General' row + - tap: { text: General } + - await: { visible: About } + - echo: On Settings > General, tapping 'About' + - tap: { text: About } + - await: { visible: Model Name } ``` -## 9. When to Proactively Record a Flow +Note there is **no device id** anywhere in the file — the recorder strips them and the runner injects the bound device. + +## When to proactively record a flow You do not need the user to ask for a flow. Record one proactively when you recognize any of these patterns: @@ -148,28 +179,28 @@ You do not need the user to ask for a flow. Record one proactively when you reco - **Complex path discovered**: You worked through a non-trivial sequence of taps/swipes/navigation to reach a desired app state. Capture it before it is lost. - **User says "again" / "one more time"**: Any request to redo what you just did is a signal to record first, then replay. -## 10. Flow Self-Improvement +## Flow self-improvement Flows break. UI layouts change, coordinates drift, screens get added or removed. When `flow-execute` returns a failure, follow this procedure to diagnose and fix the flow instead of silently re-recording or giving up. -### 10.1 Classify the Result +### Classify the result After every `flow-execute`, classify the outcome before proceeding: | Outcome | Signal | Action | | ---------------------- | --------------------------------------------------------------------- | ------------------ | | **Success** | All steps completed, final screenshot shows expected state | Continue with task | -| **Hard error** | A step has `ERROR` in the result — engine stopped there | Enter §10.2 | -| **Silent misfire** | All steps completed but final screenshot shows wrong screen | Enter §10.2 | -| **Partial divergence** | Intermediate screenshot shows wrong state even though later steps ran | Enter §10.2 | +| **Hard error** | A step has `ERROR` in the result — engine stopped there | Enter **Diagnose** | +| **Silent misfire** | All steps completed but final screenshot shows wrong screen | Enter **Diagnose** | +| **Partial divergence** | Intermediate screenshot shows wrong state even though later steps ran | Enter **Diagnose** | -For silent misfires and partial divergence, echo annotations (§10.5) are your reference for what each screen _should_ look like. +For silent misfires and partial divergence, echo annotations (see _Making flows resilient_) are your reference for what each screen _should_ look like. -### 10.2 Diagnose +### Diagnose 1. Note the failure step index and error message (if hard error). 2. Call `screenshot` to see where the app actually is now. -3. Call `describe` or `debugger-component-tree` to get the current element tree. +3. Call `describe` or `debugger-component-tree` to get the current element tree. Remember `describe` shows less than the flow tree — a testID missing from its output can still resolve as a selector (see Selectors). 4. Compare current state to what the failed step expected. Classify the root cause: | Root cause | Symptoms | @@ -182,7 +213,7 @@ For silent misfires and partial divergence, echo annotations (§10.5) are your r 5. State the diagnosis in one sentence before attempting any correction. -### 10.3 Correct +### Correct Choose the lightest strategy that fits: @@ -190,7 +221,7 @@ Choose the lightest strategy that fits: Read `.argent/flows/.yaml`, update the broken step's `x`/`y`, `bundleId`, `text`, or other args. Re-run `flow-execute` to verify. **Strategy 2 — Manual recovery + continue** (timing/transient issues, one-off replay). -Manually execute the failed step with corrected coordinates from §10.2 discovery, then manually execute remaining steps. Does not fix the YAML — use only when re-recording is not worth it. +Manually execute the failed step with corrected coordinates from the Diagnose step, then manually execute remaining steps. Does not fix the YAML — use only when re-recording is not worth it. **Strategy 3 — Re-record from failure point** (structural changes, new intermediate screens). Navigate the app to the state just before the failure point. Call `flow-start-recording` with the same flow name (overwrites). Re-add the working prefix steps via `flow-add-step`, then continue recording new steps from the divergence point. Call `flow-finish-recording`. @@ -206,22 +237,22 @@ Reset the app to prerequisite state (`restart-app` + `launch-app`). Record from - 3+ steps broken, or unclear root cause → Strategy 4 - Flow used for profiling comparison (must be identical) → Strategy 4 -### 10.4 Verify and Bound Retries +### Verify and bound retries After applying a correction, re-run `flow-execute` to verify. - If it succeeds → done. Report what changed (e.g. "Fixed step 4: updated tap coordinates from 0.5,0.35 to 0.5,0.42"). -- If it fails at a **different** step → return to §10.2 for a second attempt. +- If it fails at a **different** step → return to Diagnose for a second attempt. - If this is already the second correction attempt → **stop**. Report the diagnosis to the user and recommend a full re-record or manual investigation. **Hard cap: 2 correction cycles.** Do not enter an unbounded fix loop. -### 10.5 Making Flows Resilient +### Making flows resilient Apply these when recording new flows to reduce future breakage: - **Echo expected state, not just actions.** Write `"On Settings > General screen, about to tap About"` not `"Tap About"`. During diagnosis these tell you what the screen _should_ look like. -- **Gate transitions with `await-ui-element`, not fixed delays.** After a tap that triggers a navigation, record an `await-ui-element` step that waits for the next screen's element to be `visible` (or a spinner to be `hidden`) before the following step. This removes the **Timing** failure mode in §10.2 (the element is in the tree but the tap fired before the screen settled) and is more reliable than `delayMs` or an extra `screenshot`. An unmet wait stops replay at that step, so a mistimed step can never run blind. +- **Gate transitions with `await-ui-element`, not fixed delays.** After a tap that triggers a navigation, record an `await-ui-element` step that waits for the next screen's element to be `visible` (or a spinner to be `hidden`) before the following step — converted to an `await:` directive during polish. This removes the **Timing** failure mode in Diagnose (the element is in the tree but the tap fired before the screen settled) and is more reliable than `delayMs` or an extra `screenshot`. An unmet wait stops replay at that step, so a mistimed step can never run blind. - **Add screenshot steps after critical navigation.** Insert `screenshot` steps after screen transitions. These produce images in the flow result you can inspect during diagnosis. - **Write specific executionPrerequisites.** `"App on home tab, user logged in, simulator UDID is "` — not `"App running"`. Verify with `screenshot` + `describe` before acknowledging. - **Prefer launch-app / open-url over navigation chains.** Deep links are more resilient to layout changes than tap sequences. diff --git a/packages/skills/skills/argent-device-interact/SKILL.md b/packages/skills/skills/argent-device-interact/SKILL.md index 1f73007c9..779a6ed56 100644 --- a/packages/skills/skills/argent-device-interact/SKILL.md +++ b/packages/skills/skills/argent-device-interact/SKILL.md @@ -188,8 +188,8 @@ Instead of polling `screenshot`/`describe` in a loop, use `await-ui-element` to ``` - `condition`: `exists`, `visible`, `hidden`, or `text`. -- `selector`: `{ text?, identifier?, role? }` — every provided field must match (case-insensitive substring). `text` matches the element's label or value; `identifier` matches its accessibility id / resource-id / testID; `role` matches its element role (e.g. `AXButton`, `button`, `TextView`, `StaticText`). The synthetic `ROOT` container `describe` prints is never matched, so a `role` like `AXGroup`/`html` won't trivially "match the screen". -- Prefer a **specific** selector. A loose substring can match several elements, and the tool may then key off one you didn't mean: `text` reads the **first** match in **reading order** (top-to-bottom, left-to-right — the same order `describe` lists them, so it's the one you saw first), while `visible`/`exists` are satisfied by **any** match. Disambiguate with a longer or more exact string, an `identifier`, or a `role` (e.g. pin to a text role like `StaticText` to skip a same-named button). On a `text` timeout the `note` quotes the matched element's text, so you can see which one it landed on. +- `selector`: `{ text?, identifier?, role? }` — every provided field must match. `text` matches the element's label or value and `role` its element role (e.g. `AXButton`, `button`, `TextView`, `StaticText`), both as case-insensitive substrings; `identifier` matches its accessibility id / resource-id / testID **exactly** (case-insensitive), also accepting the unqualified Android resource-id name (`submit` matches `com.example.app:id/submit`). The synthetic `ROOT` container `describe` prints is never matched, so a `role` like `AXGroup`/`html` won't trivially "match the screen". +- Prefer a **specific** selector. A loose substring can match several elements, and the tool may then key off one you didn't mean: `text` reads the first **visible** match in **reading order** (top-to-bottom, left-to-right — the same order `describe` lists them, so it's the one you saw first; when no match is visible, the first match overall), while `visible`/`exists` are satisfied by **any** match. Disambiguate with a longer or more exact string, an `identifier`, or a `role` (e.g. pin to a text role like `StaticText` to skip a same-named button). On a `text` timeout the `note` quotes the matched element's text, so you can see which one it landed on. - `text` condition also needs `expectedText` (substring the matched element must contain). - `hidden` treats a selector that matches **nothing** as already-hidden, so a typo'd selector returns an instant (false) success. Double-check the selector for `hidden` waits — the result `note` flags when the selector never matched any element. (On iOS, if the accessibility backend is down the tree comes back empty; the tool will **not** report `hidden` success off such a degraded read and the `note` surfaces the boot hint instead.) - Optional `timeoutMs` (default 5000) and `pollIntervalMs` (default 400). diff --git a/packages/tool-server/src/blueprints/android-devtools.ts b/packages/tool-server/src/blueprints/android-devtools.ts index 744cfc83b..02aa21fd2 100644 --- a/packages/tool-server/src/blueprints/android-devtools.ts +++ b/packages/tool-server/src/blueprints/android-devtools.ts @@ -37,6 +37,13 @@ export interface GetHierarchyOptions { waitForIdleMs?: number; maxDepth?: number; maxNodes?: number; + /** + * Drop the helper's accessibility-node cache before capturing — it can serve + * stale text (notably after the inspected app restarts under the helper's + * connection). Opt in when asserting on changing text; default keeps the + * cheaper cached read. + */ + clearCache?: boolean; } export interface HierarchyResult { @@ -332,6 +339,7 @@ export const androidDevtoolsBlueprint: ServiceBlueprint( err: unknown, ctor: new (...args: never[]) => T @@ -712,12 +729,31 @@ export function createHttpApp(registry: Registry, options?: HttpAppOptions): Htt } } + // Progress streaming, opted into per request: a client that accepts + // NDJSON gets each `ctx.emitProgress` event as its own line the moment + // the tool emits it, then a terminal `result` (or in-band `error`) line. + // The gates above still answer with their plain-JSON status codes — the + // response only commits to streaming here, after every gate has passed. + const wantsStream = Boolean(req.headers.accept?.includes("application/x-ndjson")); + const writeLine = (payload: unknown): void => { + res.write(`${JSON.stringify(payload)}\n`); + }; + if (wantsStream) { + res.writeHead(200, { + "Content-Type": "application/x-ndjson", + "Cache-Control": "no-cache", + }); + } + try { const data = await registry.invokeTool(name, parsedData, { signal: controller.signal, ...(resolvedFileInputs ? { fileInputs: resolvedFileInputs } : {}), toolInvocationId, ...(recordChildInvocation ? { recordChildInvocation } : {}), + ...(wantsStream + ? { emitProgress: (event: unknown) => writeLine({ event: "progress", data: event }) } + : {}), }); // Gate on `updateInstallable` (not `updateAvailable`) and advertise the // version the resolver would install — both account for the release-age policy. @@ -732,13 +768,21 @@ export function createHttpApp(registry: Registry, options?: HttpAppOptions): Htt // ignore } } - res.json({ - data, - ...(shouldNotify - ? { note: buildUpdateNote(currentVersion, installableVersion ?? "unknown") } - : {}), - }); + const notePayload = shouldNotify + ? { note: buildUpdateNote(currentVersion, installableVersion ?? "unknown") } + : {}; + if (wantsStream) { + writeLine({ event: "result", data, ...notePayload }); + res.end(); + } else { + res.json({ data, ...notePayload }); + } } catch (err: unknown) { + if (wantsStream) { + writeLine({ event: "error", error: streamErrorMessage(err) }); + res.end(); + return; + } if (err instanceof ToolNotFoundError) { res.status(404).json({ error: err.message }); return; diff --git a/packages/tool-server/src/tools/await-screen-idle/index.ts b/packages/tool-server/src/tools/await-screen-idle/index.ts index 7875144e3..4f30ca1bd 100644 --- a/packages/tool-server/src/tools/await-screen-idle/index.ts +++ b/packages/tool-server/src/tools/await-screen-idle/index.ts @@ -10,6 +10,7 @@ import type { import { chromiumCdpRef, type ChromiumCdpApi } from "../../blueprints/chromium-cdp"; import { resolveDevice } from "../../utils/device-info"; import { isTvOsSimulator } from "../../utils/ios-devices"; +import { isAndroidTv } from "../../utils/adb"; import { assertSupported } from "../../utils/capability"; import { ensureDeps } from "../../utils/check-deps"; import { pollDescribeTree } from "../../utils/poll-describe-tree"; @@ -99,13 +100,14 @@ export function createAwaitScreenIdleTool(registry: Registry): ToolDefinition, - isTvOs: boolean + isTvOs: boolean, + androidIsTv: boolean ): Promise { if (device.platform === "ios") { return describeIos(registry, device, {}, { isTvOs }); } if (device.platform === "android") { - return describeAndroid(registry, device.id); + return describeAndroid(registry, device.id, undefined, androidIsTv); } return describeChromium(services.chromium as ChromiumCdpApi); } @@ -136,14 +138,18 @@ still before the timeout. Use after a launch/navigation to wait for the UI to re if (device.platform === "ios") await ensureDeps(iosRequires); else if (device.platform === "android") await ensureDeps(androidRequires); + // Resolved once, outside the poll loop, like `isTvOs` — an unlisted + // serial's TV probe is never cached, so leaving it inside + // `describeAndroid` would spawn `adb devices` per poll. const isTvOs = device.platform === "ios" && (await isTvOsSimulator(device.id)); + const androidIsTv = device.platform === "android" && (await isAndroidTv(device.id)); const minStableMs = params.minStableMs ?? DEFAULT_MIN_STABLE_MS; let stableSignature: string | undefined; let stableSince = 0; const poll = await pollDescribeTree({ - fetchTree: () => fetchTree(device, services, isTvOs), + fetchTree: () => fetchTree(device, services, isTvOs, androidIsTv), timeoutMs: params.timeoutMs ?? DEFAULT_TIMEOUT_MS, pollIntervalMs: params.pollIntervalMs ?? DEFAULT_POLL_INTERVAL_MS, signal: ctx?.signal, diff --git a/packages/tool-server/src/tools/await-ui-element/index.ts b/packages/tool-server/src/tools/await-ui-element/index.ts index 864fa4037..21adb30c2 100644 --- a/packages/tool-server/src/tools/await-ui-element/index.ts +++ b/packages/tool-server/src/tools/await-ui-element/index.ts @@ -10,6 +10,7 @@ import type { import { chromiumCdpRef, type ChromiumCdpApi } from "../../blueprints/chromium-cdp"; import { resolveDevice } from "../../utils/device-info"; import { isTvOsSimulator } from "../../utils/ios-devices"; +import { isAndroidTv } from "../../utils/adb"; import { assertSupported } from "../../utils/capability"; import { ensureDeps } from "../../utils/check-deps"; import { pollDescribeTree } from "../../utils/poll-describe-tree"; @@ -17,6 +18,15 @@ import type { DescribeNode, DescribeTreeData } from "../describe/contract"; import { describeIos, iosRequires } from "../describe/platforms/ios"; import { describeAndroid, androidRequires } from "../describe/platforms/android"; import { describeChromium } from "../describe/platforms/chromium"; +import { describeVega, vegaRequires } from "../describe/platforms/vega"; +import { + selectorSchema, + nodeText, + findAll, + isVisible, + firstInReadingOrder, + evaluateCondition, +} from "../../utils/ui-tree-match"; // Tool id. Exported so run-sequence can both allow this tool and recognise its // result shape (it returns { success: false } instead of throwing on an unmet @@ -42,37 +52,6 @@ export function isUnmetUiWaitResult(tool: string, result: unknown): boolean { const DEFAULT_TIMEOUT_MS = 5000; const DEFAULT_POLL_INTERVAL_MS = 400; -// A selector locates a node in the accessibility / DOM tree returned by -// `describe`. Every provided field must match (logical AND); matching is a -// case-insensitive substring test so the agent doesn't need the exact label. -const selectorSchema = z - .object({ - text: z - .string() - .min(1) - .optional() - .describe("Case-insensitive substring of the element's visible label or value."), - identifier: z - .string() - .min(1) - .optional() - .describe( - "Case-insensitive substring of the element's identifier (accessibilityIdentifier / resource-id / testid)." - ), - role: z - .string() - .min(1) - .optional() - .describe( - "Case-insensitive substring of the element's role (e.g. AXButton, button, TextView)." - ), - }) - .refine((s) => Boolean(s.text || s.identifier || s.role), { - message: "selector needs at least one of text, identifier, or role", - }); - -type Selector = z.infer; - const zodSchema = z .object({ udid: z @@ -84,8 +63,9 @@ const zodSchema = z .describe( "What to wait for. `exists`: selector is anywhere in the tree. " + "`visible`: selector is present with a non-zero on-screen frame. `hidden`: selector is absent " + - "or zero-area. `text`: the first match in reading order (topmost) contains expectedText — if a loose " + - "selector hits several elements, only that topmost one is checked, so narrow it to target the intended element." + "or zero-area. `text`: the first visible match in reading order (topmost), falling back to the first match overall if none is visible, " + + "contains (or, with textMatch `equals`, exactly matches) " + + "expectedText — if a loose selector hits several elements, only that one is checked, so narrow it to target the intended element." ), selector: selectorSchema.describe("Element to match (text / identifier / role)."), expectedText: z @@ -93,7 +73,13 @@ const zodSchema = z .min(1) .optional() .describe( - "For condition `text`: case-insensitive substring the first matched element (topmost in reading order) must contain." + "For condition `text`: the string the first visible matched element (topmost in reading order; the first match overall if none is visible) must contain (default) or equal — see `textMatch`. Case-insensitive." + ), + textMatch: z + .enum(["contains", "equals"]) + .optional() + .describe( + "For condition `text`: how expectedText is compared. `contains` (default) is a case-insensitive substring; `equals` is a case-insensitive full-string match." ), bundleId: z .string() @@ -135,118 +121,17 @@ const capability: ToolCapability = { apple: { simulator: true, device: true }, android: { emulator: true, device: true, unknown: true }, chromium: { app: true }, + vega: { vvd: true }, }; // ── Tree matching ──────────────────────────────────────────────────────── +// The matching engine (matchNode, findAll, isVisible, firstInReadingOrder, …) +// lives in utils/ui-tree-match so the flow directives and recorder reuse the +// exact selector semantics. `evaluateMatches` is kept as a params-shaped wrapper +// for this tool and its tests. -function nodeText(node: DescribeNode): string { - return [node.label, node.value].filter(Boolean).join(" "); -} - -function includesCI(haystack: string | undefined, needle: string): boolean { - return Boolean(haystack) && haystack!.toLowerCase().includes(needle.toLowerCase()); -} - -function matchNode(node: DescribeNode, selector: Selector): boolean { - if (selector.text !== undefined) { - if (!includesCI(node.label, selector.text) && !includesCI(node.value, selector.text)) { - return false; - } - } - if (selector.identifier !== undefined && !includesCI(node.identifier, selector.identifier)) { - return false; - } - if (selector.role !== undefined && !includesCI(node.role, selector.role)) { - return false; - } - return true; -} - -function collectMatches(node: DescribeNode, selector: Selector, acc: DescribeNode[]): void { - if (matchNode(node, selector)) acc.push(node); - for (const child of node.children) collectMatches(child, selector, acc); -} - -// Every node matching the selector in the subtree, EXCLUDING `root` itself. -// -// `root` is the top-level container describe puts at the head of the tree. On -// iOS / Android it's a synthetic full-screen node (iOS `AXGroup`, Android -// `hierarchy`/`Screen`; frame `0,0,1,1`); on Chromium it's the REAL `` -// element (`describeChromium` walks `document.documentElement`), framed from -// `getBoundingClientRect` rather than a synthetic `0,0,1,1`. Whatever its frame, -// `describe` renders this node only as a non-selectable `ROOT` header line, never -// as a matchable element, and its frame always passes `isVisible`. Matching it -// would let a role selector that is a substring of the root role (e.g. -// `role:"AXGroup"`, also iOS's default role for untyped elements) satisfy -// `visible`/`exists` on any screen — including an empty AX tree — and make -// `hidden` impossible. So we skip it, walking `root.children` only — the one rule -// we share with format-tree. (Chromium side effect: the `` element's own -// id / aria-label / author role sit on this excluded root, so a selector -// targeting those attributes matches nothing there.) -// -// Past that root exclusion we do NOT mirror describe's rendered body: describe -// drops structural / unlabeled nodes through a content-and-role filter before -// printing, but `findAll` tests every remaining node. So a role- or -// identifier-only selector can match a container (e.g. an unlabeled `AXGroup`) -// that never appears in describe's output — keep that in mind when a -// `visible`/`exists` selector is broad. A substring selector can also match -// several real nodes, so conditions are evaluated across the whole set (see -// evaluateMatches). Exported for unit tests. -export function findAll(root: DescribeNode, selector: Selector): DescribeNode[] { - const acc: DescribeNode[] = []; - for (const child of root.children) collectMatches(child, selector, acc); - return acc; -} - -// describe prunes off-screen / zero-size nodes on Chromium and the compressed -// Android dump, and iOS AX only returns on-screen leaves — so a non-zero frame -// area is a cheap, reliable proxy for "visible". -function isVisible(node: DescribeNode): boolean { - return node.frame.width > 0 && node.frame.height > 0; -} - -// The describe tool renders iOS leaves sorted into reading order (top-to-bottom, -// then left-to-right) via format-tree's renderFlat, so the element the agent -// "sees first" is the one with the smallest (y, then x). The `text` verdict and -// the timeout note key off a single element, so pick that same one — otherwise -// they'd refer to whichever node DFS happened to reach first, which on iOS can -// differ from what the agent read at the top of describe. Returns undefined for -// an empty set. -function firstInReadingOrder(matches: DescribeNode[]): DescribeNode | undefined { - let best: DescribeNode | undefined; - for (const n of matches) { - if ( - best === undefined || - n.frame.y < best.frame.y || - (n.frame.y === best.frame.y && n.frame.x < best.frame.x) - ) { - best = n; - } - } - return best; -} - -// Evaluate the condition over ALL elements matching the selector, not just the -// first in tree order. `visible` holds if ANY match is on-screen; `hidden` only -// if NONE is — so a zero-area match that sorts before a visible one can't flip -// the verdict the wrong way. `text` deliberately inspects the first match in -// reading order: it asserts a specific element's content, and aggregating would -// hide which element the selector actually landed on. export function evaluateMatches(params: Params, matches: DescribeNode[]): boolean { - switch (params.condition) { - case "exists": - return matches.length > 0; - case "visible": - return matches.some(isVisible); - case "hidden": - return !matches.some(isVisible); - case "text": { - const first = firstInReadingOrder(matches); - return first !== undefined && includesCI(nodeText(first), params.expectedText!); - } - default: - return false; - } + return evaluateCondition(params.condition, params.expectedText, matches, params.textMatch); } // A degraded / blind read: the tree came back EMPTY and that emptiness is not @@ -295,9 +180,12 @@ function timeoutNote( let base: string; switch (params.condition) { case "text": { - const first = firstInReadingOrder(matches); + // Visible-first, mirroring evaluateCondition — the note must quote the + // same element the check read, or the two can contradict each other. + const first = firstInReadingOrder(matches.filter(isVisible)) ?? firstInReadingOrder(matches); + const wanted = params.textMatch === "equals" ? "equal" : "contain"; base = first - ? `element matched but its text was "${nodeText(first)}" (wanted to contain "${params.expectedText}")` + ? `element matched but its text was "${nodeText(first)}" (wanted to ${wanted} "${params.expectedText}")` : "no element matched the selector before timeout"; break; } @@ -329,13 +217,17 @@ export function createAwaitUiElementTool(registry: Registry): ToolDefinition, - isTvOs: boolean + isTvOs: boolean, + androidIsTv: boolean ): Promise { if (device.platform === "ios") { return describeIos(registry, device, { bundleId: params.bundleId }, { isTvOs }); } if (device.platform === "android") { - return describeAndroid(registry, device.id); + return describeAndroid(registry, device.id, undefined, androidIsTv); + } + if (device.platform === "vega") { + return describeVega(device.id); } return describeChromium(services.chromium as ChromiumCdpApi); } @@ -348,14 +240,18 @@ Conditions: exists — the selector matches an element anywhere in the tree. visible — the selector matches an element with a non-zero on-screen frame. hidden — the selector matches nothing, or only a zero-area element (e.g. a spinner that disappeared). - text — the FIRST match in reading order (topmost, then leftmost) contains expectedText (case-insensitive - substring). A loose selector can match several elements; only that topmost one is inspected, so if a - lower match is the one holding the text the wait still reports failure — narrow the selector to target it. - -The selector is { text?, identifier?, role? }; every provided field must match (case-insensitive substring). -text matches the element's label or value. It polls the same accessibility / DOM tree as \`describe\` -(iOS AXRuntime, Android uiautomator, Chromium CDP) every pollIntervalMs (default ${DEFAULT_POLL_INTERVAL_MS}ms) -until timeoutMs (default ${DEFAULT_TIMEOUT_MS}ms). + text — the first VISIBLE match in reading order (topmost, then leftmost; falling back to the first match + overall if none is visible) contains expectedText (case-insensitive substring), or exactly matches + it when textMatch is \`equals\`. A loose selector can match several elements; only that one is + inspected, so if a different match is the one holding the text the wait still reports failure — + narrow the selector to target it. + +The selector is { text?, identifier?, role? }; every provided field must match. text and role match as +case-insensitive substrings of the element's label/value and role; identifier matches exactly (case-insensitive), +also accepting the unqualified Android resource-id name ('submit' matches 'com.example.app:id/submit'). +It polls the same accessibility / DOM tree as \`describe\` +(iOS AXRuntime, Android uiautomator, Chromium CDP, Vega automation toolkit) every pollIntervalMs +(default ${DEFAULT_POLL_INTERVAL_MS}ms) until timeoutMs (default ${DEFAULT_TIMEOUT_MS}ms). Returns { success: boolean, elapsed: number } — success=false means the condition never held before the timeout (a \`note\` then explains what was seen). Use this after a tap/navigation to wait for the next screen, @@ -380,10 +276,14 @@ or before tapping an element that appears asynchronously.`, assertSupported(AWAIT_UI_ELEMENT_TOOL_ID, capability, device); if (device.platform === "ios") await ensureDeps(iosRequires); else if (device.platform === "android") await ensureDeps(androidRequires); + else if (device.platform === "vega") await ensureDeps(vegaRequires); // Resolve once, outside the poll loop — re-probing `xcrun` per fetch would - // blow the per-fetch budget for a fake UDID that never caches. + // blow the per-fetch budget for a fake UDID that never caches. Same for + // the Android TV probe: a serial that isn't listed is never cached, so + // leaving it inside `describeAndroid` would spawn `adb devices` per poll. const isTvOs = device.platform === "ios" && (await isTvOsSimulator(device.id)); + const androidIsTv = device.platform === "android" && (await isAndroidTv(device.id)); // Start the wait clock after setup so its fixed cost isn't charged against // timeoutMs (the deadline should bound polling, not device resolution). @@ -404,7 +304,7 @@ or before tapping an element that appears asynchronously.`, let everMatched = false; const poll = await pollDescribeTree({ - fetchTree: () => fetchTree(device, params, services, isTvOs), + fetchTree: () => fetchTree(device, params, services, isTvOs, androidIsTv), timeoutMs, pollIntervalMs, signal, diff --git a/packages/tool-server/src/tools/describe/contract.ts b/packages/tool-server/src/tools/describe/contract.ts index cae35e884..a61b4dc98 100644 --- a/packages/tool-server/src/tools/describe/contract.ts +++ b/packages/tool-server/src/tools/describe/contract.ts @@ -16,6 +16,14 @@ export interface DescribeNode { label?: string; identifier?: string; value?: string; + // Text hoisted up from descendant nodes during the flow adapters' flatten + // pass (see flow-ios-tree / flow-android-tree). The flat-leaves shape + // discards nesting, so a testID container's own `label`/`value` is empty even + // when it visibly wraps text (e.g. a counter whose number is a child `Text`). + // `subtreeText` carries that descendant text so an `assert`/`text` check can + // read "what this container shows" without the structure. Only the flow trees + // populate it; the agent-facing describe path leaves it unset. + subtreeText?: string; // Interactivity flags surfaced by the Android uiautomator dump. iOS // consumers leave these unset; adding them as optional avoids breaking // existing payloads. `scrollHidden` counts children that fell outside an @@ -46,6 +54,7 @@ export const describeNodeSchema: z.ZodType = z.lazy(() => label: z.string().optional(), identifier: z.string().optional(), value: z.string().optional(), + subtreeText: z.string().optional(), clickable: z.boolean().optional(), longClickable: z.boolean().optional(), scrollable: z.boolean().optional(), diff --git a/packages/tool-server/src/tools/describe/platforms/android/uiautomator-parser.ts b/packages/tool-server/src/tools/describe/platforms/android/uiautomator-parser.ts index 0fad33474..d773681ef 100644 --- a/packages/tool-server/src/tools/describe/platforms/android/uiautomator-parser.ts +++ b/packages/tool-server/src/tools/describe/platforms/android/uiautomator-parser.ts @@ -174,6 +174,11 @@ const NOISY_CLASSES = new Set([ "com.horcrux.svg.SvgView", ]); +/** Whether a raw class is decorative implementation detail to drop with its subtree. */ +export function isNoisyUiAutomatorClass(className: string): boolean { + return NOISY_CLASSES.has(className); +} + const SYSTEM_PACKAGES = new Set([ // Status bar / nav bar / quick settings — these exist on every dump but // rarely matter for app-level navigation. Note we deliberately do NOT drop @@ -200,6 +205,11 @@ const LAYOUT_CONTAINERS = new Set([ "android.view.View", ]); +/** Whether a raw class is hierarchy scaffolding rather than a role target. */ +export function isUiAutomatorLayoutContainer(className: string): boolean { + return !className || LAYOUT_CONTAINERS.has(className); +} + const SCROLL_CLASSES = new Set([ "android.widget.ScrollView", "android.widget.HorizontalScrollView", @@ -207,6 +217,17 @@ const SCROLL_CLASSES = new Set([ "android.widget.ListView", ]); +/** + * Whether a node scrolls its content — a known scroll class, or anything the + * framework marks `scrollable` (RN's ScrollView dumps as a scrollable + * ViewGroup). Such a node's bounds become the clip window for the scroll-clip + * prune. Shared with the flow tree adapter (`flow-android-tree`) so both trees + * agree on which containers clip. + */ +export function isUiAutomatorScrollable(attrs: Record): boolean { + return SCROLL_CLASSES.has(attrs.class ?? "") || attrIsTrue(attrs, "scrollable"); +} + const WEBVIEW_CLASSES = new Set(["android.webkit.WebView", "android.webkit.WebViewChromium"]); interface PixelRect { @@ -291,7 +312,16 @@ function rectsEqual(a: PixelRect, b: PixelRect): boolean { return a.x === b.x && a.y === b.y && a.w === b.w && a.h === b.h; } -function rectFullyOutside(kid: PixelRect, clip: PixelRect): boolean { +/** + * Whether a rect lies entirely outside a clip window — the scroll-clip test + * `pruneSubtree` applies to drop views a scrolling container has scrolled out + * of its viewport. Exported for the flow tree adapters (`flow-tree-flatten`), + * which must agree with the agent-facing describe on what "scrolled out" means. + */ +export function rectFullyOutside( + kid: { x: number; y: number; w: number; h: number }, + clip: { x: number; y: number; w: number; h: number } +): boolean { return ( kid.x + kid.w <= clip.x || kid.x >= clip.x + clip.w || @@ -400,9 +430,8 @@ function pruneSubtree(root: ParsedXmlNode, opts: PruneOptions): UiNode[] { if (!top.visited) { top.visited = true; const attrs = top.parsed.attrs; - const cls = attrs.class ?? ""; const myBounds = parseUiAutomatorBounds(attrs.bounds ?? ""); - const isScroll = SCROLL_CLASSES.has(cls) || attrIsTrue(attrs, "scrollable"); + const isScroll = isUiAutomatorScrollable(attrs); // Children inherit either MY bounds (if I'm a scroll) or whatever clip // I was handed. They'll filter their own kids against this rect. const childClip = isScroll && myBounds ? myBounds : top.scrollClip; diff --git a/packages/tool-server/src/tools/describe/platforms/chromium.ts b/packages/tool-server/src/tools/describe/platforms/chromium.ts index fc2a4b1fa..5942f5b8f 100644 --- a/packages/tool-server/src/tools/describe/platforms/chromium.ts +++ b/packages/tool-server/src/tools/describe/platforms/chromium.ts @@ -41,13 +41,27 @@ const DESCRIBE_FAILURE = { * (~50MB). Cap depth at 60 purely to bound recursion: modern React DOMs * (React Native Web, navigator/provider stacks) routinely nest 25+ levels * before reaching leaf text, so a shallower cap silently clips real content. + * Callers with a bigger appetite (the flow tree keeps more than the + * agent-facing describe, like Android's FLOW_MAX_NODES) can raise both via + * `limits`. */ -// Exported for test/describe-chromium-script.test.ts, which evals it against a mock -// DOM to lock in the visibility/pruning rules (the script runs in the renderer, so -// the rest of the suite can only mock its CDP response). -export const DESCRIBE_DOM_SCRIPT = `(() => { - const MAX_DEPTH = 60; - const MAX_NODES = 5000; +export interface ChromiumWalkLimits { + maxDepth: number; + maxNodes: number; +} + +const DEFAULT_WALK_LIMITS: ChromiumWalkLimits = { maxDepth: 60, maxNodes: 5000 }; + +// Interpolated into the in-page script: force a plain positive integer literal +// so no caller can ever smuggle text (or a cap-disabling NaN) into the +// renderer. Non-finite input falls back to the default. +function intForScript(value: number, fallback: number): number { + return Number.isFinite(value) ? Math.max(1, Math.floor(value)) : fallback; +} + +const buildDescribeDomScript = ({ maxDepth, maxNodes }: ChromiumWalkLimits) => `(() => { + const MAX_DEPTH = ${intForScript(maxDepth, DEFAULT_WALK_LIMITS.maxDepth)}; + const MAX_NODES = ${intForScript(maxNodes, DEFAULT_WALK_LIMITS.maxNodes)}; let nodeBudget = MAX_NODES; let truncated = false; const w = window.innerWidth; @@ -84,6 +98,15 @@ export const DESCRIBE_DOM_SCRIPT = `(() => { const getAttr = Element.prototype.getAttribute; const hasAttr = Element.prototype.hasAttribute; const getBCR = Element.prototype.getBoundingClientRect; + // Document's named getter is [LegacyOverrideBuiltIns] (like a form's): a + //
shadows document.activeElement, so both focus + // reads go through prototype accessors. The typeof guard covers the test + // harness's mock DOM, which defines no Document constructor — protoGetter's + // direct-read fallback then degrades focus reporting instead of throwing. + const docProto = typeof Document === "undefined" ? {} : Document.prototype; + const getOwnerDocument = protoGetter(Node.prototype, "ownerDocument"); + const getActiveElement = protoGetter(docProto, "activeElement"); + const getDocBody = protoGetter(docProto, "body"); function nodeRole(el) { const r = getAttr.call(el, "role"); @@ -110,7 +133,10 @@ export const DESCRIBE_DOM_SCRIPT = `(() => { } if (el instanceof HTMLInputElement || el instanceof HTMLTextAreaElement) { if (el.placeholder) return el.placeholder.slice(0, 200); - if (el.value) return el.value.slice(0, 200); + // Never fall through to a password input's value: with no aria label or + // placeholder (a floating/uncontrolled-label pattern) the typed secret + // would become the node's label and reach every describe consumer. + if (el.value && !isPassword(el)) return el.value.slice(0, 200); } if (el instanceof HTMLImageElement && el.alt) return el.alt.slice(0, 200); // getAttribute, not el.title: a with a control named "title" clobbers the @@ -167,8 +193,7 @@ export const DESCRIBE_DOM_SCRIPT = `(() => { return el instanceof HTMLInputElement && el.type === "password"; } - function isScrollable(el) { - const style = window.getComputedStyle(el); + function isScrollable(el, style) { const oy = style.overflowY; const ox = style.overflowX; if (oy === "auto" || oy === "scroll" || ox === "auto" || ox === "scroll") { @@ -401,8 +426,12 @@ export const DESCRIBE_DOM_SCRIPT = `(() => { selfFrame = frame(el); } - // Prune structural wrappers with no info that just add a layer. - // Keep them if they're roots/clickable/named/identified/have text. + const scrollable = isScrollable(el, style); + // Prune structural wrappers with no info that just add a layer. Keep them + // if they're roots/clickable/named/identified/have text — or scroll their + // content: an RN-web ScrollView renders as exactly this shape (a scroller + // div wrapping a single content-container div), and pruning it would drop + // the node scroll gestures and flow 'within' selectors anchor to. if ( depth > 0 && childResults.length === 1 && @@ -410,6 +439,7 @@ export const DESCRIBE_DOM_SCRIPT = `(() => { !name && !text && !id && + !scrollable && role === "div" ) { return childResults[0]; @@ -426,7 +456,25 @@ export const DESCRIBE_DOM_SCRIPT = `(() => { if (isDisabled(el)) node.disabled = true; if (isChecked(el)) node.checked = true; if (isPassword(el)) node.password = true; - if (isScrollable(el)) node.scrollable = true; + if (scrollable) node.scrollable = true; + // Input focus: el is its document's activeElement. Deliberately emitted to + // EVERY describe consumer (the agent-facing tool as much as the flow type + // directive's focus wait) — where the caret is is useful targeting info. + // The body is excluded — it's the default holder when nothing is focused, + // and its screen-spanning frame would satisfy any overlap check. A host + //