Add restore-memory step injection for custom jobs#44037
Conversation
- 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>
|
@copilot make it simpler and only support restore-memory: true/false |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Test Quality Sentinel completed test quality analysis. |
There was a problem hiding this comment.
Pull request overview
This PR adds a jobs.<job-id>.restore-memory configuration to let custom (non-agent) jobs restore read-only memory stores (cache-memory, repo-memory, comment-memory) by injecting the appropriate restore/clone/prepare steps into those jobs, enabling orchestrator-style workflows that can read shared state before the agent job runs.
Changes:
- Inject restore-memory step sequences into custom jobs during compilation, positioned after GH_HOST setup and before pre-steps/steps.
- Introduce a dedicated compiler implementation for parsing/validating
restore-memoryand generating restore steps for each memory type. - Extend the workflow JSON schema to allow the new
restore-memoryfield on custom jobs.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/compiler_custom_jobs.go | Parses/validates restore-memory and injects restore steps into custom job step lists. |
| pkg/workflow/compiler_custom_job_memory.go | Implements restore-memory config extraction/validation and step generation for cache/repo/comment memory. |
| pkg/workflow/compiler_custom_job_memory_test.go | Adds unit tests verifying step injection, ordering, and read-only behavior for custom jobs. |
| pkg/parser/schemas/main_workflow_schema.json | Adds restore-memory to the job schema with boolean flags for each supported memory type. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 4
- Review effort level: Low
| // 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...) | ||
| } |
There was a problem hiding this comment.
Fixed in e413231: GH_AW_WORKFLOW_ID_SANITIZED is now injected into the custom job's env when cache-memory restore is requested, without overwriting user-provided values. The sanitised value is derived from data.WorkflowID via SanitizeWorkflowIDForCacheKey, mirroring the main job and update-cache-memory job behaviour.
| 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 | ||
| } | ||
| } |
| 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. |
There was a problem hiding this comment.
The ADR (docs/adr/44037-restore-memory-read-only-access-custom-jobs.md) now shows the correct canonical paths (/tmp/gh-aw/cache-memory for the default cache, /tmp/gh-aw/cache-memory-<id> for named caches) and the updated restore-memory: true boolean API (committed in 1fe026c). The PR description is stale from the original code generation and reflects the pre-simplification per-type map API; the ADR is the authoritative documentation going forward.
| "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": { |
… custom jobs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (882 new lines in 📄 Draft ADR committed:
📋 What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. ❓ Why ADRs MatterADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
|
🧪 Test Quality Sentinel Report
📊 Metrics (12 tests)
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: Add restore-memory step injection for custom jobs
The feature design is sound — isolating restore-only memory access in custom/orchestrator jobs is a clean pattern and the schema + validation structure is good. Three blocking issues found.
Issues (3)
1. Double trailing newline in generateRepoMemoryRestoreLines (line 194) — Correctness
strings.Split(raw, "\n") on a newline-terminated string appends an empty string at the end. Adding "\n" to each part emits an extra blank "\n" element, introducing a spurious empty line between repo-memory steps and the steps that follow. Fix: use strings.SplitAfter and skip empty parts.
2. Setup action silently skipped while memory steps still execute (line 124) — Reliability
When setupActionRef == "" and !c.actionMode.IsScript(), setup lines are omitted but generateRepoMemoryRestoreLines/generateCommentMemoryRestoreLines are still appended. Both of those scripts (clone_repo_memory_branch.sh, setup_comment_memory_files.cjs) are installed by the setup action, so the compiled workflow will fail at runtime. Should be a compile-time error or the memory lines must be conditionally skipped alongside the setup lines.
3. Silent no-op for non-bool values in extractRestoreMemoryConfig (line 79) — Reliability
If cache-memory: "true" (string) is passed, the v.(bool) assertion fails silently, the field stays false, and no restore steps are injected — no error, no warning. The schema validates this at parse time, but the Go-level guard should also be defensive and return an explicit error for non-bool values.
References:
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 97.3 AIC · ⌖ 6.41 AIC · ⊞ 4.8K
| } | ||
|
|
||
| // Split into individual lines, preserving their trailing newlines. | ||
| parts := strings.Split(raw, "\n") |
There was a problem hiding this comment.
Bug: double trailing newline in generateRepoMemoryRestoreLines
strings.Split(raw, "\n") on a newline-terminated string always produces an empty string as the last element. Adding "\n" to every part then emits an extra blank "\n" line at the end, injecting a spurious empty line between the repo-memory steps and whatever follows (pre-steps, regular steps).
Fix: skip the trailing empty element, or use strings.SplitAfter which preserves newlines without adding a phantom one:
// Using SplitAfter — each element keeps its trailing newline; no phantom entry.
parts := strings.SplitAfter(raw, "\n")
lines := make([]string, 0, len(parts))
for _, p := range parts {
if p != "" {
lines = append(lines, p)
}
}@copilot please address this.
There was a problem hiding this comment.
Fixed in e413231: generateRepoMemoryRestoreLines now uses strings.SplitAfter(raw, "\n") and skips empty parts, eliminating the spurious trailing blank line.
| var githubConfig *GitHubToolConfig | ||
| if data.ParsedTools != nil { | ||
| githubConfig = data.ParsedTools.GitHub | ||
| } |
There was a problem hiding this comment.
Silent skip of setup step leaves repo-memory/comment-memory broken at runtime
When setupActionRef == "" and !c.actionMode.IsScript(), the if guard exits without appending any setup lines — but the generateRepoMemoryRestoreLines / generateCommentMemoryRestoreLines steps are still emitted unconditionally in the block below. Both of those steps call scripts installed by the setup action (clone_repo_memory_branch.sh, setup_comment_memory_files.cjs). If the setup action is not injected, those scripts will not exist on the runner and the steps will fail at runtime.
Other callers (e.g. compiler_safe_outputs_job.go:124) follow the same guard and also skip their memory steps when the setup action is absent, or they log the situation. This site should either (a) also skip the memory lines, or (b) treat the missing setup action as a compilation error:
if setupActionRef == "" && !c.actionMode.IsScript() {
return nil, fmt.Errorf("jobs.%s.restore-memory: repo-memory/comment-memory requires the setup action; no action ref found", jobName)
}@copilot please address this.
There was a problem hiding this comment.
Fixed in e413231: buildRestoreMemorySteps now returns a compile-time error when resolveActionReference returns "" and !c.actionMode.IsScript(). The method signature was updated to (setupLines, memoryLines []string, err error) and the call site propagates the error.
| // - 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) { |
There was a problem hiding this comment.
Silent no-op for non-bool memory values — should return a validation error
If the user writes cache-memory: "true" or any non-boolean YAML value, the v.(bool) type assertion fails silently and the field is ignored (left false). The schema says "type": "boolean" so the YAML parser may already reject this, but at the Go level this pattern allows a misconfigured workflow to compile without any indication that the requested memory restore was skipped.
Consider returning an error for unexpected non-bool values to give the user an actionable message:
if v, ok := rmMap["cache-memory"]; ok {
switch b := v.(type) {
case bool:
cfg.CacheMemory = b
default:
return nil, fmt.Errorf("jobs.%s.restore-memory.cache-memory must be a boolean, got %T", jobName, v)
}
}@copilot please address this.
There was a problem hiding this comment.
Superseded by the API simplification: restore-memory now accepts only a top-level bool. The extractRestoreMemoryConfig function already returns fmt.Errorf("jobs.%s.restore-memory must be a boolean (true or false)", jobName) for any non-bool value. The per-type rmMap no longer exists, so silent no-ops for sub-keys are not possible.
There was a problem hiding this comment.
Skills-Based Review
Applied /tdd, /codebase-design, and /diagnosing-bugs — requesting changes on three correctness issues.
Key Themes & Highlights
Issues (3 actionable)
-
Silent no-op on missing setup action (
buildRestoreMemorySteps, line 88) — whenresolveActionReferencereturns""andactionMode.IsScript()is false,setupLinesis empty butrepo-memory/comment-memorysteps are still emitted, referencing scripts that were never installed. This will fail at runtime with no compile-time warning. -
Non-bool values silently ignored (
extractRestoreMemoryConfig, lines 32-46) —cache-memory: 1orcache-memory: yescauses the field to be silently dropped instead of producing an error. Users will see the restore step simply absent with no feedback. -
Duplicate restore-key construction logic (lines 147-167 vs
cache.go~529-560) — the restore-key derivation is copy-pasted. A new scope type or key-format change must be applied in both places.
Minor issues (non-blocking)
generateRepoMemoryRestoreLines(strings.Split+ re-append"\n") produces a trailing empty line — usestrings.SplitAfterand filter empty parts.- No nil-data tests for
generateRepoMemoryRestoreLines/generateCommentMemoryRestoreLines(onlygenerateCacheMemoryRestoreLineshas one). TestCustomJobRestoreMemoryStandaloneJobshould assertsteps:is present in the generated section.
Positive Highlights
- Clean separation:
extractRestoreMemoryConfig→validateRestoreMemoryConfig→buildRestoreMemoryStepsis a well-structured pipeline. - Validation correctly ties
restore-memorykeys to existingtools:entries; the error messages are clear and include the job name. - Comprehensive integration tests covering step ordering, write-back exclusion, and error paths.
generateRepoMemoryRestoreLinesreusesgenerateRepoMemoryStepsrather than duplicating it — good module reuse.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 112.8 AIC · ⌖ 6.07 AIC · ⊞ 6.6K
Comment /matt to run again
| // 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() { |
There was a problem hiding this comment.
[/diagnosing-bugs] Silent no-op when resolveActionReference returns "" and actionMode.IsScript() is false — setupLines is empty but memoryLines for repo-memory/comment-memory are still emitted, causing the injected steps to reference scripts (clone_repo_memory_branch.sh, setup_comment_memory_files.cjs) that were never installed.
Suggestion
Either surface an error or short-circuit all memory lines when setup cannot be satisfied:
if cfg.RepoMemory || cfg.CommentMemory {
setupActionRef := c.resolveActionReference("./actions/setup", data)
if setupActionRef == "" && !c.actionMode.IsScript() {
// Scripts unavailable; skip injecting dependent memory lines.
return nil, nil
}
setupLines = append(setupLines, c.generateCheckoutActionsFolder(data)...)
setupLines = append(setupLines, c.generateSetupStep(data, setupActionRef, SetupActionDestination, false, "", "")...)
}A unit test covering this code path would prevent silent regressions.
@copilot please address this.
There was a problem hiding this comment.
Fixed in e413231: raised to a compile-time error. When resolveActionReference("./actions/setup", data) returns "" and !c.actionMode.IsScript(), buildRestoreMemorySteps now returns an error message identifying the job and the requirement.
| cfg := &restoreMemoryConfig{} | ||
|
|
||
| if v, ok := rmMap["cache-memory"]; ok { | ||
| if b, ok := v.(bool); ok { |
There was a problem hiding this comment.
[/tdd] Non-bool values for cache-memory, repo-memory, or comment-memory are silently dropped — there's no error returned when a user writes cache-memory: 1 or cache-memory: yes (which YAML parsers may resolve to non-bool types). The value is simply ignored, leading to confusing behaviour where the requested restore never appears.
Suggestion
Return an error on type mismatch rather than silently ignoring the value:
if v, ok := rmMap["cache-memory"]; ok {
b, ok := v.(bool)
if !ok {
return nil, fmt.Errorf("jobs.%s.restore-memory.cache-memory: expected bool, got %T", jobName, v)
}
cfg.CacheMemory = b
}Add a test case such as cache-memory: 1 to TestExtractRestoreMemoryConfig to document the expectation.
@copilot please address this.
| } else { | ||
| keyParts := strings.Split(cacheKey, "-") | ||
| if len(keyParts) >= 2 { | ||
| restoreKeys = append(restoreKeys, strings.Join(keyParts[:len(keyParts)-1], "-")+"-") |
There was a problem hiding this comment.
[/codebase-design] The restore-key construction logic here is a near-duplicate of the same logic in cache.go (lines ~529-560). Any future change to restore-key semantics (e.g. adding a new scope type) must now be applied in two places.
Suggestion
Extract the restore-key derivation into a shared helper, for example buildCacheRestoreKeys(cacheKey string, scope string) []string, and call it from both generateCacheMemoryRestoreLines and the existing generateCacheMemorySteps in cache.go.
This removes the duplication and makes the parallel structure explicit.
@copilot please address this.
There was a problem hiding this comment.
Fixed in e413231: extracted buildCacheRestoreKeys(cacheKey, scope string) []string into cache.go and replaced both the inline logic in generateCacheMemorySteps (cache.go) and the duplicate in generateCacheMemoryRestoreLines (compiler_custom_job_memory.go) with calls to this shared helper.
| if raw == "" { | ||
| return nil | ||
| } | ||
|
|
There was a problem hiding this comment.
[/codebase-design] generateRepoMemoryRestoreLines splits a strings.Builder output on "\n" and re-appends "\n" to every part, which adds a spurious trailing "\n" for the last element (the final strings.Split result for "...\n" will have an empty string at the end, becoming "\n"). This produces a blank extra line appended to the step block.
Suggestion
parts := strings.SplitAfter(raw, "\n") // keeps the newline on each part
lines := make([]string, 0, len(parts))
for _, p := range parts {
if p != "" {
lines = append(lines, p)
}
}Or test that the generated YAML for a repo-memory custom job has no duplicate blank lines.
@copilot please address this.
There was a problem hiding this comment.
Fixed in e413231: generateRepoMemoryRestoreLines now uses strings.SplitAfter(raw, "\n") and skips empty elements, so no phantom blank line is injected.
| } | ||
|
|
||
| // TestGenerateCacheMemoryRestoreLinesNilData verifies graceful handling of nil config. | ||
| func TestGenerateCacheMemoryRestoreLinesNilData(t *testing.T) { |
There was a problem hiding this comment.
[/tdd] The only nil/empty-data test is TestGenerateCacheMemoryRestoreLinesNilData — there are no equivalent nil-data tests for generateRepoMemoryRestoreLines or generateCommentMemoryRestoreLines. A nil RepoMemoryConfig or SafeOutputs should also return nil without panicking.
Suggestion
func TestGenerateRepoMemoryRestoreLinesNilData(t *testing.T) {
assert.Nil(t, generateRepoMemoryRestoreLines(&WorkflowData{}))
}
func TestGenerateCommentMemoryRestoreLinesNilData(t *testing.T) {
assert.Nil(t, generateCommentMemoryRestoreLines(&WorkflowData{}))
assert.Nil(t, generateCommentMemoryRestoreLines(&WorkflowData{
SafeOutputs: &SafeOutputsConfig{},
}))
}@copilot please address this.
There was a problem hiding this comment.
Added in e413231: TestGenerateRepoMemoryRestoreLinesNilData and TestGenerateCommentMemoryRestoreLinesNilData (with both nil SafeOutputs and non-nil but empty SafeOutputs) are now in compiler_custom_job_memory_test.go.
|
|
||
| testFile := filepath.Join(tmpDir, "test.md") | ||
| require.NoError(t, os.WriteFile(testFile, []byte(frontmatter), 0644)) | ||
|
|
There was a problem hiding this comment.
[/tdd] TestCustomJobRestoreMemoryStandaloneJob confirms that the job section is present and contains Restore cache-memory, but does not assert that the generated YAML is valid (e.g. that the steps: key appears and is non-empty). A workflow with only restore-memory: and no steps: is the exact edge case the PR description highlights; the test should parse the YAML or at minimum assert steps: is present in the section.
Suggestion
assert.Contains(t, section, "steps:", "section must have a steps key even with no explicit steps")@copilot please address this.
There was a problem hiding this comment.
Added in e413231: TestCustomJobRestoreMemoryStandaloneJob now includes assert.Contains(t, section, "steps:", "steps key must be present even with no explicit steps").
There was a problem hiding this comment.
REQUEST_CHANGES — three correctness bugs must be fixed before merge; one is a silent data-loss vector and one produces invalid YAML.
### Blocking issues (3)
-
Setup step silently skipped for script-dependent memory types (
compiler_custom_job_memory.goline 122–128): WhenresolveActionReferencereturns""andactionMode.IsScript()is false, the gh-aw setup step is omitted, butrepo-memoryandcomment-memoryrestore lines are still emitted — referencing scripts that only exist after setup runs. The compiled workflow will always fail at runtime in this configuration. Fix: add a compile-time error invalidateRestoreMemoryConfig(or a new validator) when these types are requested but setup is unresolvable. -
Empty
restore-keysblock emits invalid YAML (compiler_custom_job_memory.goline 205–208):restore-keys: |is emitted unconditionally even when therestoreKeysslice is empty — a literal block scalar with no content lines is invalid YAML. Wrap the block inif len(restoreKeys) > 0. -
Non-boolean field values silently ignored (
compiler_custom_job_memory.golines 69–81):cache-memory: 1orcache-memory: "yes"silently resolves tofalse, compile succeeds, and no restore steps are emitted. This is a silent misconfiguration. Add explicit type-mismatch errors.
### Non-blocking observations (1)
- Double-newline in
generateRepoMemoryRestoreLines(compiler_custom_job_memory.goline 229–235): trailing"\n"fromgenerateRepoMemoryStepscausesstrings.Splitto produce a final empty element, which is then re-appended with"\n"— inserting a spurious blank line after every generated step block. Not a YAML validity issue in practice (most YAML parsers tolerate blank lines), but it breaks any exact-match assertions on the generated output.
🔎 Code quality review by PR Code Quality Reviewer · 237.9 AIC · ⌖ 8.74 AIC · ⊞ 5.4K
Comment /review to run again
Comments that could not be inline-anchored
pkg/workflow/compiler_custom_job_memory.go:127
Setup step silently omitted while script-dependent memory restore lines are still emitted — guaranteed runtime failure. When resolveActionReference returns "" and c.actionMode.IsScript() is false, the entire setupLines block is skipped. But generateCommentMemoryRestoreLines and generateRepoMemoryRestoreLines always emit steps that reference ${{ runner.temp }}/gh-aw/actions/setup_globals.cjs, setup_comment_memory_files.cjs, and clone_repo_memory_branch.sh. Those scripts o…
pkg/workflow/compiler_custom_job_memory.go:234
Double-newline corruption in generated YAML. generateRepoMemoryRestoreLines splits the output of generateRepoMemorySteps on "\n" and then re-appends "\n" to every part. If the original string ends with "\n" (which all well-formed YAML emitters do), strings.Split produces a trailing empty string "", and the loop appends "\n" to it — inserting a blank line after every step block. For N steps this adds N spurious blank lines, and the final line of the last step gets an extra ne…
pkg/workflow/compiler_custom_job_memory.go:192
Restore key logic silently produces empty restore-keys block for most cache key formats. The two branches that build restoreKeys both have narrow guards: the runIDSuffix branch strips ${{ github.run_id }} only when the key literally ends with that suffix; the keyParts branch requires len(keyParts) >= 2. When neither condition holds — e.g., a key with no - separator — restoreKeys stays empty, yet the code still unconditionally emits restore-keys: | with no keys underneath. …
pkg/workflow/compiler_custom_job_memory.go:75
Non-boolean values (e.g. cache-memory: 1) are silently ignored instead of producing a validation error. The three type assertions v.(bool) are guarded by an inner ok check — when a user writes cache-memory: 1 or cache-memory: "yes", the value is silently discarded and the field stays false. Combined with the all-false → nil early-return, this means a misconfigured restore-memory block compiles without error but produces no restore steps, giving the user no feedback.
<details>…
pkg/workflow/compiler_custom_jobs.go:902
hasRestoreMemory alone forces full step-scaffolding even when the job has zero user-defined steps, which changes the semantics for existing restore-memory-only jobs. Before this PR the condition hasSetupStepsField || hasPreStepsField || hasStepsField allowed a no-steps custom job to exist with no steps at all. Adding || hasRestoreMemory means a job with only restore-memory: and no steps: now gets the GHES config step plus memory steps injected — which is the intended behaviour p…
pkg/workflow/compiler_custom_job_memory.go:223
generateRepoMemoryRestoreLines delegates to the shared agent-job builder with no structural write-back fence. generateRepoMemorySteps is today read-only, but nothing prevents a future contributor from adding push/commit steps to it (e.g. an auto-sync feature). When that happens, the new steps will silently appear in every restore-memory custom job without any compile warning, violating the stated invariant that restore-memory never emits write-back steps. The invariant is currently en…
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot please run the
|
- 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>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
…paths Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
All unresolved review threads have been addressed in commits
|
|
@copilot please run the
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
All unresolved threads have been addressed. One remaining code-correctness issue was found and fixed: Empty
|
Memory stores (
cache-memory,repo-memory,comment-memory) were exclusively available in the agent job, making orchestrator patterns impossible — e.g. a scheduled job that readscache-memoryto build a dispatch list before the agent runs.Changes
New
restore-memoryfield forjobs.<job-id>Compiler (
pkg/workflow/compiler_custom_jobs.go, newcompiler_custom_job_memory.go)extractRestoreMemoryConfig— parses therestore-memorymap from custom job configvalidateRestoreMemoryConfig— rejects if the requested memory type is absent fromtools:; write-back is architecturally impossible (only restore steps are ever emitted)buildRestoreMemorySteps— returns setup lines (gh-aw setup action, needed for repo/comment scripts) and memory restore lines; injected after GHES host config, beforepre-steps/stepsmkdir -p+actions/cache/restoreonly — noactions/cache, no git integrity setup, no artifact uploadgenerateRepoMemorySteps(already read-only)Prepare comment memory filesstep used in the agent jobSchema (
pkg/parser/schemas/main_workflow_schema.json)Added
restore-memoryobject property (withcache-memory,repo-memory,comment-memoryboolean keys) to the custom jobadditionalPropertiesdefinition.