Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion pkg/workflow/compiler_yaml_prompt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
64 changes: 64 additions & 0 deletions pkg/workflow/compiler_yaml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
199 changes: 199 additions & 0 deletions pkg/workflow/runtime_import_expansion_integration_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
}
14 changes: 10 additions & 4 deletions pkg/workflow/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,30 +88,36 @@ 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

// Check if we need template rendering
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 {
templateLog.Print("No interpolation or template rendering needed, skipping step generation")
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))
Expand Down
35 changes: 30 additions & 5 deletions pkg/workflow/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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{
Expand All @@ -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")
}
Expand All @@ -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")
Expand All @@ -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")
}
Loading