Skip to content

fix(catalog): forward pagination cursor for planes and pipelines - #765

Open
InduwaraSMPN wants to merge 2 commits into
openchoreo:mainfrom
InduwaraSMPN:pr-a2/fetch-all-pages-misuse-fixes
Open

fix(catalog): forward pagination cursor for planes and pipelines#765
InduwaraSMPN wants to merge 2 commits into
openchoreo:mainfrom
InduwaraSMPN:pr-a2/fetch-all-pages-misuse-fixes

Conversation

@InduwaraSMPN

Copy link
Copy Markdown

Stacked on #764 (merge that first; this PR's diff then shows only its own commit).

Three fetchAllPages closures in the scheduled OpenChoreoEntityProvider were written () => instead of cursor => and omitted the limit query parameter, so workflowplanes, observabilityplanes and deploymentpipelines silently ingested only the first server-default page per namespace. Every other list call in the provider passes cursor and limit: 100 — these three now do too.

Notes for reviewers: the four limit: 1000 sites sometimes mentioned alongside these (WorkflowService.ts:532,675, GenericWorkflowService.ts:583, ObservabilityApi.ts:423) are not bugs — they are POST bodies to the observability query API (/api/v1/logs/query, /api/v1/events/query), whose own spec documents maximum: 1000 for limit. The max-100 LimitParam belongs to the OpenChoreo API's cursor-paginated list endpoints; the two APIs have different ceilings. Left untouched.

Found while auditing every fetchAllPages call site for #764; kept out of that PR to keep it at exactly four files.

Induwara added 2 commits August 23, 2026 07:21
…r guards

fetchAllPages gains an optional options bag without changing behaviour for
existing single-argument callers:

- timeoutMs: wall-clock budget for the entire run. Defaults to 60s;
  0 disables the budget. One deadline, one setTimeout raced per page,
  cleared in finally.
- signal: AbortSignal cancellation, checked at entry and between pages;
  the once-listener is removed in finally so it never leaks.
- maxPages: opt-in hard cap on pages fetched. Exceeding it throws with
  the collected item count - it never silently truncates, since a
  truncated catalog sync looks like a successful one.
- Malformed-page guards: a null/undefined page or a page without an
  items array throws naming the page index and cursor instead of
  surfacing as a bare TypeError.
- Stuck-cursor guard: a server echoing the request cursor back as
  nextCursor throws instead of looping forever. One guard only.
- nextCursor of null or empty string is treated as terminal, matching
  undefined.

Also exports PaginatedResponse and FetchAllPagesOptions from the package
barrel. Statement coverage of pagination-utils.ts is 100%; the seven
pre-existing tests pass unmodified.

Signed-off-by: Induwara <induwara@induwara.com>
Three fetchAllPages closures in OpenChoreoEntityProvider (workflowplanes,
observabilityplanes, deploymentpipelines) were written as () => instead of
cursor => and omitted the limit query parameter, so those resource types
silently ingested only the first server-default page per namespace.
Forward the cursor and pass limit: 100 like every other list call in the
provider.

Note: the four limit: 1000 sites in ci-backend/workflows-backend/
observability POST bodies are NOT changed - the observability query API
documents maximum: 1000 for its limit, unlike the OpenChoreo API's
cursor-paginated list endpoints which cap at 100.

Signed-off-by: Induwara <induwara@induwara.com>
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 59 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 763f7659-fe3d-47cd-abce-8e51b0b7e96e

📥 Commits

Reviewing files that changed from the base of the PR and between 2f36a17 and a06aa12.

📒 Files selected for processing (6)
  • .changeset/fetch-all-pages-cursor-forwarding.md
  • .changeset/fetch-all-pages-hardening.md
  • packages/openchoreo-client-node/src/index.ts
  • packages/openchoreo-client-node/src/pagination-utils.test.ts
  • packages/openchoreo-client-node/src/pagination-utils.ts
  • plugins/catalog-backend-module-openchoreo/src/provider/OpenChoreoEntityProvider.ts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.14815% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...penchoreo/src/provider/OpenChoreoEntityProvider.ts 87.50% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant