Get the FREE Ultimate OpenClaw Setup Guide →

fresh-eyes-review

Scanned
npx machina-cli add skill aiskillstore/marketplace/fresh-eyes-review --openclaw
Files (1)
SKILL.md
6.2 KB

Fresh-Eyes Review

Core Principle

"NO COMMIT WITHOUT FRESH-EYES REVIEW FIRST"

This represents a final quality gate executed after implementation completion, passing tests, and peer review. The discipline applies universally, even without explicit skill activation.

Key Distinctions

Fresh-eyes review differs fundamentally from testing and code review:

ApproachFocusBlind Spots
TestingValidates expected behaviorCan't test for unknown edge cases
Code reviewPatterns and qualityReviewer trusts author's intent
Fresh-eyesDeliberate re-reading with psychological distanceCatches what you thought was correct

Critical insight: "100% test coverage and passing scenarios" can coexist with "critical bugs" waiting discovery.

Required Process

Step 1 - Announce Commitment

Explicitly declare: "Starting fresh-eyes review of [N] files. This will take 2-5 minutes."

This announcement creates accountability and reframes your mindset from implementation to audit.

Step 2 - Security Vulnerability Checklist

Review all touched files for security issues:

VulnerabilityWhat to Check
SQL InjectionAll database queries use parameterized statements, never string concatenation
XSSAll user-provided content is escaped before rendering in HTML
Path TraversalFile paths are validated, ../ sequences rejected or normalized
Command InjectionShell commands don't include unsanitized user input
IDORResources are access-controlled, not just unguessable IDs
Auth BypassEvery protected endpoint checks authentication and authorization

Example finding:

// Before: SQL injection vulnerability
const user = await db.query(`SELECT * FROM users WHERE id = '${userId}'`);

// After: Parameterized query
const user = await db.query('SELECT * FROM users WHERE id = $1', [userId]);

Step 3 - Logic Error Checklist

Error TypeWhat to Check
Off-by-oneArray indices, loop bounds, pagination limits
Race conditionsConcurrent access to shared state, async operations
Null/undefinedEvery . chain could throw; defensive checks present?
Type coercion== vs ===, implicit conversions
State mutationsUnexpected side effects on input parameters?
Error swallowingEmpty catch blocks, ignored promise rejections

Example finding:

// Before: Off-by-one in pagination
const hasMore = results.length < pageSize;

// After: Correct boundary
const hasMore = results.length === pageSize;

Step 4 - Business Rule Checklist

CheckQuestions
CalculationsDo formulas match requirements exactly? Currency rounding correct?
ConditionsAND vs OR logic correct? Negations applied properly?
Edge casesEmpty input, single item, maximum values, zero values?
Error messagesUser-friendly? Leak no sensitive information?
Default valuesSensible defaults when optional fields omitted?

Example finding:

// Before: Tax calculation uses wrong rounding
const tax = price * 0.08;

// After: Proper currency rounding
const tax = Math.round(price * 0.08 * 100) / 100;

Step 5 - Performance Checklist

IssueWhat to Check
N+1 queriesLoops that make database calls should be batched
Unbounded loopsMaximum iterations, timeout protection
Memory leaksEvent listeners removed, streams closed, references cleared
Missing indexesQueries filter/sort on indexed columns?
Large payloadsPagination implemented? Response size bounded?

Step 6 - Fix Immediately

Address findings before declaring completion:

  1. Make the fix
  2. Add test covering the issue (if not present)
  3. Re-run full test suite
  4. Re-run linter/type checker

Step 7 - Declare Results

Mandatory announcement:

Fresh-eyes complete. [N] issues found and fixed:
- [Brief description of each issue and fix]

Include this even for zero findings—it proves execution.

Time Commitment

File CountExpected Duration
1-3 files2 minutes
4-10 files3-4 minutes
10+ files5 minutes max

Warning signs:

  • Faster than 2 minutes suggests insufficient depth
  • More than 5 minutes indicates scope creep—stay focused on the changed code

Resistance Patterns to Reject

The following rationalizations must be explicitly rejected:

RationalizationReality
"Tests are comprehensive"Tests validate design, not correctness
"I'm confident it's correct"Confidence is inversely correlated with bugs
"Partner is waiting"3 minutes now saves 3 hours debugging later
"Production is blocked"Rushing causes the outages being rushed to fix
"Senior dev already approved"They reviewed intent, not implementation details
"It's just a small change"Small changes cause large outages

These circumstances represent precisely when critical bugs escape into production.

Example Workflow

User: "I'm done implementing the auth feature, let me commit"
Assistant: [Triggers fresh-eyes-review skill]

Starting fresh-eyes review of 4 files. This will take 2-5 minutes.

[Reviews auth.ts for security]
- Found: Missing input validation on email field
- Fixed: Added email format validation

[Reviews handler.ts for logic]
- Found: Off-by-one error in pagination
- Fixed: Changed < to <= in boundary check

[Reviews db.ts for performance]
- No issues found

[Reviews api.ts for business rules]
- No issues found

Re-running tests... All tests pass

Fresh-eyes complete. 2 issues found and fixed.

Source

git clone https://github.com/aiskillstore/marketplace/blob/main/skills/2389-research/fresh-eyes-review/SKILL.mdView on GitHub

Overview

Fresh-eyes-review is a mandatory final sanity gate that runs after implementation, tests, and peer review. It activates on commit, push, PR, or when work is marked done, and it catches security vulnerabilities, logic errors, and business-rule bugs that slip through standard checks.

How This Skill Works

After finishing a feature or fix and passing tests, you perform a five-step audit: announce commitment, complete the security vulnerability checklist, complete the logic error checklist, complete the business rule checklist, and complete the performance checklist (N+1 queries and unbounded loops). This structured pass is designed to surface issues missed by earlier stages.

When to Use It

  • Before you commit code to your branch
  • Before pushing changes or opening a pull request
  • When marking work as done or ready to merge
  • Before deploying to staging or production
  • Whenever you want a final sanity check after implementing a feature or fix

Quick Start

  1. Step 1: Announce commitment: say Starting fresh-eyes review of N files. This will take 2-5 minutes.
  2. Step 2: Run through security vulnerability and logic/business rule checklists.
  3. Step 3: Declare readiness to merge once all findings are resolved and tests pass

Best Practices

  • Make Fresh-Eyes Review the mandatory final gate before merge
  • Scan for security vulnerabilities: SQL injection, XSS, path traversal, command injection, IDOR, and auth bypass
  • Use the logic error checklist to catch off-by-one errors, race conditions, null/undefined, type coercion, and unexpected state mutations
  • Validate business rules: correct calculations, conditions, edge cases, and clear error messages
  • Check performance for N+1 queries and unbounded loops and optimize accordingly

Example Use Cases

  • Before commit: SQL injection fixed by parameterizing a query
  • Off-by-one pagination error detected and corrected
  • Tax calculation fixed to use proper currency rounding
  • Endpoint protected with proper authentication/authorization to prevent auth bypass
  • Identified and mitigated potential unbounded loop risk by introducing a max iterations cap

Frequently Asked Questions

Add this skill to your agents
Sponsor this space

Reach thousands of developers