mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-22 00:02:23 +00:00
fix(remote): stop the server on machines that have no /proc, and show the install on the strip
Restarting the remote server timed out after ten seconds on every Mac and BSD, with the old daemon still running and the new binary already sitting next to it, unlaunched. Both the probe that finds the running `tty7-server-*` and the command that terminates it walked `/proc/[0-9]*` and read each `exe` symlink. There is no `/proc` there. Two things then went wrong at once. zsh is the login shell on macOS, and it aborts the whole command line when a glob matches nothing, so even the trailing `true` never ran; and `cycle_daemon` discards the result of the terminate, so a command that killed nothing was indistinguishable from one that worked. `daemon_is_serving` then answered yes until the deadline. Guard the glob behind `[ -d /proc ]` — unreached, it is never expanded, so zsh has nothing to abort on — and fall back to `ps`, whose `comm` is the full path on the BSDs. It cannot be the only branch: Linux truncates `comm` to 15 characters, one short of `tty7-server-c7p5`, which is why `/proc` stays the first choice where it exists. `check_running_build` reads the same probe and was equally blind on those machines; it can see now. Separately, the install progress bar only ever existed inside the switcher. Pressing Update Server from a parked workspace with no switcher open froze the window for the length of the download and then produced a modal, with nothing in between. The strip draws it too now — caption and bar from the same source the switcher uses, and no button while an install is in flight, since pressing it again would start a second one on top of the first.
This commit is contained in:
l0ng-ai
@@ -1045,9 +1045,23 @@ impl<'a> Installer<'a> {
|
||||
}
|
||||
}
|
||||
|
||||
const RUNNING_EXE_COMMAND: &str = r#"for p in /proc/[0-9]*; do e=$(readlink "$p/exe" 2>/dev/null) || continue; case "$e" in */tty7-server-*) printf '%s' "${e% (deleted)}"; break;; esac; done; true"#;
|
||||
/// Finding the running server takes two shapes because `/proc` is a Linux
|
||||
/// thing. On Linux, `/proc/<pid>/exe` is the honest answer: a symlink to the
|
||||
/// file that is actually executing, whatever anyone did to `argv[0]`. macOS and
|
||||
/// the BSDs have no `/proc` at all, so fall back to `ps`, whose `comm` is the
|
||||
/// full path there — on Linux it would be the name truncated to 15 characters,
|
||||
/// one short of `tty7-server-c7p5`, which is exactly why the fallback is a
|
||||
/// fallback and not the only branch.
|
||||
///
|
||||
/// The `/proc` glob lives *inside* the `[ -d /proc ]` arm on purpose. The far
|
||||
/// end runs these through the user's login shell, and zsh — the default on
|
||||
/// macOS — aborts the whole command line when a glob matches nothing, so with
|
||||
/// the loop at top level the trailing `true` never ran and every one of these
|
||||
/// was a silent no-op on every Mac. That is what left `restart_daemon` waiting
|
||||
/// out its ten seconds for a daemon nobody had asked to stop.
|
||||
const RUNNING_EXE_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-*) printf '%s' "${e% (deleted)}"; break;; esac; done; else ps -xwwo pid=,comm= 2>/dev/null | while read -r pid e; do case "$e" in */tty7-server-*) printf '%s' "$e"; break;; esac; done; fi; true"#;
|
||||
|
||||
const TERMINATE_RUNNING_COMMAND: &str = r#"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; true"#;
|
||||
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 {
|
||||
let bin = shell_quote(binary);
|
||||
|
||||
@@ -1029,6 +1029,55 @@ fn the_running_exe_probe_cannot_fail_the_command() {
|
||||
assert!(TERMINATE_RUNNING_COMMAND.contains("*/tty7-server-*"));
|
||||
}
|
||||
|
||||
/// Both commands have to work on a machine with no `/proc`, which is every Mac
|
||||
/// and every BSD, and the `/proc` glob has to stay inside the guard: zsh is the
|
||||
/// login shell over there, and a top-level glob that matches nothing takes the
|
||||
/// rest of the command line with it — including the trailing `true`.
|
||||
#[test]
|
||||
fn finding_the_running_server_survives_a_machine_without_proc() {
|
||||
for cmd in [RUNNING_EXE_COMMAND, TERMINATE_RUNNING_COMMAND] {
|
||||
assert!(
|
||||
cmd.starts_with("if [ -d /proc ]; then"),
|
||||
"the glob has to be unreachable before the guard passes: {cmd}"
|
||||
);
|
||||
let (guarded, fallback) = cmd
|
||||
.split_once("; else ")
|
||||
.expect("a machine with no /proc still needs an answer");
|
||||
assert!(
|
||||
guarded.contains("/proc/[0-9]*") && !fallback.contains("/proc/"),
|
||||
"the fallback reads ps, not a filesystem that is not there: {cmd}"
|
||||
);
|
||||
assert!(
|
||||
fallback.contains("ps -xwwo pid=,comm="),
|
||||
"unwrapped, unabbreviated, and this user's processes only: {cmd}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Whichever branch the far end takes, the command has to parse — a syntax
|
||||
/// error here is invisible in production, where the output is read as "no
|
||||
/// server is running" and the failure is a ten-second timeout.
|
||||
///
|
||||
/// Parsed, not run: the terminate command would kill this developer's own
|
||||
/// server, and this test is not the place to find that out.
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn both_branches_of_the_probe_are_valid_shell() {
|
||||
for cmd in [RUNNING_EXE_COMMAND, TERMINATE_RUNNING_COMMAND] {
|
||||
let out = std::process::Command::new("/bin/sh")
|
||||
.arg("-n")
|
||||
.arg("-c")
|
||||
.arg(cmd)
|
||||
.output()
|
||||
.expect("every unix has /bin/sh");
|
||||
assert!(
|
||||
out.status.success(),
|
||||
"{cmd}\nstderr: {}",
|
||||
String::from_utf8_lossy(&out.stderr)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_failures_quote_stderr_when_there_is_any() {
|
||||
let with_stderr = ExecOutput {
|
||||
|
||||
+42
-21
@@ -6144,12 +6144,25 @@ impl Tty7App {
|
||||
let status = self.remote_status(cx)?;
|
||||
let machine = self.remote_machine_label(cx);
|
||||
let message = status.strip_message(&machine)?;
|
||||
let action = self.remote_strip_action(&status, cx);
|
||||
// While an install runs, the strip is that install's progress: the
|
||||
// refusal it is answering is no longer the news, and the button that
|
||||
// started it would only start a second one.
|
||||
let installing = self.remote_strip_progress(cx);
|
||||
let action = installing
|
||||
.is_none()
|
||||
.then(|| self.remote_strip_action(&status, cx))
|
||||
.flatten();
|
||||
let theme = cx.theme();
|
||||
let bar = gpui_component::h_flex()
|
||||
let message = match installing {
|
||||
Some(phase) => format!(
|
||||
"{machine} — {}",
|
||||
crate::ui::remote_workspace::install_phase_caption(phase)
|
||||
),
|
||||
None => message,
|
||||
};
|
||||
let bar = gpui_component::v_flex()
|
||||
.occlude()
|
||||
.items_center()
|
||||
.gap_2()
|
||||
.gap(px(6.))
|
||||
.px_3()
|
||||
.py_1p5()
|
||||
.rounded_lg()
|
||||
@@ -6159,25 +6172,33 @@ impl Tty7App {
|
||||
.shadow_md()
|
||||
.text_xs()
|
||||
.text_color(theme.muted_foreground)
|
||||
.child(gpui_component::Icon::new(gpui_component::IconName::Globe))
|
||||
.child(
|
||||
div()
|
||||
.font_weight(gpui::FontWeight::MEDIUM)
|
||||
.text_color(theme.foreground)
|
||||
.child(message),
|
||||
gpui_component::h_flex()
|
||||
.items_center()
|
||||
.gap_2()
|
||||
.child(gpui_component::Icon::new(gpui_component::IconName::Globe))
|
||||
.child(
|
||||
div()
|
||||
.font_weight(gpui::FontWeight::MEDIUM)
|
||||
.text_color(theme.foreground)
|
||||
.child(message),
|
||||
)
|
||||
.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")
|
||||
.label(label)
|
||||
.primary()
|
||||
.small()
|
||||
.on_click(cx.listener(move |this, _, window, cx| {
|
||||
this.run_strip_action(action.clone(), window, cx);
|
||||
})),
|
||||
)
|
||||
}),
|
||||
)
|
||||
.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")
|
||||
.label(label)
|
||||
.primary()
|
||||
.small()
|
||||
.on_click(cx.listener(move |this, _, window, cx| {
|
||||
this.run_strip_action(action.clone(), window, cx);
|
||||
})),
|
||||
)
|
||||
.when_some(installing, |this, phase| {
|
||||
this.child(crate::ui::remote_workspace::install_progress_bar(phase, cx))
|
||||
});
|
||||
Some(
|
||||
div()
|
||||
|
||||
+40
-17
@@ -259,12 +259,25 @@ impl Tty7App {
|
||||
let machine = self.remote_machine_label(cx);
|
||||
let status = self.remote_status(cx)?;
|
||||
let message = status.strip_message(&machine)?;
|
||||
let action = self.remote_strip_action(&status, cx);
|
||||
// An install in flight replaces both halves of the strip: its own line
|
||||
// instead of the complaint that is being answered, and no button, since
|
||||
// pressing Update Server again would start a second one on top of it.
|
||||
let installing = self.remote_strip_progress(cx);
|
||||
let action = installing
|
||||
.is_none()
|
||||
.then(|| self.remote_strip_action(&status, cx))
|
||||
.flatten();
|
||||
let theme = cx.theme();
|
||||
let message = match installing {
|
||||
Some(phase) => format!(
|
||||
"{machine} — {}",
|
||||
crate::ui::remote_workspace::install_phase_caption(phase)
|
||||
),
|
||||
None => message,
|
||||
};
|
||||
Some(
|
||||
h_flex()
|
||||
.items_center()
|
||||
.gap_2()
|
||||
v_flex()
|
||||
.gap(px(6.))
|
||||
.px(px(12.))
|
||||
.py(px(6.))
|
||||
.rounded(px(10.))
|
||||
@@ -273,19 +286,29 @@ impl Tty7App {
|
||||
.border_color(theme.border)
|
||||
.text_xs()
|
||||
.text_color(theme.muted_foreground)
|
||||
.child(gpui_component::Icon::new(IconName::Globe))
|
||||
.child(message)
|
||||
.when_some(action, |this, (label, action)| {
|
||||
this.child(
|
||||
Button::new("home-remote-status-action")
|
||||
.label(label)
|
||||
.ghost()
|
||||
.small()
|
||||
.on_click(cx.listener(move |this, _, window, cx| {
|
||||
this.run_strip_action(action.clone(), window, cx);
|
||||
}))
|
||||
.on_mouse_down(MouseButton::Left, |_, _, cx| cx.stop_propagation()),
|
||||
)
|
||||
.child(
|
||||
h_flex()
|
||||
.items_center()
|
||||
.gap_2()
|
||||
.child(gpui_component::Icon::new(IconName::Globe))
|
||||
.child(message)
|
||||
.when_some(action, |this, (label, action)| {
|
||||
this.child(
|
||||
Button::new("home-remote-status-action")
|
||||
.label(label)
|
||||
.ghost()
|
||||
.small()
|
||||
.on_click(cx.listener(move |this, _, window, cx| {
|
||||
this.run_strip_action(action.clone(), window, cx);
|
||||
}))
|
||||
.on_mouse_down(MouseButton::Left, |_, _, cx| {
|
||||
cx.stop_propagation()
|
||||
}),
|
||||
)
|
||||
}),
|
||||
)
|
||||
.when_some(installing, |this, phase| {
|
||||
this.child(crate::ui::remote_workspace::install_progress_bar(phase, cx))
|
||||
}),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -168,6 +168,56 @@ pub(crate) fn mismatch_action_key(refusal: &str) -> L10nKey {
|
||||
}
|
||||
}
|
||||
|
||||
/// One line for an install in flight — bytes moved, or the wait for the far end
|
||||
/// to come back up. Shared by the switcher's progress bar and the strip's, so a
|
||||
/// user watching both is not told two different things.
|
||||
pub(crate) fn install_phase_caption(phase: crate::daemon::install::InstallPhase) -> String {
|
||||
use crate::daemon::install::InstallPhase;
|
||||
use crate::ui::remote_connect::human_bytes;
|
||||
match phase {
|
||||
InstallPhase::Restarting => t(L10nKey::SwitcherRestartingServer).to_string(),
|
||||
InstallPhase::Downloading { done, total } => match total {
|
||||
Some(total) => t_fmt(
|
||||
L10nKey::SwitcherDownloadingServerWithTotal,
|
||||
&[("done", &human_bytes(done)), ("total", &human_bytes(total))],
|
||||
),
|
||||
None => t_fmt(
|
||||
L10nKey::SwitcherDownloadingServerNoTotal,
|
||||
&[("done", &human_bytes(done))],
|
||||
),
|
||||
},
|
||||
InstallPhase::Uploading { done, total } => t_fmt(
|
||||
L10nKey::SwitcherCopyingServer,
|
||||
&[("done", &human_bytes(done)), ("total", &human_bytes(total))],
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
/// The thin filled bar under an install's caption. `Restarting` has no
|
||||
/// fraction to show — the far end is either back or it is not — so it draws
|
||||
/// empty, which is what the switcher has always done and is still better than
|
||||
/// the bar vanishing for the last leg.
|
||||
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_component::ActiveTheme as _;
|
||||
let theme = cx.theme();
|
||||
gpui::div()
|
||||
.w_full()
|
||||
.h(gpui::px(3.))
|
||||
.rounded_full()
|
||||
.bg(theme.border)
|
||||
.child(
|
||||
gpui::div()
|
||||
.h_full()
|
||||
.w(gpui::relative(phase.fraction().unwrap_or(0.0)))
|
||||
.rounded_full()
|
||||
.bg(theme.warning),
|
||||
)
|
||||
}
|
||||
|
||||
/// What the remote strip's button is for, once the status has been read.
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub(crate) enum StripAction {
|
||||
@@ -401,6 +451,22 @@ impl Tty7App {
|
||||
})
|
||||
}
|
||||
|
||||
/// What the install running against *this* window's machine has reached, if
|
||||
/// one is running at all.
|
||||
///
|
||||
/// The switcher has drawn this since installs got a progress bar, but the
|
||||
/// strip never did — so pressing Update Server on a parked workspace with
|
||||
/// no switcher open froze the screen for as long as the download took and
|
||||
/// then produced a modal, with nothing in between. Same source, so
|
||||
/// the two cannot disagree about how far along it is.
|
||||
pub(crate) fn remote_strip_progress(
|
||||
&self,
|
||||
cx: &gpui::App,
|
||||
) -> Option<crate::daemon::install::InstallPhase> {
|
||||
let own = WorkspaceStore::remote_ref(cx, self.workspace)?;
|
||||
remote_connect::install_progress_for(own.target.host_id())
|
||||
}
|
||||
|
||||
pub(crate) fn run_strip_action(
|
||||
&mut self,
|
||||
action: StripAction,
|
||||
|
||||
+2
-18
@@ -18,7 +18,7 @@ use crate::daemon::install::InstallPhase;
|
||||
use crate::terminal::pane_liveness::Liveness;
|
||||
use crate::ui::app::Tty7App;
|
||||
use crate::ui::i18n::{L10nKey, t, t_fmt};
|
||||
use crate::ui::remote_connect::{self, HostChoice, RemoteWorkspaceRow, human_bytes};
|
||||
use crate::ui::remote_connect::{self, HostChoice, RemoteWorkspaceRow};
|
||||
use crate::ui::remote_workspace::{ConnectFlow, MachineStatus, RemoteLinks};
|
||||
|
||||
const CARD_W: f32 = 840.0;
|
||||
@@ -2156,23 +2156,7 @@ impl Tty7App {
|
||||
let theme = cx.theme();
|
||||
let accent = theme.warning;
|
||||
let fraction = phase.fraction().unwrap_or(0.0);
|
||||
let caption = match phase {
|
||||
InstallPhase::Restarting => t(L10nKey::SwitcherRestartingServer).to_string(),
|
||||
InstallPhase::Downloading { done, total } => match total {
|
||||
Some(total) => t_fmt(
|
||||
L10nKey::SwitcherDownloadingServerWithTotal,
|
||||
&[("done", &human_bytes(done)), ("total", &human_bytes(total))],
|
||||
),
|
||||
None => t_fmt(
|
||||
L10nKey::SwitcherDownloadingServerNoTotal,
|
||||
&[("done", &human_bytes(done))],
|
||||
),
|
||||
},
|
||||
InstallPhase::Uploading { done, total } => t_fmt(
|
||||
L10nKey::SwitcherCopyingServer,
|
||||
&[("done", &human_bytes(done)), ("total", &human_bytes(total))],
|
||||
),
|
||||
};
|
||||
let caption = crate::ui::remote_workspace::install_phase_caption(phase);
|
||||
|
||||
v_flex()
|
||||
.gap(px(6.))
|
||||
|
||||
Reference in New Issue
Block a user