fix(screenshot): honor --output paths with a directory component - #119
Closed
HermeticOrmus wants to merge 2 commits into
Closed
fix(screenshot): honor --output paths with a directory component#119HermeticOrmus wants to merge 2 commits into
HermeticOrmus wants to merge 2 commits into
Conversation
Before this change, `vibium screenshot -o /tmp/foo.png` (or `./foo.png`, or `subdir/foo.png`) silently landed at `~/Pictures/Vibium/foo.png` — the daemon stripped any path information down to the basename and joined with the configured screenshot dir. The behaviour was guarded by a "prevent path traversal" comment, but the daemon is local-only and the flag is supplied by the operator who already controls the host, so the guard was costing usability without buying real safety. Now: - `-o foo.png` → `~/Pictures/Vibium/foo.png` (default preserved) - `-o ./foo.png` → `<cwd>/foo.png` - `-o /tmp/foo.png` → `/tmp/foo.png` - `-o subdir/foo.png` → `<cwd>/subdir/foo.png` (parent dir auto-created) The CLI resolves any path with a directory component to an absolute path against the operator's CWD before sending to the daemon, so relative paths land where the operator typed them rather than against the daemon process's CWD. Backwards compatible: a bare basename (no separator) still uses the configured `screenshotDir`, preserving the "Screenshots save to a sensible location automatically" default from CLAUDE.md. Tests: - `tests/daemon/cli-commands.test.js` gains 4 new cases under "Daemon CLI: Screenshot honors --output path" covering absolute, relative, subdir, and bare-basename inputs. - The existing `Screenshot --full-page` test continues to pass — it uses a bare basename and the default behaviour is unchanged. - All 4 new tests pass locally; the only failing test in cli-commands.test.js is the pre-existing `Daemon CLI: quit command` which references a `vibium quit` command that no longer exists in the binary (unrelated to this PR).
Drop multi-line comment blocks and verbose flag prose for the project's "one short line max" comment style.
hugs
added a commit
that referenced
this pull request
Aug 3, 2026
vibium screenshot reduced -o to its basename and joined it with the screenshot dir, so every path the user typed was discarded: -o /tmp/x.png landed in ~/Pictures/Vibium/x.png. The daemon is a separate long-lived process, so it cannot resolve a relative path against the user's shell. The CLI now resolves -o itself and sends an absolute path; the daemon writes an absolute path as given and creates any missing parent directories. A bare filename still goes to the screenshot dir, which keeps the MCP default (~/Pictures/Vibium) intact. That default exists because an agent has no working directory the user can see; a developer typing into a terminal does, so the CLI now writes there. Adds docs/how-to-guides/screenshots.md.
Contributor
|
Thanks for this! You had the right diagnosis. The daemon half alone could not fix it though, since the daemon's working directory is not yours, so the CLI had to resolve |
hugs
added a commit
that referenced
this pull request
Aug 4, 2026
vibium pdf -o and vibium record stop -o hand the path to the daemon, whose working directory is not the caller's, so a relative path landed wherever the daemon happened to be started while the command reported success. #286 fixed this for screenshot and stopped there. These are the same bug in the two sibling commands. storage -o was already fine: it writes the file client-side. pdf also now creates missing parent directories, which screenshot and record already did.
vincebln2
pushed a commit
to vincebln2/vibium
that referenced
this pull request
Aug 10, 2026
vibium record start without -o declared record.zip relative to the daemon's working directory, which is not the user's shell (VibiumDev#119 fixed the same surprise for stop -o). The CLI now resolves the default before it goes over the socket, so the zip lands where the user ran the command. The MCP/CLI start line also no longer inlines the full engine reason when video is unavailable — the sentence stays compact and the full actionable reason still arrives in the stop result.
vincebln2
pushed a commit
to vincebln2/vibium
that referenced
this pull request
Aug 10, 2026
…d name-seeded
stop() no longer buffers the zip to hand back bytes nobody asked for.
Every client (JS async+sync, Python async+sync, Java) now returns
{path, steps, durationMs, videos | videoUnavailable}; the zip bytes
ride along only for bytes-only recordings (path null at start), where
they are the point. The Java file read-back is gone with it. stopChunk
returns the same shape.
The default destination is record-YYYYMMDD-HHMMSS.zip — a rerun never
clobbers the previous artifact; same-second collisions get a -2
suffix. The recording's name, sanitized, seeds the stem:
start({name: 'login'}) lands login-20260808-094123.zip. Explicit
paths are honored exactly and overwrite. The CLI resolves the default
against the user's shell, not the daemon's working directory (VibiumDev#119).
The recording-video spec records both decisions; the vibium play spec
gains the no-arg form: `vibium play` opens the most recently modified
recording zip in the working directory — the counterpart to
timestamped names.
vincebln2
pushed a commit
to vincebln2/vibium
that referenced
this pull request
Aug 10, 2026
A pathless browser_record_start landed wherever the MCP server process happened to start. Now it lands in the server's working directory when that is a real, writable place — hosts like Claude Code launch the server in the project, so the recording sits next to the code — and in ~/Documents/Vibium otherwise. Same reasoning as the screenshots' ~/Pictures/Vibium default (VibiumDev#119): an MCP caller may have no working directory to reason about. Recordings are work artifacts, so Documents, not Pictures or Movies. Explicit paths and the CLI (which resolves in the user's shell) are unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
vibium screenshot -o /tmp/foo.png(or./foo.png, orsubdir/foo.png) silently lands at~/Pictures/Vibium/foo.png— the daemon strips any path information down to the basename and joins with the configured screenshot dir. Driving vibium against SPAs from a project working dir, every screenshot needed a follow-upmvto land where I expected.The basename-strip is guarded by a "prevent path traversal" comment, but the daemon is local-only and the flag is supplied by the operator who already controls the host — the guard wasn't buying real safety, just costing usability.
What changed
The CLI resolves any
-ovalue with a directory component to an absolute path against the operator's CWD before sending to the daemon. The daemon honors a filename-with-directory as written; bare basenames still go to the configuredscreenshotDir, so the "Screenshots save to a sensible location automatically" default from CLAUDE.md is preserved.-o foo.png~/Pictures/Vibium/foo.png~/Pictures/Vibium/foo.png(unchanged)-o ./foo.png~/Pictures/Vibium/foo.png<cwd>/foo.png-o /tmp/foo.png~/Pictures/Vibium/foo.png/tmp/foo.png-o subdir/foo.png~/Pictures/Vibium/foo.png<cwd>/subdir/foo.png(parent auto-created)Help text and
-oflag description updated to document the new behaviour.How to test
New cases under
Daemon CLI: Screenshot honors --output path:absolute path is honored as writtenrelative path with ./ prefix lands in CWD, not screenshot dirsubdir relative path creates parent and saves therebare basename still goes to default screenshot dir(regression guard)All 4 pass; the existing
Screenshot --full-pagetest continues to pass with default behaviour unchanged.Notes
Daemon CLI: quit commandtest fails onmaintoo — it references avibium quitcommand that no longer exists in the binary. Not introduced here, but flagging.make testcouldn't run on this Linux dev box becausenpm installerrors withInvalid Version:during dedup of the@vibium/<platform>optionalDependencies (npm 11.12.1 + arborist quirk). Daemon test ran directly withnode --test, which only depends on the locally-built binary atclicker/bin/vibium.