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
45 changes: 0 additions & 45 deletions pkg/cli/compile_force_refresh_action_pins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,55 +9,10 @@ import (
"testing"

"github.com/github/gh-aw/pkg/testutil"
"github.com/github/gh-aw/pkg/workflow"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestForceRefreshActionPins_ClearCache(t *testing.T) {
// Create temporary directory for testing
tmpDir := testutil.TempDir(t, "test-*")

// Change to temp directory to simulate running from repo root
oldCwd, err := os.Getwd()
require.NoError(t, err, "Failed to get current working directory")
defer func() {
_ = os.Chdir(oldCwd)
}()

err = os.Chdir(tmpDir)
require.NoError(t, err, "Failed to change to temp directory")

// Create a cache with some entries
cache := workflow.NewActionCache(tmpDir)
cache.Set("actions/checkout", "v5", "abc123")
cache.Set("actions/setup-node", "v4", "def456")
err = cache.Save()
require.NoError(t, err, "Failed to save initial cache")

// Verify cache file exists and has entries
cachePath := filepath.Join(tmpDir, ".github", "aw", workflow.CacheFileName)
require.FileExists(t, cachePath, "Cache file should exist before test")

// Load the cache to verify it has entries
testCache := workflow.NewActionCache(tmpDir)
err = testCache.Load()
require.NoError(t, err, "Failed to load cache")
assert.Len(t, testCache.Entries, 2, "Cache should have 2 entries before force refresh")

// Create compiler with force refresh enabled
compiler := workflow.NewCompiler(
workflow.WithVersion("test"),
)
compiler.SetForceRefreshActionPins(true)

// Get the shared action resolver - this should skip loading the cache
actionCache, _ := compiler.GetSharedActionResolverForTest()

// Verify cache is empty (not loaded from disk)
assert.Empty(t, actionCache.Entries, "Cache should be empty when force refresh is enabled")
}

func TestForceRefreshActionPins_ResetFile(t *testing.T) {
// Create temporary directory for testing
tmpDir := testutil.TempDir(t, "test-*")
Expand Down
6 changes: 0 additions & 6 deletions pkg/workflow/compiler_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,12 +292,6 @@ func (c *Compiler) getSharedActionResolver() (*ActionCache, *ActionResolver) {
return c.actionCache, c.actionResolver
}

// GetSharedActionResolverForTest exposes the shared action resolver for testing purposes
// This should only be used in tests
func (c *Compiler) GetSharedActionResolverForTest() (*ActionCache, *ActionResolver) {
return c.getSharedActionResolver()
}

// getSharedImportCache returns the shared import cache, initializing it on first use
// This ensures all workflows compiled by this compiler instance share the same import cache
func (c *Compiler) getSharedImportCache() *parser.ImportCache {
Expand Down
144 changes: 0 additions & 144 deletions pkg/workflow/prompts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,156 +15,12 @@ import (
// Safe Outputs Prompt Tests
// ============================================================================

func TestGenerateSafeOutputsPromptStep_IncludesWhenEnabled(t *testing.T) {
// Test that safe outputs are included in unified prompt step when enabled
compiler := &Compiler{}
var yaml strings.Builder

// Create a config with create-issue enabled
safeOutputs := &SafeOutputsConfig{
CreateIssues: &CreateIssuesConfig{},
}

data := &WorkflowData{
ParsedTools: NewTools(map[string]any{}),
SafeOutputs: safeOutputs,
}

compiler.generateUnifiedPromptStep(&yaml, data)

output := yaml.String()
if !strings.Contains(output, "Create prompt with built-in context") {
t.Error("Expected unified prompt step to be generated when safe outputs enabled")
}
// Static intro is now in safe_outputs_prompt.md (referenced by file, not inline)
if !strings.Contains(output, "safe_outputs_prompt.md") {
t.Error("Expected reference to safe_outputs_prompt.md for static safe outputs intro")
}
// Per-tool instructions are still inline
if !strings.Contains(output, "create_issue") {
t.Error("Expected prompt to include create_issue tool name")
}
}

func TestGenerateSafeOutputsPromptStep_SkippedWhenDisabled(t *testing.T) {
// Test that safe outputs are not included in unified prompt step when disabled
compiler := &Compiler{}
var yaml strings.Builder

data := &WorkflowData{
ParsedTools: NewTools(map[string]any{}),
SafeOutputs: nil,
}

compiler.generateUnifiedPromptStep(&yaml, data)

output := yaml.String()
// Should still have unified step (for temp folder), but not safe outputs
if strings.Contains(output, "safe_outputs_prompt.md") {
t.Error("Expected safe outputs section to NOT be in unified prompt when disabled")
}
}

func TestSafeOutputsPromptText_FollowsXMLFormat(t *testing.T) {
// This test is for the embedded prompt text which is no longer used
// Skip it as we now generate the prompt dynamically
t.Skip("Safe outputs prompt is now generated dynamically based on enabled tools")
}

func TestSafeOutputsPrompt_IncludesPerToolInstructions(t *testing.T) {
// Test that per-tool instructions are included in the safe outputs prompt
// for each enabled tool, helping the agent understand how to use them.
compiler := &Compiler{}
var yaml strings.Builder

// Create a config with multiple safe outputs enabled
safeOutputs := &SafeOutputsConfig{
CreateIssues: &CreateIssuesConfig{},
AddComments: &AddCommentsConfig{},
CreateDiscussions: &CreateDiscussionsConfig{},
UpdateIssues: &UpdateIssuesConfig{},
}

data := &WorkflowData{
ParsedTools: NewTools(map[string]any{}),
SafeOutputs: safeOutputs,
}

compiler.generateUnifiedPromptStep(&yaml, data)
output := yaml.String()

// Static intro is now in safe_outputs_prompt.md (file reference, not inline)
if !strings.Contains(output, "safe_outputs_prompt.md") {
t.Fatal("Expected safe_outputs_prompt.md file reference in generated prompt")
}

// Per-tool instructions are wrapped in <safe-output-tools>
if !strings.Contains(output, "<safe-output-tools>") {
t.Fatal("Expected <safe-output-tools> section in generated prompt")
}

// Verify enabled tool names are present
for _, toolName := range []string{"create_issue", "add_comment", "create_discussion", "update_issue"} {
t.Run(toolName, func(t *testing.T) {
if !strings.Contains(output, toolName) {
t.Errorf("Expected per-tool instruction to include tool name %q", toolName)
}
})
}
}

func TestSafeOutputsPrompt_AlwaysIncludesNoop(t *testing.T) {
// noop should always appear in the <safe-output-tools> Tools list for any
// workflow that has a safe-outputs section, regardless of whether noop was
// explicitly listed in the frontmatter (it is auto-injected).
tests := []struct {
name string
safeOutputs *SafeOutputsConfig
}{
{
name: "noop only",
safeOutputs: &SafeOutputsConfig{
NoOp: &NoOpConfig{},
},
},
{
name: "noop with other tools",
safeOutputs: &SafeOutputsConfig{
CreateIssues: &CreateIssuesConfig{},
NoOp: &NoOpConfig{},
},
},
{
name: "auto-injected noop (missing_tool and missing_data auto-enabled)",
safeOutputs: &SafeOutputsConfig{
CreateIssues: &CreateIssuesConfig{},
MissingTool: &MissingToolConfig{},
MissingData: &MissingDataConfig{},
NoOp: &NoOpConfig{},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
compiler := &Compiler{}
var yaml strings.Builder

data := &WorkflowData{
ParsedTools: NewTools(map[string]any{}),
SafeOutputs: tt.safeOutputs,
}

compiler.generateUnifiedPromptStep(&yaml, data)
output := yaml.String()

if !strings.Contains(output, "noop") {
t.Errorf("Expected 'noop' to be present in <safe-output-tools> Tools list, got:\n%s", output)
}
})
}
}

// ============================================================================
// Cache Memory Prompt Tests
// ============================================================================
Expand Down
62 changes: 0 additions & 62 deletions pkg/workflow/safe_outputs_default_create_issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ package workflow

import (
"fmt"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -255,67 +254,6 @@ func TestAutoInjectCreateIssue(t *testing.T) {
}
}

// TestAutoInjectedCreateIssuePrompt verifies that the auto-injected create-issue produces
// a specific prompt instruction to create an issue with results or call noop.
func TestAutoInjectedCreateIssuePrompt(t *testing.T) {
tests := []struct {
name string
safeOutputs *SafeOutputsConfig
expectSpecific bool // expect the auto_create_issue file reference
}{
{
name: "auto-injected create-issue produces specific prompt",
safeOutputs: &SafeOutputsConfig{
CreateIssues: &CreateIssuesConfig{
BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")},
Labels: []string{"my-workflow"},
TitlePrefix: "[my-workflow]",
},
AutoInjectedCreateIssue: true,
},
expectSpecific: true,
},
{
name: "user-configured create-issue does NOT produce specific prompt",
safeOutputs: &SafeOutputsConfig{
CreateIssues: &CreateIssuesConfig{
TitlePrefix: "[custom]",
},
AutoInjectedCreateIssue: false,
},
expectSpecific: false,
},
{
name: "no create-issue configured",
safeOutputs: &SafeOutputsConfig{
AddComments: &AddCommentsConfig{},
},
expectSpecific: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
compiler := &Compiler{}
var yaml strings.Builder
data := &WorkflowData{
ParsedTools: NewTools(map[string]any{}),
SafeOutputs: tt.safeOutputs,
}
compiler.generateUnifiedPromptStep(&yaml, data)
output := yaml.String()

if tt.expectSpecific {
assert.Contains(t, output, safeOutputsAutoCreateIssueFile,
"Auto-injected create-issue should include the auto_create_issue file reference")
} else {
assert.NotContains(t, output, safeOutputsAutoCreateIssueFile,
"Non-auto-injected create-issue should not include the auto_create_issue file reference")
}
})
}
}

// TestAutoInjectCreateIssueWithVariousWorkflowIDs verifies correct label/prefix generation
func TestAutoInjectCreateIssueWithVariousWorkflowIDs(t *testing.T) {
workflowIDs := []string{
Expand Down
Loading
Loading