Conversation
Fixes #2208. MCP call_workflow accepts idempotency_key; call route begins idempotency on executing paths only (never on 402 probes) and exposes the header in CORS. Co-authored-by: Cursor <cursoragent@cursor.com>
Begin HTTP idempotency before gatePayment to prevent double x402 settlement on same-key replays, return 503+release on recordPayment failures, and restore broken test suites with idempotency mocks. Co-authored-by: Cursor <cursoragent@cursor.com>
Scope HTTP idempotency to verified payer addresses, remove free-path reservation, release rows on payment gate rejection, split MPP recordPayment failure handling from x402, and wrap finalize in safeRecordIdempotentResponse. Co-authored-by: Cursor <cursoragent@cursor.com>
Fixes #2211. Skip Collect aggregation and done-targets when any iteration failed so downstream transfers/webhooks do not run after a failed loop body. Co-authored-by: Cursor <cursoragent@cursor.com>
Return handleForEachExecution summary from handleNestedForEach and gate routeAfterSuccess downstream on failedIterations so nested loop failures stop inner and outer post-loop continuation at any depth. Co-authored-by: Cursor <cursoragent@cursor.com>
Fix typecheck on PR #2217 by importing the summary type and NestedForEachHandler in tests. Co-authored-by: Cursor <cursoragent@cursor.com>
Use __forEachBodyFailure marker for post-loop gating, mark Collect visited on skip path, count failed iterations accurately, and wire firstFailureNodeId. Co-authored-by: Cursor <cursoragent@cursor.com>
…ver-only Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Keying on a decoded payer address let an attacker replay a victim's 200. Scope the pre-gate reservation to the payment header hash so identical retries skip settlement while a new signature remains a new charge. Co-authored-by: Cursor <cursoragent@cursor.com>
A skipped Collect was last in results with no data, so failed loops wrote undefined execution output. One shared failure type also stops concurrency throws from silently bypassing the gate. Co-authored-by: Cursor <cursoragent@cursor.com>
A recordPayment failure without Idempotency-Key passes null into safeRecordIdempotentResponse; expect.anything() does not match null. Co-authored-by: Cursor <cursoragent@cursor.com>
Normalize test indent and re-export the shared failure type in one statement so CI check matches Ultracite import rules. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Reservation stays inside the verified handler, but MPP left rows processing because finalize ran with no idem handle. Missing payer after settlement also 400'd after funds moved.
The summary was attached but unread, so outer failures still
named the nested loop. Skip output now keeps {error,nodeId}
instead of bare strings.
…ol write step The ETH Value field is resolved on its own path, so the per-input transform pass could not reach it. Transforms registered under the input name ethValue now run before resolveEthValue, so both the core write and the daily value cap see the converted value. The documented unit of the field stays ether. Claude-Session: https://claude.ai/code/session_01VmdsN69YAEto7D7pvjA2Pt
The synthesiser's exhaustiveness check returned the `never`-typed variable, which at runtime would emit the transform kind name in place of the argument expression if an unregistered kind ever reached it. Keep the compile-time guarantee and fall back to the untransformed expression instead. weiToEther now trims before the template check, so a padded template reference passes through rather than being rejected as non-integer wei. Correct the weiToEther branch comment: the kind is intended for the virtual ethValue field, not guaranteed to be registered only there. Extract findProtocolAction so the args builder and the ethValue transform pass share one (function, contract) lookup rule. Claude-Session: https://claude.ai/code/session_01JoBsmnuUJrgRxUyH8Mremw
The synthesiser test now asserts the kind is actually registered before checking the emitted source, and that the source contains no ether conversion call. The protocol write step's lookup-miss test now registers a transform on a different action of the same protocol, so it proves the (protocol, action, input) lookup misses rather than proving an empty registry does nothing. A lookup that ignored the action slug fails this test; before the change it passed. weiToEther's rejection test covers exponent and hex notation, both of which BigInt would otherwise accept or misread. Claude-Session: https://claude.ai/code/session_01JoBsmnuUJrgRxUyH8Mremw
A wait-timeout running body no longer drops the reservation, so the same Idempotency-Key replays the executionId. Log payment gate failures, bound key length, and drop unused GatePaymentOptions.idem. Co-authored-by: Cursor <cursoragent@cursor.com>
Skipped Collect now mirrors the success path in currentOutputs. Nested firstFailureNodeId is only read when failedIterations is present. Drop unnecessary step-registry test mocks. Co-authored-by: Cursor <cursoragent@cursor.com>
…orm in the harness Review findings from @suisuss. Blocking 1: applyEthValueTransform passed the raw value through when findProtocolAction missed. meta comes from an unvalidated JSON.parse of a possibly stale _protocolMeta, so a stale contractKey or functionName handed resolveEthValue a wei integer that parseEther reads as ether - 10^18 times the intended amount - and value-ledger.ts runs uncapped when a reservation is held or organizationId is absent, so the cap does not always catch it. It now returns a result type and the step refuses with an actionable error naming the stale metadata. The guard is scoped to a present, non-empty value: an unresolvable action that sends nothing still runs, which is the pre-existing behaviour of the args builder. Blocking 2: lib/test-data/encode-action.ts parseEther'd ethValue without ever looking up an ethValue transform, so the golden and on-chain harnesses would have carried 10^18x silently while the calldata matched. It now performs the same lookup protocol-write.ts does. Three existing fixtures declared `actions: []` while their metas named an action to execute. Nothing read that field on this path before, so the inconsistency was invisible; the payable value now resolves through the same lookup, so the fixtures carry the action they claim. Slugs, contract keys and function names match protocols/uniswap-v3.ts and protocols/wrapped.ts. No test's assertion changed. Mechanical, all from the review: - The synthesiser comment had the direction backwards. The SDK already treats ethValue as wei; the runtime treats it as ether, so registering weiToEther makes the runtime agree with the SDK for that action and leaves untransformed actions divergent. Corrected, and it no longer implies the SDK alone is the tracked follow-up. - The synthesiser test registered weiToEther on a real ABI input and asserted the SDK does not convert, entrenching the divergence the adjacent comment forbids. Rewritten: the legitimate ethValue case is asserted, and the ABI-input case is framed as blast-radius if the invariant is violated, not as a supported shape. - Added that invariant as a registry-wide guard: no weiToEther registration may target a declared ABI input. It is paired with a test proving the detector catches a deliberate violation, so it cannot pass vacuously. - Pinned the (contract, function) uniqueness the action lookup depends on. Chose a registry-wide test over a runtime assert in protocol-derive.ts: same invariant, no risk of a core registration path throwing on a shape no protocol has today. - Noted the seam at checkUniswapNativeEthPreflight, the one consumer reading ethValue pre-transform, and why a zero check survives the unit change while a future threshold would not. Tests added for the gaps the review named: the fail-closed branch, the scoped no-value case, a throwing transform reserving nothing against the cap, and encode-action.ts. The cap assertion is no longer shallow - it parses both consumers' strings and compares against the wei that went in, rather than only proving the same string reached both. Not addressed here: whether ethValue should mean one unit everywhere rather than per registry entry. That is the question you are taking to the core team, and nothing above depends on the answer. Claude-Session: https://claude.ai/code/session_016ANfo1ucTf9Q2v4e253WgJ
Second review from @suisuss, plus an adversarial pass over the result. From the review: - The refusal now calls logUserError, so a node whose _protocolMeta has drifted is findable rather than only visible when someone reports a failed run. Pinned by the refusal test. - tests/unit/protocol-encode-transform-invariants.test.ts no longer mutates the registry at all. The detector takes the entries as an argument, so the live-registry case and the synthetic violation share one function with no state between them, and nothing wipes the eager production entries for whatever runs next in that file. - The scoping test said "sends no value" while passing "", which short-circuits before the lookup. It now runs over "" and undefined and says what it exercises. There is no third case: any value reaching the lookup is non-empty and belongs to the refusal test. - The invariant is a runtime check now, not only a CI one. registerEncodeTransform throws on weiToEther against a declared ABI input. lib/protocol-registry injects the lookup, which keeps the dependency one-way, and registerProtocol re-checks the ordering the first check cannot see: a transform registered eagerly before its protocol exists. The adversarial pass found three more, all fixed: - registerProtocol asserted AFTER inserting into the registry, so a caller catching the throw was left holding exactly the pairing the guard prevents - protocol resolvable, illegal transform live. It now asserts first, reading the definition's own actions. Regression test included. - A typo'd slug was accepted, never applied, and invisible to both the guard and the detector, which cannot judge an action they cannot find. So a registration could read as done while the step went on parsing a raw wei quote as ether. orphanEncodeTransforms plus two tests close it. - The synthesiser's weiToEther branch returned the raw expression on a path that should be unreachable. A silent passthrough there is the wrong-number shape: SDK source converting nothing while the runtime converts. It throws now. The earlier claim that this branch could not be covered was wrong - a partial mock of getEncodeTransformKind reaches it - so the test is restored rather than deleted. Verified: 21,991 unit tests pass, tsc clean. The 9 failures are the pre-existing jsdom/localStorage tests that fail on staging too. Claude-Session: https://claude.ai/code/session_01HtRvnNWK84xJaS5SpwXxpy
Reject over-long Idempotency-Key in POST before paid handlers, and return invalid_key from the shared helper instead of throwing RangeError.
…utputs Preserve nodeId from stamped throws in concurrency catches, and write results/outputs for legacy in-body Collect on the skip path so findOrphanedNodes does not treat it as clean.
Drop the non-null assertion in the invalid_key test, and stop importing the server-only idempotency module from MCP tools so Vitest can collect tool registration suites.
…ecution-metric fix: zero webhook-execution metric via lazy label init
joelorzet
added
the
metrics-db-reviewed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.