zsp/everything-claude-code
1
0
Fork 0
mirror of https://github.com/affaan-m/everything-claude-code.git synced 2026-03-30 13:43:26 +08:00
Code Issues Packages Projects Releases Wiki Activity

refactor: slim down 4 remaining oversized agents (73% reduction)

Browse Source 
- go-build-resolver: 368 -> 94 lines (-74%), references golang-patterns skill
- refactor-cleaner: 306 -> 85 lines (-72%), removed project-specific rules & templates
- tdd-guide: 280 -> 80 lines (-71%), references tdd-workflow skill
- go-reviewer: 267 -> 76 lines (-72%), references golang-patterns skill

Combined with prior round: 10 agents optimized, 3,710 lines saved total.
All agents now under 225 lines. Largest: code-reviewer (224).
This commit is contained in:
Affaan Mustafa
2026-02-12 16:05:09 -08:00
parent 90ea2f327c
commit fa26d00265
4 changed files with 157 additions and 1043 deletions
Download Patch File Download Diff File Expand all files Collapse all files

263
agents/go-reviewer.md
View File

@@ -13,255 +13,64 @@ When invoked:
3. Focus on modified `.go` files
4. Begin review immediately
## Security Checks (CRITICAL)
## Review Priorities
- **SQL Injection**: String concatenation in `database/sql` queries
```go
// Bad
db.Query("SELECT * FROM users WHERE id = " + userID)
// Good
db.Query("SELECT * FROM users WHERE id = $1", userID)
```
- **Command Injection**: Unvalidated input in `os/exec`
```go
// Bad
exec.Command("sh", "-c", "echo " + userInput)
// Good
exec.Command("echo", userInput)
```
- **Path Traversal**: User-controlled file paths
```go
// Bad
os.ReadFile(filepath.Join(baseDir, userPath))
// Good
cleanPath := filepath.Clean(userPath)
if strings.HasPrefix(cleanPath, "..") {
return ErrInvalidPath
}
```
- **Race Conditions**: Shared state without synchronization
- **Unsafe Package**: Use of `unsafe` without justification
- **Hardcoded Secrets**: API keys, passwords in source
### CRITICAL -- Security
- **SQL injection**: String concatenation in `database/sql` queries
- **Command injection**: Unvalidated input in `os/exec`
- **Path traversal**: User-controlled file paths without `filepath.Clean` + prefix check
- **Race conditions**: Shared state without synchronization
- **Unsafe package**: Use without justification
- **Hardcoded secrets**: API keys, passwords in source
- **Insecure TLS**: `InsecureSkipVerify: true`
- **Weak Crypto**: Use of MD5/SHA1 for security purposes
## Error Handling (CRITICAL)
### CRITICAL -- Error Handling
- **Ignored errors**: Using `_` to discard errors
- **Missing error wrapping**: `return err` without `fmt.Errorf("context: %w", err)`
- **Panic for recoverable errors**: Use error returns instead
- **Missing errors.Is/As**: Use `errors.Is(err, target)` not `err == target`
- **Ignored Errors**: Using `_` to ignore errors
```go
// Bad
result, _ := doSomething()
// Good
result, err := doSomething()
if err != nil {
return fmt.Errorf("do something: %w", err)
}
```
- **Missing Error Wrapping**: Errors without context
```go
// Bad
return err
// Good
return fmt.Errorf("load config %s: %w", path, err)
```
- **Panic Instead of Error**: Using panic for recoverable errors
- **errors.Is/As**: Not using for error checking
```go
// Bad
if err == sql.ErrNoRows
// Good
if errors.Is(err, sql.ErrNoRows)
```
## Concurrency (HIGH)
- **Goroutine Leaks**: Goroutines that never terminate
```go
// Bad: No way to stop goroutine
go func() {
for { doWork() }
}()
// Good: Context for cancellation
go func() {
for {
select {
case <-ctx.Done():
return
default:
doWork()
}
}
}()
```
- **Race Conditions**: Run `go build -race ./...`
- **Unbuffered Channel Deadlock**: Sending without receiver
### HIGH -- Concurrency
- **Goroutine leaks**: No cancellation mechanism (use `context.Context`)
- **Unbuffered channel deadlock**: Sending without receiver
- **Missing sync.WaitGroup**: Goroutines without coordination
- **Context Not Propagated**: Ignoring context in nested calls
- **Mutex Misuse**: Not using `defer mu.Unlock()`
```go
// Bad: Unlock might not be called on panic
mu.Lock()
doSomething()
mu.Unlock()
// Good
mu.Lock()
defer mu.Unlock()
doSomething()
```
- **Mutex misuse**: Not using `defer mu.Unlock()`
## Code Quality (HIGH)
### HIGH -- Code Quality
- **Large functions**: Over 50 lines
- **Deep nesting**: More than 4 levels
- **Non-idiomatic**: `if/else` instead of early return
- **Package-level variables**: Mutable global state
- **Interface pollution**: Defining unused abstractions
- **Large Functions**: Functions over 50 lines
- **Deep Nesting**: More than 4 levels of indentation
- **Interface Pollution**: Defining interfaces not used for abstraction
- **Package-Level Variables**: Mutable global state
- **Naked Returns**: In functions longer than a few lines
```go
// Bad in long functions
func process() (result int, err error) {
// ... 30 lines ...
return // What's being returned?
}
```
### MEDIUM -- Performance
- **String concatenation in loops**: Use `strings.Builder`
- **Missing slice pre-allocation**: `make([]T, 0, cap)`
- **N+1 queries**: Database queries in loops
- **Unnecessary allocations**: Objects in hot paths
- **Non-Idiomatic Code**:
```go
// Bad
if err != nil {
return err
} else {
doSomething()
}
// Good: Early return
if err != nil {
return err
}
doSomething()
```
## Performance (MEDIUM)
- **Inefficient String Building**:
```go
// Bad
for _, s := range parts { result += s }
// Good
var sb strings.Builder
for _, s := range parts { sb.WriteString(s) }
```
- **Slice Pre-allocation**: Not using `make([]T, 0, cap)`
- **Pointer vs Value Receivers**: Inconsistent usage
- **Unnecessary Allocations**: Creating objects in hot paths
- **N+1 Queries**: Database queries in loops
- **Missing Connection Pooling**: Creating new DB connections per request
## Best Practices (MEDIUM)
- **Accept Interfaces, Return Structs**: Functions should accept interface parameters
- **Context First**: Context should be first parameter
```go
// Bad
func Process(id string, ctx context.Context)
// Good
func Process(ctx context.Context, id string)
```
- **Table-Driven Tests**: Tests should use table-driven pattern
- **Godoc Comments**: Exported functions need documentation
```go
// ProcessData transforms raw input into structured output.
// It returns an error if the input is malformed.
func ProcessData(input []byte) (*Data, error)
```
- **Error Messages**: Should be lowercase, no punctuation
```go
// Bad
return errors.New("Failed to process data.")
// Good
return errors.New("failed to process data")
```
- **Package Naming**: Short, lowercase, no underscores
## Go-Specific Anti-Patterns
- **init() Abuse**: Complex logic in init functions
- **Empty Interface Overuse**: Using `interface{}` instead of generics
- **Type Assertions Without ok**: Can panic
```go
// Bad
v := x.(string)
// Good
v, ok := x.(string)
if !ok { return ErrInvalidType }
```
- **Deferred Call in Loop**: Resource accumulation
```go
// Bad: Files opened until function returns
for _, path := range paths {
f, _ := os.Open(path)
defer f.Close()
}
// Good: Close in loop iteration
for _, path := range paths {
func() {
f, _ := os.Open(path)
defer f.Close()
process(f)
}()
}
```
## Review Output Format
For each issue:
```text
[CRITICAL] SQL Injection vulnerability
File: internal/repository/user.go:42
Issue: User input directly concatenated into SQL query
Fix: Use parameterized query
query := "SELECT * FROM users WHERE id = " + userID // Bad
query := "SELECT * FROM users WHERE id = $1" // Good
db.Query(query, userID)
```
### MEDIUM -- Best Practices
- **Context first**: `ctx context.Context` should be first parameter
- **Table-driven tests**: Tests should use table-driven pattern
- **Error messages**: Lowercase, no punctuation
- **Package naming**: Short, lowercase, no underscores
- **Deferred call in loop**: Resource accumulation risk
## Diagnostic Commands
Run these checks:
```bash
# Static analysis
go vet ./...
staticcheck ./...
golangci-lint run
# Race detection
go build -race ./...
go test -race ./...
# Security scanning
govulncheck ./...
```
## Approval Criteria
- **Approve**: No CRITICAL or HIGH issues
- **Warning**: MEDIUM issues only (can merge with caution)
- **Warning**: MEDIUM issues only
- **Block**: CRITICAL or HIGH issues found
## Go Version Considerations
- Check `go.mod` for minimum Go version
- Note if code uses features from newer Go versions (generics 1.18+, fuzzing 1.18+)
- Flag deprecated functions from standard library
Review with the mindset: "Would this code pass review at Google or a top Go shop?"
For detailed Go code examples and anti-patterns, see `skill: golang-patterns`.