Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-batch-trigger-and-wait-debounce.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@trigger.dev/sdk": patch
---

Forward per-item debounce options in array batchTriggerAndWait calls.
115 changes: 114 additions & 1 deletion packages/trigger-sdk/src/v3/shared.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { describe, it, expect } from "vitest";
import { describe, it, expect, afterEach, vi } from "vitest";
import { readableStreamToAsyncIterable } from "./shared.js";

let taskIdCounter = 0;

describe("readableStreamToAsyncIterable", () => {
it("yields all values from the stream", async () => {
const values = [1, 2, 3, 4, 5];
Expand Down Expand Up @@ -217,3 +219,114 @@ describe("readableStreamToAsyncIterable", () => {
});
});

describe("batchTriggerAndWait debounce forwarding", () => {
afterEach(() => {
vi.doUnmock("@trigger.dev/core/v3");
vi.resetModules();
vi.restoreAllMocks();
});

async function setupBatchTriggerAndWaitHarness() {
vi.resetModules();

const capturedItems: any[] = [];

const createBatch = vi.fn(async ({ runCount }: { runCount: number }) => ({
id: "batch_test123",
runCount,
publicAccessToken: "access_token",
isCached: false,
}));

const streamBatchItems = vi.fn(async (_batchId: string, items: any[]) => {
capturedItems.push(...items);

return {
id: "batch_test123",
itemsAccepted: items.length,
itemsDeduplicated: 0,
sealed: true,
};
});

const waitForBatch = vi.fn(async ({ id }: { id: string }) => ({
id,
items: [],
}));

vi.doMock("@trigger.dev/core/v3", async (importOriginal) => {
const original = await importOriginal<typeof import("@trigger.dev/core/v3")>();

return {
...original,
apiClientManager: {
clientOrThrow: vi.fn(() => ({
createBatch,
streamBatchItems,
})),
} as any,
runtime: {
...original.runtime,
waitForBatch,
} as any,
taskContext: {
ctx: {
run: {
id: "run_123",
isTest: false,
},
},
worker: {
version: "worker_123",
},
} as any,
};
});

const tasksModule = await import("./tasks.js");

return {
...tasksModule,
capturedItems,
createBatch,
streamBatchItems,
waitForBatch,
};
}

it("forwards per-item debounce for task.batchTriggerAndWait array items", async () => {
const { task, capturedItems, streamBatchItems, waitForBatch } =
await setupBatchTriggerAndWaitHarness();
const debounce = { key: "same-key", delay: "30s", mode: "trailing" as const };
const taskId = `batch-debounce-task-${++taskIdCounter}`;

const myTask = task({
id: taskId,
run: async (payload: { id: string }) => payload,
});

await myTask.batchTriggerAndWait([{ payload: { id: "a" }, options: { debounce } }]);

expect(streamBatchItems).toHaveBeenCalledTimes(1);
expect(waitForBatch).toHaveBeenCalledTimes(1);
expect(capturedItems).toHaveLength(1);
expect(capturedItems[0]?.task).toBe(taskId);
expect(capturedItems[0]?.options?.debounce).toEqual(debounce);
});

it("forwards per-item debounce for tasks.batchTriggerAndWait array items", async () => {
const { tasks, capturedItems, streamBatchItems, waitForBatch } =
await setupBatchTriggerAndWaitHarness();
const debounce = { key: "same-key", delay: "30s", mode: "trailing" as const };

await tasks.batchTriggerAndWait("batch-debounce-by-id-task", [
{ payload: { id: "a" }, options: { debounce } },
]);

expect(streamBatchItems).toHaveBeenCalledTimes(1);
expect(waitForBatch).toHaveBeenCalledTimes(1);
expect(capturedItems).toHaveLength(1);
expect(capturedItems[0]?.task).toBe("batch-debounce-by-id-task");
expect(capturedItems[0]?.options?.debounce).toEqual(debounce);
});
});
Comment on lines +222 to +332
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 New tests use vi.doMock/vi.fn contrary to repo testing conventions

The new batchTriggerAndWait debounce forwarding test suite at packages/trigger-sdk/src/v3/shared.test.ts:222-332 uses vi.doMock, vi.fn(), vi.resetModules(), and vi.restoreAllMocks(). The repository's testing guidelines in ai/references/tests.md:78-83 explicitly state 'Do not mock anything', 'Do not use mocks in tests', and 'Do not use spies in tests'. However, this is a case where testing SDK option forwarding through internal functions practically requires mocking the API client and runtime — there's no Redis/Postgres dependency where testcontainers could substitute. The prose in ai/references/tests.md:26 also says 'We almost NEVER mock anything' (emphasis on 'almost'), suggesting rare exceptions exist. Worth discussing whether this test approach is acceptable or if an alternative testing strategy should be used.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

1 change: 1 addition & 0 deletions packages/trigger-sdk/src/v3/shared.ts
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 batchTrigger_internal array path silently drops per-item debounce options

The PR fixes the missing debounce forwarding in batchTriggerAndWait_internal's array path (line 2510), but the same bug still exists in the sister function batchTrigger_internal at lines 2214-2236. When users call batchTrigger() with an array of items that include per-item debounce options, those options are silently dropped. The streaming counterpart transformSingleTaskBatchItemsStream correctly includes debounce: item.options?.debounce at packages/trigger-sdk/src/v3/shared.ts:2041, confirming this is an oversight. The BatchItem type uses TriggerOptions which includes a debounce field (packages/core/src/v3/types/tasks.ts:946), so users can set it but it has no effect in the array path.

(Refers to lines 2234-2236)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
Expand Up @@ -2507,6 +2507,7 @@ async function batchTriggerAndWait_internal<TIdentifier extends string, TPayload
machine: item.options?.machine,
priority: item.options?.priority,
region: item.options?.region,
debounce: item.options?.debounce,
},
};
})
Expand Down