Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ Each enforceable guide in `docs/` should have a matching agent in `agents/`. The
| `python-pydantic-guide.md` | `python-quality-assistant` |
| `python-project-structure.md` | `python-architecture-assistant` |
| `python-makefile-commands.md` | `python-quality-assistant` |
| `go-mod-replace-guide.md` | `go-quality-assistant` |
| `go-glog-guide.md` | `go-quality-assistant` |
| `go-concurrency-patterns.md` | `go-architecture-assistant` |

Reference-only docs (patterns, setup guides) don't need agents.

Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ All guides live in [`docs/`](docs/) and can be read standalone without the plugi
| [Time Injection](docs/go-time-injection.md) | bborbe/time, CurrentDateTimeGetter |
| [GoDoc](docs/go-doc-best-practices.md) | Documentation standards |
| [Logging](docs/go-logging-guide.md) | Structured logging |
| [glog Levels](docs/go-glog-guide.md) | glog verbosity-level discipline (legacy projects) |
| [Design Patterns](docs/go-patterns.md) | Common Go patterns |

### Go — Testing
Expand Down
8 changes: 1 addition & 7 deletions agents/go-quality-assistant.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,7 @@ Code review categories:
- WaitGroup patterns
- Infinite loops must handle context cancellation

**Logging (glog)**:
- `glog.Error()` - genuine errors requiring immediate attention
- `glog.Warning()` - unexpected conditions that don't break functionality
- `glog.V(0).Info()` - production default, operator info only
- `glog.V(1).Info()` - operator debug, production troubleshooting
- `glog.V(2).Info()` - external communication, developer default
- `glog.V(3+).Info()` - developer debug, deep troubleshooting
**Logging (glog)**: See `docs/go-glog-guide.md` for level semantics. Canonical rule: `go-glog/use-v-for-debug-not-info` (V0 is production-default only; debug-shaped logs use `V(N)` with N≥1). Do not duplicate the level table here — the doc is the source of truth.

**External-call logging (boundary rule)**:
- Every call crossing the process boundary (HTTP, gRPC, DB, message bus, subprocess) MUST emit a log line on response. Without it, runtime debugging — "did the call go out? what was the status? did the response match what we expected?" — becomes guesswork.
Expand Down
92 changes: 85 additions & 7 deletions docs/go-concurrency-patterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,39 @@

## Core Rule: `go func()` Is a Smell

Raw goroutines leak, race, and are hard to test. Use `run.CancelOnFirstErrorWait` instead.
### RULE go-concurrency/no-raw-go-func (MUST)

**Owner**: go-architecture-assistant
**Applies when**: a Go file uses the raw `go func() { ... }()` / `go someMethod(...)` syntax outside `main.go` / top-level entry points — instead of one of the `github.com/bborbe/run` strategies (`CancelOnFirstErrorWait`, `CancelOnFirstFinishWait`, `All`, `Sequential`).
**Enforcement**: judgment (ast-grep follow-up: `go_statement` outside `main.go` / `cmd/**`. Test files exempt; `main` entry-point goroutine spawners exempt by path filter)
**Why**: Raw goroutines have three failure modes the `run` package solves: (1) they leak when the parent context is cancelled but the goroutine doesn't observe it; (2) they race when the parent function returns before the goroutine writes its result; (3) error propagation requires hand-rolled channels + `sync.WaitGroup` that drift toward subtle deadlocks. `run.CancelOnFirstErrorWait` wires context cancellation, error aggregation, and synchronization in one call — every consumer learns the same primitives, refactors stay safe, and goroutine lifetimes are explicit at the type signature.

#### Bad

```go
// Bad — goroutine leaks on error
// Goroutine leaks on error — no cancellation, no wait, no error propagation
go func() { results <- doWork(ctx) }()

// Good
// And worse — multiple raw goroutines with hand-rolled sync.WaitGroup
var wg sync.WaitGroup
for _, item := range items {
wg.Add(1)
go func(it Item) {
defer wg.Done()
_ = process(ctx, it) // error swallowed
}(item)
}
wg.Wait()
```

#### Good

```go
// run.CancelOnFirstErrorWait — context cancellation, error propagation, deterministic wait
return run.CancelOnFirstErrorWait(ctx, producer, consumer)

// For parallel processing of a slice, use run.All
return run.All(ctx, fns...)
```

## `run` Package
Expand Down Expand Up @@ -81,6 +106,49 @@ func process(ctx context.Context) error {

## Channel Ownership

### RULE go-concurrency/channel-closed-by-sender-only (MUST)

**Owner**: go-architecture-assistant
**Applies when**: a Go file calls `close(ch)` on a channel that was passed in as a function parameter from elsewhere — i.e. closed by a consumer/receiver rather than by the goroutine that produces values into it.
**Enforcement**: judgment (ast-grep follow-up: `close(X)` where `X` is a parameter type `chan T` or `chan<- T`; the agent rules in whether the function is the producer or consumer based on whether it sends into `X`)
**Why**: Closing a channel from the receiver side is a textbook race — the sender may still be writing when the close happens, producing `send on closed channel` panic. The Go convention is: **the producer owns the channel and is the only one allowed to close it.** Receivers learn of "no more values" via `for v := range ch` or the `comma-ok` idiom (`v, ok := <-ch`), never by closing themselves. Multi-producer cases use `sync.WaitGroup` + a single dedicated closer goroutine, not concurrent closes (which also panic).

#### Bad

```go
// Consumer closes the channel — race against the producer's still-pending send
func consume(items <-chan Item) {
for item := range items {
process(item)
}
close(items) // ← wrong direction; would also be a compile error on `<-chan`
}
```

#### Good

```go
// Producer closes; consumer ranges and exits naturally when the channel closes
func produce(ctx context.Context, out chan<- Item) error {
defer close(out) // ← producer owns the close
for _, raw := range source {
select {
case <-ctx.Done():
return ctx.Err()
case out <- transform(raw):
}
}
return nil
}

func consume(in <-chan Item) {
for item := range in {
process(item)
}
}
```


**Caller creates and owns the channel. Pass `chan<- T` into producer.**

```go
Expand Down Expand Up @@ -140,9 +208,19 @@ if errors.Is(err, errReachedUntil) { return nil }

## Rules

1. Never `go func()` — use `run.CancelOnFirstErrorWait`
1. Never `go func()` — use `run.CancelOnFirstErrorWait` (canonicalised as `go-concurrency/no-raw-go-func`)
2. Caller owns the channel, passes `chan<- T` as parameter
3. Producer closes bounded channels (`defer close(ch)`)
4. Always check `ctx.Done()` in consumer loops
3. Producer closes bounded channels (`defer close(ch)`) (canonicalised as `go-concurrency/channel-closed-by-sender-only`)
4. Always check `ctx.Done()` in consumer loops (cross-references `go-context/cancel-check-in-loop`)
5. Never close channel in unbounded/polling producers
6. Capture loop variables by value: `x := x`

> Note: an earlier revision listed a sixth rule "Capture loop variables by value (`x := x`)". That rule was removed because Go 1.22+ fixed loop-variable capture semantics — the variable is now scoped per iteration. For pre-1.22 projects, the `x := x` workaround is still required; this guide assumes Go 1.22+.

## Antipatterns

See the `#### Bad` blocks under each `### RULE` above for the canonical antipattern shapes. Summary:

- **Raw `go func()`** — `go-concurrency/no-raw-go-func` (leaks, races, hand-rolled `sync.WaitGroup` deadlocks).
- **Consumer closes channel** — `go-concurrency/channel-closed-by-sender-only` (panics on still-pending sends).
- **Unbounded producer with `defer close(ch)`** — bounded-shape primitive misapplied; the polling producer's `defer close` fires when the function returns on ctx-cancel, but the consumer goroutine may still be reading and panic on subsequent sends from anywhere else in the codebase.
- **Hidden `Results()` channel-returning method** — anti-pattern shown in Channel Ownership above; pass `chan<- T` as a parameter instead so ownership is explicit at the call site.
29 changes: 29 additions & 0 deletions docs/go-glog-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,35 @@

Google's [glog](https://github.com/golang/glog) provides more granular logging levels than standard Go logging. This guide explains when to use each level for consistent logging across Go services.

### RULE go-glog/use-v-for-debug-not-info (MUST)

**Owner**: go-quality-assistant
**Applies when**: a Go file calls `glog.Info` / `glog.Infof` for debug, trace, internal-state, or developer-troubleshooting log lines that should be filtered out in production — instead of `glog.V(N).Info...` / `glog.V(N).Infof...` with an appropriate verbosity level (1 for sysop debug, 2 for dev default, 3+ for deep debug). Scope: applies across ALL V levels — V0 is reserved for production-default operator info; everything debug-shaped goes through `V(N)`.
**Enforcement**: judgment (ast-grep follow-up: `call_expression` matching `glog.Info(...)` / `glog.Infof(...)` outside startup/shutdown contexts; the agent rules in "is this production-default-worthy?" based on the log content)
**Why**: V0 (bare `glog.Info`) is the *always-on* production logging level. Every line at V0 ships to log aggregators, gets indexed, and burns CPU on every request whether anyone reads it or not. Using V0 for debug-style logs ("Cache miss for key %s", "Internal state: %+v") produces gigabytes of noise in production, drowns operators in irrelevant detail, and inflates log-storage cost. The `glog.V(N)` mechanism exists exactly to gate verbosity at runtime: V0 stays operator-readable (startup, shutdown, health changes), V1+ stays off by default and turns on for troubleshooting. Inverting the levels means production logs are useless and dev debugging requires a deploy.

#### Bad

```go
// V0 used for debug-style content — runs in production, drowns operators
glog.Infof("Cache miss for key %s, fetching from database", key)
glog.Infof("Processing batch of %d items", len(batch))
glog.Infof("Internal state: %+v", internalStruct)
```

#### Good

```go
// V0 reserved for production-default operator info
glog.Info("Service started successfully on port 8080")
glog.Infof("Recovered from panic: %v", recovered)

// Debug-style content gated behind V(N)
glog.V(2).Infof("Cache miss for key %s, fetching from database", key)
glog.V(3).Infof("Processing batch of %d items", len(batch))
glog.V(4).Infof("Internal state: %+v", internalStruct)
```

## Error - Always an Error / Requires System Operator Action

Use for genuine errors that require immediate attention:
Expand Down
31 changes: 31 additions & 0 deletions docs/go-mod-replace-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,37 @@ When to use `replace` in `go.mod`, and when not to.

## Rule

### RULE go-mod-replace/no-cross-repo-replace (MUST)

**Owner**: go-quality-assistant
**Applies when**: a Go project's `go.mod` contains a `replace` directive whose right-hand side path points outside the current repo's working tree — relative paths that escape the repo root (`../../other-repo/lib`), absolute filesystem paths (`/Users/<name>/work/...`), or any other off-repo location.
**Enforcement**: judgment (ast-grep follow-up: pattern over `go.mod` `replace` lines with right-hand side starting with `../` reaching above the repo root or starting with `/`. Same-repo `replace github.com/acme/monorepo/libs/shared => ../../libs/shared` is fine — the rule fires only when the destination escapes the repo)
**Why**: An off-repo replace points to a path that exists only on the author's machine. Anyone else cloning the repo gets a broken build; CI can't resolve it; the module graph becomes non-reproducible. Worse, it hides the fact that consumers of your module require an unreleased change — the dependency looks fine locally but breaks the moment someone else pulls it. Within a monorepo, relative-path replaces ARE correct (every clone has the sibling module at the same path) — that's the same-repo exception. The rule fires only on the cross-repo escape.

#### Bad

```go
// consumer/go.mod
replace github.com/acme/producer/lib => ../../producer/lib
// Builds locally, breaks for everyone else. CI fails: path doesn't exist in their checkout.

// Or worse: absolute path tied to one machine
replace github.com/acme/producer/lib => /Users/alice/work/producer/lib
```

#### Good

```go
// consumer/go.mod — pin to a tagged or pseudo-version
require github.com/acme/producer/lib v0.0.0-20260403114524-913de8870914
// No replace. Reproducible for everyone.

// monorepo/services/api/go.mod — same-repo replace IS correct
replace github.com/acme/monorepo/libs/shared => ../../libs/shared
```

### Quick reference

- **Replace inside a single repo** → **OK**. Multi-module repos (monorepos with several `go.mod`) use relative-path replaces so sibling modules resolve to the working tree, not a released version.
- **Replace across repo boundaries** → **NO**. Never use a `replace` that points outside the current repo's checkout. Consume cross-repo modules as released pseudo-versions or tagged releases.

Expand Down
1 change: 1 addition & 0 deletions llms.txt
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
- [Error Wrapping](docs/go-error-wrapping-guide.md): Context-aware error handling
- [Context Cancellation](docs/go-context-cancellation-in-loops.md): Non-blocking select in loops
- [Logging](docs/go-logging-guide.md): Structured logging patterns
- [glog Levels](docs/go-glog-guide.md): glog verbosity-level discipline (legacy projects)

## Python
- [Project Structure](docs/python-project-structure.md): src/ layout, pyproject.toml, uv
Expand Down
36 changes: 36 additions & 0 deletions rules/index.json
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,24 @@
"level": "SHOULD",
"owner": "go-architecture-assistant"
},
{
"anchor": "go-concurrency/channel-closed-by-sender-only",
"applies_when": "a Go file calls `close(ch)` on a channel that was passed in as a function parameter from elsewhere — i.e. closed by a consumer/receiver rather than by the goroutine that produces values into it.",
"doc_path": "docs/go-concurrency-patterns.md",
"enforcement": "judgment (ast-grep follow-up: `close(X)` where `X` is a parameter type `chan T` or `chan<- T`; the agent rules in whether the function is the producer or consumer based on whether it sends into `X`)",
"id": "go-concurrency/channel-closed-by-sender-only",
"level": "MUST",
"owner": "go-architecture-assistant"
},
{
"anchor": "go-concurrency/no-raw-go-func",
"applies_when": "a Go file uses the raw `go func() { ... }()` / `go someMethod(...)` syntax outside `main.go` / top-level entry points — instead of one of the `github.com/bborbe/run` strategies (`CancelOnFirstErrorWait`, `CancelOnFirstFinishWait`, `All`, `Sequential`).",
"doc_path": "docs/go-concurrency-patterns.md",
"enforcement": "judgment (ast-grep follow-up: `go_statement` outside `main.go` / `cmd/**`. Test files exempt; `main` entry-point goroutine spawners exempt by path filter)",
"id": "go-concurrency/no-raw-go-func",
"level": "MUST",
"owner": "go-architecture-assistant"
},
{
"anchor": "go-context/cancel-check-in-loop",
"applies_when": "Go `for` loop body lacks a non-blocking `select { case <-ctx.Done(): ...; default: }` check, outside `*_test.go` and `vendor/`.",
Expand Down Expand Up @@ -260,6 +278,15 @@
"level": "SHOULD",
"owner": "go-quality-assistant"
},
{
"anchor": "go-glog/use-v-for-debug-not-info",
"applies_when": "a Go file calls `glog.Info` / `glog.Infof` for debug, trace, internal-state, or developer-troubleshooting log lines that should be filtered out in production — instead of `glog.V(N).Info...` / `glog.V(N).Infof...` with an appropriate verbosity level (1 for sysop debug, 2 for dev default, 3+ for deep debug). Scope: applies across ALL V levels — V0 is reserved for production-default operator info; everything debug-shaped goes through `V(N)`.",
"doc_path": "docs/go-glog-guide.md",
"enforcement": "judgment (ast-grep follow-up: `call_expression` matching `glog.Info(...)` / `glog.Infof(...)` outside startup/shutdown contexts; the agent rules in \"is this production-default-worthy?\" based on the log content)",
"id": "go-glog/use-v-for-debug-not-info",
"level": "MUST",
"owner": "go-quality-assistant"
},
{
"anchor": "go-http-handler/kebab-case-handler-files",
"applies_when": "a `*.go` file under `**/pkg/handler/**` is named with non-kebab-case style (e.g. `exists_handler.go`, `existshandler.go`, or `handler.go`) instead of the documented `<action>-<noun>.go` kebab-case (e.g. `exists.go`, `forward-invoice.go`). Pure ast-grep operates on file contents, not filenames — this is a filesystem convention check.",
Expand Down Expand Up @@ -386,6 +413,15 @@
"level": "MUST",
"owner": "go-quality-assistant"
},
{
"anchor": "go-mod-replace/no-cross-repo-replace",
"applies_when": "a Go project's `go.mod` contains a `replace` directive whose right-hand side path points outside the current repo's working tree — relative paths that escape the repo root (`../../other-repo/lib`), absolute filesystem paths (`/Users/<name>/work/...`), or any other off-repo location.",
"doc_path": "docs/go-mod-replace-guide.md",
"enforcement": "judgment (ast-grep follow-up: pattern over `go.mod` `replace` lines with right-hand side starting with `../` reaching above the repo root or starting with `/`. Same-repo `replace github.com/acme/monorepo/libs/shared => ../../libs/shared` is fine — the rule fires only when the destination escapes the repo)",
"id": "go-mod-replace/no-cross-repo-replace",
"level": "MUST",
"owner": "go-quality-assistant"
},
{
"anchor": "go-prometheus/composed-metrics-interface",
"applies_when": "a single `Metrics` interface aggregates methods spanning two or more distinct functional domains (handlers + senders + schedulers + …), forcing consumers to depend on methods they don't use.",
Expand Down
Loading