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

A read-only tab refuses edits instead of painting what it cannot save (improvise-j4qt)

Browse Source 
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
Edward Langley
2026-09-22 21:12:24 -07:00
parent 699f0da05e
commit a77e3a9146
4 changed files with 122 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/code/clients.md
+41
View File
@@ -124,6 +124,47 @@ So this is the browser's own answer, and `ClientFrame::Save` is the other one
for a socket client editing a TUI-hosted document, where there *is* a file
somebody owns.
### A read-only session refuses edits; it does not accept them and fail to save (improvise-j4qt)
A second tab on the same document cannot take the exclusive OPFS handle, so it
boots **read-only** (improvise-i0v9). It reads the bytes, paints the grid, and
navigates. Until improvise-j4qt, "read-only" described only the *storage*. The
session was writable: an edit was applied to the tab's own in-memory `App` and
projected, so the grid **painted a value the document did not have**
(`18,400.00` became `184,004,242.00`). Only then did the automatic persist
fail, with `NOT SAVED` replacing the read-only reason in the status. The
first tab never saw the edit.
The fix sets the session's access at boot: `Worker::read_only` versus
`Worker::new`, as a private `Access`. A read-only worker answers any
`ClientFrame::Commands` that contains a `Command::Model` with
`Rejected(Rejection::ReadOnly)` **before the host sees it**. So nothing is
applied, nothing is projected, and nothing is persisted. Three things make
this the right place:
- **It is decided up front, not observed afterwards.** The persist failure is
an observation, and it arrives *after* the paint. By the time a failed save
can be seen, the user has already been shown the number.
- **The client's rollback is already correct for it** (improvise-nxah).
`Rejection::ReadOnly` answers `refused_an_edit() == true`, so the client
pops that frame's entry and replays `ViewState` from its settled slice. No
cache needs rolling back: `Command::Model` is a no-op in `reduce_view`, so
the optimistic fold never painted the value. The paint came from the host's
projection, and that projection is exactly what the refusal prevents.
- **Only the document is refused.** A frame of view commands, such as a cursor
move, a panel or a help page, is answered as before. Read-only is a claim
about the document, not about this client's own slice.
It is a property of the *boot*, not the backend. The backend in both cases is
a `vfs::ReadOnly`, and `worker.rs`'s `a_persist_that_fails_raises_a_notice…`
still uses one as a disk-full injection over a *writable* session. Reading
access off the storage would have made those two indistinguishable.
`ClientFrame::Save` is unchanged: a read-only tab that asks to save is still
answered `SaveFailed` by the backend.
What this does **not** do is i0v9's gap 2. There is still no upgrade path when
the first tab closes, and the tab stays read-only until it is reloaded.
### Ordering
`Worker::on_frame` returns the frames for one inbound frame, in order, and a
context/test-evidence/clients.md
+37
View File
@@ -4177,3 +4177,40 @@ and converged.
**Fault injected:** the same `Binding::Prefix` → `Upstream` reversion. Fails on
the second press.
### `a_read_only_session_refuses_an_edit_before_anything_is_painted`
improvise-j4qt. The finder measured this in Chromium and Firefox: two tabs, one
document. The second tab boots read-only. Typing `4242` into a cell applied it
to that tab's in-memory `App` and projected it, so the grid showed
`184,004,242.00`. Only then did the persist fail and `NOT SAVED` replace the
read-only reason. The first tab never saw the edit.
The test builds the read-only boot the way `bindings::read_only` does: the
document's bytes over a `vfs::ReadOnly` backend. It sends a caught-up `SetCell`
and asserts four things:
- the whole reply is `[Rejected(ReadOnly)]`;
- the client's folded grid still shows the old text;
- no notice was raised (no persist ran);
- the document is not dirty.
**Red first**: the same test over `Worker::new`, which was the boot's
constructor before this change, failed. Its reply was a `Commands` frame of
projections carrying the new value.
The control is `a_read_only_session_still_answers_a_view_command`: a
`SetSelected` frame is not refused. Without it, a worker that refused *every*
frame would pass the test above.
End to end: `harness/tests/shell.spec.js`, "a read-only tab refuses an edit and
never paints it". It opens two real tabs on one document in Chromium, edits in
the second, and asserts four things:
- the toast says `read-only`;
- the cell keeps its text;
- the grid contains no `4,242`;
- `#status` still says `read-only` and not `NOT SAVED`.
Fault injected: `bindings.rs` reverted to `Worker::new(app)` for the read-only
boot, then `harness/build.sh` rebuilt. The spec then failed on the toast, which
received `""` because no refusal arrived. Reverted and rebuilt, and the whole
of `shell.spec.js` passed (18).
crates/improvise-worker-server/src/worker.rs
+10 -3
View File
@@ -315,7 +315,8 @@ mod tests {
let fs = MemoryFs::new();
let path = PathBuf::from("demo.improv");
fs.write(&path, &demo_bytes()).unwrap();
let locked = crate::vfs::ReadOnly::new(fs, "another tab or window is holding this document");
let locked =
crate::vfs::ReadOnly::new(fs, "another tab or window is holding this document");
let wb = vfs::open(&locked, &Opening::named(&path)).unwrap().workbook;
let mut app = App::with_storage(wb, Some(path), Box::new(locked));
app.rebuild_layout();
@@ -351,7 +352,10 @@ mod tests {
None,
"nothing was applied, so no persist ran and none failed"
);
assert!(!worker.app().model_state.dirty, "the edit reached the document");
assert!(
!worker.app().model_state.dirty,
"the edit reached the document"
);
}
/// The control: read-only is about the DOCUMENT. Moving the cursor writes
@@ -360,7 +364,10 @@ mod tests {
fn a_read_only_session_still_answers_a_view_command() {
let mut worker = read_only_worker();
let _ = connect(&mut worker);
let out = worker.on_frame(ClientFrame::command(Command::SetSelected { row: 1, col: 0 }));
let out = worker.on_frame(ClientFrame::command(Command::SetSelected {
row: 1,
col: 0,
}));
assert!(
!out.iter().any(|f| matches!(f, ServerFrame::Rejected(_))),
"a read-only tab refused a cursor move: {out:?}"
harness/tests/shell.spec.js
+34
View File
@@ -257,6 +257,40 @@ test.describe("the standalone deployment", () => {
await first.close();
});
// improvise-j4qt: the read-only tab used to APPLY an edit to its own copy
// and paint it, a number the document never had. It must refuse the edit.
test("a read-only tab refuses an edit and never paints it", async ({ context }) => {
const doc = `read-only-edit-${Date.now()}.improv`;
const first = await context.newPage();
await first.goto(`/app/index.html?doc=${doc}`);
await expect(first.locator("#grid table")).toBeVisible({ timeout: 20_000 });
const second = await context.newPage();
await second.goto(`/app/index.html?doc=${doc}`);
await expect(second.locator("#grid table")).toBeVisible({ timeout: 20_000 });
await expect(second.locator("#status")).toContainText("read-only");
const cell = second.locator("#grid td.editable").first();
const before = await cell.textContent();
await cell.click();
const input = cell.locator("input");
await expect(input).toBeFocused();
await input.fill("4242");
await input.press("Enter");
// The refusal is what arrives, and a document message is a toast. Waiting
// on it first means the grid below is asserted AFTER the answer landed.
await expect(second.locator("#toast")).toContainText("read-only", { timeout: 10_000 });
await expect(cell).toHaveText(before);
await expect(second.locator("#grid")).not.toContainText("4,242");
// The boot's reason stands; no failed save replaced it (the old symptom).
await expect(second.locator("#status")).toContainText("read-only");
await expect(second.locator("#status")).not.toContainText("NOT SAVED");
await second.close();
await first.close();
});
// What lands on screen is the HOST's value, not the string typed — the
// round trip through a real worker, which no unit test can stand in for.
test("committing an edit sends one frame and shows the host's value", async ({