Repository navigation
Add inline YAML fix example to persist-credentials compile error - #54957
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
| } | ||
|
|
||
| const persistCredentialsExample = ` # example (keep your existing "uses:" ref): | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
@copilot resolve the current release of checkout and insert in message
There was a problem hiding this comment.
Updated to the current pinned release: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 (matches the repo's pkg/actionpins/data/action_pins.json entry and how it's pinned in compiled .lock.yml workflows).
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
| } | ||
|
|
||
| const persistCredentialsExample = ` # example (keep your existing "uses:" ref): | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 |
There was a problem hiding this comment.
@copilot resolve actions/checkout with a action pin manager and use the version ref only in the example
There was a problem hiding this comment.
Updated in 5e9... — now resolves actions/checkout's current pin via actionpins.GetLatestActionPinByRepo("actions/checkout") and uses only the version ref (e.g. actions/checkout@v7.0.1) in the example, falling back to v4 if resolution fails.
There was a problem hiding this comment.
Correction: the commit is e6e792e, not the placeholder hash in my previous reply. It resolves actions/checkout's current pin via actionpins.GetLatestActionPinByRepo("actions/checkout") and uses only the version ref (e.g. actions/checkout@v7.0.1) in the example, falling back to v4 if resolution fails.
… in example Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. This PR only modified pkg/workflow/imported_steps_validation.go (production code). Test Quality Sentinel skipped.
|
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship. Diff is a small, direct change (resolve action pin, format example string, append to message) with no speculative abstractions, unused flexibility, or reinvented stdlib functionality.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ PR Code Quality Reviewer completed the code quality review. Reviewed PR #54957: no actionable changed-line issues found, so no review comments or final review submission were needed.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR does not have the 'implementation' label and has ≤100 new lines of code in business logic directories (13 additions detected, threshold is 100).
|
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.
Skills-Based Review 🧠
Applied /tdd — one observation on test coverage; no blocking issues.
📋 Key Themes & Highlights
Key Themes
- Test coverage gap: the new YAML snippet embedded in the error/warning message has no test asserting its presence or format. A future refactor of the
fmt.Sprintfassembly could silently drop the user-facing example without failing any test.
Positive Highlights
- ✅ Using
actionpins.GetLatestActionPinByRepois the right call — the version stays in sync with pinned data automatically and degrades gracefully to"v4". - ✅ The comment
# example (keep your existing "uses:" ref):correctly sets user expectations and avoids implying they should change their pinned SHA. - ✅ Change is minimal and surgical — one logical addition to one function.
@copilot please address the review comment above.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 32.3 AIC · ⌖ 12.7 AIC · ⊞ 7.6K
Comment /matt to run again
| "Without this setting the git token is stored in .git/config and leaked to the agent. "+ | ||
| "Add 'persist-credentials: false' to the 'with:' block of each checkout step. "+ | ||
| "Add 'persist-credentials: false' to the 'with:' block of each checkout step, for example:\n"+ | ||
| persistCredentialsExample+"\n"+ |
There was a problem hiding this comment.
{
"pull_request_number": 54957,
"path": "pkg/workflow/imported_steps_validation.go",
"line": 110,
"body": "[/tdd] The new YAML example snippet has no test asserting its presence or format. A future refactor of the fmt.Sprintf assembly could silently drop the user-facing example without any test failure.\n\n
💡 Suggested test assertions
\n\nAdd to the existingvalidateCheckoutPersistCredentials tests or a new small test:\n\ngo\nassert.Contains(t, err.Error(), \"- uses: actions/checkout@\")\nassert.Contains(t, err.Error(), \"persist-credentials: false\")\nassert.Contains(t, err.Error(), \"# example\")\n\n\nThese string-contains checks are version-agnostic and add virtually no maintenance cost.\n\n}
There was a problem hiding this comment.
Impeccable review — clarify mode
The change makes the persist-credentials: false compile error actionable by embedding a concrete YAML snippet directly in the message. Using actionpins.GetLatestActionPinByRepo to surface the pinned version is a nice touch, and the fallback to v4 is safe. The comment signals the snippet is illustrative, not copy-paste verbatim.
No blocking issues found.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 22.3 AIC · ⌖ 8.97 AIC · ⊞ 6.2K
There was a problem hiding this comment.
Pull request overview
Adds an actionable YAML example to the checkout credential validation message.
Changes:
- Generates the example using the latest embedded checkout version, with a
v4fallback. - Includes the snippet in strict errors and non-strict warnings.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/imported_steps_validation.go |
Adds the inline remediation example. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
| persistCredentialsExample := fmt.Sprintf(` # example (keep your existing "uses:" ref): | ||
| - uses: actions/checkout@%s | ||
| with: | ||
| persist-credentials: false`, checkoutVersion) |
|
🎉 This pull request is included in a new release. Release: |
Strict-mode compile errors (and non-strict warnings) for missing
persist-credentials: falsetold users what to fix but not how, forcing a context switch to docs mid-build-failure to recall the exact YAML syntax.Changes
pkg/workflow/imported_steps_validation.go: append a copy-pasteable YAML snippet to the error/warning message emitted byvalidateCheckoutPersistCredentials, so the fix can be applied and recompiled immediately.The example is annotated as illustrative to avoid implying users should change their pinned
uses:ref when copying the snippet.