From 5f3f38dfd7eeb2cacb9c9df1d8790c20390c6684 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sun, 9 Aug 2026 21:02:10 +0200 Subject: [PATCH] fix(window): keep a panel commit's restriction across nested scopes Revision 8 refuses, inside a "panel" commit_to, the mutations that would make its relaxed preflight wrong. A nested commit_to REPLACED the enclosing contract with its own and restored it afterwards, so the outer restriction went out of force for the whole inner body: commit_to(outer, function() -- "panel", relaxed preflight commit_to(inner, function() -- "document", MASKS the outer set_params(panel(), { dedicated = true }) -- ...and succeeds end) display(result, { side = "bottom" }) -- ...which now FALLS BACK end, "panel") Every step is legal on its own, and the outer commit then overwrote a newer document buffer --- the P1a failure the lane exists to remove, reached through one extra call. What this invalidated, precisely: NOT the enumeration of dedication write sites. Every site in it is real and still guarded. What was wrong was the claim that the guard was in force for the whole outer body. So the enumeration is inherited and qualified, not redone. Contracts now COMPOSE rather than replace. The core holds a stack; ScopedFrontendGuard pushes on entry and truncates back to its own depth on every exit path; panel_commit_dedication_refusal consults every contract in force rather than the innermost. The strictest active restriction wins. Matching stays per frontend --- a nested commit for a different frontend may dedicate its own side slot, which cannot change where this frontend's side request lands. Nesting itself is NOT forbidden, which was the other candidate fix. It closes the hole by prohibiting a construction no rule objects to: commit_to is public Lua API for saying where a continuation's result belongs, and a body committing to a second destination (a diff beside a status panel) is where #227's adoption is heading. Only the restriction needed preserving. Detecting the dedication when the outer commit resumed was not available either --- that is a late refusal, which is what revision 7 was rejected for. Two pins, and they are a pair rather than one test written twice: * a_nested_commit_cannot_mask_an_outer_panel_restriction drives the same four write-site rows through a nested, entirely valid "document" commit, and asserts the attempt is refused, the slot is still undedicated, and the outer commit's destination is intact. * an_ordinary_nested_commit_still_runs_and_restores_the_outer_restriction pins that nesting without dedication is accepted, that the enclosing restriction is back in force once the nested commit returns, and that outside every commit dedication is ordinary again. Mutation-checked: restoring the guard to the innermost contract (.last(), exactly revision 8's swapped slot) fails only the first of those. The other 13 pins, journey_acceptance (31), dired_acceptance (47) and cargo test --lib (1920) all stay green. The ordinary-nesting pin deliberately survives it --- it exists to fail the other candidate fix. Also sweeps the comments left by revision 7, which revision 8 superseded: no fallback_commit_refusal symbol remains, but six doc sites still described placement-boundary enforcement as the guarantee (ViewDestination, CommitProfile::Panel, CommitContract, capture_view_destination, commit_destination_refusal, panel_placement_can_fall_back), plus two comment blocks in the commit_to binding and one stale mutation note in the acceptance suite. Net rustdoc warnings down three. Framing to revision 9; the active-work lane entry updated in place. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/active-work.md | 121 ++++++-- docs/destination-capture-framing.md | 109 ++++++- src/editor.rs | 32 +- src/editor_core.rs | 185 +++++++---- src/lua_bindings/window_panel.rs | 41 +-- tests/destination_capture_acceptance.rs | 388 ++++++++++++++++++++---- 6 files changed, 695 insertions(+), 181 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index 0c9e1e4..c81de48 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -265,13 +265,14 @@ also removed: this branch's "R8 NEEDS A LANE" investigation block, and durable facts are in the retired registry row and the handoff §6 census. -## Destination capture (Q#JR14 generalization) — revision 8 IMPLEMENTED, gate green, no PR yet +## Destination capture (Q#JR14 generalization) — revision 9 IMPLEMENTED, gate green, no PR yet The mechanism landed at `0efc8c0`; review found a correctness blocker; `ca72461` implemented **revision 7**, which review then **also** -rejected; the commit below replaces it with **revision 8** and its -§3 enumeration is **performed and recorded in the framing**. No PR — the -lane was told not to open one. +rejected; `469d5c8` replaced it with **revision 8** and its §3 +enumeration is **performed and recorded in the framing**; review then +found a hole in revision 8's guard **scope** and the commit below closes +it as **revision 9**. No PR — the lane was told not to open one. **The original blocker:** the panel profile skipped checks 2–4 on the claim that a panel result never touches a document window. **Panel @@ -280,8 +281,8 @@ is not panel-capable or its side slot is dedicated, so a `"panel"` commit could replace a **newer** document with every stale-intent guard skipped. Reproduced in review. -**Three designs, two rejected — the sequence is the part worth not -re-learning:** +**Four designs, two rejected outright and one corrected — the sequence +is the part worth not re-learning:** 1. **Revision 6 — predict at preflight.** Rejected: the `await` refusal stops concurrent interleaving, not the body, which is arbitrary @@ -293,13 +294,37 @@ re-learning:** already created buffers, handles and paint by then, so a placement-time refusal is a partial commit with an error return. 3. **Revision 8 — keep the preflight, REFUSE the scope-invalidating - mutation.** Current design. Same shape as `Handle:await` being - refused inside a commit scope: the fallback never comes into + mutation.** The shape the tree implements. Same as `Handle:await` + being refused inside a commit scope: the fallback never comes into existence, and refusal stays mutation-free on `(false, reason)`. +4. **Revision 9 — make the refusal hold for the WHOLE body.** Not a new + shape; a correction to revision 8's scope. A nested `commit_to` + **replaced** the enclosing contract and restored it afterwards, so + an outer `"panel"` commit's restriction went out of force for the + inner body's extent: nested `"document"` commit → callback dedicates + the side slot, unrefused → outer commit resumes, falls back, + overwrites a newer document. Reproduced in review. Contracts now + **compose** — the core holds a stack, `commit_to` pushes and pops + rather than swapping, and the guard consults every contract in force, + so the strictest active restriction wins. Nesting itself is **not** + forbidden: only the mutation is refused, so a nested commit that + touches no dedication runs exactly as before. Detecting the + dedication when the outer commit resumed was not available — that is + a late refusal, which is what revision 7 was rejected for. -**THE ENUMERATION IS THE LOAD-BEARING PART, AND IT IS NOW CLOSED — for -a structural reason, not because inspection ran out of ideas.** Full -working in the framing §3; the short form: +**WHAT REVISION 9 DID *NOT* INVALIDATE — read this before re-opening the +enumeration.** The write-site enumeration below survived intact: every +site is real, every one is still guarded, and review of the nesting +defect found no missing route. What was wrong was the *surrounding* +claim — that the guard was in force for the whole outer body. A complete +list of write sites is not a complete argument until the guard's extent +is stated too. The acceptance suite now drives the same rows at **two +depths**, directly and through a nested `commit_to`. + +**THE ENUMERATION IS THE LOAD-BEARING PART, AND IT IS CLOSED AS AN +ENUMERATION OF WRITE SITES — for a structural reason, not because +inspection ran out of ideas.** Full working in the framing §3; the short +form: - **Only two pieces of state can matter**, because `resolve_placement` reaches `Ordinary` from a side request through exactly two branches: @@ -348,38 +373,60 @@ from #171 and #215. authoritative tip** — the ref, not a SHA. Recover with `git fetch githubsucks && git checkout destination-capture`. -- **Framing `docs/destination-capture-framing.md`, revision 8.** - Revisions 1–5 were approved over four review rounds; revisions 6, 7 - and 8 are corrections carrying the blocker above, and **revision 8's - design is what the tree implements**. Revisions 6 and 7 are described - in that document as the record of why *not* those; neither is in the - tree and neither should be restored from it. -- **Implemented in three commits.** `779bb02` is the mechanism +- **Framing `docs/destination-capture-framing.md`, revision 9.** + Revisions 1–5 were approved over four review rounds; revisions 6–9 are + corrections carrying the blocker above, and **revision 8's design as + scoped by revision 9 is what the tree implements**. Revisions 6 and 7 + are described in that document as the record of why *not* those; + neither is in the tree and neither should be restored from it. +- **Implemented in four commits.** `779bb02` is the mechanism (`pmacs.window.capture_destination()`, the `ViewDestination` rename, the profile argument); `d5a6170` is - `tests/destination_capture_acceptance.rs`; the revision-8 commit is - the panel-profile correction plus the invalid-UTF-8 hole. **12 pins**, - and both preservation suites pass **unchanged** (journey 47, dired 31) - — §7's stop signal not firing rather than being suppressed. + `tests/destination_capture_acceptance.rs`; `469d5c8` is the + revision-8 panel-profile correction plus the invalid-UTF-8 hole; the + commit below is revision 9's contract stack. **14 pins**, and both + preservation suites pass **unchanged** (journey 47, dired 31) — §7's + stop signal not firing rather than being suppressed. - **HOW THE PANEL PROFILE IS ENFORCED, in one sentence so no earlier revision gets reinstated by someone reading only that document:** the preflight stays exactly where it was, and the mutations that would invalidate it are **refused at the attempt**. - `EditorCore::panel_commit_dedication_refusal` is the one rule. It - fires while a `"panel"` `CommitContract` is on the core for this - frontend, and is consulted from `display_buffer` (before - `apply_placement`, so a refused attempt mutates nothing), - `pmacs.window.set_params` (before its borrow, so `fixed_rows` in the - same table is not applied either), and `quit_window`. + fires while **any** `"panel"` `CommitContract` for this frontend is + in force — every contract on the stack, not the innermost — and is + consulted from `display_buffer` (before `apply_placement`, so a + refused attempt mutates nothing), `pmacs.window.set_params` (before + its borrow, so `fixed_rows` in the same table is not applied + either), and `quit_window`. - **This is the same shape as `Handle:await` being refused inside a commit scope**, and for the identical reason: something that would invalidate the scope's guarantee is rejected outright rather than predicted around or caught late. - The contract (`CommitContract { destination, profile }`) rides on - the core, installed and restored by the **same** `ScopedFrontendGuard` - that scopes the frontend, so a `"panel"` profile can never outlive - the body that declared it. The field is private to the crate — Lua - cannot claim a profile for a placement it did not commit to. + the core in a **stack**, pushed and popped by the **same** + `ScopedFrontendGuard` that scopes the frontend, so a `"panel"` + profile can never outlive the body that declared it. The field is + private to the crate — Lua cannot claim a profile for a placement it + did not commit to. + - **A stack, not a slot, and the distinction is revision 9 (above).** + The frontend override and the ambient frontend are *substitutions*, + so a nested scope rightly replaces them; a contract is a + *restriction*, and replacing one suspends it. The guard stores a + depth and truncates back to it, so an inner exit removes exactly the + contract it added and leaves every enclosing one in force. Matching + is per **frontend**: a nested commit for a different frontend may + dedicate *its* side slot, which cannot change where this frontend's + side request lands. + - **Prohibiting nested `commit_to` was the other candidate and was + rejected.** It closes the hole by forbidding a construction no rule + objects to — `commit_to` is public Lua API for saying where a + continuation's result belongs, and a body committing to a second + destination (a diff beside a status panel) is where #227's adoption + is heading. Only the restriction needed preserving. **No Lua in the + tree nests today** — `builtin/runtime/dired.lua` is the only + `commit_to` consumer and it does not — so this is a decision about + the API's future rather than about a live consumer, which is why it + is recorded rather than left implicit. - **`panel_placement_can_fall_back` remains the preflight**, unchanged in role: it measures whether this frontend places side requests in the panel *right now*. With the invalidating mutations refused, that @@ -468,6 +515,18 @@ authoritative tip** — the ref, not a SHA. Recover with And reverting the byte comparison to `to_str()?` fails the `invalid utf-8` row with mlua's conversion error, on content. + + **Revision 9's, run across all three suites and the lib:** restore + `panel_commit_dedication_refusal` to reading only the innermost + contract (`.last()`, which is exactly revision 8's swapped slot) → + **only** `a_nested_commit_cannot_mask_an_outer_panel_restriction` + fails. The other 13 pins, `journey_acceptance` (31), + `dired_acceptance` (47) and `cargo test --lib` (1920) all stay green, + which is what makes the new test the pin for this defect and not a + restatement of the depth-1 one. Note the ordinary-nesting pin + deliberately survives that mutation — it exists to fail the *other* + candidate fix (prohibit nesting), so the two are a pair rather than + one test written twice. - **The public API #227 adopts against (Q#DC-5), pinned so it is a contract rather than an intention:** `pmacs.window.commit_to(dest, body [, profile])`. Profile is an diff --git a/docs/destination-capture-framing.md b/docs/destination-capture-framing.md index fafa83e..2e19165 100644 --- a/docs/destination-capture-framing.md +++ b/docs/destination-capture-framing.md @@ -1,17 +1,54 @@ # A destination capture any async continuation can use -**Status: revision 8. The mechanism is implemented at `0efc8c0`; the -correctness blocker revisions 6–8 carry is IMPLEMENTED, in revision 8's -shape, with §3's enumeration performed and recorded below.** Revisions -6 and 7 proposed fixes that review rejected; **neither is in the tree**, -and the two paragraphs describing them are kept as the record of why -this shape and not those. +**Status: revision 9. The mechanism is implemented at `0efc8c0`; the +correctness blocker revisions 6–9 carry is IMPLEMENTED, in revision 8's +shape with revision 9's scope correction, and §3's enumeration is +performed and recorded below.** Revisions 6 and 7 proposed fixes that +review rejected; **neither is in the tree**, and the two paragraphs +describing them are kept as the record of why this shape and not those. *(Revisions 2–5 said "Pre-implementation. Awaiting approval" while the ledger recorded the lane as approved and implemented. Same contradiction class this document keeps correcting elsewhere, left standing in its own header.)* +**Revision 9 fixes a hole in revision 8's guard — one that is about the +guard's SCOPE, not about which mutations it names.** Revision 8 refuses, +inside a `"panel"` commit, the mutations that would make its relaxed +preflight wrong. But a **nested `commit_to` REPLACED** the enclosing +contract with its own and restored it afterwards (`src/editor.rs:129`, +`src/lua_bindings/window_panel.rs`), so the outer restriction went out of +force for the whole of the inner body. Review reproduced the sequence: +an outer `"panel"` commit passes the relaxed preflight; a nested +`"document"` commit masks its contract; the nested callback dedicates the +side slot and **is not refused**; the outer commit resumes, its side +request falls back, and it overwrites a newer document — the original +P1a failure, reached through one extra call. + +**What this invalidated, precisely.** *Not* §3's enumeration of +dedication write sites. That enumeration was performed against the tree, +it is still complete, and every site in it is still guarded. What was +wrong was the surrounding claim — that the guard was **in force for the +whole outer body**. §3's "PREFLIGHT STAYS WHERE IT IS" paragraph and the +enumeration that follows it are therefore kept and **qualified**, not +withdrawn. + +**The fix: contracts COMPOSE across nested scopes; the strictest active +restriction wins.** The core holds a *stack* of contracts rather than one +slot: `commit_to` pushes and pops rather than swapping, and the +dedication guard consults **every** contract in force rather than the +innermost. Matching stays per frontend, so a nested commit for a +different frontend may still dedicate *its* side slot — that cannot +change where this frontend's side request lands. The alternative shape, +**prohibiting nested `commit_to` outright**, was rejected: it closes the +hole by forbidding a construction no rule objects to. `commit_to` is +public Lua API for saying where a continuation's result belongs, and a +body that commits to a second destination (a diff beside a status panel) +is where #227's adoption is heading. Only the *restriction* needed +preserving. **Detecting the dedication when the outer commit resumed was +not available**: by then the mutation has happened, which is a late +refusal, which is what revision 7 was rejected for. + **Revision 8 rejects BOTH of the previous two fixes and takes a third shape.** Revision 6 predicted the fallback at preflight (the body can change it). Revision 7 moved enforcement to the placement boundary — @@ -338,6 +375,15 @@ invalidate the scope's guarantee is rejected rather than predicted around. With them refused, the preflight measurement cannot go stale, and refusal stays mutation-free on the normal `(false, reason)` path. +**"Inside a panel-profile commit" MEANS THE WHOLE BODY, INCLUDING ANY +NESTED `commit_to` (revision 9), and the unqualified version of that +phrase is what revision 8 got wrong.** Contracts **compose**: the core +holds a stack, `commit_to` pushes and pops rather than swapping, and the +guard consults every contract in force rather than the innermost. Read +every "inside a `\"panel\"` commit" below with that scope attached. +Nesting itself is *not* refused — only the mutation is, so a nested +commit that touches no dedication runs exactly as it did. + **The mutation surface is narrow, which is what makes this tight rather than aspirational:** @@ -361,10 +407,21 @@ first are: guarding only route 1 passes revision 8's test while keeping the original defect.** -**THE ENUMERATION, PERFORMED. It is CLOSED, and it is closed for a -structural reason rather than by inspection stopping when it ran out of -ideas.** Recorded here as the framing required, with what was looked -for, what was found, and what cannot be ruled out. +**THE ENUMERATION, PERFORMED. It is CLOSED as an enumeration of WRITE +SITES, and it is closed for a structural reason rather than by inspection +stopping when it ran out of ideas.** Recorded here as the framing +required, with what was looked for, what was found, and what cannot be +ruled out. + +**Read "closed" as scoped to the question it answers (revision 9).** It +answers *which writes can dedicate the side slot*, and that answer +survived review of the nesting defect intact — every site below is real +and every one is still guarded. It says nothing about *when the guard is +in force*, and that is the axis revision 8 got wrong: a nested +`commit_to` used to mask the enclosing contract, so all five reachable +sites were momentarily unguarded together. A complete list of write sites +is not a complete argument until the guard's extent is stated too, which +is what the composing-contracts paragraph above now does. *Step 1 — how few pieces of state can matter.* `resolve_placement` reaches `Ordinary` from a side request through exactly two branches, so @@ -431,6 +488,15 @@ directly, reopens it. `Window::params.dedicated` is a public field, so the compiler does not enforce the funnel — the acceptance rows are what would catch a regression, one per reachable site. +**And it never ruled out a defect in the guard's EXTENT, which is what +revision 9 found.** Nothing above is about *when* +`panel_commit_dedication_refusal` answers; a list of write sites cannot +notice that the contract it reads was masked by a nested scope. The +acceptance suite now drives the same write-site rows at **two depths** — +directly in a `"panel"` body, and through a nested `commit_to` — so a +route guarded at one depth and not the other fails loudly rather than +being covered by the enumeration's word "closed". + **If the enumeration had turned out open-ended**, the fallback was to **collapse the two profiles** — run all four checks always, losing the panel relaxation. That is safe, simple, and honest; it is not the @@ -630,6 +696,29 @@ incidental: no arguments is what keeps capture profile-blind. passes on a design that lets the body mutate freely and merely declines the final installation, leaving every other side effect behind. The refusal must land on the mutation, not on the outcome. +- **THE SAME WRITE-SITE ROWS, DRIVEN THROUGH A NESTED `commit_to`** + (revision 9), in their own test: an outer `"panel"` commit whose body + opens a nested **`"document"`** commit — a perfectly valid one, whose + destination is captured fresh inside the outer body so it passes all + four of its own checks and its callback really runs — and *that* + callback attempts the dedication. Asserted: the attempt is **refused**, + the slot is **still undedicated** afterwards, and the outer commit's + destination is **intact** (its result lands in the panel; the user's + newer document buffer survives). The bullet above cannot catch this — + its mutation runs at commit depth 1, where revision 8's single-slot + contract was the right one to read. Rows per write site rather than one + row, because a fix that reinstated the outer contract for only one site + would pass a single-row version. +- **ORDINARY NESTING STILL WORKS**, asserted rather than assumed: a + nested `commit_to` that touches no dedication is accepted, its body + runs, and its return value comes back through both frames. This is the + pin against the other candidate fix — prohibiting nested `commit_to` + outright — which would close the hole by forbidding a shape no rule + objects to. Two further assertions, and the second is the one a + `pop`-shaped fix gets wrong: the enclosing restriction is **back in + force after the nested commit returns** (popped, not cleared), and + **outside every commit dedication is ordinary again**, so the fix + leaked no permanent restriction onto the editor. - **A `"panel"` commit that really lands in the panel still skips checks 2–4** — otherwise the fix has quietly collapsed the two profiles into one and the parameterization buys nothing. diff --git a/src/editor.rs b/src/editor.rs index c3da5bb..285f1f6 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -119,13 +119,21 @@ impl ScopedFrontend { } /// Enter a background frontend scope, also swapping the core's - /// ambient `active_frontend` and publishing `contract`. All three are + /// ambient `active_frontend` and **pushing** `contract`. All three are /// restored on drop, on every exit path including a raising callback. /// /// The frontend comes from `contract.destination` rather than being /// passed separately: a scope entered for one frontend while carrying /// another's destination would let the placement guard check the /// wrong window, and there is no caller that wants them to differ. + /// + /// **The contract is pushed, not swapped (Q#DC-2, revision 9).** The + /// frontend override and the ambient frontend are *substitutions* — + /// an inner scope means what it says and the outer one resumes + /// afterwards — but a contract is a *restriction*, and a nested scope + /// masking one would suspend it for the extent of the inner body + /// while the outer commit's relaxed preflight still depended on it. + /// See [`crate::editor_core::EditorCore::push_commit_contract`]. pub(crate) fn enter( &self, core: &SharedCore, @@ -134,11 +142,11 @@ impl ScopedFrontend { ) -> ScopedFrontendGuard { let frontend_id = contract.destination.frontend; let previous = self.0.replace(Some(frontend_id)); - let (previous_active, previous_contract) = { + let (previous_active, contract_depth) = { let mut core = core.borrow_mut(); let was = core.active_frontend; core.active_frontend = frontend_id; - (was, core.enter_commit_contract(Some(contract))) + (was, core.push_commit_contract(contract)) }; let previous_commit = commit_scope.0.replace(true); ScopedFrontendGuard { @@ -146,7 +154,7 @@ impl ScopedFrontend { core: core.clone(), previous, previous_active, - previous_contract, + contract_depth, commit_scope: commit_scope.clone(), previous_commit, } @@ -158,11 +166,15 @@ pub(crate) struct ScopedFrontendGuard { core: SharedCore, previous: Option, previous_active: FrontendId, - /// The contract in force before this commit, restored with the rest - /// (Q#DC-2). Held here rather than on a separate guard so a - /// `"panel"` profile can never outlive the body that declared it and - /// govern an unrelated later display. - previous_contract: Option, + /// Contract-stack depth to truncate back to (Q#DC-2). Held here + /// rather than on a separate guard so a `"panel"` profile can never + /// outlive the body that declared it and govern an unrelated later + /// display. + /// + /// A depth rather than a saved contract because nesting **composes** + /// (revision 9): this scope adds one restriction and removes exactly + /// that one, leaving every enclosing commit's still in force. + contract_depth: usize, /// Cleared together with the scope, so an awaiting callback cannot /// leave `await` refused after the commit ends (Q#JR14b). commit_scope: CommitScopeActive, @@ -175,7 +187,7 @@ impl Drop for ScopedFrontendGuard { { let mut core = self.core.borrow_mut(); core.active_frontend = self.previous_active; - core.enter_commit_contract(self.previous_contract); + core.exit_commit_contract(self.contract_depth); } self.commit_scope.0.set(self.previous_commit); } diff --git a/src/editor_core.rs b/src/editor_core.rs index 4cc92f1..517989c 100644 --- a/src/editor_core.rs +++ b/src/editor_core.rs @@ -167,8 +167,10 @@ pub enum ResolvedTarget { /// the document profile requires all of them, and the panel profile /// requires only a live `frontend` **while its result really lands in a /// panel**. A side request that falls back into a document window *is* a -/// document replacement, and is held to all of them at the placement -/// boundary ([`EditorCore::fallback_commit_refusal`]). Capture stays +/// document replacement, so the panel profile's relaxed preflight is +/// taken only when the fallback cannot happen, and the mutations that +/// would manufacture one mid-commit are refused at the attempt +/// (`EditorCore::panel_commit_dedication_refusal`). Capture stays /// profile-blind so a caller does not have to know at capture time what /// it will do at commit time. /// @@ -202,16 +204,19 @@ pub enum CommitProfile { Document, /// The body puts its result in a bottom panel rather than in the /// captured document window, and so does not depend on checks 2–4 — - /// **for as long as its result really lands in a panel**. When a side - /// request falls back into a document window the relaxation is - /// withdrawn at the placement boundary, which is the only place the - /// fallback is a fact rather than a guess - /// ([`EditorCore::display_buffer`]). + /// **for as long as its result really lands in a panel**. The + /// preflight grants the relaxation only when a fallback into a + /// document window is impossible ([`EditorCore::commit_destination_refusal`]), + /// and what keeps that measurement true for the body's whole extent + /// is that the mutations which would manufacture a fallback are + /// refused at the attempt + /// (`EditorCore::panel_commit_dedication_refusal`). Panel, } /// The contract a `commit_to` body is running under, published on the -/// core for the placement path to consult (Q#DC-2, revision 7). +/// core so the mutations that could invalidate it can consult it +/// (Q#DC-2, revisions 8 and 9). /// /// **Why this exists rather than a preflight prediction.** Revision 6 /// tried to decide at preflight whether a `"panel"` commit's placement @@ -221,12 +226,17 @@ pub enum CommitProfile { /// the body itself, which is arbitrary Lua running synchronously and can /// change the very state the snapshot measured — obtain the panel, set /// it `dedicated`, then request a side display. A snapshot cannot bind -/// that. The fact "this asked for a side and landed in a document -/// window" is only ever known where placement resolves, so that is where -/// the document preconditions are enforced. +/// that. So the preflight stays where it is and the contract is what +/// lets those mutations be **refused at the attempt**, which is the only +/// point early enough to leave nothing behind +/// (`EditorCore::panel_commit_dedication_refusal`). /// -/// Installed and restored by the same guard that scopes the frontend, so -/// the two can never disagree about whether a commit is on the stack. +/// Pushed and popped by the same guard that scopes the frontend, so the +/// two can never disagree about whether a commit is on the stack. +/// **Pushed** rather than swapped: a contract is a restriction, and a +/// nested `commit_to` must add to the ones in force rather than mask +/// them for the extent of its body +/// (`EditorCore::push_commit_contract`, revision 9). #[derive(Clone, Copy, Debug)] pub struct CommitContract { /// The destination the continuation captured. @@ -690,14 +700,34 @@ pub struct EditorCore { /// slot; the producer clears any untaken record when the fan-out /// returns. typed_edit_armed: Option<(FrontendId, TypedEditRecord)>, - /// The `commit_to` contract currently on the stack, if any (Q#DC-2). + /// Every `commit_to` contract currently on the stack, outermost + /// first (Q#DC-2, revision 9). /// - /// Private and `pub(crate)`-free on purpose: it is installed only by - /// [`crate::editor::ScopedFrontend::enter`]'s guard, which restores - /// the previous value on every exit path including a raising body. - /// Nothing outside this crate can set it, so a `"panel"` profile is - /// not something Lua can claim for a placement it did not commit to. - commit_contract: Option, + /// **A STACK, NOT A SLOT, and that is the whole of revision 9's + /// fix.** Revision 8 held one contract and had a nested `commit_to` + /// replace it for the inner body's extent. That MASKED the enclosing + /// contract: an outer `"panel"` commit took the relaxed preflight, + /// its body opened a nested `"document"` commit, and inside that + /// nested body the very mutation the outer commit's relaxation + /// depends on — dedicating the one side slot — was no longer refused, + /// because the guard consulted only the innermost contract. The outer + /// commit then resumed and fell back into the document window, + /// overwriting a newer buffer, which is exactly the defect the panel + /// profile's relaxation was made safe against. + /// + /// So restrictions **compose** rather than replace: a contract is + /// pushed for its body and popped after, and every restriction + /// pushed by an enclosing commit stays in force for the whole of it, + /// nested scopes included. See + /// [`Self::panel_commit_dedication_refusal`], the one reader. + /// + /// Private and `pub(crate)`-free on purpose: entries are pushed only + /// by [`crate::editor::ScopedFrontend::enter`]'s guard, which + /// truncates back to its own depth on every exit path including a + /// raising body. Nothing outside this crate can push one, so a + /// `"panel"` profile is not something Lua can claim for a placement + /// it did not commit to. + commit_contracts: Vec, } impl EditorCore { @@ -752,22 +782,40 @@ impl EditorCore { query_replace: None, typed_edit_pending: None, typed_edit_armed: None, - commit_contract: None, + commit_contracts: Vec::new(), } } - /// Install `contract` for the duration of a `commit_to` body, - /// returning the previous one for the guard to restore. + /// Push `contract` for the duration of a `commit_to` body, returning + /// the depth [`Self::exit_commit_contract`] must truncate back to. + /// + /// **Pushes rather than replaces (revision 9).** A nested `commit_to` + /// adds its contract to the ones already in force instead of masking + /// them, so an enclosing `"panel"` commit's mutation refusal covers + /// its *whole* body — including the part that runs inside a nested + /// commit of a different profile. Replacing was revision 8's defect: + /// the guard read only the innermost contract, so a nested + /// `"document"` commit was a hole through which the body could + /// dedicate the side slot the outer relaxation rests on. /// /// Crate-private and paired with the frontend scope rather than a /// standalone setter: a contract that could be installed without - /// being restored would outlive its body and silently govern the - /// next unrelated display. - pub(crate) fn enter_commit_contract( - &mut self, - contract: Option, - ) -> Option { - std::mem::replace(&mut self.commit_contract, contract) + /// being popped would outlive its body and silently govern the next + /// unrelated display. + pub(crate) fn push_commit_contract(&mut self, contract: CommitContract) -> usize { + let depth = self.commit_contracts.len(); + self.commit_contracts.push(contract); + depth + } + + /// Drop every contract pushed at or above `depth`. + /// + /// Truncation rather than a bare `pop` so the guard restores exactly + /// the set that was in force when it was entered, whatever happened + /// in between — the same reason the frontend scope saves a value + /// rather than assuming it can invert its own change. + pub(crate) fn exit_commit_contract(&mut self, depth: usize) { + self.commit_contracts.truncate(depth); } /// Build a core from raw bytes under `name`. Used by tests. @@ -3143,8 +3191,9 @@ impl EditorCore { /// frontend id exists. A frontend with no document window yields a /// destination carrying only `frontend` — enough for a panel commit /// that really places in the panel, and refused by a document commit - /// (or by a panel commit that falls back into a document window, see - /// [`Self::fallback_commit_refusal`]) with a reason naming the + /// (or by a panel commit on a frontend where a side request would + /// fall back into a document window, see + /// [`Self::commit_destination_refusal`]) with a reason naming the /// missing window. Returning `None` here instead would push the /// caller back onto ambient state, which is the misrouting the /// capture exists to remove. @@ -3237,15 +3286,16 @@ impl EditorCore { /// buffer, registers a handle, paints) long before it reaches any /// call that could refuse, so a late refusal leaves debris behind. /// - /// **This is an early refusal, NOT the guarantee.** For the panel - /// profile it can only read the state that holds *now*, and the body - /// is arbitrary synchronous Lua that may change it — dedicate the - /// side slot, then request a side display. The guarantee that a - /// `"panel"` commit never replaces a newer document therefore lives - /// at the placement boundary in [`Self::display_buffer`], where the - /// fallback is a fact. What this buys is that the common case — a - /// frontend that simply cannot render a panel — refuses **before** - /// the body allocates anything. + /// **This measurement is only half the guarantee.** For the panel + /// profile it can read only the state that holds *now*, and the body + /// is arbitrary synchronous Lua that could change it — dedicate the + /// side slot, then request a side display. What keeps the + /// measurement true is that those mutations are **refused at the + /// attempt**, for the body's whole extent including any nested + /// `commit_to` (`Self::panel_commit_dedication_refusal`). Refusing + /// at the placement boundary instead was revision 7, and it was + /// rejected: by then the body has allocated buffers, handles and + /// paint, which is the debris this preflight exists to avoid. #[must_use] pub fn commit_destination_refusal( &self, @@ -4199,17 +4249,18 @@ impl EditorCore { /// dedicated, and a second one is never created, so a different /// buffer falls through instead (Q#BP3 2.iii). /// - /// **A PREDICTION, AND ONLY USED AS ONE.** This is consulted by + /// **A MEASUREMENT, AND NOT SELF-SUPPORTING.** This is consulted by /// [`Self::commit_destination_refusal`] to refuse the statically /// knowable case *before* a body allocates anything — a frontend that /// cannot render a panel at all will not acquire the capability - /// mid-body. It is **not** what makes the panel profile safe. A - /// `commit_to` body is arbitrary synchronous Lua and can dedicate the - /// side slot itself between this answer and the placement it - /// describes; refusing `await` prevents another coroutine + /// mid-body. On its own it would **not** make the panel profile safe: + /// a `commit_to` body is arbitrary synchronous Lua and could dedicate + /// the side slot itself between this answer and the placement it + /// describes, and refusing `await` prevents another coroutine /// interleaving, not the body rewriting the state it was measured - /// against. The guarantee is enforced where the fallback is a fact, - /// in [`Self::fallback_commit_refusal`]. + /// against. What holds the measurement true is + /// `Self::panel_commit_dedication_refusal`, which refuses exactly + /// those mutations for the body's whole extent. /// /// Arm 2 is answered **conservatively**: `resolve_placement` falls /// back only when the arriving buffer differs from the dedicated one, @@ -4233,9 +4284,9 @@ impl EditorCore { } /// **The guarantee** behind the `"panel"` commit profile (Q#DC-2, - /// revision 8): inside such a commit, the operations that would make - /// this frontend's side request fall back are **refused at the - /// attempt**. + /// revisions 8 and 9): anywhere inside such a commit — nested + /// `commit_to` scopes included — the operations that would make this + /// frontend's side request fall back are **refused at the attempt**. /// /// # The defect this closes /// @@ -4270,6 +4321,25 @@ impl EditorCore { /// fallback never comes into existence, and nothing needs refusing /// late. /// + /// # Every enclosing contract, not just the innermost (revision 9) + /// + /// This scans the whole contract stack. Revision 8 read a single + /// slot, and a nested `commit_to` replaced it — so an outer + /// `"panel"` commit whose body opened a nested `"document"` commit + /// had its restriction **masked** for that body's extent, and the + /// nested callback could dedicate the side slot the outer relaxation + /// rests on. The outer commit then resumed and fell back into the + /// document window, overwriting a newer buffer: the original defect, + /// reachable through one extra call. Detecting it when the outer + /// commit resumed would have been a late refusal, which revision 7 + /// was already rejected for. The restriction has to hold for the + /// whole body, so **the strictest active restriction wins** and + /// nesting is otherwise untouched. + /// + /// Matching is per **frontend**, not per stack: a nested commit for a + /// *different* frontend may dedicate *its* side slot, because that + /// cannot change where this frontend's side request lands. + /// /// # The enumeration this rests on /// /// [`Self::resolve_placement`] can only reach @@ -4312,9 +4382,18 @@ impl EditorCore { /// a panel degrades gracefully exactly as it does today; this /// refuses the *mutation that manufactures* a fallback, never the /// fallback itself. + /// * **Nesting is untouched.** Only the mutation is refused, not the + /// nested `commit_to` that reaches it, so a nested commit that does + /// not dedicate this frontend's side slot runs exactly as before. + /// Prohibiting nesting outright would have closed the hole by + /// forbidding a shape no rule objects to (revision 9). pub(crate) fn panel_commit_dedication_refusal(&self, fid: FrontendId) -> Option { - let contract = self.commit_contract.as_ref()?; - if contract.profile != CommitProfile::Panel || contract.destination.frontend != fid { + // ANY enclosing contract, not the innermost one: a nested commit + // composes with the restrictions already in force rather than + // masking them (revision 9). + if !self.commit_contracts.iter().any(|contract| { + contract.profile == CommitProfile::Panel && contract.destination.frontend == fid + }) { return None; } Some( diff --git a/src/lua_bindings/window_panel.rs b/src/lua_bindings/window_panel.rs index 9f7f137..01f4d61 100644 --- a/src/lua_bindings/window_panel.rs +++ b/src/lua_bindings/window_panel.rs @@ -538,20 +538,19 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result let profile = commit_profile(&profile)?; // The preflight itself lives on the core - // (`commit_destination_refusal`), because the panel - // profile's relaxation now has a SECOND evaluation - // site --- the placement boundary, where a fallback - // into a document window stops being a prediction and - // becomes a fact --- and two hand-written copies of - // the same three checks is how the backstop ends up - // weaker than the thing it backs. + // (`commit_destination_refusal`) rather than being + // hand-written here, so the panel profile's + // relaxation is decided in one place: two copies of + // the same three checks is how one of them ends up + // weaker than the other. // - // What survives here, and only here: an early refusal - // costs the body nothing, so the statically knowable - // case (a frontend that cannot render a panel at all) - // never reaches the body's buffer creation. The - // GUARANTEE is not this call; see - // `EditorCore::fallback_commit_refusal`. + // This call is only HALF the panel guarantee. It + // measures whether this frontend places side requests + // in the panel; what keeps that measurement true + // while the body runs --- nested `commit_to` scopes + // included --- is + // `EditorCore::panel_commit_dedication_refusal`, + // which refuses the mutations that would falsify it. let refusal = cc.borrow().commit_destination_refusal(&dest, profile); if let Some(reason) = refusal { let mut out = mlua::MultiValue::new(); @@ -581,10 +580,18 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result // guard drops -- on the normal return AND on a // raising callback, which is why the result is // captured rather than `?`-propagated through the - // drop. The contract rides with the scope because the - // placement boundary needs to know, for every display - // this body performs, which destination and which - // profile it is running under. + // drop. The contract rides with the scope because + // every mutation this body reaches has to know which + // destination and which profile it is running under. + // + // A NESTED `commit_to` PUSHES its contract onto the + // ones already in force rather than replacing them + // (Q#DC-2, revision 9). Replacing was a hole: an + // outer `"panel"` commit's mutation refusal went out + // of force for the extent of a nested body, which is + // long enough to dedicate the side slot its relaxed + // preflight depends on. Nesting itself is allowed -- + // only the mutation is refused. let result = { let _guard = scope.enter( &cc, diff --git a/tests/destination_capture_acceptance.rs b/tests/destination_capture_acceptance.rs index fee4ba9..54fdf99 100644 --- a/tests/destination_capture_acceptance.rs +++ b/tests/destination_capture_acceptance.rs @@ -473,7 +473,7 @@ fn the_preflight_matrix_holds_in_both_profiles() { } // --------------------------------------------------------------------------- -// §7 — the panel profile's relaxation is CONDITIONAL (Q#DC-2, revision 7) +// §7 — the panel profile's relaxation is CONDITIONAL (Q#DC-2, rev 6–9) // --------------------------------------------------------------------------- /// The Lua a `"panel"` continuation runs: put a result buffer in the @@ -490,6 +490,65 @@ const PANEL_BODY: &str = "pmacs.window.display(pmacs.buffer.create('*result*'), const PANEL_ARRANGED: &str = "pmacs.window.display(pmacs.buffer.create('*pinned*'), \ { side = 'bottom', dedicated = false, select = false })"; +/// Every route by which a `commit_to` body can reach a write to a **side** +/// window's `Window::params.dedicated`, as `(label, arrangement before the +/// capture, the attempted mutation)`. +/// +/// **One row per WRITE SITE, not per call spelling** (§3's enumeration). A +/// single row is exactly what would let a second route keep the defect — +/// which is not hypothetical: review found the `display{side, dedicated}` +/// route *after* `set_params` was specified, and one spelling of it +/// reaches three different writes. +/// +/// | row | reaches | +/// |---|---| +/// | `set_params` | the direct write in the binding (Q#BP2c) | +/// | `display{side, dedicated}` replacing | `apply_placement`'s **replacing** arm | +/// | `display{side, dedicated}` same buffer | its **non-replacing** arm | +/// | `display{side, dedicated}` with no panel | its **created** arm | +/// +/// The three `display` rows converge on one guard, in `display_buffer` — +/// `apply_placement` has exactly one caller, so every request-driven +/// dedication passes through it. They are still separate rows because that +/// convergence is a property of today's call graph, and a row per arm +/// fails loudly if it stops holding. +/// +/// Shared by the two tests that drive them, at commit depth 1 and through +/// a nested commit: a route guarded at one depth and not the other is the +/// defect revision 9 fixes, and a table each would let the two drift. +const DEDICATION_ROUTES: [(&str, &str, &str); 4] = [ + ( + "set_params", + PANEL_ARRANGED, + "pmacs.window.set_params(pmacs.window.panel(), { dedicated = true })", + ), + ( + "display{side, dedicated} replacing", + PANEL_ARRANGED, + "pmacs.window.display(pmacs.buffer.create('*usurp*'), + { side = 'bottom', dedicated = true, select = false })", + ), + ( + // The same buffer the panel already shows: `replacing` is false, + // so this lands in a DIFFERENT arm of the same function, which a + // row against the replacing arm alone would not exercise. + "display{side, dedicated} same buffer", + PANEL_ARRANGED, + "pmacs.window.display(pmacs.window.buffer(pmacs.window.panel()), + { side = 'bottom', dedicated = true, select = false })", + ), + ( + // NO panel at capture time: the preflight relaxes because + // `side_window_for` is None (a side request would CREATE a panel, + // never fall back). The body then creates one dedicated, which + // makes the next side request fall back. + "display{side, dedicated} creating the panel", + "", + "pmacs.window.display(pmacs.buffer.create('*usurp*'), + { side = 'bottom', dedicated = true, select = false })", + ), +]; + /// Arrange one of the two reasons a side request falls back into a /// document window, and assert the arrangement took. /// @@ -644,25 +703,8 @@ fn a_panel_commit_that_falls_back_runs_the_document_preflight() { /// /// # One row per WRITE SITE, not per call spelling /// -/// A single row is exactly what would let a second route keep the -/// defect — which is not hypothetical: review found the -/// `display{side, dedicated}` route *after* `set_params` was specified. -/// So the rows are chosen to hit each distinct write to -/// `Window::params.dedicated` that a side window can receive, rather -/// than each way of phrasing the call: -/// -/// | row | reaches | -/// |---|---| -/// | `set_params` | the direct write in the binding (Q#BP2c) | -/// | `display{side, dedicated}` replacing | `apply_placement`'s **replacing** arm | -/// | `display{side, dedicated}` same buffer | its **non-replacing** arm | -/// | `display{side, dedicated}` with no panel | its **created** arm | -/// -/// The three `display` rows converge on one guard, in `display_buffer` — -/// `apply_placement` has exactly one caller, so every request-driven -/// dedication passes through it. They are still separate rows because -/// that convergence is a property of today's call graph, and a row per -/// arm fails loudly if it stops holding. +/// The rows are `DEDICATION_ROUTES`, which documents why it is a write-site +/// enumeration rather than a list of call spellings. /// /// The rest of the enumeration is **unreachable rather than refused** /// and is recorded in `EditorCore::panel_commit_dedication_refusal`, @@ -678,42 +720,7 @@ fn a_panel_commit_that_falls_back_runs_the_document_preflight() { /// drops rows 2–4 — and every other test in this file still passes. #[test] fn a_body_that_tries_to_create_the_fallback_is_refused_at_the_attempt() { - // (label, panel arrangement before the capture, attempted mutation) - let routes: [(&str, &str, &str); 4] = [ - ( - "set_params", - PANEL_ARRANGED, - "pmacs.window.set_params(pmacs.window.panel(), { dedicated = true })", - ), - ( - "display{side, dedicated} replacing", - PANEL_ARRANGED, - "pmacs.window.display(pmacs.buffer.create('*usurp*'), - { side = 'bottom', dedicated = true, select = false })", - ), - ( - // The same buffer the panel already shows: `replacing` is - // false, so this lands in a DIFFERENT arm of the same - // function, which a row against the replacing arm alone - // would not exercise. - "display{side, dedicated} same buffer", - PANEL_ARRANGED, - "pmacs.window.display(pmacs.window.buffer(pmacs.window.panel()), - { side = 'bottom', dedicated = true, select = false })", - ), - ( - // NO panel at capture time: the preflight relaxes because - // `side_window_for` is None (a side request would CREATE a - // panel, never fall back). The body then creates one - // dedicated, which makes the next side request fall back. - "display{side, dedicated} creating the panel", - "", - "pmacs.window.display(pmacs.buffer.create('*usurp*'), - { side = 'bottom', dedicated = true, select = false })", - ), - ]; - - for (label, arrange, attempt) in routes { + for (label, arrange, attempt) in DEDICATION_ROUTES { let s = editor(); exec(&s, arrange); @@ -795,6 +802,266 @@ fn a_body_that_tries_to_create_the_fallback_is_refused_at_the_attempt() { } } +/// **N** — a **nested** `commit_to` cannot mask the restriction an +/// enclosing `"panel"` commit is relying on (revision 9). +/// +/// # The defect +/// +/// Revision 8 held **one** contract on the core, and entering a commit +/// *replaced* it for the inner body's extent, restoring it afterwards +/// (`ScopedFrontend::enter`). So the guarantee above had a hole exactly +/// one call wide: +/// +/// ```lua +/// pmacs.window.commit_to(outer, function() -- "panel": relaxed preflight +/// pmacs.window.commit_to(inner, function() -- "document": MASKS the outer contract +/// pmacs.window.set_params(pmacs.window.panel(), { dedicated = true }) +/// end) -- ...and succeeds +/// pmacs.window.display(result, { side = "bottom" }) +/// end, "panel") -- ...which now FALLS BACK +/// ``` +/// +/// Every step is legal on its own. The outer commit's relaxed preflight +/// was granted because this frontend places side requests in the panel; +/// the nested commit put the refusal that keeps that true out of force; +/// and the outer commit then resumed and overwrote the user's newer +/// document buffer — the original P1a failure, reached through one extra +/// call rather than through a route the write-site enumeration missed. +/// +/// **What this invalidated, precisely.** Not §3's enumeration of +/// dedication write sites: all four rows below are the same writes, and +/// each is still guarded. What was wrong was the claim that the guard was +/// **in force for the whole outer body**. So the fix composes contracts +/// instead of replacing them — the strictest active restriction wins — +/// and the enumeration is inherited unchanged. +/// +/// **A late refusal would not have been a fix**, and revision 7 was +/// already rejected for being one: by the time the outer commit resumes, +/// the nested callback has already dedicated the slot. The dedication has +/// to be *prevented*, which is why this asserts on the nested attempt and +/// on the slot's state, not merely on where the outer result landed. +/// +/// # Why the rows are the same four +/// +/// A fix that reinstated the outer contract for only one write site would +/// pass a single-row version of this. `DEDICATION_ROUTES` therefore drives +/// both depths, so a route guarded at depth 1 and not through a nested +/// scope fails loudly. +/// +/// *Mutation:* restore `push_commit_contract`/`exit_commit_contract` to a +/// single swapped slot (revision 8's `enter_commit_contract`) and only +/// this test fails. +#[test] +fn a_nested_commit_cannot_mask_an_outer_panel_restriction() { + for (label, arrange, attempt) in DEDICATION_ROUTES { + let s = editor(); + exec(&s, arrange); + + let panel_before = s.core.borrow().side_window_for(FrontendId::LOCAL); + if let Some(panel) = panel_before { + assert!( + !dedicated(&s, panel), + "{label}: the slot must start UNDEDICATED, or the outer preflight would \ + have refused and this row would be re-proving the preflight" + ); + } + + capture(&s); + let doc = local_window(&s); + // The user's newer buffer: what the outer commit overwrites if its + // side request is made to fall back. + exec( + &s, + "pmacs.window.switch_buffer(pmacs.buffer.create('*newer*'))", + ); + + // The nested commit is a plain, valid, DOCUMENT-profile commit — + // its destination is captured fresh inside the outer body, so it + // passes all four checks on its own account and its callback + // really runs. Nothing about it is malformed; that is the point. + commit_body( + &s, + Some("'panel'"), + &format!( + "local inner = pmacs.window.capture_destination() + nested_ran = false + local caught, a = pcall(pmacs.window.commit_to, inner, function() + nested_ran = true + {attempt} + end) + nested_raised = (not caught) and tostring(a) or nil + {PANEL_BODY}" + ), + ); + + assert!( + ran(&s), + "{label}: the outer body must have run -- its preflight could not have known" + ); + assert!( + eval::(&s, "return nested_ran"), + "{label}: the nested callback must have run -- a nested commit refused at its \ + own preflight would prove nothing about masking" + ); + + // 1. THE MUTATION IS STILL REFUSED, inside the nested scope. + let nested_raised: Option = eval(&s, "return nested_raised"); + let nested_raised = nested_raised.unwrap_or_else(|| { + panic!( + "{label}: the enclosing \"panel\" restriction must survive the nested \ + commit -- masking it is revision 9's defect" + ) + }); + assert!( + nested_raised.contains("cannot dedicate the side window"), + "{label}: the refusal must name the operation it is refusing; got \ + {nested_raised:?}" + ); + assert!( + nested_raised.contains("\"panel\" commit_to"), + "{label}: and why it is refused here specifically; got {nested_raised:?}" + ); + + // 2. THE SLOT IS STILL UNDEDICATED. Prevention, not detection: + // the outer commit resumes after the nested one returns, so a + // refusal that arrived then would already be too late. + let panel_after = s.core.borrow().side_window_for(FrontendId::LOCAL); + let panel_after = panel_after.unwrap_or_else(|| { + panic!("{label}: the outer body's own side display must have found a panel") + }); + assert!( + !dedicated(&s, panel_after), + "{label}: a refused mutation must not have happened -- the outer commit's \ + relaxed preflight rests on the slot still being free" + ); + if let Some(before) = panel_before { + assert_eq!( + panel_after, before, + "{label}: the refusal must not have replaced the side slot" + ); + } + + // 3. THE OUTER COMMIT'S DESTINATION IS INTACT: its result went to + // the PANEL, and the user's newer document buffer survived. + // This is the assertion that fails loudest on the unfixed + // tree — the outer side request falls back and `*result*` + // lands on top of `*newer*`. + assert!( + ok(&s), + "{label}: the outer commit must still be accepted; got {:?}", + reason(&s) + ); + assert_eq!(raised(&s), None, "{label}: the outer commit must not raise"); + assert_eq!( + name_in(&s, panel_after), + "*result*", + "{label}: the outer \"panel\" commit's result belongs in the panel" + ); + assert_eq!( + name_in(&s, doc), + "*newer*", + "{label}: and the user's newer buffer must survive" + ); + } +} + +/// **P** — nesting itself is **not** forbidden: a nested `commit_to` that +/// touches no dedication runs, returns its value, and leaves the enclosing +/// restriction exactly as it found it. +/// +/// The other acceptable shape for revision 9's fix was to refuse a nested +/// `commit_to` outright. That closes the hole by forbidding a construction +/// no rule objects to — `commit_to` is public Lua API whose whole purpose +/// is to let a continuation say where its result belongs, and a body that +/// commits to a *second* destination (a diff beside a status panel, say) +/// is the shape #227's adoption is heading for. Only the **restriction** +/// needed preserving, so only the mutation is refused. +/// +/// Three things are pinned, and the third is the one a `Vec::pop`-shaped +/// fix would get wrong: +/// +/// 1. the nested commit is accepted, its body runs, and its result value +/// comes back through both frames; +/// 2. the enclosing restriction is back in force **after** the nested +/// commit returns — not cleared with it; +/// 3. **outside** every commit, dedication is ordinary and allowed — +/// otherwise the fix would have leaked a permanent restriction onto the +/// editor. +/// +/// *Mutation:* refuse nested `commit_to` at the attempt, and this fails +/// while the masking test above still passes — which is what makes the two +/// a pair rather than one test written twice. +#[test] +fn an_ordinary_nested_commit_still_runs_and_restores_the_outer_restriction() { + let s = editor(); + exec(&s, PANEL_ARRANGED); + let panel = s + .core + .borrow() + .side_window_for(FrontendId::LOCAL) + .expect("the arrangement creates the panel"); + capture(&s); + + commit_body( + &s, + Some("'panel'"), + "local inner = pmacs.window.capture_destination() + -- A nested commit doing ordinary work: no dedication anywhere. + nested_ok, nested_value = pmacs.window.commit_to(inner, function() + pmacs.window.display(pmacs.buffer.create('*nested*'), { select = false }) + return 'inner-result' + end) + -- And the enclosing restriction is back afterwards. + local caught, a = pcall(pmacs.window.set_params, + pmacs.window.panel(), { dedicated = true }) + after_nested_raised = (not caught) and tostring(a) or nil", + ); + + assert_eq!(raised(&s), None, "the outer commit must not raise"); + assert!(ok(&s), "the outer commit must be accepted: {}", reason(&s)); + + // 1. The nested commit ran and its value came back through both frames. + assert!( + eval::(&s, "return nested_ok"), + "a nested commit that touches no dedication must be accepted -- forbidding all \ + nesting when only the restriction needed preserving is a behaviour regression" + ); + assert_eq!( + eval::(&s, "return tostring(nested_value)"), + "inner-result", + "the nested body's return value must come back through both commit frames" + ); + assert!( + buffer_exists(&s, "*nested*"), + "the nested body's own work must have happened" + ); + + // 2. The enclosing restriction is back in force after the nested + // commit returned -- popped, not cleared. + let after: Option = eval(&s, "return after_nested_raised"); + let after = after.expect( + "the enclosing \"panel\" restriction must be back in force once the nested commit \ + returns -- a fix that cleared the stack on the inner exit would leave the rest of \ + the outer body unguarded", + ); + assert!( + after.contains("cannot dedicate the side window"), + "and it must be the same refusal; got {after:?}" + ); + assert!(!dedicated(&s, panel), "the slot must still be undedicated"); + + // 3. OUTSIDE every commit, dedication is ordinary again: the guard + // must not have leaked a permanent restriction onto the editor. + exec( + &s, + "pmacs.window.set_params(pmacs.window.panel(), { dedicated = true })", + ); + assert!( + dedicated(&s, panel), + "outside a commit the field is writable as it always was (Q#BP2c)" + ); +} + /// **P** — a `"panel"` commit that falls back with a **still-valid** /// destination lands in the document window, exactly as it does today. /// @@ -809,10 +1076,11 @@ fn a_body_that_tries_to_create_the_fallback_is_refused_at_the_attempt() { /// and then dropped the display on the floor would pass a weaker version /// of this. /// -/// *Mutation:* make `fallback_commit_refusal` refuse whenever a `"panel"` -/// commit falls back, instead of only when a document precondition fails. -/// Both rows fail here; every refusal test still passes, which is what -/// makes this the pin that stops the fix over-reaching. +/// *Mutation:* make `commit_destination_refusal` refuse outright whenever +/// a `"panel"` commit could fall back, instead of holding it to the +/// document preconditions. Both rows fail here; every refusal test still +/// passes, which is what makes this the pin that stops the fix +/// over-reaching. #[test] fn a_panel_commit_that_falls_back_with_a_valid_destination_still_lands() { for cause in ["not panel-capable", "side slot dedicated elsewhere"] {