docs: dired Stage 2 framing rev 7 — cross-lane reconciliation with #186
PR #186 frames a guard for apply_resource_op's delete arm that refuses
before touching disk when any affected buffer is modified. Rev 6 said the
opposite for the same call site: the file is deleted, the modified buffer
survives orphaned, and §11 named that orphaning as accepted residue. Two
lanes, two answers.
Rev 7 adopts the settled split, recorded verbatim in the framing and in
the ledger lane so the two cannot diverge again: #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. Not re-litigated.
Census of orphaning claims: 20 matched lines resolving to 13 distinct
passages, classified by reading each enclosing passage rather than the
matched line. 4 ownership claims (this lane closes the LSP data-loss
defect) reassigned to #186; 4 policy claims (LSP intentionally deletes
and orphans) deleted; 3 ground-truth statements kept, because main really
does destroy unsaved work on that arm, but attributed to #186 as fixer;
2 async-race claims kept and narrowed.
Q#DR18 is rewritten, not withdrawn. The reconcile_delete seam survives
and is explicitly not cancelled. Three changes: the synchronous path
refuses before disk (#186 Q#RD1, adopted not re-decided); the policy
becomes symmetric rather than asymmetric; and the walk rule is adopted
from #186's Q#RD6 rather than reinvented — scan every path-bound buffer,
normalize once, component-aware Path::starts_with — which is
character-for-character what rev 6 wrote independently. Whichever lane
lands first owns the query, per #186's own boundary wording.
DeleteReconcile.kept_modified stays and is not dead. The synchronous
caller can no longer produce it; the asynchronous one still can.
Residue kept, and narrowed to one path: dired never goes through
apply_resource_op — it calls pmacs.fs.remove, which dispatches a worker
that an in-applier guard cannot see. So the LSP path has no residue and
dired's is its own. Rev 6 called it "one deferral, two paths"; that was
backwards. Also newly named: pmacs.fs.remove itself is guarded by
neither lane, since both guards sit one layer above it. Latent — zero
production callers.
The full post-delete lifecycle stays here, and #186 hands it over
explicitly rather than by omission: its Q#RD8 parks the window and
last-buffer defects and independently reaches rev 6's R4 finding that
kill_buffer and remove_buffer_and_fire clean disjoint sets; its Q#RD5
keeps reconciliation exact-path precisely so as not to promote those
defects tree-wide. That parked work is this lane's Q#DR27.
LSP failure handling: this document never made the claim — grep returns
10 lines, all about hook fan-out or dired's batch semantics, none about
WorkspaceEdit recovery. Verified the spec anyway and recorded it, since
the two framings will sit side by side: only textOnlyTransactional
degrades to abort for resource ops, transactional covers them, and LSP
3.18 states no default for a client advertising no failureHandling.
Rev 6's one adjacent claim — "refusing mid-edit leaves a half-applied
refactor" — was the load-bearing support for the deleted policy and goes
with it.
Noted without re-litigating: #186's Q#RD5 cites the ledger's "OPEN,
STALE, 153 commits behind, under re-scout" assessment of #171 in support
of taking the delete side now. That citation is stale — the re-scout is
done and this lane is integrated to ad41cf1 — but the conclusion stands
on urgency alone, so nothing about the split changes.
Status unchanged: PROPOSED, needs explicit user approval. Do not
implement, do not merge.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T
This commit is contained in:
parent
bc16716b86
commit
fd7ae37071
|
|
@ -459,17 +459,64 @@ has **no branch and no framing yet**.
|
|||
`FrontendView.fold_projection` to `true` for semantic frontends, which
|
||||
Stage 2 deliberately left `false` (Q#FD21).
|
||||
|
||||
## dired Stage 2 framing lane — PR #171 AT REVISION 6, AWAITING APPROVAL
|
||||
## dired Stage 2 framing lane — PR #171 AT REVISION 7, AWAITING APPROVAL
|
||||
|
||||
- Portable branch: `githubsucks/dired-stage2-framing`;
|
||||
worktree `../pmacs-dired-stage1`. **PR #171**, base `main`, integrated
|
||||
up to `main` @ `ad41cf1`. Framing only —
|
||||
`docs/dired-stage2-framing.md`, now **~2,630 lines**, no runtime code.
|
||||
This lane rides that PR.
|
||||
- **Status: PROPOSED, never approved.** **Revision 5 was reviewed and
|
||||
NOT approved** — six findings, four P1 — and revision 6 answers them.
|
||||
The commit history embodies five revisions and four review rounds;
|
||||
**that is not the same as approval**, and it must not be read as one.
|
||||
- **Status: PROPOSED, never approved.** Revision 5 was reviewed and NOT
|
||||
approved (six findings, four P1); revision 6 answered them; **revision
|
||||
7 resolves a cross-lane conflict with PR #186**. The commit history
|
||||
embodies six revisions and five rounds of findings; **that is not the
|
||||
same as approval**, and it must not be read as one.
|
||||
- **CROSS-LANE SPLIT WITH PR #186 — settled, recorded verbatim so the
|
||||
two lanes cannot diverge again:**
|
||||
|
||||
> #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**. #171's revision 7 adopts the
|
||||
> refusal and stops saying LSP intentionally deletes modified files.
|
||||
|
||||
**PR #186 (`resource-op-delete-guard`) owns the pre-filesystem
|
||||
refusal.** Do not implement a competing dirty check from #171. Rev 7
|
||||
adopts #186's Q#RD1 (refuse, do not prompt, do not save, do not back
|
||||
up) and its Q#RD6 shared query (scan every path-bound buffer, not
|
||||
first-match; normalize once; component-aware `Path::starts_with`);
|
||||
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
|
||||
|
|
|
|||
|
|
@ -1,11 +1,12 @@
|
|||
# Dired Stage 2 — marks and operations — framing
|
||||
|
||||
**Revision 6 — 2026-07-28. Status: PROPOSED — NOT APPROVED. This
|
||||
**Revision 7 — 2026-07-28. Status: PROPOSED — NOT APPROVED. This
|
||||
document has never received a formal framing approval, and it needs one
|
||||
from the user before any implementation branch is cut.** Its commits
|
||||
embody four rounds of review findings; that is not the same as approval.
|
||||
Revision 5 was reviewed and **not approved** — six findings, four P1.
|
||||
§0's round-5 section says what each one changed.
|
||||
embody five rounds of findings; that is not the same as approval.
|
||||
Revision 6 answered review round 5; **revision 7 resolves a cross-lane
|
||||
conflict with PR #186**, which framed the opposite answer to the same
|
||||
event. §0's round-6 section says what the reconciliation changed.
|
||||
|
||||
**Ground truth: re-scouted 2026-07-28 against canonical `main` @
|
||||
`6bee09d`** (`Merge pull request #184 from levineuwirth/bottom-panel-stage2b`).
|
||||
|
|
@ -43,6 +44,167 @@ numbers drift and this document has now watched them drift twice.
|
|||
|
||||
## 0. Revision history
|
||||
|
||||
### Round 6 (rev 6 → rev 7) — cross-lane reconciliation with PR #186
|
||||
|
||||
Not a review of this document. **PR #186**
|
||||
(`docs/resource-op-delete-guard-framing.md`, branch
|
||||
`resource-op-delete-guard`, based on the same `ad41cf1`) frames a guard
|
||||
for `apply_resource_op`'s delete arm that **refuses before touching
|
||||
disk** when any affected buffer is modified. Rev 6 said the opposite for
|
||||
the same event: the file *is* deleted, the modified buffer survives
|
||||
orphaned, and §11 named that orphaning as accepted residue. Two lanes,
|
||||
two answers, same call site.
|
||||
|
||||
**The settled split, recorded verbatim so the two lanes cannot diverge
|
||||
again:**
|
||||
|
||||
> #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**. #171's revision 7 adopts the refusal
|
||||
> and stops saying LSP intentionally deletes modified files.
|
||||
|
||||
Rev 7 adopts it. **This was not re-litigated**, and the changes below
|
||||
are what adopting it costs this document.
|
||||
|
||||
#### The census of orphaning claims, and its arithmetic
|
||||
|
||||
Built by grep over the whole file, then classified by reading each site's
|
||||
enclosing passage rather than the matched line. The pattern set was
|
||||
`orphan`, `destroys/destroy/destroying unsaved`, `unsaved work`,
|
||||
`keeps/keep modified`, `kept_modified`, `still deletes the file`,
|
||||
`no longer destroys`, `accepted the refactor`.
|
||||
|
||||
**20 matched lines, resolving to 13 distinct passages**, classified:
|
||||
|
||||
| Class | Count | What it said | Rev 7 |
|
||||
|---|---|---|---|
|
||||
| **A — ownership** | **4** | that *this lane* closes the LSP-delete data-loss defect | **Reassigned to #186.** §1, §10, §13 item 23, §16 |
|
||||
| **B — policy** | **4** | that an LSP-authored delete *intentionally* removes the file and orphans the buffer, as accepted residue | **Deleted.** §6's asymmetric split, §6's residue paragraph, §11's deferral, Q#DR18 |
|
||||
| **C — ground truth** | **3** | that the defect exists on `main` today | **Kept — it is still true — but attributed to #186 as the fixer.** §0's round-4 list, §2, §6 |
|
||||
| **D — the async race** | **2** | that a buffer modified after dispatch is orphaned | **Kept, and narrowed** — see below |
|
||||
|
||||
Class C is deliberately not deleted: `main` at `ad41cf1` really does
|
||||
destroy unsaved work on that arm, and a framing that stopped saying so
|
||||
because another lane is fixing it would be describing a tree that does
|
||||
not exist yet. What changed is the attribution, not the fact.
|
||||
|
||||
#### What changed in Q#DR18
|
||||
|
||||
The decision is **rewritten, not withdrawn** — `reconcile_delete`
|
||||
survives, and the coordinator's instruction is explicit that the shared
|
||||
seam is not cancelled. Three changes:
|
||||
|
||||
- **The synchronous path refuses before disk.** For `apply_resource_op`,
|
||||
a modified buffer in the affected set means the delete **fails**,
|
||||
changing nothing on disk and nothing in the registry (#186 Q#RD1).
|
||||
Rev 6's "still deletes the file, because the user accepted the
|
||||
refactor" is gone.
|
||||
- **`reconcile_delete`'s modified arm is no longer *reached* on that
|
||||
path** — not removed from the seam. `DeleteReconcile.kept_modified`
|
||||
stays, because the **asynchronous** path still reaches it (Class D).
|
||||
Saying "the field is unreachable" would be wrong; saying "the sync
|
||||
caller can no longer produce it" is right.
|
||||
- **The walk rule is adopted from #186, not reinvented.** #186's Q#RD6
|
||||
already specifies the shared query — scans **every** path-bound
|
||||
buffer rather than first-match, normalizes once, and matches with
|
||||
component-aware `Path::starts_with` so `/tree` does not match
|
||||
`/tree-sibling`. That is character-for-character the rule rev 6 wrote
|
||||
for `reconcile_delete`, arrived at independently. **Whichever lands
|
||||
first owns the query and the other adopts it**, which is #186's own
|
||||
Q#RD5 boundary wording. `reconcile_rename` then uses the same query —
|
||||
#186's §6 explicitly parks "the rename side of prefix-aware,
|
||||
normalizing lookup" as dired Stage 2's.
|
||||
|
||||
#### What residue survives the refusal, and why
|
||||
|
||||
- **The async race is real, and rev 7 narrows whose it is.** #186's
|
||||
refusal is synchronous and lives inside `apply_resource_op`. **Dired
|
||||
never goes through `apply_resource_op`** — it calls `pmacs.fs.remove`,
|
||||
which dispatches a worker (§2), so the refusal cannot reach it at any
|
||||
strength. Dired's own pre-dispatch check closes the window up to
|
||||
dispatch; between dispatch and `remove_blocking`'s `remove_file` the
|
||||
interval stays open, and a buffer modified in it is orphaned.
|
||||
**Correcting rev 6's framing of this:** rev 6 called it "the residue
|
||||
rev 3 scoped to the LSP path applies to dired too — one deferral, two
|
||||
paths". After #186 that is backwards. The LSP path has no residue,
|
||||
because it refuses. **It is one deferral, one path, and the path is
|
||||
dired's** — it always was, and rev 6 obscured it by bundling.
|
||||
- **The full post-delete lifecycle** stays this lane's, and #186 hands
|
||||
it over explicitly rather than by omission. Its Q#RD8 parks "the
|
||||
window/last-buffer defects" and states the trap rev 6's R4 found
|
||||
independently: `kill_buffer` and `remove_buffer_and_fire` clean
|
||||
**disjoint** sets, so neither is a superset and "just call
|
||||
`kill_buffer`" silently regresses four cleanups. Its Q#RD5 then
|
||||
deliberately keeps reconciliation **exact-path** — leaving descendant
|
||||
buffers orphaned-and-clean after a recursive delete — precisely
|
||||
because widening it "would promote mode (d)'s dangling-window and
|
||||
last-buffer defects from an exact-path defect to a tree-wide one".
|
||||
**That is this lane's Q#DR27.** #186 narrows on purpose so that #171
|
||||
can widen safely once the lifecycle is correct; the two decisions fit,
|
||||
and neither is complete alone.
|
||||
|
||||
#### The LSP failure-handling claim — checked, and not inherited
|
||||
|
||||
The coordinator flagged that #186 mis-assigns `Abort` to edits
|
||||
containing resource operations and warned this document might carry the
|
||||
same error. **It does not.** `grep` over this file for
|
||||
`failureHandling|failure handling|Abort|transactional` returns **10
|
||||
lines, none of which is a claim about the LSP specification** — every
|
||||
one is about hook fan-out or dired's own batch semantics ("a per-entry
|
||||
failure does not abort the batch"). Rev 6 asserted nothing about
|
||||
`WorkspaceEdit` recovery.
|
||||
|
||||
The one adjacent claim was §6's *"refusing mid-edit leaves a
|
||||
half-applied refactor"*, offered as the reason the LSP arm should
|
||||
delete anyway. That was a claim about consequence rather than about the
|
||||
protocol — but it was the load-bearing support for Class B, and it goes
|
||||
with it.
|
||||
|
||||
Verified against the **LSP 3.18** specification so the record is
|
||||
accurate for both lanes:
|
||||
|
||||
- `abort` — *"Applying the workspace change is simply aborted if one of
|
||||
the changes provided fails. All operations executed before the failing
|
||||
operation stay executed."* **No mention of resource operations.**
|
||||
- `transactional` — *"All operations are executed transactionally. That
|
||||
means they either all succeed or no changes at all are applied to the
|
||||
workspace."* This **does** cover resource operations.
|
||||
- `textOnlyTransactional` — *"If the workspace edit contains only
|
||||
textual file changes they are executed transactionally. If resource
|
||||
changes (create, rename or delete file) are part of the change the
|
||||
failure handling strategy is abort."* **This is the only kind that
|
||||
degrades for resource ops**, and it is the sentence that gets
|
||||
misattributed to `abort`.
|
||||
- `undo` — *"The client tries to undo the operations already executed.
|
||||
But there is no guarantee that this is succeeding."*
|
||||
- The `failureHandling` capability is described only as *"The failure
|
||||
handling strategy of a client if applying the workspace edit fails."*
|
||||
**The specification states no default for a client that does not
|
||||
advertise it.**
|
||||
|
||||
So "pmacs advertises none, therefore `Abort` semantics apply by
|
||||
omission" does not follow, and neither does "the protocol declines to
|
||||
promise transactionality for resource-op edits" — `transactional`
|
||||
promises exactly that. Recorded here rather than only in a review
|
||||
comment because this document's §6 will sit next to #186's §1.7 in the
|
||||
same arc, and the corrected reading should be findable from either.
|
||||
|
||||
#### One supporting citation in #186 that has gone stale
|
||||
|
||||
Not a disagreement with the split, and not grounds to revisit it —
|
||||
#186's Q#RD5 justifies taking the delete side now partly by citing "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."
|
||||
That was accurate when written and is no longer: #171 completed the
|
||||
re-scout, is integrated to `ad41cf1`, and is at revision 7. The
|
||||
*conclusion* stands on urgency alone — #186 fixes live data loss and
|
||||
this lane is not implementable until its framing is approved — so
|
||||
nothing about the split changes. Recorded so #186's next revision can
|
||||
drop a citation that would otherwise read as current.
|
||||
|
||||
---
|
||||
|
||||
### Review round 5 (rev 5 → rev 6) — six findings, four P1, none approved
|
||||
|
||||
Round 5's theme is **one theme, not six**: rev 5 changed the slice split
|
||||
|
|
@ -342,7 +504,8 @@ still doing a raw-path `find_by_path` first-match rebind
|
|||
(`src/lua_bindings/mod.rs:3306`) and its delete arm still killing
|
||||
through `remove_buffer_and_fire` with **no modified check**
|
||||
(`:3339-3341`) — **so an LSP-authored delete still destroys unsaved work
|
||||
on `main` today**; `apply_workspace_edit` still capturing `origin` as a
|
||||
on `main` today** (still true at `ad41cf1`; **PR #186 is what fixes it**,
|
||||
not this lane — see round 6); `apply_workspace_edit` still capturing `origin` as a
|
||||
**string** and restoring with `find_or_open` (`builtin/runtime/lsp.lua:1338`,
|
||||
`:1352`), its own comment still conceding the path *"may have just been
|
||||
renamed or deleted"*; `resolve_target_buffer`'s `NotFound` arm still
|
||||
|
|
@ -714,8 +877,15 @@ themselves.** That distinction is load-bearing and rev 5 blurred it
|
|||
this stage. Stage 2 supplies `resource.renamed`; Lean's state stays
|
||||
stale until Lean's owner subscribes (§11). What Stage 2 *does* deliver
|
||||
is a correctness fix to shared substrate: it is the reason `R` is safe
|
||||
on a directory at all, and it closes a path on which an LSP-authored
|
||||
delete currently destroys unsaved work.
|
||||
on a directory at all, and it gives rename and delete **one** registry
|
||||
walk instead of the raw first-match lookup each arm does today.
|
||||
|
||||
**What this lane no longer claims** (rev 7, round 6): closing the
|
||||
LSP-authored delete that destroys unsaved work. That defect is real on
|
||||
`main` and is **PR #186's**, which refuses the delete *before disk*
|
||||
rather than reconciling after it. This lane inherits the refusal and
|
||||
owns what remains once it is in place — the full post-delete lifecycle,
|
||||
and the async race #186's synchronous guard cannot reach (§6).
|
||||
|
||||
Not in Stage 2: `wdired` (Stage 3), subdirectory insertion (`i`),
|
||||
shell commands on marks (`!`), regexp marking (`% m`), and
|
||||
|
|
@ -1558,6 +1728,10 @@ and unmentioned. Verified: `apply_resource_op`'s delete arm
|
|||
lookup §5 fixes for rename, with **no descendant handling and no
|
||||
modified check**. So an LSP-authored delete **destroys unsaved work
|
||||
today**, silently, and a second buffer on the same path survives.
|
||||
**That data-loss half is PR #186's to fix** (rev 7, round 6): it refuses
|
||||
the delete before disk. What this lane takes from the same arm is the
|
||||
lookup — descendants and duplicate path-bound buffers — and the removal
|
||||
lifecycle (Q#DR27).
|
||||
|
||||
### One seam
|
||||
|
||||
|
|
@ -1569,7 +1743,13 @@ kept_modified, refused }`**, symmetric with `reconcile_rename`:
|
|||
included and a second buffer on one path is not missed;
|
||||
- **kills unmodified** buffers, through the full two-phase lifecycle
|
||||
below;
|
||||
- **keeps modified ones alive** and returns them, so a caller can report;
|
||||
- **keeps modified ones alive** and returns them, so a caller can
|
||||
report. *(Rev 7, round 6: the **synchronous** caller can no longer
|
||||
produce this outcome — #186 refuses the delete before disk when any
|
||||
affected buffer is modified, so `apply_resource_op` never reaches a
|
||||
live file with a dirty buffer. The arm is retained for the
|
||||
**asynchronous** path, which `pmacs.fs.remove` puts on a worker where
|
||||
no in-applier guard can see it.)*;
|
||||
- returns anything it **could not** kill separately from what it kept
|
||||
deliberately — the two are different events and collapsing them makes
|
||||
a failure look like a policy decision.
|
||||
|
|
@ -1627,23 +1807,38 @@ not gain a Lua handle.
|
|||
**Both paths call it**: the drain harvest for `pmacs.fs.remove`, and
|
||||
`apply_resource_op`'s delete arm, replacing its first-match lookup.
|
||||
|
||||
The **policy split is deliberate and asymmetric**, and this is the part
|
||||
worth arguing with:
|
||||
**Both paths refuse. The policy is symmetric** (rev 7 — round 6; rev 6
|
||||
had them asymmetric, and that half is withdrawn):
|
||||
|
||||
- **dired refuses the whole entry** when a visited buffer is modified —
|
||||
the file is never deleted. A direct user gesture on a file with unsaved
|
||||
changes should stop, not proceed-and-cope.
|
||||
- **`apply_resource_op` still deletes the file**, because the delete is
|
||||
part of a server-authored workspace edit the user already accepted, and
|
||||
refusing mid-edit leaves a half-applied refactor. But it **no longer
|
||||
destroys the buffer**: a modified buffer survives the delete with its
|
||||
contents. That is strictly better than today and changes no file-side
|
||||
behavior.
|
||||
the file is never deleted. A direct user gesture on a file with
|
||||
unsaved changes should stop, not proceed-and-cope.
|
||||
- **`apply_resource_op` refuses the operation** when any affected buffer
|
||||
is modified — **before touching disk**, so nothing is deleted and
|
||||
nothing is removed from the registry. That is **PR #186's Q#RD1**, not
|
||||
this lane's decision, and this lane adopts it.
|
||||
|
||||
The residue — an LSP-driven delete can still orphan a modified buffer —
|
||||
is **named, not fixed** (§11). Fixing it properly means deciding what a
|
||||
partially-applied workspace edit does, which is a larger question than
|
||||
dired.
|
||||
Rev 6 argued the opposite for the LSP arm: that the delete should
|
||||
proceed because "the user already accepted the refactor" and refusing
|
||||
mid-edit "leaves a half-applied refactor", with the orphaned buffer as
|
||||
accepted residue. **That is withdrawn.** Two reasons it was wrong beyond
|
||||
the cross-lane conflict:
|
||||
|
||||
- Accepting a *refactor* is not accepting the loss of edits made after
|
||||
it was requested, and pmacs cannot tell the two apart from inside the
|
||||
applier.
|
||||
- The "half-applied refactor" argument leaned on an assumption about
|
||||
`WorkspaceEdit` recovery that this document never actually checked.
|
||||
Checked now, against **LSP 3.18** (§0, round 6): 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` —
|
||||
which pmacs does not. So the protocol does not license "deleting
|
||||
anyway is the safer half", and the argument had no support.
|
||||
|
||||
**So on the synchronous path there is no orphan to reconcile.** The
|
||||
residue that remains is the asynchronous one, below — and it is dired's
|
||||
alone.
|
||||
|
||||
### Deletion is harvested, not hand-fired (G4)
|
||||
|
||||
|
|
@ -1674,7 +1869,10 @@ lands) is the primitive's.
|
|||
deletes and leaves an orphaned buffer. The reasoning: the orphan is
|
||||
indistinguishable from a normal buffer, and the next `C-x C-s` silently
|
||||
resurrects the file. A refusal is visible, recoverable, and the user can
|
||||
save-or-discard and retry.
|
||||
save-or-discard and retry. **PR #186 reached the same conclusion
|
||||
independently for the LSP arm** (its Q#RD1 — "refuse, do not prompt, do
|
||||
not save, do not back up"), which is why rev 7's policy is symmetric
|
||||
rather than split.
|
||||
|
||||
### Checked twice — and still only best-effort (G4, narrowed H1)
|
||||
|
||||
|
|
@ -1706,9 +1904,15 @@ worth being precise about:
|
|||
because it runs at drain time on whatever state exists then;
|
||||
- the **file** is gone.
|
||||
|
||||
Which means **the orphaned-modified-buffer residue rev 3 scoped to the LSP
|
||||
path applies to dired too**, on this narrow race. §11 carries it as one
|
||||
deferral, not two.
|
||||
Which means the orphaned-modified-buffer residue is **dired's, and only
|
||||
dired's** (rev 7 — round 6). Rev 6 called it "the residue rev 3 scoped
|
||||
to the LSP path applies to dired too — one deferral, two paths". After
|
||||
#186 that is backwards: **the LSP path has no residue, because it
|
||||
refuses before disk.** Dired's remains because **dired never goes
|
||||
through `apply_resource_op`** — it calls `pmacs.fs.remove`, which
|
||||
dispatches a worker (§2), so a synchronous guard inside the applier
|
||||
cannot reach it at any strength. §11 carries it as **one deferral, one
|
||||
path**.
|
||||
|
||||
Closing it properly needs one of two things this stage should not invent:
|
||||
a **reservation** (some lock or generation the worker re-validates before
|
||||
|
|
@ -1928,11 +2132,18 @@ substrate items in 2b and one dired item in 2a.
|
|||
| Acceptance | 23–38, **50–53** | 1–22, 39–41 | 42–47 |
|
||||
|
||||
**Why 2a first, with no dired surface at all.** It is a self-contained
|
||||
substrate correctness fix that stands on its own merits: it closes a path
|
||||
where an LSP-authored delete **destroys unsaved work today**
|
||||
(`mod.rs:3313-3341`), and one where renaming the active file through a
|
||||
workspace edit **materializes a phantom buffer** (`lsp.lua:1352`). Neither
|
||||
needs dired to be worth fixing, and neither is dired's fault. Landing it
|
||||
substrate correctness fix that stands on its own merits: it closes the
|
||||
path where renaming the active file through a workspace edit
|
||||
**materializes a phantom buffer** (`lsp.lua:1352`); it replaces the
|
||||
raw first-match, un-normalized registry lookup that **both**
|
||||
`apply_resource_op` arms use with one shared prefix-aware query; and it
|
||||
supplies the **full removal lifecycle** that neither existing path
|
||||
performs (§6, Q#DR27). None of that needs dired to be worth fixing, and
|
||||
none of it is dired's fault. *(Rev 6 also claimed 2a closes the
|
||||
LSP-authored delete that destroys unsaved work. **PR #186 owns that** —
|
||||
it refuses before disk rather than reconciling after — so rev 7 drops
|
||||
the claim. What is left is still substrate-level and still worth its own
|
||||
review round.)* Landing it
|
||||
alone also means the LSP lifecycle work — a fourteen-store inventory, an
|
||||
in-flight route purge, an awaiter drain, and a new `View` hook swept across
|
||||
every window — gets a review round of its own rather than sharing one with
|
||||
|
|
@ -1977,16 +2188,30 @@ that deserve an undivided reviewer.
|
|||
- **A no-replace rename primitive** (G6) — `renameat2(RENAME_NOREPLACE)`
|
||||
on Linux, `renamex_np` on macOS, link/unlink elsewhere. Until then `R`'s
|
||||
refusal is a TOCTOU-bounded preflight, which §8 states plainly.
|
||||
- **A delete can still orphan a modified buffer** — one deferral, two
|
||||
paths (H1). On the LSP path this is by design (the user accepted the
|
||||
refactor); on dired's it is the residue of the open
|
||||
dispatch-to-syscall interval. Stage 2 stops both from *destroying* the
|
||||
buffer; the file still goes. Closing dired's half needs a
|
||||
**reservation** the worker re-validates, or a **synchronous** delete
|
||||
path — the latter would also make the delete harvest unnecessary for
|
||||
dired, but puts N blocking syscalls on the main thread and leaves every
|
||||
other `pmacs.fs.remove` caller unprotected. Closing the LSP half means
|
||||
deciding what a partially-applied workspace edit does.
|
||||
- **The async race: a buffer modified after dired dispatch is still
|
||||
orphaned** — **one deferral, one path** (H1; narrowed in rev 7, round
|
||||
6). Rev 6 carried this as two paths, LSP and dired. It is dired's
|
||||
alone: **#186 refuses the synchronous `apply_resource_op` delete
|
||||
before disk**, so that path orphans nothing, while dired calls
|
||||
`pmacs.fs.remove`, which **dispatches a worker** — the interval
|
||||
between dired's pre-dispatch check and `remove_blocking`'s
|
||||
`remove_file` is not closed by anything, and a buffer modified inside
|
||||
it loses its file. Closing it needs a **reservation** the worker
|
||||
re-validates before the syscall, or a **synchronous** delete path —
|
||||
the latter would also make the delete harvest unnecessary for dired,
|
||||
but puts N blocking syscalls on the main thread for an N-entry batch.
|
||||
Note this is the *only* thing left of rev 6's orphaning story; the
|
||||
LSP half went away with #186 rather than being solved here.
|
||||
- **`pmacs.fs.remove` itself is guarded by neither lane** (new in rev 7,
|
||||
round 6). After both land, the refusal exists at the
|
||||
`apply_resource_op` primitive (#186) and in dired's policy layer
|
||||
(§6) — but `pmacs.fs.remove` is public Lua API with **no dirty check
|
||||
of its own**, so a third caller inherits neither guard. This is
|
||||
latent rather than live: §2 verified `pmacs.fs.remove` has **zero
|
||||
production callers** (only `tests/m8_1_acceptance.rs:438,439,472`).
|
||||
Naming it because the natural reading of "both lanes guard deletion"
|
||||
is that the primitive is guarded, and it is not — the guards are one
|
||||
layer above it on each side.
|
||||
- **A passive window that never received a `DiagnosticView` still has
|
||||
none** (H3). `_attach_view` takes `active_window_mut()` and errors
|
||||
otherwise (`lua_bindings/diag.rs:218-224`); `ensure_overlay` +
|
||||
|
|
@ -2095,6 +2320,15 @@ that deserve an undivided reviewer.
|
|||
depends on the intercept's exact wording rather than on the substring
|
||||
`read-only`, this bet fails. §3.1 checked the one test that looked
|
||||
like a risk and it does not.
|
||||
- **B8** *(new in rev 7, round 6)*. Adopting #186's refusal costs this
|
||||
lane **no design change beyond deletion**: `reconcile_delete`'s
|
||||
signature, its callers, the drain harvest and `resource.deleted` are
|
||||
all unaffected, because the refusal happens *before* the seam rather
|
||||
than inside it. Falsified if 2a turns out to need a different seam
|
||||
shape once #186 has landed — most plausibly if #186's validation phase
|
||||
ends up owning the affected-set query in a form `reconcile_delete`
|
||||
cannot reuse, in which case the two must be reconciled before 2a is
|
||||
cut rather than after.
|
||||
- **B7** *(new in rev 5, N3)*. The journey ratchet stays at **≥ 24**
|
||||
rows in `tests/journey_acceptance.rs` and keeps its GPU row in
|
||||
`tests/gpu_invocation_acceptance.rs`, with no row weakened. Falsified
|
||||
|
|
@ -2175,10 +2409,15 @@ that deserve an undivided reviewer.
|
|||
|
||||
**2a — reconciliation (§5, §6)**
|
||||
|
||||
23. **`apply_resource_op`'s delete** no longer kills a **modified**
|
||||
buffer, and now reaches **descendants** and a **second buffer on the
|
||||
same path** (G4 — today it is raw-path first-match with no modified
|
||||
check, so it destroys unsaved work).
|
||||
23. **`apply_resource_op`'s delete reaches descendants and a second
|
||||
buffer on the same path** — the raw-path first-match lookup replaced
|
||||
by the shared prefix-aware, normalizing query (G4). *(Rev 7, round
|
||||
6: the **modified**-buffer half of this item moved to **PR #186**,
|
||||
which refuses the delete before disk, so by the time this lane's
|
||||
reconciliation runs there is no modified buffer on the synchronous
|
||||
path to spare. Assert the lookup fix here; #186 asserts the
|
||||
refusal. If #186 has not landed when 2a is implemented, this item
|
||||
still stands on the lookup alone.)*
|
||||
24. A **fire-and-forget** `pmacs.fs.remove` reconciles too — never taking
|
||||
the handle still kills the unmodified buffer, which is what makes the
|
||||
drain harvest the right seam rather than dired firing the hook.
|
||||
|
|
@ -2435,24 +2674,45 @@ mark, operation and subscriber items.
|
|||
`mkdir`/`copy`/`remove_dir_all` with `+ C` and recursive delete. 2a
|
||||
first because a bug in it is silent data loss, and because every
|
||||
blocking finding across three rounds landed on it. (§10)
|
||||
- **Q#DR18** *(new in rev 2, F2; given a seam in rev 3, G4)* Deleting a
|
||||
path something holds. **One shared `EditorCore::reconcile_delete`**,
|
||||
symmetric with `reconcile_rename` — whole registry, equality or
|
||||
path-component prefix, kills unmodified buffers and **keeps modified
|
||||
ones** — called by both the drain harvest and `apply_resource_op`,
|
||||
replacing the latter's raw first-match lookup. `remove` is **harvested
|
||||
in the drain** like rename, firing **`resource.deleted(path)`**, so a
|
||||
fire-and-forget remove reconciles too. The **policy** is deliberately
|
||||
asymmetric: dired **refuses the whole entry** when a visited buffer is
|
||||
modified, while an LSP-authored delete still removes the file (the user
|
||||
accepted the refactor) but **no longer destroys the buffer**. The
|
||||
modified check runs **before** the confirm *and again before each
|
||||
dispatch*, because another frontend can edit while the prompt is open —
|
||||
but it is a **pre-dispatch check, not a lock** (H1): `pmacs.fs.remove`
|
||||
dispatches a worker, so the dispatch-to-syscall interval stays open
|
||||
and the promise is stated at that strength, as G6 forced for `R`.
|
||||
Deliberately diverges from Emacs, which orphans the
|
||||
buffer and lets the next save resurrect the file. (§6)
|
||||
- **Q#DR18** *(new in rev 2, F2; seam in rev 3, G4; **policy rewritten
|
||||
in rev 7, round 6**)* Deleting a path something holds.
|
||||
|
||||
**The seam is unchanged and is not cancelled.** One shared
|
||||
`EditorCore::reconcile_delete`, symmetric with `reconcile_rename`,
|
||||
called by both the drain harvest and `apply_resource_op`, replacing
|
||||
the latter's raw first-match lookup. Its walk is the **shared
|
||||
prefix-aware, normalizing query** — every path-bound buffer, not the
|
||||
first match; normalized once; component-aware `Path::starts_with` so
|
||||
`/tree` does not match `/tree-sibling`. **That query is #186's Q#RD6**;
|
||||
whichever lane lands first owns it and the other adopts it, which is
|
||||
#186's own boundary wording, and `reconcile_rename` then shares it.
|
||||
`remove` is still **harvested in the drain** like rename, firing
|
||||
**`resource.deleted(path)`**, so a fire-and-forget remove reconciles
|
||||
too.
|
||||
|
||||
**The policy is now symmetric: both paths refuse.** Rev 6 had dired
|
||||
refusing while an LSP-authored delete proceeded and orphaned the
|
||||
buffer as accepted residue. **Withdrawn.** For the synchronous
|
||||
`apply_resource_op` path a modified buffer in the affected set means
|
||||
the delete is **refused before disk** — nothing removed, nothing
|
||||
deleted — which is **PR #186's Q#RD1**, adopted here rather than
|
||||
re-decided. dired refuses the entry on the same condition.
|
||||
|
||||
**`DeleteReconcile.kept_modified` stays, and is not dead.** The
|
||||
synchronous caller can no longer produce it, because the refusal
|
||||
happens earlier; the **asynchronous** path still can, since dired
|
||||
goes through `pmacs.fs.remove`, which dispatches a worker that #186's
|
||||
in-applier guard never sees. dired's modified check therefore still
|
||||
runs **before** the confirm *and again before each dispatch* — but it
|
||||
is a **pre-dispatch check, not a lock** (H1), the interval from
|
||||
dispatch to `remove_blocking` stays open, and the promise is stated at
|
||||
that strength, as G6 forced for `R`. That interval is the whole of
|
||||
this lane's remaining orphaning residue (§11).
|
||||
|
||||
Deliberately diverges from Emacs, which orphans the buffer and lets
|
||||
the next save resurrect the file — a conclusion **#186 reached
|
||||
independently** for the LSP arm (its Q#RD1: refuse, do not prompt, do
|
||||
not save, do not back up). (§6)
|
||||
- **Q#DR19** *(new in rev 2, F3)* `M` (chmod) is **new scope** beyond the
|
||||
parent's approved table and needs explicit approval. It **refuses
|
||||
symlink entries**, because `chmod` follows links while the listing is
|
||||
|
|
@ -2571,10 +2831,14 @@ lands.
|
|||
**Naming 2a for the substrate, not for dired**, following #161's
|
||||
precedent: its diff contains no dired code, and a cross-cutting
|
||||
correctness fix to rename/delete reconciliation must not be reviewable
|
||||
only as a dired feature. The PR body should lead with the two defects it
|
||||
closes on `main` today — the LSP-authored delete that destroys unsaved
|
||||
work, and the workspace-edit phantom buffer — because neither needs dired
|
||||
to be worth fixing.
|
||||
only as a dired feature. The PR body should lead with the defects it
|
||||
closes on `main` — the **workspace-edit phantom buffer**, the **raw
|
||||
first-match lookup** shared by both `apply_resource_op` arms, and the
|
||||
**incomplete removal lifecycle** (Q#DR27) — because none needs dired to
|
||||
be worth fixing. *(Rev 7: it should **not** lead with the
|
||||
LSP-authored delete that destroys unsaved work. That is **PR #186's**,
|
||||
and 2a's body should say so and cite it rather than appearing to claim
|
||||
it.)*
|
||||
|
||||
**Ledger note (corrected in rev 5, W6):** this framing branch
|
||||
deliberately touches **only** this file. Rev 4 said the durable records
|
||||
|
|
|
|||
Loading…
Reference in New Issue