Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Independent code review via /convert:review at 5cb979758460c9c23665e0e6ecea9dfc9cf7a107. Verdict: APPROVED — 0 blocking, 5 non-blocking. B-G4 review marker written; human approval is a separate step via /convert:approve.

The load-bearing check was whether anything left the runtime closure of a published package. It did not: all 13 packages/* declare dependencies: {} except packages/utils (murmurhash: ^2.0.1), identical at both revisions — so bucketing/rule parity, the cross-SDK oracle role, is untouched. browserify and watchify have zero references anywhere in the tracked tree outside yarn.lock.

One methodology note worth recording: seeding the dependency walk from the lockfile workspace entry gives a false result, because Yarn Berry merges devDependencies into that entry's dependencies field (visible at yarn.lock:2347). The walk was re-derived from the real manifests instead.

The title's word "SDK-side" is load-bearing and accurate: 19 advisories remain vulnerable at HEAD, and all 19 are reachable only from demo/* — zero from any packages/*.

Review — convertcom/javascript-sdk PR #429

• HEAD sha: 5cb979758460c9c23665e0e6ecea9dfc9cf7a107
• Base (merge-base with main): c0c93c56114f00132c4533c6f0f95b0b70a145e7 (verified: git merge-base HEAD origin/main)
• Title: chore(deps): clear every remaining SDK-side advisory via resolutions
• Diff: 3 files, +248/-1648 (package.json, packages/js-sdk/package.json, yarn.lock)
• Reviewed: 2026-08-10, read-only. No file in the repo was modified.
• Verdict: APPROVE — 0 blocking findings, 5 non-blocking observations.

What I verified clean

V1. No runtime dependency of any published package changed — verified exhaustively

The single highest-stakes question. Method: read each workspace's real package.json
(dependencies + peerDependencies only) at both revisions, expand @convertcom/* workspace
peers transitively, then walk the Berry lockfile graph applying resolutions overrides.

Methodology note: seeding the walk from the lockfile workspace entry is wrong — Yarn Berry
merges devDependencies into the workspace entry's dependencies field (verified at
yarn.lock:2347 HEAD, where @babel/cli, eslint, @playwright/test — all devDeps in
packages/js-sdk/package.json — appear under dependencies). Seeds must come from the manifests.

Result — the entire third-party runtime surface of all 13 published packages is one package:

PUBLISHED_RUNTIME_CHANGED = False. Every published package declares "dependencies": {} except
packages/utils (murmurhash: ^2.0.1); all other inter-package links are peerDependencies on
sibling @convertcom/js-sdk-* workspaces. None of the 7 new resolutions touches murmurhash,
and the runtime manifests are byte-identical between base and HEAD for all 20 workspaces.
Bucketing/rule parity semantics are therefore untouched by this PR.

V2. browserify / watchify are genuinely unused — zero references

git grep -n -I -E 'browserify|watchify|babelify|tsify|esmify|envify|brfs' \
  5cb97975 -- . ':(exclude)yarn.lock'     ->  exit 1, no output

Grep form sanity-checked with the same invocation against a known-present token (webpack),
which returned hits in 8+ files — so the empty result is a real absence, not a broken pathspec.
This covers every workspace scripts block, rollup/webpack/karma configs, .github/workflows,
and docs in the tracked tree. Removing the two devDeps breaks no call site.

V3. The title's claim — "every remaining SDK-side advisory" — is accurate

41 open Dependabot alerts. Cross-checked every one against the resolved versions in the HEAD
lockfile, then attributed each still-vulnerable version to the workspaces that reach it
(full dev+runtime closure):

• 3 packages removed from the tree entirely: taffydb (high), vue-template-compiler, elliptic
• 19 alerts cleared by version movement (incl. underscore high, serialize-javascript high+medium, linkify-it 2x high, diff, pug, pug-code-gen, markdown-it)
• 19 alerts still vulnerable at HEAD — all of them reachable only from demo/*:

Zero still-vulnerable package is reachable from any packages/* workspace. The scoping word
"SDK-side" in the title is doing real work and is correct.

V4. All 7 resolutions are honored; none is a dead no-op entry

V5. Every forced major bump traced to its consumer and cleared

resolutions are blunt — each one overrides a declared range. I enumerated every consumer at base
and checked each SDK-side one:

mocha + diff@9 — the one that needed real proof. mocha/lib/reporters/base.js:16 does
var diff = require('diff') and line 518 calls diff.createPatch('string', actual, expected).
That is the failure-diff path — it only executes when a test fails, so a green CI run does not
exercise it. Verified directly against the published artifact instead:

curl https://registry.npmjs.org/diff/-/diff-9.0.0.tgz | tar xz
node -e "const d=require('./package/libcjs/index.js');
         console.log(typeof d.createPatch);
         console.log(d.createPatch('string','foo\nbar\n','foo\nbaz\n'))"
->  function
->  Index: string / --- string / +++ string / @@ -1,2 +1,2 @@ / foo / -bar / +baz

diff@9.0.0 still ships CJS (main: ./libcjs/index.js) and exports createPatch with the same
3-arg signature. Mocha's failure reporter is safe.

better-docs + vue-docgen-api@4 — zero functional exposure. vue-docgen-api is consumed only
by node_modules/better-docs/component.js. packages/js-sdk/jsdoc.config.json loads only
better-docs/typescript and better-docs/category as plugins — not component — and the repo
contains 0 .vue files. That resolution exists purely to evict vue-template-compiler
(GHSA-g3ch-rx76-35fx) from the tree; no code path changes.

V6. Lockfile is internally consistent

• Dangling descriptors: 0. Every dependencies edge in all 2353 entries resolves.
• Semver integrity: all 3021 npm: descriptors checked with semver.satisfies — 0 entries
  resolve outside their own declared range. (One apparent hit, node-gyp@npm:latest -> 13.0.1, is a
  dist-tag not a range, and is present identically at base: base.lock:18892, head.lock:18110.)
• Resolution sources: 2326 npm:, 21 workspace:, 6 patch:. All 6 patches are Yarn builtin
  compat patches (#optional!builtin<compat/fsevents|resolve|typescript>), not custom patch files.
  No git:, http:, file:, or portal: sources — clean for a published SDK.
• Metadata version: 8, cacheKey: 10c0; packageManager: yarn@4.10.3.

V7. CI has an immutable-install gate, and it passed on this exact sha

.github/workflows/qa.yml runs yarn install --immutable, then in packages/js-sdk:
yarn lint && yarn build && yarn test:mocha && yarn test:browser.
All check runs are bound to head_sha 5cb979758460c9c23665e0e6ecea9dfc9cf7a107 (verified via
/commits/<sha>/check-runs), and pass-qa (22) = success. The immutable install succeeding at
this sha is direct proof the lockfile matches the manifests with no drift — stronger evidence than a
local install would give here (see Coverage gaps). publish-package.yml and pages.yml also use
yarn install --immutable.

Blocking findings

None.

Non-blocking observations

N1. Five of the seven new resolutions have an unbounded upper bound

serialize-javascript: >=7.0.5, diff: >=8.0.3, pug: >=3.0.3, pug-code-gen: >=3.0.3,
underscore: >=1.13.8 accept any future major. The lockfile pins exact versions and CI is
--immutable, so nothing floats today — but the next deliberate re-resolution can jump a major of
dev tooling with no range guard. The two caret entries (jsdoc: ^4.0.5, vue-docgen-api: ^4.79.2)
are the safer form.

To be fair to the author: this mixed style is pre-existing, not introduced here — the 14 prior
resolutions are 13x >= and 1x caret (path-to-regexp: ^0.1.12). So the caret/>= inconsistency
flagged in the brief is consistent with existing repo practice rather than an oversight in this PR.
Worth a follow-up to settle on one form (>=X <Y+1 would keep the security floor and add a ceiling).

N2. pug / pug-code-gen resolutions are defensive no-ops at present

The pug advisories came from pug@2.0.4, pulled in by vue-docgen-api@3.26.0 (pug@^2.0.3).
Bumping vue-docgen-api to 4.79.2 removes that edge entirely. The only remaining pug consumer is
demo/nodejs (pug@^3.0.4), which already resolves to 3.0.4 — satisfying >=3.0.3 with or without
the resolution. Harmless as a floor; just note they are belt-and-braces, not the active fix.

N3. resolutions do not protect published consumers — fine here, worth knowing

Yarn resolutions in the root (private: true) manifest apply only to this repo's install tree;
they are not published. Anyone npm installing @convertcom/js-sdk gets none of them. That is
harmless in this case precisely because V1 holds — no resolved package is in any published
package's runtime closure. It does mean these entries fix CI/dev-tree advisories only.

N4. The "SDK-side" safety argument leans on demos never being published

demo/nodejs (@convertcom/js-sdk-demo-nodejs) and demo/reactjs (@convertcom/js-sdk-demo-react)
omit private: true. They are not publishable in practice — publish-package.yml publishes only
explicit packages/* directories gated on js-sdk-*-v tags, and release-please-config.json lists
only packages/*. Pre-existing and untouched by this PR; adding "private": true to both would make
the SDK-vs-demo boundary that this PR's scoping relies on structural rather than incidental.

N5. SonarCloud check is attributed to PR #426

gh pr checks 429 reports the SonarCloud check at
https://sonarcloud.io/dashboard?id=convertcom_javascript-sdk&pullRequest=426, and the workflow-run
list for this sha contains both a PR #426 and a PR #429 dynamic run. The check is bound to the
correct head_sha, so the code analyzed is this code; only the PR-number attribution looks off.
Cosmetic, but if SonarCloud decorations land on the wrong PR it is worth a glance.

Coverage gaps

1. No local install performed. corepack is not installed in this environment
   (which corepack -> not found) and the global yarn is 4.5.3 vs the repo's pinned
   packageManager: yarn@4.10.3. Per the brief I did not force it — running 4.5.3 against a
   4.10.3-written lockfile risks rewriting it. The green yarn install --immutable in CI at this
   exact sha covers this question better than a mismatched local install would.
2. Docs content is unverified. yarn build runs jsdoc only for the main package
   (generate-rollup-config.mjs:217 isMainPackage), and pages.yml copies
   packages/js-sdk/docs to gh-pages on push to main. A green build proves jsdoc 4.0.5 +
   better-docs ran; it does not prove the generated HTML is equivalent to what jsdoc 3.6.11
   produced. Recommend eyeballing the gh-pages output after merge — this is the one user-visible
   surface the major bumps touch.
3. test:browser / Playwright ran in CI; I did not re-run it locally.
4. Dependabot alert state is a snapshot of the default branch taken during this review
   (41 open). Alert closure after merge is inferred from lockfile versions, not observed.

Verdict

APPROVE — 0 blocking findings.

The two claims that carried real risk both hold under direct verification: no published package's
runtime dependency closure changes (it is murmurhash@2.0.1 and nothing else, identical at both
revisions), and browserify/watchify have zero references anywhere in the tracked tree. The
advisory bookkeeping in the title is precise — all 19 surviving advisories are demo-only. The one
genuinely non-obvious hazard, mocha's failure-only diff.createPatch path being force-bumped two
majors, was checked against the published diff@9.0.0 artifact rather than assumed safe from green CI.