Follow next_page_token in SearchEntitlements

[chinkulkarni]

Problem

SearchEntitlements in pkg/client/entitlement.go made a single search request (page size 100) and ignored next_page_token. The server is starting to return paginated results for this endpoint, so a single-page client silently truncates results (affecting search-entitlements, generate-alias, aws, and entitlement lookup in get-drop-task).

Change

• SearchEntitlements now loops on next_page_token, passing it back until it is empty, keeping page size 100 and the existing return type and error handling. It checks ctx cancellation between pages.
• Expanded objects are referenced by index into each response's own expanded array, so each page is resolved against its own expanded list before results are appended (merging arrays across pages would make the indexes wrong). Callers only read per-entitlement expanded maps, so none needed changes.

Testing

• go build ./... and go test ./pkg/... ./cmd/... pass.
• Added pkg/client/entitlement_test.go: an httptest server serving three pages, asserting tokens are passed back, results are accumulated in order, and each entitlement resolves its own page's expanded app (every page uses expanded[0]); plus a ctx cancellation test.

🤖 Generated with Claude Code

[github-actions]

Superseded — see the current review report for commit 62201c506d17

General PR Review: Follow next_page_token in SearchEntitlements

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base 5ffe74330f1c.
Review mode: full
View review run

Review Summary

SearchEntitlements (pkg/client/entitlement.go:100-148) used to make one request. It now loops: it checks ctx before each page and passes next_page_token back until the token is empty. Each page goes through the new convertSearchEntitlementsPage, which resolves $.expanded[N] paths against that page's own expanded array before appending the results, so indexes stay correct across pages. The return type and error behavior stay the same. A nil list still errors on the first page and is tolerated on later pages. A nil response body now returns an error instead of risking a nil dereference. New tests in pkg/client/entitlement_test.go cover token threading, ordering, per-page expanded resolution and ctx cancellation, using existing helpers (newPaperSecretTestClient, atTypeApp, GetExpanded). I scanned the full PR diff for security and correctness issues. No repo-local criteria loaded, so the base criteria were applied. No prior findings existed, and there are no coverage gaps.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• New (medium confidence) pkg/client/entitlement.go:142-145: nothing guards against a repeated or cyclic next_page_token. If the server misbehaves, the loop never ends and rv keeps growing until ctx is cancelled, and CLI callers generally pass a context with no deadline.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `pkg/client/entitlement.go`:
- Around line 142-145: The pagination loop in SearchEntitlements trusts the server to eventually return an empty next_page_token. Add a guard: if *body.NextPageToken equals the token just sent (StringFromPtr(pageToken)), return an error (e.g. "search-entitlements: server returned repeated page token"); optionally also cap the number of pages (e.g. 10,000) and return an error when exceeded. Add a test case where the server always returns the same token and assert an error is returned.

[github-actions]

🟡 Suggestion (medium confidence): nothing guards against a server that keeps returning the same next_page_token (or a cycle of tokens). That case loops forever and keeps growing rv until ctx is cancelled, and most CLI callers pass a context with no deadline. Consider breaking or erroring when *body.NextPageToken == StringFromPtr(pageToken), and/or capping the page count.

[github-actions]

General PR Review: Follow next_page_token in SearchEntitlements

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base 5ffe74330f1c.
Review mode: full
View review run

Review Summary

SearchEntitlements (pkg/client/entitlement.go:100-148) used to make one request. It now loops: it checks ctx before each page and passes next_page_token back until the token is empty. Each page goes through the new convertSearchEntitlementsPage, which resolves $.expanded[N] paths against that page's own expanded array before appending the results, so indexes stay correct across pages. The return type and error behavior stay the same. A nil list still errors on the first page and is tolerated on later pages. A nil response body now returns an error instead of risking a nil dereference. New tests in pkg/client/entitlement_test.go cover token threading, ordering, per-page expanded resolution and ctx cancellation, using existing helpers (newPaperSecretTestClient, atTypeApp, GetExpanded). I scanned the full PR diff for security and correctness issues. No repo-local criteria loaded, so the base criteria were applied. No prior findings existed, and there are no coverage gaps.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• New (medium confidence) pkg/client/entitlement.go:142-145: nothing guards against a repeated or cyclic next_page_token. If the server misbehaves, the loop never ends and rv keeps growing until ctx is cancelled, and CLI callers generally pass a context with no deadline.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `pkg/client/entitlement.go`:
- Around line 142-145: The pagination loop in SearchEntitlements trusts the server to eventually return an empty next_page_token. Add a guard: if *body.NextPageToken equals the token just sent (StringFromPtr(pageToken)), return an error (e.g. "search-entitlements: server returned repeated page token"); optionally also cap the number of pages (e.g. 10,000) and return an error when exceeded. Add a test case where the server always returns the same token and assert an error is returned.

---

Reviewed commit: 62201c506d17

[github-actions]

No blocking issues found — see the full review report