fix(doctor): check the shell every new tab is going to launch

$ cat config.json
    { "shell": { "program": "/nonexistent/shell" } }
    $ tty7 new
    tty7: spawning a shell: daemon refused Spawn:
          no such program on this machine: /nonexistent/shell
    $ tty7 doctor | grep config
    config           ok

Nothing can open a tab, and the verb that checks the install says the config
is fine — which it is, in the only sense that row means: the file parses. The
same table already reports an unusable `custom_shells` entry, and that costs a
menu row. This costs every tab, every `tty7 new`, and the GUI's new-tab button.

The check is the daemon's own `shell_program_problem`, moved from
`daemon::pane` to `core::shells` beside `unusable_custom_shells` so both
callers share one definition and the row cannot drift from the refusal it is
predicting — it prints the same sentence the spawn will. Missing, a directory,
and not executable are told apart, because they are three different fixes.

A program given as a bare name is still not reported: the OS resolves it
through PATH and guessing at that would be worse than silence — the moved
comment says so and the moved tests pin it. Skipped under `-m` for the same
reason the hooks row is: a path checked here would answer about the wrong
machine.

Verified against a live daemon across all five cases — missing, directory,
non-executable, bare name, and no `shell` set — with exit 1 for the three that
break and 0 for the two that do not.
This commit is contained in:
l0ng-ai
2026-08-23 06:11:12 +08:00
parent 4e5699dc89
commit fa5b901d8d
5 changed files with 80 additions and 33 deletions
+10
View File
@@ -153,6 +153,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- **`doctor` now checks the configured shell.** A `shell` naming something
that is not there — missing, a directory, not executable — makes every new
tab and every `tty7 new` fail, while `doctor` reported `config ok`, because
the file parses perfectly well. It already checked `custom_shells`, which
only costs a menu row; the setting that costs the whole app was the one
nobody asked about. The check is the daemon's own, moved somewhere both can
reach rather than written twice, so the row says exactly what the refusal
will say. A shell given as a bare name is still left to `PATH`, and under
`-m` the row is skipped, because the shell resolves on the far machine.
- **A config file that will not load now says what is wrong and where.**
`doctor` reported "NOT VALID JSON" for three different mistakes, and two of
them were valid JSON: `"font_size": "big"`, or a string where an object goes,
+31 -1
View File
@@ -1882,6 +1882,32 @@ fn doctor(ctx: &Context, backend: &mut dyn Backend) -> Result<Outcome> {
),
]);
}
// The configured shell is what every new tab and every `tty7 new` launches,
// so a `shell` naming something that is not there breaks all of them — and
// the config row above says `ok`, because the file parses perfectly well.
// `custom_shells` was already checked here and only costs a menu row; this
// one costs the whole app, and was the one nobody asked about.
//
// Local only, like the hooks row: under `-m` the shell is resolved on the
// far machine and a path checked here would answer about the wrong disk.
let shell_problem = config_ok
.then(|| {
backend.is_this_machine().then(|| {
loaded
.0
.shell
.as_ref()
.and_then(|s| tty7_core::core::shells::program_problem(&s.program))
})
})
.flatten()
.flatten();
if let Some(problem) = &shell_problem {
rows.push(vec![
"shell".to_string(),
format!("{problem} — every new tab and `tty7 new` fails until this is fixed"),
]);
}
if !config_dir_writable {
rows.push(vec![
"config dir".to_string(),
@@ -2004,6 +2030,7 @@ fn doctor(ctx: &Context, backend: &mut dyn Backend) -> Result<Outcome> {
"ok": config_ok,
"state": config_state,
"dir_writable": config_dir_writable,
"shell_problem": shell_problem,
},
"hooks": hooks_json(&hooks),
}),
@@ -2018,6 +2045,9 @@ fn doctor(ctx: &Context, backend: &mut dyn Backend) -> Result<Outcome> {
if !config_dir_writable {
eprintln!("tty7: doctor: the config directory cannot be written to");
}
if shell_problem.is_some() {
eprintln!("tty7: doctor: the configured shell cannot be launched");
}
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:
@@ -2026,7 +2056,7 @@ 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 || !config_dir_writable {
if !config_ok || !config_dir_writable || shell_problem.is_some() {
return Ok(Outcome::Exit(1, report));
}
Ok(Outcome::Report(report))
+31
View File
@@ -61,6 +61,37 @@ pub fn inventory() -> ShellInventory {
inventory
}
/// Why `program` could not be launched, if there is a reason to be had.
///
/// "Program", not "shell": the same check stands in front of the configured
/// shell and of a command handed to `tty7 run`, and calling the latter a shell
/// tells someone who typed `tty7 run -- ./build.sh` that tty7 misunderstood
/// what they asked for. The caller supplies the context — the CLI prefixes
/// "spawning `…`" — so the sentence only has to carry the fact.
///
/// Only a program given as a path can be checked here; a bare name is resolved
/// through PATH by the OS, and guessing at that would be worse than silence.
pub fn program_problem(program: &str) -> Option<String> {
let path = std::path::Path::new(program);
if path.components().count() < 2 {
return None;
}
let Ok(meta) = std::fs::metadata(path) else {
return Some(format!("no such program on this machine: {program}"));
};
if meta.is_dir() {
return Some(format!("that program is a directory: {program}"));
}
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
if meta.permissions().mode() & 0o111 == 0 {
return Some(format!("that program is not executable: {program}"));
}
}
None
}
/// Adds the user's own menu entries after everything that was detected.
///
/// After, not among: the detected list is ordered so the shell a new tab
+1 -29
View File
@@ -11,6 +11,7 @@ use portable_pty::{Child, CommandBuilder, MasterPty, PtySize, native_pty_system}
use crate::core::kitty_graphics::{GraphicsSniffer, Segment, Sniffed};
use crate::core::osc::OscTokenizer;
use crate::core::shells::program_problem as shell_program_problem;
use crate::daemon::protocol::{
AuthResponse, DaemonMsg, MAX_FRAME, NativeSshSpec, PaneInfo, RemoteContext, RemoteKind,
ShellSpec, WinSize,
@@ -127,35 +128,6 @@ struct SpawnConfig {
/// "daemon refused Spawn: spawn failed: Unable to spawn … (ENOENT: No such
/// file or directory)" — and the one fact that matters is buried in it.
///
/// "Program", not "shell": the same check stands in front of the configured
/// shell and of a command handed to `tty7 run`, and calling the latter a shell
/// tells someone who typed `tty7 run -- ./build.sh` that tty7 misunderstood
/// what they asked for. The caller supplies the context — the CLI prefixes
/// "spawning `…`" — so the sentence only has to carry the fact.
///
/// Only a program given as a path can be checked here; a bare name is resolved
/// through PATH by the OS, and guessing at that would be worse than silence.
fn shell_program_problem(program: &str) -> Option<String> {
let path = std::path::Path::new(program);
if path.components().count() < 2 {
return None;
}
let Ok(meta) = std::fs::metadata(path) else {
return Some(format!("no such program on this machine: {program}"));
};
if meta.is_dir() {
return Some(format!("that program is a directory: {program}"));
}
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
if meta.permissions().mode() & 0o111 == 0 {
return Some(format!("that program is not executable: {program}"));
}
}
None
}
fn build_spawn_config(
pane: u64,
cwd: Option<PathBuf>,
+7 -3
View File
@@ -320,7 +320,7 @@ 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","dir_writable"},"hooks":{"installed","outdated","not_installed"}}`
JSON: `{"context":{"config_dir","workspace","pane"},"server":{"reachable","dialect_ok","build","status","routes"},"config":{"ok","state","dir_writable","shell_problem"},"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
@@ -329,8 +329,12 @@ false only when the file failed to parse and tty7 is running on defaults;
`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 —
cannot be saved and `tty7 new` cannot file a workspace. `config.shell_problem` is null unless the
configured `shell` names something that cannot be launched — missing, a
directory, not executable — in which case every new tab and every `tty7 new`
fails; a shell given as a bare name is left to `PATH` and never reported here,
and under `-m` the field is null because the shell resolves on the far machine.
Any of those three 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