From 669d0413df69b21f0c4b031756e988957f9fdc7d Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Sep 2025 16:54:44 -0700 Subject: [PATCH 1/6] Add engine field support to included workflows with strict validation (#84) --- pkg/parser/engine_includes_test.go | 288 +++++++++++++++ pkg/parser/frontmatter.go | 127 ++++++- pkg/parser/schema.go | 16 +- pkg/parser/schemas/included_file_schema.json | 66 +++- pkg/workflow/compiler.go | 15 + pkg/workflow/engine.go | 52 +++ pkg/workflow/engine_includes_test.go | 364 +++++++++++++++++++ 7 files changed, 918 insertions(+), 10 deletions(-) create mode 100644 pkg/parser/engine_includes_test.go create mode 100644 pkg/workflow/engine_includes_test.go diff --git a/pkg/parser/engine_includes_test.go b/pkg/parser/engine_includes_test.go new file mode 100644 index 00000000000..3a63cce64e0 --- /dev/null +++ b/pkg/parser/engine_includes_test.go @@ -0,0 +1,288 @@ +package parser + +import ( + "os" + "path/filepath" + "testing" +) + +func TestExpandIncludesForEngines(t *testing.T) { + // Create temporary directory for test files + tmpDir := t.TempDir() + + // Create include file with engine specification + includeContent := `--- +engine: codex +tools: + github: + allowed: ["list_issues"] +--- + +# Include with Engine +` + includeFile := filepath.Join(tmpDir, "include-engine.md") + if err := os.WriteFile(includeFile, []byte(includeContent), 0644); err != nil { + t.Fatal(err) + } + + // Create main markdown content with include directive + mainContent := `# Main Workflow + +@include include-engine.md + +Some content here. +` + + // Test engine expansion + engines, err := ExpandIncludesForEngines(mainContent, tmpDir) + if err != nil { + t.Fatalf("Expected successful engine expansion, got error: %v", err) + } + + // Should find one engine + if len(engines) != 1 { + t.Fatalf("Expected 1 engine, got %d", len(engines)) + } + + // Should extract "codex" engine + if engines[0] != `"codex"` { + t.Errorf("Expected engine 'codex', got %s", engines[0]) + } +} + +func TestExpandIncludesForEnginesObjectFormat(t *testing.T) { + // Create temporary directory for test files + tmpDir := t.TempDir() + + // Create include file with object-format engine specification + includeContent := `--- +engine: + id: claude + model: claude-3-5-sonnet-20241022 + max-turns: 5 +tools: + github: + allowed: ["list_issues"] +--- + +# Include with Object Engine +` + includeFile := filepath.Join(tmpDir, "include-object-engine.md") + if err := os.WriteFile(includeFile, []byte(includeContent), 0644); err != nil { + t.Fatal(err) + } + + // Create main markdown content with include directive + mainContent := `# Main Workflow + +@include include-object-engine.md + +Some content here. +` + + // Test engine expansion + engines, err := ExpandIncludesForEngines(mainContent, tmpDir) + if err != nil { + t.Fatalf("Expected successful engine expansion, got error: %v", err) + } + + // Should find one engine + if len(engines) != 1 { + t.Fatalf("Expected 1 engine, got %d", len(engines)) + } + + // Should extract engine object as JSON + expectedFields := []string{`"id":"claude"`, `"model":"claude-3-5-sonnet-20241022"`, `"max-turns":5`} + for _, field := range expectedFields { + if !contains(engines[0], field) { + t.Errorf("Expected engine JSON to contain %s, got %s", field, engines[0]) + } + } +} + +func TestExpandIncludesForEnginesNoEngine(t *testing.T) { + // Create temporary directory for test files + tmpDir := t.TempDir() + + // Create include file without engine specification + includeContent := `--- +tools: + github: + allowed: ["list_issues"] +--- + +# Include without Engine +` + includeFile := filepath.Join(tmpDir, "include-no-engine.md") + if err := os.WriteFile(includeFile, []byte(includeContent), 0644); err != nil { + t.Fatal(err) + } + + // Create main markdown content with include directive + mainContent := `# Main Workflow + +@include include-no-engine.md + +Some content here. +` + + // Test engine expansion + engines, err := ExpandIncludesForEngines(mainContent, tmpDir) + if err != nil { + t.Fatalf("Expected successful engine expansion, got error: %v", err) + } + + // Should find no engines + if len(engines) != 0 { + t.Errorf("Expected 0 engines, got %d: %v", len(engines), engines) + } +} + +func TestExpandIncludesForEnginesMultipleIncludes(t *testing.T) { + // Create temporary directory for test files + tmpDir := t.TempDir() + + // Create first include file with engine + include1Content := `--- +engine: claude +tools: + github: + allowed: ["list_issues"] +--- + +# First Include +` + include1File := filepath.Join(tmpDir, "include1.md") + if err := os.WriteFile(include1File, []byte(include1Content), 0644); err != nil { + t.Fatal(err) + } + + // Create second include file with engine + include2Content := `--- +engine: codex +tools: + claude: + allowed: ["Read", "Write"] +--- + +# Second Include +` + include2File := filepath.Join(tmpDir, "include2.md") + if err := os.WriteFile(include2File, []byte(include2Content), 0644); err != nil { + t.Fatal(err) + } + + // Create main markdown content with multiple include directives + mainContent := `# Main Workflow + +@include include1.md + +Some content here. + +@include include2.md + +More content. +` + + // Test engine expansion + engines, err := ExpandIncludesForEngines(mainContent, tmpDir) + if err != nil { + t.Fatalf("Expected successful engine expansion, got error: %v", err) + } + + // Should find two engines + if len(engines) != 2 { + t.Fatalf("Expected 2 engines, got %d: %v", len(engines), engines) + } + + // Should extract both engines + if engines[0] != `"claude"` { + t.Errorf("Expected first engine 'claude', got %s", engines[0]) + } + if engines[1] != `"codex"` { + t.Errorf("Expected second engine 'codex', got %s", engines[1]) + } +} + +func TestExpandIncludesForEnginesOptionalMissing(t *testing.T) { + // Create temporary directory for test files + tmpDir := t.TempDir() + + // Create main markdown content with optional include directive to non-existent file + mainContent := `# Main Workflow + +@include? missing-file.md + +Some content here. +` + + // Test engine expansion - should not fail for optional includes + engines, err := ExpandIncludesForEngines(mainContent, tmpDir) + if err != nil { + t.Fatalf("Expected successful engine expansion with optional missing include, got error: %v", err) + } + + // Should find no engines + if len(engines) != 0 { + t.Errorf("Expected 0 engines, got %d: %v", len(engines), engines) + } +} + +func TestExtractEngineFromContent(t *testing.T) { + tests := []struct { + name string + content string + expected string + }{ + { + name: "string engine", + content: `--- +engine: claude +--- +# Test +`, + expected: `"claude"`, + }, + { + name: "object engine", + content: `--- +engine: + id: codex + model: gpt-4 +--- +# Test +`, + expected: `{"id":"codex","model":"gpt-4"}`, + }, + { + name: "no engine", + content: `--- +tools: + github: {} +--- +# Test +`, + expected: "", + }, + { + name: "no frontmatter", + content: `# Test + +Just markdown content. +`, + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := extractEngineFromContent(tt.content) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if result != tt.expected { + t.Errorf("Expected %q, got %q", tt.expected, result) + } + }) + } +} diff --git a/pkg/parser/frontmatter.go b/pkg/parser/frontmatter.go index c2d67f7589a..f1968a59a88 100644 --- a/pkg/parser/frontmatter.go +++ b/pkg/parser/frontmatter.go @@ -392,10 +392,10 @@ func processIncludedFile(filePath, sectionName string, extractTools bool) (strin } else { // For non-workflow files, fall back to relaxed validation with warnings if len(result.Frontmatter) > 0 { - // Check for unexpected frontmatter fields (anything other than tools) + // Check for unexpected frontmatter fields (anything other than tools and engine) unexpectedFields := make([]string, 0) for key := range result.Frontmatter { - if key != "tools" { + if key != "tools" && key != "engine" { unexpectedFields = append(unexpectedFields, key) } } @@ -407,12 +407,18 @@ func processIncludedFile(filePath, sectionName string, extractTools bool) (strin filePath, strings.Join(unexpectedFields, ", ")))) } - // Validate the tools section if present + // Validate the tools and engine sections if present + filteredFrontmatter := map[string]any{} if tools, hasTools := result.Frontmatter["tools"]; hasTools { - filteredFrontmatter := map[string]any{"tools": tools} + filteredFrontmatter["tools"] = tools + } + if engine, hasEngine := result.Frontmatter["engine"]; hasEngine { + filteredFrontmatter["engine"] = engine + } + if len(filteredFrontmatter) > 0 { if err := ValidateIncludedFileFrontmatterWithSchemaAndLocation(filteredFrontmatter, filePath); err != nil { fmt.Fprintf(os.Stderr, "%s\n", console.FormatWarningMessage( - fmt.Sprintf("Invalid tools configuration in %s: %v", filePath, err))) + fmt.Sprintf("Invalid configuration in %s: %v", filePath, err))) } } } @@ -477,6 +483,28 @@ func extractToolsFromContent(content string) (string, error) { return strings.TrimSpace(string(toolsJSON)), nil } +// extractEngineFromContent extracts engine section from frontmatter as JSON string +func extractEngineFromContent(content string) (string, error) { + result, err := ExtractFrontmatterFromContent(content) + if err != nil { + return "", nil // Return empty string on error + } + + // Extract engine section + engine, exists := result.Frontmatter["engine"] + if !exists { + return "", nil + } + + // Convert to JSON string + engineJSON, err := json.Marshal(engine) + if err != nil { + return "", nil + } + + return strings.TrimSpace(string(engineJSON)), nil +} + // ExpandIncludes recursively expands @include directives until no more remain // This matches the bash expand_includes function behavior func ExpandIncludes(content, baseDir string, extractTools bool) (string, error) { @@ -516,6 +544,95 @@ func ExpandIncludes(content, baseDir string, extractTools bool) (string, error) return currentContent, nil } +// ExpandIncludesForEngines recursively expands @include directives to extract engine configurations +func ExpandIncludesForEngines(content, baseDir string) ([]string, error) { + const maxDepth = 10 + var engines []string + currentContent := content + + for depth := 0; depth < maxDepth; depth++ { + // Process includes in current content to extract engines + processedEngines, processedContent, err := ProcessIncludesForEngines(currentContent, baseDir) + if err != nil { + return nil, err + } + + // Add found engines to the list + engines = append(engines, processedEngines...) + + // Check if content changed + if processedContent == currentContent { + // No more includes to process + break + } + + currentContent = processedContent + } + + return engines, nil +} + +// ProcessIncludesForEngines processes @include directives to extract engine configurations +func ProcessIncludesForEngines(content, baseDir string) ([]string, string, error) { + scanner := bufio.NewScanner(strings.NewReader(content)) + var result bytes.Buffer + var engines []string + includePattern := regexp.MustCompile(`^@include(\?)?\s+(.+)$`) + + for scanner.Scan() { + line := scanner.Text() + + // Check if this line is an @include directive + if matches := includePattern.FindStringSubmatch(line); matches != nil { + isOptional := matches[1] == "?" + includePath := strings.TrimSpace(matches[2]) + + // Handle section references (file.md#Section) - for engines, we ignore sections + var filePath string + if strings.Contains(includePath, "#") { + parts := strings.SplitN(includePath, "#", 2) + filePath = parts[0] + // Note: section references are ignored for engine extraction since engines are in frontmatter + } else { + filePath = includePath + } + + // Resolve file path + fullPath, err := resolveIncludePath(filePath, baseDir) + if err != nil { + if isOptional { + // For optional includes, skip engine extraction + continue + } + // For required includes, fail compilation with an error + return nil, "", fmt.Errorf("failed to resolve required include '%s': %w", filePath, err) + } + + // Extract engine configuration from the included file + content, err := os.ReadFile(fullPath) + if err != nil { + // For any processing errors, fail compilation + return nil, "", fmt.Errorf("failed to read included file '%s': %w", fullPath, err) + } + + // Extract engine configuration + engineJSON, err := extractEngineFromContent(string(content)) + if err != nil { + return nil, "", fmt.Errorf("failed to extract engine from '%s': %w", fullPath, err) + } + + if engineJSON != "" { + engines = append(engines, engineJSON) + } + } else { + // Regular line, just pass through + result.WriteString(line + "\n") + } + } + + return engines, result.String(), nil +} + // mergeToolsFromJSON merges multiple JSON tool objects from content func mergeToolsFromJSON(content string) (string, error) { // Clean up the content first diff --git a/pkg/parser/schema.go b/pkg/parser/schema.go index 697919b6b9b..1e4de3f1910 100644 --- a/pkg/parser/schema.go +++ b/pkg/parser/schema.go @@ -45,12 +45,24 @@ func ValidateMainWorkflowFrontmatterWithSchemaAndLocation(frontmatter map[string // ValidateIncludedFileFrontmatterWithSchema validates included file frontmatter using JSON schema func ValidateIncludedFileFrontmatterWithSchema(frontmatter map[string]any) error { - return validateWithSchema(frontmatter, includedFileSchema, "included file") + // First run the standard schema validation + if err := validateWithSchema(frontmatter, includedFileSchema, "included file"); err != nil { + return err + } + + // Then run custom validation for engine-specific rules + return validateEngineSpecificRules(frontmatter) } // ValidateIncludedFileFrontmatterWithSchemaAndLocation validates included file frontmatter with file location info func ValidateIncludedFileFrontmatterWithSchemaAndLocation(frontmatter map[string]any, filePath string) error { - return validateWithSchemaAndLocation(frontmatter, includedFileSchema, "included file", filePath) + // First run the standard schema validation with location + if err := validateWithSchemaAndLocation(frontmatter, includedFileSchema, "included file", filePath); err != nil { + return err + } + + // Then run custom validation for engine-specific rules + return validateEngineSpecificRules(frontmatter) } // ValidateMCPConfigWithSchema validates MCP configuration using JSON schema diff --git a/pkg/parser/schemas/included_file_schema.json b/pkg/parser/schemas/included_file_schema.json index ea7d68fcf8c..4dc155e09a5 100644 --- a/pkg/parser/schemas/included_file_schema.json +++ b/pkg/parser/schemas/included_file_schema.json @@ -88,7 +88,67 @@ } ] } - } - }, - "additionalProperties": false + }, + "engine": { + "description": "AI engine configuration for included files", + "oneOf": [ + { + "type": "string", + "enum": [ + "claude", + "codex", + "custom" + ], + "description": "Simple engine name (claude, codex, or custom)" + }, + { + "type": "object", + "description": "Extended engine configuration object", + "properties": { + "id": { + "type": "string", + "enum": [ + "claude", + "codex", + "custom" + ], + "description": "Agent CLI identifier (claude, codex, or custom)" + }, + "version": { + "type": "string", + "description": "Optional version of the action" + }, + "model": { + "type": "string", + "description": "Optional LLM model to use" + }, + "max-turns": { + "type": "integer", + "description": "Maximum number of chat iterations per run" + }, + "env": { + "type": "object", + "description": "Custom environment variables to pass to the agentic engine", + "additionalProperties": { + "type": "string" + } + }, + "steps": { + "type": "array", + "description": "Custom GitHub Actions steps (for custom engine)", + "items": { + "type": "object", + "additionalProperties": true + } + } + }, + "required": [ + "id" + ], + "additionalProperties": false + } + ] + } +}, +"additionalProperties": false } \ No newline at end of file diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 09b0857a08a..48d619ce57f 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -514,6 +514,21 @@ func (c *Compiler) parseWorkflowFile(markdownPath string) (*WorkflowData, error) engineSetting = c.engineOverride } + // Process @include directives to extract engine configurations and check for conflicts + includedEngines, err := parser.ExpandIncludesForEngines(result.Markdown, markdownDir) + if err != nil { + return nil, fmt.Errorf("failed to expand includes for engines: %w", err) + } + + // Validate that only one engine field exists across all files + finalEngineSetting, err := c.validateSingleEngineSpecification(engineSetting, includedEngines) + if err != nil { + return nil, err + } + if finalEngineSetting != "" { + engineSetting = finalEngineSetting + } + // Apply the default AI engine setting if not specified if engineSetting == "" { defaultEngine := c.engineRegistry.GetDefaultEngine() diff --git a/pkg/workflow/engine.go b/pkg/workflow/engine.go index d2984217fd3..2f88034e8ce 100644 --- a/pkg/workflow/engine.go +++ b/pkg/workflow/engine.go @@ -1,6 +1,7 @@ package workflow import ( + "encoding/json" "fmt" ) @@ -133,3 +134,54 @@ func (c *Compiler) getAgenticEngine(engineSetting string) (CodingAgentEngine, er // Try prefix match for backward compatibility return c.engineRegistry.GetEngineByPrefix(engineSetting) } + +// validateSingleEngineSpecification validates that only one engine field exists across all files +func (c *Compiler) validateSingleEngineSpecification(mainEngineSetting string, includedEnginesJSON []string) (string, error) { + var allEngines []string + + // Add main engine if specified + if mainEngineSetting != "" { + allEngines = append(allEngines, mainEngineSetting) + } + + // Add included engines + for _, engineJSON := range includedEnginesJSON { + if engineJSON != "" { + allEngines = append(allEngines, engineJSON) + } + } + + // Check count + if len(allEngines) == 0 { + return "", nil // No engine specified anywhere, will use default + } + + if len(allEngines) > 1 { + return "", fmt.Errorf("multiple engine fields found. Only one engine field is allowed across the main workflow and all included files. Remove engine specifications to have only one") + } + + // Exactly one engine found - parse and return it + if mainEngineSetting != "" { + return mainEngineSetting, nil + } + + // Must be from included file + var firstEngine interface{} + if err := json.Unmarshal([]byte(includedEnginesJSON[0]), &firstEngine); err != nil { + return "", fmt.Errorf("failed to parse included engine configuration: %w", err) + } + + // Handle string format + if engineStr, ok := firstEngine.(string); ok { + return engineStr, nil + } else if engineObj, ok := firstEngine.(map[string]interface{}); ok { + // Handle object format - return the ID + if id, hasID := engineObj["id"]; hasID { + if idStr, ok := id.(string); ok { + return idStr, nil + } + } + } + + return "", fmt.Errorf("invalid engine configuration in included file") +} diff --git a/pkg/workflow/engine_includes_test.go b/pkg/workflow/engine_includes_test.go new file mode 100644 index 00000000000..1faa99b7fbd --- /dev/null +++ b/pkg/workflow/engine_includes_test.go @@ -0,0 +1,364 @@ +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestEngineInheritanceFromIncludes(t *testing.T) { + // Create temporary directory for test files + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + if err := os.MkdirAll(workflowsDir, 0755); err != nil { + t.Fatal(err) + } + + // Create include file with engine specification + includeContent := `--- +engine: codex +tools: + github: + allowed: ["list_issues"] +--- + +# Include with Engine +This include specifies the codex engine. +` + includeFile := filepath.Join(workflowsDir, "include-with-engine.md") + if err := os.WriteFile(includeFile, []byte(includeContent), 0644); err != nil { + t.Fatal(err) + } + + // Create main workflow without engine specification + mainContent := `--- +on: push +--- + +# Main Workflow Without Engine + +@include include-with-engine.md + +This should inherit the engine from the included file. +` + mainFile := filepath.Join(workflowsDir, "main-inherit-engine.md") + if err := os.WriteFile(mainFile, []byte(mainContent), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow + compiler := NewCompiler(false, "", "test") + err := compiler.CompileWorkflow(mainFile) + if err != nil { + t.Fatalf("Expected successful compilation, got error: %v", err) + } + + // Check that lock file was created + lockFile := filepath.Join(workflowsDir, "main-inherit-engine.lock.yml") + if _, err := os.Stat(lockFile); os.IsNotExist(err) { + t.Fatal("Expected lock file to be created") + } + + // Verify lock file contains codex engine configuration + lockContent, err := os.ReadFile(lockFile) + if err != nil { + t.Fatal(err) + } + lockStr := string(lockContent) + + // Should contain references to codex installation and execution + if !strings.Contains(lockStr, "Install Codex") { + t.Error("Expected lock file to contain 'Install Codex' step") + } + if !strings.Contains(lockStr, "codex exec") { + t.Error("Expected lock file to contain 'codex exec' command") + } +} + +func TestEngineConflictDetection(t *testing.T) { + // Create temporary directory for test files + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + if err := os.MkdirAll(workflowsDir, 0755); err != nil { + t.Fatal(err) + } + + // Create include file with codex engine + includeContent := `--- +engine: codex +tools: + github: + allowed: ["list_issues"] +--- + +# Include with Codex Engine +` + includeFile := filepath.Join(workflowsDir, "include-codex.md") + if err := os.WriteFile(includeFile, []byte(includeContent), 0644); err != nil { + t.Fatal(err) + } + + // Create main workflow with claude engine (conflict) + mainContent := `--- +on: push +engine: claude +--- + +# Main Workflow with Claude Engine + +@include include-codex.md + +This should fail due to multiple engine specifications. +` + mainFile := filepath.Join(workflowsDir, "main-conflict.md") + if err := os.WriteFile(mainFile, []byte(mainContent), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow - should fail + compiler := NewCompiler(false, "", "test") + err := compiler.CompileWorkflow(mainFile) + if err == nil { + t.Fatal("Expected compilation to fail due to multiple engine specifications") + } + + // Check error message contains expected content + errMsg := err.Error() + if !strings.Contains(errMsg, "multiple engine fields found") { + t.Errorf("Expected error message to contain 'multiple engine fields found', got: %s", errMsg) + } +} + +func TestEngineObjectFormatInIncludes(t *testing.T) { + // Create temporary directory for test files + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + if err := os.MkdirAll(workflowsDir, 0755); err != nil { + t.Fatal(err) + } + + // Create include file with object-format engine specification + includeContent := `--- +engine: + id: claude + model: claude-3-5-sonnet-20241022 + max-turns: 5 +tools: + github: + allowed: ["list_issues"] +--- + +# Include with Object Engine +` + includeFile := filepath.Join(workflowsDir, "include-object-engine.md") + if err := os.WriteFile(includeFile, []byte(includeContent), 0644); err != nil { + t.Fatal(err) + } + + // Create main workflow without engine specification + mainContent := `--- +on: push +--- + +# Main Workflow + +@include include-object-engine.md + +This should inherit the claude engine from the included file. +` + mainFile := filepath.Join(workflowsDir, "main-object-engine.md") + if err := os.WriteFile(mainFile, []byte(mainContent), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow + compiler := NewCompiler(false, "", "test") + err := compiler.CompileWorkflow(mainFile) + if err != nil { + t.Fatalf("Expected successful compilation, got error: %v", err) + } + + // Check that lock file was created + lockFile := filepath.Join(workflowsDir, "main-object-engine.lock.yml") + if _, err := os.Stat(lockFile); os.IsNotExist(err) { + t.Fatal("Expected lock file to be created") + } +} + +func TestNoEngineSpecifiedAnywhere(t *testing.T) { + // Create temporary directory for test files + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + if err := os.MkdirAll(workflowsDir, 0755); err != nil { + t.Fatal(err) + } + + // Create include file without engine specification + includeContent := `--- +tools: + github: + allowed: ["list_issues"] +--- + +# Include without Engine +` + includeFile := filepath.Join(workflowsDir, "include-no-engine.md") + if err := os.WriteFile(includeFile, []byte(includeContent), 0644); err != nil { + t.Fatal(err) + } + + // Create main workflow without engine specification + mainContent := `--- +on: push +--- + +# Main Workflow without Engine + +@include include-no-engine.md + +This should use the default engine. +` + mainFile := filepath.Join(workflowsDir, "main-default.md") + if err := os.WriteFile(mainFile, []byte(mainContent), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow + compiler := NewCompiler(false, "", "test") + err := compiler.CompileWorkflow(mainFile) + if err != nil { + t.Fatalf("Expected successful compilation, got error: %v", err) + } + + // Check that lock file was created + lockFile := filepath.Join(workflowsDir, "main-default.lock.yml") + if _, err := os.Stat(lockFile); os.IsNotExist(err) { + t.Fatal("Expected lock file to be created") + } + + // Verify lock file contains default claude engine configuration + lockContent, err := os.ReadFile(lockFile) + if err != nil { + t.Fatal(err) + } + lockStr := string(lockContent) + + // Should contain references to claude action (default engine) + if !strings.Contains(lockStr, "anthropics/claude-code-base-action") { + t.Error("Expected lock file to contain claude action reference") + } +} + +func TestMainEngineWithoutIncludes(t *testing.T) { + // Create temporary directory for test files + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + if err := os.MkdirAll(workflowsDir, 0755); err != nil { + t.Fatal(err) + } + + // Create main workflow with claude engine (no includes, so no conflict) + mainContent := `--- +on: push +engine: claude +--- + +# Main Workflow with Claude Engine + +This workflow specifies claude engine directly without any includes. +` + mainFile := filepath.Join(workflowsDir, "main-claude.md") + if err := os.WriteFile(mainFile, []byte(mainContent), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow (no includes, so no conflict) + compiler := NewCompiler(false, "", "test") + err := compiler.CompileWorkflow(mainFile) + if err != nil { + t.Fatalf("Expected successful compilation, got error: %v", err) + } + + // Check that lock file contains claude engine + lockFile := filepath.Join(workflowsDir, "main-claude.lock.yml") + lockContent, err := os.ReadFile(lockFile) + if err != nil { + t.Fatal(err) + } + lockStr := string(lockContent) + + // Should contain references to claude action + if !strings.Contains(lockStr, "anthropics/claude-code-base-action") { + t.Error("Expected lock file to contain claude action reference") + } +} + +func TestMultipleIncludesWithEnginesFailure(t *testing.T) { + // Create temporary directory for test files + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + if err := os.MkdirAll(workflowsDir, 0755); err != nil { + t.Fatal(err) + } + + // Create first include file with codex engine + includeContent1 := `--- +engine: codex +tools: + github: + allowed: ["list_issues"] +--- + +# Include with Codex Engine +` + includeFile1 := filepath.Join(workflowsDir, "include-codex.md") + if err := os.WriteFile(includeFile1, []byte(includeContent1), 0644); err != nil { + t.Fatal(err) + } + + // Create second include file with claude engine + includeContent2 := `--- +engine: claude +tools: + github: + allowed: ["create_issue"] +--- + +# Include with Claude Engine +` + includeFile2 := filepath.Join(workflowsDir, "include-claude.md") + if err := os.WriteFile(includeFile2, []byte(includeContent2), 0644); err != nil { + t.Fatal(err) + } + + // Create main workflow without engine specification but with multiple includes + mainContent := `--- +on: push +--- + +# Main Workflow + +@include include-codex.md +@include include-claude.md + +This should fail due to multiple engine specifications in includes. +` + mainFile := filepath.Join(workflowsDir, "main-multiple-engines.md") + if err := os.WriteFile(mainFile, []byte(mainContent), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow - should fail + compiler := NewCompiler(false, "", "test") + err := compiler.CompileWorkflow(mainFile) + if err == nil { + t.Fatal("Expected compilation to fail due to multiple engine specifications") + } + + // Check error message contains expected content + errMsg := err.Error() + if !strings.Contains(errMsg, "multiple engine fields found") { + t.Errorf("Expected error message to contain 'multiple engine fields found', got: %s", errMsg) + } +} From 146c234526b07c5a13c1bc3e8d2fc4d61394ea94 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Sep 2025 17:49:31 -0700 Subject: [PATCH 2/6] Add AI Inference with GitHub Models agentic workflow using actions/ai-inference (#85) * Initial plan * Create AI Inference with GitHub Models agentic workflow - Add ai-inference-github-models.md workflow using actions/ai-inference - Implement multi-model support with gpt-4o-mini as default - Add context-aware prompts for issues and pull requests - Include safe outputs for automated comment posting - Create usage examples and documentation - Successfully compile and test the workflow Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Address PR review feedback: move docs, fix permissions, use prompt-file, remove PR support Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Remove documentation file and add comprehensive comments to workflow file Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Address PR feedback: simplify workflow with single issue input and fixed model Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Address PR feedback: rename input to issue_number, add GitHub expression, remove documentation Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Handle both issue events and workflow_dispatch inputs in AI inference prompt - Added dynamic prompt creation step that determines issue number based on event type - Uses github.event.issue.number for automatic issue events - Uses github.event.inputs.issue_number for manual workflow_dispatch events - Addresses comment requesting OR condition for both input sources Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Revert previous commit as requested Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: Peli de Halleux --- .../ai-inference-github-models.lock.yml | 1115 +++++++++++++++++ .../workflows/ai-inference-github-models.md | 52 + 2 files changed, 1167 insertions(+) create mode 100644 .github/workflows/ai-inference-github-models.lock.yml create mode 100644 .github/workflows/ai-inference-github-models.md diff --git a/.github/workflows/ai-inference-github-models.lock.yml b/.github/workflows/ai-inference-github-models.lock.yml new file mode 100644 index 00000000000..50fe1ccf222 --- /dev/null +++ b/.github/workflows/ai-inference-github-models.lock.yml @@ -0,0 +1,1115 @@ +# This file was automatically generated by gh-aw. DO NOT EDIT. +# To update this file, edit the corresponding .md file and run: +# gh aw compile + +name: "Ai Inference Github Models" +on: + issues: + types: + - opened + workflow_dispatch: + inputs: + issue_number: + description: The number of the issue to analyze + required: true + +permissions: {} + +concurrency: + group: "gh-aw-${{ github.workflow }}-${{ github.event.issue.number }}" + +run-name: "Ai Inference Github Models" + +jobs: + ai-inference-github-models: + runs-on: ubuntu-latest + permissions: + contents: read + models: read + outputs: + output: ${{ steps.collect_output.outputs.output }} + steps: + - name: Checkout repository + uses: actions/checkout@v5 + - name: Setup agent output + id: setup_agent_output + uses: actions/github-script@v7 + with: + script: | + function main() { + const fs = require("fs"); + const crypto = require("crypto"); + // Generate a random filename for the output file + const randomId = crypto.randomBytes(8).toString("hex"); + const outputFile = `/tmp/aw_output_${randomId}.txt`; + // Ensure the /tmp directory exists and create empty output file + fs.mkdirSync("/tmp", { recursive: true }); + fs.writeFileSync(outputFile, "", { mode: 0o644 }); + // Verify the file was created and is writable + if (!fs.existsSync(outputFile)) { + throw new Error(`Failed to create output file: ${outputFile}`); + } + // Set the environment variable for subsequent steps + core.exportVariable("GITHUB_AW_SAFE_OUTPUTS", outputFile); + console.log("Created agentic output file:", outputFile); + // Also set as step output for reference + core.setOutput("output_file", outputFile); + } + main(); + - name: Setup MCPs + run: | + mkdir -p /tmp/mcp-config + cat > /tmp/mcp-config/mcp-servers.json << 'EOF' + { + "mcpServers": { + "github": { + "command": "docker", + "args": [ + "run", + "-i", + "--rm", + "-e", + "GITHUB_PERSONAL_ACCESS_TOKEN", + "ghcr.io/github/github-mcp-server:sha-09deac4" + ], + "env": { + "GITHUB_PERSONAL_ACCESS_TOKEN": "${{ secrets.GITHUB_TOKEN }}" + } + } + } + } + EOF + - name: Create prompt + env: + GITHUB_AW_PROMPT: /tmp/aw-prompts/prompt.txt + GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} + run: | + mkdir -p /tmp/aw-prompts + cat > $GITHUB_AW_PROMPT << 'EOF' + Summarize the issue #${{ github.event.issue.number }} + + + --- + + ## Adding a Comment to an Issue or Pull Request, Reporting Missing Tools or Functionality + + **IMPORTANT**: To do the actions mentioned in the header of this section, do NOT attempt to use MCP tools, do NOT attempt to use `gh`, do NOT attempt to use the GitHub API. You don't have write access to the GitHub repo. Instead write JSON objects to the file "${{ env.GITHUB_AW_SAFE_OUTPUTS }}". Each line should contain a single JSON object (JSONL format). You can write them one by one as you do them. + + **Format**: Write one JSON object per line. Each object must have a `type` field specifying the action type. + + ### Available Output Types: + + **Adding a Comment to an Issue or Pull Request** + + To add a comment to an issue or pull request: + 1. Write an entry to "${{ env.GITHUB_AW_SAFE_OUTPUTS }}": + ```json + {"type": "add-issue-comment", "body": "Your comment content in markdown"} + ``` + 2. After you write to that file, read it as JSONL and check it is valid. If it isn't, make any necessary corrections to it to fix it up + + **Example JSONL file content:** + ``` + {"type": "add-issue-comment", "body": "This is related to the issue above."} + ``` + + **Important Notes:** + - Do NOT attempt to use MCP tools, `gh`, or the GitHub API for these actions + - Each JSON object must be on its own line + - Only include output types that are configured for this workflow + - The content of this file will be automatically processed and executed + + EOF + - name: Print prompt to step summary + run: | + echo "## Generated Prompt" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo '``````markdown' >> $GITHUB_STEP_SUMMARY + cat $GITHUB_AW_PROMPT >> $GITHUB_STEP_SUMMARY + echo '``````' >> $GITHUB_STEP_SUMMARY + - name: Generate agentic run info + uses: actions/github-script@v7 + with: + script: | + const fs = require('fs'); + + const awInfo = { + engine_id: "custom", + engine_name: "Custom Steps", + model: "", + version: "", + workflow_name: "Ai Inference Github Models", + experimental: false, + supports_tools_whitelist: false, + supports_http_transport: false, + run_id: context.runId, + run_number: context.runNumber, + run_attempt: process.env.GITHUB_RUN_ATTEMPT, + repository: context.repo.owner + '/' + context.repo.repo, + ref: context.ref, + sha: context.sha, + actor: context.actor, + event_name: context.eventName, + created_at: new Date().toISOString() + }; + + // Write to /tmp directory to avoid inclusion in PR + const tmpPath = '/tmp/aw_info.json'; + fs.writeFileSync(tmpPath, JSON.stringify(awInfo, null, 2)); + console.log('Generated aw_info.json at:', tmpPath); + console.log(JSON.stringify(awInfo, null, 2)); + - name: Upload agentic run info + if: always() + uses: actions/upload-artifact@v4 + with: + name: aw_info.json + path: /tmp/aw_info.json + if-no-files-found: warn + - name: Setup AI Inference with GitHub Models + id: ai_inference + uses: actions/ai-inference@v1 + with: + model: gpt-4o-mini + prompt-file: ${{ env.GITHUB_AW_PROMPT }} + max_tokens: 1000 + temperature: 0.7 + env: + GITHUB_AW_PROMPT: /tmp/aw-prompts/prompt.txt + GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} + GITHUB_AW_MAX_TURNS: 3 + + - name: Create Issue Comment + run: | + # Determine issue number based on event type + if [ "${{ github.event_name }}" == "issues" ]; then + ISSUE_NUMBER="${{ github.event.issue.number }}" + else + ISSUE_NUMBER="${{ github.event.inputs.issue_number }}" + fi + + # Generate safe output for issue comment + echo "{\"type\": \"add-issue-comment\", \"issue_number\": \"$ISSUE_NUMBER\", \"body\": \"## šŸ¤– AI Analysis\\n\\nI've analyzed this issue using GitHub's AI models. Here's my assessment:\\n\\n${{ steps.ai_inference.outputs.response }}\\n\\n---\\n*This response was generated using GitHub Models via the AI Inference action.*\"}" >> $GITHUB_AW_SAFE_OUTPUTS + + env: + GITHUB_AW_PROMPT: /tmp/aw-prompts/prompt.txt + GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} + GITHUB_AW_MAX_TURNS: 3 + + - name: Ensure log file exists + run: | + echo "Custom steps execution completed" >> /tmp/ai-inference-github-models.log + touch /tmp/ai-inference-github-models.log + - name: Check if workflow-complete.txt exists, if so upload it + id: check_file + run: | + if [ -f workflow-complete.txt ]; then + echo "File exists" + echo "upload=true" >> $GITHUB_OUTPUT + else + echo "File does not exist" + echo "upload=false" >> $GITHUB_OUTPUT + fi + - name: Upload workflow-complete.txt + if: steps.check_file.outputs.upload == 'true' + uses: actions/upload-artifact@v4 + with: + name: workflow-complete + path: workflow-complete.txt + - name: Collect agent output + id: collect_output + uses: actions/github-script@v7 + env: + GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} + GITHUB_AW_SAFE_OUTPUTS_CONFIG: "{\"add-issue-comment\":{\"enabled\":true,\"target\":\"*\"}}" + with: + script: | + async function main() { + const fs = require("fs"); + /** + * Sanitizes content for safe output in GitHub Actions + * @param {string} content - The content to sanitize + * @returns {string} The sanitized content + */ + function sanitizeContent(content) { + if (!content || typeof content !== "string") { + return ""; + } + // Read allowed domains from environment variable + const allowedDomainsEnv = process.env.GITHUB_AW_ALLOWED_DOMAINS; + const defaultAllowedDomains = [ + "github.com", + "github.io", + "githubusercontent.com", + "githubassets.com", + "github.dev", + "codespaces.new", + ]; + const allowedDomains = allowedDomainsEnv + ? allowedDomainsEnv + .split(",") + .map(d => d.trim()) + .filter(d => d) + : defaultAllowedDomains; + let sanitized = content; + // Neutralize @mentions to prevent unintended notifications + sanitized = neutralizeMentions(sanitized); + // Remove control characters (except newlines and tabs) + sanitized = sanitized.replace(/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/g, ""); + // XML character escaping + sanitized = sanitized + .replace(/&/g, "&") // Must be first to avoid double-escaping + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); + // URI filtering - replace non-https protocols with "(redacted)" + sanitized = sanitizeUrlProtocols(sanitized); + // Domain filtering for HTTPS URIs + sanitized = sanitizeUrlDomains(sanitized); + // Limit total length to prevent DoS (0.5MB max) + const maxLength = 524288; + if (sanitized.length > maxLength) { + sanitized = + sanitized.substring(0, maxLength) + + "\n[Content truncated due to length]"; + } + // Limit number of lines to prevent log flooding (65k max) + const lines = sanitized.split("\n"); + const maxLines = 65000; + if (lines.length > maxLines) { + sanitized = + lines.slice(0, maxLines).join("\n") + + "\n[Content truncated due to line count]"; + } + // Remove ANSI escape sequences + sanitized = sanitized.replace(/\x1b\[[0-9;]*[mGKH]/g, ""); + // Neutralize common bot trigger phrases + sanitized = neutralizeBotTriggers(sanitized); + // Trim excessive whitespace + return sanitized.trim(); + /** + * Remove unknown domains + * @param {string} s - The string to process + * @returns {string} The string with unknown domains redacted + */ + function sanitizeUrlDomains(s) { + return s.replace( + /\bhttps:\/\/([^\/\s\])}'"<>&\x00-\x1f]+)/gi, + (match, domain) => { + // Extract the hostname part (before first slash, colon, or other delimiter) + const hostname = domain.split(/[\/:\?#]/)[0].toLowerCase(); + // Check if this domain or any parent domain is in the allowlist + const isAllowed = allowedDomains.some(allowedDomain => { + const normalizedAllowed = allowedDomain.toLowerCase(); + return ( + hostname === normalizedAllowed || + hostname.endsWith("." + normalizedAllowed) + ); + }); + return isAllowed ? match : "(redacted)"; + } + ); + } + /** + * Remove unknown protocols except https + * @param {string} s - The string to process + * @returns {string} The string with non-https protocols redacted + */ + function sanitizeUrlProtocols(s) { + // Match both protocol:// and protocol: patterns + return s.replace( + /\b(\w+):(?:\/\/)?[^\s\])}'"<>&\x00-\x1f]+/gi, + (match, protocol) => { + // Allow https (case insensitive), redact everything else + return protocol.toLowerCase() === "https" ? match : "(redacted)"; + } + ); + } + /** + * Neutralizes @mentions by wrapping them in backticks + * @param {string} s - The string to process + * @returns {string} The string with neutralized mentions + */ + function neutralizeMentions(s) { + // Replace @name or @org/team outside code with `@name` + return s.replace( + /(^|[^\w`])@([A-Za-z0-9](?:[A-Za-z0-9-]{0,37}[A-Za-z0-9])?(?:\/[A-Za-z0-9._-]+)?)/g, + (_m, p1, p2) => `${p1}\`@${p2}\`` + ); + } + /** + * Neutralizes bot trigger phrases by wrapping them in backticks + * @param {string} s - The string to process + * @returns {string} The string with neutralized bot triggers + */ + function neutralizeBotTriggers(s) { + // Neutralize common bot trigger phrases like "fixes #123", "closes #asdfs", etc. + return s.replace( + /\b(fixes?|closes?|resolves?|fix|close|resolve)\s+#(\w+)/gi, + (match, action, ref) => `\`${action} #${ref}\`` + ); + } + } + /** + * Gets the maximum allowed count for a given output type + * @param {string} itemType - The output item type + * @param {Object} config - The safe-outputs configuration + * @returns {number} The maximum allowed count + */ + function getMaxAllowedForType(itemType, config) { + // Check if max is explicitly specified in config + if ( + config && + config[itemType] && + typeof config[itemType] === "object" && + config[itemType].max + ) { + return config[itemType].max; + } + // Use default limits for plural-supported types + switch (itemType) { + case "create-issue": + return 1; // Only one issue allowed + case "add-issue-comment": + return 1; // Only one comment allowed + case "create-pull-request": + return 1; // Only one pull request allowed + case "create-pull-request-review-comment": + return 10; // Default to 10 review comments allowed + case "add-issue-label": + return 5; // Only one labels operation allowed + case "update-issue": + return 1; // Only one issue update allowed + case "push-to-branch": + return 1; // Only one push to branch allowed + case "create-discussion": + return 1; // Only one discussion allowed + case "missing-tool": + return 1000; // Allow many missing tool reports (default: unlimited) + case "create-security-report": + return 1000; // Allow many security reports (default: unlimited) + default: + return 1; // Default to single item for unknown types + } + } + /** + * Attempts to repair common JSON syntax issues in LLM-generated content + * @param {string} jsonStr - The potentially malformed JSON string + * @returns {string} The repaired JSON string + */ + function repairJson(jsonStr) { + let repaired = jsonStr.trim(); + // Fix single quotes to double quotes (must be done first) + repaired = repaired.replace(/'/g, '"'); + // Fix missing quotes around object keys + repaired = repaired.replace( + /([{,]\s*)([a-zA-Z_$][a-zA-Z0-9_$]*)\s*:/g, + '$1"$2":' + ); + // Fix newlines and tabs inside strings by escaping them + repaired = repaired.replace(/"([^"\\]*)"/g, (match, content) => { + if ( + content.includes("\n") || + content.includes("\r") || + content.includes("\t") + ) { + const escaped = content + .replace(/\\/g, "\\\\") + .replace(/\n/g, "\\n") + .replace(/\r/g, "\\r") + .replace(/\t/g, "\\t"); + return `"${escaped}"`; + } + return match; + }); + // Fix unescaped quotes inside string values + repaired = repaired.replace( + /"([^"]*)"([^":,}\]]*)"([^"]*)"(\s*[,:}\]])/g, + (match, p1, p2, p3, p4) => `"${p1}\\"${p2}\\"${p3}"${p4}` + ); + // Fix wrong bracket/brace types - arrays should end with ] not } + repaired = repaired.replace( + /(\[\s*(?:"[^"]*"(?:\s*,\s*"[^"]*")*\s*),?)\s*}/g, + "$1]" + ); + // Fix missing closing braces/brackets + const openBraces = (repaired.match(/\{/g) || []).length; + const closeBraces = (repaired.match(/\}/g) || []).length; + if (openBraces > closeBraces) { + repaired += "}".repeat(openBraces - closeBraces); + } else if (closeBraces > openBraces) { + repaired = "{".repeat(closeBraces - openBraces) + repaired; + } + // Fix missing closing brackets for arrays + const openBrackets = (repaired.match(/\[/g) || []).length; + const closeBrackets = (repaired.match(/\]/g) || []).length; + if (openBrackets > closeBrackets) { + repaired += "]".repeat(openBrackets - closeBrackets); + } else if (closeBrackets > openBrackets) { + repaired = "[".repeat(closeBrackets - openBrackets) + repaired; + } + // Fix trailing commas in objects and arrays (AFTER fixing brackets/braces) + repaired = repaired.replace(/,(\s*[}\]])/g, "$1"); + return repaired; + } + /** + * Attempts to parse JSON with repair fallback + * @param {string} jsonStr - The JSON string to parse + * @returns {Object|undefined} The parsed JSON object, or undefined if parsing fails + */ + function parseJsonWithRepair(jsonStr) { + try { + // First, try normal JSON.parse + return JSON.parse(jsonStr); + } catch (originalError) { + try { + // If that fails, try repairing and parsing again + const repairedJson = repairJson(jsonStr); + return JSON.parse(repairedJson); + } catch (repairError) { + // If repair also fails, print error to console and return undefined + console.log( + `JSON parsing failed. Original: ${originalError.message}. After repair: ${repairError.message}` + ); + return undefined; + } + } + } + const outputFile = process.env.GITHUB_AW_SAFE_OUTPUTS; + const safeOutputsConfig = process.env.GITHUB_AW_SAFE_OUTPUTS_CONFIG; + if (!outputFile) { + console.log("GITHUB_AW_SAFE_OUTPUTS not set, no output to collect"); + core.setOutput("output", ""); + return; + } + if (!fs.existsSync(outputFile)) { + console.log("Output file does not exist:", outputFile); + core.setOutput("output", ""); + return; + } + const outputContent = fs.readFileSync(outputFile, "utf8"); + if (outputContent.trim() === "") { + console.log("Output file is empty"); + core.setOutput("output", ""); + return; + } + console.log("Raw output content length:", outputContent.length); + // Parse the safe-outputs configuration + let expectedOutputTypes = {}; + if (safeOutputsConfig) { + try { + expectedOutputTypes = JSON.parse(safeOutputsConfig); + console.log("Expected output types:", Object.keys(expectedOutputTypes)); + } catch (error) { + console.log( + "Warning: Could not parse safe-outputs config:", + error.message + ); + } + } + // Parse JSONL content + const lines = outputContent.trim().split("\n"); + const parsedItems = []; + const errors = []; + for (let i = 0; i < lines.length; i++) { + const line = lines[i].trim(); + if (line === "") continue; // Skip empty lines + try { + const item = parseJsonWithRepair(line); + // If item is undefined (failed to parse), add error and process next line + if (item === undefined) { + errors.push(`Line ${i + 1}: Invalid JSON - JSON parsing failed`); + continue; + } + // Validate that the item has a 'type' field + if (!item.type) { + errors.push(`Line ${i + 1}: Missing required 'type' field`); + continue; + } + // Validate against expected output types + const itemType = item.type; + if (!expectedOutputTypes[itemType]) { + errors.push( + `Line ${i + 1}: Unexpected output type '${itemType}'. Expected one of: ${Object.keys(expectedOutputTypes).join(", ")}` + ); + continue; + } + // Check for too many items of the same type + const typeCount = parsedItems.filter( + existing => existing.type === itemType + ).length; + const maxAllowed = getMaxAllowedForType(itemType, expectedOutputTypes); + if (typeCount >= maxAllowed) { + errors.push( + `Line ${i + 1}: Too many items of type '${itemType}'. Maximum allowed: ${maxAllowed}.` + ); + continue; + } + // Basic validation based on type + switch (itemType) { + case "create-issue": + if (!item.title || typeof item.title !== "string") { + errors.push( + `Line ${i + 1}: create-issue requires a 'title' string field` + ); + continue; + } + if (!item.body || typeof item.body !== "string") { + errors.push( + `Line ${i + 1}: create-issue requires a 'body' string field` + ); + continue; + } + // Sanitize text content + item.title = sanitizeContent(item.title); + item.body = sanitizeContent(item.body); + // Sanitize labels if present + if (item.labels && Array.isArray(item.labels)) { + item.labels = item.labels.map(label => + typeof label === "string" ? sanitizeContent(label) : label + ); + } + break; + case "add-issue-comment": + if (!item.body || typeof item.body !== "string") { + errors.push( + `Line ${i + 1}: add-issue-comment requires a 'body' string field` + ); + continue; + } + // Sanitize text content + item.body = sanitizeContent(item.body); + break; + case "create-pull-request": + if (!item.title || typeof item.title !== "string") { + errors.push( + `Line ${i + 1}: create-pull-request requires a 'title' string field` + ); + continue; + } + if (!item.body || typeof item.body !== "string") { + errors.push( + `Line ${i + 1}: create-pull-request requires a 'body' string field` + ); + continue; + } + // Sanitize text content + item.title = sanitizeContent(item.title); + item.body = sanitizeContent(item.body); + // Sanitize branch name if present + if (item.branch && typeof item.branch === "string") { + item.branch = sanitizeContent(item.branch); + } + // Sanitize labels if present + if (item.labels && Array.isArray(item.labels)) { + item.labels = item.labels.map(label => + typeof label === "string" ? sanitizeContent(label) : label + ); + } + break; + case "add-issue-label": + if (!item.labels || !Array.isArray(item.labels)) { + errors.push( + `Line ${i + 1}: add-issue-label requires a 'labels' array field` + ); + continue; + } + if (item.labels.some(label => typeof label !== "string")) { + errors.push( + `Line ${i + 1}: add-issue-label labels array must contain only strings` + ); + continue; + } + // Sanitize label strings + item.labels = item.labels.map(label => sanitizeContent(label)); + break; + case "update-issue": + // Check that at least one updateable field is provided + const hasValidField = + item.status !== undefined || + item.title !== undefined || + item.body !== undefined; + if (!hasValidField) { + errors.push( + `Line ${i + 1}: update-issue requires at least one of: 'status', 'title', or 'body' fields` + ); + continue; + } + // Validate status if provided + if (item.status !== undefined) { + if ( + typeof item.status !== "string" || + (item.status !== "open" && item.status !== "closed") + ) { + errors.push( + `Line ${i + 1}: update-issue 'status' must be 'open' or 'closed'` + ); + continue; + } + } + // Validate title if provided + if (item.title !== undefined) { + if (typeof item.title !== "string") { + errors.push( + `Line ${i + 1}: update-issue 'title' must be a string` + ); + continue; + } + item.title = sanitizeContent(item.title); + } + // Validate body if provided + if (item.body !== undefined) { + if (typeof item.body !== "string") { + errors.push( + `Line ${i + 1}: update-issue 'body' must be a string` + ); + continue; + } + item.body = sanitizeContent(item.body); + } + // Validate issue_number if provided (for target "*") + if (item.issue_number !== undefined) { + if ( + typeof item.issue_number !== "number" && + typeof item.issue_number !== "string" + ) { + errors.push( + `Line ${i + 1}: update-issue 'issue_number' must be a number or string` + ); + continue; + } + } + break; + case "push-to-branch": + // Validate message if provided (optional) + if (item.message !== undefined) { + if (typeof item.message !== "string") { + errors.push( + `Line ${i + 1}: push-to-branch 'message' must be a string` + ); + continue; + } + item.message = sanitizeContent(item.message); + } + // Validate pull_request_number if provided (for target "*") + if (item.pull_request_number !== undefined) { + if ( + typeof item.pull_request_number !== "number" && + typeof item.pull_request_number !== "string" + ) { + errors.push( + `Line ${i + 1}: push-to-branch 'pull_request_number' must be a number or string` + ); + continue; + } + } + break; + case "create-pull-request-review-comment": + // Validate required path field + if (!item.path || typeof item.path !== "string") { + errors.push( + `Line ${i + 1}: create-pull-request-review-comment requires a 'path' string field` + ); + continue; + } + // Validate required line field + if ( + item.line === undefined || + (typeof item.line !== "number" && typeof item.line !== "string") + ) { + errors.push( + `Line ${i + 1}: create-pull-request-review-comment requires a 'line' number or string field` + ); + continue; + } + // Validate line is a positive integer + const lineNumber = + typeof item.line === "string" ? parseInt(item.line, 10) : item.line; + if ( + isNaN(lineNumber) || + lineNumber <= 0 || + !Number.isInteger(lineNumber) + ) { + errors.push( + `Line ${i + 1}: create-pull-request-review-comment 'line' must be a positive integer` + ); + continue; + } + // Validate required body field + if (!item.body || typeof item.body !== "string") { + errors.push( + `Line ${i + 1}: create-pull-request-review-comment requires a 'body' string field` + ); + continue; + } + // Sanitize required text content + item.body = sanitizeContent(item.body); + // Validate optional start_line field + if (item.start_line !== undefined) { + if ( + typeof item.start_line !== "number" && + typeof item.start_line !== "string" + ) { + errors.push( + `Line ${i + 1}: create-pull-request-review-comment 'start_line' must be a number or string` + ); + continue; + } + const startLineNumber = + typeof item.start_line === "string" + ? parseInt(item.start_line, 10) + : item.start_line; + if ( + isNaN(startLineNumber) || + startLineNumber <= 0 || + !Number.isInteger(startLineNumber) + ) { + errors.push( + `Line ${i + 1}: create-pull-request-review-comment 'start_line' must be a positive integer` + ); + continue; + } + if (startLineNumber > lineNumber) { + errors.push( + `Line ${i + 1}: create-pull-request-review-comment 'start_line' must be less than or equal to 'line'` + ); + continue; + } + } + // Validate optional side field + if (item.side !== undefined) { + if ( + typeof item.side !== "string" || + (item.side !== "LEFT" && item.side !== "RIGHT") + ) { + errors.push( + `Line ${i + 1}: create-pull-request-review-comment 'side' must be 'LEFT' or 'RIGHT'` + ); + continue; + } + } + break; + case "create-discussion": + if (!item.title || typeof item.title !== "string") { + errors.push( + `Line ${i + 1}: create-discussion requires a 'title' string field` + ); + continue; + } + if (!item.body || typeof item.body !== "string") { + errors.push( + `Line ${i + 1}: create-discussion requires a 'body' string field` + ); + continue; + } + // Sanitize text content + item.title = sanitizeContent(item.title); + item.body = sanitizeContent(item.body); + break; + case "missing-tool": + // Validate required tool field + if (!item.tool || typeof item.tool !== "string") { + errors.push( + `Line ${i + 1}: missing-tool requires a 'tool' string field` + ); + continue; + } + // Validate required reason field + if (!item.reason || typeof item.reason !== "string") { + errors.push( + `Line ${i + 1}: missing-tool requires a 'reason' string field` + ); + continue; + } + // Sanitize text content + item.tool = sanitizeContent(item.tool); + item.reason = sanitizeContent(item.reason); + // Validate optional alternatives field + if (item.alternatives !== undefined) { + if (typeof item.alternatives !== "string") { + errors.push( + `Line ${i + 1}: missing-tool 'alternatives' must be a string` + ); + continue; + } + item.alternatives = sanitizeContent(item.alternatives); + } + break; + case "create-security-report": + // Validate required sarif field + if (!item.sarif) { + errors.push( + `Line ${i + 1}: create-security-report requires a 'sarif' field` + ); + continue; + } + // SARIF content can be object or string + if ( + typeof item.sarif !== "object" && + typeof item.sarif !== "string" + ) { + errors.push( + `Line ${i + 1}: create-security-report 'sarif' must be an object or string` + ); + continue; + } + // If SARIF is a string, sanitize it + if (typeof item.sarif === "string") { + item.sarif = sanitizeContent(item.sarif); + } + // Validate optional category field + if (item.category !== undefined) { + if (typeof item.category !== "string") { + errors.push( + `Line ${i + 1}: create-security-report 'category' must be a string` + ); + continue; + } + item.category = sanitizeContent(item.category); + } + break; + default: + errors.push(`Line ${i + 1}: Unknown output type '${itemType}'`); + continue; + } + console.log(`Line ${i + 1}: Valid ${itemType} item`); + parsedItems.push(item); + } catch (error) { + errors.push(`Line ${i + 1}: Invalid JSON - ${error.message}`); + } + } + // Report validation results + if (errors.length > 0) { + core.warning("Validation errors found:"); + errors.forEach(error => core.warning(` - ${error}`)); + // For now, we'll continue with valid items but log the errors + // In the future, we might want to fail the workflow for invalid items + } + console.log(`Successfully parsed ${parsedItems.length} valid output items`); + // Set the parsed and validated items as output + const validatedOutput = { + items: parsedItems, + errors: errors, + }; + core.setOutput("output", JSON.stringify(validatedOutput)); + core.setOutput("raw_output", outputContent); + } + // Call the main function + await main(); + - name: Print agent output to step summary + env: + GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} + run: | + echo "## Agent Output (JSONL)" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo '``````json' >> $GITHUB_STEP_SUMMARY + cat ${{ env.GITHUB_AW_SAFE_OUTPUTS }} >> $GITHUB_STEP_SUMMARY + # Ensure there's a newline after the file content if it doesn't end with one + if [ -s ${{ env.GITHUB_AW_SAFE_OUTPUTS }} ] && [ "$(tail -c1 ${{ env.GITHUB_AW_SAFE_OUTPUTS }})" != "" ]; then + echo "" >> $GITHUB_STEP_SUMMARY + fi + echo '``````' >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "## Processed Output" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo '``````json' >> $GITHUB_STEP_SUMMARY + echo '${{ steps.collect_output.outputs.output }}' >> $GITHUB_STEP_SUMMARY + echo '``````' >> $GITHUB_STEP_SUMMARY + - name: Upload agentic output file + if: always() && steps.collect_output.outputs.output != '' + uses: actions/upload-artifact@v4 + with: + name: aw_output.txt + path: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} + if-no-files-found: warn + - name: Upload agent logs + if: always() + uses: actions/upload-artifact@v4 + with: + name: ai-inference-github-models.log + path: /tmp/ai-inference-github-models.log + if-no-files-found: warn + + create_issue_comment: + needs: ai-inference-github-models + if: always() + runs-on: ubuntu-latest + permissions: + contents: read + issues: write + pull-requests: write + timeout-minutes: 10 + outputs: + comment_id: ${{ steps.create_comment.outputs.comment_id }} + comment_url: ${{ steps.create_comment.outputs.comment_url }} + steps: + - name: Add Issue Comment + id: create_comment + uses: actions/github-script@v7 + env: + GITHUB_AW_AGENT_OUTPUT: ${{ needs.ai-inference-github-models.outputs.output }} + GITHUB_AW_COMMENT_TARGET: "*" + with: + script: | + async function main() { + // Read the validated output content from environment variable + const outputContent = process.env.GITHUB_AW_AGENT_OUTPUT; + if (!outputContent) { + console.log("No GITHUB_AW_AGENT_OUTPUT environment variable found"); + return; + } + if (outputContent.trim() === "") { + console.log("Agent output content is empty"); + return; + } + console.log("Agent output content length:", outputContent.length); + // Parse the validated output JSON + let validatedOutput; + try { + validatedOutput = JSON.parse(outputContent); + } catch (error) { + console.log( + "Error parsing agent output JSON:", + error instanceof Error ? error.message : String(error) + ); + return; + } + if (!validatedOutput.items || !Array.isArray(validatedOutput.items)) { + console.log("No valid items found in agent output"); + return; + } + // Find all add-issue-comment items + const commentItems = validatedOutput.items.filter( + /** @param {any} item */ item => item.type === "add-issue-comment" + ); + if (commentItems.length === 0) { + console.log("No add-issue-comment items found in agent output"); + return; + } + console.log(`Found ${commentItems.length} add-issue-comment item(s)`); + // Get the target configuration from environment variable + const commentTarget = process.env.GITHUB_AW_COMMENT_TARGET || "triggering"; + console.log(`Comment target configuration: ${commentTarget}`); + // Check if we're in an issue or pull request context + const isIssueContext = + context.eventName === "issues" || context.eventName === "issue_comment"; + const isPRContext = + context.eventName === "pull_request" || + context.eventName === "pull_request_review" || + context.eventName === "pull_request_review_comment"; + // Validate context based on target configuration + if (commentTarget === "triggering" && !isIssueContext && !isPRContext) { + console.log( + 'Target is "triggering" but not running in issue or pull request context, skipping comment creation' + ); + return; + } + const createdComments = []; + // Process each comment item + for (let i = 0; i < commentItems.length; i++) { + const commentItem = commentItems[i]; + console.log( + `Processing add-issue-comment item ${i + 1}/${commentItems.length}:`, + { bodyLength: commentItem.body.length } + ); + // Determine the issue/PR number and comment endpoint for this comment + let issueNumber; + let commentEndpoint; + if (commentTarget === "*") { + // For target "*", we need an explicit issue number from the comment item + if (commentItem.issue_number) { + issueNumber = parseInt(commentItem.issue_number, 10); + if (isNaN(issueNumber) || issueNumber <= 0) { + console.log( + `Invalid issue number specified: ${commentItem.issue_number}` + ); + continue; + } + commentEndpoint = "issues"; + } else { + console.log( + 'Target is "*" but no issue_number specified in comment item' + ); + continue; + } + } else if (commentTarget && commentTarget !== "triggering") { + // Explicit issue number specified in target + issueNumber = parseInt(commentTarget, 10); + if (isNaN(issueNumber) || issueNumber <= 0) { + console.log( + `Invalid issue number in target configuration: ${commentTarget}` + ); + continue; + } + commentEndpoint = "issues"; + } else { + // Default behavior: use triggering issue/PR + if (isIssueContext) { + if (context.payload.issue) { + issueNumber = context.payload.issue.number; + commentEndpoint = "issues"; + } else { + console.log("Issue context detected but no issue found in payload"); + continue; + } + } else if (isPRContext) { + if (context.payload.pull_request) { + issueNumber = context.payload.pull_request.number; + commentEndpoint = "issues"; // PR comments use the issues API endpoint + } else { + console.log( + "Pull request context detected but no pull request found in payload" + ); + continue; + } + } + } + if (!issueNumber) { + console.log("Could not determine issue or pull request number"); + continue; + } + // Extract body from the JSON item + let body = commentItem.body.trim(); + // Add AI disclaimer with run id, run htmlurl + const runId = context.runId; + const runUrl = context.payload.repository + ? `${context.payload.repository.html_url}/actions/runs/${runId}` + : `https://github.com/actions/runs/${runId}`; + body += `\n\n> Generated by Agentic Workflow Run [${runId}](${runUrl})\n`; + console.log(`Creating comment on ${commentEndpoint} #${issueNumber}`); + console.log("Comment content length:", body.length); + try { + // Create the comment using GitHub API + const { data: comment } = await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issueNumber, + body: body, + }); + console.log("Created comment #" + comment.id + ": " + comment.html_url); + createdComments.push(comment); + // Set output for the last created comment (for backward compatibility) + if (i === commentItems.length - 1) { + core.setOutput("comment_id", comment.id); + core.setOutput("comment_url", comment.html_url); + } + } catch (error) { + core.error( + `āœ— Failed to create comment: ${error instanceof Error ? error.message : String(error)}` + ); + throw error; + } + } + // Write summary for all created comments + if (createdComments.length > 0) { + let summaryContent = "\n\n## GitHub Comments\n"; + for (const comment of createdComments) { + summaryContent += `- Comment #${comment.id}: [View Comment](${comment.html_url})\n`; + } + await core.summary.addRaw(summaryContent).write(); + } + console.log(`Successfully created ${createdComments.length} comment(s)`); + return createdComments; + } + await main(); + diff --git a/.github/workflows/ai-inference-github-models.md b/.github/workflows/ai-inference-github-models.md new file mode 100644 index 00000000000..9f9ba12ff3f --- /dev/null +++ b/.github/workflows/ai-inference-github-models.md @@ -0,0 +1,52 @@ +--- +name: AI Inference with GitHub Models +on: + workflow_dispatch: + inputs: + issue_number: + description: 'The number of the issue to analyze' + required: true + issues: + types: [opened] + +permissions: + contents: read + models: read + +engine: + id: custom + max-turns: 3 + steps: + - name: Setup AI Inference with GitHub Models + uses: actions/ai-inference@v1 + id: ai_inference + with: + # Use gpt-4o-mini model + model: gpt-4o-mini + # Use the provided prompt or create one based on the event + prompt-file: ${{ env.GITHUB_AW_PROMPT }} + # Configure the AI inference settings + max_tokens: 1000 + temperature: 0.7 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Create Issue Comment + run: | + # Determine issue number based on event type + if [ "${{ github.event_name }}" == "issues" ]; then + ISSUE_NUMBER="${{ github.event.issue.number }}" + else + ISSUE_NUMBER="${{ github.event.inputs.issue_number }}" + fi + + # Generate safe output for issue comment + echo "{\"type\": \"add-issue-comment\", \"issue_number\": \"$ISSUE_NUMBER\", \"body\": \"## šŸ¤– AI Analysis\\n\\nI've analyzed this issue using GitHub's AI models. Here's my assessment:\\n\\n${{ steps.ai_inference.outputs.response }}\\n\\n---\\n*This response was generated using GitHub Models via the AI Inference action.*\"}" >> $GITHUB_AW_SAFE_OUTPUTS + +safe-outputs: + add-issue-comment: + max: 1 + target: "*" +--- + +Summarize the issue #${{ github.event.issue.number }} \ No newline at end of file From a63e783596ea641bfe9d99d3921402d0f821a52c Mon Sep 17 00:00:00 2001 From: Peli de Halleux Date: Sat, 6 Sep 2025 00:52:50 +0000 Subject: [PATCH 3/6] Enhance AI Inference workflow with content sanitization and improved output formatting --- .../ai-inference-github-models.lock.yml | 225 +++++++++++++++++- .../workflows/ai-inference-github-models.md | 8 +- 2 files changed, 228 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ai-inference-github-models.lock.yml b/.github/workflows/ai-inference-github-models.lock.yml index 50fe1ccf222..ecec4b2466c 100644 --- a/.github/workflows/ai-inference-github-models.lock.yml +++ b/.github/workflows/ai-inference-github-models.lock.yml @@ -21,7 +21,222 @@ concurrency: run-name: "Ai Inference Github Models" jobs: + task: + runs-on: ubuntu-latest + outputs: + text: ${{ steps.compute-text.outputs.text }} + steps: + - name: Compute current body text + id: compute-text + uses: actions/github-script@v7 + with: + script: | + /** + * Sanitizes content for safe output in GitHub Actions + * @param {string} content - The content to sanitize + * @returns {string} The sanitized content + */ + function sanitizeContent(content) { + if (!content || typeof content !== "string") { + return ""; + } + // Read allowed domains from environment variable + const allowedDomainsEnv = process.env.GITHUB_AW_ALLOWED_DOMAINS; + const defaultAllowedDomains = [ + "github.com", + "github.io", + "githubusercontent.com", + "githubassets.com", + "github.dev", + "codespaces.new", + ]; + const allowedDomains = allowedDomainsEnv + ? allowedDomainsEnv + .split(",") + .map(d => d.trim()) + .filter(d => d) + : defaultAllowedDomains; + let sanitized = content; + // Neutralize @mentions to prevent unintended notifications + sanitized = neutralizeMentions(sanitized); + // Remove control characters (except newlines and tabs) + sanitized = sanitized.replace(/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/g, ""); + // XML character escaping + sanitized = sanitized + .replace(/&/g, "&") // Must be first to avoid double-escaping + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); + // URI filtering - replace non-https protocols with "(redacted)" + // Step 1: Temporarily mark HTTPS URLs to protect them + sanitized = sanitizeUrlProtocols(sanitized); + // Domain filtering for HTTPS URIs + // Match https:// URIs and check if domain is in allowlist + sanitized = sanitizeUrlDomains(sanitized); + // Limit total length to prevent DoS (0.5MB max) + const maxLength = 524288; + if (sanitized.length > maxLength) { + sanitized = + sanitized.substring(0, maxLength) + "\n[Content truncated due to length]"; + } + // Limit number of lines to prevent log flooding (65k max) + const lines = sanitized.split("\n"); + const maxLines = 65000; + if (lines.length > maxLines) { + sanitized = + lines.slice(0, maxLines).join("\n") + + "\n[Content truncated due to line count]"; + } + // Remove ANSI escape sequences + sanitized = sanitized.replace(/\x1b\[[0-9;]*[mGKH]/g, ""); + // Neutralize common bot trigger phrases + sanitized = neutralizeBotTriggers(sanitized); + // Trim excessive whitespace + return sanitized.trim(); + /** + * Remove unknown domains + * @param {string} s - The string to process + * @returns {string} The string with unknown domains redacted + */ + function sanitizeUrlDomains(s) { + s = s.replace( + /\bhttps:\/\/([^\/\s\])}'"<>&\x00-\x1f]+)/gi, + (match, domain) => { + // Extract the hostname part (before first slash, colon, or other delimiter) + const hostname = domain.split(/[\/:\?#]/)[0].toLowerCase(); + // Check if this domain or any parent domain is in the allowlist + const isAllowed = allowedDomains.some(allowedDomain => { + const normalizedAllowed = allowedDomain.toLowerCase(); + return ( + hostname === normalizedAllowed || + hostname.endsWith("." + normalizedAllowed) + ); + }); + return isAllowed ? match : "(redacted)"; + } + ); + return s; + } + /** + * Remove unknown protocols except https + * @param {string} s - The string to process + * @returns {string} The string with non-https protocols redacted + */ + function sanitizeUrlProtocols(s) { + // Match both protocol:// and protocol: patterns + // This covers URLs like https://example.com, javascript:alert(), mailto:user@domain.com, etc. + return s.replace( + /\b(\w+):(?:\/\/)?[^\s\])}'"<>&\x00-\x1f]+/gi, + (match, protocol) => { + // Allow https (case insensitive), redact everything else + return protocol.toLowerCase() === "https" ? match : "(redacted)"; + } + ); + } + /** + * Neutralizes @mentions by wrapping them in backticks + * @param {string} s - The string to process + * @returns {string} The string with neutralized mentions + */ + function neutralizeMentions(s) { + // Replace @name or @org/team outside code with `@name` + return s.replace( + /(^|[^\w`])@([A-Za-z0-9](?:[A-Za-z0-9-]{0,37}[A-Za-z0-9])?(?:\/[A-Za-z0-9._-]+)?)/g, + (_m, p1, p2) => `${p1}\`@${p2}\`` + ); + } + /** + * Neutralizes bot trigger phrases by wrapping them in backticks + * @param {string} s - The string to process + * @returns {string} The string with neutralized bot triggers + */ + function neutralizeBotTriggers(s) { + // Neutralize common bot trigger phrases like "fixes #123", "closes #asdfs", etc. + return s.replace( + /\b(fixes?|closes?|resolves?|fix|close|resolve)\s+#(\w+)/gi, + (match, action, ref) => `\`${action} #${ref}\`` + ); + } + } + async function main() { + let text = ""; + const actor = context.actor; + const { owner, repo } = context.repo; + // Check if the actor has repository access (admin, maintain permissions) + const repoPermission = await github.rest.repos.getCollaboratorPermissionLevel( + { + owner: owner, + repo: repo, + username: actor, + } + ); + const permission = repoPermission.data.permission; + console.log(`Repository permission level: ${permission}`); + if (permission !== "admin" && permission !== "maintain") { + core.setOutput("text", ""); + return; + } + // Determine current body text based on event context + switch (context.eventName) { + case "issues": + // For issues: title + body + if (context.payload.issue) { + const title = context.payload.issue.title || ""; + const body = context.payload.issue.body || ""; + text = `${title}\n\n${body}`; + } + break; + case "pull_request": + // For pull requests: title + body + if (context.payload.pull_request) { + const title = context.payload.pull_request.title || ""; + const body = context.payload.pull_request.body || ""; + text = `${title}\n\n${body}`; + } + break; + case "pull_request_target": + // For pull request target events: title + body + if (context.payload.pull_request) { + const title = context.payload.pull_request.title || ""; + const body = context.payload.pull_request.body || ""; + text = `${title}\n\n${body}`; + } + break; + case "issue_comment": + // For issue comments: comment body + if (context.payload.comment) { + text = context.payload.comment.body || ""; + } + break; + case "pull_request_review_comment": + // For PR review comments: comment body + if (context.payload.comment) { + text = context.payload.comment.body || ""; + } + break; + case "pull_request_review": + // For PR reviews: review body + if (context.payload.review) { + text = context.payload.review.body || ""; + } + break; + default: + // Default: empty text + text = ""; + break; + } + // Sanitize the text before output + const sanitizedText = sanitizeContent(text); + // Display sanitized text in logs + console.log(`text: ${sanitizedText}`); + // Set the sanitized text as output + core.setOutput("text", sanitizedText); + } + await main(); + ai-inference-github-models: + needs: task runs-on: ubuntu-latest permissions: contents: read @@ -86,7 +301,11 @@ jobs: run: | mkdir -p /tmp/aw-prompts cat > $GITHUB_AW_PROMPT << 'EOF' - Summarize the issue #${{ github.event.issue.number }} + Summarize the issue inlined below and provide suggestions for next steps. + + --- + + ${{ needs.task.outputs.text }} --- @@ -169,10 +388,10 @@ jobs: id: ai_inference uses: actions/ai-inference@v1 with: + temperature: 0.7 model: gpt-4o-mini prompt-file: ${{ env.GITHUB_AW_PROMPT }} max_tokens: 1000 - temperature: 0.7 env: GITHUB_AW_PROMPT: /tmp/aw-prompts/prompt.txt GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} @@ -188,7 +407,7 @@ jobs: fi # Generate safe output for issue comment - echo "{\"type\": \"add-issue-comment\", \"issue_number\": \"$ISSUE_NUMBER\", \"body\": \"## šŸ¤– AI Analysis\\n\\nI've analyzed this issue using GitHub's AI models. Here's my assessment:\\n\\n${{ steps.ai_inference.outputs.response }}\\n\\n---\\n*This response was generated using GitHub Models via the AI Inference action.*\"}" >> $GITHUB_AW_SAFE_OUTPUTS + echo "{\"type\": \"add-issue-comment\", \"issue_number\": \"$ISSUE_NUMBER\", \"body\": \"${{ steps.ai_inference.outputs.response }}\"}" >> $GITHUB_AW_SAFE_OUTPUTS env: GITHUB_AW_PROMPT: /tmp/aw-prompts/prompt.txt diff --git a/.github/workflows/ai-inference-github-models.md b/.github/workflows/ai-inference-github-models.md index 9f9ba12ff3f..6a4eda86a5f 100644 --- a/.github/workflows/ai-inference-github-models.md +++ b/.github/workflows/ai-inference-github-models.md @@ -41,7 +41,7 @@ engine: fi # Generate safe output for issue comment - echo "{\"type\": \"add-issue-comment\", \"issue_number\": \"$ISSUE_NUMBER\", \"body\": \"## šŸ¤– AI Analysis\\n\\nI've analyzed this issue using GitHub's AI models. Here's my assessment:\\n\\n${{ steps.ai_inference.outputs.response }}\\n\\n---\\n*This response was generated using GitHub Models via the AI Inference action.*\"}" >> $GITHUB_AW_SAFE_OUTPUTS + echo "{\"type\": \"add-issue-comment\", \"issue_number\": \"$ISSUE_NUMBER\", \"body\": \"${{ steps.ai_inference.outputs.response }}\"}" >> $GITHUB_AW_SAFE_OUTPUTS safe-outputs: add-issue-comment: @@ -49,4 +49,8 @@ safe-outputs: target: "*" --- -Summarize the issue #${{ github.event.issue.number }} \ No newline at end of file +Summarize the issue inlined below and provide suggestions for next steps. + +--- + +${{ needs.task.outputs.text }} \ No newline at end of file From 7bc24d9750bdfa7024228b8f2b609f40b5f8a14a Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Sep 2025 20:44:43 -0700 Subject: [PATCH 4/6] Add create-security-report safe output to custom engine test workflow (#86) --- .../test-safe-outputs-custom-engine.lock.yml | 309 +++++++++++++++++- .../test-safe-outputs-custom-engine.md | 7 + 2 files changed, 315 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test-safe-outputs-custom-engine.lock.yml b/.github/workflows/test-safe-outputs-custom-engine.lock.yml index 1fc42b3164d..ad5060fdace 100644 --- a/.github/workflows/test-safe-outputs-custom-engine.lock.yml +++ b/.github/workflows/test-safe-outputs-custom-engine.lock.yml @@ -106,6 +106,7 @@ jobs: - **missing-tool**: Reports missing functionality (test simulation) - **create-discussion**: Creates repository discussions - **create-pull-request-review-comment**: Creates PR review comments + - **create-security-report**: Generates SARIF security reports ## Custom Engine Implementation @@ -347,6 +348,14 @@ jobs: GITHUB_AW_PROMPT: /tmp/aw-prompts/prompt.txt GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} + - name: Generate Security Report Output + run: | + echo '{"type": "create-security-report", "file": "README.md", "line": 1, "severity": "note", "message": "This is a test security finding generated by the custom engine workflow to validate the create-security-report safe output functionality. This is a notice-level finding used for testing purposes and does not represent an actual security issue.", "ruleIdSuffix": "test-custom-engine-notice"}' >> $GITHUB_AW_SAFE_OUTPUTS + + env: + GITHUB_AW_PROMPT: /tmp/aw-prompts/prompt.txt + GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} + - name: List generated outputs run: | echo "Generated safe output entries:" @@ -388,7 +397,7 @@ jobs: uses: actions/github-script@v7 env: GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} - GITHUB_AW_SAFE_OUTPUTS_CONFIG: "{\"add-issue-comment\":{\"enabled\":true,\"target\":\"*\"},\"add-issue-label\":true,\"create-discussion\":{\"enabled\":true,\"max\":1},\"create-issue\":true,\"create-pull-request\":true,\"create-pull-request-review-comment\":{\"enabled\":true,\"max\":1},\"missing-tool\":{\"enabled\":true,\"max\":5},\"push-to-branch\":{\"branch\":\"triggering\",\"enabled\":true,\"target\":\"*\"},\"update-issue\":true}" + GITHUB_AW_SAFE_OUTPUTS_CONFIG: "{\"add-issue-comment\":{\"enabled\":true,\"target\":\"*\"},\"add-issue-label\":true,\"create-discussion\":{\"enabled\":true,\"max\":1},\"create-issue\":true,\"create-pull-request\":true,\"create-pull-request-review-comment\":{\"enabled\":true,\"max\":1},\"create-security-report\":{\"enabled\":true,\"max\":5},\"missing-tool\":{\"enabled\":true,\"max\":5},\"push-to-branch\":{\"branch\":\"triggering\",\"enabled\":true,\"target\":\"*\"},\"update-issue\":true}" with: script: | async function main() { @@ -1975,6 +1984,304 @@ jobs: } await main(); + create_security_report: + needs: test-safe-outputs-custom-engine + runs-on: ubuntu-latest + permissions: + contents: read + security-events: write + actions: read + timeout-minutes: 10 + outputs: + artifact_uploaded: ${{ steps.create_security_report.outputs.artifact_uploaded }} + codeql_uploaded: ${{ steps.create_security_report.outputs.codeql_uploaded }} + findings_count: ${{ steps.create_security_report.outputs.findings_count }} + sarif_file: ${{ steps.create_security_report.outputs.sarif_file }} + steps: + - name: Create Security Report + id: create_security_report + uses: actions/github-script@v7 + env: + GITHUB_AW_AGENT_OUTPUT: ${{ needs.test-safe-outputs-custom-engine.outputs.output }} + GITHUB_AW_SECURITY_REPORT_MAX: 5 + GITHUB_AW_SECURITY_REPORT_DRIVER: Test Safe Outputs - Custom Engine + GITHUB_AW_WORKFLOW_FILENAME: test-safe-outputs-custom-engine + with: + script: | + async function main() { + // Read the validated output content from environment variable + const outputContent = process.env.GITHUB_AW_AGENT_OUTPUT; + if (!outputContent) { + console.log("No GITHUB_AW_AGENT_OUTPUT environment variable found"); + return; + } + if (outputContent.trim() === "") { + console.log("Agent output content is empty"); + return; + } + console.log("Agent output content length:", outputContent.length); + // Parse the validated output JSON + let validatedOutput; + try { + validatedOutput = JSON.parse(outputContent); + } catch (error) { + console.log( + "Error parsing agent output JSON:", + error instanceof Error ? error.message : String(error) + ); + return; + } + if (!validatedOutput.items || !Array.isArray(validatedOutput.items)) { + console.log("No valid items found in agent output"); + return; + } + // Find all create-security-report items + const securityItems = validatedOutput.items.filter( + /** @param {any} item */ item => item.type === "create-security-report" + ); + if (securityItems.length === 0) { + console.log("No create-security-report items found in agent output"); + return; + } + console.log(`Found ${securityItems.length} create-security-report item(s)`); + // Get the max configuration from environment variable + const maxFindings = process.env.GITHUB_AW_SECURITY_REPORT_MAX + ? parseInt(process.env.GITHUB_AW_SECURITY_REPORT_MAX) + : 0; // 0 means unlimited + console.log( + `Max findings configuration: ${maxFindings === 0 ? "unlimited" : maxFindings}` + ); + // Get the driver configuration from environment variable + const driverName = + process.env.GITHUB_AW_SECURITY_REPORT_DRIVER || + "GitHub Agentic Workflows Security Scanner"; + console.log(`Driver name: ${driverName}`); + // Get the workflow filename for rule ID prefix + const workflowFilename = + process.env.GITHUB_AW_WORKFLOW_FILENAME || "workflow"; + console.log(`Workflow filename for rule ID prefix: ${workflowFilename}`); + const validFindings = []; + // Process each security item and validate the findings + for (let i = 0; i < securityItems.length; i++) { + const securityItem = securityItems[i]; + console.log( + `Processing create-security-report item ${i + 1}/${securityItems.length}:`, + { + file: securityItem.file, + line: securityItem.line, + severity: securityItem.severity, + messageLength: securityItem.message + ? securityItem.message.length + : "undefined", + ruleIdSuffix: securityItem.ruleIdSuffix || "not specified", + } + ); + // Validate required fields + if (!securityItem.file) { + console.log('Missing required field "file" in security report item'); + continue; + } + if ( + !securityItem.line || + (typeof securityItem.line !== "number" && + typeof securityItem.line !== "string") + ) { + console.log( + 'Missing or invalid required field "line" in security report item' + ); + continue; + } + if (!securityItem.severity || typeof securityItem.severity !== "string") { + console.log( + 'Missing or invalid required field "severity" in security report item' + ); + continue; + } + if (!securityItem.message || typeof securityItem.message !== "string") { + console.log( + 'Missing or invalid required field "message" in security report item' + ); + continue; + } + // Parse line number + const line = parseInt(securityItem.line, 10); + if (isNaN(line) || line <= 0) { + console.log(`Invalid line number: ${securityItem.line}`); + continue; + } + // Parse optional column number + let column = 1; // Default to column 1 + if (securityItem.column !== undefined) { + if ( + typeof securityItem.column !== "number" && + typeof securityItem.column !== "string" + ) { + console.log( + 'Invalid field "column" in security report item (must be number or string)' + ); + continue; + } + const parsedColumn = parseInt(securityItem.column, 10); + if (isNaN(parsedColumn) || parsedColumn <= 0) { + console.log(`Invalid column number: ${securityItem.column}`); + continue; + } + column = parsedColumn; + } + // Parse optional rule ID suffix + let ruleIdSuffix = null; + if (securityItem.ruleIdSuffix !== undefined) { + if (typeof securityItem.ruleIdSuffix !== "string") { + console.log( + 'Invalid field "ruleIdSuffix" in security report item (must be string)' + ); + continue; + } + // Validate that the suffix doesn't contain invalid characters + const trimmedSuffix = securityItem.ruleIdSuffix.trim(); + if (trimmedSuffix.length === 0) { + console.log( + 'Invalid field "ruleIdSuffix" in security report item (cannot be empty)' + ); + continue; + } + // Check for characters that would be problematic in rule IDs + if (!/^[a-zA-Z0-9_-]+$/.test(trimmedSuffix)) { + console.log( + `Invalid ruleIdSuffix "${trimmedSuffix}" (must contain only alphanumeric characters, hyphens, and underscores)` + ); + continue; + } + ruleIdSuffix = trimmedSuffix; + } + // Validate severity level and map to SARIF level + const severityMap = { + error: "error", + warning: "warning", + info: "note", + note: "note", + }; + const normalizedSeverity = securityItem.severity.toLowerCase(); + if (!severityMap[normalizedSeverity]) { + console.log( + `Invalid severity level: ${securityItem.severity} (must be error, warning, info, or note)` + ); + continue; + } + const sarifLevel = severityMap[normalizedSeverity]; + // Create a valid finding object + validFindings.push({ + file: securityItem.file.trim(), + line: line, + column: column, + severity: normalizedSeverity, + sarifLevel: sarifLevel, + message: securityItem.message.trim(), + ruleIdSuffix: ruleIdSuffix, + }); + // Check if we've reached the max limit + if (maxFindings > 0 && validFindings.length >= maxFindings) { + console.log(`Reached maximum findings limit: ${maxFindings}`); + break; + } + } + if (validFindings.length === 0) { + console.log("No valid security findings to report"); + return; + } + console.log(`Processing ${validFindings.length} valid security finding(s)`); + // Generate SARIF file + const sarifContent = { + $schema: + "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json", + version: "2.1.0", + runs: [ + { + tool: { + driver: { + name: driverName, + version: "1.0.0", + informationUri: "https://github.com/githubnext/gh-aw-copilots", + }, + }, + results: validFindings.map((finding, index) => ({ + ruleId: finding.ruleIdSuffix + ? `${workflowFilename}-${finding.ruleIdSuffix}` + : `${workflowFilename}-security-finding-${index + 1}`, + message: { text: finding.message }, + level: finding.sarifLevel, + locations: [ + { + physicalLocation: { + artifactLocation: { uri: finding.file }, + region: { + startLine: finding.line, + startColumn: finding.column, + }, + }, + }, + ], + })), + }, + ], + }; + // Write SARIF file to filesystem + const fs = require("fs"); + const path = require("path"); + const sarifFileName = "security-report.sarif"; + const sarifFilePath = path.join(process.cwd(), sarifFileName); + try { + fs.writeFileSync(sarifFilePath, JSON.stringify(sarifContent, null, 2)); + console.log(`āœ“ Created SARIF file: ${sarifFilePath}`); + console.log(`SARIF file size: ${fs.statSync(sarifFilePath).size} bytes`); + // Set outputs for the GitHub Action + core.setOutput("sarif_file", sarifFilePath); + core.setOutput("findings_count", validFindings.length); + core.setOutput("artifact_uploaded", "pending"); + core.setOutput("codeql_uploaded", "pending"); + // Write summary with findings + let summaryContent = "\n\n## Security Report\n"; + summaryContent += `Found **${validFindings.length}** security finding(s):\n\n`; + for (const finding of validFindings) { + const emoji = + finding.severity === "error" + ? "šŸ”“" + : finding.severity === "warning" + ? "šŸŸ”" + : "šŸ”µ"; + summaryContent += `${emoji} **${finding.severity.toUpperCase()}** in \`${finding.file}:${finding.line}\`: ${finding.message}\n`; + } + summaryContent += `\nšŸ“„ SARIF file created: \`${sarifFileName}\`\n`; + summaryContent += `šŸ” Findings will be uploaded to GitHub Code Scanning\n`; + await core.summary.addRaw(summaryContent).write(); + } catch (error) { + core.error( + `āœ— Failed to create SARIF file: ${error instanceof Error ? error.message : String(error)}` + ); + throw error; + } + console.log( + `Successfully created security report with ${validFindings.length} finding(s)` + ); + return { + sarifFile: sarifFilePath, + findingsCount: validFindings.length, + findings: validFindings, + }; + } + await main(); + - name: Upload SARIF artifact + if: steps.create_security_report.outputs.sarif_file + uses: actions/upload-artifact@v4 + with: + name: security-report.sarif + path: ${{ steps.create_security_report.outputs.sarif_file }} + - name: Upload SARIF to GitHub Security + if: steps.create_security_report.outputs.sarif_file + uses: github/codeql-action/upload-sarif@v3 + with: + sarif_file: ${{ steps.create_security_report.outputs.sarif_file }} + create_pull_request: needs: test-safe-outputs-custom-engine runs-on: ubuntu-latest diff --git a/.github/workflows/test-safe-outputs-custom-engine.md b/.github/workflows/test-safe-outputs-custom-engine.md index 044503281f5..4015cab4a73 100644 --- a/.github/workflows/test-safe-outputs-custom-engine.md +++ b/.github/workflows/test-safe-outputs-custom-engine.md @@ -41,6 +41,8 @@ safe-outputs: create-pull-request-review-comment: max: 1 side: "RIGHT" + create-security-report: + max: 5 engine: id: custom @@ -92,6 +94,10 @@ engine: run: | echo '{"type": "missing-tool", "tool": "example-missing-tool", "reason": "This is a test of the missing-tool safe output functionality. No actual tool is missing.", "alternatives": "This is a simulated missing tool report generated by the custom engine test workflow.", "context": "test-safe-outputs-custom-engine workflow validation"}' >> $GITHUB_AW_SAFE_OUTPUTS + - name: Generate Security Report Output + run: | + echo '{"type": "create-security-report", "file": "README.md", "line": 1, "severity": "note", "message": "This is a test security finding generated by the custom engine workflow to validate the create-security-report safe output functionality. This is a notice-level finding used for testing purposes and does not represent an actual security issue.", "ruleIdSuffix": "test-custom-engine-notice"}' >> $GITHUB_AW_SAFE_OUTPUTS + - name: List generated outputs run: | echo "Generated safe output entries:" @@ -124,6 +130,7 @@ This is a comprehensive test workflow that exercises every available safe output - **missing-tool**: Reports missing functionality (test simulation) - **create-discussion**: Creates repository discussions - **create-pull-request-review-comment**: Creates PR review comments +- **create-security-report**: Generates SARIF security reports ## Custom Engine Implementation From e2ba9e00f5ab93f1274a83509bfe9f8b9445964c Mon Sep 17 00:00:00 2001 From: Peli de Halleux Date: Sat, 6 Sep 2025 04:15:44 +0000 Subject: [PATCH 5/6] Add agent output JSON storage and upload to AI Inference workflow --- .../ai-inference-github-models.lock.yml | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ai-inference-github-models.lock.yml b/.github/workflows/ai-inference-github-models.lock.yml index ecec4b2466c..9e6245c969d 100644 --- a/.github/workflows/ai-inference-github-models.lock.yml +++ b/.github/workflows/ai-inference-github-models.lock.yml @@ -388,10 +388,10 @@ jobs: id: ai_inference uses: actions/ai-inference@v1 with: - temperature: 0.7 model: gpt-4o-mini prompt-file: ${{ env.GITHUB_AW_PROMPT }} max_tokens: 1000 + temperature: 0.7 env: GITHUB_AW_PROMPT: /tmp/aw-prompts/prompt.txt GITHUB_AW_SAFE_OUTPUTS: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} @@ -1110,6 +1110,19 @@ jobs: items: parsedItems, errors: errors, }; + // Store validatedOutput JSON in "agent_output.json" file + const agentOutputFile = "/tmp/agent_output.json"; + const validatedOutputJson = JSON.stringify(validatedOutput); + try { + // Ensure the /tmp directory exists + fs.mkdirSync("/tmp", { recursive: true }); + fs.writeFileSync(agentOutputFile, validatedOutputJson, "utf8"); + console.log(`Stored validated output to: ${agentOutputFile}`); + // Set the environment variable GITHUB_AW_AGENT_OUTPUT to the file path + core.exportVariable("GITHUB_AW_AGENT_OUTPUT", agentOutputFile); + } catch (error) { + console.error(`Failed to write agent output file: ${error.message}`); + } core.setOutput("output", JSON.stringify(validatedOutput)); core.setOutput("raw_output", outputContent); } @@ -1138,9 +1151,16 @@ jobs: if: always() && steps.collect_output.outputs.output != '' uses: actions/upload-artifact@v4 with: - name: aw_output.txt + name: safe_output.jsonl path: ${{ env.GITHUB_AW_SAFE_OUTPUTS }} if-no-files-found: warn + - name: Upload agent output JSON + if: always() && env.GITHUB_AW_AGENT_OUTPUT + uses: actions/upload-artifact@v4 + with: + name: agent_output.json + path: ${{ env.GITHUB_AW_AGENT_OUTPUT }} + if-no-files-found: warn - name: Upload agent logs if: always() uses: actions/upload-artifact@v4 From adf3bdc5da392a8ebb910b04e3b7127a4fc2557a Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Sep 2025 21:33:07 -0700 Subject: [PATCH 6/6] Sort `with` section keys for stable output in engine step conversion (#87) * Initial plan * Sort with section keys for stable output in engine step conversion Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../ai-inference-github-models.lock.yml | 2 +- pkg/workflow/claude_engine.go | 10 +++- pkg/workflow/claude_engine_test.go | 55 +++++++++++++++++++ pkg/workflow/codex_engine.go | 10 +++- pkg/workflow/codex_engine_test.go | 55 +++++++++++++++++++ pkg/workflow/custom_engine.go | 11 +++- pkg/workflow/custom_engine_test.go | 55 +++++++++++++++++++ 7 files changed, 191 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ai-inference-github-models.lock.yml b/.github/workflows/ai-inference-github-models.lock.yml index 9e6245c969d..c8fe18311d4 100644 --- a/.github/workflows/ai-inference-github-models.lock.yml +++ b/.github/workflows/ai-inference-github-models.lock.yml @@ -388,9 +388,9 @@ jobs: id: ai_inference uses: actions/ai-inference@v1 with: + max_tokens: 1000 model: gpt-4o-mini prompt-file: ${{ env.GITHUB_AW_PROMPT }} - max_tokens: 1000 temperature: 0.7 env: GITHUB_AW_PROMPT: /tmp/aw-prompts/prompt.txt diff --git a/pkg/workflow/claude_engine.go b/pkg/workflow/claude_engine.go index 5ae20b7591d..ae94ff1cc35 100644 --- a/pkg/workflow/claude_engine.go +++ b/pkg/workflow/claude_engine.go @@ -257,8 +257,14 @@ func (e *ClaudeEngine) convertStepToYAML(stepMap map[string]any) (string, error) if with, hasWith := stepMap["with"]; hasWith { if withMap, ok := with.(map[string]any); ok { stepYAML = append(stepYAML, " with:") - for key, value := range withMap { - stepYAML = append(stepYAML, fmt.Sprintf(" %s: %v", key, value)) + // Sort keys for stable output + keys := make([]string, 0, len(withMap)) + for key := range withMap { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + stepYAML = append(stepYAML, fmt.Sprintf(" %s: %v", key, withMap[key])) } } } diff --git a/pkg/workflow/claude_engine_test.go b/pkg/workflow/claude_engine_test.go index f5c9c905ae3..a3ec70f8c1d 100644 --- a/pkg/workflow/claude_engine_test.go +++ b/pkg/workflow/claude_engine_test.go @@ -251,3 +251,58 @@ func TestClaudeEngineWithoutVersion(t *testing.T) { t.Errorf("Expected action '%s' in step content:\n%s", expectedAction, stepContent) } } + +func TestClaudeEngineConvertStepToYAMLWithSection(t *testing.T) { + engine := NewClaudeEngine() + + // Test step with 'with' section to ensure keys are sorted + stepMap := map[string]any{ + "name": "Test step with sorted with section", + "uses": "actions/checkout@v4", + "with": map[string]any{ + "zebra": "value-z", + "alpha": "value-a", + "beta": "value-b", + "gamma": "value-g", + }, + } + + yaml, err := engine.convertStepToYAML(stepMap) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + // Verify that the with keys are in alphabetical order + lines := strings.Split(yaml, "\n") + withSection := false + withKeyOrder := []string{} + + for _, line := range lines { + if strings.TrimSpace(line) == "with:" { + withSection = true + continue + } + if withSection && strings.HasPrefix(strings.TrimSpace(line), "- ") { + // End of with section if we hit another top-level key + break + } + if withSection && strings.Contains(line, ":") { + // Extract the key (before the colon) + parts := strings.SplitN(strings.TrimSpace(line), ":", 2) + if len(parts) > 0 { + withKeyOrder = append(withKeyOrder, strings.TrimSpace(parts[0])) + } + } + } + + expectedOrder := []string{"alpha", "beta", "gamma", "zebra"} + if len(withKeyOrder) != len(expectedOrder) { + t.Errorf("Expected %d with keys, got %d", len(expectedOrder), len(withKeyOrder)) + } + + for i, key := range expectedOrder { + if i >= len(withKeyOrder) || withKeyOrder[i] != key { + t.Errorf("Expected with key at position %d to be '%s', got '%s'. Full order: %v", i, key, withKeyOrder[i], withKeyOrder) + } + } +} diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index 43bffe6f2b1..595f76aeff2 100644 --- a/pkg/workflow/codex_engine.go +++ b/pkg/workflow/codex_engine.go @@ -187,8 +187,14 @@ func (e *CodexEngine) convertStepToYAML(stepMap map[string]any) (string, error) if with, hasWith := stepMap["with"]; hasWith { if withMap, ok := with.(map[string]any); ok { stepYAML = append(stepYAML, " with:") - for key, value := range withMap { - stepYAML = append(stepYAML, fmt.Sprintf(" %s: %v", key, value)) + // Sort keys for stable output + keys := make([]string, 0, len(withMap)) + for key := range withMap { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + stepYAML = append(stepYAML, fmt.Sprintf(" %s: %v", key, withMap[key])) } } } diff --git a/pkg/workflow/codex_engine_test.go b/pkg/workflow/codex_engine_test.go index 161b7c024cf..0908f28c6bc 100644 --- a/pkg/workflow/codex_engine_test.go +++ b/pkg/workflow/codex_engine_test.go @@ -200,3 +200,58 @@ func TestCodexEngineExecutionIncludesGitHubAWPrompt(t *testing.T) { t.Error("Expected GITHUB_AW_PROMPT environment variable in codex execution steps") } } + +func TestCodexEngineConvertStepToYAMLWithSection(t *testing.T) { + engine := NewCodexEngine() + + // Test step with 'with' section to ensure keys are sorted + stepMap := map[string]any{ + "name": "Test step with sorted with section", + "uses": "actions/checkout@v4", + "with": map[string]any{ + "zebra": "value-z", + "alpha": "value-a", + "beta": "value-b", + "gamma": "value-g", + }, + } + + yaml, err := engine.convertStepToYAML(stepMap) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + // Verify that the with keys are in alphabetical order + lines := strings.Split(yaml, "\n") + withSection := false + withKeyOrder := []string{} + + for _, line := range lines { + if strings.TrimSpace(line) == "with:" { + withSection = true + continue + } + if withSection && strings.HasPrefix(strings.TrimSpace(line), "- ") { + // End of with section if we hit another top-level key + break + } + if withSection && strings.Contains(line, ":") { + // Extract the key (before the colon) + parts := strings.SplitN(strings.TrimSpace(line), ":", 2) + if len(parts) > 0 { + withKeyOrder = append(withKeyOrder, strings.TrimSpace(parts[0])) + } + } + } + + expectedOrder := []string{"alpha", "beta", "gamma", "zebra"} + if len(withKeyOrder) != len(expectedOrder) { + t.Errorf("Expected %d with keys, got %d", len(expectedOrder), len(withKeyOrder)) + } + + for i, key := range expectedOrder { + if i >= len(withKeyOrder) || withKeyOrder[i] != key { + t.Errorf("Expected with key at position %d to be '%s', got '%s'. Full order: %v", i, key, withKeyOrder[i], withKeyOrder) + } + } +} diff --git a/pkg/workflow/custom_engine.go b/pkg/workflow/custom_engine.go index a7f0a8eccca..24c877b5322 100644 --- a/pkg/workflow/custom_engine.go +++ b/pkg/workflow/custom_engine.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "sort" "strings" ) @@ -145,8 +146,14 @@ func (e *CustomEngine) convertStepToYAML(stepMap map[string]any) (string, error) if with, hasWith := stepMap["with"]; hasWith { if withMap, ok := with.(map[string]any); ok { stepYAML = append(stepYAML, " with:") - for key, value := range withMap { - stepYAML = append(stepYAML, fmt.Sprintf(" %s: %v", key, value)) + // Sort keys for stable output + keys := make([]string, 0, len(withMap)) + for key := range withMap { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + stepYAML = append(stepYAML, fmt.Sprintf(" %s: %v", key, withMap[key])) } } } diff --git a/pkg/workflow/custom_engine_test.go b/pkg/workflow/custom_engine_test.go index 39b2fa30423..0e79dd43101 100644 --- a/pkg/workflow/custom_engine_test.go +++ b/pkg/workflow/custom_engine_test.go @@ -240,3 +240,58 @@ func TestCustomEngineGetLogParserScript(t *testing.T) { t.Errorf("Expected log parser script 'parse_custom_log', got '%s'", script) } } + +func TestCustomEngineConvertStepToYAMLWithSection(t *testing.T) { + engine := NewCustomEngine() + + // Test step with 'with' section to ensure keys are sorted + stepMap := map[string]any{ + "name": "Test step with sorted with section", + "uses": "actions/checkout@v4", + "with": map[string]any{ + "zebra": "value-z", + "alpha": "value-a", + "beta": "value-b", + "gamma": "value-g", + }, + } + + yaml, err := engine.convertStepToYAML(stepMap) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + // Verify that the with keys are in alphabetical order + lines := strings.Split(yaml, "\n") + withSection := false + withKeyOrder := []string{} + + for _, line := range lines { + if strings.TrimSpace(line) == "with:" { + withSection = true + continue + } + if withSection && strings.HasPrefix(strings.TrimSpace(line), "- ") { + // End of with section if we hit another top-level key + break + } + if withSection && strings.Contains(line, ":") { + // Extract the key (before the colon) + parts := strings.SplitN(strings.TrimSpace(line), ":", 2) + if len(parts) > 0 { + withKeyOrder = append(withKeyOrder, strings.TrimSpace(parts[0])) + } + } + } + + expectedOrder := []string{"alpha", "beta", "gamma", "zebra"} + if len(withKeyOrder) != len(expectedOrder) { + t.Errorf("Expected %d with keys, got %d", len(expectedOrder), len(withKeyOrder)) + } + + for i, key := range expectedOrder { + if i >= len(withKeyOrder) || withKeyOrder[i] != key { + t.Errorf("Expected with key at position %d to be '%s', got '%s'. Full order: %v", i, key, withKeyOrder[i], withKeyOrder) + } + } +}