From 28b51757c0447a204d7a45649b6a2233fcb929a2 Mon Sep 17 00:00:00 2001
From: Patrick Lewis <4015312+locus313@users.noreply.github.com>
Date: Thu, 8 Oct 2026 20:16:10 -0700
Subject: [PATCH] docs: replace generic code review instructions with repo
 rules

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 .../code-review-generic.instructions.md       | 418 ------------------
 .../instructions/code-review.instructions.md  |  25 ++
 2 files changed, 25 insertions(+), 418 deletions(-)
 delete mode 100644 .github/instructions/code-review-generic.instructions.md
 create mode 100644 .github/instructions/code-review.instructions.md

diff --git a/.github/instructions/code-review-generic.instructions.md b/.github/instructions/code-review-generic.instructions.md
deleted file mode 100644
index bcd7365..0000000
--- a/.github/instructions/code-review-generic.instructions.md
+++ /dev/null
@@ -1,418 +0,0 @@
----
-description: 'Generic code review instructions that can be customized for any project using GitHub Copilot'
-applyTo: '**'
-excludeAgent: ["coding-agent"]
----
-
-# Generic Code Review Instructions
-
-Comprehensive code review guidelines for GitHub Copilot that can be adapted to any project. These instructions follow best practices from prompt engineering and provide a structured approach to code quality, security, testing, and architecture review.
-
-## Review Language
-
-When performing a code review, respond in **English** (or specify your preferred language).
-
-> **Customization Tip**: Change to your preferred language by replacing "English" with "Portuguese (Brazilian)", "Spanish", "French", etc.
-
-## Review Priorities
-
-When performing a code review, prioritize issues in the following order:
-
-### 🔴 CRITICAL (Block merge)
-- **Security**: Vulnerabilities, exposed secrets, authentication/authorization issues
-- **Correctness**: Logic errors, data corruption risks, race conditions
-- **Breaking Changes**: API contract changes without versioning
-- **Data Loss**: Risk of data loss or corruption
-
-### 🟡 IMPORTANT (Requires discussion)
-- **Code Quality**: Severe violations of SOLID principles, excessive duplication
-- **Test Coverage**: Missing tests for critical paths or new functionality
-- **Performance**: Obvious performance bottlenecks (N+1 queries, memory leaks)
-- **Architecture**: Significant deviations from established patterns
-
-### 🟢 SUGGESTION (Non-blocking improvements)
-- **Readability**: Poor naming, complex logic that could be simplified
-- **Optimization**: Performance improvements without functional impact
-- **Best Practices**: Minor deviations from conventions
-- **Documentation**: Missing or incomplete comments/documentation
-
-## General Review Principles
-
-When performing a code review, follow these principles:
-
-1. **Be specific**: Reference exact lines, files, and provide concrete examples
-2. **Provide context**: Explain WHY something is an issue and the potential impact
-3. **Suggest solutions**: Show corrected code when applicable, not just what's wrong
-4. **Be constructive**: Focus on improving the code, not criticizing the author
-5. **Recognize good practices**: Acknowledge well-written code and smart solutions
-6. **Be pragmatic**: Not every suggestion needs immediate implementation
-7. **Group related comments**: Avoid multiple comments about the same topic
-
-## Code Quality Standards
-
-When performing a code review, check for:
-
-### Clean Code
-- Descriptive and meaningful names for variables, functions, and classes
-- Single Responsibility Principle: each function/class does one thing well
-- DRY (Don't Repeat Yourself): no code duplication
-- Functions should be small and focused (ideally < 20-30 lines)
-- Avoid deeply nested code (max 3-4 levels)
-- Avoid magic numbers and strings (use constants)
-- Code should be self-documenting; comments only when necessary
-
-### Examples
-```javascript
-// ❌ BAD: Poor naming and magic numbers
-function calc(x, y) {
-    if (x > 100) return y * 0.15;
-    return y * 0.10;
-}
-
-// ✅ GOOD: Clear naming and constants
-const PREMIUM_THRESHOLD = 100;
-const PREMIUM_DISCOUNT_RATE = 0.15;
-const STANDARD_DISCOUNT_RATE = 0.10;
-
-function calculateDiscount(orderTotal, itemPrice) {
-    const isPremiumOrder = orderTotal > PREMIUM_THRESHOLD;
-    const discountRate = isPremiumOrder ? PREMIUM_DISCOUNT_RATE : STANDARD_DISCOUNT_RATE;
-    return itemPrice * discountRate;
-}
-```
-
-### Error Handling
-- Proper error handling at appropriate levels
-- Meaningful error messages
-- No silent failures or ignored exceptions
-- Fail fast: validate inputs early
-- Use appropriate error types/exceptions
-
-### Examples
-```python
-# ❌ BAD: Silent failure and generic error
-def process_user(user_id):
-    try:
-        user = db.get(user_id)
-        user.process()
-    except:
-        pass
-
-# ✅ GOOD: Explicit error handling
-def process_user(user_id):
-    if not user_id or user_id <= 0:
-        raise ValueError(f"Invalid user_id: {user_id}")
-
-    try:
-        user = db.get(user_id)
-    except UserNotFoundError:
-        raise UserNotFoundError(f"User {user_id} not found in database")
-    except DatabaseError as e:
-        raise ProcessingError(f"Failed to retrieve user {user_id}: {e}")
-
-    return user.process()
-```
-
-## Security Review
-
-When performing a code review, check for security issues:
-
-- **Sensitive Data**: No passwords, API keys, tokens, or PII in code or logs
-- **Input Validation**: All user inputs are validated and sanitized
-- **SQL Injection**: Use parameterized queries, never string concatenation
-- **Authentication**: Proper authentication checks before accessing resources
-- **Authorization**: Verify user has permission to perform action
-- **Cryptography**: Use established libraries, never roll your own crypto
-- **Dependency Security**: Check for known vulnerabilities in dependencies
-
-### Examples
-```java
-// ❌ BAD: SQL injection vulnerability
-String query = "SELECT * FROM users WHERE email = '" + email + "'";
-
-// ✅ GOOD: Parameterized query
-PreparedStatement stmt = conn.prepareStatement(
-    "SELECT * FROM users WHERE email = ?"
-);
-stmt.setString(1, email);
-```
-
-```javascript
-// ❌ BAD: Exposed secret in code
-const API_KEY = "sk_live_abc123xyz789";
-
-// ✅ GOOD: Use environment variables
-const API_KEY = process.env.API_KEY;
-```
-
-## Testing Standards
-
-When performing a code review, verify test quality:
-
-- **Coverage**: Critical paths and new functionality must have tests
-- **Test Names**: Descriptive names that explain what is being tested
-- **Test Structure**: Clear Arrange-Act-Assert or Given-When-Then pattern
-- **Independence**: Tests should not depend on each other or external state
-- **Assertions**: Use specific assertions, avoid generic assertTrue/assertFalse
-- **Edge Cases**: Test boundary conditions, null values, empty collections
-- **Mock Appropriately**: Mock external dependencies, not domain logic
-
-### Examples
-```typescript
-// ❌ BAD: Vague name and assertion
-test('test1', () => {
-    const result = calc(5, 10);
-    expect(result).toBeTruthy();
-});
-
-// ✅ GOOD: Descriptive name and specific assertion
-test('should calculate 10% discount for orders under $100', () => {
-    const orderTotal = 50;
-    const itemPrice = 20;
-
-    const discount = calculateDiscount(orderTotal, itemPrice);
-
-    expect(discount).toBe(2.00);
-});
-```
-
-## Performance Considerations
-
-When performing a code review, check for performance issues:
-
-- **Database Queries**: Avoid N+1 queries, use proper indexing
-- **Algorithms**: Appropriate time/space complexity for the use case
-- **Caching**: Utilize caching for expensive or repeated operations
-- **Resource Management**: Proper cleanup of connections, files, streams
-- **Pagination**: Large result sets should be paginated
-- **Lazy Loading**: Load data only when needed
-
-### Examples
-```python
-# ❌ BAD: N+1 query problem
-users = User.query.all()
-for user in users:
-    orders = Order.query.filter_by(user_id=user.id).all()  # N+1!
-
-# ✅ GOOD: Use JOIN or eager loading
-users = User.query.options(joinedload(User.orders)).all()
-for user in users:
-    orders = user.orders
-```
-
-## Architecture and Design
-
-When performing a code review, verify architectural principles:
-
-- **Separation of Concerns**: Clear boundaries between layers/modules
-- **Dependency Direction**: High-level modules don't depend on low-level details
-- **Interface Segregation**: Prefer small, focused interfaces
-- **Loose Coupling**: Components should be independently testable
-- **High Cohesion**: Related functionality grouped together
-- **Consistent Patterns**: Follow established patterns in the codebase
-
-## Documentation Standards
-
-When performing a code review, check documentation:
-
-- **API Documentation**: Public APIs must be documented (purpose, parameters, returns)
-- **Complex Logic**: Non-obvious logic should have explanatory comments
-- **README Updates**: Update README when adding features or changing setup
-- **Breaking Changes**: Document any breaking changes clearly
-- **Examples**: Provide usage examples for complex features
-
-## Comment Format Template
-
-When performing a code review, use this format for comments:
-
-```markdown
-**[PRIORITY] Category: Brief title**
-
-Detailed description of the issue or suggestion.
-
-**Why this matters:**
-Explanation of the impact or reason for the suggestion.
-
-**Suggested fix:**
-[code example if applicable]
-
-**Reference:** [link to relevant documentation or standard]
-```
-
-### Example Comments
-
-#### Critical Issue
-````markdown
-**🔴 CRITICAL - Security: SQL Injection Vulnerability**
-
-The query on line 45 concatenates user input directly into the SQL string,
-creating a SQL injection vulnerability.
-
-**Why this matters:**
-An attacker could manipulate the email parameter to execute arbitrary SQL commands,
-potentially exposing or deleting all database data.
-
-**Suggested fix:**
-```sql
--- Instead of:
-query = "SELECT * FROM users WHERE email = '" + email + "'"
-
--- Use:
-PreparedStatement stmt = conn.prepareStatement(
-    "SELECT * FROM users WHERE email = ?"
-);
-stmt.setString(1, email);
-```
-
-**Reference:** OWASP SQL Injection Prevention Cheat Sheet
-````
-
-#### Important Issue
-````markdown
-**🟡 IMPORTANT - Testing: Missing test coverage for critical path**
-
-The `processPayment()` function handles financial transactions but has no tests
-for the refund scenario.
-
-**Why this matters:**
-Refunds involve money movement and should be thoroughly tested to prevent
-financial errors or data inconsistencies.
-
-**Suggested fix:**
-Add test case:
-```javascript
-test('should process full refund when order is cancelled', () => {
-    const order = createOrder({ total: 100, status: 'cancelled' });
-
-    const result = processPayment(order, { type: 'refund' });
-
-    expect(result.refundAmount).toBe(100);
-    expect(result.status).toBe('refunded');
-});
-```
-````
-
-#### Suggestion
-````markdown
-**🟢 SUGGESTION - Readability: Simplify nested conditionals**
-
-The nested if statements on lines 30-40 make the logic hard to follow.
-
-**Why this matters:**
-Simpler code is easier to maintain, debug, and test.
-
-**Suggested fix:**
-```javascript
-// Instead of nested ifs:
-if (user) {
-    if (user.isActive) {
-        if (user.hasPermission('write')) {
-            // do something
-        }
-    }
-}
-
-// Consider guard clauses:
-if (!user || !user.isActive || !user.hasPermission('write')) {
-    return;
-}
-// do something
-```
-````
-
-## Review Checklist
-
-When performing a code review, systematically verify:
-
-### Code Quality
-- [ ] Code follows consistent style and conventions
-- [ ] Names are descriptive and follow naming conventions
-- [ ] Functions/methods are small and focused
-- [ ] No code duplication
-- [ ] Complex logic is broken into simpler parts
-- [ ] Error handling is appropriate
-- [ ] No commented-out code or TODO without tickets
-
-### Security
-- [ ] No sensitive data in code or logs
-- [ ] Input validation on all user inputs
-- [ ] No SQL injection vulnerabilities
-- [ ] Authentication and authorization properly implemented
-- [ ] Dependencies are up-to-date and secure
-
-### Testing
-- [ ] New code has appropriate test coverage
-- [ ] Tests are well-named and focused
-- [ ] Tests cover edge cases and error scenarios
-- [ ] Tests are independent and deterministic
-- [ ] No tests that always pass or are commented out
-
-### Performance
-- [ ] No obvious performance issues (N+1, memory leaks)
-- [ ] Appropriate use of caching
-- [ ] Efficient algorithms and data structures
-- [ ] Proper resource cleanup
-
-### Architecture
-- [ ] Follows established patterns and conventions
-- [ ] Proper separation of concerns
-- [ ] No architectural violations
-- [ ] Dependencies flow in correct direction
-
-### Documentation
-- [ ] Public APIs are documented
-- [ ] Complex logic has explanatory comments
-- [ ] README is updated if needed
-- [ ] Breaking changes are documented
-
-## Project-Specific Customizations
-
-To customize this template for your project, add sections for:
-
-1. **Language/Framework specific checks**
-   - Example: "When performing a code review, verify React hooks follow rules of hooks"
-   - Example: "When performing a code review, check Spring Boot controllers use proper annotations"
-
-2. **Build and deployment**
-   - Example: "When performing a code review, verify CI/CD pipeline configuration is correct"
-   - Example: "When performing a code review, check database migrations are reversible"
-
-3. **Business logic rules**
-   - Example: "When performing a code review, verify pricing calculations include all applicable taxes"
-   - Example: "When performing a code review, check user consent is obtained before data processing"
-
-4. **Team conventions**
-   - Example: "When performing a code review, verify commit messages follow conventional commits format"
-   - Example: "When performing a code review, check branch names follow pattern: type/ticket-description"
-
-## Additional Resources
-
-For more information on effective code reviews and GitHub Copilot customization:
-
-- [GitHub Copilot Prompt Engineering](https://docs.github.com/en/copilot/concepts/prompting/prompt-engineering)
-- [GitHub Copilot Custom Instructions](https://code.visualstudio.com/docs/copilot/customization/custom-instructions)
-- [Awesome GitHub Copilot Repository](https://github.com/github/awesome-copilot)
-- [GitHub Code Review Guidelines](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests)
-- [Google Engineering Practices - Code Review](https://google.github.io/eng-practices/review/)
-- [OWASP Security Guidelines](https://owasp.org/)
-
-## Prompt Engineering Tips
-
-When performing a code review, apply these prompt engineering principles from the [GitHub Copilot documentation](https://docs.github.com/en/copilot/concepts/prompting/prompt-engineering):
-
-1. **Start General, Then Get Specific**: Begin with high-level architecture review, then drill into implementation details
-2. **Give Examples**: Reference similar patterns in the codebase when suggesting changes
-3. **Break Complex Tasks**: Review large PRs in logical chunks (security → tests → logic → style)
-4. **Avoid Ambiguity**: Be specific about which file, line, and issue you're addressing
-5. **Indicate Relevant Code**: Reference related code that might be affected by changes
-6. **Experiment and Iterate**: If initial review misses something, review again with focused questions
-
-## Project Context
-
-This is a generic template. Customize this section with your project-specific information:
-
-- **Tech Stack**: [e.g., Java 17, Spring Boot 3.x, PostgreSQL]
-- **Architecture**: [e.g., Hexagonal/Clean Architecture, Microservices]
-- **Build Tool**: [e.g., Gradle, Maven, npm, pip]
-- **Testing**: [e.g., JUnit 5, Jest, pytest]
-- **Code Style**: [e.g., follows Google Style Guide]
diff --git a/.github/instructions/code-review.instructions.md b/.github/instructions/code-review.instructions.md
new file mode 100644
index 0000000..c2bd6b4
--- /dev/null
+++ b/.github/instructions/code-review.instructions.md
@@ -0,0 +1,25 @@
+---
+description: 'Review rules for Plesk automation scripts'
+applyTo: '**'
+excludeAgent: ["coding-agent"]
+---
+
+# Code review
+
+Shellcheck, the self-update drift check and the docs check run in CI; leave what they catch alone. Review for the judgement calls below.
+
+**Block merge**
+
+- Real credentials, tokens or passwords in a diff. Windows scripts keep `<password_for_mysql>`; Linux scripts use `plesk db`.
+- A `rm`, `find ... -delete` or `mysqldump` path that can act outside its intended directory (unquoted or empty variable, glob that widens).
+- Windows batch that uses `%var%` where `!var!` is needed, or leaves a path unquoted. Plesk's default path contains parentheses.
+- A self-update block edited in one script only. It changes in `.github/self-update.template.sh` first, then everywhere.
+- A script that can overlap itself on a schedule, or writes backups, state or reports, without a PID lock or `umask 077`.
+
+**Discuss**
+
+- A feature added to one side of a `.bat`/`.sh` pair that ships both (`mysql-backup`, `pci-dss-scan`) without a note that the Windows version is a subset.
+- A behaviour, flag or default changed without the folder `README.md` and header comment following.
+- A new script without a folder README, a row in the root README Scripts table, or a Linux-only justification for a missing `.bat`.
+- A new term that overlaps one in `GLOSSARY.md`.
+- A decision that is hard to reverse and surprising without context but has no ADR.