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 @@ -51,6 +51,9 @@ Each enforceable guide in `docs/` should have a matching agent in `agents/`. The
| `go-http-service-guide.md` | `go-http-handler-assistant` |
| `go-cqrs.md` | `go-architecture-assistant` |
| `go-cli-guide.md` | `go-quality-assistant` |
| `go-enum-type-pattern.md` | `go-architecture-assistant` |
| `go-composition.md` | `go-architecture-assistant` |
| `go-build-args-guide.md` | `go-quality-assistant` |

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

Expand Down
32 changes: 32 additions & 0 deletions docs/go-build-args-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,38 @@ This guide defines the canonical layout. Reference implementation: `~/Documents/

**Why `--dirty`**: a `make buca` accidentally run from a worktree with uncommitted changes tags the image as `v0.52.7-dirty`, making it obvious in logs that the deployed binary isn't reproducible from the declared version. Catches mistakes like building from the dark-factory-active master branch while it's mid-commit.

### RULE go-build-args/three-args-required (MUST)

**Owner**: go-quality-assistant
**Applies when**: a Go service's `Makefile.docker` / `Dockerfile` is missing any of the three canonical `--build-arg` values: `BUILD_GIT_VERSION` (`git describe --tags --always --dirty`), `BUILD_GIT_COMMIT` (`git rev-parse --short HEAD`), `BUILD_DATE` (`date -u +%Y-%m-%dT%H:%M:%SZ`). Equivalent: the `main.go` `var` block doesn't declare matching `buildGitVersion` / `buildGitCommit` / `buildDate` variables wired via `-ldflags "-X"`.
**Enforcement**: judgment (file-grep across `Makefile.docker` for the three `--build-arg` lines + `Dockerfile` ARG/ENV declarations + `main.go` ldflag-wired var declarations)
**Why**: All three answer different runtime questions. `BUILD_GIT_VERSION` answers "which release is running?" — friendly for operators when builds are tagged, falls back to short SHA when untagged, gains `-dirty` suffix when built from a tree with uncommitted changes (catching `make buca` accidents from mid-rebase worktrees). `BUILD_GIT_COMMIT` answers "exact source hash?" — unambiguous when tags get moved or shared across history. `BUILD_DATE` answers "when was this image built?" — separates dev/staging/prod builds with identical source. Omitting any one removes the answer to its specific question. The cost is three lines per file; the value is "what shipped?" being a one-line log read instead of a forensic investigation.

#### Bad

```makefile
# Makefile.docker — missing BUILD_DATE; dev/staging/prod builds with same SHA
# are indistinguishable in logs
build:
docker build \
--build-arg BUILD_GIT_VERSION=$$(git describe --tags --always --dirty) \
--build-arg BUILD_GIT_COMMIT=$$(git rev-parse --short HEAD) \
-t $(REGISTRY)/$(SERVICE):$(BRANCH) -f Dockerfile .
```

#### Good

```makefile
# Makefile.docker — all three args, canonical order (operators read version first)
build:
DOCKER_BUILDKIT=1 \
docker build \
--build-arg BUILD_GIT_VERSION=$$(git describe --tags --always --dirty) \
--build-arg BUILD_GIT_COMMIT=$$(git rev-parse --short HEAD) \
--build-arg BUILD_DATE=$$(date -u +%Y-%m-%dT%H:%M:%SZ) \
-t $(REGISTRY)/$(SERVICE):$(BRANCH) -f Dockerfile .
```

## Layout

Four files per service participate.
Expand Down
120 changes: 118 additions & 2 deletions docs/go-composition.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,69 @@

Orchestrators compose small single-responsibility services. Never call package-level functions directly from business logic.

### RULE go-composition/no-package-function-calls-in-business-logic (MUST)

**Owner**: go-architecture-assistant
**Applies when**: a Go service method calls a top-level package function (`prompt.ListQueued(...)`, `git.CommitAndRelease(...)`) directly instead of receiving the equivalent capability through an injected interface declared in the same package as the consumer.
**Enforcement**: judgment (ast-grep follow-up: `call_expression` of the form `<package>.<Function>(...)` inside method bodies, where `<package>` is a non-stdlib non-bborbe-library import; the agent rules out leaf packages whose functions are pure helpers).
**Why**: Direct package-function calls are hidden dependencies. The constructor doesn't surface them, tests can't mock them, and replacing the implementation requires editing every call site instead of swapping one constructor argument. Wrapping each capability in a small interface (`PromptScanner`, `Releaser`) makes the dep graph explicit at the type signature, makes Counterfeiter-mockable points obvious, and lets factories swap real for fake without touching business logic. The cost is one interface declaration per wrapped capability; the value is testability + composability + Single Responsibility at the right granularity.

#### Bad

```go
// runner calls package functions directly — untestable, uncomposable
type runner struct {
dir string
executor Executor // only this dep is visible at the constructor
}

func (r *runner) Run(ctx context.Context) error {
prompt.ResetExecuting(ctx, r.dir) // hidden dep on package prompt
prompt.NormalizeFilenames(ctx, r.dir) // hidden dep
queued, _ := prompt.ListQueued(ctx, r.dir) // hidden dep
for _, p := range queued {
r.executor.Execute(ctx, p)
}
git.CommitAndRelease(ctx, "release") // hidden dep on package git
return nil
}
```

#### Good

```go
// Runner is the orchestration interface — small (1 method), drives the workflow
// via injected dependencies declared as their own interfaces.
type Runner interface {
Run(ctx context.Context) error
}

// PromptScanner returns the queued prompts that have not yet been executed.
type PromptScanner interface {
ListQueued(ctx context.Context) ([]Prompt, error)
}

// Releaser commits and tags the most recent execution batch.
type Releaser interface {
CommitAndRelease(ctx context.Context, title string) error
}

// runner is the canonical Runner implementation. All deps surfaced as
// injected interfaces — constructor tells the full story.
type runner struct {
scanner PromptScanner
executor Executor
releaser Releaser
}

// NewRunner returns a Runner wired with the given scanner, executor, and
// releaser. Each dep is a separate interface so factories can swap real for
// fake without touching business logic.
func NewRunner(scanner PromptScanner, executor Executor, releaser Releaser) Runner {
return &runner{scanner: scanner, executor: executor, releaser: releaser}
}
```

## Anti-Pattern: God Object

```go
Expand Down Expand Up @@ -58,10 +121,63 @@ func CreateRunner(promptsDir string) Runner {
}
```

### RULE go-composition/small-interfaces-1-2-methods (SHOULD)

**Owner**: go-architecture-assistant
**Applies when**: a Go interface declares 3+ methods spanning more than one logical capability — i.e. consumers of the interface use a subset of methods, and the unused-by-this-consumer methods are inferred from method-count plus call-site analysis.
**Enforcement**: judgment (ast-grep follow-up: `type_declaration` with `interface_type` body containing 3+ method specifications; the "one logical capability" check is semantic)
**Why**: The Go convention is "the bigger the interface, the weaker the abstraction" (Rob Pike). A 5-method interface forces every consumer to depend on all 5 methods even when they only use 1 — Counterfeiter generates 5 stubs per test, refactors propagate everywhere, and Interface Segregation Principle violations breed. 1-2 method interfaces are easier to mock, easier to compose, and make each consumer's actual dep surface visible at the type signature. Existing standard library interfaces (`io.Reader`, `io.Writer`, `sort.Interface`, `error`) show the shape: small, focused, composable. SHOULD-level because composition cases (`io.ReadWriteCloser`) and certain framework interfaces legitimately bundle several methods.

#### Bad

```go
// Fat interface — consumers that only need to log get the kitchen sink
type Service interface {
Log(msg string)
Save(ctx context.Context, item Item) error
Load(ctx context.Context, id string) (Item, error)
Delete(ctx context.Context, id string) error
Notify(ctx context.Context, evt Event) error
}
```

#### Good

```go
// Logger emits a single log line. Tiny interface — one method.
type Logger interface {
Log(ctx context.Context, msg string)
}

// ItemStore is the CRUD-like persistence interface for Item entities.
// Three methods because they form one coherent capability (item lifecycle).
type ItemStore interface {
Save(ctx context.Context, item Item) error
Load(ctx context.Context, id string) (Item, error)
Delete(ctx context.Context, id string) error
}

// Notifier publishes an event to the downstream notification fabric.
type Notifier interface {
Notify(ctx context.Context, evt Event) error
}

// Service composes the three focused interfaces above. Consumers that need
// only one capability take only that one as a parameter — Service is the
// type for the rare consumer that legitimately needs all three.
// Embedded interfaces satisfy ISP: each focused interface is independently
// mockable; Service just declares the union.
type Service interface {
Logger
ItemStore
Notifier
}
```

## Rules

1. **Small interfaces** — 1-2 methods per interface (SRP)
2. **All deps via constructor** — never call `pkg.Function()` from business logic
1. **Small interfaces** — 1-2 methods per interface (canonicalised as `go-composition/small-interfaces-1-2-methods`)
2. **All deps via constructor** — never call `pkg.Function()` from business logic (canonicalised as `go-composition/no-package-function-calls-in-business-logic`)
3. **Constructor shows intent** — reading `NewRunner(scanner, executor, releaser)` tells you exactly what it needs
4. **Package functions → wrap in interface** — if you call `git.Push()` directly, extract a `Pusher` interface
5. **Factory composes** — `CreateRunner()` wires all small services together
Expand Down
94 changes: 94 additions & 0 deletions docs/go-enum-type-pattern.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,64 @@ Use this pattern when you need:

## Core Pattern Structure

### RULE go-enum-type/typed-constants-with-collection (MUST)

**Owner**: go-architecture-assistant
**Applies when**: a Go package introduces a finite set of enum-like values (status codes, kinds, modes, etc.) declared as untyped `const` strings or `const` ints, without a paired typed `string`/`int` newtype AND without an `Available<Name>s` collection containing every valid value.
**Enforcement**: judgment (ast-grep follow-up: `const_declaration` of `string` literals with shared semantic prefix; pair-check against the existence of a `type X string` newtype + a `var AvailableXs Xs` collection in the same package)
**Why**: Untyped enum constants spread through the codebase as bare strings — every call site can pass any string, every comparison can typo, and the linter has no signal that "scheduled" / "completed" / "pending" are meant to be members of a closed set. Typed constants paired with an `AvailableXs` collection turn the enum into a first-class type the type-checker enforces and that `Validate()` can range over. The `Available*` collection is what makes the pattern self-describing — a new contributor reads the package and immediately sees the full value space.

#### Bad

```go
// Untyped string constants — every call site can pass any string
const (
OrderStatusPending = "pending"
OrderStatusProcessing = "processing"
OrderStatusCompleted = "completed"
OrderStatusFailed = "failed"
)

func Process(status string) error { // accepts any string; typo silent
if status == "compleated" { ... } // compiles cleanly; runtime no-op
}
```

#### Good

```go
// OrderStatus is the closed set of lifecycle states an Order may occupy.
type OrderStatus string

// Recognised OrderStatus values. The Available* collection below mirrors
// this set — keep both in sync.
const (
PendingOrderStatus OrderStatus = "pending"
ProcessingOrderStatus OrderStatus = "processing"
CompletedOrderStatus OrderStatus = "completed"
FailedOrderStatus OrderStatus = "failed"
)

// OrderStatuses is a collection of OrderStatus values, used by Validate to
// check membership in the closed set.
type OrderStatuses []OrderStatus

// AvailableOrderStatuses lists every OrderStatus value the system accepts.
// Validate() ranges over this collection; tests and dispatchers iterate it.
var AvailableOrderStatuses = OrderStatuses{
PendingOrderStatus,
ProcessingOrderStatus,
CompletedOrderStatus,
FailedOrderStatus,
}

// Process advances an order to the next state for the given status.
// Returns an error if the status is unknown or the order is in a terminal state.
func Process(ctx context.Context, status OrderStatus) error { // type-checked at call site
// ...
}
```

### Minimal Complete Implementation

```go
Expand Down Expand Up @@ -68,6 +126,42 @@ func (o OrderStatuses) Contains(status OrderStatus) bool {
}
```

### RULE go-enum-type/validate-against-available-collection (MUST)

**Owner**: go-architecture-assistant
**Applies when**: a Go enum type's `Validate(ctx context.Context) error` method validates against an inline switch / hardcoded value list / regex, instead of `AvailableXs.Contains(value)`.
**Enforcement**: judgment (ast-grep follow-up: `method_declaration` named `Validate` on an enum-shaped type, body containing inline `switch` / `||` chain over string literals, paired with the package having a defined `AvailableXs` collection)
**Why**: Validating against an inline switch duplicates the enum's value space — adding a new enum constant requires updating both `const (...)` and the `Validate()` body, and the type checker can't enforce the pair. Range-over-`AvailableXs` collapses the two into one declaration: the collection IS the validation source, so the only place to add a value is the collection literal. Adds-a-constant-but-forgets-to-update-validate becomes structurally impossible.

#### Bad

```go
func (o OrderStatus) Validate(ctx context.Context) error {
switch o {
case PendingOrderStatus, ProcessingOrderStatus,
CompletedOrderStatus, FailedOrderStatus:
return nil
default:
return errors.Wrapf(ctx, validation.Error, "unknown order status '%s'", o)
}
// Adding a new status to const(...) without updating this switch
// produces silent validation failures.
}
```

#### Good

```go
func (o OrderStatus) Validate(ctx context.Context) error {
if !AvailableOrderStatuses.Contains(o) {
return errors.Wrapf(ctx, validation.Error, "unknown order status '%s'", o)
}
return nil
}
// Adding a new status to AvailableOrderStatuses automatically extends
// the validation surface. Single source of truth.
```

## Implementation Checklist

When creating a new enum type, ensure you have:
Expand Down
45 changes: 45 additions & 0 deletions rules/index.json
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,15 @@
"level": "SHOULD",
"owner": "go-architecture-assistant"
},
{
"anchor": "go-build-args/three-args-required",
"applies_when": "a Go service's `Makefile.docker` / `Dockerfile` is missing any of the three canonical `--build-arg` values: `BUILD_GIT_VERSION` (`git describe --tags --always --dirty`), `BUILD_GIT_COMMIT` (`git rev-parse --short HEAD`), `BUILD_DATE` (`date -u +%Y-%m-%dT%H:%M:%SZ`). Equivalent: the `main.go` `var` block doesn't declare matching `buildGitVersion` / `buildGitCommit` / `buildDate` variables wired via `-ldflags \"-X\"`.",
"doc_path": "docs/go-build-args-guide.md",
"enforcement": "judgment (file-grep across `Makefile.docker` for the three `--build-arg` lines + `Dockerfile` ARG/ENV declarations + `main.go` ldflag-wired var declarations)",
"id": "go-build-args/three-args-required",
"level": "MUST",
"owner": "go-quality-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).",
Expand All @@ -134,6 +143,24 @@
"level": "MUST",
"owner": "go-quality-assistant"
},
{
"anchor": "go-composition/no-package-function-calls-in-business-logic",
"applies_when": "a Go service method calls a top-level package function (`prompt.ListQueued(...)`, `git.CommitAndRelease(...)`) directly instead of receiving the equivalent capability through an injected interface declared in the same package as the consumer.",
"doc_path": "docs/go-composition.md",
"enforcement": "judgment (ast-grep follow-up: `call_expression` of the form `<package>.<Function>(...)` inside method bodies, where `<package>` is a non-stdlib non-bborbe-library import; the agent rules out leaf packages whose functions are pure helpers).",
"id": "go-composition/no-package-function-calls-in-business-logic",
"level": "MUST",
"owner": "go-architecture-assistant"
},
{
"anchor": "go-composition/small-interfaces-1-2-methods",
"applies_when": "a Go interface declares 3+ methods spanning more than one logical capability — i.e. consumers of the interface use a subset of methods, and the unused-by-this-consumer methods are inferred from method-count plus call-site analysis.",
"doc_path": "docs/go-composition.md",
"enforcement": "judgment (ast-grep follow-up: `type_declaration` with `interface_type` body containing 3+ method specifications; the \"one logical capability\" check is semantic)",
"id": "go-composition/small-interfaces-1-2-methods",
"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.",
Expand Down Expand Up @@ -215,6 +242,24 @@
"level": "SHOULD",
"owner": "godoc-assistant"
},
{
"anchor": "go-enum-type/typed-constants-with-collection",
"applies_when": "a Go package introduces a finite set of enum-like values (status codes, kinds, modes, etc.) declared as untyped `const` strings or `const` ints, without a paired typed `string`/`int` newtype AND without an `Available<Name>s` collection containing every valid value.",
"doc_path": "docs/go-enum-type-pattern.md",
"enforcement": "judgment (ast-grep follow-up: `const_declaration` of `string` literals with shared semantic prefix; pair-check against the existence of a `type X string` newtype + a `var AvailableXs Xs` collection in the same package)",
"id": "go-enum-type/typed-constants-with-collection",
"level": "MUST",
"owner": "go-architecture-assistant"
},
{
"anchor": "go-enum-type/validate-against-available-collection",
"applies_when": "a Go enum type's `Validate(ctx context.Context) error` method validates against an inline switch / hardcoded value list / regex, instead of `AvailableXs.Contains(value)`.",
"doc_path": "docs/go-enum-type-pattern.md",
"enforcement": "judgment (ast-grep follow-up: `method_declaration` named `Validate` on an enum-shaped type, body containing inline `switch` / `||` chain over string literals, paired with the package having a defined `AvailableXs` collection)",
"id": "go-enum-type/validate-against-available-collection",
"level": "MUST",
"owner": "go-architecture-assistant"
},
{
"anchor": "go-errors/inner-closure-no-double-wrap",
"applies_when": "an inner closure (passed to `db.Update`, `filepath.WalkDir`, or similar callback APIs) calls `errors.Wrap`/`errors.Wrapf` while the surrounding function ALSO wraps the closure's return value.",
Expand Down
Loading