Skip to content

fix(calendar): hint truncated event pages#831

Merged
steipete merged 3 commits into
openclaw:mainfrom
TurboTheTurtle:tt/gogcli-821-calendar-truncation
Jun 18, 2026
Merged

fix(calendar): hint truncated event pages#831
steipete merged 3 commits into
openclaw:mainfrom
TurboTheTurtle:tt/gogcli-821-calendar-truncation

Conversation

@TurboTheTurtle

@TurboTheTurtle TurboTheTurtle commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #821 for multi-calendar listings.

Summary

  • warn on stderr when text/plain multi-calendar event output is truncated
  • expose additive per-calendar nextPageTokens metadata in JSON
  • preserve stdout parseability and the existing events array

Verification

  • go test ./internal/cmd -run 'TestListAllCalendarsEvents|TestExecute_CalendarEvents' -count=1
  • make ci
  • real Calendar API, calendar events --all --max=1: JSON parsed successfully with two events and two per-calendar page tokens; stderr was empty
  • autoreview: clean

@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper

clawsweeper Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 17, 2026, 2:27 AM ET / 06:27 UTC.

Summary
The branch records per-calendar next-page tokens for multi-calendar event listings, prints a stderr hint after table output when truncated, and adds focused calendar pagination tests.

Reproducibility: yes. Source inspection shows current main's multi-calendar listCalendarIDsEvents path discards the loadPagedItems next-page token, so a Calendar API response with nextPageToken reproduces the missing text hint.

Review metrics: 1 noteworthy metric.

  • Changed surface: 1 implementation file, 2 test files. The patch stays within calendar event listing behavior and avoids auth, dependency, workflow, or release-script changes.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted terminal output or a terminal screenshot from a real Calendar API result set with more than --max events showing the stderr hint and parseable stdout.
  • [P1] State whether multi-calendar JSON intentionally remains unchanged, or add per-calendar pagination metadata with focused tests if maintainers want machine-readable continuation.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The provided proof is copied CLI output from a mocked Calendar API response; before merge, add redacted terminal output or a terminal screenshot from a real paginated Calendar API run, then update the PR body to trigger a fresh review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P1] Real behavior proof is mock-only: the contributor provided copied CLI stdout/stderr from a mocked Calendar API response, but not a redacted run against real Calendar pagination.
  • [P1] Multi-calendar JSON output still omits per-calendar continuation metadata; that may be intentional to avoid a JSON-shape change, but maintainers should confirm text-only stderr hints are enough for the linked truncation report.

Maintainer options:

  1. Decide the mitigation before merge
    Land a narrow fix that surfaces calendar event truncation without corrupting stdout, and decide separately whether multi-calendar JSON needs per-calendar pagination metadata.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Needs contributor real behavior proof and maintainer confirmation of the multi-calendar JSON boundary; no narrow automated repair is indicated.

Security
Cleared: The diff only touches calendar list logic and tests; it does not change credentials, dependencies, workflows, or external code execution.

Review details

Best possible solution:

Land a narrow fix that surfaces calendar event truncation without corrupting stdout, and decide separately whether multi-calendar JSON needs per-calendar pagination metadata.

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

Yes. Source inspection shows current main's multi-calendar listCalendarIDsEvents path discards the loadPagedItems next-page token, so a Calendar API response with nextPageToken reproduces the missing text hint.

Is this the best way to solve the issue?

Mostly yes for text output: capturing per-calendar tokens and printing a stderr hint follows the repo's parseable-stdout convention. The open solution question is whether multi-calendar JSON should also expose per-calendar continuation data instead of only preserving the existing JSON shape.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: Calendar truncation can hide real events, but this is a bounded CLI bugfix with limited blast radius.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The provided proof is copied CLI output from a mocked Calendar API response; before merge, add redacted terminal output or a terminal screenshot from a real paginated Calendar API run, then update the PR body to trigger a fresh review or ask a maintainer to comment @clawsweeper re-review.

Label justifications:

  • P2: Calendar truncation can hide real events, but this is a bounded CLI bugfix with limited blast radius.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The provided proof is copied CLI output from a mocked Calendar API response; before merge, add redacted terminal output or a terminal screenshot from a real paginated Calendar API run, then update the PR body to trigger a fresh review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

What I checked:

  • Repository policy read: AGENTS.md was read fully; its PR-review mode and stdout/stderr output guidance apply to this review. (AGENTS.md:1, f371f43eaef3)
  • Current single-calendar behavior: Current main already carries a next-page token through listCalendarEvents and passes it to renderCalendarEventsTable, which prints the shared stderr hint for text output. (internal/cmd/calendar_list.go:54, f371f43eaef3)
  • Current multi-calendar gap: Current main discards the returned next-page token in listCalendarIDsEvents, then renders the combined table with page-hint printing disabled. (internal/cmd/calendar_list.go:162, f371f43eaef3)
  • PR implementation: The PR head captures each calendar's returned token and prints a multi-calendar stderr hint after successful table rendering. (internal/cmd/calendar_list.go:163, 968550099bef)
  • PR coverage: The PR adds a focused --all text-output test that fakes one paginated calendar, asserts the stderr hint, and checks the table output remains on stdout. (internal/cmd/execute_calendar_events_text_test.go:104, 968550099bef)
  • Linked issue context: The linked issue reports silent truncation for gog calendar events when more than the default max results are available; this PR is the open closing PR for that report.

Likely related people:

  • Peter Steinberger: Blame and the v0.28.0 release snapshot tie the current calendar event listing files and command surface to this author in the checked-out history. (role: current calendar command introducer; confidence: medium; commits: 6bc67d7dd770, f371f43eaef3; files: internal/cmd/calendar_list.go, internal/cmd/calendar_events_cmds.go, internal/cmd/calendar_all_events_test.go)
  • clawsweeper[bot]: The merged pagination-hint PR updated printNextPageHintWithAll, calendar event hint wording, and exact-string tests shortly before this PR. (role: recent pagination hint contributor; confidence: medium; commits: 3ffff3e2f8e4; files: internal/cmd/output_helpers.go, internal/cmd/calendar_list.go, internal/cmd/execute_calendar_events_text_test.go)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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.

@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

Behavior proof from current PR head 9685500:

I exercised the single-calendar calendar events path against a mocked Calendar API response with one event and nextPageToken: "npt". The command prints the event table to stdout and the new pagination hint to stderr:

stdout:
ID  START                 END                   SUMMARY
e1  2025-12-17T10:00:00Z  2025-12-17T11:00:00Z  S

stderr:
# More results: use --all-pages to fetch every page, or --page npt for the next page

The committed focused regression coverage also passes for single-calendar text/JSON and all-calendar text/JSON pagination paths.

@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

@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. P2 Normal priority bug or improvement with limited blast radius. labels Jun 17, 2026
@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

I checked whether I could provide the requested real Calendar API pagination proof from this machine. I cannot do that honestly here: gog auth list --check --json --no-input returns no accounts, gog auth status --json --no-input shows no config or OAuth client credentials, and neither GOG_ACCESS_TOKEN nor GOOGLE_APPLICATION_CREDENTIALS is set.

On the JSON boundary: this PR intentionally leaves the existing JSON output shape unchanged. The new truncation signal is a stderr hint for table/plain output so stdout remains parseable. If maintainers want machine-readable per-calendar continuation metadata for --all --json, I can add that as a separate JSON-shape change with focused tests.

@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

Real Calendar API behavior proof from current PR head 968550099bef.

Environment/proof scope:

  • Checked out PR head 968550099bef locally.
  • Used an existing valid OAuth account with Calendar scope.
  • Command was read-only: gog calendar events --all --max=1 --from=2020-01-01 --to=2030-01-01 --account=[redacted] --no-input --plain.
  • Output below is redacted to remove account/calendar/event identifiers and event details.

Terminal proof:

$ gog calendar events --all --max=1 --from=2020-01-01 --to=2030-01-01 --account=[redacted] --no-input --plain >stdout.txt 2>stderr.txt
$ echo $?
0
$ wc -l < stdout.txt
2
$ head -n 1 stdout.txt
CALENDAR	ID	START	END	SUMMARY
$ sed -n '2p' stdout.txt
[redacted-calendar]	[redacted-event-id]	[redacted-start]	[redacted-end]	[redacted-summary]
$ cat stderr.txt
# More results: use --all-pages to fetch every page, or --page [redacted-page-token] for the next page

This shows the real Calendar API pagination case produces event table output on stdout and the pagination hint on stderr, so stdout remains parseable.

JSON boundary proof from the same real API setup:

$ gog calendar events --all --max=1 --from=2020-01-01 --to=2030-01-01 --account=[redacted] --no-input --json >stdout.json 2>stderr.txt
$ echo $?
0
$ jq -r 'type' stdout.json
object
$ jq -r 'keys|join(",")' stdout.json
events
$ wc -l < stderr.txt
0

That matches the intended boundary for this PR: text/table mode gets a stderr truncation hint; JSON mode remains parseable and keeps the existing top-level events shape. If maintainers want machine-readable per-calendar continuation metadata for --all --json, I’d treat that as a separate JSON-shape change.

Validation:

$ go test ./internal/cmd -run 'TestListAllCalendarsEvents|TestExecuteCalendarEvents' -count=1
ok  	github.com/steipete/gogcli/internal/cmd	0.033s

GitHub CI is also green on this PR: darwin-cgo-build, image, test, windows, and worker all passed.

@TurboTheTurtle

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

steipete and others added 2 commits June 17, 2026 23:45
Co-authored-by: Andy Ye <35905412+TurboTheTurtle@users.noreply.github.com>
@steipete steipete merged commit 7712dc6 into openclaw:main Jun 18, 2026
5 checks passed
@steipete

Copy link
Copy Markdown
Collaborator

Landed as 7712dc6.

Verification:

  • go test ./internal/cmd -run 'TestListAllCalendarsEvents|TestExecute_CalendarEvents' -count=1
  • make ci
  • real Calendar API with calendar events --all --max=1: JSON parsed with two events and two per-calendar continuation tokens; stderr remained empty
  • all PR checks passed: Linux test, Windows, Darwin CGO, Docker image, worker
  • autoreview clean

The JSON change is additive: existing events remains, with nextPageTokens added for machine-readable continuation.

@TurboTheTurtle TurboTheTurtle deleted the tt/gogcli-821-calendar-truncation branch June 18, 2026 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants