From e7ae0f442685daba19c5c4b8c6feb21b2c428ed9 Mon Sep 17 00:00:00 2001 From: BFlat Date: Fri, 30 Oct 2026 07:29:40 +0700 Subject: [PATCH] M9: share links can comment: a comment access level beside edit and view (#1525) (#1576) Someone who joins with a comment link (or its join code) sees the file live and adds, replies to, reacts to and resolves comment threads, but can't change the design: their app refuses other edits and the server accepts a comment link's batch only when every change in it touches comment threads. Sharing online makes a comment link and code beside the edit and view ones; the Share popover offers Can edit, Can comment and Can view, and the commenter's toolbar shows a Can comment badge with Move, Hand and Comment. Backward compatible: clients announce the comments word; one that doesn't is welcomed with a comment link as a viewer and gets comment links listed as view links, a level this build doesn't know reads as view, and an older server that can't make a comment link leaves the share without one. --- ROADMAP.md | 3 +- apps/figcraft-server/src/cli.rs | 9 +- apps/figcraft-server/src/codes_tests.rs | 9 +- .../figcraft-server/src/comment_link_tests.rs | 145 +++++++++++++ apps/figcraft-server/src/lib.rs | 2 + apps/figcraft-server/src/owned_tests.rs | 2 +- crates/collab-net/src/exchange.rs | 59 ++++-- crates/collab-net/src/exchange_tests.rs | 28 ++- crates/collab-net/src/hub.rs | 30 +-- crates/collab-net/src/hub/barred_tests.rs | 2 +- crates/collab-net/src/hub/queue_tests.rs | 2 +- crates/collab-net/src/manage.rs | 17 +- crates/collab-net/src/requests.rs | 44 +++- crates/collab/src/change.rs | 10 + crates/collab/src/client.rs | 6 + crates/collab/src/comment_access_tests.rs | 200 ++++++++++++++++++ crates/collab/src/lib.rs | 4 +- crates/collab/src/message.rs | 97 ++++++++- crates/collab/src/server.rs | 32 ++- crates/engine/src/cmd/view.rs | 5 +- crates/engine/src/comment_link_tests.rs | 91 ++++++++ crates/engine/src/lib.rs | 2 + crates/engine/src/live_share_tests.rs | 4 +- crates/engine/src/online_tests.rs | 2 +- crates/engine/src/session/exec.rs | 7 +- crates/engine/src/sharing/live.rs | 18 ++ crates/engine/src/sharing/live/events.rs | 24 ++- crates/engine/src/sharing/live/host.rs | 34 +-- crates/engine/src/sharing/live/join.rs | 10 +- crates/engine/src/sharing/live/sync.rs | 9 +- crates/engine/src/sharing/live/types.rs | 27 ++- crates/engine/src/sharing/online.rs | 14 +- crates/ui-egui/locales/ar.toml | 3 + crates/ui-egui/locales/bn.toml | 3 + crates/ui-egui/locales/cs.toml | 3 + crates/ui-egui/locales/de.toml | 3 + crates/ui-egui/locales/en.toml | 3 + crates/ui-egui/locales/es.toml | 3 + crates/ui-egui/locales/fi.toml | 3 + crates/ui-egui/locales/fr.toml | 3 + crates/ui-egui/locales/id.toml | 3 + crates/ui-egui/locales/it.toml | 3 + crates/ui-egui/locales/ja.toml | 3 + crates/ui-egui/locales/ka.toml | 3 + crates/ui-egui/locales/ko.toml | 3 + crates/ui-egui/locales/nl.toml | 3 + crates/ui-egui/locales/pl.toml | 3 + crates/ui-egui/locales/pt-BR.toml | 3 + crates/ui-egui/locales/ru.toml | 3 + crates/ui-egui/locales/sv.toml | 3 + crates/ui-egui/locales/th.toml | 3 + crates/ui-egui/locales/tr.toml | 3 + crates/ui-egui/locales/vi.toml | 3 + crates/ui-egui/locales/zh-Hans.toml | 3 + crates/ui-egui/src/live.rs | 48 +++-- crates/ui-egui/src/live_server_tests.rs | 26 ++- crates/ui-egui/src/live_share_tests.rs | 42 +++- crates/ui-egui/src/online_tests.rs | 2 +- crates/ui-egui/src/toolbar.rs | 22 +- docs/collab-protocol.md | 19 +- docs/collaboration-parity.md | 5 +- docs/control-protocol.md | 2 +- docs/mcp.md | 17 +- docs/roadmap.md | 3 +- docs/server.md | 17 +- 65 files changed, 1039 insertions(+), 178 deletions(-) create mode 100644 apps/figcraft-server/src/comment_link_tests.rs create mode 100644 crates/collab/src/comment_access_tests.rs create mode 100644 crates/engine/src/comment_link_tests.rs diff --git a/ROADMAP.md b/ROADMAP.md index ec9dce99a..8e4f66f17 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: alpha** · next: beta, ~26 points and ~162–292 h away (one Opus 5.5 agent; ~33–67 h wall-clock with 5 agents) -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-29 · **Change:** minor (Preferences ▸ Scroll wheel zoom: a mouse wheel zooms the canvas about the pointer and Cmd/Ctrl+wheel scrolls (#1390); `comment.list {mentions}` lists only the threads that @mention a name (#1415); in the browser an internal error that stops the page keeps its unsaved files, which open again on reload (#1123); boolean operations refuse input past their caps, and a boolean group past them draws without its result (#1104); figcraft-cli parses its arguments with clap, following RFC-0012 (#1396); the selection and hover outlines of rounded rectangles and frames follow their corners (#1506); component properties keep the order they were made in when one is renamed, and Make slot asks for the slot's name first (#1536, #1537); the browser build publishes files to a server, so Share ▸ Online and Publish to server work there (#917); the browser build uses the fonts installed on the computer once asked, each read when text uses it (#1051); SVG import reads `color()` and `color-mix()` colours, with a note and a grey for those it can't (#1550); the portable Windows zip keeps its settings, recovery copies and plugins in `config\` beside the executable, starting with an installed copy's preferences; the window starts on other graphics when its first ones fail before the first frame: Direct3D 12 compiles with FXC and no device compiles wgpu's indirect-validation shaders, then Vulkan, OpenGL and lower requirements on the power-saving GPU (#965, #1438); Thai, Lao, Khmer and Burmese lines break between words (#1219); an AI agent attached over MCP shows where it works (#1416); a manual overlay's position can be dragged on the canvas (#405); one name for live sessions and comments: Preferences ▸ Your name, in the Join dialog too, kept in the browser, and a rename shows at once in the sessions you are in (#1380); requests that change a server's files have a budget per address, the server key's too, and join codes are found by their hash (#1075); the Join dialog shows a join from the click until its file is ready, with Cancel, and a failed join with Try again (#1381); requests that change a server's files have a budget per address, the server key's too, and join codes are found by their hash (#1075); SVG import draws each pattern tile once per scale, within the layer budget (#1080); Vietnamese letters typed with combining marks (decomposed) take one font that has every mark, instead of missing-glyph boxes beside the letter (#1478); comment.list reports each message's @mentions (#1415); mask groups draw only over the area they can paint, and nested isolated layers share one memory budget (#1061); components can't contain their own instances, directly or through other components; `edit.duplicate` returns an id map with `map: true` (#995); the batches kept for reconnecting apps are bounded in bytes by default, and `figcraft-server` takes `max_log_mb` (#1069); MCP and control calls name their document (#998); sibling placement in live sessions searches instead of counting (#1065); an AI agent attached over MCP comments under its own name, with an Agent label (#1414); the instance-swap menus have a search field (#1338); a refused batch's fixes take time linear in the batch (#1063); the connector tool shows the connector it will make while dragging, ends snapping to attach points (#1387); large images on the canvas draw from a smaller copy, so two 16k images no longer lag (#1483); a file's JPEG thumbnail can't crash the home screen (#1491's progressive JPEG check, now shared); `.fig` import: library text styles linked by asset key, and override rows restating a nested level (#1234); `.fig` import keeps the file's saved line breaks while the text and its box are unedited (#1236); Download a missing free font from the Missing fonts window (`font.download`, #754); the web build's wasm back under its 24 MiB gate (#1472); shadows of images with transparent pixels follow their alpha (#1289); Crop image: Object ▸ Crop image, the right-click menu and the image fill row enter crop mode (`image.crop`, #1464); offline Penpot 2.x import/export (#1143); optional batch continue-on-error with per-step outcomes (#1386); the Design panel's sections on one grid (#858); FigJam outer shape outlines and sticky body fallback beside author text (#1341, #1397); images placed at their own size, 2x screenshots in points (#1360); the toolbar on one 32 pt grid, without its Resources button (#1210); M4: zero-length round-capped dashes draw dots; dash expansion uses the existing safety budget; extracted from #1143; Arabic draft catalog; scaled instances keep their scale when their main is rebuilt (#922); .fig embedded images are copied once per blob, not once per hash (#1059); page ruler guides survive save and reopen (#1476); colour picker LCH and OKLCH models; non-ASCII layer names transliterate in code export (#1235); branches in live sessions and branch history entries, gap 17; plugin widgets take text input and a property menu (plugin API 4); boards: templates and tables; boards: a shared timer and voting sessions; boards: the dot grid, connector ends dragged to re-attach, curved connectors; Installed weights of an embedded font family (#1246); `.sketch` import, best effort (Sketch 48 and newer); CJK line breaking: JIS X 4051's line-start and line-end sets completed with the halfwidth small kana, the halfwidth and spacing (semi-)voiced sound marks, the iteration marks 〱–〵 and the halfwidth parentheses, with behaviour cases for each; Adobe Fonts found on macOS (#298); egui 0.37 hidden-capture deadline contract; Dev Mode measurements kept on the canvas and a read-only layers panel; boards: a board area row and progress entry; community fixes in #507, #510, #529 and #530: auto-save, a text box by dragging, batch step references, removing someone from a live session, Russian and Turkish catalogs reviewed, Czech and Indonesian drafts, the Japanese draft completed, zoom previews, the web clipboard, Alt-drag snapping and Cmd+D repeat, recovery copies, crash and rollback fixes, colour emoji, `.fig` import fixes; earlier changes are in the revision history below) · **Target:** Figma Design (desktop app 126.10.7) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-29 · **Change:** minor (share links can comment: a third access level beside edit and view, whose holder sees the file live and adds, replies to, reacts to and resolves comments but can't change the design; Preferences ▸ Scroll wheel zoom: a mouse wheel zooms the canvas about the pointer and Cmd/Ctrl+wheel scrolls (#1390); `comment.list {mentions}` lists only the threads that @mention a name (#1415); in the browser an internal error that stops the page keeps its unsaved files, which open again on reload (#1123); boolean operations refuse input past their caps, and a boolean group past them draws without its result (#1104); figcraft-cli parses its arguments with clap, following RFC-0012 (#1396); the selection and hover outlines of rounded rectangles and frames follow their corners (#1506); component properties keep the order they were made in when one is renamed, and Make slot asks for the slot's name first (#1536, #1537); the browser build publishes files to a server, so Share ▸ Online and Publish to server work there (#917); the browser build uses the fonts installed on the computer once asked, each read when text uses it (#1051); SVG import reads `color()` and `color-mix()` colours, with a note and a grey for those it can't (#1550); the portable Windows zip keeps its settings, recovery copies and plugins in `config\` beside the executable, starting with an installed copy's preferences; the window starts on other graphics when its first ones fail before the first frame: Direct3D 12 compiles with FXC and no device compiles wgpu's indirect-validation shaders, then Vulkan, OpenGL and lower requirements on the power-saving GPU (#965, #1438); Thai, Lao, Khmer and Burmese lines break between words (#1219); an AI agent attached over MCP shows where it works (#1416); a manual overlay's position can be dragged on the canvas (#405); one name for live sessions and comments: Preferences ▸ Your name, in the Join dialog too, kept in the browser, and a rename shows at once in the sessions you are in (#1380); requests that change a server's files have a budget per address, the server key's too, and join codes are found by their hash (#1075); the Join dialog shows a join from the click until its file is ready, with Cancel, and a failed join with Try again (#1381); requests that change a server's files have a budget per address, the server key's too, and join codes are found by their hash (#1075); SVG import draws each pattern tile once per scale, within the layer budget (#1080); Vietnamese letters typed with combining marks (decomposed) take one font that has every mark, instead of missing-glyph boxes beside the letter (#1478); comment.list reports each message's @mentions (#1415); mask groups draw only over the area they can paint, and nested isolated layers share one memory budget (#1061); components can't contain their own instances, directly or through other components; `edit.duplicate` returns an id map with `map: true` (#995); the batches kept for reconnecting apps are bounded in bytes by default, and `figcraft-server` takes `max_log_mb` (#1069); MCP and control calls name their document (#998); sibling placement in live sessions searches instead of counting (#1065); an AI agent attached over MCP comments under its own name, with an Agent label (#1414); the instance-swap menus have a search field (#1338); a refused batch's fixes take time linear in the batch (#1063); the connector tool shows the connector it will make while dragging, ends snapping to attach points (#1387); large images on the canvas draw from a smaller copy, so two 16k images no longer lag (#1483); a file's JPEG thumbnail can't crash the home screen (#1491's progressive JPEG check, now shared); `.fig` import: library text styles linked by asset key, and override rows restating a nested level (#1234); `.fig` import keeps the file's saved line breaks while the text and its box are unedited (#1236); Download a missing free font from the Missing fonts window (`font.download`, #754); the web build's wasm back under its 24 MiB gate (#1472); shadows of images with transparent pixels follow their alpha (#1289); Crop image: Object ▸ Crop image, the right-click menu and the image fill row enter crop mode (`image.crop`, #1464); offline Penpot 2.x import/export (#1143); optional batch continue-on-error with per-step outcomes (#1386); the Design panel's sections on one grid (#858); FigJam outer shape outlines and sticky body fallback beside author text (#1341, #1397); images placed at their own size, 2x screenshots in points (#1360); the toolbar on one 32 pt grid, without its Resources button (#1210); M4: zero-length round-capped dashes draw dots; dash expansion uses the existing safety budget; extracted from #1143; Arabic draft catalog; scaled instances keep their scale when their main is rebuilt (#922); .fig embedded images are copied once per blob, not once per hash (#1059); page ruler guides survive save and reopen (#1476); colour picker LCH and OKLCH models; non-ASCII layer names transliterate in code export (#1235); branches in live sessions and branch history entries, gap 17; plugin widgets take text input and a property menu (plugin API 4); boards: templates and tables; boards: a shared timer and voting sessions; boards: the dot grid, connector ends dragged to re-attach, curved connectors; Installed weights of an embedded font family (#1246); `.sketch` import, best effort (Sketch 48 and newer); CJK line breaking: JIS X 4051's line-start and line-end sets completed with the halfwidth small kana, the halfwidth and spacing (semi-)voiced sound marks, the iteration marks 〱–〵 and the halfwidth parentheses, with behaviour cases for each; Adobe Fonts found on macOS (#298); egui 0.37 hidden-capture deadline contract; Dev Mode measurements kept on the canvas and a read-only layers panel; boards: a board area row and progress entry; community fixes in #507, #510, #529 and #530: auto-save, a text box by dragging, batch step references, removing someone from a live session, Russian and Turkish catalogs reviewed, Czech and Indonesian drafts, the Japanese draft completed, zoom previews, the web clipboard, Alt-drag snapping and Cmd+D repeat, recovery copies, crash and rollback fixes, colour emoji, `.fig` import fixes; earlier changes are in the revision history below) · **Target:** Figma Design (desktop app 126.10.7) FigCraft is a clean-room, open-source, pure-Rust reimplementation of the Figma design workflow, native on macOS, Windows, Linux and FreeBSD and in the browser, fully drivable by agents through @@ -355,6 +355,7 @@ Ranked; detail and milestones in [`docs/roadmap.md`](docs/roadmap.md). | Date | Change | Summary | |---|---|---| +| 2026-10-30 | minor | M9, #1525: share links can comment. A third access level, `comment`, sits between `edit` and `view`: someone who joins with a comment link (or its join code) sees the file live, as with a view link, and adds comment threads, replies, reactions and resolves them; any other edit is refused, by their app and by the server, which accepts a comment link's batch only when every change in it touches comment threads. Sharing online makes a comment link and code beside the edit and view ones, and the Share popover offers Can edit, Can comment and Can view; the toolbar shows a "Can comment" badge with Move, Hand and Comment. `collab.share {access: "comment"}`, `collab.manageFile {action: createLink, access: "comment"}` and `figcraft-server links create FILE --access comment` make and pick it. Older apps are unaffected: one that joins with a comment link is welcomed as a viewer, links listed to it read as view links, and a level an app doesn't know reads as view; an older server makes no comment link and sharing goes on without one. Comment authorship is the name in Preferences, as before (no accounts), so a commenter can edit or remove other people's threads. Tests: `comment_access_tests.rs` (collab), `comment_link_tests.rs` (engine, server end to end), the Share popover and toolbar in `live_share_tests.rs` and `live_server_tests.rs`. | | 2026-10-30 | minor | M1, #1390: Preferences ▸ Scroll wheel zoom (`pref.scrollWheelZoom`, off by default, kept with the preferences). On, a mouse wheel zooms the canvas about the pointer at the rate Cmd/Ctrl+wheel zooms otherwise, and Cmd/Ctrl+wheel scrolls; Shift+wheel still scrolls sideways, and trackpad scrolls, pinches and touch screens are unchanged. A wheel is told from a trackpad by what the platform reports: macOS names the device; elsewhere a wheel scrolls by lines and a trackpad by pixels, so a mouse whose wheel reports pixels (Chromium-based browsers) keeps scrolling, and a trackpad that reports lines (some X11 setups) zooms. Numbers unchanged. Tests: `wheel_zoom_tests`. | | 2026-10-30 | minor | M9, part of #1415: `comment.list` takes `mentions` (a name, any case, a leading `@` allowed) and lists only the threads where a message, the first or a reply, @mentions it, combined with the other filters (`{"mentions": "Claude", "filter": "unread"}`). It matches the names each message's `mentions` reports, by the same parser the thread view highlights with (`figcraft_doc::mentions`), so `@Ada Lovelace` is Ada Lovelace, not Ada, and an email address mentions nobody; the name counts as one of the file's people, so an unknown name can be several words. Numbers unchanged. Tests: `comment_mentions_tests`. | | 2026-10-30 | minor | M8, #1123: a safety net for internal errors in the browser. wasm32 can't catch a panic (stable Rust has no unwinding std for it), so there the command guard and the frame guard do nothing and a panic stopped the page, losing every unsaved change. The page now follows its files with unsaved changes after every frame (shared copies, taken again only when a file changed); its panic hook writes them to the page's local storage as `.figcraft` copies, as many as fit (browsers give a page about 5 MB), and says on the page that it stopped and what reloading brings back; the next page opens them again as unsaved files (`file.recover` also takes the copy as base64 `data`). Numbers unchanged. Tests: `recovery_tests` (figcraft-web). | diff --git a/apps/figcraft-server/src/cli.rs b/apps/figcraft-server/src/cli.rs index d92689df6..02bdfbed9 100644 --- a/apps/figcraft-server/src/cli.rs +++ b/apps/figcraft-server/src/cli.rs @@ -29,7 +29,7 @@ Usage: figcraft-server files [list] [--sort size|age|idle] the files kept (biggest, oldest or longest unopened first) figcraft-server files remove FILE remove a file and its links for good figcraft-server links list FILE a file's share links - figcraft-server links create FILE --access edit|view [--label TEXT] + figcraft-server links create FILE --access edit|comment|view [--label TEXT] figcraft-server links revoke FILE LINK revoke a link (whoever joined with it is disconnected) figcraft-server plugins [list] the plugin directory: every plugin and its versions figcraft-server plugins remove ID [--version V] unpublish a plugin (every version, or one) @@ -124,7 +124,7 @@ pub fn parse(args: &[String]) -> Result { "--listen" => config.listen = value("--listen")?, "--data" => config.data = PathBuf::from(value("--data")?), "--public-url" => config.public_url = Some(value("--public-url")?), - "--access" => access = Some(Access::parse(&value("--access")?).ok_or("--access must be edit or view")?), + "--access" => access = Some(Access::parse(&value("--access")?).ok_or("--access must be edit, comment or view")?), "--label" => label = value("--label")?, "--rotate" => rotate = true, "--sort" => { @@ -147,7 +147,7 @@ pub fn parse(args: &[String]) -> Result { ["files", "remove", file] => Command::RemoveFile { file: file.to_string() }, ["links", "list", file] => Command::Links { file: file.to_string() }, ["links", "create", file] => { - Command::CreateLink { file: file.to_string(), access: access.ok_or("links create needs --access edit or --access view")?, label } + Command::CreateLink { file: file.to_string(), access: access.ok_or("links create needs --access edit, comment or view")?, label } } ["links", "revoke", file, link] => Command::Revoke { file: file.to_string(), link: link.to_string() }, ["plugins"] | ["plugins", "list"] => Command::Plugins, @@ -361,8 +361,7 @@ fn new_link(cfg: &Config, l: &NewLink) -> Result { }; let invite = Invite { server: public_endpoint(cfg, listen)?, file: l.link.file.clone(), token: l.token.clone() }; let mut out = String::new(); - let what = if l.link.access == Access::View { "view" } else { "edit" }; - let _ = writeln!(out, "New {what} link {} for file {}:", l.link.id, l.link.file); + let _ = writeln!(out, "New {} link {} for file {}:", l.link.access.id(), l.link.id, l.link.file); let _ = writeln!(out, " {}", invite.link()); let _ = writeln!(out, " {}", invite.url()); let _ = writeln!(out, "This is the only time the link is shown (the server keeps only a hash of its token)."); diff --git a/apps/figcraft-server/src/codes_tests.rs b/apps/figcraft-server/src/codes_tests.rs index e226d2ec3..cbc7f70a7 100644 --- a/apps/figcraft-server/src/codes_tests.rs +++ b/apps/figcraft-server/src/codes_tests.rs @@ -57,7 +57,7 @@ fn only_hashes_on_disk(dir: &std::path::Path, file: &str, wrapped: &str, edit: & assert!(!links.contains(secret), "{links}"); } assert!(!links.contains(wrapped.rsplit('=').next().unwrap()), "the wrapped token isn't the token"); - assert_eq!(links.matches("\"code\"").count(), 2); + assert_eq!(links.matches("\"code\"").count(), 3); } #[test] @@ -76,7 +76,8 @@ fn a_file_shared_online_is_joined_by_its_codes() { assert_eq!(shared["code"], edit.as_str(), "the code shown lets people edit unless asked otherwise"); let p = a.live().published.clone().unwrap(); assert!(p.owner.is_some(), "the owner token is kept as before"); - // Others get links of their own (the owner keeps the file's first ones): four in all. + // Others get links of their own (edit, view and comment; the owner keeps the file's first + // ones): five in all. let edit_link = shared["share"]["editLink"].as_str().unwrap().to_string(); assert!(edit_link != p.edit && shared["share"]["viewLink"] != p.view.as_str() && shared["share"]["shared"] == true); let ids = |dir: &std::path::Path| -> Vec { @@ -84,7 +85,7 @@ fn a_file_shared_online_is_joined_by_its_codes() { links.as_array().unwrap().iter().map(|l| l["id"].as_str().unwrap().to_string()).collect() }; let before_stop = ids(dir.path()); - assert_eq!(before_stop.len(), 4); + assert_eq!(before_stop.len(), 5); // On disk: hashes only, of the codes as of the links' tokens. only_hashes_on_disk(dir.path(), &p.file, &p.edit, &edit, &view); // Someone types the edit code, the way people type. @@ -142,7 +143,7 @@ fn a_file_shared_online_is_joined_by_its_codes() { let (edit2, link2) = (again["code"].as_str().unwrap().to_string(), again["share"]["editLink"].as_str().unwrap().to_string()); assert!(is_code(&edit2) && edit2 != edit && link2 != edit_link && again["published"]["file"] == p.file.as_str(), "{again}"); assert_eq!(dir.path().join("files").read_dir().unwrap().count(), 1, "still one file on the server"); - assert_eq!(ids(dir.path()).len(), 4); + assert_eq!(ids(dir.path()).len(), 5); ex(&mut b, "collab.join", json!({"code": edit2})); pump(&mut [&mut a, &mut b], "Bo back in", |s| live(s[1]) && layers(s[1]) == before + 3 && s[0].live().peers.len() == 1); // The old code is still gone. diff --git a/apps/figcraft-server/src/comment_link_tests.rs b/apps/figcraft-server/src/comment_link_tests.rs new file mode 100644 index 000000000..50af8a012 --- /dev/null +++ b/apps/figcraft-server/src/comment_link_tests.rs @@ -0,0 +1,145 @@ +//! Comment links end to end over loopback: sharing online makes a comment link and code beside the +//! edit and view ones; someone who joins with it comments and everyone sees it, their app refuses +//! their other edits, and the server refuses them too for a client that skips the check; a client +//! that can't read comment links gets in as a viewer, and links listed to it read as view links. + +use std::time::Duration; + +use figcraft_collab::message::{Access, Admin, AdminReply, ClientMsg, Manage, PROTOCOL, ServerMsg}; +use figcraft_collab::{COMMENT_ONLY, Change, Client, Entry, Json}; +use figcraft_collab_net::{Endpoint, Invite, NetLimits, manage}; +use figcraft_doc::{Comment, Message}; +use figcraft_engine::Session; +use serde_json::{Value, json}; + +use crate::test_util::{Raw, TempDir, config, join, key, live, publish, pump, session}; + +fn ex(s: &mut Session, cmd: &str, p: Value) -> Value { + s.execute(cmd, &p).unwrap_or_else(|e| panic!("{cmd}: {e}")) +} + +/// The welcome `raw` gets after saying hello with `client_version` through `link`. +fn welcome(raw: &mut Raw, link: &str, client_version: &str) -> Client { + let invite = Invite::parse(link).unwrap(); + let ClientMsg::Hello(mut h) = Client::hello(&invite.file, &invite.token, "Raw", "") else { panic!("not a hello") }; + h.client_version = client_version.into(); + raw.send(&ClientMsg::Hello(h)); + loop { + if let w @ ServerMsg::Welcome { .. } = raw.next() { + return Client::join(w).unwrap(); + } + } +} + +/// What the server says to the batch `raw` sends next. +fn answer(raw: &mut Raw) -> ServerMsg { + loop { + if let m @ (ServerMsg::Reject { .. } | ServerMsg::Changes(_)) = raw.next() { + return m; + } + } +} + +#[test] +fn a_comment_link_comments_and_the_server_refuses_its_edits() { + let dir = TempDir::new("comment-link"); + let mut server = crate::start(&config(dir.path())).unwrap(); + let addr = server.addr(); + let mut a = session("Ada"); + ex(&mut a, "shape.rectangle", json!({"x": 0, "y": 0, "width": 80, "height": 80})); + let p = publish(&mut a, dir.path(), addr); + let file = Invite::parse(&p.edit).unwrap().file; + let endpoint = Endpoint::plain(addr.to_string()); + let ask = |req: Admin| manage::request(&endpoint, &key(dir.path()), vec![req], &NetLimits::default(), Duration::from_secs(5)).unwrap().remove(0); + let AdminReply::Link(made) = ask(Admin::CreateLink { file: file.clone(), access: Access::Comment, label: "Client".into() }) else { panic!() }; + assert_eq!(made.link.access, Access::Comment); + let link = Invite { server: endpoint.clone(), file: file.clone(), token: made.token.clone() }.link(); + // Cy joins with it and comments; Ada sees the thread. + let mut c = join(&link, "Cy"); + pump(&mut [&mut a, &mut c], "Cy in", |s| live(s[1]) && s[0].live().peers.len() == 1); + assert_eq!((&c.live_summary()["access"], &c.live_summary()["readOnly"]), (&json!("comment"), &json!(true))); + let thread = ex(&mut c, "comment.add", json!({"x": 10, "y": 10, "text": "Bigger logo?"}))["id"].as_u64().unwrap(); + pump(&mut [&mut a, &mut c], "the thread at Ada", |s| s[0].doc().unwrap().comment(thread).is_some()); + ex(&mut a, "comment.reply", json!({"id": thread, "text": "On it"})); + pump(&mut [&mut a, &mut c], "the reply at Cy", |s| s[1].doc().unwrap().comment(thread).is_some_and(|t| t.replies.len() == 1)); + ex(&mut c, "comment.resolve", json!({"id": thread})); + pump(&mut [&mut a, &mut c], "resolved at Ada", |s| s[0].doc().unwrap().comment(thread).is_some_and(|t| t.resolved)); + // Cy's app refuses anything else before it is sent. + let e = c.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 5, "height": 5})).unwrap_err().to_string(); + assert!(e.contains("Can comment"), "{e}"); + // A client that skips the app's check: its comment goes through, its edit is refused. + let mut raw = Raw::connect(addr); + let mut client = welcome(&mut raw, &link, &figcraft_collab::message::client_version()); + assert_eq!(client.access(), Access::Comment); + let mut d = client.document().clone(); + let page = d.first_page().unwrap(); + let id = d.new_comment_id(); + d.comments.push(Comment { + id, + page, + x: 1.0, + y: 1.0, + anchor: None, + first: Message::new("Raw", "Hi", 0), + resolved: false, + replies: vec![], + read_by: None, + }); + raw.send(&client.edit(&d).unwrap().unwrap()); + let ServerMsg::Changes(acc) = answer(&mut raw) else { panic!("refused") }; + client.receive(ServerMsg::Changes(acc), &figcraft_collab::NoRecompute).unwrap(); + let mut d = client.document().clone(); + d.node_mut(page).unwrap().name = "Sneaky".into(); + raw.send(&client.edit(&d).unwrap().unwrap()); + assert!(matches!(answer(&mut raw), ServerMsg::Reject { reason, .. } if reason == COMMENT_ONLY)); + // So is a forged batch that hides an edit among comment changes. + let forged = Change::Put { entry: Entry::Comment(id), value: Json::of(&client.document().comments[0]).unwrap() }; + let rename = Change::SetDoc { prop: figcraft_collab::DocProp::Name, value: Json::of("Sneaky").unwrap() }; + raw.send(&ClientMsg::Changes { base_seq: 0, batch_id: 99, changes: vec![forged, rename] }); + assert!(matches!(answer(&mut raw), ServerMsg::Reject { reason, .. } if reason == COMMENT_ONLY)); + pump(&mut [&mut a], "the raw comment, nothing else", |s| { + let d = s[0].doc().unwrap(); + d.comment(id).is_some() && d.node(page).is_some_and(|n| n.name != "Sneaky") && d.name != "Sneaky" + }); + // A client from before comment links gets in as a viewer. + let mut old = Raw::connect(addr); + assert_eq!(welcome(&mut old, &link, "0.12.1 signals hosting").access(), Access::View); + // And lists the link as a view link. + let mut m = Raw::connect(addr); + m.send(&ClientMsg::Manage(Manage { protocol: PROTOCOL, key: key(dir.path()), client_version: "0.12.1".into() })); + assert!(matches!(m.next(), ServerMsg::Admin(r) if *r == AdminReply::Ready)); + m.send(&ClientMsg::Admin(Admin::Links { file: file.clone() })); + let ServerMsg::Admin(r) = m.next() else { panic!("no listing") }; + let AdminReply::Links(links) = *r else { panic!("{r:?}") }; + assert!(links.iter().all(|l| l.access != Access::Comment) && links.iter().any(|l| l.id == made.link.id && l.access == Access::View)); + let AdminReply::Links(links) = ask(Admin::Links { file }) else { panic!() }; + assert!(links.iter().any(|l| l.id == made.link.id && l.access == Access::Comment), "this build reads it"); + server.stop(); +} + +#[test] +fn sharing_online_offers_a_comment_code() { + let dir = TempDir::new("comment-code"); + let mut server = crate::start(&config(dir.path())).unwrap(); + let url = format!("ws://{}/?key={}", server.addr(), key(dir.path())); + let mut a = session("Ada"); + ex(&mut a, "collab.share", json!({"server": url})); + pump(&mut [&mut a], "the share", |s| live(s[0]) && s[0].live().published.is_some() && s[0].live_summary()["share"]["shared"] == true); + let shared = a.live_summary()["share"].clone(); + let comment = shared["commentCode"].as_str().unwrap().to_string(); + assert!(comment != shared["editCode"] && comment != shared["viewCode"], "{shared}"); + assert_eq!(ex(&mut a, "collab.share", json!({"access": "comment"}))["code"], comment.as_str()); + // Someone types it: they can comment, not edit. + let mut c = session("Cy"); + ex(&mut c, "pref.publishServer", json!({"url": format!("ws://{}/", server.addr())})); + ex(&mut c, "collab.join", json!({"code": comment})); + pump(&mut [&mut a, &mut c], "Cy in", |s| live(s[1]) && s[0].live().peers.len() == 1); + assert_eq!(c.live_summary()["access"], "comment"); + let thread = ex(&mut c, "comment.add", json!({"x": 10, "y": 10, "text": "Looks good"}))["id"].as_u64().unwrap(); + pump(&mut [&mut a, &mut c], "the thread at Ada", |s| s[0].doc().unwrap().comment(thread).is_some()); + assert!(c.execute("shape.ellipse", &json!({"x": 0, "y": 0, "width": 5, "height": 5})).is_err()); + // Stopping revokes it with the others: Cy is out. + ex(&mut a, "collab.stop", json!({})); + pump(&mut [&mut a, &mut c], "Cy out", |s| !s[1].live().status.active() && s[0].live().peers.is_empty()); + server.stop(); +} diff --git a/apps/figcraft-server/src/lib.rs b/apps/figcraft-server/src/lib.rs index 746205d1b..ce5eefdc7 100644 --- a/apps/figcraft-server/src/lib.rs +++ b/apps/figcraft-server/src/lib.rs @@ -95,6 +95,8 @@ mod cli_tests; #[cfg(test)] mod codes_tests; #[cfg(test)] +mod comment_link_tests; +#[cfg(test)] mod directory_tests; #[cfg(test)] mod hardening_tests; diff --git a/apps/figcraft-server/src/owned_tests.rs b/apps/figcraft-server/src/owned_tests.rs index b4df05024..767b9e39f 100644 --- a/apps/figcraft-server/src/owned_tests.rs +++ b/apps/figcraft-server/src/owned_tests.rs @@ -158,7 +158,7 @@ fn only_a_files_own_token_learns_about_it() { assert_eq!(list.len(), 5, "one answer per pair, in order"); let first = list[0].as_ref().expect("its own token"); assert_eq!(first.name, "Untitled"); - assert_eq!(first.links.len(), 4, "the owner's two links and the two made for others"); + assert_eq!(first.links.len(), 5, "the owner's two links and the three made for others"); assert!(first.expires.is_some()); assert!(list[1..].iter().all(Option::is_none), "a wrong token and a missing file read the same"); // Nothing asked: nothing answered; too many: refused, and the connection stays usable. diff --git a/crates/collab-net/src/exchange.rs b/crates/collab-net/src/exchange.rs index a01e9d598..b0d103ae1 100644 --- a/crates/collab-net/src/exchange.rs +++ b/crates/collab-net/src/exchange.rs @@ -53,9 +53,7 @@ impl Exchange { /// `file` with its `owner` token. pub(crate) fn new(server: Endpoint, key: Option<&str>, file: &str, owner: &str, requests: Vec) -> Self { let open = match key { - Some(key) => { - ClientMsg::Manage(Manage { protocol: PROTOCOL, key: key.to_string(), client_version: env!("CARGO_PKG_VERSION").to_string() }) - } + Some(key) => ClientMsg::Manage(Manage { protocol: PROTOCOL, key: key.to_string(), client_version: message::client_version() }), None => ClientMsg::OpenHosting(OpenHosting { protocol: PROTOCOL, client_version: message::client_version(), @@ -144,7 +142,7 @@ pub(crate) fn publish(job: Publish) -> Next { // The file's first links stay its owner's (this app joins with one): others get their // own, which stopping takes away again. let owner = published.owner.unwrap_or_default(); - let then: Box, String>) -> Next> = Box::new(move |made| { + let then: LinksThen = Box::new(move |made| { events.push(Event::Shared(made.and_then(|s| s.ok_or_else(|| "the server didn't make the links".to_string())))); Next::Done(events, Some(edit)) }); @@ -163,26 +161,45 @@ struct Sharing { create: bool, } -/// Revoke the links `s.revoke` names, then (to create) make new ones and ask for their codes on a -/// connection of their own; `then` gets the links. -fn share_links(s: Sharing, then: Box, String>) -> Next>) -> Next { +/// What [`share_links`] hands the links to. +type LinksThen = Box, String>) -> Next>; + +/// Revoke the links `s.revoke` names, then (to create) make new ones, then a comment link and their +/// codes, each on a connection of its own; `then` gets the links. +fn share_links(s: Sharing, then: LinksThen) -> Next { let ex = Exchange::new(s.server.clone(), s.key.as_deref(), &s.file, &s.owner, requests::link_requests(&s.file, &s.revoke, s.create)); Next::Ask( Box::new(ex), - Box::new(move |answers| { - let links = match answers.and_then(|a| requests::made_links(a.replies, s.create)) { - Ok(Some(links)) => links, - other => return then(other.map(|_| None)), - }; - let ex = Exchange::new(s.server.clone(), s.key.as_deref(), &s.file, &s.owner, requests::code_requests(&s.file, &links)); - Next::Ask( - Box::new(ex), - Box::new(move |codes| { - // A server from before join codes: the links are shared without them. - let codes = codes.map_err(|e| log::info!("live session: no join codes from {}: {e}", s.server.display())).unwrap_or_default(); - then(Ok(Some(requests::shared(&s.server, &s.file, links, codes.replies)))) - }), - ) + Box::new(move |answers| match answers.and_then(|a| requests::made_links(a.replies, s.create)) { + Ok(Some(links)) => comment_link(s, links, then), + other => then(other.map(|_| None)), + }), + ) +} + +/// Ask for a comment link beside `links`, then for their codes. +fn comment_link(s: Sharing, mut links: requests::Links, then: LinksThen) -> Next { + let ex = Exchange::new(s.server.clone(), s.key.as_deref(), &s.file, &s.owner, requests::comment_request(&s.file)); + Next::Ask( + Box::new(ex), + Box::new(move |made| { + // A server from before comment links: the file is shared without one. + let made = made.map_err(|e| log::info!("live session: no comment link from {}: {e}", s.server.display())); + links.comment = made.ok().and_then(|a| requests::made_comment(a.replies)); + codes(s, links, then) + }), + ) +} + +/// Ask for the codes of `links`; `then` gets them with their codes. +fn codes(s: Sharing, links: requests::Links, then: LinksThen) -> Next { + let ex = Exchange::new(s.server.clone(), s.key.as_deref(), &s.file, &s.owner, requests::code_requests(&s.file, &links)); + Next::Ask( + Box::new(ex), + Box::new(move |codes| { + // A server from before join codes: the links are shared without them. + let codes = codes.map_err(|e| log::info!("live session: no join codes from {}: {e}", s.server.display())).unwrap_or_default(); + then(Ok(Some(requests::shared(&s.server, &s.file, links, codes.replies)))) }), ) } diff --git a/crates/collab-net/src/exchange_tests.rs b/crates/collab-net/src/exchange_tests.rs index 0e38039af..85274e411 100644 --- a/crates/collab-net/src/exchange_tests.rs +++ b/crates/collab-net/src/exchange_tests.rs @@ -129,6 +129,11 @@ fn make_links(d: &mut Driver