diff --git a/apps/ely-pdf/src/annotate.rs b/apps/ely-pdf/src/annotate.rs index 2fec6cb..99cbb90 100644 --- a/apps/ely-pdf/src/annotate.rs +++ b/apps/ely-pdf/src/annotate.rs @@ -12,7 +12,7 @@ use crate::pages::{rect_on, to_view}; use crate::view::PdfView; /// How far a click may move, in window pixels. -const CLICK: f32 = 3.0; +pub(crate) const CLICK: f32 = 3.0; /// Pages read for their annotations at once. const BATCH: usize = 20; @@ -253,8 +253,9 @@ impl PdfView { if annots.is_empty() { return; } + let author = ely_ui::settings::current(cx).author.clone(); self.settled_edit(what, cx, move |worker, doc| { - let name = ely_platform::user::name()?; + let name = ely_platform::user::named(&author)?; annots.iter_mut().for_each(|annot| annot.author = name.clone()); worker.annotate(doc, &annots) }); diff --git a/crates/ely-lok/src/client.rs b/crates/ely-lok/src/client.rs index 78d8f46..f95e2a2 100644 --- a/crates/ely-lok/src/client.rs +++ b/crates/ely-lok/src/client.rs @@ -16,6 +16,7 @@ mod doc; mod feed; mod frames; mod install; +mod load; pub use install::{install_dir, templates}; use install::worker_bin; pub use doc::{Pdf, Sheet, Sheets}; @@ -37,6 +38,8 @@ pub struct Worker { session: u64, stderr: ely_ipc::Relay, check: Check, + /// The name new comments sign with, once set. + author: Option, } /// The start-up check: to run, passed, or why it failed. @@ -188,6 +191,7 @@ impl Worker { session: SESSIONS.fetch_add(1, std::sync::atomic::Ordering::SeqCst), stderr, check: Check::Pending(profile_dir.to_path_buf()), + author: None, }; Ok(worker) } @@ -201,49 +205,15 @@ impl Worker { /// The name new comments and tracked changes carry. pub fn set_author(&mut self, name: &str) -> io::Result<()> { log::info!("lok: the author is set"); + // Wanted even if this worker fails; a restart sends it. + self.author = Some(name.to_string()); self.request(tag::SET_AUTHOR, &Pack::new().str(name).bytes())?; Ok(()) } - /// Loads `url`; macros never run (LOK-11). - pub fn load(&mut self, url: &str) -> io::Result { - self.unlock(url, "") - } - - /// Loads text `url` as a sheet, read as `how` says. - pub fn import(&mut self, url: &str, how: &crate::sheet::csv::Import) -> io::Result { - use crate::sheet::csv::{COLUMNS, ENCODINGS}; - let at = |found: Option| found.expect("a listed choice") as u64; - let pack = Pack::new() - .str(url) - .u64(u64::from(how.separator)) - .u64(how.quote.map_or(0, u64::from)) - .u64(at(ENCODINGS.iter().position(|e| *e == how.encoding))) - .u64(how.columns.len() as u64); - let pack = how.columns.iter().fold(pack, |pack, column| { - pack.u64(at(COLUMNS.iter().position(|c| c == column))) - }); - let payload = self.request(tag::IMPORT, &pack.bytes())?; - let mut reply = Unpack::new(&payload); - let (id, kind) = (reply.u64()?, reply.u64()?); - self.feed.opened(id); - Ok(Doc { worker: self.session, id, rev: 0, kind }) - } - - /// Loads `url` with its password; a locked one is PermissionDenied. - pub fn unlock(&mut self, url: &str, password: &str) -> io::Result { - let request = Pack::new().str(url).str(password).bytes(); - let payload = self.request(tag::LOAD, &request).map_err(|error| { - if error.to_string() == crate::ipc::LOCKED { - io::Error::new(io::ErrorKind::PermissionDenied, error.to_string()) - } else { - error - } - })?; - let mut reply = Unpack::new(&payload); - let (id, kind) = (reply.u64()?, reply.u64()?); - self.feed.opened(id); - Ok(Doc { worker: self.session, id, rev: 0, kind }) + /// The name last asked for; a restart signs with it. + pub fn author(&self) -> Option<&str> { + self.author.as_deref() } /// Runs a macro by its URL (LOK-11 probe). diff --git a/crates/ely-lok/src/client/load.rs b/crates/ely-lok/src/client/load.rs new file mode 100644 index 0000000..4c80f38 --- /dev/null +++ b/crates/ely-lok/src/client/load.rs @@ -0,0 +1,51 @@ +//! Loading documents: by URL, with a password, or as text. + +use std::io; + +use super::{Doc, Worker}; +use crate::ipc::{Pack, Unpack, tag}; + +impl Worker { + /// Loads `url`; macros never run (LOK-11). + pub fn load(&mut self, url: &str) -> io::Result { + self.unlock(url, "") + } + + /// Loads text `url` as a sheet, read as `how` says. + pub fn import(&mut self, url: &str, how: &crate::sheet::csv::Import) -> io::Result { + use crate::sheet::csv::{COLUMNS, ENCODINGS}; + let at = |found: Option| found.expect("a listed choice") as u64; + let pack = Pack::new() + .str(url) + .u64(u64::from(how.separator)) + .u64(how.quote.map_or(0, u64::from)) + .u64(at(ENCODINGS.iter().position(|e| *e == how.encoding))) + .u64(how.columns.len() as u64); + let pack = how.columns.iter().fold(pack, |pack, column| { + pack.u64(at(COLUMNS.iter().position(|c| c == column))) + }); + let payload = self.request(tag::IMPORT, &pack.bytes())?; + self.opened(&payload) + } + + /// Loads `url` with its password; a locked one is PermissionDenied. + pub fn unlock(&mut self, url: &str, password: &str) -> io::Result { + let request = Pack::new().str(url).str(password).bytes(); + let payload = self.request(tag::LOAD, &request).map_err(|error| { + if error.to_string() == crate::ipc::LOCKED { + io::Error::new(io::ErrorKind::PermissionDenied, error.to_string()) + } else { + error + } + })?; + self.opened(&payload) + } + + /// A load's reply: the document's number and kind. + fn opened(&mut self, payload: &[u8]) -> io::Result { + let mut reply = Unpack::new(payload); + let (id, kind) = (reply.u64()?, reply.u64()?); + self.feed.opened(id); + Ok(Doc { worker: self.session, id, rev: 0, kind }) + } +} diff --git a/crates/ely-office/src/lok.rs b/crates/ely-office/src/lok.rs index bfea7b8..82d670d 100644 --- a/crates/ely-office/src/lok.rs +++ b/crates/ely-office/src/lok.rs @@ -77,9 +77,14 @@ impl Engine for Lok { } fn restart(app: AppId, worker: &mut Worker) -> io::Result<()> { - *worker = spawn(app)?; - worker.ready()?; - worker.set_author(&ely_platform::user::name()?) + let author = worker.author().map(str::to_string); + let author = author.ok_or_else(|| io::Error::other("the ended worker had no author"))?; + // The ended worker stays until the new one is ready. + let mut fresh = spawn(app)?; + fresh.ready()?; + fresh.set_author(&author)?; + *worker = fresh; + Ok(()) } fn settled(view: &LokView) -> bool { @@ -129,10 +134,13 @@ pub fn start(app: AppId, cx: &mut App) -> io::Result<(Arc>, PathBu let worker = Arc::new(Mutex::new(spawn(app)?)); // The window opens now; the kernel finishes starting behind it. let starting = Arc::clone(&worker); + // Settings as read at launch; defaults if unreadable. + let wanted = Arc::new(Mutex::new(ely_ui::settings::current(cx).author.clone())); + let first = Arc::clone(&wanted); let started = cx.background_executor().spawn(async move { let mut worker = starting.lock().expect("the worker lock"); worker.ready()?; - worker.set_author(&ely_platform::user::name()?) + sign(&mut worker, &first) }); cx.spawn(async move |_| { if let Err(error) = started.await { @@ -140,10 +148,16 @@ pub fn start(app: AppId, cx: &mut App) -> io::Result<(Arc>, PathBu } }) .detach(); - follow_author(&worker, cx); + follow_author(&worker, wanted, cx); Ok((worker, work)) } +/// Sends the newest wanted name, read under the worker lock. +fn sign(worker: &mut Worker, wanted: &Mutex) -> io::Result<()> { + let author = wanted.lock().expect("the author lock").clone(); + worker.set_author(&ely_platform::user::named(&author)?) +} + /// A worker process on the app's profile, not yet checked. fn spawn(app: AppId) -> io::Result { let data = ely_platform::data_dir(app).map_err(io::Error::other)?; @@ -154,19 +168,19 @@ fn spawn(app: AppId) -> io::Result { } /// A new author name signs from the next comment on. -fn follow_author(worker: &Arc>, cx: &mut App) { +fn follow_author(worker: &Arc>, wanted: Arc>, cx: &mut App) { let worker = Arc::clone(worker); - let mut signed = ely_ui::settings::current(cx).author.clone(); cx.observe_global::(move |cx| { let author = ely_ui::settings::current(cx).author.clone(); - if author == signed { + if *wanted.lock().expect("the author lock") == author { return; } - signed = author; - let worker = Arc::clone(&worker); + *wanted.lock().expect("the author lock") = author; + let (worker, wanted) = (Arc::clone(&worker), Arc::clone(&wanted)); cx.background_executor() .spawn(async move { - let set = ely_platform::user::name().and_then(|name| worker.lock().expect("the worker lock").set_author(&name)); + // Any order of these tasks ends on the newest name. + let set = sign(&mut worker.lock().expect("the worker lock"), &wanted); if let Err(error) = set { log::error!("lok: the new author name is not set: {error}"); } diff --git a/crates/ely-office/tests/restart.rs b/crates/ely-office/tests/restart.rs new file mode 100644 index 0000000..5e917db --- /dev/null +++ b/crates/ely-office/tests/restart.rs @@ -0,0 +1,36 @@ +//! A failed engine restart keeps the author for retries. + +use ely_office::{Engine as _, Lok}; +use ely_platform::AppId; + +#[test] +fn a_restart_signs_with_the_newest_name_and_survives_a_failure() { + let Ok(install) = ely_lok::install_dir() else { + eprintln!("SKIP: no LOK build"); + return; + }; + let home = tempfile::tempdir().expect("a temp dir"); + // The one test here: nothing else reads HOME. + unsafe { std::env::set_var("HOME", home.path()) }; + let profile = ely_platform::data_dir(AppId::Doc).expect("a data dir").join("lok-profile"); + std::fs::create_dir_all(&profile).expect("the profile"); + let mut worker = ely_lok::Worker::launch(&install, &profile).expect("the worker"); + worker.ready().expect("checks"); + worker.set_author("Ada").expect("signs"); + // The new worker's check cannot write its sample here. + let blocker = profile.join("lok-verify.fodt"); + if blocker.exists() { + std::fs::remove_file(&blocker).expect("the last sample goes"); + } + std::fs::create_dir(&blocker).expect("a folder in the way"); + assert!(Lok::restart(AppId::Doc, &mut worker).is_err(), "the check fails"); + assert_eq!(worker.author(), Some("Ada"), "the author survives a failed restart"); + std::fs::remove_dir(&blocker).expect("cleared"); + Lok::restart(AppId::Doc, &mut worker).expect("the next try restarts"); + assert_eq!(worker.author(), Some("Ada")); + // A name asked while down signs after the restart. + worker.shutdown().expect("stops"); + assert!(worker.set_author("Latest").is_err(), "a stopped engine cannot take it"); + Lok::restart(AppId::Doc, &mut worker).expect("restarts"); + assert_eq!(worker.author(), Some("Latest"), "the newest name signs"); +} diff --git a/crates/ely-platform/src/user.rs b/crates/ely-platform/src/user.rs index b5c06bd..b9edda7 100644 --- a/crates/ely-platform/src/user.rs +++ b/crates/ely-platform/src/user.rs @@ -2,11 +2,10 @@ use std::io; -/// The author name from settings, else the account's. -pub fn name() -> io::Result { - let set = crate::settings::Shared::load()?.author; - if !set.is_empty() { - return Ok(set); +/// `author` as the settings give it, else the account's name. +pub fn named(author: &str) -> io::Result { + if !author.is_empty() { + return Ok(author.to_string()); } account() } @@ -61,8 +60,10 @@ fn full_name() -> io::Result { #[cfg(test)] mod tests { #[test] - fn the_account_has_a_name() { - let name = super::name().expect("a name"); - assert!(!name.is_empty()); + fn a_set_name_signs_else_the_account() { + assert_eq!(super::named("Ada").expect("a name"), "Ada"); + let account = super::named("").expect("a name"); + assert!(!account.is_empty()); + assert_eq!(account, super::account().expect("the account")); } } diff --git a/tasks/BUGS.md b/tasks/BUGS.md index 3f7eee3..308783d 100644 --- a/tasks/BUGS.md +++ b/tasks/BUGS.md @@ -88,7 +88,7 @@ - [ ] K-03 `rect_of` 宽容解析把读不出的载荷悄悄吞掉 - [ ] K-04 `Sheets.current` 用 `as u64` 转有符号的 part - [x] X-01 设置窗口在前时 Cmd+Q 直接退出，跳过未保存询问并抹掉恢复点 — 退出一律经主窗口宿主的流程（route_quit），延后到动作结束、先把宿主窗口提到前面；R1 主窗口 Cmd+Q 不问；R2 询问被设置窗口挡住；R3 PASS -- [ ] X-02 共用 `settings.json` 读不出时 Doc、Table、Show 根本起不来 +- [x] X-02 共用 `settings.json` 读不出时 Doc、Table、Show 根本起不来 — 作者名取启动时读入的设置；R1 重启失败丢作者、脚本退出码、client.rs 超行；R2 作者更新的先后与停机期间的更新；R3 PASS - [ ] X-03 第二个实例启动即清掉运行中实例的工作目录与恢复点 - [ ] X-04 属性栏字号菜单的 `expect("sizes are numbers")` 是脆弱点，但当前不可达 - [ ] X-05 恢复点写不成时每 10 秒弹一次提示 diff --git a/tasks/reviews/X-02-r1.md b/tasks/reviews/X-02-r1.md new file mode 100644 index 0000000..325b81a --- /dev/null +++ b/tasks/reviews/X-02-r1.md @@ -0,0 +1,7 @@ +FAIL + +crates/ely-office/src/lok.rs:82 — 重启在自检、设置作者成功前就替换原 worker；新 worker 的 author 初始为 None，任一步失败都会丢失恢复所需的作者。实测以当前构建的真实 LOK worker 设置作者后停止，在隔离的 lok-profile/lok-verify.fodt 路径创建目录，再调用 Engine::restart：首次返回 IsADirectory，worker.author() 变为 None；移除目录后再次调用仍返回 “the ended worker had no author”，后续恢复永久卡在第 81 行，违反 PRD §6.3 的重新打开要求。空引擎目录导致首次启动失败时也独立复现。应先在局部变量中完成新 worker 的启动、自检与作者设置，全部成功后替换原 worker，并覆盖失败后重试的回归场景。证据：/tmp/ely-x02-restart-profile.log、/tmp/ely-x02-restart.log。 + +tests/scripts/bad-settings.sh:9 — EXIT 清理函数中的 wait 返回被 SIGTERM 终止的 ElyDoc 的状态 143，set -e 随即退出，跳过临时目录删除，并把成功测试变成失败退出码。连续两次运行 CARGO_NET_OFFLINE=true CARGO_INCREMENTAL=0 bash tests/scripts/bad-settings.sh，均打印 “ok: ElyDoc starts past an unreadable settings file, which stays as it was” 后退出 143；bash -x 确认清理止于 wait，两个临时 HOME 均遗留。应处理清理阶段预期的进程终止状态，保证清理执行完毕并保留测试本身的退出状态。证据：/tmp/ely-x02-bad-settings-trace.log；遗留目录已由本轮审查清理。 + +crates/ely-lok/src/client.rs:214 — 本次新增作者字段、记录与读取方法，使文件由 HEAD 的 495 行增长到 504 行，超过 agents.md 的每文件 500 行限制；wc -l crates/ely-lok/src/client.rs 可直接复现。应将一组现有客户端实现移入子模块，使该文件回到 500 行以内。 diff --git a/tasks/reviews/X-02-r2.md b/tasks/reviews/X-02-r2.md new file mode 100644 index 0000000..f362a7e --- /dev/null +++ b/tasks/reviews/X-02-r2.md @@ -0,0 +1,5 @@ +FAIL + +crates/ely-office/src/lok.rs:176 — 作者变更各自捕获名字并启动后台任务，执行顺序可使旧值覆盖最新设置；启动任务第 142 行也会参与这种覆盖。实测直接调用生产 start、更新 Current、经 GPUI 调度器 seed 0 执行并用真实 LOK 插入批注：启动期间由 Initial Author 改为 Latest Author，批注仍署 Initial Author；启动完成后依次改为 Earlier Author、Latest Author，批注仍署 Earlier Author，后者独立复跑一致。应让启动与后续作者更新共用有序写入或代次控制，在取得 worker 锁后核对最新请求，并覆盖这些交错的回归测试。证据：/tmp/ely-x02-r2-native-order.log、/tmp/ely-x02-r2-latest_author_after_two_changes.log；探针源码保存在 /tmp/ely-x02-r2-probe-source.log。 + +crates/ely-office/src/lok.rs:80 — 重启只沿用最后成功设置的作者，而 follow_author 第 172 行在发送成功前就记下新设置；worker 停止期间的作者更新失败后，该更新永久丢失。实测以 Initial Author 启动真实 worker，停止它，将 Current.author 改为 Latest Author 并执行观察任务，再调用 Lok::restart；重启成功后新插入的真实批注仍署 Initial Author，独立复跑一致，后续相同设置也被 signed 相等检查跳过。应保留待生效的最新作者设置，重启时应用它，并在设置成功后确认同步状态；继续保留上一轮修复的失败重试信息。增加停止期间修改作者后重启的回归测试。证据：/tmp/ely-x02-r2-latest_author_after_recovery.log、/tmp/ely-x02-r2-probe-source.log。 diff --git a/tasks/reviews/X-02-r3.md b/tasks/reviews/X-02-r3.md new file mode 100644 index 0000000..7ef22e9 --- /dev/null +++ b/tasks/reviews/X-02-r3.md @@ -0,0 +1 @@ +PASS diff --git a/tests/scripts/bad-settings.sh b/tests/scripts/bad-settings.sh new file mode 100755 index 0000000..c4815d8 --- /dev/null +++ b/tests/scripts/bad-settings.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +# Unreadable shared settings still let ElyDoc start. +set -euo pipefail + +root=$(cd "$(dirname "$0")/../.." && pwd) +[ "$(uname)" = Darwin ] || { echo "SKIP: macOS only"; exit 0; } +work=$(mktemp -d) +cleanup() { + if [ -n "${pid:-}" ]; then + # Its worker writes into the folder too. + pkill -9 -P "$pid" 2>/dev/null || true + kill "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + fi + rm -rf "$work" +} +trap cleanup EXIT + +cargo build -q -p ely-lok --bins -p ely-doc --manifest-path "$root/Cargo.toml" +shared="$work/home/Library/Application Support/ElyOffice" +mkdir -p "$shared" +printf '{"version": 2}\n' >"$shared/settings.json" +HOME="$work/home" ELY_LOG=info ELY_LOCALE=en ELY_NO_DIALOG=1 ELY_SMOKE=hold exec "$root/target/debug/ely-doc" >"$work/out" 2>&1 & +pid=$! +for _ in $(seq 90); do + grep -q "the author is set\|did not start" "$work/out" && break + kill -0 "$pid" 2>/dev/null || break + sleep 1 +done +grep -q "lok: the author is set" "$work/out" || { echo "FAIL: the engine did not start"; tail -20 "$work/out"; exit 1; } +kill -0 "$pid" 2>/dev/null || { echo "FAIL: ElyDoc exited"; tail -20 "$work/out"; exit 1; } +grep -q '{"version": 2}' "$shared/settings.json" || { echo "FAIL: the unreadable file was overwritten"; exit 1; } +echo "ok: ElyDoc starts past an unreadable settings file, which stays as it was"