Skip to content

Improve 100 validation error messages #52137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 2 commits into from
Closed

Improve 100 validation error messages #52137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
85525b1
Improve 100 validation error messages
Copilot Aug 11, 2026
c059938
Merge branch 'main' into copilot/fix-100-error-messages
github-actions[bot] Aug 11, 2026
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
44 changes: 22 additions & 22 deletions pkg/workflow/checkout_config_parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func ParseCheckoutConfigs(raw any) ([]*CheckoutConfig, error) {
for i, item := range arr {
itemMap, ok := item.(map[string]any)
if !ok {
return nil, fmt.Errorf("checkout[%d]: expected object, got %T", i, item)
return nil, fmt.Errorf("checkout[%d]: expected object, got %T. Example: checkout: [{repository: github/gh-aw}]", i, item)
}
cfg, err := checkoutConfigFromMap(itemMap)
if err != nil {
Expand All @@ -44,7 +44,7 @@ func ParseCheckoutConfigs(raw any) ([]*CheckoutConfig, error) {
configs = append(configs, cfg)
}
} else {
return nil, fmt.Errorf("checkout must be an object or an array of objects, got %T", raw)
return nil, fmt.Errorf("checkout must be an object or an array of objects, got %T. Example: checkout: {repository: github/gh-aw}", raw)
}

// Validate that at most one logical checkout target has current: true.
Expand Down Expand Up @@ -83,23 +83,23 @@ func checkoutConfigFromMap(m map[string]any) (*CheckoutConfig, error) {
if v, ok := m["repository"]; ok {
s, ok := v.(string)
if !ok {
return nil, errors.New("checkout.repository must be a string")
return nil, errors.New("checkout.repository must be a string. Example: checkout: {repository: github/gh-aw}")
}
cfg.Repository = s
}

if v, ok := m["ref"]; ok {
s, ok := v.(string)
if !ok {
return nil, errors.New("checkout.ref must be a string")
return nil, errors.New("checkout.ref must be a string. Example: checkout: {ref: main}")
}
cfg.Ref = s
}

if v, ok := m["path"]; ok {
s, ok := v.(string)
if !ok {
return nil, errors.New("checkout.path must be a string")
return nil, errors.New("checkout.path must be a string. Example: checkout: {path: source}")
}
cfg.PathExplicit = true
// Normalize "." to empty string: both mean the workspace root and
Expand All @@ -114,14 +114,14 @@ func checkoutConfigFromMap(m map[string]any) (*CheckoutConfig, error) {
if v, ok := m["github-token"]; ok {
s, ok := v.(string)
if !ok {
return nil, errors.New("checkout.github-token must be a string")
return nil, errors.New("checkout.github-token must be a string. Example: checkout: {github-token: '${{ secrets.GITHUB_TOKEN }}'}")
}
cfg.GitHubToken = s
} else if v, ok := m["token"]; ok {
// Backward compatibility: "token" is accepted but "github-token" is preferred
s, ok := v.(string)
if !ok {
return nil, errors.New("checkout.token must be a string")
return nil, errors.New("checkout.token must be a string. Example: checkout: {token: '${{ secrets.CHECKOUT_TOKEN }}'}")
}
cfg.GitHubToken = s
}
Expand All @@ -130,7 +130,7 @@ func checkoutConfigFromMap(m map[string]any) (*CheckoutConfig, error) {
if v, ok := m["github-app"]; ok {
appMap, ok := v.(map[string]any)
if !ok {
return nil, errors.New("checkout.github-app must be an object")
return nil, errors.New("checkout.github-app must be an object. Example: checkout: {github-app: {app-id: 123, private-key: '${{ secrets.APP_PRIVATE_KEY }}'}}")
}
cfg.GitHubApp = parseAppConfig(appMap)
if cfg.GitHubApp.AppID == "" || cfg.GitHubApp.PrivateKey == "" {
Expand All @@ -142,7 +142,7 @@ func checkoutConfigFromMap(m map[string]any) (*CheckoutConfig, error) {
parseSafeOutputAppConfig := func(fieldName string, value any) (*GitHubAppConfig, error) {
appMap, ok := value.(map[string]any)
if !ok {
return nil, fmt.Errorf("checkout.%s must be an object", fieldName)
return nil, fmt.Errorf("checkout.%s must be an object. Example: checkout: {%s: {owner: github}}", fieldName, fieldName)

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example for safe-outputs-github-app (and any safe-output-* app fields) is incorrect — it shows {owner: github} but this field is a GitHub App config that requires app-id/client-id and private-key. The next error message even validates that both are present.

Suggested fix:

checkout: {safe-outputs-github-app: {app-id: 123, private-key: '${{ secrets.APP_PRIVATE_KEY }}'}}

@copilot please address this.

}
appConfig := parseAppConfig(appMap)
if appConfig.AppID == "" || appConfig.PrivateKey == "" {
Expand Down Expand Up @@ -183,22 +183,22 @@ func checkoutConfigFromMap(m map[string]any) (*CheckoutConfig, error) {
cfg.FetchDepth = &depth
case float64:
if n != float64(int64(n)) {
return nil, errors.New("checkout.fetch-depth must be an integer")
return nil, errors.New("checkout.fetch-depth must be an integer. Example: checkout: {fetch-depth: 1}")
}
depth := int(n)
cfg.FetchDepth = &depth
default:
return nil, errors.New("checkout.fetch-depth must be an integer")
return nil, errors.New("checkout.fetch-depth must be an integer. Example: checkout: {fetch-depth: 1}")
}
if cfg.FetchDepth != nil && *cfg.FetchDepth < 0 {
return nil, errors.New("checkout.fetch-depth must be >= 0")
return nil, errors.New("checkout.fetch-depth must be at least 0. Example: checkout: {fetch-depth: 0}")
}
}

if v, ok := m["sparse-checkout"]; ok {
s, ok := v.(string)
if !ok {
return nil, errors.New("checkout.sparse-checkout must be a string")
return nil, errors.New("checkout.sparse-checkout must be a string. Example: checkout: {sparse-checkout: src}")
}
cfg.SparseCheckout = s
}
Expand All @@ -214,22 +214,22 @@ func checkoutConfigFromMap(m map[string]any) (*CheckoutConfig, error) {
cfg.Submodules = "false"
}
default:
return nil, errors.New("checkout.submodules must be a string or boolean")
return nil, errors.New("checkout.submodules must be a string or boolean. Example: checkout: {submodules: recursive}")
}
}

if v, ok := m["lfs"]; ok {
b, ok := v.(bool)
if !ok {
return nil, errors.New("checkout.lfs must be a boolean")
return nil, errors.New("checkout.lfs must be a boolean. Example: checkout: {lfs: true}")
}
cfg.LFS = b
}

if v, ok := m["current"]; ok {
b, ok := v.(bool)
if !ok {
return nil, errors.New("checkout.current must be a boolean")
return nil, errors.New("checkout.current must be a boolean. Example: checkout: {current: true}")
}
cfg.Current = b
}
Expand All @@ -239,39 +239,39 @@ func checkoutConfigFromMap(m map[string]any) (*CheckoutConfig, error) {
case string:
// Single string shorthand: treat as a one-element list
if strings.TrimSpace(fv) == "" {
return nil, errors.New("checkout.fetch string value must not be empty")
return nil, errors.New("checkout.fetch string value must not be empty. Example: checkout: {fetch: main}")
}
cfg.Fetch = []string{fv}
case []any:
refs := make([]string, 0, len(fv))
for i, item := range fv {
s, ok := item.(string)
if !ok {
return nil, fmt.Errorf("checkout.fetch[%d] must be a string, got %T", i, item)
return nil, fmt.Errorf("checkout.fetch[%d] must be a string, got %T. Example: checkout: {fetch: [main]}", i, item)
}
if strings.TrimSpace(s) == "" {
return nil, fmt.Errorf("checkout.fetch[%d] must not be empty", i)
return nil, fmt.Errorf("checkout.fetch[%d] must not be empty. Example: checkout: {fetch: [main]}", i)
}
refs = append(refs, s)
}
cfg.Fetch = refs
default:
return nil, errors.New("checkout.fetch must be a string or an array of strings")
return nil, errors.New("checkout.fetch must be a string or an array of strings. Example: checkout: {fetch: [main, release]}")
}
}

if v, ok := m["wiki"]; ok {
b, ok := v.(bool)
if !ok {
return nil, errors.New("checkout.wiki must be a boolean")
return nil, errors.New("checkout.wiki must be a boolean. Example: checkout: {wiki: true}")
}
cfg.Wiki = b
}

if v, ok := m["force-clean-git-credentials"]; ok {
b, ok := v.(bool)
if !ok {
return nil, errors.New("checkout.force-clean-git-credentials must be a boolean")
return nil, errors.New("checkout.force-clean-git-credentials must be a boolean. Example: checkout: {force-clean-git-credentials: true}")
}
cfg.CleanGitCredentials = b
}
Expand Down
16 changes: 8 additions & 8 deletions pkg/workflow/compiler_custom_jobs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@ func extractBuiltinJobNeedsAugmentation(jobName string, configMap map[string]any
}
return needs, nil
default:
return nil, fmt.Errorf("jobs.%s.needs must be a string or array of strings, got %T", jobName, needsValue)
return nil, fmt.Errorf("jobs.%s.needs must be a string or array of strings, got %T. Example: jobs: {%s: {needs: agent}}", jobName, needsValue, jobName)
}
}

Expand All @@ -734,7 +734,7 @@ func extractBuiltinJobIfAugmentation(jobName string, configMap map[string]any) (

ifCondition, ok := ifValue.(string)
if !ok {
return "", fmt.Errorf("jobs.%s.if must be a string, got %T", jobName, ifValue)
return "", fmt.Errorf("jobs.%s.if must be a string, got %T. Example: jobs: {%s: {if: '${{ success() }}'}}", jobName, ifValue, jobName)
}

// Strip "if: " prefix to match the Job.If contract (bare expression, no prefix).
Expand Down Expand Up @@ -764,7 +764,7 @@ func (c *Compiler) applyBuiltinJobAugmentations(data *WorkflowData) error {

configMap, ok := rawConfig.(map[string]any)
if !ok {
return fmt.Errorf("jobs.%s must be an object, got %T", configuredJobName, rawConfig)
return fmt.Errorf("jobs.%s must be an object, got %T. Example: jobs: {%s: {steps: []}}", configuredJobName, rawConfig, configuredJobName)
}

augmentedNeeds, err := extractBuiltinJobNeedsAugmentation(configuredJobName, configMap)
Expand Down Expand Up @@ -793,7 +793,7 @@ func (c *Compiler) applyBuiltinJobAugmentations(data *WorkflowData) error {
} else if augmentedIf != "" || hasPermissions {
augmentedField = configuredJobName
}
return fmt.Errorf("jobs.%s: cannot augment %q because this workflow does not generate that job", augmentedField, targetJobName)
return fmt.Errorf("jobs.%s: cannot augment %q because this workflow does not generate that job. Example: jobs: {%s: {steps: []}}", augmentedField, targetJobName, configuredJobName)
}

if hasPermissions {
Expand All @@ -806,10 +806,10 @@ func (c *Compiler) applyBuiltinJobAugmentations(data *WorkflowData) error {
for _, rawNeed := range augmentedNeeds {
need := normalizeBuiltinJobAlias(rawNeed)
if need == targetJobName {
return fmt.Errorf("jobs.%s.needs: %q cannot depend on itself", configuredJobName, rawNeed)
return fmt.Errorf("jobs.%s.needs: %q cannot depend on itself. Example: jobs: {%s: {needs: agent}}", configuredJobName, rawNeed, configuredJobName)
}
if _, known := allJobs[need]; !known {
return fmt.Errorf("jobs.%s.needs: unknown job %q", configuredJobName, rawNeed)
return fmt.Errorf("jobs.%s.needs: unknown job %q. Example: jobs: {%s: {needs: agent}}", configuredJobName, rawNeed, configuredJobName)
}
normalizedNeeds = append(normalizedNeeds, need)
}
Expand Down Expand Up @@ -1073,14 +1073,14 @@ func (c *Compiler) extractPinnedJobSteps(fieldName string, jobName string, confi

stepsList, ok := raw.([]any)
if !ok {
return nil, fmt.Errorf("%s for job '%s' must be an array of step objects", fieldName, jobName)
return nil, fmt.Errorf("%s for job '%s' must be an array of step objects. Example: jobs: {%s: {%s: [{run: echo ready}]}}", fieldName, jobName, jobName, fieldName)
}

pinnedSteps := make([]string, 0, len(stepsList))
for i, step := range stepsList {
stepMap, ok := step.(map[string]any)
if !ok {
return nil, fmt.Errorf("%s for job '%s' contains invalid step at index %d: expected object", fieldName, jobName, i)
return nil, fmt.Errorf("%s for job '%s' contains invalid step at index %d: expected object. Example: jobs: {%s: {%s: [{run: echo ready}]}}", fieldName, jobName, i, jobName, fieldName)
}

typedStep, err := MapToStep(stepMap)
Expand Down
24 changes: 12 additions & 12 deletions pkg/workflow/compiler_pre_activation_job.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func (c *Compiler) buildPreActivationJob(data *WorkflowData, needsPermissionChec

setupActionRef := c.resolveActionReference("./actions/setup", data)
if setupActionRef == "" {
return nil, errors.New("setup action reference is required but could not be resolved")
return nil, errors.New("setup action reference is required but could not be resolved. Example: actions/setup-node@v4")

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error is triggered when the compiler's internal resolveActionReference("./actions/setup", data) returns empty — it's an internal resolution failure, not a user-configurable field. The appended example actions/setup-node@v4 is misleading since users don't configure setup action references directly in workflow frontmatter.

Consider removing the example entirely or replacing it with actionable guidance, e.g.:

setup action reference is required but could not be resolved; ensure your workflow specifies a valid actions version in its configuration

@copilot please address this.

}

steps, permissions := c.buildPreActivationPermissions(data, setupActionRef)
Expand Down Expand Up @@ -674,7 +674,7 @@ func validatePreActivationJobConfig(jobs map[string]any, jobName string) (map[st

configMap, ok := preActivationJob.(map[string]any)
if !ok {
return nil, fmt.Errorf("jobs.%s must be an object, got %T", jobName, preActivationJob)
return nil, fmt.Errorf("jobs.%s must be an object, got %T. Example: jobs: {%s: {steps: []}}", jobName, preActivationJob, jobName)
}

allowedFields := map[string]struct{}{
Expand All @@ -690,7 +690,7 @@ func validatePreActivationJobConfig(jobs map[string]any, jobName string) (map[st
)
}
if !setutil.Contains(allowedFields, field) {
return nil, fmt.Errorf("jobs.%s: unsupported field '%s' - only 'steps', 'outputs', and 'pre-steps' are allowed", jobName, field)
return nil, fmt.Errorf("jobs.%s: unsupported field '%s' - only 'steps', 'outputs', and 'pre-steps' are allowed. Example: jobs: {%s: {steps: []}}", jobName, field, jobName)
}
}
return configMap, nil
Expand All @@ -705,14 +705,14 @@ func extractPreActivationJobSteps(jobName string, configMap map[string]any) ([]s

stepsList, ok := stepsValue.([]any)
if !ok {
return nil, fmt.Errorf("jobs.%s.steps must be an array, got %T", jobName, stepsValue)
return nil, fmt.Errorf("jobs.%s.steps must be an array, got %T. Example: jobs: {%s: {steps: [{run: echo ready}]}}", jobName, stepsValue, jobName)
}

var steps []string
for i, step := range stepsList {
stepMap, ok := step.(map[string]any)
if !ok {
return nil, fmt.Errorf("jobs.%s.steps[%d] must be an object, got %T", jobName, i, step)
return nil, fmt.Errorf("jobs.%s.steps[%d] must be an object, got %T. Example: jobs: {%s: {steps: [{run: echo ready}]}}", jobName, i, step, jobName)
}
stepYAML, err := ConvertStepToYAML(stepMap)
if err != nil {
Expand All @@ -733,15 +733,15 @@ func extractPreActivationJobOutputs(jobName string, configMap map[string]any) (m

outputsMap, ok := outputsValue.(map[string]any)
if !ok {
return nil, fmt.Errorf("jobs.%s.outputs must be an object, got %T", jobName, outputsValue)
return nil, fmt.Errorf("jobs.%s.outputs must be an object, got %T. Example: jobs: {%s: {outputs: {ready: '${{ steps.check.outputs.ready }}'}}}", jobName, outputsValue, jobName)
}

// If the same output key is defined in both variants, the second one (pre_activation) wins.
result := make(map[string]string, len(outputsMap))
for key, val := range outputsMap {
valStr, ok := val.(string)
if !ok {
return nil, fmt.Errorf("jobs.%s.outputs.%s must be a string, got %T", jobName, key, val)
return nil, fmt.Errorf("jobs.%s.outputs.%s must be a string, got %T. Example: jobs: {%s: {outputs: {%s: '${{ steps.check.outputs.value }}'}}}", jobName, key, val, jobName, key)
}
result[key] = valStr
}
Expand Down Expand Up @@ -839,14 +839,14 @@ func extractOnSteps(frontmatter map[string]any) ([]map[string]any, error) {

stepsList, ok := stepsValue.([]any)
if !ok {
return nil, fmt.Errorf("on.steps must be an array, got %T", stepsValue)
return nil, fmt.Errorf("on.steps must be an array, got %T. Example: on: {steps: [{run: echo ready}]}", stepsValue)
}

result := make([]map[string]any, 0, len(stepsList))
for i, step := range stepsList {
stepMap, ok := step.(map[string]any)
if !ok {
return nil, fmt.Errorf("on.steps[%d] must be an object, got %T", i, step)
return nil, fmt.Errorf("on.steps[%d] must be an object, got %T. Example: on: {steps: [{run: echo ready}]}", i, step)
}
result = append(result, stepMap)
}
Expand Down Expand Up @@ -917,7 +917,7 @@ func extractOnRestoreMemory(frontmatter map[string]any) (bool, error) {

restoreMemory, ok := restoreMemoryValue.(bool)
if !ok {
return false, fmt.Errorf("on.restore-memory must be a boolean, got %T", restoreMemoryValue)
return false, fmt.Errorf("on.restore-memory must be a boolean, got %T. Example: on: {restore-memory: true}", restoreMemoryValue)
}

return restoreMemory, nil
Expand All @@ -935,14 +935,14 @@ func parseOnNeedsValues(onMap map[string]any) ([]string, error) {

needsList, ok := needsValue.([]any)
if !ok {
return nil, fmt.Errorf("on.needs must be an array, got %T", needsValue)
return nil, fmt.Errorf("on.needs must be an array, got %T. Example: on: {needs: [build]}", needsValue)
}

result := make([]string, 0, len(needsList))
for i, need := range needsList {
needStr, ok := need.(string)
if !ok {
return nil, fmt.Errorf("on.needs[%d] must be a string, got %T", i, need)
return nil, fmt.Errorf("on.needs[%d] must be a string, got %T. Example: on: {needs: [build]}", i, need)
}
result = append(result, needStr)
}
Expand Down
Loading
Loading