fix(executor): set github.action before the step env is evaluated
- commit
- f9d254a607dbce8a8e30a2e70fdfde85b337bbdd
- author
- brice · · 2026-10-12 22:46 UTC
- parent
- 76ad031
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +30 -1
M internal/executor/exec_step.go +0 -1
| @@ -91,7 +91,6 @@ func (j *job) prepareFileCommands(st *workflow.Step) error { | |||
| 91 | 91 | for _, c := range fileCommands { | |
| 92 | 92 | j.state.SetGitHub(c.ContextName, files[envVarName(c.ContextName)]) | |
| 93 | 93 | } | |
| 94 | - | j.state.SetGitHub("action", st.ID) | |
| 95 | 94 | j.state.SetGitHub("action_repository", "") | |
| 96 | 95 | j.state.SetGitHub("action_ref", "") | |
| 97 | 96 | if ev := j.state.EventJSON(); ev != "" { | |
| @@ -91,7 +91,6 @@ func (j *job) prepareFileCommands(st *workflow.Step) error { | |||||
| 91 | for _, c := range fileCommands { | 91 | for _, c := range fileCommands { | ||
| 92 | j.state.SetGitHub(c.ContextName, files[envVarName(c.ContextName)]) | 92 | j.state.SetGitHub(c.ContextName, files[envVarName(c.ContextName)]) | ||
| 93 | } | 93 | } | ||
| 94 | - | j.state.SetGitHub("action", st.ID) | |||
| 95 | j.state.SetGitHub("action_repository", "") | 94 | j.state.SetGitHub("action_repository", "") | ||
| 96 | j.state.SetGitHub("action_ref", "") | 95 | j.state.SetGitHub("action_ref", "") | ||
| 97 | if ev := j.state.EventJSON(); ev != "" { | 96 | if ev := j.state.EventJSON(); ev != "" { | ||
M internal/executor/job_test.go +26 -0
| @@ -619,3 +619,29 @@ func TestDeadlineOverridesTheTimeout(t *testing.T) { | |||
| 619 | 619 | t.Fatalf("took %v: %+v", time.Since(start), rec.steps[1]) | |
| 620 | 620 | } | |
| 621 | 621 | } | |
| 622 | + | ||
| 623 | + | // The env of a step reads its own id in github.action (StepsRunner.cs:118 sets | |
| 624 | + | // it before the env is evaluated), and the file command paths of the step | |
| 625 | + | // before it, as the recordings of github.com show for the dump step. | |
| 626 | + | func TestGithubActionInStepEnvIsTheStepsOwnID(t *testing.T) { | |
| 627 | + | needTools(t, "bash") | |
| 628 | + | rec, res := runLocal(t, wfHead+` steps: | |
| 629 | + | - run: echo one | |
| 630 | + | - id: named | |
| 631 | + | run: echo "$A|$GITHUB_ACTION|$GITHUB_ARTIFACTS" | sed 's#/[^|]*/##' | |
| 632 | + | env: | |
| 633 | + | A: ${{ github.action }} | |
| 634 | + | - run: echo "$A" | |
| 635 | + | env: | |
| 636 | + | A: ${{ github.action }} | |
| 637 | + | `, "j", nil) | |
| 638 | + | if res.Conclusion != ConclusionSuccess { | |
| 639 | + | t.Fatalf("%+v", res) | |
| 640 | + | } | |
| 641 | + | if got := rec.steps[2].Log[len(rec.steps[2].Log)-1]; !strings.HasPrefix(got, "named|named|artifacts_") { | |
| 642 | + | t.Errorf("named step: %q", got) | |
| 643 | + | } | |
| 644 | + | if got := rec.steps[3].Log[len(rec.steps[3].Log)-1]; got != "__run_2" { | |
| 645 | + | t.Errorf("third step: %q", got) | |
| 646 | + | } | |
| 647 | + | } | |
| @@ -619,3 +619,29 @@ func TestDeadlineOverridesTheTimeout(t *testing.T) { | |||||
| 619 | t.Fatalf("took %v: %+v", time.Since(start), rec.steps[1]) | 619 | t.Fatalf("took %v: %+v", time.Since(start), rec.steps[1]) | ||
| 620 | } | 620 | } | ||
| 621 | } | 621 | } | ||
| 622 | + | ||||
| 623 | + | // The env of a step reads its own id in github.action (StepsRunner.cs:118 sets | |||
| 624 | + | // it before the env is evaluated), and the file command paths of the step | |||
| 625 | + | // before it, as the recordings of github.com show for the dump step. | |||
| 626 | + | func TestGithubActionInStepEnvIsTheStepsOwnID(t *testing.T) { | |||
| 627 | + | needTools(t, "bash") | |||
| 628 | + | rec, res := runLocal(t, wfHead+` steps: | |||
| 629 | + | - run: echo one | |||
| 630 | + | - id: named | |||
| 631 | + | run: echo "$A|$GITHUB_ACTION|$GITHUB_ARTIFACTS" | sed 's#/[^|]*/##' | |||
| 632 | + | env: | |||
| 633 | + | A: ${{ github.action }} | |||
| 634 | + | - run: echo "$A" | |||
| 635 | + | env: | |||
| 636 | + | A: ${{ github.action }} | |||
| 637 | + | `, "j", nil) | |||
| 638 | + | if res.Conclusion != ConclusionSuccess { | |||
| 639 | + | t.Fatalf("%+v", res) | |||
| 640 | + | } | |||
| 641 | + | if got := rec.steps[2].Log[len(rec.steps[2].Log)-1]; !strings.HasPrefix(got, "named|named|artifacts_") { | |||
| 642 | + | t.Errorf("named step: %q", got) | |||
| 643 | + | } | |||
| 644 | + | if got := rec.steps[3].Log[len(rec.steps[3].Log)-1]; got != "__run_2" { | |||
| 645 | + | t.Errorf("third step: %q", got) | |||
| 646 | + | } | |||
| 647 | + | } | |||
M internal/executor/step.go +4 -0
| @@ -130,6 +130,10 @@ func (j *job) runStep(num int, st *workflow.Step) { | |||
| 130 | 130 | // Environment of the step (StepsRunner.cs:108-139): job env, then the step's | |
| 131 | 131 | // own, evaluated against the job env. | |
| 132 | 132 | j.mu.Lock() | |
| 133 | + | // github.action is the id of the step, set before the step's env is | |
| 134 | + | // evaluated (StepsRunner.cs:118), so that the env and the if of a step see | |
| 135 | + | // its own id (the dump step of the recordings reads __run_6, not __run_5). | |
| 136 | + | j.state.SetGitHub("action", st.ID) | |
| 133 | 137 | jobEnv := j.state.JobEnv() | |
| 134 | 138 | j.mu.Unlock() | |
| 135 | 139 | s.env = jobEnv.Clone() | |
| @@ -130,6 +130,10 @@ func (j *job) runStep(num int, st *workflow.Step) { | |||||
| 130 | // Environment of the step (StepsRunner.cs:108-139): job env, then the step's | 130 | // Environment of the step (StepsRunner.cs:108-139): job env, then the step's | ||
| 131 | // own, evaluated against the job env. | 131 | // own, evaluated against the job env. | ||
| 132 | j.mu.Lock() | 132 | j.mu.Lock() | ||
| 133 | + | // github.action is the id of the step, set before the step's env is | |||
| 134 | + | // evaluated (StepsRunner.cs:118), so that the env and the if of a step see | |||
| 135 | + | // its own id (the dump step of the recordings reads __run_6, not __run_5). | |||
| 136 | + | j.state.SetGitHub("action", st.ID) | |||
| 133 | jobEnv := j.state.JobEnv() | 137 | jobEnv := j.state.JobEnv() | ||
| 134 | j.mu.Unlock() | 138 | j.mu.Unlock() | ||
| 135 | s.env = jobEnv.Clone() | 139 | s.env = jobEnv.Clone() | ||