fix(ci): write the Sonar exclusions as path patterns, not directory prefixes

.please/docs/knowledge/gotchas.md

@@ -22,7 +22,9 @@
 
 Neither service is invoked by a workflow — both analyse every push through their GitHub App. That changes where their configuration lives and when it takes effect.
 
-- **SonarQube Cloud is in Automatic Analysis mode, which reads `.sonarcloud.properties` and ignores `sonar-project.properties`.** The docs are explicit that the two files are different and that a `sonar-project.properties` in an imported project is ignored. Three further constraints: only the copy on the **default branch** applies (a change does not affect the PR that makes it), **wildcards are not allowed** in the values, and where the file and the SonarQube Cloud UI disagree the **file wins** — so an entry that matches nothing cannot be corrected from the UI while it is still there.
+- **SonarQube Cloud is in Automatic Analysis mode, which reads `.sonarcloud.properties` and ignores `sonar-project.properties`.** The docs are explicit that the two files are different and that a `sonar-project.properties` in an imported project is ignored. Three further constraints: only the copy on the **default branch** applies (a change does not affect the PR that makes it), the **exclusion values are path patterns, not directory prefixes**, and where the file and the SonarQube Cloud UI disagree the **file wins** — so an entry that matches nothing cannot be corrected from the UI while it is still there.
+
+- **The [Automatic Analysis page](https://docs.sonarsource.com/sonarqube-cloud/analyzing-source-code/automatic-analysis/)'s "Wildcard patterns are not allowed" is about `sonar.sources` and `sonar.tests`, not the exclusions.** Those two take plain path lists; `sonar.exclusions` and `sonar.cpd.exclusions` take [path patterns](https://docs.sonarsource.com/sonarqube-cloud/managing-your-projects/project-analysis/setting-analysis-scope/excluding-files-based-on-patterns/) matched against whole file paths, where a whole subtree is written `dir/**/*` and a bare `dir/` matches nothing and is discarded in silence. Reading that line as covering every value in the file is what made ee09832's exclusion inert: `main` kept indexing 5,018 lines of generated bundle and reported 359 of its 412 findings inside the directory that was supposed to be excluded, until #26. `scripts/__tests__/generated-bundle-exclusions.test.ts` now fails on any exclusion pattern that matches no committed bundle — a pattern matching nothing is indistinguishable from a working one by reading it.
 
 - **Codacy reads `.codacy.yml` (or `.codacy.yaml`), and the first line must be `---`.** Additions are honoured on the PR that makes them; only removals wait for the default branch. Once the file exists, the UI's "Ignored files" settings stop applying. Validate before pushing: `docker run --rm -v "$(pwd)":/src codacy/codacy-analysis-cli validate-configuration --directory /src`.
 

.sonarcloud.properties

@@ -1,23 +1,31 @@
 # SonarQube Cloud runs here in Automatic Analysis mode — no workflow invokes a
 # scanner, the GitHub App analyses every push on its own. That mode reads this
-# file and ignores `sonar-project.properties`, and it accepts no wildcards in
-# these values, so paths are written as plain directory prefixes.
+# file and ignores `sonar-project.properties`.
 #
-# Two consequences worth knowing before editing:
+# Three consequences worth knowing before editing:
 #   - Only the copy on the default branch takes effect. A change here does not
 #     alter the analysis of the pull request that makes it; it applies once
 #     merged.
 #   - Where this file and the SonarQube Cloud UI disagree, this file wins. An
 #     entry that matches nothing therefore cannot be corrected from the UI while
 #     it is still here.
+#   - The exclusion values below are path *patterns* matched against whole file
+#     paths, not directory prefixes: `*` matches within one path segment, `**`
+#     spans directories, and a whole subtree is written `dir/**/*`. A bare
+#     `dir/` matches no file and is discarded in silence. The Automatic Analysis
+#     page's "Wildcard patterns are not allowed" is about the plain path lists
+#     `sonar.sources` and `sonar.tests`; ee09832 read it as covering these too
+#     and wrote bare prefixes, which left the exclusion inert — `main` still
+#     indexed 5,018 lines of generated code and reported 359 findings in it.
 
 # Generated by `bun run build:skill` from `scripts/*.ts`, committed because the
 # `npx skills` install channel copies only the skill directory and runs no
 # dependency install. Analysing them scores the same program twice: every line
 # of `scripts/docs.ts` reappears in the bundle, which read as 7.7% duplication
 # on new code, and the `var` declarations Bun emits cost a reliability rating
 # that no edit can recover — `bun run build:skill:check` byte-compares the
-# bundle against a fresh build, so hand-editing it fails CI.
-# `eslint.config.js` ignores the same directory, for the same reason.
-sonar.exclusions=skills/spring-docs/scripts/
-sonar.cpd.exclusions=skills/spring-docs/scripts/
+# bundle against a fresh build, so hand-editing it fails CI. `detect.mjs` also
+# inlines `fast-xml-parser`, whose entity tables read as first-party source.
+# `eslint.config.js` and `.codacy.yml` exclude the same directories.
+sonar.exclusions=skills/*/scripts/**/*
+sonar.cpd.exclusions=skills/*/scripts/**/*

scripts/__tests__/generated-bundle-exclusions.test.ts

@@ -0,0 +1,122 @@
+/**
+ * Guards the analyser exclusions that keep `skills/<skill>/scripts/` out of Sonar,
+ * Codacy and ESLint.
+ *
+ * The exclusion in ee09832 was written as a bare directory prefix
+ * (`skills/spring-docs/scripts/`). Every analyser matches patterns against the
+ * whole file path, so that prefix matched nothing and was dropped without a
+ * warning — SonarCloud kept indexing 5,018 generated lines for two days before
+ * anyone noticed. A pattern that matches no file looks identical to a pattern
+ * that works, so assert the match instead of reading the config.
+ *
+ * Each reader below takes the *active* setting, never the file's raw text: a
+ * commented-out exclusion still contains a pattern that would match, so a text
+ * scan passes while the analyser receives nothing — the same blind spot one
+ * level down. Each reader therefore drops the file's comment lines before it
+ * takes anything from them.
+ *
+ * `Bun.Glob` stands in for three matchers it is not, so a pass is not proof
+ * that SonarCloud reads a pattern the same way. What it does catch is the
+ * failure that actually happened: a pattern that matches no bundle at all.
+ */
+import { join } from 'node:path'
+
+import { describe, expect, test } from 'bun:test'
+
+const ROOT = join(import.meta.dir, '..', '..')
+
+/** The committed bundles every exclusion below is supposed to cover. */
+const BUNDLES = Array.from(new Bun.Glob('skills/*/scripts/*.mjs').scanSync(ROOT), p => p.replaceAll('\\', '/'))
+
+/**
+ * Split on `\r?\n`, not `\n`: the repository carries no `.gitattributes`, so a
+ * Windows checkout with `core.autocrlf=true` leaves a `\r` on every value, and
+ * a pattern ending in `\r` matches no bundle — a false failure that reads
+ * exactly like the misconfiguration this file exists to catch.
+ */
+async function lines(file: string): Promise<string[]> {
+  return (await Bun.file(join(ROOT, file)).text()).split(/\r?\n/)
+}
+
+/**
+ * The patterns `.sonarcloud.properties` sets for `key`, as a `key=a,b` line.
+ *
+ * Split each line on its first `=` and compare the trimmed key, rather than
+ * matching a prefix: `.properties` allows whitespace around the separator and
+ * before the key, and a reformatted file should not fail the guard. A
+ * commented-out setting keeps its `#`, so its key never compares equal.
+ */
+async function sonarExclusions(key: string): Promise<string[]> {
+  for (const line of await lines('.sonarcloud.properties')) {
+    const separator = line.indexOf('=')
+    if (separator < 0 || line.slice(0, separator).trim() !== key)
+      continue
+    return line.slice(separator + 1).split(',').map(v => v.trim()).filter(Boolean)
+  }
+  return []
+}
+
+/** The `skills/` entries of `.codacy.yml`'s `exclude_paths:` block. */
+async function codacyExclusions(): Promise<string[]> {
+  const source = await lines('.codacy.yml')
+  const start = source.findIndex(l => l.startsWith('exclude_paths:'))
+  expect(start).toBeGreaterThanOrEqual(0)
+
+  const patterns: string[] = []
+  for (const line of source.slice(start + 1)) {
+    const entry = line.trim()
+    if (entry.startsWith('- ')) {
+      // Strip the quotes YAML does not require here; single, double, or none.
+      patterns.push(entry.slice(2).trim().replace(/^["']|["']$/g, ''))
+      continue
+    }
+    // A comment or a blank line sits inside the block; anything else ends it.
+    if (entry !== '' && !entry.startsWith('#'))
+      break
+  }
+  return patterns.filter(p => p.startsWith('skills/'))
+}
+
+/**
+ * The `skills/` ignore patterns in `eslint.config.js`'s `ignores` array.
+ *
+ * Read as text with comment lines dropped, not by importing the module. The
+ * evaluated config would be the stronger source, but reaching it needs either a
+ * non-literal dynamic `import()` or `allowJs` in `tsconfig.json`, and neither is
+ * worth a project-wide change here. Dropping `//` lines closes the gap that
+ * matters: a commented-out ignore no longer supplies a pattern that passes.
+ */
+async function eslintIgnores(): Promise<string[]> {
+  return (await lines('eslint.config.js'))
+    .map(l => l.trim())
+    .filter(l => !l.startsWith('//') && l.startsWith('\'skills/'))
+    .map(l => l.slice(1, l.indexOf('\'', 1)))
+}
+
+function expectCoversBundles(patterns: string[]): void {
+  expect(patterns.length).toBeGreaterThan(0)
+  for (const pattern of patterns) {
+    const glob = new Bun.Glob(pattern)
+    for (const bundle of BUNDLES) expect([pattern, bundle, glob.match(bundle)]).toEqual([pattern, bundle, true])
+  }
+}
+
+describe('generated bundle exclusions', () => {
+  test('the bundles the exclusions target are committed', () => {
+    // A Set, because `Bun.Glob.scanSync` fixes no traversal order.
+    expect(new Set(BUNDLES)).toEqual(new Set(['skills/spring-docs/scripts/detect.mjs', 'skills/spring-docs/scripts/docs.mjs']))
+  })
+
+  test('.sonarcloud.properties excludes them from issues and duplication', async () => {
+    expectCoversBundles(await sonarExclusions('sonar.exclusions'))
+    expectCoversBundles(await sonarExclusions('sonar.cpd.exclusions'))
+  })
+
+  test('.codacy.yml excludes them', async () => {
+    expectCoversBundles(await codacyExclusions())
+  })
+
+  test('eslint.config.js ignores them', async () => {
+    expectCoversBundles(await eslintIgnores())
+  })
+})