fix(sonar): the scan needs a build step, because a workspace tests through dist/ - #30
Merged
Conversation
CI-based analysis, because Automatic Analysis does not import coverage — its own documentation says "code coverage information is not supported". The nine `theokit*` projects were analysing today and reporting no coverage at all, which leaves the quality gate blind to the metric the suites are actually enforcing. One workflow rather than a step copied into eight repositories, and the reason is measured. The one inline version that existed — `theokit-tui`, added by B-126 — carried `if: matrix.node-version == '22.x'` against a matrix of `['22.12', '22']`. That value never occurs, so the step reported `skipped` on every run since it was written, and a skipped step renders exactly like a passing one. Copying it eight times would have copied that. Three decisions worth stating: - `fetch-depth: 0`. On a shallow clone every line looks like it arrived in the last commit, so the scanner attributes all issues to it and the new-code period — which is what the quality gate measures — describes the wrong change. - Coverage runs with `continue-on-error`. The caller's own test job gates the suite; here a broken coverage run should cost the coverage number, not take the security findings down with it. - `sonar-project.properties` is required, not defaulted. Without it the scanner analyses build output and reports a file as a duplicate of its own bundle — measured on theokit-tui. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186oaXQM4r2CdhRJgdTGhLq
… does not replace it
The header claimed SonarCloud "disables Automatic Analysis for a project as soon as a CI-based
analysis reports". It does not. The scanner fails:
[ERROR] ScannerEngine: You are running CI analysis while Automatic Analysis is enabled.
Please consider disabling one or the other.
Measured on theokit-tui: with the step finally executing (the `22.x` condition fixed) and the
token present, the run failed on exactly this and nothing else. Turning the toggle off by hand
under Administration > Analysis Method is a prerequisite, not something the workflow handles.
Written down here because a caller reading the old sentence would adopt the workflow and get a
red check with no idea it needed a manual step.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186oaXQM4r2CdhRJgdTGhLq
…everything `secrets: inherit` hands the called workflow every secret the caller holds. zizmor flags it (`secrets-inherit`), and it is right to: this workflow needs one token to publish an analysis, and inheriting gives it npm credentials, app private keys and anything else the repository carries. Declared as `required: false`, so a repository without the secret still runs the job — the existing step warns and skips rather than failing, which is what lets a repository adopt the workflow before the secret exists. Caught by running zizmor locally against the four callers before pushing them, rather than by watching four CI runs go red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186oaXQM4r2CdhRJgdTGhLq
…rough dist/
theokit-sdk's first analysis passed both checks and reported no coverage at all. The
scanner said only:
No coverage information will be saved because all LCOV files cannot be found.
The coverage step above it had already failed, and the log carried the reason:
Failed to resolve entry for package "@theokit/sdk"
sdk-cache: missing dist/index.js — run 'pnpm build'
Its packages import each other through their published entry points, so nine suites
cannot even load before `pnpm build` has run. This workflow installed and measured,
never built — and `continue-on-error` on the coverage step turned that into a green
check with an empty number, which is the failure mode this whole workflow exists to
remove.
`build-command` is a separate step rather than a prefix inside `coverage-command`:
the two fail for different reasons, and a build failure that reads as a coverage
failure is how the last one hid for a day.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186oaXQM4r2CdhRJgdTGhLq
# Conflicts: # .github/workflows/sonar.yml
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.
O que quebrou
theokit-sdkpassou nos dois checks do Sonar e reportou cobertura nenhuma. O scanner disse apenas:O motivo estava no step anterior, engolido pelo
continue-on-error:→
Failed Suites 9, e nenhum lcov escrito.Os pacotes do
theokit-sdkse importam pelos entry points publicados. Sempnpm build, nove suítes nem carregam. O jobcoverage floorsdo próprio repo já buildava antes; este workflow não.O conserto
Novo input
build-command, rodado entreInstalleCoverage. Step separado — não prefixo dentro decoverage-command— porque build e cobertura falham por motivos diferentes, e foi exatamente uma falha de build lida como falha de cobertura que escondeu isto por um dia.O comentário do step
Coveragepassa a registrar por que ocontinue-on-errorfica: ele custa o número, não o scan — mas emite warning, e é isso que faltou.Impacto nos callers
Nenhuma quebra:
build-commandtem default vazio. Só o caller dotheokit-sdkpassa a preenchê-lo.--min-severity low: 0 achados