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
29 changes: 29 additions & 0 deletions pkg/parser/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2703,3 +2703,32 @@ func TestValidateMainWorkflowFrontmatter_OnPermissionsUnknownScopeRejected(t *te
t.Error("unknown scope in on.permissions should be rejected by schema validation")
}
}

func TestValidateMainWorkflowFrontmatter_JobsInputsRejectedBeforeSchema(t *testing.T) {
frontmatter := map[string]any{
"on": "push",
"engine": "copilot",
"jobs": map[string]any{
"my-job": map[string]any{
"runs-on": "ubuntu-latest",
"inputs": map[string]any{
"name": map[string]any{"description": "test"},
},
"steps": []any{map[string]any{"run": "echo hi"}},
},
},
}

err := ValidateMainWorkflowFrontmatterWithSchemaAndLocation(frontmatter, "/tmp/gh-aw/jobs-inputs-pre-schema-test.md")
if err == nil {
t.Fatal("expected jobs.<name>.inputs validation error")
}

errMsg := err.Error()
if !strings.Contains(errMsg, "jobs.my-job.inputs: inputs are not supported on jobs") {
t.Fatalf("expected actionable jobs.inputs error, got: %v", err)
}
if strings.Contains(errMsg, "Unknown property: inputs") {
t.Fatalf("expected pre-schema validation error instead of schema unknown-property error, got: %v", err)
}
}
24 changes: 24 additions & 0 deletions pkg/parser/schema_triggers.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,30 @@ func validateCommandTriggerConflicts(frontmatter map[string]any) error {
return nil
}

func validateUnsupportedJobInputs(frontmatter map[string]any) error {
jobsValue, hasJobs := frontmatter["jobs"]
if !hasJobs || jobsValue == nil {
return nil
}

jobsMap, ok := jobsValue.(map[string]any)
if !ok {
return nil
}

for jobName, jobValue := range jobsMap {
jobMap, ok := jobValue.(map[string]any)
if !ok {
continue
}
if _, hasInputs := jobMap["inputs"]; hasInputs {
return fmt.Errorf("jobs.%s.inputs: inputs are not supported on jobs; use 'env' to pass values to job steps", jobName)
}
}

return nil
}

// IsLabelOnlyEvent checks if an event configuration only contains labeled/unlabeled types
// This is exported for use in the compiler to validate command trigger combinations
func IsLabelOnlyEvent(eventValue any) bool {
Expand Down
3 changes: 3 additions & 0 deletions pkg/parser/schema_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ func ValidateMainWorkflowFrontmatterWithSchemaAndLocation(frontmatter map[string
if err := validateCommandTriggerConflicts(filtered); err != nil {
return err
}
if err := validateUnsupportedJobInputs(filtered); err != nil {
return err
}

// Then run the standard schema validation with location
if err := validateWithSchemaAndLocation(filtered, mainWorkflowSchema, "main workflow file", filePath); err != nil {
Expand Down
4 changes: 4 additions & 0 deletions pkg/workflow/compiler_custom_jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,10 @@ func (c *Compiler) extractCustomJobProperties(job *Job, jobName string, configMa
}

func (c *Compiler) extractCustomJobCoreProperties(job *Job, jobName string, configMap map[string]any) error {
if _, hasInputs := configMap["inputs"]; hasInputs {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Implemented in a3561ac. I added a pre-schema validation in parser validation flow so jobs.<name>.inputs now fails before JSON schema, and added both parser-level and compile-string end-to-end tests to verify the actionable error is returned (not the generic unknown-property message). Kept the compiler-side check as defense-in-depth for direct builder paths that bypass schema.

return fmt.Errorf("jobs.%s.inputs: inputs are not supported on jobs; use 'env' to pass values to job steps", jobName)
Comment on lines +178 to +179

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 check is dead code in the real compile path — schema validation already rejects jobs.<name>.inputs before buildCustomJob is ever called.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in a3561ac by adding pre-schema validation for jobs.<name>.inputs plus end-to-end coverage through ParseWorkflowString, so the normal compile path now emits the actionable jobs.inputs error instead of a generic schema unknown-property message.

}

if err := c.extractCustomJobRunsOn(job, jobName, configMap); err != nil {
return err
}
Expand Down
27 changes: 27 additions & 0 deletions pkg/workflow/compiler_custom_jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,33 @@ func TestBuildCustomJob_InvalidTimeoutMinutesError(t *testing.T) {
require.ErrorContains(t, err, "timeout-minutes")
}

func TestBuildCustomJob_InputsNotSupportedError(t *testing.T) {
compiler := NewCompiler()
compiler.jobManager = NewJobManager()

data := &WorkflowData{Name: "Test"}
configMap := map[string]any{
"runs-on": "ubuntu-latest",
"inputs": map[string]any{
"my-input": map[string]any{"description": "an input"},
},
"steps": []any{map[string]any{"run": "echo hello"}},
}

_, err := compiler.buildCustomJob(
"my-job",
configMap,
data,
false,
map[string]struct{}{},
map[string]struct{}{},
)

require.Error(t, err)
require.ErrorContains(t, err, "jobs.my-job.inputs")
require.ErrorContains(t, err, "inputs are not supported on jobs")
}

func TestBuildCustomJob_UsesReusableWorkflow(t *testing.T) {
compiler := NewCompiler()
compiler.jobManager = NewJobManager()
Expand Down
31 changes: 31 additions & 0 deletions pkg/workflow/compiler_string_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,37 @@ bogus-field: true
assert.Contains(t, errorStr, "virtual/workflow.md:3:1: error:")
}

func TestParseWorkflowString_JobsInputsValidationReportedBeforeSchemaErrors(t *testing.T) {
markdown := `---
name: jobs-inputs-test
on: push
engine: copilot
jobs:
my-job:
runs-on: ubuntu-latest
inputs:
greeting:
description: Greeting
steps:
- run: echo hi
---

# Test
`

compiler := NewCompiler(
WithNoEmit(true),
WithSkipValidation(true),
)

_, err := compiler.ParseWorkflowString(markdown, "virtual/workflow.md")
require.Error(t, err)

errorStr := err.Error()
assert.Contains(t, errorStr, "jobs.my-job.inputs: inputs are not supported on jobs")
assert.NotContains(t, errorStr, "Unknown property: inputs")
}

func TestCompileToYAML_BasicCompilation(t *testing.T) {
markdown := `---
name: compile-test
Expand Down
Loading