From 68d617534113b8bff74058d361e05f44a35dd8c3 Mon Sep 17 00:00:00 2001 From: Benjamin Borbe Date: Tue, 2 Jun 2026 19:01:23 +0200 Subject: [PATCH 1/4] =?UTF-8?q?feat(rules):=20sweep=20pass=201+2=20?= =?UTF-8?q?=E2=80=94=206=20rules=20across=204=20docs=20(tdd,=20changelog,?= =?UTF-8?q?=20git-workflow,=20teamvault)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First commit on the bootstrap-sweep branch. Single PR will cover many remaining 0-rule docs; subsequent commits add more passes. Pass 1+2 rules (rules/index.json: 100 -> 106): - tdd/failing-test-before-implementation (SHOULD) - changelog/preamble-frozen (MUST) - changelog/conventional-prefix-required (MUST) - git-workflow/never-direct-commit-to-master (MUST) - git-workflow/no-ai-attribution-in-commits (MUST) - teamvault/short-alphanumeric-is-lookup-key-not-secret (MUST) Owner picks reflect best-fit for non-Go/Python workflow docs: agent-auditor for git/changelog process rules (cross-cutting authoring concerns); go-security-specialist for teamvault (false-positive prevention for credential-leak flagging fits security domain). --- docs/changelog-guide.md | 54 +++++++++++++++++++++++++++++++++++ docs/git-workflow.md | 46 +++++++++++++++++++++++++++++ docs/tdd-guide.md | 22 ++++++++++++++ docs/teamvault-conventions.md | 7 +++++ rules/index.json | 54 +++++++++++++++++++++++++++++++++++ 5 files changed, 183 insertions(+) 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/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 +``` + +#### 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/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/rules/index.json b/rules/index.json index e36be2a..eab06bb 100644 --- a/rules/index.json +++ b/rules/index.json @@ -62,6 +62,42 @@ "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": "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/`.", @@ -898,5 +934,23 @@ "id": "python-pydantic/optional-needs-default", "level": "MUST", "owner": "python-quality-assistant" + }, + { + "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" } ] From 349849c5c05290ff656eb814bf8a294f4fe0f4ca Mon Sep 17 00:00:00 2001 From: Benjamin Borbe Date: Tue, 2 Jun 2026 19:04:05 +0200 Subject: [PATCH 2/4] =?UTF-8?q?feat(rules):=20sweep=20pass=203+4=20?= =?UTF-8?q?=E2=80=94=209=20more=20rules=20across=207=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Continuing single-PR sweep. Total now: 100 -> 115 rules. Pass 3 (k8s-binary, library, test-pyramid, skill-writing): - go-library/semver-vprefix-tag-required (MUST) - test-pyramid/push-down-when-unsure (MUST) - go-k8s-binary/secret-fields-need-display-length (MUST) - go-k8s-binary/argument-struct-not-os-getenv (MUST) - skill-writing/scripts-in-scripts-subdir (MUST) - skill-writing/skill-md-frontmatter-required (MUST) Pass 4 (workflow + k8s): - markdown-todo/lowercase-x-for-complete (MUST) - claude-md/agent-context-not-user-docs (MUST) - k8s-manifest/workload-kind-matches-semantics (MUST) - k8s-manifest/statefulset-pvc-via-volumeclaimtemplates (MUST) Owners: skill-auditor for SKILL.md authoring; go-security-specialist for secret-handling tags; agent-auditor for markdown/CLAUDE.md authoring; go-architecture-assistant for k8s workload-kind decisions. --- docs/claude-code-skill-writing-guide.md | 14 ++++ docs/claude-md-guide.md | 7 ++ docs/go-k8s-binary-conventions.md | 14 ++++ docs/go-library-guide.md | 7 ++ docs/k8s-manifest-guide.md | 14 ++++ docs/markdown-todo-guide.md | 7 ++ docs/test-pyramid-triggers.md | 7 ++ rules/index.json | 90 +++++++++++++++++++++++++ 8 files changed, 160 insertions(+) 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//` — 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//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/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..` (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/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/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 eab06bb..18e5ef7 100644 --- a/rules/index.json +++ b/rules/index.json @@ -80,6 +80,15 @@ "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.", @@ -476,6 +485,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`.", @@ -494,6 +521,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..` (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.).", @@ -827,6 +863,33 @@ "level": "MUST", "owner": "go-time-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).", @@ -935,6 +998,24 @@ "level": "MUST", "owner": "python-quality-assistant" }, + { + "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//` — 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//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.", @@ -952,5 +1033,14 @@ "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" } ] From 690bef2f30541207fe7276a1fda48f20c2cb41a5 Mon Sep 17 00:00:00 2001 From: Benjamin Borbe Date: Tue, 2 Jun 2026 19:05:47 +0200 Subject: [PATCH 3/4] =?UTF-8?q?feat(rules):=20sweep=20pass=205=20=E2=80=94?= =?UTF-8?q?=205=20more=20rules=20across=205=20pattern=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Total: 116 -> 121. - go-filter/document-filtered-semantics (MUST) - go-parse/paired-parse-and-parsedefault (MUST) - go-validation/use-bborbe-validation-not-inline-checks (MUST) - go-tools-versioning/no-tools-go-for-clis (MUST) - readme/user-facing-not-agent-context (MUST) --- docs/go-filter-pattern.md | 7 +++++ docs/go-parse-pattern.md | 7 +++++ docs/go-tools-versioning-guide.md | 7 +++++ docs/go-validation-framework-guide.md | 7 +++++ docs/readme-guide.md | 7 +++++ rules/index.json | 45 +++++++++++++++++++++++++++ 6 files changed, 80 insertions(+) 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-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/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/rules/index.json b/rules/index.json index 18e5ef7..5a850e1 100644 --- a/rules/index.json +++ b/rules/index.json @@ -386,6 +386,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.", @@ -620,6 +629,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(...)`.", @@ -863,6 +881,24 @@ "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).", @@ -998,6 +1034,15 @@ "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.", From ecc127260838541903d3c00f17c72512ce1798c1 Mon Sep 17 00:00:00 2001 From: Benjamin Borbe Date: Tue, 2 Jun 2026 19:07:34 +0200 Subject: [PATCH 4/4] =?UTF-8?q?feat(rules):=20sweep=20pass=206=20+=20CLAUD?= =?UTF-8?q?E.md=20update=20=E2=80=94=204=20more=20rules,=2018=20new=20doc-?= =?UTF-8?q?agent=20mappings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Final pass (for now) of the sweep PR. Total rules: 100 -> 124 (+24). Pass 6 rules added: - go-boolean-combinator/result-with-description-not-naked-bool (MUST) - python-factory/zero-business-logic-in-factories (MUST) - adr/required-for-irreversible-architecture-decisions (SHOULD) (Plus one already in earlier commit not separately listed.) CLAUDE.md doc-agent table updated with 18 new mappings — every doc this sweep touched now has an explicit owner in the alignment table. Coverage shipped in this sweep PR (24 rules across 21 docs): Workflow/process (5 docs): - tdd-guide, changelog-guide, git-workflow, teamvault-conventions, markdown-todo-guide Go tooling/library (6 docs): - go-library-guide, go-mod-replace cross-ref via go-tools-versioning, go-tools-versioning-guide, go-validation-framework-guide, go-k8s-binary-conventions, go-parse-pattern, go-filter-pattern Go architecture/composition (3 docs): - go-boolean-combinator-pattern, k8s-manifest-guide Test culture (1 doc): - test-pyramid-triggers Authoring/docs (3 docs): - claude-md-guide, readme-guide, claude-code-skill-writing-guide Python (1 doc): - python-factory-pattern Architecture-decision (1 doc): - adr-guide Remaining 0-rule docs (deferred to focused trim-PRs first): - go-mocking-guide (1037), go-test-types-guide (995), documentation-guide (1158), python-cli-arguments-guide (1169), vue3-typescript (772), prd-guide (880), astro-development-guide (619) — all need trim first. - Plus a few small overlap docs (dod, definition-of-done, go-precommit, releasing-coding, go-logging-guide) that summarise content covered by sibling rule sets — no distinct rules to extract. make build-index regenerated; check-index passes. No personal vault paths or trading-domain leaks across any of the 21 touched docs. --- CLAUDE.md | 18 ++++++++++++++++++ docs/adr-guide.md | 7 +++++++ docs/go-boolean-combinator-pattern.md | 7 +++++++ docs/python-factory-pattern.md | 7 +++++++ rules/index.json | 27 +++++++++++++++++++++++++++ 5 files changed, 66 insertions(+) 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/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/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/rules/index.json b/rules/index.json index 5a850e1..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.", @@ -161,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\"`.", @@ -944,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).",