mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-21 16:02:20 +00:00
fix(doctor): say when a workspace tree was set aside
A `machine.json` that does not parse is copied to `machine.json.corrupt` and the machine comes up with no workspaces at all — every tab and every pane layout on it. `MachineStore::open` calls that "recoverable by hand", and it is, but only for a hand that knows where to look. Nothing told it. The quarantine announces itself with a `log::warn!`, and per `docs/reference/privacy.mdx` there is no log at all unless `TTY7_LOG` or `RUST_LOG` is set — so on a default install the whole thing is silent. What the user sees is `tty7 ws ls` saying "no workspaces — `tty7 new <path>` starts one", which reads as an empty machine rather than as a lost one, and gives no reason to look in the config directory. `doctor` already makes this argument for `config.json`: a file that does not parse is exactly the state someone runs `doctor` in, and none of it is visible from the rows around it. The tree is the same case with more at stake — settings are still on screen when `config.json` is quarantined; workspaces are not. The row only appears when a copy is really there. One that said "no tree was set aside" beside every intact machine would be noise on every install, and this has to read as news. Verified end to end against a running server rather than only in a test: corrupt the tree, restart, `doctor` names the copy; restore it, the workspaces come back and the row goes away.
This commit is contained in:
@@ -1786,12 +1786,45 @@ fn doctor(ctx: &Context, backend: &mut dyn Backend) -> Result<Outcome> {
|
||||
false,
|
||||
),
|
||||
};
|
||||
// The same argument as the config row, for the file where it matters more.
|
||||
// A machine tree that does not parse is copied aside and the machine comes
|
||||
// up with *no workspaces at all* — every tab and pane layout on it. The
|
||||
// only notice is a `log::warn!`, and there is no log unless `TTY7_LOG` is
|
||||
// set, so the copy that makes this recoverable is invisible: the reader
|
||||
// sees "no workspaces" and no reason to look in the config directory.
|
||||
//
|
||||
// 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.
|
||||
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))
|
||||
.filter(|kept| !kept.is_empty());
|
||||
let mut rows = vec![
|
||||
vec![address::ENV_CONFIG_DIR.to_string(), mark(&ctx.config_dir)],
|
||||
vec![address::ENV_WS.to_string(), mark(&ctx.ws)],
|
||||
vec![address::ENV_PANE.to_string(), mark(&ctx.pane)],
|
||||
vec!["config".to_string(), config_state.clone()],
|
||||
];
|
||||
if let Some(kept) = &quarantined_tree {
|
||||
let newest = kept
|
||||
.last()
|
||||
.map(|p| p.display().to_string())
|
||||
.unwrap_or_default();
|
||||
rows.push(vec![
|
||||
"workspace tree".to_string(),
|
||||
match kept.len() {
|
||||
1 => format!(
|
||||
"a tree that did not parse was kept at {newest} — if workspaces are \
|
||||
missing, that copy is them"
|
||||
),
|
||||
n => format!(
|
||||
"{n} trees that did not parse were kept beside it, most recently \
|
||||
{newest} — if workspaces are missing, those copies are them"
|
||||
),
|
||||
},
|
||||
]);
|
||||
}
|
||||
let mut server = json!({ "reachable": false });
|
||||
// `None` until the server answers: with no tree to check against, "not
|
||||
// gone" would be a claim rather than an answer, so the two fields are
|
||||
|
||||
@@ -1002,6 +1002,23 @@ pub(crate) fn quarantine(path: &std::path::Path) -> bool {
|
||||
true
|
||||
}
|
||||
|
||||
/// Every quarantined copy of `path` that is actually on disk, oldest name
|
||||
/// first.
|
||||
///
|
||||
/// Quarantining announces itself with a `log::warn!` and nothing else, and
|
||||
/// there is no log at all unless `TTY7_LOG` is set — so a state file that
|
||||
/// failed to parse is set aside in complete silence. For `config.json` the
|
||||
/// silence is survivable, because `LoadOutcome` reaches `doctor` and the
|
||||
/// settings are still on screen. For the machine tree it is not: the whole
|
||||
/// of it — every workspace, every tab, every pane layout — comes back empty,
|
||||
/// and `MachineStore::open` calls that "recoverable by hand" while nothing
|
||||
/// tells the hand where to look. This is how a caller asks.
|
||||
pub fn quarantined_copies(path: &std::path::Path) -> Vec<PathBuf> {
|
||||
quarantine_candidates(path)
|
||||
.filter(|kept| kept.exists())
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Whether an earlier quarantine of `path` already holds exactly `bytes`.
|
||||
fn already_kept(path: &std::path::Path, bytes: &[u8]) -> bool {
|
||||
quarantine_candidates(path).any(|kept| std::fs::read(&kept).is_ok_and(|held| held == bytes))
|
||||
@@ -1416,6 +1433,54 @@ mod tests {
|
||||
|
||||
/// What a ninth corruption does to the eight kept copies.
|
||||
///
|
||||
/// What `doctor` can find out about a tree that was set aside.
|
||||
///
|
||||
/// Quarantining a machine tree costs the user every workspace on the
|
||||
/// machine, and says so only through a `log::warn!` — into a log that is
|
||||
/// not written at all unless `TTY7_LOG` is set. So the copy that makes it
|
||||
/// recoverable had no reader: `tty7 ws ls` says "no workspaces" and
|
||||
/// nothing suggests looking in the config directory. This is the question
|
||||
/// `doctor` asks instead.
|
||||
///
|
||||
/// Empty on a healthy machine matters as much as non-empty on a broken
|
||||
/// one: the row is only worth printing as news, and one that appeared
|
||||
/// beside every intact tree would be noise on every install.
|
||||
#[test]
|
||||
fn a_quarantined_tree_can_be_found_by_something_that_wants_to_report_it() {
|
||||
let dir = std::env::temp_dir().join(format!("tty7-quar-find-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
std::fs::create_dir_all(&dir).expect("scratch dir");
|
||||
let path = dir.join("machine.json");
|
||||
|
||||
assert!(
|
||||
quarantined_copies(&path).is_empty(),
|
||||
"nothing was ever set aside here"
|
||||
);
|
||||
|
||||
std::fs::write(&path, b"{ not json").expect("write the file");
|
||||
quarantine(&path);
|
||||
let kept = quarantined_copies(&path);
|
||||
assert_eq!(kept.len(), 1, "one corruption, one copy");
|
||||
assert_eq!(
|
||||
kept[0].file_name().and_then(|n| n.to_str()),
|
||||
Some("machine.json.corrupt")
|
||||
);
|
||||
|
||||
// A second, *different* corruption earns its own copy; the same bytes
|
||||
// again would not, which is what `already_kept` is for.
|
||||
std::fs::write(&path, b"{ still not json").expect("rewrite the file");
|
||||
quarantine(&path);
|
||||
let kept = quarantined_copies(&path);
|
||||
assert_eq!(kept.len(), 2, "a distinct corruption is kept beside it");
|
||||
assert_eq!(
|
||||
kept[1].file_name().and_then(|n| n.to_str()),
|
||||
Some("machine.json.corrupt.1"),
|
||||
"oldest name first, so the last is the most recent"
|
||||
);
|
||||
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
/// `quarantine_path` ends in `unwrap_or(base)`, which reads like a
|
||||
/// defensive branch and is the wrap: the cap is on how much history a
|
||||
/// config directory may accumulate, and the oldest copy is what gives way.
|
||||
|
||||
Reference in New Issue
Block a user