fix(server): keyset message pagination + attachments in List (F36, F37)

[Calmingstorm]

Two confirmed server findings from the campaign audit, both in MessageHandler.List:

F36 — pagination loses messages at timestamp ties. The before cursor compared created_at alone with no ORDER BY tie-breaker: messages sharing the cursor's exact created_at were neither strictly older (skipped forever) nor stably ordered across pages. Fixed with a keyset cursor on (created_at, id) and ORDER BY created_at DESC, id DESC in both branches. The client cursor stays the oldest message id — no client change.

F37 — List omits attachments. Reactions were bulk-fetched; attachments never were, so they only arrived via realtime MESSAGE_CREATE — any reload or history scroll showed attachment messages without them. List now bulk-fetches attachments the same way (additive to the response shape the client already renders).

Both fixes are query-only: no migration, no config, no client change.

Tests (both mutation-verified):

• 55 messages forced onto one timestamp: under the old cursor, page 2 returned zero (5 messages silently lost); with the keyset, both pages cover all 55 exactly once, no duplicates.
• A two-attachment message lists them in order with URLs; a plain message has none; disabling the fetch fails the test.

Full Go suite green; gofmt/vet clean; golangci-lint 0 new issues vs main.

[Calmingstorm]

Reviewed PR #23 at e220eda283289871b15e5ed42999a1644176274b. The (created_at, id) keyset itself closes the timestamp-tie loss, and the bulk attachment mapping has the intended response shape, but three blockers remain.

1. The cursor lookup is not scoped to the requested channel.

   messages.go:355-357 looks up beforeID by message ID alone. A cursor from any other channel is therefore accepted and its timestamp is used to paginate the target channel. In a deterministic HTTP probe, requesting channel B with a real cursor from channel A returned 200 [] rather than 400.

   A before message ID is only a valid cursor for its own channel. Bind the lookup to both values, e.g. WHERE id = $1 AND channel_id = $2, and regression-pin a foreign-channel cursor. This also avoids accepting a cursor whose source channel the caller cannot view.

2. Attachment-read failures are silently converted into a successful attachment-free history response.

   At messages.go:485-508, an attachment query failure is ignored entirely, per-row Scan failures skip rows, and Rows.Err() is merely logged before returning 200. A probe that made the attachment query fail received 200 with the message and no attachments—the same user-visible corruption F37 is meant to fix.

   Once List claims attachments as authoritative history data, query, scan, and iteration failures must fail the request rather than masquerade as “this message has no files.” Return the normal logged 500 on all three paths. The existing best-effort reactions pattern is not a sound error contract to copy for this fix.

3. Attachment order is undefined for real uploads, and the regression does not exercise that case.

   The new query orders only by created_at. SendWithAttachments inserts every attachment inside one PostgreSQL transaction, while created_at DEFAULT NOW() uses the transaction timestamp, so a multi-file upload normally gives every row the exact same created_at. SQL may return those ties in any order. The test uses separate autocommit Pool.Exec calls, giving the rows different timestamps and making its “insertion order” assertion pass without pinning production behavior.

   Add an explicit deterministic tie-breaker and a same-timestamp regression. If multipart/upload order is intended to survive reload and match realtime—as the current assertion suggests—persist an ordinal and order by it. If only stable order is required, define and test a stable secondary key such as id; do not call it insertion order.

Validation performed at the exact head:

• Full Go suite with -race passed against local PostgreSQL 16 and Redis 7
• Targeted F36/F37/cursor tests passed
• Go vet and gofmt passed
• Diff checks passed
• GitHub Go, Lint, and Web jobs are green; PR is mergeable
• Two deterministic review probes reproduced blockers 1 and 2, then were removed; checkout is clean

Changes requested. GitHub would not accept a formal changes-requested review because the authenticated account owns the PR, so this verdict is posted as a PR comment instead.

[Calmingstorm]

Pushed the three requested blocker fixes directly to fix/server-message-list.

Commit: 384989db9ca07571522baaec0147ee0e61ff1594

Changes:

• Scoped the before cursor lookup to both message ID and requested channel ID; foreign-channel cursors now return 400.
• Made attachment hydration mandatory for a successful List response: query, Scan, and Rows.Err() failures are logged and return the normal 500.
• Added migration 018 with a required zero-based attachment position, deterministic legacy backfill, nonnegative constraint, and unique (message_id, position) index.
• Persisted the multipart loop index during upload and ordered List hydration by message_id, position.
• Added regressions for foreign-channel cursors, failed attachment queries, production multi-file same-transaction ordering, and migration backfill/rollback.

Validation at this exact head:

• Full go test -race ./... against PostgreSQL 16 and Redis 7
• Targeted regressions repeated five times
• gofmt, go vet, and GolangCI lint
• git diff --check; checkout clean
• GitHub Go, Lint (Go), and Web jobs all green