Fail dev and deploy on duplicate task ids (#3865)
## What `dev` and `deploy` now fail with a clear error when two tasks are defined with the same id — including across task types (e.g. a scheduled task and a regular task sharing an id). ## Why Tasks are registered into the resource catalog keyed by id, so a second definition with the same id silently overwrote the first. One of the tasks would just vanish from the worker with no warning — easy to miss, hard to debug. (Any earlier duplicate-id check ran against the post-registration task list, which is already de-duplicated, so it never actually fired.) ## How - **Detect at registration** (`@trigger.dev/core`): `StandardResourceCatalog` records a collision when a task id is registered more than once, capturing the files involved — the only point where both definitions are visible before the id-keyed map collapses them. Exposed via `listTaskIdCollisions()`. - **Fail indexing** (`trigger.dev` CLI): both index workers report collisions via a new `TASKS_FAILED_TO_INDEX` message; `indexWorkerManifest` rejects with a new `DuplicateTaskIdsError`. `dev` renders a dedicated error (offending ids + files + docs link); `deploy` fails with the same message. Runtime worker boot is unaffected — it never reads the collisions. - **Server-side backstop** (webapp): background-worker registration also rejects duplicate ids with a clear `ServiceValidationError`, so duplicates are caught even from an older CLI. ## Testing - Unit tests for collision collection in the catalog and for the error-message formatting (standard, same-file, and 3+-definition cases). - Verified end to end against a local webapp: a project with a regular task and a scheduled task sharing an id now fails `dev` with the dedicated error; a project with distinct ids still starts normally. ## Changeset Patch for `@trigger.dev/core` and `trigger.dev`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
||||
@@ -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<string, string>
|
||||
): Promise<TaskMetadataEntry[]> {
|
||||
// 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);
|
||||
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
|
||||
@@ -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<TaskIdResource>): string[] {
|
||||
const seen = new Set<string>();
|
||||
const duplicates = new Set<string>();
|
||||
|
||||
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<TaskIdResource>): 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
|
||||
);
|
||||
}
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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;
|
||||
|
||||
@@ -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<string>,
|
||||
tasks: Array<TaskManifest>
|
||||
) {
|
||||
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}`;
|
||||
}
|
||||
|
||||
@@ -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<void>((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
|
||||
|
||||
@@ -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<void>((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
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<boolean> {
|
||||
const collisions = resourceCatalog.listTaskIdCollisions();
|
||||
|
||||
if (collisions.length === 0) {
|
||||
return false;
|
||||
}
|
||||
|
||||
await sendMessageInCatalog(
|
||||
indexerToWorkerMessages,
|
||||
"TASKS_FAILED_TO_INDEX",
|
||||
{ collisions },
|
||||
async (msg) => {
|
||||
send(msg);
|
||||
}
|
||||
);
|
||||
|
||||
return true;
|
||||
}
|
||||
@@ -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"');
|
||||
});
|
||||
});
|
||||
@@ -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,
|
||||
|
||||
@@ -14,6 +14,7 @@ export interface ResourceCatalog {
|
||||
registerTaskMetadata(task: TaskMetadataWithFunctions): void;
|
||||
updateTaskMetadata(id: string, task: Partial<TaskMetadataWithFunctions>): void;
|
||||
listTaskManifests(): Array<TaskManifest>;
|
||||
listTaskIdCollisions(): Array<{ id: string; filePaths: string[] }>;
|
||||
getTaskManifest(id: string): TaskManifest | undefined;
|
||||
getTask(id: string): TaskMetadataWithFunctions | undefined;
|
||||
taskExists(id: string): boolean;
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -30,6 +30,10 @@ export class NoopResourceCatalog implements ResourceCatalog {
|
||||
return [];
|
||||
}
|
||||
|
||||
listTaskIdCollisions(): Array<{ id: string; filePaths: string[] }> {
|
||||
return [];
|
||||
}
|
||||
|
||||
getTaskManifest(id: string): TaskManifest | undefined {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
@@ -38,6 +38,11 @@ export class StandardResourceCatalog implements ResourceCatalog {
|
||||
private _skillMetadata: Map<string, SkillMetadata> = new Map();
|
||||
private _skillFileMetadata: Map<string, TaskFileMetadata> = new Map();
|
||||
private _sentinelContextWarned: Set<string> = 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,
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
|
||||
|
||||
@@ -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"] },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user