fix(client)!: every limit query-parameter emitter sends what the caller wrote

.changeset/19567-client-limit-guards.md

@@ -0,0 +1,41 @@
+---
+'@objectstack/client': minor
+---
+
+fix(client)!: every `limit` query-parameter emitter sends what the caller wrote, so `{ limit: 0 }` is no longer silently swapped for the server's default window on three methods (#19567)
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) a runtime behaviour change in the SDK's query-string building: no spec key, export, type or stored shape is added, removed or renamed, so objectstack migrate meta has nothing to rewrite, and the one caller-side adjustment is to leave limit out rather than pass 0 or null -->
+
+**BREAKING** — a narrowing, shipped as `minor` under the launch-window convention
+(`check-changeset-no-major` refuses `major` until GA; the breaking-ness is carried by
+this banner and the ADR-0087 disposition above, not by the level). A call that used
+to answer `200` can now answer `400`.
+
+**What changed.** The SDK set the `limit` query parameter behind three different
+guards, so one input got a different answer depending on the method. Seven methods
+already sent every value except `undefined`. Three used a truthy test, so `0` and
+`NaN` never left the client and the server answered `200` with its default window —
+rows the caller did not ask for. Four used `!= null`, which dropped an untyped `null`
+as the truthy three did, while the seven others sent it as the text `null`. All
+fourteen emitters now leave only an absent (`undefined`) `limit` off the wire and send
+everything else as written; the door that declares the bound decides. The SDK itself
+still does not validate `limit`.
+
+| method | what changes on the wire |
+|:--|:--|
+| `automation.runs.list` | `0`, `NaN` and `null` are now sent; the door declares `1..100` and refuses all three with `400 VALIDATION_FAILED` |
+| `notifications.list` | `0`, `NaN` and `null` are now sent; the inbox clamps `0` to one row (its declared clamp into `1..200`) and refuses `NaN` / `null` with `400` |
+| `environments.listRevisions` | `0`, `NaN` and `null` are now sent to the control-plane door |
+| `automation.listRuns`, `environment(id).automation.listRuns` | an untyped `null` is now sent and refused with `400` (`0` and `NaN` were already sent) |
+| `data.listImportJobs`, `environment(id).data.listImportJobs` | an untyped `null` is now sent; the door reads it as its default of 50, so the answer does not move |
+
+`meta.getHistory`, `meta.getAudit`, `search`, `ai.conversations.list`,
+`ai.pendingActions.list`, `data.export` and `environment(id).meta.getHistory`
+already sent every value except `undefined`, and are unchanged.
+
+**If you relied on the old behaviour:** a call that passed `limit: 0` (or `null`) to
+mean "the server's default window" should leave `limit` out instead. Every `limit`
+here is typed `number | undefined`, so `null` only reaches these methods through an
+untyped caller.

packages/client/src/index.ts

@@ -3240,7 +3240,7 @@ export class ObjectStackClient {
      */
     listRevisions: async (id: string, opts?: { limit?: number; cursor?: string; branch?: string }) => {
       const params = new URLSearchParams();
-      if (opts?.limit) params.set('limit', String(opts.limit));
+      if (opts?.limit !== undefined) params.set('limit', String(opts.limit));
       if (opts?.cursor) params.set('cursor', opts.cursor);
       if (opts?.branch) params.set('branch', opts.branch);
       const qs = params.toString();
@@ -5547,20 +5547,21 @@ export class ObjectStackClient {
            * REFUSED with `400 VALIDATION_FAILED`, never clamped — so raise it
            * deliberately to see further back.
            *
-           * ⚠️ `0` and `NaN` are the exception, and they are dropped rather
-           * than refused: the guard below is truthy, so a falsy `limit` never
-           * leaves the client and the server answers its DEFAULT window
-           * instead. `-5`, `1.5` and `101` are truthy, are sent, and are
-           * refused. The two `listRuns` surfaces guard on `!= null` and do
-           * send `0`.
+           * There is no exception: the SDK does not judge `limit`, it sends
+           * whatever it is given and leaves only an ABSENT (`undefined`) value
+           * off the wire. `0`, `NaN`, `-5`, `1.5` and `101` all reach the door
+           * and are all refused there — none of them is swapped for the
+           * default window. An untyped `null` is outside the declared type; it
+           * is sent as the text `null` and refused as not a whole number. The
+           * two `listRuns` surfaces guard the same way.
            *
            * There is no continuation token — read `hasMore` to learn whether
            * the window was short.
            */
           list: async (flowName: string, options?: { limit?: number }): Promise<{ runs: ExecutionLog[]; hasMore: boolean }> => {
               const route = this.getRoute('automation');
               const params = new URLSearchParams();
-              if (options?.limit) params.set('limit', String(options.limit));
+              if (options?.limit !== undefined) params.set('limit', String(options.limit));
               const qs = params.toString();
               const res = await this.fetch(`${this.baseUrl}${route}/${flowName}/runs${qs ? `?${qs}` : ''}`);
               return this.unwrapResponse(res);
@@ -5639,7 +5640,7 @@ export class ObjectStackClient {
       ): Promise<T> => {
           const route = this.getRoute('automation');
           const params = new URLSearchParams();
-          if (opts?.limit != null) params.set('limit', String(opts.limit));
+          if (opts?.limit !== undefined) params.set('limit', String(opts.limit));
           // [#7359] The route's declared `status` filter, now that the boundary
           // honours it instead of dropping it. Until this card the typed client
           // could not send it at all — which is why nothing had tripped over the
@@ -6479,7 +6480,7 @@ export class ObjectStackClient {
       const params = new URLSearchParams();
       if (options?.read !== undefined) params.set('read', String(options.read));
       if (options?.type) params.set('type', options.type);
-      if (options?.limit) params.set('limit', String(options.limit));
+      if (options?.limit !== undefined) params.set('limit', String(options.limit));
       const qs = params.toString();
       const res = await this.fetch(`${this.baseUrl}${route}${qs ? `?${qs}` : ''}`);
       return this.unwrapResponse<ListNotificationsResponse>(res);
@@ -7083,7 +7084,7 @@ export class ObjectStackClient {
         const qs = new URLSearchParams();
         if (query.object) qs.set('object', query.object);
         if (query.status) qs.set('status', query.status);
-        if (query.limit != null) qs.set('limit', String(query.limit));
+        if (query.limit !== undefined) qs.set('limit', String(query.limit));
         if (query.offset != null) qs.set('offset', String(query.offset));
         const suffix = qs.toString() ? `?${qs.toString()}` : '';
         const res = await this.fetch(`${this.baseUrl}${route}/import/jobs${suffix}`);
@@ -7929,7 +7930,7 @@ export class ScopedEnvironmentClient {
       const qs = new URLSearchParams();
       if (query.object) qs.set('object', query.object);
       if (query.status) qs.set('status', query.status);
-      if (query.limit != null) qs.set('limit', String(query.limit));
+      if (query.limit !== undefined) qs.set('limit', String(query.limit));
       if (query.offset != null) qs.set('offset', String(query.offset));
       const suffix = qs.toString() ? `?${qs.toString()}` : '';
       const res = await this.parent._fetch(this.dataUrl(`/import/jobs${suffix}`));
@@ -8123,7 +8124,7 @@ export class ScopedEnvironmentClient {
       opts?: { limit?: number; status?: ExecutionStatus },
     ): Promise<T> => {
       const params = new URLSearchParams();
-      if (opts?.limit != null) params.set('limit', String(opts.limit));
+      if (opts?.limit !== undefined) params.set('limit', String(opts.limit));
       // [#7359] — see the sibling `listRuns` alias above.
       if (opts?.status) params.set('status', opts.status);
       const qs = params.toString();

packages/client/src/limit-guard-family.test.ts

@@ -0,0 +1,107 @@
+import { describe, it, expect, vi } from 'vitest';
+import { readFileSync } from 'node:fs';
+import { ObjectStackClient } from './index';
+
+/**
+ * The `limit` query-parameter emitters of this SDK, pinned as ONE family (#19567).
+ *
+ * They used to guard three different ways — a truthy test, `!= null`, and
+ * `!== undefined` — so one input got a different answer by method:
+ * `{ limit: 0 }` was dropped by some (the server then answered `200` with its
+ * DEFAULT window, a page the caller did not ask for) and sent by the others
+ * (`400` from a door that declares `min(1)`); `null` was dropped by some and
+ * sent by the rest.
+ *
+ * The rule now is the one the declared doors need: the SDK does not judge
+ * `limit`. Only an ABSENT value (`undefined`) stays off the wire; `0`, `NaN`
+ * and an untyped `null` all leave the client exactly as written, and the door
+ * that declares the bound is the one that refuses. Every emitter is driven
+ * through the real method against a stubbed `fetch`, and the assertion reads
+ * the URL it actually requested.
+ *
+ * `null` is outside every emitter's declared type (`limit?: number`); it is
+ * driven through a cast because an untyped caller can still pass it. Pinning
+ * it as SENT is deliberate: dropping it again would turn the `400` those
+ * doors answer today into a silent `200` with a default page.
+ */
+
+type Emitter = {
+  /** The public path a caller writes. */
+  label: string;
+  /** Drives the method with `{ limit }` (or its positional equivalent). */
+  call: (client: ObjectStackClient, limit: number | undefined) => Promise<unknown>;
+};
+
+const ENV = 'env_limit';
+
+const EMITTERS: readonly Emitter[] = [
+  { label: 'meta.getHistory', call: (c, limit) => c.meta.getHistory('object', 'task', { limit }) },
+  { label: 'meta.getAudit', call: (c, limit) => c.meta.getAudit('object', 'task', { limit }) },
+  { label: 'environments.listRevisions', call: (c, limit) => c.environments.listRevisions(ENV, { limit }) },
+  { label: 'automation.runs.list', call: (c, limit) => c.automation.runs.list('my_flow', { limit }) },
+  { label: 'automation.listRuns', call: (c, limit) => c.automation.listRuns('my_flow', { limit }) },
+  { label: 'search', call: (c, limit) => c.search('acme', { limit }) },
+  { label: 'notifications.list', call: (c, limit) => c.notifications.list({ limit }) },
+  { label: 'ai.conversations.list', call: (c, limit) => c.ai.conversations.list({ limit }) },
+  { label: 'ai.pendingActions.list', call: (c, limit) => c.ai.pendingActions.list({ limit }) },
+  { label: 'data.listImportJobs', call: (c, limit) => c.data.listImportJobs({ limit }) },
+  { label: 'data.export', call: (c, limit) => c.data.export('task', { limit }) },
+  { label: 'environment().meta.getHistory', call: (c, limit) => c.environment(ENV).meta.getHistory('object', 'task', { limit }) },
+  { label: 'environment().data.listImportJobs', call: (c, limit) => c.environment(ENV).data.listImportJobs({ limit }) },
+  { label: 'environment().automation.listRuns', call: (c, limit) => c.environment(ENV).automation.listRuns('my_flow', { limit }) },
+];
+
+/** The URL the method requested, after it has run to completion or failed on the stub body. */
+async function requestedUrl(emitter: Emitter, limit: number | undefined): Promise<URL> {
+  const fetchMock = vi.fn().mockResolvedValue({
+    ok: true,
+    status: 200,
+    statusText: 'OK',
+    json: async () => ({ success: true, data: {} }),
+    headers: new Headers(),
+  });
+  const client = new ObjectStackClient({ baseUrl: 'http://localhost:3000', fetch: fetchMock });
+  // Only the request matters here; a method that trips over the stub body
+  // AFTER fetching has still told us what it sent.
+  await emitter.call(client, limit).then(() => undefined, () => undefined);
+  expect(fetchMock, `${emitter.label} never reached fetch`).toHaveBeenCalledTimes(1);
+  return new URL(String(fetchMock.mock.calls[0][0]));
+}
+
+describe('the limit emitter family sends what the caller wrote', () => {
+  it.each(EMITTERS.map((e) => [e.label, e] as const))('%s: an absent limit stays off the wire', async (_label, emitter) => {
+    const url = await requestedUrl(emitter, undefined);
+    expect(url.searchParams.has('limit')).toBe(false);
+  });
+
+  it.each(EMITTERS.map((e) => [e.label, e] as const))('%s: an ordinary limit is sent', async (_label, emitter) => {
+    const url = await requestedUrl(emitter, 20);
+    expect(url.searchParams.getAll('limit')).toEqual(['20']);
+  });
+
+  it.each(EMITTERS.map((e) => [e.label, e] as const))('%s: limit 0 is sent, not swapped for the default window', async (_label, emitter) => {
+    const url = await requestedUrl(emitter, 0);
+    expect(url.searchParams.getAll('limit')).toEqual(['0']);
+  });
+
+  it.each(EMITTERS.map((e) => [e.label, e] as const))('%s: limit NaN is sent, not swapped for the default window', async (_label, emitter) => {
+    const url = await requestedUrl(emitter, Number.NaN);
+    expect(url.searchParams.getAll('limit')).toEqual(['NaN']);
+  });
+
+  it.each(EMITTERS.map((e) => [e.label, e] as const))('%s: an untyped null is sent as written, for the door to refuse', async (_label, emitter) => {
+    const url = await requestedUrl(emitter, null as unknown as number);
+    expect(url.searchParams.getAll('limit')).toEqual(['null']);
+  });
+
+  it('drives every limit emitter in index.ts — a new one has to join this table', () => {
+    // The census the table above must cover: every `set('limit', …)` call plus
+    // the one emitter that spells the key inside a template literal. A new
+    // emitter that is not added to EMITTERS changes this count, so it cannot
+    // arrive with a fourth guard spelling unpinned.
+    const source = readFileSync(new URL('./index.ts', import.meta.url), 'utf8');
+    const setCalls = source.match(/\.set\('limit',/g) ?? [];
+    const templateEmitters = source.match(/\?limit=\$\{/g) ?? [];
+    expect(setCalls.length + templateEmitters.length).toBe(EMITTERS.length);
+  });
+});