feat(webapp): CI guard for unindexed onDelete cascade FK columns - #4618
Conversation
A relation with onDelete Cascade/SetNull whose child FK column has no index makes every parent delete fire a cascade that sequentially scans the whole child table. This shipped three times recently (ProjectAlert.channelId, EnvironmentVariableValue.valueReferenceId, PersonalAccessToken.userId). Adds a schema-aware guard (modeled on runOpsLegacyGuard) that parses both Prisma schemas and flags any cascade/SetNull FK whose leading scalar is not the leading column of an index. Whether a missing index is a live bug depends on whether the parent is hard- or soft-deleted, which is not in the schema, so the guard carries a baseline of the 72 currently-accepted cases and only fails --check on new un-baselined violations. Wired into pr_checks as fk-cascade-guard.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (37)
🧰 Additional context used📓 Path-based instructions (6)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.ts📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (12)📚 Learning: 2026-03-22T13:26:12.060ZApplied to files:
📚 Learning: 2026-03-22T19:24:14.403ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-06-13T19:53:13.759ZApplied to files:
📚 Learning: 2026-06-17T17:13:49.929ZApplied to files:
📚 Learning: 2026-06-23T13:04:21.413ZApplied to files:
📚 Learning: 2026-05-12T21:04:05.815ZApplied to files:
📚 Learning: 2026-06-25T18:21:51.905ZApplied to files:
📚 Learning: 2026-07-03T17:10:21.498ZApplied to files:
📚 Learning: 2026-06-04T18:16:35.386ZApplied to files:
📚 Learning: 2026-06-09T17:58:04.699ZApplied to files:
🔇 Additional comments (1)
WalkthroughAdded a schema-aware guard for cascading and Merge Risk: ⚪ Minimal · up to This change adds a CI check for unindexed cascade foreign keys without changing runtime behavior or data, so no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…e, oxfmt-clean output Addresses CodeRabbit review on #4618: - Reconstruct logical statements by balancing brackets so a relation whose onDelete/fields span multiple lines is no longer skipped. - Match baseline entries on key + onDelete + ordered fkColumns, so changing a relation's FK column or flipping Cascade/SetNull re-triggers the guard instead of inheriting the old acceptance. Exit 2 on malformed baseline JSON/shape. - Emit the baseline in oxfmt's format (inline short arrays) so regeneration stays code-quality clean.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… validation Addresses CodeRabbit follow-up on #4618: - Mask string literals before counting delimiters and stripping // comments, so a quoted delimiter (e.g. a relation name containing a paren) or a quoted // (e.g. a URL @default) no longer splits a multiline relation and silently skips its cascade. - Baseline validation now requires onDelete to be Cascade or SetNull and fkColumns to be non-empty, rejecting a hand-edited baseline with junk values (exit 2).
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ent rot Addresses Devin review on #4618: --check only failed on NEW violations, so a baselined FK that later gets indexed leaves a stale entry that is never pruned. Because entries match by fingerprint, a later change that re-removes that index would be silently re-accepted by the leftover entry (the run-ops guard had the same drift). --check now also fails on baseline fingerprints no longer present in the schema, forcing a regenerate when a baselined FK is fixed or removed.
2 participants
What
A relation with
onDelete: Cascade | SetNullwhose child FK column has no index makes every parent delete fire a cascade that sequentially scans the whole child table. That has shipped three times recently and had to be fixed after the fact (#4554ProjectAlert.channelId, #4555EnvironmentVariableValue.valueReferenceId, #4588PersonalAccessToken.userId).This adds a schema-aware CI guard that catches the next one before it merges.
How
apps/webapp/scripts/fkCascadeIndexGuard.tsparses both Prisma schemas (@trigger.dev/database,@internal/run-ops-database) and flags anyonDelete: Cascade | SetNullrelation whose leading FK scalar is not the leading column of some index (@@index/@@unique/@@id/ field-level@id/@unique) on the child model. A leading FK column lets the cascade'sWHERE fk = $1use the index instead of a seq scan.It is modeled on the existing
runOpsLegacyGuard(same--checkgate, same baseline-regenerate pattern), and it is lighter: it only readsschema.prismaas text, so its CI job needs no Prisma client generation and no raised heap.Why a baseline, not a hard rule
Not every unindexed cascade FK is a live bug. When the parent is only ever soft-deleted, the cascade never fires, so the missing index is harmless. Hard vs soft delete lives in application code (
parent.delete()vsparent.update({ deletedAt })), not in the schema, and adeletedAtcolumn proves neither direction. So the guard makes no such judgment: it flags every unindexed cascade FK uniformly and carries a baseline of the 72 currently-accepted cases. Only violations not in the baseline fail--check.The value is the forcing function: a newly added cascade FK stops CI and makes the author answer "is the parent ever hard-deleted?" Add the index if yes; regenerate the baseline with a reason if no.
Wiring
apps/webapp/package.json:guard:fk-cascade-indexscript (regenerate with no args, gate with-- --check)..github/workflows/fk-cascade-guard.yml: the reusable workflow..github/workflows/pr_checks.yml: runs on webapp-affecting changes, aggregated intoall-checks.Verification
--checkpasses on the current schemas (72 baselined, 0 new).@@index([fk]), or a composite leading with the FK, clears it. No false positives.oxfmtandoxlintclean on the new script.Rollback
Pure tooling addition, no runtime code, no schema or data change. Revert to remove.