Merge pull request #186 from levineuwirth/resource-op-delete-guard

docs(framing): guard the resource-op delete arm against data loss [rev 5 — PROPOSED, do not merge]
This commit is contained in:
Levi Neuwirth 2026-07-29 01:16:28 +00:00 committed by GitHub
commit 0f4e9e0ed2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 1776 additions and 0 deletions

View File

@ -459,6 +459,133 @@ has **no branch and no framing yet**.
`FrontendView.fold_projection` to `true` for semantic frontends, which `FrontendView.fold_projection` to `true` for semantic frontends, which
Stage 2 deliberately left `false` (Q#FD21). 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`. Currently
framing only — `docs/resource-op-delete-guard-framing.md`, **revision
5** — plus this lane entry. No runtime code yet.
- **Measured 2026-07-28, `main` @ `7586905`:**
```
$ git rev-list --left-right --count HEAD...githubsucks/main
5 0
```
Five commits ahead, **0 behind** at the pushed revision-5 head. This
count includes the revision commit itself; revision 4 recorded the
pre-commit count and was therefore one short.
- **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 5 closes round
4's two contract P1s and the ledger-ownership P1. Still PROPOSED,
still not approved for implementation.** Settled: refuse
unconditionally; take the delete side now. Withdrawn: rev 1's
buffer-first ordering. The design is `stat/no-op/refuse → enumerate
and validate → mutate filesystem → reconcile`, which keeps
`on_removed`'s "path already gone" invariant and makes a failed
deletion leave buffers intact automatically.
- **The stable cross-lane ownership split with #171:**
> #186 owns the urgent **pre-filesystem refusal** for synchronous
> `apply_resource_op`. #171 later owns **full post-delete lifecycle
> reconciliation**, including the **async race where a buffer becomes
> modified after dired dispatch**.
#186 additionally **owns the shared walk query** (scan every
path-bound buffer, normalize once, component-aware `Path::starts_with`)
under the boundary's "whichever lands first owns the query"; #171
adopts it and extends it to `reconcile_rename`. **Neither lane guards
`pmacs.fs.remove`** — zero production callers today, named out of
scope by both.
- **#171 owns its own lane entry.** Revision 4 rewrote that sibling
block and was stale before push when #171 revision 8 landed 67 seconds
earlier. Revision 5 restores the block to `main`'s tree, so #186's
diff no longer changes it. The one fact this lane depends on is the
policy split above, which is stable through #171's pushed revision 8
and independent of its commit count.
- **Standing rule this lane learned the expensive way.** A census is a
reading, not a constant. **Do not write an ahead/behind count, a line
count, or a call-site count into this file that you have not just
produced with a command whose output you can paste.** #186 shipped a
stale line count, then a stale commit count, then a stale ledger
citation, in three consecutive revisions — each time by carrying a
measurement across a base change instead of re-running it. The
specific trap: a count taken against `ad41cf1` was reported in
present tense after `main` had moved to `7586905`, which silently
converted "0 behind" into a falsehood.
- **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 in-repo:
two delete ops, the second raises, the first stayed applied. Any
framing claiming batch atomicity here is wrong. **Do not justify
this from the LSP spec.** Revision 2 of #186 wrote that LSP 3.18
"assigns `FailureHandlingKind.Abort` to resource-op-bearing edits";
**it does not** — recovery is described by the client's advertised
`workspace.workspaceEdit.failureHandling`, `Abort` is one of four
strategies, only `TextOnlyTransactional` degrades to abort for
resource changes, and **pmacs advertises none of them**. The
justification is repository evidence plus the judgement that a
visible partial refactor beats unrecoverable unsaved work.
- **`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, concretely.** For this lane's duration #186 owns:
the pre-filesystem refusal inside synchronous `apply_resource_op`; the
**shared walk query**; and `builtin/runtime/lsp.lua`'s
`apply_workspace_edit` plus the `workspace/applyEdit` server-request
boundary. It does **not** own: full post-delete lifecycle
reconciliation, the dired async race between dispatch and
`remove_blocking`, the rename side of the walk, or `pmacs.fs.remove`.
Do not run the two lanes concurrently over `builtin/runtime/lsp.lua`
without re-splitting that claim.
- **Two residues #186 deliberately leaves for #171**, both named rather
than silent: after a successful *clean* recursive delete, descendant
buffers stay orphaned-and-clean (widening removal would promote the
dangling-window/empty-registry defect from exact-path to tree-wide);
and after a successful delete with several clean duplicates on one
path, only the first is reconciled. #186 validates **every** match but
reconciles **one**, which is today's behaviour preserved on purpose.
- Files the implementation will touch: `src/lua_bindings/mod.rs`,
`builtin/runtime/lsp.lua`, `tests/m4_acceptance.rs`,
`tests/lsp_dispatch_seams_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 ## dired Stage 2 framing lane — PR #171 OPEN, STALE, DO NOT MERGE AS-IS
- Portable branch: `githubsucks/dired-stage2-framing` (head `ab42a79`, - Portable branch: `githubsucks/dired-stage2-framing` (head `ab42a79`,

File diff suppressed because it is too large Load Diff