Conversation
…EEP-442)
HTTP Request `endpoint` (and `httpHeaders`/`httpBody`) string fields
already pass through the workflow template substitution layer like
every other config string -- but `{{@prep:Prep.url}}` was resolving to
the empty string when `prep` was a `code/run-code` returning
`{ url: "..." }`. The bug shows up at HTTP request validation as
`URL is required`.
Root cause: `resolveFromOutputData` only unwraps the HTTP-style
`{ data: ... }` wrapper. `runCodeStep` returns `{ success, result, logs }`,
so when a downstream template references `Prep.url` (no explicit
`result.` prefix) the resolver finds neither `data.url` nor `data.data.url`
and falls back to the "missing path" branch, which substitutes "".
Fix: extend `resolveFromOutputData` with a `.result` fallback that
mirrors the existing `.data` fallback. Backward-compatible (`Prep.result.url`
still resolves directly via the top-level path) and also fixes any other
string field that references a code/run-code output's inner field
(protocol-action arg fields, downstream `code` strings, etc.).
Unblocks the dynamic Bridge Route Optimizer / MEV-Aware Swap Quote
workflows on the catalog roadmap.
- Add `hasNestedResultShape` helper alongside `hasNestedDataShape`
- Walk `.data` then `.result` so HTTP responses still take precedence
when both wrappers are present
- Export `processTemplates` and `resolveFromOutputData` so the new
unit test can drive the executor's substitution layer directly
- Add `tests/unit/http-request-template-substitution.test.ts` covering
the bug repro plus header/body templating and ordering precedence
Verified end-to-end on local dev: trigger -> prep (code/run-code
returning {url}) -> across (HTTP GET with endpoint={{@prep:Prep.url}})
returned a real Across API response.
Address review feedback on PR #1147: - Tighten `hasNestedDataShape` to also reject `data === null`, matching `hasNestedResultShape` -- removes the asymmetry the reviewer flagged and keeps the type guard honest (the runtime was already null-safe via resolveConfigFieldPath, but the predicate now matches behavior). - Pin the intentional `.data` -> `.result` fall-through with an explicit test, so future readers know the behavior on outputs that carry both wrappers is by design. - Document the existing limitation: a primitive `.result` (e.g. a code/run-code that returned a bare string) is not unwrapped because the fallback can only walk into objects. Whole-output references still resolve via top-level. - Cover null-wrapper guards for both `.data` and `.result`. Tests: 11 -> 15 cases, all pass. No code path changes beyond the hasNestedDataShape predicate; existing 3696 unit tests still pass.
🧹 PR Environment Cleaned UpThe PR environment has been successfully deleted. Deleted Resources:
All resources have been cleaned up and will no longer incur costs. |
ℹ️ No PR Environment to Clean UpNo PR environment was found for this PR. This is expected if:
|
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
1 participant
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.
Summary
endpoint,httpHeaders, andhttpBodystring fields silently resolved to""when the template referenced acode/run-codeupstream node, surfacing asHTTP request failed: URL is requiredat validation{success, data, ...}shapes;code/run-codewraps user returns in{success, result, logs}and the resolver had no fallback for thatlib/workflow/executor/executor.workflow.ts: extendresolveFromOutputDatawith a.resultfallback that mirrors the existing.datafallback. Backward-compatible -- order is top-level ->.data->.result, so every previously-resolving template still resolves to the same valueLinear: KEEP-442 -- unblocks dynamic Bridge Route Optimizer / MEV-Aware Swap Quote on the KEEP-430 catalog roadmap.
Repro (now passes)
Pre-fix:
acrossfailed withURL is required(input.endpoint = "").Post-fix:
acrossreturns a real Across API quote (verified end-to-end on local dev).Test plan
pnpm test:unit tests/unit/http-request-template-substitution.test.ts-- 11 new tests, all pass; covers endpoint, httpHeaders, httpBody templating against run-code and HTTP upstreams plus precedence orderingpnpm test:unit-- 3696 passed, 1 skipped (no regressions)pnpm type-check-- cleanpnpm check-- no new lint issues in changed fileskh wf create+kh wf runagainstlocalhost:3000returned a real Across quote