ci(code-quality): skip cleanly when unconfigured, fail only when dispatched - #276
Merged
Merged
Conversation
…atched `code-quality` has failed on every merge to `main` since #272 added the `push` trigger. The cause is exactly what the job was written to report: Missing: SONAR_TOKEN SNYK_TOKEN ##[error]Process completed with exit code 1 Neither secret exists at repo or org level, so the preflight exits 1 and both scans are skipped. Nothing is broken in the code -- the workflow is correctly telling us it is not configured. WHY THAT ANSWER STOPPED BEING RIGHT. Failing loudly was correct when this ran weekly and on dispatch: an unconfigured repo produced one red X every Monday, and the message named the missing secrets instead of letting Sonar and Snyk fail with their own unhelpful auth errors. #272 added `push: [main]`, and the same behaviour now means `main` is permanently red for a condition that is not a defect. A red X on every merge is worse than a missing scan: it is a signal the reader learns to ignore, and it hides the next real failure. WHAT CHANGES. The preflight now reports rather than decides, and who asked determines the verdict: workflow_dispatch -> a human asked for a scan. Still FAILS, loudly. Silently doing nothing in response to a direct request is the worse outcome, so that path is unchanged. push / schedule -> nobody asked. Skips both scans, writes the missing secrets and where to get them to the run summary, and emits a ::notice. The build stays green. The two scan jobs are gated on a `configured` output rather than a condition written on them directly, because `secrets` cannot be referenced from a job-level `if:`. The skip is announced, never silent -- same standard the rest of this repo holds discovery to. A scan that did not run and says so is honest; one that quietly does nothing is not. Verified by running the preflight script directly in all three states: push without tokens exits 0 with `configured=false` and a correctly rendered summary; dispatch without tokens exits 1; either event with both tokens exits 0 with `configured=true`. This does not add the secrets. Once they exist the scans run and none of the above applies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Why it's failing
Exactly what the job was written to report:
Neither secret exists at repo or org level (
gh secret listis empty in both), so the preflight exits 1 and both scans are skipped. Nothing is broken in the code — the workflow is correctly telling us it isn't configured. It has failed on every merge since #272 added thepushtrigger.Why that answer stopped being right
Failing loudly was correct when this ran weekly and on dispatch: an unconfigured repo produced one red X a week, and the message named the missing secrets rather than letting Sonar and Snyk fail with their own unhelpful auth errors.
With
push: [main], the same behaviour makesmainpermanently red for a condition that is not a defect. A red X on every merge is worse than a missing scan — it's a signal the reader learns to ignore, and it hides the next real failure.What changes
The preflight now reports rather than decides, and who asked determines the verdict:
workflow_dispatchpush/schedule::notice. Build stays green.The scan jobs are gated on a
configuredoutput rather than a job-level condition, becausesecretscan't be referenced from a job-levelif:.The skip is announced, never silent — the same standard this repo already holds discovery to. A scan that didn't run and says so is honest; one that quietly does nothing isn't.
Verified
Ran the preflight script directly in all three states:
exit 0,configured=false, summary renders correctlyexit 1exit 0,configured=trueThis does not add the secrets
That's still yours to do, and it's the real fix — these scans have never run. Once
SONAR_TOKENandSNYK_TOKENexist, the scans execute and none of the above applies.🤖 Generated with Claude Code