Conversation
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (27)
WalkthroughIntroduces a TaskIdentifier registry: new Prisma model/table, migrations, and a covering index; a Redis-backed cache and cache helpers; and services Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts (1)
117-119: Add deployment context fields to the error log.Include key identifiers in this log payload to simplify production debugging.
Proposed diff
- if (syncError) { - logger.error("Error syncing task identifiers on deployment change", { error: syncError }); - } + if (syncError) { + logger.error("Error syncing task identifiers on deployment change", { + error: syncError, + deploymentId: deployment.id, + environmentId: deployment.environmentId, + projectId: deployment.projectId, + workerId: deployment.workerId, + direction, + }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/v3/services/changeCurrentDeployment.server.ts` around lines 117 - 119, The logger.error call that logs syncError in changeCurrentDeployment should include deployment context fields to aid debugging: add identifying properties (e.g., deployment.id, deployment.name, projectId, environment or similar variables available in scope) to the payload object passed to logger.error alongside error: syncError; update the call site where logger.error("Error syncing task identifiers on deployment change", { error: syncError }) to include these context keys so logs contain both the error and the relevant deployment identifiers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/webapp/app/v3/services/changeCurrentDeployment.server.ts`:
- Around line 117-119: The logger.error call that logs syncError in
changeCurrentDeployment should include deployment context fields to aid
debugging: add identifying properties (e.g., deployment.id, deployment.name,
projectId, environment or similar variables available in scope) to the payload
object passed to logger.error alongside error: syncError; update the call site
where logger.error("Error syncing task identifiers on deployment change", {
error: syncError }) to include these context keys so logs contain both the error
and the relevant deployment identifiers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c7ab9ef2-d485-49f8-a75a-4b68be3bcde5
📒 Files selected for processing (2)
apps/webapp/app/services/taskIdentifierRegistry.server.tsapps/webapp/app/v3/services/changeCurrentDeployment.server.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webapp/app/services/taskIdentifierRegistry.server.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
apps/webapp/app/v3/services/**/*.server.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Organize services in the webapp following the pattern
app/v3/services/*/*.server.ts
Files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
**/*.ts: Usetypecheckto verify changes in apps and internal packages (apps/*,internal-packages/*), notbuild- building proves almost nothing about correctness
When writing Trigger.dev tasks, always import from@trigger.dev/sdk. Never use@trigger.dev/sdk/v3or deprecatedclient.defineJob
Add crumbs as you write code - mark lines with//@Crumbsor wrap blocks in `// `#region` `@crumbsfor agentcrumbs debug tracing, then strip before merge
Files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
apps/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
When modifying only server components (
apps/webapp/,apps/supervisor/, etc.) with no package changes, add a.server-changes/file instead of a changeset
Files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
apps/webapp/**/*.server.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/**/*.server.{ts,tsx}: Environment variables must be accessed via theenvexport fromapp/env.server.tsand never useprocess.envdirectly
Always usefindFirstinstead offindUniquein Prisma queries due to implicit DataLoader batching issues and performance concerns
Files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
apps/webapp/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Use named constants for sentinel/placeholder values instead of raw string literals scattered across comparisons
Files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
🧠 Learnings (19)
📓 Common learnings
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/app/services/taskIdentifierRegistry.server.ts:49-58
Timestamp: 2026-04-13T21:43:58.707Z
Learning: In `triggerdotdev/trigger.dev`, a deployment with zero tasks is not a realistic scenario in practice. Do not flag missing handling for empty-task deployments in `apps/webapp/app/services/taskIdentifierRegistry.server.ts` or related registry/sync logic.
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/app/services/taskIdentifierRegistry.server.ts:24-67
Timestamp: 2026-04-13T21:43:58.548Z
Learning: In `apps/webapp/app/services/taskIdentifierRegistry.server.ts`, the sequential upsert/updateMany/findMany writes in `syncTaskIdentifiers` are intentionally NOT wrapped in a Prisma transaction. This function runs only during deployment-change events (low-concurrency path), and any partial `isInLatestDeployment` state is acceptable because it self-corrects on the next deployment. Do not flag this as a missing-transaction/atomicity issue in future reviews.
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/webapp/CLAUDE.md:0-0
Timestamp: 2026-03-30T22:25:33.107Z
Learning: Applies to apps/webapp/{app/v3/services/triggerTask.server.ts,app/v3/services/batchTriggerV3.server.ts} : Do NOT add database queries to `triggerTask.server.ts` or `batchTriggerV3.server.ts`; instead piggyback on the existing `backgroundWorkerTask.findFirst()` query in `queues.server.ts`
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: internal-packages/database/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:17.177Z
Learning: Applies to internal-packages/database/**/prisma/migrations/*/*.sql : Clean up generated Prisma migrations by removing extraneous lines for junction tables (`_BackgroundWorkerToBackgroundWorkerFile`, `_BackgroundWorkerToTaskQueue`, `_TaskRunToTaskRunTag`, `_WaitpointRunConnections`, `_completedWaitpoints`) and indexes (`SecretStore_key_idx`, various `TaskRun` indexes) unless explicitly added
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: internal-packages/database/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:17.177Z
Learning: New code should always target Prisma RunEngineVersion V2 (run-engine + redis-worker), not V1 (legacy MarQS + Graphile)
📚 Learning: 2026-04-13T21:43:58.548Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/app/services/taskIdentifierRegistry.server.ts:24-67
Timestamp: 2026-04-13T21:43:58.548Z
Learning: In `apps/webapp/app/services/taskIdentifierRegistry.server.ts`, the sequential upsert/updateMany/findMany writes in `syncTaskIdentifiers` are intentionally NOT wrapped in a Prisma transaction. This function runs only during deployment-change events (low-concurrency path), and any partial `isInLatestDeployment` state is acceptable because it self-corrects on the next deployment. Do not flag this as a missing-transaction/atomicity issue in future reviews.
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-04-13T21:43:58.707Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/app/services/taskIdentifierRegistry.server.ts:49-58
Timestamp: 2026-04-13T21:43:58.707Z
Learning: In `triggerdotdev/trigger.dev`, a deployment with zero tasks is not a realistic scenario in practice. Do not flag missing handling for empty-task deployments in `apps/webapp/app/services/taskIdentifierRegistry.server.ts` or related registry/sync logic.
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-03-30T22:25:33.107Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/webapp/CLAUDE.md:0-0
Timestamp: 2026-03-30T22:25:33.107Z
Learning: Applies to apps/webapp/{app/v3/services/triggerTask.server.ts,app/v3/services/batchTriggerV3.server.ts} : Do NOT add database queries to `triggerTask.server.ts` or `batchTriggerV3.server.ts`; instead piggyback on the existing `backgroundWorkerTask.findFirst()` query in `queues.server.ts`
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-03-02T12:43:34.140Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: packages/cli-v3/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:34.140Z
Learning: Applies to packages/cli-v3/src/commands/promote.ts : Implement `promote.ts` command in `src/commands/` for deployment promotion
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-03-30T22:25:33.107Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/webapp/CLAUDE.md:0-0
Timestamp: 2026-03-30T22:25:33.107Z
Learning: Applies to apps/webapp/app/v3/queues.server.ts : When adding new task-level defaults, add to the existing `select` clause in `backgroundWorkerTask.findFirst()` query in `queues.server.ts` instead of adding a second query
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-03-02T12:43:34.140Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: packages/cli-v3/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:34.140Z
Learning: Applies to packages/cli-v3/src/commands/deploy.ts : Implement `deploy.ts` command in `src/commands/` for production deployment
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-03-31T21:37:31.732Z
Learnt from: isshaddad
Repo: triggerdotdev/trigger.dev PR: 3283
File: docs/migration-n8n.mdx:19-21
Timestamp: 2026-03-31T21:37:31.732Z
Learning: In the trigger.dev SDK (`packages/trigger-sdk/src/v3`), `tasks.triggerAndWait()` and `tasks.batchTriggerAndWait()` are real, valid exported APIs defined in `shared.ts` and re-exported via the `tasks` object in `tasks.ts`. They accept a task ID string as their first argument (not a task instance). These are distinct from the instance methods `yourTask.triggerAndWait()` and `yourTask.batchTriggerAndWait()`. Do not flag `tasks.triggerAndWait()` or `tasks.batchTriggerAndWait()` as non-existent APIs.
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-03-25T15:29:25.889Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2026-03-25T15:29:25.889Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `task()` from `trigger.dev/sdk` for basic task definitions with `id` and `run` properties
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-03-25T15:29:25.889Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2026-03-25T15:29:25.889Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `idempotencyKeys.create()` to create idempotency keys for task triggering to ensure idempotent operations
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-03-02T12:43:37.906Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: packages/core/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:37.906Z
Learning: Exercise caution with changes to trigger.dev/core as they affect both the customer-facing SDK and server-side webapp - breaking changes can impact deployed user tasks and the platform simultaneously
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-03-26T10:02:25.354Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3254
File: apps/webapp/app/services/platformNotifications.server.ts:363-385
Timestamp: 2026-03-26T10:02:25.354Z
Learning: In `triggerdotdev/trigger.dev`, the `getNextCliNotification` fallback in `apps/webapp/app/services/platformNotifications.server.ts` intentionally uses `prisma.orgMember.findFirst` (single org) when no `projectRef` is provided. This is acceptable for v1 because the CLI (`dev` and `login` commands) always passes `projectRef` in normal usage, making the fallback a rare edge case. Do not flag the single-org fallback as a multi-org correctness bug in this file.
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-03-29T21:04:38.173Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-29T21:04:38.173Z
Learning: Applies to **/*.ts : When writing Trigger.dev tasks, always import from `trigger.dev/sdk`. Never use `trigger.dev/sdk/v3` or deprecated `client.defineJob`
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-03-30T22:25:33.107Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/webapp/CLAUDE.md:0-0
Timestamp: 2026-03-30T22:25:33.107Z
Learning: Applies to apps/webapp/app/v3/**/*Worker.server.ts : Do NOT add new background jobs using zodworker/graphile-worker; use `trigger.dev/redis-worker` instead
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-03-02T12:43:43.173Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: packages/redis-worker/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:43.173Z
Learning: Applies to packages/redis-worker/**/*@(job|queue|worker|background).{ts,tsx} : Use trigger.dev/redis-worker for all new background job implementations, replacing graphile-worker and zodworker
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-03-10T17:56:20.938Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3201
File: apps/webapp/app/v3/services/setSeatsAddOn.server.ts:25-29
Timestamp: 2026-03-10T17:56:20.938Z
Learning: Do not implement local userId-to-organizationId authorization checks inside org-scoped service classes (e.g., SetSeatsAddOnService, SetBranchesAddOnService) in the web app. Rely on route-layer authentication (requireUserId(request)) and org membership enforcement via the _app.orgs.$organizationSlug layout route. Any userId/organizationId that reaches these services from org-scoped routes has already been validated. Apply this pattern across all org-scoped services to avoid redundant auth checks and maintain consistency.
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
📚 Learning: 2026-03-29T19:16:28.864Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3291
File: apps/webapp/app/v3/featureFlags.ts:53-65
Timestamp: 2026-03-29T19:16:28.864Z
Learning: When reviewing TypeScript code that uses Zod v3, treat `z.coerce.*()` schemas as their direct Zod type (e.g., `z.coerce.boolean()` returns a `ZodBoolean` with `_def.typeName === "ZodBoolean"`) rather than a `ZodEffects`. Only `.preprocess()`, `.refine()`/`.superRefine()`, and `.transform()` are expected to wrap schemas in `ZodEffects`. Therefore, in reviewers’ logic like `getFlagControlType`, do not flag/unblock failures that require unwrapping `ZodEffects` when the input schema is a `z.coerce.*` schema.
Applied to files:
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
🔇 Additional comments (2)
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts (2)
1-2: Import changes are aligned with webapp conventions.Using
@trigger.dev/coresubpath imports here looks correct.As per coding guidelines, "When importing from
@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path."Also applies to: 4-5
102-115: Sync-before-enqueue flow is solid and failure-tolerant.Wrapping identifier sync with
tryCatchand continuing keeps deployment changes from being blocked by registry/cache transient failures.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
ericallam
force-pushed
the
feature/tri-8441-taskidentifier-registry-eliminate-728-db-runtime-from
branch
from
c0ceb90 to
1114577
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…getAllTaskIdentifiers refs TRI-8441
…eploy call - Remove AGENT case from toTriggerSource (not yet on this branch) - Restore accidentally removed ExecuteTasksWaitingForDeployService.enqueue call in ChangeCurrentDeploymentService
ericallam
force-pushed
the
feature/tri-8441-taskidentifier-registry-eliminate-728-db-runtime-from
branch
from
1114577 to
2b365a1
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/webapp/app/components/logs/LogsTaskFilter.tsx`:
- Around line 135-163: The SelectItem components in LogsTaskFilter (rendering
items from filtered) use key={item.slug} which causes React to reuse nodes when
the same slug appears with different triggerSource; update the keys to a
composite identifier that includes triggerSource (e.g.,
`${item.slug}-${item.triggerSource}`) for both the active map and the archived
map (the SelectItem in the block filtering !item.isInLatestDeployment), and
apply the same composite-key change to the analogous SelectItem usages in
RunFilters.tsx; this ensures unique React keys when getAllTaskIdentifiers() can
return the same slug with multiple triggerSource values.
In `@apps/webapp/app/services/taskIdentifierCache.server.ts`:
- Around line 33-39: The decode function trusts JSON casts and can accept
invalid/stale Redis payloads; replace that with a Zod validation: define a
CachedTaskIdentifier Zod schema (matching fields s, ts, live), parse the raw
JSON using JSON.parse then schema.safeParse, and if valid return a
TaskIdentifierEntry ({ slug: s, triggerSource: ts, isInLatestDeployment: live
}); if validation fails return null/undefined (do not throw) so callers can skip
bad entries; also update the code that consumes decode (the code paths currently
invoking decode around the entries at the other spots mentioned) to filter out
falsy results before using them.
In `@internal-packages/database/prisma/schema.prisma`:
- Line 666: The Prisma schema added an index on
BackgroundWorkerTask(runtimeEnvironmentId, slug, triggerSource) but the
migration 20260413000001_add_task_identifier_table only creates TaskIdentifier
and its indexes; add a companion CREATE INDEX statement for
BackgroundWorkerTask(runtimeEnvironmentId, slug, triggerSource) to that
migration (or add a new migration) so the database matches the Prisma schema,
and regenerate/apply Prisma migrations (e.g., prisma migrate dev/resolve) so the
index is created in production; target the BackgroundWorkerTask table and the
exact composite columns runtimeEnvironmentId, slug, triggerSource in the
migration change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d3b875d3-1e75-4f92-b1ed-8fda39cc171d
📒 Files selected for processing (26)
.server-changes/task-identifier-registry.mdapps/webapp/app/components/logs/LogsTaskFilter.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/models/task.server.tsapps/webapp/app/presenters/v3/ErrorsListPresenter.server.tsapps/webapp/app/presenters/v3/LogsListPresenter.server.tsapps/webapp/app/presenters/v3/NextRunListPresenter.server.tsapps/webapp/app/presenters/v3/ScheduleListPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardKey/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.custom.$dashboardId/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.ai-filter.tsxapps/webapp/app/runEngine/concerns/queues.server.tsapps/webapp/app/runEngine/services/batchTrigger.server.tsapps/webapp/app/runEngine/services/createBatch.server.tsapps/webapp/app/runEngine/services/triggerFailedTask.server.tsapps/webapp/app/services/taskIdentifierCache.server.tsapps/webapp/app/services/taskIdentifierRegistry.server.tsapps/webapp/app/v3/runEngineHandlers.server.tsapps/webapp/app/v3/services/changeCurrentDeployment.server.tsapps/webapp/app/v3/services/createBackgroundWorker.server.tsapps/webapp/app/v3/services/createDeploymentBackgroundWorkerV3.server.tsapps/webapp/app/v3/services/triggerTask.server.tsapps/webapp/test/engine/taskIdentifierRegistry.test.tsinternal-packages/database/prisma/migrations/20260413000000_add_bwt_covering_index/migration.sqlinternal-packages/database/prisma/migrations/20260413000001_add_task_identifier_table/migration.sqlinternal-packages/database/prisma/schema.prisma
✅ Files skipped from review due to trivial changes (6)
- apps/webapp/app/runEngine/services/batchTrigger.server.ts
- apps/webapp/app/v3/runEngineHandlers.server.ts
- internal-packages/database/prisma/migrations/20260413000000_add_bwt_covering_index/migration.sql
- .server-changes/task-identifier-registry.md
- apps/webapp/app/models/task.server.ts
- internal-packages/database/prisma/migrations/20260413000001_add_task_identifier_table/migration.sql
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/webapp/app/runEngine/services/createBatch.server.ts
- apps/webapp/app/presenters/v3/ErrorsListPresenter.server.ts
- apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.ai-filter.tsx
- apps/webapp/app/runEngine/services/triggerFailedTask.server.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.custom.$dashboardId/route.tsx
- apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
- apps/webapp/app/runEngine/concerns/queues.server.ts
- apps/webapp/test/engine/taskIdentifierRegistry.test.ts
- apps/webapp/app/v3/services/createBackgroundWorker.server.ts
ericallam
deleted the
feature/tri-8441-taskidentifier-registry-eliminate-728-db-runtime-from
branch
2 participants
Replace the expensive DISTINCT query for task filter dropdowns with a dedicated TaskIdentifier registry table backed by Redis. Environments migrate automatically on their next deploy, with a transparent fallback to the legacy query for unmigrated environments. Also fixes duplicate dropdown entries when a task changes trigger source, and adds active/archived grouping for removed tasks. Moves BackgroundWorkerTask reads in the trigger hot path to the read replica.