From 0947001b7073842dc7e6e31896d2c5e30e2a393e Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Wed, 23 Sep 2026 17:35:27 -0700 Subject: [PATCH] Each crate's tests compile alone; resolve.rs folds what it sends through the send door (improvise-a61f, improvise-aopa) a61f: `cargo test -p improvise-wasm-client --lib` failed with E0277, because `FormulaEntry: From<&str>` exists only under improvise-protocol's `test-support` feature, which that crate's dev-dependencies never asked for. `--workspace` hid it: feature unification pulled the feature in through improvise-command's dev-dep. A sweep of every member with `cargo test -p --no-run` found the same failure in the root `improvise` crate (tests/bug_hunt_reproductions.rs). Both now request the feature. The normal wasm tree still carries no `test-support`. aopa: eight helpers in resolve.rs's tests folded client-composed commands through `fold`, the host's door, so a composed Command::Model was never awaited in `in_flight`. All eight use `fold_outgoing`, one frame per key, as web/app.js does. `a_folded_browser_commit_waits_for_the_host` pins press_folded; the rival that folds through `fold` again fails it. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- Cargo.toml | 4 +- context/test-evidence/clients.md | 26 +++++++++++++ crates/improvise-protocol/src/resolve.rs | 47 +++++++++++++++--------- crates/improvise-wasm-client/Cargo.toml | 3 ++ 4 files changed, 61 insertions(+), 19 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 259d8478..8da5e69f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,7 +60,9 @@ enum_dispatch = "0.3.13" # binary does not grow a wasm client; `improvise-wasm-client` builds # natively, which is how its own tests already run. improvise-wasm-client = { path = "crates/improvise-wasm-client" } -improvise-protocol = { path = "crates/improvise-protocol" } +# `test-support` gives `FormulaEntry: From<&str>`, which the tests use; asked for +# here, or `-p improvise` alone does not compile (improvise-a61f). +improvise-protocol = { path = "crates/improvise-protocol", features = ["test-support"] } # `tests/headless_cli.rs` detaches the binary from any terminal with `setsid`. libc = "0.2" pest = "2.9.2" diff --git a/context/test-evidence/clients.md b/context/test-evidence/clients.md index 1b893acb..8c097931 100644 --- a/context/test-evidence/clients.md +++ b/context/test-evidence/clients.md @@ -5117,3 +5117,29 @@ Rival injected: `claim_as_read` ignoring the captured generation and claiming the current one. That is the pre-fix behaviour, restated after the fix. Only this test failed in the whole workspace. The layout tier staying current is still pinned by `an_edit_writes_the_cell_the_user_clicked_after_the_rows_renumber`. + +### a_folded_browser_commit_waits_for_the_host + +improvise-aopa. The helpers in `resolve.rs`'s tests fold what `resolve` +composed, which is what a browser **sends**. Eight of them folded it through +`ClientState::fold`, the door for what the **host** sends. That door does not +record a frame in `in_flight`, so a `Command::Model` folded there is never +awaited. Any assertion on rollback or settle state made through those helpers +would describe a client that cannot exist: the vacuity improvise-xoxh found in +its renaming test. All eight now use `fold_outgoing`, one frame per key, as +`web/app.js` and `lib.rs` do. + +The view moves the same way through either door, so no existing test could +tell them apart. This test is the cheap one that can. Through `press_folded` it +types `i` and `9`, neither of which composes a `Model`, so `in_flight` must be +empty. Then it presses `Enter`, which commits `9`, and requires exactly one +awaited frame whose commands are the commit's. + +Rival: `press_folded` folding through `fold` again. This test fails, and the +other 186 protocol tests pass. + +The other seven helpers are not pinned the same way: `heard`, `press`, the +sequence loop, the `enter-mode` and `append-char` folds, `arm` and the `yy` +fold. None of the tests that use them reads `in_flight` or `settled`, which is +why the bug was latent. A test that does read them now gets the true door +without having to know about it. diff --git a/crates/improvise-protocol/src/resolve.rs b/crates/improvise-protocol/src/resolve.rs index 981b5227..d9f4a0b0 100644 --- a/crates/improvise-protocol/src/resolve.rs +++ b/crates/improvise-protocol/src/resolve.rs @@ -2293,7 +2293,7 @@ mod tests { ..view.clone() }; let before = quiet(&state.view); - commands.iter().for_each(|c| state.fold(c)); + state.fold_outgoing(&commands); let sent = commands.iter().any(|c| matches!(c, Command::Model { .. })); let said = commands.iter().any(|c| matches!(c, Command::SetStatus(_))); match (sent || quiet(&state.view) != before, said) { @@ -2353,7 +2353,7 @@ mod tests { let (last, prefix) = sequence.split_last().expect("a sequence has a key"); for key in prefix { if let KeyOutcome::Local(commands) = resolve(&state, *key) { - commands.iter().for_each(|c| state.fold(c)); + state.fold_outgoing(&commands); } } let outcome = resolve(&state, *last); @@ -2520,7 +2520,7 @@ mod tests { let view = state.view.active_view.clone(); for key in &sequence { if let KeyOutcome::Local(commands) = resolve(&state, *key) { - commands.iter().for_each(|c| state.fold(c)); + state.fold_outgoing(&commands); } } let landed = state.view.cursor().selected; @@ -3448,9 +3448,7 @@ mod tests { let KeyOutcome::Local(commands) = resolve(client, ev(code)) else { panic!("{code:?} must resolve here"); }; - for command in &commands { - client.fold(command); - } + client.fold_outgoing(&commands); commands } @@ -3497,6 +3495,27 @@ mod tests { ); } + /// A frame this client composed goes through `fold_outgoing`, so a commit + /// waits for the host (improvise-aopa). See `context/test-evidence/clients.md`. + #[test] + fn a_folded_browser_commit_waits_for_the_host() { + let mut client = client_on_row_one("7"); + press_folded(&mut client, KeyCode::Char('i')); + press_folded(&mut client, KeyCode::Char('9')); + assert!( + client.in_flight.is_empty(), + "no Model yet, so nothing waits" + ); + let commit = press_folded(&mut client, KeyCode::Enter); + assert!( + commit.iter().any(|c| matches!(c, Command::Model { .. })), + "a typed commit writes: {commit:?}" + ); + assert_eq!(client.in_flight.len(), 1, "the commit is unanswered"); + assert!(client.in_flight[0].awaited); + assert_eq!(client.in_flight[0].cmds, commit); + } + /// improvise-465t: the editor an advance reopens starts untouched, even /// after a typed commit. See `context/test-evidence/clients.md`. #[test] @@ -3608,9 +3627,7 @@ mod tests { &["editing".into()], ) .expect("enter-mode editing composes here"); - for command in &entered { - client.fold(command); - } + client.fold_outgoing(&entered); assert!( client.view.mode.is_editing(), "enter-mode did not open an editor" @@ -3726,9 +3743,7 @@ mod tests { let KeyOutcome::Local(typed) = resolve(&client, ev(KeyCode::Char('5'))) else { panic!("a character in Editing is append-char"); }; - for command in &typed { - client.fold(command); - } + client.fold_outgoing(&typed); assert!( matches!( committed(&client), @@ -3824,9 +3839,7 @@ mod tests { let KeyOutcome::Local(commands) = resolve(state, prefix) else { panic!("{prefix:?} is a prefix; arming it must resolve locally"); }; - for command in &commands { - state.fold(command); - } + state.fold_outgoing(&commands); assert_eq!( state.view.transient_prefix, Some((prefix.code, prefix.modifiers)), @@ -3870,9 +3883,7 @@ mod tests { "the cell under the cursor reads 1200: {yank:?}" ); - for command in &yank { - state.fold(command); - } + state.fold_outgoing(&yank); assert_eq!( state.view.transient_prefix, None, "and the y prefix is spent" diff --git a/crates/improvise-wasm-client/Cargo.toml b/crates/improvise-wasm-client/Cargo.toml index 1abfbb14..34738d92 100644 --- a/crates/improvise-wasm-client/Cargo.toml +++ b/crates/improvise-wasm-client/Cargo.toml @@ -40,6 +40,9 @@ wasm-bindgen = "0.2" [dev-dependencies] improvise-command = { path = "../improvise-command" } improvise-core = { path = "../improvise-core" } +# The tests build `FormulaEntry` from `&str`, which only `test-support` provides. +# Asked for here, or `-p improvise-wasm-client` alone does not compile (a61f). +improvise-protocol = { path = "../improvise-protocol", features = ["test-support"] } improvise-ws-server = { path = "../improvise-ws-server" } # The `wasm-opt` wasm-pack downloads predates bulk-memory being enabled by