fix(backend): flip ai_analyzed via service role so idempotency holds#2331
Merged
Conversation
…ly holds
`POST /build/ai_analyze` ran the post-success
`UPDATE build_requests SET ai_analyzed = true` through the user-context
apikey client (`supabaseApikey`). The `build_requests` table has only
two RLS policies — `Allow org members to select build_requests` (SELECT
for org members) and `Service role manages build requests` (ALL for the
service role) — so the user-context UPDATE was silently blocked by RLS:
PostgREST reported zero rows matched, returned `{ error: null }`, and
the flag stayed `false` for every successful analysis.
Consequence: the `if (row.ai_analyzed === true)` idempotency guard at
the top of the handler never fired for any subsequent request on the
same job. A single failed build could be re-analyzed indefinitely, each
call burning a fresh Workers AI invocation and racking up cost.
Fix:
- Switch the post-success `UPDATE` to `supabaseAdmin(c)` (service-role
client), bypassing RLS for this targeted write. Authorization is
unchanged — it's still enforced upstream by `checkPermission` and the
user-context `SELECT` that fails closed if the row isn't accessible
to the caller. The SELECT remains on the apikey client.
- Add `.select('builder_job_id')` to the UPDATE so we can detect the
zero-rows-matched case (e.g. the row was deleted between our SELECT
and our UPDATE). The analysis has already happened by this point, so
we still don't throw — but we log loudly because idempotency is
broken for that job and someone needs to look.
### Tests
- Refactored `mockBuildRequestRow` to mock the apikey client (SELECT
only) and the admin client (UPDATE only) independently, returning
both `apikeyUpdate` and `adminUpdate` jest mocks.
- The success test now asserts:
* `adminUpdate` was called exactly once with `ai_analyzed: true`
* `apikeyUpdate` was NEVER called (regression guard — if anyone
moves the UPDATE back to the apikey client, this test fails)
- Added a new test: when the admin UPDATE matches zero rows, the
handler still returns the analysis and still fires the success Result
event (because the AI work succeeded), but the bookkeeping miss is
logged.
All 8 tests in `build-ai-analyze.test.ts` pass; related suites
(`ai-analysis-telemetry`, `build-lifecycle-emit`, `build-timeout`,
`build-tracking-helpers`) — 50/50 still green.
Contributor
Merging this PR will not alter performance
Comparing 
Footnotes
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe AI build analysis handler switches the idempotency flag update from the user-context Supabase client to the service-role admin client, adds detection for zero-row UPDATE results to identify when the idempotency guard fails, and updates tests to verify both the admin-client usage and zero-row defensive behavior. ChangesAI Analyze Idempotency Handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
POST /build/ai_analyzewas running its post-successUPDATE build_requests SET ai_analyzed = truethrough the user-context apikey client (supabaseApikey). Thebuild_requeststable has onlySELECT(org members) andALL(service role) RLS policies — noUPDATEpolicy for non-service-role users — so the UPDATE was silently blocked. PostgREST returned{ error: null }, the flag stayedfalse, and theif (row.ai_analyzed === true)idempotency guard never fired on subsequent requests for the same job.success).supabaseAdmin(c)(service-role client). The SELECT stays on the apikey client — that's the authorization gate and still fails closed if the row isn't accessible to the caller. Also added.select('builder_job_id')so we can detect the zero-rows-matched case and log it loudly.Test plan
mockBuildRequestRowto mock both clients independently (apikeyUpdateandadminUpdatejest mocks)adminUpdatecalled exactly once withai_analyzed: trueapikeyUpdatenever called (regression guard — if anyone moves the UPDATE back to the apikey client, this test fails)build-ai-analyze.test.tspassai-analysis-telemetry,build-lifecycle-emit,build-timeout,build-tracking-helpers)jobId, hit the endpoint twice, confirm the second call returns 409already_analyzedselect ai_analyzed from build_requests where builder_job_id = '<jobid>'returnstrueOut of scope
postAnalyzeRequestreturns{ kind: 'already_analyzed' }); no client change needed.UPDATE … SET ai_analyzed=falsebetween runs because it intentionally re-uses the same jobId. Once this fix lands, real (non-debug) runs will be idempotent automatically.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes