[STG-2275] feat(cli): default screenshot to file output (--base64 for legacy stdout)

[shrey150]

Summary

Bare browse screenshot now writes a file by default — screenshot-<yyyymmdd-hhmmss>.<type> in the current directory, never overwriting (atomic collision counter) — and prints the small { "saved": "<path>" } JSON. A new --base64 flag preserves the legacy stdout contract ({ "base64": "..." }); it is mutually exclusive with --path. Explicit --path behavior is unchanged.

Linear: STG-2275

Impact if merged

screenshot is one of the two commands in the runaway agent retry loops (the loop population generates 92.3% of all CLI telemetry) and is used by 1,337 users/30d (89.9% success last-7d). Every bare invocation today prints ~22KB of base64 JSON directly into the calling agent's context window — a per-call token tax on exactly the ICP population (claude-code/codex agents driving the CLI; agent-tagged usage is 90%-successful and growing). Defaulting to a file write makes the common case agent-safe at zero cost to --path users; --base64 preserves the old contract for scripts. Stdout contract change for bare invocations is called out in the changeset with a --base64 migration note (patch bump per repo convention).

Implementation notes

• Breaking change (stdout contract): bare browse screenshot now prints { "saved": "<path>" } instead of { "base64": "..." }. Migration: pass --base64 to restore the old output.
• Command-layer only: the driver handler (runtime.ts) already supported both branches ({ saved } when a path is given, { base64 } otherwise), so the change is confined to src/commands/screenshot.ts.
• Default filename respects --type (.jpeg vs .png) and resolves against the invoking shell's cwd (absolute path passed to the driver so a daemon with a different cwd can't misplace the file).
• The default filename is atomically reserved via exclusive create (openSync(path, "wx")), advancing a -2, -3, ... counter on EEXIST — concurrent same-second invocations can never claim the same file (addresses cubic's race-condition review). If the command fails afterward, the empty placeholder is removed best-effort.
• --base64 is mutually exclusive with --path via oclif's exclusive option.
• Changeset: browse patch (per review) — the bare-invocation stdout contract change is called out in the changeset body with the --base64 migration note.
• Bundled skill doc: skills/browse/SKILL.md screenshot snippet updated in this PR (moved from [STG-2274] docs(cli): add 401 playbook + anti-retry guidance to bundled browse skill #2245 per review) to document the new default: bare saves a file, --path chooses it, --base64 is the legacy stdout form.

E2E Test Matrix

All rows ran against the local build (pnpm build in packages/cli, invoked as node bin/run.js) from a <scratch dir>.

Command / flow	Observed output	Confidence / sufficiency
node bin/run.js open https://example.com	{ "title": "Example Domain", "url": "https://example.com/", ... }, exit 0	Session setup for the runs below; proves the local build is functional end-to-end.
node bin/run.js screenshot (bare)	{ "saved": "<scratch dir>/screenshot-20260612-123732.png" }, exit 0; ls -la shows the file at 15,988 bytes; stdout is the small JSON only	Proves the new default: file written to cwd with timestamped name, no base64 on stdout.
node bin/run.js screenshot --base64 | head -c 200	{ "base64": "iVBORw0KGgoAAAANSUhEUgAABQgAAALH..." (PNG magic in base64), exit 0	Proves the legacy stdout contract is fully preserved behind --base64.
node bin/run.js screenshot --path /tmp/custom.png	{ "saved": "/tmp/custom.png" }, exit 0; file exists at 15,238 bytes	Proves explicit --path behavior is unchanged.
Two concurrent bare runs (same second, shell & + wait)	{ "saved": ".../screenshot-20260612-123741.png" } and { "saved": ".../screenshot-20260612-123741-2.png" }; both files present and non-empty (4,202 bytes each)	Proves the atomic no-overwrite reservation under true same-second concurrency — the exact race cubic flagged.
node bin/run.js screenshot --cdp http://127.0.0.1:1 (forced failure)	TypeError: fetch failed, nonzero exit; directory left empty — no placeholder file	Proves failed runs do not leave empty screenshot-*.png placeholders behind.
node bin/run.js screenshot --base64 --path /tmp/x.png	Error: --path=/tmp/x.png cannot also be provided when using --base64, exit 2	Proves the oclif mutual exclusion works.
node bin/run.js stop	{ "stopped": true, "session": "default" }, exit 0	Clean session teardown.
pnpm test (packages/cli)	Test Files 15 passed (15), Tests 213 passed (213) — re-run after the race fix	Supporting: no regressions in the existing CLI suite (includes the screenshot --help surface test).
pnpm lint (packages/cli)	tsc/eslint/prettier clean	Supporting only.

🤖 Generated with Claude Code

[changeset-bot]

🦋 Changeset detected

Latest commit: 81159c5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[cubic-dev-ai]

1 issue found across 2 files

Confidence score: 3/5

• In packages/cli/src/commands/screenshot.ts, the default collision handling does a non-atomic existence check before write, so concurrent CLI runs can still pick the same path and overwrite each other’s screenshot output; switch to an atomic create/unique-name strategy (or include a guaranteed-unique suffix) before merging to avoid intermittent data loss.

Architecture diagram

sequenceDiagram
    participant User as User/Agent
    participant CLI as browse CLI
    participant FS as File System
    participant Driver as Driver Handler (runtime.ts)
    participant Browser as Browser Process

    Note over User,Browser: Screenshot command flow (new default path)

    User->>CLI: browse screenshot [--type png|jpeg] [--path ...|--base64]

    alt Bare invocation (no --path, no --base64)
        CLI->>CLI: defaultScreenshotPath(type)
        CLI->>CLI: generate timestamp "screenshot-<yyyymmdd-hhmmss>.<ext>"
        CLI->>FS: check exists? (collision counter loop)
        FS-->>CLI: not found / counter found
        CLI->>CLI: resolve absolute path from cwd
        CLI->>Driver: screenshot({ path: absolutePath, type, ... })
        Driver->>Browser: capture to buffer
        Browser-->>Driver: PNG/JPEG buffer
        Driver->>FS: write file at path
        FS-->>Driver: written
        Driver-->>CLI: { saved: "absolutePath" }
        CLI-->>User: { "saved": "<absolutePath>" }
    else --base64 flag (legacy stdout contract)
        CLI->>Driver: screenshot({ base64: true, type, ... })
        Driver->>Browser: capture to buffer
        Browser-->>Driver: PNG/JPEG buffer
        Driver->>Driver: encode base64
        Driver-->>CLI: { base64: "..." }
        CLI-->>User: { "base64": "..." }
    else --path flag (explicit path, unchanged)
        CLI->>CLI: use provided path directly
        CLI->>Driver: screenshot({ path: explicitPath, type, ... })
        Driver->>Browser: capture to buffer
        Browser-->>Driver: PNG/JPEG buffer
        Driver->>FS: write file at path
        FS-->>Driver: written
        Driver-->>CLI: { saved: "explicitPath" }
        CLI-->>User: { "saved": "<explicitPath>" }
    end

    Note over CLI: --base64 and --path are mutually exclusive (oclif validation)

Loading

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

1 issue found across 2 files (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[shrey150]

this feels like there's a utility function for it

[shrey150]

There isn't a stdlib or repo utility for "delete a file if it's empty" — it's inherently stat + unlink, and we don't depend on fs-extra (and even that has no unlink-if-empty helper). So no clean util to lean on here.

But the real question is whether this function needs to exist at all, and it only does as the cleanup half of the atomic wx reservation: we pre-create a 0-byte placeholder to win the filename race (the concurrent-overwrite issue cubic flagged), so if the screenshot then fails before the driver writes, we'd leave a stray empty screenshot-<ts>.png behind. removeIfEmpty deletes that placeholder.

Two ways to simplify if the complexity isn't worth it:

1. Drop removeIfEmpty and accept that a failed screenshot leaves a 0-byte file in cwd (rare, minor litter). −7 lines.
2. Drop the whole reservation (placeholder + loop + cleanup, ~20 lines) by making the name collision-proof without touching disk — e.g. a short random suffix screenshot-<ts>-<rand>.png. Removes the race entirely, but the filename is less clean.

I'd lean toward keeping it as-is (zero litter, clean names, and it's tested), but happy to take either simplification — your call.