feat(webapp): RUN_OPS_SHARDS config, topology and N-way store wiring - #4764
feat(webapp): RUN_OPS_SHARDS config, topology and N-way store wiring#4764d-cs wants to merge 13 commits into
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ith an injected shard resolver Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Includes the cross-field boot refinement requiring RUN_OPS_DATABASE_URL when the shard list is non-empty, since gen-1 v1 ids resolve to the new store permanently. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…actory Dedupes buildRunOpsWriterClient/buildRunOpsReplicaClient into a single buildRunOpsClient parameterized by role and the resolved pool knobs. The control-plane builders (buildWriterClient/buildReplicaClient) are a separate path and stay untouched. Every resolved value matches the former builders, so split-on deployments are byte-identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…get per pool Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
selectRunOpsTopology gains a shard loop and returns a keyed shard map. An aliasOf:"new" descriptor reuses the new store's clients by reference and opens no pool. Each real shard gets its own resilience budget and the new-role pool knobs merged with its per-shard overrides. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…the shard table at boot buildRunStore now produces one dedicated store per shard descriptor and the N-way router via RoutingRunStore.fromShards, keeping the two-store compat router when no shards are configured. The topology singleton logs the resolved shard table (key, address fingerprint, role) only when RUN_OPS_SHARDS is non-empty, so the unset case adds no output. The fingerprint is an address, never an identity claim. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… descriptor keys computeMintShard now intersects the active shard set with routableKeys (the RUN_OPS_SHARDS descriptor keys), so a stored key with no descriptor is never minted into and falls back to gen-1. The empty-set check runs first, so an unconfigured deployment is unchanged. Inert until the gen-2 write path wires in resolveMintShard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…at/lint Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
WalkthroughAdded validated Merge Risk: 🟠 High · up to The PR adds configurable N-way run-ops routing and shard clients, but a configured deployment with more than two shards can miss waitpoints and prematurely unblock runs, while client connection failures may be mishandled during startup. These correctness and availability risks should be fixed or explicitly accepted before merge; the unset-default path remains unchanged. 🚥 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}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
apps/webapp/test/runOpsDbTopology.test.ts (1)
164-195: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the dedicated-shard misconfiguration throw.
selectRunOpsTopologylines 366-370 throw when a non-aliased descriptor has nourl, or when the shard builders are absent. That guard is what makes theshard.url!non-null assertion atapps/webapp/app/db.server.tsline 521 sound. No test in this file covers it.💚 Proposed test
+ it("a non-aliased shard without a url throws", () => { + expect(() => + selectRunOpsTopology( + { ...baseSplit, shards: [{ key: "a" }] }, + { ...baseBuilders(), buildShardWriter: vi.fn(), buildShardReplica: vi.fn() } + ) + ).toThrow(/shard "a" needs a url/); + }); + + it("a non-aliased shard without shard builders throws", () => { + expect(() => + selectRunOpsTopology( + { ...baseSplit, shards: [{ key: "a", url: "postgres://a" }] }, + baseBuilders() + ) + ).toThrow(/shard "a" needs a url and shard builders/); + });
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: aae0d756-d24d-44b7-8154-04c13f7976f5
📒 Files selected for processing (19)
apps/webapp/app/db.server.tsapps/webapp/app/env.server.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.tsapps/webapp/app/v3/runOpsShards.server.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/transactionResilience.server.tsapps/webapp/test/runOpsDbTopology.test.tsapps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/test/runOpsShards.test.tsapps/webapp/test/runStoreShardWiring.test.tsapps/webapp/test/transactionResilience.test.tsinternal-packages/run-store/src/runOpsStore.fromShards.test.tsinternal-packages/run-store/src/runOpsStore.tspackages/core/src/v3/isomorphic/friendlyId.test.tspackages/core/src/v3/isomorphic/friendlyId.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (48)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: typecheck / typecheck
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: runops-guard / runops-guard
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: sdk-compat / Node.js 24.18 (warp-ubuntu-latest-x64-4x)
- GitHub Check: sdk-compat / Node.js 20.20 (warp-ubuntu-latest-x64-4x)
- GitHub Check: sdk-compat / Node.js 22.23 (warp-ubuntu-latest-x64-4x)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - npm)
- GitHub Check: sdk-compat / Node.js 26.4 (warp-ubuntu-latest-x64-4x)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-ubuntu-latest-x64-4x - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-ubuntu-latest-x64-4x - npm)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: obsmap / 🧪 Unit Tests: Observability Map
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
- GitHub Check: fk-cascade-guard / fk-cascade-guard
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: code-quality / code-quality
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build and publish previews
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Prefer static imports over dynamic imports. Only use dynamicimport()when:
- Circular dependencies cannot be resolved otherwise
- Code splitting is genuinely needed for performance
- The module must be loaded conditionally at runtime
Files:
apps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runStoreShardWiring.test.tspackages/core/src/v3/isomorphic/friendlyId.tspackages/core/src/v3/isomorphic/friendlyId.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/test/transactionResilience.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/test/runOpsDbTopology.test.tsapps/webapp/app/env.server.tsinternal-packages/run-store/src/runOpsStore.tsinternal-packages/run-store/src/runOpsStore.fromShards.test.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/transactionResilience.server.tsapps/webapp/app/db.server.tsapps/webapp/test/runOpsShards.test.tsapps/webapp/app/v3/runOpsShards.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runStoreShardWiring.test.tspackages/core/src/v3/isomorphic/friendlyId.tspackages/core/src/v3/isomorphic/friendlyId.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/test/transactionResilience.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/test/runOpsDbTopology.test.tsapps/webapp/app/env.server.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/transactionResilience.server.tsapps/webapp/app/db.server.tsapps/webapp/test/runOpsShards.test.tsapps/webapp/app/v3/runOpsShards.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runStoreShardWiring.test.tspackages/core/src/v3/isomorphic/friendlyId.tspackages/core/src/v3/isomorphic/friendlyId.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/test/transactionResilience.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/test/runOpsDbTopology.test.tsapps/webapp/app/env.server.tsinternal-packages/run-store/src/runOpsStore.tsinternal-packages/run-store/src/runOpsStore.fromShards.test.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/transactionResilience.server.tsapps/webapp/app/db.server.tsapps/webapp/test/runOpsShards.test.tsapps/webapp/app/v3/runOpsShards.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use vitest for all tests in the Trigger.dev repository
**/*.{test,spec}.{ts,tsx}: We use vitest exclusively. Never mock anything - use testcontainers instead.
Test files go next to source files (e.g.,MyService.ts->MyService.test.ts).
Files:
apps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runStoreShardWiring.test.tspackages/core/src/v3/isomorphic/friendlyId.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.tsapps/webapp/test/transactionResilience.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/test/runOpsDbTopology.test.tsinternal-packages/run-store/src/runOpsStore.fromShards.test.tsapps/webapp/test/runOpsShards.test.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runStoreShardWiring.test.tspackages/core/src/v3/isomorphic/friendlyId.tspackages/core/src/v3/isomorphic/friendlyId.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/test/transactionResilience.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/test/runOpsDbTopology.test.tsapps/webapp/app/env.server.tsinternal-packages/run-store/src/runOpsStore.tsinternal-packages/run-store/src/runOpsStore.fromShards.test.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/transactionResilience.server.tsapps/webapp/app/db.server.tsapps/webapp/test/runOpsShards.test.tsapps/webapp/app/v3/runOpsShards.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepath
Files:
apps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runStoreShardWiring.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/test/transactionResilience.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/test/runOpsDbTopology.test.tsapps/webapp/app/env.server.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/transactionResilience.server.tsapps/webapp/app/db.server.tsapps/webapp/test/runOpsShards.test.tsapps/webapp/app/v3/runOpsShards.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.ts
apps/webapp/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Do not import
env.server.tsdirectly or indirectly into test files; instead pass environment-dependent values through options/parameters to make code testable
Files:
apps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runStoreShardWiring.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.tsapps/webapp/test/transactionResilience.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/test/runOpsDbTopology.test.tsapps/webapp/test/runOpsShards.test.ts
apps/webapp/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Test files must not import
app/env.server.ts; pass configuration as options instead.
Files:
apps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runStoreShardWiring.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.tsapps/webapp/test/transactionResilience.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/test/runOpsDbTopology.test.tsapps/webapp/test/runOpsShards.test.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Add crumbs as you write code — not just when debugging. Mark lines with
//@Crumbsor wrap blocks in `// `#region` `@crumbs. They stay on the
branch throughout development and are stripped byagentcrumbs strip
before merge.
Do not invent new namespaces — pick from this table or ask first.
Do not flag//@Crumbsmarkers, `#region `@crumbsblocks, or agentcrumbs
imports in reviews. These are temporary debug instrumentation stripped
before merge. Data logged in crumbs (IDs, names, values) never reaches
production.
Files:
apps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runStoreShardWiring.test.tspackages/core/src/v3/isomorphic/friendlyId.tspackages/core/src/v3/isomorphic/friendlyId.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/test/transactionResilience.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/test/runOpsDbTopology.test.tsapps/webapp/app/env.server.tsinternal-packages/run-store/src/runOpsStore.tsinternal-packages/run-store/src/runOpsStore.fromShards.test.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/transactionResilience.server.tsapps/webapp/app/db.server.tsapps/webapp/test/runOpsShards.test.tsapps/webapp/app/v3/runOpsShards.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.ts
packages/core/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/core/CLAUDE.md)
Never import the root package (
@trigger.dev/core). Always use subpath imports such as@trigger.dev/core/v3,@trigger.dev/core/v3/utils,@trigger.dev/core/logger, or@trigger.dev/core/schemas
Files:
packages/core/src/v3/isomorphic/friendlyId.tspackages/core/src/v3/isomorphic/friendlyId.test.ts
packages/core/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Import subpaths only (never root).
Files:
packages/core/src/v3/isomorphic/friendlyId.tspackages/core/src/v3/isomorphic/friendlyId.test.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.{ts,tsx}: For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
UseuseCallbackanduseMemoonly for context provider values, expensive derived data used as a dependency, or stable references required by dependency arrays; do not wrap ordinary event handlers or trivial computations.
Use named constants for sentinel or placeholder values instead of scattering raw string literals across comparisons.
Files:
apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/app/env.server.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/transactionResilience.server.tsapps/webapp/app/db.server.tsapps/webapp/app/v3/runOpsShards.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.ts: Never userequest.signalto detect client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.ts, which is wired to Express response close events.
Access environment variables through theenvexport fromapp/env.server.ts; never useprocess.envdirectly.
Always use PrismafindFirstinstead offindUnique.
Always use the$transactionhelper from~/db.server, never callprisma.$transactionor$replica.$transactiondirectly. Pass isolation levels as strings, useSerializablefor correctness-critical read-then-write invariants, and guard possibly undefined helper results when a definite value is required.
Files:
apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/app/env.server.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/transactionResilience.server.tsapps/webapp/app/db.server.tsapps/webapp/app/v3/runOpsShards.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.ts
apps/webapp/app/v3/**/*.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
New code must target Run Engine V2 through the singleton in
app/v3/runEngine.server.ts; do not reintroduce V1 execution paths. V1 branches may only reject or finalize gracefully with a clean 4xx.
Files:
apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/transactionResilience.server.tsapps/webapp/app/v3/runOpsShards.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.ts
🧠 Learnings (6)
📚 Learning: 2026-06-16T09:19:47.637Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3960
File: apps/webapp/test/prismaInfrastructureErrorCapture.test.ts:0-0
Timestamp: 2026-06-16T09:19:47.637Z
Learning: In this repo’s Vitest setup, `vitest.config.ts` uses `globals: true`, so identifiers like `vi`, `describe`, `it`, and `expect` are available as globals in Vitest test files. During code review, do not flag missing `vi`/`describe`/`it`/`expect` imports as a runtime error or correctness issue when they’re used in `*.test.ts/tsx` or `*.spec.ts/tsx` files. Explicit imports are still preferred for consistency, but they’re not required for runtime behavior.
Applied to files:
apps/webapp/test/runStoreShardWiring.test.tspackages/core/src/v3/isomorphic/friendlyId.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/test/runOpsDbTopology.test.ts
📚 Learning: 2026-05-28T20:02:10.647Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 3772
File: apps/webapp/test/findOrCreateBackgroundWorker.test.ts:1-1
Timestamp: 2026-05-28T20:02:10.647Z
Learning: In the triggerdotdev/trigger.dev monorepo, for the `apps/webapp` package use the established convention of storing Vitest tests (unit, integration, and e2e) under `apps/webapp/test/` rather than colocating them next to source files. Do not flag files located in `apps/webapp/test/` as violating any rule that says to colocate tests with source.
Applied to files:
apps/webapp/test/runStoreShardWiring.test.ts
📚 Learning: 2026-08-24T12:38:01.585Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 4755
File: apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts:1-9
Timestamp: 2026-08-24T12:38:01.585Z
Learning: Keep mint-shard assignment logic in the pure `mintShardAssignment.ts` module so it remains independent of `env.server.ts`, and test that logic in `mintShardAssignment.test.ts` without importing environment-bound modules. Keep runtime and environment integration in the `runOpsMintShard.server.ts` wrapper; do not reintroduce module-level boot warnings tied to the removed `RUN_OPS_MINT_SHARDS` setting.
Applied to files:
apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.ts
📚 Learning: 2026-05-18T14:40:02.173Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3658
File: packages/core/src/v3/realtimeStreams/manager.test.ts:1-147
Timestamp: 2026-05-18T14:40:02.173Z
Learning: In the triggerdotdev/trigger.dev repo, the policy “Never mock anything — use testcontainers instead” should only be enforced for integration tests that interact with real external services (e.g., Redis, Postgres) via actual infrastructure. For unit tests that exercise pure in-memory logic (e.g., cache semantics) it is OK to stub collaborators such as `ApiClient` using Vitest (`vi.fn()`) to assert call counts or control behavior. Do not flag `vi.fn()`-based `ApiClient` stubs in unit tests as violations of the testcontainers policy.
Applied to files:
apps/webapp/test/runOpsDbTopology.test.tsinternal-packages/run-store/src/runOpsStore.fromShards.test.ts
📚 Learning: 2026-05-20T17:21:18.543Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3678
File: apps/webapp/app/entry.server.tsx:0-0
Timestamp: 2026-05-20T17:21:18.543Z
Learning: In env.server.ts (Zod env schema), any environment variable you plan to access via the typed `env` export (e.g., `env.SENTRY_DSN`) must be explicitly declared in the schema. For `SENTRY_DSN`, include `SENTRY_DSN: z.string().optional()`; otherwise switching from `process.env.SENTRY_DSN` to `env.SENTRY_DSN` will fail TypeScript typechecking.
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: 2026-05-12T21:04:05.815Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3542
File: apps/webapp/app/components/sessions/v1/SessionStatus.tsx:1-3
Timestamp: 2026-05-12T21:04:05.815Z
Learning: In this Remix + TypeScript codebase, do not flag a server/client boundary violation when a file imports only types from a module matching `*.server`.
Specifically, it’s safe to import types using `import type { Foo } from "*.server"` or `import { type Foo } from "*.server"` because TypeScript erases type-only imports at compile time and they emit no JavaScript, so they won’t cross the Remix server/client bundle boundary.
Only raise the boundary concern for value imports (e.g., `import { Foo }` without `type`, or `import Foo`), since those produce JavaScript output.
Applied to files:
apps/webapp/app/v3/runOpsPoolKnobs.server.ts
🔇 Additional comments (9)
apps/webapp/app/v3/runOpsPoolKnobs.server.ts (1)
1-81: LGTM!apps/webapp/app/v3/transactionResilience.server.ts (1)
20-24: LGTM!Also applies to: 70-107
apps/webapp/test/transactionResilience.test.ts (1)
1-17: LGTM!apps/webapp/app/db.server.ts (2)
34-44: LGTM!Also applies to: 280-292, 302-302, 312-315, 334-338, 411-421, 430-435, 444-451, 465-472, 510-553, 579-589, 1039-1096, 1114-1133, 1163-1188
357-378: 🩺 Stability & AvailabilityNo change needed.
parseRunOpsShardsrejects duplicate keys, andisValidShardCharrejectsnewandlegacybecause keys must be a single[a-z0-9]character.> Likely an incorrect or invalid review comment.apps/webapp/test/runOpsDbTopology.test.ts (1)
146-162: LGTM!apps/webapp/test/runOpsShardBootTable.test.ts (1)
4-34: LGTM!internal-packages/run-store/src/runOpsStore.ts (1)
43-69: LGTM!Also applies to: 88-105, 135-141, 166-176
apps/webapp/app/v3/runStore.server.ts (1)
2-7: LGTM!Also applies to: 17-17, 40-49, 98-128, 133-136, 164-165, 183-192
…ri-13429 # Conflicts: # packages/core/src/v3/isomorphic/friendlyId.test.ts
@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: |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/v3/isomorphic/friendlyId.ts (1)
43-47: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd required crumb markers to the new code and tests. The changed paths do not contain the required crumb instrumentation.
packages/core/src/v3/isomorphic/friendlyId.ts#L43-L47: add crumb instrumentation for shard-character validation.packages/core/src/v3/isomorphic/friendlyId.ts#L246-L343: add crumb instrumentation for waitpoint ID generation, parsing, and derivation.packages/core/src/v3/isomorphic/friendlyId.test.ts#L422-L590: add crumb instrumentation for the new test paths.Source: Coding guidelines
packages/core/src/v3/isomorphic/friendlyId.test.ts (1)
472-480: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDo not mock the system clock.
vi.useFakeTimers()replaces global time. Capture timestamps before and aftergenerateWaitpointId()and assert thatparsed.timestampis within those bounds.As per coding guidelines, “Never mock anything - use testcontainers instead.”
Proposed test change
- vi.useFakeTimers(); - try { - vi.setSystemTime(new Date("2026-08-21T12:00:00.000Z")); - const parsed = parseWaitpointId(generateWaitpointId("DATETIME")); - if (parsed.format !== "b32hexW") throw new Error("unreachable"); - expect(parsed.timestamp.toISOString()).toBe("2026-08-21T12:00:00.000Z"); - } finally { - vi.useRealTimers(); - } + const startedAt = Date.now(); + const parsed = parseWaitpointId(generateWaitpointId("DATETIME")); + const completedAt = Date.now(); + if (parsed.format !== "b32hexW") throw new Error("unreachable"); + expect(parsed.timestamp.getTime()).toBeGreaterThanOrEqual(startedAt); + expect(parsed.timestamp.getTime()).toBeLessThanOrEqual(completedAt);Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b3a91159-ae69-4aca-b3e6-566a40418e1f
📒 Files selected for processing (2)
packages/core/src/v3/isomorphic/friendlyId.test.tspackages/core/src/v3/isomorphic/friendlyId.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Build and publish previews
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Prefer static imports over dynamic imports. Only use dynamicimport()when:
- Circular dependencies cannot be resolved otherwise
- Code splitting is genuinely needed for performance
- The module must be loaded conditionally at runtime
Files:
packages/core/src/v3/isomorphic/friendlyId.tspackages/core/src/v3/isomorphic/friendlyId.test.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
packages/core/src/v3/isomorphic/friendlyId.tspackages/core/src/v3/isomorphic/friendlyId.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
packages/core/src/v3/isomorphic/friendlyId.tspackages/core/src/v3/isomorphic/friendlyId.test.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
packages/core/src/v3/isomorphic/friendlyId.tspackages/core/src/v3/isomorphic/friendlyId.test.ts
packages/core/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/core/CLAUDE.md)
Never import the root package (
@trigger.dev/core). Always use subpath imports such as@trigger.dev/core/v3,@trigger.dev/core/v3/utils,@trigger.dev/core/logger, or@trigger.dev/core/schemas
Files:
packages/core/src/v3/isomorphic/friendlyId.tspackages/core/src/v3/isomorphic/friendlyId.test.ts
packages/core/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Import subpaths only (never root).
Files:
packages/core/src/v3/isomorphic/friendlyId.tspackages/core/src/v3/isomorphic/friendlyId.test.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Add crumbs as you write code — not just when debugging. Mark lines with
//@Crumbsor wrap blocks in `// `#region` `@crumbs. They stay on the
branch throughout development and are stripped byagentcrumbs strip
before merge.
Do not invent new namespaces — pick from this table or ask first.
Do not flag//@Crumbsmarkers, `#region `@crumbsblocks, or agentcrumbs
imports in reviews. These are temporary debug instrumentation stripped
before merge. Data logged in crumbs (IDs, names, values) never reaches
production.
Files:
packages/core/src/v3/isomorphic/friendlyId.tspackages/core/src/v3/isomorphic/friendlyId.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use vitest for all tests in the Trigger.dev repository
**/*.{test,spec}.{ts,tsx}: We use vitest exclusively. Never mock anything - use testcontainers instead.
Test files go next to source files (e.g.,MyService.ts->MyService.test.ts).
Files:
packages/core/src/v3/isomorphic/friendlyId.test.ts
🧠 Learnings (1)
📚 Learning: 2026-06-16T09:19:47.637Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3960
File: apps/webapp/test/prismaInfrastructureErrorCapture.test.ts:0-0
Timestamp: 2026-06-16T09:19:47.637Z
Learning: In this repo’s Vitest setup, `vitest.config.ts` uses `globals: true`, so identifiers like `vi`, `describe`, `it`, and `expect` are available as globals in Vitest test files. During code review, do not flag missing `vi`/`describe`/`it`/`expect` imports as a runtime error or correctness issue when they’re used in `*.test.ts/tsx` or `*.spec.ts/tsx` files. Explicit imports are still preferred for consistency, but they’re not required for runtime behavior.
Applied to files:
packages/core/src/v3/isomorphic/friendlyId.test.ts
- Make probeOrder a true reverse of precedence so the merge and probe paths agree on a duplicate id, matching the RoutingRunStore invariant. - Split resolveRunOpsPoolKnobs into a pure applyPoolKnobOverrides (tested with literal defaults, no env import) plus an env-reading defaults function. - Move the pure boot-table helpers to runOpsShardTable.ts so their test does not construct the db.server Prisma topology. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ectRunOpsTopology Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@coderabbitai Addressed all review feedback in commits 20f0ee3 and 908fcb5: Fixed (4):
Declined (2), with reasoning in the thread replies:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/webapp/app/db.server.ts (3)
1085-1088: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse the validated
envexport for Prisma flags.
buildRunOpsClientreadsprocess.env.VERBOSE_PRISMA_LOGS,process.env.VERY_SLOW_QUERY_THRESHOLD_MS, andprocess.env.PRISMA_LOG_TO_STDOUT. Inapps/webapp/app/**/*.ts, read environment variables throughenvinstead. Add these fields toenv.server.tsif needed, then useenv.*here.As per coding guidelines: “Access environment variables through the
envexport fromapp/env.server.ts; never useprocess.envdirectly.”Also applies to: 1111-1112, 1128-1131
Source: Coding guidelines
1127-1134: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle
client.$connect()before logging success.
buildRunOpsClientlogs${connectedLabel}before Prisma 6.14.0 finishesengine.start(). A rejected connection is unhandled in production. Await or handle the promise before emitting the success log.
645-655: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHonor
connectTimeoutin driver-adapter pools.
pg@8.15.6usesconnectionTimeoutMillisfor both pool acquisition and new connections.@prisma/adapter-pg@6.14.0passes the existing pool through unchanged. Therefore, adapter mode ignoresconnectTimeoutand appliespoolTimeoutto both operations. Define the timeout precedence, implement it, and add a test with different timeout values.Source: MCP tools
apps/webapp/test/runOpsDbTopology.test.ts (1)
151-156: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftReplace mocked shard builders with testcontainers-backed builders.
baseBuildersusesvi.fn().mockReturnValue(...). These mocks can let topology tests pass while real Prisma client construction, connection settings, or per-shard wiring is incompatible. Use testcontainers-backed builders and assert against real clients.As per coding guidelines: “We use vitest exclusively. Never mock anything - use testcontainers instead.”
Source: Coding guidelines
♻️ Duplicate comments (1)
apps/webapp/test/runOpsPoolKnobs.test.ts (1)
2-5: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winKeep this test outside the
env.server.tsmodule graph.Line 2 statically imports
runOpsPoolKnobs.server.ts. That module readsenvinpoolKnobDefaults, so this test still indirectly importsenv.server.ts.Move
applyPoolKnobOverridesandResolvedPoolKnobsinto a dependency-free module. Move this test beside that module. Keep environment resolution in the server-only module.As per coding guidelines: “Do not import
env.server.tsdirectly or indirectly into test files” and “Test files go next to source files”.#!/bin/bash set -euo pipefail # Inspect the test import and the imported module's environment dependency. rg -n -C 2 'runOpsPoolKnobs\.server|applyPoolKnobOverrides' \ apps/webapp/test/runOpsPoolKnobs.test.ts rg -n -C 2 'env\.|env\.server' \ apps/webapp/app/v3/runOpsPoolKnobs.server.tsSource: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 38158b7e-a94d-4d27-b34e-b7acc1cdc9af
📒 Files selected for processing (7)
apps/webapp/app/db.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.tsapps/webapp/app/v3/runOpsShardTable.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/test/runOpsDbTopology.test.tsapps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runOpsShardBootTable.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (43)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: sdk-compat / Node.js 22.23 (warp-ubuntu-latest-x64-4x)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: sdk-compat / Node.js 24.18 (warp-ubuntu-latest-x64-4x)
- GitHub Check: sdk-compat / Node.js 26.4 (warp-ubuntu-latest-x64-4x)
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-ubuntu-latest-x64-4x - npm)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - pnpm)
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: obsmap / 🧪 Unit Tests: Observability Map
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-ubuntu-latest-x64-4x - pnpm)
- GitHub Check: runops-guard / runops-guard
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Prefer static imports over dynamic imports. Only use dynamicimport()when:
- Circular dependencies cannot be resolved otherwise
- Code splitting is genuinely needed for performance
- The module must be loaded conditionally at runtime
Files:
apps/webapp/app/v3/runOpsShardTable.tsapps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/test/runOpsDbTopology.test.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.tsapps/webapp/app/db.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/v3/runOpsShardTable.tsapps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/test/runOpsDbTopology.test.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.tsapps/webapp/app/db.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/v3/runOpsShardTable.tsapps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/test/runOpsDbTopology.test.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.tsapps/webapp/app/db.server.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/v3/runOpsShardTable.tsapps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/test/runOpsDbTopology.test.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.tsapps/webapp/app/db.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepath
Files:
apps/webapp/app/v3/runOpsShardTable.tsapps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/test/runOpsDbTopology.test.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.tsapps/webapp/app/db.server.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.{ts,tsx}: For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
UseuseCallbackanduseMemoonly for context provider values, expensive derived data used as a dependency, or stable references required by dependency arrays; do not wrap ordinary event handlers or trivial computations.
Use named constants for sentinel or placeholder values instead of scattering raw string literals across comparisons.
Files:
apps/webapp/app/v3/runOpsShardTable.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.tsapps/webapp/app/db.server.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.ts: Never userequest.signalto detect client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.ts, which is wired to Express response close events.
Access environment variables through theenvexport fromapp/env.server.ts; never useprocess.envdirectly.
Always use PrismafindFirstinstead offindUnique.
Always use the$transactionhelper from~/db.server, never callprisma.$transactionor$replica.$transactiondirectly. Pass isolation levels as strings, useSerializablefor correctness-critical read-then-write invariants, and guard possibly undefined helper results when a definite value is required.
Files:
apps/webapp/app/v3/runOpsShardTable.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.tsapps/webapp/app/db.server.ts
apps/webapp/app/v3/**/*.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
New code must target Run Engine V2 through the singleton in
app/v3/runEngine.server.ts; do not reintroduce V1 execution paths. V1 branches may only reject or finalize gracefully with a clean 4xx.
Files:
apps/webapp/app/v3/runOpsShardTable.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Add crumbs as you write code — not just when debugging. Mark lines with
//@Crumbsor wrap blocks in `// `#region` `@crumbs. They stay on the
branch throughout development and are stripped byagentcrumbs strip
before merge.
Do not invent new namespaces — pick from this table or ask first.
Do not flag//@Crumbsmarkers, `#region `@crumbsblocks, or agentcrumbs
imports in reviews. These are temporary debug instrumentation stripped
before merge. Data logged in crumbs (IDs, names, values) never reaches
production.
Files:
apps/webapp/app/v3/runOpsShardTable.tsapps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/test/runOpsDbTopology.test.tsapps/webapp/app/v3/runStore.server.tsapps/webapp/app/v3/runOpsPoolKnobs.server.tsapps/webapp/app/db.server.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use vitest for all tests in the Trigger.dev repository
**/*.{test,spec}.{ts,tsx}: We use vitest exclusively. Never mock anything - use testcontainers instead.
Test files go next to source files (e.g.,MyService.ts->MyService.test.ts).
Files:
apps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/test/runOpsDbTopology.test.ts
apps/webapp/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Do not import
env.server.tsdirectly or indirectly into test files; instead pass environment-dependent values through options/parameters to make code testable
Files:
apps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/test/runOpsDbTopology.test.ts
apps/webapp/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Test files must not import
app/env.server.ts; pass configuration as options instead.
Files:
apps/webapp/test/runOpsPoolKnobs.test.tsapps/webapp/test/runOpsShardBootTable.test.tsapps/webapp/test/runOpsDbTopology.test.ts
🧠 Learnings (2)
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.
Applied to files:
apps/webapp/app/v3/runOpsShardTable.ts
📚 Learning: 2026-06-16T09:19:47.637Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3960
File: apps/webapp/test/prismaInfrastructureErrorCapture.test.ts:0-0
Timestamp: 2026-06-16T09:19:47.637Z
Learning: In this repo’s Vitest setup, `vitest.config.ts` uses `globals: true`, so identifiers like `vi`, `describe`, `it`, and `expect` are available as globals in Vitest test files. During code review, do not flag missing `vi`/`describe`/`it`/`expect` imports as a runtime error or correctness issue when they’re used in `*.test.ts/tsx` or `*.spec.ts/tsx` files. Explicit imports are still preferred for consistency, but they’re not required for runtime behavior.
Applied to files:
apps/webapp/test/runOpsDbTopology.test.ts
🔇 Additional comments (12)
apps/webapp/app/v3/runOpsPoolKnobs.server.ts (1)
4-4: LGTM!Also applies to: 19-80
apps/webapp/test/runOpsPoolKnobs.test.ts (1)
6-40: LGTM!apps/webapp/app/db.server.ts (1)
35-35: LGTM!Also applies to: 281-303, 440-476
apps/webapp/app/v3/runOpsShardTable.ts (1)
1-28: LGTM!apps/webapp/test/runOpsDbTopology.test.ts (1)
197-213: LGTM!apps/webapp/test/runOpsShardBootTable.test.ts (1)
1-34: LGTM!apps/webapp/app/v3/runStore.server.ts (6)
2-7: LGTM!
17-17: LGTM!
40-49: LGTM!
98-140: LGTM!
168-169: LGTM!
187-196: LGTM!
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Reported commits ==='
git show --stat --oneline --no-renames 20f0ee363 908fcb587
echo '=== Relevant implementation outlines ==='
ast-grep outline apps/webapp/app/v3/runStore.server.ts --items all --match 'buildRunStore|probeOrder|precedence' || true
ast-grep outline internal-packages/run-store/src/runOpsStore.ts --items all --match 'fromShards|RoutingRunStore|ShardTopology' || true
ast-grep outline apps/webapp/app/v3/runOpsPoolKnobs.server.ts --items all --match 'applyPoolKnobOverrides|resolveRunOpsPoolKnobs|poolKnobDefaults' || true
ast-grep outline apps/webapp/app/v3/runOpsShardTable.ts --items all || true
echo '=== Relevant source and tests ==='
rg -n -C 8 'probeOrder|precedence|fromShards|applyPoolKnobOverrides|selectRunOpsTopology' \
apps/webapp/app/v3/runStore.server.ts \
internal-packages/run-store/src/runOpsStore.ts \
apps/webapp/app/v3/runOpsPoolKnobs.server.ts \
apps/webapp/test/runOpsDbTopology.test.ts \
apps/webapp/test/runOpsPoolKnobs.test.ts \
apps/webapp/test/runOpsShardBootTable.test.ts \
apps/webapp/app/v3/runOpsShardTable.ts || true
echo '=== CodeRabbit PR review comments ==='
gh api --paginate 'repos/triggerdotdev/trigger.dev/pulls/4764/comments?per_page=100' \
--jq '.[] | select(.user.login == "coderabbitai") | {id, path, line, original_line, body, html_url}'Length of output: 77683
The four fixes are present in commits
I accept the two declined suggestions. The debug instrumentation is not required by repository guidance. A hard ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
1 participant
Part of the RunOps N-way sharding work.
This lets the webapp hold N run-ops stores, configured by a single
RUN_OPS_SHARDSJSON descriptor, and routes to them through the existing keyed router. Inert withRUN_OPS_SHARDSunset — the topology, the wiring andROUTING_ENABLEDare byte-identical to today.What's here
RUN_OPS_SHARDS— a zod-validated JSON array of shard descriptors (key,region,url,replicaUrl,directUrl,replication,knobs,aliasOf), validated at boot in theparseMachinePresetCsvstyle. Unset or[]→ no shards.buildRunOpsWriterClient/buildRunOpsReplicaClientcollapse into onebuildRunOpsClientparameterized by role and resolved pool knobs. The control-plane builders (buildWriterClient/buildReplicaClient) are a separate path and stay untouched; every resolved value matches the former builders.selectRunOpsTopology— one client pair per descriptor; analiasOf: "new"descriptor reuses the new store's clients by reference and opens no pool.buildRunStore— builds N dedicated stores + the keyed router via a newRoutingRunStore.fromShards, keeping the two-store compat router when no shards are configured.UnknownShardKey— raised when an id resolves to an unconfigured key; never falls back to another store.fromShardsinjectsresolveShardso a gen-2 id routes to its own shard.computeMintShardintersects the active mint list with the configured descriptor keys, so a key with no descriptor is never minted into.key, address fingerprint (host:port/db, no credentials), and role, only when shards are configured.Ordering constraint
Do not configure a
RUN_OPS_SHARDSdescriptor in any environment until the routing-semantics change (TRI-13427) lands — three fan-out sites still truncate at N>2. Merging this PR alone is safe (inert with the var unset); configuring a descriptor is what must wait.Testing
runOpsDbTopology.test.ts17/17,runStore.server.test.ts4/4,runOpsMigrationfamily 149/149.fromShardsrouting +UnknownShardKey, boot-table formatter, mint bound.Changelog
Internal run-ops sharding infrastructure. No changeset or
.server-changes: the change is inert withRUN_OPS_SHARDSunset and has no user-visible behaviour.🤖 Generated with Claude Code