diff --git a/pkg/workflow/compiler_yaml_prompt.go b/pkg/workflow/compiler_yaml_prompt.go index 22f4f94116f..70f0aad1c20 100644 --- a/pkg/workflow/compiler_yaml_prompt.go +++ b/pkg/workflow/compiler_yaml_prompt.go @@ -73,7 +73,10 @@ func (c *Compiler) generatePrompt(yaml *strings.Builder, data *WorkflowData, pre allExpressionMappings = mergeKnownNeedsExpressions(allExpressionMappings, knownNeedsExpressions) } - c.generateInterpolationAndTemplateStep(yaml, expressionMappings, data) + hasRuntimeImports := sliceutil.Any(userPromptChunks, func(chunk string) bool { + return strings.Contains(chunk, "{{#runtime-import ") || strings.Contains(chunk, "{{#runtime-import? ") + }) + c.generateInterpolationAndTemplateStep(yaml, expressionMappings, data, hasRuntimeImports) if len(allExpressionMappings) > 0 { generatePlaceholderSubstitutionStep(yaml, allExpressionMappings, " ", data) diff --git a/pkg/workflow/compiler_yaml_test.go b/pkg/workflow/compiler_yaml_test.go index 130f6e2996c..bb897e9ec2f 100644 --- a/pkg/workflow/compiler_yaml_test.go +++ b/pkg/workflow/compiler_yaml_test.go @@ -2096,3 +2096,67 @@ func TestAddCustomStepsAsIsTrimsStructuralTrailingSpaces(t *testing.T) { }) } } + +// TestInterpolationStepPresentWithGitHubFalse verifies the bug fix for the scenario where +// a workflow has tools.github: false (no GitHub MCP server), no template expressions, and no +// {{#if}} blocks. Before the fix the compiler skipped the "Interpolate variables and render +// templates" step because it didn't account for the {{#runtime-import}} self-import macro that +// is always emitted in normal (non-inline) compilation mode. This caused the agent to receive +// an unresolved macro and no effective instructions. +func TestInterpolationStepPresentWithGitHubFalse(t *testing.T) { + tmpDir := testutil.TempDir(t, "interpolation-step-github-false") + workflowDir := filepath.Join(tmpDir, ".github", "workflows") + if err := os.MkdirAll(workflowDir, 0755); err != nil { + t.Fatalf("failed to create workflow directory: %v", err) + } + + // Minimal workflow that previously triggered the bug: + // - engine.id set (no GitHub tool inferred) + // - tools.github: false (hasGitHubContext == false) + // - no {{#if}} or ${{ }} in body (hasTemplatePattern == false, hasExpressions == false) + workflowContent := `--- +on: repository_dispatch +permissions: + contents: read +engine: + id: claude +tools: + edit: + github: false +safe-outputs: + create-pull-request: +--- + +Do some important work. +` + workflowPath := filepath.Join(workflowDir, "test-workflow.md") + if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { + t.Fatalf("failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("compilation failed: %v", err) + } + + lockPath := strings.TrimSuffix(workflowPath, ".md") + ".lock.yml" + lockBytes, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("failed to read lock file: %v", err) + } + lockContent := string(lockBytes) + + // The compiled lock must contain a runtime-import macro (always emitted in normal mode). + if !strings.Contains(lockContent, "{{#runtime-import") { + t.Error("expected lock file to contain a {{#runtime-import}} macro") + } + + // And it must contain the interpolation step to resolve that macro. + if !strings.Contains(lockContent, "Interpolate variables and render templates") { + t.Error("expected lock file to contain 'Interpolate variables and render templates' step, " + + "but it was absent; the {{#runtime-import}} macro will not be resolved at runtime") + } + if !strings.Contains(lockContent, "interpolate_prompt.cjs") { + t.Error("expected lock file to reference interpolate_prompt.cjs in the interpolation step") + } +} diff --git a/pkg/workflow/runtime_import_expansion_integration_test.go b/pkg/workflow/runtime_import_expansion_integration_test.go new file mode 100644 index 00000000000..75eb3423e84 --- /dev/null +++ b/pkg/workflow/runtime_import_expansion_integration_test.go @@ -0,0 +1,199 @@ +//go:build integration + +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/github/gh-aw/pkg/stringutil" + "github.com/github/gh-aw/pkg/testutil" +) + +// TestRuntimeImportExpansionWithGitHubFalse verifies the primary regression scenario: +// a workflow with tools.github: false, no expressions, and no {{#if}} blocks must still +// receive the "Interpolate variables and render templates" step so that the +// {{#runtime-import}} self-import macro emitted by the compiler is resolved at runtime. +func TestRuntimeImportExpansionWithGitHubFalse(t *testing.T) { + tmpDir := testutil.TempDir(t, "runtime-import-expansion-github-false") + workflowDir := filepath.Join(tmpDir, ".github", "workflows") + if err := os.MkdirAll(workflowDir, 0755); err != nil { + t.Fatalf("failed to create workflow directory: %v", err) + } + + workflowContent := `--- +on: repository_dispatch +permissions: + contents: read +engine: + id: claude +tools: + edit: + github: false +safe-outputs: + create-pull-request: +--- + +Do some important work. +` + workflowPath := filepath.Join(workflowDir, "test-workflow.md") + if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { + t.Fatalf("failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("compilation failed: %v", err) + } + + lockPath := stringutil.MarkdownToLockFile(workflowPath) + lockBytes, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("failed to read lock file: %v", err) + } + lockContent := string(lockBytes) + + // The compiler always emits a self-import runtime-import macro in normal mode. + if !strings.Contains(lockContent, "{{#runtime-import") { + t.Error("lock file must contain a {{#runtime-import}} macro") + } + + // The interpolation step must be present to resolve that macro at runtime. + if !strings.Contains(lockContent, "Interpolate variables and render templates") { + t.Error("lock file must contain 'Interpolate variables and render templates' step so that {{#runtime-import}} is resolved") + } + if !strings.Contains(lockContent, "interpolate_prompt.cjs") { + t.Error("lock file must reference interpolate_prompt.cjs in the interpolation step") + } +} + +// TestRuntimeImportExpansionWithOptionalMacro verifies that a workflow embedding an +// optional {{#runtime-import? ...}} macro in the body also gets the interpolation step, +// even when tools.github is disabled and there are no template expressions. +func TestRuntimeImportExpansionWithOptionalMacro(t *testing.T) { + tmpDir := testutil.TempDir(t, "runtime-import-expansion-optional") + workflowDir := filepath.Join(tmpDir, ".github", "workflows") + sharedDir := filepath.Join(tmpDir, ".github", "shared") + if err := os.MkdirAll(workflowDir, 0755); err != nil { + t.Fatalf("failed to create workflow directory: %v", err) + } + if err := os.MkdirAll(sharedDir, 0755); err != nil { + t.Fatalf("failed to create shared directory: %v", err) + } + + // Optional shared file - its presence is not required for the workflow to compile. + if err := os.WriteFile(filepath.Join(sharedDir, "extra.md"), []byte("# Extra instructions\n"), 0644); err != nil { + t.Fatalf("failed to write shared file: %v", err) + } + + workflowContent := `--- +on: issues +permissions: + contents: read + issues: read +engine: claude +tools: + github: false +strict: false +--- + +Core task description. + +{{#runtime-import? .github/shared/extra.md}} +` + workflowPath := filepath.Join(workflowDir, "optional-import.md") + if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { + t.Fatalf("failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("compilation failed: %v", err) + } + + lockPath := stringutil.MarkdownToLockFile(workflowPath) + lockBytes, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("failed to read lock file: %v", err) + } + lockContent := string(lockBytes) + + // The optional macro must be preserved in the lock file. + if !strings.Contains(lockContent, "{{#runtime-import") { + t.Error("lock file must contain a {{#runtime-import}} macro (either the self-import or the optional user macro)") + } + + // The interpolation step must be present so the optional macro is expanded at runtime. + if !strings.Contains(lockContent, "Interpolate variables and render templates") { + t.Error("lock file must contain 'Interpolate variables and render templates' step so that {{#runtime-import?}} is resolved") + } + if !strings.Contains(lockContent, "interpolate_prompt.cjs") { + t.Error("lock file must reference interpolate_prompt.cjs in the interpolation step") + } +} + +// TestRuntimeImportExpansionWithMacroAfterText verifies that a {{#runtime-import}} macro +// embedded after other body text in the workflow is detected by strings.Contains (not +// HasPrefix), and the interpolation step is still emitted. +func TestRuntimeImportExpansionWithMacroAfterText(t *testing.T) { + tmpDir := testutil.TempDir(t, "runtime-import-expansion-after-text") + workflowDir := filepath.Join(tmpDir, ".github", "workflows") + sharedDir := filepath.Join(tmpDir, ".github", "shared") + if err := os.MkdirAll(workflowDir, 0755); err != nil { + t.Fatalf("failed to create workflow directory: %v", err) + } + if err := os.MkdirAll(sharedDir, 0755); err != nil { + t.Fatalf("failed to create shared directory: %v", err) + } + + if err := os.WriteFile(filepath.Join(sharedDir, "appendix.md"), []byte("# Appendix\n\nExtra context.\n"), 0644); err != nil { + t.Fatalf("failed to write shared file: %v", err) + } + + // Body text comes before the explicit user runtime-import directive. + workflowContent := `--- +on: push +permissions: + contents: read +engine: claude +tools: + github: false +strict: false +--- + +Primary instructions that appear before the import. + +{{#runtime-import .github/shared/appendix.md}} +` + workflowPath := filepath.Join(workflowDir, "after-text.md") + if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { + t.Fatalf("failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("compilation failed: %v", err) + } + + lockPath := stringutil.MarkdownToLockFile(workflowPath) + lockBytes, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("failed to read lock file: %v", err) + } + lockContent := string(lockBytes) + + // The macro must appear in the lock file. + if !strings.Contains(lockContent, "{{#runtime-import") { + t.Error("lock file must contain a {{#runtime-import}} macro") + } + + // The interpolation step must be present regardless of where in the chunk the macro appears. + if !strings.Contains(lockContent, "Interpolate variables and render templates") { + t.Error("lock file must contain 'Interpolate variables and render templates' step so that {{#runtime-import}} macros embedded after other text are resolved") + } + if !strings.Contains(lockContent, "interpolate_prompt.cjs") { + t.Error("lock file must reference interpolate_prompt.cjs in the interpolation step") + } +} diff --git a/pkg/workflow/template.go b/pkg/workflow/template.go index 13aac56fa47..0c2a2daf63e 100644 --- a/pkg/workflow/template.go +++ b/pkg/workflow/template.go @@ -88,13 +88,16 @@ func wrapExpressionsInTemplateConditionals(markdown string) string { // - yaml: The string builder to write the YAML to // - expressionMappings: Array of ExpressionMapping containing the mappings between placeholders and GitHub expressions // - data: WorkflowData containing markdown content and parsed tools +// - hasRuntimeImports: True when any prompt chunk contains a {{#runtime-import}} macro that must +// be resolved by interpolate_prompt.cjs at runtime. Pass true whenever the compiled prompt +// includes such macros (always the case in non-inline compilation mode). // // The generated step: // - Uses actions/github-script action // - Sets GH_AW_PROMPT environment variable to the prompt file path // - Sets GH_AW_EXPR_* environment variables with the actual GitHub expressions (${{ ... }}) // - Runs interpolate_prompt.cjs script to replace placeholders and render template conditionals -func (c *Compiler) generateInterpolationAndTemplateStep(yaml *strings.Builder, expressionMappings []*ExpressionMapping, data *WorkflowData) { +func (c *Compiler) generateInterpolationAndTemplateStep(yaml *strings.Builder, expressionMappings []*ExpressionMapping, data *WorkflowData, hasRuntimeImports bool) { // Check if we need interpolation hasExpressions := len(expressionMappings) > 0 @@ -102,7 +105,10 @@ func (c *Compiler) generateInterpolationAndTemplateStep(yaml *strings.Builder, e hasTemplatePattern := strings.Contains(data.MarkdownContent, "{{#if ") hasGitHubContext := hasGitHubTool(data.ParsedTools) hasInlineSubAgents := inlineSubAgentPattern.MatchString(data.MarkdownContent) - hasTemplates := hasTemplatePattern || hasGitHubContext || hasInlineSubAgents + // hasRuntimeImports is true when the compiled prompt contains {{#runtime-import}} macros that + // must be resolved by interpolate_prompt.cjs. The compiler always emits a self-import macro in + // non-inline mode, so this is true even when the markdown source contains no explicit macros. + hasTemplates := hasTemplatePattern || hasGitHubContext || hasInlineSubAgents || hasRuntimeImports // Skip if neither interpolation nor template rendering is needed if !hasExpressions && !hasTemplates { @@ -110,8 +116,8 @@ func (c *Compiler) generateInterpolationAndTemplateStep(yaml *strings.Builder, e return } - templateLog.Printf("Generating interpolation and template step: expressions=%d, hasPattern=%v, hasGitHubContext=%v, hasInlineSubAgents=%v", - len(expressionMappings), hasTemplatePattern, hasGitHubContext, hasInlineSubAgents) + templateLog.Printf("Generating interpolation and template step: expressions=%d, hasPattern=%v, hasGitHubContext=%v, hasInlineSubAgents=%v, hasRuntimeImports=%v", + len(expressionMappings), hasTemplatePattern, hasGitHubContext, hasInlineSubAgents, hasRuntimeImports) yaml.WriteString(" - name: Interpolate variables and render templates\n") fmt.Fprintf(yaml, " uses: %s\n", getCachedActionPin("actions/github-script", data)) diff --git a/pkg/workflow/template_test.go b/pkg/workflow/template_test.go index cf068c3a343..430ff9cd190 100644 --- a/pkg/workflow/template_test.go +++ b/pkg/workflow/template_test.go @@ -27,7 +27,7 @@ func TestGenerateInterpolationAndTemplateStep_DeduplicatesEnvVars(t *testing.T) } var yaml strings.Builder - compiler.generateInterpolationAndTemplateStep(&yaml, expressionMappings, data) + compiler.generateInterpolationAndTemplateStep(&yaml, expressionMappings, data, false) result := yaml.String() @@ -40,7 +40,8 @@ func TestGenerateInterpolationAndTemplateStep_DeduplicatesEnvVars(t *testing.T) } // TestGenerateInterpolationAndTemplateStep_SkipPath tests that the step is not generated -// when there are no expression mappings, no template patterns, and no GitHub context tool. +// when there are no expression mappings, no template patterns, no GitHub context tool, and +// no runtime-import macros (i.e. inline/Wasm compilation mode where no self-import is emitted). func TestGenerateInterpolationAndTemplateStep_SkipPath(t *testing.T) { compiler := &Compiler{} data := &WorkflowData{ @@ -49,7 +50,7 @@ func TestGenerateInterpolationAndTemplateStep_SkipPath(t *testing.T) { } var yaml strings.Builder - compiler.generateInterpolationAndTemplateStep(&yaml, nil, data) + compiler.generateInterpolationAndTemplateStep(&yaml, nil, data, false) assert.Empty(t, yaml.String(), "step YAML should be empty when MarkdownContent has no expressions or template patterns") } @@ -64,7 +65,7 @@ func TestGenerateInterpolationAndTemplateStep_GeneratePath(t *testing.T) { } var yaml strings.Builder - compiler.generateInterpolationAndTemplateStep(&yaml, nil, data) + compiler.generateInterpolationAndTemplateStep(&yaml, nil, data, false) result := yaml.String() assert.Contains(t, result, "Interpolate variables and render templates", "step name should be present when template patterns exist") @@ -85,9 +86,33 @@ func TestGenerateInterpolationAndTemplateStep_WithInlineSubAgent(t *testing.T) { } var yaml strings.Builder - compiler.generateInterpolationAndTemplateStep(&yaml, nil, data) + compiler.generateInterpolationAndTemplateStep(&yaml, nil, data, false) result := yaml.String() assert.Contains(t, result, "Interpolate variables and render templates", "step should be present when inline sub-agents are defined") assert.Contains(t, result, "interpolate_prompt.cjs", "interpolate_prompt script should run to extract inline sub-agents") } + +// TestGenerateInterpolationAndTemplateStep_WithRuntimeImport ensures the interpolation step +// is emitted whenever the compiled prompt contains a {{#runtime-import}} macro, even when +// the markdown body has no expressions, no {{#if}} patterns, and github is disabled. +// This is the scenario from the bug report: github: false + no expressions = skipped step, +// but the compiler always emits a self-import macro that requires interpolate_prompt.cjs. +func TestGenerateInterpolationAndTemplateStep_WithRuntimeImport(t *testing.T) { + compiler := &Compiler{} + data := &WorkflowData{ + // Plain body with no expressions, no {{#if}}, no github tool — previously caused the step + // to be skipped even though the compiled prompt always contains a runtime-import macro. + MarkdownContent: "Do some work.", + ParsedTools: NewTools(map[string]any{}), + } + + var yaml strings.Builder + compiler.generateInterpolationAndTemplateStep(&yaml, nil, data, true) + + result := yaml.String() + assert.Contains(t, result, "Interpolate variables and render templates", + "step must be emitted when runtime-import macros are present so they are resolved at runtime") + assert.Contains(t, result, "interpolate_prompt.cjs", + "interpolate_prompt script must be referenced to resolve {{#runtime-import}} macros") +}