Commit Graph

2 Commits

Author SHA1 Message Date
Levi Neuwirth ffe5ae2d8d
fix(git): reserve the refresh generation at the command, not on arrival
PR #227 review, P1. Two `git.status` invocations against different
repositories were ordered by which `rev-parse` returned first, not by
which the user asked for last.

`git.status` started an UNVERSIONED `rev-parse`, and the generation was
minted later, inside `start_status`, which runs from that lookup's
completion callback. So invoke status in repo A, then repo B: if B's
root resolved first and A's resolved second, A claimed the NEWER
generation and replaced B. The counter that exists to make the newest
INVOCATION win instead made the slowest SUBPROCESS win --- and it did it
silently, since both requests looked well-formed.

The fix is the ordering, not a new check:

* `reserve_generation()` is called at the point the user ASKS ---
  `git.status` after its early returns and before the spawn, `g` at the
  keypress. An invocation that starts no work reserves nothing, so it
  cannot invalidate one already in flight.
* `start_status` takes the reserved generation as a PARAMETER instead of
  minting its own, so the value survives the round trip through the root
  lookup.
* The root-lookup completion is now `pmacs.git._deliver_root`, and it
  drops a superseded result before any effect: no status spawn, no
  `state.root` write, and no status-line message. A message from an
  invocation the user has already replaced is as wrong as a panel from
  one, and the previous shape would have written both.

Exposed for the same reason `_deliver_status` is exposed: the contract
is about completions arriving in an order the caller did not choose, and
no arrangement of real subprocess timing can guarantee two `rev-parse`
runs finish in a chosen order.

Witness: `g6_21_a_superseded_root_lookup_does_not_spawn_its_status`
drives two real invocations, then completes their ROOT LOOKUPS out of
order --- newer first, older second --- and asserts the superseded one
spawns nothing at all, comparing the status-spawn count before and
after. `g6_17` cannot see this: it drives the STATUS completions out of
order, by which point the generation each carries is already fixed.

Bite: restoring the old ordering (mint on arrival, no staleness check)
fails `g6_21` and nothing else. A test that merely hoped for the bad
subprocess order would have passed on the broken code about half the
time, which is why this one drives the completion directly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
2026-08-09 15:52:17 +02:00
Levi Neuwirth 40027340df
feat(git): Stage 1 --- *git-status* and *git-diff*, no wire change
Implements docs/git-integration-framing.md revision 5 (approved
2026-08-09). COHERENCE.md section 15's largest named gap gets something
to attach to: a user can now answer "what have I changed?" without
leaving the editor.

  *git-status*  a `pmacs.listview` panel over
                `git --no-optional-locks -C <root> status
                 --porcelain=v2 --branch -z`. RET visits the file, `d`
                shows its diff, `g` refreshes.
  *git-diff*    the file-level diff, in a generated buffer rendered as
                plain text --- there is no bundled `diff` grammar and no
                hunk model anywhere in the tree.

NO WIRE CHANGE: no pmacs-protocol edit, no PROTOCOL_VERSION bump, no
DecorationKind variant. That is load-bearing for scheduling, not a
coincidence --- gutter markers (Stage 2) need all three and must be
scheduled alone, while this lane could run beside another.

One additive `listview` change, and the framing was wrong to say there
would be none: an optional `keys` table on the open spec. `d` cannot be
bound from outside the primitive safely, because a name collision
disambiguates to `<2>` and the name a consumer passed is not necessarily
the buffer it got. Keys are INSTALLED ONCE with the panel's buffer and
COMPARED on reopen: `Keymap::bind` refuses duplicates, and the async
completion model re-opens on every refresh, so a naive implementation
would have errored on every successful refresh.

One config-registry setting, `git.enabled`, through `pmacs.config.define`.

Facts measured against real git rather than reasoned about, each pinned
by a test:

* `ProjectKind::Git` means a BARE repository, and a language marker
  beside `.git` wins --- so pmacs reports `kind = "rust"` for its own
  repository. This module never asks pmacs whether something is a repo;
  it runs `rev-parse --show-toplevel` and lets a non-zero exit answer.
* `git diff --no-index` implies `--exit-code`: exit 1 means it
  SUCCESSFULLY found differences. The untracked predicate is exit in
  {0,1}; only >= 2 is failure. Under the naive predicate every untracked
  diff --- the case `--no-index` exists for --- would render a failure.
* An unborn HEAD makes `git diff HEAD` exit 128. Detected from
  `# branch.oid (initial)` in output already being parsed, never from a
  second `rev-parse`. `AM`/`AD` carry both states and get two labelled
  patches; rename/copy is asserted UNREACHABLE, because `git mv` on a
  staged-but-uncommitted file yields `1 A.`, not a `2` record.
* Under `-z` a rename's origin is the NEXT NUL-terminated field, not a
  tab-joined suffix, so the record tokenizer is new rather than ported
  from `tests/fixtures/pmacs-magit/`. What ports is that fixture's
  SEPARATION --- pure `parse_*` over a string --- and its case coverage.
  The fixture is untouched: it exists to prove the package system can
  host this, and bundled code becoming its dependency would make
  `m8_6_acceptance` test less than it claims.

Coherence impact, stated per CLAUDE.md:

* Section 14: `*git-status*` is the FIFTH `listview` call site and the
  first outside `lsp.lua` --- the evidence P5 asked for that the
  primitive generalizes past its first consumer.
* Section 6: no new interaction island. `d` is an ordinary buffer-local
  binding through the primitive's own path, so `describe-key` reports
  the truth and `init.lua` can rebind it. The count stays at six.
* Section 9: NEGATIVE, and named as such. A spawned process does not
  appear in `*workers*` --- that view is `async.lua`'s job list. This
  adds a fifth background thing with no single place to see it. Every
  spawn is labelled, which is better than anonymous, but a label is not
  attribution. Accepted only because these are short-lived reads.
* Journey: no step added. Git is not a journey step and this does not
  make it one.

Section 15's "no Git integration at all ... anywhere in the tree" is
narrowed here. It was literally false when written ---
`tests/fixtures/pmacs-magit/` is a tracked, installable package that
spawns git and parses porcelain v2 --- and the product gap it described
is what this closes.

Five things found by biting the suite rather than by reading, recorded
in docs/active-work.md: `listview.open`'s `seat_cursor` walks DOWN from
wherever the cursor is (so a re-opened panel lands one row low, and the
completion handler seats unconditionally from line 0); a selection test
that inserts ONE row above the selection is vacuous against exactly that
off-by-one; `{:?}` on a Rust string cannot build a `-z` fixture, because
Lua's decimal escape swallows the digit after `\0` --- which made one
test pass while parsing nothing; a path may contain a newline, so rows
escape it; and untracked rows sort after every tracked row.

Gates: scripts/gate --acceptance git_status_stage1_acceptance
--acceptance listview_acceptance --acceptance config_registry_acceptance.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
2026-08-09 14:43:21 +02:00