feat(EL-1458): let the host supply the bearer token and extra headers per request

[mariusz-peplinski]

JIRA TASK: EL-1458 - JWT / tenant via query params

Important

Related PRs, in merge order:

• feat(EL-1458): let the host supply the bearer token and extra headers per request #38 (this PR — publish 3.6.11 first)
• Elfsquad/showroom#389

WHAT

Two optional IConfiguratorOptions, both no-ops when absent:

• accessTokenProvider — supplies the bearer for every request instead of the AuthenticationContext. Returning null/undefined falls back to the existing resolution, so the host decides per request.
• additionalHeaders — headers resolved per request and applied last, overwriting what the library sets itself.

HOW

fetchRequest gains one branch and one loop. A provided token takes the authenticated path even under ANONYMOUS_AND_USER_LOGIN, because a host-supplied token stands in for a signed-in user — otherwise the request would go out anonymously with x-elfsquad-id and the token would be ignored.

Showroom V2 needs both: EMS launches the showroom with ?jwt=…&tenantId=…&organizationId=…, and today none of it can reach configurator traffic. x-elf-orgid / x-elf-tenantid decide which organization's pricing and assortment the configuration resolves against, so without this the SDK's requests answer for the wrong organization while apiClient's answer for the right one.

FOOTNOTES / CAVEATS / WEIRD STUFF

• Both options are called on every request; a host that resolves them expensively should cache.
• format:check still reports ConfiguratorContext.ts and ConfiguratorHttpError.ts — pre-existing on main, and reformatting them would bury this diff. The lines added here are prettier-clean.
• package.json is bumped to 3.6.11; package-lock.json still says 3.6.9, which it already did on main.

[mariusz-peplinski]

Review-with-friends-but-good — feat/EL-1458-request-context-options

Synthesis by Claude (Anthropic CLI), aggregating three parallel review agents: claude, agy, kimi. codex was skipped for this run at the author's request. Posted from the PR author's account via gh.

Reviewer legend: ♊ agy (Gemini) · 🌙 kimi · 🦀 claude

---

tail -f review.log

Three reviewers, off the record, before the file:line formatting kicks in.

agy ♊: Clean diff, nothing to roast here. 🫡 The host-supplied token and extra header hooks in ConfiguratorContext.ts correctly await promises and fall back to anonymous x-elfsquad-id headers when accessTokenProvider returns null. Tests cover the async paths and header overrides nicely. We can ship version 3.6.11 without breaking downstream consumers. 🫠

kimi 🌙: mostly a clean diff, tests actually cover the new branches which is more than I can say for most feature PRs I read at this hour. one real landmine though: the whole point of accessTokenProvider is "I have my own token, leave me alone", but the constructor still throws a tantrum if you don't pass tenantId under the default ANONYMOUS method — and the early return skips the apiUrl default, so your requests go to undefined/configurator/3/settings 💀 no error thrown, just vibes. also heads up that supplying a token silently drops x-elfsquad-id even if you set tenantId — consistent with USER_LOGIN, but nobody wrote that down anywhere. 🫡

claude 🦀: I went in expecting the interesting bug to be header leakage between requests — reused Request objects plus append is the classic shape — and it isn't there: every call builds a fresh Request and there's exactly one fetch in the package, so both hooks land everywhere and nothing bleeds. What I did not expect is that the feature's own headline use case can't be expressed: pass only accessTokenProvider and the constructor console.errors, bails, and never defaults apiUrl, so you get requests to undefined/configurator/3/.... I ran it to be sure rather than trust the read. Two of three of us landed on that independently, which is the part worth acting on. Also: I checked the patch-vs-minor bump before flagging it and the repo genuinely ships features as patches (3.6.9 → 3.6.10 was a feature too), so 3.6.11 is convention, not a slip.

---

🔴 Critical (correctness / data loss / security)

None. No security, injection, or secret-leakage issues; the change is purely additive and tsc --noEmit plus the full suite (43/43) are green.

🟠 Medium (inconsistencies, broken contracts that won't crash but will mislead)

1. The feature's headline use case trips the constructor's tenant guard and silently loses apiUrl [🌙🦀]
   src/configurator/ConfiguratorContext.ts:51-58 (guard) and :78-80 (the default it skips).
   authenticationMethod defaults to ANONYMOUS, and under ANONYMOUS the constructor hard-requires tenantId or tenantDomain — then console.errors and returns before apiUrl is defaulted. But the documented point of accessTokenProvider is a host that has its own token and gets tenant context via additionalHeaders (x-elf-tenantid), i.e. exactly the caller who has no reason to pass tenantId. Verified by construction, not just by reading:

   new ConfiguratorContext({ accessTokenProvider: () => "tok" })      → options.apiUrl === undefined
   new ConfiguratorContext({ tenantId: "t", accessTokenProvider: … }) → "https://api.elfsquad.io"
   

   Every request URL then becomes undefined/configurator/3/… — no throw, just a relative URL resolved against whatever page the SDK is embedded in. Fix either way: treat a supplied accessTokenProvider as satisfying the tenant requirement, or hoist the apiUrl default above the validation guards so the early return can't strand it. All five new tests pass tenantId or tenantDomain, so nothing covers this.

2. Supplying a token silently drops x-elfsquad-id even when tenantId is configured, and that isn't documented [🌙]
   src/configurator/ConfiguratorContext.ts:428-434, doc at src/configurator/IConfiguratorOptions.ts:46-58.
   The else if chain means an existing consumer who sets tenantId and adopts the new provider stops sending x-elfsquad-id. That's deliberate — it matches USER_LOGIN behaviour and ConfiguratorContext.spec.ts:233 asserts it — but for a published library it's a behavioural contract worth one sentence in the JSDoc, which currently documents only the null-fallback.

3. The comment's central claim is the one path with no test [🦀]
   src/configurator/ConfiguratorContext.ts:426-427 says the provided token wins "even when the configured method would otherwise fall back to anonymous". The only method where that's non-trivial is ANONYMOUS_AND_USER_LOGIN, where the untaken branch would await this.authenticationContext.isSignedIn() (:470-471). All three accessTokenProvider tests run under the default ANONYMOUS. One test with ANONYMOUS_AND_USER_LOGIN and a stub context asserting isSignedIn is never consulted would pin the actual claim — and would also catch a future refactor that reorders the branches into an isSignedIn() call the token was supposed to make unnecessary.

🟡 Worth a look (single source, lower confidence)

• Empty-string token falls back too, but the doc only promises that for nullish [🌙🦀]
  IConfiguratorOptions.ts:53-54 vs the if (providedToken) truthiness check at ConfiguratorContext.ts:429. Behaviour is fine; the wording should say "empty or nullish" so a provider returning "" during token refresh isn't a surprise.
• additionalHeaders's type and its implementation disagree about returning nothing [🦀]
  IConfiguratorOptions.ts:72 types the return as Record<string, string> with no null/undefined, while ConfiguratorContext.ts:443 defensively does ?? {}. accessTokenProvider explicitly allows nullish; the asymmetry means a consumer can't tell whether "no extra headers this request" is legal. Pick one.
• A rejecting provider escapes the ConfiguratorHttpError contract [🦀]
  ConfiguratorContext.ts:428 and :442 are outside the try at :448, so a throwing/rejecting accessTokenProvider or additionalHeaders surfaces a raw error rather than the ConfiguratorHttpError every other failure in fetchRequest produces. Pre-existing pattern — the getAccessToken() call at :438 has the same exposure — but hosts wiring in their own async token plumbing will hit it far more often than the OAuth path does.
• package-lock.json still says 3.6.9 [🦀]
  package.json is now 3.6.11; the lock was also missed on the 3.6.10 bump, so it's two behind.

✅ What looks clean

• One funnel, no leakage [🌙♊🦀] — exactly one fetch( in the package (ConfiguratorContext.ts:449) and three new Request sites (:386, :401, :413), each constructing a fresh Request per call. Both hooks therefore apply to every request, and mutating input.headers can't bleed across calls.
• set over append is the right call [♊🌙🦀] — Headers.set is case-insensitive and replaces, so additionalHeaders genuinely overrides the appended x-elfsquad-domain; the override test proves it rather than assuming it.
• Await handling [♊🌙🦀] — await on an optional-call handles sync and async providers uniformly, and the nullish fallbacks are guarded on both hooks.
• Test coverage of the new branches is real [♊🌙🦀] — async resolution, null fallback to x-elfsquad-id, per-request re-resolution with a mutated closure, override ordering, and the untouched default path.
• Public surface is automatic [🌙] — IConfiguratorOptions is already re-exported from src/index.ts:3, and dist/ is gitignored, so there's no stale-build risk.
• 3.6.11 is the repo's convention, not a semver slip [🦀] — features here ship as patches (3.6.9 → 3.6.10 added a field to ConfigurationFeature), so the bump is consistent.
• The malformed @link{X} JSDoc in the new comments matches the file's existing style [🦀] (IConfiguratorOptions.ts:30, ConfiguratorContext.ts:15) — not introduced here, so out of scope for this PR.

---

Synthesis

• Convergent across ≥2 sources: the constructor/apiUrl trap for token-only hosts (🌙🦀, both verified by actually constructing the object); the empty-string-vs-nullish doc wording (🌙🦀); "one fetch funnel, set semantics correct, tests cover the branches" (♊🌙🦀).
• Unique-but-load-bearing: kimi's point that adopting the provider silently drops x-elfsquad-id for consumers who set tenantId — a real contract change hiding behind a deliberate-looking assertion; and the untested ANONYMOUS_AND_USER_LOGIN case, which is the only one the new comment is actually about.
• Divergence worth naming: agy returned a clean bill of health with no findings, so the constructor trap rests on two of three reviewers rather than three.
• My take: nothing here blocks on correctness of the happy path — the header plumbing is sound and well tested. The one item I'd fix before merge is the constructor guard, because it makes the documented primary use case of this very feature fail silently instead of loudly. The x-elfsquad-id doc sentence and the ANONYMOUS_AND_USER_LOGIN test are cheap follow-ups; everything in the 🟡 tier is optional.

[p-bartosz]

Review: APPROVE ✅

Adds two optional IConfiguratorOptions hooks — accessTokenProvider (host-supplied bearer, nullish falls back to existing resolution) and additionalHeaders (per-request headers applied last) — both no-ops when absent. No must-fix issues found.

Checked, not just read:

• fetchRequest is the only fetch call in the package (ConfiguratorContext.ts:449) and every caller builds a fresh Request, so both hooks apply to all traffic and nothing bleeds between requests.
• set vs append is right in both new spots: the bearer replaces rather than stacks, and additionalHeaders overwriting library-set headers is the documented intent, asserted at ConfiguratorContext.spec.ts:296.
• Purely additive to a published interface — no signature or contract changes for existing consumers, and the else if only diverts requests where a host opts in by returning a token.
• On the PR head: 43/43 jest tests pass, tsc --noEmit clean, eslint clean, webpack build succeeds.

Non-blocking, for the record: the constructor guard at ConfiguratorContext.ts:51-58 early-returns before the apiUrl default at :78-80, so a host passing only accessTokenProvider (no tenantId/tenantDomain) gets apiUrl === undefined. That guard is pre-existing and untouched here, and the consumer in Elfsquad/showroom#389 always passes apiUrl + tenantDomain, so nothing in this PR regresses — worth a separate ticket rather than a change request. Same for the x-elfsquad-id drop when a token is supplied: deliberate, tested, and cheap to add one sentence to the accessTokenProvider JSDoc whenever this file is next touched.

The package-lock.json version drift (3.6.9 vs 3.6.11) is pre-existing on main and npm ci passes in CI.