re2: env based queue selection algo #1775
Conversation
…ll building and typechecks passing
…tabase, using better tsconfig setups for tests, moving isomorphic code into core/v3/isomorphic
|
WalkthroughThis pull request introduces a series of configuration and refactoring updates. It removes the outdated ESLint configuration file and revises VSCode launch configurations to enable debugging of RunQueue tests. Multiple TypeScript configuration files have been streamlined with simplified path mappings and added custom conditions. Numerous import statements throughout the project are updated to reference new module paths (e.g., from Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant VSCode as VS Code
participant Node as Node Terminal
participant Tester as Test Runner
Dev->>VSCode: Launch "Debug RunQueue tests"
VSCode->>Node: Execute command: pnpm run test ./src/engine/tests/waitpoints.test.ts (cwd: run-engine package)
Node->>Tester: Start test execution
Tester-->>Node: Return test results and logs
Node-->>VSCode: Send debug output
VSCode-->>Dev: Display debugging information
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
apps/webapp/app/v3/services/enqueueDelayedRun.server.ts (1)
85-103: Robust Transaction Handling:
The transactional update for marking the run as "PENDING" and scheduling an expiration job (ifttlis provided) is well implemented. Ensure that the return values fromparseNaturalLanguageDurationare properly validated (e.g., non-negative) within your business logic.internal-packages/run-engine/src/engine/tests/batchTrigger.test.ts (3)
66-85: Initial run trigger logic
Triggering the first run while assigning batch IDs and specifying a queue is good. Consider testing a negative or error scenario in the future (e.g., invalid payload).
127-135: Starting run attempts
The logic to callstartRunAttempton dequeued items is correct. If feasible, consider verifying invalid snapshots or concurrency limit scenarios.
167-175: Ensuring batch completion check
Waiting 3 seconds then verifying the batch is completed is fine, though it relies on timing. For more deterministic tests, consider using event-based or poll-based checks.internal-packages/run-engine/src/engine/tests/batchTriggerAndWait.test.ts (2)
64-83: Parent run trigger
Triggering the parent run withfriendlyId, environment, and queue data is implemented correctly. Consider adding edge-case tests for missing or malformed properties if untested elsewhere.
205-230: Completing first child run
The process of dequeuing, starting, and then completing this child run is thorough. Testing an error or cancellation flow might further strengthen coverage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (107)
.eslintrc.js(0 hunks).vscode/launch.json(1 hunks)apps/supervisor/tsconfig.json(0 hunks)apps/webapp/.eslintrc(1 hunks)apps/webapp/app/components/run/TriggerDetail.tsx(1 hunks)apps/webapp/app/entry.server.tsx(1 hunks)apps/webapp/app/env.server.ts(1 hunks)apps/webapp/app/hooks/useSyncTraceRuns.ts(1 hunks)apps/webapp/app/presenters/HttpEndpointPresenter.server.ts(1 hunks)apps/webapp/app/presenters/v3/SpanPresenter.server.ts(1 hunks)apps/webapp/app/presenters/v3/TaskListPresenter.server.ts(1 hunks)apps/webapp/app/routes/api.v1.$endpointSlug.schedules.$id.registrations.ts(1 hunks)apps/webapp/app/routes/api.v1.$endpointSlug.sources.$id.ts(1 hunks)apps/webapp/app/routes/api.v1.$endpointSlug.triggers.$id.registrations.$key.ts(1 hunks)apps/webapp/app/routes/api.v1.$endpointSlug.triggers.$id.registrations.ts(1 hunks)apps/webapp/app/routes/api.v1.accounts.$accountId.connections.$clientSlug/route.ts(1 hunks)apps/webapp/app/routes/api.v1.endpointindex.$indexId.ts(1 hunks)apps/webapp/app/routes/api.v1.event-dispatchers.ephemeral.ts(1 hunks)apps/webapp/app/routes/api.v1.events.$eventId.ts(1 hunks)apps/webapp/app/routes/api.v1.events.bulk.ts(1 hunks)apps/webapp/app/routes/api.v1.events.ts(1 hunks)apps/webapp/app/routes/api.v1.http-endpoints.$httpEndpointId.env.$envType.$shortcode.ts(1 hunks)apps/webapp/app/routes/api.v1.jobs.$jobSlug.invoke.ts(1 hunks)apps/webapp/app/routes/api.v1.runs.$runId.logs/route.ts(1 hunks)apps/webapp/app/routes/api.v1.runs.$runId.statuses.$id/route.ts(1 hunks)apps/webapp/app/routes/api.v1.runs.$runId.statuses.ts(1 hunks)apps/webapp/app/routes/api.v1.runs.$runId.tasks.$id.complete/route.ts(2 hunks)apps/webapp/app/routes/api.v1.runs.$runId.tasks.$id.fail/route.ts(1 hunks)apps/webapp/app/routes/api.v1.runs.$runId.tasks/route.ts(2 hunks)apps/webapp/app/routes/api.v1.waitpoints.tokens.$waitpointFriendlyId.complete.ts(1 hunks)apps/webapp/app/routes/api.v1.waitpoints.tokens.ts(1 hunks)apps/webapp/app/routes/api.v1.webhooks.$key.ts(1 hunks)apps/webapp/app/routes/api.v2.$endpointSlug.sources.$id.ts(1 hunks)apps/webapp/app/routes/api.v2.$endpointSlug.triggers.$id.registrations.$key.ts(1 hunks)apps/webapp/app/routes/api.v2.events.$eventId.ts(1 hunks)apps/webapp/app/routes/api.v2.runs.$runId.statuses.ts(1 hunks)apps/webapp/app/routes/engine.v1.dev.dequeue.ts(1 hunks)apps/webapp/app/routes/engine.v1.dev.runs.$runFriendlyId.logs.debug.ts(1 hunks)apps/webapp/app/routes/engine.v1.dev.runs.$runFriendlyId.snapshots.$snapshotFriendlyId.attempts.complete.ts(1 hunks)apps/webapp/app/routes/engine.v1.dev.runs.$runFriendlyId.snapshots.$snapshotFriendlyId.attempts.start.ts(1 hunks)apps/webapp/app/routes/engine.v1.dev.runs.$runFriendlyId.snapshots.$snapshotFriendlyId.heartbeat.ts(1 hunks)apps/webapp/app/routes/engine.v1.dev.runs.$runFriendlyId.snapshots.latest.ts(1 hunks)apps/webapp/app/routes/engine.v1.runs.$runFriendlyId.wait.duration.ts(1 hunks)apps/webapp/app/routes/engine.v1.runs.$runFriendlyId.waitpoints.tokens.$waitpointFriendlyId.wait.ts(1 hunks)apps/webapp/app/routes/engine.v1.worker-actions.deployments.$deploymentFriendlyId.dequeue.ts(1 hunks)apps/webapp/app/routes/engine.v1.worker-actions.runs.$runFriendlyId.logs.debug.ts(1 hunks)apps/webapp/app/routes/resources.batches.$batchId.check-completion.ts(1 hunks)apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.waitpoints.$waitpointFriendlyId.complete/route.tsx(1 hunks)apps/webapp/app/services/endpoints/indexEndpoint.server.ts(1 hunks)apps/webapp/app/services/triggers/registerWebhook.server.ts(1 hunks)apps/webapp/app/services/worker.server.ts(1 hunks)apps/webapp/app/utils/delays.ts(1 hunks)apps/webapp/app/v3/friendlyIdentifiers.ts(1 hunks)apps/webapp/app/v3/handleSocketIo.server.ts(1 hunks)apps/webapp/app/v3/marqs/devQueueConsumer.server.ts(1 hunks)apps/webapp/app/v3/marqs/fairDequeuingStrategy.server.ts(0 hunks)apps/webapp/app/v3/models/workerDeployment.server.ts(1 hunks)apps/webapp/app/v3/runEngine.server.ts(1 hunks)apps/webapp/app/v3/runEngineHandlers.server.ts(1 hunks)apps/webapp/app/v3/services/batchTriggerV4.server.ts(1 hunks)apps/webapp/app/v3/services/changeCurrentDeployment.server.ts(1 hunks)apps/webapp/app/v3/services/createBackgroundWorker.server.ts(1 hunks)apps/webapp/app/v3/services/createCheckpoint.server.ts(1 hunks)apps/webapp/app/v3/services/createDeployedBackgroundWorker.server.ts(1 hunks)apps/webapp/app/v3/services/createDeploymentBackgroundWorker.server.ts(1 hunks)apps/webapp/app/v3/services/enqueueDelayedRun.server.ts(1 hunks)apps/webapp/app/v3/services/finalizeDeployment.server.ts(0 hunks)apps/webapp/app/v3/services/triggerTaskV1.server.ts(1 hunks)apps/webapp/app/v3/services/triggerTaskV2.server.ts(1 hunks)apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts(1 hunks)apps/webapp/package.json(2 hunks)apps/webapp/remix.config.js(1 hunks)apps/webapp/tsconfig.check.json(1 hunks)apps/webapp/tsconfig.json(1 hunks)internal-packages/database/package.json(1 hunks)internal-packages/redis-worker/package.json(2 hunks)internal-packages/redis-worker/src/index.ts(1 hunks)internal-packages/redis-worker/src/queue.ts(2 hunks)internal-packages/redis-worker/src/telemetry.ts(0 hunks)internal-packages/redis-worker/src/worker.test.ts(0 hunks)internal-packages/redis-worker/src/worker.ts(1 hunks)internal-packages/redis-worker/tsconfig.build.json(1 hunks)internal-packages/redis-worker/tsconfig.json(1 hunks)internal-packages/redis-worker/tsconfig.src.json(1 hunks)internal-packages/redis-worker/tsconfig.test.json(1 hunks)internal-packages/redis/package.json(1 hunks)internal-packages/redis/src/index.ts(1 hunks)internal-packages/redis/tsconfig.json(1 hunks)internal-packages/run-engine/package.json(1 hunks)internal-packages/run-engine/src/engine/db/worker.ts(1 hunks)internal-packages/run-engine/src/engine/eventBus.ts(1 hunks)internal-packages/run-engine/src/engine/executionSnapshots.ts(1 hunks)internal-packages/run-engine/src/engine/index.ts(4 hunks)internal-packages/run-engine/src/engine/locking.ts(2 hunks)internal-packages/run-engine/src/engine/retrying.ts(1 hunks)internal-packages/run-engine/src/engine/tests/batchTrigger.test.ts(1 hunks)internal-packages/run-engine/src/engine/tests/batchTriggerAndWait.test.ts(1 hunks)internal-packages/run-engine/src/engine/tests/cancelling.test.ts(2 hunks)internal-packages/run-engine/src/engine/tests/checkpoints.test.ts(1 hunks)internal-packages/run-engine/src/engine/tests/delays.test.ts(2 hunks)internal-packages/run-engine/src/engine/tests/dequeuing.test.ts(2 hunks)internal-packages/run-engine/src/engine/tests/heartbeats.test.ts(3 hunks)internal-packages/run-engine/src/engine/tests/notDeployed.test.ts(1 hunks)internal-packages/run-engine/src/engine/tests/priority.test.ts(1 hunks)internal-packages/run-engine/src/engine/tests/trigger.test.ts(2 hunks)internal-packages/run-engine/src/engine/tests/triggerAndWait.test.ts(1 hunks)internal-packages/run-engine/src/engine/tests/ttl.test.ts(1 hunks)
⛔ Files not processed due to max files limit (31)
- internal-packages/run-engine/src/engine/tests/waitpoints.test.ts
- internal-packages/run-engine/src/engine/types.ts
- internal-packages/run-engine/src/index.ts
- internal-packages/run-engine/src/run-queue/constants.ts
- internal-packages/run-engine/src/run-queue/fairQueueSelectionStrategy.test.ts
- internal-packages/run-engine/src/run-queue/fairQueueSelectionStrategy.ts
- internal-packages/run-engine/src/run-queue/index.test.ts
- internal-packages/run-engine/src/run-queue/index.ts
- internal-packages/run-engine/src/run-queue/keyProducer.test.ts
- internal-packages/run-engine/src/run-queue/keyProducer.ts
- internal-packages/run-engine/src/run-queue/simpleWeightedPriorityStrategy.ts
- internal-packages/run-engine/src/run-queue/types.ts
- internal-packages/run-engine/src/shared/index.ts
- internal-packages/run-engine/tsconfig.build.json
- internal-packages/run-engine/tsconfig.json
- internal-packages/run-engine/tsconfig.src.json
- internal-packages/run-engine/tsconfig.test.json
- internal-packages/run-engine/vitest.config.ts
- internal-packages/testcontainers/src/setup.ts
- internal-packages/tracing/README.md
- internal-packages/tracing/package.json
- internal-packages/tracing/src/index.ts
- internal-packages/tracing/tsconfig.json
- internal-packages/zod-worker/package.json
- internal-packages/zod-worker/src/index.ts
- package.json
- packages/core/package.json
- packages/core/src/v3/apps/index.ts
- packages/core/src/v3/isomorphic/index.ts
- packages/react-hooks/src/package.json
- turbo.json
💤 Files with no reviewable changes (6)
- apps/supervisor/tsconfig.json
- internal-packages/redis-worker/src/worker.test.ts
- internal-packages/redis-worker/src/telemetry.ts
- apps/webapp/app/v3/marqs/fairDequeuingStrategy.server.ts
- .eslintrc.js
- apps/webapp/app/v3/services/finalizeDeployment.server.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
🔇 Additional comments (149)
apps/webapp/app/routes/engine.v1.runs.$runFriendlyId.wait.duration.ts (1)
3-3: Update Import for RunId EntityThe import for
RunIdhas been updated from@trigger.dev/core/v3/appsto@trigger.dev/core/v3/isomorphic. Please verify that the new module provides the expected API and type support to ensure complete compatibility with the rest of the code. If all tests pass and the functionality remains intact, this change is well-aligned with the broader module reorganization effort.apps/webapp/app/v3/handleSocketIo.server.ts (1)
12-12: Correct Import Update for RunIdThe
RunIdimport has been updated to reference@trigger.dev/core/v3/isomorphicinstead of the previous module. This change aligns with the broader refactoring efforts mentioned in the PR summary and ensures that the entity is imported from the appropriate isomorphic module.internal-packages/redis/package.json (1)
18-18: Newline at End of File:
The addition of a newline at the end of the file is a good practice to ensure proper file termination.apps/webapp/app/routes/api.v1.waitpoints.tokens.ts (1)
6-6: Updated Import for WaitpointId:
The import has been updated to fetchWaitpointIdfrom@trigger.dev/core/v3/isomorphic, aligning with the recent module reorganization. Please verify that all usages ofWaitpointIdwork seamlessly with this new source.apps/webapp/app/routes/resources.batches.$batchId.check-completion.ts (1)
3-3: Refined Import for assertExhaustive:
The updated import forassertExhaustivefrom@trigger.dev/core/utilsimproves module clarity. Ensure that this change is consistently reflected across the codebase.apps/webapp/app/routes/api.v1.runs.$runId.statuses.ts (1)
3-3: Consistent Schema Import Update:
The change to importJobRunStatusRecordSchemafrom@trigger.dev/core/schemasis consistent with the broader refactoring effort. The downstream usage in the records parsing appears correct.apps/webapp/app/v3/services/enqueueDelayedRun.server.ts (1)
1-1: Updated Import for parseNaturalLanguageDuration:
The import forparseNaturalLanguageDurationhas been updated to use@trigger.dev/core/v3/isomorphic, reflecting the new module organization. This change, along with the consistent project-wide updates, improves maintainability.apps/webapp/app/routes/engine.v1.dev.dequeue.ts (1)
3-3: Update import path for BackgroundWorkerId.The import now coming from
@trigger.dev/core/v3/isomorphicensures consistency across modules. Please verify that downstream consumers ofBackgroundWorkerIdare compatible with this updated import source.apps/webapp/app/routes/api.v1.jobs.$jobSlug.invoke.ts (1)
3-3: Update import path for InvokeJobRequestBodySchema.Switching to importing from
@trigger.dev/core/schemasmakes the schema’s source more precise, which enhances maintainability. No logic changes are introduced.apps/webapp/app/routes/api.v2.runs.$runId.statuses.ts (1)
3-3: Update import path for JobRunStatusRecordSchema.The change to import from
@trigger.dev/core/schemasstandardizes schema locations. This update is straightforward and does not impact functionality.internal-packages/run-engine/src/engine/db/worker.ts (1)
8-8: Update import path for CURRENT_DEPLOYMENT_LABEL.Updating the source to
@trigger.dev/core/v3/isomorphicaligns with the broader refactoring strategy. Ensure that the constant’s value and behavior remain as expected after this change.apps/webapp/app/routes/engine.v1.worker-actions.deployments.$deploymentFriendlyId.dequeue.ts (1)
2-2: Update import path for CURRENT_DEPLOYMENT_LABEL.This modification to import from
@trigger.dev/core/v3/isomorphicbrings clarity to where the deployment labels are defined. The change is consistent with similar updates elsewhere in the codebase.apps/webapp/app/routes/api.v1.runs.$runId.statuses.$id/route.ts (1)
3-3: Schema Import Path UpdateThe import for both
JobRunStatusRecordSchemaandStatusUpdateSchemahas been updated to use the new path (@trigger.dev/core/schemas). This update improves clarity and consistency with the reorganized schema exports across the codebase.apps/webapp/app/v3/friendlyIdentifiers.ts (1)
1-1: Export Source Update for generateFriendlyIdThe export now correctly retrieves
generateFriendlyIdfrom@trigger.dev/core/v3/isomorphicrather than from the old@trigger.dev/core/v3/apps. Please ensure that all module consumers have been updated to reflect this new source.apps/webapp/app/v3/marqs/devQueueConsumer.server.ts (1)
22-22: Import Update for getMaxDurationThe import of
getMaxDurationhas been revised to import from@trigger.dev/core/v3/isomorphicinstead of the previous module path. This is consistent with the broader refactoring efforts and helps maintain a streamlined module structure.apps/webapp/app/routes/api.v1.runs.$runId.logs/route.ts (1)
3-3: Updated Schema Import for LogMessageSchemaThe import path for
LogMessageSchemahas been updated to@trigger.dev/core/schemas, aligning it with other similar schema imports across the project. No functionality is affected by this change, and it enhances clarity.internal-packages/run-engine/src/engine/eventBus.ts (1)
2-2: Refined Import Path for AuthenticatedEnvironmentThe import for
AuthenticatedEnvironmentnow explicitly includes the.jsextension (../shared/index.js). This change enforces a consistent module resolution approach. Please verify that the build and bundler configurations support explicit file extensions to avoid any runtime issues.apps/webapp/app/v3/models/workerDeployment.server.ts (1)
3-6: Import path update looks goodThe refactoring of the import path from
@trigger.dev/core/v3/appsto@trigger.dev/core/v3/isomorphicis consistent with the broader reorganization of module imports in this PR. The constants continue to be used correctly throughout the file.apps/webapp/app/routes/api.v1.waitpoints.tokens.$waitpointFriendlyId.complete.ts (1)
8-8: Import path update looks goodThe import of
WaitpointIdfrom the isomorphic module aligns with the broader refactoring pattern in this PR. The type is used appropriately on line 27 to convert the friendly ID to an internal ID format.apps/webapp/app/v3/services/batchTriggerV4.server.ts (1)
9-9: Import path update looks goodThe migration of
BatchIdandRunIdimports from apps to isomorphic module is consistent with the pattern established across this PR. These types are used correctly throughout the batch processing logic.apps/webapp/app/v3/services/createDeploymentBackgroundWorker.server.ts (1)
12-12: Import path update looks goodUpdating the import path for
BackgroundWorkerIdto use the isomorphic module follows the consistent pattern throughout this PR. The ID generator is used correctly on line 39 in the worker creation process.apps/webapp/app/v3/services/changeCurrentDeployment.server.ts (1)
5-5: Update Import Path for CURRENT_DEPLOYMENT_LABEL
The import has been updated to source from@trigger.dev/core/v3/isomorphicinstead of the older module. This change standardizes the imports across the codebase without impacting the service’s functionality.apps/webapp/app/routes/api.v1.runs.$runId.tasks.$id.fail/route.ts (1)
3-3: Update Schema Import for FailTaskBodyInputSchema
The import has been modified to pullFailTaskBodyInputSchemafrom@trigger.dev/core/schemas, aligning it with the new module structure. This change is purely a refactor and does not affect the runtime behavior.apps/webapp/app/utils/delays.ts (1)
1-1: Update Import for parseNaturalLanguageDuration
The changed import source to@trigger.dev/core/v3/isomorphicis consistent with other similar updates in the codebase. The functional logic ofcalculateDurationInMsandparseDelayremains intact.apps/webapp/app/presenters/v3/TaskListPresenter.server.ts (1)
23-23: Update Import for CURRENT_DEPLOYMENT_LABEL in TaskListPresenter
By switching the import to@trigger.dev/core/v3/isomorphic, the file now aligns with the overall refactoring effort across the project. Please ensure that all related modules are consistent with this change.apps/webapp/app/presenters/v3/SpanPresenter.server.ts (1)
14-14: Update Import for getMaxDuration
The updated import from@trigger.dev/core/v3/isomorphicensures that thegetMaxDurationfunction is consistently sourced, matching the broader codebase refactor. No additional action is necessary.apps/webapp/app/routes/engine.v1.runs.$runFriendlyId.waitpoints.tokens.$waitpointFriendlyId.wait.ts (1)
3-3: Update Import Path for Type Declarations
The import forRunIdandWaitpointIdhas been successfully updated to use the reorganized module@trigger.dev/core/v3/isomorphic. Please verify that the new module exports these identifiers correctly across all usage points.apps/webapp/app/v3/services/createBackgroundWorker.server.ts (1)
24-24: Revised BackgroundWorkerId Import
The import ofBackgroundWorkerIdis now sourced from@trigger.dev/core/v3/isomorphic, which aligns with the project’s new module organization. This change is straightforward and appears correct.apps/webapp/app/v3/services/triggerTaskV1.server.ts (1)
11-14: Update Utility Imports for Consistency
The import for utility functions (parseNaturalLanguageDuration,sanitizeQueueName, andstringifyDuration) has been updated to be imported from@trigger.dev/core/v3/isomorphic. This reflects the broader refactoring effort to consolidate imports. Ensure that any related types (e.g. forBatchIdandRunId) are also updated consistently across the codebase.apps/webapp/app/v3/services/createCheckpoint.server.ts (1)
11-11: Align CheckpointId Import with New Module Structure
The import forCheckpointIdnow comes from@trigger.dev/core/v3/isomorphic, which is consistent with the recent refactorings. No further changes appear necessary, but please verify that all dependent code seamlessly integrates with this update.apps/webapp/app/routes/api.v1.endpointindex.$indexId.ts (1)
1-4: Schema Import Path Update
The import forGetEndpointIndexResponseandGetEndpointIndexResponseSchemahas been changed to use@trigger.dev/core/schemasinstead of the main module. This improves module organization and clarity about schema sources. The rest of the file logic remains intact.apps/webapp/app/routes/engine.v1.dev.runs.$runFriendlyId.logs.debug.ts (1)
2-3: Refactored Import PathsThe import paths for both
assertExhaustiveandRunIdhave been updated to reflect the new module organization. This change is consistent with the broader restructuring effort and should help improve clarity and maintainability..vscode/launch.json (1)
144-151: New Debug RunQueue Tests ConfigurationA new launch configuration named "Debug RunQueue tests" has been added, which targets a specific test (
./src/engine/tests/waitpoints.test.ts) with an appropriate working directory. Ensure that the command and cwd correctly reflect the project structure for the run-engine package.apps/webapp/app/routes/engine.v1.worker-actions.runs.$runFriendlyId.logs.debug.ts (1)
1-2: Consistent Import Update for Worker ActionsThe updated import statements for
assertExhaustiveandRunIdnow source these utilities from@trigger.dev/core/utilsand@trigger.dev/core/v3/isomorphicrespectively. This update aligns with changes in similar files across the codebase.apps/webapp/app/routes/api.v1.accounts.$accountId.connections.$clientSlug/route.ts (1)
3-6: Updated Schema Import PathsThe import of
CreateExternalConnectionBodySchemaandErrorWithStackSchemahas been shifted from the root package to@trigger.dev/core/schemas. This improved modularization should make the sources of the schemas clearer and help maintain consistency.apps/webapp/app/v3/services/triggerTaskV2.server.ts (1)
9-14: Refactored Import Paths for Isomorphic EntitiesThe imports for
BatchId,RunId,sanitizeQueueName, andstringifyDurationhave been updated to use the@trigger.dev/core/v3/isomorphicmodule instead of the previous location. This change is expected to harmonize usage across the project. Please verify that all dependencies on these entities function as expected.internal-packages/redis-worker/tsconfig.build.json (1)
1-21: Well-structured TypeScript configurationThis build-specific TypeScript configuration follows best practices with appropriate settings for a production build:
- Properly excludes test files
- Uses modern ECMAScript target and libraries
- Configures Node16 module system
- Enables strict type checking and declaration generation
- Includes necessary compiler flags for reliable builds
apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts (1)
27-30: Import path refactoring looks goodImports for
CURRENT_UNMANAGED_DEPLOYMENT_LABELandfromFriendlyIdhave been updated to source from@trigger.dev/core/v3/isomorphicinstead ofapps. This aligns with the broader refactoring effort to improve module organization seen throughout this PR.apps/webapp/app/v3/services/createDeployedBackgroundWorker.server.ts (1)
13-13: Import path updated correctlyThe import of
CURRENT_DEPLOYMENT_LABELandBackgroundWorkerIdfrom@trigger.dev/core/v3/isomorphicis consistent with the module reorganization pattern in this PR. This change maintains functionality while improving code organization.apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.waitpoints.$waitpointFriendlyId.complete/route.tsx (1)
11-12: Import optimizationsTwo useful import optimizations:
WaitpointIdis now sourced from@trigger.dev/core/v3/isomorphicinstead of theappspathWaitpointis marked as a type-only import, which is appropriate since it's only used for TypeScript type checkingBoth changes align with best practices for module organization and type usage.
apps/webapp/app/routes/api.v1.runs.$runId.tasks/route.ts (2)
3-3: Updated import path to use the dedicated schemas module.Import now uses the more specific
@trigger.dev/core/schemaspath rather than the general@trigger.dev/core, which aligns with the module reorganization in this PR.
12-15:❓ Verification inconclusive
Locally defined API_VERSIONS constant.
The API version constants have been defined locally rather than being imported from elsewhere. This is consistent with other changes in the PR and improves module organization.
Make sure this constant definition matches any other definitions of API_VERSIONS in the codebase to maintain consistency.
🏁 Script executed:
#!/bin/bash # Search for other definitions of API_VERSIONS in the codebase rg "const API_VERSIONS = \{" --type tsLength of output: 273
Action Required: Verify Consistency of API Version Definitions
The
API_VERSIONSconstant is defined locally in this file as intended. However, please note that similar declarations exist in:
apps/webapp/app/routes/api.v1.runs.$runId.tasks.$id.complete/route.tspackages/core/src/versions.tsEnsure that the version values and overall structure match across all these definitions to maintain consistency in the module organization.
apps/webapp/app/routes/api.v1.event-dispatchers.ephemeral.ts (1)
3-3: Updated import path to use the dedicated schemas module.Import now uses the more specific
@trigger.dev/core/schemaspath rather than the general@trigger.dev/core, which aligns with the module reorganization in this PR. The removal of unused imports (InvokeJobRequestBodySchema,InvokeJobService, andz) is a good cleanup.apps/webapp/app/routes/api.v1.runs.$runId.tasks.$id.complete/route.ts (2)
3-3: Updated import paths to use the dedicated schemas module.Imports now use the more specific
@trigger.dev/core/schemaspath rather than the general@trigger.dev/core, which aligns with the module reorganization in this PR.Also applies to: 7-7
16-19:❓ Verification inconclusive
Locally defined API_VERSIONS constant.
The API version constants have been defined locally rather than being imported from elsewhere. This is consistent with other similar changes in the PR and improves module organization.
This constant is referenced in line 73 to conditionally handle serialized task output.
🏁 Script executed:
#!/bin/bash # Check for consistency across API_VERSIONS definitions rg "const API_VERSIONS = \{.*?SERIALIZED_TASK_OUTPUT" --multiline --type tsLength of output: 77
Local API_VERSIONS Definition Verified – Please Confirm
The API_VERSIONS constant defined on lines 16–19 in
apps/webapp/app/routes/api.v1.runs.$runId.tasks.$id.complete/route.ts
appears to be deliberately local, matching similar changes elsewhere in the PR. Its usage on line 73 for handling serialized task output is consistent.Our automated search for API_VERSIONS definitions (including instances containing "SERIALIZED_TASK_OUTPUT") did not return any conflicting entries. That said, since the initial grep did not produce any output, please manually verify that no additional (and possibly unintended) API_VERSIONS definitions exist in the codebase.
internal-packages/run-engine/src/engine/retrying.ts (2)
1-10: Updated imports from @trigger.dev/core/v3.Added imports for
calculateNextRetryDelayandsanitizeError, which are used in the file (lines 50 and 59). Also reordered the imports for better organization.
11-13: Updated import paths for better module resolution.The changes include:
- Removing the unused
TaskRunStatusimport- Adding
.jsextensions to local imports, which is important for ESM compatibilityThese changes improve code cleanliness and ensure proper module resolution in different environments.
internal-packages/database/package.json (3)
5-6: Updated output location to dist directory - good practice!Changing from source files to compiled output is a best practice for library packages. This ensures consumers use compiled JavaScript rather than requiring TypeScript compilation themselves.
12-12: Added rimraf as a dev dependencyAdding rimraf is appropriate for cleaning the dist directory before builds. This helps prevent stale files from persisting across builds.
15-15: Added proper build scripts for improved development workflowThe new scripts create a complete build process with:
clean- removing previous build artifactstypecheck- validating types without emitting filesbuild- creating a production builddev- providing watch mode for developmentThis follows standard practices for TypeScript projects and improves the development experience.
Also applies to: 22-24
apps/webapp/app/routes/engine.v1.dev.runs.$runFriendlyId.snapshots.$snapshotFriendlyId.attempts.complete.ts (1)
2-2:✅ Verification successful
Import path updated to use isomorphic module
Good refactoring to move from
/appsto/isomorphicfor these ID types. This suggests these types are now shared between client and server code, improving code organization.
🏁 Script executed:
#!/bin/bash # Verify that the RunId and SnapshotId implementations are compatible between the old and new paths # to ensure this refactor doesn't change behavior # Check for both old and new imports across the codebase echo "Checking for old imports still in use:" rg "@trigger.dev/core/v3/apps.+(RunId|SnapshotId)" --type ts echo "Checking implementation differences between old and new paths:" rg -A 10 "export.+(RunId|SnapshotId)" --type tsLength of output: 1652
Import path updated to use isomorphic module
Great work on refactoring the import for
RunIdandSnapshotIdfrom@trigger.dev/core/v3/appsto@trigger.dev/core/v3/isomorphic. This change improves code organization by making these types available in both client and server contexts. The verification script output confirms that there are no lingering references to the old import path and that the implementations in the isomorphic module are consistent.apps/webapp/tsconfig.check.json (1)
8-9: Added customConditions for module resolution configurationAdding
"customConditions": []aligns with modern TypeScript module resolution practices. This property allows specifying conditions for package.json exports field resolution, though it's currently set to an empty array.This change likely brings consistency with other TypeScript configurations in the project.
internal-packages/redis/tsconfig.json (1)
5-6: Updated to modern Node16 module systemUpgrading to
"module": "Node16"and"moduleResolution": "Node16"brings several benefits:
- Better support for both ESM and CommonJS interoperability
- Support for package.json "exports" field and conditional exports
- Improved resolution algorithms matching Node.js 16+ behavior
This modernization aligns with the
customConditionsaddition in other configuration files and represents a forward-looking approach to module management.apps/webapp/.eslintrc (1)
2-2:❓ Verification inconclusive
Verify ESLint functionality after plugin removal
The plugin
"@trigger.dev/eslint-plugin"has been removed from the plugins array, but the rule"@trigger.dev/no-trigger-core-import"is still being used on line 9. Ensure that this rule is now defined elsewhere or that ESLint will still function correctly without this plugin.
🏁 Script executed:
#!/bin/bash # Check if the ESLint rule is defined elsewhere or if it will cause errors # First, check if any package provides this rule npm list | grep eslint-plugin # Then check if the rule is actually used in the codebase rg "@trigger.dev/no-trigger-core-import" --type tsLength of output: 2616
Action Required: Confirm ESLint Rule Definition & Linting Behavior
The plugin
@trigger.dev/eslint-pluginwas removed from the plugins list, yet the ESLint rule@trigger.dev/no-trigger-core-importis still referenced (line 9). The initial verification commands (npm list and a TypeScript-limited search) did not return useful information, likely because:
- The npm command returned errors unrelated to the specific ESLint rule.
- Restricting the search to TypeScript files may have missed its occurrence in configuration (e.g., JSON) files.
Please ensure that either:
- The rule
@trigger.dev/no-trigger-core-importis now defined via another configuration or extended rules package, or- The rule is removed from your ESLint configuration if it is no longer supported.
To verify this, run ESLint on the repository and/or search without file-type restrictions (for example, using
rg "@trigger.dev/no-trigger-core-import"without limiting to TS files) to confirm whether a valid definition exists or if lint errors occur.apps/webapp/app/presenters/HttpEndpointPresenter.server.ts (1)
4-4: Import path updated to server-specific moduleThe import path has been updated to include
.serversuffix, which is likely part of a broader refactoring to better differentiate between client and server code.internal-packages/redis/src/index.ts (1)
4-5: Enhanced API exports from Redis moduleThe module now re-exports additional types from
ioredis, making it easier for consumers to access these types without importing directly from the external library.apps/webapp/app/routes/api.v1.events.ts (1)
3-3: More specific import path for schemaThe import has been updated to reference a more specific path (
@trigger.dev/core/schemasinstead of@trigger.dev/core), which is part of a broader refactoring to better organize imports across the codebase.apps/webapp/app/routes/api.v2.events.$eventId.ts (1)
3-3: Update Import Path for GetEvent
The import forGetEventhas been updated to use the new module path (@trigger.dev/core/schemas), which is consistent with the refactoring across the codebase.apps/webapp/app/v3/runEngineHandlers.server.ts (1)
15-15: Update Import for RunId
The import statement forRunIdnow correctly references@trigger.dev/core/v3/isomorphicinstead of the old path. This change aligns with the recent module reorganization.apps/webapp/package.json (1)
17-17: Enhance TypeScript Typecheck Script
The"typecheck"script has been updated to include the--noEmitflag, ensuring that the TypeScript compiler only performs type-checking without generating output files. This is a good practice that enhances efficiency during development.apps/webapp/app/routes/api.v1.$endpointSlug.triggers.$id.registrations.$key.ts (1)
3-3: Update Schema Import for Trigger Registration
The import forRegisterTriggerBodySchemaV1is now sourced from@trigger.dev/core/schemas. This change clarifies the module structure and maintains consistency with similar updates in the project.apps/webapp/app/routes/api.v1.events.$eventId.ts (1)
3-3: Update GetEvent Import for v1 Events
The import forGetEventhas been modified to load from@trigger.dev/core/schemas. This change is consistent with the overall module reorganization aimed at better managing schema exports.apps/webapp/app/routes/engine.v1.dev.runs.$runFriendlyId.snapshots.latest.ts (1)
2-2: Updated Import Path for RunId Approved.
The change now importsRunIdfrom@trigger.dev/core/v3/isomorphicinstead of the old module. This aligns the code with the new module organization and should help improve maintainability. Please verify all usages ofRunIdwork correctly with the new source.apps/webapp/app/routes/api.v1.$endpointSlug.triggers.$id.registrations.ts (1)
3-3: Update Import Path for InitializeTriggerBodySchema Approved.
The import now comes from@trigger.dev/core/schemas, which centralizes schema definitions. This refactor clarifies the origin of the schema and is consistent with similar changes across the codebase.apps/webapp/app/entry.server.tsx (1)
17-17: Refactored Server Import for SQS Event Consumer Approved.
The updated import forgetSharedSqsEventConsumerfrom./services/events/sqsEventConsumer.serverensures that the consumer implementation is correctly isolated to server-side functionality. Confirm that any dependent modules or runtime behaviors respond appropriately to this change.apps/webapp/app/routes/engine.v1.dev.runs.$runFriendlyId.snapshots.$snapshotFriendlyId.attempts.start.ts (1)
3-3: Standardized Import for RunId and SnapshotId Approved.
Importing bothRunIdandSnapshotIdfrom@trigger.dev/core/v3/isomorphicmaintains consistency in identifier sourcing across the project. Ensure that functions leveraging these identifiers continue to operate as expected.apps/webapp/app/routes/api.v2.$endpointSlug.sources.$id.ts (1)
3-3: Schema Import Path Update for UpdateTriggerSourceBodyV2Schema Approved.
The change updates the import forUpdateTriggerSourceBodyV2Schemato come from@trigger.dev/core/schemas, which improves code clarity by consolidating the schema definitions. Verify that schema validations and subsequent processing remain unaffected.apps/webapp/app/routes/api.v1.$endpointSlug.sources.$id.ts (1)
3-3: Update import path for schema resolutionThe import for
UpdateTriggerSourceBodyV1Schemanow references"@trigger.dev/core/schemas", which aligns with the new module structure. This change is clear and consistent—please ensure that all related usages in the codebase have been updated accordingly.apps/webapp/app/routes/api.v1.webhooks.$key.ts (1)
3-3: Refine schema import for webhook updatesThe updated import for
UpdateWebhookBodySchemafrom"@trigger.dev/core/schemas"reflects the reorganization of schema exports. This update improves clarity and maintainability.apps/webapp/app/routes/api.v1.events.bulk.ts (1)
3-3: Align schema import with revised module pathsThe change to import
SendBulkEventsBodySchemafrom"@trigger.dev/core/schemas"rather than the previous location is consistent with the overall refactoring. This modification does not impact the logic but enhances module organization and clarity.internal-packages/run-engine/src/engine/executionSnapshots.ts (1)
2-2: Update identifier imports for isomorphic compatibilityThe import of
BatchId,RunId, andSnapshotIdhas been shifted from"@trigger.dev/core/v3/apps"to"@trigger.dev/core/v3/isomorphic". This change appropriately reflects the new module organization for identifiers used across both server and client contexts.apps/webapp/app/hooks/useSyncTraceRuns.ts (1)
1-1: Adopt type-only import for improved type claritySwitching the import of
Prettifyto a type-only import (import type { Prettify } from "@trigger.dev/core";) ensures that no unnecessary runtime code is imported, aligning with TypeScript best practices.apps/webapp/app/services/endpoints/indexEndpoint.server.ts (1)
3-3: Import path updated to include server suffixThe import for
PerformEndpointIndexServicehas been updated to use the.serversuffix, which aligns with the server-side nature of this service. This change is consistent with similar import updates across other files in this PR.apps/webapp/app/routes/api.v1.http-endpoints.$httpEndpointId.env.$envType.$shortcode.ts (1)
2-5: Import path updated to include server suffixThe imports for
HandleHttpEndpointServiceandHttpEndpointParamsSchemanow properly reference the.serversuffix in their path, which correctly identifies these components as server-side services. This change maintains consistency with the server-side architecture patterns being established throughout the codebase.internal-packages/run-engine/src/engine/tests/checkpoints.test.ts (2)
8-8: Updated tracing import sourceThe import for
tracehas been changed from@opentelemetry/apito@internal/tracing, which suggests an internal abstraction or wrapper has been created for tracing functionality. This change helps standardize and possibly extend the tracing capabilities across the application.
14-15: Added global test timeout configurationSetting a global timeout of 60 seconds for these tests is appropriate given their nature. This eliminates the need for individual timeout parameters on each test case, improving code cleanliness and consistency across test files.
apps/webapp/app/routes/api.v1.$endpointSlug.schedules.$id.registrations.ts (1)
3-6: Import path updated to more specific module locationThe import paths for
RegisterScheduleBodySchemaandRegisterScheduleResponseBodySchemahave been updated from@trigger.dev/coreto the more specific@trigger.dev/core/schemas. This change provides better organization and clarity about where these schemas are located in the module structure.apps/webapp/app/components/run/TriggerDetail.tsx (1)
14-14: Good use of type-only importConverting the
DisplayPropertyimport to a type-only import is a good practice that helps the TypeScript compiler understand this is only needed for type checking and not at runtime. This contributes to better tree-shaking and potentially smaller bundle sizes.apps/webapp/remix.config.js (1)
12-13: Smart simplification of bundling configurationUsing regex patterns to include all packages with specific prefixes is more maintainable than listing individual packages. This approach automatically captures any new packages with these prefixes without requiring configuration updates.
apps/webapp/app/routes/api.v2.$endpointSlug.triggers.$id.registrations.$key.ts (1)
7-7: Cleaner import path organizationImporting schemas from a more specific path (
@trigger.dev/core/schemas) instead of the root package improves code organization and makes the codebase more maintainable by clearly indicating where specific components come from.internal-packages/redis-worker/src/index.ts (1)
1-2: Adding explicit file extensions for ESM compatibilityAdding
.jsextensions to import paths is necessary for ECMAScript modules (ESM) compatibility. This future-proofs the codebase and ensures consistent module resolution across different environments.apps/webapp/app/services/triggers/registerWebhook.server.ts (1)
9-9: Import path updated for server-specific implementationThe import path has been updated to specifically reference the server implementation, which is a good practice in Remix applications as it creates a clearer distinction between server and client code.
apps/webapp/app/routes/engine.v1.dev.runs.$runFriendlyId.snapshots.$snapshotFriendlyId.heartbeat.ts (1)
2-3: Imports refactored to use isomorphic moduleGood architectural change moving
RunIdandSnapshotIdimports from@trigger.dev/core/v3/appsto@trigger.dev/core/v3/isomorphic. This better reflects their isomorphic nature (usable in both client and server contexts) and aligns with the broader import path standardization in this PR.apps/webapp/app/v3/runEngine.server.ts (1)
46-56: Enhanced queue selection strategy configurationThe addition of
queueSelectionStrategyOptionswith multiple configurable parameters improves the flexibility of the run engine. Each parameter is appropriately sourced from environment variables, allowing for runtime configuration without code changes.The strategy includes several key components:
- Parent queue limits
- Biasing factors for queue selection
- Snapshot reuse configuration
- Environment count limitations
- Tracing integration
This configuration will enable more sophisticated queue selection algorithms.
internal-packages/redis-worker/tsconfig.src.json (1)
1-19: Well-structured TypeScript configuration for redis-workerThis new configuration file is appropriately set up with:
- Proper source inclusion/exclusion patterns
- ES2019 target with appropriate lib references
- Node16 module configuration
- Strict type checking enabled
- Standard TypeScript best practices for module resolution
The separation into a src-specific config suggests better organization of the build process for this package.
apps/webapp/app/services/worker.server.ts (1)
28-28: Update to service import path looks goodThe import path for
PerformEndpointIndexServicehas been updated to include the.serversuffix, maintaining consistency with other server-side imports in the codebase.internal-packages/run-engine/src/engine/tests/trigger.test.ts (4)
7-7: Approved: Updated tracing importChanged import source for
tracefrom@opentelemetry/apito@internal/tracing, which aligns with the codebase's move to use an internal tracing package.
12-13: Standardized test timeout configurationGood addition of a global test timeout setting of 60 seconds. This replaces the individual timeout parameters previously set on each test case and provides consistency across all tests.
15-15: Removed explicit timeout parameterThe explicit timeout parameter has been removed from this test case, now relying on the global timeout setting defined earlier. This simplifies the test configuration.
209-209: Removed explicit timeout parameterSimilar to the previous test case, the explicit timeout parameter has been removed, now using the global timeout setting.
internal-packages/run-engine/src/engine/tests/ttl.test.ts (3)
7-7: Approved: Updated tracing importChanged import source for
tracefrom@opentelemetry/apito@internal/tracing, consistent with other test files in this PR.
13-14: Standardized test timeout configurationAdded global test timeout setting of 60 seconds, providing consistent timeout behavior across tests.
16-16: Removed explicit timeout parameterRemoved the explicit timeout parameter from the test case in favor of the global timeout setting.
internal-packages/run-engine/src/engine/tests/notDeployed.test.ts (3)
7-7: Approved: Updated tracing importUpdated the import source for
traceto use the internal tracing package, consistent with other file changes in this PR.
12-13: Standardized test timeout configurationAdded a global test timeout setting, providing consistency across all test files.
15-15: Removed explicit timeout parameterRemoved the explicit timeout parameter from the test case, now using the global timeout configuration.
internal-packages/redis-worker/src/worker.ts (2)
1-1: Centralized tracing implementationThis change moves from a local telemetry module to a shared internal tracing package, which is a good practice for standardizing observability across the codebase.
5-5: Using internal Redis abstractionSwitching from direct ioredis dependency to @internal/redis provides better encapsulation and allows for centralized Redis configuration management.
internal-packages/redis-worker/tsconfig.test.json (1)
1-20: Good separation of test configurationThe addition of a dedicated test configuration file follows best practices for TypeScript projects. The configuration correctly includes:
- Test file pattern targeting
- Reference to source configuration
- Appropriate compiler options for testing environment
- Vitest types integration
- Proper module resolution for Node.js
This separation allows for test-specific settings without affecting the main build configuration.
apps/webapp/tsconfig.json (2)
21-24: Simplified path mappingsRemoving multiple specific path mappings and leaving only the essential ones simplifies the module resolution strategy, making imports more consistent and maintainable.
25-26:❓ Verification inconclusive
New compiler options
The addition of
noEmit: trueand custom conditions for module resolution indicates a shift in how modules are processed and resolved in the application.Verify that all dependent packages and build systems are compatible with these new compiler options.
🏁 Script executed:
#!/bin/bash # Check if any build scripts rely on TypeScript emitting JavaScript files find . -type f -name "*.json" -not -path "*/node_modules/*" | xargs grep -l "tsc" | xargs grep -l "build"Length of output: 946
Ensure Dependent Build Scripts Are Compatible with the New TS Options
The changes in
apps/webapp/tsconfig.json(adding"noEmit": trueand custom module resolution conditions) mean that TypeScript will no longer output JavaScript files during compilation. Our repository-wide search ran a script that found multiple packages (e.g., in./packages/trigger-sdk/package.json,./packages/rsc/package.json,./apps/webapp/package.json, among others) with build scripts referencingtscin a build context. Please verify that:
- None of these build systems assume that TypeScript will emit files.
- If any package still requires emitted JavaScript from
tsc, consider providing a separate configuration (or adjusting the build process) for those scenarios.- The new custom conditions (e.g.,
"@triggerdotdev/source") align with how modules are resolved across packages.Review all affected packages and update their build configurations as needed.
internal-packages/run-engine/src/engine/tests/priority.test.ts (4)
6-7: Updated import pathsImport paths have been updated to align with the new module structure:
- Using
@internal/tracinginstead of direct OpenTelemetry imports- Using
isomorphicinstead ofappsnamespaceThis change maintains consistency with the broader refactoring effort.
14-14: Extended test timeoutSetting a longer test timeout is appropriate for integration tests that involve container setup and multiple queue operations.
17-48: Added machine configuration to RunEngineThe RunEngine initialization now includes detailed machine specifications and cost calculations. This is an important addition that enables environment-based queue selection algorithms.
The machine configuration provides:
- CPU and memory specifications
- Cost calculations (centsPerMs)
- Base cost in cents
This change aligns with the PR objective of implementing environment-based queue selection.
50-101: Improved test structure with proper cleanupThe test has been structured with a try-finally block to ensure that the engine is properly shut down, which is a good practice for resource cleanup in tests involving external services like Redis.
internal-packages/run-engine/src/engine/tests/delays.test.ts (3)
7-7: Updated import for tracingThe import has been standardized to use the internal tracing module instead of directly depending on OpenTelemetry.
12-13: Improved test configurationSetting a global test timeout centralizes timeout management, making the tests cleaner by removing individual timeout specifications across test cases.
97-189: Good addition of reschedule test coverageThis new test case properly verifies that rescheduling a delayed run works as expected, confirming that:
- The run stays in "RUN_CREATED" state after rescheduling
- It doesn't queue when the initial delay passes
- It only queues after the updated delay time passes
The test increases coverage of an important edge case in run scheduling.
internal-packages/run-engine/src/engine/tests/cancelling.test.ts (3)
7-7: Updated import for tracingThe import has been standardized to use the internal tracing module instead of directly depending on OpenTelemetry.
13-14: Improved test configurationSetting a global test timeout centralizes timeout management, making the tests cleaner by removing individual timeout specifications.
232-330: Clean test restructuringThe test case has been improved by removing the unnecessary timeout parameter and maintaining consistent indentation, making the code cleaner while preserving the same functionality.
internal-packages/run-engine/src/engine/tests/heartbeats.test.ts (3)
7-7: Updated import for tracingThe import has been standardized to use the internal tracing module instead of directly depending on OpenTelemetry.
12-13: Improved test configurationSetting a global test timeout centralizes timeout management, making the tests cleaner by removing individual timeout specifications.
15-15: Simplified test declarationsRemoved individual timeout parameters from multiple test cases, relying on the global timeout configuration. This creates a more consistent pattern across all tests.
Also applies to: 129-129, 385-385, 487-487
internal-packages/run-engine/src/engine/tests/dequeuing.test.ts (3)
6-7: Updated imports for package restructuringThe imports have been updated to reflect the new package structure:
- Tracing now uses the internal module
generateFriendlyIdis now imported from the isomorphic module rather than appsThese changes align with the package restructuring mentioned in the PR description.
14-15: Improved test configurationSetting a global test timeout centralizes timeout management, making the tests cleaner by removing individual timeout specifications.
82-169: Clean test restructuringThe "Dequeues runs within machine constraints" test has been improved by removing the unnecessary timeout parameter and maintaining consistent indentation. The functionality remains the same while making the code more readable.
internal-packages/run-engine/src/engine/tests/batchTrigger.test.ts (6)
6-7: Import statements updated to internal modules
These import paths from@internal/tracingand@trigger.dev/core/v3/isomorphiclook correct and align with the broader refactoring in this PR.
12-12: Extended test timeout
Settingvi.setConfig({ testTimeout: 60_000 })ensures longer-running tests have sufficient time to complete, which is fine given the setup and teardown steps in these tests.
19-46: RunEngine configuration verified
Creating theRunEngineinstance withprisma, redis configurations, and machine settings looks consistent. Ensure that the testing environment is well isolated so these concurrency and pricing values won’t inadvertently impact other tests. Overall, this setup is solid.
87-106: Second run trigger reuse
Similar approach to run #1. The parallel creation ensures the engine can handle multiple runs under the same batch. Solid coverage.
137-147: Completing run attempts
Completing runs withengine.completeRunAttemptis tested thoroughly. You might also want to ensure a test path for erroneous run completions (e.g., partial results or exceptions).
177-179: Cleanup and teardown
Usingfinallyto callengine.quit()is a good practice to avoid test leaks. No issues here.internal-packages/redis-worker/tsconfig.json (1)
2-7: Simplified TypeScript project references
The addition of"references"fortsconfig.src.jsonandtsconfig.test.jsonplus the new Node16moduleandmoduleResolutionis consistent with the rest of the monorepo changes. ThecustomConditionsarray is also a useful addition for specialized builds. No immediate issues.apps/webapp/app/env.server.ts (1)
422-428: New environment variables for Run Engine
These new fields (RUN_ENGINE_PARENT_QUEUE_LIMIT,RUN_ENGINE_CONCURRENCY_LIMIT_BIAS, etc.) look well-defined and maintain consistent data types. Verify that any accompanying config in the Run Engine logic properly handles optional fields (likeRUN_ENGINE_MAXIMUM_ENV_COUNT).internal-packages/run-engine/src/engine/tests/batchTriggerAndWait.test.ts (3)
7-13: Internal tracing import and augmented test timeout
Switching to@internal/tracingand setting a 60-second global test timeout helps standardize test behavior. No concerns here.
19-47: RunEngine config
InitializingRunEnginewith broader concurrency and queue settings is consistent with the rest of the PR. ThedefaultMachineandbaseCostInCentsfields align with environment-based tests. Looks good.
296-350: Completing second child run
Similar logic to the first child run. Verifying the waitpoints and final parent state is a strong demonstration of the batch-and-wait logic.internal-packages/run-engine/src/engine/tests/triggerAndWait.test.ts (3)
7-7: Good update to use internal tracing packageSwitching from direct OpenTelemetry API dependency to internal tracing package improves maintainability and abstraction.
12-13: Improved test timeout configurationSetting the test timeout globally is a good practice, especially for integration tests that interact with containers and Redis.
15-15: Removed redundant timeout parameterGood cleanup by removing the local timeout parameter in favor of using the global timeout configuration.
internal-packages/redis-worker/package.json (5)
5-6: Enhanced package entrypoint configurationGood update to point main and types fields to compiled output files which follows Node.js package best practices.
8-15: Added package exports configurationThe exports field properly defines module resolution paths including source files for development and compiled files for consumers. This supports both TypeScript source references and JavaScript imports.
17-17: Updated dependency from OpenTelemetry to internal tracingThis change aligns with other files that now use the internal tracing package.
28-29: Updated dev dependenciesAdded rimraf for clean directory removal and updated vitest dependency.
32-36: Added build script workflowGood enhancement to the build process with clean, typecheck, test, build, and dev scripts.
internal-packages/run-engine/src/engine/locking.ts (3)
4-5: Added imports for Redis and redlock namespaceGood enhancement to import Redis from the internal package and import redlock as a namespace for more precise typing.
9-9: Updated types for Redlock integrationGood updates to use proper namespace-qualified types for Redlock interfaces. This ensures type safety when working with the Redlock library.
Also applies to: 13-13, 31-31
1-2:❓ Verification inconclusive
Switched import style for Redlock
Changed from ES import to CommonJS require style. This might be to address compatibility issues with the Redlock library.
🏁 Script executed:
#!/bin/bash # Check if the Redlock library is ESM compatible npm view redlock type # Look for any issues related to ESM in the project grep -r "redlock" --include="*.md" --include="*.ts" --include="*.js" .Length of output: 1021
Attention: Verify and Consolidate Redlock Import Strategy
The code currently switches the import style for Redlock—from an ES import (now commented out) to a CommonJS require—which appears to be a workaround for compatibility issues. Our verification confirms that the Redlock package is published as an ESM module (
npm view redlock typereturnsmodule). Additionally, we found that the file also uses an ES import (import * as redlock from "redlock") further down, which creates mixed import styles within the same module.Please review the following points:
- Confirm whether the dual import styles (using
requirefor default export extraction and an ES module import for namespace/type usage) are intentional.- If not, consider unifying the import approach for clarity and consistency—preferably using the ES module syntax given the library’s ESM nature.
- Ensure that any necessary runtime or transpiler configurations needed to support the chosen import style are properly documented.
internal-packages/redis-worker/src/queue.ts (2)
1-7: Enhanced Redis importsThe expanded imports from @internal/redis improve type safety by explicitly importing needed types.
444-444: Updated module declarationChanged module declaration from "ioredis" to "@internal/redis", which aligns with the move to use the internal Redis package instead of direct ioredis dependency.
This change is part of a larger effort to encapsulate Redis functionality within an internal package, which improves maintainability and dependency management.
internal-packages/run-engine/package.json (4)
5-15: Package configuration modernized to use ECMAScript modulesThe package has been updated to modern JavaScript module standards by:
- Setting output files to the dist directory
- Adding
"type": "module"to indicate ESM support- Adding a comprehensive exports field for better module resolution
These changes align with modern JavaScript tooling and improve compatibility across different environments.
19-27: Dependencies updated to improve tracing and add caching capabilitiesThe package now:
- Removes direct OpenTelemetry dependencies in favor of the internal tracing package
- Adds
@unkey/cacheandseedrandomfor improved caching and deterministic randomness- Specifies a specific version of zod (3.23.8)
These changes suggest improvements to tracing and the addition of caching capabilities.
31-33: Dev dependencies updated to support new build processesAdded necessary type definitions for seedrandom and rimraf for clean directory removal, which supports the new build process.
36-40: Build scripts enhanced for better developer experienceAdded comprehensive build scripts including:
- Clean script using rimraf to remove previous builds
- Build process with proper TypeScript configuration
- Watch mode for active development
- Enhanced test configuration with
--no-file-parallelismflagThese improvements create a more robust development workflow.
internal-packages/run-engine/src/engine/index.ts (4)
1-3: Import statements updated to use internal tracing moduleReplaced direct OpenTelemetry imports with the internal tracing module. This provides more consistent tracing throughout the application and centralizes tracing configuration.
28-38: Imports updated to use isomorphic module pathThe imports for various IDs and utility functions have been updated to use the
@trigger.dev/core/v3/isomorphicpath rather than the previousappspath. This change aligns with the broader refactoring of module structures mentioned in the PR summary.
51-74: Import statements updated with .js extensionsImports have been updated to include .js extensions, improving ESM compatibility. The imports also reflect a more organized approach to file referencing.
156-167: RunQueue configuration updated to use fair queue selection strategyThe implementation of RunQueue has been updated to:
- Create a new instance of
RunQueueFullKeyProducerfor consistent key generation- Replace the separate queue priority strategies with a unified
FairQueueSelectionStrategy- Configure Redis and concurrency limits consistently
This change appears to be the core of the PR, implementing the environment-based queue selection algorithm mentioned in the PR title.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/cli-v3/e2e/utils.ts (1)
366-371: Adding machine configuration for queue selection algorithmThis addition of the
machineconfiguration object aligns with the PR's focus on environment-based queue selection. The machine specifications (CPU, memory, and cost) will likely be used by the queue selection algorithm to determine appropriate resource allocation.Consider making these machine configuration values configurable through parameters or environment variables rather than hardcoding them. This would facilitate testing different machine tiers and configurations, which could be valuable for comprehensive testing of the queue selection algorithm.
- machine: { - name: "small-1x", - cpu: 1, - memory: 256, - centsPerMs: 0.0000001, - }, + machine: options.machine ?? { + name: "small-1x", + cpu: 1, + memory: 256, + centsPerMs: 0.0000001, + },package.json (2)
18-18: Removal of--parallelflag in thedevscript.
The removal of the--parallelflag means that your development tasks will now run sequentially rather than in parallel. Please ensure this change is intentional and aligns with the new environment-based queue selection algorithm and overall task orchestration strategy.
19-19: Removal of--parallelflag in thei:devscript.
Similar to thedevscript update, this change removes the--parallelflag from the Infisical run command. Verify that the sequential execution of tasks does not adversely affect any process that might have assumed parallelism, especially given the new environment-based queue selection approach.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
internal-packages/run-engine/src/engine/tests/attemptFailures.test.ts(1 hunks)internal-packages/run-engine/src/engine/tests/heartbeats.test.ts(3 hunks)internal-packages/run-engine/src/engine/tests/notDeployed.test.ts(1 hunks)internal-packages/run-engine/src/engine/tests/priority.test.ts(1 hunks)package.json(1 hunks)packages/cli-v3/e2e/utils.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- internal-packages/run-engine/src/engine/tests/attemptFailures.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- internal-packages/run-engine/src/engine/tests/heartbeats.test.ts
- internal-packages/run-engine/src/engine/tests/priority.test.ts
- internal-packages/run-engine/src/engine/tests/notDeployed.test.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
Summary by CodeRabbit
RunEngineclass configuration, introducing new machine specifications for test case execution.These behind-the-scenes improvements help ensure a more robust and responsive experience for end-users without altering app functionality.