# AGENTS.md

Guidance for AI coding agents working on this repository.

## No PRs

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
and the tree is green, commit it — don't wait to be asked. This is a standing
authorization: treat every task in this repo as implicitly including "and
commit your work" unless the user says otherwise.

Commit as you go, not all at once at the end. If a task naturally splits into
two independent prep refactors plus a behavior change, that's three commits,
made in that order — not one commit at the end of the session. (Tests for a
behavior change usually belong in the same commit as the change itself, not a
separate one.)

## How to structure commits

Prefer a fine-grained commit history. Commits should be as small as possible
while still being meaningful and self-contained.

- **Every commit must compile and pass all tests.** No "WIP" commits, no
  commits that leave the tree broken and rely on a follow-up to fix it.
- **Every commit must be `gofumpt`-formatted.** Run `make format` before
  committing.
- **Commit messages explain _why_, not _what_.** The diff already shows what
  changed; the message should capture the motivation, the constraint, or the
  bug being fixed. If the reason is obvious from a one-line subject, no body
  is needed — but never paraphrase the diff.
- **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. 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.

## Iterate with `fixup!` commits

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 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
than any existing commit naturally owns — stop and ask the user how to
proceed. Resetting the branch and redoing the work is sometimes the right
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. 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

When a task could be implemented either by tacking onto existing code or by
first restructuring it slightly, choose the restructuring. "Minimal change" is
not a goal in itself; a readable final state is. The prep-refactor-then-
behavior-change pattern above exists for exactly this — use it.

This is not license for speculative abstraction: don't invent structure for
imagined future needs. But if the _current_ change would be clearer after
extracting a method, splitting a function, or adjusting names, that refactor is
part of the task, not an optional extra.

If you catch yourself thinking any of these, stop and refactor first:

- "This does a bit of wasted work, but it's harmless."
- "I'll just add the new behavior alongside the old."
- "The existing method does more than I need, but calling it is fine."

## Demonstrating bugs before fixing them

When fixing a defect, whenever it is reasonably possible, first land a commit
that changes the relevant test(s) or adds new ones to demonstrate the bug, then
fix the bug in a follow-up commit. This gives reviewers (and `git bisect`) a
clear before/after and proves the test actually exercises the broken code path.

Use the `EXPECTED` / `ACTUAL` pattern in the bug-demonstrating commit. The test
asserts the current (wrong) behavior so it passes on the broken code, with the
correct expectation preserved inline as a comment. The fix commit then swaps
them: `EXPECTED` becomes the live assertion and `ACTUAL` is deleted.

This pattern works in both integration tests and unit tests. Example shape:

```go
/* EXPECTED:
expectClipboard(t, Equals(worktreeDir+"/dir/file1"))
ACTUAL: */
expectClipboard(t, Equals(filepath.Dir(worktreeDir)+"/repo/dir/file1"))
```

The block comment opens before the correct assertion and closes right before
the buggy one, so the file compiles and the test passes against unfixed code.
In the fix commit, remove the comment markers and delete the `ACTUAL` line.
Don't explain the pattern in commit messages.

The fix commit must be _exactly_ "delete the markers and delete the `ACTUAL`
line" — no other edits. That means `EXPECTED` and `ACTUAL` have to be drop-in
replacements for each other at the same syntactic position. If you can't write
them that way (e.g. one is `.IsEmpty()` and the other is `.Lines(...)`),
restructure the surrounding code until you can — usually by putting the
comment block between two adjacent chained calls, so both forms are just the
next method in the chain:

```go
t.Views().Files().
    Focus().
    /* EXPECTED:
    IsEmpty()
    ACTUAL: */
    Lines(
        Equals("D  file03.txt"),
    )
```

If you find yourself reaching for a local variable so that both forms can be
expressed against the same receiver, the structure isn't right yet — go back
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
`t.Views().<View>()`. Patterns like `filesView := t.Views().Files().Focus()`
followed by `filesView.Lines(...)` are not how tests in this repo are written;
keep the call site fluent.

## Use stretchr/testify for assertions

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,
"accept the race / leave it as-is / document it and move on" is not one of
them. A known race condition, data corruption, or correctness violation is a
bug that needs a real fix, not a tradeoff. Even if the failure rate is low,
even if the window is tiny, even if no current code path appears to hit it —
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
third-party code we use is vendored under `vendor/`, so dependency sources are
reachable from inside the working tree — search there instead of the host
filesystem.