Skip to content

CI: Sonar and Codacy gates fail on generated bundles and on filesystem paths by design #19

Description

@amondnet

SonarCloud and Codacy both fail on #17, and neither failure is about the change under review. Both come from repository-level analyzer configuration that has never been written down — there is no sonar-project.properties and no .codacy.yml in the repo, so both services analyze everything with their defaults.

#17 was merged with these gates red, deliberately: the fixes below change what the repository's quality gates measure, which is a separate decision from the change #17 was making.

What fails, and why

SonarCloud — two failed quality-gate conditions

Condition Value Required
Duplication on New Code 7.7% ≤ 3%
Reliability Rating on New Code C ≥ A
  • The duplication is a file against its own generated bundle. skills/spring-docs/scripts/docs.mjs is built from scripts/docs.ts by bun run build:skill and committed, because the npx skills install channel copies only the skill directory and runs no dependency install (see the README's "Which one" section). The two files are the same program, so any PR that adds real code to scripts/docs.ts adds it twice as far as Sonar is concerned.
  • The reliability rating is the bundler's output. Both issues are Unexpected var, use let or const instead at skills/spring-docs/scripts/docs.mjs:162 and :234. Bun emits those; they cannot be edited away, and editing the bundle by hand would fail bun run build:skill:check, which byte-compares it against a fresh build.

Codacy — 36 new issues (3 critical, 33 high)

35 of them are one rule: Found <fn> from package "node:fs" with non literal argument, on existsSync, lstatSync, readdirSync, readFileSync, readlinkSync, mkdirSync, writeFileSync, symlinkSync, utimesSync. scripts/docs.ts resolves a cache path and reads, writes, links and deletes under it; a non-literal path is what the module is for. The rule fires on the test file too.

The 36th — a non-null assertion in a test — was a real finding and is fixed in #17.

What to decide

  1. Exclude the committed bundles from analysis. skills/*/scripts/*.mjs are generated artifacts; sonar.exclusions plus sonar.cpd.exclusions, and the Codacy equivalent, would take them out of both duplication and issue counting. SonarCloud runs here in Automatic Analysis mode, so confirm a committed sonar-project.properties is actually read before assuming it works.
  2. Decide what the non-literal-fs rule is worth in this repository. The path this module builds is validated — isSafeSegment in scripts/lib/docs-cache.ts rejects ., .., and anything outside [\w.+-], and docsCachePath is the only thing that joins it. Either that argument is written down once and the rule is disabled for the repo, or the rule stays and every filesystem PR carries ~35 findings that nobody reads.
  3. Confirm whether these checks should block. main has no branch protection and no ruleset, yet mergeStateStatus reads BLOCKED and allow_auto_merge is false — so the merge path is manual and the gates' real authority is unclear. Worth settling alongside the above.

Why it surfaced now

#15 and #16 passed both services because they changed documentation and settings — no new code, so the "new code" metrics were trivially clean. #17 is the first PR since these services were enabled that adds substantial code to scripts/docs.ts and therefore to its bundle.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions