Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions docs/adr/44037-restore-memory-read-only-access-custom-jobs.md
Original file line number Diff line number Diff line change
@@ -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 `<id>`: `/tmp/gh-aw/cache-memory-<id>`

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.*
4 changes: 4 additions & 0 deletions pkg/parser/schemas/main_workflow_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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."
}
}
}
Expand Down
68 changes: 34 additions & 34 deletions pkg/workflow/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
184 changes: 184 additions & 0 deletions pkg/workflow/compiler_custom_job_memory.go
Original file line number Diff line number Diff line change
@@ -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
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.


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.
Comment on lines +102 to +106

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

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
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in e413231: generateRepoMemoryRestoreLines now uses strings.SplitAfter(raw, "\n") and skips empty elements, so no phantom blank line is injected.

// 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
}
Loading
Loading