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
148 changes: 148 additions & 0 deletions pkg/cli/compile_guard_policy_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
//go:build !integration

package cli

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

"github.com/github/gh-aw/pkg/workflow"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// TestGuardPolicyMinIntegrityOnly verifies that specifying only min-integrity
// under tools.github compiles successfully without requiring an explicit repos field.
// When repos is omitted, it should default to "all" (regression test for the fix).
func TestGuardPolicyMinIntegrityOnly(t *testing.T) {
tests := []struct {
name string
workflowContent string
expectError bool
errorContains string
}{
{
name: "min-integrity only defaults repos to all",
workflowContent: `---
on:
workflow_dispatch:
permissions:
contents: read
engine: copilot
tools:
github:
min-integrity: none
---

# Guard Policy Test

This workflow uses min-integrity without specifying repos.
`,
expectError: false,
},
{
name: "min-integrity with explicit repos=all compiles",
workflowContent: `---
on:
workflow_dispatch:
permissions:
contents: read
engine: copilot
tools:
github:
repos: all
min-integrity: unapproved
---

# Guard Policy Test

This workflow uses both repos and min-integrity.
`,
expectError: false,
},
{
name: "min-integrity with repos=public compiles",
workflowContent: `---
on:
workflow_dispatch:
permissions:
contents: read
engine: copilot
tools:
github:
repos: public
min-integrity: approved
---

# Guard Policy Test

This workflow restricts to public repos.
`,
expectError: false,
},
{
name: "min-integrity with repos array compiles",
workflowContent: `---
on:
workflow_dispatch:
permissions:
contents: read
engine: copilot
tools:
github:
repos:
- owner/repo
min-integrity: merged
---

# Guard Policy Test

This workflow specifies a repos array.
`,
expectError: false,
},
{
name: "repos only without min-integrity fails validation",
workflowContent: `---
on:
workflow_dispatch:
permissions:
contents: read
engine: copilot
tools:
github:
repos: all
---

# Guard Policy Test

This workflow specifies repos without min-integrity.
`,
expectError: true,
errorContains: "min-integrity",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tmpDir := t.TempDir()

workflowPath := filepath.Join(tmpDir, "test-guard-policy.md")
err := os.WriteFile(workflowPath, []byte(tt.workflowContent), 0644)
require.NoError(t, err, "Failed to write workflow file")

compiler := workflow.NewCompiler()
err = CompileWorkflowWithValidation(compiler, workflowPath, false, false, false, false, false, false)

if tt.expectError {
require.Error(t, err, "Expected compilation to fail")
if tt.errorContains != "" {
assert.Contains(t, err.Error(), tt.errorContains, "Error should mention %q", tt.errorContains)
}
} else {
assert.NoError(t, err, "Expected compilation to succeed")
}
})
}
}
22 changes: 22 additions & 0 deletions pkg/cli/workflows/test-guard-policy-min-integrity-only.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
on:
workflow_dispatch:
permissions:
contents: read
issues: read
pull-requests: read
engine: copilot
tools:
github:
min-integrity: none
---

# Test Guard Policy with min-integrity Only

This workflow verifies that specifying only `min-integrity` under `tools.github`
works correctly without requiring an explicit `repos` field.

When `repos` is omitted, it should default to `all`, allowing the workflow to compile
successfully.

Please list the first 3 open issues in this repository.
8 changes: 4 additions & 4 deletions pkg/workflow/tools_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func validateGitHubToolConfig(tools *Tools, workflowName string) error {

// validateGitHubGuardPolicy validates the GitHub guard policy configuration.
// Guard policy fields (repos, min-integrity) are specified flat under github:.
// Both fields must be present if either is specified.
// If repos is not specified but min-integrity is, repos defaults to "all".
func validateGitHubGuardPolicy(tools *Tools, workflowName string) error {
if tools == nil || tools.GitHub == nil {
return nil
Expand All @@ -77,10 +77,10 @@ func validateGitHubGuardPolicy(tools *Tools, workflowName string) error {
return nil
}

// Validate repos field (required when min-integrity is set)
// Default repos to "all" when not specified
if !hasRepos {
toolsValidationLog.Printf("Missing repos in guard policy for workflow: %s", workflowName)
return errors.New("invalid guard policy: 'github.repos' is required. Use 'all', 'public', or an array of repository patterns (e.g., ['owner/repo', 'owner/*'])")
toolsValidationLog.Printf("Defaulting repos to 'all' in guard policy for workflow: %s", workflowName)
github.Repos = "all"
}

// Validate repos format
Expand Down
5 changes: 2 additions & 3 deletions pkg/workflow/tools_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,14 +301,13 @@ func TestValidateGitHubGuardPolicy(t *testing.T) {
shouldError: false,
},
{
name: "missing repos field",
name: "missing repos field defaults to all",
toolsMap: map[string]any{
"github": map[string]any{
"min-integrity": "unapproved",
},
},
shouldError: true,
errorMsg: "'github.repos' is required",
shouldError: false,
},
Comment on lines +304 to 311
{
name: "missing min-integrity field",
Expand Down
2 changes: 1 addition & 1 deletion scratchpad/dev.md
Original file line number Diff line number Diff line change
Expand Up @@ -1771,7 +1771,7 @@ tools:
min-integrity: unapproved # none | unapproved | approved | merged
```

Both `repos` and `min-integrity` are required when either is specified under `github:`.
`min-integrity` is required when using GitHub guard policies. `repos` defaults to `"all"` if not specified.

**Repository Pattern Options**:
- `"all"` — All repositories accessible by the token
Expand Down
6 changes: 2 additions & 4 deletions scratchpad/guard-policies-specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -238,16 +238,14 @@ The design supports future MCP servers (Jira, WorkIQ) through:
- Case-sensitive

**Required Fields:**
- Both `repos` and `min-integrity` are required when either is specified under `github:`
- `min-integrity` is required when using GitHub guard policies
- `repos` defaults to `"all"` if not specified

## Error Messages

The implementation provides clear, actionable error messages:

```
invalid guard policy: 'github.repos' is required.
Use 'all', 'public', or an array of repository patterns (e.g., ['owner/repo', 'owner/*'])

invalid guard policy: repository pattern 'Owner/Repo' must be lowercase

invalid guard policy: repository pattern 'owner/re*po' has wildcard in the middle.
Expand Down
Loading