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 numberDiff line numberDiff line change
Expand Up@@ -48,6 +48,9 @@ Each enforceable guide in `docs/` should have a matching agent in `agents/`. The
| `go-mod-replace-guide.md` | `go-quality-assistant` |
| `go-glog-guide.md` | `go-quality-assistant` |
| `go-concurrency-patterns.md` | `go-architecture-assistant` |
| `go-http-service-guide.md` | `go-http-handler-assistant` |
| `go-cqrs.md` | `go-architecture-assistant` |
| `go-cli-guide.md` | `go-quality-assistant` |

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

Expand Down
91 changes: 90 additions & 1 deletion docs/go-cli-guide.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,7 +2,59 @@

## Flag Parsing: cobra + slog

**NEVER use stdlib `flag` package** — transitive dependencies like `github.com/golang/glog` register flags via `init()`, polluting `--help` output with unwanted flags (`-alsologtostderr`, `-log_dir`, `-v`, etc.).
### RULE go-cli/cobra-not-stdlib-flag (MUST)

**Owner**: go-quality-assistant
**Applies when**: a Go CLI binary's `main.go` / `pkg/cli/...` imports `flag` (stdlib) and calls `flag.String` / `flag.Bool` / `flag.Parse`, instead of using `github.com/spf13/cobra` (with its `pflag` library).
**Enforcement**: judgment (ast-grep follow-up: `import "flag"` in any `main` package + `call_expression` matching `flag.Parse` / `flag.String`. Test files exempt.)
**Why**: Stdlib `flag` uses a process-global `flag.CommandLine` FlagSet. Any transitive dependency that calls `flag.String(...)` in its `init()` adds flags to this global set — and `github.com/golang/glog` is the most common offender, adding 8+ flags (`-alsologtostderr`, `-log_dir`, `-log_backtrace_at`, `-stderrthreshold`, `-v`, `-vmodule`, …) to every binary that transitively imports it. The result: `my-tool --help` displays a wall of irrelevant glog flags before your three actual flags, and the binary accepts those flags at runtime even though no one wanted them. Cobra uses `pflag` which is isolated from `flag.CommandLine` — the global pollution can't reach it, `--help` shows only your flags, and your flag namespace stays under your control.

#### Bad

```go
// main.go — stdlib flag pollutes --help with transitive glog flags
package main

import (
"flag"
"fmt"
)

func main() {
var config string
flag.StringVar(&config, "config", "", "Path to config")
flag.Parse()
fmt.Println(config)
}
// my-tool --help prints --config AND -alsologtostderr, -log_dir, -v, -vmodule, ...
```

#### Good

```go
// main.go
package main

import "github.com/bborbe/my-tool/pkg/cli"

func main() {
cli.Execute()
}

// pkg/cli/cli.go — cobra/pflag, isolated from flag.CommandLine
func Run(ctx context.Context, args []string) error {
var config string
rootCmd := &cobra.Command{
Use: "my-tool",
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error { return nil },
}
rootCmd.Flags().StringVar(&config, "config", "", "Path to config")
rootCmd.SetArgs(args)
return rootCmd.ExecuteContext(ctx)
}
// my-tool --help prints only --config (and cobra's --help itself)
```

**Always use `github.com/spf13/cobra`** for CLI flag parsing, even for single-command binaries.

Expand DownExpand Up@@ -117,6 +169,43 @@ func Run(ctx context.Context, args []string) error {

## Logging

### RULE go-cli/slog-not-glog-in-new-projects (MUST)

**Owner**: go-quality-assistant
**Applies when**: a *new* Go CLI binary (created after Go 1.21 release; no prior glog usage in the same module) imports `github.com/golang/glog`. Existing glog-using projects are exempt — they should not introduce slog and glog side by side.
**Enforcement**: judgment (semantic — distinguishing "new project" from "existing project mid-migration" requires checking git history / module age; ast-grep partial: `import "github.com/golang/glog"` in any main module without prior glog usage)
**Why**: glog has two structural problems slog doesn't: (1) it registers 8+ flags via stdlib `flag.init()` which pollutes every binary's `--help` output (see `go-cli/cobra-not-stdlib-flag`); (2) it predates structured logging — every log line is a free-form string, so log aggregators can't reliably parse `user_id=<X>` / `request_id=<Y>` fields. `log/slog` (stdlib Go 1.21+) emits structured key-value logs, integrates with `context.Context` for request-scoped fields, and has no `flag` pollution. For *existing* glog projects, the migration cost is real and not always worth it — but new projects should not pay the glog tax.

#### Bad

```go
// New CLI binary in 2026 importing glog
import (
"github.com/golang/glog"
"github.com/spf13/cobra"
)

func runE(cmd *cobra.Command, args []string) error {
glog.Infof("starting with config=%s", configPath) // free-form, no structured fields
return nil
}
```

#### Good

```go
// New CLI binary using log/slog
import (
"log/slog"
"github.com/spf13/cobra"
)

func runE(cmd *cobra.Command, args []string) error {
slog.Info("starting", "config", configPath, "verbose", verbose)
return nil
}
```

Use `log/slog` (stdlib Go 1.21+). See [go-glog-guide.md](go-glog-guide.md) for legacy glog projects only.

## Why Not stdlib `flag`?
Expand Down
81 changes: 79 additions & 2 deletions docs/go-cqrs.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -102,9 +102,86 @@ In normal error-handling paths, the only case where offsets do NOT commit is whe

## Rules

### RULE go-cqrs/auto-tx-wrapper-no-manual-wrap (MUST)

**Owner**: go-architecture-assistant
**Applies when**: a Go CQRS consumer in this framework manually wraps its command executor with `kv.NewTransactionMiddleware` / similar transaction-management code instead of using `RunCommandConsumerTx` (which auto-wraps).
**Enforcement**: judgment (ast-grep partial: `call_expression` matching `NewTransactionMiddleware` / `Wrap...` patterns adjacent to a CQRS command consumer registration)
**Why**: `RunCommandConsumerTx` is the framework's transaction-management entry point. It opens a kv transaction per command, hands the txn-bound store to the executor, commits on success, rolls back on error — exactly once per message, in the exact order the framework expects. Manual wrapping duplicates that logic and introduces subtle drift: the manual wrapper may rollback while the framework also rolls back (double-rollback panic on closed txn), or may commit while the framework expects rollback semantics on a downstream failure. The bug surfaces as "this CQRS consumer occasionally double-applies state changes" — hard to reproduce, harder to diagnose.

#### Bad

```go
// Manual transaction wrapping — duplicates RunCommandConsumerTx's contract
wrappedExecutor := kv.NewTransactionMiddleware(db, executor)
err := cdb.RunCommandConsumerTx(saramaClientProvider, syncProducer, db,
schemaID, wrappedExecutor) // double-wrapping smell — Tx variant already wraps
```

#### Good

```go
// Tx auto-wrapped — framework owns the transaction lifecycle
err := cdb.RunCommandConsumerTx(saramaClientProvider, syncProducer, db,
schemaID, executor)
```

### RULE go-cqrs/skipped-not-nil-for-non-retryable (MUST)

**Owner**: go-architecture-assistant
**Applies when**: a Go CQRS executor handles a *non-retryable* condition (idempotent skip, command targets an already-terminal entity, duplicate detected, validation against immutable state failed) by returning `nil` or an arbitrary `err` instead of `ErrCommandObjectSkipped`.
**Enforcement**: judgment (semantic — distinguishing "non-retryable skip" from "transient error worth a Failure result" requires reading the executor's intent)
**Why**: Three different result-topic outcomes hinge on the executor's return:
- `nil` → `ResultObjectSuccess` published. The publisher thinks the command succeeded.
- `err` → `ResultObjectFailure` published, one per occurrence. Duplicate publisher emissions produce N Failure entries + N error log lines + N noisy alerts.
- `ErrCommandObjectSkipped` → no result sent, offset commits silently.

For non-retryable conditions (terminal state, duplicate, immutable-validation failure), only `Skipped` is correct: publishing `Success` lies to the publisher; publishing `Failure` spams the result topic. The classic bug: an order processor returns `InvalidStateError` for `Completed` orders. Publisher retries 50× on a network blip → result topic gets 50 Failure entries and the on-call sees 50 alerts for an idempotent no-op.

#### Bad (variant A — `return err`)

```go
func (e *Executor) Execute(ctx context.Context, cmd Command) error {
order := e.store.Get(cmd.OrderID)
if order.Status == Completed {
// produces noisy Failure result; N duplicate commands → N Failure entries +
// N error log lines + N alerts on the on-call's pager
return errors.New("order already completed")
}
// ... real work
}
```

#### Bad (variant B — `return nil`)

```go
func (e *Executor) Execute(ctx context.Context, cmd Command) error {
order := e.store.Get(cmd.OrderID)
if order.Status == Completed {
// publishes Success — lies to the publisher; downstream code thinks
// the command actually advanced the entity state
return nil
}
// ... real work
}
```

#### Good

```go
func (e *Executor) Execute(ctx context.Context, cmd Command) error {
order := e.store.Get(cmd.OrderID)
if order.Status == Completed {
// silent idempotent skip, no result published, offset commits cleanly
return cdb.ErrCommandObjectSkipped
}
// ... real work
}
```

### Other rules (judgment-tier, not yet canonicalised as RULE blocks)

- Never consume event topic to wait for command results — use result topic
- `RunCommandConsumerTx` wraps executors automatically — don't wrap manually
- `ErrCommandObjectSkipped` skips silently (no result sent) — use for non-retryable situations
- Normal `err` returns are NOT retried by the framework; they emit one Failure result and commit the offset — same offset behaviour as Skipped, different result-topic behaviour
- `SendResultEnabled() == false` + no error → no result sent
- Context timeout → `ResultFor()` returns `Success: false`
Expand Down
54 changes: 54 additions & 0 deletions docs/go-http-service-guide.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -49,6 +49,37 @@ func (a *application) createHTTPServer(
}
```

### RULE go-http-service/canonical-admin-endpoints (MUST)

**Owner**: go-http-handler-assistant
**Applies when**: a Go service's admin HTTP server (port 9090, mounted under `/admin/<svc>/...` by the gateway) is missing any of the five always-required endpoints: `/healthz`, `/readiness`, `/metrics`, `/setloglevel/{level}`, `/gc`.
**Enforcement**: judgment (route-table inspection in `main.go` / handler factory; ast-grep partial: `router.Handle(...)` / `mux.Handle(...)` patterns checked against the five-endpoint set)
**Why**: The five endpoints are the cross-service contract between every bborbe Go service and the supervisor (Kubernetes probes, Prometheus scrapes, on-call debugging, manual GC inspection). A service missing `/healthz` blocks Kubernetes liveness probes; missing `/readiness` breaks rollout coordination; missing `/setloglevel` forces a StatefulSet edit + pod restart for every debug session. The endpoints are cheap (a few lines each, factory-built) and the cost of omitting one shows up at the worst possible time — usually during an incident.

#### Bad

```go
// main.go — admin server missing /setloglevel and /gc
router := mux.NewRouter()
router.Path("/healthz").Handler(libhttp.NewPrintHandler("OK"))
router.Path("/readiness").Handler(libhttp.NewPrintHandler("OK"))
router.Path("/metrics").Handler(promhttp.Handler())
// debug session means: edit StatefulSet -v=, restart pod, wait — every time
```

#### Good

```go
// main.go — all five canonical endpoints registered
router := mux.NewRouter()
router.Path("/healthz").Handler(libhttp.NewPrintHandler("OK"))
router.Path("/readiness").Handler(libhttp.NewPrintHandler("OK"))
router.Path("/metrics").Handler(promhttp.Handler())
router.Path("/setloglevel/{level}").
Handler(log.NewSetLoglevelHandler(ctx, log.NewLogLevelSetter(2, 5*time.Minute)))
router.Path("/gc").Handler(libhttp.NewGarbageCollectorHandler())
```

## Endpoint Catalog

| Endpoint | Required | Purpose | Library |
Expand All@@ -66,6 +97,29 @@ func (a *application) createHTTPServer(

## Port Conventions

### RULE go-http-service/admin-port-9090 (MUST)

**Owner**: go-http-handler-assistant
**Applies when**: a Go service's admin HTTP server (the one serving `/healthz`, `/metrics`, etc.) defaults to a port other than `9090` — either hardcoded or via a `--listen` flag/env-var whose default isn't `:9090`.
**Enforcement**: judgment (config inspection in `main.go` / `application` struct; ast-grep partial: struct-tag `default:":9090"` on the Listen field)
**Why**: 9090 is the cross-service contract for the admin endpoint. Prometheus scrape configs, the gateway's `admin/port: '9090'` annotation, the operator's muscle memory for `kubectl port-forward 9090`, and shared tooling all assume it. A custom port per service breaks scrape configs (Prometheus probes the wrong port → no metrics), breaks the gateway's auto-routing (admin URL doesn't resolve), and forces every operator to look up the per-service port before they can curl an endpoint at 3am. The deviation cost is real; the standardisation cost is zero.

#### Bad

```go
type application struct {
Listen string `required:"false" arg:"listen" env:"LISTEN" default:":8080"` // custom port
}
```

#### Good

```go
type application struct {
Listen string `required:"false" arg:"listen" env:"LISTEN" default:":9090"`
}
```

- **Always `9090`** for the admin HTTP server. Mirrors the standard across all bborbe services and the Prometheus scrape annotations.
- Listen address comes from a flag/env: `Listen string \`required:"false" arg:"listen" env:"LISTEN" default:":9090"\``.
- Public API (frontend-accessible data) lives on a different prefix (`/api/1.0/...`) — typically same port via the same router, or a separate listener.
Expand Down
54 changes: 54 additions & 0 deletions rules/index.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -116,6 +116,24 @@
"level": "SHOULD",
"owner": "go-architecture-assistant"
},
{
"anchor": "go-cli/cobra-not-stdlib-flag",
"applies_when": "a Go CLI binary's `main.go` / `pkg/cli/...` imports `flag` (stdlib) and calls `flag.String` / `flag.Bool` / `flag.Parse`, instead of using `github.com/spf13/cobra` (with its `pflag` library).",
"doc_path": "docs/go-cli-guide.md",
"enforcement": "judgment (ast-grep follow-up: `import \"flag\"` in any `main` package + `call_expression` matching `flag.Parse` / `flag.String`. Test files exempt.)",
"id": "go-cli/cobra-not-stdlib-flag",
"level": "MUST",
"owner": "go-quality-assistant"
},
{
"anchor": "go-cli/slog-not-glog-in-new-projects",
"applies_when": "a *new* Go CLI binary (created after Go 1.21 release; no prior glog usage in the same module) imports `github.com/golang/glog`. Existing glog-using projects are exempt — they should not introduce slog and glog side by side.",
"doc_path": "docs/go-cli-guide.md",
"enforcement": "judgment (semantic — distinguishing \"new project\" from \"existing project mid-migration\" requires checking git history / module age; ast-grep partial: `import \"github.com/golang/glog\"` in any main module without prior glog usage)",
"id": "go-cli/slog-not-glog-in-new-projects",
"level": "MUST",
"owner": "go-quality-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.",
Expand DownExpand Up@@ -143,6 +161,24 @@
"level": "SHOULD",
"owner": "go-context-assistant"
},
{
"anchor": "go-cqrs/auto-tx-wrapper-no-manual-wrap",
"applies_when": "a Go CQRS consumer in this framework manually wraps its command executor with `kv.NewTransactionMiddleware` / similar transaction-management code instead of using `RunCommandConsumerTx` (which auto-wraps).",
"doc_path": "docs/go-cqrs.md",
"enforcement": "judgment (ast-grep partial: `call_expression` matching `NewTransactionMiddleware` / `Wrap...` patterns adjacent to a CQRS command consumer registration)",
"id": "go-cqrs/auto-tx-wrapper-no-manual-wrap",
"level": "MUST",
"owner": "go-architecture-assistant"
},
{
"anchor": "go-cqrs/skipped-not-nil-for-non-retryable",
"applies_when": "a Go CQRS executor handles a *non-retryable* condition (idempotent skip, command targets an already-terminal entity, duplicate detected, validation against immutable state failed) by returning `nil` or an arbitrary `err` instead of `ErrCommandObjectSkipped`.",
"doc_path": "docs/go-cqrs.md",
"enforcement": "judgment (semantic — distinguishing \"non-retryable skip\" from \"transient error worth a Failure result\" requires reading the executor's intent)",
"id": "go-cqrs/skipped-not-nil-for-non-retryable",
"level": "MUST",
"owner": "go-architecture-assistant"
},
{
"anchor": "go-doc/comment-starts-with-name",
"applies_when": "an exported identifier has a doc comment whose first word does not match the identifier's name exactly (case-sensitive).",
Expand DownExpand Up@@ -323,6 +359,24 @@
"level": "MUST",
"owner": "go-http-handler-assistant"
},
{
"anchor": "go-http-service/admin-port-9090",
"applies_when": "a Go service's admin HTTP server (the one serving `/healthz`, `/metrics`, etc.) defaults to a port other than `9090` — either hardcoded or via a `--listen` flag/env-var whose default isn't `:9090`.",
"doc_path": "docs/go-http-service-guide.md",
"enforcement": "judgment (config inspection in `main.go` / `application` struct; ast-grep partial: struct-tag `default:\":9090\"` on the Listen field)",
"id": "go-http-service/admin-port-9090",
"level": "MUST",
"owner": "go-http-handler-assistant"
},
{
"anchor": "go-http-service/canonical-admin-endpoints",
"applies_when": "a Go service's admin HTTP server (port 9090, mounted under `/admin/<svc>/...` by the gateway) is missing any of the five always-required endpoints: `/healthz`, `/readiness`, `/metrics`, `/setloglevel/{level}`, `/gc`.",
"doc_path": "docs/go-http-service-guide.md",
"enforcement": "judgment (route-table inspection in `main.go` / handler factory; ast-grep partial: `router.Handle(...)` / `mux.Handle(...)` patterns checked against the five-endpoint set)",
"id": "go-http-service/canonical-admin-endpoints",
"level": "MUST",
"owner": "go-http-handler-assistant"
},
{
"anchor": "go-json-error-handler/structured-response-shape",
"applies_when": "an HTTP handler in a Go service emits an error response that is not a JSON object with the canonical `{error: {code, message, details}}` shape — e.g. plain-text bodies, top-level `{message: ...}` without an `error` wrapper, or `details` as a string instead of a `map[string]string`.",
Expand Down
Loading