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 whenfilter.limitis 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— thetask_listMCP handler.
Tests (24):
src/__tests__/domains/tasks/repository.test.ts— 19 call sitessrc/__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'— callslistTasks(db, { limit: 10_000 })directly; only asserts row count. Will need extension to assert the newclamped_limitsignal.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
listTasksreturn 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(notitems) aligns with the existing tool envelope ({tasks, total_count}) and the documenteddata.tasksfield 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_LIMITnumeric 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_listfrom “rejects limit>500” to “accepts and clamps”. Documented and tested. - No DB schema change, no migration.