From fbe74dcf04b71ca423c0fc7568534493418f13a2 Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Wed, 17 Jun 2026 14:49:22 -0300 Subject: [PATCH] ci(quality): dedup the doubly-run check:docs-sync + record validated ROI backlog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Onda 3 (gate ROI-review) Phase 2. Two parts, both low-risk: 1. Remove the standalone `check:docs-sync` from the `lint` job — it already runs in the `docs-sync-strict` job (via `check:docs-all`) and the husky pre-commit hook, so the `lint`-job copy was a pure duplicate. No coverage lost. 2. Update the Rationalization Backlog in QUALITY_GATES.md with trust-but-verify findings: several "obvious" merges/flips from the ROI review turned out to hide debt and are NOT clean drop-ins — - CVE merge (audit:deps→osv): different semantics (hard high/critical vs regression-ratchet) — keep both. - cycles→circular-deps: dpdm reports 91 cycles (can't promote to blocking) and is broader-scope than the green curated check:cycles — keep both. - openapi-security-tiers flip: blocked by traffic-inspector routes missing the x-loopback-only annotation. - complexity + /api merges: valid but real config/script surgery — deferred. - node-runtime ×11: ~10s savings vs a cheap guard — low ROI, skip. The remaining flips (typecheck:noimplicit, test:vitest:ui, check:secrets, pr-evidence, semgrep) are operator policy decisions, left for the owner. --- .github/workflows/ci.yml | 3 ++- docs/architecture/QUALITY_GATES.md | 19 +++++++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2237f8bb679..50372770013 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -59,7 +59,8 @@ jobs: - run: npm run check:tracked-artifacts - run: npm run check:lockfile - run: npm run check:licenses - - run: npm run check:docs-sync + # check:docs-sync is run by the docs-sync-strict job (via check:docs-all) and the + # husky pre-commit hook; the standalone copy here was redundant (ROI dedup). - run: npm run typecheck:core # typecheck:noimplicit:core is a forward-looking gate (noImplicitAny). # Run informationally for now — many pre-existing call sites still need diff --git a/docs/architecture/QUALITY_GATES.md b/docs/architecture/QUALITY_GATES.md index 39bd6930c19..cd41b1ad2c6 100644 --- a/docs/architecture/QUALITY_GATES.md +++ b/docs/architecture/QUALITY_GATES.md @@ -253,20 +253,23 @@ currently orphaned. ### Merge / dedup (mechanical, lower risk) -- **CVE scanning** — `audit:deps` (npm audit) overlaps `check:vuln-ratchet` (osv-scanner) and Dependabot. Make osv canonical; keep only the `electron/` npm-audit slice if osv misses it. -- **Complexity** — `check:complexity` (core ESLint) and `check:cognitive-complexity` (sonarjs) spawn two full ESLint passes over `src` + `open-sse`. Merge into one config/one tree-walk emitting both metrics. -- **Cycle detection** — `check:cycles` (custom 5-subtree AST) is dominated by `check:circular-deps` (dpdm: path-aliases + transitive). Promote dpdm to blocking, drop `check:cycles`. -- **`/api` anti-hallucination** — `check:openapi-routes` (spec→route) and `check:docs-symbols` (prose→route) share resolution logic; collapse to one gate with two inputs. -- **`check:docs-sync` runs twice** — standalone in the `lint` job and again inside `check:docs-all` (`docs-sync-strict`). Drop the standalone invocation. -- **`check:node-runtime` runs in 11 jobs** — each is a separate runner with `setup-node` already pinning the version; keep it where it gates tsx-loading gates, drop the rest. +Each candidate was validated against the live gate state on 2026-06-17 (trust-but-verify); +several "obvious" merges turned out to hide debt and are **not** clean drop-ins. + +- **`check:docs-sync` runs twice** — standalone in the `lint` job and again inside `check:docs-all` (`docs-sync-strict`) and the husky pre-commit hook. ✅ **DONE** — standalone `lint` invocation removed. +- **CVE scanning** — ❌ **NOT a clean merge.** `audit:deps` hard-fails on any high/critical CVE; `check:vuln-ratchet` (osv) only fails on a *regression* vs baseline (currently 1 MODERATE). Different semantics — dropping `audit:deps` would lose the absolute high/critical gate. Keep both. +- **Cycle detection** — ❌ **NOT a clean merge.** `check:circular-deps` (dpdm) reports **91 cycles** (that is why it is advisory); it cannot be promoted to blocking without first resolving them, and it has a broader scope than the green, curated `check:cycles`. Keep `check:cycles` blocking; resolving the 91 dpdm cycles is its own backlog. +- **Complexity** — ⏳ valid but real surgery. `check:complexity` (core ESLint) + `check:cognitive-complexity` (sonarjs) are two ESLint passes over `src` + `open-sse`; merging into one config emitting both metrics needs careful ratchet re-wiring. Deferred. +- **`/api` anti-hallucination** — ⏳ valid but script surgery. `check:openapi-routes` (spec→route) + `check:docs-symbols` (prose→route) share resolution logic; collapsing them is a non-trivial script change. Deferred. +- **`check:node-runtime` runs in 11 jobs** — ⚠️ **low ROI.** Each is a separate runner and the check is <1s; total savings ~10s, against losing a cheap per-job guard. Not worth the churn. ### Flip / decide (operator policy) +- `check:openapi-security-tiers` (advisory) — ❌ **NOT cleanly flippable.** It exits 0 but warns that several `traffic-inspector` routes under `LOCAL_ONLY_API_PREFIXES` lack the `x-loopback-only: true` annotation. Enforcing it requires adding those annotations to `openapi.yaml` first. - `typecheck:noimplicit:core` (advisory) — largely subsumed by the blocking `check:type-coverage` ratchet. Flip to a ratchet or drop the redundant second `tsc` pass. - `test:vitest:ui` (advisory, 14 parked fails) — fix-and-block or delete; don't leave rotting. - `check:secrets` (gitleaks, blocking ratchet frozen at 3 documented false-positives) — allowlist the 3 to reach 0, or demote to advisory. Overlaps GitHub native secret-scanning + `check:public-creds`. -- `check:openapi-security-tiers` (advisory) — block (the tier↔routeGuard invariant matters) or drop. -- `check:pr-evidence` (blocking, greps PR-body prose) — high false-positive risk; consider advisory. +- `check:pr-evidence` (blocking, greps PR-body prose) — high false-positive risk; weakens Hard Rule #18 enforcement if dropped, so this is a genuine policy call. - `semgrep` (advisory standalone) — overlaps CodeQL for the OWASP families; wire its baseline to a ratchet or drop. ---