Merge pull request #174 from fiddlerwoaroof/wire2/gho5

Rolled-back-load tests witness that the load ran (gho5)

context/test-evidence/command.md

@@ -2382,6 +2382,31 @@ keystroke to land in.
 Injected fault: dropping `self.view_state = snapshot.view_state` from
 `App::restore` fails this test and only this test.
 
+**Addendum (improvise-gho5).** Improvise-gho5 found that this
+test and `a_rolled_back_load_tells_nobody_and_puts_every_mirror_back` passed
+against a host that **never ran the load**. The review's fault, reproduced
+here, was placed at the top of `Host::commands`: refuse any multi-command frame
+that contains a `LoadModel` with `Rejected(DocumentReplaced)`, before
+`CheckedBatch` runs. Both tests stayed green. A frame that never got that far
+leaves the generation, the view, the status line and every mirror untouched,
+which is exactly what the tests asserted.
+
+Both tests now assert that the asker's whole reply is
+`the_load_ran_then_the_write_broke()`, which is `Rejected(ChainBroken(North))`.
+They also assert, before the frame, that `North` resolves. Together these make
+the rejection a witness that the load ran. `ChainBroken` comes only from
+`reduce::broken_chain`, which runs inside `mutating_frame` just before each
+mutation. It names `North` only once the document no longer holds that
+record, and only an applied `LoadModel` in this frame can have removed it.
+
+With the same fault re-injected, both tests fail. The left side is
+`[Rejected(DocumentReplaced)]` and the right is
+`[Rejected(ChainBroken(<North>))]`. The fault was reverted.
+
+The residual: a rogue up-front check that forged `ChainBroken` specifically
+would still pass. That is the same forgery a hand-rolled client could commit
+against any basis check, and a test cannot guard against it.
+
 ### a_refused_mutation_rolls_back_its_predecessor_and_the_abort_flag
 
 The `Rejection::MutationRefused` arm — the one the first hand-back wrongly called
@@ -4885,9 +4910,14 @@ Three things make it non-vacuous:
   `LoadModel` silently failed, `Applied::Rebased` would never run and every
   assertion here would pass over nothing. The control asserts the generation
   moves and that *both* clients are owed a `Snapshot`.
-- **The rollback is asserted before anything is concluded from it** — the
+- **The load is witnessed to have RUN before anything is concluded from the
-  `document_generation` equality is checked first, so "the frame was rejected"
+  rollback** (improvise-gho5). The asker's whole reply must be
-  cannot be confused with "the frame never got that far".
+  `Rejected(ChainBroken(North))`. `North` is asserted to resolve before the
+  frame, so only `broken_chain` can give that answer, and only after the
+  `LoadModel` has replaced the document. `CheckedBatch` never produces
+  `ChainBroken`. This bullet used to say the `document_generation` equality
+  did the job, and that was false: generation equality is exactly what holds
+  when the frame never got that far.
 - **A bystander is seated where a wrong relay could move it.** A single-client
   fixture cannot tell "the mirrors were restored" from "there was one mirror and
   it happened to be the asker's".

crates/improvise-command/src/ui/host.rs

@@ -1049,6 +1049,12 @@ mod tests {
         let was_view = host.app().view_state.active_view.clone();
         let was_status = host.app().view_state.status_msg.clone();
         let basis = frame_basis(&host, &["North"]);
+        assert!(
+            host.app()
+                .resolve_record(addressed("North").record)
+                .is_some(),
+            "North must resolve BEFORE the load, or a refusal naming it proves nothing"
+        );
 
         // The load lands, then the write names a record the incoming document
         // has never had — so the frame fails at its second command.
@@ -1069,11 +1075,10 @@ mod tests {
             ]),
         );
 
-        assert!(
+        assert_eq!(
-            answer
+            to(&answer, asker),
-                .iter()
+            vec![the_load_ran_then_the_write_broke()],
-                .any(|a| matches!(a.msg, ServerFrame::Rejected(_))),
+            "only a load that RAN can refuse the write this way: {answer:?}"
-            "the fixture must actually refuse the frame: {answer:?}"
         );
         assert_eq!(
             host.app().view_state.active_view,
@@ -1097,6 +1102,12 @@ mod tests {
         );
     }
 
+    /// `North` resolves in the outgoing document, so only `broken_chain` AFTER the
+    /// load applied can answer this. Evidence: `context/test-evidence/command.md`.
+    fn the_load_ran_then_the_write_broke() -> ServerFrame {
+        ServerFrame::Rejected(Rejection::ChainBroken(addressed("North").record))
+    }
+
     /// Everything one client was told, flattened to the projections it carries.
     fn projections(frames: &[Addressed], id: SessionId) -> Vec<Projection> {
         to(frames, id)
@@ -1147,6 +1158,12 @@ mod tests {
         let (bystander, _) = host.connect(10, 10);
         let was_generation = host.app().document_generation();
         let basis = frame_basis(&host, &["North"]);
+        assert!(
+            host.app()
+                .resolve_record(addressed("North").record)
+                .is_some(),
+            "North must resolve BEFORE the load, or a refusal naming it proves nothing"
+        );
 
         // The load lands and rebases every mirror; the write then names a
         // record the incoming document has never had, so the frame fails.
@@ -1167,6 +1184,11 @@ mod tests {
             ]),
         );
 
+        assert_eq!(
+            to(&answer, asker),
+            vec![the_load_ran_then_the_write_broke()],
+            "only a load that RAN can refuse the write this way: {answer:?}"
+        );
         assert_eq!(
             host.app().document_generation(),
             was_generation,