fix(messages): clamp conversation list limit

[Jorel97]

Summary

• normalize conversation message list limit before querying Supabase
• clamp negative values to 1, non-numeric/missing values to 50, and large values to 100
• add GET route coverage for auth plus limit boundaries

Fixes #330.

Testing

• Not run locally: this workspace has Node but no npm/pnpm/npx/corepack available to install or invoke Vitest.

[greptile-apps]

Greptile Summary

This PR replaces an unsafe parseInt one-liner with a parseLimit helper that normalises the limit query parameter before it reaches Supabase, and adds a Vitest file covering the auth gate and limit boundary cases.

• parseLimit correctly handles null, blank strings, non-numeric strings, negative numbers, floats, and values above 100 — all produce a finite integer in [1, 100].
• The test suite verifies five limit scenarios and the 401 path, but does not cover limit=0 (which silently maps to 1) and does not exercise the nextCursor pagination path, which has a pre-existing off-by-one bug that causes overlapping pages on subsequent fetches.

Confidence Score: 3/5

Safe to merge for the limit-clamping fix specifically, but the cursor pagination produces overlapping pages on any multi-page conversation, which should be addressed before this ships to users relying on infinite-scroll.

The parseLimit change itself is correct and an improvement. However, the same file contains a real defect in how nextCursor is computed: after calling data.reverse() the code still reads data[data.length - 1] (the newest item) as the cursor, so every subsequent page overlaps heavily with the previous one. This affects any conversation long enough to paginate.

src/app/api/conversations/[id]/messages/route.ts — specifically the nextCursor expression on the response return statement.

Important Files Changed

Filename	Overview
src/app/api/conversations/[id]/messages/route.ts	Replaces unsafe parseInt with a new parseLimit helper that clamps to [1,100] and defaults to 50; the new function is correct, but the pre-existing nextCursor pagination logic in the same file is buggy and will return overlapping pages.
src/app/api/conversations/[id]/messages/route.test.ts	New test file covering auth rejection and limit boundary cases (negative, non-numeric, missing, in-range, oversized); test structure is correct but does not cover limit=0, and the nextCursor pagination path is untested.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GET /api/conversations/id/messages] --> B{getAuthContext}
    B -- null --> C[401 Unauthorized]
    B -- auth --> D[Query conversation by id]
    D -- not found --> E[404 Not Found]
    D -- found --> F{user in participant_ids?}
    F -- no --> G[403 Forbidden]
    F -- yes --> H[parseLimit from searchParams]

    subgraph parseLimit
        H --> I{value null or blank?}
        I -- yes --> J[use 50]
        I -- no --> K[Number coerce value]
        K --> L{isFinite?}
        L -- no --> M[use 50]
        L -- yes --> N[Math.trunc]
        N --> O[clamp to 1..100]
        J --> O
        M --> O
    end

    O --> P[Query messages DESC limit+1]
    P -- error --> Q[400 Bad Request]
    P -- ok --> R{messages.length > limit?}
    R -- yes hasMore=true --> S[slice to limit, reverse]
    R -- no hasMore=false --> S
    S --> T[Mark unread messages as read]
    T --> U[Return data + hasMore + nextCursor]

Loading

Comments Outside Diff (1)

1. src/app/api/conversations/[id]/messages/route.ts, line 100 (link)

   Wrong cursor value for pagination — causes overlapping pages

   After data.reverse(), data is in ascending (oldest-first) chronological order. data[data.length - 1] is therefore the newest message in the returned page. The query filters with .lt("created_at", cursor), so on the next fetch every message older than the newest — including most of what was already returned — is re-fetched. The cursor should be data[0].created_at (the oldest in the page) so that subsequent requests reliably advance backward in time.

   This bug pre-dates this PR but is in the changed file and the hasMore path is now indirectly exercised by the new tests.

_{Reviews (1): Last reviewed commit: "fix(messages): clamp conversation list l..." | Re-trigger Greptile}

[greptile-apps]

limit=0 boundary not tested

parseLimit("0") calls Math.max(0, 1) and returns 1, so a client sending ?limit=0 silently gets the minimum page size. Given that the PR description calls out "clamp negative values to 1" as intentional, a test asserting expectMessageLimit({ limit: "0" }, 1) would make this edge-case explicit and prevent a future reader from accidentally changing the minimum.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!