add git quality standards section: interlocking checks, linter policy, PR pipeline
All checks were successful
check / check (push) Successful in 11s
All checks were successful
check / check (push) Successful in 11s
This commit is contained in:
parent
3858a80846
commit
51677e3adb
@ -459,6 +459,230 @@ permanently private and serves as a backup/DR system, not a development repo. If
|
|||||||
your workspace state repo were ever to become public, it would be a catastrophic
|
your workspace state repo were ever to become public, it would be a catastrophic
|
||||||
leak. Treat it accordingly: private visibility, restricted access, no forks.
|
leak. Treat it accordingly: private visibility, restricted access, no forks.
|
||||||
|
|
||||||
|
### Git Quality Standards — Interlocking Automated Checks
|
||||||
|
|
||||||
|
An AI agent will forget things. It will skip the formatter, push without
|
||||||
|
testing, weaken a linter rule to make CI pass, or use `git add .` and commit
|
||||||
|
junk files. You cannot rely on "be careful" — you need automated gates that make
|
||||||
|
it structurally impossible to ship bad code.
|
||||||
|
|
||||||
|
The approach we use is based on
|
||||||
|
[sneak/prompts](https://git.eeqj.de/sneak/prompts), a repo of standardized
|
||||||
|
policies that every project copies. The key document is
|
||||||
|
[REPO_POLICIES.md](https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/REPO_POLICIES.md),
|
||||||
|
which defines the interlocking check system.
|
||||||
|
|
||||||
|
#### The Interlocking Chain
|
||||||
|
|
||||||
|
The checks form a dependency chain where each layer requires the previous:
|
||||||
|
|
||||||
|
```
|
||||||
|
Gitea Actions CI
|
||||||
|
└── docker build .
|
||||||
|
└── make check (inside Dockerfile)
|
||||||
|
├── make fmt-check (code formatting)
|
||||||
|
├── make lint (static analysis)
|
||||||
|
└── make test (unit/integration tests)
|
||||||
|
```
|
||||||
|
|
||||||
|
This means:
|
||||||
|
|
||||||
|
- **CI runs `docker build .`** — that's it, one command
|
||||||
|
- **The Dockerfile runs `make check`** as a build step — if checks fail, the
|
||||||
|
Docker build fails, CI fails
|
||||||
|
- **`make check` depends on `fmt-check`, `lint`, and `test`** — all three must
|
||||||
|
pass
|
||||||
|
- **You can't skip any layer** — they're structurally linked
|
||||||
|
|
||||||
|
From
|
||||||
|
[REPO_POLICIES.md](https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/REPO_POLICIES.md):
|
||||||
|
|
||||||
|
> Every repo with software must have a root `Makefile` with these targets:
|
||||||
|
> `make test`, `make lint`, `make fmt` (writes), `make fmt-check` (read-only),
|
||||||
|
> `make check` (prereqs: `test`, `lint`, `fmt-check`), `make docker`, and
|
||||||
|
> `make hooks` (installs pre-commit hook).
|
||||||
|
|
||||||
|
> Every repo should have a `Dockerfile`. All Dockerfiles must run `make check`
|
||||||
|
> as a build step so the build fails if the branch is not green.
|
||||||
|
|
||||||
|
> Every repo should have a Gitea Actions workflow (`.gitea/workflows/`) that
|
||||||
|
> runs `docker build .` on push. Since the Dockerfile already runs `make check`,
|
||||||
|
> a successful build implies all checks pass.
|
||||||
|
|
||||||
|
#### Why Docker as the CI Runner
|
||||||
|
|
||||||
|
Running `make check` inside `docker build` solves the "works on my machine"
|
||||||
|
problem:
|
||||||
|
|
||||||
|
- **Clean environment every time.** No stale caches, no leftover files, no wrong
|
||||||
|
toolchain version
|
||||||
|
- **Reproducible.** The Dockerfile pins the base image by SHA (not tag), so the
|
||||||
|
build environment is identical everywhere
|
||||||
|
- **No CI configuration drift.** The CI workflow is one line: `docker build .`.
|
||||||
|
All the actual logic lives in the Dockerfile and Makefile, which are
|
||||||
|
version-controlled in the repo
|
||||||
|
|
||||||
|
From REPO_POLICIES.md:
|
||||||
|
|
||||||
|
> ALL external references must be pinned by cryptographic hash. This includes
|
||||||
|
> Docker base images, Go modules, npm packages, GitHub Actions, and anything
|
||||||
|
> else fetched from a remote source. Version tags (`@v4`, `@latest`, `:3.21`,
|
||||||
|
> etc.) are server-mutable and therefore remote code execution vulnerabilities.
|
||||||
|
|
||||||
|
#### The Makefile as Authoritative Documentation
|
||||||
|
|
||||||
|
The Makefile isn't just a build tool — it's the single source of truth for how
|
||||||
|
the repo works:
|
||||||
|
|
||||||
|
> The Makefile is authoritative documentation for how the repo is used. Beyond
|
||||||
|
> the required targets above, it should have targets for every common operation:
|
||||||
|
> running a local development server (`make run`, `make dev`), re-initializing
|
||||||
|
> or migrating the database (`make db-reset`, `make migrate`), building
|
||||||
|
> artifacts (`make build`), generating code, seeding data, or anything else a
|
||||||
|
> developer would do regularly. If someone checks out the repo and types
|
||||||
|
> `make<tab>`, they should see every meaningful operation available.
|
||||||
|
|
||||||
|
This matters for AI agents because:
|
||||||
|
|
||||||
|
- The agent always uses `make test`, never `go test ./...` directly
|
||||||
|
- If the test command needs flags, timeouts, or environment setup, it's in the
|
||||||
|
Makefile — the agent doesn't need to know the details
|
||||||
|
- A new sub-agent spawned on any repo can immediately see every available
|
||||||
|
operation
|
||||||
|
|
||||||
|
#### Pre-Commit Hooks
|
||||||
|
|
||||||
|
Local enforcement before code even reaches the remote:
|
||||||
|
|
||||||
|
> Pre-commit hook: `make check` if local testing is possible, otherwise
|
||||||
|
> `make lint && make fmt-check`. The Makefile should provide a `make hooks`
|
||||||
|
> target to install the pre-commit hook.
|
||||||
|
|
||||||
|
Our PR checklist requires agents to install hooks after every clone:
|
||||||
|
|
||||||
|
```sh
|
||||||
|
echo '#!/bin/sh\nmake check' > .git/hooks/pre-commit && chmod +x .git/hooks/pre-commit
|
||||||
|
```
|
||||||
|
|
||||||
|
#### Linter Config Is Sacred
|
||||||
|
|
||||||
|
One of the most dangerous failure modes (documented in the failures section
|
||||||
|
above) is an agent modifying linter config to make checks pass:
|
||||||
|
|
||||||
|
> `.golangci.yml` is standardized and must _NEVER_ be modified by an agent, only
|
||||||
|
> manually by the user.
|
||||||
|
|
||||||
|
This is enforced in our PR checklist:
|
||||||
|
|
||||||
|
```markdown
|
||||||
|
## After sub-agent pushes code
|
||||||
|
|
||||||
|
1. Check diff for .golangci.yml / linter / test config changes
|
||||||
|
2. Check diff for `-short` / `-timeout` flags added to test commands
|
||||||
|
3. If any config files changed: reject and rework
|
||||||
|
```
|
||||||
|
|
||||||
|
The principle: **if the check fails, fix the code, not the check.** This applies
|
||||||
|
universally — linter rules, test assertions, formatting config, CI workflows.
|
||||||
|
Weakening a gate to make it pass is worse than a loud failure, because loud
|
||||||
|
failures get fixed while silent rot compounds.
|
||||||
|
|
||||||
|
#### Formatting Standards
|
||||||
|
|
||||||
|
From
|
||||||
|
[REPO_POLICIES.md](https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/REPO_POLICIES.md):
|
||||||
|
|
||||||
|
> Use platform-standard formatters: `black` for Python, `prettier` for
|
||||||
|
> JS/CSS/Markdown/HTML, `go fmt` for Go. Always use default configuration with
|
||||||
|
> two exceptions: four-space indents (except Go), and `proseWrap: always` for
|
||||||
|
> Markdown (hard-wrap at 80 columns).
|
||||||
|
|
||||||
|
This means formatting is never a judgment call — run the formatter, done. The
|
||||||
|
`make fmt` target writes changes, `make fmt-check` verifies without modifying
|
||||||
|
(used in CI). Agents run `make fmt` before committing, CI runs `make fmt-check`
|
||||||
|
to catch anything that slipped through.
|
||||||
|
|
||||||
|
#### Test Requirements
|
||||||
|
|
||||||
|
> All repos with software must have tests that run via the platform-standard
|
||||||
|
> test framework (`go test`, `pytest`, `jest`/`vitest`, etc.). If no meaningful
|
||||||
|
> tests exist yet, add the most minimal test possible — e.g. importing the
|
||||||
|
> module under test to verify it compiles/parses. There is no excuse for
|
||||||
|
> `make test` to be a no-op.
|
||||||
|
|
||||||
|
> `make test` must complete in under 20 seconds. Add a 30-second timeout in the
|
||||||
|
> Makefile.
|
||||||
|
|
||||||
|
The timeout is critical for agents — without it, a hanging test blocks the
|
||||||
|
entire sub-agent session. If tests take too long, that's a bug to file, not a
|
||||||
|
timeout to increase.
|
||||||
|
|
||||||
|
#### Git Hygiene Rules
|
||||||
|
|
||||||
|
From REPO_POLICIES.md and our operational experience:
|
||||||
|
|
||||||
|
- **Never `git add -A` or `git add .`** — always stage files explicitly. Agents
|
||||||
|
love to `git add .` and accidentally commit `.DS_Store`, editor swap files, or
|
||||||
|
debug output
|
||||||
|
- **Never force-push to main** — feature branches only
|
||||||
|
- **Each change = separate commit** — formatting changes go in their own commit
|
||||||
|
before logic changes
|
||||||
|
- **Rebase before and after** — PRs must be mergeable at time of push
|
||||||
|
- **Never commit secrets** — `.env`, credentials, API keys in `.gitignore`
|
||||||
|
|
||||||
|
#### The PR Pipeline
|
||||||
|
|
||||||
|
Our agent follows a strict PR lifecycle:
|
||||||
|
|
||||||
|
```markdown
|
||||||
|
## PR pipeline (every PR, no exceptions)
|
||||||
|
|
||||||
|
1. **Review/rework loop**: code review → rework → re-review → repeat until clean
|
||||||
|
2. **Check/rework loop**: `make check` + `docker build .` → rework → re-check →
|
||||||
|
repeat until clean
|
||||||
|
3. Only after BOTH loops pass with zero issues: assign to human
|
||||||
|
|
||||||
|
- "Passes checks" ≠ "ready for human"
|
||||||
|
- Never weaken tests/linters. Fix the code.
|
||||||
|
- Pre-existing failures are YOUR problem. Fix them as part of your PR.
|
||||||
|
```
|
||||||
|
|
||||||
|
The agent doesn't just create a PR and hand it off — it drives the PR through
|
||||||
|
review, rework, and verification until it's genuinely ready. A PR assigned to
|
||||||
|
the human means: all checks pass, code reviewed, review feedback addressed,
|
||||||
|
rebased against main, no conflicts. Anything less is the agent's open task.
|
||||||
|
|
||||||
|
#### New Repo Bootstrap
|
||||||
|
|
||||||
|
Every new repo follows a checklist from REPO_POLICIES.md:
|
||||||
|
|
||||||
|
> New repos must contain at minimum: `README.md`, `.git`, `.gitignore`,
|
||||||
|
> `.editorconfig`, `LICENSE`, `REPO_POLICIES.md`, `Makefile`, `Dockerfile`,
|
||||||
|
> `.dockerignore`, `.gitea/workflows/check.yml`
|
||||||
|
|
||||||
|
Plus language-specific files (Go: `go.mod`, `.golangci.yml`; JS: `package.json`,
|
||||||
|
`.prettierrc`; Python: `pyproject.toml`).
|
||||||
|
|
||||||
|
The full standardized configs are available at
|
||||||
|
`https://git.eeqj.de/sneak/prompts/raw/branch/main/<filename>` — agents fetch
|
||||||
|
them when bootstrapping a new repo, ensuring consistency across all projects.
|
||||||
|
|
||||||
|
#### Why This Matters for AI Agents Specifically
|
||||||
|
|
||||||
|
AI agents have a unique failure mode: they're confidently wrong. An agent will
|
||||||
|
push code that "should work," assert that checks pass without running them, or
|
||||||
|
silently weaken a gate to make the build green. Automated interlocking checks
|
||||||
|
turn these soft failures into hard failures:
|
||||||
|
|
||||||
|
- Can't push unformatted code → `make fmt-check` in pre-commit hook
|
||||||
|
- Can't skip tests → `make check` depends on `make test`
|
||||||
|
- Can't weaken linters → config file changes flagged in PR review
|
||||||
|
- Can't claim "CI passes" without proof → Docker build is pass/fail
|
||||||
|
- Can't ship without the human seeing it → PR assignment rules
|
||||||
|
|
||||||
|
The agent doesn't need willpower or attention to detail. It needs a system where
|
||||||
|
doing the wrong thing fails loudly.
|
||||||
|
|
||||||
### Putting It All Together
|
### Putting It All Together
|
||||||
|
|
||||||
The system works as a loop:
|
The system works as a loop:
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user