# Patterns & Conventions

What I've learned about how we write code in this project.

## Go Style

### Loop Idiom
Always `range N` for integer loops. Never `for i := 0; i < N; i++`. This is a hard rule Fritz set early.

### Table-Driven Tests
Every test file uses `t.Run` with named subtests. The pattern:
```go
tests := []struct {
    name string
    // inputs...
    // expected...
}{
    {"descriptive name", ...},
}
for _, tt := range tests {
    t.Run(tt.name, func(t *testing.T) {
        // test body
    })
}
```

### Mock Streams for Handlers
Handler tests construct a `bytes.Buffer` with a raw request, pass it as a stream, and read the response. No QUIC, no network, fast tests.

### Interface-Based Mocking for MCP Handlers
The MCP handler uses a `markClient` interface instead of `*fetch.Client` directly. This enables testing handler logic (like version auto-resolution) without a QUIC connection. Stub implementations in tests configure only the methods under test.

### t.TempDir() for Fixtures
Never create test fixtures in the working directory. Always `t.TempDir()` — Go cleans it up automatically.

## Build & Test

```bash
make all        # Build everything
make test       # Run all tests
make fmt        # Format
make vet        # Vet
make lint       # Run golangci-lint

# Single module / single test
cd server && go test -run TestHandleFetch/path_traversal_blocked ./internal/handler/

# Dev server
./server/bin/demarkus-server -root ./docs/site
```

### IMPORTANT: Build Output Rule

**Never run bare `go build ./cmd/<name>/`** — this drops binaries in the current directory, polluting the repo. Always use one of:

- `make client` / `make server` / `make all` — preferred
- `go build -o bin/<name> ./cmd/<name>/` — if building manually
- `go vet ./cmd/<name>/` — if you just need to check compilation without producing a binary

Binaries belong in `bin/` directories only. Stray binaries in source directories get accidentally committed.

**After completing a task**: Run `bash pre-commit.sh` to format, vet, and lint all modules before committing.

## Development Workflow

### Pre-Commit
Run `bash pre-commit.sh` before committing. Formats, vets, and lints all modules.

### Conventional Commits
Module-scoped: `feat(server): description`, `fix(client): description`. This drives auto-versioning and release tagging (`server/v0.1.0`, etc.).

### Git Commits
Fritz handles all commits himself. Never commit on his behalf — just prepare the changes and let him know what's ready.

### CI/CD
Tags: `server/v0.1.0`, `client/v0.1.0`, `protocol/v0.1.0`. Push to main triggers auto-release. CI runs test, vet, and golangci-lint for each module.

## Architecture (Quick Reference)

Go monorepo, four modules with local `replace` directives:

- `protocol/` — wire format types, parsing, serialization (no network code)
- `server/` — QUIC server (depends on protocol)
- `client/` — CLI, TUI, MCP server (depends on protocol)
- `tools/` — dev utilities

Protocol constants: port `6309`, ALPN `"mark"`, scheme `mark://`.

See [Architecture](/architecture.md) for full details.

## Core Invariants

- **Version immutability**: every write creates a new version, published versions are permanent
- **Security**: no tracking, no telemetry, encrypted transport, capability-based auth

## Philosophy

### Small and Incremental
Every change should be the smallest working increment. Get something tested and working before moving on. Don't batch up large changes.

### Robustness First
Handle the error. Test the edge case. Make it correct before making it elegant.

### Simplest Solution
Short functions, clear names, obvious flow. If I find myself writing a comment to explain what code does, the code should be rewritten to not need the comment. Comments explain *why*, not *what*.

### No Over-Engineering
Don't add features beyond what's asked. Don't refactor surroundings while fixing a bug. Don't add abstractions for one-time operations. Three similar lines are better than a premature helper function.

## What I've Learned About Working With Fritz

Fritz values directness. Short answers over long explanations. Working code over architecture astronautics. He'll push back on unnecessary complexity and he's usually right when he does. The best sessions are when we move fast through small, clean changes — each one tested, each one committed. Momentum matters.

## GitHub Pages

The `pages` branch of this repo is the GitHub Pages site. Documentation changes that should be published to the website need to be pushed to the `pages` branch, not `main`.


## Protocol Patterns

### MCP APPEND: Auto-Resolved Versioning

The `mark_append` MCP tool now auto-resolves `expected_version` when omitted or 0. Agents no longer need to manually call VERSIONS before appending — the tool handles it internally:

1. Agent calls `mark_append` with just `url` and `body`
2. Tool internally calls **VERSIONS** → parses `current` from frontmatter
3. Tool calls **APPEND** with the resolved version

Optimistic concurrency is still enforced — the server always receives an `expected_version`. The difference is who provides it:
- **Omitted/0**: tool auto-resolves via VERSIONS (one extra round trip, but invisible to the agent)
- **Explicit**: agent passes it directly (no extra round trip, useful when the agent already knows the version from a prior fetch)

This does NOT apply to `mark_publish` — publish requires the agent to have read the document first, so it should always know the version.

### APPEND: Getting the Latest Version (CLI/Library)

For non-MCP clients that use APPEND directly, follow this pattern to get the version without fetching the full document:

1. **VERSIONS** /path → parse frontmatter, extract `current` field
2. **APPEND** with `expected_version: current`

VERSIONS response always includes `current` in its frontmatter metadata.

Example:
```
VERSIONS /journal.md
→ frontmatter: { "status": "ok", "current": "5", "total": "5", ... }
→ APPEND /journal.md with expected_version=5
```


## Tech Debt

### graphstore Save() lock scope
`Save()` holds `RLock` across JSON marshal + disk I/O. Fine while Save is only called from `CrawlAndPersist` (infrequent, user-triggered). If concurrent or periodic saves are added later, snapshot state under lock and write outside it to avoid blocking `Merge()` during I/O. Two concurrent `Save()` calls would also race on the `.tmp` file. Comment added in code at `client/internal/graphstore/store.go`.