diff --git a/CLAUDE.md b/CLAUDE.md index 79f54cf..3cb3108 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,6 +57,24 @@ Each enforceable guide in `docs/` should have a matching agent in `agents/`. The | `go-mod-dependency-fix-guide.md` | `go-quality-assistant` | | `go-makefile-commands.md` | `go-quality-assistant` | | `go-patterns.md` | `go-quality-assistant` | +| `tdd-guide.md` | `go-test-quality-assistant` | +| `changelog-guide.md` | `agent-auditor` | +| `git-workflow.md` | `agent-auditor` | +| `teamvault-conventions.md` | `go-security-specialist` | +| `go-library-guide.md` | `go-quality-assistant` | +| `test-pyramid-triggers.md` | `go-test-quality-assistant` | +| `go-k8s-binary-conventions.md` | `go-security-specialist` (secret-handling) + `go-quality-assistant` (struct conventions) | +| `markdown-todo-guide.md` | `agent-auditor` | +| `claude-md-guide.md` | `agent-auditor` | +| `k8s-manifest-guide.md` | `go-architecture-assistant` | +| `go-filter-pattern.md` | `go-architecture-assistant` | +| `go-parse-pattern.md` | `go-quality-assistant` | +| `go-validation-framework-guide.md` | `go-quality-assistant` | +| `go-tools-versioning-guide.md` | `go-quality-assistant` | +| `readme-guide.md` | `agent-auditor` | +| `go-boolean-combinator-pattern.md` | `go-architecture-assistant` | +| `python-factory-pattern.md` | `python-architecture-assistant` | +| `adr-guide.md` | `go-architecture-assistant` | Reference-only docs (patterns, setup guides) don't need agents. diff --git a/docs/adr-guide.md b/docs/adr-guide.md index 7c8b368..ff624f4 100644 --- a/docs/adr-guide.md +++ b/docs/adr-guide.md @@ -14,6 +14,13 @@ ADRs provide: **Critical for AI assistants:** ADRs enable AI to understand not just WHAT the architecture is, but WHY it is that way, preventing inappropriate suggestions that conflict with documented decisions. +### RULE adr/required-for-irreversible-architecture-decisions (SHOULD) + +**Owner**: go-architecture-assistant +**Applies when**: a PR introduces an irreversible architectural change (database technology choice, message-bus selection, framework swap, major topology change like blue-green vs canary) without a corresponding ADR document under `docs/adr/NNNN-.md` capturing the decision, alternatives considered, rationale, and consequences. +**Enforcement**: judgment (semantic — distinguishing "irreversible architectural decision" from "tactical implementation choice" requires reading the change scope; symptom: PR introduces new top-level package or external dependency without ADR reference) +**Why**: Six months from now, the next contributor will read the code and ask "why did we pick X over Y?" — and the answer needs to be already-written, not "ask Alice, she remembers." ADRs make the decision durable: they capture the alternatives the team rejected, the trade-offs at the time, the constraints that made one option preferable. Without the ADR, future contributors either rediscover the same constraints (waste effort) or quietly revert to the rejected option (waste effort + the original problem reappears). SHOULD because the line between "irreversible" and "tactical" is judgment. + ## When to Create an ADR Create an ADR when: diff --git a/docs/changelog-guide.md b/docs/changelog-guide.md index b79378a..32c59aa 100644 --- a/docs/changelog-guide.md +++ b/docs/changelog-guide.md @@ -35,6 +35,35 @@ Please choose versions by [Semantic Versioning](http://semver.org/). - fix: Fix WaiterUntil to handle equal times correctly ``` +### RULE changelog/preamble-frozen (MUST) + +**Owner**: agent-auditor +**Applies when**: a CHANGELOG.md edit inserts content above the `# Changelog` title, modifies the SemVer preamble bullets (MAJOR/MINOR/PATCH), or places a `## Unreleased` / `## vX.Y.Z` section inside (rather than after) the preamble block. +**Enforcement**: judgment (markdown-structure inspection: every CHANGELOG.md must have the canonical preamble first, then sections in `## Unreleased` → `## vX.Y.Z` order) +**Why**: The preamble is the API contract between the changelog and every tool that parses it (dark-factory's version-bump detector, /coding:commit's CHANGELOG validator, downstream release-notes generators). Moving / deleting / shifting it breaks the parsers silently — the next release attempt either bumps the wrong version or misses entries entirely. Restoring is cheap; preventing the edit is cheaper. + +#### Bad + +```markdown +## Unreleased +- feat: new thing + +# Changelog ← preamble shoved below +All notable changes... +``` + +#### Good + +```markdown +# Changelog +All notable changes to this project will be documented in this file. +Please choose versions by [Semantic Versioning](http://semver.org/). +* MAJOR / MINOR / PATCH bullets here + +## Unreleased +- feat: new thing +``` + **Rules:** - Preamble with SemVer explanation always present - **Header is frozen**: everything from the start of the file to the FIRST `##` heading (the `# Changelog` title, the "All notable changes..." line, the SemVer link, and the MAJOR/MINOR/PATCH bullets) MUST NOT be moved, deleted, or have anything inserted above or inside it. Insert `## Unreleased` (or any version section) immediately AFTER the last header line — never before any header line. If the header is incomplete, restore it; never leave it partial. @@ -45,6 +74,31 @@ Please choose versions by [Semantic Versioning](http://semver.org/). ## Conventional Prefixes (REQUIRED) +### RULE changelog/conventional-prefix-required (MUST) + +**Owner**: agent-auditor +**Applies when**: a bullet under `## Unreleased` in CHANGELOG.md does not start with one of the recognised conventional prefixes (`feat:`, `fix:`, `refactor:`, `test:`, `docs:`, `chore:`, `perf:`). +**Enforcement**: judgment (regex over `## Unreleased` bullets: `^- ([a-z]+:)` first token must be in the allowed prefix set) +**Why**: dark-factory and `/coding:commit` parse the prefix to decide the version bump automatically — any `feat:` entry triggers a minor bump, everything else triggers a patch. Missing or wrong prefix means the version-bump detection fails: the release may patch-bump a feature commit (downstream consumers miss the new functionality in their range queries) or minor-bump a chore. Standardising the prefix is the cheapest possible structure for unambiguous machine parsing. + +#### Bad + +```markdown +## Unreleased +- Add SpecWatcher ← no prefix +- update go and deps ← no prefix +- fix and refactor ← ambiguous; multiple prefixes +``` + +#### Good + +```markdown +## Unreleased +- feat: Add SpecWatcher to monitor specs/ for approved status changes +- chore: Update Go from 1.25.5 to 1.26.0 +- refactor: Extract worktree cleanup to reduce cognitive complexity +``` + Every `## Unreleased` entry must start with a conventional prefix: | Prefix | Meaning | Version bump | diff --git a/docs/claude-code-skill-writing-guide.md b/docs/claude-code-skill-writing-guide.md index aed2b00..ebff096 100644 --- a/docs/claude-code-skill-writing-guide.md +++ b/docs/claude-code-skill-writing-guide.md @@ -4,6 +4,20 @@ Tags: [[Claude Code]] [[Claude Code Plugin System]] [[Claude Code Agent Developm How to write Claude Code skills — self-contained capabilities that auto-activate based on conversation context. +### RULE skill-writing/scripts-in-scripts-subdir (MUST) + +**Owner**: skill-auditor +**Applies when**: a Claude Code skill places executable scripts (`*.sh`, `*.py`) directly alongside `SKILL.md` instead of in a `scripts/` subdirectory. +**Enforcement**: judgment (file-layout check on `skills/<name>/` — only `SKILL.md` at top-level; scripts under `scripts/`) +**Why**: The `scripts/` subdirectory keeps `SKILL.md` discoverable at a glance (one file at top level), groups all executables under a single permission-allowed glob pattern (`Bash(scripts/*.sh)`), and matches the convention every existing bborbe skill follows. Loose-next-to-SKILL.md scripts produce ambiguity ("is this part of the skill or a stray script?") and require enumerating individual files in the skill's `allowed-tools`. + +### RULE skill-writing/skill-md-frontmatter-required (MUST) + +**Owner**: skill-auditor +**Applies when**: a `skills/<name>/SKILL.md` file is missing the required frontmatter fields — `name:` (must match the directory name) and `description:` (Claude's discovery signal). +**Enforcement**: judgment (YAML-frontmatter inspection: presence of `name` + `description` at the top of every SKILL.md) +**Why**: `description:` is the trigger phrase Claude pattern-matches against conversation context to auto-activate the skill. Without it, the skill is invisible to autonomous discovery — users must type the full `/plugin:skill-name` slash command every time. `name:` is the dispatch key the runtime resolves; mismatch with the directory name produces 404s on invocation. Both fields are cheap to add and break the skill loudly if absent. + ## Structure ``` diff --git a/docs/claude-md-guide.md b/docs/claude-md-guide.md index 662fb5a..fdc42d8 100644 --- a/docs/claude-md-guide.md +++ b/docs/claude-md-guide.md @@ -2,6 +2,13 @@ Guide for writing CLAUDE.md files. CLAUDE.md is operational context for AI agents working in the codebase — it tells them how to change the code safely. +### RULE claude-md/agent-context-not-user-docs (MUST) + +**Owner**: agent-auditor +**Applies when**: a project's CLAUDE.md duplicates README.md user-facing content (install instructions, feature marketing, usage tutorials) instead of serving as terse agent-operational context (build commands, architecture map, constraints). +**Enforcement**: judgment (semantic — distinguishing "agent needs this to change the code" from "user needs this to use it" requires reading the content) +**Why**: CLAUDE.md exists to make AI agents safe + productive in the codebase. Duplicating README content bloats the agent's per-turn context, costs tokens, drowns the actually-load-bearing rules (build commands, ban lists, version-alignment requirements) in marketing copy. Tone signals the audience: README is welcoming and explanatory; CLAUDE.md is terse and imperative. Agents that read CLAUDE.md expect "do this, never that"; users expect "here's what this project does." + ## CLAUDE.md vs README.md | | README.md | CLAUDE.md | diff --git a/docs/git-workflow.md b/docs/git-workflow.md index f343011..87c247e 100644 --- a/docs/git-workflow.md +++ b/docs/git-workflow.md @@ -2,6 +2,52 @@ ## Hard Rules +### RULE git-workflow/never-direct-commit-to-master (MUST) + +**Owner**: agent-auditor +**Applies when**: a git commit lands directly on `master` / `main` without going through a feature branch + PR — typically caught by `pre-push` hook rejecting cross-name pushes to the default branch, or by GitHub's `master-protection` ruleset. +**Enforcement**: judgment + tooling (`~/.git-hooks/pre-push` rejects feature-branch-to-master pushes; GitHub ruleset enforces required PR). Release commits (`release vX.Y.Z`) are the documented exception. +**Why**: Direct-to-master commits skip review, skip CI, skip the audit trail. The 14-commits-on-origin-master trap (PR #1) happened this way: `git worktree add -b feat/foo origin/master` left upstream pointing at master so `git push` shipped to the wrong place. Hook + ruleset catch it before it happens. + +#### Bad + +```bash +git checkout master && git commit -m "quick fix" && git push # no PR, no review +``` + +#### Good + +```bash +wt-feat coding fix/payment-bug # feature worktree + branch + push -u in one shot +git commit -m "fix: payment bug" && git push +gh pr create # PR triggers review + CI +``` + +### RULE git-workflow/no-ai-attribution-in-commits (MUST) + +**Owner**: agent-auditor +**Applies when**: a git commit message body or trailer contains "Co-Authored-By: Claude", "Generated with Claude Code", "Co-Authored-By: GitHub Copilot", or any other AI-attribution line. +**Enforcement**: judgment (commit-message grep at PR-review time; can be a `commit-msg` hook reject) +**Why**: AI attribution inflates commit metadata noise, signals tool use rather than authorship, and makes commits look "automated" even when the human did substantive design + review work. The human + commit message together constitute the canonical history. AI is a tool; tools don't get authorship credit any more than the editor or compiler does. + +#### Bad + +``` +add login endpoint + +Generated with Claude Code +Co-Authored-By: Claude <noreply@anthropic.com> +``` + +#### Good + +``` +add login endpoint + +Validates the OAuth token against the configured issuer, returns +the corresponding user record or 401. +``` + - **NEVER commit directly to master/main** - **NEVER add AI attribution** to commits (no "Co-Authored-By: Claude", no "Generated with Claude Code") - **NEVER use `git -C /path`** — always `cd /path && git ...` diff --git a/docs/go-boolean-combinator-pattern.md b/docs/go-boolean-combinator-pattern.md index 5c532d5..4caf197 100644 --- a/docs/go-boolean-combinator-pattern.md +++ b/docs/go-boolean-combinator-pattern.md @@ -19,6 +19,13 @@ Skip this pattern when: - Decisions need to share state during evaluation (use Chain of Responsibility instead) - The "decision" returns a value, not a bool (use Strategy or pipeline patterns) +### RULE go-boolean-combinator/result-with-description-not-naked-bool (MUST) + +**Owner**: go-architecture-assistant +**Applies when**: a Go boolean-combinator / decision interface's method returns a bare `bool` instead of a `Result` type carrying both the boolean decision AND a human-readable `Description() string` explaining the reason. +**Enforcement**: judgment (interface declaration check: decision-style interfaces (`Check`, `IsTrusted`, `Filtered`, `Allowed`, `Matches`) returning naked `bool`) +**Why**: Combinator decisions show up in audit logs, error messages, and UI tooltips — and "why did this evaluate to false?" is the question every consumer eventually asks. A naked `bool` answers "yes/no"; a `Result` with `Description()` answers "yes/no AND why". `And{}` compositions concatenate child descriptions ("requires X AND requires Y AND not Z"); `Or{}` reports which branch carried the decision. Without the description, debugging "why is this user blocked?" requires reproducing the full decision tree by hand. The cost is one extra method per interface; the value is decisions that explain themselves. + ## Components A boolean combinator pattern has five parts: diff --git a/docs/go-filter-pattern.md b/docs/go-filter-pattern.md index 313d0ac..6080507 100644 --- a/docs/go-filter-pattern.md +++ b/docs/go-filter-pattern.md @@ -11,6 +11,13 @@ Filters are predicates that determine whether data should be included or exclude 3. **Semantic Clarity**: Clear naming that matches user intent 4. **Performance Optimization**: Preprocessing to minimize runtime overhead +### RULE go-filter/document-filtered-semantics (MUST) + +**Owner**: go-architecture-assistant +**Applies when**: a Go filter / predicate interface uses ambiguous method names like `Match(item)` / `Check(item)` / `Apply(item)` without doc-comment clarifying whether `true` means "include" or "exclude" — OR uses contradictory naming (`Filtered` returning true for "passes the filter" instead of "filtered out"). +**Enforcement**: judgment (interface declaration check: method-name + doc-comment alignment for predicate methods returning bool) +**Why**: Filter semantics inversion is the textbook off-by-true bug — every consumer either gets all the records (filter inverted, treated as pass-through) or zero records (filter inverted, everything excluded). The `Filtered()` convention used in bborbe Go code returns true for "exclude" (the item HAS been filtered out); other codebases use the opposite. Pick one, document it in the interface comment, stick to it everywhere — and never let a new filter type use the opposite semantic in the same codebase. + ## Core Filter Interface ```go diff --git a/docs/go-k8s-binary-conventions.md b/docs/go-k8s-binary-conventions.md index 9d289e2..904dba7 100644 --- a/docs/go-k8s-binary-conventions.md +++ b/docs/go-k8s-binary-conventions.md @@ -16,6 +16,20 @@ Every Go binary that runs in a k8s pod (StatefulSet, Deployment, CronJob, epheme | `run.CancelOnFirstFinish(ctx, work..., httpServer)` | yes | One goroutine exit cancels the others; clean shutdown | | Auth via `application` struct fields, not `os.Getenv` | yes | Framework handles defaults + validation + redaction | +### RULE go-k8s-binary/secret-fields-need-display-length (MUST) + +**Owner**: go-security-specialist +**Applies when**: an `application` struct field in a Go k8s-deployed binary holds secret material (PEM key, OAuth token, password, API key, JWT signing secret) without the `display:"length"` tag — meaning glog / structured-logging dumps of the application config will print the secret value. +**Enforcement**: judgment (ast-grep follow-up: `struct_field_declaration` with name matching `PEM*` / `*Token*` / `*Secret*` / `*Password*` / `*Key` outside `display:"length"` tag) +**Why**: `argument.Parse()` prints the application config at startup. Without `display:"length"`, the secret value lands in stdout / glog / log aggregators — searchable, indexed, and impossible to redact retroactively once the log batch has shipped. `display:"length"` substitutes `length=42` for the value, preserving the "is it set?" signal without the leak. The tag costs zero runtime; the leak costs a credential rotation. + +### RULE go-k8s-binary/argument-struct-not-os-getenv (MUST) + +**Owner**: go-quality-assistant +**Applies when**: a Go k8s-deployed binary calls `os.Getenv("FOO")` to read a config / auth value that's already declared as an `application` struct field bound via `argument` tags. +**Enforcement**: judgment (ast-grep partial: `call_expression` matching `os.Getenv(...)` outside `main.go` early bootstrap) +**Why**: `argument.Parse()` already populates the struct field with the right precedence (CLI flag > env var > default), validates `required:"true"` fields at startup, and redacts via `display:"length"`. Calling `os.Getenv` directly duplicates the env-read, skips the validation, and bypasses the redaction (`os.Getenv("PEM_KEY")` returns the raw secret with no `display:"length"` involvement). Use `a.PEMKey` etc. + ## Application struct shape ```go diff --git a/docs/go-library-guide.md b/docs/go-library-guide.md index 24ce70d..628c890 100644 --- a/docs/go-library-guide.md +++ b/docs/go-library-guide.md @@ -94,6 +94,13 @@ ginkgo -v ## 📦 Versioning +### RULE go-library/semver-vprefix-tag-required (MUST) + +**Owner**: go-quality-assistant +**Applies when**: a public Go library repo cuts a release without a `git tag` matching `v<MAJOR>.<MINOR>.<PATCH>` (e.g. `v1.0.0`, `v0.12.3`) — either tagless commits, date-based tags (`2026-06-03`), or non-prefixed semver (`1.0.0` without the leading `v`). +**Enforcement**: judgment (git-tag inspection: `git tag --list` filtered against `^v[0-9]+\.[0-9]+\.[0-9]+$`) +**Why**: Go's module system parses tags as `vMAJOR.MINOR.PATCH` — consumers pin to versions via `go get github.com/x/y@v1.2.3`. A tag without the `v` prefix doesn't resolve as a module version; a date-tag doesn't either. Untagged commits force consumers to depend on pseudo-versions (`v0.0.0-20260403114524-913de8870914`), which work but are unreadable and don't survive Go's MVS upgrade logic predictably. The `v` prefix is a hard requirement of `go.mod`'s grammar; semver is the convention Go's module proxy is built on. + Tag releases using semantic versioning: ```bash diff --git a/docs/go-parse-pattern.md b/docs/go-parse-pattern.md index f0ffc51..65c6434 100644 --- a/docs/go-parse-pattern.md +++ b/docs/go-parse-pattern.md @@ -11,6 +11,13 @@ Use this pattern when you need: **Don't use** for simple type assertions where you control the type (use direct type assertion instead). +### RULE go-parse/paired-parse-and-parsedefault (MUST) + +**Owner**: go-quality-assistant +**Applies when**: a Go package adds a `ParseX(ctx, value)` function returning `(X, error)` without a paired `ParseXDefault(ctx, value, default) X` that suppresses the error and returns the default — OR vice versa (`ParseXDefault` without `ParseX`). +**Enforcement**: judgment (ast-grep follow-up: `function_declaration` named `Parse*` with `error` return type; pair-check against same-named `*Default` function in the same package) +**Why**: Two call sites consume parsed values: ones where parse failure is a real error worth bubbling (config validation, request parsing) and ones where it's a "fall back to default" condition (optional fields, legacy data with missing keys). Shipping only `ParseX` forces every default-using call site to `if err != nil { use default }` boilerplate; shipping only `ParseXDefault` hides real errors from call sites that need to know. The paired-API convention makes both call patterns one line at the call site and shares the actual parse implementation under the hood. + ## Core Pattern Structure The parse pattern consists of two complementary functions: diff --git a/docs/go-tools-versioning-guide.md b/docs/go-tools-versioning-guide.md index 0d401a6..495097d 100644 --- a/docs/go-tools-versioning-guide.md +++ b/docs/go-tools-versioning-guide.md @@ -9,6 +9,13 @@ How to version-pin CLI tools (linters, scanners, code generators) used by Go pro - **Do** invoke tools via `go run pkg@$(VERSION)` in the Makefile - **Do** invoke `//go:generate` directives with `go run pkg@version` (hardcoded version per directive) +### RULE go-tools-versioning/no-tools-go-for-clis (MUST) + +**Owner**: go-quality-assistant +**Applies when**: a Go project has a `tools.go` file with `//go:build tools` that imports CLI tools (`_ "github.com/golangci/golangci-lint/v2/cmd/golangci-lint"`, `_ "github.com/google/osv-scanner/v2/cmd/osv-scanner"`, etc.) — instead of declaring CLI versions in a separate `tools.env` consumed by the Makefile via `go run pkg@$(VERSION)`. +**Enforcement**: judgment (file-presence check: `tools.go` with `//go:build tools` header + CLI tool imports) +**Why**: `tools.go` pulls every transitive dependency of every CLI tool into your project's `go.mod`. A typical library ends up with 400+ indirect requires, most of which are lint-tool internals nobody uses at runtime. The cascade gets worse: downstream services depending on the library inherit the tool deps, conflicts produce permanent `replace` workarounds, and version conflicts force every developer to maintain the same broken workaround set. `tools.env` + `go run pkg@$(VERSION)` keeps tool versions pinned + reproducible without polluting `go.mod` — the tool runs in its own module space, your project stays clean. + ## Why Not `tools.go`? The historical pattern was a `tools.go` file with `//go:build tools` that imports each CLI tool: diff --git a/docs/go-validation-framework-guide.md b/docs/go-validation-framework-guide.md index 6e9ab39..c443980 100644 --- a/docs/go-validation-framework-guide.md +++ b/docs/go-validation-framework-guide.md @@ -23,6 +23,13 @@ The `github.com/bborbe/validation` library provides a declarative approach to va - **Type-safe**: Works with Go's type system and generics - **Context-aware**: All validation functions accept `context.Context` +### RULE go-validation/use-bborbe-validation-not-inline-checks (MUST) + +**Owner**: go-quality-assistant +**Applies when**: a Go service implements input validation via hand-rolled `if`/`if-else` chains in a `Validate(ctx context.Context) error` method or HTTP handler, instead of using `github.com/bborbe/validation` (`validation.All{...}` + `validation.Name(...)` + `validation.NotEmptyString(...)` etc.). +**Enforcement**: judgment (ast-grep follow-up: `method_declaration` named `Validate` whose body contains 3+ inline `if v == "" || v == ""` chains with manual `errors.Errorf` calls, in a package not importing `github.com/bborbe/validation`) +**Why**: Hand-rolled validation drifts: every package writes its own "is this empty?" check, error message format, field-naming convention, and short-circuiting policy. Code review can't enforce consistency across 30 services. `bborbe/validation` provides one composable library with named fields, structured errors, and consistent message formatting — adopting it everywhere eliminates 40+ lines of boilerplate per Validate method and produces consistent error JSON downstream consumers can parse. + ## Basic Patterns ### Simple Field Validation diff --git a/docs/k8s-manifest-guide.md b/docs/k8s-manifest-guide.md index 21f0097..590f466 100644 --- a/docs/k8s-manifest-guide.md +++ b/docs/k8s-manifest-guide.md @@ -4,6 +4,20 @@ How to organize Kubernetes YAML manifests in a service repository. Rules only **Reference**: [`github.com/bborbe/go-skeleton/tree/master/k8s`](https://github.com/bborbe/go-skeleton/tree/master/k8s) +### RULE k8s-manifest/workload-kind-matches-semantics (MUST) + +**Owner**: go-architecture-assistant +**Applies when**: a Kubernetes manifest uses `kind: Deployment` for a workload with per-replica disk state, stable network identity needs, or ordered start/stop semantics — instead of `kind: StatefulSet`. Equivalent: using `Deployment` for a workload that runs to completion (should be `Job` / `CronJob`) or runs on every node (should be `DaemonSet`). +**Enforcement**: judgment (semantic — the workload's actual lifecycle semantics aren't visible in the YAML itself, requires reading the corresponding Go binary + understanding the data-persistence requirements) +**Why**: Wrong workload kind produces silent operational failures: a `Deployment` with a shared PVC corrupts data on replica scale-up; a `Job` deployed as `Deployment` restart-loops because the pod exits 0 and Kubernetes treats it as a crash; a `DaemonSet`-shaped workload deployed as `Deployment` misses every new node added to the cluster. Defaulting to `Deployment` because it's familiar is the most common error. Pick by lifecycle semantics, not by author muscle memory. + +### RULE k8s-manifest/statefulset-pvc-via-volumeclaimtemplates (MUST) + +**Owner**: go-architecture-assistant +**Applies when**: a `StatefulSet` mounts persistent storage via a standalone `PersistentVolumeClaim` + `volumes[].persistentVolumeClaim.claimName`, instead of `spec.volumeClaimTemplates` (which generates one PVC per replica automatically). +**Enforcement**: judgment (YAML inspection: `kind: StatefulSet` with `spec.template.spec.volumes` containing a `persistentVolumeClaim` — should be in `volumeClaimTemplates` instead) +**Why**: `volumeClaimTemplates` is the StatefulSet primitive for per-replica PVCs. Standalone PVCs are cluster-wide singletons — `mysvc-0`, `mysvc-1`, `mysvc-2` all mount the same volume and corrupt each other's writes. `volumeClaimTemplates` produces `datadir-mysvc-0`, `datadir-mysvc-1`, `datadir-mysvc-2` automatically, each its own PVC bound to one replica. Using the wrong shape works for replicas=1 and fails at scale-up; using the right shape works at any replica count. + ## 0. Choose the workload kind first Decide BEFORE writing any YAML. Wrong kind = wrong rolling-update semantics, wrong PVC binding, wrong restart behavior, silent failures. Defaulting to `Deployment` because it's familiar is the most common error. diff --git a/docs/markdown-todo-guide.md b/docs/markdown-todo-guide.md index 6999008..3325f09 100644 --- a/docs/markdown-todo-guide.md +++ b/docs/markdown-todo-guide.md @@ -6,6 +6,13 @@ Read, create, and update Markdown files using checkboxes to manage to-do items a --- +### RULE markdown-todo/lowercase-x-for-complete (MUST) + +**Owner**: agent-auditor +**Applies when**: a markdown checkbox uses `[X]` (uppercase) or any other variant (`[v]`, `[*]`, `[/]`) to mark a task complete instead of `[x]` (lowercase). +**Enforcement**: judgment (regex over `^- \[.\]` markdown bullets: only `[ ]` (incomplete) and `[x]` (complete) are recognised by GitHub-flavoured markdown renderers and most checkbox parsers) +**Why**: GitHub, Obsidian, and most markdown checkbox parsers strictly require lowercase `[x]` to render the checkbox as checked. `[X]` renders as a literal X-in-brackets in some renderers and as a checked checkbox in others — inconsistent across the team's tooling. Picking one form (lowercase, per GFM spec) keeps progress-tracking parsers reliable and visual rendering uniform. + ## 📝 1. Markdown Checkbox Format ### Basic Syntax diff --git a/docs/python-factory-pattern.md b/docs/python-factory-pattern.md index d385362..cf9d61d 100644 --- a/docs/python-factory-pattern.md +++ b/docs/python-factory-pattern.md @@ -2,6 +2,13 @@ Factory functions compose objects by wiring dependencies together. They contain **zero business logic** - only constructor calls. +### RULE python-factory/zero-business-logic-in-factories (MUST) + +**Owner**: python-architecture-assistant +**Applies when**: a Python factory function (in `factory.py` or named `create_*` / `make_*`) contains business logic — loops over domain data, conditional dispatch on runtime state, inline function implementations, validation logic, etc. — instead of pure dependency-wiring calls (constructor invocations and object-tree composition only). +**Enforcement**: judgment (ast-grep partial: `function_definition` in `factory.py` whose body contains `for_statement` / `if_statement` over non-config values, or inline `lambda` / nested `def`) +**Why**: Factories are the application's composition root — they answer "how do you build the object graph?". Mixing in business logic conflates two concerns: object lifecycle (a wiring concern) and domain decisions (a service concern). Tests of the business logic then need to construct the full factory state; refactors of the wiring propagate to business code. Keeping factories to pure constructor calls (and at most config-level conditionals like "if test_mode, use FakeDB else RealDB") makes them obvious to read, trivially testable as composition graphs, and a clean boundary between "what gets built" and "what it does." + ## 1. Core Principles **Factories should only:** diff --git a/docs/readme-guide.md b/docs/readme-guide.md index 442c7fb..92bab60 100644 --- a/docs/readme-guide.md +++ b/docs/readme-guide.md @@ -11,6 +11,13 @@ Guide for writing README.md files. README.md is for humans browsing GitHub — i | **Contains** | Features, install, usage, config, license | Build commands, architecture map, constraints | | **Never contains** | Architecture internals, workflow rules | Install instructions, feature marketing | +### RULE readme/user-facing-not-agent-context (MUST) + +**Owner**: agent-auditor +**Applies when**: a project's README.md includes agent-operational content (build commands, architecture internals, ban lists, version-alignment requirements, structured constraints) instead of user-facing content (description, install, quick-start, usage). +**Enforcement**: judgment (semantic — distinguishing "user needs this to use it" from "agent needs this to change it" requires reading the content; symptom: README has sections labelled "Constraints" / "Hard Rules" / "Architecture") +**Why**: README is the user's first impression of the project (rendered on the GitHub repo home page, on pkg.go.dev, in package manager listings). Agent-operational content drowns the install steps in noise and signals "this project is hard to use" to potential adopters. Move agent context to `CLAUDE.md`; keep README focused on "can a new user get this running in 5 minutes?". The README and CLAUDE.md serve different audiences — separating them sharpens both. + ## Project Types ### Public Projects (Libraries, Tools, CLIs) diff --git a/docs/tdd-guide.md b/docs/tdd-guide.md index 74127c1..2dc8306 100644 --- a/docs/tdd-guide.md +++ b/docs/tdd-guide.md @@ -14,6 +14,28 @@ This guide enables an AI agent to develop features using **Test-Driven Developme --- +### RULE tdd/failing-test-before-implementation (SHOULD) + +**Owner**: go-test-quality-assistant +**Applies when**: a PR introduces new functionality (new file, new function, new branch) without a corresponding test-file change that demonstrates the new behavior failing before the implementation lands. +**Enforcement**: judgment (commit-ordering inspection; ast-grep can flag new exported functions without adjacent test additions as a first-pass filter) +**Why**: Writing the failing test first forces the author to specify the behavior before implementing — pinning down inputs, outputs, edge cases, and error paths *as a contract* rather than as an afterthought. Test-after coding tends to test what the implementation already does instead of what the requirement actually was. SHOULD because the signal is weak (commit timing); pair-review confirms intent. + +#### Bad + +``` +commit 1: feat: add login endpoint [+ pkg/auth/login.go new] +commit 2: test: add tests for login [+ login_test.go — passes on first run] +``` + +#### Good + +``` +commit 1: test: failing test for login [RED — no implementation yet] +commit 2: feat: implement login [GREEN — minimal code to pass] +commit 3: refactor: extract validator [REFACTOR — keep tests green] +``` + ## ➕ Step 2: Add Test for New Functionality 1. **Understand the new feature or behavior.** diff --git a/docs/teamvault-conventions.md b/docs/teamvault-conventions.md index e90bbf5..e5a23f3 100644 --- a/docs/teamvault-conventions.md +++ b/docs/teamvault-conventions.md @@ -71,6 +71,13 @@ export WATCHER_GITHUB_PR_APP_ID=3798945 # NOT a teamvault key (numeric ID) export GIT_SSH_KEY=9qNBoq # teamvault key → SSH deploy key ``` +### RULE teamvault/short-alphanumeric-is-lookup-key-not-secret (MUST) + +**Owner**: go-security-specialist +**Applies when**: a code-review agent or security-lint pass flags a 6-12 char alphanumeric value in a `.env` file / k8s manifest / config file as an exposed credential, when the corresponding k8s manifest uses `teamvaultFileBase64` / `teamvaultPassword` / `teamvaultConfig` template functions to resolve the value. +**Enforcement**: judgment (semantic — distinguishing a lookup key from a real short token requires reading the k8s manifest the env var feeds into) +**Why**: Teamvault lookup keys are `[A-Za-z0-9]{6,12}` identifiers, NOT secrets. The real secret material lives in teamvault; the lookup key resolves to it at `kubectl apply` time. Flagging lookup keys as credentials produces noisy false positives, delays releases, and trains reviewers to ignore "this looks like an exposed secret" warnings — which is the opposite of what credential-leak detection should do. The trigger is "short alphanumeric AND value consumed by a teamvault function downstream"; real secrets are 100+ chars of PEM / base64 / hex. + ## Reviewer guidance **Do NOT flag as exposed credentials:** diff --git a/docs/test-pyramid-triggers.md b/docs/test-pyramid-triggers.md index 5d56f8b..81189b7 100644 --- a/docs/test-pyramid-triggers.md +++ b/docs/test-pyramid-triggers.md @@ -2,6 +2,13 @@ Concrete, action-oriented criteria for choosing the right test type when generating or implementing code. Language-neutral. Use this as the operational rule; for Go-specific patterns see [go-test-types-guide.md](go-test-types-guide.md), for theory see the team's [Test Pyramid](obsidian://open?vault=Personal&file=50%20Knowledge%20Base/Test%20Pyramid) note. +### RULE test-pyramid/push-down-when-unsure (MUST) + +**Owner**: go-test-quality-assistant +**Applies when**: a new test is written at a higher layer (E2E, integration) when an equivalent assertion could be made at a lower layer (integration → unit, E2E → integration). Specifically: the test exercises pure business logic via a full HTTP/DB round-trip, or it tests 5+ input combinations via E2E browser automation. +**Enforcement**: judgment (semantic — distinguishing "this needs the real boundary" from "I just defaulted to the higher layer" requires reading test intent) +**Why**: Each layer up the pyramid is roughly 10× slower and 10× more flaky. A 50ms unit test runs millions of times in CI lifetime; a 5s integration test runs hundreds of thousands; a 30s E2E test runs thousands. Defaulting up the pyramid quietly costs the team minutes-per-PR forever. Forcing the question "could this be one layer down?" before writing the higher-layer test surfaces the right level — and "yes, it can" beats "let's just E2E it" in 80%+ of cases. + ## Default: Push tests down the pyramid | Layer | Share of test count | Frequency | diff --git a/rules/index.json b/rules/index.json index e36be2a..c3a35d3 100644 --- a/rules/index.json +++ b/rules/index.json @@ -1,4 +1,13 @@ [ + { + "anchor": "adr/required-for-irreversible-architecture-decisions", + "applies_when": "a PR introduces an irreversible architectural change (database technology choice, message-bus selection, framework swap, major topology change like blue-green vs canary) without a corresponding ADR document under `docs/adr/NNNN-<title>.md` capturing the decision, alternatives considered, rationale, and consequences.", + "doc_path": "docs/adr-guide.md", + "enforcement": "judgment (semantic — distinguishing \"irreversible architectural decision\" from \"tactical implementation choice\" requires reading the change scope; symptom: PR introduces new top-level package or external dependency without ADR reference)", + "id": "adr/required-for-irreversible-architecture-decisions", + "level": "SHOULD", + "owner": "go-architecture-assistant" + }, { "anchor": "agent-cmd/agent-frontmatter", "applies_when": "any `agents/*.md` file is created.", @@ -62,6 +71,51 @@ "level": "SHOULD", "owner": "agent-auditor" }, + { + "anchor": "changelog/conventional-prefix-required", + "applies_when": "a bullet under `## Unreleased` in CHANGELOG.md does not start with one of the recognised conventional prefixes (`feat:`, `fix:`, `refactor:`, `test:`, `docs:`, `chore:`, `perf:`).", + "doc_path": "docs/changelog-guide.md", + "enforcement": "judgment (regex over `## Unreleased` bullets: `^- ([a-z]+:)` first token must be in the allowed prefix set)", + "id": "changelog/conventional-prefix-required", + "level": "MUST", + "owner": "agent-auditor" + }, + { + "anchor": "changelog/preamble-frozen", + "applies_when": "a CHANGELOG.md edit inserts content above the `# Changelog` title, modifies the SemVer preamble bullets (MAJOR/MINOR/PATCH), or places a `## Unreleased` / `## vX.Y.Z` section inside (rather than after) the preamble block.", + "doc_path": "docs/changelog-guide.md", + "enforcement": "judgment (markdown-structure inspection: every CHANGELOG.md must have the canonical preamble first, then sections in `## Unreleased` → `## vX.Y.Z` order)", + "id": "changelog/preamble-frozen", + "level": "MUST", + "owner": "agent-auditor" + }, + { + "anchor": "claude-md/agent-context-not-user-docs", + "applies_when": "a project's CLAUDE.md duplicates README.md user-facing content (install instructions, feature marketing, usage tutorials) instead of serving as terse agent-operational context (build commands, architecture map, constraints).", + "doc_path": "docs/claude-md-guide.md", + "enforcement": "judgment (semantic — distinguishing \"agent needs this to change the code\" from \"user needs this to use it\" requires reading the content)", + "id": "claude-md/agent-context-not-user-docs", + "level": "MUST", + "owner": "agent-auditor" + }, + { + "anchor": "git-workflow/never-direct-commit-to-master", + "applies_when": "a git commit lands directly on `master` / `main` without going through a feature branch + PR — typically caught by `pre-push` hook rejecting cross-name pushes to the default branch, or by GitHub's `master-protection` ruleset.", + "doc_path": "docs/git-workflow.md", + "enforcement": "judgment + tooling (`~/.git-hooks/pre-push` rejects feature-branch-to-master pushes; GitHub ruleset enforces required PR). Release commits (`release vX.Y.Z`) are the documented exception.", + "id": "git-workflow/never-direct-commit-to-master", + "level": "MUST", + "owner": "agent-auditor" + }, + { + "anchor": "git-workflow/no-ai-attribution-in-commits", + "applies_when": "a git commit message body or trailer contains \"Co-Authored-By: Claude\", \"Generated with Claude Code\", \"Co-Authored-By: GitHub Copilot\", or any other AI-attribution line.", + "doc_path": "docs/git-workflow.md", + "enforcement": "judgment (commit-message grep at PR-review time; can be a `commit-msg` hook reject)", + "id": "git-workflow/no-ai-attribution-in-commits", + "level": "MUST", + "owner": "agent-auditor" + }, { "anchor": "go-architecture/business-logic-not-in-main", "applies_when": "`main.go` (production code only — `main_test.go` is exempt) or `application.Run` contains domain operations (validation, business rules, data transformation, decision logic) instead of delegating to a service in `pkg/`.", @@ -116,6 +170,15 @@ "level": "SHOULD", "owner": "go-architecture-assistant" }, + { + "anchor": "go-boolean-combinator/result-with-description-not-naked-bool", + "applies_when": "a Go boolean-combinator / decision interface's method returns a bare `bool` instead of a `Result` type carrying both the boolean decision AND a human-readable `Description() string` explaining the reason.", + "doc_path": "docs/go-boolean-combinator-pattern.md", + "enforcement": "judgment (interface declaration check: decision-style interfaces (`Check`, `IsTrusted`, `Filtered`, `Allowed`, `Matches`) returning naked `bool`)", + "id": "go-boolean-combinator/result-with-description-not-naked-bool", + "level": "MUST", + "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\"`.", @@ -341,6 +404,15 @@ "level": "MUST", "owner": "go-factory-pattern-assistant" }, + { + "anchor": "go-filter/document-filtered-semantics", + "applies_when": "a Go filter / predicate interface uses ambiguous method names like `Match(item)` / `Check(item)` / `Apply(item)` without doc-comment clarifying whether `true` means \"include\" or \"exclude\" — OR uses contradictory naming (`Filtered` returning true for \"passes the filter\" instead of \"filtered out\").", + "doc_path": "docs/go-filter-pattern.md", + "enforcement": "judgment (interface declaration check: method-name + doc-comment alignment for predicate methods returning bool)", + "id": "go-filter/document-filtered-semantics", + "level": "MUST", + "owner": "go-architecture-assistant" + }, { "anchor": "go-functional-options/singular-option-type", "applies_when": "a Go package using the functional-options pattern uses suboptimal pair naming — function type with plural `XxxOptions` (matching the struct so they collide semantically), OR config struct with singular `XxxOption` (clashing with the function type). Both shapes work; this rule promotes the industry-standard singular-function / plural-struct pair for clarity, not as a correctness fix.", @@ -440,6 +512,24 @@ "level": "MUST", "owner": "go-http-handler-assistant" }, + { + "anchor": "go-k8s-binary/argument-struct-not-os-getenv", + "applies_when": "a Go k8s-deployed binary calls `os.Getenv(\"FOO\")` to read a config / auth value that's already declared as an `application` struct field bound via `argument` tags.", + "doc_path": "docs/go-k8s-binary-conventions.md", + "enforcement": "judgment (ast-grep partial: `call_expression` matching `os.Getenv(...)` outside `main.go` early bootstrap)", + "id": "go-k8s-binary/argument-struct-not-os-getenv", + "level": "MUST", + "owner": "go-quality-assistant" + }, + { + "anchor": "go-k8s-binary/secret-fields-need-display-length", + "applies_when": "an `application` struct field in a Go k8s-deployed binary holds secret material (PEM key, OAuth token, password, API key, JWT signing secret) without the `display:\"length\"` tag — meaning glog / structured-logging dumps of the application config will print the secret value.", + "doc_path": "docs/go-k8s-binary-conventions.md", + "enforcement": "judgment (ast-grep follow-up: `struct_field_declaration` with name matching `PEM*` / `*Token*` / `*Secret*` / `*Password*` / `*Key` outside `display:\"length\"` tag)", + "id": "go-k8s-binary/secret-fields-need-display-length", + "level": "MUST", + "owner": "go-security-specialist" + }, { "anchor": "go-k8s-crd/generated-client-not-dynamic", "applies_when": "a Go consumer service interacts with a first-party CRD (the CR schema is known at compile time, defined in the same repo or a sibling library) using `k8s.io/client-go/dynamic` instead of the typed clientset generated via `hack/update-codegen.sh`.", @@ -458,6 +548,15 @@ "level": "SHOULD", "owner": "go-architecture-assistant" }, + { + "anchor": "go-library/semver-vprefix-tag-required", + "applies_when": "a public Go library repo cuts a release without a `git tag` matching `v<MAJOR>.<MINOR>.<PATCH>` (e.g. `v1.0.0`, `v0.12.3`) — either tagless commits, date-based tags (`2026-06-03`), or non-prefixed semver (`1.0.0` without the leading `v`).", + "doc_path": "docs/go-library-guide.md", + "enforcement": "judgment (git-tag inspection: `git tag --list` filtered against `^v[0-9]+\\.[0-9]+\\.[0-9]+$`)", + "id": "go-library/semver-vprefix-tag-required", + "level": "MUST", + "owner": "go-quality-assistant" + }, { "anchor": "go-licensing/copyright-year-discipline", "applies_when": "a PR diff modifies copyright years in `*.go` source-file headers — either bulk-updating across many files or setting future / non-numeric years (`2099`, `present`, etc.).", @@ -548,6 +647,15 @@ "level": "MUST", "owner": "go-quality-assistant" }, + { + "anchor": "go-parse/paired-parse-and-parsedefault", + "applies_when": "a Go package adds a `ParseX(ctx, value)` function returning `(X, error)` without a paired `ParseXDefault(ctx, value, default) X` that suppresses the error and returns the default — OR vice versa (`ParseXDefault` without `ParseX`).", + "doc_path": "docs/go-parse-pattern.md", + "enforcement": "judgment (ast-grep follow-up: `function_declaration` named `Parse*` with `error` return type; pair-check against same-named `*Default` function in the same package)", + "id": "go-parse/paired-parse-and-parsedefault", + "level": "MUST", + "owner": "go-quality-assistant" + }, { "anchor": "go-patterns/bborbe-collection-ptr-not-helpers", "applies_when": "a Go file declares a custom pointer helper function (`func stringPtr(s string) *string { return &s }`, `func intPtr(...)`, etc.) instead of importing `github.com/bborbe/collection` and calling `collection.Ptr(...)`.", @@ -791,6 +899,51 @@ "level": "MUST", "owner": "go-time-assistant" }, + { + "anchor": "go-tools-versioning/no-tools-go-for-clis", + "applies_when": "a Go project has a `tools.go` file with `//go:build tools` that imports CLI tools (`_ \"github.com/golangci/golangci-lint/v2/cmd/golangci-lint\"`, `_ \"github.com/google/osv-scanner/v2/cmd/osv-scanner\"`, etc.) — instead of declaring CLI versions in a separate `tools.env` consumed by the Makefile via `go run pkg@$(VERSION)`.", + "doc_path": "docs/go-tools-versioning-guide.md", + "enforcement": "judgment (file-presence check: `tools.go` with `//go:build tools` header + CLI tool imports)", + "id": "go-tools-versioning/no-tools-go-for-clis", + "level": "MUST", + "owner": "go-quality-assistant" + }, + { + "anchor": "go-validation/use-bborbe-validation-not-inline-checks", + "applies_when": "a Go service implements input validation via hand-rolled `if`/`if-else` chains in a `Validate(ctx context.Context) error` method or HTTP handler, instead of using `github.com/bborbe/validation` (`validation.All{...}` + `validation.Name(...)` + `validation.NotEmptyString(...)` etc.).", + "doc_path": "docs/go-validation-framework-guide.md", + "enforcement": "judgment (ast-grep follow-up: `method_declaration` named `Validate` whose body contains 3+ inline `if v == \"\" || v == \"\"` chains with manual `errors.Errorf` calls, in a package not importing `github.com/bborbe/validation`)", + "id": "go-validation/use-bborbe-validation-not-inline-checks", + "level": "MUST", + "owner": "go-quality-assistant" + }, + { + "anchor": "k8s-manifest/statefulset-pvc-via-volumeclaimtemplates", + "applies_when": "a `StatefulSet` mounts persistent storage via a standalone `PersistentVolumeClaim` + `volumes[].persistentVolumeClaim.claimName`, instead of `spec.volumeClaimTemplates` (which generates one PVC per replica automatically).", + "doc_path": "docs/k8s-manifest-guide.md", + "enforcement": "judgment (YAML inspection: `kind: StatefulSet` with `spec.template.spec.volumes` containing a `persistentVolumeClaim` — should be in `volumeClaimTemplates` instead)", + "id": "k8s-manifest/statefulset-pvc-via-volumeclaimtemplates", + "level": "MUST", + "owner": "go-architecture-assistant" + }, + { + "anchor": "k8s-manifest/workload-kind-matches-semantics", + "applies_when": "a Kubernetes manifest uses `kind: Deployment` for a workload with per-replica disk state, stable network identity needs, or ordered start/stop semantics — instead of `kind: StatefulSet`. Equivalent: using `Deployment` for a workload that runs to completion (should be `Job` / `CronJob`) or runs on every node (should be `DaemonSet`).", + "doc_path": "docs/k8s-manifest-guide.md", + "enforcement": "judgment (semantic — the workload's actual lifecycle semantics aren't visible in the YAML itself, requires reading the corresponding Go binary + understanding the data-persistence requirements)", + "id": "k8s-manifest/workload-kind-matches-semantics", + "level": "MUST", + "owner": "go-architecture-assistant" + }, + { + "anchor": "markdown-todo/lowercase-x-for-complete", + "applies_when": "a markdown checkbox uses `[X]` (uppercase) or any other variant (`[v]`, `[*]`, `[/]`) to mark a task complete instead of `[x]` (lowercase).", + "doc_path": "docs/markdown-todo-guide.md", + "enforcement": "judgment (regex over `^- \\[.\\]` markdown bullets: only `[ ]` (incomplete) and `[x]` (complete) are recognised by GitHub-flavoured markdown renderers and most checkbox parsers)", + "id": "markdown-todo/lowercase-x-for-complete", + "level": "MUST", + "owner": "agent-auditor" + }, { "anchor": "python-architecture/constructor-injection-only", "applies_when": "a Python service class receives a dependency (logger, repository, API client, validator, etc.) through a *method parameter* instead of through `__init__`. Related but distinct anti-patterns covered by other rules: dependency from a global module variable (see `python-architecture/main-py-composition-root`); post-construction `self.foo = bar.foo` mutation (see `python-ioc/dependencies-as-private-fields`, which mandates private-field storage that makes mutation socially difficult).", @@ -809,6 +962,15 @@ "level": "SHOULD", "owner": "python-architecture-assistant" }, + { + "anchor": "python-factory/zero-business-logic-in-factories", + "applies_when": "a Python factory function (in `factory.py` or named `create_*` / `make_*`) contains business logic — loops over domain data, conditional dispatch on runtime state, inline function implementations, validation logic, etc. — instead of pure dependency-wiring calls (constructor invocations and object-tree composition only).", + "doc_path": "docs/python-factory-pattern.md", + "enforcement": "judgment (ast-grep partial: `function_definition` in `factory.py` whose body contains `for_statement` / `if_statement` over non-config values, or inline `lambda` / nested `def`)", + "id": "python-factory/zero-business-logic-in-factories", + "level": "MUST", + "owner": "python-architecture-assistant" + }, { "anchor": "python-ioc/dependencies-as-private-fields", "applies_when": "a Python service class stores an injected dependency on `self.<name>` (public attribute) instead of `self._<name>` (single-underscore private convention).", @@ -898,5 +1060,59 @@ "id": "python-pydantic/optional-needs-default", "level": "MUST", "owner": "python-quality-assistant" + }, + { + "anchor": "readme/user-facing-not-agent-context", + "applies_when": "a project's README.md includes agent-operational content (build commands, architecture internals, ban lists, version-alignment requirements, structured constraints) instead of user-facing content (description, install, quick-start, usage).", + "doc_path": "docs/readme-guide.md", + "enforcement": "judgment (semantic — distinguishing \"user needs this to use it\" from \"agent needs this to change it\" requires reading the content; symptom: README has sections labelled \"Constraints\" / \"Hard Rules\" / \"Architecture\")", + "id": "readme/user-facing-not-agent-context", + "level": "MUST", + "owner": "agent-auditor" + }, + { + "anchor": "skill-writing/scripts-in-scripts-subdir", + "applies_when": "a Claude Code skill places executable scripts (`*.sh`, `*.py`) directly alongside `SKILL.md` instead of in a `scripts/` subdirectory.", + "doc_path": "docs/claude-code-skill-writing-guide.md", + "enforcement": "judgment (file-layout check on `skills/<name>/` — only `SKILL.md` at top-level; scripts under `scripts/`)", + "id": "skill-writing/scripts-in-scripts-subdir", + "level": "MUST", + "owner": "skill-auditor" + }, + { + "anchor": "skill-writing/skill-md-frontmatter-required", + "applies_when": "a `skills/<name>/SKILL.md` file is missing the required frontmatter fields — `name:` (must match the directory name) and `description:` (Claude's discovery signal).", + "doc_path": "docs/claude-code-skill-writing-guide.md", + "enforcement": "judgment (YAML-frontmatter inspection: presence of `name` + `description` at the top of every SKILL.md)", + "id": "skill-writing/skill-md-frontmatter-required", + "level": "MUST", + "owner": "skill-auditor" + }, + { + "anchor": "tdd/failing-test-before-implementation", + "applies_when": "a PR introduces new functionality (new file, new function, new branch) without a corresponding test-file change that demonstrates the new behavior failing before the implementation lands.", + "doc_path": "docs/tdd-guide.md", + "enforcement": "judgment (commit-ordering inspection; ast-grep can flag new exported functions without adjacent test additions as a first-pass filter)", + "id": "tdd/failing-test-before-implementation", + "level": "SHOULD", + "owner": "go-test-quality-assistant" + }, + { + "anchor": "teamvault/short-alphanumeric-is-lookup-key-not-secret", + "applies_when": "a code-review agent or security-lint pass flags a 6-12 char alphanumeric value in a `.env` file / k8s manifest / config file as an exposed credential, when the corresponding k8s manifest uses `teamvaultFileBase64` / `teamvaultPassword` / `teamvaultConfig` template functions to resolve the value.", + "doc_path": "docs/teamvault-conventions.md", + "enforcement": "judgment (semantic — distinguishing a lookup key from a real short token requires reading the k8s manifest the env var feeds into)", + "id": "teamvault/short-alphanumeric-is-lookup-key-not-secret", + "level": "MUST", + "owner": "go-security-specialist" + }, + { + "anchor": "test-pyramid/push-down-when-unsure", + "applies_when": "a new test is written at a higher layer (E2E, integration) when an equivalent assertion could be made at a lower layer (integration → unit, E2E → integration). Specifically: the test exercises pure business logic via a full HTTP/DB round-trip, or it tests 5+ input combinations via E2E browser automation.", + "doc_path": "docs/test-pyramid-triggers.md", + "enforcement": "judgment (semantic — distinguishing \"this needs the real boundary\" from \"I just defaulted to the higher layer\" requires reading test intent)", + "id": "test-pyramid/push-down-when-unsure", + "level": "MUST", + "owner": "go-test-quality-assistant" } ]