-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix(core): don't assume a 64-character idempotency key is pre-hashed on reset #4626
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
c8d8177
6016987
34057ba
d38e1b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,26 +235,30 @@ 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); | ||
| } | ||
| } | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
Comment on lines
+241
to
+247
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 parentRunId/attemptNumber without an explicit scope is still ignored for 64-character keys The new pass-through condition treats "no Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Intentional, so leaving it. It also wouldn't be strictly better. For a 64-character key with only Generated by Claude Code |
||
|
|
||
| const scope = options?.scope ?? "run"; | ||
| const keyArray = Array.isArray(idempotencyKey) ? idempotencyKey : [idempotencyKey]; | ||
|
|
||
| // Build scope suffix based on scope type | ||
| let scopeSuffix: string[] = []; | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
|
||
| switch (scope) { | ||
| 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; | ||
| } | ||
| } | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
|
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.