Skip to content

fix(test): unbreak vtex_segment cookie-forward tests on Node 22#49

Merged
vibe-dex merged 1 commit into
nextfrom
fix/segment-cookie-test
May 18, 2026
Merged

fix(test): unbreak vtex_segment cookie-forward tests on Node 22#49
vibe-dex merged 1 commit into
nextfrom
fix/segment-cookie-test

Conversation

@vibe-dex

@vibe-dex vibe-dex commented May 18, 2026

Copy link
Copy Markdown
Contributor

What broke

The Release workflow on next failed after PR #48 was merged because two tests in vtex/__tests__/client-segment-cookie.test.ts failed:

× forwards vtex_segment cookie when present and caller didn't set one
× preserves auth headers alongside the forwarded cookie

Both were checking that vtexFetchResponse forwards the vtex_segment cookie when one is present on the incoming RequestContext.

Why it was actually broken from day one (not introduced by #48)

Two compounding issues, both pre-existing:

  1. new Request(url, { headers: { cookie } }) strips cookie under Node 22 / undici. Request headers are in "request" guard mode, which silently drops forbidden request headers including cookie. The cookie never reaches request.headers.get("cookie") inside the production code.
  2. @decocms/start's RequestContext defaults to a NOOP RequestStore. The ALS-backed store is installed by site code at worker boot, not in unit tests. So RequestContext.run(req, fn) calls fn() without any propagation, and RequestContext.current inside fn returns null — production code under test never sees the test's cookie.

The five other cases in the file happen to expect headers.cookie === undefined, which the broken setup also produces, masking the bug. PR #48 is the trigger but not the cause — git stash-ing all of #48's changes and running the test on stock main reproduces the same 2 failures.

Fix

Replace the RequestContext.run(new Request(...)) fixture with:

  • a fresh standalone Headers (which uses the "none" guard, so set("cookie", ...) works);
  • a minimal Ctx-shaped object ({ request: { headers }, signal, responseHeaders, bag, startedAt });
  • a vi.spyOn(RequestContext, "current", "get").mockReturnValue(fakeCtx) so the static getter returns the fake instead of consulting the noop store.

The spy is restored after fn resolves to keep tests isolated. Nothing here depends on undici internals or ALS plumbing.

Test plan

Followups

When the next push to next succeeds, this PR plus #48 will be packaged into a single prerelease on the @next dist-tag. After validating on a real storefront, both can be promoted to main via the normal next → main merge.

Made with Cursor


Summary by cubic

Unbreaks the vtex_segment cookie-forwarding tests on Node 22 by simulating a real request context and setting the cookie on a standalone Headers. Restores forwarding behavior and auth header preservation, unblocking the Release pipeline on next.

  • Bug Fixes
    • Replace RequestContext.run(new Request(...)) with a minimal fake context using a standalone Headers to set cookie.
    • Stub RequestContext.current via vi.spyOn, restoring it after each run.
    • Avoids Node 22/undici dropping cookie and the default NOOP store in @decocms/start that hid the cookie.

Written for commit 427bc73. Summary will update on new commits. Review in cubic

The two failing cases in `client-segment-cookie.test.ts` had been
silently broken since the test was first written — they pass the
default NOOP `RequestStore` so `RequestContext.current` returns null
inside the test callback and the production code never sees the
cookie under test. The five passing cases happen to expect
`headers.cookie === undefined`, which the broken setup also produces,
masking the bug. The release pipeline on `next` finally caught it
because the merged observability work bumped the Node target enough
that another spec wedged the runner into surfacing all failures
instead of one.

There are two compounding issues:

  1. `new Request(url, { headers: { cookie } })` silently drops the
     `cookie` header under Node 22 / undici because the Request
     headers guard is "request", which strips forbidden request
     headers. The cookie never reached `request.headers.get("cookie")`.

  2. `RequestContext` is backed by a `RequestStore` that defaults to
     a NOOP. The ALS-backed store is wired only at worker boot in
     site code; in unit tests the run callback executes without any
     context propagation, so `RequestContext.current` returns null.

Fix replaces the `RequestContext.run(new Request(...))` fixture with
a fresh `Headers` (which uses the "none" guard, so `set("cookie", ...)`
works) wrapped in a minimal Ctx-shaped object, and overrides the
`RequestContext.current` getter via `vi.spyOn`. The spy is restored
after `fn` resolves to keep tests isolated. No reliance on undici
internals or ALS plumbing.

Result: 413/413 tests pass. Unblocks the Release pipeline on `next`,
which runs `npm test` before `npx semantic-release` and was failing
on the post-merge run of PR #48 (the observability work).

Co-authored-by: Cursor <cursoragent@cursor.com>
@vibe-dex vibe-dex requested a review from a team May 18, 2026 22:36
@vibe-dex vibe-dex merged commit 3e0ee95 into next May 18, 2026
1 check passed
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 1.15.0-next.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant