diff --git a/docs/src/content/docs/reference/steps-jobs.md b/docs/src/content/docs/reference/steps-jobs.md index 8ab2c91ba46..cda48b71921 100644 --- a/docs/src/content/docs/reference/steps-jobs.md +++ b/docs/src/content/docs/reference/steps-jobs.md @@ -85,6 +85,7 @@ jobs: | `setup-steps` | Steps injected immediately after the compiler-generated `actions/setup` step for that job (except `activation` and `pre_activation`, where compile fails) | | `pre-steps` | Steps injected after compiler setup steps and before checkout/`steps` in that job | | `steps` | List of steps — supports complete GitHub Actions step specification | +| `restore-memory` | Restore configured memory stores read-only before `pre-steps`/`steps` so deterministic jobs can read prior state | | `uses` | Reusable workflow to call | | `with` | Input parameters for a reusable workflow | | `secrets` | Secrets passed to a reusable workflow | @@ -115,8 +116,11 @@ Use this map to see where compiler-inserted steps land for each job type. 1. `jobs..setup-steps` 2. Compiler host setup (`Configure GH_HOST for enterprise compatibility`) -3. `jobs..pre-steps` -4. `jobs..steps` +3. `restore-memory` injected setup/restore steps (when enabled) +4. `jobs..pre-steps` +5. `jobs..steps` + +Set `jobs..restore-memory: true` to restore any configured `cache-memory`, `repo-memory`, and `comment-memory` stores before deterministic job steps run. This surface is read-only: gh-aw injects restore/clone/prepare steps, but never emits cache save, repo push, or safe-output write-back steps for custom jobs. ### Built-in jobs diff --git a/pkg/workflow/compiler_custom_job_memory.go b/pkg/workflow/compiler_custom_job_memory.go index 1b1547e352e..bcd04a288df 100644 --- a/pkg/workflow/compiler_custom_job_memory.go +++ b/pkg/workflow/compiler_custom_job_memory.go @@ -82,7 +82,10 @@ func (c *Compiler) buildRestoreMemorySteps(cfg *restoreMemoryConfig, jobName str memoryLines = append(memoryLines, generateRepoMemoryRestoreLines(data)...) } if cfg.CommentMemory { - memoryLines = append(memoryLines, generateCommentMemoryRestoreLines(data)...) + if configLines, ok := c.generateCommentMemoryEarlyConfigLines(data); ok { + memoryLines = append(memoryLines, configLines...) + memoryLines = append(memoryLines, generateCommentMemoryRestoreLines(data)...) + } } return setupLines, memoryLines, nil diff --git a/pkg/workflow/compiler_custom_job_memory_test.go b/pkg/workflow/compiler_custom_job_memory_test.go index 2fbe856a35b..ac699dec829 100644 --- a/pkg/workflow/compiler_custom_job_memory_test.go +++ b/pkg/workflow/compiler_custom_job_memory_test.go @@ -172,9 +172,15 @@ jobs: require.NotEmpty(t, section, "Expected orchestrator job section in lock file") // Must contain prepare comment memory step + assert.Contains(t, section, "Write comment-memory configuration", "comment memory config step should be present") 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") + assertStepOrderInSection(t, section, + "- name: Write comment-memory configuration", + "- name: Prepare comment memory files", + "- name: Process comment memory", + ) } // TestCustomJobRestoreMemoryMultipleTypes verifies that a custom job with diff --git a/pkg/workflow/compiler_pre_activation_job.go b/pkg/workflow/compiler_pre_activation_job.go index 4ce8886d125..1bd60ba561c 100644 --- a/pkg/workflow/compiler_pre_activation_job.go +++ b/pkg/workflow/compiler_pre_activation_job.go @@ -251,17 +251,20 @@ func (c *Compiler) buildPreActivationMemoryRestoreSteps(data *WorkflowData, step } if data.SafeOutputs != nil && data.SafeOutputs.CommentMemory != nil { - var commentMemorySteps strings.Builder - commentMemorySteps.WriteString(" - name: Prepare comment memory files\n") - fmt.Fprintf(&commentMemorySteps, " uses: %s\n", getCachedActionPin("actions/github-script", data)) - commentMemorySteps.WriteString(" with:\n") - fmt.Fprintf(&commentMemorySteps, " github-token: %s\n", getEffectiveSafeOutputGitHubToken(data.SafeOutputs.CommentMemory.GitHubToken)) - commentMemorySteps.WriteString(" script: |\n") - commentMemorySteps.WriteString(" const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs');\n") - commentMemorySteps.WriteString(" setupGlobals(core, github, context, exec, io, getOctokit);\n") - commentMemorySteps.WriteString(" const { main } = require('${{ runner.temp }}/gh-aw/actions/setup_comment_memory_files.cjs');\n") - commentMemorySteps.WriteString(" await main();\n") - steps = append(steps, commentMemorySteps.String()) + if configLines, ok := c.generateCommentMemoryEarlyConfigLines(data); ok { + steps = append(steps, strings.Join(configLines, "")) + var commentMemorySteps strings.Builder + commentMemorySteps.WriteString(" - name: Prepare comment memory files\n") + fmt.Fprintf(&commentMemorySteps, " uses: %s\n", getCachedActionPin("actions/github-script", data)) + commentMemorySteps.WriteString(" with:\n") + fmt.Fprintf(&commentMemorySteps, " github-token: %s\n", getEffectiveSafeOutputGitHubToken(data.SafeOutputs.CommentMemory.GitHubToken)) + commentMemorySteps.WriteString(" script: |\n") + commentMemorySteps.WriteString(" const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs');\n") + commentMemorySteps.WriteString(" setupGlobals(core, github, context, exec, io, getOctokit);\n") + commentMemorySteps.WriteString(" const { main } = require('${{ runner.temp }}/gh-aw/actions/setup_comment_memory_files.cjs');\n") + commentMemorySteps.WriteString(" await main();\n") + steps = append(steps, commentMemorySteps.String()) + } } return steps diff --git a/pkg/workflow/compiler_yaml_main_job.go b/pkg/workflow/compiler_yaml_main_job.go index 39e2c2c22de..79681631def 100644 --- a/pkg/workflow/compiler_yaml_main_job.go +++ b/pkg/workflow/compiler_yaml_main_job.go @@ -424,14 +424,29 @@ func (c *Compiler) generateActivationArtifactAndCommentMemorySteps(yaml *strings // The full safeoutputs config written by MCP setup will overwrite this file. // Returns true if the step was emitted, false if the step was skipped (e.g. handler missing). func (c *Compiler) generateCommentMemoryEarlyConfigStep(yaml *strings.Builder, data *WorkflowData) bool { + lines, ok := c.generateCommentMemoryEarlyConfigLines(data) + if !ok { + return false + } + for _, line := range lines { + yaml.WriteString(line) + } + return true +} + +// generateCommentMemoryEarlyConfigLines returns the formatted YAML lines for the +// early comment-memory config write step. This is shared by the agent job, custom +// jobs, and pre-activation memory restore so all deterministic paths can prepare +// comment-memory files before the full safe-outputs config exists. +func (c *Compiler) generateCommentMemoryEarlyConfigLines(data *WorkflowData) ([]string, bool) { builder := handlerRegistry[commentMemoryHandlerKey] if builder == nil { compilerYamlLog.Printf("Warning: %s handler not found in registry; skipping early config write", commentMemoryHandlerKey) - return false + return nil, false } cfg := builder(data.SafeOutputs) if cfg == nil { - return false + return nil, false } // INTENTIONALLY MINIMAL: this config contains only the comment_memory section and // deliberately omits workspace-path injections and checkout mappings, which are not @@ -446,24 +461,25 @@ func (c *Compiler) generateCommentMemoryEarlyConfigStep(yaml *strings.Builder, d jsonBytes, err := json.Marshal(configMap) if err != nil { compilerYamlLog.Printf("Warning: failed to marshal comment-memory config: %v", err) - return false + return nil, false } configJSON := string(jsonBytes) delimiter := GenerateHeredocDelimiterFromContent("COMMENT_MEMORY_CONFIG", configJSON) if err := ValidateHeredocContent(configJSON, delimiter); err != nil { compilerYamlLog.Printf("Warning: comment-memory config contains heredoc delimiter; skipping early config write: %v", err) - return false + return nil, false } - yaml.WriteString(" - name: Write comment-memory configuration\n") - yaml.WriteString(" run: |\n") - yaml.WriteString(" mkdir -p \"${RUNNER_TEMP}/gh-aw/safeoutputs\"\n") - fmt.Fprintf(yaml, " cat > \"${RUNNER_TEMP}/gh-aw/safeoutputs/config.json\" << '%s'\n", delimiter) + var lines []string + lines = append(lines, " - name: Write comment-memory configuration\n") + lines = append(lines, " run: |\n") + lines = append(lines, " mkdir -p \"${RUNNER_TEMP}/gh-aw/safeoutputs\"\n") + lines = append(lines, fmt.Sprintf(" cat > \"${RUNNER_TEMP}/gh-aw/safeoutputs/config.json\" << '%s'\n", delimiter)) // The 10-space YAML block-scalar indentation is stripped by the YAML parser before the // shell script is executed, so the JSON content lands at column 0 inside the heredoc. // This matches the pattern used by mcp_setup_generator.go for the full config write. - fmt.Fprintf(yaml, " %s\n", configJSON) - fmt.Fprintf(yaml, " %s\n", delimiter) - return true + lines = append(lines, fmt.Sprintf(" %s\n", configJSON)) + lines = append(lines, fmt.Sprintf(" %s\n", delimiter)) + return lines, true } // generateEngineInstallAndPreAgentSteps emits git credential configuration, the PR-ready-for-review diff --git a/pkg/workflow/on_steps_test.go b/pkg/workflow/on_steps_test.go index bfe5a11eb8c..5a180199dc7 100644 --- a/pkg/workflow/on_steps_test.go +++ b/pkg/workflow/on_steps_test.go @@ -291,18 +291,21 @@ Test on.steps with memory stores restored in pre-activation assert.Contains(t, lockContentStr, "# restore-memory: true # Restore-memory enables pre-activation memory restore") assert.Contains(t, preActivationSection, "Restore cache-memory file share data") assert.Contains(t, preActivationSection, "Clone repo-memory branch (default)") + assert.Contains(t, preActivationSection, "Write comment-memory configuration") assert.Contains(t, preActivationSection, "Prepare comment memory files") assert.Contains(t, preActivationSection, " id: gate") cacheRestoreIdx := strings.Index(preActivationSection, "Restore cache-memory file share data") repoRestoreIdx := strings.Index(preActivationSection, "Clone repo-memory branch (default)") + commentMemoryConfigIdx := strings.Index(preActivationSection, "Write comment-memory configuration") commentMemoryIdx := strings.Index(preActivationSection, "Prepare comment memory files") gateStepIdx := strings.Index(preActivationSection, " id: gate") - if cacheRestoreIdx == -1 || repoRestoreIdx == -1 || commentMemoryIdx == -1 || gateStepIdx == -1 { + if cacheRestoreIdx == -1 || repoRestoreIdx == -1 || commentMemoryConfigIdx == -1 || commentMemoryIdx == -1 || gateStepIdx == -1 { t.Fatalf("Expected memory restore steps and gate step in pre_activation section. Section:\n%s", preActivationSection) } assert.Less(t, cacheRestoreIdx, gateStepIdx, "cache-memory should be restored before on.steps") assert.Less(t, repoRestoreIdx, gateStepIdx, "repo-memory should be restored before on.steps") + assert.Less(t, commentMemoryConfigIdx, commentMemoryIdx, "comment-memory config should be written before restoring comment-memory files") assert.Less(t, commentMemoryIdx, gateStepIdx, "comment-memory should be restored before on.steps") // Pre-activation must not include write-back/commit steps.