Skip to content

Commit 722028d

Browse files
yohamta0yohamta0
authored
feat(spec): add continue-on shorthand (#1454)
* **New Features** * continueOn now accepts a shorthand string ("skipped" or "failed") or a detailed object, with support for exitCode and output as single values or arrays and a markSuccess flag. * **Bug Fixes** * Stronger, field-specific validation and clearer error messages for continueOn inputs, including case-insensitive string parsing. * **Tests** * Added coverage for shorthand strings, object forms, arrays, and invalid continueOn values. * **Documentation** * Schema updated to document the string-or-object union and expanded field options.
1 parent 07929d9 commit 722028d

5 files changed

Lines changed: 224 additions & 68 deletions

File tree

‎internal/core/spec/builder.go‎

Lines changed: 49 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,21 +1304,58 @@ func buildContinueOn(_ StepBuildContext, def stepDef, step *core.Step) error {
13041304
if def.ContinueOn == nil {
13051305
return nil
13061306
}
1307-
step.ContinueOn.Skipped = def.ContinueOn.Skipped
1308-
step.ContinueOn.Failure = def.ContinueOn.Failure
1309-
step.ContinueOn.MarkSuccess = def.ContinueOn.MarkSuccess
13101307

1311-
exitCodes, err := parseIntOrArray(def.ContinueOn.ExitCode)
1312-
if err != nil {
1313-
return core.NewValidationError("continueOn.exitCode", def.ContinueOn.ExitCode, ErrContinueOnExitCodeMustBeIntOrArray)
1314-
}
1315-
step.ContinueOn.ExitCode = exitCodes
1308+
switch v := def.ContinueOn.(type) {
1309+
case string:
1310+
// Shorthand syntax: "skipped" or "failed"
1311+
switch strings.ToLower(strings.TrimSpace(v)) {
1312+
case "skipped":
1313+
step.ContinueOn.Skipped = true
1314+
case "failed":
1315+
step.ContinueOn.Failure = true
1316+
default:
1317+
return core.NewValidationError("continueOn", v, ErrContinueOnInvalidStringValue)
1318+
}
13161319

1317-
output, err := parseStringOrArray(def.ContinueOn.Output)
1318-
if err != nil {
1319-
return core.NewValidationError("continueOn.stdout", def.ContinueOn.Output, ErrContinueOnOutputMustBeStringOrArray)
1320+
case map[string]any:
1321+
// Object syntax with detailed configuration
1322+
if val, exists := v["failure"]; exists {
1323+
b, ok := val.(bool)
1324+
if !ok {
1325+
return core.NewValidationError("continueOn.failure", val, ErrContinueOnFieldMustBeBool)
1326+
}
1327+
step.ContinueOn.Failure = b
1328+
}
1329+
if val, exists := v["skipped"]; exists {
1330+
b, ok := val.(bool)
1331+
if !ok {
1332+
return core.NewValidationError("continueOn.skipped", val, ErrContinueOnFieldMustBeBool)
1333+
}
1334+
step.ContinueOn.Skipped = b
1335+
}
1336+
if val, exists := v["markSuccess"]; exists {
1337+
b, ok := val.(bool)
1338+
if !ok {
1339+
return core.NewValidationError("continueOn.markSuccess", val, ErrContinueOnFieldMustBeBool)
1340+
}
1341+
step.ContinueOn.MarkSuccess = b
1342+
}
1343+
1344+
exitCodes, err := parseIntOrArray(v["exitCode"])
1345+
if err != nil {
1346+
return core.NewValidationError("continueOn.exitCode", v["exitCode"], ErrContinueOnExitCodeMustBeIntOrArray)
1347+
}
1348+
step.ContinueOn.ExitCode = exitCodes
1349+
1350+
output, err := parseStringOrArray(v["output"])
1351+
if err != nil {
1352+
return core.NewValidationError("continueOn.output", v["output"], ErrContinueOnOutputMustBeStringOrArray)
1353+
}
1354+
step.ContinueOn.Output = output
1355+
1356+
default:
1357+
return core.NewValidationError("continueOn", def.ContinueOn, ErrContinueOnMustBeStringOrMap)
13201358
}
1321-
step.ContinueOn.Output = output
13221359

13231360
return nil
13241361
}

‎internal/core/spec/builder_test.go‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1278,6 +1278,121 @@ steps:
12781278
assert.True(t, th.Steps[0].ContinueOn.Failure)
12791279
assert.True(t, th.Steps[0].ContinueOn.Skipped)
12801280
})
1281+
t.Run("ContinueOnStringSkipped", func(t *testing.T) {
1282+
t.Parallel()
1283+
1284+
data := []byte(`
1285+
steps:
1286+
- command: "echo 1"
1287+
continueOn: skipped
1288+
`)
1289+
dag, err := spec.LoadYAML(context.Background(), data)
1290+
require.NoError(t, err)
1291+
th := DAG{t: t, DAG: dag}
1292+
assert.Len(t, th.Steps, 1)
1293+
assert.True(t, th.Steps[0].ContinueOn.Skipped)
1294+
assert.False(t, th.Steps[0].ContinueOn.Failure)
1295+
})
1296+
t.Run("ContinueOnStringFailed", func(t *testing.T) {
1297+
t.Parallel()
1298+
1299+
data := []byte(`
1300+
steps:
1301+
- command: "echo 1"
1302+
continueOn: failed
1303+
`)
1304+
dag, err := spec.LoadYAML(context.Background(), data)
1305+
require.NoError(t, err)
1306+
th := DAG{t: t, DAG: dag}
1307+
assert.Len(t, th.Steps, 1)
1308+
assert.False(t, th.Steps[0].ContinueOn.Skipped)
1309+
assert.True(t, th.Steps[0].ContinueOn.Failure)
1310+
})
1311+
t.Run("ContinueOnStringCaseInsensitive", func(t *testing.T) {
1312+
t.Parallel()
1313+
1314+
data := []byte(`
1315+
steps:
1316+
- command: "echo 1"
1317+
continueOn: SKIPPED
1318+
`)
1319+
dag, err := spec.LoadYAML(context.Background(), data)
1320+
require.NoError(t, err)
1321+
th := DAG{t: t, DAG: dag}
1322+
assert.Len(t, th.Steps, 1)
1323+
assert.True(t, th.Steps[0].ContinueOn.Skipped)
1324+
})
1325+
t.Run("ContinueOnInvalidString", func(t *testing.T) {
1326+
t.Parallel()
1327+
1328+
data := []byte(`
1329+
steps:
1330+
- command: "echo 1"
1331+
continueOn: invalid
1332+
`)
1333+
_, err := spec.LoadYAML(context.Background(), data)
1334+
require.Error(t, err)
1335+
assert.Contains(t, err.Error(), "continueOn")
1336+
})
1337+
t.Run("ContinueOnObjectWithExitCode", func(t *testing.T) {
1338+
t.Parallel()
1339+
1340+
data := []byte(`
1341+
steps:
1342+
- command: "echo 1"
1343+
continueOn:
1344+
exitCode: [1, 2, 3]
1345+
markSuccess: true
1346+
`)
1347+
dag, err := spec.LoadYAML(context.Background(), data)
1348+
require.NoError(t, err)
1349+
th := DAG{t: t, DAG: dag}
1350+
assert.Len(t, th.Steps, 1)
1351+
assert.Equal(t, []int{1, 2, 3}, th.Steps[0].ContinueOn.ExitCode)
1352+
assert.True(t, th.Steps[0].ContinueOn.MarkSuccess)
1353+
})
1354+
t.Run("ContinueOnInvalidFailureType", func(t *testing.T) {
1355+
t.Parallel()
1356+
1357+
data := []byte(`
1358+
steps:
1359+
- command: "echo 1"
1360+
continueOn:
1361+
failure: "true"
1362+
`)
1363+
_, err := spec.LoadYAML(context.Background(), data)
1364+
require.Error(t, err)
1365+
assert.Contains(t, err.Error(), "continueOn.failure")
1366+
assert.Contains(t, err.Error(), "boolean")
1367+
})
1368+
t.Run("ContinueOnInvalidSkippedType", func(t *testing.T) {
1369+
t.Parallel()
1370+
1371+
data := []byte(`
1372+
steps:
1373+
- command: "echo 1"
1374+
continueOn:
1375+
skipped: 1
1376+
`)
1377+
_, err := spec.LoadYAML(context.Background(), data)
1378+
require.Error(t, err)
1379+
assert.Contains(t, err.Error(), "continueOn.skipped")
1380+
assert.Contains(t, err.Error(), "boolean")
1381+
})
1382+
t.Run("ContinueOnInvalidMarkSuccessType", func(t *testing.T) {
1383+
t.Parallel()
1384+
1385+
data := []byte(`
1386+
steps:
1387+
- command: "echo 1"
1388+
continueOn:
1389+
markSuccess: "yes"
1390+
`)
1391+
_, err := spec.LoadYAML(context.Background(), data)
1392+
require.Error(t, err)
1393+
assert.Contains(t, err.Error(), "continueOn.markSuccess")
1394+
assert.Contains(t, err.Error(), "boolean")
1395+
})
12811396
t.Run("RetryPolicy", func(t *testing.T) {
12821397
t.Parallel()
12831398

‎internal/core/spec/definition.go‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,8 @@ type stepDef struct {
132132
// Depends is the list of steps to depend on.
133133
Depends any `yaml:"depends,omitempty"` // string or []string
134134
// ContinueOn is the condition to continue on.
135-
ContinueOn *continueOnDef `yaml:"continueOn,omitempty"`
135+
// Can be a string ("skipped", "failed") or an object with detailed config.
136+
ContinueOn any `yaml:"continueOn,omitempty"`
136137
// RetryPolicy is the retry policy.
137138
RetryPolicy *retryPolicyDef `yaml:"retryPolicy,omitempty"`
138139
// RepeatPolicy is the repeat policy.
@@ -168,15 +169,6 @@ type stepDef struct {
168169
TimeoutSec int `yaml:"timeoutSec,omitempty"`
169170
}
170171

171-
// continueOnDef defines the conditions to continue on failure or skipped.
172-
type continueOnDef struct {
173-
Failure bool // Continue on failure
174-
Skipped bool // Continue on skipped
175-
ExitCode any // Continue on specific exit codes
176-
Output any // Continue on specific output (string or []string)
177-
MarkSuccess bool // Mark the step as success when the condition is met
178-
}
179-
180172
// repeatPolicyDef defines the repeat policy for a step.
181173
type repeatPolicyDef struct {
182174
Repeat any `yaml:"repeat,omitempty"` // Flag to indicate if the step should be repeated, can be bool (legacy) or string ("while" or "until")

‎internal/core/spec/errors.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,11 @@ var (
1313
ErrPreconditionMustBeArrayOrString = errors.New("precondition must be a string or an array of strings")
1414
ErrInvalidStepData = errors.New("invalid step data")
1515
ErrStepsMustBeArrayOrMap = errors.New("steps must be an array or a map")
16-
ErrContinueOnExitCodeMustBeIntOrArray = errors.New("continueOn.ExitCode must be an int or an array of ints")
17-
ErrContinueOnOutputMustBeStringOrArray = errors.New("continueOn.Output must be a string or an array of strings")
16+
ErrContinueOnExitCodeMustBeIntOrArray = errors.New("continueOn.exitCode must be an int or an array of ints")
17+
ErrContinueOnOutputMustBeStringOrArray = errors.New("continueOn.output must be a string or an array of strings")
18+
ErrContinueOnMustBeStringOrMap = errors.New("continueOn must be a string ('skipped' or 'failed') or an object")
19+
ErrContinueOnInvalidStringValue = errors.New("continueOn string value must be 'skipped' or 'failed'")
20+
ErrContinueOnFieldMustBeBool = errors.New("value must be a boolean")
1821
ErrInvalidSignal = errors.New("invalid signal")
1922
ErrDependsMustBeStringOrArray = errors.New("depends must be a string or an array of strings")
2023
ErrExecutorTypeMustBeString = errors.New("executor.type value must be string")

‎schemas/dag.schema.json‎

Lines changed: 53 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -654,56 +654,65 @@
654654
]
655655
},
656656
"continueOn": {
657-
"type": "object",
658-
"properties": {
659-
"failure": {
660-
"type": "boolean",
661-
"description": "Continue dag-run even if this step fails"
662-
},
663-
"skipped": {
664-
"type": "boolean",
665-
"description": "Continue dag-run even if this step is skipped due to preconditions"
657+
"oneOf": [
658+
{
659+
"type": "string",
660+
"enum": ["skipped", "failed"],
661+
"description": "Shorthand: 'skipped' to continue if step is skipped, 'failed' to continue if step fails"
666662
},
667-
"exitCode": {
668-
"oneOf": [
669-
{
670-
"type": "integer",
671-
"description": "Exit code that should be treated as successful"
663+
{
664+
"type": "object",
665+
"properties": {
666+
"failure": {
667+
"type": "boolean",
668+
"description": "Continue dag-run even if this step fails"
672669
},
673-
{
674-
"type": "string",
675-
"description": "Exit code represented as a string (parsed as integer at runtime)"
670+
"skipped": {
671+
"type": "boolean",
672+
"description": "Continue dag-run even if this step is skipped due to preconditions"
676673
},
677-
{
678-
"type": "array",
679-
"items": {
680-
"type": "integer"
681-
},
682-
"description": "List of exit codes that should be treated as successful"
683-
}
684-
]
685-
},
686-
"output": {
687-
"oneOf": [
688-
{
689-
"type": "string",
690-
"description": "Output text or pattern that indicates success. Supports regex with 're:' prefix."
674+
"exitCode": {
675+
"oneOf": [
676+
{
677+
"type": "integer",
678+
"description": "Exit code that should be treated as successful"
679+
},
680+
{
681+
"type": "string",
682+
"description": "Exit code represented as a string (parsed as integer at runtime)"
683+
},
684+
{
685+
"type": "array",
686+
"items": {
687+
"type": "integer"
688+
},
689+
"description": "List of exit codes that should be treated as successful"
690+
}
691+
]
691692
},
692-
{
693-
"type": "array",
694-
"items": {
695-
"type": "string",
696-
"description": "Output text or patterns that indicate success. Supports regex with 're:' prefix."
697-
}
693+
"output": {
694+
"oneOf": [
695+
{
696+
"type": "string",
697+
"description": "Output text or pattern that indicates success. Supports regex with 're:' prefix."
698+
},
699+
{
700+
"type": "array",
701+
"items": {
702+
"type": "string",
703+
"description": "Output text or patterns that indicate success. Supports regex with 're:' prefix."
704+
}
705+
}
706+
]
707+
},
708+
"markSuccess": {
709+
"type": "boolean",
710+
"description": "Mark the step as successful even if it technically failed but met continue conditions"
698711
}
699-
]
700-
},
701-
"markSuccess": {
702-
"type": "boolean",
703-
"description": "Mark the step as successful even if it technically failed but met continue conditions"
712+
}
704713
}
705-
},
706-
"description": "Conditions under which the DAG should continue executing even if this step fails or is skipped."
714+
],
715+
"description": "Conditions under which the DAG should continue executing even if this step fails or is skipped. Can be a string ('skipped' or 'failed') or an object with detailed configuration."
707716
},
708717
"retryPolicy": {
709718
"type": "object",

0 commit comments

Comments
 (0)