fix(env): scope branch-override lookup by branchId#109
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 Run ID: 📒 Files selected for processing (2)
Warning Walkthrough skippedFile diffs could not be summarized. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
`findVariableByNaturalKey` listed environment variables by `(projectId, class, key)` only and filtered by branch client-side, but the list endpoint is paginated (default 100 rows, ordered createdAt asc). Once a project accumulates more than a page of preview overrides for a key, the newest branch's row falls off the first page, so the lookup returns null even though the row exists. `project env update --branch` then reports the variable as not found, and `project env add --branch`'s pre-check also misses it and POSTs, which the server rejects with a 409. The two paths contradict each other and neither can touch an existing branch override. Pass `branchId` to the list query for branch-scoped lookups (the endpoint already supports the filter) so the server returns the single exact row regardless of how many branches exist. Role-scoped lookups are unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kristof-siket
force-pushed
the
fix/env-branch-scope-lookup
branch
from
cfe80a4 to
3eec258
Compare
|
@CodeRabbit review |
✅ Action performedReview finished.
|
|
@CodeRabbit approve |
✅ Action performedComments resolved and changes approved. |
1 participant
Problem
project env update --branchandproject env add --branchfail contradictorily on any branch that already has an override for a key:add --branch→ server 409 "already exists in this scope"update --branch→ CLIENV_VARIABLE_NOT_FOUND"not found in branch:…"One says the variable exists, the other says it doesn't. Neither path can touch an existing branch override, so re-running a workflow that sets a per-branch override hard-fails.
Root cause
findVariableByNaturalKeylisted environment variables by(projectId, class, key)and filtered by branch client-side, inspecting only the first page.GET /v1/environment-variablesis paginated (defaultlimit=100, orderedcreatedAt asc). Once a project accumulates more than a page of preview overrides for a given key, the newest branch's row sorts onto a later page and the lookup returnsnulleven though the row exists on the server.Downstream:
update's pre-lookup returns null → throwsENV_VARIABLE_NOT_FOUNDbefore it ever PATCHes.add's existence pre-check also returns null → it POSTs → the server (which enforces uniqueness onprojectId, branchId, class, key) rejects with 409.The endpoint already supports a
branchIdfilter — the lookup just wasn't using it.Why this surfaced now
This is a latent bug (the branch-override lookup has never passed
branchId), triggered by data volume rather than a code change. In one affected project, the count of preview-class rows forBUILD_RUNNER_BASE_URLhad just crossed the page limit (104 rows, 104 distinct branches, 4 past the first page) — so the freshly-created branch override was the row that fell off page 1. First deploy of a branch (which takes theadd/create path) still worked; every re-deploy (the lookup path) failed. That first-run-vs-rerun signature is exactly what the pagination boundary produces.Fix
Pass
branchIdto the list query for branch-scoped lookups so the server returns the single exact row regardless of how many branches exist. Role-scoped lookups (branchId === null) are unchanged.Testing
pnpm --filter @prisma/cli exec tsc --noEmit— cleanpnpm --filter @prisma/cli test— 563 passedbranchId; verified it fails without the source change and passes with it.Known residual (out of scope)
Role-scoped (template) lookups still fetch a single page and filter
branchId === nullclient-side — the same pagination class of bug in theory — but the list query type only acceptsbranchId?: string, so there's no way to express "branchId IS NULL" as a server filter today, and template rows are singular per key and created early (page 1). Worth a follow-up if the API grows a null-branch filter. Separately, the projects hitting this also have a data-hygiene problem: preview-branch overrides (and their branches/databases) are never cleaned up, which is what let the row count climb past the page limit in the first place.🤖 Generated with Claude Code