Three small residuals from PR #1366 (issue #1164, part of #1127), judged non-blocking at review. Grouped because they are all in MCP's argument handling.
1. Root take / skip are range-checked but not type-checked
#1366 added guards refusing a negative root take/skip. The nested selector checks the type as well; the root guard does not, so a string-typed take slips past and reaches .limit().
2. Root take: 0 silently means 10
A root take of 0 falls through to the default of 10 rather than being honoured or refused. The new test pins only that it is not refused, which leaves the actual behaviour unpinned.
An explicit 0 is a reasonable thing for a client to send (it usually means "count only, no rows"), and silently returning ten rows is the surprising answer.
3. combinedCount returning undefined would leave rows on the wire
In the count-only path, toWire drops items so the response is the bare number it has always been. If combinedCount ever returns undefined on an unexpected shape, the fallback leaves { items, count } — rows included — on the wire for a request that asked only for a count.
This is unreachable today — it is a latent shape, not a live leak. But the whole point of the count-only fix in #1366 was that the caller receives a number, and a defensive branch that fails towards more data is the wrong direction for a public surface.
Notes
None of these is a live defect. They are grouped as one cleanup so they are not lost, and so a fix can be reviewed as a single small diff.
Generated by Claude Code
Three small residuals from PR #1366 (issue #1164, part of #1127), judged non-blocking at review. Grouped because they are all in MCP's argument handling.
1. Root
take/skipare range-checked but not type-checked#1366 added guards refusing a negative root
take/skip. The nested selector checks the type as well; the root guard does not, so a string-typedtakeslips past and reaches.limit().take/skipare type-checked as well as range-checked, matching the nested selectortakeis refused with the same wording a negative one gets2. Root
take: 0silently means 10A root
takeof0falls through to the default of 10 rather than being honoured or refused. The new test pins only that it is not refused, which leaves the actual behaviour unpinned.An explicit
0is a reasonable thing for a client to send (it usually means "count only, no rows"), and silently returning ten rows is the surprising answer.take: 0has a defined, documented behaviour — honoured as zero rows, or refused3.
combinedCountreturningundefinedwould leave rows on the wireIn the count-only path,
toWiredropsitemsso the response is the bare number it has always been. IfcombinedCountever returnsundefinedon an unexpected shape, the fallback leaves{ items, count }— rows included — on the wire for a request that asked only for a count.This is unreachable today — it is a latent shape, not a live leak. But the whole point of the count-only fix in #1366 was that the caller receives a number, and a defensive branch that fails towards more data is the wrong direction for a public surface.
Notes
None of these is a live defect. They are grouped as one cleanup so they are not lost, and so a fix can be reviewed as a single small diff.
Generated by Claude Code