frappe-agent-validator · diff
v2.0 to v2.0
221 added, 258 removed. Audit A to A.
---
name: frappe-agent-validator
description: >
- Use when reviewing or validating ERPNext/Frappe code against best practices
- and common pitfalls. Checks generated code before deployment, validates
- code quality, and catches errors. Keywords: review code, check script,
- validate deployment, find bugs, check errors, code quality.
+ Use when reviewing or validating Frappe/ERPNext code against best
+ practices and common pitfalls. Checks generated code before deployment,
+ validates against all 53 frappe-* skills, catches v16 patterns
+ (extend_doctype_class, type annotations), validates ops patterns (bench
+ commands, deployment), and generates correction reports. Keywords: review
+ code, check script, validate deployment, find bugs, code quality.
license: MIT
compatibility: "Claude Code, Claude.ai Projects, Claude API. Frappe v14-v16."
metadata:
author: OpenAEC-Foundation
version: "2.0"
---
- # ERPNext Code Validator Agent
+ # Frappe Code Validator Agent
- This agent validates ERPNext/Frappe code against established patterns, common pitfalls, and version compatibility requirements.
+ Validates Frappe/ERPNext code against the complete 53-skill knowledge base, catching errors BEFORE deployment.
- **Purpose**: Catch errors BEFORE deployment, not after
+ **Purpose**: Catch errors before deployment, not after
## When to Use This Agent
```
- ┌─────────────────────────────────────────────────────────────────────┐
- │ CODE VALIDATION TRIGGERS │
- ├─────────────────────────────────────────────────────────────────────┤
- │ │
- │ ► Code has been generated and needs review │
- │ "Check this Server Script before I save it" │
- │ └── USE THIS AGENT │
- │ │
- │ ► Code is causing errors │
- │ "Why isn't this working?" │
- │ └── USE THIS AGENT │
- │ │
- │ ► Pre-deployment validation │
- │ "Is this production-ready?" │
- │ └── USE THIS AGENT │
- │ │
- │ ► Code review for best practices │
- │ "Can this be improved?" │
- │ └── USE THIS AGENT │
- │ │
- └─────────────────────────────────────────────────────────────────────┘
+ CODE VALIDATION TRIGGERS
+ |
+ +-- Code has been generated and needs review
+ | "Check this Server Script before I save it"
+ | --> USE THIS AGENT
+ |
+ +-- Code is causing errors
+ | "Why isn't this working?"
+ | --> USE THIS AGENT
+ |
+ +-- Pre-deployment validation
+ | "Is this production-ready?"
+ | --> USE THIS AGENT
+ |
+ +-- Code review for best practices
+ | "Can this be improved?"
+ | --> USE THIS AGENT
+ |
+ +-- Ops/deployment validation
+ | "Is my bench setup correct?"
+ | --> USE THIS AGENT
```
## Validation Workflow
```
- ┌─────────────────────────────────────────────────────────────────────┐
- │ CODE VALIDATOR WORKFLOW │
- ├─────────────────────────────────────────────────────────────────────┤
- │ │
- │ STEP 1: IDENTIFY CODE TYPE │
- │ ══════════════════════════ │
- │ • Client Script (JavaScript) │
- │ • Server Script (Python sandbox) │
- │ • Controller (Python full) │
- │ • hooks.py configuration │
- │ • Jinja template │
- │ • Whitelisted method │
- │ │
- │ STEP 2: RUN TYPE-SPECIFIC CHECKS │
- │ ═════════════════════════════════ │
- │ • Apply checklist for identified code type │
- │ • Check syntax patterns │
- │ • Verify API usage │
- │ │
- │ STEP 3: CHECK UNIVERSAL RULES │
- │ ══════════════════════════════ │
- │ • Error handling present │
- │ • User feedback appropriate │
- │ • Security considerations │
- │ • Performance implications │
- │ │
- │ STEP 4: VERIFY VERSION COMPATIBILITY │
- │ ════════════════════════════════════ │
- │ • v14/v15/v16 specific features │
- │ • Deprecated patterns │
- │ • Version-specific behaviors │
- │ │
- │ STEP 5: GENERATE VALIDATION REPORT │
- │ ══════════════════════════════════ │
- │ • Critical errors (must fix) │
- │ • Warnings (should fix) │
- │ • Suggestions (nice to have) │
- │ • Corrected code (if errors found) │
- │ │
- └─────────────────────────────────────────────────────────────────────┘
- ```
-
- → See [references/workflow.md](references/workflow.md) for detailed validation steps.
+ STEP 1: IDENTIFY CODE TYPE
+ Client Script | Server Script | Controller | hooks.py |
+ Jinja | Whitelisted | Bench/Ops | DocType JSON
- ## Critical Checks by Code Type
+ STEP 2: RUN TYPE-SPECIFIC CHECKS
+ Apply checklist for identified code type
- ### Server Script Checks
+ STEP 3: CHECK UNIVERSAL RULES
+ Error handling | Security | Performance | User feedback
- ```
- ┌─────────────────────────────────────────────────────────────────────┐
- │ ⚠️ SERVER SCRIPT CRITICAL CHECKS │
- ├─────────────────────────────────────────────────────────────────────┤
- │ │
- │ [FATAL] Import statements │
- │ ═══════════════════════════ │
- │ ❌ import json → Use frappe.parse_json() │
- │ ❌ from frappe.utils import X → Use frappe.utils.X() │
- │ ❌ import requests → IMPOSSIBLE in Server Script │
- │ │
- │ [FATAL] Undefined variables │
- │ ════════════════════════════ │
- │ ❌ self.field → Use doc.field │
- │ ❌ document.field → Use doc.field │
- │ │
- │ [FATAL] Wrong event handling │
- │ ═══════════════════════════════ │
- │ ❌ try/except for validation → Just frappe.throw() │
- │ │
- │ [ERROR] Event name mismatch │
- │ ═══════════════════════════ │
- │ ❌ Event "Before Save" code in "After Save" script │
- │ │
- │ [WARNING] Missing validation │
- │ ═══════════════════════════════ │
- │ ⚠️ No null/empty checks before operations │
- │ │
- └─────────────────────────────────────────────────────────────────────┘
- ```
+ STEP 4: VERIFY VERSION COMPATIBILITY
+ v14/v15/v16 features | Deprecated patterns
- ### Client Script Checks
+ STEP 5: VALIDATE AGAINST SKILL CATALOG
+ Cross-reference with relevant frappe-* skills
- ```
- ┌─────────────────────────────────────────────────────────────────────┐
- │ CLIENT SCRIPT CRITICAL CHECKS │
- ├─────────────────────────────────────────────────────────────────────┤
- │ │
- │ [FATAL] Wrong API usage │
- │ ═════════════════════════ │
- │ ❌ frappe.db.get_value() → Server-side only! │
- │ ❌ frappe.get_doc() → Server-side only! │
- │ ✓ frappe.call() for server data │
- │ │
- │ [FATAL] Missing async handling │
- │ ══════════════════════════════ │
- │ ❌ let result = frappe.call() → Returns undefined │
- │ ✓ frappe.call({callback: fn}) → Use callback │
- │ ✓ await frappe.call({async:false}) → Or async/await │
- │ │
- │ [ERROR] Field refresh issues │
- │ ════════════════════════════ │
- │ ❌ frm.set_value() without refresh │
- │ ✓ frm.set_value() then frm.refresh_field() │
- │ │
- │ [WARNING] Form state checks │
- │ ═══════════════════════════ │
- │ ⚠️ Not checking frm.doc.__islocal for new docs │
- │ ⚠️ Not checking frm.doc.docstatus for submitted docs │
- │ │
- └─────────────────────────────────────────────────────────────────────┘
+ STEP 6: GENERATE VALIDATION REPORT
+ Critical errors | Warnings | Suggestions | Corrected code
```
- ### Controller Checks
+ See [references/workflow.md](references/workflow.md) for detailed steps.
- ```
- ┌─────────────────────────────────────────────────────────────────────┐
- │ CONTROLLER CRITICAL CHECKS │
- ├─────────────────────────────────────────────────────────────────────┤
- │ │
- │ [FATAL] Wrong lifecycle usage │
- │ ═════════════════════════════ │
- │ ❌ Modifying self.field in on_update → Changes NOT saved! │
- │ ✓ Use frappe.db.set_value() in on_update │
- │ │
- │ [FATAL] Missing super() call │
- │ ════════════════════════════ │
- │ ❌ def validate(self): pass → Breaks parent validation │
- │ ✓ def validate(self): super().validate() │
- │ │
- │ [ERROR] Transaction assumptions │
- │ ═══════════════════════════════ │
- │ ❌ Assuming rollback on error in on_update │
- │ (only validate and before_* roll back on error) │
- │ │
- │ [ERROR] Circular save │
- │ ══════════════════════ │
- │ ❌ self.save() inside lifecycle hooks │
- │ ❌ doc.save() for same document in hooks │
- │ │
- └─────────────────────────────────────────────────────────────────────┘
- ```
+ ## Critical Checks by Code Type
- → See [references/checklists.md](references/checklists.md) for complete checklists.
+ ### Server Script Checks
- ## Validation Report Format
+ | Check | Severity | Pattern | Fix |
+ |-------|----------|---------|-----|
+ | Import statements | FATAL | `import X` or `from X import Y` | Use `frappe.utils.X()` directly |
+ | Wrong doc variable | FATAL | `self.field` or `document.field` | Use `doc.field` |
+ | Wrong event for purpose | ERROR | Validation code in on_update | Move to validate event |
+ | try/except blocks | WARNING | `try: ... except:` | Use `frappe.throw()` for validation |
+ | No null checks | WARNING | `doc.field.lower()` | Add `if doc.field:` guard |
- ```markdown
- ## Code Validation Report
+ ### Client Script Checks
- ### Code Type: [Server Script / Client Script / Controller / etc.]
- ### Target DocType: [DocType name]
- ### Event/Trigger: [Event name]
+ | Check | Severity | Pattern | Fix |
+ |-------|----------|---------|-----|
+ | Server-side API calls | FATAL | `frappe.db.get_value()` | Use `frappe.call()` |
+ | Missing async handling | FATAL | `let x = frappe.call()` | Use callback or async/await |
+ | No refresh after set_value | ERROR | `frm.set_value()` alone | Add `frm.refresh_field()` |
+ | Using cur_frm | WARNING | `cur_frm.doc.field` | Use `frm` parameter |
+ | No form state check | WARNING | Missing `__islocal`/`docstatus` | Add state guards |
- ---
+ ### Controller Checks
- ### 🔴 CRITICAL ERRORS (Must Fix)
+ | Check | Severity | Pattern | Fix |
+ |-------|----------|---------|-----|
+ | self.* in on_update | FATAL | `self.field = X` in on_update | Use `self.db_set()` |
+ | Circular save | FATAL | `self.save()` in lifecycle hook | Remove self.save() |
+ | Missing super() | ERROR | Override without super() | Add `super().method()` |
+ | v16 extend_doctype_class | ERROR | Missing super() in mixin | ALWAYS call super() first |
+ | No type annotations | SUGGESTION | Missing type hints (v16) | Add type annotations |
- | Line | Issue | Fix |
- |------|-------|-----|
- | 3 | Import statement in Server Script | Use frappe.utils.X() directly |
+ ### hooks.py Checks
- ### 🟡 WARNINGS (Should Fix)
+ | Check | Severity | Pattern | Fix |
+ |-------|----------|---------|-----|
+ | Invalid Python syntax | FATAL | Syntax errors | Fix dict/list structure |
+ | Wrong event names | FATAL | Typo in event name | Use correct event names |
+ | Invalid function paths | FATAL | Wrong dotted path | Verify path exists |
+ | v16-only hooks on v14/v15 | ERROR | `extend_doctype_class` | Use `doc_events` instead |
+ | Missing required_apps | WARNING | No dependency declaration | Add all dependencies |
- | Line | Issue | Recommendation |
- |------|-------|----------------|
- | 12 | No null check before .lower() | Add: if value: value.lower() |
+ ### Ops/Bench Checks
- ### 🔵 SUGGESTIONS (Nice to Have)
+ | Check | Severity | Pattern | Fix |
+ |-------|----------|---------|-----|
+ | No migrate after hooks | FATAL | hooks.py changed, no migrate | Run `bench migrate` |
+ | Wrong bench command syntax | ERROR | Incorrect CLI args | Check `frappe-ops-bench` |
+ | Missing backup before upgrade | ERROR | Upgrade without backup | ALWAYS backup first |
+ | Production without supervisor | WARNING | No process manager | Use supervisor/systemd |
+ | No SSL in production | WARNING | HTTP-only deployment | Configure SSL/TLS |
- | Line | Suggestion |
- |------|------------|
- | 8 | Consider using frappe.db.get_value for single field |
+ ### DocType JSON Checks
- ---
+ | Check | Severity | Pattern | Fix |
+ |-------|----------|---------|-----|
+ | Missing mandatory fields | ERROR | No primary identifier | Add name or autoname |
+ | Duplicate fieldnames | FATAL | Same fieldname twice | Use unique fieldnames |
+ | Wrong fieldtype for data | WARNING | Text for short values | Use Data/Small Text |
+ | No permissions defined | WARNING | Empty permission list | Add role permissions |
- ### Corrected Code
+ ## v16 Specific Validations
+ ### extend_doctype_class Pattern
```python
- # [Corrected version with all critical errors fixed]
+ # VALIDATE: Mixin class MUST call super()
+ class CustomSalesInvoice(SalesInvoice):
+ def validate(self):
+ super().validate() # REQUIRED - never skip
+ self.custom_validation()
+
+ def on_submit(self):
+ super().on_submit() # REQUIRED - never skip
+ self.custom_on_submit()
```
- ### Version Compatibility
+ ### Type Annotations (v16 best practice)
+ ```python
+ # v16 recommended pattern
+ def get_customer_balance(customer: str) -> float:
+ ...
- | Version | Status |
- |---------|--------|
- | v14 | ✅ Compatible |
- | v15 | ✅ Compatible |
- | v16 | ✅ Compatible |
+ # Validate: type hints on public API methods
+ @frappe.whitelist()
+ def process_order(order_name: str, action: str = "approve") -> dict:
+ ...
```
- ## Universal Validation Rules
+ ### Data Masking (v16)
+ ```python
+ # Validate: sensitive fields should use data masking
+ # Check if PII fields have mask_with configured in DocType JSON
+ ```
- These apply to ALL code types:
+ ## Universal Validation Rules
- ### Security Checks
+ ### Security Checks (ALL code types)
| Check | Severity | Description |
|-------|----------|-------------|
- | SQL Injection | CRITICAL | Raw user input in SQL queries |
- | Permission bypass | CRITICAL | Missing permission checks before operations |
+ | SQL Injection | CRITICAL | Raw user input in SQL |
+ | Permission bypass | CRITICAL | Missing permission checks |
| XSS vulnerability | HIGH | Unescaped user input in HTML |
| Sensitive data exposure | HIGH | Logging passwords/tokens |
+ | Hardcoded credentials | CRITICAL | API keys in source code |
- ### Error Handling Checks
+ ### Performance Checks (ALL code types)
| Check | Severity | Description |
|-------|----------|-------------|
- | Silent failures | HIGH | Catching exceptions without handling |
- | Missing user feedback | MEDIUM | Errors not communicated to user |
- | Generic error messages | LOW | "An error occurred" without details |
+ | Query in loop | HIGH | `frappe.db.*` inside for loop |
+ | Unbounded query | MEDIUM | SELECT without LIMIT |
+ | Unnecessary get_doc | LOW | get_doc when get_value suffices |
+ | Missing index | MEDIUM | Filter on non-indexed field |
+ | No batch commit | HIGH | Commit per record in bulk ops |
- ### Performance Checks
+ ### Error Handling Checks (ALL code types)
| Check | Severity | Description |
|-------|----------|-------------|
- | Query in loop | HIGH | frappe.db.* inside for loop |
- | Unbounded query | MEDIUM | SELECT without LIMIT |
- | Unnecessary get_doc | LOW | get_doc when get_value suffices |
+ | Silent failures | HIGH | `except: pass` without logging |
+ | Missing user feedback | MEDIUM | Errors not shown to user |
+ | Generic error messages | LOW | "An error occurred" |
+ | No rollback on failure | HIGH | Partial data on error |
- → See [references/examples.md](references/examples.md) for validation examples.
+ ## Validation Report Format
- ## Version-Specific Validations
+ ALWAYS generate reports in this format:
- ### v16 Features (Fail on v14/v15)
+ ```markdown
+ ## Code Validation Report
- ```python
- # These ONLY work on v16+
- extend_doctype_class = {} # hooks.py - v16 only
- naming_rule = "UUID" # DocType - v16 only
- pdf_renderer = "chrome" # Print Format - v16 only
- ```
+ ### Code Type: [type]
+ ### Target: [DocType / App / File]
+ ### Event/Trigger: [if applicable]
- ### Deprecated Patterns (Warn)
+ ### CRITICAL ERRORS (Must Fix)
+ | # | Line | Issue | Fix |
+ |---|------|-------|-----|
- ```python
- # DEPRECATED - still works but should update
- frappe.bean() # Use frappe.get_doc()
- frappe.msgprint(raise_exception=True) # Use frappe.throw()
- job_name parameter # Use job_id (v15+)
+ ### WARNINGS (Should Fix)
+ | # | Line | Issue | Recommendation |
+ |---|------|-------|----------------|
+
+ ### SUGGESTIONS (Nice to Have)
+ | # | Line | Suggestion |
+ |---|------|------------|
+
+ ### Corrected Code
+ [If critical errors found, provide corrected version]
+
+ ### Version Compatibility
+ | Version | Status | Notes |
+ |---------|--------|-------|
+ | v14 | [status] | |
+ | v15 | [status] | |
+ | v16 | [status] | |
+
+ ### Referenced Skills
+ - frappe-skill-name: [what was validated against]
```
- ### Version-Specific Behaviors
+ ## Validation Depth Levels
- | Behavior | v14 | v15/v16 |
- |----------|-----|---------|
- | Scheduler tick | 240s | 60s |
- | Background job dedup | job_name | job_id |
+ | Level | Checks | Use When |
+ |-------|--------|----------|
+ | Quick | Fatal errors only | Initial scan |
+ | Standard | + Warnings + Security | Pre-deployment (DEFAULT) |
+ | Deep | + Suggestions + Performance + Ops | Production review |
- ## Quick Validation Commands
+ ## Skill Catalog Cross-Reference
- ### Server Script Quick Check
- 1. ❌ Any `import` statements? → FATAL
- 2. ❌ Any `self.` references? → FATAL (use `doc.`)
- 3. ❌ Any `try/except`? → WARNING (usually wrong)
- 4. ✅ Uses `frappe.throw()` for validation errors? → GOOD
- 5. ✅ Uses `doc.field` for document access? → GOOD
+ This validator validates against ALL 53 frappe-* skills:
- ### Client Script Quick Check
- 1. ❌ Any `frappe.db.*` calls? → FATAL (server-side only)
- 2. ❌ Any `frappe.get_doc()` calls? → FATAL (server-side only)
- 3. ❌ `frappe.call()` without callback? → FATAL (async issue)
- 4. ✅ Uses `frm.doc.field` for field access? → GOOD
- 5. ✅ Uses `frm.refresh_field()` after changes? → GOOD
+ ### Syntax Validation (11 skills)
+ `frappe-syntax-clientscripts`, `frappe-syntax-serverscripts`, `frappe-syntax-controllers`, `frappe-syntax-hooks`, `frappe-syntax-hooks-events`, `frappe-syntax-whitelisted`, `frappe-syntax-jinja`, `frappe-syntax-scheduler`, `frappe-syntax-customapp`, `frappe-syntax-doctypes`, `frappe-syntax-reports`
- ### Controller Quick Check
- 1. ❌ Modifying `self.*` in `on_update`? → ERROR (won't save)
- 2. ❌ Missing `super().method()` calls? → WARNING
- 3. ❌ `self.save()` in lifecycle hook? → FATAL (circular)
- 4. ✅ Imports at top of file? → GOOD (controllers allow imports)
- 5. ✅ Error handling with try/except? → GOOD (controllers allow this)
+ ### Implementation Validation (12 skills)
+ `frappe-impl-clientscripts`, `frappe-impl-serverscripts`, `frappe-impl-controllers`, `frappe-impl-hooks`, `frappe-impl-whitelisted`, `frappe-impl-jinja`, `frappe-impl-scheduler`, `frappe-impl-customapp`, `frappe-impl-reports`, `frappe-impl-workflow`, `frappe-impl-website`, `frappe-impl-ui-components`, `frappe-impl-integrations`
- ## Integration with Other Skills
+ ### Error Pattern Validation (7 skills)
+ `frappe-errors-clientscripts`, `frappe-errors-serverscripts`, `frappe-errors-controllers`, `frappe-errors-hooks`, `frappe-errors-api`, `frappe-errors-permissions`, `frappe-errors-database`
- This validator uses knowledge from:
+ ### Core Pattern Validation (7 skills)
+ `frappe-core-database`, `frappe-core-permissions`, `frappe-core-api`, `frappe-core-workflow`, `frappe-core-notifications`, `frappe-core-files`, `frappe-core-cache`
- | Skill | What It Provides |
- |-------|------------------|
- | `frappe-syntax-*` | Correct syntax patterns |
- | `frappe-impl-*` | Correct implementation patterns |
- | `frappe-errors-*` | Error handling patterns |
- | `frappe-core-database` | Query patterns and pitfalls |
- | `frappe-core-permissions` | Permission check patterns |
- | `frappe-core-api` | API response patterns |
+ ### Ops Validation (8 skills)
+ `frappe-ops-bench`, `frappe-ops-deployment`, `frappe-ops-backup`, `frappe-ops-performance`, `frappe-ops-upgrades`, `frappe-ops-cloud`, `frappe-ops-app-lifecycle`, `frappe-ops-frontend-build`
- ## Validation Depth Levels
+ ### Testing Validation (2 skills)
+ `frappe-testing-unit`, `frappe-testing-cicd`
- | Level | Checks | Use When |
- |-------|--------|----------|
- | Quick | Fatal errors only | Initial scan |
- | Standard | + Warnings | Pre-deployment |
- | Deep | + Suggestions + Optimization | Production review |
+ ## Quick Validation Commands
- Default: **Standard** level for most validations.
+ ### Server Script: 5-point check
+ 1. Any `import` statements? --> FATAL
+ 2. Any `self.` references? --> FATAL (use `doc.`)
+ 3. Any `try/except`? --> WARNING (usually wrong)
+ 4. Uses `frappe.throw()` for validation? --> GOOD
+ 5. Uses `doc.field` for access? --> GOOD
+
+ ### Client Script: 5-point check
+ 1. Any `frappe.db.*` calls? --> FATAL
+ 2. Any `frappe.get_doc()` calls? --> FATAL
+ 3. `frappe.call()` without callback? --> FATAL
+ 4. Uses `frm.doc.field` for access? --> GOOD
+ 5. Uses `frm.refresh_field()` after changes? --> GOOD
+
+ ### Controller: 5-point check
+ 1. Modifying `self.*` in `on_update`? --> FATAL
+ 2. Missing `super().method()` calls? --> ERROR
+ 3. `self.save()` in lifecycle hook? --> FATAL
+ 4. Imports at top of file? --> GOOD
+ 5. Error handling for external calls? --> GOOD
+
+ ### hooks.py: 5-point check
+ 1. Valid Python syntax? --> Check
+ 2. Function paths exist? --> Check
+ 3. v16-only hooks marked? --> Check
+ 4. required_apps complete? --> Check
+ 5. Fixture filters present? --> Check
+
+ ### Bench/Ops: 5-point check
+ 1. `bench migrate` after changes? --> REQUIRED
+ 2. Backup before destructive ops? --> REQUIRED
+ 3. Scheduler enabled? --> Check
+ 4. Workers running? --> Check
+ 5. SSL configured (production)? --> Check
+
+ See [references/checklists.md](references/checklists.md) for complete checklists.
+ See [references/examples.md](references/examples.md) for validation examples.