From be84d0fb948f0ac8c13552401051c749836e5c10 Mon Sep 17 00:00:00 2001 From: Eric Allam Date: Tue, 12 May 2026 14:33:57 +0100 Subject: [PATCH 1/4] chore: add .claude/REVIEW.md with CI drift check --- .claude/REVIEW.md | 49 +++++++++ .claude/scripts/check-review-md.sh | 137 ++++++++++++++++++++++++++ .github/workflows/check-review-md.yml | 58 +++++++++++ 3 files changed, 244 insertions(+) create mode 100644 .claude/REVIEW.md create mode 100755 .claude/scripts/check-review-md.sh create mode 100644 .github/workflows/check-review-md.yml diff --git a/.claude/REVIEW.md b/.claude/REVIEW.md new file mode 100644 index 00000000000..06c34394128 --- /dev/null +++ b/.claude/REVIEW.md @@ -0,0 +1,49 @@ +# REVIEW.md — Trigger.dev OSS + +Injected by `/code-review` as the highest-priority block in the reviewer's prompt. Repo-specific signal only; generic review rules live in the skill. + +## What makes a 🔴 Important finding here + +Reserve 🔴 for things that would page someone or block a rollback. In this codebase, that means: + +- **Rolling-deploy breakage.** Old and new versions of the webapp/supervisor run side-by-side during deploys. A change is broken if: + - A Lua script's behavior changes for a given key set without versioning (rename the script with a behavior-descriptive suffix like `Tracked` rather than `V2` — both versions must coexist safely). + - A Redis data shape used by both versions changes in place. New shapes need a new key namespace. + - A migration is not backward-compatible with the prior image. +- **Schema / migration safety.** Prisma migrations must be backward-compatible with the prior deploy. Adding NOT NULL without a default, dropping a column an old image still reads, renaming a column — all 🔴. +- **Queue / concurrency correctness.** RunQueue, MarQS (V1, legacy), redis-worker — any change to enqueue / dequeue / locking semantics. Re-derive the invariant on paper before flagging or accepting. +- **Missing index on a hot table.** New Prisma queries against `TaskRun`, `TaskRunExecutionSnapshot`, `JobRun`, `Project`, etc. must use an existing index. Check `internal-packages/database/prisma/schema.prisma` for the relevant `@@index` lines — don't guess and don't propose `EXPLAIN`. +- **Recovery-path queries.** Any `TaskRun.findFirst` / `findMany` added to a schedule, run-recovery, or restart loop. Recovery fan-outs (Redis crash, restart storms) turn "rare indexed query" into a DB incident. 🔴 even if indexed. +- **Aggregations on hot tables.** No `COUNT` / `GROUP BY` on `TaskRun` or other multi-million-row tables. Use Redis or ClickHouse for counts. +- **Prod Redis blast-radius.** New code paths that `SCAN` with broad patterns (`*foo*`) on prod-shaped Redis, or `EVAL` Lua with `SCAN` loops inside. Both are 🔴. +- **`@trigger.dev/core` direct import** from anywhere outside the SDK package. Always import from `@trigger.dev/sdk`. Core direct imports are 🔴 — they break the public API contract. +- **Heavy execute-deps imported into request-handler bundles.** Specifically `chat.handover` and similar split-bundle entry points must not transitively import the agent task's execute path. Watch for new imports added at module top-level of route files. +- **V1 engine code modified in a "V2 only" PR.** The `apps/webapp/app/v3/` directory contains both. If the PR description says V2-only but it touches `triggerTaskV1`, `cancelTaskRunV1`, `MarQS`, etc. — 🔴. + +## Always check + +- **Tests use testcontainers, not mocks.** Vitest with `redisTest` / `postgresTest` / `containerTest` from `@internal/testcontainers`. Any new `vi.mock(...)` on Redis, Postgres, BullMQ, or other infra is wrong here — 🔴 if added in production-path tests, 🟡 if isolated unit test. +- **Public-package changes have a changeset.** `pnpm run changeset:add` produces `.changeset/*.md`. Required for any edit under `packages/*`. Missing → 🟡; missing on a breaking change → 🔴. +- **Server-only changes have `.server-changes/*.md`.** Required for `apps/webapp/`, `apps/supervisor/` edits with no public-package change. Body should be 1-2 sentences (it has to fit as one bullet in a future changelog). Missing → 🟡. +- **Lua script naming.** Coexisting scripts use behavior-descriptive suffixes (`Tracked`), never `V2`. Old name must keep working until the next deploy clears it. +- **RunQueue payload shape.** V2 run-queue payload's `projectId` is consumed by `workerQueueResolver` for override matching. If a PR drops it from the payload, 🔴. +- **`safeSend` scope.** Defensive IPC wrappers belong on loop / interval / handler contexts, not one-shot terminal sends. If the PR adds `safeSend` to a single terminal call for consistency, 🟡 with a "remove this" suggestion. +- **Zod version.** Pinned to `3.25.76` monorepo-wide. New package adding zod with a different version or range — 🔴. + +## Skip (do NOT flag) + +- Anything Prettier / ESLint catches. CI runs both. +- TypeScript style preferences (`type` vs `interface`) — already covered by repo standards. +- Test coverage exhortations as a generic suggestion. Only flag missing tests when a specific code path is genuinely untested and the path has prior incidents. +- `agentcrumbs` markers (`// @crumbs`, `// #region @crumbs`) and `agentcrumbs` imports — these are temporary debug instrumentation stripped before merge. +- `// removed comments for removed code`, renamed `_unused` vars, re-exported types as "backwards compatibility shims" — also covered by repo standards. +- Suggestions to "add error handling" without naming a specific scenario that breaks. +- Documentation prose nitpicks in `docs/*` MDX files unless factually wrong. + +## Things V1/legacy that should NOT block a PR + +The `apps/webapp/app/v3/` directory name is misleading — most code there is V2. Only specific files are V1-only legacy: `MarQS` queue, `triggerTaskV1`, `cancelTaskRunV1`, and a handful of others (see `apps/webapp/CLAUDE.md` for the exact list). Don't flag "you should refactor this to use V2" on those — they're frozen. + +## Confidence calibration for this repo + +The most common false-positive pattern: speculating about race conditions in code paths the agent doesn't have runtime visibility into. If the only evidence is "this *could* race", drop it. If you can point to a specific interleaving with file:line for each step, surface it. diff --git a/.claude/scripts/check-review-md.sh b/.claude/scripts/check-review-md.sh new file mode 100755 index 00000000000..65d64d68820 --- /dev/null +++ b/.claude/scripts/check-review-md.sh @@ -0,0 +1,137 @@ +#!/usr/bin/env bash +# Check that paths and packages cited in .claude/REVIEW.md still exist. +# Exits 1 if any cited path/package is missing, 0 otherwise. +# Designed to be runnable locally and from CI. + +set -uo pipefail + +REVIEW="${1:-.claude/REVIEW.md}" +REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)" +cd "$REPO_ROOT" + +if [[ ! -f "$REVIEW" ]]; then + echo "No $REVIEW found — skipping check" + exit 0 +fi + +declare -a ERRORS=() +declare -a WARNINGS=() + +# Extract every backtick-quoted token. We deliberately ignore fenced code +# blocks (```...```) — those are illustrative examples, not citations to +# verify. +mapfile -t REFS < <( + awk ' + /^```/ { inblock = !inblock; next } + !inblock { + while (match($0, /`[^`]+`/)) { + print substr($0, RSTART+1, RLENGTH-2) + $0 = substr($0, RSTART+RLENGTH) + } + } + ' "$REVIEW" | sort -u +) + +is_path_like() { + local s="$1" + # Path-like: contains a slash, OR ends in a recognized file extension + case "$s" in + */*) return 0 ;; + *.ts|*.tsx|*.js|*.jsx|*.mjs|*.cjs|*.json|*.md|*.mdx|*.sql|*.prisma|*.lua|*.yml|*.yaml|*.sh|*.toml) return 0 ;; + *) return 1 ;; + esac +} + +is_package_like() { + local s="$1" + [[ "$s" == @*/?* ]] +} + +# Heuristic skips: things in backticks that aren't paths or packages. +# These are usually code snippets, function names, table names, etc. +# We do NOT check them — too many false positives. +should_skip() { + local s="$1" + # Slash-command form (starts with / and has no further /): e.g. /code-review + if [[ "$s" == /* && "$s" != /*/* ]]; then + return 0 + fi + case "$s" in + *" "*) return 0 ;; # contains space — prose + *"("*|*")"*) return 0 ;; # function call syntax + *"{"*|*"}"*) return 0 ;; + *"="*|*";"*) return 0 ;; + *"<"*|*">"*) return 0 ;; + *) return 1 ;; + esac +} + +# Resolve glob-bearing paths to their longest static prefix dir. +# `packages/*` → `packages` +# `.changeset/*.md` → `.changeset` +# Pure paths return unchanged. +resolve_glob_prefix() { + local s="$1" + case "$s" in + *"*"*|*"?"*) + # Strip from the first segment containing a wildcard onward + printf '%s\n' "${s%%/\**}" | sed 's:/$::' + ;; + *) + printf '%s\n' "$s" + ;; + esac +} + +for ref in "${REFS[@]}"; do + # Strip leading/trailing punctuation that snuck through + ref="${ref#[(\[]}" + ref="${ref%[,.):\]]}" + + [[ -z "$ref" ]] && continue + should_skip "$ref" && continue + + if is_package_like "$ref"; then + # Check that any package.json in the repo declares this name + if ! grep -rqE "\"name\":[[:space:]]*\"${ref}\"" --include=package.json . 2>/dev/null; then + ERRORS+=("package not found in workspace: \`$ref\`") + fi + continue + fi + + if is_path_like "$ref"; then + resolved="$(resolve_glob_prefix "$ref")" + case "$ref" in + */) + if [[ ! -d "${resolved%/}" ]]; then + ERRORS+=("directory missing: \`$ref\`") + fi + ;; + *) + # Accept file OR directory (some refs omit trailing slash) + if [[ ! -e "$resolved" ]]; then + ERRORS+=("path missing: \`$ref\`") + fi + ;; + esac + continue + fi + + # Anything else: not checked. +done + +if [[ ${#ERRORS[@]} -gt 0 ]]; then + echo "REVIEW.md cites paths/packages that no longer exist:" + printf ' - %s\n' "${ERRORS[@]}" + echo + echo "Either restore the referenced paths or update .claude/REVIEW.md." + exit 1 +fi + +if [[ ${#WARNINGS[@]} -gt 0 ]]; then + echo "Warnings:" + printf ' - %s\n' "${WARNINGS[@]}" +fi + +echo "REVIEW.md OK — ${#REFS[@]} backtick refs scanned, all cited paths/packages exist." +exit 0 diff --git a/.github/workflows/check-review-md.yml b/.github/workflows/check-review-md.yml new file mode 100644 index 00000000000..31695f074a1 --- /dev/null +++ b/.github/workflows/check-review-md.yml @@ -0,0 +1,58 @@ +name: 🔎 REVIEW.md drift check + +on: + pull_request: + paths: + - ".claude/REVIEW.md" + - ".claude/scripts/check-review-md.sh" + schedule: + # Mondays 09:00 UTC — catches drift introduced by merges that deleted + # paths referenced by REVIEW.md without updating it. + - cron: "0 9 * * 1" + workflow_dispatch: + +concurrency: + group: check-review-md-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + check: + runs-on: ubuntu-latest + permissions: + contents: read + issues: write + steps: + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + + - name: Run check + id: check + run: | + set +e + bash .claude/scripts/check-review-md.sh | tee check-output.txt + echo "exit_code=$?" >> "$GITHUB_OUTPUT" + + - name: Open / update tracking issue (scheduled runs only) + if: github.event_name == 'schedule' && steps.check.outputs.exit_code != '0' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -euo pipefail + TITLE="REVIEW.md is out of sync with the codebase" + BODY=$'The weekly drift check found stale references in `.claude/REVIEW.md`:\n\n```\n'"$(cat check-output.txt)"$'\n```\n\nFix by updating REVIEW.md to remove or correct the stale citations.' + + EXISTING=$(gh issue list --label "review-md-drift" --state open --json number --jq '.[0].number // empty') + if [[ -n "$EXISTING" ]]; then + gh issue comment "$EXISTING" --body "$BODY" + else + gh label create "review-md-drift" --color "fbca04" --description "REVIEW.md drift detected by scheduled check" 2>/dev/null || true + gh issue create --title "$TITLE" --label "review-md-drift" --body "$BODY" + fi + + - name: Fail PR if check failed + if: github.event_name == 'pull_request' && steps.check.outputs.exit_code != '0' + run: | + echo "REVIEW.md check failed. See log above." + exit 1 From f1b3378eb6119bbe3f5eefce5c708c36d3092a91 Mon Sep 17 00:00:00 2001 From: Eric Allam Date: Tue, 12 May 2026 15:13:06 +0100 Subject: [PATCH 2/4] chore: switch drift check to claude-code-action --- .claude/scripts/check-review-md.sh | 137 -------------------------- .github/workflows/check-review-md.yml | 108 ++++++++++++-------- 2 files changed, 67 insertions(+), 178 deletions(-) delete mode 100755 .claude/scripts/check-review-md.sh diff --git a/.claude/scripts/check-review-md.sh b/.claude/scripts/check-review-md.sh deleted file mode 100755 index 65d64d68820..00000000000 --- a/.claude/scripts/check-review-md.sh +++ /dev/null @@ -1,137 +0,0 @@ -#!/usr/bin/env bash -# Check that paths and packages cited in .claude/REVIEW.md still exist. -# Exits 1 if any cited path/package is missing, 0 otherwise. -# Designed to be runnable locally and from CI. - -set -uo pipefail - -REVIEW="${1:-.claude/REVIEW.md}" -REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)" -cd "$REPO_ROOT" - -if [[ ! -f "$REVIEW" ]]; then - echo "No $REVIEW found — skipping check" - exit 0 -fi - -declare -a ERRORS=() -declare -a WARNINGS=() - -# Extract every backtick-quoted token. We deliberately ignore fenced code -# blocks (```...```) — those are illustrative examples, not citations to -# verify. -mapfile -t REFS < <( - awk ' - /^```/ { inblock = !inblock; next } - !inblock { - while (match($0, /`[^`]+`/)) { - print substr($0, RSTART+1, RLENGTH-2) - $0 = substr($0, RSTART+RLENGTH) - } - } - ' "$REVIEW" | sort -u -) - -is_path_like() { - local s="$1" - # Path-like: contains a slash, OR ends in a recognized file extension - case "$s" in - */*) return 0 ;; - *.ts|*.tsx|*.js|*.jsx|*.mjs|*.cjs|*.json|*.md|*.mdx|*.sql|*.prisma|*.lua|*.yml|*.yaml|*.sh|*.toml) return 0 ;; - *) return 1 ;; - esac -} - -is_package_like() { - local s="$1" - [[ "$s" == @*/?* ]] -} - -# Heuristic skips: things in backticks that aren't paths or packages. -# These are usually code snippets, function names, table names, etc. -# We do NOT check them — too many false positives. -should_skip() { - local s="$1" - # Slash-command form (starts with / and has no further /): e.g. /code-review - if [[ "$s" == /* && "$s" != /*/* ]]; then - return 0 - fi - case "$s" in - *" "*) return 0 ;; # contains space — prose - *"("*|*")"*) return 0 ;; # function call syntax - *"{"*|*"}"*) return 0 ;; - *"="*|*";"*) return 0 ;; - *"<"*|*">"*) return 0 ;; - *) return 1 ;; - esac -} - -# Resolve glob-bearing paths to their longest static prefix dir. -# `packages/*` → `packages` -# `.changeset/*.md` → `.changeset` -# Pure paths return unchanged. -resolve_glob_prefix() { - local s="$1" - case "$s" in - *"*"*|*"?"*) - # Strip from the first segment containing a wildcard onward - printf '%s\n' "${s%%/\**}" | sed 's:/$::' - ;; - *) - printf '%s\n' "$s" - ;; - esac -} - -for ref in "${REFS[@]}"; do - # Strip leading/trailing punctuation that snuck through - ref="${ref#[(\[]}" - ref="${ref%[,.):\]]}" - - [[ -z "$ref" ]] && continue - should_skip "$ref" && continue - - if is_package_like "$ref"; then - # Check that any package.json in the repo declares this name - if ! grep -rqE "\"name\":[[:space:]]*\"${ref}\"" --include=package.json . 2>/dev/null; then - ERRORS+=("package not found in workspace: \`$ref\`") - fi - continue - fi - - if is_path_like "$ref"; then - resolved="$(resolve_glob_prefix "$ref")" - case "$ref" in - */) - if [[ ! -d "${resolved%/}" ]]; then - ERRORS+=("directory missing: \`$ref\`") - fi - ;; - *) - # Accept file OR directory (some refs omit trailing slash) - if [[ ! -e "$resolved" ]]; then - ERRORS+=("path missing: \`$ref\`") - fi - ;; - esac - continue - fi - - # Anything else: not checked. -done - -if [[ ${#ERRORS[@]} -gt 0 ]]; then - echo "REVIEW.md cites paths/packages that no longer exist:" - printf ' - %s\n' "${ERRORS[@]}" - echo - echo "Either restore the referenced paths or update .claude/REVIEW.md." - exit 1 -fi - -if [[ ${#WARNINGS[@]} -gt 0 ]]; then - echo "Warnings:" - printf ' - %s\n' "${WARNINGS[@]}" -fi - -echo "REVIEW.md OK — ${#REFS[@]} backtick refs scanned, all cited paths/packages exist." -exit 0 diff --git a/.github/workflows/check-review-md.yml b/.github/workflows/check-review-md.yml index 31695f074a1..7ea98d9e9b3 100644 --- a/.github/workflows/check-review-md.yml +++ b/.github/workflows/check-review-md.yml @@ -1,58 +1,84 @@ -name: 🔎 REVIEW.md drift check +name: 🔎 REVIEW.md Drift Audit on: pull_request: - paths: - - ".claude/REVIEW.md" - - ".claude/scripts/check-review-md.sh" - schedule: - # Mondays 09:00 UTC — catches drift introduced by merges that deleted - # paths referenced by REVIEW.md without updating it. - - cron: "0 9 * * 1" - workflow_dispatch: + types: [opened, ready_for_review, synchronize] + paths-ignore: + - "docs/**" + - ".changeset/**" + - ".server-changes/**" + - "references/**" concurrency: - group: check-review-md-${{ github.event.pull_request.number || github.ref }} + group: review-md-drift-${{ github.event.pull_request.number }} cancel-in-progress: true jobs: - check: + audit: + if: >- + github.event.pull_request.draft == false && + github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest permissions: contents: read + pull-requests: write issues: write + id-token: write steps: - - name: Checkout + - name: Checkout repository uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: + fetch-depth: 0 persist-credentials: false - - name: Run check - id: check - run: | - set +e - bash .claude/scripts/check-review-md.sh | tee check-output.txt - echo "exit_code=$?" >> "$GITHUB_OUTPUT" - - - name: Open / update tracking issue (scheduled runs only) - if: github.event_name == 'schedule' && steps.check.outputs.exit_code != '0' - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - set -euo pipefail - TITLE="REVIEW.md is out of sync with the codebase" - BODY=$'The weekly drift check found stale references in `.claude/REVIEW.md`:\n\n```\n'"$(cat check-output.txt)"$'\n```\n\nFix by updating REVIEW.md to remove or correct the stale citations.' - - EXISTING=$(gh issue list --label "review-md-drift" --state open --json number --jq '.[0].number // empty') - if [[ -n "$EXISTING" ]]; then - gh issue comment "$EXISTING" --body "$BODY" - else - gh label create "review-md-drift" --color "fbca04" --description "REVIEW.md drift detected by scheduled check" 2>/dev/null || true - gh issue create --title "$TITLE" --label "review-md-drift" --body "$BODY" - fi - - - name: Fail PR if check failed - if: github.event_name == 'pull_request' && steps.check.outputs.exit_code != '0' - run: | - echo "REVIEW.md check failed. See log above." - exit 1 + - name: Run Claude Code + id: claude + uses: anthropics/claude-code-action@fefa07e9c665b7320f08c3b525980457f22f58aa # v1.0.111 + with: + anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} + use_sticky_comment: true + allowed_bots: "devin-ai-integration[bot]" + + claude_args: | + --max-turns 15 + --allowedTools "Read,Glob,Grep,Bash(git diff:*)" + + prompt: | + You are auditing this PR for drift against `.claude/REVIEW.md`. + + ## Context + + `.claude/REVIEW.md` is the repo's source of truth for what AI / agent code reviewers should treat as critical findings (rolling-deploy safety, hot-table indexes, recovery-path queries, testcontainers usage, Lua versioning, etc.). It is consumed by review agents to calibrate severity. If REVIEW.md goes stale, every future agent review degrades. + + ## Your task + + 1. Read `.claude/REVIEW.md` in full. + 2. Run `git diff origin/main...HEAD --name-only` to see which files changed in this PR. + 3. Sample the diff itself for any of these four signals: + - **Stale references** — does any rule cite a file, directory, function, table, Prisma model, or package name that has been removed or renamed in this PR or already gone from `main`? + - **Contradictions** — does code in this PR violate a current REVIEW.md rule? (Only flag if one side is clearly wrong — do not re-review the PR.) + - **Missing rules** — does this PR introduce a new pattern future reviewers should know about? Examples: a new hot table, a new Lua-script versioning convention, a new safety wrapper, a new "must always check" invariant. + - **Obsolete rules** — has the repo moved past a constraint REVIEW.md still asserts (e.g. a deprecated path is gone, a pattern is now linted, V1 code is deleted)? + + ## Response format + + If nothing needs changing: + + ✅ REVIEW.md looks current for this PR. + + Otherwise: + + 📝 **REVIEW.md updates suggested:** + + - **[stale]** `` — + - **[contradiction]** `` — + - **[missing]** under `##
` — + - **[obsolete]** `` — + + ## Rules + + - Keep it tight. Maximum 3 suggestions per audit. Pick the highest-signal ones. + - Only flag things that would actually mislead a future reviewer. Style nits and wording preferences do not count. + - Do NOT review the PR itself. Do NOT propose rules outside REVIEW.md's existing sections. + - Do NOT propose adding rules for one-off PR specifics that don't generalize to future PRs. + - If REVIEW.md does not exist in the repo, respond with `(skip)` and stop. From 0cbed6c63be24fc3d0f6eae892cb381cd4f39439 Mon Sep 17 00:00:00 2001 From: Eric Allam Date: Tue, 12 May 2026 15:25:43 +0100 Subject: [PATCH 3/4] chore: drop /code-review reference from REVIEW.md preamble --- .claude/REVIEW.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/REVIEW.md b/.claude/REVIEW.md index 06c34394128..67f7a9f15cb 100644 --- a/.claude/REVIEW.md +++ b/.claude/REVIEW.md @@ -1,6 +1,6 @@ # REVIEW.md — Trigger.dev OSS -Injected by `/code-review` as the highest-priority block in the reviewer's prompt. Repo-specific signal only; generic review rules live in the skill. +Repo-specific signal for anyone (human or agent) reviewing a PR in this codebase. Calibrates what counts as critical, what to always check, and what to skip. ## What makes a 🔴 Important finding here From 453aab59d199ce8ca9334807a539cea7ece202dc Mon Sep 17 00:00:00 2001 From: Eric Allam Date: Tue, 12 May 2026 15:34:02 +0100 Subject: [PATCH 4/4] chore: drop issues:write permission from drift audit workflow --- .github/workflows/check-review-md.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/check-review-md.yml b/.github/workflows/check-review-md.yml index 7ea98d9e9b3..ecf44a47b27 100644 --- a/.github/workflows/check-review-md.yml +++ b/.github/workflows/check-review-md.yml @@ -22,7 +22,6 @@ jobs: permissions: contents: read pull-requests: write - issues: write id-token: write steps: - name: Checkout repository