✨ Improve preview command with smart exclusions and dry-run

[Robdel12]

Summary

• Fix form-data upload by using native FormData/Blob for fetch compatibility (the form-data npm package doesn't stream correctly with Node's native fetch)
• Add smart default exclusions for non-static files: node_modules, tests/, __tests__/, coverage/, *.config.js, package.json, *.md, etc.
• Add --dry-run flag to preview what would be uploaded without actually uploading
• Add --exclude/-x flag to add custom exclusion patterns (repeatable, use trailing / for directories)
• Add --include/-i flag to override default exclusions (e.g., --include package.json)
• Show helpful error message with examples when path argument is missing
• Skip API token validation for dry-run mode

Test plan

• Run vizzly preview ./dist --dry-run to verify file listing works
• Run vizzly preview ./dist --dry-run --verbose to see exclusion lists
• Run vizzly preview ./dist --dry-run --exclude "*.css" to test custom exclusions
• Run vizzly preview ./dist --dry-run --include package.json to test including excluded files
• Run vizzly preview (no path) to verify helpful error message
• Run actual upload to verify form-data fix works

[claude]

PR Review: Preview Command Improvements

I've reviewed the changes in this PR. Overall, the improvements are solid and well-implemented. Below are my findings organized by category:

✅ Strengths

1. Native FormData Migration

• The switch from form-data npm package to native FormData/Blob is the correct approach for Node 18+ compatibility with fetch
• Simplifies dependencies and reduces potential streaming issues
• Test correctly validates the change (checking instanceof FormData)

2. Smart Default Exclusions

• Comprehensive list of directories and files to exclude makes sense for preview uploads
• Prevents uploading build artifacts, configs, and test files that aren't needed for previews
• The --include flag allows users to override exclusions when needed

3. Dry-Run Feature

• Well-implemented with both JSON and human-readable output
• Skips API token validation (appropriate for dry-run)
• Shows file list with size info (limited to 50 files unless verbose)
• Helpful for users to preview what would be uploaded

4. User Experience

• Clear error message when path argument is missing (with examples)
• Proper handling of repeatable flags (--exclude, --include)

[claude]

🐛 Potential Issues

1. SECURITY: Command Injection Vulnerability in Exclusion Handling

The exclusion patterns are interpolated directly into shell commands without proper escaping (src/commands/preview.js:178-181). If a user passes a malicious exclusion pattern, it could lead to command injection.

Impact: Critical security vulnerability

Recommendation: Escape exclusion patterns or use array-based command execution. The isPathSafe function only validates sourceDir, not exclusion patterns.

2. Windows Platform Exclusions Not Applied

On Windows (PowerShell path), exclusions are not passed to the zip command. The comment mentions server-side filtering but this was not mentioned in the PR description.

Impact: Windows users will upload excluded files (node_modules, tests, etc.), leading to larger uploads and potentially exposing unwanted files.

Recommendation: Filter files before archiving on Windows, or document this limitation clearly.

3. Missing Test Coverage for New Features

The test file (tests/commands/preview.test.js) does not cover:

• Dry-run functionality
• Exclusion/inclusion flags
• Pattern matching logic
• File collection with exclusions
• The new CLI argument validation (missing path)

Recommendation: Add tests for these features to prevent regressions.

[claude]

⚠️ Code Quality & Best Practices

1. Duplicate Comment (src/commands/preview.js:74-75)
Two identical comments about excluding directories

2. Template Literal Inconsistency (Line 136)
Uses string concatenation instead of template literals - inconsistent with codebase style

3. Magic Number (Lines 509 and 517)
The value 50 is hardcoded twice. Consider extracting to a named constant like MAX_FILES_PREVIEW

🚀 Performance Considerations

1. File Collection in Dry-Run
The countFiles function collects all file paths into an array when collectPaths is true. For directories with thousands of files, this could consume significant memory. Consider adding a limit or warning users when file count exceeds a threshold.

2. Sequential Pattern Matching
The matchesPattern function is O(files × patterns). For large directories, consider compiling patterns into a single regex or using a more efficient matching library.

📝 Documentation & Usability

1. Trailing Slash Convention
The convention of using a trailing slash for directories (--exclude node_modules/) is not documented in the help text. Users might not discover this feature.

Recommendation: Update the option description to mention this convention.

[claude]

🔒 Security Summary

1. CRITICAL: Command injection vulnerability in exclusion patterns (needs immediate fix)
2. MEDIUM: Windows platform does not apply exclusions (potential data exposure)
3. LOW: Pattern validation could be more robust

💡 Recommendations

Must Fix Before Merge:

1. Fix command injection vulnerability in exclusion handling
2. Add tests for new functionality or document manual testing results

Should Fix:

1. Apply exclusions on Windows or document the limitation clearly
2. Remove duplicate comment
3. Extract magic number to constant

Nice to Have:

1. Add performance safeguards for large directories in dry-run
2. Improve pattern matching efficiency
3. Document trailing slash convention in help text

Summary

This PR adds valuable features (dry-run, smart exclusions) that improve the preview command significantly. However, there is a critical security vulnerability in the exclusion pattern handling that must be addressed before merging. The missing test coverage for new features is also concerning and should be remedied.

The native FormData migration is solid and well-tested. The dry-run feature implementation is thoughtful with good UX.

Recommendation: Request changes to fix the security issue and add test coverage before merging.

[vizzly-testing]

🐻 Vizzly — 14 changes need review

Status	Count
Passed	5
Changed	14
Auto-approved	5

Changes needing review (14)

filter-failed-only · Firefox · 1920×1080 · 1.2% diff

search-homepage · Firefox · 1920×1080 · 0.2% diff

search-no-results · Firefox · 1920×1080 · 0.2% diff

fullscreen-viewer · Firefox · 1920×1080 · 5.8% diff

bulk-accept-dialog · Firefox · 1920×1080 · 10.3% diff

viewer-toggle-mode · Firefox · 1920×1080 · 0.7% diff

...and 8 more in Vizzly.

Review changes →

---

_{CLI Reporter · preview-command-improvements · 1e6799d5}

[vizzly-testing]

🐻 Vizzly — 1 change needs review

Status	Count
Changed	1
Auto-approved	4

Changes needing review (1)

vizzly-help · 1202×1430 · 556.0% diff

Review changes →

---

_{CLI TUI · preview-command-improvements · 1e6799d5}