diff --git a/COHERENCE.md b/COHERENCE.md index dd4a16e..39b3f40 100644 --- a/COHERENCE.md +++ b/COHERENCE.md @@ -219,16 +219,31 @@ Automatic, background failures are swallowed. The canonical case, hit on **every file open** when a language server is preconfigured but not installed: `Command::spawn` ENOENT propagates up through `LspManager::spawn` and raises in Lua — where `ensure_server` `pcall`s -it and returns nil (`builtin/runtime/lsp.lua:614-626`), and the -`buffer.after-load` hook `pcall`s the whole attach -(`builtin/runtime/lsp.lua:895-897`). Net user-visible result: nothing. -No status message, no `*errors*` entry, no modeline marker (the LSP -segment is gated on an attachment record existing, so absence is -indistinguishable from "unsupported file type"). Working tree-sitter -highlighting **actively masks** the failure — the user sees colored text -and assumes language intelligence is on. Post-crash is the same shape: -`LspEventKind::Crashed` is pushed (`src/lsp.rs:2394`) and no builtin -subscriber surfaces it. +it, and the `buffer.after-load` hook `pcall`s the whole attach. Net +user-visible result used to be nothing: no status message, no `*errors*` +entry, no modeline marker (the LSP segment is gated on an attachment +record existing, so absence was indistinguishable from "unsupported file +type"). Working tree-sitter highlighting **actively masks** the failure — +the user sees colored text and assumes language intelligence is on. + +**Journey Stage 1b-2 answers this specific case** +(`docs/journey-stage1b2-lsp-guidance-framing.md`): the failure is +reported once per `(language, root, command)` with the command, the +language and the errno; `M-x lsp.status` renders a durable `*lsp*` panel; +and the modeline says `LSP:!` instead of nothing. **The asymmetry itself +is not retired** — the rule below still needs adopting site by site, and +`pmacs.error` is still undefined. + +Post-crash is the same shape and is **not** covered: +`LspEventKind::Crashed` is pushed and no builtin subscriber surfaces it. +A server that started and then died is a different failure with a +different message. + +*(Citation note: this paragraph carried three stale line references — +`ensure_server` was cited at `:614-626` when the spawn `pcall` is at +`:658-674`, and the `buffer.after-load` hook at `:895-897` when it is at +`:1019-1021`. Symbols are authoritative per §25; the numbers are dropped +rather than re-pinned.)* This directly contradicts the product thesis (§23): the "without freezing" half is delivered; the "without becoming opaque" half is @@ -258,10 +273,14 @@ wiring must log attributed failure, never discard it. Corollary from the above: report through a channel with a **test that observes it**, or the guard is indistinguishable from the silence it was meant to fix. -**Frequency note (PR #161):** per-root server affinity means the -preconfigured-but-missing-server failure now fires **once per project -root** rather than once per language per session. The silence is -unchanged in kind; it is strictly more frequent. Surfacing it stays +**Frequency note — corrected by Stage 1b-2.** This previously said the +failure fires "once per project root". It did not: `LspManager::spawn` +returns early *before* both `status_tracker.ensure` and +`clients.insert`, so a failed spawn left **no record at all**, +`pmacs.lsp.list()` could not see it, and `ensure_server`'s affinity loop +re-spawned. The real rate was **once per file open** — strictly worse +than recorded, and the reason the fix memoizes the *report* while still +retrying the spawn. Surfacing it stays Priority 1 work with its own framing — it is a user-visible product behavior (what message, where, with what guidance), not a substrate fix to smuggle into an affinity PR. @@ -375,7 +394,7 @@ Full verdict table: | 3 | Open real project | **Works at the CLI** | Journey Stage 1a: `resolve_target_buffer` answers `ResolvedTarget::Directory` before the EISDIR-producing load, and `EditorState::open` / the daemon bootstrap dispatch the `path.open-directory` chain, whose fallback is dired (#165's buffer, reached rather than duplicated). Startup no longer fails: an unreadable directory, a crashed resolver, and a cleared handler all report on the status line and leave the session running. Because the listing is async and the bootstrap is synchronous, the commit runs against a destination captured at request time (`pmacs.window.commit_to`) rather than against the ambient frontend | | 4 | Understand interface | **Partial** | Mode line gives name/modified/L:C/scroll + mode/LSP/terminal segments; but no welcome text (`EditorCore::new` sets `status: String::new()`), no cheat sheet, and `C-h` deletes a word (§18) | | 5 | Edit | **Works** | Full CUA + Emacs keymap in 161 lines (`builtin/keymaps/default.lua`); isearch, query-replace, kill ring, undo/redo, auto-indent/pair/comment, atomic save. Genuinely excellent zero-config | -| 6 | Language intelligence | **Partial** | Rust grammar bundled and auto-attaches; rust-analyzer preconfigured (`builtin/runtime/lsp.lua:44-52`) — but a missing binary fails silently (§1.2) and highlighting masks it. No LSP status command exists to diagnose | +| 6 | Language intelligence | **Partial** | Rust grammar bundled and auto-attaches; rust-analyzer preconfigured (`builtin/runtime/lsp.lua`). **Journey Stage 1b-2 (PR open) ends the silence** for a server that fails to *start*: the status line names the command, language and errno once per `(language, root, command)`; the modeline reads `LSP:!` instead of nothing; and `M-x lsp.status` renders `*lsp*` over the `status_buffer_text()` renderer that had existed since M4.8 with no caller. **Still Partial**, and flips only on merge (§25): a server that starts and then *crashes* is still unsurfaced | | 7 | Find symbol / file | **File: fixed (open by path merged #162; browsing #165). Symbol: works but undiscoverable** | No find-file/dired/picker existed at audit. Now `C-x C-f` opens a known path and `C-x d` / `C-x C-j` browse (flat listing, `dired` mode keymap); `M-.`/`M-?`/`C-c o` still bound but advertised nowhere and server-gated; no workspace-symbol command; `pmacs.index.*` has no UI | | 8 | Open terminal | **Works** | Full PTY with scrollback + modeline segment, bound to `C-c t` and configurable through three registered settings (`terminal.default-profile`, `terminal.scrollback-rows`, `terminal.escape-key`) plus named `pmacs.terminal.profiles` (PR #173), and searchable through `M-x terminal.copy-mode` / `C-c C-t`, which materializes the retained scrollback into an ordinary read-only buffer (Stage 2). Named limitations: `C-c t` is unreachable from *inside* a terminal window, where `C-c` is consumed as the escape — `M-x terminal` still works there; and there is still **no close/kill command**, which is the remaining half of this step's discoverability gap. *Was broken outright on the GPU frontend until the double terminal-layout sync was fixed: the child took a `SIGWINCH` storm at tick cadence, so typing into it was impossible while output still flowed.* | | 9 | Build / test | **Partial** | `M-x compile.run` works, defaults cwd to detected project root, parses Rust `-->` errors — but no keybinding, an **empty first prompt** (`initial = last and last.cmdline or ""`, `builtin/runtime/compile.lua:1134-1138`), and no `cargo build`/`cargo test` suggestion despite `ProjectKind::Cargo` existing (`src/project.rs:77`) | @@ -1541,15 +1560,16 @@ Establish the end-to-end workflow; treat regressions as release blockers. **State: runs to step 5; thin from step 6 (§2). Mostly wiring, and unusually cheap:** directory-argument handling (**done**: Journey Stage 1a); a find-file surface (**done**: #162 open-by-path, #165 -browsing); surfacing the LSP spawn failure with guidance (§1.2); a -compile keybinding + `cargo build`/`test` default from the existing +browsing); surfacing the LSP spawn failure with guidance (**in flight**: +Journey Stage 1b-2, §1.2); a compile keybinding + `cargo build`/`test` default from the existing `ProjectKind::Cargo`; a terminal keybinding (**done**: `C-c t`, #173); a welcome buffer. The journey acceptance suite (§19) is the ratchet that keeps it fixed — it **exists now** (`tests/journey_acceptance.rs`, Stage 1a), seeded with steps 2, 3, and 5. -Journey Stage 1b is the named remainder: the compile binding + Cargo -defaults, LSP spawn guidance, and the welcome buffer. +Journey Stage 1b is the named remainder, and it splits: **1b-1** (the +compile binding + project-kind defaults) and **1b-2** (LSP spawn +guidance) are both in flight; **1b-3**, the welcome buffer, remains. ### Priority 2: Make workspace and location explicit @@ -1620,8 +1640,8 @@ implementation — this list is direction, not commitment): `resolve_target_buffer` unification, the destination-scope substrate, and the first journey acceptance suite. It routes `pmacs .` into #165's dired buffer rather than growing a second directory surface. - **Stage 1b — remaining**: compile defaults, LSP-failure surfacing, - bindings, welcome buffer. + **Stage 1b-1 / 1b-2 — in flight**: compile defaults and bindings; + LSP-failure surfacing. **Stage 1b-3 — remaining**: welcome buffer. 2. **Discovery surface** (P4): the describe/list/where-is command family, M-x rich rows, help unification, help prefix. 3. **Transient keymap layer** (§6): the overlay scope + lifetime @@ -1713,6 +1733,11 @@ available without being imposed. Found during the audit; fix opportunistically, ideally before this document is wired into CLAUDE.md/AGENTS.md as required reading: +- **§1.2's frequency note was wrong**, not merely stale: it recorded the + missing-server failure as firing once per project root when the real + rate was once per file open, because a failed spawn leaves no record + for the affinity loop to find. Corrected in place by Journey Stage + 1b-2, along with three stale line citations in the same paragraph. - `docs/keybindings.md` — every `src/editor.rs` line citation in §3 is stale by ~250–1000 lines despite a "last verified @ `f8096ff` (2026-07-20)" stamp; its shadow list also omits the terminal `C-c` diff --git a/builtin/runtime/lsp.lua b/builtin/runtime/lsp.lua index d23409b..7c636cb 100644 --- a/builtin/runtime/lsp.lua +++ b/builtin/runtime/lsp.lua @@ -613,6 +613,163 @@ end -- only after this module itself successfully spawns a server. local default_servers = {} +-- --------------------------------------------------------------------- +-- Spawn-failure reporting (journey Stage 1b-2; COHERENCE §1.2) +-- --------------------------------------------------------------------- +-- +-- `ensure_server`'s spawn `pcall` used to end in a bare `return nil`: +-- the canonical background failure — a preconfigured server that is not +-- installed — produced no status message, no record, and no modeline +-- marker, while tree-sitter highlighting kept working and masked it. +-- +-- The reporting SHAPE is not new here. `root_resolver_for` and +-- `report_subscriber_error` in this same file already report through +-- `pmacs.editor.set_status` with the `pmacs.error` arm riding along; +-- this finishes that adoption at the site that matters most. + +-- Lua strings are 8-bit clean, so \0 is a separator no language id, +-- URI, or command can contain. +-- +-- The `u` / `n` discriminator is what makes the markerless case +-- expressible at all. `ensure_server`'s affinity key is `key_uri`, which +-- is deliberately **nil** for a file with no project marker (so loose +-- files share one server per language) — and `t[nil]` raises "table +-- index is nil". A bare `key_uri or ""` would instead collide with an +-- empty URI. One function for both tables, so the two encodings cannot +-- drift apart. +local function affinity_key(language, key_uri) + return language .. "\0" .. (key_uri and ("u" .. key_uri) or "n") +end + +local function reported_key(language, key_uri, command) + return affinity_key(language, key_uri) .. "\0" .. tostring(command) +end + +-- Session-scoped and NEVER cleared: has this exact (language, root, +-- command) triple already been named on the status line? The command is +-- part of the key so repointing config at a *different* missing +-- executable is a new failure and reports again. +local reported = {} + +-- Current failure state per affinity key, CLEARED when a spawn for that +-- key succeeds. This is what `lsp.status` renders, so recovery makes it +-- go quiet. Separate from `reported` because one table cannot both +-- dedupe forever and forget on recovery. +local failures = {} + +-- Per-buffer projection of `failures`, keyed by `tostring(buffer)`. +-- +-- It exists so the modeline provider stays a PURE per-buffer lookup: +-- that provider runs for every window on every paint, and deriving an +-- affinity key inside it would invoke `project_root_for` — user root +-- resolvers and project detection — during painting. +-- +-- Each entry carries the affinity key that produced it, so a success can +-- sweep every buffer sharing that key rather than only the one that +-- succeeded, and the path it was recorded against, so a rename or delete +-- can find it the way `attachments_under` finds attachments. +local failed_attachments = {} + +-- Drop one buffer's projection, releasing its removal callback. Safe to +-- call for a buffer that has none. +local function clear_failed_buffer(bkey) + local entry = failed_attachments[bkey] + if not entry then return end + if entry.on_removed then + pcall(function() entry.on_removed:remove() end) + end + failed_attachments[bkey] = nil +end + +-- A spawn for `key` succeeded: forget the failure and every buffer +-- projecting it. +-- +-- Assigning nil to the CURRENT key during `pairs` is explicitly allowed +-- by Lua's `next` contract; adding a key would not be. +local function clear_failure(key) + failures[key] = nil + for bkey, entry in pairs(failed_attachments) do + if entry.key == key then clear_failed_buffer(bkey) end + end +end + +-- Record and (at most once per identity) announce a spawn failure. +-- Returns the affinity key so the caller can project it onto a buffer. +local function report_spawn_failure(language, key_uri, command, err) + local key = affinity_key(language, key_uri) + failures[key] = { + language = language, + command = tostring(command), + error = tostring(err), + root_uri = key_uri, + } + local rkey = reported_key(language, key_uri, command) + if reported[rkey] then return key end + reported[rkey] = true + -- The underlying error names NEITHER the program nor the language: + -- `Command::spawn`'s io::Error becomes "spawn: No such file or + -- directory (os error 2)". So the guidance is composed here, where + -- both are still in scope, in the spirit of `src/main.rs`'s GPU + -- missing-binary message. The errno is passed through verbatim rather + -- than classified — EACCES is not "not installed". + local msg = string.format( + "LSP: %s for %s did not start (%s) — install it or set " .. + "pmacs.lsp.config.%s.command in init.lua. M-x lsp.status for detail.", + tostring(command), language, tostring(err), language) + pcall(pmacs.editor.set_status, msg) + if pmacs.error then pcall(pmacs.error, msg) end + return key +end + +-- Project a failure onto the buffer that hit it, registering the +-- teardown ONCE. `attach_buffer` is reachable more than once for the +-- same buffer, so registering per failed attempt would stack callbacks +-- on one buffer — the same unbounded-registrar leak, moved rather than +-- fixed. +-- +-- Without this registration the table would be bounded by nothing: a +-- killed buffer's entry would outlive it for the session, and no +-- existing cleanup could reach it, because `attachments_under` iterates +-- `attachments` and a failed buffer has no attachment by construction. +local function project_failure(buf, key, path) + local bkey = tostring(buf) + local existing = failed_attachments[bkey] + if existing then + existing.key = key + existing.path = path + return + end + local handle + local ok, h = pcall(pmacs.buffer.on_removed, buf, function() + -- The registry does `callbacks.take(id)` and then iterates an owned + -- list, so the entry is already gone: clear the projection, and do + -- NOT call `remove()` on our own handle from in here. + failed_attachments[bkey] = nil + end) + if ok then handle = h end + failed_attachments[bkey] = { key = key, path = path, on_removed = handle } +end + +--- Current LSP spawn failures, newest-first is not meaningful here so +--- they are returned sorted by language for a stable render. Public +--- getter (per API conventions). +function pmacs.lsp.spawn_failures() + local out = {} + for _, f in pairs(failures) do + out[#out + 1] = { + language = f.language, + command = f.command, + error = f.error, + root_uri = f.root_uri, + } + end + table.sort(out, function(a, b) + if a.language ~= b.language then return a.language < b.language end + return tostring(a.root_uri) < tostring(b.root_uri) + end) + return out +end + local function ensure_server(language, path) local cfg = pmacs.lsp.config[language] if not cfg or not cfg.command then return nil end @@ -651,6 +808,8 @@ local function ensure_server(language, path) and info.root_uri == key_uri then local kind = info.state.kind if kind ~= "crashed" and kind ~= "stopped" then + -- A live server for this key means there is no failure for it. + clear_failure(affinity_key(language, key_uri)) return info.id end end @@ -668,9 +827,13 @@ local function ensure_server(language, path) }) if ok then default_servers[tostring(sid)] = language + clear_failure(affinity_key(language, key_uri)) return sid end - return nil + -- `sid` holds the error on the failing branch. Second return value is + -- the affinity key, so `attach_buffer` can project it onto the buffer + -- without recomputing a root. + return nil, report_spawn_failure(language, key_uri, cfg.command, sid) end -- Internal ownership seam for builtins whose lifecycle follows the @@ -853,8 +1016,17 @@ local function attach_buffer(buf) -- Path resolved before spawn so the server's `rootUri` can be -- derived from the file's project (see `project_root_for`). local path = active_buffer_path() - local sid = ensure_server(language, path) - if not sid then return nil end + local sid, failure_key = ensure_server(language, path) + if not sid then + -- Journey Stage 1b-2: remember WHY there is no attachment, so the + -- modeline can say "failed" instead of rendering nothing — which is + -- indistinguishable from "this file type has no server". + if failure_key then project_failure(buf, failure_key, path) end + return nil + end + -- Any successful resolution clears this buffer's stale projection; + -- `clear_failure` has already swept peers sharing the affinity. + clear_failed_buffer(key) local uri = file_uri_for(path) if not uri then return nil end local rec = { @@ -976,9 +1148,14 @@ pmacs.statusline.register { priority = 0, face = "ui.modeline.lsp", fn = function(ctx) - local rec = attachments[tostring(ctx.buffer)] - if not rec then return nil end - return "LSP:" .. pmacs.lsp.modeline_label(rec.server) + local bkey = tostring(ctx.buffer) + local rec = attachments[bkey] + if rec then return "LSP:" .. pmacs.lsp.modeline_label(rec.server) end + -- Journey Stage 1b-2. A plain map lookup, deliberately: deriving an + -- affinity key here would run root resolvers and project detection + -- once per window per paint. + if failed_attachments[bkey] then return "LSP:!" end + return nil end, } @@ -2767,6 +2944,58 @@ end -- Default commands + keymap entries -------------------------------------- +-- Journey Stage 1b-2. `LspManager::status_buffer_text()` and +-- `last_error` have existed and been exposed to Lua since M4.8, with no +-- production caller and no buffer to render into — several doc comments +-- in `src/lsp.rs` refer to "the `*lsp*` buffer" as though it existed. +-- This is that buffer. +-- +-- Opened through `pmacs.listview.open` rather than hand-rolled, which is +-- what buys owned-handle identity (a foreign `*lsp*` is never adopted), +-- `<2>` collision behaviour, an immutable generated buffer, `q`, and +-- `g`. `on_refresh` is NOT optional: `listview.refresh` early-returns +-- without one, which would leave `g` bound and silently dead. +local function lsp_status_rows() + local rows = {} + local fails = pmacs.lsp.spawn_failures() + if #fails > 0 then + rows[#rows + 1] = { text = string.format("Failed to start (%d):", #fails) } + for _, f in ipairs(fails) do + rows[#rows + 1] = { text = string.format(" %s (%s) — %s", + f.command, f.language, f.error) } + if f.root_uri then + rows[#rows + 1] = { text = " root: " .. f.root_uri } + else + rows[#rows + 1] = { text = " root: (none detected)" } + end + end + rows[#rows + 1] = { text = "" } + end + rows[#rows + 1] = { text = "Servers:" } + local ok, text = pcall(pmacs.lsp.status_buffer_text) + if ok and type(text) == "string" then + for line in (text .. "\n"):gmatch("([^\n]*)\n") do + rows[#rows + 1] = { text = line } + end + else + rows[#rows + 1] = { text = " (status unavailable)" } + end + return rows +end + +pmacs.command.define { + name = "lsp.status", + description = "Show language-server status and start failures in *lsp*.", + fn = function() + pmacs.listview.open { + name = "*lsp*", + header = "LSP status g refresh q quit", + rows = lsp_status_rows(), + on_refresh = lsp_status_rows, + } + end, +} + pmacs.command.define { name = "lsp.go-to-definition", description = "Jump to the definition of the symbol under the cursor (LSP).", @@ -2942,6 +3171,22 @@ local function attachments_under(path) return out end +-- Journey Stage 1b-2: the same query for FAILED buffers, which have no +-- attachment by construction and are therefore invisible to +-- `attachments_under`. Matches on the path the projection was recorded +-- against, exactly as the attachment query matches on the cached +-- `rec.uri` — the buffer's own path has already been rebound by the time +-- `resource.renamed` fires. +local function failed_projections_under(path) + local out = {} + for bkey, entry in pairs(failed_attachments) do + if entry.path and paths_related(entry.path, path) then + out[#out + 1] = bkey + end + end + return out +end + -- How many attributed failures one status line spells out before -- collapsing the rest into a count. local RESOURCE_REPORT_LIMIT = 2 @@ -3003,6 +3248,16 @@ end pmacs.hook.add("resource.renamed", function(old_path, new_path) if type(old_path) ~= "string" or type(new_path) ~= "string" then return end + -- A failed buffer's projection asserts "this buffer's server failed + -- for affinity K". After a rename that is no longer known to hold — + -- the new path may be in a different project, or none — so it is + -- CLEARED rather than re-keyed. Re-keying would assert a failure at a + -- location where none was observed. Clearing degrades to "we no longer + -- know", which renders as no marker; the next attach re-establishes + -- the truth. + for _, bkey in ipairs(failed_projections_under(old_path)) do + clear_failed_buffer(bkey) + end local sink = failure_sink("resource.renamed") for _, hit in ipairs(attachments_under(old_path)) do local key, rec, old_uri = hit.key, hit.rec, hit.rec.uri @@ -3073,6 +3328,11 @@ end) pmacs.hook.add("resource.deleted", function(path) if type(path) ~= "string" then return end + -- Same disposition as rename, for the stronger reason that the path is + -- gone entirely. + for _, bkey in ipairs(failed_projections_under(path)) do + clear_failed_buffer(bkey) + end local sink = failure_sink("resource.deleted") for _, hit in ipairs(attachments_under(path)) do local key, rec = hit.key, hit.rec diff --git a/docs/active-work.md b/docs/active-work.md index ceb4596..c3218b7 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -255,11 +255,10 @@ If it does not, stop and repair the remote/fetch configuration. never been enforced. Any CI job that compiles the `crdt` targets has to fix them first or it will be red on arrival. -## Journey Stage 1b-2 (P1) — FRAMING OPEN, revision 4 +## Journey Stage 1b-2 (P1) — IMPLEMENTED, PR OPEN - **Branch `journey-stage1b2-lsp-guidance`**, worktree `../pmacs-journey-1b2`, based on `githubsucks/main` @ `fbcf235`. - **Framing only; no code, no PR yet.** `docs/journey-stage1b2-lsp-guidance-framing.md` revision 4, three review rounds closed (round 1: two blocking, three major, one minor; round 2: two blocking, two cleanups; round 3: one blocking; all @@ -343,6 +342,18 @@ If it does not, stop and repair the remote/fetch configuration. (`:614-626` → `:658-674`; `:895-897` → `:1019-1021`; the frequency note; and the now-false implication that no background failure is reported anywhere). +- **ON MERGE, flip the step-6 grade.** `COHERENCE.md` §2's step-6 row + stays **Partial** while the PR is open, per §25's landed-evidence + rule, and says so in the row itself. §20 Priority 1 and the arc list + say "in flight". Same obligation shape as 1b-1's. +- **Bites, all directed.** Full revert fails 14 of 16 pins; removing the + modeline branch fails 6; removing the sweep fails **exactly one** — + the shared-affinity pin written for round 2's blocking finding, which + confirms no other pin covers it; removing the rename/delete + disposition fails exactly those two; removing the dedupe memo fails + the two dedupe pins; and keying the memo on the resolved root (the + revision-1 design) fails **only** the markerless pin, which is the + case where root and affinity key differ. - Recovery from a clean checkout — **the two-argument form does not work** (`git worktree add ` fails with `fatal: invalid reference`): diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index d7e4d2b..16cebc7 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -257,6 +257,45 @@ commands, read `docs/active-work.md` immediately after this file. disagree — and it still establishes no identity, because it is read inside the same read-then-act window and no portable mechanism closes that for a *group* (`pidfd` covers a process; macOS has neither). +- **Journey arc (P1) — Stage 1b-2 IMPLEMENTED, PR open** + (`docs/journey-stage1b2-lsp-guidance-framing.md`, rev 4, three review + rounds). `COHERENCE.md` §1.2's canonical silence: a preconfigured + language server that is not installed now reports with guidance, marks + the modeline `LSP:!`, and appears in `M-x lsp.status`. Per §25 the + step-6 grade flips only on merge. + - **`status_buffer_text()` had existed since M4.8, exposed to Lua and + tested, with no production caller and no `*lsp*` buffer** — several + `src/lsp.rs` and `src/project.rs` doc comments referred to that + buffer as though it existed. Half the stage was wiring dark matter. + - **The reporting shape was already adopted twice in `lsp.lua`** (root + resolvers, notification subscribers). The canonical case was silent + because nobody had converted it, not for want of a mechanism. + - **A failed spawn leaves NO record**: `LspManager::spawn` returns + early before both `status_tracker.ensure` and `clients.insert`, so + `pmacs.lsp.list()` cannot see it and the affinity loop re-spawns. + The failure therefore recurs **once per file open**, not once per + project root as COHERENCE recorded. Hence: **memoize the report, not + the failure** — the spawn is still retried, so installing the binary + mid-session recovers with nothing to invalidate. + - **The affinity key is `(language, key_uri)` and `key_uri` is nil for + markerless files**, which deliberately share one server per + language. Lua cannot index by nil, so one encoding function serves + both tables with a `u`/`n` discriminator no URI can collide with. + - **Three tables, three lifetimes.** `reported` (never cleared, + includes the command so repointing at another missing executable + re-reports); `failures` (cleared on a successful spawn for that + key); and a **buffer-keyed projection** for the modeline, because + that provider runs for every window on every paint and deriving an + affinity key inside it would invoke root resolvers during painting. + - **A success must SWEEP the projections, not clear one.** Clearing + only the succeeding buffer leaves an earlier buffer rendering + `LSP:!` while `lsp.status` reports nothing wrong. + - **A new per-buffer table needs its own teardown.** Nothing existing + reaches it: the LSP resource reconciliation iterates `attachments`, + and a failed buffer has none by construction. `pmacs.buffer.on_removed` + is registered once per projection; rename and delete **clear** rather + than re-key, because after a rename the failure is no longer known to + apply at the new location. - **Reap-ledger silent failures — DIAGNOSTIC, in flight** (`docs/reap-ledger-silent-failures-framing.md`). The lane #200's framing §5 parked and its evidence unparked. **Four `kill(2)` results diff --git a/tests/journey_acceptance.rs b/tests/journey_acceptance.rs index 7ab36c0..416d276 100644 --- a/tests/journey_acceptance.rs +++ b/tests/journey_acceptance.rs @@ -1297,3 +1297,90 @@ fn preservation_display_file_still_refuses_a_directory() { "a refused display_file must not create a buffer" ); } + +// --------------------------------------------------------------------------- +// Step 6 — receive language intelligence (Journey Stage 1b-2) +// +// `COHERENCE.md` §2 graded this **Partial**: a preconfigured server that +// is not installed failed silently, and working tree-sitter highlighting +// masked it. This row pins the failure being *told*, end to end, through +// the same walk a user takes. +// --------------------------------------------------------------------------- + +/// The `lsp` modeline segment for the active buffer, found by face — +/// `EvaluatedStatuslineSegment` carries `provider_id`, not the +/// registration's name. +fn lsp_segment(s: &EditorState) -> Option { + let outcome = pmacs::statusline::evaluate_statusline( + s.lua_host.lua(), + &s.core, + &s.statusline_registry, + pmacs::statusline::StatuslineEvaluationTarget::Grid { + frontend_id: FrontendId::LOCAL, + }, + ); + let pmacs::statusline::StatuslineEvaluationOutcome::Ready(windows) = outcome.outcome else { + return None; + }; + windows + .into_iter() + .flat_map(|w| w.right) + .find(|seg| seg.face == "ui.modeline.lsp") + .map(|seg| seg.text) +} + +/// **N** — step 6: when language intelligence cannot start, the user is +/// told, on the path `pmacs .` actually takes. +/// +/// The configured command is one that cannot exist, so a developer with +/// `rust-analyzer` installed gets the same result as CI — and the +/// fixture asserts that precondition, or every assertion here would be +/// vacuous. +#[test] +fn journey_step6_a_missing_language_server_is_reported_not_swallowed() { + let td = tempfile::tempdir().expect("tempdir"); + std::fs::write(td.path().join("Cargo.toml"), b"[package]\nname=\"x\"\n").expect("write toml"); + std::fs::write(td.path().join("main.rs"), b"fn main() {}\n").expect("write rs"); + + let absent = td.path().join("no-such-bin").join("rust-analyzer"); + assert!( + !absent.exists(), + "fixture precondition: the configured server must not exist" + ); + + // Launch as `pmacs .` does — this lists the directory in dired. + let mut s = launch(td.path()); + // `launch` clears `pmacs.lsp.config`, so configure after it. + exec( + &s, + &format!( + "pmacs.project.set_search_boundary({:?}) + pmacs.lsp.config.rust = {{ command = {:?} }}", + td.path().display().to_string(), + absent.display().to_string() + ), + ); + + // Visit the source file with the real key, as step 5 does. + let line = line_of(&s, "main.rs"); + exec(&s, &format!("pmacs.editor.move_to_line({line})")); + press(&mut s, KeyCode::Enter); + pump(&mut s); + assert_eq!( + active_name(&s), + td.path().join("main.rs").display().to_string(), + "precondition: the walk must actually open the source file" + ); + + let msg = status(&s); + assert!( + msg.contains(&absent.display().to_string()), + "the user is told which command did not start; got {msg:?}" + ); + assert_eq!( + lsp_segment(&s).as_deref(), + Some("LSP:!"), + "and the modeline says so, rather than rendering nothing — which \ + is what made highlighting able to mask this" + ); +} diff --git a/tests/lsp_spawn_guidance_acceptance.rs b/tests/lsp_spawn_guidance_acceptance.rs new file mode 100644 index 0000000..fd48ace --- /dev/null +++ b/tests/lsp_spawn_guidance_acceptance.rs @@ -0,0 +1,567 @@ +// tests/lsp_spawn_guidance_acceptance.rs --- journey Stage 1b-2. + +//! `COHERENCE.md` §1.2's canonical silence: a preconfigured language +//! server that is not installed used to fail with no status message, no +//! record, and no modeline marker, while tree-sitter highlighting kept +//! working and masked it. +//! +//! Pins are labelled **N** (new behaviour, must fail on full revert) or +//! **P** (preservation, falsified by a named targeted mutation), per +//! `docs/journey-stage1b2-lsp-guidance-framing.md` §4. +//! +//! **Every fixture points the config at a path that does not exist**, +//! rather than relying on a real server's absence — a developer with +//! `rust-analyzer` installed must get the same result as CI. Each +//! fixture asserts that precondition, because a fixture that +//! accidentally named a real binary would make every absence assertion +//! here vacuous. + +use std::path::{Path, PathBuf}; + +use pmacs::editor::EditorState; +use pmacs::protocol::FrontendId; +use pmacs::statusline::{ + StatuslineEvaluationOutcome, StatuslineEvaluationTarget, evaluate_statusline, +}; +use tempfile::TempDir; + +fn exec(state: &EditorState, source: &str) { + state.lua_host.lua().load(source.to_owned()).exec().unwrap(); +} + +fn eval(state: &EditorState, source: &str) -> T { + state.lua_host.lua().load(source.to_owned()).eval().unwrap() +} + +fn status(state: &EditorState) -> String { + state.core.borrow().status.clone() +} + +fn lua_str(s: &str) -> String { + format!("{s:?}") +} + +/// A command path that cannot exist. Asserted, not assumed: if this ever +/// resolved, every "did not start" assertion below would be vacuous. +fn absent_command(dir: &Path, name: &str) -> String { + let p = dir.join("no-such-bin").join(name); + assert!( + !p.exists(), + "fixture precondition: {} must not exist", + p.display() + ); + p.display().to_string() +} + +/// Point the default `rust` server at `command`. +fn configure_rust(state: &EditorState, command: &str) { + exec( + state, + &format!( + "pmacs.lsp.config.rust = {{ command = {} }}", + lua_str(command) + ), + ); +} + +/// A Cargo project, so `key_uri` is non-nil (root detected by marker). +fn cargo_project() -> TempDir { + let td = tempfile::tempdir().expect("tempdir"); + std::fs::write(td.path().join("Cargo.toml"), b"[package]\nname=\"x\"\n").expect("write"); + td +} + +fn write_rs(dir: &Path, name: &str) -> PathBuf { + let p = dir.join(name); + std::fs::write(&p, b"fn main() {}\n").expect("write rs"); + p +} + +/// Open a file the way `buffer.after-load` sees it. +fn open(state: &EditorState, path: &Path) { + exec( + state, + &format!( + "pmacs.buffer.find_or_open({})", + lua_str(&path.display().to_string()) + ), + ); +} + +fn editor_for(dir: &Path) -> EditorState { + let state = EditorState::new(); + // Clamp detection so a stray marker above the tempdir cannot leak in. + exec( + &state, + &format!( + "pmacs.project.set_search_boundary({})", + lua_str(&dir.display().to_string()) + ), + ); + state +} + +/// The `lsp` modeline segment's text for the active buffer, or `None`. +fn lsp_segment(state: &EditorState) -> Option { + let outcome = evaluate_statusline( + state.lua_host.lua(), + &state.core, + &state.statusline_registry, + StatuslineEvaluationTarget::Grid { + frontend_id: FrontendId::LOCAL, + }, + ); + let StatuslineEvaluationOutcome::Ready(windows) = outcome.outcome else { + return None; + }; + // Found by face rather than by name: `EvaluatedStatuslineSegment` + // carries `provider_id`, not the registration's name, and the face + // is the stable public identity of this segment. + windows + .into_iter() + .flat_map(|w| w.right) + .find(|s| s.face == "ui.modeline.lsp") + .map(|s| s.text) +} + +fn clear_status(state: &EditorState) { + exec(state, "pmacs.editor.set_status('')"); +} + +fn failure_count(state: &EditorState) -> i64 { + eval(state, "return #pmacs.lsp.spawn_failures()") +} + +// --------------------------------------------------------------------------- +// Reporting +// --------------------------------------------------------------------------- + +/// **N** (framing acceptance 2) — the failure is reported, through the +/// real `buffer.after-load` path, naming the command, the language and +/// the underlying error. +#[test] +fn j1b2_a_missing_server_is_reported_with_guidance() { + let td = cargo_project(); + let state = editor_for(td.path()); + let cmd = absent_command(td.path(), "rust-analyzer"); + configure_rust(&state, &cmd); + open(&state, &write_rs(td.path(), "a.rs")); + + let msg = status(&state); + assert!(msg.contains(&cmd), "names the command; got {msg:?}"); + assert!(msg.contains("rust"), "names the language; got {msg:?}"); + assert!( + msg.contains("No such file") || msg.contains("os error 2"), + "passes the underlying error through; got {msg:?}" + ); + assert!( + msg.contains("init.lua"), + "says what the user can do; got {msg:?}" + ); +} + +/// **N** (3) — reported once per `(language, key_uri, command)`. The +/// spawn is still attempted on the second open; only the message is +/// suppressed. +#[test] +fn j1b2_a_repeat_failure_in_the_same_project_is_not_reannounced() { + let td = cargo_project(); + let state = editor_for(td.path()); + configure_rust(&state, &absent_command(td.path(), "rust-analyzer")); + open(&state, &write_rs(td.path(), "a.rs")); + assert!(!status(&state).is_empty(), "first open reports"); + + clear_status(&state); + open(&state, &write_rs(td.path(), "b.rs")); + assert_eq!( + status(&state), + "", + "a second file in the same project must not re-announce" + ); + // The failure is still current — the memo is on the report, not the + // failure — so the surface still knows about it. + assert_eq!(failure_count(&state), 1); +} + +/// **N** (4) — the markerless case shares one memo, because it shares +/// one server. Two loose files in *different* directories both resolve +/// `key_uri = nil`. +/// +/// Falsified by keying the memo on the resolved root, which reports +/// twice. This is the pin where the root and the affinity key differ. +#[test] +fn j1b2_markerless_files_in_different_directories_report_once() { + let td = tempfile::tempdir().expect("tempdir"); + let one = td.path().join("one"); + let two = td.path().join("two"); + std::fs::create_dir_all(&one).expect("mkdir one"); + std::fs::create_dir_all(&two).expect("mkdir two"); + let state = editor_for(td.path()); + configure_rust(&state, &absent_command(td.path(), "rust-analyzer")); + + open(&state, &write_rs(&one, "a.rs")); + assert!(!status(&state).is_empty(), "first markerless open reports"); + + clear_status(&state); + open(&state, &write_rs(&two, "b.rs")); + assert_eq!( + status(&state), + "", + "a markerless file elsewhere shares the same (language, nil) key" + ); + assert_eq!( + failure_count(&state), + 1, + "and therefore one failure, not two" + ); +} + +/// **N** (5) — a genuinely different root reports again. +#[test] +fn j1b2_a_different_project_root_reports_again() { + let outer = tempfile::tempdir().expect("tempdir"); + let a = outer.path().join("a"); + let b = outer.path().join("b"); + std::fs::create_dir_all(&a).expect("mkdir a"); + std::fs::create_dir_all(&b).expect("mkdir b"); + std::fs::write(a.join("Cargo.toml"), b"[package]\nname=\"a\"\n").expect("w"); + std::fs::write(b.join("Cargo.toml"), b"[package]\nname=\"b\"\n").expect("w"); + let state = editor_for(outer.path()); + configure_rust(&state, &absent_command(outer.path(), "rust-analyzer")); + + open(&state, &write_rs(&a, "a.rs")); + clear_status(&state); + open(&state, &write_rs(&b, "b.rs")); + assert!( + !status(&state).is_empty(), + "a different detected root is a different affinity" + ); + assert_eq!(failure_count(&state), 2); +} + +/// **N** (6) — a changed command reports again, because the reported +/// identity includes it. +#[test] +fn j1b2_repointing_at_another_missing_command_reports_again() { + let td = cargo_project(); + let state = editor_for(td.path()); + configure_rust(&state, &absent_command(td.path(), "rust-analyzer")); + open(&state, &write_rs(td.path(), "a.rs")); + + clear_status(&state); + let second = absent_command(td.path(), "rust-analyzer-2"); + configure_rust(&state, &second); + open(&state, &write_rs(td.path(), "b.rs")); + let msg = status(&state); + assert!( + msg.contains(&second), + "a different missing executable is a new failure; got {msg:?}" + ); +} + +// --------------------------------------------------------------------------- +// Recovery +// --------------------------------------------------------------------------- + +/// **N** (7, 8) — the memo is on the report, not the failure: the spawn +/// is retried, so a resolvable command recovers with nothing to +/// invalidate, and both surfaces go quiet. +#[test] +fn j1b2_recovery_clears_the_failure_surface() { + let td = cargo_project(); + let state = editor_for(td.path()); + configure_rust(&state, &absent_command(td.path(), "rust-analyzer")); + open(&state, &write_rs(td.path(), "a.rs")); + assert_eq!(failure_count(&state), 1, "precondition: a failure exists"); + + // `/bin/sh` exists and is spawnable; it is not an LSP server, but + // this pin is about the spawn succeeding, not about initialize. + configure_rust(&state, "/bin/sh"); + open(&state, &write_rs(td.path(), "b.rs")); + assert_eq!( + failure_count(&state), + 0, + "a successful spawn for the same affinity clears the failure" + ); +} + +/// **N** (9) — recovery reaches **every** buffer sharing the affinity, +/// not just the one that succeeded. +/// +/// Asserted on A, deliberately: a version of this pin that checked B +/// passes on the broken implementation, where only the succeeding +/// buffer's projection is cleared and A keeps rendering `LSP:!` while +/// `lsp.status` reports nothing wrong. +#[test] +fn j1b2_recovery_reaches_every_buffer_sharing_the_affinity() { + let td = cargo_project(); + let state = editor_for(td.path()); + configure_rust(&state, &absent_command(td.path(), "rust-analyzer")); + let a = write_rs(td.path(), "a.rs"); + open(&state, &a); + assert_eq!( + lsp_segment(&state).as_deref(), + Some("LSP:!"), + "precondition: A is marked failed" + ); + + configure_rust(&state, "/bin/sh"); + open(&state, &write_rs(td.path(), "b.rs")); + + // Back to A — the buffer that never succeeded itself. + open(&state, &a); + assert_ne!( + lsp_segment(&state).as_deref(), + Some("LSP:!"), + "A must stop claiming a failure that the shared affinity has resolved" + ); +} + +// --------------------------------------------------------------------------- +// Modeline +// --------------------------------------------------------------------------- + +/// **N** (13) — the modeline distinguishes "failed" from "not +/// applicable". Both halves asserted: a pin checking only the failing +/// case passes if the segment renders `!` unconditionally. +#[test] +fn j1b2_the_modeline_distinguishes_failed_from_unsupported() { + let td = cargo_project(); + let state = editor_for(td.path()); + configure_rust(&state, &absent_command(td.path(), "rust-analyzer")); + + open(&state, &write_rs(td.path(), "a.rs")); + assert_eq!( + lsp_segment(&state).as_deref(), + Some("LSP:!"), + "a source file whose server failed says so" + ); + + let txt = td.path().join("notes.txt"); + std::fs::write(&txt, b"plain\n").expect("write txt"); + open(&state, &txt); + assert_eq!( + lsp_segment(&state), + None, + "a file with no configured server renders nothing at all" + ); +} + +// --------------------------------------------------------------------------- +// Lifecycle +// --------------------------------------------------------------------------- + +/// **N** (14) — a killed buffer's projection is removed. +/// +/// Without the `pmacs.buffer.on_removed` registration the entry outlives +/// its buffer for the session, and nothing else can reach it: the LSP +/// resource reconciliation iterates `attachments`, and a failed buffer +/// has none by construction. +#[test] +fn j1b2_a_killed_buffer_drops_its_failure_projection() { + let td = cargo_project(); + let state = editor_for(td.path()); + configure_rust(&state, &absent_command(td.path(), "rust-analyzer")); + let a = write_rs(td.path(), "a.rs"); + open(&state, &a); + assert_eq!(lsp_segment(&state).as_deref(), Some("LSP:!")); + + let before: i64 = eval(&state, "return #pmacs.buffer.list()"); + exec(&state, "pmacs.buffer.remove(pmacs.window.buffer())"); + let after: i64 = eval(&state, "return #pmacs.buffer.list()"); + assert!( + after < before, + "precondition: the buffer really was removed" + ); + + // Re-open the same path: a fresh buffer that has never failed must + // not inherit a marker, and the stale projection must not be what + // answers for it. + open(&state, &a); + configure_rust(&state, "/bin/sh"); + open(&state, &write_rs(td.path(), "b.rs")); + assert_eq!( + failure_count(&state), + 0, + "the sweep still terminates and clears with a killed buffer in play" + ); +} + +/// **N** (15) — a rename clears the projection rather than leaving an +/// old-path failure attached to a changed buffer. +#[test] +fn j1b2_a_rename_clears_the_failure_projection() { + let td = cargo_project(); + let state = editor_for(td.path()); + configure_rust(&state, &absent_command(td.path(), "rust-analyzer")); + let a = write_rs(td.path(), "a.rs"); + open(&state, &a); + assert_eq!(lsp_segment(&state).as_deref(), Some("LSP:!")); + + let renamed = td.path().join("renamed.rs"); + exec( + &state, + &format!( + "pmacs.hook.run('resource.renamed', {}, {})", + lua_str(&a.display().to_string()), + lua_str(&renamed.display().to_string()) + ), + ); + assert_ne!( + lsp_segment(&state).as_deref(), + Some("LSP:!"), + "after a rename the projection no longer describes this buffer" + ); +} + +/// **N** (16) — a delete clears it too. +#[test] +fn j1b2_a_delete_clears_the_failure_projection() { + let td = cargo_project(); + let state = editor_for(td.path()); + configure_rust(&state, &absent_command(td.path(), "rust-analyzer")); + let a = write_rs(td.path(), "a.rs"); + open(&state, &a); + assert_eq!(lsp_segment(&state).as_deref(), Some("LSP:!")); + + exec( + &state, + &format!( + "pmacs.hook.run('resource.deleted', {})", + lua_str(&a.display().to_string()) + ), + ); + assert_ne!( + lsp_segment(&state).as_deref(), + Some("LSP:!"), + "a deleted path leaves no failure to project" + ); +} + +// --------------------------------------------------------------------------- +// The *lsp* panel +// --------------------------------------------------------------------------- + +fn named_text(state: &EditorState, name: &str) -> String { + eval( + state, + &format!( + r#" + for _, id in ipairs(pmacs.buffer.list()) do + if pmacs.describe.buffer(id).name == {name:?} then + return id:slice(0, id:len()) + end + end + return "" + "# + ), + ) +} + +/// **N** (10) — `M-x lsp.status` renders both sections. Asserts content +/// produced, not that a buffer exists. +#[test] +fn j1b2_lsp_status_renders_failures_and_servers() { + let td = cargo_project(); + let state = editor_for(td.path()); + let cmd = absent_command(td.path(), "rust-analyzer"); + configure_rust(&state, &cmd); + open(&state, &write_rs(td.path(), "a.rs")); + + exec(&state, "pmacs.command.invoke('lsp.status')"); + let text = named_text(&state, "*lsp*"); + assert!( + text.contains(&cmd), + "the failure section names the command; got:\n{text}" + ); + assert!( + text.contains("Servers:"), + "and `status_buffer_text` still renders beneath it; got:\n{text}" + ); +} + +/// **N** (11) — `g` refreshes. The **reattach is load-bearing**: making +/// the command resolvable changes no state on its own, since `failures` +/// is cleared by a successful spawn. +#[test] +fn j1b2_g_refreshes_the_lsp_panel_after_recovery() { + let td = cargo_project(); + let state = editor_for(td.path()); + let cmd = absent_command(td.path(), "rust-analyzer"); + configure_rust(&state, &cmd); + open(&state, &write_rs(td.path(), "a.rs")); + exec(&state, "pmacs.command.invoke('lsp.status')"); + assert!(named_text(&state, "*lsp*").contains(&cmd)); + + // Resolve AND reattach, then refresh in place. + configure_rust(&state, "/bin/sh"); + open(&state, &write_rs(td.path(), "b.rs")); + exec(&state, "pmacs.command.invoke('lsp.status')"); + exec(&state, "pmacs.command.invoke('listview.refresh')"); + assert!( + !named_text(&state, "*lsp*").contains(&cmd), + "g must re-render, not leave the panel stale" + ); +} + +/// **N** (12) — a foreign `*lsp*` buffer is never adopted. This is +/// `listview.open`'s guarantee, pinned rather than assumed because it is +/// exactly what a hand-rolled panel loses. +#[test] +fn j1b2_a_foreign_lsp_buffer_is_not_adopted() { + let td = cargo_project(); + let state = editor_for(td.path()); + configure_rust(&state, &absent_command(td.path(), "rust-analyzer")); + exec( + &state, + "local b = pmacs.buffer.create('*lsp*') b:insert(0, 'user bytes')", + ); + + exec(&state, "pmacs.command.invoke('lsp.status')"); + assert_eq!( + named_text(&state, "*lsp*"), + "user bytes", + "the user's buffer is untouched" + ); + assert!( + named_text(&state, "*lsp*<2>").contains("Servers:"), + "and the panel opens beside it" + ); +} + +// --------------------------------------------------------------------------- +// Preservation +// --------------------------------------------------------------------------- + +/// **P** (20) — a failed spawn never fabricates an attachment record. +/// Targeted mutation: recording the failure in `attachments`, which +/// would route requests at a server that does not exist. +#[test] +fn j1b2_preservation_a_failed_spawn_creates_no_attachment() { + let td = cargo_project(); + let state = editor_for(td.path()); + configure_rust(&state, &absent_command(td.path(), "rust-analyzer")); + open(&state, &write_rs(td.path(), "a.rs")); + + assert!( + eval::(&state, "return pmacs.lsp.attachment_for_request() == nil"), + "no request may be issued against a server that failed to start" + ); +} + +/// **P** (18) — a spawnable server is unaffected: it attaches, and the +/// modeline reports the server rather than a failure. +#[test] +fn j1b2_preservation_a_spawnable_server_still_attaches() { + let td = cargo_project(); + let state = editor_for(td.path()); + configure_rust(&state, "/bin/sh"); + open(&state, &write_rs(td.path(), "a.rs")); + + let seg = lsp_segment(&state); + assert!( + seg.is_some() && seg.as_deref() != Some("LSP:!"), + "a started server keeps its own label; got {seg:?}" + ); + assert_eq!(failure_count(&state), 0); +}