Additions to AGENTS.md (#5667)

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 <name>` 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=<sha>`) 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=<sha>`) 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! <original subject>` 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:<sha>`, 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! <original subject>
+
+<new subject>
+
+<new body>
+```
+
+The first line (`amend! <original subject>`) 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! <subject>" -m "<subject>" -m "<body>"` — 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