🧪 Verify Storybook 10 local TDD compatibility

[Robdel12]

Why

The Storybook SDK needed a real end-to-end verification pass against the latest stable Storybook release and the actual local TDD workflow, not just isolated package tests.

What changed

• upgraded the bundled example Storybook app to Storybook 10.3.6
• documented Storybook 10 compatibility requirements in the Storybook SDK README
• fixed local TDD metadata sanitization so Storybook iframe URLs keep &viewMode=story
• hardened the context review-queue test to avoid local .vizzly state leaking into cloud-path assertions
• cleaned up a couple of Storybook SDK Playwright references in comments/docs

Test plan

• npm run lint
• npm test
• cd clients/storybook && npm test
• cd clients/storybook && npm run build
• cd clients/storybook/example-storybook && npm run build-storybook
• cd clients/storybook && node ../../bin/vizzly.js tdd start --json
• cd clients/storybook && node ../../bin/vizzly.js storybook ./example-storybook/dist
• cd clients/storybook && node ../../bin/vizzly.js tdd stop --json

[vizzly-testing]

Vizzly - Visual Test Results

CLI Reporter - Approved

19 comparisons approved.

Review changes

CLI TUI - Approved

5 comparisons, no changes detected.

View build

---

_{feat/storybook-10-tdd-compat · aa9c0c08}

[claude]

Code Review

Overview

This PR upgrades the bundled example Storybook app from v8 to v10.3.6, documents Storybook 10 compatibility in the README, and fixes two actual bugs: a metadata sanitization issue that was stripping & from Storybook iframe URLs, and a test isolation issue where local .vizzly state could contaminate cloud-path assertions.

---

src/utils/security.js — Sanitization change

The change: & is removed from the character strip list in validateScreenshotProperties, so metadata values like http://localhost:6006/iframe.html?id=button--primary&viewMode=story are preserved intact.

Assessment: The fix is correct and the new comment explains the intent clearly. & alone is not an XSS vector—the remaining strip of <>"' still blocks tag injection and attribute injection. The principle (don't mangle data at the storage layer; let rendering layers HTML-encode) is sound.

One question worth confirming: Are all rendering paths that surface these metadata values (reporter UI, static HTML report) HTML-encoding their output? The previous behavior was defensive-in-depth at the data layer; removing that means the rendering layer is now solely responsible. If any path uses these values via innerHTML or similar without escaping, & could reconstruct entities in a potentially surprising way. If you've audited the reporter and confirmed it escapes all dynamic content before injection, this is fine and worth a brief note in the PR or a comment near the relevant renderer.

---

tests/utils/security.test.js — New test

Good addition. The test directly covers the regression case (a full Storybook iframe URL with &viewMode=story) and the assertion is precise.

---

tests/commands/context.test.js — Test isolation fix

Adding source: 'cloud' to the fixture is the right fix. The test was previously relying on implicit behavior that could be affected by a local .vizzly/ directory in the developer's environment, which is the kind of flaky test that's hard to debug later.

Minor question: Is this the only test in context.test.js affected by this local-state leakage, or are there sibling tests that make similar cloud-path assertions without an explicit source? Worth a quick scan to make sure this fix is complete.

---

Storybook upgrade (clients/storybook/example-storybook/)

• Upgrading from SB 8 → 10 skips a major version (9). That's fine for an example app, and the PR description confirms manual verification steps were run.
• Removing @storybook/addon-essentials from addons: [] is correct for SB10 (zero-config by default).
• The ESM-only config requirement is correctly called out in the README.
• Node requirement bump to 20.19+/22.12+ is accurately documented.

---

Documentation fixes

• --browser-args description: Puppeteer → Playwright — correct, no concerns.
• Same fix in hooks.js and tasks.js JSDoc — straightforward.

---

Summary

Solid PR overall. The security.js change is intentional and defensible; the main follow-up is confirming the rendering layer properly HTML-encodes these metadata values. The test hardening is a good improvement. No blockers.