chore: switch to oxfmt, oxlint - add ci checks#3977
Open
carderne wants to merge 10 commits into
Open
Conversation
|
Contributor
|
Important Review skippedToo many files! This PR contains 283 files, which is 133 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (283)
You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe pull request replaces ESLint and Prettier with oxlint and oxfmt as the monorepo's formatting and linting toolchain. Two new root-level configuration files are added: 🚥 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 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 |
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/webapp/package.json (1)
280-281: ⚡ Quick winRemove unused Prettier dependencies.
prettierandprettier-plugin-tailwindcssare still indevDependenciesbut are no longer used after switching tooxfmt. Consider removing them to keep dependencies clean.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 76cce126-8ee3-44af-8b51-bd7dd1f767db
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
.eslintignore.github/workflows/code-quality.yml.github/workflows/pr_checks.yml.oxfmtrc.json.oxlintrc.json.prettierignoreapps/webapp/.eslintrcapps/webapp/.prettierignoreapps/webapp/package.jsonapps/webapp/prettier.config.jspackage.jsonprettier.config.jsturbo.json
💤 Files with no reviewable changes (6)
- .eslintignore
- .prettierignore
- apps/webapp/prettier.config.js
- apps/webapp/.eslintrc
- prettier.config.js
- apps/webapp/.prettierignore
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (39)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: internal / 🧪 Unit Tests: Internal (12, 12)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: internal / 🧪 Unit Tests: Internal (6, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (3, 12)
- GitHub Check: code-quality / code-quality
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: internal / 🧪 Unit Tests: Internal (11, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (8, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (2, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (5, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (7, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: internal / 🧪 Unit Tests: Internal (9, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (10, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (1, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: typecheck / typecheck
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: internal / 🧪 Unit Tests: Internal (4, 12)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build and publish previews
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,jsx,css,json,md}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier for code formatting and run
pnpm run formatbefore committing
Files:
package.jsonturbo.jsonapps/webapp/package.json
🧠 Learnings (6)
📚 Learning: 2026-04-13T21:43:58.707Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/app/services/taskIdentifierRegistry.server.ts:49-58
Timestamp: 2026-04-13T21:43:58.707Z
Learning: In `apps/webapp/app/services/taskIdentifierRegistry.server.ts` (and closely related task registry/sync code in this module), an empty deployment (zero tasks) is not a realistic/expected scenario. Do not flag or require special handling or fallback logic for missing empty-task cases in this registry/sync flow.
Applied to files:
.oxlintrc.json.oxfmtrc.json
📚 Learning: 2026-04-23T13:26:27.529Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3430
File: apps/webapp/app/presenters/v3/RunStreamPresenter.server.ts:0-0
Timestamp: 2026-04-23T13:26:27.529Z
Learning: In this codebase’s SSE implementation (notably `apps/webapp/app/utils/sse.ts` and its SSE callers such as `apps/webapp/app/presenters/v3/RunStreamPresenter.server.ts`), do not treat `.abort()` vs `.abort("<reason string>")` differences as a heap retention/memory leak issue on Node 20. Heap deltas are identical once `AbortSignal.any` is not involved; the investigated SSE memory leak root cause was `AbortSignal.any` (via FinalizationRegistry behavior on abort). When reviewing, focus on whether `AbortSignal.any` (or similar signal-combining/finalization patterns) is used in the SSE abort path rather than the presence/contents of the abort reason string. Named abort-reason sentinel constants (e.g., `ABORT_REASON_*`) are for readability/style only, not correctness.
Applied to files:
.oxlintrc.json.oxfmtrc.json
📚 Learning: 2026-04-30T21:28:35.705Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3473
File: internal-packages/database/prisma/schema.prisma:59-60
Timestamp: 2026-04-30T21:28:35.705Z
Learning: In this repo, session-duration lower-bound validation is intentionally enforced only at the app layer (via `isAllowedSessionDuration` and `SESSION_DURATION_OPTIONS`), where the minimum allowed value is 300 seconds (5 minutes). `User.sessionDuration` is `Int` (non-nullable) and `Organization.maxSessionDuration` is `Int?` (nullable). There are currently no DB-level `CHECK` constraints for these fields anywhere in the migrations, so future reviews should not flag the absence of DB `CHECK` constraints on these session-duration fields—focus validation on the app-layer logic instead.
Applied to files:
.oxlintrc.json.oxfmtrc.json
📚 Learning: 2026-05-12T06:43:09.116Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3558
File: internal-packages/run-engine/src/run-queue/index.ts:420-424
Timestamp: 2026-05-12T06:43:09.116Z
Learning: In internal-packages/run-engine/src/run-queue/index.ts, the RunQueue “read-path” counter/concurrency methods (e.g., lengthOfQueue/lengthOfQueues/currentConcurrencyOfQueues) intentionally “fail open” on transient Redis pipeline result errors. If you see pipeline result errors (e.g., baseErr/ctrErr) being coerced to 0 (treating Redis command errors the same as missing keys) rather than surfaced/thrown, do not flag it as a bug and do not suggest propagating those errors in future reviews.
Applied to files:
.oxlintrc.json.oxfmtrc.json
📚 Learning: 2026-05-19T21:04:49.990Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3671
File: packages/trigger-sdk/src/v3/ai.ts:724-769
Timestamp: 2026-05-19T21:04:49.990Z
Learning: In packages/trigger-sdk/src/v3/ai.ts, when reviewing recovery/replay of turns, do NOT flag “lost metadata on recovered turns” if the recovered turns are reconstructed via the recovery boot queue that maps metadata by message.id (using replaySessionInTail’s {message, metadata, seqNum} per session.in record). If a message has no matching session.in record (e.g., hook-synthesized messages), the code intentionally falls back to the current boot payload’s metadata—this fallback is expected behavior.
Applied to files:
.oxlintrc.json.oxfmtrc.json
📚 Learning: 2026-04-27T16:46:03.861Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3456
File: apps/webapp/package.json:152-152
Timestamp: 2026-04-27T16:46:03.861Z
Learning: In `apps/webapp/package.json`, treat the `effect` npm package as an intentional runtime dependency (not unused/misplaced) for the Schedule + Fiber-based metadata update logic. This should apply when reviewing `apps/webapp` code paths used by `apps/webapp/app/utils/updateMetadata.server.ts` (and closely related modules) that use Effect APIs such as `Duration.divide`, `STM.cond`, namespace exports for `Effect`/`Schedule`/`Duration`/`Fiber`, and the `Fiber.RuntimeFiber` type.
Applied to files:
apps/webapp/package.json
🪛 Biome (2.5.0)
.oxfmtrc.json
[error] 24-24: Expected an array, an object, or a literal but instead found ']'.
(parse)
🔇 Additional comments (8)
.oxlintrc.json (1)
1-25: LGTM!package.json (1)
21-23: LGTM!apps/webapp/package.json (1)
13-14: LGTM!turbo.json (1)
1-106: LGTM!.github/workflows/code-quality.yml (2)
34-63: LGTM!
68-73: Oxlint config auto-discovery is working as expected.Oxlint automatically discovers
.oxlintrc.jsonby walking up the directory tree from each file's location. The CI invocation (pnpm exec oxlint $FILES) will correctly resolve to the root config without an explicit flag. The explicit-c ../../.oxlintrc.jsoninapps/webapp/package.jsonis redundant but harmless.> Likely an incorrect or invalid review comment..github/workflows/pr_checks.yml (2)
105-108: LGTM!
159-170: LGTM!
ericallam
approved these changes
Jun 17, 2026
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.
Currently we have prettier and eslint setup, but no global formatting/linting, and no CI gates on PRs.
Changes:
Disabled lints: