diff --git a/.changeset/reset-idempotency-key-64-char.md b/.changeset/reset-idempotency-key-64-char.md new file mode 100644 index 0000000000..b203043a48 --- /dev/null +++ b/.changeset/reset-idempotency-key-64-char.md @@ -0,0 +1,5 @@ +--- +"@trigger.dev/core": patch +--- + +`idempotencyKeys.reset()` now works when your idempotency key is itself 64 characters long (for example if you use a hash of your own as the key). Previously any 64-character key was assumed to be already hashed, so passing one along with a `scope` silently ignored the scope and the reset never found a matching run. Keys returned by `idempotencyKeys.create()` continue to be reset exactly as before. diff --git a/packages/core/src/v3/idempotencyKeys.test.ts b/packages/core/src/v3/idempotencyKeys.test.ts index f511a85f86..7c9a8a9cf2 100644 --- a/packages/core/src/v3/idempotencyKeys.test.ts +++ b/packages/core/src/v3/idempotencyKeys.test.ts @@ -1,9 +1,15 @@ -import { describe, it, expect } from "vitest"; +import { createServer, type Server } from "node:http"; +import type { AddressInfo } from "node:net"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { apiClientManager } from "./apiClientManager-api.js"; import { createIdempotencyKey, getIdempotencyKeyOptions, + makeIdempotencyKey, + resetIdempotencyKey, resetIdempotencyKeyCatalog, } from "./idempotencyKeys.js"; +import { digestSHA256 } from "./utils/crypto.js"; describe("idempotencyKeys metadata retention", () => { it("retains key/scope options for every key created in a run, even beyond 1000", async () => { @@ -40,3 +46,254 @@ describe("idempotencyKeys metadata retention", () => { expect(getIdempotencyKeyOptions(key)).toBeUndefined(); }); }); + +describe("resetIdempotencyKey", () => { + const digestShapedKey = "a".repeat(64); + + let server: Server; + let resetKeys: string[] = []; + /** Keys the server has runs for. `undefined` means "accept every key". */ + let existingKeys: Set | undefined; + /** Per-key failure statuses, applied before the existence check. */ + let statusByKey: Map; + + function notFoundMessage(key: string) { + return `No runs found with idempotency key: ${key}`; + } + + async function resetAndCaptureKey( + ...args: Parameters + ): Promise { + resetKeys = []; + await resetIdempotencyKey(...args); + expect(resetKeys).toHaveLength(1); + return resetKeys[0]!; + } + + beforeEach(async () => { + resetIdempotencyKeyCatalog(); + resetKeys = []; + existingKeys = undefined; + statusByKey = new Map(); + + server = createServer((req, res) => { + req.resume(); + req.on("end", () => { + const match = /^\/api\/v1\/idempotencyKeys\/(.+)\/reset$/.exec(req.url ?? ""); + if (!match) { + res.writeHead(404).end(); + return; + } + + const key = decodeURIComponent(match[1]!); + resetKeys.push(key); + + const failWith = statusByKey.get(key); + if (failWith !== undefined) { + res.writeHead(failWith, { "content-type": "application/json" }); + res.end(JSON.stringify({ error: `request failed for ${key}` })); + return; + } + + if (existingKeys !== undefined && !existingKeys.has(key)) { + res.writeHead(404, { "content-type": "application/json" }); + res.end(JSON.stringify({ error: notFoundMessage(key) })); + return; + } + + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ id: "run_reset" })); + }); + }); + + await new Promise((resolve) => { + server.listen(0, "127.0.0.1", () => resolve()); + }); + + apiClientManager.setGlobalAPIClientConfiguration({ + baseURL: `http://127.0.0.1:${(server.address() as AddressInfo).port}`, + accessToken: "tr_test_key", + }); + }); + + afterEach(async () => { + apiClientManager.disable(); + resetIdempotencyKeyCatalog(); + await new Promise((resolve) => server.close(() => resolve())); + }); + + it("hashes 64-character key material when an explicit scope is passed", async () => { + const created = await createIdempotencyKey(digestShapedKey, { scope: "global" }); + + // The reset can happen in a different process from the trigger + resetIdempotencyKeyCatalog(); + + expect(await resetAndCaptureKey("my-task", digestShapedKey, { scope: "global" })).toBe(created); + }); + + it("hashes 64-character key material for run scope when an explicit scope is passed", async () => { + const parentRunId = "run_abc123"; + const expected = await digestSHA256(`${digestShapedKey}-${parentRunId}`); + + expect( + await resetAndCaptureKey("my-task", digestShapedKey, { scope: "run", parentRunId }) + ).toBe(expected); + }); + + it("sends a key created with idempotencyKeys.create() unchanged while the catalog knows it", async () => { + const created = await createIdempotencyKey("my-key", { scope: "global" }); + existingKeys = new Set([created]); + + expect(await resetAndCaptureKey("my-task", created)).toBe(created); + expect(await resetAndCaptureKey("my-task", created, { scope: "global" })).toBe(created); + }); + + it("sends a created key unchanged when no scope is passed and the catalog is cold", async () => { + const created = await createIdempotencyKey("my-key", { scope: "global" }); + + // The reset can happen in a different process from the create + resetIdempotencyKeyCatalog(); + existingKeys = new Set([created]); + + expect(await resetAndCaptureKey("my-task", created)).toBe(created); + }); + + it("falls back to the verbatim key when a created key is reset with a scope and the catalog is cold", async () => { + const created = await createIdempotencyKey("my-key", { scope: "global" }); + + resetIdempotencyKeyCatalog(); + existingKeys = new Set([created]); + + await resetIdempotencyKey("my-task", created, { scope: "global" }); + + // The derived hash misses, so the already-hashed key is retried verbatim + expect(resetKeys).toEqual([await digestSHA256(created), created]); + }); + + it("sends a 64-character key unchanged when no scope is passed", async () => { + expect(await resetAndCaptureKey("my-task", digestShapedKey)).toBe(digestShapedKey); + }); + + it("resets 64-character material that trigger stored verbatim when no scope is passed", async () => { + // trigger() forwards 64-character material as-is, so that is what the server stored + expect(await makeIdempotencyKey(digestShapedKey)).toBe(digestShapedKey); + existingKeys = new Set([digestShapedKey]); + + expect(await resetAndCaptureKey("my-task", digestShapedKey)).toBe(digestShapedKey); + }); + + it("does not fall back when the derived hash for 64-character material matches", async () => { + const created = await createIdempotencyKey(digestShapedKey, { scope: "global" }); + + resetIdempotencyKeyCatalog(); + existingKeys = new Set([created]); + + await resetIdempotencyKey("my-task", digestShapedKey, { scope: "global" }); + + expect(resetKeys).toEqual([created]); + }); + + it("falls back to the verbatim key when the derived hash fails with a 503", async () => { + // The server answers 503, not 404, when it cannot check the buffer for a miss + const created = await createIdempotencyKey("my-key", { scope: "global" }); + + resetIdempotencyKeyCatalog(); + statusByKey.set(await digestSHA256(created), 503); + existingKeys = new Set([created]); + + await resetIdempotencyKey( + "my-task", + created, + { scope: "global" }, + { retry: { maxAttempts: 1 } } + ); + + expect(resetKeys).toEqual([await digestSHA256(created), created]); + }); + + it("surfaces the derived key's error when it fails with a 503 and the fallback finds nothing", async () => { + const derived = await digestSHA256(digestShapedKey); + statusByKey.set(derived, 503); + existingKeys = new Set(); + + await expect( + resetIdempotencyKey( + "my-task", + digestShapedKey, + { scope: "global" }, + { retry: { maxAttempts: 1 } } + ) + ).rejects.toMatchObject({ status: 503 }); + + expect(resetKeys).toEqual([derived, digestShapedKey]); + }); + + it("surfaces the fallback's error when it fails with something other than a 404", async () => { + const derived = await digestSHA256(digestShapedKey); + statusByKey.set(derived, 404); + statusByKey.set(digestShapedKey, 503); + + await expect( + resetIdempotencyKey( + "my-task", + digestShapedKey, + { scope: "global" }, + { retry: { maxAttempts: 1 } } + ) + ).rejects.toMatchObject({ status: 503 }); + + expect(resetKeys).toEqual([derived, digestShapedKey]); + }); + + it("surfaces the derived key's error when both attempts 404", async () => { + const derived = await digestSHA256(digestShapedKey); + existingKeys = new Set(); + + await expect( + resetIdempotencyKey("my-task", digestShapedKey, { scope: "global" }) + ).rejects.toThrow(notFoundMessage(derived)); + + expect(resetKeys).toEqual([derived, digestShapedKey]); + }); + + it("hashes key material that is not 64 characters", async () => { + const created = await createIdempotencyKey("my-key", { scope: "global" }); + resetIdempotencyKeyCatalog(); + + expect(await resetAndCaptureKey("my-task", "my-key", { scope: "global" })).toBe(created); + }); + + it("sends a 64-character key verbatim when run scope cannot be derived", async () => { + const created = await createIdempotencyKey("my-key", { scope: "run" }); + + resetIdempotencyKeyCatalog(); + existingKeys = new Set([created]); + + // No parentRunId and no task context, so the hash is underivable + expect(await resetAndCaptureKey("my-task", created, { scope: "run" })).toBe(created); + }); + + it("sends a 64-character key verbatim when attempt scope cannot be derived", async () => { + existingKeys = new Set([digestShapedKey]); + + expect(await resetAndCaptureKey("my-task", digestShapedKey, { scope: "attempt" })).toBe( + digestShapedKey + ); + }); + + it("still throws for non-64-character material when run scope cannot be derived", async () => { + await expect(resetIdempotencyKey("my-task", "my-key", { scope: "run" })).rejects.toThrow( + "parentRunId is required for 'run' scope" + ); + + expect(resetKeys).toEqual([]); + }); + + it("still throws for non-64-character material when attempt scope cannot be derived", async () => { + await expect( + resetIdempotencyKey("my-task", "my-key", { scope: "attempt", parentRunId: "run_abc123" }) + ).rejects.toThrow("parentRunId and attemptNumber are required for 'attempt' scope"); + + expect(resetKeys).toEqual([]); + }); +}); diff --git a/packages/core/src/v3/idempotencyKeys.ts b/packages/core/src/v3/idempotencyKeys.ts index 585f38c1c3..643a22ae98 100644 --- a/packages/core/src/v3/idempotencyKeys.ts +++ b/packages/core/src/v3/idempotencyKeys.ts @@ -8,6 +8,7 @@ import { taskContext } from "./task-context-api.js"; import type { IdempotencyKey } from "./types/idempotencyKeys.js"; import { digestSHA256 } from "./utils/crypto.js"; import type { ZodFetchOptions } from "./apiClient/core.js"; +import { NotFoundError } from "./apiClient/errors.js"; // Re-export types from catalog for backwards compatibility export type { @@ -234,19 +235,19 @@ export async function resetIdempotencyKey( ): Promise<{ id: string }> { const client = apiClientManager.clientOrThrow(); - // If the key is already a 64-char hash, use it directly - if (typeof idempotencyKey === "string" && idempotencyKey.length === 64) { - return client.resetIdempotencyKey(taskIdentifier, idempotencyKey, requestOptions); - } + // A 64-char key is only assumed pre-hashed if the catalog knows it, or there's no scope to hash with + const is64CharKey = typeof idempotencyKey === "string" && idempotencyKey.length === 64; - // Try to extract options from an IdempotencyKey created with idempotencyKeys.create() - const attachedOptions = - typeof idempotencyKey === "string" ? getIdempotencyKeyOptions(idempotencyKey) : undefined; + if (is64CharKey) { + const isCreatedKey = getIdempotencyKeyOptions(idempotencyKey) !== undefined; - const scope = attachedOptions?.scope ?? options?.scope ?? "run"; - const keyArray = Array.isArray(idempotencyKey) - ? idempotencyKey - : [attachedOptions?.key ?? String(idempotencyKey)]; + if (isCreatedKey || options?.scope === undefined) { + return client.resetIdempotencyKey(taskIdentifier, idempotencyKey, requestOptions); + } + } + + const scope = options?.scope ?? "run"; + const keyArray = Array.isArray(idempotencyKey) ? idempotencyKey : [idempotencyKey]; // Build scope suffix based on scope type let scopeSuffix: string[] = []; @@ -254,6 +255,10 @@ export async function resetIdempotencyKey( case "run": { const parentRunId = options?.parentRunId ?? taskContext?.ctx?.run.id; if (!parentRunId) { + // We can't derive a hash, but a 64-char key may already be one, so try it rather than fail + if (is64CharKey) { + return client.resetIdempotencyKey(taskIdentifier, idempotencyKey, requestOptions); + } throw new Error( "resetIdempotencyKey: parentRunId is required for 'run' scope when called outside a task context" ); @@ -265,6 +270,9 @@ export async function resetIdempotencyKey( const parentRunId = options?.parentRunId ?? taskContext?.ctx?.run.id; const attemptNumber = options?.attemptNumber ?? taskContext?.ctx?.attempt.number; if (!parentRunId || attemptNumber === undefined) { + if (is64CharKey) { + return client.resetIdempotencyKey(taskIdentifier, idempotencyKey, requestOptions); + } throw new Error( "resetIdempotencyKey: parentRunId and attemptNumber are required for 'attempt' scope when called outside a task context" ); @@ -277,5 +285,18 @@ export async function resetIdempotencyKey( // Generate the hash using the same algorithm as createIdempotencyKey const hash = await generateIdempotencyKey(keyArray.concat(scopeSuffix)); - return client.resetIdempotencyKey(taskIdentifier, hash, requestOptions); + if (!is64CharKey) { + return client.resetIdempotencyKey(taskIdentifier, hash, requestOptions); + } + + // Hashing a 64-char key is a guess, so if it fails at all, still try the key verbatim + try { + return await client.resetIdempotencyKey(taskIdentifier, hash, requestOptions); + } catch (error) { + try { + return await client.resetIdempotencyKey(taskIdentifier, idempotencyKey, requestOptions); + } catch (fallbackError) { + throw fallbackError instanceof NotFoundError ? error : fallbackError; + } + } }