motovaultpro/.claude/skills/mvp-change-control/SKILL.md

---
name: mvp-change-control
description: Load before making ANY change to the MotoVaultPro repo - creating issues, branches, commits, or PRs; deciding whether a change needs an issue, sub-issues, a plan, or owner sign-off; reviewing code (RULE 0/1/2 taxonomy and verdicts); setting Gitea status/type labels; or asking "is this safe to merge / can I run this migration / can I run make clean". Symptom keywords - "create a PR", "what branch name", "commit message format", "which label", "sub-issue", "quality review", "RULE 0", "is CI green", "run the integration tests", "drop database", "edit file on staging server".
---

# MotoVaultPro Change Control

How changes are classified, gated, and shipped in this repo. Applies to human engineers and AI coding sessions equally. Reality check first: development here is done by AI sessions; the human owner reviews PRs; end-to-end verification happens on STAGING via the PR deploy pipeline because there is no fully working local dev loop.

## When to use / When NOT to use

Use this skill when you are about to create an issue, branch, commit, or PR; when deciding how big a change's process footprint should be; when performing or requesting a quality review; or before any operation that could destroy data or touch a server.

Do NOT use this skill for:
- Diagnosing a failure -> `mvp-debugging-playbook`
- "Has this been tried before / why is it this way" -> `mvp-failure-archaeology`, `mvp-architecture-contract`
- Running tests, what counts as evidence, mobile+desktop validation -> `mvp-validation-and-qa`
- Setting up a working environment or the local unit-test loop -> `mvp-build-and-env`
- Deploying, rolling back, backup/restore mechanics -> `mvp-run-and-operate`
- Fixing the CI pipeline itself -> `mvp-deploy-safety-campaign`

## 1. The change workflow as actually practiced

Issues are the source of truth. Gitea (self-hosted Git forge) at `git.motovaultpro.com`, owner `egullickson`, repo `motovaultpro`. Use the Gitea MCP tools (`mcp__gitea-mcp__*`) for all issue/label/branch/PR operations. Sprints and milestones are ABANDONED (owner directive 2026-05-12) - work flows directly from issues by priority. The `.ai/workflow-contract.json` still describes sprints and per-sub-issue status labels; on both points it is stale and `CLAUDE.md` wins.

### Standard flow

1. Find or create an issue. Set exactly one `status/*` and one `type/*` label.
2. Move it to `status/in-progress` (REPLACE the old status label - see label discipline below).
3. Branch off `main`: `issue-{N}-{slug}`, e.g. `issue-246-reorder-log-fuel-fields`.
4. Commit as `{type}: {summary} (refs #{N})`, e.g. `feat: add fuel report (refs #42)`. Allowed types: `feat`, `fix`, `chore`, `docs`, `refactor`, `test`.
5. Self-gate locally (Section 5) - CI will not catch anything for you.
6. Open ONE PR targeting `main`. Title `{type}: {summary} (#{N})`. Body must contain `Fixes #{N}` (and `Fixes #M` per sub-issue, one per line). Template: `.gitea/PULL_REQUEST_TEMPLATE.md` - its test-plan checkboxes are honor-system, so fill them honestly.
7. Move the issue to `status/review`. Owner reviews and merges (squash or rebase both acceptable - `merge_policy: squash_or_rebase_ok` in `.ai/workflow-contract.json`).
8. After merge: `status/done`. Update docs if behavior or interfaces changed (`mvp-docs-and-writing`).

Honest note: direct pushes to `main` occur and are tolerated for trivial fixes - `git log --first-parent --no-merges main` shows commits like `fix: Guide formatting`, `chore: Update Images`. That is the exception, not the license. Anything substantive - code behavior, schema, config, workflows, dependencies - goes issue -> branch -> PR. A direct push still triggers the full staging deploy pipeline, so it is not "safer" than a PR, just unreviewed.

Side effect to know: every PR push deploys over the SINGLE shared staging environment (last PR wins). Two open PRs fight over staging; coordinate accordingly.

### Sub-issue decomposition (3+ files)

Multi-file changes (3 or more files) must be decomposed into sub-issues so each unit fits a small AI context window:

| Rule | Convention |
|------|------------|
| Sub-issue title | `{type}: {summary} (#{parent_index})` - parent index in the title |
| Sub-issue body | First line: `Relates to #{parent_index}`; each sub-issue must be executable standalone |
| Branches | ONE branch for the parent only (`issue-{parent}-{slug}`). Never a branch per sub-issue |
| PRs | ONE PR for the parent. Body lists `Fixes #N` for parent AND every sub-issue |
| Commits | Reference the specific sub-issue: `feat: add dashboard (refs #107)` |
| Status labels | Tracked on the PARENT only. Sub-issues stay `status/backlog`. (workflow-contract.json says otherwise; it is wrong) |
| Plan milestones | Map 1:1 to sub-issues |

### Label discipline

Exactly one `status/*` and one `type/*` per issue. When changing status, REPLACE - do not stack. Prefer `mcp__gitea-mcp__replace_issue_labels` (atomic) over add/remove pairs. Gitea marks most of these labels exclusive but `status/blocked` is not, so the tool cannot fully save you from stacking.

Label IDs (verified against the live repo 2026-07-07):

| Label | ID | Label | ID |
|-------|----|-------|----|
| status/backlog | 8 | status/blocked | 12 |
| status/ready | 9 | status/done | 13 |
| status/in-progress | 10 | type/feature | 14 |
| status/review | 11 | type/bug | 15 |
| | | type/chore | 16 |
| | | type/docs | 17 |

Lifecycle: `status/backlog` -> `status/ready` -> `status/in-progress` -> `status/review` -> `status/done` (`status/blocked` from any state).

## 2. Change classification

| Change | Issue? | Sub-issues? | Written plan? | Owner sign-off before executing? |
|--------|--------|-------------|---------------|----------------------------------|
| Typo, doc wording, image asset | Optional (direct push tolerated) | No | No | No |
| Single-file bug fix | Yes | No | No | No (PR review suffices) |
| Feature or fix touching 1-2 files | Yes | No | No | No |
| Feature touching 3+ files | Yes | Yes (Section 1) | Yes - plan posted as issue comments, milestones 1:1 with sub-issues | No, unless a row below also applies |
| New/changed DB schema migration | Yes | If 3+ files | Yes | YES - migrations have no rollback (verified: 50 migration SQL files under `backend/src/features/*/migrations/`, zero down/rollback scripts). Fresh backup first (Section 4) |
| Deploy pipeline (`.gitea/workflows/`, `scripts/ci/`, compose files, Traefik config) | Yes | If 3+ files | Yes | YES - a bad workflow deploys itself; staging.yaml runs on every PR push |
| Anything touching a non-negotiable (destructive DB ops, server-side files) | Yes | - | Yes | YES - always |
| Dependency major-version bumps, secrets/config axis changes | Yes | If 3+ files | Recommended | Recommended (see `mvp-config-and-secrets`) |

"Plan" means: decomposition into milestones with acceptance criteria, posted as comments on the issue/sub-issues, reviewed before implementation. "Owner sign-off" means: state the intent and blast radius on the issue or PR and get explicit approval before executing - not after.

## 3. Quality-review taxonomy: RULE 0/1/2

Carried forward from the retired agent library (`git show HEAD:.claude/role-agents/quality-reviewer.md` - now deleted from the working tree; this section is the living copy). Apply it to every substantive PR, whether you are the author self-reviewing or a reviewer session.

Override order: RULE 0 overrides RULE 1; RULE 1 overrides RULE 2.

### RULE 0: Production Reliability (CRITICAL/HIGH)

- Unhandled errors causing data loss or corruption
- Security vulnerabilities (injection, auth bypass)
- Resource exhaustion (unbounded loops, leaks)
- Race conditions affecting correctness
- Silent failures masking problems

Verification: use OPEN questions ("What happens when X fails?"), not yes/no.
CRITICAL findings require dual-path verification: reason forward from the code to the failure AND backward from the claimed failure to the code before flagging.

### RULE 1: Project Conformance (HIGH)

MotoVaultPro-specific standards:

- Mobile + desktop validation required
- snake_case in DB, camelCase in TypeScript
- Feature capsule pattern (`backend/src/features/{feature}/` - 21 capsules as of 2026-07-07)
- Repository pattern with `mapRow()` for case conversion (and numeric coercion - never return raw pg rows; see `mvp-failure-archaeology`)
- CI/CD pipeline must pass

Verification: cite the specific standard from `CLAUDE.md` or project docs before flagging. No citation, no RULE 1 finding.

### RULE 2: Structural Quality (SHOULD_FIX/SUGGESTION)

- God objects (>15 methods or >10 dependencies)
- God functions (>50 lines or >3 nesting levels)
- Duplicate logic (copy-pasted blocks)
- Dead code (unused, unreachable)
- Inconsistent error handling

Verification: confirm project docs do not explicitly permit the pattern.

### Severity and verdicts

Severity: CRITICAL (data loss, security breach, system failure) > HIGH (production reliability or project standard violation) > SHOULD_FIX (structural) > SUGGESTION (improvement opportunity).

Review output format:

```
## VERDICT: [PASS | PASS_WITH_CONCERNS | NEEDS_CHANGES | CRITICAL_ISSUES]

## Findings
### [RULE] [SEVERITY]: [Title]
- Location: [file:line]
- Issue: [What is wrong]
- Failure Mode: [Why this matters]
- Suggested Fix: [Concrete action]

## Considered But Not Flagged
[Items examined but not issues, with rationale]
```

The "Considered But Not Flagged" section is mandatory - it proves the review looked at the risky spots and keeps false positives down on re-review. Before flagging anything: read the relevant standards (RULE 1 scope), skip risks already acknowledged in the plan's Known Risks, and only file findings that are actionable with a specific fix.

Calibration: this is a PRE-LAUNCH product heading toward paying users. RULE 0 findings on billing (subscriptions/Stripe), auth, or data integrity are launch blockers, not debt.

## 4. The non-negotiables (owner-set, 2026-07-07)

These are absolute. Never route around them, never "just this once".

### 4.1 No destructive database operation without a fresh backup

Take the backup IMMEDIATELY before the operation:

```bash
make db-backup    # runs scripts/export-database.sh --output backup_<timestamp>  (Makefile:208)
```

Rationale and what counts as destructive:

- `make clean` runs `docker compose down -v --rmi all` (Makefile:73-76). The `-v` DESTROYS the PostgreSQL data volume. It reads like a tidy-up command; it is a database wipe.
- `scripts/import-database.sh --drop-existing` executes `DROP DATABASE IF EXISTS` before importing.
- The backend integration tests are destructive to the SHARED dev database: e.g. `backend/src/features/vehicles/tests/integration/vehicles.integration.test.ts` runs the real migration SQL in `beforeAll` and `DROP TABLE IF EXISTS vehicles CASCADE` plus function drops in `afterAll` (the admin integration test does the same for its tables). Running `npm test` in the backend hits these. Backup first, or point the tests at a throwaway database.
- Schema migrations on staging/prod have NO rollback path - there are no down migrations anywhere in the repo. The only undo is restoring a backup. The production maintenance workflow (`.gitea/workflows/maintenance.yaml`) has a `create_backup` input defaulting to `yes` - never set it to `no`.

### 4.2 Never hand-edit files on the staging/prod servers

Every deploy runs `rsync -av --delete` from the fresh checkout over `$DEPLOY_PATH/config/` and `$DEPLOY_PATH/scripts/` (`.gitea/workflows/staging.yaml:117-118`, `.gitea/workflows/production.yaml:108-109`). `--delete` means any file you create or edit by hand on the server under those paths is silently reverted or removed on the NEXT deploy - which on staging is the next PR push by anyone. Application code is baked into Docker images, so hand edits there do not survive a container restart either. All server changes flow through the repo and the workflows.

Known standing exception (do not "fix" it): the Docker prune cron on the staging runner host is deliberately out-of-band and NOT in the repo - it manages runner disk, not the application.

## 5. What "green" honestly means, and the author self-gate

CI reality: the PR pipeline gates nothing beyond build + boot - a PR can be "green" with failing tests and lint errors, and root `CLAUDE.md`'s contrary CI claims are false (canonical statement and details: `mvp-validation-and-qa` section 1; fixing the gap is `mvp-deploy-safety-campaign`).

Therefore the AUTHOR is the gate. Before opening a PR, run and honestly report in the PR body:

```bash
# NOTE: root package.json has NO scripts - npm test at repo root fails. Run per package.
cd backend  && npm run lint && npm run type-check
cd frontend && npm run lint && npm run type-check
# or both at once: make lint && make type-check   (Makefile:226-244, no Docker needed)

cd backend && npm test -- --forceExit        # unit tests; --forceExit because open pg/redis
                                             # handles otherwise hang jest (known trap)
```

Container-only pieces (see `mvp-build-and-env` for setup and traps):

| Check | Where it works |
|-------|----------------|
| backend lint / type-check / unit tests | Host machine (unit tests: with `--forceExit`) |
| backend integration tests | Currently runnable NOWHERE as-shipped: the backend production image has no jest (`npm ci --omit=dev`, dist-only), so docs/TESTING.md's `make shell-backend` + `npm test` recipe fails. Requires a builder-stage image or a host run with ephemeral Postgres/Redis - see `mvp-deploy-safety-campaign` 1B. DESTRUCTIVE, see Section 4.1 |
| frontend lint / type-check | Host machine |
| frontend jest | Host machine via `npx jest --reporters=default` (`npm test` is broken by the tdd-guard-jest reporter + hardcoded Linux `projectRoot`; no container run exists - nginx-only image, tests dockerignored). Mechanics and known-red baseline: `mvp-build-and-env` |
| ocr pytest | `cd ocr && python -m pytest` with deps installed; never runs in CI |

This checklist is the minimum bar to open a PR. The full evidence bar - integration tests, mobile (320/768px) + desktop (1920px) validation on staging, definition of done - is `mvp-validation-and-qa`.

## Provenance and maintenance

Authored 2026-07-07 against repo HEAD e729d42 and the live Gitea instance. Volatile facts and how to re-verify each:

- Label names/IDs: `mcp__gitea-mcp__list_repo_labels` (owner egullickson, repo motovaultpro)
- CI runs no tests/lint: `grep -nE "npm (run )?(test|lint|type-check)|jest|eslint|pytest" .gitea/workflows/*.yaml` (expect no hits)
- rsync --delete lines: `grep -n "rsync" .gitea/workflows/staging.yaml .gitea/workflows/production.yaml`
- make clean destroys volumes: `sed -n '73,77p' Makefile`
- No rollback migrations: `find backend/src -path "*migrations*" -iname "*down*" -o -path "*migrations*" -iname "*rollback*"` (only false positive: `002_add_vehicle_dropdown_fields.sql`)
- Integration-test destructiveness: `grep -rn "DROP TABLE" backend/src/features/*/tests/integration/`
- Feature capsule count (21): `ls backend/src/features | grep -v CLAUDE.md | wc -l`
- Root package.json has no scripts: `python3 -c "import json; print(json.load(open('package.json')).get('scripts'))"`
- Frontend `npm test` broken on host (fallback: `npx jest --reporters=default`): `sed -n '28,36p' frontend/jest.config.ts` (tdd-guard-jest reporter, hardcoded projectRoot)
- Merge policy / sub-issue conventions: `python3 -m json.tool .ai/workflow-contract.json` (remember: stale on sprints and sub-issue labels; CLAUDE.md wins)
- RULE 0/1/2 original text: `git show e729d42:.claude/role-agents/quality-reviewer.md`
- Direct-push tolerance: `git log --first-parent --no-merges --oneline main | head -20`