PPLT-5844: add IntelliStory storybook affected-story filtering

[RaghavsBrowserStack]

What

Adds IntelliStory: for a Storybook build, it diffs against a baseline (explicit or API-predicted) and filters the snapshot set down to only the stories a change actually affects, bailing to a full snapshot run whenever it can't safely reason about the change.

This restores the functionality previously reverted in #2310, under the IntelliStory name.

cli-command

• src/intelliStory.js — applyIntelliStory() and IntelliStoryBailError, exported via the new ./intelliStory subpath.
• src/lockfileDiff.js, src/graphTrace.js (+ graphTraceTemplate.html).
• Adds glob-to-regexp, stream-json, and optional snyk-nodejs-lockfile-parser dependencies.

client

• getStatus() accepts the intelli_story_graph job type (sync response carries the graph payload).
• getIntelliStorySnapshotNameToCommit() and generateIntelliStoryGraph() hit the /intelli_story endpoints.

Binary-crash fix (folds in #2306)

intelliStory.js and lockfileDiff.js bind createRequire to cjsRequire (not require). When transpiled to CommonJS for the packaged binary, naming it require collides with Babel's preset-env + transform-import-meta and crashes on startup with TypeError: _require is not a function. A new static regression guard, test/noRequireBinding.test.js, scans all packages/*/src for the footgun (with a matching .semgrepignore rationale).

Testing

• cli-command: 145/145 specs pass (intelliStory, lockfileDiff, graphTrace, index, noRequireBinding).
• client: IntelliStory client tests pass. (Two pre-existing proxy.test.js failures are unrelated — a Node 22 Invalid URL message-format difference in files this PR does not touch.)
• yarn build compiles cleanly; dist/intelliStory.js emits cjsRequire.

Follow-up

The external @percy/storybook consumer will need the matching @percy/cli-command/intelliStory / applyIntelliStory update.

[RaghavsBrowserStack]

Claude Code PR Review

PR: #2339 • Head: 001c036 • Reviewers: stack:code-reviewer

Summary

Adds IntelliStory — an affected-story graph feature for @percy/storybook: parses Storybook build stats, diffs against a git baseline (incl. lockfile/dependency diffing via optional snyk-nodejs-lockfile-parser), enqueues a server-side affected-story graph job, tags snapshots for selection, and optionally renders a debug trace.html. Plumbs intelliStory/storybookPath through @percy/client and @percy/core, and adds a Node 20 CI leg for the Node-18+-only snyk path.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	None introduced.
High	Security	Authentication/authorization checks present	N/A	No auth surface changed.
High	Security	Input validation and sanitization	Pass	assertSafeRef blocks flag-injection; safeJson escapes HTML/JS breakouts; paths anchored via basename.
High	Security	No IDOR — resource ownership validated	N/A	No resource-ownership surface.
High	Security	No SQL injection (parameterized queries)	N/A	No SQL.
High	Correctness	Logic is correct, handles edge cases	Fail	Finding 1 ($-pattern .replace) still open. Finding 2 (glob detection gap) resolved in 001c036.
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass*	Mostly graceful IntelliStoryBailError; one inconsistent rethrow — Finding 3.
High	Correctness	No race conditions or concurrency issues	Pass	No new concurrency.
Medium	Testing	New code has corresponding tests	Pass	Broad coverage across new modules.
Medium	Testing	Error paths and edge cases tested	Pass*	Package-affected merge branch istanbul ignored rather than tested — Finding 5.
Medium	Testing	Existing tests still pass (no regressions)	Pass	Not re-run here; CI covers.
Medium	Performance	No N+1 queries or unbounded data fetching	N/A	No queries.
Medium	Performance	Long-running tasks use background jobs	Pass	Graph job is server-side/async with timeout bail.
Medium	Quality	Follows existing codebase patterns	Pass	Consistent with client/core plumbing conventions.
Medium	Quality	Changes are focused (single concern)	Pass	Scoped to IntelliStory feature.
Low	Quality	Meaningful names, no dead code	Pass	Clear naming.
Low	Quality	Comments explain why, not what	Pass	.semgrepignore rationale well-documented.
Low	Quality	No unnecessary dependencies added	Pass	snyk-nodejs-lockfile-parser is optional/lazy-loaded.

Findings

• File: packages/cli-command/src/graphTrace.js:114-116

• Severity: Medium

• Reviewer: stack:code-reviewer (verified)

• Issue: .replace('__VERTICES_JSON__', safeJson(...)) uses plain-string replacement values. JS honors $$, $&, $`, $', $n special patterns in the replacement string even when the search arg is a literal. Embedded JSON contains arbitrary file paths; a path with $' re-injects the rest of the template, breaking the embedded JS. safeJson's escaping doesn't cover this — the corruption happens in the .replace call itself.

• Suggestion: Pass a function as the replacement: .replace('__VERTICES_JSON__', () => safeJson(...)). Add a graphTrace.test.js case with $&/$'/$$ in a vertex name.

• File: packages/cli-command/src/intelliStory.js:16, 34-41

• Severity: Medium — ✅ RESOLVED in 001c036

• Reviewer: stack:code-reviewer (verified)

• Issue: GLOB_CHARS = /[*?]/ only routed */? patterns through the glob matcher, but patternToRegex enables {extended:true, globstar:true} (brace/bracket support). A configured bailOnChanges: ['src/{a,b}.js'] or bracket pattern silently fell through to literal str === pattern and never matched — silently disabling a documented config option (bailOnChanges/untraced).

• Fix applied: Broadened detection to const GLOB_CHARS = /[*?{}[\]]/;. Added brace- and bracket-glob tests to assertNoBailOnChanges() and enforceUntraced() — verified failing under the old regex and passing under the fix (full 156-spec suite green).

• File: packages/cli-command/src/intelliStory.js:326-336

• Severity: Medium

• Reviewer: stack:code-reviewer

• Issue: Only SNYK_LOCKFILE_PARSER_UNAVAILABLE is converted to a graceful IntelliStoryBailError; a buildDepTree parse failure (malformed/older-ref lockfile) rethrows as a raw Error. Every other failure path degrades to "running full snapshot set."

• Suggestion: Confirm the @percy/storybook caller treats all applyIntelliStory throws as fall-back-to-full-set. If not, wrap this rethrow in IntelliStoryBailError for consistency.

• File: packages/cli-command/src/intelliStory.js:67-70

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: assertSafeRef regex correctly blocks leading -, but also rejects ordinary safe ref syntax like HEAD~5 / abc123^2.

• Suggestion: Confirm intentional, or widen charset to include ~^ (still safe given the leading-char restriction).

• File: packages/cli-command/src/intelliStory.js:438-440

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: The packageAffectedNodes merge into affectedNodes (the actual mechanism folding lockfile-driven dep changes into the graph) is /* istanbul ignore next */ rather than covered by a real test.

• Suggestion: Add an itPosix-style Node≥18 integration test exercising the merge end-to-end.

• File: packages/cli-command/src/intelliStory.js (resolveAndIndex)

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: Absolute module ids resolving outside projectRoot (symlinked pkg, monorepo sibling) produce ../-prefixed paths added straight into the files index without a guard.

• Suggestion: Bail or drop entries where rel.startsWith('..').

• File: .semgrepignore:118-131 / packages/client/src/client.js:2591-2592 / .github/workflows/test.yml

• Severity: Info

• Reviewer: stack:code-reviewer

• Issue: (a) Whole-file semgrep suppression means future traversal code in the growing intelliStory.js goes unchecked. (b) client.intelliStoryStats is lazily-created, never reset — accumulates across builds if a client is reused. (c) New test-node20 CI job (workflow change) — flagged for visibility; mirror logic looks consistent, no action requested.

• Suggestion: Consider narrowing semgrep scope over time; reset stats in createBuild.

---

Verdict: PASS