From b130deac23af0f2bfdf64e7ea24779721d5510e4 Mon Sep 17 00:00:00 2001 From: Goran Gajic Date: Mon, 7 Sep 2026 15:00:42 +0200 Subject: [PATCH 1/5] feat(runner): write a screenshot to stdout with --out - `qawolf runner screenshot` only wrote a file, so a caller that wants the bytes in a process reserved a path, ran the command, read the file back and deleted it on every computer-use step. `--out -` writes the decoded JPEG to stdout instead, the same `-` that reads stdin for act and exec. Stdout then carries the image alone: the confirmation, and the JSON line under --json, moves to stderr so nothing follows the bytes into a reader that takes stdout as the file. --- .changeset/runner-screenshot-stdout.md | 5 + skills/qawolf-cli/SKILL.md | 2 +- src/commands/__snapshots__/help.test.ts.snap | 11 +- src/commands/runner/interact.register.ts | 14 ++- .../messages/interactiveRunner/interact.ts | 4 + .../interactiveRunner/deps.testUtils.ts | 30 +++++- .../takeScreenshot.stdout.test.ts | 101 ++++++++++++++++++ .../interactiveRunner/takeScreenshot.test.ts | 1 + .../interactiveRunner/takeScreenshot.ts | 65 +++++++---- .../interactiveRunner/writeScreenshot.test.ts | 85 ++++++++++++++- .../interactiveRunner/writeScreenshot.ts | 41 ++++++- 11 files changed, 324 insertions(+), 35 deletions(-) create mode 100644 .changeset/runner-screenshot-stdout.md create mode 100644 src/domains/interactiveRunner/takeScreenshot.stdout.test.ts diff --git a/.changeset/runner-screenshot-stdout.md b/.changeset/runner-screenshot-stdout.md new file mode 100644 index 000000000..ae831d504 --- /dev/null +++ b/.changeset/runner-screenshot-stdout.md @@ -0,0 +1,5 @@ +--- +"@qawolf/cli": minor +--- + +`qawolf runner screenshot --out -` writes the JPEG bytes to stdout instead of a file, so a caller that is a process reads the image off the pipe rather than reserving a temp file, running the command, reading it back and deleting it. Stdout carries the image alone: the confirmation, and the JSON line under `--json`, goes to stderr. diff --git a/skills/qawolf-cli/SKILL.md b/skills/qawolf-cli/SKILL.md index 03d3faa92..56dbbab7e 100644 --- a/skills/qawolf-cli/SKILL.md +++ b/skills/qawolf-cli/SKILL.md @@ -174,7 +174,7 @@ that `url`; never guess a route and never send a repository link in its place. | `qawolf runner list` | read | List the runners this directory holds that are still running | | `qawolf runner promote-snapshot` | write | Accept a run's screenshot as the new baseline for an image diff, on the runner that produced it | | `qawolf runner run` | write | Run a flow on an interactive runner, shipping the flow and what it imports | -| `qawolf runner screenshot` | read | Save a JPEG of an interactive runner's screen to a file | +| `qawolf runner screenshot` | read | Save a JPEG of an interactive runner's screen to a file, or write it to stdout with --out - | | `qawolf runner stop-run` | write | Stop what a runner is currently executing, leaving the runner up | | `qawolf runner terminate` | write | End an interactive runner, and the pod it runs on with it | | `qawolf tag create` | write | Create a tag on the caller's team. Tags select flows in run.create. | diff --git a/src/commands/__snapshots__/help.test.ts.snap b/src/commands/__snapshots__/help.test.ts.snap index 7f4099952..21d371144 100644 --- a/src/commands/__snapshots__/help.test.ts.snap +++ b/src/commands/__snapshots__/help.test.ts.snap @@ -296,7 +296,7 @@ Commands: keepalive [options] Reset a runner's inactivity clock, for a caller that pauses between actions run [options] Run a flow on an interactive runner, shipping the flow and what it imports events [options] Print a runner's journal, one entry per line. QA Wolf writes console, recorder, run-events, run-logs, run-status - screenshot [options] Save a JPEG of an interactive runner's screen to a file + screenshot [options] Save a JPEG of an interactive runner's screen to a file, or write it to stdout with --out - act [options] Perform one raw action on a runner's screen: click, double_click, scroll, move, drag, keypress, navigate or type. Use - to read a whole action as JSON from stdin. On a mobile runner only click (button left), drag and type have a touchscreen equivalent; the rest answer action-not-supported-on-mobile exec [options] Evaluate a snippet against a runner's live page. Use - to read the snippet from stdin inspect Read one thing off a runner's live page (browser) or Appium session (mobile) @@ -438,10 +438,13 @@ Examples: exports[`--help output qawolf runner screenshot 1`] = ` "Usage: qawolf runner screenshot [options] -Save a JPEG of an interactive runner's screen to a file +Save a JPEG of an interactive runner's screen to a file, or write it to stdout +with --out - Options: - --out File to write the image to (default: "screenshot.jpg") + --out File to write the image to. - writes the JPEG bytes to stdout + on their own and moves the confirmation, JSON included, to + stderr (default: "screenshot.jpg") --runner Runner to target. Defaults to QAWOLF_RUNNER_ID, then this directory's stored runner -h, --help display help for command @@ -449,6 +452,8 @@ Options: Examples: $ qawolf runner screenshot $ qawolf runner screenshot --out screens/step-3.jpg + $ qawolf runner screenshot --out - > step-3.jpg + $ qawolf runner screenshot --out - | my-vision-tool " `; diff --git a/src/commands/runner/interact.register.ts b/src/commands/runner/interact.register.ts index 73b6ec52c..0d5b54e30 100644 --- a/src/commands/runner/interact.register.ts +++ b/src/commands/runner/interact.register.ts @@ -16,7 +16,9 @@ const defaultScreenshotPath = "screenshot.jpg"; const screenshotExamples = ` Examples: $ qawolf runner screenshot - $ qawolf runner screenshot --out screens/step-3.jpg`; + $ qawolf runner screenshot --out screens/step-3.jpg + $ qawolf runner screenshot --out - > step-3.jpg + $ qawolf runner screenshot --out - | my-vision-tool`; const actExamples = ` Examples: @@ -51,8 +53,14 @@ export function registerRunnerInteractCommands( signals: SignalRegistry, ): void { declareCommandKind(runner.command("screenshot"), "read") - .description("Save a JPEG of an interactive runner's screen to a file") - .option("--out ", "File to write the image to", defaultScreenshotPath) + .description( + "Save a JPEG of an interactive runner's screen to a file, or write it to stdout with --out -", + ) + .option( + "--out ", + "File to write the image to. - writes the JPEG bytes to stdout on their own and moves the confirmation, JSON included, to stderr", + defaultScreenshotPath, + ) .option("--runner ", runnerFlagDescription) .addHelpText("after", screenshotExamples) .action((opts: { out: string; runner?: string }, command: Command) => diff --git a/src/core/messages/interactiveRunner/interact.ts b/src/core/messages/interactiveRunner/interact.ts index ee5a62ebc..dcaa7a882 100644 --- a/src/core/messages/interactiveRunner/interact.ts +++ b/src/core/messages/interactiveRunner/interact.ts @@ -34,9 +34,13 @@ export const interactMessages = { "The runner has a screen and cannot serve this yet. Its virtual desktop restarts when a run changes the display size, and it serves one request at a time, so something already in flight is the usual reason. Retry in a second or two.", screenshotNotAnImage: "The screen was captured but did not arrive as a JPEG, so nothing was written. Nothing about the command needs changing: try it again, and report it if it keeps happening.", + screenshotStdoutUnwritable: (detail: string) => + `The screen was captured but could not be written to stdout: ${detail}. Keep the pipe reading stdout open, or give --out a file path instead of "-".`, screenshotUnwritable: (path: string, detail: string) => `The screen was captured but could not be written to "${path}": ${detail}. Give --out a path this process can write to.`, screenshotWritten: (path: string) => `Wrote the runner's screen to ${path}.`, + screenshotWrittenToStdout: + "Wrote the runner's screen to stdout as a JPEG. Stdout holds the image bytes alone; this line, and the JSON with --json, is on stderr.", snippetEmpty: (path: string) => `"${path}" holds no code to evaluate.`, snippetErrored: (errorMessage: string | undefined) => errorMessage === undefined diff --git a/src/domains/interactiveRunner/deps.testUtils.ts b/src/domains/interactiveRunner/deps.testUtils.ts index fdbba422c..cbb927bf9 100644 --- a/src/domains/interactiveRunner/deps.testUtils.ts +++ b/src/domains/interactiveRunner/deps.testUtils.ts @@ -5,7 +5,10 @@ import type { RunFiles } from "@qawolf/api-contracts/v1"; import { makeCtx } from "~/shell/commandContext.testUtils.js"; import type { Fs } from "~/shell/fs.js"; import { makeMemoryFs } from "~/shell/fs.testUtils.js"; -import { writeScreenshot } from "~/shell/interactiveRunner/writeScreenshot.js"; +import { + type ScreenshotStdout, + writeScreenshot, +} from "~/shell/interactiveRunner/writeScreenshot.js"; import { makeCallPublicApiMock, makeMockPlatformClient, @@ -29,6 +32,7 @@ export function makeAuthCtx(mode: OutputMode = "human"): { outputs: () => { data: unknown; humanMessage: string }[]; streamed: () => string[]; streamedData: () => unknown[]; + successes: () => string[]; warnings: () => string[]; } { const callPublicApi = makeCallPublicApiMock(); @@ -50,6 +54,10 @@ export function makeAuthCtx(mode: OutputMode = "human"): { ).mock.calls.map(([data, humanMessage]) => ({ data, humanMessage })), streamed: () => streamCalls(base).map(([, line]) => line), streamedData: () => streamCalls(base).map(([data]) => data), + successes: () => + (base.ui.success as Mock<(message: string) => void>).mock.calls.map( + ([message]) => message, + ), warnings: () => (base.ui.warn as Mock<(message: string) => void>).mock.calls.map( ([message]) => message, @@ -67,7 +75,11 @@ export type WrittenScreenshot = { bytes: Uint8Array; path: string }; export function makeTestDeps( overrides: Partial = {}, -): InteractiveRunnerDeps & { written: WrittenScreenshot[] } { +): InteractiveRunnerDeps & { + /** Every chunk handed to stdout, in order. */ + stdoutWrites: Uint8Array[]; + written: WrittenScreenshot[]; +} { const files: RunFiles = { "flow.ts": "export default {};", "package.json": "{}", @@ -85,6 +97,13 @@ export function makeTestDeps( written.push({ bytes: Uint8Array.from(data as Uint8Array), path }); }, }; + const stdoutWrites: Uint8Array[] = []; + const recordingStdout: ScreenshotStdout = { + write(chunk, callback) { + stdoutWrites.push(Uint8Array.from(chunk)); + callback(); + }, + }; return { collectRunFiles: async () => ({ files, unresolvedImports: [] }), cwd: testCwd, @@ -105,8 +124,13 @@ export function makeTestDeps( }), sleep: async () => {}, store: makeRunnerStore({ cwd: testCwd, fs: makeMemoryFs() }), + stdoutWrites, writeScreenshot: (screenshot) => - writeScreenshot({ ...screenshot, fs: recordingFs }), + writeScreenshot({ + ...screenshot, + fs: recordingFs, + stdout: recordingStdout, + }), written, ...overrides, }; diff --git a/src/domains/interactiveRunner/takeScreenshot.stdout.test.ts b/src/domains/interactiveRunner/takeScreenshot.stdout.test.ts new file mode 100644 index 000000000..ff7b01b2f --- /dev/null +++ b/src/domains/interactiveRunner/takeScreenshot.stdout.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, it } from "bun:test"; + +import { handleRunnerScreenshot } from "./takeScreenshot.js"; +import { makeAuthCtx, makeTestDeps } from "./deps.testUtils.js"; + +const jpegBytes = Uint8Array.from([0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10]); +const imageJpegBase64 = Buffer.from(jpegBytes).toString("base64"); + +describe("handleRunnerScreenshot --out -", () => { + // The Tester session reads the bytes off the pipe instead of reserving a + // file, handing it to the sandboxed user, reading it back and deleting it. + it("writes the decoded image bytes to stdout and no file", async () => { + const { callPublicApi, ctx } = makeAuthCtx(); + callPublicApi.mockResolvedValue({ + ok: true, + value: { imageJpegBase64, outcome: "success" }, + }); + const deps = makeTestDeps(); + + const result = await handleRunnerScreenshot( + ctx, + { out: "-", runner: "ci" }, + deps, + ); + + expect(result).toBeUndefined(); + expect(deps.stdoutWrites).toEqual([jpegBytes]); + expect(deps.written).toEqual([]); + }); + + // Stdout is the image, so nothing else may land there: in json mode the + // answer line would otherwise follow the JPEG bytes into the reader's file. + // These two are the modes a piped stdout lands in; a terminal is human mode + // and has no reader to protect. + for (const mode of ["json", "agent"] as const) { + it(`keeps the confirmation off stdout in ${mode} mode`, async () => { + const { callPublicApi, ctx, outputs, streamed, successes } = + makeAuthCtx(mode); + callPublicApi.mockResolvedValue({ + ok: true, + value: { imageJpegBase64, outcome: "success" }, + }); + + await handleRunnerScreenshot( + ctx, + { out: "-", runner: "ci" }, + makeTestDeps(), + ); + + expect(outputs()).toEqual([]); + expect(streamed()).toEqual([]); + expect(successes()).toHaveLength(1); + expect(successes()[0]).toContain("stdout"); + expect(successes()[0]).toContain("stderr"); + }); + } + + it("reports a pipe that closed, naming stdout rather than a file", async () => { + const { callPublicApi, ctx } = makeAuthCtx(); + callPublicApi.mockResolvedValue({ + ok: true, + value: { imageJpegBase64, outcome: "success" }, + }); + + const result = await handleRunnerScreenshot( + ctx, + { out: "-", runner: "ci" }, + makeTestDeps({ + writeScreenshot: async () => ({ + detail: "EPIPE: broken pipe", + ok: false, + reason: "unwritable", + }), + }), + ); + + expect(result?.error).toContain("stdout"); + expect(result?.error).toContain("EPIPE"); + expect(result?.error).not.toContain('to "-"'); + expect(result?.exitCode).toBe(2); + }); + + it("writes nothing to stdout when the answer was not an image", async () => { + const { callPublicApi, ctx, successes } = makeAuthCtx(); + callPublicApi.mockResolvedValue({ + ok: true, + value: { imageJpegBase64: "", outcome: "success" }, + }); + const deps = makeTestDeps(); + + const result = await handleRunnerScreenshot( + ctx, + { out: "-", runner: "ci" }, + deps, + ); + + expect(result?.error).toContain("did not arrive as a JPEG"); + expect(deps.stdoutWrites).toEqual([]); + expect(successes()).toEqual([]); + }); +}); diff --git a/src/domains/interactiveRunner/takeScreenshot.test.ts b/src/domains/interactiveRunner/takeScreenshot.test.ts index 6ff82f9e1..9a80b0e60 100644 --- a/src/domains/interactiveRunner/takeScreenshot.test.ts +++ b/src/domains/interactiveRunner/takeScreenshot.test.ts @@ -33,6 +33,7 @@ describe("handleRunnerScreenshot", () => { runnerCallOptions, ); expect(deps.written).toEqual([{ bytes: jpegBytes, path: "shot.jpg" }]); + expect(deps.stdoutWrites).toEqual([]); }); it("says where it wrote the image", async () => { diff --git a/src/domains/interactiveRunner/takeScreenshot.ts b/src/domains/interactiveRunner/takeScreenshot.ts index fbcead3f0..2250db224 100644 --- a/src/domains/interactiveRunner/takeScreenshot.ts +++ b/src/domains/interactiveRunner/takeScreenshot.ts @@ -6,6 +6,10 @@ import type { CommandResult, } from "~/shell/commandContext.js"; import { exitCodes } from "~/shell/exit.js"; +import { + type ScreenshotWrite, + stdoutPath, +} from "~/shell/interactiveRunner/writeScreenshot.js"; import { failureFields } from "~/shell/platform/requestWithRetry.js"; import type { InteractiveRunnerDeps } from "./deps.js"; @@ -13,11 +17,18 @@ import { resolveRunner } from "./resolveRunner.js"; import { runnerCallOptions } from "./runnerCallOptions.js"; /** - * Takes one screenshot and writes it to a file. + * Takes one screenshot and writes it to a file, or to stdout when `out` is `-`. * - * A file, because that is what a foreign harness can read: every coding agent can - * open an image on disk and none can read a base64 field out of a JSON answer. - * The bytes are decoded on the way (see `writeScreenshot`). + * A file by default, because that is what a foreign harness can read: every + * coding agent can open an image on disk and none can read a base64 field out of + * a JSON answer. Stdout for the caller that is a process rather than an agent, + * which would otherwise reserve a path, run the command, read the file back and + * delete it on every step. The bytes are decoded either way (see + * `writeScreenshot`). + * + * With stdout taken by the image, the confirmation moves to stderr in every + * mode, JSON included: a JSON line after the JPEG bytes would corrupt the image + * for any reader that takes stdout as the file. * * The four non-image answers are kept apart at the terminal and in `--json`, * because each implies a different next move and only one of them is retrying. @@ -61,24 +72,15 @@ export async function handleRunnerScreenshot( }); // A payload that is not an image is the API's to fix, not the caller's; // a path that cannot be written is the other way round. - if (!written.ok) { - return written.reason === "not-a-jpeg" - ? { - error: interactiveRunnerMessages.screenshotNotAnImage, - exitCode: exitCodes.network, - } - : { - error: interactiveRunnerMessages.screenshotUnwritable( - options.out, - written.detail, - ), - exitCode: exitCodes.invalidArgs, - }; + if (!written.ok) return describeUnwritten(written, options.out); + if (options.out === stdoutPath) { + ctx.ui.success(interactiveRunnerMessages.screenshotWrittenToStdout); + } else { + ctx.ui.output( + { outcome: "success", path: options.out }, + interactiveRunnerMessages.screenshotWritten(options.out), + ); } - ctx.ui.output( - { outcome: "success", path: options.out }, - interactiveRunnerMessages.screenshotWritten(options.out), - ); return undefined; } @@ -119,3 +121,24 @@ export async function handleRunnerScreenshot( } } } + +// A payload that is not an image is the API's to fix, not the caller's; a +// destination that cannot be written is the other way round. +function describeUnwritten( + written: Exclude, + out: string, +): Exclude { + if (written.reason === "not-a-jpeg") { + return { + error: interactiveRunnerMessages.screenshotNotAnImage, + exitCode: exitCodes.network, + }; + } + return { + error: + out === stdoutPath + ? interactiveRunnerMessages.screenshotStdoutUnwritable(written.detail) + : interactiveRunnerMessages.screenshotUnwritable(out, written.detail), + exitCode: exitCodes.invalidArgs, + }; +} diff --git a/src/shell/interactiveRunner/writeScreenshot.test.ts b/src/shell/interactiveRunner/writeScreenshot.test.ts index 83afc7893..1ee5ce2ed 100644 --- a/src/shell/interactiveRunner/writeScreenshot.test.ts +++ b/src/shell/interactiveRunner/writeScreenshot.test.ts @@ -3,11 +3,32 @@ import { describe, expect, it } from "bun:test"; import type { Fs } from "~/shell/fs.js"; import { makeMemoryFs } from "~/shell/fs.testUtils.js"; -import { writeScreenshot } from "./writeScreenshot.js"; +import { type ScreenshotStdout, writeScreenshot } from "./writeScreenshot.js"; const jpegBytes = Uint8Array.from([0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10]); const imageJpegBase64 = Buffer.from(jpegBytes).toString("base64"); +/** Records what reached stdout, and lets a test close the pipe. */ +function makeRecordingStdout(failWith?: Error): { + stdout: ScreenshotStdout; + chunks: Uint8Array[]; +} { + const chunks: Uint8Array[] = []; + return { + chunks, + stdout: { + write(chunk, callback) { + if (failWith) { + callback(failWith); + return; + } + chunks.push(Uint8Array.from(chunk)); + callback(); + }, + }, + }; +} + /** Records what reached the filesystem, which is the only thing worth asserting. */ function makeRecordingFs(): { fs: Fs; @@ -37,6 +58,7 @@ describe("writeScreenshot", () => { fs, imageJpegBase64, path: "shot.jpg", + stdout: makeRecordingStdout().stdout, }); expect(result).toEqual({ ok: true }); @@ -52,6 +74,7 @@ describe("writeScreenshot", () => { fs, imageJpegBase64, path: "screens/step-3.jpg", + stdout: makeRecordingStdout().stdout, }); expect(result).toEqual({ ok: true }); @@ -79,6 +102,7 @@ describe("writeScreenshot", () => { fs: refusing, imageJpegBase64, path: "shot.jpg", + stdout: makeRecordingStdout().stdout, }); expect(result).toEqual({ @@ -99,15 +123,74 @@ describe("writeScreenshot", () => { for (const [name, payload] of Object.entries(notJpeg)) { it(`refuses ${name}, writing nothing`, async () => { const { fs, writes } = makeRecordingFs(); + const { chunks, stdout } = makeRecordingStdout(); const result = await writeScreenshot({ fs, imageJpegBase64: payload, path: "shot.jpg", + stdout, + }); + + expect(result).toEqual({ ok: false, reason: "not-a-jpeg" }); + expect(writes).toEqual([]); + expect(chunks).toEqual([]); + }); + + it(`refuses ${name} for stdout too, writing nothing`, async () => { + const { fs, writes } = makeRecordingFs(); + const { chunks, stdout } = makeRecordingStdout(); + + const result = await writeScreenshot({ + fs, + imageJpegBase64: payload, + path: "-", + stdout, }); expect(result).toEqual({ ok: false, reason: "not-a-jpeg" }); expect(writes).toEqual([]); + expect(chunks).toEqual([]); }); } + + describe("to stdout", () => { + // The same trap as the file: the bytes go out, not the base64 text. + it("writes decoded image bytes to stdout and nothing to the filesystem", async () => { + const { fs, writes } = makeRecordingFs(); + const { chunks, stdout } = makeRecordingStdout(); + + const result = await writeScreenshot({ + fs, + imageJpegBase64, + path: "-", + stdout, + }); + + expect(result).toEqual({ ok: true }); + expect(chunks).toEqual([jpegBytes]); + expect(writes).toEqual([]); + expect(await fs.pathExists("-")).toBe(false); + }); + + // A reader that went away is an unwritable destination, answered before the + // command claims success rather than as an EPIPE after it. + it("reports a pipe that would not take the bytes", async () => { + const { fs } = makeRecordingFs(); + const { stdout } = makeRecordingStdout(new Error("EPIPE: broken pipe")); + + const result = await writeScreenshot({ + fs, + imageJpegBase64, + path: "-", + stdout, + }); + + expect(result).toEqual({ + detail: "EPIPE: broken pipe", + ok: false, + reason: "unwritable", + }); + }); + }); }); diff --git a/src/shell/interactiveRunner/writeScreenshot.ts b/src/shell/interactiveRunner/writeScreenshot.ts index 1842857a4..5d55f8f54 100644 --- a/src/shell/interactiveRunner/writeScreenshot.ts +++ b/src/shell/interactiveRunner/writeScreenshot.ts @@ -12,13 +12,26 @@ import type { Fs } from "~/shell/fs.js"; */ const jpegStartOfImage = [0xff, 0xd8, 0xff]; +/** + * The `--out` value that sends the image to stdout instead of a file, the same + * `-` that reads stdin for `act` and `exec`. A caller that wants the bytes in a + * process reads them off the pipe rather than reserving a path, running the + * command, reading the file back and deleting it. + */ +export const stdoutPath = "-"; + +/** The one stdout method the writer needs, so a test can stand in for it. */ +export type ScreenshotStdout = { + write(chunk: Uint8Array, callback: (error?: Error | null) => void): unknown; +}; + export type ScreenshotWrite = | { ok: true } | { ok: false; reason: "not-a-jpeg" } | { ok: false; detail: string; reason: "unwritable" }; /** - * Writes a screenshot the API answered with to a file. + * Writes a screenshot the API answered with to a file, or to stdout. * * The decode is the whole job. The contract carries the image as base64 because * every answer on the API is JSON, so a caller that writes what it received @@ -29,21 +42,43 @@ export type ScreenshotWrite = * * The parent directory is created because `--out` is how a caller files its * screenshots, and a run of them into `screens/` should not need a mkdir first. + * + * Stdout is awaited rather than fire-and-forget: on a pipe the write is + * asynchronous on some platforms, and a closed reader has to be answered as an + * unwritable destination rather than lost in an EPIPE after the command reported + * success. */ export async function writeScreenshot(options: { fs: Fs; imageJpegBase64: string; path: string; + stdout?: ScreenshotStdout; }): Promise { const bytes = Buffer.from(options.imageJpegBase64, "base64"); if (jpegStartOfImage.some((byte, index) => bytes[index] !== byte)) { return { ok: false, reason: "not-a-jpeg" }; } try { - await options.fs.mkdir(dirname(options.path), { recursive: true }); - await options.fs.writeFile(options.path, bytes); + if (options.path === stdoutPath) { + await writeToStdout(options.stdout ?? process.stdout, bytes); + } else { + await options.fs.mkdir(dirname(options.path), { recursive: true }); + await options.fs.writeFile(options.path, bytes); + } } catch (error) { return { detail: errorMessage(error), ok: false, reason: "unwritable" }; } return { ok: true }; } + +function writeToStdout( + stdout: ScreenshotStdout, + bytes: Uint8Array, +): Promise { + return new Promise((resolve, reject) => { + stdout.write(bytes, (error) => { + if (error) reject(error); + else resolve(); + }); + }); +} From 329f313895b441d32de96028ae1629ab7e0451f6 Mon Sep 17 00:00:00 2001 From: Goran Gajic Date: Mon, 7 Sep 2026 15:00:50 +0200 Subject: [PATCH 2/5] docs(runner): describe screenshot --out - in the runner guide --- skills/qawolf-cli/references/runner.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/skills/qawolf-cli/references/runner.md b/skills/qawolf-cli/references/runner.md index 978a5a74e..8f60f57d8 100644 --- a/skills/qawolf-cli/references/runner.md +++ b/skills/qawolf-cli/references/runner.md @@ -127,7 +127,9 @@ vision loop on this surface. `qawolf runner screenshot --out page.jpg` writes a real JPEG to disk, decoded, because every coding harness can open an image file. Read it with whatever -vision you have. +vision you have. `--out -` writes the JPEG bytes to stdout instead, on their own, +for a caller that is a process rather than an agent: the confirmation, and the +JSON line under `--json`, goes to stderr so nothing follows the image on stdout. `qawolf runner act ` performs exactly one action per call, in the computer-use tool vocabulary a vision model already emits: `click`, From b5032750de57588dee9806064d8b5b253d08e820 Mon Sep 17 00:00:00 2001 From: Goran Gajic Date: Mon, 7 Sep 2026 16:41:50 +0200 Subject: [PATCH 3/5] fix(runner): survive the error event a closed stdout emits Under Node a write to a closed pipe reports EPIPE twice: to the write callback, and as an error event a tick later. With no listener the event is an uncaught exception, so the unwritable answer the writer had just produced was followed by a crash and exit 1. The listener now goes on before the write and comes off only after a success. The test doubles are real Writable streams, since an object with only a write method never emits the event and hid the crash. --- .../interactiveRunner/deps.testUtils.ts | 7 +-- .../interactiveRunner/writeScreenshot.test.ts | 46 +++++++++++++------ .../interactiveRunner/writeScreenshot.ts | 24 +++++++--- 3 files changed, 53 insertions(+), 24 deletions(-) diff --git a/src/domains/interactiveRunner/deps.testUtils.ts b/src/domains/interactiveRunner/deps.testUtils.ts index cbb927bf9..beaa1db8a 100644 --- a/src/domains/interactiveRunner/deps.testUtils.ts +++ b/src/domains/interactiveRunner/deps.testUtils.ts @@ -1,4 +1,5 @@ import { sep } from "node:path"; +import { Writable } from "node:stream"; import type { Mock } from "bun:test"; import type { RunFiles } from "@qawolf/api-contracts/v1"; @@ -98,12 +99,12 @@ export function makeTestDeps( }, }; const stdoutWrites: Uint8Array[] = []; - const recordingStdout: ScreenshotStdout = { - write(chunk, callback) { + const recordingStdout: ScreenshotStdout = new Writable({ + write(chunk: Uint8Array, _encoding, callback) { stdoutWrites.push(Uint8Array.from(chunk)); callback(); }, - }; + }); return { collectRunFiles: async () => ({ files, unresolvedImports: [] }), cwd: testCwd, diff --git a/src/shell/interactiveRunner/writeScreenshot.test.ts b/src/shell/interactiveRunner/writeScreenshot.test.ts index 1ee5ce2ed..499a3e4d8 100644 --- a/src/shell/interactiveRunner/writeScreenshot.test.ts +++ b/src/shell/interactiveRunner/writeScreenshot.test.ts @@ -1,3 +1,4 @@ +import { Writable } from "node:stream"; import { describe, expect, it } from "bun:test"; import type { Fs } from "~/shell/fs.js"; @@ -8,25 +9,28 @@ import { type ScreenshotStdout, writeScreenshot } from "./writeScreenshot.js"; const jpegBytes = Uint8Array.from([0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10]); const imageJpegBase64 = Buffer.from(jpegBytes).toString("base64"); -/** Records what reached stdout, and lets a test close the pipe. */ +/** + * Records what reached stdout, and lets a test close the pipe. A real + * `Writable` rather than an object with a `write`, because a failed write on a + * real stream also emits `error`, and a double that only calls back would pass + * a writer that lets that event crash the process. + */ function makeRecordingStdout(failWith?: Error): { stdout: ScreenshotStdout; chunks: Uint8Array[]; } { const chunks: Uint8Array[] = []; - return { - chunks, - stdout: { - write(chunk, callback) { - if (failWith) { - callback(failWith); - return; - } - chunks.push(Uint8Array.from(chunk)); - callback(); - }, + const stdout = new Writable({ + write(chunk: Uint8Array, _encoding, callback) { + if (failWith) { + callback(failWith); + return; + } + chunks.push(Uint8Array.from(chunk)); + callback(); }, - }; + }); + return { chunks, stdout }; } /** Records what reached the filesystem, which is the only thing worth asserting. */ @@ -174,8 +178,10 @@ describe("writeScreenshot", () => { }); // A reader that went away is an unwritable destination, answered before the - // command claims success rather than as an EPIPE after it. - it("reports a pipe that would not take the bytes", async () => { + // command claims success rather than as an EPIPE after it. The stream also + // emits `error` after the callback, so the test waits a tick for the event + // that would otherwise be uncaught. + it("reports a pipe that would not take the bytes, and survives the error event", async () => { const { fs } = makeRecordingFs(); const { stdout } = makeRecordingStdout(new Error("EPIPE: broken pipe")); @@ -185,6 +191,7 @@ describe("writeScreenshot", () => { path: "-", stdout, }); + await new Promise((resolve) => setImmediate(resolve)); expect(result).toEqual({ detail: "EPIPE: broken pipe", @@ -192,5 +199,14 @@ describe("writeScreenshot", () => { reason: "unwritable", }); }); + + it("leaves no error listener behind after a successful write", async () => { + const { fs } = makeRecordingFs(); + const { stdout } = makeRecordingStdout(); + + await writeScreenshot({ fs, imageJpegBase64, path: "-", stdout }); + + expect((stdout as Writable).listenerCount("error")).toBe(0); + }); }); }); diff --git a/src/shell/interactiveRunner/writeScreenshot.ts b/src/shell/interactiveRunner/writeScreenshot.ts index 5d55f8f54..8c885726b 100644 --- a/src/shell/interactiveRunner/writeScreenshot.ts +++ b/src/shell/interactiveRunner/writeScreenshot.ts @@ -1,4 +1,5 @@ import { dirname } from "node:path"; +import type { Writable } from "node:stream"; import { errorMessage } from "~/core/errors.js"; import type { Fs } from "~/shell/fs.js"; @@ -20,10 +21,8 @@ const jpegStartOfImage = [0xff, 0xd8, 0xff]; */ export const stdoutPath = "-"; -/** The one stdout method the writer needs, so a test can stand in for it. */ -export type ScreenshotStdout = { - write(chunk: Uint8Array, callback: (error?: Error | null) => void): unknown; -}; +/** What the writer needs of stdout, so a test can stand a `Writable` in for it. */ +export type ScreenshotStdout = Pick; export type ScreenshotWrite = | { ok: true } @@ -71,14 +70,27 @@ export async function writeScreenshot(options: { return { ok: true }; } +/** + * A closed reader reaches Node twice: the write callback gets the EPIPE, and + * the stream emits `error` a tick later, which with no listener is an uncaught + * exception that takes the process down after this function already answered. + * So the listener goes on before the write and comes off only after a success; + * on a failure it stays to absorb the event. + */ function writeToStdout( stdout: ScreenshotStdout, bytes: Uint8Array, ): Promise { return new Promise((resolve, reject) => { + const onError = (error: Error) => reject(error); + stdout.once("error", onError); stdout.write(bytes, (error) => { - if (error) reject(error); - else resolve(); + if (error) { + reject(error); + return; + } + stdout.off("error", onError); + resolve(); }); }); } From ee1516ebfd475dbe251607e424fe2dea4923e041 Mon Sep 17 00:00:00 2001 From: Goran Gajic Date: Mon, 7 Sep 2026 16:41:55 +0200 Subject: [PATCH 4/5] feat(runner): refuse screenshot --out - when stdout is a terminal Only a terminal on stdout selects human mode, and a terminal cannot read JPEG bytes; the confirmation would also land among them, since clack prints to stdout, rather than on stderr as the message promises. --- skills/qawolf-cli/references/runner.md | 1 + .../messages/interactiveRunner/interact.ts | 2 ++ .../takeScreenshot.stdout.test.ts | 28 +++++++++++++++---- .../interactiveRunner/takeScreenshot.ts | 16 +++++++---- 4 files changed, 37 insertions(+), 10 deletions(-) diff --git a/skills/qawolf-cli/references/runner.md b/skills/qawolf-cli/references/runner.md index 8f60f57d8..114606115 100644 --- a/skills/qawolf-cli/references/runner.md +++ b/skills/qawolf-cli/references/runner.md @@ -130,6 +130,7 @@ because every coding harness can open an image file. Read it with whatever vision you have. `--out -` writes the JPEG bytes to stdout instead, on their own, for a caller that is a process rather than an agent: the confirmation, and the JSON line under `--json`, goes to stderr so nothing follows the image on stdout. +A terminal on stdout is refused: redirect or pipe it. `qawolf runner act ` performs exactly one action per call, in the computer-use tool vocabulary a vision model already emits: `click`, diff --git a/src/core/messages/interactiveRunner/interact.ts b/src/core/messages/interactiveRunner/interact.ts index dcaa7a882..ebd261498 100644 --- a/src/core/messages/interactiveRunner/interact.ts +++ b/src/core/messages/interactiveRunner/interact.ts @@ -34,6 +34,8 @@ export const interactMessages = { "The runner has a screen and cannot serve this yet. Its virtual desktop restarts when a run changes the display size, and it serves one request at a time, so something already in flight is the usual reason. Retry in a second or two.", screenshotNotAnImage: "The screen was captured but did not arrive as a JPEG, so nothing was written. Nothing about the command needs changing: try it again, and report it if it keeps happening.", + screenshotStdoutIsATerminal: + 'Stdout is a terminal, so the JPEG bytes would have nowhere to go. Redirect stdout to a file or pipe it into a reader, or give --out a file path instead of "-".', screenshotStdoutUnwritable: (detail: string) => `The screen was captured but could not be written to stdout: ${detail}. Keep the pipe reading stdout open, or give --out a file path instead of "-".`, screenshotUnwritable: (path: string, detail: string) => diff --git a/src/domains/interactiveRunner/takeScreenshot.stdout.test.ts b/src/domains/interactiveRunner/takeScreenshot.stdout.test.ts index ff7b01b2f..244a0cc2b 100644 --- a/src/domains/interactiveRunner/takeScreenshot.stdout.test.ts +++ b/src/domains/interactiveRunner/takeScreenshot.stdout.test.ts @@ -10,7 +10,7 @@ describe("handleRunnerScreenshot --out -", () => { // The Tester session reads the bytes off the pipe instead of reserving a // file, handing it to the sandboxed user, reading it back and deleting it. it("writes the decoded image bytes to stdout and no file", async () => { - const { callPublicApi, ctx } = makeAuthCtx(); + const { callPublicApi, ctx } = makeAuthCtx("json"); callPublicApi.mockResolvedValue({ ok: true, value: { imageJpegBase64, outcome: "success" }, @@ -30,8 +30,7 @@ describe("handleRunnerScreenshot --out -", () => { // Stdout is the image, so nothing else may land there: in json mode the // answer line would otherwise follow the JPEG bytes into the reader's file. - // These two are the modes a piped stdout lands in; a terminal is human mode - // and has no reader to protect. + // These two are the modes a piped stdout lands in. for (const mode of ["json", "agent"] as const) { it(`keeps the confirmation off stdout in ${mode} mode`, async () => { const { callPublicApi, ctx, outputs, streamed, successes } = @@ -55,8 +54,27 @@ describe("handleRunnerScreenshot --out -", () => { }); } + // Human mode means a terminal on stdout, where nothing can read the bytes and + // the confirmation would land among them rather than on stderr. + it("refuses a terminal on stdout before asking the runner", async () => { + const { callPublicApi, ctx } = makeAuthCtx("human"); + const deps = makeTestDeps(); + + const result = await handleRunnerScreenshot( + ctx, + { out: "-", runner: "ci" }, + deps, + ); + + expect(result?.error).toContain("terminal"); + expect(result?.error).toContain("--out a file path"); + expect(result?.exitCode).toBe(2); + expect(callPublicApi).not.toHaveBeenCalled(); + expect(deps.stdoutWrites).toEqual([]); + }); + it("reports a pipe that closed, naming stdout rather than a file", async () => { - const { callPublicApi, ctx } = makeAuthCtx(); + const { callPublicApi, ctx } = makeAuthCtx("json"); callPublicApi.mockResolvedValue({ ok: true, value: { imageJpegBase64, outcome: "success" }, @@ -81,7 +99,7 @@ describe("handleRunnerScreenshot --out -", () => { }); it("writes nothing to stdout when the answer was not an image", async () => { - const { callPublicApi, ctx, successes } = makeAuthCtx(); + const { callPublicApi, ctx, successes } = makeAuthCtx("json"); callPublicApi.mockResolvedValue({ ok: true, value: { imageJpegBase64: "", outcome: "success" }, diff --git a/src/domains/interactiveRunner/takeScreenshot.ts b/src/domains/interactiveRunner/takeScreenshot.ts index 2250db224..c2a451e42 100644 --- a/src/domains/interactiveRunner/takeScreenshot.ts +++ b/src/domains/interactiveRunner/takeScreenshot.ts @@ -26,9 +26,9 @@ import { runnerCallOptions } from "./runnerCallOptions.js"; * delete it on every step. The bytes are decoded either way (see * `writeScreenshot`). * - * With stdout taken by the image, the confirmation moves to stderr in every - * mode, JSON included: a JSON line after the JPEG bytes would corrupt the image - * for any reader that takes stdout as the file. + * With stdout taken by the image, the confirmation moves to stderr, JSON + * included, so nothing follows the bytes into a reader that takes stdout as the + * file. A terminal on stdout is refused, since nothing there can read them. * * The four non-image answers are kept apart at the terminal and in `--json`, * because each implies a different next move and only one of them is retrying. @@ -55,6 +55,14 @@ export async function handleRunnerScreenshot( if (resolved.type === "failed") { return { ...failureFields(resolved), exitCode: resolved.exitCode }; } + // Only a terminal on stdout selects human mode, and a terminal cannot read + // JPEG bytes; the confirmation would land among them rather than on stderr. + if (options.out === stdoutPath && ctx.outputMode === "human") { + return { + error: interactiveRunnerMessages.screenshotStdoutIsATerminal, + exitCode: exitCodes.invalidArgs, + }; + } const result = await ctx.platformClient.callPublicApi( publicContractsV1.runner.takeScreenshot, @@ -70,8 +78,6 @@ export async function handleRunnerScreenshot( imageJpegBase64: result.value.imageJpegBase64, path: options.out, }); - // A payload that is not an image is the API's to fix, not the caller's; - // a path that cannot be written is the other way round. if (!written.ok) return describeUnwritten(written, options.out); if (options.out === stdoutPath) { ctx.ui.success(interactiveRunnerMessages.screenshotWrittenToStdout); From 6cb494282113c95a3057ee295b52f690022893b6 Mon Sep 17 00:00:00 2001 From: Goran Gajic Date: Mon, 7 Sep 2026 16:42:00 +0200 Subject: [PATCH 5/5] chore(runner): shorten the screenshot stdout changeset --- .changeset/runner-screenshot-stdout.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/runner-screenshot-stdout.md b/.changeset/runner-screenshot-stdout.md index ae831d504..87018ef17 100644 --- a/.changeset/runner-screenshot-stdout.md +++ b/.changeset/runner-screenshot-stdout.md @@ -2,4 +2,4 @@ "@qawolf/cli": minor --- -`qawolf runner screenshot --out -` writes the JPEG bytes to stdout instead of a file, so a caller that is a process reads the image off the pipe rather than reserving a temp file, running the command, reading it back and deleting it. Stdout carries the image alone: the confirmation, and the JSON line under `--json`, goes to stderr. +`qawolf runner screenshot --out -` writes the JPEG bytes to stdout instead of a file. Stdout carries the image alone; the confirmation, a JSON line under `--json`, goes to stderr.