fix(screenshot): honor --output paths with a directory component#119
Open
HermeticOrmus wants to merge 2 commits into
Open
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.
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
1 participant
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.