From 513deb5c7bc42eb7c9376ab4a58758ca07c640d2 Mon Sep 17 00:00:00 2001 From: Jon Zeolla Date: Sat, 7 Mar 2026 18:24:40 -0500 Subject: [PATCH 1/3] fix: only resolve Ref vars in global var post-processing to avoid clobbering sh: vars The ReplaceVars call added in #2632 re-processes all TaskfileVars after they've been resolved by rangeFunc. For sh: variables, this overwrites the already-resolved Value with a new Var that has the original Sh string but nil Value, causing dynamic variables to resolve to empty strings when the Taskfile has includes. Fix by only resolving variables that actually have Ref values set, leaving already-resolved sh: variables untouched. Fixes #2720 Co-Authored-By: Claude Opus 4.6 --- compiler.go | 8 +++++- executor_test.go | 26 +++++++++++++++++++ testdata/var_sh_with_includes/Included.yml | 6 +++++ testdata/var_sh_with_includes/Taskfile.yml | 18 +++++++++++++ ...es-sh_var_not_clobbered_by_includes.golden | 2 ++ 5 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 testdata/var_sh_with_includes/Included.yml create mode 100644 testdata/var_sh_with_includes/Taskfile.yml create mode 100644 testdata/var_sh_with_includes/testdata/TestShVarWithIncludes-sh_var_not_clobbered_by_includes.golden diff --git a/compiler.go b/compiler.go index c0679c2cbf..022ee263fe 100644 --- a/compiler.go +++ b/compiler.go @@ -115,7 +115,13 @@ func (c *Compiler) getVariables(t *ast.Task, call *Call, evaluateShVars bool) (* } } // Resolve any outstanding 'Ref' values in global vars (esp. globals from imported Taskfiles). - c.TaskfileVars = templater.ReplaceVars(c.TaskfileVars, &templater.Cache{Vars: result}) + // Only process variables with Ref set to avoid clobbering already-resolved sh: variables. + cache := &templater.Cache{Vars: result} + for k, v := range c.TaskfileVars.All() { + if v.Ref != "" { + c.TaskfileVars.Set(k, ast.Var{Value: templater.ResolveRef(v.Ref, cache)}) + } + } if t != nil { for k, v := range t.IncludeVars.All() { diff --git a/executor_test.go b/executor_test.go index 05080b3474..370e3befd5 100644 --- a/executor_test.go +++ b/executor_test.go @@ -946,6 +946,32 @@ func TestReference(t *testing.T) { } } +func TestShVarWithIncludes(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + call string + }{ + { + name: "sh var not clobbered by includes", + call: "default", + }, + } + + for _, test := range tests { + NewExecutorTest(t, + WithName(test.name), + WithExecutorOptions( + task.WithDir("testdata/var_sh_with_includes"), + task.WithSilent(true), + task.WithForce(true), + ), + WithTask(cmp.Or(test.call, "default")), + ) + } +} + func TestVarInheritance(t *testing.T) { enableExperimentForTest(t, &experiments.EnvPrecedence, 1) tests := []struct { diff --git a/testdata/var_sh_with_includes/Included.yml b/testdata/var_sh_with_includes/Included.yml new file mode 100644 index 0000000000..9c0f4e437a --- /dev/null +++ b/testdata/var_sh_with_includes/Included.yml @@ -0,0 +1,6 @@ +version: '3' + +tasks: + noop: + cmds: + - "true" diff --git a/testdata/var_sh_with_includes/Taskfile.yml b/testdata/var_sh_with_includes/Taskfile.yml new file mode 100644 index 0000000000..7e3bf197dc --- /dev/null +++ b/testdata/var_sh_with_includes/Taskfile.yml @@ -0,0 +1,18 @@ +version: '3' + +includes: + other: + taskfile: ./Included.yml + internal: true + +vars: + DYNAMIC_VAR: + sh: echo "hello" + DERIVED_VAR: "{{.DYNAMIC_VAR}} world" + +tasks: + default: + cmds: + - echo "DYNAMIC_VAR={{.DYNAMIC_VAR}}" + - echo "DERIVED_VAR={{.DERIVED_VAR}}" + silent: true diff --git a/testdata/var_sh_with_includes/testdata/TestShVarWithIncludes-sh_var_not_clobbered_by_includes.golden b/testdata/var_sh_with_includes/testdata/TestShVarWithIncludes-sh_var_not_clobbered_by_includes.golden new file mode 100644 index 0000000000..066c073193 --- /dev/null +++ b/testdata/var_sh_with_includes/testdata/TestShVarWithIncludes-sh_var_not_clobbered_by_includes.golden @@ -0,0 +1,2 @@ +DYNAMIC_VAR=hello +DERIVED_VAR=hello world From ba7f26567c7f6286733740c2cc3bf0b95b983974 Mon Sep 17 00:00:00 2001 From: Jon Zeolla Date: Sat, 7 Mar 2026 18:29:27 -0500 Subject: [PATCH 2/3] test: revert src fix to prove test catches the regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Temporarily reverting the compiler.go fix while keeping the test to demonstrate TDD — CI should fail on TestShVarWithIncludes. Co-Authored-By: Claude Opus 4.6 --- compiler.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/compiler.go b/compiler.go index 022ee263fe..c0679c2cbf 100644 --- a/compiler.go +++ b/compiler.go @@ -115,13 +115,7 @@ func (c *Compiler) getVariables(t *ast.Task, call *Call, evaluateShVars bool) (* } } // Resolve any outstanding 'Ref' values in global vars (esp. globals from imported Taskfiles). - // Only process variables with Ref set to avoid clobbering already-resolved sh: variables. - cache := &templater.Cache{Vars: result} - for k, v := range c.TaskfileVars.All() { - if v.Ref != "" { - c.TaskfileVars.Set(k, ast.Var{Value: templater.ResolveRef(v.Ref, cache)}) - } - } + c.TaskfileVars = templater.ReplaceVars(c.TaskfileVars, &templater.Cache{Vars: result}) if t != nil { for k, v := range t.IncludeVars.All() { From a750b3bb388be8fa639b1e205a84b589309a7a8a Mon Sep 17 00:00:00 2001 From: Jon Zeolla Date: Sat, 7 Mar 2026 18:29:42 -0500 Subject: [PATCH 3/3] =?UTF-8?q?fix:=20restore=20fix=20=E2=80=94=20only=20r?= =?UTF-8?q?esolve=20Ref=20vars=20to=20avoid=20clobbering=20sh:=20vars?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-applying the fix now that CI has demonstrated the test catches the regression. CI should pass on this commit. Co-Authored-By: Claude Opus 4.6 --- compiler.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/compiler.go b/compiler.go index c0679c2cbf..022ee263fe 100644 --- a/compiler.go +++ b/compiler.go @@ -115,7 +115,13 @@ func (c *Compiler) getVariables(t *ast.Task, call *Call, evaluateShVars bool) (* } } // Resolve any outstanding 'Ref' values in global vars (esp. globals from imported Taskfiles). - c.TaskfileVars = templater.ReplaceVars(c.TaskfileVars, &templater.Cache{Vars: result}) + // Only process variables with Ref set to avoid clobbering already-resolved sh: variables. + cache := &templater.Cache{Vars: result} + for k, v := range c.TaskfileVars.All() { + if v.Ref != "" { + c.TaskfileVars.Set(k, ast.Var{Value: templater.ResolveRef(v.Ref, cache)}) + } + } if t != nil { for k, v := range t.IncludeVars.All() {