Code Quality Bar

Code quality bar

This page states the quality bar a change has to clear, and how to check each part of it locally. It is for anyone preparing a pull request.

The short version, from the README: nothing merges on a green tick alone. The bar is a clean board.

The bar

Dimension	Target	Enforced by
Lint	Zero errors, zero warnings	ESLint 9 flat config, per workspace, and CI
Types	Zero errors under strict mode	tsc, per workspace, and CI
Formatting	Prettier clean	The pre-commit hook and pnpm format:check
Coverage	Per-app floors, moving to 95	CI, on the merged coverage map
Sonar issues	Zero new issues, zero new code smells	Sonar quality gate, when enabled
Security hotspots	Zero unreviewed	Sonar, CodeQL
Duplication	No new duplication	Sonar
React health	95 or above	pnpm doctor, by hand
Secrets	Zero	secretlint locally, Gitleaks in CI
Dependencies	No new advisory at moderate or above	Dependency review, grype
Licences	Allow list only	grant

Checking each one

Lint and types. Scope them:

pnpm --filter <workspace> lint
pnpm --filter <workspace> type-check

The base TypeScript config turns on strict, noUncheckedIndexedAccess, noImplicitOverride, noFallthroughCasesInSwitch, forceConsistentCasingInFileNames, isolatedModules, and verbatimModuleSyntax. noUncheckedIndexedAccess in particular means array and record access yields a possibly-undefined type. That is deliberate: silently trusting rows[0] exists is how a clinical list renders the wrong patient.

No eslint-disable comments. Fix the root cause. If a rule is genuinely wrong for this repository, change it in the flat config with a justification in the pull request. A disable comment moves the problem out of sight without solving it.

Formatting.

pnpm format          # write
pnpm format:check    # verify

Prettier owns formatting entirely; ESLint owns correctness. The pre-commit hook formats staged files, so this rarely comes up.

Coverage. See Testing strategy for the floors and the reasoning. Locally:

pnpm --filter <workspace> test

React health.

pnpm doctor

Runs react-doctor over the repository. doctor.config.json sets ignore paths only: coverage output, build output, .next, static Storybook, and the generated Prisma client. It sets no score threshold, and no CI job runs it. The 95 bar is a team convention checked by hand, not a gate. If you want it enforced, that is a pull request against .github/workflows/.

Secrets.

pnpm check:secrets

Supply chain. The commands are on Security and supply chain. They need syft, grype, and grant installed at the versions the workflow pins.

Everything at once.

pnpm verify   # lint + type-check + test + build

Sonar

Two projects, yosemitecrew_openrunic_Web and yosemitecrew_openrunic_Api, configured through sonar-project.properties in each app directory.

The Sonar stage is currently skipped. The repository variable DISABLE_SONAR is set to true because the projects do not exist yet. The aggregate check counts a skipped stage as a pass, so a green pull request today has not been Sonar-scanned.

Turning it on takes more than deleting the variable: create the projects, name each project's main branch main, add sonar-project.properties for each app, add the SONAR_TOKEN_WEB and SONAR_TOKEN_API secrets, and only then remove the variable. Naming a project's main branch dev produces the mirror-image failure of the others, which is a confusing hour to debug.

Two structural notes for when it is on. The scan runs with -Dsonar.qualitygate.wait=true, so a red gate reds the job and therefore the aggregate required check. And the scan job does no install, so rules needing type resolution from node_modules see less than they would. If that gap starts to matter, the documented answer is a scheduled full analysis as a type-aware backstop, not reinstalling on every scan.

Conventions that are part of the bar

These are not tool-enforced but are expected in review.

Comments explain why, not what. The existing codebase is unusually heavily commented, and the comments carry decisions: why lcov is not merged directly, why the React plugin is absent from the vitest config, why _offset must be a multiple of _count, why createdAt is excluded from the audit hash. Match that. A comment that restates the code is noise; a comment that records a rejected alternative saves the next person an afternoon.

Order that matters is declared as data. The middleware chain is an exported array so the suite asserts it directly. Do the same when the correctness of a sequence is the point.

Fail closed. The CI aggregate treats a cancelled dependency as a failure. The coverage merge fails on a missing floor. The audit sink throws rather than falling back to a non-transactional write. The tenant client throws rather than passing raw SQL through. When you add a guard, make its failure mode loud.

No silent drops. A rejected search parameter is an error, not a shrug. A wrongly-shaped promoted form value throws rather than being skipped. A mapper that does not carry a field lists it in a *_DROPPED_FIELDS export.

Synthetic data only, everywhere. Tests, fixtures, seeds, screenshots, logs. This is a hard rule, not a preference.

Related pages

• Testing strategy
• CI pipeline
• Security and supply chain
• Development workflow