chore: sync upstream devenv main 72e18a7a (RIG-5030)

.github/workflows/build.yml

@@ -124,6 +124,14 @@ jobs:
         run: |
           ${{ steps.build-devenv.outputs.path }}/bin/devenv --system ${{ inputs.system }} shell -- cargo nextest run -j 4 --features devenv/test-all --retries 3
 
+      # devenv-tasks is outside Cargo's default workspace members. Run its
+      # process scheduling regressions explicitly on Linux and macOS.
+      - name: Check concurrent process scheduling
+        run: |
+          ${{ steps.build-devenv.outputs.path }}/bin/devenv --system ${{ inputs.system }} shell -- \
+            cargo nextest run -p devenv-tasks -j 2 --retries 0 \
+              -E 'test(concurrent_dynamic_starts_share_never_started_process_prerequisite) | test(concurrent_start_stop_keeps_explicitly_stopped_dependencies_off)'
+
       # Exercise the actual terminal boundary on every platform: PTY resize,
       # crossterm key decoding, rendering, and reactive process commands.
       - name: Run deterministic TUI replay regression

CHANGELOG.md

@@ -2,11 +2,19 @@
 
 ## 2.4.1 (unreleased)
 
+### Improvements
+
+- Added the `tui.emoji` user setting. Set it to `false` to replace the emoji in the interactive shell status line, such as the eye in `devenv watching 11 files`, with non-emoji Unicode glyphs. It defaults to `true`, and the status line stays visible with file watching and automatic reload unchanged.
+
 ### Bug Fixes
 
+- Task cancellation now stops status checks and waits for nested commands to stop before the scheduler exits ([#3251](https://github.com/cachix/devenv/issues/3251)).
+- Fixed rapid process restarts failing because a stopped process still held its ownership claim.
+- Starting a process on demand now starts never-started, enabled process dependencies. Explicitly stopped and disabled dependencies stay off ([#3229](https://github.com/cachix/devenv/issues/3229)).
+- Fixed `devenv machines install` hanging while uploading store paths when no SSH connection was already open ([#3254](https://github.com/cachix/devenv/issues/3254)).
 - Nushell hot reload no longer breaks commands that use structured environment variables and now removes variables that disappeared after a reload ([#3230](https://github.com/cachix/devenv/issues/3230)).
 - MySQL now creates configured databases and users when started with `devenv up`, including `devenv up mysql` ([#2843](https://github.com/cachix/devenv/issues/2843)).
-- fish auto-activation no longer hangs when `env` is a fish function, such as grc's wrapper ([#3222](https://github.com/cachix/devenv/issues/3222)).
+- fish auto-activation no longer hangs when `env` is a fish function, such as grc's wrapper; `cat`, `rm` and `devenv` functions are bypassed too ([#3222](https://github.com/cachix/devenv/issues/3222)).
 - Task names containing `::`, such as `foo::bar`, are now rejected with an invalid task name error. Previously they were accepted but could not be run by name ([#3227](https://github.com/cachix/devenv/issues/3227)).
 - Detached process startup now publishes the manager as soon as processes are scheduled, without waiting for service readiness, and keeps HTTPS proxy routes during a slow startup ([#3216](https://github.com/cachix/devenv/issues/3216)).
 - `devenv test` with the native process manager now reports a process dependency on `start.enable = false` before starting services, instead of waiting indefinitely for the disabled process ([#3215](https://github.com/cachix/devenv/issues/3215)).

devenv-processes/src/manager.rs

@@ -233,6 +233,8 @@ pub struct JobHandle {
     pub status_rx: tokio::sync::watch::Receiver<crate::supervisor_state::JobStatus>,
     /// Supervisor task handling restarts
     pub supervisor_task: JoinHandle<()>,
+    /// Job controller task, joined before publishing a completed stop.
+    pub job_task: JoinHandle<()>,
     /// Channel to send lifecycle commands (restart/stop) into the supervisor loop.
     pub cmd_tx: mpsc::Sender<crate::supervisor::SupervisorCommand>,
     /// Output reader tasks (stdout, stderr)
@@ -344,6 +346,7 @@ fn lifecycle_phase(terminal_phase: Option<ProcessPhase>) -> ProcessPhase {
 /// Resources consumed by [`ProcessRunner::finish_stop`].
 struct StopParts {
     job: Arc<Job>,
+    job_task: JoinHandle<()>,
     cmd_tx: mpsc::Sender<crate::supervisor::SupervisorCommand>,
     supervisor_task: JoinHandle<()>,
     notify_forwarder: JoinHandle<()>,
@@ -371,6 +374,7 @@ fn take_active_for_stop(
         resources,
         cmd_tx,
         supervisor_task,
+        job_task,
         notify_forwarder,
         output_readers,
         ..
@@ -398,6 +402,7 @@ fn take_active_for_stop(
 
     StopParts {
         job,
+        job_task,
         cmd_tx,
         supervisor_task,
         notify_forwarder,
@@ -780,6 +785,7 @@ async fn wait_for_port_conflicts_to_settle(ports: &[u16], timeout: Duration) ->
 /// Everything a launch produces before the entry settles to Active.
 struct LaunchSetup {
     job: Arc<Job>,
+    job_task: JoinHandle<()>,
     status_tx: tokio::sync::watch::Sender<crate::supervisor_state::JobStatus>,
     status_rx: tokio::sync::watch::Receiver<crate::supervisor_state::JobStatus>,
     notify_socket: Option<Arc<NotifySocket>>,
@@ -798,6 +804,9 @@ impl LaunchSetup {
         self.stdout_tailer.abort();
         self.stderr_tailer.abort();
         crate::process_guardian::stop_job(&self.job, &self.scopes, &self.shutdown).await;
+        self.job.unset_spawn_child_fn().await;
+        self.job.delete().await;
+        let _ = self.job_task.await;
     }
 }
 
@@ -1395,6 +1404,7 @@ impl ProcessRunner {
                                 status_rx: setup.status_rx,
                                 cmd_tx,
                                 supervisor_task,
+                                job_task: setup.job_task,
                                 output_readers: Some((setup.stdout_tailer, setup.stderr_tailer)),
                                 notify_forwarder,
                             }),
@@ -1479,7 +1489,7 @@ impl ProcessRunner {
         let _ = std::fs::write(&proc_cmd.stdout_log, "");
         let _ = std::fs::write(&proc_cmd.stderr_log, "");
 
-        let (job, _task) = start_job(proc_cmd.command);
+        let (job, job_task) = start_job(proc_cmd.command);
         let job = Arc::new(job);
         let scopes = Arc::new(crate::process_guardian::ProcessScopeRegistry::default());
 
@@ -1597,7 +1607,9 @@ impl ProcessRunner {
 
         job.start().await;
         if let Ok(error) = start_error_rx.try_recv() {
+            job.unset_spawn_child_fn().await;
             job.delete().await;
+            let _ = job_task.await;
             bail!("Failed to spawn process '{}': {}", config.name, error);
         }
         // Spawn file tailers to emit output to activity
@@ -1618,6 +1630,7 @@ impl ProcessRunner {
 
         Ok(LaunchSetup {
             job,
+            job_task,
             status_tx,
             status_rx,
             notify_socket,
@@ -1637,6 +1650,7 @@ impl ProcessRunner {
     async fn finish_stop(&self, name: &str, parts: StopParts) {
         let StopParts {
             job,
+            job_task,
             cmd_tx,
             supervisor_task,
             notify_forwarder,
@@ -1655,6 +1669,12 @@ impl ProcessRunner {
         }
 
         crate::process_guardian::stop_job(&job, &scopes, &shutdown).await;
+        // The controller's spawn hook owns the process claim. Dropping the
+        // last job handle only queues its exit, so a rapid restart can race
+        // that release. Join the controller before making Stopped visible.
+        job.unset_spawn_child_fn().await;
+        job.delete().await;
+        let _ = job_task.await;
 
         if !ports.is_empty() {
             let release_status =
@@ -3124,21 +3144,40 @@ mod tests {
         let manager = ProcessRunner::new(temp_dir.path().to_path_buf()).unwrap();
         let config = long_running_config("restartable");
 
-        manager.start_command(&config, None).await.unwrap();
+        // A caller may still hold its launch result when stop finishes.
+        let original_job = manager.start_command(&config, None).await.unwrap().unwrap();
         assert!(manager.list().await.contains(&"restartable".to_string()));
+        // A custom spawner can hold ownership too, and is shared with the
+        // retained job handle independently of the controller task.
+        let custom_owner = Arc::new(());
+        let owner = Arc::downgrade(&custom_owner);
+        original_job
+            .set_spawn_child_fn(move |_| {
+                let _owner = &custom_owner;
+                Err(std::io::Error::other("stopped job must not spawn"))
+            })
+            .await;
 
         manager.stop_and_keep("restartable").await.unwrap();
         assert_eq!(
             manager.get_phase("restartable").await,
             Some(ProcessPhase::Stopped)
         );
 
+        assert!(
+            owner.upgrade().is_none(),
+            "stopped job retained its custom spawner"
+        );
         manager.start_not_started("restartable").await.unwrap();
         assert!(
             manager.list().await.contains(&"restartable".to_string()),
             "process should be active again after restart"
         );
 
+        assert!(
+            original_job.is_dead(),
+            "stopped job must release its ownership claim"
+        );
         let _ = manager.stop_all().await;
     }
 

devenv-run-tests/src/main.rs

@@ -555,18 +555,30 @@ async fn run_tests_in_directory(args: &RunArgs) -> Result<Vec<TestResult>> {
 
         // Run .patch.sh if it exists (must run before loading config)
         let patch_script = PathBuf::from(".patch.sh");
-        if patch_script.exists() {
+        let patch_result = if patch_script.exists() {
             devenv_activity::message(ActivityLevel::Info, "Running .patch.sh");
             let mut command = tokio::process::Command::new("bash");
             command.arg(&patch_script);
             with_test_env_path(&mut command);
-            let _ = command.status().await.into_diagnostic()?;
-        }
+            let status = command.status().await.into_diagnostic()?;
+            if status.success() {
+                Ok(())
+            } else {
+                Err(miette::miette!(
+                    "Patch script failed. Status code: {}",
+                    status.code().unwrap_or(1)
+                ))
+            }
+        } else {
+            Ok(())
+        };
 
         // A script to run inside the shell before the test.
         let setup_script = ".setup.sh";
 
-        let status: miette::Result<()> = if test_config.use_shell {
+        let status: miette::Result<()> = if let Err(error) = patch_result {
+            Err(error)
+        } else if test_config.use_shell {
             // Now load config from the current directory (which might be temp dir)
             let mut config = Config::load_from(&devenv_root)?;
             for input in args.override_inputs.chunks_exact(2) {

devenv-shell/src/indicators.rs

@@ -0,0 +1,127 @@
+//! Display indicators that have an emoji and a non-emoji form.
+//!
+//! Renderers ask [`Indicators`] for a glyph instead of hard-coding one, so the
+//! `tui.emoji` user setting is applied in exactly one place.
+//!
+//! Emoji glyphs are not width-stable: terminals disagree on whether they occupy
+//! one or two cells, and Unicode width tables report one. The non-emoji glyphs
+//! are plain symbols that occupy a single cell, like the checkmark and spinner
+//! already used by the status line, so the label that follows lines up with
+//! every other status.
+
+use crate::status_line::{DOT_RING, DOT_RUNNING};
+
+/// A display indicator with an emoji and a non-emoji form.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum Indicator {
+    /// File watching is paused.
+    Paused,
+    /// Files are being watched for changes.
+    Watching,
+}
+
+impl Indicator {
+    /// Every indicator.
+    pub const ALL: [Indicator; 2] = [Indicator::Paused, Indicator::Watching];
+}
+
+/// A glyph and the space that separates it from the label that follows.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub struct Glyph {
+    /// The text to draw.
+    pub text: &'static str,
+    /// Columns between the glyph and its label.
+    pub gap: u32,
+}
+
+/// Selects the glyph used for each [`Indicator`].
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub struct Indicators {
+    emoji: bool,
+}
+
+impl Indicators {
+    /// Select emoji glyphs when `emoji` is true and non-emoji glyphs otherwise.
+    pub const fn new(emoji: bool) -> Self {
+        Self { emoji }
+    }
+
+    /// Whether emoji glyphs are selected.
+    pub const fn emoji(self) -> bool {
+        self.emoji
+    }
+
+    /// The glyph for `indicator`.
+    pub const fn glyph(self, indicator: Indicator) -> Glyph {
+        match (self.emoji, indicator) {
+            // The historical glyphs and spacing.
+            (true, Indicator::Paused) => Glyph {
+                text: "⏸", gap: 2
+            },
+            (true, Indicator::Watching) => Glyph {
+                text: "👁", gap: 2
+            },
+            // The status dots used for process states: alive while watching,
+            // an idle ring while paused. They are single-cell symbols, so the
+            // label starts in the same column as after the checkmark, cross,
+            // and spinner.
+            (false, Indicator::Paused) => Glyph {
+                text: DOT_RING,
+                gap: 1,
+            },
+            (false, Indicator::Watching) => Glyph {
+                text: DOT_RUNNING,
+                gap: 1,
+            },
+        }
+    }
+}
+
+impl Default for Indicators {
+    /// Emoji glyphs, preserving the existing output.
+    fn default() -> Self {
+        Self::new(true)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn defaults_to_emoji() {
+        assert_eq!(Indicators::default(), Indicators::new(true));
+        assert!(Indicators::default().emoji());
+    }
+
+    #[test]
+    fn emoji_glyphs_are_the_historical_ones() {
+        let indicators = Indicators::new(true);
+        assert_eq!(indicators.glyph(Indicator::Paused).text, "⏸");
+        assert_eq!(indicators.glyph(Indicator::Watching).text, "👁");
+    }
+
+    #[test]
+    fn non_emoji_glyphs_are_single_non_emoji_characters() {
+        for indicator in Indicator::ALL {
+            let glyph = Indicators::new(false).glyph(indicator);
+            let mut chars = glyph.text.chars();
+            assert!(chars.next().is_some());
+            assert!(
+                chars.next().is_none(),
+                "{indicator:?} must be one character"
+            );
+            assert_eq!(glyph.gap, 1);
+        }
+    }
+
+    #[test]
+    fn glyphs_differ_between_modes() {
+        for indicator in Indicator::ALL {
+            assert_ne!(
+                Indicators::new(true).glyph(indicator),
+                Indicators::new(false).glyph(indicator)
+            );
+        }
+    }
+}

devenv-shell/src/lib.rs

@@ -6,6 +6,7 @@
 pub mod dialect;
 pub mod escape;
 pub mod escape_state;
+pub mod indicators;
 pub mod keybindings;
 mod protocol;
 mod pty;

devenv-shell/src/session.rs

@@ -8,6 +8,7 @@ use crate::escape_state::{
     EscapeState, cleanup_forwarded_modes as escape_state_cleanup,
     process_escape_events as escape_state_process,
 };
+use crate::indicators::Indicators;
 use crate::keybindings::{ShellAction, ShellKeybindings};
 use crate::protocol::{ShellCommand, ShellEvent};
 use crate::pty::{Pty, PtyError, get_terminal_size};
@@ -1642,6 +1643,12 @@ impl ShellSession {
         self
     }
 
+    /// Select the glyphs used for status-line indicators.
+    pub fn with_indicators(mut self, indicators: Indicators) -> Self {
+        self.status_line.set_indicators(indicators);
+        self
+    }
+
     /// Wire a shutdown token. On cancellation the session kills the inner
     /// shell so devenv can exit instead of orphaning it after a terminal
     /// hangup or SIGHUP/SIGINT/SIGTERM.