Skip to content

Add missing changeset and update node version to 22 - #597

Merged
ancheetah merged 2 commits into
developfrom
aj-ci-updates-2
Oct 20, 2026
Merged

ancheetah merged 2 commits into
developfrom
aj-ci-updates-2

Conversation

@ancheetah

Copy link
Copy Markdown
Contributor

JIRA Ticket

Description

@changeset-bot

changeset-bot Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: f959b98

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@forgerock/token-vault Patch

Not sure what this means? Click here to learn what changesets are.

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

@nx-cloud

nx-cloud Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit f959b98

Command Status Duration Result
nx affected -t build lint test docs e2e-ci ✅ Succeeded 44s View ↗
nx-cloud record -- nx format:check --verbose ✅ Succeeded 3s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-10-20 20:36:39 UTC

@nx-cloud nx-cloud Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.

Nx Cloud is proposing a fix for your failed CI:

We fixed the failing javascript-sdk:test run by running device.store.test.ts in the node vitest environment instead of jsdom. This avoids the jsdom/undici AbortSignal mismatch that made the RTK Query requests fail and return no data. With these changes, all 158 tests pass locally and lint passes.

Tip

✅ We verified this fix by re-running javascript-sdk:test.

diff --git a/packages/javascript-sdk/src/device-client/device.store.test.ts b/packages/javascript-sdk/src/device-client/device.store.test.ts
index 897e45b..c60d2a2 100644
--- a/packages/javascript-sdk/src/device-client/device.store.test.ts
+++ b/packages/javascript-sdk/src/device-client/device.store.test.ts
@@ -1,3 +1,4 @@
+// @vitest-environment node
 /*
  * @forgerock/javascript-sdk
  *
diff --git a/packages/javascript-sdk/vitest.setup.ts b/packages/javascript-sdk/vitest.setup.ts
index 644736e..6677b41 100644
--- a/packages/javascript-sdk/vitest.setup.ts
+++ b/packages/javascript-sdk/vitest.setup.ts
@@ -1 +1,4 @@
-import 'vitest-canvas-mock';
+// Some suites opt into the node environment (no `window`), e.g. when using Node's native fetch
+if (typeof window !== 'undefined') {
+  await import('vitest-canvas-mock');
+}

Apply fix via Nx Cloud  Reject fix via Nx Cloud


Or Apply changes locally with:

npx nx-cloud apply-locally SS5U-j5aM

Apply fix locally with your editor ↗   View interactive diff ↗



🎓 Learn more about Self-Healing CI on nx.dev

@SteinGabriel SteinGabriel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CONTRIBUTING.md:16 still says "Node 20" instead of 22. Other than that, it looks ready to merge.

@pkg-pr-new

pkg-pr-new Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

pnpm add https://pkg.pr.new/ForgeRock/forgerock-javascript-sdk/@forgerock/ping-protect@597
pnpm add https://pkg.pr.new/ForgeRock/forgerock-javascript-sdk/@forgerock/javascript-sdk@597
pnpm add https://pkg.pr.new/ForgeRock/forgerock-javascript-sdk/@forgerock/token-vault@597

commit: f959b98

@codecov-commenter

codecov-commenter commented Oct 14, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.58%. Comparing base (87775ec) to head (f959b98).
⚠️ Report is 7 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #597      +/-   ##
===========================================
- Coverage    56.72%   56.58%   -0.15%     
===========================================
  Files          105      105              
  Lines        28457    28534      +77     
  Branches      1644     1644              
===========================================
+ Hits         16143    16145       +2     
- Misses       12314    12389      +75     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cerebrl cerebrl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR looks fine, but I have a question about the Changeset not really describing what's in this PR.

'@forgerock/token-vault': patch
---

Compose AM request URLs from config to prevent attacker-supplied request URLs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Changeset is a little confusing for me. Did we not include a Changeset with the original PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. Not one that I could find. Unless it the original fix was already released in which case the changeset would have been automatically deleted.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need to stub out the changeset in this PR and then manually change the commit SHA in the release PR later so it's tied to the right commit. See this discussion: https://pingidentity.slack.com/archives/C0ANZ21HKCH/p1792080788495179

Comment thread tasks/todo.md Outdated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is from Claude. Can we gitignore this folder?

Comment thread migrations.json Outdated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file can also be deleted if the migration completed successfully

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I swear I tried this but tests were still failing randomly for me, ughhh

@ryanbas21
ryanbas21 force-pushed the aj-ci-updates-2 branch 2 times, most recently from 8325dcd to 2fd8352 Compare October 15, 2026 20:38
@github-actions github-actions Bot removed the docs label Oct 15, 2026
chore: fix
  - upgrade nx
  - fix ci
  - fix node issues and upgrade to node 24
@github-actions github-actions Bot added the docs label Oct 20, 2026
@ancheetah
ancheetah merged commit ab5f42e into develop Oct 20, 2026
7 checks passed
@ancheetah
ancheetah deleted the aj-ci-updates-2 branch October 20, 2026 20:41
@ryanbas21 ryanbas21 mentioned this pull request Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants