From 3254d3c827fa01b590d3416a3c7c2deb97123462 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 31 May 2026 01:25:04 +0000 Subject: [PATCH 1/3] Initial plan From 3f7f327190233ef7baf2c1123679a43ccf09a934 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 31 May 2026 01:33:12 +0000 Subject: [PATCH 2/3] chore: initial plan - P1 semantic refactoring Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- .github/workflows/daily-model-inventory.lock.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/daily-model-inventory.lock.yml b/.github/workflows/daily-model-inventory.lock.yml index 4ad7596a490..89bcc656530 100644 --- a/.github/workflows/daily-model-inventory.lock.yml +++ b/.github/workflows/daily-model-inventory.lock.yml @@ -1,4 +1,4 @@ -# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"941bf2df6cf246f352cfb0607cd13f711a6fa33bb44bfd133f7ff0ccd4f3aa33","body_hash":"eddd84cb14b52557f5fd4337e9e7d5fcf36fa38a1bbe0e74d773293aa86b64b6","strict":true,"agent_id":"copilot"} +# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"941bf2df6cf246f352cfb0607cd13f711a6fa33bb44bfd133f7ff0ccd4f3aa33","body_hash":"1411aa03bf28a3c98eff9d54cdee03ed6e87584a3b11c9f6dd740cb2b466ea00","strict":true,"agent_id":"copilot"} # gh-aw-manifest: {"version":1,"secrets":["ANTHROPIC_API_KEY","COPILOT_GITHUB_TOKEN","GEMINI_API_KEY","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GH_AW_OTEL_GRAFANA_AUTHORIZATION","GH_AW_OTEL_GRAFANA_ENDPOINT","GH_AW_OTEL_SENTRY_AUTHORIZATION","GH_AW_OTEL_SENTRY_ENDPOINT","GITHUB_TOKEN","OPENAI_API_KEY"],"actions":[{"repo":"actions/checkout","sha":"de0fac2e4500dabe0009e67214ff5f5447ce83dd","version":"v6.0.2"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-node","sha":"48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e","version":"v6.4.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.25.58"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.25.58"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.25.58"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.3.22"},{"image":"ghcr.io/github/github-mcp-server:v1.1.0"},{"image":"node:lts-alpine","digest":"sha256:2bdb65ed1dab192432bc31c95f94155ca5ad7fc1392fb7eb7526ab682fa5bf14","pinned_image":"node:lts-alpine@sha256:2bdb65ed1dab192432bc31c95f94155ca5ad7fc1392fb7eb7526ab682fa5bf14"}]} # ___ _ _ # / _ \ | | (_) From e1b9928d4ecc068cefc77fa322aa88fc893a7d49 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 31 May 2026 01:41:57 +0000 Subject: [PATCH 3/3] refactor(P1): consolidate YAML escapers, MCP config helper, and validation helpers P1.1: Add escapeYAMLSingleQuotedScalar to yaml_env_helpers.go, remove duplicates from observability_otlp.go and central_slash_command_workflow.go, and route all inline strings.ReplaceAll(x, \"'\", \"''\") uses through it. P1.2: Add renderJSONMCPConfigForEngine helper to mcp_renderer_factory.go; collapse the 5 standard JSON-MCP engine RenderMCPConfig methods (claude, gemini, antigravity, crush, opencode) to one-liner delegations, removing per-engine logger vars. P1.3: Extract formatWorkflowNotFoundError and formatMissingTriggerError to validation_helpers.go; extract workflowFileValidation struct and readAndValidateWorkflowFileTrigger to dispatch_workflow_file_resolver.go; refactor call_workflow_validation.go (collapsing duplicated lock/yml branches) and dispatch_workflow_validation.go to use all helpers. Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/workflow/antigravity_mcp.go | 9 +-- pkg/workflow/cache.go | 2 +- pkg/workflow/call_workflow_validation.go | 63 ++++------------ .../central_slash_command_workflow.go | 8 +- pkg/workflow/claude_mcp.go | 9 +-- pkg/workflow/compiler_experiments.go | 2 +- pkg/workflow/compiler_yaml.go | 2 +- pkg/workflow/crush_mcp.go | 9 +-- .../dispatch_workflow_file_resolver.go | 56 ++++++++++++++ pkg/workflow/dispatch_workflow_validation.go | 74 +++++++------------ pkg/workflow/engine_helpers.go | 2 +- pkg/workflow/expression_nodes.go | 2 +- pkg/workflow/gemini_mcp.go | 9 +-- pkg/workflow/mcp_renderer_factory.go | 8 ++ pkg/workflow/observability_otlp.go | 6 -- pkg/workflow/opencode_mcp.go | 8 +- pkg/workflow/service_ports.go | 2 +- pkg/workflow/validation_helpers.go | 16 ++++ pkg/workflow/yaml_env_helpers.go | 6 ++ 19 files changed, 139 insertions(+), 154 deletions(-) diff --git a/pkg/workflow/antigravity_mcp.go b/pkg/workflow/antigravity_mcp.go index d37d7b02c9f..770a516c632 100644 --- a/pkg/workflow/antigravity_mcp.go +++ b/pkg/workflow/antigravity_mcp.go @@ -2,16 +2,9 @@ package workflow import ( "strings" - - "github.com/github/gh-aw/pkg/logger" ) -var antigravityMCPLog = logger.New("workflow:antigravity_mcp") - // RenderMCPConfig renders MCP server configuration for Antigravity CLI func (e *AntigravityEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) error { - antigravityMCPLog.Printf("Rendering MCP config for Antigravity: tool_count=%d, mcp_tool_count=%d", len(tools), len(mcpTools)) - - // Antigravity uses JSON format without Copilot-specific fields and multi-line args - return renderDefaultJSONMCPConfig(yaml, tools, mcpTools, workflowData, "${RUNNER_TEMP}/gh-aw/mcp-config/mcp-servers.json") + return renderJSONMCPConfigForEngine("Antigravity", yaml, tools, mcpTools, workflowData, "${RUNNER_TEMP}/gh-aw/mcp-config/mcp-servers.json") } diff --git a/pkg/workflow/cache.go b/pkg/workflow/cache.go index bae1ff157bf..6842e21e031 100644 --- a/pkg/workflow/cache.go +++ b/pkg/workflow/cache.go @@ -567,7 +567,7 @@ func generateCacheMemoryGitSetupStep(builder *strings.Builder, cache CacheMemory // Single quotes in the value are escaped ('' in YAML single-quoted scalars) as defense-in-depth, // even though isValidFileExtension already rejects values containing single quotes at parse time. if len(cache.AllowedExtensions) > 0 { - escaped := strings.ReplaceAll(strings.Join(cache.AllowedExtensions, ":"), "'", "''") + escaped := escapeYAMLSingleQuotedScalar(strings.Join(cache.AllowedExtensions, ":")) fmt.Fprintf(builder, " GH_AW_ALLOWED_EXTENSIONS: '%s'\n", escaped) } builder.WriteString(" run: bash \"${RUNNER_TEMP}/gh-aw/actions/setup_cache_memory_git.sh\"\n") diff --git a/pkg/workflow/call_workflow_validation.go b/pkg/workflow/call_workflow_validation.go index 3ce8c2d953d..80619366f82 100644 --- a/pkg/workflow/call_workflow_validation.go +++ b/pkg/workflow/call_workflow_validation.go @@ -8,7 +8,6 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/parser" - "github.com/goccy/go-yaml" ) var callWorkflowValidationLog = newValidationLogger("call_workflow") @@ -74,75 +73,40 @@ func (c *Compiler) validateCallWorkflow(data *WorkflowData, workflowPath string) githubDir := filepath.Dir(currentDir) repoRoot := filepath.Dir(githubDir) workflowsDir := filepath.Join(repoRoot, constants.GetWorkflowDir()) - - notFoundErr := fmt.Errorf("call-workflow: workflow '%s' not found in %s\n\nChecked for: %s.md, %s.lock.yml, %s.yml\n\nTo fix:\n1. Verify the workflow file exists in %s/\n2. Ensure the filename matches exactly (case-sensitive)\n3. Use the filename without extension in your configuration", workflowName, workflowsDir, workflowName, workflowName, workflowName, workflowsDir) + notFoundErr := formatWorkflowNotFoundError("call-workflow", workflowName, workflowsDir) if returnErr := collector.Add(notFoundErr); returnErr != nil { return returnErr } continue } - // Validate that the workflow supports workflow_call. - // Priority: .lock.yml > .yml > .md (same-batch compilation target) - if fileResult.lockExists { - workflowContent, readErr := os.ReadFile(fileResult.lockPath) // #nosec G304 -- lockPath is validated via isPathWithinDir() in findWorkflowFile() before being returned - if readErr != nil { - fileReadErr := fmt.Errorf("call-workflow: failed to read workflow file %s: %w", fileResult.lockPath, readErr) + // Validate trigger: .lock.yml and .yml take priority over .md + v := readAndValidateWorkflowFileTrigger(fileResult, "workflow_call") + if v.filePath != "" { + // A compiled file (.lock.yml or .yml) was found + if v.readErr != nil { + fileReadErr := fmt.Errorf("call-workflow: failed to read workflow file %s: %w", v.filePath, v.readErr) if returnErr := collector.Add(fileReadErr); returnErr != nil { return returnErr } continue } - var workflow map[string]any - if err := yaml.Unmarshal(workflowContent, &workflow); err != nil { - parseErr := fmt.Errorf("call-workflow: failed to parse workflow file %s: %w", fileResult.lockPath, err) + if v.parseErr != nil { + parseErr := fmt.Errorf("call-workflow: failed to parse workflow file %s: %w", v.filePath, v.parseErr) if returnErr := collector.Add(parseErr); returnErr != nil { return returnErr } continue } - onSection, hasOn := workflow["on"] - if !hasOn { + if v.noOnSection { onErr := fmt.Errorf("call-workflow: workflow '%s' does not have an 'on' trigger section", workflowName) if returnErr := collector.Add(onErr); returnErr != nil { return returnErr } continue } - if !containsWorkflowCall(onSection) { - callErr := fmt.Errorf("call-workflow: workflow '%s' does not support workflow_call trigger (must include 'workflow_call' in the 'on' section)", workflowName) - if returnErr := collector.Add(callErr); returnErr != nil { - return returnErr - } - continue - } - } else if fileResult.ymlExists { - workflowContent, readErr := os.ReadFile(fileResult.ymlPath) // #nosec G304 -- ymlPath is validated via isPathWithinDir() in findWorkflowFile() before being returned - if readErr != nil { - fileReadErr := fmt.Errorf("call-workflow: failed to read workflow file %s: %w", fileResult.ymlPath, readErr) - if returnErr := collector.Add(fileReadErr); returnErr != nil { - return returnErr - } - continue - } - var workflow map[string]any - if err := yaml.Unmarshal(workflowContent, &workflow); err != nil { - parseErr := fmt.Errorf("call-workflow: failed to parse workflow file %s: %w", fileResult.ymlPath, err) - if returnErr := collector.Add(parseErr); returnErr != nil { - return returnErr - } - continue - } - onSection, hasOn := workflow["on"] - if !hasOn { - onErr := fmt.Errorf("call-workflow: workflow '%s' does not have an 'on' trigger section", workflowName) - if returnErr := collector.Add(onErr); returnErr != nil { - return returnErr - } - continue - } - if !containsWorkflowCall(onSection) { - callErr := fmt.Errorf("call-workflow: workflow '%s' does not support workflow_call trigger (must include 'workflow_call' in the 'on' section)", workflowName) + if !v.hasTrigger { + callErr := formatMissingTriggerError("call-workflow", workflowName, "workflow_call") if returnErr := collector.Add(callErr); returnErr != nil { return returnErr } @@ -160,7 +124,7 @@ func (c *Compiler) validateCallWorkflow(data *WorkflowData, workflowPath string) continue } if !mdHasCall { - callErr := fmt.Errorf("call-workflow: workflow '%s' does not support workflow_call trigger (must include 'workflow_call' in the 'on' section)", workflowName) + callErr := formatMissingTriggerError("call-workflow", workflowName, "workflow_call") if returnErr := collector.Add(callErr); returnErr != nil { return returnErr } @@ -179,6 +143,7 @@ func (c *Compiler) validateCallWorkflow(data *WorkflowData, workflowPath string) return collector.FormattedError("call-workflow") } + // extractWorkflowCallInputs parses a workflow file and extracts the workflow_call inputs schema. // Returns a map of input definitions that can be used to generate MCP tool schemas. func extractWorkflowCallInputs(workflowPath string) (map[string]any, error) { diff --git a/pkg/workflow/central_slash_command_workflow.go b/pkg/workflow/central_slash_command_workflow.go index 853a33c3e88..6439a42a125 100644 --- a/pkg/workflow/central_slash_command_workflow.go +++ b/pkg/workflow/central_slash_command_workflow.go @@ -309,8 +309,8 @@ jobs: - name: Route slash command uses: ` + getActionPin("actions/github-script") + ` env: - GH_AW_SLASH_ROUTING: '` + escapeSingleQuotedYAMLString(string(slashRoutesJSON)) + `' - GH_AW_LABEL_ROUTING: '` + escapeSingleQuotedYAMLString(string(labelRoutesJSON)) + `' + GH_AW_SLASH_ROUTING: '` + escapeYAMLSingleQuotedScalar(string(slashRoutesJSON)) + `' + GH_AW_LABEL_ROUTING: '` + escapeYAMLSingleQuotedScalar(string(labelRoutesJSON)) + `' with: script: | const { setupGlobals } = require('` + SetupActionDestination + `/setup_globals.cjs'); @@ -496,7 +496,3 @@ func uniqueSorted(values []string) []string { sort.Strings(result) return result } - -func escapeSingleQuotedYAMLString(input string) string { - return strings.ReplaceAll(input, "'", "''") -} diff --git a/pkg/workflow/claude_mcp.go b/pkg/workflow/claude_mcp.go index 459a6cabf94..77aac3d597b 100644 --- a/pkg/workflow/claude_mcp.go +++ b/pkg/workflow/claude_mcp.go @@ -2,16 +2,9 @@ package workflow import ( "strings" - - "github.com/github/gh-aw/pkg/logger" ) -var claudeMCPLog = logger.New("workflow:claude_mcp") - // RenderMCPConfig renders the MCP configuration for Claude engine func (e *ClaudeEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) error { - claudeMCPLog.Printf("Rendering MCP config for Claude: tool_count=%d, mcp_tool_count=%d", len(tools), len(mcpTools)) - - // Claude uses JSON format without Copilot-specific fields and multi-line args - return renderDefaultJSONMCPConfig(yaml, tools, mcpTools, workflowData, "${RUNNER_TEMP}/gh-aw/mcp-config/mcp-servers.json") + return renderJSONMCPConfigForEngine("Claude", yaml, tools, mcpTools, workflowData, "${RUNNER_TEMP}/gh-aw/mcp-config/mcp-servers.json") } diff --git a/pkg/workflow/compiler_experiments.go b/pkg/workflow/compiler_experiments.go index 0f0e319936a..c8c59dbf029 100644 --- a/pkg/workflow/compiler_experiments.go +++ b/pkg/workflow/compiler_experiments.go @@ -403,7 +403,7 @@ func (c *Compiler) generatePickExperimentStep(data *WorkflowData, experimentName " id: pick-experiment\n", fmt.Sprintf(" uses: %s\n", getCachedActionPin("actions/github-script", data)), " env:\n", - fmt.Sprintf(" GH_AW_EXPERIMENT_SPEC: '%s'\n", strings.ReplaceAll(specJSON, "'", "''")), + fmt.Sprintf(" GH_AW_EXPERIMENT_SPEC: '%s'\n", escapeYAMLSingleQuotedScalar(specJSON)), fmt.Sprintf(" GH_AW_EXPERIMENT_STATE_FILE: %s\n", experimentStateFile), fmt.Sprintf(" GH_AW_EXPERIMENT_STATE_DIR: %s\n", experimentsCacheDir), " with:\n", diff --git a/pkg/workflow/compiler_yaml.go b/pkg/workflow/compiler_yaml.go index 98f57c8d37f..c617c896efc 100644 --- a/pkg/workflow/compiler_yaml.go +++ b/pkg/workflow/compiler_yaml.go @@ -899,7 +899,7 @@ func (c *Compiler) generateCreateAwInfo(yaml *strings.Builder, data *WorkflowDat if data.EngineConfig != nil && data.EngineConfig.TokenWeights != nil && len(data.EngineConfig.TokenWeights.Multipliers) > 0 { if tokenWeightsJSON, err := json.Marshal(data.EngineConfig.TokenWeights); err == nil { // Escape single quotes for YAML single-quoted scalar safety - escapedTokenWeightsJSON := strings.ReplaceAll(string(tokenWeightsJSON), "'", "''") + escapedTokenWeightsJSON := escapeYAMLSingleQuotedScalar(string(tokenWeightsJSON)) fmt.Fprintf(yaml, " GH_AW_INFO_TOKEN_WEIGHTS: '%s'\n", escapedTokenWeightsJSON) } } diff --git a/pkg/workflow/crush_mcp.go b/pkg/workflow/crush_mcp.go index dcb15cc8d80..57c58177cfa 100644 --- a/pkg/workflow/crush_mcp.go +++ b/pkg/workflow/crush_mcp.go @@ -2,16 +2,9 @@ package workflow import ( "strings" - - "github.com/github/gh-aw/pkg/logger" ) -var crushMCPLog = logger.New("workflow:crush_mcp") - // RenderMCPConfig renders MCP server configuration for Crush CLI func (e *CrushEngine) RenderMCPConfig(sb *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) error { - crushMCPLog.Printf("Rendering MCP config for Crush: tool_count=%d, mcp_tool_count=%d", len(tools), len(mcpTools)) - - // Crush uses JSON format without Copilot-specific fields and multi-line args - return renderDefaultJSONMCPConfig(sb, tools, mcpTools, workflowData, "/tmp/gh-aw/mcp-config/mcp-servers.json") + return renderJSONMCPConfigForEngine("Crush", sb, tools, mcpTools, workflowData, "/tmp/gh-aw/mcp-config/mcp-servers.json") } diff --git a/pkg/workflow/dispatch_workflow_file_resolver.go b/pkg/workflow/dispatch_workflow_file_resolver.go index 3254ec7c86b..900e22565f6 100644 --- a/pkg/workflow/dispatch_workflow_file_resolver.go +++ b/pkg/workflow/dispatch_workflow_file_resolver.go @@ -9,6 +9,7 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/fileutil" "github.com/github/gh-aw/pkg/parser" + "github.com/goccy/go-yaml" ) // getCurrentWorkflowName extracts the workflow name from the file path @@ -126,3 +127,58 @@ func extractMDWorkflowDispatchInputs(mdPath string) (map[string]any, error) { dispatchWorkflowValidationLog.Printf("Extracted %d workflow_dispatch input(s) from: %s", len(inputs), mdPath) return inputs, nil } + +// workflowFileValidation holds the result of readAndValidateWorkflowFileTrigger. +type workflowFileValidation struct { + // filePath is the compiled file that was read (.lock.yml or .yml). + // Empty when neither file exists (md-only case). + filePath string + // readErr is set when the file could not be read. + readErr error + // parseErr is set when the file content could not be parsed as YAML. + parseErr error + // noOnSection is true when the workflow has no 'on:' section at all. + noOnSection bool + // hasTrigger is true when the workflow's 'on:' section includes triggerName. + hasTrigger bool +} + +// readAndValidateWorkflowFileTrigger reads the compiled workflow file (preferring +// .lock.yml over .yml) from fileResult and checks whether it declares triggerName. +// Returns an empty workflowFileValidation (filePath == "") when neither compiled +// file exists; callers handle the .md-only case separately. +func readAndValidateWorkflowFileTrigger(fileResult *findWorkflowFileResult, triggerName string) *workflowFileValidation { + result := &workflowFileValidation{} + + if !fileResult.lockExists && !fileResult.ymlExists { + return result // md-only case — caller handles it + } + + if fileResult.lockExists { + result.filePath = fileResult.lockPath + } else { + result.filePath = fileResult.ymlPath + } + + content, err := os.ReadFile(result.filePath) // #nosec G304 -- paths are validated via isPathWithinDir() in findWorkflowFile() + if err != nil { + result.readErr = err + return result + } + + var workflow map[string]any + if err = yaml.Unmarshal(content, &workflow); err != nil { + result.parseErr = err + return result + } + + onSection, hasOn := workflow["on"] + if !hasOn { + result.noOnSection = true + return result + } + + result.hasTrigger = containsTrigger(onSection, triggerName) + return result +} + diff --git a/pkg/workflow/dispatch_workflow_validation.go b/pkg/workflow/dispatch_workflow_validation.go index b23195c15b4..e53cb299fa4 100644 --- a/pkg/workflow/dispatch_workflow_validation.go +++ b/pkg/workflow/dispatch_workflow_validation.go @@ -3,12 +3,10 @@ package workflow import ( "errors" "fmt" - "os" "path/filepath" "strings" "github.com/github/gh-aw/pkg/constants" - "github.com/goccy/go-yaml" ) var dispatchWorkflowValidationLog = newValidationLogger("dispatch_workflow") @@ -60,38 +58,47 @@ func (c *Compiler) validateDispatchWorkflow(data *WorkflowData, workflowPath str githubDir := filepath.Dir(currentDir) repoRoot := filepath.Dir(githubDir) workflowsDir := filepath.Join(repoRoot, constants.GetWorkflowDir()) - notFoundErr := fmt.Errorf("dispatch-workflow: workflow '%s' not found in %s\n\nChecked for: %s.md, %s.lock.yml, %s.yml\n\nTo fix:\n1. Verify the workflow file exists in %s/\n2. Ensure the filename matches exactly (case-sensitive)\n3. Use the filename without extension in your configuration", workflowName, workflowsDir, workflowName, workflowName, workflowName, workflowsDir) + notFoundErr := formatWorkflowNotFoundError("dispatch-workflow", workflowName, workflowsDir) if returnErr := collector.Add(notFoundErr); returnErr != nil { return returnErr } continue } - var workflowContent []byte // #nosec G304 -- All file paths are validated via isPathWithinDir before use - var workflowFile string - var readErr error - - if fileResult.lockExists { - workflowFile = fileResult.lockPath - workflowContent, readErr = os.ReadFile(fileResult.lockPath) // #nosec G304 -- Path is validated via isPathWithinDir in findWorkflowFile - if readErr != nil { - fileReadErr := fmt.Errorf("dispatch-workflow: failed to read workflow file %s: %w", fileResult.lockPath, readErr) + v := readAndValidateWorkflowFileTrigger(fileResult, "workflow_dispatch") + if v.filePath != "" { + // A compiled file (.lock.yml or .yml) was found + if v.readErr != nil { + fileReadErr := fmt.Errorf("dispatch-workflow: failed to read workflow file %s: %w", v.filePath, v.readErr) if returnErr := collector.Add(fileReadErr); returnErr != nil { return returnErr } continue } - } else if fileResult.ymlExists { - workflowFile = fileResult.ymlPath - workflowContent, readErr = os.ReadFile(fileResult.ymlPath) // #nosec G304 -- Path is validated via isPathWithinDir in findWorkflowFile - if readErr != nil { - fileReadErr := fmt.Errorf("dispatch-workflow: failed to read workflow file %s: %w", fileResult.ymlPath, readErr) - if returnErr := collector.Add(fileReadErr); returnErr != nil { + if v.parseErr != nil { + parseErr := fmt.Errorf("dispatch-workflow: failed to parse workflow file %s: %w", v.filePath, v.parseErr) + if returnErr := collector.Add(parseErr); returnErr != nil { + return returnErr + } + continue + } + if v.noOnSection { + onSectionErr := fmt.Errorf("dispatch-workflow: workflow '%s' does not have an 'on' trigger section", workflowName) + if returnErr := collector.Add(onSectionErr); returnErr != nil { + return returnErr + } + continue + } + if !v.hasTrigger { + dispatchErr := formatMissingTriggerError("dispatch-workflow", workflowName, "workflow_dispatch") + if returnErr := collector.Add(dispatchErr); returnErr != nil { return returnErr } continue } + dispatchWorkflowValidationLog.Printf("Workflow '%s' is valid for dispatch (found in %s)", workflowName, v.filePath) } else { + // Only .md exists — it may be a same-batch compilation target. mdHasDispatch, checkErr := mdHasWorkflowDispatch(fileResult.mdPath) if checkErr != nil { readErr := fmt.Errorf("dispatch-workflow: failed to read workflow source %s: %w", fileResult.mdPath, checkErr) @@ -101,43 +108,14 @@ func (c *Compiler) validateDispatchWorkflow(data *WorkflowData, workflowPath str continue } if !mdHasDispatch { - dispatchErr := fmt.Errorf("dispatch-workflow: workflow '%s' does not support workflow_dispatch trigger (must include 'workflow_dispatch' in the 'on' section)", workflowName) + dispatchErr := formatMissingTriggerError("dispatch-workflow", workflowName, "workflow_dispatch") if returnErr := collector.Add(dispatchErr); returnErr != nil { return returnErr } continue } dispatchWorkflowValidationLog.Printf("Workflow '%s' is valid for dispatch (found .md source at %s with workflow_dispatch trigger)", workflowName, fileResult.mdPath) - continue - } - - var workflow map[string]any - if err := yaml.Unmarshal(workflowContent, &workflow); err != nil { - parseErr := fmt.Errorf("dispatch-workflow: failed to parse workflow file %s: %w", workflowFile, err) - if returnErr := collector.Add(parseErr); returnErr != nil { - return returnErr - } - continue - } - - onSection, hasOn := workflow["on"] - if !hasOn { - onSectionErr := fmt.Errorf("dispatch-workflow: workflow '%s' does not have an 'on' trigger section", workflowName) - if returnErr := collector.Add(onSectionErr); returnErr != nil { - return returnErr - } - continue } - - if !containsWorkflowDispatch(onSection) { - dispatchErr := fmt.Errorf("dispatch-workflow: workflow '%s' does not support workflow_dispatch trigger (must include 'workflow_dispatch' in the 'on' section)", workflowName) - if returnErr := collector.Add(dispatchErr); returnErr != nil { - return returnErr - } - continue - } - - dispatchWorkflowValidationLog.Printf("Workflow '%s' is valid for dispatch (found in %s)", workflowName, workflowFile) } dispatchWorkflowValidationLog.Printf("Dispatch workflow validation completed: error_count=%d, total_workflows=%d", collector.Count(), len(config.Workflows)) diff --git a/pkg/workflow/engine_helpers.go b/pkg/workflow/engine_helpers.go index 399ef739d90..f0e7152e5de 100644 --- a/pkg/workflow/engine_helpers.go +++ b/pkg/workflow/engine_helpers.go @@ -271,7 +271,7 @@ func yamlStringValue(value string) string { return value } // Single-quote the value, escaping any embedded single quotes by doubling them. - return "'" + strings.ReplaceAll(value, "'", "''") + "'" + return "'" + escapeYAMLSingleQuotedScalar(value) + "'" } // FilterEnvForSecrets filters environment variables to only include allowed secrets. diff --git a/pkg/workflow/expression_nodes.go b/pkg/workflow/expression_nodes.go index 70bc2eb0fa5..4fac1ab1e2b 100644 --- a/pkg/workflow/expression_nodes.go +++ b/pkg/workflow/expression_nodes.go @@ -169,7 +169,7 @@ type StringLiteralNode struct { func (s *StringLiteralNode) Render() string { // GitHub Actions single-quoted strings escape embedded single quotes by doubling them. - escaped := strings.ReplaceAll(s.Value, "'", "''") + escaped := escapeYAMLSingleQuotedScalar(s.Value) return "'" + escaped + "'" } diff --git a/pkg/workflow/gemini_mcp.go b/pkg/workflow/gemini_mcp.go index 444c51e78ff..dbd34be2597 100644 --- a/pkg/workflow/gemini_mcp.go +++ b/pkg/workflow/gemini_mcp.go @@ -2,16 +2,9 @@ package workflow import ( "strings" - - "github.com/github/gh-aw/pkg/logger" ) -var geminiMCPLog = logger.New("workflow:gemini_mcp") - // RenderMCPConfig renders MCP server configuration for Gemini CLI func (e *GeminiEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) error { - geminiMCPLog.Printf("Rendering MCP config for Gemini: tool_count=%d, mcp_tool_count=%d", len(tools), len(mcpTools)) - - // Gemini uses JSON format without Copilot-specific fields and multi-line args - return renderDefaultJSONMCPConfig(yaml, tools, mcpTools, workflowData, "${RUNNER_TEMP}/gh-aw/mcp-config/mcp-servers.json") + return renderJSONMCPConfigForEngine("Gemini", yaml, tools, mcpTools, workflowData, "${RUNNER_TEMP}/gh-aw/mcp-config/mcp-servers.json") } diff --git a/pkg/workflow/mcp_renderer_factory.go b/pkg/workflow/mcp_renderer_factory.go index 9e90051f712..a8448889230 100644 --- a/pkg/workflow/mcp_renderer_factory.go +++ b/pkg/workflow/mcp_renderer_factory.go @@ -110,6 +110,14 @@ func renderDefaultJSONMCPConfig( }) } +// renderJSONMCPConfigForEngine is a shared base used by the 5 standard JSON-MCP engines +// (Claude, Gemini, Antigravity, Crush, OpenCode). It logs the engine name and tool counts, +// then delegates to renderDefaultJSONMCPConfig with the engine-specific config path. +func renderJSONMCPConfigForEngine(engineName string, sb *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData, configPath string) error { + mcpRenderingLog.Printf("Rendering MCP config for %s: tool_count=%d, mcp_tool_count=%d", engineName, len(tools), len(mcpTools)) + return renderDefaultJSONMCPConfig(sb, tools, mcpTools, workflowData, configPath) +} + // renderStandardJSONMCPConfig is a shared helper for JSON MCP config rendering used by // Claude, Gemini, Copilot, and Codex engines. It consolidates the repeated sequence of: // buildMCPRendererFactory → buildMCPGatewayConfig → buildStandardJSONMCPRenderers → RenderJSONMCPConfig. diff --git a/pkg/workflow/observability_otlp.go b/pkg/workflow/observability_otlp.go index 1d5d61d258c..e1814d88b7b 100644 --- a/pkg/workflow/observability_otlp.go +++ b/pkg/workflow/observability_otlp.go @@ -19,12 +19,6 @@ var otlpLog = logger.New("workflow:observability_otlp") // equals (`=`) per the OpenTelemetry env-var resource attribute grammar. var otelResourceValueEscaper = strings.NewReplacer(`\`, `\\`, ",", `\,`, "=", `\=`) -// escapeYAMLSingleQuotedScalar escapes single quotes for YAML single-quoted -// scalars by doubling each `'` per YAML 1.2. -func escapeYAMLSingleQuotedScalar(value string) string { - return strings.ReplaceAll(value, "'", "''") -} - var sentryEndpointExpressionPattern = regexp.MustCompile(`(?i)^\$\{\{\s*secrets\.` + regexp.QuoteMeta(constants.OTELSentryEndpointSecretName) + `\s*\}\}$`) func normalizeOTLPHeadersForEndpoint(raw any, endpoint string) string { diff --git a/pkg/workflow/opencode_mcp.go b/pkg/workflow/opencode_mcp.go index 169c9669635..e2f76050540 100644 --- a/pkg/workflow/opencode_mcp.go +++ b/pkg/workflow/opencode_mcp.go @@ -2,15 +2,9 @@ package workflow import ( "strings" - - "github.com/github/gh-aw/pkg/logger" ) -var openCodeMCPLog = logger.New("workflow:opencode_mcp") - // RenderMCPConfig renders MCP server configuration for OpenCode CLI func (e *OpenCodeEngine) RenderMCPConfig(sb *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) error { - openCodeMCPLog.Printf("Rendering MCP config for OpenCode: tool_count=%d, mcp_tool_count=%d", len(tools), len(mcpTools)) - - return renderDefaultJSONMCPConfig(sb, tools, mcpTools, workflowData, "/tmp/gh-aw/mcp-config/mcp-servers.json") + return renderJSONMCPConfigForEngine("OpenCode", sb, tools, mcpTools, workflowData, "/tmp/gh-aw/mcp-config/mcp-servers.json") } diff --git a/pkg/workflow/service_ports.go b/pkg/workflow/service_ports.go index 05abf553819..ce3cd4797ad 100644 --- a/pkg/workflow/service_ports.go +++ b/pkg/workflow/service_ports.go @@ -122,7 +122,7 @@ func ExtractServicePortExpressions(servicesYAML string) (string, []string) { warnings = append(warnings, fmt.Sprintf("service %q: %s", serviceID, w)) } for _, cp := range containerPorts { - escapedServiceID := strings.ReplaceAll(serviceID, "'", "''") + escapedServiceID := escapeYAMLSingleQuotedScalar(serviceID) expr := fmt.Sprintf("${{ job.services['%s'].ports['%d'] }}", escapedServiceID, cp) expressions = append(expressions, expr) } diff --git a/pkg/workflow/validation_helpers.go b/pkg/workflow/validation_helpers.go index 6cef6a994cf..b433d8f9e04 100644 --- a/pkg/workflow/validation_helpers.go +++ b/pkg/workflow/validation_helpers.go @@ -221,3 +221,19 @@ func containsTrigger(onSection any, triggerName string) bool { validationHelpersLog.Printf("containsTrigger: trigger=%s, found=%t", triggerName, found) return found } + +// formatWorkflowNotFoundError returns the standard "workflow not found" error used by +// call-workflow and dispatch-workflow validators. workflowType is the human-readable +// prefix (e.g. "call-workflow" or "dispatch-workflow"). +func formatWorkflowNotFoundError(workflowType, workflowName, workflowsDir string) error { + return fmt.Errorf("%s: workflow '%s' not found in %s\n\nChecked for: %s.md, %s.lock.yml, %s.yml\n\nTo fix:\n1. Verify the workflow file exists in %s/\n2. Ensure the filename matches exactly (case-sensitive)\n3. Use the filename without extension in your configuration", + workflowType, workflowName, workflowsDir, workflowName, workflowName, workflowName, workflowsDir) +} + +// formatMissingTriggerError returns the standard error for a workflow that lacks the +// expected GitHub Actions trigger in its 'on:' section. workflowType is the prefix +// (e.g. "call-workflow"), triggerName is the expected trigger (e.g. "workflow_call"). +func formatMissingTriggerError(workflowType, workflowName, triggerName string) error { + return fmt.Errorf("%s: workflow '%s' does not support %s trigger (must include '%s' in the 'on' section)", + workflowType, workflowName, triggerName, triggerName) +} diff --git a/pkg/workflow/yaml_env_helpers.go b/pkg/workflow/yaml_env_helpers.go index 631a8e7b3b5..eef502630c0 100644 --- a/pkg/workflow/yaml_env_helpers.go +++ b/pkg/workflow/yaml_env_helpers.go @@ -11,6 +11,12 @@ import ( var envLog = logger.New("workflow:env") +// escapeYAMLSingleQuotedScalar escapes single quotes for YAML single-quoted +// scalars by doubling each ' per YAML 1.2. +func escapeYAMLSingleQuotedScalar(value string) string { + return strings.ReplaceAll(value, "'", "''") +} + // writeYAMLEnv emits a single YAML env variable with proper escaping. // Uses %q to produce a valid YAML double-quoted scalar that escapes ", \, newlines, and control characters, // preventing YAML structure injection from frontmatter-derived values.