Compare commits

..

No commits in common. "main" and "destination-capture" have entirely different histories.

13 changed files with 51 additions and 5963 deletions

View File

@ -1306,17 +1306,6 @@ Primitive-by-primitive against the list above:
that found it (§25). All four remain in `lsp.lua`; per §25 the
symbols are authoritative and the `ad41cf1` line numbers have drifted.
**Updated again: FIVE, and the fifth is the first outside
`lsp.lua`.** Git Stage 1's `*git-status*`
(`builtin/runtime/git.lua`) is the concrete evidence P5 asked for
that the primitive generalizes past its first consumer — the
remediation here was always adoption, not construction. It also
added the primitive's one extension: an optional **`keys`** table on
the open spec, installed once with the panel's buffer and compared
(not re-bound) on reopen, because `Keymap::bind` refuses duplicates
and an async consumer re-opens on every refresh. `*buffer-list*` and
project-search remain the un-migrated hand-rolled pair.
**`*lsp*` is the only one of the four with a working refresh** — it is
the only one supplying `on_refresh`. `g` is bound on all four
unconditionally by `bind_local_keymap`, so the other three carry a
@ -1446,25 +1435,10 @@ What does not:
- **Code actions apply the first action blindly** — no picker (a
roadmap "dark matter" item still true at audit).
- **Git integration reaches status and diff, and no further.** Stage 1
(`docs/git-integration-framing.md`) ships `*git-status*` — a
`listview` panel over `git status --porcelain=v2 --branch -z`, with
RET visiting the file and `d` showing its file-level diff. There is
still **no stage, revert, blame, or gutter marker** anywhere in the
tree; gutter git riders need new `DecorationKind` variants (Stage 2,
which must be scheduled alone), and the `ResourceOffer` diff/blame
family remains a named deferral. The Git affordance list above now has
something to attach to; the affordances themselves are unbuilt, and
the menu's context vocabulary (`src/menu.rs`) has no `git` context to
host them.
The original audit said "there is no Git integration at all … anywhere
in the tree", and that was **literally false when it was written**:
`tests/fixtures/pmacs-magit/` is a tracked, installable package that
spawns git and parses porcelain v2, with a 32-test acceptance suite
(`tests/m8_6_acceptance.rs`). The **product** gap it described was
real; the sentence overstated it, and the framing that found the
overstatement is the one that closed the gap.
- **There is no Git integration at all** — no status, stage, diff,
blame, or gutter markers anywhere in the tree (gutter git riders and
the `ResourceOffer` diff/blame family are named deferrals). The Git
affordance list above has nothing to attach to yet.
- No test run/debug affordances (DAP is a future arc,
`docs/dap-debugging-framing.md`).
- No missing-tool guidance affordances (§1.2 — the diagnostic that

File diff suppressed because it is too large Load Diff

View File

@ -25,7 +25,6 @@
-- rows = { { text = "src/foo.rs:12:4", item = <any> }, ... },
-- on_visit = function(item) ... end, -- RET/SPC (optional)
-- on_refresh = function() return rows end, -- g (optional)
-- keys = { d = "git.diff-file" }, -- extra buffer-local keys
-- }
pmacs.listview = pmacs.listview or {}
@ -264,193 +263,19 @@ local function seat_cursor(p, line)
end
end
-- The primitive's own key surface, named ONCE so the binder below and
-- the `keys` validator consult the same list. Previously this was a
-- sequence of `bind(...)` calls and the set existed nowhere as data,
-- which is why the git framing had to quote it from the source
-- (docs/git-integration-framing.md Q#G-7).
local FIXED_KEYS = {
{ "RET", "listview.visit" },
{ "SPC", "listview.visit" },
{ "n", "cursor.down" },
{ "<down>", "cursor.down" },
{ "p", "cursor.up" },
{ "<up>", "cursor.up" },
{ "TAB", "listview.toggle" },
{ "g", "listview.refresh" },
{ "q", "listview.quit" },
}
local function bind_local_keymap(buf)
for _, entry in ipairs(FIXED_KEYS) do
pmacs.keymap.bind {
scope = "buffer", buffer = buf, sequence = entry[1], command = entry[2],
}
local function bind(seq, command)
pmacs.keymap.bind { scope = "buffer", buffer = buf, sequence = seq, command = command }
end
end
-- ---------------------------------------------------------------------
-- Consumer-supplied keys (Q#G-7)
-- ---------------------------------------------------------------------
--
-- An optional `keys = { <sequence> = <command name> }` on the open
-- spec, bound through the SAME `pmacs.keymap.bind { scope = "buffer" }`
-- path as the fixed set above. It exists because a consumer cannot
-- safely bind its own key from outside: `open` disambiguates a name
-- collision to `<2>`, so the name a consumer passed is not necessarily
-- the buffer it got, and this module is the only place the handle is
-- known. No key is intercepted anywhere — COHERENCE.md §6's shadow
-- count is unchanged by this.
--
-- INSTALL-ONCE, MATCH-ON-REOPEN. `Keymap::bind` refuses duplicates
-- (`KeymapError::DuplicateBinding`, "Refuse rather than silently
-- overwrite", src/keymap_tree.rs), and a consumer built on the async
-- completion model calls `open` again on EVERY refresh. So keys are
-- installed when the buffer is created and a later `open` for a live
-- panel does not re-bind — it COMPARES, and errors on divergence.
-- Silently keeping the old binding would hand the consumer a key that
-- does something other than what it just asked for, which is the dead-
-- or-lying-key defect this module already condemns for `g`.
-- A key sequence's whitespace-separated chord tokens. That is exactly
-- how `parse_sequence` (src/key.rs) splits one, so a prefix relation
-- computed here is the same relation the trie would find.
local function chords_of(sequence)
local out = {}
for token in sequence:gmatch("%S+") do out[#out + 1] = token end
return out
end
-- True when one chord list is a STRICT prefix of the other. Either
-- direction is a conflict: `Keymap` refuses both turning a leaf into a
-- submap (`WouldExtendLeaf`) and shadowing a submap with a leaf
-- (`WouldShadowSubmap`), and a `keys` table must not be able to reach
-- either.
local function prefix_conflict(a, b)
local short, long = a, b
if #a > #b then short, long = b, a end
if #short == 0 or #short == #long then return false end
for i = 1, #short do
if short[i] ~= long[i] then return false end
end
return true
end
-- Normalize `keys` into a sorted array of `{ sequence, command }`.
-- Sorted so the comparison on reopen and every error message are
-- deterministic (`pairs` order is not).
local function normalized_keys(keys)
if keys == nil then return {} end
if type(keys) ~= "table" then
error(string.format(
"listview: `keys` must be a table of sequence -> command name; got %s",
type(keys)))
end
local out = {}
for sequence, command in pairs(keys) do
if type(sequence) ~= "string" or sequence == "" then
error("listview: every `keys` entry must be keyed by a non-empty key sequence")
end
if type(command) ~= "string" or command == "" then
error(string.format(
"listview: `keys[%q]` must be a command NAME (a non-empty string); got %s",
sequence, type(command)))
end
out[#out + 1] = { sequence = sequence, command = command }
end
table.sort(out, function(a, b) return a.sequence < b.sequence end)
return out
end
-- A FIRST-PASS collision check, for a better message than the keymap's.
--
-- It compares RAW TOKENS, and that is deliberately not sufficient: the
-- key parser canonicalizes aliases before it ever reaches the trie
-- (`parse_key_code`, src/key.rs, uppercases and folds `RET`/`RETURN`/
-- `ENTER`, `SPC`/`SPACE`, `ESC`/`ESCAPE`, `BS`/`BACKSPACE`,
-- `DEL`/`DELETE`), so `keys = { RETURN = ... }` is a collision this
-- function cannot see.
--
-- **`Keymap::bind` is the authority, and `ensure_panel` tears the panel
-- down when it refuses.** That is not a fallback for a check that
-- happens to be weak --- it is the only version that cannot go stale. A
-- Lua-side canonicalizer would be a second copy of `parse_key_code`'s
-- alias table, and the day the Rust one gains a name the Lua one would
-- silently stop seeing that alias, reintroducing exactly this bug for
-- it. (There is also no way to canonicalize an arbitrary sequence from
-- Lua today: `display_sequence` is reachable only through
-- `describe.key` and `keymap.list`, which both require the sequence to
-- be BOUND already.)
--
-- So what this buys is diagnosis, not safety: a named "that is the
-- panel's own `g`" instead of a raw `DuplicateBinding`.
local function check_key_collisions(entries)
for i, entry in ipairs(entries) do
local mine = chords_of(entry.sequence)
for _, fixed in ipairs(FIXED_KEYS) do
if entry.sequence == fixed[1] then
error(string.format(
"listview: `keys` may not rebind %q --- it is part of the panel's "
.. "own key surface (RET SPC n <down> p <up> TAB g q), bound to %q",
entry.sequence, fixed[2]))
end
if prefix_conflict(mine, chords_of(fixed[1])) then
error(string.format(
"listview: `keys` entry %q conflicts with the panel's own %q --- "
.. "one is a prefix of the other, which the keymap refuses rather "
.. "than turning a binding into a submap",
entry.sequence, fixed[1]))
end
end
for j = i + 1, #entries do
if prefix_conflict(mine, chords_of(entries[j].sequence)) then
error(string.format(
"listview: `keys` entries %q and %q conflict --- one is a prefix "
.. "of the other", entry.sequence, entries[j].sequence))
end
end
end
end
-- Bind the entries, naming which one the keymap refused.
--
-- It does NOT roll back the keys it already bound: its caller owns
-- teardown, and the caller's teardown is killing the whole buffer,
-- which takes the buffer's entire keymap scope with it
-- (`after_buffer_removed` -> `KeymapStack::remove_buffer`). Unbinding
-- here as well would be a second, weaker cleanup mechanism for the same
-- failure --- and the weaker one is what let a half-built panel survive.
local function install_keys(buf, entries)
for _, entry in ipairs(entries) do
local ok, err = pcall(pmacs.keymap.bind, {
scope = "buffer", buffer = buf,
sequence = entry.sequence, command = entry.command,
})
if not ok then
error(string.format(
"listview: cannot bind %q to %q: %s",
entry.sequence, entry.command, tostring(err)))
end
end
end
local function keys_match(a, b)
if #a ~= #b then return false end
for i = 1, #a do
if a[i].sequence ~= b[i].sequence or a[i].command ~= b[i].command then
return false
end
end
return true
end
local function render_keys(entries)
if #entries == 0 then return "none" end
local parts = {}
for i, entry in ipairs(entries) do
parts[i] = string.format("%s=%s", entry.sequence, entry.command)
end
return table.concat(parts, " ")
bind("RET", "listview.visit")
bind("SPC", "listview.visit")
bind("n", "cursor.down")
bind("<down>", "cursor.down")
bind("p", "cursor.up")
bind("<up>", "cursor.up")
bind("TAB", "listview.toggle")
bind("g", "listview.refresh")
bind("q", "listview.quit")
end
-- Build the persistent panel record for `name`. A user-killed panel
@ -468,23 +293,9 @@ end
-- collision disambiguates `<2>`..`<99>`, and exhausting the limit raises
-- rather than adopting --- the rule terminal.lua:300-305 states and
-- dired.lua:476-504 already implements.
local function ensure_panel(name, key_entries)
local function ensure_panel(name)
local p = panel_for_requested_name(name)
if p then
-- Match-on-reopen (Q#G-7). A live panel keeps the keys it was
-- created with; a DIFFERENT table is a consumer asking for
-- something it will not get, so it is an error rather than a
-- silently ignored request.
if not keys_match(p.keys, key_entries) then
error(string.format(
"listview: %s is already open with keys [%s]; this open asks for "
.. "[%s]. Keys are installed once with the panel's buffer, so the "
.. "second table would be silently ignored --- close the panel "
.. "first, or pass the same keys",
name, render_keys(p.keys), render_keys(key_entries)))
end
return p
end
if p then return p end
local actual = name
if find_buffer_by_name(actual) then
@ -504,20 +315,8 @@ local function ensure_panel(name, key_entries)
local buf = pmacs.buffer.create(actual)
p = { requested_name = name, buffer = buf, line_to_item = {},
line_to_row = {}, collapsed = {}, rows = {}, visible = 0,
keys = key_entries }
-- ALL-OR-NOTHING from here. Everything below mutates a buffer that
-- does not yet belong to a panel, and `install_keys` can genuinely
-- fail: the raw-token preflight cannot see an alias spelling of a
-- fixed key (`RETURN` for `RET`), so `Keymap::bind` is the first thing
-- to notice, and by then the buffer exists, carries a read-only
-- intercept and a round-trip mark, and holds the fixed keymap.
--
-- Leaving it behind is worse than it sounds: it is read-only, it is in
-- no `panels` record so nothing owns or can reach it, and the next
-- `open` for the same name finds it by name and disambiguates itself
-- to `<2>` --- so a rejected `keys` table silently renames the panel.
local built, err = pcall(function()
line_to_row = {}, collapsed = {}, rows = {}, visible = 0 }
panels[#panels + 1] = p
-- Read-only (Q#P3): every non-bypass edit is rejected, with a NAMED
-- error. Kept beside the rope lock, not replaced by it: the layering
-- at terminal.lua:351-366 --- the rope lock protects the daemon copy,
@ -532,36 +331,13 @@ local function ensure_panel(name, key_entries)
-- is focused (RET = visit, not an optimistic newline).
pmacs.buffer.set_round_trip_input(buf, true)
bind_local_keymap(buf)
install_keys(buf, key_entries)
end)
if not built then
-- `kill` is the whole teardown, not a convenience: it removes the
-- buffer AND, through `after_buffer_removed`, prunes the buffer's
-- keymap scope, its config locals and its folds. Unbinding key by
-- key would leave the buffer itself --- which is the defect.
pcall(pmacs.buffer.kill, buf)
-- Level 0: re-raise the inner message verbatim rather than stacking
-- this line's position onto it.
error(err, 0)
end
-- Registered LAST, deliberately: a failure above must leave no record
-- claiming keys it did not bind. Nothing above needs the panel to be
-- in `panels` --- the intercept, the round-trip mark and the keymap
-- all address the buffer directly.
panels[#panels + 1] = p
return p
end
function pmacs.listview.open(spec)
assert(type(spec) == "table" and type(spec.name) == "string",
"listview.open: spec.name (string) required")
-- The cheap checks first, so the common mistakes are named before
-- anything is created. The ones this pass cannot see --- alias
-- spellings --- are caught by `Keymap::bind` inside `ensure_panel`,
-- which tears the panel down rather than leaving it half-built.
local key_entries = normalized_keys(spec.keys)
check_key_collisions(key_entries)
local p = ensure_panel(spec.name, key_entries)
local p = ensure_panel(spec.name)
p.header = spec.header or spec.name
p.on_visit = spec.on_visit
p.on_refresh = spec.on_refresh

View File

@ -1924,8 +1924,7 @@ end
local FILE_WATCH_INTERVAL_MS = 250
-- file_watchers[tostring(sid)][registrationId] = list of watch records
-- ({ cancelled = bool, form = "relative"|"absolute", _sleep = handle? }),
-- one per glob watcher.
-- ({ cancelled = bool, _sleep = handle? }), one per glob watcher.
local file_watchers = {}
-- WatchKind is a bitmask (Create=1, Change=2, Delete=4); test it
@ -2059,18 +2058,7 @@ end
local FC_CREATED, FC_CHANGED, FC_DELETED = 1, 2, 3
local function start_file_watcher(sid, base, glob, kind_mask, record)
-- Per LSP, a plain-string glob matches the file's ABSOLUTE path,
-- while a RelativePattern's pattern is relative to its base — the
-- record's `form` (from resolve_watcher) picks the match subject.
-- scan_tree always walks in relative terms; only the string handed
-- to the matcher changes.
local match_glob = glob_matcher(glob)
local matches = match_glob
if record.form == "absolute" then
matches = function(rel)
return match_glob(base .. "/" .. rel)
end
end
local matches = glob_matcher(glob)
pmacs.async(function()
local prev = scan_tree(base, matches)
while not record.cancelled and server_is_live(sid) do
@ -2081,29 +2069,6 @@ local function start_file_watcher(sid, base, glob, kind_mask, record)
if record.cancelled or not server_is_live(sid) then break end
local cur = scan_tree(base, matches)
-- The seam that makes the recheck below WITNESSABLE. `scan_tree`
-- suspends on `read_dir` once per directory, and the race is a
-- cancel arriving during one of those suspensions --- which no
-- arrangement of real timing can be made to happen on demand.
-- Same reason `git.lua` exposes `_deliver_status`: the contract is
-- about an interleaving the caller does not choose. Unset in
-- production, so this costs one nil test per tick.
-- `cur` is handed over so a test can cancel on THE SCAN THAT
-- OBSERVED a given change. Cancelling on any other scan is not a
-- witness: the loop would break at the post-sleep check on the
-- next iteration and emit nothing anyway, so the assertion would
-- pass with the recheck below deleted.
if pmacs.lsp._after_scan_for_tests then
pcall(pmacs.lsp._after_scan_for_tests, record, cur)
end
-- RECHECKED AFTER THE SCAN, not only after the sleep (review P2).
-- The coroutine is suspended for most of a tick with `_sleep`
-- already cleared, so a cancel landing there sets `cancelled` and
-- has no sleep to interrupt. Without this line the resumed scan
-- runs on to `did_change_watched_files` below and a SUPERSEDED
-- watcher emits one last batch under its OLD pattern. One batch is
-- enough: it is a wrong-pattern notification the server acts on.
if record.cancelled or not server_is_live(sid) then break end
local changes = {}
for rel, sig in pairs(cur) do
local was = prev[rel]
@ -2132,44 +2097,22 @@ local function start_file_watcher(sid, base, glob, kind_mask, record)
end
-- Resolve a GlobPattern (string | { baseUri, pattern }) to
-- (base_dir, pattern, form). The form must travel with the pair: a
-- RelativePattern's pattern is relative to its baseUri, and dropping
-- that distinction is what made absolute server globs unable to match
-- anything. A bare string with no base falls back to the directory of
-- an attached file on `sid` (best effort).
--
-- THE FORM COMES FROM THE PATTERN, NOT FROM THE UNION ARM (review P1).
-- The first fix for #233 returned `"absolute"` for every string, which
-- is a different bug wearing the same shape: LSP 3.17 defines `Pattern`
-- relative to a base path, and VS Code treats a string watcher as
-- applying across workspace folders, so a bare `*.txt` is a VALID
-- relative pattern. Classifying it absolute matched it against
-- `<base>/foo.txt`, which `^[^/]*%.txt$` can never match --- so that
-- fix silently broke a case that worked before it. A leading `/` is
-- what makes a pattern absolute; the arm it arrived in is not.
-- (base_dir, pattern). A bare string with no base falls back to the
-- directory of an attached file on `sid` (best effort).
local function resolve_watcher(sid, gp)
if type(gp) == "table" and gp.baseUri then
return pmacs.lsp.path_for_uri(gp.baseUri), gp.pattern or "**", "relative"
return pmacs.lsp.path_for_uri(gp.baseUri), gp.pattern or "**"
end
if type(gp) == "string" then
for _, rec in pairs(attachments) do
if rec.server == sid and rec.uri then
local p = pmacs.lsp.path_for_uri(rec.uri)
local dir = p and p:match("^(.*)/[^/]*$")
if dir then
return dir, gp, (gp:sub(1, 1) == "/") and "absolute" or "relative"
if dir then return dir, gp end
end
end
end
end
return nil, nil, nil
end
local function cancel_watch_records(recs)
for _, r in ipairs(recs or {}) do
r.cancelled = true
if r._sleep then pcall(function() r._sleep:cancel() end) end
end
return nil, nil
end
local function register_file_watchers(sid, registrations)
@ -2177,16 +2120,11 @@ local function register_file_watchers(sid, registrations)
file_watchers[skey] = file_watchers[skey] or {}
for _, reg in ipairs(registrations or {}) do
if reg.method == "workspace/didChangeWatchedFiles" then
-- Re-registering a live id supersedes it (rust-analyzer does
-- this): cancel the outgoing records first, because the table
-- write below drops the only reference to them and an
-- uncancelled record polls until the server dies.
cancel_watch_records(file_watchers[skey][reg.id])
local recs = {}
for _, w in ipairs((reg.registerOptions or {}).watchers or {}) do
local base, pat, form = resolve_watcher(sid, w.globPattern)
local base, pat = resolve_watcher(sid, w.globPattern)
if base and pat then
local r = { cancelled = false, form = form }
local r = { cancelled = false }
recs[#recs + 1] = r
start_file_watcher(sid, base, pat, w.kind or 7, r)
end
@ -2201,7 +2139,10 @@ local function unregister_file_watchers(sid, unregs)
if not byid then return end
for _, u in ipairs(unregs or {}) do
if u.method == "workspace/didChangeWatchedFiles" and byid[u.id] then
cancel_watch_records(byid[u.id])
for _, r in ipairs(byid[u.id]) do
r.cancelled = true
if r._sleep then pcall(function() r._sleep:cancel() end) end
end
byid[u.id] = nil
end
end

View File

@ -5,21 +5,6 @@ landed on `main`. Read it after `docs/agent-handoff.md`. Remove completed
entries when their PR merges; do not let this become a second permanent
backlog.
**Updated 2026-08-11 — two merges and a discharge.** The LSP file
watcher D1+D2 landed as **#234** (`ae84d58`) after one review round
(P1 form-from-the-pattern, P2 cancelled-scan emission — both
bite-verified), and git integration Stage 1 landed as **#227**
(`b867f64`) immediately after, refreshed onto the merged base and
re-gated. The 2026-08-10 hold on #227 is **discharged in order**, not
overridden. Both lanes below are rewritten to their remainders (D3;
git Stage 2), their durable facts absorbed into
`docs/agent-handoff.md` §1. **Next, by user ruling: D3.** The
canonical-base line below and the handoff anchor both moved to
`b867f64`. Several other lane headers still say OPEN for PRs that have
since merged (#224#232) — per this file's own rule, trust the
canonical-base line over any lane header; those absorptions remain
owed by their own lanes.
**Updated later the same day, on a new machine.** Development moved to
the laptop; the recovery path in "Repository authority" below was
exercised from this checkout and the `githubsucks` alias was absent and
@ -126,14 +111,7 @@ lesson, §1 for the two framings).
are identical on every machine. Remote names are otherwise
machine-local: `origin` may name this canonical URL, a release mirror,
or something else, and therefore has no authority by name alone.
- Canonical base at this snapshot: **`githubsucks/main` @ `b867f64`** —
git integration Stage 1 **#227**, atop `ae84d58` the LSP file-watcher
fix **#234**, atop `0e4c58d` destination capture **#231**, `3cc1b85`
worker identity Stage 1 **#232**, `0857bf4` discovery Stage 2
**#228**, `0190102` LSP LaTeX coverage **#230**, `7cf4653` the gate
`--protocol` build step **#229**, `4bc55e8` per-worktree gate target
dirs **#225**, `dcb852e` the R8 fixture fix **#226** and `b833b13`
the QoL docs retirement **#224**. Beneath those, `9a26ac8`:
- Canonical base at this snapshot: **`githubsucks/main` @ `9a26ac8`** —
GPU horizontal scroll **#223**, which **closes the QoL arc**, atop
`2b56d16` TUI horizontal scroll **#222**, `02f3ec3` `ui.line-wrap`
**#221** (protocol v22), `218d2e7` GUI zoom **#220** and `da56bec`
@ -232,43 +210,6 @@ hazard in a shape that looks committed. **A documented error message
that never appears is worse than no documentation**, because the reader
waits for a signal that is not coming.
## LSP file watcher (issue #233) — D1+D2 MERGED as #234; D3 IS NEXT
**Issue #233** — https://github.com/levineuwirth/pmacs/issues/233,
still OPEN: it closes when D3 does. **PR #234 MERGED 2026-08-11**
(`main` @ `ae84d58`), one review round. The framing is
`docs/lsp-file-watcher-framing.md`, revision 2 — it carries the full
record: the approved design, the answered ruling, and the two review
findings (P1 form-from-the-pattern, P2 cancelled-scan emission) with
their bite results. Durable facts are absorbed in
`docs/agent-handoff.md` §1.
**The #227 hold is DISCHARGED.** The 2026-08-10 ruling held #227
unmerged until this was resolved; #234 merged first and #227 followed
the same day (`b867f64`), refreshed and re-gated on the merged base.
**D3 — the polling cost — is the remainder, and the user has ruled it
is next (2026-08-11).** No branch and no framing yet. What is known,
verified while framing D1/D2:
- After #234 the watcher is *correct* but still walks: `walk` recurses
unconditionally and `matches` gates only recording, so rust-analyzer
walks the whole tree — `.git`, `target`, `node_modules` included —
every 250 ms, six times per tick (was twelve before D2), one async
job per directory. The modeline still shows the churn, at roughly
half the pre-#234 rate.
- **No `notify`/inotify dependency in the tree** — a real
filesystem-notification primitive is a new crate plus a new Rust
primitive plus its Lua binding.
- **No ignore-list infrastructure to reuse**`src/project.rs` knows
`.git` as a *marker* name, not as something to skip.
- Options named in the issue: coalesce a server's watchers into one
scan; root the scan at the workspace root; an ignore list; back off
when nothing changes; a real notification primitive.
- It is a `COHERENCE.md` §9 concern — background work with no
ownership model — and the activity indicator that surfaced it is
§9's own Stage 1. The framing must state its §20 coherence impact.
## `scripts/gate` — PR #225 OPEN (build tooling)
**PR #225** — https://github.com/levineuwirth/pmacs/pull/225. Written
@ -324,40 +265,6 @@ also removed: this branch's "R8 NEEDS A LANE" investigation block, and
durable facts are in the retired registry row and the handoff §6
census.
## Git integration — STAGE 1 MERGED as #227; Stage 2 must be scheduled alone
**PR #227 MERGED 2026-08-11** (`main` @ `b867f64`), after five review
rounds, a macOS CI round, and a base refresh: it was held unmerged
behind issue #233 by the 2026-08-10 ruling, refreshed onto the merged
base (`e2394c7`, a clean merge whose only file shared with #234 was
this ledger), re-gated 11/11 locally and 14/14 on CI. Durable facts
are absorbed in `docs/agent-handoff.md` §1; the framing
(`docs/git-integration-framing.md`, revision 5) and the PR carry the
full five-round review history.
**What shipped:** `*git-status*` — a `listview` panel over
`git --no-optional-locks -C <dir> status --porcelain=v2 --branch -z`
and `*git-diff*` (file-level, plain generated text), with the
install-once `keys` extension on `listview`. `builtin/runtime/git.lua`,
34 acceptance tests, **no wire change**.
**Stage 2 (gutter markers) is the remainder, and it is NOT freely
schedulable: it needs new `DecorationKind` variants — a
`PROTOCOL_VERSION` bump — so it must run alone**, per the strict
serialization rule on wire changes. No branch, no framing yet.
**Residue that stays live here:**
- **§9 negative impact stands:** git runs as a spawned process, and
spawned processes do not appear in `*workers*`. A fifth
unattributable background thing, labelled honestly; a label is not
attribution. The D3 lane (above) and §9 Stage 2 own the model.
- **The latent macOS sibling:** `tests/gpu_invocation_acceptance.rs`
writes a non-UTF-8 filename to disk inside
`#[cfg(feature = "crdt")]`, and the crdt job is ubuntu-only — it
fails the day that job gains a macOS leg, the same way `g6_2` did
(handoff §1: macOS cannot hold a non-UTF-8 filename).
## Destination capture (Q#JR14 generalization) — PR #231 OPEN, revision 9, cleared to merge
**PR #231** — https://github.com/levineuwirth/pmacs/pull/231. #227
@ -1490,7 +1397,6 @@ authoritative tip** — the ref, not a SHA. Recover with
— added in the second round — a **rename of either** the build or the
sweep step each fail the suite.
## QoL arc retirement — PR #224 OPEN (docs only)
**PR #224** — https://github.com/levineuwirth/pmacs/pull/224. Written

View File

@ -1,20 +1,6 @@
# Agent handoff — cross-machine continuity
**Last updated: 2026-08-11.** `main` is **`b867f64`** — git integration
Stage 1 **#227** (`*git-status*` / `*git-diff*`, no wire change), atop
`ae84d58` **#234**, the LSP file-watcher correctness fix (issue #233
D1+D2, one review round; **D3 — the polling cost — is deliberately
unfixed and is the ruled next lane**). Beneath them, in first-parent
order: **#231** destination capture, **#232** worker identity Stage 1,
**#228** discovery Stage 2, **#230** LSP LaTeX coverage, **#229** the
gate `--protocol` build step, **#225** per-worktree gate target dirs,
**#226** the R8 fixture fix, and **#224** the QoL docs retirement.
**Only #227 and #234 are absorbed into §1 at this anchor**; the eight
between carry their facts in their `docs/active-work.md` lanes, several
of whose headers still say OPEN — trust this chain over any lane
header, per the ledger's own rule.
Previously **2026-08-08**: `main` was `9a26ac8` — GPU horizontal
**Last updated: 2026-08-08.** `main` is **`9a26ac8`** — GPU horizontal
scroll **#223**, which **closes the QoL arc** (§1). Beneath it the arc's
other four: **#222** TUI horizontal scroll, **#221** `ui.line-wrap` at
protocol v22, **#220** GUI zoom, **#219** `full_grid` honored by the
@ -100,94 +86,8 @@ reads it the way you just did.
For volatile branches, checkpoints, verification, and recovery
commands, read `docs/active-work.md` immediately after this file.
## 1. Where the project stands (2026-08-11)
## 1. Where the project stands (2026-08-08)
- **Git integration Stage 1 — MERGED as #227 (2026-08-11).**
`*git-status*` (a `listview` panel over `git --no-optional-locks -C
<dir> status --porcelain=v2 --branch -z`) and `*git-diff*`
(file-level, plain generated text), plus an install-once `keys`
extension on `listview`. No wire change; **Stage 2 (gutter markers)
needs new `DecorationKind` variants — a `PROTOCOL_VERSION` bump — and
must be scheduled alone.** Held unmerged behind issue #233 by user
ruling, then landed the same day as #234, refreshed and re-gated on
the merged base. Durable facts:
- **Capture at invocation, never read at continuation — enforced by
ONE mechanism, not four counters.** Review found FOUR instances of
the same shape (ordering by `rev-parse` completion; `state.root`
read mid-plan; no generation on the diff path; one byte inside a
fix). `new_channel()` tickets answer "is this still the request in
force?" — **two channels, deliberately**: status and diff are
independent things a user asks for, so each gets its own ordering
and what is shared is the mechanism. The async-continuation census
(three continuations, one dispatcher, one synchronous impostor)
lives in the PR and framing.
- **macOS cannot hold a non-UTF-8 filename, and this project will
hit it again.** APFS/HFS+ reject invalid UTF-8 at the syscall
(`EILSEQ`, errno 92); Linux's VFS treats names as opaque bytes. It
cannot be reached around the filesystem either: an index-only
entry still fails `git status`'s lstat with `EILSEQ`, which git
skips — **there is no macOS arrangement in which real `git status`
names a non-UTF-8 path.** The fix pattern: split coverage along
the line the platform draws — behaviour runs everywhere (rows
supplied through the `_deliver_status` seam), only *provenance* is
Linux-gated. A latent sibling in `gpu_invocation_acceptance`'s
crdt module is recorded in the git lane.
- **Both root-parsing bugs lived in a pattern.** `rev-parse
--show-toplevel` gets exactly ONE trailing `\n` removed, by an
explicit last-byte test: `\r` is a legal POSIX name byte, and
`rev-parse` has **no `-z`** — it echoes a literal `-z` onto stdout
with exit 0, checked against the installed git rather than
assumed. `first_line` was deliberately left alone both times: its
three callers feed the single-line status band, where truncation
is right.
- **`{:?}` on a string containing NUL cannot build a `-z` fixture**
— Lua's decimal escape swallows following digits, so payloads are
assembled as raw bytes with three-digit escapes, joined in Lua
with `string.char(0)`.
- **A copy renders `copied from`, but `kind` stays `"rename"` for
both, deliberately.** Every behaviour keyed on it is identical;
splitting would force every present and future consumer to spell
both arms, and a forgotten arm silently degrades copies. The score
byte carries the distinction where presentation needs it.
- **LSP file watcher — D1+D2 MERGED as #234 (2026-08-11); D3 is
next.** Issue #233: with any server that dynamically registers
`workspace/didChangeWatchedFiles`, plain-string globs never matched
(matched **relative** where LSP says **absolute** — rust-analyzer
saw no file change, ever; gopls saw `go.mod` but never `.go`), and
re-registering a live id leaked the previous pollers uncancellably
(rust-analyzer registers twice under one id: 12 pollers, 6
unreachable). Invisible for three months until #232's activity
indicator — §9's instrument doing exactly its job. Durable facts:
- **The GlobPattern form travels with the pattern, and it is read
FROM the pattern, not from the union arm.** `resolve_watcher`
returns `(base, pattern, form)`; a leading `/` is what makes a
string absolute. The first fix classified every string absolute —
repairing rust-analyzer while silently breaking bare `*.txt`, a
case that had worked since May. Review caught it (P1).
- **A scan that completes after cancellation must not emit (P2).**
The watcher coroutine spends most of a tick suspended in
`read_dir` awaits with `_sleep` already cleared, so a cancel
landing there had nothing to interrupt and the resumed scan
emitted one stale batch under the superseded pattern. Cancellation
and liveness are rechecked after the scan;
`pmacs.lsp._after_scan_for_tests` (nil in production, handed the
scan result) exists because no real timing produces that
interleaving on demand — `git.lua`'s `_deliver_status` device
again.
- **F1's lesson fired twice in one lane.** The pre-existing test was
insensitive (`**/` compiles to `.-`, which spans `/`, so it passes
under either match subject) — and then the lane's own flat-pattern
guard constrained the RelativePattern *object* arm while P1's
regression lived in the *string* arm. A guard proves things about
the arm it exercises, nothing more.
- **All six watcher tests are mutation-verified, each bite failing
only its own defect** — the two review fixes re-verified
independently after review.
- **D3 is deliberately unfixed and ruled next**: the walk still
recurses into everything every 250 ms, six jobs per tick for
rust-analyzer. The D3 lane in `docs/active-work.md` carries what
was checked (no notify dependency, no ignore-list infrastructure)
and the option space.
- **QoL arc — CLOSED. All five stages merged (#219, #220, #221, #222,
#223).** From one daily-driver report: terminal zoom broke TUI
rendering and did nothing in the GUI, and a long line was unreadable

View File

@ -1,708 +0,0 @@
# Git integration — Stage 1: seeing what changed
**Status: revision 5, APPROVED 2026-08-09. Implementation may
proceed.**
**Revision 5 completes the unborn-repository policy, which revision 4
wrote as three disjoint rows when a single file can be in two states at
once.** `AM` — staged, then edited again — is not exotic; it is what a
first commit looks like halfway through. The states below were
**enumerated from a real unborn repository**, not reasoned about, and
one of them settles a case by ruling it out entirely.
**Revision 4 fixes two contracts that would have failed in ordinary
use, both verified against real behaviour rather than reasoned about:**
re-binding `d` on every refresh (keymap binds refuse duplicates, so
*every successful refresh* would have errored), and two git exit states
the failure predicate got wrong. Measured, not assumed — the exit codes
below were produced in a scratch repository.
**Revision 3 pins four Stage 1 contracts revision 2 left loose, and two
of those were again claims I made without reading the code I was
crediting.** I attributed selection preservation to `listview` and
`d` to its key surface; neither is true, and both were checkable in the
file I had already cited. The pattern is worth naming since it has now
recurred across three revisions: **I cite a file, then describe what I
expect it to contain.**
**Revision 2 answers four blockers, two of which were factual errors in
revision 1 that scouting should have caught and did not.** I read
`ProjectKind::Git`'s name instead of its doc comment, and I quoted
`COHERENCE.md` §15's "no Git integration anywhere in the tree" without
checking whether it was still true of the tree. It is not.
---
## 1. Why this, and why now
`COHERENCE.md` §15 is blunt about it:
> **There is no Git integration at all** — no status, stage, diff,
> blame, or gutter markers anywhere in the tree (gutter git riders and
> the `ResourceOffer` diff/blame family are named deferrals). The Git
> affordance list above has nothing to attach to yet.
**That sentence is literally false about the tree, and revision 1
repeated it without checking.** `tests/fixtures/pmacs-magit/` is a
tracked, installable package — 1,914 lines across four modules, with
`status.lua` spawning git through `pmacs.process.spawn` and parsing
**`--porcelain=v2 --branch`** into structured sections, plus a 662-line
acceptance suite (`tests/m8_6_acceptance.rs`, 32 tests) covering
status, refresh, staging, commit, push and branch behaviour.
**The PRODUCT gap is real and unchanged** — none of that is bundled
runtime, so a user who installs pmacs gets no git integration. But
"nothing to attach to" understates what exists to *learn from*, and
§15's wording should be corrected when this lands.
For a **daily driver**, this is the largest remaining gap. Not because
git is the most architecturally interesting thing missing — §7
workspaces and §9 worker identity are both deeper — but because it is
the one a user touches *every working hour*, and pmacs currently makes
them leave the editor to answer "what have I changed?".
That is the criterion this lane is chosen against: **frequency of use
per day**, not depth of model.
## 2. Ground truth — what already exists
Scouted, not assumed:
- **`ProjectKind::Git` is NOT general repository detection**, and
revision 1 said it was. Its doc comment is explicit: *"A bare git
repository (no language marker found inside)"* (`src/project.rs:89`).
Markers are ordered and a language marker beside `.git` **wins**
(`src/project.rs:10`), so a normal Rust repository reports
`kind = "rust"` and would have been invisible to a lane that gated on
`kind == "git"`. That gate would have failed on this very repository.
**The rule this lane uses instead: never ask pmacs whether it is a
git repo.** Run git in the **active file's directory** and let git
resolve its own worktree — `git -C <dir> rev-parse --show-toplevel`
establishes the root, and a non-zero exit *is* the "not a repository"
answer. Git's own resolution handles submodules, worktrees, `GIT_DIR`
and `.git` files; a marker walk reimplements a subset of that and
gets it subtly wrong.
- **`pmacs.process.spawn` / `events_take` / `terminate` / `forget`**
is the working model for running an external tool asynchronously;
`builtin/runtime/compile.lua` is a full worked example, including
spawn-failure handling and exit markers.
- **`pmacs.listview.open`** is a real primitive with existing adopters
(`*references*`, `*lsp*`), carrying optional `depth`/`id`,
primitive-owned collapse, and selection re-seated by id. `COHERENCE.md`
P5 says the remaining work there is **adoption, not construction**
a `*git-status*` panel is exactly that.
- **Gutter signs exist in both frontends** — the TUI's leading-column
glyph (`src/diag.rs`) and the GPU's `GUTTER_SIGN_X` bars
(`pmacs-gpu/src/main.rs:420`).
- **A tested porcelain-v2 parser exists as a package fixture** (above).
Its `status.lua` deliberately separates **pure `parse_*` functions
that take a string and return structure** from the spawning around
them — which is the shape that makes a parser testable without a
repository, and it is already proven by 32 tests.
And the constraint that shapes the staging:
- **`DecorationKind` is a CLOSED enum on the wire**
(`pmacs-protocol/src/message.rs:1472`): four diagnostic severities,
`Selection`, `SearchMatch`, `SearchMatchActive`, `CurrentLine`.
**Gutter markers for git hunks therefore require new variants, which
is a protocol version bump.** The gutter signs that exist are keyed
on `diagnostic_severity_rank` and have no notion of anything else.
## 3. The staging, and why the line falls where it does
**Stage 1 (this lane): read-only, panel-based, NO WIRE CHANGE.**
- `*git-status*` — a `listview` panel over
`git status --porcelain=v2 --branch -z` (Q#G-6), rows visiting the
file at RET, refreshed by `g` under the completion model in Q#G-1.
- `*git-diff*` — the diff for the **file** under point (Q#G-7), in a
generated buffer rendered as **plain text** (no `diff` grammar
exists). **No hunk model** — hunks are Stage 2's concern.
**Stage 2 (separate lane): gutter markers.** Needs new
`DecorationKind` variants and a `PROTOCOL_VERSION` bump, plus both
frontends' gutter renderers learning a second rider family.
**Stage 3+ (unscheduled): staging, commit, blame.** Staging and commit
are where an editor becomes a git *client*; blame is a lower-frequency
read. Neither belongs in front of the two above.
**The line is drawn at the wire on purpose, and it is a scheduling
decision as much as a design one.** Parallel lanes are about to start,
and `PROTOCOL_VERSION` is a strict serialization point — two lanes
bumping it collide, and this session already recorded what that costs
(eight broken version assertions on CI from a single bump). Stage 1
touching no wire is what lets it run **concurrently** with other work.
Stage 2 must be scheduled alone.
## 4. Coherence impact (§20)
Required by `CLAUDE.md` for coherence-affecting work, and this is
coherence-affecting — it is §15's named gap.
- **Journey steps touched:** none directly. Git is not currently a
journey step; the golden journey runs open → edit → build → test →
navigate. This lane does **not** add a step, and I would rather say
so than inflate the claim.
- **§15 contextual affordances — the direct target.** The audit's git
affordance list ("a Git change stage/revert/diff") has *nothing to
attach to*. Stage 1 creates the thing to attach to; the affordances
themselves follow it, and the menu's context vocabulary
(`src/menu.rs:44`) would need a `git` context to host them — **out of
scope here**, named so it is not forgotten.
- **§14 workbench primitives — adoption, which is the stated need.**
`*git-status*` becomes the **fifth** `listview` call site and the
first outside the LSP panels, which is the concrete evidence P5 asks
for that the primitive generalizes past its first consumer.
- **Interaction islands (§6): none added, and this is a real
constraint.** The panel gets no hardcoded key interception; it uses
`listview`'s existing key handling. §6 records six such shadows and
calls them "weak, and growing" — this lane must not make it seven.
- **Config registry adoption:** at least one setting
(`git.enabled`, Q#G-4), defined through `pmacs.config.define` like
`ui.line-wrap` and the zoom settings, not a bare Lua global.
- **Background-work attribution (§9): NEGATIVE, and named as such.**
Git runs as a spawned process, and spawned processes do **not** appear
in `*workers*` — that view is `async.lua`'s job list; processes live
under `pmacs.process.list` (Q#G-5). This lane therefore adds a fifth
thing running in the background with no single place to see it. The
process is labelled honestly, which is better than anonymous, but
**a label is not attribution and this document does not pretend
otherwise.** Accepted because these are short-lived reads; it would
not be acceptable for Stage 3's push/pull.
## 5. Open questions
### Q#G-1 — is the status panel a snapshot or a live view?
A snapshot is a command that opens a panel; a live view refreshes on
buffer save, on focus, or on a filesystem watch.
*My vote: **snapshot, refreshed explicitly***, with `g` re-running
inside the panel. Live refresh needs a watch mechanism, an invalidation
rule, and a §9 story for the recurring work — all real arcs. A snapshot
is honest, useful the first day, and does not pretend to a currency it
cannot maintain.
**But "explicit refresh" does not fit `listview` unmodified, and
revision 1 missed that.** `listview.refresh` is synchronous:
```lua
local rows = check_ids(p.on_refresh() or {}) -- listview.lua:402
```
The result is consumed immediately. `pmacs.process.spawn` cannot return
rows there — it returns a process id whose output is drained later. So
revision 1's "adopt `listview`" would have produced exactly one of the
two failures the reviewer named: a reimplemented list, or a `g` that
silently does nothing. The primitive's own docs already call a dead `g`
out as a defect it must not repeat (`listview.lua:416`).
**The completion model, specified.** `on_refresh` stays synchronous and
honest:
1. **`on_refresh` returns the CURRENT rows immediately**, with a
`refreshing…` marker row appended, and *kicks off* the spawn. `g` is
therefore never a no-op — it always re-renders and always shows that
work started.
2. **On exit, the completion handler re-opens the panel** via
`listview.open` with the same `name` — **and re-seats the selection
itself.**
Revision 2 credited that to the primitive and was wrong.
`listview.open` **resets collapse** (`p.collapsed = {}`) and
**always seats line 1** (`seat_cursor(p, 1)`,
`builtin/runtime/listview.lua:337-378`). The `listview.lua:82` note
I cited is about **name** disambiguation to `<2>`, not selection.
Only `listview.refresh` preserves a selection, and that is the
synchronous path this model cannot use.
So the contract is explicit and owned here: **capture the selected
row's git id (its current path) before re-opening, and after
re-opening move to the line whose row carries that id**, computed
from the handler's own rows array via `pmacs.editor.move_to_line`.
If the id is gone from the new status — the commonest case, since a
file that stopped being modified drops out — seat line 1 and say
nothing; that is the correct answer, not a failure.
**Collapse state is moot in Stage 1** because the rows are flat: no
`depth`, so nothing to collapse. Stage 2 or a sectioned view would
have to revisit this, and would then face the same reset.
3. **Concurrent refresh is suppressed by a generation counter.** A
second `g` while one is in flight bumps the generation; the older
completion sees a stale generation and **discards its rows** rather
than racing. It does not terminate the first process — reaping is
`pmacs.process.forget`'s job and killing git mid-read buys nothing.
4. **Failure is a row, not a silence.** Non-zero exit or spawn failure
renders a row carrying the exit code and the first stderr line, plus
a status message. §1.2's silence asymmetry.
5. **Panel lifetime.** If the panel's buffer is gone when the process
exits, the handler drops the result. `compile.lua:252` already
handles the buffer-killed case for its own slot; the same shape.
**The alternative — extending `listview` with an async contract — is
the more correct long-term answer** and is deliberately not taken here:
it changes a primitive with four existing adopters, and doing that from
inside its fifth adopter's lane is how a primitive acquires a consumer's
idiosyncrasies. **If review prefers it, it belongs in its own lane
before this one.**
### Q#G-0 — what is the relationship to `pmacs-magit`? **(new in rev 2)**
The reviewer's framing of the choice is right: adopt, replace, or
declare it out-of-product precedent. Doing none of those and quietly
writing a second parser is the option that must not happen.
*My vote: **port its pure `parse_*` functions and its test corpus into
the bundled runtime; leave the fixture itself untouched.***
- **The record TOKENIZER is deliberately rewritten, not ported.** The
fixture parses **newline-delimited** v2; Stage 1 reads **`-z`**, and
those are different grammars — under `-z` a record's fields are
NUL-terminated and a rename carries its two paths as separate fields
rather than tab-joined. Saying "port the parser" would have been
wrong; what ports is the **separation** (pure `parse_*` functions
over a string, testable with no repository) and the **case coverage**
its 32 tests encode. The tokenizer underneath is new, and its
correctness rests on this lane's own corpus.
- **Port, not import.** The fixture's purpose is to prove the *package
system* can host this. If bundled code became its dependency, it
would stop demonstrating an independent package and `m8_6` would test
less than it claims.
- **The duplication is therefore deliberate**, and it is the one place
this framing accepts two copies of a rule after a session spent
removing them. The justification is that they answer different
questions — one is product behaviour, one is package-system
capability — and coupling them weakens the second. **If review
prefers the coupling, that is a defensible call and I will take it**;
what I will not do is leave the duplication unstated.
- **It also settles Q#G-2's format**: the existing, tested parser is
**porcelain v2**, so Stage 1 is v2. Revision 1 said v1 for no reason
beyond familiarity.
### Q#G-2 — `git` the binary, or a library?
*My vote: **the binary**, via `pmacs.process.spawn`. `compile.lua` is
the worked precedent, the daemon already spawns external tools, and a
git library is a dependency with a much larger surface than "run one
command and parse porcelain". `--porcelain=v2` is explicitly a stable
machine format; that is what it is for.
**Named risk:** no `git` on `PATH`. §1.2's *silence asymmetry* says the
failure must be **surfaced with guidance**, not swallowed — the same
lesson #204 landed for a missing language server.
### Q#G-6 — the status data contract **(new in rev 2)**
Revision 1 said "`--porcelain=v1`" and proposed "a path with a space"
as the parsing witness. **Both were inadequate.** Porcelain without
`-z` emits paths in git's **C quoting** for anything non-ASCII or
containing special characters, and rename/copy records carry *two*
paths whose separation is positional. A single space-in-path fixture
proves none of that.
*My vote: the exact invocation*
```
git --no-optional-locks -C <dir> status --porcelain=v2 --branch -z
```
**`--no-optional-locks` is part of the contract, not a nicety.**
`git status` is **not strictly read-only**: it may refresh and write
the index, and git's own documentation recommends this flag for
background scripts precisely so a background reader does not contend
for `index.lock` with the user's real git commands
(<https://git-scm.com/docs/git-status>). This lane runs status
*asynchronously, from an editor, while the user may be running git in a
terminal* — the exact scenario the flag exists for. Revision 2 called
the lane "read-only" and that was wrong about the mechanism.
It is **witnessed structurally** — the assembled argv is asserted to
carry the flag — because observing a lock that was *not* taken is not
something a test can do directly. Verified accepted by the git in use
here.
The rest: `--porcelain=v2 --branch -z`, also verified accepted. NUL delimiting removes C quoting from
the problem **entirely** rather than obliging a hand-written unquoter,
and it makes the two-path rename record unambiguous: the paths are
separate NUL-terminated fields rather than tab-joined inside one.
The rename/copy identity rule to pin: a `2` record carries the current
path **and** its origin, and the panel must show which file it is now
while remembering where it came from — a row whose id is the current
path, since that is what RET visits.
**Witness corpus, not one case:** modified, added, deleted, untracked,
**renamed (both paths)**, **copied**, a path with a space, a path with
a newline, and a non-UTF-8 path. The last two are exactly what `-z`
buys and what a quoted parser gets wrong.
### Q#G-7 — the diff gesture **(new in rev 2)**
Revision 1 wrote "the diff for the file or hunk under point" while also
committing RET to visiting the file. **RET cannot do both, there is no
second binding proposed, and no hunk model exists anywhere in the
tree.**
*My vote:*
- **RET visits the file** — unchanged, and the behaviour a list of
files should have.
- **A named command, `git.diff-file`, bound to `d` inside the panel.**
**`d` is not on `listview`'s key surface**, and revision 2 said it
was. The bound set is exactly `RET SPC n <down> p <up> TAB g q`
(`builtin/runtime/listview.lua:266-279`), bound buffer-locally inside
the primitive, which is the only place the panel's buffer handle is
known. **Looking the buffer up by name from outside is unsafe**
`listview` deliberately disambiguates a collision to `<2>`, so the
name a consumer passed is not necessarily the buffer it got.
*My vote: **a `keys` table on the open spec***, e.g.
`keys = { d = "git.diff-file" }`, bound through the same
`bind_local_keymap` that already binds the fixed set. It is additive,
general to any adopter, keeps binding where the buffer is known, and
adds **no** interception — the §6 constraint holds.
**The registration lifecycle, which revision 3 omitted and which
would have broken the refresh path it depends on.** `Keymap::bind`
**refuses duplicates**`KeymapError::DuplicateBinding`, *"Refuse
rather than silently overwrite"* (`src/keymap_tree.rs:75`) — and the
completion model calls `listview.open` again on **every** refresh. A
naive `keys` implementation therefore errors on the second open, so
**every successful refresh would have failed while re-binding `d`.**
The contract:
1. **Keys are installed once, when the panel's buffer is created**,
and stored on the panel.
2. **A later `open` for a live panel does not re-bind.** It
**compares** the supplied `keys` against the stored table and
**errors on divergence** rather than ignoring it. Silently keeping
the old binding would give the consumer a key that does something
other than what it just asked for — a dead or lying key, which is
the defect `listview` already condemns for `g`.
3. **Collisions are rejected at install time**, against both the
fixed set (`RET SPC n <down> p <up> TAB g q`) and any
**prefix conflict**`Keymap` has a separate error for turning a
leaf into a submap, and a `keys` table must not be able to reach
it.
(The alternative, idempotent re-registration, is tolerable but
strictly weaker: it makes a consumer that changes its keys mid-session
silently wrong instead of loudly wrong.)
**This IS a `listview` modification, and revision 2's "no listview
modification" was false.** I distinguish it from the async-contract
change I deferred: that one alters *when* an existing callback's
result is consumed for four existing adopters; this adds an optional
field that changes nothing for a spec that omits it. **If review
judges any primitive change out of an adopter's lane, the alternative
is `listview.open` returning the panel buffer** so the consumer binds
its own key — smaller still, but it pushes binding to every adopter.
- **No hunk model in Stage 1.** Hunks are precisely what gutter markers
need, and that is Stage 2's protocol work. Introducing a half hunk
model here to serve one gesture would prejudge Stage 2's design from
the wrong side.
**And what `d` actually SHOWS, which revision 2 left unstated.** "File,
not hunk" is a scope, not a contract. A porcelain-v2 row carries an
**XY** pair — X staged (index vs HEAD), Y unstaged (worktree vs index)
— and the three plausible diffs answer three different questions:
`git diff` shows only Y, `--cached` only X, and neither shows an
untracked file at all.
*My vote: **`d` answers the lane's own question — "what have I
changed?" — against `HEAD`:***
| row | `d` runs | why |
|---|---|---|
| staged, unstaged, or both | `git diff HEAD -- <path>` | one view of the total change; splitting X from Y is a staging UI, which is Stage 3 |
| deleted | `git diff HEAD -- <path>` | shows the deletion; no special case needed |
| renamed / copied | `git diff HEAD -- <orig> <current>` | v2 gives both paths; passing both is what lets rename detection render it as a rename rather than an unrelated add+delete |
| **untracked** | `git diff --no-index -- /dev/null <path>` | **a normal diff shows nothing at all** for an untracked file. Without this case `d` is silently dead on the rows a user is most likely to press it on |
| non-UTF-8 path | *refuses, with a message* | see Q#G-8 |
The `HEAD` choice is deliberate and is the one thing here I would most
expect review to push back on: it is the right default for *reading*
what changed, and the wrong one for *staging*, which is why it is
correct for Stage 1 and will need revisiting when Stage 3 arrives.
**The exit-state contract, which revision 3 got wrong in two ways.**
"Non-zero exit renders a failure row" is not correct for `git diff`.
Both cases below were measured in a scratch repository, not inferred:
**(a) `--no-index` implies `--exit-code`.** It exits **1 when it
successfully finds differences** — measured: `exit=1` for an untracked
file against `/dev/null`. Under revision 3's predicate, *every*
untracked diff — the case `--no-index` exists to serve — would have
rendered a failure row instead of the diff it just produced.
So for the untracked path the success predicate is **exit ∈ {0, 1}**,
rendering whatever came out; **exit ≥ 2 is a real failure**. That
asymmetry is confined to the `--no-index` invocation and does not leak
to the others, where non-zero still means failure.
**(b) An unborn repository has no `HEAD`.** Measured:
`git diff HEAD -- <path>` exits **128** with `fatal: bad revision
'HEAD'`. This is not an edge case — it is a freshly `git init`-ed
repository with the first files staged, which is exactly when someone
opens a status panel to see what they are about to commit.
*Policy: **detect once, then split**.*
**Detection needs no extra subprocess.** `--branch` already reports
`# branch.oid (initial)` when `HEAD` is unborn — observed in the
output this lane already parses. Revision 4 proposed a separate
`git rev-parse --verify --quiet HEAD`; that is a second process for a
fact the first one hands over.
**The reachable states, enumerated from a real unborn repository** —
`git init`, stage three files, then edit one, delete one, and `git mv`
one:
```
# branch.oid (initial)
1 AD ... ad.txt
1 AM ... am.txt
1 A. ... r_new.txt <- the `git mv`
? untracked.txt
```
Two findings fall straight out:
- **`AM` and `AD` are ordinary and carry BOTH states**, which is
exactly the gap: `--cached` alone loses the worktree delta, plain
`git diff` alone loses the staged base.
- **Rename and copy CANNOT occur under an unborn `HEAD`.** The
`git mv` produced `1 A. … r_new.txt` — an ordinary add of the new
path, **not** a `2` record. With no `HEAD` there is nothing to
rename *from*, so the rename/copy row class is unreachable here and
needs no unborn policy. That is a case closed by evidence rather than
handled speculatively.
| unborn row | `d` renders |
|---|---|
| `A.` staged only | one patch: `git diff --cached -- <path>` |
| **`AM` staged + edited** | **two labelled patches***staged* `git diff --cached -- <path>`, then *unstaged* `git diff -- <path>` |
| **`AD` staged + deleted** | **two labelled patches**, same pair; the second renders the deletion |
| `.M` / `.D` unstaged only | one patch: `git diff -- <path>` |
| `?` untracked | `git diff --no-index -- /dev/null <path>` (exit ∈ {0,1}) |
| rename / copy | **unreachable** — see above |
All four `--cached` / plain invocations above were run against that
repository and render the expected patches.
**The split is unborn-only, and that asymmetry is deliberate.** Once
`HEAD` exists, `git diff HEAD` gives one total — which is the lane's
question — and splitting it would be a staging UI (Stage 3). The split
appears here only because there is no `HEAD` to total *against*.
The generated buffer carries a **header naming what it is showing**:
*"no commits yet — split view: staged (index) above, unstaged
(worktree) below"*. Revision 4's wording ("showing staged changes")
would have described a single total-against-`HEAD` diff, which is
precisely what this is not. A diff that silently answers a different
question than the one asked is worse than one that says so — and a
header that misdescribes a split view is the same failure in smaller
type.
### Q#G-8 — non-UTF-8 paths: an honest boundary **(new in rev 3)**
Revision 2 listed a non-UTF-8 path in the witness corpus as though it
were an end-to-end case. **It cannot be**, and the boundary is in the
bindings: `pmacs.process.spawn` takes `args: Vec<String>`
(`src/lua_bindings/mod.rs:8683`) and `pmacs.buffer.find_or_open` takes
`path: String` (`:3564`). Both are Rust `String`, i.e. UTF-8 by
construction. A path that is valid bytes but not valid UTF-8 can be
*read* from git's `-z` output and *displayed*, but it cannot be passed
back to `spawn` for a diff, nor opened.
*My vote: **parse it, show it, and refuse the gesture with a
message***:
- the row **appears** in the panel, so the user is not lied to about
what is modified;
- **RET and `d` on that row report** that the path is not representable
and do nothing else — a witnessed refusal, not a stack trace or a
silent no-op;
- **it is removed from the end-to-end promise.** The witness is
parser-and-display **plus the refusal**, and the framing does not
claim visiting works.
Making it work end-to-end means `OsString`/bytes through two binding
boundaries — a real change to the Lua API surface, and not this lane's.
### Q#G-3 — what does the diff view render into?
*My vote: **a generated buffer**, reusing the generated-buffer
immutability work (Stage 1 merged; that lane's Stage 2 is queued).
Diff output is read-only text and that machinery exists.
**RESOLVED in rev 2 — there is no bundled `diff` grammar.**
`BUILTIN_LANGUAGES` (`src/syntax.rs`) has no `diff` entry; checked, not
assumed. **Stage 1 renders plain generated text**, and diff
highlighting is later work needing a grammar first.
### Q#G-4 — what is configurable?
*My vote: **one setting to start**`git.enabled` (boolean, default
`true`), through the config registry. Resist more until there is use
evidence; §11's grade is "partial (foundation only)" and adding five
speculative settings is how a registry becomes noise.
### Q#G-5 — §9 attribution — **RESOLVED, and the answer is negative**
Revision 1 deferred this to implementation. That was wrong: it is
answerable by reading, and deferring it would have meant discovering a
known coherence cost *after* committing to the design.
**A spawned git process does not appear in `*workers*` at all.** That
buffer is `builtin/runtime/async.lua`'s (`:490`) and lists **async
jobs**; spawned processes live separately under `pmacs.process.list`.
They are two of the four disjoint activity views §9 grades as
"mechanism without identity".
So, stated plainly rather than dressed up:
- **This lane adds a fifth thing that runs in the background and is not
attributable from one place.** That is a **negative** coherence impact
against §9, and it is the honest cost of shipping git status before
worker identity exists.
- **Labelling the process is still required** — a clear label under
`pmacs.process.list` is strictly better than an anonymous `git`. But
**a label does not solve attribution**, and this document does not
claim it does. The claim is only: do not make it worse than it has to
be.
- **The mitigation is bounded in time, not in kind.** These are
short-lived reads, not long-running jobs; a `git status` that has not
finished is a bug, not a background task a user needs to supervise.
That is why the cost is acceptable *now* and would not be for
Stage 3's push/pull.
## 6. Verification
- **Parsing, against a corpus rather than a case (Q#G-6):** modified,
added, deleted, untracked, **renamed with both paths**, **copied**, a
path with a space, and **a path with a newline** — the last is what
`-z` buys, and a parser that passes only the space case is the one
that ships broken.
- **A non-UTF-8 path is parsed and displayed, and its gestures refuse
with a message** (Q#G-8) — a witnessed refusal at the binding
boundary, **not** an end-to-end visit.
- **The argv carries `--no-optional-locks`** (Q#G-6), asserted
structurally. A lock not taken cannot be observed directly, so the
invocation is what gets pinned.
- **`d` is witnessed on every row class** (Q#G-7): staged, unstaged,
both, deleted, renamed, and **untracked** — the last because a normal
`git diff` shows nothing there, so a missing `--no-index` case makes
`d` silently dead exactly where it is most used.
- **A copy is reported as a COPY, not a rename** (Q#G-7). Porcelain v2
folds both into the one `2` record, so `kind` stays `"rename"` for
both — every *behaviour* keyed on it is the same — and the
distinction is made where it is a distinction: the diff header reads
the `<Xscore>` field's leading `R`/`C` and says which one happened.
The status row is left alone, because its `XY` prefix already reads
`R.` against `C.`. Both classes are asserted, and so is the **argv**:
the two-path `git diff HEAD -- <orig> <current>` is right for a copy
and a rename alike, so a fix to what the user is *told* must not
reach what runs. **Parser-level, deliberately** — the copy ROW is
supplied through `_deliver_status` while the repository, the panel,
the `d` dispatch and the spawned diff around it are real.
**The reason, narrowed after review.** This bullet used to say real
`git` emits no `2 C` record "even under `status.renames=copies`".
**That is too strong, and git's own documentation contradicts it**
`git-status(1)` lists `C` as *"copied (if config option
status.renames is set to `copies`)"*. What the test measures is
narrower: **for its fixture, whose copy source is left unchanged**,
git reports `1 A.`. That is a fact about the fixture, and it is
sufficient reason to craft the row — a weaker and true justification
in place of a stronger false one. No mechanism is claimed for why an
unchanged source is not offered as a candidate; that was never
established.
- **The untracked diff renders on exit 1**, not a failure row (Q#G-7a)
— the case `--exit-code` semantics would otherwise break, and the
one most likely to be "fixed" later by someone who reads exit 1 as an
error.
- **An unborn repository is witnessed end to end**, and the fixture is
**`AM`** specifically — staged then edited again, the shape a first
commit actually has partway through. `git init`, stage, edit, open
the panel, press `d`, and get **two labelled patches** with the
split-view header — not `fatal: bad revision 'HEAD'`, and not a
single `--cached` patch that silently drops the worktree edit.
**`AD` rides the same fixture**, since one repository can hold both.
- **Unborn detection reads `# branch.oid (initial)`** from the status
output already being parsed — asserted, so nobody later reintroduces
a second `rev-parse` process for a fact already in hand.
- **Rename/copy under an unborn `HEAD` is asserted UNREACHABLE**: the
fixture `git mv`s a staged-but-uncommitted file and the parser sees a
`1 A.` record, never a `2`. Pinned so a future reader does not
"fix" the missing unborn rename policy by inventing one.
- **Re-binding across a refresh does not error** (Q#G-7): two
successive refreshes on a live panel, asserting `d` still works and
no `DuplicateBinding` surfaced. This is the one that would have
broken on every refresh.
- **A `keys` table colliding with the fixed set is rejected at install
time**, as is a prefix conflict.
- **Selection is re-seated by the completion handler** (Q#G-1), across
a refresh that reorders rows, and **falls back to line 1 without
complaint when the selected path drops out of status** — the common
case, not an error.
- **The pure `parse_*` functions are tested without a repository**,
which is the shape `pmacs-magit/status.lua` already proves works and
the reason to port that separation rather than invent one.
- **A repository fixture built with real `git`**, in a tempdir, and
**bounded with `set_search_boundary`** — R8 was retired two commits
ago and is precisely what happens when a fixture lets project
detection escape into the developer's environment.
- **The root rule is witnessed on a repository whose `ProjectKind` is
NOT `Git`** — i.e. an ordinary language project with a `.git` beside
its manifest. That is the case revision 1's `kind == "git"` gate
would have failed, and this repository is one.
- **Missing `git` on `PATH` is witnessed**, not assumed (Q#G-2), and
surfaces guidance rather than silence.
- **`g` is never a no-op** (Q#G-1): it re-renders and marks that work
started, even mid-flight. A dead `g` is a defect `listview` already
names.
- **Concurrent refresh discards the stale generation** rather than
racing — asserted by driving two refreshes and completing them out of
order.
- **Failure renders a row**, carrying exit code and stderr.
- **The panel is a `listview` adopter**, asserted structurally, so a
future re-implementation of list behaviour inside git code fails the
test rather than passing review.
- **No new interaction island**`d` is bound buffer-locally through
`listview`'s own binding path (Q#G-7), not a hardcoded interception.
§6 stays at six shadows.
Gates via `scripts/gate --acceptance <the new suite>`.
**What this will NOT prove:** that background git work is attributable
(Q#G-5 — it is not, by construction), or that the parser handles
porcelain versions other than v2.
## 7. Not in scope
Gutter markers and any `DecorationKind`/`PROTOCOL_VERSION` change
(Stage 2 — must be scheduled alone). Staging, commit, push, pull,
branch operations, merge-conflict resolution. Blame. A `git` context in
the menu vocabulary. Any git *library* dependency. Live refresh
(Q#G-1). Fixing §9's worker identity — this lane makes it marginally
worse and says so (Q#G-5). Any hunk model (Q#G-7). Modifying the
`listview` primitive to carry an async contract — the better long-term
answer, but it belongs in its own lane before this one, not inside its
fifth adopter (Q#G-1). Changing `tests/fixtures/pmacs-magit/` or
`tests/m8_6_acceptance.rs` (Q#G-0).
**A `listview` change IS in scope after all** (Q#G-7): an optional
`keys` table on the open spec. Revision 2 said no primitive
modification; that was false, because `d` cannot be bound from outside
the primitive safely. The async-contract change stays out.
**One correction this lane should carry when it lands:** `COHERENCE.md`
§15's "no Git integration anywhere in the tree" is literally false —
`tests/fixtures/pmacs-magit/` exists. The *product* gap it describes is
real; the sentence needs narrowing to say so.

View File

@ -231,33 +231,12 @@ all share one keymap:
| `RET` / `SPC` | `listview.visit` — act on the item under the cursor |
| `n` / `<down>` | `cursor.down` |
| `p` / `<up>` | `cursor.up` |
| `TAB` | `listview.toggle` — collapse/expand the tree node under the cursor; a panel with no tree rows delegates to `buffer.tab` |
| `g` | `listview.refresh` — re-run the data source and re-render |
| `q` | `listview.quit` — restore the buffer that was active before the panel opened |
(`TAB` arrived with the tree primitive and this table had not recorded
it. Noted rather than quietly added: the omission predates the git lane
that found it.)
Panels currently built on this: `*references*`, `*outline*`,
`*lsp-help*` (hover docs), `*lsp*` (`lsp.status`), and `*git-status*`
(`git.status`). Header text always spells out the panel's own legend
inline.
A panel may add keys of its own through an optional `keys` table on the
open spec, bound through the same buffer-local path — so they are
inspectable by `describe-key` and rebindable from `init.lua`, exactly
like the fixed set. They are installed once with the panel's buffer and
may not collide with the fixed set, nor prefix it. One panel uses this
today:
| Buffer | Key | Command |
|---|---|---|
| `*git-status*` (`git.status`) | `d` | `git.diff-file` — the diff for the file under the cursor, into `*git-diff*` |
`git.status` gets **no global chord**: an opening key is a
command-surface decision the Stage 1 framing did not make, so the entry
point is `M-x git.status`.
`*lsp-help*` (hover docs). Header text always spells out the same
`RET`/`n`/`p`/`g`/`q` legend inline.
`*buffer-list*` (`editor.list-buffers`, `C-x C-b`) uses its own
keymap, layered on the same idiom, in `builtin/commands/default.lua`:

View File

@ -1,240 +0,0 @@
# LSP file watcher — framing
**Status: revision 2 — approved design (2026-08-10), plus two
correctness findings from review OF THE IMPLEMENTATION.** The user ruled
that D1 and D2 proceed with the walking explicitly surviving this lane;
D3 gets its own framing. The acceptance bar for this lane is correctness
and the leak, not "the flipping stops".
**Revision 2 records a review round against the code, not the design.
Both findings are cases where the first fix was itself wrong**, and both
were confirmed against the tree before being acted on:
- **P1 — the form must be read from the PATTERN, not from the union
arm.** `resolve_watcher` returned `"absolute"` for *every* string, so
a bare `*.txt` — a valid relative pattern under LSP 3.17, and how VS
Code treats string watchers across workspace folders — was matched
against `<base>/foo.txt` and could never fire. **That case worked
before this lane touched it**, so the repair for #233 silently broke a
live path while fixing another. A leading `/` is what makes a pattern
absolute; classification now reads the string.
- **P2 — a scan completing after cancellation still emitted.**
`scan_tree` awaits `read_dir` once per directory, so the coroutine
sits suspended for most of a tick with `_sleep` already cleared. A
cancel arriving there sets `cancelled` and has no sleep to interrupt,
and the resumed scan ran on to `did_change_watched_files` — one stale
batch under the superseded pattern, which is a wrong-pattern
notification the server acts on. Cancellation and liveness are now
rechecked after the scan.
**F1's lesson repeated itself inside this lane.** The flat-pattern test
constrains the RelativePattern **object** arm, so it said nothing about
the **string** arm P1's regression lived in — the same
tested-path/exercised-path split this framing opened by naming. Both
findings now have tests, and both tests were mutation-checked: each
fails only its own defect.
**A test seam was added, and is recorded here rather than buried.**
`pmacs.lsp._after_scan_for_tests` is a production hook, nil in normal
operation, that P2's witness requires: the race is a cancel landing
during one of the scan's suspensions, which no arrangement of real
timing produces on demand. Same device and justification as `git.lua`'s
`_deliver_status`. It is handed the scan result deliberately — a test
that cancels on any *other* scan passes with the fix deleted, because
the loop would break at the post-sleep check and emit nothing anyway.
Answers issue #233. **Scope is D1 and D2 only** — the two bug-shaped
defects. D3 (the polling cost) is named here, deferred with reasons, and
gets its own framing.
## What is and is not a regression
**#232 is not at fault and nothing about it should be reverted.** The
statusline activity indicator it added is *correct*: it renders real
in-flight jobs from `AsyncRuntime::activity_summary`, and the jobs it
names (`sleep 250ms`, `read_dir <path>`) are real. What changed on
2026-08-09 is **visibility**, not behaviour.
The behaviour has been there since `1c25730` (2026-05-19). So the user-
facing report — "the modeline flips several times a second" — is a
three-month-old defect that became observable last week, and the fix
belongs to the watcher, not the indicator.
Recorded plainly because the tempting move is to quiet the indicator,
and that would delete the only instrument that found this.
## Verified against the tree at `0e4c58d`
Every claim below was read or executed this session, not carried from
the issue.
- `FILE_WATCH_INTERVAL_MS = 250` (`lsp.lua:1924`); each watcher is one
`pmacs.async` coroutine looping sleep → `scan_tree`
(`lsp.lua:2060-2097`).
- `scan_tree` builds `rel` from an empty prefix and calls
`matches(rel)`**relative** paths (`lsp.lua:2035-2056`).
- **`walk` recurses into every directory unconditionally.** `matches`
gates only whether an entry is *recorded*. A watcher that can never
match still walks the whole tree every tick.
- `resolve_watcher`'s string branch returns the pattern **unchanged**
with the base guessed from an attached file's directory
(`lsp.lua:2102-2116`).
- `register_file_watchers` ends `file_watchers[skey][reg.id] = recs`
with no cancellation of the outgoing list (`lsp.lua:2132`).
- Job purposes are `format!("sleep {}ms", …)` (`async_runtime.rs:1027`)
and `format!("read_dir {}", …)` (`:1178`).
- The fake LSP registers **one** watcher, a `RelativePattern`
`{ baseUri, pattern: "**/*.txt" }`, id `watch-1`
(`pmacs_fake_lsp.rs:312-331`).
### The glob table, reproduced
Ran the tree's own `expand_braces` / `glob_one_to_pattern` /
`glob_matcher` under LuaJIT. Output matches the issue exactly, compiled
patterns included:
| glob | compiled | `main.go` | `go.mod` | absolute |
|---|---|---|---|---|
| `**/*.{mod,work}` | `^.-[^/]*%.mod$` | false | **true** | true |
| `<abs>/goproj/**/*.{go,…}` | `^/tmp/goproj/.-[^/]*%.go$` | false | false | true |
| `<abs>/rsproj/**/*.rs` | `^/tmp/rsproj/.-[^/]*%.rs$` | false | false | true |
## Two findings the issue does not carry, both of which shape the fix
### F1 — the existing test cannot discriminate this fix, in either direction
`**/*.txt` compiles to `^.-[^/]*%.txt$`, and `.-` spans `/`. Measured:
it matches `a.txt`, `sub/a.txt`, `/base/a.txt` **and**
`/base/sub/a.txt`. So `m4_24_workspace_did_change_watched_files` passes
whether the matching subject is relative or absolute.
The issue says the tested path and the exercised path are disjoint. The
sharper statement is that the existing test is **insensitive**: it
cannot fail for D1 and it cannot confirm D1's fix. New coverage must use
a pattern whose two readings disagree, or it will inherit the same
blindness.
### F2 — the fix cannot simply "match absolute"; the form must be carried
Per LSP, a plain-string glob matches the **absolute** path while a
`RelativePattern`'s pattern is relative to **its base**. Matching
everything absolutely breaks the second. Measured on `*.txt`:
| subject | matches |
|---|---|
| `a.txt` (relative, correct for RelativePattern) | **true** |
| `/base/a.txt` (absolute) | **false** |
`resolve_watcher` returns `(base, pattern)` and **discards which form it
came from**, so both callers below it are already unable to tell. The
fix therefore changes that function's contract — a third return value or
an explicit record field — rather than only changing the subject string
at the match site. A fix that ignores this trades rust-analyzer's six
broken globs for every `RelativePattern` whose pattern does not begin
`**/`.
## D1 — plain-string globs never match
**Consequences, as measured in the issue and confirmed by the table
above:** rust-analyzer is never told about any file change (all six
globs absolute); gopls is told about `go.mod`/`go.work` but never `.go`
sources (only its relative glob matches).
**Fix:** match a plain-string glob against `base .. "/" .. rel`; keep a
`RelativePattern` matched against `rel`. `resolve_watcher` gains the
form in its return, and the record carries it.
The leading `**/` in gopls' relative glob compiles to `.-`, which spans
`/`, so that glob keeps matching under the absolute subject — which is
why one server's working case does not regress.
## D2 — re-registration leaks the previous coroutines
`file_watchers[skey][reg.id] = recs` replaces the record list without
setting `cancelled` or cancelling the in-flight `_sleep`. The old
coroutines poll until the server dies and are unreachable by
`unregister_file_watchers`, which can only see what the table now holds.
**Reachable today**: rust-analyzer registers
`workspace/didChangeWatchedFiles` **twice under the same id**, six
watchers each, with no intervening unregister — 12 concurrent
coroutines, six permanently uncancellable. The issue's 44.1/s dir-open
rate against a ~270 ms period implies 12 watchers, so the leak is
measured from outside the process, not only read from the source.
**Fix:** cancel the outgoing list before replacing it, with the same
treatment `unregister_file_watchers` already applies.
## What this lane does NOT fix, stated so the report is not mistaken for closed
**The poll cost survives both fixes.** D1 makes matching correct and D2
halves rust-analyzer's watcher count; neither stops the walk. After this
lane, rust-analyzer still walks the entire tree every 250 ms — six times
per tick instead of twelve — including `.git`, `target` and
`node_modules`, at one async job per directory.
So the modeline will still show activity, at roughly half the rate. **If
the acceptance bar for this lane is "the flipping stops", this lane does
not meet it** and should not be started until D3 is framed. That is a
ruling for the user, not an assumption to make quietly.
**Answered 2026-08-10: the user accepted this scope.** D1 and D2
proceed; the walking is D3's problem, framed separately.
## D3 — deferred, with what was checked
Options named in the issue: coalesce a server's watchers into one scan;
root the scan at the workspace rather than an attached file's directory;
an ignore list; back off when nothing changes; or a real
filesystem-notification primitive.
Checked while framing: **there is no `notify`/inotify dependency in the
tree**, so the last option is a new crate *and* a new Rust primitive
plus its Lua binding — not a small change. There is also **no existing
ignore-list infrastructure** to reuse; `src/project.rs` knows `.git` as
a *marker* name, not as something to skip.
D3 is a `COHERENCE.md` §9 concern — background work with no ownership
model — and §9's own Stage 1 is the indicator that surfaced it.
## Verification
The suite must fail without each fix, which the existing suite cannot
(F1). Planned:
- **A fake-LSP mode registering a plain-string ABSOLUTE glob**, with a
pattern whose relative and absolute readings **disagree** — so the
test fails today and passes after D1.
- **A fake-LSP mode registering a `RelativePattern` whose pattern does
not begin `**/`** (e.g. `*.txt` at the base). This is F2's guard: it
passes today, and fails against a fix that matches everything
absolutely. Without it, the obvious wrong fix is green.
- **A re-registration mode**: the same id twice, no unregister. The
witness is that the superseded watchers **stop**, asserted on
observable polling rather than on internal table shape, since the
defect is precisely that the old records are unreachable.
- Existing `m4_24` kept and expected **unchanged** — it covers the
working branch and its insensitivity is now recorded rather than
mistaken for coverage.
Each new test is mutation-tested against the fix it names.
## Coherence impact (§20)
- **Journey steps**: none added; step 5's editing surface is affected
only in that a correct watcher makes servers see edits they currently
miss.
- **Interaction islands**: none.
- **Config registry**: no new setting. The interval stays a module
constant; making it configurable would offer the user a knob for a
defect rather than a preference, and D3 may remove the poll entirely.
- **Background-work attribution (§9)**: this lane *reduces* unattributed
background work but does not model it. D3 owns that, and the honest
statement is that the indicator worked — it made three months of
invisible churn visible on its first week.
## Gates
`./scripts/gate --acceptance m4_acceptance` plus the touched LSP
acceptance suites; no `--protocol` (no wire change, no
`PROTOCOL_VERSION` bump).

View File

@ -332,106 +332,6 @@ fn main() {
});
write_frame(&mut stdout, &req);
}
// Issue #233 D1: `filewatchabs` registers the same watcher
// as a PLAIN-STRING glob — `<base>/**/*.txt`, the form
// rust-analyzer and gopls actually send. Per LSP it matches
// the file's ABSOLUTE path; its relative reading matches
// nothing, so the mode discriminates the match subject.
("initialized", _) if mode == "filewatchabs" => {
let base = std::env::var("PMACS_FAKE_LSP_WATCH_BASE").unwrap_or_default();
let req = serde_json::json!({
"jsonrpc": "2.0",
"id": 9301,
"method": "client/registerCapability",
"params": { "registrations": [{
"id": "watch-abs",
"method": "workspace/didChangeWatchedFiles",
"registerOptions": { "watchers": [{
"globPattern": format!("{base}/**/*.txt"),
"kind": 7
}] }
}] }
});
write_frame(&mut stdout, &req);
}
// Issue #233 review P1 guard: `filewatchbare` registers a
// BARE STRING with no base and no leading `/` — `*.txt`.
// The string arm and the `filewatchflat` arm below carry the
// same pattern deliberately: `flat` proves a
// RelativePattern stays relative, and this proves the
// classification is read from THE PATTERN rather than from
// the union arm it arrived in. The first fix for #233
// called every string absolute, which matched this against
// `<base>/foo.txt` and broke a case that had worked since
// May. Without this mode that regression is invisible.
("initialized", _) if mode == "filewatchbare" => {
let req = serde_json::json!({
"jsonrpc": "2.0",
"id": 9304,
"method": "client/registerCapability",
"params": { "registrations": [{
"id": "watch-bare",
"method": "workspace/didChangeWatchedFiles",
"registerOptions": { "watchers": [{
"globPattern": "*.txt",
"kind": 7
}] }
}] }
});
write_frame(&mut stdout, &req);
}
// Issue #233 F2 guard: `filewatchflat` registers a
// RelativePattern whose pattern has no leading `**/`
// (`*.txt` at the base). It matches base-level files
// RELATIVELY and no absolute path at all, so a fix that
// matches every form against the absolute path goes red.
("initialized", _) if mode == "filewatchflat" => {
let base = std::env::var("PMACS_FAKE_LSP_WATCH_BASE").unwrap_or_default();
let req = serde_json::json!({
"jsonrpc": "2.0",
"id": 9302,
"method": "client/registerCapability",
"params": { "registrations": [{
"id": "watch-flat",
"method": "workspace/didChangeWatchedFiles",
"registerOptions": { "watchers": [{
"globPattern": {
"baseUri": format!("file://{base}"),
"pattern": "*.txt"
},
"kind": 7
}] }
}] }
});
write_frame(&mut stdout, &req);
}
// Issue #233 D2: `filewatchrereg` registers the SAME id
// twice with no unregister between — `**/*.old` then
// `**/*.new` — exactly rust-analyzer's shape. The second
// registration must supersede the first: only `.new`
// events may ever reach `.received`.
("initialized", _) if mode == "filewatchrereg" => {
let base = std::env::var("PMACS_FAKE_LSP_WATCH_BASE").unwrap_or_default();
for (rid, pattern) in [(9303, "**/*.old"), (9304, "**/*.new")] {
let req = serde_json::json!({
"jsonrpc": "2.0",
"id": rid,
"method": "client/registerCapability",
"params": { "registrations": [{
"id": "watch-re",
"method": "workspace/didChangeWatchedFiles",
"registerOptions": { "watchers": [{
"globPattern": {
"baseUri": format!("file://{base}"),
"pattern": pattern
},
"kind": 7
}] }
}] }
});
write_frame(&mut stdout, &req);
}
}
("initialized", _) => {}
// T M4.5: the client's file-watch notifications. Append
// `type uri` lines to `<base>/.received` as a test

View File

@ -797,20 +797,6 @@ impl EditorState {
include_str!("../builtin/runtime/linewrap.lua"),
)
.expect("load linewrap builtin chunk");
// Git integration Stage 1 (docs/git-integration-framing.md):
// `*git-status*` and `*git-diff*`. Loaded after `listview.lua`,
// whose `open` (and whose new optional `keys` table) it drives,
// and after `window.lua`, which owns `window.panel-height` — the
// setting a `display = "panel"` listview resolves. It binds no
// global key: an opening chord is a command-surface decision and
// the framing did not make one, so the entry point is
// `M-x git.status`.
lua_host
.eval(
Some("@pmacs/builtin/runtime/git.lua"),
include_str!("../builtin/runtime/git.lua"),
)
.expect("load git builtin chunk");
// T M7.11 bundled-package bootstrap. Through M7.10 the REPL
// was loaded directly via `eval(include_str!(...))`; the
// M7.11 deliverable migrates it to the package system so it

File diff suppressed because it is too large Load Diff

View File

@ -5351,416 +5351,6 @@ fn m4_24_workspace_did_change_watched_files() {
);
}
/// Issue #233 D1 — a PLAIN-STRING `GlobPattern` matches the file's
/// ABSOLUTE path (LSP 3.17), not the walk's relative path. The
/// `filewatchabs` fake registers `<base>/**/*.txt` as a bare string —
/// the form rust-analyzer and gopls actually send. Its relative
/// reading matches nothing (an anchored `^<base>/…` can never match
/// `foo.txt`), so before the fix no event could ever be reported.
/// The watcher's base is guessed from the attached file's directory —
/// the tempdir here, and the production path for bare-string globs.
#[test]
fn m4_24_plain_string_glob_matches_absolute_path() {
use pmacs::editor::EditorState;
let dir = tempfile::tempdir().expect("tempdir");
let base = dir.path().to_path_buf();
let base_disp = base.display().to_string();
let a_path = base.join("a.rs");
std::fs::write(&a_path, b"fn a() {}\n").expect("write a");
let a_disp = a_path.display().to_string();
let received = base.join(".received");
let foo_uri = format!("file://{}", base.join("foo.txt").display());
let mut state = EditorState::new_with_roots(&crate::iso::roots());
let fake = fake_lsp_path();
state
.lua_host
.lua()
.load(format!(
"pmacs.lsp.config.rust = {{ command = '{fake}',
env = {{ PMACS_FAKE_LSP_MODE = 'filewatchabs',
PMACS_FAKE_LSP_WATCH_BASE = '{base_disp}' }} }}"
))
.exec()
.expect("override rust config");
state
.lua_host
.lua()
.load(format!("pmacs.buffer.find_or_open('{a_disp}')"))
.exec()
.expect("open a.rs");
assert!(
pump_lua_flag(
&mut state,
"(function() for _,r in ipairs(pmacs.lsp.list()) do \
if r.state and r.state.kind=='initialized' then return true end \
end return false end)()",
5,
),
"fake never initialized"
);
// Same warm-up as m4_24: let registerCapability land and the
// watcher take its empty baseline before files appear.
let warm = Instant::now() + Duration::from_millis(900);
while Instant::now() < warm {
state.tick_processes();
state.tick_lsp();
state.tick_async();
std::thread::sleep(Duration::from_millis(15));
}
std::fs::write(base.join("foo.txt"), b"one\n").expect("write foo.txt");
std::fs::write(base.join("bar.md"), b"md\n").expect("write bar.md");
assert!(
pump_until_file_contains(&mut state, &received, &format!("1 {foo_uri}"), 6),
"CREATED for foo.txt never reported under a plain-string glob; \
.received = {:?}",
std::fs::read_to_string(&received).unwrap_or_default()
);
assert!(
!std::fs::read_to_string(&received)
.unwrap_or_default()
.contains("bar.md"),
"non-matching .md must be filtered out"
);
}
/// Issue #233 F2 guard — a `RelativePattern` stays relative to its
/// base. The `filewatchflat` fake registers `{ baseUri, pattern =
/// "*.txt" }`, whose pattern has no leading `**/`: it matches
/// base-level files RELATIVELY and cannot match any absolute path
/// (`[^/]*` spans no `/`). Green before and after D1's fix; red
/// against the obvious wrong fix that matches every form absolutely.
/// `sub/nested.txt` pins the other half of the same contract: a
/// base-level pattern must not match into subdirectories.
#[test]
fn m4_24_relative_pattern_without_globstar_stays_relative() {
use pmacs::editor::EditorState;
let dir = tempfile::tempdir().expect("tempdir");
let base = dir.path().to_path_buf();
let base_disp = base.display().to_string();
let a_path = base.join("a.rs");
std::fs::write(&a_path, b"fn a() {}\n").expect("write a");
let a_disp = a_path.display().to_string();
let received = base.join(".received");
let foo_uri = format!("file://{}", base.join("foo.txt").display());
std::fs::create_dir(base.join("sub")).expect("mkdir sub");
let mut state = EditorState::new_with_roots(&crate::iso::roots());
let fake = fake_lsp_path();
state
.lua_host
.lua()
.load(format!(
"pmacs.lsp.config.rust = {{ command = '{fake}',
env = {{ PMACS_FAKE_LSP_MODE = 'filewatchflat',
PMACS_FAKE_LSP_WATCH_BASE = '{base_disp}' }} }}"
))
.exec()
.expect("override rust config");
state
.lua_host
.lua()
.load(format!("pmacs.buffer.find_or_open('{a_disp}')"))
.exec()
.expect("open a.rs");
assert!(
pump_lua_flag(
&mut state,
"(function() for _,r in ipairs(pmacs.lsp.list()) do \
if r.state and r.state.kind=='initialized' then return true end \
end return false end)()",
5,
),
"fake never initialized"
);
let warm = Instant::now() + Duration::from_millis(900);
while Instant::now() < warm {
state.tick_processes();
state.tick_lsp();
state.tick_async();
std::thread::sleep(Duration::from_millis(15));
}
// nested.txt is written BEFORE foo.txt, so a watcher that wrongly
// matched it would report it no later than foo.txt's event — the
// negative assertion after the positive one is race-free.
std::fs::write(base.join("sub").join("nested.txt"), b"deep\n").expect("write nested.txt");
std::fs::write(base.join("foo.txt"), b"one\n").expect("write foo.txt");
assert!(
pump_until_file_contains(&mut state, &received, &format!("1 {foo_uri}"), 6),
"CREATED for base-level foo.txt never reported under a \
RelativePattern without `**/`; .received = {:?}",
std::fs::read_to_string(&received).unwrap_or_default()
);
assert!(
!std::fs::read_to_string(&received)
.unwrap_or_default()
.contains("nested.txt"),
"a base-level `*.txt` RelativePattern must not match into \
subdirectories"
);
}
/// Issue #233 review P2 — a scan that completes AFTER cancellation
/// must not emit.
///
/// `scan_tree` awaits `read_dir` once per directory, so the watcher
/// coroutine spends most of a tick suspended with `_sleep` already
/// cleared. A cancel arriving there — re-registration or unregistration
/// — sets `cancelled` and has no sleep to interrupt, so before the fix
/// the resumed scan ran on and emitted one last batch under the
/// superseded pattern.
///
/// No arrangement of real timing produces that interleaving on demand,
/// so it is driven through `pmacs.lsp._after_scan_for_tests`, the same
/// device `git.lua` uses for out-of-order completions. The hook is
/// handed the scan result and cancels **only on the scan that observed
/// `foo.txt`** — cancelling on any other scan would pass with the fix
/// deleted, because the loop would break at the post-sleep check and
/// emit nothing regardless.
#[test]
fn m4_24_a_scan_finishing_after_cancellation_emits_nothing() {
use pmacs::editor::EditorState;
let dir = tempfile::tempdir().expect("tempdir");
let base = dir.path().to_path_buf();
let base_disp = base.display().to_string();
let a_path = base.join("a.rs");
std::fs::write(&a_path, b"fn a() {}\n").expect("write a");
let a_disp = a_path.display().to_string();
let received = base.join(".received");
let mut state = EditorState::new_with_roots(&crate::iso::roots());
let fake = fake_lsp_path();
state
.lua_host
.lua()
.load(format!(
"pmacs.lsp.config.rust = {{ command = '{fake}',
env = {{ PMACS_FAKE_LSP_MODE = 'filewatch',
PMACS_FAKE_LSP_WATCH_BASE = '{base_disp}' }} }}"
))
.exec()
.expect("override rust config");
state
.lua_host
.lua()
.load(format!("pmacs.buffer.find_or_open('{a_disp}')"))
.exec()
.expect("open a.rs");
assert!(
pump_lua_flag(
&mut state,
"(function() for _,r in ipairs(pmacs.lsp.list()) do \
if r.state and r.state.kind=='initialized' then return true end \
end return false end)()",
5,
),
"fake never initialized"
);
// Armed BEFORE the file exists, so the cancel cannot land early:
// the hook fires on every scan and only cancels once the scan it is
// inspecting actually contains foo.txt.
state
.lua_host
.lua()
.load(
"pmacs.lsp._after_scan_for_tests = function(record, cur)
if cur and cur['foo.txt'] then record.cancelled = true end
end",
)
.exec()
.expect("install scan hook");
let warm = Instant::now() + Duration::from_millis(900);
while Instant::now() < warm {
state.tick_processes();
state.tick_lsp();
state.tick_async();
std::thread::sleep(Duration::from_millis(15));
}
std::fs::write(base.join("foo.txt"), b"one\n").expect("write foo.txt");
let deadline = Instant::now() + Duration::from_secs(4);
while Instant::now() < deadline {
state.tick_processes();
state.tick_lsp();
state.tick_async();
std::thread::sleep(Duration::from_millis(15));
}
let got = std::fs::read_to_string(&received).unwrap_or_default();
assert!(
!got.contains("foo.txt"),
"a watcher cancelled during its scan emitted a stale batch \
anyway; .received = {got:?}"
);
}
/// Issue #233 review P1 — a BARE-STRING glob with no leading `/` is a
/// relative pattern and must stay one.
///
/// The first fix for #233 classified every string-arm pattern as
/// absolute, so `*.txt` was matched against `<base>/foo.txt` and could
/// never fire — silently breaking a case that had worked since May
/// while fixing the absolute one. `m4_24_relative_pattern_without_globstar_stays_relative`
/// does not cover it: that mode sends the `RelativePattern` OBJECT form,
/// so it constrains the object arm only. This sends the same pattern
/// through the STRING arm, which is the arm the regression lived in.
#[test]
fn m4_24_bare_string_glob_stays_relative() {
use pmacs::editor::EditorState;
let dir = tempfile::tempdir().expect("tempdir");
let base = dir.path().to_path_buf();
let base_disp = base.display().to_string();
let a_path = base.join("a.rs");
std::fs::write(&a_path, b"fn a() {}\n").expect("write a");
let a_disp = a_path.display().to_string();
let received = base.join(".received");
let foo_uri = format!("file://{}", base.join("foo.txt").display());
let mut state = EditorState::new_with_roots(&crate::iso::roots());
let fake = fake_lsp_path();
state
.lua_host
.lua()
.load(format!(
"pmacs.lsp.config.rust = {{ command = '{fake}',
env = {{ PMACS_FAKE_LSP_MODE = 'filewatchbare',
PMACS_FAKE_LSP_WATCH_BASE = '{base_disp}' }} }}"
))
.exec()
.expect("override rust config");
state
.lua_host
.lua()
.load(format!("pmacs.buffer.find_or_open('{a_disp}')"))
.exec()
.expect("open a.rs");
assert!(
pump_lua_flag(
&mut state,
"(function() for _,r in ipairs(pmacs.lsp.list()) do \
if r.state and r.state.kind=='initialized' then return true end \
end return false end)()",
5,
),
"fake never initialized"
);
let warm = Instant::now() + Duration::from_millis(900);
while Instant::now() < warm {
state.tick_processes();
state.tick_lsp();
state.tick_async();
std::thread::sleep(Duration::from_millis(15));
}
std::fs::write(base.join("foo.txt"), b"one\n").expect("write foo.txt");
assert!(
pump_until_file_contains(&mut state, &received, &format!("1 {foo_uri}"), 6),
"CREATED for foo.txt never reported under a bare-string `*.txt` \
glob the string arm is being classified absolute again; \
.received = {:?}",
std::fs::read_to_string(&received).unwrap_or_default()
);
}
/// Issue #233 D2 — re-registering a live id supersedes it. The
/// `filewatchrereg` fake registers `watch-re` TWICE with no
/// unregister between — `**/*.old`, then `**/*.new` — exactly the
/// shape rust-analyzer sends. The superseded watchers must STOP,
/// asserted on observable polling rather than on table shape (the
/// defect is precisely that the replaced records become unreachable
/// while still polling): `f.old` exists on disk before either `.new`
/// event lands, so a leaked first-registration watcher, polling at
/// the same 250 ms cadence, would have reported it by the time the
/// second `.new` positive arrives.
#[test]
fn m4_24_reregistration_supersedes_previous_watchers() {
use pmacs::editor::EditorState;
let dir = tempfile::tempdir().expect("tempdir");
let base = dir.path().to_path_buf();
let base_disp = base.display().to_string();
let a_path = base.join("a.rs");
std::fs::write(&a_path, b"fn a() {}\n").expect("write a");
let a_disp = a_path.display().to_string();
let received = base.join(".received");
let f_old_uri = format!("file://{}", base.join("f.old").display());
let f_new_uri = format!("file://{}", base.join("f.new").display());
let g_new_uri = format!("file://{}", base.join("g.new").display());
let mut state = EditorState::new_with_roots(&crate::iso::roots());
let fake = fake_lsp_path();
state
.lua_host
.lua()
.load(format!(
"pmacs.lsp.config.rust = {{ command = '{fake}',
env = {{ PMACS_FAKE_LSP_MODE = 'filewatchrereg',
PMACS_FAKE_LSP_WATCH_BASE = '{base_disp}' }} }}"
))
.exec()
.expect("override rust config");
state
.lua_host
.lua()
.load(format!("pmacs.buffer.find_or_open('{a_disp}')"))
.exec()
.expect("open a.rs");
assert!(
pump_lua_flag(
&mut state,
"(function() for _,r in ipairs(pmacs.lsp.list()) do \
if r.state and r.state.kind=='initialized' then return true end \
end return false end)()",
5,
),
"fake never initialized"
);
let warm = Instant::now() + Duration::from_millis(900);
while Instant::now() < warm {
state.tick_processes();
state.tick_lsp();
state.tick_async();
std::thread::sleep(Duration::from_millis(15));
}
std::fs::write(base.join("f.old"), b"old\n").expect("write f.old");
std::fs::write(base.join("f.new"), b"new\n").expect("write f.new");
assert!(
pump_until_file_contains(&mut state, &received, &format!("1 {f_new_uri}"), 6),
"CREATED for f.new never reported by the superseding watcher; \
.received = {:?}",
std::fs::read_to_string(&received).unwrap_or_default()
);
// A second positive puts at least one more full poll cycle between
// f.old appearing on disk and the negative assertion below.
std::fs::write(base.join("g.new"), b"new\n").expect("write g.new");
assert!(
pump_until_file_contains(&mut state, &received, &format!("1 {g_new_uri}"), 6),
"CREATED for g.new never reported by the superseding watcher"
);
assert!(
!std::fs::read_to_string(&received)
.unwrap_or_default()
.contains(&f_old_uri),
"the superseded `**/*.old` watcher is still polling after \
re-registration under the same id; .received = {:?}",
std::fs::read_to_string(&received).unwrap_or_default()
);
}
/// Tier 1 single-binary language servers ship pre-configured in the
/// default bundle. Binary-independent: we don't spawn anything, just
/// assert the `pmacs.lsp.config` tables and the `pmacs.lsp.filetypes`