fix(prisma-cloud): bound the last three Management API listing loops - #204
Conversation
PR #195 introduced the bounded page driver and converted the loops it touched; three hand-rolled, unbounded loops stayed on the deploy path. listAllProjects (container.ts) and listAllConnections (state/bootstrap.ts) now use collectPages. The env-var visibility search in target's preflight speaks the SDK's Promise {data, error} shape directly, so pagination.ts gains drivePagesAsync — a Promise twin of drivePages sharing the page cap and non-advancing-cursor failure, kept as a sibling loop so the caller's own thrown errors (listFailedError) propagate unwrapped — and preflight's short-circuiting search runs through it. pagination.ts is exported from the lowering barrel so target can import it like every other lowering helper. Fakes for /v1/projects and the env-var listing gained real cursor paging plus stuck- and runaway-cursor modes; both failure paths are pinned per package. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
|
Warning Review limit reached
Next review available in: 51 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Summary by CodeRabbit
WalkthroughThe change replaces manual cursor loops for projects, connections, and environment variables with shared bounded pagination helpers. It adds the Promise-based 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.drive/deferred.md:
- Around line 179-183: Insert one blank line after each of the headings “Remove
the composer-demo CI USER workaround (after TML-3157 ships)” and “Convert the
remaining Management API listing loops to drivePages” in the deferred
documentation.
In `@packages/1-prisma-cloud/0-lowering/lowering/src/pagination.ts`:
- Around line 87-90: Update the pagination logic in collectPages and drivePages
to return only when hasMore is false; when hasMore is true with a null
nextCursor, throw brokenPaginationError instead of treating the listing as
complete. Preserve the non-advancing cursor check and add regression coverage
for both drivers.
In `@packages/1-prisma-cloud/0-lowering/lowering/src/state/bootstrap.ts`:
- Around line 78-84: Update cleanupAgedConnections so PrismaApiError failures
from listAllConnections, including stuck or MAX_PAGES-exceeded pagination,
propagate out of bootstrap instead of being swallowed by the outer
Effect.ignore; retain best-effort ignoring only for individual deleteConnection
failures. Add a bootstrap test covering a non-advancing connection cursor or
capped connection pages and assert the cleanup/bootstrap operation fails.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: efca5cdd-5b20-4f2b-a344-776eb5f9747f
📒 Files selected for processing (8)
.drive/deferred.mdpackages/1-prisma-cloud/0-lowering/lowering/src/__tests__/container.test.tspackages/1-prisma-cloud/0-lowering/lowering/src/container.tspackages/1-prisma-cloud/0-lowering/lowering/src/exports/index.tspackages/1-prisma-cloud/0-lowering/lowering/src/pagination.tspackages/1-prisma-cloud/0-lowering/lowering/src/state/bootstrap.tspackages/1-prisma-cloud/1-extensions/target/src/__tests__/preflight.test.tspackages/1-prisma-cloud/1-extensions/target/src/preflight.ts
Both page drivers treated `hasMore: true` with `nextCursor: null` as a clean end, so a listing the API itself says is incomplete came back as if it were complete — the exact outcome the drivers exist to prevent. Both now return only when `hasMore` is false, and raise the shared broken- pagination error when more pages are reported without a cursor to fetch them with. The non-advancing-cursor check, the 1000-page cap, and the early stop when `onPage` returns true are unchanged. The lowering and target fakes gained a matching mode, and each driver has a test pinning the new failure. Without the fix, the target test shows the old behaviour concretely: preflight reported a provisioned secret as missing because it stopped reading after the first page. Also rewraps the two newest `.drive/deferred.md` entries to the file's style — blank line after the heading (markdownlint MD022), prose wrapped at 80 columns. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
Reconciles the preflight env-var listing: main bounded the loop with drivePagesAsync (#204); this branch made the same loop track the newest updatedAt for the deploy fingerprint. The merged version walks every page through the bounded driver and keeps both sides' tests. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
2 participants
Converts the three remaining hand-rolled cursor-pagination loops to the bounded page driver introduced in #195, so a misbehaving Management API (a non-advancing cursor, or endless pages) fails a deploy loudly instead of hanging it — in the
listAllConnectionscase, hanging while holding the deploy lock.Changes
listAllProjects(lowering/src/container.ts) andlistAllConnections(lowering/src/state/bootstrap.ts): shape-only conversion tocollectPages— same accumulate-everything semantics on a healthy API.target/src/preflight.ts): converted to a newdrivePagesAsync— a Promise twin ofdrivePagessharing the page cap and error builder. It is deliberately a sibling loop rather than an Effect wrapper: routing the fetch throughEffect.runPromisewould re-wrap preflight's ownlistFailedErroras a FiberFailure, changing the error identity its callers see (the doc comment records this). The short-circuit on the first visible row is preserved.pagination.tsjoins the lowering barrel (export *, matching the file's idiom) sotargetcan import it.With this, no hand-rolled Management API cursor loop remains in the package (swept during review).
Why
#195's review found this loop shape can spin forever on a stuck cursor; it bounded the three loops that PR touched and deferred these three, which run on every deploy. Same trade-off as accepted there: a broken listing fails the deploy with a named error instead of hanging or acting on incomplete data.
Verification: lowering 145 / target 342 tests pass (both failure paths pinned per package), lint clean, cast-ratchet delta 0, typecheck 74/74.
🤖 Generated with Claude Code