nightly: 1 production fix (2026-06-05) - #220
Closed
njrini99-code wants to merge 1 commit into
Closed
Conversation
Sentry 24h on release 22e1d32 (latest prod, deployed ~22h ago): JAVASCRIPT-NEXTJS-50: 46 events / 1 user (escalating), pg_error_code 42501 "Auto-save update failed: permission denied for table golf_rounds" JAVASCRIPT-NEXTJS-4Z: 46 mirrored server-side events (same root) JAVASCRIPT-NEXTJS-51: 22 client-side captures (same root, UI wrapper) JAVASCRIPT-NEXTJS-58: 2 new events, "Error updating shot:" — same trace (3a53170303b74d38b63d28e3f2e96078) as #50, edit-shot path that fans out into the same savePartialRound call Root cause: PR #217 shipped supabase/migrations/20260603040000_grant_update_ golf_rounds_authenticated.sql (column GRANT for player_id, team_id, qualifier_id, qualifier_round_number), but the migration file was on disk in main yet never applied to the Supabase production database — this repo applies migrations manually via the Supabase MCP (no auto-apply step in build), so the GRANT never landed. The fallback UPDATE path in savePartialRound (golf.ts:3914, hit when the client doesn't pass existingRoundId and we look up the in-progress round) kept 42501-ing on every auto-save tick for the affected user. Two-part fix that's safe regardless of whether the prior migration ever gets applied: 1. golf.ts:3911-3921 — strip the four identity columns from the auto-save UPDATE payload via object destructure. Identity columns are set on INSERT and never change for an in-progress round; re-SETting them on each auto-save tick is pointless AND triggers Postgres' per-column UPDATE-privilege check on columns the baseline GRANT intentionally omits. After the strip, savePartialRound's UPDATE no longer needs column grants on identity columns at all. RLS continues to scope rows. 2. supabase/migrations/20260605040000_reaffirm_golf_rounds_update_grants.sql — restate the same GRANT under a fresh timestamp so the next `supabase db push` / MCP apply picks it up. Idempotent no-op if the prior 20260603040000 has since been applied. Aligns DB to code for any other UPDATE caller that might still SET these columns. NEEDS HUMAN STEP: apply the migration to prod Supabase via MCP. The code change alone fixes the failing path, but the migration brings the DB to the documented end-state. Typecheck: pre-existing vitest/globals TS2688 (1 error on main, 1 with this change — zero new errors). No related vitest unit tests exist for savePartialRound. Skipped from the 24h triage: - JAVASCRIPT-NEXTJS-57 (fetchShotDriversByCategory timeout, 5 events / 0 users) — actively addressed by PR #219, deploy currently INITIALIZING. - JAVASCRIPT-NEXTJS-52 (Error deleting shot, 1 event) — single-occurrence transient on the same code path; resolved by the same fix above.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
njrini99-code has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Summary by CodeRabbitBug Fixes
WalkthroughDatabase grants and application logic now coordinate to restrict golf_rounds updates. A new migration reapplies column-scoped UPDATE permissions to the authenticated role, and savePartialRound filters the payload to exclude identity columns before updating in-progress rounds. ChangesGolf Round Update Permissions
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 8❌ Failed checks (1 warning, 7 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.43.0)src/app/golf/actions/golf.tsError: Cannot parse rule /ast-grep-rules/../git/.coderabbit/ast-grep/no-explicit-any.yml ✖ Caused by supabase/migrations/20260605040000_reaffirm_golf_rounds_update_grants.sqlError: Cannot parse rule /ast-grep-rules/../git/.coderabbit/ast-grep/no-explicit-any.yml ✖ Caused by 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Owner
Author
|
Folded into #304 (feat/coachhelm-stats-roundup) — merged clean, combined gates green. Branch intact + reopenable. |
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
2 participants
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.
Daily brief — 2026-06-05 (24h window)
Totals
dpl_Bwpo7xDaXPMco9X1tDBPLBWQ17AT, commit22e1d32, 22h ago) — stateREADY. 0 failed deploys.dpl_BQn2Ljknrf2Us1xXYZ3iFs2xu8d9, PR fix(coachhelm): log best-effort insight-delivery failures as warning #219, severity-demotion fix for wave21: engine_version + BaseGenerator + PuttDistanceGenerator #57) — stateINITIALIZING.Top patterns
savePartialRound.updateExisting42501s on every auto-save tick. Migration20260603040000(column GRANT for player_id/team_id/qualifier_id/qualifier_round_number) was committed in PR #217 but never applied to prod Supabase — repo applies migrations manually via MCP, no auto-apply step.fetchShotDriversByCategoryhits Postgres statement timeout on the IN-list-of-200-round-ids + nested joins query. Best-effort enrichment — page renders fine. Already addressed by in-flight PR #219 (demotes severity fromerrortowarningto stop paging).savePartialRoundUPDATE path; same root cause.Performance
Could not pull
pg_stat_statements— Supabase MCP is unauthenticated in this remote session (Host-not-in-allowlist on the OAuth callback). No DB-side query timing available. The one timeout we DO see (issue #57) is thegolf_shotsshot-drivers query on/golf/dashboard/coachhelm(200 round_id IN-list + nested 1:1 joins, 5000-shot cap) — a known heavy query already gated by best-effort try/catch.Deployment health
dpl_Bwpo7xDaXPMco9X1tDBPLBWQ17AT22e1d32dpl_BQn2Ljknrf2Us1xXYZ3iFs2xu8d951dbc76No
Error-state deploys in the 24h window.Fixes applied
1. golf_rounds auto-save 42501 cluster (#50 / #4Z / #51 / #58 / #52) — 117 of 122 events
File:
src/app/golf/actions/golf.ts(around line 3911)Diagnosis:
savePartialRoundconstructsroundDatawith player_id/team_id/qualifier_id/qualifier_round_number every tick. On the fallback path (noexistingRoundIdpassed, in-progress round found by query), it callssupabase.from('golf_rounds').update(roundData)directly. Postgres checks per-column UPDATE privilege on every column in the SET list regardless of whether the value differs from the current row — and the baselineauthenticatedGRANT intentionally omits identity columns. The previous fix migration20260603040000was shipped in the deploy commit but never applied to prod Supabase, so prod has been 42501-ing for ~22h on the same release SHA.Fix: Strip the four identity columns from the UPDATE payload via object destructure. Identity columns are set on INSERT and don't change for an in-progress round — re-SETting them is gratuitous AND a permission liability. After the strip, this code path no longer depends on column grants for identity columns at all. RLS continues to scope rows.
2. Belt-and-suspenders migration
File:
supabase/migrations/20260605040000_reaffirm_golf_rounds_update_grants.sqlDiagnosis: Same as #1 — the previous migration file exists on disk but is unapplied in prod. Restating the same GRANT under a fresh timestamp ensures the next
supabase db push/ MCP apply picks it up. Idempotent in Postgres, so a safe no-op if the prior was already applied.NEEDS HUMAN STEP: Apply this migration to prod Supabase via the MCP (or
supabase db push). The code change alone fixes the failing path even if the migration is never applied; this just brings the DB back to the documented end-state.Couldn't fix / unresolved
admin_events,error_logs, orpg_stat_statements. The Sentry data covered the actual error patterns; the DB-side tables would have given a second signal source. If those tables carry independent errors not seen by Sentry, this run missed them.fetchShotDriversByCategorystatement timeout) — skipped because PR fix(coachhelm): log best-effort insight-delivery failures as warning #219 is the active fix and was deploying as I wrote this. That PR only demotes severity (warning instead of error). The underlying timeout on the IN-200-round-ids + nested-joins query is still there; if it recurs, the real fix is to reduceSHOT_DRIVERS_ROUNDS_CAPor rewrite as a subquery.🤖 Generated nightly by Claude Code. Do not auto-merge — human review required.
Generated by Claude Code