Audit — fix-pagination-clamp-signal

Code-review Medium Finding #12: listTasks(db, filter) in src/domains/tasks/repository.ts silently clamps filter.limit > MAX_LIMIT (500) to 500. Clients receive 500 rows with no signal they were throttled, leading to under-pagination or infinite-loop bugs in client-side paginating UIs. The same is true for the task_list MCP tool — except, as the audit below shows, the tool layer does not even reach the clamp because the Zod schema rejects limit > 500 outright.

Surface inventory

Constants (repository.ts)

  • DEFAULT_LIMIT = 50 (line 164) — used when filter.limit is undefined.
  • MAX_LIMIT = 500 (line 167) — hard cap for clamping.
  • Neither value changes in this task.

listTasks function (repository.ts:536–559)

Current shape:

export function listTasks(db: Database, filter: ListTasksFilter = {}): Task[] {
  ...
  const rawLimit = filter.limit ?? DEFAULT_LIMIT;
  const limit = rawLimit > MAX_LIMIT ? MAX_LIMIT : rawLimit;       // ← silent clamp
  ...
  return rows.map(rowToTask);
}

The function returns Task[] directly. The clamp is a single line and has no observable signal.

Zod tool input schema (TaskListInputSchema, repository.ts:624–630)

const TaskListInputSchema = z.object({
  status: z.enum(TASK_STATES).optional(),
  project_id: z.string().nullable().optional(),
  limit: z.number().int().positive().max(500).optional(),         // ← rejects > 500
  offset: z.number().int().min(0).optional(),
  include_deleted: z.boolean().optional(),
});

The .max(500) constraint means MCP clients passing limit: 1000 get INVALID_PARAMS from the α schema-validate stage, never reaching the repository. For the clamp signal to be observable at the tool layer, this constraint must be relaxed.

task_list MCP handler (repository.ts:773–789)

(input): { tasks: Task[]; total_count: number } => {
  const tasks = listTasks(getDb(), input as ListTasksFilter);
  return { tasks, total_count: tasks.length };
},

Returns {tasks, total_count} payload. The α middleware wraps it in {ok:true, data:...}. To signal clamping, this payload needs a clamped_limit field.

Logger surface

ColibriServerContext.logger (server.ts:162) is captured in the closure of registerTaskTools(ctx), so the task_list handler can call ctx.logger(...) without a separate plumbing change.

Callers of listTasks

$ rg -n 'listTasks\(' src/

Production (1):

  • src/domains/tasks/repository.ts:786 — the task_list MCP handler.

Tests (24):

  • src/__tests__/domains/tasks/repository.test.ts — 19 call sites
  • src/__tests__/domains/tasks/tools.test.ts — 5 call sites

All test callers treat the return value as Task[] directly (.map(), .length, .toHaveLength(N), equality checks). All must adapt if the return shape changes.

Existing tests touching the clamp / Zod limit

  • repository.test.ts:571–580'clamps limit > 500 to 500' — calls listTasks(db, { limit: 10_000 }) directly; only asserts row count. Will need extension to assert the new clamped_limit signal.
  • tools.test.ts:439–445'Zod schema rejects limit > 500' and 'Zod schema accepts limit=500 (boundary)'. With the relaxed schema, the rejection assertion must flip — large limits should now parse and surface as a clamp at the handler.

Decision: option (a)

The task prompt names two options:

  • (a) Change listTasks return to { items: Task[], clamped_limit: number | null }.
  • (b) Out-parameter / second return path.

Choice: (a), with items named tasks to match the existing tool envelope’s tasks key and the function’s existing semantics. Concretely:

export interface ListTasksResult {
  readonly tasks: readonly Task[];
  readonly clamped_limit: number | null;
}

export function listTasks(db: Database, filter: ListTasksFilter = {}): ListTasksResult { ... }

Rationale:

  • Typed return — no out-parameter discipline required.
  • 1 production caller + 24 test callers must adapt; all are in this repo, so the cascade is bounded and mechanical.
  • Naming tasks (not items) aligns with the existing tool envelope ({tasks, total_count}) and the documented data.tasks field that the prompt’s BC clause protects.

The task_list payload becomes {tasks, total_count, clamped_limit} — additive, BC-preserving for clients reading data.tasks / data.total_count.

Zod schema relaxation

TaskListInputSchema.limit drops .max(500). The constraint becomes z.number().int().positive().optional(). The clamp moves entirely into the repository layer — the same place it lives today. No new validation policy; just removes a redundant rejection that prevents the existing clamp from being observable.

A reasonable upper sanity bound is still useful — Number.MAX_SAFE_INTEGER is implicit via z.number().int() (rejects non-finite / non-integer). For Phase 0 we accept any positive integer; the SQL bind handles arbitrary integers safely; the clamp protects memory.

Logger emission

When the handler observes a clamped limit, emit ONE INFO line per call:

[task_list] clamped limit: requested=<N> max=500

Using ctx.logger(...), captured in the closure. Suppressed when clamped_limit === null. No WARN flood — one INFO per call where clamping occurred is acceptable.

Out of scope

  • MAX_LIMIT / DEFAULT_LIMIT numeric values — unchanged.
  • Clamp behavior itself — unchanged (still silently clamps to 500 server-side; the new signal is observability only).
  • task_get, task_create, task_update, task_next_actions — untouched.
  • Writeback enforcement, FSM, soft-delete, or any other β surface — untouched.
  • Prepared-statement cache — unchanged (clamp logic is pre-bind).

Risk

  • Test cascade across 24 sites is mechanical — destructure {tasks} from the return and reuse.
  • Zod schema flip changes the public API surface for task_list from “rejects limit>500” to “accepts and clamps”. Documented and tested.
  • No DB schema change, no migration.

Back to top

Colibri — documentation-first MCP runtime. Apache 2.0 + Commons Clause.

This site uses Just the Docs, a documentation theme for Jekyll.