[Adapters][Plugin][Test] Delete packages/service and packages/store-postgres - #290
Merged
vedanshujain merged 5 commits intoSep 20, 2026
Conversation
INC-D3b. With commerce running in-process inside the plugin since INC-D3a, the standalone Hono service and the Kysely/Postgres store adapter are both dead code. Both packages go entirely, along with their project references. `@otta-sh/store-postgres` took its 24 migrations, every `Kysely*Store`, its `.`/`./pg`/`./testing` subpaths, 29 `*.dialects.test.ts` files and 12 `*.pg.test.ts` race files with it. NONE of that concurrency coverage is lost: every one of the 12 race files already has a same-named, re-pointed `store-emdash` counterpart from the Phase A/B increments, and all 12 were run against the local test Postgres and confirmed green BEFORE this deletion — adjust-concurrency, coupon-no-over-redeem, no-oversell, no-oversell-cart, no-oversell-checkout, no-oversell-checkout-multiline, refund-race, resolve-reconciliation-race, restock-concurrency, rules-cas-race, sku-rename-race and variant-sku-rename-race, 52 tests passing. The no-oversell gate is intact; it just runs over the document adapter now. Nothing had to be rescued from the package. `store-emdash`'s dialect harness builds its own Kysely `PostgresDialect`/`SqliteDialect` straight from `kysely`, `pg` and `better-sqlite3`, all declared in its own package.json, and `store-emdash/src/id-gen.ts` has carried its own copy of `uuidIdGen` since it was written precisely so this copy could go. `test:pg` needs no change to keep selecting the right files: its glob walks `packages/*/test`, so it now resolves to 60 `store-emdash` files and zero deleted ones. The CI `integration` job, its Postgres service container and `test:pg` are all untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CQbJYWWm8tf8owshm7XRp8
…ervice harness
INC-D3b. INC-D3a collapsed `makeCommerceClient`/`makeAdminClients` to the
in-process tier unconditionally, which orphaned every HTTP client behind
them. They go here, with the tests that only ever exercised the wire.
Deleted: `HttpCommerceClient` (whole module), and the four admin HTTP
clients — `AdminOrdersClient`, `AdminProductsClient`, `AdminRulesClient`,
`ReportingSettingsClient` — with their `*Options` types; the six
`http-commerce-client*.test.ts` suites; `commerce-client-contract.http.test.ts`;
`test/helpers/start-live-service.ts`; and `@hono/node-server`, which had
no other user. `commerceClientContract` now has one tier, the in-process
one.
What did NOT go: the wire types and the four `*Surface` ports, which the
console's surviving in-process code depends on throughout. Each Surface
was `Pick<ThatClient, …>`, so deleting the class would have taken the port
with it. Each is now an explicit interface with the signatures lifted
verbatim — Orders 12 methods, Products 6, Rules 25, ReportingSettings 6.
That is a strictly better home for them: the `Pick`-over-a-nominal-class
idiom existed only because `#`-private fields made the class unassignable,
and the property it was protecting ("every method is listed, so adding one
without deciding what the in-process tier does is a compile error") is
preserved. The four `InProcess*Client` classes now `implements` their
Surface, so tsc checks the lifted signatures structurally rather than
taking them on trust. The files are renamed to say what they now are:
`admin-*-client.ts` → `admin-*-surface.ts`, `reporting-client.ts` →
`reporting-settings-surface.ts`.
`test/helpers/stub-commerce-server.ts` is KEPT, renamed to
`stub-http-server.ts`. The plan had it deleted as a stand-in for the HTTP
transport, but it is not one any more: it is a generic recording HTTP
server, and three surviving sandbox suites use it for things that have
nothing to do with commerce — `sandbox-harness.test.ts` and
`in-process-egress.sandbox.test.ts` as an email-API endpoint proving
`allowedHosts` egress control, `stripe-settle-route.sandbox.test.ts` to
back a Settings re-render while asserting no secret leaks. Deleting it
would have silently dropped that coverage.
`playwright.config.ts` was a live break, not just stale prose: its
webServer stack still booted `packages/service/src/index.ts` and waited on
its `/health`. The stack is one process now, so that entry and the dead
`E2E_SERVICE_URL` knob are gone; §0.3's rule that no e2e surface may name
port 5432 is untouched and still enforced by `harness.spec.ts`.
The rest is prose: comments across the plugin, domain, store-emdash,
admin-react and the staging site that cited `@otta-sh/service` or
`@otta-sh/store-postgres` as present tense now read as history. The
synthetic fixture strings in `depcruise-boundary.test.ts` are deliberately
left — they test unresolved-specifier handling and must name packages that
do not resolve.
On the plan's open question for INC-D4: `packages/plugin/src/types.ts` is
unchanged, because the premise was a misreading. It holds no commerce wire
types at all — every hand-mirrored block in it mirrors the HOST (EmDash's
Block Kit contract) plus `HttpAccess`, the `ctx.http` capability surface,
which stays for the email API and the payment gateways. The types that did
mirror the service's wire format are the `*Wire` interfaces in the admin
surfaces and `commerce-client.ts`; the call to keep them, and the narrower
question left for D4, is recorded at the top of `commerce-client.ts`.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CQbJYWWm8tf8owshm7XRp8
INC-D3b. `changeset version` hard-fails on a changeset naming a package that is no longer in the workspace, so this is part of the deletion rather than housekeeping after it. Of 148 unreleased changesets, 59 named `@otta-sh/service` or `@otta-sh/store-postgres` in their frontmatter. 51 also named a surviving package and were re-pointed — the dead frontmatter line dropped, every surviving line and the note kept. 8 were entirely about a deleted package and are removed outright. 140 changesets remain. Five of the re-pointed notes needed a prose edit too, because their summaries made a claim that spanned the dead and surviving packages and stopped being true once the line went: a count of "all four published packages", a dangling "store-postgres stays patch" clause, and three paragraphs explaining bump reasoning for a package that will never be bumped again. Left deliberately: 25 notes still describe, in the past tense, what shipped inside those packages at the time. That is accurate history and `changeset version` only reads frontmatter, so nothing breaks — but the released CHANGELOG will carry bullets attributed to packages that no longer exist, which is worth a pass in the D4/D5 docs increments. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CQbJYWWm8tf8owshm7XRp8
Review of the deletion increment found the changesets under-stated it and a handful of guards that died with `@otta-sh/service` had no surviving counterpart. This closes both. **Changesets.** A new changeset records the increment itself: the six public exports leaving `@otta-sh/plugin`'s entry point (a `minor`), and the removal of `@otta-sh/service` and `@otta-sh/store-postgres`. Neither deleted package can be named in frontmatter — changesets refuses a release plan for a package whose directory is gone — so the removal is recorded in the prose of the package that outlived them. The remaining changesets were swept: the earlier pass trimmed their frontmatter but left bodies narrating deleted packages, dead REST routes and deleted client classes, all of which would have published verbatim into surviving packages' CHANGELOGs. Two changesets described nothing that survived and are deleted. **The two concurrency races are re-created, not just mourned.** The once-only note append under a shared idempotency key, and the single audit event under racing state flips, were local to the deleted adapter's dialect suites, so `store-emdash` never inherited them. Both now run against `EmdashOrderNotesStore` and `EmdashOrderStore` as `runIf(ctx.canRace)` cases in the existing `describeEachDialect` blocks, on Postgres, where a race is real. **Restored negative guards.** Cart lines carry no price; a guest's read of a fulfilled or cancelled order drops the staff witness and the cancellation detail. Each is a typed whitelist at the producer, so these are belt-and-braces — but they are the assertions the deleted suite held. The admin route's `public: false` gate, which had lost its only coverage, is pinned at the manifest. **Corrected a false comment.** Two comments claimed no guard existed for `serializeCart` dropping `state`. It does: `serializeCart` is annotated `: CartWire`, whose `state` is required, so dropping it fails to compile. The comments now say so. Three `skipIf(tier.payments)` cases that skip on every tier now explain themselves and point at their domain-layer coverage. Smaller unasserted bounds in the in-process admin clients are tracked in #289. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CQbJYWWm8tf8owshm7XRp8
Both cases were mutation-tested and neither died. Breaking the loser path in `EmdashOrderNotesStore.append` (return `appended: true` unconditionally) and in `EmdashOrderStore`'s guarded flip (report a won flip on a refused compare-and-set) left both suites GREEN. The CAS logic is correct; the tests were not reaching it. **Why.** `Promise.all` over N store calls is not a race here. `pg.Pool` opens connections lazily, so the first caller takes the one warm connection and completes its whole read-modify-write while its peers are still finishing a TCP connect. Instrumented on the note-append shape: all 8 callers entered within 3 ms, the winner's pre-read returned at +15 ms and its create-if-absent committed at +28 ms, and the other 7 pre-reads returned at +47 ms or later — every one of them finding the committed note. So all 7 took the replay branch and no two callers ever held the same revision. The `markPaid` case degenerates the same way, one step earlier: the peers read `state: "paid"` and refuse at the `doc.state !== fromState` guard without issuing a compare-and-set at all. **The fix** is a barrier, not more callers. `barrierCall` joins the existing fault-injection decorators: it holds the first N matching writes until every one has arrived, then releases the crowd into the real repository at once. Arrival at the barrier is itself the proof of contention — a caller only gets there after its own read decided to write — so the two cases now assert `barrier.arrived()` alongside their outcome. The barrier is one-shot, so the retry each loser performs passes straight through and the crowd cannot deadlock. Both mutations were re-applied against the barriered tests: 3/3 runs RED, with 8 of 8 appends and 12 of 12 flips claiming a win. Restored, 3/3 runs GREEN. Two changesets from the same review round: `admin-wire-completeness` restates the operator-facing warning that died with the REST service — the three admin lists still issue their count concurrently with the page read, so each request peaks at two host connections, now on the in-process path. `entitlements-check-auth` gets back the concrete false→true behaviour change that justifies its `minor` on `@otta-sh/domain`. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CQbJYWWm8tf8owshm7XRp8
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.
Summary
Deletes
@otta-sh/serviceand@otta-sh/store-postgresentirely — both were fully retired as ofthe prior increment's Worker/Hyperdrive/Neon retirement, so nothing depends on their deployment
surface anymore. Collapses
commerceClientContractto a single in-process tier, removes 6 now-deadpublic exports from
@otta-sh/plugin's index, and rebuilds the four admin*Surfaceportinterfaces as standalone types (they no longer need to mirror an HTTP client's shape).
Verification
pnpm lint— pass, 0 violations (includes the dependency-cruiser domain-purity check)pnpm typecheck— passpnpm format:check— pass (640 files)pnpm -r build— passpnpm test— 224 files / 4349 tests passed (820 skipped, 14 todo)pnpm test:pg(against Postgres) — 60 files / 1551 tests passed (7 skipped), including twonewly-added, barrier-synchronized concurrency race tests (order-notes concurrent append
idempotency; order-store concurrent
markPaidaudit-once). Both were verified by mutationtesting to genuinely force CAS contention rather than pass vacuously.
pnpm test:d1— 14 files / 525 tests passedchangeset status— exit 0 (patch: site-staging; minor: plugin, admin-presentation,admin-react, domain, store-emdash, payments-x402, payments-stripe)
A new changeset documents the package removals and the 6 removed
@otta-sh/pluginexports.~84 pre-existing changesets that referenced the deleted packages were swept to remove those stale
references.
GitHub issue #289 tracks four smaller, non-blocking test-coverage gaps (bounds checks) that were
intentionally deferred rather than blocking this PR.
Review
Two rounds of independent review plus a targeted follow-up round confirmed: domain-layer purity is
intact (dependency-cruiser rule), scope discipline holds (no unrelated changes, no
dependency/lockfile drift), and the concurrency race tests genuinely exercise contention rather than
passing vacuously.
🤖 Generated with Claude Code
https://claude.ai/code/session_01CQbJYWWm8tf8owshm7XRp8