This PR adds an opinionated harness-engineering layer for agent-driven workflows: a small set of agent-readable docs, mechanical structure checks, structured CI failure summaries, an architecture-lint umbrella, and per-worktree dev-server isolation. The goal is to make local dev, tests, and CI mechanically inspectable by agents without changing app runtime behavior. ## What landed **Agent docs and navigation** - `.claude/docs/OBSERVABILITY.md`, `.claude/docs/DEV_ISOLATION.md`, `.claude/docs/AGENT_FAILURES.md`: task-oriented guides for logs, tracing, Prometheus, dev-server isolation, and a seeded failure catalog. - `AGENTS.md`: added an `Agent navigation` block, then trimmed the file from 375 to 229 lines by migrating duplicated detail into `WORKFLOWS.md`, `GO.md`, `TESTING.md`, and `DATABASE.md`. The user-managed custom-instructions block is preserved. - `.agents/docs`: symlink mirror of `.claude/docs` for agent runtimes that look under `.agents`. **Mechanical checks** - `scripts/check_agents_structure.sh`: validates `@...` references in tracked `AGENTS.md` files and warns when root grows past 600 lines. Wired as `make lint/agents` and into `make lint`. - `scripts/audit-agent-readiness.sh`: report-first audit of harness readiness. Currently `10 ok, 0 warn, 0 fail`. - `scripts/check_architecture.sh` / `make lint/architecture`: umbrella architecture-lint target. Consolidates the existing `check_enterprise_imports.sh` and `check_codersdk_imports.sh` so they run exactly once via the umbrella. Slot is open for new high-confidence rules. **Structured CI failure summaries** - `scripts/playwright-failure-summary.sh`: parses `site/test-results/results.json` and writes Markdown to `$GITHUB_STEP_SUMMARY` on failure. Wired into the `test-e2e` matrix job. - `scripts/go-test-failure-summary.sh`: parses `go test -json` line-delimited output the same way. Wired into `test-go-pg`, `test-go-pg-17`, and `test-go-race-pg` by injecting `gotestsum --jsonfile` in the workflow without touching `Makefile`. JSON also uploaded as a CI artifact on failure. - `site/e2e/playwright.config.ts`: enables `screenshot: only-on-failure`, `trace: retain-on-failure`, JSON reporter, and HTML reporter alongside existing reporters. - `.github/workflows/ci.yaml`: failure artifact uploads for Playwright now use `if: failure()` and predictable names (`playwright-artifacts-<variant>-<sha>`). **Per-worktree dev-server isolation** (`scripts/develop/main.go`) - Deterministic FNV-64a hash of the worktree path produces a port offset in `[0, 1000)` (50 buckets, step 20 to avoid API/proxy overlap across adjacent buckets). - Offset is applied only to defaults; both env vars (`CODER_DEV_PORT`, `CODER_DEV_WEB_PORT`, `CODER_DEV_PROXY_PORT`, `CODER_DEV_PROMETHEUS_PORT`) and CLI flags retain priority. - Hardcoded ports `9090` (embedded Prometheus UI) and `12345` (Delve) are unchanged by design. - Startup banner shows each port's source: `default`, `offset`, or `explicit`. - Unit tests in `scripts/develop/main_test.go` cover determinism, bounds, no-overlap across the four ports, and explicit-skip behavior. - State (`.coderv2/`) was already worktree-isolated via `os.Getwd()`, so no state-dir changes were needed. ## Validation `make lint/agents`, `make lint/architecture`, `make lint/emdash`, `bash scripts/audit-agent-readiness.sh` (10 ok, 0 warn, 0 fail), `shellcheck` on all 5 new scripts, `go test ./scripts/develop/...`, and `js-yaml` parse of `ci.yaml` all pass. Synthetic fixtures verify both failure-summary scripts handle empty/missing input (silent exit 0), ANSI-stripped output, and parent/subtest formatting. ## Known follow-ups (deferred) - Frontend Storybook/Vitest failure summary: lowest-leverage slice of the failure-summary work. Skipping until observed pain. - Architecture lint currently only delegates to existing import checks; new rules (`InTx` outer-store detection, swagger-annotation lint) plug in as needed. - 50 port-offset buckets means two worktree paths can occasionally collide. The DEV_ISOLATION doc tells users to set the relevant env var when this happens. > Mux opened this PR on Mike's behalf.
16 KiB
Modern Go (1.18-1.26)
Reference for writing idiomatic Go. Covers what changed, what it
replaced, and what to reach for. Respect the project's go.mod go
line: don't emit features from a version newer than what the module
declares. Check go.mod before writing code.
Go LSP Navigation
Use Go LSP tools first for backend code navigation:
- Find definitions:
mcp__go-language-server__definition symbolName - Find references:
mcp__go-language-server__references symbolName - Get type info:
mcp__go-language-server__hover filePath line column - Rename symbol:
mcp__go-language-server__rename_symbol filePath line column newName
Code Comments
Code comments should be clear, well-formatted, and add meaningful context.
- Comments are sentences and should end with periods or other appropriate punctuation.
- Explain why, not what. The code itself should be self-documenting through clear naming and structure. Focus comments on non-obvious decisions, edge cases, or business logic.
- Keep comment lines to 80 characters wide, including the comment prefix
like
//or#. When a comment spans multiple lines, wrap it naturally at word boundaries.
// Good: Explains the rationale with proper sentence structure.
// We need a custom timeout here because workspace builds can take several
// minutes on slow networks, and the default 30s timeout causes false
// failures during initial template imports.
ctx, cancel := context.WithTimeout(ctx, 5*time.Minute)
// Bad: Describes what the code does without punctuation or wrapping.
// Set a custom timeout
// Workspace builds can take a long time
// Default timeout is too short
ctx, cancel := context.WithTimeout(ctx, 5*time.Minute)
Avoid Unnecessary Changes
When fixing a bug or adding a feature, don't modify code unrelated to your task. Unnecessary changes make PRs harder to review and can introduce regressions.
- Don't reword existing comments or code unless the change is directly motivated by your task.
- Don't delete existing comments that explain non-obvious behavior.
- When adding tests for new behavior, read existing tests first to understand what's covered. Add new cases for uncovered behavior. Edit existing tests as needed, but don't change what they verify.
How modern Go thinks differently
Generics (1.18): Design reusable code with type parameters instead
of interface{} casts, code generation, or the sort.Interface
pattern. Use any for unconstrained types, comparable for map keys
and equality, cmp.Ordered for sortable types. Type inference usually
makes explicit type arguments unnecessary (improved in 1.21).
Per-iteration loop variables (1.22): Each loop iteration gets its
own variable copy. Closures inside loops capture the correct value. The
v := v shadow trick is dead. Remove it when you see it.
Iterators (1.23): iter.Seq[V] and iter.Seq2[K,V] are the
standard iterator types. Containers expose .All() methods returning
these. Combined with slices.Collect, slices.Sorted, maps.Keys,
etc., they replace ad-hoc "loop and append" code with composable,
lazy pipelines. When a sequence is consumed only once, prefer an
iterator over materializing a slice.
Error trees (1.20-1.26): Errors compose as trees, not chains.
errors.Join aggregates multiple errors. fmt.Errorf accepts multiple
%w verbs. errors.Is/As traverse the full tree. Custom error
types that wrap multiple causes must implement Unwrap() []error (the
slice form), not Unwrap() error, or tree traversal won't find the
children. errors.AsType[T] (1.26) is the type-safe way to match
error types. Propagate cancellation reasons with
context.WithCancelCause.
Structured logging (1.21): log/slog is the standard structured
logger. This project uses cdr.dev/slog/v3 instead, which has a
different API. Do not use log/slog directly.
Replace these patterns
The left column reflects common patterns from pre-1.22 Go. Write the
right column instead. The "Since" column tells you the minimum go
directive version required in go.mod.
| Old pattern | Modern replacement | Since |
|---|---|---|
interface{} |
any |
1.18 |
v := v inside loops |
remove it | 1.22 |
for i := 0; i < n; i++ |
for i := range n |
1.22 |
for i := 0; i < b.N; i++ (benchmarks) |
for b.Loop() (correct timing, future-proof) |
1.24 |
sort.Slice(s, func(i,j int) bool{…}) |
slices.SortFunc(s, cmpFn) |
1.21 |
wg.Add(1); go func(){ defer wg.Done(); … }() |
wg.Go(func(){…}) |
1.25 |
func ptr[T any](v T) *T { return &v } |
new(expr) e.g. new(time.Now()) |
1.26 |
var target *E; errors.As(err, &target) |
t, ok := errors.AsType[*E](err) |
1.26 |
| Custom multi-error type | errors.Join(err1, err2, …) |
1.20 |
Single %w for multiple causes |
fmt.Errorf("…: %w, %w", e1, e2) |
1.20 |
rand.Seed(time.Now().UnixNano()) |
delete it (auto-seeded); prefer math/rand/v2 |
1.20/1.22 |
sync.Once + captured variable |
sync.OnceValue(func() T {…}) / OnceValues |
1.21 |
Custom min/max helpers |
min(a, b) / max(a, b) builtins (any ordered type) |
1.21 |
for k := range m { delete(m, k) } |
clear(m) (also zeroes slices) |
1.21 |
Index+slice or SplitN(s, sep, 2) |
strings.Cut(s, sep) / bytes.Cut |
1.18 |
TrimPrefix + check if anything was trimmed |
strings.CutPrefix / CutSuffix (returns ok bool) |
1.20 |
strings.Split + loop when no slice is needed |
strings.SplitSeq / Lines / FieldsSeq (iterator, no alloc) |
1.24 |
"2006-01-02" / "2006-01-02 15:04:05" / "15:04:05" |
time.DateOnly / time.DateTime / time.TimeOnly |
1.20 |
Manual Before/After/Equal chains for comparison |
time.Time.Compare (returns -1/0/+1; works with slices.SortFunc) |
1.20 |
| Loop collecting map keys into slice | slices.Sorted(maps.Keys(m)) |
1.23 |
fmt.Sprintf + append to []byte |
fmt.Appendf(buf, …) (also Append, Appendln) |
1.18 |
reflect.TypeOf((*T)(nil)).Elem() |
reflect.TypeFor[T]() |
1.22 |
*(*[4]byte)(slice) unsafe cast |
[4]byte(slice) direct conversion |
1.20 |
atomic.LoadInt64 / AddInt64 / StoreInt64 etc. |
atomic.Int64 (also Int32, Uint32, Uint64, Bool, Pointer[T]) |
1.19 |
crypto/rand.Read(buf) + hex/base64 encode |
crypto/rand.Text() (one call) |
1.24 |
Checking crypto/rand.Read error |
don't: return is always nil | 1.24 |
time.Sleep in tests |
testing/synctest (deterministic fake clock) |
1.24/1.25 |
json:",omitempty" on zero-value structs like time.Time{} |
json:",omitzero" (uses IsZero() method) |
1.24 |
strings.Title |
golang.org/x/text/cases |
1.18 |
net.IP in new code |
net/netip.Addr (immutable, comparable, lighter) |
1.18 |
tools.go with blank imports |
tool directive in go.mod |
1.24 |
runtime.SetFinalizer |
runtime.AddCleanup (multiple per object, no pointer cycles) |
1.24 |
httputil.ReverseProxy.Director |
.Rewrite hook + ProxyRequest (Director deprecated in 1.26) |
1.20 |
sql.NullString, sql.NullInt64, etc. |
sql.Null[T] |
1.22 |
Manual ctx, cancel := context.WithCancel(…) + t.Cleanup(cancel) |
t.Context() (auto-canceled when test ends) |
1.24 |
if d < 0 { d = -d } on durations |
d.Abs() (handles math.MinInt64) |
1.19 |
Implement only TextMarshaler |
also implement TextAppender for alloc-free marshaling |
1.24 |
Custom Unwrap() error on multi-cause errors |
Unwrap() []error (slice form; required for tree traversal) |
1.20 |
New capabilities
These enable things that weren't practical before. Reach for them in the described situations.
| What | Since | When to use it |
|---|---|---|
cmp.Or(a, b, c) |
1.22 | Defaults/fallback chains: returns first non-zero value. Replaces verbose if a != "" { return a } cascades. |
context.WithoutCancel(ctx) |
1.21 | Background work that must outlive the request (e.g. async cleanup after HTTP response). Derived context keeps parent's values but ignores cancellation. |
context.AfterFunc(ctx, fn) |
1.21 | Register cleanup that fires on context cancellation without spawning a goroutine that blocks on <-ctx.Done(). |
context.WithCancelCause / Cause |
1.20 | When callers need to know WHY a context was canceled, not just that it was. Retrieve cause with context.Cause(ctx). |
context.WithDeadlineCause / WithTimeoutCause |
1.21 | Attach a domain-specific error to deadline/timeout expiry (e.g. distinguish "DB query timed out" from "HTTP request timed out"). |
errors.ErrUnsupported |
1.21 | Standard sentinel for "not supported." Use instead of per-package custom sentinels. Check with errors.Is. |
http.ResponseController |
1.20 | Per-request flush, hijack, and deadline control without type-asserting ResponseWriter to http.Flusher or http.Hijacker. |
Enhanced ServeMux routing |
1.22 | "GET /items/{id}" patterns in http.ServeMux. Access with r.PathValue("id"). Wildcards: {name}, catch-all: {path...}, exact: {$}. Eliminates many third-party router dependencies. |
os.Root / OpenRoot |
1.24 | Confined directory access that prevents symlink escape. 1.25 adds MkdirAll, ReadFile, WriteFile for real use. |
os.CopyFS |
1.23 | Copy an entire fs.FS to local filesystem in one call. |
os/signal.NotifyContext with cause |
1.26 | Cancellation cause identifies which signal (SIGTERM vs SIGINT) triggered shutdown. |
io/fs.SkipAll / filepath.SkipAll |
1.20 | Return from WalkDir callback to stop walking entirely. Cleaner than a sentinel error. |
GOMEMLIMIT env / debug.SetMemoryLimit |
1.19 | Soft memory limit for GC. Use alongside or instead of GOGC in memory-constrained containers. |
net/url.JoinPath |
1.19 | Join URL path segments correctly. Replaces error-prone string concatenation. |
go test -skip |
1.20 | Skip tests matching a pattern. Useful when running a subset of a large test suite. |
Key packages
slices (1.21, iterators added 1.23)
Replaces sort.Slice, manual search loops, and manual contains checks.
Search: Contains, ContainsFunc, Index, IndexFunc,
BinarySearch, BinarySearchFunc.
Sort: Sort, SortFunc, SortStableFunc, IsSorted, IsSortedFunc,
Min, MinFunc, Max, MaxFunc.
Transform: Clone, Compact, CompactFunc, Grow, Clip,
Concat (1.22), Repeat (1.23), Reverse, Insert, Delete,
Replace.
Compare: Equal, EqualFunc, Compare.
Iterators (1.23): All, Values, Backward, Collect, AppendSeq,
Sorted, SortedFunc, SortedStableFunc, Chunk.
maps (1.21, iterators added 1.23)
Core: Clone, Copy, Equal, EqualFunc, DeleteFunc.
Iterators (1.23): All, Keys, Values, Insert, Collect.
cmp (1.21, Or added 1.22)
Ordered constraint for any ordered type. Compare(a, b) returns
-1/0/+1. Less(a, b) returns bool. Or(vals...) returns first
non-zero value.
iter (1.23)
Seq[V] is func(yield func(V) bool). Seq2[K,V] is
func(yield func(K, V) bool). Return these from your container's
.All() methods. Consume with for v := range seq or pass to
slices.Collect, slices.Sorted, maps.Collect, etc.
math/rand/v2 (1.22)
Replaces math/rand. IntN not Intn. Generic N[T]() for any
integer type. Default source is ChaCha8 (crypto-quality). No global
Seed. Use rand.New(source) for reproducible sequences.
log/slog (1.21)
slog.Info, slog.Warn, slog.Error, slog.Debug with key-value
pairs. slog.With(attrs...) for logger with preset fields.
slog.GroupAttrs (1.25) for clean group creation. Implement
slog.Handler for custom backends.
Note: This project uses cdr.dev/slog/v3, not log/slog. The
API is different. Read existing code for usage patterns.
Pitfalls
Things that are easy to get wrong, even when you know the modern API exists. Check your output against these.
Version misuse. The replacement table has a "Since" column. If the
project's go.mod says go 1.22, you cannot use wg.Go (1.25),
errors.AsType (1.26), new(expr) (1.26), b.Loop() (1.24), or
testing/synctest (1.24). Fall back to the older pattern. Always
check before reaching for a replacement.
slices.Sort vs slices.SortFunc. slices.Sort requires
cmp.Ordered types (int, string, float64, etc.). For structs, custom
types, or multi-field sorting, use slices.SortFunc with a comparator
function. Using slices.Sort on a non-ordered type is a compile error.
for range n still binds the index. for range n discards the
index. If you need it, write for i := range n. Writing
for range n and then trying to use i inside the loop is a compile
error.
Don't hand-roll iterators when the stdlib returns one. Functions
like maps.Keys, slices.Values, strings.SplitSeq, and
strings.Lines already return iter.Seq or iter.Seq2. Don't
reimplement them. Compose with slices.Collect, slices.Sorted, etc.
Don't mix math/rand and math/rand/v2. They have different
function names (Intn vs IntN) and different default sources. Pick
one per package. Prefer v2 for new code. The v1 global source is
auto-seeded since 1.20, so delete rand.Seed calls either way.
Iterator protocol. When implementing iter.Seq, you must respect
the yield return value. If yield returns false, stop iteration
immediately and return. Ignoring it violates the contract and causes
panics when consumers break out of for range loops early.
errors.Join with nil. errors.Join skips nil arguments. This is
intentional and useful for aggregating optional errors, but don't
assume the result is always non-nil. errors.Join(nil, nil) returns
nil.
cmp.Or evaluates all arguments. Unlike a chain of if
statements, cmp.Or(a(), b(), c()) calls all three functions. If any
have side effects or are expensive, use if/else instead.
Timer channel semantics changed in 1.23. Code that checks
len(timer.C) to see if a value is pending no longer works (channel
capacity is 0). Use a non-blocking select receive instead:
select { case <-timer.C: default: }.
context.WithoutCancel still propagates values. The derived
context inherits all values from the parent. If any middleware stores
request-scoped state (deadlines, trace IDs) via context.WithValue,
the background work sees it. This is usually desired but can be
surprising if the values hold references that should not outlive the
request.
Behavioral changes that affect code
- Timers (1.23): unstopped
Timer/Tickerare GC'd immediately. Channels are unbuffered: no stale values afterReset/Stop. You no longer needdefer t.Stop()to prevent leaks. - Error tree traversal (1.20):
errors.Is/AsfollowUnwrap() []error, not justUnwrap() error. Multi-error types must expose the slice form for child errors to be found. math/randauto-seeded (1.20): the global RNG is auto-seeded.rand.Seedis a no-op in 1.24+. Don't call it.- GODEBUG compat (1.21): behavioral changes are gated by
go.mod'sgoline. Upgrading the version opts into new defaults. - Build tags (1.18):
//go:buildis the only syntax.// +buildis gone. - Tool install (1.18):
go getno longer builds. Usego install pkg@version. - Doc comments (1.19): support
[links], lists, and headings. go test -skip(1.20): skip tests by name pattern from the command line.go fix ./...modernizers (1.26): auto-rewrites code to use newer idioms. Run after Go version upgrades.
Transparent improvements (no code changes)
Swiss Tables maps, Green Tea GC, PGO, faster io.ReadAll,
stack-allocated slices, reduced cgo overhead, container-aware
GOMAXPROCS. Free on upgrade.