Conversation
…riables are accessible in subprocesses and maintain precedence over OS environment variables
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #57 +/- ##
==========================================
- Coverage 92.33% 92.07% -0.27%
==========================================
Files 32 32
Lines 2309 2333 +24
==========================================
+ Hits 2132 2148 +16
- Misses 111 115 +4
- Partials 66 70 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Exports raid “Set” variables into the environment of subprocesses spawned by Shell and Script tasks so scripts/child processes can read them via $VAR, with raid variables taking precedence over OS env on key collisions.
Changes:
- Add
buildSubprocessEnv()to merge OS env + session exports + raidVars in precedence order. - Apply the merged env to
execShellandexecScriptsubprocesses. - Add tests and docs describing/validating subprocess env propagation and precedence.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/internal/lib/task_runner.go |
Introduces env-merging helper and sets cmd.Env for Shell/Script subprocesses. |
src/internal/lib/task_runner_test.go |
Adds helpers + tests asserting raid var inheritance/precedence in subprocess env. |
site/docs/features/variables.mdx |
Documents availability/precedence of raid variables inside Shell/Script subprocesses. |
| // buildSubprocessEnv returns the OS environment merged with the current | ||
| // commandSession exports and raidVars (Set tasks). Order matters: exec.Cmd | ||
| // uses the LAST occurrence of a duplicate key, so we append in increasing | ||
| // priority — OS env first, session next, raidVars last so they win on | ||
| // collision. Mirrors the lookup order used by expandRaid. | ||
| // | ||
| // Applied to Shell and Script tasks so a `Set FOO bar` task earlier in the | ||
| // sequence is visible to the spawned process (and any children it spawns) | ||
| // as $FOO. Without this, raidVars only resolved via raid's pre-expansion of | ||
| // the cmd string — which couldn't reach into a Script's source or into a | ||
| // child process spawned from inside a Shell command. | ||
| func buildSubprocessEnv() []string { | ||
| env := os.Environ() | ||
| if commandSession != nil { | ||
| commandSession.mu.RLock() | ||
| for k, v := range commandSession.vars { | ||
| env = append(env, k+"="+v) | ||
| } | ||
| commandSession.mu.RUnlock() | ||
| } | ||
| raidVarsMu.RLock() | ||
| for k, v := range raidVars { | ||
| env = append(env, k+"="+v) | ||
| } | ||
| raidVarsMu.RUnlock() | ||
| return env |
There was a problem hiding this comment.
buildSubprocessEnv blindly appends raidVars into cmd.Env. Since Set tasks currently only validate non-empty names (and the JSON schema doesn’t constrain var), a user could set an invalid env key/value (e.g., containing NUL, or a name containing '=' which would be interpreted as a different key at exec time). With this change, a single bad raidVar could cause subsequent Shell/Script tasks to fail at process start or behave unexpectedly. Consider validating task.Var/task.Value in Set tasks (preferred) or filtering/guarding in buildSubprocessEnv before appending.
| scriptPath := writeTempScript(t, "#!/bin/sh\nprintf 'got=%s' \"$ISSUE20_FOO\"\n") | ||
|
|
||
| if err := execScript(Task{Type: Script, Path: scriptPath}); err != nil { | ||
| t.Fatalf("execScript: %v", err) | ||
| } |
There was a problem hiding this comment.
These new tests call execScript() with direct script execution (no Runner). That path is already skipped elsewhere on Windows because executing a .sh file directly isn’t supported there; CI runs windows-latest, so this test will fail. Consider either skipping this test on Windows (like TestExecuteTask_script_directExecution) or running the script via a runner (e.g., set Runner to "bash" and keep asserting the env behavior).
| scriptPath := writeTempScript(t, "#!/bin/sh\nprintf 'got=%s' \"$ISSUE20_OVERRIDE\"\n") | ||
|
|
||
| if err := execScript(Task{Type: Script, Path: scriptPath}); err != nil { | ||
| t.Fatalf("execScript: %v", err) | ||
| } |
There was a problem hiding this comment.
Same Windows issue as the previous test: execScript(Task{Path: scriptPath}) triggers direct .sh execution which isn’t supported on Windows, so this will fail in the Windows CI job. Please skip on Windows or set a Runner (e.g., "bash") so the test is portable while still validating raid var precedence in subprocess env.
| task := Task{Type: Shell, Cmd: childPath, Literal: true} | ||
| if err := execShell(task); err != nil { | ||
| t.Fatalf("execShell: %v", err) | ||
| } |
There was a problem hiding this comment.
On Windows, execShell defaults to cmd /c, which won’t execute a .sh child script path like this. Since CI includes windows-latest, this test is likely to fail there. Options: skip on Windows, or force a POSIX shell in the task (e.g., Shell: "bash") and invoke the child script in a bash-friendly way so the child still inherits the environment.
… entries and ensure proper inheritance in Script tasks
This pull request ensures that variables set by
Settasks ("raid variables") are exported as environment variables to subprocesses spawned byScriptandShelltasks. This allows scripts and child processes to access these variables directly via$VAR, and guarantees that raid variables override any OS environment variable with the same name. The implementation includes a new helper to merge environments in the correct order and comprehensive tests to validate the behavior.The most important changes include:
Environment Variable Handling for Subprocesses:
buildSubprocessEnvhelper function intask_runner.goto merge the OS environment, session variables, and raid variables, ensuring raid variables take precedence when names collide. This function is now used for allScriptandShellsubprocesses. [1] [2] [3]variables.mdxto explain that raid variables are available in the environment of subprocesses spawned byScriptandShelltasks, and that raid variables override OS environment variables with the same name.Testing and Validation:
task_runner_test.goto verify that raid variables are correctly exported to subprocess environments, that they override OS variables, and that the environment order is correct. [1] [2]These changes ensure more predictable and powerful scripting capabilities when using raid tasks.…riables are accessible in subprocesses and maintain precedence over OS environment variables