Skip to content
Merged
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
242 changes: 242 additions & 0 deletions pkg/parser/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,248 @@ func TestValidateWithSchema_YAMLTypedSlice(t *testing.T) {
}
}

// TestValidateMainWorkflowFrontmatterWithSchemaAndLocation_ProtectedFilesObjectForm
// verifies that the protected-files field on create-pull-request and
// push-to-pull-request-branch accepts the documented object form
// {policy, exclude} in addition to the plain string enum.
//
// This is a regression test for the bug where the schema only accepted
// "string or null" for protected-files, rejecting object-form configurations
// with "expected string or null, got object".
func TestValidateMainWorkflowFrontmatterWithSchemaAndLocation_ProtectedFilesObjectForm(t *testing.T) {
t.Parallel()

baseFrontmatter := func(safeOutputs map[string]any) map[string]any {
return map[string]any{
"on": map[string]any{"issues": map[string]any{"types": []any{"opened"}}},
"engine": "copilot",
"safe-outputs": safeOutputs,
}
}

tests := []struct {
name string
safeOutputs map[string]any
wantErr bool
errContains string
}{
{
name: "create-pull-request: string form passes",
safeOutputs: map[string]any{
"create-pull-request": map[string]any{
"protected-files": "fallback-to-issue",
},
},
wantErr: false,
},
{
name: "create-pull-request: object form with policy and exclude passes",
safeOutputs: map[string]any{
"create-pull-request": map[string]any{
"protected-files": map[string]any{
"policy": "fallback-to-issue",
"exclude": []any{".claude/", ".github/instructions/"},
},
},
},
wantErr: false,
},
{
name: "create-pull-request: object form with only exclude passes",
safeOutputs: map[string]any{
"create-pull-request": map[string]any{
"protected-files": map[string]any{
"exclude": []any{"AGENTS.md"},
},
},
},
wantErr: false,
},
{
name: "create-pull-request: object form with only policy passes",
safeOutputs: map[string]any{
"create-pull-request": map[string]any{
"protected-files": map[string]any{
"policy": "allowed",
},
},
},
wantErr: false,
},
{
name: "create-pull-request: object form with invalid extra field is rejected",
safeOutputs: map[string]any{
"create-pull-request": map[string]any{
"protected-files": map[string]any{
"policy": "blocked",
"unknown-prop": "value",
},
},
},
wantErr: true,
errContains: "unknown-prop",
},
{
name: "push-to-pull-request-branch: object form with policy and exclude passes",
safeOutputs: map[string]any{
"push-to-pull-request-branch": map[string]any{
"protected-files": map[string]any{
"policy": "fallback-to-issue",
"exclude": []any{"AGENTS.md", ".agents/"},
},
},
},
wantErr: false,
},
{
name: "push-to-pull-request-branch: string form passes",
safeOutputs: map[string]any{
"push-to-pull-request-branch": map[string]any{
"protected-files": "blocked",
},
},
wantErr: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
frontmatter := baseFrontmatter(tt.safeOutputs)
err := ValidateMainWorkflowFrontmatterWithSchemaAndLocation(frontmatter, "/tmp/gh-aw/protected-files-schema-test.md")
if tt.wantErr {
if err == nil {
t.Fatalf("expected validation error for %q, got nil", tt.name)
}
if tt.errContains != "" && !strings.Contains(err.Error(), tt.errContains) {
t.Errorf("expected error to contain %q, got: %v", tt.errContains, err)
}
} else {
if err != nil {
t.Fatalf("expected %q to pass schema validation, got: %v", tt.name, err)
}
}
})
}
}

// TestMainWorkflowSchema_ProtectedFilesObjectFormStructure verifies that the
// main workflow JSON schema defines protected-files as a oneOf [string, object],
// not as a oneOf [string, null] (the old broken form that caused
// "expected string or null, got object" errors).
func TestMainWorkflowSchema_ProtectedFilesObjectFormStructure(t *testing.T) {
t.Parallel()

schemaContent, err := os.ReadFile("schemas/main_workflow_schema.json")
if err != nil {
t.Fatalf("failed to read schema: %v", err)
}

var schema map[string]any
if err := json.Unmarshal(schemaContent, &schema); err != nil {
t.Fatalf("failed to parse schema JSON: %v", err)
}

properties, ok := schema["properties"].(map[string]any)
if !ok {
t.Fatal("schema missing 'properties'")
}
safeOutputsSchema, ok := properties["safe-outputs"].(map[string]any)
if !ok {
t.Fatal("schema missing 'properties.safe-outputs'")
}
safeOutputsProps, ok := safeOutputsSchema["properties"].(map[string]any)
if !ok {
t.Fatal("schema missing 'properties.safe-outputs.properties'")
}

for _, handlerName := range []string{"create-pull-request", "push-to-pull-request-branch"} {
t.Run(handlerName, func(t *testing.T) {
handlerSchema, ok := safeOutputsProps[handlerName].(map[string]any)
if !ok {
t.Fatalf("schema missing 'safe-outputs.%s'", handlerName)
}
handlerOneOf, ok := handlerSchema["oneOf"].([]any)
if !ok {
t.Fatalf("'safe-outputs.%s' missing oneOf", handlerName)
}

// Find the object branch (the one with properties)
var objectBranchProps map[string]any
for _, candidate := range handlerOneOf {
c, ok := candidate.(map[string]any)
if !ok {
continue
}
if c["type"] == "object" {
if props, ok := c["properties"].(map[string]any); ok {
objectBranchProps = props
break
}
}
}
if objectBranchProps == nil {
t.Fatalf("'safe-outputs.%s' has no object branch in oneOf", handlerName)
}

pfSchema, ok := objectBranchProps["protected-files"].(map[string]any)
if !ok {
t.Fatalf("'safe-outputs.%s.properties.protected-files' not found", handlerName)
}

pfOneOf, ok := pfSchema["oneOf"].([]any)
if !ok {
t.Fatalf("'safe-outputs.%s.properties.protected-files' missing oneOf", handlerName)
}

var hasStringBranch, hasObjectBranch bool
for _, branch := range pfOneOf {
b, ok := branch.(map[string]any)
if !ok {
continue
}
switch b["type"] {
case "string":
hasStringBranch = true
case "object":
hasObjectBranch = true
case "null":
t.Errorf("'safe-outputs.%s.protected-files' has a null branch in its oneOf; "+
"the object form would produce 'expected string or null, got object' errors", handlerName)
}
}
if !hasStringBranch {
t.Errorf("'safe-outputs.%s.protected-files' missing string branch in oneOf", handlerName)
}
if !hasObjectBranch {
t.Errorf("'safe-outputs.%s.protected-files' missing object branch in oneOf; "+
"the object form {policy, exclude} would fail compilation", handlerName)
}

// Verify the object branch has the expected sub-fields
for _, branch := range pfOneOf {
b, ok := branch.(map[string]any)
if !ok || b["type"] != "object" {
continue
}
objProps, ok := b["properties"].(map[string]any)
if !ok {
t.Fatalf("'safe-outputs.%s.protected-files' object branch missing properties", handlerName)
}
if _, hasPolicyField := objProps["policy"]; !hasPolicyField {
t.Errorf("'safe-outputs.%s.protected-files' object branch missing 'policy' field", handlerName)
}
if _, hasExcludeField := objProps["exclude"]; !hasExcludeField {
t.Errorf("'safe-outputs.%s.protected-files' object branch missing 'exclude' field", handlerName)
}
if b["additionalProperties"] != false {
t.Errorf("'safe-outputs.%s.protected-files' object branch should have additionalProperties: false", handlerName)
}
}
})
}
}

// TestValidateWithSchema_YAMLIntegerTypes verifies that validateWithSchema accepts
// YAML-native integer types (uint64/int64) when the schema expects number/integer.
func TestValidateWithSchema_YAMLIntegerTypes(t *testing.T) {
Expand Down
Loading