fix(sdk-utils): drop the CLI major-version gate that blocks 2.x

[aryanku-dev]

Removes the check in isPercyEnabled() that disables snapshots unless the CLI's major is exactly 1.

if (percy.enabled && percy.version.major !== 1) {
  log.info('Unsupported Percy CLI version, disabling snapshots');

Against a 2.x CLI this silently stops capture — the build still passes, with zero snapshots and one info log as the only signal. That is the worst possible failure shape for a visual testing tool.

Why the check existed

Added 2020-10-19 in #63, the commit that created @percy/sdk-utils, as getInfo.version[0] !== 1. That was the CLI 1.0 cutover from the legacy 0.x @percy/agent, whose protocol predates /percy/healthcheck. The test it replaced pinned 0.1.0 — the fingerprint of that intent.

So it was written as a floor at a time when nothing above 1 existed, and only reads as a ceiling because it used !== where >= was meant. Blocking 2.x was never intended; the case simply was not imaginable in 2020. The 0.x agent has been gone for years, so the guard protects nothing and only blocks the next major.

percy.version stays populated for SDKs that read it.

Tests

Replaces the old spec with two, one of which is the direct regression guard:

• a 2.x CLI stays enabled — this is the bug
• a 0.x CLI is no longer special-cased

Verified: 170/170 specs, 100% coverage, node and karma suites both green.

This does not make a 2.0.0 CLI safe on its own

Two limits worth being explicit about:

1. The check runs in the customer's installed copy. Publishing this changes nothing for anyone whose lockfile predates it — they must upgrade first.

2. This covers only the nine JS SDKs. Sixteen non-JS SDK repos each carry their own hardcoded copy of the same major != 1 comparison:

   percy-capybara · percy-selenium-ruby · percy-appium-ruby · percy-selenium-python · percy-playwright-python · percy-appium-python · percy-selenium-java · percy-playwright-java · percy-appium-java · percy-espresso-java · percy-selenium-dotnet · percy-playwright-dotnet · percy-appium-dotnet · percy-tosca-dotnet (2 targets) · app-percy-tosca-dotnet · percy-xcui-swift

So the ordering for any future major is: fix all SDKs → release → wait for adoption → then bump the CLI. This PR is step one of that, not a green light.

(Scope caveat: found by searching the exact string "Unsupported Percy CLI version", so treat the count as a floor rather than a verified total.)

🤖 Generated with Claude Code

[aryanku-dev]

Claude Code Review (automated) — 1 inline finding(s). Full report in the PR comment below. Verdict: Passed.

[aryanku-dev]

[Low] 0.x CLI spec does not represent a reachable scenario

This mocks /percy/healthcheck returning 0.1.0, but per the PR's own account the legacy pre-1.0 agent never implemented that endpoint — a real 0.x agent fails in the try/catch and is disabled as "Percy is not running" before any version is read. The spec is really re-asserting "the major gate is gone" with an arbitrary version string, so the name implies a legacy-agent compatibility guarantee the code does not make.

Suggestion: rename to something like 'stays enabled regardless of the reported version', or fold it into the 2.x spec as parametrized cases.

Reviewer: stack-code-reviewer

[aryanku-dev]

Claude Code PR Review

PR: #2409 • Head: d53abed • Reviewers: stack-code-reviewer

Summary

Removes the percy.version.major !== 1 gate from isPercyEnabled() in @percy/sdk-utils, which silently disabled snapshot capture whenever the running CLI reported any major other than 1 — turning a 2.x CLI into a green build with zero snapshots. The else if (!percy.enabled) branch collapses to a plain if, and the old "unsupported version" spec is replaced by two: a 2.x CLI stays enabled (the regression guard), and a 0.x CLI is no longer special-cased.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	No credentials, tokens, or URLs introduced.
High	Security	Authentication/authorization checks present	N/A	Local healthcheck against the SDK's own CLI process; no authz surface.
High	Security	Input validation and sanitization	N/A	No new external input is parsed; the removed code only compared a version field.
High	Security	No IDOR — resource ownership validated	N/A	No resource access in this path.
High	Security	No SQL injection (parameterized queries)	N/A	No database access.
High	Correctness	Logic is correct, handles edge cases	Pass	The else if (!percy.enabled) collapse is behaviour-preserving for the not-running path: previously that branch was reached only when the first condition was false, and with the first condition gone !percy.enabled is the only remaining case. percy.version is still populated at line 13 for SDKs that read it.
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	The try/catch around the healthcheck is untouched; the caught error is still surfaced via log.debug(error) on the disabled path.
High	Correctness	No race conditions or concurrency issues	Pass	Same single-shot percy.enabled == null memoization as before; no new shared state.
Medium	Testing	New code has corresponding tests	Pass	A 2.x CLI staying enabled is asserted directly — the exact regression this PR fixes.
Medium	Testing	Error paths and edge cases tested	Pass	The pre-existing "Percy is not running" and build-failure specs still cover the disabled paths; the new specs cover the version dimension.
Medium	Testing	Existing tests still pass (no regressions)	Pass	The only spec removed is the one asserting the deleted behaviour. PR reports 170/170 specs green with 100% coverage across the node and karma suites.
Medium	Performance	No N+1 queries or unbounded data fetching	N/A	Net removal of a comparison; no data fetching changed.
Medium	Performance	Long-running tasks use background jobs	N/A	Not applicable.
Medium	Quality	Follows existing codebase patterns	Pass	Matches the surrounding logging and control-flow style in percy-enabled.js.
Medium	Quality	Changes are focused (single concern)	Pass	Two files, one behaviour, plus its tests. Nothing incidental.
Low	Quality	Meaningful names, no dead code	Pass	Test names describe the behaviour asserted. See finding 1 for one naming nit.
Low	Quality	Comments explain why, not what	Pass	Explanatory comments were deliberately dropped in d53abed; the rationale lives in the PR description, which is unusually thorough about the 2020 origin of the gate.
Low	Quality	No unnecessary dependencies added	Pass	No dependency changes.

Findings

No High or Critical findings. The four items below are non-blocking.

• File: packages/sdk-utils/test/index.test.js:116

• Severity: Low

• Reviewer: stack-code-reviewer

• Issue: The new 'stays enabled against a 0.x CLI' spec mocks /percy/healthcheck returning version 0.1.0, but by the PR's own account the legacy pre-1.0 agent never implemented that endpoint — a real 0.x agent fails in the try/catch and is disabled as "Percy is not running" long before any version is read. The spec is really re-asserting "the major gate is gone" with an arbitrary version string, so its name implies a legacy-agent compatibility guarantee the code does not actually make.

• Suggestion: Rename to something like 'stays enabled regardless of the reported version', or fold it into the 2.x spec as parametrized cases.

• File: packages/sdk-utils/src/percy-enabled.js:25

• Severity: Low

• Reviewer: stack-code-reviewer

• Issue: With the gate gone, nothing sanity-checks percy.version before the healthcheck body is trusted for config, build, type, widths, and deviceDetails (lines 13–19). If a future CLI major ever reshapes that payload, SDKs get enabled: true and then fail downstream on an undefined property instead of the clear "Unsupported Percy CLI version" message. This is very likely the intended trade-off — the premise of the PR is that 1.x and 2.x are protocol-compatible — but this was the last place in the JS SDK path that compared versions at all.

• Suggestion: Confirm with the owner of the CLI↔SDK healthcheck contract that no breaking response-shape change is planned for 2.x. If one ever is, the right guard is a payload-shape or protocol-version check, not a major-version equality test.

• File: packages/sdk-utils/src/percy-enabled.js:25

• Severity: Low

• Reviewer: stack-code-reviewer

• Issue: The PR description lists sixteen non-JS SDK repos carrying their own hardcoded copy of the same major != 1 comparison, and notes the count is a floor (found by string search, not verified exhaustively). Until those land and are adopted, this fix covers only the nine JS SDKs — a 2.x CLI would still silently disable capture everywhere else.

• Suggestion: Link a tracking issue from the PR description so the follow-up does not rest on tribal memory, and keep the stated ordering explicit: fix all SDKs → release → wait for adoption → then bump the CLI major.

• File: packages/sdk-utils/src/percy-enabled.js:25

• Severity: Low

• Reviewer: stack-code-reviewer

• Issue: The deleted branch carried the only log.debug('Found version: ...') call, so the detected CLI version is now never logged at any level. Correct to remove along with the gate, but it leaves support triage of CLI/SDK compatibility issues without the version in the debug output.

• Suggestion: Optional — emit log.debug(\Found version: ${percy.version}`)` unconditionally on the success path.

Raised by other reviewers (not independently confirmed)

@rishigupta1599 approved on 2026-08-28 with no written comments — no concerns to reconcile.

---

Verdict: PASS — the diff is minimal, correct, and directly guarded by a regression test; the four findings are polish and follow-up tracking, none blocking.