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

name, description

name

description

mvp-change-control

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:

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:

# 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