agentregistry

# AGENTS.md - Development Guidelines

This document provides guidelines for both AI coding assistants (Claude, Copilot, Cursor, etc.) and human developers working on the AgentRegistry codebase.

## Project Overview

AgentRegistry is a centralized registry for securely curating, discovering, deploying, and managing agentic infrastructure including MCP (Model Context Protocol) servers, agents, and skills.

**Tech Stack:**
- **Backend/CLI:** Go 1.26+
- **Database:** PostgreSQL (accessed via pgx)
- **Frontend:** Next.js 14 (App Router) with Tailwind CSS
- **CLI Framework:** Cobra
- **API Framework:** Huma (OpenAPI)

---

## Architecture

### Directory Structure

```
agentregistry/
├── cmd/           # Entry points only - minimal code
│   ├── cli/       # CLI entry point
│   └── server/    # Server entry point
├── pkg/           # Public, reusable packages
├── internal/      # Private implementation
│   ├── registry/  # Core registry implementation
│   │   ├── api/       # HTTP handlers
│   │   ├── database/  # Database layer (pgx)
│   │   ├── service/   # Business logic
│   │   └── ...
│   ├── cli/       # CLI command implementations
│   └── mcp/       # MCP protocol handling
├── ui/            # Next.js frontend
└── docker/        # Container images and local Compose configuration
```

### Layer Responsibilities

1. **cmd/** - Entry points only. Delegate immediately to pkg/ or internal/
2. **pkg/** - Public APIs for external consumption and reusability
3. **internal/** - Implementation details, not importable by external packages
4. **internal/registry/database/** - **ONLY** place that accesses the database directly
5. **internal/registry/service/** - Business logic, receives database interface via constructor

### Runtime Ownership

Deployment/runtime code should be organized by **clear ownership**, not by vague helper layers:

1. **`internal/registry/runtimes/<runtime>/` owns runtime behavior** - concrete packages such as kubernetes should contain their adapter plus the runtime-specific materialization/apply/discovery logic they need
2. **`internal/registry/runtimes/utils/` is for narrowly shared deployment utilities only** - use it for adapter-shared materialization helpers, validation, name generation, and request parsing that are truly cross-runtime
3. **`internal/registry/runtimes/types/` is for shared contracts only** - keep shared schemas and DTO-style runtime types here, not behavior-heavy logic
4. **`internal/registry/api/handlers/` is transport only** - HTTP handlers should parse requests, call services/adapters, and map errors; they should not own deployment/runtime behavior
5. **`internal/registry/registry_app.go` is the composition root** - wire concrete runtime adapters here explicitly instead of hiding registration/factory behavior in handler packages

Avoid introducing broad "translator" layers or catch-all shared packages when the code really belongs to one concrete runtime. Prefer concentrated runtime packages with small, explicit shared utilities.

---

## Critical Rules

### Database Access

**The database MUST only be accessed by:**
1. The database layer (`internal/registry/database/`)
2. The authorizer component

**No other component should have direct database access.** All data operations must go through the appropriate interfaces.

```go
// CORRECT: Service receives database interface
type RegistryService struct {
    db DatabaseInterface
}

func NewRegistryService(db DatabaseInterface) *RegistryService {
    return &RegistryService{db: db}
}

// INCORRECT: Service creates its own database connection
type RegistryService struct {
    db *pgxpool.Pool  // Direct database access - DO NOT DO THIS
}
```

### Authorization

Authz is enforced at the **database layer** by default — every store method calls `s.authz.Check(...)` before the query. Services don't normally need to invoke authz themselves; the DB call fails with `auth.ErrForbidden` when a caller lacks permission.

**When to gate at the API or service layer instead:** only when the operation doesn't reach the DB with a check. Current cases:

- External runtime calls with no downstream DB write — e.g. `UndeployDeployment` and `CancelDeployment` hit adapters before any DB update, so the gate has to fire in the service before the adapter call.
- Admin-scope handlers with no per-resource authz — call `authz.IsRegistryAdmin` directly in the handler.

**List operations intentionally skip per-row authz checks.** The DB's `List*` methods return what matches the SQL filter; they do not invoke `authz.Check` per row. The `AuthzProvider` interface only gates single-resource operations (`Check`, `IsRegistryAdmin`) — it has no row-filter hook. Per-row visibility filtering for Lists would require a custom `database.Store` implementation wired in at the composition root (`registry_app.go`), either joining against a permissions table in SQL or calling `authz.Check` per row.

Prefer DB-layer gates. If you add an API-layer gate, document the reason in the handler or service comment.

See [docs/auth/authz-matrix.md](./docs/auth/authz-matrix.md) for the per-endpoint permission table.

### Interface Design

Every significant component must define an interface for its dependencies. This enables:
- Unit testing with mocks
- Loose coupling between packages
- Clear contract definitions

```go
// Define interfaces for dependencies
type AgentRepository interface {
    GetAgent(ctx context.Context, id string) (*Agent, error)
    ListAgents(ctx context.Context, opts ListOptions) ([]Agent, error)
    CreateAgent(ctx context.Context, agent *Agent) error
}

// Implementation receives interface, not concrete type
type AgentService struct {
    repo AgentRepository
}
```

### Single Responsibility

Each package and file should have one clear purpose. Signs of mixed responsibilities:
- Files over 500 lines
- Packages importing many unrelated dependencies
- Functions doing multiple unrelated things

**Split large components into focused units.**

---

## Error Handling

Use standard Go error patterns with wrapping:

```go
import (
    "errors"
    "fmt"
)

// Wrap errors with context
func (s *Service) GetAgent(ctx context.Context, id string) (*Agent, error) {
    agent, err := s.repo.GetAgent(ctx, id)
    if err != nil {
        return nil, fmt.Errorf("getting agent %s: %w", id, err)
    }
    return agent, nil
}

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

// Define sentinel errors for domain-specific cases
var (
    ErrNotFound     = errors.New("not found")
    ErrUnauthorized = errors.New("unauthorized")
)
```

**Consistency requirements:**
- Always wrap errors with `fmt.Errorf("context: %w", err)`
- Use lowercase error messages (they may be wrapped)
- Define sentinel errors for cases that callers need to check

---

## Logging

Use the `github.com/agentregistry-dev/agentregistry/pkg/logging` package for structured logging. Loggers should be package scoped in most cases, but the global logger can be directly used via the `slog` package if necessary. `logging.New` keeps track of `slog.LevelVar` to allow log levels to be changed at runtime via the `/logging` HTTP endpoint, so calling `logging.New*` within a re-entrant function will leak memory and should be avoided. If `logging.New*` is invoked within a re-entrant function, the tracked leveler should be explicitly deleted by a call to `logging.DeleteLeveler("component-name")` before the function returns.

```go
import (
    "log/slog"
    "github.com/agentregistry-dev/agentregistry/pkg/logging"
)

var logger = logging.New("my-component")

// package scoped logger
logger.Info("agent created",
    "agent_id", agent.ID,
    "name", agent.Name,
)

// global logger
slog.Error("failed to create agent",
    "error", err,
    "agent_name", name,
)
```

---

## Testing

### Requirements

The codebase needs significantly more test coverage. When adding or modifying code:

1. **Write unit tests with mocks** for business logic
2. **Write table-driven tests** for functions with multiple cases
3. **Write integration tests** for database and API operations

### Table-Driven Tests

```go
func TestValidateAgentName(t *testing.T) {
    tests := []struct {
        name    string
        input   string
        wantErr bool
    }{
        {"valid name", "my-agent", false},
        {"empty name", "", true},
        {"too long", strings.Repeat("a", 256), true},
        {"special chars", "my@agent", true},
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            err := ValidateAgentName(tt.input)
            if (err != nil) != tt.wantErr {
                t.Errorf("ValidateAgentName(%q) error = %v, wantErr %v",
                    tt.input, err, tt.wantErr)
            }
        })
    }
}
```

### Mock-Based Unit Tests

```go
type mockAgentRepo struct {
    agents map[string]*Agent
}

func (m *mockAgentRepo) GetAgent(ctx context.Context, id string) (*Agent, error) {
    if agent, ok := m.agents[id]; ok {
        return agent, nil
    }
    return nil, ErrNotFound
}

func TestAgentService_GetAgent(t *testing.T) {
    repo := &mockAgentRepo{
        agents: map[string]*Agent{
            "agent-1": {ID: "agent-1", Name: "Test Agent"},
        },
    }
    svc := NewAgentService(repo)

    agent, err := svc.GetAgent(context.Background(), "agent-1")
    if err != nil {
        t.Fatalf("unexpected error: %v", err)
    }
    if agent.Name != "Test Agent" {
        t.Errorf("got name %q, want %q", agent.Name, "Test Agent")
    }
}
```

### Running Tests

```bash
# Run unit tests (no database needed)
make test-unit

# Run tests, including Postgres-backed integration tests (requires Postgres: make run-docker)
make test

# Run with coverage
make test-coverage

# Run with HTML coverage report
make test-coverage-report
```

---

## Dependency Injection

Use manual constructor injection. No DI frameworks.

```go
// Define what you need
type AgentService struct {
    repo   AgentRepository
    logger *slog.Logger
}

// Accept dependencies via constructor
func NewAgentService(repo AgentRepository, logger *slog.Logger) *AgentService {
    return &AgentService{
        repo:   repo,
        logger: logger,
    }
}

// Wire up in main or initialization code
func main() {
    db := database.NewPostgresDB(connString)
    logger := slog.Default()

    agentRepo := database.NewAgentRepository(db)
    agentSvc := service.NewAgentService(agentRepo, logger)

    // ...
}
```

---

## CLI Development

Use Cobra for CLI commands. Follow existing patterns in `internal/cli/`.

```go
var agentCmd = &cobra.Command{
    Use:   "agent",
    Short: "Manage agents",
}

var agentListCmd = &cobra.Command{
    Use:   "list",
    Short: "List all agents",
    RunE: func(cmd *cobra.Command, args []string) error {
        // Implementation
        return nil
    },
}

func init() {
    agentCmd.AddCommand(agentListCmd)
}
```

### CLI Output

Use the `printer` package (`pkg/printer`) for user-facing output instead of raw `fmt.Printf`:

```go
printer.PrintSuccess("Added skill 'my-skill' to agent.yaml")
printer.PrintInfo("Processing...")
printer.PrintError("something went wrong")
```

---

## API Development

Use Huma for REST APIs. Huma generates OpenAPI documentation automatically.

```go
// Define input/output types
type GetAgentInput struct {
    ID string `path:"id"`
}

type AgentOutput struct {
    Body Agent
}

// Register routes
huma.Get(api, "/agents/{id}", func(ctx context.Context, input *GetAgentInput) (*AgentOutput, error) {
    agent, err := svc.GetAgent(ctx, input.ID)
    if err != nil {
        return nil, err
    }
    return &AgentOutput{Body: *agent}, nil
})
```

---

## Frontend Development

Use Next.js App Router patterns with React Server Components where appropriate.

```tsx
// app/agents/page.tsx
export default async function AgentsPage() {
    const agents = await fetchAgents();
    return (
        <div>
            {agents.map(agent => (
                <AgentCard key={agent.id} agent={agent} />
            ))}
        </div>
    );
}
```

Use shadcn/ui components. Check `ui/components/ui/` for available components.

---

## AI Assistant Guidelines

When working with this codebase, AI assistants should:

### Do

1. **Read before writing** - Always read existing code before suggesting modifications
2. **Follow existing patterns** - Match the style of surrounding code
3. **Add tests** - Include tests for any new functionality
4. **Use interfaces** - Define interfaces for new dependencies
5. **Keep changes minimal** - Only modify what's necessary for the task
6. **Check database access** - Ensure database operations go through proper layers

### Don't

1. **Access database directly** - Never add direct database access outside the database layer
2. **Create god objects** - Keep components focused and small
3. **Skip error handling** - Always handle and wrap errors appropriately
4. **Add unnecessary abstractions** - Don't over-engineer solutions
5. **Ignore existing interfaces** - Use defined interfaces, don't bypass them
6. **Create new files unnecessarily** - Prefer editing existing files

### When Adding New Features

1. Check if similar functionality exists
2. Identify the appropriate layer (api/service/database)
3. Define interfaces for new dependencies
4. Implement with proper error handling
5. Add unit tests with mocks
6. Add integration tests if touching database/API
7. Update any affected documentation

### When Fixing Bugs

1. Write a failing test that reproduces the bug
2. Fix the bug with minimal changes
3. Verify the test passes
4. Check for similar issues elsewhere
5. Don't refactor unrelated code

---

## Pull Requests

When creating or updating a pull request in this repo, use `.github/PULL_REQUEST_TEMPLATE.md` as the source of truth for the PR body.

- Use the **exact top-level headings** from the template (`# Description`, `# Change Type`, `# Changelog`). The CI labeler (`kgateway-dev/pr-kind-labeler`) enforces the presence of a `# Description` section and will fail if headings use `##` or are missing.
- Include at least one `/kind ...` entry in the Change Type section.
- Fill the `release-note` block exactly as required by the template. If no release note applies, write `NONE`.
- When the PR is intended to close an issue, include `Fixes #<issue>` in the description so GitHub links the issue correctly.

If an agent opens a PR without following the template, it should update the PR body before considering the task complete.

---

## Code Review Checklist

- [ ] Database access only through database layer or authorizer
- [ ] New dependencies injected via constructor
- [ ] Interfaces defined for mockability
- [ ] Errors wrapped with context
- [ ] Unit tests with mocks included
- [ ] Table-driven tests for multiple cases
- [ ] No mixed responsibilities in components
- [ ] No hardcoded values that should be configurable

---

## Quick Reference

| Task | Command |
|------|---------|
| Build CLI | `make build-cli` |
| Build Server | `make build-server` |
| Run Unit Tests | `make test-unit` |
| Run all Tests | `make test` |
| Run Tests w/ Coverage | `make test-coverage` |
| Coverage HTML Report | `make test-coverage-report` |
| Run Linter | `make lint` |
| Format Code | `make fmt` |
| Build UI | `make build-ui` |
| Dev UI | `make dev-ui` |
| Run Docker Environment | `make run-docker` |
| Stop Local Environment | `make down` |

---

## Related Documentation

- [README.md](./README.md) - Project overview and quick start
- [DEVELOPMENT.md](./DEVELOPMENT.md) - Architecture details
- [CONTRIBUTING.md](./CONTRIBUTING.md) - Contribution guidelines
- [docs/auth/authz-matrix.md](./docs/auth/authz-matrix.md) - AuthZ matrix for HTTP endpoints