Skip to content

Commit f6f8174

Browse files
matt-aitkenclaude
matt-aitken
and
claude
committed
feat(cli,core,webapp): fail dev and deploy on duplicate task ids
Two tasks defined with the same id — including across task types (e.g. a scheduled task and a regular task sharing an id) — previously caused the second definition to silently overwrite the first in the resource catalog, so one task would vanish with no warning. Detect collisions at registration time in StandardResourceCatalog (the only point where both definitions are visible before the id-keyed map collapses them) and fail indexing via a new TASKS_FAILED_TO_INDEX message that names each offending id and the files it was found in. This blocks both `dev` and `deploy`. The same rule is enforced server-side when the background worker is registered, as a backstop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f261ff2 commit f6f8174

19 files changed

Lines changed: 401 additions & 45 deletions

‎.changeset/duplicate-task-ids.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@trigger.dev/core": patch
3+
"trigger.dev": patch
4+
---
5+
6+
`dev` and `deploy` now fail with a clear error when two tasks are defined with the same id, including across different task types (e.g. a scheduled task and a regular task sharing an id). Previously the second definition silently overwrote the first, so one of the tasks would vanish with no warning. Task ids are detected as duplicates during indexing (naming each offending id and the files it was found in), and the same rule is enforced server-side when the background worker is registered.

‎apps/webapp/app/v3/services/createBackgroundWorker.server.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import { engine } from "../runEngine.server";
3535
import { scheduleEngine } from "../scheduleEngine.server";
3636

3737
import { stripBackgroundWorkerMetadataForStorage } from "./stripBackgroundWorkerMetadataForStorage.server";
38+
import { assertNoDuplicateTaskIds } from "./duplicateTaskIds.server";
3839
export { stripBackgroundWorkerMetadataForStorage };
3940

4041
export class CreateBackgroundWorkerService extends BaseService {
@@ -151,6 +152,15 @@ export class CreateBackgroundWorkerService extends BaseService {
151152
);
152153

153154
if (resourcesError) {
155+
if (resourcesError instanceof ServiceValidationError) {
156+
// Customer-facing config error (e.g. duplicate task ids). Surface the
157+
// real message to the client via the rethrow.
158+
logger.warn("Error creating worker resources", {
159+
error: resourcesError.message,
160+
});
161+
throw resourcesError;
162+
}
163+
154164
logger.error("Error creating worker resources", {
155165
error: resourcesError,
156166
backgroundWorker,
@@ -279,6 +289,13 @@ export async function createWorkerResources(
279289
prisma: PrismaClientOrTransaction,
280290
tasksToBackgroundFiles?: Map<string, string>
281291
): Promise<TaskMetadataEntry[]> {
292+
// Defense-in-depth against two tasks sharing an id (across all task types,
293+
// e.g. a schedule and a regular task). Note: the CLI's resource catalog keys
294+
// tasks by id and overwrites collisions, so duplicates are normally already
295+
// collapsed before reaching here — this guards against any client that sends
296+
// an un-deduplicated task list.
297+
assertNoDuplicateTaskIds(metadata.tasks);
298+
282299
// Create the queues
283300
const queues = await createWorkerQueues(metadata, worker, environment, prisma);
284301

‎apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV4.server.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,17 @@ export class CreateDeploymentBackgroundWorkerServiceV4 extends BaseService {
160160
);
161161

162162
if (resourcesError) {
163+
if (resourcesError instanceof ServiceValidationError) {
164+
// Customer-facing config error (e.g. duplicate task ids). Surface the
165+
// real message to the client via the rethrow.
166+
logger.warn("Error creating background worker resources", {
167+
error: resourcesError.message,
168+
});
169+
170+
await this.#failBackgroundWorkerDeployment(deployment, resourcesError);
171+
throw resourcesError;
172+
}
173+
163174
logger.error("Error creating background worker resources", {
164175
error: resourcesError,
165176
});

‎apps/webapp/app/v3/services/duplicateTaskIds.server.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import { ServiceValidationError } from "./common.server";
2+
3+
type TaskIdResource = {
4+
id: string;
5+
filePath?: string;
6+
exportName?: string;
7+
};
8+
9+
/**
10+
* Returns the set of task ids that are defined more than once. All task types
11+
* (regular tasks, scheduled tasks, agents, etc.) share a single id namespace,
12+
* so a schedule and a regular task that use the same id count as a duplicate.
13+
*/
14+
export function findDuplicateTaskIds(tasks: Array<TaskIdResource>): string[] {
15+
const seen = new Set<string>();
16+
const duplicates = new Set<string>();
17+
18+
for (const task of tasks) {
19+
if (seen.has(task.id)) {
20+
duplicates.add(task.id);
21+
} else {
22+
seen.add(task.id);
23+
}
24+
}
25+
26+
return Array.from(duplicates);
27+
}
28+
29+
/**
30+
* Throws a customer-facing {@link ServiceValidationError} (HTTP 400) if any
31+
* task id is defined more than once, naming each offending id and the files it
32+
* was found in.
33+
*/
34+
export function assertNoDuplicateTaskIds(tasks: Array<TaskIdResource>): void {
35+
const duplicateTaskIds = findDuplicateTaskIds(tasks);
36+
37+
if (duplicateTaskIds.length === 0) {
38+
return;
39+
}
40+
41+
const details = duplicateTaskIds
42+
.map((id) => {
43+
const locations = tasks
44+
.filter((task) => task.id === id)
45+
.map((task) => task.filePath ?? "unknown file")
46+
.join(", ");
47+
48+
return `"${id}" (defined in ${locations})`;
49+
})
50+
.join("; ");
51+
52+
throw new ServiceValidationError(
53+
`Duplicate task ids detected: ${details}. Each task must have a unique id across all task types (including scheduled tasks). Please rename one of them.`,
54+
400
55+
);
56+
}

‎apps/webapp/test/duplicateTaskIds.test.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { describe, it, expect } from "vitest";
2+
import { ServiceValidationError } from "~/v3/services/common.server";
3+
import { assertNoDuplicateTaskIds } from "~/v3/services/duplicateTaskIds.server";
4+
5+
function task(partial: { id: string; filePath?: string; exportName?: string; triggerSource?: string }) {
6+
return {
7+
filePath: "src/trigger/example.ts",
8+
exportName: "exampleTask",
9+
...partial,
10+
} as any;
11+
}
12+
13+
describe("assertNoDuplicateTaskIds", () => {
14+
it("does not throw when all task ids are unique", () => {
15+
const tasks = [task({ id: "a" }), task({ id: "b" }), task({ id: "c" })];
16+
17+
expect(() => assertNoDuplicateTaskIds(tasks)).not.toThrow();
18+
});
19+
20+
it("throws a ServiceValidationError when a task id is duplicated", () => {
21+
const tasks = [task({ id: "a" }), task({ id: "a" })];
22+
23+
expect(() => assertNoDuplicateTaskIds(tasks)).toThrow(ServiceValidationError);
24+
});
25+
26+
it("reports a 400 and names the duplicate id and its files", () => {
27+
const tasks = [
28+
task({ id: "report", filePath: "src/trigger/report.ts" }),
29+
task({ id: "report", filePath: "src/trigger/scheduled.ts" }),
30+
];
31+
32+
let error: unknown;
33+
try {
34+
assertNoDuplicateTaskIds(tasks);
35+
} catch (e) {
36+
error = e;
37+
}
38+
39+
expect(error).toBeInstanceOf(ServiceValidationError);
40+
const validationError = error as ServiceValidationError;
41+
expect(validationError.status).toBe(400);
42+
expect(validationError.message).toContain("report");
43+
expect(validationError.message).toContain("src/trigger/report.ts");
44+
expect(validationError.message).toContain("src/trigger/scheduled.ts");
45+
});
46+
47+
it("detects duplicates across different task types (a schedule and a regular task sharing an id)", () => {
48+
const tasks = [
49+
task({ id: "report", triggerSource: undefined, filePath: "src/trigger/report.ts" }),
50+
task({ id: "report", triggerSource: "schedule", filePath: "src/trigger/scheduled.ts" }),
51+
];
52+
53+
expect(() => assertNoDuplicateTaskIds(tasks)).toThrow(ServiceValidationError);
54+
});
55+
});

‎packages/cli-v3/src/dev/devOutput.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { formatDurationMilliseconds } from "@trigger.dev/core/v3";
22
import { ResolvedConfig } from "@trigger.dev/core/v3/build";
33
import {
44
createTaskMetadataFailedErrorStack,
5+
DuplicateTaskIdsError,
56
TaskIndexingImportError,
67
TaskMetadataParseError,
78
} from "@trigger.dev/core/v3/errors";
@@ -130,6 +131,27 @@ export function startDevOutput(options: DevOutputOptions) {
130131
project: config.project,
131132
query: `Could not parse task metadata:\n ${errorStack}`,
132133
});
134+
} else if (error instanceof DuplicateTaskIdsError) {
135+
const body = error.collisions
136+
.map(({ id, filePaths }) => {
137+
const distinct = Array.from(new Set(filePaths));
138+
139+
return distinct.length === 1
140+
? `${chalkTask(id)} was defined more than once in ${distinct[0]}`
141+
: `${chalkTask(id)} was defined in:\n${distinct.map((f) => ` ${f}`).join("\n")}`;
142+
})
143+
.join("\n\n");
144+
145+
prettyError(
146+
"Duplicate task ids detected",
147+
`${body}\n\nTask ids must be unique across your project (including scheduled tasks). Please rename one of them.`,
148+
cliLink("View the task docs", "https://trigger.dev/docs/tasks/overview")
149+
);
150+
aiHelpLink({
151+
dashboardUrl,
152+
project: config.project,
153+
query: `Duplicate task ids: ${error.collisions.map((c) => c.id).join(", ")}`,
154+
});
133155
} else {
134156
const errorText = error instanceof Error ? error.message : "Unknown error";
135157
const stack = error instanceof Error ? error.stack : undefined;

‎packages/cli-v3/src/dev/devSupervisor.ts‎

Lines changed: 8 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import {
88
CreateBackgroundWorkerRequestBody,
99
DevConfigResponseBody,
1010
SemanticInternalAttributes,
11-
TaskManifest,
1211
WorkerManifest,
1312
} from "@trigger.dev/core/v3";
1413
import { ResolvedConfig } from "@trigger.dev/core/v3/build";
@@ -20,7 +19,7 @@ import { resolveSourceFiles } from "../utilities/sourceFiles.js";
2019
import { BackgroundWorker } from "./backgroundWorker.js";
2120
import { copySkillFolders } from "../build/bundleSkills.js";
2221
import { WorkerRuntime } from "./workerRuntime.js";
23-
import { chalkTask, cliLink, prettyError } from "../utilities/cliOutput.js";
22+
import { cliLink, prettyError } from "../utilities/cliOutput.js";
2423
import { DevRunController } from "../entryPoints/dev-run-controller.js";
2524
import { io, Socket } from "socket.io-client";
2625
import {
@@ -841,38 +840,24 @@ class DevSupervisor implements WorkerRuntime {
841840
}
842841
}
843842

844-
type ValidationIssue =
845-
| {
846-
type: "duplicateTaskId";
847-
duplicationTaskIds: string[];
848-
}
849-
| {
850-
type: "noTasksDefined";
851-
};
843+
type ValidationIssue = {
844+
type: "noTasksDefined";
845+
};
852846

847+
// Duplicate task ids (including across task types, e.g. a schedule and a
848+
// regular task sharing an id) are enforced server-side when the background
849+
// worker is registered, so both `dev` and `deploy` surface a single,
850+
// authoritative error from the backend rather than a separate client check.
853851
function validateWorkerManifest(manifest: WorkerManifest): ValidationIssue | undefined {
854-
const issues: ValidationIssue[] = [];
855-
856852
if (!manifest.tasks || manifest.tasks.length === 0) {
857853
return { type: "noTasksDefined" };
858854
}
859855

860-
// Check for any duplicate task ids
861-
const taskIds = manifest.tasks.map((task) => task.id);
862-
const duplicateTaskIds = taskIds.filter((id, index) => taskIds.indexOf(id) !== index);
863-
864-
if (duplicateTaskIds.length > 0) {
865-
return { type: "duplicateTaskId", duplicationTaskIds: duplicateTaskIds };
866-
}
867-
868856
return undefined;
869857
}
870858

871859
function generationValidationIssueHeader(issue: ValidationIssue) {
872860
switch (issue.type) {
873-
case "duplicateTaskId": {
874-
return `Duplicate task ids detected`;
875-
}
876861
case "noTasksDefined": {
877862
return `No tasks exported from your trigger files`;
878863
}
@@ -881,9 +866,6 @@ function generationValidationIssueHeader(issue: ValidationIssue) {
881866

882867
function generateValidationIssueFooter(issue: ValidationIssue) {
883868
switch (issue.type) {
884-
case "duplicateTaskId": {
885-
return cliLink("View the task docs", "https://trigger.dev/docs/tasks/overview");
886-
}
887869
case "noTasksDefined": {
888870
return cliLink("View the task docs", "https://trigger.dev/docs/tasks/overview");
889871
}
@@ -896,9 +878,6 @@ function generateValidationIssueMessage(
896878
buildManifest: BuildManifest
897879
) {
898880
switch (issue.type) {
899-
case "duplicateTaskId": {
900-
return createDuplicateTaskIdOutputErrorMessage(issue.duplicationTaskIds, manifest.tasks);
901-
}
902881
case "noTasksDefined": {
903882
return `
904883
Files:
@@ -923,19 +902,3 @@ function generateValidationIssueMessage(
923902
}
924903
}
925904

926-
function createDuplicateTaskIdOutputErrorMessage(
927-
duplicateTaskIds: Array<string>,
928-
tasks: Array<TaskManifest>
929-
) {
930-
const duplicateTable = duplicateTaskIds
931-
.map((id) => {
932-
const $tasks = tasks.filter((task) => task.id === id);
933-
934-
return `\n\n${chalkTask(id)} was found in:${tasks
935-
.map((task) => `\n${task.filePath} -> ${task.exportName}`)
936-
.join("")}`;
937-
})
938-
.join("");
939-
940-
return `Duplicate ${chalkTask("task id")} detected:${duplicateTable}`;
941-
}

‎packages/cli-v3/src/entryPoints/dev-index-worker.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { sendMessageInCatalog, ZodSchemaParsedError } from "@trigger.dev/core/v3
1616
import { readFile } from "node:fs/promises";
1717
import sourceMapSupport from "source-map-support";
1818
import { registerResources } from "../indexing/registerResources.js";
19+
import { reportTaskIdCollisions } from "../indexing/reportTaskIdCollisions.js";
1920
import { env } from "std-env";
2021
import { normalizeImportPath } from "../utilities/normalizeImportPath.js";
2122
import { detectRuntimeVersion } from "@trigger.dev/core/v3/build";
@@ -117,6 +118,15 @@ async function bootstrap() {
117118

118119
const { buildManifest, importErrors, config, timings } = await bootstrap();
119120

121+
// Fail indexing if two task definitions share an id (across files and task
122+
// types). The catalog keys tasks by id, so without this the second definition
123+
// would silently overwrite the first.
124+
if (await reportTaskIdCollisions(safeSend)) {
125+
// Give the message time to flush before the parent kills the worker.
126+
await new Promise<void>((resolve) => setTimeout(resolve, 10));
127+
process.exit(0);
128+
}
129+
120130
let tasks = await convertSchemasToJsonSchemas(resourceCatalog.listTaskManifests());
121131

122132
// If the config has retry defaults, we need to apply them to all tasks that don't have any retry settings

‎packages/cli-v3/src/entryPoints/managed-index-worker.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { sendMessageInCatalog, ZodSchemaParsedError } from "@trigger.dev/core/v3
1616
import { readFile } from "node:fs/promises";
1717
import sourceMapSupport from "source-map-support";
1818
import { registerResources } from "../indexing/registerResources.js";
19+
import { reportTaskIdCollisions } from "../indexing/reportTaskIdCollisions.js";
1920
import { env } from "std-env";
2021
import { normalizeImportPath } from "../utilities/normalizeImportPath.js";
2122
import { detectRuntimeVersion } from "@trigger.dev/core/v3/build";
@@ -111,6 +112,15 @@ async function bootstrap() {
111112

112113
const { buildManifest, importErrors, config, timings } = await bootstrap();
113114

115+
// Fail indexing if two task definitions share an id (across files and task
116+
// types). The catalog keys tasks by id, so without this the second definition
117+
// would silently overwrite the first.
118+
if (await reportTaskIdCollisions(safeSend)) {
119+
// Give the message time to flush before the parent kills the worker.
120+
await new Promise<void>((resolve) => setTimeout(resolve, 10));
121+
process.exit(0);
122+
}
123+
114124
let tasks = await convertSchemasToJsonSchemas(resourceCatalog.listTaskManifests());
115125

116126
// If the config has retry defaults, we need to apply them to all tasks that don't have any retry settings

‎packages/cli-v3/src/indexing/indexWorkerManifest.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { execPathForRuntime } from "@trigger.dev/core/v3/build";
22
import {
3+
DuplicateTaskIdsError,
34
TaskIndexingImportError,
45
TaskMetadataParseError,
56
UncaughtExceptionError,
@@ -89,6 +90,13 @@ export async function indexWorkerManifest({
8990
child.kill("SIGKILL");
9091
break;
9192
}
93+
case "TASKS_FAILED_TO_INDEX": {
94+
clearTimeout(timeout);
95+
resolved = true;
96+
reject(new DuplicateTaskIdsError(message.payload.collisions));
97+
child.kill("SIGKILL");
98+
break;
99+
}
92100
case "UNCAUGHT_EXCEPTION": {
93101
clearTimeout(timeout);
94102
resolved = true;

0 commit comments

Comments
 (0)