|
| 1 | +# AGENTS.md |
| 2 | + |
| 3 | +Guidance for coding agents working in `github.com/ncode/twamp`. |
| 4 | +This repository is a Go TWAMP implementation with strict RFC semantics and strong test coverage. |
| 5 | + |
| 6 | +## Scope and priorities |
| 7 | + |
| 8 | +1. Keep changes small, local, and protocol-correct. |
| 9 | +2. Preserve RFC behavior (especially MBZ fields, mode negotiation, and message sizes). |
| 10 | +3. Treat RFC 4656, RFC 5357, RFC 5618, RFC 5938, and RFC 6038 as mandatory constraints for all protocol-affecting changes. |
| 11 | +4. Prefer readable code over clever abstractions. |
| 12 | +5. Add or update tests for behavior changes. |
| 13 | +6. Never weaken security checks for convenience. |
| 14 | + |
| 15 | +## Toolchain and environment |
| 16 | + |
| 17 | +- Go version: `1.25.x` (module uses `go 1.25.7`). |
| 18 | +- Module path: `github.com/ncode/twamp`. |
| 19 | +- Main packages: `client`, `server`, `messages`, `common`, `crypto`, `metrics`, `logging`. |
| 20 | +- Integration tests: `test/integration`. |
| 21 | + |
| 22 | +## Build, lint, and test commands |
| 23 | + |
| 24 | +Run from repository root. |
| 25 | + |
| 26 | +### Dependency/bootstrap |
| 27 | + |
| 28 | +- Download modules: `go mod download` |
| 29 | +- Verify module files: `go mod tidy` |
| 30 | + |
| 31 | +### Build |
| 32 | + |
| 33 | +- Build all packages (CI build parity): `go build ./...` |
| 34 | +- Build one package: `go build ./client` |
| 35 | + |
| 36 | +### Lint/format/static checks |
| 37 | + |
| 38 | +- Format all code: `go fmt ./...` |
| 39 | +- Vet all packages: `go vet ./...` |
| 40 | +- Optional extra strictness (if installed): `staticcheck ./...` |
| 41 | + |
| 42 | +### Tests |
| 43 | + |
| 44 | +- Run all tests: `go test ./...` |
| 45 | +- Run with race detector: `go test -race ./...` |
| 46 | +- Run with verbose output: `go test -v ./...` |
| 47 | + |
| 48 | +### Run a single test (important) |
| 49 | + |
| 50 | +- Single test in a package: |
| 51 | + - `go test ./client -run '^TestConnect$'` |
| 52 | +- Single subtest: |
| 53 | + - `go test ./common -run '^TestValidateRequestedMode$/mixed_authenticated_with_extensions$'` |
| 54 | +- Multiple matching tests by regex: |
| 55 | + - `go test ./server -run 'TestHandleRequest|TestStart'` |
| 56 | +- Single package, single count (disable cache): |
| 57 | + - `go test ./messages -run '^TestServerGreeting_Unmarshal$' -count=1` |
| 58 | + |
| 59 | +### Integration tests |
| 60 | + |
| 61 | +- All integration tests: `go test -v ./test/integration` |
| 62 | +- Single integration test: |
| 63 | + - `go test ./test/integration -run '^TestRFC6038' -count=1` |
| 64 | +- perfSONAR helper script: |
| 65 | + - `chmod +x test/integration/perfsonar/run.sh` |
| 66 | + - `INTEROP_DIAGNOSTICS_DIR="$(pwd)/test-results/perfsonar-interop" test/integration/perfsonar/run.sh` |
| 67 | + |
| 68 | +### Coverage and benchmarks |
| 69 | + |
| 70 | +- Coverage profile (repo-wide): |
| 71 | + - `go test -race -coverpkg=./... ./... -coverprofile=coverage.out -covermode=atomic` |
| 72 | +- View coverage report: `go tool cover -html=coverage.out` |
| 73 | +- Benchmarks: `go test -bench . -benchmem ./...` |
| 74 | + |
| 75 | +## Code style guidelines |
| 76 | + |
| 77 | +Follow existing patterns in the touched package first. |
| 78 | + |
| 79 | +### Imports |
| 80 | + |
| 81 | +- Group imports in Go standard style: |
| 82 | + 1. standard library |
| 83 | + 2. blank line |
| 84 | + 3. internal/external module imports |
| 85 | +- Do not use dot imports. |
| 86 | +- Use import aliases only when required to avoid collisions or improve clarity. |
| 87 | + |
| 88 | +### Formatting and layout |
| 89 | + |
| 90 | +- Always keep files `gofmt`-clean. |
| 91 | +- Prefer small functions with clear control flow. |
| 92 | +- Keep protocol constants near related types/functions. |
| 93 | +- Use comments for non-obvious protocol rules or invariants, not obvious code. |
| 94 | + |
| 95 | +### Types and data modeling |
| 96 | + |
| 97 | +- Use domain types from `common` (e.g., `common.Mode`, `common.SessionID`, `common.TWAMPTimestamp`) instead of raw primitives when available. |
| 98 | +- Prefer concrete types over `interface{}` unless polymorphism is needed. |
| 99 | +- Keep zero-value behavior safe and explicit in config constructors (`NewClient`, `NewServer`). |
| 100 | +- Use typed constants for protocol values and accept codes. |
| 101 | + |
| 102 | +### Naming conventions |
| 103 | + |
| 104 | +- Exported identifiers: `PascalCase` with clear domain meaning. |
| 105 | +- Unexported identifiers: `camelCase`. |
| 106 | +- Sentinel errors: `ErrXxx` in package-level `var` blocks. |
| 107 | +- Constructors: `NewXxx`. |
| 108 | +- Test names: `TestXxx`, table entries with descriptive `name` fields. |
| 109 | + |
| 110 | +### Error handling |
| 111 | + |
| 112 | +- Return errors; do not panic in library code. |
| 113 | +- Wrap lower-level errors with context using `%w` (e.g., `fmt.Errorf("failed to parse greeting: %w", err)`). |
| 114 | +- Preserve sentinel semantics so callers can use `errors.Is`. |
| 115 | +- Prefer fail-closed behavior for auth/crypto validation. |
| 116 | +- For combined cleanup failures, use `errors.Join` where appropriate. |
| 117 | + |
| 118 | +### Concurrency and context |
| 119 | + |
| 120 | +- Propagate `context.Context` through network operations. |
| 121 | +- Respect timeouts/deadlines; avoid goroutine leaks. |
| 122 | +- Protect shared mutable state with mutexes/atomics as existing code does. |
| 123 | +- Validate concurrency changes with `go test -race ./...`. |
| 124 | + |
| 125 | +### Protocol and encoding rules |
| 126 | + |
| 127 | +- Message lengths and field offsets are strict; validate before parsing. |
| 128 | +- Preserve big-endian encoding conventions for wire fields. |
| 129 | +- Enforce MBZ fields (`must be zero`) on unmarshal paths. |
| 130 | +- Keep mode negotiation logic consistent with RFC 4656/5357/5618/5938/6038 behavior already implemented. |
| 131 | +- Treat RFC 4656/5357/5618/5938/6038 as required review checklist items before merging protocol changes. |
| 132 | +- Cite RFC sections in comments where behavior may look surprising. |
| 133 | + |
| 134 | +### RFC compliance checklist (required for protocol changes) |
| 135 | + |
| 136 | +- Confirm wire sizes, offsets, and field ordering match RFC definitions before/after changes. |
| 137 | +- Verify all MBZ fields are written as zero and rejected on parse when non-zero. |
| 138 | +- Validate mode negotiation behavior against RFC 4656/5357 and extensions from RFC 5618/5938/6038. |
| 139 | +- Ensure authenticated/encrypted paths verify HMAC before consuming dependent fields. |
| 140 | +- Add or update tests that exercise protocol changes, including at least one RFC-specific regression case. |
| 141 | + |
| 142 | +### Security-sensitive code |
| 143 | + |
| 144 | +- Verify HMAC before using dependent authenticated fields. |
| 145 | +- Do not log secrets, keys, IVs, or raw auth material. |
| 146 | +- Keep key derivation and crypto stream setup paths explicit and test-covered. |
| 147 | + |
| 148 | +## Testing guidelines |
| 149 | + |
| 150 | +- Prefer table-driven tests for validation-heavy logic. |
| 151 | +- Use `t.Run` for case naming and selective execution. |
| 152 | +- Use `t.Parallel()` only when shared state and port usage are safe. |
| 153 | +- Keep tests deterministic (avoid flaky timing assumptions). |
| 154 | +- Add regression tests when fixing bugs. |
| 155 | +- Existing integration tests use `testify/require`; stay consistent in that area. |
0 commit comments