fix(execs): improve consistency across arg and env proprogration - #456
Merged
Conversation
Any $word inside an argument value was expanded against flow's internal env map and blanked when it did not resolve, so `flow run x 'costs $5'` silently became `costs `. Params were never expanded, so the two input types disagreed about what a value meant. The expansion was added in #284 so a serial ref's `args: ["var=$PARENT_VAR"]` could reference the parent env - a template use that only ever applied to strings written in a flow file. It was applied to every argument value instead, including text a user types. Split the two: user-supplied values are literals, and only author-written strings (an argument's `default`, and ref args as of a later commit) expand, through a single ExpandAuthored helper. It leaves an unresolved variable as written rather than deleting it, and treats `$$` as a literal `$`, for which there was previously no escape. Removing the expansion in resolveArgValues also stops it mutating the caller's inputArgs slice in place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016EQBWcu28DN2TmaceGca3P
`body` was always evaluated as an Expr expression, and a JSON object is valid Expr - it parses as a map literal. EvaluateString then stringified it with %v, so the example in the docs went out over the wire as `map[environment:prod version:1.0]`, with no error. A body that is already a JSON object or array is now sent as written; anything else is still an expression, so building a body with toJSON keeps working. Bare JSON scalars stay on the expression path, since `"hello"` means different things to the two languages. Also route url, headers and body through env.ExpandAuthored, so an unresolved $FOO in a URL now shows up as `$FOO` instead of an empty string. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016EQBWcu28DN2TmaceGca3P
Whether a parent's args and params reached a ref'd child depended on two unrelated things: whether the step declared an `args:` list, and which runner the child used. A params-only parent reached an exec child (leaking through the OS process environment) but not a request child, which builds its own map. An args-only parent reached neither, unless the step carried a non-empty `args:` whose contents were then ignored - a placeholder impossible to guess from the schema. The cause was that SetEnv resolved a complete environment and discarded it, only calling os.Setenv, so the sole path back into a child map was an os.Environ() scrape gated behind `if len(refConfig.Args) > 0`. SetEnv now returns what it resolved, and serial and parallel hand that to every child unconditionally. Seeding from the resolved map rather than os.Environ also keeps the ambient shell out of request URLs and Expr `env`. Arguments written on a step now win over inherited values, via a child-scoped precedence in BuildChildArgsEnvMap; at the top level env still wins, so a --param override continues to beat a positional arg. The two runners' copies of this block were byte-identical; they now share runner.ChildEnvAndArgs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016EQBWcu28DN2TmaceGca3P
The `#` closure shorthand was shown unquoted inside a YAML block, where a
space then `#` starts a comment. The example was silently truncated and flow
reported `unexpected token EOF` pointing past the end of a line the reader
never wrote. Quote it, and add a section to the expressions guide on the three
YAML characters that bite: `#`, a leading `{`, and a bare `: `.
Also document what the code now does: which contexts expand `$VAR` and which
carry literals, that `$$` escapes and an unresolved variable stays as written,
how a request body is interpreted, and that a step's `args:` list overrides
inherited values. The ref `args:` field had no guide coverage at all.
Adds the four behavior changes to the breaking changes page, and fixes a
copy-paste carryover describing a parallel step's `ref` as running in serial.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016EQBWcu28DN2TmaceGca3P
The shape that broke - a serial ref with no `args:` list - had no end-to-end coverage, which is part of why it went unnoticed. Asserts the child sees both the parent's positional arg and its param. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016EQBWcu28DN2TmaceGca3P
Comment-only, from the request body and step args descriptions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016EQBWcu28DN2TmaceGca3P
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
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.
No description provided.