fix(message-format): honor caller --limit for read, pins and search#79593
fix(message-format): honor caller --limit for read, pins and search#79593pahuchi-joe wants to merge 1 commit into
Conversation
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Close this PR as superseded: the same central message formatter limit fix is now covered by a newer clean, mergeable, proof-positive PR, while this branch is conflicting and still has unresolved review blockers. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Canonical path: Close this conflicting branch and use #99089 as the landing path for the shared formatter limit fix; fold in the small So I’m closing this here and keeping the remaining discussion on #99089. Review detailsBest possible solution: Close this conflicting branch and use #99089 as the landing path for the shared formatter limit fix; fold in the small Do we have a high-confidence way to reproduce the issue? Yes. Source inspection on current main shows Is this the best way to solve the issue? No for this branch. The shared formatter and CLI bridge are the right fix boundary, but the cleaner current solution is the newer mergeable PR that drops the release-changelog edit and adds total-aware search hints. Security review: Security review cleared: The diff is limited to CLI formatter plumbing, formatter tests, docs, and changelog text with no dependency, workflow, credential, install, or supply-chain surface change. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 9d68f877ac3e. |
e4cee81 to
3e984c7
Compare
|
This pull request has been automatically marked as stale due to inactivity. |
|
ClawSweeper PR egg 🔥 Warming up: real-behavior proof passed; findings, security review, or rank-up moves are still in progress. Hatch commandComment Hatchability rules:
What is this egg doing here?
|
|
🦞✅ Reason: structured ClawSweeper close marker: close-required (sha=3e984c7096a809293fd0f94aadbf0b64ac9a3de5) Closed:
|
Summary
renderMessageListinsrc/commands/message-format.tstruncated CLI output with a hardcodedmessages.slice(0, 25). The CLI exposed--limitformessage read,message pins, and Discordmessage search, and providers honored the upstream limit, but the rendered table still capped at 25 rows. Provider pagination signals (hasMore,nextBatch,@odata.nextLink) were also discarded, and Discord's read action returns nohasMorefield at all so a full page gave no signal that older history existed.--limit 50saw 25 rows with no indication more existed. The maintainer review on feat(message): list chat / list messages should support pagination instead of hardcoded 25 limit #71452 (steipete + clawsweeper) explicitly asked for this contract fix.renderMessageListnow takesdisplayLimitfromFormatOptsand returns{ lines, total, displayed, limit }.formatMessageCliTextaccepts an optional{ displayLimit }. The CLI parses--limitvia the existingparsePositiveIntOrUndefinedhelper. A newrenderPaginationHintemits a "Showing X of Y; raise --limit to see more" line when the payload had more rows than were rendered, "More results available beyond this page; use --json for the raw cursor" when the payload exposeshasMore/nextBatch/@odata.nextLink, and a heuristic "Reached --limit (N); raise it to fetch older history if any" when the provider returned exactly the requested limit and no other signal is available.list-pinsstill ignores callerlimitupstream, MS Teamschannel-liststill has no exposed cursor, and Feishu pagination is untouched. The default 25-row cap is preserved when--limitis absent. JSON payloads are unchanged. The hint deliberately avoids prescribing--before/--afterbecause those flags only exist onmessage read(notpins/search) and take message ids rather than the raw cursor tokens returned by Matrix/Graph.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Real behavior proof (required for external PRs)
message read --limit N,message pins --limit N, and Discordmessage search --limit Ntruncated to 25 rows; full Discord pages gave no hint that older history existed.pnpm buildpnpm openclaw message read --target channel:<id> --channel discord --limit 50-> renders 50 rows + "Reached --limit (50); raise it to fetch older history if any"pnpm openclaw message read --target channel:<id> --channel discord --limit 50 --before <id> --after <id>-> renders 50 rows + heuristic hintpnpm openclaw message read --target channel:<id> --channel discord-> renders 25 rows + "Showing 25 of 50; raise --limit to see more" (Discord returns 50 by default; we display 25)pnpm openclaw message read --target channel:<id> --channel discord --limit 50 --after <recent-id>-> renders 3 rows, no hint (range exhausted)--limit. Hint line varies based on signal: truncation whentotal > displayed, cursor when payload exposes pagination metadata, heuristic when the page filled exactly to--limit. No false positives when the range was exhausted (3 < limit).@odata.nextLinkand MatrixnextBatchcursor branches against real provider sessions. Both branches are covered insrc/commands/message-format.test.tsagainst representative payload shapes.mainrendered 25 rows with no hint line; the cap was deterministic from the literalslice(0, 25).Root Cause (if applicable)
renderMessageListinsrc/commands/message-format.tshad a hardcodedmessages.slice(0, 25)predating the--limitCLI option; the limit was never threaded into the renderer.--limit > 25actually produces more than 25 rendered rows. Existing tests asserted JSON payload shapes, not rendered text.--limitand cursor handling over time but the shared CLI renderer was never updated to match. Pagination metadata (hasMore,nextBatch,@odata.nextLink) was already populated by Slack/Matrix/MS Teams but consumed nowhere; Discord returned no metadata at all, so a full page silently looked the same as a complete page.Regression Test Plan (if applicable)
src/commands/message-format.test.ts.formatMessageCliTexthonorsdisplayLimitforread,list-pins, andsearch; default behavior unchanged when no limit is passed; pagination hint appears forhasMore,nextBatch,@odata.nextLink, when payload exceeds the requested limit, and when the provider returned exactly--limitrows; no false hint when the page is partial.src/commands/message.test.tscovers config plumbing, not rendered output.User-visible / Behavior Changes
openclaw message read --limit N,openclaw message pins --limit N, andopenclaw message search --limit Nnow render up to N rows instead of always capping at 25. Default remains 25 when--limitis omitted.Showing X of Y; raise --limit to see morehasMore: true, a MatrixnextBatch, or an MS Teams Graph@odata.nextLink:More results available beyond this page; use --json for the raw cursor--limitrows and no other signal exists:Reached --limit (N); raise it to fetch older history if any--json) is unchanged and still carries the raw cursor fields.Diagram (if applicable)
n/a
Security Impact (required)
NoNoNoNoNoYes, explain risk + mitigation: n/aRepro + Verification
Environment
Steps
pnpm buildpnpm openclaw message read --target channel:<id> --channel discord --limit 50(full-page heuristic)pnpm openclaw message read --target channel:<id> --channel discord(default-cap truncation)pnpm openclaw message read --target channel:<id> --channel discord --limit 50 --after <recent-id>(partial page; no hint)Expected
mainstep 3 renders 25 rows with no hint regardless.Actual
Evidence
The new formatter tests in
src/commands/message-format.test.tswould have failed against the priorslice(0, 25)cap (e.g.expect(countRowsInOutput(out, 30)).toBe(30)forreadwithdisplayLimit: 30).Human Verification (required)
message read --limit 50against a live Discord channel with hundreds of messages and confirmed 50 rows + "Reached --limit (50)" hint; ranmessage readwithout--limitand confirmed "Showing 25 of 50; raise --limit to see more" hint; ranmessage read --limit 50 --after <recent-id>and confirmed partial page renders without a false-positive hint.--limitvalues (0,-3, non-numeric strings) fall throughparsePositiveIntOrUndefinedtoundefined, which the formatter treats as "use default 25"; payload smaller than the requested limit renders all rows with no hint; provider cursor branches (hasMore,nextBatch,@odata.nextLink) covered by unit tests against representative payload shapes.nextBatchand live MS Teams Graph@odata.nextLinkcursor branches against real provider sessions.Review Conversations
Compatibility / Migration
YesNoNoRisks and Mitigations
--limitquickly verifies.--limit.sessions.ts,status.command-sections.ts).Test plan
Before / After
Before:
openclaw message read --limit 50rendered 25 rows even when the provider returned 50.hasMore,nextBatch,@odata.nextLink) were silently discarded by the CLI renderer.After:
--limitrows forread,list-pins, andsearch. Default stays at 25 when--limitis omitted.hasMore, a MatrixnextBatch, or an MS Teams Graph@odata.nextLink.--limitrows and no other signal exists (covers Discord, which returns nohasMore).Bugfix trigger
User invokes
openclaw message read --target <channel> --limit 50(ormessage pins --limit N, or Discordmessage search --limit N) against a channel/source that returns more than 25 messages. Before this fix, the rendered table always stops at 25 rows regardless of the requested limit, and anyhasMore/nextBatch/@odata.nextLinkreturned by the provider is hidden. For Discord specifically, even after raising--limitthe user has no way to tell whether the page is exhaustive or partial because Discord doesn't return ahasMoresignal.