fix(core): don't assume a 64-character idempotency key is pre-hashed on reset (#4626)
<!-- ccr-slack-attribution --> _Requested by **Matt Aitken** · [Slack thread](https://triggerdotdev.slack.com/archives/C045W9WM3E1/p1786741966214949?thread_ts=1786741966.214949&cid=C045W9WM3E1)_ `idempotencyKeys.reset()` now honours an explicitly passed `scope` even when the key material happens to be 64 characters long. **Before:** `resetIdempotencyKey` treated *any* 64-character string as an already-computed hash and sent it to the API verbatim. That short-circuit ran before the scope logic, so if your key material is itself a 64-character digest (a common pattern when you hash your own dedup identity) the `scope` you passed was silently discarded and the un-hashed material went on the wire. The server stores the hash, so the reset matched no run and returned 404 every single time. Key material of any other length worked fine, which made this look arbitrary. **After:** a 64-character key with an explicit `scope` is sent verbatim first and, only when that attempt comes back a definitive not-found, retried as the derived scope hash. Every call that worked before behaves identically, and the previously impossible case now resolves on the fallback. ## How A 64-character string is forwarded unchanged, exactly as before, when: - the idempotency key catalog recognises it (it came from `idempotencyKeys.create()` in this process), or - no `scope` was passed, so there is nothing to derive a hash from, or - the scope hash cannot be derived (e.g. `scope: "run"` outside a task context with no `parentRunId`). Otherwise the key is ambiguous: it may be raw material the caller wants hashed with the scope, or it may already be the stored hash. Reset sends the verbatim value first because that is what every previous version sent, so anything that resolved before still resolves with the same single request, the same target run, and the same errors. The derived hash is the new behaviour, so it only runs once the verbatim attempt has failed with a 404, a definitive "no run under this key". Any other error (a 503, a connection error) leaves the verbatim key's state unknown, and resetting a different key on unknown state would be an untargeted write the caller never asked for, so those errors surface unchanged. That has an honest cost: when the endpoint answers 503 for a miss it cannot confirm, the caller sees the 503 and retries rather than silently falling through to the derived key. When both attempts miss, the verbatim attempt's 404 is surfaced, again matching what previous versions threw. A side benefit of this order: a key from `idempotencyKeys.create()` reset with a `scope` from a cold process resolves in a single request, because the created key is itself the stored value. `isIdempotencyKey` is deliberately left alone: it applies the same length rule on the trigger path, but it is self-consistent there, and changing it would invalidate already-stored keys. The `attachedOptions?.key` / `attachedOptions?.scope` fallbacks below the old guard were unreachable (every catalog entry is a 64-character digest, so it always hit the short-circuit first) and re-deriving from them produces the identical hash anyway. They are removed rather than left as dead code. --- ## ✅ Checklist - [x] I have followed every step in the [contributing guide](https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md) - [x] The PR title follows the convention. - [x] I ran and tested the code works --- ## Testing Tests in `packages/core/src/v3/idempotencyKeys.test.ts` drive the real `resetIdempotencyKey` against a local HTTP server and assert on the exact values that reach the wire, in order. Nothing is mocked. They cover: - 64-character material + explicit `scope` derives the global- and run-scoped hash once the verbatim key misses (fails without this change) - the verbatim key wins when runs exist under both the verbatim value and the derived hash, so the pre-existing target is preserved - keys from `idempotencyKeys.create()` are forwarded unchanged: catalog hit, no scope, and scope with a cold catalog (the last now a single request) - a transient failure of the verbatim attempt surfaces its error without ever touching the derived key - error surfacing: a double miss reports the key the caller passed, and a non-404 from the fallback is not swallowed - ordinary short material is still hashed, and underivable run/attempt scopes still send a 64-character key verbatim while still throwing for shorter material ``` pnpm run test ./src/v3/idempotencyKeys.test.ts --run # 18 passed pnpm run build --filter @trigger.dev/core # clean pnpm run format && pnpm run lint # clean ``` --- ## Changelog `idempotencyKeys.reset()` now works when your idempotency key is itself 64 characters long. 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. --- ## Follow-ups (not in this PR) - `docs/idempotency.mdx` describes the `idempotencyKey` parameter of `reset()` as "the 64-character hash string" in one place while showing raw material plus `{ scope: "global" }` a few lines later. Worth reconciling. - No surface currently exposes the stored hash that the reset endpoint matches on: `ctx.run.idempotencyKey`, the run page and the `idempotency_key` query column all show the user-provided key. That is what leads people to send a value reset cannot match. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Matt Aitken <matt@mattaitken.com>
This commit is contained in:
claude[bot]
@@ -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.
|
||||
@@ -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,238 @@ 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<string> | undefined;
|
||||
/** Per-key failure statuses, applied before the existence check. */
|
||||
let statusByKey: Map<string, number>;
|
||||
|
||||
function notFoundMessage(key: string) {
|
||||
return `No runs found with idempotency key: ${key}`;
|
||||
}
|
||||
|
||||
async function resetAndCaptureKey(
|
||||
...args: Parameters<typeof resetIdempotencyKey>
|
||||
): Promise<string> {
|
||||
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<void>((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<void>((resolve) => server.close(() => resolve()));
|
||||
});
|
||||
|
||||
it("derives the hash for 64-character key material with an explicit scope when the verbatim key misses", async () => {
|
||||
const created = await createIdempotencyKey(digestShapedKey, { scope: "global" });
|
||||
|
||||
resetIdempotencyKeyCatalog();
|
||||
existingKeys = new Set([created]);
|
||||
|
||||
await resetIdempotencyKey("my-task", digestShapedKey, { scope: "global" });
|
||||
|
||||
expect(resetKeys).toEqual([digestShapedKey, created]);
|
||||
});
|
||||
|
||||
it("derives the run-scoped hash for 64-character key material when the verbatim key misses", async () => {
|
||||
const parentRunId = "run_abc123";
|
||||
const expected = await digestSHA256(`${digestShapedKey}-${parentRunId}`);
|
||||
existingKeys = new Set([expected]);
|
||||
|
||||
await resetIdempotencyKey("my-task", digestShapedKey, { scope: "run", parentRunId });
|
||||
|
||||
expect(resetKeys).toEqual([digestShapedKey, 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("resolves a created key in one request when reset with a scope and the catalog is cold", async () => {
|
||||
const created = await createIdempotencyKey("my-key", { scope: "global" });
|
||||
|
||||
resetIdempotencyKeyCatalog();
|
||||
existingKeys = new Set([created]);
|
||||
|
||||
expect(await resetAndCaptureKey("my-task", created, { scope: "global" })).toBe(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("resets the verbatim run when runs exist under both the verbatim key and the derived hash", async () => {
|
||||
const created = await createIdempotencyKey(digestShapedKey, { scope: "global" });
|
||||
|
||||
resetIdempotencyKeyCatalog();
|
||||
existingKeys = new Set([digestShapedKey, created]);
|
||||
|
||||
await resetIdempotencyKey("my-task", digestShapedKey, { scope: "global" });
|
||||
|
||||
expect(resetKeys).toEqual([digestShapedKey]);
|
||||
});
|
||||
|
||||
it("does not reset the derived run when the verbatim attempt fails transiently", async () => {
|
||||
const created = await createIdempotencyKey(digestShapedKey, { scope: "global" });
|
||||
|
||||
resetIdempotencyKeyCatalog();
|
||||
statusByKey.set(digestShapedKey, 503);
|
||||
existingKeys = new Set([created]);
|
||||
|
||||
await expect(
|
||||
resetIdempotencyKey(
|
||||
"my-task",
|
||||
digestShapedKey,
|
||||
{ scope: "global" },
|
||||
{ retry: { maxAttempts: 1 } }
|
||||
)
|
||||
).rejects.toMatchObject({ status: 503 });
|
||||
|
||||
expect(resetKeys).toEqual([digestShapedKey]);
|
||||
});
|
||||
|
||||
it("surfaces the fallback's error when it fails with something other than a 404", async () => {
|
||||
const derived = await digestSHA256(digestShapedKey);
|
||||
statusByKey.set(digestShapedKey, 404);
|
||||
statusByKey.set(derived, 503);
|
||||
|
||||
await expect(
|
||||
resetIdempotencyKey(
|
||||
"my-task",
|
||||
digestShapedKey,
|
||||
{ scope: "global" },
|
||||
{ retry: { maxAttempts: 1 } }
|
||||
)
|
||||
).rejects.toMatchObject({ status: 503 });
|
||||
|
||||
expect(resetKeys).toEqual([digestShapedKey, derived]);
|
||||
});
|
||||
|
||||
it("surfaces the verbatim 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(digestShapedKey));
|
||||
|
||||
expect(resetKeys).toEqual([digestShapedKey, derived]);
|
||||
});
|
||||
|
||||
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([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
|
||||
if (is64CharKey) {
|
||||
const isCreatedKey = getIdempotencyKeyOptions(idempotencyKey) !== undefined;
|
||||
|
||||
if (isCreatedKey || options?.scope === undefined) {
|
||||
return client.resetIdempotencyKey(taskIdentifier, idempotencyKey, requestOptions);
|
||||
}
|
||||
}
|
||||
|
||||
// Try to extract options from an IdempotencyKey created with idempotencyKeys.create()
|
||||
const attachedOptions =
|
||||
typeof idempotencyKey === "string" ? getIdempotencyKeyOptions(idempotencyKey) : undefined;
|
||||
|
||||
const scope = attachedOptions?.scope ?? options?.scope ?? "run";
|
||||
const keyArray = Array.isArray(idempotencyKey)
|
||||
? idempotencyKey
|
||||
: [attachedOptions?.key ?? String(idempotencyKey)];
|
||||
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,21 @@ 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);
|
||||
}
|
||||
|
||||
try {
|
||||
return await client.resetIdempotencyKey(taskIdentifier, idempotencyKey, requestOptions);
|
||||
} catch (error) {
|
||||
if (!(error instanceof NotFoundError)) {
|
||||
throw error;
|
||||
}
|
||||
|
||||
try {
|
||||
return await client.resetIdempotencyKey(taskIdentifier, hash, requestOptions);
|
||||
} catch (fallbackError) {
|
||||
throw fallbackError instanceof NotFoundError ? error : fallbackError;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user