git724/forgejo
1
0
Fork 0
Code Issues Pull requests 3 Projects 1 Releases Packages Wiki Activity

[v13.0/forgejo] fix: failure to parse on block results in unconditional workflow execution (#9536)

Browse source 
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9536
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
This commit is contained in:
Earl Warren 2025-10-06 06:50:52 +02:00
commit 80fcbf165c
11 changed files with 182 additions and 63 deletions
Download patch file Download diff file Expand all files Collapse all files

2
models/actions/run.go
View file

@ -56,6 +56,8 @@ type ActionRun struct {
Created timeutil.TimeStamp `xorm:"created"`
Updated timeutil.TimeStamp `xorm:"updated"`
NotifyEmail bool
PreExecutionError string `xorm:"LONGTEXT"` // used to report errors that blocked execution of a workflow
}
func init() {

3
models/forgejo_migrations/migrate.go
View file

@ -119,6 +119,9 @@ var migrations = []*Migration{
NewMigration("Migrate `data` column of `secret` table to store keying material", MigrateActionSecretsToKeying),
// v39 -> v40
NewMigration("Add index for release sha1", AddIndexForReleaseSha1),
// NOTE: v42 -> v43 -- effectively backported into Forgejo v13 as part of backporting
// https://codeberg.org/forgejo/forgejo/pulls/9530, but the migration was omitted to avoid future upgrade conflicts.
// The migration will effectively occur automatically via table `Sync` on DB engine initialization.
}
// GetCurrentDBVersion returns the current Forgejo database version.

10
modules/actions/workflows.go
View file

@ -21,9 +21,10 @@ import (
)
type DetectedWorkflow struct {
EntryName string
TriggerEvent *jobparser.Event
Content []byte
EntryName string
TriggerEvent *jobparser.Event
Content []byte
EventDetectionError error
}
func init() {
@ -127,7 +128,8 @@ func DetectWorkflows(
TriggerEvent: &jobparser.Event{
Name: triggedEvent.Event(),
},
Content: content,
Content: content,
EventDetectionError: err,
}
workflows = append(workflows, dwf)
continue

3
options/locale_next/locale_en-US.json
View file

@ -150,5 +150,8 @@
"actions.runs.run_attempt_label": "Run attempt #%[1]s (%[2]s)",
"actions.runs.viewing_out_of_date_run": "You are viewing an out-of-date run of this job that was executed %[1]s.",
"actions.runs.view_most_recent_run": "View most recent run",
"actions.workflow.job_parsing_error": "Unable to parse jobs in workflow: %v",
"actions.workflow.event_detection_error": "Unable to parse supported events in workflow: %v",
"actions.workflow.pre_execution_error": "Workflow was not executed due to an error that blocked the execution attempt.",
"meta.last_line": "Thank you for translating Forgejo! This line isn't seen by the users but it serves other purposes in the translation management. You can place a fun fact in the translation instead of translating it."
}

2
routers/web/repo/actions/view.go
View file

@ -178,6 +178,7 @@ type ViewRunInfo struct {
Done bool `json:"done"`
Jobs []*ViewJob `json:"jobs"`
Commit ViewCommit `json:"commit"`
PreExecutionError string `json:"preExecutionError"`
}
type ViewCurrentJob struct {
@ -285,6 +286,7 @@ func getViewResponse(ctx *context_module.Context, req *ViewRequest, runIndex, jo
resp.State.Run.CanDeleteArtifact = run.Status.IsDone() && ctx.Repo.CanWrite(unit.TypeActions)
resp.State.Run.Jobs = make([]*ViewJob, 0, len(jobs)) // marshal to '[]' instead of 'null' in json
resp.State.Run.Status = run.Status.String()
resp.State.Run.PreExecutionError = run.PreExecutionError
// It's possible for the run to be marked with a finalized status (eg. failure) because of a single job within the
// run; eg. one job fails, the run fails. But other jobs can still be running. The frontend RepoActionView uses the

19
services/actions/notifier_helper.go
View file

@ -26,6 +26,7 @@ import (
"forgejo.org/modules/log"
"forgejo.org/modules/setting"
api "forgejo.org/modules/structs"
"forgejo.org/modules/translation"
"forgejo.org/modules/util"
webhook_module "forgejo.org/modules/webhook"
"forgejo.org/services/convert"
@ -378,13 +379,25 @@ func handleWorkflows(
continue
}
jobs, err := jobParser(dwf.Content, jobparser.WithVars(vars))
if err != nil {
var jobs []*jobparser.SingleWorkflow
if dwf.EventDetectionError != nil { // don't even bother trying to parse jobs due to event detection error
tr := translation.NewLocale(input.Doer.Language)
run.PreExecutionError = tr.TrString("actions.workflow.event_detection_error", dwf.EventDetectionError)
run.Status = actions_model.StatusFailure
log.Info("jobparser.Parse: invalid workflow, setting job status to failed: %v", err)
jobs = []*jobparser.SingleWorkflow{{
Name: dwf.EntryName,
}}
} else {
jobs, err = jobParser(dwf.Content, jobparser.WithVars(vars))
if err != nil {
log.Info("jobparser.Parse: invalid workflow, setting job status to failed: %v", err)
tr := translation.NewLocale(input.Doer.Language)
run.PreExecutionError = tr.TrString("actions.workflow.job_parsing_error", err)
run.Status = actions_model.StatusFailure
jobs = []*jobparser.SingleWorkflow{{
Name: dwf.EntryName,
}}
}
}
// cancel running jobs if the event is push or pull_request_sync

86
services/actions/notifier_helper_test.go
View file

@ -4,6 +4,7 @@
package actions
import (
"errors"
"testing"
actions_model "forgejo.org/models/actions"
@ -144,3 +145,88 @@ func Test_OpenForkPullRequestEvent(t *testing.T) {
assert.Equal(t, webhook_module.HookEventPullRequest, runs[0].Event)
assert.True(t, runs[0].IsForkPullRequest)
}
func TestActionsPreExecutionErrorInvalidJobs(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10})
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3})
commit := &git.Commit{
ID: git.MustIDFromString("0000000000000000000000000000000000000000"),
CommitMessage: "test",
}
detectedWorkflows := []*actions_module.DetectedWorkflow{
{
EntryName: "test.yml",
TriggerEvent: &jobparser.Event{
Name: "pull_request",
},
Content: []byte("{ on: pull_request, jobs: 'hello, I am the jobs!' }"),
},
}
input := &notifyInput{
Repo: repo,
Doer: doer,
Event: webhook_module.HookEventPullRequestSync,
PullRequest: pr,
Payload: &api.PullRequestPayload{},
}
err := handleWorkflows(db.DefaultContext, detectedWorkflows, commit, input, "refs/head/main")
require.NoError(t, err)
runs, err := db.Find[actions_model.ActionRun](db.DefaultContext, actions_model.FindRunOptions{
RepoID: repo.ID,
})
require.NoError(t, err)
require.Len(t, runs, 1)
createdRun := runs[0]
assert.Equal(t, actions_model.StatusFailure, createdRun.Status)
assert.Contains(t, createdRun.PreExecutionError, "actions.workflow.job_parsing_error%!(EXTRA *fmt.wrapError=")
}
func TestActionsPreExecutionEventDetectionError(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10})
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3})
commit := &git.Commit{
ID: git.MustIDFromString("0000000000000000000000000000000000000000"),
CommitMessage: "test",
}
detectedWorkflows := []*actions_module.DetectedWorkflow{
{
EntryName: "test.yml",
TriggerEvent: &jobparser.Event{
Name: "pull_request",
},
Content: []byte("{ on: nothing, jobs: { j1: {} }}"),
EventDetectionError: errors.New("nothing is not a valid event"),
},
}
input := &notifyInput{
Repo: repo,
Doer: doer,
Event: webhook_module.HookEventPullRequestSync,
PullRequest: pr,
Payload: &api.PullRequestPayload{},
}
err := handleWorkflows(db.DefaultContext, detectedWorkflows, commit, input, "refs/head/main")
require.NoError(t, err)
runs, err := db.Find[actions_model.ActionRun](db.DefaultContext, actions_model.FindRunOptions{
RepoID: repo.ID,
})
require.NoError(t, err)
require.Len(t, runs, 1)
createdRun := runs[0]
assert.Equal(t, actions_model.StatusFailure, createdRun.Status)
assert.Equal(t, "actions.workflow.event_detection_error%!(EXTRA *errors.errorString=nothing is not a valid event)", createdRun.PreExecutionError)
}

1
templates/repo/actions/view.tmpl
View file

@ -33,6 +33,7 @@
data-locale-run-attempt-label="{{ctx.Locale.Tr "actions.runs.run_attempt_label"}}"
data-locale-viewing-out-of-date-run="{{ctx.Locale.Tr "actions.runs.viewing_out_of_date_run"}}"
data-locale-view-most-recent-run="{{ctx.Locale.Tr "actions.runs.view_most_recent_run"}}"
data-locale-pre-execution-error="{{ctx.Locale.Tr "actions.workflow.pre_execution_error"}}"
>
</div>
</div>

4
tests/integration/actions_view_test.go
View file

@ -152,7 +152,7 @@ func TestActionViewsView(t *testing.T) {
re = regexp.MustCompile(pattern)
actualClean = re.ReplaceAllString(actualClean, `"time_since_started_html":"_time_"`)
return assert.JSONEq(t, "{\"state\":{\"run\":{\"link\":\"/user5/repo4/actions/runs/187\",\"title\":\"update actions\",\"titleHTML\":\"update actions\",\"status\":\"success\",\"canCancel\":false,\"canApprove\":false,\"canRerun\":false,\"canDeleteArtifact\":false,\"done\":true,\"jobs\":[{\"id\":192,\"name\":\"job_2\",\"status\":\"success\",\"canRerun\":false,\"duration\":\"_duration_\"}],\"commit\":{\"localeCommit\":\"Commit\",\"localePushedBy\":\"pushed by\",\"localeWorkflow\":\"Workflow\",\"shortSHA\":\"c2d72f5484\",\"link\":\"/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0\",\"pusher\":{\"displayName\":\"user1\",\"link\":\"/user1\"},\"branch\":{\"name\":\"master\",\"link\":\"/user5/repo4/src/branch/master\",\"isDeleted\":false}}},\"currentJob\":{\"title\":\"job_2\",\"detail\":\"Success\",\"steps\":[{\"summary\":\"Set up job\",\"duration\":\"_duration_\",\"status\":\"success\"},{\"summary\":\"Complete job\",\"duration\":\"_duration_\",\"status\":\"success\"}],\"allAttempts\":[{\"number\":3,\"time_since_started_html\":\"_time_\",\"status\":\"running\"},{\"number\":2,\"time_since_started_html\":\"_time_\",\"status\":\"success\"},{\"number\":1,\"time_since_started_html\":\"_time_\",\"status\":\"success\"}]}},\"logs\":{\"stepsLog\":[]}}\n", actualClean)
return assert.JSONEq(t, "{\"state\":{\"run\":{\"preExecutionError\":\"\",\"link\":\"/user5/repo4/actions/runs/187\",\"title\":\"update actions\",\"titleHTML\":\"update actions\",\"status\":\"success\",\"canCancel\":false,\"canApprove\":false,\"canRerun\":false,\"canDeleteArtifact\":false,\"done\":true,\"jobs\":[{\"id\":192,\"name\":\"job_2\",\"status\":\"success\",\"canRerun\":false,\"duration\":\"_duration_\"}],\"commit\":{\"localeCommit\":\"Commit\",\"localePushedBy\":\"pushed by\",\"localeWorkflow\":\"Workflow\",\"shortSHA\":\"c2d72f5484\",\"link\":\"/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0\",\"pusher\":{\"displayName\":\"user1\",\"link\":\"/user1\"},\"branch\":{\"name\":\"master\",\"link\":\"/user5/repo4/src/branch/master\",\"isDeleted\":false}}},\"currentJob\":{\"title\":\"job_2\",\"detail\":\"Success\",\"steps\":[{\"summary\":\"Set up job\",\"duration\":\"_duration_\",\"status\":\"success\"},{\"summary\":\"Complete job\",\"duration\":\"_duration_\",\"status\":\"success\"}],\"allAttempts\":[{\"number\":3,\"time_since_started_html\":\"_time_\",\"status\":\"running\"},{\"number\":2,\"time_since_started_html\":\"_time_\",\"status\":\"success\"},{\"number\":1,\"time_since_started_html\":\"_time_\",\"status\":\"success\"}]}},\"logs\":{\"stepsLog\":[]}}\n", actualClean)
})
htmlDoc.AssertAttrEqual(t, selector, "data-initial-artifacts-response", "{\"artifacts\":[{\"name\":\"multi-file-download\",\"size\":2048,\"status\":\"completed\"}]}\n")
}
@ -184,7 +184,7 @@ func TestActionViewsViewAttemptOutOfRange(t *testing.T) {
re = regexp.MustCompile(pattern)
actualClean = re.ReplaceAllString(actualClean, `"time_since_started_html":"_time_"`)
return assert.JSONEq(t, "{\"state\":{\"run\":{\"link\":\"/user5/repo4/actions/runs/190\",\"title\":\"job output\",\"titleHTML\":\"job output\",\"status\":\"success\",\"canCancel\":false,\"canApprove\":false,\"canRerun\":false,\"canDeleteArtifact\":false,\"done\":false,\"jobs\":[{\"id\":396,\"name\":\"job_2\",\"status\":\"waiting\",\"canRerun\":false,\"duration\":\"_duration_\"}],\"commit\":{\"localeCommit\":\"Commit\",\"localePushedBy\":\"pushed by\",\"localeWorkflow\":\"Workflow\",\"shortSHA\":\"c2d72f5484\",\"link\":\"/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0\",\"pusher\":{\"displayName\":\"user1\",\"link\":\"/user1\"},\"branch\":{\"name\":\"test\",\"link\":\"/user5/repo4/src/branch/test\",\"isDeleted\":true}}},\"currentJob\":{\"title\":\"job_2\",\"detail\":\"Waiting\",\"steps\":[],\"allAttempts\":null}},\"logs\":{\"stepsLog\":[]}}\n", actualClean)
return assert.JSONEq(t, "{\"state\":{\"run\":{\"preExecutionError\":\"\",\"link\":\"/user5/repo4/actions/runs/190\",\"title\":\"job output\",\"titleHTML\":\"job output\",\"status\":\"success\",\"canCancel\":false,\"canApprove\":false,\"canRerun\":false,\"canDeleteArtifact\":false,\"done\":false,\"jobs\":[{\"id\":396,\"name\":\"job_2\",\"status\":\"waiting\",\"canRerun\":false,\"duration\":\"_duration_\"}],\"commit\":{\"localeCommit\":\"Commit\",\"localePushedBy\":\"pushed by\",\"localeWorkflow\":\"Workflow\",\"shortSHA\":\"c2d72f5484\",\"link\":\"/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0\",\"pusher\":{\"displayName\":\"user1\",\"link\":\"/user1\"},\"branch\":{\"name\":\"test\",\"link\":\"/user5/repo4/src/branch/test\",\"isDeleted\":true}}},\"currentJob\":{\"title\":\"job_2\",\"detail\":\"Waiting\",\"steps\":[],\"allAttempts\":null}},\"logs\":{\"stepsLog\":[]}}\n", actualClean)
})
htmlDoc.AssertAttrEqual(t, selector, "data-initial-artifacts-response", "{\"artifacts\":[]}\n")
}

107
web_src/js/components/RepoActionView.test.js
View file

@ -17,6 +17,7 @@ const testLocale = {
runAttemptLabel: 'Run attempt %[1]s %[2]s',
viewingOutOfDateRun: 'oh no, out of date since %[1]s give or take or so',
viewMostRecentRun: '',
preExecutionError: 'pre-execution error',
status: {
unknown: '',
waiting: '',
@ -53,6 +54,18 @@ const minimalInitialJobData = {
const minimalInitialArtifactData = {
artifacts: [],
};
const defaultTestProps = {
actionsURL: 'https://example.com/example-org/example-repo/actions',
jobIndex: '1',
attemptNumber: '1',
runIndex: '10',
runID: '1001',
initialJobData: minimalInitialJobData,
initialArtifactData: minimalInitialArtifactData,
locale: testLocale,
workflowName: 'workflow name',
workflowURL: 'https://example.com/example-org/example-repo/actions?workflow=test.yml',
};
test('processes ##[group] and ##[endgroup]', async () => {
Object.defineProperty(document.documentElement, 'lang', {value: 'en'});
@ -105,13 +118,7 @@ test('processes ##[group] and ##[endgroup]', async () => {
});
const wrapper = mount(RepoActionView, {
props: {
jobIndex: '1',
attemptNumber: '1',
initialJobData: minimalInitialJobData,
initialArtifactData: minimalInitialArtifactData,
locale: testLocale,
},
props: defaultTestProps,
});
await flushPromises();
await wrapper.get('.job-step-summary').trigger('click');
@ -208,15 +215,7 @@ test('load multiple steps on a finished action', async () => {
});
const wrapper = mount(RepoActionView, {
props: {
actionsURL: 'https://example.com/example-org/example-repo/actions',
initialJobData: minimalInitialJobData,
initialArtifactData: minimalInitialArtifactData,
runIndex: '1',
jobIndex: '2',
attemptNumber: '1',
locale: testLocale,
},
props: defaultTestProps,
});
wrapper.vm.loadJob(); // simulate intermittent reload immediately so UI switches from minimalInitialJobData to the mock data from the test's fetch spy.
await flushPromises();
@ -288,13 +287,11 @@ function configureForMultipleAttemptTests({viewHistorical}) {
const wrapper = mount(RepoActionView, {
props: {
...defaultTestProps,
runIndex: '123',
jobIndex: '1',
attemptNumber: viewHistorical ? '1' : '2',
actionsURL: toAbsoluteUrl('/user1/repo2/actions'),
initialJobData: {...minimalInitialJobData, state: myJobState},
initialArtifactData: minimalInitialArtifactData,
locale: testLocale,
},
});
return wrapper;
@ -465,16 +462,7 @@ test('artifacts download links', async () => {
});
const wrapper = mount(RepoActionView, {
props: {
actionsURL: 'https://example.com/example-org/example-repo/actions',
initialJobData: minimalInitialJobData,
initialArtifactData: minimalInitialArtifactData,
runIndex: '10',
runID: '1001',
jobIndex: '2',
attemptNumber: '1',
locale: testLocale,
},
props: defaultTestProps,
});
wrapper.vm.loadJob(); // simulate intermittent reload immediately so UI switches from minimalInitialJobData to the mock data from the test's fetch spy.
await flushPromises();
@ -501,11 +489,8 @@ test('initial load schedules refresh when job is not done', async () => {
doneInitialJobData.state.run.done = true;
const wrapper = mount(RepoActionView, {
props: {
jobIndex: '1',
attemptNumber: '1',
...defaultTestProps,
initialJobData: doneInitialJobData,
initialArtifactData: minimalInitialArtifactData,
locale: testLocale,
},
});
await flushPromises();
@ -520,13 +505,7 @@ test('initial load schedules refresh when job is not done', async () => {
const runningInitialJobData = structuredClone(minimalInitialJobData);
runningInitialJobData.state.run.done = false;
const wrapper = mount(RepoActionView, {
props: {
jobIndex: '1',
attemptNumber: '1',
initialJobData: runningInitialJobData,
initialArtifactData: minimalInitialArtifactData,
locale: testLocale,
},
props: defaultTestProps,
});
await flushPromises();
const container = wrapper.find('.action-view-container');
@ -548,13 +527,7 @@ test('initial load data is used without calling fetch()', async () => {
});
mount(RepoActionView, {
props: {
jobIndex: '1',
attemptNumber: '1',
initialJobData: minimalInitialJobData,
initialArtifactData: minimalInitialArtifactData,
locale: testLocale,
},
props: defaultTestProps,
});
await flushPromises();
expect(fetchSpy).not.toHaveBeenCalled();
@ -564,11 +537,7 @@ test('view non-picked action run job', async () => {
Object.defineProperty(document.documentElement, 'lang', {value: 'en'});
const wrapper = mount(RepoActionView, {
props: {
actionsURL: 'https://example.com/example-org/example-repo/actions',
runIndex: '10',
runID: '1001',
jobIndex: '2',
attemptNumber: '1',
...defaultTestProps,
initialJobData: {
...minimalInitialJobData,
// Definitions here should match the same type of content as the related backend test,
@ -613,8 +582,6 @@ test('view non-picked action run job', async () => {
},
},
},
initialArtifactData: minimalInitialArtifactData,
locale: testLocale,
},
});
await flushPromises();
@ -624,3 +591,35 @@ test('view non-picked action run job', async () => {
expect(wrapper.get('.job-brief-list .job-brief-item:nth-of-type(2) .job-brief-name').text()).toEqual('check-2');
expect(wrapper.get('.job-brief-list .job-brief-item:nth-of-type(3) .job-brief-name').text()).toEqual('check-3');
});
test('view without pre-execution error', async () => {
Object.defineProperty(document.documentElement, 'lang', {value: 'en'});
const wrapper = mount(RepoActionView, {
props: defaultTestProps,
});
await flushPromises();
expect(wrapper.find('.pre-execution-error').exists()).toBe(false);
});
test('view with pre-execution error', async () => {
Object.defineProperty(document.documentElement, 'lang', {value: 'en'});
const wrapper = mount(RepoActionView, {
props: {
...defaultTestProps,
initialJobData: {
...minimalInitialJobData,
state: {
...minimalInitialJobData.state,
run: {
...minimalInitialJobData.state.run,
preExecutionError: 'Oops, I dropped it.',
},
},
},
},
});
await flushPromises();
const block = wrapper.find('.pre-execution-error');
expect(block.exists()).toBe(true);
expect(block.text()).toBe('pre-execution error Oops, I dropped it.');
});

8
web_src/js/components/RepoActionView.vue
View file

@ -52,6 +52,7 @@ const sfc = {
canApprove: false,
canRerun: false,
done: false,
preExecutionError: '',
jobs: [
// {
// id: 0,
@ -525,6 +526,7 @@ export function initRepositoryActionView() {
runAttemptLabel: el.getAttribute('data-locale-run-attempt-label'),
viewingOutOfDateRun: el.getAttribute('data-locale-viewing-out-of-date-run'),
viewMostRecentRun: el.getAttribute('data-locale-view-most-recent-run'),
preExecutionError: el.getAttribute('data-locale-pre-execution-error'),
status: {
unknown: el.getAttribute('data-locale-status-unknown'),
waiting: el.getAttribute('data-locale-status-waiting'),
@ -582,6 +584,12 @@ export function initRepositoryActionView() {
{{ run.commit.localeWorkflow }}
<a class="muted" :href="workflowURL">{{ workflowName }}</a>
</div>
<div class="ui error message pre-execution-error" v-if="run.preExecutionError">
<div class="header">
{{ locale.preExecutionError }}
</div>
{{ run.preExecutionError }}
</div>
</div>
<div class="action-view-body">
<div class="action-view-left" v-if="displayOtherJobs">