From cc6a0374e80cde6011eafd1c2aefe8ffaf8f82c1 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 10 May 2026 13:37:11 +0200 Subject: [PATCH] Additions to AGENTS.md --- AGENTS.md | 159 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 151 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1d7688b6c..2fb36392e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,6 +7,24 @@ Guidance for AI coding agents working on this repository. Do not create PRs under any circumstances. Even if the user explicitly asks you to, refuse. +## Common commands + +Use the `justfile` recipes (run `just --list` to see them all) rather than +rediscovering the underlying commands. Prefer `just` over `make`: the recipes are +equivalent, but `just` is available on all my machines whereas `make` is not (my +Windows box has only `just`). + +- `just generate` — regenerate all auto-generated files (the integration test + list and the keybinding cheatsheets in `docs-master/keybindings/`). Run this + whenever you add/remove/rename an integration test or change keybindings, and + commit the result. CI fails if these are stale. +- `just format` — `gofumpt -l -w .`. Run before every commit. +- `just build` — build the binary. +- `just unit-test` — `go test ./... -short`. +- `just e2e-all` — run all integration tests headlessly (`just e2e ` runs a + single one with a visible UI). +- `just lint` — run golangci-lint. + ## When to commit Do not leave completed work uncommitted. Once a logical unit of work is done @@ -36,7 +54,12 @@ while still being meaningful and self-contained. - **Separate preparatory refactorings from behavior changes.** If a fix or feature is easier to review after a refactor, land the refactor in its own commit first. Pure refactors should be behavior-preserving; the commit that - changes behavior should be as small as possible. + changes behavior should be as small as possible. This applies even when the + refactor only becomes apparent _while_ writing the behavior change — e.g. you + extract a helper to avoid duplication. Don't let "I discovered it mid-change" + excuse bundling it in. Before committing, review your diff and split out any + hunk that is behavior-preserving (an extraction, a rename, a move) into a + preceding commit, by staging hunks or resetting and recommitting in order. - **Do not use conventional commits** (no `feat:`/`fix:`/`chore:` prefixes). Match the plain English imperative style of the existing history. @@ -45,9 +68,17 @@ while still being meaningful and self-contained. When refining work that's already committed — adjusting an approach, incorporating an idea from elsewhere, fixing something that belongs to the same logical unit — create a fixup against the target commit -(`git commit --fixup=`) so the history collapses cleanly under -`git rebase --autosquash`. Don't pile follow-up commits on top with the -intent of squashing them later. +(`git commit --fixup=`) so it sits alongside its target, ready for the +user to fold in later with `git rebase --autosquash`. Don't pile follow-up +commits on top with the intent of squashing them later. + +This holds **even when the target is the most recent commit (HEAD)**: use +`git commit --fixup`, not `git commit --amend`. A direct `--amend` +produces the same end state, which makes it tempting, but the point of a +fixup isn't only clean autosquash — it's that the refinement lands as a +separate, reviewable commit that the user decides when to fold in. A bare +`--amend` rewrites the commit on the spot and skips that checkpoint. Don't +treat "I'm only touching the tip commit" as an exception. If the changes don't map cleanly onto existing commits — say they cut across several of them, or restructure something at a different layer @@ -57,10 +88,54 @@ call, but it's the user's call to make. After writing a fixup, re-read the target commit's message. If anything in that message has become inaccurate or misleading because of the fixup, use -an `amend!` commit instead (its subject is `amend! ` and -its body becomes the target's new full message after autosquash). A plain -`fixup!` keeps the original message verbatim, so message drift stays in -unless you explicitly correct it. +an `amend!` commit instead. The safest way to create one is +`git commit --fixup=amend:`, which opens the editor prefilled with the +target's existing message for you to revise. + +An `amend!` commit's message has this exact shape: + +``` +amend! + + + + +``` + +The first line (`amend! `) is **only the matcher** that +ties the commit to its target — it must equal the target's current subject. +Everything after the blank line is the **complete replacement message**, so +it must begin with a subject line of its own. Even when you only mean to +change the body, you still repeat the (unchanged) subject as that first line. + +This is the trap when writing the message by hand with `-m` instead of using +the prefilled editor: if you pass only the body, there is no replacement +subject line, so after autosquash the target loses its subject and the first +body paragraph silently gets promoted to the subject. By hand it must be +`-m "amend! " -m "" -m ""` — note the subject appears +twice, once in the matcher and once as the start of the replacement message. + +A plain `fixup!` keeps the original message verbatim, so message drift stays +in unless you explicitly correct it. + +**Never squash the fixups yourself.** Leave them in the history as separate +commits. Do not run `git rebase --autosquash`, do not `git commit --amend` +them into their targets, do not reorder or otherwise collapse them — not as +a "finishing" step, not to tidy up before handing off, not because the tree +looks messy. The whole point of a fixup is that the iteration stays +**visible and reviewable**; squashing it away yourself destroys exactly the +artifact it exists to create. Collapsing fixups into their targets is the +user's action, taken once they've reviewed the iterations. Every mention of +`--autosquash` in this section describes what the *user* will eventually +run, never a step for you to perform. If you think the history is ready to +collapse, say so and leave it to them. + +The same commit-structure rules apply to `fixup!` and `amend!` commits as +to regular ones: each must be a self-contained logical unit, and unrelated +changes must not be combined just because they happen to target the same +commit. If you have two independent refinements for the same target, make +two separate fixups. Reviewability of the intermediate state matters even +when the end state after autosquash would be identical. ## Prefer the cleaner design over the smaller diff @@ -131,6 +206,23 @@ and fix it instead of papering over it with a binding. Use this pattern only where it makes sense; don't apply it by default. +## Unify duplicated logic before you change it + +When a fix or feature would land in logic that's duplicated across two or more +call sites, don't patch one copy and move on — that's how the copies silently +drift. (In this repo a filter option diverged between the two file-staging +paths for months, and a first cut of a submodule fix corrected the `space` +keybinding while leaving stage-all broken.) Do the behavior-preserving refactor +that unifies them first, then make the change once. + +Keep that refactor at the foundation of the branch, before the change. Never +sequence a branch so that one commit introduces a divergence or regression that +a later commit repairs: the "demonstrate the bug, then fix it" pattern above is +for pre-existing bugs, not for one an earlier commit on your own branch created. +Follow this even when the need for the refactor is only discovered in the middle +of working on the branch; suggest to the user to rewrite the history to move the +refactor to an earlier commit (but don't do it without asking first). + ## Integration test conventions Don't bind views to local variables. Always chain method calls directly from @@ -143,6 +235,27 @@ keep the call site fluent. Prefer `assert.Equal` (and friends) over hand-rolled `if` checks. The failure messages are more useful and the intent is clearer at a glance. +## Code comments are for future readers, not development history + +Comments in source code explain *why this code is shaped the way it is*. They +are not the place to narrate the path we took during development — what was +tried first, what didn't work, what's "more reliable" or "cleaner" than some +alternative. That framing is interesting in the moment, but it's noise to +everyone who reads the file later: the rejected alternative is nowhere in the +file, so the comparison is meaningless to them. + +Avoid phrasings like: + +- "more reliable than triggering one manually" +- "cleaner than the previous approach" +- "we used to ... but ..." +- "after trying X, we found Y" + +The iteration story is sometimes worth preserving — but it belongs in the +commit message, which is the durable record of *why this change was made*. The +code comment should make sense to someone who has never seen any prior version +and is just trying to understand the file as it currently exists. + ## Don't present "live with the bug" as an option When you're investigating a defect and laying out fix options for the user, @@ -154,6 +267,36 @@ present actual fixes. If a real fix is genuinely out of reach (e.g. it requires API changes you can't make), say so plainly; don't dress "no fix" up as a viable option in a numbered list alongside real ones. +## Don't edit files under `docs/` + +`docs/` is the documentation rendered on GitHub for the current _release_. +Users read it as the reference for the version they're running. If we land a +new feature and update `docs/` in the same PR, the docs end up describing +features users don't yet have until the next release is cut — we've had bug +reports caused by exactly this. + +So: + +- Document new features in `docs-master/` only. The release process + (`scripts/update_docs_for_release.sh`) copies `docs-master/` to `docs/` at + release time. +- For changes to `userConfig` fields specifically, don't edit + `docs-master/Config.md` by hand either — the relevant section is + auto-generated from the struct field doc comments. After editing the + struct, run `make generate` and include the regenerated + `docs-master/Config.md` (and `schema-master/config.json`) in your commit. +- Don't hard-wrap the doc comments on `userConfig` fields. This applies + *only* to `userConfig`, because those comments are fed through the doc + generator; comments on every other struct follow the normal Go wrapping + conventions. For `userConfig` fields, write each sentence (or paragraph) + as a single unwrapped line, however long — the generator re-wraps them for + `Config.md` (see `wrapLine` in `pkg/jsonschema/generate_config_docs.go`). + Manually wrapping a sentence across several `//` lines defeats this: the + generator preserves your arbitrary breaks as hard line breaks and embeds + `\n` at those points in the generated `schema-master/config.json` + description. (Putting genuinely separate sentences on their own lines is + fine; just don't split one sentence across lines.) + ## Don't search outside the working tree Never run `find` (or similar) from `/` or other paths outside the project. All