Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/src/content/docs/reference/steps-jobs.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ jobs:
if: needs.build.outputs.outcome == 'failure'
```

`jobs.<built-in>.needs` is merged with compiler-generated dependencies, and `jobs.<built-in>.if` is combined with compiler-generated conditions using logical `&&`. `jobs.<built-in>.timeout-minutes` is accepted for the `agent` and `detection` jobs only; see [Agent and Detection Job Timeouts](#agent-and-detection-job-timeouts).
`jobs.<built-in>.needs` is merged with compiler-generated dependencies, and `jobs.<built-in>.if` is combined with compiler-generated conditions using logical `&&`. `jobs.agent.continue-on-error` accepts a boolean and applies it to the generated agent job; other built-in jobs reject this field. `jobs.<built-in>.timeout-minutes` is accepted for the `agent` and `detection` jobs only; see [Agent and Detection Job Timeouts](#agent-and-detection-job-timeouts).

Example using `timeout-minutes` and `env`:

Expand Down
128 changes: 128 additions & 0 deletions pkg/workflow/compiler_agent_job_continue_on_error_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
//go:build !integration

package workflow

import (
"os"
"path/filepath"
"testing"

"github.com/github/gh-aw/pkg/constants"
"github.com/github/gh-aw/pkg/testutil"
"github.com/goccy/go-yaml"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestGeneratedAgentJobContinueOnError(t *testing.T) {
tests := []struct {
name string
frontmatter string
want bool
wantPresent bool
}{
{
name: "true",
frontmatter: "jobs:\n agent:\n continue-on-error: true\n",
want: true,
wantPresent: true,
},
{
name: "explicit false",
frontmatter: "jobs:\n agent:\n continue-on-error: false\n",
wantPresent: true,
},
{
name: "omitted",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
agent := compileContinueOnErrorTestWorkflow(t, tt.frontmatter)[string(constants.AgentJobName)]
value, present := agent["continue-on-error"]
assert.Equal(t, tt.wantPresent, present)
if tt.wantPresent {
assert.Equal(t, tt.want, value)
}
})
}
}

func TestGeneratedAgentJobContinueOnErrorFromImport(t *testing.T) {
tmpDir := testutil.TempDir(t, "agent-continue-on-error-import")
sharedPath := filepath.Join(tmpDir, "shared.md")
require.NoError(t, os.WriteFile(sharedPath, []byte("---\njobs:\n agent:\n continue-on-error: true\n---\n"), 0o644))

workflowPath := filepath.Join(tmpDir, "imported.md")
workflow := "---\non: workflow_dispatch\npermissions:\n contents: read\nengine: copilot\nstrict: false\nimports:\n - ./shared.md\n---\n\nTest workflow.\n"
require.NoError(t, os.WriteFile(workflowPath, []byte(workflow), 0o644))

jobs := compileContinueOnErrorWorkflowFile(t, workflowPath)
assert.Equal(t, true, jobs[string(constants.AgentJobName)]["continue-on-error"])
}

func TestGeneratedAgentJobContinueOnErrorImportAppliesWhenMainOmitsField(t *testing.T) {
tmpDir := testutil.TempDir(t, "agent-continue-on-error-import-merge")
sharedPath := filepath.Join(tmpDir, "shared.md")
require.NoError(t, os.WriteFile(sharedPath, []byte("---\njobs:\n agent:\n continue-on-error: true\n---\n"), 0o644))

workflowPath := filepath.Join(tmpDir, "imported.md")
workflow := "---\non: workflow_dispatch\npermissions:\n contents: read\nengine: copilot\nstrict: false\nimports:\n - ./shared.md\njobs:\n agent:\n timeout-minutes: 30\n---\n\nTest workflow.\n"
require.NoError(t, os.WriteFile(workflowPath, []byte(workflow), 0o644))

jobs := compileContinueOnErrorWorkflowFile(t, workflowPath)
assert.Equal(t, true, jobs[string(constants.AgentJobName)]["continue-on-error"])
assert.Equal(t, uint64(30), jobs[string(constants.AgentJobName)]["timeout-minutes"])
}

func TestGeneratedAgentJobContinueOnErrorMainWinsOverImport(t *testing.T) {
tmpDir := testutil.TempDir(t, "agent-continue-on-error-main-wins")
sharedPath := filepath.Join(tmpDir, "shared.md")
require.NoError(t, os.WriteFile(sharedPath, []byte("---\njobs:\n agent:\n continue-on-error: true\n---\n"), 0o644))

workflowPath := filepath.Join(tmpDir, "imported.md")
workflow := "---\non: workflow_dispatch\npermissions:\n contents: read\nengine: copilot\nstrict: false\nimports:\n - ./shared.md\njobs:\n agent:\n continue-on-error: false\n---\n\nTest workflow.\n"
require.NoError(t, os.WriteFile(workflowPath, []byte(workflow), 0o644))

jobs := compileContinueOnErrorWorkflowFile(t, workflowPath)
assert.Equal(t, false, jobs[string(constants.AgentJobName)]["continue-on-error"])
}

func TestContinueOnErrorRejectedForUnsupportedBuiltinJob(t *testing.T) {
tmpDir := testutil.TempDir(t, "activation-continue-on-error")
workflowPath := filepath.Join(tmpDir, "unsupported.md")
workflow := "---\non: workflow_dispatch\npermissions:\n contents: read\nengine: copilot\nstrict: false\njobs:\n activation:\n continue-on-error: true\n---\n\nTest workflow.\n"
require.NoError(t, os.WriteFile(workflowPath, []byte(workflow), 0o644))

err := NewCompiler().CompileWorkflow(workflowPath)
require.Error(t, err)
assert.Contains(t, err.Error(), "jobs.activation.continue-on-error is supported only for the generated agent job")
}

func TestCustomJobContinueOnErrorRemainsSupported(t *testing.T) {
jobs := compileContinueOnErrorTestWorkflow(t, "jobs:\n optional:\n runs-on: ubuntu-latest\n continue-on-error: true\n steps:\n - run: echo optional\n")
assert.Equal(t, true, jobs["optional"]["continue-on-error"])
}

func compileContinueOnErrorTestWorkflow(t *testing.T, frontmatter string) map[string]map[string]any {
t.Helper()
tmpDir := testutil.TempDir(t, "job-continue-on-error")
workflowPath := filepath.Join(tmpDir, "workflow.md")
workflow := "---\non: workflow_dispatch\npermissions:\n contents: read\nengine: copilot\nstrict: false\n" + frontmatter + "---\n\nTest workflow.\n"
require.NoError(t, os.WriteFile(workflowPath, []byte(workflow), 0o644))
return compileContinueOnErrorWorkflowFile(t, workflowPath)
}

func compileContinueOnErrorWorkflowFile(t *testing.T, workflowPath string) map[string]map[string]any {
t.Helper()
require.NoError(t, NewCompiler().CompileWorkflow(workflowPath))

lockContent, err := os.ReadFile(workflowPath[:len(workflowPath)-len(filepath.Ext(workflowPath))] + ".lock.yml")
require.NoError(t, err)
var compiled struct {
Jobs map[string]map[string]any `yaml:"jobs"`
}
require.NoError(t, yaml.Unmarshal(lockContent, &compiled))
return compiled.Jobs
}
31 changes: 24 additions & 7 deletions pkg/workflow/compiler_builtin_job_augmentation.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,11 @@ func (c *Compiler) applyBuiltinJobAugmentations(data *WorkflowData) error {
}

type builtinJobAugmentation struct {
needs []string
ifCondition string
hasPermissions bool
hasTimeout bool
needs []string
ifCondition string
hasPermissions bool
hasTimeout bool
hasContinueOnError bool
}

func parseBuiltinJobAugmentation(jobName, targetJobName string, rawConfig any) (builtinJobAugmentation, map[string]any, error) {
Expand All @@ -231,10 +232,20 @@ func parseBuiltinJobAugmentation(jobName, targetJobName string, rawConfig any) (
}
_, hasPermissions := configMap["permissions"]
_, hasTimeout := configMap["timeout-minutes"]
_, hasContinueOnError := configMap["continue-on-error"]
if hasTimeout && targetJobName != string(constants.AgentJobName) && targetJobName != string(constants.DetectionJobName) {
return builtinJobAugmentation{}, nil, fmt.Errorf("jobs.%s.timeout-minutes is supported only for the generated agent and detection jobs", jobName)
}
return builtinJobAugmentation{needs, ifCondition, hasPermissions, hasTimeout}, configMap, nil
if hasContinueOnError && targetJobName != string(constants.AgentJobName) {
return builtinJobAugmentation{}, nil, fmt.Errorf("jobs.%s.continue-on-error is supported only for the generated agent job", jobName)
}
return builtinJobAugmentation{
needs: needs,
ifCondition: ifCondition,
hasPermissions: hasPermissions,
hasTimeout: hasTimeout,
hasContinueOnError: hasContinueOnError,
}, configMap, nil
}

func (c *Compiler) applyBuiltinJobAugmentation(jobName string, rawConfig any, data *WorkflowData, allJobs map[string]*Job) error {
Expand All @@ -246,7 +257,7 @@ func (c *Compiler) applyBuiltinJobAugmentation(jobName string, rawConfig any, da
if err != nil {
return err
}
if len(augmentation.needs) == 0 && augmentation.ifCondition == "" && !augmentation.hasPermissions && !augmentation.hasTimeout {
if len(augmentation.needs) == 0 && augmentation.ifCondition == "" && !augmentation.hasPermissions && !augmentation.hasTimeout && !augmentation.hasContinueOnError {
return nil
}
targetJob, exists := c.jobManager.GetJob(targetJobName)
Expand All @@ -263,11 +274,14 @@ func (c *Compiler) applyBuiltinJobAugmentation(jobName string, rawConfig any, da
return err
}
}
if augmentation.hasContinueOnError {
extractCustomJobContinueOnError(targetJob, configMap)
}
return c.applyBuiltinJobNeedsAndIf(jobName, targetJobName, targetJob, data.Jobs, allJobs, augmentation)
}

func augmentedBuiltinJobField(jobName, targetJobName string, augmentation builtinJobAugmentation) string {
if len(augmentation.needs) > 0 && (augmentation.ifCondition != "" || augmentation.hasPermissions || augmentation.hasTimeout) {
if len(augmentation.needs) > 0 && (augmentation.ifCondition != "" || augmentation.hasPermissions || augmentation.hasTimeout || augmentation.hasContinueOnError) {
return jobName
}
if len(augmentation.needs) > 0 {
Expand All @@ -279,6 +293,9 @@ func augmentedBuiltinJobField(jobName, targetJobName string, augmentation builti
if augmentation.hasTimeout {
return jobName + ".timeout-minutes"
}
if augmentation.hasContinueOnError {
return jobName + ".continue-on-error"
}
return targetJobName + ".permissions"
}

Expand Down
11 changes: 11 additions & 0 deletions pkg/workflow/workflow_import_merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,17 @@ func mergeJobInjectedSteps(jobName string, mainJob any, importedJob any) (map[st
mergedAny = true
}

// continue-on-error is a scalar built-in job augmentation: the main workflow's value
// always wins when present, but an imported value should still apply when the main
// workflow leaves the field unset entirely (rather than being silently dropped just
// because the job is also declared in the main workflow for other fields, e.g. needs).
if _, hasMain := mainMap["continue-on-error"]; !hasMain {
if importedValue, hasImported := importedMap["continue-on-error"]; hasImported {
merged["continue-on-error"] = importedValue
mergedAny = true
}
}

return merged, mergedAny
}

Expand Down
Loading