Eric Allam
4c21af8669
## What 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. That has shipped three times recently and had to be fixed after the fact (#4554 `ProjectAlert.channelId`, #4555 `EnvironmentVariableValue.valueReferenceId`, #4588 `PersonalAccessToken.userId`). This adds a schema-aware CI guard that catches the next one before it merges. ## How `apps/webapp/scripts/fkCascadeIndexGuard.ts` parses both Prisma schemas (`@trigger.dev/database`, `@internal/run-ops-database`) and flags any `onDelete: Cascade | SetNull` relation 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's `WHERE fk = $1` use the index instead of a seq scan. It is modeled on the existing `runOpsLegacyGuard` (same `--check` gate, same baseline-regenerate pattern), and it is lighter: it only reads `schema.prisma` as 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()` vs `parent.update({ deletedAt })`), not in the schema, and a `deletedAt` column 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-index` script (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 into `all-checks`. ## Verification - The three already-fixed columns are correctly seen as indexed (absent from the baseline). - `--check` passes on the current schemas (72 baselined, 0 new). - A synthetic new unindexed cascade FK fails with exit 1 and an actionable message. - Adding `@@index([fk])`, or a composite leading with the FK, clears it. No false positives. - `oxfmt` and `oxlint` clean on the new script. ## Rollback Pure tooling addition, no runtime code, no schema or data change. Revert to remove.
237 lines
8.7 KiB
YAML
237 lines
8.7 KiB
YAML
name: 🤖 PR Checks
|
|
|
|
on:
|
|
pull_request:
|
|
types: [opened, synchronize, reopened]
|
|
|
|
concurrency:
|
|
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
|
|
cancel-in-progress: true
|
|
|
|
permissions:
|
|
contents: read
|
|
pull-requests: read
|
|
|
|
jobs:
|
|
changes:
|
|
name: Detect changes
|
|
runs-on: warp-ubuntu-latest-x64-2x
|
|
outputs:
|
|
code: ${{ steps.code_filter.outputs.code }}
|
|
typecheck_self: ${{ steps.filter.outputs.typecheck_self }}
|
|
webapp: ${{ steps.filter.outputs.webapp }}
|
|
packages: ${{ steps.filter.outputs.packages }}
|
|
internal: ${{ steps.filter.outputs.internal }}
|
|
obsmap: ${{ steps.filter.outputs.obsmap }}
|
|
cli: ${{ steps.filter.outputs.cli }}
|
|
sdk: ${{ steps.filter.outputs.sdk }}
|
|
steps:
|
|
# `code` uses `every` semantics so the negation patterns actually subtract.
|
|
# With the default `some` quantifier, `**` matches every file and the
|
|
# subsequent `!...` patterns are no-ops (each pattern is OR'd, not AND'd).
|
|
- uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1
|
|
id: code_filter
|
|
with:
|
|
predicate-quantifier: every
|
|
filters: |
|
|
code:
|
|
- '**'
|
|
- '!docs/**'
|
|
- '!.changeset/**'
|
|
- '!hosting/**'
|
|
- '!.github/**'
|
|
- '!**/*.md'
|
|
- '!**/.env.example'
|
|
- uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1
|
|
id: filter
|
|
with:
|
|
filters: |
|
|
typecheck_self:
|
|
- '.github/workflows/pr_checks.yml'
|
|
- '.github/workflows/typecheck.yml'
|
|
- '.github/workflows/code-quality.yml'
|
|
webapp:
|
|
- 'apps/webapp/**'
|
|
- 'packages/**'
|
|
- 'internal-packages/**'
|
|
- '.github/workflows/pr_checks.yml'
|
|
- '.github/workflows/unit-tests-webapp.yml'
|
|
- '.github/workflows/e2e-webapp.yml'
|
|
- '.github/workflows/runops-guard.yml'
|
|
- '.github/workflows/fk-cascade-guard.yml'
|
|
- '.configs/**'
|
|
- 'package.json'
|
|
- 'pnpm-lock.yaml'
|
|
- 'pnpm-workspace.yaml'
|
|
- 'turbo.json'
|
|
packages:
|
|
- 'packages/**'
|
|
- '.github/workflows/pr_checks.yml'
|
|
- '.github/workflows/unit-tests-packages.yml'
|
|
- '.configs/**'
|
|
- 'package.json'
|
|
- 'pnpm-lock.yaml'
|
|
- 'pnpm-workspace.yaml'
|
|
- 'turbo.json'
|
|
internal:
|
|
- 'internal-packages/**'
|
|
- 'packages/**'
|
|
- '.github/workflows/pr_checks.yml'
|
|
- '.github/workflows/unit-tests-internal.yml'
|
|
- '.configs/**'
|
|
- 'package.json'
|
|
- 'pnpm-lock.yaml'
|
|
- 'pnpm-workspace.yaml'
|
|
- 'turbo.json'
|
|
# The whole webapp app tree, not just its routes, and that is the whole reason this
|
|
# filter exists. Two tests in @internal/observability-map read it: integration.test.ts
|
|
# scans the live route tree, and webappSymbols.test.ts walks all of apps/webapp/app and
|
|
# fails when a guard, sensitive or audit symbol stops resolving. Routes-only was this
|
|
# filter's own bug: renaming e.g. requireUserId in app/services/session.server.ts
|
|
# matched `webapp` and nothing else, so no job ran the suite and the break landed on
|
|
# main, or on the next unrelated internal-packages PR.
|
|
#
|
|
# The cost of the wider set, measured over the last 400 commits on main: 31% touch
|
|
# routes, 52% touch apps/webapp/app, so the job goes from firing on roughly a third of
|
|
# PRs to roughly a half. It is the cheap one -- a single 4x runner, no containers, no
|
|
# database, no prisma generate -- which is what makes that affordable.
|
|
#
|
|
# observability-map.yml is here because integration.test.ts asserts on its text and no
|
|
# other filter watches it, so editing the report workflow alone ran nothing at all.
|
|
#
|
|
# Deliberately NOT here: this package's own paths, and packages/plugins/src and
|
|
# internal-packages/rbac/src, the other two trees webappSymbols.test.ts reads.
|
|
# `internal` above already matches `internal-packages/**` and `packages/**`, and
|
|
# `unit-tests-internal.yml` runs `turbo run test --filter "@internal/*"`, which picks up
|
|
# @internal/observability-map and runs the same vitest suite. Listing them here as well
|
|
# ran the suite twice on every PR touching them, which was this filter's own doing.
|
|
#
|
|
# Also deliberately NOT here: pr_checks.yml, package.json, pnpm-lock.yaml,
|
|
# pnpm-workspace.yaml. `internal` already lists all four, so a PR touching only one of
|
|
# them ran this suite twice for the same reason as above. Editing pr_checks.yml no
|
|
# longer runs this job live as a result; integration.test.ts still asserts on its text
|
|
# via the `internal` job.
|
|
obsmap:
|
|
- 'apps/webapp/app/**'
|
|
- '.github/workflows/unit-tests-observability-map.yml'
|
|
- '.github/workflows/observability-map.yml'
|
|
cli:
|
|
- 'packages/cli-v3/**'
|
|
- 'packages/build/**'
|
|
- 'packages/core/**'
|
|
- 'packages/schema-to-json/**'
|
|
- '.github/workflows/pr_checks.yml'
|
|
- '.github/workflows/e2e.yml'
|
|
- '.configs/**'
|
|
- 'package.json'
|
|
- 'pnpm-lock.yaml'
|
|
- 'pnpm-workspace.yaml'
|
|
- 'turbo.json'
|
|
sdk:
|
|
- 'packages/trigger-sdk/**'
|
|
- 'packages/core/**'
|
|
- '.github/workflows/pr_checks.yml'
|
|
- '.github/workflows/sdk-compat.yml'
|
|
- '.configs/**'
|
|
- 'package.json'
|
|
- 'pnpm-lock.yaml'
|
|
- 'pnpm-workspace.yaml'
|
|
- 'turbo.json'
|
|
|
|
code-quality:
|
|
uses: ./.github/workflows/code-quality.yml
|
|
|
|
typecheck:
|
|
needs: changes
|
|
if: needs.changes.outputs.code == 'true' || needs.changes.outputs.typecheck_self == 'true'
|
|
uses: ./.github/workflows/typecheck.yml
|
|
|
|
runops-guard:
|
|
needs: changes
|
|
if: needs.changes.outputs.webapp == 'true'
|
|
uses: ./.github/workflows/runops-guard.yml
|
|
|
|
fk-cascade-guard:
|
|
needs: changes
|
|
if: needs.changes.outputs.webapp == 'true'
|
|
uses: ./.github/workflows/fk-cascade-guard.yml
|
|
|
|
webapp:
|
|
needs: changes
|
|
if: needs.changes.outputs.webapp == 'true'
|
|
uses: ./.github/workflows/unit-tests-webapp.yml
|
|
secrets:
|
|
DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }}
|
|
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
|
|
|
|
e2e-webapp:
|
|
needs: changes
|
|
if: needs.changes.outputs.webapp == 'true'
|
|
uses: ./.github/workflows/e2e-webapp.yml
|
|
secrets:
|
|
DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }}
|
|
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
|
|
|
|
packages:
|
|
needs: changes
|
|
if: needs.changes.outputs.packages == 'true'
|
|
uses: ./.github/workflows/unit-tests-packages.yml
|
|
secrets:
|
|
DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }}
|
|
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
|
|
|
|
internal:
|
|
needs: changes
|
|
if: needs.changes.outputs.internal == 'true'
|
|
uses: ./.github/workflows/unit-tests-internal.yml
|
|
secrets:
|
|
DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }}
|
|
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
|
|
|
|
obsmap:
|
|
needs: changes
|
|
if: needs.changes.outputs.obsmap == 'true'
|
|
uses: ./.github/workflows/unit-tests-observability-map.yml
|
|
|
|
e2e:
|
|
needs: changes
|
|
if: needs.changes.outputs.cli == 'true'
|
|
uses: ./.github/workflows/e2e.yml
|
|
with:
|
|
package: cli-v3
|
|
|
|
sdk-compat:
|
|
needs: changes
|
|
if: needs.changes.outputs.sdk == 'true'
|
|
uses: ./.github/workflows/sdk-compat.yml
|
|
|
|
all-checks:
|
|
name: All PR Checks
|
|
needs:
|
|
- changes
|
|
- code-quality
|
|
- typecheck
|
|
- runops-guard
|
|
- fk-cascade-guard
|
|
- webapp
|
|
- e2e-webapp
|
|
- packages
|
|
- internal
|
|
- obsmap
|
|
- e2e
|
|
- sdk-compat
|
|
if: always()
|
|
runs-on: warp-ubuntu-latest-x64-2x
|
|
steps:
|
|
- name: Verify all checks
|
|
run: |
|
|
if [[ "${{ contains(needs.*.result, 'failure') }}" == "true" ]]; then
|
|
echo "One or more checks failed"
|
|
exit 1
|
|
fi
|
|
if [[ "${{ contains(needs.*.result, 'cancelled') }}" == "true" ]]; then
|
|
echo "One or more checks were cancelled"
|
|
exit 1
|
|
fi
|
|
echo "All checks passed or were skipped due to path filters"
|