[Plugin] Route every commerce-client construction through one factory behind a transitional mode flag - #252
Merged
vedanshujain merged 1 commit intoSep 13, 2026
Conversation
… behind a transitional mode flag
Six modules each hand-rolled the same four-line `new HttpCommerceClient({
fetch: ctx.http.fetch, baseUrl: COMMERCE_SERVICE_BASE_URL, ...serviceToken })`,
across nineteen call sites — four modules behind a near-identical private
helper, two inline: the PDP commerce loader, the cart, checkout and account
routes, the entitlement download route, and the four content-sync hooks. Cutting commerce over to an
in-process client would therefore have been a six-file diff with six chances to
miss one. It is now a one-line diff in one file.
`src/commerce/make-commerce-client.ts` is the composition root. In "http" mode
it constructs exactly what those six sites constructed — same base URL, same
`ctx.http.fetch` as the only egress, same ADR-0007 write-gate token read from
write-only plugin kv, and still no header at all when the token is unset, so the
wire is byte-identical. `src/commerce/in-process-commerce-client.ts` is the
other branch: it `implements CommerceClient` so the compiler, not a reviewer, is
what guarantees the stub spans the whole port, and every one of its 25 methods
is `async` and rejects with one typed `NotImplementedError` until the bodies
land. Async, not a synchronous throw: the port is promise-returning and callers
compose it, so a synchronous throw would escape those shapes by a path the real
implementation never will.
`checkEntitlement` is now declared on the `CommerceClient` port rather than only
on the HTTP adapter. The download route called it through the concrete class;
routing that route through the factory means it is handed the port, so the port
has to carry it. The adapter already implemented exactly that signature, so this
changes no behaviour — only what the type system knows.
`ALLOWED_HOSTS` becomes mode-resolved at module load rather than derived
unconditionally: in "http" mode it is byte-identical to before (the single host
from COMMERCE_SERVICE_BASE_URL, which is what every build, every vitest run and
the sandbox harness resolve to), and in "in-process" mode it is empty for now.
It stays a `string[]` value rather than becoming a function on purpose: the
descriptor, the sandbox entry's http access, the three sync-hook defaults and
both guard suites all consume it as a value, and the descriptor shape must not
move here.
`resolveCommerceMode` lives in its own module rather than in `manifest.ts`
because the workerd sandbox harness does not bundle `manifest.ts` — it writes a
hand-rolled copy of that module's exported surface, so anything declared there
would simply be missing from the sandbox bundle. An unrecognized
`__OTTA_COMMERCE_MODE__` throws rather than falling back: the value is a
build-time define, and silently defaulting to "http" would ship a site talking
to a service the operator believed had been folded in.
Packaging: `@otta-sh/domain` and `@otta-sh/store-emdash` are promoted to
`dependencies` and marked `noExternal`, so the in-process client's future
imports land inside the emitted bundle instead of surviving as bare specifiers —
which in workerd fail at module instantiation, not anywhere readable.
`test/bundle-imports.test.ts` runs the build itself and asserts on the emitted
output for that reason; the only bare specifier it permits is
`@otta-sh/admin-presentation`, which is IO-free and deliberately external.
DELETED AT INC-D3b, all of it: `__OTTA_COMMERCE_MODE__`, `resolveCommerceMode`,
the factory's mode branch, `COMMERCE_SERVICE_BASE_URL` and the derivation of
`ALLOWED_HOSTS` from it, `HttpCommerceClient`, and the four admin HTTP clients
(`admin-orders-client`, `admin-products-client`, `admin-rules-client`,
`reporting-client`). The flag buys exactly one thing — the ability to run the
extracted client contract against both implementations and prove them
behaviourally identical before the HTTP transport is removed. It is not
permanent architecture and nothing may be designed around it.
The four admin HTTP clients are NOT routed through the factory: they are
function-export modules over a bare `{ fetch, baseUrl }` transport rather than
implementations of this port, so folding them in is its own change.
Zero behavioural change. Every pre-existing plugin suite and all 20 workerd
sandbox suites are green unmodified; the only test-count delta is the three new
files here and one added site-config case.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NjdC8awspUte5wML6eY2X
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
1 participant
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.
What
A pure refactor: the nineteen places that used to construct the plugin's commerce client directly now call a single factory instead. A build-time mode flag selects which implementation the factory hands back — the HTTP client (today's behaviour, byte-identical) or a not-yet-implemented in-process client that currently rejects on every call. This is the seventh increment of work order 02. Zero behavioural change.
Why a flag at all
The flag exists for exactly one reason: so the commerce-client contract can be run against both implementations — HTTP and in-process — and proven behaviourally identical before the HTTP transport is ever removed. It is not permanent architecture and nothing should be designed around it. At the service-removal increment, all of the following go away together: the
__OTTA_COMMERCE_MODE__define, the mode resolver, the factory's mode branch, the HTTP client itself, the four admin HTTP clients (admin-orders-client,admin-products-client,admin-rules-client,reporting-client), the service base-URL define, and the allowed-hosts list derived from it.What changed
makeCommerceClient) and its mode resolver as the single composition root for building a commerce client.ALLOWED_HOSTSis now derived per mode at module load rather than unconditionally — identical to before in HTTP mode.@otta-sh/domainand@otta-sh/store-emdashare promoted to runtime dependencies and inlined into the emitted bundle rather than left as bare imports.checkEntitlementwas already implemented on the concrete HTTP client but missing from the port — it's now declared on the port itself, with no change to the adapter's behaviour.Tests added
Verification
Review
Two independent reviews, both approve at round 1 — every construction site verified byte-identical in HTTP mode; advisory findings on test hermeticity and stub rejection semantics folded in; an independent verification run confirmed behaviour-neutrality against the base branch.
🤖 Generated with Claude Code
https://claude.ai/code/session_011NjdC8awspUte5wML6eY2X