fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge commit 'a634b3f356bdbc725bf2c42bb5a56e0b2f2ef0eb' into fix/we1a-abort-refuses

Browse Source 
# Conflicts:
#	context/test-evidence/command.md
This commit is contained in:
Edward Langley
2026-09-23 08:59:59 -07:00
parent 9da9e08e57 a634b3f356
commit 600d0a01d3
3 changed files with 145 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/code/command.md
+25 -3
View File
@@ -2322,9 +2322,31 @@ would be sent and every client would stay stale *permanently* rather than until
next scroll: a partial fix strictly worse than none. Broadcasting only would leave
the server projecting each client a window it is no longer looking at.
The audience is `Broadcast::ToAll`, and the asker is not an exception. Its mirror
was reset here like everyone else's, and it folded nothing optimistically, because
a `Command::Model` is not something a client can apply to itself.
The audience is every session on a reframed view, each addressed with
`Broadcast::To` (improvise-nynk), and the asker is not an exception: a transpose
invalidates its own position too. What an earlier version of this paragraph got
wrong is the claim that the asker "folded nothing optimistically". A
`Command::Model` is not something a client can apply to itself, but the **rest of
its frame** is, and a browser frame routinely places the cursor *after* the
mutation. The `+cat` commit is `[ChangeMode, AddCategory, SetSelected]`. The
mirror folds that in order: reset, then placed, which is exactly the order the TUI
applies the same batch in. The client folded all of it **before** hearing the
reset, so the relayed `ResetCursor` arrived last and threw the placement away.
The client sat at the origin while its mirror, and every projection built from
it, sat at the placement (improvise-wdmp).
So the host answers the asker in the order the mirror saw. Once the asker has been
told `ResetCursor`, `Host::mutating_frame` relays back to it every **later**
cursor-placing command of the same frame (`SetSelected`, `SetRowOffset`,
`SetColOffset`), ahead of that command's projections. All three are absolute, so
folding them twice is idempotent. Excluding the asker from the reset, which the
ticket proposed, was rejected. For a frame that places nothing after the reframe
it is gotcha 24's own "strictly worse than nothing": the mirror is reset and the
client never hears it. `a_frame_that_reframes_and_then_places_the_cursor_leaves_the_asker_placed`
runs both shapes. One residue is stated rather than handled: a frame that
reframes and then *switches views* before placing would reset the view the
client ends on rather than the one it reframed. No resolver composes that frame
today.
What is *not* broadcast is a position: another client's transpose must not move
where I am looking — that is UI and it is mine (improvise-g0f). It may only declare
context/test-evidence/command.md
+24
View File
@@ -6078,6 +6078,30 @@ runs `cargo test --workspace --no-fail-fast`. Four of four fail this test, and
nothing else fails. None of the four needs an abort: each is the only effect its
command emits, and `p`/`i` are single bindings, not sequences.
### a_frame_that_reframes_and_then_places_the_cursor_leaves_the_asker_placed
improvise-wdmp. A browser frame that reframes the grid and then places the cursor,
for example the records `+cat` commit `[ChangeMode, AddCategory, SetSelected]`, is
folded by the client all at once and before any answer arrives. The host's mirror
folds the same frame in order. `adopt_cursor_epoch` resets the mirror at the
mutation, and the later `SetSelected` places it. The relayed `ResetCursor`
reached the client **after** its own placement and undid it. So the client sat at
the origin while its mirror, and every projection built from it, sat at the
placement. That is a split, not merely the jump the ticket described. The test
drives a real `Host` and a real `ClientState` through `fold_outgoing`, then folds
every frame addressed to the asker. It seats the cursor off the origin first, so
a reset that never arrived is visible. It runs two shapes: with a placement
after the reframe (expect `(1, 0)`), and without one (expect `(0, 0)`, gvnb's
reset). In both, the client's cursor must equal its mirror's. At the base the
placed case failed: the client was at `(0, 0)` and the mirror at `(1, 0)`.
Two rivals. First, the echo disabled, which is the pre-fix behaviour: the placed
case fails. Second, the ticket's suggested fix, which drops the asker's
`ResetCursor` from the relay: the unplaced case fails, because the mirror was
reset and the client never heard. That rival also fails
`a_geometry_command_relays_the_cursor_reset_to_the_asker_too` and the
ws-server's `the_other_document_would_also_have_a_cursor_to_invalidate`.
## `ui/host.rs` — the abort arm of `Host::mutating_frame` (improvise-we1a)
### every_failing_model_effect_that_aborts_also_refuses
crates/improvise-command/src/ui/host.rs
+96 -1
View File
@@ -162,6 +162,7 @@ impl Host {
let undo = self.app.snapshot();
let mirrors = self.sessions.clone();
let mut out = Vec::new();
let mut asker_reset = false;
for cmd in batch {
if let Some(reason) = reduce::broken_chain(&self.app, cmd.command()) {
return self.undo_frame(id, undo, mirrors, reason);
@@ -169,7 +170,23 @@ impl Host {
// Only a mutation runs `apply_effects`, which resets both flags — so
// after anything else they are STALE. A refusal need not abort (cz8d).
let mutation = matches!(cmd.command(), Command::Model { .. });
out.extend(self.command(id, cmd));
let command = cmd.command();
let mut answered = self.command(id, cmd);
// The asker folded this placement BEFORE hearing its reset, so it is owed
// again after it (improvise-wdmp, `context/code/command.md`).
if asker_reset && places_cursor(command) {
answered.insert(
0,
Addressed {
to: id,
msg: ServerFrame::Commands(vec![command.clone()]),
},
);
}
asker_reset |= answered
.iter()
.any(|a| a.to == id && is_cursor_reset(&a.msg));
out.extend(answered);
let failed = self.app.last_batch_refused() || self.app.last_batch_aborted();
if mutation && failed {
return self.undo_frame(id, undo, mirrors, Rejection::MutationRefused);
@@ -266,6 +283,19 @@ impl Host {
}
}
/// A command that writes this client's cursor, and so survives a reset only if
/// it is folded after it.
fn places_cursor(cmd: &Command) -> bool {
matches!(
cmd,
Command::SetSelected { .. } | Command::SetRowOffset(_) | Command::SetColOffset(_)
)
}
fn is_cursor_reset(msg: &ServerFrame) -> bool {
matches!(msg, ServerFrame::Commands(cmds) if cmds.contains(&Command::ResetCursor))
}
/// One file per way `ImportJsonHeadless` can fail, plus one that imports; the
/// `absent.*` paths are left unwritten. Shared by the frame and key-path tests.
#[cfg(test)]
@@ -1706,6 +1736,71 @@ mod tests {
}
}
/// improvise-wdmp: the asker's own frame placed its cursor after the reframe,
/// and the relayed reset undid it. Evidence: `context/test-evidence/command.md`.
#[test]
fn a_frame_that_reframes_and_then_places_the_cursor_leaves_the_asker_placed() {
use improvise_protocol::{AppMode, ClientState};
// With a placement after the reframe, and without one: the asker ends where
// the TUI would, and its client agrees with its mirror either way.
for (placed, expected) in [(true, (1, 0)), (false, (0, 0))] {
let mut host = host();
let (asker, snapshot) = host.connect(10, 10);
let (bystander, _) = host.connect(10, 10);
let mut client = ClientState::default();
match snapshot.msg {
ServerFrame::Snapshot(s) => client.bootstrap(*s),
other => panic!("setup: connect answers a Snapshot, got {other:?}"),
}
let send = |host: &mut Host, client: &mut ClientState, frame: Vec<Command>| {
client.fold_outgoing(&frame);
let answer = host.handle(asker, ClientFrame::Commands(frame));
for msg in to(&answer, asker) {
match msg {
ServerFrame::Commands(cmds) => cmds.iter().for_each(|c| client.fold(c)),
ServerFrame::Accepted => client.accepted(),
other => panic!("the frame must be accepted: {other:?}"),
}
}
answer
};
// Off the origin first, so a reset that never arrived is visible.
send(
&mut host,
&mut client,
vec![Command::SetSelected { row: 1, col: 0 }],
);
// The `+cat` commit's shape: tidy-up, the schema change, then (maybe)
// the cursor put back where the user was (resolve.rs::create_column).
let add = ModelCommand::AddCategory("Product".into());
let mut frame = vec![
Command::ChangeMode(AppMode::Normal),
crate::ui::reduce::caught_up(host.app(), add),
];
if placed {
frame.push(Command::SetSelected { row: 1, col: 0 });
}
let answer = send(&mut host, &mut client, frame);
assert!(
to(&answer, bystander).contains(&ServerFrame::Commands(vec![Command::ResetCursor])),
"setup: the frame must really reframe, or nothing here is at stake"
);
let mirror = host.sessions.get(asker).unwrap().view().cursor();
assert_eq!(
mirror.selected, expected,
"setup, placed={placed}: the mirror"
);
assert_eq!(
client.view.cursor(),
mirror,
"placed={placed}: the asker's client and its mirror disagree"
);
}
}
/// A refused batch is answered to the **asker and nobody else** (ADR 001):
/// nothing happened, so there is nothing to tell the others.
#[test]