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..8eb14dfa96a --- /dev/null +++ b/docs/adr/44037-restore-memory-read-only-access-custom-jobs.md @@ -0,0 +1,78 @@ +# 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` 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 + +#### 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. + +#### 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 +- 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. +- `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. +- 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.* diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 0bff225c8a4..453c5dd453c 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -2690,6 +2690,10 @@ } } ] + }, + "restore-memory": { + "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/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 new file mode 100644 index 00000000000..46641103769 --- /dev/null +++ b/pkg/workflow/compiler_custom_job_memory.go @@ -0,0 +1,184 @@ +package workflow + +import ( + "fmt" + "strings" +) + +// restoreMemoryConfig holds the parsed restore-memory configuration for a custom job. +// 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 + RepoMemory bool + CommentMemory bool +} + +// extractRestoreMemoryConfig parses the restore-memory field from a custom job config map. +// 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 + } + + enabled, ok := rawVal.(bool) + if !ok { + return nil, fmt.Errorf("jobs.%s.restore-memory must be a boolean (true or false)", jobName) + } + if !enabled { + return nil, nil + } + + 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, fmt.Errorf("jobs.%s.restore-memory: no memory stores are configured in tools", jobName) + } + return cfg, 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, jobName string, data *WorkflowData) (setupLines []string, memoryLines []string, err error) { + if cfg == 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() { + 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 { + memoryLines = append(memoryLines, generateCacheMemoryRestoreLines(data)...) + } + if cfg.RepoMemory { + memoryLines = append(memoryLines, generateRepoMemoryRestoreLines(data)...) + } + if cfg.CommentMemory { + memoryLines = append(memoryLines, generateCommentMemoryRestoreLines(data)...) + } + + return setupLines, memoryLines, nil +} + +// 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 shared helper (same semantics as the agent job). + // 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)) + } + } + } + + 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 + } + + // 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 { + if p != "" { + lines = append(lines, p) + } + } + 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..2fbe856a35b --- /dev/null +++ b/pkg/workflow/compiler_custom_job_memory_test.go @@ -0,0 +1,547 @@ +//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: 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") + + 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: 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: 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") + + 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: 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: 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") + + 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: 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 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") + + 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: 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: 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 with +// restore-memory: true fails when no memory stores are configured in tools:. +func TestCustomJobRestoreMemoryErrorWhenNotConfigured(t *testing.T) { + frontmatter := `--- +name: Test +on: workflow_dispatch +permissions: + contents: read +engine: copilot +strict: false +jobs: + orchestrator: + runs-on: ubuntu-latest + restore-memory: true + steps: + - name: Step + run: echo hi +--- +# Test +` + tmpDir := testutil.TempDir(t, "restore-memory-error-*") + testFile := filepath.Join(tmpDir, "test.md") + require.NoError(t, os.WriteFile(testFile, []byte(frontmatter), 0644)) + + 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 +// 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: 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: 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") + assert.Contains(t, section, "steps:", "steps key must be present even with no explicit steps") +} + +// 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: "false disables restore-memory", + configMap: map[string]any{"restore-memory": false}, + data: cacheData, + want: nil, + }, + { + name: "true with cache-memory only", + configMap: map[string]any{"restore-memory": true}, + data: cacheData, + want: &restoreMemoryConfig{CacheMemory: 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: "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", tc.data) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +// 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{})) +} + +// 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{})) + 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 561b030544f..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 @@ -507,13 +513,49 @@ func (c *Compiler) configureCustomJobSteps(job *Job, jobName string, configMap m } } - if hasSetupStepsField || hasPreStepsField || hasStepsField { + // 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, data) + if err != nil { + return err + } + + 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. // 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, memErr := c.buildRestoreMemorySteps(restoreMemCfg, jobName, data) + if memErr != nil { + return memErr + } + job.Steps = append(job.Steps, memorySetupLines...) + job.Steps = append(job.Steps, memoryRestoreLines...) + } + job.Steps = append(job.Steps, preSteps...) job.Steps = append(job.Steps, regularSteps...) } diff --git a/pkg/workflow/restore_memory_integration_test.go b/pkg/workflow/restore_memory_integration_test.go new file mode 100644 index 00000000000..7ddc2dce879 --- /dev/null +++ b/pkg/workflow/restore_memory_integration_test.go @@ -0,0 +1,679 @@ +//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" +) + +// 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. +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") + } +}