diff --git a/crates/app-cli/src/mcp.rs b/crates/app-cli/src/mcp.rs index 8a355d9..dc1f0e7 100644 --- a/crates/app-cli/src/mcp.rs +++ b/crates/app-cli/src/mcp.rs @@ -18,8 +18,9 @@ //! CLI `export` verb). //! - `firecraft.undo` / `firecraft.redo` — step the session's undo //! stack; the step label comes back as text. -//! - `firecraft.listCommands` — every registry command id the session -//! binds (`engine::catalog::ids()` authority), one per line. +//! - `firecraft.listCommands` — every command id the session runs +//! (the registry's ids plus the session-scoped ones — the +//! `engine::catalog::all_ids()` authority), one per line. //! //! Resources: `document://current` is the `info` summary as //! `application/json` — agents can read it without a tool round-trip. @@ -333,10 +334,15 @@ fn undo_tool(session: &mut Session, id: serde_json::Value, redo: bool) -> serde_ } } -/// `firecraft.listCommands`: the registry ids the session binds, one +/// `firecraft.listCommands`: every command id the session runs, one /// per line (sorted — discovery wants scan order, not insertion order). +/// +/// The list is the registry's ids plus the session-scoped ones +/// (`history.rewind`): a command `firecraft.apply` can run belongs in +/// the discovery list, or an agent cannot find it. fn list_commands_tool(session: &Session, id: serde_json::Value) -> serde_json::Value { let mut names = session.command_names(); + names.extend(engine::catalog::session_ids()); names.sort_unstable(); tool_result(id, false, &names.join("\n")) } diff --git a/crates/app/src/shell/mod.rs b/crates/app/src/shell/mod.rs index 5aaf469..43a29ce 100644 --- a/crates/app/src/shell/mod.rs +++ b/crates/app/src/shell/mod.rs @@ -541,12 +541,14 @@ impl Shell { self.smart_guides = self.layout.smart_guides; self.ui_scale = self.layout.ui_scale; // Recents naming a command this build lacks are dropped — - // same rule as the panel inventory above. + // same rule as the panel inventory above. The list is the + // registry's ids plus the session-scoped ones, because the + // palette offers both. self.palette.recents = self .layout .palette_recents .iter() - .filter(|recent| engine::catalog::ids().contains(&recent.as_str())) + .filter(|recent| engine::catalog::all_ids().contains(&recent.as_str())) .cloned() .collect(); } @@ -4112,7 +4114,7 @@ mod tests { restored.palette.filter.clear(); let hits = restored.palette_matches(); assert_eq!(&hits[..2], ["object.setOpacity", "autoshape.rect"]); - assert_eq!(hits.len(), engine::catalog::ids().len()); + assert_eq!(hits.len(), engine::catalog::all_ids().len()); } /// reachable by an empty filter, carries a non-null payload @@ -4124,12 +4126,16 @@ mod tests { let mut shell = gui_shell(gui_editor()); shell.toggle_palette(); assert!(shell.palette.open, "the palette opens on demand"); - let catalog = engine::catalog::ids(); + let catalog = engine::catalog::all_ids(); assert_eq!( shell.palette_matches(), catalog, "palette lists the catalog" ); + assert!( + catalog.contains(&engine::REWIND_ID), + "the session-scoped rewind is offered alongside the registry" + ); for name in catalog { let payload = shell.payload_for(name); assert!(payload.is_object(), "{name}: payload is not an object"); @@ -4144,6 +4150,44 @@ mod tests { } } + /// The palette runs the session-scoped rewind: the command is + /// offered beside the registry's ids, its payload is grounded to + /// the live history (one step back, not a fuzz depth), and running + /// it takes that step back. + #[test] + fn palette_runs_the_session_scoped_rewind() { + let mut shell = gui_shell(gui_editor()); + assert!( + shell.palette_matches().contains(&engine::REWIND_ID), + "the palette offers the rewind" + ); + let before = shell.editor.session.undo_len(); + let id = shell.editor.current_cell().expect("cell").elements[0]; + shell + .editor + .apply( + "object.rename", + serde_json::json!({ "id": id, "name": "renamed" }), + ) + .expect("rename"); + assert_eq!(shell.editor.session.undo_len(), before + 1); + + // The grounded payload names the depth below where the + // document sits — the step the run takes back. + let payload = shell.payload_for(engine::REWIND_ID); + assert_eq!( + payload["depth"], + serde_json::json!(before), + "the payload is grounded to the live history" + ); + assert!( + shell.run_palette_command(engine::REWIND_ID), + "the rewind runs: {status}", + status = shell.status + ); + assert_eq!(shell.editor.session.undo_len(), before); + } + /// The palette filter narrows by substring (case-insensitive), the /// selection index is reset when the query changes, and grounding /// clamps the fuzzdrive template's out-of-range targets to the @@ -4152,7 +4196,7 @@ mod tests { #[test] fn palette_filters_and_grounds_payloads() { let mut shell = gui_shell(gui_editor()); - let total = engine::catalog::ids().len(); + let total = engine::catalog::all_ids().len(); shell.palette.filter = "rect".to_owned(); for name in shell.palette_matches() { diff --git a/crates/app/src/shell/palette.rs b/crates/app/src/shell/palette.rs index 446ccf6..ee881fa 100644 --- a/crates/app/src/shell/palette.rs +++ b/crates/app/src/shell/palette.rs @@ -3,13 +3,13 @@ use super::*; -/// The command palette: Ctrl+P opens; typing filters `catalog::ids()` -/// by substring; Enter runs the top hit through `Editor::apply` (the -/// same registry the menus and the journal use). Payload templates -/// come from the engine's fuzzdrive table — the same table whose -/// coverage test (`the_call_table_covers_the_catalog`) guarantees one -/// arm per registered command. Templates are built lazily, for the -/// visible page of hits only. +/// The command palette: Ctrl+P opens; typing filters +/// `catalog::all_ids()` by substring; Enter runs the top hit through +/// `Editor::apply` (the same registry the menus and the journal use). +/// Payload templates come from the engine's fuzzdrive table — the same +/// table whose coverage test (`the_call_table_covers_the_catalog`) +/// guarantees one arm per registered command. Templates are built +/// lazily, for the visible page of hits only. #[derive(Default)] pub(crate) struct CommandPalette { pub(super) open: bool, @@ -55,9 +55,13 @@ impl Shell { /// first. An empty filter shows the recents, then the catalog in /// order. Ties and unscored names keep catalog order (stable /// sort). + /// + /// The catalog is the registry's ids *plus* the session-scoped + /// ones (`history.rewind`): a command the engine can run belongs + /// in the palette even when the registry cannot own it. pub(super) fn palette_matches(&self) -> Vec<&'static str> { let query = self.palette.filter.to_lowercase(); - let catalog = engine::catalog::ids(); + let catalog = engine::catalog::all_ids(); if query.is_empty() { // Recents first (deduplicated, catalog-resident only), // then the remaining catalog in order. @@ -103,10 +107,30 @@ impl Shell { /// The payload template for one command under one seed. pub(super) fn payload_for_seed(&self, name: &str, seed: u64) -> serde_json::Value { + if name == engine::REWIND_ID { + // The rewind's payload names a depth in the undo history. + // A palette run is a *user's* undo — one step back from + // where the document sits now — not a fuzz depth. + return serde_json::json!({ "depth": self.rewind_depth() }); + } let mut rng = engine::fuzzdrive::FuzzRng::new(seed); engine::fuzzdrive::payload_template(name, &mut rng, self.editor.document()) } + /// The depth the palette's rewind lands on: one step below the + /// document's current place in the undo history. + /// + /// The history's rows are the document as opened (depth `0`) plus + /// one row per recorded edit, so the row count is the depth the + /// document sits at plus one. + fn rewind_depth(&self) -> usize { + self.editor + .session + .history_entries() + .len() + .saturating_sub(2) + } + /// Grounds a template to the live editor state. The fuzzdrive /// table aims targets out of range on purpose (a fuzz driver wants /// refused calls); a user does not. Target keys are clamped to the @@ -370,7 +394,7 @@ impl Shell { ui.weak(format!( "{} of {}", names.len(), - engine::catalog::ids().len() + engine::catalog::all_ids().len() )); }); }); diff --git a/crates/app/tests/history_rewind.rs b/crates/app/tests/history_rewind.rs index 31d3a08..f369629 100644 --- a/crates/app/tests/history_rewind.rs +++ b/crates/app/tests/history_rewind.rs @@ -4,8 +4,13 @@ #![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +use std::rc::Rc; + use app::editor::Editor; +use app_cli::mcp; use base64::Engine as _; +use engine::Session; +use engine::platform::{FileSystem, MemoryFs}; /// Imports one 2×2 opaque-red raster and returns its id. fn import_raster(editor: &mut Editor, name: &str) -> doc::ObjectId { @@ -92,3 +97,68 @@ fn a_rewind_past_the_recorded_depth_is_refused() { ); assert_eq!(editor.session.undo_len(), base + 1); } + +/// The MCP surface names the session-scoped rewind and runs it: a +/// command an agent cannot discover is a command the agent cannot +/// use. +#[test] +fn the_mcp_surface_lists_and_runs_the_rewind() { + let base = Editor::untitled().session.document().clone(); + let mut session = Session::new(base); + + let call = |session: &mut Session, name: &str, arguments: serde_json::Value| { + mcp::handle( + session, + Rc::new(MemoryFs::new()) as Rc, + &serde_json::json!({ + "jsonrpc": "2.0", "id": 1, "method": "tools/call", + "params": { "name": name, "arguments": arguments }, + }), + ) + .expect("tools/call answers") + }; + + // The discovery list carries the session-scoped id beside the + // registry's. + let reply = call( + &mut session, + "firecraft.listCommands", + serde_json::json!({}), + ); + let text = reply["result"]["content"][0]["text"] + .as_str() + .expect("text"); + assert!( + text.split('\n').any(|line| line == engine::REWIND_ID), + "the rewind is listed: {text}" + ); + + // And it runs: one step of the undo stack is taken back. + let before = session.undo_len(); + session + .apply( + "object.createPath", + serde_json::json!({ + "kind": "rect", "x": 10.0, "y": 20.0, + "width": 80.0, "height": 40.0, + "page": 0, "layer": 0, "cell": 0, "name": "plate", + }), + ) + .expect("create"); + assert_eq!(session.undo_len(), before + 1); + + let reply = call( + &mut session, + "firecraft.apply", + serde_json::json!({ + "command": engine::REWIND_ID, + "args": { "depth": before }, + }), + ); + assert_eq!( + reply["result"]["isError"], + serde_json::json!(false), + "{reply}" + ); + assert_eq!(session.undo_len(), before); +} diff --git a/crates/engine/src/fuzzdrive.rs b/crates/engine/src/fuzzdrive.rs index 463447c..597fdbd 100644 --- a/crates/engine/src/fuzzdrive.rs +++ b/crates/engine/src/fuzzdrive.rs @@ -3056,6 +3056,13 @@ pub fn payload_template(name: &str, rng: &mut FuzzRng, document: &Document) -> s "contiguous": rng.chance(3), "mode": rng.pick(&["replace", "add", "subtract", "intersect"]).copied(), }), + // The session-scoped rewind names a depth in the undo + // history. The depth is a *count*, not an index, so the + // template aims one past the top of a fresh session: a + // refused call, exactly like the other out-of-range arms. + crate::session::REWIND_ID => serde_json::json!({ + "depth": rng.below(4), + }), other => { let _ = other; // A catalog command without a builder: an empty payload is diff --git a/docs/NEXT.md b/docs/NEXT.md index 97f2414..addcc2b 100644 --- a/docs/NEXT.md +++ b/docs/NEXT.md @@ -450,10 +450,15 @@ evidence work, tracked outside that campaign. `new_parks_the_current_document`, `close_refuses_a_dirty_document`. (Window-menu panel toggles landed in Phase 19.6 batches 3 and 10.) -- **20.6 History panel click-to-rewind** (S): **LANDED 2026-11-11** - in #107 — journal replay to seq N; the session-scoped rewind - command is also discoverable from the command palette and MCP - list/run surface in open PR #122. +- **20.6 History panel click-to-rewind** (S): **LANDED 2026-11-11** — the + clickable History rows (the engine half, decision 0082) and the + command surface: `history.rewind` is a session-scoped id, so every + surface that enumerates commands enumerates + `catalog::all_ids()` — the command palette offers it (with its + payload grounded one step below the document's live depth) and + `firecraft.listCommands` advertises it + ([T10](tasks/T10-history-rewind-command.md), decision + [0096](decisions/0096-the-rewind-is-a-command-every-surface-can-name.md)). - **20.7 Slice/hotspot behavior editing + HTML preview** (M): **LANDED 2026-11-11** in #107. diff --git a/docs/PROGRESS.md b/docs/PROGRESS.md index a2db7a8..9a40c24 100644 --- a/docs/PROGRESS.md +++ b/docs/PROGRESS.md @@ -69,6 +69,14 @@ evidence. - **Clean-room boundary:** local CS6 files and Adobe-derived extracts remain uncommitted; visual assets require attribution. +- **History rewind as a command (2026-11-11, decision 0096):** the + session-scoped `history.rewind` is enumerated by + `catalog::all_ids()`, so every surface that lists commands lists it + too: the command palette offers it (its payload grounded one step + below the document's live depth, not a fuzz depth) and the MCP + bridge's `firecraft.listCommands` advertises it. The registry + catalog and the reachability measurement are unchanged — the id is + not a registry id, so 168/253 (66.40%) stands. ## Build and deployment evidence diff --git a/docs/decisions/0096-the-rewind-is-a-command-every-surface-can-name.md b/docs/decisions/0096-the-rewind-is-a-command-every-surface-can-name.md new file mode 100644 index 0000000..eb0ef73 --- /dev/null +++ b/docs/decisions/0096-the-rewind-is-a-command-every-surface-can-name.md @@ -0,0 +1,74 @@ +# 0096 — The rewind is a command every surface can name + +Date: 2026-11-11 · Ticket: [T10](../tasks/T10-history-rewind-command.md) · Status: accepted + +## Context + +Decision 0082 landed the engine half of the history rewind: +`history.rewind` is a **session-scoped** command id — handled by +`Session::apply` before the registry lookup, because a rewind acts on +the undo stack rather than on the document, and a catalog command is +required to have an exact document-level inverse. + +That record named the app half as a separate change and warned about +the trap it leaves behind: + +> A surface enumerating commands must use `catalog::all_ids()` to see +> the session-scoped ones. + +The app surface was still enumerating `catalog::ids()`. The History +panel's rows were clickable (the panel rewinds through +`Session::rewind_to` directly), but the command palette offered only +the registry's ids — so `history.rewind` was a command the engine +could run, the palette could not name, and the MCP bridge's +`firecraft.listCommands` could not advertise. An agent that discovers +its commands by asking could not find the rewind; a user who knew the +id could not type it. + +## Decision + +Every surface that enumerates commands enumerates the commands it can +*run*, not the subset the registry owns. + +- `crates/app/src/shell/palette.rs` filters `engine::catalog::all_ids()` + — the registry's ids plus the session-scoped ones — and the palette's + counter ("N of M") counts the same list. +- The shell's persisted *recents* are restored against the same list, + so a recent `history.rewind` survives a restart (a name the build + cannot run is still dropped). +- `crates/app-cli/src/mcp.rs` — `firecraft.listCommands` lists + `session.command_names()` plus `catalog::session_ids()`. + `firecraft.apply` already routed the id through `Session::apply`, so + the command was runnable but invisible; discovery now matches + capability. +- `crates/engine/src/fuzzdrive.rs` carries a payload arm for + `history.rewind` (`{"depth": n}`), so the template table that the + palette and the fuzz driver share can build the command's payload. + The arm is deliberately **not** in `fuzzdrive::CALLS`: that table is + asserted to equal `catalog::ids()` exactly, and the session-scoped + id is not a registry id. + +The palette's payload for the rewind is **grounded to the live +history**: one step below the depth the document currently sits at. +The fuzz template's depth is a random number in `0..4` — right for a +fuzz driver, wrong for a user, who means "undo the last thing". + +## Consequences + +- The palette's catalogue grows by one command. The reachability + measurement does not move: the gate counts the *registry* ids the + shell binds, and `history.rewind` is not a registry id, so + 168/253 (66.40%) stands and the floor is untouched. +- The palette's "covers every command" property now covers the + session-scoped id too, and the property is stated against + `all_ids()` so it cannot silently drift back to the registry subset. +- `catalog::ids()` remains the registry's own list, and the fuzz + properties that depend on it are unchanged. +- A future session-scoped command (a session save, a session reset) + becomes nameable in the palette and in the MCP discovery list by + being added to `catalog::session_ids()` — the enumeration sites do + not need to change again. + +See also decision 0082 (the engine half this completes) and decision +0076 (the runtime-resolved workspace root that lets the gate measure +whichever worktree it runs in). diff --git a/docs/tasks/T10-history-rewind-command.md b/docs/tasks/T10-history-rewind-command.md new file mode 100644 index 0000000..104ccff --- /dev/null +++ b/docs/tasks/T10-history-rewind-command.md @@ -0,0 +1,44 @@ +# T10 — 20.6 History rewind as a named command + +**Objective:** the session-scoped `history.rewind` becomes a command +every surface can *name*: the command palette offers it, the MCP +bridge's discovery list advertises it, and a palette run takes back the +step the user meant rather than a fuzz depth. + +**Inputs:** decision 0082 (the engine half — `history.rewind` as a +session-scoped id, `catalog::session_ids()`, `catalog::all_ids()`); +the shell's command palette and its persisted recents; the MCP bridge's +`firecraft.listCommands` / `firecraft.apply` tools; the engine's +fuzzdrive payload-template table. + +**Authority:** decision 0082's design — a rewind is a session +operation, so it stays out of the registry catalog and is enumerated +through `all_ids()`. CS6's History panel remains the clickable +surface; this ticket adds the *command* surface beside it. + +**Stopping condition:** + +- The palette lists `history.rewind` beside the registry's ids, and its + "N of M" counter counts the same list. +- The palette's payload for the rewind is grounded to the live history + (one step below the current depth), and running it takes that step + back. +- The shell's persisted recents keep a recent `history.rewind` across a + restart. +- `firecraft.listCommands` lists the session-scoped id, and + `firecraft.apply` runs it. +- The fuzzdrive payload table carries an arm for the id *without* + entering it in `CALLS` (that table is asserted to equal + `catalog::ids()` exactly). +- The reachability floor does not move: the gate counts registry ids, + and this id is not one. + +**Verification:** `cargo test -p app -p app-cli -p engine`; the +palette unit test +`palette_runs_the_session_scoped_rewind`; the integration test +`the_mcp_surface_lists_and_runs_the_rewind`; the full gate +(`cargo run -p xtask -- ci`) before commit. + +**Status — LANDED (2026-11-11):** the code, tests, decision 0096 and +the ledger rows are in the tree. The gate receipts are attached to the +pull request.