Skip to content

Align generated workflow step names for engine config and cache-memory steps #26960

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 2 commits into from
Apr 18, 2026
Merged

Align generated workflow step names for engine config and cache-memory steps #26960

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
495da90
Initial plan
Copilot Apr 17, 2026
76ea54e
fix step name inconsistencies for engine config and cache-memory
Copilot Apr 18, 2026
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
2 changes: 1 addition & 1 deletion .github/workflows/ai-moderator.lock.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions .github/workflows/smoke-crush.lock.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions .github/workflows/smoke-gemini.lock.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/workflow/awf_helpers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1199,7 +1199,7 @@ func TestGeminiEngineIncludesGeminiAPITarget(t *testing.T) {
t.Fatal("Expected at least two execution steps (settings + execution)")
}

// steps[0] = Write Gemini Settings, steps[1] = Execute Gemini CLI
// steps[0] = Write Gemini Config, steps[1] = Execute Gemini CLI
stepContent := strings.Join(steps[1], "\n")

assert.Contains(t, stepContent, "--gemini-api-target", "Should include --gemini-api-target flag")
Expand Down
7 changes: 1 addition & 6 deletions pkg/workflow/cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,12 +494,7 @@ func generateCacheMemorySteps(builder *strings.Builder, data *WorkflowData) {
threatDetectionEnabled := IsDetectionJobEnabled(data.SafeOutputs)
useRestoreOnly := cache.RestoreOnly || threatDetectionEnabled

var actionName string
if useRestoreOnly {
actionName = "Restore cache-memory file share data"
} else {
actionName = "Cache cache-memory file share data"
}
actionName := "Restore cache-memory file share data"

if useBackwardCompatiblePaths {
fmt.Fprintf(builder, " - name: %s\n", actionName)
Expand Down
6 changes: 3 additions & 3 deletions pkg/workflow/cache_memory_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ tools:
expectedInLock: []string{
"# Cache memory file share configuration from frontmatter processed below",
"- name: Create cache-memory directory",
"- name: Cache cache-memory file share data",
"- name: Restore cache-memory file share data",
"uses: actions/cache@",
"key: memory-none-nopolicy-${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}-${{ github.run_id }}",
"path: /tmp/gh-aw/cache-memory",
Expand Down Expand Up @@ -77,12 +77,12 @@ tools:
"# Cache memory file share configuration from frontmatter processed below",
"- name: Create cache-memory directory (default)",
"mkdir -p /tmp/gh-aw/cache-memory",
"- name: Cache cache-memory file share data (default)",
"- name: Restore cache-memory file share data (default)",
"key: memory-none-nopolicy-memory-default-${{ github.run_id }}",
"path: /tmp/gh-aw/cache-memory",
"- name: Create cache-memory directory (session)",
"mkdir -p /tmp/gh-aw/cache-memory-session",
"- name: Cache cache-memory file share data (session)",
"- name: Restore cache-memory file share data (session)",
"key: memory-none-nopolicy-memory-session-${{ github.run_id }}",
"path: /tmp/gh-aw/cache-memory-session",
"cache_memory_prompt_multi.md", // Template file reference for multiple caches
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/cache_memory_restore_only_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ tools:
---`,
expectedInLock: []string{
"# Cache memory file share configuration from frontmatter processed below",
"- name: Cache cache-memory file share data (default)",
"- name: Restore cache-memory file share data (default)",
"uses: actions/cache@", // SHA varies
"key: memory-none-nopolicy-memory-default-${{ github.run_id }}",
"- name: Restore cache-memory file share data (readonly)",
Expand Down Expand Up @@ -103,7 +103,7 @@ tools:
restore-only: true
---`,
expectedInLock: []string{
"- name: Cache cache-memory file share data (writeable)",
"- name: Restore cache-memory file share data (writeable)",
"uses: actions/cache@", // SHA varies
"- name: Restore cache-memory file share data (readonly1)",
"uses: actions/cache/restore@", // SHA varies
Expand Down
8 changes: 4 additions & 4 deletions pkg/workflow/cache_memory_threat_detection_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ Test workflow with cache-memory and threat detection enabled.`,
},
notExpectedInLock: []string{
// Should NOT use regular actions/cache in agent job
"- name: Cache cache-memory file share data\n uses: actions/cache@",
"- name: Restore cache-memory file share data\n uses: actions/cache@",
Copy link

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

notExpectedInLock checks for the substring "- name: Restore cache-memory file share data\n uses: actions/cache@", but the generated YAML indents uses: with 8 spaces under a step ( uses:). This means the assertion likely never matches, so the test may pass even if actions/cache@ is incorrectly used when threat detection is enabled. Adjust the substring to match the actual indentation, or assert on "uses: actions/cache@" separately (without relying on a newline/indentation match).

Suggested change
"- name: Restore cache-memory file share data\n uses: actions/cache@",
"uses: actions/cache@",

Copilot uses AI. Check for mistakes.
},
},
{
Expand All @@ -83,8 +83,8 @@ tools:

Test workflow with cache-memory but no threat detection.`,
expectedInLock: []string{
// Without threat detection, should use regular actions/cache
"- name: Cache cache-memory file share data",
// Without threat detection, should still restore from cache before execution
"- name: Restore cache-memory file share data",
"uses: actions/cache@",
"key: memory-none-nopolicy-${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}-${{ github.run_id }}",
},
Expand Down Expand Up @@ -143,7 +143,7 @@ Test workflow with multiple cache-memory and threat detection enabled.`,
},
notExpectedInLock: []string{
// Should NOT use regular actions/cache
"- name: Cache cache-memory file share data (default)",
"- name: Restore cache-memory file share data (default)\n uses: actions/cache@",
},
},
{
Expand Down
2 changes: 1 addition & 1 deletion pkg/workflow/crush_engine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ else
fi
chmod 600 "$CONFIG"`, configJSON)

stepLines := []string{" - name: Write Crush configuration"}
stepLines := []string{" - name: Write Crush Config"}
stepLines = FormatStepWithCommandAndEnv(stepLines, command, nil)
return GitHubActionStep(stepLines)
}
2 changes: 1 addition & 1 deletion pkg/workflow/crush_engine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ func TestCrushEngineExecution(t *testing.T) {
configContent := strings.Join(steps[0], "\n")
execContent := strings.Join(steps[1], "\n")

assert.Contains(t, configContent, "Write Crush configuration", "First step should be Write Crush configuration")
assert.Contains(t, configContent, "Write Crush Config", "First step should be Write Crush Config")
assert.Contains(t, configContent, ".crush.json", "Config step should reference .crush.json")
assert.Contains(t, configContent, "permissions", "Config step should set permissions")
assert.Contains(t, execContent, "Execute Crush CLI", "Second step should be Execute Crush CLI")
Expand Down
2 changes: 1 addition & 1 deletion pkg/workflow/enable_api_proxy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func TestEngineAWFEnableApiProxy(t *testing.T) {
t.Fatal("Expected at least two execution steps (settings + execution)")
}

// steps[0] = Write Gemini Settings, steps[1] = Execute Gemini CLI
// steps[0] = Write Gemini Config, steps[1] = Execute Gemini CLI
stepContent := strings.Join(steps[1], "\n")

if !strings.Contains(stepContent, "--enable-api-proxy") {
Expand Down
6 changes: 3 additions & 3 deletions pkg/workflow/gemini_engine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func TestGeminiEngineExecution(t *testing.T) {
steps := engine.GetExecutionSteps(workflowData, "/tmp/test.log")
require.Len(t, steps, 2, "Should generate settings step and execution step")

// steps[0] = Write Gemini Settings, steps[1] = Execute Gemini CLI
// steps[0] = Write Gemini Config, steps[1] = Execute Gemini CLI
stepContent := strings.Join(steps[1], "\n")

assert.Contains(t, stepContent, "name: Execute Gemini CLI", "Should have correct step name")
Expand Down Expand Up @@ -310,7 +310,7 @@ func TestGeminiEngineExecution(t *testing.T) {
settingsContent := strings.Join(steps[0], "\n")
execContent := strings.Join(steps[1], "\n")

assert.Contains(t, settingsContent, "Write Gemini Settings", "First step should be Write Gemini Settings")
assert.Contains(t, settingsContent, "Write Gemini Config", "First step should be Write Gemini Config")
assert.Contains(t, settingsContent, "includeDirectories", "Settings step should set includeDirectories")
assert.Contains(t, settingsContent, "/tmp/", "Settings step should include /tmp/ in include directories")
assert.Contains(t, execContent, "Execute Gemini CLI", "Second step should be Execute Gemini CLI")
Expand Down Expand Up @@ -464,7 +464,7 @@ func TestGenerateGeminiSettingsStep(t *testing.T) {
step := engine.generateGeminiSettingsStep(workflowData)
content := strings.Join(step, "\n")

assert.Contains(t, content, "Write Gemini Settings", "Should have correct step name")
assert.Contains(t, content, "Write Gemini Config", "Should have correct step name")
assert.Contains(t, content, "/tmp/", "Should include /tmp/ in include directories")
assert.Contains(t, content, "includeDirectories", "Should set includeDirectories")
assert.Contains(t, content, ".gemini", "Should reference .gemini directory")
Expand Down
2 changes: 1 addition & 1 deletion pkg/workflow/gemini_tools.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ else
fi`

stepLines := []string{
" - name: Write Gemini Settings",
" - name: Write Gemini Config",
}
env := map[string]string{
"GH_AW_GEMINI_BASE_CONFIG": string(configJSON),
Expand Down
Loading