{{ message }}
Repository navigation
Don't count about-to-be-pinned workflows as failed - #86
Merged
Merged
Conversation
There was a problem hiding this comment.
⚠️ Not ready to approve
The new “remediable-only” logic can incorrectly suppress real failures because NotPinned is also used for non-remediable workflow errors (e.g., load/deps errors), and the current exclusion logic can hide the only failure reason.
Pull request overview
Adjusts the terminal renderer’s failure summary so workflows that are about to be auto-pinned (dep-level not-pinned) don’t get reported as “failed” during a remediation run, avoiding misleading onboarding output.
Changes:
- Adds
isRemediableOnlyto detect workflows that are only failing due to dep-levelNotPinnedwhenwillRemediateis true. - Updates
PresentResultsto exclude remediable-only workflows fromfailedCount, and to conditionally suppressnot-pinnedin the error summary during remediation. - Adds a focused regression test covering onboarding-only output vs mixed failures vs read-only behavior.
File summaries
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Low
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The pre-remediation summary computed its failed count by treating every workflow where !IsValid() as a failure. A workflow whose only non-valid finding is a dep-level NotPinned is exactly what the current run is about to pin, so a plain onboarding run reported "N of N workflows failed: N not-pinned" for work it was actively doing. Exclude remediable-only workflows (willRemediate and every non-valid finding is NotPinned) from the failed count, gate the error block on a non-zero failed count, and drop not-pinned from the error parts when remediating. Read-only and verify surfaces (willRemediate=false) still count not-pinned as a genuine coverage gap. Confined to the terminal renderer and its test; finding classification (checks/IsValid/NeedsAttention) is unchanged. Resolves github/actions-dispatch#802
The #802 remediable-only logic treated any workflow whose only invalid finding was not-pinned as "about to be pinned", but diagnose.go also reports fatal workflow load / deps-read failures as workflow-level not-pinned (SeverityError, no ActionRef). Those are not remediable, so suppressing them let a genuinely broken workflow drop out of the failed count, and the unconditional not-pinned exclusion could blank out the only reason in the summary line. Restrict remediable-only to dep-level not-pinned (non-nil ActionRef), and exclude not-pinned from the summary only when another category already explains a failure. Update the regression test to model dep-level not-pinned at its production SeverityError and add fatal-load-error cases. Adds a catalog scenario asserting a fresh multi-workflow pin run does not report "N of N workflows failed".
nodeselector
force-pushed
the
nodeselector-issue-802-summary-miscount
branch
from
July 24, 2026 13:18
aef0dba to
c9b9b32
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Count only workflows that remain blocked after partial lockfile onboarding.
Why
Onboarding is atomic per workflow, not per repository. A mixed run can persist successful workflows while leaving a blocked workflow untouched, but the post-fix summary counted every pre-fix
not-pinnedfinding as a failure.That produced output such as
4 of 4 workflows failedafter three workflows had been pinned successfully.Resolves github/actions-dispatch#802.
How
not-pinnedfindings after remediation.Alternatives considered
not-pinnedfinding as fatal: rejected because dependency-inventory errors can be repaired during the same run.not-pinnedfinding after remediation: rejected because workflow parse failures must remain visible and fail the command.Risk
Testing