diff --git a/.github/workflows/grumpy-reviewer.lock.yml b/.github/workflows/grumpy-reviewer.lock.yml index f0767b939c0..235620f694d 100644 --- a/.github/workflows/grumpy-reviewer.lock.yml +++ b/.github/workflows/grumpy-reviewer.lock.yml @@ -742,7 +742,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/mattpocock-skills-reviewer.lock.yml b/.github/workflows/mattpocock-skills-reviewer.lock.yml index 630293ffdf3..700ecf36ac1 100644 --- a/.github/workflows/mattpocock-skills-reviewer.lock.yml +++ b/.github/workflows/mattpocock-skills-reviewer.lock.yml @@ -772,7 +772,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/pr-code-quality-reviewer.lock.yml b/.github/workflows/pr-code-quality-reviewer.lock.yml index c54b67124fe..d45a2313870 100644 --- a/.github/workflows/pr-code-quality-reviewer.lock.yml +++ b/.github/workflows/pr-code-quality-reviewer.lock.yml @@ -736,7 +736,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/pr-nitpick-reviewer.lock.yml b/.github/workflows/pr-nitpick-reviewer.lock.yml index 8c2646e811d..6130f10bf6c 100644 --- a/.github/workflows/pr-nitpick-reviewer.lock.yml +++ b/.github/workflows/pr-nitpick-reviewer.lock.yml @@ -771,7 +771,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/pr-triage-agent.lock.yml b/.github/workflows/pr-triage-agent.lock.yml index a5f1b83fdef..36618b1e0ce 100644 --- a/.github/workflows/pr-triage-agent.lock.yml +++ b/.github/workflows/pr-triage-agent.lock.yml @@ -790,7 +790,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/refiner.lock.yml b/.github/workflows/refiner.lock.yml index e316b50c6a2..b9c4c472934 100644 --- a/.github/workflows/refiner.lock.yml +++ b/.github/workflows/refiner.lock.yml @@ -784,7 +784,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/security-review.lock.yml b/.github/workflows/security-review.lock.yml index cc0487ee1b3..813428b8085 100644 --- a/.github/workflows/security-review.lock.yml +++ b/.github/workflows/security-review.lock.yml @@ -802,7 +802,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index 09e2304f27e..0b49593d1c6 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -1252,7 +1252,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/smoke-copilot-aoai-apikey.lock.yml b/.github/workflows/smoke-copilot-aoai-apikey.lock.yml index 05fca4ab840..90d951b7862 100644 --- a/.github/workflows/smoke-copilot-aoai-apikey.lock.yml +++ b/.github/workflows/smoke-copilot-aoai-apikey.lock.yml @@ -1168,7 +1168,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/smoke-copilot-aoai-entra.lock.yml b/.github/workflows/smoke-copilot-aoai-entra.lock.yml index b956d54065f..2dd7ce15d8b 100644 --- a/.github/workflows/smoke-copilot-aoai-entra.lock.yml +++ b/.github/workflows/smoke-copilot-aoai-entra.lock.yml @@ -1169,7 +1169,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/smoke-copilot-arm.lock.yml b/.github/workflows/smoke-copilot-arm.lock.yml index a84512ac19f..838dc591aa4 100644 --- a/.github/workflows/smoke-copilot-arm.lock.yml +++ b/.github/workflows/smoke-copilot-arm.lock.yml @@ -1037,7 +1037,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/smoke-copilot.lock.yml b/.github/workflows/smoke-copilot.lock.yml index 6ec9501f68a..568e58f5714 100644 --- a/.github/workflows/smoke-copilot.lock.yml +++ b/.github/workflows/smoke-copilot.lock.yml @@ -1174,7 +1174,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/test-quality-sentinel.lock.yml b/.github/workflows/test-quality-sentinel.lock.yml index 416ffad3543..c87faeb4013 100644 --- a/.github/workflows/test-quality-sentinel.lock.yml +++ b/.github/workflows/test-quality-sentinel.lock.yml @@ -731,7 +731,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/pkg/cli/update_command.go b/pkg/cli/update_command.go index 3d76f23a090..f71de72127f 100644 --- a/pkg/cli/update_command.go +++ b/pkg/cli/update_command.go @@ -175,13 +175,6 @@ func RunUpdateWorkflows(ctx context.Context, opts UpdateWorkflowsOptions) error fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Warning: Failed to update actions-lock.json: %v", err))) } - // Resolve and store SHA-256 digest pins for container images referenced in lock files. - updateLog.Print("Updating container image digest pins") - if err := UpdateContainerPins(ctx, opts.WorkflowsDir, opts.Verbose); err != nil { - // Non-fatal: Docker may not be available in all environments. - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Warning: Failed to update container pins: %v", err))) - } - // Update action references in user-provided steps within workflow .md files. // By default all org/repo@version references are updated to the latest major version. updateLog.Print("Updating action references in workflow .md files") @@ -190,6 +183,16 @@ func RunUpdateWorkflows(ctx context.Context, opts UpdateWorkflowsOptions) error fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Warning: Failed to update action references in workflow files: %v", err))) } + // Resolve and store SHA-256 digest pins for container images referenced in lock files. + // This runs after compilation (via UpdateActionsInWorkflowFiles) so that the lock files + // already reflect the current AWF version; stale pins from superseded versions are pruned + // and new versions are resolved in a single pass. + updateLog.Print("Updating container image digest pins") + if err := UpdateContainerPins(ctx, opts.WorkflowsDir, opts.Verbose); err != nil { + // Non-fatal: Docker may not be available in all environments. + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Warning: Failed to update container pins: %v", err))) + } + updateLog.Printf("Update process complete: had_error=%v", firstErr != nil) return firstErr } diff --git a/pkg/cli/update_container_pins.go b/pkg/cli/update_container_pins.go index df09b3f51fd..f66c12ccecb 100644 --- a/pkg/cli/update_container_pins.go +++ b/pkg/cli/update_container_pins.go @@ -88,6 +88,21 @@ func UpdateContainerPins(ctx context.Context, workflowDir string, verbose bool) } } + // Build a set of images currently referenced in the compiled lock files so + // that stale entries (e.g. superseded AWF versions) can be pruned. + imageSet := make(map[string]bool, len(images)) + for _, img := range images { + imageSet[img] = true + } + + // Remove any container pin entries that are no longer referenced by the + // compiled lock files. This keeps actions-lock.json consistent with what + // compile actually emits and prevents stale version accumulation. + prunedCount := actionCache.PruneStaleContainerPins(imageSet) + if prunedCount > 0 { + containerPinsLog.Printf("Pruned %d stale container pin(s) from actions-lock.json", prunedCount) + } + // Resolve digests for images that are not yet pinned. type pinnedEntry struct { image string @@ -141,6 +156,11 @@ func UpdateContainerPins(ctx context.Context, workflowDir string, verbose bool) fmt.Fprintln(os.Stderr, "") } + if prunedCount > 0 { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Pruned %d stale container pin(s) from actions-lock.json", prunedCount))) + fmt.Fprintln(os.Stderr, "") + } + if len(skippedImages) > 0 && verbose { fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("%d container image(s) already up to date", len(skippedImages)))) fmt.Fprintln(os.Stderr, "") @@ -154,7 +174,7 @@ func UpdateContainerPins(ctx context.Context, workflowDir string, verbose bool) fmt.Fprintln(os.Stderr, "") } - if len(updatedImages) > 0 { + if len(updatedImages) > 0 || prunedCount > 0 { if err := actionCache.Save(); err != nil { return fmt.Errorf("failed to save actions-lock.json: %w", err) } diff --git a/pkg/cli/update_container_pins_test.go b/pkg/cli/update_container_pins_test.go index 48f6132f936..12183d15f2e 100644 --- a/pkg/cli/update_container_pins_test.go +++ b/pkg/cli/update_container_pins_test.go @@ -7,6 +7,7 @@ import ( "path/filepath" "testing" + "github.com/github/gh-aw/pkg/workflow" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -168,3 +169,75 @@ Manifests: }) } } + +// TestUpdateContainerPins_PrunesStaleEntries verifies that UpdateContainerPins +// removes container pin entries from actions-lock.json that are no longer +// referenced in the compiled lock files (e.g. superseded AWF versions). +func TestUpdateContainerPins_PrunesStaleEntries(t *testing.T) { + // Create a temp directory acting as the repo root. + tmpDir := t.TempDir() + + // Write an actions-lock.json with a stale container pin and a live one. + // The live pin (0.27.2) should be kept; the stale one (0.27.0) should be pruned. + actionsLockDir := filepath.Join(tmpDir, ".github", "aw") + require.NoError(t, os.MkdirAll(actionsLockDir, 0755)) + actionsLockContent := `{ + "entries": {}, + "containers": { + "ghcr.io/github/gh-aw-firewall/agent:0.27.0": { + "image": "ghcr.io/github/gh-aw-firewall/agent:0.27.0", + "digest": "sha256:olddigest0000000000000000000000000000000000000000000000000000000", + "pinned_image": "ghcr.io/github/gh-aw-firewall/agent:0.27.0@sha256:olddigest0000000000000000000000000000000000000000000000000000000" + }, + "ghcr.io/github/gh-aw-firewall/agent:0.27.2": { + "image": "ghcr.io/github/gh-aw-firewall/agent:0.27.2", + "digest": "sha256:newdigest0000000000000000000000000000000000000000000000000000000", + "pinned_image": "ghcr.io/github/gh-aw-firewall/agent:0.27.2@sha256:newdigest0000000000000000000000000000000000000000000000000000000" + } + } +} +` + actionsLockPath := filepath.Join(actionsLockDir, "actions-lock.json") + require.NoError(t, os.WriteFile(actionsLockPath, []byte(actionsLockContent), 0644)) + + // Write a lock file referencing the NEW AWF version (0.27.2), not the old one. + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + lockFileContent := `name: test +jobs: + setup: + steps: + - name: Download container images + run: bash "${RUNNER_TEMP}/gh-aw/actions/download_docker_images.sh" ghcr.io/github/gh-aw-firewall/agent:0.27.2 +` + require.NoError(t, os.WriteFile(filepath.Join(workflowsDir, "my-workflow.lock.yml"), []byte(lockFileContent), 0644)) + + // collectImagesFromLockFiles should find the new version only. + images, err := collectImagesFromLockFiles(workflowsDir) + require.NoError(t, err) + assert.Equal(t, []string{"ghcr.io/github/gh-aw-firewall/agent:0.27.2"}, images) + + // Load the cache and exercise PruneStaleContainerPins directly (Docker is not + // available in unit tests so we can't call the full UpdateContainerPins function). + cache := workflow.NewActionCache(tmpDir) + require.NoError(t, cache.Load()) + + imageSet := map[string]bool{"ghcr.io/github/gh-aw-firewall/agent:0.27.2": true} + pruned := cache.PruneStaleContainerPins(imageSet) + assert.Equal(t, 1, pruned, "stale 0.27.0 entry should be pruned") + + _, ok := cache.GetContainerPin("ghcr.io/github/gh-aw-firewall/agent:0.27.0") + assert.False(t, ok, "old-version pin should not be in cache after prune") + + pin, ok := cache.GetContainerPin("ghcr.io/github/gh-aw-firewall/agent:0.27.2") + require.True(t, ok, "current-version pin should still be in cache") + assert.Equal(t, "sha256:newdigest0000000000000000000000000000000000000000000000000000000", pin.Digest) + + // Save and verify the stale entry is gone from disk. + require.NoError(t, cache.Save()) + + data, err := os.ReadFile(actionsLockPath) + require.NoError(t, err) + assert.NotContains(t, string(data), "0.27.0", "stale version should not appear in saved file") + assert.Contains(t, string(data), "0.27.2", "current version should be in saved file") +} diff --git a/pkg/cli/upgrade_command.go b/pkg/cli/upgrade_command.go index 959c9094c15..11bd0f5c710 100644 --- a/pkg/cli/upgrade_command.go +++ b/pkg/cli/upgrade_command.go @@ -282,21 +282,6 @@ func runUpgradeCommand(opts upgradeOptions) error { } } - // Step 3b: Update container image digest pins (unless --no-fix or --no-actions is specified) - // Container pins are stored alongside action pins in .github/aw/actions-lock.json. - // Running this before compilation means the next compile step will embed the - // pinned @sha256: references in the generated lock files. - if !opts.noFix && !opts.noActions { - upgradeLog.Print("Updating container image digest pins") - if err := UpdateContainerPins(opts.ctx, opts.workflowDir, opts.verbose); err != nil { - upgradeLog.Printf("Failed to update container pins: %v", err) - // Non-critical — Docker may not be available in all environments. - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Warning: Failed to update container pins: %v", err))) - } else if opts.verbose { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("✓ Updated container image pins")) - } - } - // Step 4: Compile all workflows (unless --no-fix or --no-compile is specified) if !opts.noFix && !opts.noCompile { fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Compiling all workflows...")) @@ -344,6 +329,24 @@ func runUpgradeCommand(opts upgradeOptions) error { } } + // Step 4b: Update container image digest pins (unless --no-fix or --no-actions is specified) + // Container pins are stored alongside action pins in .github/aw/actions-lock.json. + // This runs AFTER compilation so that the compiled lock files already reflect the + // current AWF version; stale pins from superseded versions are pruned and new + // versions are resolved in a single pass. When --no-compile is set, the existing + // lock files are used as-is — pins are still pruned and refreshed against whatever + // lock files are currently on disk. + if !opts.noFix && !opts.noActions { + upgradeLog.Print("Updating container image digest pins") + if err := UpdateContainerPins(opts.ctx, opts.workflowDir, opts.verbose); err != nil { + upgradeLog.Printf("Failed to update container pins: %v", err) + // Non-critical — Docker may not be available in all environments. + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Warning: Failed to update container pins: %v", err))) + } else if opts.verbose { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("✓ Updated container image pins")) + } + } + // Print success message fmt.Fprintln(os.Stderr, "") fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Upgrade complete")) diff --git a/pkg/workflow/action_cache.go b/pkg/workflow/action_cache.go index afe32d415de..800edf44f98 100644 --- a/pkg/workflow/action_cache.go +++ b/pkg/workflow/action_cache.go @@ -96,6 +96,26 @@ func (c *ActionCache) DeleteContainerPin(image string) { } } +// PruneStaleContainerPins removes container pin entries whose keys are not present +// in knownImages. It returns the number of entries that were removed. +// This is used to keep actions-lock.json consistent with the set of images +// actually referenced by the compiled lock files. +func (c *ActionCache) PruneStaleContainerPins(knownImages map[string]bool) int { + if c.ContainerPins == nil { + return 0 + } + pruned := 0 + for image := range c.ContainerPins { + if !knownImages[image] { + delete(c.ContainerPins, image) + c.dirty = true + pruned++ + actionCacheLog.Printf("Pruned stale container pin for image=%s", image) + } + } + return pruned +} + // Load loads the cache from disk func (c *ActionCache) Load() error { actionCacheLog.Printf("Loading action cache from: %s", c.path) diff --git a/pkg/workflow/action_cache_container_pin_test.go b/pkg/workflow/action_cache_container_pin_test.go index c305b8139b5..dbac363966c 100644 --- a/pkg/workflow/action_cache_container_pin_test.go +++ b/pkg/workflow/action_cache_container_pin_test.go @@ -138,3 +138,74 @@ func TestContainerPinMarshalSortedOutput(t *testing.T) { require.True(t, ok) assert.Equal(t, "sha256:zzz", pin.Digest) } + +// TestPruneStaleContainerPins verifies that PruneStaleContainerPins removes +// entries not present in the known-image set and preserves entries that are. +func TestPruneStaleContainerPins(t *testing.T) { + cache := NewActionCache(t.TempDir()) + + // Populate with three pins. + cache.SetContainerPin("ghcr.io/github/gh-aw-firewall/agent:0.27.0", "sha256:old", "ghcr.io/github/gh-aw-firewall/agent:0.27.0@sha256:old") + cache.SetContainerPin("ghcr.io/github/gh-aw-firewall/agent:0.27.2", "sha256:new", "ghcr.io/github/gh-aw-firewall/agent:0.27.2@sha256:new") + cache.SetContainerPin("node:lts-alpine", "sha256:node", "node:lts-alpine@sha256:node") + + // Lock files now only reference the new AWF version and the node image. + knownImages := map[string]bool{ + "ghcr.io/github/gh-aw-firewall/agent:0.27.2": true, + "node:lts-alpine": true, + } + + pruned := cache.PruneStaleContainerPins(knownImages) + assert.Equal(t, 1, pruned, "exactly one stale pin should be pruned") + + // Old version should be gone. + _, ok := cache.GetContainerPin("ghcr.io/github/gh-aw-firewall/agent:0.27.0") + assert.False(t, ok, "stale old-version pin should be removed") + + // Current versions should still be present. + pin, ok := cache.GetContainerPin("ghcr.io/github/gh-aw-firewall/agent:0.27.2") + require.True(t, ok, "current pin should be kept") + assert.Equal(t, "sha256:new", pin.Digest) + + pin, ok = cache.GetContainerPin("node:lts-alpine") + require.True(t, ok, "node pin should be kept") + assert.Equal(t, "sha256:node", pin.Digest) +} + +// TestPruneStaleContainerPins_AllStale verifies that pruning with an empty known set +// removes all container pins. +func TestPruneStaleContainerPins_AllStale(t *testing.T) { + cache := NewActionCache(t.TempDir()) + cache.SetContainerPin("image-a:v1", "sha256:aaa", "image-a:v1@sha256:aaa") + cache.SetContainerPin("image-b:v2", "sha256:bbb", "image-b:v2@sha256:bbb") + + pruned := cache.PruneStaleContainerPins(map[string]bool{}) + assert.Equal(t, 2, pruned, "all pins should be pruned when known set is empty") + assert.Empty(t, cache.ContainerPins, "ContainerPins map should be empty after full prune") +} + +// TestPruneStaleContainerPins_NoneStale verifies that pruning with a set matching all +// existing pins removes nothing. +func TestPruneStaleContainerPins_NoneStale(t *testing.T) { + cache := NewActionCache(t.TempDir()) + cache.SetContainerPin("node:lts-alpine", "sha256:abc", "node:lts-alpine@sha256:abc") + + // Mark cache as clean to verify dirty flag is not set when nothing changes. + cache.dirty = false + + pruned := cache.PruneStaleContainerPins(map[string]bool{"node:lts-alpine": true}) + assert.Equal(t, 0, pruned, "no pins should be pruned") + assert.False(t, cache.dirty, "dirty flag should not be set when nothing was pruned") +} + +// TestPruneStaleContainerPins_NilMap verifies that pruning a cache with a nil +// ContainerPins map returns 0 without panicking. +func TestPruneStaleContainerPins_NilMap(t *testing.T) { + cache := NewActionCache(t.TempDir()) + cache.ContainerPins = nil + + assert.NotPanics(t, func() { + pruned := cache.PruneStaleContainerPins(map[string]bool{"any:image": true}) + assert.Equal(t, 0, pruned, "nil map should return 0 pruned") + }) +}