fix(adyen): remount the Drop-in when the session identity changes - #818
Open
acasazza wants to merge 27 commits into
Open
fix(adyen): remount the Drop-in when the session identity changes#818acasazza wants to merge 27 commits into
acasazza wants to merge 27 commits into
Conversation
Nothing in the options handed to `AdyenCheckout` is a session token: the
instance is assembled from `paymentMethodsResponse` and from `amount`, which
comes straight off `order.total_amount_with_taxes_cents`. So the session goes
stale in two ways — the payment source is replaced, or the order total moves
under a source that keeps its id, which is what applying or removing a coupon
does. Either way the instance on screen is still talking to something the API
has moved past, and only a remount fixes it.
Until now hosts got that remount by accident, by unmounting the component:
mfe-checkout's payment step is rendered behind `accordionCtx.isActive &&`, so
every accordion toggle tore the Drop-in down and built a fresh one. That is
also why it leaks on Safari — each rebuild strands three cross-origin iframe
Windows that WebKit never reclaims — and a host that stops unmounting to fix
the leak loses the rebuild with it.
A `key` states the dependency directly, without touching the `checkout` latch
in AdyenPayment, which is set once at initialization and never cleared and so
keeps the init branch shut for the rest of the component's life.
The key only advances on a *ready* source. A replacement is created empty and
filled a moment later, so keying on the id alone would yield a Drop-in with no
payment methods — the risk already documented at AdyenPayment.tsx:174-176.
Until it is ready the old instance stays up, which is still more use than a new
empty one.
A partial authorization does NOT move the order total: there it is the
remaining amount that changes, refreshed in place by
`checkoutRef.update({ amount }, { shouldReinitializeCheckout: true })`. The two
signals are disjoint, so this key does not tread on that path.
Verified from mfe-checkout against the local build. On its own the key changes
no behaviour: the three givex expired-order tests stay green. Paired with a
host that keeps the payment step mounted, the full Adyen suite is green (14
passed, 1 pre-existing skip) and the WebKit RSS measurement drops from +504 MB
over 25 cycles, monotonic, to +100.8 MB that saturates.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for commercelayer-react-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
gciotola
previously approved these changes
Aug 26, 2026
commit: |
The publish workflow relied entirely on OIDC trusted publishing, which cannot work for a package that does not yet exist on the registry: npm configures trusted publishers per package, so the token exchange 404s, pnpm falls back to an env token, and with none set the unauthenticated PUT is rejected as a 404 as well. Set NODE_AUTH_TOKEN on both publish steps so the first release of @commercelayer/core-components and @commercelayer/react-hooks-components can bootstrap. Trusted publishing can be configured per package afterwards, and the token then becomes a fallback. Both new packages also lacked a files field, so they would have shipped src/ and coverage/ alongside dist. Restrict them to dist as react-components already does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The token env vars are removed: publishing already worked through OIDC trusted publishing, and a token was never the missing piece. The 404 comes from core-components and react-hooks-components not existing on the registry yet, so no trusted publisher can be configured for them. They get bootstrapped once with a token locally, outside CI, which stays token-free. Add the repository field both packages lacked. npm requires it to match the GitHub repository for trusted publishing and provenance, so OIDC would keep failing for them even once they exist. homepage and bugs come along to match react-components, which already publishes through OIDC. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
pnpm implements its own OIDC token exchange, and it answers 404 against npmjs for packages whose trusted publisher is configured -- the failure is reported as ERR_PNPM_AUTH_TOKEN_EXCHANGE, a pnpm code, and the misleading E404 on the PUT follows from publishing unauthenticated. npmjs trusted publishing expects npm's implementation, which is why the workflow already installs npm@latest with a comment about the 11.5.1 requirement -- a step that had no effect while pnpm did the publishing. pnpm still packs, since only it resolves the workspace: protocol into concrete versions; npm publishes the resulting tarball. Each package packs into its own temp dir so the glob passes exactly one file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The publish reached npm's own error at last: a 404 on the PUT with no OIDC
attempt logged at all. The cause is setup-node, not the trusted publisher
config. Given registry-url, v6 writes an .npmrc carrying
//registry.npmjs.org/:_authToken=${NODE_AUTH_TOKEN} and sets NODE_AUTH_TOKEN
to the dummy XXXXX-XXXXX-XXXXX-XXXXX. npm reads it, considers itself
authenticated, and so never exchanges the OIDC token.
The run that did publish through OIDC, v4.29.8-beta.0, used setup-node@v7,
where the dummy is gone: its .npmrc kept ${NODE_AUTH_TOKEN} unresolved, so
npm found no usable credential and fell through to OIDC.
Drop registry-url, which is what makes that .npmrc exist, and move to v7.
The default registry is registry.npmjs.org either way. Known trap, filed as
actions/setup-node#1551 and npm/documentation#1960.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tions Removing registry-url moved the failure from E404 to ENEEDAUTH: npm no longer believes the dummy token authenticates it, but it still does not exchange the OIDC token. Two preconditions were never visible in any log -- npm >= 11.5.1 and the ACTIONS_ID_TOKEN_REQUEST_* variables -- so print both. Publishing a pre-built tarball is also a suspect, since provenance and the OIDC exchange are documented as unsupported for an out-of-band tarball. Run npm against the real package directory instead, with pnpm's resolved manifest written in place, which keeps the workspace: protocol handled. Packing happens for every package before any manifest is swapped, so pnpm always resolves against untouched workspace versions. The directory list is a bash array rather than a word-split string. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removing registry-url outright was the wrong correction. It took away the registry configuration along with the token, and npm went from believing a dummy token authenticated it (404) to having nothing at all and not attempting the exchange either (ENEEDAUTH). npm needs to know which registry it publishes to before it will exchange the OIDC token. The reported fix deletes just the //registry.npmjs.org/:_authToken line that setup-node writes, keeping registry-url in place. Do that, and print the resulting npmrc so the state is visible rather than inferred. This run also ruled out the two preconditions by measurement: npm 12.0.2 is well past 11.5.1, and ACTIONS_ID_TOKEN_REQUEST_URL is present. npm 12 is not the culprit either, since it shipped on 2026-07-29, before the publish that did work on 2026-08-01. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the accumulated attempts with the configuration known to publish successfully: plain pnpm -r publish, no npmrc surgery, no manifest rewriting. Notable deltas from what we had: pnpm/action-setup pins version 11.x, which overrides the packageManager field, and checkout moves to v7 with the Slack action on v3.0.5 and its webhook inputs. The JSR step is left out on purpose. No package here declares a jsr:publish script and there is no jsr.json, since JSR was removed from this repo in 73d67c3 (2026-02-10). Including it would only fire on a non-prerelease, so it would have broken the 5.0.0 release rather than any beta. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
action-setup@v6 aborts outright when a version input and the packageManager field disagree: "Multiple versions of pnpm specified". The sibling workflow carries version: 11.x because that repo has no packageManager field; here the field has to stay, since netlify.toml takes the pnpm version from it through Corepack and Netlify offers no PNPM_VERSION variable. So the input goes and the field moves to 11.24.0, the newest 11.x, which is what version: 11.x would have resolved to. That keeps the newer pnpm without two places disagreeing. lockfileVersion stays at 9.0 across the 11 line, so --frozen-lockfile is unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comparing the logs settles the mechanism. In v4.29.8-beta.0, the publish that
worked, there is no idtoken request and no exchange from pnpm at all: pnpm 10
delegated to npm, and npm signed the provenance statement. pnpm 11 instead
performs its own exchange, that one 404s, and pnpm then falls back to
publishing with no credentials.
So pnpm only packs, which is what resolves the workspace: protocol, and npm
publishes from the package directory -- the same path npm took on August 1st.
An earlier attempt at this failed only because the same commit had also
removed registry-url, leaving npm with no registry to exchange against.
The warning about ${NODE_AUTH_TOKEN} is not a symptom: it appears six times
in the run that succeeded. With setup-node@v7 the variable is deliberately
absent, which is how npm concludes it has no credentials and falls through to
OIDC.
The loop skips versions already on the registry, so a package bootstrapped by
hand no longer aborts the whole run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The timings say npm never attempted an exchange. On August 1st, 651ms passed
between "Publishing to" and "Signed provenance statement"; in beta.9 it took
83ms to reach E404. npm went straight to the PUT with a credential it could
not use.
Both failures now fit one model. Without registry= npm answers ENEEDAUTH,
because it has no registry to exchange against; with registry= but the
//registry.npmjs.org/:_authToken=${NODE_AUTH_TOKEN} line present, npm reads
the key as configured credentials, skips OIDC, and the PUT 404s. Both
conditions have to hold: registry present, token line gone.
pnpm 10 satisfied that by accident. It failed to expand the value, warned
about it, and invoked npm without a token, so npm fell through to OIDC.
Also dump ~/.npm/_logs/*-debug-0.log on failure. Every error points at that
file and it has never been read; it carries the HTTP exchange that would say
why OIDC does not start.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reverts every change I made across 0977ca2..162b817, to get back to a clean baseline. The workflow is byte-identical to bca6888. packageManager returns to pnpm@11.22.0, and the files, repository, homepage and bugs fields are removed from the two new packages. Version numbers are untouched and stay at 5.0.0-beta.9. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
npm's OIDC token exchange answers "package not found" for @commercelayer/core-components and @commercelayer/react-hooks-components even though the registry serves their metadata over a plain GET. That is the exact condition in npm/cli#8678, filed in October 2025 against a scoped package and closed with no fix, no maintainer reply and a Needs Triage label. It is not something this repository can correct: react-components exchanges fine, and it is a five year old package with 626 versions, while these two were created yesterday. So the publish authenticates with a token. pnpm still attempts its own OIDC exchange first and warns when it fails, then falls back to the token in the npmrc that setup-node writes. The same token already published beta.3 of both packages by hand, so the combination is known to work. A preflight fails the job when the secret is empty. That state produced an opaque 404 once already, and it costs a release tag to discover. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Without a files field these two ship src/ and coverage/ alongside dist, and unpublishing stops being available after 72 hours. Same value react-components already carries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…w packages" This reverts commit 9f90fe1.
With the trusted publisher finally matching the workflow filename, the OIDC exchange succeeds and npm builds a provenance bundle -- which it then refuses, because it compares the bundle against package.json and finds repository.url empty. This field was dropped in the revert; provenance is what needs it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #817.
Nothing in the options handed to
AdyenCheckoutis a session token: the instance is assembled frompaymentMethodsResponseand fromamount, which comes straight offorder.total_amount_with_taxes_cents. So the session goes stale in two ways — the payment source is replaced, or the order total moves under a source that keeps its id, which is what applying or removing a coupon does. Either way the instance on screen is still talking to something the API has moved past, and only a remount fixes it.Until now hosts got that remount by accident, by unmounting the component. In mfe-checkout the payment step is rendered behind
accordionCtx.isActive &&, so every accordion toggle tore the Drop-in down and built a fresh one. That is also why it leaks on Safari — each rebuild strands three cross-origin iframeWindows that WebKit never reclaims — and a host that stops unmounting to fix the leak loses the rebuild with it.A
keystates the dependency directly, without touching thecheckoutlatch inAdyenPayment, which is set once at initialization and never cleared.Two details that shaped the key
It only advances on a ready source. A replacement is created empty and filled a moment later, so keying on the id alone would yield a Drop-in with no payment methods — the risk already documented at
AdyenPayment.tsx:174-176. Until it is ready the old instance stays up, which is still more use than a new empty one.It does not tread on the in-place amount refresh. A partial authorization does not move the order total: there it is the remaining amount that changes, refreshed by
checkoutRef.update({ amount }, { shouldReinitializeCheckout: true }). The two signals are disjoint.Verification
Run from mfe-checkout against the local build.
On its own the key changes no behaviour — the three givex expired-order tests stay green, so this can land ahead of any host change.
Paired with a host that keeps the payment step mounted: full Adyen suite green (14 passed, 1 pre-existing skip), plus accordion, accordion-race and checkout. The WebKit RSS measurement over 25 cycles goes from +504 MB, monotonic with no plateau, to +100.8 MB that saturates.
What this does not cover
The measurement harness cycles the accordion without ever touching the order, so it does not exercise the remounts this key introduces. A flow that repeatedly applies and removes coupons would pay for some of them, and that was not measured.
Separately, a dead Adyen session still cannot be recovered outside the gift card branch:
setSessionExpiredAtis raised only by the givex balance check atAdyenPayment.tsx:388. Paying by card after the session dies leaves "Place order" disabled. Pre-existing, out of scope here, recorded in #817.🤖 Generated with Claude Code