Skip to content

feat(rpc): add rowid-forward message pagination - #201

Merged
steipete merged 2 commits into
mainfrom
feat/messages-after-rpc
Aug 2, 2026
Merged

feat(rpc): add rowid-forward message pagination#201
steipete merged 2 commits into
mainfrom
feat/messages-after-rpc

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Aug 1, 2026

Copy link
Copy Markdown
Member

Summary

  • add additive messages.after JSON-RPC pagination in stable message.ROWID ASC order
  • return an authoritative physical next_rowid and database-backed has_more signal
  • preserve URL-preview coalescing across page boundaries without losing cursor progress
  • support optional chat filtering, attachments, conversion, and standalone reaction events
  • advertise and document the capability for downstream consumers

Closes #200.

Downstream consumer: openclaw/openclaw#116961

Contract

  • since_rowid is required, non-negative, and exclusive
  • chat_id is optional; omit it to page across all chats
  • limit defaults to 100 and is capped at 500
  • results are ordered by physical ROWID, not timestamp
  • callers persist next_rowid after every response, including empty pages
  • callers continue only while has_more is true
  • callers that need reaction rows covered by the cursor set include_reactions: true

Validation

  • focused rpcMessagesAfter|messagesAfterPage tests: 10 passed
  • non-stdout-capture suite: 356 tests passed
  • make lint: passed; 12 existing non-serious warnings, none in changed files
  • make build: universal CLI/helper build passed; existing unarchiveObjectWithData: deprecation warning only
  • exact-tree automated review: clean, no actionable findings
  • git diff --check: passed

A full local make test attempt remains blocked by a pre-existing StdoutCapture pipe deadlock in output-heavy tests such as statusCommandProducesTextOutput and chatBackgroundStatusEmitsReadOnlyJSON. Those 139 capture-based tests were excluded from the broad local run; focused changed-surface tests pass.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 1, 2026
@clawsweeper clawsweeper Bot added the P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. label Aug 1, 2026
@clawsweeper

clawsweeper Bot commented Aug 1, 2026

Copy link
Copy Markdown

Codex review: found issues before merge. Reviewed August 2, 2026, 1:34 PM ET / 17:34 UTC.

ClawSweeper review

What this changes

This PR exposes a new messages.after JSON-RPC endpoint that returns Messages records in forward ROWID order with a durable continuation cursor, optional reactions, attachments, and chat filtering.

Merge readiness

⚠️ Needs maintainer review before merge - 5 items remain

Keep open. Current main has internal ROWID-forward scanning but no public messages.after RPC, so this PR remains the active implementation candidate for #200. Before merge, the shared Message documentation must stop saying reaction events are live-watch-only, and the new stable RPC contract needs the API owner's approval. Likely related people: steipete is the high-confidence routing candidate from the existing cursor implementation and current maintainer review.

Priority: P3
Reviewed head: b61a3c95bcc3d326fe09b165e21ca40ca741991b
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The real behavior evidence is strong, but a P2 public-documentation contradiction and a pending API-contract decision prevent merge readiness.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (terminal): A collaborator independently recorded focused test results, a successful macOS build, and a live release-RPC fixture run showing a message, next_rowid: 1, and has_more: false; no additional contributor proof is needed.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): A collaborator independently recorded focused test results, a successful macOS build, and a live release-RPC fixture run showing a message, next_rowid: 1, and has_more: false; no additional contributor proof is needed.
Evidence reviewed 5 items Current main lacks the requested RPC surface: Current main lists messages.history but neither messages.after, its handler, nor its page type appears in the RPC, documentation, or test paths; the existing messagesAfter helper is internal-only.
Existing cursor behavior predates this PR: The current internal messagesAfter helper already scans forward over Message ROWIDs and supports optional reactions, establishing the underlying feature boundary this PR exposes through RPC.
Shared Message documentation contradicts the proposed endpoint: The new RPC documentation advertises include_reactions and returns [Message], while the current shared Message reference says reaction events only surface in the live watch stream.
Findings 1 actionable finding [P2] Align the shared reaction-event documentation
Security None None.

How this fits together

The iMessage RPC server turns read requests into queries against the local Messages SQLite database and serializes Message payloads for long-running clients. The added endpoint would give downstream catch-up consumers a stable forward cursor instead of relying on newest-first history reads.

flowchart LR
  Client[Catch-up client] --> RPC[JSON-RPC server]
  RPC --> Validation[Request validation]
  Validation --> Store[Messages SQLite store]
  Store --> Paging[ROWID scan and URL preview handling]
  Paging --> Response[Messages plus continuation cursor]
Loading

Decision needed

Question Recommendation
Should messages.after, including optional standalone reaction events and physical ROWID cursor semantics, become a stable public RPC contract for downstream catch-up clients? Approve and document the stable contract: Accept the public endpoint, align the shared Message reaction documentation, and land after the branch is refreshed.

Why: The implementation is additive and proof-backed, but accepting a permanent RPC method and its reaction/cursor behavior is an API-direction decision rather than a mechanical repair.

Before merge

  • Align the shared reaction-event documentation (P2) - include_reactions makes this endpoint return standalone reaction Message payloads, but docs/json.md still says those events only surface in live watch. Because this result is documented as [Message], update the shared reference before merge so API clients do not receive conflicting payload guidance.
  • Resolve merge risk (P1) - include_reactions makes standalone reaction events available in a paginated result, but the shared Message reference still tells API users those events only surface in live watch; merging as-is creates contradictory stable API guidance.
  • Resolve merge risk (P1) - This is an additive public RPC contract with cursor persistence semantics and a downstream consumer, so the API owner must explicitly accept its permanent compatibility boundary before landing.
  • Resolve merge risk (P1) - GitHub currently reports the draft branch as dirty against main; refresh the branch and re-evaluate the actual merge result after the contract decision.

Findings

  • [P2] Align the shared reaction-event documentation — docs/rpc.md:96-97
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch scope 742 added, 1 removed across 10 files The feature spans the public RPC, core cursor logic, documentation, and two focused test files.
Focused regression coverage 10 tests added Four core pagination tests and six RPC tests exercise cursor ordering, suppressed previews, parameters, and payload behavior.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #200
Summary: This PR is the explicit implementation candidate for the open feature request; it should remain open until the feature contract is approved and the branch can land.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Approve the contract and synchronize Message docs (recommended)
    Make the shared Message reference describe reaction events for both live watch and the new paginated endpoint before merging the approved RPC contract.
  2. Defer reaction pagination
    Keep forward message pagination but remove or postpone include_reactions if maintainers do not want this endpoint to extend the stable reaction-event contract.
  3. Pause the endpoint
    Close or defer the proposal if a permanent cross-client cursor contract is not wanted in the public RPC surface.

Technical review

Best possible solution:

Approve the messages.after contract only if it is intended as a permanent public RPC, then align docs/json.md and docs/rpc.md so the shared Message reaction semantics cover both watch and messages.after, followed by a refreshed merge check.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR proposes an additive RPC capability rather than repairing an existing failing behavior. The contributor and collaborator nevertheless provide a high-confidence fixture-based after-fix validation path for the proposed behavior.

Is this the best way to solve the issue?

Unclear until an API owner accepts the stable contract. The implementation reuses current-main forward scanning rather than duplicating the database logic, but the shared reaction-event documentation must be corrected before this is the maintainable solution.

Full review comments:

  • [P2] Align the shared reaction-event documentation — docs/rpc.md:96-97
    include_reactions makes this endpoint return standalone reaction Message payloads, but docs/json.md still says those events only surface in live watch. Because this result is documented as [Message], update the shared reference before merge so API clients do not receive conflicting payload guidance.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5bdd2b0b003a.

Labels

Label justifications:

  • P3: This is a feature addition for downstream catch-up ergonomics rather than a current user-facing regression.
  • merge-risk: 🚨 compatibility: The PR creates a stable public RPC and changes where consumers may receive standalone reaction Message payloads.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): A collaborator independently recorded focused test results, a successful macOS build, and a live release-RPC fixture run showing a message, next_rowid: 1, and has_more: false; no additional contributor proof is needed.
  • proof: sufficient: Contributor real behavior proof is sufficient. A collaborator independently recorded focused test results, a successful macOS build, and a live release-RPC fixture run showing a message, next_rowid: 1, and has_more: false; no additional contributor proof is needed.

Evidence

What I checked:

  • Current main lacks the requested RPC surface: Current main lists messages.history but neither messages.after, its handler, nor its page type appears in the RPC, documentation, or test paths; the existing messagesAfter helper is internal-only. (Sources/imsg/RPCServer.swift:36, 5bdd2b0b003a)
  • Existing cursor behavior predates this PR: The current internal messagesAfter helper already scans forward over Message ROWIDs and supports optional reactions, establishing the underlying feature boundary this PR exposes through RPC. (Sources/IMsgCore/MessageStore+Messages.swift:90, 1d8b679cc3a3)
  • Shared Message documentation contradicts the proposed endpoint: The new RPC documentation advertises include_reactions and returns [Message], while the current shared Message reference says reaction events only surface in the live watch stream. (docs/rpc.md:96, b61a3c95bcc3)
  • Real behavior proof and maintainer review: A collaborator independently reported focused cursor tests, a live Messages-compatible SQLite RPC fixture result, successful macOS build, and green macOS/Linux CI, while explicitly reserving the stable RPC contract for Peter’s approval. (Tests/imsgTests/RPCMessagesAfterTests.swift:1, b61a3c95bcc3)
  • Branch remains unmerged and unshipped: The requested external endpoint is not contained in current main; the latest release is v0.13.4 at the parent release commit, so no release provenance can establish this feature as already shipped. (Sources/imsg/RPCServer.swift:36, 1d8b679cc3a3)

Likely related people:

  • steipete: Peter Steinberger authored the current-main messagesAfter cursor implementation and explicitly stated that landing this new stable RPC contract should be gated on his approval. (role: existing behavior introducer and API decision owner; confidence: high; commits: 1d8b679cc3a3, d9bd9c754d50; files: Sources/IMsgCore/MessageStore+Messages.swift, Sources/imsg/RPCServer.swift, docs/json.md)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Synchronize docs/json.md with the reaction behavior documented for messages.after.
  • Obtain explicit API-owner approval for the stable cursor and reaction-event contract.
  • Refresh the branch against current main and re-check mergeability after the contract decision.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (11 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-01T11:54:28.593Z sha b61a3c9 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-01T13:12:34.988Z sha b61a3c9 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T00:16:14.244Z sha b61a3c9 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T02:41:59.012Z sha b61a3c9 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T05:08:23.663Z sha b61a3c9 :: found issues before merge. :: [P2] Align the public reaction-event documentation | [P3] Remove the release-owned changelog edit
  • reviewed 2026-08-02T07:14:35.076Z sha b61a3c9 :: found issues before merge. :: [P2] Align the shared reaction-event documentation | [P3] Remove the release-owned changelog entry
  • reviewed 2026-08-02T09:40:02.480Z sha b61a3c9 :: found issues before merge. :: [P2] Align the shared reaction-event documentation
  • reviewed 2026-08-02T12:47:42.730Z sha b61a3c9 :: found issues before merge. :: [P2] Align the shared reaction-event documentation

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Aug 1, 2026
@steipete

steipete commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Independent verification on exact head b61a3c9 is technically clean. I recommend LAND, but this should remain gated on Peter’s approval of the new stable RPC contract rather than treated as routine maintenance.

Proof on macOS arm64:

  • Focused messages.after suite: 17 tests passed, including cursor progress across suppressed/coalesced URL-preview rows, cross-chat ordering, reaction exclusion, reply enrichment, attributed-body fallback, and empty advancing pages.
  • make build ARCHES="$(uname -m)": release CLI and universal helper built successfully.
  • Live release RPC integration against a Messages-compatible SQLite fixture: messages.after from ROWID 0 returned the real fixture message, next_rowid: 1, and has_more: false.
  • GitHub CI is green on macOS and Linux; the PR is cleanly mergeable.
  • Full-branch autoreview: clean, no actionable findings.

Before landing, maintainers should align docs/json.md with this endpoint’s reaction payload behavior and move/remove the contributor-authored changelog entry per repository policy. No contributor revision is requested; those are maintainer-side landing edits after the API decision.

@clawsweeper clawsweeper Bot added the proof: sufficient Contributor real behavior proof is sufficient. label Aug 2, 2026
Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
@steipete
steipete marked this pull request as ready for review August 2, 2026 18:39
@steipete
steipete merged commit e9580a3 into main Aug 2, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(rpc): add rowid-forward message pagination

2 participants