docs(quality): Phase 6A critical-audit plan + Phase 7 additions (both gated to 2026-06-16) - #3529
diegosouzapw wants to merge 14 commits into
Conversation
…0), tier npm audit, wire orphaned contract gates, re-enable cheap husky pre-commit
…and provider-consistency gate - collect-metrics.mjs: emits quality-metrics.json (ESLint warnings + coverage when present) - quality-baseline.json: frozen baseline (eslintWarnings=3482, regression-only) - ci.yml: quality-gate job (ratchet + step summary + artifact) and check:provider-consistency in lint job - check-provider-consistency.ts: every REGISTRY id must be a canonical provider (found krutrim half-registered → allowlisted as known pre-existing, blocks any NEW orphan) - TDD: 9 tests (5 ratchet + 4 provider-consistency)
…pi-routes, deps allowlist
- check-fetch-targets: every dashboard fetch(/api/...) resolves to a real route.ts; found 7 pre-existing dashboard->route mismatches frozen as KNOWN_MISSING for triage
- check-openapi-routes: every openapi.yaml path resolves to a real route; found 1 stale spec entry (agent-bridge agents/{id}/state) frozen as KNOWN_STALE_SPEC
- check-deps: anti-slopsquatting allowlist (105 deps); new deps need explicit human-reviewed entry
- all wired into CI lint/docs jobs; TDD +12 tests (21 total across 5 gates)
… cap 800 for new) - check-file-size.mjs: frozen files can only shrink; new files must be <= cap (kills the next 12k-line god-component) - file-size-baseline.json: 91 files frozen at current LOC (largest 12883) - wired into CI lint job; TDD 5 tests; --update ratchets the baseline down on shrink
- check-duplication.mjs: runs jscpd@4 (pinned; v5 is an incompatible Rust rewrite) over src+open-sse, fails if duplication % rises vs frozen baseline (5.72%, measured: 1358 clones / 22967 dup lines). Targets the executor copy-paste (48/50 override execute() wholesale) - wired into the parallel quality-gate CI job (off the lint critical path); TDD 4 tests; --update ratchets down - snapshot now complete: coverage ~82.6%, eslint 3482 (98.5% no-explicit-any), duplication 5.72%, 91 files >800 LOC
- check-test-masking.mjs: for each MODIFIED test file in a PR, flags net assert removal + new assert.ok(true) tautologies (base...HEAD diff). Directly enforces CLAUDE.md 'never weaken asserts to go green' - wired into pr-test-policy CI job (reuses base fetch); no-op outside PR; TDD 5 tests
…nsumes merged coverage)
- quality-baseline.json: coverage.{statements,lines,functions,branches} floors (80/80/82/73, real ~82.58/82.58/84.23/75.22 with margin; tighten via --update after a green main run)
- check-quality-ratchet.mjs: --allow-missing (local quality:gate skips coverage.* without a coverage run; CI runs strict)
- ci.yml quality-gate job: needs test-coverage + downloads merged coverage-report so the ratchet enforces 'coverage cannot drop'
- TDD +1 test (6 total)
…symbols, route-guard, complexity, docs-symbols, db-rules) Deterministic gates, each freezing pre-existing violations in a documented allowlist (ratchet) so they pass now and block only NEW regressions: - check-error-helper (Rule #12): 7 executors/handlers forwarding raw err.message frozen - check-public-creds (Rule #11): 5 literal client_ids (Claude/Codex/Qwen/Kimi/Copilot) frozen - check-migration-numbering: gaps 026/055 + dup 041 frozen (prevents the git-rm-deleted-migration incident) - check-known-symbols: 93 executors conformance + 15 combo strategies + 18 translator pairs - check-route-guard-membership (#15/#17): all 25 spawn-capable routes verified local-only (0 gaps) - check-complexity: cyclomatic>15 / fn-length>80 ratchet (baseline 1739) - check-docs-symbols: 30 stale doc /api refs frozen (docs hallucination) - check-db-rules (#2/#5): 25 unexported db modules + 15 raw-SQL routes frozen Wired into CI (lint / docs-sync-strict / quality-gate jobs). 115 TDD tests, all green. ESLint ratchet held at 3482.
…oling) — GATED to 2026-06-16 Stored, not active. 7 suggested gates + all discussed OSS/Community tools (SonarQube Community + osv-scanner + CodeQL + knip + sonarjs + type-coverage + lockfile-lint + Stryker + size-limit + axe-core + semcheck + agent-lsp + Qlty). Activation gate: do not start before 2026-06-16 (use Phases 0-6 in production for 1 week, validate in practice, then evolve).
…stale-allowlist enforcement, scope gaps) + Fase 7 additions (gitleaks, actionlint+zizmor, license compliance) — both GATED to 2026-06-16
|
Superseded by the clean cherry-pick PR from the release tip — this one showed 47 files in the Files tab (stale merge-base after the #3471 squash) even though the real content delta is only the 2 planning docs. Closing in favor of the new PR. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a comprehensive quality gate and anti-hallucination framework, introducing various static checks for code complexity, duplication, file size, dependency allowlisting, route guard membership, and public credentials security, alongside a generic quality ratchet motor and extensive unit tests. The review feedback is highly constructive and should be fully addressed: it identifies a critical duplicate declaration syntax error in collect-metrics.mjs, recommends using Object.hasOwn instead of the in operator across several scripts to avoid prototype lookup bugs, suggests regex improvements in check-public-creds.mjs to prevent quoted-key bypasses, and offers minor compatibility and robustness enhancements for path and route resolution.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const p = path.join(cwd, "coverage", "coverage-summary.json"); | ||
| if (!fs.existsSync(p)) return; | ||
| const t = JSON.parse(fs.readFileSync(p, "utf8")).total; | ||
| out["coverage.statements"] = t.statements.pct; | ||
| out["coverage.lines"] = t.lines.pct; | ||
| out["coverage.functions"] = t.functions.pct; |
There was a problem hiding this comment.
The variable t is declared twice within the same block scope (on line 31 and line 36), which will cause a SyntaxError: Identifier 't' has already been declared and crash the script immediately when executed. Remove the duplicate declaration.
| const p = path.join(cwd, "coverage", "coverage-summary.json"); | |
| if (!fs.existsSync(p)) return; | |
| const t = JSON.parse(fs.readFileSync(p, "utf8")).total; | |
| out["coverage.statements"] = t.statements.pct; | |
| out["coverage.lines"] = t.lines.pct; | |
| out["coverage.functions"] = t.functions.pct; | |
| const t = JSON.parse(fs.readFileSync(p, "utf8")).total; | |
| out["coverage.statements"] = t.statements.pct; | |
| out["coverage.lines"] = t.lines.pct; | |
| out["coverage.functions"] = t.functions.pct; | |
| out["coverage.branches"] = t.branches.pct; |
| import fs from "node:fs"; | ||
| import path from "node:path"; |
There was a problem hiding this comment.
Import pathToFileURL from node:url to safely resolve the main module execution path across different operating systems.
| import fs from "node:fs"; | |
| import path from "node:path"; | |
| import fs from "node:fs"; | |
| import path from "node:path"; | |
| import { pathToFileURL } from "node:url"; |
| import fs from "node:fs"; | ||
| import path from "node:path"; |
There was a problem hiding this comment.
Import fileURLToPath from node:url to safely resolve the directory name in ESM.
import fs from "node:fs";
import path from "node:path";
import { fileURLToPath } from "node:url";| // --- Real dataset: the frozen allowlists must keep the live dir green --- | ||
|
|
||
| test("the real migrations dir produces ZERO anomalies under the frozen allowlists", () => { | ||
| const dir = path.resolve(import.meta.dirname, "../../src/lib/db/migrations"); |
There was a problem hiding this comment.
Using import.meta.dirname requires Node.js >= 20.11.0, which can fail on older Node 20 LTS environments. Use path.dirname(fileURLToPath(import.meta.url)) for maximum compatibility and consistency with the rest of the codebase.
| const dir = path.resolve(import.meta.dirname, "../../src/lib/db/migrations"); | |
| const dir = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../../src/lib/db/migrations"); |
| const violations = []; | ||
| const improvements = []; | ||
| for (const [file, loc] of Object.entries(currentLocByFile)) { | ||
| if (file in frozen) { |
There was a problem hiding this comment.
Using the in operator can lead to prototype lookup bugs if a file path matches a built-in property of Object.prototype (e.g., toString). Use Object.hasOwn for a safe and robust lookup.
| if (file in frozen) { | |
| if (Object.hasOwn(frozen, file)) { |
| apiPaths: string[], | ||
| isLocalOnly: (path: string) => boolean, | ||
| allowlist: Record<string, string> | ||
| ): string[] { |
There was a problem hiding this comment.
Using the in operator is prone to prototype lookup bugs (e.g., matching toString). Use Object.hasOwn for safe lookup.
return apiPaths.filter((p) => !isLocalOnly(p) && !Object.hasOwn(allowlist, p));| (s) => !canonicalSet.has(s) && !(s in implicitDefaults) | ||
| ); | ||
| return { canonicalNotHandled, handledNotCanonical }; |
There was a problem hiding this comment.
Using the in operator is prone to prototype lookup bugs (e.g., matching toString). Use Object.hasOwn for safe lookup.
const handledNotCanonical = [...handled].filter(
(s) => !canonicalSet.has(s) && !Object.hasOwn(implicitDefaults, s)
);| const CRED_KEY_RE = | ||
| /(?:^|[\s{,([])(clientIdDefault|clientSecretDefault|clientId|clientSecret|apiKeyDefault|apiKey)\s*:/; |
There was a problem hiding this comment.
The CRED_KEY_RE regex does not match quoted keys (e.g., "clientIdDefault":), which allows simple bypasses of the public credentials check. Update the regex to support optional matching single or double quotes around the key names.
| const CRED_KEY_RE = | |
| /(?:^|[\s{,([])(clientIdDefault|clientSecretDefault|clientId|clientSecret|apiKeyDefault|apiKey)\s*:/; | |
| const CRED_KEY_RE = | |
| /(?:^|[\\s{,([])(["']?)(clientIdDefault|clientSecretDefault|clientId|clientSecret|apiKeyDefault|apiKey)\\2\\s*:/; |
References
- Never commit secrets or credentials. Passwords, OAuth secrets, API keys, and Cookie values must never appear in committed files. (link)
| const effLen = catchAll ? rnorm.indexOf("") : rnorm.length; | ||
| if (!catchAll && segs.length > rnorm.length) continue; // doc mais profunda que a rota | ||
| if (catchAll && segs.length < effLen) continue; | ||
| const cmpLen = Math.min(segs.length, effLen || rnorm.length); |
There was a problem hiding this comment.
If effLen is 0, effLen || rnorm.length evaluates to rnorm.length. Although effLen is currently at least 1 because all routes start with api/, using catchAll ? effLen : rnorm.length is much safer and more robust against future changes.
| const cmpLen = Math.min(segs.length, effLen || rnorm.length); | |
| const cmpLen = Math.min(segs.length, catchAll ? effLen : rnorm.length); |
Code Review SummaryStatus: PR Closed/Superseded | Recommendation: No action required This PR (#3529) is already closed and superseded by a clean cherry-pick PR from the release tip. The PR author noted it showed 47 files due to stale merge-base but the real content delta was only 2 planning documents. No code changes were present in this PR for review. The diff shown contained test files that would verify quality gate scripts, but these changes are superseded. Files in Original Diff
|
Code Review SummaryStatus: PR Closed/Superseded | Recommendation: No action required This PR (#3529) is already closed and superseded by a clean cherry-pick PR from the release tip. The PR author noted it showed 47 files due to stale merge-base but the real content delta was only 2 planning documents. Existing Review Feedback9 inline comments were already posted on this PR by gemini-code-assist[bot]. I concur with all findings:
Since this PR is superseded, these issues should be addressed in the replacement PR. Files in Original Diff
Reviewed by laguna-m.1-20260312:free · 867,369 tokens |
1 participant
What
Two planning documents only (no code changes) — follow-up to #3471 (Phases 0-6, merged):
PLANO-QUALITY-GATES-FASE6A.md(new) — a critical self-audit of the Phases 0-6 implementation. Inline re-read of all 18 gates + ratchet engine + CI wiring found real gaps, frozen as a 12-task plan:tests/unit/**subdirectories are not collected by ANY runner (test:unit/test:coverage/CI shards use a non-recursive glob; vitest only includesautoCombo). Sample run proves rot: 2 asserts intests/unit/authz/routeGuard.test.ts(Hard Rules feat(npm): add npm package publishing with CLI entry point #15/fix(ci): explicit .npmrc auth for npm publish #17 surface) fail today, unseen.test:vitest, contradicting CLAUDE.md "both runners must pass").KNOWN_*entry stops being needed — fixed violations can silently regress back (pattern validated against Notion's ratcheting system + ESLint--report-unused-disable-directives).--require-tighten, per-metric eps), scope expansions (fetch-targets beyond(dashboard), error-helper to MCP/routes, deps to all workspace manifests incl. the published@omniroute/opencode-plugin, known-symbols to MCP tools/A2A skills/cloud agents), test-masking v2 (deleted files,.skip/.only), jscpd pinning, ops hygiene.PLANO-QUALITY-GATES-FASE7.md(updated) — 3 community/OSS tools added from the audit research: gitleaks (secret scanning; Betterleaks noted as drop-in successor), actionlint + zizmor (the 10 workflows currently have zero validation; motivated by the 2026-03 trivy-action/LiteLLMpull_request_targetincident), license compliance (SPDX allowlist; project is MIT). Pre-conditions and handoff now order Phase 6A before Phase 7.Activation gate
Both documents are stored, not active — same owner-set gate: do not start before 2026-06-16 (one week of production signal from Phases 0-6 first). Tasks 6A.1/6A.2 are flagged as pre-existing bugs the owner may choose to fast-track.
Validation
Docs-only change; no tests required (no production code touched).
🤖 Generated with Claude Code