diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index e818f447..98408ad3 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -294,6 +294,32 @@ folklore in agent-brief prose. The standing rules: assertions); octocov-style coverage actions (a 15-line floor script suffices; no new CI dependency). +## Quality gates: duplication + cognitive complexity (goal 0109) + +The SonarQube-class gates, wired measurement-first (a full read-only +run over the tree BEFORE thresholds were committed — hit lists in the +goal file), never aspiration-first: + +- **Duplication (`dupl` @ 150, repo-wide)**: lands green because the + only existing clusters are deliberate and excluded BY NAME — test + twins (`_test.go`), the per-entity seed/CRUD shape in + `configuresvc/`, and `atlasservice_builtin.go`. New duplication + anywhere else fails the build. Anti-gaming: exclusions are named + files/families with recorded reasons, never a threshold raise. +- **Cognitive complexity (`gocognit` @ 15, NEW/CHANGED code only)**: + 25 legacy production functions sit over the threshold (max 65 — + burn-down list in the goal file), so the gate runs Sonar's own + clean-as-you-code posture via `--enable-only gocognit + --new-from-merge-base=origin/main` (lefthook `gocognit-new` + a CI + step; the lint job's checkout carries `fetch-depth: 0` for the + merge-base). Legacy refuses to rot further without failing today's + build; touching a legacy offender means paying its complexity down + or consciously splitting the change. +- **Deferred to the npm-deps landing** (lockfile-conflict avoidance, + recorded in the goal file): eslint-plugin-sonarjs (TS cognitive + complexity + duplicate-branch rules) and the diff-cover + changed-lines coverage gate. + ## Shared-pool vs dedicated e2e servers — declare it up front A spec whose assertions read GLOBAL app state that other tests can diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ee200722..4e42d6cb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -206,6 +206,11 @@ jobs: CGO_ENABLED: '0' steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + # Full history: the gocognit new-code gate below diffs against + # the merge-base with origin/main, which a depth-1 clone + # cannot compute. + fetch-depth: 0 - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: name: frontend-dist @@ -218,6 +223,12 @@ jobs: with: version: v2.12 args: --build-tags=server + # Cognitive-complexity gate on NEW/CHANGED code only (goal 0109, + # the clean-as-you-code posture -- see lefthook.yml's gocognit-new + # job for the full reasoning). Reuses the binary the action above + # already installed onto PATH. Requires the full-history checkout + # this job's own fetch-depth: 0 provides (merge-base needs it). + - run: golangci-lint run . ./internal/... --build-tags=server --enable-only gocognit --new-from-merge-base=origin/main build-go: needs: [changes, frontend] @@ -357,7 +368,29 @@ jobs: cache-dependency-path: frontend/package-lock.json - run: npm ci working-directory: frontend - - run: npx playwright install --with-deps chromium + # Browser binaries cached by Playwright version: the uncached + # `install --with-deps` path stalls in its apt phase on flaky + # ubuntu mirrors (three 15-minute shard hangs observed in one + # day), and the runner image already carries Chromium's system + # deps -- so a cache hit skips the whole install, and only a + # real Playwright version bump pays the download again (the + # upstream-documented CI caching pattern). + - name: Resolve Playwright version + id: pw-version + working-directory: frontend + run: echo "version=$(node -p "require('@playwright/test/package.json').version")" >> "$GITHUB_OUTPUT" + - uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + id: pw-cache + with: + path: ~/.cache/ms-playwright + key: playwright-chromium-${{ runner.os }}-${{ steps.pw-version.outputs.version }} + # No --with-deps: the ubuntu runner image already ships + # Chromium's system libraries, and the apt phase behind + # --with-deps is the exact stall (archive.ubuntu.com hanging + # until the job's 15-minute cap) this caching exists to kill -- + # observed persisting even on cache-miss runs. + - if: steps.pw-cache.outputs.cache-hit != 'true' + run: npx playwright install chromium working-directory: frontend # 3-shard matrix + playwright.config.ts's workers: CI ? 1 : 4 (goal # 0024): the prior single-job, workers:2-in-CI shape was the actual diff --git a/.golangci.yml b/.golangci.yml index 0dbbd774..a4097f3b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -26,6 +26,13 @@ linters: - prealloc - contextcheck - sqlclosecheck + # Third pass (goal 0109, the SonarQube-class duplication gate): + # threshold and exclusions tuned from a full measurement run over + # the tree (28 pairs found, every one either the deliberate + # per-entity seed-reconcile shape or a test twin -- hit lists in + # the goal file), so the gate lands green and catches NEW + # duplication anywhere outside the named, deliberate families. + - dupl exclusions: paths: - frontend @@ -33,8 +40,34 @@ linters: - third_party$ - builtin$ - examples$ + rules: + # Test twins (a Go test asserting the same shape twice) are the + # documented-noise class Sonar's own defaults also exempt. + - path: _test\.go + linters: [dupl] + # The per-entity-kind seed/CRUD shape: each Configure entity + # repeats the same reconcile/upgrade body deliberately (goal + # 0109's measurement found every cluster here); collapsing them + # behind generics was considered and rejected -- the repetition + # is per-entity clarity, not an accident. + - path: internal/services/configuresvc/ + linters: [dupl] + - path: internal/services/atlassvc/atlasservice_builtin\.go + linters: [dupl] settings: + dupl: + threshold: 150 + # gocognit is deliberately NOT in `enable:` above -- 25 legacy + # production functions sit over its threshold (max 65; burn-down + # list in docs/goals/0109), so a repo-wide gate would either fail + # every build or need a threshold too high to mean anything. + # Instead it gates NEW/CHANGED code only (Sonar's own + # clean-as-you-code posture), via a dedicated + # `--enable-only gocognit --new-from-merge-base` invocation in + # lefthook and CI, which reads this threshold. + gocognit: + min-complexity: 15 revive: rules: # This repo has never doc-commented every exported symbol -- diff --git a/lefthook.yml b/lefthook.yml index a79c2170..cc43804d 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -56,6 +56,15 @@ pre-commit: # so a local pass here was checking something CI doesn't and a # local fail could block a commit CI's real gate would accept. run: golangci-lint run . ./internal/... --build-tags=server + - name: gocognit-new + glob: "*.go" + # Cognitive complexity gates NEW/CHANGED code only (goal 0109, + # Sonar's clean-as-you-code posture): 25 legacy functions sit + # over the threshold (burn-down in the goal file), so the gate + # scopes to the merge-base diff instead of failing every build + # or inflating the threshold into meaninglessness. Threshold + # lives in .golangci.yml's gocognit settings. + run: golangci-lint run . ./internal/... --build-tags=server --enable-only gocognit --new-from-merge-base=origin/main - name: go-test glob: "*.go" # Root package (`.`) included alongside ./internal/... -- the