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"] {