feat(webapp): RUN_OPS_SHARDS config, topology and N-way store wiring - #4764
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>
|
WalkthroughThe change adds validated Merge Risk: 🟠 High · up to A non-empty shard configuration can be accepted while the application silently continues with the fallback topology, and current N-way routing can miss or double-count waitpoint work or mishandle connection failures. These correctness and availability risks make the PR unsafe to merge without fixes or explicit owner acceptance. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description clearly explains the RUN_OPS_SHARDS implementation, scope, ordering constraint, testing, and changelog. It omits the template checklist, issue-closing line, and screenshots section, but the core required information is complete. ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…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. |
The run-ops client factory eagerly $connects for warm-up, but only caught the rejection under NODE_ENV=test — outside test an unreachable shard/run-ops DB at boot surfaced as an unhandled promise rejection. Always catch and log instead; Prisma reconnects lazily on first query, so one unreachable shard must not take down startup. Scoped to the run-ops factory only; the control-plane/legacy builders are unchanged, so the RUN_OPS_SHARDS-unset path stays byte-identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ust the first When a gen-2 shard is configured, RoutingRunStore builds three or more stores (legacy + new + shard) and a waitpoint that is not on its home/run store must be found by probing the others. Three call sites took only the first "other" store (`#shardsExcept(key)[0]`), which was correct with the two-store compat router but silently skips the remaining stores once a shard exists. The effect, observed with a single shard configured: waitpoint lookups return "Waitpoint not found", pending-token counts undercount (which prematurely unblocks a still-waiting run), and many-waitpoint reads miss rows. Fix `#resolveWaitpointStore`, `countPendingWaitpoints` and `#collectManyWaitpoints` to fan out over every other store and merge. Adds a routing unit test that reproduces all three at the production probe order, with the target placed on the store the first-other truncation skipped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Manual testing performedI exercised this change locally across every supported run-ops split topology, using the Topologies exercised
The 4-DB run was the first to exercise the new N-way path. I stood up a second run-ops database ( MethodFor each topology I triggered What it foundThe 4-DB canary reproduced a waitpoint-routing bug in all three warm runs (clean at baseline and in 2-DB / 3-DB): 7 tasks failed identically — e.g. Fixed here by fanning Scope / caveats
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Resolve conflicts in runOpsStore.ts and runStore.server.ts against main's
RoutingRunStore refactor (constructor-based shards, #distinctStores,
#partitionAbsentIds/#gen1PairExcept, duplicate-id/probe-fallback metrics).
- runOpsStore.ts: take main. Main's #partitionAbsentIds/#gen1PairExcept
supersede this branch's #shardsExcept waitpoint fan-out fix and are more
precise (cuid relocation confined to the gen-1 pair; gen-2 id to its one
shard), so the earlier fix is dropped.
- runStore.server.ts: re-wire buildRunStore's N-way arm onto main's
RoutingRunStore constructor (shards: [{ key, store, aliasOf }],
resolveShard, metrics), replacing the removed fromShards factory.
- db.server.ts: carry each shard's declared aliasOf through
runOpsShardHandles so the router dedups aliased shards from fan-out sums.
- Rewrite the waitpoint fan-out guard test against the constructor API and
add a gen-2-shard collect case.
Verified: webapp typecheck, run-store typecheck, and the run-store routing
suite (shardMap, threeDbTopology, waitpoints, runKeyedRouting, guard) pass.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…dMatrix covers it Main's runOpsStore.nShardMatrix.test.ts is a four-store testcontainer matrix that already guards the N-way waitpoint fan-out on real databases — the gen-2-shard union with no double count, the mirrored-cuid case, alias dedup, and cross-tree completion. The removed test used fakeStore() stubs, which both duplicates that coverage and violates the repo's "never mock, use testcontainers" rule (CodeRabbit). Removing it also clears the code-quality oxfmt --check failure the unformatted file caused.
runOpsStore.fromShards.test.ts imported UnknownShardKey and called RoutingRunStore.fromShards — both removed in main's RoutingRunStore refactor (constructor-based shards). The file is unique to this branch and now references APIs that no longer exist, so it fails the run-store suite. Its routing coverage lives in main's shardMap/runKeyedRouting/nShardMatrix tests.
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 (1)
apps/webapp/app/env.server.ts (1)
2497-2504: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject shard descriptors that cannot build the sharded topology.
This check requires only
RUN_OPS_DATABASE_URL. A non-emptyRUN_OPS_SHARDSvalue withoutRUN_OPS_LEGACY_DATABASE_URLpasses validation, butapps/webapp/app/db.server.tsreturns the control-plane fallback andapps/webapp/app/v3/runStore.server.tsleaves routing disabled. The configured shards are silently ignored.Require the effective split-routing prerequisites when
RUN_OPS_SHARDSis non-empty, or fail topology construction instead of falling back.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0a1e2635-0029-4912-905e-1dcd5c333e48
📒 Files selected for processing (3)
apps/webapp/app/db.server.tsapps/webapp/app/env.server.tsapps/webapp/app/v3/runStore.server.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. (14)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - pnpm)
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
🧰 Additional context used
📓 Path-based instructions (10)
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.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/v3/runStore.server.ts
Never use `request.signal` to detect client disconnects. Use `getRequestAbortSignal()` from `app/services/httpAsyncStorage.server.ts`, which is wired to Express response close events.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/v3/runStore.server.tsapps/webapp/app/env.server.tsapps/webapp/app/db.server.ts
For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/v3/runStore.server.tsapps/webapp/app/env.server.tsapps/webapp/app/db.server.ts
**Prefer static imports over dynamic imports.** Only use dynamic `import()` when:
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/app/v3/runStore.server.tsapps/webapp/app/env.server.tsapps/webapp/app/db.server.ts
Add crumbs as you write code — not just when debugging. Mark lines with
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/app/v3/runStore.server.tsapps/webapp/app/env.server.tsapps/webapp/app/db.server.ts
Use zod for validation in packages/core and apps/webapp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/v3/runStore.server.tsapps/webapp/app/env.server.tsapps/webapp/app/db.server.ts
Access environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env`
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/app/v3/runStore.server.tsapps/webapp/app/env.server.tsapps/webapp/app/db.server.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/v3/runStore.server.tsapps/webapp/app/env.server.tsapps/webapp/app/db.server.ts
Use types over interfaces for TypeScript
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/v3/runStore.server.tsapps/webapp/app/env.server.tsapps/webapp/app/db.server.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
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
apps/webapp/app/v3/runStore.server.tsapps/webapp/app/env.server.tsapps/webapp/app/db.server.ts
🔇 Additional comments (2)
apps/webapp/app/db.server.ts (1)
281-316: LGTM!Also applies to: 335-379, 581-594
apps/webapp/app/v3/runStore.server.ts (1)
2-9: LGTM!Also applies to: 40-50, 100-142, 168-169, 187-197
…bases (#4780) ## Summary The run-ops boot interlocks and the migration entrypoint each assume exactly two run-ops databases. This generalizes them to any number, so a deployment that configures `RUN_OPS_SHARDS` gets the same safety guarantees it gets today with two stores: no two stores may point at one database, every store that owns its own database must replicate to ClickHouse, and every store must have its schema migrated. With `RUN_OPS_SHARDS` unset, nothing changes. The distinctness check over a two-element set is the pairwise compare it replaces, replication coverage is the check it was, and the entrypoint runs the same two migration invocations. A shard may declare `aliasOf: "new"`, which shares an existing store's client by reference. An aliased shard is not its own database, so it is exempt from the distinctness check and needs no replication slot of its own. Every check keys that exemption on the declared field, never on client object identity: two client objects can sit over one database, which identity comparison cannot see. ## Design **Distinctness.** `probeDistinctDatabases` compared two URLs. It now delegates to `probeDistinctStores`, which reads every fingerprint in parallel and groups them by system identifier and database name. Any two stores under one key refuse the boot. The old pairwise entry point stays, so its existing container tests are the proof that set uniqueness over one pair gives the verdict it gave before. Fail-closed is unchanged: a probe that cannot answer returns not-distinct, because "distinct" is a positive claim a failed probe cannot support. **Co-residency.** The advisory runs once per store against the control plane. The legacy emission keeps its exact call shape and its untagged metric series, so an existing dashboard does not change. Each shard emits its own point carrying its shard key. Every store emits before any enforcement throw, so one offending store never costs another store its metric. **Replication.** `buildReplicationSources` appends one source per shard that owns its own database, taking the slot, publication and origin generation its descriptor declares. `assertReplicationCoversSplit` then requires a source per such shard. That check also closes a hole it inherited. The descriptor parser validates uniqueness among shards only, so a shard could take the slot name, publication name or origin generation of the legacy or the new source. The replication service does validate this, but it throws from its constructor, and the caller reaches that constructor only after shutting the bootstrap instance down: ```ts if (sources.length > 1) { await service.shutdown(); // legacy stream stops here service = new RunsReplicationService({ ... }); // throws: duplicate slotName } ``` The throw was not a `SplitReplicationMisconfiguredError`, so the process stayed up with no replication at all, legacy included, behind one logged line. That is the silent ClickHouse under-count the error exists to prevent. The check now runs at the boot gate, before anything is torn down, and raises a subclass the existing exit path already recognizes. A correct deployment already satisfies it, because two consumers on one WAL slot is a data race that cannot work. **Migrations.** Every shard runs the identical schema, so a new shard is the existing migrations against a new DSN. The runner image has no `jq`, so a small node script prints one DSN per line and the entrypoint loops over them. The loop is a `for` and not a `while read` pipeline: a pipeline subshell swallows a failed migration on any iteration but the last, which would let a broken shard boot. Tracing stays off across the capture and the loop, because `set -x` prints an assignment and a DSN carries credentials. Verified end to end against real Postgres containers for the fingerprint probes, and against the real shell block with a stubbed migration command: an aliased shard is skipped, `directUrl` wins over `url`, a failing shard stops the container on the first failure, and a malformed descriptor stops it before it migrates anything. Stacked on #4764. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ncy (#4781) Gives read-through and idempotency their gen-2 shard arms, so an id that names its own shard is read there and nowhere else. #4764 has landed, so this now targets `main` directly and no longer depends on an unmerged branch. It builds on what that PR supplied: `resolveShard`, `runOpsShardHandles` and the keyed router. TRI-13431 ## What changes **Read-through routes by `resolveShard`, not by the binary residency classifier.** A gen-2 id reads its own shard's replica once and probes no other store. A gen-1 v1 id still reads new only. **Callers now declare `idKind`.** A cuid gives no way to tell a run id from a waitpoint id, and the two must route differently: - a legacy-classified **run** id reads the legacy replica only — there is no cuid run migration, so the new-store probe cannot find it; - a cuid **waitpoint** keeps the new-first pair probe, which is load-bearing because a cuid waitpoint can be co-located with its run on the new store. There is no default, because a default would pick one of those arms silently. The field `runId` is renamed to `id`, since it carried both kinds already. **`ReadThroughResult` carries `found`.** `source` is an open-ended union once shards exist, so a consumer testing found-ness by listing the hit sources reads a gen-2 hit as a miss. One consumer did exactly that. Discriminating on `found` makes that class of bug a compile error rather than something a reviewer has to spot. **Idempotency resolves its client through one shard-keyed map.** Both call sites go through `clientForShardKey`, so they cannot disagree about which store owns an id. An absent key takes an explicit logged branch to the fallback, not a silent legacy default. The `classify` seam is retyped to return a `ShardKey`: `Residency` (`"NEW"`) and the reserved shard keys (`"new"`) differ only by case, and `ShardKey` collapses to `string`, so the compiler would not have caught feeding one into the other. The dead `isMigrated` branch is deleted. Nothing implemented it, and the one production comment recorded that omitting it was deliberate. **`PostgresRunStore._residency` widens to `ShardKey`.** Still unused; the store stays unaware of its siblings. ## Two behaviour fixes found while doing the above **An unconfigured shard key logs and returns not-found instead of throwing.** The waitpoint route takes the id from a URL parameter, and any base32hex core plus `[a-z0-9]` plus `"2"` parses as gen-2. The route turns a throw into a 500, so throwing here would let any authenticated client generate 500s and error logs by guessing shard chars, of which there are 36. An error-logged not-found is neither silent nor a misroute. Throwing stays correct on the router path, where ids are minted rather than received. **The two cross-seam batch hydration sites were gen-2 blind.** `hydrateRunsAcrossSeam` and `ApiBatchResultsPresenter` classified with the binary `ownerEngine`, so a gen-2 run id joined the gen-1 `new` group, missed there, and — classifying dedicated-family — never reached the legacy probe either. The id was dropped from a bulk-action page and from batch results with no error. Both now partition ids by shard key and read each configured shard once. Also: a gen-2 waitpoint that missed its shard replica fell back to the gen-1 new writer, a different database, silently disabling read-your-writes for the freshly minted token that fallback exists to serve. It now falls back to its own shard's writer. ## Merge safety Inert while `RUN_OPS_SHARDS` is unset: the shard maps are empty, so every gen-2 arm is unreachable, and gen-2 minting is not live yet. The one live change is the gen-1 run arm, and it removes work rather than adding it. `RoutingRunStore.findRun` never forwards the caller's client object — it routes by id and reads only the client's presence and replica brand — so `readRunForEvent`'s "new" closure already resolved a legacy-classified run id to the legacy store. The arm removes a duplicated read of the legacy replica. A test pins this, because a future caller passing a raw client and a run id would lose the pre-cutover 27-char case, which is new-resident but classifies legacy. ## Testing 14 tests added, testcontainers throughout, no mocks. 22 affected test files pass; typecheck, lint, format and knip are clean. Both arms were verified by neutralising them and confirming the new tests fail. The batch-results test needed rewriting after that check: the first version passed with the fix neutralised, because it used one container as both the gen-1 new client and the shard replica, so it was not testing what it claimed. Note for review: run testcontainer suites in small batches. Sixteen at once starves Docker and everything times out at 60 seconds. The run-ops legacy-guard baseline is refreshed in its own commit. The baseline is keyed by line number, so partitioning the batch-results read shifted four pre-existing entries and added one. Baselined violations in that file go from four to five, all reads; the new one is the shard read beside two gen-1 reads already there. No changeset and no `.server-changes` entry: a user notices nothing while the flag is unset.
2 participants
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