---
name: software-architecture
description: Software architecture best practices, patterns, and review guidance. Foundation for architecture-related agents and commands including repo audits, issue refinement, and PR reviews.
user-invocable: false
---

# Software Architecture

Best practices, patterns, and review guidance for software architecture. This skill serves as the knowledge base for architecture-related agents and commands.

## Go-Specific Best Practices

### Package Organization

**Good package structure:**
```
project/
├── cmd/                    # Application entry points
│   └── server/
│       └── main.go
├── internal/               # Private packages
│   ├── domain/             # Core business logic
│   │   ├── user/
│   │   └── order/
│   ├── service/            # Application services
│   ├── repository/         # Data access
│   └── handler/            # HTTP/gRPC handlers
├── pkg/                    # Public, reusable packages
└── go.mod
```

**Package naming:**
- Short, concise, lowercase: `user`, `order`, `auth`
- Avoid generic names: `util`, `common`, `helpers`, `misc`
- Name after what it provides, not what it contains
- One package per concept, not per file

**Package cohesion:**
- A package should have a single, focused responsibility
- Package internal files can use internal types freely
- Minimize exported types - export interfaces, hide implementations

### Interfaces

**Accept interfaces, return structs:**
```go
// Good: Accept interface, return concrete type
func NewUserService(repo UserRepository) *UserService {
    return &UserService{repo: repo}
}

// Bad: Accept and return interface
func NewUserService(repo UserRepository) UserService {
    return &userService{repo: repo}
}
```

**Define interfaces at point of use:**
```go
// Good: Interface defined where it's used (consumer owns the interface)
package service

type UserRepository interface {
    FindByID(ctx context.Context, id string) (*User, error)
}

// Bad: Interface defined with implementation (producer owns the interface)
package repository

type UserRepository interface {
    FindByID(ctx context.Context, id string) (*User, error)
}
```

**Keep interfaces small:**
- Prefer single-method interfaces
- Large interfaces indicate missing abstraction
- Compose small interfaces when needed

### Error Handling

**Wrap errors with context:**
```go
// Good: Wrap with context
if err != nil {
    return fmt.Errorf("fetching user %s: %w", id, err)
}

// Bad: Return bare error
if err != nil {
    return err
}
```

**Use sentinel errors for expected conditions:**
```go
var ErrNotFound = errors.New("not found")
var ErrConflict = errors.New("conflict")

// Check with errors.Is
if errors.Is(err, ErrNotFound) {
    // handle not found
}
```

**Error types for rich errors:**
```go
type ValidationError struct {
    Field   string
    Message string
}

func (e *ValidationError) Error() string {
    return fmt.Sprintf("%s: %s", e.Field, e.Message)
}

// Check with errors.As
var valErr *ValidationError
if errors.As(err, &valErr) {
    // handle validation error
}
```

### Dependency Injection

**Constructor injection:**
```go
type UserService struct {
    repo   UserRepository
    logger Logger
}

func NewUserService(repo UserRepository, logger Logger) *UserService {
    return &UserService{
        repo:   repo,
        logger: logger,
    }
}
```

**Wire dependencies in main:**
```go
func main() {
    // Create dependencies
    db := database.Connect()
    logger := slog.Default()

    // Wire up services
    userRepo := repository.NewUserRepository(db)
    userService := service.NewUserService(userRepo, logger)
    userHandler := handler.NewUserHandler(userService)

    // Start server
    http.Handle("/users", userHandler)
    http.ListenAndServe(":8080", nil)
}
```

**Avoid global state:**
- No `init()` for service initialization
- No package-level variables for dependencies
- Pass context explicitly, don't store in structs

### Testing

**Table-driven tests:**
```go
func TestUserService_Create(t *testing.T) {
    tests := []struct {
        name    string
        input   CreateUserInput
        want    *User
        wantErr error
    }{
        {
            name:  "valid user",
            input: CreateUserInput{Email: "test@example.com"},
            want:  &User{Email: "test@example.com"},
        },
        {
            name:    "invalid email",
            input:   CreateUserInput{Email: "invalid"},
            wantErr: ErrInvalidEmail,
        },
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            // arrange, act, assert
        })
    }
}
```

**Test doubles:**
- Use interfaces for test doubles
- Prefer hand-written mocks over generated ones for simple cases
- Use `testify/mock` or `gomock` for complex mocking needs

**Test package naming:**
- `package user_test` for black-box testing (preferred)
- `package user` for white-box testing when needed

## Generic Architecture Patterns

### Layered Architecture

```
┌─────────────────────────────────┐
│         Presentation            │  HTTP handlers, CLI, gRPC
├─────────────────────────────────┤
│         Application             │  Use cases, orchestration
├─────────────────────────────────┤
│           Domain                │  Business logic, entities
├─────────────────────────────────┤
│        Infrastructure           │  Database, external services
└─────────────────────────────────┘
```

**Rules:**
- Dependencies point downward only
- Upper layers depend on interfaces, not implementations
- Domain layer has no external dependencies

### SOLID Principles

**Single Responsibility (S):**
- Each module has one reason to change
- Split code that changes for different reasons

**Open/Closed (O):**
- Open for extension, closed for modification
- Add new behavior through new types, not changing existing ones

**Liskov Substitution (L):**
- Subtypes must be substitutable for their base types
- Interfaces should be implementable without surprises

**Interface Segregation (I):**
- Clients shouldn't depend on interfaces they don't use
- Prefer many small interfaces over few large ones

**Dependency Inversion (D):**
- High-level modules shouldn't depend on low-level modules
- Both should depend on abstractions

### Dependency Direction

```
                    ┌──────────────┐
                    │    Domain    │
                    │  (no deps)   │
                    └──────────────┘
                           ▲
              ┌────────────┴────────────┐
              │                         │
      ┌───────┴───────┐         ┌───────┴───────┐
      │  Application  │         │Infrastructure │
      │ (uses domain) │         │(implements    │
      └───────────────┘         │ domain intf)  │
              ▲                 └───────────────┘
              │
      ┌───────┴───────┐
      │ Presentation  │
      │(calls app)    │
      └───────────────┘
```

**Key insight:** Infrastructure implements domain interfaces, doesn't define them. This inverts the "natural" dependency direction.

### Module Boundaries

**Signs of good boundaries:**
- Modules can be understood in isolation
- Changes are localized within modules
- Clear, minimal public API
- Dependencies flow in one direction

**Signs of bad boundaries:**
- Circular dependencies between modules
- "Shotgun surgery" - small changes require many file edits
- Modules reach into each other's internals
- Unclear ownership of concepts

## Repository Health Indicators

### Positive Indicators

| Indicator | What to Look For |
|-----------|------------------|
| Clear structure | Obvious package organization, consistent naming |
| Small interfaces | Most interfaces have 1-3 methods |
| Explicit dependencies | Constructor injection, no globals |
| Test coverage | Unit tests for business logic, integration tests for boundaries |
| Error handling | Wrapped errors, typed errors for expected cases |
| Documentation | CLAUDE.md accurate, code comments explain "why" |

### Warning Signs

| Indicator | What to Look For |
|-----------|------------------|
| God packages | `utils/`, `common/`, `helpers/` with 20+ files |
| Circular deps | Package A imports B, B imports A |
| Deep nesting | 4+ levels of directory nesting |
| Huge files | Files with 500+ lines |
| Interface pollution | Interfaces for everything, even single implementations |
| Global state | Package-level variables, `init()` for setup |

### Metrics to Track

- **Package fan-out:** How many packages does each package import?
- **Cyclomatic complexity:** How complex are the functions?
- **Test coverage:** What percentage of code is tested?
- **Import depth:** How deep is the import tree?

## Review Checklists

### Repository Audit Checklist

Use this when evaluating overall repository health.

**Structure:**
- [ ] Clear package organization following Go conventions
- [ ] No circular dependencies between packages
- [ ] Appropriate use of `internal/` for private packages
- [ ] `cmd/` for application entry points

**Dependencies:**
- [ ] Dependencies flow inward (toward domain)
- [ ] Interfaces defined at point of use (not with implementation)
- [ ] No global state or package-level dependencies
- [ ] Constructor injection throughout

**Code Quality:**
- [ ] Consistent naming conventions
- [ ] No "god" packages (utils, common, helpers)
- [ ] Errors wrapped with context
- [ ] Small, focused interfaces

**Testing:**
- [ ] Unit tests for domain logic
- [ ] Integration tests for boundaries (DB, HTTP)
- [ ] Tests are readable and maintainable
- [ ] Test coverage for critical paths

**Documentation:**
- [ ] CLAUDE.md is accurate and helpful
- [ ] vision.md explains the product purpose
- [ ] Code comments explain "why", not "what"

### Issue Refinement Checklist

Use this when reviewing issues for architecture impact.

**Scope:**
- [ ] Issue is a vertical slice (user-visible value)
- [ ] Changes are localized to specific packages
- [ ] No cross-cutting concerns hidden in implementation

**Design:**
- [ ] Follows existing patterns in the codebase
- [ ] New abstractions are justified
- [ ] Interface changes are backward compatible (or breaking change is documented)

**Dependencies:**
- [ ] New dependencies are minimal and justified
- [ ] No new circular dependencies introduced
- [ ] Integration points are clearly defined

**Testability:**
- [ ] Acceptance criteria are testable
- [ ] New code can be unit tested in isolation
- [ ] Integration test requirements are clear

### PR Review Checklist

Use this when reviewing pull requests for architecture concerns.

**Structure:**
- [ ] Changes respect existing package boundaries
- [ ] New packages follow naming conventions
- [ ] No new circular dependencies

**Interfaces:**
- [ ] Interfaces are defined where used
- [ ] Interfaces are minimal and focused
- [ ] Breaking interface changes are justified

**Dependencies:**
- [ ] Dependencies injected via constructors
- [ ] No new global state
- [ ] External dependencies properly abstracted

**Error Handling:**
- [ ] Errors wrapped with context
- [ ] Sentinel errors for expected conditions
- [ ] Error types for rich error information

**Testing:**
- [ ] New code has appropriate test coverage
- [ ] Tests are clear and maintainable
- [ ] Edge cases covered

## Anti-Patterns to Flag

### God Packages

**Problem:** Packages like `utils/`, `common/`, `helpers/` become dumping grounds.

**Symptoms:**
- 20+ files in one package
- Unrelated functions grouped together
- Package imported by everything

**Fix:** Extract cohesive packages based on what they provide: `validation`, `httputil`, `timeutil`.

### Circular Dependencies

**Problem:** Package A imports B, and B imports A (directly or transitively).

**Symptoms:**
- Import cycle compile errors
- Difficulty understanding code flow
- Changes cascade unexpectedly

**Fix:**
- Extract shared types to a third package
- Use interfaces to invert dependency
- Merge packages if truly coupled

### Leaky Abstractions

**Problem:** Implementation details leak through abstraction boundaries.

**Symptoms:**
- Database types in domain layer
- HTTP types in service layer
- Framework types in business logic

**Fix:** Define types at each layer, map between them explicitly.

### Anemic Domain Model

**Problem:** Domain objects are just data containers, logic is elsewhere.

**Symptoms:**
- Domain types have only getters/setters
- All logic in "service" classes
- Domain types can be in invalid states

**Fix:** Put behavior with data. Domain types should enforce their own invariants.

### Shotgun Surgery

**Problem:** Small changes require editing many files across packages.

**Symptoms:**
- Feature adds touch 10+ files
- Similar changes in multiple places
- Copy-paste between packages

**Fix:** Consolidate related code. If things change together, they belong together.

### Feature Envy

**Problem:** Code in one package is more interested in another package's data.

**Symptoms:**
- Many calls to another package's methods
- Pulling data just to compute something
- Logic that belongs elsewhere

**Fix:** Move the code to where the data lives, or extract the behavior to a shared place.

### Premature Abstraction

**Problem:** Creating interfaces and abstractions before they're needed.

**Symptoms:**
- Interfaces with single implementations
- "Factory" and "Manager" classes everywhere
- Configuration for things that never change

**Fix:** Write concrete code first. Extract abstractions when you have multiple implementations or need to break dependencies.

### Deep Hierarchy

**Problem:** Excessive layers of abstraction or inheritance.

**Symptoms:**
- 5+ levels of embedding/composition
- Hard to trace code flow
- Changes require understanding many layers

**Fix:** Prefer composition over inheritance. Flatten hierarchies where possible.