---
name: coding-standards
description: >
  Engineering quality bar for GroomBook code: priority ordering of correctness
  vs. clarity vs. maintainability vs. performance vs. elegance, PR and test
  requirements, no-hardcoded-values rules, branch discipline, the no-self-
  merge contract, and the uat→main merge-gate policy (CTO Gitea Approve
  reserved for novel auth, infra/prod-affecting, and risk-flagged merges).
---

# Coding Standards

These rules apply to any GroomBook agent that writes, reviews, or merges code.

## Priority ordering

When making technical decisions, prioritize in this order:

1. **Correctness** — does it work? Does it handle edge cases? Have you proven it, not assumed it?
2. **Clarity** — will another engineer understand this without context in 6 months?
3. **Maintainability** — will it be safe to change?
4. **Performance** — fast enough for the use case? Profile before optimizing.
5. **Elegance** — nice if free; never trade any of the above for it.

## Pull request discipline

* All changes go through a PR. **Never push directly to `dev`, `uat`, or `main`.**
* Never merge a PR without the reviews required by the `sdlc` (or `devops`) skill for that branch. The engineer who opened the PR may click merge once those prerequisites are satisfied.
* Always include `cc @cpfarhood` at the bottom of the PR body for visibility (not as a reviewer).

## Test requirements

* **Every PR must include tests** for new code paths. No exceptions for "small" changes.
* Run unit tests, type check, and lint locally (or rely on CI) **before** requesting review.
* A PR without passing tests does not get approval.
* New code paths require coverage. No coverage = no approval.

## Code review tone

Hold a high bar. PRs with obvious mistakes, missing tests, hardcoded values, or policy violations get firm, specific review comments citing what's wrong and what the fix is. Cite the file and line. Suggest the fix when you know it. Don't sugarcoat — but be professional and constructive. "This looks wrong" is not a review comment.

## Hardcoded values

* **Colors** use CSS variables / theme tokens. Never raw hex in components.
* **Strings** use constants or i18n. No magic strings.
* **Numbers** that aren't trivially obvious go in named constants.
* **No magic numbers** in business logic.

## Secrets in code

Secrets never touch source. See the `safety` skill for the SealedSecrets workflow. If your implementation requires a Kubernetes secret you cannot create, file an issue for the agent who owns the SealedSecrets workflow rather than committing a plaintext value.

## Releases and versioning

All releases use CalVer (`YYYY.MMDD.PATCH`, e.g. `2026.0504.0`). No SemVer, no custom schemes.

## Container images

Push to `git.farh.net` only. Never Docker Hub for first-party images.

## uat→main merge-gate policy

The **CTO Gitea Approve click is not the default gate** on `uat → main` PRs. Once the four pre-gates — **QA**, **UAT deploy**, **UAT regression**, and **security review** — are all green, the engineer self-merges. A CTO Gitea Approve click is required only for PRs in one of the three categories below.

### Categories that require CTO Gitea Approve

1. **Novel auth / session paths** — login, OIDC, OOBE, session middleware, token issuance, password reset, MFA, or any new auth provider integration. Routine changes to auth-gated UI (button styling, error messages, form layout, copy edits) are **not** in this category.
2. **Infra / prod-affecting merges** — deploys, infra manifests, secrets, GitOps overlays, CI/CD pipelines, `main` branch protection, production routing/ingress, or any change that mutates prod state. **All Phase 5 infra overlay PRs (`groombook/infra`) require CTO Gitea Approve** without exception.
3. **Risk-flagged merges** — any PR that carries the `risk:cto-approve` label, or where the CTO or CEO has explicitly requested CTO sign-off in the PR or issue thread.

The engineer who opened the PR classifies it against the three categories above (escalating to the CTO via comment if the call is unclear), then:

* **In a category** → request a CTO Gitea Approve click. The engineer merges once the CTO has approved.
* **Outside all three categories** → no CTO click needed. The engineer merges once the four pre-gates are green.

The pre-gates (QA, UAT deploy, UAT regression, security) do **not** change. This rule only removes the CTO Gitea Approve click from the default `uat → main` path for routine PRs that already pass every pre-gate.

## When uncertain

If a code-quality call isn't covered above and you can't decide cleanly, escalate to the CTO via comment rather than guessing.