✨ Require --public-link flag for private project previews

[Robdel12]

Summary

• Adds --public-link flag to the vizzly preview command
• Private projects now fail preview upload without this flag
• Clear error message explains the access model (anyone with link can view)

Why

Preview URLs for private projects grant access to anyone with the link until expiration. This change ensures users explicitly acknowledge this access model before uploading.

Test plan

• Run vizzly preview ./dist on a private project → should fail with clear error
• Run vizzly preview ./dist --public-link on a private project → should work
• Run vizzly preview ./dist on a public project → should work (no flag needed)

[claude]

Code Review

Overview

This PR adds the --public-link flag to the vizzly preview command to ensure users explicitly acknowledge that preview URLs for private projects grant access to anyone with the link. The implementation is well-motivated and addresses a legitimate security/UX concern.

Positive Aspects

1. Security-conscious design - Requiring explicit user acknowledgment for potentially sensitive access is good practice
2. Clear error messaging - The error message at src/commands/preview.js:453-464 clearly explains the access model and provides actionable next steps
3. Respects dry-run mode - The check is appropriately skipped during --dry-run (line 442)
4. Consistent with codebase patterns - Uses existing dependency injection pattern for getBuild

Issues & Concerns

1. Missing Test Coverage (Critical)

The new functionality has zero test coverage. The test file tests/commands/preview.test.js needs tests for:

• Private project without --public-link flag → should fail with correct error
• Private project with --public-link flag → should succeed
• Public project without flag → should succeed (no flag required)
• Dry-run mode on private project → should skip the check

Recommendation: Add comprehensive test coverage before merging. The test file already has good patterns to follow (see lines 98-122 for API token validation tests).

2. Redundant API Call (Performance)

The code creates a new API client and calls getBuild() at lines 443-449, but then creates another API client later at lines 647-651. This results in:

• Two API clients for the same operation
• An extra API call that could be avoided
• Potential for different client configurations

Recommendation: Refactor to reuse the build data or consolidate API client creation. For example:

// Fetch build info once and reuse
let client = createApiClient({...});
let build = await getBuild(client, buildId);

// Check visibility
if (build.project && !build.project.isPublic && !options.publicLink) {
  // ... error handling
}

// Later, reuse the same client
let result = await uploadPreviewZip(client, buildId, zipBuffer);

3. Unreachable Return Statement (Code Quality)

Line 466 has a return statement after exit(1):

exit(1);
return { success: false, reason: 'private-project-no-flag' };

The exit() function terminates the process, making the return unreachable in production. While this pattern exists elsewhere in the file for testing purposes (where exit is mocked), it's inconsistent and confusing.

Recommendation: Either remove the unreachable return or add a comment explaining it's for testing. Consider if there's a better pattern.

4. Potential Missing Error Handling

The getBuild() call (line 449) could throw if:

• Network fails
• Build doesn't exist
• API returns unexpected format

While there's a global try-catch (line 705), failing here would show a generic "Preview upload failed" error instead of something specific like "Failed to verify project visibility" or "Build not found".

Recommendation: Add specific error handling:

let build;
try {
  build = await getBuild(client, buildId);
} catch (error) {
  if (error.status === 404) {
    output.error(`Build not found: ${buildId}`);
  } else {
    output.error('Failed to verify project visibility', error);
  }
  exit(1);
  return { success: false, reason: 'build-fetch-failed' };
}

5. API Response Shape Assumption

The code assumes build.project.isPublic exists without validation (line 452):

if (build.project && !build.project.isPublic && !options.publicLink)

If the API response changes or isPublic is undefined (vs false), this could behave unexpectedly.

Recommendation: Add defensive checks:

let isPrivate = build.project && build.project.isPublic === false;
if (isPrivate && !options.publicLink) {
  // ... error handling
}

6. Timing of Check

The visibility check happens after file scanning (lines 519-537), which means:

• Users wait for file counting (potentially slow on large directories)
• Only then discover they need the flag
• Must restart the entire command

Recommendation: Move the visibility check earlier, right after build ID resolution (after line 439), to fail fast.

Minor Observations

1. CLI flag description (src/cli.js:586) is clear but could mention it's only required for private projects
2. No documentation updates - Consider if user-facing docs need updates about this new requirement

Security Considerations

No security vulnerabilities introduced. The change actually improves security posture by ensuring informed consent.

Test Plan Verification

The PR description includes a test plan, but it should be automated:

• ✅ Manual testing is described
• ❌ No automated tests added

Recommendation

Request Changes - This needs test coverage before merging. The core logic is sound, but without tests, future refactoring could break this security feature.

Priority fixes:

1. Add comprehensive test coverage
2. Optimize to avoid redundant API call
3. Move visibility check earlier for better UX

Nice to have:
4. Add specific error handling for getBuild
5. Add defensive checks for API response shape
6. Clean up unreachable return or document it

[vizzly-testing]

Vizzly - Visual Test Results

CLI Reporter - 5 changes need review

Status	Count
Passed	14
Changed	5
Auto-approved	14

Changes needing review (5)

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

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

viewer-overlay-mode · Firefox · 1920×1080 · 5.8% diff

fullscreen-viewer · Firefox · 375×667 · 79.1% diff

bulk-accept-dialog · Firefox · 375×667 · 91.2% diff

Review changes

CLI TUI - 1 change needs review

Status	Count
Passed	4
Changed	1
Auto-approved	4

Changes needing review (1)

vizzly-help · 1202×1430 · 166.0% diff

Review changes

---

_{feat/preview-public-link-flag · a11c31b7}