test(rest): state IHttpRequest once, in a typed request test builder

packages/rest/src/http-request-test-builder.ts

@@ -0,0 +1,158 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * The one place this package builds an `IHttpRequest` for a test (#13377).
+ *
+ * Test layer only — nothing in `src/index.ts` reaches it, so tsup (entry:
+ * `src/index.ts`) never emits it into `dist` and it is not published. Same
+ * placement, and for the same reason, as `src/xlsx-test-loader.ts`.
+ *
+ * ## The defect, stated once
+ *
+ * `IHttpRequest` (`packages/spec/src/contracts/http-server.ts`) declares FIVE
+ * required members — `params`, `query`, `headers`, `method`, `path` — and a
+ * route handler's first parameter IS that interface. So a test that hands a
+ * handler an object literal owes all five. Before this file the package did
+ * neither of the two honest things: 151 of its 281 `.handler(` call sites cast
+ * the literal with `as any`, and four more sat in `test-typecheck-debt.json` as
+ * ledgered `TS2345`:
+ *
+ * ```
+ * src/rest.test.ts(2063,7): error TS2345: Argument of type
+ *   '{ params: { environmentId: string; object: string; }; query: {}; }'
+ *   is not assignable to parameter of type 'IHttpRequest'.
+ * ```
+ *
+ * ⚠️ No annotation repairs those four. Adding the missing members changes what
+ * the handler RECEIVES — the fixture's data, not its typing — so inventing
+ * values silently changes what each test measures. That is why this is one
+ * builder and not four edits, and it is why the defaults below are argued
+ * rather than picked.
+ *
+ * ## Why each default is the default
+ *
+ * **`method` and `path` are NOT defaulted — they are read off the route under
+ * test.** A constant default here is precisely the hazard #13377 names: a
+ * `path` that does not match the route under test makes a passing test measure
+ * something other than what it names, and `req.path` is live rather than
+ * decorative — `RestServer.enforceAuth` feeds it to `isAuthGateAllowlisted`
+ * (`src/rest-server.ts:1254`), where a wrong value decides whether the ADR-0069
+ * gate fires at all. Every call site already locates its route
+ * (`getRoutes().find(r => r.method === 'GET' && r.path === '…')`); handing that
+ * same route object here means the request and the route it is sent to CANNOT
+ * disagree, because one is derived from the other.
+ *
+ * **`path` is materialized from `params`, not copied from the pattern.** A
+ * transport hands the handler a CONCRETE path; `/api/v1/data/:object` is a
+ * pattern no request ever carries. So the builder substitutes the request's own
+ * `params` into the pattern's `:segments` — which also means `path` and
+ * `params` cannot drift apart, since one is computed from the other. A
+ * `:segment` with no matching param is REFUSED loudly instead of emitted: a
+ * path carrying a literal `:` is one no transport could produce, and quietly
+ * shipping it is exactly the plausible-looking value this card exists to
+ * prevent. A test that wants an unmatched path says so, by overriding `path`.
+ *
+ * **`headers: {}`** is the one required member with a genuinely neutral
+ * default. It means "a request that carried no headers", which is what every
+ * one of these fixtures intends, and every header read in this package is
+ * either optional-chained or an index access — both yield `undefined` against
+ * `{}`, identical to what the fixture meant by omitting it. It is also strictly
+ * better-formed than what the ledgered literals produced: they left
+ * `req.headers` `undefined`, so the unguarded `req.headers['if-none-match']`
+ * (`src/rest-server.ts:5232`) would have THROWN had those tests reached it.
+ *
+ * **`params: {}` / `query: {}`** — empty is the honest reading of "the fixture
+ * supplied none", and both are `Record`s every handler reads defensively.
+ *
+ * **`remoteAddress` is deliberately ABSENT, and has no default at all.** Its
+ * contract note (#4910) is explicit that it is "the TRANSPORT's own peer
+ * address … the unforgeable half of caller identification", which a client
+ * cannot influence and which the inbound rate limiter keys anonymous traffic
+ * off. A builder that supplied a plausible `'127.0.0.1'` would put a forged
+ * value in the one member whose entire worth is that it cannot be forged. The
+ * interface makes it optional because "not every runtime exposes it", so absent
+ * is both legal and true; a test about it states it.
+ *
+ * **`body` / `rawBody`** are optional and stay absent unless stated: a GET has
+ * no body, and the sites that have one pass it.
+ */
+
+import type { RouteHandler } from '@objectstack/core';
+
+/**
+ * The request type a handler is actually handed, read off the handler's own
+ * signature instead of spelled by hand — the discipline `xlsx-test-loader.ts`
+ * applies to its dependency, applied here to ours. It resolves to
+ * `IHttpRequest`; deriving it means this builder tracks whatever a handler's
+ * first parameter becomes, and a change to that contract fails HERE, loudly, in
+ * one file, instead of leaving a helper that still compiles and lies.
+ */
+type HandlerRequest = Parameters<RouteHandler>[0];
+
+/**
+ * The half of a mounted route this builder reads. Structurally what
+ * `RestServer.getRoutes()` returns, so a `find(…)` result is passed straight
+ * in — but declared narrowly on purpose: building a request must not be able to
+ * reach the handler it is building the request FOR.
+ */
+export interface RouteUnderTest {
+    /** HTTP verb as registered. */
+    readonly method: string;
+    /** Full wire path, with `:param` segments — the pattern, not a request. */
+    readonly path: string;
+}
+
+/**
+ * Whatever this call site's assertion is actually about. Every member is the
+ * contract's, so a site can state any of them — including `method` and `path`,
+ * which override the route's and are then a deliberate, reviewable divergence
+ * from the route the request is sent to.
+ */
+export type HttpRequestOverrides = Partial<HandlerRequest>;
+
+/**
+ * Build a complete `IHttpRequest` aimed at `route`, overriding only what this
+ * test is about.
+ *
+ * @param route     the mounted route the request is being sent to; supplies
+ *                  `method`, and the pattern `path` is materialized from.
+ * @param overrides the members this call site's assertion is about.
+ * @throws if the route's pattern has a `:param` that `overrides.params` does
+ *         not supply and no explicit `path` was given.
+ */
+export function httpRequestForRoute(
+    route: RouteUnderTest,
+    overrides: HttpRequestOverrides = {},
+): HandlerRequest {
+    const { params = {}, query = {}, headers = {}, method, path, ...optional } = overrides;
+    return {
+        ...optional,
+        params,
+        query,
+        headers,
+        method: method ?? route.method,
+        path: path ?? materializeRoutePath(route.path, params),
+    };
+}
+
+/**
+ * Substitute `params` into a route pattern's `:segments`, yielding the concrete
+ * path a transport would have produced for this request.
+ */
+function materializeRoutePath(pattern: string, params: Record<string, string>): string {
+    return pattern
+        .split('/')
+        .map((segment) => {
+            if (!segment.startsWith(':')) return segment;
+            const name = segment.slice(1);
+            if (!Object.prototype.hasOwnProperty.call(params, name)) {
+                throw new Error(
+                    `httpRequestForRoute: route '${pattern}' has a ':${name}' segment but params supplied no '${name}'. ` +
+                    `A path holding a literal ':' is one no transport produces — supply params.${name}, or state ` +
+                    `the 'path' override deliberately if the mismatch is what the test is about.`,
+                );
+            }
+            return params[name];
+        })
+        .join('/');
+}

packages/rest/src/meta-public-book-grant.test.ts

@@ -12,6 +12,7 @@
 
 import { describe, it, expect, vi } from 'vitest';
 import { RestServer } from './rest-server';
+import { httpRequestForRoute } from './http-request-test-builder.js';
 
 const PUBLIC_BOOK = { name: 'manual', label: 'Manual', audience: 'public', groups: [] };
 const ORG_BOOK = { name: 'internal', label: 'Internal', audience: 'org', groups: [] };
@@ -135,7 +136,7 @@ describe('the exemption does not widen past book/doc reads (#3963)', () => {
     const put = rest.getRoutes().find((r: any) => r.method === 'PUT' && r.path === ITEM);
     if (put) {
       const res = makeRes();
-      await put.handler({ method: 'PUT', params: { type: 'book', name: 'manual' }, query: {}, body: {} }, res);
+      await put.handler(httpRequestForRoute(put, { params: { type: 'book', name: 'manual' }, body: {} }), res);
       expect(res.statusCode).toBe(401);
       expect(protocol.saveMetaItem).not.toHaveBeenCalled();
     }

packages/rest/src/rest-batch-size-cap.test.ts

@@ -11,6 +11,7 @@
 
 import { describe, it, expect, vi } from 'vitest';
 import { RestServer } from './rest-server';
+import { httpRequestForRoute } from './http-request-test-builder.js';
 
 function createMockServer() {
   return {
@@ -149,12 +150,9 @@ describe('cross-object batch reports the cap like everyone else (#3939)', () =>
     const { rest, ql } = setup(5);
     const route = rest.getRoutes().find((r: any) => r.method === 'POST' && r.path === '/api/v1/batch');
     const res = makeRes();
-    await route!.handler({
-      method: 'POST',
-      params: {},
-      query: {},
+    await route!.handler(httpRequestForRoute(route!, {
       body: { operations: ids(6).map((id) => ({ object: 'invoice', action: 'delete', id })) },
-    }, res);
+    }), res);
 
     expect(res.statusCode).toBe(400);
     expect(res.body).toMatchObject({ code: 'BATCH_TOO_LARGE', count: 6, max: 5 });

packages/rest/src/rest.test.ts

@@ -6,6 +6,7 @@ import { RestServer, mapDataError } from './rest-server';
 import { createRestApiPlugin } from './rest-api-plugin';
 import type { RestApiPluginConfig } from './rest-api-plugin';
 import { loadXlsxWorkbook } from './xlsx-test-loader.js';
+import { httpRequestForRoute } from './http-request-test-builder.js';
 
 // ---------------------------------------------------------------------------
 // Mocks & Helpers
@@ -2060,7 +2061,7 @@ describe('RestServer project-scoped routing', () => {
 
     const res = { json: vi.fn(), status: vi.fn().mockReturnThis() };
     await listRoute!.handler(
-      { params: { environmentId: 'proj-123', object: 'task' }, query: {} },
+      httpRequestForRoute(listRoute!, { params: { environmentId: 'proj-123', object: 'task' } }),
       res,
     );
 
@@ -2085,7 +2086,7 @@ describe('RestServer project-scoped routing', () => {
 
     const res = { json: vi.fn(), status: vi.fn().mockReturnThis() };
     await unscoped!.handler(
-      { params: { object: 'task' }, query: {} },
+      httpRequestForRoute(unscoped!, { params: { object: 'task' } }),
       res,
     );
 

packages/rest/test-typecheck-debt.json

@@ -1,9 +1,7 @@
 {
   "_comment": "Per-file tsc error debt of the @objectstack/rest TEST layer (#5286). `tsconfig.test.json` compiles `src/**/*.test.ts` — which `tsconfig.json` excludes and therefore no gate ever read — and every file below still carries errors from before that gate existed. THIS FIELD IS GENERATED: every regeneration rewrites it from scripts/check-test-typecheck.mts, and the EXACT ratchet below requires a regeneration on every repair — so an edit made here is gone by the next one. Anything true of THIS package goes in the sibling `_note` field, which is authored, is preserved verbatim, and is never written by the generator (#12624). This comment states NO cause for the errors, deliberately: the classes differ per package and per file, they move as the debt is paid down, and a cause written here is rewritten verbatim into every ledger by every regeneration — so it outlives its own repair and cannot be corrected in the file where it is read. Measure instead, before repairing anything: `tsc --noEmit --pretty false -p tsconfig.test.json` in the package prints the real classes with their TS codes. EXACT ratchet, judged by re-running tsc: a file that gains errors is red, a file that loses them is red until its number is re-recorded, a file that reaches zero is red until its entry is deleted, and a file NOT listed here may have no errors at all. Regenerate with: pnpm --filter @objectstack/rest gen:test-typecheck-debt",
-  "_note": "Everything still recorded here is held by its own card, and none of it is an annotation repair. #13377 holds every entry that remains: the request literals this package builds by hand against IHttpRequest, which omit members that interface requires. Repairing them here would mean changing a fixture's data, so these entries shrink when that card lands and not before. The exceljs call that #13378 held is no longer in this ledger: that dependency declares its own module-local Buffer, which shadows Node's inside every exceljs signature, so no Node Buffer can be passed to Workbook.xlsx.load — the assertion that costs is now stated once, in src/xlsx-test-loader.ts, and every xlsx-reading test in this package goes through it.",
+  "_note": "Everything still recorded here is held by its own card, and none of it is an annotation repair. #13454 holds the one entry that remains: src/rest.test.ts's two hand-built IHttpResponse literals, which omit the send and header members that interface requires and whose status is not typed as returning the interface, so res.status(...).json(...) does not chain. WARNING, and the reason this note exists: the count did NOT move when #13377 landed, but the errors underneath it were replaced wholesale. This file was recorded at 2 before that card and measures 2 after, and neither of the two is the same error. tsc reports at most ONE argument-assignability error per call expression, so the request literals #13377 removed had been masking these response literals at the very same two call sites. Read that as the warning an EXACT ledger cannot give you itself: a per-file count measures a QUANTITY, never an identity, and a constant number is not evidence that nothing changed. The request literals #13377 held are gone from this package: the five members IHttpRequest requires are stated once, in src/http-request-test-builder.ts, and a request built there takes its method and its path from the route under test instead of from a default, so the two cannot disagree. The exceljs call that #13378 held is likewise no longer here: that dependency declares its own module-local Buffer, which shadows Node's inside every exceljs signature, so no Node Buffer can be passed to Workbook.xlsx.load - the assertion that costs is stated once, in src/xlsx-test-loader.ts, and every xlsx-reading test in this package goes through it.",
   "entries": {
-    "src/meta-public-book-grant.test.ts": 1,
-    "src/rest-batch-size-cap.test.ts": 1,
     "src/rest.test.ts": 2
   }
 }