Apply container digest pinning during gh aw update and include container pins in frontmatter hash computation#27744
Apply container digest pinning during gh aw update and include container pins in frontmatter hash computation#27744
gh aw update and include container pins in frontmatter hash computation#27744Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/e01ccfa4-6e14-4d1a-9dd4-a405977ed212 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/e01ccfa4-6e14-4d1a-9dd4-a405977ed212 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
gh aw update by recompiling affected lock workflows
|
@copilot build and run update |
Built successfully with 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 compile should resolve container pins. The container pins should be part of the frontmatter hash computation. |
|
@copilot use container syntax image@sha256: when digest available |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a1d479ae-0d4e-44e6-b66e-8dd046768725 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Ensures gh aw update immediately applies newly resolved container image digest pins to compiled .lock.yml workflows by recompiling affected workflows during the same run.
Changes:
- Updated
UpdateContainerPinsto return whether pins were updated ((bool, error)) and adapted callers. - Added an update flow step to recompile workflows that already have
.lock.ymlfiles when container pins change. - Added unit tests covering discovery and deterministic ordering of workflow/lock pairs for recompilation.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/upgrade_command.go | Updates the upgrade flow to match the new UpdateContainerPins return signature. |
| pkg/cli/update_container_pins.go | Changes UpdateContainerPins to return whether any new container pins were added. |
| pkg/cli/update_command.go | Recompiles workflows with existing lock files when container pins change; adds workflow/lock pairing helper. |
| pkg/cli/update_command_container_pins_test.go | Adds tests for workflow selection and stable ordering for recompilation. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 1
| for _, workflowFile := range workflowFiles { | ||
| quiet := false | ||
| refreshStopTime := false | ||
| if err := compileWorkflowWithRefresh(workflowFile, verbose, quiet, engineOverride, refreshStopTime); err != nil { | ||
| return fmt.Errorf("failed to recompile %s after updating container pins: %w", filepath.Base(workflowFile), err) | ||
| } | ||
| } |
There was a problem hiding this comment.
recompileWorkflowsForContainerPins returns immediately on the first compilation error, which can leave remaining workflows with existing lock files un-recompiled (and therefore not updated to use the newly pinned image digests) in the same gh aw update run. Consider continuing through all workflows, collecting failures, and returning a combined error (or logging per-workflow warnings) so one bad workflow doesn't prevent pins being applied everywhere.
| for _, workflowFile := range workflowFiles { | |
| quiet := false | |
| refreshStopTime := false | |
| if err := compileWorkflowWithRefresh(workflowFile, verbose, quiet, engineOverride, refreshStopTime); err != nil { | |
| return fmt.Errorf("failed to recompile %s after updating container pins: %w", filepath.Base(workflowFile), err) | |
| } | |
| } | |
| var failures []string | |
| for _, workflowFile := range workflowFiles { | |
| quiet := false | |
| refreshStopTime := false | |
| if err := compileWorkflowWithRefresh(workflowFile, verbose, quiet, engineOverride, refreshStopTime); err != nil { | |
| failures = append(failures, fmt.Sprintf("%s: %v", filepath.Base(workflowFile), err)) | |
| } | |
| } | |
| if len(failures) > 0 { | |
| return fmt.Errorf("failed to recompile workflows after updating container pins: %s", strings.Join(failures, "; ")) | |
| } |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a1d479ae-0d4e-44e6-b66e-8dd046768725 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented in 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:
|
🧪 Test Quality Sentinel ReportTest Quality Score: 100/100✅ Excellent test quality
Test Classification DetailsView all test classifications (3 tests)
Flagged Tests — Requires ReviewNone. All tests are high-quality behavioral contracts. Language SupportTests analyzed:
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References: §24759857106
|
There was a problem hiding this comment.
✅ Test Quality Sentinel: 100/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 3 new tests enforce clear behavioral contracts with proper build tags, no mock libraries, and descriptive assertion messages.
gh aw update by recompiling affected lock workflowsgh aw update and include container pins in frontmatter hash computation
|
Warning The 🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (362 new lines in The Design Decision Gate has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: 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. 📋 What the draft ADR coversThe draft captures three interrelated decisions introduced by this PR:
The draft records the API signature change to 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
References: §24759857121 Caution Security scanning requires review for Design Decision Gate 🏗️ DetailsPotential security threats were detected in the agent output. The workflow output should be reviewed before merging. Review the workflow run logs for details.
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/262bf903-1374-47f0-9755-4489543f89f1 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Compiled lock workflows were not reliably updated to use newly resolved container digests during
gh aw update; pin data could be written toactions-lock.jsonwithout being reflected in.lock.ymloutputs in the same run.Update flow now applies container pins to lock files immediately
RunUpdateWorkflowsnow consumes whetherUpdateContainerPinsproduced new pins.--no-compileis not set), update triggers recompilation of workflows that already have lock files, so lock outputs pick upimage@sha256:...references in the same command execution.Container pin API adjusted for orchestration
UpdateContainerPinsnow returns(pinsUpdated bool, err error)instead of onlyerror.upgradepath was updated to match the new signature with no behavior change beyond compatibility.Deterministic workflow selection for recompilation
.mdworkflows with matching.lock.ymlfiles and recompile them in sorted order.Compile hash now reflects relevant container digest pins
.github/aw/actions-lock.jsonfor container images declared in workflow tool config.Focused coverage for lock/workflow pairing
> [!WARNING]
>
>