From c554aa3b074b6881964e2297375722e492c75afe Mon Sep 17 00:00:00 2001 From: Vedanshu Date: Sun, 20 Sep 2026 14:35:59 +0000 Subject: [PATCH 1/2] [CI][Docs] Stop naming the packages INC-D3b deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit INC-D3b removed packages/service and packages/store-postgres. Two kinds of reference to them survived, and both are the sort that rots quietly. `.dependency-cruiser.cjs`: `plugin-is-sandbox-clean` banned `service` in all three of its spellings — the node_modules path, the bare specifier and the `^packages/` path. A ban on a package that cannot be imported is a clause no fixture can exercise, so nothing would notice if it stopped matching, which is precisely the failure mode the `^node:`-only builtin clause had for months. Dropped, with the reasoning recorded in the rule's own comment next to the three narrowings before it. store-postgres was never named literally in that rule: it was caught by the `(?!store-emdash(/|$))store-[^/]+` lookahead, which is untouched. That is the difference the cleanup turns on — the lookahead is a statement about the store family and still bans a store-postgres reintroduced tomorrow, whereas `service` was a statement about one dead package. `domain-is-io-free` and `store-emdash-is-sandbox-clean` still name `service`; they are out of this increment's scope and are flagged rather than edited. `depcruise-boundary.test.ts`: the "unresolved service import is forbidden" case tested the clause that is gone, so it goes with it. The store case is the one worth keeping — it is the only one exercising the RESOLVED half of the store ban — so its fixture moves from `store-postgres` to a `store-sqlite` stub that is deliberately not a real package, and the suite stops depending on whichever SQL adapter happens to exist. CLAUDE.md: the `[Service]` tag row named an area that no longer exists, and the status paragraph listed `@otta-sh/service` among the packages under `packages/`. The adapters row loses its postgres/sqlite/d1 examples for the same reason. CI needed no change and gets none: there is no service matrix and no service deploy job — `unit` and `integration` are both workspace-wide. Verified by grep, not assumed. The Postgres service container and `test:pg` stay: store-emdash still needs a real database for the no-oversell race. No changeset: nothing under `packages/*/src` changed, so no published package's behaviour moves. Verified: pnpm lint green (551 modules, no violations), pnpm typecheck green, pnpm format:check green, depcruise-boundary.test.ts 16/16 passing, changeset status exit 0. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01CQbJYWWm8tf8owshm7XRp8 --- .dependency-cruiser.cjs | 26 +++++++++++++---- CLAUDE.md | 10 +++---- .../plugin/test/depcruise-boundary.test.ts | 28 +++++++++++-------- 3 files changed, 41 insertions(+), 23 deletions(-) diff --git a/.dependency-cruiser.cjs b/.dependency-cruiser.cjs index 497af14..21011e3 100644 --- a/.dependency-cruiser.cjs +++ b/.dependency-cruiser.cjs @@ -23,7 +23,7 @@ module.exports = { comment: "@otta-sh/plugin's src (loaded inside the workerd sandbox) has NO DB/" + "driver, filesystem, process, socket or network-client surface, and no " + - "dependency on a SQL store, the service or a payment adapter. Its egress " + + "dependency on a SQL store or a payment adapter. Its egress " + "is the injected ctx.http; its commerce truth is the injected ctx.storage " + "(DEVELOPMENT.md §5, ADR-0018, sandbox-clean guard). The forbidden list is " + "a superset of domain-is-io-free's, plus HTTP/WS client libs (undici, " + @@ -58,12 +58,12 @@ module.exports = { "importing an ADAPTER can, which is why every adapter except one stays " + "banned. Second, that one exception: `store-[^/]+` in the packages clause " + "became `(?!store-emdash/)store-[^/]+`, so packages/store-emdash is " + - "admitted while store-postgres — and any store-* added later — is banned " + + "admitted while any other store-* — including any added later — is banned " + "by default rather than by anyone remembering to add it — and the same list " + "is mirrored into the two SPECIFIER clauses, not only the packages " + "clause, because pnpm's strict isolation leaves an UNDECLARED import as " + "a bare specifier that never resolves to a packages/ path: naming only " + - "admin-react there meant an undeclared @otta-sh/store-postgres, service " + + "admin-react there meant an undeclared @otta-sh/store-* " + "or payments-* import tripped nothing at all, which is the same class of " + "silent miss as the `^node:`-only builtin clause. store-emdash is " + "admissible because it carries no IO of its own: it is written against a " + @@ -87,11 +87,25 @@ module.exports = { "still be caught — by the driver and node-builtin clauses of this same " + "rule, which the carve-out does not touch. " + "packages/plugin/test/depcruise-boundary.test.ts pins both halves: these " + - "two admitted, a third payments-* package still forbidden.", + "two admitted, a third payments-* package still forbidden.\n\n" + + "FOURTH CHANGE (work order 02, INC-D3c): `service` is no longer named in " + + "any of the three clauses, because @otta-sh/service no longer EXISTS — " + + "INC-D3b deleted packages/service (and packages/store-postgres with it) " + + "once the service was folded into the plugin. A ban on a package that " + + "cannot be imported is a clause no fixture can exercise, so it rots " + + "silently: nothing would notice if it stopped matching, which is the same " + + "failure mode as the `^node:`-only builtin clause above. store-postgres " + + "was never named literally — it was caught by the " + + "`(?!store-emdash(/|$))store-[^/]+` lookahead, which is untouched and " + + "still bans every store-* but the one, so a store-postgres reintroduced " + + "tomorrow is forbidden on the day it is created. A reintroduced `service` " + + "package would NOT be, and that is deliberate: after the fold-in " + + "(ADR-0018) a second deployable is a decision that needs its own ADR, not " + + "something a lint rule should pre-judge on a name.", severity: "error", from: { path: "^packages/plugin/src" }, to: { - path: "(node_modules/(pg|pg-pool|kysely|better-sqlite3|workerd|hono|node-fetch|undici|axios|ws)(/|$)|node_modules/@otta-sh/((?!store-emdash(/|$))store-[^/]+|service|(?!payments-(stripe|x402)(/|$))payments-[^/]+|admin-react)(/|$)|^(pg|pg-pool|kysely|better-sqlite3|workerd|hono|node-fetch|undici|axios|ws)(/|$)|^@otta-sh/((?!store-emdash(/|$))store-[^/]+|service|(?!payments-(stripe|x402)(/|$))payments-[^/]+|admin-react)(/|$)|^(node:)?(fs|child_process|net|http|https|os|dgram|dns|tls|worker_threads|cluster|vm)(/|$)|^packages/((?!store-emdash(/|$))store-[^/]+|service|(?!payments-(stripe|x402)(/|$))payments-[^/]+|admin-react)/)", + path: "(node_modules/(pg|pg-pool|kysely|better-sqlite3|workerd|hono|node-fetch|undici|axios|ws)(/|$)|node_modules/@otta-sh/((?!store-emdash(/|$))store-[^/]+|(?!payments-(stripe|x402)(/|$))payments-[^/]+|admin-react)(/|$)|^(pg|pg-pool|kysely|better-sqlite3|workerd|hono|node-fetch|undici|axios|ws)(/|$)|^@otta-sh/((?!store-emdash(/|$))store-[^/]+|(?!payments-(stripe|x402)(/|$))payments-[^/]+|admin-react)(/|$)|^(node:)?(fs|child_process|net|http|https|os|dgram|dns|tls|worker_threads|cluster|vm)(/|$)|^packages/((?!store-emdash(/|$))store-[^/]+|(?!payments-(stripe|x402)(/|$))payments-[^/]+|admin-react)/)", }, }, { @@ -276,7 +290,7 @@ module.exports = { "static import would be a second one — compiled in, invisible to the " + "empty capability set and to the empty allowedHosts that are this " + "descriptor's only declared controls. It is also, for the server " + - "packages (domain/service/store/payments), Node and database code " + + "packages (domain/store/payments), Node and database code " + "reached from a module that ships to a BROWSER. So: no workspace " + "package, in either direction. The consequence is deliberate and has " + "one known bill to pay — INC-20 owes the React tier a formatMoney, and " + diff --git a/CLAUDE.md b/CLAUDE.md index ed1a5ac..bf1af0b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,9 +6,10 @@ Operational guide for Claude working in this repo. The **why** lives in conventions, and the guardrails that must not be crossed. > **Status: shipped, pre-1.0.** Phases 0–7 are merged and the full toolchain below is wired — -> `@otta-sh/domain`, `@otta-sh/service`, the storefront/admin adapters, and the EmDash plugin -> all exist under `packages/`. Treat the commands below as live, not aspirational; if one -> genuinely doesn't exist, say so rather than inventing output. +> `@otta-sh/domain`, the EmDash plugin (which now carries the commerce service in-process), +> `@otta-sh/store-emdash`, the payment adapters and the React admin all exist under +> `packages/`. Treat the commands below as live, not aspirational; if one genuinely doesn't +> exist, say so rather than inventing output. --- @@ -72,8 +73,7 @@ changed. Migrations are forward-only. | Area changed | Tag | |---|---| | `@otta-sh/domain` (ports, use-cases, invariants) | `[Domain]` | - | `@otta-sh/service` (REST API, HTTP serialization) | `[Service]` | - | Store/client/payment **adapters** (postgres, sqlite, d1, stripe, x402) | `[Adapters]` | + | Store/client/payment **adapters** (store-emdash, stripe, x402) | `[Adapters]` | | The EmDash **plugin** (storefront, Block Kit panel, sync hooks) | `[Plugin]` | | `sites/*` (the reference storefront site/theme) | `[Site]` | | Shared test/contract packages | `[Test]` | diff --git a/packages/plugin/test/depcruise-boundary.test.ts b/packages/plugin/test/depcruise-boundary.test.ts index 132758d..e76332c 100644 --- a/packages/plugin/test/depcruise-boundary.test.ts +++ b/packages/plugin/test/depcruise-boundary.test.ts @@ -48,7 +48,14 @@ const STUB_PACKAGES = [ "domain", "admin-react", "store-emdash", - "store-postgres", + // A hypothetical sibling SQL store, resolvable on purpose: it is how the + // RESOLVED half of the store ban (`^packages/…store-[^/]+/`) gets exercised, + // the bare-specifier half being covered by the unresolved `store-d1` cases. + // Deliberately not a real package name — the rule bans every store-* but + // store-emdash by lookahead, so the fixture must not depend on any particular + // adapter continuing to exist (`store-postgres`, which this stub replaced, + // was deleted in INC-D3b). + "store-sqlite", "payments-stripe", "payments-x402", "plugin", @@ -188,20 +195,17 @@ describe("plugin-is-sandbox-clean: what the plugin perimeter forbids", () => { ).toEqual(["plugin-is-sandbox-clean"]); }); - test("an unresolved service import is forbidden, for the same reason", () => { + test("a RESOLVED SQL store adapter is still forbidden — the narrowing admitted one store, not every store", () => { + // The counterpart to the case above: this name IS linked into the fixture's + // node_modules, so dependency-cruiser reports it as a `packages/…` path and + // the third clause is what has to catch it. INC-D3c dropped the deleted + // `service` case that used to sit here; the store ban is a lookahead over + // the whole family, so it is exercised by a stand-in rather than by whichever + // SQL adapter happens to exist this month. expect( rulesViolatedBy( "plugin", - 'import { stub } from "@otta-sh/service";\nexport const x = stub;\n', - ), - ).toEqual(["plugin-is-sandbox-clean"]); - }); - - test("a SQL store adapter is still forbidden — narrowing admitted one store, not every store", () => { - expect( - rulesViolatedBy( - "plugin", - 'import { stub } from "@otta-sh/store-postgres";\nexport const x = stub;\n', + 'import { stub } from "@otta-sh/store-sqlite";\nexport const x = stub;\n', ), ).toEqual(["plugin-is-sandbox-clean"]); }); From ae36b686ce8ed7e5a172c520f616895183a251df Mon Sep 17 00:00:00 2001 From: Vedanshu Date: Sun, 20 Sep 2026 14:49:53 +0000 Subject: [PATCH 2/2] [CI][Docs] Close the review findings on the INC-D3c reference trim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two files outside this increment's named scope carried the same rot it was opened to clear, and nothing later in the plan reaches them. CONTRIBUTING.md's PR-tag table was a copy of the one CLAUDE.md already had fixed: a `[Service]` row for a package that no longer exists, and an adapters row naming postgres/sqlite/d1. A contributor reading it would pick a tag for a deleted package. Fixed the same way CLAUDE.md was — the `[Service]` row is gone, and the adapters row names the three adapter packages that survive. DEVELOPMENT.md §3 claimed the REST API in `@otta-sh/service` mirrors the port 1:1 and that the contract suite runs against `HttpCommerceClient` over a live test server. Both were deleted with the service. CLAUDE.md's non-negotiables cite DEVELOPMENT.md as the authority for that section, so the two documents contradicted each other. Restated rather than dropped: what verification looks like now is the same behavioral suite, running every case it ever ran, against the one in-process tier over a real document store, with `ctx.http` bound to a rejecting stub. `domain-is-io-free` and `store-emdash-is-sandbox-clean` still hardcode `service`. That is outside this increment's scope and is a comment-level inaccuracy rather than a behavior change — a ban on a package that cannot be imported can never fire — so each now carries a one-line pointer to #291 instead of being edited here. The reported intermittent failure in depcruise-boundary.test.ts did not reproduce: ten consecutive standalone runs were 16/16 green. The harness has no shared mutable state to race on — one mkdtemp per run, every filesystem call synchronous, depcruise invoked through spawnSync, and cases within the file sequential. No test change. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01CQbJYWWm8tf8owshm7XRp8 --- .dependency-cruiser.cjs | 4 ++++ CONTRIBUTING.md | 3 +-- DEVELOPMENT.md | 12 ++++++++---- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/.dependency-cruiser.cjs b/.dependency-cruiser.cjs index 21011e3..7f73b97 100644 --- a/.dependency-cruiser.cjs +++ b/.dependency-cruiser.cjs @@ -6,6 +6,8 @@ module.exports = { forbidden: [ { name: "domain-is-io-free", + // TODO(#291): this rule still names the deleted `service` package, in the + // comment below and in the last clause of `to.path`. Tracked separately. comment: "@otta-sh/domain imports nothing with IO — no pg/kysely/better-sqlite3/hono/http, " + "and no dependency on adapter/service/plugin packages (DEVELOPMENT.md §3).", @@ -210,6 +212,8 @@ module.exports = { "the plugin rule's comment sets out. Every case this rule and " + "the plugin rule turn on are executed in " + "packages/plugin/test/depcruise-boundary.test.ts.", + // TODO(#291): this rule still names the deleted `service` package, in the + // comment above and in three clauses of `to.path`. Tracked separately. severity: "error", from: { path: "^packages/store-emdash/src" }, to: { diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ade68c5..9213eab 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -73,8 +73,7 @@ Pick the tag for the area your change touches: | Area changed | Tag | |---|---| | `@otta-sh/domain` (ports, use-cases, invariants) | `[Domain]` | -| `@otta-sh/service` (REST API, HTTP serialization) | `[Service]` | -| Store/client/payment **adapters** (postgres, sqlite, d1, stripe, x402) | `[Adapters]` | +| Store/client/payment **adapters** (store-emdash, stripe, x402) | `[Adapters]` | | The EmDash **plugin** (storefront, Block Kit panel, sync hooks) | `[Plugin]` | | Shared test/contract packages | `[Test]` | | CI / tooling / build | `[CI]` | diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index e0882d5..8bfd37d 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -68,10 +68,14 @@ domain is a build-breaking bug, not a code-review nit. - Enforce the boundary with a dependency check (dependency-cruiser or an import-restriction lint rule) wired into `lint`, so the layering can't rot silently. -- **HTTP mirrors the port 1:1.** The REST API in `@otta-sh/service` is a serialization of the - domain use-cases — no endpoint has semantics the port lacks, no status-code-as-logic. The - same client-side contract suite runs against `HttpCommerceClient` (over a live test - server) so the wire format can't drift from the port. +- **There is no wire to keep in step.** Commerce runs in-process: the plugin builds + `InProcessCommerceClient` through its single composition root, `makeCommerceClient`, which + binds the `@otta-sh/domain` use-cases to the `@otta-sh/store-emdash` stores over + `ctx.storage` (ADR-0018). No REST API, no `@otta-sh/service`, no serialization layer that + could drift from the port. The behavioral contract suite that used to run twice — once + over HTTP against a live test server, once in-process — still runs every one of those + cases, now against that single tier, over a real document store, with `ctx.http` bound to + a rejecting stub so an accidental egress fails the suite. - **Add an adapter only when a second real implementation exists.** No speculative `EmdashStore` / `InProcessCommerceClient` before the EmDash primitive ships.