|
| 1 | +# Agent guide |
| 2 | + |
| 3 | +This file is read by AI coding assistants before working in this repo. Human contributors may find it useful too — see `README.md` for the user-facing tool description. |
| 4 | + |
| 5 | +## Project |
| 6 | + |
| 7 | +`connectivity` is a small Go CLI that validates network connectivity at OSI layers 3, 4, and 7. Source layout is flat — every `.go` file lives in the repo root and belongs to `package main`. `README.md` is the source of truth for end-user behavior. |
| 8 | + |
| 9 | +## Development loop |
| 10 | + |
| 11 | +Standard Go workflow. Before opening a PR: |
| 12 | + |
| 13 | + go vet ./... |
| 14 | + gofmt -l . # must print nothing |
| 15 | + go build ./... |
| 16 | + go test -race ./... |
| 17 | + |
| 18 | +A handful of tests in `resolver_test.go` and `router_test.go` reach the real network and routing table; they may fail in sandboxed environments. That's a known gap (#24), not a regression. Before treating a test failure as your fault, confirm it doesn't reproduce on `origin/main`. |
| 19 | + |
| 20 | +`./build.sh` runs vet, build with version-injected ldflags, and tests. It's primarily for release builds; daily development is fine with the plain `go` commands above. |
| 21 | + |
| 22 | +## Test-driven development |
| 23 | + |
| 24 | +Default to writing the test first. |
| 25 | + |
| 26 | +1. Write a failing test that captures the bug or the new behavior. |
| 27 | +2. Run `go test -run TestName` and confirm it fails *for the reason you expect* — not a compile error, not a typo. |
| 28 | +3. Make the smallest change that turns it green. |
| 29 | +4. Refactor while green; rerun the test after each step. |
| 30 | + |
| 31 | +This catches a class of mistakes that retroactive tests miss: tests that happen to pass against the broken code (because they don't actually exercise the failure mode), and tests that pass trivially (because they don't assert what they claim to). |
| 32 | + |
| 33 | +For Go specifically: |
| 34 | + |
| 35 | +- Prefer table-driven tests. Each case is one row in a slice of structs; loop with `t.Run(tc.name, ...)` so failures point at the row that failed. |
| 36 | +- Tests must be hermetic by default — no real network, no filesystem outside `t.TempDir()`, no routing-table reads. Several existing tests violate this (#24); don't follow that pattern in new code. |
| 37 | +- When a fix has no unit-testable seam (the function couples directly to `net.Dial`, the kernel routing table, etc.), it's acceptable to ship the fix without a new test — but say so explicitly in the PR's Test Plan and file a follow-up to add the seam. |
| 38 | +- Use descriptive test names: `TestParseDestinations_DropsFirstURLFromConfig` beats `TestParseDestinations_Bug5`. |
| 39 | +- Use `t.Cleanup` and `t.TempDir` to manage fixtures; don't leave state behind between cases. |
| 40 | + |
| 41 | +Don't go through the motions. A test that calls the function and `t.Fatal`s on a returned error without asserting on outputs is checklist theater — and worse, it gives false confidence that the behavior is covered. |
| 42 | + |
| 43 | +## Code style |
| 44 | + |
| 45 | +- Idiomatic Go. Prefer the standard library; new third-party deps need justification. |
| 46 | +- `fmt.Errorf` with `%w` for wrapping. Older `errors.New(fmt.Sprintf(...))` can be modernized opportunistically — don't churn whole files for it. |
| 47 | +- `log.Fatalf` is acceptable only at process startup; library-level code returns errors. |
| 48 | +- `os.ReadFile` / `os.WriteFile`, not deprecated `ioutil`. |
| 49 | +- `strings.ReplaceAll`, not `strings.Replace(..., -1)`. |
| 50 | +- `net.JoinHostPort` when building `host:port` — the codebase is moving toward IPv6 support (#10). |
| 51 | +- HTTP requests must close response bodies and set a `Timeout`; see #7. |
| 52 | +- Plumb `context.Context` through network operations when the call site can supply one. |
| 53 | +- Logging currently uses the `log` package. New structured logging should use `log/slog`. |
| 54 | + |
| 55 | +## Repository conventions |
| 56 | + |
| 57 | +- Branches: `claude/<slug>` for AI-generated work (e.g. `claude/fix-issue-13-toolchain`). Reference the issue number when the change closes one. |
| 58 | +- Commits: imperative mood, lowercase first word, no trailing period. Keep the subject under 72 characters. |
| 59 | +- PRs: include a Summary and Test Plan. Use `Fixes #N` / `Refs #N`. The release workflow reads labels (`release:patch` / `release:minor` / `release:major` / `release:skip`) — default is `patch`. |
| 60 | +- Don't merge your own PRs. Don't push to `main`. Don't commit generated artifacts (the `connectivity` binary is gitignored). |
| 61 | + |
| 62 | +## Issue triage |
| 63 | + |
| 64 | +Two label dimensions: |
| 65 | + |
| 66 | +- Type: `bug` or `enhancement`. |
| 67 | +- Priority: `priority:critical` / `priority:high` / `priority:medium` / `priority:low`. |
| 68 | + |
| 69 | +Rubric: |
| 70 | + |
| 71 | +- `critical` — drop everything; production-impacting. |
| 72 | +- `high` — significant correctness, security, or reliability; fix in the next release cycle. |
| 73 | +- `medium` — important quality-of-life or prevention work. |
| 74 | +- `low` — nice to have. |
| 75 | + |
| 76 | +Check open issues for overlap before filing; cross-reference rather than duplicate. |
| 77 | + |
| 78 | +## Scope discipline |
| 79 | + |
| 80 | +- Single-purpose PRs. A bug fix should not slip in unrelated cleanups; a refactor should not slip in behavior changes. |
| 81 | +- If you discover a separate defect while working on something else, file an issue rather than expanding the current PR. |
| 82 | +- Don't add backwards-compatibility shims for behavior that has no production users yet. |
| 83 | +- Resist the urge to "fix it while I'm in here" if it's not in scope. The cost of an unrelated change is borne by every future reviewer. |
| 84 | + |
| 85 | +## Known traps |
| 86 | + |
| 87 | +Landmines in the current code. Don't reintroduce them after a fix lands, and be aware when touching adjacent code: |
| 88 | + |
| 89 | +- `ParseDestinations` in `connectivity.go` unconditionally skips index 0 of the URL slice — a stale CLI-args convention that silently drops the first config-loaded URL (#5). |
| 90 | +- The YAML config loader unmarshals into `map[string]string`, ignoring the typed `Config` struct, so `statsd_host` / `statsd_port` / `statsd_protocol` are silently dropped (#6). |
| 91 | +- The HTTPS check uses Go's default `http.Client`: no timeout, no body close, no status-code check, follows up to 10 redirects (#7, #12, #15). |
| 92 | +- IPv6 is silently filtered out in three places (`resolver.go`, `destinations.go`, `source.go`); IPv6-only destinations report success without being checked (#10). |
| 93 | +- The statsd emitter opens a new connection per metric and has a 100-message queue that blocks callers when full (#11). |
| 94 | +- `gopacket/routing` panics on non-Linux at runtime; the project is implicitly Linux-only (#34). |
| 95 | + |
| 96 | +## Out of scope here |
| 97 | + |
| 98 | +- See `README.md` for what the tool does and how end users invoke it. |
| 99 | +- See `LICENSE` for licensing terms. |
| 100 | +- See `.github/workflows/` for the exact CI and release pipelines. |
0 commit comments