diff --git a/COHERENCE.md b/COHERENCE.md index 1594b69..954e621 100644 --- a/COHERENCE.md +++ b/COHERENCE.md @@ -120,9 +120,8 @@ asymmetry**, and **per-arc coherence debt**. Coherence-shaped work already in flight at audit time: find-file / dired Stage 0 (`C-x C-f`, PR #162, `docs/dired-framing.md`), bottom -panel Stage 1 (merged #155), multi-root LSP affinity (branch -`lsp-multi-root-affinity`), the config registry foundation (merged -#127). +panel Stage 1 (merged #155), multi-root LSP affinity (PR #161), the +config registry foundation (merged #127). --- @@ -230,9 +229,37 @@ This directly contradicts the product thesis (§23): the "without freezing" half is delivered; the "without becoming opaque" half is currently false for exactly the failures a new user will hit first. +**The reporting channel the runtime believes it has does not exist.** +Fifteen call sites — `async.lua` (5), `syntax.lua` (4), and one each in +`lsp.lua`, `mcp.lua`, `fs.lua`, `editops.lua`, `autosave.lua`, and +`commands/default.lua` — report background failures through +`pmacs.error`, each guarded as `if pmacs.error then pmacs.error(...)`. +**`pmacs.error` is never defined in production** — the only assignment +in the tree is a test stub (`src/editor.rs:9881`), and +`type(pmacs.error)` is `nil` in a fresh `EditorState`. So every one of +those fifteen reports is dead: the guard makes the silence look +deliberate and keeps it from ever being noticed. `pmacs.errors` (plural, +`builtin/runtime/compile.lua:45`) is an unrelated namespace and is not +it. This is the silence asymmetry one level deeper than §1.2 first +recorded — not "the failure isn't surfaced" but "the surface was +written, guarded, and never built." Found while landing PR #161, which +nearly added a sixteenth; that one reports via +`pmacs.editor.set_status` (which exists) with the `pmacs.error` arm +riding along for when the channel is built. + **Rule to adopt:** anything that fails automatically must leave a user-visible trace with a named owner. A `pcall` around background -wiring must log attributed failure, never discard it. +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 +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. ### 1.3 Ground truth: coherence debt compounds per-arc @@ -726,12 +753,14 @@ per-subsystem conventions.** nil/`"."`. Nothing owns the set {roots, servers, terminals, tasks, layout} — which is why desktop-save under a daemon had nothing principled to attach to (Q#DS9, §2 step 12). -- **First slice in flight**: the multi-root LSP server-affinity work - (branch `lsp-multi-root-affinity`) makes *(language, found-root)* the - server identity — the first time a root functions as an identity key - rather than a spawn parameter. Note it is again per-subsystem: LSP - learns roots; compile, search, index, and trust do not share the - object. +- **First slice landed (PR #161)**: the multi-root LSP server-affinity + work makes *(language, found-root)* the server identity — the first + time a root functions as an identity key rather than a spawn + parameter. It also establishes the rule that a *fallback* root (the + file's own directory, when no marker was found) is deliberately **not** + an identity, so markerless files keep sharing one server per language. + Note it is again per-subsystem: LSP learns roots; compile, search, + index, and trust do not share the object. A workspace entity is a **model gap** (real arc), not wiring. It is also the prerequisite that keeps §8 (locations), §9 (task ownership), §11 diff --git a/builtin/runtime/lsp.lua b/builtin/runtime/lsp.lua index 4181156..6021134 100644 --- a/builtin/runtime/lsp.lua +++ b/builtin/runtime/lsp.lua @@ -30,9 +30,13 @@ pmacs.lsp = pmacs.lsp or {} -- env (table) extra environment -- init_options (table) `initializationOptions` -- settings (table) answered to `workspace/configuration` --- root (string) optional explicit project root; overrides +-- root (string|function) optional explicit project root; overrides -- the `pmacs.project.detect` marker walk used --- to set `rootUri`/`cwd` (see project_root_for) +-- to set `rootUri`/`cwd`. A `function(path) -> +-- string|nil` is resolved per file and +-- memoized per directory; returning nil +-- declines and falls through to the marker +-- walk (see project_root_for) pmacs.lsp.config = pmacs.lsp.config or {} -- Default rust-analyzer config. Users replace any field from init.lua @@ -507,34 +511,144 @@ end -- the rest of the editor uses, honoring set_search_boundary, -- 3. the file's own directory (a lone file still gets a sane root -- rather than leaking the editor cwd). --- This is single-root: it fixes which root the one per-language server --- uses, NOT one-server-per-root scoping (still deferred post-v0.1). +-- Returns `root, source`, where `source` is "config", "detected", or +-- "fallback" — and nil alongside a nil root. The source matters because +-- only the first two mean a root was actually *found*; `ensure_server` +-- keys server affinity on those and treats the fallback as rootless. +-- +-- `config[language].root` may be a `function(path) -> string|nil` as +-- well as a plain string, for languages whose root rule the shared +-- marker walk cannot express (an innermost-wins walk cannot find an +-- *outermost* marker). A resolver that returns nil declines, and +-- resolution falls through to the marker walk. +-- +-- **A configured root — string or resolver return — MUST be a canonical +-- absolute path.** The `"detected"` arm is canonicalized for free +-- (`pmacs.project.detect` canonicalizes before walking), but a +-- configured one is fed to `file_uri_for` exactly as written, and the +-- affinity key is that URI. On macOS a resolver returning `/var/…` and +-- a detected `/private/var/…` are different keys for the same +-- directory, which silently yields two servers for one project. There +-- is no Lua-side canonicalizer to normalize this for you. +-- +-- Resolver results are memoized per directory, because `ensure_server` +-- resolves the root on the *reuse* path as well as the spawn path — so +-- an unmemoized filesystem-walking resolver would re-walk on every +-- attach rather than once per project. The memo is keyed by the +-- resolver function itself, weakly: replacing `config[lang].root` +-- installs a new key and the old memo is collected, so a swapped +-- resolver can never serve a root the previous one computed. +local root_resolver_memo = setmetatable({}, { __mode = "k" }) + +local function resolve_root_fn(language, resolver, path) + local dir = dir_of(path) + if not dir then return nil end + local memo = root_resolver_memo[resolver] + if not memo then + memo = {} + root_resolver_memo[resolver] = memo + end + local hit = memo[dir] + -- `false` is the memoized form of "this resolver declined"; nil means + -- "not yet asked", so the two must stay distinguishable. + if hit ~= nil then + return hit or nil + end + local ok, resolved = pcall(resolver, path) + -- COHERENCE §1.2: background wiring must not DISCARD a failure. A + -- resolver that raises, or that returns something other than a string + -- or nil, is a config bug — and the memo below would otherwise bury + -- it permanently for this directory, so it is never observed again. + -- Returning nil is the documented decline and stays silent. + local failure + if not ok then + failure = "raised: " .. tostring(resolved) + elseif resolved ~= nil and type(resolved) ~= "string" then + failure = "returned a " .. type(resolved) .. "; want string or nil" + end + if failure then + local msg = string.format( + "LSP: %s root resolver for %s %s", language, dir, failure) + -- Report on the channel that EXISTS. `pmacs.error` is referenced by + -- fifteen guarded call sites across the runtime and is defined + -- nowhere in production (only by a test stub in `src/editor.rs`), so + -- `if pmacs.error then ...` alone would be a sixteenth report that + -- never fires — the unwired-guard shape, not a fix for it. The + -- status line is what lsp.lua already uses for every other LSP + -- error. The `pmacs.error` arm rides along so this upgrades for free + -- if that channel is ever built. + -- + -- Both reports are pcall'd: a broken reporting channel must not turn + -- a declined root into a failed attach. + pcall(pmacs.editor.set_status, msg) + if pmacs.error then pcall(pmacs.error, msg) end + resolved = nil + end + if type(resolved) ~= "string" then resolved = nil end + memo[dir] = resolved or false + return resolved +end + local function project_root_for(language, path) local cfg = pmacs.lsp.config[language] - if cfg and cfg.root then return cfg.root end - if not path then return nil end + local configured = cfg and cfg.root + -- Truthiness, not `~= nil`: `root = false` has always read as "unset", + -- and a `false` leaking through as a root would reach `file_uri_for`. + if configured and type(configured) ~= "function" then + return configured, "config" + end + if not path then return nil, nil end + if configured then + local resolved = resolve_root_fn(language, configured, path) + if resolved then return resolved, "config" end + end local ok, det = pcall(pmacs.project.detect, path) - if ok and det and det.root then return det.root end - return dir_of(path) + if ok and det and det.root then return det.root, "detected" end + return dir_of(path), "fallback" end local function ensure_server(language, path) local cfg = pmacs.lsp.config[language] if not cfg or not cfg.command then return nil end - -- Reuse an existing same-language server if one is up. Multi-root - -- scoping (one server per project root) ships post-v0.1, so the - -- first file that attaches a given language fixes that server's - -- root; later files of the same language reuse it regardless of - -- their own project (known, documented limitation). + -- Reuse an existing same-language server *serving the same root*. + -- One server per project root: `lake serve` is bound to one Lake + -- package and rust-analyzer/gopls to one workspace, so handing the + -- second project's files to the first project's server yields + -- unresolvable imports and empty diagnostics. + -- + -- The affinity key is the root only when a root was actually FOUND + -- (config override or marker walk). `project_root_for` never returns + -- nil for a file that has a path — its last resort is the file's own + -- directory — so keying on the fallback would give every directory + -- of loose scratch files its own server, for every language: two + -- stray .py files in different directories would spawn two pyrights + -- where today they share one. The fallback therefore keys on nil. + -- + -- Matching is on the spawned spec's `root_uri`, nil matching nil, so + -- the fallback spawn must pass `root_uri = nil` for the key and the + -- stored spec to agree. `cwd` still carries the directory and + -- `build_initialize` derives the identical `rootUri` from it when the + -- field is None (src/lsp.rs), so the initialize payload is unchanged + -- for that case — only what this loop matches on changes. + -- + -- Consequence, deliberate: a server hand-spawned from `init.lua` with + -- only `cwd` set also reads back nil, so a root-bearing attach will + -- not adopt it. We cannot know which root it was meant to serve, and + -- guessing wrongly routes a project's files to the wrong server. + local root, source = project_root_for(language, path) + local key_uri = nil + if source == "config" or source == "detected" then + key_uri = file_uri_for(root) + end for _, info in ipairs(pmacs.lsp.list()) do - if info.language_id == language and info.state then + if info.language_id == language and info.state + and info.root_uri == key_uri then local kind = info.state.kind if kind ~= "crashed" and kind ~= "stopped" then return info.id end end end - local root = project_root_for(language, path) local ok, sid = pcall(pmacs.lsp.spawn, { label = "default-" .. language, language_id = language, @@ -544,7 +658,7 @@ local function ensure_server(language, path) init_options = cfg.init_options, settings = cfg.settings, cwd = root, - root_uri = root and file_uri_for(root) or nil, + root_uri = key_uri, }) if ok then return sid end return nil diff --git a/docs/active-work.md b/docs/active-work.md index 3141a24..9ff342d 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -58,26 +58,46 @@ If it does not, stop and repair the remote/fetch configuration. - Portable branch: `githubsucks/inline-math-slice`; worktree `../pmacs-math-slice`. **PR #158**, base `main`. -- **Canonical `main` @ `8c86d34` merged into the lane** (2026-07-25), - 28 commits behind at the time. Merged rather than rebased, per the - #135/#137 precedent: the PR is awaiting review rounds and a rebase - would break every review anchor. The only conflict was this ledger — - both sides' lanes were kept — and it was **pre-existing**, not - introduced by the dired (#164) or Lean 4 ledger commits; it already - conflicted against `main` @ `e745068`. -- **Integration surface** (derived from `git diff ..main`, - not from another PR's file list): `pmacs-gpu/src/main.rs` gained 72 - lines on main from `e547a90` — the minimap all-blank-slab - divide-by-zero fix — and this lane rewrites large parts of the same - file. Git auto-merged it **textually**; a clean auto-merge is not - evidence the tree compiles (the folding-arc lesson), so the full gate - suite below is what actually discharges it. -- **CI had never run on this branch** — zero workflow runs since the PR - opened on 2026-07-24, while every other open PR has a full 12-check - run. Not a fork and not a trigger-config issue (the workflow fires on - all `pull_request` events); cause unidentified. The integration push - is what gets it its first run, so treat that run as the branch's - first real CI evidence. +- **Canonical `main` merged into the lane twice on 2026-07-25**, both + times merged rather than rebased, per the #135/#137 precedent: the PR + is awaiting review rounds and a rebase would break every review anchor. + - First at `8c86d34` (28 commits behind). The conflict was + **pre-existing**, not introduced by the dired (#164) or Lean 4 + ledger commits; it already conflicted against `main` @ `e745068`. + - Then at `46a1b8f`, after Lean 4 Stage 2 (#161) landed while this + branch's CI was still running. Same single conflict, same shape, + same resolution. +- **Both conflicts were this ledger and nothing else** — both sides' + lanes kept verbatim each time. That is the standing cost of a + long-lived PR here: every merge to `main` edits this file, so a branch + awaiting review re-conflicts on it and only on it. It is a docs + collision, never a code one, and it says nothing about integration + risk — do not read a `CONFLICTING` badge on this PR as a code signal + without checking which file `git merge-tree` names. +- **First integration's surface** (derived from `git diff + ..main`, not from another PR's file list): + `pmacs-gpu/src/main.rs` gained 72 lines on main from `e547a90` — the + minimap all-blank-slab divide-by-zero fix — and this lane rewrites + large parts of the same file. Git auto-merged it **textually**; a + clean auto-merge is not evidence the tree compiles (the folding-arc + lesson), so the full gate suite below is what actually discharges it. +- **Second integration's surface is code-disjoint.** #161 touched + `COHERENCE.md`, `builtin/runtime/lsp.lua`, `src/lua_bindings/mod.rs`, + and a new `tests/lsp_multi_root_acceptance.rs`; intersecting that + against this lane's own changed-file set leaves exactly one entry, + `docs/active-work.md`. No source file is touched by both sides, so + this one carries none of the first integration's semantic risk. +- **CI ran on this branch for the first time on 2026-07-25 and passed + all twelve** (Format, both Lints, GPU Render headless, all four Test + matrix jobs, M1/M4/M5/M6 gates) at `8b457de` — the first-integration + tip. Before that there were zero workflow runs since the PR opened on + 2026-07-24, while every other open PR had a full run; not a fork and + not a trigger-config issue (the workflow fires on all `pull_request` + events), cause never identified. So the green run **validates the + first integration, including the `pmacs-gpu/src/main.rs` auto-merge, + on macOS and Linux both** — the platforms local gating could not + cover. The second integration is not yet CI-covered, but its surface + is the ledger alone. - Framing: `docs/inline-math-slice-framing.md` rev 3, approved after two review rounds; parent arc framing merged as #154. - State: parser, font bundle (GUST licence), MATH-table layout with the @@ -124,10 +144,11 @@ If it does not, stop and repair the remote/fetch configuration. spacer draws its box whole at the first run's origin; the fit budget reads the bundled code face even under a custom `set_font` family (the draw anchors to the real shaped baseline either way). -## Lean 4 lane (Arc 8) — Stage 1 IN REVIEW (PR #160) +## Lean 4 lane (Arc 8) — Stage 1 MERGED; Stage 2 IN REVIEW (PR #161) -- Portable branch: `githubsucks/lean4-stage1`, worked in the shared - checkout (no sibling worktree), based on `githubsucks/main` @ `e745068`. +- Stage 1 **merged as #160** (`main` @ `0827dd1`, 2026-07-25, one review + round, all twelve checks green). Branch `githubsucks/lean4-stage1` + retained; it was worked in the shared checkout (no sibling worktree). - Approved framing: `docs/lean4-mode-framing.md` revision 4, committed as the branch's first commit (`a382965`) after three review rounds. **Seven stages**, 19 decisions (Q#LN1–19), 64 acceptance criteria. North star: @@ -183,16 +204,79 @@ If it does not, stop and repair the remote/fetch configuration. 152; **isolated-config workspace sweep 3,150 across 90 suites**; `git diff --check` clean. The sweep needs an isolated `XDG_CONFIG_HOME` for the reason recorded in the bottom-panel lane below. -- **Stage 2 is multi-root LSP server affinity** — pure substrate, no Lean - content, and it changes `ensure_server`, which every LSP language - shares. It is sequenced next because Lean is the language that makes its - absence a correctness failure rather than an inconvenience. Two - corrections the framing already carries for it: `root` is computed at - `lsp.lua:537`, **after** the reuse loop, so the fix must hoist it; and - `project_root_for` never returns nil for a file with a path, so the - affinity key must be the root only when a root was actually *detected*, - or markerless scratch files fragment into one server per directory for - every language. +### Stage 2 — multi-root LSP server affinity (Q#LN15) + +- Portable branch: `githubsucks/lsp-multi-root-affinity`, shared checkout, + based on `githubsucks/main` @ `0827dd1`. Named for the substrate, not + for Lean: **the diff contains no Lean content**, because `ensure_server` + is the one server-affinity function every LSP language shares and a + cross-cutting change to it must not be reviewable only as a Lean + feature. +- Three files, no protocol change: `src/lua_bindings/mod.rs` (the + `lsp.list()` row builder gains `root_uri` + `cwd`), + `builtin/runtime/lsp.lua` (`project_root_for` returns `root, source`; + `ensure_server` hoists it above the reuse loop and matches on it), + `tests/lsp_multi_root_acceptance.rs` (9 tests, acceptance 13–21). +- **The rule that keeps this from regressing every other language: the + affinity key is the root only when a root was actually FOUND.** + `project_root_for` never returns nil for a file with a path — its last + resort is the file's own directory — so a naive `(language_id, root)` + key gives every directory of loose scratch files its own server, for + every language. `source` is `"config" | "detected" | "fallback"` and + only the first two become a key. +- **Wire-identical for the fallback case, and that is provable rather + than hoped.** Matching is on the spawned spec's `root_uri` (nil matching + nil), so the fallback spawn passes `root_uri = nil`; `cwd` still carries + the directory and `build_initialize` derives the identical `rootUri` + from `cwd` when the field is None, using a percent-encoder with the same + allowed set as Lua's `file_uri_for`. `build_initialize` (`src/lsp.rs`) + is the **only** reader of `spec.root_uri` in the tree. +- Deliberate behavior change, asserted not discovered: a server + hand-spawned from `init.lua` with only `cwd` set also reads back nil, so + a root-bearing attach will not adopt it. +- `config[language].root` may now be a `function(path) -> string|nil`, + memoized per directory — needed because the hoist puts root resolution + on every attach rather than every spawn. The memo is keyed **weakly by + the resolver function itself**, so replacing `config[lang].root` cannot + serve a root the previous resolver computed. This is Q#LN8's + generalization landing early; the Lean resolver that uses it is Stage 3. +- Bite-verified three ways: 5/9 fail against the pre-change `lsp.lua`, + 8/9 against the pre-change `mod.rs`, and — the one that matters most — + installing the naive always-key-on-root variant fails acceptance 20 and + 21 exactly as Q#LN15 part 2 predicts. The four that survive the first + bite (13, 15, 16, 19) are the regression pins; passing on both sides is + their job. +- Every fixture sets `pmacs.project.set_search_boundary` at its own + tempdir root. Without it the marker walk climbs to the filesystem root + and a stray `.git` above the temp directory turns the markerless cases + into detected ones — the assertions would still pass while testing + nothing. +- **Found but not fixed here (pre-existing, own lane):** `ensure_server` + never forwards `cfg.restart` to `pmacs.lsp.spawn`, so a + `restart = "never"` in `pmacs.lsp.config[lang]` is silently dropped on + the auto-attach path. At least one existing test sets it believing it + takes effect. Out of scope for a PR whose acceptance 16 pins existing + attach behavior as unchanged. +- **Review round 1 addressed.** The blocker was process, not design: the + test file was committed *before* `cargo fmt` ran, so the fix sat + uncommitted in the working tree and the branch as pushed failed the + first gate. The reported "fmt clean" described the worktree, not the + branch — gate results are only meaningful when run against the pushed + tree. Also added the two pins review asked for (a **string** `config + .root` as an affinity key — acc17 only covered the function form; and + `root = false` reading as unset), each bite-verified against exactly + the mutation it targets and neither against the other. And documented + the canonicalization obligation: the `"detected"` arm is canonicalized + for free, a **configured** root is not, so on macOS a resolver + returning `/var/…` and a detected `/private/var/…` are different keys + for one directory. Stage 3's Lean resolver is the first real consumer, + so the obligation is written at the point of use. +- Verification on this branch: `cargo fmt --check` clean; strict + workspace Clippy clean; 1,826 default + 2,003 CRDT library tests; + multi-root 11/11; M4 121; statusline 7; completion popup 9; auto-pair + 45; required GPU 155; **isolated-config workspace sweep 3,164 across 91 + suites**; `git diff --check` clean. The sweep needs an isolated + `XDG_CONFIG_HOME` and `-- --skip basedpyright`. ## Dired lane — framing APPROVED; Stage 0 MERGED, Stage 1 next diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 29e3b47..3a92520 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -9923,12 +9923,26 @@ pub fn install_lsp( let ids: Vec = mgr.ids().collect(); let out = lua.create_table_with_capacity(ids.len(), 0)?; for (i, id) in ids.iter().enumerate() { - let row = lua.create_table_with_capacity(0, 5)?; + let row = lua.create_table_with_capacity(0, 7)?; row.set("id", LspServerIdLua(*id))?; if let Some(spec) = mgr.spec(*id) { row.set("label", spec.label.as_str())?; row.set("language_id", spec.language_id.as_str())?; row.set("command", spec.command.as_str())?; + // Server *affinity* fields. `root_uri` is the spec + // field verbatim — deliberately NOT the URI the + // server was initialized with, which `build_initialize` + // derives from `cwd` when the field is `None`. Lua's + // `ensure_server` matches on this exact value, so a + // server that never asked for a specific root must + // read back as nil rather than as its cwd; see the + // affinity-key comment in `builtin/runtime/lsp.lua`. + if let Some(root_uri) = spec.root_uri.as_deref() { + row.set("root_uri", root_uri)?; + } + if let Some(cwd) = spec.cwd.as_deref() { + row.set("cwd", cwd.display().to_string())?; + } } if let Some(state) = mgr.state(*id) { row.set("state", lsp_state_to_lua(lua, state)?)?; diff --git a/tests/lsp_multi_root_acceptance.rs b/tests/lsp_multi_root_acceptance.rs new file mode 100644 index 0000000..39ac68f --- /dev/null +++ b/tests/lsp_multi_root_acceptance.rs @@ -0,0 +1,704 @@ +//! Arc 8 Stage 2 acceptance — multi-root LSP server affinity. +//! +//! `docs/lean4-mode-framing.md` Q#LN15, acceptance 13–21. +//! +//! This suite deliberately contains **no Lean content**. `ensure_server` +//! (`builtin/runtime/lsp.lua`) is the single server-affinity function for +//! every LSP language in pmacs, so the change is exercised through the +//! four languages that already shipped attach paths — rust, python, go, +//! typescript — driven against `pmacs_fake_lsp` so nothing here needs a +//! real toolchain on PATH. +//! +//! Every fixture calls `pmacs.project.set_search_boundary` at its own +//! tempdir root. Without it the marker walk climbs to the filesystem +//! root, and a stray `.git` above the temp directory would silently turn +//! the "markerless" cases into detected ones — the assertions would still +//! pass while testing nothing. + +use std::path::{Path, PathBuf}; +use std::time::Duration; + +use pmacs::editor::EditorState; + +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 fake_lsp_path() -> String { + env!("CARGO_BIN_EXE_pmacs_fake_lsp").to_owned() +} + +/// A fresh editor with the shipped language configs cleared, so the only +/// server any test can spawn is the fake one it configures itself. +fn editor() -> EditorState { + let state = EditorState::new(); + exec(&state, "pmacs.lsp.config = {}"); + state +} + +fn lua_str(path: &Path) -> String { + path.display() + .to_string() + .replace('\\', "\\\\") + .replace('"', "\\\"") +} + +/// Mirror of `file_uri_for` in `builtin/runtime/lsp.lua` and +/// `path_to_file_uri` in `src/lsp.rs`. Reimplemented rather than +/// imported so the test states the expected encoding independently of +/// the code under test. +fn file_uri(path: &Path) -> String { + let mut out = String::from("file://"); + for ch in path.display().to_string().chars() { + match ch { + 'a'..='z' | 'A'..='Z' | '0'..='9' | '/' | '-' | '_' | '.' | '~' | ':' => out.push(ch), + _ => { + use std::fmt::Write as _; + let mut buf = [0u8; 4]; + for byte in ch.encode_utf8(&mut buf).as_bytes() { + let _ = write!(out, "%{byte:02X}"); + } + } + } + } + out +} + +struct Fixture { + _dir: tempfile::TempDir, + root: PathBuf, +} + +impl Fixture { + /// Canonicalized so the expected roots below compare equal to what + /// `pmacs.project.detect` returns (it canonicalizes before walking, + /// which matters on macOS where `/var` is a symlink to `/private/var`). + fn new() -> Self { + let dir = tempfile::tempdir().unwrap(); + let root = std::fs::canonicalize(dir.path()).unwrap(); + Self { _dir: dir, root } + } + + fn write(&self, rel: &str, contents: &str) -> PathBuf { + let path = self.root.join(rel); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(&path, contents).unwrap(); + path + } + + fn dir(&self, rel: &str) -> PathBuf { + self.root.join(rel) + } + + fn bind(&self, state: &EditorState) { + exec( + state, + &format!( + "pmacs.project.set_search_boundary(\"{}\")", + lua_str(&self.root) + ), + ); + } +} + +fn configure(state: &EditorState, language: &str) { + exec( + state, + &format!( + "pmacs.lsp.config.{language} = {{ command = \"{}\" }}", + fake_lsp_path() + ), + ); +} + +fn open(state: &EditorState, path: &Path) { + exec( + state, + &format!("pmacs.buffer.find_or_open(\"{}\")", lua_str(path)), + ); +} + +fn settle(state: &mut EditorState) { + for _ in 0..8 { + state.tick_processes(); + state.tick_lsp(); + std::thread::sleep(Duration::from_millis(2)); + } +} + +/// One `language_id|root_uri|cwd|state` row per live server, sorted so +/// assertions do not depend on spawn order. Absent fields read as "". +fn rows(state: &EditorState) -> Vec { + let joined: String = eval( + state, + r#" + local out = {} + for _, s in ipairs(pmacs.lsp.list()) do + out[#out + 1] = table.concat({ + s.language_id or "", + s.root_uri or "", + s.cwd or "", + (s.state and s.state.kind) or "", + }, "|") + end + table.sort(out) + return table.concat(out, "\n") + "#, + ); + if joined.is_empty() { + Vec::new() + } else { + joined.lines().map(str::to_owned).collect() + } +} + +fn status(state: &EditorState) -> String { + state.core.borrow().status.clone() +} + +fn count(state: &EditorState) -> usize { + let n: i64 = eval(state, "return #pmacs.lsp.list()"); + usize::try_from(n).expect("server count is non-negative") +} + +// --------------------------------------------------------------------------- +// Acceptance 13 — `lsp.list()` rows carry `root_uri` and `cwd`. +// --------------------------------------------------------------------------- + +#[test] +fn acc13_list_rows_carry_root_uri_and_cwd() { + let fx = Fixture::new(); + fx.write("proj/Cargo.toml", "[package]\nname = \"p\"\n"); + let file = fx.write("proj/src/main.rs", "fn main() {}\n"); + let mut state = editor(); + fx.bind(&state); + configure(&state, "rust"); + open(&state, &file); + settle(&mut state); + + let proj = fx.dir("proj"); + let rows = rows(&state); + assert_eq!(rows.len(), 1, "{rows:?}"); + let fields: Vec<&str> = rows[0].split('|').collect(); + assert_eq!(fields[0], "rust"); + assert_eq!( + fields[1], + file_uri(&proj), + "root_uri must be the project root" + ); + assert_eq!( + fields[2], + proj.display().to_string(), + "cwd must be the root" + ); +} + +// --------------------------------------------------------------------------- +// Acceptance 14 — two roots, same language, two servers. +// --------------------------------------------------------------------------- + +#[test] +fn acc14_two_project_roots_of_one_language_spawn_two_servers() { + let fx = Fixture::new(); + fx.write("a/Cargo.toml", "[package]\nname = \"a\"\n"); + fx.write("b/Cargo.toml", "[package]\nname = \"b\"\n"); + let first = fx.write("a/src/main.rs", "fn main() {}\n"); + let second = fx.write("b/src/main.rs", "fn main() {}\n"); + let mut state = editor(); + fx.bind(&state); + configure(&state, "rust"); + open(&state, &first); + settle(&mut state); + open(&state, &second); + settle(&mut state); + + let rows = rows(&state); + assert_eq!(rows.len(), 2, "one server per project root: {rows:?}"); + let roots: Vec<&str> = rows.iter().map(|r| r.split('|').nth(1).unwrap()).collect(); + assert!( + roots.contains(&file_uri(&fx.dir("a")).as_str()), + "{roots:?}" + ); + assert!( + roots.contains(&file_uri(&fx.dir("b")).as_str()), + "{roots:?}" + ); +} + +// --------------------------------------------------------------------------- +// Acceptance 15 — same root, two files, one server. The pre-change +// behavior, pinned so the fix cannot degrade into "always spawn". +// --------------------------------------------------------------------------- + +#[test] +fn acc15_two_files_in_one_root_reuse_a_single_server() { + let fx = Fixture::new(); + fx.write("proj/Cargo.toml", "[package]\nname = \"p\"\n"); + let first = fx.write("proj/src/main.rs", "fn main() {}\n"); + let second = fx.write("proj/src/other.rs", "pub fn other() {}\n"); + let mut state = editor(); + fx.bind(&state); + configure(&state, "rust"); + open(&state, &first); + settle(&mut state); + open(&state, &second); + settle(&mut state); + + let rows = rows(&state); + assert_eq!(rows.len(), 1, "same root must reuse: {rows:?}"); + assert_eq!( + rows[0].split('|').nth(1).unwrap(), + file_uri(&fx.dir("proj")) + ); +} + +// --------------------------------------------------------------------------- +// Acceptance 16 — per-language regression pin. The single-root case is +// all the shipped attach paths ever exercised; it must be untouched. +// --------------------------------------------------------------------------- + +#[test] +fn acc16_shipped_languages_are_unchanged_for_the_single_root_case() { + // (language id, project marker, two source files under it) + let cases: [(&str, &str, &str, &str); 4] = [ + ("rust", "Cargo.toml", "one.rs", "two.rs"), + ("python", "pyproject.toml", "one.py", "two.py"), + ("go", "go.mod", "one.go", "two.go"), + ("typescript", "package.json", "one.ts", "two.ts"), + ]; + for (language, marker, first_name, second_name) in cases { + let fx = Fixture::new(); + fx.write(&format!("proj/{marker}"), "{}\n"); + let first = fx.write(&format!("proj/src/{first_name}"), "\n"); + let second = fx.write(&format!("proj/src/{second_name}"), "\n"); + let mut state = editor(); + fx.bind(&state); + configure(&state, language); + open(&state, &first); + settle(&mut state); + open(&state, &second); + settle(&mut state); + + let rows = rows(&state); + assert_eq!( + rows.len(), + 1, + "{language}: expected one server, got {rows:?}" + ); + let fields: Vec<&str> = rows[0].split('|').collect(); + assert_eq!(fields[0], language, "{language}: language_id"); + assert_eq!( + fields[1], + file_uri(&fx.dir("proj")), + "{language}: root must be the marker directory" + ); + } +} + +// --------------------------------------------------------------------------- +// Acceptance 17 — hoist pin. `project_root_for` now runs on the *reuse* +// path, and a function-valued `root` is memoized per directory. +// --------------------------------------------------------------------------- + +#[test] +fn acc17_function_root_runs_on_the_reuse_path_and_memoizes_per_directory() { + let fx = Fixture::new(); + let shared = fx.dir("shared"); + std::fs::create_dir_all(&shared).unwrap(); + let a1 = fx.write("one/a.rs", "fn a() {}\n"); + let a2 = fx.write("one/b.rs", "fn b() {}\n"); + let b1 = fx.write("two/c.rs", "fn c() {}\n"); + let mut state = editor(); + fx.bind(&state); + // A resolver that answers the same root for every directory: the + // second directory therefore REUSES the first directory's server, + // which is exactly the path the hoist put the resolver on. + exec( + &state, + &format!( + r#" + _G.ROOT_CALLS = 0 + pmacs.lsp.config.rust = {{ + command = "{}", + root = function(_) + _G.ROOT_CALLS = _G.ROOT_CALLS + 1 + return "{}" + end, + }} + "#, + fake_lsp_path(), + lua_str(&shared) + ), + ); + + open(&state, &a1); + settle(&mut state); + assert_eq!(eval::(&state, "return _G.ROOT_CALLS"), 1, "spawn path"); + + // Same directory: served from the memo, so the count does not move. + open(&state, &a2); + settle(&mut state); + assert_eq!( + eval::(&state, "return _G.ROOT_CALLS"), + 1, + "second file in the same directory must hit the memo" + ); + + // Different directory: the resolver runs again — proving the reuse + // path resolves at all — but resolves to the same root, so no second + // server appears. + open(&state, &b1); + settle(&mut state); + assert_eq!( + eval::(&state, "return _G.ROOT_CALLS"), + 2, + "a new directory must consult the resolver on the reuse path" + ); + let rows = rows(&state); + assert_eq!(rows.len(), 1, "one resolved root, one server: {rows:?}"); + assert_eq!(rows[0].split('|').nth(1).unwrap(), file_uri(&shared)); +} + +// --------------------------------------------------------------------------- +// Acceptance 18 — a hand-spawned server carrying only `cwd` is not +// adopted by a root-bearing attach. A deliberate behavior change. +// --------------------------------------------------------------------------- + +#[test] +fn acc18_hand_spawned_server_without_root_uri_is_not_adopted() { + let fx = Fixture::new(); + fx.write("proj/Cargo.toml", "[package]\nname = \"p\"\n"); + let file = fx.write("proj/src/main.rs", "fn main() {}\n"); + let proj = fx.dir("proj"); + let mut state = editor(); + fx.bind(&state); + configure(&state, "rust"); + // Exactly what an init.lua would write: cwd, no root_uri. + exec( + &state, + &format!( + r#" + pmacs.lsp.spawn({{ + label = "hand-rolled", + language_id = "rust", + command = "{}", + cwd = "{}", + }}) + "#, + fake_lsp_path(), + lua_str(&proj) + ), + ); + settle(&mut state); + assert_eq!(count(&state), 1, "the hand-spawned server is up"); + + open(&state, &file); + settle(&mut state); + + let rows = rows(&state); + assert_eq!(rows.len(), 2, "the attach must not adopt it: {rows:?}"); + let roots: Vec<&str> = rows.iter().map(|r| r.split('|').nth(1).unwrap()).collect(); + assert!( + roots.contains(&""), + "hand-spawned reads back nil: {roots:?}" + ); + assert!( + roots.contains(&file_uri(&proj).as_str()), + "the attach's own server carries the root: {roots:?}" + ); +} + +// --------------------------------------------------------------------------- +// Acceptance 19 — a dead server in the matching root is not reused. +// --------------------------------------------------------------------------- + +#[test] +fn acc19_stopped_server_in_the_matching_root_is_not_reused() { + let fx = Fixture::new(); + fx.write("proj/Cargo.toml", "[package]\nname = \"p\"\n"); + let first = fx.write("proj/src/main.rs", "fn main() {}\n"); + let second = fx.write("proj/src/other.rs", "pub fn other() {}\n"); + let mut state = editor(); + fx.bind(&state); + configure(&state, "rust"); + open(&state, &first); + settle(&mut state); + let original: i64 = eval(&state, "return pmacs.lsp.list()[1].id:raw()"); + + exec(&state, "pmacs.lsp.stop(pmacs.lsp.list()[1].id)"); + for _ in 0..200 { + settle(&mut state); + let dead: bool = eval( + &state, + r#" + for _, s in ipairs(pmacs.lsp.list()) do + local k = s.state and s.state.kind + if k == "stopped" or k == "crashed" then return true end + end + return false + "#, + ); + if dead { + break; + } + } + + open(&state, &second); + settle(&mut state); + let live: i64 = eval( + &state, + r#" + for _, s in ipairs(pmacs.lsp.list()) do + local k = s.state and s.state.kind + if k ~= "stopped" and k ~= "crashed" then + return s.id:raw() + end + end + return -1 + "#, + ); + assert_ne!(live, -1, "a replacement server must exist"); + assert_ne!(live, original, "the dead server must not be reused"); +} + +// --------------------------------------------------------------------------- +// Acceptance 20 — the loose-file pin (Q#LN15 part 2). This is the +// no-change case, and the one a naive `(language_id, root)` key breaks. +// --------------------------------------------------------------------------- + +#[test] +fn acc20_markerless_files_in_different_directories_share_one_server() { + let fx = Fixture::new(); + let first = fx.write("loose_a/one.rs", "fn one() {}\n"); + let second = fx.write("loose_b/two.rs", "fn two() {}\n"); + let mut state = editor(); + fx.bind(&state); + configure(&state, "rust"); + open(&state, &first); + settle(&mut state); + open(&state, &second); + settle(&mut state); + + let rows = rows(&state); + assert_eq!( + rows.len(), + 1, + "loose files must keep sharing one server: {rows:?}" + ); + let fields: Vec<&str> = rows[0].split('|').collect(); + assert_eq!(fields[1], "", "the fallback root is not an affinity key"); + assert_eq!( + fields[2], + fx.dir("loose_a").display().to_string(), + "cwd still carries the first file's directory" + ); +} + +// --------------------------------------------------------------------------- +// Acceptance 21 — detected and fallback are different servers, and the +// fallback one still carries its directory as `cwd`. +// --------------------------------------------------------------------------- + +#[test] +fn acc21_detected_root_and_markerless_file_get_different_servers() { + let fx = Fixture::new(); + fx.write("proj/Cargo.toml", "[package]\nname = \"p\"\n"); + let inside = fx.write("proj/src/main.rs", "fn main() {}\n"); + let loose = fx.write("loose/stray.rs", "fn stray() {}\n"); + let mut state = editor(); + fx.bind(&state); + configure(&state, "rust"); + open(&state, &inside); + settle(&mut state); + open(&state, &loose); + settle(&mut state); + + let rows = rows(&state); + assert_eq!(rows.len(), 2, "detected and fallback must differ: {rows:?}"); + let detected = rows + .iter() + .find(|r| r.split('|').nth(1).unwrap() == file_uri(&fx.dir("proj"))) + .unwrap_or_else(|| panic!("no server rooted at the project: {rows:?}")); + assert_eq!( + detected.split('|').nth(2).unwrap(), + fx.dir("proj").display().to_string() + ); + let fallback = rows + .iter() + .find(|r| r.split('|').nth(1).unwrap().is_empty()) + .unwrap_or_else(|| panic!("no rootless server: {rows:?}")); + assert_eq!( + fallback.split('|').nth(2).unwrap(), + fx.dir("loose").display().to_string(), + "the markerless server keeps the fallback directory as cwd" + ); +} + +// --------------------------------------------------------------------------- +// Review-round-1 pins. Neither is a numbered acceptance criterion; both +// cover a branch the nine above leave untested. +// --------------------------------------------------------------------------- + +/// A *string* `config.root` is an affinity key. acc17 covers the function +/// form; without this the `return configured, "config"` arm has no test. +/// +/// The bite: both files sit in their own marked project, so if the config +/// arm were dropped they would key on their own detected roots and spawn +/// two servers. One server keyed on the configured root is only possible +/// if the override wins. +#[test] +fn config_string_root_overrides_detection_as_the_affinity_key() { + let fx = Fixture::new(); + fx.write("a/Cargo.toml", "[package]\nname = \"a\"\n"); + fx.write("b/Cargo.toml", "[package]\nname = \"b\"\n"); + let first = fx.write("a/src/main.rs", "fn main() {}\n"); + let second = fx.write("b/src/main.rs", "fn main() {}\n"); + let shared = fx.dir("shared"); + std::fs::create_dir_all(&shared).unwrap(); + let mut state = editor(); + fx.bind(&state); + exec( + &state, + &format!( + "pmacs.lsp.config.rust = {{ command = \"{}\", root = \"{}\" }}", + fake_lsp_path(), + lua_str(&shared) + ), + ); + open(&state, &first); + settle(&mut state); + open(&state, &second); + settle(&mut state); + + let rows = rows(&state); + assert_eq!( + rows.len(), + 1, + "a configured root outranks both detected roots: {rows:?}" + ); + let fields: Vec<&str> = rows[0].split('|').collect(); + assert_eq!(fields[1], file_uri(&shared), "keyed on the configured root"); + assert_eq!(fields[2], shared.display().to_string()); +} + +/// `root = false` reads as unset, as it always has. Defended in +/// `project_root_for` by a truthiness check rather than `~= nil`; this +/// pins the behavior instead of trusting the comment. +/// +/// The bite: under a `~= nil` test the config arm would return +/// `false, "config"`, and `file_uri_for(false)` returns nil — so the file +/// would land on a rootless server instead of its detected project. +#[test] +fn config_root_false_reads_as_unset_and_detection_still_wins() { + let fx = Fixture::new(); + fx.write("proj/Cargo.toml", "[package]\nname = \"p\"\n"); + let file = fx.write("proj/src/main.rs", "fn main() {}\n"); + let mut state = editor(); + fx.bind(&state); + exec( + &state, + &format!( + "pmacs.lsp.config.rust = {{ command = \"{}\", root = false }}", + fake_lsp_path() + ), + ); + open(&state, &file); + settle(&mut state); + + let rows = rows(&state); + assert_eq!(rows.len(), 1, "{rows:?}"); + assert_eq!( + rows[0].split('|').nth(1).unwrap(), + file_uri(&fx.dir("proj")), + "`false` must not become a root; detection still wins" + ); +} + +/// COHERENCE §1.2: background wiring must leave an attributed trace +/// rather than discard a failure. A throwing root resolver is a config +/// bug, and the per-directory memo would otherwise bury it permanently. +/// +/// The bite: drop the reporting arm and `*errors*` stays empty while the +/// attach still succeeds — the exact silence §1.2 names as the canonical +/// anti-pattern, in the function it cites. +#[test] +fn a_throwing_root_resolver_leaves_an_attributed_trace() { + let fx = Fixture::new(); + fx.write("proj/Cargo.toml", "[package]\nname = \"p\"\n"); + let file = fx.write("proj/src/main.rs", "fn main() {}\n"); + let mut state = editor(); + fx.bind(&state); + exec( + &state, + &format!( + r#" + pmacs.lsp.config.rust = {{ + command = "{}", + root = function(_) error("resolver blew up") end, + }} + "#, + fake_lsp_path() + ), + ); + open(&state, &file); + settle(&mut state); + + let msg = status(&state); + assert!( + msg.contains("root resolver"), + "a raising resolver must leave an attributed trace; got: {msg:?}" + ); + assert!( + msg.contains("rust"), + "the trace must name the language that owns it; got: {msg:?}" + ); + assert!( + msg.contains("resolver blew up"), + "the underlying error text must survive; got: {msg:?}" + ); + + // ...and the failure must degrade to a decline, not a failed attach: + // detection still wins and the buffer still gets its server. + let rows = rows(&state); + assert_eq!(rows.len(), 1, "the attach must still succeed: {rows:?}"); + assert_eq!( + rows[0].split('|').nth(1).unwrap(), + file_uri(&fx.dir("proj")), + "a declining resolver falls through to the marker walk" + ); +} + +/// The decline path stays silent. Without this, "report failures" could +/// be satisfied by reporting *every* resolution, which would spam +/// `*errors*` on every attach in a Lean project. +#[test] +fn a_resolver_returning_nil_declines_silently() { + let fx = Fixture::new(); + fx.write("proj/Cargo.toml", "[package]\nname = \"p\"\n"); + let file = fx.write("proj/src/main.rs", "fn main() {}\n"); + let mut state = editor(); + fx.bind(&state); + exec( + &state, + &format!( + "pmacs.lsp.config.rust = {{ command = \"{}\", root = function(_) return nil end }}", + fake_lsp_path() + ), + ); + open(&state, &file); + settle(&mut state); + + let msg = status(&state); + assert!( + !msg.contains("root resolver"), + "returning nil is the documented decline, not a failure; got: {msg:?}" + ); + assert_eq!( + rows(&state)[0].split('|').nth(1).unwrap(), + file_uri(&fx.dir("proj")) + ); +}