fix(doctor): notice a config directory that cannot be written to

Made the config directory read-only and asked doctor about it:

    CHECK            RESULT
    TTY7_CONFIG_DIR  set (/tmp/…)
    config           none yet — the defaults are the config
    server           ok (build 26.8.3)
    …
    rc=0

Meanwhile, on the same directory:

    $ tty7 new
    tty7: could not write the machine tree at /tmp/…/machine.json:
          Permission denied (os error 13)

Every `new`, every `tab new`, and every settings save fails, and the verb
whose entire job is "check this install" called it healthy. The config row is
about *reading* — it is answering "does the file parse", and there is no file
— so nothing in the table was wrong, and nothing in it was the answer either.
`tty7 doctor || alert` is exactly the thing that should have fired.

Now: a row naming the directory and what stops working, a headline on stderr
so `-q` still says it, `config.dir_writable` in the JSON, and exit 1 alongside
the unparseable-config case it sits next to.

The check writes a probe file and removes it, rather than reading the mode
bits. A read-only mount, an ACL, an immutable flag or another user's directory
all leave `0700` on something that refuses every write, and what is being
diagnosed is whether the write succeeds. A directory that cannot be reached at
all counts as unwritable — an install nobody can open is not a healthy one.

Verified against a live daemon in all three states: healthy exits 0, read-only
exits 1 with the row and the headline, and fixing the permissions goes back to
0. The unit test covers the probe both ways and that it cleans up after
itself; replacing it with an `exists()` check fails it.
This commit is contained in:
l0ng-ai
2026-08-23 05:47:16 +08:00
parent c69a39f7a0
commit 4805cb1765
3 changed files with 118 additions and 5 deletions
+12
View File
@@ -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
+99 -2
View File
@@ -1797,6 +1797,21 @@ fn doctor(ctx: &Context, backend: &mut dyn Backend) -> Result<Outcome> {
// 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<Outcome> {
),
]);
}
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<Outcome> {
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<Outcome> {
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<Outcome> {
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
+7 -3
View File
@@ -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