chore: bump js-yaml to ^5.3.0

[wrn14897]

Bumps js-yaml from ^4.2.0 to ^5.3.0 (latest). v5 keeps the CJS load/dump API used by scripts/update-chart-versions.js; verified the script still produces byte-identical Chart.yaml output after the upgrade.

---

[changeset-bot]

⚠️ No Changeset found

Latest commit: a228ba5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[github-actions]

<!-- deep-review -->

Deep Review

🔴 P0/P1 — must fix

• scripts/update-chart-versions.js:2 — The release script loads the bumped dependency with CommonJS require("js-yaml") from a package with no "type": "module", while the lockfile entry in this diff renames the package bin from bin/js-yaml.js to bin/js-yaml.mjs and the v5.0.0 changelog embedded in base commit d370fb5 records a TypeScript rewrite that "reorganized the public API around flat named exports" with a dedicated migrate_v4_to_v5.md guide, so require() may throw ERR_REQUIRE_ESM or yield a namespace object whose .load is undefined. Because .github/workflows/release.yml:42 (version: yarn run version) is the only place this script ever runs and no PR-time workflow installs root dependencies, the first real execution against v5 happens post-merge inside changesets/action on main, where the failure blocks all chart releases rather than failing this PR.
  • Fix: Run yarn install && node scripts/update-chart-versions.js on Node 20 and confirm a zero exit before merging, and if require() fails, move to ^4.3.0 instead — it carries the maxTotalMergeKeys security backport that this repo's current 4.2.0 lacks, without the v5 API rewrite.
  • _{correctness, testing, maintainability, project-standards, reliability, security}

🟡 P2 — recommended

• scripts/update-chart-versions.js:12 — yaml.dump(chart) is called with no options, so the entire published charts/clickstack/Chart.yaml is re-serialized from whatever emitter defaults the installed major ships, and v5.1.0's changelog marks quoteStyle as [breaking] while v5.0.0 reworked scalar resolution and added seven new dump options. Release commit b42bbac confirms the v4 round-trip currently rewrites the file with only the version: line changing, meaning the folded description: >- at charts/clickstack/Chart.yaml:3-5 and the literal artifacthub.io/links: | block at lines 12-16 are v4-emitter-normalized and would silently reformat in the release commit under any v5 emitter delta.
  • Fix: Pin the emitter by passing explicit options such as yaml.dump(chart, { lineWidth: 80, noRefs: true }), or follow the precedent already set in .github/workflows/update-app-version.yml:26 and replace the version: line with a targeted edit instead of a full parse-and-re-serialize.
  • _{correctness, maintainability, reliability}
• .github/workflows/release.yml:42 — Neither chart-test.yml nor helm-test.yaml installs root Node dependencies or invokes update-chart-versions, so this dependency bump merges with zero automated evidence that its single consumer still runs, which is precisely why both findings above can only surface during a live release.
  • Fix: Add a PR-time job that runs the script against a scratch copy of Chart.yaml and asserts both a zero exit and that git diff reports no change beyond the version: field.
  • _{testing, correctness, maintainability, project-standards, reliability, security}

🔵 P3 nitpicks (1)

• package.json:11 — "version": "changeset version && npm run update-chart-versions" sequences two mutations non-atomically, so a throw in the second step leaves package.json bumped and charts/clickstack/Chart.yaml:18 stale — the exact skew the script exists to prevent; CI fails safe here because changesets/action aborts without opening a PR, but a maintainer running yarn version locally can commit the half-applied state.
  • Fix: Add a post-version assertion that package.json version equals the chart's version.

---

Reviewers (7): correctness, testing, maintainability, project-standards, reliability, security, learnings-researcher

Testing gaps:

• No CI job executes scripts/update-chart-versions.js before merge; the only invocation path is changesets/action on main after merge.
• No golden-file or idempotency assertion that a yaml.load + yaml.dump round-trip leaves charts/clickstack/Chart.yaml unchanged apart from the version: field.
• No smoke check that require("js-yaml").load and .dump are callable functions under the release runtime.
• docs/solutions/ does not exist, and scripts/update-chart-versions.js has never been modified since creation, so there is no prior incident record for this upgrade path.

Cleared during review: No secrets in the diff; the lockfile entry is self-consistent with a plain npm: resolution, an unchanged transitive set (argparse ^2.0.1), and a well-formed checksum. yaml.load uses DEFAULT_SCHEMA on repo-owned input only, so there is no parsing-safety regression. The caret range is inert because Yarn 4 enforces immutable installs in CI and the lockfile pins 5.3.0, and omitting a changeset matches this repo's precedent for dependency bumps.

[wrn14897]

Re: P0 — require("js-yaml") under v5

Verified the concern doesn't materialize, and closed the coverage gap:

• js-yaml v5 is a dual CJS/ESM build: exports.require → dist/js-yaml.cjs.js. Only the bin entrypoint moved to .mjs, which this repo never invokes. CJS require() returns an object with load/dump functions.
• Ran on Node 20.19.3 (matching release.yml): yarn install && node scripts/update-chart-versions.js → exit 0, and the Chart.yaml round-trip output is byte-identical to the committed file.
• The real gap was that this script only ever executed post-merge in release.yml. Added a version-script-smoke job to the PR-time helm-test.yaml workflow (373f7ce) that installs root deps on Node 20, runs the script, and asserts the round-tripped Chart.yaml parses with the version from package.json — so future dependency bumps fail on the PR instead of blocking releases on main.

No downgrade to ^4.3.0 needed; v5.3.0 includes the same maxTotalMergeKeys hardening.

[github-actions]

Deep Review

✅ No critical issues found. The bump itself is well-formed and, per the js-yaml changelog quoted in commit d370fb5, both scripts use only the load/dump surface that survives v4→v5. This PR also incidentally repairs a real lockfile desync: d370fb5 deleted the js-yaml/argparse resolutions from yarn.lock without touching package.json, leaving the base commit with a manifest entry that had no lockfile counterpart. The findings below are all about the new guard being weaker than it looks.

🟡 P2 -- recommended

• .github/workflows/helm-test.yaml:42 -- The version assertion is a tautology: package.json and charts/clickstack/Chart.yaml are both already 3.3.0, so chart.version !== version cannot fail even if update-chart-versions.js stopped writing the version entirely.
  • Fix: Write a sentinel version into package.json before invoking the script, assert the sentinel landed in Chart.yaml, then git checkout -- package.json charts/*/Chart.yaml.
  • _correctness
• .github/workflows/helm-test.yaml:41 -- The round-trip check inspects only version, so a yaml.dump formatting change from the v5 rewrite could mangle the folded description, the annotations."artifacthub.io/links" literal block that ArtifactHub re-parses, or the dependencies constraint ~0.146.0, and still pass; js-yaml 5.1.0 lists quoteStyle as an explicit breaking dump change.
  • Fix: Deep-compare the regenerated document against git show HEAD:charts/clickstack/Chart.yaml with version normalized, or run git diff --exit-code on the file after resetting the version line, and helm lint charts/clickstack against the regenerated file before discarding it.
  • _{testing, correctness, security, maintainability}
• .github/workflows/helm-test.yaml:52 -- The extract-release-notes.js step asserts nothing and deletes its own output, while the script process.exit(0)s with only a console.warn when no ## <version> section matches, so the success and silent-skip paths are indistinguishable to CI.
  • Fix: Add test -s charts/clickstack/RELEASE_NOTES.md between the script invocation and the rm -f.
  • _{testing, correctness, reliability, maintainability}
• .github/workflows/helm-test.yaml:6 -- The push trigger's paths: ['charts/**'] filter means a merged dependency-only bump never runs this job on main, yet chart-test.yml has no paths filter, so its run still satisfies release.yml's workflow_run gate and the release scripts execute against a js-yaml version the smoke job never saw on that commit.
  • Fix: Add package.json, yarn.lock, and scripts/** to the push paths list.
  • _{security, reliability}
• .github/workflows/helm-test.yaml:12 -- This is the first job in helm-test.yaml to run an npm install, and the workflow declares no permissions: block while actions/checkout@v3 defaults to persisting GITHUB_TOKEN into .git/config, so dependency lifecycle scripts (Yarn's enableScripts is on by default; .yarnrc.yml sets only nodeLinker) run with that credential readable in the workspace.
  • Fix: Add permissions: contents: read to the workflow and with: persist-credentials: false to the new job's checkout step.
  • _security
• AGENTS.md:153 -- The CI Workflows table still describes helm-test.yaml as "Unit tests + example validation", omitting the release-script smoke job this diff adds.
  • Fix: Extend that row to mention the version-script-smoke job.
  • _{maintainability, project-standards}

🔵 P3 nitpicks (5)

• .github/workflows/helm-test.yaml:48 -- git checkout -- charts/*/Chart.yaml reverts Chart.yaml before the second step, so the dump→load chaining that release.yml actually relies on (script 2 reads the file script 1 dumped) is never exercised.
  • Fix: Run both scripts in release order within one step and do the cleanup once at the end.
• .github/workflows/helm-test.yaml:18 -- The checkout / Node 20 / Corepack / yarn block is duplicated verbatim from release.yml with no shared source of truth, so a future toolchain change in one file silently invalidates the other.
  • Fix: Extract the four steps into a composite action under .github/actions/ and reference it from both workflows.
• .github/workflows/helm-test.yaml:37 -- The verification logic is an inline node -e block, while every other piece of release tooling in this repo lives in a reviewable file under scripts/.
  • Fix: Move it to scripts/verify-chart-version.js and invoke that from the workflow.
• .github/workflows/helm-test.yaml:12 -- The job has no timeout-minutes, so a hung registry install can occupy the runner up to the 6-hour default while release.yml's workflow_run gate waits on the workflow conclusion.
  • Fix: Set timeout-minutes: 10 on the job.
• .github/workflows/helm-test.yaml:30 -- run: yarn depends on Yarn 4 inferring enableImmutableInstalls from the CI env var for lockfile integrity.
  • Fix: Use yarn install --immutable to make the guarantee explicit.

---

Reviewers (7): correctness, testing, maintainability, project-standards, security, reliability, learnings-researcher.

Testing gaps:

• No assertion exercises the only state in which update-chart-versions.js does anything observable — package.json version differing from Chart.yaml version, which is exactly what changeset version produces in release.yml.
• require("js-yaml") against the published 5.3.0 package could not be verified offline; the lockfile's bin entry moved to bin/js-yaml.mjs and 5.0.0 is described as a TypeScript rewrite "reorganized around flat named exports", though Restore umd builds back to es5 in the same changelog suggests a CJS build still ships. The new job is the only signal here, and per the paths finding above it does not run on the merge commit.
• Neither release script has unit-test coverage; the smoke job is their sole exercise.

[wrn14897]

Re: P0/P1 — v5 ESM rewrite vs CommonJS require() in the release scripts

Verified directly against the installed js-yaml@5.3.0 — no ESM breakage:

• The package's exports map declares an explicit require condition → dist/js-yaml.cjs.js (import gets dist/js-yaml.mjs). Only the bin entrypoint moved to .mjs, and nothing in this repo invokes the CLI.
• require.resolve("js-yaml") → node_modules/js-yaml/dist/js-yaml.cjs.js; typeof load / typeof dump are both function, and a parse/dump round-trip succeeds — on Node 20 (release.yml's version).
• Both consumers use only load/dump: scripts/update-chart-versions.js and scripts/extract-release-notes.js. Ran both on Node 20.19.3 with exit 0.

Coverage: update-chart-versions.js was already smoke-tested at PR time by the version-script-smoke job; a228ba5 extends that job to also run extract-release-notes.js, so both release-only scripts now execute on every PR. No ESM conversion or hold on 4.x needed.