fix(core): don't assume a 64-character idempotency key is pre-hashed on reset - #4626
fix(core): don't assume a 64-character idempotency key is pre-hashed on reset#4626claude[bot] wants to merge 4 commits into
Conversation
…on reset `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 a user key that is itself a 64-character digest had an explicitly passed `scope` silently discarded and was sent un-hashed, matching no run. A 64-character string is now only passed through when there is evidence it is already a hash: the idempotency key catalog recognises it (so it came from `idempotencyKeys.create()`), or no `scope` was passed and the length is the only signal available. An explicit `scope` is an explicit request to derive the hash, so it is always honoured. This keeps both existing behaviours intact: a key from `idempotencyKeys.create()` is still forwarded unchanged, and 64-character key material passed straight to `trigger()` and reset without a scope is still sent verbatim. `isIdempotencyKey` is deliberately untouched, since the trigger path is self-consistent and changing it would invalidate already-stored keys. Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: d38e1b8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Co-Authored-By: Claude <noreply@anthropic.com>
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ived hash misses A 64-character string passed to reset() with an explicit scope is ambiguous: it may be raw key material to hash, or a key already produced by create(). The catalog can only tell the two apart in-process, and workers clear it at each run boundary, so resetting a created key from another run or process while passing a scope would double-hash it and match nothing. Send the derived hash first, then fall back to the value verbatim on a 404. Non-404s propagate immediately, and a double miss surfaces the derived attempt's error. Co-Authored-By: Claude <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…lback Two problems with the 64-character reset fallback: The server answers 503, not 404, when Postgres matched nothing and it could not check the buffer, so a miss could arrive as a non-404 and the NotFoundError-only catch skipped the verbatim retry. The speculative request is a guess by construction, so any failure now falls through to the verbatim key. A double miss still surfaces the derived attempt's error, and a non-404 from the fallback surfaces instead. A pre-hashed key with "run" or "attempt" scope and no parentRunId threw before any request was made, which used to work. Send those verbatim when the key is already 64 characters; shorter material still throws, since there is nothing useful to send. Co-Authored-By: Claude <noreply@anthropic.com>
| 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); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔍 parentRunId/attemptNumber without an explicit scope is still ignored for 64-character keys
The new pass-through condition treats "no scope passed" as evidence the key is already a hash (packages/core/src/v3/idempotencyKeys.ts:244), but a caller can also signal hashing intent by passing only parentRunId (or parentRunId + attemptNumber) and relying on the default scope: "run". For non-64-character material that path derives sha256(key-parentRunId); for 64-character material the key is sent verbatim and no fallback hash is attempted, so the reset can 404 with no retry. This is pre-existing behaviour rather than a regression, and the JSDoc for resetIdempotencyKey (packages/core/src/v3/idempotencyKeys.ts:207-211) does say a raw string "requires options.scope", so it may be intentional — but the new "positive evidence" rule would be more consistent if the presence of parentRunId/attemptNumber also counted as evidence that the caller wants a derived hash.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Intentional, so leaving it. parentRunId is documented as subordinate to scope rather than an independent signal — ResetIdempotencyKeyOptions describes it as "Required if scope is 'run' or 'attempt'" (:198), and the JSDoc at :210 names scope as what the raw-string form requires — so counting it as derivation intent would be a contract change rather than a consistency fix.
It also wouldn't be strictly better. For a 64-character key with only parentRunId, verbatim is the more likely intent, so deriving first would add a guaranteed extra request and a server-side 404 to every such call that works today, and where both a verbatim-keyed and a derived-keyed run exist it would reset the latter instead of the former. Since this is pre-existing rather than a regression here, it's better as a follow-up if someone actually hits it.
Generated by Claude Code
1 participant
Requested by Matt Aitken · Slack thread
idempotencyKeys.reset()now honours an explicitly passedscopeeven when the key material happens to be 64 characters long.Before:
resetIdempotencyKeytreated 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 — thescopeyou 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 string is only passed through when there is evidence it is already a hash. Otherwise an explicit
scopeis honoured and the hash is derived, at any key length.How
The pass-through now requires positive evidence rather than a length guess:
idempotencyKeys.create(), orscopewas passed, so there is nothing to derive a hash from and the length is the only available signal.An explicit
scopeis an explicit request to derive the hash, so it is never short-circuited past.Both existing behaviours are preserved:
idempotencyKeys.create()is still forwarded unchanged (catalog hit), including when ascopeis also passed — it is never hashed twice.trigger()is stored un-hashed, and resetting it with noscopestill sends it verbatim.isIdempotencyKeyis 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?.scopefallbacks below the 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
Testing
New tests in
packages/core/src/v3/idempotencyKeys.test.tsdrive the realresetIdempotencyKeyagainst a local HTTP server and assert on the exact value that reaches the wire — nothing is mocked. They cover:{ scope: "global" }resolves to the same valueidempotencyKeys.create()produces (fails without this change){ scope: "run", parentRunId }derives the run-salted hash (fails without this change)idempotencyKeys.create()is forwarded unchanged, with and without ascopescopeis still forwarded unchangedReverting only the source change turns the two new scope tests red and leaves the three compatibility tests green.
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 ascopesilently ignored the scope and the reset never found a matching run.Follow-ups (not in this PR)
docs/idempotency.mdxdescribes theidempotencyKeyparameter ofreset()as "the 64-character hash string" in one place while showing raw material plus{ scope: "global" }a few lines later. Worth reconciling.ctx.run.idempotencyKey, the run page and theidempotency_keyquery column all show the user-provided key. That is what leads people to send a value reset cannot match.