diff --git a/src/core/operations-descriptions.ts b/src/core/operations-descriptions.ts index 7cf77ae07..57d79574d 100644 --- a/src/core/operations-descriptions.ts +++ b/src/core/operations-descriptions.ts @@ -58,7 +58,11 @@ export const GET_RECENT_TRANSCRIPTS_DESCRIPTION = export const LIST_PAGES_DESCRIPTION = "List pages with optional filters. " + "For 'what's recent / what did I touch this week' questions, use list_pages " + - "with sort=updated_desc instead of semantic search."; + "with sort=updated_desc instead of semantic search. " + + "Default 50 rows; remote callers are capped at 100 (local CLI callers' explicit " + + "limits are honored). A result with exactly `limit` rows may be truncated. " + + "For exhaustive listing, page with sort=updated_asc + " + + "updated_after= until a page returns fewer rows than the limit."; export const QUERY_DESCRIPTION = "Hybrid search with vector + keyword + multi-query expansion. " + diff --git a/src/core/operations.ts b/src/core/operations.ts index 8082f4016..ff6dfc139 100644 --- a/src/core/operations.ts +++ b/src/core/operations.ts @@ -1542,16 +1542,39 @@ const list_pages: Operation = { requestedOffset !== undefined && Number.isFinite(requestedOffset) && requestedOffset > 0 ? Math.floor(requestedOffset) : undefined; - const pages = await ctx.engine.listPages({ + // Probe one row past the effective limit so truncation is detectable + // without a COUNT query. The bug class sealed here is SILENT truncation + // — an exhaustive consumer (audit, scan, backfill) gets a full-looking + // list and never learns rows were dropped, and with the default + // updated_desc sort the dropped rows are always the OLDEST, i.e. exactly + // the pages such consumers exist to find. + const rows = await ctx.engine.listPages({ type: p.type as any, tag: p.tag as string, - limit, + limit: limit + 1, offset, includeDeleted: (p.include_deleted as boolean) === true, updated_after: typeof p.updated_after === 'string' ? p.updated_after : undefined, sort, ...scope, }); + const truncated = rows.length > limit; + const pages = truncated ? rows.slice(0, limit) : rows; + // Warn only when the caller's limit was NOT honored (unset → default 50): + // an explicit honored limit that happens to land on more rows is ordinary + // pagination, not a trap. Local (CLI) only — same operator-facing stderr + // channel as the put_page unknown-type hint above — but with no isTTY + // gate: scripted callers are precisely the consumers that cannot detect + // truncation any other way, and stderr keeps stdout parseable for them. + // (Local explicit limits are honored unbounded since #3322, so the + // requestedLimit > limit arm is defense in depth only.) + if (truncated && isLocal && (requestedLimit === undefined || requestedLimit > limit)) { + console.error( + `[list_pages] output truncated at ${limit} rows (default 50). ` + + `Pass an explicit limit, page through with sort=updated_asc + ` + + `updated_after=, or narrow with type/tag.`, + ); + } return pages.map(pg => ({ slug: pg.slug, source_id: pg.source_id, diff --git a/test/list-pages-truncation.test.ts b/test/list-pages-truncation.test.ts new file mode 100644 index 000000000..2b8bf124e --- /dev/null +++ b/test/list-pages-truncation.test.ts @@ -0,0 +1,155 @@ +/** + * list_pages silent-truncation seal. + * + * The op defaults limit to 50 and clamps remote callers to max 100 (local + * explicit limits are honored since #3322 — pinned in + * test/list-clamp-local-trust.test.ts). Pre-fix, a caller whose limit was + * defaulted (or remotely clamped) received a full-looking array with NO + * signal that rows were dropped, and with the default updated_desc sort the + * dropped rows are always the OLDEST — precisely what exhaustive consumers + * (audits, scans, backfills) exist to find. + * + * Covers, at the op-handler layer (engine listPages surface unchanged — + * the handler only probes limit+1): + * - default-limit truncation returns exactly 50 rows and warns on stderr + * (local ctx only) + * - an explicit, honored limit does NOT warn (ordinary pagination) + * - a clamped-but-complete result (requested > cap, rows ≤ cap) does NOT + * warn — nothing was dropped + * - remote ctx never writes to stderr (MCP server logs stay clean) + * - the pagination recipe in LIST_PAGES_DESCRIPTION (sort=updated_asc + + * updated_after cursor) actually enumerates every row to completion + * + * Runs against PGLite in-memory (both engines share the SQL surface; the + * handler change touches no engine code). + */ + +import { describe, test, expect, beforeAll, afterAll, beforeEach, spyOn } from 'bun:test'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { resetPgliteState } from './helpers/reset-pglite.ts'; +import { operations, type OperationContext } from '../src/core/operations.ts'; + +const list_pages = operations.find(o => o.name === 'list_pages')!; + +let engine: PGLiteEngine; + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); +}); +afterAll(async () => { await engine.disconnect(); }); +beforeEach(async () => { await resetPgliteState(engine); }); + +function ctxOf(overrides: Partial = {}): OperationContext { + return { + engine: engine as any, + config: {} as any, + logger: console as any, + dryRun: false, + remote: false, + sourceId: 'default', + ...overrides, + }; +} + +const page = (n: number) => ({ + type: 'note' as const, + title: `Note ${String(n).padStart(3, '0')}`, + compiled_truth: `Body of note ${n}.`, + timeline: '', + frontmatter: {}, +}); + +async function seed(count: number) { + for (let i = 1; i <= count; i++) { + await engine.putPage(`notes/note-${String(i).padStart(3, '0')}`, page(i), { sourceId: 'default' }); + } +} + +/** Run the handler while capturing stderr writes made through console.error. */ +async function runCapturing(ctx: OperationContext, params: Record) { + const spy = spyOn(console, 'error').mockImplementation(() => {}); + try { + const result = await list_pages.handler(ctx, params) as any[]; + const warnings = spy.mock.calls.map(args => args.join(' ')).filter(s => s.includes('[list_pages]')); + return { result, warnings }; + } finally { + spy.mockRestore(); + } +} + +describe('list_pages truncation signal', () => { + test('default limit: 51 rows → exactly 50 returned + stderr warning', async () => { + await seed(51); + const { result, warnings } = await runCapturing(ctxOf(), {}); + expect(result.length).toBe(50); + expect(warnings.length).toBe(1); + expect(warnings[0]).toContain('truncated at 50 rows'); + expect(warnings[0]).toContain('sort=updated_asc'); + }, 30_000); + + test('explicit honored limit: no warning even when more rows exist', async () => { + await seed(12); + const { result, warnings } = await runCapturing(ctxOf(), { limit: 10 }); + expect(result.length).toBe(10); + expect(warnings.length).toBe(0); + }, 30_000); + + test('clamped but complete: requested > cap with rows ≤ cap → all rows, no warning', async () => { + await seed(12); + const { result, warnings } = await runCapturing(ctxOf(), { limit: 200 }); + expect(result.length).toBe(12); + expect(warnings.length).toBe(0); + }, 30_000); + + test('local explicit limit above 100 is honored (#3322) — all rows, no warning', async () => { + await seed(101); + const { result, warnings } = await runCapturing(ctxOf(), { limit: 200 }); + expect(result.length).toBe(101); + expect(warnings.length).toBe(0); + }, 60_000); + + test('remote requested above cap: clamped to 100, truncation stays off stderr', async () => { + await seed(101); + const { result, warnings } = await runCapturing(ctxOf({ remote: true }), { limit: 200 }); + expect(result.length).toBe(100); + // The #3322 clamp warning goes through ctx.logger.warn; the [list_pages] + // truncation hint is local-only, so console.error stays clean here. + expect(warnings.length).toBe(0); + }, 60_000); + + test('remote ctx: truncation stays silent on stderr (MCP logs clean)', async () => { + await seed(51); + const { result, warnings } = await runCapturing(ctxOf({ remote: true }), {}); + expect(result.length).toBe(50); + expect(warnings.length).toBe(0); + }, 30_000); + + test('documented cursor recipe enumerates all rows to completion', async () => { + await seed(23); + // Spread updated_at deterministically: back-to-back putPage calls can land + // on identical timestamps, and a strict `updated_at > cursor` walk would + // then skip the tied rows — that would be a flake in THIS test, not a + // property of the recipe (real corpora update over time). + await engine.executeRaw( + `UPDATE pages SET updated_at = now() - (interval '1 minute' * (100 - id)) WHERE slug LIKE 'notes/note-%'`, + ); + const seen = new Set(); + let cursor: string | undefined; + // sort=updated_asc + updated_after=, stop when a + // page returns fewer rows than the limit — verbatim the recipe in + // LIST_PAGES_DESCRIPTION. + for (let guard = 0; guard < 10; guard++) { + const params: Record = { limit: 10, sort: 'updated_asc' }; + if (cursor !== undefined) params.updated_after = cursor; + const { result } = await runCapturing(ctxOf(), params); + for (const row of result) seen.add(row.slug); + if (result.length < 10) break; + cursor = result[result.length - 1].updated_at instanceof Date + ? result[result.length - 1].updated_at.toISOString() + : String(result[result.length - 1].updated_at); + } + expect(seen.size).toBe(23); + }, 30_000); +});