From 082dd7bb80682263b6fcdec505ec122b43225609 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 13:30:17 +0000 Subject: [PATCH 01/11] Initial plan From 1261f3d974f40ff694178dc9a54bddc2b71ce38d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 13:54:52 +0000 Subject: [PATCH 02/11] Add restore-memory support to custom jobs via step injection - New compiler_custom_job_memory.go: extractRestoreMemoryConfig, validateRestoreMemoryConfig, buildRestoreMemorySteps, and helpers for each memory type (cache-memory, repo-memory, comment-memory) - configureCustomJobSteps updated to parse and inject restore-memory steps - Schema: restore-memory property added to custom job additionalProperties - Tests: compiler_custom_job_memory_test.go covers all acceptance criteria No write-back/commit steps are ever emitted for memory in custom jobs. Validation rejects custom jobs requesting a memory type not in tools:. Closes #43946 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/schemas/main_workflow_schema.json | 19 + pkg/workflow/compiler_custom_job_memory.go | 222 +++++++ .../compiler_custom_job_memory_test.go | 617 ++++++++++++++++++ pkg/workflow/compiler_custom_jobs.go | 25 +- 4 files changed, 882 insertions(+), 1 deletion(-) create mode 100644 pkg/workflow/compiler_custom_job_memory.go create mode 100644 pkg/workflow/compiler_custom_job_memory_test.go diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 0bff225c8a4..7b0c84ef0c6 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -2690,6 +2690,25 @@ } } ] + }, + "restore-memory": { + "type": "object", + "description": "Inject read-only memory restore steps into this custom job. Each enabled memory type must already be configured in the workflow's tools: section. No write-back or commit steps are ever emitted.", + "additionalProperties": false, + "properties": { + "cache-memory": { + "type": "boolean", + "description": "Restore cache-memory file share data (read-only). Requires cache-memory to be configured in tools:." + }, + "repo-memory": { + "type": "boolean", + "description": "Clone repo-memory branch for reading (read-only). Requires repo-memory to be configured in tools:." + }, + "comment-memory": { + "type": "boolean", + "description": "Prepare comment-memory files for reading (read-only). Requires comment-memory to be configured in tools:." + } + } } } } diff --git a/pkg/workflow/compiler_custom_job_memory.go b/pkg/workflow/compiler_custom_job_memory.go new file mode 100644 index 00000000000..52994d84dd5 --- /dev/null +++ b/pkg/workflow/compiler_custom_job_memory.go @@ -0,0 +1,222 @@ +package workflow + +import ( + "fmt" + "strings" +) + +// restoreMemoryConfig holds the parsed restore-memory configuration for a custom job. +// When a memory type is true, the compiler injects read-only restore steps for that store. +// No write-back or commit steps are ever emitted for restore-memory. +type restoreMemoryConfig struct { + CacheMemory bool + RepoMemory bool + CommentMemory bool +} + +// extractRestoreMemoryConfig parses the restore-memory field from a custom job config map. +// Returns nil if the field is absent or all values are false. +func extractRestoreMemoryConfig(configMap map[string]any, jobName string) (*restoreMemoryConfig, error) { + rawVal, hasField := configMap["restore-memory"] + if !hasField { + return nil, nil + } + + rmMap, ok := rawVal.(map[string]any) + if !ok { + return nil, fmt.Errorf("jobs.%s.restore-memory must be an object with memory-type keys (cache-memory, repo-memory, comment-memory)", jobName) + } + + cfg := &restoreMemoryConfig{} + + if v, ok := rmMap["cache-memory"]; ok { + if b, ok := v.(bool); ok { + cfg.CacheMemory = b + } + } + if v, ok := rmMap["repo-memory"]; ok { + if b, ok := v.(bool); ok { + cfg.RepoMemory = b + } + } + if v, ok := rmMap["comment-memory"]; ok { + if b, ok := v.(bool); ok { + cfg.CommentMemory = b + } + } + + if !cfg.CacheMemory && !cfg.RepoMemory && !cfg.CommentMemory { + return nil, nil + } + return cfg, nil +} + +// validateRestoreMemoryConfig ensures that every requested memory type is actually +// configured in the workflow's tools: section. Custom jobs cannot declare memory +// stores independently; they can only restore from stores the workflow already uses. +func validateRestoreMemoryConfig(cfg *restoreMemoryConfig, data *WorkflowData, jobName string) error { + if cfg == nil { + return nil + } + if cfg.CacheMemory && (data.CacheMemoryConfig == nil || len(data.CacheMemoryConfig.Caches) == 0) { + return fmt.Errorf("jobs.%s.restore-memory.cache-memory: cache-memory must be configured in tools: to use restore-memory", jobName) + } + if cfg.RepoMemory && (data.RepoMemoryConfig == nil || len(data.RepoMemoryConfig.Memories) == 0) { + return fmt.Errorf("jobs.%s.restore-memory.repo-memory: repo-memory must be configured in tools: to use restore-memory", jobName) + } + if cfg.CommentMemory && (data.SafeOutputs == nil || data.SafeOutputs.CommentMemory == nil) { + return fmt.Errorf("jobs.%s.restore-memory.comment-memory: comment-memory must be configured in tools: to use restore-memory", jobName) + } + return nil +} + +// buildRestoreMemorySteps returns two slices: +// - setupLines: gh-aw checkout + setup steps (only when repo-memory or comment-memory +// is requested, since those need scripts from the setup action) +// - memoryLines: the actual memory restore/clone/prepare steps +// +// No write-back steps are ever emitted; all injected steps are read-only. +func (c *Compiler) buildRestoreMemorySteps(cfg *restoreMemoryConfig, data *WorkflowData) (setupLines []string, memoryLines []string) { + if cfg == nil { + return nil, nil + } + + // repo-memory and comment-memory rely on scripts installed by the gh-aw setup action. + // Inject the setup step before any memory steps so those scripts are available. + if cfg.RepoMemory || cfg.CommentMemory { + setupActionRef := c.resolveActionReference("./actions/setup", data) + if setupActionRef != "" || c.actionMode.IsScript() { + setupLines = append(setupLines, c.generateCheckoutActionsFolder(data)...) + // Pass empty trace IDs — custom jobs do not inherit the activation span. + setupLines = append(setupLines, c.generateSetupStep(data, setupActionRef, SetupActionDestination, false, "", "")...) + } + } + + if cfg.CacheMemory { + memoryLines = append(memoryLines, generateCacheMemoryRestoreLines(data)...) + } + if cfg.RepoMemory { + memoryLines = append(memoryLines, generateRepoMemoryRestoreLines(data)...) + } + if cfg.CommentMemory { + memoryLines = append(memoryLines, generateCommentMemoryRestoreLines(data)...) + } + + return setupLines, memoryLines +} + +// generateCacheMemoryRestoreLines produces read-only cache-memory restore steps for a +// custom job. Unlike the agent-job path, these steps: +// - always use actions/cache/restore (never actions/cache, so no auto-save) +// - create the cache directory inline with mkdir -p (no script needed) +// - omit the git integrity setup step (only relevant for the write path) +func generateCacheMemoryRestoreLines(data *WorkflowData) []string { + if data.CacheMemoryConfig == nil || len(data.CacheMemoryConfig.Caches) == 0 { + return nil + } + + var lines []string + lines = append(lines, " # restore-memory: cache-memory (read-only restore)\n") + + var githubConfig *GitHubToolConfig + if data.ParsedTools != nil { + githubConfig = data.ParsedTools.GitHub + } + + for i, cache := range data.CacheMemoryConfig.Caches { + cacheDir := cacheMemoryDirFor(cache.ID) + restoreStepID := fmt.Sprintf("restore_cache_memory_%d", i) + + // Create the cache directory inline; no script required. + lines = append(lines, fmt.Sprintf(" - name: Create cache-memory directory (%s)\n", cache.ID)) + lines = append(lines, " run: |\n") + lines = append(lines, fmt.Sprintf(" mkdir -p %s\n", cacheDir)) + + // Compute the same integrity-aware cache key as the agent job uses. + cacheKey := computeIntegrityCacheKey(cache, githubConfig) + + // Restore step — always read-only in custom jobs. + lines = append(lines, fmt.Sprintf(" - name: Restore cache-memory (%s)\n", cache.ID)) + lines = append(lines, fmt.Sprintf(" id: %s\n", restoreStepID)) + lines = append(lines, fmt.Sprintf(" uses: %s\n", getActionPin("actions/cache/restore"))) + lines = append(lines, " with:\n") + lines = append(lines, fmt.Sprintf(" key: %s\n", cacheKey)) + lines = append(lines, fmt.Sprintf(" path: %s\n", cacheDir)) + + // Build restore keys using the same logic as generateCacheMemorySteps. + var restoreKeys []string + runIDSuffix := "-${{ github.run_id }}" + if strings.HasSuffix(cacheKey, runIDSuffix) { + restoreKeys = append(restoreKeys, strings.TrimSuffix(cacheKey, "${{ github.run_id }}")) + } else { + keyParts := strings.Split(cacheKey, "-") + if len(keyParts) >= 2 { + restoreKeys = append(restoreKeys, strings.Join(keyParts[:len(keyParts)-1], "-")+"-") + } + } + + scope := cache.Scope + if scope == "" { + scope = "workflow" + } + if scope == "repo" { + repoKey := strings.TrimSuffix(cacheKey, "${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}-${{ github.run_id }}") + if repoKey != cacheKey && repoKey != "" { + restoreKeys = append(restoreKeys, repoKey) + } + } + + lines = append(lines, " restore-keys: |\n") + for _, key := range restoreKeys { + lines = append(lines, fmt.Sprintf(" %s\n", key)) + } + } + + return lines +} + +// generateRepoMemoryRestoreLines produces read-only repo-memory clone steps for a +// custom job by reusing the existing generateRepoMemorySteps builder and converting +// its output to the []string line format expected by job.Steps. +func generateRepoMemoryRestoreLines(data *WorkflowData) []string { + if data.RepoMemoryConfig == nil || len(data.RepoMemoryConfig.Memories) == 0 { + return nil + } + + var b strings.Builder + generateRepoMemorySteps(&b, data) + raw := b.String() + if raw == "" { + return nil + } + + // Split into individual lines, preserving their trailing newlines. + parts := strings.Split(raw, "\n") + lines := make([]string, 0, len(parts)) + for _, p := range parts { + lines = append(lines, p+"\n") + } + return lines +} + +// generateCommentMemoryRestoreLines produces a read-only comment-memory prepare step +// for a custom job. The step fetches the comment-memory content from GitHub and +// materialises it as local files — the same operation performed in the agent job. +func generateCommentMemoryRestoreLines(data *WorkflowData) []string { + if data.SafeOutputs == nil || data.SafeOutputs.CommentMemory == nil { + return nil + } + + var lines []string + lines = append(lines, " # restore-memory: comment-memory (read-only restore)\n") + lines = append(lines, " - name: Prepare comment memory files\n") + lines = append(lines, fmt.Sprintf(" uses: %s\n", getCachedActionPin("actions/github-script", data))) + lines = append(lines, " with:\n") + lines = append(lines, fmt.Sprintf(" github-token: %s\n", getEffectiveSafeOutputGitHubToken(data.SafeOutputs.CommentMemory.GitHubToken))) + lines = append(lines, " script: |\n") + lines = append(lines, " const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs');\n") + lines = append(lines, " setupGlobals(core, github, context, exec, io, getOctokit);\n") + lines = append(lines, " const { main } = require('${{ runner.temp }}/gh-aw/actions/setup_comment_memory_files.cjs');\n") + lines = append(lines, " await main();\n") + return lines +} diff --git a/pkg/workflow/compiler_custom_job_memory_test.go b/pkg/workflow/compiler_custom_job_memory_test.go new file mode 100644 index 00000000000..ac0ab2eee87 --- /dev/null +++ b/pkg/workflow/compiler_custom_job_memory_test.go @@ -0,0 +1,617 @@ +//go:build !integration + +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/github/gh-aw/pkg/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// ======================================== +// restore-memory: Tests for custom jobs +// ======================================== + +// TestCustomJobRestoreMemoryCacheMemory verifies that a custom job with +// restore-memory.cache-memory: true gets cache restore steps injected. +// No artifact-upload or cache-save steps should be emitted. +func TestCustomJobRestoreMemoryCacheMemory(t *testing.T) { + tmpDir := testutil.TempDir(t, "custom-job-restore-cache-memory") + + frontmatter := `--- +name: Orchestrator +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +tools: + cache-memory: true +jobs: + orchestrator: + runs-on: ubuntu-latest + restore-memory: + cache-memory: true + steps: + - name: Read memory and dispatch + run: echo "dispatching" +--- + +# Orchestrator Workflow + +Reads cache memory and dispatches tasks. +` + + testFile := filepath.Join(tmpDir, "test.md") + require.NoError(t, os.WriteFile(testFile, []byte(frontmatter), 0644)) + + compiler := NewCompiler() + require.NoError(t, compiler.CompileWorkflow(testFile)) + + lockFile := filepath.Join(tmpDir, "test.lock.yml") + content, err := os.ReadFile(lockFile) + require.NoError(t, err) + + yamlStr := string(content) + section := extractJobSection(yamlStr, "orchestrator") + require.NotEmpty(t, section, "Expected orchestrator job section in lock file") + + // Must contain cache restore step + assert.Contains(t, section, "actions/cache/restore@", "cache-memory restore step should use actions/cache/restore") + assert.Contains(t, section, "Create cache-memory directory", "dir creation step should be present") + assert.Contains(t, section, "Restore cache-memory", "cache restore step should be present") + assert.Contains(t, section, "restore_cache_memory_0", "cache restore step ID should be present") + + // Must NOT contain write-back steps + assert.NotContains(t, section, "actions/cache/save@", "no cache-save step should be emitted") + assert.NotContains(t, section, "actions/cache@", "no write-mode cache step should be emitted") + assert.NotContains(t, section, "actions/upload-artifact@", "no artifact-upload step should be emitted") + assert.NotContains(t, section, "Setup cache-memory git", "git integrity setup should not be emitted for read-only restore") + assert.NotContains(t, section, "commit_cache_memory_git", "no git commit script should be emitted") + + // The main agent job should also have cache-memory steps + agentSection := extractJobSection(yamlStr, "agent") + assert.Contains(t, agentSection, "Restore cache-memory file share data", "agent job should still have its own cache restore step") + + // No separate update_cache_memory job (threat detection not enabled) + assert.NotContains(t, yamlStr, "update_cache_memory:", "update_cache_memory job should not be created without threat detection") +} + +// TestCustomJobRestoreMemoryRepoMemory verifies that a custom job with +// restore-memory.repo-memory: true gets repo-memory clone steps injected. +func TestCustomJobRestoreMemoryRepoMemory(t *testing.T) { + tmpDir := testutil.TempDir(t, "custom-job-restore-repo-memory") + + frontmatter := `--- +name: Orchestrator +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +tools: + repo-memory: true +jobs: + orchestrator: + runs-on: ubuntu-latest + restore-memory: + repo-memory: true + steps: + - name: Read repo memory + run: cat /tmp/gh-aw/repo-memory/default/state.json || echo "{}" +--- + +# Orchestrator Workflow +` + + testFile := filepath.Join(tmpDir, "test.md") + require.NoError(t, os.WriteFile(testFile, []byte(frontmatter), 0644)) + + compiler := NewCompiler() + require.NoError(t, compiler.CompileWorkflow(testFile)) + + lockFile := filepath.Join(tmpDir, "test.lock.yml") + content, err := os.ReadFile(lockFile) + require.NoError(t, err) + + yamlStr := string(content) + section := extractJobSection(yamlStr, "orchestrator") + require.NotEmpty(t, section, "Expected orchestrator job section in lock file") + + // Must contain repo-memory clone step + assert.Contains(t, section, "Clone repo-memory branch", "clone step should be present") + assert.Contains(t, section, "clone_repo_memory_branch.sh", "clone script should be referenced") + assert.Contains(t, section, "GH_TOKEN:", "GH_TOKEN env var should be set for clone") + + // Must NOT contain write-back steps (push job is a separate job, not injected here) + assert.NotContains(t, section, "push_repo_memory", "no push step in orchestrator job") +} + +// TestCustomJobRestoreMemoryCommentMemory verifies that a custom job with +// restore-memory.comment-memory: true gets the prepare-comment-memory step injected. +func TestCustomJobRestoreMemoryCommentMemory(t *testing.T) { + tmpDir := testutil.TempDir(t, "custom-job-restore-comment-memory") + + frontmatter := `--- +name: Orchestrator +on: workflow_dispatch +permissions: + contents: read + pull-requests: read +engine: copilot +strict: false +tools: + comment-memory: true +jobs: + orchestrator: + runs-on: ubuntu-latest + restore-memory: + comment-memory: true + steps: + - name: Process comment memory + run: ls /tmp/gh-aw/comment-memory/ || echo "empty" +--- + +# Orchestrator Workflow +` + + testFile := filepath.Join(tmpDir, "test.md") + require.NoError(t, os.WriteFile(testFile, []byte(frontmatter), 0644)) + + compiler := NewCompiler() + require.NoError(t, compiler.CompileWorkflow(testFile)) + + lockFile := filepath.Join(tmpDir, "test.lock.yml") + content, err := os.ReadFile(lockFile) + require.NoError(t, err) + + yamlStr := string(content) + section := extractJobSection(yamlStr, "orchestrator") + require.NotEmpty(t, section, "Expected orchestrator job section in lock file") + + // Must contain prepare comment memory step + assert.Contains(t, section, "Prepare comment memory files", "comment memory prepare step should be present") + assert.Contains(t, section, "setup_comment_memory_files.cjs", "comment memory CJS script should be referenced") + assert.Contains(t, section, "actions/github-script@", "github-script action should be used") +} + +// TestCustomJobRestoreMemoryMultipleTypes verifies that a custom job can request +// multiple memory types at once. +func TestCustomJobRestoreMemoryMultipleTypes(t *testing.T) { + tmpDir := testutil.TempDir(t, "custom-job-restore-multiple-memory") + + frontmatter := `--- +name: Orchestrator +on: workflow_dispatch +permissions: + contents: read + pull-requests: read +engine: copilot +strict: false +tools: + cache-memory: true + repo-memory: true +jobs: + orchestrator: + runs-on: ubuntu-latest + restore-memory: + cache-memory: true + repo-memory: true + steps: + - name: Process memories + run: echo "done" +--- + +# Orchestrator Workflow +` + + testFile := filepath.Join(tmpDir, "test.md") + require.NoError(t, os.WriteFile(testFile, []byte(frontmatter), 0644)) + + compiler := NewCompiler() + require.NoError(t, compiler.CompileWorkflow(testFile)) + + lockFile := filepath.Join(tmpDir, "test.lock.yml") + content, err := os.ReadFile(lockFile) + require.NoError(t, err) + + yamlStr := string(content) + section := extractJobSection(yamlStr, "orchestrator") + require.NotEmpty(t, section) + + assert.Contains(t, section, "Restore cache-memory", "cache-memory steps should be present") + assert.Contains(t, section, "Clone repo-memory branch", "repo-memory steps should be present") +} + +// TestCustomJobRestoreMemoryStepOrder verifies that restore-memory steps are placed +// after GHES host config and before pre-steps and regular steps. +func TestCustomJobRestoreMemoryStepOrder(t *testing.T) { + tmpDir := testutil.TempDir(t, "custom-job-restore-memory-order") + + frontmatter := `--- +name: Orchestrator +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +tools: + cache-memory: true +jobs: + orchestrator: + runs-on: ubuntu-latest + restore-memory: + cache-memory: true + pre-steps: + - name: My pre-step + run: echo "pre" + steps: + - name: My main step + run: echo "main" +--- + +# Orchestrator Workflow +` + + testFile := filepath.Join(tmpDir, "test.md") + require.NoError(t, os.WriteFile(testFile, []byte(frontmatter), 0644)) + + compiler := NewCompiler() + require.NoError(t, compiler.CompileWorkflow(testFile)) + + lockFile := filepath.Join(tmpDir, "test.lock.yml") + content, err := os.ReadFile(lockFile) + require.NoError(t, err) + + yamlStr := string(content) + section := extractJobSection(yamlStr, "orchestrator") + require.NotEmpty(t, section) + + // Expected order: + // 1. Configure GH_HOST for enterprise compatibility + // 2. Create cache-memory directory (restore-memory) + // 3. Restore cache-memory (restore-memory) + // 4. My pre-step (pre-steps) + // 5. My main step (steps) + assertStepOrderInSection(t, section, + "- name: Configure GH_HOST for enterprise compatibility", + "- name: Create cache-memory directory", + "- name: Restore cache-memory", + "- name: My pre-step", + "- name: My main step", + ) +} + +// TestCustomJobRestoreMemoryErrorWhenNotConfigured verifies that a custom job +// requesting restore-memory for a type not configured in tools: produces an error. +func TestCustomJobRestoreMemoryErrorWhenNotConfigured(t *testing.T) { + tests := []struct { + name string + frontmatter string + wantErrMsg string + }{ + { + name: "cache-memory not configured", + frontmatter: `--- +name: Test +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +jobs: + orchestrator: + runs-on: ubuntu-latest + restore-memory: + cache-memory: true + steps: + - name: Step + run: echo hi +--- +# Test +`, + wantErrMsg: "cache-memory must be configured in tools:", + }, + { + name: "repo-memory not configured", + frontmatter: `--- +name: Test +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +jobs: + orchestrator: + runs-on: ubuntu-latest + restore-memory: + repo-memory: true + steps: + - name: Step + run: echo hi +--- +# Test +`, + wantErrMsg: "repo-memory must be configured in tools:", + }, + { + name: "comment-memory not configured", + frontmatter: `--- +name: Test +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +jobs: + orchestrator: + runs-on: ubuntu-latest + restore-memory: + comment-memory: true + steps: + - name: Step + run: echo hi +--- +# Test +`, + wantErrMsg: "comment-memory must be configured in tools:", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tmpDir := testutil.TempDir(t, "restore-memory-error-*") + testFile := filepath.Join(tmpDir, "test.md") + require.NoError(t, os.WriteFile(testFile, []byte(tc.frontmatter), 0644)) + + compiler := NewCompiler() + err := compiler.CompileWorkflow(testFile) + require.Error(t, err, "expected compilation to fail") + assert.Contains(t, err.Error(), tc.wantErrMsg) + }) + } +} + +// TestCustomJobRestoreMemoryOnlyEmitsRestoreSteps verifies that when restore-memory +// is configured, no write-back steps (artifact upload, cache save, git commit, push) +// are emitted for the custom job. +func TestCustomJobRestoreMemoryOnlyEmitsRestoreSteps(t *testing.T) { + tmpDir := testutil.TempDir(t, "custom-job-restore-only") + + frontmatter := `--- +name: Orchestrator +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +tools: + cache-memory: true + repo-memory: true +jobs: + orchestrator: + runs-on: ubuntu-latest + restore-memory: + cache-memory: true + repo-memory: true + steps: + - name: Process data + run: echo "processing" +--- + +# Orchestrator Workflow +` + + testFile := filepath.Join(tmpDir, "test.md") + require.NoError(t, os.WriteFile(testFile, []byte(frontmatter), 0644)) + + compiler := NewCompiler() + require.NoError(t, compiler.CompileWorkflow(testFile)) + + lockFile := filepath.Join(tmpDir, "test.lock.yml") + content, err := os.ReadFile(lockFile) + require.NoError(t, err) + + orchestratorSection := extractJobSection(string(content), "orchestrator") + require.NotEmpty(t, orchestratorSection) + + writePatterns := []string{ + "actions/upload-artifact@", + "actions/cache/save@", + "commit_cache_memory_git.sh", + "push_repo_memory", + "Setup cache-memory git", + } + for _, pattern := range writePatterns { + assert.NotContains(t, orchestratorSection, pattern, + "write-back pattern %q should not be emitted in restore-memory custom job", pattern) + } +} + +// TestCustomJobRestoreMemoryStandaloneJob verifies that restore-memory works even +// when no steps, pre-steps, or setup-steps are provided (steps-only trigger). +func TestCustomJobRestoreMemoryStandaloneJob(t *testing.T) { + tmpDir := testutil.TempDir(t, "custom-job-restore-standalone") + + frontmatter := `--- +name: Orchestrator +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +tools: + cache-memory: true +jobs: + orchestrator: + runs-on: ubuntu-latest + restore-memory: + cache-memory: true +--- + +# Orchestrator Workflow +` + + testFile := filepath.Join(tmpDir, "test.md") + require.NoError(t, os.WriteFile(testFile, []byte(frontmatter), 0644)) + + compiler := NewCompiler() + require.NoError(t, compiler.CompileWorkflow(testFile)) + + lockFile := filepath.Join(tmpDir, "test.lock.yml") + content, err := os.ReadFile(lockFile) + require.NoError(t, err) + + yamlStr := string(content) + section := extractJobSection(yamlStr, "orchestrator") + require.NotEmpty(t, section, "orchestrator job should appear even without explicit steps") + + assert.Contains(t, section, "Restore cache-memory", "restore step must be present") + assert.Contains(t, section, "Configure GH_HOST", "GHES config step must be present") +} + +// TestExtractRestoreMemoryConfig unit-tests the config extraction logic. +func TestExtractRestoreMemoryConfig(t *testing.T) { + tests := []struct { + name string + configMap map[string]any + want *restoreMemoryConfig + wantErr bool + }{ + { + name: "no restore-memory field", + configMap: map[string]any{}, + want: nil, + }, + { + name: "all false values returns nil", + configMap: map[string]any{ + "restore-memory": map[string]any{ + "cache-memory": false, + "repo-memory": false, + "comment-memory": false, + }, + }, + want: nil, + }, + { + name: "cache-memory only", + configMap: map[string]any{ + "restore-memory": map[string]any{ + "cache-memory": true, + }, + }, + want: &restoreMemoryConfig{CacheMemory: true}, + }, + { + name: "all enabled", + configMap: map[string]any{ + "restore-memory": map[string]any{ + "cache-memory": true, + "repo-memory": true, + "comment-memory": true, + }, + }, + want: &restoreMemoryConfig{CacheMemory: true, RepoMemory: true, CommentMemory: true}, + }, + { + name: "invalid type returns error", + configMap: map[string]any{ + "restore-memory": "true", + }, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := extractRestoreMemoryConfig(tc.configMap, "test_job") + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +// TestValidateRestoreMemoryConfig unit-tests the config validation logic. +func TestValidateRestoreMemoryConfig(t *testing.T) { + emptyCacheData := &WorkflowData{} + cacheData := &WorkflowData{ + CacheMemoryConfig: &CacheMemoryConfig{ + Caches: []CacheMemoryEntry{{ID: "default"}}, + }, + } + repoData := &WorkflowData{ + RepoMemoryConfig: &RepoMemoryConfig{ + Memories: []RepoMemoryEntry{{ID: "default"}}, + }, + } + commentData := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + CommentMemory: &CommentMemoryConfig{}, + }, + } + + tests := []struct { + name string + cfg *restoreMemoryConfig + data *WorkflowData + wantErr bool + }{ + {name: "nil config is ok", cfg: nil, data: emptyCacheData, wantErr: false}, + {name: "cache-memory configured correctly", cfg: &restoreMemoryConfig{CacheMemory: true}, data: cacheData, wantErr: false}, + {name: "cache-memory missing from tools", cfg: &restoreMemoryConfig{CacheMemory: true}, data: emptyCacheData, wantErr: true}, + {name: "repo-memory configured correctly", cfg: &restoreMemoryConfig{RepoMemory: true}, data: repoData, wantErr: false}, + {name: "repo-memory missing from tools", cfg: &restoreMemoryConfig{RepoMemory: true}, data: emptyCacheData, wantErr: true}, + {name: "comment-memory configured correctly", cfg: &restoreMemoryConfig{CommentMemory: true}, data: commentData, wantErr: false}, + {name: "comment-memory missing from tools", cfg: &restoreMemoryConfig{CommentMemory: true}, data: emptyCacheData, wantErr: true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := validateRestoreMemoryConfig(tc.cfg, tc.data, "test_job") + if tc.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} + +// TestGenerateCacheMemoryRestoreLines unit-tests the cache restore line generation. +func TestGenerateCacheMemoryRestoreLines(t *testing.T) { + data := &WorkflowData{ + CacheMemoryConfig: &CacheMemoryConfig{ + Caches: []CacheMemoryEntry{ + { + ID: "default", + Key: "memory-${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}-${{ github.run_id }}", + Scope: "workflow", + }, + }, + }, + } + + lines := generateCacheMemoryRestoreLines(data) + combined := strings.Join(lines, "") + + assert.Contains(t, combined, "mkdir -p", "should have inline mkdir") + assert.Contains(t, combined, "actions/cache/restore@", "should use restore-only action") + assert.NotContains(t, combined, "actions/cache@", "should not use read-write action") + assert.NotContains(t, combined, "setup_cache_memory_git", "should not include git setup script") + assert.Contains(t, combined, "restore_cache_memory_0", "should have step ID") +} + +// TestGenerateCacheMemoryRestoreLinesNilData verifies graceful handling of nil config. +func TestGenerateCacheMemoryRestoreLinesNilData(t *testing.T) { + assert.Nil(t, generateCacheMemoryRestoreLines(&WorkflowData{})) +} diff --git a/pkg/workflow/compiler_custom_jobs.go b/pkg/workflow/compiler_custom_jobs.go index 561b030544f..72b8dcf55db 100644 --- a/pkg/workflow/compiler_custom_jobs.go +++ b/pkg/workflow/compiler_custom_jobs.go @@ -507,13 +507,36 @@ func (c *Compiler) configureCustomJobSteps(job *Job, jobName string, configMap m } } - if hasSetupStepsField || hasPreStepsField || hasStepsField { + // Parse and validate restore-memory configuration. + // restore-memory injects read-only memory restore steps into the custom job. + // No write-back or commit steps are ever emitted for memory in custom jobs. + restoreMemCfg, err := extractRestoreMemoryConfig(configMap, jobName) + if err != nil { + return err + } + if err := validateRestoreMemoryConfig(restoreMemCfg, data, jobName); err != nil { + return err + } + + hasRestoreMemory := restoreMemCfg != nil + + if hasSetupStepsField || hasPreStepsField || hasStepsField || hasRestoreMemory { job.Steps = append(job.Steps, setupSteps...) // Prepend GH_HOST configuration step for GHES/GHEC compatibility. // Custom frontmatter jobs run as independent GitHub Actions jobs that // don't inherit GITHUB_ENV from the agent job, so the gh CLI won't // know which host to target without this step. job.Steps = append(job.Steps, generateGHESHostConfigurationStep()) + + // Inject gh-aw setup + memory restore steps when restore-memory is requested. + // Setup lines come first (they install scripts needed by repo/comment memory). + // Memory lines follow immediately after (restore/clone/prepare steps). + if hasRestoreMemory { + memorySetupLines, memoryRestoreLines := c.buildRestoreMemorySteps(restoreMemCfg, data) + job.Steps = append(job.Steps, memorySetupLines...) + job.Steps = append(job.Steps, memoryRestoreLines...) + } + job.Steps = append(job.Steps, preSteps...) job.Steps = append(job.Steps, regularSteps...) } From 1350e5832e0082073f91b8736c151a4bc4fd5cc8 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 7 Jul 2026 14:45:15 +0000 Subject: [PATCH 03/11] docs(adr): add draft ADR-44037 for restore-memory read-only access in custom jobs Co-Authored-By: Claude Sonnet 4.6 --- ...ore-memory-read-only-access-custom-jobs.md | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 docs/adr/44037-restore-memory-read-only-access-custom-jobs.md diff --git a/docs/adr/44037-restore-memory-read-only-access-custom-jobs.md b/docs/adr/44037-restore-memory-read-only-access-custom-jobs.md new file mode 100644 index 00000000000..9b31ac4d15e --- /dev/null +++ b/docs/adr/44037-restore-memory-read-only-access-custom-jobs.md @@ -0,0 +1,50 @@ +# ADR-44037: Read-Only Memory Store Access for Custom Jobs via `restore-memory` + +**Date**: 2026-07-07 +**Status**: Draft +**Deciders**: pelikhan, copilot-swe-agent + +--- + +### Context + +Memory stores (`cache-memory`, `repo-memory`, `comment-memory`) in the gh-aw workflow compiler were exclusively accessible to the built-in agent job. Custom jobs (defined under `jobs:` in a workflow's frontmatter) had no way to read from these stores, making orchestrator patterns architecturally impossible. A common need is a scheduled or dispatch job that reads `cache-memory` to build a task-dispatch list before spawning the agent — but with no access path, authors had to resort to duplicating state or restructuring workflows entirely around the agent job. + +### Decision + +We will add a `restore-memory` field to the custom job config schema that allows job authors to explicitly opt in to read-only restore steps for any subset of the three memory types. The compiler injects the corresponding setup and restore steps (gh-aw setup action, `actions/cache/restore`, `clone_repo_memory_branch.sh`, or `setup_comment_memory_files.cjs`) directly into the custom job's step list, positioned after GHES host config and before `pre-steps`/`steps`. No write-back, git-commit, or artifact-upload steps are ever emitted for custom jobs — enforcement is structural (using `actions/cache/restore` instead of `actions/cache`), not advisory. + +### Alternatives Considered + +#### Alternative 1: Full Read-Write Memory Access in Custom Jobs + +Allow custom jobs to both read and write to memory stores, mirroring the agent job's full lifecycle. This would give orchestrators maximum flexibility, including the ability to pre-populate or update state. + +Rejected because write-back requires coordinated git push logic, artifact uploads, and integrity-check steps that are complex and dangerous when multiple jobs run concurrently. Orchestrator jobs typically only need to read shared state, not modify it. Allowing writes would require the same mutex/conflict-resolution machinery already present in the agent job, adding significant complexity for minimal gain. + +#### Alternative 2: Expose Memory via Workflow Outputs or Job Artifacts + +Have the agent job export its memory state as a named workflow output or GitHub Actions artifact, which other jobs could then consume via the standard `needs:` dependency mechanism. + +Rejected because this approach requires the agent job to run first and explicitly export state — it cannot be used in pre-dispatch orchestrator patterns where the orchestrator runs *before* the agent. It also conflates memory (a persistent side channel) with ephemeral per-run job outputs, muddying the conceptual model. Existing memory restore machinery (`generateRepoMemorySteps`, `generateCacheMemoryRestoreLines`) is already well-tested and reusable, making step injection lower risk than a new artifact-based path. + +### Consequences + +#### Positive +- Enables orchestrator job patterns: a non-agent job can read `cache-memory` state (e.g., a dispatch list or rate-limit counter) before deciding whether and how to invoke the agent. +- Read-only is enforced structurally — `actions/cache/restore` never auto-saves, so concurrent orchestrators cannot corrupt the memory store. +- Reuses existing, tested step generators (`generateRepoMemorySteps`, `generateCommentMemoryRestoreLines`), minimising new code surface. +- Validation at compile time rejects requests for memory types not declared in `tools:`, surfacing misconfiguration early. + +#### Negative +- Increases compiler complexity: a new source file (`compiler_custom_job_memory.go`), new config parsing, validation, and injection plumbing into `configureCustomJobSteps`. +- Job authors must explicitly enumerate every memory type they need under `restore-memory:`; there is no auto-detection based on what the workflow's `tools:` section declares. +- The injected setup action step (required for repo-memory and comment-memory scripts) adds overhead to custom jobs that may not otherwise need the full gh-aw setup. + +#### Neutral +- The JSON schema for the custom job `additionalProperties` definition gains a new `restore-memory` object property; existing workflows without this field are unaffected. +- Step ordering is deterministic: GHES host config → gh-aw setup (if needed) → memory restore steps → pre-steps → regular steps. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* From 75e0efda24d9ae234079d4872571ae3b9ee9abed Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 15:00:14 +0000 Subject: [PATCH 04/11] Simplify restore-memory to boolean true/false for custom jobs Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/schemas/main_workflow_schema.json | 19 +- pkg/workflow/compiler_custom_job_memory.go | 57 ++-- .../compiler_custom_job_memory_test.go | 243 ++++++------------ pkg/workflow/compiler_custom_jobs.go | 7 +- 4 files changed, 93 insertions(+), 233 deletions(-) diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 7b0c84ef0c6..453c5dd453c 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -2692,23 +2692,8 @@ ] }, "restore-memory": { - "type": "object", - "description": "Inject read-only memory restore steps into this custom job. Each enabled memory type must already be configured in the workflow's tools: section. No write-back or commit steps are ever emitted.", - "additionalProperties": false, - "properties": { - "cache-memory": { - "type": "boolean", - "description": "Restore cache-memory file share data (read-only). Requires cache-memory to be configured in tools:." - }, - "repo-memory": { - "type": "boolean", - "description": "Clone repo-memory branch for reading (read-only). Requires repo-memory to be configured in tools:." - }, - "comment-memory": { - "type": "boolean", - "description": "Prepare comment-memory files for reading (read-only). Requires comment-memory to be configured in tools:." - } - } + "type": "boolean", + "description": "When true, injects read-only restore steps for all memory stores configured in the workflow's tools: section into this custom job. No write-back or commit steps are ever emitted." } } } diff --git a/pkg/workflow/compiler_custom_job_memory.go b/pkg/workflow/compiler_custom_job_memory.go index 52994d84dd5..282e155589b 100644 --- a/pkg/workflow/compiler_custom_job_memory.go +++ b/pkg/workflow/compiler_custom_job_memory.go @@ -6,7 +6,7 @@ import ( ) // restoreMemoryConfig holds the parsed restore-memory configuration for a custom job. -// When a memory type is true, the compiler injects read-only restore steps for that store. +// Each field is set to true only when the corresponding memory store is configured in tools:. // No write-back or commit steps are ever emitted for restore-memory. type restoreMemoryConfig struct { CacheMemory bool @@ -15,61 +15,36 @@ type restoreMemoryConfig struct { } // extractRestoreMemoryConfig parses the restore-memory field from a custom job config map. -// Returns nil if the field is absent or all values are false. -func extractRestoreMemoryConfig(configMap map[string]any, jobName string) (*restoreMemoryConfig, error) { +// The field accepts a boolean: true enables all memory stores that are configured in tools:, +// false (or absent) disables restore-memory entirely. +// Returns nil if the field is absent or false, an error if the value is not a boolean, +// and an error if true is set but no memory stores are configured in tools:. +func extractRestoreMemoryConfig(configMap map[string]any, jobName string, data *WorkflowData) (*restoreMemoryConfig, error) { rawVal, hasField := configMap["restore-memory"] if !hasField { return nil, nil } - rmMap, ok := rawVal.(map[string]any) + enabled, ok := rawVal.(bool) if !ok { - return nil, fmt.Errorf("jobs.%s.restore-memory must be an object with memory-type keys (cache-memory, repo-memory, comment-memory)", jobName) + return nil, fmt.Errorf("jobs.%s.restore-memory must be a boolean (true or false)", jobName) } - - cfg := &restoreMemoryConfig{} - - if v, ok := rmMap["cache-memory"]; ok { - if b, ok := v.(bool); ok { - cfg.CacheMemory = b - } - } - if v, ok := rmMap["repo-memory"]; ok { - if b, ok := v.(bool); ok { - cfg.RepoMemory = b - } + if !enabled { + return nil, nil } - if v, ok := rmMap["comment-memory"]; ok { - if b, ok := v.(bool); ok { - cfg.CommentMemory = b - } + + cfg := &restoreMemoryConfig{ + CacheMemory: data.CacheMemoryConfig != nil && len(data.CacheMemoryConfig.Caches) > 0, + RepoMemory: data.RepoMemoryConfig != nil && len(data.RepoMemoryConfig.Memories) > 0, + CommentMemory: data.SafeOutputs != nil && data.SafeOutputs.CommentMemory != nil, } if !cfg.CacheMemory && !cfg.RepoMemory && !cfg.CommentMemory { - return nil, nil + return nil, fmt.Errorf("jobs.%s.restore-memory: no memory stores are configured in tools", jobName) } return cfg, nil } -// validateRestoreMemoryConfig ensures that every requested memory type is actually -// configured in the workflow's tools: section. Custom jobs cannot declare memory -// stores independently; they can only restore from stores the workflow already uses. -func validateRestoreMemoryConfig(cfg *restoreMemoryConfig, data *WorkflowData, jobName string) error { - if cfg == nil { - return nil - } - if cfg.CacheMemory && (data.CacheMemoryConfig == nil || len(data.CacheMemoryConfig.Caches) == 0) { - return fmt.Errorf("jobs.%s.restore-memory.cache-memory: cache-memory must be configured in tools: to use restore-memory", jobName) - } - if cfg.RepoMemory && (data.RepoMemoryConfig == nil || len(data.RepoMemoryConfig.Memories) == 0) { - return fmt.Errorf("jobs.%s.restore-memory.repo-memory: repo-memory must be configured in tools: to use restore-memory", jobName) - } - if cfg.CommentMemory && (data.SafeOutputs == nil || data.SafeOutputs.CommentMemory == nil) { - return fmt.Errorf("jobs.%s.restore-memory.comment-memory: comment-memory must be configured in tools: to use restore-memory", jobName) - } - return nil -} - // buildRestoreMemorySteps returns two slices: // - setupLines: gh-aw checkout + setup steps (only when repo-memory or comment-memory // is requested, since those need scripts from the setup action) diff --git a/pkg/workflow/compiler_custom_job_memory_test.go b/pkg/workflow/compiler_custom_job_memory_test.go index ac0ab2eee87..2bad8b553b7 100644 --- a/pkg/workflow/compiler_custom_job_memory_test.go +++ b/pkg/workflow/compiler_custom_job_memory_test.go @@ -18,7 +18,7 @@ import ( // ======================================== // TestCustomJobRestoreMemoryCacheMemory verifies that a custom job with -// restore-memory.cache-memory: true gets cache restore steps injected. +// restore-memory: true gets cache restore steps injected when cache-memory is configured. // No artifact-upload or cache-save steps should be emitted. func TestCustomJobRestoreMemoryCacheMemory(t *testing.T) { tmpDir := testutil.TempDir(t, "custom-job-restore-cache-memory") @@ -35,8 +35,7 @@ tools: jobs: orchestrator: runs-on: ubuntu-latest - restore-memory: - cache-memory: true + restore-memory: true steps: - name: Read memory and dispatch run: echo "dispatching" @@ -83,7 +82,7 @@ Reads cache memory and dispatches tasks. } // TestCustomJobRestoreMemoryRepoMemory verifies that a custom job with -// restore-memory.repo-memory: true gets repo-memory clone steps injected. +// restore-memory: true gets repo-memory clone steps injected when repo-memory is configured. func TestCustomJobRestoreMemoryRepoMemory(t *testing.T) { tmpDir := testutil.TempDir(t, "custom-job-restore-repo-memory") @@ -99,8 +98,7 @@ tools: jobs: orchestrator: runs-on: ubuntu-latest - restore-memory: - repo-memory: true + restore-memory: true steps: - name: Read repo memory run: cat /tmp/gh-aw/repo-memory/default/state.json || echo "{}" @@ -133,7 +131,7 @@ jobs: } // TestCustomJobRestoreMemoryCommentMemory verifies that a custom job with -// restore-memory.comment-memory: true gets the prepare-comment-memory step injected. +// restore-memory: true gets the prepare-comment-memory step injected when comment-memory is configured. func TestCustomJobRestoreMemoryCommentMemory(t *testing.T) { tmpDir := testutil.TempDir(t, "custom-job-restore-comment-memory") @@ -150,8 +148,7 @@ tools: jobs: orchestrator: runs-on: ubuntu-latest - restore-memory: - comment-memory: true + restore-memory: true steps: - name: Process comment memory run: ls /tmp/gh-aw/comment-memory/ || echo "empty" @@ -180,8 +177,8 @@ jobs: assert.Contains(t, section, "actions/github-script@", "github-script action should be used") } -// TestCustomJobRestoreMemoryMultipleTypes verifies that a custom job can request -// multiple memory types at once. +// TestCustomJobRestoreMemoryMultipleTypes verifies that a custom job with +// restore-memory: true restores all configured memory types at once. func TestCustomJobRestoreMemoryMultipleTypes(t *testing.T) { tmpDir := testutil.TempDir(t, "custom-job-restore-multiple-memory") @@ -199,9 +196,7 @@ tools: jobs: orchestrator: runs-on: ubuntu-latest - restore-memory: - cache-memory: true - repo-memory: true + restore-memory: true steps: - name: Process memories run: echo "done" @@ -245,8 +240,7 @@ tools: jobs: orchestrator: runs-on: ubuntu-latest - restore-memory: - cache-memory: true + restore-memory: true pre-steps: - name: My pre-step run: echo "pre" @@ -287,61 +281,10 @@ jobs: ) } -// TestCustomJobRestoreMemoryErrorWhenNotConfigured verifies that a custom job -// requesting restore-memory for a type not configured in tools: produces an error. +// TestCustomJobRestoreMemoryErrorWhenNotConfigured verifies that a custom job with +// restore-memory: true fails when no memory stores are configured in tools:. func TestCustomJobRestoreMemoryErrorWhenNotConfigured(t *testing.T) { - tests := []struct { - name string - frontmatter string - wantErrMsg string - }{ - { - name: "cache-memory not configured", - frontmatter: `--- -name: Test -on: workflow_dispatch -permissions: - contents: read -engine: copilot -strict: false -jobs: - orchestrator: - runs-on: ubuntu-latest - restore-memory: - cache-memory: true - steps: - - name: Step - run: echo hi ---- -# Test -`, - wantErrMsg: "cache-memory must be configured in tools:", - }, - { - name: "repo-memory not configured", - frontmatter: `--- -name: Test -on: workflow_dispatch -permissions: - contents: read -engine: copilot -strict: false -jobs: - orchestrator: - runs-on: ubuntu-latest - restore-memory: - repo-memory: true - steps: - - name: Step - run: echo hi ---- -# Test -`, - wantErrMsg: "repo-memory must be configured in tools:", - }, - { - name: "comment-memory not configured", - frontmatter: `--- + frontmatter := `--- name: Test on: workflow_dispatch permissions: @@ -351,30 +294,21 @@ strict: false jobs: orchestrator: runs-on: ubuntu-latest - restore-memory: - comment-memory: true + restore-memory: true steps: - name: Step run: echo hi --- # Test -`, - wantErrMsg: "comment-memory must be configured in tools:", - }, - } +` + tmpDir := testutil.TempDir(t, "restore-memory-error-*") + testFile := filepath.Join(tmpDir, "test.md") + require.NoError(t, os.WriteFile(testFile, []byte(frontmatter), 0644)) - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - tmpDir := testutil.TempDir(t, "restore-memory-error-*") - testFile := filepath.Join(tmpDir, "test.md") - require.NoError(t, os.WriteFile(testFile, []byte(tc.frontmatter), 0644)) - - compiler := NewCompiler() - err := compiler.CompileWorkflow(testFile) - require.Error(t, err, "expected compilation to fail") - assert.Contains(t, err.Error(), tc.wantErrMsg) - }) - } + compiler := NewCompiler() + err := compiler.CompileWorkflow(testFile) + require.Error(t, err, "expected compilation to fail") + assert.Contains(t, err.Error(), "no memory stores are configured in tools") } // TestCustomJobRestoreMemoryOnlyEmitsRestoreSteps verifies that when restore-memory @@ -396,9 +330,7 @@ tools: jobs: orchestrator: runs-on: ubuntu-latest - restore-memory: - cache-memory: true - repo-memory: true + restore-memory: true steps: - name: Process data run: echo "processing" @@ -450,8 +382,7 @@ tools: jobs: orchestrator: runs-on: ubuntu-latest - restore-memory: - cache-memory: true + restore-memory: true --- # Orchestrator Workflow @@ -477,60 +408,78 @@ jobs: // TestExtractRestoreMemoryConfig unit-tests the config extraction logic. func TestExtractRestoreMemoryConfig(t *testing.T) { + cacheData := &WorkflowData{ + CacheMemoryConfig: &CacheMemoryConfig{ + Caches: []CacheMemoryEntry{{ID: "default"}}, + }, + } + allData := &WorkflowData{ + CacheMemoryConfig: &CacheMemoryConfig{ + Caches: []CacheMemoryEntry{{ID: "default"}}, + }, + RepoMemoryConfig: &RepoMemoryConfig{ + Memories: []RepoMemoryEntry{{ID: "default"}}, + }, + SafeOutputs: &SafeOutputsConfig{ + CommentMemory: &CommentMemoryConfig{}, + }, + } + emptyData := &WorkflowData{} + tests := []struct { name string configMap map[string]any + data *WorkflowData want *restoreMemoryConfig wantErr bool }{ { name: "no restore-memory field", configMap: map[string]any{}, + data: emptyData, want: nil, }, { - name: "all false values returns nil", - configMap: map[string]any{ - "restore-memory": map[string]any{ - "cache-memory": false, - "repo-memory": false, - "comment-memory": false, - }, - }, - want: nil, + name: "false disables restore-memory", + configMap: map[string]any{"restore-memory": false}, + data: cacheData, + want: nil, }, { - name: "cache-memory only", - configMap: map[string]any{ - "restore-memory": map[string]any{ - "cache-memory": true, - }, - }, - want: &restoreMemoryConfig{CacheMemory: true}, + name: "true with cache-memory only", + configMap: map[string]any{"restore-memory": true}, + data: cacheData, + want: &restoreMemoryConfig{CacheMemory: true}, }, { - name: "all enabled", - configMap: map[string]any{ - "restore-memory": map[string]any{ - "cache-memory": true, - "repo-memory": true, - "comment-memory": true, - }, - }, - want: &restoreMemoryConfig{CacheMemory: true, RepoMemory: true, CommentMemory: true}, + name: "true with all memory types", + configMap: map[string]any{"restore-memory": true}, + data: allData, + want: &restoreMemoryConfig{CacheMemory: true, RepoMemory: true, CommentMemory: true}, }, { - name: "invalid type returns error", - configMap: map[string]any{ - "restore-memory": "true", - }, - wantErr: true, + name: "true with no memory configured returns error", + configMap: map[string]any{"restore-memory": true}, + data: emptyData, + wantErr: true, + }, + { + name: "non-boolean value returns error", + configMap: map[string]any{"restore-memory": "true"}, + data: emptyData, + wantErr: true, + }, + { + name: "object value returns error", + configMap: map[string]any{"restore-memory": map[string]any{"cache-memory": true}}, + data: emptyData, + wantErr: true, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got, err := extractRestoreMemoryConfig(tc.configMap, "test_job") + got, err := extractRestoreMemoryConfig(tc.configMap, "test_job", tc.data) if tc.wantErr { require.Error(t, err) return @@ -541,52 +490,6 @@ func TestExtractRestoreMemoryConfig(t *testing.T) { } } -// TestValidateRestoreMemoryConfig unit-tests the config validation logic. -func TestValidateRestoreMemoryConfig(t *testing.T) { - emptyCacheData := &WorkflowData{} - cacheData := &WorkflowData{ - CacheMemoryConfig: &CacheMemoryConfig{ - Caches: []CacheMemoryEntry{{ID: "default"}}, - }, - } - repoData := &WorkflowData{ - RepoMemoryConfig: &RepoMemoryConfig{ - Memories: []RepoMemoryEntry{{ID: "default"}}, - }, - } - commentData := &WorkflowData{ - SafeOutputs: &SafeOutputsConfig{ - CommentMemory: &CommentMemoryConfig{}, - }, - } - - tests := []struct { - name string - cfg *restoreMemoryConfig - data *WorkflowData - wantErr bool - }{ - {name: "nil config is ok", cfg: nil, data: emptyCacheData, wantErr: false}, - {name: "cache-memory configured correctly", cfg: &restoreMemoryConfig{CacheMemory: true}, data: cacheData, wantErr: false}, - {name: "cache-memory missing from tools", cfg: &restoreMemoryConfig{CacheMemory: true}, data: emptyCacheData, wantErr: true}, - {name: "repo-memory configured correctly", cfg: &restoreMemoryConfig{RepoMemory: true}, data: repoData, wantErr: false}, - {name: "repo-memory missing from tools", cfg: &restoreMemoryConfig{RepoMemory: true}, data: emptyCacheData, wantErr: true}, - {name: "comment-memory configured correctly", cfg: &restoreMemoryConfig{CommentMemory: true}, data: commentData, wantErr: false}, - {name: "comment-memory missing from tools", cfg: &restoreMemoryConfig{CommentMemory: true}, data: emptyCacheData, wantErr: true}, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - err := validateRestoreMemoryConfig(tc.cfg, tc.data, "test_job") - if tc.wantErr { - require.Error(t, err) - } else { - require.NoError(t, err) - } - }) - } -} - // TestGenerateCacheMemoryRestoreLines unit-tests the cache restore line generation. func TestGenerateCacheMemoryRestoreLines(t *testing.T) { data := &WorkflowData{ diff --git a/pkg/workflow/compiler_custom_jobs.go b/pkg/workflow/compiler_custom_jobs.go index 72b8dcf55db..3cf169edb09 100644 --- a/pkg/workflow/compiler_custom_jobs.go +++ b/pkg/workflow/compiler_custom_jobs.go @@ -507,16 +507,13 @@ func (c *Compiler) configureCustomJobSteps(job *Job, jobName string, configMap m } } - // Parse and validate restore-memory configuration. + // Parse restore-memory configuration. // restore-memory injects read-only memory restore steps into the custom job. // No write-back or commit steps are ever emitted for memory in custom jobs. - restoreMemCfg, err := extractRestoreMemoryConfig(configMap, jobName) + restoreMemCfg, err := extractRestoreMemoryConfig(configMap, jobName, data) if err != nil { return err } - if err := validateRestoreMemoryConfig(restoreMemCfg, data, jobName); err != nil { - return err - } hasRestoreMemory := restoreMemCfg != nil From 080e9773503132e8482ca45633ede98881147f44 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 15:01:01 +0000 Subject: [PATCH 05/11] Update ADR to reflect simplified boolean restore-memory API Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../adr/44037-restore-memory-read-only-access-custom-jobs.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/adr/44037-restore-memory-read-only-access-custom-jobs.md b/docs/adr/44037-restore-memory-read-only-access-custom-jobs.md index 9b31ac4d15e..91331a0d8b8 100644 --- a/docs/adr/44037-restore-memory-read-only-access-custom-jobs.md +++ b/docs/adr/44037-restore-memory-read-only-access-custom-jobs.md @@ -37,12 +37,11 @@ Rejected because this approach requires the agent job to run first and explicitl - Validation at compile time rejects requests for memory types not declared in `tools:`, surfacing misconfiguration early. #### Negative -- Increases compiler complexity: a new source file (`compiler_custom_job_memory.go`), new config parsing, validation, and injection plumbing into `configureCustomJobSteps`. -- Job authors must explicitly enumerate every memory type they need under `restore-memory:`; there is no auto-detection based on what the workflow's `tools:` section declares. +- Increases compiler complexity: a new source file (`compiler_custom_job_memory.go`), new config parsing, and injection plumbing into `configureCustomJobSteps`. - The injected setup action step (required for repo-memory and comment-memory scripts) adds overhead to custom jobs that may not otherwise need the full gh-aw setup. #### Neutral -- The JSON schema for the custom job `additionalProperties` definition gains a new `restore-memory` object property; existing workflows without this field are unaffected. +- The JSON schema for the custom job `additionalProperties` definition gains a new `restore-memory` boolean property; existing workflows without this field are unaffected. - Step ordering is deterministic: GHES host config → gh-aw setup (if needed) → memory restore steps → pre-steps → regular steps. --- From e4132313ac5c00af168e272266d63ea6bc81f857 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 16:02:56 +0000 Subject: [PATCH 06/11] fix: address review comments on restore-memory custom job support - Extract shared buildCacheRestoreKeys helper from cache.go, replacing the duplicate restore-key logic in compiler_custom_job_memory.go - Fix double trailing newline in generateRepoMemoryRestoreLines: use strings.SplitAfter and skip empty parts - Fix silent skip: buildRestoreMemorySteps now returns a compile-time error when repo-memory/comment-memory are requested but the setup action ref is unavailable (was silently skipping setup lines while still emitting memory lines that depend on those scripts) - Inject GH_AW_WORKFLOW_ID_SANITIZED into custom job env when cache-memory restore is requested, without overwriting user-provided values, so cache keys match those used by the agent job - Add nil-data tests: TestGenerateRepoMemoryRestoreLinesNilData and TestGenerateCommentMemoryRestoreLinesNilData - Add steps: assertion to TestCustomJobRestoreMemoryStandaloneJob Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/workflow/cache.go | 68 +++++++++---------- pkg/workflow/compiler_custom_job_memory.go | 49 +++++-------- .../compiler_custom_job_memory_test.go | 17 +++++ pkg/workflow/compiler_custom_jobs.go | 18 ++++- 4 files changed, 84 insertions(+), 68 deletions(-) diff --git a/pkg/workflow/cache.go b/pkg/workflow/cache.go index e1b1def05ba..18689f0ec67 100644 --- a/pkg/workflow/cache.go +++ b/pkg/workflow/cache.go @@ -456,6 +456,39 @@ func writeCachePath(builder *strings.Builder, path any) { fmt.Fprintf(builder, " path: %v\n", path) } +// buildCacheRestoreKeys derives the ordered list of restore-keys for a cache entry. +// The primary key (without the run_id suffix) is always included. +// For "repo" scope, a second key that also strips the workflow ID is appended to allow +// cross-workflow cache sharing. +// +// cacheKey must be the fully-formed primary key (e.g. as returned by +// computeIntegrityCacheKey) and scope is the cache entry's scope field +// ("workflow" or "repo"; empty is treated as "workflow"). +func buildCacheRestoreKeys(cacheKey, scope string) []string { + if scope == "" { + scope = "workflow" + } + const runIDSuffix = "-${{ github.run_id }}" + + var keys []string + if strings.HasSuffix(cacheKey, runIDSuffix) { + keys = append(keys, strings.TrimSuffix(cacheKey, "${{ github.run_id }}")) + } else { + parts := strings.Split(cacheKey, "-") + if len(parts) >= 2 { + keys = append(keys, strings.Join(parts[:len(parts)-1], "-")+"-") + } + } + + if scope == "repo" { + repoKey := strings.TrimSuffix(cacheKey, "${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}-${{ github.run_id }}") + if repoKey != cacheKey && repoKey != "" { + keys = append(keys, repoKey) + } + } + return keys +} + func writeCacheRestoreKeys(builder *strings.Builder, restoreKeys any) { if restoreKeys == nil { return @@ -526,40 +559,7 @@ func generateCacheMemorySteps(builder *strings.Builder, data *WorkflowData) { // Generate restore keys based on scope // - "workflow" (default): Single restore key with workflow ID (secure) // - "repo": Two restore keys - with and without workflow ID (allows cross-workflow sharing) - var restoreKeys []string - - // Determine scope (default to "workflow" for safety) - scope := cache.Scope - if scope == "" { - scope = "workflow" - } - - // First restore key: remove the run_id suffix as a single unit (don't split the key) - // The cacheKey always ends with "-${{ github.run_id }}" (ensured by code above) - if strings.HasSuffix(cacheKey, runIdSuffix) { - // Remove the run_id suffix to create the restore key - restoreKey := strings.TrimSuffix(cacheKey, "${{ github.run_id }}") // Keep the trailing "-" - restoreKeys = append(restoreKeys, restoreKey) - } else { - // Fallback: split on last dash if run_id suffix not found - // This handles edge cases where the key format might be different - keyParts := strings.Split(cacheKey, "-") - if len(keyParts) >= 2 { - workflowLevelKey := strings.Join(keyParts[:len(keyParts)-1], "-") + "-" - restoreKeys = append(restoreKeys, workflowLevelKey) - } - } - - // For repo scope, add an additional restore key without the workflow ID - // This allows cache sharing across all workflows in the repository - if scope == "repo" { - // Remove both workflow and run_id to create a repo-wide restore key - // For example: "memory-none-nopolicy-chroma-${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}-${{ github.run_id }}" -> "memory-none-nopolicy-chroma-" - repoKey := strings.TrimSuffix(cacheKey, "${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}-${{ github.run_id }}") - if repoKey != cacheKey && repoKey != "" { - restoreKeys = append(restoreKeys, repoKey) - } - } + restoreKeys := buildCacheRestoreKeys(cacheKey, cache.Scope) // Step name and action // Use actions/cache/restore for restore-only caches or when threat detection is enabled diff --git a/pkg/workflow/compiler_custom_job_memory.go b/pkg/workflow/compiler_custom_job_memory.go index 282e155589b..9e6cded8c59 100644 --- a/pkg/workflow/compiler_custom_job_memory.go +++ b/pkg/workflow/compiler_custom_job_memory.go @@ -51,20 +51,21 @@ func extractRestoreMemoryConfig(configMap map[string]any, jobName string, data * // - memoryLines: the actual memory restore/clone/prepare steps // // No write-back steps are ever emitted; all injected steps are read-only. -func (c *Compiler) buildRestoreMemorySteps(cfg *restoreMemoryConfig, data *WorkflowData) (setupLines []string, memoryLines []string) { +func (c *Compiler) buildRestoreMemorySteps(cfg *restoreMemoryConfig, jobName string, data *WorkflowData) (setupLines []string, memoryLines []string, err error) { if cfg == nil { - return nil, nil + return nil, nil, nil } // repo-memory and comment-memory rely on scripts installed by the gh-aw setup action. // Inject the setup step before any memory steps so those scripts are available. if cfg.RepoMemory || cfg.CommentMemory { setupActionRef := c.resolveActionReference("./actions/setup", data) - if setupActionRef != "" || c.actionMode.IsScript() { - setupLines = append(setupLines, c.generateCheckoutActionsFolder(data)...) - // Pass empty trace IDs — custom jobs do not inherit the activation span. - setupLines = append(setupLines, c.generateSetupStep(data, setupActionRef, SetupActionDestination, false, "", "")...) + if setupActionRef == "" && !c.actionMode.IsScript() { + return nil, nil, fmt.Errorf("jobs.%s.restore-memory: repo-memory/comment-memory require the setup action but no action ref was found", jobName) } + setupLines = append(setupLines, c.generateCheckoutActionsFolder(data)...) + // Pass empty trace IDs — custom jobs do not inherit the activation span. + setupLines = append(setupLines, c.generateSetupStep(data, setupActionRef, SetupActionDestination, false, "", "")...) } if cfg.CacheMemory { @@ -77,7 +78,7 @@ func (c *Compiler) buildRestoreMemorySteps(cfg *restoreMemoryConfig, data *Workf memoryLines = append(memoryLines, generateCommentMemoryRestoreLines(data)...) } - return setupLines, memoryLines + return setupLines, memoryLines, nil } // generateCacheMemoryRestoreLines produces read-only cache-memory restore steps for a @@ -118,29 +119,8 @@ func generateCacheMemoryRestoreLines(data *WorkflowData) []string { lines = append(lines, fmt.Sprintf(" key: %s\n", cacheKey)) lines = append(lines, fmt.Sprintf(" path: %s\n", cacheDir)) - // Build restore keys using the same logic as generateCacheMemorySteps. - var restoreKeys []string - runIDSuffix := "-${{ github.run_id }}" - if strings.HasSuffix(cacheKey, runIDSuffix) { - restoreKeys = append(restoreKeys, strings.TrimSuffix(cacheKey, "${{ github.run_id }}")) - } else { - keyParts := strings.Split(cacheKey, "-") - if len(keyParts) >= 2 { - restoreKeys = append(restoreKeys, strings.Join(keyParts[:len(keyParts)-1], "-")+"-") - } - } - - scope := cache.Scope - if scope == "" { - scope = "workflow" - } - if scope == "repo" { - repoKey := strings.TrimSuffix(cacheKey, "${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}-${{ github.run_id }}") - if repoKey != cacheKey && repoKey != "" { - restoreKeys = append(restoreKeys, repoKey) - } - } - + // Build restore keys using the shared helper (same semantics as the agent job). + restoreKeys := buildCacheRestoreKeys(cacheKey, cache.Scope) lines = append(lines, " restore-keys: |\n") for _, key := range restoreKeys { lines = append(lines, fmt.Sprintf(" %s\n", key)) @@ -165,11 +145,14 @@ func generateRepoMemoryRestoreLines(data *WorkflowData) []string { return nil } - // Split into individual lines, preserving their trailing newlines. - parts := strings.Split(raw, "\n") + // SplitAfter keeps each line's trailing newline; skip any empty final element + // that strings.Split would produce for a newline-terminated string. + parts := strings.SplitAfter(raw, "\n") lines := make([]string, 0, len(parts)) for _, p := range parts { - lines = append(lines, p+"\n") + if p != "" { + lines = append(lines, p) + } } return lines } diff --git a/pkg/workflow/compiler_custom_job_memory_test.go b/pkg/workflow/compiler_custom_job_memory_test.go index 2bad8b553b7..ce485717d35 100644 --- a/pkg/workflow/compiler_custom_job_memory_test.go +++ b/pkg/workflow/compiler_custom_job_memory_test.go @@ -404,6 +404,7 @@ jobs: assert.Contains(t, section, "Restore cache-memory", "restore step must be present") assert.Contains(t, section, "Configure GH_HOST", "GHES config step must be present") + assert.Contains(t, section, "steps:", "steps key must be present even with no explicit steps") } // TestExtractRestoreMemoryConfig unit-tests the config extraction logic. @@ -518,3 +519,19 @@ func TestGenerateCacheMemoryRestoreLines(t *testing.T) { func TestGenerateCacheMemoryRestoreLinesNilData(t *testing.T) { assert.Nil(t, generateCacheMemoryRestoreLines(&WorkflowData{})) } + +// TestGenerateRepoMemoryRestoreLinesNilData verifies nil/empty RepoMemoryConfig returns nil. +func TestGenerateRepoMemoryRestoreLinesNilData(t *testing.T) { + assert.Nil(t, generateRepoMemoryRestoreLines(&WorkflowData{})) + assert.Nil(t, generateRepoMemoryRestoreLines(&WorkflowData{ + RepoMemoryConfig: &RepoMemoryConfig{}, + })) +} + +// TestGenerateCommentMemoryRestoreLinesNilData verifies nil/empty SafeOutputs returns nil. +func TestGenerateCommentMemoryRestoreLinesNilData(t *testing.T) { + assert.Nil(t, generateCommentMemoryRestoreLines(&WorkflowData{})) + assert.Nil(t, generateCommentMemoryRestoreLines(&WorkflowData{ + SafeOutputs: &SafeOutputsConfig{}, + })) +} diff --git a/pkg/workflow/compiler_custom_jobs.go b/pkg/workflow/compiler_custom_jobs.go index 3cf169edb09..7cb53f71e2f 100644 --- a/pkg/workflow/compiler_custom_jobs.go +++ b/pkg/workflow/compiler_custom_jobs.go @@ -517,6 +517,19 @@ func (c *Compiler) configureCustomJobSteps(job *Job, jobName string, configMap m hasRestoreMemory := restoreMemCfg != nil + // When cache-memory restore is requested, inject GH_AW_WORKFLOW_ID_SANITIZED so that + // restore keys match those used by the agent job. Only set it when the user has not + // already provided the variable in their job's env: block. + if hasRestoreMemory && restoreMemCfg.CacheMemory && data.WorkflowID != "" { + sanitized := SanitizeWorkflowIDForCacheKey(data.WorkflowID) + if job.Env == nil { + job.Env = make(map[string]string) + } + if _, alreadySet := job.Env["GH_AW_WORKFLOW_ID_SANITIZED"]; !alreadySet { + job.Env["GH_AW_WORKFLOW_ID_SANITIZED"] = sanitized + } + } + if hasSetupStepsField || hasPreStepsField || hasStepsField || hasRestoreMemory { job.Steps = append(job.Steps, setupSteps...) // Prepend GH_HOST configuration step for GHES/GHEC compatibility. @@ -529,7 +542,10 @@ func (c *Compiler) configureCustomJobSteps(job *Job, jobName string, configMap m // Setup lines come first (they install scripts needed by repo/comment memory). // Memory lines follow immediately after (restore/clone/prepare steps). if hasRestoreMemory { - memorySetupLines, memoryRestoreLines := c.buildRestoreMemorySteps(restoreMemCfg, data) + memorySetupLines, memoryRestoreLines, memErr := c.buildRestoreMemorySteps(restoreMemCfg, jobName, data) + if memErr != nil { + return memErr + } job.Steps = append(job.Steps, memorySetupLines...) job.Steps = append(job.Steps, memoryRestoreLines...) } From 74a4e18a3c57e282c5c281ee6a5ea0390a65e725 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 16:04:21 +0000 Subject: [PATCH 07/11] fix: clarify SplitAfter comment in generateRepoMemoryRestoreLines Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/workflow/compiler_custom_job_memory.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/workflow/compiler_custom_job_memory.go b/pkg/workflow/compiler_custom_job_memory.go index 9e6cded8c59..7d9f39fbc61 100644 --- a/pkg/workflow/compiler_custom_job_memory.go +++ b/pkg/workflow/compiler_custom_job_memory.go @@ -145,8 +145,9 @@ func generateRepoMemoryRestoreLines(data *WorkflowData) []string { return nil } - // SplitAfter keeps each line's trailing newline; skip any empty final element - // that strings.Split would produce for a newline-terminated string. + // SplitAfter keeps each line's trailing newline, so no phantom extra newline is + // appended (unlike strings.Split + "\n" which adds a spurious blank line for the + // empty trailing element produced by a newline-terminated string). parts := strings.SplitAfter(raw, "\n") lines := make([]string, 0, len(parts)) for _, p := range parts { From 1fe026c544146a0bbdde24d1279730949addce9a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 16:06:07 +0000 Subject: [PATCH 08/11] docs(adr): update ADR-44037 for simplified boolean API and canonical paths Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- ...ore-memory-read-only-access-custom-jobs.md | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/docs/adr/44037-restore-memory-read-only-access-custom-jobs.md b/docs/adr/44037-restore-memory-read-only-access-custom-jobs.md index 91331a0d8b8..8eb14dfa96a 100644 --- a/docs/adr/44037-restore-memory-read-only-access-custom-jobs.md +++ b/docs/adr/44037-restore-memory-read-only-access-custom-jobs.md @@ -12,7 +12,28 @@ Memory stores (`cache-memory`, `repo-memory`, `comment-memory`) in the gh-aw wor ### Decision -We will add a `restore-memory` field to the custom job config schema that allows job authors to explicitly opt in to read-only restore steps for any subset of the three memory types. The compiler injects the corresponding setup and restore steps (gh-aw setup action, `actions/cache/restore`, `clone_repo_memory_branch.sh`, or `setup_comment_memory_files.cjs`) directly into the custom job's step list, positioned after GHES host config and before `pre-steps`/`steps`. No write-back, git-commit, or artifact-upload steps are ever emitted for custom jobs — enforcement is structural (using `actions/cache/restore` instead of `actions/cache`), not advisory. +We will add a `restore-memory` boolean field to the custom job config schema. Setting `restore-memory: true` automatically enables read-only restore steps for all memory types that are declared in the workflow's `tools:` section. The compiler injects the corresponding setup and restore steps (gh-aw setup action, `actions/cache/restore`, `clone_repo_memory_branch.sh`, or `setup_comment_memory_files.cjs`) directly into the custom job's step list, positioned after GHES host config and before `pre-steps`/`steps`. No write-back, git-commit, or artifact-upload steps are ever emitted for custom jobs — enforcement is structural (using `actions/cache/restore` instead of `actions/cache`), not advisory. + +**Example**: + +```yaml +tools: + cache-memory: true + +jobs: + orchestrator: + runs-on: ubuntu-latest + restore-memory: true # enables all configured memory stores (read-only) + steps: + - name: Read state and dispatch + run: cat /tmp/gh-aw/cache-memory/state.json # default cache dir +``` + +Canonical runtime paths: +- Default cache-memory: `/tmp/gh-aw/cache-memory` +- Named cache-memory ``: `/tmp/gh-aw/cache-memory-` + +The cache-memory key uses `${{ env.GH_AW_WORKFLOW_ID_SANITIZED }}` as part of its key; the compiler automatically injects this env var into the custom job when cache-memory restore is requested so that keys match those used by the agent job. ### Alternatives Considered @@ -28,6 +49,12 @@ Have the agent job export its memory state as a named workflow output or GitHub Rejected because this approach requires the agent job to run first and explicitly export state — it cannot be used in pre-dispatch orchestrator patterns where the orchestrator runs *before* the agent. It also conflates memory (a persistent side channel) with ephemeral per-run job outputs, muddying the conceptual model. Existing memory restore machinery (`generateRepoMemorySteps`, `generateCacheMemoryRestoreLines`) is already well-tested and reusable, making step injection lower risk than a new artifact-based path. +#### Alternative 3: Per-Type Boolean Map (`restore-memory.cache-memory: true` etc.) + +Allow the user to select which memory types to restore individually (e.g. `restore-memory: { cache-memory: true, repo-memory: false }`). + +Rejected in favour of the simple boolean form: any memory type declared in `tools:` is already opt-in at the workflow level, so re-selecting them at the job level adds friction without meaningful benefit. The compiler selects only the types that are configured, so `restore-memory: true` is strictly additive — it cannot restore types that weren't enabled globally. + ### Consequences #### Positive @@ -35,10 +62,12 @@ Rejected because this approach requires the agent job to run first and explicitl - Read-only is enforced structurally — `actions/cache/restore` never auto-saves, so concurrent orchestrators cannot corrupt the memory store. - Reuses existing, tested step generators (`generateRepoMemorySteps`, `generateCommentMemoryRestoreLines`), minimising new code surface. - Validation at compile time rejects requests for memory types not declared in `tools:`, surfacing misconfiguration early. +- `GH_AW_WORKFLOW_ID_SANITIZED` is injected automatically, ensuring cache keys match the agent job without requiring user configuration. #### Negative - Increases compiler complexity: a new source file (`compiler_custom_job_memory.go`), new config parsing, and injection plumbing into `configureCustomJobSteps`. - The injected setup action step (required for repo-memory and comment-memory scripts) adds overhead to custom jobs that may not otherwise need the full gh-aw setup. +- When repo-memory or comment-memory is requested but the setup action ref cannot be resolved, the compiler surfaces a compile-time error (not a runtime failure). #### Neutral - The JSON schema for the custom job `additionalProperties` definition gains a new `restore-memory` boolean property; existing workflows without this field are unaffected. From f05f676b94557bd95f1c96684ac166602e4d549a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 17:03:17 +0000 Subject: [PATCH 09/11] fix: guard empty restore-keys block in generateCacheMemoryRestoreLines Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/workflow/compiler_custom_job_memory.go | 11 +++++++---- pkg/workflow/compiler_custom_job_memory_test.go | 10 ++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/pkg/workflow/compiler_custom_job_memory.go b/pkg/workflow/compiler_custom_job_memory.go index 7d9f39fbc61..46641103769 100644 --- a/pkg/workflow/compiler_custom_job_memory.go +++ b/pkg/workflow/compiler_custom_job_memory.go @@ -120,10 +120,13 @@ func generateCacheMemoryRestoreLines(data *WorkflowData) []string { lines = append(lines, fmt.Sprintf(" path: %s\n", cacheDir)) // Build restore keys using the shared helper (same semantics as the agent job). - restoreKeys := buildCacheRestoreKeys(cacheKey, cache.Scope) - lines = append(lines, " restore-keys: |\n") - for _, key := range restoreKeys { - lines = append(lines, fmt.Sprintf(" %s\n", key)) + // Only emit the restore-keys block when there is at least one key; an empty + // literal block scalar (restore-keys: | with no lines) is invalid YAML. + if restoreKeys := buildCacheRestoreKeys(cacheKey, cache.Scope); len(restoreKeys) > 0 { + lines = append(lines, " restore-keys: |\n") + for _, key := range restoreKeys { + lines = append(lines, fmt.Sprintf(" %s\n", key)) + } } } diff --git a/pkg/workflow/compiler_custom_job_memory_test.go b/pkg/workflow/compiler_custom_job_memory_test.go index ce485717d35..2fbe856a35b 100644 --- a/pkg/workflow/compiler_custom_job_memory_test.go +++ b/pkg/workflow/compiler_custom_job_memory_test.go @@ -520,6 +520,16 @@ func TestGenerateCacheMemoryRestoreLinesNilData(t *testing.T) { assert.Nil(t, generateCacheMemoryRestoreLines(&WorkflowData{})) } +// TestBuildCacheRestoreKeysEmptyForSinglePartKey verifies that buildCacheRestoreKeys +// returns nil when the cache key has no separators and does not end with the run_id +// suffix — ensuring the guard in generateCacheMemoryRestoreLines is exercised correctly. +func TestBuildCacheRestoreKeysEmptyForSinglePartKey(t *testing.T) { + // A single-part key (no dashes) that doesn't end with the run_id suffix + // must produce no restore keys, confirming the len>0 guard is needed. + keys := buildCacheRestoreKeys("noseparator", "workflow") + assert.Empty(t, keys, "single-part key with no dashes must produce no restore keys") +} + // TestGenerateRepoMemoryRestoreLinesNilData verifies nil/empty RepoMemoryConfig returns nil. func TestGenerateRepoMemoryRestoreLinesNilData(t *testing.T) { assert.Nil(t, generateRepoMemoryRestoreLines(&WorkflowData{})) From 800c01cd61d714cca845a0b9406129dd63be50fe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 20:28:21 +0000 Subject: [PATCH 10/11] test: add restore-memory integration workflow tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../restore_memory_integration_test.go | 638 ++++++++++++++++++ 1 file changed, 638 insertions(+) create mode 100644 pkg/workflow/restore_memory_integration_test.go diff --git a/pkg/workflow/restore_memory_integration_test.go b/pkg/workflow/restore_memory_integration_test.go new file mode 100644 index 00000000000..01549b214f2 --- /dev/null +++ b/pkg/workflow/restore_memory_integration_test.go @@ -0,0 +1,638 @@ +//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" +) + +// TestRestoreMemoryScheduledOrchestrator covers the primary use-case described in +// the PR: a scheduled job that reads cache-memory to build a dispatch list before +// the agent job runs. The agent job must still carry its own full cache write-path; +// the orchestrator job must carry only read-only restore steps. +func TestRestoreMemoryScheduledOrchestrator(t *testing.T) { + tmpDir := testutil.TempDir(t, "restore-memory-orchestrator") + workflowPath := filepath.Join(tmpDir, "orchestrator.md") + + content := `--- +name: Scheduled Orchestrator +on: + schedule: + - cron: "0 * * * *" +permissions: + contents: read +engine: copilot +strict: false +tools: + cache-memory: true +jobs: + orchestrator: + runs-on: ubuntu-latest + needs: [] + restore-memory: true + steps: + - name: Read state and dispatch + run: | + state=$(cat /tmp/gh-aw/cache-memory/state.json 2>/dev/null || echo '{}') + echo "state=$state" +--- + +# Scheduled Orchestrator + +Reads cache-memory state and dispatches tasks accordingly. +` + + if err := os.WriteFile(workflowPath, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + lockPath := stringutil.MarkdownToLockFile(workflowPath) + raw, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockFile := string(raw) + + orchestratorSection := extractJobSection(lockFile, "orchestrator") + if orchestratorSection == "" { + t.Fatal("Expected orchestrator job section in lock file") + } + + // Orchestrator must have read-only restore steps. + if !strings.Contains(orchestratorSection, "Create cache-memory directory") { + t.Error("Expected cache-memory directory creation step in orchestrator job") + } + if !strings.Contains(orchestratorSection, "actions/cache/restore@") { + t.Error("Expected actions/cache/restore step in orchestrator job") + } + if !strings.Contains(orchestratorSection, "Restore cache-memory") { + t.Error("Expected Restore cache-memory step in orchestrator job") + } + + // Orchestrator must NOT have write-back steps. + if strings.Contains(orchestratorSection, "actions/cache@") && !strings.Contains(orchestratorSection, "actions/cache/restore@") { + t.Error("Orchestrator job must not use the read-write actions/cache action") + } + if strings.Contains(orchestratorSection, "actions/upload-artifact@") { + t.Error("Orchestrator job must not upload artifacts") + } + if strings.Contains(orchestratorSection, "Setup cache-memory git") { + t.Error("Orchestrator job must not set up git integrity") + } + + // Agent job must still have its own full cache path (restore + save). + agentSection := extractJobSection(lockFile, "agent") + if agentSection == "" { + t.Fatal("Expected agent job section in lock file") + } + if !strings.Contains(agentSection, "Restore cache-memory file share data") { + t.Error("Agent job should still have its own cache restore step") + } +} + +// TestRestoreMemoryWorkflowIDSanitizedEnvInjection verifies that when cache-memory +// restore is requested the compiler injects GH_AW_WORKFLOW_ID_SANITIZED into the +// custom job's env block so that the cache key matches what the agent job uses. +func TestRestoreMemoryWorkflowIDSanitizedEnvInjection(t *testing.T) { + tmpDir := testutil.TempDir(t, "restore-memory-env-inject") + // File name: "my-orchestrator.md" + // GetWorkflowIDFromPath → "my-orchestrator" + // SanitizeWorkflowIDForCacheKey → "myorchestrator" + workflowPath := filepath.Join(tmpDir, "my-orchestrator.md") + + content := `--- +name: Env Inject Test +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +tools: + cache-memory: true +jobs: + orchestrator: + runs-on: ubuntu-latest + restore-memory: true + steps: + - name: Use cache + run: ls /tmp/gh-aw/cache-memory/ +--- + +# Env Inject Test +` + + if err := os.WriteFile(workflowPath, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + lockPath := stringutil.MarkdownToLockFile(workflowPath) + raw, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockFile := string(raw) + + orchestratorSection := extractJobSection(lockFile, "orchestrator") + if orchestratorSection == "" { + t.Fatal("Expected orchestrator job section in lock file") + } + + // The env block for the orchestrator job should carry the sanitized workflow ID. + // SanitizeWorkflowIDForCacheKey("my-orchestrator") == "myorchestrator" + if !strings.Contains(orchestratorSection, "GH_AW_WORKFLOW_ID_SANITIZED") { + t.Error("Expected GH_AW_WORKFLOW_ID_SANITIZED in orchestrator job env") + } + if !strings.Contains(orchestratorSection, "myorchestrator") { + t.Error("Expected sanitized workflow ID 'myorchestrator' in orchestrator job env") + } +} + +// TestRestoreMemoryFalseIsNoop verifies that restore-memory: false does not inject +// any restore steps into the custom job, even when memory stores are configured. +func TestRestoreMemoryFalseIsNoop(t *testing.T) { + tmpDir := testutil.TempDir(t, "restore-memory-noop") + workflowPath := filepath.Join(tmpDir, "noop.md") + + content := `--- +name: No-Op Restore Memory +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +tools: + cache-memory: true +jobs: + processor: + runs-on: ubuntu-latest + restore-memory: false + steps: + - name: Do something else + run: echo "no memory needed" +--- + +# No-Op Test +` + + if err := os.WriteFile(workflowPath, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + lockPath := stringutil.MarkdownToLockFile(workflowPath) + raw, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockFile := string(raw) + + processorSection := extractJobSection(lockFile, "processor") + if processorSection == "" { + t.Fatal("Expected processor job section in lock file") + } + + if strings.Contains(processorSection, "actions/cache/restore@") { + t.Error("No cache restore step should be injected when restore-memory is false") + } + if strings.Contains(processorSection, "Restore cache-memory") { + t.Error("No restore step name should appear when restore-memory is false") + } + if strings.Contains(processorSection, "Create cache-memory directory") { + t.Error("No directory creation step should be injected when restore-memory is false") + } +} + +// TestRestoreMemoryMultipleNamedCaches verifies that all named cache entries receive +// their own restore steps when the cache-memory tool uses the array form. +func TestRestoreMemoryMultipleNamedCaches(t *testing.T) { + tmpDir := testutil.TempDir(t, "restore-memory-multi-cache") + workflowPath := filepath.Join(tmpDir, "multi-cache.md") + + content := `--- +name: Multi Cache Restore +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +tools: + cache-memory: + - id: default + key: memory-default + - id: session + key: memory-session +jobs: + aggregator: + runs-on: ubuntu-latest + restore-memory: true + steps: + - name: Aggregate results + run: | + cat /tmp/gh-aw/cache-memory/result.json || true + cat /tmp/gh-aw/cache-memory-session/session.json || true +--- + +# Multi Cache Restore +` + + if err := os.WriteFile(workflowPath, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + lockPath := stringutil.MarkdownToLockFile(workflowPath) + raw, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockFile := string(raw) + + aggregatorSection := extractJobSection(lockFile, "aggregator") + if aggregatorSection == "" { + t.Fatal("Expected aggregator job section in lock file") + } + + // Both named caches must have their own mkdir and restore steps. + if !strings.Contains(aggregatorSection, "Create cache-memory directory (default)") { + t.Error("Expected create step for default cache") + } + if !strings.Contains(aggregatorSection, "Restore cache-memory (default)") { + t.Error("Expected restore step for default cache") + } + if !strings.Contains(aggregatorSection, "Create cache-memory directory (session)") { + t.Error("Expected create step for session cache") + } + if !strings.Contains(aggregatorSection, "Restore cache-memory (session)") { + t.Error("Expected restore step for session cache") + } + + // Step IDs must be present and distinct. + if !strings.Contains(aggregatorSection, "restore_cache_memory_0") { + t.Error("Expected restore_cache_memory_0 step ID for first cache") + } + if !strings.Contains(aggregatorSection, "restore_cache_memory_1") { + t.Error("Expected restore_cache_memory_1 step ID for second cache") + } + + // No write-back steps. + if strings.Contains(aggregatorSection, "actions/upload-artifact@") { + t.Error("No artifact upload should be emitted in restore-memory job") + } +} + +// TestRestoreMemoryJobIsolation verifies that when two custom jobs are defined and +// only one has restore-memory: true, no restore steps bleed into the other job. +func TestRestoreMemoryJobIsolation(t *testing.T) { + tmpDir := testutil.TempDir(t, "restore-memory-isolation") + workflowPath := filepath.Join(tmpDir, "isolation.md") + + content := `--- +name: Job Isolation Test +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +tools: + cache-memory: true +jobs: + with_restore: + runs-on: ubuntu-latest + restore-memory: true + steps: + - name: Read memory + run: cat /tmp/gh-aw/cache-memory/data.json || true + without_restore: + runs-on: ubuntu-latest + steps: + - name: Other task + run: echo "I do not use memory" +--- + +# Job Isolation Test +` + + if err := os.WriteFile(workflowPath, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + lockPath := stringutil.MarkdownToLockFile(workflowPath) + raw, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockFile := string(raw) + + withRestoreSection := extractJobSection(lockFile, "with_restore") + if withRestoreSection == "" { + t.Fatal("Expected with_restore job section in lock file") + } + if !strings.Contains(withRestoreSection, "actions/cache/restore@") { + t.Error("Expected cache restore step in with_restore job") + } + + withoutRestoreSection := extractJobSection(lockFile, "without_restore") + if withoutRestoreSection == "" { + t.Fatal("Expected without_restore job section in lock file") + } + if strings.Contains(withoutRestoreSection, "actions/cache/restore@") { + t.Error("Cache restore steps must not bleed into without_restore job") + } + if strings.Contains(withoutRestoreSection, "Restore cache-memory") { + t.Error("Restore step name must not appear in without_restore job") + } +} + +// TestRestoreMemoryNonBooleanError verifies that a non-boolean restore-memory value +// (e.g. a quoted string) produces a compile-time error with a clear message. +func TestRestoreMemoryNonBooleanError(t *testing.T) { + tmpDir := testutil.TempDir(t, "restore-memory-bad-type") + workflowPath := filepath.Join(tmpDir, "bad-type.md") + + content := `--- +name: Bad Type Test +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +tools: + cache-memory: true +jobs: + orchestrator: + runs-on: ubuntu-latest + restore-memory: "yes" + steps: + - name: Step + run: echo hi +--- + +# Bad Type Test +` + + if err := os.WriteFile(workflowPath, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + err := compiler.CompileWorkflow(workflowPath) + if err == nil { + t.Fatal("Expected compile error for non-boolean restore-memory, got nil") + } + if !strings.Contains(err.Error(), "restore-memory") { + t.Errorf("Error should mention restore-memory, got: %v", err) + } + if !strings.Contains(err.Error(), "boolean") { + t.Errorf("Error should mention boolean type, got: %v", err) + } +} + +// TestRestoreMemoryRepoMemoryInCustomJob verifies that a custom job with +// restore-memory: true and a repo-memory tool gets the read-only clone steps +// injected, but no push/write-back steps. +func TestRestoreMemoryRepoMemoryInCustomJob(t *testing.T) { + tmpDir := testutil.TempDir(t, "restore-memory-repo") + workflowPath := filepath.Join(tmpDir, "repo-restore.md") + + content := `--- +name: Repo Memory Restore +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +tools: + repo-memory: true +jobs: + reader: + runs-on: ubuntu-latest + restore-memory: true + steps: + - name: Read state + run: cat /tmp/gh-aw/repo-memory/default/state.json || echo '{}' +--- + +# Repo Memory Restore +` + + if err := os.WriteFile(workflowPath, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + lockPath := stringutil.MarkdownToLockFile(workflowPath) + raw, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockFile := string(raw) + + readerSection := extractJobSection(lockFile, "reader") + if readerSection == "" { + t.Fatal("Expected reader job section in lock file") + } + + // Clone step must be present. + if !strings.Contains(readerSection, "Clone repo-memory branch") { + t.Error("Expected Clone repo-memory branch step in reader job") + } + if !strings.Contains(readerSection, "clone_repo_memory_branch.sh") { + t.Error("Expected clone_repo_memory_branch.sh script reference in reader job") + } + + // No push step must be present in the reader custom job (push lives in push_repo_memory job). + if strings.Contains(readerSection, "push_repo_memory") { + t.Error("Push step must not appear in restore-memory custom job") + } + + // Agent job still has its own push (the push_repo_memory job must exist). + if !strings.Contains(lockFile, "push_repo_memory") { + t.Error("Expected push_repo_memory job for the agent write-path") + } +} + +// TestRestoreMemoryNeedsAndRestoreMemory verifies that a custom job can declare both +// an explicit needs dependency and restore-memory: true in the same job, and that +// the compiled output reflects both the dependency and the injected restore steps. +func TestRestoreMemoryNeedsAndRestoreMemory(t *testing.T) { + tmpDir := testutil.TempDir(t, "restore-memory-needs") + workflowPath := filepath.Join(tmpDir, "needs-and-restore.md") + + content := `--- +name: Needs And Restore +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +tools: + cache-memory: true +jobs: + setup: + runs-on: ubuntu-latest + needs: [] + steps: + - name: Initialize + run: echo "setting up" + outputs: + done: ${{ steps.init.outputs.done }} + consumer: + runs-on: ubuntu-latest + needs: [setup] + restore-memory: true + steps: + - name: Consume memory + run: cat /tmp/gh-aw/cache-memory/output.json || true +--- + +# Needs And Restore +` + + if err := os.WriteFile(workflowPath, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + lockPath := stringutil.MarkdownToLockFile(workflowPath) + raw, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockFile := string(raw) + + consumerSection := extractJobSection(lockFile, "consumer") + if consumerSection == "" { + t.Fatal("Expected consumer job section in lock file") + } + + // consumer must depend on setup. + if !strings.Contains(consumerSection, "setup") { + t.Error("Expected consumer job to depend on setup job") + } + + // consumer must also have cache restore steps. + if !strings.Contains(consumerSection, "Restore cache-memory") { + t.Error("Expected Restore cache-memory step in consumer job") + } + if !strings.Contains(consumerSection, "actions/cache/restore@") { + t.Error("Expected actions/cache/restore action in consumer job") + } +} + +// TestRestoreMemoryStepOrderIntegration is an end-to-end check that GHES host config +// comes before restore-memory steps, which in turn come before user-defined steps, +// mirroring the ordering requirements from the unit tests. +func TestRestoreMemoryStepOrderIntegration(t *testing.T) { + tmpDir := testutil.TempDir(t, "restore-memory-step-order") + workflowPath := filepath.Join(tmpDir, "step-order.md") + + content := `--- +name: Step Order Test +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +tools: + cache-memory: true +jobs: + ordered: + runs-on: ubuntu-latest + restore-memory: true + pre-steps: + - name: My Pre Step + run: echo "pre" + steps: + - name: My Main Step + run: echo "main" +--- + +# Step Order Test +` + + if err := os.WriteFile(workflowPath, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + lockPath := stringutil.MarkdownToLockFile(workflowPath) + raw, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + lockFile := string(raw) + + orderedSection := extractJobSection(lockFile, "ordered") + if orderedSection == "" { + t.Fatal("Expected ordered job section in lock file") + } + + ghesPos := strings.Index(orderedSection, "Configure GH_HOST") + restorePos := strings.Index(orderedSection, "Restore cache-memory") + preStepPos := strings.Index(orderedSection, "My Pre Step") + mainStepPos := strings.Index(orderedSection, "My Main Step") + + if ghesPos < 0 { + t.Fatal("GH_HOST configuration step not found in ordered job") + } + if restorePos < 0 { + t.Fatal("Restore cache-memory step not found in ordered job") + } + if preStepPos < 0 { + t.Fatal("My Pre Step not found in ordered job") + } + if mainStepPos < 0 { + t.Fatal("My Main Step not found in ordered job") + } + + if ghesPos >= restorePos { + t.Error("GH_HOST configuration must come before restore-memory steps") + } + if restorePos >= preStepPos { + t.Error("restore-memory steps must come before pre-steps") + } + if preStepPos >= mainStepPos { + t.Error("pre-steps must come before main steps") + } +} From ef5f3abfcfa516acd40bd433e708c42f82821d60 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 20:55:10 +0000 Subject: [PATCH 11/11] fix: reject restore-memory on reusable workflow (uses:) jobs Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/workflow/compiler_custom_jobs.go | 6 +++ .../restore_memory_integration_test.go | 43 ++++++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/pkg/workflow/compiler_custom_jobs.go b/pkg/workflow/compiler_custom_jobs.go index 7cb53f71e2f..83f46ec1031 100644 --- a/pkg/workflow/compiler_custom_jobs.go +++ b/pkg/workflow/compiler_custom_jobs.go @@ -438,6 +438,12 @@ func (c *Compiler) configureCustomJobExecution(job *Job, jobName string, configM func configureCustomReusableWorkflow(job *Job, jobName string, usesStr string, configMap map[string]any) error { compilerJobsLog.Printf("Custom job '%s' is a reusable workflow call: %s", jobName, usesStr) + + // restore-memory cannot inject steps into reusable-workflow call jobs (no steps block). + if rm, ok := configMap["restore-memory"]; ok && rm != nil && rm != false { + return fmt.Errorf("jobs.%s.restore-memory: not supported for reusable workflow call jobs (uses: %s)", jobName, usesStr) + } + job.Uses = usesStr // Extract with parameters for reusable workflow diff --git a/pkg/workflow/restore_memory_integration_test.go b/pkg/workflow/restore_memory_integration_test.go index 01549b214f2..7ddc2dce879 100644 --- a/pkg/workflow/restore_memory_integration_test.go +++ b/pkg/workflow/restore_memory_integration_test.go @@ -12,7 +12,48 @@ import ( "github.com/github/gh-aw/pkg/testutil" ) -// TestRestoreMemoryScheduledOrchestrator covers the primary use-case described in +// TestRestoreMemoryWithUsesError verifies that restore-memory: true on a reusable +// workflow call job (one that has a `uses:` key) produces a compile-time error with +// a clear message, rather than being silently ignored. +func TestRestoreMemoryWithUsesError(t *testing.T) { + tmpDir := testutil.TempDir(t, "restore-memory-uses-error") + workflowPath := filepath.Join(tmpDir, "restore-memory-uses.md") + + content := `--- +name: Restore Memory Uses Error +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +tools: + cache-memory: true +jobs: + reusable-caller: + uses: ./.github/workflows/called.yml + restore-memory: true +--- + +# Restore Memory Uses Error +` + + if err := os.WriteFile(workflowPath, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := NewCompiler() + err := compiler.CompileWorkflow(workflowPath) + if err == nil { + t.Fatal("Expected compile error for restore-memory on reusable workflow job, got nil") + } + if !strings.Contains(err.Error(), "restore-memory") { + t.Errorf("Error should mention restore-memory, got: %v", err) + } + if !strings.Contains(err.Error(), "reusable") || !strings.Contains(err.Error(), "reusable-caller") { + t.Errorf("Error should mention the job name and reusable context, got: %v", err) + } +} + // the PR: a scheduled job that reads cache-memory to build a dispatch list before // the agent job runs. The agent job must still carry its own full cache write-path; // the orchestrator job must carry only read-only restore steps.