-
-
Notifications
You must be signed in to change notification settings - Fork 708
re2: Queue indexing, queue trigger changes #1808
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
WalkthroughThis PR introduces a new task run status, "PENDING_VERSION," and updates associated logic throughout the codebase. Changes affect task run status representations, UI components, database schema migrations, queue and resource management, and testing setups. Several components now handle "PENDING_VERSION" consistently, while deprecated usage of the "exportName" field is removed or replaced with fallback values. Renaming efforts also shift terminology from task- to resource-centric patterns in both the core API and CLI packages, with updates to logging, error handling, and configuration across modules. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UIComponent
participant Presenter
participant Database
Client->>UIComponent: Render task run status view
UIComponent->>Database: Query run status (e.g., PENDING_VERSION)
Database-->>UIComponent: Return run status data
UIComponent->>Presenter: Invoke status mapping & helper functions
Presenter-->>UIComponent: Provide updated info and callout (if pending)
UIComponent-->>Client: Display "Pending Version" state with fallback values
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
packages/core/src/v3/schemas/schemas.ts (1)
130-174
: 💡 Verification agent🧩 Analysis chain
Renamed
QueueOptions
toQueueManifest
and madename
requiredThe schema has been renamed from
QueueOptions
toQueueManifest
, reflecting a shift to a more declarative resource model. Thename
property is now required, which is a breaking change but ensures proper identification of queues.A new
releaseConcurrencyOnWaitpoint
property has been added, providing additional control over concurrency behavior when tasks reach a wait point.
🏁 Script executed:
#!/bin/bash # Search for any remaining references to QueueOptions that might need to be updated rg "QueueOptions" --type ts -g "!**/node_modules/**" -g "!**/dist/**"Length of output: 2633
Action Required: Update Remaining References to QueueOptions
The changes in
packages/core/src/v3/schemas/schemas.ts
correctly rename the schema fromQueueOptions
toQueueManifest
, withname
now required and a newreleaseConcurrencyOnWaitpoint
property added. However, the grep results indicate that several other parts of the codebase still referenceQueueOptions
. Please verify that these references are updated accordingly—or, if they are intentionally left behind, confirm they remain compatible with the new schema changes.
- Files to review:
internal-packages/run-engine/src/engine/types.ts
packages/trigger-sdk/src/v3/shared.ts
packages/core/src/v3/types/queues.ts
apps/webapp/app/v3/services/triggerTaskV1.server.ts
and othersEnsure that these consumers of the old type, or any re-exports, are aligned with the breaking changes introduced in
QueueManifest
.
🧹 Nitpick comments (9)
internal-packages/run-engine/src/engine/systems/pendingVersionSystem.ts (1)
87-87
: Individual updates in a loop can be costly at scale.Each run is wrapped in a separate transaction. If you have hundreds of runs, consider batching to reduce overhead. However, if concurrency or partial failures must be handled distinctly, leaving it as is makes sense.
- for (const run of pendingRuns) { - await this.$.prisma.$transaction(async (tx) => { - ... - }); - } +// Potential approach: group updates or run them in parallel +// if partial failures are not a concerninternal-packages/run-engine/src/engine/index.ts (1)
63-65
: Exposingprisma
andrunQueue
as public properties.This might be required for testing or setup. Be cautious about direct data layer access from external modules, as it can couple them to internal implementation details.
internal-packages/run-engine/src/engine/tests/setup.ts (1)
274-293
:calculateNextBuildVersion
works well for YYYYMMDD-based builds with incremental counters.Be cautious of unusual versions lacking a dot separator, as
split(".")
could yield an undefined build number. Consider a safe fallback or validation for unexpected formats.- const [date, buildNumber] = latestVersion.split("."); + const parts = latestVersion.split("."); + if (parts.length < 2) { + // fallback logic + }references/hello-world/src/trigger/priority.ts (1)
6-6
: Minor text change in log messageThe log message has been changed from "Hello, world from the parent" to "Hello, world from the parents" - a small pluralization update. While this change is minor, ensure it aligns with the actual behavior of the code (is it actually handling multiple parents?).
Consider using more precise logging terminology that matches the actual functionality. If this is meant to be a single parent task, revert to singular, or clarify the comment if it's handling multiple parent contexts.
references/hello-world/src/trigger/pendingVersions.ts (1)
8-29
: Task implementation appears correct, but consider adding types for payload.The pendingVersionsTester implementation triggers two downstream tasks correctly, with appropriate payloads. However, using
any
for the payload type reduces type safety.Consider adding a specific type for the payload:
- run: async (payload: any, { ctx }) => { + run: async (payload: Record<string, unknown>, { ctx }) => {packages/cli-v3/src/indexing/registerResources.ts (1)
33-33
: Unnecessary continue statementThe continue statement at the end of the error handling block is unnecessary since it's the last statement in the loop body.
- continue;
🧰 Tools
🪛 Biome (1.9.4)
[error] 33-33: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
packages/core/src/v3/resource-catalog/standardResourceCatalog.ts (2)
26-28
:registerQueueMetadata
method addition.
Storing queue manifests by theirname
is logical, but be mindful of potential name collisions or overwrites. A possible improvement is to enforce uniqueness or detect duplicates more explicitly.
40-42
: Early return if_currentFileContext
is absent.
Prevents inadvertent metadata registration when no context is set. Possibly add logging or warnings if this occurs unexpectedly.apps/webapp/app/v3/services/createBackgroundWorker.server.ts (1)
233-313
:createWorkerTask
with fallback on a virtual queue.
Dynamically creating a queue if none is provided is a flexible approach. Consider logging or notifying if the queue is auto-created to ensure ops teams remain aware of new queues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (95)
apps/webapp/app/components/runs/v3/TaskRunStatus.tsx
(8 hunks)apps/webapp/app/components/runs/v3/TaskRunsTable.tsx
(1 hunks)apps/webapp/app/database-types.ts
(1 hunks)apps/webapp/app/hooks/useFilterTasks.ts
(0 hunks)apps/webapp/app/models/runtimeEnvironment.server.ts
(1 hunks)apps/webapp/app/models/taskQueue.server.ts
(2 hunks)apps/webapp/app/models/taskRun.server.ts
(1 hunks)apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts
(2 hunks)apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts
(1 hunks)apps/webapp/app/presenters/v3/QueueListPresenter.server.ts
(1 hunks)apps/webapp/app/presenters/v3/RunListPresenter.server.ts
(2 hunks)apps/webapp/app/presenters/v3/SpanPresenter.server.ts
(3 hunks)apps/webapp/app/presenters/v3/TestTaskPresenter.server.ts
(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam._index/route.tsx
(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments.$deploymentParam/route.tsx
(0 hunks)apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
(2 hunks)apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts
(1 hunks)apps/webapp/app/v3/services/alerts/deliverAlert.server.ts
(1 hunks)apps/webapp/app/v3/services/createBackgroundWorker.server.ts
(2 hunks)apps/webapp/app/v3/services/createDeployedBackgroundWorker.server.ts
(2 hunks)apps/webapp/app/v3/services/createDeploymentBackgroundWorker.server.ts
(3 hunks)apps/webapp/app/v3/services/createTaskRunAttempt.server.ts
(1 hunks)apps/webapp/app/v3/services/pauseEnvironment.server.ts
(1 hunks)apps/webapp/app/v3/services/triggerTaskV1.server.ts
(1 hunks)apps/webapp/app/v3/services/triggerTaskV2.server.ts
(4 hunks)apps/webapp/app/v3/taskRunHeartbeatFailed.server.ts
(1 hunks)apps/webapp/app/v3/taskStatus.ts
(3 hunks)docs/v3-openapi.yaml
(3 hunks)internal-packages/database/prisma/migrations/20250319222954_add_task_queue_indexing_columns_for_v2/migration.sql
(1 hunks)internal-packages/database/prisma/migrations/20250320085824_add_queue_to_background_worker_task/migration.sql
(1 hunks)internal-packages/database/prisma/migrations/20250320111737_add_pending_version_task_run_status/migration.sql
(1 hunks)internal-packages/database/prisma/migrations/20250320130354_add_many_to_many_relationship_task_queue_background_worker/migration.sql
(1 hunks)internal-packages/database/prisma/migrations/20250320223742_add_status_reason_to_task_run/migration.sql
(1 hunks)internal-packages/database/prisma/schema.prisma
(8 hunks)internal-packages/run-engine/src/engine/db/worker.ts
(11 hunks)internal-packages/run-engine/src/engine/index.ts
(6 hunks)internal-packages/run-engine/src/engine/systems/dequeueSystem.ts
(7 hunks)internal-packages/run-engine/src/engine/systems/pendingVersionSystem.ts
(4 hunks)internal-packages/run-engine/src/engine/systems/runAttemptSystem.ts
(1 hunks)internal-packages/run-engine/src/engine/tests/attemptFailures.test.ts
(13 hunks)internal-packages/run-engine/src/engine/tests/batchTrigger.test.ts
(4 hunks)internal-packages/run-engine/src/engine/tests/batchTriggerAndWait.test.ts
(5 hunks)internal-packages/run-engine/src/engine/tests/cancelling.test.ts
(6 hunks)internal-packages/run-engine/src/engine/tests/checkpoints.test.ts
(14 hunks)internal-packages/run-engine/src/engine/tests/delays.test.ts
(7 hunks)internal-packages/run-engine/src/engine/tests/dequeuing.test.ts
(4 hunks)internal-packages/run-engine/src/engine/tests/heartbeats.test.ts
(11 hunks)internal-packages/run-engine/src/engine/tests/notDeployed.test.ts
(0 hunks)internal-packages/run-engine/src/engine/tests/pendingVersion.test.ts
(1 hunks)internal-packages/run-engine/src/engine/tests/priority.test.ts
(3 hunks)internal-packages/run-engine/src/engine/tests/releaseConcurrency.test.ts
(13 hunks)internal-packages/run-engine/src/engine/tests/setup.ts
(7 hunks)internal-packages/run-engine/src/engine/tests/trigger.test.ts
(5 hunks)internal-packages/run-engine/src/engine/tests/triggerAndWait.test.ts
(8 hunks)internal-packages/run-engine/src/engine/tests/ttl.test.ts
(3 hunks)internal-packages/run-engine/src/engine/tests/waitpoints.test.ts
(20 hunks)internal-packages/run-engine/src/engine/types.ts
(1 hunks)internal-packages/run-engine/src/engine/workerCatalog.ts
(1 hunks)internal-packages/run-engine/src/index.ts
(1 hunks)internal-packages/run-engine/tsconfig.src.json
(1 hunks)internal-packages/run-engine/tsconfig.test.json
(1 hunks)internal-packages/testcontainers/src/index.ts
(1 hunks)packages/cli-v3/src/dev/devSupervisor.ts
(3 hunks)packages/cli-v3/src/entryPoints/dev-index-worker.ts
(5 hunks)packages/cli-v3/src/entryPoints/dev-run-worker.ts
(6 hunks)packages/cli-v3/src/entryPoints/managed-index-controller.ts
(1 hunks)packages/cli-v3/src/entryPoints/managed-index-worker.ts
(5 hunks)packages/cli-v3/src/entryPoints/managed-run-worker.ts
(6 hunks)packages/cli-v3/src/indexing/registerResources.ts
(1 hunks)packages/cli-v3/src/indexing/registerTasks.ts
(0 hunks)packages/core/src/v3/apiClient/runStream.ts
(1 hunks)packages/core/src/v3/index.ts
(1 hunks)packages/core/src/v3/resource-catalog-api.ts
(1 hunks)packages/core/src/v3/resource-catalog/catalog.ts
(1 hunks)packages/core/src/v3/resource-catalog/index.ts
(1 hunks)packages/core/src/v3/resource-catalog/noopResourceCatalog.ts
(2 hunks)packages/core/src/v3/resource-catalog/standardResourceCatalog.ts
(2 hunks)packages/core/src/v3/schemas/api.ts
(4 hunks)packages/core/src/v3/schemas/build.ts
(2 hunks)packages/core/src/v3/schemas/common.ts
(1 hunks)packages/core/src/v3/schemas/resources.ts
(2 hunks)packages/core/src/v3/schemas/runEngine.ts
(1 hunks)packages/core/src/v3/schemas/schemas.ts
(4 hunks)packages/core/src/v3/task-catalog/index.ts
(0 hunks)packages/core/src/v3/tryCatch.ts
(1 hunks)packages/core/src/v3/types/index.ts
(1 hunks)packages/core/src/v3/types/queues.ts
(1 hunks)packages/core/src/v3/types/tasks.ts
(3 hunks)packages/core/src/v3/utils/globals.ts
(2 hunks)packages/core/src/v3/workers/index.ts
(1 hunks)packages/trigger-sdk/src/v3/schedules/index.ts
(2 hunks)packages/trigger-sdk/src/v3/shared.ts
(30 hunks)references/hello-world/src/trigger/pendingVersions.ts
(1 hunks)references/hello-world/src/trigger/priority.ts
(1 hunks)references/hello-world/src/trigger/queues.ts
(2 hunks)
💤 Files with no reviewable changes (5)
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments.$deploymentParam/route.tsx
- packages/core/src/v3/task-catalog/index.ts
- apps/webapp/app/hooks/useFilterTasks.ts
- packages/cli-v3/src/indexing/registerTasks.ts
- internal-packages/run-engine/src/engine/tests/notDeployed.test.ts
🧰 Additional context used
🧬 Code Definitions (24)
apps/webapp/app/models/taskRun.server.ts (2)
apps/webapp/app/database-types.ts (1) (1)
TaskRunStatus
(30-47)packages/core/src/v3/schemas/runEngine.ts (2) (2)
TaskRunStatus
(21-38)TaskRunStatus
(40-40)
apps/webapp/app/v3/services/triggerTaskV1.server.ts (1)
packages/core/src/v3/types/queues.ts (1) (1)
QueueOptions
(1-43)
apps/webapp/app/presenters/v3/RunListPresenter.server.ts (1)
apps/webapp/app/v3/taskStatus.ts (1) (1)
isPendingRunStatus
(103-105)
packages/core/src/v3/tryCatch.ts (2)
apps/webapp/app/v3/services/alerts/deliverAlert.server.ts (1) (1)
T
(910-949)apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts (1) (1)
T
(1491-1523)
packages/core/src/v3/schemas/build.ts (1)
packages/core/src/v3/schemas/schemas.ts (2) (2)
QueueManifest
(130-172)QueueManifest
(174-174)
apps/webapp/app/components/runs/v3/TaskRunStatus.tsx (2)
apps/webapp/app/database-types.ts (1) (1)
TaskRunStatus
(30-47)packages/core/src/v3/schemas/runEngine.ts (2) (2)
TaskRunStatus
(21-38)TaskRunStatus
(40-40)
packages/core/src/v3/utils/globals.ts (2)
packages/core/src/v3/resource-catalog/catalog.ts (1) (1)
ResourceCatalog
(4-16)packages/core/src/v3/resource-catalog/index.ts (1) (1)
ResourceCatalog
(76-78)
packages/core/src/v3/resource-catalog-api.ts (1)
packages/core/src/v3/resource-catalog/index.ts (1) (1)
ResourceCatalogAPI
(11-79)
packages/cli-v3/src/indexing/registerResources.ts (2)
packages/core/src/v3/schemas/build.ts (4) (4)
BuildManifest
(20-68)BuildManifest
(70-70)ImportTaskFileErrors
(118-118)ImportTaskFileErrors
(120-120)packages/core/src/v3/resource-catalog-api.ts (1) (1)
resourceCatalog
(5-5)
packages/trigger-sdk/src/v3/schedules/index.ts (1)
packages/core/src/v3/resource-catalog-api.ts (1) (1)
resourceCatalog
(5-5)
internal-packages/run-engine/src/engine/tests/releaseConcurrency.test.ts (2)
internal-packages/run-engine/src/engine/tests/setup.ts (1) (1)
setupBackgroundWorker
(68-272)apps/webapp/app/v3/runEngine.server.ts (1) (1)
engine
(9-9)
apps/webapp/app/v3/taskStatus.ts (2)
apps/webapp/app/database-types.ts (1) (1)
TaskRunStatus
(30-47)packages/core/src/v3/schemas/runEngine.ts (2) (2)
TaskRunStatus
(21-38)TaskRunStatus
(40-40)
packages/core/src/v3/resource-catalog/catalog.ts (5)
packages/core/src/v3/resource-catalog/index.ts (1) (1)
ResourceCatalog
(76-78)packages/core/src/v3/types/tasks.ts (1) (1)
TaskMetadataWithFunctions
(854-870)packages/core/src/v3/schemas/schemas.ts (4) (4)
TaskManifest
(213-216)TaskManifest
(218-218)QueueManifest
(130-172)QueueManifest
(174-174)packages/core/src/v3/schemas/build.ts (2) (2)
WorkerManifest
(81-96)WorkerManifest
(98-98)packages/trigger-sdk/src/v3/shared.ts (1) (1)
queue
(110-118)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx (1)
apps/webapp/app/components/runs/v3/TaskRunStatus.tsx (1) (1)
TaskRunStatusCombo
(94-112)
apps/webapp/app/v3/services/createDeploymentBackgroundWorker.server.ts (3)
packages/core/src/v3/tryCatch.ts (1) (1)
tryCatch
(8-15)apps/webapp/app/v3/services/createBackgroundWorker.server.ts (3) (3)
createBackgroundFiles
(578-626)createWorkerResources
(199-211)syncDeclarativeSchedules
(434-576)apps/webapp/app/v3/services/timeoutDeployment.server.ts (1) (1)
TimeoutDeploymentService
(7-69)
packages/core/src/v3/types/tasks.ts (1)
packages/core/src/v3/types/queues.ts (1) (1)
QueueOptions
(1-43)
internal-packages/run-engine/src/engine/tests/checkpoints.test.ts (1)
internal-packages/run-engine/src/engine/tests/setup.ts (1) (1)
setupBackgroundWorker
(68-272)
packages/core/src/v3/resource-catalog/standardResourceCatalog.ts (4)
packages/core/src/v3/resource-catalog/catalog.ts (1) (1)
ResourceCatalog
(4-16)packages/core/src/v3/schemas/schemas.ts (6) (6)
TaskMetadata
(192-192)TaskMetadata
(194-194)TaskFileMetadata
(209-209)TaskFileMetadata
(211-211)QueueManifest
(130-172)QueueManifest
(174-174)packages/core/src/v3/types/tasks.ts (1) (1)
TaskMetadataWithFunctions
(854-870)packages/core/src/v3/schemas/build.ts (2) (2)
WorkerManifest
(81-96)WorkerManifest
(98-98)
packages/core/src/v3/resource-catalog/index.ts (4)
packages/core/src/v3/resource-catalog/noopResourceCatalog.ts (1) (1)
NoopResourceCatalog
(5-53)packages/core/src/v3/resource-catalog/catalog.ts (1) (1)
ResourceCatalog
(4-16)packages/core/src/v3/utils/globals.ts (3) (3)
registerGlobal
(17-33)unregisterGlobal
(41-47)getGlobal
(35-39)packages/core/src/v3/types/tasks.ts (1) (1)
TaskMetadataWithFunctions
(854-870)
internal-packages/run-engine/src/engine/tests/setup.ts (6)
internal-packages/run-engine/src/engine/index.ts (1) (1)
RunEngine
(56-1326)internal-packages/run-engine/src/index.ts (2) (2)
RunEngine
(1-1)AuthenticatedEnvironment
(4-4)apps/webapp/app/v3/services/triggerTaskV2.server.ts (1) (1)
environment
(449-468)packages/core/src/v3/schemas/common.ts (2) (2)
MachineConfig
(109-113)MachineConfig
(115-115)packages/core/src/v3/schemas/schemas.ts (2) (2)
RetryOptions
(99-126)RetryOptions
(128-128)apps/webapp/app/v3/services/createDeploymentBackgroundWorker.server.ts (1) (1)
deployment
(142-160)
internal-packages/run-engine/src/engine/index.ts (1)
internal-packages/run-engine/src/engine/systems/pendingVersionSystem.ts (1) (1)
PendingVersionSystem
(10-121)
apps/webapp/app/v3/services/createBackgroundWorker.server.ts (6)
packages/core/src/v3/tryCatch.ts (1) (1)
tryCatch
(8-15)packages/core/src/v3/schemas/resources.ts (4) (4)
BackgroundWorkerMetadata
(29-36)BackgroundWorkerMetadata
(38-38)TaskResource
(5-16)TaskResource
(18-18)apps/webapp/app/services/apiAuth.server.ts (1) (1)
AuthenticatedEnvironment
(30-33)internal-packages/run-engine/src/engine/tests/setup.ts (1) (1)
AuthenticatedEnvironment
(16-18)packages/core/src/v3/schemas/schemas.ts (2) (2)
QueueManifest
(130-172)QueueManifest
(174-174)apps/webapp/app/models/taskQueue.server.ts (1) (1)
sanitizeQueueName
(61-63)
packages/cli-v3/src/entryPoints/dev-run-worker.ts (2)
packages/core/src/v3/resource-catalog-api.ts (1) (1)
resourceCatalog
(5-5)packages/core/src/v3/resource-catalog/standardResourceCatalog.ts (1) (1)
StandardResourceCatalog
(11-141)
packages/trigger-sdk/src/v3/shared.ts (2)
packages/core/src/v3/types/queues.ts (1) (1)
QueueOptions
(1-43)packages/core/src/v3/resource-catalog-api.ts (1) (1)
resourceCatalog
(5-5)
🪛 Biome (1.9.4)
packages/cli-v3/src/indexing/registerResources.ts
[error] 33-33: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
packages/core/src/v3/resource-catalog/index.ts
[error] 17-17: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 18-18: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 21-21: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
🔇 Additional comments (273)
internal-packages/run-engine/tsconfig.test.json (1)
2-2
:✅ Verification successful
Removed specific test file reference from the include pattern.
The configuration now only includes the general pattern
"src/**/*.test.ts"
and no longer explicitly includes"src/run-queue/tests/dequeueMessageFromMasterQueue.ts"
. This simplifies the configuration, but ensure that this test file either follows the standard naming pattern or has been intentionally removed/relocated as part of the queue-related changes in this PR.
🏁 Script executed:
#!/bin/bash # Check if the specific test file still exists and if it follows the *.test.ts pattern echo "Checking for the dequeueMessageFromMasterQueue test file..." find src -name "dequeueMessageFromMasterQueue*" -type fLength of output: 225
I'll run a new shell script to search the whole repository for any file matching the pattern "dequeueMessageFromMasterQueue*.ts", which will help us verify whether the removed reference is still required.
🏁 Script executed:
#!/bin/bash echo "Searching for any occurrence of the 'dequeueMessageFromMasterQueue' test file in the repository..." find . -type f -name "dequeueMessageFromMasterQueue*.ts"Length of output: 360
Attention: TSConfig Pattern Verification and Test File Naming
The specific test file is still present as
internal-packages/run-engine/src/run-queue/tests/dequeueMessageFromMasterQueue.test.ts
and correctly adheres to the*.test.ts
naming convention. With the updated tsconfig including"src/**/*.test.ts"
, the file is automatically included. No further changes are required.internal-packages/run-engine/tsconfig.src.json (1)
17-18
:❓ Verification inconclusive
Added custom condition for module resolution.
The addition of
"customConditions": ["@triggerdotdev/source"]
to the compiler options enables conditional module resolution, which appears to be related to the resource-centric model transition mentioned in the PR description.Could you clarify how this custom condition is used in the codebase? This helps with understanding the module resolution strategy for the resource-centric model.
🌐 Web query:
What are TypeScript custom conditions used for in module resolution?
💡 Result:
TypeScript custom conditions enhance module resolution by allowing developers to define environment-specific or tool-specific import logic in
package.json
exports
/imports
fields. Here's how they work:Core Functionality
The
customConditions
compiler option (tsconfig.json
) lets you specify additional resolution conditions beyond default Node.js/Bundler ones likeimport
,require
, ornode
[1][4]. These custom values are checked when resolving entries in package.json's conditional exports:// package.json { "exports": { ".": { "my-env": "./special-build.js", "default": "./standard-build.js" } } }With
"customConditions": ["my-env"]
intsconfig.json
, TypeScript resolves tospecial-build.js
instead of the default[1][8].Key Use Cases
Environment-Specific Builds
- Target different implementations for specific runtimes (e.g.,
"react-native"
,"electron"
)[8][11]- Enable experimental features through custom flags like
"with-new-feature"
Toolchain Integration
- Work with bundlers that recognize custom conditions (Webpack, Vite, etc.)[11]
- Enable TypeScript-first monorepo patterns through shared conditions[6]
Resolution Control
- Override default selection logic in dependency packages
- Create alternative development/production resolution paths[1][8]
Requirements
- Only works with
moduleResolution: node16/nodenext/bundler
[1][4]- Conditions are checked in the order they appear in the array[1]
- Node.js equivalents require matching CLI flags (e.g.,
--conditions=my-env
)[6][8]Example Workflow
A monorepo usingcustomConditions: ["workspace"]
could resolve local package versions first before falling back to npm registry versions[6]. This allows seamless integration between local development and published dependencies.Custom conditions bridge TypeScript's type resolution with runtime environment requirements, making them essential for complex project setups and advanced module resolution scenarios.
Citations:
- 1: https://www.typescriptlang.org/tsconfig/customConditions.html
- 2: https://cursa.app/en/page/configuring-tsconfig-module-resolution-strategies
- 3: https://www.typescriptlang.org/docs/handbook/2/modules.html
- 4: https://www.typescriptlang.org/tsconfig/moduleResolution.html
- 5: https://blog.logrocket.com/common-typescript-module-problems-how-to-solve/
- 6: Support
customConditions
intsconfig.json
privatenumber/tsx#574- 7: https://www.typescriptlang.org/docs/handbook/modules/theory.html
- 8: https://bun.sh/docs/runtime/modules
- 9: https://dev.to/katallaxie/module-resolution-in-large-projects-with-typescript-2ak9
- 10: https://www.typescriptlang.org/tsconfig/
- 11: Add support for custom module resolution conditions in tsconfig.json microsoft/TypeScript#53213
Clarification on the use of the
@triggerdotdev/source
custom conditionIn the file
internal-packages/run-engine/tsconfig.src.json
, your changes show:"strict": true, "customConditions": ["@triggerdotdev/source"]Since TypeScript custom conditions are designed to enable environment- or tool-specific module resolution—typically by selecting alternative paths in a package’s conditional exports—it would be helpful to understand how this specific condition is wired into our codebase. For example:
- How does
@triggerdotdev/source
influence module selection in practice?- Is there a corresponding configuration (e.g., in
package.json
or elsewhere) that maps this condition to a particular build or runtime behavior for the resource-centric model?Providing a pointer to where this behavior is implemented or documented in our repository would clarify the overall module resolution strategy.
internal-packages/database/prisma/migrations/20250319222954_add_task_queue_indexing_columns_for_v2/migration.sql (1)
1-10
: Well-structured migration for V2 task queue support.This migration creates a foundation for supporting V2 task queues while maintaining compatibility with existing V1 queues. The addition of
orderableName
suggests improved queue sorting or indexing capabilities, while theversion
column with a default value ensures backward compatibility.The migration follows good practices by:
- Creating a proper enum type for version control
- Using appropriate column types
- Setting sensible defaults to maintain compatibility with existing data
packages/core/src/v3/utils/globals.ts (1)
3-3
:❓ Verification inconclusive
Refactored from task-centric to resource-centric model.
The changes replace
TaskCatalog
withResourceCatalog
and update the global API key from["task-catalog"]
to["resource-catalog"]
. This reflects the architectural shift mentioned in the PR description, transitioning from a task-centric to a resource-centric model.This change may require updates in any code that directly accesses the global task catalog. Let's verify that all references have been updated:
Also applies to: 58-58
🏁 Script executed:
#!/bin/bash # Check for any remaining references to the old task-catalog echo "Checking for remaining task-catalog references..." rg --type-add 'ts:*.{ts,tsx}' --type ts "\[\"task-catalog\"\]" -l rg --type-add 'ts:*.{ts,tsx}' --type ts "TaskCatalog" -l | grep -v "resource-catalog"Length of output: 258
Resource-centric refactor verified – please perform a manual check
The change correctly updates the import to use
ResourceCatalog
and modifies the global API key from["task-catalog"]
to["resource-catalog"]
. The shell script search did not surface any remaining references to the oldTaskCatalog
or["task-catalog"]
string in TypeScript files. However, since the output was minimal and could be low-quality inference, please verify manually (or with additional scripts) that no residual dependencies on the task-centric model remain in the broader codebase.
- Confirm that any direct access to the global task catalog has been fully replaced.
- Ensure that non-TypeScript files or configurations are also updated if applicable.
internal-packages/testcontainers/src/index.ts (2)
4-4
: Import cleanup for RedisThe import of
Redis
class fromioredis
has been properly removed while keeping only the necessaryRedisOptions
type. This reduces unused imports in the file.
10-10
: Export statement reorganizedThe export of
StartedRedisContainer
has been explicitly maintained, ensuring that consumers of this module can still access this container type.packages/core/src/v3/schemas/schemas.ts (2)
184-184
: Queue reference updated with compatibility layerWhile
QueueManifest
now requires a name, this extended schema makesname
optional again for task metadata, which provides backward compatibility for existing code.
205-205
: MadeexportName
optional in task file metadataMaking
exportName
optional allows for more flexibility in how tasks are defined and exported, potentially supporting cases where the export name can be inferred.packages/core/src/v3/resource-catalog/noopResourceCatalog.ts (3)
1-5
: Class renamed and imports updated for resource-centric approachThe class has been renamed from
NoopTaskCatalog
toNoopResourceCatalog
and now implements theResourceCatalog
interface instead ofTaskCatalog
. This reflects a shift from task-centric to resource-centric architecture, allowing for better organization of different resource types.
10-16
: Replaced single registration method with context management methodsThe previous
registerTaskFileMetadata
method has been replaced with two separate methods:setCurrentFileContext
andclearCurrentFileContext
. This provides more granular control over file context management and better separation of concerns.
42-52
: Added resource registration methods for workers and queuesThree new methods have been added to support the resource-centric approach:
registerWorkerManifest
for worker registrationregisterQueueMetadata
for queue registrationlistQueueManifests
for retrieving available queuesThese methods complete the implementation of the
ResourceCatalog
interface and enable proper handling of the new resource types.packages/trigger-sdk/src/v3/shared.ts (6)
21-21
: Updated import to use resourceCatalogImport statement changed from
taskCatalog
toresourceCatalog
, aligning with the broader architectural shift to a resource-centric model.
110-118
: Simplified queue function signature and implementationThe
queue
function has been updated to:
- Remove the requirement for a
name
property in the options parameter- Register queue metadata directly with
resourceCatalog.registerQueueMetadata(options)
This simplifies the API while maintaining the same functionality.
203-220
: Updated task registration to use resourceCatalogTask metadata registration now uses
resourceCatalog
instead oftaskCatalog
. Additionally, a new check has been added to register queue metadata when a queue with a name is provided, ensuring that queues are properly tracked in the resource catalog.Also applies to: 224-229
351-369
: Consistent resource catalog usage in schema tasksSimilar changes for schema tasks, ensuring consistent usage of the resource catalog for both task and queue metadata registration.
Also applies to: 373-378
1259-1262
: Enhanced queue option handling in batch operationsThe code now handles queue options more comprehensively by checking three scenarios:
- Item-specific queue option
- Task-level queue option
- No queue specified
This provides more flexibility and clearer queue assignment logic for batch operations.
1433-1437
: Consistent queue handling in batch trigger and wait operationsThe same enhanced queue option handling has been applied to batch trigger and wait operations, maintaining consistency throughout the API.
internal-packages/run-engine/src/engine/systems/pendingVersionSystem.ts (12)
4-7
: Definition ofPendingVersionSystemOptions
looks fine.The optional property
queueRunsPendingVersionBatchSize
enables flexible batch sizes. This is clearly documented and consistent with the rest of your approach.
10-10
: Class export approach is correct.Exporting this class under the new
PendingVersionSystem
name aligns with the newly introduced status and clarifies its responsibilities.
14-14
: Constructor initialization appears well-structured.Injecting dependencies through
options
is a clean approach that promotes testability and future extensibility.
19-22
: Use of a fallback batch size is good.Providing a default of
200
helps avoid processing an excessively large set in a single pass. This ensures manageable chunking and safer execution.
37-37
: Includingqueues: true
is relevant.This helps gather queue information that is crucial for filtering task runs by queue in subsequent logic.
41-41
: Error logging is helpful here.The logged context (worker ID) provides clarity for debugging, and the function gracefully returns early on
backgroundWorker
not found.
47-51
: Debug logs add valuable context.Printing the worker ID, task identifiers, and queue names is beneficial for diagnosing any filtering issues.
53-63
: Search for PENDING_VERSION runs accurately filters relevant tasks.The additional filters for matching
taskIdentifier
andqueue
ensure that only relevant runs are picked for this background worker.
72-73
: Early return is optimal when no runs match.Skipping further logic when there are no pending runs prevents unnecessary overhead.
74-85
: Debug log provides visibility into the enqueuing operation.Capturing each run’s ID, identifier, and queue details is useful for auditing and diagnosing order issues.
109-110
: Conditional recursion is appropriate.Re-invoking the same logic for the remaining items ensures that all runs get processed in a predictable, incremental manner.
114-120
: Scheduling a job to resolve pending version runs is consistent.Enqueuing the “queueRunsPendingVersion” job for background processing keeps the system modular and prevents blocking other operations.
internal-packages/run-engine/src/engine/index.ts (4)
51-51
: Importing and registering thePendingVersionSystem
.This clearly distinguishes the system handling the new
PENDING_VERSION
status from other systems.
75-77
: Instantiation ofpendingVersionSystem
with the new options object is consistent.This is a reasonable approach to keep the engine’s design modular, giving each subsystem the resources it needs.
150-153
: New job forqueueRunsPendingVersion
.Linking the worker job to
pendingVersionSystem.enqueueRunsForBackgroundWorker
consistently extends the job catalog to handle the new status. Good practice to keep it self-contained.
726-728
: MethodscheduleEnqueueRunsForBackgroundWorker
elegantly delegates topendingVersionSystem
.Keeping engine-level methods minimal while deferring logic to specialized systems promotes clarity.
internal-packages/run-engine/src/engine/tests/setup.ts (10)
14-14
: Refactoring to importRunEngine
.Switching from a raw Prisma client to the
RunEngine
for background worker setup is a cohesive design choice, centralizing logic within the engine.
69-79
: Updated function signature to acceptengine: RunEngine
.Streamlining to use
engine.prisma
instead of passing the Prisma client directly helps maintain a single source of truth and consistent environment usage.
75-75
: Addition ofcustomQueues
inqueueOptions
.This is a neat enhancement. Exposing custom queues as a typed option fosters flexibility for different task distributions.
80-90
: Retrieving the latest worker with descending order.Capturing only the newest record is appropriate for determining the next build version.
92-92
: Calculating the next build version.You store the newly computed version in
nextVersion
which is then assigned to the created worker. This keeps versioning consistent.
115-115
: Creation ofBackgroundWorkerTask
.Using consistent naming for direct references to each task ensures clarity in both the codebase and database tables.
131-170
:taskQueue.upsert
for each queue is effective.Upserting concurrency limits allows you to either create or update existing queues in a single call, keeping queue data in sync with the background worker.
172-181
: Dynamically adjusting concurrency limits.Invoking
updateQueueConcurrencyLimits
orremoveQueueConcurrencyLimits
addresses both the numeric and null cases. This ensures the queue concurrency truly reflects the updated configuration.
183-223
: HandlingcustomQueues
in a similar upsert pattern.Consistent logic for creating custom queues mirrors the earlier approach and prevents duplication.
225-259
: Deploying the worker outside of DEVELOPMENT and scheduling its runs.Creating a deployment record then calling
engine.scheduleEnqueueRunsForBackgroundWorker(worker.id)
integrates the new flow forPENDING_VERSION
tasks smoothly. This ensures the newly added worker can start processing runs in the environment.internal-packages/database/prisma/schema.prisma (7)
1616-1617
: Enhance BackgroundWorker with multiple queue assignmentsThe relationship between BackgroundWorker and TaskQueue has been improved by adding a
queues
field. This is a good enhancement that allows a single worker to process tasks from multiple queues, improving resource utilization and deployment flexibility.
1660-1661
: Making exportName optional promotes decouplingMaking the
exportName
field optional is a positive change. This allows more flexibility in how tasks are referenced and executed, reducing coupling between the database schema and the actual implementation. This will make it easier to refactor task code without database changes in the future.
1684-1686
: Queue association supports better task managementAdding the
queueId
andqueue
relationship to BackgroundWorkerTask provides explicit task-to-queue mapping. This improves the task organization system, allowing for better tracking and management of tasks across different queues.
1710-1712
: Adding statusReason improves observabilityThe addition of the
statusReason
field enhances the task run status model by providing more context about why a task is in a particular state. This is valuable for debugging and monitoring, especially when tasks encounter issues or special conditions.
2544-2546
: TaskQueue versioning provides migration pathAdding version tracking to TaskQueue with the
version
field and nullableorderableName
facilitates a smooth transition between queue implementations. This is good practice for managing system evolution while maintaining backward compatibility.
2565-2567
: Bidirectional relationships improve query capabilitiesThese bidirectional relationships between TaskQueue and both BackgroundWorkerTask and BackgroundWorker make it easier to query related entities in both directions. This improves database access patterns and provides more flexibility when implementing features.
2576-2579
: TaskQueueVersion enum supports evolutionThe new TaskQueueVersion enum with V1 and V2 options indicates a planned evolution of the queuing system. This is a good approach to managing changes while maintaining compatibility with existing implementations.
internal-packages/run-engine/src/index.ts (1)
4-4
: Exposing AuthenticatedEnvironment improves API usabilityExporting the AuthenticatedEnvironment type from the shared index makes it available to consumers of this package. This is a good practice for public APIs as it allows other modules to utilize this type without needing to import it from internal paths.
internal-packages/database/prisma/migrations/20250320223742_add_status_reason_to_task_run/migration.sql (1)
1-5
: Database migration for enhanced task status informationThis migration adds a
statusReason
column to the TaskRun table, which aligns with the schema changes and supports providing more context about task run states. This addition is safe as it's a nullable field that won't affect existing records.The migration is particularly important for supporting the new
PENDING_VERSION
status, where additional context about why a task is pending would be valuable to users.apps/webapp/app/models/taskRun.server.ts (1)
128-128
: The new PENDING_VERSION status is correctly handled.The PENDING_VERSION status is appropriately mapped to BatchTaskRunItemStatus.PENDING, consistent with other pending-like statuses such as PENDING, WAITING_FOR_DEPLOY, etc. This ensures consistent behavior for the new status in batch task runs.
packages/core/src/v3/types/index.ts (1)
9-9
: Good addition of queue exports.Exporting the queue-related types from the index file is a clean approach that will make these types more accessible throughout the codebase. This supports the transition to resource-centric models mentioned in the PR objectives.
internal-packages/database/prisma/migrations/20250320111737_add_pending_version_task_run_status/migration.sql (1)
1-4
: Migration looks correct for adding the new task run status.The SQL migration correctly adds the PENDING_VERSION value to the TaskRunStatus enum. This database change aligns with the TypeScript type definitions seen in other files and ensures consistency between the application code and database schema.
packages/core/src/v3/schemas/common.ts (1)
245-245
: Appropriate schema update making exportName optional.Making the exportName property optional in the TaskRunExecutionTask schema is a good change that provides more flexibility in task definitions. This aligns with the PR's objective of transitioning from a task-centric to a resource-centric model.
apps/webapp/app/presenters/v3/QueueListPresenter.server.ts (1)
59-59
: Filter added to retrieve only V2 queuesThis change adds a version filter to the queue retrieval query, limiting results to only include V2 queues. This aligns with the PR's focus on queue versioning improvements.
internal-packages/run-engine/src/engine/workerCatalog.ts (1)
32-32
: Property renamed to better reflect its purposeRenamed
queueRunsWaitingForWorker
toqueueRunsPendingVersion
to more accurately describe the waiting state. This matches the introduction of the newPENDING_VERSION
status elsewhere in the codebase.apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts (1)
1691-1691
: Added fallback for exportName propertyAdded a nullish coalescing operator to use
backgroundWorkerTask.slug
as a fallback whenexportName
is undefined. This improves robustness and prevents potentialundefined
values from being used.packages/core/src/v3/workers/index.ts (1)
8-8
: Transitioning from task-centric to resource-centric modelReplaced the export of
StandardTaskCatalog
withStandardResourceCatalog
. This change is part of the broader transition from a task-centric to a resource-centric model as mentioned in the PR description.packages/core/src/v3/apiClient/runStream.ts (1)
427-429
: Correctly handled new "PENDING_VERSION" statusThe addition of the "PENDING_VERSION" case in the
apiStatusFromRunStatus
function properly handles the new status string, maintaining the same pattern as other status handlers.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam._index/route.tsx (1)
510-510
: Appropriate visualization for new "PENDING_VERSION" statusThe new bar visualization for "PENDING_VERSION" status uses an amber/orange color (#F59E0B) which intuitively communicates a waiting state. The bar maintains consistent styling with existing status bars (same stack ID, stroke width, and bar size).
apps/webapp/app/v3/services/createTaskRunAttempt.server.ts (1)
195-195
: Added fallback for potentially missing exportNameThe change adds a fallback value "@deprecated" for when
lockedBy.exportName
is null or undefined. This supports the transition from task-centric to resource-centric model mentioned in the PR objectives.apps/webapp/app/v3/services/alerts/deliverAlert.server.ts (1)
576-576
: Consistent handling of potentially missing exportName in webhooksThe addition of the fallback value for
task.exportName
ensures that external systems receiving the webhook payload will still get a value even ifexportName
is no longer present, maintaining backward compatibility during the transition period.apps/webapp/app/database-types.ts (1)
30-32
: New TaskRunStatus added: PENDING_VERSIONGood addition of the new status type. This extends the enumeration of possible task run statuses, providing more granular control over task execution states in the queue processing system.
apps/webapp/app/models/runtimeEnvironment.server.ts (1)
1-1
: Import source updated correctlyThe change from importing from
@internal/testcontainers
to importing from@internal/run-engine
as a type-only import is appropriate. This helps with the reorganization of the codebase to a more resource-centric model as mentioned in the PR objectives.internal-packages/run-engine/src/engine/systems/runAttemptSystem.ts (1)
273-273
: Improved null handling for exportNameGood enhancement using the nullish coalescing operator to explicitly set
exportName
toundefined
when the value is null or undefined. This provides more consistent behavior for downstream consumers of this property.apps/webapp/app/presenters/v3/RunListPresenter.server.ts (2)
5-5
: Added isPendingRunStatus importGood addition of the
isPendingRunStatus
import from the task status utilities, which will be used to determine whether a run is in a pending state.
370-370
: Added isPending property to run objectsNice addition of the
isPending
property to each run object. This allows the UI to easily identify and display pending runs (including the new PENDING_VERSION status) without needing additional logic in the frontend components.packages/core/src/v3/resource-catalog-api.ts (1)
3-5
:✅ Verification successful
Renamed API from task-centric to resource-centric
Good refactoring to rename from
TaskCatalogAPI
toResourceCatalogAPI
and fromtaskCatalog
toresourceCatalog
. This aligns with the broader transition to a resource-centric model throughout the codebase.
🏁 Script executed:
#!/bin/bash # Verify all references to taskCatalog have been updated to resourceCatalog echo "Searching for any remaining references to taskCatalog:" rg "taskCatalog" --type tsLength of output: 145
Refactoring Verified: All outdated task-centric references have been successfully replaced.
- Verified that no instances of
taskCatalog
orTaskCatalogAPI
remain in the codebase.- The updated file (
packages/core/src/v3/resource-catalog-api.ts
) now correctly imports and exposes the instance fromResourceCatalogAPI
.The code snippet now:
import { ResourceCatalogAPI } from "./resource-catalog/index.js"; /** Entrypoint for runtime API */ export const resourceCatalog = ResourceCatalogAPI.getInstance();No further changes are required.
packages/core/src/v3/resource-catalog/index.ts (5)
1-7
: Import statements and constants look good.The imports and API name constant are well-organized and follow the project's pattern.
9-10
: Good use of a constant for the NoopResourceCatalog.Creating a constant for the NoopResourceCatalog is a good practice to avoid repeated instantiation.
24-30
: Global resource catalog registration logic looks good.The method to set the global resource catalog and the disable method work correctly, utilizing the global registry utilities.
32-74
: Comprehensive API for resource management.The implementation provides a complete set of methods for managing task and queue metadata, with proper delegation to the catalog instance. Methods for registering, updating, and retrieving resources are all present.
76-78
: Good fallback to NoopResourceCatalog.The private
#getCatalog()
method correctly handles the case when no catalog is registered by falling back to the NOOP implementation.internal-packages/run-engine/src/engine/types.ts (1)
82-82
:❓ Verification inconclusive
Changed queue property to required string.
The queue property in TriggerParams has been changed from an optional QueueOptions object to a required string. This simplifies the interface but requires verification that all code using this property has been updated.
🏁 Script executed:
#!/bin/bash # Search for TriggerParams usage that might be affected by the queue property change rg -A 2 "queue\s*\?:" --type ts rg -A 2 "queue\s*:" --type ts | grep -E "QueueOptions|{" --context=3Length of output: 68876
TriggerParams Queue Property Update – Verify Downstream Usage
The change in internal-packages/run-engine/src/engine/types.ts (line 82) now defines TriggerParams.queue as a required string instead of an optional QueueOptions object. Our grep results confirm that within run-engine (e.g., in keyProducer and run-queue modules) usages of the queue property are now treated as strings. However, several files outside of run-engine (for example, in packages/trigger-sdk and some test utilities) still reference queue in its former form (either as an optional string or as an object with properties such as name and concurrencyLimit).
Please review and ensure that:
• All consumers of TriggerParams are updated to pass a plain string for the queue property.
• If external packages still require a QueueOptions object, introduce the necessary conversion or consider scoping the change appropriately.packages/core/src/v3/schemas/runEngine.ts (1)
24-24
: Added new task run status PENDING_VERSION.The new status PENDING_VERSION has been added to TaskRunStatus, which is a good addition for handling version-specific pending states. This replaces the usage of WAITING_FOR_DEPLOY in some contexts as seen in the OpenAPI spec changes.
docs/v3-openapi.yaml (3)
680-680
: Updated CommonRunsFilter status enum.The WAITING_FOR_DEPLOY status has been replaced with PENDING_VERSION in the CommonRunsFilter schema. This aligns with the change in the TaskRunStatus object in the core code.
755-755
: Updated ListRunItem status enum.The WAITING_FOR_DEPLOY status has been replaced with PENDING_VERSION in the ListRunItem schema, keeping the API documentation in sync with the core code changes.
950-950
: Updated CommonRunObject status enum.The WAITING_FOR_DEPLOY status has been replaced with PENDING_VERSION in the CommonRunObject schema, ensuring consistent status representation across the API.
packages/core/src/v3/tryCatch.ts (2)
1-5
: Well-designed discriminated union type for error handling.The
Result<T, E>
type is a clear and clean implementation of the Either pattern for error handling, using a discriminated union approach.
7-15
: Solid implementation of the tryCatch utility function.The
tryCatch
function provides a clean way to handle asynchronous operations and their potential errors by returning a tuple that clearly indicates success or failure. This pattern will help standardize error handling across the codebase.packages/core/src/v3/types/queues.ts (1)
1-43
: Excellent type definition with comprehensive documentation.The
QueueOptions
type is well structured with detailed JSDoc comments that include clear examples. The three properties (name
,concurrencyLimit
, andreleaseConcurrencyOnWaitpoint
) are all properly documented with their purpose and behavior.The examples demonstrating how to define a shared queue and associate tasks with it provide valuable context for developers implementing this feature.
apps/webapp/app/v3/services/triggerTaskV1.server.ts (2)
45-51
: Good backward compatibility implementation.Creating a local Zod schema for
QueueOptions
ensures backward compatibility while the system transitions from task-centric to resource-centric models. The schema correctly mirrors the essential properties from the TypeScript type.
705-705
:❓ Verification inconclusive
Confirm that QueueOptions is still compatible with existing queue config data.
The line uses the new local
QueueOptions
schema to parsetask.queueConfig
. Verify that this refactoring doesn't break existing queue configurations.
🏁 Script executed:
#!/bin/bash # Search for existing usages of queueConfig to verify compatibility rg -A 3 -B 3 "queueConfig" --type typescriptLength of output: 79
Confirm Compatibility of New QueueOptions Schema with Existing Queue Configurations
In apps/webapp/app/v3/services/triggerTaskV1.server.ts at line 705, the code now uses the local
QueueOptions
schema to parsetask.queueConfig
:const queueConfig = QueueOptions.optional().nullable().safeParse(task.queueConfig);Please verify that the updated schema remains compatible with all existing queue configuration data. Since the previous search command using
--type typescript
returned an error, try re-running the search using the correct file glob (e.g.,--glob "*.ts"
) to gather all related usage contexts. Manual verification is also recommended to ensure that no existing queue configurations are adversely affected.packages/cli-v3/src/entryPoints/managed-index-controller.ts (1)
98-98
: Good addition of queues to the background worker metadata.Adding the
queues
property to the background worker metadata ensures that queue definitions from the worker manifest are properly passed to the deployment. This change aligns with the new queue management system being introduced in this PR.apps/webapp/app/models/taskQueue.server.ts (1)
1-1
: Schema update for queue configuration parsingThe import statement has been updated from
QueueOptions
toQueueManifest
, and line 38 now usesQueueManifest.safeParse
instead ofQueueOptions.safeParse
. This change aligns with the broader restructuring to use a more comprehensive queue definition schema.Make sure that all properties used from
queueConfig.data
remain compatible across both schemas to avoid potential runtime issues.Also applies to: 38-38
references/hello-world/src/trigger/queues.ts (3)
1-1
: Updated import statement to include queue functionThe import statement now includes the
queue
function from the SDK, which is used to define custom queues.
43-46
: Good modularization of queue configurationExtracting the queue definition to a separate constant improves reusability and maintainability. This allows the same queue configuration to be shared across multiple tasks.
50-50
: Queue reference updated to use declared constantThe task now references the predefined queue constant rather than including an inline queue definition. This is cleaner and ensures consistency if multiple tasks use the same queue.
packages/core/src/v3/schemas/build.ts (2)
3-3
: Updated import to include QueueManifestThe import statement has been updated to include QueueManifest from schemas.js, which is necessary for the changes to WorkerManifest.
84-84
: Added queues property to WorkerManifest schemaThe WorkerManifest schema now includes an optional
queues
property that accepts an array of QueueManifest objects. This enhancement allows workers to define their own queues, which is consistent with the queue indexing changes in this PR.This change enables better management of queue resources at the worker level and supports the transition to a more resource-centric model.
packages/core/src/v3/schemas/resources.ts (4)
2-2
: Updated import statement for QueueManifestThe import statement has been modified to include QueueManifest from schemas.js, which is used in multiple places in this file.
10-10
: Updated queue field to use QueueManifest instead of QueueOptionsThe
queue
field in TaskResource now uses an extended QueueManifest schema with an optional name property. This change is part of the transition to using QueueManifest throughout the codebase for consistent queue definitions.Ensure that consumers of this schema are updated to handle the new structure correctly.
34-34
: Added queues field to BackgroundWorkerMetadataThe BackgroundWorkerMetadata schema now includes an optional
queues
array, allowing background workers to define multiple queues. This change complements the update to WorkerManifest and supports the queue indexing feature.This is a good enhancement that allows for better organization and management of queues within background workers.
9-9
:❓ Verification inconclusive
Made exportName optional in TaskResource schema
The
exportName
field in TaskResource is now optional. This change provides more flexibility for how tasks can be defined and referenced.Make sure that any code that previously relied on exportName being required is updated to handle the case when it's undefined.
🏁 Script executed:
#!/bin/bash # Check for usages of exportName in the codebase that might not handle undefined values rg "\.exportName\b" --type typescript -A 2 -B 2Length of output: 84
I'll update the search command so that it correctly targets TypeScript files and then re-run the check. The previous command used an unrecognized file type ("typescript"), so we should add the file type mapping and re-run the search. Please run the following shell script:
#!/bin/bash # Check for usages of exportName in TypeScript files that might not handle undefined values. rg "\.exportName\b" --type-add 'typescript:*.{ts,tsx}' --type typescript -A 2 -B 2This should correctly search within files ending in .ts and .tsx. Once you have the output from this script, please verify whether there are any cases that assume exportName is defined, and update them accordingly if necessary.
references/hello-world/src/trigger/pendingVersions.ts (2)
3-6
: Queue configuration looks good.The pendingVersionsQueue with concurrency limit of 1 ensures tasks will be processed one at a time, which is appropriate for controlled sequential processing.
31-37
: Task ID should match trigger identifier.The pendingVersionsTester3 task ID is "pending-versions-tester-3" but it's triggered with "pending-versions-tester-3". While these match now, consider standardizing the hyphenation pattern across all task definitions for consistency.
packages/trigger-sdk/src/v3/schedules/index.ts (2)
12-12
: Import renamed from taskCatalog to resourceCatalog.This change is part of the transition from a task-centric to a resource-centric model as described in the PR objectives.
62-62
:❓ Verification inconclusive
Function call updated to use resourceCatalog.
Correctly updated the function call to use resourceCatalog.updateTaskMetadata instead of taskCatalog.updateTaskMetadata, maintaining consistency with the import change.
🏁 Script executed:
#!/bin/bash # Verify that all references to taskCatalog have been updated to resourceCatalog rg "taskCatalog\." --type tsLength of output: 30
ResourceCatalog update verified – please perform a manual check
- Confirmed the function call at line 62 in
packages/trigger-sdk/src/v3/schedules/index.ts
now usesresourceCatalog.updateTaskMetadata(...)
as intended.- A repository-wide search for
"taskCatalog."
returned no results, suggesting that all instances have been updated.- Given the absence of output from the static search, please manually verify that no legacy references to
taskCatalog
remain elsewhere in the codebase or in integration points.apps/webapp/app/components/runs/v3/TaskRunsTable.tsx (1)
333-336
: Improved handling of pending task runs.The conditional logic now correctly handles the new PENDING_VERSION status by first checking if a run is pending before checking if it has started. This ensures proper display of duration information for various task states.
The hierarchical approach to checking conditions (
isPending
first, thenstartedAt
) makes the code more maintainable as new states are added in the future.packages/core/src/v3/index.ts (2)
20-20
: Added export for resource-catalog-api.This change aligns with the transition from task-centric to resource-centric model mentioned in the PR objectives. The resourceCatalog export replaces the previous taskCatalog.
27-27
:✅ Verification successful
Added export for tryCatch utility.
New export for error handling utilities, which will be useful for managing exceptions that might occur during task processing, particularly with the new PENDING_VERSION status.
🏁 Script executed:
#!/bin/bash # Check the implementation of tryCatch.js to understand its functionality cat packages/core/src/v3/tryCatch.js 2>/dev/null || cat packages/core/src/v3/tryCatch.ts 2>/dev/null || echo "File not found"Length of output: 484
Action: Confirm Export and Utility Implementation
The export in
packages/core/src/v3/index.ts
(line 27) correctly re-exports thetryCatch
utility. I verified that the implementation (as seen inpackages/core/src/v3/tryCatch.ts
) properly wraps asynchronous operations and handles exceptions, returning a tuple that differentiates between success and error conditions. The new export is well-designed to support error handling—including scenarios related to the new PENDING_VERSION status.packages/cli-v3/src/dev/devSupervisor.ts (3)
116-116
: Good addition of debug logging for worker skippingThis log statement improves visibility into worker initialization by showing when a worker is skipped due to a matching content hash. This helps with debugging and understanding the worker lifecycle.
130-131
: Good addition of debug logging for worker initializationAdding debug logging when initializing a background worker improves observability and helps trace the worker initialization process.
159-159
: Good addition of queues property to background worker metadataIncluding the queues in the background worker metadata aligns with the PR objectives related to queue indexing and queue trigger changes. This ensures that queue information is properly captured when registering background workers.
apps/webapp/app/v3/taskStatus.ts (3)
19-19
: Good addition of PENDING_VERSION statusAdding the new PENDING_VERSION status to the NON_FINAL_RUN_STATUSES array is appropriate since it represents a non-terminal state.
29-35
: Good grouping of pending statusesCreating a dedicated constant for pending statuses improves code organization and makes the relationship between these statuses explicit. This will make status checks more semantic and maintainable.
103-105
: Good addition of isPendingRunStatus helper functionThis helper function follows the established pattern of other status-checking functions in the file and provides a clean way to check if a status is in a pending state.
apps/webapp/app/presenters/v3/TestTaskPresenter.server.ts (1)
24-24
: Good change to make exportName optionalMaking the
exportName
property optional provides more flexibility in task representation and aligns with the changes in other parts of the codebase.apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts (1)
283-285
: Good handling of PENDING_VERSION statusAdding a specific case for the new PENDING_VERSION status in the apiStatusToRunStatuses method ensures consistent status mapping across the API layer. This addition follows the established pattern and contributes to a complete implementation of the new status.
internal-packages/database/prisma/migrations/20250320130354_add_many_to_many_relationship_task_queue_background_worker/migration.sql (3)
1-8
: Well-structured many-to-many relationship table implementationThis migration creates a new many-to-many relationship table
_BackgroundWorkerToTaskQueue
with appropriate columns and a unique index to ensure each relationship is only stored once.
10-14
: Proper foreign key constraint implementation for column AThe foreign key constraint correctly links column A to the BackgroundWorker table with appropriate cascade behavior.
16-20
: Proper foreign key constraint implementation for column BThe foreign key constraint correctly links column B to the TaskQueue table with appropriate cascade behavior, completing the many-to-many relationship.
internal-packages/database/prisma/migrations/20250320085824_add_queue_to_background_worker_task/migration.sql (2)
1-7
: Good implementation of table modificationsThe migration correctly adds the nullable
queueId
column and modifies theexportName
column to drop the NOT NULL constraint. This change supports the transition from a task-centric to a resource-centric model.
9-15
: Proper foreign key constraint with appropriate deletion behaviorThe foreign key constraint correctly links the
queueId
column to theTaskQueue
table with SET NULL on delete, which is appropriate for this relationship.internal-packages/run-engine/src/engine/tests/dequeuing.test.ts (4)
4-8
: Import statements updated to reflect architectural changesThe imports have been correctly updated to reflect the new location of setup functions and the addition of the RunEngine import.
49-49
: Updated setupBackgroundWorker parameter from prisma to engineThe function now correctly takes the engine instance as its first parameter instead of prisma, reflecting the architectural changes.
114-116
: Updated setupBackgroundWorker call to use engine parameterThe second instance of the function call has also been correctly updated to use the engine parameter.
196-197
: Updated parameter name from queueName to queueThe parameter name has been updated to better reflect its purpose, aligning with the resource-centric model.
apps/webapp/app/components/runs/v3/TaskRunStatus.tsx (11)
18-18
: Added Callout import for new status reason featureImport of the Callout component added to support the new status reason display functionality.
22-26
: Added PENDING_VERSION to allTaskRunStatusesThe new PENDING_VERSION status has been correctly added to the allTaskRunStatuses array.
41-43
: Added PENDING_VERSION to filterableTaskRunStatusesThe new status has been added to the filterableTaskRunStatuses array, replacing WAITING_FOR_DEPLOY in the filtering list.
60-62
: Added description for the new PENDING_VERSION statusClear description added for the PENDING_VERSION status, matching the updated description for WAITING_FOR_DEPLOY.
77-82
: Updated QUEUED_STATUSES to include PENDING_VERSIONThe new status has been correctly added to the QUEUED_STATUSES array.
94-104
: Enhanced TaskRunStatusCombo to support status reasonsThe component now accepts an optional statusReason prop, which enables more detailed status information.
106-110
: Added TaskRunStatusReason component to the status displayThe TaskRunStatusReason component is now included in the status display to show additional context when available.
114-149
: Well-implemented TaskRunStatusReason componentThe new component properly handles displaying status reasons for the PENDING_VERSION status. It includes clear mappings for various reason codes and safely handles cases where no reason is provided.
167-169
: Updated TaskRunStatusIcon to handle PENDING_VERSIONThe TaskRunStatusIcon function now correctly handles the new status, reusing the same icon as WAITING_FOR_DEPLOY.
201-209
: Updated runStatusClassNameColor to handle PENDING_VERSIONThe function now correctly applies the amber-500 color to the new status, consistent with WAITING_FOR_DEPLOY.
237-246
: Updated runStatusTitle to include the new statusThe function now returns "Pending version" for the PENDING_VERSION status, matching the text for WAITING_FOR_DEPLOY.
internal-packages/run-engine/src/engine/tests/releaseConcurrency.test.ts (3)
5-5
: Improved organization of test dependenciesMoving the test setup imports from
@internal/testcontainers
to a localsetup.js
file centralizes the test utilities, making the code more maintainable and the dependencies more explicit.
51-51
: Updated setupBackgroundWorker parameter to use engine instead of prismaThis change correctly aligns the function parameter with the updated signature in
setup.js
, passing the RunEngine instance directly instead of the Prisma client. This makes more sense architecturally since the background worker is part of the engine's functionality.Also applies to: 243-243, 429-429, 617-617, 779-779, 961-961
66-66
: Property rename from queueName to queueThis change standardizes the property name used throughout the codebase, matching the restructuring described in the PR objective for better queue indexing and trigger changes.
Also applies to: 266-266, 454-454, 632-632, 794-794, 976-976
internal-packages/run-engine/src/engine/tests/cancelling.test.ts (3)
7-7
: Improved organization of test dependenciesMoving the test setup imports from
@internal/testcontainers
to a localsetup.js
file centralizes the test utilities, enhancing code maintainability.
52-52
: Updated setupBackgroundWorker parameter to use engine instead of prismaPassing the RunEngine instance instead of the Prisma client aligns better with the component's architecture. This change properly reflects that the background worker is a part of the engine's functionality rather than a direct database operation.
Also applies to: 265-265
68-68
: Property rename from queueName to queueStandardizing on
queue
property name across the codebase improves consistency and follows the transition from task-centric to resource-centric model mentioned in the PR summary.Also applies to: 103-103, 281-281
internal-packages/run-engine/src/engine/tests/delays.test.ts (3)
6-6
: Improved organization of test dependenciesMoving the test setup imports to a local
setup.js
file creates a more maintainable structure for test utilities.
49-50
: Updated setupBackgroundWorker parameter to use engine instead of prismaThis change properly aligns with the updated function signature in the setup module, passing the engine instance that the worker actually needs to operate.
Also applies to: 131-133, 228-230
68-68
: Property rename from queueName to queueThis change is part of the broader refactoring effort to standardize queue-related nomenclature across the codebase, improving consistency.
Also applies to: 150-150, 247-247
internal-packages/run-engine/src/engine/tests/heartbeats.test.ts (3)
5-5
: Improved organization of test dependenciesCentralizing the imports for setup functions in a dedicated file improves code organization and makes dependencies more explicit.
56-58
: Updated setupBackgroundWorker parameter to use engine instead of prismaThis change correctly passes the RunEngine instance to the background worker setup, which is more logical since the worker functionality is tied to the engine rather than directly to the database.
Also applies to: 173-175, 293-295, 422-424, 524-526
75-75
: Property rename from queueName to queueThe renaming from
queueName
toqueue
maintains consistency with the changes throughout the codebase, reflecting the move toward a more resource-centric model mentioned in the PR summary.Also applies to: 192-192, 312-312, 441-441, 543-543
internal-packages/run-engine/src/engine/tests/priority.test.ts (3)
8-8
: Consistent refactoring of setup imports.The import statement has been updated to import the setup functions from a local
setup.js
file rather than from the@internal/testcontainers
module, centralizing these utilities and making them easier to maintain.
51-51
: Updated parameter fromprisma
toengine
for setupBackgroundWorker.This change reflects a shift in the setup function's interface, now expecting the engine object directly instead of the Prisma client, aligning with the resource-centric model mentioned in the PR objectives.
129-130
: Renamed property fromqueueName
toqueue
for consistency.This property renaming from
queueName
toqueue
aligns with the queue indexing changes mentioned in the PR objectives, providing a more consistent API across the codebase.apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts (2)
207-209
: Added support for new 'PENDING_VERSION' status.The addition of a case for the "PENDING_VERSION" status in the
apiStatusFromRunStatus
function properly handles this new run status, ensuring it's correctly translated for the API response.
263-267
: Updated isQueued condition to include 'PENDING_VERSION'.The
isQueued
helper now correctly includes "PENDING_VERSION" as one of the queued states, ensuring consistent behavior in UI and API endpoints that need to know if a run is in a queued state.internal-packages/run-engine/src/engine/tests/pendingVersion.test.ts (5)
1-8
: Well-structured test setup with appropriate imports.The imports and configuration for these tests are correctly set up using the containerTest framework, appropriate assertions, and necessary timeouts. The file includes essential imports from the tracing module, the RunEngine class, and the setup utilities.
9-45
: Comprehensive test for handling runs when background tasks aren't created yet.This test case properly validates the behavior of the RunEngine when a run is triggered before its background task has been created. The RunEngine configuration is thorough, including appropriate settings for the worker, queue, run locks, and machine specifications.
92-126
: Robust validation of PENDING_VERSION status.The test correctly validates that runs are initially queued but transition to a PENDING_VERSION state when the background task hasn't been created yet. It properly checks execution data, queue status, and run status to ensure the system behaves as expected.
127-152
: Validates state transition from PENDING_VERSION to PENDING.This section appropriately tests that runs transition from PENDING_VERSION to PENDING after the background worker is set up. The async wait ensures the system has time to process the state change, and the assertions verify both execution status and run status.
159-318
: Thorough test for queue creation scenarios.The second test case properly validates behavior when custom queues haven't been created yet. It checks queue lengths and run statuses at various stages, ensuring runs transition correctly from PENDING_VERSION to PENDING when custom queues are registered.
internal-packages/run-engine/src/engine/tests/attemptFailures.test.ts (6)
6-6
: Updated import sources for setup utilities.The import statement has been updated to source the setup functions from a local
setup.js
file, consistent with the changes made in the priority test file, maintaining consistency across test files.
47-48
: Updated parameter fromprisma
toengine
for setupBackgroundWorker.Similar to the change in the priority test, the function now expects the engine object directly, aligning with the resource-centric approach of the refactoring.
66-66
: Standardized queue property name.The property
queueName
has been renamed toqueue
for consistency with other changes in the PR, reflecting the queue indexing improvements mentioned in the PR objectives.
199-201
: Updated setupBackgroundWorker parameter.The engine is now passed directly to setupBackgroundWorker, consistent with the pattern established in other test files, while maintaining the existing maxAttempts configuration.
217-217
: Consistent queue property naming throughout the file.This change aligns with similar changes in other trigger calls, ensuring a consistent API interface across the codebase.
310-312
: Same parameter update pattern applied consistently.The setupBackgroundWorker function calls have been consistently updated throughout all test cases to use engine instead of prisma as the first parameter.
apps/webapp/app/v3/services/triggerTaskV2.server.ts (3)
345-345
: Simplification of queue parameter assignmentNow consistently setting the
queue
parameter toqueueName
, derived from a sanitized version of the task identifier. This change improves consistency and removes the dependency onbody.options?.queue
.
493-495
: Improved database query to include queue relationshipAdding the queue relationship in the database query is more efficient than fetching the task first and then having to query for its queue separately.
507-517
: Improved error handling for missing queueThe updated logic correctly checks for the existence of
task.queue
instead of validating a parsedqueueConfig
. The direct access totask.queue.name
simplifies the code and makes it more robust.packages/cli-v3/src/entryPoints/dev-index-worker.ts (5)
5-6
: Transition from task-centric to resource-centric modelThe renaming of entities from
taskCatalog
toresourceCatalog
,StandardTaskCatalog
toStandardResourceCatalog
, andregisterTasks
toregisterResources
reflects a significant shift in the architecture towards a resource-centric model. This change improves the flexibility of the system to handle different types of resources beyond just tasks.Also applies to: 10-11, 17-17
54-54
: Updated global catalog initializationSetting the global resource catalog to use the new
StandardResourceCatalog
class aligns with the architectural changes.
89-89
: Updated registration functionChanged from
registerTasks
toregisterResources
to align with the resource-centric approach.
101-101
: Updated task listing methodNow obtaining task manifests through the resource catalog, which is consistent with the new resource-centric model.
137-138
: Added queue manifests to the INDEX_COMPLETE messageIncluding queue manifests in the message payload provides more comprehensive information about available resources, which is important for the new resource-centric model.
packages/core/src/v3/types/tasks.ts (3)
16-18
: Simplified Queue type definitionThe
Queue
type is now directly assigned toQueueOptions
, simplifying the type system by removing theRequireOne
utility type. This makes the code more maintainable and easier to understand.
196-199
: Simplified queue configuration in CommonTaskOptionsThe
queue
property has been redefined with a more explicit structure that includes optionalname
andconcurrencyLimit
properties. This makes the API more intuitive and less prone to errors.
710-712
: Updated queue property in TriggerOptionsChanged the
queue
property from a complex type to a simple string, which aligns with the comment update that explains the PENDING_VERSION state when a queue doesn't exist. This simplification makes the API more straightforward to use.internal-packages/run-engine/src/engine/tests/trigger.test.ts (3)
1-6
: Updated import statementsImport statements have been streamlined, now importing
setupAuthenticatedEnvironment
andsetupBackgroundWorker
from a localsetup.js
file instead of from@internal/testcontainers
. This change likely improves modularity and testing setup.
49-50
: Updated setupBackgroundWorker parameterThe parameter passed to the
setupBackgroundWorker
function has been changed fromprisma
toengine
, suggesting a refactoring to inject the engine dependency directly rather than just the database connection.Also applies to: 243-244
68-69
: Renamed queue property in trigger parametersChanged the property name from
queueName
toqueue
in the object passed to theengine.trigger
method, aligning with the interface changes made in the core code.Also applies to: 262-263
packages/cli-v3/src/entryPoints/dev-run-worker.ts (6)
12-12
: Renamed import from taskCatalog to resourceCatalogThis change is part of the transition from a task-centric to a resource-centric model. The
resourceCatalog
import replaces the previoustaskCatalog
import, reflecting the architectural shift in the system.
33-33
: Updated import from StandardTaskCatalog to StandardResourceCatalogThis renaming aligns with the broader transition to a resource-centric model throughout the codebase.
99-99
: Updated catalog initialization to use ResourceCatalogThe global manager is now initialized with a
StandardResourceCatalog
instance instead of aStandardTaskCatalog
, continuing the pattern of transition to the resource-centric model.
142-143
: Replaced task registration with worker manifest registrationThe code now registers the worker manifest with the resource catalog instead of registering individual tasks. This approach provides better organization of resources and aligns with the new architecture.
259-261
: Added file context management during importsThe resource catalog now handles file context during imports, which improves resource tracking and management. The context is properly set before import and cleared afterward, ensuring clean state management.
296-296
: Updated task retrieval to use resource catalogTask retrieval now uses the resource catalog's getTask method instead of a task-specific approach, completing the transition to the resource-centric model.
packages/cli-v3/src/indexing/registerResources.ts (2)
1-38
: New function for registering resources from a build manifestThis new function implements the resource-centric approach to importing and registering task files. It properly manages file context during imports and collects any errors that occur during the process.
🧰 Tools
🪛 Biome (1.9.4)
[error] 33-33: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
40-50
: Well-implemented utility function for safe importsThe
tryImport
helper function is a clean implementation of the try-catch pattern for dynamic imports, returning a standardized result tuple.packages/core/src/v3/resource-catalog/catalog.ts (1)
1-1
: Expanded ResourceCatalog interface with new resource management capabilitiesThe ResourceCatalog interface (renamed from TaskCatalog) has been enhanced with methods for:
- File context management (
setCurrentFileContext
,clearCurrentFileContext
)- Worker manifest registration (
registerWorkerManifest
)- Queue management (
registerQueueMetadata
,listQueueManifests
)These additions support the transition to a resource-centric model and provide a more comprehensive approach to managing different resource types.
Also applies to: 4-16
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx (2)
405-405
: Added statusReason to TaskRunStatusCombo componentThe component now displays the reason for a particular status, providing more context about why a run is in its current state. This enhances the UI's ability to communicate detailed status information.
716-720
: Updated TaskRunStatusCombo with statusReason in another instanceConsistent application of the new statusReason prop in another instance of the TaskRunStatusCombo component, ensuring uniform status display throughout the UI.
internal-packages/run-engine/src/engine/tests/batchTrigger.test.ts (4)
7-7
: Import paths have been updated for better organization.The imports for setup functions have been moved from
@internal/testcontainers
to a localsetup.js
file, which improves code organization by consolidating related test setup utilities.
50-50
: Parameter change fromprisma
toengine
.The parameter passed to
setupBackgroundWorker
has been updated fromprisma
toengine
, better reflecting the dependency being injected into the worker setup.
76-77
: Property name changed fromqueueName
toqueue
.The property name has been standardized from
queueName
toqueue
, ensuring consistent naming conventions across the codebase.
97-97
: Property name changed fromqueueName
toqueue
.Same naming convention update as above, maintaining consistency throughout multiple function calls.
internal-packages/run-engine/src/engine/tests/ttl.test.ts (4)
1-1
: Import updated to includeassertNonNullable
.The import now explicitly includes
assertNonNullable
from the@internal/testcontainers
package, which is used in the test to validate non-null values.
7-7
: Import paths have been updated for better organization.Similar to the change in batchTrigger.test.ts, the imports for setup functions have been moved from
@internal/testcontainers
to a localsetup.js
file, improving code organization.
50-50
: Parameter change fromprisma
toengine
.The dependency injection has been updated to pass the
engine
instance directly to thesetupBackgroundWorker
function rather than theprisma
client.
69-69
: Property name changed fromqueueName
toqueue
.Property naming has been standardized to use
queue
instead ofqueueName
, maintaining consistency with other files in the codebase.packages/cli-v3/src/entryPoints/managed-run-worker.ts (6)
9-9
: RenamedtaskCatalog
toresourceCatalog
in imports.This change reflects a shift from a task-centric model to a resource-centric model in the system architecture.
32-32
: RenamedStandardTaskCatalog
toStandardResourceCatalog
.Continuing the architectural shift to a resource-centric model, the class name has been updated to reflect its new purpose.
110-110
: Updated global catalog registration.Changed from setting a global task catalog to setting a global resource catalog, aligning with the new resource-centric architecture.
151-151
: Updated worker manifest registration.The worker manifest is now registered with the resource catalog instead of the task catalog, consistent with the architectural changes.
269-271
: Added file context management for imports.New context management functions have been added to track the current file context during task imports and clear it afterward. This improves resource tracking and error handling during the import process.
306-306
: Updated task retrieval from resource catalog.Task retrieval now uses the resource catalog instead of the task catalog, completing the transition to the resource-centric model.
apps/webapp/app/v3/services/createDeploymentBackgroundWorker.server.ts (7)
1-2
: Enhanced imports for error handling.Added
tryCatch
from core library to improve error handling throughout the service.
5-5
: Added ServiceValidationError import.The ServiceValidationError is now properly imported to provide more specific error types when validation fails.
8-8
: Replaced createBackgroundTasks with createWorkerResources.Updated to use the new
createWorkerResources
function, aligning with the architectural shift to resource-centric operations.
63-82
: Improved error handling for file creation.Implemented robust error handling using
tryCatch
for creating background worker files. This now provides proper logging and a specific ServiceValidationError with appropriate failure handling.
84-106
: Enhanced error handling for worker resources creation.Similar to the file creation error handling, this section now uses
tryCatch
to properly handle errors when creating worker resources, providing better diagnostics and a cleaner error propagation path.
108-122
: Added robust error handling for declarative schedules sync.Implemented proper error handling for the schedule synchronization process using
tryCatch
, ensuring any errors during this phase are correctly captured, logged, and handled.
142-160
: Added private helper method for failure handling.The new
#failBackgroundWorkerDeployment
private method centralizes the logic for handling deployment failures, ensuring consistent status updates and error reporting across different failure scenarios. This reduces code duplication and improves maintainability.apps/webapp/app/presenters/v3/SpanPresenter.server.ts (3)
7-7
: Improved import organization.The imports have been reorganized more logically, with core modules first, then internal modules. This follows better code organization practices.
Also applies to: 9-9, 13-14
152-152
: AddedstatusReason
field to database query.This properly selects the new
statusReason
field from the database, which will contain additional context about run statuses.
316-316
: AddedstatusReason
to the response object.The new field is correctly added with a nullish coalescing operator to default to
undefined
when null. This ensures consistent typing in the response.internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (5)
189-190
: Added handling for QUEUE_NOT_FOUND result code.This change improves error handling by addressing the case where a queue is not found, consistent with the existing handling of other error codes.
198-204
: Updated method call from#waitingForDeploy
to#pendingVersion
.The rename reflects the status change from "WAITING_FOR_DEPLOY" to "PENDING_VERSION". The additional
statusReason
parameter provides better context for the pending state.
237-243
: Consistent usage of#pendingVersion
with appropriate status reason.The second instance of using
#pendingVersion
correctly uses "NO_DEPLOYMENT" as the status reason, which is more specific than the previous implementation.
516-546
: Renamed method from#waitingForDeploy
to#pendingVersion
with enhanced status tracking.The method has been renamed to better reflect its purpose and now:
- Updates the run status to "PENDING_VERSION" instead of "WAITING_FOR_DEPLOY"
- Stores the specific
statusReason
in the database- Adds debug logging with relevant information
This change improves diagnostics and provides more context about why runs are in a pending state.
562-565
: Added helpful debug logging.This debug log message provides clear information about the run being in a pending version state, which will be valuable for troubleshooting.
internal-packages/run-engine/src/engine/tests/batchTriggerAndWait.test.ts (3)
1-7
: Improved import organization.The test setup imports have been refactored to use a local
setup.js
file, which helps centralize test setup logic and makes the imports cleaner.
50-50
: UpdatedsetupBackgroundWorker
parameter fromprisma
toengine
.The function now receives the engine instance instead of the Prisma instance, which is a more logical dependency injection approach since the engine has access to Prisma.
74-74
: Renamed property fromqueueName
toqueue
for consistency.The parameter name has been standardized to
queue
rather thanqueueName
across all trigger calls, maintaining consistency with other parts of the codebase.Also applies to: 122-122, 149-149
internal-packages/run-engine/src/engine/tests/triggerAndWait.test.ts (3)
1-6
: Improved import organization.The test setup imports have been refactored to use a local
setup.js
file, which helps centralize test setup logic and makes the imports cleaner.
49-49
: UpdatedsetupBackgroundWorker
parameter fromprisma
toengine
.The function now receives the engine instance instead of the Prisma instance, which is a more logical dependency injection approach since the engine has access to Prisma.
Also applies to: 237-237
65-65
: Renamed property fromqueueName
toqueue
for consistency.The parameter name has been standardized to
queue
rather thanqueueName
across all trigger calls, maintaining consistency with other parts of the codebase.Also applies to: 100-100, 253-253, 285-285, 342-342
packages/cli-v3/src/entryPoints/managed-index-worker.ts (7)
5-5
: Renamed import aligns with resource-centric model transitionThe change from
taskCatalog
toresourceCatalog
reflects the PR's shift from a task-centric to a resource-centric architecture.
10-10
: Updated catalog class import for resource-centric approachChanging from
StandardTaskCatalog
toStandardResourceCatalog
maintains consistency with the resource-centric architectural shift.
17-17
: Imported resource registration functionImporting
registerResources
instead of the previousregisterTasks
function aligns with the resource-centric approach being implemented across the codebase.
54-54
: Global catalog instance updated to resource-based implementationUpdated the global catalog setter to use the new resource-based catalog implementation.
89-89
: Updated function call to use resource registrationChanged from
registerTasks
toregisterResources
while maintaining the same error handling approach.
101-101
: Maintained task listing functionality with resource catalogThe code still retrieves task manifests but now through the resource catalog, ensuring backward compatibility while transitioning to the resource-centric model.
137-137
: Added queues to manifest dataEnhanced the manifest object to include queue manifests from the resource catalog, supporting the queue indexing functionality introduced in this PR.
internal-packages/run-engine/src/engine/tests/waitpoints.test.ts (6)
1-2
: Streamlined importsSimplified imports while maintaining functionality by restructuring how test dependencies are imported.
8-8
: Changed dependency sourceImporting setup functions from local module instead of from
@internal/testcontainers
, which improves modularity and localization of test utilities.
50-50
: Updated setupBackgroundWorker parameterChanged the first parameter from
prisma
toengine
, which is a more appropriate dependency injection pattern as the engine already contains the prisma instance.
66-66
: Queue property format updateChanged from
queueName
toqueue
in the trigger configuration, reflecting the schema changes in the API.
165-165
: Standardized queue property across all triggersAll trigger calls have been updated to use the new
queue
property format instead ofqueueName
, ensuring consistency across the codebase.Also applies to: 181-181, 243-243, 323-323, 462-462, 530-530, 577-577, 676-676, 823-823, 877-877, 1026-1026, 1196-1196, 1250-1250
1171-1180
: Enhanced background worker setup with concurrency optionsAdded explicit concurrency configuration to the worker setup, which is crucial for properly testing the queue behavior with limited concurrency.
packages/core/src/v3/schemas/api.ts (4)
1-5
: Simplified importsRemoved unused imports and consolidated necessary imports, making the code cleaner and more maintainable.
100-106
: Updated queue schema to object structureChanged the
queue
property from a string to an object with aname
property, while also deprecating theconcurrencyLimit
at this level. This provides a more structured approach to queue configuration.
160-164
: Consistent queue schema in batch trigger itemsApplied the same object-based queue structure to batch trigger items, ensuring consistency across different trigger methods.
612-613
: Added PENDING_VERSION statusIntroduced a new task run status
PENDING_VERSION
that replacesWAITING_FOR_DEPLOY
for tasks requiring version updates. This provides more granular status tracking when tasks need additional information before execution.internal-packages/run-engine/src/engine/tests/checkpoints.test.ts (5)
2-2
: Reorganized importsAdjusted import order and structure for better readability while maintaining functionality.
8-8
: Updated setup module sourceImported setup functions from local module instead of from the global test containers module, improving modularity of test utilities.
51-51
: Updated setupBackgroundWorker parameterChanged the first parameter from
prisma
toengine
, which is more appropriate as the engine already contains the prisma instance.Also applies to: 224-224, 310-310, 511-511, 657-657
70-70
: Updated queue property format in triggersChanged from
queueName
toqueue
in all trigger configurations, reflecting the schema changes in the API.Also applies to: 243-243, 329-329, 530-530, 676-676, 823-823, 877-877
798-807
: Enhanced worker setup with concurrency configurationAdded explicit concurrency configuration to the worker setup for the checkpoint test with a limited concurrency of 1, which is essential for properly testing queueing behavior when a run is checkpointed.
internal-packages/run-engine/src/engine/db/worker.ts (9)
6-6
: Added TaskQueue import looks good.
No issues spotted with the extended type coverage.
34-35
: New"QUEUE_NOT_FOUND"
error code addition.
Ensure all call sites handle this new error condition properly, especially in any upstream logic expecting different error codes.
54-54
: Addedqueue
property in success result.
Including thequeue
here is consistent with the refactor to handle queue-based logic. Good addition.
164-172
: Finding and validating the queue reference.
This new check ensures that runs cannot proceed if the associated queue is missing. This is a solid defensive approach to data integrity.
180-180
: Passingqueue
in the success object.
This helps unify queue data with the rest of the run details. No issues here.
188-188
: Addedqueues: TaskQueue[]
to the type.
This property clarifies that multiple queues may be associated with a worker deployment, aligning with the new queue-based logic.
203-211
: Includingqueues: true
in the Prisma query and returning them.
Fetching queue data ensures the newqueue
field can be properly resolved. Looks consistent.
224-235
: Addedqueues
handling ingetMostRecentWorker
.
Fetching and returningqueues
provides a complete snapshot of the current worker's queue configuration. No issues spotted.
249-260
: Retrievingqueues
ingetWorkerById
.
Maintains consistency across retrieval methods by including queue data. Implementation matches the pattern in other functions.packages/core/src/v3/resource-catalog/standardResourceCatalog.ts (7)
1-11
: Refactored imports and class rename toStandardResourceCatalog
.
The shifted imports and the new class name indicate a clear move toward managing resources beyond tasks alone.
15-16
: New private fields_currentFileContext
and_queueMetadata
.
Storing a separate_currentFileContext
avoids confusion with export names, and_queueMetadata
expands the catalog beyond tasks.
18-20
:setCurrentFileContext
method introduction.
Centralizes file-path and entry-point tracking for subsequent resource registrations. Straightforward and useful for context management.
22-24
:clearCurrentFileContext
method introduction.
A complementary method to reset the current file context ensures the catalog remains consistent across multiple registrations.
30-37
:registerWorkerManifest
method to track tasks from the worker manifest.
This centralizes the location for each task's file metadata. Shows good alignment with the updated approach to resource management.
50-53
: Registering file metadata in_taskFileMetadata
using_currentFileContext
.
Ensures each task is associated with the same context as the file in which it’s declared. No concerns here.
100-102
:listQueueManifests
method.
Exposes a simple read of all stored queue manifests, providing a thorough overview of declared queues within this catalog. Looks good.apps/webapp/app/v3/services/createBackgroundWorker.server.ts (12)
2-2
: Refactored import statements.
Bringing inBackgroundWorkerMetadata
,QueueManifest
,sanitizeQueueName
, and other utilities. This cleanup clarifies dependencies and sets up a broader resource-creation flow.Also applies to: 5-5, 8-8, 9-9, 10-10, 12-12, 21-21, 22-22, 24-24, 25-25, 26-26, 27-27, 28-28
101-108
: UsingtryCatch
forcreateBackgroundFiles
.
This pattern improves error handling by splitting the logic into success and failure tuples, providing a consistent approach across this service.
110-118
: Robust error handling forfilesError
.
Logging the error and throwingServiceValidationError
fosters better observability and aligns with the existing error-handling pattern.
120-128
: Creating worker resources withcreateWorkerResources
.
Again wrapping intryCatch
clarifies intent and ensures consistent error reporting if resource creation fails.
130-137
: HandlingresourcesError
.
The code logs enough context for troubleshooting, and re-throws aServiceValidationError
if needed. This is consistent with the approach used in this file.
139-139
:syncDeclarativeSchedules
error handling.
Wrapping schedule synchronization. The fallback toServiceValidationError
helps unify how top-level errors are handled within the same flow.Also applies to: 143-143
152-152
: Updating environment concurrency limits.
UsingtryCatch
for concurrency-limit updates. The code logs the error if it occurs but doesn’t throw, indicating a non-fatal operation. This is a considered design choice.Also applies to: 156-156
164-164
: PublishingWORKER_CREATED
event error handling.
Serves as a crucial piece of system communication; the logs and graceful fallback on error are prudent for maintaining system stability.Also applies to: 174-174
182-182
: Scheduling pending versions for V2 engine.
Selective logic ensures we only enqueue runs for the new worker if engine is V2. Sensible approach for backward compatibility.Also applies to: 183-183, 187-187
199-211
: IntroducingcreateWorkerResources
function.
Splitting out queue creation and task creation fosters clarity and maintainability. Good organizational approach.
213-231
:createWorkerTasks
with chunk-based strategy.
Batching tasks in sets of 20 is helpful to avoid large transaction overhead or memory usage. Solid improvement for scalability.
315-424
: ImplementingcreateWorkerQueues
andcreateWorkerQueue
.
- Processing queues in chunks to avoid large writes is a strong design.
- Sanitizing queue names and verifying concurrency limits show thorough coverage of edge cases.
- Updating or removing concurrency constraints based on whether a limit is set is well-handled.
apps/webapp/app/v3/services/pauseEnvironment.server.ts (1)
6-6
: Import source updated for AuthenticatedEnvironment typeThe import for
AuthenticatedEnvironment
has been changed from"@internal/testcontainers"
to"~/services/apiAuth.server"
. This change appears to be part of the larger refactoring effort mentioned in the PR objectives, likely moving from a test-related module to a service-related module.apps/webapp/app/v3/services/createDeployedBackgroundWorker.server.ts (2)
9-9
: Function renamed from createBackgroundTasks to createWorkerResourcesThis import change reflects the shift from a task-centric to a resource-centric model mentioned in the PR objectives. The import now correctly references the renamed function.
67-67
: Updated function call to createWorkerResourcesThe function call has been updated to use the renamed function
createWorkerResources
instead ofcreateBackgroundTasks
. The parameters remain the same, ensuring compatibility with the existing implementation.apps/webapp/app/v3/taskRunHeartbeatFailed.server.ts (1)
101-101
: Added new PENDING_VERSION case to status handlingThe new
PENDING_VERSION
status has been added to the appropriate case group alongside similar statuses likeDELAYED
andWAITING_FOR_DEPLOY
. This ensures consistent handling for the new status, which aligns with the PR objective of introducing a new task run status.The implementation correctly treats this new status similarly to the other non-executable statuses, where the task run is removed from the queue since it cannot be processed at the moment.
/// The run is pending a version update because it cannot execute without additional information (task, queue, etc.). Replaces WAITING_FOR_DEPLOY | ||
PENDING_VERSION | ||
|
||
/// Task hasn't been deployed yet but is waiting to be executed. Deprecated in favor of PENDING_VERSION | ||
WAITING_FOR_DEPLOY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
PENDING_VERSION status improves the task lifecycle model
The introduction of a more descriptive PENDING_VERSION
status to replace the deprecated WAITING_FOR_DEPLOY
clarifies the reason for task pending state. This reflects a shift from a deployment-centric to a more resource-centric model, providing clearer intent about why tasks are waiting.
Ensure that all code handling the deprecated WAITING_FOR_DEPLOY
status is updated to support the new PENDING_VERSION
status.
🏁 Script executed:
#!/bin/bash
# Check for code references to WAITING_FOR_DEPLOY to ensure they're updated
echo "Searching for code references to the deprecated WAITING_FOR_DEPLOY status..."
rg "WAITING_FOR_DEPLOY" --type ts
Length of output: 3225
Action Required: Update All WAITING_FOR_DEPLOY References to PENDING_VERSION
The Prisma schema changes introducing the new PENDING_VERSION
status are a positive step toward a more descriptive task lifecycle model. However, our verification revealed numerous remaining references to the deprecated WAITING_FOR_DEPLOY
status across the codebase. These include (but are not limited to):
packages/cli-v3/src/dev/mcpServer.ts
packages/core/src/v3/apiClient/runStream.ts
packages/core/src/v3/schemas/runEngine.ts
andpackages/core/src/v3/schemas/api.ts
- Several files within the
apps/webapp
directory (e.g., task status handling and API presenters)
Next Steps:
- Update all occurrences of
WAITING_FOR_DEPLOY
in the code to either usePENDING_VERSION
or ensure that both statuses are correctly handled if needed for backward compatibility. - Verify that any conditional logic or messaging based on the status reflects the updated task lifecycle model.
export class ResourceCatalogAPI { | ||
private static _instance?: ResourceCatalogAPI; | ||
|
||
private constructor() {} | ||
|
||
public static getInstance(): ResourceCatalogAPI { | ||
if (!this._instance) { | ||
this._instance = new ResourceCatalogAPI(); | ||
} | ||
|
||
return this._instance; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the use of this
in static methods.
The singleton pattern implementation is good, but there are some issues with using this
in static methods.
public static getInstance(): ResourceCatalogAPI {
- if (!this._instance) {
- this._instance = new ResourceCatalogAPI();
+ if (!ResourceCatalogAPI._instance) {
+ ResourceCatalogAPI._instance = new ResourceCatalogAPI();
}
- return this._instance;
+ return ResourceCatalogAPI._instance;
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export class ResourceCatalogAPI { | |
private static _instance?: ResourceCatalogAPI; | |
private constructor() {} | |
public static getInstance(): ResourceCatalogAPI { | |
if (!this._instance) { | |
this._instance = new ResourceCatalogAPI(); | |
} | |
return this._instance; | |
} | |
export class ResourceCatalogAPI { | |
private static _instance?: ResourceCatalogAPI; | |
private constructor() {} | |
public static getInstance(): ResourceCatalogAPI { | |
if (!ResourceCatalogAPI._instance) { | |
ResourceCatalogAPI._instance = new ResourceCatalogAPI(); | |
} | |
return ResourceCatalogAPI._instance; | |
} | |
} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 17-17: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 18-18: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 21-21: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
await tasks.trigger( | ||
"pending-versions-tester-3", | ||
{ | ||
message: "Hello, world!", | ||
}, | ||
{ | ||
queue: "pending-version-queue-2", | ||
} | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Queue specification in second trigger is inconsistent.
The task triggers "pending-versions-tester-3" on queue "pending-version-queue-2", but the implementation of pendingVersionsTester3 uses "pending-version-queue". This mismatch could lead to unexpected behavior.
🏁 Script executed:
#!/bin/bash
# Check for any other instances of pending-version-queue-2 to verify if it exists
rg -A 2 -B 2 "pending-version-queue-2" --type ts
Length of output: 392
🏁 Script executed:
#!/bin/bash
# Check for any instances of "pending-version-queue" without the trailing "-2"
rg -A 2 -B 2 "pending-version-queue[^-]" --type ts
Length of output: 427
Queue Name Mismatch Detected
The code currently triggers the task "pending-versions-tester-3" using the queue named "pending-version-queue-2"
(see lines 19–27 in references/hello-world/src/trigger/pendingVersions.ts
), while the actual queue implementation for the pending versions uses the name "pending-version-queue"
. This discrepancy could result in the task being enqueued to the wrong queue, possibly leading to unexpected behavior.
- File:
references/hello-world/src/trigger/pendingVersions.ts
- Trigger invocation (lines 19–27): Uses
"pending-version-queue-2"
- Queue definition: Uses
"pending-version-queue"
- Trigger invocation (lines 19–27): Uses
Please update either the trigger’s queue name to match the queue definition or vice versa to ensure consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.changeset/twelve-onions-melt.md (1)
6-6
: Remove Extraneous LineIt appears there is an extraneous line containing only "6". This likely represents an accidental inclusion and should be removed to avoid confusion.
packages/cli-v3/e2e/fixtures.ts (1)
67-67
: Consider removingexportName
here for consistency.Since
exportName
has been made optional and removed from all other test cases, consider removing it here as well to maintain consistency across all test fixtures.- task: { id: "helloWorld", filePath: "src/trigger/helloWorld.ts", exportName: "helloWorld" }, + task: { id: "helloWorld", filePath: "src/trigger/helloWorld.ts" },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.changeset/config.json
(1 hunks).changeset/twelve-onions-melt.md
(1 hunks)internal-packages/redis/src/index.ts
(1 hunks)packages/cli-v3/e2e/fixtures.ts
(5 hunks)packages/react-hooks/src/hooks/useTaskTrigger.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
🔇 Additional comments (7)
internal-packages/redis/src/index.ts (1)
11-11
: Enhanced Redis retry configuration for different environments.The change improves resilience in GitHub Actions by increasing
maxRetriesPerRequest
to 50, while maintaining existing behavior for other environments. This is a good approach to handle potential network flakiness in CI/CD pipelines.packages/react-hooks/src/hooks/useTaskTrigger.ts (1)
78-78
: Update to queue structure for task triggeringThe change enhances the queue option by wrapping it in an object with a
name
property instead of passing the string value directly. This aligns with the queue indexing changes mentioned in the PR title and supports a more structured approach to queue management.This change maintains backward compatibility by handling undefined values correctly while allowing for potential future expansion of queue-related properties beyond just the name.
.changeset/twelve-onions-melt.md (2)
1-3
: Confirm Dependency Version UpdateThe changeset now bumps
"@trigger.dev/sdk"
to a patch version. Please verify that this update aligns with the broader changes in the PR and that all affected components have been tested with this new version.
5-5
: Clarify Queue Creation BehaviorThe description now states that "Queues are now created when creating a new version, and no longer can be created 'on-demand'." This clearly communicates the change in queue creation policy. Please ensure that related documentation and release notes also reflect this change for consistency.
.changeset/config.json (2)
10-15
: Improved “fixed” Field FormattingReformatting the “fixed” field into a multi-line array significantly enhances readability and maintainability without affecting functionality. This structured approach makes it easier to manage and review the fixed package definitions.
20-26
: Expansion of “ignore” ListThe addition of
"supervisor"
to the"ignore"
array is clear and consistent with the purpose of excluding specific modules from changeset processing. Please verify that excluding “supervisor” is intended and that it will not inadvertently hide relevant changes in the context of the new “PENDING_VERSION” status and related queue trigger changes.packages/cli-v3/e2e/fixtures.ts (1)
13-13
: Good change to makeexportName
optional in the TestCaseRun interface.This change aligns with the PR objective of transitioning from a task-centric to a resource-centric model. Making this field optional provides more flexibility in how tasks are defined and referenced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/trigger-sdk/src/v3/shared.ts (1)
567-567
: 💡 Verification agent🧩 Analysis chain
Check undefined task metadata and optional queue
IfresourceCatalog.getTask(item.id)
returnsundefined
, the batch trigger might proceed without valid metadata. Ensure unexpected errors or missing queue definitions are handled.Also applies to: 587-587
🏁 Script executed:
#!/bin/bash # Check usage of resourceCatalog.getTask for signs of missing metadata handling rg "resourceCatalog\.getTask" -A 10Length of output: 5389
Action Required: Add explicit error handling for missing task metadata in batch trigger flows
Inpackages/trigger-sdk/src/v3/shared.ts
the code callsresourceCatalog.getTask(item.id)
and immediately proceeds to use its methods (via optional chaining) without confirming that valid task metadata is returned. This risks allowing the batch trigger to continue with an invalid (or missing) task configuration—which may lead to unexpected behavior, especially when compared to the explicit error handling in the CLI code (e.g., logging and aborting if a task isn’t found).Recommendations:
- Guard Clause: Insert a conditional check right after retrieving the task metadata. If the metadata is
undefined
, either throw an error or log a meaningful message and abort the trigger operation.- Consistent Handling: Align the error handling in this module with the approach used in
packages/cli-v3/src/entryPoints/managed-run-worker.ts
anddev-run-worker.ts
, where a lack of task information is explicitly caught and handled.
🧹 Nitpick comments (1)
packages/trigger-sdk/src/v3/shared.ts (1)
110-115
: Remove completed TODO and clarify type for queue
The queue registration is already implemented at line 112, so theTODO
can be removed. Also consider clarifying the type ofqueue
to avoid reliance on@ts-expect-error
.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/webapp/app/presenters/v3/DeploymentPresenter.server.ts
(1 hunks)apps/webapp/app/presenters/v3/SpanPresenter.server.ts
(3 hunks)apps/webapp/app/presenters/v3/TaskPresenter.server.ts
(0 hunks)apps/webapp/app/presenters/v3/TestPresenter.server.ts
(1 hunks)apps/webapp/app/presenters/v3/TestTaskPresenter.server.ts
(0 hunks)apps/webapp/app/routes/api.v1.deployments.$deploymentId.ts
(1 hunks)apps/webapp/app/routes/api.v1.projects.$projectRef.background-workers.$envSlug.$version.ts
(1 hunks)apps/webapp/app/routes/resources.runs.$runParam.ts
(1 hunks)apps/webapp/app/v3/otlpExporter.server.ts
(1 hunks)packages/core/src/v3/schemas/api.ts
(5 hunks)packages/core/src/v3/schemas/webhooks.ts
(2 hunks)packages/trigger-sdk/src/v3/shared.ts
(22 hunks)
💤 Files with no reviewable changes (2)
- apps/webapp/app/presenters/v3/TaskPresenter.server.ts
- apps/webapp/app/presenters/v3/TestTaskPresenter.server.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/webapp/app/presenters/v3/SpanPresenter.server.ts
- packages/core/src/v3/schemas/api.ts
🧰 Additional context used
🧬 Code Definitions (1)
packages/trigger-sdk/src/v3/shared.ts (2)
packages/core/src/v3/types/queues.ts (1) (1)
QueueOptions
(1-43)packages/core/src/v3/resource-catalog-api.ts (1) (1)
resourceCatalog
(5-5)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
🔇 Additional comments (23)
apps/webapp/app/v3/otlpExporter.server.ts (1)
385-385
: Hardcoded deprecation marker replaces dynamic extraction.The
taskExportName
property is now hardcoded to"@deprecated"
instead of being dynamically extracted from the attributes. This aligns with the broader refactoring across the codebase where theexportName
property is being phased out.packages/core/src/v3/schemas/webhooks.ts (2)
14-14
: Schema updated to make exportName optional.The Zod schema is modified to make the
exportName
property optional in theAlertWebhookRunFailedObject
, which is consistent with the deprecation approach taken throughout the codebase.
145-145
: Schema updated to make exportName optional.Similarly, the
exportName
property is now optional in theAlertWebhookDeploymentSuccessObject
. This change maintains backward compatibility while signaling to API consumers that this field is being phased out.apps/webapp/app/routes/api.v1.projects.$projectRef.background-workers.$envSlug.$version.ts (1)
69-69
: Added fallback for deprecated exportName.The code now provides a fallback value of
"@deprecated"
whentask.exportName
is null or undefined. This maintains the API shape while clearly indicating to consumers that this field is being deprecated.apps/webapp/app/routes/api.v1.deployments.$deploymentId.ts (1)
64-64
: Added fallback for deprecated exportName.Similar to other API endpoints, this change ensures that the
exportName
property always returns either its original value or"@deprecated"
when undefined, maintaining backward compatibility while signaling the deprecation.apps/webapp/app/presenters/v3/DeploymentPresenter.server.ts (1)
114-114
: Task ordering updated to use slug instead of exportNameThe ordering criteria for tasks has been changed from using
exportName
to usingslug
. This change aligns with the broader codebase refactoring that's moving away fromexportName
as seen in other files as well.apps/webapp/app/presenters/v3/TestPresenter.server.ts (1)
54-57
: Removed exportName from task selection and updated orderingThe SQL query has been updated to:
- Remove the
exportName
field from selection- Change ordering from
bwt."exportName"
toslug
These changes are consistent with the PR goal of transitioning away from the
exportName
property across the codebase.apps/webapp/app/routes/resources.runs.$runParam.ts (1)
152-152
: exportName marked as deprecatedThe
exportName
field is now hardcoded to"@deprecated"
instead of being populated from the database. This approach maintains backward compatibility while clearly signaling that this field should no longer be used.Consider adding a comment explaining when this field will be fully removed to help consumers prepare for the future change.
packages/trigger-sdk/src/v3/shared.ts (15)
21-21
: Use ofresourceCatalog
import is correct
The import from@trigger.dev/core/v3
forresourceCatalog
aligns well with the new resource-centric approach.
133-133
: No issues with descriptive name
Passing"trigger()"
as a descriptive label for logging and tracing appears consistent with the style used elsewhere.
138-138
: Consistent forwarding of queue string
Extracting onlyparams.queue?.name
ensures the correct string is passed totrigger_internal
. This neatly separates internal queue object logic from the higher-level call site.
145-145
: Batch trigger naming & queue parameter
Using"batchTrigger()"
as the tracing label and passingparams.queue?.name
follows the same pattern as other trigger methods.Also applies to: 151-151
157-157
: Consistent queue usage fortriggerAndWait
Forwardingparams.queue?.name
to the internal trigger function ensures the queue name is handled the same way as in other triggers.Also applies to: 162-162
176-176
: Consistent queue usage forbatchTriggerAndWait
The code consistently setsparams.queue?.name
when invoking batch triggers, maintaining uniform queue handling across operations.Also applies to: 182-182
187-187
: Registering task metadata inresourceCatalog
Switching toresourceCatalog.registerTaskMetadata
aligns with the new resource-based design.
319-319
: Consistent resourceCatalog usage
UsingresourceCatalog.registerTaskMetadata()
increateSchemaTask
is consistent with resource-centric practices.
738-738
: Confirm queue usage inbatchTriggerByIdAndWait
Retrieving the queue fromitem.options
ensures that per-item queue settings are respected.Also applies to: 755-755
899-899
: Task metadata retrieval and queue assignment
The logic checksresourceCatalog
for each task ID and applies the queue object if supplied, maintaining a unified approach for batch triggers.Also applies to: 915-915
1147-1147
: Ensuring payload parse and version lock
Validating payloads and locking tooptions?.version
or theTRIGGER_VERSION
environment variable ensures consistent versioned task runs.Also applies to: 1174-1174
1204-1204
: Parameter-based queue fallback
Allowing a function-levelqueue?: string
param ensures items without customqueue
definitions still run in a controlled queue environment.Also applies to: 1226-1226
1364-1364
: Batch trigger fallback queue
Similar tobatchTrigger_internal
, providing a fallbackqueue?: string
for items lacking aqueue
preserves consistent concurrency rules.Also applies to: 1401-1401
339-346
:❓ Verification inconclusive
Queue registration duplication check
As increateTask
, verifying that re-registering the same queue metadata does not introduce concurrency limit confusion or unexpected overrides would be prudent.
🏁 Script executed:
#!/bin/bash # Searching for multiple calls to `registerQueueMetadata` to verify repeated queue registration scenarios. rg "registerQueueMetadata" -A 2Length of output: 1805
Queue registration duplication: Verify behavior for consistent concurrency limits
The current code registers queue metadata multiple times inpackages/trigger-sdk/src/v3/shared.ts
(evidenced by the repeated calls) and in other parts of the codebase. Please confirm that re-registering the same queue—especially with identical metadata (name and concurrencyLimit)—is either intentionally designed to override prior settings or is safely guarded against duplicate registrations. In particular, compare this behavior with that increateTask
and verify whether adding a duplicate registration check (or documentation on the override semantics) is necessary to prevent any unintended concurrency limit confusion.
206-213
:❓ Verification inconclusive
Ensure no double registration of queue metadata
If a user also callsqueue(...)
directly, it might register the same queue. Double-check whether multiple registrations could cause collisions or overrides.
🏁 Script executed:
#!/bin/bash # Searching for multiple calls to `registerQueueMetadata` to see if the same queue name can be registered multiple times. rg "registerQueueMetadata" -A 2Length of output: 1805
Double Registration Risk for Queue Metadata
- In
packages/trigger-sdk/src/v3/shared.ts
we’ve identified multiple invocations ofresourceCatalog.registerQueueMetadata
—one using anoptions
parameter and at least two using explicit object literals.- Please verify that if a user calls
queue(...)
(or triggers registration via another code path) the same queue isn’t unintentionally registered twice. In particular, check whether duplicate registrations using the same queue name could lead to collisions or unintended overrides (given that in the standard catalog the metadata is stored in a Map, where subsequent calls may simply override previous values).- Confirm if this behavior is intentional (for idempotency) or if additional safeguards (like checking for an existing registration) should be implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/unit-tests.yml (1)
51-51
: Remove trailing spaces to adhere to YAML style.Static analysis detected trailing spaces on this line. Removing them will improve code quality and avoid lint warnings.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 51-51: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/unit-tests.yml
(2 hunks)package.json
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/unit-tests.yml
[error] 51-51: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: units / 🧪 Unit Tests
🔇 Additional comments (3)
.github/workflows/unit-tests.yml (3)
41-41
: Validated update to webapp unit tests command.The command has been updated to use
pnpm run test:webapp
, which aligns with the new testing workflow and syntax improvements described in the PR objectives. Ensure the corresponding test script is defined correctly in your package configuration.
50-50
: Validated update to package unit tests command.The updated command
pnpm run test:packages
is clear and consistent with the new syntax. This change improves the maintainability of the test scripts by providing a dedicated command.
53-53
: Validated update to internal unit tests command.The command
pnpm run test:internal
correctly replaces the previous filter-based approach. This enhances clarity in running tests for internal modules.
…kground worker request bodies
…itpoint to the SDK and the dashboard
2dc0799
to
f46a409
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/core/src/v3/resource-catalog/index.ts (1)
11-22
:⚠️ Potential issueFix the use of
this
in static methods.The singleton pattern implementation is good, but there are issues with using
this
in static methods. This can lead to confusion and potential errors.Apply this fix to use the class name directly:
public static getInstance(): ResourceCatalogAPI { - if (!this._instance) { - this._instance = new ResourceCatalogAPI(); + if (!ResourceCatalogAPI._instance) { + ResourceCatalogAPI._instance = new ResourceCatalogAPI(); } - return this._instance; + return ResourceCatalogAPI._instance; }🧰 Tools
🪛 Biome (1.9.4)
[error] 17-17: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 18-18: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 21-21: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
🧹 Nitpick comments (7)
packages/cli-v3/src/indexing/registerResources.ts (1)
33-33
: Remove unnecessary continue statementThe continue statement at the end of the
if
block is redundant since it's at the end of the loop body and the loop would continue to the next iteration anyway.} - continue; }
🧰 Tools
🪛 Biome (1.9.4)
[error] 33-33: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
.github/workflows/unit-tests.yml (1)
49-51
: Updated Test Command - Package Tests & Trailing Whitespace Correction
The command change topnpm run test:packages
streamlines the command syntax. However, YAMLlint has identified trailing spaces on line 51. Please remove these extra spaces to keep the YAML clean and compliant.Suggested Diff to remove trailing spaces:
- +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 51-51: trailing spaces
(trailing-spaces)
internal-packages/database/prisma/schema.prisma (1)
2576-2579
: New TaskQueueVersion Enum:
The introduction of theTaskQueueVersion
enum (lines 2576–2579) with valuesV1
andV2
offers a mechanism for distinguishing between different versions of queues. WithV1
set as the default, please verify that systems interacting with queues can correctly handle both versions and that any necessary migration or backward compatibility considerations are addressed.packages/trigger-sdk/src/v3/shared.ts (4)
110-111
: Remove or update the TODO comment.
The queue registration is already handled on line 112, so this comment appears outdated.- // TODO register queue here
114-116
: Refine TypeScript approach for Symbol usage.
Currently using@ts-expect-error
. Consider extendingQueueOptions
or adding an interface to handle theSymbol.for("trigger.dev/queue")
property directly.
208-213
: Avoid potential duplication in queue registration.
This logic repeats the same registration call as line 112. Consider centralizing the queue registration to reduce duplication and confusion.
341-346
: Consider avoiding repeated registration logic.
This block duplicates registration code seen above. A shared helper might reduce confusion and potential mismatch.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (14)
packages/cli-v3/e2e/fixtures/emit-decorator-metadata/package-lock.json
is excluded by!**/package-lock.json
packages/cli-v3/e2e/fixtures/emit-decorator-metadata/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
packages/cli-v3/e2e/fixtures/emit-decorator-metadata/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
packages/cli-v3/e2e/fixtures/esm-only-external/package-lock.json
is excluded by!**/package-lock.json
packages/cli-v3/e2e/fixtures/esm-only-external/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
packages/cli-v3/e2e/fixtures/esm-only-external/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
packages/cli-v3/e2e/fixtures/hello-world/package-lock.json
is excluded by!**/package-lock.json
packages/cli-v3/e2e/fixtures/hello-world/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
packages/cli-v3/e2e/fixtures/hello-world/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
packages/cli-v3/e2e/fixtures/monorepo-react-email/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
packages/cli-v3/e2e/fixtures/monorepo-react-email/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
packages/cli-v3/e2e/fixtures/otel-telemetry-loader/package-lock.json
is excluded by!**/package-lock.json
packages/cli-v3/e2e/fixtures/otel-telemetry-loader/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
packages/cli-v3/e2e/fixtures/otel-telemetry-loader/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (125)
.changeset/config.json
(1 hunks).github/workflows/unit-tests.yml
(2 hunks)apps/webapp/app/components/navigation/SideMenu.tsx
(2 hunks)apps/webapp/app/components/primitives/Table.tsx
(1 hunks)apps/webapp/app/components/runs/v3/TaskPath.tsx
(1 hunks)apps/webapp/app/components/runs/v3/TaskRunStatus.tsx
(8 hunks)apps/webapp/app/components/runs/v3/TaskRunsTable.tsx
(1 hunks)apps/webapp/app/database-types.ts
(1 hunks)apps/webapp/app/hooks/useFilterTasks.ts
(0 hunks)apps/webapp/app/models/runtimeEnvironment.server.ts
(1 hunks)apps/webapp/app/models/taskQueue.server.ts
(2 hunks)apps/webapp/app/models/taskRun.server.ts
(1 hunks)apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts
(2 hunks)apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts
(1 hunks)apps/webapp/app/presenters/v3/DeploymentPresenter.server.ts
(1 hunks)apps/webapp/app/presenters/v3/QueueListPresenter.server.ts
(2 hunks)apps/webapp/app/presenters/v3/QueueRetrievePresenter.server.ts
(3 hunks)apps/webapp/app/presenters/v3/RunListPresenter.server.ts
(2 hunks)apps/webapp/app/presenters/v3/SpanPresenter.server.ts
(3 hunks)apps/webapp/app/presenters/v3/TaskListPresenter.server.ts
(2 hunks)apps/webapp/app/presenters/v3/TaskPresenter.server.ts
(0 hunks)apps/webapp/app/presenters/v3/TestPresenter.server.ts
(1 hunks)apps/webapp/app/presenters/v3/TestTaskPresenter.server.ts
(0 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam._index/route.tsx
(7 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments.$deploymentParam/route.tsx
(0 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
(2 hunks)apps/webapp/app/routes/api.v1.deployments.$deploymentId.ts
(1 hunks)apps/webapp/app/routes/api.v1.projects.$projectRef.background-workers.$envSlug.$version.ts
(1 hunks)apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
(2 hunks)apps/webapp/app/routes/resources.runs.$runParam.ts
(1 hunks)apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts
(1 hunks)apps/webapp/app/v3/otlpExporter.server.ts
(1 hunks)apps/webapp/app/v3/services/alerts/deliverAlert.server.ts
(1 hunks)apps/webapp/app/v3/services/createBackgroundWorker.server.ts
(2 hunks)apps/webapp/app/v3/services/createDeployedBackgroundWorker.server.ts
(2 hunks)apps/webapp/app/v3/services/createDeploymentBackgroundWorker.server.ts
(3 hunks)apps/webapp/app/v3/services/createTaskRunAttempt.server.ts
(1 hunks)apps/webapp/app/v3/services/pauseEnvironment.server.ts
(1 hunks)apps/webapp/app/v3/services/pauseQueue.server.ts
(1 hunks)apps/webapp/app/v3/services/triggerTaskV1.server.ts
(1 hunks)apps/webapp/app/v3/services/triggerTaskV2.server.ts
(4 hunks)apps/webapp/app/v3/taskRunHeartbeatFailed.server.ts
(1 hunks)apps/webapp/app/v3/taskStatus.ts
(3 hunks)docs/v3-openapi.yaml
(3 hunks)internal-packages/database/prisma/migrations/20250319222954_add_task_queue_indexing_columns_for_v2/migration.sql
(1 hunks)internal-packages/database/prisma/migrations/20250320085824_add_queue_to_background_worker_task/migration.sql
(1 hunks)internal-packages/database/prisma/migrations/20250320111737_add_pending_version_task_run_status/migration.sql
(1 hunks)internal-packages/database/prisma/migrations/20250320130354_add_many_to_many_relationship_task_queue_background_worker/migration.sql
(1 hunks)internal-packages/database/prisma/migrations/20250320223742_add_status_reason_to_task_run/migration.sql
(1 hunks)internal-packages/database/prisma/schema.prisma
(8 hunks)internal-packages/redis/src/index.ts
(1 hunks)internal-packages/run-engine/src/engine/db/worker.ts
(11 hunks)internal-packages/run-engine/src/engine/index.ts
(6 hunks)internal-packages/run-engine/src/engine/systems/dequeueSystem.ts
(7 hunks)internal-packages/run-engine/src/engine/systems/pendingVersionSystem.ts
(4 hunks)internal-packages/run-engine/src/engine/systems/runAttemptSystem.ts
(1 hunks)internal-packages/run-engine/src/engine/tests/attemptFailures.test.ts
(13 hunks)internal-packages/run-engine/src/engine/tests/batchTrigger.test.ts
(4 hunks)internal-packages/run-engine/src/engine/tests/batchTriggerAndWait.test.ts
(5 hunks)internal-packages/run-engine/src/engine/tests/cancelling.test.ts
(6 hunks)internal-packages/run-engine/src/engine/tests/checkpoints.test.ts
(14 hunks)internal-packages/run-engine/src/engine/tests/delays.test.ts
(7 hunks)internal-packages/run-engine/src/engine/tests/dequeuing.test.ts
(4 hunks)internal-packages/run-engine/src/engine/tests/heartbeats.test.ts
(11 hunks)internal-packages/run-engine/src/engine/tests/notDeployed.test.ts
(0 hunks)internal-packages/run-engine/src/engine/tests/pendingVersion.test.ts
(1 hunks)internal-packages/run-engine/src/engine/tests/priority.test.ts
(3 hunks)internal-packages/run-engine/src/engine/tests/releaseConcurrency.test.ts
(13 hunks)internal-packages/run-engine/src/engine/tests/setup.ts
(7 hunks)internal-packages/run-engine/src/engine/tests/trigger.test.ts
(5 hunks)internal-packages/run-engine/src/engine/tests/triggerAndWait.test.ts
(8 hunks)internal-packages/run-engine/src/engine/tests/ttl.test.ts
(3 hunks)internal-packages/run-engine/src/engine/tests/waitpoints.test.ts
(20 hunks)internal-packages/run-engine/src/engine/types.ts
(1 hunks)internal-packages/run-engine/src/engine/workerCatalog.ts
(1 hunks)internal-packages/run-engine/src/index.ts
(1 hunks)internal-packages/run-engine/tsconfig.src.json
(1 hunks)internal-packages/run-engine/tsconfig.test.json
(1 hunks)internal-packages/testcontainers/src/index.ts
(2 hunks)package.json
(1 hunks)packages/cli-v3/e2e/README.md
(1 hunks)packages/cli-v3/e2e/fixtures.ts
(5 hunks)packages/cli-v3/e2e/fixtures/emit-decorator-metadata/package.json
(1 hunks)packages/cli-v3/e2e/fixtures/esm-only-external/package.json
(2 hunks)packages/cli-v3/e2e/fixtures/hello-world/package.json
(2 hunks)packages/cli-v3/e2e/fixtures/monorepo-react-email/packages/trigger/package.json
(1 hunks)packages/cli-v3/e2e/fixtures/otel-telemetry-loader/package.json
(2 hunks)packages/cli-v3/src/commands/workers/build.ts
(1 hunks)packages/cli-v3/src/dev/devSupervisor.ts
(3 hunks)packages/cli-v3/src/entryPoints/dev-index-worker.ts
(5 hunks)packages/cli-v3/src/entryPoints/dev-run-worker.ts
(6 hunks)packages/cli-v3/src/entryPoints/managed-index-controller.ts
(1 hunks)packages/cli-v3/src/entryPoints/managed-index-worker.ts
(5 hunks)packages/cli-v3/src/entryPoints/managed-run-worker.ts
(6 hunks)packages/cli-v3/src/indexing/registerResources.ts
(1 hunks)packages/cli-v3/src/indexing/registerTasks.ts
(0 hunks)packages/core/src/v3/apiClient/runStream.ts
(1 hunks)packages/core/src/v3/index.ts
(1 hunks)packages/core/src/v3/resource-catalog-api.ts
(1 hunks)packages/core/src/v3/resource-catalog/catalog.ts
(1 hunks)packages/core/src/v3/resource-catalog/index.ts
(1 hunks)packages/core/src/v3/resource-catalog/noopResourceCatalog.ts
(2 hunks)packages/core/src/v3/resource-catalog/standardResourceCatalog.ts
(2 hunks)packages/core/src/v3/schemas/api.ts
(5 hunks)packages/core/src/v3/schemas/build.ts
(2 hunks)packages/core/src/v3/schemas/common.ts
(1 hunks)packages/core/src/v3/schemas/queues.ts
(1 hunks)packages/core/src/v3/schemas/resources.ts
(2 hunks)packages/core/src/v3/schemas/runEngine.ts
(1 hunks)packages/core/src/v3/schemas/schemas.ts
(4 hunks)packages/core/src/v3/schemas/webhooks.ts
(2 hunks)packages/core/src/v3/task-catalog/index.ts
(0 hunks)packages/core/src/v3/tryCatch.ts
(1 hunks)packages/core/src/v3/types/index.ts
(1 hunks)packages/core/src/v3/types/queues.ts
(1 hunks)packages/core/src/v3/types/tasks.ts
(3 hunks)packages/core/src/v3/utils/globals.ts
(2 hunks)packages/core/src/v3/workers/index.ts
(1 hunks)packages/react-hooks/src/hooks/useTaskTrigger.ts
(1 hunks)packages/trigger-sdk/src/v3/schedules/index.ts
(2 hunks)packages/trigger-sdk/src/v3/shared.ts
(22 hunks)references/hello-world/src/trigger/example.ts
(1 hunks)references/hello-world/src/trigger/pendingVersions.ts
(1 hunks)references/hello-world/src/trigger/priority.ts
(1 hunks)references/hello-world/src/trigger/queues.ts
(2 hunks)
💤 Files with no reviewable changes (7)
- internal-packages/run-engine/src/engine/tests/notDeployed.test.ts
- apps/webapp/app/presenters/v3/TaskPresenter.server.ts
- packages/core/src/v3/task-catalog/index.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments.$deploymentParam/route.tsx
- apps/webapp/app/hooks/useFilterTasks.ts
- apps/webapp/app/presenters/v3/TestTaskPresenter.server.ts
- packages/cli-v3/src/indexing/registerTasks.ts
🚧 Files skipped from review as they are similar to previous changes (97)
- internal-packages/run-engine/src/index.ts
- internal-packages/database/prisma/migrations/20250320223742_add_status_reason_to_task_run/migration.sql
- apps/webapp/app/models/taskRun.server.ts
- internal-packages/database/prisma/migrations/20250320111737_add_pending_version_task_run_status/migration.sql
- packages/core/src/v3/types/index.ts
- references/hello-world/src/trigger/priority.ts
- apps/webapp/app/models/runtimeEnvironment.server.ts
- internal-packages/run-engine/tsconfig.test.json
- packages/core/src/v3/schemas/queues.ts
- .changeset/config.json
- internal-packages/run-engine/tsconfig.src.json
- packages/core/src/v3/schemas/common.ts
- apps/webapp/app/presenters/v3/DeploymentPresenter.server.ts
- apps/webapp/app/v3/services/createTaskRunAttempt.server.ts
- packages/cli-v3/src/entryPoints/managed-index-controller.ts
- apps/webapp/app/database-types.ts
- internal-packages/run-engine/src/engine/types.ts
- apps/webapp/app/presenters/v3/QueueListPresenter.server.ts
- internal-packages/run-engine/src/engine/workerCatalog.ts
- packages/core/src/v3/apiClient/runStream.ts
- packages/cli-v3/e2e/fixtures/monorepo-react-email/packages/trigger/package.json
- apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts
- internal-packages/redis/src/index.ts
- packages/react-hooks/src/hooks/useTaskTrigger.ts
- apps/webapp/app/v3/services/pauseEnvironment.server.ts
- apps/webapp/app/routes/api.v1.deployments.$deploymentId.ts
- apps/webapp/app/v3/services/pauseQueue.server.ts
- apps/webapp/app/v3/services/alerts/deliverAlert.server.ts
- apps/webapp/app/models/taskQueue.server.ts
- internal-packages/run-engine/src/engine/systems/runAttemptSystem.ts
- apps/webapp/app/components/primitives/Table.tsx
- packages/core/src/v3/workers/index.ts
- packages/core/src/v3/types/queues.ts
- apps/webapp/app/v3/services/createDeployedBackgroundWorker.server.ts
- references/hello-world/src/trigger/example.ts
- apps/webapp/app/v3/taskRunHeartbeatFailed.server.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
- apps/webapp/app/presenters/v3/QueueRetrievePresenter.server.ts
- references/hello-world/src/trigger/queues.ts
- packages/core/src/v3/tryCatch.ts
- packages/cli-v3/e2e/README.md
- packages/cli-v3/src/dev/devSupervisor.ts
- docs/v3-openapi.yaml
- packages/cli-v3/e2e/fixtures/emit-decorator-metadata/package.json
- packages/core/src/v3/resource-catalog-api.ts
- packages/core/src/v3/schemas/runEngine.ts
- references/hello-world/src/trigger/pendingVersions.ts
- apps/webapp/app/components/runs/v3/TaskRunsTable.tsx
- apps/webapp/app/v3/taskStatus.ts
- apps/webapp/app/components/navigation/SideMenu.tsx
- internal-packages/database/prisma/migrations/20250319222954_add_task_queue_indexing_columns_for_v2/migration.sql
- apps/webapp/app/components/runs/v3/TaskPath.tsx
- apps/webapp/app/presenters/v3/TaskListPresenter.server.ts
- internal-packages/run-engine/src/engine/tests/dequeuing.test.ts
- internal-packages/testcontainers/src/index.ts
- packages/core/src/v3/resource-catalog/catalog.ts
- internal-packages/run-engine/src/engine/tests/batchTriggerAndWait.test.ts
- apps/webapp/app/v3/otlpExporter.server.ts
- internal-packages/run-engine/src/engine/tests/batchTrigger.test.ts
- packages/cli-v3/e2e/fixtures.ts
- apps/webapp/app/v3/services/triggerTaskV1.server.ts
- apps/webapp/app/presenters/v3/SpanPresenter.server.ts
- apps/webapp/app/routes/resources.runs.$runParam.ts
- internal-packages/run-engine/src/engine/tests/priority.test.ts
- apps/webapp/app/presenters/v3/RunListPresenter.server.ts
- internal-packages/run-engine/src/engine/tests/releaseConcurrency.test.ts
- apps/webapp/app/components/runs/v3/TaskRunStatus.tsx
- internal-packages/database/prisma/migrations/20250320130354_add_many_to_many_relationship_task_queue_background_worker/migration.sql
- packages/trigger-sdk/src/v3/schedules/index.ts
- packages/core/src/v3/index.ts
- package.json
- internal-packages/database/prisma/migrations/20250320085824_add_queue_to_background_worker_task/migration.sql
- internal-packages/run-engine/src/engine/tests/waitpoints.test.ts
- packages/core/src/v3/schemas/resources.ts
- packages/cli-v3/e2e/fixtures/hello-world/package.json
- internal-packages/run-engine/src/engine/tests/ttl.test.ts
- packages/core/src/v3/schemas/webhooks.ts
- packages/core/src/v3/resource-catalog/noopResourceCatalog.ts
- internal-packages/run-engine/src/engine/systems/dequeueSystem.ts
- internal-packages/run-engine/src/engine/tests/triggerAndWait.test.ts
- apps/webapp/app/v3/services/triggerTaskV2.server.ts
- apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
- internal-packages/run-engine/src/engine/tests/delays.test.ts
- packages/core/src/v3/schemas/build.ts
- apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts
- apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam._index/route.tsx
- internal-packages/run-engine/src/engine/tests/trigger.test.ts
- packages/core/src/v3/schemas/schemas.ts
- internal-packages/run-engine/src/engine/tests/cancelling.test.ts
- internal-packages/run-engine/src/engine/tests/heartbeats.test.ts
- packages/cli-v3/e2e/fixtures/otel-telemetry-loader/package.json
- packages/core/src/v3/schemas/api.ts
- apps/webapp/app/routes/api.v1.projects.$projectRef.background-workers.$envSlug.$version.ts
- internal-packages/run-engine/src/engine/tests/checkpoints.test.ts
- packages/cli-v3/e2e/fixtures/esm-only-external/package.json
- internal-packages/run-engine/src/engine/tests/attemptFailures.test.ts
🧰 Additional context used
🧬 Code Definitions (11)
packages/core/src/v3/resource-catalog/index.ts (4)
packages/core/src/v3/resource-catalog/noopResourceCatalog.ts (1) (1)
NoopResourceCatalog
(5-53)packages/core/src/v3/resource-catalog/catalog.ts (1) (1)
ResourceCatalog
(4-16)packages/core/src/v3/utils/globals.ts (3) (3)
registerGlobal
(17-33)unregisterGlobal
(41-47)getGlobal
(35-39)packages/core/src/v3/types/tasks.ts (1) (1)
TaskMetadataWithFunctions
(854-870)
packages/core/src/v3/utils/globals.ts (2)
packages/core/src/v3/resource-catalog/catalog.ts (1) (1)
ResourceCatalog
(4-16)packages/core/src/v3/resource-catalog/index.ts (1) (1)
ResourceCatalog
(76-78)
packages/core/src/v3/types/tasks.ts (1)
packages/core/src/v3/types/queues.ts (1) (1)
QueueOptions
(1-43)
internal-packages/run-engine/src/engine/tests/pendingVersion.test.ts (2)
internal-packages/run-engine/src/engine/tests/setup.ts (2) (2)
setupAuthenticatedEnvironment
(20-66)setupBackgroundWorker
(68-272)internal-packages/run-engine/src/engine/index.ts (1) (1)
RunEngine
(56-1327)
packages/cli-v3/src/entryPoints/managed-run-worker.ts (2)
packages/core/src/v3/resource-catalog-api.ts (1) (1)
resourceCatalog
(5-5)packages/core/src/v3/resource-catalog/standardResourceCatalog.ts (1) (1)
StandardResourceCatalog
(11-141)
internal-packages/run-engine/src/engine/systems/pendingVersionSystem.ts (1)
internal-packages/run-engine/src/engine/index.ts (1) (1)
backgroundWorkerId
(1324-1326)
internal-packages/run-engine/src/engine/tests/setup.ts (4)
internal-packages/run-engine/src/engine/index.ts (1) (1)
RunEngine
(56-1327)internal-packages/run-engine/src/index.ts (1) (1)
RunEngine
(1-1)packages/core/src/v3/schemas/common.ts (2) (2)
MachineConfig
(109-113)MachineConfig
(115-115)packages/core/src/v3/schemas/schemas.ts (2) (2)
RetryOptions
(99-126)RetryOptions
(128-128)
packages/trigger-sdk/src/v3/shared.ts (2)
packages/core/src/v3/types/queues.ts (1) (1)
QueueOptions
(1-43)packages/core/src/v3/resource-catalog-api.ts (1) (1)
resourceCatalog
(5-5)
apps/webapp/app/v3/services/createBackgroundWorker.server.ts (1)
packages/core/src/v3/schemas/resources.ts (4) (4)
BackgroundWorkerMetadata
(29-36)BackgroundWorkerMetadata
(38-38)TaskResource
(5-16)TaskResource
(18-18)
internal-packages/run-engine/src/engine/index.ts (1)
internal-packages/run-engine/src/engine/systems/pendingVersionSystem.ts (1) (1)
PendingVersionSystem
(10-121)
packages/cli-v3/src/indexing/registerResources.ts (2)
packages/core/src/v3/schemas/build.ts (4) (4)
BuildManifest
(20-68)BuildManifest
(70-70)ImportTaskFileErrors
(118-118)ImportTaskFileErrors
(120-120)packages/core/src/v3/resource-catalog-api.ts (1) (1)
resourceCatalog
(5-5)
🪛 Biome (1.9.4)
packages/core/src/v3/resource-catalog/index.ts
[error] 17-17: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 18-18: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 21-21: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
packages/cli-v3/src/indexing/registerResources.ts
[error] 33-33: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
🪛 YAMLlint (1.35.1)
.github/workflows/unit-tests.yml
[error] 51-51: trailing spaces
(trailing-spaces)
🔇 Additional comments (122)
packages/cli-v3/src/commands/workers/build.ts (1)
422-422
: Appropriate fallback for deprecated exportName property.Using the nullish coalescing operator to provide a fallback value of "@deprecated" is a good change that aligns with the PR objective of modifying queue management. This ensures that the table output will always have a value for the export field, even when task.exportName is null or undefined, which helps maintain backward compatibility while signaling the deprecation.
apps/webapp/app/v3/services/createDeploymentBackgroundWorker.server.ts (4)
63-82
: Improved error handling for background worker file creationThe code now uses the
tryCatch
utility for better error handling with detailed logging and clear error messages. This is a good improvement for error visibility and debugging.
84-106
: Updated to use the renamedcreateWorkerResources
functionThe function call has been updated from
createBackgroundTasks
tocreateWorkerResources
with proper error handling. This aligns with the PR's objective to modify queue management to only create/update queues at version creation time.
108-122
: Consistent error handling pattern applied to declarative schedulesThe same
tryCatch
pattern has been applied to syncing declarative schedules, maintaining consistency throughout the codebase.
142-160
: Well-designed centralized failure handlingThe new private method
#failBackgroundWorkerDeployment
centralizes deployment failure logic, reducing code duplication and ensuring consistent failure handling. This improves maintainability and reliability of the deployment process.apps/webapp/app/v3/services/createBackgroundWorker.server.ts (8)
199-211
: Renamed and refactored function to improve queue managementThe function has been renamed from
createBackgroundTasks
tocreateWorkerResources
and now takes the entire metadata object rather than just tasks. This change supports the PR objective of modifying queue management to be created/updated only at version creation time.
213-231
: Improved task creation with batch processingProcessing tasks in chunks of 20 improves performance and resource utilization when dealing with a large number of tasks. This is a good optimization.
233-313
: Well-structured task creation with queue associationThe new
createWorkerTask
function properly handles task creation and queue association. It attempts to find an existing queue first, and if not found, creates a virtual queue for the task. The error handling is comprehensive, covering different error scenarios with appropriate logging.
315-340
: Efficient queue processing with chunkingSimilar to task creation, the queue creation process uses chunking for better performance when dealing with many queues. The code returns early if no queues are defined, which is an appropriate optimization.
342-425
: Robust queue creation with proper constraint handlingThe
createWorkerQueue
function thoroughly handles queue creation and updating, respecting organizational and environment concurrency limits. It appropriately handles both the creation of new queues and updating of existing ones, and applies the correct concurrency limits.The function also includes detailed logging which helps with debugging and understanding queue behavior.
101-118
: Consistent error handling for background files creationSimilar to the deployment worker service, this service now uses the
tryCatch
utility for improved error handling with better error visibility and logging.
120-137
: Consistent error handling for worker resourcesThe
tryCatch
pattern is properly applied to worker resources creation, maintaining consistency throughout the codebase.
182-192
: Added support for V2 engine pending version handlingThis additional check and processing ensures that runs are properly scheduled for background workers using the V2 engine. Error handling is properly implemented with detailed logging.
internal-packages/run-engine/src/engine/tests/pendingVersion.test.ts (5)
1-6
: Well-structured imports and setupGood job with organizing imports and including necessary dependencies from both internal packages and Node.js. The code is properly importing
containerTest
which is essential for testing, along with tracing utilities and the RunEngine class.
7-8
: Test timeout configuration is appropriateSetting a longer timeout (60 seconds) for these tests is a good practice, as they involve container setup, multiple async operations, and waiting periods.
9-157
: Well-structured test for background task pending version scenarioThis test case effectively validates the behavior when runs are triggered before a background task is created. The test flow is logical:
- Set up the environment
- Trigger runs
- Verify initial queued status
- Create a worker with mismatched task identifier
- Attempt to dequeue (which should fail)
- Verify runs transition to PENDING_VERSION
- Create the correct worker
- Verify runs transition to PENDING and are properly queued
The test correctly validates the
PENDING_VERSION
status which is a key part of this PR's objectives.
159-318
: Thorough test for queue pending version scenarioThe second test case follows a similar pattern but focuses on missing queues rather than tasks. It correctly:
- Sets up the environment with the task but not the custom queues
- Triggers runs with custom queue names
- Verifies the PENDING_VERSION status
- Adds the custom queues to the background worker
- Confirms the runs transition to PENDING status
- Verifies each queue contains exactly one run
This test effectively validates another important aspect of the new queue indexing and trigger changes.
292-307
: Good queue validation logicThe validation of queue lengths after queue creation is thorough and specific, checking that each custom queue has exactly one run. This ensures the runs were properly re-queued after the queues became available.
Also applies to: 309-314
internal-packages/run-engine/src/engine/tests/setup.ts (7)
68-79
: Good function signature updateChanging the signature from accepting
prisma: PrismaClient
to acceptingengine: RunEngine
aligns well with the PR's objectives. This change enables direct access to the RunEngine's systems, which is necessary for queue management and resolving pending version runs.I also like the addition of
customQueues
to queueOptions, which supports the new test scenarios.
80-91
: Well-implemented version management logicThe approach to finding the latest worker and calculating the next version is robust. By ordering by creation date and taking the first result, you ensure you're working with the most recent worker version, which is important for continuity in the testing environment.
131-170
: Good use of upsert for task queue managementUsing
upsert
for task queues allows the function to handle both creation and update scenarios in a single operation, which aligns with the PR's objective of managing queues at version creation time. The worker connection is properly maintained in both create and update operations.
172-181
: Proper concurrency limit handlingThe code correctly updates queue concurrency limits through the engine's runQueue interface after creating or updating the queue in the database. This ensures consistency between the database state and the runtime queue configuration.
183-222
: Well-implemented custom queues supportThe addition of support for custom queues is important for testing the new queue indexing functionality. The implementation follows the same pattern as the task-based queues, ensuring consistent behavior.
257-259
: Good deployment completionThe call to
engine.scheduleEnqueueRunsForBackgroundWorker(worker.id)
is crucial for activating the new background worker and ensuring that any pending runs are properly processed. This aligns with the PR's objective of resolving runs that are in the PENDING_VERSION state.
274-293
: Well-implemented version calculation helperThe
calculateNextBuildVersion
function is clean and handles all edge cases:
- No previous version exists
- A version exists from today (increment build number)
- A version exists from a different day (start new day with build 1)
This ensures consistent and logical version numbering across test runs.
packages/core/src/v3/utils/globals.ts (2)
3-3
: Rename aligns with resource-centric terminology shift.The import change from
TaskCatalog
toResourceCatalog
properly implements the PR's objective to shift terminology from task-centric to resource-centric patterns.
58-58
:❓ Verification inconclusive
Consistent type definition update.
The property change from
["task-catalog"]
to["resource-catalog"]
completes the refactoring. This is a breaking change that correctly aligns with the architectural shift described in the PR objectives.
🏁 Script executed:
#!/bin/bash # Verify all usages of the previous "task-catalog" global have been updated # to prevent potential runtime errors echo "Searching for any remaining references to 'task-catalog' that might need updating..." rg -e "getGlobal\(\s*['\"]task-catalog['\"]\s*\)" --type ts echo "Verifying implementation of new 'resource-catalog' references..." rg -e "getGlobal\(\s*['\"]resource-catalog['\"]\s*\)" --type tsLength of output: 465
Refactoring update verified – Please confirm with manual review
- The update in
packages/core/src/v3/utils/globals.ts
successfully changed the property from["task-catalog"]
to["resource-catalog"]
.- Our regex searches did not return any remaining references to
task-catalog
and confirmed the usage ofresource-catalog
.- Note that the search commands produced no output, so please manually verify that these changes propagate correctly across all dependent modules, ensuring no runtime errors occur.
packages/cli-v3/src/entryPoints/dev-index-worker.ts (5)
5-17
: Well-structured refactoring from task-centric to resource-centric architectureThe renaming of imported entities and references from
taskCatalog
toresourceCatalog
,StandardTaskCatalog
toStandardResourceCatalog
, andregisterTasks
toregisterResources
shows a consistent architectural shift. This aligns well with the PR objectives of improving queue management.
54-54
: Global catalog implementation correctly updatedThe global catalog setter has been properly updated to use the new resource-centric approach, ensuring that the resource catalog is available throughout the application.
89-89
: Function call properly updated to match new architectureThe bootstrap function correctly uses the new
registerResources
function instead of the previousregisterTasks
, maintaining consistency with the architectural changes.
101-101
: Task listing reference correctly updatedThe code properly uses the new resource catalog to list task manifests, maintaining functionality while aligning with the new architecture.
131-138
: Enhanced message payload with queue manifestsThe addition of
queues: resourceCatalog.listQueueManifests()
in the payload is a good enhancement that aligns with the PR objectives. This change ensures that queue information is properly included in the indexing data.packages/cli-v3/src/entryPoints/dev-run-worker.ts (5)
9-34
: Consistent imports refactoring from task to resource architectureThe import statements have been properly updated to reflect the architectural shift from task-centric to resource-centric approach, with
taskCatalog
replaced byresourceCatalog
andStandardTaskCatalog
replaced byStandardResourceCatalog
.
99-99
: Global resource catalog properly initializedThe initialization of the global resource catalog has been correctly updated to use the new resource-centric approach.
142-143
: Worker manifest registration improvedThe previous task registration approach has been replaced with a cleaner, more direct method of registering the worker manifest with the resource catalog. This simplifies the code and improves the resource management workflow.
259-262
: Enhanced file context management during importsThe addition of file context management with
setCurrentFileContext
andclearCurrentFileContext
surrounding the import operation is an excellent improvement. This ensures proper resource association during the dynamic import process and proper cleanup afterward, preventing context leakage between imports.
296-296
: Task retrieval updated to use resource catalogThe task retrieval has been properly updated to use the resource catalog, maintaining consistency with the architectural changes.
packages/cli-v3/src/indexing/registerResources.ts (3)
1-3
: Well-structured imports for the new resource registration moduleThe imports are clean and focused, bringing in only the necessary components for resource registration functionality.
4-38
: Well-implemented resource registration with proper error handlingThe
registerResources
function effectively iterates through the build manifest files, properly manages file context during imports, and handles potential errors. The error handling is comprehensive, capturing both string-based errors and Error objects.🧰 Tools
🪛 Biome (1.9.4)
[error] 33-33: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
40-50
: Well-structured helper function for import handlingThe
tryImport
helper function is a clean implementation that provides consistent error handling for dynamic imports. The usage of a tuple return type makes it clear what the expected outcome is and simplifies error handling in the calling code.packages/cli-v3/src/entryPoints/managed-run-worker.ts (5)
9-33
: Consistent imports refactoring in managed workerThe imports have been properly updated to reflect the architectural shift from task-centric to resource-centric approach, with
resourceCatalog
replacingtaskCatalog
andStandardResourceCatalog
replacingStandardTaskCatalog
.
110-110
: Global resource catalog properly initialized in managed workerThe global resource catalog initialization has been correctly updated to use the new resource-centric approach, maintaining consistency with other worker implementations.
151-152
: Worker manifest registration simplifiedThe registration of the worker manifest with the resource catalog has been streamlined compared to the previous implementation. This direct registration approach is cleaner and more maintainable.
269-271
: File context management properly implementedThe addition of file context management during task imports ensures proper resource association and cleanup. This implementation mirrors the approach in the development worker, showing consistency across the codebase.
306-306
: Task retrieval updated consistently with architecture changesThe task retrieval has been properly updated to use the resource catalog, maintaining consistency with the architectural changes throughout the codebase.
packages/cli-v3/src/entryPoints/managed-index-worker.ts (5)
5-5
: Import statements reflect transition to resource-centric modelThe imports have been updated to reflect the architectural shift from task-centric to resource-centric. This is consistent with the PR's objective to modify queue management processes.
Also applies to: 10-10, 17-17
54-54
: Global catalog initialization follows resource-centric patternThe initialization has been updated to use
resourceCatalog.setGlobalResourceCatalog(new StandardResourceCatalog())
instead of the previous task-focused approach, maintaining consistency with the architectural changes.
89-89
: Registration function updated to align with resource modelThe registration function has been updated from
registerTasks
toregisterResources
, which is consistent with the broader refactoring effort moving from task management to resource management.
101-101
: Task manifest retrieval updated to use resource catalogThe code now retrieves task manifests through the resource catalog, demonstrating consistent implementation of the architectural change while maintaining the same functionality.
137-137
: Queue manifests now included in the index complete messageThe addition of
queues: resourceCatalog.listQueueManifests()
directly supports the PR's primary objective of modifying queue management to ensure queues are only created or updated at version creation time, similar to task management. This is a key change enabling the new queue indexing functionality.packages/core/src/v3/types/tasks.ts (3)
16-16
: Added import for QueueOptions aligns with refactoring approach.This import supports the shift to leveraging the QueueOptions type from a separate file, which promotes better separation of concerns.
18-18
: Queue type simplification is appropriate.The change from using a utility type (
RequireOne<QueueOptions, "name">
) to directly usingQueueOptions
simplifies the type definition while maintaining the requirement for a name property based on the QueueOptions definition.
710-712
:❓ Verification inconclusive
Breaking change: Queue property in TriggerOptions simplified to string.
Previously, the queue property accepted a complex object (
TaskRunConcurrencyOptions
). Now it only accepts a string name. This is a significant breaking change as mentioned in the PR objectives, but promotes a more structured approach by requiring queues to be pre-defined. The updated comment clearly explains the new PENDING_VERSION state that occurs when a queue doesn't exist.
🏁 Script executed:
#!/bin/bash # Search for code that might be using the old queue object format in trigger options echo "Searching for potential usages of queue as an object in trigger calls..." rg -A 3 -B 3 "trigger\(.*\{.*queue:.*\{" --type tsLength of output: 207
Action Required: Confirm Impact of Queue Property Update
The breaking change in
packages/core/src/v3/types/tasks.ts
(lines 710–712) updates thequeue
property from a complex object (TaskRunConcurrencyOptions
) to a simple string. Our initial automated search for instances where the old object format might still be used in trigger calls returned no results. However, due to the critical nature of this change—and given that the search output was minimal—we recommend that you manually verify that:
- No dependent code still assumes or constructs a queue object.
- The new behavior (i.e., tasks entering the
PENDING_VERSION
state when a non-existent queue is referenced) is consistently handled across the codebase.Please review these areas closely to ensure full compatibility with the updated design.
packages/core/src/v3/resource-catalog/index.ts (4)
1-10
: LGTM: Well-structured constants and imports.The API name constant and imports are well organized, with clear separation between internal and external dependencies.
24-30
: LGTM: Clean global registration and deregistration.The methods for setting and removing the global resource catalog instance utilize the utility functions appropriately.
32-74
: LGTM: Well-implemented delegation pattern.These public methods correctly delegate to the underlying resource catalog instance, maintaining a clean separation of concerns.
76-79
: LGTM: Effective fallback mechanism.The private
#getCatalog()
method correctly handles the case when no global catalog is registered by falling back to the NOOP implementation.packages/core/src/v3/resource-catalog/standardResourceCatalog.ts (7)
1-11
: LGTM: Clean transition from TaskCatalog to ResourceCatalog.The imports and class declaration have been properly updated to reflect the shift from task-centric to resource-centric architecture, aligning with the PR objectives.
15-16
: LGTM: Well-structured private properties for file context and queue metadata.The new private properties clearly define their purpose and types, supporting the expanded functionality of the resource catalog.
18-24
: LGTM: Clean implementation of file context management.The file context methods provide a simple and effective way to track the current file being processed, which is then used during task registration.
26-28
: LGTM: Queue metadata registration.The implementation for registering queue metadata is straightforward and follows the same pattern as the task metadata registration.
39-56
: LGTM: Improved task registration with file context.The enhanced
registerTaskMetadata
method now properly utilizes the current file context, ensuring tasks are properly associated with their source files.
100-102
: LGTM: Simple and effective queue manifest listing.The
listQueueManifests
method correctly returns all registered queue manifests from the map.
30-37
:❓ Verification inconclusive
Verify worker manifest registration behavior.
The
registerWorkerManifest
method only updates file metadata without registering the actual task metadata or functions. Ensure this is intentional and that the tasks are properly registered elsewhere.
🏁 Script executed:
#!/bin/bash # Description: Checking if tasks from worker manifests are registered elsewhere # Look for calls to registerWorkerManifest to understand usage pattern echo "Searching for registerWorkerManifest calls:" rg -A 5 "registerWorkerManifest" --glob="*.ts" --glob="*.js" # Look for code that might register task metadata after worker manifest registration echo -e "\nSearching for potential task registration after worker manifest:" rg -A 10 "registerWorkerManifest.*\n.*registerTaskMetadata" --glob="*.ts" --glob="*.js"Length of output: 3716
Action: Confirm Task Registration Flow
It appears that the
registerWorkerManifest
method inpackages/core/src/v3/resource-catalog/standardResourceCatalog.ts
currently only updates task file metadata (storingfilePath
andentryPoint
for each task). I did not find any immediate evidence in the repository—such as a corresponding call to aregisterTaskMetadata
or similar function—indicating that task functions or additional task metadata are being registered in this method.Please verify that:
- This behavior is intentional, and task registration is meant to be handled elsewhere (for example, in subsequent initialization routines or within CLI entry points).
- The tasks declared in the worker manifest are fully registered by another component in the application.
.github/workflows/unit-tests.yml (2)
40-42
: Updated Test Command - Webapp Tests
The step now uses the dedicated commandpnpm run test:webapp
instead of filtering which improves clarity and consistency. Make sure that any related documentation or scripts follow this updated syntax.
52-54
: Updated Test Command - Internal Tests
Switching topnpm run test:internal
aligns the command with the new syntax pattern used across the workflow. Confirm that all related configurations and documentation are updated accordingly.apps/webapp/app/presenters/v3/TestPresenter.server.ts (1)
54-57
: SQL query update correctly removes exportName references.The query has been properly updated to remove the
exportName
field from the selection and change the ordering frombwt."exportName"
toslug
, which aligns with the PR's goal of shifting from task- to resource-centric patterns.internal-packages/run-engine/src/engine/systems/pendingVersionSystem.ts (8)
4-8
: Renamed options type correctly reflects the system's new purpose.The rename from
WaitingForWorkerSystemOptions
toPendingVersionSystemOptions
with corresponding property renames aligns with the PR's objective to replace theWAITING_FOR_DEPLOY
status withPENDING_VERSION
.
10-17
: Class rename provides better semantic clarity.Changing from
WaitingForWorkerSystem
toPendingVersionSystem
improves the code readability and better reflects the system's purpose of managing pending version statuses rather than merely waiting for workers.
19-23
: Method renamed for clarity and consistency.The method rename from
enqueueRunsWaitingForWorker
toenqueueRunsForBackgroundWorker
better describes its actual functionality and maintains consistency with the overall naming changes.
35-37
: Now includes queues in background worker query.Added
queues: true
to the include section, which is crucial for the updated filtering logic that will match task runs with appropriate queues.
47-51
: Enhanced logging provides better operational visibility.The updated logging now includes task identifiers and queue information, improving the ability to troubleshoot issues with pending version runs.
53-63
: Updated query properly filters by task identifier and queue.The query now uses
PENDING_VERSION
status instead ofWAITING_FOR_DEPLOY
and adds crucial filtering by queue names from the background worker. This ensures that only runs with matching task identifiers and queues are processed.
74-84
: Detailed logging enhances traceability.The enhanced logging provides valuable context about each run being enqueued, including taskIdentifier, queue, creation time, and priority. This will greatly improve debugging capabilities.
114-120
: Job name updated to match the new system purpose.The job name in the worker.enqueue call has been correctly updated from "queueRunsWaitingForWorker" to "queueRunsPendingVersion", maintaining consistency with the renamed system.
internal-packages/run-engine/src/engine/index.ts (6)
51-51
: Updated import to use the new PendingVersionSystem.Correctly imports the renamed system, ensuring consistent naming throughout the codebase.
76-76
: Renamed system property for consistency.The property has been appropriately renamed from
waitingForWorkerSystem
topendingVersionSystem
to match the system's new name.
151-155
: Updated job handler to use the new system method.The job handler for "queueRunsPendingVersion" now correctly calls
pendingVersionSystem.enqueueRunsForBackgroundWorker()
, ensuring proper handling of pending version runs.
269-272
: Correctly initializes the new PendingVersionSystem.The constructor now properly initializes the
pendingVersionSystem
with the required resources and enqueue system dependencies.
727-729
: Method renamed for clarity and delegates to the new system.The method has been appropriately renamed from
queueRunsWaitingForWorker
toscheduleEnqueueRunsForBackgroundWorker
and now delegates to thependingVersionSystem
'sscheduleResolvePendingVersionRuns
method.
64-65
:❓ Verification inconclusive
Modified property visibility from private to public.
Changing
prisma
andrunQueue
from private to public properties increases their accessibility. This may be necessary for the new architecture, but ensure this doesn't unintentionally expose these properties to inappropriate external usage.
🏁 Script executed:
#!/bin/bash # Check if the exposed properties are referenced from outside the class echo "Checking for external references to RunEngine.prisma and RunEngine.runQueue" rg -A 3 -B 3 "engine\.(prisma|runQueue)" --type tsLength of output: 54950
Public Property Exposure – Verify Intentional Usage
The change makingprisma
andrunQueue
public is clearly reflected in extensive external references across test files (e.g., in several files underinternal-packages/run-engine/src/engine/tests/
) and production usage (e.g., inapps/webapp/app/v3/runQueue.server.ts
). While this aligns with the new architecture’s requirements, please confirm that:
- The public accessibility of these properties is intentional and that downstream consumers are correctly handling these instances.
- Proper safeguards (such as documentation or additional encapsulation, if necessary) are in place to prevent accidental misuse or modification from external sources.
internal-packages/run-engine/src/engine/db/worker.ts (10)
34-36
: Added new error code for queue not found condition.The addition of "QUEUE_NOT_FOUND" to the error codes enhances error handling capabilities, aligning with the PR's focus on improved queue management.
54-54
: Success result type now includes queue information.Added
queue: TaskQueue
to the successful result type, providing necessary queue data to callers of this function. This enables proper queue handling throughout the system.
164-173
: Added queue validation with appropriate error handling.This new code section properly validates that the queue specified in the run exists in the worker's queues. If not found, it returns a clear error, preventing issues with invalid queue references.
180-181
: Updated successful return to include queue information.The successful return object now includes the queue, providing callers with the queue data they need to proceed with run execution.
188-188
: Updated type definition to include queues.The
WorkerDeploymentWithWorkerTasks
type now includesqueues: TaskQueue[]
, ensuring proper typing throughout the codebase.
203-203
: Added queues to worker query.Added
queues: true
to the include section of the worker query, ensuring that queue data is loaded when needed.
211-211
: Updated return object to include queues.The return object from
getWorkerDeploymentFromWorker
now correctly includes queues, providing callers with complete worker information.
224-224
: Updated getMostRecentWorker to include queues.Both the query and return object in
getMostRecentWorker
now handle queues, maintaining consistency with the other worker retrieval functions.Also applies to: 235-235
249-249
: Updated getWorkerById to include queues.Both the query and return object in
getWorkerById
now handle queues, ensuring consistent queue handling across the codebase.Also applies to: 260-260
280-280
: Updated promoted deployment worker retrieval to include queues.Both the query and return object in
getWorkerFromCurrentlyPromotedDeployment
now include queues, completing the queue-related changes across all worker retrieval functions.Also applies to: 295-295
internal-packages/database/prisma/schema.prisma (5)
1617-1617
: New Relation Field in BackgroundWorker:
The addition of thequeues TaskQueue[]
field to theBackgroundWorker
model establishes an implicit many-to-many relation with task queues. Please verify that this relation is correctly configured (including the inverse relation inTaskQueue
, if desired) and that appropriate indexing/migration steps are in place.
2544-2546
: Enhancements in TaskQueue Model:
The addition of theversion TaskQueueVersion @default(V1)
andorderableName String?
fields in theTaskQueue
model (lines 2544–2546) provides explicit versioning and optional ordering for queues. This change supports a more structured approach to queue management. Please confirm that these new fields integrate well with existing queue creation and update workflows and that a proper migration plan exists if there are pre-existing records.
2565-2567
: Updated Relation Fields in TaskQueue:
The modifications to thetasks
andworkers
relation fields in theTaskQueue
model (lines 2565–2567)—now marked with a tilde—appear intended to support the new queue versioning and associated background worker relationships. Please review these changes to ensure they maintain relational consistency and meet the new requirements for static queue definitions.
1659-1661
:❓ Verification inconclusive
Updated BackgroundWorkerTask Queue Attributes:
In theBackgroundWorkerTask
model, theexportName
field is now made optional (line 1660), and new queue-related fields (queueId
and its relationqueue
) have been introduced (lines 1684–1686). This update aligns with the PR’s objective of deprecating on-demand queue updates in favor of a pre-defined structure. Please ensure that downstream logic gracefully handles a missingexportName
and that existing records are migrated appropriately to includequeueId
when applicable.Also applies to: 1684-1686
Action: Validate Schema Updates in BackgroundWorkerTask
Upon reviewing the changes in
internal-packages/database/prisma/schema.prisma
, please verify that the following updates have been correctly implemented:
Optional
exportName
:
TheexportName
field in theBackgroundWorkerTask
model is now marked optional (see line 1660). Confirm that all downstream logic can handle a missing value without causing failures.New Queue Attributes:
New fields (queueId
and its corresponding relationqueue
) were introduced (see lines 1684–1686) to transition from on-demand queue updates to a pre-defined queue structure. Ensure that:
- Existing records are updated to include a valid
queueId
where applicable, and- The new queue relationships are properly integrated within the database and application logic.
If not already done, please add migration tests and runtime validations to confirm that these schema changes integrate seamlessly with the rest of the system.
1907-1911
:✅ Verification successful
TaskRunStatus Enum Update – Introducing PENDING_VERSION:
The enum changes introducePENDING_VERSION
(lines 1907–1910) to replaceWAITING_FOR_DEPLOY
as the more descriptive status for task runs pending necessary version information. Note that whileWAITING_FOR_DEPLOY
remains (now deprecated), consumers of this status should transition to usingPENDING_VERSION
. Ensure that all parts of the system (including documentation and client SDKs) are updated accordingly.
🏁 Script executed:
#!/bin/bash # Verify that the schema.prisma file in the internal-packages/database/prisma directory contains the expected enum entries. # This script checks for the presence of 'PENDING_VERSION' and 'WAITING_FOR_DEPLOY' as well as the 'TaskRunStatus' enum definition. echo "Searching for 'PENDING_VERSION' in internal-packages/database/prisma/schema.prisma..." rg -n "PENDING_VERSION" internal-packages/database/prisma/schema.prisma echo "Searching for 'WAITING_FOR_DEPLOY' in internal-packages/database/prisma/schema.prisma..." rg -n "WAITING_FOR_DEPLOY" internal-packages/database/prisma/schema.prisma echo "Searching for the 'TaskRunStatus' enum definition in internal-packages/database/prisma/schema.prisma..." rg -n "enum TaskRunStatus" internal-packages/database/prisma/schema.prismaLength of output: 1165
Enum Update Verified – Proceed with Refactoring Documentation and SDKs
The
TaskRunStatus
enum now correctly includes the newPENDING_VERSION
status (line 1908) and retains the deprecatedWAITING_FOR_DEPLOY
(line 1911) with appropriate commentary. Please ensure that all related documentation, client SDKs, and any consumer references are updated to utilizePENDING_VERSION
going forward.
- Internal Update: The enum entries are correctly implemented and comments clearly indicate the deprecation of
WAITING_FOR_DEPLOY
.- Action Required: Update downstream consumers and documentation to align with the new naming.
packages/trigger-sdk/src/v3/shared.ts (26)
21-21
: Import usage looks good.
No issues spotted with introducingresourceCatalog
in the import list.
112-112
: Queue registration call is appropriate.
No concerns; this ensures queue metadata is registered immediately.
133-133
: Explicit name for internal trigger reference is fine.
No issues with passing"trigger()"
as the operation label.
138-138
: Queue parameter looks good.
Assigningqueue: params.queue?.name
is consistent with the new approach.
145-145
: Batch trigger label updated.
No concerns with using"batchTrigger()"
as the internal name.
151-151
: Queue reference for batchTrigger is consistent.
157-157
: triggerAndWait operation label is clear.
162-162
: Queue parameter in triggerAndWait is correct.
187-187
: Registering task metadata.
Ensures each task is tracked in the resource catalog. Looks good.
206-206
: Local queue variable.
Storingparams.queue
in a variable improves clarity.
264-264
: "trigger()" name for createSchemaTask is good.
No concerns.
269-269
: Queue assignment for createSchemaTask trigger is consistent.
277-277
: "batchTrigger()" label is aligned with createTask pattern.
283-283
: Consistent queue assignment in batchTrigger.
289-289
: Naming for triggerAndWait is clear in createSchemaTask.
294-294
: Preserving queue reference is correct.
308-308
: "batchTriggerAndWait()" label is appropriate.
315-315
: Queue assignment for batchTriggerAndWait remains consistent.
319-319
: Task metadata registration in createSchemaTask is valid.
339-339
: Localizing queue at this point is harmless.
588-588
: Queue property assignment in batch items is correct.
No issues spotted.
736-736
: Mapping items for batch trigger logic looks reasonable.
No concerns.
738-738
: Double-check task metadata retrieval.
Again, ensure you handle the case whengetTask(item.id)
is undefined.
755-755
: Queue assignment in batch trigger options is aligned with naming.
899-899
: Confirm that the task is registered before retrieving metadata.
If a user references a nonexistent task, we need to ensure a graceful failure mode.
915-915
: Queue assignment in batch trigger by tasks is consistent.
queue?: { | ||
name?: string; | ||
concurrencyLimit?: number; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Breaking change: Queue configuration simplified in CommonTaskOptions.
The queue property has been simplified to only include name
and concurrencyLimit
, removing the releaseConcurrencyOnWaitpoint
property. This aligns with the PR's objective to make queue management more structured but may break existing code that uses this property.
🏁 Script executed:
#!/bin/bash
# Check for existing usage of releaseConcurrencyOnWaitpoint in the codebase
echo "Searching for releaseConcurrencyOnWaitpoint usage in task definitions..."
rg -A 2 -B 2 "releaseConcurrencyOnWaitpoint" --type ts
Length of output: 7951
Action required: Update all usages of the removed releaseConcurrencyOnWaitpoint
property
The latest changes in the packages/core/src/v3/types/tasks.ts
file simplify the queue
configuration to only include name
and concurrencyLimit
. However, our search shows that releaseConcurrencyOnWaitpoint
is still referenced in several parts of the codebase. Please address the following:
-
Tests:
internal-packages/run-engine/src/engine/tests/setup.ts
internal-packages/run-engine/src/engine/tests/releaseConcurrency.test.ts
-
Business Logic:
internal-packages/run-engine/src/engine/systems/releaseConcurrencySystem.ts
-
Type Definitions/Schemas:
packages/core/src/v3/types/queues.ts
packages/core/src/v3/schemas/queues.ts
packages/core/src/v3/schemas/schemas.ts
-
Frontend and Server Code:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
apps/webapp/app/v3/services/pauseQueue.server.ts
apps/webapp/app/presenters/v3/QueueListPresenter.server.ts
apps/webapp/app/presenters/v3/QueueRetrievePresenter.server.ts
Ensure that these usages are refactored to remove or replace the dependency on releaseConcurrencyOnWaitpoint
so that the codebase correctly aligns with the new queue configuration.
@@ -590,7 +568,7 @@ export async function batchTriggerById<TTask extends AnyTask>( | |||
{ | |||
items: await Promise.all( | |||
items.map(async (item, index) => { | |||
const taskMetadata = taskCatalog.getTask(item.id); | |||
const taskMetadata = resourceCatalog.getTask(item.id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Validate retrieved task metadata.
If getTask(item.id)
returns undefined, consider handling or throwing a descriptive error.
🏁 Script executed:
#!/bin/bash
# Searching for usage of resourceCatalog.getTask across the codebase
rg --context 5 'resourceCatalog\.getTask\('
Length of output: 5097
Action: Validate Task Metadata Handling in shared.ts
It appears that in packages/trigger-sdk/src/v3/shared.ts
(line 571) the code retrieves task metadata via resourceCatalog.getTask(item.id)
and subsequently uses optional chaining (i.e. taskMetadata?.fns.parsePayload
) to determine the payload. In contrast, similar calls in the CLI worker files (such as in packages/cli-v3/src/entryPoints/managed-run-worker.ts
and dev-run-worker.ts
) explicitly verify that the retrieved task isn’t undefined and log an error when it is.
Please consider one of the following:
- Introduce explicit error handling: If a missing task metadata is unexpected, throw a descriptive error or log a clear message.
- Clarify the intended behavior: If falling back to
item.payload
upon a missing task is by design, add an inline comment to explain why this variance from other parts of the codebase is acceptable.
Previous to this PR, queues could be created and updated "on-demand" when triggering a task:
This could mean that at any point, any single trigger of a task could completely change the concurrency limit of a queue, and it when it did happen, was difficult to track down exact why the queue concurrency limit changed.
This dynamic queue creation/updating behavior also was confusing for users, as it made it more difficult to explain when to use a queue and when to use a concurrency queue.
This PR changes the way queues are created and updated to only happen when a new version is created (index time), very similar to how tasks work.
This PR also introduces a breaking SDK change, because now at trigger time you can only specify the queue name:
We will also now encourage users to pre-define their queues in code, like so:
For backwards compat reasons, you can still define a custom queue on the task without using
queue
:PENDING_VERSION and queues
There is a new TaskRun status that is meant to replace WAITING_FOR_DEPLOY in this PR called PENDING_VERSION. TaskRun's can be put into this state when one of the following happens:
taskIdentifier
of the TaskRun does not exist in the latest versionqueue
of the TaskRun does not exist in the latest versionSummary by CodeRabbit
New Features
UI Enhancements