diff --git a/docs/active-work.md b/docs/active-work.md index 7262ab5..c2b19b3 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -265,7 +265,402 @@ 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. -## Worker identity Stage 1 (§9) — IMPLEMENTED, no PR yet +## Destination capture (Q#JR14 generalization) — PR #231 OPEN, revision 9, cleared to merge + +**PR #231** — https://github.com/levineuwirth/pmacs/pull/231. #227 +blocks on this lane. + +The mechanism landed at `0efc8c0`; review found a correctness blocker; +`ca72461` implemented **revision 7**, which review then **also** +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**. + +**The macOS red that blocked this lane, and how it was cleared.** Both +CI attempts at `4654b94` failed `a_pty_resize_blanks_the_host_before_repainting` +on `Test (macos-latest / luajit)`. A control experiment was run at the +exact base commit `0190102`: **five valid observations, all green on +both macOS flavours**, against the branch's 0/2 — 1/C(7,2) = 4.8% under +an equal-rate model. That implicates the branch statistically. **The +diff exonerates it mechanically**: grepping this lane's entire `src/` +diff for `full_grid|resize|resync|Geometry|reconcile_panel_layout` +matches an **import line and nothing else**, and +`full_grid_resync_acceptance` (191 lines) has no panel, side-window, +dedication, display or directory surface at all. Merged on that reading, +with the equal-rate model itself in doubt — see the U4 row, and note a +sixth base attempt reddened on a *third, unrelated* macOS selector +(U8), which is what a background platform failure rate looks like. + +**The original blocker:** the panel profile skipped checks 2–4 on the +claim that a panel result never touches a document window. **Panel +placement falls back to an ordinary document window** when the frontend +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. + +**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 + synchronous Lua and can create the fallback itself. +2. **Revision 7 — enforce at the placement boundary.** Implemented at + `ca72461`, then rejected: `docs/agent-handoff.md:748` requires + `commit_to` to preflight **before** the callback, because + "validating at display time is four mutations too late". A body has + 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.** 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. + +**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: + `panel_capable`, and the one side window's `dedicated`. +- **`panel_capable` is unreachable from a body.** It is written only + where a `FrontendView` is constructed, and nothing in + `src/lua_bindings/` constructs, registers or unregisters one — + `register_frontend_view` has callers only in `daemon.rs` and core + unit tests. +- **Eight writes to `dedicated` exist** (`rg 'params\.dedicated\s*=' + src/`); **four are reachable and a fifth is guarded defensively** — + `apply_placement`'s `Side` created / replacing / non-replacing arms + and `set_params` are the reachable four, and `quit_window`'s + `QuitAction::Restore` is the fifth, proved unreachable below and + guarded anyway. **All five are guarded**, which is the count that + matters; listing four under the word "five" is what an earlier version + of this bullet did. Two `Ordinary` arms are harmless (their target is + never a side window; one only ever clears the flag) and one is a unit + test. +- **The guards are sited where the property converges, not per caller.** + All three `Side` arms are reached through `apply_placement`, which has + **exactly one caller** — so one guard in `display_buffer` covers every + request-driven dedication, including spellings that do not exist yet. + `set_params` is a genuinely separate write and is guarded separately; + dedication does **not** converge before the field itself, and that is + stated rather than papered over. +- **Closing the side window is NOT a route**, checked rather than + assumed: with no side leaf `side_window_for` returns `None` and + placement **creates** a fresh panel instead of falling back. Hiding is + likewise irrelevant — `panel_hidden` is not consulted by placement. +- **`quit_window`'s `QuitAction::Restore { dedicated: true }` is + UNREACHABLE**, and this was the surprise. `Restore` is stored only on + a *replacing* side placement, and a dedicated slot can never be the + target of one. Guarded anyway, labelled defensive, because its + unreachability is emergent from two rules in another function. +- **What this does not rule out:** the enumeration is closed over the + current tree, not future edits. `params.dedicated` is a public field, + so nothing but the acceptance rows would catch a new direct writer. + +**Also closed:** an invalid-UTF-8 profile (`string.char(255)`) reached +`to_str()` and surfaced mlua's generic conversion error instead of the +documented message naming the accepted values — the same reachability +class as revision 5's `Option` defect, one layer down. The +comparison is on bytes now. + +**Written with the lane's first commit**, per the standing correction +from #171 and #215. + +**Branch `destination-capture`**, base `githubsucks/main` @ `4bc55e8` +(the #225 merge). **`githubsucks/destination-capture` is the +authoritative tip** — the ref, not a SHA. Recover with +`git fetch githubsucks && git checkout destination-capture`. + +- **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`; `469d5c8` is the + revision-8 panel-profile correction plus the invalid-UTF-8 hole; + `394fa43` is revision 9's contract stack and the commit below adds its + cross-frontend pin. **15 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 **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 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 as well as per profile, and that is a + deliberate exception with its own positive pin.** A nested commit for + a different frontend may dedicate *its* side slot: `resolve_placement` + consults only the requesting frontend's `panel_capable` and its own + one side window, so nothing done to B can change where A's side + request lands. Pinned by + `a_nested_commit_for_another_frontend_may_dedicate_its_own_slot`, + which is the file's only row asserting that something is **allowed** + — every other asserts a refusal, and an exception only the doc + comment knows about is one review round from being simplified out. + - **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 + measurement stays true for the life of the body, which is what makes + it a guarantee rather than a forecast. + - The four document checks live once, in + `EditorCore::document_destination_refusal`. + - **Three deliberate limits**, each a different decision rather than a + stricter version of this one: the **document profile is untouched** + (constraining its body would newly refuse dired's own documented + panel path — a preservation-suite stop signal); **dedicating a + document window is still allowed** (it cannot change which of + panel-or-document a side request resolves to); and **falling back is + still allowed** — a frontend that cannot render a panel degrades + gracefully exactly as today, because this refuses the mutation that + *manufactures* a fallback, never the fallback itself. +- **Mutation-checked per guard, and the pattern is the evidence the rows + are independent rather than one assertion repeated.** Deleting the + `display_buffer` guard fails the three `display{side, dedicated}` rows + — verified **individually**, by rotating each to the front of the + table, since the first failure otherwise masks the rest. Deleting the + `set_params` guard fails only that row and leaves the display rows + passing. Both leave every other test in the file green. +- **Audit: nothing else relied on "a panel never touches a document".** + Four doc sites repeated the claim (`ViewDestination`'s own doc twice, + `capture_view_destination`, `ViewDestinationLua`) and were corrected; + no other code depended on it. Dired — the only Lua `commit_to` + consumer — takes the **two-argument document profile**, so all four + checks already applied to it, and it separately documents and accepts + the side-slot fallback (`builtin/runtime/dired.lua`). + `compile.lua`'s `already_in_panel` queries live state rather than + assuming, and the terminal adopter's rollback keys off + `DisplayOutcome::created_side`, already false on a fallback. +- **TWO FRAMING CLAIMS THE TREE DID NOT MATCH.** Neither changed a + decision; both are recorded because the framing says "counted, not + estimated" and a reader will check. + 1. **The rename was 11 references across 5 files, not 8 across 4.** + `src/daemon.rs:1804` also calls the capture (the attaching + frontend's directory open), and `editor.rs` holds six references + rather than the counted total. Mechanical either way. + 2. **Q#DC-4's "a frontend with no document window" is a DEFENSIVE + branch, not a routine one.** The obvious spelling — a frontend + showing only a bottom panel — is asserted impossible: Q#BP6 says a + layout always retains at least one non-side window, and + `EditorCore::non_side_target` carries a `debug_assert!` that fires + under `cargo test` when one does. So with Q#BP6 held a *registered* + frontend always has a live document window. The decision still + stands (capture stays total; an adopter with nowhere to land gets a + refusal naming that rather than permission to fall back to ambient + state), and the two Q#DC-4 pins drive the reachable spelling of the + same condition — a layout whose document window has gone while the + view remains. **#227 should not expect to hit this refusal**; it is + insurance, not a path. +- **Mutation-tested, since a matrix of deliberate omissions is exactly + what passes vacuously.** Retyping the profile to `Option` + fails the table and boolean rows with mlua's conversion error (the + number row survives — Lua coerces it — which is why the closed set is + witnessed by more than one non-string). Applying all four checks in + both profiles fails the panel column; applying only check 1 in both + fails the document column. Defaulting an omitted profile to `"panel"` + fails **`journey_acceptance`'s two preservation pins**, which is the + contract claim being executable rather than asserted. Dropping the + frontend scope for the panel profile fails the survives-a-switch pin's + panel row; dropping the no-document-window arm fails the Q#DC-4 pair. + + **Revision 8's four, each isolating a different way to get it wrong** — + and the pattern of *which* rows survive each is the evidence the parts + are independent rather than redundant: + 1. delete the `panel_commit_dedication_refusal` call from + `display_buffer` → the three `display{side, dedicated}` rows fail, + **verified individually** by rotating each to the front of the + table so the first failure cannot mask the rest. Every other test + passes — which is exactly the hole an implementation guarding only + `set_params` would ship. + 2. delete it from `set_params` → **only** that row fails; the three + display rows still pass. + 3. delete the `panel_placement_can_fall_back` arm from + `commit_destination_refusal` → **only** the two pre-established + fallback rows fail, which is the preflight half. + 4. make `panel_placement_can_fall_back` unconditionally `true` (the + "widen the predicate" non-fix) → the really-lands-in-the-panel pin, + the Q#DC-4 panel pin and the matrix's three panel rows all fail. + That is the two profiles collapsing into one, made visible — the + named fallback design, showing up as a test diff rather than + silently. + + And reverting the byte comparison to `to_str()?` fails the + `invalid utf-8` row with mlua's conversion error, on content. + + **Revision 9's two, each isolating a different half of the rule:** + 1. 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. + Note the ordinary-nesting pin deliberately survives this — it + exists to fail the *other* candidate fix (prohibit nesting), so the + two are a pair rather than one test written twice. + 2. delete `&& contract.destination.frontend == fid` from the same + scan, making any outer `"panel"` contract **globally** restrictive + → **only** + `a_nested_commit_for_another_frontend_may_dedicate_its_own_slot` + fails. Both single-frontend nesting tests pass under it, which is + the evidence they are independent of the frontend match rather than + merely looking so; the cross-frontend exception had no pin at all + before this row, since every other test in the file drives one + frontend. + + Both were run across all three acceptance suites and the lib: in each + case `journey_acceptance` (47), `dired_acceptance` (31) and + `cargo test --lib` (1920) stay green, along with every other pin in + this file. + + **The counts above are journey 47 / dired 31**, matching the bullet + further up. The mutation paragraph committed at `394fa43` had them + **reversed** in both the ledger and that commit's message; the ledger + is corrected here and the message is left as written, since rewriting + a pushed commit is worse than a footnote. A reader following that SHA + should take these numbers, not those. +- **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 + optional trailing argument typed **`mlua::Value`, not + `Option`** — with `Option` mlua rejects a number or + table during argument *conversion*, before the closure runs, making + the promised "accepted values are…" message unreachable. That is the + same trap the existing binding documents for `dest`. Validated in the + body against a **closed** set — `"document"` and + `"panel"`. **Omitted means `"document"`**, so every existing + two-argument caller keeps all four preflight checks *by definition of + the signature*, which is what makes `journey_acceptance` passing + untouched a consequence rather than a hope. An unrecognized or + non-string profile **errors**, naming the accepted values — a silent + fallback would hand a caller different checks than it asked for, + which is the exact failure the parameterization exists to prevent. + Git's mapping is settled here too: `*git-status*` → panel, + `*git-diff*` → document. Revision 2 took three findings: Q#DC-2's parameterization was + incomplete (a panel depends on **none** of checks 2–4, not just check + 3, so the question now carries a full preflight matrix with every + omission testable); `tests/journey_acceptance.rs` joins dired as a + **preservation suite and stop signal**, since it holds the + `commit_to` scope, forged-userdata, preflight and restoration pins + this lane generalizes; and the **coherence-impact section was missing + entirely**, which `CLAUDE.md` and `COHERENCE.md` §25 both require. +- **A PREREQUISITE LANE. PR #227 (git Stage 1) blocks on it.** #227's + P1a review finding is why it exists: git's async completions mutate + and display UI without capturing the initiating frontend + (`builtin/runtime/git.lua:609`, `:854`), so a result surfaces in + whichever frontend is active when git exits. +- **The mechanism existed but was not Lua-reachable** until `779bb02`. + `pmacs.window.commit_to` took a `DirectoryDestinationLua`, which is + **nonconstructible from Lua** by design + (`src/lua_bindings/mod.rs:4256`) and minted only inside the + `path.open-directory` listener dispatch (`src/editor.rs:1311`) from a + `pub(crate)` capture (`:1241`). So no async Lua continuation outside + a directory open could say where its result belongs. Line numbers are + the pre-lane ones, kept because they are what the finding was written + against. +- **Scope:** a Lua-reachable capture, a generic rename + (`DirectoryDestination` → `ViewDestination`; the framing counted 8 + references across 4 files, the tree held **11 across 5** — see the + finding above), and the preflight question below. + **No adopter**: git's adoption is #227's work after this lands, since + a prerequisite that converts its own first consumer cannot be + reviewed separately from it. +- **The substantive question (Q#DC-2)** is that git's two continuations + differ in kind. `*git-status*` goes to the **bottom panel** + (`listview.open` defaults `display` to `"panel"`, + `builtin/runtime/listview.lua:550`); `*git-diff*` replaces a + **document** window. `commit_to`'s stale-intent check (Q#JR14c) is + right for the second and, *when the placement really is a panel*, + irrelevant to the first. One shape over-refuses the panel or + under-checks the document. + + **DO NOT READ THE OLDER FORM OF THIS BULLET, WHICH SAID "the panel + never touches the captured window's buffer".** That is the claim + revisions 6–8 invalidate: panel placement **falls back** to an + ordinary document window when the frontend is not panel-capable or + its side slot is dedicated. The relaxation is conditional, and the + mutations that could make it fall back are refused inside a + panel-profile commit (revision 8) rather than predicted at preflight + (revision 6) or caught at placement (revision 7, which would refuse + after the callback had already mutated). +- **Stop signal recorded in the framing:** if any existing dired test + needs editing, the generalization changed Journey Stage 1a's + semantics, and that is cause to stop rather than to adjust the test. +- **Gates, as the executable line rather than a description:** + + ``` + scripts/gate --acceptance destination_capture_acceptance \ + --acceptance journey_acceptance \ + --acceptance dired_acceptance + ``` + + `--acceptance` is repeatable, so there is no reason for this ledger + to say "plus dired's" and leave the reader to reconstruct it. + **`journey_acceptance` and `dired_acceptance` are preservation suites + and a STOP SIGNAL**: they carry the `commit_to` scope, + forged-userdata, preflight and restoration pins this lane + generalizes, and if either needs editing, the change altered Journey + Stage 1a's semantics rather than closing a gap in them. No + `--protocol` — core and Lua bindings only. + +## Worker identity Stage 1 (§9) — MERGED as #232 (`3cc1b85`) **Written with the lane's first commit**, per the standing correction from #171 and #215. @@ -1001,9 +1396,6 @@ authoritative tip** — the ref, not a SHA. Recover with emission, an aborting runner, the build folded into `sweep-crdt`, and — added in the second round — a **rename of either** the build or the sweep step each fail the suite. -||||||| parent of 72bbb96 (docs: LSP LaTeX coverage framing revision 2, on a branch at last) -||||||| parent of 312ec7a (docs: frame Discovery Stage 2 (revision 2) — M-x rows) -||||||| parent of 8f86908 (docs: frame worker identity Stage 1 (revision 1)) ## QoL arc retirement — PR #224 OPEN (docs only) diff --git a/docs/ci-red-signatures.md b/docs/ci-red-signatures.md index 8ef65c7..414be2f 100644 --- a/docs/ci-red-signatures.md +++ b/docs/ci-red-signatures.md @@ -693,18 +693,40 @@ not caused by the PRs they appeared on — that PR is **docs-only and its tree is byte-identical to a green `main`**. It is not evidence that any of them is harmless. -### U4 — `a_pty_resize_blanks_the_host_before_repainting`, macOS `lua54`, one occurrence +### U4 — `a_pty_resize_blanks_the_host_before_repainting`, macOS **both flavours**, three occurrences -Surfaced on PR #229's CI. +Surfaced on PR #229's CI; twice more on PR #231's. + +**The `lua54` in this row's original title was wrong as a signature +component, and matching on it would have missed two occurrences.** The +row was filed from #229's single `lua54` red and recorded the flavour in +the matching key. #231 then reddened the identical selector with the +identical three fragments **twice on `luajit`** — so flavour is not part +of this signature, and the row's own caution that "a deterministic +defect *can* be Lua-flavour-specific" is now settled in the other +direction: this one is not. Occurrence-keyed by suffix length, the three +are `25 362` (#229, `lua54`), `25 222` (#231 attempt 1, `luajit`) and +`25 054` (#231 attempt 2, `luajit`). + +**A fourth sighting of these fragments was NOT an occurrence and must +not be counted as one.** It came from a deliberate bite during this +test's own development — the defect reintroduced on purpose (`consumer +ignores full_grid`), 34 831 bytes, failing in 20.09 s. It earns its +place here for what it proves instead: **the genuine defect and these +CI reds are signature-indistinguishable**, same message class and same +full-timeout duration, so the fragments alone can never tell a real +resync failure from whatever this is. | field | value | |---|---| | **selector** | `--test full_grid_resync_acceptance a_pty_resize_blanks_the_host_before_repainting` | -| **job / flavor** | GitHub Actions, `Test (macos-latest / lua54)`, `macos-26-arm64` | +| **job / flavor** | GitHub Actions, `Test (macos-latest / lua54)` **and** `Test (macos-latest / luajit)`, `macos-26-arm64`. **Flavour is not a matching key for this row** | | **required fragments** | `FG-INV: the post-resize resync must blank the host` · `no CSI 2 J appeared in the` · `bytes emitted after the first painted frame` | | **NOT fragments** | the byte count and the `:LINE` suffix are **occurrence-specific** and must not be matched on — the count is the collected suffix length, which varies per run, and the line moves with the file | -| **status** | **one occurrence; INTERMITTENT — passed on rerun** | -| **why the diff is excluded** | #229 changes only `scripts/gate`, `tests/gate_script_acceptance.rs` and documentation — **no `src/`, and the workflow never invokes `scripts/gate`**. Decisively, `full_grid_resync_acceptance` runs **before** the changed gate suite, so even a cross-suite leaked-state path is not available. The `luajit` leg passing on the same commit is **corroboration only** — a deterministic defect *can* be Lua-flavour-specific, so that observation must not be used as a structural exclusion | +| **status** | **three occurrences on two branches; INTERMITTENT on #229 (passed on rerun), NOT observed to pass on #231 (0/2)** | +| **the #231 control experiment, and what it does and does not license** | Five valid observations at #231's exact base `0190102` — `run_attempt` 1, 2, 3, 4 and 6 — **all green on both macOS flavours**, against #231's 0/2. Under an equal-rate model the chance both failures land on the two branch runs is 1/C(7,2) = **4.8%**. Two things bound that number. First, **attempt 5 was discarded** because it reddened a *different* selector (U8) — so the base leg is 5/5 green *for this signature* and 5/6 overall, and "the base never fails" is not what was observed. Second, three unrelated macOS selectors reddening in one session is **a background platform failure rate**, and the equal-rate model the 4.8% assumes is exactly what such a rate violates. **The branch side was never resampled**: 5-vs-2 is an asymmetric experiment, and rerunning #231's failing job three more times at `4654b94` was the outstanding discriminator when it merged | +| **why #231's diff is excluded** | grepping its **entire** `src/` diff for `full_grid\|resize\|resync\|Geometry\|reconcile_panel_layout` matches **one import line** and nothing else; all 721 changed lines are placement, dedication and commit-contract logic. From the other side, `full_grid_resync_acceptance` (191 lines) contains no panel, side-window, dedication, display or directory surface — grep for those matches only a comment about CSI 2 J. #231 merged on this reading **over** the statistical signal above, which is a judgement recorded here so that a fourth occurrence can revisit it rather than re-derive it | +| **why #229's diff is excluded** | #229 changes only `scripts/gate`, `tests/gate_script_acceptance.rs` and documentation — **no `src/`, and the workflow never invokes `scripts/gate`**. Decisively, `full_grid_resync_acceptance` runs **before** the changed gate suite, so even a cross-suite leaked-state path is not available. The `luajit` leg passing on the same commit is **corroboration only** — a deterministic defect *can* be Lua-flavour-specific, so that observation must not be used as a structural exclusion | | **what IS established** | **no blank was OBSERVED after the mark** within the test's fixed 20-second deadline. The collected suffix was the **entire** post-mark output (`suffix.len()`, 25 362 bytes on this occurrence — not a capped window; only the *displayed* head is truncated to 400 bytes), and that head shows ordinary repaint traffic (`ZQXMARKERQZ` rows with SGR + CUP), so the host was painting | | **what is NOT** | any mechanism. Whether the blank was never emitted, emitted after the deadline, or lost in transport is **open** — and "it never emitted the blank" is a claim this evidence does not support. **The failing run's ~20 s duration is the fixed `Duration::from_secs(20)` timeout**, so the spread against a fast passing run is mechanically determined and is **not** independent timing evidence | | **discriminating control — ASYMMETRIC, and only one direction concludes** | the suffix is already complete, so "capture more bytes" is not the gap — arrival time is. Extending the deadline and recording whether `CLEAR_ALL` arrives, and at what offset: **if it arrives, "emitted late" is established.** **If it does not, that establishes only "not observed by the longer deadline"** — *not* "never emitted", because transport loss produces the same absence. Separating non-emission from transport loss needs **producer-side emission evidence** (did pmacs write the clear?) cross-checked against the collected stream; no deadline, however long, can do it alone | @@ -727,3 +749,53 @@ incident, not U4 occurring twice**. | **exclusion strength — WEAKER than U4's, deliberately** | the changed `gate_script_acceptance` ran **earlier in the same job**, and it creates worktrees and directories. No leaked child or persistent signal-state mutation was observed, but "the diff touches no `src/`" is **not** the argument here that it is for U4, because cross-suite leaked state is a path reachability reasoning does not close | | **control 1 — CROSS-SUITE ATTRIBUTION, and asymmetric** | run `m5_8_acceptance` alone on macOS `lua54`, without the gate suite ahead of it. **A matching isolated RED proves the gate suite is not necessary** for the failure. **An isolated GREEN proves nothing beyond that run** — the failure is intermittent, so absence under one run is not evidence of dependence. It also does **not** discriminate among the three mechanisms in either direction | | **control 2 — mechanism** | observe **readiness and raw-mode state at the moment of injection**. Another isolated pass, however many times repeated, cannot separate "injected before raw mode" from "raw mode lost" from a third cause | + +### U8 — `acc28_child_input_and_the_c_c_escape_work_unchanged_in_a_panel`, macOS `luajit`, one occurrence, **fragments destroyed** + +**Numbered U8 deliberately: U6 and U7 are reserved** for the two +wall-clock rows on `worker-identity-stage1` (PR #232), which renumbered +into that range when #229 took U4/U5. Taking U6 here would recreate the +duplicate-id collision that rebase already produced once. + +**This row exists mostly as an admission.** It surfaced on attempt 5 of +a merge-base control at `0190102`, and **I reran the job before reading +its log**, which discarded it. GitHub keeps only the latest attempt's +logs for a rerun job. So this is U2's original condition exactly — a +selector with no fragments, unmatchable — and it was produced by the +very mistake U3 is named for. + +| field | value | +|---|---| +| **selector** | `--test bottom_panel_stage1_acceptance acc28_child_input_and_the_c_c_escape_work_unchanged_in_a_panel` | +| **job / flavor** | GitHub Actions, `Test (macos-latest / luajit)`, at base `0190102`, control attempt 5 | +| **required fragments** | **NONE CAPTURED — destroyed by rerunning the job before reading its log.** Recovery attempted via the jobs API and the attempt-scoped jobs endpoint; the log is gone | +| **what IS established** | it failed once (`46 passed; 1 failed`), panicking at `tests/bottom_panel_stage1_acceptance.rs:2454`, on the **exact merge base** — so it is not attributable to any open branch | +| **what is NOT** | everything else. Without the assertion text this cannot be matched against a future occurrence, which is the whole purpose of a row here | +| **why it matters anyway** | it is the **third distinct macOS selector** to red in one session, after U4 (`full_grid_resync`) and U5 (`ctrl_c_during_reconnect`). Three unrelated selectors failing on the macOS legs suggests a **background failure rate on that platform** rather than three independent test bugs — and that materially affects any equal-rate reasoning about which branch a failure "landed on" | +| **next occurrence** | **read the log BEFORE rerunning anything.** That is U3's stated lesson and this row is its fourth violation | + +### U9 — a PTY test and a budget test red **together** in one `11-sweep`, with an in-run control + +Recorded on the `destination-capture` merge tree, 2026-08-10, in the +gate run that was meant to clear PR #231. + +**This row's value is its control, not its selectors.** U6 and U7 could +only compare a red run against a *different* run. Here both selectors +ran green **inside the same gate invocation**, minutes earlier, on the +same tree and machine — `03-lib` (1928 passed, 0 failed) and +`04-lib-crdt` (2113 passed, 0 failed) — and then failed in `11-sweep`. +Whatever this is, it is not the tree. + +| field | value | +|---|---| +| **selector** | `--lib process::tests::m6_1_pty_canonical_mode_keeps_kernel_echo` **and** `editor::tests::composition_overhead_under_ten_percent`, failing in the same `11-sweep` step | +| **job / flavor** | local (Linux), `scripts/gate` step `11-sweep` (`cargo test --workspace --no-fail-fast -- --skip basedpyright`), fresh per-lane target dir, no sibling worktrees building | +| **required fragments** | ``canonical mode should leave echo enabled (no `-echo` flag); stty -a output was: ""`` **and** `composition machinery added more than 10% overhead` | +| **NOT fragments** | the measured numbers (`1.613`, `single=191935 ns`, `dispatch=309602 ns`) and every `:LINE` suffix — occurrence-specific | +| **status** | **one occurrence; INTERMITTENT — the identical sweep command on the same tree was green (118 targets, 1928 passed, exit 0)** | +| **what IS established** | intermittence, with the strongest available exclusion of the tree: green in two earlier steps of the **same run**, green isolated afterwards (`2 passed`, 1.70 s), green on a full sweep rerun. Both assertions are **timing-sensitive by construction** — one reads collected child output within a deadline, the other measures wall-clock composition overhead (observed 1.613× against a 1.10× budget; 61.3% dispatch and 124.6% realistic overhead) | +| **what is NOT** | cause, and the load confound is **partially measured but NOT controlled**. The failing sweep ran inside a full gate; the green rerun started at load average 1.98 with the 5-minute figure still at 8.03 from that gate. Different conditions is not a measurement of the mechanism, and this row does not treat it as one | +| **the structural difference worth testing next** | `cargo test --workspace` runs **many test binaries concurrently**; `--lib` runs **one**. That is a difference in kind between the passing steps and the failing one, not merely a difference in load average — and it is the first candidate this family has had that is checkable rather than atmospheric. **Discriminating control:** rerun the sweep with test-binary concurrency pinned to 1, and separately run the `--lib` binary alone under synthetic load. A red under synthetic load at low sweep concurrency implicates load; a red at high concurrency and low load implicates the concurrency itself | +| **relation to U2 — a NEAR MISS, do not match it there** | the PTY fragment is U2's exact family (`stty -a output was: ""`), but U2's selector field names only `m6_1_pty_raw_mode_disables_kernel_echo`. U2's occurrence 2 saw raw **and** canonical fail together; here **canonical redded alone and raw passed**, which U2's evidence has never shown. It is recorded here rather than folded into U2 so that the "canonical alone" case stays visible | +| **relation to U6 — its own instruction, honoured** | `composition_overhead_under_ten_percent` is one of U6's two selectors, and U6 says plainly: "If a future run reds **one** of these without the other, that is a different incident and should be judged as one." It redded without `criterion_1_end_of_line_typing…`, in a different step, at a far larger margin (1.613× here against U6's 1.297×). Judged as a different incident, as instructed | +| **what this row does NOT assert** | that the two selectors share a mechanism. They failed together once; they belong to different subsystems; and U7 already refused this exact merge for U6. The **co-failure inside one step with an in-run green control** is the signature — not either name, and not a shared cause | diff --git a/docs/destination-capture-framing.md b/docs/destination-capture-framing.md new file mode 100644 index 0000000..530da05 --- /dev/null +++ b/docs/destination-capture-framing.md @@ -0,0 +1,816 @@ +# A destination capture any async continuation can use + +**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 that can dedicate the slot 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 — +which **breaks the invariant `commit_to` exists for**: +`docs/agent-handoff.md:748` says it preflights *before* the callback +because "validating at display time is four mutations too late", so a +placement-time refusal arrives after arbitrary Lua has created buffers, +handles and paint. Revision 8 keeps the preflight and **refuses the +mutations that would invalidate it**, the same shape as the existing +await refusal. Refusal stays mutation-free on the `(false, reason)` +path. + +**Revision 6 fixes an UNSOUND matrix, not a preference.** Q#DC-2 gave +the panel profile only check 1, on the claim that a panel result never +touches a document window. **Panel placement falls back to an ordinary +document window** when the frontend is not panel-capable or its side +slot is dedicated — so a `"panel"` commit could replace a *newer* +document while skipping every stale-intent guard. Reproduced in review. +The relaxation is now conditional on the placement really being a +panel. Revision 6 also closes an invalid-UTF-8 hole in the profile +diagnostic — the same reachability class as revision 5's, one layer +down. + +**Revision 5 fixes a binding-level contradiction in revision 4's own +API spec.** It required `profile: Option` *and* a pointed error +naming the accepted values for a non-string — but mlua rejects a +number or table during argument conversion, before the closure runs, so +that message was unreachable. This is the exact trap the existing +binding documents for `dest`, in a comment revision 4 quoted while +repeating the mistake one argument to the right. The profile is now +`mlua::Value`, validated in the body, with `nil` and absence both +meaning `"document"`. + +**Revision 4 specifies the call shape the last two revisions kept +referring to without defining.** "The profile is declared at +`commit_to`" named no signature, no value set, no invalid-profile +behaviour, and nothing about the existing two-argument callers — so +#227 had no stable API to adopt and the Journey preservation promise +rested on care rather than contract. Q#DC-5 fixes that: +`commit_to(dest, body [, profile])`, a **closed** two-value set, +**omitted means `"document"`** so every existing call keeps all four +preflight checks by definition, and an unrecognized profile **errors** +rather than falling back. + +**Revision 3 decides Q#DC-4, which revision 2 left contradicting +Q#DC-2 — on the primary panel API.** Q#DC-2 concluded a panel needs +only a live frontend; Q#DC-4 still returned `nil` without a document +window and told git to fall back to ambient behaviour, which is the +very bug this lane removes. Resolved: the destination's document pair +is **optional**, `capture_destination()` is **profile-blind and +argument-free**, the profile is declared at `commit_to`, and a +document-profile commit without a document pair is refused. §4 and +Q#DC-1 were updated to match rather than left to disagree. + +**Revision 2 takes three review findings.** Q#DC-2's parameterization +was **incomplete** — a panel result does not depend on the captured +document window being live or non-dedicated either, not just on its +buffer, so the question now carries a full **preflight matrix** with +every omission testable. `tests/journey_acceptance.rs` joins dired as a +named **preservation suite and stop signal**; it carries the +`commit_to` scope, forged-userdata, preflight and restoration pins this +lane generalizes, and Journey Stage 1a's framing treats it as a +required gate. And **§5 (coherence impact) was missing entirely**, +which `CLAUDE.md` and `COHERENCE.md` §25 both require of +coherence-affecting work — this lane adds Lua API surface and +generalizes a Journey substrate, so it qualifies twice over. + +**A prerequisite lane. PR #227 (git Stage 1) blocks on it**, and its +P1a review finding is the reason this exists. + +--- + +## 1. Why, and why as its own lane + +PR #227's review found that git's async completions mutate and display +UI without capturing the initiating frontend +(`builtin/runtime/git.lua:609`, `:854`), so a result can surface in +whichever frontend happens to be active when git exits. Run +`git.status` in frontend A, let frontend B become active, and A's panel +opens in B. + +**The finding named the right mechanism.** `pmacs.window.commit_to` +exists for exactly this continuation boundary: Journey Stage 1a's +Q#JR14 built it because "the listing settles a tick or more later, and +by then the ambient frontend, selected window, and active buffer may +all name something else" (`src/editor.rs:1238-1240`). + +**But it is not reachable from Lua outside one path**, which is why +this is a lane and not a line in #227: + +- `commit_to` takes a `DirectoryDestinationLua`, **nonconstructible + from Lua** by deliberate design (`src/lua_bindings/mod.rs:4256`) — + userdata with no constructor and no setters, so a caller cannot + fabricate a plausible triple. +- The only site that mints one is inside the `path.open-directory` + listener dispatch (`src/editor.rs:1311`), from + `capture_directory_destination`, which is `pub(crate)` + (`src/editor.rs:1241`). + +So any async Lua continuation that is **not** a directory open has no +way to say where its result belongs. Git is the first to need it; it +will not be the last. + +Landing this inside #227 would put new Lua API surface, over another +lane's merged mechanism, inside a feature branch — the same folding +that was declined for the `scripts/gate` repair, for the same reason. + +## 2. Ground truth + +- **The captured data is already generic.** + `DirectoryDestination { frontend, window, buffer }` + (`src/editor_core.rs:159-166`) contains nothing directory-specific. + Only its **name** and its **capture site** are. +- **The blast radius of a rename is small**: 8 references across 4 + files (`editor_core.rs`, `editor.rs`, `lua_bindings/mod.rs`, + `lua_bindings/window_panel.rs`). Checked, not estimated. +- **`commit_to`'s preflight is four checks** + (`src/lua_bindings/window_panel.rs:488-525`), in order: the + requesting frontend still has a layout; the destination window is + still live in it; **the window still shows the captured buffer** + (Q#JR14c stale intent); and the window is not dedicated (Q#JR14f). +- **`Handle:await` refuses inside a commit scope** + (`builtin/runtime/async.lua:87-90`) — yielding would restore the + scope while the coroutine is still parked. Any adopter awaits + *before* committing, as dired does. +- **Git's two continuations do not have the same shape**, and this is + the finding that shapes the design: + - `*git-status*` goes through `listview.open`, which resolves + `display` with a **`"panel"`** default + (`builtin/runtime/listview.lua:550`). It **requests** the bottom + panel rather than a document window — *requests*, because a side + request FALLS BACK into a document window on a frontend that is not + `panel_capable` or whose one slot is dedicated elsewhere. That + fallback is this lane's blocker; §3 and Q#DC-2 carry it. + - `*git-diff*` calls `pmacs.window.display(buf, { select = true })` + — the **document** target, deliberately, "so the status panel it + was invoked from stays visible beside it" + (`builtin/runtime/git.lua:852-854`). + +## 3. The tension this lane has to resolve + +`DirectoryDestination.buffer` exists for one purpose, stated at its +definition: *"what that window held at capture time, so **stale intent +loses to the user**"* — a user who replaced the buffer while work was +in flight is newer information than the request. + +**That predicate is right for a document replacement and wrong for a +panel — WHILE THE PANEL REALLY IS A PANEL, which is the qualification +the rest of this document exists to add.** A git status panel that +lands in the bottom panel does not replace the captured window's +buffer; it opens beside it. Refusing to show it because the user +switched files in the document window would be a refusal with no +relationship to what the continuation actually does, and that case +would inherit a check about a window it never touches. + +**Read the previous paragraph with its condition attached, not as a +standing fact.** Panel placement **falls back** into an ordinary +document window when the frontend is not `panel_capable` or its one +side slot is dedicated elsewhere — and then the panel case *does* touch +the captured window, replacing whatever the user put there. That +fallback is this lane's correctness blocker, and the unqualified +version of this claim is precisely what made revision 5's matrix +unsound. The resolution is below, at the end of Q#DC-2: the preflight +measures whether this frontend places side requests in the panel, and +the mutations that would falsify that measurement mid-commit are +refused. + +Meanwhile the diff case *is* a document replacement, and wants exactly +the dired semantics. + +So a single one-size destination either **over-refuses** the panel case +or **under-checks** the document case. Q#DC-2 is where that gets +decided, and it is the substance of this lane. + +## 4. The change, in outline + +- **A Lua-reachable capture**, returning the same nonconstructible + userdata for the *current* frontend, **with** its document window and + buffer when it has one and without them when it does not (Q#DC-4). + The capture takes no arguments and is profile-blind; the profile is + declared at `commit_to`. +- **Generic naming.** `DirectoryDestination` becomes something that + does not lie about a git panel; `capture_directory_destination` and + the userdata type follow. 8 references (§2). +- **The directory path keeps behaving exactly as it does today** — this + lane generalizes the capture, it does not change Journey Stage 1a's + semantics. +- **No adopter in this lane.** Git's adoption is #227's, after this + lands. A prerequisite that also converts its first consumer makes the + two impossible to review separately. + +## 5. Coherence impact (§20) + +**Revision 1 omitted this section entirely, and it is required.** +`CLAUDE.md` and `COHERENCE.md` §25 both say a framing for +coherence-affecting work must cite the section it serves and state its +impact — and this lane adds **new Lua API surface** and generalizes a +Journey-substrate mechanism, which is coherence-affecting on both +counts. Recording the impacts as neutral where they are neutral is part +of the requirement, not a way around it. + +- **§16 semantic frontend — the section this serves.** The defect it + removes is a continuation resolving its target from *ambient* state a + tick after the request, which is precisely the multi-frontend + correctness §16 exists to protect. A capture makes "which frontend + asked" a value rather than a guess. +- **§14 workbench primitives — indirect, and the honest framing is + *enabling*.** This does not add a primitive. It removes the reason an + async adopter would hand-roll frontend tracking, which is the + mechanism by which primitives acquire per-consumer idiosyncrasies. +- **Journey steps touched: none directly, one PROTECTED.** The golden + journey does not gain a step. But Journey Stage 1a's Q#JR14 substrate + is what this generalizes, and §7 makes `tests/journey_acceptance.rs` + a preservation suite precisely so a generalization cannot erode the + step it came from. +- **Interaction islands (§6): none added.** No key interception, no + dispatch precedence rung. `dispatch_key` is untouched. +- **Config registry: no setting.** Where a continuation lands is a + correctness property, not a preference, and a toggle would offer to + turn correctness off. +- **Background-work attribution (§9): NEUTRAL, and worth stating + precisely rather than skipping.** This lane adds no background work + and no new unattributable surface. It also does **not** improve §9 — + knowing which frontend a result belongs to is not knowing who asked + for it or why. That is the worker-identity lane's arc, and the two + should not be confused because both concern async continuations. +- **§10 extension trust — a small positive.** The capture keeps the + Q#JR14d property that a destination is **nonconstructible from Lua**, + so generalizing the mechanism does not widen what extension code can + fabricate. §7 re-asserts the forged-destination refusal after the + rename for exactly this reason. + +## 6. Open questions + +### Q#DC-1 — what does the capture take as arguments? + +*My vote: **no arguments** — capture the acting frontend and its +document window from the ambient state at call time.* That is what the +existing `capture_directory_destination(frontend, window)` is handed by +its one caller, and a Lua-supplied frontend id would reintroduce the +fabrication hole the userdata design closes. + +### Q#DC-2 — one destination shape, or a panel/document distinction? **(the substantive one)** + +§3 is the problem. Three candidates: + +1. **One shape, all four checks.** Simplest; over-refuses the panel + case, and the refusal reason would be about a window the panel does + not touch. +2. **One shape, preflight parameterized by the continuation** — the + caller declares whether it is replacing the captured window's + buffer, and the stale-intent check applies only then. +3. **Two capture kinds**, document and panel, with different preflights. + +*My vote: **(2)***, with the profiles spelled out below rather than +left to implementation. + +**Revision 1 said only "skip the stale-buffer check for a non-replacing +continuation", and that was incomplete.** Review is right: a panel +result does not depend on the captured **document window** at all. It +does not replace that window's buffer, so check 3 is irrelevant; it +does not occupy that window, so check 4 (dedicated) is irrelevant; and +it does not need that specific window to exist, so check 2 is +irrelevant. Retaining any of the three can reject `git.status` for a +document-window change that has nothing to do with where the panel +goes. But dropping them **without an explicit profile** is how document +replacement quietly loses its guarantees. + +**The matrix, stated so every omission is deliberate and testable:** + +| # | Precondition (`window_panel.rs:488-525`) | Document replacement | Frontend/panel scope | +|---|---|---|---| +| 1 | Requesting frontend still has a layout | **required** | **required** | +| 2 | Destination window still live in it | **required** | not applicable | +| 3 | Window still shows the captured buffer (Q#JR14c stale intent) | **required** | not applicable | +| 4 | Window is not dedicated (Q#JR14f) | **required** | not applicable | + +**Check 1 is the entire panel profile ONLY WHEN THE PLACEMENT REALLY IS +A PANEL — revision 5's matrix was unsound, and this is the correction.** + +The matrix rested on "the panel never touches the captured window's +buffer". **That is false when panel placement falls back.** +`editor_core.rs:4138-4148` says so in its own comment: *"Reaching +`Ordinary` while a side was REQUESTED means the request fell back (not +panel-capable, or the one slot is dedicated elsewhere)"* — and the +result is then installed into an ordinary **document** window. So a +`"panel"` commit on a non-panel-capable frontend replaces a document +view while skipping every check that exists to stop it replacing a +*newer* one. That reintroduces exactly the stale-intent failure the +API was built to prevent, which makes it a correctness defect and not +a strictness preference. + +**The rule, restated:** the panel profile's relaxation is conditional +on the placement actually being a panel. Whenever placement **can** +fall back to a document window, the panel profile runs the **full +document preflight**. + +**PREFLIGHT STAYS WHERE IT IS; THE MUTATION THAT WOULD INVALIDATE IT IS +REFUSED. Revisions 6 and 7 were both wrong, in opposite directions.** + +Revision 6 predicted the fallback at preflight and argued the body +could not change it. **False**: the await refusal stops *concurrent +interleaving*, not the body, which is arbitrary synchronous Lua and can +dedicate the side slot itself. + +Revision 7 then moved enforcement to the placement boundary. **That +breaks the invariant `commit_to` exists for.** `docs/agent-handoff.md` +`docs/agent-handoff.md:748` states it without qualification: + +> [`commit_to`] preflights every precondition *before* invoking the +> callback — dired mutates handle state, `prev`, and paint long before +> it reaches anything that could refuse, so **validating at display +> time is four mutations too late**. + +Refusing at placement means refusing *after* arbitrary callback code has +created buffers, handles and paint. A late refusal is not a refusal; it +is a partial commit with an error return. + +**So neither predict nor refuse late — forbid the mutation.** Inside a +panel-profile commit, the operations that could change the placement +outcome are **refused**, exactly as `Handle:await` is refused inside a +commit scope and for the identical reason: something that would +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:** + +- `dedicated` **is** writable from Lua — and it is one of only two + writable window fields (`window_panel.rs:888`, *"Only `fixed_rows` + and `dedicated` are writable (Q#BP2c)"*). +- `panel_capable` has **no Lua binding at all** — checked across + `src/lua_bindings/`. A body cannot make a frontend panel-incapable. + +**FOUR WRITES REACH DEDICATION, AND A FIFTH IS GUARDED DEFENSIVELY.** +Review found the second *after* the first was specified, which is the +evidence that guarding one named call site is not a design — and the +enumeration below, performed against the tree rather than by recall, +found three more: two further `apply_placement` arms, plus +`quit_window`'s `QuitAction::Restore`, which step 6 proves *unreachable* +and which is guarded anyway. So **four are reachable, a fifth is guarded +defensively, and all five are guarded** — the last is the count the +safety argument actually runs on. (Historical note, not the current +count: earlier revisions of this section counted all five as +*reachable*. The table below has always said four; the ledger was +corrected in `fb3974b` and this section with it.) The two review named +first are: + +1. **`set_params`** — the writable-field path (`window_panel.rs:888`). +2. **`display(buf, { side = …, dedicated = true })`** — writes + `request.dedicated` straight into the side window + (`editor_core.rs:4535`). A body can take this route, then request a + second panel buffer and cause the fallback. **An implementation + guarding only route 1 passes revision 8's test while keeping the + original defect.** + +**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 that can dedicate the slot 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 guarded sites — the four reachable ones and the +defensive fifth — 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 +only two pieces of state are levers at all: `FrontendView::panel_capable`, +and the one side window's `Window::params.dedicated`. Everything else a +body can touch is irrelevant by construction, which is what makes the +enumeration finite instead of "every mutation in the editor". + +*Step 2 — `panel_capable` is unreachable, not merely unguarded.* It is +written **only** where a `FrontendView` is constructed, and no +`FrontendView` is constructed, registered or unregistered anywhere in +`src/lua_bindings/` — `register_frontend_view` and +`unregister_frontend_view` have callers only in `daemon.rs` (attach and +detach) and in core unit tests. A body cannot reach it. + +*Step 3 — every write to `dedicated`, from `rg 'params\.dedicated\s*=' +src/`, classified.* Eight sites, no exceptions: + +| # | site | verdict | +|---|---|---| +| 1 | `apply_placement`, `Side` **created** | reachable — `display{side, dedicated}` with no panel yet | +| 2 | `apply_placement`, `Side` **replacing** | reachable — `display{side, dedicated}`, different buffer | +| 3 | `apply_placement`, `Side` **non-replacing** | reachable — `display{side, dedicated}`, same buffer | +| 4 | `apply_placement`, `Ordinary` (`!fell_back`) | harmless — every `Ordinary` target is filtered `!is_side`, so it is never the slot | +| 5 | `apply_placement`, `Ordinary` (clear) | harmless — only ever writes `false` | +| 6 | `set_params` | reachable — the direct write (Q#BP2c) | +| 7 | `quit_window`, `QuitAction::Restore` | **unreachable** — guarded anyway, defensively; see below | +| 8 | an `EditorCore` unit test | not Lua-reachable | + +*Step 4 — the guards, sited where the property converges rather than at +each caller.* Sites 1, 2, 3 (and 4, 5) are all reached through +`apply_placement`, which has **exactly one caller**, `display_buffer`. +So one guard there covers every request-driven dedication, including +routes that do not exist yet. `set_params` is a genuinely separate write +and is guarded separately — dedication does *not* converge before the +field itself, and that is stated rather than papered over. Two live +guards over the four reachable sites; site 7 carries a third guard, +defensive because the site is unreachable (step 6), so **all five are +guarded**. + +*Step 5 — what was looked for and found NOT to be a route.* Closing the +side window is **not** one: with no side leaf `side_window_for` returns +`None` and `resolve_placement` **creates** a fresh panel rather than +falling back, so quitting or hiding the panel mid-commit is safe, and +`panel_hidden` is not consulted by placement at all. `params.side` is +likewise unreachable — `set_params` refuses it and only +`apply_placement`'s created branch writes it, so a body cannot promote +an already-dedicated document window into the slot. + +*Step 6 — site 7 is unreachable, and this is the one finding that +surprised.* `QuitAction::Restore` carries the outgoing `dedicated` flag, +so quitting the panel looked like a route with no `dedicated` argument +at the call site at all. It cannot be constructed: `Restore` is only +ever *stored* on a **replacing** side placement, and a dedicated slot +can never be the target of one — a side request with a different buffer +falls through to `Ordinary`, and an exact-target request is refused by +`window_accepts_buffer`. So `Restore { dedicated: true }` has no +producer. It is guarded anyway, defensively and labelled as such, +because its unreachability is an emergent property of two rules in a +different function. + +**What this does NOT rule out.** The enumeration is closed over the +current tree, not over future edits: relaxing `resolve_placement`'s +dedicated arm, or adding a binding that writes `params.dedicated` +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 +preferred answer only because it makes the parameterization pointless. +Choosing it is a design decision needing its own approval, not a +silent retreat. **It was not needed.** + +**What is NOT the fix: refusing a panel commit that would fall back.** +Falling back to an ordinary window is existing, deliberate behaviour +for a frontend without panel capability; refusing would turn a +graceful degradation into an error and regress consumers that work +today. The panel profile relaxes checks; it does not get to change +where things land. + +**Consequence for the capture, which follows and should not be +discovered later:** if the panel profile needs only the frontend, then +a frontend with **no document window** can still host a panel — so +Q#DC-4's "return `nil`" is right for the document profile and possibly +wrong for the panel one. That interaction is settled as part of +answering this, not after it. + +**I hold the *choice* loosely, not the matrix.** (1) has a real +argument — a uniform rule is easier to reason about, and over-refusal +is safe — but it would refuse the git panel for reasons unrelated to +it, and "safe" refusals that users cannot explain are how a mechanism +gets worked around. If review prefers (1) or (3), the matrix above is +what changes, and **every cell marked "not applicable" must still be +tested as deliberately omitted** (§7) so a future reader cannot mistake +an omission for an oversight. + +### Q#DC-3 — what is the type called? + +*My vote: **`ViewDestination`***, with `pmacs.window.capture_destination()` +as the Lua entry point. It names what it is — a place in a view where a +continuation's result belongs — without claiming a directory or a +buffer kind. + +The Q#JR14 doc comments should keep their references intact; a rename +that orphans the rationale is worse than a slightly stale name. + +### Q#DC-5 — the exact Lua call shape for the profile **(new in rev 4)** + +Revisions 2 and 3 said "the profile is declared at `commit_to`" and +never said **how**. That is not a detail: today's binding accepts +exactly `(dest, body)` (`window_panel.rs:453-456`), so without a +specified form #227 has no stable API to adopt against, and the +promise that existing callers keep their semantics is a hope rather +than a contract. + +**The signature:** + +```lua +pmacs.window.commit_to(dest, body) -- document profile +pmacs.window.commit_to(dest, body, "panel") -- panel profile +``` + +- **`profile` is an OPTIONAL THIRD argument, typed `mlua::Value` at + the binding — NOT `Option`.** + + **Revision 4 said `Option` and that contradicted its own + error requirement.** mlua rejects a number or table *during argument + conversion*, before the closure body runs, so the promised message + naming `"document"` and `"panel"` would be **unreachable** — a caller + passing `42` would get mlua's generic conversion error instead. This + is the identical trap the existing binding already documented for + `dest`, in a comment revision 4 cited while making the same mistake + one argument to the right: + + > Typed as `Value` rather than `AnyUserData` so this message is + > REACHABLE: with the narrower type mlua rejects a table during + > argument conversion, and a caller who fabricated one got "error + > converting Lua table to userdata" — true, but it names neither the + > rule nor how to get a real destination. + + So: accept `Value`, and validate in the body. + - **`Nil` or absent → `"document"`.** Both spellings, since + `commit_to(dest, body, nil)` is what a Lua caller threading an + optional variable produces, and it must not be a third behaviour. + - **`String` → must be `"document"` or `"panel"`**, else refused, + naming both accepted values. + - **Anything else → refused by the SAME message**, which now names + the accepted values *and* says a string was expected. That message + only exists if the type is `Value`. +- No arity sniffing and no table-or-function dispatch on argument 2 — + a polymorphic second argument would put the *destination*'s error + message back at risk, which is what that comment was protecting. +- **Trailing, and readable in practice.** A profile after a long inline + closure would read badly, but that is not the call shape in use: + dired defines `local function commit() … end` and calls + `commit_to(opts.dest, commit)` (`builtin/runtime/dired.lua:670,717`). + Against a named body, `commit_to(dest, commit, "panel")` reads fine. +- **The value set is CLOSED: `"document"` and `"panel"`.** Exactly the + two profiles in Q#DC-2's matrix. Not an open string namespace — a + third profile is a decision, not a spelling. +- **Omitted means `"document"`.** This is the load-bearing part: every + existing `commit_to(dest, fn)` call keeps **all four** preflight + checks, unchanged, by definition of the signature. `journey_acceptance` + passing untouched (§7) then follows from the API shape rather than + from care. +- **An unrecognized profile is an ERROR**, naming the accepted values — + **not** a silent fall back to `"document"`. A fallback would hand a + caller stricter or looser checks than it asked for, which is the + failure mode the whole parameterization exists to prevent. A + non-string profile errors the same way. + +**Which profile each of git's continuations takes**, so #227's adoption +is decided here rather than rediscovered: `*git-status*` → **panel** +(it lands in the bottom panel, `listview.lua:550`); `*git-diff*` → +**document** (it replaces a document window deliberately, +`git.lua:852-854`). + +### Q#DC-4 — what happens when there is no document window? **(DECIDED in rev 3)** + +**Revision 2 left this contradicting Q#DC-2 and it is the primary panel +API, so it is decided here rather than voted on.** Q#DC-2 concluded a +panel profile depends only on a live frontend — so it can commit with +no document window at all — while this question still said the capture +returns `nil` in exactly that case, and told git to fall back to +ambient behaviour. Those cannot both hold, and the fallback advice was +independently wrong: falling back to ambient **is** the P1a bug this +lane exists to remove. + +**The decision:** + +- **`ViewDestination { frontend, window: Option, buffer: + Option }`.** The frontend is always present; the document + pair is optional and absent exactly when the frontend has no document + window. +- **`capture_destination()` is NOT profile-aware and takes no + arguments.** It records what is there. Making capture profile-aware + would force the caller to know at *capture* time what it will do at + *commit* time, which is the opposite of why capture exists — the + whole point is to freeze the truth early and decide later. +- **The profile is declared at `commit_to`**, which is where Q#DC-2's + parameterization already lives. One place makes the decision, and it + is the place that knows. **Its exact call shape is Q#DC-5**, which + revisions 2 and 3 left unspecified. +- **A document-profile commit on a destination with no document pair is + REFUSED**, with a reason naming that, joining the four preflight + refusals rather than being a separate failure mode. +- **Capture therefore never returns `nil`** while a frontend exists, + and the "adopter degrades to ambient" advice is **withdrawn**. An + adopter with nowhere to land gets a refusal it can report; it does + not get permission to guess. + +**What this changes elsewhere, so the decision does not sit alone:** +§4's outline says the capture returns userdata "for the *current* +frontend and its document window" — it returns one for the current +frontend, **with** its document window when there is one. Q#DC-1's "no +arguments" answer is unchanged and now load-bearing rather than +incidental: no arguments is what keeps capture profile-blind. + +## 7. Verification + +- **A captured destination survives a frontend switch**: capture in A, + make B active, commit, and assert the result lands in **A**. This is + P1a's actual failure and the reason the lane exists — asserting only + that the API returns userdata would pass on a capture that does + nothing. +- **A fabricated destination is still refused** — the existing Q#JR14d + guarantee, re-asserted after the rename so the generalization cannot + quietly open the hole it was built to close. +- **Every preflight refusal is witnessed by its own case, in BOTH + profiles** (Q#DC-2's matrix): frontend gone, window gone, stale + buffer, dedicated window — each asserted to **refuse** under the + document profile, and each of the three marked "not applicable" + asserted to **NOT refuse** under the panel profile. A deliberately + omitted check that has no test is indistinguishable from a check + someone forgot, and the next reader will restore it. +- **A legacy two-argument `commit_to(dest, body)` gets the DOCUMENT + profile** (Q#DC-5), witnessed by a check the panel profile omits — + a stale-buffer refusal. Asserting merely that it does not error would + pass on a call silently downgraded to the panel profile, which is the + regression that would quietly void Journey Stage 1a's guarantees. +- **A `"panel"` commit that FALLS BACK to a document window is checked + against the document preconditions**, witnessed for **both** causes + separately — a non-panel-capable frontend, and a dedicated side slot. + Each asserts the stale-intent refusal fires: capture A, make B newer, + commit `"panel"`, observe the refusal rather than B being replaced. +- **A BODY THAT TRIES TO CREATE THE FALLBACK IS REFUSED AT THE ATTEMPT**, + in its own test: the callback dedicates the side slot **mid-commit**. + Three assertions, and the second and third are the ones that matter: + the dedication call itself is **refused**; the side slot is **still + undedicated afterwards**; and no partial result was installed. + **One row per reachable WRITE SITE** (§3), which is four and not two: + `set_params`, and `display{side, dedicated}` in each of + `apply_placement`'s **created**, **replacing** and **non-replacing** + arms. A single row against one route is what would let another keep + the defect — and rows per *call spelling* would have missed that one + spelling reaches three different writes. The + two bullets above cannot catch this — both establish their fallback + state *before* `commit_to` is entered, so a preflight-snapshot design + passes them. + + **Asserting only "document B was not replaced" is insufficient**, and + revision 7's version of this test made exactly that mistake: it + 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. +- **THE CROSS-FRONTEND EXCEPTION IS PINNED POSITIVELY**, over **two** + frontends: while an outer `"panel"` commit for A is in force, a nested + commit for **B** dedicates **B's** side slot and is **allowed** — and + B's slot is asserted really dedicated afterwards, not merely + unrefused. The far side runs in the same test: A's slot is still + undedicated and A's result still lands in A's panel, so this cannot + pass by having weakened the restriction generally. **This is the one + row asserting that something is permitted**; every other in the suite + asserts a refusal, and without it, deleting the `fid` comparison — + making any outer panel contract *globally* restrictive — passes the + whole file, because both nesting rows above drive a single frontend. + The exception is real and not a convenience: `resolve_placement` + consults only the requesting frontend's `panel_capable` and its own + one side window, so nothing done to B can change where A's side + request lands. +- **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. +- **An unrecognized profile string is REFUSED**, with a message naming + the accepted values — not silently treated as `"document"`. +- **An invalid-UTF-8 profile is refused by that SAME message.** Lua + strings are byte strings, so a `string.char(255)` profile reaches + `to_str()` and produces mlua's generic conversion error *before* + the documented message is ever constructed — the same reachability + class as the `Option` defect, one layer deeper. Compare + bytes, or map the conversion failure onto the message; asserted on + content, in the bad-profile matrix beside the number and table rows. +- **A non-string profile (a number, a table) is refused by that SAME + message**, asserted **on its content**, not merely that an error + occurred. This is the bullet that fails if the argument is ever + retyped to `Option`: mlua would reject the value during + conversion and the assertion on the message would stop matching. The + test is therefore the guard on the type choice, not just on the + behaviour. +- **An explicit `nil` profile takes the document profile**, identical + to omitting it — witnessed separately, because a Lua caller threading + an optional variable produces `nil` rather than absence, and a third + behaviour there would be invisible until someone hit it. +- **Capture SUCCEEDS with no document window** (Q#DC-4), returning a + destination whose document pair is absent — asserted as a successful + capture, not as `nil`. +- **A panel-profile commit on that destination SUCCEEDS**, and a + **document-profile commit on it is REFUSED** with a reason naming the + missing document window. Both halves, because asserting only the + refusal would pass on a capture that refuses everything. +- **The directory path is unchanged** — dired's existing acceptance + coverage passes untouched. +- **`tests/journey_acceptance.rs` passes UNCHANGED**, as a named + preservation suite. It carries the established contract this lane + generalizes — 27 `commit_to` references across nine named pins + including `commit_to_refuses_a_forged_destination`, + `commit_to_scopes_and_restores_on_a_normal_return`, + `commit_to_restores_when_the_callback_raises`, + `commit_to_refuses_an_await_and_restores`, + `commit_to_delivers_to_the_requesting_frontend_not_the_ambient_one`, + `a_declining_listener_cannot_redirect_the_destination`, and two + rows already named `preservation_*`. Journey Stage 1a's own framing + treats this suite as a required gate; a lane that generalizes its + substrate does not get to relax that. +- **STOP SIGNAL, for both suites.** If any existing `dired` or + `journey_acceptance` test needs editing, the generalization changed + Journey Stage 1a's semantics. That is cause to stop and report, not + to adjust the test — a suite edited to accommodate the change under + test has stopped being evidence. +- **`Handle:await` still refuses inside the scope**, including through + `pmacs.async.yield_to_next_tick` if the worker-identity lane's Q#W-7 + has landed by then; if it has not, this lane does **not** add that + guard — it belongs to that lane and duplicating it would produce a + conflict for no benefit. + +**What this will NOT prove:** that git surfaces in the right frontend — +that is #227's adoption, after this lands. This lane ships the +mechanism and one set of tests for the mechanism. + +## 8. Not in scope + +**Adopting the capture anywhere**, including git (#227 does that) and +including migrating other async continuations that have the same latent +bug — worth an audit, not this lane's work. Changing Journey Stage 1a's +directory semantics. The `commit_to` scope guard for +`yield_to_next_tick` (worker identity Q#W-7). Any protocol change — +this is entirely core + Lua bindings. Panel geometry or placement +policy, which is the bottom-panel arc's. diff --git a/src/daemon.rs b/src/daemon.rs index e5c5a94..121b469 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -1827,7 +1827,7 @@ fn open_initial_target( let (buffer_id, fire) = match resolved { crate::editor_core::ResolvedTarget::Directory { path } => { let dest = editor - .capture_directory_destination(frontend_id, origin_window) + .capture_view_destination(frontend_id, origin_window) .ok_or_else(|| format!("cannot open {}: no document window", path.display()))?; editor.dispatch_directory_open(&path, dest); editor.reconcile_panel_layout(frontend_id); diff --git a/src/editor.rs b/src/editor.rs index 0e78fe8..63f617d 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -25,7 +25,7 @@ use unicode_width::UnicodeWidthStr; use crate::async_runtime::SharedAsyncRuntime; use crate::cell::{CellCoord, CellSize}; -use crate::editor_core::{EditorCore, GeometryUpdate}; +use crate::editor_core::{CommitContract, EditorCore, GeometryUpdate}; use crate::frontend::{Event, Frontend, KeyEvent, KeyEventKind, MouseEvent, install_panic_hook}; use crate::key::{Chord, display_sequence}; use crate::keymap_stack::{Action, KeyDispatcher}; @@ -119,20 +119,34 @@ impl ScopedFrontend { } /// Enter a background frontend scope, also swapping the core's - /// ambient `active_frontend`. Both are restored on drop, on every - /// exit path including a raising callback. + /// 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, commit_scope: &CommitScopeActive, - frontend_id: FrontendId, + contract: CommitContract, ) -> ScopedFrontendGuard { + let frontend_id = contract.destination.frontend; let previous = self.0.replace(Some(frontend_id)); - let previous_active = { + let (previous_active, contract_depth) = { let mut core = core.borrow_mut(); let was = core.active_frontend; core.active_frontend = frontend_id; - was + (was, core.push_commit_contract(contract)) }; let previous_commit = commit_scope.0.replace(true); ScopedFrontendGuard { @@ -140,6 +154,7 @@ impl ScopedFrontend { core: core.clone(), previous, previous_active, + contract_depth, commit_scope: commit_scope.clone(), previous_commit, } @@ -151,6 +166,15 @@ pub(crate) struct ScopedFrontendGuard { core: SharedCore, previous: Option, previous_active: FrontendId, + /// 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, @@ -160,7 +184,11 @@ pub(crate) struct ScopedFrontendGuard { impl Drop for ScopedFrontendGuard { fn drop(&mut self) { self.scope.0.set(self.previous); - self.core.borrow_mut().active_frontend = self.previous_active; + { + let mut core = self.core.borrow_mut(); + core.active_frontend = self.previous_active; + core.exit_commit_contract(self.contract_depth); + } self.commit_scope.0.set(self.previous_commit); } } @@ -1219,22 +1247,32 @@ impl EditorState { } /// Capture the destination a directory open must commit to - /// (Q#JR14), or `None` when `frontend` has no document window. + /// (Q#JR14), or `None` when `window` is gone. /// /// Synchronous by necessity: the listing settles a tick or more /// later, and by then the ambient frontend, selected window, and /// active buffer may all name something else. - pub(crate) fn capture_directory_destination( + /// + /// Takes the window **explicitly**, unlike + /// [`crate::editor_core::EditorCore::capture_view_destination`], + /// which reads the ambient one. Both directory callers already hold + /// the exact window the open was resolved against — the daemon's is + /// read before `resolve_target_buffer` runs (Q#BP11b) — and + /// recapturing it from ambient state here would discard that. + /// A directory open therefore always yields a full document pair, + /// which is why this keeps returning `Option` rather than the total + /// capture's `ViewDestination`. + pub(crate) fn capture_view_destination( &self, frontend: crate::protocol::FrontendId, window: crate::window::WindowId, - ) -> Option { + ) -> Option { let core = self.core.borrow(); let buffer = core.windows.get(&window)?.buffer_id; - Some(crate::editor_core::DirectoryDestination { + Some(crate::editor_core::ViewDestination { frontend, - window, - buffer, + window: Some(window), + buffer: Some(buffer), }) } @@ -1258,7 +1296,7 @@ impl EditorState { .borrow() .primary_document_window(crate::protocol::FrontendId::LOCAL); let dest = window.and_then(|window| { - self.capture_directory_destination(crate::protocol::FrontendId::LOCAL, window) + self.capture_view_destination(crate::protocol::FrontendId::LOCAL, window) }); let Some(dest) = dest else { self.core.borrow_mut().status = @@ -1288,13 +1326,13 @@ impl EditorState { pub(crate) fn dispatch_directory_open( &mut self, path: &std::path::Path, - dest: crate::editor_core::DirectoryDestination, + dest: crate::editor_core::ViewDestination, ) { let display = path.display().to_string(); let args = { let lua = self.lua_host.lua(); let destination = - match lua.create_userdata(crate::lua_bindings::DirectoryDestinationLua(dest)) { + match lua.create_userdata(crate::lua_bindings::ViewDestinationLua(dest)) { Ok(userdata) => mlua::Value::UserData(userdata), Err(error) => { self.core.borrow_mut().status = format!("cannot open {display}: {error}"); diff --git a/src/editor_core.rs b/src/editor_core.rs index 0976756..517989c 100644 --- a/src/editor_core.rs +++ b/src/editor_core.rs @@ -130,39 +130,119 @@ pub enum ResolvedTarget { }, } -/// Where a directory open was requested, captured **synchronously** at -/// resolve time (Journey Stage 1a, Q#JR14). +/// Where an asynchronous continuation's result belongs, captured +/// **synchronously** at request time (Journey Stage 1a, Q#JR14; +/// generalized by `docs/destination-capture-framing.md`). /// -/// The listing that satisfies a directory open is asynchronous -/// (`pmacs.fs.read_dir` is worker-dispatched and must be awaited), so the -/// code that finally builds and displays the listing runs a tick or more -/// later — outside interactive dispatch, where `pmacs.window.*` acts on -/// the *ambient* frontend by documented design (`builtin/runtime/dired.lua`). -/// Without a captured destination, a second frontend dispatching in the -/// meantime silently redirects the listing. +/// The work that satisfies such a request is asynchronous (a directory +/// listing is worker-dispatched and must be awaited; so is a `git` +/// invocation), so the code that finally builds and displays the result +/// runs a tick or more later — outside interactive dispatch, where +/// `pmacs.window.*` acts on the *ambient* frontend by documented design +/// (`builtin/runtime/dired.lua`). Without a captured destination, a +/// second frontend dispatching in the meantime silently redirects the +/// result. /// -/// All three fields are load-bearing: +/// The fields are load-bearing, and the document pair is **optional** +/// (Q#DC-4) because a panel result needs only a live frontend *when it +/// really lands in a panel*, so a frontend whose document window has +/// gone can still host one: /// -/// * `frontend` — the scope the commit must run in. +/// * `frontend` — the scope the commit must run in. Always present. /// * `window` — the exact destination; the ambient selected window is -/// not it. +/// not it. Absent when the frontend had no document window at capture +/// time. /// * `buffer` — what that window held at capture time, so **stale /// intent loses to the user** (Q#JR14c). A user who replaced the -/// buffer while the listing was in flight is newer information than -/// the launch argument, and must not be overwritten. +/// buffer while the work was in flight is newer information than the +/// launch argument, and must not be overwritten. Present exactly when +/// `window` is. +/// +/// The pair is set or cleared together — see +/// [`EditorCore::capture_view_destination`], which is the only place +/// that reads them off ambient state. +/// +/// Which of those a commit actually requires is the **profile**, chosen +/// at `pmacs.window.commit_to` rather than at capture (Q#DC-2/Q#DC-5): +/// 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, 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. /// /// Exposed to Lua only as nonconstructible userdata (Q#JR14d): as a /// table, the *same* value is handed to every resolver listener in turn, /// so one could mutate it and then decline — redirecting later listeners /// — and any Lua could fabricate a plausible triple. #[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub struct DirectoryDestination { - /// Frontend that requested the directory. +pub struct ViewDestination { + /// Frontend that requested the work. pub frontend: FrontendId, - /// Window the listing must land in. - pub window: WindowId, + /// Window the result must land in, when there is one. + pub window: Option, /// Buffer that window held at capture time (stale-intent check). - pub buffer: BufferId, + pub buffer: Option, +} + +/// Which of `commit_to`'s preconditions a body actually depends on +/// (Q#DC-2). +/// +/// A **closed** set of two, not an open string namespace: a third +/// profile is a decision about what a continuation may depend on, not a +/// spelling. Chosen at `commit_to` rather than at capture, because the +/// caller knows what it is about to do only then. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum CommitProfile { + /// The body replaces the captured window's buffer: **all four** + /// preflight checks apply. This is what an omitted profile means, so + /// every caller written before the profile existed keeps exactly the + /// guarantees it was written against. + 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**. 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 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 +/// could fall back into a document window, on the argument that nothing +/// could change in between because the body cannot `await`. Refusing +/// `await` stops another coroutine interleaving; it says nothing about +/// 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. 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`). +/// +/// 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. + pub destination: ViewDestination, + /// What that continuation declared it depends on. + pub profile: CommitProfile, } /// A `display_buffer` request (Q#BP3). @@ -620,6 +700,34 @@ pub struct EditorCore { /// slot; the producer clears any untaken record when the fan-out /// returns. typed_edit_armed: Option<(FrontendId, TypedEditRecord)>, + /// Every `commit_to` contract currently on the stack, outermost + /// first (Q#DC-2, revision 9). + /// + /// **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 { @@ -674,9 +782,42 @@ impl EditorCore { query_replace: None, typed_edit_pending: None, typed_edit_armed: None, + commit_contracts: Vec::new(), } } + /// 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 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. /// Replaces the scratch buffer's content; the active window is /// retained. @@ -3042,6 +3183,143 @@ impl EditorCore { self.non_side_target(fid).ok() } + /// Capture where `fid`'s next asynchronous result belongs (Q#JR14, + /// generalized by Q#DC-1/Q#DC-4). + /// + /// **Profile-blind and total**: it records what is there rather than + /// what a caller intends to do later, and it never fails while a + /// 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 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. + /// + /// The document pair is set or cleared **together**: a window whose + /// entry has gone yields neither half, so no consumer has to handle + /// a window without its captured buffer. + /// + /// **How reachable the empty pair is, stated because the framing + /// implies more than the tree does.** Q#BP6 says a frontend layout + /// always retains at least one non-side window, and + /// [`Self::non_side_target`] carries a `debug_assert!` that fires + /// when one does not — so with that invariant held, a *registered* + /// frontend always has a live document window and this branch is + /// **defensive** rather than routine. It stays because the + /// alternative is a capture that can fail, and a caller that can + /// fail is a caller that falls back to ambient state. + #[must_use] + pub fn capture_view_destination(&self, fid: FrontendId) -> ViewDestination { + let pair = self + .primary_document_window(fid) + .and_then(|window| Some((window, self.windows.get(&window)?.buffer_id))); + ViewDestination { + frontend: fid, + window: pair.map(|(window, _)| window), + buffer: pair.map(|(_, buffer)| buffer), + } + } + + /// The document profile's preconditions on a captured destination — + /// Q#DC-2's checks 2, 3 and 4, plus Q#DC-4's missing-pair case. + /// + /// **One rule in one place**, because it is now evaluated from two + /// sites and they must not drift: `commit_to`'s preflight runs it + /// before the body, and [`Self::display_buffer`] runs it again when a + /// `"panel"` commit's side request actually falls back into a + /// document window. A second copy of these three checks is how the + /// backstop ends up subtly weaker than the thing it backs. + /// + /// Check 1 (the requesting frontend still has a layout) is + /// deliberately *not* here: it is shared by both profiles rather than + /// specific to the document one, and the placement path cannot fail + /// it — it is placing into that very frontend. + #[must_use] + pub fn document_destination_refusal(&self, dest: &ViewDestination) -> Option { + let Some(window) = dest.window else { + // The capture found no document window (Q#DC-4). A refusal + // rather than a raise, so it joins the others as one more + // thing the destination can fail to satisfy and an adopter + // handles it the same way. + return Some( + "destination has no document window (capture it from a frontend that has \ + one, or commit with the \"panel\" profile)" + .to_string(), + ); + }; + // 2. The destination window is still live in the frontend. + if !self + .views + .get(&dest.frontend) + .is_some_and(|view| view.layout.iter_ids().contains(&window)) + { + return Some(format!("window {} is gone", window.raw())); + } + // 3. Stale intent (Q#JR14c): the user replaced the buffer while + // the work was in flight. Their action is newer information + // than the request, so the request loses. + if self + .windows + .get(&window) + .is_some_and(|w| Some(w.buffer_id) != dest.buffer) + { + return Some(format!("window {} now shows another buffer", window.raw())); + } + // 4. Replaceability (Q#JR14f). `None` because the replacement + // does not exist yet — passing the captured buffer would + // approve a window dedicated to *it*, and the handler's + // different buffer would be refused later, after mutating. + if !self.window_accepts_buffer(window, None) { + return Some(format!("window {} is dedicated", window.raw())); + } + None + } + + /// `commit_to`'s **preflight**: what a commit under `profile` can be + /// refused for before its body runs at all (Q#DC-2). + /// + /// Ordering is the whole point of preflighting rather than validating + /// at display time: an async body mutates real state (claims a + /// buffer, registers a handle, paints) long before it reaches any + /// call that could refuse, so a late refusal leaves debris behind. + /// + /// **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, + dest: &ViewDestination, + profile: CommitProfile, + ) -> Option { + // 1. The requesting frontend still has a layout. Required by + // BOTH profiles, because a frontend that is gone can host + // nothing. + if !self.views.contains_key(&dest.frontend) { + return Some("requesting frontend is gone".to_string()); + } + // 2, 3 and 4 are DELIBERATELY OMITTED for a panel result that + // really lands in a panel, not overlooked (Q#DC-2): it does not + // occupy the captured document window, does not replace its + // buffer, and does not need it to exist, so each would refuse for + // a reason unrelated to what the continuation does. Every one of + // the three is pinned as NOT refusing under this profile. + if profile == CommitProfile::Panel && !self.panel_placement_can_fall_back(dest.frontend) { + return None; + } + self.document_destination_refusal(dest) + } + /// [`Self::primary_document_window`]'s buffer, falling back to the /// focused window's when the layout is degenerate. #[must_use] @@ -3260,6 +3538,23 @@ impl EditorCore { } other => other, }; + // Q#DC-2 (revision 8). A `Restore` carries the OUTGOING + // presentation's `dedicated` flag (see `apply_placement`), so + // quitting the panel can re-dedicate the one slot without any + // `dedicated` argument appearing at the call site. Refused for + // the same reason and at the same point as the other attempts — + // before `quit_window` has touched anything. + if let QuitAction::Restore { + dedicated: true, .. + } = action + && self + .windows + .get(&target) + .is_some_and(crate::window::Window::is_side) + && let Some(reason) = self.panel_commit_dedication_refusal(fid) + { + return Err(format!("window.quit: {reason}")); + } match action { QuitAction::Delete => { // Capture the remembered origin BEFORE the window dies: @@ -3801,6 +4096,20 @@ impl EditorCore { .ok_or_else(|| format!("frontend {fid:?} has no window layout"))? .active; let placement = self.resolve_placement(fid, request)?; + // Q#DC-2 (revision 8): dedicating the side slot inside a + // `"panel"` commit is refused AT THE ATTEMPT, so the preflight's + // measurement cannot go stale. `resolve_placement` is pure, so + // this still refuses before anything is mutated. + // + // Note the guard is on the DEDICATION, not on the display: the + // body's ordinary `display(buf, {side = "bottom"})` is exactly + // what a panel continuation is for and always proceeds. + if request.dedicated == Some(true) + && matches!(placement.kind, PlacementKind::Side { .. }) + && let Some(reason) = self.panel_commit_dedication_refusal(fid) + { + return Err(format!("display: {reason}")); + } self.apply_placement(fid, request, &placement)?; let select = request .select @@ -3926,6 +4235,176 @@ impl EditorCore { .ok_or_else(|| "display_file: no eligible document window is available".into()) } + /// Whether a `{side = ...}` request in `fid` would fall back into an + /// ordinary document window **given the state right now** (Q#DC-2). + /// + /// Adjacent to [`Self::resolve_placement`] because that is the rule + /// it predicts, and a prediction that drifts from the rule is worse + /// than none. The two fallback arms, in that function's own order: + /// + /// 1. **step 2's capability guard** — `side` is honoured only on a + /// `panel_capable` frontend; without the capability the request + /// falls through to step 3's ordinary policy (Q#BP13). + /// 2. **step 2's dedicated arm** — the one side slot exists but is + /// dedicated, and a second one is never created, so a different + /// buffer falls through instead (Q#BP3 2.iii). + /// + /// **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. 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. 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, + /// and at preflight the body has not chosen a buffer yet. + /// + /// A frontend with no view answers `false`: where placement would + /// land is moot when there is nothing to place into, and + /// `commit_destination_refusal` has already refused that case by its + /// first check. + #[must_use] + pub fn panel_placement_can_fall_back(&self, fid: FrontendId) -> bool { + let Some(view) = self.views.get(&fid) else { + return false; + }; + if !view.panel_capable { + return true; + } + self.side_window_for(fid) + .and_then(|side| self.windows.get(&side)) + .is_some_and(|side| side.params.dedicated) + } + + /// **The guarantee** behind the `"panel"` commit profile (Q#DC-2, + /// 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 + /// + /// The panel profile skips preflight checks 2–4 on the strength of "a + /// panel result never touches a document window". Panel placement + /// **falls back** into an ordinary document window when the frontend + /// is not `panel_capable` or its one side slot is dedicated elsewhere + /// ([`Self::apply_placement`] says so in its own comment), and then + /// installs the result there. So a `"panel"` commit that reached a + /// fallback would replace a document view with no stale-intent guard: + /// capture A, the user opens B, the continuation lands, B is gone. + /// + /// # Why this shape, and not the two that were tried first + /// + /// * **Predicting the fallback at preflight is unsound.** The body is + /// arbitrary *synchronous* Lua and can create the condition itself. + /// Refusing `await` inside the commit scope stops a second + /// coroutine interleaving; it places no restriction on the body's + /// own statements. + /// * **Refusing at the placement boundary is too late.** `commit_to` + /// preflights *before* invoking the callback precisely because a + /// body creates buffers, registers handles and paints long before + /// it asks to display anything — "validating at display time is + /// four mutations too late" (`docs/agent-handoff.md`). A refusal + /// arriving after all of that is not a refusal; it is a partial + /// commit with an error return. + /// + /// So the preflight stays where it is and **the mutation that would + /// invalidate it is rejected** — the same shape as `Handle:await` + /// being refused inside a commit scope, for the identical reason. + /// With these refused, the preflight measurement cannot go stale, the + /// 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 + /// [`PlacementKind::Ordinary`] from a side request in two ways, so + /// only two pieces of state matter: + /// + /// 1. `FrontendView::panel_capable` is false. It is written **only** + /// where a `FrontendView` is constructed, and no `FrontendView` is + /// constructed, registered or unregistered anywhere in + /// `src/lua_bindings/` — that is the daemon's attach path. **A + /// body cannot reach it at all.** + /// 2. The frontend's one side slot exists **and is dedicated** to a + /// different buffer. `Window::params.dedicated` is the only + /// remaining lever, and every write to it is guarded or harmless: + /// the two in `apply_placement`'s `Ordinary` arm target a document + /// window (never a side one — every `Ordinary` target is filtered + /// `!is_side`) and one of them only ever clears the flag; the + /// three in its `Side` arm and the one in `pmacs.window.set_params` + /// are the attempts refused here; and `quit_window` restoring a + /// saved `dedicated: true` presentation is refused too. + /// + /// **Losing the side window is NOT a route** and was checked rather + /// than assumed: with no side leaf, `side_window_for` returns `None` + /// and `resolve_placement` **creates** a fresh panel instead of + /// falling back. Closing or hiding the panel mid-commit is therefore + /// safe, and `panel_hidden` is not consulted by placement at all. + /// `params.side` is likewise unreachable — `set_params` refuses it, + /// and only `apply_placement`'s created branch ever writes it, so a + /// body cannot turn an already-dedicated document window into the + /// side slot. + /// + /// # What is deliberately NOT refused + /// + /// * **The document profile is untouched.** Its preflight already + /// checked the same destination, and constraining its body would + /// newly refuse dired's own documented panel path. + /// * **Dedicating a *document* window is fine.** It cannot change + /// which of panel-or-document a side request resolves to. + /// * **Falling back is still allowed.** A frontend that cannot render + /// 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 { + // 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( + "cannot dedicate the side window inside a \"panel\" commit_to --- the commit's \ + preflight was relaxed because this frontend places side requests in the panel, \ + and dedicating the one slot would silently redirect the result into a document \ + window instead (dedicate outside the commit, or use the \"document\" profile)" + .to_string(), + ) + } + /// Q#BP3's precedence: exact target, then side affinity, then /// ordinary reuse. Placement affinity precedes generic reuse — /// otherwise a persistent `*compilation*` buffer already visible in a diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 1a9795a..0a9fa59 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -4239,25 +4239,35 @@ fn install_path_module(lua: &Lua) -> mlua::Result { Ok(path) } -/// Lua handle for a captured directory destination (Q#JR14d). +/// Lua handle for a captured view destination (Q#JR14d). /// -/// Deliberately **nonconstructible from Lua** and read-only. The same -/// value is passed to every `path.open-directory` listener in turn: as a -/// table, an earlier listener could mutate it and then decline, -/// redirecting later listeners or the fallback to a window the user -/// never asked for — and any Lua could fabricate a plausible -/// frontend/window/buffer triple and hand it to `commit_to`. Userdata -/// with no constructor and no setters makes both unrepresentable rather -/// than merely discouraged. +/// Deliberately **nonconstructible from Lua** and read-only, which the +/// generalization to `pmacs.window.capture_destination()` preserves: +/// capture mints one from editor state, and there is still no +/// constructor and no setter. The same value is passed to every +/// `path.open-directory` listener in turn: as a table, an earlier +/// listener could mutate it and then decline, redirecting later +/// listeners or the fallback to a window the user never asked for — and +/// any Lua could fabricate a plausible frontend/window/buffer triple and +/// hand it to `commit_to`. Userdata with no constructor and no setters +/// makes both unrepresentable rather than merely discouraged. /// /// The single accessor exists because dired needs the exact window for /// its `display{window = …}` target; nothing needs the frontend or the /// captured buffer, which stay private to the preflight. -pub(crate) struct DirectoryDestinationLua(pub(crate) crate::editor_core::DirectoryDestination); +/// +/// `window()` returns **nil** when the capturing frontend had no +/// document window (Q#DC-4) — such a destination is still commitable +/// under the panel profile wherever that profile's relaxation actually +/// applies, so the accessor reports the absence rather than inventing an +/// id. +pub(crate) struct ViewDestinationLua(pub(crate) crate::editor_core::ViewDestination); -impl mlua::UserData for DirectoryDestinationLua { +impl mlua::UserData for ViewDestinationLua { fn add_methods>(methods: &mut M) { - methods.add_method("window", |_, this, ()| Ok(this.0.window.raw())); + methods.add_method("window", |_, this, ()| { + Ok(this.0.window.map(crate::window::WindowId::raw)) + }); } } diff --git a/src/lua_bindings/window_panel.rs b/src/lua_bindings/window_panel.rs index fe8b758..01f4d61 100644 --- a/src/lua_bindings/window_panel.rs +++ b/src/lua_bindings/window_panel.rs @@ -34,7 +34,9 @@ use mlua::{Lua, Table, Value}; use super::{BufferIdLua, SharedCore, config_u32, run_hook_if_defined}; -use crate::editor_core::{DisplayOutcome, DisplayRequest, HookKind, QuitOutcome}; +use crate::editor_core::{ + CommitContract, CommitProfile, DisplayOutcome, DisplayRequest, HookKind, QuitOutcome, +}; use crate::protocol::FrontendId; use crate::window::{DEFAULT_PANEL_ROWS, MIN_WINDOW_OUTER_ROWS, Side, WindowId}; @@ -63,6 +65,51 @@ pub(crate) fn acting_frontend(lua: &Lua, core: &SharedCore) -> FrontendId { .unwrap_or_else(|| core.borrow().active_frontend_key()) } +/// One message for every bad profile — an unrecognized string and a +/// non-string alike (Q#DC-5). +/// +/// Stated once so the parser and the message cannot drift, and phrased +/// to name the accepted values *and* the default, because a caller who +/// gets this wrong is guessing at the vocabulary. +const BAD_COMMIT_PROFILE: &str = "pmacs.window.commit_to: profile must be the string \"document\" \ + or \"panel\" (omitting it, or passing nil, means \"document\")"; + +/// Resolve the optional third argument of `commit_to`. +/// +/// Takes a [`Value`] rather than an `Option` **so this refusal +/// is reachable**: with the narrower type mlua rejects a number or a +/// table during argument conversion, before the closure body runs, and +/// the caller gets a generic conversion error that names neither the +/// accepted values nor the default. That is the same trap the `dest` +/// argument documents at its own borrow site. +/// +/// `Nil` and absence are the **same** answer, not two: a Lua caller +/// threading an optional variable produces `commit_to(dest, body, nil)`, +/// and a third behaviour there would stay invisible until someone hit +/// it. +/// +/// The comparison is on **bytes**, for the same reachability reason one +/// layer down. A Lua string is a byte string, not UTF-8, so +/// `commit_to(dest, body, string.char(255))` fails a `to_str()` +/// conversion and surfaces mlua's generic UTF-8 error *before* the +/// message below is ever constructed. An invalid-UTF-8 profile is a bad +/// profile like any other and gets the documented refusal. +fn commit_profile(value: &Value) -> mlua::Result { + match value { + Value::Nil => Ok(CommitProfile::Document), + Value::String(name) => match name.as_bytes().as_ref() { + b"document" => Ok(CommitProfile::Document), + b"panel" => Ok(CommitProfile::Panel), + // An unrecognized profile ERRORS rather than falling back to + // the document one: a fallback would silently hand a caller + // stricter or looser checks than it asked for, which is the + // failure the parameterization exists to prevent. + _ => Err(mlua::Error::runtime(BAD_COMMIT_PROFILE)), + }, + _ => Err(mlua::Error::runtime(BAD_COMMIT_PROFILE)), + } +} + /// Run the panel-reconciliation transaction from a Lua-owning context /// (Q#BP2b). /// @@ -452,7 +499,7 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result "commit_to", lua.create_function( move |lua, - (dest, body): (mlua::Value, mlua::Function)| + (dest, body, profile): (mlua::Value, mlua::Function, mlua::Value)| -> mlua::Result { // Journey Stage 1a (Q#JR14). Preflight FIRST, then // scope, then run. The ordering is the whole point: @@ -472,7 +519,7 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result // rule nor how to get a real destination. let dest = match &dest { mlua::Value::UserData(userdata) => { - userdata.borrow::().ok() + userdata.borrow::().ok() } _ => None, }; @@ -484,45 +531,27 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result ) })? .0; + // Q#DC-5. Resolved AFTER the destination so a caller + // who got both wrong hears about the destination + // first --- it is the argument that cannot be fixed + // by reading this signature. + let profile = commit_profile(&profile)?; - // 1. The requesting frontend still has a layout. - let refusal = { - let core = cc.borrow(); - if !core.views.contains_key(&dest.frontend) { - Some("requesting frontend is gone".to_string()) - } else if !core - .views - .get(&dest.frontend) - .is_some_and(|view| view.layout.iter_ids().contains(&dest.window)) - { - // 2. The destination window is still live in it. - Some(format!("window {} is gone", dest.window.raw())) - } else if core - .windows - .get(&dest.window) - .is_some_and(|w| w.buffer_id != dest.buffer) - { - // 3. Stale intent (Q#JR14c): the user - // replaced the buffer while the work was - // in flight. Their action is newer - // information than the request, so the - // request loses. - Some(format!( - "window {} now shows another buffer", - dest.window.raw() - )) - } else if !core.window_accepts_buffer(dest.window, None) { - // 4. Replaceability (Q#JR14f). `None` - // because the replacement does not exist - // yet — passing the captured buffer would - // approve a window dedicated to *it*, and - // the handler's different buffer would be - // refused later, after mutating. - Some(format!("window {} is dedicated", dest.window.raw())) - } else { - None - } - }; + // The preflight itself lives on the core + // (`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. + // + // 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(); out.push_back(mlua::Value::String(lua.create_string(reason.as_bytes())?)); @@ -546,13 +575,32 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result ) })? .clone(); - // Both the override and the core's ambient - // `active_frontend` are restored when this 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 override, the core's ambient `active_frontend`, + // and the CONTRACT below are all restored when this + // 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 + // 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, &commit, dest.frontend); + let _guard = scope.enter( + &cc, + &commit, + CommitContract { + destination: dest, + profile, + }, + ); body.call::(()) }; let mut out = result?; @@ -563,6 +611,39 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result )?; } + { + // Q#DC-1 — the capture half, reachable from Lua at last. + // + // Journey Stage 1a built `commit_to` for the continuation + // boundary, but the only thing that could mint a destination was + // the `path.open-directory` dispatch, so every other async + // continuation had to resolve its target from ambient state a + // tick after the request --- which is a misrouting waiting for a + // second frontend to become active. + // + // NO ARGUMENTS, and that is load-bearing rather than + // minimalism. A Lua-supplied frontend id would reintroduce + // exactly the fabrication hole the nonconstructible userdata + // closes (Q#JR14d): the point of userdata is that Lua names a + // destination it was *given*, never one it composed. + // + // PROFILE-BLIND, likewise (Q#DC-4). Capture records what is + // there; what a commit depends on is declared at `commit_to`, + // because a caller knows what it is about to do only then. + // Making capture profile-aware would force it to know at capture + // time what it will do at commit time, which is the opposite of + // why capture exists --- freeze the truth early, decide later. + let cc = core.clone(); + win.set( + "capture_destination", + lua.create_function(move |lua, ()| { + let fid = acting_frontend(lua, &cc); + let dest = cc.borrow().capture_view_destination(fid); + lua.create_userdata(super::ViewDestinationLua(dest)) + })?, + )?; + } + { // Q#S3-1 — the shared adopter-display rule, reachable from Lua. // @@ -834,6 +915,24 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result None => None, }; let dedicated = opts.get::>("dedicated")?; + // Q#DC-2 (revision 8). The direct route to the one + // mutation that could make a `"panel"` commit's + // relaxed preflight wrong. Refused BEFORE the borrow + // below, so the attempt changes nothing --- including + // `fixed_rows`, which is in the same option table. + if dedicated == Some(true) { + let core = cc.borrow(); + if core + .windows + .get(&id) + .is_some_and(crate::window::Window::is_side) + && let Some(reason) = core.panel_commit_dedication_refusal(fid) + { + return Err(mlua::Error::runtime(format!( + "pmacs.window.set_params: {reason}" + ))); + } + } { let mut core = cc.borrow_mut(); let window = core.windows.get_mut(&id).ok_or_else(|| { diff --git a/tests/destination_capture_acceptance.rs b/tests/destination_capture_acceptance.rs new file mode 100644 index 0000000..0ae179f --- /dev/null +++ b/tests/destination_capture_acceptance.rs @@ -0,0 +1,1519 @@ +// tests/destination_capture_acceptance.rs --- the Lua-reachable capture. + +//! Acceptance for `docs/destination-capture-framing.md` §7: a +//! destination any asynchronous continuation can capture, and the +//! profile that says which of `commit_to`'s preconditions it depends on +//! (Q#DC-1 … Q#DC-5). +//! +//! **What this suite does NOT prove**, deliberately: that git — or any +//! other adopter — surfaces in the right frontend. This lane ships the +//! mechanism and the tests for the mechanism; adoption is #227's, after +//! it lands (§8). Every test here therefore drives the Lua surface +//! directly rather than through a consumer. +//! +//! Three disciplines it keeps: +//! +//! * **Every "not applicable" cell in Q#DC-2's preflight matrix is +//! asserted as NOT refusing**, not merely left untested. A check +//! deliberately omitted and a check someone forgot look identical from +//! the outside, and the next reader restores the second one. +//! * **A refusal is asserted on its reason**, never on the mere fact +//! that something failed. `commit_to` has five distinct refusals and a +//! raise; "it errored" would pass on any of the wrong ones. +//! * **The panel profile's relaxation is pinned as a preflight PLUS the +//! refusal that keeps it true** (revision 8). The preflight measures +//! whether this frontend places side requests in the panel; the body is +//! arbitrary *synchronous* Lua, so refusing `await` — which only stops +//! another coroutine interleaving — does not stop it invalidating that +//! measurement. The answer is neither to predict the body nor to catch +//! it late at placement (by then it has created buffers, handles and +//! paint, which is "four mutations too late" all over again) but to +//! **refuse the mutation at the attempt**, exactly as `await` is +//! refused. Three tests carry it and none subsumes another: +//! `a_panel_commit_that_falls_back_runs_the_document_preflight` (the +//! body must not run at all when the fallback already holds), +//! `a_body_that_tries_to_create_the_fallback_is_refused_at_the_attempt` +//! (the mutation is refused, and nothing partial is left behind), and +//! `a_panel_commit_that_falls_back_with_a_valid_destination_still_lands` +//! (falling back is still graceful degradation, not an error). +//! +//! `tests/journey_acceptance.rs` and `tests/dired_acceptance.rs` are the +//! preservation half of the same §7 and are run alongside this suite: +//! they hold the Stage 1a contract this lane generalizes, and if either +//! needed editing the generalization changed Journey semantics rather +//! than extending them. + +use pmacs::buffer::BufferId; +use pmacs::editor::EditorState; +use pmacs::protocol::FrontendId; +use pmacs::window::{FrontendView, Layout, Window, WindowId}; +use tempfile::TempDir; + +// --------------------------------------------------------------------------- +// Harness +// --------------------------------------------------------------------------- + +fn exec(s: &EditorState, src: &str) { + s.lua_host.lua().load(src.to_string()).exec().unwrap(); +} + +fn eval(s: &EditorState, src: &str) -> T { + s.lua_host.lua().load(src.to_string()).eval().unwrap() +} + +/// A fresh editor with LSP disabled: no test here asserts anything about +/// a language server, so the wipe cannot make an assertion vacuous. +fn editor() -> EditorState { + let s = EditorState::new_with_roots(&crate::iso::roots()); + exec(&s, "pmacs.lsp.config = {}"); + s +} + +/// A directory with a file worth displaying. +fn project() -> TempDir { + let td = tempfile::tempdir().expect("tempdir"); + std::fs::write(td.path().join("alpha.txt"), b"alpha\n").expect("write alpha"); + td +} + +fn active_name(s: &EditorState) -> String { + eval(s, "return pmacs.window.buffer():name()") +} + +fn buffer_in(s: &EditorState, window: WindowId) -> Option { + s.core.borrow().windows.get(&window).map(|w| w.buffer_id) +} + +/// A window's buffer **by name**, so a placement assertion reads as +/// "`*result*` went to the panel" rather than as two opaque ids. +fn name_in(s: &EditorState, window: WindowId) -> String { + let buffer = buffer_in(s, window).expect("window is live"); + let core = s.core.borrow(); + let registry = core.registry.borrow(); + registry.get(buffer).expect("buffer").name().to_string() +} + +/// Whether `window` is pinned to its buffer (Q#BP2c `dedicated`). +fn dedicated(s: &EditorState, window: WindowId) -> bool { + s.core + .borrow() + .windows + .get(&window) + .is_some_and(|w| w.params.dedicated) +} + +/// Whether a buffer by this name exists at all. +/// +/// The "nothing partial was installed" assertion needs to see a side +/// effect the body would have left *before* reaching any display, and a +/// created-but-never-shown buffer is exactly that. +fn buffer_exists(s: &EditorState, name: &str) -> bool { + eval( + s, + &format!( + "for _, id in ipairs(pmacs.buffer.list()) do + if pmacs.describe.buffer(id).name == {name:?} then return true end + end + return false" + ), + ) +} + +fn local_window(s: &EditorState) -> WindowId { + s.core + .borrow() + .views + .get(&FrontendId::LOCAL) + .expect("LOCAL view") + .active +} + +/// The frontend that competes for ambient authority. +const COMPETITOR: FrontendId = FrontendId(7); + +/// A frontend that has a layout but no live document window (Q#DC-4). +const DOCUMENTLESS: FrontendId = FrontendId(9); + +/// Register a second frontend with its own single-window layout, +/// mirroring `build_fresh_frontend_view` — the same helper shape +/// `journey_acceptance` and `bottom_panel_stage1_acceptance` use. +fn attach_frontend(s: &EditorState, fid: FrontendId) -> WindowId { + let win = WindowId::next(); + let mut core = s.core.borrow_mut(); + let buffer_id = core.active_buffer_id(); + let text_view = { + let reg = core.registry.borrow(); + pmacs::text_view::TextView::new(reg.get(buffer_id).expect("buffer")) + }; + core.windows + .insert(win, Window::new(win, buffer_id, text_view)); + core.register_frontend_view(fid, view_over(win)); + win +} + +/// Register a frontend whose layout names a window that is **not live**, +/// so `primary_document_window` finds nothing to hand back. +/// +/// **Why this shape and not a side-window-only layout.** The obvious +/// reading of "a frontend with no document window" is a frontend showing +/// only a bottom panel — but that state is asserted impossible: Q#BP6 +/// says a layout always retains at least one non-side window, and +/// `EditorCore::non_side_target` carries a `debug_assert!` that fires +/// under `cargo test` if one ever does. So the reachable spelling of the +/// same condition is a layout whose document window has gone while the +/// view remains, which is what this builds. +/// +/// **Recorded honestly, because the framing implies more than the tree +/// does** (`docs/destination-capture-framing.md` Q#DC-4): with Q#BP6 +/// held, a *registered* frontend in a healthy editor always has a live +/// document window, so the absent document pair is a **defensive** +/// branch rather than a routine one. It is still the right decision — +/// capture stays total, and an adopter with nowhere to land gets a +/// refusal naming that rather than permission to fall back to ambient +/// state — and it is still worth pinning, because the alternative to +/// pinning it is a branch nothing ever executes. +fn attach_documentless_frontend(s: &EditorState, fid: FrontendId) { + let mut core = s.core.borrow_mut(); + core.register_frontend_view(fid, view_over(WindowId::next())); +} + +fn view_over(win: WindowId) -> FrontendView { + FrontendView { + layout: Layout::single(win), + active: win, + fold_projection: true, + panel_capable: true, + frame_geometry: None, + panel_hidden: false, + } +} + +/// Capture through the **production** Lua entry point and leave the +/// userdata in the global `dest`. +/// +/// Nothing in this suite can construct one by any other route — that is +/// what `a_forged_destination_is_still_refused` is about — so every test +/// below runs against a destination the editor minted. +fn capture(s: &EditorState) { + exec(s, "dest = pmacs.window.capture_destination()"); + assert!( + eval::(s, "return dest ~= nil"), + "the capture must always yield a destination while a frontend exists" + ); +} + +/// Run a body that also executes `also` under `profile`, reporting +/// `(ok, reason)`. +/// +/// `profile` is spliced as a Lua expression, so a caller can pass +/// `"nil"`, `"'panel'"`, `"42"` — the argument-shape distinctions +/// Q#DC-5 turns on are exactly what this suite has to vary. `also` is +/// spliced as Lua statements, for the rows that must observe *where* an +/// accepted commit put its result and not merely that it was accepted. +fn commit_body(s: &EditorState, profile: Option<&str>, also: &str) { + let call = match profile { + Some(profile) => format!("pmacs.window.commit_to(dest, body, {profile})"), + None => "pmacs.window.commit_to(dest, body)".to_string(), + }; + exec( + s, + &format!( + "ran = false + local body = function() ran = true; {also} end + raised = nil + local caught, a, b = pcall(function() return {call} end) + if caught then ok, reason = a, b + else ok, reason, raised = false, nil, tostring(a) end" + ), + ); +} + +/// Run an inert body under `profile` and report `(ok, reason)`. +fn commit(s: &EditorState, profile: Option<&str>) { + commit_body(s, profile, ""); +} + +fn ok(s: &EditorState) -> bool { + eval(s, "return ok == true") +} + +fn ran(s: &EditorState) -> bool { + eval(s, "return ran") +} + +fn reason(s: &EditorState) -> String { + eval(s, "return tostring(reason)") +} + +/// The message a raise (as opposed to a `(false, reason)` refusal) +/// carried, or `None` if nothing was raised. +fn raised(s: &EditorState) -> Option { + eval::>(s, "return raised") +} + +// --------------------------------------------------------------------------- +// §7 — a captured destination survives a frontend switch +// --------------------------------------------------------------------------- + +/// **N** — the failure the lane exists for: the result lands in the +/// frontend that *asked*, not in whichever one is ambient when the work +/// settles. +/// +/// Asserted for **both** profiles. The panel profile drops three of the +/// four preflight checks, and a plausible way to implement that is to +/// drop the scope with them — which would leave a panel continuation +/// resolving its target from ambient state, the exact P1a defect. So the +/// scope is pinned per profile rather than once. +/// +/// Falsified by making the commit display ambiently: the file then +/// appears in the competitor's window. Asserting merely that +/// `capture_destination()` returns userdata would pass on a capture that +/// does nothing. +#[test] +fn a_captured_destination_survives_a_frontend_switch() { + for profile in [None, Some("'panel'")] { + let td = project(); + let s = editor(); + capture(&s); + + let local_win = local_window(&s); + let other_win = attach_frontend(&s, COMPETITOR); + let other_before = buffer_in(&s, other_win); + + // The competitor becomes the dispatching frontend while the work + // is "in flight" — the state a worker completion returns to. + s.core.borrow_mut().active_frontend = COMPETITOR; + + let alpha = td.path().join("alpha.txt").display().to_string(); + exec( + &s, + &format!( + "committed = pmacs.window.commit_to(dest, function() + pmacs.window.display_file({alpha:?}) + end{})", + profile.map_or(String::new(), |p| format!(", {p}")) + ), + ); + + assert!( + eval::(&s, "return committed"), + "{profile:?}: the commit must be accepted" + ); + assert_eq!( + buffer_in(&s, other_win), + other_before, + "{profile:?}: the competing frontend's window must be untouched" + ); + s.core.borrow_mut().active_frontend = FrontendId::LOCAL; + assert_eq!( + active_name(&s), + alpha, + "{profile:?}: the commit must land in the capturing frontend's window" + ); + assert_eq!( + local_window(&s), + local_win, + "{profile:?}: and in that window, not a new one" + ); + } +} + +// --------------------------------------------------------------------------- +// §7 — the forged destination stays refused +// --------------------------------------------------------------------------- + +/// **P (Q#JR14d)** — generalizing the capture does not widen what +/// extension code can fabricate. +/// +/// A plausible `{frontend, window, buffer}` table is what any Lua could +/// build, and the capture now hands out the *same* userdata type through +/// a public entry point — so the type check is re-asserted after the +/// rename rather than assumed to have survived it. +/// +/// *Mutation:* accept `mlua::Value::Table` in the borrow arm. This +/// fails; nothing in `journey_acceptance` covers the new entry point. +#[test] +fn a_forged_destination_is_still_refused() { + let s = editor(); + capture(&s); + let win = eval::(&s, "return dest:window()"); + + exec( + &s, + &format!( + "ran = false + local caught, err = pcall(pmacs.window.commit_to, + {{ frontend = 0, window = {win}, buffer = 0 }}, + function() ran = true end) + rejected = (not caught) and tostring(err) or ''" + ), + ); + + let rejected: String = eval(&s, "return rejected"); + assert!( + rejected.contains("cannot be constructed from Lua"), + "a forged table must be rejected by type, not merely fail later; got {rejected:?}" + ); + assert!( + !ran(&s), + "a rejected destination must not reach the callback" + ); +} + +// --------------------------------------------------------------------------- +// §7 — the preflight matrix, in BOTH profiles (Q#DC-2) +// --------------------------------------------------------------------------- + +/// **N** — each of the four preconditions refuses under the document +/// profile, and each of the three the panel profile omits does **not** +/// refuse under it. +/// +/// This is the substance of Q#DC-2. The matrix: +/// +/// | # | precondition | document | panel | +/// |---|--------------|----------|-------| +/// | 1 | frontend has a layout | required | **required** | +/// | 2 | window still live | required | not applicable | +/// | 3 | window still shows the captured buffer | required | not applicable | +/// | 4 | window is not dedicated | required | not applicable | +/// +/// The panel column is the half that could not be written before this +/// lane, and the half most at risk of being "fixed" later by someone who +/// reads an omission as an oversight — a panel result does not occupy +/// the captured document window, does not replace its buffer, and does +/// not need it to exist, so each of checks 2–4 would refuse `git.status` +/// for a document-window change unrelated to where the panel goes. +/// +/// Table-driven so the failure message names *which* cell regressed, +/// which eight near-identical tests would give up in exchange for +/// nothing. +/// +/// *Mutation:* apply all four checks in both profiles — the three panel +/// rows fail. *Second mutation:* apply only check 1 in both profiles — +/// the three document rows fail. +#[test] +fn the_preflight_matrix_holds_in_both_profiles() { + // (label, Lua that breaks the precondition, reason fragment, + // whether the PANEL profile refuses too) + let cases: [(&str, &str, &str, bool); 4] = [ + ( + "frontend gone", + // Handled in Rust below: unregistering a view has no Lua surface. + "", + "requesting frontend is gone", + true, + ), + ( + "window gone", + "local doomed = dest:window() + pmacs.window.split_horizontal() + while pmacs.window.current() == doomed do pmacs.window.focus_next() end + pmacs.window.close_others()", + "is gone", + false, + ), + ( + "stale buffer", + "pmacs.window.switch_buffer(pmacs.buffer.create('*usurper*'))", + "now shows another buffer", + false, + ), + ( + "dedicated", + "pmacs.window.set_params(dest:window(), { dedicated = true })", + "is dedicated", + false, + ), + ]; + + for (label, break_it, expected, panel_refuses) in cases { + for profile in [None, Some("'panel'")] { + let s = editor(); + capture(&s); + + if label == "frontend gone" { + s.core + .borrow_mut() + .unregister_frontend_view(FrontendId::LOCAL); + } else { + exec(&s, break_it); + } + + commit(&s, profile); + assert_eq!( + raised(&s), + None, + "{label}/{profile:?}: a precondition is a refusal, not a raise" + ); + + let refuses = profile.is_none() || panel_refuses; + if refuses { + assert!(!ok(&s), "{label}/{profile:?}: commit_to must refuse"); + assert!( + reason(&s).contains(expected), + "{label}/{profile:?}: reason must say why; wanted {expected:?}, got {:?}", + reason(&s) + ); + assert!( + !ran(&s), + "{label}/{profile:?}: the callback must not run at all -- validating \ + after it is four mutations too late" + ); + } else { + assert!( + ok(&s), + "{label}/panel: this check is DELIBERATELY omitted for a panel \ + result, which touches no document window; got refusal {:?}", + reason(&s) + ); + assert!(ran(&s), "{label}/panel: the callback must run"); + } + } + } +} + +// --------------------------------------------------------------------------- +// §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 +/// bottom panel. It is the shape `listview.open` resolves to by default +/// (`builtin/runtime/listview.lua`), and the shape git's `*git-status*` +/// adoption will take. +const PANEL_BODY: &str = "pmacs.window.display(pmacs.buffer.create('*result*'), \ + { side = 'bottom' })"; + +/// A reusable panel: present and **undedicated**, so the preflight +/// measures "this frontend places side requests in the panel" and the +/// relaxation applies. Every mutation row starts from here except the +/// one whose whole point is that no panel exists yet. +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. +/// +/// The two arms are independent branches of +/// `EditorCore::resolve_placement`, so a fix that handled only one would +/// leave the other live. Every fallback test below drives both. +fn arrange_fallback(s: &EditorState, cause: &str) { + if cause == "not panel-capable" { + // Q#BP13's capability gate: `side` is honoured only on a + // panel-capable frontend. + s.core + .borrow_mut() + .views + .get_mut(&FrontendId::LOCAL) + .expect("LOCAL view") + .panel_capable = false; + } else { + // Q#BP3 2.iii: the one side slot is dedicated to another buffer, + // and a second panel is never created. + exec( + s, + "pmacs.window.display(pmacs.buffer.create('*pinned*'), + { side = 'bottom', dedicated = true, select = false })", + ); + assert!( + s.core.borrow().side_window_for(FrontendId::LOCAL).is_some(), + "{cause}: the arrangement must actually create the side slot" + ); + } +} + +/// **N** — a `"panel"` commit whose placement *already* falls back is +/// refused **before its body runs**, on the stale-intent reason. +/// +/// The defect: the panel column dropped checks 2–4 on the claim that a +/// panel result never touches a document window — but panel placement +/// falls back to an ordinary document window and then *installs the +/// result there* (`EditorCore::apply_placement` says so in its own +/// comment). The relaxation therefore handed a `"panel"` commit +/// permission to overwrite a document view with no stale-intent guard: +/// capture A, the user opens B, the continuation lands and B is gone. +/// +/// **This is the EARLY half, not the guarantee.** It is served by +/// `EditorCore::commit_destination_refusal` consulting +/// `panel_placement_can_fall_back`, which can only read the state that +/// holds *now*. The reason that is worth having anyway is the same reason +/// `commit_to` preflights at all: a body allocates a buffer, registers a +/// handle and paints long before it reaches any call that could refuse, +/// so refusing here leaves no debris. A frontend that cannot render a +/// panel will not acquire the capability mid-body, which is exactly the +/// case this catches. +/// +/// The guarantee — for the case a snapshot **cannot** catch, where the +/// body creates the fallback itself — is +/// `a_panel_commit_whose_body_creates_the_fallback_is_refused_at_placement`. +/// Neither test subsumes the other: this one pins that nothing runs, that +/// one pins that nothing lands. +/// +/// Each row asserts four things: the commit **refuses**, it refuses for +/// the stale-intent reason (not incidentally), the body never ran, and +/// the newer buffer is still there. +/// +/// *Mutation:* delete the `panel_placement_can_fall_back` arm from +/// `commit_destination_refusal`. Both rows fail — the body runs, and the +/// placement backstop then refuses as a *raise*, so `ok`/`ran`/`reason` +/// all move. +#[test] +fn a_panel_commit_that_falls_back_runs_the_document_preflight() { + for cause in ["not panel-capable", "side slot dedicated elsewhere"] { + let s = editor(); + + // Arrange the fallback cause BEFORE capturing, so the preflight + // can see it — which is exactly what distinguishes this test from + // the body-induced one below. + arrange_fallback(&s, cause); + + capture(&s); + let doc = local_window(&s); + assert_eq!( + eval::>(&s, "return dest:window()"), + Some(doc.raw()), + "{cause}: the capture must name the document window, not the panel" + ); + + // The user replaces the captured buffer while the work is in + // flight: `*newer*` is newer information than the request. + exec( + &s, + "pmacs.window.switch_buffer(pmacs.buffer.create('*newer*'))", + ); + assert_eq!( + name_in(&s, doc), + "*newer*", + "{cause}: the arrangement must make the captured window stale" + ); + + commit_body(&s, Some("'panel'"), PANEL_BODY); + + assert_eq!( + raised(&s), + None, + "{cause}: a precondition is a refusal, not a raise" + ); + assert!( + !ok(&s), + "{cause}: a \"panel\" commit that lands in a DOCUMENT window must run the \ + document preflight -- the relaxation is conditional on the placement really \ + being a panel" + ); + assert!( + reason(&s).contains("now shows another buffer"), + "{cause}: and refuse on stale intent; got {:?}", + reason(&s) + ); + assert!(!ran(&s), "{cause}: the callback must not run"); + assert_eq!( + name_in(&s, doc), + "*newer*", + "{cause}: the user's newer buffer must survive -- this is the assertion that \ + fails loudest when the guard is removed" + ); + } +} + +/// **N** — a body that tries to **create** the fallback is refused **at +/// the attempt**, and the refusal lands on the mutation rather than on +/// the outcome. +/// +/// This is the case no preflight snapshot can catch, and the two rows +/// above cannot reach it: both establish their fallback state *before* +/// `commit_to` is entered. The body is arbitrary **synchronous** Lua, so +/// refusing `await` — which stops another coroutine interleaving — +/// places no restriction on it: +/// +/// ```lua +/// pmacs.window.set_params(pmacs.window.panel(), { dedicated = true }) +/// pmacs.window.display(result, { side = "bottom" }) +/// ``` +/// +/// Two statements: the first invalidates the preflight, the second cashes +/// it in. The arrangement is deliberately the **inverse** of the rows +/// above — the preflight says "this lands in the panel", the relaxation +/// applies, and the body runs. +/// +/// **Asserting only "document B was not replaced" is insufficient**, and +/// an earlier version of this test made exactly that mistake: it passes +/// on a design that lets the body mutate freely and merely declines the +/// final installation, leaving every other side effect behind. So the +/// three assertions that matter are that the **dedication call itself is +/// refused**, the slot is **still undedicated afterwards**, and **nothing +/// partial was installed**. +/// +/// # One row per WRITE SITE, not per call spelling +/// +/// 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`, +/// because a test cannot express it: `panel_capable` has no Lua binding; +/// **losing** the side window is not a fallback route at all +/// (`resolve_placement` creates a fresh panel instead); and `quit` +/// restoring a `dedicated: true` presentation cannot be constructed, +/// since `QuitAction::Restore` only captures that flag on a *replacing* +/// side placement and a dedicated slot can never be the target of one. +/// +/// *Mutation:* delete the `panel_commit_dedication_refusal` call from +/// either guarded site — `set_params` drops row 1, `display_buffer` +/// 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() { + 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 preflight would have \ + refused and this row would be re-proving the preflight" + ); + } + let panel_buffer_before = panel_before.map(|panel| name_in(&s, panel)); + + capture(&s); + let doc = local_window(&s); + exec( + &s, + "pmacs.window.switch_buffer(pmacs.buffer.create('*newer*'))", + ); + + commit_body(&s, Some("'panel'"), &format!("{attempt}\n{PANEL_BODY}")); + + assert!( + ran(&s), + "{label}: the body must have run -- the preflight could not have known" + ); + + // 1. THE MUTATION ITSELF IS REFUSED, on content. + let raised = raised(&s).unwrap_or_else(|| { + panic!("{label}: the attempted mutation must be refused, not merely declined later") + }); + assert!( + raised.contains("cannot dedicate the side window"), + "{label}: the refusal must name the operation it is refusing; got {raised:?}" + ); + assert!( + raised.contains("\"panel\" commit_to"), + "{label}: and why it is refused here specifically; got {raised:?}" + ); + + // 2. THE SLOT IS STILL UNDEDICATED -- including the row where + // the slot would have been created dedicated, which must + // leave no slot at all rather than an undedicated one. + let panel_after = s.core.borrow().side_window_for(FrontendId::LOCAL); + assert_eq!( + panel_after, panel_before, + "{label}: a refused mutation must not have created or removed the side slot" + ); + if let Some(panel) = panel_after { + assert!( + !dedicated(&s, panel), + "{label}: a refused mutation must not have happened -- the whole design \ + rests on the preflight's measurement still being true afterwards" + ); + } + + // 3. NOTHING PARTIAL WAS INSTALLED. + if let (Some(panel), Some(before)) = (panel_after, panel_buffer_before.as_ref()) { + assert_eq!( + &name_in(&s, panel), + before, + "{label}: the panel must still show what it showed" + ); + } + assert_eq!( + name_in(&s, doc), + "*newer*", + "{label}: and the user's newer buffer must survive" + ); + assert!( + !buffer_exists(&s, "*result*"), + "{label}: the refusal must land BEFORE the body's own display -- a `*result*` \ + buffer means the commit got partway and then stopped" + ); + assert!( + !buffer_exists(&s, "*usurp*") || panel_after == panel_before, + "{label}: no usurping presentation may have been installed" + ); + } +} + +/// **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** — the restriction is scoped to its **frontend**: a nested commit +/// for a *different* frontend may still dedicate that frontend's own side +/// slot (revision 9). +/// +/// `panel_commit_dedication_refusal` scans every contract in force, but it +/// matches on `fid` as well as on the profile, and that comparison is a +/// deliberate exception rather than an oversight: frontend B's side slot +/// has no bearing on where **A's** side request lands. `resolve_placement` +/// consults only the requesting frontend's `panel_capable` and its own one +/// side window, so a contract for A cannot be invalidated by anything done +/// to B. +/// +/// **This is a POSITIVE pin, which is the shape this suite is thinnest +/// on** — every other row asserts a refusal. Without it, deleting the +/// `fid` comparison and making any outer `"panel"` contract *globally* +/// restrictive passes the whole file: the two nesting tests above use one +/// frontend, so the comparison is trivially true throughout them. An +/// exception that only the doc comment knows about is one review round +/// away from being "simplified" out. +/// +/// The far side is still asserted in the same run: A's slot stays +/// undedicated and A's commit still lands in A's panel, so this cannot +/// pass by having weakened the restriction generally. +/// +/// *Mutation:* delete `&& contract.destination.frontend == fid` from +/// `panel_commit_dedication_refusal` and only this test fails. +#[test] +fn a_nested_commit_for_another_frontend_may_dedicate_its_own_slot() { + let s = editor(); + + // Frontend B: its own layout, its own undedicated panel, and a + // destination captured while it is the acting frontend. + attach_frontend(&s, COMPETITOR); + s.core.borrow_mut().active_frontend = COMPETITOR; + exec( + &s, + "pmacs.window.display(pmacs.buffer.create('*b-panel*'), + { side = 'bottom', select = false }) + dest_b = pmacs.window.capture_destination()", + ); + let b_panel = s + .core + .borrow() + .side_window_for(COMPETITOR) + .expect("the competitor gets its own side slot"); + assert!( + !dedicated(&s, b_panel), + "B's slot must start undedicated, or the row would prove nothing" + ); + s.core.borrow_mut().active_frontend = FrontendId::LOCAL; + + // Frontend A: an undedicated panel, so its `"panel"` commit takes the + // relaxed preflight and the restriction is really in force. + exec(&s, PANEL_ARRANGED); + let a_panel = s + .core + .borrow() + .side_window_for(FrontendId::LOCAL) + .expect("the arrangement creates A's side slot"); + capture(&s); + let doc = local_window(&s); + exec( + &s, + "pmacs.window.switch_buffer(pmacs.buffer.create('*newer*'))", + ); + + commit_body( + &s, + Some("'panel'"), + &format!( + "b_ok, b_reason = pmacs.window.commit_to(dest_b, function() + pmacs.window.set_params(pmacs.window.panel(), {{ dedicated = true }}) + end) + {PANEL_BODY}" + ), + ); + + // 1. THE CROSS-FRONTEND DEDICATION IS ALLOWED. + assert_eq!( + raised(&s), + None, + "dedicating ANOTHER frontend's side slot must not be refused -- it cannot change \ + where this frontend's side request lands" + ); + assert!( + eval::(&s, "return b_ok == true"), + "the nested commit for B must be accepted: {}", + eval::(&s, "return tostring(b_reason)") + ); + assert!( + dedicated(&s, b_panel), + "B's slot must really be dedicated -- asserting only that nothing was refused \ + would pass on a call that was silently dropped" + ); + + // 2. AND A'S RESTRICTION IS UNWEAKENED: its slot is untouched and its + // commit still lands in its own panel rather than falling back. + assert!(ok(&s), "A's commit must be accepted: {}", reason(&s)); + assert!( + !dedicated(&s, a_panel), + "A's own slot must be untouched -- this row must not pass by having relaxed the \ + restriction for everyone" + ); + assert_eq!( + name_in(&s, a_panel), + "*result*", + "A's \"panel\" commit still belongs in A's panel" + ); + assert_eq!( + name_in(&s, doc), + "*newer*", + "and A's newer document buffer must survive" + ); +} + +/// **P** — a `"panel"` commit that falls back with a **still-valid** +/// destination lands in the document window, exactly as it does today. +/// +/// The guard refuses on *staleness*, not on *falling back*. Falling back +/// is deliberate graceful degradation for a frontend that cannot render a +/// panel (`EditorCore::apply_placement`), and turning it into an error +/// would regress every consumer that works today on such a frontend — a +/// much bigger behaviour change than the defect being fixed. +/// +/// Both causes, and asserted on **where the result landed** rather than +/// on the commit merely being accepted: a design that accepted the commit +/// and then dropped the display on the floor would pass a weaker version +/// of this. +/// +/// *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"] { + let s = editor(); + arrange_fallback(&s, cause); + capture(&s); + let doc = local_window(&s); + + // No staleness: the captured window still holds what it held. + commit_body(&s, Some("'panel'"), PANEL_BODY); + + assert_eq!(raised(&s), None, "{cause}: the commit must not raise"); + assert!( + ok(&s), + "{cause}: a fallback with an intact destination is graceful degradation, not \ + an error; got refusal {:?}", + reason(&s) + ); + assert!(ran(&s), "{cause}: the callback must run"); + assert_eq!( + name_in(&s, doc), + "*result*", + "{cause}: and the result really must land in the document window it fell \ + back to" + ); + } +} + +/// **P** — a `"panel"` commit that really lands in the panel still skips +/// checks 2–4. +/// +/// The other half of the correction, and it is not optional coverage. +/// The cheapest way to close the fallback hole is to make the panel +/// profile run the document preflight unconditionally — which passes +/// every fallback row above while quietly collapsing the two profiles +/// into one, leaving the whole parameterization buying nothing and +/// `git.status` refused for a document-window change unrelated to where +/// its panel goes. +/// +/// Deliberately arranged in the **same stale-intent state** the fallback +/// rows refuse on, so the only difference between this test and those is +/// whether the placement is really a panel. And it asserts *where* the +/// result went, not merely that the commit was accepted: an accepted +/// commit that still overwrote the document window would be the same +/// defect wearing a `true`. +/// +/// *Mutation:* widen the relaxation's condition back — i.e. make +/// `panel_placement_can_fall_back` return `true` unconditionally, or run +/// the document preflight for every `"panel"` commit. This fails on the +/// refusal; the fallback rows above still pass. **This is the pin that +/// makes "collapse the two profiles into one" a visible design change +/// rather than a quiet implementation choice.** +#[test] +fn a_panel_commit_that_really_lands_in_the_panel_keeps_its_relaxation() { + let s = editor(); + capture(&s); + let doc = local_window(&s); + + // Exactly the state the fallback rows refuse on. + exec( + &s, + "pmacs.window.switch_buffer(pmacs.buffer.create('*newer*'))", + ); + + commit_body(&s, Some("'panel'"), PANEL_BODY); + + assert!( + ok(&s), + "a panel-capable frontend with no dedicated side slot really places in the \ + panel, so checks 2-4 stay omitted; got refusal {:?}", + reason(&s) + ); + assert!(ran(&s), "and the callback must run"); + + let panel = s + .core + .borrow() + .side_window_for(FrontendId::LOCAL) + .expect("the commit must have created the side window"); + assert_eq!( + name_in(&s, panel), + "*result*", + "the result must land in the PANEL -- an accepted commit that fell back would \ + be the same defect with a `true` in front of it" + ); + assert_eq!( + name_in(&s, doc), + "*newer*", + "and the captured document window must be untouched" + ); +} + +// --------------------------------------------------------------------------- +// §7 — the profile argument (Q#DC-5) +// --------------------------------------------------------------------------- + +/// **P** — a two-argument `commit_to(dest, body)` takes the **document** +/// profile, so every caller written before the profile existed keeps all +/// four checks. +/// +/// Witnessed by a check the panel profile omits — a stale buffer. +/// Asserting merely that the call does not error would pass on a legacy +/// call silently downgraded to the panel profile, which is the +/// regression that would quietly void Journey Stage 1a's guarantees for +/// dired and every future two-argument caller. +/// +/// *Mutation:* default the profile to `Panel`. This fails; +/// `journey_acceptance` also fails, which is the point — the default is +/// what makes that suite's untouched pass a consequence of the signature +/// rather than of care. +#[test] +fn a_two_argument_commit_takes_the_document_profile() { + let s = editor(); + capture(&s); + exec( + &s, + "pmacs.window.switch_buffer(pmacs.buffer.create('*usurper*'))", + ); + + commit(&s, None); + + assert!( + !ok(&s), + "a two-argument commit must keep the stale-intent check" + ); + assert!( + reason(&s).contains("now shows another buffer"), + "and refuse for that reason; got {:?}", + reason(&s) + ); + assert!(!ran(&s), "the callback must not run"); +} + +/// **N** — an explicit `nil` profile is the document profile, exactly as +/// omitting it is. +/// +/// Witnessed separately from the two-argument case rather than assumed +/// equivalent: a Lua caller threading an optional variable produces +/// `commit_to(dest, body, nil)`, and a third behaviour there would stay +/// invisible until someone hit it in production. +/// +/// *Mutation:* treat `Value::Nil` as an unrecognized profile. This +/// fails; the two-argument test above does not, because mlua supplies +/// `Nil` for a missing argument either way only if the binding asks for +/// a `Value` — which is the type this suite also pins below. +#[test] +fn an_explicit_nil_profile_is_the_document_profile() { + let s = editor(); + capture(&s); + exec( + &s, + "pmacs.window.switch_buffer(pmacs.buffer.create('*usurper*'))", + ); + + commit(&s, Some("nil")); + + assert_eq!( + raised(&s), + None, + "an explicit nil must not be treated as a bad profile" + ); + assert!(!ok(&s), "an explicit nil must keep the stale-intent check"); + assert!( + reason(&s).contains("now shows another buffer"), + "and refuse for that reason; got {:?}", + reason(&s) + ); +} + +/// **N** — an unrecognized profile is an ERROR naming the accepted +/// values, and a non-string profile is refused by the **same** message. +/// +/// Two claims, one test, because their whole content is that they agree: +/// +/// * a fallback to `"document"` would hand a caller different checks +/// than it asked for — the failure the parameterization exists to +/// prevent — so an unknown string raises; +/// * **this is the guard on the argument's type.** With +/// `profile: Option` mlua rejects `42` and `{}` during +/// argument *conversion*, before the closure body runs, and the +/// message below becomes unreachable — the caller gets a generic +/// conversion error naming neither the rule nor the vocabulary. So the +/// number and table cases are asserted on the message's *content* and +/// against the string case's message, not merely on "an error +/// occurred". +/// +/// **The `invalid utf-8` row is the same reachability class one layer +/// down.** A Lua string is a *byte* string, so `string.char(255)` is a +/// perfectly ordinary `Value::String` that a `to_str()` inside the body +/// still fails to convert — surfacing mlua's generic UTF-8 error before +/// the documented message is ever constructed. Accepting `Value` is not +/// enough on its own; the comparison has to be on bytes. +/// +/// *Mutation:* retype the argument to `Option`. The number and +/// table rows fail. *Second mutation:* compare via `name.to_str()?`. The +/// `invalid utf-8` row fails. +#[test] +fn a_bad_profile_is_refused_by_one_message_that_names_the_accepted_values() { + let mut messages = Vec::new(); + for (label, profile) in [ + ("unknown string", "'documents'"), + ("number", "42"), + ("table", "{}"), + ("boolean", "true"), + ("invalid utf-8", "string.char(255)"), + ] { + let s = editor(); + capture(&s); + commit(&s, Some(profile)); + + let raised = raised(&s).unwrap_or_else(|| panic!("{label}: a bad profile must raise")); + assert!( + raised.contains("\"document\"") && raised.contains("\"panel\""), + "{label}: the message must name both accepted values; got {raised:?}" + ); + assert!( + raised.contains("must be the string"), + "{label}: and say a string was expected; got {raised:?}" + ); + assert!( + !ran(&s), + "{label}: a bad profile must not reach the callback" + ); + messages.push((label, raised)); + } + + let (_, first) = &messages[0]; + for (label, message) in &messages[1..] { + assert_eq!( + message, first, + "{label}: a non-string profile must be refused by the SAME message as an \ + unrecognized one -- a different message means mlua rejected the value \ + during argument conversion, which is what `Option` would do" + ); + } +} + +// --------------------------------------------------------------------------- +// §7 — no document window (Q#DC-4) +// --------------------------------------------------------------------------- + +/// **N** — a frontend with no live document window still captures, and +/// the destination reports the absence. +/// +/// Asserted as a *successful* capture rather than as `nil`: returning +/// `nil` here would push the adopter back onto ambient behaviour, which +/// is the P1a bug this lane removes. An adopter with nowhere to land +/// gets a refusal it can report; it does not get permission to guess. +/// +/// The `window()` accessor reporting **nil** is the other half: the pair +/// is set or cleared together, so no consumer ever sees a window id +/// without the buffer that was captured with it. +/// +/// *Mutation:* return `None` from `capture_view_destination` when +/// `primary_document_window` finds nothing. This fails on the capture +/// assertion inside the helper. *Second mutation:* keep `window` while +/// clearing `buffer`. This fails here. +#[test] +fn capture_succeeds_with_no_document_window() { + let s = editor(); + attach_documentless_frontend(&s, DOCUMENTLESS); + s.core.borrow_mut().active_frontend = DOCUMENTLESS; + + capture(&s); + + assert!( + eval::(&s, "return dest:window() == nil"), + "the document pair must be reported as ABSENT, not invented" + ); +} + +/// **N** — on that destination a panel commit **succeeds** and a +/// document commit is **refused**, naming the missing window. +/// +/// Both halves, because asserting only the refusal would pass on a +/// capture that refuses everything, and asserting only the success would +/// pass on one that checks nothing. Together they are Q#DC-4's decision: +/// the document pair is optional, and the profile is what decides +/// whether its absence matters. +/// +/// The refusal is a `(false, reason)` like the other four rather than a +/// raise, so an adopter handles all five the same way. +/// +/// *Mutation:* drop the `dest.window == None` arm. The document half +/// then commits against no window at all. +#[test] +fn a_panel_commit_succeeds_where_a_document_commit_is_refused() { + let s = editor(); + attach_documentless_frontend(&s, DOCUMENTLESS); + s.core.borrow_mut().active_frontend = DOCUMENTLESS; + capture(&s); + + commit(&s, Some("'panel'")); + assert!( + ok(&s), + "a panel result needs only a live frontend; got refusal {:?}", + reason(&s) + ); + assert!(ran(&s), "and its callback must run"); + + commit(&s, None); + assert_eq!( + raised(&s), + None, + "the missing document window joins the preflight refusals rather than raising" + ); + assert!(!ok(&s), "a document commit has nowhere to land"); + assert!( + reason(&s).contains("no document window"), + "and must say so; got {:?}", + reason(&s) + ); + assert!(!ran(&s), "and must not reach the callback"); +} + +// Isolated bootstrap storage roots (see the module docs): an +// integration test is compiled without `cfg(test)`, so a raw +// `EditorState::new()` would read the developer's real `init.lua` and +// write into their real data root. +#[path = "common/iso.rs"] +mod iso;