diff --git a/CHANGELOG.md b/CHANGELOG.md index e59d1f22..d3b8b2d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -153,6 +153,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **`doctor` now notices a config directory it cannot write to.** With the + directory unwritable, every `tty7 new` and `tab new` failed with "could not + write the machine tree — Permission denied" and settings could not be saved, + while `tty7 doctor` printed a clean table and exited 0. Its config row says + "none yet — the defaults are the config", which is true of reading and says + nothing about writing, so the one verb whose job is catching a broken + install missed this one and `tty7 doctor || alert` never fired. There is now + a row, a line on stderr, `config.dir_writable` in the JSON, and exit 1 — and + the check writes a probe file rather than reading the mode bits, because a + read-only mount or an ACL leaves `0700` on a directory that refuses + everything. + - **A clamped setting now says so.** A hand-edited `config.json` asking for `ui_font_size: 8`, `font_size: 999` or `scrollback_limit: 5` runs at 12, 256 and 100 — clamping beats refusing the whole file over one silly number, and diff --git a/crates/tty7-cli/src/commands.rs b/crates/tty7-cli/src/commands.rs index f8366f6a..d90c3418 100644 --- a/crates/tty7-cli/src/commands.rs +++ b/crates/tty7-cli/src/commands.rs @@ -1797,6 +1797,21 @@ fn doctor(ctx: &Context, backend: &mut dyn Backend) -> Result { // Only when a copy is really there. A row that says "no tree was set // aside" on every healthy machine is noise, and this one has to read as // news. + // Whether the config directory can actually be written to, probed rather + // than inferred from its mode: a read-only mount, an ACL, or a directory + // owned by somebody else all read as writable by permission bits alone, + // and what matters is whether the write would succeed. + // + // This is the check `doctor` was missing. With the directory unwritable + // every `tty7 new` and every `tab new` fails with "could not write the + // machine tree — Permission denied", settings cannot be saved, and doctor + // reported the install healthy and exited 0: the config row says "none yet + // — the defaults are the config", which is true of reading and says + // nothing about writing. `tty7 doctor || alert` is the thing that is + // supposed to fire here. + let config_dir_writable = tty7_core::core::config::config_dir_path() + .map(|dir| dir_is_writable(&dir)) + .unwrap_or(true); let quarantined_tree = tty7_core::core::config::config_dir_path() .map(|dir| dir.join(tty7_core::core::machine::MACHINE_FILE)) .map(|path| tty7_core::core::config::quarantined_copies(&path)) @@ -1851,6 +1866,18 @@ fn doctor(ctx: &Context, backend: &mut dyn Backend) -> Result { ), ]); } + if !config_dir_writable { + rows.push(vec![ + "config dir".to_string(), + format!( + "not writable{} — settings cannot be saved and new workspaces \ + cannot be filed; fix the permissions on that directory", + tty7_core::core::config::config_dir_path() + .map(|d| format!(" ({})", d.display())) + .unwrap_or_default() + ), + ]); + } if let Some(kept) = &quarantined_tree { let newest = kept .last() @@ -1957,7 +1984,11 @@ fn doctor(ctx: &Context, backend: &mut dyn Backend) -> Result { json: json!({ "context": context_json(ctx, dangling), "server": server, - "config": { "ok": config_ok, "state": config_state }, + "config": { + "ok": config_ok, + "state": config_state, + "dir_writable": config_dir_writable, + }, "hooks": hooks_json(&hooks), }), }; @@ -1968,6 +1999,9 @@ fn doctor(ctx: &Context, backend: &mut dyn Backend) -> Result { if !config_ok { eprintln!("tty7: doctor: the config file is not being used"); } + if !config_dir_writable { + eprintln!("tty7: doctor: the config directory cannot be written to"); + } if report.json["server"]["reachable"] == false { // doctor is the verb people run when something is not working, so an // unreachable server is *the* finding — not a row to exit 0 over: @@ -1976,12 +2010,28 @@ fn doctor(ctx: &Context, backend: &mut dyn Backend) -> Result { eprintln!("tty7: doctor: the server is unreachable"); return Ok(Outcome::Exit(1, report)); } - if !config_ok { + if !config_ok || !config_dir_writable { return Ok(Outcome::Exit(1, report)); } Ok(Outcome::Report(report)) } +/// Whether a directory can be written to, by writing to it. +/// +/// The mode bits are not the question. A read-only mount, an ACL, an immutable +/// flag or a directory owned by somebody else can all leave `0700` on a +/// directory that refuses every write, and the thing being diagnosed is +/// whether the write succeeds. +/// +/// The probe is removed either way, and a directory that cannot be read at all +/// counts as unwritable — an install nobody can reach is not a healthy one. +fn dir_is_writable(dir: &std::path::Path) -> bool { + let probe = dir.join(format!(".tty7-doctor-{}", std::process::id())); + let ok = std::fs::write(&probe, b"").is_ok(); + let _ = std::fs::remove_file(&probe); + ok +} + /// Where every installable status hook stands on this machine. /// /// Agents whose state cannot be read at all are left out rather than guessed @@ -4631,6 +4681,53 @@ mod tests { assert!(out.contains("set (/cfg/tty7)"), "{out}"); } + /// The config directory is checked by writing to it, not by its mode. + /// + /// `doctor` reported a healthy install and exited 0 on a directory it + /// could not write to — while every `tty7 new` failed with "could not + /// write the machine tree — Permission denied" and settings could not be + /// saved. The config row says "none yet — the defaults are the config", + /// which is true of *reading* and says nothing about writing, so the one + /// verb whose job is to catch a broken install missed this one. + /// + /// The probe writes because the mode bits are not the question: a + /// read-only mount, an ACL, an immutable flag or another user's directory + /// all leave `0700` on something that refuses every write. + /// + /// The wiring — the row, the stderr headline, the exit 1 — is checked + /// against a live daemon rather than here, because it turns on the + /// process-wide config directory and this suite runs many threads in one + /// process. + #[test] + fn the_config_dir_check_asks_by_writing() { + let dir = std::env::temp_dir().join(format!("tty7-doctor-rw-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + assert!(dir_is_writable(&dir), "a fresh directory is writable"); + assert!( + std::fs::read_dir(&dir).unwrap().next().is_none(), + "the probe has to clean up after itself" + ); + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o500)).unwrap(); + assert!( + !dir_is_writable(&dir), + "a directory that refuses a write is not writable" + ); + std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700)).unwrap(); + assert!(dir_is_writable(&dir), "and it recovers when it is fixed"); + } + + assert!( + !dir_is_writable(&dir.join("does-not-exist")), + "a directory nobody can reach is not a healthy install either" + ); + let _ = std::fs::remove_dir_all(&dir); + } + /// `doctor --json` carries the sections the reference says it does. /// /// This is one of the three verbs whose JSON is printed even when it diff --git a/docs/cli/reference.mdx b/docs/cli/reference.mdx index 5c59620b..47a1b28e 100644 --- a/docs/cli/reference.mdx +++ b/docs/cli/reference.mdx @@ -320,14 +320,18 @@ nothing, so `tty7 agents` shows it standing still and `tty7 wait` sits there until it times out. Outdated hooks fail the same quiet way. Hooks are a local install, so under `-m` the row reads `unknown`. -JSON: `{"context":{"config_dir","workspace","pane"},"server":{"reachable","dialect_ok","build","status","routes"},"config":{"ok","state"},"hooks":{"installed","outdated","not_installed"}}` +JSON: `{"context":{"config_dir","workspace","pane"},"server":{"reachable","dialect_ok","build","status","routes"},"config":{"ok","state","dir_writable"},"hooks":{"installed","outdated","not_installed"}}` — the context fields are booleans, not values, and each `hooks` field is a list of agent slugs. `context` also carries `workspace_gone` and `pane_gone` when a server answered and could be asked; both are absent when none did, so that "it names nothing here" stays distinct from "nobody could check". `config.ok` is false only when the file failed to parse and tty7 is running on defaults; -`config.state` is the same sentence the table prints. The extra rows the table -can show — unread keys, unusable `custom_shells` — are prose, not JSON fields. +`config.state` is the same sentence the table prints. `config.dir_writable` is +probed by writing, not read off the mode bits, because a read-only mount or an +ACL leaves a `0700` directory that refuses every write — false means settings +cannot be saved and `tty7 new` cannot file a workspace. Either of those false +exits `1`, so `tty7 doctor || alert` fires. The extra rows the table can show — +unread keys, unusable `custom_shells` — are prose, not JSON fields. ## `ws` — workspaces