From 1aaae2c675b34c7a5b0ff1a719e62215f60a0ab3 Mon Sep 17 00:00:00 2001 From: Ali Al Dallal Date: Tue, 18 Aug 2026 13:53:54 -0400 Subject: [PATCH 1/3] ci: quality gates phase 1 -- dupl repo-wide, gocognit clean-as-you-code (goal 0109) dupl@150 enabled with named deliberate-family exclusions (test twins, configuresvc's per-entity seed/CRUD shape, atlasservice_builtin.go) -- lands green from a full measurement run, fails on NEW duplication anywhere else. gocognit@15 gates NEW/CHANGED code only via --enable-only + --new-from-merge-base (lefthook gocognit-new job + CI step; lint job checkout gains fetch-depth 0 for the merge-base) -- 25 legacy production functions sit over threshold, so a repo-wide gate would fail every build or mean nothing. TS-side sonarjs + diff-cover deferred behind the in-flight npm dependency batch. Claude-Session: https://claude.ai/code/session_01FW5GkkAG8du7tNdYLk2zSd Co-Authored-By: Claude Fable 5 --- .claude/rules/testing.md | 26 ++++++++++++++++++++++++++ .github/workflows/ci.yml | 11 +++++++++++ .golangci.yml | 33 +++++++++++++++++++++++++++++++++ lefthook.yml | 9 +++++++++ 4 files changed, 79 insertions(+) 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..195729bb 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] 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 From 54d67561cbae6fdc619ed0d45f31cf25d9ea19a2 Mon Sep 17 00:00:00 2001 From: Ali Al Dallal Date: Tue, 18 Aug 2026 14:24:44 -0400 Subject: [PATCH 2/3] ci: cache Playwright browsers -- the uncached install's apt phase stalls on flaky mirrors Three e2e shards hung for their full 15-minute cap in one day, every one inside `npx playwright install --with-deps chromium` while apt fetched from ubuntu mirrors. Browser binaries now cache keyed by the installed @playwright/test version; a hit skips the install entirely (the runner image already carries Chromium's system deps), so only a real Playwright bump pays the download path again. Claude-Session: https://claude.ai/code/session_01FW5GkkAG8du7tNdYLk2zSd Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 195729bb..f82bd957 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -368,7 +368,24 @@ 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 }} + - if: steps.pw-cache.outputs.cache-hit != 'true' + run: npx playwright install --with-deps 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 From 69beeee4ea7af96239ed87b7afb4c28c3de8783b Mon Sep 17 00:00:00 2001 From: Ali Al Dallal Date: Tue, 18 Aug 2026 14:43:52 -0400 Subject: [PATCH 3/3] ci: drop the apt phase from Playwright install -- the runner image already has the deps The 15-minute shard hangs persisted on cache-miss runs: the stall is --with-deps' apt fetch itself, not the browser download. The ubuntu runner image ships Chromium's system libraries, so the install is browser-binaries-only now (and skipped entirely on a cache hit). Claude-Session: https://claude.ai/code/session_01FW5GkkAG8du7tNdYLk2zSd Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f82bd957..4e42d6cb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -384,8 +384,13 @@ jobs: 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 --with-deps chromium + 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