Restore GHES-compatible artifact action pins - #53966
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot ghes mode is false by default. The current repo is NOT in ghes mode. |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
❌ Ponytail Reviewer failed. Please review the logs for details. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
There was a problem hiding this comment.
Request changes
This fixes the default GHES artifact pins, but it also introduces a regression where GHES mode silently overrides explicit aw.json action_pins remaps for artifact actions.
Blocking theme
- GHES compatibility now hard-codes
upload-artifact/download-artifactafter mapping, so repositories can no longer redirect those actions to an enterprise mirror or replacement implementation inaction_pins.
That is a behavior change in the pinning pipeline, not just a docs update, and it needs to preserve explicit admin-provided mappings.
🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 12.4 AIC · ⌖ 6.91 AIC · ⊞ 7K
Comment /review to run again
There was a problem hiding this comment.
Review: Restore GHES-compatible artifact action pins
The changes correctly restore explicit v3 artifact pins for GHES compatibility mode and update tests, docs, and the aw.json default. The logic is sound: GHES-specific pins are applied before dynamic resolution, mappings still take precedence, and tests cover both cases.
One documentation error found (blocking):
docs/enterprise-configuration.mdline 279 — theDefaultcolumn foractions/download-artifactwas changed to@v8, which does not exist. The latest release is v4.x. This appears to be a copy-paste from theupload-artifactrow (v7). Please correct it.
Otherwise the implementation is clean and the test assertions are accurate.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 31.9 AIC · ⌖ 8.87 AIC · ⊞ 5.7K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd — commenting with two issues to address before merge.
📋 Key Themes & Highlights
Issues Found
- Double GHES config call:
configureGHESCompatibility()is now called in bothParseWorkflowFileandCompileWorkflowData. For the normalCompileWorkflowpath this is harmless but redundant; the real cost is a secondloadRepoConfig()I/O per compile. - Weak test assertion:
TestResolveActionPin_GHESMappingTakesPrecedenceuses a resolver stub that always returns"", soassert.Empty(t, got)can never fail. The test doesn't actually verify that the enterprise mapping overrides the GHES static pin.
Positive Highlights
- ✅ Static SHA pins are a solid approach — no network call at compile time for the GHES path.
- ✅ Good coverage across unit, compiler, and CLI integration layers.
- ✅ Documentation correctly updated in both
docs/anddocs/src/mirrors. - ✅
aw.jsonGHES field changed fromtrue→falsein the repo config to avoid self-hosting on the compatibility path — good hygiene. - ✅
workflowData.GHESis propagated so downstream consumers can observe the effective mode.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 48.9 AIC · ⌖ 10.2 AIC · ⊞ 7.8K
Comment /matt to run again
Draft ADR documenting the decision to emit SHA-pinned v3 artifact action references in GHES compatibility mode instead of continuing to use the latest non-v3 pins. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ADR Required — Draft Committed to This BranchThis PR touches business logic across A draft ADR has been committed to this branch at: What to do next
Why ADRs matterADRs capture the why behind significant decisions so future contributors understand the constraints and trade-offs without having to reverse-engineer intent from code. Once accepted, an ADR is immutable — if the decision changes, a new ADR supersedes the old one. ADR sections required for gate to passThe gate checks for all four Nygard sections:
|
There was a problem hiding this comment.
Pull request overview
Restores GHES-compatible v3 artifact action pins throughout the compiler and documents the compatibility mode.
Changes:
- Adds GHES-aware artifact pin resolution and compiler propagation.
- Adds unit, workflow, and CLI integration coverage.
- Updates configuration, CLI help, and GHES documentation.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/workflow_data.go |
Propagates GHES mode into pin resolution. |
pkg/workflow/workflow_builder.go |
Initializes workflow GHES state. |
pkg/workflow/repo_config.go |
Updates GHES configuration documentation. |
pkg/workflow/ghes_artifact_pins_integration_test.go |
Tests flag and repository configuration paths. |
pkg/workflow/compiler.go |
Centralizes GHES configuration handling. |
pkg/workflow/compiler_types.go |
Updates compiler state documentation. |
pkg/workflow/compiler_orchestrator_workflow.go |
Configures GHES before file parsing. |
pkg/workflow/compiler_mutators.go |
Updates GHES setter documentation. |
pkg/workflow/action_pins.go |
Resolves generated artifact actions to v3 pins. |
pkg/workflow/action_pins_test.go |
Updates GHES pin tests. |
pkg/parser/schemas/repo_config_schema.json |
Documents emitted v3 versions. |
pkg/cli/compile_integration_test.go |
Tests compile --ghes. |
pkg/cli/compile_config.go |
Updates GHES option documentation. |
pkg/cli/compile_compiler_setup.go |
Updates setup logging and comments. |
pkg/actionpins/types.go |
Adds GHES pin context state. |
pkg/actionpins/resolve.go |
Defines and applies GHES artifact pins. |
pkg/actionpins/actionpins_internal_test.go |
Tests resolution and mapping precedence. |
docs/src/content/docs/reference/self-hosted-runners.md |
Documents the GHES compatibility path. |
docs/src/content/docs/reference/enterprise-configuration.md |
Adds GHES version guidance. |
docs/enterprise-configuration.md |
Corrects the default download version. |
cmd/gh-aw/main.go |
Updates --ghes help text. |
.github/workflows/aw.json |
Disables repository-level GHES mode. |
Review details
- Files reviewed: 23/23 changed files
- Comments generated: 5
- Review effort level: Balanced
PR Triage
Restores GHES-compatible SHA-pinned artifact actions (v3.2.2/v3.1.0), fixing workflow failures in GHES compatibility mode.
|
Addressed in |
|
@copilot This PR looks close, but it still needs a maintainer-facing finish pass.
|
…fact-actions Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Finish pass completed in Validation:
Remaining risks / notes:
|
|
@copilot This PR looks close, but it still needs one maintainer-visible finish pass.
|
Final finish pass completed on Validation:
Remaining risks / notes:
|
PR TriageCategory: bug · Risk: medium · Score: 58/100 (impact 30 + urgency 20 + quality 8) Wide-reaching GHES compatibility fix (27 files) with CI green but reviewer requested changes; needs-review label present, escalate for human attention. Automated triage — see run report for full details.
|
|
@copilot This PR still needs one maintainer-facing finish pass.
|
Finish pass completed on current HEAD Validation:
Remaining merge risks:
|
|
🎉 This pull request is included in a new release. Release: |
GHES compatibility mode emitted artifact actions requiring the unsupported v4+ backend, causing workflows to fail before agent execution.
Action pinning
upload-artifact@v3.2.2anddownload-artifact@v3.1.0.Compiler integration
gh aw compile --ghesand"ghes": trueinaw.json.Coverage and documentation
run: https://github.com/github/gh-aw/actions/runs/32276767055> Generated by 👨🍳 PR Sous Chef · gpt54 · 26.4 AIC · ⌖ 9.42 AIC · ⊞ 9.3K · ◷