From 7e3e630c3b35042ce10e8647a9526540483245d7 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 29 Jul 2026 18:09:03 -0400 Subject: [PATCH] =?UTF-8?q?wip(stage2a):=20the=20Lua=20half=20=E2=80=94=20?= =?UTF-8?q?hooks,=20LSP=20subscribers,=20applier=20origin?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resource.renamed` and `resource.deleted` are declared `all-must-succeed`, so one raising subscriber does not stop the rest from reconciling. `lsp.lua` gains the two subscribers. Rename runs the ordered teardown per attachment — flush the pending didChange, didClose the old URI, `forget_uri` against the OLD server, re-run `ensure_server` (a rename across project roots needs a different one), didOpen the new URI, then re-root the diagnostic overlays. Delete tears the attachment down, because the buffer may be gone entirely and a retained record is a dangling handle. The workspace-edit applier captures the origin BUFFER instead of its path, and restores nothing when that buffer is gone. A captured Lua local is unreachable to any transaction, and the old path fallback is what materialized a phantom empty buffer at the renamed-away path. `fs.lua` states the overlapping-mutation serialization precondition as a correctness rule, with the counterexample showing why no static ordering rule substitutes for it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T --- builtin/hooks/default.lua | 32 +++++++++ builtin/runtime/fs.lua | 24 +++++++ builtin/runtime/lsp.lua | 143 +++++++++++++++++++++++++++++++++++--- 3 files changed, 191 insertions(+), 8 deletions(-) diff --git a/builtin/hooks/default.lua b/builtin/hooks/default.lua index 4fabfe9..69b04c3 100644 --- a/builtin/hooks/default.lua +++ b/builtin/hooks/default.lua @@ -16,6 +16,12 @@ -- format-on-save subscribe here. -- * editor.before-quit --- short-circuit. A callback may veto quit -- (e.g. "buffer modified --- save first?"). +-- * resource.renamed --- all-must-succeed (dired Stage 2a). Fired +-- after a successful rename, with (old, new) +-- canonical absolute paths. +-- * resource.deleted --- all-must-succeed (dired Stage 2a). Fired +-- after a successful delete, with the +-- canonical absolute path. -- -- These are *defined* here so user config can attach callbacks via -- pmacs.hook.add. Run sites are in Rust (after-load, after-edit) and in @@ -76,6 +82,32 @@ define { kind = "short-circuit", } +define { + name = "resource.renamed", + description = "Fired once per SUCCESSFUL filesystem rename, with the old " .. + "and new paths as canonical absolute strings. The core " .. + "reconciles what it can reach -- buffer paths and names, the " .. + "URI-keyed LSP stores, attached diagnostic overlays -- but a " .. + "package that keys its own state by path or URI is invisible " .. + "to that, so this hook is the mechanism that scales. It " .. + "carries PATHS rather than a rebind list precisely because " .. + "dired's listing buffers are pathless: a path-keyed consumer " .. + "must be able to reconcile from (old, new) alone. Does not " .. + "fire for a rename that failed or was cancelled.", + kind = "all-must-succeed", +} + +define { + name = "resource.deleted", + description = "Fired once per SUCCESSFUL filesystem delete, with the " .. + "canonical absolute path. Buffers on the path and beneath it " .. + "have already been reconciled: unmodified ones killed " .. + "through both removal phases, modified ones kept alive. " .. + "Subscribers drop their own path-keyed state. Does not fire " .. + "for a delete that failed or was cancelled.", + kind = "all-must-succeed", +} + define { name = "editor.before-quit", description = "Fired before the editor exits. Return false to veto.", diff --git a/builtin/runtime/fs.lua b/builtin/runtime/fs.lua index 39e3baa..35b25a2 100644 --- a/builtin/runtime/fs.lua +++ b/builtin/runtime/fs.lua @@ -163,6 +163,30 @@ end -- If a package needs at-most-one-pending semantics for mutations, -- it should serialize on the package side (await each op before -- dispatching the next). The fs primitive can't enforce that. +-- +-- **And that is a CORRECTNESS precondition, not only a cancellation +-- one (dired Stage 2a, Q#DR29).** A successful `rename` or `remove` +-- reconciles the editor's path owners in the main-thread drain — buffer +-- paths and names, the URI-keyed LSP state, the `resource.renamed` / +-- `resource.deleted` hooks. That reconciliation is deliberately +-- order-INDEPENDENT: the runtime drains the reply bus with `try_recv` +-- and establishes no execution token, so a worker can finish first and +-- be descheduled before sending, and reply order therefore does not +-- recover filesystem execution order. +-- +-- Independent mutations commute, so nothing is owed for them. But +-- **mutations whose source/target paths overlap must be serialized by +-- dispatching the next only after the previous handle settles.** There +-- is no static ordering rule that would substitute: rename `dir` -> +-- `newdir` racing delete `dir/child.txt` needs delete-then-rename if +-- the delete ran first on disk and rename-then-delete if the rename +-- did, and a fixed "deletes before renames" rule gets one of the two +-- wrong — the kill misses, the rename then rebinds the buffer onto a +-- path whose file is gone, and it survives pointing at nothing. +-- +-- A caller that ignores this owns the residue: a buffer left bound to a +-- stale path, or killed when it should have been rebound. Recoverable +-- and visible, not data loss — but real. function fs.rename(from, to) if type(from) ~= "string" then diff --git a/builtin/runtime/lsp.lua b/builtin/runtime/lsp.lua index f17827f..679d573 100644 --- a/builtin/runtime/lsp.lua +++ b/builtin/runtime/lsp.lua @@ -1124,7 +1124,7 @@ pmacs.hook.add("buffer.after-edit", function() -- Stale suppression must stay keystroke-accurate even though the -- O(file) didChange send below is coalesced: render families -- anchored to pre-edit positions are hidden from this edit on. - pcall(pmacs.lsp._mark_document_stale, rec.uri) + pcall(pmacs.lsp._mark_document_stale, rec.server, rec.uri) -- Arc 1d: was this edit a typed character? The input-origin signal -- (see the trigger block below). local typed = pmacs.editor.this_command @@ -1437,19 +1437,33 @@ local function apply_workspace_edit(ops) end end if #plan == 0 then return 0, 0, 0 end - local origin = active_buffer_path() + -- G1 — capture the origin BUFFER, not its path. A path captured here + -- is a plain Lua local, and no amount of reconciliation can reach an + -- already-captured local: when the batch renames the active file, the + -- old path no longer resolves, `find_or_open` hits + -- `resolve_target_buffer`'s NotFound arm, and that arm CREATES an + -- empty path-backed buffer and selects it. The user was returned to a + -- phantom file that never existed. The handle follows the rename for + -- free, because the buffer is what moved. + local origin_buf = pmacs.window.buffer() local edit_total, files, res_ops = 0, 0, 0 -- Plan items fully applied before a failure. Q#RD3 permits partial -- application, so this is what stops a caller claiming "nothing was -- mutated" when something was. local applied_ops = 0 - -- Return the user to where they invoked from — best-effort, since - -- that path may have just been renamed or deleted. Runs on the - -- FAILURE path too (Q#RD7): previously this ran only after a - -- successful loop, so a mid-batch refusal stranded the user in - -- whatever buffer the last applied op left active. + -- Return the user to where they invoked from. Runs on the FAILURE + -- path too (Q#RD7): previously this ran only after a successful loop, + -- so a mid-batch refusal stranded the user in whatever buffer the last + -- applied op left active. + -- + -- **No path fallback (G1).** If the origin buffer is gone — the batch + -- deleted its file and reconciliation killed it — restore NOTHING. + -- The old code's path fallback is exactly what fabricated a phantom + -- buffer; "return the user somewhere plausible" is not worth inventing + -- a file that does not exist. local function restore_origin() - if origin then pcall(pmacs.buffer.find_or_open, origin) end + if not origin_buf then return end + pcall(pmacs.window.switch_buffer, origin_buf) end for _, item in ipairs(plan) do local ok, err @@ -2882,3 +2896,116 @@ pmacs.command.define { pmacs.keymap.bind { scope = "global", sequence = "M-g n", command = "diag.next" } pmacs.keymap.bind { scope = "global", sequence = "M-g p", command = "diag.previous" } + +-- Resource reconciliation --------------------------------------------------- +-- +-- dired Stage 2a, §5. A rename or delete moves or destroys a path that +-- FOURTEEN URI-keyed store families, the `documents` mirror, the pending +-- response routes and the attached diagnostic overlays are all keyed by. +-- `EditorCore` reconciles the buffer's own path and name; these two +-- subscribers reconcile the LSP layer, which is buffer-keyed here +-- (`rec.uri` is cached per buffer and read at dozens of sites, so ONE +-- rebind reaches all of them) and URI-keyed in Rust. +-- +-- These subscribers are independent of every other `resource.renamed` +-- consumer by construction: this one touches URI-keyed state, dired's +-- touches its own handle table, and neither reads what the other wrote. +-- That matters because `all-must-succeed` does NOT abort the fan-out — +-- `run_all_must_succeed` collects each callback's error and continues — +-- so a subscriber may not rely on a raising peer to stop the sequence, +-- and the ordered teardown below is ordered INTERNALLY rather than by +-- registration. + +-- Every attachment whose document is `path` or lies beneath it, as +-- `{ key, rec, path }`. Resolved through `path_for_uri` and compared +-- with `paths_related`, so the comparison is component-aware and runs on +-- the same canonical form the buffer registry keys on. +local function attachments_under(path) + local out = {} + for key, rec in pairs(attachments) do + local rec_path = rec.uri and pmacs.lsp.path_for_uri(rec.uri) + if rec_path and paths_related(rec_path, path) then + out[#out + 1] = { key = key, rec = rec, path = rec_path } + end + end + return out +end + +pmacs.hook.add("resource.renamed", function(old_path, new_path) + if type(old_path) ~= "string" or type(new_path) ~= "string" then return end + for _, hit in ipairs(attachments_under(old_path)) do + local key, rec, old_uri = hit.key, hit.rec, hit.rec.uri + -- The buffer's own path was rebound before this hook fired, so ask + -- it rather than reconstructing the tail ourselves. A buffer that + -- somehow lost its path (killed, unbound) cannot be re-opened, and + -- falls through to the teardown-only path below. + local ok_path, new_buf_path = pcall(function() return rec.buffer:path() end) + local new_uri = (ok_path and new_buf_path) and file_uri_for(new_buf_path) or nil + + -- 1. Flush any pending didChange for the OLD uri, so the server is + -- not left holding an edit it can no longer attribute. + flush_did_change_for(rec) + pending_did_change[key] = nil + + -- 2. didClose the old uri — this removes the open-document + -- registration and nothing else. + pcall(pmacs.lsp.did_close, rec.server, old_uri) + + -- 3. Purge the routes, drain their awaiters, and clear all fourteen + -- stores plus `documents` for the old key. Runs against the OLD + -- server, which matters when step 4 picks a different one. + pcall(pmacs.lsp.forget_uri, rec.server, old_uri) + + if not new_uri then + attachments[key] = nil + styled_buffers[key] = nil + diag_viewed_buffers[key] = nil + else + -- 4. Re-run ensure_server. Server affinity keys on the detected + -- project root, so a rename ACROSS roots needs a different + -- server; a same-root rename reuses the existing one. + local sid = ensure_server(rec.language, new_buf_path) + if not sid then + attachments[key] = nil + styled_buffers[key] = nil + diag_viewed_buffers[key] = nil + else + -- 5. didOpen the new uri with the buffer's current text and a + -- fresh version. This also reclaims the tombstone for + -- exactly (server, new uri). + rec.server = sid + rec.uri = new_uri + rec.version = 1 + local ok_text, text = pcall(buffer_text, rec.buffer) + pcall(pmacs.lsp.did_open, sid, new_uri, rec.version, + ok_text and text or "") + -- 6. Re-root the diagnostic overlays. `DiagnosticView.uri` is + -- set once at construction and is private, so this is the + -- only way to move it — and the sweep reaches PASSIVE + -- windows, which the attach path cannot, while preserving + -- each overlay's position in the composition order. + pcall(pmacs.diag._rename_resource, old_uri, new_uri) + end + end + end +end) + +pmacs.hook.add("resource.deleted", function(path) + if type(path) ~= "string" then return end + for _, hit in ipairs(attachments_under(path)) do + local key, rec = hit.key, hit.rec + -- No flush: the document is gone, and shipping a didChange for a + -- file the server can no longer read buys nothing. + pending_did_change[key] = nil + pcall(pmacs.lsp.did_close, rec.server, rec.uri) + pcall(pmacs.lsp.forget_uri, rec.server, rec.uri) + -- Drop the record unconditionally. The buffer may be gone entirely + -- (an unmodified visited file is killed), in which case a retained + -- record is a dangling handle that `repull_for_attachments` would + -- iterate; and a modified buffer kept alive has no file to analyze + -- until it is saved, which re-attaches through the ordinary path. + attachments[key] = nil + styled_buffers[key] = nil + diag_viewed_buffers[key] = nil + end +end)