detached a674b39

fix(executor): parity for workflow commands, matchers and file commands (CI-2 compare)

commit
a674b3992fa887f0e03178f9c7241982ec2d191e
author
brice · · 2026-10-14 11:00 UTC
committer
bnema · · 2026-10-14 11:00 UTC
parent
9e57131
signature
SSH UnverifiedSHA256:VOCA5lYARHcbKDZ7GhTV45VmcjWnJ6ZQIBazj17LivA The key is unknown
11 files · +328 -23
M internal/executor/commands.go +20 -4
@@ -163,6 +163,18 @@ type commandManager struct {
163163 stopped bool
164164 stopToken string
165165 echo bool
166+ // fatal is the exception that ActionCommandManager.ValidateStopToken throws
167+ // outside the try block of TryProcessCommand: it leaves OutputDataReceived,
168+ // ProcessInvoker.ExecuteAsync and the step with it, so the step fails with
169+ // the message alone and the output that follows is never read. The
170+ // official runner leaves the process running (ProcessInvoker is only
171+ // disposed; "Complete job" kills it as an orphan); ermite stops it through
172+ // the cancellation path (SIGINT, SIGTERM, SIGKILL) so that the step does not
173+ // wait for it, and discards what it still writes.
174+ fatal error
175+ // onFatal, when set, is called once when fatal is set: the process that
176+ // writes the output is then stopped (runProc).
177+ onFatal func()
166178 }
167179
168180 func newCommandManager(h commandHost, echo bool) *commandManager {
@@ -178,7 +190,7 @@ func (m *commandManager) isRegistered(name string) bool { return m.registered[st
178190 // TryProcess handles one line of output. It returns true when the line was a
179191 // command and must not be logged as plain text (ActionCommandManager.cs:61).
180192 func (m *commandManager) TryProcess(input string) bool {
181- if input == "" {
193+ if input == "" || m.fatal != nil {
182194 return false
183195 }
184196 cmd, ok := parseCommandV2(input, m.isRegistered)
@@ -202,7 +214,10 @@ func (m *commandManager) TryProcess(input string) bool {
202214 }
203215 if name == "stop-commands" {
204216 if err := m.validateStopToken(cmd.Data); err != nil {
205- m.fail(input, false, err)
217+ m.fatal = err
218+ if m.onFatal != nil {
219+ m.onFatal()
220+ }
206221 return true
207222 }
208223 m.stopToken, m.stopped = cmd.Data, true
@@ -259,7 +274,8 @@ func (m *commandManager) run(name string, c *actionCommand) error {
259274 m.commandIssue(LevelNotice, name, c)
260275 case "echo":
261276 if c.Data == "" {
262- return ghErrorf("Value cannot be null or empty. (Parameter 'value')")
277+ // ArgUtil.NotNullOrEmpty throws ArgumentNullException.
278+ return ghErrorf("Value cannot be null. (Parameter 'value')")
263279 }
264280 switch strings.ToUpper(strings.TrimSpace(c.Data)) {
265281 case "ON":
@@ -306,7 +322,7 @@ func (m *commandManager) run(name string, c *actionCommand) error {
306322 return ghErrorf(unsupportedCommandMessageDisabled, name)
307323 }
308324 if c.Data == "" {
309- return ghErrorf("Value cannot be null or empty. (Parameter 'path')")
325+ return ghErrorf("Value cannot be null. (Parameter 'path')")
310326 }
311327 h.AddPath(c.Data)
312328 case "add-mask":
@@ -163,6 +163,18 @@ type commandManager struct {
163 stopped bool163 stopped bool
164 stopToken string164 stopToken string
165 echo bool165 echo bool
166+ // fatal is the exception that ActionCommandManager.ValidateStopToken throws
167+ // outside the try block of TryProcessCommand: it leaves OutputDataReceived,
168+ // ProcessInvoker.ExecuteAsync and the step with it, so the step fails with
169+ // the message alone and the output that follows is never read. The
170+ // official runner leaves the process running (ProcessInvoker is only
171+ // disposed; "Complete job" kills it as an orphan); ermite stops it through
172+ // the cancellation path (SIGINT, SIGTERM, SIGKILL) so that the step does not
173+ // wait for it, and discards what it still writes.
174+ fatal error
175+ // onFatal, when set, is called once when fatal is set: the process that
176+ // writes the output is then stopped (runProc).
177+ onFatal func()
166 }178 }
167 179
168 func newCommandManager(h commandHost, echo bool) *commandManager {180 func newCommandManager(h commandHost, echo bool) *commandManager {
@@ -178,7 +190,7 @@ func (m *commandManager) isRegistered(name string) bool { return m.registered[st
178 // TryProcess handles one line of output. It returns true when the line was a190 // TryProcess handles one line of output. It returns true when the line was a
179 // command and must not be logged as plain text (ActionCommandManager.cs:61).191 // command and must not be logged as plain text (ActionCommandManager.cs:61).
180 func (m *commandManager) TryProcess(input string) bool {192 func (m *commandManager) TryProcess(input string) bool {
181- if input == "" {193+ if input == "" || m.fatal != nil {
182 return false194 return false
183 }195 }
184 cmd, ok := parseCommandV2(input, m.isRegistered)196 cmd, ok := parseCommandV2(input, m.isRegistered)
@@ -202,7 +214,10 @@ func (m *commandManager) TryProcess(input string) bool {
202 }214 }
203 if name == "stop-commands" {215 if name == "stop-commands" {
204 if err := m.validateStopToken(cmd.Data); err != nil {216 if err := m.validateStopToken(cmd.Data); err != nil {
205- m.fail(input, false, err)217+ m.fatal = err
218+ if m.onFatal != nil {
219+ m.onFatal()
220+ }
206 return true221 return true
207 }222 }
208 m.stopToken, m.stopped = cmd.Data, true223 m.stopToken, m.stopped = cmd.Data, true
@@ -259,7 +274,8 @@ func (m *commandManager) run(name string, c *actionCommand) error {
259 m.commandIssue(LevelNotice, name, c)274 m.commandIssue(LevelNotice, name, c)
260 case "echo":275 case "echo":
261 if c.Data == "" {276 if c.Data == "" {
262- return ghErrorf("Value cannot be null or empty. (Parameter 'value')")277+ // ArgUtil.NotNullOrEmpty throws ArgumentNullException.
278+ return ghErrorf("Value cannot be null. (Parameter 'value')")
263 }279 }
264 switch strings.ToUpper(strings.TrimSpace(c.Data)) {280 switch strings.ToUpper(strings.TrimSpace(c.Data)) {
265 case "ON":281 case "ON":
@@ -306,7 +322,7 @@ func (m *commandManager) run(name string, c *actionCommand) error {
306 return ghErrorf(unsupportedCommandMessageDisabled, name)322 return ghErrorf(unsupportedCommandMessageDisabled, name)
307 }323 }
308 if c.Data == "" {324 if c.Data == "" {
309- return ghErrorf("Value cannot be null or empty. (Parameter 'path')")325+ return ghErrorf("Value cannot be null. (Parameter 'path')")
310 }326 }
311 h.AddPath(c.Data)327 h.AddPath(c.Data)
312 case "add-mask":328 case "add-mask":
A internal/executor/commands_parity_test.go +185 -0
@@ -0,0 +1,185 @@
1+// SPDX-License-Identifier: MIT
2+
3+package executor
4+
5+import (
6+ "regexp"
7+ "strings"
8+ "testing"
9+ "time"
10+
11+ "bnema.dev/ermite-actions/expr"
12+)
13+
14+// Texts recorded on github.com (ermite-conformance, scenarios match-basic).
15+func TestMatcherFileErrorsAreTheRunners(t *testing.T) {
16+ _, err := parseMatchersConfig([]byte("not json\n"))
17+ const want = "Unexpected character encountered while parsing value: n. Path '', line 0, position 0."
18+ if err == nil || err.Error() != want {
19+ t.Errorf("not JSON: %v", err)
20+ }
21+ cfg, err := parseMatchersConfig([]byte(`{"problemMatcher":[{"owner":"badregexp","pattern":[{"regexp":"^INVALID (.+$","message":1}]}]}`))
22+ if err != nil {
23+ t.Fatal(err)
24+ }
25+ err = cfg.validate()
26+ const wantRe = "Invalid pattern '^INVALID (.+$' at offset 13. Not enough )'s."
27+ if err == nil || err.Error() != wantRe {
28+ t.Errorf("regexp: %v", err)
29+ }
30+ // Json.NET accepts what RFC 8259 does not.
31+ if _, err := parseMatchersConfig([]byte(`{'problemMatcher':[],}`)); err != nil {
32+ t.Errorf("lenient JSON: %v", err)
33+ }
34+}
35+
36+// cmd-add-path-disabled: ArgUtil.NotNullOrEmpty throws ArgumentNullException,
37+// whose message is "Value cannot be null. (Parameter 'path')".
38+func TestAddPathWithoutPathMessage(t *testing.T) {
39+ needTools(t, "bash")
40+ rec, _ := runLocal(t, wfHead+" steps:\n - name: p\n env:\n ACTIONS_ALLOW_UNSECURE_COMMANDS: 'true'\n run: echo '::add-path::'\n", "j", nil)
41+ if got := logOf(rec.step(t, "p")); !strings.Contains(got, "##[error]Value cannot be null. (Parameter 'path')") || strings.Contains(got, "null or empty") {
42+ t.Error(got)
43+ }
44+}
45+
46+// cmd-stop-commands: the exception of ValidateStopToken is not inside the try
47+// block of TryProcessCommand, so the step fails with the message alone (no
48+// "Unable to process command") and the output after it is not read.
49+func TestInvalidStopTokenFailsTheStepWithTheMessageOnly(t *testing.T) {
50+ needTools(t, "bash")
51+ for _, tok := range []string{"pause-logging", "", "warning", "PAUSE-LOGGING"} {
52+ rec, res := runLocal(t, wfHead+" steps:\n - name: s\n run: |\n echo '::stop-commands::"+tok+"'\n echo '::warning::still processed'\n", "j", nil)
53+ got := logOf(rec.step(t, "s"))
54+ if strings.Contains(got, "Unable to process command") || strings.Contains(got, "still processed\n") && strings.Contains(got, "##[warning]") {
55+ t.Errorf("%q: %s", tok, got)
56+ }
57+ if !strings.Contains(got, "##[error]"+unsupportedStopTokenDisabled) || rec.step(t, "s").Conclusion != ConclusionFailure || res.Conclusion != ConclusionFailure {
58+ t.Errorf("%q: %s", tok, got)
59+ }
60+ }
61+}
62+
63+// ann-levels, ann-error-fails, match-basic: GitHub reports end_line = line (and
64+// end_column = col) when the command gave only the start.
65+func TestAnnotationEndsWhereItStartsByDefault(t *testing.T) {
66+ j := &job{}
67+ j.masker.Store(newMasker())
68+ a := j.annotationOf(issue{Level: LevelWarning, Props: map[string]string{"line": "2", "col": "3"}}, "m")
69+ if a.StartLine != 2 || a.EndLine != 2 || a.StartColumn != 3 || a.EndColumn != 3 {
70+ t.Errorf("%+v", a)
71+ }
72+ a = j.annotationOf(issue{Level: LevelWarning, Props: map[string]string{"line": "2", "endline": "5"}}, "m")
73+ if a.EndLine != 5 || a.EndColumn != 0 {
74+ t.Errorf("%+v", a)
75+ }
76+}
77+
78+// sum-env: the file of a step is still there in the next one.
79+func TestFileCommandsOfAStepSurviveTheNextStep(t *testing.T) {
80+ needTools(t, "bash")
81+ rec, _ := runLocal(t, wfHead+` steps:
82+ - run: echo "$GITHUB_STEP_SUMMARY" > "$RUNNER_TEMP/p"
83+ - name: next
84+ run: test -e "$(cat "$RUNNER_TEMP/p")" && echo yes || echo no
85+`, "j", nil)
86+ if got := logOf(rec.step(t, "next")); !strings.HasSuffix(got, "\nyes") {
87+ t.Error(got)
88+ }
89+}
90+
91+// ctr-*, svc-*, act-docker-image: a step made by the runner is in the steps
92+// context under a 32-hex id (a Guid in "N" format), with its outcome and
93+// conclusion and no outputs.
94+func TestRunnerStepsAreInTheStepsContext(t *testing.T) {
95+ st := newTestState(t)
96+ j := &job{state: st}
97+ j.recordRunnerStep(resSucceeded)
98+ v, ok := st.Values(nil)["steps"].(*expr.Dictionary)
99+ if !ok || len(v.Pairs()) != 1 {
100+ t.Fatalf("%v", v)
101+ }
102+ p := v.Pairs()[0]
103+ if !regexp.MustCompile(`^[0-9a-f]{32}$`).MatchString(p.Key) {
104+ t.Errorf("id %q", p.Key)
105+ }
106+ d, _ := p.Value.(*expr.Dictionary)
107+ if d == nil || strings.Join(d.Keys(), ",") != "outputs,outcome,conclusion" {
108+ t.Errorf("%v", p.Value)
109+ }
110+ if o, _ := d.Get("outcome"); o != expr.String("success") {
111+ t.Errorf("outcome %v", o)
112+ }
113+}
114+
115+// sum-too-large: a summary of exactly 1 MiB is accepted (the runner tests the
116+// size of the file), 1 MiB plus one byte is refused with "got 1024k", 2 MiB
117+// with "got 2048k".
118+func TestStepSummarySizeLimit(t *testing.T) {
119+ needTools(t, "bash")
120+ rr, _ := runResults(t, wfHead+` steps:
121+ - name: exact
122+ run: head -c $((1024 * 1024)) /dev/zero | tr '\0' b >> "$GITHUB_STEP_SUMMARY"
123+ - name: plus1
124+ run: head -c $((1024 * 1024 + 1)) /dev/zero | tr '\0' c >> "$GITHUB_STEP_SUMMARY"
125+ - name: two
126+ run: head -c $((2 * 1024 * 1024)) /dev/zero | tr '\0' d >> "$GITHUB_STEP_SUMMARY"
127+`, nil)
128+ const tail = ", supports content up to a size of 1024k, got "
129+ if got := logOf(rr.step(t, "exact")); strings.Contains(got, "upload aborted") {
130+ t.Errorf("exact: %.200s", got)
131+ }
132+ if r := rr.results[2]; !r.HasSummary || len(r.Summary) != stepSummaryLimit {
133+ t.Errorf("exact: has=%v len=%d", r.HasSummary, len(r.Summary))
134+ }
135+ if got := logOf(rr.step(t, "plus1")); !strings.Contains(got, "##[error]$GITHUB_STEP_SUMMARY upload aborted"+tail+"1024k.") {
136+ t.Errorf("plus1: %.300s", got)
137+ }
138+ if got := logOf(rr.step(t, "two")); !strings.Contains(got, "##[error]$GITHUB_STEP_SUMMARY upload aborted"+tail+"2048k.") {
139+ t.Errorf("two: %.300s", got)
140+ }
141+}
142+
143+// An invalid stop-commands token ends the process: the step does not wait for
144+// the script (here 600 s) and fails with the endToken message.
145+func TestInvalidStopTokenStopsTheProcess(t *testing.T) {
146+ needTools(t, "bash", "sleep")
147+ start := time.Now()
148+ rec, res := runLocal(t, wfHead+" steps:\n - name: s\n run: |\n echo '::stop-commands::warning'\n sleep 600\n echo after\n", "j", nil)
149+ if d := time.Since(start); d > 30*time.Second {
150+ t.Fatalf("the step took %v", d)
151+ }
152+ got := logOf(rec.step(t, "s"))
153+ if !strings.Contains(got, "##[error]"+unsupportedStopTokenDisabled) || strings.Contains(got, "The operation was canceled.") {
154+ t.Error(got)
155+ }
156+ for _, l := range rec.step(t, "s").Log {
157+ if strings.TrimSpace(l) == "after" {
158+ t.Errorf("output after the fatal line was logged: %q", l)
159+ }
160+ }
161+ if rec.step(t, "s").Conclusion != ConclusionFailure || res.Conclusion != ConclusionFailure {
162+ t.Errorf("%v %v", rec.step(t, "s").Conclusion, res.Conclusion)
163+ }
164+}
165+
166+// Output written after the fatal line, a lot of it, does not block the process
167+// or the executor.
168+func TestInvalidStopTokenDrainsHighVolumeOutput(t *testing.T) {
169+ needTools(t, "bash", "yes", "head")
170+ done := make(chan struct{})
171+ var rec *recorder
172+ go func() {
173+ defer close(done)
174+ rec, _ = runLocal(t, wfHead+" steps:\n - name: s\n run: |\n echo '::stop-commands::'\n yes '::warning::x' | head -c 200000000\n sleep 600\n", "j", nil)
175+ }()
176+ select {
177+ case <-done:
178+ case <-time.After(60 * time.Second):
179+ t.Fatal("deadlock")
180+ }
181+ got := logOf(rec.step(t, "s"))
182+ if !strings.Contains(got, "##[error]"+unsupportedStopTokenDisabled) || strings.Contains("\n"+got+"\n", "\n::warning::x\n") || strings.Contains(got, "##[warning]x") {
183+ t.Errorf("%.300s", got)
184+ }
185+}
@@ -0,0 +1,185 @@
1+// SPDX-License-Identifier: MIT
2+
3+package executor
4+
5+import (
6+ "regexp"
7+ "strings"
8+ "testing"
9+ "time"
10+
11+ "bnema.dev/ermite-actions/expr"
12+)
13+
14+// Texts recorded on github.com (ermite-conformance, scenarios match-basic).
15+func TestMatcherFileErrorsAreTheRunners(t *testing.T) {
16+ _, err := parseMatchersConfig([]byte("not json\n"))
17+ const want = "Unexpected character encountered while parsing value: n. Path '', line 0, position 0."
18+ if err == nil || err.Error() != want {
19+ t.Errorf("not JSON: %v", err)
20+ }
21+ cfg, err := parseMatchersConfig([]byte(`{"problemMatcher":[{"owner":"badregexp","pattern":[{"regexp":"^INVALID (.+$","message":1}]}]}`))
22+ if err != nil {
23+ t.Fatal(err)
24+ }
25+ err = cfg.validate()
26+ const wantRe = "Invalid pattern '^INVALID (.+$' at offset 13. Not enough )'s."
27+ if err == nil || err.Error() != wantRe {
28+ t.Errorf("regexp: %v", err)
29+ }
30+ // Json.NET accepts what RFC 8259 does not.
31+ if _, err := parseMatchersConfig([]byte(`{'problemMatcher':[],}`)); err != nil {
32+ t.Errorf("lenient JSON: %v", err)
33+ }
34+}
35+
36+// cmd-add-path-disabled: ArgUtil.NotNullOrEmpty throws ArgumentNullException,
37+// whose message is "Value cannot be null. (Parameter 'path')".
38+func TestAddPathWithoutPathMessage(t *testing.T) {
39+ needTools(t, "bash")
40+ rec, _ := runLocal(t, wfHead+" steps:\n - name: p\n env:\n ACTIONS_ALLOW_UNSECURE_COMMANDS: 'true'\n run: echo '::add-path::'\n", "j", nil)
41+ if got := logOf(rec.step(t, "p")); !strings.Contains(got, "##[error]Value cannot be null. (Parameter 'path')") || strings.Contains(got, "null or empty") {
42+ t.Error(got)
43+ }
44+}
45+
46+// cmd-stop-commands: the exception of ValidateStopToken is not inside the try
47+// block of TryProcessCommand, so the step fails with the message alone (no
48+// "Unable to process command") and the output after it is not read.
49+func TestInvalidStopTokenFailsTheStepWithTheMessageOnly(t *testing.T) {
50+ needTools(t, "bash")
51+ for _, tok := range []string{"pause-logging", "", "warning", "PAUSE-LOGGING"} {
52+ rec, res := runLocal(t, wfHead+" steps:\n - name: s\n run: |\n echo '::stop-commands::"+tok+"'\n echo '::warning::still processed'\n", "j", nil)
53+ got := logOf(rec.step(t, "s"))
54+ if strings.Contains(got, "Unable to process command") || strings.Contains(got, "still processed\n") && strings.Contains(got, "##[warning]") {
55+ t.Errorf("%q: %s", tok, got)
56+ }
57+ if !strings.Contains(got, "##[error]"+unsupportedStopTokenDisabled) || rec.step(t, "s").Conclusion != ConclusionFailure || res.Conclusion != ConclusionFailure {
58+ t.Errorf("%q: %s", tok, got)
59+ }
60+ }
61+}
62+
63+// ann-levels, ann-error-fails, match-basic: GitHub reports end_line = line (and
64+// end_column = col) when the command gave only the start.
65+func TestAnnotationEndsWhereItStartsByDefault(t *testing.T) {
66+ j := &job{}
67+ j.masker.Store(newMasker())
68+ a := j.annotationOf(issue{Level: LevelWarning, Props: map[string]string{"line": "2", "col": "3"}}, "m")
69+ if a.StartLine != 2 || a.EndLine != 2 || a.StartColumn != 3 || a.EndColumn != 3 {
70+ t.Errorf("%+v", a)
71+ }
72+ a = j.annotationOf(issue{Level: LevelWarning, Props: map[string]string{"line": "2", "endline": "5"}}, "m")
73+ if a.EndLine != 5 || a.EndColumn != 0 {
74+ t.Errorf("%+v", a)
75+ }
76+}
77+
78+// sum-env: the file of a step is still there in the next one.
79+func TestFileCommandsOfAStepSurviveTheNextStep(t *testing.T) {
80+ needTools(t, "bash")
81+ rec, _ := runLocal(t, wfHead+` steps:
82+ - run: echo "$GITHUB_STEP_SUMMARY" > "$RUNNER_TEMP/p"
83+ - name: next
84+ run: test -e "$(cat "$RUNNER_TEMP/p")" && echo yes || echo no
85+`, "j", nil)
86+ if got := logOf(rec.step(t, "next")); !strings.HasSuffix(got, "\nyes") {
87+ t.Error(got)
88+ }
89+}
90+
91+// ctr-*, svc-*, act-docker-image: a step made by the runner is in the steps
92+// context under a 32-hex id (a Guid in "N" format), with its outcome and
93+// conclusion and no outputs.
94+func TestRunnerStepsAreInTheStepsContext(t *testing.T) {
95+ st := newTestState(t)
96+ j := &job{state: st}
97+ j.recordRunnerStep(resSucceeded)
98+ v, ok := st.Values(nil)["steps"].(*expr.Dictionary)
99+ if !ok || len(v.Pairs()) != 1 {
100+ t.Fatalf("%v", v)
101+ }
102+ p := v.Pairs()[0]
103+ if !regexp.MustCompile(`^[0-9a-f]{32}$`).MatchString(p.Key) {
104+ t.Errorf("id %q", p.Key)
105+ }
106+ d, _ := p.Value.(*expr.Dictionary)
107+ if d == nil || strings.Join(d.Keys(), ",") != "outputs,outcome,conclusion" {
108+ t.Errorf("%v", p.Value)
109+ }
110+ if o, _ := d.Get("outcome"); o != expr.String("success") {
111+ t.Errorf("outcome %v", o)
112+ }
113+}
114+
115+// sum-too-large: a summary of exactly 1 MiB is accepted (the runner tests the
116+// size of the file), 1 MiB plus one byte is refused with "got 1024k", 2 MiB
117+// with "got 2048k".
118+func TestStepSummarySizeLimit(t *testing.T) {
119+ needTools(t, "bash")
120+ rr, _ := runResults(t, wfHead+` steps:
121+ - name: exact
122+ run: head -c $((1024 * 1024)) /dev/zero | tr '\0' b >> "$GITHUB_STEP_SUMMARY"
123+ - name: plus1
124+ run: head -c $((1024 * 1024 + 1)) /dev/zero | tr '\0' c >> "$GITHUB_STEP_SUMMARY"
125+ - name: two
126+ run: head -c $((2 * 1024 * 1024)) /dev/zero | tr '\0' d >> "$GITHUB_STEP_SUMMARY"
127+`, nil)
128+ const tail = ", supports content up to a size of 1024k, got "
129+ if got := logOf(rr.step(t, "exact")); strings.Contains(got, "upload aborted") {
130+ t.Errorf("exact: %.200s", got)
131+ }
132+ if r := rr.results[2]; !r.HasSummary || len(r.Summary) != stepSummaryLimit {
133+ t.Errorf("exact: has=%v len=%d", r.HasSummary, len(r.Summary))
134+ }
135+ if got := logOf(rr.step(t, "plus1")); !strings.Contains(got, "##[error]$GITHUB_STEP_SUMMARY upload aborted"+tail+"1024k.") {
136+ t.Errorf("plus1: %.300s", got)
137+ }
138+ if got := logOf(rr.step(t, "two")); !strings.Contains(got, "##[error]$GITHUB_STEP_SUMMARY upload aborted"+tail+"2048k.") {
139+ t.Errorf("two: %.300s", got)
140+ }
141+}
142+
143+// An invalid stop-commands token ends the process: the step does not wait for
144+// the script (here 600 s) and fails with the endToken message.
145+func TestInvalidStopTokenStopsTheProcess(t *testing.T) {
146+ needTools(t, "bash", "sleep")
147+ start := time.Now()
148+ rec, res := runLocal(t, wfHead+" steps:\n - name: s\n run: |\n echo '::stop-commands::warning'\n sleep 600\n echo after\n", "j", nil)
149+ if d := time.Since(start); d > 30*time.Second {
150+ t.Fatalf("the step took %v", d)
151+ }
152+ got := logOf(rec.step(t, "s"))
153+ if !strings.Contains(got, "##[error]"+unsupportedStopTokenDisabled) || strings.Contains(got, "The operation was canceled.") {
154+ t.Error(got)
155+ }
156+ for _, l := range rec.step(t, "s").Log {
157+ if strings.TrimSpace(l) == "after" {
158+ t.Errorf("output after the fatal line was logged: %q", l)
159+ }
160+ }
161+ if rec.step(t, "s").Conclusion != ConclusionFailure || res.Conclusion != ConclusionFailure {
162+ t.Errorf("%v %v", rec.step(t, "s").Conclusion, res.Conclusion)
163+ }
164+}
165+
166+// Output written after the fatal line, a lot of it, does not block the process
167+// or the executor.
168+func TestInvalidStopTokenDrainsHighVolumeOutput(t *testing.T) {
169+ needTools(t, "bash", "yes", "head")
170+ done := make(chan struct{})
171+ var rec *recorder
172+ go func() {
173+ defer close(done)
174+ rec, _ = runLocal(t, wfHead+" steps:\n - name: s\n run: |\n echo '::stop-commands::'\n yes '::warning::x' | head -c 200000000\n sleep 600\n", "j", nil)
175+ }()
176+ select {
177+ case <-done:
178+ case <-time.After(60 * time.Second):
179+ t.Fatal("deadlock")
180+ }
181+ got := logOf(rec.step(t, "s"))
182+ if !strings.Contains(got, "##[error]"+unsupportedStopTokenDisabled) || strings.Contains("\n"+got+"\n", "\n::warning::x\n") || strings.Contains(got, "##[warning]x") {
183+ t.Errorf("%.300s", got)
184+ }
185+}
M internal/executor/containers_init.go +1 -1
@@ -76,7 +76,7 @@ var (
7676 // (JobExtension.cs:328-331), run if success().
7777 func (j *job) initContainersStep() qstep {
7878 return qstep{name: "Initialize containers", setup: func(s *stepRun, cancel <-chan struct{}) stepResult {
79- return j.initContainers(s, cancel)
79+ return j.recordRunnerStep(j.initContainers(s, cancel))
8080 }}
8181 }
8282
@@ -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.initContainers(s, cancel)79+ return j.recordRunnerStep(j.initContainers(s, cancel))
80 }}80 }}
81 }81 }
82 82
M internal/executor/contexts_containers.go +22 -1
@@ -2,7 +2,13 @@
22
33 package executor
44
5-import "bnema.dev/ermite-actions/expr"
5+import (
6+ "strings"
7+
8+ "github.com/google/uuid"
9+
10+ "bnema.dev/ermite-actions/expr"
11+)
612
713 // The job.container and job.services contexts (JobContext.cs:20-47,
814 // ContainerOperationProvider.cs:96, :215-232, :237-240). The runner's
@@ -61,3 +67,18 @@ func (s *State) SetJobService(alias, id, network string, ports []portMapping) {
6167 func replaceKey(d *expr.Dictionary, key string, v expr.Value) *expr.Dictionary {
6268 return setKey(d, key, v)
6369 }
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+}
@@ -2,7 +2,13 @@
2 2
3 package executor3 package executor
4 4
5-import "bnema.dev/ermite-actions/expr"5+import (
6+ "strings"
7+
8+ "github.com/google/uuid"
9+
10+ "bnema.dev/ermite-actions/expr"
11+)
6 12
7 // The job.container and job.services contexts (JobContext.cs:20-47,13 // The job.container and job.services contexts (JobContext.cs:20-47,
8 // ContainerOperationProvider.cs:96, :215-232, :237-240). The runner's14 // ContainerOperationProvider.cs:96, :215-232, :237-240). The runner's
@@ -61,3 +67,18 @@ func (s *State) SetJobService(alias, id, network string, ports []portMapping) {
61 func replaceKey(d *expr.Dictionary, key string, v expr.Value) *expr.Dictionary {67 func replaceKey(d *expr.Dictionary, key string, v expr.Value) *expr.Dictionary {
62 return setKey(d, key, v)68 return setKey(d, key, v)
63 }69 }
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+}
M internal/executor/exec_step.go +26 -1
@@ -6,6 +6,7 @@ import (
66 "fmt"
77 "path/filepath"
88 "strings"
9+ "sync"
910 )
1011
1112 // execute is ActionRunner.RunAsync plus ScriptHandler for one run step.
@@ -222,16 +223,40 @@ func (j *job) runProc(s *stepRun, shown, dir string, argv []string, exitDebug st
222223 num := s.num
223224 om := newOutputManager(s, stepCancel)
224225 defer om.dispose()
226+ // The process is stopped when the step is cancelled or when the output
227+ // handler throws (an invalid stop-commands token).
228+ procCancel := make(chan struct{})
229+ fatal := make(chan struct{})
230+ var fatalOnce sync.Once
231+ s.cmds.onFatal = func() { fatalOnce.Do(func() { close(fatal) }) }
232+ defer func() { s.cmds.onFatal = nil }()
233+ watchDone := make(chan struct{})
234+ defer func() { close(watchDone) }()
235+ go func() {
236+ select {
237+ case <-stepCancel:
238+ case <-fatal:
239+ case <-watchDone:
240+ return
241+ }
242+ close(procCancel)
243+ }()
225244 res, err := runProcess(procSpec{
226245 Argv: argv, Dir: dir, Env: s.procEnv(), Credential: j.cfg.Credential,
227246 OnLine: om.onData,
228- Cancel: stepCancel, TermAfter: j.cfg.TermAfter, KillAfter: j.cfg.KillAfter,
247+ Cancel: procCancel, TermAfter: j.cfg.TermAfter, KillAfter: j.cfg.KillAfter,
229248 WaitDelay: j.cfg.WaitDelay, Signal: j.cfg.Signal,
230249 })
231250 if err != nil {
232251 j.errorf(num, "%s", startError(shown, dir, err))
233252 return resFailed
234253 }
254+ if err := s.cmds.fatal; err != nil {
255+ // An exception of the output handler (an invalid stop-commands token)
256+ // ends ExecuteAsync; StepsRunner logs its message and fails the step.
257+ j.errorf(num, "%s", err.Error())
258+ return resFailed
259+ }
235260 if res.Cancelled {
236261 // The process was stopped on cancellation: ExecuteAsync throws
237262 // (ProcessInvoker.cs:369) and StepsRunner logs the exception
@@ -6,6 +6,7 @@ import (
6 "fmt"6 "fmt"
7 "path/filepath"7 "path/filepath"
8 "strings"8 "strings"
9+ "sync"
9 )10 )
10 11
11 // execute is ActionRunner.RunAsync plus ScriptHandler for one run step.12 // execute is ActionRunner.RunAsync plus ScriptHandler for one run step.
@@ -222,16 +223,40 @@ func (j *job) runProc(s *stepRun, shown, dir string, argv []string, exitDebug st
222 num := s.num223 num := s.num
223 om := newOutputManager(s, stepCancel)224 om := newOutputManager(s, stepCancel)
224 defer om.dispose()225 defer om.dispose()
226+ // The process is stopped when the step is cancelled or when the output
227+ // handler throws (an invalid stop-commands token).
228+ procCancel := make(chan struct{})
229+ fatal := make(chan struct{})
230+ var fatalOnce sync.Once
231+ s.cmds.onFatal = func() { fatalOnce.Do(func() { close(fatal) }) }
232+ defer func() { s.cmds.onFatal = nil }()
233+ watchDone := make(chan struct{})
234+ defer func() { close(watchDone) }()
235+ go func() {
236+ select {
237+ case <-stepCancel:
238+ case <-fatal:
239+ case <-watchDone:
240+ return
241+ }
242+ close(procCancel)
243+ }()
225 res, err := runProcess(procSpec{244 res, err := runProcess(procSpec{
226 Argv: argv, Dir: dir, Env: s.procEnv(), Credential: j.cfg.Credential,245 Argv: argv, Dir: dir, Env: s.procEnv(), Credential: j.cfg.Credential,
227 OnLine: om.onData,246 OnLine: om.onData,
228- Cancel: stepCancel, TermAfter: j.cfg.TermAfter, KillAfter: j.cfg.KillAfter,247+ Cancel: procCancel, TermAfter: j.cfg.TermAfter, KillAfter: j.cfg.KillAfter,
229 WaitDelay: j.cfg.WaitDelay, Signal: j.cfg.Signal,248 WaitDelay: j.cfg.WaitDelay, Signal: j.cfg.Signal,
230 })249 })
231 if err != nil {250 if err != nil {
232 j.errorf(num, "%s", startError(shown, dir, err))251 j.errorf(num, "%s", startError(shown, dir, err))
233 return resFailed252 return resFailed
234 }253 }
254+ if err := s.cmds.fatal; err != nil {
255+ // An exception of the output handler (an invalid stop-commands token)
256+ // ends ExecuteAsync; StepsRunner logs its message and fails the step.
257+ j.errorf(num, "%s", err.Error())
258+ return resFailed
259+ }
235 if res.Cancelled {260 if res.Cancelled {
236 // The process was stopped on cancellation: ExecuteAsync throws261 // The process was stopped on cancellation: ExecuteAsync throws
237 // (ProcessInvoker.cs:369) and StepsRunner logs the exception262 // (ProcessInvoker.cs:369) and StepsRunner logs the exception
M internal/executor/filecmd.go +13 -7
@@ -148,17 +148,16 @@ func (f *FileCommands) path(c fileCommand, suffix string) string {
148148 return filepath.Join(f.dir, f.name(c, suffix))
149149 }
150150
151-// Init creates fresh, empty files for the next step (after deleting the ones
152-// of the previous step) and returns the environment variables that name them
153-// (InitializeFiles, FileCommandManager.cs:42).
151+// Init creates fresh, empty files for the next step and returns the
152+// environment variables that name them (InitializeFiles,
153+// FileCommandManager.cs:42). The files of the previous step are kept: the
154+// runner makes a new FileCommandManager for every step (ActionRunner.cs:171),
155+// whose suffix starts empty, so it never deletes an old file (recorded:
156+// sum-env, "previous file still exists: yes").
154157 func (f *FileCommands) Init() (map[string]string, error) {
155- old := f.suffix
156158 f.suffix = uuid.NewString()
157159 env := make(map[string]string, len(fileCommands))
158160 for _, c := range fileCommands {
159- if old != "" {
160- _ = f.root.Remove(f.name(c, old))
161- }
162161 if err := createExcl(f.root, f.name(c, f.suffix), nil, 0o644, f.owner); err != nil {
163162 return nil, err
164163 }
@@ -414,6 +413,13 @@ func processStepSummary(sink Sink, root *os.Root, name string) error {
414413 mask = func(s string) string { return s }
415414 }
416415 content := summaryContent(text, mask)
416+ // The size test of the runner is on the file (above). A file of exactly
417+ // 1 MiB without a final line feed gets one from WriteLine and would pass the
418+ // limit of the contract by that byte (recorded: sum-too-large, "exactly
419+ // 1 MiB" uploads on GitHub): drop that line feed.
420+ if len(content) == stepSummaryLimit+1 && st.Size() <= stepSummaryLimit && !strings.HasSuffix(text, "\n") {
421+ content = content[:stepSummaryLimit]
422+ }
417423 if len(content) > stepSummaryLimit {
418424 // Masking made it longer than the limit of the contract.
419425 tooLarge(int64(len(content)))
@@ -148,17 +148,16 @@ func (f *FileCommands) path(c fileCommand, suffix string) string {
148 return filepath.Join(f.dir, f.name(c, suffix))148 return filepath.Join(f.dir, f.name(c, suffix))
149 }149 }
150 150
151-// Init creates fresh, empty files for the next step (after deleting the ones151+// Init creates fresh, empty files for the next step and returns the
152-// of the previous step) and returns the environment variables that name them152+// environment variables that name them (InitializeFiles,
153-// (InitializeFiles, FileCommandManager.cs:42).153+// FileCommandManager.cs:42). The files of the previous step are kept: the
154+// runner makes a new FileCommandManager for every step (ActionRunner.cs:171),
155+// whose suffix starts empty, so it never deletes an old file (recorded:
156+// sum-env, "previous file still exists: yes").
154 func (f *FileCommands) Init() (map[string]string, error) {157 func (f *FileCommands) Init() (map[string]string, error) {
155- old := f.suffix
156 f.suffix = uuid.NewString()158 f.suffix = uuid.NewString()
157 env := make(map[string]string, len(fileCommands))159 env := make(map[string]string, len(fileCommands))
158 for _, c := range fileCommands {160 for _, c := range fileCommands {
159- if old != "" {
160- _ = f.root.Remove(f.name(c, old))
161- }
162 if err := createExcl(f.root, f.name(c, f.suffix), nil, 0o644, f.owner); err != nil {161 if err := createExcl(f.root, f.name(c, f.suffix), nil, 0o644, f.owner); err != nil {
163 return nil, err162 return nil, err
164 }163 }
@@ -414,6 +413,13 @@ func processStepSummary(sink Sink, root *os.Root, name string) error {
414 mask = func(s string) string { return s }413 mask = func(s string) string { return s }
415 }414 }
416 content := summaryContent(text, mask)415 content := summaryContent(text, mask)
416+ // The size test of the runner is on the file (above). A file of exactly
417+ // 1 MiB without a final line feed gets one from WriteLine and would pass the
418+ // limit of the contract by that byte (recorded: sum-too-large, "exactly
419+ // 1 MiB" uploads on GitHub): drop that line feed.
420+ if len(content) == stepSummaryLimit+1 && st.Size() <= stepSummaryLimit && !strings.HasSuffix(text, "\n") {
421+ content = content[:stepSummaryLimit]
422+ }
417 if len(content) > stepSummaryLimit {423 if len(content) > stepSummaryLimit {
418 // Masking made it longer than the limit of the contract.424 // Masking made it longer than the limit of the contract.
419 tooLarge(int64(len(content)))425 tooLarge(int64(len(content)))
M internal/executor/filecmd_test.go +5 -3
@@ -119,13 +119,15 @@ func TestFileCommandsInit(t *testing.T) {
119119 if !strings.HasPrefix(filepath.Base(env["GITHUB_ARTIFACTS"]), "artifacts_") || !strings.HasPrefix(filepath.Base(env["GITHUB_ARTIFACTS_LIST"]), "artifacts_list_") {
120120 t.Fatalf("artifact prefixes: %v", env)
121121 }
122- // A second Init deletes the previous files and uses new names.
122+ // A second Init uses new names and keeps the previous files: the runner
123+ // makes a new FileCommandManager for each step, which never deletes an old
124+ // file (recorded: sum-env).
123125 env2, _ := fc.Init()
124126 if env2["GITHUB_ENV"] == env["GITHUB_ENV"] {
125127 t.Fatal("same file for two steps")
126128 }
127- if _, err := os.Stat(env["GITHUB_ENV"]); !os.IsNotExist(err) {
128- t.Fatalf("old file kept: %v", err)
129+ if _, err := os.Stat(env["GITHUB_ENV"]); err != nil {
130+ t.Fatalf("old file removed: %v", err)
129131 }
130132 }
131133
@@ -119,13 +119,15 @@ func TestFileCommandsInit(t *testing.T) {
119 if !strings.HasPrefix(filepath.Base(env["GITHUB_ARTIFACTS"]), "artifacts_") || !strings.HasPrefix(filepath.Base(env["GITHUB_ARTIFACTS_LIST"]), "artifacts_list_") {119 if !strings.HasPrefix(filepath.Base(env["GITHUB_ARTIFACTS"]), "artifacts_") || !strings.HasPrefix(filepath.Base(env["GITHUB_ARTIFACTS_LIST"]), "artifacts_list_") {
120 t.Fatalf("artifact prefixes: %v", env)120 t.Fatalf("artifact prefixes: %v", env)
121 }121 }
122- // A second Init deletes the previous files and uses new names.122+ // A second Init uses new names and keeps the previous files: the runner
123+ // makes a new FileCommandManager for each step, which never deletes an old
124+ // file (recorded: sum-env).
123 env2, _ := fc.Init()125 env2, _ := fc.Init()
124 if env2["GITHUB_ENV"] == env["GITHUB_ENV"] {126 if env2["GITHUB_ENV"] == env["GITHUB_ENV"] {
125 t.Fatal("same file for two steps")127 t.Fatal("same file for two steps")
126 }128 }
127- if _, err := os.Stat(env["GITHUB_ENV"]); !os.IsNotExist(err) {129+ if _, err := os.Stat(env["GITHUB_ENV"]); err != nil {
128- t.Fatalf("old file kept: %v", err)130+ t.Fatalf("old file removed: %v", err)
129 }131 }
130 }132 }
131 133
M internal/executor/issues.go +10 -1
@@ -190,7 +190,8 @@ func parseInt(s string) (int, bool) {
190190 // runner puts the properties in Issue.Data and lets the service build the
191191 // check annotation; negative or non numeric numbers cannot be sent, and are
192192 // left out. The title and the file are masked too (the runner masks only the
193-// message).
193+// message; on GitHub a secret in a title stays visible, recorded in
194+// mask-add-mask, which ermite deliberately does not reproduce).
194195 func (j *job) annotationOf(is issue, msg string) Annotation {
195196 a := Annotation{Level: is.Level, Message: msg}
196197 p := is.Props
@@ -209,6 +210,14 @@ func (j *job) annotationOf(is issue, msg string) Annotation {
209210 if a.EndLine != 0 && a.EndLine < a.StartLine {
210211 a.EndLine = 0
211212 }
213+ // GitHub's annotations end where they start when the command gave no end
214+ // (recorded: ann-levels, match-basic).
215+ if a.StartLine != 0 && a.EndLine == 0 {
216+ a.EndLine = a.StartLine
217+ }
218+ if a.StartColumn != 0 && a.EndColumn == 0 {
219+ a.EndColumn = a.StartColumn
220+ }
212221 return a
213222 }
214223
@@ -190,7 +190,8 @@ func parseInt(s string) (int, bool) {
190 // runner puts the properties in Issue.Data and lets the service build the190 // runner puts the properties in Issue.Data and lets the service build the
191 // check annotation; negative or non numeric numbers cannot be sent, and are191 // check annotation; negative or non numeric numbers cannot be sent, and are
192 // left out. The title and the file are masked too (the runner masks only the192 // left out. The title and the file are masked too (the runner masks only the
193-// message).193+// message; on GitHub a secret in a title stays visible, recorded in
194+// mask-add-mask, which ermite deliberately does not reproduce).
194 func (j *job) annotationOf(is issue, msg string) Annotation {195 func (j *job) annotationOf(is issue, msg string) Annotation {
195 a := Annotation{Level: is.Level, Message: msg}196 a := Annotation{Level: is.Level, Message: msg}
196 p := is.Props197 p := is.Props
@@ -209,6 +210,14 @@ func (j *job) annotationOf(is issue, msg string) Annotation {
209 if a.EndLine != 0 && a.EndLine < a.StartLine {210 if a.EndLine != 0 && a.EndLine < a.StartLine {
210 a.EndLine = 0211 a.EndLine = 0
211 }212 }
213+ // GitHub's annotations end where they start when the command gave no end
214+ // (recorded: ann-levels, match-basic).
215+ if a.StartLine != 0 && a.EndLine == 0 {
216+ a.EndLine = a.StartLine
217+ }
218+ if a.StartColumn != 0 && a.EndColumn == 0 {
219+ a.EndColumn = a.StartColumn
220+ }
212 return a221 return a
213 }222 }
214 223
M internal/executor/matchers.go +40 -2
@@ -4,12 +4,16 @@ package executor
44
55 import (
66 "encoding/json"
7+ "errors"
78 "fmt"
89 "strings"
910 "sync"
1011 "time"
1112
1213 "github.com/dlclark/regexp2"
14+ "github.com/dlclark/regexp2/syntax"
15+
16+ "bnema.dev/ermite-actions/expr"
1317 )
1418
1519 // Problem matchers: a port of Runner.Worker/IssueMatcher.cs (actions/runner,
@@ -85,7 +89,14 @@ func parseMatchersConfig(data []byte) (*matchersConfig, error) {
8589 if len(strings.TrimSpace(string(data))) == 0 {
8690 return nil, nil // StringUtil.ConvertFromJson of an empty text is null
8791 }
88- if err := json.Unmarshal(data, &c); err != nil {
92+ // IOUtil.LoadObject reads with Json.NET: its reader decides what is valid
93+ // (single quotes, trailing commas...) and words the errors. The value it
94+ // accepts is turned back into strict JSON for the structure below.
95+ v, err := expr.FromJSON(data)
96+ if err != nil {
97+ return nil, msgError(err.Error())
98+ }
99+ if err := json.Unmarshal([]byte(expr.ToJSON(v)), &c); err != nil {
89100 return nil, err
90101 }
91102 return &c, nil
@@ -96,7 +107,34 @@ func parseMatchersConfig(data []byte) (*matchersConfig, error) {
96107 const regexOptions = regexp2.ECMAScript
97108
98109 func compilePattern(p string) (*regexp2.Regexp, error) {
99- return regexp2.Compile(p, regexOptions)
110+ re, err := regexp2.Compile(p, regexOptions)
111+ if err != nil {
112+ return nil, dotnetRegexError(p, err)
113+ }
114+ return re, nil
115+}
116+
117+// dotnetRegexError words a pattern that does not compile like .NET's
118+// RegexParseException ("Invalid pattern '<p>' at offset <n>. <reason>"), for the
119+// failures whose offset is known: those at the end of the pattern. Others keep
120+// the text of the engine.
121+func dotnetRegexError(p string, err error) error {
122+ var se *syntax.Error
123+ if !errors.As(err, &se) {
124+ return err
125+ }
126+ var reason string
127+ switch se.Code {
128+ case syntax.ErrMissingParen:
129+ reason = "Not enough )'s."
130+ case syntax.ErrUnterminatedBracket:
131+ reason = "Unterminated [] set."
132+ case syntax.ErrIllegalEndEscape:
133+ reason = "Illegal \\ at end of pattern."
134+ default:
135+ return err
136+ }
137+ return ghErrorf("Invalid pattern '%s' at offset %d. %s", p, utf16Len(p), reason)
100138 }
101139
102140 // validate is IssueMatchersConfig.Validate (IssueMatcher.cs:262). The error
@@ -4,12 +4,16 @@ package executor
4 4
5 import (5 import (
6 "encoding/json"6 "encoding/json"
7+ "errors"
7 "fmt"8 "fmt"
8 "strings"9 "strings"
9 "sync"10 "sync"
10 "time"11 "time"
11 12
12 "github.com/dlclark/regexp2"13 "github.com/dlclark/regexp2"
14+ "github.com/dlclark/regexp2/syntax"
15+
16+ "bnema.dev/ermite-actions/expr"
13 )17 )
14 18
15 // Problem matchers: a port of Runner.Worker/IssueMatcher.cs (actions/runner,19 // Problem matchers: a port of Runner.Worker/IssueMatcher.cs (actions/runner,
@@ -85,7 +89,14 @@ func parseMatchersConfig(data []byte) (*matchersConfig, error) {
85 if len(strings.TrimSpace(string(data))) == 0 {89 if len(strings.TrimSpace(string(data))) == 0 {
86 return nil, nil // StringUtil.ConvertFromJson of an empty text is null90 return nil, nil // StringUtil.ConvertFromJson of an empty text is null
87 }91 }
88- if err := json.Unmarshal(data, &c); err != nil {92+ // IOUtil.LoadObject reads with Json.NET: its reader decides what is valid
93+ // (single quotes, trailing commas...) and words the errors. The value it
94+ // accepts is turned back into strict JSON for the structure below.
95+ v, err := expr.FromJSON(data)
96+ if err != nil {
97+ return nil, msgError(err.Error())
98+ }
99+ if err := json.Unmarshal([]byte(expr.ToJSON(v)), &c); err != nil {
89 return nil, err100 return nil, err
90 }101 }
91 return &c, nil102 return &c, nil
@@ -96,7 +107,34 @@ func parseMatchersConfig(data []byte) (*matchersConfig, error) {
96 const regexOptions = regexp2.ECMAScript107 const regexOptions = regexp2.ECMAScript
97 108
98 func compilePattern(p string) (*regexp2.Regexp, error) {109 func compilePattern(p string) (*regexp2.Regexp, error) {
99- return regexp2.Compile(p, regexOptions)110+ re, err := regexp2.Compile(p, regexOptions)
111+ if err != nil {
112+ return nil, dotnetRegexError(p, err)
113+ }
114+ return re, nil
115+}
116+
117+// dotnetRegexError words a pattern that does not compile like .NET's
118+// RegexParseException ("Invalid pattern '<p>' at offset <n>. <reason>"), for the
119+// failures whose offset is known: those at the end of the pattern. Others keep
120+// the text of the engine.
121+func dotnetRegexError(p string, err error) error {
122+ var se *syntax.Error
123+ if !errors.As(err, &se) {
124+ return err
125+ }
126+ var reason string
127+ switch se.Code {
128+ case syntax.ErrMissingParen:
129+ reason = "Not enough )'s."
130+ case syntax.ErrUnterminatedBracket:
131+ reason = "Unterminated [] set."
132+ case syntax.ErrIllegalEndEscape:
133+ reason = "Illegal \\ at end of pattern."
134+ default:
135+ return err
136+ }
137+ return ghErrorf("Invalid pattern '%s' at offset %d. %s", p, utf16Len(p), reason)
100 }138 }
101 139
102 // validate is IssueMatchersConfig.Validate (IssueMatcher.cs:262). The error140 // validate is IssueMatchersConfig.Validate (IssueMatcher.cs:262). The error
M internal/executor/outputmgr.go +3 -0
@@ -225,6 +225,9 @@ func (o *outputManager) removeMatcher(m *issueMatcher) {
225225 // onData is OutputManager.OnDataReceived.
226226 func (o *outputManager) onData(line string) {
227227 s := o.s
228+ if s.cmds.fatal != nil {
229+ return // the handler threw: ExecuteAsync is over, nothing more is read
230+ }
228231 // ## commands
229232 if line != "" && (strings.Contains(line, "##[") || strings.Contains(line, "::")) {
230233 if s.cmds.TryProcess(line) {
@@ -225,6 +225,9 @@ func (o *outputManager) removeMatcher(m *issueMatcher) {
225 // onData is OutputManager.OnDataReceived.225 // onData is OutputManager.OnDataReceived.
226 func (o *outputManager) onData(line string) {226 func (o *outputManager) onData(line string) {
227 s := o.s227 s := o.s
228+ if s.cmds.fatal != nil {
229+ return // the handler threw: ExecuteAsync is over, nothing more is read
230+ }
228 // ## commands231 // ## commands
229 if line != "" && (strings.Contains(line, "##[") || strings.Contains(line, "::")) {232 if line != "" && (strings.Contains(line, "##[") || strings.Contains(line, "::")) {
230 if s.cmds.TryProcess(line) {233 if s.cmds.TryProcess(line) {
M internal/executor/results_test.go +3 -3
@@ -42,9 +42,9 @@ func TestProblemMatcherEndToEnd(t *testing.T) {
4242 }
4343 got := rr.results[3].Annotations
4444 want := []Annotation{
45- {Level: LevelFailure, Message: "bad thing", File: "src/main.go", StartLine: 12, StartColumn: 5},
46- {Level: LevelWarning, Message: "just a warning", File: "src/main.go", StartLine: 7, StartColumn: 1},
47- {Level: LevelFailure, Message: "no such file", StartLine: 1, StartColumn: 1}, // the file does not exist: dropped
45+ {Level: LevelFailure, Message: "bad thing", File: "src/main.go", StartLine: 12, EndLine: 12, StartColumn: 5, EndColumn: 5},
46+ {Level: LevelWarning, Message: "just a warning", File: "src/main.go", StartLine: 7, EndLine: 7, StartColumn: 1, EndColumn: 1},
47+ {Level: LevelFailure, Message: "no such file", StartLine: 1, EndLine: 1, StartColumn: 1, EndColumn: 1}, // the file does not exist: dropped
4848 }
4949 if fmt.Sprint(got) != fmt.Sprint(want) {
5050 t.Errorf("annotations\n got %+v\nwant %+v", got, want)
@@ -42,9 +42,9 @@ func TestProblemMatcherEndToEnd(t *testing.T) {
42 }42 }
43 got := rr.results[3].Annotations43 got := rr.results[3].Annotations
44 want := []Annotation{44 want := []Annotation{
45- {Level: LevelFailure, Message: "bad thing", File: "src/main.go", StartLine: 12, StartColumn: 5},45+ {Level: LevelFailure, Message: "bad thing", File: "src/main.go", StartLine: 12, EndLine: 12, StartColumn: 5, EndColumn: 5},
46- {Level: LevelWarning, Message: "just a warning", File: "src/main.go", StartLine: 7, StartColumn: 1},46+ {Level: LevelWarning, Message: "just a warning", File: "src/main.go", StartLine: 7, EndLine: 7, StartColumn: 1, EndColumn: 1},
47- {Level: LevelFailure, Message: "no such file", StartLine: 1, StartColumn: 1}, // the file does not exist: dropped47+ {Level: LevelFailure, Message: "no such file", StartLine: 1, EndLine: 1, StartColumn: 1, EndColumn: 1}, // the file does not exist: dropped
48 }48 }
49 if fmt.Sprint(got) != fmt.Sprint(want) {49 if fmt.Sprint(got) != fmt.Sprint(want) {
50 t.Errorf("annotations\n got %+v\nwant %+v", got, want)50 t.Errorf("annotations\n got %+v\nwant %+v", got, want)

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 4

Tags 11

↑↓ moveenter openesc close