From f6f8174361e7ddcdb0bf0072221d4ad900d54471 Mon Sep 17 00:00:00 2001 From: Matt Aitken Date: Mon, 8 Jun 2026 11:28:36 +0100 Subject: [PATCH] feat(cli,core,webapp): fail dev and deploy on duplicate task ids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .changeset/duplicate-task-ids.md | 6 ++ .../services/createBackgroundWorker.server.ts | 17 ++++++ ...eateDeploymentBackgroundWorkerV4.server.ts | 11 ++++ .../v3/services/duplicateTaskIds.server.ts | 56 +++++++++++++++++++ apps/webapp/test/duplicateTaskIds.test.ts | 55 ++++++++++++++++++ packages/cli-v3/src/dev/devOutput.ts | 22 ++++++++ packages/cli-v3/src/dev/devSupervisor.ts | 53 +++--------------- .../src/entryPoints/dev-index-worker.ts | 10 ++++ .../src/entryPoints/managed-index-worker.ts | 10 ++++ .../src/indexing/indexWorkerManifest.ts | 8 +++ .../src/indexing/reportTaskIdCollisions.ts | 26 +++++++++ packages/core/src/v3/errors.test.ts | 43 ++++++++++++++ packages/core/src/v3/errors.ts | 33 +++++++++++ .../core/src/v3/resource-catalog/catalog.ts | 1 + .../core/src/v3/resource-catalog/index.ts | 4 ++ .../resource-catalog/noopResourceCatalog.ts | 4 ++ .../standardResourceCatalog.ts | 32 +++++++++++ packages/core/src/v3/schemas/messages.ts | 4 ++ packages/core/test/resourceCatalog.test.ts | 51 +++++++++++++++++ 19 files changed, 401 insertions(+), 45 deletions(-) create mode 100644 .changeset/duplicate-task-ids.md create mode 100644 apps/webapp/app/v3/services/duplicateTaskIds.server.ts create mode 100644 apps/webapp/test/duplicateTaskIds.test.ts create mode 100644 packages/cli-v3/src/indexing/reportTaskIdCollisions.ts create mode 100644 packages/core/src/v3/errors.test.ts diff --git a/.changeset/duplicate-task-ids.md b/.changeset/duplicate-task-ids.md new file mode 100644 index 00000000000..c68724bfedd --- /dev/null +++ b/.changeset/duplicate-task-ids.md @@ -0,0 +1,6 @@ +--- +"@trigger.dev/core": patch +"trigger.dev": patch +--- + +`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. diff --git a/apps/webapp/app/v3/services/createBackgroundWorker.server.ts b/apps/webapp/app/v3/services/createBackgroundWorker.server.ts index b3052fa65b6..3f658643e7f 100644 --- a/apps/webapp/app/v3/services/createBackgroundWorker.server.ts +++ b/apps/webapp/app/v3/services/createBackgroundWorker.server.ts @@ -35,6 +35,7 @@ import { engine } from "../runEngine.server"; import { scheduleEngine } from "../scheduleEngine.server"; import { stripBackgroundWorkerMetadataForStorage } from "./stripBackgroundWorkerMetadataForStorage.server"; +import { assertNoDuplicateTaskIds } from "./duplicateTaskIds.server"; export { stripBackgroundWorkerMetadataForStorage }; export class CreateBackgroundWorkerService extends BaseService { @@ -151,6 +152,15 @@ export class CreateBackgroundWorkerService extends BaseService { ); if (resourcesError) { + if (resourcesError instanceof ServiceValidationError) { + // Customer-facing config error (e.g. duplicate task ids). Surface the + // real message to the client via the rethrow. + logger.warn("Error creating worker resources", { + error: resourcesError.message, + }); + throw resourcesError; + } + logger.error("Error creating worker resources", { error: resourcesError, backgroundWorker, @@ -279,6 +289,13 @@ export async function createWorkerResources( prisma: PrismaClientOrTransaction, tasksToBackgroundFiles?: Map ): Promise { + // Defense-in-depth against two tasks sharing an id (across all task types, + // e.g. a schedule and a regular task). Note: the CLI's resource catalog keys + // tasks by id and overwrites collisions, so duplicates are normally already + // collapsed before reaching here — this guards against any client that sends + // an un-deduplicated task list. + assertNoDuplicateTaskIds(metadata.tasks); + // Create the queues const queues = await createWorkerQueues(metadata, worker, environment, prisma); diff --git a/apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV4.server.ts b/apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV4.server.ts index d240af34118..d34db841cd6 100644 --- a/apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV4.server.ts +++ b/apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV4.server.ts @@ -160,6 +160,17 @@ export class CreateDeploymentBackgroundWorkerServiceV4 extends BaseService { ); if (resourcesError) { + if (resourcesError instanceof ServiceValidationError) { + // Customer-facing config error (e.g. duplicate task ids). Surface the + // real message to the client via the rethrow. + logger.warn("Error creating background worker resources", { + error: resourcesError.message, + }); + + await this.#failBackgroundWorkerDeployment(deployment, resourcesError); + throw resourcesError; + } + logger.error("Error creating background worker resources", { error: resourcesError, }); diff --git a/apps/webapp/app/v3/services/duplicateTaskIds.server.ts b/apps/webapp/app/v3/services/duplicateTaskIds.server.ts new file mode 100644 index 00000000000..4ade92fd9fd --- /dev/null +++ b/apps/webapp/app/v3/services/duplicateTaskIds.server.ts @@ -0,0 +1,56 @@ +import { ServiceValidationError } from "./common.server"; + +type TaskIdResource = { + id: string; + filePath?: string; + exportName?: string; +}; + +/** + * Returns the set of task ids that are defined more than once. All task types + * (regular tasks, scheduled tasks, agents, etc.) share a single id namespace, + * so a schedule and a regular task that use the same id count as a duplicate. + */ +export function findDuplicateTaskIds(tasks: Array): string[] { + const seen = new Set(); + const duplicates = new Set(); + + for (const task of tasks) { + if (seen.has(task.id)) { + duplicates.add(task.id); + } else { + seen.add(task.id); + } + } + + return Array.from(duplicates); +} + +/** + * Throws a customer-facing {@link ServiceValidationError} (HTTP 400) if any + * task id is defined more than once, naming each offending id and the files it + * was found in. + */ +export function assertNoDuplicateTaskIds(tasks: Array): void { + const duplicateTaskIds = findDuplicateTaskIds(tasks); + + if (duplicateTaskIds.length === 0) { + return; + } + + const details = duplicateTaskIds + .map((id) => { + const locations = tasks + .filter((task) => task.id === id) + .map((task) => task.filePath ?? "unknown file") + .join(", "); + + return `"${id}" (defined in ${locations})`; + }) + .join("; "); + + throw new ServiceValidationError( + `Duplicate task ids detected: ${details}. Each task must have a unique id across all task types (including scheduled tasks). Please rename one of them.`, + 400 + ); +} diff --git a/apps/webapp/test/duplicateTaskIds.test.ts b/apps/webapp/test/duplicateTaskIds.test.ts new file mode 100644 index 00000000000..f0f95806c56 --- /dev/null +++ b/apps/webapp/test/duplicateTaskIds.test.ts @@ -0,0 +1,55 @@ +import { describe, it, expect } from "vitest"; +import { ServiceValidationError } from "~/v3/services/common.server"; +import { assertNoDuplicateTaskIds } from "~/v3/services/duplicateTaskIds.server"; + +function task(partial: { id: string; filePath?: string; exportName?: string; triggerSource?: string }) { + return { + filePath: "src/trigger/example.ts", + exportName: "exampleTask", + ...partial, + } as any; +} + +describe("assertNoDuplicateTaskIds", () => { + it("does not throw when all task ids are unique", () => { + const tasks = [task({ id: "a" }), task({ id: "b" }), task({ id: "c" })]; + + expect(() => assertNoDuplicateTaskIds(tasks)).not.toThrow(); + }); + + it("throws a ServiceValidationError when a task id is duplicated", () => { + const tasks = [task({ id: "a" }), task({ id: "a" })]; + + expect(() => assertNoDuplicateTaskIds(tasks)).toThrow(ServiceValidationError); + }); + + it("reports a 400 and names the duplicate id and its files", () => { + const tasks = [ + task({ id: "report", filePath: "src/trigger/report.ts" }), + task({ id: "report", filePath: "src/trigger/scheduled.ts" }), + ]; + + let error: unknown; + try { + assertNoDuplicateTaskIds(tasks); + } catch (e) { + error = e; + } + + expect(error).toBeInstanceOf(ServiceValidationError); + const validationError = error as ServiceValidationError; + expect(validationError.status).toBe(400); + expect(validationError.message).toContain("report"); + expect(validationError.message).toContain("src/trigger/report.ts"); + expect(validationError.message).toContain("src/trigger/scheduled.ts"); + }); + + it("detects duplicates across different task types (a schedule and a regular task sharing an id)", () => { + const tasks = [ + task({ id: "report", triggerSource: undefined, filePath: "src/trigger/report.ts" }), + task({ id: "report", triggerSource: "schedule", filePath: "src/trigger/scheduled.ts" }), + ]; + + expect(() => assertNoDuplicateTaskIds(tasks)).toThrow(ServiceValidationError); + }); +}); diff --git a/packages/cli-v3/src/dev/devOutput.ts b/packages/cli-v3/src/dev/devOutput.ts index 6365eee2ed7..a12309cc803 100644 --- a/packages/cli-v3/src/dev/devOutput.ts +++ b/packages/cli-v3/src/dev/devOutput.ts @@ -2,6 +2,7 @@ import { formatDurationMilliseconds } from "@trigger.dev/core/v3"; import { ResolvedConfig } from "@trigger.dev/core/v3/build"; import { createTaskMetadataFailedErrorStack, + DuplicateTaskIdsError, TaskIndexingImportError, TaskMetadataParseError, } from "@trigger.dev/core/v3/errors"; @@ -130,6 +131,27 @@ export function startDevOutput(options: DevOutputOptions) { project: config.project, query: `Could not parse task metadata:\n ${errorStack}`, }); + } else if (error instanceof DuplicateTaskIdsError) { + const body = error.collisions + .map(({ id, filePaths }) => { + const distinct = Array.from(new Set(filePaths)); + + return distinct.length === 1 + ? `${chalkTask(id)} was defined more than once in ${distinct[0]}` + : `${chalkTask(id)} was defined in:\n${distinct.map((f) => ` ${f}`).join("\n")}`; + }) + .join("\n\n"); + + prettyError( + "Duplicate task ids detected", + `${body}\n\nTask ids must be unique across your project (including scheduled tasks). Please rename one of them.`, + cliLink("View the task docs", "https://trigger.dev/docs/tasks/overview") + ); + aiHelpLink({ + dashboardUrl, + project: config.project, + query: `Duplicate task ids: ${error.collisions.map((c) => c.id).join(", ")}`, + }); } else { const errorText = error instanceof Error ? error.message : "Unknown error"; const stack = error instanceof Error ? error.stack : undefined; diff --git a/packages/cli-v3/src/dev/devSupervisor.ts b/packages/cli-v3/src/dev/devSupervisor.ts index 0d972384c40..6c825ea4714 100644 --- a/packages/cli-v3/src/dev/devSupervisor.ts +++ b/packages/cli-v3/src/dev/devSupervisor.ts @@ -8,7 +8,6 @@ import { CreateBackgroundWorkerRequestBody, DevConfigResponseBody, SemanticInternalAttributes, - TaskManifest, WorkerManifest, } from "@trigger.dev/core/v3"; import { ResolvedConfig } from "@trigger.dev/core/v3/build"; @@ -20,7 +19,7 @@ import { resolveSourceFiles } from "../utilities/sourceFiles.js"; import { BackgroundWorker } from "./backgroundWorker.js"; import { copySkillFolders } from "../build/bundleSkills.js"; import { WorkerRuntime } from "./workerRuntime.js"; -import { chalkTask, cliLink, prettyError } from "../utilities/cliOutput.js"; +import { cliLink, prettyError } from "../utilities/cliOutput.js"; import { DevRunController } from "../entryPoints/dev-run-controller.js"; import { io, Socket } from "socket.io-client"; import { @@ -841,38 +840,24 @@ class DevSupervisor implements WorkerRuntime { } } -type ValidationIssue = - | { - type: "duplicateTaskId"; - duplicationTaskIds: string[]; - } - | { - type: "noTasksDefined"; - }; +type ValidationIssue = { + type: "noTasksDefined"; +}; +// Duplicate task ids (including across task types, e.g. a schedule and a +// regular task sharing an id) are enforced server-side when the background +// worker is registered, so both `dev` and `deploy` surface a single, +// authoritative error from the backend rather than a separate client check. function validateWorkerManifest(manifest: WorkerManifest): ValidationIssue | undefined { - const issues: ValidationIssue[] = []; - if (!manifest.tasks || manifest.tasks.length === 0) { return { type: "noTasksDefined" }; } - // Check for any duplicate task ids - const taskIds = manifest.tasks.map((task) => task.id); - const duplicateTaskIds = taskIds.filter((id, index) => taskIds.indexOf(id) !== index); - - if (duplicateTaskIds.length > 0) { - return { type: "duplicateTaskId", duplicationTaskIds: duplicateTaskIds }; - } - return undefined; } function generationValidationIssueHeader(issue: ValidationIssue) { switch (issue.type) { - case "duplicateTaskId": { - return `Duplicate task ids detected`; - } case "noTasksDefined": { return `No tasks exported from your trigger files`; } @@ -881,9 +866,6 @@ function generationValidationIssueHeader(issue: ValidationIssue) { function generateValidationIssueFooter(issue: ValidationIssue) { switch (issue.type) { - case "duplicateTaskId": { - return cliLink("View the task docs", "https://trigger.dev/docs/tasks/overview"); - } case "noTasksDefined": { return cliLink("View the task docs", "https://trigger.dev/docs/tasks/overview"); } @@ -896,9 +878,6 @@ function generateValidationIssueMessage( buildManifest: BuildManifest ) { switch (issue.type) { - case "duplicateTaskId": { - return createDuplicateTaskIdOutputErrorMessage(issue.duplicationTaskIds, manifest.tasks); - } case "noTasksDefined": { return ` Files: @@ -923,19 +902,3 @@ function generateValidationIssueMessage( } } -function createDuplicateTaskIdOutputErrorMessage( - duplicateTaskIds: Array, - tasks: Array -) { - const duplicateTable = duplicateTaskIds - .map((id) => { - const $tasks = tasks.filter((task) => task.id === id); - - return `\n\n${chalkTask(id)} was found in:${tasks - .map((task) => `\n${task.filePath} -> ${task.exportName}`) - .join("")}`; - }) - .join(""); - - return `Duplicate ${chalkTask("task id")} detected:${duplicateTable}`; -} diff --git a/packages/cli-v3/src/entryPoints/dev-index-worker.ts b/packages/cli-v3/src/entryPoints/dev-index-worker.ts index 3e48c70e42e..a5ff5cb5597 100644 --- a/packages/cli-v3/src/entryPoints/dev-index-worker.ts +++ b/packages/cli-v3/src/entryPoints/dev-index-worker.ts @@ -16,6 +16,7 @@ import { sendMessageInCatalog, ZodSchemaParsedError } from "@trigger.dev/core/v3 import { readFile } from "node:fs/promises"; import sourceMapSupport from "source-map-support"; import { registerResources } from "../indexing/registerResources.js"; +import { reportTaskIdCollisions } from "../indexing/reportTaskIdCollisions.js"; import { env } from "std-env"; import { normalizeImportPath } from "../utilities/normalizeImportPath.js"; import { detectRuntimeVersion } from "@trigger.dev/core/v3/build"; @@ -117,6 +118,15 @@ async function bootstrap() { const { buildManifest, importErrors, config, timings } = await bootstrap(); +// Fail indexing if two task definitions share an id (across files and task +// types). The catalog keys tasks by id, so without this the second definition +// would silently overwrite the first. +if (await reportTaskIdCollisions(safeSend)) { + // Give the message time to flush before the parent kills the worker. + await new Promise((resolve) => setTimeout(resolve, 10)); + process.exit(0); +} + let tasks = await convertSchemasToJsonSchemas(resourceCatalog.listTaskManifests()); // If the config has retry defaults, we need to apply them to all tasks that don't have any retry settings diff --git a/packages/cli-v3/src/entryPoints/managed-index-worker.ts b/packages/cli-v3/src/entryPoints/managed-index-worker.ts index 1bf73b22621..4923beb3af9 100644 --- a/packages/cli-v3/src/entryPoints/managed-index-worker.ts +++ b/packages/cli-v3/src/entryPoints/managed-index-worker.ts @@ -16,6 +16,7 @@ import { sendMessageInCatalog, ZodSchemaParsedError } from "@trigger.dev/core/v3 import { readFile } from "node:fs/promises"; import sourceMapSupport from "source-map-support"; import { registerResources } from "../indexing/registerResources.js"; +import { reportTaskIdCollisions } from "../indexing/reportTaskIdCollisions.js"; import { env } from "std-env"; import { normalizeImportPath } from "../utilities/normalizeImportPath.js"; import { detectRuntimeVersion } from "@trigger.dev/core/v3/build"; @@ -111,6 +112,15 @@ async function bootstrap() { const { buildManifest, importErrors, config, timings } = await bootstrap(); +// Fail indexing if two task definitions share an id (across files and task +// types). The catalog keys tasks by id, so without this the second definition +// would silently overwrite the first. +if (await reportTaskIdCollisions(safeSend)) { + // Give the message time to flush before the parent kills the worker. + await new Promise((resolve) => setTimeout(resolve, 10)); + process.exit(0); +} + let tasks = await convertSchemasToJsonSchemas(resourceCatalog.listTaskManifests()); // If the config has retry defaults, we need to apply them to all tasks that don't have any retry settings diff --git a/packages/cli-v3/src/indexing/indexWorkerManifest.ts b/packages/cli-v3/src/indexing/indexWorkerManifest.ts index e4ae72283ff..ed7e810460a 100644 --- a/packages/cli-v3/src/indexing/indexWorkerManifest.ts +++ b/packages/cli-v3/src/indexing/indexWorkerManifest.ts @@ -1,5 +1,6 @@ import { execPathForRuntime } from "@trigger.dev/core/v3/build"; import { + DuplicateTaskIdsError, TaskIndexingImportError, TaskMetadataParseError, UncaughtExceptionError, @@ -89,6 +90,13 @@ export async function indexWorkerManifest({ child.kill("SIGKILL"); break; } + case "TASKS_FAILED_TO_INDEX": { + clearTimeout(timeout); + resolved = true; + reject(new DuplicateTaskIdsError(message.payload.collisions)); + child.kill("SIGKILL"); + break; + } case "UNCAUGHT_EXCEPTION": { clearTimeout(timeout); resolved = true; diff --git a/packages/cli-v3/src/indexing/reportTaskIdCollisions.ts b/packages/cli-v3/src/indexing/reportTaskIdCollisions.ts new file mode 100644 index 00000000000..8c4b27999d2 --- /dev/null +++ b/packages/cli-v3/src/indexing/reportTaskIdCollisions.ts @@ -0,0 +1,26 @@ +import { indexerToWorkerMessages, resourceCatalog } from "@trigger.dev/core/v3"; +import { sendMessageInCatalog } from "@trigger.dev/core/v3/zodMessageHandler"; + +/** + * If the indexer registered any duplicate task ids (across files and task + * types), report them to the parent via TASKS_FAILED_TO_INDEX and return true. + * Callers must stop indexing (skip INDEX_COMPLETE) when this returns true. + */ +export async function reportTaskIdCollisions(send: (message: unknown) => void): Promise { + const collisions = resourceCatalog.listTaskIdCollisions(); + + if (collisions.length === 0) { + return false; + } + + await sendMessageInCatalog( + indexerToWorkerMessages, + "TASKS_FAILED_TO_INDEX", + { collisions }, + async (msg) => { + send(msg); + } + ); + + return true; +} diff --git a/packages/core/src/v3/errors.test.ts b/packages/core/src/v3/errors.test.ts new file mode 100644 index 00000000000..b02bbf5f3cf --- /dev/null +++ b/packages/core/src/v3/errors.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, test } from "vitest"; +import { DuplicateTaskIdsError } from "./errors.js"; + +describe("DuplicateTaskIdsError", () => { + test("is an Error with a stable name and the collisions attached", () => { + const collisions = [{ id: "foo", filePaths: ["src/a.ts", "src/b.ts"] }]; + const error = new DuplicateTaskIdsError(collisions); + + expect(error).toBeInstanceOf(Error); + expect(error.name).toBe("DuplicateTaskIdsError"); + expect(error.collisions).toEqual(collisions); + }); + + test("names the id and both files when an id is defined in two files", () => { + const error = new DuplicateTaskIdsError([ + { id: "foo", filePaths: ["src/a.ts", "src/b.ts"] }, + ]); + + expect(error.message).toContain('"foo"'); + expect(error.message).toContain("src/a.ts"); + expect(error.message).toContain("src/b.ts"); + }); + + test("collapses identical file paths into a single 'more than once' location", () => { + const error = new DuplicateTaskIdsError([ + { id: "foo", filePaths: ["src/dupe.ts", "src/dupe.ts"] }, + ]); + + expect(error.message).toContain("more than once in src/dupe.ts"); + // The same path must not be listed twice. + expect(error.message.match(/src\/dupe\.ts/g)).toHaveLength(1); + }); + + test("lists every collision when multiple ids clash", () => { + const error = new DuplicateTaskIdsError([ + { id: "foo", filePaths: ["src/a.ts", "src/b.ts"] }, + { id: "bar", filePaths: ["src/c.ts", "src/d.ts"] }, + ]); + + expect(error.message).toContain('"foo"'); + expect(error.message).toContain('"bar"'); + }); +}); diff --git a/packages/core/src/v3/errors.ts b/packages/core/src/v3/errors.ts index f7f2ad0dd9b..187cbef9b64 100644 --- a/packages/core/src/v3/errors.ts +++ b/packages/core/src/v3/errors.ts @@ -580,6 +580,39 @@ export class TaskIndexingImportError extends Error { } } +export type TaskIdCollision = { id: string; filePaths: string[] }; + +function formatDuplicateTaskIds(collisions: TaskIdCollision[]): string { + const lines = collisions.map(({ id, filePaths }) => { + const distinct = Array.from(new Set(filePaths)); + + if (distinct.length === 1) { + return ` - "${id}" found more than once in ${distinct[0]}`; + } + + const last = distinct[distinct.length - 1]; + const head = distinct.slice(0, -1).join(", "); + + return ` - "${id}" found in ${head} and ${last}`; + }); + + return [ + "Duplicate task ids detected:", + "", + ...lines, + "", + "Task ids must be unique across your project (including scheduled tasks). Please rename one of them.", + ].join("\n"); +} + +export class DuplicateTaskIdsError extends Error { + constructor(public readonly collisions: TaskIdCollision[]) { + super(formatDuplicateTaskIds(collisions)); + + this.name = "DuplicateTaskIdsError"; + } +} + export class UnexpectedExitError extends Error { constructor( public code: number, diff --git a/packages/core/src/v3/resource-catalog/catalog.ts b/packages/core/src/v3/resource-catalog/catalog.ts index 5c443b253cf..1036152c928 100644 --- a/packages/core/src/v3/resource-catalog/catalog.ts +++ b/packages/core/src/v3/resource-catalog/catalog.ts @@ -14,6 +14,7 @@ export interface ResourceCatalog { registerTaskMetadata(task: TaskMetadataWithFunctions): void; updateTaskMetadata(id: string, task: Partial): void; listTaskManifests(): Array; + listTaskIdCollisions(): Array<{ id: string; filePaths: string[] }>; getTaskManifest(id: string): TaskManifest | undefined; getTask(id: string): TaskMetadataWithFunctions | undefined; taskExists(id: string): boolean; diff --git a/packages/core/src/v3/resource-catalog/index.ts b/packages/core/src/v3/resource-catalog/index.ts index f809ede8135..4e6e278ad02 100644 --- a/packages/core/src/v3/resource-catalog/index.ts +++ b/packages/core/src/v3/resource-catalog/index.ts @@ -64,6 +64,10 @@ export class ResourceCatalogAPI { return this.#getCatalog().listTaskManifests(); } + public listTaskIdCollisions(): Array<{ id: string; filePaths: string[] }> { + return this.#getCatalog().listTaskIdCollisions(); + } + public getTaskManifest(id: string): TaskManifest | undefined { return this.#getCatalog().getTaskManifest(id); } diff --git a/packages/core/src/v3/resource-catalog/noopResourceCatalog.ts b/packages/core/src/v3/resource-catalog/noopResourceCatalog.ts index 5da74d4a9b1..509b76c1792 100644 --- a/packages/core/src/v3/resource-catalog/noopResourceCatalog.ts +++ b/packages/core/src/v3/resource-catalog/noopResourceCatalog.ts @@ -30,6 +30,10 @@ export class NoopResourceCatalog implements ResourceCatalog { return []; } + listTaskIdCollisions(): Array<{ id: string; filePaths: string[] }> { + return []; + } + getTaskManifest(id: string): TaskManifest | undefined { return undefined; } diff --git a/packages/core/src/v3/resource-catalog/standardResourceCatalog.ts b/packages/core/src/v3/resource-catalog/standardResourceCatalog.ts index 6333706317f..bf200936d6f 100644 --- a/packages/core/src/v3/resource-catalog/standardResourceCatalog.ts +++ b/packages/core/src/v3/resource-catalog/standardResourceCatalog.ts @@ -38,6 +38,11 @@ export class StandardResourceCatalog implements ResourceCatalog { private _skillMetadata: Map = new Map(); private _skillFileMetadata: Map = new Map(); private _sentinelContextWarned: Set = new Set(); + // Task ids registered more than once (across files and task types). Tasks are + // keyed by id below, so a second registration silently overwrites the first; + // we record the collision here so the indexer can fail loudly instead. Only + // consumed by the index workers — runtime never reads it. + private _taskIdCollisions: Array<{ id: string; filePaths: string[] }> = []; setCurrentFileContext(filePath: string, entryPoint: string) { this._currentFileContext = { filePath, entryPoint }; @@ -47,6 +52,11 @@ export class StandardResourceCatalog implements ResourceCatalog { this._currentFileContext = undefined; } + // Task ids that were registered more than once during this indexing pass. + listTaskIdCollisions(): Array<{ id: string; filePaths: string[] }> { + return this._taskIdCollisions; + } + registerQueueMetadata(queue: QueueManifest): void { const existingQueue = this._queueMetadata.get(queue.name); @@ -104,6 +114,28 @@ export class StandardResourceCatalog implements ResourceCatalog { ); } + // Detect a duplicate task id before the maps below overwrite the first + // registration. Skip the runtime sentinel context (a task() firing during + // another task's run) — that's a re-registration, not a duplicate + // definition, and the indexer never uses the sentinel. + if ( + this._taskMetadata.has(task.id) && + this._currentFileContext.filePath !== NO_FILE_CONTEXT + ) { + const existingFilePath = this._taskFileMetadata.get(task.id)?.filePath; + const currentFilePath = this._currentFileContext.filePath; + const collision = this._taskIdCollisions.find((c) => c.id === task.id); + + if (collision) { + collision.filePaths.push(currentFilePath); + } else { + this._taskIdCollisions.push({ + id: task.id, + filePaths: [existingFilePath ?? currentFilePath, currentFilePath], + }); + } + } + this._taskFileMetadata.set(task.id, { ...this._currentFileContext, }); diff --git a/packages/core/src/v3/schemas/messages.ts b/packages/core/src/v3/schemas/messages.ts index b58babba717..2c7c357c3b7 100644 --- a/packages/core/src/v3/schemas/messages.ts +++ b/packages/core/src/v3/schemas/messages.ts @@ -156,6 +156,10 @@ export const indexerToWorkerMessages = { importErrors: ImportTaskFileErrors, }), TASKS_FAILED_TO_PARSE: TaskMetadataFailedToParseData, + TASKS_FAILED_TO_INDEX: z.object({ + version: z.literal("v1").default("v1"), + collisions: z.array(z.object({ id: z.string(), filePaths: z.array(z.string()) })), + }), UNCAUGHT_EXCEPTION: UncaughtExceptionMessage, }; diff --git a/packages/core/test/resourceCatalog.test.ts b/packages/core/test/resourceCatalog.test.ts index fa7270f669c..a34e9373f0a 100644 --- a/packages/core/test/resourceCatalog.test.ts +++ b/packages/core/test/resourceCatalog.test.ts @@ -152,3 +152,54 @@ describe("StandardResourceCatalog — runtime registration via sentinel context" } ); }); + +describe("StandardResourceCatalog — duplicate task id collisions", () => { + function register(catalog: StandardResourceCatalog, id: string, filePath: string) { + catalog.setCurrentFileContext(filePath, filePath); + catalog.registerTaskMetadata({ id, fns: { run: async () => "ok" } }); + catalog.clearCurrentFileContext(); + } + + it("reports no collisions when every task id is unique", () => { + const catalog = new StandardResourceCatalog(); + + register(catalog, "a", "src/a.ts"); + register(catalog, "b", "src/b.ts"); + + expect(catalog.listTaskIdCollisions()).toEqual([]); + }); + + it("records a collision with both file paths when an id is reused across files", () => { + const catalog = new StandardResourceCatalog(); + + register(catalog, "dupe", "src/a.ts"); + register(catalog, "dupe", "src/b.ts"); + + expect(catalog.listTaskIdCollisions()).toEqual([ + { id: "dupe", filePaths: ["src/a.ts", "src/b.ts"] }, + ]); + }); + + it("collects every distinct file path when an id is defined three or more times", () => { + const catalog = new StandardResourceCatalog(); + + register(catalog, "dupe", "src/a.ts"); + register(catalog, "dupe", "src/b.ts"); + register(catalog, "dupe", "src/c.ts"); + + expect(catalog.listTaskIdCollisions()).toEqual([ + { id: "dupe", filePaths: ["src/a.ts", "src/b.ts", "src/c.ts"] }, + ]); + }); + + it("records two definitions in the same file (e.g. two exports sharing an id)", () => { + const catalog = new StandardResourceCatalog(); + + register(catalog, "dupe", "src/dupe.ts"); + register(catalog, "dupe", "src/dupe.ts"); + + expect(catalog.listTaskIdCollisions()).toEqual([ + { id: "dupe", filePaths: ["src/dupe.ts", "src/dupe.ts"] }, + ]); + }); +});