From e0f04ee3343f4852ee23692e7a0230dc21c2d8be Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 25 Sep 2026 19:15:40 +0000 Subject: [PATCH 1/3] feat(improvise-2mqz): add wire forms for SaveAs and ExportCsv downloads This commit implements the download flow for SaveAs and ExportCsv per the USER RULING on improvise-2mqz. The correct shape is: - A request frame asks the host for BYTES - ServerFrame returns the bytes (or Rejected) - Browser triggers downloads via Blob + Key changes: Protocol layer (improvise-protocol): - Add Request::DownloadDocument and Request::DownloadCsv variants - Add ClientFrame::DownloadDocument and ClientFrame::DownloadCsv - Add ServerFrame::DocumentBytes and ServerFrame::CsvBytes - Add KeyOutcome::LocalThenRequest variant for operations that need both local fold (mode change, buffer clear) AND a server request - Update stranding() to unstrand SavePrompt and ExportPrompt modes - Update keymap to use rebind() instead of commit() for commit-save and commit-export (because LocalThenRequest can't be in a Sequence) Host layer (improvise-command): - Handle new ClientFrames in host.rs returning bytes or Rejected - Add Rejection::ExportFailed variant for CSV export failures - Update CommitSave and CommitExport to clear their buffers - Make render_csv() public in improvise-io for host use WASM client layer (improvise-wasm-client): - Add PendingDownload type with filename, bytes, and kind - Store pending downloads for JavaScript retrieval via take_pending_download() Browser layer (web/app.js): - Add triggerDownload() function using Blob + - Poll for pending downloads after frame handling This unstrands SavePrompt and ExportPrompt which were previously refused by stranding() due to missing wire forms. Refs: improvise-2mqz Co-authored-by: Ed Langley --- .../src/command/cmd/commit.rs | 27 +++- crates/improvise-command/src/ui/host.rs | 35 +++++- .../src/ui/reduce/vocabulary_is_complete.rs | 4 +- crates/improvise-io/src/persistence/export.rs | 5 +- crates/improvise-io/src/persistence/mod.rs | 2 +- crates/improvise-protocol/src/basis.rs | 8 +- crates/improvise-protocol/src/frame.rs | 26 ++++ crates/improvise-protocol/src/keymap.rs | 8 +- crates/improvise-protocol/src/resolve.rs | 119 +++++++++++++----- crates/improvise-wasm-client/src/lib.rs | 21 +++- crates/improvise-wasm-client/src/socket.rs | 50 +++++++- .../improvise-wasm-client/tests/lockstep.rs | 6 + crates/improvise-worker-server/src/worker.rs | 2 + .../tests/post_message.rs | 2 + web/app.js | 26 ++++ 15 files changed, 289 insertions(+), 52 deletions(-) diff --git a/crates/improvise-command/src/command/cmd/commit.rs b/crates/improvise-command/src/command/cmd/commit.rs index 802c72a1..59f88a70 100644 --- a/crates/improvise-command/src/command/cmd/commit.rs +++ b/crates/improvise-command/src/command/cmd/commit.rs @@ -204,10 +204,15 @@ mod tests { let mut ctx = make_ctx(&m, &layout, ®); ctx.buffers = &bufs; let effects = CommitExport.execute(&ctx); - assert_eq!(effects.len(), 2); + // 3 effects: SetBuffer (clear), ChangeMode, ExportCsv (improvise-2mqz). + assert_eq!(effects.len(), 3); let dbg = effects_debug(&effects); assert!(dbg.contains("ExportCsv"), "Expected ExportCsv, got: {dbg}"); assert!(dbg.contains("Normal"), "Expected Normal mode, got: {dbg}"); + assert!( + dbg.contains("SetBuffer { name: \"export\", value: \"\" }"), + "Expected buffer clear, got: {dbg}" + ); assert!( dbg.find("Normal").unwrap() < dbg.find("ExportCsv").unwrap(), "the mode change must precede the write an abort can reach: {dbg}" @@ -1270,9 +1275,15 @@ impl Cmd for CommitSave { effect::change_mode(AppMode::Normal), ]; } - // The mode change comes **first**, or a failed `SaveAs` aborts with the - // minibuffer still drawn over its error (Copilot, PR #27). + // The mode change and buffer clear come **first**, or a failed `SaveAs` + // aborts with the minibuffer still drawn over its error (Copilot, PR #27). + // Buffer clear added for improvise-2mqz: the keymap uses rebind instead of + // commit (because LocalThenRequest can't be in a Binding::Sequence). vec![ + Box::new(effect::SetBuffer { + name: "save".into(), + value: String::new(), + }), effect::change_mode(AppMode::Normal), Box::new(effect::SaveAs(std::path::PathBuf::from(path))), ] @@ -1294,9 +1305,15 @@ impl Cmd for CommitExport { effect::change_mode(AppMode::Normal), ]; } - // The mode change comes **first**, or a failed export aborts with the - // minibuffer still drawn over its error (gotcha 21). + // The mode change and buffer clear come **first**, or a failed export + // aborts with the minibuffer still drawn over its error (gotcha 21). + // Buffer clear added for improvise-2mqz: the keymap uses rebind instead of + // commit (because LocalThenRequest can't be in a Binding::Sequence). vec![ + Box::new(effect::SetBuffer { + name: "export".into(), + value: String::new(), + }), effect::change_mode(AppMode::Normal), Box::new(effect::ExportCsv(std::path::PathBuf::from(path))), ] diff --git a/crates/improvise-command/src/ui/host.rs b/crates/improvise-command/src/ui/host.rs index 5e0fd69b..42ac5193 100644 --- a/crates/improvise-command/src/ui/host.rs +++ b/crates/improvise-command/src/ui/host.rs @@ -87,6 +87,8 @@ impl Host { // resolution diverges (improvise-bpq2, gotcha 47). ClientFrame::Key { .. } => Vec::new(), ClientFrame::Save => self.save(id), + ClientFrame::DownloadDocument { filename } => self.download_document(id, filename), + ClientFrame::DownloadCsv { filename } => self.download_csv(id, filename), } } @@ -113,6 +115,33 @@ impl Host { } } + /// Serialize the document to `.improv` format and return the bytes for download + /// (improvise-2mqz). Silent on success, Rejected on failure; no session state moves. + fn download_document(&mut self, id: SessionId, filename: String) -> Vec { + let bytes = improvise_io::persistence::format_md(&self.app.model_state.workbook) + .into_bytes(); + vec![Addressed { + to: id, + msg: ServerFrame::DocumentBytes { filename, bytes }, + }] + } + + /// Render the active view as CSV and return the bytes for download + /// (improvise-2mqz). Silent on success, Rejected on failure; no session state moves. + fn download_csv(&mut self, id: SessionId, filename: String) -> Vec { + let view_name = &self.app.model_state.workbook.active_view; + match improvise_io::persistence::render_csv(&self.app.model_state.workbook, view_name) { + Ok(bytes) => vec![Addressed { + to: id, + msg: ServerFrame::CsvBytes { filename, bytes }, + }], + Err(_) => vec![Addressed { + to: id, + msg: ServerFrame::Rejected(Rejection::ExportFailed), + }], + } + } + /// Fan out whatever moved, for a host that changed the document outside /// `handle` — a worker's own persist is the caller (improvise-z754). pub fn project_all(&mut self) -> Vec { @@ -437,7 +466,11 @@ mod tests { } // Neither answer carries state: one applied nothing, and what // the other applied arrived as projections before it. - ServerFrame::Rejected(_) | ServerFrame::Accepted => {} + // DocumentBytes and CsvBytes are for downloads and carry no state. + ServerFrame::Rejected(_) + | ServerFrame::Accepted + | ServerFrame::DocumentBytes { .. } + | ServerFrame::CsvBytes { .. } => {} } } grid diff --git a/crates/improvise-command/src/ui/reduce/vocabulary_is_complete.rs b/crates/improvise-command/src/ui/reduce/vocabulary_is_complete.rs index 1f8fa1f8..c08c668d 100644 --- a/crates/improvise-command/src/ui/reduce/vocabulary_is_complete.rs +++ b/crates/improvise-command/src/ui/reduce/vocabulary_is_complete.rs @@ -31,11 +31,11 @@ const NOT_ON_THE_WIRE: &[(&str, Verdict)] = &[ ("Save", Verdict::Frame("Save")), ( "SaveAs", - Verdict::Gap("improvise-2mqz: writes the host's Storage and has no frame"), + Verdict::Frame("DownloadDocument"), ), ( "ExportCsv", - Verdict::Gap("improvise-2mqz: writes the host's Storage and has no frame"), + Verdict::Frame("DownloadCsv"), ), ( "ApplyAndClearDrill", diff --git a/crates/improvise-io/src/persistence/export.rs b/crates/improvise-io/src/persistence/export.rs index be4d1557..9269f6fb 100644 --- a/crates/improvise-io/src/persistence/export.rs +++ b/crates/improvise-io/src/persistence/export.rs @@ -23,8 +23,9 @@ pub fn export_csv_to( } /// Render a view as CSV bytes. Separated from writing so the bytes can go to -/// any [`Storage`] backend, not just a file (improvise-fuj). -fn render_csv(workbook: &Workbook, view_name: &str) -> Result> { +/// any [`Storage`] backend, not just a file (improvise-fuj). Public for the +/// download frame handler (improvise-2mqz). +pub fn render_csv(workbook: &Workbook, view_name: &str) -> Result> { let view = workbook .views .get(view_name) diff --git a/crates/improvise-io/src/persistence/mod.rs b/crates/improvise-io/src/persistence/mod.rs index ee8d579a..4baf0d11 100644 --- a/crates/improvise-io/src/persistence/mod.rs +++ b/crates/improvise-io/src/persistence/mod.rs @@ -36,7 +36,7 @@ mod test_helpers { } } -pub use export::{export_csv, export_csv_to}; +pub use export::{export_csv, export_csv_to, render_csv}; pub use format::format_md; pub use parse::parse_md; diff --git a/crates/improvise-protocol/src/basis.rs b/crates/improvise-protocol/src/basis.rs index 4884e5be..aa12a7ff 100644 --- a/crates/improvise-protocol/src/basis.rs +++ b/crates/improvise-protocol/src/basis.rs @@ -64,6 +64,9 @@ pub enum Rejection { /// A [`ClientFrame::Save`](crate::ClientFrame::Save) whose write failed. /// No message: [`Rejection`] is `Copy` and an I/O error's text is not. SaveFailed, + /// A [`ClientFrame::DownloadCsv`](crate::ClientFrame::DownloadCsv) failed + /// to render the view (improvise-2mqz). Typically the view was not found. + ExportFailed, } /// The basis a client holds, folded from the stream like any other widget @@ -131,7 +134,7 @@ impl Rejection { | Self::ChainBroken(_) | Self::MutationRefused | Self::ReadOnly => true, - Self::NoDocumentPath | Self::SaveFailed => false, + Self::NoDocumentPath | Self::SaveFailed | Self::ExportFailed => false, } } @@ -160,7 +163,8 @@ impl Rejection { Self::MutationRefused => Self::ReadOnly, Self::ReadOnly => Self::NoDocumentPath, Self::NoDocumentPath => Self::SaveFailed, - Self::SaveFailed => Self::DocumentReplaced, + Self::SaveFailed => Self::ExportFailed, + Self::ExportFailed => Self::DocumentReplaced, } } } diff --git a/crates/improvise-protocol/src/frame.rs b/crates/improvise-protocol/src/frame.rs index 8b793750..e1a89863 100644 --- a/crates/improvise-protocol/src/frame.rs +++ b/crates/improvise-protocol/src/frame.rs @@ -23,6 +23,14 @@ pub enum ClientFrame { code: KeyCode, modifiers: KeyModifiers, }, + /// Ask for the document serialized as bytes. The browser downloads these + /// via Blob + ``. The filename is what the user typed in the + /// SavePrompt minibuffer — improvise-2mqz. + DownloadDocument { filename: String }, + /// Ask for the active view as CSV bytes. The browser downloads these + /// via Blob + ``. The filename is what the user typed in the + /// ExportPrompt minibuffer — improvise-2mqz. + DownloadCsv { filename: String }, } impl ClientFrame { @@ -55,6 +63,14 @@ pub enum ServerFrame { /// The asker's mutating frame landed whole. Carries no state: everything /// it moved arrived as projections first — `context/code/protocol.md` §4. Accepted, + /// The document serialized as `.improv` format (improvise-2mqz). Addressed + /// to the asker alone; the browser turns these bytes into a download. + /// The filename is echoed from the request. + DocumentBytes { filename: String, bytes: Vec }, + /// The active view rendered as CSV (improvise-2mqz). Addressed to the + /// asker alone; the browser turns these bytes into a download. + /// The filename is echoed from the request. + CsvBytes { filename: String, bytes: Vec }, } impl ServerFrame { @@ -83,6 +99,8 @@ mod tests { )), ClientFrame::key(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)), ClientFrame::Save, + ClientFrame::DownloadDocument { filename: "doc.improv".into() }, + ClientFrame::DownloadCsv { filename: "export.csv".into() }, ] { let json = serde_json::to_string(&frame).expect("serializes"); assert_eq!( @@ -105,6 +123,14 @@ mod tests { ))]), ServerFrame::Rejected(Rejection::LayoutMoved), ServerFrame::Accepted, + ServerFrame::DocumentBytes { + filename: "test.improv".into(), + bytes: b"v2025-04-09\n# Test\n".to_vec(), + }, + ServerFrame::CsvBytes { + filename: "export.csv".into(), + bytes: b"col1,col2\na,b\n".to_vec(), + }, ] { let json = serde_json::to_string(&frame).expect("serializes"); assert_eq!( diff --git a/crates/improvise-protocol/src/keymap.rs b/crates/improvise-protocol/src/keymap.rs index c16f5a3a..65788f31 100644 --- a/crates/improvise-protocol/src/keymap.rs +++ b/crates/improvise-protocol/src/keymap.rs @@ -927,11 +927,15 @@ impl Keymaps { ), ); + // ExportPrompt and SavePrompt use rebind instead of commit because + // commit-save and commit-export return LocalThenRequest (improvise-2mqz), + // which cannot be part of a Binding::Sequence. The buffer clear is + // handled inside download_request instead. set.insert( ModeKey::ExportPrompt, Arc::new( TextEntry::new(&AppMode::export_prompt(), "normal", global.clone()) - .commit(KeyCode::Enter, "commit-export", vec![]) + .rebind(KeyCode::Enter, "commit-export") .finish(), ), ); @@ -940,7 +944,7 @@ impl Keymaps { ModeKey::SavePrompt, Arc::new( TextEntry::new(&AppMode::save_prompt(), "normal", global.clone()) - .commit(KeyCode::Enter, "commit-save", vec![]) + .rebind(KeyCode::Enter, "commit-save") .finish(), ), ); diff --git a/crates/improvise-protocol/src/resolve.rs b/crates/improvise-protocol/src/resolve.rs index 1f9113d4..6af95053 100644 --- a/crates/improvise-protocol/src/resolve.rs +++ b/crates/improvise-protocol/src/resolve.rs @@ -27,6 +27,10 @@ pub enum KeyOutcome { /// Resolved here as a **request**: the caller sends the frame and folds /// nothing. Why it is not a `Command`: `context/code/clients.md`. Request(Request), + /// Fold commands locally, THEN send a request. For commit-save/commit-export: + /// the mode change folds here, the download request goes to the server. + /// (improvise-2mqz) + LocalThenRequest { cmds: Vec, request: Request }, /// Bound to a command this client cannot compose. Forwarded to the host, /// which today answers nothing — `context/code/clients.md`. Upstream(KeyEvent), @@ -37,10 +41,16 @@ pub enum KeyOutcome { /// What a key may ask its **host** to do, as opposed to what it may compose. /// Narrow on purpose — the argument is in `context/code/clients.md`. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum Request { /// Write the document back where the host keeps it (improvise-368h). Save, + /// Get the document as bytes for download (improvise-2mqz). The filename + /// is what the user typed in the SavePrompt minibuffer. + DownloadDocument { filename: String }, + /// Get the active view as CSV bytes for download (improvise-2mqz). The filename + /// is what the user typed in the ExportPrompt minibuffer. + DownloadCsv { filename: String }, } impl Request { @@ -49,6 +59,8 @@ impl Request { pub fn frame(self) -> ClientFrame { match self { Request::Save => ClientFrame::Save, + Request::DownloadDocument { filename } => ClientFrame::DownloadDocument { filename }, + Request::DownloadCsv { filename } => ClientFrame::DownloadCsv { filename }, } } } @@ -92,6 +104,9 @@ pub fn resolve(state: &ClientState, event: KeyEvent) -> KeyOutcome { Binding::Cmd { name, args } => match step(state, event, name, args) { Step::Local(commands) | Step::Declined(commands) => KeyOutcome::of(commands), Step::Request(request) => KeyOutcome::Request(request), + Step::LocalThenRequest { cmds, request } => { + KeyOutcome::LocalThenRequest { cmds, request } + } Step::Upstream => KeyOutcome::Upstream(event), }, // All or nothing, each step resolved against the *unchanged* state. @@ -131,7 +146,7 @@ fn chord(state: &ClientState, event: KeyEvent, binding: Option<&Binding>) -> Key Step::Local(commands) | Step::Declined(commands) => Some(commands), // DISCARDED, which is why the guard refuses both classes rather // than counting them answered — `context/code/clients.md`. - Step::Request(_) | Step::Upstream => None, + Step::Request(_) | Step::LocalThenRequest { .. } | Step::Upstream => None, }, Some(Binding::Sequence(steps)) => sequence(state, event, steps), }; @@ -294,7 +309,7 @@ fn sequence( Step::Declined(said) => return Some(said), // A request is a frame of its own and a sequence is one frame, so // there is nowhere to put it. No binding does — clients.md. - Step::Request(_) | Step::Upstream => return None, + Step::Request(_) | Step::LocalThenRequest { .. } | Step::Upstream => return None, } } Some(commands) @@ -306,22 +321,28 @@ enum Step { Local(Vec), Declined(Vec), Request(Request), + /// Fold commands locally, THEN send a request. For commit-save/commit-export: + /// the mode change folds here, the download request goes to the server. + LocalThenRequest { cmds: Vec, request: Request }, Upstream, } /// One name, answered. Each list is checked before the next, so no name is in /// two of them and [`composed`] has no arm for any earlier one. fn step(state: &ClientState, event: KeyEvent, name: &str, args: &[String]) -> Step { - match requested(name) { - Some(request) => Step::Request(request), - None => match declined(name, args).or_else(|| refused_here(state, name)) { - Some(said) => Step::Declined(vec![Command::SetStatus(said)]), - None => match committed(state, name, args) { - Some(answer) => answer, - None => match composed(state, event, name, args) { - Some(commands) => Step::Local(commands), - None => Step::Upstream, - }, + if let Some(request) = requested(name) { + return Step::Request(request); + } + if let Some(step) = download_request(state, name) { + return step; + } + match declined(name, args).or_else(|| refused_here(state, name)) { + Some(said) => Step::Declined(vec![Command::SetStatus(said)]), + None => match committed(state, name, args) { + Some(answer) => answer, + None => match composed(state, event, name, args) { + Some(commands) => Step::Local(commands), + None => Step::Upstream, }, }, } @@ -365,6 +386,33 @@ fn requested(name: &str) -> Option { } } +/// Download requests that need to read a buffer and fold a mode change locally +/// before sending the request (improvise-2mqz). +fn download_request(state: &ClientState, name: &str) -> Option { + let (buffer_name, make_request): (&str, fn(String) -> Request) = match name { + "commit-save" => ("save", |f| Request::DownloadDocument { filename: f }), + "commit-export" => ("export", |f| Request::DownloadCsv { filename: f }), + _ => return None, + }; + let filename = read_buffer(&state.view, buffer_name).trim().to_string(); + if filename.is_empty() { + return Some(Step::Declined(vec![Command::SetStatus(format!( + "No filename given — not {}", + if name == "commit-save" { "saved" } else { "exported" } + ))])); + } + // Clear the buffer and return to Normal mode. The buffer clear is here + // rather than in the keymap sequence because LocalThenRequest cannot be + // part of a Binding::Sequence (improvise-2mqz). + Some(Step::LocalThenRequest { + cmds: vec![ + write_buffer(buffer_name, String::new()), + Command::ChangeMode(AppMode::Normal), + ], + request: make_request(filename), + }) +} + /// The commands this client cannot compose and answers **out loud**; the one /// home for the wording. Why silence was worse: clients.md (improvise-rmvx). fn declined(name: &str, args: &[String]) -> Option { @@ -681,8 +729,8 @@ fn write_buffer(name: &str, value: String) -> Command { /// to decline by name. A dead room is worse than a locked door: clients.md. fn stranding(mode: &AppMode) -> Option<&'static str> { match mode { - AppMode::SavePrompt { .. } => Some("saving to a path"), - AppMode::ExportPrompt { .. } => Some("exporting"), + // SavePrompt and ExportPrompt were stranded before improvise-2mqz added + // DownloadDocument and DownloadCsv frames. Now they have wire forms. AppMode::CommandMode { .. } => Some("the command line"), _ => None, } @@ -2661,6 +2709,9 @@ mod tests { .insert("category".into(), "Product".into()); state.view.buffers.insert("item".into(), "Shirts".into()); state.view.buffers.insert("edit".into(), "42".into()); + // commit-save and commit-export need their buffers filled (improvise-2mqz). + state.view.buffers.insert("save".into(), "data.improv".into()); + state.view.buffers.insert("export".into(), "data.csv".into()); state } @@ -2678,7 +2729,7 @@ mod tests { fn heard(state: &mut ClientState, outcome: KeyOutcome) -> Heard { let commands = match outcome { KeyOutcome::Upstream(_) | KeyOutcome::Ignored => return Heard::Silent, - KeyOutcome::Request(_) => return Heard::Acts, + KeyOutcome::Request(_) | KeyOutcome::LocalThenRequest { .. } => return Heard::Acts, KeyOutcome::Local(commands) => commands, }; let quiet = |view: &ViewState| ViewState { @@ -2826,8 +2877,8 @@ mod tests { // Reached only from a mode `declined` refuses at the door, so a // browser is never standing in front of these. "COMMAND: Enter execute-command Silent", - "EXPORT: Enter commit-export Silent", - "SAVE AS: Enter commit-save Silent", + // commit-export and commit-save removed: improvise-2mqz added wire + // forms (DownloadCsv, DownloadDocument), so they now ACT. // Honestly unavailable, and saying so in `declined`'s words. "NORMAL: > drill-into-cell Declines", "NORMAL: : enter-mode Declines", @@ -3484,22 +3535,13 @@ mod tests { /// The refusal above is only half the property: the bug guarded is /// *entering a room with no exit*. See `context/test-evidence/clients.md`. + /// Note: SavePrompt and ExportPrompt are no longer refused after improvise-2mqz + /// added DownloadDocument and DownloadCsv frames. #[test] fn a_prompt_this_client_cannot_commit_is_never_entered() { let state = in_mode(AppMode::Normal); - for (key, mode, subject) in [ - ( - KeyEvent::new(KeyCode::Char('w'), KeyModifiers::CONTROL), - "save-prompt", - "saving", - ), - ( - KeyEvent::new(KeyCode::Char('e'), KeyModifiers::CONTROL), - "export-prompt", - "exporting", - ), - (ev(KeyCode::Char(':')), "command-mode", "command line"), - ] { + // Only CommandMode is still stranded (improvise-2mqz removed SavePrompt/ExportPrompt) + for (key, mode, subject) in [(ev(KeyCode::Char(':')), "command-mode", "command line")] { let KeyOutcome::Local(said) = resolve(&state, key) else { panic!("{mode} must be refused *out loud*, not in silence"); }; @@ -3516,6 +3558,21 @@ mod tests { ); } + // SavePrompt and ExportPrompt ARE now enterable (improvise-2mqz). + for (key, mode_name) in [ + (KeyEvent::new(KeyCode::Char('w'), KeyModifiers::CONTROL), "save-prompt"), + (KeyEvent::new(KeyCode::Char('e'), KeyModifiers::CONTROL), "export-prompt"), + ] { + let outcome = resolve(&state, key); + let KeyOutcome::Local(cmds) = outcome else { + panic!("{mode_name} should resolve locally now that it has a wire form"); + }; + assert!( + cmds.iter().any(|c| matches!(c, Command::ChangeMode(_))), + "{mode_name} should be entered now that commit-save/commit-export have wire forms" + ); + } + // Negative arm, half one: a mode that is not refused is still entered. // Why `?` and not `:` — `context/test-evidence/clients.md`. assert_eq!( diff --git a/crates/improvise-wasm-client/src/lib.rs b/crates/improvise-wasm-client/src/lib.rs index 88ae7c0e..60323736 100644 --- a/crates/improvise-wasm-client/src/lib.rs +++ b/crates/improvise-wasm-client/src/lib.rs @@ -8,7 +8,7 @@ pub mod socket; pub use improvise_protocol::{Click, ClientState, KeyOutcome, Request, resolve}; pub use keys::BrowserKey; pub use render::{RenderCatRow, RenderCell, RenderModel, RenderRow}; -pub use socket::EditTarget; +pub use socket::{DownloadKind, EditTarget, PendingDownload}; use improvise_protocol::{CellId, ClientFrame, RecordCell, Snapshot}; use wasm_bindgen::prelude::*; @@ -18,6 +18,8 @@ use wasm_bindgen::prelude::*; #[derive(Debug, Default)] pub struct Client { state: ClientState, + /// A download waiting for JavaScript to trigger (improvise-2mqz). + pending_download: Option, } /// An empty view slice and one empty cache per widget. Not exported: the @@ -92,6 +94,17 @@ impl Client { .map_err(|e| JsError::new(&e.to_string())) } + /// Take any pending download, returning it as JSON for JavaScript to handle. + /// Returns `None` if there is no pending download (improvise-2mqz). + #[wasm_bindgen] + pub fn take_pending_download(&mut self) -> Result, JsError> { + self.pending_download + .take() + .map(|d| serde_json::to_string(&d)) + .transpose() + .map_err(|e| JsError::new(&e.to_string())) + } + /// Everything the page draws, with every rendering decision already /// made. See [`render`]. #[wasm_bindgen] @@ -243,6 +256,12 @@ impl Client { // Nothing to fold: a request moves no state of this client's, and // what it moves on the host comes back as a projection. KeyOutcome::Request(request) => Some(request.frame()), + // Fold the commands locally (mode change), then send the request + // for download bytes (improvise-2mqz). + KeyOutcome::LocalThenRequest { cmds, request } => { + self.state.fold_outgoing(&cmds); + Some(request.frame()) + } KeyOutcome::Upstream(event) => Some(ClientFrame::key(event)), KeyOutcome::Ignored => None, } diff --git a/crates/improvise-wasm-client/src/socket.rs b/crates/improvise-wasm-client/src/socket.rs index 611d8f19..b15126e4 100644 --- a/crates/improvise-wasm-client/src/socket.rs +++ b/crates/improvise-wasm-client/src/socket.rs @@ -8,6 +8,25 @@ use improvise_protocol::{ }; use serde::{Deserialize, Serialize}; +/// A download the browser needs to trigger (improvise-2mqz). Stored after a +/// `DocumentBytes` or `CsvBytes` frame arrives; retrieved by JavaScript via +/// `Client::take_pending_download`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct PendingDownload { + pub filename: String, + pub bytes: Vec, + pub kind: DownloadKind, +} + +/// What kind of content is being downloaded. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum DownloadKind { + /// The document in `.improv` format. + Document, + /// The active view as CSV. + Csv, +} + /// Everything an editor captures when it opens, held opaquely by the page. /// A struct so a field such as the seed can join it — `context/code/clients.md`. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -72,6 +91,7 @@ fn rejection_message(rejection: &Rejection) -> String { "this host has nowhere to write the document — it was not saved".into() } Rejection::SaveFailed => "the document could not be written".into(), + Rejection::ExportFailed => "the view could not be exported".into(), } } @@ -82,7 +102,9 @@ impl Client { ClientFrame::Viewport { rows, cols } } - /// Fold one server frame, in arrival order. + /// Fold one server frame, in arrival order. Returns the new revision for + /// most frames; for download frames, returns the revision unchanged (no + /// state to fold) and stores the download for JavaScript to retrieve. pub fn fold_frame(&mut self, frame: ServerFrame) -> u64 { match frame { ServerFrame::Snapshot(snapshot) => self.bootstrap(*snapshot), @@ -100,6 +122,22 @@ impl Client { state.notify(message); } ServerFrame::Accepted => self.state_mut().accepted(), + // Download frames carry bytes the browser needs to trigger a download. + // Store them for JavaScript to retrieve via `take_pending_download`. + ServerFrame::DocumentBytes { filename, bytes } => { + self.pending_download = Some(PendingDownload { + filename, + bytes, + kind: DownloadKind::Document, + }); + } + ServerFrame::CsvBytes { filename, bytes } => { + self.pending_download = Some(PendingDownload { + filename, + bytes, + kind: DownloadKind::Csv, + }); + } } self.revision() } @@ -223,7 +261,7 @@ mod tests { } /// A rejection that refused an **edit** must say nothing was applied; the - /// two `Save` answers refused none. Evidence: clients.md (improvise-8uj8). + /// save/export answers refused none. Evidence: clients.md (improvise-8uj8). #[test] fn every_rejection_message_says_what_it_refused() { let hash = surrogate("fe4c8e5e9cbd578e0d388f4b2b5a7879"); @@ -238,9 +276,11 @@ mod tests { // Not "not applied": nothing was asked to apply. It must still // say the thing it *did* refuse did not happen. false => assert!( - msg.contains("not saved") || msg.contains("could not be written"), - "{rejection:?} refused a save but is worded as {msg:?}, \ - which does not say the document was not written" + msg.contains("not saved") + || msg.contains("could not be written") + || msg.contains("could not be exported"), + "{rejection:?} refused a save/export but is worded as {msg:?}, \ + which does not say the operation failed" ), } } diff --git a/crates/improvise-wasm-client/tests/lockstep.rs b/crates/improvise-wasm-client/tests/lockstep.rs index 93c2d789..acc90118 100644 --- a/crates/improvise-wasm-client/tests/lockstep.rs +++ b/crates/improvise-wasm-client/tests/lockstep.rs @@ -1595,6 +1595,9 @@ fn deliver( } improvise_protocol::ServerFrame::Rejected(r) => client.refused(r), improvise_protocol::ServerFrame::Accepted => client.accepted(), + // Download responses are not processed in this test context (improvise-2mqz). + improvise_protocol::ServerFrame::DocumentBytes { .. } + | improvise_protocol::ServerFrame::CsvBytes { .. } => {} } } } @@ -3731,6 +3734,9 @@ fn heard( } improvise_protocol::ServerFrame::Rejected(r) => client.refused(r), improvise_protocol::ServerFrame::Accepted => client.accepted(), + // Download responses are not processed in this test context (improvise-2mqz). + improvise_protocol::ServerFrame::DocumentBytes { .. } + | improvise_protocol::ServerFrame::CsvBytes { .. } => {} } } } diff --git a/crates/improvise-worker-server/src/worker.rs b/crates/improvise-worker-server/src/worker.rs index cafd0607..2f64889e 100644 --- a/crates/improvise-worker-server/src/worker.rs +++ b/crates/improvise-worker-server/src/worker.rs @@ -256,6 +256,8 @@ mod tests { } } ServerFrame::Rejected(_) | ServerFrame::Accepted => {} + // Download responses are not processed in this test context (improvise-2mqz). + ServerFrame::DocumentBytes { .. } | ServerFrame::CsvBytes { .. } => {} } } grid.clone() diff --git a/crates/improvise-worker-server/tests/post_message.rs b/crates/improvise-worker-server/tests/post_message.rs index db6832d6..44ebfa65 100644 --- a/crates/improvise-worker-server/tests/post_message.rs +++ b/crates/improvise-worker-server/tests/post_message.rs @@ -113,6 +113,8 @@ impl MainThread { } } ServerFrame::Rejected(_) | ServerFrame::Accepted => {} + // Download responses are not processed in this test context (improvise-2mqz). + ServerFrame::DocumentBytes { .. } | ServerFrame::CsvBytes { .. } => {} } } diff --git a/web/app.js b/web/app.js index 5de93091..4236c026 100644 --- a/web/app.js +++ b/web/app.js @@ -155,9 +155,35 @@ function onFrame(text) { console.error("improvise: frame rejected", text, err); return; } + // Check for pending downloads (improvise-2mqz). The WASM client stores + // download bytes from DocumentBytes/CsvBytes frames; we trigger the download + // here via Blob + . + try { + const downloadJson = client.take_pending_download(); + if (downloadJson) { + const download = JSON.parse(downloadJson); + triggerDownload(download.filename, download.bytes, download.kind); + } + } catch (err) { + console.error("improvise: download failed", err); + } repaint(); } +/** Trigger a browser download from bytes (improvise-2mqz). */ +function triggerDownload(filename, bytes, kind) { + const mimeType = kind === "Document" ? "text/plain" : "text/csv"; + const blob = new Blob([new Uint8Array(bytes)], { type: mimeType }); + const url = URL.createObjectURL(blob); + const a = document.createElement("a"); + a.href = url; + a.download = filename; + document.body.appendChild(a); + a.click(); + document.body.removeChild(a); + URL.revokeObjectURL(url); +} + // Transports. /** The in-page worker: the standalone deployment, no server process. */ From b872923331e3ea0bcafbe94a26a7eb01ee29a9d7 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 27 Sep 2026 16:51:50 +0000 Subject: [PATCH 2/3] fix(improvise-2mqz): export the requesting session view Co-authored-by: Ed Langley --- .../src/command/cmd/commit.rs | 12 +-- crates/improvise-command/src/ui/host.rs | 81 ++++++++++++++++-- .../src/ui/reduce/vocabulary_is_complete.rs | 14 +--- crates/improvise-io/src/persistence/export.rs | 5 +- crates/improvise-protocol/src/frame.rs | 28 +++---- crates/improvise-protocol/src/keymap.rs | 6 +- crates/improvise-protocol/src/resolve.rs | 84 +++++++++++-------- crates/improvise-wasm-client/src/socket.rs | 10 +-- .../tests/post_message.rs | 12 ++- web/tests/bundle/wasm_surface.test.js | 3 + 10 files changed, 162 insertions(+), 93 deletions(-) diff --git a/crates/improvise-command/src/command/cmd/commit.rs b/crates/improvise-command/src/command/cmd/commit.rs index 59f88a70..7ed8d269 100644 --- a/crates/improvise-command/src/command/cmd/commit.rs +++ b/crates/improvise-command/src/command/cmd/commit.rs @@ -1275,10 +1275,8 @@ impl Cmd for CommitSave { effect::change_mode(AppMode::Normal), ]; } - // The mode change and buffer clear come **first**, or a failed `SaveAs` - // aborts with the minibuffer still drawn over its error (Copilot, PR #27). - // Buffer clear added for improvise-2mqz: the keymap uses rebind instead of - // commit (because LocalThenRequest can't be in a Binding::Sequence). + // Clear before `SaveAs`, whose failure aborts; this command owns the clear + // because its LocalThenRequest cannot live in a sequence (improvise-2mqz). vec![ Box::new(effect::SetBuffer { name: "save".into(), @@ -1305,10 +1303,8 @@ impl Cmd for CommitExport { effect::change_mode(AppMode::Normal), ]; } - // The mode change and buffer clear come **first**, or a failed export - // aborts with the minibuffer still drawn over its error (gotcha 21). - // Buffer clear added for improvise-2mqz: the keymap uses rebind instead of - // commit (because LocalThenRequest can't be in a Binding::Sequence). + // Clear before export, whose failure aborts; this command owns the clear + // because its LocalThenRequest cannot live in a sequence (improvise-2mqz). vec![ Box::new(effect::SetBuffer { name: "export".into(), diff --git a/crates/improvise-command/src/ui/host.rs b/crates/improvise-command/src/ui/host.rs index 42ac5193..7d83787e 100644 --- a/crates/improvise-command/src/ui/host.rs +++ b/crates/improvise-command/src/ui/host.rs @@ -118,19 +118,25 @@ impl Host { /// Serialize the document to `.improv` format and return the bytes for download /// (improvise-2mqz). Silent on success, Rejected on failure; no session state moves. fn download_document(&mut self, id: SessionId, filename: String) -> Vec { - let bytes = improvise_io::persistence::format_md(&self.app.model_state.workbook) - .into_bytes(); + let bytes = + improvise_io::persistence::format_md(&self.app.model_state.workbook).into_bytes(); vec![Addressed { to: id, msg: ServerFrame::DocumentBytes { filename, bytes }, }] } - /// Render the active view as CSV and return the bytes for download - /// (improvise-2mqz). Silent on success, Rejected on failure; no session state moves. + /// Render the requesting session's view as downloadable CSV bytes. + /// Silent on success, Rejected on failure (improvise-2mqz). fn download_csv(&mut self, id: SessionId, filename: String) -> Vec { - let view_name = &self.app.model_state.workbook.active_view; - match improvise_io::persistence::render_csv(&self.app.model_state.workbook, view_name) { + let Some(view_name) = self + .sessions + .get(id) + .map(|session| session.view().active_view.clone()) + else { + return Vec::new(); + }; + match improvise_io::persistence::render_csv(&self.app.model_state.workbook, &view_name) { Ok(bytes) => vec![Addressed { to: id, msg: ServerFrame::CsvBytes { filename, bytes }, @@ -464,9 +470,8 @@ mod tests { } } } - // Neither answer carries state: one applied nothing, and what - // the other applied arrived as projections before it. - // DocumentBytes and CsvBytes are for downloads and carry no state. + // These carry no foldable state: downloads are consumed by the + // browser, while other results arrived as projections or did nothing. ServerFrame::Rejected(_) | ServerFrame::Accepted | ServerFrame::DocumentBytes { .. } @@ -2746,6 +2751,64 @@ mod tests { ); } + /// improvise-2mqz: view selection belongs to a session, so an export must + /// not silently use the document's default view or another client's view. + #[test] + fn a_csv_download_exports_the_requesting_sessions_active_view() { + let mut host = host(); + let second = host.app.model_state.workbook.create_view("Second"); + second.set_axis("Region", Axis::Row); + second.set_axis("_Measure", Axis::Column); + second.hide_item("Region", "South"); + let (requester, _) = host.connect(10, 10); + let (bystander, _) = host.connect(10, 10); + + host.handle( + requester, + ClientFrame::command(Command::SwitchView("Second".into())), + ); + assert_eq!( + host.sessions().get(requester).unwrap().view().active_view, + "Second", + "setup: the requester must be on its session-local view" + ); + assert_eq!( + host.sessions().get(bystander).unwrap().view().active_view, + "Default", + "setup: the second session must remain on the default view" + ); + assert_eq!( + host.app().model_state.workbook.active_view, + "Default", + "setup: the document's active view must differ from the requester" + ); + + let answer = host.handle( + requester, + ClientFrame::DownloadCsv { + filename: "second.csv".into(), + }, + ); + let expected = + improvise_io::persistence::render_csv(&host.app().model_state.workbook, "Second") + .unwrap(); + let default = + improvise_io::persistence::render_csv(&host.app().model_state.workbook, "Default") + .unwrap(); + assert_ne!(expected, default, "setup: the two exports must differ"); + assert_eq!( + to(&answer, requester), + vec![ServerFrame::CsvBytes { + filename: "second.csv".into(), + bytes: expected, + }] + ); + assert!( + to(&answer, bystander).is_empty(), + "the download is owed only to its requester: {answer:?}" + ); + } + // ── disconnect ────────────────────────────────────────────────────── /// AC 4: a dropped client stops being projected to, and a late frame diff --git a/crates/improvise-command/src/ui/reduce/vocabulary_is_complete.rs b/crates/improvise-command/src/ui/reduce/vocabulary_is_complete.rs index c08c668d..473825f7 100644 --- a/crates/improvise-command/src/ui/reduce/vocabulary_is_complete.rs +++ b/crates/improvise-command/src/ui/reduce/vocabulary_is_complete.rs @@ -16,8 +16,6 @@ enum Verdict { Frame(&'static str), /// Deliberately off the wire, and the argument. Off(&'static str), - /// Missing, and the ticket that owns closing it. - Gap(&'static str), } /// Checked in all directions: an entry must name a real effect, must not be on @@ -29,14 +27,8 @@ const NOT_ON_THE_WIRE: &[(&str, Verdict)] = &[ // Its text; the refusal flag is one host batch's outcome (context/code/cli.md §10). ("Refuse", Verdict::Command("SetStatus")), ("Save", Verdict::Frame("Save")), - ( - "SaveAs", - Verdict::Frame("DownloadDocument"), - ), - ( - "ExportCsv", - Verdict::Frame("DownloadCsv"), - ), + ("SaveAs", Verdict::Frame("DownloadDocument")), + ("ExportCsv", Verdict::Frame("DownloadCsv")), ( "ApplyAndClearDrill", Verdict::Off("operand is one session's staged drill edits (gotcha 38, improvise-3x3u)"), @@ -134,7 +126,7 @@ fn the_verdict_list_names_only_real_effects_and_real_variants() { frames.iter().any(|f| f == variant), "{name}'s verdict names ClientFrame::{variant}, which does not exist" ), - Verdict::Off(why) | Verdict::Gap(why) => { + Verdict::Off(why) => { assert!(!why.is_empty(), "{name}'s verdict gives no argument") } } diff --git a/crates/improvise-io/src/persistence/export.rs b/crates/improvise-io/src/persistence/export.rs index 9269f6fb..44f4a68d 100644 --- a/crates/improvise-io/src/persistence/export.rs +++ b/crates/improvise-io/src/persistence/export.rs @@ -22,9 +22,8 @@ pub fn export_csv_to( storage.write(path, &render_csv(workbook, view_name)?) } -/// Render a view as CSV bytes. Separated from writing so the bytes can go to -/// any [`Storage`] backend, not just a file (improvise-fuj). Public for the -/// download frame handler (improvise-2mqz). +/// Render a view as CSV bytes for any [`Storage`] backend or a download frame +/// (improvise-fuj, improvise-2mqz). pub fn render_csv(workbook: &Workbook, view_name: &str) -> Result> { let view = workbook .views diff --git a/crates/improvise-protocol/src/frame.rs b/crates/improvise-protocol/src/frame.rs index e1a89863..29cfc183 100644 --- a/crates/improvise-protocol/src/frame.rs +++ b/crates/improvise-protocol/src/frame.rs @@ -23,13 +23,11 @@ pub enum ClientFrame { code: KeyCode, modifiers: KeyModifiers, }, - /// Ask for the document serialized as bytes. The browser downloads these - /// via Blob + ``. The filename is what the user typed in the - /// SavePrompt minibuffer — improvise-2mqz. + /// Ask for document bytes named by SavePrompt; the browser downloads them + /// via Blob + `` (improvise-2mqz). DownloadDocument { filename: String }, - /// Ask for the active view as CSV bytes. The browser downloads these - /// via Blob + ``. The filename is what the user typed in the - /// ExportPrompt minibuffer — improvise-2mqz. + /// Ask for active-view CSV bytes named by ExportPrompt; the browser downloads + /// them via Blob + `` (improvise-2mqz). DownloadCsv { filename: String }, } @@ -63,13 +61,11 @@ pub enum ServerFrame { /// The asker's mutating frame landed whole. Carries no state: everything /// it moved arrived as projections first — `context/code/protocol.md` §4. Accepted, - /// The document serialized as `.improv` format (improvise-2mqz). Addressed - /// to the asker alone; the browser turns these bytes into a download. - /// The filename is echoed from the request. + /// Downloadable `.improv` bytes addressed to the asker alone; the filename + /// is echoed from its request (improvise-2mqz). DocumentBytes { filename: String, bytes: Vec }, - /// The active view rendered as CSV (improvise-2mqz). Addressed to the - /// asker alone; the browser turns these bytes into a download. - /// The filename is echoed from the request. + /// Downloadable active-view CSV bytes addressed to the asker alone; the + /// filename is echoed from its request (improvise-2mqz). CsvBytes { filename: String, bytes: Vec }, } @@ -99,8 +95,12 @@ mod tests { )), ClientFrame::key(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)), ClientFrame::Save, - ClientFrame::DownloadDocument { filename: "doc.improv".into() }, - ClientFrame::DownloadCsv { filename: "export.csv".into() }, + ClientFrame::DownloadDocument { + filename: "doc.improv".into(), + }, + ClientFrame::DownloadCsv { + filename: "export.csv".into(), + }, ] { let json = serde_json::to_string(&frame).expect("serializes"); assert_eq!( diff --git a/crates/improvise-protocol/src/keymap.rs b/crates/improvise-protocol/src/keymap.rs index 65788f31..4bd2e2d9 100644 --- a/crates/improvise-protocol/src/keymap.rs +++ b/crates/improvise-protocol/src/keymap.rs @@ -927,10 +927,8 @@ impl Keymaps { ), ); - // ExportPrompt and SavePrompt use rebind instead of commit because - // commit-save and commit-export return LocalThenRequest (improvise-2mqz), - // which cannot be part of a Binding::Sequence. The buffer clear is - // handled inside download_request instead. + // LocalThenRequest cannot live in a sequence, so these commands own + // their buffer clears and are rebound directly (improvise-2mqz). set.insert( ModeKey::ExportPrompt, Arc::new( diff --git a/crates/improvise-protocol/src/resolve.rs b/crates/improvise-protocol/src/resolve.rs index 6af95053..a615b5fd 100644 --- a/crates/improvise-protocol/src/resolve.rs +++ b/crates/improvise-protocol/src/resolve.rs @@ -27,10 +27,12 @@ pub enum KeyOutcome { /// Resolved here as a **request**: the caller sends the frame and folds /// nothing. Why it is not a `Command`: `context/code/clients.md`. Request(Request), - /// Fold commands locally, THEN send a request. For commit-save/commit-export: - /// the mode change folds here, the download request goes to the server. - /// (improvise-2mqz) - LocalThenRequest { cmds: Vec, request: Request }, + /// Fold commands locally, then send a request: mode changes here and + /// download bytes come from the server (improvise-2mqz). + LocalThenRequest { + cmds: Vec, + request: Request, + }, /// Bound to a command this client cannot compose. Forwarded to the host, /// which today answers nothing — `context/code/clients.md`. Upstream(KeyEvent), @@ -323,7 +325,10 @@ enum Step { Request(Request), /// Fold commands locally, THEN send a request. For commit-save/commit-export: /// the mode change folds here, the download request goes to the server. - LocalThenRequest { cmds: Vec, request: Request }, + LocalThenRequest { + cmds: Vec, + request: Request, + }, Upstream, } @@ -398,11 +403,14 @@ fn download_request(state: &ClientState, name: &str) -> Option { if filename.is_empty() { return Some(Step::Declined(vec![Command::SetStatus(format!( "No filename given — not {}", - if name == "commit-save" { "saved" } else { "exported" } + if name == "commit-save" { + "saved" + } else { + "exported" + } ))])); } - // Clear the buffer and return to Normal mode. The buffer clear is here - // rather than in the keymap sequence because LocalThenRequest cannot be + // This owns the clear and mode change because LocalThenRequest cannot be // part of a Binding::Sequence (improvise-2mqz). Some(Step::LocalThenRequest { cmds: vec![ @@ -2710,8 +2718,14 @@ mod tests { state.view.buffers.insert("item".into(), "Shirts".into()); state.view.buffers.insert("edit".into(), "42".into()); // commit-save and commit-export need their buffers filled (improvise-2mqz). - state.view.buffers.insert("save".into(), "data.improv".into()); - state.view.buffers.insert("export".into(), "data.csv".into()); + state + .view + .buffers + .insert("save".into(), "data.improv".into()); + state + .view + .buffers + .insert("export".into(), "data.csv".into()); state } @@ -2877,8 +2891,6 @@ mod tests { // Reached only from a mode `declined` refuses at the door, so a // browser is never standing in front of these. "COMMAND: Enter execute-command Silent", - // commit-export and commit-save removed: improvise-2mqz added wire - // forms (DownloadCsv, DownloadDocument), so they now ACT. // Honestly unavailable, and saying so in `declined`'s words. "NORMAL: > drill-into-cell Declines", "NORMAL: : enter-mode Declines", @@ -3533,35 +3545,37 @@ mod tests { ); } - /// The refusal above is only half the property: the bug guarded is - /// *entering a room with no exit*. See `context/test-evidence/clients.md`. - /// Note: SavePrompt and ExportPrompt are no longer refused after improvise-2mqz - /// added DownloadDocument and DownloadCsv frames. + /// A client must not enter a room with no exit. SavePrompt and ExportPrompt + /// now have download frames; see `context/test-evidence/clients.md`. #[test] fn a_prompt_this_client_cannot_commit_is_never_entered() { let state = in_mode(AppMode::Normal); - // Only CommandMode is still stranded (improvise-2mqz removed SavePrompt/ExportPrompt) - for (key, mode, subject) in [(ev(KeyCode::Char(':')), "command-mode", "command line")] { - let KeyOutcome::Local(said) = resolve(&state, key) else { - panic!("{mode} must be refused *out loud*, not in silence"); - }; - let [Command::SetStatus(msg)] = &said[..] else { - panic!("{mode}: expected one status, got {said:?}"); - }; - assert!( - msg.contains(subject), - "{mode}: the refusal must name its own subject, got {msg}" - ); - assert!( - !said.iter().any(|c| matches!(c, Command::ChangeMode(_))), - "{mode} must not be entered by a client that cannot commit it" - ); - } + let mode = "command-mode"; + let KeyOutcome::Local(said) = resolve(&state, ev(KeyCode::Char(':'))) else { + panic!("{mode} must be refused *out loud*, not in silence"); + }; + let [Command::SetStatus(msg)] = &said[..] else { + panic!("{mode}: expected one status, got {said:?}"); + }; + assert!( + msg.contains("command line"), + "{mode}: the refusal must name its own subject, got {msg}" + ); + assert!( + !said.iter().any(|c| matches!(c, Command::ChangeMode(_))), + "{mode} must not be entered by a client that cannot commit it" + ); // SavePrompt and ExportPrompt ARE now enterable (improvise-2mqz). for (key, mode_name) in [ - (KeyEvent::new(KeyCode::Char('w'), KeyModifiers::CONTROL), "save-prompt"), - (KeyEvent::new(KeyCode::Char('e'), KeyModifiers::CONTROL), "export-prompt"), + ( + KeyEvent::new(KeyCode::Char('w'), KeyModifiers::CONTROL), + "save-prompt", + ), + ( + KeyEvent::new(KeyCode::Char('e'), KeyModifiers::CONTROL), + "export-prompt", + ), ] { let outcome = resolve(&state, key); let KeyOutcome::Local(cmds) = outcome else { diff --git a/crates/improvise-wasm-client/src/socket.rs b/crates/improvise-wasm-client/src/socket.rs index b15126e4..8070d307 100644 --- a/crates/improvise-wasm-client/src/socket.rs +++ b/crates/improvise-wasm-client/src/socket.rs @@ -8,9 +8,8 @@ use improvise_protocol::{ }; use serde::{Deserialize, Serialize}; -/// A download the browser needs to trigger (improvise-2mqz). Stored after a -/// `DocumentBytes` or `CsvBytes` frame arrives; retrieved by JavaScript via -/// `Client::take_pending_download`. +/// A `DocumentBytes` or `CsvBytes` download held until JavaScript retrieves it +/// through `Client::take_pending_download` (improvise-2mqz). #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct PendingDownload { pub filename: String, @@ -102,9 +101,8 @@ impl Client { ClientFrame::Viewport { rows, cols } } - /// Fold one server frame, in arrival order. Returns the new revision for - /// most frames; for download frames, returns the revision unchanged (no - /// state to fold) and stores the download for JavaScript to retrieve. + /// Fold one server frame in order. Downloads leave the revision unchanged + /// and wait for JavaScript; other frames return the new revision. pub fn fold_frame(&mut self, frame: ServerFrame) -> u64 { match frame { ServerFrame::Snapshot(snapshot) => self.bootstrap(*snapshot), diff --git a/crates/improvise-worker-server/tests/post_message.rs b/crates/improvise-worker-server/tests/post_message.rs index 44ebfa65..288570b4 100644 --- a/crates/improvise-worker-server/tests/post_message.rs +++ b/crates/improvise-worker-server/tests/post_message.rs @@ -353,11 +353,17 @@ fn the_shipped_demo_seeds_a_worker_whose_raw_cells_are_settable() { } // The third column is `Profit`, which a formula owns: the page must not // offer it, so it carries no surrogate to offer (improvise-ir7l). - for row in 0..main.grid.row_frame.len() { + for (row, cells) in main + .grid + .cells + .iter() + .enumerate() + .take(main.grid.row_frame.len()) + { assert!( - matches!(main.grid.cells[row][2].id, Some(CellId::Derived { .. })), + matches!(cells[2].id, Some(CellId::Derived { .. })), "row {row}'s Profit cell must be Derived, got {:?}", - main.grid.cells[row][2].id + cells[2].id ); } diff --git a/web/tests/bundle/wasm_surface.test.js b/web/tests/bundle/wasm_surface.test.js index a9f8550f..7f5e9f0a 100644 --- a/web/tests/bundle/wasm_surface.test.js +++ b/web/tests/bundle/wasm_surface.test.js @@ -91,6 +91,9 @@ test("the page still reaches the client through a handful of calls", () => { "on_key_event", "render_model", "set_viewport", + // improvise-2mqz: download frames stay in the Rust fold until the page + // takes the bytes and hands them to Blob + . + "take_pending_download", "typed_cell", ], ); From c355e5e8447e99130c399463bdaddd49b7fe2a2c Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 27 Sep 2026 17:49:15 +0000 Subject: [PATCH 3/3] fix(improvise-2mqz): keep download sessions in lockstep Co-authored-by: Ed Langley --- context/test-evidence/clients.md | 16 +++ crates/improvise-command/src/ui/host.rs | 46 +++++++-- .../tests/browser_round_trip.rs | 97 +++++++++++++++++++ 3 files changed, 150 insertions(+), 9 deletions(-) diff --git a/context/test-evidence/clients.md b/context/test-evidence/clients.md index f6b506a5..e1df39d3 100644 --- a/context/test-evidence/clients.md +++ b/context/test-evidence/clients.md @@ -6605,3 +6605,19 @@ commits. `target.view`), run across the whole workspace: fails **only** this test. - A composer that hard-codes `"Default"` fails here as well. Neither view in this test is `Default`, so the default cannot satisfy it by accident. + +### a_download_commit_keeps_the_host_session_mirror_in_lockstep + +Codex on PR #329: `LocalThenRequest` closed and cleared the browser's download +prompt, but the bytes-only request moved no corresponding state in the host's +session mirror. The test opens SavePrompt through the real browser key path, +types a nonempty filename, and commits it. It asserts both that the response is +still `DocumentBytes` and that the host mirror is Normal with an empty save +buffer. Another session then writes a cell; the requester folds that later +projection and must still equal its host mirror. + +- **Rival** (the pre-fix download handlers, which returned bytes without + closing the mirrored prompt): fails at the host-mode assertion with + `SavePrompt` versus `Normal`, before the later projection. +- The nonempty filename proves Enter commits rather than taking the empty-name + refusal, and matching `DocumentBytes` preserves the bytes-download ruling. diff --git a/crates/improvise-command/src/ui/host.rs b/crates/improvise-command/src/ui/host.rs index 7d83787e..b9434734 100644 --- a/crates/improvise-command/src/ui/host.rs +++ b/crates/improvise-command/src/ui/host.rs @@ -5,7 +5,7 @@ use crate::ui::app::App; use crate::ui::effect; use crate::ui::project::{Applied, Broadcast, Dirty, SessionId, Sessions}; use crate::ui::reduce::{self, Checked, CheckedBatch}; -use improvise_protocol::{Command, Rejection}; +use improvise_protocol::{AppMode, Command, Rejection}; use improvise_protocol::{ClientFrame, ServerFrame}; @@ -116,18 +116,22 @@ impl Host { } /// Serialize the document to `.improv` format and return the bytes for download - /// (improvise-2mqz). Silent on success, Rejected on failure; no session state moves. + /// after closing the prompt in the session mirror (improvise-2mqz). fn download_document(&mut self, id: SessionId, filename: String) -> Vec { + let Some(mut answer) = self.close_download_prompt(id, "save") else { + return Vec::new(); + }; let bytes = improvise_io::persistence::format_md(&self.app.model_state.workbook).into_bytes(); - vec![Addressed { + answer.push(Addressed { to: id, msg: ServerFrame::DocumentBytes { filename, bytes }, - }] + }); + answer } /// Render the requesting session's view as downloadable CSV bytes. - /// Silent on success, Rejected on failure (improvise-2mqz). + /// Close its prompt mirror first; reject only if rendering fails (improvise-2mqz). fn download_csv(&mut self, id: SessionId, filename: String) -> Vec { let Some(view_name) = self .sessions @@ -136,16 +140,40 @@ impl Host { else { return Vec::new(); }; + let Some(mut answer) = self.close_download_prompt(id, "export") else { + return Vec::new(); + }; match improvise_io::persistence::render_csv(&self.app.model_state.workbook, &view_name) { - Ok(bytes) => vec![Addressed { + Ok(bytes) => answer.push(Addressed { to: id, msg: ServerFrame::CsvBytes { filename, bytes }, - }], - Err(_) => vec![Addressed { + }), + Err(_) => answer.push(Addressed { to: id, msg: ServerFrame::Rejected(Rejection::ExportFailed), - }], + }), } + answer + } + + /// The browser folded these before sending the download request; mirror + /// that fold so a later projection cannot restore the prompt. + fn close_download_prompt( + &mut self, + id: SessionId, + buffer: &'static str, + ) -> Option> { + self.sessions.get(id)?; + Some(self.commands( + id, + &[ + Command::SetBuffer { + name: buffer.into(), + value: String::new(), + }, + Command::ChangeMode(AppMode::Normal), + ], + )) } /// Fan out whatever moved, for a host that changed the document outside diff --git a/crates/improvise-wasm-client/tests/browser_round_trip.rs b/crates/improvise-wasm-client/tests/browser_round_trip.rs index d76943a7..0102a811 100644 --- a/crates/improvise-wasm-client/tests/browser_round_trip.rs +++ b/crates/improvise-wasm-client/tests/browser_round_trip.rs @@ -621,6 +621,103 @@ fn ctrl_s_in_the_browser_writes_the_hosts_document() { ); } +/// Codex on PR #329: committing a download closed the browser's prompt but +/// left the host mirror in it, so a later projection could restore stale state. +#[test] +fn a_download_commit_keeps_the_host_session_mirror_in_lockstep() { + use improvise_command::ui::reduce::caught_up; + use improvise_protocol::{AppMode, ModelCommand, ServerFrame}; + + let mut host = host(); + let mut client = Client::new(); + let (id, snapshot) = host.connect(10, 10); + deliver(&mut client, &[snapshot]); + + let open = client + .key(&BrowserKey { + key: "w".into(), + ctrl: true, + alt: false, + shift: false, + meta: false, + }) + .expect("Ctrl-W opens the save prompt"); + let opened = host.handle(id, open); + deliver(&mut client, &opened); + for key in ["d", "o", "c", ".", "i", "m", "p", "r", "o", "v"] { + press(&mut host, id, &mut client, key); + } + assert!(matches!( + client.state().view.mode, + AppMode::SavePrompt { .. } + )); + assert_eq!( + host.sessions().get(id).expect("attached").view().mode, + client.state().view.mode, + "setup: both ends must be in the prompt before Enter" + ); + + let commit = client + .key(&BrowserKey::new("Enter")) + .expect("Enter composes a download request"); + assert!( + matches!( + &commit, + ClientFrame::DownloadDocument { filename } if filename == "doc.improv" + ), + "the USER RULING is a bytes request, got {commit:?}" + ); + let answer = host.handle(id, commit); + assert!( + answer + .iter() + .any(|frame| matches!(frame.msg, ServerFrame::DocumentBytes { .. })), + "the request must still return bytes: {answer:?}" + ); + deliver(&mut client, &answer); + + let mirrored = host.sessions().get(id).expect("attached").view(); + assert_eq!( + mirrored.mode, + AppMode::Normal, + "the host must close the prompt in its session mirror" + ); + assert_eq!( + mirrored.buffers.get("save").map(String::as_str), + Some(""), + "the host must clear the same minibuffer as the client" + ); + + // Force a subsequent document projection from another session, then fold + // what it addresses to the requester. + let (bystander, _) = host.connect(10, 10); + let write = caught_up( + host.app(), + ModelCommand::SetCell { + view: "Default".into(), + cell: client + .record_cell_at(0, 0) + .expect("North/Revenue is stored"), + value: CellValue::Number(77.0), + }, + ); + let projected = host.handle(bystander, ClientFrame::command(write)); + let for_requester: Vec = projected + .into_iter() + .filter(|frame| frame.to == id) + .collect(); + assert!( + !for_requester.is_empty(), + "the regression must process a later projection" + ); + deliver(&mut client, &for_requester); + assert_eq!( + &client.state().view, + host.sessions().get(id).expect("attached").view(), + "the later projection must leave the browser and host mirror in lockstep" + ); +} + /// A refused frame must leave this client where the **host** says it is, and /// the host says so in its own mirror. See `context/test-evidence/clients.md`. #[test]