diff --git a/.github/agents/agentic-workflows.agent.md b/.github/agents/agentic-workflows.agent.md index 34ea6500cc1..e34a4da6123 100644 --- a/.github/agents/agentic-workflows.agent.md +++ b/.github/agents/agentic-workflows.agent.md @@ -15,6 +15,7 @@ This is a **dispatcher agent** that routes your request to the appropriate speci - **Updating existing workflows**: Routes to `update` prompt - **Debugging workflows**: Routes to `debug` prompt - **Upgrading workflows**: Routes to `upgrade-agentic-workflows` prompt +- **Creating report-generating workflows**: Routes to `report` prompt — consult this whenever the workflow posts status updates, audits, analyses, or any structured output as issues, discussions, or comments - **Creating shared components**: Routes to `create-shared-agentic-workflow` prompt - **Fixing Dependabot PRs**: Routes to `dependabot` prompt — use this when Dependabot opens PRs that modify generated manifest files (`.github/workflows/package.json`, `.github/workflows/requirements.txt`, `.github/workflows/go.mod`). Never merge those PRs directly; instead update the source `.md` files and rerun `gh aw compile --dependabot` to bundle all fixes @@ -87,6 +88,16 @@ When you interact with this agent, it will: - "Fix deprecated fields in workflows" - "Apply breaking changes from the new release" +### Create a Report-Generating Workflow +**Load when**: The workflow being created or updated produces reports — recurring status updates, audit summaries, analyses, or any structured output posted as a GitHub issue, discussion, or comment + +**Prompt file**: https://github.com/github/gh-aw/blob/main/.github/aw/report.md + +**Use cases**: +- "Create a weekly CI health report" +- "Post a daily security audit to Discussions" +- "Add a status update comment to open PRs" + ### Create Shared Agentic Workflow **Load when**: User wants to create a reusable workflow component or wrap an MCP server diff --git a/.github/workflows/smoke-codex.lock.yml b/.github/workflows/smoke-codex.lock.yml index 2cd8dd7a390..ffe35d4255d 100644 --- a/.github/workflows/smoke-codex.lock.yml +++ b/.github/workflows/smoke-codex.lock.yml @@ -28,7 +28,7 @@ # - shared/gh.md # - shared/reporting.md # -# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"fbb9ed29477ed621b1c3827b01a8f7bf0052063618b2900bb1e739453a3aa770"} +# gh-aw-metadata: {"schema_version":"v1","frontmatter_hash":"bb46b86a2eb0aa7f857448cb6c55f108cb59f8457436996aa7af27d40bc7bff5"} name: "Smoke Codex" "on": @@ -308,6 +308,7 @@ jobs: uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: persist-credentials: false + fetch-depth: 2 - name: Setup Go uses: actions/setup-go@7a3fe6cf4cb3a834922a1244abfce67bcef6a0c5 # v6.2.0 with: diff --git a/.github/workflows/smoke-codex.md b/.github/workflows/smoke-codex.md index c574f3214cd..bccabd34f94 100644 --- a/.github/workflows/smoke-codex.md +++ b/.github/workflows/smoke-codex.md @@ -62,6 +62,8 @@ safe-outputs: run-success: "✨ The prophecy is fulfilled... [{workflow_name}]({run_url}) has completed its mystical journey. The stars align. 🌟" run-failure: "🌑 The shadows whisper... [{workflow_name}]({run_url}) {status}. The oracle requires further meditation..." timeout-minutes: 15 +checkout: + fetch-depth: 2 --- # Smoke Test: Codex Engine Validation diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index a0a2e5f0ecc..7b43a6fd097 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -1396,12 +1396,12 @@ "description": "Skip workflow execution for specific GitHub users. Useful for preventing workflows from running for specific accounts (e.g., bots, specific team members)." }, "roles": { - "description": "Repository access roles required to trigger agentic workflows. Defaults to ['admin', 'maintainer', 'write'] for security. Use 'all' to allow any authenticated user (⚠️ security consideration).", + "description": "Repository access roles required to trigger agentic workflows. Defaults to ['admin', 'maintainer', 'write'] for security. Use 'all' to allow any authenticated user (\u26a0\ufe0f security consideration).", "oneOf": [ { "type": "string", "enum": ["all"], - "description": "Allow any authenticated user to trigger the workflow (⚠️ disables permission checking entirely - use with caution)" + "description": "Allow any authenticated user to trigger the workflow (\u26a0\ufe0f disables permission checking entirely - use with caution)" }, { "type": "array", @@ -2186,7 +2186,7 @@ }, "network": { "$comment": "Strict mode requirements: When strict=true, the 'network' field must be present (not null/undefined) and cannot contain standalone wildcard '*' in allowed domains (but patterns like '*.example.com' ARE allowed). This is validated in Go code (pkg/workflow/strict_mode_validation.go) via validateStrictNetwork().", - "description": "Network access control for AI engines using ecosystem identifiers and domain allowlists. Supports wildcard patterns like '*.example.com' to match any subdomain. Controls web fetch and search capabilities. IMPORTANT: For workflows that build/install/test code, always include the language ecosystem identifier alongside 'defaults' — 'defaults' alone only covers basic infrastructure, not package registries. Key ecosystem identifiers by runtime: 'dotnet' (.NET/NuGet), 'python' (pip/PyPI), 'node' (npm/yarn), 'go' (go modules), 'java' (Maven/Gradle), 'ruby' (Bundler), 'rust' (Cargo), 'swift' (Swift PM). Example: a .NET project needs network: { allowed: [defaults, dotnet] }.", + "description": "Network access control for AI engines using ecosystem identifiers and domain allowlists. Supports wildcard patterns like '*.example.com' to match any subdomain. Controls web fetch and search capabilities. IMPORTANT: For workflows that build/install/test code, always include the language ecosystem identifier alongside 'defaults' \u2014 'defaults' alone only covers basic infrastructure, not package registries. Key ecosystem identifiers by runtime: 'dotnet' (.NET/NuGet), 'python' (pip/PyPI), 'node' (npm/yarn), 'go' (go modules), 'java' (Maven/Gradle), 'ruby' (Bundler), 'rust' (Cargo), 'swift' (Swift PM). Example: a .NET project needs network: { allowed: [defaults, dotnet] }.", "examples": [ "defaults", { @@ -2574,7 +2574,7 @@ ] }, "plugins": { - "description": "⚠️ EXPERIMENTAL: Plugin configuration for installing plugins before workflow execution. Supports array format (list of repos/plugin configs) and object format (repos + custom token). Note: Plugin support is experimental and may change in future releases.", + "description": "\u26a0\ufe0f EXPERIMENTAL: Plugin configuration for installing plugins before workflow execution. Supports array format (list of repos/plugin configs) and object format (repos + custom token). Note: Plugin support is experimental and may change in future releases.", "examples": [ ["github/copilot-plugin", "acme/custom-tools"], [ @@ -2771,7 +2771,7 @@ [ { "name": "Verify Post-Steps Execution", - "run": "echo \"✅ Post-steps are executing correctly\"\necho \"This step runs after the AI agent completes\"\n" + "run": "echo \"\u2705 Post-steps are executing correctly\"\necho \"This step runs after the AI agent completes\"\n" }, { "name": "Upload Test Results", @@ -5361,7 +5361,7 @@ "oneOf": [ { "type": "object", - "description": "Configuration for resolving review threads on pull requests. Resolution is scoped to the triggering PR only — threads on other PRs cannot be resolved.", + "description": "Configuration for resolving review threads on pull requests. Resolution is scoped to the triggering PR only \u2014 threads on other PRs cannot be resolved.", "properties": { "max": { "description": "Maximum number of review threads to resolve (default: 10) Supports integer or GitHub Actions expression (e.g. '${{ inputs.max }}').", @@ -6795,8 +6795,8 @@ }, "staged-title": { "type": "string", - "description": "Custom title template for staged mode preview. Available placeholders: {operation}. Example: '🎭 Preview: {operation}'", - "examples": ["🎭 Preview: {operation}", "## Staged Mode: {operation}"] + "description": "Custom title template for staged mode preview. Available placeholders: {operation}. Example: '\ud83c\udfad Preview: {operation}'", + "examples": ["\ud83c\udfad Preview: {operation}", "## Staged Mode: {operation}"] }, "staged-description": { "type": "string", @@ -6810,18 +6810,18 @@ }, "run-success": { "type": "string", - "description": "Custom message template for successful workflow completion. Available placeholders: {workflow_name}, {run_url}. Default: '✅ Agentic [{workflow_name}]({run_url}) completed successfully.'", - "examples": ["✅ Agentic [{workflow_name}]({run_url}) completed successfully.", "✅ [{workflow_name}]({run_url}) finished."] + "description": "Custom message template for successful workflow completion. Available placeholders: {workflow_name}, {run_url}. Default: '\u2705 Agentic [{workflow_name}]({run_url}) completed successfully.'", + "examples": ["\u2705 Agentic [{workflow_name}]({run_url}) completed successfully.", "\u2705 [{workflow_name}]({run_url}) finished."] }, "run-failure": { "type": "string", - "description": "Custom message template for failed workflow. Available placeholders: {workflow_name}, {run_url}, {status}. Default: '❌ Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.'", - "examples": ["❌ Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.", "❌ [{workflow_name}]({run_url}) {status}."] + "description": "Custom message template for failed workflow. Available placeholders: {workflow_name}, {run_url}, {status}. Default: '\u274c Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.'", + "examples": ["\u274c Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.", "\u274c [{workflow_name}]({run_url}) {status}."] }, "detection-failure": { "type": "string", - "description": "Custom message template for detection job failure. Available placeholders: {workflow_name}, {run_url}. Default: '⚠️ Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.'", - "examples": ["⚠️ Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.", "⚠️ Detection job failed in [{workflow_name}]({run_url})."] + "description": "Custom message template for detection job failure. Available placeholders: {workflow_name}, {run_url}. Default: '\u26a0\ufe0f Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.'", + "examples": ["\u26a0\ufe0f Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.", "\u26a0\ufe0f Detection job failed in [{workflow_name}]({run_url})."] }, "agent-failure-issue": { "type": "string", @@ -7272,6 +7272,22 @@ } }, "additionalProperties": false + }, + "checkout": { + "description": "Checkout configuration for the agent job. Controls how actions/checkout is invoked. Can be a single checkout configuration or an array for multiple checkouts.", + "oneOf": [ + { + "$ref": "#/$defs/checkoutConfig", + "description": "Single checkout configuration for the default workspace" + }, + { + "type": "array", + "description": "Multiple checkout configurations", + "items": { + "$ref": "#/$defs/checkoutConfig" + } + } + ] } }, "additionalProperties": false, @@ -7792,6 +7808,60 @@ "required": ["run"] } ] + }, + "checkoutConfig": { + "type": "object", + "description": "Configuration for a single actions/checkout step", + "additionalProperties": false, + "properties": { + "repository": { + "type": "string", + "description": "Repository to checkout in owner/repo format. Defaults to the current repository.", + "examples": ["owner/repo", "github/gh-aw"] + }, + "ref": { + "type": "string", + "description": "Branch, tag, or SHA to checkout. Defaults to the ref that triggered the workflow.", + "examples": ["main", "v1.0.0", "feature/my-branch"] + }, + "path": { + "type": "string", + "description": "Relative path within GITHUB_WORKSPACE to place the checkout. Defaults to the workspace root.", + "examples": [".", "./libs/other-repo", "./workspace"] + }, + "fetch-depth": { + "type": "integer", + "minimum": 0, + "description": "Number of commits to fetch. 0 fetches all history. 1 (default) is a shallow clone. When multiple configs target the same path, the deepest value is used.", + "examples": [0, 1, 10] + }, + "sparse-checkout": { + "type": "string", + "description": "Enable sparse-checkout with newline-separated patterns. When multiple configs target the same path, patterns are merged.", + "examples": [".github/\nsrc/", "docs/"] + }, + "submodules": { + "oneOf": [ + { + "type": "string", + "enum": ["recursive", "true", "false"] + }, + { + "type": "boolean" + } + ], + "description": "Controls submodule checkout. Use \"recursive\" for all submodules, \"true\" for immediate submodules, or \"false\" to skip." + }, + "lfs": { + "type": "boolean", + "description": "Whether to download Git LFS objects. Defaults to false." + }, + "github-token": { + "type": "string", + "description": "GitHub token for authentication. Use ${{ secrets.MY_TOKEN }} to reference a secret. Credentials are always removed after checkout (persist-credentials: false is enforced).", + "examples": ["${{ secrets.MY_PAT }}", "${{ secrets.GITHUB_TOKEN }}"] + } + } } } } diff --git a/pkg/workflow/checkout_manager.go b/pkg/workflow/checkout_manager.go new file mode 100644 index 00000000000..fd35f3f1434 --- /dev/null +++ b/pkg/workflow/checkout_manager.go @@ -0,0 +1,486 @@ +package workflow + +import ( + "errors" + "fmt" + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var checkoutManagerLog = logger.New("workflow:checkout_manager") + +// CheckoutConfig represents a single checkout configuration from workflow frontmatter. +// It controls how actions/checkout is invoked in the agent job. +// +// Supports all relevant options from actions/checkout: +// +// checkout: +// fetch-depth: 0 +// github-token: ${{ secrets.MY_TOKEN }} +// +// Or multiple checkouts: +// +// checkout: +// - path: . +// fetch-depth: 0 +// - repository: owner/other-repo +// path: ./libs/other +// ref: main +type CheckoutConfig struct { + // Repository to checkout in owner/repo format. Defaults to the current repository. + Repository string `json:"repository,omitempty"` + + // Ref (branch, tag, or SHA) to checkout. Defaults to the ref that triggered the workflow. + Ref string `json:"ref,omitempty"` + + // Path within GITHUB_WORKSPACE to place the checkout. Defaults to the workspace root. + Path string `json:"path,omitempty"` + + // GitHubToken overrides the default GITHUB_TOKEN for authentication. + // Use ${{ secrets.MY_TOKEN }} to reference a repository secret. + GitHubToken string `json:"github-token,omitempty"` + + // FetchDepth controls the number of commits to fetch. + // 0 fetches all history (full clone). 1 is a shallow clone (default). + FetchDepth *int `json:"fetch-depth,omitempty"` + + // SparseCheckout enables sparse-checkout mode. Provide newline-separated patterns + // (e.g., ".github/\nsrc/"). When multiple configs target the same path, patterns + // are merged into a single checkout. + SparseCheckout string `json:"sparse-checkout,omitempty"` + + // Submodules controls submodule checkout behavior: "recursive", "true", or "false". + Submodules string `json:"submodules,omitempty"` + + // LFS enables checkout of Git LFS objects. + LFS bool `json:"lfs,omitempty"` +} + +// checkoutKey uniquely identifies a checkout target used for grouping/deduplication. +// Only repository and path are used as key fields — ref and token are settings +// that can be merged across configs targeting the same (repository, path). +type checkoutKey struct { + repository string + path string +} + +// resolvedCheckout is an internal merged checkout entry used by CheckoutManager. +type resolvedCheckout struct { + key checkoutKey + ref string // last non-empty ref wins + token string // last non-empty token wins + fetchDepth *int // nil means use default (1) + sparsePatterns []string // merged sparse-checkout patterns + submodules string + lfs bool +} + +// CheckoutManager collects checkout requests and merges them to minimize +// the number of actions/checkout steps emitted. +// +// Merging rules: +// - Checkouts with the same (repository, ref, path, token) are merged into one. +// - The deepest fetch-depth wins: 0 (full history) overrides any shallower value. +// - Sparse-checkout patterns are unioned across merged configs. +// - LFS and submodules are OR-ed (if any request enables them, the result enables them). +type CheckoutManager struct { + // ordered preserves insertion order for deterministic output + ordered []*resolvedCheckout + // index maps checkoutKey to the position in ordered + index map[checkoutKey]int +} + +// NewCheckoutManager creates a new CheckoutManager pre-loaded with user-supplied +// CheckoutConfig entries from the frontmatter. +func NewCheckoutManager(userCheckouts []*CheckoutConfig) *CheckoutManager { + cm := &CheckoutManager{ + index: make(map[checkoutKey]int), + } + for _, cfg := range userCheckouts { + cm.add(cfg) + } + return cm +} + +// add processes a single CheckoutConfig and either creates a new entry or merges +// it into an existing entry with the same key. +func (cm *CheckoutManager) add(cfg *CheckoutConfig) { + if cfg == nil { + return + } + + key := checkoutKey{ + repository: cfg.Repository, + path: cfg.Path, + } + + if idx, exists := cm.index[key]; exists { + // Merge into existing entry; first-seen wins for ref and token + entry := cm.ordered[idx] + entry.fetchDepth = deeperFetchDepth(entry.fetchDepth, cfg.FetchDepth) + if cfg.Ref != "" && entry.ref == "" { + entry.ref = cfg.Ref // first-seen ref wins + } + if cfg.GitHubToken != "" && entry.token == "" { + entry.token = cfg.GitHubToken // first-seen token wins + } + if cfg.SparseCheckout != "" { + entry.sparsePatterns = mergeSparsePatterns(entry.sparsePatterns, cfg.SparseCheckout) + } + if cfg.LFS { + entry.lfs = true + } + if cfg.Submodules != "" && entry.submodules == "" { + entry.submodules = cfg.Submodules + } + checkoutManagerLog.Printf("Merged checkout for path=%q repository=%q", key.path, key.repository) + } else { + entry := &resolvedCheckout{ + key: key, + ref: cfg.Ref, + token: cfg.GitHubToken, + fetchDepth: cfg.FetchDepth, + submodules: cfg.Submodules, + lfs: cfg.LFS, + } + if cfg.SparseCheckout != "" { + entry.sparsePatterns = mergeSparsePatterns(nil, cfg.SparseCheckout) + } + cm.index[key] = len(cm.ordered) + cm.ordered = append(cm.ordered, entry) + checkoutManagerLog.Printf("Added checkout for path=%q repository=%q", key.path, key.repository) + } +} + +// HasUserCheckouts returns true if any user-supplied checkouts were registered. +func (cm *CheckoutManager) HasUserCheckouts() bool { + return len(cm.ordered) > 0 +} + +// GetDefaultCheckoutOverride returns the resolved checkout for the default workspace +// (empty path, empty repository). Returns nil if the user did not configure one. +func (cm *CheckoutManager) GetDefaultCheckoutOverride() *resolvedCheckout { + key := checkoutKey{} + if idx, ok := cm.index[key]; ok { + return cm.ordered[idx] + } + return nil +} + +// GenerateAdditionalCheckoutSteps generates YAML step lines for all non-default +// (additional) checkouts — those that target a specific path other than the root. +// The caller is responsible for emitting the default workspace checkout separately. +func (cm *CheckoutManager) GenerateAdditionalCheckoutSteps(getActionPin func(string) string) []string { + var lines []string + for _, entry := range cm.ordered { + // Skip the default checkout (handled separately) + if entry.key.path == "" && entry.key.repository == "" { + continue + } + lines = append(lines, generateCheckoutStepLines(entry, getActionPin)...) + } + return lines +} + +// generateDefaultCheckoutStep emits the default workspace checkout, applying any +// user-supplied overrides (token, fetch-depth, ref, etc.) on top of the required +// security defaults (persist-credentials: false). +// +// Parameters: +// - trialMode: if true, optionally sets repository and token for trial execution +// - trialLogicalRepoSlug: the repository to checkout in trial mode +// - getActionPin: resolves an action reference to a pinned SHA form +// +// Returns a slice of YAML lines (each ending with \n). +func (cm *CheckoutManager) GenerateDefaultCheckoutStep( + trialMode bool, + trialLogicalRepoSlug string, + getActionPin func(string) string, +) []string { + override := cm.GetDefaultCheckoutOverride() + + var sb strings.Builder + sb.WriteString(" - name: Checkout repository\n") + fmt.Fprintf(&sb, " uses: %s\n", getActionPin("actions/checkout")) + sb.WriteString(" with:\n") + + // Security: always disable credential persistence so the agent cannot + // exfiltrate credentials from disk. + sb.WriteString(" persist-credentials: false\n") + + // Apply trial mode overrides + if trialMode { + if trialLogicalRepoSlug != "" { + fmt.Fprintf(&sb, " repository: %s\n", trialLogicalRepoSlug) + } + effectiveToken := getEffectiveGitHubToken("") + fmt.Fprintf(&sb, " token: %s\n", effectiveToken) + } + + // Apply user overrides (only when NOT in trial mode to avoid conflicts) + if !trialMode && override != nil { + if override.key.repository != "" { + fmt.Fprintf(&sb, " repository: %s\n", override.key.repository) + } + if override.ref != "" { + fmt.Fprintf(&sb, " ref: %s\n", override.ref) + } + if override.token != "" { + fmt.Fprintf(&sb, " github-token: %s\n", override.token) + } + if override.fetchDepth != nil { + fmt.Fprintf(&sb, " fetch-depth: %d\n", *override.fetchDepth) + } + if len(override.sparsePatterns) > 0 { + sb.WriteString(" sparse-checkout: |\n") + for _, pattern := range override.sparsePatterns { + fmt.Fprintf(&sb, " %s\n", strings.TrimSpace(pattern)) + } + } + if override.submodules != "" { + fmt.Fprintf(&sb, " submodules: %s\n", override.submodules) + } + if override.lfs { + sb.WriteString(" lfs: true\n") + } + } + + return []string{sb.String()} +} + +// generateCheckoutStepLines generates YAML step lines for a single non-default checkout. +func generateCheckoutStepLines(entry *resolvedCheckout, getActionPin func(string) string) []string { + name := "Checkout " + checkoutStepName(entry.key) + var sb strings.Builder + fmt.Fprintf(&sb, " - name: %s\n", name) + fmt.Fprintf(&sb, " uses: %s\n", getActionPin("actions/checkout")) + sb.WriteString(" with:\n") + + // Security: always disable credential persistence + sb.WriteString(" persist-credentials: false\n") + + if entry.key.repository != "" { + fmt.Fprintf(&sb, " repository: %s\n", entry.key.repository) + } + if entry.ref != "" { + fmt.Fprintf(&sb, " ref: %s\n", entry.ref) + } + if entry.key.path != "" { + fmt.Fprintf(&sb, " path: %s\n", entry.key.path) + } + if entry.token != "" { + fmt.Fprintf(&sb, " github-token: %s\n", entry.token) + } + if entry.fetchDepth != nil { + fmt.Fprintf(&sb, " fetch-depth: %d\n", *entry.fetchDepth) + } + if len(entry.sparsePatterns) > 0 { + sb.WriteString(" sparse-checkout: |\n") + for _, pattern := range entry.sparsePatterns { + fmt.Fprintf(&sb, " %s\n", strings.TrimSpace(pattern)) + } + } + if entry.submodules != "" { + fmt.Fprintf(&sb, " submodules: %s\n", entry.submodules) + } + if entry.lfs { + sb.WriteString(" lfs: true\n") + } + + return []string{sb.String()} +} + +// checkoutStepName returns a human-readable description for a checkout step. +func checkoutStepName(key checkoutKey) string { + if key.repository != "" && key.path != "" { + return fmt.Sprintf("%s into %s", key.repository, key.path) + } + if key.repository != "" { + return key.repository + } + if key.path != "" { + return key.path + } + return "repository" +} + +// deeperFetchDepth returns the deeper of two fetch-depth values. +// 0 means full history and is always "deepest"; otherwise lower positive values +// are shallower. nil means "use default". +func deeperFetchDepth(a, b *int) *int { + if a == nil && b == nil { + return nil + } + if a == nil { + return b + } + if b == nil { + return a + } + // 0 = full history = deepest + if *a == 0 || *b == 0 { + zero := 0 + return &zero + } + // For positive depths, larger value = more history = deeper + if *a > *b { + return a + } + return b +} + +// mergeSparsePatterns parses and unions sparse-checkout patterns. +// Patterns can be newline-separated. +func mergeSparsePatterns(existing []string, newPatterns string) []string { + seen := make(map[string]bool, len(existing)) + result := make([]string, 0, len(existing)) + + for _, p := range existing { + p = strings.TrimSpace(p) + if p != "" && !seen[p] { + seen[p] = true + result = append(result, p) + } + } + + for p := range strings.SplitSeq(newPatterns, "\n") { + p = strings.TrimSpace(p) + if p != "" && !seen[p] { + seen[p] = true + result = append(result, p) + } + } + + return result +} + +// ParseCheckoutConfigs converts a raw frontmatter value (single map or array of maps) +// into a slice of CheckoutConfig entries. +// Returns (nil, nil) if the value is nil; for non-nil values, invalid types or shapes +// result in a non-nil error. +func ParseCheckoutConfigs(raw any) ([]*CheckoutConfig, error) { + if raw == nil { + return nil, nil + } + + // Try single object first + if singleMap, ok := raw.(map[string]any); ok { + cfg, err := checkoutConfigFromMap(singleMap) + if err != nil { + return nil, fmt.Errorf("invalid checkout configuration: %w", err) + } + return []*CheckoutConfig{cfg}, nil + } + + // Try array of objects + if arr, ok := raw.([]any); ok { + configs := make([]*CheckoutConfig, 0, len(arr)) + 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) + } + cfg, err := checkoutConfigFromMap(itemMap) + if err != nil { + return nil, fmt.Errorf("checkout[%d]: %w", i, err) + } + configs = append(configs, cfg) + } + return configs, nil + } + + return nil, fmt.Errorf("checkout must be an object or an array of objects, got %T", raw) +} + +// checkoutConfigFromMap converts a raw map to a CheckoutConfig. +func checkoutConfigFromMap(m map[string]any) (*CheckoutConfig, error) { + cfg := &CheckoutConfig{} + + if v, ok := m["repository"]; ok { + s, ok := v.(string) + if !ok { + return nil, errors.New("checkout.repository must be a string") + } + 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") + } + 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") + } + cfg.Path = s + } + + if v, ok := m["github-token"]; ok { + s, ok := v.(string) + if !ok { + return nil, errors.New("checkout.github-token must be a string") + } + cfg.GitHubToken = s + } + + if v, ok := m["fetch-depth"]; ok { + switch n := v.(type) { + case int: + depth := n + cfg.FetchDepth = &depth + case int64: + depth := int(n) + cfg.FetchDepth = &depth + case uint64: + depth := int(n) + cfg.FetchDepth = &depth + case float64: + if n != float64(int64(n)) { + return nil, errors.New("checkout.fetch-depth must be an integer") + } + depth := int(n) + cfg.FetchDepth = &depth + default: + return nil, errors.New("checkout.fetch-depth must be an integer") + } + } + + if v, ok := m["sparse-checkout"]; ok { + s, ok := v.(string) + if !ok { + return nil, errors.New("checkout.sparse-checkout must be a string") + } + cfg.SparseCheckout = s + } + + if v, ok := m["submodules"]; ok { + switch sv := v.(type) { + case string: + cfg.Submodules = sv + case bool: + if sv { + cfg.Submodules = "true" + } else { + cfg.Submodules = "false" + } + default: + return nil, errors.New("checkout.submodules must be a string or boolean") + } + } + + if v, ok := m["lfs"]; ok { + b, ok := v.(bool) + if !ok { + return nil, errors.New("checkout.lfs must be a boolean") + } + cfg.LFS = b + } + + return cfg, nil +} diff --git a/pkg/workflow/checkout_manager_test.go b/pkg/workflow/checkout_manager_test.go new file mode 100644 index 00000000000..11af372dca6 --- /dev/null +++ b/pkg/workflow/checkout_manager_test.go @@ -0,0 +1,338 @@ +//go:build !integration + +package workflow + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestNewCheckoutManager verifies that a CheckoutManager can be created with user configs. +func TestNewCheckoutManager(t *testing.T) { + t.Run("empty configs produces empty manager", func(t *testing.T) { + cm := NewCheckoutManager(nil) + assert.False(t, cm.HasUserCheckouts(), "empty manager should report no user checkouts") + assert.Nil(t, cm.GetDefaultCheckoutOverride(), "empty manager should have no default override") + }) + + t.Run("single default override", func(t *testing.T) { + depth := 0 + cm := NewCheckoutManager([]*CheckoutConfig{ + {FetchDepth: &depth}, + }) + assert.True(t, cm.HasUserCheckouts(), "should have user checkouts") + override := cm.GetDefaultCheckoutOverride() + require.NotNil(t, override, "should have default override") + require.NotNil(t, override.fetchDepth, "fetch depth should be set") + assert.Equal(t, 0, *override.fetchDepth, "fetch depth should be 0") + }) + + t.Run("custom token on default checkout", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + {GitHubToken: "${{ secrets.MY_TOKEN }}"}, + }) + override := cm.GetDefaultCheckoutOverride() + require.NotNil(t, override, "should have default override") + assert.Equal(t, "${{ secrets.MY_TOKEN }}", override.token, "token should be set") + }) +} + +// TestCheckoutManagerMerging verifies that duplicate checkout configs are merged. +func TestCheckoutManagerMerging(t *testing.T) { + t.Run("duplicate default checkout takes deepest fetch-depth", func(t *testing.T) { + depth1 := 1 + depth10 := 10 + cm := NewCheckoutManager([]*CheckoutConfig{ + {FetchDepth: &depth1}, + {FetchDepth: &depth10}, + }) + assert.Len(t, cm.ordered, 1, "should have merged into a single entry") + override := cm.GetDefaultCheckoutOverride() + require.NotNil(t, override.fetchDepth, "fetch depth should be set after merge") + assert.Equal(t, 10, *override.fetchDepth, "should use deeper fetch-depth (10 > 1)") + }) + + t.Run("zero fetch-depth wins over any positive value", func(t *testing.T) { + depth0 := 0 + depth5 := 5 + cm := NewCheckoutManager([]*CheckoutConfig{ + {FetchDepth: &depth5}, + {FetchDepth: &depth0}, + }) + override := cm.GetDefaultCheckoutOverride() + require.NotNil(t, override.fetchDepth, "fetch depth should be set") + assert.Equal(t, 0, *override.fetchDepth, "0 (full history) should win") + }) + + t.Run("sparse-checkout patterns are merged", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + {Path: "./workspace", SparseCheckout: ".github/"}, + {Path: "./workspace", SparseCheckout: "src/"}, + }) + assert.Len(t, cm.ordered, 1, "should have merged into a single entry") + additional := cm.GenerateAdditionalCheckoutSteps(func(s string) string { return s }) + combined := strings.Join(additional, "") + assert.Contains(t, combined, ".github/", "should contain first sparse pattern") + assert.Contains(t, combined, "src/", "should contain second sparse pattern") + }) + + t.Run("different paths produce separate checkouts", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + {Path: "./workspace1"}, + {Path: "./workspace2"}, + }) + assert.Len(t, cm.ordered, 2, "different paths should not be merged") + }) + + t.Run("different repos produce separate checkouts", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + {Repository: "owner/repo1", Path: "./r1"}, + {Repository: "owner/repo2", Path: "./r2"}, + }) + assert.Len(t, cm.ordered, 2, "different repos should not be merged") + }) + + t.Run("same path with different refs merges to first ref", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + {Path: "./workspace", Ref: "main"}, + {Path: "./workspace", Ref: "develop"}, + }) + assert.Len(t, cm.ordered, 1, "same path should be merged") + assert.Equal(t, "main", cm.ordered[0].ref, "first-seen ref should win") + }) +} + +// TestGenerateDefaultCheckoutStep verifies the default checkout step output. +func TestGenerateDefaultCheckoutStep(t *testing.T) { + getPin := func(action string) string { return action + "@v4" } + + t.Run("default checkout has persist-credentials false", func(t *testing.T) { + cm := NewCheckoutManager(nil) + lines := cm.GenerateDefaultCheckoutStep(false, "", getPin) + combined := strings.Join(lines, "") + assert.Contains(t, combined, "persist-credentials: false", "must always have persist-credentials: false") + assert.Contains(t, combined, "Checkout repository", "should have default step name") + assert.Contains(t, combined, "actions/checkout@v4", "should use pinned checkout action") + }) + + t.Run("user token is included in default checkout", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + {GitHubToken: "${{ secrets.MY_TOKEN }}"}, + }) + lines := cm.GenerateDefaultCheckoutStep(false, "", getPin) + combined := strings.Join(lines, "") + assert.Contains(t, combined, "github-token: ${{ secrets.MY_TOKEN }}", "should include custom token") + assert.Contains(t, combined, "persist-credentials: false", "must always have persist-credentials: false even with custom token") + }) + + t.Run("fetch-depth override is included", func(t *testing.T) { + depth := 0 + cm := NewCheckoutManager([]*CheckoutConfig{ + {FetchDepth: &depth}, + }) + lines := cm.GenerateDefaultCheckoutStep(false, "", getPin) + combined := strings.Join(lines, "") + assert.Contains(t, combined, "fetch-depth: 0", "should include fetch-depth override") + }) + + t.Run("ref override is included", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + {Ref: "develop"}, + }) + lines := cm.GenerateDefaultCheckoutStep(false, "", getPin) + combined := strings.Join(lines, "") + assert.Contains(t, combined, "ref: develop", "should include ref override") + }) + + t.Run("trial mode overrides user config", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + {GitHubToken: "${{ secrets.MY_TOKEN }}"}, + }) + lines := cm.GenerateDefaultCheckoutStep(true, "owner/trial-repo", getPin) + combined := strings.Join(lines, "") + assert.Contains(t, combined, "repository: owner/trial-repo", "trial repo should be in output") + // In trial mode, user token should NOT be emitted (trial uses its own token) + assert.NotContains(t, combined, "secrets.MY_TOKEN", "user token should not appear in trial mode") + }) + + t.Run("sparse-checkout override is included", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + {SparseCheckout: ".github/\nsrc/"}, + }) + lines := cm.GenerateDefaultCheckoutStep(false, "", getPin) + combined := strings.Join(lines, "") + assert.Contains(t, combined, "sparse-checkout: |", "should include sparse-checkout header") + assert.Contains(t, combined, ".github/", "should include first pattern") + assert.Contains(t, combined, "src/", "should include second pattern") + }) +} + +// TestGenerateAdditionalCheckoutSteps verifies that non-default checkouts are emitted correctly. +func TestGenerateAdditionalCheckoutSteps(t *testing.T) { + getPin := func(action string) string { return action + "@v4" } + + t.Run("no additional checkouts when only default configured", func(t *testing.T) { + depth := 0 + cm := NewCheckoutManager([]*CheckoutConfig{ + {FetchDepth: &depth}, + }) + lines := cm.GenerateAdditionalCheckoutSteps(getPin) + assert.Empty(t, lines, "should produce no additional checkout steps") + }) + + t.Run("additional checkout for different path", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + {Repository: "owner/libs", Path: "./libs/owner-libs", Ref: "main"}, + }) + lines := cm.GenerateAdditionalCheckoutSteps(getPin) + combined := strings.Join(lines, "") + assert.Contains(t, combined, "repository: owner/libs", "should include repo") + assert.Contains(t, combined, "path: ./libs/owner-libs", "should include path") + assert.Contains(t, combined, "ref: main", "should include ref") + assert.Contains(t, combined, "persist-credentials: false", "must always have persist-credentials: false") + }) + + t.Run("additional checkout with LFS enabled", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + {Path: "./lfs-repo", LFS: true}, + }) + lines := cm.GenerateAdditionalCheckoutSteps(getPin) + combined := strings.Join(lines, "") + assert.Contains(t, combined, "lfs: true", "should include LFS option") + }) + + t.Run("additional checkout with recursive submodules", func(t *testing.T) { + cm := NewCheckoutManager([]*CheckoutConfig{ + {Path: "./with-submodules", Submodules: "recursive"}, + }) + lines := cm.GenerateAdditionalCheckoutSteps(getPin) + combined := strings.Join(lines, "") + assert.Contains(t, combined, "submodules: recursive", "should include submodules option") + }) +} + +// TestParseCheckoutConfigs verifies parsing of raw frontmatter values. +func TestParseCheckoutConfigs(t *testing.T) { + t.Run("nil returns nil", func(t *testing.T) { + configs, err := ParseCheckoutConfigs(nil) + require.NoError(t, err, "nil should not error") + assert.Nil(t, configs, "nil input should return nil configs") + }) + + t.Run("single object", func(t *testing.T) { + raw := map[string]any{ + "fetch-depth": float64(0), + "github-token": "${{ secrets.MY_TOKEN }}", + } + configs, err := ParseCheckoutConfigs(raw) + require.NoError(t, err, "single object should parse without error") + require.Len(t, configs, 1, "should produce one config") + assert.Equal(t, "${{ secrets.MY_TOKEN }}", configs[0].GitHubToken, "token should be set") + require.NotNil(t, configs[0].FetchDepth, "fetch-depth should be set") + assert.Equal(t, 0, *configs[0].FetchDepth, "fetch-depth should be 0") + }) + + t.Run("array of objects", func(t *testing.T) { + raw := []any{ + map[string]any{"path": "."}, + map[string]any{"repository": "owner/repo", "path": "./libs"}, + } + configs, err := ParseCheckoutConfigs(raw) + require.NoError(t, err, "array should parse without error") + require.Len(t, configs, 2, "should produce two configs") + assert.Equal(t, ".", configs[0].Path, "first path should be set") + assert.Equal(t, "owner/repo", configs[1].Repository, "second repo should be set") + }) + + t.Run("invalid type returns error", func(t *testing.T) { + _, err := ParseCheckoutConfigs("invalid") + assert.Error(t, err, "string value should return an error") + }) + + t.Run("array with non-object entry returns error", func(t *testing.T) { + raw := []any{"not-an-object"} + _, err := ParseCheckoutConfigs(raw) + assert.Error(t, err, "array with non-object entry should return error") + }) + + t.Run("submodules as bool true", func(t *testing.T) { + raw := map[string]any{"submodules": true} + configs, err := ParseCheckoutConfigs(raw) + require.NoError(t, err) + require.Len(t, configs, 1) + assert.Equal(t, "true", configs[0].Submodules, "bool true should convert to string 'true'") + }) + + t.Run("submodules as bool false", func(t *testing.T) { + raw := map[string]any{"submodules": false} + configs, err := ParseCheckoutConfigs(raw) + require.NoError(t, err) + require.Len(t, configs, 1) + assert.Equal(t, "false", configs[0].Submodules, "bool false should convert to string 'false'") + }) + + t.Run("submodules as string recursive", func(t *testing.T) { + raw := map[string]any{"submodules": "recursive"} + configs, err := ParseCheckoutConfigs(raw) + require.NoError(t, err) + require.Len(t, configs, 1) + assert.Equal(t, "recursive", configs[0].Submodules, "string should be preserved") + }) +} + +// TestDeeperFetchDepth tests the fetch-depth comparison logic. +func TestDeeperFetchDepth(t *testing.T) { + ptr := func(n int) *int { return &n } + + tests := []struct { + name string + a, b *int + expected *int + }{ + {"both nil returns nil", nil, nil, nil}, + {"a nil returns b", nil, ptr(5), ptr(5)}, + {"b nil returns a", ptr(5), nil, ptr(5)}, + {"0 beats positive", ptr(0), ptr(5), ptr(0)}, + {"positive beats 0 (reversed)", ptr(5), ptr(0), ptr(0)}, + {"larger positive wins", ptr(3), ptr(10), ptr(10)}, + {"smaller positive loses", ptr(10), ptr(3), ptr(10)}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := deeperFetchDepth(tt.a, tt.b) + if tt.expected == nil { + assert.Nil(t, result, "should be nil") + } else { + require.NotNil(t, result, "should not be nil") + assert.Equal(t, *tt.expected, *result, "should return correct value") + } + }) + } +} + +// TestMergeSparsePatterns tests pattern deduplication and merging. +func TestMergeSparsePatterns(t *testing.T) { + t.Run("merges unique patterns", func(t *testing.T) { + result := mergeSparsePatterns([]string{".github/"}, "src/\ndocs/") + assert.Equal(t, []string{".github/", "src/", "docs/"}, result, "should contain all unique patterns") + }) + + t.Run("deduplicates patterns", func(t *testing.T) { + result := mergeSparsePatterns([]string{".github/"}, ".github/\nsrc/") + assert.Equal(t, []string{".github/", "src/"}, result, "should deduplicate .github/") + }) + + t.Run("nil existing with new patterns", func(t *testing.T) { + result := mergeSparsePatterns(nil, "src/\ndocs/") + assert.Equal(t, []string{"src/", "docs/"}, result, "should return new patterns") + }) + + t.Run("empty new patterns preserves existing", func(t *testing.T) { + result := mergeSparsePatterns([]string{"src/"}, "") + assert.Equal(t, []string{"src/"}, result, "should preserve existing patterns") + }) +} diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index aa1f1720d00..6d7b0653e7f 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -145,7 +145,7 @@ func (c *Compiler) buildInitialWorkflowData( agentImportSpec = "" } - return &WorkflowData{ + workflowData := &WorkflowData{ Name: toolsResult.workflowName, FrontmatterName: toolsResult.frontmatterName, FrontmatterYAML: strings.Join(result.FrontmatterLines, "\n"), @@ -183,6 +183,19 @@ func (c *Compiler) buildInitialWorkflowData( ActionMode: c.actionMode, InlinedImports: inlinedImports, } + + // Populate checkout configs from parsed frontmatter. + // Fall back to raw frontmatter parsing when full ParseFrontmatterConfig fails + // (e.g. due to unrecognised tool config shapes like bash: ["*"]). + if toolsResult.parsedFrontmatter != nil { + workflowData.CheckoutConfigs = toolsResult.parsedFrontmatter.CheckoutConfigs + } else if rawCheckout, ok := result.Frontmatter["checkout"]; ok { + if configs, err := ParseCheckoutConfigs(rawCheckout); err == nil { + workflowData.CheckoutConfigs = configs + } + } + + return workflowData } // resolveInlinedImports returns true if inlined-imports is enabled. diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index cdd43bf97e4..7c78fa2ada4 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -462,6 +462,7 @@ type WorkflowData struct { ActionMode ActionMode // action mode for workflow compilation (dev, release, script) HasExplicitGitHubTool bool // true if tools.github was explicitly configured in frontmatter InlinedImports bool // if true, inline all imports at compile time (from inlined-imports frontmatter field) + CheckoutConfigs []*CheckoutConfig // user-configured checkout settings from frontmatter } // BaseSafeOutputConfig holds common configuration fields for all safe output types diff --git a/pkg/workflow/compiler_yaml_main_job.go b/pkg/workflow/compiler_yaml_main_job.go index 53bcb718e40..ff45765315a 100644 --- a/pkg/workflow/compiler_yaml_main_job.go +++ b/pkg/workflow/compiler_yaml_main_job.go @@ -15,24 +15,19 @@ func (c *Compiler) generateMainJobSteps(yaml *strings.Builder, data *WorkflowDat needsCheckout := c.shouldAddCheckoutStep(data) compilerYamlLog.Printf("Checkout step needed: %t", needsCheckout) + // Build a CheckoutManager with any user-configured checkouts + checkoutMgr := NewCheckoutManager(data.CheckoutConfigs) + // Add checkout step first if needed if needsCheckout { - yaml.WriteString(" - name: Checkout repository\n") - fmt.Fprintf(yaml, " uses: %s\n", GetActionPin("actions/checkout")) - // Always add with section for persist-credentials - yaml.WriteString(" with:\n") - yaml.WriteString(" persist-credentials: false\n") - // In trial mode without cloning, checkout the logical repo if specified - if c.trialMode { - if c.trialLogicalRepoSlug != "" { - fmt.Fprintf(yaml, " repository: %s\n", c.trialLogicalRepoSlug) - // trialTargetRepoName := strings.Split(c.trialLogicalRepoSlug, "/") - // if len(trialTargetRepoName) == 2 { - // yaml.WriteString(fmt.Sprintf(" path: %s\n", trialTargetRepoName[1])) - // } - } - effectiveToken := getEffectiveGitHubToken("") - fmt.Fprintf(yaml, " token: %s\n", effectiveToken) + // Emit the default workspace checkout, applying any user-supplied overrides + defaultLines := checkoutMgr.GenerateDefaultCheckoutStep( + c.trialMode, + c.trialLogicalRepoSlug, + GetActionPin, + ) + for _, line := range defaultLines { + yaml.WriteString(line) } // Add CLI build steps in dev mode (after automatic checkout, before other steps) @@ -48,6 +43,12 @@ func (c *Compiler) generateMainJobSteps(yaml *strings.Builder, data *WorkflowDat } } + // Emit additional (non-default) user-configured checkouts + additionalLines := checkoutMgr.GenerateAdditionalCheckoutSteps(GetActionPin) + for _, line := range additionalLines { + yaml.WriteString(line) + } + // Add checkout steps for repository imports // Each repository import needs to be checked out into a temporary folder // so the merge script can copy files from it diff --git a/pkg/workflow/frontmatter_types.go b/pkg/workflow/frontmatter_types.go index a9ec3ae5108..2b3bd808926 100644 --- a/pkg/workflow/frontmatter_types.go +++ b/pkg/workflow/frontmatter_types.go @@ -164,6 +164,12 @@ type FrontmatterConfig struct { // Rate limiting configuration RateLimit *RateLimitConfig `json:"rate-limit,omitempty"` + + // Checkout configuration for the agent job. + // Controls how actions/checkout is invoked. + // Can be a single CheckoutConfig object or an array of CheckoutConfig objects. + Checkout any `json:"checkout,omitempty"` // Raw value (object or array) + CheckoutConfigs []*CheckoutConfig `json:"-"` // Parsed checkout configs (not in JSON) } // unmarshalFromMap converts a value from a map[string]any to a destination variable @@ -257,6 +263,15 @@ func ParseFrontmatterConfig(frontmatter map[string]any) (*FrontmatterConfig, err } } + // Parse checkout field - supports single object or array of objects + if config.Checkout != nil { + checkoutConfigs, err := ParseCheckoutConfigs(config.Checkout) + if err == nil { + config.CheckoutConfigs = checkoutConfigs + frontmatterTypesLog.Printf("Parsed checkout config: %d entries", len(checkoutConfigs)) + } + } + frontmatterTypesLog.Printf("Successfully parsed frontmatter config: name=%s, engine=%s", config.Name, config.Engine) return &config, nil }