From e1e9b441545257cd6ad14e1104a34b45c4dd5046 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Tue, 28 Jul 2026 18:23:52 -0400 Subject: [PATCH] docs(framing): revision 2 of the resource-op delete guard Review round 1 approved the refusal strategy in principle and rejected revision 1 as written. Six blocking points, all accepted, plus three further overclaims found by the requested sweep. Q#RD1 and Q#RD5 are settled yes; Q#RD9 is settled no and is withdrawn. Q#RD2 conflated inspection with removal. Revision 1 removed the buffer before the filesystem call, which fires arbitrary Lua `on_removed` callbacks while the file still exists and accepts losing the buffer if the deletion then fails. The sequence is now `stat/no-op -> enumerate and validate -> mutate filesystem -> reconcile`. Validation inspects `is_modified` and `editing_in_progress` without removing anything, so a failed deletion leaves buffers intact automatically and `on_removed` still observes the path already gone. Q#RD3 overclaimed whole-batch atomicity. `documentChanges` are sequential, so an earlier edit can dirty a clean buffer and an earlier rename can move a modified buffer into a later delete's subtree, after the snapshot. LSP 3.18 assigns `FailureHandlingKind.Abort` to any edit containing resource changes --- "all operations executed before the failing operation stay executed" --- so the protocol itself declines to promise what revision 1 claimed. The preflight is now described as an early conflict check, with robustness coming from per-op `pcall`, an always-sent server response, and best-effort origin restore. The lookup cannot be `EditorCore::find_buffer_for_path`: it normalizes but delegates to the first-match-only `find_by_path`, and `pmacs.buffer.from_file` creates path-bound buffers with no dedup, so a clean first match can hide a modified second. Q#RD6 now requires a full scan with component-aware `Path::starts_with`. Recursive deletion now inspects the tree but reconciles only the exact path, so the parked lifecycle defect stays exact-path rather than becoming tree-wide. Q#RD4 holds at both layers, so the preflight cannot reject an absent path the primitive treats as a no-op. The prompt argument was overclaimed and that was my error. `pmacs.lsp.send_response` takes `request_id` as an ordinary value, so a `workspace/applyEdit` can be answered on a later tick, and a callback continuation would reuse the existing minibuffer shadow rather than add a seventh dispatcher rung. Prompting is expensive and separately scoped, not impossible; the section now claims only what the evidence carries. Sweep found three more of the same defect class --- an absence or a guarantee asserted rather than established: * a durable error surface does exist (`append_to_errors_buffer` -> `*errors*`), so Q#RD7 now records the refusal there as well; * no caller reliably surfaces a raise, because the async path routes uncaught coroutine errors through the undefined `pmacs.error`; * pmacs advertises no `workspace.workspaceEdit` capability at all --- no `documentChanges`, no `resourceOperations`, no `failureHandling`. Adds seven acceptance pins with their bite obligations, adds the `docs/active-work.md` lane the ledger requires for every open PR, and drops the two-PR plan: #186 is revised in place and becomes the implementation PR. Still PROPOSED. No runtime code. Implementation begins only after explicit user approval. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T --- docs/active-work.md | 72 ++ docs/resource-op-delete-guard-framing.md | 1164 +++++++++++++--------- 2 files changed, 781 insertions(+), 455 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index a303151..7bac9fc 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -459,6 +459,78 @@ has **no branch and no framing yet**. `FrontendView.fold_projection` to `true` for semantic frontends, which Stage 2 deliberately left `false` (Q#FD21). +## Resource-op delete guard lane — PR #186 OPEN, PROPOSED, DO NOT MERGE + +- Portable branch: `githubsucks/resource-op-delete-guard`; worktree + `../pmacs-resource-op-delete`. **PR #186**, base `main`, forked from + `ad41cf1` with **no drift** (`main` is still `ad41cf1`). Currently + framing only — `docs/resource-op-delete-guard-framing.md`, revision 2 + — plus this lane entry. No runtime code yet. +- **This PR becomes the implementation PR.** Revision 2 dropped rev 1's + framing-PR-then-implementation-PR plan as a one-feature/one-branch/ + one-PR violation. The framing is revised in place; implementation + commits land on this same branch **only after explicit user + approval**. +- **Live data-loss bug, reproduced four ways against `ad41cf1`.** + `pmacs.buffer.apply_resource_op`'s delete arm removes the path from + disk and *then* drops any buffer bound to it, with no dirty check at + any link — not the arm, not `remove_buffer_and_fire`, and not + `BufferRegistry::remove`, whose only guard is `editing_in_progress`. + Reachable through any language server's `WorkspaceEdit`. The four + modes: (a) the plain case returns `Ok(())` with file and buffer both + gone; (b) `ignore_if_not_exists = true` does **zero** filesystem work + and still destroys the buffer; (c) `recursive = true` reconciles + **nothing**, so a whole tree leaves orphaned buffers — the most + destructive arm does the least reconciliation, and it bypasses any + exact-path guard; (d) removal is not `kill_buffer`, so windows are + left bound to a removed `BufferId` and the registry can be driven to + **empty**. +- **Approved in principle after review round 1**, revision 1 rejected. + Settled: refuse unconditionally; take delete-side prefix-awareness now + rather than waiting on #171. Withdrawn: rev 1's buffer-first ordering. + The design is now `stat/no-op → enumerate and validate → mutate + filesystem → reconcile`, which keeps `on_removed`'s "path already + gone" invariant and makes a failed deletion leave buffers intact + automatically. +- **Four facts a re-scout should not have to rediscover**, all verified + at `ad41cf1`: + - **No caller reliably surfaces a raise.** The server pump runs under + `pcall(handle_server_requests)` (`builtin/runtime/lsp.lua:1892`), so + a raise unwinds past the `send_response` and the server is never + answered; and the two user-initiated paths route uncaught coroutine + errors through `pmacs.error`, which is **undefined** (11 call sites + in `builtin/`, zero definitions). Refusals must travel as values. + - **A partial batch is already the status quo** — verified: two delete + ops, the second raises, the first stayed applied. LSP 3.18 says so + too: resource-op-bearing edits get `FailureHandlingKind.Abort`, + "all operations executed before the failing operation stay + executed". Any framing claiming batch atomicity here is wrong. + - **`find_by_path` is singular and duplicates are reachable.** + `BufferRegistry::find_by_path` returns the first match in insertion + order, `EditorCore::find_buffer_for_path` inherits that, and + `pmacs.buffer.from_file` creates path-bound buffers with **no + dedup** — so a clean first match can hide a modified second. The + guard needs a full scan with component-aware `Path::starts_with`. + - **pmacs advertises no `workspace.workspaceEdit` capability at all** — + `"applyEdit": true` but no `documentChanges`, no + `resourceOperations`, no `failureHandling`; `grep -rn + failureHandling` returns 0. Parked, not fixed here. +- **Ownership claim, per the dired lane's own warning below:** dired + Stage 2a is "rename/delete reconciliation substrate" and overlaps + `builtin/runtime/lsp.lua`. **This lane claims the delete half of that + substrate and the `apply_workspace_edit` applier for its duration**; + dired Stage 2 keeps the rename half. Whichever lands second adopts the + first's shared lookup helper. Do not run the two concurrently over + `builtin/runtime/lsp.lua` without re-splitting that claim. +- Files the implementation will touch: `src/lua_bindings/mod.rs`, + `builtin/runtime/lsp.lua`, `tests/m4_acceptance.rs`, + `src/bin/pmacs_fake_lsp.rs`. **Not** `src/daemon.rs`, + `pmacs-protocol/`, `builtin/runtime/dired.lua`, + `docs/agent-handoff.md` or `COHERENCE.md`. No protocol change. +- Recovery from a clean checkout: + `git fetch githubsucks && git worktree add ../pmacs-resource-op-delete + -b resource-op-delete-guard githubsucks/resource-op-delete-guard`. + ## dired Stage 2 framing lane — PR #171 OPEN, STALE, DO NOT MERGE AS-IS - Portable branch: `githubsucks/dired-stage2-framing` (head `ab42a79`, diff --git a/docs/resource-op-delete-guard-framing.md b/docs/resource-op-delete-guard-framing.md index 02414a7..352f30b 100644 --- a/docs/resource-op-delete-guard-framing.md +++ b/docs/resource-op-delete-guard-framing.md @@ -1,23 +1,120 @@ # Framing — `apply_resource_op` delete destroys unsaved work -**Revision 1.** Status: **PROPOSED — needs explicit user approval before +**Revision 2.** Status: **PROPOSED — needs explicit user approval before implementation. DO NOT implement, DO NOT merge.** Lane: `resource-op-delete-guard`, worktree `../pmacs-resource-op-delete`, -based on `githubsucks/main` @ `ad41cf1`. +based on `githubsucks/main` @ `ad41cf1` (re-checked at revision 2: no +drift, `main` is still `ad41cf1`). This is a live data-loss bug, reproduced four ways against `ad41cf1` (§1.1). A language server can destroy a buffer's unsaved edits *and* the file that would have held them, with no prompt, no status message, and no error return. +**This PR is the implementation PR.** Revision 2 drops revision 1's +framing-PR-then-implementation-PR plan, which conflicted with +one-feature/one-branch/one-PR. The framing is revised in place; once the +design is approved, the implementation lands on this same branch and in +this same PR (§8). + ## Revision history -**Revision 1.** First cut. Every claim about pmacs in §1 was verified by -reading the tree at `ad41cf1` and, for §1.1, by running throwaway tests -in this worktree (removed before commit — this lane ships no runtime -code). Every claim about Emacs is marked as such in §1.9 and was -verified against `lisp/progmodes/eglot.el` on `emacs-mirror/emacs` -`master`, not from recollection. +### Revision 1 → 2, after review round 1 + +The refusal strategy was approved in principle; revision 1 as written +was not. Q#RD1 (refuse unconditionally) and Q#RD5 (take prefix-awareness +now) are **settled yes**; Q#RD9 is **settled no** and is withdrawn. +Six blocking points, all accepted, plus three further overclaims found +by the sweep the review asked for. + +1. **Q#RD2 conflated inspection with removal — rewritten.** Revision 1 + proposed removing the buffer *before* the filesystem call. That fires + arbitrary Lua `on_removed` callbacks while the file still exists, + destroying today's useful invariant that a subscriber observes the + path already gone, and it accepts losing the buffer if the deletion + then fails. The correct sequence separates the two: + `stat/no-op → enumerate and validate → mutate filesystem → reconcile`. + Validation needs no removal, so a failed deletion leaves the buffer + intact automatically. **The review is right and revision 1 was + wrong.** `Buffer::editing_in_progress()` (`src/buffer.rs:747`) is a + public getter, so the re-entrancy condition can also be checked + during validation rather than discovered during removal. +2. **Q#RD3 overclaimed whole-batch atomicity — rewritten and + downgraded.** Revision 1 called the preflight "whole-batch + atomicity" and said "**nothing** in the batch is mutated". That is + false for a sequential batch: an earlier text edit can dirty a + clean buffer, and an earlier rename can move a modified buffer + *into* a later delete's subtree, after the snapshot was taken. The + preflight is now described as an **early conflict check** — a + cheap, honest first filter, not a transaction (§2.1, Q#RD3). The + real robustness comes from per-op `pcall` and an always-sent + response (Q#RD7). +3. **The lookup cannot be `EditorCore::find_buffer_for_path` — accepted + and independently re-verified.** It normalizes but delegates to + `BufferRegistry::find_by_path` (`src/buffer_registry.rs:168`), whose + own doc says "First buffer bound to `path`" — singular, insertion + order. And `pmacs.buffer.from_file` (`src/lua_bindings/mod.rs:3112`) + calls `create_from_bytes` with no dedup check, so duplicate + path-bound buffers are reachable from public Lua. A clean first match + hides a modified second. Q#RD6 now requires a full scan. +4. **Do not expand the parked lifecycle defect — accepted.** Revision 1 + left it ambiguous whether a recursive delete should reconcile + descendants. It must not: mode (d)'s dangling-window and + last-buffer defects would be promoted from exact-path to tree-wide. + Q#RD5 now says explicitly that the tree is **inspected** but only the + exact path is **reconciled**. +5. **Q#RD4 must hold at both layers — accepted.** Revision 1 applied the + `ignore_if_not_exists` early return only to the primitive. The Lua + preflight must not reject an absent path merely because a modified + buffer still names it. +6. **The prompt argument was overclaimed — rewritten, and this was my + error.** Revision 1 said prompting was "architecturally + unavailable", "the only option that is *possible*", and that the + server-initiated path "cannot produce that answer". All three are + wrong. `pmacs.lsp.send_response` (`src/lua_bindings/mod.rs:9680`) + takes `request_id` as an ordinary value, so a `workspace/applyEdit` + **can** be answered on a later tick; and a callback continuation + would reuse the **existing** minibuffer shadow (rung 4), not add a + seventh rung. The honest claim is that prompting is *expensive and + separately scoped*, and §2.2 now argues only that, on evidence + (§1.8). + +**Three further overclaims found by the sweep** (the review asked for +the defect class, not just the three cited instances — all three are +the same shape: revision 1 asserted an absence or a guarantee it had +not established): + +7. **"There is no `*Messages*` buffer and no `*warnings*` buffer" was + misleading by omission.** There is a durable append-only error + surface: `LuaHost::append_to_errors_buffer` (`src/lua.rs:401`) + writing `*errors*` (`ERRORS_BUFFER_NAME`, `src/lua.rs:32`; 49 + references across `src/` and `builtin/`), already used by + `log_hook_error` (`src/lua_bindings/mod.rs:6061`), + `log_statusline_provider_error` (`:6099`) and `log_buffer_removed_error`. + This **improves the design**: Q#RD7 now records the refusal there + too, so it survives the status line being cleared and leaves a trace + on the unattended path. +8. **"Only path 1 is fully unattended" understated the problem.** + Revision 1 said a raise on paths 2 and 3 "propagates out of the + `pmacs.async` coroutine" without establishing where it lands. It + lands nowhere useful: `step` (`builtin/runtime/async.lua:196`) routes + an uncaught coroutine error to `pmacs.error`, **which is undefined** + — 11 call sites in `builtin/`, zero definitions — so the `error(...)` + fallback re-raises at the spawn site. **No caller reliably reports a + raise**, which strengthens rather than weakens point 2's requirement. +9. **pmacs advertises no `workspace.workspaceEdit` capability at all.** + `default_client_capabilities` (`src/lsp.rs:3242`) sends + `"applyEdit": true` but no `workspaceEdit` object, so no + `documentChanges`, no `resourceOperations`, and no `failureHandling` + — `grep -rn failureHandling` over the tree returns **0**. Revision 1 + discussed batch semantics without noting that pmacs declares no + failure-handling strategy. Named as ground truth (§1.11); **not + fixed here** (§6). + +### Revision 1 + +First cut. Established the bug and the four modes, the caller +inventory, the Emacs prior art, and the refusal recommendation. ## 0. Coherence impact (COHERENCE §20) @@ -26,21 +123,24 @@ verified against `lisp/progmodes/eglot.el` on `emacs-mirror/emacs` consequence **step 5, "Edit immediately"** — the loss is of exactly the edits step 5 grades as "genuinely excellent". §2's verdict table grades step 6 **Partial**; this lane does not raise that grade, it - removes a way the step can destroy the user's work. A journey that - eats unsaved edits at step 6 is not a journey worth protecting, so - this serves **Priority 1** ("treat regressions as release blockers") - as a correctness floor rather than a feature. -- **Interaction islands: none added.** This is the load-bearing - constraint, not an afterthought. §6 grades islands "weak, and growing - by one island per modal feature", and the count is **six**. A - confirmation prompt for this op would need a seventh (§2.2), which is - the single strongest argument against the prompt option. -- **Config registry: not adopted.** No knob is proposed. The refusal is - unconditional. An `lsp.confirm-server-edits`-style setting is the - natural future adopter and is parked in §6, not shipped here. + removes a way the step can destroy the user's work. Serves + **Priority 1** ("treat regressions as release blockers") as a + correctness floor rather than a feature. +- **Interaction islands: none added.** The recommended design adds no + modal surface at all — it refuses and reports. *Revision 2 correction:* + revision 1 additionally claimed that the rejected prompt option would + have added a seventh dispatcher rung. It would not; a callback + continuation reuses the existing minibuffer shadow (rung 4). The + count stays at six either way, and §6's island budget is **not** an + argument against prompting (§2.2). +- **Config registry: not adopted.** No knob is proposed; the refusal is + unconditional (Q#RD1). An `lsp.confirm-server-edits`-style setting is + the natural future adopter and is parked in §6. - **Background-work attribution: unchanged.** - **No audited claim in COHERENCE.md changes**, so under §25 no - COHERENCE edit rides this PR. + COHERENCE edit rides this PR. The `docs/active-work.md` lane for this + PR does ride it, per that file's "When a PR is opened, give it a + lane." ## 1. Ground truth (scouted and verified @ `ad41cf1`) @@ -49,11 +149,10 @@ verified against `lisp/progmodes/eglot.el` on `emacs-mirror/emacs` `pmacs.buffer.apply_resource_op` with `kind = "delete"` destroys a modified buffer and the file backing it. Reproduced in this worktree by -throwaway acceptance tests against `ad41cf1` (written, run, then -removed — no runtime code ships in this lane). Four distinct modes: +throwaway acceptance tests against `ad41cf1`, written, run, then removed +— they are the model for §5's pins, not shipped artefacts. Four modes: -**(a) The reported bug.** Open a file, edit it without saving, apply a -delete op for its path: +**(a) The reported bug.** ``` PRE: modified=true text="UNSAVED EDIT ORIGINAL ON DISK\n" @@ -67,7 +166,7 @@ text existed in exactly one place and now exists nowhere. **(b) `ignore_if_not_exists = true` destroys the buffer having done no filesystem work at all.** When the path is already absent, the delete -arm skips the `remove_file` — and then falls through to the buffer +arm skips the `remove_file` — and falls through to the buffer reconciliation anyway: ``` @@ -77,16 +176,11 @@ buffers after=2 CONFIRMED: ignore_if_not_exists=true did ZERO fs work yet still destroyed the only copy. ``` -This is the sub-case where the buffer is *most* certainly the only -copy, and it is the one arm where the op explicitly decided to do -nothing. Compare the `create` arm, which returns early -(`return Ok(())`) under the analogous `ignore_if_exists` condition; the -delete arm's `Err(NotFound)` branch does not return, it falls through. +The `create` arm returns early (`return Ok(())`) under the analogous +`ignore_if_exists` condition; the delete arm's `Err(NotFound)` branch +does not return. -**(c) `recursive = true` fails in the opposite direction.** A recursive -directory delete removes the whole tree from disk but reconciles *no* -buffer, because the lookup is for the directory path and buffers hold -file paths: +**(c) `recursive = true` fails in the opposite direction.** ``` recursive delete result: Ok(()) @@ -95,17 +189,16 @@ buffers before=3 after=3 inner buffer still in registry? true ``` -The most destructive arm does the least reconciliation. Here the data -survives — in an orphaned buffer pointing at a path that no longer -exists — which is strictly safer than (a) and is why the fix cannot be -"make delete behave like the recursive case". +The tree goes; no buffer is reconciled, because the lookup is for the +directory path and buffers hold file paths. The most destructive arm +does the least reconciliation. Here the data survives — in an orphaned +buffer — which is strictly safer than (a), and is why the fix must not +be "make delete behave like the recursive case". -**(d) Removal is not `kill_buffer`.** The delete path leaves a window -bound to a removed `BufferId`, and can drive the registry to empty: +**(d) Removal is not `kill_buffer`.** ``` victim is the active buffer? true -delete result: Ok(()) window.buffer():is_valid() after delete => Ok(Boolean(false)) editor.file_path() after delete => Ok(Nil) ``` @@ -115,27 +208,20 @@ delete of the LAST buffer: Ok(()) buffers after: 0 (0 => registry driven empty) ``` -`EditorCore::kill_buffer` (`src/editor_core.rs:4590`) explicitly refuses -the last buffer and rebinds every window to a fallback. This path does -neither. **(d) is named here for the record and is parked** — see §6 and -Q#RD8; it is a different failure from data loss and it is shared with -`pmacs.buffer.remove`. +`EditorCore::kill_buffer` (`src/editor_core.rs:4590`) refuses the last +buffer and rebinds every window to a fallback. This path does neither. +**(d) is parked** (§6, Q#RD8) and, per Q#RD5, must not be *widened* by +this lane. ### 1.2 There is no dirty check at any link in the chain -Three links, verified by reading each: - -1. **`apply_resource_op`'s delete arm** (`src/lua_bindings/mod.rs:3313`) - — stats the path, calls `remove_file` / `remove_dir` / - `remove_dir_all`, then `find_by_path`, then `remove_buffer_and_fire`. - No `is_modified` anywhere in the arm. -2. **`remove_buffer_and_fire`** (`src/lua_bindings/mod.rs:1592`) — four - lines: `registry.borrow_mut().remove(id)`, then - `after_buffer_removed`. `after_buffer_removed` - (`src/lua_bindings/mod.rs:1602`) clears keymaps, config, folds and - fires `on_removed` callbacks. No dirty check. -3. **`BufferRegistry::remove`** (`src/buffer_registry.rs:127`) — has - exactly one guard, and it is not this one: +1. **The delete arm** (`src/lua_bindings/mod.rs:3313`) — stats, deletes, + then `find_by_path`, then `remove_buffer_and_fire`. No `is_modified`. +2. **`remove_buffer_and_fire`** (`:1592`) — `registry.remove(id)` then + `after_buffer_removed` (`:1602`), which clears keymaps, config, folds + and fires `on_removed`. No dirty check. +3. **`BufferRegistry::remove`** (`src/buffer_registry.rs:127`) — one + guard, and not this one: ```rust if let Some(buf) = self.buffers.get(&id) @@ -148,65 +234,87 @@ Three links, verified by reading each: That refuses re-entrant removal from inside an edit intercept (T M7.4). It says nothing about unsaved content. -The user's description of the bug is **accurate at every link**. Modes -(b), (c) and (d) are additional and were not in the report. +The report's description is **accurate at every link**. Modes (b), (c) +and (d) are additional. ### 1.3 The ordering is disk-first, so no guard placed later can help The arm performs the irreversible filesystem operation **before** it has -even looked for a buffer. By the time any `is_modified` check could run, -the file is gone. This is the single most important structural fact in -this document: **a fix that adds a dirty check to the buffer-reconcile -block is not a fix.** It converts "file deleted, buffer destroyed" into -"file deleted, buffer orphaned" — mode (c) — which is better, but the -user's on-disk file is still gone and they never consented. +looked for a buffer. **A fix that adds a dirty check to the existing +buffer-reconcile block is not a fix** — it converts mode (a) into mode +(c), and the user's file is still gone. The check must happen in a phase +that precedes the filesystem call, which is what Q#RD2 introduces. -### 1.4 `is_modified` is available, singular, and not on `pmacs.buffer` +### 1.4 `is_modified` is available, singular, and the lookup around it is not - One field, `Buffer::is_modified: bool` (`src/buffer.rs:164`); one - accessor `Buffer::is_modified()` (`src/buffer.rs:473`); one public - mutator `Buffer::mark_clean()` (`src/buffer.rs:488`). There is no - `dirty`, no `saved_revision`, no public setter to true. -- **Zero features refuse or confirm on it today.** `kill_buffer` - (`src/editor_core.rs:4590`) does not check it; `editor.quit` - (`builtin/commands/default.lua:250`) has a veto hook - (`editor.before-quit`) that nothing subscribes to with a modified - check; `dired.revert` does not check it. The one behavioural consumer - is autosave's `gather` filter (`src/autosave.rs:363`), which *includes* - rather than refuses. **So this lane introduces the first refusal in the - codebase keyed on unsaved state**, and should be read as setting that + accessor (`:473`); one public mutator `mark_clean` (`:488`). + `Buffer::editing_in_progress()` (`:747`) is likewise a public getter, + so both conditions Q#RD2 needs are inspectable without mutating. +- **Zero features refuse or confirm on unsaved state today.** + `kill_buffer`, `editor.quit` (whose `editor.before-quit` veto hook has + no subscriber) and `dired.revert` all ignore it; the only behavioural + consumer is autosave's `gather` filter (`src/autosave.rs:363`), which + *includes* rather than refuses. **This lane introduces the first + refusal keyed on unsaved state** and should be read as setting that precedent. -- Lua reaches it as the userdata method `buf:is_modified()` - (`src/lua_bindings/mod.rs:1261`) and as +- **The registry lookup is singular and duplicates are reachable.** + `BufferRegistry::find_by_path` (`src/buffer_registry.rs:168`) returns + the *first* match in insertion order — its own doc says "First buffer + bound to `path`". `EditorCore::find_buffer_for_path` + (`src/editor_core.rs:935`) normalizes and then delegates to it, so it + inherits the singularity. And duplicates are creatable from public + Lua: `pmacs.buffer.find_or_open` (`src/lua_bindings/mod.rs:3162`) + dedups via `find_by_path`, but **`pmacs.buffer.from_file` (`:3112`) + does not** — it calls `create_from_bytes` unconditionally and then + `set_buffer_path`. Two `from_file` calls on one path yield two + path-bound buffers, and a clean first match hides a modified second. + This is why Q#RD6 requires a full scan rather than the existing + wrapper. +- Lua reaches modified state as `buf:is_modified()` (`:1261`) and `pmacs.describe.buffer(id).modified` (`:6359`). It is **not** a key on - the `pmacs.buffer` module table. Relevant because the batch preflight - (Q#RD3) is Lua and needs a path-keyed query it does not have today. + the `pmacs.buffer` module table, so the preflight needs a new query + (Q#RD3). ### 1.5 Who calls this, and what happens to a raised error -`pmacs.buffer.apply_resource_op` has exactly one production caller: -`apply_workspace_edit` (`builtin/runtime/lsp.lua:1301`), at `:1346`. -That function has three callers: +One production caller: `apply_workspace_edit` +(`builtin/runtime/lsp.lua:1301`), at `:1346`. Three callers of that: -| # | Call site | Origin | Error disposition | +| # | Call site | Origin | Disposition of a raise | |---|---|---|---| -| 1 | `handle_server_requests` (`lsp.lua:1815`), call at `:1836` | **server-initiated** `workspace/applyEdit` | **Swallowed silently.** The pump is driven by `pcall(handle_server_requests)` (`lsp.lua:1892`). A raise unwinds past the `pcall(pmacs.lsp.send_response, ...)` that answers the request, so the user sees nothing *and the server is never answered.* | -| 2 | LSP rename (`lsp.lua:2311`) | user, `M-x` | `apply_workspace_edit` never returns `nil` for an op failure, so a raise propagates out of the `pmacs.async` coroutine. | +| 1 | `handle_server_requests` (`lsp.lua:1815`), call at `:1836` | **server-initiated** `workspace/applyEdit` | **Swallowed.** The pump runs under `pcall(handle_server_requests)` (`:1892`); the raise unwinds past the `pcall(pmacs.lsp.send_response, ...)` that answers the request, so the user sees nothing **and the server is never answered**. | +| 2 | LSP rename (`lsp.lua:2311`) | user, `M-x` | Raises out of the `pmacs.async` coroutine — see below. | | 3 | code action apply (`lsp.lua:2373`) | user, `M-x` | as #2. | -Only path 1 is fully unattended. This matters: **the refusal cannot be -delivered by raising**, or the most dangerous path reports nothing and -hangs the server's request. That is a design constraint, not a nicety -(Q#RD7). +**Revision 2 correction.** Revision 1 called only path 1 unattended. +Paths 2 and 3 are no better: `step` (`builtin/runtime/async.lua:196`) +handles an uncaught coroutine error by calling `pmacs.error` if it +exists and `error(...)` otherwise — -Note also `src/rename.rs:25`, which documents the division of labour: -`rename.rs` parses and never mutates; Lua drives `pmacs.buffer.*` -"so the application strategy stays configurable". The application -strategy is the thing this framing is choosing. +```lua +if not ok then + if pmacs.error then + pmacs.error("pmacs.async: coroutine raised: " .. tostring(yielded)) + else + error("pmacs.async: coroutine raised: " .. tostring(yielded)) + end + return +end +``` + +— and **`pmacs.error` is undefined**: 11 call sites across `builtin/`, +zero definitions. So the fallback always runs and re-raises at the spawn +site. **No caller reliably surfaces a raise to the user.** Hence Q#RD7: +the refusal travels as a value, never as an exception alone. + +`src/rename.rs:25` documents the division of labour — `rename.rs` parses +and never mutates; Lua drives the primitives "so the application +strategy stays configurable". That strategy is what this framing picks. ### 1.6 A partial batch is already possible today — verified -The applier's loop (`lsp.lua:1340-1349`) calls `apply_resource_op` +The applier's loop (`lsp.lua:1340-1349`) calls the primitive unprotected. Two delete ops where the second raises: ``` @@ -214,80 +322,140 @@ batch result: Err(... "apply_resource_op delete: No such file or directory (os e a.txt still exists? false (false ⇒ partial batch) ``` -The first op stayed applied. So **partial application on I/O error is -the status quo**, not something a refusal would introduce. This -substantially weakens the "is a partially-applied WorkspaceEdit worse -than the data loss?" objection — the partial batch already exists and -data loss is strictly worse than a class of failure the code already -tolerates. +The first op stayed applied. **Partial application on I/O error is the +status quo**, not something a refusal introduces. Data loss is strictly +worse than a failure class the code already tolerates. -It also shows the preflight's documented contract is narrower than its -comment implies. `lsp.lua:1287-1291` says the applier "refuses to mutate +It also shows the preflight's contract is narrower than its comment +implies. `lsp.lua:1287-1291` says the applier "refuses to mutate *anything* unless every URI it touches resolves to a real file path -first". True — but URI resolution is the *only* thing preflighted. The -plan loop (`:1302-1336`) validates nothing about the filesystem or the -buffer registry. **The preflight phase exists and is the natural place -to add a second precondition** (Q#RD3). +first". True — but URI resolution is the *only* precondition; the plan +loop (`:1302-1336`) validates nothing about the filesystem or the +registry. That loop is where Q#RD3's conflict check goes. -### 1.7 The rename arm is more careful, and differently careful +### 1.7 The batch is sequential, and the protocol says so -Directly above the delete arm, `"rename"` (`mod.rs:3291`) does -`std::fs::rename`, then `find_by_path`, then -`core.borrow_mut().set_buffer_path(id, Some(to))`. It **rebinds** the -buffer and preserves its contents and its modified state. Delete -**destroys**. The asymmetry is the whole bug: rename treats the buffer -as the valuable thing and the path as the mutable attribute; delete +Claims about **the LSP specification** (3.18), not about pmacs: + +- "If resource operations are present, clients need to execute the + operations in the order in which they are provided." +- `FailureHandlingKind.Abort`: "All operations executed before the + failing operation stay executed." +- `FailureHandlingKind.TextOnlyTransactional`: "If the workspace edit + contains only textual file changes they are executed transactionally. + **If resource changes are part of the change the failure handling + strategy is abort.**" + +So the protocol itself declines to promise transactionality for exactly +the edits this lane is about. **This is the evidence that revision 1's +"whole-batch atomicity" claim was unsupportable**, and the reason Q#RD3 +now describes an early conflict check instead. Sequential execution is +also why a snapshot preflight is necessarily incomplete: an earlier op +can change the facts a later op's precondition was evaluated against. + +### 1.8 What prompting would actually cost — corrected + +Revision 1 called prompting impossible. It is not. Establishing what is +and is not true: + +**True, and verified:** +- The primitive cannot suspend. `apply_resource_op` is a synchronous + Rust closure performing its `std::fs` calls inline; there is no yield + point. A prompt therefore cannot be issued *from inside it* — the + applier would have to be restructured into a continuation chain. +- `pmacs.minibuffer.read` (`src/lua_bindings/mod.rs:13380`) is + asynchronous-by-callback, and `Minibuffer::accept` + (`src/minibuffer.rs:334`) deliberately *returns* the callback rather + than invoking it, because "firing user code from inside the minibuffer + would re-enter the registry" (`:332`). +- **The minibuffer is a single slot that replaces without asking.** + `Minibuffer::session: Option` (`src/minibuffer.rs:71`), + and `begin` (`:106`) is documented "**Replaces any existing + session**". A prompt raised mid-batch while the user has a minibuffer + open silently destroys the in-flight prompt and its callbacks. +- **There is no `y_or_n` helper in the tree** — a named deferral + (`docs/dired-framing.md:854`). + +**False, as revision 1 had it:** +- *"The server-initiated path cannot produce that answer."* It can. + `pmacs.lsp.send_response` (`src/lua_bindings/mod.rs:9680`) takes + `(server_id, request_id, result, err)` as ordinary values; nothing + binds it to the pump's call frame, and `request_id` arrives on the + event as a plain Lua value that can be stashed. A `workspace/applyEdit` + **can** be answered on a later tick. +- *"It costs a seventh dispatcher shadow."* It does not. The minibuffer + is already rung 4; a continuation reuses it. + +**So the honest case against prompting** (§2.2) is scope, not +possibility: queuing, cancellation, collision with an already-active +single-slot minibuffer, and revalidation of every precondition after the +user turn — because the world moves during the turn, which is §1.7's +problem again, only worse. + +### 1.9 Autosave cannot serve as a pre-delete backup + +Verified against `src/autosave.rs`: there is no per-buffer write entry +point (the only public writer is `sweep`, `:261`, which walks the whole +registry); `sweep` skips clean buffers (`:363`); **removing a buffer +purges its recovery file** — the `on_removed` callback registered at +`builtin/runtime/autosave.lua:167` calls `discard_buffer` (`:511`), with +a sweep-time GC backstop (`:290-306`), pinned by +`tests/autosave_acceptance.rs:702`; and deleting the file flips the +recovery to `Stale`, which is never auto-offered. + +### 1.10 Report channels — corrected + +- `pmacs.editor.set_status` (`src/lua_bindings/mod.rs:13036`) is + transient; it is cleared at the top of every `dispatch_key`. +- **Revision 2 correction: a durable surface exists.** + `LuaHost::append_to_errors_buffer` (`src/lua.rs:401`) appends to + `*errors*` (`ERRORS_BUFFER_NAME`, `src/lua.rs:32`), creating it on + first use, and is the established idiom for "a callback failed and the + user was not watching" — `log_hook_error` + (`src/lua_bindings/mod.rs:6061`), `log_statusline_provider_error` + (`:6099`), `log_buffer_removed_error`, and the config error path + (`src/lua_bindings/config.rs:511`). Revision 1 claimed no such channel + existed. It does, it is Rust-side, and Q#RD7 now uses it. + +### 1.11 pmacs advertises no `workspace.workspaceEdit` capability + +`default_client_capabilities` (`src/lsp.rs:3242`) sends `"applyEdit": +true` (`:3259`) inside its `"workspace"` block (`:3253`) but **no +`workspaceEdit` object at all**. So pmacs declares neither +`documentChanges` ("The client supports versioned document changes in +`WorkspaceEdit`s"), nor `resourceOperations` ("The resource operations +the client supports"), nor `failureHandling` ("The failure handling +strategy of a client if applying the workspace edit fails") — +`grep -rn "failureHandling"` over the tree returns **0**. + +Two consequences worth stating plainly. pmacs applies resource +operations it never declared support for; and it declares no failure +strategy, so §1.7's `Abort` semantics are the de facto behaviour by +omission rather than by choice. **Neither is fixed by this lane** — +declaring capabilities changes what servers send, which is a behavioural +change needing its own evidence (§6). It is recorded because a framing +about batch failure semantics that did not notice pmacs declares none +would be describing half the system. + +### 1.12 The rename arm is more careful, and differently careful + +`"rename"` (`src/lua_bindings/mod.rs:3291`) does `std::fs::rename`, then +`find_by_path`, then `set_buffer_path` — it **rebinds**, preserving +contents and modified state. Delete **destroys**. Rename treats the +buffer as the valuable thing and the path as a mutable attribute; delete treats the buffer as a cache of the file. -Both arms share two latent defects, already recorded against the dired -arc: `find_by_path` (`src/buffer_registry.rs:168`) is exact `Path` -equality, first match only, called with the raw path while stored paths -are normalized on write. `EditorCore::find_buffer_for_path` -(`src/editor_core.rs:935`) is the normalizing wrapper that exists and is -bypassed. See `docs/dired-framing.md:807-819` and the ledger note at -`docs/active-work.md:605`, both of which claim the **rename** side of -this for dired Stage 2. §6 draws the boundary. +Both arms share the §1.4 lookup defects. `docs/dired-framing.md:807-819` +and the dired Stage 1 entry under "Closed since the last snapshot" in +`docs/active-work.md` claim the **rename** side for dired Stage 2. §6 +draws the boundary; the dired lane is recorded as **OPEN, STALE, DO NOT +MERGE AS-IS** and under re-scout, which is why this lane does not wait +on it. -### 1.8 What pmacs cannot do, established rather than assumed - -- **A prompt cannot be issued from inside this binding.** - `pmacs.minibuffer.read` (`src/lua_bindings/mod.rs:13380`) is - asynchronous-by-callback: it registers `on_accept`/`on_cancel` and - returns immediately. `Minibuffer::accept` - (`src/minibuffer.rs:334`) deliberately *returns* the callback rather - than calling it, with the reason stated at `:332` — "firing user code - from inside the minibuffer would re-enter the registry". The answer - arrives on a later keystroke, through the event loop. - `apply_resource_op` is a synchronous Rust closure that performs its - `std::fs` calls inline and returns; there is no point at which it can - suspend. **There is no `y_or_n` helper in the tree at all** — a named - deferral in `docs/dired-framing.md:854`. -- **Autosave cannot be used as a pre-delete backup.** Verified against - `src/autosave.rs`: - - there is no per-buffer write entry point; the only public writer is - `sweep` (`:261`), which walks the whole registry; - - `sweep` skips clean buffers (`:363`); - - **removing a buffer purges its recovery file** — the `on_removed` - callback registered at `builtin/runtime/autosave.lua:167-169` calls - `discard_buffer` (`src/autosave.rs:511`), and a sweep-time GC - (`:290-306`) catches whatever the callback misses. Pinned by - `tests/autosave_acceptance.rs:702`. - - deleting the file flips the recovery's status to `Stale`, and - `Stale` is never auto-offered. - - So "back up, then delete" is self-defeating four times over: no - entry point, wrong filter, the backup is deleted by the very removal - it was protecting against, and what survives is never surfaced. -- **`pmacs.editor.set_status` is the available report channel** - (`src/lua_bindings/mod.rs:13036`), cleared at the top of every - `dispatch_key`. There is no `*Messages*` buffer and no `*warnings*` - buffer. `pmacs.error` is referenced in `async.lua` and **never - defined**. - -### 1.9 Prior art — claims about **Emacs**, not pmacs +### 1.13 Prior art — claims about **Emacs**, not pmacs Verified against `lisp/progmodes/eglot.el`, `emacs-mirror/emacs` -`master`. Everything in this subsection is a statement about Emacs. +`master`. **Eglot orders the operations the other way round.** Its `do-delete`: @@ -303,23 +471,26 @@ Verified against `lisp/progmodes/eglot.el`, `emacs-mirror/emacs` (delete-file path recursive)))) ``` -The buffer is killed **before** the file is deleted. In Emacs, -`kill-buffer` on a modified file-visiting buffer prompts, so the consent -gate sits ahead of the irreversible step by construction. (Eglot does -not check `kill-buffer`'s return value, so declining the kill still -deletes the file — but the buffer, and therefore the text, survives. -Even Eglot's failure mode is strictly milder than pmacs's.) +The buffer is killed **before** the file is deleted, and in Emacs +`kill-buffer` on a modified file-visiting buffer prompts — so the +consent gate precedes the irreversible step. (Eglot ignores +`kill-buffer`'s return value, so declining still deletes the file, but +the buffer and its text survive. Even that failure mode is milder than +pmacs's.) Note also that the `exists` guard means `ignoreIfNotExists` +does **not** fall through to the buffer kill — the asymmetry mode (b) +exposes in pmacs. -Note also the `exists` guard: Eglot's `ignoreIfNotExists` path does -**not** fall through to the buffer kill. That is exactly the asymmetry -mode (b) exposes in pmacs. +*Revision 2 note:* Emacs's ordering is **not** what Q#RD2 adopts. +Emacs can afford buffer-first because `kill-buffer` is itself the +consent gate; pmacs has no such gate, so it validates first and +reconciles last (Q#RD2), which yields the same safety without firing +callbacks against a file that still exists. **Eglot confirms server-initiated edits by default, as a whole-batch -decision made before anything is applied.** `eglot-confirm-server-edits` -defaults to `'((t . maybe-summary))`. The prepare/decide/apply structure -is explicit: `prepare` builds a list of closures touching nothing, then -`eglot--confirm-server-edits` decides, then `apply-all` runs. The -`maybe-*` decisions skip the prompt only when the batch is `peaceful`: +decision taken before anything is applied.** `eglot-confirm-server-edits` +defaults to `'((t . maybe-summary))`; `prepare` builds closures touching +nothing, then the decision, then `apply-all`. The `maybe-*` decisions +skip the prompt only when the batch is `peaceful`: ```elisp (peaceful @@ -329,279 +500,309 @@ is explicit: `prepare` builds a list of closures touching nothing, then always (find-buffer-visiting (cadddr op))))) ``` -`all-text-edits` is a conjunction over the whole batch, so **a batch -containing any create/rename/delete is never `peaceful` and always -prompts** under the default. Confirmation is all-or-nothing and -strictly precedes mutation; there is no mid-batch interrupt. +`all-text-edits` is a conjunction over the whole batch, so a batch +containing any create/rename/delete **always** prompts under the +default. -**The transferable lessons** (now claims about what pmacs should do): -the consent gate belongs *before* the irreversible step; the decision is -made over the *whole* batch during a preparation phase; and -`ignoreIfNotExists` must short-circuit the buffer half too. - -### 1.10 `apply_resource_op` has no direct test coverage +### 1.14 `apply_resource_op` has no direct test coverage `grep -rn "apply_resource_op" tests/ src/` returns **4 lines**: one doc -comment in `src/rename.rs`, and three inside the binding's own -definition in `src/lua_bindings/mod.rs`. Zero tests name it. +comment in `src/rename.rs` and three inside the binding's own +definition. Zero tests name it. -It is exercised indirectly by exactly one acceptance, +One indirect acceptance exercises it: `m4_15_workspace_edit_resource_ops_apply_in_order` (`tests/m4_acceptance.rs:4014`), driven by the `resourceops` mode of the -fake server (`src/bin/pmacs_fake_lsp.rs:834`), which emits an ordered -create / edit / rename / delete. **Its deleted file `c.rs` is never -opened in a buffer**, so the entire buffer-reconciliation half of the -delete arm is untested. That suite and that fake are where this lane's -acceptances belong. +fake server (`src/bin/pmacs_fake_lsp.rs:834`). **Its deleted `c.rs` is +never opened**, so the entire buffer-reconciliation half is untested. +That suite and that fake are where §5's pins belong. ## 2. The decision space -### 2.1 Recommended — refuse before touching disk, at both layers +### 2.1 Recommended — validate before mutating, at the primitive; conflict-check early, in the applier -**Refuse the delete when it would destroy unsaved work, and refuse it -before the filesystem call. Do this at the Rust primitive (the -invariant) *and* in the applier's existing preflight (batch -atomicity).** +**The primitive refuses before touching disk. The applier catches what +it can early, and reports honestly what it cannot.** -Concretely: +**Layer 1 — the primitive (the invariant).** The delete arm becomes four +ordered phases: -- **Layer 1, the primitive.** The delete arm reconciles the buffer - registry *first*. If the path resolves to a modified buffer, it - returns an error and touches nothing. Inverting the order is what - makes the guard possible at all (§1.3), and it also closes a smaller - hole: today a `ConcurrentEdit` refusal from `BufferRegistry::remove` - arrives after the file is already gone. -- **Layer 2, the applier preflight.** `apply_workspace_edit`'s plan loop - (`lsp.lua:1302-1336`) gains a second precondition alongside URI - resolution, and returns its existing `nil, message` when any delete op - targets a modified buffer — so **nothing** in the batch is mutated. +``` +stat / no-op decision → enumerate and validate affected buffers + → mutate the filesystem + → reconcile the registry +``` -Both layers are needed, and neither is redundant: +Validation inspects; it does not remove. If any affected buffer is +modified — or is mid-edit (`editing_in_progress`) — the op returns an +error having touched nothing. Because validation removes nothing, a +filesystem failure leaves every buffer intact automatically, and +`on_removed` still fires only in the reconcile phase, i.e. with the path +already gone, preserving today's invariant. -- Layer 1 alone leaves partial batches (§1.6): op 1 deletes, op 2 - refuses, and the user is left mid-edit with no way back. -- Layer 2 alone leaves the primitive armed. `pmacs.buffer.apply_resource_op` - is public Lua API; dired Stage 2 and any package can call it directly. - A guard that lives only in one caller is exactly the shape the project - has been burned by — "pin the guard through the real path". +**Layer 2 — the applier (early conflict check + robust reporting).** +`apply_workspace_edit`'s existing plan loop gains a modified-buffer +conflict check for delete ops and returns its existing `nil, message`. +This is a **filter, not a transaction** (§1.7): it catches the common +case cheaply, before anything is mutated, and it is honest that a +sequential batch can still refuse mid-flight. What makes mid-flight +refusal survivable is Q#RD7: each primitive call is wrapped, every +failure becomes `nil, message`, the origin buffer is restored +best-effort, and the unattended caller **always** answers the server. -**Why this beats the runners-up, in one sentence each:** +Neither layer is redundant. Layer 1 alone leaves every batch failure +reported through a channel that does not work (§1.5). Layer 2 alone +leaves the primitive armed for direct callers — `pmacs.buffer.apply_resource_op` +is public Lua API, and dired Stage 2's own plan names delete +reconciliation as its 2a substrate. -- It is the only option that is *possible* — prompting is architecturally - unavailable (§1.8) — and it is the option Emacs's structure already - endorses (§1.9): consent gate before the irreversible step, decision - taken over the whole batch during preparation. +**Why this beats the runners-up, in one sentence each:** it is the only +option that puts the check strictly before the irreversible step without +either firing callbacks into a half-changed world (buffer-first) or +inventing a recovery surface (backup), and it is the only one whose cost +is bounded by this lane. -### 2.2 Prompt the user — rejected +### 2.2 Prompt the user — rejected on scope, not on possibility -Rejected on three independent grounds, any one sufficient. +**Revision 2 rewrite.** Revision 1 argued impossibility on three +grounds; two were wrong (§1.8) and are withdrawn. The surviving argument +is narrower and is about cost: -1. **Architecturally unavailable.** `apply_resource_op` is synchronous - Rust; prompts are callback-continuations resumed by a later keystroke - (§1.8). Making this work means restructuring the applier into a - callback chain carrying the remaining plan as a closure upvalue, with - revalidation of every precondition at each resumption — a large, - independently-risky change to LSP edit application, for a guard. -2. **It costs an interaction island.** The alternative to a callback - chain is a Rust modal shadow (the query-replace shape). That is a - **seventh** dispatcher shadow. COHERENCE §6 grades this area "weak, - and growing by one island per modal feature" and records that - terminal copy mode was deliberately engineered *not* to become the - seventh. Spending that budget on an error path would be a poor trade. -3. **It cannot serve the most dangerous caller.** The server-initiated - path (§1.5, row 1) must answer `workspace/applyEdit` synchronously - with `applied: true | false`. There is no user turn available inside - it, and a prompt that resolves three keystrokes later cannot produce - that answer. +- **The applier must become a continuation chain.** The primitive cannot + suspend (§1.8), so the remaining plan has to be carried as a closure + across the user turn — with cancellation, and with **revalidation of + every precondition afterwards**, because the world moves during the + turn. That is §1.7's sequential-batch problem with a human-scale delay + inserted into it. +- **The minibuffer is a single slot that replaces without asking** + (`Minibuffer::begin`, "Replaces any existing session"). A prompt + raised while the user is mid-`M-x` destroys their in-flight prompt. + Queuing is therefore a prerequisite, and no queue exists. +- **Deferred answers need a pending-request ledger.** Answering + `workspace/applyEdit` later is possible (§1.8) but means retaining + `(server_id, request_id)` across ticks and deciding what happens if + the server dies first. `purge_dead_pending` exists for *client* + requests; there is no equivalent for held server requests. -Note that "what happens to the rest of the batch?" — the question that -makes prompting genuinely hard — is dissolved by the recommendation -rather than answered: the decision is taken in the preflight, before any -op runs, so there is no rest-of-batch to strand. +None of that is impossible; all of it is a separate lane with its own +framing. **Refusing is the correct move for a live data-loss bug**, and +a prompt can later *loosen* an unconditional refusal without either +change invalidating the other. ### 2.3 Save first, then delete — rejected Silently converts an unsaved edit into a committed one and then destroys -it. It is *more* destructive than refusing, not less: it overwrites the -user's on-disk original — the copy they might have wanted — immediately -before removing the file. It also cannot be relied on: `save_inner` -refuses when the file changed on disk since it was read -(`src/editor_core.rs:1908`, guard at `:1917`), so the fallback question is unanswered and -we are back to refusing. +it — more destructive, not less, because it overwrites the on-disk +original immediately before removing the file. It also cannot be relied +on: `save_inner` (`src/editor_core.rs:1908`) refuses at `:1917` when the +file changed on disk since it was read, so the fallback question is +unanswered and we are back to refusing. ### 2.4 Back up the contents somewhere recoverable — rejected -Rejected on evidence, not taste. §1.8 establishes that the existing -autosave machinery defeats this four ways, the decisive one being that -**removing the buffer deletes the recovery file** — the backup is -destroyed by the very operation it exists to survive. Building a -parallel side-store outside `autosave/` means inventing a second -recovery surface with its own discovery, GC and lifecycle, to make a -destructive operation *feel* safe. Refusing is cheaper and honest. +Rejected on evidence (§1.9): the decisive fact is that **removing the +buffer deletes the recovery file**, so the backup is destroyed by the +operation it exists to survive. Building a side-store outside +`autosave/` means a second recovery surface with its own discovery, GC +and lifecycle, to make a destructive operation *feel* safe. ### 2.5 Key the behaviour on LSP-versus-user provenance — rejected -Superficially attractive because Eglot's default keys on exactly this -(`eglot-confirm-server-edits`). But: `apply_resource_op` takes no -provenance argument and there is no ambient caller identity to read. -Adding one makes the primitive's safety depend on a caller-supplied -flag — any caller that omits it, or passes the permissive value, is -unguarded, which is the failure mode the whole lane exists to remove. -COHERENCE §10 (extension trust classes) is unbuilt, so there is no -existing trust dimension to key on either. **The refusal is -unconditional and provenance-blind.** A future -`lsp.confirm-server-edits` setting can *loosen* it once a prompt -mechanism exists; §6. +`apply_resource_op` takes no provenance argument and there is no ambient +caller identity. Adding one makes the primitive's safety depend on a +caller-supplied flag — any caller that omits it is unguarded, which is +the failure mode the lane exists to remove. COHERENCE §10 (extension +trust classes) is unbuilt, so there is no trust dimension to key on. +**The refusal is unconditional and provenance-blind.** ## 3. Decisions -### Q#RD1 — Refuse. Do not prompt, do not save, do not back up +### Q#RD1 — Refuse. Do not prompt, do not save, do not back up — **SETTLED YES** -An `apply_resource_op` delete whose target resolves to a modified buffer -**fails**, changing nothing on disk and nothing in the registry. -Rationale: §2.2–§2.5. This is the first refusal in the codebase keyed on -unsaved state (§1.4) and is intended as the precedent for `kill_buffer` -and `editor.quit`, which have the same gap. +A delete whose target set contains a modified buffer **fails**, changing +nothing on disk and nothing in the registry. This is the first refusal +in the codebase keyed on unsaved state (§1.4) and is intended as the +precedent for `kill_buffer` and `editor.quit`, which have the same gap. -### Q#RD2 — Reconcile the registry before touching the filesystem +### Q#RD2 — Validate before mutating; reconcile last — **REWRITTEN at rev 2** -The delete arm's order inverts: resolve the buffer set, decide, and only -then call `remove_file` / `remove_dir` / `remove_dir_all`. This is what -makes the guard expressible (§1.3) and it also moves the pre-existing -`ConcurrentEdit` refusal ahead of the irreversible step. +Four phases, in order: **stat/no-op decision → enumerate and validate +affected buffers → mutate the filesystem → reconcile the registry.** -**What inverting breaks, considered:** the current order means a -successful `remove_buffer_and_fire` implies the file is already gone, -so `on_removed` subscribers observe a consistent "file and buffer both -gone" world. After inversion, `on_removed` fires with the file still -present for the remainder of the call. Two mitigations: the fs call -follows immediately with no yield point in between (Lua callbacks run -synchronously inside `after_buffer_removed`), and if the fs call then -fails the correct end state is *ambiguous either way* — today it cannot -happen because the buffer removal is unreachable on fs failure. Q#RD9 -records the resolution. +- **Validation inspects only.** It checks `Buffer::is_modified()` and + `Buffer::editing_in_progress()` (`src/buffer.rs:473`, `:747`) across + the affected set. Nothing is removed, so nothing can be lost if a + later phase fails. +- **`editing_in_progress` moves from discovery to validation.** Today a + `ConcurrentEdit` refusal from `BufferRegistry::remove` arrives *after* + the file is gone. Checking it during validation means a delete invoked + from inside the target's own edit intercept refuses before disk. +- **`on_removed` still observes the path already gone.** Reconciliation + is the last phase, so the invariant revision 1 would have broken is + preserved. This is the specific defect revision 1's buffer-first + ordering introduced, and it is why that ordering is withdrawn. +- **A filesystem failure leaves buffers untouched**, automatically + rather than by compensation. -### Q#RD3 — The batch aborts in the preflight, before any op runs +### Q#RD3 — The preflight is an early conflict check, **not** a transaction — **DOWNGRADED at rev 2** -`apply_workspace_edit`'s plan loop gains a modified-buffer precondition -for delete ops and returns `nil, message`. This reuses the applier's -existing, documented abort contract — "aborts the whole edit cleanly, -origin buffer untouched, rather than half-applying" -(`lsp.lua:1287-1291`) — and all three callers already handle the -`nil, message` shape (§1.5). It is Eglot's prepare/decide/apply shape -(§1.9) implemented in the phase pmacs already has. +`apply_workspace_edit`'s plan loop gains a modified-buffer conflict +check for delete ops and returns its existing `nil, message`. It is +described in the code comment and here as a **filter**: -The preflight needs a path-keyed modified query, which Lua lacks today -(§1.4). The minimal addition is a `pmacs.buffer` surface answering -"is there a modified buffer at or beneath this path"; its exact shape is -an implementation choice, but it must be **one** query so the preflight -and the primitive cannot drift apart. +- **What it guarantees:** when the conflict is visible at plan time, + nothing in the batch is mutated at all, and the user gets one clear + message. +- **What it does not guarantee, stated plainly:** `documentChanges` are + sequential (§1.7). An earlier text edit can dirty a clean buffer, and + an earlier rename can move a modified buffer *into* a later delete's + subtree, after the snapshot. Then the preflight passes and the + primitive refuses mid-batch, leaving earlier operations applied — + which is `FailureHandlingKind.Abort`, the strategy the spec itself + assigns to any edit containing resource changes. +- Revision 1 called this "whole-batch atomicity" and said "nothing in + the batch is mutated". **That was false and is withdrawn.** -### Q#RD4 — `ignore_if_not_exists` short-circuits the buffer half too +The check needs a path-keyed modified query that Lua lacks (§1.4). It +must be **one** query shared with the primitive's validation phase, so +the two cannot drift apart. + +### Q#RD4 — `ignore_if_not_exists` short-circuits at **both** layers — **WIDENED at rev 2** + +When the path is absent and `ignore_if_not_exists` is set, the op is a +no-op: + +- **Primitive:** return early without touching the registry — the + `create` arm's existing idiom and Eglot's `exists` guard (§1.13). +- **Preflight:** must **not** reject the batch merely because a modified + buffer still names that absent path. Revision 1 applied this only to + the primitive, which would have made the preflight refuse an op the + primitive treats as a no-op — a refusal with no underlying + destruction, i.e. a false positive that blocks legitimate edits. -When the path is absent and `ignore_if_not_exists` is set, the arm -returns early **without** touching the registry — matching the `create` -arm's existing `return Ok(())` idiom and Eglot's `exists` guard (§1.9). Mode (b) is not a special case of the main bug; it is a missing early return, and a fix aimed only at the "we actually deleted something" branch leaves it live. -### Q#RD5 — `recursive` deletes are prefix-aware, or the guard has a documented bypass +### Q#RD5 — Recursive deletes are **inspected** tree-wide but **reconciled** exact-path — **SETTLED YES, NARROWED at rev 2** -Mode (c) proves that `recursive = true` reconciles nothing, so an -exact-path guard is bypassed entirely by the most destructive arm: a -server that sends `{kind: "delete", uri: , recursive: true}` walks -straight past it. A guard with a trivial, reachable bypass is not a -guard, so the check must cover every buffer whose path lies **beneath** -the deleted directory, not merely one whose path equals it. +Mode (c) proves `recursive = true` reconciles nothing, so an exact-path +guard is bypassed by the most destructive arm. Therefore: -**This overlaps dired Stage 2 and the boundary is drawn explicitly.** -`docs/dired-framing.md:807-819` and `docs/active-work.md:605` claim -prefix-aware, normalize-before-lookup rebinding for the **rename** side. -This lane takes the **delete** side only, because without it this lane -ships nothing. The two want the same helper; whichever lands second -adopts the first's. If the user prefers, the alternative is to sequence -this lane after dired Stage 2 and consume its helper — but the bug is -live and dired Stage 2 is unframed for this, so shipping first is -recommended. +- **Validation is prefix-aware**: every buffer whose path lies beneath + the deleted directory is inspected, and any modified one refuses the + op. Without this the guard has a trivial reachable bypass. +- **Reconciliation is not widened**: after a successful *clean* + recursive delete, descendant buffers are left exactly as today — + orphaned and clean. **Removing them now would promote mode (d)'s + dangling-window and last-buffer defects from an exact-path defect to a + tree-wide one**, which is precisely the parked lifecycle work this + lane must not expand into (Q#RD8). -### Q#RD6 — Lookups normalize; "modified" means `Buffer::is_modified` +The asymmetry is deliberate and is the point: **inspect widely, mutate +narrowly.** -The guard resolves paths through the normalizing wrapper -(`EditorCore::find_buffer_for_path`, `src/editor_core.rs:935`) rather -than raw `find_by_path`, because a lookup miss is a *silent* guard -bypass (§1.7). "Modified" is `Buffer::is_modified()` — the single -existing predicate (§1.4). No new notion of dirtiness is introduced. +**Boundary with dired.** `docs/dired-framing.md:807-819` and the ledger +claim prefix-aware, normalize-before-lookup rebinding for the **rename** +side. This lane takes the **delete** side only. Taking it now rather +than consuming dired Stage 2's helper is settled, and is supported by +the ledger's own assessment of that lane: PR #171 is **OPEN, STALE, DO +NOT MERGE AS-IS**, 153 commits behind at the last snapshot, under +re-scout. Whichever lands second adopts the first's helper. + +### Q#RD6 — The shared query scans **all** path-bound buffers — **REWRITTEN at rev 2** + +Revision 1 said the guard would use `EditorCore::find_buffer_for_path`. +**That is wrong** and is withdrawn: it normalizes but delegates to the +singular, first-match-only `find_by_path` (§1.4), and duplicate +path-bound buffers are reachable from public Lua via +`pmacs.buffer.from_file`. A clean first match would hide a modified +second — a silent guard bypass. + +The shared query therefore: + +- **scans every path-bound buffer**, returning all matches rather than + the first; +- **normalizes once** and compares normalized forms, so a raw-path + lookup cannot miss a stored normalized path; +- **matches with component-aware `Path::starts_with`**, not string + prefix — so `/tree` does not match `/tree-sibling`; +- is the **single** query used by both the primitive's validation phase + and the Lua preflight (Q#RD3). + +"Modified" is `Buffer::is_modified()`. No new notion of dirtiness. Explicitly **not** guarded: a clean buffer. A delete whose target is -open but unmodified proceeds and removes the buffer, exactly as today. -Overreach here would break `m4_15` and, more importantly, would make the -LSP's legitimate deletes fail for users who merely have the file open. +open but unmodified proceeds and removes the buffer, as today. +Overreach would break `m4_15` and would fail legitimate deletes for +users who merely have the file open. -### Q#RD7 — The refusal is reported on every path, and never by raising alone +### Q#RD7 — Failures travel as values, are always answered, and leave a durable trace — **WIDENED at rev 2** -Per §1.5, a raise on the server-initiated path is swallowed by -`pcall(handle_server_requests)` and the server is left unanswered. So: +§1.5 established that **no** caller reliably surfaces a raise. So: -- **Preflight refusal (all three callers)** returns `nil, message`. - Callers 2 and 3 already render that to `set_status`; caller 1 already - turns it into `{ applied = false, failureReason = ... }` and sends the - response. -- **Primitive refusal** still raises — it must, being a Rust binding — - but that is now a defence-in-depth path reached only by direct callers, - because the preflight catches the LSP path first. - -The message names the buffer and says what to do: save it, or use the -buffer-level command to discard. It must not be a bare errno. +- **Every primitive call inside `apply_workspace_edit` is wrapped**, and + every execution failure — refusal or I/O error — is converted to the + existing `nil, message` return. No exception escapes the applier. +- **The origin buffer is restored best-effort on the failure path too.** + Today `pcall(pmacs.buffer.find_or_open, origin)` runs only after a + successful loop; an early failure return would strand the user in + whatever buffer the last op left active. +- **The unattended caller always answers.** Path 1 must send + `{ applied = false, failureReason = ... }` in every failure case. + Today the raise unwinds past the send and the server waits forever. +- **The refusal is also recorded in `*errors*`** via the existing + Rust-side `append_to_errors_buffer` idiom (§1.10), so it survives the + status line being cleared on the next keystroke and leaves a trace on + the path the user was never watching. +- The message **names the buffer** and says what to do. Not a bare + errno. ### Q#RD8 — The window/last-buffer defects do **not** land here -Mode (d) is real and is parked (§6). Two reasons. It is a different -failure (a dangling window and an empty registry, not data loss), and it -is shared with `pmacs.buffer.remove` rather than specific to delete. +Mode (d) is real and parked (§6). It is a different failure from data +loss, and it is shared with `pmacs.buffer.remove`. -**And there is a trap that makes the obvious fix wrong.** The two removal -paths clean *disjoint* sets: `kill_buffer` handles the last-buffer -refusal, `round_trip_buffers`, side-window collapse and window rebinding, -but **not** keymaps, config, folds or `on_removed` callbacks; +**The trap that makes the obvious fix wrong:** the two removal paths +clean *disjoint* sets. `kill_buffer` handles the last-buffer refusal, +`round_trip_buffers`, side-window collapse and window rebinding, but +**not** keymaps, config, folds or `on_removed` callbacks; `remove_buffer_and_fire` handles exactly the latter and none of the -former. Neither is a superset of the other, so "just call `kill_buffer` -instead" would silently regress four cleanups. Unifying them is its own -lane with its own census. +former. Neither is a superset, so "just call `kill_buffer` instead" +would silently regress four cleanups. Unifying them needs its own census +and its own lane — and per Q#RD5 this lane must not enlarge the surface +that lane will have to fix. -### Q#RD9 — On filesystem failure after the buffer is removed, the buffer wins +### Q#RD9 — **WITHDRAWN at rev 2** -Given Q#RD2's inversion, a delete can now fail *after* the buffer is -gone. The buffer is not restored. Rationale: the buffer removal is only -reached for a clean buffer (Q#RD1), so nothing unsaved is at stake, and -re-inserting a buffer would need a new registry primitive and would -resurrect it with a fresh `BufferId` that no window, keymap or callback -refers to. The op reports the fs error as it does today. This is a -deliberate, narrow widening of the failure surface and is called out so -review can reject it rather than discover it. +Revision 1 proposed that, after a buffer-first removal, a filesystem +failure would leave the buffer unrestored. Q#RD2's phase ordering makes +the situation unreachable: nothing is removed before the filesystem +mutation succeeds, so there is no lost buffer to restore. The decision +number is retained rather than reused, so review can see it went away +rather than being renumbered. ## 4. Bets (falsifiable) - **B1 — Refusing breaks no legitimate server workflow.** A server deleting a file the user has unsaved edits in is a conflict the user - must resolve; no server needs that delete to succeed silently. - Falsified by a real server whose normal operation deletes files the - user is actively editing. -- **B2 — The preflight is the right layer for batch atomicity.** - Falsified if a `WorkspaceEdit` legitimately depends on a delete whose - precondition can only be evaluated after an earlier op runs (e.g. a - rename that moves the modified buffer out of the delete's path first). - **This is the sharpest risk in the design** and acceptance 8 pins the - behaviour so the failure is loud rather than silent. -- **B3 — Prefix-aware checking does not over-refuse.** Falsified if a + must resolve. Falsified by a real server whose normal operation + deletes files the user is actively editing. +- **B2 — Mid-batch refusal is acceptable because the protocol already + specifies it.** §1.7's `Abort` semantics are the spec's own answer for + resource-op-bearing edits. Falsified if a server is found that + requires transactional application and degrades badly under `Abort`. + Acceptance 12 pins the observable behaviour either way. +- **B3 — Prefix-aware validation does not over-refuse.** Falsified if a common workflow deletes a directory while an unrelated modified buffer sits beneath it and the refusal is judged unhelpful. -- **B4 — Inverting the order breaks no `on_removed` subscriber.** - Evidence: the fs call follows synchronously with no yield in between. - Falsified by a subscriber that stats the path. +- **B4 — Leaving clean descendants orphaned is the lesser evil** + (Q#RD5). Falsified if orphaned clean buffers after a recursive delete + prove more disruptive than the tree-wide lifecycle defect that + removing them would create. ## 5. Acceptance @@ -610,88 +811,126 @@ Each criterion states the **pre-image it must fail against**. A test that passes against its pre-image has no bite and is rejected. 1. **A delete op targeting a modified buffer refuses, and the file - survives.** Assert three things together: the call fails, the buffer - is still in the registry with its exact unsaved text, and - `path.exists()` is still true. - *Bite:* fails against `ad41cf1` unmodified (today: `Ok(())`, file - gone, buffer gone). **Asserting only that the buffer survived is - vacuous** — that is mode (c)'s behaviour, which this lane must not - ship. The `exists()` assertion is the load-bearing one. + survives.** Assert together: the call fails, the buffer is still in + the registry with its exact unsaved text, and `path.exists()` is + still true. + *Bite:* fails against `ad41cf1` unmodified. **Asserting only that the + buffer survived is vacuous** — that is mode (c)'s existing behaviour. + The `exists()` assertion carries the bite. 2. **A delete op targeting a *clean* open buffer still succeeds**, file removed and buffer removed. *Bite:* fails against an over-broad guard that refuses whenever a - buffer is open. Assert **both directions**, per the bottom-panel - lesson that a blanket rewrite passes an "everything moved" test. + buffer is open. Assert **both directions**. -3. **`ignore_if_not_exists = true` on an absent path leaves a modified - buffer intact** (Q#RD4). - *Bite:* fails against a fix that guards only the branch where the fs - delete actually ran — reproduce mode (b) exactly: file removed behind - pmacs's back first, then the op. +3. **A filesystem failure preserves the clean buffer** (Q#RD2). Force + the fs mutation to fail (e.g. a non-empty directory without + `recursive`) and assert the buffer is still present and intact. + *Bite:* fails against revision 1's buffer-first ordering, which would + have removed the buffer and then failed. -4. **`recursive = true` on a directory containing a modified buffer's - file refuses, and the whole tree survives** (Q#RD5). - *Bite:* fails against an exact-path-equality guard. Assert the inner - file still exists — not just that the buffer does, which is already +4. **`on_removed` observes the path absent** (Q#RD2). Register an + `on_removed` callback that stats the path and records the result; + assert it saw the path already gone. + *Bite:* fails against revision 1's buffer-first ordering, under which + the callback would observe the file still present. This is the pin + that keeps the phase order from silently regressing. + +5. **A delete called from inside the target's own edit intercept refuses + before disk** (Q#RD2). Assert the file still exists. + *Bite:* fails against `ad41cf1`, where `ConcurrentEdit` is discovered + only at removal time — after `remove_file` has already run. + +6. **Duplicate path-bound buffers cannot hide a modified copy** (Q#RD6). + Create two buffers on one path via `pmacs.buffer.from_file`, leave + the first clean and modify the second, then delete. + *Bite:* fails against any first-match lookup, including + `EditorCore::find_buffer_for_path` — which is exactly what revision 1 + specified. + +7. **Component-prefix false positives are rejected** (Q#RD6). A modified + buffer under `/tree-sibling` must **not** block a recursive delete of + `/tree`. + *Bite:* fails against a string-prefix implementation. Pairs with + criterion 8 so both directions of the prefix rule are pinned. + +8. **`recursive = true` over a directory containing a modified buffer's + file refuses, and the whole tree survives** (Q#RD5). Assert the inner + file still exists — not merely that the buffer does, which is already true today (mode (c)). + *Bite:* fails against exact-path-equality validation. -5. **Whole-batch atomicity: a `documentChanges` whose *second* op is a - blocked delete leaves the *first* op unapplied** (Q#RD3). - *Bite:* fails against a primitive-only fix. §1.6 verified that op 1 - currently stays applied, so this is a real behaviour change and the - assertion must name op 1's effect (e.g. a file that must still exist, - or must not yet have been created). +9. **A clean recursive delete leaves descendant buffers orphaned, not + removed** (Q#RD5). Assert the descendant buffer is still in the + registry after a successful recursive delete. + *Bite:* fails against an implementation that widens reconciliation to + the tree. This pin exists specifically to stop the parked defect from + being enlarged, and it is expected to look odd — it asserts today's + imperfect behaviour deliberately. -6. **The refusal reaches the server on the unattended path.** Drive a - server-initiated `workspace/applyEdit` carrying a blocked delete and - assert the server receives a response with `applied = false` and a - non-empty `failureReason`. - *Bite:* fails against a fix that refuses by raising — §1.5 established - that `pcall(handle_server_requests)` (`lsp.lua:1892`) swallows the - raise and the response is never sent. This is the "pin the guard - through the outermost user-reachable seam" obligation; a direct-call - test on `apply_resource_op` does not satisfy it and is rejected as - insufficient for this criterion. +10. **`ignore_if_not_exists = true` on an absent path leaves a modified + buffer intact** (Q#RD4), reproducing mode (b): file removed behind + pmacs's back first, then the op. + *Bite:* fails against a fix guarding only the branch where the fs + delete actually ran. -7. **The user-initiated paths report on the status line.** LSP rename - and code action each surface a message naming the buffer. - *Bite:* fails against a fix that returns `nil` without a message, or - whose message is a bare errno. +11. **Absent-plus-ignore succeeds through the real server pump** + (Q#RD4, layer 2). Drive it end to end and assert the batch is + **not** refused and the server is told `applied = true`. + *Bite:* fails against a preflight that rejects on the presence of a + modified buffer without consulting `ignore_if_not_exists` — the + false-positive Q#RD4 exists to prevent. -8. **The B2 risk is pinned:** a batch that renames the modified buffer's - file *and then* deletes the old path is refused by the preflight - rather than silently mis-evaluated. Whichever behaviour review - chooses, it is asserted, so the limitation is a documented decision - rather than an accident. +12. **Edit-then-delete and rename-into-delete still answer the server** + (Q#RD3, Q#RD7). Two batches that defeat the snapshot preflight: one + where an earlier text edit dirties the buffer a later op deletes, + one where an earlier rename moves a modified buffer into a later + delete's subtree. Assert in both cases that the server receives + `applied = false` with a non-empty `failureReason`. + *Bite:* fails against `ad41cf1` (the raise is swallowed at + `lsp.lua:1892` and no response is sent) **and** against a + preflight-only fix that claims atomicity — these are the cases + revision 1's atomicity claim asserted could not happen. -9. **`m4_15_workspace_edit_resource_ops_apply_in_order` stays green** - unmodified, pinning "no regression to the ordered-resource-op path" - from outside. Its `c.rs` is never opened, so it exercises exactly the - unguarded case that must keep working (Q#RD6). +13. **The refusal reaches the server on the unattended path**, and the + user-initiated paths report on the status line naming the buffer. + *Bite:* fails against a fix that refuses by raising. A direct-call + test on `apply_resource_op` does **not** satisfy this and is + rejected as insufficient — the guard must be pinned through the + outermost user-reachable seam. -10. **Every new test is checked with `scripts/bite`** and none reports +14. **`m4_15_workspace_edit_resource_ops_apply_in_order` stays green + unmodified**, pinning no-regression from outside. Its `c.rs` is + never opened, so it exercises exactly the unguarded case that must + keep working (Q#RD6). + +15. **Every new test is checked with `scripts/bite`** and none reports VACUOUS. ## 6. Parked — not deferred-and-forgotten - **Mode (d): the dangling window and the emptiable registry** (§1.1, - Q#RD8). Real, reachable, shared with `pmacs.buffer.remove`. Needs its - own lane and a census, because the two removal paths clean disjoint - sets and the obvious unification regresses four cleanups. -- **`kill_buffer` and `editor.quit` have the same gap** (§1.4): both - discard unsaved work with no check. `editor.before-quit` exists as a - veto channel with no subscriber. This lane sets the precedent; those - are separate lanes. -- **A `y_or_n` helper**, and with it any confirm-instead-of-refuse - option. Already a named deferral (`docs/dired-framing.md:854`). + Q#RD8). Needs its own lane and a census, because the two removal paths + clean disjoint sets and the obvious unification regresses four + cleanups. Q#RD5 is written to avoid enlarging it. +- **`kill_buffer` and `editor.quit` have the same gap** (§1.4). + `editor.before-quit` exists as a veto channel with no subscriber. This + lane sets the precedent; those are separate lanes. +- **Declaring `workspace.workspaceEdit` capabilities** — `documentChanges`, + `resourceOperations`, `failureHandling` (§1.11). Declaring them changes + what servers send, so it needs its own evidence and its own lane. +- **`pmacs.error` is undefined** (§1.5) — 11 dead call sites in + `builtin/`, including the one that is supposed to surface every + uncaught async coroutine error. A known standing defect, widened in + relevance by this framing but not fixed by it. +- **A `y_or_n` helper**, minibuffer queuing, and a held-server-request + ledger — the three prerequisites that would make §2.2 cheap rather + than merely expensive. - **`lsp.confirm-server-edits`**, the config-registry adopter that would - let a user loosen Q#RD1 once a prompt mechanism exists (§2.5). + let a user loosen Q#RD1 once a prompt mechanism exists. - **The rename side of prefix-aware, normalizing lookup** — dired Stage 2's, per Q#RD5. -- **A general transient-keymap layer** (COHERENCE §6), the prerequisite - that would make §2.2 cheap rather than impossible. ## 7. Gates @@ -702,28 +941,43 @@ Full suite per `CLAUDE.md`: `cargo fmt --check`; `cargo clippy suites; `cargo test --test m4_acceptance -- --skip basedpyright`; `PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu`; `git diff --check`. -Touched suites: **`m4_acceptance`** (the resource-op home, §1.10) and -`lsp_dispatch_seams_acceptance`. `dired_acceptance` is a watch item for -Q#RD5's shared lookup change. +Touched suites: **`m4_acceptance`** (the resource-op home, §1.14) and +`lsp_dispatch_seams_acceptance`. `dired_acceptance` and +`autosave_acceptance` are watch items — the former for Q#RD6's shared +lookup, the latter because `on_removed` ordering (acceptance 4) is where +autosave's `discard_buffer` hangs. Gate the pushed tree, not the worktree — commit first, then gate. -This framing-only PR ships no runtime code, so its own gate is -`git diff --check` plus a docs read. +While the document is still PROPOSED the gate is `git diff --check` plus +a docs read; the full suite runs once implementation lands on this +branch. ## 8. Branch plan `resource-op-delete-guard`, worktree `../pmacs-resource-op-delete`, one -PR. **This framing is the entire first PR.** Implementation does not -begin until the user approves the design — specifically Q#RD1 (refuse -rather than prompt), Q#RD5 (take the delete side of prefix-awareness now -rather than sequencing behind dired Stage 2), and Q#RD9 (the buffer is -not restored on fs failure). +branch, **one PR — #186, which becomes the implementation PR.** +Revision 2 withdraws revision 1's two-PR plan, which conflicted with +one-feature/one-branch/one-PR: the framing is revised in place, and once +approved the implementation commits land on this same branch. -Files this lane will touch when approved: `src/lua_bindings/mod.rs` -(the delete arm and a modified-at-or-beneath query), -`builtin/runtime/lsp.lua` (the preflight), `tests/m4_acceptance.rs` and -`src/bin/pmacs_fake_lsp.rs` (a fake mode carrying a blocked delete). -It will **not** touch `src/daemon.rs`, `pmacs-protocol/`, or -`builtin/runtime/dired.lua`. No protocol change. +**Implementation does not begin until the user approves this revision.** + +Files the implementation will touch: `src/lua_bindings/mod.rs` (the +delete arm's four phases and the shared all-buffers query), +`builtin/runtime/lsp.lua` (the conflict check, per-op wrapping, origin +restore, always-answer), `tests/m4_acceptance.rs` and +`src/bin/pmacs_fake_lsp.rs` (fake modes for the blocked delete, the +edit-then-delete and rename-into-delete batches, and absent-plus-ignore). +It will **not** touch `src/daemon.rs`, `pmacs-protocol/`, +`builtin/runtime/dired.lua`, `docs/agent-handoff.md` or `COHERENCE.md`. +No protocol change. + +**Ownership note.** `docs/active-work.md` records that dired Stage 2a — +"rename/delete reconciliation substrate" — overlaps +`builtin/runtime/lsp.lua` and warns against running it concurrently with +other work touching those files "without assigning those files to one +lane first". This lane claims the **delete** half of that substrate and +`builtin/runtime/lsp.lua`'s applier for its duration; the lane entry in +`docs/active-work.md` records the claim.