fix: honor explicit list_pages limit for local callers, warn on remote clamp, thread offset (#2591)

gbrain list --limit 100000 silently returned 100 rows (default 50) with
no warning, and --offset was accepted but dropped at the op layer even
though PageFilters has supported it all along.

- Local CLI callers (ctx.remote === false, the same trust boundary that
  already bypasses scope enforcement) get an explicit limit above 100
  honored — full enumeration is a legitimate local operation.
- Remote MCP/OAuth callers keep the 100-row DoS cap, now loud: one
  logger.warn (stderr, stdout stays script-clean) with both numbers,
  parity with the three search-path clamp warnings.
- offset is declared as a param (so the CLI coerces it to number) and
  threaded to engine.listPages for real pagination.


Claude-Session: https://claude.ai/code/session_01Vswwe1y5fQbJWfbaSK3enT

Co-authored-by: deacon-botdoctor <291411030+deacon-botdoctor@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

src/core/operations.ts

@@ -1388,7 +1388,11 @@ const list_pages: Operation = {
   params: {
     type: { type: 'string', description: 'Filter by page type' },
     tag: { type: 'string', description: 'Filter by tag' },
-    limit: { type: 'number', description: 'Max results (default 50)' },
+    limit: { type: 'number', description: 'Max results (default 50; remote callers are capped at 100)' },
+    offset: {
+      type: 'number',
+      description: 'Skip first N rows (pagination). Engine-supported since PageFilters gained offset; previously accepted at the CLI and silently dropped.',
+    },
     // v0.29 — surface filter that already exists on PageFilters.
     updated_after: {
       type: 'string',
@@ -1415,10 +1419,36 @@ const list_pages: Operation = {
     // were ignored at this op handler and the engine returned every source's
     // pages indiscriminately.
     const scope = sourceScopeOpts(ctx);
+    // The 100-row cap exists to protect remote MCP/OAuth transports from
+    // unbounded result dumps. Local CLI callers (ctx.remote === false — the
+    // same trust boundary that already bypasses scope enforcement, see the
+    // Operation.scope doc above) own the machine, and a full enumeration is a
+    // legitimate local operation, so an explicit limit above 100 is honored.
+    // Anything that is not strictly `false` stays remote/untrusted (defense
+    // in depth, matching the ctx.remote contract).
+    const requestedLimit = p.limit as number | undefined;
+    const isLocal = ctx.remote === false;
+    const limit = isLocal
+      ? clampSearchLimit(requestedLimit, 50, Number.MAX_SAFE_INTEGER)
+      : clampSearchLimit(requestedLimit, 50, 100);
+    if (!isLocal && requestedLimit !== undefined && Number.isFinite(requestedLimit) && requestedLimit > limit) {
+      // Loud clamp, parity with the three search paths ("search limit clamped
+      // from N to 100"). logger.warn goes to stderr — `list` stdout is
+      // tab-separated and consumed by scripts, so it must stay clean.
+      ctx.logger.warn(`[gbrain] Warning: list limit clamped from ${requestedLimit} to ${limit}; use offset to paginate`);
+    }
+    // Thread offset through — PageFilters has supported it all along; the op
+    // layer just never passed it, so `--offset` was accepted and ignored.
+    const requestedOffset = p.offset as number | undefined;
+    const offset =
+      requestedOffset !== undefined && Number.isFinite(requestedOffset) && requestedOffset > 0
+        ? Math.floor(requestedOffset)
+        : undefined;
     const pages = await ctx.engine.listPages({
       type: p.type as any,
       tag: p.tag as string,
-      limit: clampSearchLimit(p.limit as number | undefined, 50, 100),
+      limit,
+      offset,
       includeDeleted: (p.include_deleted as boolean) === true,
       updated_after: typeof p.updated_after === 'string' ? p.updated_after : undefined,
       sort,

test/list-clamp-local-trust.test.ts

@@ -0,0 +1,132 @@
+/**
+ * list_pages clamp local-trust + offset threading — op-level coverage.
+ *
+ * Pins (upstream draft "gbrain list silently clamps --limit to 100"):
+ *   - Local callers (ctx.remote === false) get an explicit limit above 100
+ *     honored — full enumeration is a legitimate local operation.
+ *   - Remote callers keep the 100-row DoS cap, and the clamp is now LOUD:
+ *     exactly one logger.warn (stderr, never stdout) naming both numbers.
+ *   - Defaults unchanged: no limit → 50 rows for both local and remote.
+ *   - `offset` threads through to the engine (PageFilters supported it all
+ *     along; the op layer dropped it, so `--offset` was silently ignored).
+ */
+
+import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
+import { PGLiteEngine } from '../src/core/pglite-engine.ts';
+import { operationsByName } from '../src/core/operations.ts';
+import type { OperationContext } from '../src/core/operations.ts';
+
+const SEED_COUNT = 120; // must exceed the remote cap (100) and the default (50)
+
+let engine: PGLiteEngine;
+
+beforeAll(async () => {
+  engine = new PGLiteEngine();
+  await engine.connect({});
+  await engine.initSchema();
+  for (let i = 0; i < SEED_COUNT; i++) {
+    // Zero-padded slugs → sort:'slug' gives a deterministic order for the
+    // offset assertions regardless of insert timestamps.
+    await engine.putPage(`listclamp/page-${String(i).padStart(3, '0')}`, {
+      type: 'note',
+      title: `Page ${i}`,
+      compiled_truth: 'body',
+    });
+  }
+});
+
+afterAll(async () => {
+  if (engine) await engine.disconnect();
+});
+
+function mkCtx(overrides: Partial<OperationContext> = {}): {
+  ctx: OperationContext;
+  warnings: string[];
+} {
+  const warnings: string[] = [];
+  const ctx = {
+    engine,
+    config: {} as any,
+    logger: {
+      info: () => {},
+      warn: (msg: string) => warnings.push(msg),
+      error: () => {},
+    } as any,
+    dryRun: false,
+    remote: false,
+    ...overrides,
+  } as OperationContext;
+  return { ctx, warnings };
+}
+
+const op = () => operationsByName['list_pages'];
+
+describe('list_pages — local callers escape the 100-row clamp', () => {
+  test('remote=false with limit 100000 returns every page', async () => {
+    const { ctx, warnings } = mkCtx({ remote: false });
+    const rows = (await op().handler(ctx, { limit: 100000 })) as any[];
+    expect(rows.length).toBe(SEED_COUNT);
+    expect(warnings.length).toBe(0);
+  });
+
+  test('remote=false default (no limit) is still 50 — default unchanged', async () => {
+    const { ctx } = mkCtx({ remote: false });
+    const rows = (await op().handler(ctx, {})) as any[];
+    expect(rows.length).toBe(50);
+  });
+});
+
+describe('list_pages — remote callers keep the cap, loudly', () => {
+  test('remote=true with limit 100000 returns 100 and warns once with both numbers', async () => {
+    const { ctx, warnings } = mkCtx({ remote: true });
+    const rows = (await op().handler(ctx, { limit: 100000 })) as any[];
+    expect(rows.length).toBe(100);
+    expect(warnings.length).toBe(1);
+    expect(warnings[0]).toContain('list limit clamped from 100000 to 100');
+  });
+
+  test('remote=true with limit <= 100 does not warn', async () => {
+    const { ctx, warnings } = mkCtx({ remote: true });
+    const rows = (await op().handler(ctx, { limit: 60 })) as any[];
+    expect(rows.length).toBe(60);
+    expect(warnings.length).toBe(0);
+  });
+
+  test('anything not strictly remote===false is treated as remote (defense in depth)', async () => {
+    // ctx.remote contract: consumers treat non-false as untrusted even if the
+    // type is bypassed via cast.
+    const { ctx, warnings } = mkCtx({ remote: undefined as any });
+    const rows = (await op().handler(ctx, { limit: 100000 })) as any[];
+    expect(rows.length).toBe(100);
+    expect(warnings.length).toBe(1);
+  });
+});
+
+describe('list_pages — offset threads through (regression: was silently ignored)', () => {
+  test('offset shifts the window under sort=slug', async () => {
+    const { ctx } = mkCtx({ remote: false });
+    const all = (await op().handler(ctx, { limit: 100000, sort: 'slug' })) as any[];
+    const paged = (await op().handler(ctx, { limit: 10, offset: 5, sort: 'slug' })) as any[];
+    expect(paged.length).toBe(10);
+    expect(paged.map(r => r.slug)).toEqual(all.slice(5, 15).map(r => r.slug));
+  });
+
+  test('offset near the end truncates the page', async () => {
+    const { ctx } = mkCtx({ remote: false });
+    const rows = (await op().handler(ctx, {
+      limit: 100000,
+      offset: SEED_COUNT - 7,
+      sort: 'slug',
+    })) as any[];
+    expect(rows.length).toBe(7);
+  });
+
+  test('garbage offset (negative / NaN) is ignored, not fatal', async () => {
+    const { ctx } = mkCtx({ remote: false });
+    const neg = (await op().handler(ctx, { limit: 10, offset: -5, sort: 'slug' })) as any[];
+    const nan = (await op().handler(ctx, { limit: 10, offset: NaN, sort: 'slug' })) as any[];
+    const base = (await op().handler(ctx, { limit: 10, sort: 'slug' })) as any[];
+    expect(neg.map(r => r.slug)).toEqual(base.map(r => r.slug));
+    expect(nan.map(r => r.slug)).toEqual(base.map(r => r.slug));
+  });
+});