Repository navigation
Use read-only cache mode for dry-run workflows - #70788
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality SentinelOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. Design Decision Gate 🏗️Outcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Ponytail Reviewer completed successfully! Ponytail ReviewerOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
![github-actions[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict
No actionable blocking issues found in the changed lines. The dry-run cache-mode emission is consistently gated by the compiler's GHES compatibility state, and the added regression test covers normal/dry-run transitions plus CLI and repository configuration paths. Static review only; no tests or builds were run.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · 1.82 AIC · ⌖ 0.543 AIC · ⊞ 1.6K
Comment /review to run again
…tash Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd — this is a clean, low-risk fix with no blocking issues.
📋 Analysis
Root cause
Previously, dry-run only guarded compiler-managed cache steps (cache-memory/drive-memory RestoreOnly), leaving authored pre-steps/setup cache actions (e.g. actions/cache@v5) free to save on dry-run. Emitting workflow-level cache-mode: read (pkg/workflow/compiler_yaml.go:94-96) closes this gap at the platform level rather than trying to intercept every authored cache step individually — a good deep-module choice per /codebase-design: one restore-only guarantee covers all current and future cache actions without per-step special-casing.
Test coverage (/tdd)
TestDryRunCompiledCacheMode in pkg/workflow/compiler_development_test.go is solid: it exercises normal → dry-run → normal transitions across github.com, GHES-via-CLI-flag, and GHES-via-aw.json, asserts against the real GitHub Actions schema (validateGitHubActionsSchemaFromParsed), and confirms the authored cache step and custom job both survive compilation unaffected. This is exactly the kind of state-transition + platform-matrix coverage that prevents regressions.
Pattern consistency
Checking data.DryRun directly inside generateWorkflowBody (rather than only in the centralized dryRunWorkflowData/dryRunConclusionData mutators) matches existing precedent in cache_memory.go, cache_steps.go, drive_memory.go, and role_checks.go — so this isn't a new architectural seam, just an extension of an established one.
Minor non-blocking note
cache-mode is schema-supported at both workflow and job level, but gh-aw doesn't currently expose a job-level cache-mode frontmatter override, so there's no present risk of an authored job silently re-enabling writes. Worth a mental note if job-level cache-mode configurability is ever added — the dry-run guard would then also need to strip/override any user-supplied job-level cache-mode: write.
Positive highlights
- ✅ Correctly scoped to GitHub.com only, with GHES explicitly and intentionally excluded (unsupported there), and existing restore-only safeguards retained
- ✅ Documentation update (
compilation-process.md) is accurate and concise - ✅ Verified: build passes, and
TestDryRunCompiledCacheModepasses locally
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 129 AIC · ⌖ 14.9 AIC · ⊞ 10.2K
Comment /matt to run again
There was a problem hiding this comment.
🟡 Changes recommended
Generated dry-run workflows are incompatible with the native Actions parser and pinned actionlint integration.
2 open issues
What changed in this PR
Extends gh-aw dry-run cache protection to caches used by authored steps and setup actions.
Changes:
- Emits
cache-mode: readfor GitHub.com dry runs, omitting it in GHES compatibility mode. - Tests normal/dry-run transitions and both GHES configuration methods.
- Documents cache behavior.
| File | Description |
|---|---|
pkg/workflow/compiler_yaml.go |
Emits conditional read-only cache mode. |
pkg/workflow/compiler_development_test.go |
Adds cache-mode regression coverage. |
docs/src/content/docs/reference/compilation-process.md |
Explains dry-run cache protection and GHES limitations. |
Review effort: Max
| yaml.WriteString("permissions: {}\n\n") | ||
|
|
||
| if data.DryRun && !data.GHES { | ||
| yaml.WriteString("cache-mode: read\n\n") |
| yaml.WriteString("permissions: {}\n\n") | ||
|
|
||
| if data.DryRun && !data.GHES { | ||
| yaml.WriteString("cache-mode: read\n\n") |
|
🎉 This pull request is included in a new release. Release: |
Dry runs should restore existing caches without saving changes, including caches used by authored steps and setup actions rather than only compiler-managed caches.
Emit workflow-level
cache-mode: readwhen compiling in dry-run mode on GitHub.com. Omit the setting for GHES, where it is unsupported, and retain existing restore-only safeguards. Normal compilation is unchanged.Adds regression coverage for normal/dry-run transitions and GHES configuration through both the CLI flag and
aw.json, plus documentation of the behavior.Validation: targeted dry-run cache tests, build, formatting, and workflow drift checks passed. The final validation gate remains blocked by pre-existing Go lint findings and unknown-linter warnings.