mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-21 16:02:20 +00:00
A remote workspace could sit in a loop nobody could get out of: every reconnect failed with "started but nothing was answering on the control socket after 15s", the strip showed a copy bar frozen at 100%, and no button was offered. The daemon is the root of it. When its control listener would not open it logged one line and kept running — and a running daemon holds the single-server lock, so every later --daemon stood down at once and every client probe failed, forever. Whether something else is serving cannot be read off the errno: bind_control_socket clears the leftovers it can, but a path it cannot clear comes back AddrInUse in the same words a live server does. Ask by connecting, and exit when nothing answers. The reason was thrown away twice over: the daemon's stdout and stderr went to /dev/null, and the readiness probe kept only out.success(). Both are kept now — output and exit status land beside the binary, stamped with the launch's own nonce so a restart never reads the outgoing daemon's status as the incoming one's. A start that has already failed no longer waits out the full timeout. The UI half: an automatic reconnect never retired its install progress, and a leftover entry draws an install in flight instead of the failure and its button. And a long error stretched the status card to 1978px in a 1440px window, taking the retry button off the screen with it. Closes part of #774. The Vim :wq cursor and the btop re-attach items in that issue are not touched. Claude-Session: https://claude.ai/code/session_015q6HRem76HYy33T39bp34c
This commit is contained in:
@@ -301,8 +301,17 @@ pub fn install_confirm() -> Arc<dyn InstallConfirm> {
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
pub enum InstallPhase {
|
||||
Downloading { done: u64, total: Option<u64> },
|
||||
Uploading { done: u64, total: u64 },
|
||||
Downloading {
|
||||
done: u64,
|
||||
total: Option<u64>,
|
||||
},
|
||||
Uploading {
|
||||
done: u64,
|
||||
total: u64,
|
||||
},
|
||||
/// The bytes are there and the far end is being waited on. Both a first
|
||||
/// install and a restart end here, which is why its caption says starting
|
||||
/// rather than restarting.
|
||||
Restarting,
|
||||
}
|
||||
|
||||
@@ -777,6 +786,18 @@ impl<'a> Installer<'a> {
|
||||
let (confirmed, _) = self.install(asset, &paths)?;
|
||||
report.installed = true;
|
||||
report.confirmed = confirmed;
|
||||
// The upload's caption was the last thing reported, so the
|
||||
// strip sat on "copying… 100%" through the startup wait —
|
||||
// and when the wait failed, that is the frame the user was
|
||||
// left looking at.
|
||||
//
|
||||
// Reported here rather than in the wait itself: an
|
||||
// `ensure_daemon` with nothing to install has to stay
|
||||
// silent. A pane route quietly restarting a dead remote
|
||||
// daemon goes through that path, and nothing on it retires
|
||||
// a progress entry — one left there freezes the strip and
|
||||
// takes its button away for the rest of the session.
|
||||
install_progress().report(&self.host, InstallPhase::Restarting);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -915,9 +936,21 @@ impl<'a> Installer<'a> {
|
||||
ReleaseDownload { fetch }.load_with_progress(&self.version, asset, &on_progress)
|
||||
}
|
||||
|
||||
/// Whether this machine has never had a tty7 server on it — the question
|
||||
/// the consent prompt turns on.
|
||||
///
|
||||
/// A launch leaves `tty7-server-c8p6.startup.log` and `.startup.exit`
|
||||
/// beside the binary, and both begin the same way it does. Counting one as
|
||||
/// a server would let someone who deleted the binary to uninstall tty7 get
|
||||
/// a new one downloaded and written without ever being asked.
|
||||
fn is_first_install(&self, paths: &RemotePaths) -> bool {
|
||||
let a_server = |name: &String| {
|
||||
name.starts_with("tty7-server-")
|
||||
&& !name.ends_with(STARTUP_LOG_SUFFIX)
|
||||
&& !name.ends_with(STARTUP_EXIT_SUFFIX)
|
||||
};
|
||||
match self.ops.list_dir(&paths.bin_dir) {
|
||||
Ok(Some(entries)) => !entries.iter().any(|name| name.starts_with("tty7-server-")),
|
||||
Ok(Some(entries)) => !entries.iter().any(a_server),
|
||||
Ok(None) => true,
|
||||
Err(_) => true,
|
||||
}
|
||||
@@ -931,41 +964,71 @@ impl<'a> Installer<'a> {
|
||||
return Ok((false, self.check_running_build(paths)));
|
||||
}
|
||||
|
||||
self.launch_daemon(paths)?;
|
||||
let log = self.launch_daemon(paths)?;
|
||||
self.wait_for_launch(paths, &log)?;
|
||||
Ok((true, self.check_running_build(paths)))
|
||||
}
|
||||
|
||||
/// Wait out a daemon this call just launched, and say what happened if it
|
||||
/// never answers.
|
||||
///
|
||||
/// Two things used to be thrown away here, and between them they made every
|
||||
/// remote startup failure read the same: the probe's exit status and stderr
|
||||
/// (only `out.success()` survived) and the daemon's own output ([`launch_command`]
|
||||
/// sent it to `/dev/null`). All anyone ever saw was "nothing was answering
|
||||
/// after 15s", which names the symptom and not one cause.
|
||||
fn wait_for_launch(&self, paths: &RemotePaths, log: &StartupLog) -> Result<(), InstallError> {
|
||||
let deadline = Instant::now() + self.startup_timeout;
|
||||
loop {
|
||||
if self.daemon_is_serving(paths)? {
|
||||
return Ok((true, self.check_running_build(paths)));
|
||||
let probe = match self.control_probe(paths)? {
|
||||
None => return Ok(()),
|
||||
Some(reason) => reason,
|
||||
};
|
||||
// A process that has already *failed* will not start answering.
|
||||
// Ask before sleeping again: waiting out the full timeout for a
|
||||
// daemon that died in the first 200ms is fifteen seconds of
|
||||
// nothing.
|
||||
//
|
||||
// A clean exit is not that. `run_daemon` returns success when
|
||||
// another server already holds the single-server lock, so status 0
|
||||
// means "someone else is the server here" — which is good news
|
||||
// arriving early. Keep probing for that someone; only the deadline
|
||||
// ends this.
|
||||
let exit = log.exit_status(self.ops).filter(|status| status != "0");
|
||||
if exit.is_none() && Instant::now() < deadline {
|
||||
std::thread::sleep(self.poll_interval);
|
||||
continue;
|
||||
}
|
||||
if Instant::now() >= deadline {
|
||||
return Err(InstallError::Launch {
|
||||
reason: format!(
|
||||
"{} started but nothing was answering on the control socket after {:?}",
|
||||
paths.binary, self.startup_timeout
|
||||
),
|
||||
});
|
||||
}
|
||||
std::thread::sleep(self.poll_interval);
|
||||
return Err(InstallError::Launch {
|
||||
reason: log.explain(self.ops, &paths.binary, &probe, exit, self.startup_timeout),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
fn daemon_is_serving(&self, paths: &RemotePaths) -> Result<bool, InstallError> {
|
||||
/// `None` when the control socket answered, `Some(why)` when it did not.
|
||||
fn control_probe(&self, paths: &RemotePaths) -> Result<Option<String>, InstallError> {
|
||||
let cmd = format!(
|
||||
"{} --stdio --bridge < /dev/null",
|
||||
shell_quote(&paths.binary)
|
||||
);
|
||||
match self.ops.run(&cmd) {
|
||||
Ok(out) => Ok(out.success()),
|
||||
Ok(out) if out.success() => Ok(None),
|
||||
Ok(out) => Ok(Some(out.failure_reason())),
|
||||
Err(reason) => Err(InstallError::Launch { reason }),
|
||||
}
|
||||
}
|
||||
|
||||
fn launch_daemon(&self, paths: &RemotePaths) -> Result<(), InstallError> {
|
||||
fn daemon_is_serving(&self, paths: &RemotePaths) -> Result<bool, InstallError> {
|
||||
Ok(self.control_probe(paths)?.is_none())
|
||||
}
|
||||
|
||||
fn launch_daemon(&self, paths: &RemotePaths) -> Result<StartupLog, InstallError> {
|
||||
let log = StartupLog::for_binary(&paths.binary);
|
||||
let settle = self.ops.launch_settle(&paths.binary);
|
||||
self.ops
|
||||
.spawn_detached(&launch_script(&paths.binary, settle))
|
||||
.map_err(|reason| InstallError::Launch { reason })
|
||||
.spawn_detached(&launch_script(&paths.binary, &log, settle))
|
||||
.map_err(|reason| InstallError::Launch { reason })?;
|
||||
Ok(log)
|
||||
}
|
||||
|
||||
fn check_running_build(&self, paths: &RemotePaths) -> Option<MismatchedRemoteDaemon> {
|
||||
@@ -1065,19 +1128,8 @@ impl<'a> Installer<'a> {
|
||||
std::thread::sleep(self.poll_interval);
|
||||
}
|
||||
|
||||
self.launch_daemon(paths)?;
|
||||
let deadline = Instant::now() + self.startup_timeout;
|
||||
loop {
|
||||
if self.daemon_is_serving(paths)? {
|
||||
return Ok(());
|
||||
}
|
||||
if Instant::now() >= deadline {
|
||||
return Err(InstallError::Launch {
|
||||
reason: format!("{} was restarted but never started answering", paths.binary),
|
||||
});
|
||||
}
|
||||
std::thread::sleep(self.poll_interval);
|
||||
}
|
||||
let log = self.launch_daemon(paths)?;
|
||||
self.wait_for_launch(paths, &log)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1110,19 +1162,163 @@ const RUNNING_EXE_COMMAND: &str = r#"if [ -d /proc ]; then for p in /proc/[0-9]*
|
||||
|
||||
const TERMINATE_RUNNING_COMMAND: &str = r#"if [ -d /proc ]; then for p in /proc/[0-9]*; do e=$(readlink "$p/exe" 2>/dev/null) || continue; case "$e" in */tty7-server-*) kill -TERM "${p#/proc/}" 2>/dev/null; break;; esac; done; else ps -xwwo pid=,comm= 2>/dev/null | while read -r pid e; do case "$e" in */tty7-server-*) kill -TERM "$pid" 2>/dev/null; break;; esac; done; fi; true"#;
|
||||
|
||||
fn launch_command(binary: &str) -> String {
|
||||
/// The two files a launch writes beside the server binary. Named here because
|
||||
/// `is_first_install` reads the same directory and must not mistake one of
|
||||
/// these for an installed server.
|
||||
pub(crate) const STARTUP_LOG_SUFFIX: &str = ".startup.log";
|
||||
pub(crate) const STARTUP_EXIT_SUFFIX: &str = ".startup.exit";
|
||||
|
||||
/// Where a launched daemon's output and exit status land on the far end.
|
||||
///
|
||||
/// One pair of files per binary, truncated at each launch rather than named
|
||||
/// uniquely per attempt: these exist to be read seconds later by the client
|
||||
/// that wrote them, and a unique name per launch would leave a file behind on
|
||||
/// every reconnect for nobody to ever delete.
|
||||
///
|
||||
/// What makes the fixed name safe is the nonce. A restart tells the old daemon
|
||||
/// to stop and the new one to start, and the old one's wrapper records its
|
||||
/// status only once the old process is fully gone — which is after its socket
|
||||
/// stopped answering, so it can land *after* the new launch truncated the file.
|
||||
/// Reading that as the new daemon's status would fail a restart that is going
|
||||
/// fine. The status is written with the nonce that asked for it, and a status
|
||||
/// carrying anyone else's is not an answer to this launch.
|
||||
pub(crate) struct StartupLog {
|
||||
log: String,
|
||||
exit: String,
|
||||
nonce: String,
|
||||
}
|
||||
|
||||
/// How much of the far end's startup log to fetch.
|
||||
const TAIL_BYTES: usize = 4096;
|
||||
|
||||
/// How much of it reaches the error string. The whole tail goes to this
|
||||
/// client's log; a status strip gets the last few lines, which is where the
|
||||
/// reason always is.
|
||||
const TAIL_LINES: usize = 4;
|
||||
|
||||
impl StartupLog {
|
||||
pub(crate) fn for_binary(binary: &str) -> Self {
|
||||
Self {
|
||||
log: format!("{binary}{STARTUP_LOG_SUFFIX}"),
|
||||
exit: format!("{binary}{STARTUP_EXIT_SUFFIX}"),
|
||||
nonce: uuid::Uuid::new_v4().simple().to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
/// The exit status the wrapper recorded for *this* launch, or `None` while
|
||||
/// the daemon is still running. Only the wrapper writes this file, and only
|
||||
/// once it has waited for the daemon, so a status carrying this launch's
|
||||
/// nonce is the death certificate.
|
||||
fn exit_status(&self, ops: &dyn RemoteOps) -> Option<String> {
|
||||
let cmd = format!("cat {} 2>/dev/null", shell_quote(&self.exit));
|
||||
let out = ops.run(&cmd).ok()?;
|
||||
let (nonce, status) = out.stdout.trim().split_once(' ')?;
|
||||
(nonce == self.nonce && !status.is_empty()).then(|| status.to_string())
|
||||
}
|
||||
|
||||
fn tail(&self, ops: &dyn RemoteOps) -> String {
|
||||
let cmd = format!(
|
||||
"tail -c {TAIL_BYTES} {} 2>/dev/null",
|
||||
shell_quote(&self.log)
|
||||
);
|
||||
ops.run(&cmd).map(|out| out.stdout).unwrap_or_default()
|
||||
}
|
||||
|
||||
/// Why the daemon never answered, in a sentence a status strip can show.
|
||||
///
|
||||
/// The full tail goes to this client's log; the message keeps the last few
|
||||
/// lines of it, because the interesting one is always the last thing the
|
||||
/// server managed to say before it gave up.
|
||||
fn explain(
|
||||
&self,
|
||||
ops: &dyn RemoteOps,
|
||||
binary: &str,
|
||||
probe: &str,
|
||||
exit: Option<String>,
|
||||
waited: Duration,
|
||||
) -> String {
|
||||
let tail = self.tail(ops);
|
||||
if !tail.trim().is_empty() {
|
||||
log::warn!("remote {binary} startup log ({}):\n{tail}", self.log);
|
||||
}
|
||||
let what = match &exit {
|
||||
Some(status) => format!("{binary} exited with status {status} before it answered"),
|
||||
None => format!("{binary} was still not answering after {waited:?}"),
|
||||
};
|
||||
let mut out = format!("{what} on the control socket");
|
||||
if !probe.trim().is_empty() {
|
||||
out.push_str(&format!("; last probe said: {}", probe.trim()));
|
||||
}
|
||||
let said: Vec<&str> = tail
|
||||
.lines()
|
||||
.map(str::trim)
|
||||
.filter(|line| !line.is_empty())
|
||||
.rev()
|
||||
.take(TAIL_LINES)
|
||||
.collect();
|
||||
if said.is_empty() {
|
||||
out.push_str(&format!("; it logged nothing to {}", self.log));
|
||||
} else {
|
||||
let said: Vec<&str> = said.into_iter().rev().collect();
|
||||
out.push_str(&format!(
|
||||
"; it said: {} (full log at {})",
|
||||
said.join(" / "),
|
||||
self.log
|
||||
));
|
||||
}
|
||||
out
|
||||
}
|
||||
}
|
||||
|
||||
/// Start the daemon detached, keeping what it says and how it ends.
|
||||
///
|
||||
/// The output used to go to `/dev/null`, which is why a remote that refused to
|
||||
/// come up could only ever be reported as silence. Everything the server
|
||||
/// prints on the way up — the control socket it bound, the listener it could
|
||||
/// not bind, the other server already holding the single-server lock — is a
|
||||
/// `startup_note!` on stderr, and all of it was being discarded.
|
||||
fn launch_command(binary: &str, log: &StartupLog) -> String {
|
||||
let bin = shell_quote(binary);
|
||||
let log_path = shell_quote(&log.log);
|
||||
let exit_path = shell_quote(&log.exit);
|
||||
let nonce = &log.nonce;
|
||||
// The wrapper outlives the daemon by one line: it waits, then records the
|
||||
// status, stamped with the nonce that asked for it.
|
||||
let supervised = shell_quote(&format!(
|
||||
"{bin} --daemon; s=$?; printf '%s %s' {nonce} \"$s\" > {exit_path}; exit \"$s\""
|
||||
));
|
||||
// Three things this preamble has to get right, each of them a way to break
|
||||
// a machine that used to work:
|
||||
//
|
||||
// - The `umask` is scoped to the subshell. Left bare it would apply to the
|
||||
// whole login shell, so the daemon would inherit it and so would every
|
||||
// pane shell it forks — and a `git clone` in a remote pane would produce
|
||||
// files nobody but the owner can read.
|
||||
// - Both files are removed and re-created here, under that umask, and only
|
||||
// truncated by the wrapper. `>` on an existing file keeps whatever mode
|
||||
// that file already had, so truncating alone would let one left at 0644 —
|
||||
// by an older build, by anything — stay that way forever.
|
||||
// - A home that is full, read-only, or not ours must not stop the daemon
|
||||
// from starting. If the files cannot be made, the redirect falls back to
|
||||
// `/dev/null` and the launch goes ahead without diagnostics, which is
|
||||
// exactly what it did before. The whole attempt sits inside a subshell so
|
||||
// that a redirection failure cannot take the login shell down with it —
|
||||
// on a POSIX shell a redirection error on the special builtin `:` ends a
|
||||
// non-interactive shell outright, and `|| true` never gets to run.
|
||||
format!(
|
||||
"if command -v setsid >/dev/null 2>&1; then \
|
||||
setsid {bin} --daemon < /dev/null > /dev/null 2>&1 & \
|
||||
"out={log_path}; \
|
||||
(umask 077; rm -f {log_path} {exit_path}; : > {log_path}; : > {exit_path}) 2>/dev/null \
|
||||
|| out=/dev/null; \
|
||||
if command -v setsid >/dev/null 2>&1; then \
|
||||
setsid sh -c {supervised} < /dev/null >> \"$out\" 2>&1 & \
|
||||
else \
|
||||
nohup {bin} --daemon < /dev/null > /dev/null 2>&1 & \
|
||||
nohup sh -c {supervised} < /dev/null >> \"$out\" 2>&1 & \
|
||||
fi"
|
||||
)
|
||||
}
|
||||
|
||||
fn launch_script(binary: &str, settle: Option<String>) -> String {
|
||||
let launch = launch_command(binary);
|
||||
fn launch_script(binary: &str, log: &StartupLog, settle: Option<String>) -> String {
|
||||
let launch = launch_command(binary, log);
|
||||
match settle {
|
||||
Some(settle) => format!("{launch}\n{settle}"),
|
||||
None => launch,
|
||||
|
||||
@@ -45,6 +45,14 @@ struct FakeRemote {
|
||||
daemon_running: Mutex<bool>,
|
||||
running_exe: Mutex<Option<String>>,
|
||||
launch_works: bool,
|
||||
/// What a launch leaves behind on the far end: whatever the daemon printed
|
||||
/// on its way up, and — once the wrapper has watched it exit — the status
|
||||
/// it ended on. A daemon still running has written no status yet.
|
||||
startup_log: Mutex<String>,
|
||||
startup_exit: Mutex<Option<String>>,
|
||||
/// The nonce the launch in flight stamped its status with.
|
||||
startup_nonce: Mutex<String>,
|
||||
dies_with: Option<(String, String)>,
|
||||
speaks: Mutex<HashMap<String, RemoteProtocol>>,
|
||||
installed_speaks: Option<RemoteProtocol>,
|
||||
/// The stop command comes back as a failure and the daemon keeps serving —
|
||||
@@ -72,6 +80,10 @@ impl FakeRemote {
|
||||
daemon_running: Mutex::new(false),
|
||||
running_exe: Mutex::new(None),
|
||||
launch_works: true,
|
||||
startup_log: Mutex::new(String::new()),
|
||||
startup_exit: Mutex::new(None),
|
||||
startup_nonce: Mutex::new(String::new()),
|
||||
dies_with: None,
|
||||
speaks: Mutex::new(HashMap::new()),
|
||||
installed_speaks: Some(ours()),
|
||||
stop_fails: false,
|
||||
@@ -83,6 +95,30 @@ impl FakeRemote {
|
||||
self
|
||||
}
|
||||
|
||||
/// A daemon that starts, complains, and exits — the shape a machine whose
|
||||
/// control socket cannot be bound actually takes.
|
||||
fn dying_at_startup(mut self, status: &str, said: &str) -> Self {
|
||||
self.launch_works = false;
|
||||
self.dies_with = Some((status.to_string(), said.to_string()));
|
||||
self
|
||||
}
|
||||
|
||||
/// A daemon that finds another server already holding the lock and exits
|
||||
/// cleanly. Nobody failed; this one simply is not the server.
|
||||
fn standing_down(mut self, said: &str) -> Self {
|
||||
self.launch_works = false;
|
||||
self.dies_with = Some(("0".to_string(), said.to_string()));
|
||||
self
|
||||
}
|
||||
|
||||
/// A daemon that stays up and never answers. Nothing writes an exit
|
||||
/// status, so the wait can only end at its deadline.
|
||||
fn hanging_at_startup(mut self, said: &str) -> Self {
|
||||
self.launch_works = false;
|
||||
*self.startup_log.lock().unwrap() = said.to_string();
|
||||
self
|
||||
}
|
||||
|
||||
fn speaking(self, exe: &str, spoken: RemoteProtocol) -> Self {
|
||||
self.speaks.lock().unwrap().insert(exe.to_string(), spoken);
|
||||
self
|
||||
@@ -193,6 +229,21 @@ impl RemoteOps for FakeRemote {
|
||||
*self.running_exe.lock().unwrap() = None;
|
||||
return ok("");
|
||||
}
|
||||
if let Some(path) = cmd
|
||||
.strip_prefix("cat ")
|
||||
.and_then(|rest| rest.strip_suffix(" 2>/dev/null"))
|
||||
&& path.trim_matches('\'').ends_with(".startup.exit")
|
||||
{
|
||||
return ok(&self
|
||||
.startup_exit
|
||||
.lock()
|
||||
.unwrap()
|
||||
.clone()
|
||||
.unwrap_or_default());
|
||||
}
|
||||
if cmd.starts_with("tail -c") && cmd.contains(".startup.log") {
|
||||
return ok(&self.startup_log.lock().unwrap());
|
||||
}
|
||||
if cmd.contains("--stdio --bridge") {
|
||||
let running = *self.daemon_running.lock().unwrap();
|
||||
return Ok(ExecOutput {
|
||||
@@ -207,12 +258,27 @@ impl RemoteOps for FakeRemote {
|
||||
}
|
||||
if cmd.contains("--daemon") {
|
||||
self.journal.lock().unwrap().push(Journal::Launch);
|
||||
// The wrapper truncates the status file before starting and stamps
|
||||
// what it writes with this launch's nonce, so a launch in flight is
|
||||
// always distinguishable from one that ended — and from the launch
|
||||
// before it.
|
||||
let nonce = cmd
|
||||
.split_whitespace()
|
||||
.find(|word| word.len() == 32 && word.chars().all(|c| c.is_ascii_hexdigit()))
|
||||
.unwrap_or_default()
|
||||
.to_string();
|
||||
*self.startup_nonce.lock().unwrap() = nonce;
|
||||
*self.startup_exit.lock().unwrap() = None;
|
||||
if self.launch_works {
|
||||
*self.daemon_running.lock().unwrap() = true;
|
||||
let mut exe = self.running_exe.lock().unwrap();
|
||||
if exe.is_none() {
|
||||
*exe = Some(BINARY.to_string());
|
||||
}
|
||||
} else if let Some((status, said)) = &self.dies_with {
|
||||
let nonce = self.startup_nonce.lock().unwrap().clone();
|
||||
*self.startup_log.lock().unwrap() = said.clone();
|
||||
*self.startup_exit.lock().unwrap() = Some(format!("{nonce} {status}"));
|
||||
}
|
||||
return ok("");
|
||||
}
|
||||
@@ -842,6 +908,133 @@ fn a_daemon_that_never_answers_is_an_error() {
|
||||
}
|
||||
}
|
||||
|
||||
/// A daemon that exited will not start answering, so there is nothing to wait
|
||||
/// for. The old loop polled the full timeout anyway, and then reported the
|
||||
/// timeout — which reads as "the far end is slow" when the truth was that the
|
||||
/// server had already given up, with a reason, in the first fraction of a
|
||||
/// second.
|
||||
#[test]
|
||||
fn a_daemon_that_died_at_startup_fails_at_once_and_in_its_own_words() {
|
||||
let remote = FakeRemote::new().dying_at_startup(
|
||||
"1",
|
||||
"tty7-server: control listener unavailable: Permission denied (os error 13)\n",
|
||||
);
|
||||
remote.preinstall(BINARY, 0o755);
|
||||
let release = FakeRelease::new();
|
||||
let user = FakeUser::declining();
|
||||
|
||||
let err = installer(&remote, &release, &user, "me@broken-box:22")
|
||||
.run()
|
||||
.unwrap_err();
|
||||
|
||||
let InstallError::Launch { reason } = &err else {
|
||||
panic!("{err:?}");
|
||||
};
|
||||
assert!(
|
||||
reason.contains("exited with status 1"),
|
||||
"the status the daemon ended on: {reason}"
|
||||
);
|
||||
assert!(
|
||||
reason.contains("control listener unavailable"),
|
||||
"and what it said before it did: {reason}"
|
||||
);
|
||||
assert!(
|
||||
reason.contains(&format!("{BINARY}.startup.log")),
|
||||
"and where the rest of it is: {reason}"
|
||||
);
|
||||
|
||||
let probes = remote
|
||||
.journal()
|
||||
.into_iter()
|
||||
.filter(|j| matches!(j, Journal::Exec(c) if c.contains("--stdio --bridge")))
|
||||
.count();
|
||||
assert!(
|
||||
probes <= 2,
|
||||
"one probe before the launch and one after it is the whole wait: {probes}"
|
||||
);
|
||||
}
|
||||
|
||||
/// A restart tells the old daemon to stop and the new one to start, and the old
|
||||
/// one's wrapper records its status only once it is fully gone — which is after
|
||||
/// its socket stopped answering, so it can land after the new launch truncated
|
||||
/// the file. Reading someone else's `143` as this launch's answer would fail a
|
||||
/// restart that is going perfectly well.
|
||||
#[test]
|
||||
fn a_status_from_the_launch_before_is_not_this_launch_s_answer() {
|
||||
let remote = FakeRemote::new();
|
||||
remote.preinstall(BINARY, 0o755);
|
||||
let release = FakeRelease::new();
|
||||
let user = FakeUser::declining();
|
||||
|
||||
// The corpse of a previous launch, stamped with a nonce nobody asked for.
|
||||
*remote.startup_exit.lock().unwrap() = Some("0123456789abcdef0123456789abcdef 143".into());
|
||||
|
||||
installer(&remote, &release, &user, "me@box:22")
|
||||
.run()
|
||||
.expect("the daemon this launch started came up, whatever the old one did");
|
||||
}
|
||||
|
||||
/// `run_daemon` returns success when another server already holds the
|
||||
/// single-server lock, so a recorded status of 0 means "someone else is the
|
||||
/// server here" — good news arriving early, not a failure. Treating any status
|
||||
/// as terminal turned a transient probe miss into a hard error the old loop
|
||||
/// would have recovered from inside its fifteen seconds.
|
||||
#[test]
|
||||
fn a_daemon_that_stood_down_cleanly_is_not_a_failed_start() {
|
||||
let remote = FakeRemote::new()
|
||||
.standing_down("tty7-server: another server already serves this config dir; exiting\n");
|
||||
remote.preinstall(BINARY, 0o755);
|
||||
let release = FakeRelease::new();
|
||||
let user = FakeUser::declining();
|
||||
|
||||
let err = installer(&remote, &release, &user, "me@box:22")
|
||||
.run()
|
||||
.unwrap_err();
|
||||
let InstallError::Launch { reason } = &err else {
|
||||
panic!("{err:?}");
|
||||
};
|
||||
assert!(
|
||||
reason.contains("still not answering"),
|
||||
"it waited for the server that was supposed to be there, and said so \
|
||||
when nothing turned up: {reason}"
|
||||
);
|
||||
|
||||
let probes = remote
|
||||
.journal()
|
||||
.into_iter()
|
||||
.filter(|j| matches!(j, Journal::Exec(c) if c.contains("--stdio --bridge")))
|
||||
.count();
|
||||
assert!(
|
||||
probes > 3,
|
||||
"and it kept probing rather than failing on the exit status: {probes}"
|
||||
);
|
||||
}
|
||||
|
||||
/// The other half: a daemon that is up and simply never binds the socket. It
|
||||
/// leaves no exit status, so this one does wait out the deadline — but it still
|
||||
/// has to say what the probe was told and where to read the rest.
|
||||
#[test]
|
||||
fn a_daemon_that_never_answers_names_the_probe_and_the_log() {
|
||||
let remote = FakeRemote::new().hanging_at_startup("tty7-server: still opening the tree\n");
|
||||
remote.preinstall(BINARY, 0o755);
|
||||
let release = FakeRelease::new();
|
||||
let user = FakeUser::declining();
|
||||
|
||||
let err = installer(&remote, &release, &user, "me@slow-box:22")
|
||||
.run()
|
||||
.unwrap_err();
|
||||
|
||||
let InstallError::Launch { reason } = &err else {
|
||||
panic!("{err:?}");
|
||||
};
|
||||
assert!(reason.contains("still not answering"), "{reason}");
|
||||
assert!(
|
||||
reason.contains("no control server"),
|
||||
"the probe's own stderr, which used to be dropped for its exit code: {reason}"
|
||||
);
|
||||
assert!(reason.contains("still opening the tree"), "{reason}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_older_running_daemon_is_kept_and_reported() {
|
||||
let (remote, legacy) = FakeRemote::new().with_legacy_install("26.7.4");
|
||||
@@ -982,7 +1175,8 @@ fn replacing_installs_the_matching_server_and_then_restarts_into_it() {
|
||||
|
||||
#[test]
|
||||
fn the_launch_command_detaches_and_closes_every_stream() {
|
||||
let cmd = launch_command("/home/me/.local/share/tty7/bin/tty7-server-26.7.5");
|
||||
let binary = "/home/me/.local/share/tty7/bin/tty7-server-26.7.5";
|
||||
let cmd = launch_command(binary, &StartupLog::for_binary(binary));
|
||||
assert!(cmd.contains("setsid"), "{cmd}");
|
||||
assert!(
|
||||
cmd.contains("nohup"),
|
||||
@@ -990,19 +1184,61 @@ fn the_launch_command_detaches_and_closes_every_stream() {
|
||||
);
|
||||
assert!(cmd.contains("--daemon"), "{cmd}");
|
||||
assert!(cmd.contains("< /dev/null"), "{cmd}");
|
||||
assert!(cmd.contains("> /dev/null 2>&1"), "{cmd}");
|
||||
assert!(
|
||||
cmd.trim_end().ends_with("fi"),
|
||||
"both branches background it: {cmd}"
|
||||
);
|
||||
}
|
||||
|
||||
fn scoped_umask(binary: &str) -> String {
|
||||
format!(
|
||||
"(umask 077; rm -f '{binary}.startup.log' '{binary}.startup.exit'; \
|
||||
: > '{binary}.startup.log'; : > '{binary}.startup.exit')"
|
||||
)
|
||||
}
|
||||
|
||||
/// The daemon's stdout and stderr used to go to `/dev/null`, and everything it
|
||||
/// says on the way up is a `startup_note!` on stderr. Discarding them is what
|
||||
/// left "nothing was answering after 15s" as the only thing a failed remote
|
||||
/// start could ever report.
|
||||
#[test]
|
||||
fn the_launch_command_keeps_what_the_daemon_says_and_how_it_ends() {
|
||||
let binary = "/home/me/.local/share/tty7/bin/tty7-server-26.7.5";
|
||||
let cmd = launch_command(binary, &StartupLog::for_binary(binary));
|
||||
assert!(
|
||||
!cmd.contains("> /dev/null 2>&1"),
|
||||
"stderr is the diagnosis, not noise: {cmd}"
|
||||
);
|
||||
assert!(
|
||||
cmd.contains(&format!("{binary}.startup.log")),
|
||||
"output lands in a file this client can read back: {cmd}"
|
||||
);
|
||||
assert!(
|
||||
cmd.contains(&format!("{binary}.startup.exit")),
|
||||
"and so does the exit status: {cmd}"
|
||||
);
|
||||
assert!(
|
||||
cmd.contains(&scoped_umask(binary)),
|
||||
"both are created private, and the umask is scoped to that — bare, it \
|
||||
would reach the daemon and every pane shell it forks: {cmd}"
|
||||
);
|
||||
assert!(
|
||||
cmd.contains("|| out=/dev/null"),
|
||||
"a home that cannot hold the files still gets its daemon started: {cmd}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_launch_settle_follows_the_launch_and_never_replaces_it() {
|
||||
let plain = launch_script(BINARY, None);
|
||||
assert_eq!(plain, launch_command(BINARY), "no settle, no wrapping");
|
||||
let log = StartupLog::for_binary(BINARY);
|
||||
let plain = launch_script(BINARY, &log, None);
|
||||
assert_eq!(
|
||||
plain,
|
||||
launch_command(BINARY, &log),
|
||||
"no settle, no wrapping"
|
||||
);
|
||||
|
||||
let settled = launch_script(BINARY, Some("sleep 1\n".to_string()));
|
||||
let settled = launch_script(BINARY, &log, Some("sleep 1\n".to_string()));
|
||||
assert!(
|
||||
settled.starts_with(&plain),
|
||||
"the launch survives: {settled}"
|
||||
@@ -1035,8 +1271,13 @@ fn remote_paths_are_shell_quoted() {
|
||||
|
||||
#[test]
|
||||
fn the_launch_command_quotes_its_binary() {
|
||||
let cmd = launch_command("/home/me/a b/tty7-server-1.0.0");
|
||||
let binary = "/home/me/a b/tty7-server-1.0.0";
|
||||
let cmd = launch_command(binary, &StartupLog::for_binary(binary));
|
||||
assert!(cmd.contains("'/home/me/a b/tty7-server-1.0.0'"), "{cmd}");
|
||||
assert!(
|
||||
cmd.contains("'/home/me/a b/tty7-server-1.0.0.startup.log'"),
|
||||
"and so are the paths derived from it: {cmd}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1529,6 +1770,36 @@ fn a_present_binary_reports_no_progress() {
|
||||
);
|
||||
}
|
||||
|
||||
/// The upload's caption used to be the last thing an install said, so the
|
||||
/// strip sat on "copying… 100%" through the whole startup wait — and when the
|
||||
/// wait failed, that stale caption is what the user was left looking at. The
|
||||
/// last word has to be the phase actually in progress.
|
||||
#[test]
|
||||
fn the_caption_moves_on_once_the_bytes_are_across() {
|
||||
let remote = FakeRemote::new();
|
||||
let release = FakeRelease::new();
|
||||
let user = FakeUser::approving();
|
||||
let reports = Arc::new(Reports::default());
|
||||
|
||||
with_install_progress(reports.clone(), || {
|
||||
installer(&remote, &release, &user, "me@build-box:22").run()
|
||||
})
|
||||
.expect("install");
|
||||
|
||||
let phases = reports.phases();
|
||||
assert!(
|
||||
phases
|
||||
.iter()
|
||||
.any(|p| matches!(p, InstallPhase::Uploading { .. })),
|
||||
"the copy is still reported: {phases:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
phases.last(),
|
||||
Some(&InstallPhase::Restarting),
|
||||
"and the wait for the far end is what the caption ends on: {phases:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_scoped_progress_sink_outranks_the_global_one() {
|
||||
let scoped = Arc::new(Reports::default());
|
||||
|
||||
@@ -339,7 +339,31 @@ pub fn run_daemon() -> anyhow::Result<()> {
|
||||
services,
|
||||
) {
|
||||
Ok(path) => startup_note!("tty7-server: control socket at {}", path.display()),
|
||||
Err(e) => startup_note!("tty7-server: control listener unavailable: {e}"),
|
||||
// Someone else already answers there. That is the ordinary local
|
||||
// shape: the GUI hosts the control listener and this process was
|
||||
// started only to own the panes. Carry on.
|
||||
//
|
||||
// Ask rather than read the errno. `bind_control_socket` clears an
|
||||
// ordinary leftover socket itself, but a path it cannot clear — a
|
||||
// directory in the way, a file owned by someone else — comes back
|
||||
// `AddrInUse` in the same words a live server does.
|
||||
Err(e) if crate::host::server::control_endpoint_answers() => {
|
||||
startup_note!(
|
||||
"tty7-server: control listener unavailable ({e}); \
|
||||
another server is answering there, so serving panes only"
|
||||
);
|
||||
}
|
||||
// Nothing is answering, and this process cannot answer either. Do
|
||||
// not stay alive: a running daemon holds the single-server lock, so
|
||||
// every later `--daemon` stands down at once and every client probe
|
||||
// of the control socket fails, forever. That pair is exactly the
|
||||
// "started but nothing was answering after 15s" a remote install
|
||||
// reports — with the reason, right here, thrown away. Exiting
|
||||
// releases the lock and hands the reason to whoever launched us.
|
||||
Err(e) => {
|
||||
startup_note!("tty7-server: control listener unavailable: {e}");
|
||||
return Err(e.into());
|
||||
}
|
||||
}
|
||||
}
|
||||
#[cfg(not(any(unix, windows)))]
|
||||
|
||||
@@ -1489,6 +1489,24 @@ mod sock {
|
||||
p.as_os_str().as_bytes().len() <= MAX_SOCKET_PATH_BYTES
|
||||
}
|
||||
|
||||
/// Whether a control server is answering at `path` right now.
|
||||
///
|
||||
/// This, and not the errno a failed bind carries, is the question that
|
||||
/// matters to a daemon whose listener would not open. [`bind_control_socket`]
|
||||
/// does clear the ordinary leftover — it connects first and unlinks a socket
|
||||
/// nobody is behind — but what it cannot clear it hands back as `AddrInUse`
|
||||
/// all the same: a directory standing where the socket goes, a file owned by
|
||||
/// another user. Those look exactly like a live server in the error, and
|
||||
/// nothing is listening behind either. Only a connect that completes tells
|
||||
/// them apart.
|
||||
pub(crate) fn control_socket_answers(path: &Path) -> bool {
|
||||
UnixStream::connect(path).is_ok()
|
||||
}
|
||||
|
||||
pub fn control_endpoint_answers() -> bool {
|
||||
control_socket_path().is_ok_and(|path| control_socket_answers(&path))
|
||||
}
|
||||
|
||||
pub fn bind_control_socket(path: &Path) -> io::Result<UnixListener> {
|
||||
let parent = path.parent().unwrap_or(Path::new("."));
|
||||
if !parent.exists() {
|
||||
@@ -1580,8 +1598,8 @@ mod sock {
|
||||
pub(crate) use sock::socket_path_in;
|
||||
#[cfg(unix)]
|
||||
pub use sock::{
|
||||
bind_control_socket, control_socket_path, serve_listener, serve_listener_with,
|
||||
spawn_control_listener, spawn_control_listener_with,
|
||||
bind_control_socket, control_endpoint_answers, control_socket_path, serve_listener,
|
||||
serve_listener_with, spawn_control_listener, spawn_control_listener_with,
|
||||
};
|
||||
|
||||
#[cfg(windows)]
|
||||
@@ -1635,6 +1653,19 @@ mod wsock {
|
||||
spawn_control_listener_with(host, Services::none())
|
||||
}
|
||||
|
||||
/// See the unix arm: a bind that failed only means "serve panes only" when
|
||||
/// something else is actually answering there.
|
||||
///
|
||||
/// Gated on the pidfile the same way [`spawn_control_listener_with`] gates
|
||||
/// its own probe, and for the same reason: a TCP connect that completes
|
||||
/// proves only that *something* accepted on the recorded port, and a port a
|
||||
/// dead daemon wrote can have been recycled by a stranger since. When the
|
||||
/// daemon that wrote it is gone, nothing behind that port is ours.
|
||||
pub fn control_endpoint_answers() -> bool {
|
||||
!crate::daemon::spawn::recorded_daemon_is_dead()
|
||||
&& transport::connect_endpoint(CONTROL_PORT_FILE).is_ok()
|
||||
}
|
||||
|
||||
pub fn serve_listener_with(
|
||||
listener: TcpListener,
|
||||
token: transport::Token,
|
||||
@@ -1678,8 +1709,8 @@ mod wsock {
|
||||
|
||||
#[cfg(windows)]
|
||||
pub use wsock::{
|
||||
CONTROL_PORT_FILE, connect_control, control_endpoint_path, remove_control_endpoint,
|
||||
spawn_control_listener, spawn_control_listener_with,
|
||||
CONTROL_PORT_FILE, connect_control, control_endpoint_answers, control_endpoint_path,
|
||||
remove_control_endpoint, spawn_control_listener, spawn_control_listener_with,
|
||||
};
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -2946,6 +2977,51 @@ mod tests {
|
||||
assert!(err.to_string().contains(CONTROL_SOCK_ENV), "{err}");
|
||||
}
|
||||
|
||||
/// `run_daemon` keeps serving panes past a control listener it could not
|
||||
/// open only when something else is answering there, and the errno cannot
|
||||
/// make that call. `bind_control_socket` clears the leftovers it can, but a
|
||||
/// path it cannot clear still comes back `AddrInUse` — indistinguishable
|
||||
/// from a live server, with nothing listening behind it. A daemon that read
|
||||
/// the error as "someone else is serving" stayed alive holding the
|
||||
/// single-server lock, answering nothing, forever.
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn only_a_connect_tells_a_live_server_from_a_blocked_path() {
|
||||
let dir = tempfile::TempDir::new().unwrap();
|
||||
|
||||
let sock = dir.path().join("s.sock");
|
||||
let listener = bind_control_socket(&sock).unwrap();
|
||||
std::thread::spawn(move || {
|
||||
for stream in listener.incoming() {
|
||||
drop(stream);
|
||||
}
|
||||
});
|
||||
assert_eq!(
|
||||
bind_control_socket(&sock).unwrap_err().kind(),
|
||||
io::ErrorKind::AddrInUse,
|
||||
"a live server is in use"
|
||||
);
|
||||
assert!(
|
||||
sock::control_socket_answers(&sock),
|
||||
"and it answers, which is what makes it live"
|
||||
);
|
||||
|
||||
// A directory where the socket goes. Nothing is listening, the connect
|
||||
// fails so there is nothing to unlink, and `bind` refuses it in the
|
||||
// same words it used for the live server above.
|
||||
let blocked = dir.path().join("blocked.sock");
|
||||
std::fs::create_dir(&blocked).unwrap();
|
||||
assert_eq!(
|
||||
bind_control_socket(&blocked).unwrap_err().kind(),
|
||||
io::ErrorKind::AddrInUse,
|
||||
"same error, and no server anywhere near it"
|
||||
);
|
||||
assert!(
|
||||
!sock::control_socket_answers(&blocked),
|
||||
"which is the difference the daemon has to act on"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn binding_clears_a_socket_a_crash_left_behind() {
|
||||
let dir = tempfile::TempDir::new().unwrap();
|
||||
|
||||
+5
-16
@@ -6883,34 +6883,23 @@ impl Tty7App {
|
||||
),
|
||||
None => message,
|
||||
};
|
||||
let bar = gpui_component::v_flex()
|
||||
let bar = crate::ui::remote_workspace::status_card(cx)
|
||||
.occlude()
|
||||
.gap(px(6.))
|
||||
.px_3()
|
||||
.py_1p5()
|
||||
.rounded_lg()
|
||||
.bg(theme.popover)
|
||||
.border_1()
|
||||
.border_color(theme.border)
|
||||
.shadow_md()
|
||||
.text_xs()
|
||||
.text_color(theme.muted_foreground)
|
||||
.child(
|
||||
gpui_component::h_flex()
|
||||
.items_center()
|
||||
.gap_2()
|
||||
crate::ui::remote_workspace::status_row()
|
||||
.child(gpui_component::Icon::new(gpui_component::IconName::Globe))
|
||||
.child(
|
||||
div()
|
||||
crate::ui::remote_workspace::status_message(message)
|
||||
.font_weight(gpui::FontWeight::MEDIUM)
|
||||
.text_color(theme.foreground)
|
||||
.child(message),
|
||||
.text_color(theme.foreground),
|
||||
)
|
||||
.when_some(action, |this, (label, action)| {
|
||||
use gpui_component::Sizable as _;
|
||||
use gpui_component::button::ButtonVariants as _;
|
||||
this.child(
|
||||
gpui_component::button::Button::new("remote-status-action")
|
||||
.flex_shrink_0()
|
||||
.label(label)
|
||||
.primary()
|
||||
.small()
|
||||
|
||||
+144
-15
@@ -267,7 +267,6 @@ impl Tty7App {
|
||||
.is_none()
|
||||
.then(|| self.remote_strip_action(&status, cx))
|
||||
.flatten();
|
||||
let theme = cx.theme();
|
||||
let message = match installing {
|
||||
Some(phase) => format!(
|
||||
"{machine} — {}",
|
||||
@@ -276,25 +275,15 @@ impl Tty7App {
|
||||
None => message,
|
||||
};
|
||||
Some(
|
||||
v_flex()
|
||||
.gap(px(6.))
|
||||
.px(px(12.))
|
||||
.py(px(6.))
|
||||
.rounded(px(10.))
|
||||
.bg(theme.popover)
|
||||
.border_1()
|
||||
.border_color(theme.border)
|
||||
.text_xs()
|
||||
.text_color(theme.muted_foreground)
|
||||
crate::ui::remote_workspace::status_card(cx)
|
||||
.child(
|
||||
h_flex()
|
||||
.items_center()
|
||||
.gap_2()
|
||||
crate::ui::remote_workspace::status_row()
|
||||
.child(gpui_component::Icon::new(IconName::Globe))
|
||||
.child(message)
|
||||
.child(crate::ui::remote_workspace::status_message(message))
|
||||
.when_some(action, |this, (label, action)| {
|
||||
this.child(
|
||||
Button::new("home-remote-status-action")
|
||||
.flex_shrink_0()
|
||||
.label(label)
|
||||
.ghost()
|
||||
.small()
|
||||
@@ -314,6 +303,146 @@ impl Tty7App {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod strip_layout_tests {
|
||||
use crate::core::session::{RemoteRef, RemoteTarget, WindowView, WindowViews, WorkspaceStore};
|
||||
use crate::daemon::install::{InstallPhase, InstallProgress as _};
|
||||
use crate::ui::remote_connect::HostChoice;
|
||||
use crate::ui::remote_workspace::ConnectFlow;
|
||||
use gpui::{TestAppContext, VisualTestContext, px, size};
|
||||
|
||||
fn box_at(port: u16) -> RemoteTarget {
|
||||
RemoteTarget::Direct {
|
||||
user: "hw".into(),
|
||||
host: "build-box".into(),
|
||||
port,
|
||||
}
|
||||
}
|
||||
|
||||
/// The home page on a remote workspace, sized to order, with the strip
|
||||
/// showing either an install in flight or a failure to explain.
|
||||
fn home_strip(
|
||||
cx: &mut TestAppContext,
|
||||
width: f32,
|
||||
installing: Option<InstallPhase>,
|
||||
failure: Option<&str>,
|
||||
) -> VisualTestContext {
|
||||
let (app, mut vcx) = crate::ui::app::test_window::harness(cx);
|
||||
let workspace = app.read_with(&vcx, |app, _| app.workspace);
|
||||
// A port per case: the install progress table is a process-wide static
|
||||
// keyed by machine, and these tests share a process.
|
||||
let target = box_at(if installing.is_some() { 22 } else { 2222 });
|
||||
vcx.update(|_, cx| {
|
||||
WorkspaceStore::install_for_test(
|
||||
cx,
|
||||
WindowViews {
|
||||
views: vec![WindowView {
|
||||
id: workspace,
|
||||
host: Some(RemoteRef::new(target.clone(), workspace)),
|
||||
open: true,
|
||||
..Default::default()
|
||||
}],
|
||||
active: Some(workspace),
|
||||
},
|
||||
);
|
||||
});
|
||||
if let Some(phase) = installing {
|
||||
crate::ui::remote_connect::GuiInstallProgress.report(&target.connection_key(), phase);
|
||||
}
|
||||
if let Some(error) = failure {
|
||||
app.update_in(&mut vcx, |app, _, _| {
|
||||
app.connect = Some(ConnectFlow::Failed {
|
||||
choice: HostChoice {
|
||||
target: target.clone(),
|
||||
label: "build-box".into(),
|
||||
detail: String::new(),
|
||||
},
|
||||
error: error.to_string(),
|
||||
});
|
||||
});
|
||||
}
|
||||
vcx.simulate_resize(size(px(width), px(900.)));
|
||||
app.update_in(&mut vcx, |_, _, cx| cx.notify());
|
||||
vcx.run_until_parked();
|
||||
vcx
|
||||
}
|
||||
|
||||
/// #774's first screenshot shows a copy bar running from inside a
|
||||
/// quarter-width card to the window's right edge. That escape does not
|
||||
/// reproduce here — `w_full` resolved against the card in every window
|
||||
/// width tried, before the fix as well as after — so this is a guard on the
|
||||
/// property, not the reproduction of a failure. It holds by construction
|
||||
/// now that the card has a width of its own; it did not before.
|
||||
#[gpui::test]
|
||||
fn the_copy_bar_stays_inside_the_home_strip(cx: &mut TestAppContext) {
|
||||
let full = InstallPhase::Uploading {
|
||||
done: 9_227_468,
|
||||
total: 9_227_468,
|
||||
};
|
||||
for width in [1440.0, 900.0, 480.0] {
|
||||
let mut vcx = home_strip(cx, width, Some(full), None);
|
||||
let card = vcx
|
||||
.debug_bounds("remote-status-card")
|
||||
.expect("an install in flight draws the strip");
|
||||
let bar = vcx.debug_bounds("remote-install-bar").expect("and its bar");
|
||||
assert!(
|
||||
bar.left() >= card.left() && bar.right() <= card.right(),
|
||||
"at {width}px the bar {bar:?} left its card {card:?}"
|
||||
);
|
||||
assert!(
|
||||
card.left() >= px(0.) && card.right() <= px(width),
|
||||
"and the card stays in the window: {card:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A startup failure now carries the far end's own words, so the strip's
|
||||
/// message is a paragraph rather than a phrase. Laid out in one row it
|
||||
/// stretched the card until both ran off the right of the window, taking
|
||||
/// the retry button with them — #774's second and third screenshots.
|
||||
#[gpui::test]
|
||||
fn a_long_failure_wraps_instead_of_stretching_the_card(cx: &mut TestAppContext) {
|
||||
let long = "the remote tty7-server did not start: \
|
||||
/home/hw/.local/share/tty7/bin/tty7-server-c8p6 exited with status 1 before \
|
||||
it answered on the control socket; last probe said: no control server at \
|
||||
/home/hw/.config/tty7/control.sock";
|
||||
|
||||
let mut short = home_strip(cx, 1440.0, None, Some("connection refused"));
|
||||
let short_card = short
|
||||
.debug_bounds("remote-status-card")
|
||||
.expect("strip drawn");
|
||||
let short_message = short
|
||||
.debug_bounds("remote-status-message")
|
||||
.expect("message drawn");
|
||||
|
||||
let mut wide = home_strip(cx, 1440.0, None, Some(long));
|
||||
let long_card = wide
|
||||
.debug_bounds("remote-status-card")
|
||||
.expect("strip drawn");
|
||||
let long_message = wide
|
||||
.debug_bounds("remote-status-message")
|
||||
.expect("message drawn");
|
||||
|
||||
assert_eq!(
|
||||
short_card.size.width, long_card.size.width,
|
||||
"the card's width is the card's, not the message's"
|
||||
);
|
||||
assert!(
|
||||
long_card.right() <= px(1440.),
|
||||
"and it stays in the window: {long_card:?}"
|
||||
);
|
||||
assert!(
|
||||
long_message.size.height > short_message.size.height,
|
||||
"a message that does not fit gets taller, not wider: \
|
||||
{long_message:?} against {short_message:?}"
|
||||
);
|
||||
assert!(
|
||||
long_message.right() <= long_card.right(),
|
||||
"and never reaches past the card: {long_message:?} in {long_card:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
+1
-1
@@ -1335,7 +1335,7 @@ pub fn translate_en(key: L10nKey) -> &'static str {
|
||||
"No running coding agent found — start one (claude, codex, …) in a pane first."
|
||||
}
|
||||
L10nKey::SwitcherThisComputer => "This Computer",
|
||||
L10nKey::SwitcherRestartingServer => "Restarting tty7's server…",
|
||||
L10nKey::SwitcherStartingServer => "Starting tty7's server…",
|
||||
L10nKey::SwitcherDownloadingServerWithTotal => {
|
||||
"Downloading tty7's server… {done} / {total}"
|
||||
}
|
||||
|
||||
+1
-1
@@ -1390,7 +1390,7 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> {
|
||||
"実行中のコーディングエージェントが見つかりません — 先にペインでコーディングエージェントを起動してください(claude、codex など)"
|
||||
}
|
||||
L10nKey::SwitcherThisComputer => "このコンピュータ",
|
||||
L10nKey::SwitcherRestartingServer => "tty7 のサーバーを再起動中…",
|
||||
L10nKey::SwitcherStartingServer => "tty7 のサーバーを起動中…",
|
||||
L10nKey::SwitcherDownloadingServerWithTotal => {
|
||||
"tty7 のサーバーをダウンロード中… {done} / {total}"
|
||||
}
|
||||
|
||||
+1
-1
@@ -1078,7 +1078,7 @@ l10n_keys! {
|
||||
RemoteMismatchVersionFromExe,
|
||||
AppNoRunningCodingAgent,
|
||||
SwitcherThisComputer,
|
||||
SwitcherRestartingServer,
|
||||
SwitcherStartingServer,
|
||||
SwitcherDownloadingServerWithTotal,
|
||||
SwitcherDownloadingServerNoTotal,
|
||||
SwitcherCopyingServer,
|
||||
|
||||
+1
-1
@@ -1259,7 +1259,7 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> {
|
||||
"未找到运行中的编码 agent——请先在某个窗格中启动一个(claude、codex 等)。"
|
||||
}
|
||||
L10nKey::SwitcherThisComputer => "本机",
|
||||
L10nKey::SwitcherRestartingServer => "正在重启 tty7 server…",
|
||||
L10nKey::SwitcherStartingServer => "正在启动 tty7 server…",
|
||||
L10nKey::SwitcherDownloadingServerWithTotal => "正在下载 tty7 server… {done} / {total}",
|
||||
L10nKey::SwitcherDownloadingServerNoTotal => "正在下载 tty7 server… {done}",
|
||||
L10nKey::SwitcherCopyingServer => "正在复制 tty7 server… {done} / {total}",
|
||||
|
||||
+128
-2
@@ -175,7 +175,7 @@ pub(crate) fn install_phase_caption(phase: crate::daemon::install::InstallPhase)
|
||||
use crate::daemon::install::InstallPhase;
|
||||
use crate::ui::remote_connect::human_bytes;
|
||||
match phase {
|
||||
InstallPhase::Restarting => t(L10nKey::SwitcherRestartingServer).to_string(),
|
||||
InstallPhase::Restarting => t(L10nKey::SwitcherStartingServer).to_string(),
|
||||
InstallPhase::Downloading { done, total } => match total {
|
||||
Some(total) => t_fmt(
|
||||
L10nKey::SwitcherDownloadingServerWithTotal,
|
||||
@@ -208,12 +208,17 @@ pub(crate) fn install_progress_bar(
|
||||
phase: crate::daemon::install::InstallPhase,
|
||||
cx: &gpui::App,
|
||||
) -> impl gpui::IntoElement + use<> {
|
||||
use gpui::{ParentElement as _, Styled as _};
|
||||
use gpui::{InteractiveElement as _, ParentElement as _, Styled as _};
|
||||
use gpui_component::ActiveTheme as _;
|
||||
let theme = cx.theme();
|
||||
gpui::div()
|
||||
.debug_selector(|| "remote-install-bar".into())
|
||||
.w_full()
|
||||
.h(gpui::px(PROGRESS_H))
|
||||
// The fill is a percentage of this track, and the track is rounded.
|
||||
// Clipping to it means no fraction and no rounding can put a pixel of
|
||||
// warning colour outside the groove it belongs in.
|
||||
.overflow_hidden()
|
||||
.rounded_full()
|
||||
.bg(theme.border)
|
||||
.child(
|
||||
@@ -225,6 +230,69 @@ pub(crate) fn install_progress_bar(
|
||||
)
|
||||
}
|
||||
|
||||
/// How wide a remote status card is when the window can spare the room.
|
||||
const STATUS_CARD_W: f32 = 560.0;
|
||||
|
||||
/// How much of a narrow window a status card may take.
|
||||
const STATUS_CARD_MAX: f32 = 0.92;
|
||||
|
||||
/// The frame both remote status strips share: home draws one under the logo, a
|
||||
/// window with tabs floats one over its panes. Same machine, same sentence, and
|
||||
/// — since a startup failure now carries the far end's own words — the same
|
||||
/// need to survive a message that is a paragraph rather than a phrase.
|
||||
///
|
||||
/// A definite width is the whole of it. Sized by its content instead, the card
|
||||
/// grew with the message: at 1440px, one startup failure laid it out 1978px
|
||||
/// wide, so the far half of the sentence and the retry button were off the
|
||||
/// screen. It also gives the progress bar's `w_full` something bounded to be a
|
||||
/// percentage of, which is what #774's first screenshot doubts — though a bar
|
||||
/// escaping its card never did reproduce here, before the change or after.
|
||||
pub(crate) fn status_card(cx: &gpui::App) -> gpui::Div {
|
||||
use gpui::{InteractiveElement as _, Styled as _};
|
||||
use gpui_component::ActiveTheme as _;
|
||||
let theme = cx.theme();
|
||||
gpui_component::v_flex()
|
||||
.debug_selector(|| "remote-status-card".into())
|
||||
.w(gpui::px(STATUS_CARD_W))
|
||||
.max_w(gpui::relative(STATUS_CARD_MAX))
|
||||
.min_w_0()
|
||||
.gap(gpui::px(6.))
|
||||
.px(gpui::px(12.))
|
||||
.py(gpui::px(6.))
|
||||
.rounded(gpui::px(10.))
|
||||
.bg(theme.popover)
|
||||
.border_1()
|
||||
.border_color(theme.border)
|
||||
.text_xs()
|
||||
.text_color(theme.muted_foreground)
|
||||
}
|
||||
|
||||
/// The row inside a status card: icon, message, and the one button. It wraps,
|
||||
/// so a long message pushes the button to a second line instead of off the
|
||||
/// window.
|
||||
pub(crate) fn status_row() -> gpui::Div {
|
||||
use gpui::Styled as _;
|
||||
gpui_component::h_flex()
|
||||
.w_full()
|
||||
.min_w_0()
|
||||
.flex_wrap()
|
||||
.items_center()
|
||||
.gap_2()
|
||||
}
|
||||
|
||||
/// The message half of that row. `flex_1` with `min_w_0` is what lets it be
|
||||
/// narrower than its longest word, which is what makes wrapping possible at
|
||||
/// all.
|
||||
pub(crate) fn status_message(message: String) -> gpui::Div {
|
||||
use gpui::{InteractiveElement as _, ParentElement as _, Styled as _};
|
||||
gpui::div()
|
||||
.debug_selector(|| "remote-status-message".into())
|
||||
.flex_1()
|
||||
.min_w_0()
|
||||
.whitespace_normal()
|
||||
.child(message)
|
||||
}
|
||||
|
||||
/// What the remote strip's button is for, once the status has been read.
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub(crate) enum StripAction {
|
||||
@@ -1832,6 +1900,14 @@ fn launch_attempt(cx: &mut gpui::App, host: HostId, target: RemoteTarget) {
|
||||
.collect();
|
||||
|
||||
RemoteLinks::mark(cx, host, |link| link.attempting = true);
|
||||
// Same bookkeeping the switcher's own connect does, and for the same
|
||||
// reason — see `connect_to_host`. An entry left behind here is worse than
|
||||
// a stale caption: the strip and the switcher both draw an install in
|
||||
// flight *instead of* the failure and its button, so a machine that needed
|
||||
// Update Server sat under a frozen bar at 100% with nothing to press,
|
||||
// counting attempts, for the rest of the session. The automatic path is
|
||||
// the one that reaches this state, because it is the one that retries.
|
||||
remote_connect::clear_install_progress(host);
|
||||
let for_finish = target.clone();
|
||||
cx.spawn(async move |cx| {
|
||||
let label_for_task = label.clone();
|
||||
@@ -1872,6 +1948,10 @@ fn finish_attempt(
|
||||
target: &RemoteTarget,
|
||||
outcome: Result<(remote_connect::Connected, Vec<String>), String>,
|
||||
) {
|
||||
// Nothing is being installed once the attempt is back, whichever way it
|
||||
// went. Retiring it here is what lets the failure — and the button that
|
||||
// answers it — reach the screen at all.
|
||||
remote_connect::clear_install_progress(host);
|
||||
let label = remote_connect::target_label(cx, target);
|
||||
match outcome {
|
||||
Ok((connected, sent)) => {
|
||||
@@ -2329,6 +2409,52 @@ mod tests {
|
||||
});
|
||||
}
|
||||
|
||||
/// A reconnect that got as far as copying the server and then failed used
|
||||
/// to leave its progress entry behind forever. That entry is not just a
|
||||
/// stale caption: the strip and the switcher both draw an install in
|
||||
/// flight *instead of* the failure and its button, so the machine sat
|
||||
/// under a bar frozen at 100%, counting attempts, with nothing to press —
|
||||
/// including the Update Server that would have fixed it.
|
||||
#[gpui::test]
|
||||
fn a_finished_attempt_retires_the_install_it_was_running(cx: &mut gpui::TestAppContext) {
|
||||
use crate::daemon::install::InstallProgress as _;
|
||||
cx.update(|cx| {
|
||||
cx.set_global(crate::core::config::Config::default());
|
||||
crate::core::session::WorkspaceStore::install_for_test(
|
||||
cx,
|
||||
crate::core::session::WindowViews::default(),
|
||||
);
|
||||
let target = RemoteTarget::Alias {
|
||||
alias: "build-box".into(),
|
||||
};
|
||||
let host = target.host_id();
|
||||
|
||||
remote_connect::GuiInstallProgress.report(
|
||||
&target.connection_key(),
|
||||
crate::daemon::install::InstallPhase::Uploading {
|
||||
done: 9_227_468,
|
||||
total: 9_227_468,
|
||||
},
|
||||
);
|
||||
assert!(
|
||||
remote_connect::install_progress_for(host).is_some(),
|
||||
"the copy is on screen"
|
||||
);
|
||||
|
||||
finish_attempt(
|
||||
cx,
|
||||
host,
|
||||
&target,
|
||||
Err("the remote tty7-server did not start".into()),
|
||||
);
|
||||
|
||||
assert!(
|
||||
remote_connect::install_progress_for(host).is_none(),
|
||||
"and the attempt that was running it is over, so it comes off"
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn taking_back_marks_the_workspace_for_a_whole_rebuild(cx: &mut gpui::TestAppContext) {
|
||||
cx.update(|cx| {
|
||||
|
||||
Reference in New Issue
Block a user