Conversation
- Add ContainerPin type and Get/Set/Delete methods to ActionCache - Add GHAWManifestContainer type to safe_update_manifest - Update NewGHAWManifest to accept container entries - Update collectDockerImages to apply cached digest pins and populate WorkflowData - Add mergeDockerImages and mergeDockerImagePins helper functions - Add DockerImages and DockerImagePins fields to WorkflowData - Embed container image list in compiled lock file headers - Add UpdateContainerPins CLI function using docker buildx imagetools inspect - Call UpdateContainerPins from update and upgrade commands - Recompile all lock files to include container image sections Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot container sha resolution must receive a context from the caller. |
…r resolution helpers Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d7c32f5c-4501-48b6-ba55-5e6ce30e663f Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in c45d5f6. |
|
@copilot merge main and recompile |
…h-aw-pull-25619 # Conflicts: # .github/workflows/test-quality-sentinel.lock.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in b42ec6f. Merged Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
Extends the existing action pinning/lock infrastructure to also track and apply Docker image digest pins, so workflows can pull immutable image@sha256:... references and record container usage in the embedded manifest.
Changes:
- Adds container pin storage to
.github/aw/actions-lock.jsonviaActionCacheCRUD APIs and deterministic JSON output. - Updates compilation to (a) apply cached container digest pins and (b) emit container usage into
# gh-aw-manifestplus a new lock header section. - Integrates
UpdateContainerPins()intogh aw updateandgh aw upgradeto discover images from*.lock.ymlfiles and resolve digests via Docker CLI.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/action_cache.go | Adds container pin storage/serialization alongside action pins. |
| pkg/workflow/safe_update_manifest.go | Extends embedded manifest schema to include container entries. |
| pkg/workflow/safe_update_manifest_test.go | Updates manifest constructor usage; tests currently don’t cover new container behavior. |
| pkg/workflow/compiler_yaml.go | Emits containers into manifest and adds “Container images used” header section. |
| pkg/workflow/compiler_types.go | Adds WorkflowData.DockerImages / DockerImagePins fields. |
| pkg/workflow/docker.go | Collects Docker images, applies cached digest pins, and stores container pin info into WorkflowData. |
| pkg/cli/update_container_pins.go | New: scans lock files, resolves image digests, persists pins to actions-lock.json. |
| pkg/cli/update_command.go | Wires context through update flow and calls UpdateContainerPins(). |
| pkg/cli/update_command_test.go | Updates tests for new RunUpdateWorkflows(ctx, ...) signature. |
| pkg/cli/upgrade_command.go | Wires context through upgrade flow and calls UpdateContainerPins() before compilation. |
| .github/workflows/workflow-skill-extractor.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/workflow-health-manager.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/workflow-generator.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/weekly-safe-outputs-spec-review.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/weekly-editors-health-check.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/video-analyzer.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/update-astro.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/ubuntu-image-analyzer.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/typist.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/test-workflow.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/test-quality-sentinel.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/test-project-url-default.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/test-dispatcher.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/terminal-stylist.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/sub-issue-closer.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/smoke-workflow-call.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/smoke-workflow-call-with-inputs.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/smoke-temporary-id.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/smoke-service-ports.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/smoke-project.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/smoke-multi-pr.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/smoke-create-cross-repo-pr.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/smoke-agent-scoped-approved.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/smoke-agent-public-none.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/smoke-agent-public-approved.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/smoke-agent-all-none.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/smoke-agent-all-merged.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/semantic-function-refactor.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/security-compliance.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/schema-feature-coverage.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/research.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/repo-tree-map.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/refiner.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/pr-triage-agent.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/plan.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/notion-issue-summary.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/mergefest.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/layout-spec-maintainer.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/issue-triage-agent.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/issue-monster.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/issue-arborist.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/go-pattern-detector.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/github-remote-mcp-auth-test.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/functional-pragmatist.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/firewall.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/example-permissions-warning.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/duplicate-code-detector.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/draft-pr-cleanup.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/docs-noob-tester.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/discussion-task-miner.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/dictation-prompt.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/dev.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/design-decision-gate.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/dependabot-go-checker.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/dependabot-burner.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/delight.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-workflow-updater.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-testify-uber-super-expert.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-team-status.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-team-evolution-insights.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-syntax-error-quality.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-security-red-team.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-secrets-analysis.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-safe-outputs-conformance.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-safe-output-integrator.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-regulatory.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-malicious-code-scan.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-file-diet.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-fact.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-community-attribution.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-cli-performance.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-choice-test.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/daily-assign-issue-to-user.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/craft.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/copilot-cli-deep-research.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/contribution-check.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/commit-changes-analyzer.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/codex-github-remote-mcp-test.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/code-simplifier.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/cli-consistency-checker.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/changeset.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/breaking-change-checker.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/brave.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/bot-detection.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/blog-auditor.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/auto-triage-issues.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/artifacts-summary.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/architecture-guardian.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/archie.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/approach-validator.lock.yml | Regenerated lock header/manifest to include container list. |
| .github/workflows/ace-editor.lock.yml | Regenerated lock header/manifest to include container list. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 201/201 changed files
- Comments generated: 2
pkg/workflow/action_cache.go
Outdated
| // Add container_pins section if non-empty | ||
| if len(c.ContainerPins) > 0 { | ||
| pinKeys := make([]string, 0, len(c.ContainerPins)) |
There was a problem hiding this comment.
The comment says this block adds a "container_pins" section, but the JSON key being written is "containers". Renaming the comment (or aligning terminology) would avoid confusion when debugging actions-lock.json output and matching it to the struct tags/docs.
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| m := NewGHAWManifest(tt.secretNames, tt.actionRefs) | ||
| m := NewGHAWManifest(tt.secretNames, tt.actionRefs, nil) | ||
| require.NotNil(t, m, "manifest should not be nil") |
There was a problem hiding this comment.
NewGHAWManifest now accepts container entries, but the tests here only pass nil and don't assert container sorting/deduplication or JSON serialization behavior (including omitempty fields). Adding a couple of test cases with repeated/unsorted containers (and with/without digest) would prevent regressions in the new manifest format.
|
@copilot review all comments |
|
Hey The implementation looks clean and thorough — All checklist items pass:
This PR looks ready for maintainer review. 🚀
|
…er tests to manifest test suite Agent-Logs-Url: https://github.com/github/gh-aw/sessions/895734e0-7885-4733-9124-8c2da8b97141 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed both review comments in 5ba993d:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot find a more robust way to resolve image shas or add more logging See https://github.com/github/gh-aw/actions/runs/24264530183/job/70856323704#step:7:1 |
… in UpdateContainerPins Agent-Logs-Url: https://github.com/github/gh-aw/sessions/29487e93-c73e-467d-9bb1-e5b9c1470db4 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
… string Agent-Logs-Url: https://github.com/github/gh-aw/sessions/29487e93-c73e-467d-9bb1-e5b9c1470db4 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in a976904 + 11ed7ea. Two improvements: More robust resolution —
Better logging — when images fail, the summary now shows the reason for each image rather than just the image name: All three per-image error messages are concatenated with |
|
@copilot See error when running in action https://github.com/github/gh-aw/actions/runs/24265363003/job/70859007129#step:7:1 |
…r digest resolution Agent-Logs-Url: https://github.com/github/gh-aw/sessions/fdcb46b8-9d91-4665-8d5a-32c660e003fb Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…omment Agent-Logs-Url: https://github.com/github/gh-aw/sessions/fdcb46b8-9d91-4665-8d5a-32c660e003fb Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in ed1f5a2. The CI log showed The fix drops the Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot does not seem to be able to resolve the sha of containers https://github.com/github/gh-aw/actions/runs/24268033876/job/70867153360#step:7:1 |
The SHA resolution is actually working in that run — the log shows |
|
@copilot show shas in logs |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a7aacdd5-ebf9-4222-aa55-58e0290175cd Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Reconstructs #25619
All MCP container images (
node:lts-alpine,ghcr.io/github/gh-aw-firewall/*,ghcr.io/github/gh-aw-mcpg,ghcr.io/github/github-mcp-server) were pulled by mutable tag with no SHA-256 pinning. This extends the existing action-pin infrastructure to cover Docker images.Storage —
actions-lock.jsonNew
containerssection alongsideentries, keyed by image tag:{ "entries": { ... }, "containers": { "node:lts-alpine": { "image": "node:lts-alpine", "digest": "sha256:abc123...", "pinned_image": "node:lts-alpine@sha256:abc123..." } } }ActionCachegainsGetContainerPin/SetContainerPin/DeleteContainerPin. Loading a legacy file (nocontainers) is fully backward-compatible.Compilation
collectDockerImagesapplies cached pins, substitutingtag@sha256:…for any image with a stored entry.WorkflowData.DockerImagesaccumulates the resolved (possibly pinned) refs so they can be surfaced in the lock file.WorkflowData.DockerImagePinsaccumulates fullGHAWManifestContainerentries (image, digest, pinned_image) for the manifest.# Container images used:comment section.gh-aw-manifestJSON gains a"containers":[…]field with full structured container info including SHA-256 digest when available.Update/Upgrade integration —
pkg/cli/update_container_pins.goUpdateContainerPins()is called from bothgh aw updateandgh aw upgrade(after action pins, before compile):.lock.ymlfiles fordownload_docker_images.shinvocations to discover in-use image tags.docker buildx imagetools inspect(no pull, preferred — works when Docker daemon is running)crane digest(no daemon required — works in CI environments without Docker socket)docker pull+docker inspect(fallback that pulls the full image)actions-lock.json; next compile embeds the@sha256:…suffix.When images cannot be pinned, the failure summary now shows the per-image reason from each failed strategy (e.g.
ghcr.io/github/github-mcp-server:v0.32.0: docker buildx imagetools: …; crane digest: …; docker pull + inspect: …) to aid diagnosis.