refactor(service-i18n): collapse the four inline success builders behind a sendOk, retire the ratchet (#3973)

.changeset/i18n-consolidate-success-builder.md

@@ -0,0 +1,27 @@
+---
+---
+
+Consolidate `service-i18n`'s four inline success builders behind a `sendOk`, and
+retire the route-envelope ratchet that pinned them (#3973 option 1). Deliberately
+empty frontmatter: **no wire shape changes**, so there is nothing for a consumer to
+read in a CHANGELOG.
+
+#3636 put the declared `{ success: true, data }` envelope on all three i18n read
+routes, but built it inline in each — four call sites for one envelope half, while
+the error half had already been consolidated behind `sendError` (#3675). Those
+bodies were never wrong; the problem was the guard.
+
+`scripts/check-route-envelope.mjs` counts response write sites per module, so a
+consolidated module sits at a fixed two however many routes it grows. This one sat
+at five with a declared ratchet, because a fifth read route could have hand-rolled
+a fourth-dialect body and only a driven test would have caught it — and a driven
+test only covers the routes that existed when it was written. The four builders now
+collapse into one, and the module declares the same `2 / 1 / 1` as the other five.
+
+Guard: **6 conformant / 1 ratcheted / 1 exempt**, down from 5 / 2 / 1. The remaining
+ratchet is `share-link-routes.ts` (#3983), which needs its own consumer sweep.
+
+Also corrects a claim #3843 left in six suite headers: it said the repo-wide scan
+"found two immediately (#3973, #3983)". #3973 was not one of them — i18n's
+unconsolidated builder was already known — the two it actually surfaced were
+`share-link-routes.ts` and the dev-only `hmr-routes.ts`.

packages/rest/src/external-datasource-envelope.conformance.test.ts

@@ -25,7 +25,8 @@
  * `pnpm check:route-envelope` in CI. It sits outside any package on purpose: the
  * three predecessors of that scan were per-package, which structurally cannot
  * notice a route module nobody thought to convert, and two such modules turned up
- * the moment it went repo-wide (#3973, #3983).
+ * the moment it went repo-wide: `share-link-routes.ts` (#3983) and the dev-only
+ * `hmr-routes.ts`, neither of them in #3843's hand-written survey.
  *
  * What stays here is the half that has to live next to the routes it drives:
  * every branch driven, every body parsed against the real spec schemas.

packages/services/service-datasource/src/__tests__/envelope.conformance.test.ts

@@ -27,7 +27,8 @@
  * `pnpm check:route-envelope` in CI. It sits outside any package on purpose: the
  * three predecessors of that scan were per-package, which structurally cannot
  * notice a route module nobody thought to convert, and two such modules turned up
- * the moment it went repo-wide (#3973, #3983).
+ * the moment it went repo-wide: `share-link-routes.ts` (#3983) and the dev-only
+ * `hmr-routes.ts`, neither of them in #3843's hand-written survey.
  *
  * What stays here is the half that has to live next to the routes it drives:
  * every branch driven, every body parsed against the real spec schemas.

packages/services/service-i18n/src/error-envelope.conformance.test.ts

@@ -22,7 +22,8 @@
  *
  * That move mattered more than deduplication. A per-package scan structurally
  * cannot notice a module nobody thought to convert, and going repo-wide found two
- * immediately (#3973, #3983). It also dropped the regex: the old block stripped
+ * immediately — `share-link-routes.ts` (#3983) and the dev-only `hmr-routes.ts`,
+ * neither of them in #3843's hand-written survey. It also dropped the regex: the old block stripped
  * comments with `String.replace`, which ate `//` inside string literals and
  * truncated the rest of that line — response writes included — and counted
  * `c.req.json()` (a request READ) as an unenveloped response. The AST has neither

packages/services/service-i18n/src/i18n-service-plugin.ts

@@ -30,6 +30,27 @@ function sendError(res: IHttpResponse, status: number, code: string, message: st
   res.status(status).json({ success: false, error: { code, message } });
 }
 
+/**
+ * Emit a success body in the DECLARED envelope — `BaseResponseSchema`
+ * (`packages/spec/src/api/contract.zod.ts`), i.e. `{ success: true, data }`.
+ *
+ * #3636 put the right shape on all three read routes, but built it inline in each
+ * one — four call sites for one envelope half, while the error half had already
+ * been consolidated behind `sendError` (#3675). Those bodies were never wrong;
+ * this is about the GUARD, not the wire.
+ *
+ * `scripts/check-route-envelope.mjs` counts response write sites per module, so a
+ * consolidated module sits at a fixed two no matter how many routes it grows. This
+ * one sat at five with a declared ratchet (#3973) precisely because a fifth read
+ * route could have hand-rolled a fourth-dialect body and only a driven test would
+ * have caught it — and a driven test only covers the routes that existed when it
+ * was written. Collapsing the four inline builders into this one retires that
+ * ratchet: the module now declares the same `2 / 1 / 1` as the other five.
+ */
+function sendOk(res: IHttpResponse, data: unknown): void {
+  res.json({ success: true, data });
+}
+
 /**
  * Configuration options for the I18nServicePlugin.
  */
@@ -187,7 +208,7 @@ export class I18nServicePlugin implements Plugin {
         // copy is what let them answer in different shapes (#3833's lesson,
         // one route over).
         const locales = toLocaleDescriptors(i18n.getLocales(), i18n.getDefaultLocale?.() ?? 'en');
-        res.json({ success: true, data: { locales } });
+        sendOk(res, { locales });
       } catch (error: any) {
         sendError(res, 500, 'INTERNAL', error?.message ?? 'Internal error');
       }
@@ -202,7 +223,7 @@ export class I18nServicePlugin implements Plugin {
           return;
         }
         const translations = i18n.getTranslations(locale);
-        res.json({ success: true, data: { locale, translations } });
+        sendOk(res, { locale, translations });
       } catch (error: any) {
         sendError(res, 500, 'INTERNAL', error?.message ?? 'Internal error');
       }
@@ -223,7 +244,7 @@ export class I18nServicePlugin implements Plugin {
         if (hasGetFieldLabels) {
           const labels = (i18n as II18nService & { getFieldLabels(obj: string, loc: string): Record<string, string> })
             .getFieldLabels(objectName, locale);
-          res.json({ success: true, data: { object: objectName, locale, labels } });
+          sendOk(res, { object: objectName, locale, labels });
         } else {
           // Fallback: read field labels out of the locale's translation data.
           // That data is NESTED (`objects.<obj>.fields.<field>.label`) — the
@@ -234,7 +255,7 @@ export class I18nServicePlugin implements Plugin {
           // drift out of shape again the way it did after that fix (#3833).
           const data = i18n.getTranslations(locale) as TranslationData | undefined;
           const labels = resolveObjectFieldLabels(data, objectName);
-          res.json({ success: true, data: { object: objectName, locale, labels } });
+          sendOk(res, { object: objectName, locale, labels });
         }
       } catch (error: any) {
         sendError(res, 500, 'INTERNAL', error?.message ?? 'Internal error');

packages/services/service-settings/src/envelope.conformance.test.ts

@@ -24,7 +24,8 @@
  * `pnpm check:route-envelope` in CI. It sits outside any package on purpose: the
  * three predecessors of that scan were per-package, which structurally cannot
  * notice a route module nobody thought to convert, and two such modules turned up
- * the moment it went repo-wide (#3973, #3983).
+ * the moment it went repo-wide: `share-link-routes.ts` (#3983) and the dev-only
+ * `hmr-routes.ts`, neither of them in #3843's hand-written survey.
  *
  * What stays here is the half that has to live next to the routes it drives:
  * every branch driven, every body parsed against the real spec schemas.

packages/services/service-storage/src/error-envelope.conformance.test.ts

@@ -26,7 +26,8 @@
  *
  * That move mattered more than deduplication. A per-package scan structurally
  * cannot notice a module nobody thought to convert, and going repo-wide found two
- * immediately (#3973, #3983). It also dropped the regex: the old block stripped
+ * immediately — `share-link-routes.ts` (#3983) and the dev-only `hmr-routes.ts`,
+ * neither of them in #3843's hand-written survey. It also dropped the regex: the old block stripped
  * comments with `String.replace`, which ate `//` inside string literals and
  * truncated the rest of that line — response writes included — and counted
  * `c.req.json()` (a request READ) as an unenveloped response. The AST has neither

packages/services/service-storage/src/success-envelope.conformance.test.ts

@@ -43,7 +43,8 @@
  *
  * That move mattered more than deduplication. A per-package scan structurally
  * cannot notice a module nobody thought to convert, and going repo-wide found two
- * immediately (#3973, #3983). It also dropped the regex: the old block stripped
+ * immediately — `share-link-routes.ts` (#3983) and the dev-only `hmr-routes.ts`,
+ * neither of them in #3843's hand-written survey. It also dropped the regex: the old block stripped
  * comments with `String.replace`, which ate `//` inside string literals and
  * truncated the rest of that line — response writes included — and counted
  * `c.req.json()` (a request READ) as an unenveloped response. The AST has neither

scripts/check-route-envelope.mjs

@@ -30,9 +30,10 @@
  * #3675 / #3689 shipped this as a regex block copied into each converted
  * package. Three copies was the signal it wanted lifting (#3843 option 3), and
  * lifting it to a repo-wide scan buys the thing per-package copies structurally
- * cannot: **a module nobody thought to convert still gets audited.** Both
- * modules in the RATCHET table below were found that way — neither is in
- * #3843's hand-written survey.
+ * cannot: **a module nobody thought to convert still gets audited.** Two modules
+ * in the table below were found exactly that way, neither of them in #3843's
+ * hand-written survey — `share-link-routes.ts` (ratcheted, #3983) and
+ * `hmr-routes.ts` (exempt).
  *
  * A module discovered by the scan but absent from the table is an ERROR, not a
  * default: silently applying `2 / 1 / 1` to an unknown module would let a new
@@ -97,6 +98,10 @@ const MODULES = {
   'packages/services/service-datasource/src/admin-routes.ts': { responses: 2, ok: 1, err: 1 },
   'packages/rest/src/external-datasource-routes.ts': { responses: 2, ok: 1, err: 1 },
   'packages/rest/src/package-routes.ts': { responses: 2, ok: 1, err: 1 },
+  // Consolidated by #3973: #3636 put the right envelope on its three read routes
+  // but built it inline in four places, so this module carried a ratchet at 5 / 4 / 1
+  // until those collapsed behind a `sendOk`.
+  'packages/services/service-i18n/src/i18n-service-plugin.ts': { responses: 2, ok: 1, err: 1 },
 
   // ── Exempt ──────────────────────────────────────────────────────────────
 
@@ -114,17 +119,7 @@ const MODULES = {
     exempt: 'dev-only SSE endpoint (/api/v1/dev/*), not on the SDK surface',
   },
 
-  // ── Ratchets: real, tracked, NOT blessed ────────────────────────────────
-
-  // The error half IS consolidated behind `sendError` (#3675). The success half
-  // is not: each of four read routes builds `{ success: true, data }` inline.
-  // Those bodies are CORRECT — `packages/runtime/src/i18n-success-envelope
-  // .conformance.test.ts` drives them — so this is not envelope drift. It is an
-  // unconsolidated builder, i.e. a weaker guard: a fifth read route could get
-  // the shape wrong and only a driven test would notice.
-  'packages/services/service-i18n/src/i18n-service-plugin.ts': {
-    responses: 5, ok: 4, err: 1, ratchet: '#3973',
-  },
+  // ── Ratchet: real, tracked, NOT blessed ─────────────────────────────────
 
   // Found BY THIS SCAN, absent from #3843's survey — the fifth drifting module.
   // `sendError` already nests `{ code, message }` (that is why #3675's changeset