Merge pull request #171 from levineuwirth/dired-stage2-framing

docs: framing for dired Stage 2 (marks and operations)

docs/active-work.md

@@ -586,57 +586,213 @@ has **no branch and no framing yet**.
   `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 AT REVISION 9, AWAITING APPROVAL
 
-- Portable branch: `githubsucks/dired-stage2-framing` (head `ab42a79`,
+- Portable branch: `githubsucks/dired-stage2-framing`;
-  four framing commits); worktree `../pmacs-dired-stage1`. **PR #171**,
+  worktree `../pmacs-dired-stage1`. **PR #171**, base `main`, integrated
-  base `main`. Framing only — `docs/dired-stage2-framing.md`, 1,570
+  up to canonical `main` @ `7586905`. Framing only —
-  lines, no runtime code.
+  `docs/dired-stage2-framing.md`, **3,844 lines measured at this
-- **Measured 2026-07-28: 4 commits ahead of `main`, 153 behind**, merge
+  revision**, no runtime code. This lane rides that PR — it has since
-  base `c8ec8f3`. GitHub reports it mergeable, and its old CI run is
+  rev 6, when #185 merged; do not open a standalone ledger PR.
-  green — **both facts are about a tree nobody has looked at in 153
+- **Status: PROPOSED, never approved.** Revision 5 was reviewed and NOT
-  commits**, and the document still says PROPOSED.
+  approved (six findings, four P1); revision 6 answered them; **revision
-- **The commit history embodies three review rounds. That is not the
+  7 resolved a cross-lane conflict with PR #186**; revision 8 was
-  same as approval**, and GitHub records no formal review or comment on
+  reviewed and NOT approved (six findings, four P1); revision 9 answers
-  it. Do not read the round count as a green light.
+  them. The framing is at revision 9 after seven review rounds; **that
-- **Its dependencies moved materially underneath it**, which is the real
+  is not the same as approval**, and it must not be read as one.
-  reason not to merge. Note that dired Stage 1 (#165) and find-file
+- **Round 8 (rev 9) made every rev-8 correction singular and
-  (#162) are its *base*, not new arrivals — the merge base `c8ec8f3`
+  executable:**
-  **is** #165's merge commit. Eighteen PRs landed after it, and at least
+  - Q#DR26 no longer retains the withdrawn execution-order claim; it
-  three change ground the framing stands on:
+    owns only the homogeneous enum/vector shape and delegates sequence
-  - **#178 gave generated buffers a write invariant**
+    semantics to Q#DR29: **bus arrival, not filesystem execution**.
-    (`Buffer::set_generated_contents`). Dired's listing is a generated
+  - 2a owns acceptance **23–38 and 50–55, including every suffixed
-    buffer, and dired is named in the handoff as one of the writer
+    item**, so rev 8's new tests cannot fall between implementation PRs.
-    mechanisms that has **not** adopted it. Stage 2's marks and
+  - The LSP gate is one manager-owned, generation-scoped
-    operations write that buffer constantly.
+    `HashSet<(LspServerId, String)>`. Both
-  - **#182 (Journey Stage 1a) made `resolve_target_buffer` the single
+    `absorb_publish_diagnostics` and the now-server-aware
-    directory-open path**, with dired demoted to a *replaceable slot*
+    `mark_document_stale` check the exact pair; `did_open` clears one
-    (`pmacs.path.directory_handler`) rather than a hook subscriber. Any
+    pair, and server teardown clears all and only that server's pairs.
-    Stage 2 claim about how a directory reaches dired is now describing
+    It is reclaimed, not falsely called size-bounded. Acceptance covers
-    a mechanism that no longer exists in that form.
+    both writers and both reclamation paths.
-  - **#179/#181 landed the typed-edit consumer chain**, which is the
+  - Buffer-name following now uses explicit
-    fan-out a rename transaction has to survive.
+    `BufferNameOrigin::{Explicit, PathDerived}` state. The acceptance
+    collision is a file `${cwd}/notes` explicitly named `notes`, which
+    defeats rev 8's path-equivalence heuristic.
+- **Round 7 (rev 8) found five contract defects of ONE family, and the
+  family is the transferable lesson: a guarantee assumed from a
+  mechanism whose implementation was never read.** Reply order assumed
+  to be execution order; a refusal assumed to be inert; `forget_uri`
+  assumed to cover every writer. Three of the five were literally that.
+  The decisions:
+  - **Reply order is NOT execution order.** `AsyncRuntime::tick`
+    (`src/async_runtime.rs:1003`) is `while let Ok(env) =
+    self.main.try_recv()` — a bus drain with **no execution token**, so
+    a worker can finish, be descheduled before sending, and have a later
+    reply arrive first. The ordering guarantee was **withdrawn, not
+    engineered**: reconciliation is order-independent, and `fs.lua:155-165`
+    already recommends serialization. Rev 9 makes the public contract
+    exact: overlapping mutations dispatch serially, awaiting one before
+    the next. A lock was rejected — it would serialize every fs mutation
+    for a hazard absent from shipped in-tree callers.
+  - **`EditorCore::kill_buffer` damages editor state BEFORE it can
+    refuse.** It clears `round_trip_buffers`, closes side windows and
+    redirects ordinary windows, and only then can
+    `BufferRegistry::remove` return `ConcurrentEdit`. So "refused" is
+    not "nothing happened". Fixed by preflighting `editing_in_progress`
+    — sound because phase 1 is pure `EditorCore` and holds no Lua
+    handle, so nothing can re-enter between check and removal.
+  - **Buffer names are set from `path.display()` AS GIVEN** while only
+    `file_path` is normalized (`src/editor_core.rs:917`,
+    `src/lua_bindings/mod.rs:3112`), so a relative open leaves a short
+    name that a string-equality rule mistakes for user-chosen. Rev 8
+    moved to path-equivalence; rev 9 replaces that inference with
+    explicit provenance because a custom name can equal the path.
+  - **`diag_store` has ZERO correlated writers**, and
+    `DiagnosticStore.by_uri` is keyed by **URI alone** with no server
+    component (`src/diag.rs:198`) — so the store most needing protection
+    is the one a `pending_routes` purge cannot help. Rev 8 incorrectly
+    inferred that the tombstone must share the store's URI-only key;
+    rev 9 gates in `LspManager`, where `sid` is still available.
+  - **A first pass of rev 8 checked only `handle_notification` and
+    concluded `publishDiagnostics` was the only uncorrelated writer.
+    Wrong lens boundary** — `pub fn mark_document_stale`
+    (`src/lsp.rs:3108`) takes **no `LspServerId`** and creates URI keys
+    across three stores for every server. Rev 9 changes that surface and
+    its private Lua binding to `(sid, uri)` and pins it independently.
+- **Both new hooks are `all-must-succeed`, not short-circuit** — a gap
+  the review did not raise and the sweep found. `src/hook.rs` defines
+  three `HookKind`s; registered short-circuit, one `resource.renamed`
+  subscriber returning falsey would silently stop every later
+  subscriber reconciling.
+- **CROSS-LANE SPLIT WITH PR #186 — settled, recorded verbatim so the
+  two lanes cannot diverge again:**
 
-  Re-scout against `6bee09d`, publish a new revision, and get explicit
+  > #186 owns the urgent **pre-filesystem refusal** for synchronous
-  framing approval before any implementation. **The re-scout is under
+  > `apply_resource_op`. #171 later owns **full post-delete lifecycle
-  way** on the existing branch, so PR #171 keeps its three-round
+  > reconciliation**, including the **async race where a buffer becomes
-  history; the product is a revision 5, not a new document. (`main` has
+  > modified after dired dispatch**. #171's revision 7 adopts the
-  since advanced to `0442d78`, but the only difference is the test-only
+  > refusal and stops saying LSP intentionally deletes modified files.
-  #174, so no re-scout conclusion turns on it.)
+
-- **The rename problem the framing must still answer**, restated because
+  **PR #186 (`resource-op-delete-guard`) owns the pre-filesystem
-  it is the hard part: a rename is a transaction across **five** path
+  refusal.** Do not implement a competing dirty check from #171. Rev 7
-  owners — the buffer path, the buffer name, the URI-keyed LSP stores
+  adopts #186's Q#RD1 (refuse, do not prompt, do not save, do not back
-  plus `DiagnosticView` (whose URI is set once at construction), dired's
+  up) and its Q#RD6 shared query (scan every path-bound buffer, not
-  pathless handles, and a captured Lua local that no transaction can
+  first-match; normalize once; component-aware `Path::starts_with`);
-  reach.
+  whichever lane lands first owns that query and the other adopts it.
+  The two fit deliberately: #186's Q#RD5 keeps reconciliation
+  **exact-path** so it does not promote the dangling-window and
+  last-buffer defects tree-wide, and #171's Q#DR27 is the lane that
+  makes widening safe.
+- **What rev 7 removed.** Rev 6 said an LSP-authored delete removes the
+  file and orphans the modified buffer as accepted residue. That is
+  withdrawn — **20 matched lines across 13 passages** were censused and
+  reclassified: 4 ownership claims reassigned to #186, 4 policy claims
+  deleted, 3 ground-truth statements kept but attributed, 2 async-race
+  claims kept and narrowed.
+- **What rev 7 kept, and it is now one path not two.** The async race —
+  a buffer modified between dired's pre-dispatch check and
+  `remove_blocking`'s syscall — survives #186's refusal, because **dired
+  never goes through `apply_resource_op`**: it calls `pmacs.fs.remove`,
+  which dispatches a worker no in-applier guard can see. Rev 6 carried
+  this as "two paths, LSP and dired"; after #186 the LSP path has no
+  residue at all.
+- **A gap neither lane closes, named in rev 7 §11:** `pmacs.fs.remove`
+  is public Lua API with **no dirty check of its own**. After both lanes
+  land the guards sit one layer above it on each side — at
+  `apply_resource_op` (#186) and in dired's policy layer (#171) — so a
+  third caller inherits neither. Latent, not live: it has zero
+  production callers today.
+- **LSP failure-handling, verified against the 3.18 spec** because #186
+  got it wrong and this lane was checked for the same error. **This lane
+  never made the claim** — it asserts nothing about `WorkspaceEdit`
+  recovery. For the record: only `textOnlyTransactional` degrades to
+  abort for resource operations; `transactional` covers them; and the
+  specification states **no default** for a client that does not
+  advertise `failureHandling` (pmacs advertises none). So "Abort applies
+  by omission" does not follow.
+- **Round 5's single theme, worth carrying because it will recur:** rev
+  5 changed the slice split and the ownership of a decision, and the
+  prose did not follow. Four of the six findings were that same defect
+  in different places — stale slice labels, an acceptance allocation
+  that contradicted the code split, and a PR body still describing
+  revision *1*'s two-slice plan. **When a cut changes, sweep for the
+  label, do not patch the instance.** Rev 6's own sweep found four more
+  the review had not cited, one of which was wrong in both halves
+  (`x`'s report tagged 2a, `remove_dir_all` tagged 2b).
+- **Rev 4 was scouted at `c8ec8f3`, which is dired Stage 1's own merge
+  commit (#165)** — so dired Stage 1 and find-file (#162) were always
+  its base, not new arrivals. Eighteen PRs landed on top before the
+  re-scout. Rev 5 is that re-scout, against `6bee09d` and then
+  integrated forward.
+- **Seven of rev 4's own claims about pmacs were wrong**, which is the
+  lane's most transferable lesson: a framing can verify all its external
+  facts and still be wrong about the codebase it is for. The load-bearing
+  one: §5 named `drain_external_cancelled` but cited `lsp.rs:1596`.
+  **Those are two different functions 35 lines apart** —
+  `drain_external_cancelled` (`:1561`) is the unconditional
+  server-scoped drain and is the right precedent;
+  `drain_cancelled_externals` (`:1596`) is a per-tick token/timeout
+  sweep. A rename flips no token, so following the line number yields a
+  `forget_uri` whose drain half is a silent no-op and the awaiting
+  coroutine hangs forever — exactly the failure that step exists to
+  prevent.
+- **The rename path-owner census is SIX, not five.** The sixth is
+  `lean.lua`'s `M.file_progress`, a URI-keyed Lua module table living in
+  no Rust store, so `forget_uri` structurally cannot reach it. That is
+  the first case outside dired proving the **hook**, not the Rust
+  method, is the mechanism that scales. The other five: buffer path,
+  buffer name, the URI-keyed LSP stores plus `DiagnosticView` (URI set
+  once at construction), dired's pathless handles, and a captured Lua
+  local no transaction can reach.
+- **The journey ratchet is split across two files**, and both are gates:
+  `tests/journey_acceptance.rs` (24 tests, "stages add rows, none
+  removes them", seven rows asserting on dired) and
+  `gpu_invocation_acceptance.rs`, where #183 actually put the GPU row.
+  A scout that checks only the first will think #183 added nothing.
+- **`open_directory` now commits under `pmacs.window.commit_to`, whose
+  scope REFUSES an `await`** (#182). That directly constrains the
+  serialize-and-await batch contract the framing proposes.
+- **The typed-edit chain does not reach dired** — verified rather than
+  assumed. Its lessons bind the framing's two *new* hooks instead.
+- **Scope MOVED OUT of this lane, and rev 6 records the handoff.**
+  Rev 5 added Q#DR25 to adopt `set_generated_contents` at the head of
+  2b, because dired's listing is a generated buffer whose paint bypasses
+  an intercept over a still writable rope. That turned out to be a
+  **class bug, not dired's** — the same idiom is in listview, compile
+  and search/grep, and **no Lua caller anywhere sets `read_only`**
+  because there is no Lua `set_read_only` to call — so it is owned by
+  the generated-buffer immutability lane. **Done: rev 6 withdraws
+  Q#DR25**, and §3.1 hands that lane what this one's re-scout found,
+  including the trap that
+  `dired_buffer_is_read_only_and_round_trips_input` passes **either
+  way** and is therefore not coverage of the adoption. Do not implement
+  Q#DR25 from this document.
+- **Two substrate facts rev 6 added that outlive this lane.** (1) Buffer
+  removal is **two phases** and **no existing Rust path composes them**:
+  `EditorCore::kill_buffer` (`src/editor_core.rs:4590`) does windows and
+  registry, `after_buffer_removed` (`src/lua_bindings/mod.rs:1602`) does
+  keymaps/config/folds/`on_removed`, and only the Lua binding
+  `pmacs.buffer.kill` (`mod.rs:5476-5491`) runs both — while
+  `apply_resource_op`'s delete uses `remove_buffer_and_fire`, which
+  skips phase 1, **so a window displaying the deleted buffer keeps a
+  removed id**. That is a third defect on that arm beside the missing
+  dirty check and the first-match lookup. (2) The drain outcome must
+  carry deletions as well as renames in one `Vec<ResourceOp>` so every
+  kind is expressible, but its sequence is **bus-arrival order, not
+  filesystem execution order**; Q#DR29 requires callers with
+  interdependent mutations to serialize.
 - Intended serial implementation once approved: **2a** rename/delete
   reconciliation substrate with no dired UI, **2b** marks and
   operations, **2c** mkdir/copy/recursive-delete primitives, then Stage
-  3 wdired.
+  3 wdired. The re-scout re-examined this cut and it holds unchanged.
 - **Ownership warning:** dired 2a overlaps `src/editor_core.rs`,
   `builtin/runtime/lsp.lua`, and the URI-keyed LSP state with other
   coherence work. Do not run it concurrently with Journey Stage 1b
   without assigning those files to one lane first.
+- **Two live bugs on `main` this lane confirmed but does NOT fix**, both
+  now owned elsewhere: `apply_resource_op`'s delete arm destroys unsaved
+  work (no dirty check anywhere on the path), and `View` still lacks
+  `rename_resource`.
 
 ## Parked lane: kill-ring browser + persistence