Conversation
Desktop API message list pagination expects sortKey as cursor, but messages list/context/export were passing message IDs through unchanged. Retrieve the message when chat+ID are available and use its sortKey; pass through values that are already sortKeys (retrieve 404). Fixes beeper#39
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The wiring tests bypass the production commands and cannot detect regressions in their cursor handling.
Review effort: Balanced
Findings: 1
What changed in this PR
Fixes #39 by resolving message IDs to Desktop API sortKey cursors before pagination.
Changes:
- Applies cursor resolution to list, context, and export.
- Clarifies cursor semantics in help and documentation.
- Adds resolver tests and simulated wiring checks.
| File | Description |
|---|---|
| packages/cli/test/messages-cursor.test.ts | Adds resolver and simulated wiring tests. |
| packages/cli/test/fixtures/fake-client.ts | Adds optional message sortKey. |
| packages/cli/src/lib/resolve.ts | Adds cursor resolution with passthrough fallbacks. |
| packages/cli/src/commands/messages/list.ts | Resolves pagination cursors and updates help. |
| packages/cli/src/commands/messages/export.ts | Resolves export cursors and updates help. |
| packages/cli/src/commands/messages/context.ts | Uses one resolved cursor for both directions. |
| packages/cli/docs/messages.md | Documents message ID and sortKey support. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Copilot's wiring-test finding is valid. The implementation fix itself is in place, but the two wiring tests currently simulate the resolver/list sequence instead of invoking |
|
Addressed Copilot's wiring-test finding in cb0a1f7: the tests now launch the real |
Summary
Fixes #39.
messages list(--before-cursor/--after-cursor),messages context(--id), andmessages exportwere passing a message ID straight through as the Desktop APIcursor. The API expects asortKey(oldestCursor/newestCursoron pages), so ID-based paging returned wrong or empty windows.This PR resolves a supplied cursor via
messages.retrievewhen chat + message ID are available, then uses that message’ssortKey. If retrieve 404s (value is already a sortKey) orsortKeyis missing, the original value is passed through unchanged.Flag help and
docs/messages.mdnow describe cursor semantics (message ID or sortKey) instead of “message ID” alone.Out of scope
Server-side Desktop API issues remain upstream and are not addressed here:
direction=afterreturning the newest page instead of the next one (beeper/desktop-api-openapi#4)Test plan
resolveMessageCursor(ID → sortKey, sortKey passthrough on 404, missing retrieve / missing sortKey, unexpected errors)cursorbun test test/messages-cursor.test.ts(8 pass) — full monorepobun install/bun testnot run in this environment