[bug] "Load older messages" re-returns the newest page — the before cursor is a UUID, the route drops it, and the DO never implemented it #428

"Load older messages" returns the NEWEST messages again — older history is unreachable, and clicking it duplicates the thread

Found while investigating something else; verified end to end against production.

Proof — live API, instance 6d3b28f1-…

page 1:  GET /v1/instances/:id/messages?limit=10
         → [06:46:33, 06:46:36, 06:47:58, 06:48:04, 06:48:27,
            06:48:32, 06:49:05, 06:49:08, 06:49:40, 06:49:43]

cursor:  before=ffb4c8f8-4247-4da1-8f5f-d20c20e4acda   (oldest message's id)

page 2:  GET /v1/instances/:id/messages?limit=10&before=<cursor>
         → [06:46:33, 06:46:36, 06:47:58, 06:48:04, 06:48:27,
            06:48:32, 06:49:05, 06:49:08, 06:49:40, 06:49:43]

IDENTICAL PAGES: True

The before cursor has no effect at all. Page 2 is byte-identical to page 1.

Three separate defects, in one feature

1. The client sends a cursor that could never work — store/console/src/pages/InstanceDetail.tsx:475:

const before = oldest?.id || oldest?.createdAt || "";

id is preferred, and it is a UUID. The DO's storage key is msg:${msg.createdAt}:${msg.id} (agent-do.ts:753), so the ordering dimension is createdAt. A UUID has no ordering relationship to that key — even a correct server implementation could not seek with it. createdAt is only reached when id is absent.

2. The route drops the parameter — workers/api/src/routes/chat.ts:160:

new Request(`https://agent/messages?limit=${limit}`),

The query string is rebuilt from scratch with only limit. before never reaches the Durable Object.

3. The DO does not implement it — agent-do.ts:768-776, handleGetMessages reads only limit, and getRecentMessages (:757) is unconditionally "the newest N":

const all = await this.ctx.storage.list<AgentMessage>({ prefix: "msg:", reverse: true, limit });
return [...all.values()].reverse();

Each defect alone would break the feature. All three are present.

What the user sees

PAGE = 20 (InstanceDetail.tsx:180), and the button is at :1069. Clicking Load older messages:

1. Requests 20 older; receives the newest 20 — the ones already on screen.
2. setMessages((prev) => [...older, ...prev]) (:481) prepends them with no dedup, so the 20 newest messages now also appear at the top of the thread, above older ones — visibly out of order and duplicated.
3. setHasMore(older.length >= PAGE) (:478) → 20 >= 20 → true, forever. The button never goes away, and every click adds another 20 duplicates.
4. Older history is unreachable in the UI. The data is intact in the DO — ?limit=2000 returns it — but nothing in the console can page back to it.

There is also a React key collision: messageKey returns m.id when present (chat-message.ts:60-62), so the duplicated rows render with duplicate keys. Per-message component state (the CopyButton's copied flag, the replay PlaybackIcon phase) is keyed on those, so it can attach to the wrong bubble.

And the whole thing fails quietly: loadMore's body is wrapped in try { … } catch {} (:493) — one of the 61 bare catches from #424 — so even a thrown error would show nothing.

What to do

1. Implement before in the DO — the storage key is already lexicographically ordered by createdAt, so this is a bounded list() rather than a scan:

// cursor = the storage key of the oldest message the client already has
const all = await this.ctx.storage.list<AgentMessage>({
  prefix: "msg:", end: cursorKey, reverse: true, limit,
});
return [...all.values()].reverse();

end is exclusive, which is exactly the "strictly older than" semantic wanted.

2. Forward the parameter — chat.ts:160 should pass before through rather than rebuilding the query string with only limit.

3. Send a cursor that sorts. The client must send something ordered on createdAt — either createdAt alone, or (better) the full storage key so the tiebreak on identical timestamps is exact. Emitting an opaque nextCursor in the response and having the client echo it back is the version that cannot drift again, because the client stops needing to know the key format.

4. Derive hasMore from the server, not from a length comparison. older.length >= PAGE is a guess that happens to be wrong in exactly this failure. Returning hasMore (or a null nextCursor) makes "there is nothing older" a fact rather than an inference.

5. Dedup on prepend regardless. [...older, ...prev] filtered by id is cheap insurance against duplicate rendering, and it would have made this bug loud (visibly nothing happens) instead of silently corrupting the order.

Alternatives considered and rejected

• Drop pagination and always load everything. The DO caps at 2000 (:770) and a long coding thread is large; this moves a UI bug into a payload problem.
• Filter client-side after fetching. Cannot work — the server never returns anything older than the newest page, so there is nothing to filter.
• Fix only the DO. Insufficient. With the route still dropping the param (defect 2) and the client still sending a UUID (defect 1), the feature stays broken and would look like the DO fix didn't work.

Acceptance criteria

• ?before=<cursor> returns messages strictly older than the cursor; two consecutive pages share no ids.
• Clicking Load older messages on a thread with >20 messages prepends genuinely older messages, in order.
• The button disappears when the start of the conversation is reached.
• No duplicate message ids in the rendered thread after repeated clicks.
• A failure in loadMore surfaces instead of being swallowed.

Regression risk

• end is exclusive on the DO list API; an off-by-one here either re-sends the cursor message (a visible duplicate) or skips one (silent loss). The test should assert both boundaries.
• Messages written in the same millisecond share a createdAt; the key's :${id} suffix disambiguates, which is why the cursor should be the full key rather than the timestamp.
• Changing the response shape to include nextCursor/hasMore touches every /messages consumer — grep shows the paginated caller is only InstanceDetail.tsx:476, so the blast radius is small, but MCP instance_messages reads the same route.

Related: #424 (the bare catch {} that hides this class of failure), #406 (the /messages handler's stamping logic, same function).