fix(executor): runner steps in the steps context and composite env errors as on GitHub
- commit
- ec1337c85ac05df1fa12134dc4e8460043676384
- author
- brice · · 2026-10-14 11:56 UTC
- committer
- bnema · · 2026-10-14 11:56 UTC
- parent
- a674b39
- signature
- SSH Unverified
SHA256:VOCA5lYARHcbKDZ7GhTV45VmcjWnJ6ZQIBazj17LivAThe key is unknown
10 files · +195 -33
M internal/executor/actions_composite.go +18 -12
| @@ -224,20 +224,26 @@ func (j *job) runEmbedded(parent *stepRun, c *node, scope string) { | |||
| 224 | 224 | // the line is masked again when it is written, as every line is. | |
| 225 | 225 | id := scope + "." + c.st.ID | |
| 226 | 226 | if envErr != nil { | |
| 227 | - | // The error of the environment is logged and the step marked failed, then | |
| 228 | - | // the start marker is printed (CompositeActionHandler.cs:296-302, :308-324). | |
| 229 | - | // DIVERGENCE: the runner does not stop there. It falls through to the | |
| 230 | - | // condition (:372-390); success() of an embedded main step reads | |
| 227 | + | // The error of the environment is logged and the step marked failed | |
| 228 | + | // (CompositeActionHandler.cs:296-302, SetStepConclusion :529-533), then the | |
| 229 | + | // runner does not stop: it prints the start marker (:308-324), evaluates | |
| 230 | + | // the condition (:372-390; success() of an embedded main step reads | |
| 231 | 231 | // github.action_status, set from the composite's result before the env was | |
| 232 | - | // evaluated (:246-248, SuccessFunction.cs:28), so the default condition still | |
| 233 | - | // holds and the runner RUNS the step, without the env that failed, and ends | |
| 234 | - | // with the marker of its failed result and the elapsed time. Here the step | |
| 235 | - | // does not run and the end marker says duration_ms=0. | |
| 232 | + | // evaluated, :246-248, SuccessFunction.cs:28, so the default condition | |
| 233 | + | // still holds) and runs the step (RunStepAsync :481-527) without the env | |
| 234 | + | // that failed. Its result stays Failed (nothing resets it), and the end | |
| 235 | + | // marker (:431-440) carries it with the elapsed time. | |
| 236 | 236 | j.errorf(s.num, "%s", templateNotValid(envErr, true)) | |
| 237 | - | j.out(parent.num, startMarker(j.mask(s.name), id)) | |
| 238 | - | j.finishStep(s, resFailed) | |
| 239 | - | j.out(parent.num, endMarker(id, "failure", "failure", 0)) | |
| 240 | - | return | |
| 237 | + | s.envFailed = true | |
| 238 | + | // SetStepConclusion publishes Failed to the steps context at once | |
| 239 | + | // (:529-533, UpdateGlobalStepsContext), so the condition of the step, and | |
| 240 | + | // of the steps after it, see steps.<id>.outcome == 'failure'. Nothing is | |
| 241 | + | // merged into the composite's result yet: that is done when the step ends | |
| 242 | + | // (:505-512, finishStep), which also replaces this entry with the final | |
| 243 | + | // outcome and conclusion. | |
| 244 | + | j.mu.Lock() | |
| 245 | + | j.state.SetStepResult(c.st.ID, string(resFailed.conclusion()), string(resFailed.conclusion())) | |
| 246 | + | j.mu.Unlock() | |
| 241 | 247 | } | |
| 242 | 248 | q := qstep{n: c, stage: parent.stage} | |
| 243 | 249 | switch parent.stage { | |
| @@ -224,20 +224,26 @@ func (j *job) runEmbedded(parent *stepRun, c *node, scope string) { | |||||
| 224 | // the line is masked again when it is written, as every line is. | 224 | // the line is masked again when it is written, as every line is. | ||
| 225 | id := scope + "." + c.st.ID | 225 | id := scope + "." + c.st.ID | ||
| 226 | if envErr != nil { | 226 | if envErr != nil { | ||
| 227 | - | // The error of the environment is logged and the step marked failed, then | 227 | + | // The error of the environment is logged and the step marked failed |
| 228 | - | // the start marker is printed (CompositeActionHandler.cs:296-302, :308-324). | 228 | + | // (CompositeActionHandler.cs:296-302, SetStepConclusion :529-533), then the |
| 229 | - | // DIVERGENCE: the runner does not stop there. It falls through to the | 229 | + | // runner does not stop: it prints the start marker (:308-324), evaluates |
| 230 | - | // condition (:372-390); success() of an embedded main step reads | 230 | + | // the condition (:372-390; success() of an embedded main step reads |
| 231 | // github.action_status, set from the composite's result before the env was | 231 | // github.action_status, set from the composite's result before the env was | ||
| 232 | - | // evaluated (:246-248, SuccessFunction.cs:28), so the default condition still | 232 | + | // evaluated, :246-248, SuccessFunction.cs:28, so the default condition |
| 233 | - | // holds and the runner RUNS the step, without the env that failed, and ends | 233 | + | // still holds) and runs the step (RunStepAsync :481-527) without the env |
| 234 | - | // with the marker of its failed result and the elapsed time. Here the step | 234 | + | // that failed. Its result stays Failed (nothing resets it), and the end |
| 235 | - | // does not run and the end marker says duration_ms=0. | 235 | + | // marker (:431-440) carries it with the elapsed time. |
| 236 | j.errorf(s.num, "%s", templateNotValid(envErr, true)) | 236 | j.errorf(s.num, "%s", templateNotValid(envErr, true)) | ||
| 237 | - | j.out(parent.num, startMarker(j.mask(s.name), id)) | 237 | + | s.envFailed = true |
| 238 | - | j.finishStep(s, resFailed) | 238 | + | // SetStepConclusion publishes Failed to the steps context at once |
| 239 | - | j.out(parent.num, endMarker(id, "failure", "failure", 0)) | 239 | + | // (:529-533, UpdateGlobalStepsContext), so the condition of the step, and |
| 240 | - | return | 240 | + | // of the steps after it, see steps.<id>.outcome == 'failure'. Nothing is |
| 241 | + | // merged into the composite's result yet: that is done when the step ends | |||
| 242 | + | // (:505-512, finishStep), which also replaces this entry with the final | |||
| 243 | + | // outcome and conclusion. | |||
| 244 | + | j.mu.Lock() | |||
| 245 | + | j.state.SetStepResult(c.st.ID, string(resFailed.conclusion()), string(resFailed.conclusion())) | |||
| 246 | + | j.mu.Unlock() | |||
| 241 | } | 247 | } | ||
| 242 | q := qstep{n: c, stage: parent.stage} | 248 | q := qstep{n: c, stage: parent.stage} | ||
| 243 | switch parent.stage { | 249 | switch parent.stage { | ||
M internal/executor/actions_docker_test.go +53 -0
| @@ -5,6 +5,7 @@ package executor | |||
| 5 | 5 | import ( | |
| 6 | 6 | "os" | |
| 7 | 7 | "path/filepath" | |
| 8 | + | "regexp" | |
| 8 | 9 | "strings" | |
| 9 | 10 | "testing" | |
| 10 | 11 | "time" | |
| @@ -252,3 +253,55 @@ func TestDockerfileContextIsTheLexicalParent(t *testing.T) { | |||
| 252 | 253 | t.Errorf("-f %q, context %q", f, c) | |
| 253 | 254 | } | |
| 254 | 255 | } | |
| 256 | + | ||
| 257 | + | // act-docker-image, act-docker-dockerfile: "Pull <image>" and "Build <action>" | |
| 258 | + | // are runner steps of the job and are in the steps context under a random | |
| 259 | + | // 32-hex id with their outcome and conclusion (JobExtension.cs:434-440, | |
| 260 | + | // ExecutionContext.cs:644). | |
| 261 | + | func TestSetupStepsAreInTheStepsContext(t *testing.T) { | |
| 262 | + | needTools(t, "bash") | |
| 263 | + | forge := newForge(t, map[string][]byte{"/tarball/o/d/v1": tarOf(t, "o-d-x", files{"Dockerfile": "FROM alpine\n"})}) | |
| 264 | + | d, _ := fakeDocker(t, 0) | |
| 265 | + | rec, res := actionRun(t, files{}, " - uses: docker://alpine:3.20\n - uses: o/d@v1\n - name: dump\n run: echo \"S=$S\"\n env:\n S: ${{ join(steps.*.conclusion, ',') }} ${{ join(steps.*.outcome, ',') }} ${{ toJSON(steps) }}\n", func(c *Config) { | |
| 266 | + | c.Actions.DockerPath = d | |
| 267 | + | c.Actions.Resolver = forge.resolver("") | |
| 268 | + | }) | |
| 269 | + | if res.Conclusion != ConclusionSuccess { | |
| 270 | + | t.Fatalf("%+v %v", res, rec.names()) | |
| 271 | + | } | |
| 272 | + | got := logOf(rec.step(t, "dump")) | |
| 273 | + | re := regexp.MustCompile(`(?s)S=success,success success,success \{.*"[0-9a-f]{32}": \{\s*"outputs": \{\},\s*"outcome": "success",\s*"conclusion": "success"\s*\}.*"[0-9a-f]{32}": \{`) | |
| 274 | + | if !re.MatchString(got) { | |
| 275 | + | t.Errorf("%s", got) | |
| 276 | + | } | |
| 277 | + | } | |
| 278 | + | ||
| 279 | + | // A setup step that is skipped (the pull before it failed) is in the steps | |
| 280 | + | // context too, with outcome and conclusion "skipped": the context names are | |
| 281 | + | // assigned before anything runs and CompleteStep(Skipped) updates the context | |
| 282 | + | // (JobExtension.cs:434-440, StepsRunner.cs:231-234, ExecutionContext.cs:644-647). | |
| 283 | + | func TestSkippedSetupStepIsInTheStepsContext(t *testing.T) { | |
| 284 | + | needTools(t, "bash") | |
| 285 | + | d, _ := fakeDocker(t, 3) // the three pulls of the first image fail | |
| 286 | + | rec, res := actionRun(t, files{}, " - uses: docker://alpine:3.20\n - uses: docker://busybox:1\n - name: dump\n if: always()\n run: echo \"S=$S\"\n env:\n S: ${{ toJSON(steps) }}\n", func(c *Config) { | |
| 287 | + | c.Actions.DockerPath = d | |
| 288 | + | c.Actions.Backoff = func() time.Duration { return time.Millisecond } | |
| 289 | + | }) | |
| 290 | + | if res.Conclusion != ConclusionFailure { | |
| 291 | + | t.Fatalf("%+v %v", res, rec.names()) | |
| 292 | + | } | |
| 293 | + | if got := strings.Join(rec.names(), "|"); got != "Set up job|Pull alpine:3.20|Pull busybox:1|Run alpine:3.20|Run busybox:1|dump|Complete job" { | |
| 294 | + | t.Fatalf("%s", got) | |
| 295 | + | } | |
| 296 | + | got := logOf(rec.step(t, "dump")) | |
| 297 | + | got = got[strings.LastIndex(got, "\nS={"):] // the header echoes the env too | |
| 298 | + | for _, want := range []string{"failure", "skipped"} { | |
| 299 | + | re := regexp.MustCompile(`(?s)"[0-9a-f]{32}": \{\s*"outputs": \{\},\s*"outcome": "` + want + `",\s*"conclusion": "` + want + `"\s*\}`) | |
| 300 | + | if !re.MatchString(got) { | |
| 301 | + | t.Errorf("no %s runner step in:\n%s", want, got) | |
| 302 | + | } | |
| 303 | + | } | |
| 304 | + | if n := len(regexp.MustCompile(`"[0-9a-f]{32}": \{`).FindAllString(got, -1)); n != 2 { | |
| 305 | + | t.Errorf("%d runner steps in the context, want 2:\n%s", n, got) | |
| 306 | + | } | |
| 307 | + | } | |
| @@ -5,6 +5,7 @@ package executor | |||||
| 5 | import ( | 5 | import ( | ||
| 6 | "os" | 6 | "os" | ||
| 7 | "path/filepath" | 7 | "path/filepath" | ||
| 8 | + | "regexp" | |||
| 8 | "strings" | 9 | "strings" | ||
| 9 | "testing" | 10 | "testing" | ||
| 10 | "time" | 11 | "time" | ||
| @@ -252,3 +253,55 @@ func TestDockerfileContextIsTheLexicalParent(t *testing.T) { | |||||
| 252 | t.Errorf("-f %q, context %q", f, c) | 253 | t.Errorf("-f %q, context %q", f, c) | ||
| 253 | } | 254 | } | ||
| 254 | } | 255 | } | ||
| 256 | + | ||||
| 257 | + | // act-docker-image, act-docker-dockerfile: "Pull <image>" and "Build <action>" | |||
| 258 | + | // are runner steps of the job and are in the steps context under a random | |||
| 259 | + | // 32-hex id with their outcome and conclusion (JobExtension.cs:434-440, | |||
| 260 | + | // ExecutionContext.cs:644). | |||
| 261 | + | func TestSetupStepsAreInTheStepsContext(t *testing.T) { | |||
| 262 | + | needTools(t, "bash") | |||
| 263 | + | forge := newForge(t, map[string][]byte{"/tarball/o/d/v1": tarOf(t, "o-d-x", files{"Dockerfile": "FROM alpine\n"})}) | |||
| 264 | + | d, _ := fakeDocker(t, 0) | |||
| 265 | + | rec, res := actionRun(t, files{}, " - uses: docker://alpine:3.20\n - uses: o/d@v1\n - name: dump\n run: echo \"S=$S\"\n env:\n S: ${{ join(steps.*.conclusion, ',') }} ${{ join(steps.*.outcome, ',') }} ${{ toJSON(steps) }}\n", func(c *Config) { | |||
| 266 | + | c.Actions.DockerPath = d | |||
| 267 | + | c.Actions.Resolver = forge.resolver("") | |||
| 268 | + | }) | |||
| 269 | + | if res.Conclusion != ConclusionSuccess { | |||
| 270 | + | t.Fatalf("%+v %v", res, rec.names()) | |||
| 271 | + | } | |||
| 272 | + | got := logOf(rec.step(t, "dump")) | |||
| 273 | + | re := regexp.MustCompile(`(?s)S=success,success success,success \{.*"[0-9a-f]{32}": \{\s*"outputs": \{\},\s*"outcome": "success",\s*"conclusion": "success"\s*\}.*"[0-9a-f]{32}": \{`) | |||
| 274 | + | if !re.MatchString(got) { | |||
| 275 | + | t.Errorf("%s", got) | |||
| 276 | + | } | |||
| 277 | + | } | |||
| 278 | + | ||||
| 279 | + | // A setup step that is skipped (the pull before it failed) is in the steps | |||
| 280 | + | // context too, with outcome and conclusion "skipped": the context names are | |||
| 281 | + | // assigned before anything runs and CompleteStep(Skipped) updates the context | |||
| 282 | + | // (JobExtension.cs:434-440, StepsRunner.cs:231-234, ExecutionContext.cs:644-647). | |||
| 283 | + | func TestSkippedSetupStepIsInTheStepsContext(t *testing.T) { | |||
| 284 | + | needTools(t, "bash") | |||
| 285 | + | d, _ := fakeDocker(t, 3) // the three pulls of the first image fail | |||
| 286 | + | rec, res := actionRun(t, files{}, " - uses: docker://alpine:3.20\n - uses: docker://busybox:1\n - name: dump\n if: always()\n run: echo \"S=$S\"\n env:\n S: ${{ toJSON(steps) }}\n", func(c *Config) { | |||
| 287 | + | c.Actions.DockerPath = d | |||
| 288 | + | c.Actions.Backoff = func() time.Duration { return time.Millisecond } | |||
| 289 | + | }) | |||
| 290 | + | if res.Conclusion != ConclusionFailure { | |||
| 291 | + | t.Fatalf("%+v %v", res, rec.names()) | |||
| 292 | + | } | |||
| 293 | + | if got := strings.Join(rec.names(), "|"); got != "Set up job|Pull alpine:3.20|Pull busybox:1|Run alpine:3.20|Run busybox:1|dump|Complete job" { | |||
| 294 | + | t.Fatalf("%s", got) | |||
| 295 | + | } | |||
| 296 | + | got := logOf(rec.step(t, "dump")) | |||
| 297 | + | got = got[strings.LastIndex(got, "\nS={"):] // the header echoes the env too | |||
| 298 | + | for _, want := range []string{"failure", "skipped"} { | |||
| 299 | + | re := regexp.MustCompile(`(?s)"[0-9a-f]{32}": \{\s*"outputs": \{\},\s*"outcome": "` + want + `",\s*"conclusion": "` + want + `"\s*\}`) | |||
| 300 | + | if !re.MatchString(got) { | |||
| 301 | + | t.Errorf("no %s runner step in:\n%s", want, got) | |||
| 302 | + | } | |||
| 303 | + | } | |||
| 304 | + | if n := len(regexp.MustCompile(`"[0-9a-f]{32}": \{`).FindAllString(got, -1)); n != 2 { | |||
| 305 | + | t.Errorf("%d runner steps in the context, want 2:\n%s", n, got) | |||
| 306 | + | } | |||
| 307 | + | } | |||
M internal/executor/actions_job.go +11 -1
| @@ -25,10 +25,20 @@ func (j *job) prepareJob() ([]qstep, error) { | |||
| 25 | 25 | return nil, j.setupError(err) | |
| 26 | 26 | } | |
| 27 | 27 | queue := j.setupSteps() | |
| 28 | + | // JobExtension.cs:434-440: every runner step of the job gets a random "N" | |
| 29 | + | // context name before anything runs; its final result, skipped included, | |
| 30 | + | // is in the steps context. The ones the composite actions run get a "__" | |
| 31 | + | // name (CompositeActionHandler.cs:121-123) and are not registered, which is | |
| 32 | + | // why only this queue has ids. | |
| 33 | + | for i := range queue { | |
| 34 | + | queue[i].ctxID = newRunnerStepID() | |
| 35 | + | } | |
| 28 | 36 | // JobExtension.cs:317-331: the images of the actions are pulled first, then | |
| 29 | 37 | // the containers start. | |
| 30 | 38 | if j.cs != nil { | |
| 31 | - | queue = append(queue, j.initContainersStep()) | |
| 39 | + | ic := j.initContainersStep() | |
| 40 | + | ic.ctxID = newRunnerStepID() | |
| 41 | + | queue = append(queue, ic) | |
| 32 | 42 | } | |
| 33 | 43 | for _, n := range nodes { | |
| 34 | 44 | if j.actions.pre[n] { | |
| @@ -25,10 +25,20 @@ func (j *job) prepareJob() ([]qstep, error) { | |||||
| 25 | return nil, j.setupError(err) | 25 | return nil, j.setupError(err) | ||
| 26 | } | 26 | } | ||
| 27 | queue := j.setupSteps() | 27 | queue := j.setupSteps() | ||
| 28 | + | // JobExtension.cs:434-440: every runner step of the job gets a random "N" | |||
| 29 | + | // context name before anything runs; its final result, skipped included, | |||
| 30 | + | // is in the steps context. The ones the composite actions run get a "__" | |||
| 31 | + | // name (CompositeActionHandler.cs:121-123) and are not registered, which is | |||
| 32 | + | // why only this queue has ids. | |||
| 33 | + | for i := range queue { | |||
| 34 | + | queue[i].ctxID = newRunnerStepID() | |||
| 35 | + | } | |||
| 28 | // JobExtension.cs:317-331: the images of the actions are pulled first, then | 36 | // JobExtension.cs:317-331: the images of the actions are pulled first, then | ||
| 29 | // the containers start. | 37 | // the containers start. | ||
| 30 | if j.cs != nil { | 38 | if j.cs != nil { | ||
| 31 | - | queue = append(queue, j.initContainersStep()) | 39 | + | ic := j.initContainersStep() |
| 40 | + | ic.ctxID = newRunnerStepID() | |||
| 41 | + | queue = append(queue, ic) | |||
| 32 | } | 42 | } | ||
| 33 | for _, n := range nodes { | 43 | for _, n := range nodes { | ||
| 34 | if j.actions.pre[n] { | 44 | if j.actions.pre[n] { | ||
M internal/executor/actions_test.go +53 -3
| @@ -1113,7 +1113,9 @@ runs: | |||
| 1113 | 1113 | // An inner step whose env does not evaluate still gets its markers: the error | |
| 1114 | 1114 | // is logged while the environment is set up, the start marker follows, then the | |
| 1115 | 1115 | // end marker with the step failed (CompositeActionHandler.cs:296-302, :308-324). | |
| 1116 | - | // The runner would go on and run the step; this port does not (see runEmbedded). | |
| 1116 | + | // The runner goes on: the condition holds (success() reads action_status) and the | |
| 1117 | + | // step runs without the env that failed; the end marker says failure with the | |
| 1118 | + | // elapsed time (:372-390, :431-440, SuccessFunction.cs:28). | |
| 1117 | 1119 | func TestCompositeEnvErrorKeepsMarkers(t *testing.T) { | |
| 1118 | 1120 | tree := files{ | |
| 1119 | 1121 | ".github/actions/e/action.yml": `name: e | |
| @@ -1138,10 +1140,12 @@ runs: | |||
| 1138 | 1140 | got = append(got, "error") | |
| 1139 | 1141 | case strings.HasPrefix(l, "##[start-action"), strings.HasPrefix(l, "##[end-action"): | |
| 1140 | 1142 | got = append(got, l) | |
| 1143 | + | case l == "never": | |
| 1144 | + | got = append(got, l) | |
| 1141 | 1145 | } | |
| 1142 | 1146 | } | |
| 1143 | - | want := []string{"error", "##[start-action display=bad env;id=__self.__run]", "##[end-action id=__self.__run;outcome=failure;conclusion=failure;duration_ms=0]"} | |
| 1144 | - | if strings.Join(got, "\n") != strings.Join(want, "\n") || strings.Contains(logOf(rec.steps[1]), "never") { | |
| 1147 | + | end := regexp.MustCompile(`^##\[end-action id=__self\.__run;outcome=failure;conclusion=failure;duration_ms=\d+\]$`) | |
| 1148 | + | if len(got) != 4 || got[0] != "error" || got[1] != "##[start-action display=bad env;id=__self.__run]" || got[2] != "never" || !end.MatchString(got[3]) { | |
| 1145 | 1149 | t.Errorf("markers:\n%s\n%s", strings.Join(got, "\n"), logOf(rec.steps[1])) | |
| 1146 | 1150 | } | |
| 1147 | 1151 | } | |
| @@ -1191,3 +1195,49 @@ func TestPathCombine(t *testing.T) { | |||
| 1191 | 1195 | } | |
| 1192 | 1196 | } | |
| 1193 | 1197 | } | |
| 1198 | + | ||
| 1199 | + | // A step of a composite action whose env cannot be evaluated is Failed in the | |
| 1200 | + | // steps context before its condition is evaluated (CompositeActionHandler.cs: | |
| 1201 | + | // 296-302, SetStepConclusion :529-533): its own `if` can read the failure and | |
| 1202 | + | // the step runs. The failed env leaves no partial value: the env is applied | |
| 1203 | + | // only after the whole mapping evaluated (:290-297). | |
| 1204 | + | func TestCompositeEnvErrorIsInTheStepsContextBeforeTheCondition(t *testing.T) { | |
| 1205 | + | tree := files{ | |
| 1206 | + | ".github/actions/e/action.yml": `name: e | |
| 1207 | + | inputs: | |
| 1208 | + | tok: | |
| 1209 | + | default: '' | |
| 1210 | + | runs: | |
| 1211 | + | using: composite | |
| 1212 | + | steps: | |
| 1213 | + | - id: bad | |
| 1214 | + | if: always() && steps.bad.outcome == 'failure' | |
| 1215 | + | shell: bash | |
| 1216 | + | env: | |
| 1217 | + | GOOD: kept-value | |
| 1218 | + | SECRET_LIKE: ${{ inputs.tok }} | |
| 1219 | + | X: ${{ fromJSON('{') }} | |
| 1220 | + | run: echo "ran good=[$GOOD] secret=[$SECRET_LIKE] x=[$X]" | |
| 1221 | + | `, | |
| 1222 | + | } | |
| 1223 | + | const tok = "ghs_partialenvtoken" | |
| 1224 | + | rec, res := actionRun(t, tree, " - uses: ./.github/actions/e\n with:\n tok: ${{ secrets.GITHUB_TOKEN }}\n", func(c *Config) { | |
| 1225 | + | c.ContextsJSON = strings.Replace(testGitHubContext("j"), `"github":{`, `"github":{"token":"`+tok+`",`, 1) | |
| 1226 | + | }) | |
| 1227 | + | if res.Conclusion != ConclusionFailure { | |
| 1228 | + | t.Fatalf("%+v", res) | |
| 1229 | + | } | |
| 1230 | + | log := logOf(rec.steps[1]) | |
| 1231 | + | if strings.Contains(log, tok) { | |
| 1232 | + | t.Errorf("the secret leaked:\n%s", log) | |
| 1233 | + | } | |
| 1234 | + | if !strings.Contains(log, "ran good=[] secret=[] x=[]") { | |
| 1235 | + | t.Errorf("the step must run without any value of the env that failed:\n%s", log) | |
| 1236 | + | } | |
| 1237 | + | if strings.Contains(log, "kept-value") && !strings.Contains(log, "good=[]") { | |
| 1238 | + | t.Errorf("partial env:\n%s", log) | |
| 1239 | + | } | |
| 1240 | + | if !strings.Contains(log, "outcome=failure;conclusion=failure") { | |
| 1241 | + | t.Errorf("end marker:\n%s", log) | |
| 1242 | + | } | |
| 1243 | + | } | |
| @@ -1113,7 +1113,9 @@ runs: | |||||
| 1113 | // An inner step whose env does not evaluate still gets its markers: the error | 1113 | // An inner step whose env does not evaluate still gets its markers: the error | ||
| 1114 | // is logged while the environment is set up, the start marker follows, then the | 1114 | // is logged while the environment is set up, the start marker follows, then the | ||
| 1115 | // end marker with the step failed (CompositeActionHandler.cs:296-302, :308-324). | 1115 | // end marker with the step failed (CompositeActionHandler.cs:296-302, :308-324). | ||
| 1116 | - | // The runner would go on and run the step; this port does not (see runEmbedded). | 1116 | + | // The runner goes on: the condition holds (success() reads action_status) and the |
| 1117 | + | // step runs without the env that failed; the end marker says failure with the | |||
| 1118 | + | // elapsed time (:372-390, :431-440, SuccessFunction.cs:28). | |||
| 1117 | func TestCompositeEnvErrorKeepsMarkers(t *testing.T) { | 1119 | func TestCompositeEnvErrorKeepsMarkers(t *testing.T) { | ||
| 1118 | tree := files{ | 1120 | tree := files{ | ||
| 1119 | ".github/actions/e/action.yml": `name: e | 1121 | ".github/actions/e/action.yml": `name: e | ||
| @@ -1138,10 +1140,12 @@ runs: | |||||
| 1138 | got = append(got, "error") | 1140 | got = append(got, "error") | ||
| 1139 | case strings.HasPrefix(l, "##[start-action"), strings.HasPrefix(l, "##[end-action"): | 1141 | case strings.HasPrefix(l, "##[start-action"), strings.HasPrefix(l, "##[end-action"): | ||
| 1140 | got = append(got, l) | 1142 | got = append(got, l) | ||
| 1143 | + | case l == "never": | |||
| 1144 | + | got = append(got, l) | |||
| 1141 | } | 1145 | } | ||
| 1142 | } | 1146 | } | ||
| 1143 | - | want := []string{"error", "##[start-action display=bad env;id=__self.__run]", "##[end-action id=__self.__run;outcome=failure;conclusion=failure;duration_ms=0]"} | 1147 | + | end := regexp.MustCompile(`^##\[end-action id=__self\.__run;outcome=failure;conclusion=failure;duration_ms=\d+\]$`) |
| 1144 | - | if strings.Join(got, "\n") != strings.Join(want, "\n") || strings.Contains(logOf(rec.steps[1]), "never") { | 1148 | + | if len(got) != 4 || got[0] != "error" || got[1] != "##[start-action display=bad env;id=__self.__run]" || got[2] != "never" || !end.MatchString(got[3]) { |
| 1145 | t.Errorf("markers:\n%s\n%s", strings.Join(got, "\n"), logOf(rec.steps[1])) | 1149 | t.Errorf("markers:\n%s\n%s", strings.Join(got, "\n"), logOf(rec.steps[1])) | ||
| 1146 | } | 1150 | } | ||
| 1147 | } | 1151 | } | ||
| @@ -1191,3 +1195,49 @@ func TestPathCombine(t *testing.T) { | |||||
| 1191 | } | 1195 | } | ||
| 1192 | } | 1196 | } | ||
| 1193 | } | 1197 | } | ||
| 1198 | + | ||||
| 1199 | + | // A step of a composite action whose env cannot be evaluated is Failed in the | |||
| 1200 | + | // steps context before its condition is evaluated (CompositeActionHandler.cs: | |||
| 1201 | + | // 296-302, SetStepConclusion :529-533): its own `if` can read the failure and | |||
| 1202 | + | // the step runs. The failed env leaves no partial value: the env is applied | |||
| 1203 | + | // only after the whole mapping evaluated (:290-297). | |||
| 1204 | + | func TestCompositeEnvErrorIsInTheStepsContextBeforeTheCondition(t *testing.T) { | |||
| 1205 | + | tree := files{ | |||
| 1206 | + | ".github/actions/e/action.yml": `name: e | |||
| 1207 | + | inputs: | |||
| 1208 | + | tok: | |||
| 1209 | + | default: '' | |||
| 1210 | + | runs: | |||
| 1211 | + | using: composite | |||
| 1212 | + | steps: | |||
| 1213 | + | - id: bad | |||
| 1214 | + | if: always() && steps.bad.outcome == 'failure' | |||
| 1215 | + | shell: bash | |||
| 1216 | + | env: | |||
| 1217 | + | GOOD: kept-value | |||
| 1218 | + | SECRET_LIKE: ${{ inputs.tok }} | |||
| 1219 | + | X: ${{ fromJSON('{') }} | |||
| 1220 | + | run: echo "ran good=[$GOOD] secret=[$SECRET_LIKE] x=[$X]" | |||
| 1221 | + | `, | |||
| 1222 | + | } | |||
| 1223 | + | const tok = "ghs_partialenvtoken" | |||
| 1224 | + | rec, res := actionRun(t, tree, " - uses: ./.github/actions/e\n with:\n tok: ${{ secrets.GITHUB_TOKEN }}\n", func(c *Config) { | |||
| 1225 | + | c.ContextsJSON = strings.Replace(testGitHubContext("j"), `"github":{`, `"github":{"token":"`+tok+`",`, 1) | |||
| 1226 | + | }) | |||
| 1227 | + | if res.Conclusion != ConclusionFailure { | |||
| 1228 | + | t.Fatalf("%+v", res) | |||
| 1229 | + | } | |||
| 1230 | + | log := logOf(rec.steps[1]) | |||
| 1231 | + | if strings.Contains(log, tok) { | |||
| 1232 | + | t.Errorf("the secret leaked:\n%s", log) | |||
| 1233 | + | } | |||
| 1234 | + | if !strings.Contains(log, "ran good=[] secret=[] x=[]") { | |||
| 1235 | + | t.Errorf("the step must run without any value of the env that failed:\n%s", log) | |||
| 1236 | + | } | |||
| 1237 | + | if strings.Contains(log, "kept-value") && !strings.Contains(log, "good=[]") { | |||
| 1238 | + | t.Errorf("partial env:\n%s", log) | |||
| 1239 | + | } | |||
| 1240 | + | if !strings.Contains(log, "outcome=failure;conclusion=failure") { | |||
| 1241 | + | t.Errorf("end marker:\n%s", log) | |||
| 1242 | + | } | |||
| 1243 | + | } | |||
M internal/executor/commands_parity_test.go +26 -1
| @@ -94,7 +94,8 @@ func TestFileCommandsOfAStepSurviveTheNextStep(t *testing.T) { | |||
| 94 | 94 | func TestRunnerStepsAreInTheStepsContext(t *testing.T) { | |
| 95 | 95 | st := newTestState(t) | |
| 96 | 96 | j := &job{state: st} | |
| 97 | - | j.recordRunnerStep(resSucceeded) | |
| 97 | + | s := &stepRun{j: j, ctxID: newRunnerStepID()} | |
| 98 | + | j.finishStep(s, resSucceeded) | |
| 98 | 99 | v, ok := st.Values(nil)["steps"].(*expr.Dictionary) | |
| 99 | 100 | if !ok || len(v.Pairs()) != 1 { | |
| 100 | 101 | t.Fatalf("%v", v) | |
| @@ -183,3 +184,27 @@ func TestInvalidStopTokenDrainsHighVolumeOutput(t *testing.T) { | |||
| 183 | 184 | t.Errorf("%.300s", got) | |
| 184 | 185 | } | |
| 185 | 186 | } | |
| 187 | + | ||
| 188 | + | // svc-redis (unhealthy): the github context of a step that runs before any | |
| 189 | + | // other step ran has no action_ref, action_repository, event_path nor the file | |
| 190 | + | // command paths: the runner sets them while it runs a step (ActionRunner.cs: | |
| 191 | + | // 147-153, FileCommandManager.cs:71, ExecutionContext.cs:1334), after the env | |
| 192 | + | // of the step was evaluated; the context object is shared by the steps, so a | |
| 193 | + | // later step has them. The recorded dump step of the unhealthy job of svc-redis | |
| 194 | + | // is the first step that ran and has none; a step run before it (the harness's | |
| 195 | + | // ermite-mask) gives it all of them. | |
| 196 | + | func TestGithubContextOfTheFirstStepThatRunsHasNoStepKeys(t *testing.T) { | |
| 197 | + | needTools(t, "bash") | |
| 198 | + | rec, _ := runLocal(t, wfHead+` steps: | |
| 199 | + | - name: skipped | |
| 200 | + | if: false | |
| 201 | + | run: echo no | |
| 202 | + | - name: dump | |
| 203 | + | run: echo "KEYS=$KEYS" | |
| 204 | + | env: | |
| 205 | + | KEYS: ${{ join(github.*, ',') != '' && 'x' }}${{ contains(toJSON(github), 'action_ref') }},${{ contains(toJSON(github), 'event_path') }},${{ contains(toJSON(github), 'artifacts') }},${{ contains(toJSON(github), 'step_summary') }} | |
| 206 | + | `, "j", nil) | |
| 207 | + | if got := logOf(rec.step(t, "dump")); !strings.Contains(got, "KEYS=xfalse,false,false,false") { | |
| 208 | + | t.Error(got) | |
| 209 | + | } | |
| 210 | + | } | |
| @@ -94,7 +94,8 @@ func TestFileCommandsOfAStepSurviveTheNextStep(t *testing.T) { | |||||
| 94 | func TestRunnerStepsAreInTheStepsContext(t *testing.T) { | 94 | func TestRunnerStepsAreInTheStepsContext(t *testing.T) { | ||
| 95 | st := newTestState(t) | 95 | st := newTestState(t) | ||
| 96 | j := &job{state: st} | 96 | j := &job{state: st} | ||
| 97 | - | j.recordRunnerStep(resSucceeded) | 97 | + | s := &stepRun{j: j, ctxID: newRunnerStepID()} |
| 98 | + | j.finishStep(s, resSucceeded) | |||
| 98 | v, ok := st.Values(nil)["steps"].(*expr.Dictionary) | 99 | v, ok := st.Values(nil)["steps"].(*expr.Dictionary) | ||
| 99 | if !ok || len(v.Pairs()) != 1 { | 100 | if !ok || len(v.Pairs()) != 1 { | ||
| 100 | t.Fatalf("%v", v) | 101 | t.Fatalf("%v", v) | ||
| @@ -183,3 +184,27 @@ func TestInvalidStopTokenDrainsHighVolumeOutput(t *testing.T) { | |||||
| 183 | t.Errorf("%.300s", got) | 184 | t.Errorf("%.300s", got) | ||
| 184 | } | 185 | } | ||
| 185 | } | 186 | } | ||
| 187 | + | ||||
| 188 | + | // svc-redis (unhealthy): the github context of a step that runs before any | |||
| 189 | + | // other step ran has no action_ref, action_repository, event_path nor the file | |||
| 190 | + | // command paths: the runner sets them while it runs a step (ActionRunner.cs: | |||
| 191 | + | // 147-153, FileCommandManager.cs:71, ExecutionContext.cs:1334), after the env | |||
| 192 | + | // of the step was evaluated; the context object is shared by the steps, so a | |||
| 193 | + | // later step has them. The recorded dump step of the unhealthy job of svc-redis | |||
| 194 | + | // is the first step that ran and has none; a step run before it (the harness's | |||
| 195 | + | // ermite-mask) gives it all of them. | |||
| 196 | + | func TestGithubContextOfTheFirstStepThatRunsHasNoStepKeys(t *testing.T) { | |||
| 197 | + | needTools(t, "bash") | |||
| 198 | + | rec, _ := runLocal(t, wfHead+` steps: | |||
| 199 | + | - name: skipped | |||
| 200 | + | if: false | |||
| 201 | + | run: echo no | |||
| 202 | + | - name: dump | |||
| 203 | + | run: echo "KEYS=$KEYS" | |||
| 204 | + | env: | |||
| 205 | + | KEYS: ${{ join(github.*, ',') != '' && 'x' }}${{ contains(toJSON(github), 'action_ref') }},${{ contains(toJSON(github), 'event_path') }},${{ contains(toJSON(github), 'artifacts') }},${{ contains(toJSON(github), 'step_summary') }} | |||
| 206 | + | `, "j", nil) | |||
| 207 | + | if got := logOf(rec.step(t, "dump")); !strings.Contains(got, "KEYS=xfalse,false,false,false") { | |||
| 208 | + | t.Error(got) | |||
| 209 | + | } | |||
| 210 | + | } | |||
M internal/executor/containers_init.go +1 -1
| @@ -76,7 +76,7 @@ var ( | |||
| 76 | 76 | // (JobExtension.cs:328-331), run if success(). | |
| 77 | 77 | func (j *job) initContainersStep() qstep { | |
| 78 | 78 | return qstep{name: "Initialize containers", setup: func(s *stepRun, cancel <-chan struct{}) stepResult { | |
| 79 | - | return j.recordRunnerStep(j.initContainers(s, cancel)) | |
| 79 | + | return j.initContainers(s, cancel) | |
| 80 | 80 | }} | |
| 81 | 81 | } | |
| 82 | 82 | ||
| @@ -76,7 +76,7 @@ var ( | |||||
| 76 | // (JobExtension.cs:328-331), run if success(). | 76 | // (JobExtension.cs:328-331), run if success(). | ||
| 77 | func (j *job) initContainersStep() qstep { | 77 | func (j *job) initContainersStep() qstep { | ||
| 78 | return qstep{name: "Initialize containers", setup: func(s *stepRun, cancel <-chan struct{}) stepResult { | 78 | return qstep{name: "Initialize containers", setup: func(s *stepRun, cancel <-chan struct{}) stepResult { | ||
| 79 | - | return j.recordRunnerStep(j.initContainers(s, cancel)) | 79 | + | return j.initContainers(s, cancel) |
| 80 | }} | 80 | }} | ||
| 81 | } | 81 | } | ||
| 82 | 82 | ||||
M internal/executor/contexts_containers.go +8 -14
| @@ -68,17 +68,11 @@ func replaceKey(d *expr.Dictionary, key string, v expr.Value) *expr.Dictionary { | |||
| 68 | 68 | return setKey(d, key, v) | |
| 69 | 69 | } | |
| 70 | 70 | ||
| 71 | - | // recordRunnerStep registers a step the runner made itself (Initialize | |
| 72 | - | // containers, Pull <image>, Build <action>) in the steps context. Its context | |
| 73 | - | // name is its Guid in "N" format, which does not start with "__": the entry | |
| 74 | - | // {outcome, conclusion, outputs: {}} is visible to the user's steps | |
| 75 | - | // (JobExtension.cs:439 CreateChild(stepId, ..., stepId.ToString("N"), ...), | |
| 76 | - | // ExecutionContext.UpdateGlobalStepsContext:644; recorded in ctr-basic, | |
| 77 | - | // svc-redis, act-docker-image). It returns res. | |
| 78 | - | func (j *job) recordRunnerStep(res stepResult) stepResult { | |
| 79 | - | id := strings.ReplaceAll(uuid.NewString(), "-", "") | |
| 80 | - | j.mu.Lock() | |
| 81 | - | j.state.SetStepResult(id, string(res.conclusion()), string(res.conclusion())) | |
| 82 | - | j.mu.Unlock() | |
| 83 | - | return res | |
| 84 | - | } | |
| 71 | + | // newRunnerStepID is the context name of a step the runner made itself | |
| 72 | + | // (Initialize containers, Pull <image>, Build <action>): its Guid in "N" format, | |
| 73 | + | // which does not start with "__", so the entry {outcome, conclusion, outputs: | |
| 74 | + | // {}} is visible to the user's steps (JobExtension.cs:439 CreateChild(stepId, | |
| 75 | + | // ..., stepId.ToString("N"), ...), ExecutionContext.UpdateGlobalStepsContext:644; | |
| 76 | + | // recorded in ctr-basic, svc-redis, act-docker-image). runStep registers its | |
| 77 | + | // result under it (qstep.ctxID). | |
| 78 | + | func newRunnerStepID() string { return strings.ReplaceAll(uuid.NewString(), "-", "") } | |
| @@ -68,17 +68,11 @@ func replaceKey(d *expr.Dictionary, key string, v expr.Value) *expr.Dictionary { | |||||
| 68 | return setKey(d, key, v) | 68 | return setKey(d, key, v) | ||
| 69 | } | 69 | } | ||
| 70 | 70 | ||||
| 71 | - | // recordRunnerStep registers a step the runner made itself (Initialize | 71 | + | // newRunnerStepID is the context name of a step the runner made itself |
| 72 | - | // containers, Pull <image>, Build <action>) in the steps context. Its context | 72 | + | // (Initialize containers, Pull <image>, Build <action>): its Guid in "N" format, |
| 73 | - | // name is its Guid in "N" format, which does not start with "__": the entry | 73 | + | // which does not start with "__", so the entry {outcome, conclusion, outputs: |
| 74 | - | // {outcome, conclusion, outputs: {}} is visible to the user's steps | 74 | + | // {}} is visible to the user's steps (JobExtension.cs:439 CreateChild(stepId, |
| 75 | - | // (JobExtension.cs:439 CreateChild(stepId, ..., stepId.ToString("N"), ...), | 75 | + | // ..., stepId.ToString("N"), ...), ExecutionContext.UpdateGlobalStepsContext:644; |
| 76 | - | // ExecutionContext.UpdateGlobalStepsContext:644; recorded in ctr-basic, | 76 | + | // recorded in ctr-basic, svc-redis, act-docker-image). runStep registers its |
| 77 | - | // svc-redis, act-docker-image). It returns res. | 77 | + | // result under it (qstep.ctxID). |
| 78 | - | func (j *job) recordRunnerStep(res stepResult) stepResult { | 78 | + | func newRunnerStepID() string { return strings.ReplaceAll(uuid.NewString(), "-", "") } |
| 79 | - | id := strings.ReplaceAll(uuid.NewString(), "-", "") | |||
| 80 | - | j.mu.Lock() | |||
| 81 | - | j.state.SetStepResult(id, string(res.conclusion()), string(res.conclusion())) | |||
| 82 | - | j.mu.Unlock() | |||
| 83 | - | return res | |||
| 84 | - | } | |||
M internal/executor/issues.go +4 -0
| @@ -192,6 +192,10 @@ func parseInt(s string) (int, bool) { | |||
| 192 | 192 | // left out. The title and the file are masked too (the runner masks only the | |
| 193 | 193 | // message; on GitHub a secret in a title stays visible, recorded in | |
| 194 | 194 | // mask-add-mask, which ermite deliberately does not reproduce). | |
| 195 | + | // | |
| 196 | + | // Deliberate divergence: GitHub keeps negative and zero lines (int.TryParse, | |
| 197 | + | // ActionCommandManager.cs:723-725; ann-levels records start_line -1); ermite's | |
| 198 | + | // contract is positive lines, 0 = no line, and the host clamps. | |
| 195 | 199 | func (j *job) annotationOf(is issue, msg string) Annotation { | |
| 196 | 200 | a := Annotation{Level: is.Level, Message: msg} | |
| 197 | 201 | p := is.Props | |
| @@ -192,6 +192,10 @@ func parseInt(s string) (int, bool) { | |||||
| 192 | // left out. The title and the file are masked too (the runner masks only the | 192 | // left out. The title and the file are masked too (the runner masks only the | ||
| 193 | // message; on GitHub a secret in a title stays visible, recorded in | 193 | // message; on GitHub a secret in a title stays visible, recorded in | ||
| 194 | // mask-add-mask, which ermite deliberately does not reproduce). | 194 | // mask-add-mask, which ermite deliberately does not reproduce). | ||
| 195 | + | // | |||
| 196 | + | // Deliberate divergence: GitHub keeps negative and zero lines (int.TryParse, | |||
| 197 | + | // ActionCommandManager.cs:723-725; ann-levels records start_line -1); ermite's | |||
| 198 | + | // contract is positive lines, 0 = no line, and the host clamps. | |||
| 195 | func (j *job) annotationOf(is issue, msg string) Annotation { | 199 | func (j *job) annotationOf(is issue, msg string) Annotation { | ||
| 196 | a := Annotation{Level: is.Level, Message: msg} | 200 | a := Annotation{Level: is.Level, Message: msg} | ||
| 197 | p := is.Props | 201 | p := is.Props | ||
M internal/executor/step.go +6 -1
| @@ -30,7 +30,12 @@ type stepRun struct { | |||
| 30 | 30 | overrides *ordered // the step env of this step and of its composite parents | |
| 31 | 31 | cmdResult *stepResult | |
| 32 | 32 | outcome *stepResult | |
| 33 | - | cmds *commandManager | |
| 33 | + | // envFailed: the env of an embedded step failed to evaluate, which marked it | |
| 34 | + | // failed before it ran (CompositeActionHandler.cs:296-302). | |
| 35 | + | envFailed bool | |
| 36 | + | // ctxID: see qstep.ctxID. | |
| 37 | + | ctxID string | |
| 38 | + | cmds *commandManager | |
| 34 | 39 | // result is ExecutionContext.Result of a step that runs embedded steps: it | |
| 35 | 40 | // gives action_status to them. | |
| 36 | 41 | result *stepResult | |
| @@ -30,7 +30,12 @@ type stepRun struct { | |||||
| 30 | overrides *ordered // the step env of this step and of its composite parents | 30 | overrides *ordered // the step env of this step and of its composite parents | ||
| 31 | cmdResult *stepResult | 31 | cmdResult *stepResult | ||
| 32 | outcome *stepResult | 32 | outcome *stepResult | ||
| 33 | - | cmds *commandManager | 33 | + | // envFailed: the env of an embedded step failed to evaluate, which marked it |
| 34 | + | // failed before it ran (CompositeActionHandler.cs:296-302). | |||
| 35 | + | envFailed bool | |||
| 36 | + | // ctxID: see qstep.ctxID. | |||
| 37 | + | ctxID string | |||
| 38 | + | cmds *commandManager | |||
| 34 | // result is ExecutionContext.Result of a step that runs embedded steps: it | 39 | // result is ExecutionContext.Result of a step that runs embedded steps: it | ||
| 35 | // gives action_status to them. | 40 | // gives action_status to them. | ||
| 36 | result *stepResult | 41 | result *stepResult | ||
M internal/executor/step_run.go +15 -0
| @@ -24,6 +24,12 @@ type qstep struct { | |||
| 24 | 24 | cond string | |
| 25 | 25 | // setup, when set, is the whole step: it runs instead of the node. | |
| 26 | 26 | setup func(s *stepRun, cancel <-chan struct{}) stepResult | |
| 27 | + | // ctxID is the context name of a runner step of the job (a Guid in "N" | |
| 28 | + | // format), assigned before the step runs: its final result, skipped | |
| 29 | + | // included, is registered in the steps context (JobExtension.cs:434-440, | |
| 30 | + | // StepsRunner.CompleteStep, ExecutionContext.UpdateGlobalStepsContext:644). | |
| 31 | + | // Empty for the setup steps a composite action runs (CompositeActionHandler.cs:121-123). | |
| 32 | + | ctxID string | |
| 27 | 33 | } | |
| 28 | 34 | ||
| 29 | 35 | func (j *job) statusFuncs() workflow.StatusFuncs { | |
| @@ -159,6 +165,7 @@ func (j *job) runStep(num int, q qstep) { | |||
| 159 | 165 | name := q.name | |
| 160 | 166 | if q.setup != nil { | |
| 161 | 167 | s.name = name | |
| 168 | + | s.ctxID = q.ctxID | |
| 162 | 169 | rep.StepStarted(num, name, j.now()) | |
| 163 | 170 | r := j.runGuarded(s, q, true) | |
| 164 | 171 | rep.StepCompleted(num, name, r.conclusion(), j.now()) | |
| @@ -208,6 +215,9 @@ func (j *job) finishStep(s *stepRun, res stepResult) { | |||
| 208 | 215 | if s.n != nil && (s.parent != nil || s.stage == stageMain) && st != nil { | |
| 209 | 216 | j.state.SetStepResult(st.ID, string(outcome.conclusion()), string(res.conclusion())) | |
| 210 | 217 | } | |
| 218 | + | if s.ctxID != "" { | |
| 219 | + | j.state.SetStepResult(s.ctxID, string(outcome.conclusion()), string(res.conclusion())) | |
| 220 | + | } | |
| 211 | 221 | if s.parent == nil { | |
| 212 | 222 | j.recordOutcome(s.num, outcome.conclusion()) | |
| 213 | 223 | } | |
| @@ -333,6 +343,11 @@ func (j *job) runGuarded(s *stepRun, q qstep, ok bool) stepResult { | |||
| 333 | 343 | r = resFailed | |
| 334 | 344 | } | |
| 335 | 345 | ||
| 346 | + | // The step was marked failed before it ran (CompositeActionHandler.cs:302): | |
| 347 | + | // nothing resets that when it goes well (RunStepAsync :481-527). | |
| 348 | + | if s.envFailed && r == resSucceeded { | |
| 349 | + | r = resFailed | |
| 350 | + | } | |
| 336 | 351 | // Merge the result of the commands (StepsRunner.cs:347-351). | |
| 337 | 352 | if s.cmdResult != nil { | |
| 338 | 353 | r = merge(&r, *s.cmdResult) | |
| @@ -24,6 +24,12 @@ type qstep struct { | |||||
| 24 | cond string | 24 | cond string | ||
| 25 | // setup, when set, is the whole step: it runs instead of the node. | 25 | // setup, when set, is the whole step: it runs instead of the node. | ||
| 26 | setup func(s *stepRun, cancel <-chan struct{}) stepResult | 26 | setup func(s *stepRun, cancel <-chan struct{}) stepResult | ||
| 27 | + | // ctxID is the context name of a runner step of the job (a Guid in "N" | |||
| 28 | + | // format), assigned before the step runs: its final result, skipped | |||
| 29 | + | // included, is registered in the steps context (JobExtension.cs:434-440, | |||
| 30 | + | // StepsRunner.CompleteStep, ExecutionContext.UpdateGlobalStepsContext:644). | |||
| 31 | + | // Empty for the setup steps a composite action runs (CompositeActionHandler.cs:121-123). | |||
| 32 | + | ctxID string | |||
| 27 | } | 33 | } | ||
| 28 | 34 | ||||
| 29 | func (j *job) statusFuncs() workflow.StatusFuncs { | 35 | func (j *job) statusFuncs() workflow.StatusFuncs { | ||
| @@ -159,6 +165,7 @@ func (j *job) runStep(num int, q qstep) { | |||||
| 159 | name := q.name | 165 | name := q.name | ||
| 160 | if q.setup != nil { | 166 | if q.setup != nil { | ||
| 161 | s.name = name | 167 | s.name = name | ||
| 168 | + | s.ctxID = q.ctxID | |||
| 162 | rep.StepStarted(num, name, j.now()) | 169 | rep.StepStarted(num, name, j.now()) | ||
| 163 | r := j.runGuarded(s, q, true) | 170 | r := j.runGuarded(s, q, true) | ||
| 164 | rep.StepCompleted(num, name, r.conclusion(), j.now()) | 171 | rep.StepCompleted(num, name, r.conclusion(), j.now()) | ||
| @@ -208,6 +215,9 @@ func (j *job) finishStep(s *stepRun, res stepResult) { | |||||
| 208 | if s.n != nil && (s.parent != nil || s.stage == stageMain) && st != nil { | 215 | if s.n != nil && (s.parent != nil || s.stage == stageMain) && st != nil { | ||
| 209 | j.state.SetStepResult(st.ID, string(outcome.conclusion()), string(res.conclusion())) | 216 | j.state.SetStepResult(st.ID, string(outcome.conclusion()), string(res.conclusion())) | ||
| 210 | } | 217 | } | ||
| 218 | + | if s.ctxID != "" { | |||
| 219 | + | j.state.SetStepResult(s.ctxID, string(outcome.conclusion()), string(res.conclusion())) | |||
| 220 | + | } | |||
| 211 | if s.parent == nil { | 221 | if s.parent == nil { | ||
| 212 | j.recordOutcome(s.num, outcome.conclusion()) | 222 | j.recordOutcome(s.num, outcome.conclusion()) | ||
| 213 | } | 223 | } | ||
| @@ -333,6 +343,11 @@ func (j *job) runGuarded(s *stepRun, q qstep, ok bool) stepResult { | |||||
| 333 | r = resFailed | 343 | r = resFailed | ||
| 334 | } | 344 | } | ||
| 335 | 345 | ||||
| 346 | + | // The step was marked failed before it ran (CompositeActionHandler.cs:302): | |||
| 347 | + | // nothing resets that when it goes well (RunStepAsync :481-527). | |||
| 348 | + | if s.envFailed && r == resSucceeded { | |||
| 349 | + | r = resFailed | |||
| 350 | + | } | |||
| 336 | // Merge the result of the commands (StepsRunner.cs:347-351). | 351 | // Merge the result of the commands (StepsRunner.cs:347-351). | ||
| 337 | if s.cmdResult != nil { | 352 | if s.cmdResult != nil { | ||
| 338 | r = merge(&r, *s.cmdResult) | 353 | r = merge(&r, *s.cmdResult) | ||