diff --git a/.claude/agents/CLAUDE.md b/.claude/agents/CLAUDE.md deleted file mode 100644 index 67016e0..0000000 --- a/.claude/agents/CLAUDE.md +++ /dev/null @@ -1,11 +0,0 @@ -# agents/ - -## Files - -| File | What | When to read | -| ---- | ---- | ------------ | -| `README.md` | Agent team overview and coordination | Understanding agent workflow | -| `feature-agent.md` | Backend feature development agent | Backend feature work | -| `frontend-agent.md` | React/mobile-first UI agent | Frontend component work | -| `platform-agent.md` | Platform services agent | Platform microservice work | -| `quality-agent.md` | Final validation agent | Pre-merge quality checks | diff --git a/.claude/agents/README.md b/.claude/agents/README.md deleted file mode 100644 index 17f7eca..0000000 --- a/.claude/agents/README.md +++ /dev/null @@ -1,45 +0,0 @@ -# MotoVaultPro Agent Team - -Specialized agents for MotoVaultPro development. Each agent has detailed instructions in their own file. - -## Quick Reference - -| Agent | File | Use When | -|-------|------|----------| -| Feature Agent | `feature-agent.md` | Backend feature development in `backend/src/features/` | -| Frontend Agent | `frontend-agent.md` | React components, mobile-first responsive UI | -| Platform Agent | `platform-agent.md` | Platform microservices in `mvp-platform-services/` | -| Quality Agent | `quality-agent.md` | Final validation before merge/deploy | - -## Sprint Workflow - -All agents follow the sprint workflow defined in `.ai/workflow-contract.json`: - -1. Pick issue from current sprint with `status/ready` -2. Move to `status/in-progress`, create branch `issue-{index}-{slug}` -3. Implement with commits referencing issue -4. Open PR, move to `status/review` -5. Quality Agent validates before `status/done` - -## Coordination - -- Agents do NOT modify each other's code -- Feature + Frontend agents can work in parallel -- Quality Agent validates all work before completion -- Conflicts escalate to Expert Software Architect - -## Context Loading - -Each agent loads minimal context: -- `.ai/context.json` - Architecture overview -- `.ai/workflow-contract.json` - Sprint workflow -- Their specific agent file - Role and responsibilities -- Feature/component README - Task-specific context - -## Quality Standards (All Agents) - -- All linters pass (zero errors) -- All tests pass -- Mobile + desktop validated -- Old code deleted -- Documentation updated diff --git a/.claude/agents/feature-agent.md b/.claude/agents/feature-agent.md deleted file mode 100644 index 73640b3..0000000 --- a/.claude/agents/feature-agent.md +++ /dev/null @@ -1,97 +0,0 @@ ---- -name: feature-agent -description: MUST BE USED when creating or maintaining backend features -model: sonnet ---- - -# Feature Agent - -Owns backend feature capsules in `backend/src/features/{feature}/`. Coordinates with role agents for execution. - -## Scope - -**You Own**: -``` -backend/src/features/{feature}/ -├── README.md, index.ts -├── api/ (controllers, routes, validation) -├── domain/ (services, types) -├── data/ (repositories) -├── migrations/, external/, tests/ -``` - -**You Don't Own**: Frontend, platform services, core services, shared utilities. - -## Delegation Protocol - -Delegate to role agents for execution: - -### To Developer -```markdown -## Delegation: Developer -- Mode: plan-execution | freeform -- Issue: #{issue_index} -- Context: [file paths, acceptance criteria] -- Return: [implementation deliverables] -``` - -### To Technical Writer -```markdown -## Delegation: Technical Writer -- Mode: plan-scrub | post-implementation -- Files: [list of modified files] -``` - -### To Quality Reviewer -```markdown -## Delegation: Quality Reviewer -- Mode: plan-completeness | plan-code | post-implementation -- Issue: #{issue_index} -``` - -## Skill Triggers - -| Situation | Skill | -|-----------|-------| -| Complex feature (3+ files) | Planner | -| Unfamiliar code area | Codebase Analysis | -| Uncertain approach | Problem Analysis, Decision Critic | -| Bug investigation | Debugger | - -## Development Workflow - -```bash -npm install # Local dependencies -npm run dev # Start dev server -npm test # Run tests -npm run lint # Linting -npm run type-check # TypeScript -``` - -Push to Gitea -> CI/CD runs -> PR review -> Merge - -## Quality Standards - -- All linters pass (zero errors) -- All tests pass -- Mobile + desktop validation -- Feature README updated - -## Handoff: To Frontend Agent - -After API complete: -``` -Feature: {name} -API: POST/GET/PUT/DELETE endpoints -Auth: JWT required -Validation: [rules] -Errors: [codes] -``` - -## References - -| Doc | When | -|-----|------| -| `.ai/workflow-contract.json` | Sprint process | -| `.claude/role-agents/quality-reviewer.md` | RULE 0/1/2 | -| `backend/src/features/{feature}/README.md` | Feature context | diff --git a/.claude/agents/frontend-agent.md b/.claude/agents/frontend-agent.md deleted file mode 100644 index 4362f0f..0000000 --- a/.claude/agents/frontend-agent.md +++ /dev/null @@ -1,87 +0,0 @@ ---- -name: first-frontend-agent -description: MUST BE USED when editing or modifying frontend design for Desktop or Mobile -model: sonnet ---- - -# Frontend Agent - -Owns React UI in `frontend/src/`. Mobile + desktop validation is non-negotiable. - -## Scope - -**You Own**: `frontend/src/` (features, core, shared-minimal, types) -**You Don't Own**: Backend, platform services, database - -## Delegation Protocol - -### To Developer -```markdown -## Delegation: Developer -- Mode: plan-execution | freeform -- Issue: #{issue_index} -- Context: [component specs, API contract] -``` - -### To Quality Reviewer -```markdown -## Delegation: Quality Reviewer -- Mode: post-implementation -- Viewports: 320px, 768px, 1920px validated -``` - -## Skill Triggers - -| Situation | Skill | -|-----------|-------| -| Complex UI (3+ components) | Planner | -| Unfamiliar patterns | Codebase Analysis | -| UX decisions | Problem Analysis | - -## Development Workflow - -```bash -npm install && npm run dev # Local development -npm test # Run tests -npm run lint && npm run type-check -``` - -Push to Gitea -> CI/CD validates -> PR review -> Merge - -## Mobile-First Requirements - -**Before any component**: -- Design for 320px first -- Touch targets >= 44px -- No hover-only interactions - -**Validation checkpoints**: -- [ ] Mobile (320px, 768px) -- [ ] Desktop (1920px) -- [ ] Touch interactions -- [ ] Keyboard navigation - -## Tech Stack - -React 18, TypeScript, Vite, MUI, Tailwind, react-hook-form + Zod, React Query, Zustand, Auth0 - -## Quality Standards - -- Zero TypeScript/ESLint errors -- All tests passing -- Mobile + desktop validated -- Accessible (WCAG AA) -- Suspense/Error boundaries in place - -## Handoff: From Feature Agent - -Receive: API documentation, endpoints, validation rules -Deliver: Responsive components working on mobile + desktop - -## References - -| Doc | When | -|-----|------| -| `.ai/workflow-contract.json` | Sprint process | -| `.claude/role-agents/quality-reviewer.md` | RULE 0/1/2 | -| Backend feature README | API contract | diff --git a/.claude/agents/platform-agent.md b/.claude/agents/platform-agent.md deleted file mode 100644 index ec46704..0000000 --- a/.claude/agents/platform-agent.md +++ /dev/null @@ -1,77 +0,0 @@ ---- -name: platform-agent -description: MUST BE USED when editing or modifying platform services -model: sonnet ---- - -# Platform Agent - -Owns independent microservices in `mvp-platform-services/{service}/`. - -## Scope - -**You Own**: `mvp-platform-services/{service}/` (FastAPI services, ETL pipelines) -**You Don't Own**: Application features, frontend, other services - -## Delegation Protocol - -### To Developer -```markdown -## Delegation: Developer -- Mode: plan-execution | freeform -- Issue: #{issue_index} -- Service: {service-name} -- Context: [API specs, data contracts] -``` - -### To Quality Reviewer -```markdown -## Delegation: Quality Reviewer -- Mode: post-implementation -- Service: {service-name} -``` - -## Skill Triggers - -| Situation | Skill | -|-----------|-------| -| New service/endpoint | Planner | -| ETL pipeline work | Problem Analysis | -| Service integration | Codebase Analysis | - -## Development Workflow - -```bash -cd mvp-platform-services/{service} -pip install -r requirements.txt -pytest # Run tests -uvicorn main:app --reload # Local dev -``` - -Push to Gitea -> CI/CD runs -> PR review -> Merge - -## Service Architecture - -- FastAPI with async endpoints -- PostgreSQL/Redis connections -- Health endpoint at `/health` -- Swagger docs at `/docs` - -## Quality Standards - -- All pytest tests passing -- Health endpoint returns 200 -- API documentation functional -- Service containers healthy - -## Handoff: To Feature Agent - -Provide: Service API documentation, request/response examples, error codes - -## References - -| Doc | When | -|-----|------| -| `docs/PLATFORM-SERVICES.md` | Service architecture | -| `.ai/workflow-contract.json` | Sprint process | -| Service README | Service-specific context | diff --git a/.claude/agents/quality-agent.md b/.claude/agents/quality-agent.md deleted file mode 100644 index d719b7f..0000000 --- a/.claude/agents/quality-agent.md +++ /dev/null @@ -1,88 +0,0 @@ ---- -name: quality-agent -description: MUST BE USED last before code is committed and signed off as production ready -model: sonnet ---- - -# Quality Agent - -Final gatekeeper ensuring nothing moves forward without passing ALL quality gates. - -**Critical mandate**: ALL GREEN. ZERO TOLERANCE. NO EXCEPTIONS. - -## Scope - -**You Validate**: Tests, linting, type checking, mobile + desktop, security -**You Don't Write**: Application code, tests, business logic (validation only) - -## Delegation Protocol - -### To Quality Reviewer (Role Agent) -```markdown -## Delegation: Quality Reviewer -- Mode: post-implementation -- Issue: #{issue_index} -- Files: [modified files list] -``` - -Delegate for RULE 0/1/2 analysis. See `.claude/role-agents/quality-reviewer.md` for definitions. - -## Quality Gates - -**All must pass**: -- [ ] All tests pass (100% green) -- [ ] Zero linting errors -- [ ] Zero type errors -- [ ] Mobile validated (320px, 768px) -- [ ] Desktop validated (1920px) -- [ ] No security vulnerabilities -- [ ] Test coverage >= 80% for new code -- [ ] CI/CD pipeline passes - -## Validation Commands - -```bash -npm run lint # ESLint -npm run type-check # TypeScript -npm test # All tests -npm test -- --coverage # Coverage report -``` - -## Sprint Workflow - -Gatekeeper for `status/review` -> `status/done`: -1. Check issues with `status/review` -2. Run complete validation suite -3. Apply RULE 0/1/2 review -4. If ALL pass: Approve PR, move to `status/done` -5. If ANY fail: Comment with specific failures, block - -## Output Format - -**Pass**: -``` -QUALITY VALIDATION: PASS -- Tests: {count} passing -- Linting: Clean -- Type check: Clean -- Coverage: {%} -- Mobile/Desktop: Validated -STATUS: APPROVED -``` - -**Fail**: -``` -QUALITY VALIDATION: FAIL -BLOCKING ISSUES: -- {specific issue with location} -REQUIRED: Fix issues and re-validate -STATUS: NOT APPROVED -``` - -## References - -| Doc | When | -|-----|------| -| `.claude/role-agents/quality-reviewer.md` | RULE 0/1/2 definitions | -| `.ai/workflow-contract.json` | Sprint process | -| `docs/TESTING.md` | Testing strategies | diff --git a/.claude/hooks/CLAUDE.md b/.claude/hooks/CLAUDE.md deleted file mode 100644 index f98cfdf..0000000 --- a/.claude/hooks/CLAUDE.md +++ /dev/null @@ -1,38 +0,0 @@ -# hooks/ - -## Files - -| File | What | When to read | -| ---- | ---- | ------------ | -| `enforce-agent-model.sh` | Enforces correct model for Task tool calls | Debugging agent model issues | - -## enforce-agent-model.sh - -PreToolUse hook that ensures Task tool calls use the correct model based on `subagent_type`. - -### Agent Model Mapping - -| Agent | Required Model | -|-------|----------------| -| feature-agent | sonnet | -| first-frontend-agent | sonnet | -| platform-agent | sonnet | -| quality-agent | sonnet | -| developer | sonnet | -| technical-writer | sonnet | -| debugger | sonnet | -| quality-reviewer | opus | -| Explore | sonnet | -| Plan | sonnet | -| Bash | sonnet | -| general-purpose | sonnet | - -### Behavior - -- Blocks Task calls where `model` parameter doesn't match expected value -- Returns error message instructing Claude to retry with correct model -- Unknown agent types are allowed through (no enforcement) - -### Adding New Agents - -Edit the `get_expected_model()` function in `enforce-agent-model.sh` to add new agent mappings. diff --git a/.claude/hooks/enforce-agent-model.sh b/.claude/hooks/enforce-agent-model.sh deleted file mode 100755 index 203b350..0000000 --- a/.claude/hooks/enforce-agent-model.sh +++ /dev/null @@ -1,58 +0,0 @@ -#!/usr/bin/env bash -# Enforces correct model usage for Task tool based on agent definitions -# Blocks Task calls that don't specify the correct model for the subagent_type - -# Read tool input from stdin -INPUT=$(cat) - -# Extract subagent_type and model from the input -SUBAGENT_TYPE=$(echo "$INPUT" | jq -r '.subagent_type // empty') -MODEL=$(echo "$INPUT" | jq -r '.model // empty') - -# If no subagent_type, allow (not an agent call) -if [[ -z "$SUBAGENT_TYPE" ]]; then - exit 0 -fi - -# Get expected model for agent type -# Most agents use sonnet, quality-reviewer uses opus -get_expected_model() { - case "$1" in - # Custom project agents - feature-agent|first-frontend-agent|platform-agent|quality-agent) - echo "sonnet" - ;; - # Role agents - developer|technical-writer|debugger) - echo "sonnet" - ;; - quality-reviewer) - echo "opus" - ;; - # Built-in agents - default to sonnet for cost efficiency - Explore|Plan|Bash|general-purpose) - echo "sonnet" - ;; - *) - # Unknown agent, no enforcement - echo "" - ;; - esac -} - -EXPECTED_MODEL=$(get_expected_model "$SUBAGENT_TYPE") - -# If agent not in mapping, allow (unknown agent type) -if [[ -z "$EXPECTED_MODEL" ]]; then - exit 0 -fi - -# Check if model matches expected -if [[ "$MODEL" != "$EXPECTED_MODEL" ]]; then - echo "BLOCKED: Agent '$SUBAGENT_TYPE' requires model: '$EXPECTED_MODEL' but got '${MODEL:-}'." - echo "Retry with: model: \"$EXPECTED_MODEL\"" - exit 1 -fi - -# Model matches, allow the call -exit 0 diff --git a/.claude/role-agents/CLAUDE.md b/.claude/role-agents/CLAUDE.md deleted file mode 100644 index d0f4f3b..0000000 --- a/.claude/role-agents/CLAUDE.md +++ /dev/null @@ -1,10 +0,0 @@ -# role-agents/ - -## Files - -| File | What | When to read | -| ---- | ---- | ------------ | -| `developer.md` | Developer role agent | Code implementation tasks | -| `technical-writer.md` | Technical writer agent | Documentation tasks | -| `quality-reviewer.md` | Quality reviewer with RULE 0/1/2 | Code review, quality gates | -| `debugger.md` | Debugging specialist agent | Bug investigation, troubleshooting | diff --git a/.claude/role-agents/debugger.md b/.claude/role-agents/debugger.md deleted file mode 100644 index fd7f925..0000000 --- a/.claude/role-agents/debugger.md +++ /dev/null @@ -1,87 +0,0 @@ ---- -name: debugger -description: Systematically gathers evidence to identify root causes - others fix -model: sonnet ---- - -# Debugger - -Systematically gathers evidence to identify root causes. Your job is investigation, not fixing. - -## RULE 0: Clean Codebase on Exit - -ALL debug artifacts MUST be removed before returning: -- Debug statements -- Test files created for debugging -- Console.log/print statements added - -Track every artifact in TodoWrite immediately when added. - -## Workflow - -1. Understand problem (symptoms, expected vs actual) -2. Plan investigation (hypotheses, test inputs) -3. Track changes (TodoWrite all debug artifacts) -4. Gather evidence (10+ debug outputs minimum) -5. Verify evidence with open questions -6. Analyze (root cause identification) -7. Clean up (remove ALL artifacts) -8. Report (findings only, no fixes) - -## Evidence Requirements - -**Minimum before concluding**: -- 10+ debug statements across suspect code paths -- 3+ test inputs covering different scenarios -- Entry/exit logs for all suspect functions -- Isolated reproduction test - -**For each hypothesis**: -- 3 debug outputs supporting it -- 1 ruling out alternatives -- Observed exact execution path - -## Debug Statement Protocol - -Format: `[DEBUGGER:location:line] variable_values` - -This format enables grep cleanup verification: -```bash -grep 'DEBUGGER:' # Should return 0 results after cleanup -``` - -## Techniques by Category - -| Category | Technique | -|----------|-----------| -| Memory | Pointer values + dereferenced content, sanitizers | -| Concurrency | Thread IDs, lock sequences, race detectors | -| Performance | Timing before/after, memory tracking, profilers | -| State/Logic | State transitions with old/new values, condition breakdowns | - -## Output Format - -``` -## Investigation: [Problem Summary] - -### Symptoms -[What was observed] - -### Root Cause -[Specific cause with evidence] - -### Evidence -| Observation | Location | Supports | -|-------------|----------|----------| -| [finding] | [file:line] | [hypothesis] | - -### Cleanup Verification -- [ ] All debug statements removed -- [ ] All test files deleted -- [ ] grep 'DEBUGGER:' returns 0 results - -### Recommended Fix (for domain agent) -[What should be changed - domain agent implements] -``` - -See `.claude/skills/debugger/` for detailed investigation protocols. diff --git a/.claude/role-agents/developer.md b/.claude/role-agents/developer.md deleted file mode 100644 index 341b5a1..0000000 --- a/.claude/role-agents/developer.md +++ /dev/null @@ -1,89 +0,0 @@ ---- -name: developer -description: Implements specs with tests - delegate for writing code -model: sonnet ---- - -# Developer - -Expert implementer translating specifications into working code. Execute faithfully; design decisions belong to domain agents. - -## Pre-Work - -Before writing code: -1. Read CLAUDE.md in repository root -2. Follow "Read when..." triggers relevant to task -3. Extract: language patterns, error handling, code style - -## Workflow - -Receive spec -> Understand -> Plan -> Execute -> Verify -> Return output - -**Before coding**: -1. Identify inputs, outputs, constraints -2. List files, functions, changes required -3. Note tests the spec requires -4. Flag ambiguities or blockers (escalate if found) - -## Spec Types - -### Detailed Specs -Prescribes HOW to implement. Signals: "at line 45", "rename X to Y" -- Follow exactly -- Add nothing beyond what is specified -- Match prescribed structure and naming - -### Freeform Specs -Describes WHAT to achieve. Signals: "add logging", "improve error handling" -- Use judgment for implementation details -- Follow project conventions -- Implement smallest change that satisfies intent - -**Scope limitation**: Do what is asked; nothing more, nothing less. - -## Priority Order - -When rules conflict: -1. Security constraints (RULE 0) - override everything -2. Project documentation (CLAUDE.md) - override spec details -3. Detailed spec instructions - follow exactly -4. Your judgment - for freeform specs only - -## MotoVaultPro Patterns - -- Feature capsules: `backend/src/features/{feature}/` -- Repository pattern with mapRow() for DB->TS case conversion -- Snake_case in DB, camelCase in TypeScript -- Mobile + desktop validation required - -## Comment Handling - -**Plan-based execution**: Transcribe comments from plan verbatim. Comments explain WHY; plan author has already optimized for future readers. - -**Freeform execution**: Write WHY comments for non-obvious code. Skip comments when code is self-documenting. - -**Exclude from output**: FIXED:, NEW:, NOTE:, location directives, planning annotations. - -## Escalation - -Return to domain agent when: -- Missing dependencies block implementation -- Spec contradictions require design decisions -- Ambiguities that project docs cannot resolve - -## Output Format - -``` -## Implementation Complete - -### Files Modified -- [file]: [what changed] - -### Tests -- [test file]: [coverage] - -### Notes -[assumptions made, issues encountered] -``` - -See `.claude/skills/planner/` for diff format specification. diff --git a/.claude/role-agents/quality-reviewer.md b/.claude/role-agents/quality-reviewer.md deleted file mode 100644 index 2c2f0cd..0000000 --- a/.claude/role-agents/quality-reviewer.md +++ /dev/null @@ -1,84 +0,0 @@ ---- -name: quality-reviewer -description: Reviews code and plans for production risks, project conformance, and structural quality -model: opus ---- - -# Quality Reviewer - -Expert reviewer detecting production risks, conformance violations, and structural defects. - -## RULE Hierarchy (CANONICAL DEFINITIONS) - -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 (forward + backward reasoning). - -### 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}/`) -- Repository pattern with mapRow() for case conversion -- CI/CD pipeline must pass - -**Verification**: Cite specific standard from CLAUDE.md or project docs. - -### 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 don't explicitly permit the pattern. - -## Invocation Modes - -| Mode | Focus | Rules Applied | -|------|-------|---------------| -| `plan-completeness` | Plan document structure | Decision Log, Policy Defaults | -| `plan-code` | Proposed code in plan | RULE 0/1/2 + codebase alignment | -| `plan-docs` | Post-TW documentation | Temporal contamination, comment quality | -| `post-implementation` | Code after implementation | All rules | -| `reconciliation` | Check milestone completion | Acceptance criteria only | - -## 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] -``` - -## Quick Reference - -**Before flagging**: -1. Read CLAUDE.md/project docs for standards (RULE 1 scope) -2. Check Planning Context for Known Risks (skip acknowledged risks) -3. Verify finding is actionable with specific fix - -**Severity guide**: -- CRITICAL: Data loss, security breach, system failure -- HIGH: Production reliability or project standard violation -- SHOULD_FIX: Structural quality issue -- SUGGESTION: Improvement opportunity - -See `.claude/skills/quality-reviewer/` for detailed review protocols. diff --git a/.claude/role-agents/technical-writer.md b/.claude/role-agents/technical-writer.md deleted file mode 100644 index b76fda2..0000000 --- a/.claude/role-agents/technical-writer.md +++ /dev/null @@ -1,66 +0,0 @@ ---- -name: technical-writer -description: Creates LLM-optimized documentation - every word earns its tokens -model: sonnet ---- - -# Technical Writer - -Creates documentation optimized for LLM consumption. Every word earns its tokens. - -## Modes - -| Mode | Input | Output | -|------|-------|--------| -| `plan-scrub` | Plan with code snippets | Plan with temporal-clean comments | -| `post-implementation` | Modified files list | CLAUDE.md indexes, README.md if needed | - -## CLAUDE.md Format (~200 tokens) - -Tabular index only, no prose: - -```markdown -| Path | What | When | -|------|------|------| -| `file.ts` | Description | Task trigger | -``` - -## README.md (Only When Needed) - -Create README.md only for Invisible Knowledge: -- Architecture decisions not apparent from code -- Invariants and constraints -- Design tradeoffs - -## Temporal Contamination Detection - -Comments must pass the **Timeless Present Rule**: written as if reader has no knowledge of code history. - -**Five detection questions**: -1. Describes action taken rather than what exists? (change-relative) -2. Compares to something not in code? (baseline reference) -3. Describes where to put code? (location directive - DELETE) -4. Describes intent rather than behavior? (planning artifact) -5. Describes author's choice rather than code behavior? (intent leakage) - -| Contaminated | Timeless Present | -|--------------|------------------| -| "Added mutex to fix race" | "Mutex serializes concurrent access" | -| "Replaced per-tag logging" | "Single summary line; per-tag would produce 1500+ lines" | -| "After the SendAsync call" | (delete - location is in diff) | - -**Transformation pattern**: Extract technical justification, discard change narrative. - -## Comment Quality - -- Document WHY, never WHAT -- Skip comments for CRUD and standard patterns -- For >3 step functions, add explanatory block - -## Forbidden Patterns - -- Marketing language: "elegant", "robust", "powerful" -- Hedging: "basically", "simply", "just" -- Aspirational: "will support", "planned for" - -See `.claude/skills/doc-sync/` for detailed documentation protocols. diff --git a/.claude/skills/CLAUDE.md b/.claude/skills/CLAUDE.md deleted file mode 100644 index fa0668f..0000000 --- a/.claude/skills/CLAUDE.md +++ /dev/null @@ -1,13 +0,0 @@ -# skills/ - -## Subdirectories - -| Directory | What | When to read | -| --------- | ---- | ------------ | -| `planner/` | Planning workflow with resource sync | Complex features (3+ files) | -| `problem-analysis/` | Structured problem decomposition | Uncertain approach, debugging | -| `decision-critic/` | Decision stress-testing | Architectural choices, tradeoffs | -| `codebase-analysis/` | Systematic codebase investigation | Unfamiliar areas, audits | -| `doc-sync/` | CLAUDE.md/README.md synchronization | After refactors, periodic audits | -| `incoherence/` | Detect doc/code drift | Documentation inconsistencies | -| `prompt-engineer/` | Prompt optimization techniques | Improving AI prompts | diff --git a/.claude/skills/codebase-analysis/CLAUDE.md b/.claude/skills/codebase-analysis/CLAUDE.md deleted file mode 100644 index ad48c18..0000000 --- a/.claude/skills/codebase-analysis/CLAUDE.md +++ /dev/null @@ -1,16 +0,0 @@ -# skills/codebase-analysis/ - -## Overview - -Systematic codebase analysis skill. IMMEDIATELY invoke the script - do NOT explore first. - -## Index - -| File/Directory | Contents | Read When | -| -------------------- | ----------------- | ------------------ | -| `SKILL.md` | Invocation | Using this skill | -| `scripts/analyze.py` | Complete workflow | Debugging behavior | - -## Key Point - -The script IS the workflow. It handles exploration dispatch, focus selection, investigation, and synthesis. Do NOT explore or analyze before invoking. Run the script and obey its output. diff --git a/.claude/skills/codebase-analysis/README.md b/.claude/skills/codebase-analysis/README.md deleted file mode 100644 index 094cff9..0000000 --- a/.claude/skills/codebase-analysis/README.md +++ /dev/null @@ -1,48 +0,0 @@ -# Analyze - -Before you plan anything non-trivial, you need to actually understand the -codebase. Not impressions -- evidence. The analyze skill forces systematic -investigation with structured phases and explicit evidence requirements. - -| Phase | Actions | -| ---------------------- | ------------------------------------------------------------------------------ | -| Exploration | Delegate to Explore agent; process structure, tech stack, patterns | -| Focus Selection | Classify areas (architecture, performance, security, quality); assign P1/P2/P3 | -| Investigation Planning | Commit to specific files and questions; create accountability contract | -| Deep Analysis | Progressive investigation; document with file:line + quoted code | -| Verification | Audit completeness; ensure all commitments addressed | -| Synthesis | Consolidate by severity; provide prioritized recommendations | - -## When to Use - -Four scenarios where this matters: - -- **Unfamiliar codebase** -- You cannot plan what you do not understand. Period. -- **Security review** -- Vulnerability assessment requires systematic coverage, - not "I looked around and it seems fine." -- **Performance analysis** -- Before optimization, know where time actually - goes, not where you assume it goes. -- **Architecture evaluation** -- Major refactors deserve evidence-backed - understanding, not vibes. - -## When to Skip - -Not everything needs this level of rigor: - -- You already understand the codebase well -- Simple bug fix with obvious scope -- User has provided comprehensive context - -The astute reader will notice all three skip conditions share a trait: you -already have the evidence. The skill exists for when you do not. - -## Example Usage - -``` -Use your analyze skill to understand this codebase. -Focus on security and architecture before we plan the authentication refactor. -``` - -The skill outputs findings organized by severity (CRITICAL/HIGH/MEDIUM/LOW), -each with file:line references and quoted code. This feeds directly into -planning -- you have evidence-backed understanding before proposing changes. diff --git a/.claude/skills/codebase-analysis/SKILL.md b/.claude/skills/codebase-analysis/SKILL.md deleted file mode 100644 index e2a608b..0000000 --- a/.claude/skills/codebase-analysis/SKILL.md +++ /dev/null @@ -1,25 +0,0 @@ ---- -name: codebase-analysis -description: Invoke IMMEDIATELY via python script when user requests codebase analysis, architecture review, security assessment, or quality evaluation. Do NOT explore first - the script orchestrates exploration. ---- - -# Codebase Analysis - -When this skill activates, IMMEDIATELY invoke the script. The script IS the workflow. - -## Invocation - -```bash -python3 scripts/analyze.py \ - --step-number 1 \ - --total-steps 6 \ - --thoughts "Starting analysis. User request: " -``` - -| Argument | Required | Description | -| --------------- | -------- | ----------------------------------------- | -| `--step-number` | Yes | Current step (starts at 1) | -| `--total-steps` | Yes | Minimum 6; adjust as script instructs | -| `--thoughts` | Yes | Accumulated state from all previous steps | - -Do NOT explore or analyze first. Run the script and follow its output. diff --git a/.claude/skills/codebase-analysis/scripts/analyze.py b/.claude/skills/codebase-analysis/scripts/analyze.py deleted file mode 100755 index 877831f..0000000 --- a/.claude/skills/codebase-analysis/scripts/analyze.py +++ /dev/null @@ -1,661 +0,0 @@ -#!/usr/bin/env python3 -""" -Analyze Skill - Step-by-step codebase analysis with exploration and deep investigation. - -Six-phase workflow: -1. EXPLORATION: Process Explore sub-agent results -2. FOCUS SELECTION: Classify investigation areas -3. INVESTIGATION PLANNING: Commit to specific files and questions -4. DEEP ANALYSIS (1-N): Progressive investigation with evidence -5. VERIFICATION: Validate completeness before synthesis -6. SYNTHESIS: Consolidate verified findings - -Usage: - python3 analyze.py --step-number 1 --total-steps 6 --thoughts "Explore found: ..." -""" - -import argparse -import sys - - -def get_phase_name(step: int, total_steps: int) -> str: - """Return the phase name for a given step number.""" - if step == 1: - return "EXPLORATION" - elif step == 2: - return "FOCUS SELECTION" - elif step == 3: - return "INVESTIGATION PLANNING" - elif step == total_steps - 1: - return "VERIFICATION" - elif step == total_steps: - return "SYNTHESIS" - else: - return "DEEP ANALYSIS" - - -def get_state_requirement(step: int) -> list[str]: - """Return state accumulation requirement for steps 2+.""" - if step < 2: - return [] - - return [ - "", - "", - "CRITICAL: Your --thoughts for this step MUST include:", - "", - "1. FOCUS AREAS: Each area identified and its priority (from step 2)", - "2. INVESTIGATION PLAN: Files and questions committed to (from step 3)", - "3. FILES EXAMINED: Every file read with key observations", - "4. ISSUES BY SEVERITY: All [CRITICAL]/[HIGH]/[MEDIUM]/[LOW] items", - "5. PATTERNS: Cross-file patterns identified", - "6. HYPOTHESES: Current theories and supporting evidence", - "7. REMAINING: What still needs investigation", - "", - "If ANY section is missing, your accumulated state is incomplete.", - "Reconstruct it before proceeding.", - "", - ] - - -def get_step_guidance(step: int, total_steps: int) -> dict: - """Return step-specific guidance and actions.""" - - next_step = step + 1 if step < total_steps else None - phase = get_phase_name(step, total_steps) - is_final = step >= total_steps - - # Minimum steps: exploration(1) + focus(2) + planning(3) + analysis(4) + verification(5) + synthesis(6) - min_steps = 6 - - # PHASE 1: EXPLORATION - if step == 1: - return { - "phase": phase, - "step_title": "Process Exploration Results", - "actions": [ - "STOP. Before proceeding, verify you have Explore agent results.", - "", - "If your --thoughts do NOT contain Explore agent output, you MUST:", - "", - "", - "Assess the scope and delegate appropriately:", - "", - "SINGLE CODEBASE, FOCUSED SCOPE:", - " - One Explore agent is sufficient", - " - Use Task tool with subagent_type='Explore'", - " - Prompt: 'Explore this repository. Report directory structure,", - " tech stack, entry points, main components, observed patterns.'", - "", - "LARGE CODEBASE OR BROAD SCOPE:", - " - Launch MULTIPLE Explore agents IN PARALLEL (single message, multiple Task calls)", - " - Divide by logical boundaries: frontend/backend, services, modules", - " - Example prompts:", - " Agent 1: 'Explore src/api/ and src/services/. Focus on API structure.'", - " Agent 2: 'Explore src/core/ and src/models/. Focus on domain logic.'", - " Agent 3: 'Explore tests/ and config/. Focus on test patterns and configuration.'", - "", - "MULTIPLE CODEBASES:", - " - Launch ONE Explore agent PER CODEBASE in parallel", - " - Each agent explores its repository independently", - " - Example:", - " Agent 1: 'Explore /path/to/repo-a. Report structure and patterns.'", - " Agent 2: 'Explore /path/to/repo-b. Report structure and patterns.'", - "", - "WAIT for ALL agents to complete before invoking this step again.", - "", - "", - "Only proceed below if you have concrete Explore output to process.", - "", - "=" * 60, - "", - "", - "From the Explore agent(s) report(s), extract and document:", - "", - "STRUCTURE:", - " - Main directories and their purposes", - " - Where core logic lives vs. configuration vs. tests", - " - File organization patterns", - " - (If multiple agents: note boundaries and overlaps)", - "", - "TECH STACK:", - " - Languages, frameworks, key dependencies", - " - Build system, package management", - " - External services or APIs", - "", - "ENTRY POINTS:", - " - Main executables, API endpoints, CLI commands", - " - Data flow through the system", - " - Key interfaces between components", - "", - "INITIAL OBSERVATIONS:", - " - Architectural patterns (MVC, microservices, monolith)?", - " - Obvious code smells or areas of concern?", - " - Parts that seem well-structured vs. problematic?", - "", - ], - "next": ( - f"Invoke step {next_step} with your processed exploration summary. " - "Include all structure, tech stack, and initial observations in --thoughts." - ), - } - - # PHASE 2: FOCUS SELECTION - if step == 2: - actions = [ - "Based on exploration findings, determine what needs deep investigation.", - "", - "", - "Evaluate the codebase against each dimension. Mark areas needing investigation:", - "", - "ARCHITECTURE (structural concerns):", - " [ ] Component relationships unclear or tangled?", - " [ ] Dependency graph needs mapping?", - " [ ] Layering violations or circular dependencies?", - " [ ] Missing or unclear module boundaries?", - "", - "PERFORMANCE (efficiency concerns):", - " [ ] Hot paths that may be inefficient?", - " [ ] Database queries needing review?", - " [ ] Memory allocation patterns?", - " [ ] Concurrency or parallelism issues?", - "", - "SECURITY (vulnerability concerns):", - " [ ] Input validation gaps?", - " [ ] Authentication/authorization flows?", - " [ ] Sensitive data handling?", - " [ ] External API integrations?", - "", - "QUALITY (maintainability concerns):", - " [ ] Code duplication patterns?", - " [ ] Overly complex functions/classes?", - " [ ] Missing error handling?", - " [ ] Test coverage gaps?", - "", - "", - "", - "Rank your focus areas by priority (P1 = most critical):", - "", - " P1: [focus area] - [why most critical]", - " P2: [focus area] - [why second]", - " P3: [focus area] - [if applicable]", - "", - "Consider: security > correctness > performance > maintainability", - "", - "", - "", - "Estimate total steps based on scope:", - "", - f" Minimum steps: {min_steps} (exploration + focus + planning + 1 analysis + verification + synthesis)", - " 1-2 focus areas, small codebase: total_steps = 6-7", - " 2-3 focus areas, medium codebase: total_steps = 7-9", - " 3+ focus areas, large codebase: total_steps = 9-12", - "", - "You can adjust this estimate as understanding grows.", - "", - ] - actions.extend(get_state_requirement(step)) - return { - "phase": phase, - "step_title": "Classify Investigation Areas", - "actions": actions, - "next": ( - f"Invoke step {next_step} with your prioritized focus areas and " - "updated total_steps estimate. Next: create investigation plan." - ), - } - - # PHASE 3: INVESTIGATION PLANNING - if step == 3: - actions = [ - "You have identified focus areas. Now commit to specific investigation targets.", - "", - "This step creates ACCOUNTABILITY. You will verify against these commitments.", - "", - "", - "For EACH focus area (in priority order), specify:", - "", - "---", - "FOCUS AREA: [name] (Priority: P1/P2/P3)", - "", - "Files to examine:", - " - path/to/file1.py", - " Question: [specific question to answer about this file]", - " Hypothesis: [what you expect to find]", - "", - " - path/to/file2.py", - " Question: [specific question to answer]", - " Hypothesis: [what you expect to find]", - "", - "Evidence needed to confirm/refute:", - " - [what specific code patterns would confirm hypothesis]", - " - [what would refute it]", - "---", - "", - "Repeat for each focus area.", - "", - "", - "", - "This is a CONTRACT. In subsequent steps, you MUST:", - "", - " 1. Read every file listed (using Read tool)", - " 2. Answer every question posed", - " 3. Document evidence with file:line references", - " 4. Update hypothesis based on actual evidence", - "", - "If you cannot answer a question, document WHY:", - " - File doesn't exist?", - " - Question was wrong?", - " - Need different files?", - "", - "Do NOT silently skip commitments.", - "", - ] - actions.extend(get_state_requirement(step)) - return { - "phase": phase, - "step_title": "Create Investigation Plan", - "actions": actions, - "next": ( - f"Invoke step {next_step} with your complete investigation plan. " - "Next: begin executing the plan with the highest priority focus area." - ), - } - - # PHASE 5: VERIFICATION (step N-1) - if step == total_steps - 1: - actions = [ - "STOP. Before synthesizing, verify your investigation is complete.", - "", - "", - "Review your investigation commitments from Step 3.", - "", - "For EACH file you committed to examine:", - " [ ] File was actually read (not just mentioned)?", - " [ ] Specific question was answered with evidence?", - " [ ] Finding documented with file:line reference and quoted code?", - "", - "For EACH hypothesis you formed:", - " [ ] Evidence collected (confirming OR refuting)?", - " [ ] Hypothesis updated based on evidence?", - " [ ] If refuted, what replaced it?", - "", - "", - "", - "Identify gaps in your investigation:", - "", - " - Files committed but not examined?", - " - Focus areas declared but not investigated?", - " - Issues referenced without file:line evidence?", - " - Patterns claimed without cross-file validation?", - " - Questions posed but not answered?", - "", - "List each gap explicitly:", - " GAP 1: [description]", - " GAP 2: [description]", - " ...", - "", - "", - "", - "If gaps exist:", - " 1. INCREASE total_steps by number of gaps that need investigation", - " 2. Return to DEEP ANALYSIS phase to fill gaps", - " 3. Re-enter VERIFICATION after gaps are filled", - "", - "If no gaps (or gaps are acceptable):", - " Proceed to SYNTHESIS (next step)", - "", - "", - "", - "For each [CRITICAL] or [HIGH] severity finding, verify:", - " [ ] Has quoted code (2-5 lines)?", - " [ ] Has exact file:line reference?", - " [ ] Impact is clearly explained?", - " [ ] Recommended fix is actionable?", - "", - "Findings without evidence are UNVERIFIED. Either:", - " - Add evidence now, or", - " - Downgrade severity, or", - " - Mark as 'needs investigation'", - "", - ] - actions.extend(get_state_requirement(step)) - return { - "phase": phase, - "step_title": "Verify Investigation Completeness", - "actions": actions, - "next": ( - "If gaps found: invoke earlier step to fill gaps, then return here. " - f"If complete: invoke step {next_step} for final synthesis." - ), - } - - # PHASE 6: SYNTHESIS (final step) - if is_final: - return { - "phase": phase, - "step_title": "Consolidate and Recommend", - "actions": [ - "Investigation verified. Synthesize all findings into actionable output.", - "", - "", - "Organize all VERIFIED findings by severity:", - "", - "CRITICAL ISSUES (must address immediately):", - " For each:", - " - file:line reference", - " - Quoted code (2-5 lines)", - " - Impact description", - " - Recommended fix", - "", - "HIGH ISSUES (should address soon):", - " For each: file:line, description, recommended fix", - "", - "MEDIUM ISSUES (consider addressing):", - " For each: description, general guidance", - "", - "LOW ISSUES (nice to fix):", - " Summarize patterns, defer to future work", - "", - "", - "", - "Identify systemic patterns:", - "", - " - Issues appearing across multiple files -> systemic problem", - " - Root causes explaining multiple symptoms", - " - Architectural changes that would prevent recurrence", - "", - "", - "", - "Provide prioritized action plan:", - "", - "IMMEDIATE (blocks other work / security risk):", - " 1. [action with specific file:line reference]", - " 2. [action with specific file:line reference]", - "", - "SHORT-TERM (address within current sprint):", - " 1. [action with scope indication]", - " 2. [action with scope indication]", - "", - "LONG-TERM (strategic improvements):", - " 1. [architectural or process recommendation]", - " 2. [architectural or process recommendation]", - "", - "", - "", - "Before presenting to user, verify:", - "", - " [ ] All CRITICAL/HIGH issues have file:line + quoted code?", - " [ ] Recommendations are actionable, not vague?", - " [ ] Findings organized by impact, not discovery order?", - " [ ] No findings lost from earlier steps?", - " [ ] Patterns are supported by multiple examples?", - "", - ], - "next": None, - } - - # PHASE 4: DEEP ANALYSIS (steps 4 to N-2) - # Calculate position within deep analysis phase - deep_analysis_step = step - 3 # 1st, 2nd, 3rd deep analysis step - remaining_before_verification = total_steps - 1 - step # steps until verification - - if deep_analysis_step == 1: - step_title = "Initial Investigation" - focus_instruction = [ - "Execute your investigation plan from Step 3.", - "", - "", - "For each file in your P1 (highest priority) focus area:", - "", - "1. READ the file using the Read tool", - "2. ANSWER the specific question you committed to", - "3. DOCUMENT findings with evidence:", - "", - " EVIDENCE FORMAT (required for each finding):", - " ```", - " [SEVERITY] Brief description (file.py:line-line)", - " > quoted code from file (2-5 lines)", - " Explanation: why this is an issue", - " ```", - "", - "4. UPDATE your hypothesis based on what you found", - " - Confirmed? Document supporting evidence", - " - Refuted? Document what you found instead", - " - Inconclusive? Note what else you need to check", - "", - "", - "Findings without quoted code are UNVERIFIED.", - ] - elif deep_analysis_step == 2: - step_title = "Deepen Investigation" - focus_instruction = [ - "Review findings from previous step. Go deeper.", - "", - "", - "For each issue found in the previous step:", - "", - "1. TRACE to root cause", - " - Why does this issue exist?", - " - What allowed it to be introduced?", - " - Are there related issues in connected files?", - "", - "2. EXAMINE related files", - " - Callers and callees of problematic code", - " - Similar patterns elsewhere in codebase", - " - Configuration that affects this code", - "", - "3. LOOK for patterns", - " - Same issue in multiple places? -> Systemic problem", - " - One-off issue? -> Localized fix", - "", - "4. MOVE to P2 focus area if P1 is sufficiently investigated", - "", - "", - "Continue documenting with file:line + quoted code.", - ] - else: - step_title = f"Extended Investigation (Pass {deep_analysis_step})" - focus_instruction = [ - "Focus on remaining gaps and open questions.", - "", - "", - "Review your accumulated state. Address:", - "", - "1. REMAINING items from your investigation plan", - " - Any files not yet examined?", - " - Any questions not yet answered?", - "", - "2. OPEN QUESTIONS from previous steps", - " - What needed further investigation?", - " - What dependencies weren't clear?", - "", - "3. PATTERN VALIDATION", - " - Cross-file patterns claimed but not verified?", - " - Need more examples to confirm systemic issues?", - "", - "4. EVIDENCE STRENGTHENING", - " - Any [CRITICAL]/[HIGH] findings without quoted code?", - " - Any claims without file:line references?", - "", - "", - "If investigation is complete, reduce total_steps to reach verification.", - ] - - actions = focus_instruction + [ - "", - "", - "After this step's investigation:", - "", - f" Remaining steps before verification: {remaining_before_verification}", - "", - " - Discovered more complexity? -> INCREASE total_steps", - " - Remaining scope smaller than expected? -> DECREASE total_steps", - " - All focus areas sufficiently covered? -> Set next step = total_steps - 1 (verification)", - "", - ] - actions.extend(get_state_requirement(step)) - - return { - "phase": phase, - "step_title": step_title, - "actions": actions, - "next": ( - f"Invoke step {next_step}. " - f"{remaining_before_verification} step(s) before verification. " - "Include ALL accumulated findings in --thoughts. " - "Adjust total_steps if scope changed." - ), - } - - -def format_output(step: int, total_steps: int, thoughts: str, guidance: dict) -> str: - """Format the output for display.""" - lines = [] - - # Header - lines.append("=" * 70) - lines.append(f"ANALYZE - Step {step}/{total_steps}: {guidance['step_title']}") - lines.append(f"Phase: {guidance['phase']}") - lines.append("=" * 70) - lines.append("") - - # Status - is_final = step >= total_steps - is_verification = step == total_steps - 1 - if is_final: - status = "analysis_complete" - elif is_verification: - status = "verification_required" - else: - status = "in_progress" - lines.append(f"STATUS: {status}") - lines.append("") - - # Current thoughts summary (truncated for display) - lines.append("YOUR ACCUMULATED STATE:") - if len(thoughts) > 600: - lines.append(thoughts[:600] + "...") - lines.append("[truncated - full state in --thoughts]") - else: - lines.append(thoughts) - lines.append("") - - # Actions - lines.append("REQUIRED ACTIONS:") - for action in guidance["actions"]: - if action: - # Handle the separator line specially - if action == "=" * 60: - lines.append(" " + action) - else: - lines.append(f" {action}") - else: - lines.append("") - lines.append("") - - # Next step or completion - if guidance["next"]: - lines.append("NEXT:") - lines.append(guidance["next"]) - else: - lines.append("WORKFLOW COMPLETE") - lines.append("") - lines.append("Present your consolidated findings to the user:") - lines.append(" - Organized by severity (CRITICAL -> LOW)") - lines.append(" - With file:line references and quoted code for serious issues") - lines.append(" - With actionable recommendations for each category") - - lines.append("") - lines.append("=" * 70) - - return "\n".join(lines) - - -def main(): - parser = argparse.ArgumentParser( - description="Analyze Skill - Systematic codebase analysis", - formatter_class=argparse.RawDescriptionHelpFormatter, - epilog=""" -Workflow Phases: - Step 1: EXPLORATION - Process Explore agent results - Step 2: FOCUS SELECTION - Classify investigation areas - Step 3: INVESTIGATION PLAN - Commit to specific files and questions - Step 4+: DEEP ANALYSIS - Progressive investigation with evidence - Step N-1: VERIFICATION - Validate completeness before synthesis - Step N: SYNTHESIS - Consolidate verified findings - -Examples: - # Step 1: After Explore agent returns - python3 analyze.py --step-number 1 --total-steps 6 \\ - --thoughts "Explore found: Python web app, Flask, SQLAlchemy..." - - # Step 2: Focus selection - python3 analyze.py --step-number 2 --total-steps 7 \\ - --thoughts "Structure: src/, tests/. Focus: security (P1), quality (P2)..." - - # Step 3: Investigation planning - python3 analyze.py --step-number 3 --total-steps 7 \\ - --thoughts "P1 Security: auth/login.py (Q: input validation?), ..." - - # Step 4: Initial investigation - python3 analyze.py --step-number 4 --total-steps 7 \\ - --thoughts "FILES: auth/login.py read. [CRITICAL] SQL injection at :45..." - - # Step 5: Deepen investigation - python3 analyze.py --step-number 5 --total-steps 7 \\ - --thoughts "[Previous state] + traced to db/queries.py, pattern in 3 files..." - - # Step 6: Verification - python3 analyze.py --step-number 6 --total-steps 7 \\ - --thoughts "[All findings] Checking: all files read, all questions answered..." - - # Step 7: Synthesis - python3 analyze.py --step-number 7 --total-steps 7 \\ - --thoughts "[Verified findings] Ready for consolidation..." -""" - ) - - parser.add_argument( - "--step-number", - type=int, - required=True, - help="Current step number (starts at 1)", - ) - parser.add_argument( - "--total-steps", - type=int, - required=True, - help="Estimated total steps (adjust as understanding grows)", - ) - parser.add_argument( - "--thoughts", - type=str, - required=True, - help="Accumulated findings, evidence, and file references", - ) - - args = parser.parse_args() - - # Validate inputs - if args.step_number < 1: - print("ERROR: step-number must be >= 1", file=sys.stderr) - sys.exit(1) - - if args.total_steps < 6: - print("ERROR: total-steps must be >= 6 (minimum workflow)", file=sys.stderr) - sys.exit(1) - - if args.total_steps < args.step_number: - print("ERROR: total-steps must be >= step-number", file=sys.stderr) - sys.exit(1) - - # Get guidance for current step - guidance = get_step_guidance(args.step_number, args.total_steps) - - # Print formatted output - print(format_output(args.step_number, args.total_steps, args.thoughts, guidance)) - - -if __name__ == "__main__": - main() diff --git a/.claude/skills/decision-critic/CLAUDE.md b/.claude/skills/decision-critic/CLAUDE.md deleted file mode 100644 index 3df3331..0000000 --- a/.claude/skills/decision-critic/CLAUDE.md +++ /dev/null @@ -1,16 +0,0 @@ -# skills/decision-critic/ - -## Overview - -Decision stress-testing skill. IMMEDIATELY invoke the script - do NOT analyze first. - -## Index - -| File/Directory | Contents | Read When | -| ---------------------------- | ----------------- | ------------------ | -| `SKILL.md` | Invocation | Using this skill | -| `scripts/decision-critic.py` | Complete workflow | Debugging behavior | - -## Key Point - -The script IS the workflow. It handles decomposition, verification, challenge, and synthesis phases. Do NOT analyze or critique before invoking. Run the script and obey its output. diff --git a/.claude/skills/decision-critic/README.md b/.claude/skills/decision-critic/README.md deleted file mode 100644 index 3e52752..0000000 --- a/.claude/skills/decision-critic/README.md +++ /dev/null @@ -1,59 +0,0 @@ -# Decision Critic - -Here's the problem: LLMs are sycophants. They agree with you. They validate your -reasoning. They tell you your architectural decision is sound and well-reasoned. -That's not what you need for important decisions -- you need stress-testing. - -The decision-critic skill forces structured adversarial analysis: - -| Phase | Actions | -| ------------- | -------------------------------------------------------------------------- | -| Decomposition | Extract claims, assumptions, constraints; assign IDs; classify each | -| Verification | Generate questions for verifiable items; answer independently; mark status | -| Challenge | Steel-man argument against; explore alternative framings | -| Synthesis | Verdict (STAND/REVISE/ESCALATE); summary and recommendation | - -## When to Use - -Use this for decisions where you actually want criticism, not agreement: - -- Architectural choices with long-term consequences -- Technology selection (language, framework, database) -- Tradeoffs between competing concerns (performance vs. maintainability) -- Decisions you're uncertain about and want stress-tested - -## Example Usage - -``` -I'm considering using Redis for our session storage instead of PostgreSQL. -My reasoning: - -- Redis is faster for key-value lookups -- Sessions are ephemeral, don't need ACID guarantees -- We already have Redis for caching - -Use your decision critic skill to stress-test this decision. -``` - -So what happens? The skill: - -1. **Decomposes** the decision into claims (C1: Redis is faster), assumptions - (A1: sessions don't need durability), constraints (K1: Redis already - deployed) -2. **Verifies** each claim -- is Redis actually faster for your access pattern? - What's the actual latency difference? -3. **Challenges** -- what if sessions DO need durability (shopping carts)? - What's the operational cost of Redis failures? -4. **Synthesizes** -- verdict with specific failed/uncertain items - -## The Anti-Sycophancy Design - -I grounded this skill in three techniques: - -- **Chain-of-Verification** -- factored verification prevents confirmation bias - by answering questions independently -- **Self-Consistency** -- multiple reasoning paths reveal disagreement -- **Multi-Expert Prompting** -- diverse perspectives catch blind spots - -The structure forces the LLM through adversarial phases rather than allowing it -to immediately agree with your reasoning. That's the whole point. diff --git a/.claude/skills/decision-critic/SKILL.md b/.claude/skills/decision-critic/SKILL.md deleted file mode 100644 index febe834..0000000 --- a/.claude/skills/decision-critic/SKILL.md +++ /dev/null @@ -1,29 +0,0 @@ ---- -name: decision-critic -description: Invoke IMMEDIATELY via python script to stress-test decisions and reasoning. Do NOT analyze first - the script orchestrates the critique workflow. ---- - -# Decision Critic - -When this skill activates, IMMEDIATELY invoke the script. The script IS the workflow. - -## Invocation - -```bash -python3 scripts/decision-critic.py \ - --step-number 1 \ - --total-steps 7 \ - --decision "" \ - --context "" \ - --thoughts "" -``` - -| Argument | Required | Description | -| --------------- | -------- | ----------------------------------------------------------- | -| `--step-number` | Yes | Current step (1-7) | -| `--total-steps` | Yes | Always 7 | -| `--decision` | Step 1 | The decision statement being criticized | -| `--context` | Step 1 | Constraints, background, system context | -| `--thoughts` | Yes | Your analysis including all IDs and status from prior steps | - -Do NOT analyze or critique first. Run the script and follow its output. diff --git a/.claude/skills/decision-critic/scripts/decision-critic.py b/.claude/skills/decision-critic/scripts/decision-critic.py deleted file mode 100755 index 2aa556e..0000000 --- a/.claude/skills/decision-critic/scripts/decision-critic.py +++ /dev/null @@ -1,468 +0,0 @@ -#!/usr/bin/env python3 -""" -Decision Critic - Step-by-step prompt injection for structured decision criticism. - -Grounded in: -- Chain-of-Verification (Dhuliawala et al., 2023) -- Self-Consistency (Wang et al., 2023) -- Multi-Expert Prompting (Wang et al., 2024) -""" - -import argparse -import sys -from typing import Optional - - -def get_phase_name(step: int) -> str: - """Return the phase name for a given step number.""" - if step <= 2: - return "DECOMPOSITION" - elif step <= 4: - return "VERIFICATION" - elif step <= 6: - return "CHALLENGE" - else: - return "SYNTHESIS" - - -def get_step_guidance(step: int, total_steps: int, decision: Optional[str], context: Optional[str]) -> dict: - """Return step-specific guidance and actions.""" - - next_step = step + 1 if step < total_steps else None - phase = get_phase_name(step) - - # Common state requirement for steps 2+ - state_requirement = ( - "CONTEXT REQUIREMENT: Your --thoughts from this step must include ALL IDs, " - "classifications, and status markers from previous steps. This accumulated " - "state is essential for workflow continuity." - ) - - # DECOMPOSITION PHASE - if step == 1: - return { - "phase": phase, - "step_title": "Extract Structure", - "actions": [ - "You are a structured decision critic. Your task is to decompose this " - "decision into its constituent parts so each can be independently verified " - "or challenged. This analysis is critical to the quality of the entire workflow.", - "", - "Extract and assign stable IDs that will persist through ALL subsequent steps:", - "", - "CLAIMS [C1, C2, ...] - Factual assertions (3-7 items)", - " What facts does this decision assume to be true?", - " What cause-effect relationships does it depend on?", - "", - "ASSUMPTIONS [A1, A2, ...] - Unstated beliefs (2-5 items)", - " What is implied but not explicitly stated?", - " What would someone unfamiliar with the context not know?", - "", - "CONSTRAINTS [K1, K2, ...] - Hard boundaries (1-4 items)", - " What technical limitations exist?", - " What organizational/timeline constraints apply?", - "", - "JUDGMENTS [J1, J2, ...] - Subjective tradeoffs (1-3 items)", - " Where are values being weighed against each other?", - " What 'it depends' decisions were made?", - "", - "OUTPUT FORMAT:", - " C1: ", - " C2: ", - " A1: ", - " K1: ", - " J1: ", - "", - "These IDs will be referenced in ALL subsequent steps. Be thorough but focused.", - ], - "next": f"Step {next_step}: Classify each item's verifiability.", - "academic_note": None, - } - - if step == 2: - return { - "phase": phase, - "step_title": "Classify Verifiability", - "actions": [ - "You are a structured decision critic continuing your analysis.", - "", - "Classify each item from Step 1. Retain original IDs and add a verifiability tag.", - "", - "CLASSIFICATIONS:", - "", - " [V] VERIFIABLE - Can be checked against evidence or tested", - " Examples: \"API supports 1000 RPS\" (testable), \"Library X has feature Y\" (checkable)", - "", - " [J] JUDGMENT - Subjective tradeoff with no objectively correct answer", - " Examples: \"Simplicity is more important than flexibility\", \"Risk is acceptable\"", - "", - " [C] CONSTRAINT - Given condition, accepted as fixed for this decision", - " Examples: \"Budget is $50K\", \"Must launch by Q2\", \"Team has 3 engineers\"", - "", - "EDGE CASE RULE: When an item could fit multiple categories, prefer [V] over [J] over [C].", - "Rationale: Verifiable items can be checked; judgments can be debated; constraints are given.", - "", - "Example edge case:", - " \"The team can deliver in 4 weeks\" - Could be [J] (judgment about capacity) or [V] (checkable", - " against past velocity). Choose [V] because it CAN be verified against evidence.", - "", - "OUTPUT FORMAT (preserve original IDs):", - " C1 [V]: ", - " C2 [J]: ", - " A1 [V]: ", - " K1 [C]: ", - "", - "COUNT: State how many [V] items require verification in the next phase.", - "", - state_requirement, - ], - "next": f"Step {next_step}: Generate verification questions for [V] items.", - "academic_note": None, - } - - # VERIFICATION PHASE - if step == 3: - return { - "phase": phase, - "step_title": "Generate Verification Questions", - "actions": [ - "You are a structured decision critic. This step is crucial for catching errors.", - "", - "For each [V] item from Step 2, generate 1-3 verification questions.", - "", - "CRITERIA FOR GOOD QUESTIONS:", - " - Specific and independently answerable", - " - Designed to reveal if the claim is FALSE (falsification focus)", - " - Do not assume the claim is true in the question itself", - " - Each question should test a different aspect of the claim", - "", - "QUESTION BOUNDS:", - " - Simple claims: 1 question", - " - Moderate claims: 2 questions", - " - Complex claims with multiple parts: 3 questions maximum", - "", - "OUTPUT FORMAT:", - " C1 [V]: ", - " Q1: ", - " Q2: ", - " A1 [V]: ", - " Q1: ", - "", - "EXAMPLE:", - " C1 [V]: Retrying failed requests creates race condition risk", - " Q1: Can a retry succeed after another request has already written?", - " Q2: What ordering guarantees exist between concurrent requests?", - "", - state_requirement, - ], - "next": f"Step {next_step}: Answer questions with factored verification.", - "academic_note": ( - "Chain-of-Verification (Dhuliawala et al., 2023): \"Plan verification questions " - "to check its work, and then systematically answer those questions.\"" - ), - } - - if step == 4: - return { - "phase": phase, - "step_title": "Factored Verification", - "actions": [ - "You are a structured decision critic. This verification step is the most important " - "in the entire workflow. Your accuracy here directly determines verdict quality. " - "Take your time and be rigorous.", - "", - "Answer each verification question INDEPENDENTLY.", - "", - "EPISTEMIC BOUNDARY (critical for avoiding confirmation bias):", - "", - " Answer using ONLY:", - " (a) Established domain knowledge - facts you would find in documentation,", - " textbooks, or widely-accepted technical references", - " (b) Stated constraints - information explicitly provided in the decision context", - " (c) Logical inference - deductions from first principles that would hold", - " regardless of whether this specific decision is correct", - "", - " Do NOT:", - " - Assume the decision is correct and work backward", - " - Assume the decision is incorrect and seek to disprove", - " - Reference whether the claim 'should' be true given the decision", - "", - "SEPARATE your answer from its implication:", - " - ANSWER: The factual response to the question (evidence-based)", - " - IMPLICATION: What this means for the original claim (judgment)", - "", - "Then mark each [V] item:", - " VERIFIED - Answers are consistent with the claim", - " FAILED - Answers reveal inconsistency, error, or contradiction", - " UNCERTAIN - Insufficient evidence; state what additional information would resolve", - "", - "OUTPUT FORMAT:", - " C1 [V]: ", - " Q1: ", - " Answer: ", - " Implication: ", - " Status: VERIFIED | FAILED | UNCERTAIN", - " Rationale: ", - "", - state_requirement, - ], - "next": f"Step {next_step}: Begin challenge phase with adversarial analysis.", - "academic_note": ( - "Chain-of-Verification: \"Factored variants which separate out verification steps, " - "in terms of which context is attended to, give further performance gains.\"" - ), - } - - # CHALLENGE PHASE - if step == 5: - return { - "phase": phase, - "step_title": "Contrarian Perspective", - "actions": [ - "You are a structured decision critic shifting to adversarial analysis.", - "", - "Your task: Generate the STRONGEST possible argument AGAINST the decision.", - "", - "START FROM VERIFICATION RESULTS:", - " - FAILED items are direct ammunition - the decision rests on false premises", - " - UNCERTAIN items are attack vectors - unverified assumptions create risk", - " - Even VERIFIED items may have hidden dependencies worth probing", - "", - "STEEL-MANNING: Present the opposition's BEST case, not a strawman.", - "Ask: What would a thoughtful, well-informed critic with domain expertise say?", - "Make the argument as strong as you can, even if you personally disagree.", - "", - "ATTACK VECTORS TO EXPLORE:", - " - What could go wrong that wasn't considered?", - " - What alternatives were dismissed too quickly?", - " - What second-order effects were missed?", - " - What happens if key assumptions change?", - " - Who would disagree, and why might they be right?", - "", - "OUTPUT FORMAT:", - "", - "CONTRARIAN POSITION: ", - "", - "ARGUMENT:", - "", - "", - "KEY RISKS:", - "- ", - "- ", - "- ", - "", - state_requirement, - ], - "next": f"Step {next_step}: Explore alternative problem framing.", - "academic_note": ( - "Multi-Expert Prompting (Wang et al., 2024): \"Integrating multiple experts' " - "perspectives catches blind spots in reasoning.\"" - ), - } - - if step == 6: - return { - "phase": phase, - "step_title": "Alternative Framing", - "actions": [ - "You are a structured decision critic examining problem formulation.", - "", - "PURPOSE: Step 5 challenged the SOLUTION. This step challenges the PROBLEM STATEMENT.", - "Goal: Reveal hidden assumptions baked into how the problem was originally framed.", - "", - "Set aside the proposed solution temporarily. Ask:", - " 'If I approached this problem fresh, how might I state it differently?'", - "", - "REFRAMING VECTORS:", - " - Is this the right problem to solve, or a symptom of a deeper issue?", - " - What would a different stakeholder (user, ops, security) prioritize?", - " - What if the constraints (K items) were different or negotiable?", - " - Is there a simpler formulation that dissolves the tradeoffs?", - " - What objectives might be missing from the original framing?", - "", - "OUTPUT FORMAT:", - "", - "ALTERNATIVE FRAMING: ", - "", - "WHAT THIS FRAMING EMPHASIZES:", - "", - "", - "HIDDEN ASSUMPTIONS REVEALED:", - "", - "", - "IMPLICATION FOR DECISION:", - "", - "", - state_requirement, - ], - "next": f"Step {next_step}: Synthesize findings into verdict.", - "academic_note": None, - } - - # SYNTHESIS PHASE - if step == 7: - return { - "phase": phase, - "step_title": "Synthesis and Verdict", - "actions": [ - "You are a structured decision critic delivering your final assessment.", - "This verdict will guide real decisions. Be confident in your analysis and precise " - "in your recommendation.", - "", - "VERDICT RUBRIC:", - "", - " ESCALATE when ANY of these apply:", - " - Any FAILED item involves safety, security, or compliance", - " - Any UNCERTAIN item is critical AND cannot be cheaply verified", - " - The alternative framing reveals the problem itself is wrong", - "", - " REVISE when ANY of these apply:", - " - Any FAILED item on a core claim (not peripheral)", - " - Multiple UNCERTAIN items on feasibility, effort, or impact", - " - Challenge phase revealed unaddressed gaps that change the calculus", - "", - " STAND when ALL of these apply:", - " - No FAILED items on core claims", - " - UNCERTAIN items are explicitly acknowledged as accepted risks", - " - Challenges from Steps 5-6 are addressable within the current approach", - "", - "BORDERLINE CASES:", - " - When between STAND and REVISE: favor REVISE (cheaper to refine than to fail)", - " - When between REVISE and ESCALATE: state both options with conditions", - "", - "OUTPUT FORMAT:", - "", - "VERDICT: [STAND | REVISE | ESCALATE]", - "", - "VERIFICATION SUMMARY:", - " Verified: ", - " Failed: ", - " Uncertain: ", - "", - "CHALLENGE ASSESSMENT:", - " Strongest challenge: ", - " Alternative framing insight: ", - " Response: ", - "", - "RECOMMENDATION:", - " ", - ], - "next": None, - "academic_note": ( - "Self-Consistency (Wang et al., 2023): \"Correct reasoning processes tend to " - "have greater agreement in their final answer than incorrect processes.\"" - ), - } - - return { - "phase": "UNKNOWN", - "step_title": "Unknown Step", - "actions": ["Invalid step number."], - "next": None, - "academic_note": None, - } - - -def format_output(step: int, total_steps: int, guidance: dict) -> str: - """Format the output for display.""" - lines = [] - - # Header - lines.append(f"DECISION CRITIC - Step {step}/{total_steps}: {guidance['step_title']}") - lines.append(f"Phase: {guidance['phase']}") - lines.append("") - - # Actions - for action in guidance["actions"]: - lines.append(action) - lines.append("") - - # Academic note if present - if guidance.get("academic_note"): - lines.append(f"[{guidance['academic_note']}]") - lines.append("") - - # Next step or completion - if guidance["next"]: - lines.append(f"NEXT: {guidance['next']}") - else: - lines.append("WORKFLOW COMPLETE - Present verdict to user.") - - return "\n".join(lines) - - -def main(): - parser = argparse.ArgumentParser( - description="Decision Critic - Structured decision criticism workflow" - ) - parser.add_argument( - "--step-number", - type=int, - required=True, - help="Current step number (1-7)", - ) - parser.add_argument( - "--total-steps", - type=int, - required=True, - help="Total steps in workflow (always 7)", - ) - parser.add_argument( - "--decision", - type=str, - help="The decision being criticized (required for step 1)", - ) - parser.add_argument( - "--context", - type=str, - help="Relevant constraints and background (required for step 1)", - ) - parser.add_argument( - "--thoughts", - type=str, - required=True, - help="Your analysis, findings, and progress from previous steps", - ) - - args = parser.parse_args() - - # Validate step number - if args.step_number < 1 or args.step_number > 7: - print("ERROR: step-number must be between 1 and 7", file=sys.stderr) - sys.exit(1) - - # Validate step 1 requirements - if args.step_number == 1: - if not args.decision: - print("ERROR: --decision is required for step 1", file=sys.stderr) - sys.exit(1) - - # Get guidance for current step - guidance = get_step_guidance( - args.step_number, - args.total_steps, - args.decision, - args.context, - ) - - # Print decision context on step 1 - if args.step_number == 1: - print("DECISION UNDER REVIEW:") - print(args.decision) - if args.context: - print("") - print("CONTEXT:") - print(args.context) - print("") - - # Print formatted output - print(format_output(args.step_number, args.total_steps, guidance)) - - -if __name__ == "__main__": - main() diff --git a/.claude/skills/doc-sync/CLAUDE.md b/.claude/skills/doc-sync/CLAUDE.md deleted file mode 100644 index 3e111d5..0000000 --- a/.claude/skills/doc-sync/CLAUDE.md +++ /dev/null @@ -1,14 +0,0 @@ -# skills/doc-sync/ - -## Files - -| File | What | When to read | -| ---- | ---- | ------------ | -| `README.md` | Skill overview and usage examples | Understanding when to use doc-sync | -| `SKILL.md` | Complete skill workflow definition | Executing the doc-sync skill | - -## Subdirectories - -| Directory | What | When to read | -| --------- | ---- | ------------ | -| `references/` | Trigger pattern examples | Writing good CLAUDE.md triggers | diff --git a/.claude/skills/doc-sync/README.md b/.claude/skills/doc-sync/README.md deleted file mode 100644 index e80d101..0000000 --- a/.claude/skills/doc-sync/README.md +++ /dev/null @@ -1,46 +0,0 @@ -# Doc Sync - -The CLAUDE.md/README.md hierarchy is central to context hygiene. CLAUDE.md files -are pure indexes -- tabular navigation with "What" and "When to read" columns -that help LLMs (and humans) find relevant files without loading everything. -README.md files capture invisible knowledge: architecture decisions, design -tradeoffs, and invariants that are not apparent from reading code. - -The doc-sync skill audits and synchronizes this hierarchy across a repository. - -## How It Works - -The skill operates in five phases: - -1. **Discovery** -- Maps all directories, identifies missing or outdated - CLAUDE.md files -2. **Audit** -- Checks for drift (files added/removed but not indexed), - misplaced content (architecture docs in CLAUDE.md instead of README.md) -3. **Migration** -- Moves architectural content from CLAUDE.md to README.md -4. **Update** -- Creates/updates indexes with proper tabular format -5. **Verification** -- Confirms complete coverage and correct structure - -## When to Use - -Use this skill for: - -- **Bootstrapping** -- Adopting this workflow on an existing repository -- **After bulk changes** -- Major refactors, directory restructuring -- **Periodic audits** -- Checking for documentation drift -- **Onboarding** -- Before starting work on an unfamiliar codebase - -If you use the planning workflow consistently, the technical writer agent -maintains documentation as part of execution. As such, doc-sync is primarily for -bootstrapping or recovery -- not routine use. - -## Example Usage - -``` -Use your doc-sync skill to synchronize documentation across this repository -``` - -For targeted updates: - -``` -Use your doc-sync skill to update documentation in src/validators/ -``` diff --git a/.claude/skills/doc-sync/SKILL.md b/.claude/skills/doc-sync/SKILL.md deleted file mode 100644 index 727de3d..0000000 --- a/.claude/skills/doc-sync/SKILL.md +++ /dev/null @@ -1,315 +0,0 @@ ---- -name: doc-sync -description: Synchronizes CLAUDE.md navigation indexes and README.md architecture docs across a repository. Use when asked to "sync docs", "update CLAUDE.md files", "ensure documentation is in sync", "audit documentation", or when documentation maintenance is needed after code changes. ---- - -# Doc Sync - -Maintains the CLAUDE.md navigation hierarchy and optional README.md architecture docs across a repository. This skill is self-contained and performs all documentation work directly. - -## Scope Resolution - -Determine scope FIRST: - -| User Request | Scope | -| ------------------------------------------------------- | ----------------------------------------- | -| "sync docs" / "update documentation" / no specific path | REPOSITORY-WIDE | -| "sync docs in src/validator/" | DIRECTORY: src/validator/ and descendants | -| "update CLAUDE.md for parser.py" | FILE: single file's parent directory | - -For REPOSITORY-WIDE scope, perform a full audit. For narrower scopes, operate only within the specified boundary. - -## CLAUDE.md Format Specification - -### Index Format - -Use tabular format with What and When columns: - -```markdown -## Files - -| File | What | When to read | -| ----------- | ------------------------------ | ----------------------------------------- | -| `cache.rs` | LRU cache with O(1) operations | Implementing caching, debugging evictions | -| `errors.rs` | Error types and Result aliases | Adding error variants, handling failures | - -## Subdirectories - -| Directory | What | When to read | -| ----------- | ----------------------------- | ----------------------------------------- | -| `config/` | Runtime configuration loading | Adding config options, modifying defaults | -| `handlers/` | HTTP request handlers | Adding endpoints, modifying request flow | -``` - -### Column Guidelines - -- **File/Directory**: Use backticks around names: `cache.rs`, `config/` -- **What**: Factual description of contents (nouns, not actions) -- **When to read**: Task-oriented triggers using action verbs (implementing, debugging, modifying, adding, understanding) -- At least one column must have content; empty cells use `-` - -### Trigger Quality Test - -Given task "add a new validation rule", can an LLM scan the "When to read" column and identify the right file? - -### ROOT vs SUBDIRECTORY CLAUDE.md - -**ROOT CLAUDE.md:** - -```markdown -# [Project Name] - -[One sentence: what this is] - -## Files - -| File | What | When to read | -| ---- | ---- | ------------ | - -## Subdirectories - -| Directory | What | When to read | -| --------- | ---- | ------------ | - -## Build - -[Copy-pasteable command] - -## Test - -[Copy-pasteable command] - -## Development - -[Setup instructions, environment requirements, workflow notes] -``` - -**SUBDIRECTORY CLAUDE.md:** - -```markdown -# [directory-name]/ - -## Files - -| File | What | When to read | -| ---- | ---- | ------------ | - -## Subdirectories - -| Directory | What | When to read | -| --------- | ---- | ------------ | -``` - -**Critical constraint:** Subdirectory CLAUDE.md files are PURE INDEX. No prose, no overview sections, no architectural explanations. Those belong in README.md. - -## README.md Specification - -### Creation Criteria (Invisible Knowledge Test) - -Create README.md ONLY when the directory contains knowledge NOT visible from reading the code: - -- Multiple components interact through non-obvious contracts or protocols -- Design tradeoffs were made that affect how code should be modified -- The directory's structure encodes domain knowledge (e.g., processing order matters) -- Failure modes or edge cases aren't apparent from reading individual files -- There are "rules" developers must follow that aren't enforced by the compiler/linter - -**DO NOT create README.md when:** - -- The directory is purely organizational (just groups related files) -- Code is self-explanatory with good function/module docs -- You'd be restating what CLAUDE.md index entries already convey - -### Content Test - -For each sentence in README.md, ask: "Could a developer learn this by reading the source files?" - -- If YES: delete the sentence -- If NO: keep it - -README.md earns its tokens by providing INVISIBLE knowledge: the reasoning behind the code, not descriptions of the code. - -### README.md Structure - -```markdown -# [Component Name] - -## Overview - -[One paragraph: what problem this solves, high-level approach] - -## Architecture - -[How sub-components interact; data flow; key abstractions] - -## Design Decisions - -[Tradeoffs made and why; alternatives considered] - -## Invariants - -[Rules that must be maintained; constraints not enforced by code] -``` - -## Workflow - -### Phase 1: Discovery - -Map directories requiring CLAUDE.md verification: - -```bash -# Find all directories (excluding .git, node_modules, __pycache__, etc.) -find . -type d \( -name .git -o -name node_modules -o -name __pycache__ -o -name .venv -o -name target -o -name dist -o -name build \) -prune -o -type d -print -``` - -For each directory in scope, record: - -1. Does CLAUDE.md exist? -2. If yes, does it have the required table-based index structure? -3. What files/subdirectories exist that need indexing? - -### Phase 2: Audit - -For each directory, check for drift and misplaced content: - -``` - -CLAUDE.md exists: [YES/NO] -Has table-based index: [YES/NO] -Files in directory: [list] -Files in index: [list] -Missing from index: [list] -Stale in index (file deleted): [list] -Triggers are task-oriented: [YES/NO/PARTIAL] -Contains misplaced content: [YES/NO] (architecture/design docs that belong in README.md) -README.md exists: [YES/NO] -README.md warranted: [YES/NO] (invisible knowledge present?) - -``` - -### Phase 3: Content Migration - -**Critical:** If CLAUDE.md contains content that does NOT belong there, migrate it: - -Content that MUST be moved from CLAUDE.md to README.md: - -- Architecture explanations or diagrams -- Design decision documentation -- Component interaction descriptions -- Overview sections with prose (in subdirectory CLAUDE.md files) -- Invariants or rules documentation -- Any "why" explanations beyond simple triggers - -Migration process: - -1. Identify misplaced content in CLAUDE.md -2. Create or update README.md with the architectural content -3. Strip CLAUDE.md down to pure index format -4. Add README.md to the CLAUDE.md index table - -### Phase 4: Index Updates - -For each directory needing work: - -**Creating/Updating CLAUDE.md:** - -1. Use the appropriate template (ROOT or SUBDIRECTORY) -2. Populate tables with all files and subdirectories -3. Write "What" column: factual content description -4. Write "When to read" column: action-oriented triggers -5. If README.md exists, include it in the Files table - -**Creating README.md (only when warranted):** - -1. Verify invisible knowledge criteria are met -2. Document architecture, design decisions, invariants -3. Apply the content test: remove anything visible from code -4. Keep under ~500 tokens - -### Phase 5: Verification - -After all updates complete, verify: - -1. Every directory in scope has CLAUDE.md -2. All CLAUDE.md files use table-based index format -3. No drift remains (files <-> index entries match) -4. No misplaced content in CLAUDE.md (architecture docs moved to README.md) -5. README.md files are indexed in their parent CLAUDE.md -6. Subdirectory CLAUDE.md files contain no prose/overview sections - -## Output Format - -``` -## Doc Sync Report - -### Scope: [REPOSITORY-WIDE | directory path] - -### Changes Made -- CREATED: [list of new CLAUDE.md files] -- UPDATED: [list of modified CLAUDE.md files] -- MIGRATED: [list of content moved from CLAUDE.md to README.md] -- CREATED: [list of new README.md files] -- FLAGGED: [any issues requiring human decision] - -### Verification -- Directories audited: [count] -- CLAUDE.md coverage: [count]/[total] (100%) -- Drift detected: [count] entries fixed -- Content migrations: [count] (architecture docs moved to README.md) -- README.md files: [count] (only where warranted) -``` - -## Exclusions - -DO NOT index: - -- Generated files (dist/, build/, _.generated._, compiled outputs) -- Vendored dependencies (node_modules/, vendor/, third_party/) -- Git internals (.git/) -- IDE/editor configs (.idea/, .vscode/ unless project-specific settings) - -DO index: - -- Hidden config files that affect development (.eslintrc, .env.example, .gitignore) -- Test files and test directories -- Documentation files (including README.md) - -## Anti-Patterns - -### Index Anti-Patterns - -**Too vague (matches everything):** - -```markdown -| `config/` | Configuration | Working with configuration | -``` - -**Content description instead of trigger:** - -```markdown -| `cache.rs` | Contains the LRU cache implementation | - | -``` - -**Missing action verb:** - -```markdown -| `parser.py` | Input parsing | Input parsing and format handling | -``` - -### Correct Examples - -```markdown -| `cache.rs` | LRU cache with O(1) get/set | Implementing caching, debugging misses, tuning eviction | -| `config/` | YAML config parsing, env overrides | Adding config options, changing defaults, debugging config loading | -``` - -## When NOT to Use This Skill - -- Single file documentation (inline comments, docstrings) - handle directly -- Code comments - handle directly -- Function/module docstrings - handle directly -- This skill is for CLAUDE.md/README.md synchronization specifically - -## Reference - -For additional trigger pattern examples, see `references/trigger-patterns.md`. diff --git a/.claude/skills/doc-sync/references/trigger-patterns.md b/.claude/skills/doc-sync/references/trigger-patterns.md deleted file mode 100644 index faa709d..0000000 --- a/.claude/skills/doc-sync/references/trigger-patterns.md +++ /dev/null @@ -1,125 +0,0 @@ -# Trigger Patterns Reference - -Examples of well-formed triggers for CLAUDE.md index table entries. - -## Column Formula - -| File | What | When to read | -| ------------ | -------------------------------- | ------------------------------------- | -| `[filename]` | [noun-based content description] | [action verb] [specific context/task] | - -## Action Verbs by Category - -### Implementation Tasks - -implementing, adding, creating, building, writing, extending - -### Modification Tasks - -modifying, updating, changing, refactoring, migrating - -### Debugging Tasks - -debugging, troubleshooting, investigating, diagnosing, fixing - -### Understanding Tasks - -understanding, learning, reviewing, analyzing, exploring - -## Examples by File Type - -### Source Code Files - -| File | What | When to read | -| -------------- | ----------------------------------- | ---------------------------------------------------------------------------------- | -| `cache.rs` | LRU cache with O(1) operations | Implementing caching, debugging cache misses, modifying eviction policy | -| `auth.rs` | JWT validation, session management | Implementing login/logout, modifying token validation, debugging auth failures | -| `parser.py` | Input parsing, format detection | Modifying input parsing, adding new input formats, debugging parse errors | -| `validator.py` | Validation rules, constraint checks | Adding validation rules, modifying validation logic, understanding validation flow | - -### Configuration Files - -| File | What | When to read | -| -------------- | -------------------------------- | ----------------------------------------------------------------------------- | -| `config.toml` | Runtime config options, defaults | Adding new config options, modifying defaults, debugging configuration issues | -| `.env.example` | Environment variable template | Setting up development environment, adding new environment variables | -| `Cargo.toml` | Rust dependencies, build config | Adding dependencies, modifying build configuration, debugging build issues | - -### Test Files - -| File | What | When to read | -| -------------------- | --------------------------- | -------------------------------------------------------------------------------- | -| `test_cache.py` | Cache unit tests | Adding cache tests, debugging test failures, understanding cache behavior | -| `integration_tests/` | Cross-component test suites | Adding integration tests, debugging cross-component issues, validating workflows | - -### Documentation Files - -| File | What | When to read | -| ----------------- | ---------------------------------------- | ---------------------------------------------------------------------------------------- | -| `README.md` | Architecture, design decisions | Understanding architecture, design decisions, component relationships | -| `ARCHITECTURE.md` | System design, component boundaries | Understanding system design, component boundaries, data flow | -| `API.md` | Endpoint specs, request/response formats | Implementing API endpoints, understanding request/response formats, debugging API issues | - -### Index Files (cross-cutting concerns) - -| File | What | When to read | -| ------------------------- | ---------------------------------- | ------------------------------------------------------------------------------- | -| `error-handling-index.md` | Error handling patterns reference | Understanding error handling patterns, failure modes, error recovery strategies | -| `performance-index.md` | Performance optimization reference | Optimizing latency, throughput, resource usage, understanding cost models | -| `security-index.md` | Security patterns reference | Implementing authentication, encryption, threat mitigation, compliance features | - -## Examples by Directory Type - -### Feature Directories - -| Directory | What | When to read | -| ---------- | --------------------------------------- | ------------------------------------------------------------------------------------- | -| `auth/` | Authentication, authorization, sessions | Implementing authentication, authorization, session management, debugging auth issues | -| `api/` | HTTP endpoints, request handling | Implementing endpoints, modifying request handling, debugging API responses | -| `storage/` | Persistence, data access layer | Implementing persistence, modifying data access, debugging storage issues | - -### Layer Directories - -| Directory | What | When to read | -| ----------- | ----------------------------- | -------------------------------------------------------------------------------- | -| `handlers/` | Request handlers, routing | Implementing request handlers, modifying routing, debugging request processing | -| `models/` | Data models, schemas | Adding data models, modifying schemas, understanding data structures | -| `services/` | Business logic, service layer | Implementing business logic, modifying service interactions, debugging workflows | - -### Utility Directories - -| Directory | What | When to read | -| ---------- | --------------------------------- | ---------------------------------------------------------------------------------- | -| `utils/` | Helper functions, common patterns | Needing helper functions, implementing common patterns, debugging utility behavior | -| `scripts/` | Maintenance tasks, automation | Running maintenance tasks, automating workflows, debugging script execution | -| `tools/` | Development tools, CLI utilities | Using development tools, implementing tooling, debugging tool behavior | - -## Anti-Patterns - -### Too Vague (matches everything) - -| File | What | When to read | -| ---------- | ------------- | -------------------------- | -| `config/` | Configuration | Working with configuration | -| `utils.py` | Utilities | When you need utilities | - -### Content Description Only (no trigger) - -| File | What | When to read | -| ---------- | --------------------------------------------- | ------------ | -| `cache.rs` | Contains the LRU cache implementation | - | -| `auth.rs` | Authentication logic including JWT validation | - | - -### Missing Action Verb - -| File | What | When to read | -| -------------- | ---------------- | --------------------------------- | -| `parser.py` | Input parsing | Input parsing and format handling | -| `validator.py` | Validation rules | Validation rules and constraints | - -## Trigger Guidelines - -- Combine 2-4 triggers per entry using commas or "or" -- Use action verbs: implementing, debugging, modifying, adding, understanding -- Be specific: "debugging cache misses" not "debugging" -- If more than 4 triggers needed, the file may be doing too much diff --git a/.claude/skills/incoherence/CLAUDE.md b/.claude/skills/incoherence/CLAUDE.md deleted file mode 100644 index 57cd6cd..0000000 --- a/.claude/skills/incoherence/CLAUDE.md +++ /dev/null @@ -1,24 +0,0 @@ -# skills/incoherence/ - -## Overview - -Incoherence detection skill using parallel agents. IMMEDIATELY invoke the -script -- do NOT explore first. - -## Index - -| File/Directory | Contents | Read When | -| ------------------------ | ----------------- | ------------------ | -| `SKILL.md` | Invocation | Using this skill | -| `scripts/incoherence.py` | Complete workflow | Debugging behavior | - -## Key Point - -The script IS the workflow. Three phases: - -- Detection (steps 1-12): Survey, explore, verify candidates -- Resolution (steps 13-15): Interactive AskUserQuestion prompts -- Application (steps 16-21): Apply changes, present final report - -Resolution is interactive - user answers structured questions inline. No manual -file editing required. diff --git a/.claude/skills/incoherence/SKILL.md b/.claude/skills/incoherence/SKILL.md deleted file mode 100644 index 559d239..0000000 --- a/.claude/skills/incoherence/SKILL.md +++ /dev/null @@ -1,37 +0,0 @@ ---- -name: incoherence -description: Detect and resolve incoherence in documentation, code, specs vs implementation. ---- - -# Incoherence Detector - -When this skill activates, IMMEDIATELY invoke the script. The script IS the -workflow. - -## Invocation - -```bash -python3 scripts/incoherence.py \ - --step-number 1 \ - --total-steps 21 \ - --thoughts "" -``` - -| Argument | Required | Description | -| --------------- | -------- | ----------------------------------------- | -| `--step-number` | Yes | Current step (1-21) | -| `--total-steps` | Yes | Always 21 | -| `--thoughts` | Yes | Accumulated state from all previous steps | - -Do NOT explore or detect first. Run the script and follow its output. - -## Workflow Phases - -1. **Detection (steps 1-12)**: Survey codebase, explore dimensions, verify - candidates -2. **Resolution (steps 13-15)**: Present issues via AskUserQuestion, collect - user decisions -3. **Application (steps 16-21)**: Apply resolutions, present final report - -Resolution is interactive - user answers structured questions inline. No manual -file editing required. diff --git a/.claude/skills/incoherence/scripts/incoherence.py b/.claude/skills/incoherence/scripts/incoherence.py deleted file mode 100755 index f145a3e..0000000 --- a/.claude/skills/incoherence/scripts/incoherence.py +++ /dev/null @@ -1,1234 +0,0 @@ -#!/usr/bin/env python3 -""" -Incoherence Detector - Step-based incoherence detection workflow - -Usage: - python3 incoherence.py --step-number 1 --total-steps 21 --thoughts "Analyzing project X" - -DETECTION PHASE (Steps 1-12): - Steps 1-3 (Parent): Survey, dimension selection, exploration dispatch - Steps 4-7 (Sub-Agent): Broad sweep, coverage check, gap-fill, format findings - Step 8 (Parent): Synthesis & candidate selection - Step 9 (Parent): Deep-dive dispatch - Steps 10-11 (Sub-Agent): Deep-dive exploration and formatting - Step 12 (Parent): Verdict analysis and grouping - -INTERACTIVE RESOLUTION PHASE (Steps 13-15): - Step 13 (Parent): Prepare resolution batches from groups - Step 14 (Parent): Present batch via AskUserQuestion - - Group batches: ask group question ONLY first - - Non-group or MODE=individual: ask per-issue questions - Step 15 (Parent): Loop controller - - If unified chosen: record for all, next batch - - If individual chosen: loop to step 14 with MODE=individual - - If all batches done: proceed to application - -APPLICATION PHASE (Steps 16-21): - Step 16 (Parent): Analyze targets and select agent types - Step 17 (Parent): Dispatch current wave of agents - Steps 18-19 (Sub-Agent): Apply resolution, format result - Step 20 (Parent): Collect wave results, check for next wave - Step 21 (Parent): Present final report to user - -Resolution is interactive - user answers AskUserQuestion prompts inline. -No manual file editing required. -""" - -import argparse -import sys -import os - -DIMENSION_CATALOG = """ -ABSTRACT DIMENSION CATALOG -========================== - -Choose dimensions from this catalog based on Step 1 info sources. - -CATEGORY A: SPECIFICATION VS BEHAVIOR - - README/docs claim X, but code does Y - - API documentation vs actual API behavior - - Examples in docs that don't actually work - Source pairs: Documentation <-> Code implementation - -CATEGORY B: INTERFACE CONTRACT INTEGRITY - - Type definitions vs actual runtime values - - Schema definitions vs validation behavior - - Function signatures vs docstrings - Source pairs: Type/Schema definitions <-> Runtime behavior - -CATEGORY C: CROSS-REFERENCE CONSISTENCY - - Same concept described differently in different docs - - Numeric constants/limits stated inconsistently - - Intra-document contradictions - Source pairs: Document <-> Document - -CATEGORY D: TEMPORAL CONSISTENCY (Staleness) - - Outdated comments referencing removed code - - TODO/FIXME comments for completed work - - References to renamed/moved files - Source pairs: Historical references <-> Current state - -CATEGORY E: ERROR HANDLING CONSISTENCY - - Documented error codes vs actual error responses - - Exception handling docs vs throw/catch behavior - Source pairs: Error documentation <-> Error implementation - -CATEGORY F: CONFIGURATION & ENVIRONMENT - - Documented env vars vs actual env var usage - - Default values in docs vs defaults in code - Source pairs: Config documentation <-> Config handling code - -CATEGORY G: AMBIGUITY & UNDERSPECIFICATION - - Vague statements that could be interpreted multiple ways - - Missing thresholds, limits, or parameters - - Implicit assumptions not stated explicitly - Detection method: Ask "could two people read this differently?" - -CATEGORY H: POLICY & CONVENTION COMPLIANCE - - Architectural decisions (ADRs) violated by implementation - - Style guide rules not followed in code - - "We don't do X" statements violated in codebase - Source pairs: Policy documents <-> Implementation patterns - -CATEGORY I: COMPLETENESS & DOCUMENTATION GAPS - - Public API endpoints with no documentation - - Functions/classes with no docstrings - - Magic values/constants without explanation - Detection method: Find code constructs, check if docs exist - -CATEGORY J: COMPOSITIONAL CONSISTENCY - - Claims individually valid but jointly impossible - - Numeric constraints that contradict when combined - - Configuration values that create impossible states - - Timing/resource constraints that cannot all be satisfied - Detection method: Gather related claims, compute implications, check for contradiction - Example: timeout=30s, retries=10, max_duration=60s → 30×10=300≠60 - -CATEGORY K: IMPLICIT CONTRACT INTEGRITY - - Names/identifiers promise behavior the code doesn't deliver - - Function named validateX() that doesn't actually validate - - Error messages that misrepresent the actual error - - Module/package names that don't match contents - - Log messages that lie about what happened - Detection method: Parse names semantically, infer promise, compare to behavior - Note: LLMs are particularly susceptible to being misled by names - -CATEGORY L: DANGLING SPECIFICATION REFERENCES - - Entity A references entity B, but B is never defined anywhere - - FK references table that has no schema (e.g., api_keys.tenant_id but no tenants table) - - UI/API mentions endpoints or types that are not specified - - Schema field references enum or type with no definition - Detection method: - 1. Extract DEFINED entities (tables, APIs, types, enums) with locations - 2. Extract REFERENCED entities (FKs, type usages, API calls) with locations - 3. Report: referenced but not defined = dangling reference - Source pairs: Any specification -> Cross-file entity registry - Note: Distinct from I (code-without-docs). L is SPEC-without-SPEC. - -CATEGORY M: INCOMPLETE SPECIFICATION DEFINITIONS - - Entity is defined but missing components required for implementation - - Table schema documented but missing fields that other docs reference - - API endpoint defined but missing request/response schema - - Proto/schema has fields but lacks types others expect - Detection method: - 1. For each defined entity, extract CLAIMED components - 2. Cross-reference with EXPECTED components from consuming docs - 3. Report: expected but not claimed = incomplete definition - Source pairs: Definition document <-> Consumer documents - Example: rules table shows (id, name, enabled) but API doc expects 'expression' field - -SELECTION RULES: -- Select ALL categories relevant to Step 1 info sources -- Typical selection is 5-8 dimensions -- G, H, I, K are especially relevant for LLM-assisted coding -- J requires cross-referencing multiple claims (more expensive) -- L, M are critical for design-phase docs and specs-to-be-implemented - Select when docs describe systems that need to be built -""" - - -def get_step_guidance(step_number, total_steps, script_path=None): - if script_path is None: - script_path = os.path.abspath(__file__) - - # ========================================================================= - # DETECTION PHASE: Steps 1-9 - # ========================================================================= - - if step_number == 1: - return { - "actions": [ - "CODEBASE SURVEY", - "", - "Gather MINIMAL context. Do NOT read domain-specific docs.", - "", - "ALLOWED: README.md (first 50 lines), CLAUDE.md, directory listing, package manifest", - "NOT ALLOWED: Detailed docs, source code, configs, tests", - "", - "Identify:", - "1. CODEBASE TYPE: library/service/CLI/framework/application", - "2. PRIMARY LANGUAGE", - "3. DOCUMENTATION LOCATIONS", - "4. INFO SOURCE TYPES:", - " [ ] README/guides [ ] API docs [ ] Code comments", - " [ ] Type definitions [ ] Configs [ ] Schemas", - " [ ] ADRs [ ] Style guides [ ] CONTRIBUTING.md", - " [ ] Test descriptions [ ] Error catalogs", - ], - "next": "Invoke step 2 with survey results in --thoughts" - } - - if step_number == 2: - return { - "actions": [ - "DIMENSION SELECTION", - "", - "Select from catalog (A-K) based on Step 1 info sources.", - "Do NOT read files. Do NOT create domain-specific dimensions.", - "", - DIMENSION_CATALOG, - "", - "OUTPUT: List selected dimensions with rationale.", - ], - "next": "Invoke step 3 with selected dimensions in --thoughts" - } - - if step_number == 3: - return { - "actions": [ - "EXPLORATION DISPATCH", - "", - "Launch one haiku Explore agent per dimension.", - "Launch ALL in a SINGLE message for parallelism.", - "", - f"SCRIPT PATH: {script_path}", - "", - "AGENT PROMPT TEMPLATE (copy exactly, fill placeholders):", - "```", - "DIMENSION EXPLORATION TASK", - "", - "DIMENSION: {category_letter} - {dimension_name}", - "DESCRIPTION: {description_from_catalog}", - "", - "Start by invoking:", - f" python3 {script_path} --step-number 4 --total-steps 21 \\", - " --thoughts \"Dimension: {category_letter} - {dimension_name}\"", - "```", - ], - "next": "After all agents complete, invoke step 8 with combined findings" - } - - # ========================================================================= - # EXPLORATION SUB-AGENT STEPS: 4-7 - # ========================================================================= - - if step_number == 4: - return { - "actions": [ - "BROAD SWEEP [SUB-AGENT]", - "", - "Cast a WIDE NET. Prioritize recall over precision.", - "Report ANYTHING that MIGHT be incoherence. Verification comes later.", - "", - "Your dimension (from --thoughts) tells you what to look for.", - "", - "SEARCH STRATEGY:", - " 1. Start with obvious locations (docs/, README, src/)", - " 2. Search for keywords related to your dimension", - " 3. Check configs, schemas, type definitions", - " 4. Look at tests for behavioral claims", - "", - "FOR OMISSION DIMENSIONS (L, M):", - " Before searching for conflicts, BUILD AN ENTITY REGISTRY:", - "", - " 5. Extract DEFINED entities from each doc:", - " - Database tables (CREATE TABLE, schema blocks)", - " - API endpoints (route definitions, endpoint specs)", - " - Types/enums (type definitions, enum declarations)", - " Record: entity_name, entity_type, file:line, components[]", - "", - " 6. Extract REFERENCED entities from each doc:", - " - FK patterns (table_id -> implies 'table' entity)", - " - Type usages (returns UserResponse -> implies UserResponse)", - " - API calls (calls /api/users -> implies endpoint)", - " Record: entity_name, reference_type, file:line", - "", - " 7. Cross-reference:", - " - REFERENCED but not DEFINED -> Category L finding", - " - DEFINED but missing expected components -> Category M finding", - "", - "FOR EACH POTENTIAL FINDING, note:", - " - Location A (file:line)", - " - Location B (file:line)", - " - What might conflict", - " - Confidence: high/medium/low (low is OK!)", - "", - "BIAS: Report more, not fewer. False positives are filtered later.", - "", - "Track which directories/files you searched.", - ], - "next": "Invoke step 5 with your findings and searched locations in --thoughts" - } - - if step_number == 5: - return { - "actions": [ - "COVERAGE CHECK [SUB-AGENT]", - "", - "Review your search coverage. Identify GAPS.", - "", - "ASK YOURSELF:", - " - What directories have I NOT searched?", - " - What file types did I skip? (.yaml, .json, .toml, tests?)", - " - Are there related modules I haven't checked?", - " - Did I only look at obvious places?", - " - What would a second reviewer check that I didn't?", - "", - "DIVERSITY CHECK:", - " - Are all my findings in one directory? (bad)", - " - Are all my findings the same file type? (bad)", - " - Did I check both docs AND code? Both should have claims.", - "", - "OUTPUT:", - " 1. List of gaps/unexplored areas (at least 3)", - " 2. Specific files or patterns to search next", - ], - "next": "Invoke step 6 with identified gaps in --thoughts" - } - - if step_number == 6: - return { - "actions": [ - "GAP-FILL EXPLORATION [SUB-AGENT]", - "", - "Explore the gaps identified in step 5.", - "", - "REQUIREMENTS:", - " - Search at least 3 new locations from your gap list", - " - Use different search strategies than before", - " - Look in non-obvious places (tests, examples, scripts/)", - "", - "ADDITIONAL TECHNIQUES:", - " - Search for negations ('not', 'don't', 'never', 'deprecated')", - " - Look for TODOs, FIXMEs, HACKs near your dimension's topic", - " - Check git-ignored or generated files if accessible", - "", - "Record any new potential incoherences found.", - "Same format: Location A, Location B, conflict, confidence.", - ], - "next": "Invoke step 7 with all findings (original + new) in --thoughts" - } - - if step_number == 7: - return { - "actions": [ - "FORMAT EXPLORATION FINDINGS [SUB-AGENT]", - "", - "Consolidate all findings from your exploration.", - "", - "OUTPUT FORMAT:", - "```", - "EXPLORATION RESULTS - DIMENSION {letter}", - "", - "FINDING 1:", - " Location A: [file:line]", - " Location B: [file:line]", - " Potential conflict: [one-line description]", - " Confidence: high|medium|low", - "", - "[repeat for each finding]", - "", - "TOTAL FINDINGS: N", - "AREAS SEARCHED: [list of directories/file patterns]", - "```", - "", - "Include ALL findings, even low-confidence ones.", - "Deduplication happens in step 8.", - ], - "next": "Output formatted results. Sub-agent task complete." - } - - # ========================================================================= - # DETECTION PHASE CONTINUED: Steps 8-13 - # ========================================================================= - - if step_number == 8: - return { - "actions": [ - "SYNTHESIS & CANDIDATE SELECTION", - "", - "Process ALL findings from exploration phase:", - "", - "1. SCORE: Rate each (0-10) on Impact + Confidence + Specificity + Fixability", - "2. SORT: Order by score descending", - "", - "Output: C1, C2, ... with location, summary, score, DIMENSION.", - "", - "IMPORTANT: Pass ALL scored candidates to verification.", - " - Do NOT limit to 10 or any arbitrary number", - " - If exploration found 25 candidates, pass all 25", - " - Step 9 will launch agents for every candidate", - " - System handles batching automatically", - "", - "NOTE: Deduplication happens AFTER Sonnet verification (step 12)", - "to leverage richer analysis for merge decisions.", - ], - "next": "Invoke step 9 with all candidates in --thoughts" - } - - if step_number == 9: - return { - "actions": [ - "DEEP-DIVE DISPATCH", - "", - "Launch Task agents (subagent_type='general-purpose', model='sonnet')", - "to verify each candidate.", - "", - "CRITICAL: Launch ALL candidates in a SINGLE message.", - " - Do NOT self-limit to 10 or any other number", - " - If you have 15 candidates, launch 15 agents", - " - If you have 30 candidates, launch 30 agents", - " - Claude Code automatically queues and batches execution", - " - All agents will complete before step 10 proceeds", - "", - "Sub-agents will invoke THIS SCRIPT to get their instructions.", - "", - f"SCRIPT PATH: {script_path}", - "", - "AGENT PROMPT TEMPLATE (copy exactly, fill placeholders):", - "```", - "DEEP-DIVE VERIFICATION TASK", - "", - "CANDIDATE: {id} at {location}", - "DIMENSION: {dimension_letter} - {dimension_name}", - "Claimed issue: {summary}", - "", - "YOUR WORKFLOW:", - "", - "STEP A: Get exploration instructions", - f" python3 {script_path} --step-number 10 --total-steps 21 --thoughts \"Verifying: {{id}}\"", - "", - "STEP B: Follow those instructions to gather evidence", - "", - "STEP C: Format your findings", - f" python3 {script_path} --step-number 11 --total-steps 21 --thoughts \"\"", - "", - "IMPORTANT: You MUST invoke step 10 before exploring, step 11 to format.", - "```", - ], - "next": "After all agents complete, invoke step 12 with all verdicts" - } - - # ========================================================================= - # DEEP-DIVE SUB-AGENT STEPS: 10-11 - # ========================================================================= - - if step_number == 10: - return { - "actions": [ - "DEEP-DIVE EXPLORATION [SUB-AGENT]", - "", - "You are verifying a specific candidate. Follow this process:", - "", - "1. LOCATE PRIMARY SOURCE", - " - Navigate to exact file:line", - " - Read 100+ lines of context", - " - Identify the claim being made", - "", - "2. FIND CONFLICTING SOURCE", - " - Locate the second source", - " - Read its context too", - "", - "3. EXTRACT EVIDENCE", - " For EACH source: file path, line number, exact quote, claim", - "", - "4. ANALYZE BY DIMENSION TYPE", - "", - " Check the DIMENSION from your task prompt, then apply:", - "", - " FOR CONTRADICTION DIMENSIONS (A, B, C, E, F, J, K):", - " - Same thing discussed?", - " - Actually contradictory?", - " - Context resolves it?", - " -> If genuinely contradictory: TRUE_INCOHERENCE", - "", - " FOR AMBIGUITY DIMENSION (G):", - " - Could two competent readers interpret this differently?", - " - Would clarification benefit users?", - " -> If ambiguous and clarification helps: SIGNIFICANT_AMBIGUITY", - "", - " FOR COMPLETENESS DIMENSION (I):", - " - Is there missing information readers need?", - " - Would documentation here benefit users?", - " -> If gap exists and docs needed: DOCUMENTATION_GAP", - "", - " FOR POLICY DIMENSION (H):", - " - Orphaned references to deleted content?", - " -> DOCUMENTATION_GAP", - " - Active policy being violated?", - " -> TRUE_INCOHERENCE", - "", - " FOR OMISSION DIMENSIONS (L, M):", - " - Is the referenced entity defined ANYWHERE in the doc corpus?", - " - If defined, does definition include the referenced component?", - " - Could this be implicit/assumed? (e.g., standard library type)", - " - Would an implementer be blocked by this omission?", - " -> If referenced entity not defined: SPECIFICATION_GAP (dangling)", - " -> If defined but incomplete: SPECIFICATION_GAP (incomplete)", - "", - "5. DETERMINE VERDICT", - " - TRUE_INCOHERENCE: genuinely conflicting claims (A says X, B says not-X)", - " - SIGNIFICANT_AMBIGUITY: could confuse readers, clarification needed", - " - DOCUMENTATION_GAP: missing info that should exist (code without docs)", - " - SPECIFICATION_GAP: entity referenced but not defined, or defined incomplete", - " * Dangling reference: spec references entity not defined anywhere", - " * Incomplete definition: entity defined but missing expected components", - " - FALSE_POSITIVE: not actually a problem", - ], - "next": "When done exploring, invoke step 11 with findings in --thoughts" - } - - if step_number == 11: - return { - "actions": [ - "FORMAT RESULTS [SUB-AGENT]", - "", - "Structure your findings. This is your FINAL OUTPUT.", - "", - "REQUIRED FORMAT:", - "```", - "VERIFICATION RESULT", - "", - "CANDIDATE: {id}", - "VERDICT: TRUE_INCOHERENCE | SIGNIFICANT_AMBIGUITY | DOCUMENTATION_GAP | SPECIFICATION_GAP | FALSE_POSITIVE", - "", - "SOURCE A:", - " File: [path]", - " Line: [number]", - " Quote: \"[exact quote]\"", - " Claims: [what it asserts]", - "", - "SOURCE B:", - " File: [path]", - " Line: [number]", - " Quote: \"[exact quote]\"", - " Claims: [what it asserts]", - "", - "ANALYSIS: [why they do/don't conflict]", - "", - "SEVERITY: critical|high|medium|low (if not FALSE_POSITIVE)", - "RECOMMENDATION: [fix action]", - "```", - ], - "next": "Output formatted result. Sub-agent task complete." - } - - if step_number == 12: - return { - "actions": [ - "VERDICT ANALYSIS", - "", - "STEP A: TALLY RESULTS", - " - Total verified", - " - TRUE_INCOHERENCE count", - " - SIGNIFICANT_AMBIGUITY count", - " - DOCUMENTATION_GAP count", - " - SPECIFICATION_GAP count", - " - FALSE_POSITIVE count", - " - By severity (critical/high/medium/low)", - "", - "STEP B: QUALITY CHECK", - " Verify each non-FALSE_POSITIVE verdict has exact quotes from sources.", - "", - "STEP C: DEDUPLICATE VERIFIED ISSUES", - "", - " With Sonnet analysis complete, merge issues that:", - " - Reference IDENTICAL source pairs (same file:line for both A and B)", - " - Have semantically equivalent conflict descriptions", - "", - " Sonnet context enables better merge decisions than raw Haiku findings.", - " Keep the version with more detailed analysis.", - "", - "STEP D: IDENTIFY ISSUE GROUPS", - "", - " Analyze confirmed incoherences for relationships. Group by:", - "", - " SHARED ROOT CAUSE:", - " - Same file appears in multiple issues", - " - Same outdated documentation affects multiple claims", - " - Same config/constant is inconsistent across locations", - "", - " SHARED THEME:", - " - Multiple issues in same dimension (e.g., all Category D)", - " - Multiple issues about same concept (e.g., 'timeout')", - " - Multiple issues requiring same type of fix", - "", - " For each group, note:", - " - Group ID (G1, G2, ...)", - " - Member issues", - " - Relationship description", - " - Potential unified resolution approach", - "", - " Issues without clear relationships remain ungrouped.", - ], - "next": "Invoke step 13 with confirmed findings and groups" - } - - if step_number == 13: - return { - "actions": [ - "PREPARE RESOLUTION BATCHES", - "", - "Transform verified incoherences from step 12 into batches for", - "interactive resolution via AskUserQuestion.", - "", - "BATCHING RULES (in priority order):", - "", - "1. GROUP-BASED BATCHING:", - " - Issues sharing a group (G1, G2, ...) go in same batch", - " - Max 4 issues per batch (AskUserQuestion limit)", - " - If group has >4 members, split by file proximity", - "", - "2. FILE-BASED BATCHING:", - " - Ungrouped issues affecting same file go together", - " - Max 4 issues per batch", - "", - "3. SINGLETON BATCHING:", - " - Remaining unrelated issues bundled up to 4 per batch", - "", - "OUTPUT FORMAT (include in --thoughts for step 14):", - "", - "```", - "RESOLUTION BATCHES", - "", - "Batch 1 (Group G1: Timeout inconsistencies):", - " Issues: I2, I5, I7", - " Theme: Timeout values differ between docs and code", - " Files: src/client.py, docs/config.md", - " Group suggestion: Update all to 30s", - "", - "Batch 2 (File: src/uploader.py):", - " Issues: I1, I6", - " No group relationship", - "", - "Batch 3 (Singletons):", - " Issues: I3, I4", - " No relationship", - "", - "Total batches: 3", - "Current batch: 1", - "```", - "", - "ISSUE DATA FORMAT (required for step 14):", - "", - "For EACH issue, output in this structure:", - "", - "```", - "ISSUE {id}: {title}", - " Severity: {critical|high|medium|low}", - " Dimension: {category name}", - " Group: {G1|G2|...|none}", - "", - " Source A:", - " File: {path}", - " Line: {number}", - " Quote: \"\"\"{exact text, max 10 lines}\"\"\"", - " Claims: {what this source asserts}", - "", - " Source B:", - " File: {path}", - " Line: {number}", - " Quote: \"\"\"{exact text, max 10 lines}\"\"\"", - " Claims: {what this source asserts}", - "", - " Analysis: {why these conflict}", - "", - " Suggestions:", - " 1. {concrete action with ACTUAL values from sources}", - " 2. {alternative action with ACTUAL values}", - "```", - "", - "CRITICAL: Suggestions must use ACTUAL values, not generic labels.", - " WRONG: 'Update docs to match code'", - " RIGHT: 'Update docs to say 60s (matching src/config.py:42)'", - ], - "next": "Invoke step 14 with batch definitions and issue data in --thoughts" - } - - # ========================================================================= - # INTERACTIVE RESOLUTION PHASE: Steps 14-15 - # ========================================================================= - - if step_number == 14: - return { - "actions": [ - "PRESENT RESOLUTION BATCH", - "", - "Use AskUserQuestion to collect resolutions for the current batch.", - "Each question MUST be self-contained with full context.", - "", - "STEP A: Identify current batch and mode from --thoughts", - "", - " Check --thoughts for 'MODE: individual' flag.", - " - If present: skip to STEP C (individual questions only)", - " - If absent: this is first pass for this batch", - "", - "EDGE CASE RULES:", - "", - "1. EMPTY BATCH (0 issues after filtering):", - " - Skip this batch entirely", - " - Proceed to next batch or step 15 if none remain", - "", - "2. SINGLE-MEMBER GROUP (group with exactly 1 issue):", - " - Treat as non-group batch (skip group question)", - " - Go directly to individual question", - "", - "3. LONG QUOTES (>10 lines):", - " - Truncate to first 10 lines", - " - Append: '[...truncated, see {file}:{line} for full context]'", - "", - "4. MARKDOWN IN QUOTES (backticks, headers, code blocks):", - " - Escape or use different fence style to prevent rendering issues", - "", - "STEP B: For GROUP BATCHES (2+ members), ask ONLY the group question:", - "", - " IMPORTANT: Do NOT include individual questions in this call.", - " The group question determines whether to ask individuals later.", - "", - "```yaml", - "questions:", - " - question: |", - " ## Group {id}: {relationship}", - "", - " **Member issues**: {I2, I5, I7}", - " **Common thread**: {what connects them}", - "", - " Apply a unified resolution to ALL members?", - " header: 'G{n}'", - " multiSelect: false", - " options:", - " - label: '{unified_suggestion}'", - " description: 'Applies to all {N} issues in this group'", - " - label: 'Resolve individually'", - " description: 'Answer for each issue separately (next prompt)'", - " - label: 'Skip all'", - " description: 'Leave all {N} issues in this group unresolved'", - "```", - "", - " After this call, step 15 will either:", - " - Record unified resolution for all members, OR", - " - Loop back here with 'MODE: individual' to ask per-issue questions", - "", - "STEP C: For NON-GROUP batches OR when MODE=individual:", - "", - " Ask individual questions for each issue:", - "", - "```yaml", - "questions:", - " - question: |", - " ## Issue {id}: {title}", - "", - " **Severity**: {severity} | **Type**: {dimension}", - "", - " ### Source A", - " **File**: `{file_a}`:{line_a}", - " ```", - " {exact_quote_a}", - " ```", - " **Claims**: {what_source_a_asserts}", - "", - " ### Source B", - " **File**: `{file_b}`:{line_b}", - " ```", - " {exact_quote_b}", - " ```", - " **Claims**: {what_source_b_asserts}", - "", - " ### Analysis", - " {why_these_conflict}", - "", - " How should this be resolved?", - " header: 'I{n}'", - " multiSelect: false", - " options:", - " - label: '{suggestion_1}'", - " description: '{what this means concretely}'", - " - label: '{suggestion_2}'", - " description: '{what this means concretely}'", - " - label: 'Skip'", - " description: 'Leave this incoherence unresolved'", - "```", - "", - "FULL CONTEXT REQUIREMENT:", - "", - "Each question MUST include:", - " - Exact file paths and line numbers", - " - Exact quotes from both sources", - " - Clear analysis of the conflict", - " - Concrete suggestion descriptions", - "", - "User should NOT need to recall earlier context or open files.", - "", - "SUGGESTION PATTERNS (use ACTUAL values, not generic labels):", - "", - "| Type | Option 1 | Option 2 |", - "|-------------------|-------------------------------|--------------------------------|", - "| Docs vs Code | Update docs to say {B_value} | Update code to use {A_value} |", - "| Stale comment | Remove the comment | Update comment to say {actual} |", - "| Missing docs | Add docs for {element} | Mark {element} as internal |", - "| Config mismatch | Use {A_value} ({A_source}) | Use {B_value} ({B_source}) |", - "| Cross-ref conflict| Use {A_claim} | Use {B_claim} |", - "", - "CRITICAL: Replace placeholders with ACTUAL values from the issue.", - "", - "EXAMPLE:", - " Issue: docs say 30s timeout, code says 60s", - " WRONG option: 'Update docs to match code'", - " RIGHT option: 'Update docs to say 60s (matching src/config.py:42)'", - "", - "Note: 'Other' option is always available (users can type custom text).", - ], - "next": "After AskUserQuestion returns, invoke step 15 with responses" - } - - if step_number == 15: - return { - "actions": [ - "RESOLUTION LOOP CONTROLLER", - "", - "Process responses from step 14 and determine next action.", - "", - "EARLY EXIT CHECK:", - "", - "If ALL collected resolutions so far are NO_RESOLUTION (user skipped everything):", - " - Skip remaining batches", - " - Output: 'No issues selected for resolution. Workflow complete.'", - " - Do NOT proceed to step 16", - "", - "This is a normal outcome, not an error. User may choose to skip all issues.", - "", - "STEP A: IDENTIFY RESPONSE TYPE", - "", - "Check what type of response was received:", - "", - " 1. GROUP QUESTION RESPONSE (header was 'G{n}'):", - " - User answered unified resolution question for a group batch", - " - Check which option was selected", - "", - " 2. INDIVIDUAL QUESTION RESPONSES (headers were 'I{n}'):", - " - User answered per-issue questions", - " - Record each resolution", - "", - "STEP B: HANDLE GROUP QUESTION RESPONSE", - "", - "If response was to a group question:", - "", - " - If user selected UNIFIED SUGGESTION:", - " -> Record that resolution for ALL member issues", - " -> Mark batch complete, proceed to next batch or step 16", - "", - " - If user selected 'Resolve individually':", - " -> Do NOT record any resolutions yet", - " -> Loop back to step 14 with 'MODE: individual' in --thoughts", - " -> Include same batch definition and issue data", - "", - " - If user selected 'Skip all':", - " -> Mark ALL member issues as NO_RESOLUTION", - " -> Mark batch complete, proceed to next batch or step 16", - "", - " - If user selected 'Other' (custom text):", - " -> Record their custom text for ALL member issues", - " -> Mark batch complete, proceed to next batch or step 16", - "", - "STEP C: HANDLE INDIVIDUAL QUESTION RESPONSES", - "", - "If response was to individual questions:", - "", - "For each issue in the batch:", - " - If user selected a suggestion -> record the resolution text", - " - If user selected 'Skip' -> mark as NO_RESOLUTION", - " - If user selected 'Other' -> record their custom text", - "", - "Mark batch complete, proceed to next batch or step 16.", - "", - "ACCUMULATED STATE FORMAT (add to --thoughts):", - "", - "```", - "COLLECTED RESOLUTIONS", - "", - "Batch 1 complete:", - " I2: 'Update timeout to 30s' [from G1 unified]", - " I5: 'Update timeout to 30s' [from G1 unified]", - " I7: 'Update timeout to 30s' [from G1 unified]", - "", - "Batch 2 complete:", - " I1: 'Use 100MB from spec' [individual]", - " I6: NO_RESOLUTION [skipped]", - "", - "Current batch: 2 of 3", - "```", - "", - "STEP D: LOOP DECISION", - "", - "Priority order:", - "", - "1. If group question answered 'Resolve individually':", - " -> Invoke step 14 with same batch + 'MODE: individual'", - "", - "2. If current_batch < total_batches:", - " -> Invoke step 14 with next batch definition", - "", - "3. If current_batch >= total_batches (all complete):", - " -> All resolutions collected, invoke step 16", - "", - "STEP E: PREPARE NEXT INVOCATION", - "", - "Include in --thoughts:", - " - All collected resolutions so far", - " - Batch definitions for remaining batches (if any)", - " - Full issue data for next batch (if looping to step 14)", - " - 'MODE: individual' flag if looping back for individual questions", - ], - "next": ( - "If 'Resolve individually' selected: invoke step 14 with MODE=individual\n" - "If more batches remain: invoke step 14 with next batch\n" - "If all batches complete: invoke step 16 with all resolutions" - ) - } - - # ========================================================================= - # APPLICATION PHASE: Steps 16-22 - # ========================================================================= - - if step_number == 16: - return { - "actions": [ - "ANALYZE TARGETS AND PLAN DISPATCH", - "", - "Read collected resolutions from --thoughts (from step 15).", - "Skip issues marked NO_RESOLUTION.", - "", - "STEP A: DETERMINE TARGET FILES", - "", - "For each issue WITH a resolution:", - " - Identify which file(s) need modification", - " - Use Source A/B locations as hints", - " - Resolution text may specify which source to change", - "", - "STEP B: SELECT AGENT TYPES BY FILE EXTENSION", - "", - " Documentation -> technical-writer:", - " .md, .rst, .txt, .adoc, .asciidoc", - "", - " Code/Config -> developer:", - " .py, .js, .ts, .go, .rs, .java, .c, .cpp, .h", - " .yaml, .yml, .json, .toml, .ini, .cfg", - "", - "STEP C: GROUP BY TARGET FILE", - "", - "```", - "FILE GROUPS", - "", - "src/uploader.py:", - " - I1: 'Use the spec value (100MB)'", - " - I6: 'Add input validation'", - " Agent: developer", - "", - "docs/config.md:", - " - I3: 'Update to match code'", - " Agent: technical-writer", - "```", - "", - "STEP D: CREATE DISPATCH WAVES", - "", - " BATCH: Multiple issues for same file -> one agent", - " PARALLEL: Different files -> dispatch in parallel", - "", - "```", - "DISPATCH PLAN", - "", - "WAVE 1 (parallel):", - " - Agent 1: developer -> src/uploader.py", - " Issues: I1, I6 (batched)", - " - Agent 2: technical-writer -> docs/config.md", - " Issues: I3", - "", - "WAVE 2 (after Wave 1):", - " [none or additional waves if file conflicts]", - "```", - ], - "next": "Invoke step 17 with dispatch plan in --thoughts" - } - - if step_number == 17: - return { - "actions": [ - "RECONCILE DISPATCH", - "", - "Launch agents for the current wave.", - "", - "WHICH WAVE?", - " - First time here: dispatch Wave 1", - " - Returned from step 20: dispatch the next wave", - "", - f"SCRIPT PATH: {script_path}", - "", - "Use the appropriate subagent_type for each agent:", - " - subagent_type='developer' for code and config files", - " - subagent_type='technical-writer' for documentation (.md, .rst, .txt)", - "", - "AGENT PROMPT TEMPLATE:", - "```", - "RECONCILIATION TASK", - "", - "TARGET FILE: {file_path}", - "", - "RESOLUTIONS TO APPLY:", - "", - "--- Issue {id} ---", - "Type: {type}", - "Severity: {severity}", - "Source A: {file}:{line}", - "Source B: {file}:{line}", - "Analysis: {analysis}", - "User's Resolution: {resolution_text}", - "", - "[Repeat for batched issues]", - "", - "YOUR WORKFLOW:", - f"1. python3 {script_path} --step-number 18 --total-steps 21 \\", - " --thoughts \"FILE: {file_path} | ISSUES: {id_list}\"", - "2. Apply the resolution(s)", - f"3. python3 {script_path} --step-number 19 --total-steps 21 \\", - " --thoughts \"\"", - "4. Output your formatted result", - "```", - "", - "Launch all agents for THIS WAVE in a SINGLE message (parallel).", - ], - "next": "After all wave agents complete, invoke step 20 with results" - } - - # ========================================================================= - # APPLICATION SUB-AGENT STEPS: 18-19 - # ========================================================================= - - if step_number == 18: - return { - "actions": [ - "RECONCILE APPLY [SUB-AGENT]", - "", - "Apply the user's resolution(s) to the target file.", - "", - "PROCESS:", - "", - "For EACH resolution assigned to you:", - "", - "1. UNDERSTAND THE RESOLUTION", - " - What did the user decide?", - " - Which source is authoritative?", - " - What specific changes are needed?", - "", - "2. LOCATE THE TARGET", - " - Find the exact location in the file", - " - Read surrounding context", - "", - "3. APPLY THE CHANGE", - " - Make the edit directly", - " - Be precise: match the user's intent", - " - Preserve surrounding context and formatting", - "", - "4. VERIFY", - " - Does the change address the incoherence?", - " - If batched: any conflicts between changes?", - "", - "BATCHED RESOLUTIONS:", - "", - "If you have multiple resolutions for the same file:", - " - Apply them in logical order", - " - Watch for interactions between changes", - " - If changes conflict, note this in output", - "", - "UNCLEAR RESOLUTIONS:", - "", - "If a resolution is genuinely unclear, do your best to interpret", - "the user's intent. Only skip if truly impossible to apply.", - "", - "BIAS: Apply the resolution. Interpret charitably. Skip rarely.", - ], - "next": "When done, invoke step 19 with results in --thoughts" - } - - if step_number == 19: - return { - "actions": [ - "RECONCILE FORMAT [SUB-AGENT]", - "", - "Format your reconciliation result(s).", - "", - "OUTPUT ONE BLOCK PER ISSUE:", - "", - "IF SUCCESSFULLY APPLIED:", - "```", - "RECONCILIATION RESULT", - "", - "ISSUE: {id}", - "STATUS: RESOLVED", - "FILE: {file_path}", - "CHANGE: {brief one-line description}", - "```", - "", - "IF COULD NOT APPLY:", - "```", - "RECONCILIATION RESULT", - "", - "ISSUE: {id}", - "STATUS: SKIPPED", - "REASON: {why it couldn't be applied}", - "```", - "", - "FOR BATCHED ISSUES: Output one block per issue, separated by ---", - "", - "Keep CHANGE descriptions brief (one line, ~60 chars max).", - ], - "next": "Output formatted result(s). Sub-agent task complete." - } - - if step_number == 20: - return { - "actions": [ - "RECONCILE COLLECT", - "", - "Collect results from the completed wave.", - "", - "STEP A: COLLECT RESULTS", - "", - "For each sub-agent that completed:", - " - Issues handled", - " - Status (RESOLVED or SKIPPED)", - " - File and change (if RESOLVED)", - " - Reason (if SKIPPED)", - "", - "```", - "WAVE N RESULTS", - "", - "Agent 1 (developer -> src/uploader.py):", - " I1: RESOLVED - Changed MAX_FILE_SIZE to 100MB", - " I6: RESOLVED - Added validation", - "", - "Agent 2 (technical-writer -> README.md):", - " I3: RESOLVED - Added file size definition", - "```", - "", - "STEP B: CHECK FOR NEXT WAVE", - "", - "Review your dispatch plan from step 16:", - " - More waves remaining? -> Invoke step 17 for next wave", - " - All waves complete? -> Invoke step 21 to write audit", - "", - "OUTPUT:", - "", - "```", - "COLLECTION SUMMARY", - "", - "Wave N complete:", - " - RESOLVED: I1, I3, I6", - " - SKIPPED: [none]", - "", - "Remaining waves: [list or \"none\"]", - "```", - ], - "next": "If more waves: invoke step 17. Otherwise: invoke step 21." - } - - if step_number >= 21: - return { - "actions": [ - "PRESENT REPORT", - "", - "Output the final report directly to the user.", - "Do NOT write to a file - present inline.", - "", - "FORMAT:", - "", - "```", - "INCOHERENCE RESOLUTION COMPLETE", - "", - "Summary:", - " - Issues detected: {N}", - " - Issues resolved: {M}", - " - Issues skipped: {K}", - "", - "+-----+----------+----------+------------------------------------------+", - "| ID | Severity | Status | Summary |", - "+-----+----------+----------+------------------------------------------+", - "| I1 | high | RESOLVED | src/uploader.py: MAX_FILE_SIZE -> 100MB |", - "| I2 | medium | RESOLVED | src/client.py: timeout -> 30s |", - "| I3 | low | RESOLVED | README.md: Added size definition |", - "| I6 | medium | SKIPPED | (user chose to skip) |", - "| I7 | low | SKIPPED | (could not apply) |", - "+-----+----------+----------+------------------------------------------+", - "```", - "", - "RULES:", - " - List ALL issues (resolved + skipped)", - " - Include severity for context", - " - Use RESOLVED for successfully applied", - " - Use SKIPPED with reason in parentheses", - " - Keep summaries brief (~40 chars)", - ], - "next": "WORKFLOW COMPLETE." - } - - return {"actions": ["Unknown step"], "next": "Check step number"} - - -def main(): - parser = argparse.ArgumentParser(description="Incoherence Detector") - parser.add_argument("--step-number", type=int, required=True) - parser.add_argument("--total-steps", type=int, required=True) - parser.add_argument("--thoughts", type=str, required=True) - args = parser.parse_args() - - script_path = os.path.abspath(__file__) - guidance = get_step_guidance(args.step_number, args.total_steps, script_path) - - # Determine agent type and phase - # Detection sub-agents: 4-7 (exploration), 10-11 (deep-dive) - if args.step_number in [4, 5, 6, 7, 10, 11]: - agent_type = "SUB-AGENT" - phase = "DETECTION" - # Application sub-agents: 18-19 (apply resolution) - elif args.step_number in [18, 19]: - agent_type = "SUB-AGENT" - phase = "APPLICATION" - # Detection parent: 1-12 - elif args.step_number <= 12: - agent_type = "PARENT" - phase = "DETECTION" - # Resolution parent: 13-15 - elif args.step_number <= 15: - agent_type = "PARENT" - phase = "RESOLUTION" - # Application parent: 16-22 - else: - agent_type = "PARENT" - phase = "APPLICATION" - - print("=" * 70) - print(f"INCOHERENCE DETECTOR - Step {args.step_number}/{args.total_steps}") - print(f"[{phase}] [{agent_type}]") - print("=" * 70) - print() - print("THOUGHTS:", args.thoughts[:300] + "..." if len(args.thoughts) > 300 else args.thoughts) - print() - print("REQUIRED ACTIONS:") - for action in guidance["actions"]: - print(f" {action}") - print() - print("NEXT:", guidance["next"]) - print("=" * 70) - - -if __name__ == "__main__": - main() diff --git a/.claude/skills/mvp-architecture-contract/SKILL.md b/.claude/skills/mvp-architecture-contract/SKILL.md new file mode 100644 index 0000000..f4c3ede --- /dev/null +++ b/.claude/skills/mvp-architecture-contract/SKILL.md @@ -0,0 +1,169 @@ +--- +name: mvp-architecture-contract +description: Load this before designing, extending, or reviewing any MotoVaultPro feature or refactor. It states the load-bearing architecture decisions (and WHY), the invariants that must hold, and the known-weak points. Symptoms that mean you need this skill; adding a new backend feature capsule or API route; a new route returns 401 everywhere or, worse, works WITHOUT a token; a migration "ran" but the table does not exist; a route registered fine but is missing the /api prefix; a new page works on desktop but is unreachable on mobile (or vice versa); a tier-gated feature is accessible to free users; wondering why this is Docker Compose and not Kubernetes; wondering whether userContext.userId is the Auth0 sub (it is not). +--- + +# MotoVaultPro Architecture Contract + +Authored 2026-07-07. Every code claim below was verified against the repo on that date. Where a doc contradicts this file, re-verify against code: code wins. + +## When to use / When NOT to use + +Use this skill when you are about to add or modify a backend feature capsule, an API route, a repository, a migration, a frontend page, or tier gating — or when a change "works" but violates one of the failure symptoms in the description above. + +Do NOT use this skill for: +- How to branch/commit/PR or what review rules apply: `mvp-change-control` +- Triage of a live failure: `mvp-debugging-playbook` +- Why a past decision was reverted, incident history: `mvp-failure-archaeology` +- Config/secret axes and how to add one: `mvp-config-and-secrets` +- Deploy, blue-green, rollback, backups: `mvp-run-and-operate` +- VIN/fuel/date domain math: `mvp-vehicle-domain-reference` +- Which docs to trust in full detail: `mvp-docs-and-writing` + +## Doc-trust rule (architecture facts) + +For architecture facts: **code > docs/CICD-DEPLOY.md and docs/LOGGING.md > everything else.** +`docs/ARCHITECTURE-OVERVIEW.md` and `docs/DATABASE-SCHEMA.md` are known-stale (DATABASE-SCHEMA.md still says "15 feature capsules" and a migration order starting with `admin`; reality is 21 capsules and an order starting with `features/vehicles`). Route to `mvp-docs-and-writing` for the full trust map. When this skill and the code disagree, the code changed after 2026-07-07 — update this skill. + +## Part 1: Load-bearing decisions and WHY + +### 1.1 9-container single-tenant Docker Compose (Kubernetes was tried and abandoned) + +9 containers: Traefik, Frontend (React/nginx), Backend (Fastify), OCR (Python), PostgreSQL, Redis + Loki, Alloy, Grafana. Base file `docker-compose.yml`; staging/prod/blue-green are overlay files. + +WHY compose, not k8s: a Kubernetes-style redesign was planned and executed as a "Kubernetes-like Docker Compose" restructure — commit `040da4c` ("k8s redesign complete", 2025-09-18) added `K8S-STATUS.md` (titled "Kubernetes-like Docker Compose Migration Status") plus compose/Makefile changes; no actual k8s manifests were ever committed anywhere in history (only Markdown planning docs). The status doc was demoted out of the repo root into `docs/changes/` four days later (`8fd7973`, 2025-09-22) and deleted entirely on 2025-10-16 (`5638d39`). The product is single-tenant on VPS-class hardware; the k8s direction bought operational complexity with no scaling need. This is a settled battle — do not propose k8s again without new facts (see `mvp-failure-archaeology`). + +Single-tenant means: one deployment per customer base is NOT the model — there is one user population in one database, all rows scoped per user (section 2.5). + +### 1.2 Backend feature capsules + +All application features are self-contained modules under `backend/src/features/` — 21 capsule directories (the 22nd entry is `CLAUDE.md`). Canonical anatomy (fuel-logs is the reference): + +``` +backend/src/features/{name}/ + index.ts # barrel export: routes + anything other capsules may import + api/ # {name}.routes.ts (FastifyPluginAsync, per-route preHandlers), + # {name}.controller.ts, {name}.validators.ts + domain/ # {name}.service.ts, {name}.types.ts (camelCase TS types) + data/ # {name}.repository.ts (class taking pool; private mapRow()) + external/ # cross-feature or third-party clients + migrations/ # NNN_name.sql, feature-owned schema + tests/ # unit/ and integration/ +``` + +Wiring: `backend/src/app.ts` imports each capsule's routes and registers every one with `{ prefix: '/api' }` (lines ~136-159). Plugin registration order before routes: helmet, cors, logging, error, multipart, auth, admin-guard, tier-guard. Shared infra lives in `backend/src/core/` (plugins, config, logging, middleware, scheduler, storage). + +WHY: capsules keep each feature's schema, logic, and HTTP surface in one directory so an AI session can load one capsule instead of the whole backend — this is the project's stated AI-context-efficiency principle. + +### 1.3 YAML config + file secrets, NOT env vars + +`backend/src/core/config/config-loader.ts` loads a zod-validated YAML file (`CONFIG_PATH`, default `/app/config/production.yml`) plus file-based secrets from `SECRETS_DIR` (default `/run/secrets`). DB, Redis, Auth0, and CORS settings live in YAML. Env vars are the exception (Stripe price IDs, `OCR_SERVICE_URL`, `MIGRATIONS_DIR`, `LOG_LEVEL`, `CONFIG_PATH`/`SECRETS_DIR` themselves). Missing required config or secret = startup crash by design. + +WHY: one schema-validated config document beats scattered env vars for auditability, and file secrets match Docker secret mounts. Do not add `process.env.X` for app config — add it to the zod schema and YAML. Full catalog: `mvp-config-and-secrets`. + +### 1.4 Repository mapRow: snake_case to camelCase, MUST coerce numerics + +Every repository exposes data only through a private mapper (`mapRow()`), converting DB snake_case to TS camelCase. Critically: node-postgres returns `NUMERIC`/`DECIMAL` (OID 1700) as **strings** and this project does NOT override that parser globally — every mapper must coerce (`parseFloat`/`Number`) or the API leaks strings where numbers are typed. This caused a multi-incident bug train (issues #239, #241, #244). New repository with decimal columns = coerce in the mapper, no exceptions. + +### 1.5 DATE columns are plain strings end-to-end + +`backend/src/core/config/database.ts` line 12: + +```typescript +types.setTypeParser(1082, (val: string) => val); +``` + +DATE (OID 1082) values stay `"YYYY-MM-DD"` strings through repository, API, and frontend. WHY: pg's default returns a `Date` at local midnight, which shifts a day when serialized to UTC (issue #237). The full DATE-handling rules (dayjs display, lexicographic sort, the three historical traps) are canonical in `mvp-vehicle-domain-reference` section 3. + +### 1.6 Auth: per-route preHandler; userId is the internal UUID, not the Auth0 sub + +`backend/src/core/plugins/auth.plugin.ts` decorates `fastify.authenticate` (line 120): validates the Auth0 JWT (JWKS, issuer, audience), then loads/creates the `user_profiles` row and hydrates `request.userContext`. + +**`userContext.userId` is the internal `user_profiles.id` UUID, NOT the Auth0 `sub`** (`auth.plugin.ts` line ~130 defaults to auth0Sub, overwritten with `profile.id` at line ~165). This is the post-#206 identity migration state (merged in PR #219). All repositories scope queries by this UUID. + +There is NO global auth hook. Every protected route must list `preHandler: [fastify.authenticate]` itself (see section 2.1). + +### 1.7 Migrations: feature-owned, hard-coded order, run at container start, NO rollback + +- SQL files live in each capsule's `migrations/` dir, executed in lexical order within a feature. +- Cross-feature order is the hard-coded `MIGRATION_ORDER` array in `backend/src/_system/migrations/run-all.ts` (17 entries; `features/vehicles` first because it defines `update_updated_at_column()` which later features depend on; `core/identity-migration` last). +- Execution is tracked in the `_migrations` table (`UNIQUE(feature, file)`); already-run files are skipped. +- Migrations run automatically on every backend container start (`backend/Dockerfile` CMD: `node dist/_system/migrations/run-all.js && npm start`), and manually via `make migrate` or `cd backend && npm run migrate`. +- **There is no rollback.** No down migrations. Recovery is restore-from-export (`scripts/import-database.sh`). WHY: single-tenant, small blast radius, and honest acknowledgment that untested down-migrations are worse than a restore path. + +OWNER NON-NEGOTIABLE: no destructive database operation without a fresh backup — that includes schema migrations on staging/prod (`./scripts/export-database.sh --env ` first), `make clean` (destroys volumes), and `import-database.sh --drop-existing`. See `mvp-change-control`. + +### 1.8 Redis: cache-aside with mvp: prefix; cache failure never breaks a request + +`backend/src/core/config/redis.ts`: singleton ioredis client; `CacheService` prefixes every key with `mvp:` and wraps every operation in try/catch that logs and returns null/continues. A dead Redis degrades to cache-miss behavior, never a 500. `DistributedLockService` (prefix `mvp:lock:`, SET NX EX + Lua release) backs scheduled jobs. Follow this pattern: read cache, on miss hit DB and set with TTL, invalidate on write — and never let a cache error propagate. + +### 1.9 Frontend dual navigation — deliberate but costly (the registration-checklist tax) + +`frontend/src/App.tsx` forks the entire app at `window.innerWidth <= 768` plus a UA regex (~lines 360-407): +- **Desktop**: react-router `` under `/garage/*`, lazy-loaded `features/*/pages/*Page`. +- **Mobile**: NO router — a Zustand screen switcher (`useNavigationStore().activeScreen`) renders `features/*/mobile/*MobileScreen` components, with two-way URL sync via the `routeToScreen` / `screenToRoute` maps in `frontend/src/core/store/navigation.ts`. + +WHY: the mobile experience is a purpose-built app shell (bottom nav, gesture transitions, error boundaries) rather than responsive pages. The cost is real and accepted: every new page must run the full registration checklist in invariant 2.7, and mobile+desktop are separate implementations that must BOTH be built and tested (hard project rule). + +All frontend HTTP goes through the shared `apiClient` (`frontend/src/core/api/client.ts`), a queued axios wrapper that holds requests until the Auth0 gate reports ready — bypassing it with raw axios/fetch reintroduces the auth race it exists to solve. + +## Part 2: INVARIANTS — break these and here is what happens + +| # | Invariant | If you break it | Enforcing code path | +|---|-----------|-----------------|---------------------| +| 2.1 | Every protected route lists `preHandler: [fastify.authenticate]` explicitly | **The route is PUBLIC.** No global hook exists; nothing fails loudly. An unlisted route ships as an unauthenticated endpoint on the open internet | `backend/src/core/plugins/auth.plugin.ts:120` (decorator only); routes files per capsule. Intentional public routes: canonical list lives in the header comment of `.claude/skills/mvp-diagnostics-and-logging/scripts/check-route-auth.sh` (webhooks + pre-auth signup routes in its allowlist, plus core-registered routes outside its scan scope: `/health`, `/api/health`, `/auth/verify`, `GET /api/config/feature-tiers`) | +| 2.2 | Every repository method returning rows goes through a mapper that coerces NUMERIC/DECIMAL | API returns `"42.50"` where the type says `number`; frontend `.toFixed()` crashes or renders garbage. This has shipped as a bug at least 3 times (#239, #241, #244) | No global parser for OID 1700 in `backend/src/core/config/database.ts` — coercion is per-mapper, by convention and review only | +| 2.3 | The DATE parser override stays | Dates shift one day for any user west of UTC (issue #237 regression) | `backend/src/core/config/database.ts:12` `types.setTypeParser(1082, ...)` | +| 2.4 | A new capsule with migrations gets appended to `MIGRATION_ORDER` | Its migrations **silently never run** — no error, the runner only walks listed directories. First symptom is "relation does not exist" on staging | `backend/src/_system/migrations/run-all.ts:17-35`; also requires an image rebuild so the SQL is packaged into `/app/migrations` | +| 2.5 | All user data scoped by the `user_profiles` UUID (`userContext.userId`) | Cross-user data leak, or zero rows returned if you scope by the Auth0 `sub`. Post-#206, feature tables' `user_id` column is **UUID type** FK to `user_profiles(id) ON DELETE CASCADE` (the migration added `user_profile_id`, backfilled, dropped old VARCHAR `user_id`, then RENAMED `user_profile_id` back to `user_id`); `admin_users` keeps a separate `user_profile_id` column | `backend/src/core/identity-migration/migrations/001_migrate_user_id_to_uuid.sql`; `auth.plugin.ts` userContext hydration | +| 2.6 | Frontend API calls only via the shared queued `apiClient` | Requests fire before the Auth0 token interceptor is installed: intermittent 401s on first load, spurious error toasts | `frontend/src/core/api/client.ts` (`createQueuedAxios`, `authReady` gate); interceptor installed in `frontend/src/core/auth/Auth0Provider.tsx` | +| 2.7 | New pages registered at EVERY registration point. This is the CANONICAL checklist (other skills point here; do not trust any "4 places" shorthand — the full union is 7 steps): (1) desktop `` in App.tsx; (2) `MobileScreen` union type in navigation.ts; (3) `routeToScreen` map; (4) `screenToRoute` map; (5) lazy import of the `*MobileScreen` component in App.tsx; (6) mobile render block `{activeScreen === "X" && ...}` in App.tsx; (7) a navigation entry point (`BottomNavigation.tsx` or `HamburgerDrawer.tsx` item, or navigation from another screen) | Page unreachable on one platform; registered-but-unreachable on mobile (missing entry point or import); or URL/back-button desync (screen renders but URL lies, deep links break) | `frontend/src/App.tsx` (lazy imports + both render paths), `frontend/src/core/store/navigation.ts` (union line ~5; maps lines ~9, ~29), `frontend/src/shared-minimal/components/mobile/` | +| 2.8 | Tier-gated features registered in `FEATURE_TIERS` — **unknown keys FAIL OPEN** | A typo'd or unregistered `featureKey` on a plugin-guarded route silently grants access to ALL tiers. No error, no log at gate time | `backend/src/core/config/feature-tiers.ts:57-62` (`canAccessFeature` returns `true` for unknown keys, by design). Registered-key and vehicle-limit literals: canonical catalog in `mvp-config-and-secrets` section 3 | +| 2.9 | Capsule routes registered in `app.ts` with `{ prefix: '/api' }` using the plain (non-fp) export | Route mounts at `/thing` instead of `/api/thing`; Traefik and the frontend both expect `/api` — the endpoint 404s in every deployed environment | `backend/src/app.ts:136-159`; see weak point 3.7 for the platform dual-export trap | + +New-capsule checklist (all must hold): barrel `index.ts`; routes registered in `app.ts` with `/api` prefix; every route lists its auth preHandler; repository mapper coerces numerics; migrations dir appended to `MIGRATION_ORDER`; queries scoped by the profile UUID; if tier-gated, key added to `FEATURE_TIERS` AND spelled identically at the gate site. + +## Part 3: Known-weak points (stated plainly — these are debt, not patterns to copy) + +3.1 **Tier gating fails open AND has two parallel mechanisms with different failure behavior** — the middleware 500s on unknown keys, the plugin decorator fails open (full mechanics and the current key catalog are canonical in `mvp-config-and-secrets` section 3). Both are used, even mixed within one file (`backend/src/features/ocr/api/ocr.routes.ts`). The plugin's dependency check on the auth plugin was deliberately removed for testability — registration order is convention-only. When gating something new, prefer the plugin decorator with a key you have verified exists in `FEATURE_TIERS`. + +3.2 **Controllers string-match error messages for status codes.** No typed error classes: catch blocks do `error.message.includes('not found')` -> 404, `includes('Unauthorized')` -> 403, else 500 (e.g. `backend/src/features/fuel-logs/api/fuel-logs.controller.ts:30,36`). Rewording a service error message silently changes an HTTP status. If you touch a service's thrown messages, grep the controller for `includes(` first. + +3.3 **Auth plugin is a per-request hot path to Auth0.** Worst case per authenticated request: profile `getOrCreate` DB roundtrip, plus up to two Auth0 Management API `getUser` calls (email backfill when the JWT lacks email; verification re-check when the profile is unverified) plus the SDK's token grant — up to ~3 Auth0 HTTP calls. Failures fall back silently to JWT claims (`auth.plugin.ts` ~line 201). Latency and rate-limit exposure live here; do not add more per-request external calls to this plugin. + +3.4 **Dual fuel-logs schema, with a documented convention break.** `backend/src/features/fuel-logs/data/fuel-logs.repository.ts` carries a legacy API (`create`/`mapRow`, gallons columns) and an enhanced API (`createEnhanced`/`mapEnhancedRow`, fuel_units/cost_per_unit, dual-writing legacy columns). `mapEnhancedRow` (line ~252) deliberately returns **snake_case** numeric-coerced rows, and the service's `toEnhancedResponse` (`fuel-logs.service.ts:273`) does the camelCase mapping — a known break from the mapRow convention, typed `any` throughout. Do not copy this pattern into new capsules; do not "fix" it casually either (dual-write compatibility is load-bearing). + +3.5 **App.tsx is a 1192-line god component** (`frontend/src/App.tsx`): inline screen components, all mobile switching, auth routing, and global error suppression in one file. It violates the repo's own RULE 2. Changes here have wide blast radius; keep diffs surgical. + +3.6 **DB pool max is hard-coded 10** (`backend/src/core/config/database.ts:16`) while the config schema defines `database.pool_size` with default 20 (`config-loader.ts:28`) — the config value is not consumed by the pool. YAML pool tuning silently does nothing. + +3.7 **platform.routes.ts dual-export trap.** `backend/src/features/platform/api/platform.routes.ts:45-46` exports both a `fastify-plugin`-wrapped default AND a raw named export. `app.ts` imports the NAMED export, so the `/api` prefix applies. Switching the import to the fp-wrapped default would silently drop the prefix (fastify-plugin breaks encapsulation, so `prefix` is ignored) — every platform dropdown endpoint would move off `/api` with no error. + +3.8 **Two shared frontend trees with no boundary rule:** `frontend/src/shared/` and `frontend/src/shared-minimal/` (theme, GlassCard, BottomNavigation live in shared-minimal). Check both before creating a "new" shared component. + +3.9 **`(fastify as any).requireAdmin`** in `backend/src/features/backup/api/backup.routes.ts` (every route) — the admin-guard decorator is used through an `any` cast, so TypeScript would not catch the decorator being renamed or unregistered; the routes would throw at runtime instead. + +Severity calibration: this is a pre-launch product heading toward paying users. 3.1 and invariant 2.1/2.8 are the security-shaped items — treat regressions there as RULE 0. The rest are RULE 1/2 debt: known, tolerated, but never to be silently extended. + +## Provenance and maintenance + +Authored 2026-07-07 from direct repo inspection (file reads, greps, git history). Re-verify volatile facts before relying on them: + +| Fact (as of 2026-07-07) | Re-verify with | +|---|---| +| 21 feature capsules | `ls backend/src/features/ \| grep -v CLAUDE.md \| wc -l` | +| Routes registered with `/api` prefix; platform uses named export | `grep -n "prefix: '/api'" backend/src/app.ts \| wc -l` and `grep -n "platformRoutes" backend/src/app.ts` | +| DATE parser override present | `grep -n "setTypeParser(1082" backend/src/core/config/database.ts` | +| Pool max hard-coded 10; schema pool_size default 20 | `grep -n "max:" backend/src/core/config/database.ts; grep -n "pool_size" backend/src/core/config/config-loader.ts` | +| MIGRATION_ORDER has 17 entries, vehicles first, identity-migration last | `sed -n '17,36p' backend/src/_system/migrations/run-all.ts` | +| Tier fail-open (key/limit catalog: `mvp-config-and-secrets` section 3) | `grep -n "return true" backend/src/core/config/feature-tiers.ts; grep -n "minTier" backend/src/core/config/feature-tiers.ts` | +| userContext.userId = profile UUID, not Auth0 sub | `grep -n "userId = profile.id" backend/src/core/plugins/auth.plugin.ts` | +| Feature tables' user_id is UUID (renamed from user_profile_id) | `grep -n "RENAME COLUMN user_profile_id TO user_id" backend/src/core/identity-migration/migrations/001_migrate_user_id_to_uuid.sql` | +| Redis `mvp:` prefix, errors swallowed | `grep -n "prefix = 'mvp:'" backend/src/core/config/redis.ts` | +| App.tsx 1192 lines; 768px fork | `wc -l frontend/src/App.tsx; grep -n "768" frontend/src/App.tsx` | +| Nav registration checklist (2.7 union) | `grep -n "routeToScreen\|screenToRoute\|MobileScreen" frontend/src/core/store/navigation.ts; grep -n "activeScreen ===" frontend/src/App.tsx \| head -3` | +| Queued apiClient auth gate | `grep -n "createQueuedAxios\|authReady" frontend/src/core/api/client.ts` | +| platform dual export | `grep -n "export" backend/src/features/platform/api/platform.routes.ts \| tail -2` | +| requireAdmin any-cast in backup routes | `grep -c "(fastify as any).requireAdmin" backend/src/features/backup/api/backup.routes.ts` | +| k8s-style redesign tried/abandoned Sept-Oct 2025 | `git log --all --oneline -i --grep=k8s` and `git log --all --oneline --follow -- docs/changes/K8S-STATUS.md` | +| Controller string-matching | `grep -rn "includes('not found')" backend/src/features/*/api/*.controller.ts \| head` | diff --git a/.claude/skills/mvp-build-and-env/SKILL.md b/.claude/skills/mvp-build-and-env/SKILL.md new file mode 100644 index 0000000..d4aa26f --- /dev/null +++ b/.claude/skills/mvp-build-and-env/SKILL.md @@ -0,0 +1,201 @@ +--- +name: mvp-build-and-env +description: Load when starting from a fresh MotoVaultPro checkout or when any build/test/install command fails locally. Triggers - "how do I run this", "npm test fails", "Could not resolve a module for a custom reporter", "tdd-guard-jest", "Jest did not exit one second after the test run", jest hangs, "Configuration file not found at /app/config/production.yml", "make setup fails", secrets are directories, "mobile-setup nothing to be done", pytest/paddleocr install, Dockerfile build questions, "why is my migration not on staging". Covers what works on the dev machine vs container-only, the exact local test loop per workspace, Docker build anatomy, and known environment traps. +--- + +# Build and Environment Reality + +All commands below were verified by running them or reading the exact source on 2026-07-07. Where docs contradict code, code wins; this file states code behavior. + +## When to use / When NOT to use + +Use this skill when you have a fresh checkout and need to build, lint, type-check, or run tests, or when a local command fails in a confusing way. + +Do NOT use this skill for: +- Deploying, rollback, staging/prod operations, backups: `mvp-run-and-operate` +- What counts as test evidence, adding tests, mobile+desktop validation: `mvp-validation-and-qa` +- Issue/branch/PR workflow and review rules: `mvp-change-control` +- Runtime failures of a deployed stack: `mvp-debugging-playbook` +- Config keys, secrets catalog, feature tiers: `mvp-config-and-secrets` + +## 1. The development model (read this first) + +Development on this project is done by AI sessions working in this repo; the human owner reviews PRs. There is NO fully working local dev loop and none is expected. The loop is: + +1. Edit code locally. Run unit tests + lint + type-check + build locally (per workspace, below). +2. Open a PR. CI builds 3 Docker images and deploys them to staging. That is the entire gate — no tests, no lint (canonical statement: `mvp-validation-and-qa` section 1). The only compile check CI performs is `tsc` inside the Dockerfiles. +3. End-to-end verification happens ON STAGING (https://staging.motovaultpro.com) after the PR pipeline deploys. See `mvp-validation-and-qa` for the evidence bar. + +Consequence: anything your local commands do not catch, nothing catches before staging. Run the local gates every time; they are the only gates. + +## 2. What works on the dev machine (verified 2026-07-07) + +Root `package.json` has NO scripts and no workspaces. `npm test` / `npm run lint` at repo root fail with "Missing script". Always cd into `backend/`, `frontend/`, or `ocr/`. + +### Bootstrap + +```bash +make install # runs npm install in frontend/ AND backend/ (Makefile:220) +# or individually: +cd backend && npm install +cd frontend && npm install +``` + +### Dev-safe make targets (Makefile:218-254; these never touch Docker) + +| Target | Does | +|---|---| +| `make install` | `npm install` in frontend + backend | +| `make type-check` | `npm run type-check` in frontend + backend | +| `make lint` | `npm run lint` in frontend + backend | +| `make build-local` | `npm run build` in frontend + backend (outputs `frontend/dist`, `backend/dist`) | + +Every other make target (`setup`, `start`, `rebuild`, `migrate`, `clean`, ...) drives Docker. Per root CLAUDE.md, `make setup`/`make rebuild` are for staging/prod-style builds, NOT development — and on this machine `make setup` does not produce a working stack anyway (section 3). + +### backend/ — everything works + +```bash +cd backend +npm run lint # eslint src (flat config eslint.config.js) +npm run type-check # tsc --noEmit +npm run build # tsc --project tsconfig.build.json -> dist/ +npm test -- --forceExit # all unit tests +npm test -- --forceExit --testPathPattern=src/features/fuel-logs # one feature +npm run test:feature --feature=fuel-logs # same, needs --forceExit caveat too +``` + +`--forceExit` is MANDATORY locally. Without it jest prints "Jest did not exit one second after the test run" and hangs indefinitely (verified: killed after 8+ min; with `--forceExit` the same suite finishes in ~2 s). Cause: `backend/src/core/config/database.ts` creates a pg Pool eagerly and unit tests leave open handles. Do not "fix" a hang by waiting. + +Expect a RED baseline on main: 15 of 25 unit suites fail pre-existing (6 die loading real config, 7 fail ts-jest compilation, 2 contain the 2 genuinely failing tests; `Tests: 2 failed, 147 passed, 149 total`, re-verified 2026-07-09 — baseline detail homed in `mvp-deploy-safety-campaign` Phase 0.3). Judge your change against that baseline, not against zero. + +Why unit tests work at all locally: they mock `core/config/config-loader` and `core/config/redis` at the top of the test file (see `src/features/stations/tests/unit/station-matching.test.ts:5-15`). The real config-loader is an eager singleton (`config-loader.ts:281`: `export const appConfig = configLoader.load()`) that throws `Configuration file not found at /app/config/production.yml` if imported unmocked outside a container. If you see that error in a test, the test (or something it imports) is loading real config — mock it like the existing unit tests do. + +Integration tests (`src/features/*/tests/integration/`) are container-only AND destructive — see section 3. + +Broken script: `npm run migrate:feature` references `src/_system/migrations/run-feature.ts`, which does not exist (only `run-all.ts` does). It fails on invocation. + +### frontend/ — build tools work, `npm test` is broken outside the container + +```bash +cd frontend +npm run lint # works +npm run type-check # works +npm run build # works: tsc --project tsconfig.build.json && vite build +npm run dev # vite dev server on :3000 (UI only; API calls need a backend) +``` + +`npm test` fails outside the container with: + +``` +Error: Could not resolve a module for a custom reporter. Module name: tdd-guard-jest +``` + +Two independent causes in `frontend/jest.config.ts:28-36`: +1. The `tdd-guard-jest` reporter is declared only in the ROOT `package.json` devDependencies, and no root `node_modules/` exists on a fresh checkout (root has no scripts, nobody runs `npm install` there), so the module cannot resolve from `frontend/`. +2. The reporter config hardcodes `projectRoot: '/home/egullickson/motovaultpro'` — a Linux path from the container/CI host, wrong on this macOS checkout even if the module resolved. + +WORKING FALLBACK (verified 2026-07-07, executes and reports results): + +```bash +cd frontend +npx jest src/path/to/File.test.tsx --reporters=default # single file +npx jest --reporters=default --testPathPattern=fuel-logs # by pattern +npx jest --reporters=default # full suite +``` + +Trap: `--reporters` is greedy. `npx jest --reporters=default src/Foo.test.tsx` (positional AFTER the flag) swallows the path as a second reporter name and fails with "Could not resolve a module for a custom reporter. Module name: src/Foo...". Put the positional path FIRST, or use `--testPathPattern`. + +Expect the fallback run to be RED at baseline on main: `Tests: 17 failed, 196 passed, 213 total` across 14 failing suites (re-verified 2026-07-09; the baseline numbers are homed in `mvp-deploy-safety-campaign` Phase 0.3). Diff your run against that baseline — do not attribute the pre-existing failures to your change, and do not report "only 1 known failure". There is NO in-container alternative: the shipped frontend image is the nginx production stage (no node/npm) and `frontend/.dockerignore` excludes `*.test.*` from every stage, so `docker compose exec mvp-frontend npm test` (still documented in docs/TESTING.md) has never been able to work. This fallback plus staging verification is the whole frontend test story today. + +Also note: `frontend/test/` contains test files outside jest `roots: ['/src']` — they never run anywhere. See `mvp-validation-and-qa`. + +### ocr/ — tests exist; running them locally is heavyweight and UNVERIFIED + +```bash +cd ocr +python3 -m venv .venv && source .venv/bin/activate +pip install -r requirements.txt # includes paddleocr, google-cloud-vision, google-genai — multi-GB, slow +python -m pytest tests/ +``` + +Verified facts only: `ocr/requirements.txt` lists `pytest>=7.4.0` and `pytest-asyncio>=0.21.0`; 16 test files exist under `ocr/tests/`; tests import `from app.main import app`; there is no venv, no `pytest.ini`, and no `conftest.py` in the repo. The pip install and pytest run above were NOT executed during authoring (paddleocr download is too heavy) — UNVERIFIED as a complete recipe. CI never runs OCR tests either (`ocr/Dockerfile` has no test step), so OCR test results always come from a manual run. + +## 3. Container-only, and why + +### Backend integration tests — DESTRUCTIVE, backup first (non-negotiable) + +`backend/src/features/*/tests/integration/*.test.ts` need a real Postgres + Redis and real config (`CONFIG_PATH`, `SECRETS_DIR` — container paths). docs/TESTING.md:24-32 documents `make shell-backend` then `npm test`, but that recipe is STALE-DANGEROUS: the shipped backend image is the production stage (`npm ci --omit=dev` — no jest; only `dist/` plus migration SQL copied), so `npm test` inside it fails with "jest: not found". As-shipped, the integration tests are runnable nowhere; the viable paths are a builder-stage image (`docker build --target builder ...`) or a host run against an ephemeral Postgres/Redis — see `mvp-deploy-safety-campaign` 1B. + +They are DESTRUCTIVE by design: `beforeAll` executes the feature's real migration SQL and `afterAll` drops the tables, e.g. `vehicles.integration.test.ts:38` runs `DROP TABLE IF EXISTS vehicles CASCADE` and drops `update_updated_at_column()` CASCADE (which other tables' triggers depend on); `admin.integration.test.ts:72-73` drops `admin_audit_logs` and `admin_users`. They run against whatever database the container config points at — the shared dev DB `motovaultpro`. + +Owner non-negotiable: never run integration tests against a database you care about without a fresh backup first (`make db-backup` or `./scripts/export-database.sh`). Restore path: `./scripts/import-database.sh`. See `mvp-run-and-operate`. + +Note: `config/app/ci.yml` ("CI-specific configuration for backend tests") exists but nothing in the repo references it — it also points at `mvp-postgres`/db `motovaultpro`, so it does not make the tests safe. + +### Full local stack — currently cannot work on this machine + +`make setup` (compose up --build + migrations) is labeled staging/prod-build only in root CLAUDE.md, and on this checkout it will not produce a healthy stack because four "secret files" exist only as Docker-created EMPTY DIRECTORIES (Docker auto-creates a directory when a bind-mount source file is missing; canonical description of this trap: `mvp-config-and-secrets` section 2): + +``` +secrets/app/stripe-secret-key.txt <- directory +secrets/app/stripe-webhook-secret.txt <- directory +secrets/app/auth0-ocr-client-id.txt <- directory +secrets/app/auth0-ocr-client-secret.txt <- directory +``` + +The backend's Zod secrets schema (`config-loader.ts:114-125`) requires `stripe_secret_key` and `stripe_webhook_secret` as non-optional strings; reading a directory fails, validation throws, and the backend crash-loops at startup. The OCR container bind-mounts the two auth0-ocr paths (`docker-compose.yml:213-214`). To ever make the local stack work: `rm -rf` the four directories, create real `.txt` files (see the `.txt.example` siblings), then `make setup`. Until then, local full-stack is off the table — which is consistent with the dev model in section 1. + +Verify current state: `ls -la secrets/app/ | grep '^d'` (any `.txt` entry that is a directory is a broken mount point). + +## 4. Docker build anatomy + +Three Dockerfiles; CI (`.gitea/workflows/staging.yaml`) builds all three on every push to main and every PR sync, tags `:<7-char-sha>` AND `:latest`, and pushes to the registry. + +| Image | Dockerfile | Compile gate | Registry | +|---|---|---|---| +| backend | `backend/Dockerfile` (2-stage) | `RUN npm run build` (tsc) at line 31 | `git.motovaultpro.com/egullickson/backend` | +| frontend | `frontend/Dockerfile` (4-stage) | `RUN npm run build` (tsc + vite) at line 35 | `git.motovaultpro.com/egullickson/frontend` | +| ocr | `ocr/Dockerfile` (1-stage) | none (pip install only, no tests) | `git.motovaultpro.com/egullickson/ocr` | + +These tsc runs are the ONLY compile gate CI has. A PR is "green" when the 3 images build and staging boots healthy — nothing else. + +Migrations ship inside the backend image: `backend/Dockerfile:58-61` sets `ENV MIGRATIONS_DIR=/app/migrations` and copies `src/features` and `src/core` there; the container CMD (`Dockerfile:84`) runs `node dist/_system/migrations/run-all.js && npm start` on every start. Therefore a NEW MIGRATION REACHES STAGING/PROD ONLY VIA AN IMAGE REBUILD — merging SQL into the repo does nothing until CI builds and deploys a new backend image. A new feature's migrations also require an entry in `MIGRATION_ORDER` in `backend/src/_system/migrations/run-all.ts` or they silently never run. + +Frontend build-time vs runtime config: all `VITE_*` values are baked at image build via compose/CI build args (`docker-compose.yml:60-64`, ARGs at `frontend/Dockerfile:21-25`). Exception: the Google Maps key/map-id are injected at container RUNTIME by `frontend/scripts/load-config.sh` from `/run/secrets`, not at build. + +PR builds clobber `:latest` — deploy implications and blue-green mechanics are in `mvp-run-and-operate`. + +## 5. Environment traps table + +| Trap | Reality | Do instead | +|---|---|---| +| Root `npm test`/`npm run lint` | Root `package.json` has NO scripts (root CLAUDE.md is wrong here; code wins) | cd into `backend/` or `frontend/` | +| Root dependencies | `test@^3.3.0` (stray/accidental package, supply-chain smell), plus `jest`, `@playwright/test`, `tdd-guard-jest` with no root node_modules — all dead weight | Do not `npm install` at root expecting anything; do not add root deps | +| `make mobile-setup` | Advertised in help and `.PHONY` but HAS NO RULE — "Nothing to be done" | Mobile testing procedure: `mvp-validation-and-qa` | +| `make clean` | `docker compose down -v --rmi all` — DESTROYS DB VOLUMES wherever run | Non-negotiable: fresh backup first (`make db-backup`) | +| Backend jest hang | pg pool open handles; jest never exits | Always `npm test -- --forceExit` locally | +| Frontend `npm test` | Broken outside container (tdd-guard-jest reporter + hardcoded Linux projectRoot) | `npx jest --reporters=default`; positional path BEFORE the flag | +| Integration tests | `DROP TABLE ... CASCADE` on the shared dev DB | Container-only, backup first, never against a DB you care about | +| `make setup` locally | 4 secrets are empty directories; backend Zod validation crash-loops | Treat local full-stack as unavailable; verify on staging | +| `frontend/.env.local` | Affects `npm run dev` only; deployed images use build args baked by CI | Change Gitea CI variables / compose build args for deployed values | +| New migration "not on staging" | Migrations live inside the backend image at `/app/migrations` | Merge -> CI image build -> deploy; check `_migrations` table | +| `npm run migrate:feature` (backend) | Points at nonexistent `run-feature.ts` | `npm run migrate` (runs all, idempotent per file) | + +## Provenance and maintenance + +Authored 2026-07-07 against commit e729d42 (main). All commands in sections 2 and the frontend fallback were executed on the dev machine that day, except the OCR venv/pytest recipe (labeled UNVERIFIED). Volatile facts and how to re-check them: + +| Fact | Re-verify with | +|---|---| +| Root package.json still has no scripts + stray `test` dep | `cat package.json` | +| Backend scripts unchanged (test/lint/type-check/build, broken migrate:feature) | `cat backend/package.json` and `ls backend/src/_system/migrations/` | +| Frontend jest still broken (reporter + projectRoot) | `grep -n -A6 reporters frontend/jest.config.ts` | +| Frontend fallback still works | `cd frontend && npx jest --reporters=default --listTests` | +| Backend jest still hangs without forceExit | `cd backend && npx jest src/features/stations/tests/unit/station-matching.test.ts` (should finish; if it hangs, trap still live) | +| 4 secrets still empty directories | `ls -la secrets/app/ \| grep '^d'` | +| `make mobile-setup` still ruleless | `grep -n "mobile-setup:" Makefile \|\| echo missing` | +| Integration tests still DROP TABLE | `grep -rn "DROP TABLE" backend/src --include="*.test.ts"` | +| Migrations still image-packaged | `grep -n "MIGRATIONS_DIR\|migrations" backend/Dockerfile` | +| CI still runs zero tests/lint | `grep -rn "npm test\|npm run lint" .gitea/workflows/ \|\| echo none` | +| Registry image names | `grep -n "egullickson/" .gitea/workflows/staging.yaml \| head` | +| OCR pytest deps present | `grep -n "pytest" ocr/requirements.txt` | diff --git a/.claude/skills/mvp-change-control/SKILL.md b/.claude/skills/mvp-change-control/SKILL.md new file mode 100644 index 0000000..c500978 --- /dev/null +++ b/.claude/skills/mvp-change-control/SKILL.md @@ -0,0 +1,218 @@ +--- +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_ (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` diff --git a/.claude/skills/mvp-config-and-secrets/SKILL.md b/.claude/skills/mvp-config-and-secrets/SKILL.md new file mode 100644 index 0000000..5fcbe85 --- /dev/null +++ b/.claude/skills/mvp-config-and-secrets/SKILL.md @@ -0,0 +1,200 @@ +--- +name: mvp-config-and-secrets +description: Load when working with MotoVaultPro configuration or secrets in any form - adding/changing an env var, YAML config field, Docker secret, feature flag, tier gate, vehicle limit, or Stripe price ID. Also load on these symptoms - backend crashes at startup with "Configuration file not found" or "Secrets loading failed", "Secret file not found" errors in logs, a secret file on disk is unexpectedly a DIRECTORY, RESEND_WEBHOOK_SECRET is not configured, Stripe checkout returns wrong tier, VITE_ env var change has no effect until rebuild, Google Maps key missing in frontend, 403 TIER_REQUIRED, or confusion about which env vars a container actually reads. +--- + +# MotoVaultPro Configuration and Secrets Catalog + +Authored 2026-07-07. All facts verified against repo code on that date. Where any doc contradicts this file or the code, code wins. + +## When to use / When NOT to use + +Use this skill when you need to know where a configuration value lives, which env vars a service actually reads, what secrets exist and how they get onto servers, how feature tiers gate behavior, or how to add any of these. + +Do NOT use this skill for: +- Deploy mechanics, blue-green, rollback, .env generation during deploys -> `mvp-run-and-operate` +- Getting a local environment running, missing-secret workarounds for local dev -> `mvp-build-and-env` +- OCR/Gemini engine knobs in depth (WIF auth chain, engine selection semantics) -> `mvp-ocr-gemini-pipeline` +- Why the config architecture is YAML-file based (design rationale) -> `mvp-architecture-contract` +- Triage of a live failure whose cause is unknown -> `mvp-debugging-playbook` +- Tier/subscription domain semantics (what tiers mean to the business) -> `mvp-vehicle-domain-reference` + +## 1. Config architecture: YAML + file secrets; env vars are the EXCEPTION + +The backend deliberately does NOT use env vars for most configuration. `backend/src/core/config/config-loader.ts` loads: + +1. A YAML config file from `CONFIG_PATH` (default `/app/config/production.yml`), validated by a Zod schema (`configSchema`). The file is the committed, non-sensitive `config/app/production.yml`, bind-mounted read-only by compose. Database host/port/name/user, Redis, Auth0 domain/audience, CORS, health probe timings, and performance settings all live HERE, not in env. +2. File-based secrets from `SECRETS_DIR` (default `/run/secrets`), validated by `secretsSchema`. Each secret is one file, name matching the compose mount (no `.txt` inside the container). + +The loader is a module-level singleton: `export const appConfig = configLoader.load()` runs at import time. Missing config file or a missing REQUIRED secret means the backend process throws before the server starts. It also self-injects two values into `process.env` after loading: `RESEND_API_KEY` always, `RESEND_WEBHOOK_SECRET` only if present (config-loader.ts lines 266-270). + +Files (all committed): +- `config/app/production.yml` - the live backend config (mounted to `/app/config/production.yml`) +- `config/app/production.yml.example` - STALE: mentions minio and admin-postgres hosts and lacks most sections the Zod schema requires. Do not copy it as a template; copy `production.yml` itself. +- `config/app/ci.yml` - no references found anywhere in the repo as of 2026-07-07 (candidate dead config) +- `config/shared/production.yml` - mounted to `/app/config/shared.yml` by compose, but NO backend code reads it (dead mount as of 2026-07-07) + +### Backend env vars that actually exist (verified against all `process.env` reads in `backend/src/`, non-test) + +| Var | Read at | Default | Notes | +|---|---|---|---| +| `CONFIG_PATH` | `core/config/config-loader.ts:155` | `/app/config/production.yml` | File must exist or startup throws | +| `SECRETS_DIR` | `core/config/config-loader.ts:156` | `/run/secrets` | | +| `LOG_LEVEL` | `core/logging/logger.ts:10` | `info` (invalid values fall back with a console warning) | Compose sets `${BACKEND_LOG_LEVEL:-debug}` | +| `NODE_ENV` | `app.ts` (single read, health payload) | none | Compose sets `production` | +| `STRIPE_PRO_MONTHLY_PRICE_ID`, `STRIPE_PRO_YEARLY_PRICE_ID`, `STRIPE_ENTERPRISE_MONTHLY_PRICE_ID`, `STRIPE_ENTERPRISE_YEARLY_PRICE_ID` | `features/subscriptions/domain/subscriptions.service.ts` (plan-to-priceId map ~line 833 and reverse tier inference ~line 885) | Sandbox price IDs baked into `docker-compose.yml` backend env | Staging/prod override via `.env` generated from Gitea Actions variables. Wrong values = checkout works but tier inference breaks | +| `OCR_SERVICE_URL` | `features/ocr/external/ocr-client.ts:8` | `http://mvp-ocr:8000` | | +| `MIGRATIONS_DIR` | `_system/migrations/run-all.ts:38` | `/app/migrations` set in `backend/Dockerfile:58`; source fallback `../../../migrations` | | +| `TERMS_CONTENT_HASH` | `features/terms-agreement/domain/terms-config.ts` | computed fallback | Optional override | +| `FROM_EMAIL` | `features/notifications/domain/email.service.ts` | `hello@notify.motovaultpro.com` | | +| `BACKUP_STORAGE_PATH` | `features/backup/domain/backup.types.ts` | `/app/data/backups` | | +| `HOSTNAME` | `features/backup/domain/backup-archive.service.ts` | `mvp-backend` | Docker-provided | +| `RESEND_API_KEY`, `RESEND_WEBHOOK_SECRET` | email service / `features/email-ingestion/external/resend-inbound.client.ts:30` | none | NOT set from outside - config-loader injects them from secret files. See the webhook gap in section 2 | + +DEAD env vars: `DATABASE_HOST` and `REDIS_HOST` are set on the backend service in `docker-compose.yml` but NO backend code reads them. Actual DB/Redis hosts come from the YAML config. Do not "fix" a connection problem by changing these; they do nothing. (Cleanup candidate.) + +### Frontend env vars (Vite - baked at BUILD time) + +Vite inlines `import.meta.env.VITE_*` into the JS bundle during `vite build`. In this project the build happens inside the Docker image build, fed by compose `build.args` (`docker-compose.yml` frontend service) and, in CI, by `--build-arg` values from Gitea Actions variables (`.gitea/workflows/staging.yaml` lines 69-73). Consequence: changing a `VITE_*` value requires REBUILDING the frontend image. Setting it in `.env` or container environment after build does nothing. + +| Var | Consumed at | Compose build-arg default | +|---|---|---| +| `VITE_AUTH0_DOMAIN` / `VITE_AUTH0_CLIENT_ID` / `VITE_AUTH0_AUDIENCE` | `src/core/auth/Auth0Provider.tsx` | Real Auth0 tenant values | +| `VITE_API_BASE_URL` | `src/core/api/client.ts`, `src/features/auth/api/auth.api.ts` | `/api` | +| `VITE_STRIPE_PUBLISHABLE_KEY` | `src/features/subscription/pages/SubscriptionPage.tsx`, `mobile/SubscriptionMobileScreen.tsx` | none (empty = Stripe silently broken) | +| `VITE_LOG_LEVEL` | `src/utils/logger.ts` (default `info`) | not passed as a build arg - container builds always get `info` | + +`import.meta.env.MODE === 'development'` gates debug panels; always `production` in container builds. + +EXCEPTION - runtime injection: the Google Maps API key and Map ID are NOT build-time. `frontend/scripts/load-config.sh` runs at container start (`frontend/Dockerfile:81` CMD) and reads `/run/secrets/google-maps-api-key` and `/run/secrets/google-maps-map-id` (mounted from `secrets/app/*.txt`), writing them into `/usr/share/nginx/html/config.js` as `window.CONFIG`. So Maps keys rotate with a container restart, no rebuild. `VITE_GOOGLE_MAPS_API_KEY` in `frontend/.env.example` is for local `npm run dev` only. + +### OCR service env vars (`ocr/app/config.py`, all `os.getenv`) + +| Var | Code default | Compose override (`docker-compose.yml` mvp-ocr) | +|---|---|---| +| `LOG_LEVEL` | `info` | `${BACKEND_LOG_LEVEL:-debug}` | +| `HOST` / `PORT` | `0.0.0.0` / `8000` | not set | +| `OCR_PRIMARY_ENGINE` | `paddleocr` | `google_vision` | +| `OCR_FALLBACK_ENGINE` | `none` | `paddleocr` | +| `OCR_CONFIDENCE_THRESHOLD` / `OCR_FALLBACK_THRESHOLD` | `0.6` / `0.6` | `0.6` / `0.6` | +| `GOOGLE_VISION_KEY_PATH` | `/run/secrets/google-wif-config.json` | same | +| `VISION_MONTHLY_LIMIT` | `1000` | `1000` | +| `VERTEX_AI_PROJECT` | `""` | `motovaultpro` | +| `VERTEX_AI_LOCATION` | `global` | `global` | +| `GEMINI_MODEL` | `gemini-2.5-flash` | `gemini-3-flash-preview` (compose wins in containers) | +| `REDIS_HOST` / `REDIS_PORT` / `REDIS_DB` | `mvp-redis` / `6379` / `1` | same (OCR uses Redis DB 1; backend uses DB 0 via YAML) | + +The compose defaults differ from code defaults - when reasoning about container behavior, read compose, not `config.py`. Engine semantics: see `mvp-ocr-gemini-pipeline`. + +## 2. Secrets inventory (names and paths ONLY - never print values) + +All app secrets live in `secrets/app/` on the host and are bind-mounted read-only to `/run/secrets/` in containers. Real `.txt` files are gitignored; only `.example` siblings and `google-wif-config.json` are committed (the WIF config is checked in by design - it contains no key material, it is a Workload Identity Federation descriptor). + +Full set (matches `scripts/inject-secrets.sh` SECRET_FILES, 12 files): +`postgres-password.txt`, `auth0-client-secret.txt`, `auth0-management-client-id.txt`, `auth0-management-client-secret.txt`, `auth0-ocr-client-id.txt`, `auth0-ocr-client-secret.txt`, `google-maps-api-key.txt`, `google-maps-map-id.txt`, `cloudflare-dns-token.txt`, `resend-api-key.txt`, `stripe-secret-key.txt`, `stripe-webhook-secret.txt` - plus committed `google-wif-config.json`. + +Who mounts what (base `docker-compose.yml`; `docker-compose.blue-green.yml` mirrors via anchors): +- backend: postgres-password, auth0-client-secret, auth0-management-client-id/secret, google-maps-api-key, google-maps-map-id, resend-api-key, stripe-secret-key, stripe-webhook-secret +- frontend: google-maps-api-key, google-maps-map-id (runtime injection, section 1) +- ocr: auth0-ocr-client-id, auth0-ocr-client-secret, google-wif-config.json +- traefik: cloudflare-dns-token (DNS-01 cert challenge) +- postgres: postgres-password (`POSTGRES_PASSWORD_FILE`) + +The backend config-loader `loadSecrets()` reads exactly these 9 names: `postgres-password`, `auth0-client-secret`, `auth0-management-client-id`, `auth0-management-client-secret`, `google-maps-api-key`, `resend-api-key`, `resend-webhook-secret`, `stripe-secret-key`, `stripe-webhook-secret`. All required by the Zod schema except `resend-webhook-secret` (optional). + +Anomalies to know (verified 2026-07-07): +- `auth0-ocr-client-id.txt` / `auth0-ocr-client-secret.txt` are mounted into mvp-ocr and required by inject-secrets, but NO Python code reads them (only a docstring mentions Auth0 M2M). Mounted-but-unread; do not assume OCR auth is enforced by them. +- `cloudflare-dns-token.txt` and `google-maps-map-id.txt` have no `.example` sibling. + +### Injection flow - editing secrets on servers is futile + +On EVERY staging and production deploy, `scripts/inject-secrets.sh` regenerates ALL 12 secret files under `$DEPLOY_PATH/secrets/app` from Gitea Actions secrets (env vars `POSTGRES_PASSWORD`, `AUTH0_CLIENT_SECRET`, `AUTH0_MANAGEMENT_CLIENT_ID`, `AUTH0_MANAGEMENT_CLIENT_SECRET`, `AUTH0_OCR_CLIENT_ID`, `AUTH0_OCR_CLIENT_SECRET`, `GOOGLE_MAPS_API_KEY`, `GOOGLE_MAPS_MAP_ID`, `CF_DNS_API_TOKEN`, `RESEND_API_KEY`, `STRIPE_SECRET_KEY`, `STRIPE_WEBHOOK_SECRET`). Any missing variable fails the deploy. Files are written chmod 644 inside a chmod 700 dir. `google-wif-config.json` is copied from the checkout by the workflows themselves (staging.yaml ~lines 123-125, production.yaml ~lines 115-117). + +Therefore: to rotate or fix a secret, update the Gitea Actions secret (repo settings or `mcp__gitea-mcp__upsert_repo_action_secret`) and redeploy. Hand-editing `/opt/motovaultpro/secrets/app/*` survives only until the next deploy - and hand-editing server files is an owner non-negotiable violation anyway. + +### The missing-secret DIRECTORY trap + +Docker bind-mounting a host file that does not exist silently creates an empty DIRECTORY at that path. Symptoms: backend logs `Secret file not found` or `Failed to read secret file` (EISDIR), then `Secrets loading failed`; `ls -la secrets/app/` shows `drwxr-xr-x` entries ending in `.txt`. This is the current state of several secrets on the dev machine (as of 2026-07-07: `auth0-ocr-client-id.txt`, `auth0-ocr-client-secret.txt`, `stripe-secret-key.txt`, `stripe-webhook-secret.txt` are directories locally). `inject-secrets.sh` and both deploy workflows carry `rm -rf` cleanup for exactly this failure mode. Local fix: `rm -rf secrets/app/.txt` then create the file with a real value (or, if you only need the stack to build, see `mvp-build-and-env` - do not invent fake Stripe keys and call billing "tested"). + +### Known gap: resend-webhook-secret + +`resend-webhook-secret` is in the config-loader's load list (optional in the schema) but: no `secrets/app/resend-webhook-secret.txt` exists, no compose file mounts it, inject-secrets.sh has no entry for it, and no Gitea secret feeds it. So `RESEND_WEBHOOK_SECRET` is never set anywhere, the backend logs `Secret file not found: /run/secrets/resend-webhook-secret` on every boot (harmless noise), and `features/email-ingestion/external/resend-inbound.client.ts:39` throws `RESEND_WEBHOOK_SECRET is not configured` if webhook signature verification is exercised. Resend inbound webhook verification is presently UNCONFIGURED (known gap, 2026-07-07). Closing it = follow the "new secret" checklist in section 4 for `resend-webhook-secret.txt` plus a backend compose mount. + +## 3. Feature flags / tiers as config + +There is no external flag service. Tier gating is code-as-config in `backend/src/core/config/feature-tiers.ts`: + +- `TIER_LEVELS`: `free(0) < pro(1) < enterprise(2)`, hierarchical (higher tier inherits lower). +- `FEATURE_TIERS` registry - 4 entries as of 2026-07-07, all `minTier: 'pro'`: `document.scanMaintenanceSchedule`, `vehicle.vinDecode`, `fuelLog.receiptScan`, `maintenance.receiptScan`. Each has `name` and `upgradePrompt`. +- `VEHICLE_LIMITS`: free=2, pro=5, enterprise=null (unlimited), with `canAddVehicle()` / `getVehicleLimitConfig()`. +- FAIL-OPEN: `canAccessFeature()` returns TRUE for unknown feature keys (deliberate). A typo in a feature key silently un-gates the feature wherever `canAccessFeature` is called directly (e.g. `features/documents/api/documents.controller.ts`). + +Two enforcement mechanisms, both returning 403 `{error: 'TIER_REQUIRED', requiredTier, currentTier, featureName, upgradePrompt, ...}`: +1. Standalone middleware `requireTier('')` from `core/middleware/require-tier.ts` - must come AFTER `requireAuth` in the preHandler array; fails CLOSED (500) on unknown keys. +2. Fastify decorator `fastify.requireTier({minTier}|{featureKey})` from `core/plugins/tier-guard.plugin.ts` - calls `authenticate` itself; the featureKey path FAILS OPEN on unknown keys, exactly like direct `canAccessFeature` calls: `canAccessFeature` returns true for unregistered keys (tier-guard.plugin.ts:70), so a typo'd or unregistered featureKey silently un-gates the route for ALL tiers. The `config?.minTier || 'pro'` fallback (line 73) only shapes the 403 denial payload and is never reached for unknown keys. Only the standalone middleware (mechanism 1) fails closed on unknown keys. + +Live examples: `features/ocr/api/ocr.routes.ts` lines 29/35/41 and `features/vehicles/api/vehicles.routes.ts` line 80. + +Frontend contract: `GET /api/config/feature-tiers` (public, no auth - `core/config/config.routes.ts`) returns `{tiers: TIER_LEVELS, features: FEATURE_TIERS}`. Consumed by `frontend/src/core/hooks/useTierAccess.ts` (react-query key `feature-tiers`); denial UX via `frontend/src/shared-minimal/components/UpgradeRequiredDialog.tsx`; vehicle limits via `frontend/src/features/vehicles/hooks/useVehicleLimitCheck.ts`. The user's own tier comes from their profile (`subscription_tier`), not this endpoint. + +## 4. How to add a configuration axis + +Reminder: there is no working full local loop for most of these (missing local secrets). End-to-end proof happens on STAGING via the PR pipeline. CI runs zero tests/lint - the only gate is that images build and staging boots healthy - so run `npm run type-check` and relevant tests yourself in `backend/` / `frontend/` before pushing (see `mvp-validation-and-qa`). + +### A. New YAML config field (non-sensitive backend config) +1. Add the field to `configSchema` in `backend/src/core/config/config-loader.ts` (give it `.optional().default(...)` unless you can update every environment's YAML in the same PR - a required field missing from the mounted YAML crashes the backend at import time). +2. Add the value to `config/app/production.yml` (this one committed file serves all environments; there are no per-env YAML variants). +3. No compose change needed - `config/app/production.yml` is already mounted. Consume via `appConfig.config.
.`. +4. Deploy note: config reaches servers via the workflows' rsync of `config/`; backend containers read it at start, so the normal deploy (which recreates backend) picks it up. + +### B. New secret +1. Create `secrets/app/.txt` locally with the real value AND a committed `.txt.example` sibling with a placeholder. +2. If the backend reads it: add the filename to the `secretFiles` array in `loadSecrets()` AND a snake_case key to `secretsSchema` in `config-loader.ts` (use `.optional()` only if genuinely optional - remember the resend-webhook-secret gap started life as "optional"). +3. Mount it in EVERY compose file that runs the consuming service: base `docker-compose.yml` and, for backend/frontend, the YAML-anchored blocks in `docker-compose.blue-green.yml` (prod runs blue/green services, not the base ones). +4. Add the Gitea Actions secret (repo settings) and an `inject_secret "VAR_NAME" ".txt"` entry plus SECRET_FILES entry in `scripts/inject-secrets.sh` - otherwise the next deploy will not create the file on the server and Docker will manufacture the directory trap. +5. Verify on staging: deploy, then `docker exec mvp-backend-staging ls -l /run/secrets/` (names only - never cat values into logs or PRs). + +### C. New env var +1. Set it in the consuming service's `environment:` block in `docker-compose.yml` (and the anchored env in `docker-compose.blue-green.yml` for backend/frontend), with a sane `${VAR:-default}`. +2. Read it in exactly one place in the consumer (backend: prefer YAML config unless it is genuinely deploy-varying like the Stripe price IDs; ocr: `ocr/app/config.py`). +3. If the value differs per environment, add it to the `.env` generation step in `.gitea/workflows/staging.yaml` and `production.yaml` (Gitea Actions variable) and document it in `.env.example`. +4. Frontend `VITE_*`: add a compose `build.args` entry AND `--build-arg` lines in the staging workflow build step - and remember it is baked at build time. + +### D. New tier-gated feature +1. Add the entry to `FEATURE_TIERS` in `backend/src/core/config/feature-tiers.ts` (key, minTier, name, upgradePrompt). +2. Guard the route: `preHandler: [requireAuth, requireTier('')]` or `fastify.requireTier({featureKey: ''})`. Copy the exact key string - fail-open means a typo disables the gate silently. Add a test in `backend/src/core/config/tests/feature-tiers.test.ts` style. +3. Frontend: gate the UI with `useTierAccess` and route denials to `UpgradeRequiredDialog`. The feature list arrives automatically via `GET /api/config/feature-tiers` - no frontend registry to update. +4. Validate on BOTH mobile and desktop (project hard requirement). + +### E. New Stripe price +1. Add the env var to the backend `environment:` block in `docker-compose.yml` AND the blue-green anchor, and to `.env.example`. +2. Wire it into BOTH maps in `backend/src/features/subscriptions/domain/subscriptions.service.ts`: the plan-name-to-env-var map (~line 833) and the reverse priceId-to-tier inference (~line 885). Missing the reverse map means webhooks classify the subscription as the wrong tier. +3. Add the Gitea Actions VARIABLE (not secret - price IDs are config) and echo it into `.env` in both deploy workflows' env-generation step. +4. The publishable key side (`VITE_STRIPE_PUBLISHABLE_KEY`) is build-time frontend config - see C.4. + +## Provenance and maintenance + +Authored 2026-07-07 from direct inspection of the repo at commit e729d42 era. Volatile facts and how to re-verify each (run from repo root): + +| Fact (as of 2026-07-07) | Re-verify with | +|---|---| +| Backend env-var catalog (and DATABASE_HOST/REDIS_HOST still dead) | `grep -rnoE "process\.env(\.[A-Z_]+|\['[A-Z_]+'\])" backend/src --include="*.ts" \| grep -v ".test." \| sort -u` | +| CONFIG_PATH / SECRETS_DIR defaults | `grep -n "CONFIG_PATH\|SECRETS_DIR" backend/src/core/config/config-loader.ts` | +| The 9 backend-loaded secret names and which are optional | `sed -n '113,190p' backend/src/core/config/config-loader.ts` | +| resend-webhook-secret still unconfigured | `grep -rn "resend-webhook" docker-compose*.yml scripts/inject-secrets.sh; ls secrets/app/ \| grep resend` | +| inject-secrets file list (12) and required env vars | `grep -n "inject_secret \|SECRET_FILES" scripts/inject-secrets.sh` | +| Compose secret mounts per service | `grep -n "/run/secrets" docker-compose.yml docker-compose.blue-green.yml` | +| Secrets committed to git (should be .example + google-wif-config.json only) | `git ls-files secrets/` | +| Local directory-trap state | `ls -la secrets/app/ \| grep ^d` | +| FEATURE_TIERS entries and VEHICLE_LIMITS (free 2 / pro 5 / enterprise unlimited) | `grep -n "minTier\|VEHICLE_LIMITS" -A3 backend/src/core/config/feature-tiers.ts` | +| Fail-open on unknown feature keys | `sed -n '57,64p' backend/src/core/config/feature-tiers.ts` | +| feature-tiers endpoint shape | `cat backend/src/core/config/config.routes.ts` | +| requireTier usage sites | `grep -rn "requireTier" backend/src/features --include="*.ts" \| grep -v test` | +| Stripe price env vars in service maps | `grep -n "STRIPE_.*PRICE_ID" backend/src/features/subscriptions/domain/subscriptions.service.ts` | +| Frontend VITE_* consumption | `grep -rnoE "import\.meta\.env\.[A-Z_]+" frontend/src \| sort -u` | +| Frontend runtime Maps-key injection | `cat frontend/scripts/load-config.sh; grep -n "load-config" frontend/Dockerfile` | +| OCR env catalog and compose overrides | `cat ocr/app/config.py; sed -n '/mvp-ocr:/,/mvp-postgres:/p' docker-compose.yml` | +| config/app/ci.yml and config/shared/production.yml still unreferenced/unread | `grep -rn "ci\.yml" backend/ scripts/ .gitea/ docker-compose*.yml Makefile; grep -rn "shared.yml" backend/src` (grep's `--include` does NOT expand brace globs like `*.{ts,sh}` - it silently matches nothing) | +| production.yml.example still stale vs schema | `diff <(grep -oE "^[a-z_]+:" config/app/production.yml) <(grep -oE "^[a-z_]+:" config/app/production.yml.example)` | +| Workflow .env generation and wif-config copy | `grep -n "generate-log-config\|inject-secrets\|google-wif" .gitea/workflows/staging.yaml .gitea/workflows/production.yaml` | +| auth0-ocr secrets still mounted-but-unread | `grep -rni "auth0" ocr/app --include="*.py"` | diff --git a/.claude/skills/mvp-debugging-playbook/SKILL.md b/.claude/skills/mvp-debugging-playbook/SKILL.md new file mode 100644 index 0000000..b575d7a --- /dev/null +++ b/.claude/skills/mvp-debugging-playbook/SKILL.md @@ -0,0 +1,246 @@ +--- +name: mvp-debugging-playbook +description: Symptom-to-cause triage for MotoVaultPro. Load when debugging any of - numbers display wrong, .toFixed is not a function, values arrive as strings, dates off by one day, unexpected 403 (EMAIL_NOT_VERIFIED, TIER_REQUIRED, Unauthorized), stale data after create/update/delete, deleted record still shows, blank page on load, API calls never fire, bug only on mobile or only on desktop, route unexpectedly public or unprotected, VIN decode timeout or failure, OCR/receipt extraction returns empty fields, migration did not run, missing table or column, container unhealthy, staging deploy failed, health check failing, jest hangs or never exits. Gives the discriminating check and fix pattern for each known failure mode. +--- + +# MotoVaultPro Debugging Playbook + +Known failure modes of this codebase, each with a discriminating check (an observation that separates this cause from lookalikes) and the sanctioned fix pattern. All file paths are repo-relative. All line numbers verified 2026-07-07 and may drift; the "Provenance and maintenance" section at the end has re-verification commands. + +**When to use:** you have a symptom (bug report, failing staging deploy, weird API response) and need to know where to look and what the likely cause is. + +**When NOT to use — route instead:** +- Investigation smells like a past battle (date bugs, numeric-string bugs, OCR engine behavior, auth races, "why is there no VIN cache") -> read `mvp-failure-archaeology` FIRST so you do not re-fight a settled war or reintroduce a reverted fix. +- You need LogQL recipes, Grafana access, health-check anatomy, or container debugging technique -> `mvp-diagnostics-and-logging`. +- You need to set up an environment or run tests at all -> `mvp-build-and-env`. +- You are deciding how to ship the fix (issue/branch/PR, review rules) -> `mvp-change-control`. +- You need rigorous root-cause methodology or benchmark discipline -> `mvp-proof-and-analysis-toolkit`. +- Deploy/rollback mechanics on staging or prod -> `mvp-run-and-operate`. + +**Environment reality (2026-07-07):** development is done by AI sessions; end-to-end verification happens on STAGING via the PR deploy pipeline (every PR push builds images and redeploys staging). There is no fully working local dev loop. CI gates nothing beyond build + boot (canonical statement: `mvp-validation-and-qa` section 1) — do not assume "CI passed" means anything about correctness. + +**Owner non-negotiables — never route around them while debugging:** +1. No destructive database operations without a fresh backup. This includes `make clean` (destroys volumes), `scripts/import-database.sh --drop-existing`, schema migrations on staging/prod, and the backend integration tests, which `DROP TABLE ... CASCADE` on the shared dev database (e.g. `backend/src/features/vehicles/tests/integration/vehicles.integration.test.ts:38`). +2. Never hand-edit files on the staging/prod servers — `rsync --delete` on every deploy reverts them. All server changes flow through the repo and workflows. + +## First five minutes checklist + +Before forming any theory, gather these observations (cross-ref `mvp-diagnostics-and-logging` for the full cookbook): + +1. **Health endpoints.** `curl -s https://staging.motovaultpro.com/api/health | jq .` — must be `status: "healthy"` with a `features` array (20 features listed in `backend/src/app.ts`; the staging verify job requires a 13-feature subset, `.gitea/workflows/staging.yaml`). +2. **Container state** (on the server, `/opt/motovaultpro`): `docker compose -f docker-compose.yml -f docker-compose.staging.yml ps` — look for `unhealthy` or restart loops. Container names on staging carry a `-staging` suffix (`mvp-backend-staging`); dev/base names are `mvp-backend`, `mvp-ocr`, etc. +3. **Backend request logs in Grafana** (logs.staging.motovaultpro.com, RFC1918-only access): + `{container="mvp-backend-staging"} | json | msg="Request processed" | status >= 400` + Fields available: `level, requestId, method, path, status, duration`. `msg="Request processed"` is the request-log filter key. +4. **Errors across all containers:** `{container=~"mvp-.*"} | json | level="error"`. +5. **OCR container logs** if the symptom involves VIN/receipts/manuals: `docker logs --tail 200 mvp-ocr-staging` on staging (dev/base name is `mvp-ocr`; prod also uses the un-suffixed `mvp-ocr` — the blue-green overlay defines no OCR variant, it is a single shared instance there). +6. **Frontend errors: browser devtools console only.** Frontend logs never reach Loki. +7. **Did a deploy just happen?** Every PR push redeploys staging with brief full downtime; mid-deploy 502s are expected noise. + +Then, before touching code: state a hypothesis that predicts a specific observation, and check the triage table below. + +## Triage table + +| # | Symptom | Likely cause | Jump to | +|---|---------|--------------|---------| +| 1 | Numbers wrong, `.toFixed is not a function`, values are strings | pg NUMERIC returned as string; mapper missed coercion | 1 | +| 2 | Dates off by one day | One of the three UTC traps reintroduced | 2 | +| 3 | Unexpected 403 | EMAIL_NOT_VERIFIED vs TIER_REQUIRED vs ownership | 3 | +| 4 | Stale data after mutation; deleted record still visible | Redis cache invalidation miss | 4 | +| 5 | Blank page; API calls never fire on load | Auth-gate race / bypassed apiClient | 5 | +| 6 | Bug only on mobile (or page missing on mobile) | 768px app fork; a registration-checklist step missed | 6 | +| 7 | Route unexpectedly public / unguarded | Missing per-route preHandler; tier guard fails open | 7 | +| 8 | VIN decode failure or timeout | Gemini path: timeout stack, event-loop blocking, SDK drift | 8 | +| 9 | OCR/receipt extraction returns empty fields | Silent Gemini fallback; Vision monthly cap; engine config | 9 | +| 10 | Migration did not run; missing table/column | Not in MIGRATION_ORDER, or image not rebuilt | 10 | +| 11 | Container unhealthy; staging deploy failed | Health-check anatomy; start_period; verify job | 11 | +| 12 | Jest hangs locally / frontend jest crashes | Open pg/redis handles; broken tdd-guard reporter config | 12 | + +### 1. Numbers display wrong / `.toFixed` crashes / values are strings + +**Likely cause.** The pg driver returns PostgreSQL `NUMERIC`/`DECIMAL` (type OID 1700) as JavaScript strings. The only global type-parser override in `backend/src/core/config/database.ts` is for `DATE` (OID 1082, line 12) — there is deliberately NO global numeric override. Every repository mapper must coerce numerics manually with `Number()`/`parseFloat()`. This has caused 5 historical incidents (#47, #49, #239, #241, #244); it is statistically the most likely bug when adding or changing any repository method. + +**Discriminating check.** Hit the API and inspect the JSON type: a numeric field arriving as `"42.50"` (string) instead of `42.5` is this bug. Or in the failing frontend code, `typeof value === 'string'` where a number is expected. + +```bash +grep -n "parseFloat\|Number(" backend/src/features//data/.repository.ts +``` + +**Fix pattern.** Coerce in the repository mapper (`mapRow()` or equivalent), never in the frontend and never by returning raw rows. Beware the fuel-logs exception: `mapEnhancedRow` in `backend/src/features/fuel-logs/data/fuel-logs.repository.ts` deliberately returns coerced snake_case rows and the service's `toEnhancedResponse` does the camelCase mapping — do not "normalize" this without reading `mvp-failure-archaeology` (incident #47 -> #244 chain). Do NOT add a global OID 1700 parser as a drive-by fix; that is an architectural decision (see `mvp-architecture-contract`). + +### 2. Dates off by one day + +**Likely cause.** One of three settled UTC traps has been reintroduced: +(a) backend: removing/bypassing the DATE parser override `types.setTypeParser(1082, ...)` in `backend/src/core/config/database.ts:12` — pg would return DATE as a local-midnight `Date` object that shifts a day under `toISOString()`; +(b) frontend: `new Date("YYYY-MM-DD")` parses as UTC midnight, then `toLocaleDateString()` shifts a day back west of UTC; +(c) OCR/backend: `toISOString().split('T')[0]` on a Date built from a date-only value. + +**Discriminating check.** Trace where the value first deviates: check the raw API response (should be a plain `"YYYY-MM-DD"` string for DATE columns). If the API string is right, the bug is frontend display; if wrong, backend/OCR. + +**Fix pattern (the settled rule — do not innovate).** DATE columns flow as plain `YYYY-MM-DD` strings end to end; the full handling rules (dayjs display, lexicographic sort, when `new Date` is legitimate) are canonical in `mvp-vehicle-domain-reference` section 3. See `mvp-failure-archaeology` (four fixes in one day, 2026-03-23) before touching any date code. + +### 3. Unexpected 403 responses + +Three distinct producers — identify which by the response body: + +| Body | Producer | Where | +|------|----------|-------| +| `code: "EMAIL_NOT_VERIFIED"` | Auth plugin email-verification guard | `backend/src/core/plugins/auth.plugin.ts` (~line 231); exempt route prefixes `/api/auth/`, `/api/onboarding/`, `/api/health`, `/health` (lines 18-23) | +| `error: "TIER_REQUIRED"` with `requiredTier`/`currentTier` | Tier gating (two parallel mechanisms) | `backend/src/core/plugins/tier-guard.plugin.ts` (~line 93) or `backend/src/core/middleware/require-tier.ts` (~line 50) | +| Generic 403, message containing "Unauthorized" | Ownership failure string-matched in the controller | e.g. `backend/src/features/fuel-logs/api/fuel-logs.controller.ts` — controllers catch and map `error.message.includes('not found')` -> 404, `includes('Unauthorized')` -> 403, else 500. There are no typed error classes. | + +**Discriminating checks.** +- EMAIL_NOT_VERIFIED: does the user's Auth0 account show verified? Guard runs on every authenticated route not in the exempt list. +- TIER_REQUIRED: is the route tier-gated? (Current key catalog and limits: `mvp-config-and-secrets` section 3.) What is `user_profiles.subscription_tier`? +- Ownership: does the row's `user_id` match the caller's internal UUID? Note `request.userContext.userId` is the internal `user_profiles` UUID, NOT the Auth0 `sub`. + +**Fix pattern.** For ownership bugs, check the service's ownership comparison and that the correct user UUID reached it. When adding error paths, match the existing string-matching contract exactly ("not found", "Unauthorized") or the controller will map your error to 500. + +### 4. Stale data after mutation + +**Likely cause.** Redis cache invalidation miss. Cache facts: keys prefixed `mvp:` (`backend/src/core/config/redis.ts:22`); fuel-logs caches per `{userId}:{unitSystem}` with TTL 300s (`fuel-logs.service.ts:18`); platform dropdown caches TTL 6h (`backend/src/features/platform/domain/platform-cache.service.ts`). Cache errors are swallowed (cacheService returns null on failure) — a broken Redis never 500s, it just serves stale or uncached. + +**KNOWN LIVE BUG.** `deleteFuelLog` invalidates only the `imperial` cache keys, so a metric-preference user sees deleted fuel logs for up to 300s. If you are debugging "deleted log still shows" for a metric user, this is it — the full record (file:line evidence and status) is canonical in `mvp-launch-readiness` section 1, gap 4; file/fix the bug rather than hunting elsewhere. + +**Discriminating check.** Does the stale window self-heal at the TTL (300s fuel-logs, 6h platform)? If yes, invalidation miss. Inspect keys directly: + +```bash +docker exec mvp-redis redis-cli --scan --pattern 'mvp:fuel-logs:*' +``` + +**Fix pattern.** Invalidate BOTH unit systems (or `deletePattern`) on every mutation path; check create/update/delete all invalidate symmetrically. + +### 5. Blank page / API calls never fire on load + +**Likely cause.** Auth-gate race. All axios calls through the shared `apiClient` (`frontend/src/core/api/client.ts`) are queued until Auth0 init completes (`frontend/src/core/auth/auth-gate.ts`; `setAuthReady` flipped by `Auth0Provider`). Historical incidents: blank Stations page (2025-11), dashboard auth-gate (#45), mobile login IndexedDB/callback saga (#188/#190). + +**Discriminating check.** Devtools Network tab: are the requests pending/queued (never sent) or sent and failing? Queued-forever means auth never initialized — check console for Auth0 errors. Sent-and-401 pre-auth means something bypassed `apiClient` (raw `axios`/`fetch` import). + +**Fix pattern.** Always use `apiClient` from `core/api/client.ts` — never raw axios. Do not add data fetches that race Auth0 init. Also settled: URL-sync effects in `App.tsx` must skip `/callback`, `/signup`, `/verify-email` (child effects fire before parent effects; stripping `?code=&state=` breaks Auth0 login). + +### 6. Mobile-only bugs / page missing on mobile + +**Likely cause.** The app hard-forks at `window.innerWidth <= 768` plus a UA regex in `frontend/src/App.tsx` (~lines 360-398). Desktop renders react-router ``; mobile renders a Zustand-driven screen switcher (`useNavigationStore().activeScreen`, NO react-router). A page can work perfectly on desktop and be entirely absent on mobile. + +**Discriminating check.** Shrink the window below 768px (or use device emulation). Then walk the CANONICAL registration checklist in `mvp-architecture-contract` invariant 2.7 (7 steps: desktop route, `MobileScreen` union, both nav maps, lazy import, mobile render block, navigation entry point) and find the missing step. + +**Fix pattern.** Complete every step of that checklist — a screen registered in the maps but missing its lazy import or a navigation entry point is silently unreachable. Note the breakpoint systems are NOT uniform: app fork = 768px, MUI component checks = `sm` (600) / `md` (900). Mobile+desktop is a hard project requirement — verify both on staging. + +### 7. Route unexpectedly public or unguarded + +**Likely cause.** Auth is per-route: every protected route must list `preHandler: [fastify.authenticate]` explicitly — there is NO global auth hook. Intentional exceptions exist (webhooks, signup, health, `/auth/verify`, feature-tiers config): the canonical list lives in the header comment of `.claude/skills/mvp-diagnostics-and-logging/scripts/check-route-auth.sh` — check a suspect route against it before "fixing" a deliberately public endpoint. Forgetting the preHandler ships an open route, and CI will not catch it. + +**Compounding traps (all verified):** +- The tier guard FAILS OPEN on unknown feature keys: `canAccessFeature` in `backend/src/core/config/feature-tiers.ts` (~lines 57-61) returns `true` for unregistered keys. A typo in a `featureKey` silently ungates a plugin-guarded route. +- Two parallel tier mechanisms exist with different failure behavior — the middleware 500s on unknown keys, the plugin decorator fails open (mechanics canonical in `mvp-config-and-secrets` section 3). +- `backend/src/features/backup/api/backup.routes.ts` uses untyped `(fastify as any).requireAdmin` 16 times (from line 32); a misspelled guard there compiles clean and ships an unguarded admin endpoint. Do not copy that file as a route template. + +**Discriminating check.** `curl` the route with no Authorization header — a 200 on a should-be-protected route confirms. Audit: + +```bash +grep -L "authenticate" backend/src/features/*/api/*.routes.ts +grep -rn "featureKey" backend/src/features/*/api/*.routes.ts # then match each key against FEATURE_TIERS +``` + +**Fix pattern.** Add the explicit preHandler; verify the featureKey exists in `FEATURE_TIERS`; prefer the typed `fastify.requireAdmin`/`fastify.requireTier` decorators. + +### 8. VIN decode failures / timeouts + +**The chain:** frontend `POST /vehicles/decode-vin` (axios timeout 120s, `frontend/src/features/vehicles/api/vehicles.api.ts:90`) -> `vehicles.controller.ts` (maps OCR 503/422 -> 502, timeout/abort -> 504) -> `backend/src/features/ocr/external/ocr-client.ts` (`OCR_TIMEOUT_MS = 120000`, line 9, hardcoded) -> Python OCR container `POST /decode/vin` -> Gemini with Google Search grounding. + +**Likely causes, in order:** +1. **Timeout race:** frontend 120s == backend-client 120s with no headroom, and there is NO service-side timeout on the Gemini call itself. Search-grounded decodes have been observed >60s. +2. **Event-loop blocking:** `ocr/app/routers/decode.py` calls the synchronous `_gemini_engine.decode_vin(vin)` inside an `async def` handler — a slow call blocks the whole uvicorn event loop including `/health`. The OCR healthcheck (docker-compose.yml: interval 5s, timeout 5s, retries 3) can then mark the container unhealthy and Docker can restart it MID-REQUEST. +3. **SDK drift:** `google-genai>=1.0.0` is unpinned in `ocr/requirements.txt`; new config params have broken staging before (pydantic validation error from `AutomaticFunctionCallingConfig` — reverted; never re-add AFC config). + +**Discriminating check.** `docker logs mvp-ocr-staging` (on staging; base/prod name `mvp-ocr`) around the failure time. Diagnostic raw-JSON logging exists: `gemini_engine.py` logs `"Gemini decoded VIN %s (confidence=...) raw=%s"` (~line 401) with the full raw Gemini response. A container restart in `docker ps` uptime plus a 504 upstream = the healthcheck-kill scenario. A pydantic ValidationError in logs = SDK drift. + +**Fix pattern.** Do not "fix" by raising the frontend timeout again (already raised 30->60->120s; the fix is service-side). Settled: there is NO VIN decode cache by design (deleted after race/staleness bugs — see `mvp-failure-archaeology`); do not re-add one. Model year is computed deterministically from VIN positions 7/10, never trusted from the LLM. + +### 9. OCR / receipt extraction returns empty results + +**Likely cause.** In `ocr/app/extractors/maintenance_receipt_extractor.py` (~lines 129-133), any Gemini failure is caught and silently falls back to OCR-only with `gemini_fields = {}` — the user sees "no fields extracted" with no error. Also check: +- **Vision monthly cap:** `VISION_MONTHLY_LIMIT` is 1000 requests/month; compose sets `OCR_PRIMARY_ENGINE: google_vision`, `OCR_FALLBACK_ENGINE: paddleocr` (docker-compose.yml ~lines 201-206, overriding the code defaults in `ocr/app/config.py`). Cap exhaustion degrades to PaddleOCR quality. +- **Model config:** compose deploys `GEMINI_MODEL: gemini-3-flash-preview` (line 210) — a preview model; the code default is `gemini-2.5-flash`. Extraction-quality regressions can be model-side. +- Email-ingestion amplification: unconfident classification runs BOTH fuel and maintenance OCR endpoints per attachment, burning the Vision cap faster. + +**Discriminating check.** `docker logs mvp-ocr-staging | grep -i "Gemini extraction failed"` (base/prod container name is `mvp-ocr`) — the fallback logs a warning with the exception. WIF/auth failures log "Gemini authentication failed". Verify effective engine config: `docker exec mvp-ocr-staging env | grep -E "OCR_|GEMINI|VISION"`. + +**Fix pattern.** Fix the upstream Gemini/auth failure; do not paper over with confidence-threshold tweaks. The WIF credential chain (Auth0 M2M -> `google-wif-config.json` at `/run/secrets/`) is duplicated in `gemini_engine.py` AND `maintenance_receipt_extractor.py` `_get_client()` — auth fixes must be applied in both. + +### 10. Migration did not run / missing table or column + +**Likely cause.** Migrations are feature-owned SQL run in the hard-coded `MIGRATION_ORDER` array in `backend/src/_system/migrations/run-all.ts` (line 17). A new feature's `migrations/` directory is SILENTLY skipped unless the feature is appended to that array. Migrations run automatically on backend container start (`backend/Dockerfile:84`: `node dist/_system/migrations/run-all.js && npm start`), which means they are packaged into the image — a migration added to source requires an IMAGE REBUILD and redeploy to run on staging/prod. + +**Discriminating check.** + +```bash +grep -n "features/" backend/src/_system/migrations/run-all.ts # is it in MIGRATION_ORDER? +docker exec mvp-postgres psql -U postgres -d motovaultpro -c "SELECT * FROM _migrations ORDER BY executed_at DESC LIMIT 20;" +docker logs mvp-backend 2>&1 | grep -i migrat +``` + +(Adjust container names/DB credentials per environment; on staging they carry `-staging`.) + +**Fix pattern.** Append to `MIGRATION_ORDER` respecting dependencies (`update_updated_at_column()` is defined in `features/vehicles`, which must run first; `core/identity-migration` runs last). NON-NEGOTIABLE: schema migrations on staging/prod require a fresh backup first (`scripts/ci/maintenance-migrate.sh backup`, or `make db-backup`). + +### 11. Container unhealthy / staging deploy failed + +**Health-check anatomy (docker-compose.yml, verified):** backend has `start_period: 180s` (line 159) specifically because auto-migrations run on start — a backend "starting" for 2 minutes is normal, not a failure. OCR: interval 5s / timeout 5s / start_period 15s (and see symptom 8 for why OCR can flap under load). The `verify-staging` job waits up to 4 minutes (48x5s) for Docker health, then curls `https://staging.motovaultpro.com/api/health` and requires `status=healthy` plus a 13-feature required list (`.gitea/workflows/staging.yaml`, `REQUIRED_FEATURES`). + +**Discriminating check, in order:** +1. Which job failed? Build failure = code/Dockerfile problem. Deploy failure = server-side (disk, secrets). Verify failure = app boot problem. +2. `docker inspect --format '{{json .State.Health}}' mvp-backend-staging | jq .` — see the actual failing probe output. +3. `docker logs mvp-backend-staging --tail 100` — migration errors are the most common boot killer. +4. Disk: `df -h /` on the staging runner — the 29G root fills with per-commit images; a full disk fails builds and deploys in confusing ways (`docker system df`; a daily prune cron exists out-of-band). +5. Missing secret trap: Docker bind-mounting a missing secret file creates a DIRECTORY; the workflows contain `rm -rf` cleanup for exactly this. An empty directory where a secret file should be breaks the consumer at read time. + +**Fix pattern.** Fix in the repo and redeploy via the workflow. Never hand-edit on the server (non-negotiable; rsync --delete reverts it). Full deploy/rollback procedure: `mvp-run-and-operate`. + +### 12. Jest hangs locally / frontend jest broken + +**Backend:** `cd backend && npm test` runs tests and then never exits — pg pool and ioredis connect eagerly at module import (`backend/src/core/config/database.ts`, `redis.ts` — `new Redis(...)` with no `lazyConnect`) and `backend/jest.config.js` sets no `forceExit`. Run: + +```bash +cd backend && npx jest --forceExit # or: npm test -- --forceExit +``` + +Also remember: backend INTEGRATION tests `DROP TABLE ... CASCADE` on the database they point at — never aim them at a database you care about without a fresh backup (non-negotiable). + +**Frontend:** `cd frontend && npm test` is broken on the host: `frontend/jest.config.ts` registers the `tdd-guard-jest` reporter (line 31) with a hardcoded `projectRoot: '/home/egullickson/motovaultpro'` (line 33) — a Linux path that does not exist on this macOS checkout. Discriminating check: the failure mentions tdd-guard or that path. A verified working fallback exists — `npx jest --reporters=default` — with its traps and the known-red baseline documented in `mvp-build-and-env` section 2 (the single home for frontend-jest mechanics). Note there is no in-container run either (nginx-only image, tests dockerignored); end-to-end verification is still staging (see `mvp-validation-and-qa`). + +## Discriminating-experiment discipline + +Cheap rules that prevent the historical "fix symptoms until root cause found" chains (the mobile-login saga took 9 commits; the Tesseract VIN saga took 12 in one day): + +1. **Predict before you look.** Write the hypothesis as "if X is the cause, then I will observe exactly Y at Z" — then look. If you find yourself explaining an observation after the fact, it did not discriminate. Full protocol: `mvp-proof-and-analysis-toolkit`. +2. **One variable per experiment.** Especially on staging, where every PR push redeploys the whole stack: change one thing, push, observe. Two changes per push means an ambiguous result. +3. **Locate the first deviation, not the last symptom.** Trace the value/request through the chain (DB -> repository mapper -> service -> controller -> API JSON -> frontend) and find the FIRST point it is wrong. Most historical band-aids (frontend `Number()` wrappers, timeout bumps) patched the last symptom. +4. **Distrust green CI.** CI runs zero tests and zero lint; "CI passed" only means images built and staging booted. Evidence standards: `mvp-validation-and-qa`. +5. **Check the archaeology first.** If the bug involves dates, numeric strings, VIN caching, auth races, Stripe IDs, or OCR engines, someone already fought it. `mvp-failure-archaeology` lists the settled outcomes and the reverts you must not resurrect. + +## Provenance and maintenance + +Authored 2026-07-07 by direct repo inspection; discovery-report claims were independently re-verified against code. Line numbers drift — re-verify before relying on them: + +| Volatile fact | Re-verification command | +|---------------|------------------------| +| DATE-only parser override (OID 1082), no NUMERIC override | `grep -n "setTypeParser" backend/src/core/config/database.ts` | +| deleteFuelLog imperial-only invalidation (live bug) | `grep -n "invalidateCaches" backend/src/features/fuel-logs/domain/fuel-logs.service.ts` | +| Cache prefix `mvp:`, fuel-logs TTL 300s, platform TTL 6h | `grep -n "prefix" backend/src/core/config/redis.ts; grep -n "cacheTTL" backend/src/features/fuel-logs/domain/fuel-logs.service.ts; grep -n "6 \* 3600" backend/src/features/platform/domain/platform-cache.service.ts` | +| EMAIL_NOT_VERIFIED guard + exempt routes | `grep -n "EMAIL_NOT_VERIFIED\|VERIFICATION_EXEMPT" backend/src/core/plugins/auth.plugin.ts` | +| Tier guard fails open on unknown keys | `grep -n -A3 "fail open" backend/src/core/config/feature-tiers.ts` | +| Controller string-matched error mapping | `grep -n "includes('not found')\|includes('Unauthorized')" backend/src/features/fuel-logs/api/fuel-logs.controller.ts` | +| Untyped requireAdmin in backup routes | `grep -c "(fastify as any).requireAdmin" backend/src/features/backup/api/backup.routes.ts` | +| 120s timeouts (frontend + backend OCR client) | `grep -n "OCR_TIMEOUT_MS" backend/src/features/ocr/external/ocr-client.ts; grep -n "timeout" frontend/src/features/vehicles/api/vehicles.api.ts` | +| Sync Gemini call in async decode handler | `grep -n "decode_vin(vin)" ocr/app/routers/decode.py` | +| Gemini raw-JSON diagnostic logging | `grep -n "raw=%s" ocr/app/engines/gemini_engine.py` | +| Silent empty-fields fallback | `grep -n -B1 "gemini_fields = {}" ocr/app/extractors/maintenance_receipt_extractor.py` | +| Compose OCR engine/model config | `grep -n "OCR_PRIMARY_ENGINE\|GEMINI_MODEL\|VISION_MONTHLY_LIMIT" docker-compose.yml` | +| MIGRATION_ORDER + migrate-on-start | `grep -n "MIGRATION_ORDER" backend/src/_system/migrations/run-all.ts; grep -n "run-all" backend/Dockerfile` | +| Backend start_period 180s; OCR healthcheck 5s/5s | `grep -n "start_period" docker-compose.yml` | +| 13-feature staging verify list | `grep -n "REQUIRED_FEATURES" .gitea/workflows/staging.yaml` | +| 768px fork + page-registration checklist (canonical: `mvp-architecture-contract` 2.7) | `grep -n "innerWidth <= 768" frontend/src/App.tsx; grep -n "routeToScreen\|screenToRoute" frontend/src/core/store/navigation.ts` | +| Auth-gate request queue | `grep -n "authReady\|queueRequest" frontend/src/core/api/client.ts` | +| Frontend jest hardcoded projectRoot | `grep -n "projectRoot" frontend/jest.config.ts` | +| Integration tests DROP TABLE CASCADE | `grep -rn "DROP TABLE" backend/src/features/*/tests/integration/` | diff --git a/.claude/skills/mvp-deploy-safety-campaign/SKILL.md b/.claude/skills/mvp-deploy-safety-campaign/SKILL.md new file mode 100644 index 0000000..4a5565a --- /dev/null +++ b/.claude/skills/mvp-deploy-safety-campaign/SKILL.md @@ -0,0 +1,338 @@ +--- +name: mvp-deploy-safety-campaign +description: Load when working on CI/CD safety for MotoVaultPro - adding tests/lint to CI, fixing the ":latest" image tag hazard, changing PR staging deploys, or when you notice symptoms like "CI runs zero tests", "PR builds overwrite latest", "production deployed an unmerged PR build", "every PR redeploys staging", "tests never ran in the pipeline", "image_tag defaults to latest", or "the PR was green but the code is broken". This is the executable, decision-gated campaign to close the deploy-safety hole (owner-confirmed hardest live problem, 2026-07-07). Also load before touching .gitea/workflows/staging.yaml or production.yaml for any reason. +--- + +# Deploy Safety Campaign + +An executable, phased campaign to close MotoVaultPro's deploy-safety hole. Verified state as of 2026-07-07: + +1. CI runs ZERO tests and ZERO lint. The only PR gate is that 3 Docker images build (tsc compiles inside them) and the staging stack boots healthy. +2. Every PR build pushes `backend:latest`, `frontend:latest`, `ocr:latest` to the registry (`.gitea/workflows/staging.yaml` build job). +3. Production deploys default to `image_tag: latest` (`.gitea/workflows/production.yaml:14-17`), so one click can ship an unmerged PR build to production. +4. Every PR open/sync/reopen tears down and redeploys the single shared staging environment (last PR wins). + +## When to use / When NOT to use + +Use this skill when: executing any phase of this campaign; adding test/lint jobs to CI; changing image tagging or the production `image_tag` input; changing when staging deploys happen; or evaluating whether the campaign's gates still hold. + +Do NOT use this skill for: +- How to classify, review, and ship a change (issue/branch/PR/labels): `mvp-change-control`. Every phase here ships THROUGH that process, never around it. +- Running or fixing tests locally, jest/pytest traps: `mvp-build-and-env` +- What counts as test evidence, adding tests, mobile+desktop validation: `mvp-validation-and-qa` +- Deploy mechanics, blue-green, rollback, backups: `mvp-run-and-operate` +- Debugging a broken staging/prod stack right now: `mvp-debugging-playbook` +- Measuring outcomes in Grafana/Loki: `mvp-diagnostics-and-logging` + +## Ground rules (non-negotiable) + +- Every change in this campaign ships via `mvp-change-control`: issue -> branch `issue-{N}-{slug}` -> PR -> staging verify -> owner review -> merge. One issue per phase (see Validation protocol at the end). +- Pipeline changes (any edit under `.gitea/workflows/`) and anything touching database strategy require explicit owner sign-off in the issue before the PR is opened. +- NEVER point the backend integration tests at a shared database. They run real migrations in `beforeAll` and `DROP TABLE ... CASCADE` in `afterAll` (verified: `backend/src/features/vehicles/tests/integration/vehicles.integration.test.ts:37-41`). Ephemeral database or nothing. +- Never hand-edit files on the staging/prod servers; `rsync --delete` on every deploy reverts them. +- Success at each gate is a number or an exact string, never "looks right". If an EXPECTED observation does not match, follow the branch instruction; do not improvise. + +Terminology: "runner" = the self-hosted Gitea act_runner on the staging host (`mvp-staging`), label `stage`, HOST mode (`runner_labels: "stage:host"` in `ansible/deploy-staging-runner.yml:25`) - jobs execute directly on the host shell, with Docker CLI and Node.js 20 (NodeSource, `ansible/deploy-staging-runner.yml:107-123`) available. Host mode means Gitea Actions `services:` blocks are NOT available; use `docker run` directly. + +--- + +## Phase 0 - Baseline evidence + +Goal: capture the current gate reality in numbers, committed to the phase issue. No code changes. + +### 0.1 Confirm CI runs zero tests/lint + +```bash +grep -rnE "npm (test|run lint|run type-check)|pytest|eslint|jest" .gitea/workflows/ +``` + +EXPECTED: no output, exit code 1 (verified 2026-07-07). +IF you get hits: someone has already started this work. Stop, read the matching workflow lines and the phase issues, and reconcile before proceeding. + +### 0.2 Confirm the :latest and PR-trigger hazards + +```bash +sed -n '8,13p' .gitea/workflows/staging.yaml # trigger block +grep -cn ":latest" .gitea/workflows/staging.yaml # latest usage count +grep -n -A4 "image_tag:" .gitea/workflows/production.yaml | head -8 +``` + +EXPECTED (verified 2026-07-07): +- Trigger block shows `push: branches: [main]` AND `pull_request: types: [opened, synchronize, reopened]`. +- 9 `:latest` occurrences in staging.yaml: 3 `--cache-from` (lines 58, 74, 85), 3 `-t ...:latest` (lines 60, 76, 87), 3 `docker push ...:latest` (lines 96-98). +- production.yaml input: `image_tag` with `required: false` and `default: 'latest'` (lines 14-17). + +IF counts differ: Phase 3 may be partially done. Diff against git history (`git log -p .gitea/workflows/staging.yaml`) before prescribing anything. + +### 0.3 Record local suite baselines + +Numbers measured 2026-07-07 on a dev machine (no `/app/config`, node v25). Your numbers go in the phase issue; if they differ materially from below, record yours as the new baseline and note the delta. + +Backend unit tests (integration excluded - note the pattern is the SUBSTRING `integration`, because `audit-log/__tests__/audit-log.integration.test.ts` and `user-import/tests/user-import.integration.test.ts` do not live under a `/integration/` directory): + +```bash +cd backend && npx jest --testPathIgnorePatterns "/node_modules/" "integration" --forceExit 2>&1 | tail -5 +``` + +EXPECTED (2026-07-07, re-confirmed 2026-07-09): `Test Suites: 15 failed, 10 passed, 25 total` / `Tests: 2 failed, 147 passed, 149 total`. The 15 failing suites split three ways, all verified: +- 6 suites die with `Configuration file not found at /app/config/production.yml` - `config-loader.ts:155` defaults `CONFIG_PATH` to a container path and loads eagerly at import; any suite that transitively imports `core/config/database.ts` dies at load. +- 7 suites fail ts-jest compilation (type errors inside test files: audit-log.routes, auth.service, documents.repository, documents.service, fuel-logs.service, vehicle-data.service, community-stations.service). `backend/tsconfig.json:28` excludes `**/*.test.ts`, so `npm run type-check` is green (verified) while ts-jest, which does type-check tests, fails these suites. Local type-check green does NOT mean tests compile. +- 2 suites (documents.controller.tier, ocr-receipt) contain the 2 genuinely failing tests. + +Frontend (the `--reporters=default` flag is mandatory outside the container): + +```bash +cd frontend && npx jest --reporters=default 2>&1 | tail -5 +``` + +EXPECTED (2026-07-07): `Test Suites: 14 failed, 17 passed, 31 total` / `Tests: 17 failed, 196 passed, 213 total`, ~11s. Without the flag you get `Error: Could not resolve a module for a custom reporter. Module name: tdd-guard-jest` - `frontend/jest.config.ts:28-36` requires a reporter that is only declared in the root package.json (which has no node_modules), and hardcodes `projectRoot: '/home/egullickson/motovaultpro'`. + +OCR - pytest is not installed on dev machines; record inventory only: + +```bash +grep -rc "def test_" ocr/tests/*.py | awk -F: '{s+=$2} END {print s}' +``` + +EXPECTED: `310` (16 test files; verified 2026-07-07). `pytest>=7.4.0` and `pytest-asyncio` are in `ocr/requirements.txt:34-35`, so pytest IS installed inside the built OCR image. + +### 0.4 Gate + +Phase 0 is complete when the phase issue contains: the three grep outputs from 0.1/0.2, the exact `Test Suites:`/`Tests:` lines for backend and frontend, the OCR count, and the list of failing suites/tests by name. That issue comment is the baseline every later phase is measured against. + +--- + +## Phase 1 - Make the suites runnable in CI at all + +Prerequisite engineering. No workflow changes yet. Owner sign-off required on decisions 1B (database strategy) before implementation. + +### 1A. Frontend jest portability (decision made: fix config, keep reporter optional) + +The blocker is two-fold: the `tdd-guard-jest` reporter cannot resolve, and `projectRoot` is a hardcoded Linux path. Two working options, verified: + +1. CI-side only: run `npx jest --reporters=default`. CLI `--reporters` replaces the config's reporters before the module is resolved (verified 2026-07-07: run proceeds to real pass/fail output). Zero code change; every CI invocation must remember the flag. +2. Fix the config (recommended): make the reporter conditional so `npx jest` works everywhere. Diff shape for `frontend/jest.config.ts`: + +```ts +// replace the hardcoded reporters block (lines 28-36) with: +const reporters: Config['reporters'] = ['default']; +if (process.env.TDD_GUARD === '1') { + reporters.push(['tdd-guard-jest', { projectRoot: process.cwd() }]); +} +// ...and inside config: reporters, +``` + +Gate 1A: `cd frontend && npx jest 2>&1 | tail -3` on a dev machine prints a `Tests:` summary line (numbers matching the Phase 0 baseline, not the reporter resolution error). + +The 17 failing frontend tests (14 suites) from Phase 0 must each be dispositioned: fixed, or moved out of the suite with a filed issue per suite. CI cannot gate on a suite that is red on day one. Record the final green count - it becomes the Phase 2 expected number. + +### 1B. Backend test database strategy (owner sign-off REQUIRED) + +Non-negotiable: an EPHEMERAL PostgreSQL, created and destroyed per CI run. The integration suites DROP TABLE CASCADE on whatever database they reach. The runner IS the staging host - a misconfigured host/port reaches the staging database. Concrete shape (runs on the `stage` runner, host mode, so `docker run`, not `services:`): + +```bash +NET="ci-test-$SHORT_SHA" +docker network create "$NET" +docker run -d --rm --name "ci-pg-$SHORT_SHA" --network "$NET" --network-alias mvp-postgres \ + -e POSTGRES_DB=motovaultpro -e POSTGRES_USER=postgres -e POSTGRES_PASSWORD=citest \ + git.motovaultpro.com/egullickson/mirrors/postgres:18-alpine +docker run -d --rm --name "ci-redis-$SHORT_SHA" --network "$NET" --network-alias mvp-redis \ + git.motovaultpro.com/egullickson/mirrors/redis:8.4-alpine +# ... run tests ... then teardown: +docker stop "ci-pg-$SHORT_SHA" "ci-redis-$SHORT_SHA"; docker network rm "$NET" +``` + +The network aliases `mvp-postgres`/`mvp-redis` match `config/app/ci.yml`, which was clearly authored for this purpose BUT is currently broken: it fails the zod schema in `config-loader.ts` with `invalid_type ... path: ["auth0"]` (verified 2026-07-07 by pointing `CONFIG_PATH` at it). Part of 1B is completing `ci.yml` until it parses - copy the missing blocks (`auth0`, and any further zod complaints) from `config/app/production.yml` with dummy-safe values, iterating until one unit suite loads. + +Secrets: `config-loader.ts:175-205` reads EXTENSION-LESS files (`postgres-password`, not `postgres-password.txt`) from `SECRETS_DIR` and requires 8 of them to parse. For tests, generate dummies: + +```bash +mkdir -p /tmp/ci-secrets +for f in auth0-client-secret auth0-management-client-id auth0-management-client-secret \ + google-maps-api-key resend-api-key stripe-secret-key stripe-webhook-secret; do + echo "ci-dummy" > /tmp/ci-secrets/$f +done +echo "citest" > /tmp/ci-secrets/postgres-password # must match POSTGRES_PASSWORD above +``` + +Execution vehicle for backend tests in CI - two options, pick one in the phase issue: +- Host node 20 (simplest): `cd backend && npm ci && CONFIG_PATH=$PWD/../config/app/ci.yml SECRETS_DIR=/tmp/ci-secrets npx jest --forceExit`. Requires connecting to the ephemeral DB via published port instead of network alias (host is not on the docker network): publish `-p 127.0.0.1:55432:5432` and set `ci.yml` database host/port accordingly. +- Builder-stage image (hermetic): the backend CI build uses context `.` with NO root `.dockerignore` (verified: `backend/.dockerignore` exists but does not apply when context is the repo root), so tests and `jest.config.js` ARE inside the builder stage. `docker build --target builder -t backend-test -f backend/Dockerfile .` then `docker run --rm --network "$NET" -v /tmp/ci-secrets:/run/secrets:ro -v $PWD/config/app/ci.yml:/app/config/ci.yml:ro -e CONFIG_PATH=/app/config/ci.yml backend-test npx jest --forceExit`. + +Also disposition the ts-jest compile failures and 2 failing tests from Phase 0 (fix or quarantine-with-issue, per suite). + +Gate 1B: on the runner (manual run or a draft workflow), backend unit suites + integration suites complete against the ephemeral DB with a recorded `Tests:` line matching the post-fix local run, AND `docker exec mvp-postgres-staging psql -U postgres -d motovaultpro -c "\dt" ` before/after shows an unchanged table count (proof the shared staging DB was untouched). + +### 1C. OCR pytest in CI + +Tests are copied into the image (`ocr/Dockerfile` `COPY . .`, no `ocr/.dockerignore` exists) and pytest is installed. Candidate command (UNVERIFIED end-to-end - `app.main` may import things that need config at load): + +```bash +docker run --rm --entrypoint python "$OCR_IMAGE" -m pytest tests -q +``` + +EXPECTED shape: `N passed` with N near 310. IF import errors on config/WIF paths: capture the traceback in the phase issue and add the minimal env/mount it names; do not skip the suite silently. + +### 1D. Frontend tests in CI + +`frontend/.dockerignore` excludes `*.test.ts` / `*.test.tsx` (verified), so image stages cannot run tests today. Options: +- Host node 20 (recommended, simplest): `cd frontend && npm ci && npx jest --reporters=default` (flag unnecessary after 1A fix). +- Remove the test exclusions from `frontend/.dockerignore` and run in the `build` stage. Safe for image size: the production stage copies only `dist/` from the build stage. + +Gate for Phase 1 overall: each of the three suites runs headless on the `stage` runner with a recorded pass count matching the post-fix local baseline. Paste all three `Tests:`/`passed` lines into the phase issue. + +--- + +## Phase 2 - Add the CI quality job to staging.yaml + +Owner sign-off required (pipeline change). Add a `test` job to `.gitea/workflows/staging.yaml` that runs, per workspace: lint (`npm run lint` / eslint), type-check (`npm run type-check`), unit tests, plus OCR pytest, using the Phase 1 mechanics. Backend integration tests use the ephemeral DB from 1B. + +Wiring decision - blocking (recommended) vs report-only: + +| | Blocking (`deploy-staging: needs: [build, test]`) | Report-only (parallel job, nothing needs it) | +|---|---|---| +| Broken code reaches staging | No | Yes (staging is the de facto QA env - weakens the whole campaign) | +| PR feedback speed | Deploy waits for tests (~minutes) | Unchanged | +| Flaky test risk | Blocks deploys until fixed/quarantined | Ignored red jobs rot within weeks | +| Recommendation | YES - staging deploy is the thing being protected | Only as a <=2-week transition while stabilizing suites | + +Canary (the measurable gate - do not skip): on a branch, commit a deliberately failing test, e.g. append to any backend unit file: + +```ts +it('CI canary - must fail', () => { expect(1).toBe(2); }); +``` + +Open a draft PR. EXPECTED: the workflow run goes red; with blocking wiring, `deploy-staging` shows as skipped and staging still serves the previous build (confirm via `curl -s https://staging.motovaultpro.com/api/health | jq -r .status` returning `healthy` from the OLD deploy). Then revert the canary commit and observe green. Record both run URLs in the phase issue. + +IF the failing test does NOT turn the run red: the test step's exit code is being swallowed (look for `|| true`, piping to `tail`, or `set +e`); fix before merging anything else. + +--- + +## Phase 3 - Kill the :latest hazard + +Owner sign-off required (pipeline change). Options ranked; (c) both is the recommended end state: + +(a) RECOMMENDED: PR builds tag `:pr-N` + short-sha only; `:latest` moves only on push to main. In `staging.yaml`, split the latest-tagging into conditional steps. Exact condition shape (Gitea Actions): + +```yaml + - name: Tag and push latest (main only) + if: gitea.event_name == 'push' + run: | + docker tag ${{ steps.tags.outputs.backend_image }} $REGISTRY/egullickson/backend:latest + docker push $REGISTRY/egullickson/backend:latest + # (same for frontend, ocr) +``` + +Keep `--cache-from ...:latest` unconditionally (harmless read). If `gitea.event_name` evaluates empty on this act_runner version, use `github.event_name` - same context, aliased. Verify with a probe run before trusting it (print it in a step). +Obligation: the registry purge script (`scripts/ci/purge-container-images.sh`) keeps `latest` by digest, but it is currently run MANUALLY - no purge schedule exists anywhere in the repo (the only scheduled workflow is the image MIRROR in `mirror-images.yaml`, and the runner's daily `docker system prune` cron cleans local images, not the registry - see `mvp-run-and-operate`). After this change PR-sha tags accumulate registry-side, so scheduling or periodically running the purge becomes a required follow-up: file it as its own issue in the same phase. + +(b) SMALL + IMMEDIATE: make `production.yaml` `image_tag` a required input with no default. Change lines 14-17 to `required: true` and delete the `default: 'latest'` line; update `run-name` (line 9). Whether Gitea's dispatch UI actually blocks an empty required input is UNVERIFIED - so ALSO add a shell guard in the `validate` job: + +```yaml + - name: Refuse latest/empty tag + run: | + TAG="${{ inputs.image_tag }}" + if [ -z "$TAG" ] || [ "$TAG" = "latest" ]; then + echo "ERROR: deploy by explicit sha tag, never latest"; exit 1 + fi +``` + +(c) Both (a) and (b). Do (b) first - it is one small PR and removes the worst outcome immediately. + +Measurable gate: PR syncs no longer move `:latest`. Digest check, modeled on the verified helper in `scripts/ci/purge-container-images.sh:69-101`, run on the runner or anywhere with a registry PAT: + +```bash +REG=https://git.motovaultpro.com +T=$(curl -fsS -u "egullickson:$PAT" "$REG/v2/token?service=container_registry&scope=repository:egullickson/backend:pull" | jq -r '.token // .access_token') +curl -fsSI -H "Authorization: Bearer $T" \ + -H "Accept: application/vnd.oci.image.manifest.v1+json, application/vnd.oci.image.index.v1+json, application/vnd.docker.distribution.manifest.v2+json" \ + "$REG/v2/egullickson/backend/manifests/latest" | tr -d '\r' | grep -i docker-content-digest +``` + +Record the digest, push a trivial commit to the campaign PR (a PR sync), re-run. EXPECTED: identical digest. Then merge to main and re-run. EXPECTED: digest changes. Both observations go in the PR body. + +--- + +## Phase 4 - Staging deploy policy (owner decision gate) + +Today every PR sync fully redeploys shared staging (`down --timeout 30` then `up -d`, brief total outage each time; last PR wins). Options, honestly: + +| Option | Pro | Con | +|---|---|---| +| 1. Keep PR-deploys (status quo) | Matches single-dev reality: the PR deploy IS the end-to-end verification loop (there is no working local full-stack). Zero work. | Two open PRs fight over staging; staging never reflects main; every sync = outage + 3 image builds on a 29G-disk runner. | +| 2. Deploy staging only on push to main; PRs build+test only | Staging always = main; PR churn stops hammering the box. | Removes the ONLY pre-merge end-to-end check. Under current practice this is a big loss - do not pick this until Phase 2 tests are strong. | +| 3. Labeled opt-in (`if: contains(gitea.event.pull_request.labels.*.name, 'deploy-staging')`) | Deliberate deploys; test/build still run on every sync. | Label-condition behavior on Gitea act_runner is UNVERIFIED; adds a manual step to the AI-session workflow contract. | + +Recommendation: stay on option 1 until Phase 2 has been green for 2+ weeks, then present 3 (with a verified label-condition probe) to the owner. Whatever is decided, record it and the reasoning as an issue comment; the gate is the owner's explicit sign-off comment, not this file. + +--- + +## Phase 5 - Security and viewport scanning (CANDIDATES, unproven) + +Everything here is an open candidate. Do not represent any of it as existing - root `CLAUDE.md`'s claim that PRs get "Mobile/desktop viewport validation" and "Security scanning" is FALSE today (verified: no such steps in any workflow, no cypress config exists, the single `frontend/cypress/e2e/stations.cy.ts` spec is unrunnable). + +- gitleaks and/or `npm audit --omit=dev` as NON-blocking report steps first; promote to blocking only after 2 weeks of observed signal-to-noise, via a new owner-approved PR. +- Viewport smoke: root `package.json` carries an unused `@playwright/test` dependency and no config. First concrete step: a post-`verify-staging` job running a 3-URL Playwright smoke against `https://staging.motovaultpro.com` at 390x844 and 1920x1080, asserting page title and no console errors. That is the whole first milestone - not "mobile+desktop CI validation". Do NOT gate on the Cypress spec. +- When (if) either lands and blocks, update root `CLAUDE.md`'s CI/CD Pipeline section to match reality (see `mvp-docs-and-writing`). + +--- + +## Wrong paths - fenced off + +| Do not | Why (verified) | +|---|---| +| Point integration tests at the shared dev/staging DB | They DROP TABLE CASCADE in `afterAll`; the runner is the staging host, so "localhost postgres" IS staging's neighbor. Owner non-negotiable. | +| Resurrect the old Docker cleanup cron | It destroyed volumes; `ansible/deploy-staging-runner.yml:315-325` exists specifically to REMOVE it. The sanctioned prune cron on the runner is deliberately out-of-repo. | +| Use `scripts/rollback.sh` | Legacy: composes without the blue-green file, rebuilds from source, checks container names that do not exist on prod. Rollback = `scripts/ci/switch-traffic.sh` / `auto-rollback.sh` (see `mvp-run-and-operate`). | +| "Fix" runner disk by pruning volumes (`docker system prune --volumes`, `make clean`) | `make clean` = `docker compose down -v --rmi all` (Makefile:73-76) - destroys DB volumes wherever run. Prune images only. | +| Gate CI on the Cypress spec | No cypress config, no cypress dependency, undefined `cy.login()`, nonexistent `.or()` chainer. It cannot run. | +| Invent root npm scripts (`npm test` at repo root) | Root `package.json` has NO scripts and no workspaces (verified). If a root fan-out script is wanted, that is its own deliberate PR, not an assumption. | +| Trust `npm run type-check` to mean "tests compile" (backend) | `tsconfig.json:28` excludes `**/*.test.ts`; ts-jest checks them anyway. Verified divergence: type-check green, 7 suites fail to compile (see Phase 0.3 split). | + +## Validation and promotion protocol + +Per phase: +1. One Gitea issue per phase (`type/chore`), titled `chore: deploy-safety phase N - {name}`. Baseline numbers and decisions (with owner sign-off comments where required) live in issue comments. +2. The PR body contains the phase's measured gate evidence verbatim: command + output (grep counts, `Tests:` lines, digests, canary run URLs). A gate that is not pasted into the PR body did not happen. +3. Merge only after the staging pipeline for the PR itself is green and the owner has reviewed. Label flow per `mvp-change-control`. + +Finished state - a future session re-runs these assertions to confirm the campaign held: + +```bash +# 1. CI runs tests and lint (expect >=1 hit each): +grep -rnE "jest|pytest" .gitea/workflows/staging.yaml | wc -l # EXPECT: >= 1 +grep -rnE "run lint|eslint" .gitea/workflows/staging.yaml | wc -l # EXPECT: >= 1 +# 2. Tests block the deploy: +grep -n "needs:" .gitea/workflows/staging.yaml # EXPECT: deploy-staging needs the test job +# 3. :latest is not pushed unconditionally: +grep -n "push.*:latest" .gitea/workflows/staging.yaml # EXPECT: only inside an if: event_name guarded step +# 4. Prod cannot default to latest: +grep -n "default: 'latest'" .gitea/workflows/production.yaml # EXPECT: no output, exit 1 +grep -n "required: true" .gitea/workflows/production.yaml # EXPECT: 1 hit under image_tag +# 5. Behavior checks (registry + a canary PR, quarterly): +# - PR sync leaves the latest digest unchanged (Phase 3 curl check) +# - a failing test on a branch turns the PR run red (Phase 2 canary) +``` + +Plus the standing numbers: last recorded green counts for backend/frontend/OCR suites in the most recent phase issue. If a re-run's counts drop below the recorded ones without a filed quarantine issue, the campaign has regressed - file an issue immediately. + +## Provenance and maintenance + +Authored 2026-07-07 from direct inspection of the repo (all line numbers, counts, and command outputs verified on that date; test counts measured on a dev machine with node v25). Where any doc contradicts these observations, the code/workflows win. + +Volatile facts and one-line re-verification commands: + +| Fact (2026-07-07) | Re-verify with | +|---|---| +| Zero test/lint steps in workflows | `grep -rnE "jest|pytest|eslint|run lint" .gitea/workflows/` (exit 1 = still true) | +| staging.yaml pushes 3 `:latest` tags on every run; PR trigger present | `grep -cn ":latest" .gitea/workflows/staging.yaml` (9) and `sed -n '8,13p' .gitea/workflows/staging.yaml` | +| production.yaml `image_tag` defaults to `latest` | `grep -n -A4 "image_tag:" .gitea/workflows/production.yaml` | +| Backend unit baseline 25 suites (10 pass/15 fail), 149 tests (147 pass/2 fail) locally | `cd backend && npx jest --testPathIgnorePatterns "/node_modules/" "integration" --forceExit 2>&1 \| tail -5` | +| Frontend baseline 31 suites (17 pass/14 fail), 213 tests (196 pass/17 fail); reporter workaround works | `cd frontend && npx jest --reporters=default 2>&1 \| tail -5` | +| OCR: 310 test functions, pytest in requirements | `grep -rc "def test_" ocr/tests/*.py \| awk -F: '{s+=$2} END {print s}'` and `grep -n pytest ocr/requirements.txt` | +| `config/app/ci.yml` fails zod schema (missing `auth0`) | `cd backend && CONFIG_PATH=$PWD/../config/app/ci.yml SECRETS_DIR=/tmp npx jest src/features/stations/tests/unit/stations.service.test.ts --forceExit 2>&1 \| head -5` | +| Runner is host mode with node 20 | `grep -n "runner_labels\|node_20" ansible/deploy-staging-runner.yml` | +| `frontend/.dockerignore` excludes test files; no root `.dockerignore`; no `ocr/.dockerignore` | `ls .dockerignore ocr/.dockerignore 2>&1; grep -n "test" frontend/.dockerignore` | +| Integration tests DROP TABLE CASCADE | `grep -rn "DROP TABLE" backend/src/features/vehicles/tests/integration/vehicles.integration.test.ts` | +| Registry digest helper pattern | `sed -n '69,101p' scripts/ci/purge-container-images.sh` | + +UNVERIFIED items called out inline: OCR pytest inside the image end-to-end (1C), Gitea UI enforcement of required dispatch inputs (3b - shell guard compensates), label-conditioned deploys on act_runner (Phase 4 option 3), causes of the 17 frontend / 2 backend currently-failing tests (dispositioned in Phase 1). diff --git a/.claude/skills/mvp-diagnostics-and-logging/SKILL.md b/.claude/skills/mvp-diagnostics-and-logging/SKILL.md new file mode 100644 index 0000000..7b1ef09 --- /dev/null +++ b/.claude/skills/mvp-diagnostics-and-logging/SKILL.md @@ -0,0 +1,470 @@ +--- +name: mvp-diagnostics-and-logging +description: >- + Load when you need to observe or measure MotoVaultPro instead of guessing - querying + logs in Grafana/Loki, writing LogQL, tracing a request by requestId/X-Request-Id, + checking /health or /api/health, cache inspection via redis-cli, PostgreSQL activity, + or running the shipped diagnostic scripts (check-numeric-coercion.sh, + check-route-auth.sh, local-gate.sh). This is the HOW-to-observe toolbox - load it when + mvp-debugging-playbook (the symptom-triage hub) or a task needs measurements. + Distinguishing triggers - "logs not appearing", Grafana unreachable, Loki curl fails, + no logs from a container, alert firing, request timing, "slow requests", "5xx spike". +--- + +# MotoVaultPro Diagnostics and Logging + +Measuring instead of eyeballing: where logs live, how to query them, how to +check health and state, and three shipped read-only diagnostic scripts. + +## When to use / When NOT to use + +Use this skill when you need to OBSERVE the system: query logs, trace a +request, verify health, measure latency, inspect cache or database activity, +or run the pre-push diagnostic scripts. + +Do NOT use this skill for: +- Symptom-to-root-cause triage of known failure modes: `mvp-debugging-playbook` +- Deploying, rolling back, blue-green mechanics, backups: `mvp-run-and-operate` +- What counts as test evidence / definition of done: `mvp-validation-and-qa` +- Historical incidents and settled dead ends: `mvp-failure-archaeology` +- Getting a working local environment at all: `mvp-build-and-env` +- Changing log config or adding config axes: `mvp-config-and-secrets` + +Environment reality (2026-07-07): development is done by AI sessions in this +repo; end-to-end verification happens on STAGING via the PR pipeline. There +is no fully working local dev loop, and CI gates nothing beyond build + boot +(canonical statement: `mvp-validation-and-qa` section 1). The scripts in +this skill exist because of that gap. + +## 1. Logging topology + +One pipeline, file-provisioned end to end. Frontend is the exception: its +logs go to the browser console ONLY (`frontend/src/utils/logger.ts`) and +never reach Loki. + +``` + mvp-traefik mvp-frontend mvp-backend mvp-ocr mvp-postgres mvp-redis + | | | | | | + +------------+------+------+-----------+----------+------------+ + | (frontend: container + v stdout only; app logs + Docker json-file driver (max-size 10m, stay in the browser) + max-file 3 - set on every service in + docker-compose.yml) + | + v + mvp-alloy (config/alloy/config.alloy) + - discovers containers via /var/run/docker.sock + - labels: container = container name, service = compose service + | + v + mvp-loki (config/loki/config.yml) + - TSDB schema v13, filesystem storage + - retention_period: 720h (30 days) + - auth disabled; distroless image, healthcheck disabled + | + v + mvp-grafana (Grafana 12.4.0) + - https://logs.motovaultpro.com (prod) + - https://logs.staging.motovaultpro.com (staging) + - 4 provisioned dashboards + 5 provisioned alert rules +``` + +Grafana access: the Traefik router applies `grafana-ipwhitelist@file` +(`config/traefik/dynamic/grafana.yml` and `dynamic-staging/grafana.yml`), +which allows only RFC1918 source ranges (10/8, 172.16/12, 192.168/16). +Grafana is unreachable from the public internet by design - reach it from +inside the network (the servers themselves, or VPN). Admin password comes +from `GRAFANA_ADMIN_PASSWORD` (default `admin`). + +Container name matrix (label `container` in LogQL): + +| Service | Dev | Staging | Production | +|----------|----------------|------------------------|---------------------------------| +| backend | `mvp-backend` | `mvp-backend-staging` | `mvp-backend-blue` / `-green` | +| frontend | `mvp-frontend` | `mvp-frontend-staging` | `mvp-frontend-blue` / `-green` | +| traefik | `mvp-traefik` | `mvp-traefik-staging` | `mvp-traefik` | +| ocr | `mvp-ocr` | `mvp-ocr-staging` | `mvp-ocr` | +| postgres | `mvp-postgres` | `mvp-postgres-staging` | `mvp-postgres` | +| redis | `mvp-redis` | `mvp-redis-staging` | `mvp-redis` | +| loki/alloy/grafana | `mvp-loki` / `mvp-alloy` / `mvp-grafana` in ALL environments (not renamed by overlays) |||| + +KNOWN GAP (verified 2026-07-07): the provisioned dashboards and the +backend-related alert rules select `{container=~"mvp-backend(-staging)?"}`. +Loki regex matchers are fully anchored, so this does NOT match the +production blue-green names `mvp-backend-blue`/`-green`. On production, +backend panels are blind and the "Container Silence: mvp-backend" alert +(noDataState: Alerting) fires from no-data. Use +`{container=~"mvp-backend-(blue|green)"}` for prod queries. + +## 2. LogQL cookbook + +Field names below are verified against +`backend/src/core/plugins/logging.plugin.ts`: every completed request logs +`msg="Request processed"` with `requestId`, `method`, `path`, `status`, +`duration` (ms, integer), `ip`; Pino adds `level` and `time`. Substitute the +right container name from the matrix above. + +Request logs (the workhorse): + +```logql +{container="mvp-backend-staging"} | json | msg="Request processed" +``` + +Error sweep across all containers: + +```logql +{container=~"mvp-.*"} | json | level="error" +``` + +Errors per container over time: + +```logql +sum by (container) (count_over_time({container=~"mvp-.*"} | json | level="error" [5m])) +``` + +PostgreSQL errors (postgres logs are not JSON - use line filter): + +```logql +{container="mvp-postgres"} |~ "ERROR|FATAL|PANIC" +``` + +OCR (Python) errors: + +```logql +{container=~"mvp-ocr(-staging)?"} |~ "ERROR|Exception|Traceback" +``` + +Slow requests, p95 latency. The `| __error__=""` after `unwrap` is +REQUIRED - it drops lines where `duration` failed to parse; without it the +quantile returns errors or garbage: + +```logql +quantile_over_time(0.95, + {container="mvp-backend-staging"} | json | msg="Request processed" + | unwrap duration | __error__="" [5m]) +``` + +Individual requests slower than 500 ms: + +```logql +{container="mvp-backend-staging"} | json | msg="Request processed" | duration > 500 +``` + +All 5xx responses: + +```logql +{container="mvp-backend-staging"} | json | msg="Request processed" | status >= 500 +``` + +Correlate one request across services. The backend takes `X-Request-Id` +from the incoming request or generates a UUID; Traefik access logs (JSON) +keep the header. The backend does not echo it in the response, so find the +ID in the request log first, then sweep: + +```logql +{container=~"mvp-.*"} |= "550e8400-e29b-41d4-a716-446655440000" +``` + +To force a known ID end-to-end, send one (CORS-allowed header): + +```bash +curl -H "X-Request-Id: trace-$(date +%s)" https://staging.motovaultpro.com/api/health +``` + +### Provisioned alerts + +Defined in `config/grafana/alerting/alert-rules.yml`; evaluated every 1m, +must hold for 5m. Contact point is a webhook placeholder - alerts are +visible in the Grafana UI but notify nobody (2026-07-07). + +| Alert | Severity | Condition | +|-------|----------|-----------| +| Error Rate Spike | critical | error-level logs > 5% of all logs over 5m | +| Container Silence: mvp-backend / mvp-postgres / mvp-redis | warning | no logs for 5m (noDataState: Alerting) | +| 5xx Response Spike | critical | > 10 HTTP 5xx from backend in 5m | + +### Log-level control is deploy-time + +One `LOG_LEVEL` fans out to all containers via +`scripts/ci/generate-log-config.sh `, whose output +the CI workflows append to `.env` on the server (NOT `.env.logging` - +docs/LOGGING.md says `.env.logging`; the code in +`.gitea/workflows/staging.yaml` appends to `.env`, and code wins). It sets +`BACKEND_LOG_LEVEL`, `TRAEFIK_LOG_LEVEL`, `POSTGRES_LOG_STATEMENT`, +`POSTGRES_LOG_MIN_DURATION`, `REDIS_LOGLEVEL`. Staging runs DEBUG, prod +runs INFO. Caveat: the postgres leg of this fan-out is dead - the +`POSTGRES_LOG_*` variables are set on the container but never applied (see +the known gap under "PostgreSQL activity" in section 4); the backend, +traefik, and redis legs are wired. There is no runtime toggle: changing +verbosity means redeploying. +Never hand-edit `.env` on a server - the next deploy's rsync/regeneration +reverts it (owner non-negotiable). + +## 3. Health and state checks + +### /health vs /api/health + +Both defined in `backend/src/app.ts`, both unauthenticated, both return +`status: "healthy"` plus a `features` array (currently 20 entries in code). + +- `GET /health` (port 3001, in-container) - used by the Docker healthcheck + and CI's in-container curl. No `/api` prefix; not routed by Traefik. +- `GET /api/health` - routed through Traefik; adds `scope: "api"`. This is + the external verification target. + +The CI contract (`.gitea/workflows/staging.yaml`, REQUIRED_FEATURES) checks +that 13 specific features are present in the array: admin, auth, onboarding, +vehicles, documents, fuel-logs, stations, maintenance, platform, +notifications, user-profile, user-preferences, user-export. The endpoint +returns more than 13; the contract is a subset check. If you add a feature +capsule, adding it to the app.ts arrays is cosmetic; adding it to +REQUIRED_FEATURES makes it deploy-gating. + +```bash +curl -s https://staging.motovaultpro.com/api/health | jq '.status, (.features | length)' +``` + +### Container state per environment + +The `-f` stack matters - a bare `docker compose ps` on staging/prod uses +only the base file and shows wrong/partial state: + +```bash +# Dev (local machine) +docker compose ps +make health-check-all # ps table + Traefik service discovery counts + +# Staging (on mvp-staging, /opt/motovaultpro) +docker compose -f docker-compose.yml -f docker-compose.staging.yml ps + +# Production (on prod server, /opt/motovaultpro) +docker compose -f docker-compose.yml -f docker-compose.blue-green.yml -f docker-compose.prod.yml ps +``` + +### config/deployment/state.json - and why it lies mid-deploy + +On the prod server, `config/deployment/state.json` records +`active_stack` (blue/green), last deployment, and per-stack health. Read it +with `cat /opt/motovaultpro/config/deployment/state.json | jq .` + +It lies in two windows: +1. Every deploy rsyncs `config/` with `--delete`, overwriting state.json + with the repo default (`active_stack: blue`, all-null history). The + workflow reads the real state BEFORE the rsync and re-stamps it at the + end - but any inspection between rsync and the final stamp shows the + repo default, not reality. +2. Traefik weights in `config/traefik/dynamic/blue-green.yml` are the + actual routing truth; state.json is a record of intent. When in doubt, + read the weights file. + +### When the log pipeline itself is broken + +1. `docker logs mvp-alloy` (same name in every environment) - Alloy is the + collector; discovery or push errors appear here. +2. You CANNOT `curl` Loki from the host or exec into it: Loki 3.x is a + distroless image (no shell, no wget/curl inside; the compose healthcheck + is explicitly disabled for this reason - see the comment in + docker-compose.yml). Verify Loki through Grafana: Connections > + Data sources > Loki > Test, or from any container on the `backend` + network, e.g. + `docker exec mvp-backend-staging wget -qO- http://mvp-loki:3100/ready`. +3. `docker logs mvp-grafana` for provisioning errors (bad dashboard JSON or + alert YAML shows up here at startup). +4. Docker json-file logs still exist even if the pipeline is down: + `docker logs --since 10m mvp-backend-staging`. + +## 4. Measuring instead of eyeballing + +### Request timings + +Timings live in the `duration` field (ms) of `msg="Request processed"` +lines - see the quantile queries above. The API Performance dashboard +(p50/p95/p99, slowest endpoints by avg duration) is provisioned from +`config/grafana/dashboards/api-performance.json`. For a one-off check +without Grafana: + +```bash +docker logs --since 5m mvp-backend-staging 2>&1 | + grep '"msg":"Request processed"' | + awk -F'"duration":' '{split($2,a,","); gsub(/[^0-9]/,"",a[1]); print a[1]}' | sort -n | tail -5 +``` + +### Cache behavior (Redis) + +Backend uses DB 0 with key prefix `mvp:` (`backend/src/core/config/redis.ts`; +DB from `config/app/*.yml` `redis.db: 0`). The OCR service uses DB 1 +(`REDIS_DB: 1` in docker-compose.yml) for job state. Locks use `mvp:lock:`. + +```bash +# Staging names shown; drop -staging for dev/prod +docker exec mvp-redis-staging redis-cli -n 0 --scan --pattern 'mvp:*' | head -50 +docker exec mvp-redis-staging redis-cli -n 0 TTL 'mvp:some-key' +docker exec mvp-redis-staging redis-cli -n 1 --scan --pattern '*' | head # OCR jobs +docker exec mvp-redis-staging redis-cli INFO keyspace +``` + +Cache reads swallow errors and return null (cache failure never breaks a +request), so a dead Redis looks like a 100% miss rate, not errors. Check +`docker logs mvp-redis-staging` and hit rates via `INFO stats`. + +### PostgreSQL activity + +Dev shell: `make db-shell-app` (wraps +`docker compose exec mvp-postgres psql -U postgres -d motovaultpro`). +Staging/prod equivalent: + +```bash +docker exec -it mvp-postgres-staging psql -U postgres -d motovaultpro +``` + +Useful read-only checks inside psql: + +```sql +SELECT pid, state, now() - query_start AS age, left(query, 80) + FROM pg_stat_activity WHERE state <> 'idle' ORDER BY age DESC; +SELECT relname, n_live_tup FROM pg_stat_user_tables ORDER BY n_live_tup DESC LIMIT 15; +``` + +KNOWN GAP: the `POSTGRES_LOG_STATEMENT` / `POSTGRES_LOG_MIN_DURATION_STATEMENT` +values set in compose (`docker-compose.yml:245,247`) are INERT - they are plain +environment variables that the official postgres image does not read, and +nothing in the repo applies them (no `command: postgres -c log_statement=...`, +no custom postgresql.conf, no initdb script). Postgres runs with defaults +(`log_statement=none`, `log_min_duration_statement=-1`) at every LOG_LEVEL, so +per-statement and `duration:` lines never reach Loki; a +`{container="mvp-postgres-staging"} |~ "duration:"` hunt finds nothing. Until +compose passes the values via `command: postgres -c ...`, slow-query hunting +must use `pg_stat_activity` (above) instead. + +### End-to-end request trace, step by step + +1. Send the request with a known `X-Request-Id` (see cookbook above). +2. Traefik: `{container="mvp-traefik-staging"} | json |= ""` - confirms + arrival, router matched, status returned to client. +3. Backend: `{container="mvp-backend-staging"} |= ""` - all application + log lines plus the final `Request processed` line with duration. +4. If OCR involved: `{container="mvp-ocr-staging"} |= ""`. +5. Postgres/Redis logs are not request-tagged; correlate by timestamp. + +## 5. Shipped diagnostic scripts + +All in `.claude/skills/mvp-diagnostics-and-logging/scripts/`, all +executable, all safe read-only checks (no network, no writes outside temp +logs), portable bash (macOS bash 3.2). Run them from anywhere inside the +repo. Each was run against the repo on 2026-07-07. + +### check-numeric-coercion.sh - the #1 recurring bug class + +node-postgres returns NUMERIC/DECIMAL columns as STRINGS. +`backend/src/core/config/database.ts` overrides only the DATE parser, so +every repository mapper must coerce manually with `Number()`/`parseFloat()`. +Missed coercion caused issues #239, #241, #244 (symptoms: MPG shows NaN, +string concatenation instead of addition, sort by cost is alphabetical). + +```bash +./.claude/skills/mvp-diagnostics-and-logging/scripts/check-numeric-coercion.sh +``` + +What it does: extracts every NUMERIC/DECIMAL column name from all migration +SQL (column defs, ADD COLUMN, ALTER COLUMN TYPE), then (a) flags any +`row.` read in a `*.repository.ts` file whose line lacks +`Number(`/`parseFloat(`, and (b) flags raw-row returns +(`return res.rows...`) in features whose migrations define numeric columns - +the exact shape of bug #244, which bypassed the mapper entirely. + +Interpretation: exit 0 = clean. Exit 1 = each SUSPECT line needs coercion +(`row.x != null ? parseFloat(row.x) : null` is the house pattern) - or is a +false positive if coercion happens on another line or in the service layer; +read before fixing. Run after ANY repository or migration change that +touches numeric columns. + +### check-route-auth.sh - unlisted-public routes + +There is no global auth hook; every route opts in via +`preHandler: [fastify.authenticate]` (or `requireAdmin`/`requireTier`, or +the common alias `const requireAuth = fastify.authenticate.bind(fastify)`). +A forgotten preHandler ships a public endpoint silently - pre-launch, this +is a RULE 0 severity gap. + +```bash +./.claude/skills/mvp-diagnostics-and-logging/scripts/check-route-auth.sh +``` + +What it does: parses every registration block in +`backend/src/features/*/api/*.routes.ts` (199 blocks as of 2026-07-07) and +prints any block containing no recognized guard. Known-intentional public +routes are allowlisted in the script and printed as OK: `/webhooks/stripe`, +`/webhooks/resend/inbound` (provider signature auth), `/auth/signup`, +`/auth/resend-verification-public` (pre-auth flows). + +Interpretation: exit 0 = only allowlisted routes are public AMONG the +`features/*/api` route files the script scans. Routes registered in core +code are OUT OF SCOPE and pass silently - including the intentionally +public `GET /api/config/feature-tiers` (`backend/src/core/config/config.routes.ts`, +no preHandler) and the `app.ts` registrations (`/health`, `/api/health`, +`/auth/verify`); the script's header comment carries the canonical +intentional-public list covering both scopes. A future unguarded route +added outside `features/*/api/` would also pass silently - exit 0 is not a +whole-backend guarantee. Exit 1 = an UNGUARDED route: either add a guard +or, if genuinely public, add it to KNOWN_PUBLIC in the script with a +justification comment. Heuristic caveats: +a guard aliased under a new name reads as UNGUARDED (safe direction); a +comment containing a guard word inside a block could mask a gap (verify by +reading the code). Run after adding or reshaping any route file. + +### local-gate.sh - the pre-push gate CI does not provide + +CI runs zero tests and zero lint. This script is the substitute: + +```bash +./.claude/skills/mvp-diagnostics-and-logging/scripts/local-gate.sh +``` + +Runs, with a PASS/FAIL table and per-step logs in `$TMPDIR`: +backend lint, backend type-check, backend UNIT tests +(`--testPathPattern='(tests/unit|src/core)' --testPathIgnorePatterns=integration --forceExit`), +frontend lint, frontend type-check. Requires `npm install` in `backend/` +and `frontend/` first. + +Deliberate exclusions: backend integration tests need a live database and +some DROP TABLE CASCADE (owner non-negotiable: no destructive DB operations +without a fresh backup - never point them at a shared DB casually); the +audit-log `__tests__` also require a live database and are excluded by +pattern; frontend jest is broken outside the container (2026-07-07). +`--forceExit` is required because open pg/redis handles otherwise hang jest. + +Interpretation: all PASS = safe to push (integration behavior still +unverified until staging). Any FAIL = fix first; CI will not catch it. +KNOWN STATE (2026-07-07): backend unit tests FAIL on main - 12 of 22 suites +have pre-existing ts-jest compile errors (stale test mocks vs current +types, e.g. `new AuthService(...)` missing the termsData argument) plus 2 +failing assertions. This is untriaged rot from CI running nothing; treat a +FAIL here as "no worse than main" only after diffing against a main-branch +run, and see `mvp-validation-and-qa` for the evidence bar. + +## Provenance and maintenance + +Authored 2026-07-07 by direct inspection of the repo (all commands, paths, +field names, and line-anchored claims verified against code; where +docs/LOGGING.md disagrees with code - `.env.logging` vs `.env` - code wins). + +Volatile facts and re-verification commands: + +| Fact (as of 2026-07-07) | Re-verify with | +|---|---| +| Request log fields (requestId/method/path/status/duration/ip) | `grep -A8 "Request processed" backend/src/core/plugins/logging.plugin.ts` | +| Loki 30-day retention, TSDB v13 | `grep -E "retention_period\|schema:" config/loki/config.yml` | +| Alloy labels container/service | `grep target_label config/alloy/config.alloy` | +| json-file 10m x 3 on all services | `grep -c "max-size" docker-compose.yml` | +| Grafana RFC1918 whitelist | `cat config/traefik/dynamic/grafana.yml config/traefik/dynamic-staging/grafana.yml` | +| Alert rules (5%/5m, silence, 5xx>10) | `grep -E "title\|for:\|noDataState" config/grafana/alerting/alert-rules.yml` | +| Dashboard/alert regex missing prod blue-green | `grep -c "mvp-backend(-staging)?" config/grafana/dashboards/*.json config/grafana/alerting/alert-rules.yml` vs `grep container_name docker-compose.blue-green.yml` | +| 13-feature CI health contract | `grep REQUIRED_FEATURES .gitea/workflows/staging.yaml` | +| /health features array (20 entries) | `grep -n "features:" backend/src/app.ts` (read the two arrays) | +| Log config appended to .env | `grep -n "generate-log-config" .gitea/workflows/*.yaml` | +| Redis DB 0 backend / DB 1 OCR, prefix mvp: | `grep "db:" config/app/*.yml; grep REDIS_DB docker-compose.yml; grep "prefix = " backend/src/core/config/redis.ts` | +| Known-public routes allowlist | `./.claude/skills/mvp-diagnostics-and-logging/scripts/check-route-auth.sh` | +| Numeric coercion clean state | `./.claude/skills/mvp-diagnostics-and-logging/scripts/check-numeric-coercion.sh` | +| Backend unit tests failing on main | `cd backend && npm test -- --testPathPattern='tests/unit' --forceExit` | +| Container names per env | `grep container_name docker-compose*.yml` | diff --git a/.claude/skills/mvp-diagnostics-and-logging/scripts/check-numeric-coercion.sh b/.claude/skills/mvp-diagnostics-and-logging/scripts/check-numeric-coercion.sh new file mode 100755 index 0000000..88b84a3 --- /dev/null +++ b/.claude/skills/mvp-diagnostics-and-logging/scripts/check-numeric-coercion.sh @@ -0,0 +1,77 @@ +#!/bin/bash +# check-numeric-coercion.sh - Find NUMERIC/DECIMAL columns returned from +# repositories without Number()/parseFloat() coercion. +# +# Why: node-postgres returns NUMERIC/DECIMAL (OID 1700) as *strings*. +# backend/src/core/config/database.ts overrides only the DATE parser, so +# every repository mapper must coerce manually. Missed coercion is this +# project's #1 recurring bug class (issues #239, #241, #244). +# +# Usage: ./check-numeric-coercion.sh (read-only; run from anywhere in repo) +# Exit: 0 = no suspects, 1 = suspects found, 2 = setup error +set -u + +ROOT="$(git rev-parse --show-toplevel 2>/dev/null)" +[ -z "$ROOT" ] && { echo "ERROR: not inside a git repo" >&2; exit 2; } +SRC="$ROOT/backend/src" +[ -d "$SRC" ] && cd "$ROOT" || { echo "ERROR: $SRC missing" >&2; exit 2; } + +# 1. Collect numeric/decimal column names from all migration SQL. +# Three shapes: column definition lines, ADD COLUMN, ALTER COLUMN ... TYPE. +COLS=$( + { + grep -rhiE '^[[:space:]]*"?[a-z_]+"?[[:space:]]+(numeric|decimal)[[:space:](]' \ + --include='*.sql' "$SRC" | awk '{gsub(/"/,""); print tolower($1)}' + grep -rhioE 'add column (if not exists )?"?[a-z_]+"? +(numeric|decimal)' \ + --include='*.sql' "$SRC" | awk '{gsub(/"/,""); print tolower($(NF-1))}' + grep -rhioE 'alter column "?[a-z_]+"? type +(numeric|decimal)' \ + --include='*.sql' "$SRC" | awk '{gsub(/"/,""); print tolower($(NF-2))}' + } | sort -u +) +[ -z "$COLS" ] && { echo "ERROR: no numeric columns found (wrong dir?)" >&2; exit 2; } + +echo "Numeric/decimal columns found in migrations:" +echo "$COLS" | tr '\n' ' '; echo; echo + +# 2. In every repository file, flag lines that read row. without +# Number( or parseFloat( on the same line. +SUSPECTS=0 +for col in $COLS; do + HITS=$(find "$SRC" -name '*.repository.ts' -exec \ + grep -nE "row\.${col}([^a-zA-Z0-9_]|\$)" {} + 2>/dev/null | + grep -v 'parseFloat(' | grep -v 'Number(') + if [ -n "$HITS" ]; then + echo "SUSPECT column '$col' (string from pg, no coercion on line):" + echo "$HITS" | sed "s|$ROOT/||; s/^/ /" + SUSPECTS=$((SUSPECTS + $(echo "$HITS" | wc -l | tr -d ' '))) + fi +done + +# 3. Flag raw-row returns (the actual shape of bug #244: "return res.rows[0]" +# bypasses the mapper entirely) in features whose migrations define +# numeric columns. Scalar returns (.count/.exists/.length) are filtered. +for repo in $(find "$SRC/features" -name '*.repository.ts'); do + feat=$(echo "$repo" | sed "s|$SRC/features/||; s|/.*||") + featcols=$(grep -rhiE "(numeric|decimal)" --include='*.sql' \ + "$SRC/features/$feat/migrations" 2>/dev/null | head -1) + [ -z "$featcols" ] && continue + RAW=$(grep -nE 'return (res|result)\.rows' "$repo" | + grep -vE 'map[A-Za-z]*\(|\.map\(|\?\.|\.length|\.count|\.exists') + if [ -n "$RAW" ]; then + echo "SUSPECT raw-row return in numeric-bearing feature '$feat':" + echo "$RAW" | sed "s|^| ${repo#"$ROOT"/}:|" + SUSPECTS=$((SUSPECTS + $(echo "$RAW" | wc -l | tr -d ' '))) + fi +done + +echo +if [ "$SUSPECTS" -eq 0 ]; then + echo "PASS: no uncoerced numeric column reads or raw-row returns found." + exit 0 +else + echo "FAIL: $SUSPECTS suspect line(s). Each must wrap the value in Number()" + echo "or parseFloat() (nullable: 'row.x != null ? parseFloat(row.x) : null')." + echo "Limitations: same-line heuristic; coercion done on a different line or" + echo "in the service layer will still be flagged - verify before fixing." + exit 1 +fi diff --git a/.claude/skills/mvp-diagnostics-and-logging/scripts/check-route-auth.sh b/.claude/skills/mvp-diagnostics-and-logging/scripts/check-route-auth.sh new file mode 100755 index 0000000..395e1b4 --- /dev/null +++ b/.claude/skills/mvp-diagnostics-and-logging/scripts/check-route-auth.sh @@ -0,0 +1,90 @@ +#!/bin/bash +# check-route-auth.sh - List backend routes with NO auth guard in their +# registration block, so unintentionally-public endpoints are visible. +# +# Why: there is no global auth hook. Every route must opt in via +# preHandler: [fastify.authenticate] / requireAdmin / requireTier +# (backend/src/core/plugins/*.plugin.ts). A forgotten preHandler ships a +# public endpoint silently. +# +# Usage: ./check-route-auth.sh (read-only; run from anywhere in repo) +# Exit: 0 = only known-public routes unguarded, 1 = unexpected unguarded route +set -u + +ROOT="$(git rev-parse --show-toplevel 2>/dev/null)" +[ -z "$ROOT" ] && { echo "ERROR: not inside a git repo" >&2; exit 2; } +cd "$ROOT" || exit 2 + +# CANONICAL intentional-public-route list (verified 2026-07-09). This comment +# is the ONE home for it; mvp-architecture-contract invariant 2.1 and +# mvp-debugging-playbook section 7 point here instead of keeping their own lists. +# +# Within this script's scan scope (backend/src/features/*/api/*.routes.ts), +# allowlisted via KNOWN_PUBLIC below: +# /webhooks/stripe, /webhooks/resend/inbound - provider signature auth +# /auth/signup, /auth/resend-verification-public - pre-auth flows +# (comments in auth.routes.ts: "public, no JWT required") +# +# OUTSIDE this script's scan scope (registered in core code, also intentionally +# public - the script can neither flag nor clear them): +# /health, /api/health - backend/src/app.ts health endpoints +# /auth/verify - backend/src/app.ts (Traefik forward-auth) +# GET /api/config/feature-tiers - backend/src/core/config/config.routes.ts +# (no preHandler; config is not sensitive) +KNOWN_PUBLIC='/webhooks/stripe /webhooks/resend/inbound /auth/signup /auth/resend-verification-public' + +OUT=$(awk ' + function flush() { + if (inblock && !guarded) printf "%s:%d %s %s\n", f, startline, method, path + inblock = 0 + } + FNR == 1 { flush() } + /fastify\.(get|post|put|patch|delete)[<(]/ { + flush() + inblock = 1; guarded = 0; startline = FNR; path = "?"; f = FILENAME + match($0, /fastify\.(get|post|put|patch|delete)/) + method = toupper(substr($0, RSTART + 8, RLENGTH - 8)) + } + inblock && path == "?" { + if (match($0, "\047/[^\047]*\047")) path = substr($0, RSTART + 1, RLENGTH - 2) + } + # requireAuth is the common local alias: const requireAuth = fastify.authenticate.bind(fastify) + inblock && /[ .\[](authenticate|requireAuth|requireAdmin|requireTier)[ ,\]\}\)\(]/ { guarded = 1 } + END { flush() } +' backend/src/features/*/api/*.routes.ts) + +TOTAL=$(grep -c 'fastify\.\(get\|post\|put\|patch\|delete\)[<(]' \ + backend/src/features/*/api/*.routes.ts | awk -F: '{s+=$2} END {print s}') + +echo "Route registrations scanned: $TOTAL" +echo + +FAIL=0 +if [ -z "$OUT" ]; then + echo "PASS: every route block contains an auth guard." + exit 0 +fi + +echo "Routes with NO authenticate/requireAdmin/requireTier in their block:" +echo "$OUT" | while read -r line; do + p=$(echo "$line" | awk '{print $3}') + case " $KNOWN_PUBLIC " in + *" $p "*) echo " OK (known-public, allowlisted): $line" ;; + *) echo " UNGUARDED: $line" ;; + esac +done +# Re-count outside the pipe subshell (bash 3.2: while-in-pipe loses vars) +FAIL=$(echo "$OUT" | awk -v kp=" $KNOWN_PUBLIC " '{ if (index(kp, " " $3 " ") == 0) n++ } END {print n+0}') + +echo +if [ "$FAIL" -eq 0 ]; then + echo "PASS: all unguarded routes are on the known-public allowlist." + exit 0 +else + echo "FAIL: $FAIL route(s) unguarded and not on the allowlist." + echo "Either add a preHandler guard or, if intentionally public, add the" + echo "path to KNOWN_PUBLIC in this script with a justification comment." + echo "Limitation: block-scope heuristic; a guard mentioned in a comment" + echo "inside the block can mask a real gap - verify hits by reading code." + exit 1 +fi diff --git a/.claude/skills/mvp-diagnostics-and-logging/scripts/local-gate.sh b/.claude/skills/mvp-diagnostics-and-logging/scripts/local-gate.sh new file mode 100755 index 0000000..7cc23b5 --- /dev/null +++ b/.claude/skills/mvp-diagnostics-and-logging/scripts/local-gate.sh @@ -0,0 +1,65 @@ +#!/bin/bash +# local-gate.sh - Run every quality check that works on a dev machine and +# print a PASS/FAIL table. +# +# Why: CI runs ZERO tests and ZERO lint (the only PR gate is that images +# build and staging boots healthy). This script IS the pre-push gate. +# +# Scope (deliberate): +# - backend lint, type-check, UNIT tests only (--forceExit; open pg/redis +# handles otherwise hang jest). Integration tests are EXCLUDED: they +# need a live database and some DROP TABLE CASCADE (owner non-negotiable: +# never without a fresh backup). audit-log __tests__ also need a DB and +# are excluded via path pattern. +# - frontend lint, type-check. Frontend jest is broken outside the +# container (2026-07-07), so no frontend tests here. +# +# Usage: ./local-gate.sh (needs node_modules in backend/ and frontend/: +# run "npm install" in each first) +# Exit: 0 = all pass, 1 = at least one failure +set -u + +ROOT="$(git rev-parse --show-toplevel 2>/dev/null)" +[ -z "$ROOT" ] && { echo "ERROR: not inside a git repo" >&2; exit 2; } +LOGDIR="${TMPDIR:-/tmp}/mvp-local-gate.$$" +mkdir -p "$LOGDIR" + +RESULTS="" +OVERALL=0 + +run_step() { # run_step