Revert smartsnap for now

[RaghavsBrowserStack]

No description provided.

[RaghavsBrowserStack]

Claude Code PR Review

PR: #2310 • Head: 3ca2487 • Reviewers: stack:code-reviewer

Summary

Intentional two-commit revert of PR #2207 and #2273, removing the SmartSnap / graph-trace / lockfile-diff client functionality (source, tests, ./smartsnap export, PercyClient methods, and glob-to-regexp / stream-json / snyk-nodejs-lockfile-parser deps). This code is moving server-side to be served/loaded dynamically by user access, so removal is the goal — the review covers correctness of the excision, not whether to keep it.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	Pure deletion + one allowlist edit; no secrets.
High	Security	Authentication/authorization checks present	N/A	No auth logic in scope.
High	Security	Input validation and sanitization	Pass	getStatus type allowlist tightened to snapshot/comparison, still throws on invalid type.
High	Security	No IDOR — resource ownership validated	N/A	No resource-access code added.
High	Security	No SQL injection (parameterized queries)	N/A	No DB access.
High	Correctness	Logic is correct, handles edge cases	Pass	Clean revert; getStatus allowlist + URL build intact.
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	Invalid-type guard retained.
High	Correctness	No race conditions or concurrency issues	N/A	No concurrency changes.
Medium	Testing	New code has corresponding tests	N/A	No new code; only removals.
Medium	Testing	Error paths and edge cases tested	Pass	getStatus snapshot/comparison cases retained; only smartsnap case dropped.
Medium	Testing	Existing tests still pass (no regressions)	Pass	Test suites pruned coherently; no orphaned setup or broken nesting.
Medium	Performance	No N+1 queries or unbounded data fetching	N/A	Not applicable.
Medium	Performance	Long-running tasks use background jobs	N/A	Not applicable.
Medium	Quality	Follows existing codebase patterns	Pass	Standard git revert; structure preserved.
Medium	Quality	Changes are focused (single concern)	Pass	Scoped to reverting the two PRs.
Low	Quality	Meaningful names, no dead code	Pass	No dangling refs to removed symbols anywhere in packages/ at HEAD.
Low	Quality	Comments explain why, not what	Pass	smartsnap-specific docstring removed with the method.
Low	Quality	No unnecessary dependencies added	Pass	Deps removed; @percy/logger correctly retained (still used).

Findings

• File: packages/cli-command/test/index.test.js (deleted)

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: The whole barrel-export test was deleted because it asserted applySmartSnap/SmartSnapBailError. This also drops coverage for the surviving exports (default, command, _resetShutdownForTest, legacyCommand, flags, PercyConfig, logger). Note this file was added by PR Export applySmartSnap functionality from CLI #2207, so its deletion is expected revert behavior — restoring a trimmed version is optional, not a defect.

• Suggestion: Optionally re-add a slim index.test.js asserting only the surviving re-exports, so a future dropped re-export is still caught.

• File: packages/client/test/client.test.js

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: No negative-path test asserts getStatus throws 'Invalid type passed'. Pre-existing gap, not introduced here.

• Suggestion: Consider a one-line negative test in a follow-up.

Verified correct

• package.json: ./smartsnap export, glob-to-regexp, stream-json, and the whole optionalDependencies (snyk-nodejs-lockfile-parser) removed; @percy/logger retained (still used).
• src/index.js: only the smartsnap re-export line removed; no other export dropped.
• client.js: getSmartsnapSnapshotNameToCommit / generateSmartsnapGraph fully removed; getStatus allowlist now snapshot/comparison.
• All 4 source files + 4 test files deleted cleanly; yarn.lock transitive pruning consistent.
• git grep at HEAD confirms zero remaining references to any removed symbol or dependency in packages/.

---

Verdict: PASS