🐛 Write session breadcrumb for plugin-created builds

[Robdel12]

Why

vizzly preview could not auto-detect builds created by plugin commands like vizzly storybook because TestRunner.createBuild did not persist session context.

What Changed

• TestRunner.createBuild now writes a session breadcrumb for non-TDD builds (buildId, branch, commit, parallelId).
• Added service-level regression tests for API vs TDD behavior.
• Added a preview workflow regression test that verifies build auto-detection after TestRunner.createBuild.

Test Plan

• npm run lint
• node --test tests/services/test-runner.test.js
• node --test tests/commands/preview.test.js

Fixes #227

[vizzly-testing]

Vizzly - Visual Test Results

CLI Reporter - 1 change needs review

Status	Count
Passed	18
Changed	1
Auto-approved	18

Changes needing review (1)

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

Review changes

CLI TUI - Approved

5 comparisons, no changes detected.

View build

---

_{fix/227-plugin-session-breadcrumb · 9bcdafbe}

[claude]

Code Review

Summary

This PR successfully addresses the issue where vizzly preview couldn't auto-detect builds created by plugin commands like vizzly storybook. The fix is well-placed in the service layer, ensuring all plugins benefit from session breadcrumbs without needing to implement this logic individually.

✅ Strengths

1. Excellent architectural decision: Placing writeSession() inside TestRunner.createBuild() is the right approach. This ensures any plugin using the TestRunner automatically gets session persistence without duplicating logic.

2. Proper dependency injection: The writeSession function is properly injected through the deps object, making it fully testable. The fallback to defaultWriteSession provides good safety.

3. Test coverage is comprehensive:

   • Service-level tests verify API vs TDD behavior correctly
   • Integration test validates the end-to-end workflow (createBuild → preview auto-detection)
   • Edge cases are covered (TDD mode should NOT write session)

4. Minimal changes: The fix is surgical - only 15 lines added to production code. This reduces risk of regression.

5. Clear documentation: The PR description explains the "why" well, making it easy for future maintainers to understand the problem.

🔍 Observations & Suggestions

1. Redundant fallback check (line 112)

let writeSession = this.deps.writeSession || defaultWriteSession;

This fallback is redundant because defaultWriteSession is already assigned in the constructor (line 50). The || defaultWriteSession can be safely removed:

if (\!tdd && buildId) {
  this.deps.writeSession({
    buildId,
    branch: options?.branch,
    commit: options?.commit,
    parallelId: options?.parallelId,
  });
}

2. Error handling consideration

The writeSession() function performs filesystem operations that could fail (disk full, permissions, etc.). While writeSession() itself doesn't throw (based on the implementation), it's worth considering whether createBuild() should:

• Log a warning if session write fails (currently silent)
• Continue gracefully (current behavior is correct - session is "best effort")

This is not a blocker, just something to consider for future robustness.

3. Test assertion style (minor)

In tests/services/test-runner.test.js:56-63, using assert.deepStrictEqual for the entire array is good, but you could also test the individual session properties separately for clearer failure messages:

assert.strictEqual(writtenSessions.length, 1);
let session = writtenSessions[0];
assert.strictEqual(session.buildId, 'api-build-123');
assert.strictEqual(session.branch, 'feature/session');
assert.strictEqual(session.commit, 'abc123def456');
assert.strictEqual(session.parallelId, 'parallel-42');

This is purely stylistic - the current approach is fine.

4. Documentation opportunity

Consider adding a brief comment in test-runner.js explaining why session is written here:

// Write session breadcrumb for non-TDD builds so subsequent commands
// (e.g., vizzly preview) can auto-detect this build
if (\!tdd && buildId) {
  this.deps.writeSession({

This helps future developers understand the "why" at a glance.

🔒 Security

No security concerns. The session file is created with mode 0o600 (user-only read/write), which is appropriate for potentially sensitive build IDs.

⚡ Performance

No performance concerns. Writing a small JSON file is negligible overhead.

🧪 Testing

The test plan is solid:

• ✅ Unit tests for service behavior (TDD vs API mode)
• ✅ Integration test for the full workflow
• ✅ Proper use of dependency injection for testability

📋 Checklist

• Fixes the reported issue (🐛 vizzly storybook should write session breadcrumb for vizzly preview #227)
• Follows repository conventions (gitmoji, clear commit message)
• Includes comprehensive tests
• No breaking changes
• Maintains backwards compatibility
• Proper dependency injection for testability

🎯 Recommendation

Approve with minor suggestions. The fix is correct, well-tested, and follows the codebase patterns. The suggestions above are all optional improvements that don't block merging.

The only change I'd recommend before merging is removing the redundant fallback on line 112, but even that's not critical.