fix(improvise-2mqz): keep download sessions in lockstep
Co-authored-by: Ed Langley <fiddlerwoaroof@users.noreply.github.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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<Addressed> {
|
||||
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<Addressed> {
|
||||
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<Vec<Addressed>> {
|
||||
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
|
||||
|
||||
@@ -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<Addressed> = 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]
|
||||
|
||||
Reference in New Issue
Block a user