DEVLAUNCH_NO_TTY has one reading again, not two

CHANGELOG.md

@@ -7,6 +7,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- **`DEVLAUNCH_NO_TTY=FALSE` no longer means one thing to the prompt and another
+  to the transport.** `dl` gates its own terminal behaviour on the same variable
+  the ssh transport is gated on, and the two read it differently: core lowercases
+  the value and strips it the way Python's `str.strip()` does before comparing it
+  against the falsey words, while `dl` kept a copy that compared the raw bytes
+  with a bare `matches!`. So `FALSE` and `" no "` opted out of the prompt and not
+  out of the pty, and a non-UTF-8 value opted out of the *pty* and not the prompt
+  — `std::env::var(..).ok()` reports such a value as unset, which is the
+  opt-out-into-opt-in inversion `osext` exists to prevent and names in as many
+  words. The copy existed because `osext` was `pub(crate)` and neither half of
+  what makes the reading right — the lossy read or Python's strip — can be
+  spelled without it, so the fix is a seam rather than a third copy:
+  `clients::ssh::tty_disabled_by_environment` is binary surface, and `dl` asks it.
+  The deleted tests only ever covered the spellings where the two agreed, which is
+  why the divergence was invisible.
+
 ### Added
 
 - **CI fails when nothing reviewed a pull request.** Sourcery answers a quota

rust/aid/tests/interactive.rs

@@ -264,6 +264,53 @@ fn the_title_switch_really_silences_it_on_a_real_pty() {
     assert_eq!(session.wait(), 0);
 }
 
+#[test]
+fn a_falsey_no_tty_leaves_the_terminal_alone_on_a_real_pty() {
+    // One variable, one reading. `aid`'s prompt and the ssh transport are both
+    // gated on `DEVLAUNCH_NO_TTY`, and they used to disagree about what it says:
+    // core lowercased and stripped the value before comparing it against the
+    // falsey words, and `dl`'s own copy of that predicate compared the raw bytes
+    // with a bare `matches!`. So `FALSE` — a spelling a person writes without
+    // thinking, and the one `DEVLAUNCH_NO_TITLE=FALSE` would get right — kept the
+    // pty for the transport and took it away from the prompt.
+    //
+    // Cased *and* padded, because those were two separate halves of the divergence
+    // (`to_lowercase` and `osext::strip`) and either one alone still hides the
+    // other. The banner appearing is the whole assertion: no banner means aid
+    // decided there was no terminal to prompt at.
+    // One world for both spellings rather than one each: these tests are timing
+    // sensitive under a loaded `--workspace` run, and a scenario build is the
+    // expensive half of a case that only needs a warm workspace to prompt about.
+    let world = World::with(&["--warm"]);
+    for value in ["FALSE", " no "] {
+        let mut session = PtyAid::spawn(&world, &[MAIN], &[("DEVLAUNCH_NO_TTY", value)]);
+        session.expect(BANNER);
+        session.send_line("fix the bug");
+        session.expect("aid -> dl");
+        assert_eq!(session.wait(), 0, "DEVLAUNCH_NO_TTY={value:?}");
+    }
+
+    // And the truthy direction, in the same test because it is the same claim:
+    // the reading is consulted at all. Without this, the wrapper's whole body
+    // could be `false` — killing `DEVLAUNCH_NO_TTY=1` for every `dl` and `aid`
+    // there is — and all three crates stay green, which is what deleting `dl`'s
+    // own truthy test left behind.
+    //
+    // Asserted by what a skipped prompt *does* rather than by waiting out a
+    // banner that never comes: with no terminal to prompt at, `aid` hands the
+    // agent line straight to dl, so `aid -> dl` arriving with nothing typed is
+    // the opt-out working. Waiting for the banner's absence would cost the
+    // 60-second deadline on the passing path.
+    let opted_out = PtyAid::spawn(&world, &[MAIN], &[("DEVLAUNCH_NO_TTY", "1")]);
+    opted_out.expect("aid -> dl");
+    assert!(
+        !opted_out.text().contains(BANNER),
+        "the editor prompted anyway; the pty said:\n{}",
+        opted_out.text()
+    );
+    assert_eq!(opted_out.wait(), 0);
+}
+
 #[test]
 fn a_typed_prompt_reaches_the_agent_with_no_shell_in_the_way() {
     // The double quotes are the point: they reach the agent literally, because

rust/devlaunch-core/public-api.rest.txt

@@ -219,6 +219,7 @@ pub fn devlaunch_core::clients::ssh::UnsafeRequest::eq(&self, &devlaunch_core::c
 impl core::fmt::Debug for devlaunch_core::clients::ssh::UnsafeRequest
 pub fn devlaunch_core::clients::ssh::UnsafeRequest::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result
 impl core::marker::StructuralPartialEq for devlaunch_core::clients::ssh::UnsafeRequest
+pub fn devlaunch_core::clients::ssh::tty_disabled_by_environment() -> bool
 pub mod devlaunch_core::domain
 pub mod devlaunch_core::domain::config
 pub enum devlaunch_core::domain::config::ConfigError

rust/devlaunch-core/src/clients/ssh.rs

@@ -154,6 +154,37 @@
     }
 }
 
+/// [`tty_disabled`], asked of this process's environment.
+///
+/// binary surface — not part of the frozen wf API (#251 §7)
+///
+/// The one reading of `DEVLAUNCH_NO_TTY`, because there was briefly more than
+/// one. `dl` gates its own terminal behaviour on the same variable, could not
+/// reach [`crate::osext`] from outside the crate, and so grew a copy built from
+/// `std::env::var(..).ok()` and a bare `matches!` over the falsey words. The copy
+/// disagreed with this module three ways: `FALSE` and ` no ` were read as
+/// opt-outs because it dropped the lowercasing and [`crate::osext::strip`], and a
+/// non-UTF-8 value read as *unset* — the opt-out-into-opt-in inversion `osext`
+/// exists to prevent, and the one this hatch shares with `DEVLAUNCH_NO_GH_TOKEN`.
+///
+/// So this is deliberately not the sharing [`FALSEY`]'s own note argues against.
+/// That note is about two *different* hatches answering to one constant, which
+/// would make an edit meant for one silently move the other. This is one hatch
+/// with one reading, which is the thing that was broken.
+///
+/// It would stay a function here even if [`crate::osext::env_str`] were reachable
+/// from the binaries, and the reason is arithmetic rather than the crate wall:
+/// what `dl` asks for is the *decision*, not the value. Composing it out there
+/// instead would want [`tty_disabled`] and [`DISABLE_VAR`] exported too — three
+/// items to say what one says — and it would put the composition back on the side
+/// of the wall that got it wrong.
+///
+/// Impure and therefore untested, like [`config_path`] beneath it: the predicate
+/// it wraps is where the spellings are pinned.
+pub fn tty_disabled_by_environment() -> bool {
+    tty_disabled(crate::osext::env_str(DISABLE_VAR).as_deref())
+}
+
 /// Whether dl was run from a terminal it can hand to the workspace.
 ///
 /// Both directions have to be a terminal. Python also has to defend against a
@@ -466,6 +497,24 @@
         assert!(!tty_disabled(None));
     }
 
+    #[test]
+    fn a_falsey_word_is_falsey_however_it_is_cased_and_padded() {
+        // The three spellings `dl`'s own copy of this predicate got wrong before
+        // it was deleted for [`tty_disabled_by_environment`]. It compared the raw
+        // value against the four words with a bare `matches!`, so each of these
+        // was "set, therefore yes" to the prompt and "no" to the ssh transport —
+        // one variable, two answers.
+        //
+        // The third spelling `dl` got wrong is not here because it is not this
+        // function's: a non-UTF-8 value read through `std::env::var(..).ok()`
+        // arrives as `None`. That inversion is pinned on the reader instead, at
+        // `osext::a_non_utf8_value_is_present_not_absent`.
+        for value in ["FALSE", " no ", "No", "\tfalse\n", "0 "] {
+            assert!(!tty_disabled(Some(value)), "{value:?}");
+            assert!(terminal_usable(Some(value), true, true), "{value:?}");
+        }
+    }
+
     // ------------------------------------------ has devpod published an alias
 
     #[test]

rust/dl/src/lib.rs

@@ -31,6 +31,7 @@ mod select;
 mod session;
 mod target;
 
+use devlaunch_core::clients::ssh;
 use devlaunch_core::flows::completion_cache;
 use devlaunch_core::flows::lifecycle::{Refresh, RefreshReason};
 use devlaunch_core::runner::ProcessRunner;
@@ -231,19 +232,11 @@ pub fn install_signal_handlers() {
 pub fn interactive_terminal() -> bool {
     // SAFETY: `isatty` reads a property of the descriptor and touches nothing.
     let tty = unsafe { libc::isatty(0) == 1 && libc::isatty(1) == 1 };
-    tty && !no_tty_requested(std::env::var("DEVLAUNCH_NO_TTY").ok().as_deref())
-}
-
-/// Whether `DEVLAUNCH_NO_TTY` asked for no terminal behaviour.
-///
-/// The falsey list is `clients/ssh.rs`'s (`FALSEY`), copied rather than shared for
-/// the reason that module gives: escape hatches answering to one shared constant
-/// are one edit away from becoming one escape hatch.
-fn no_tty_requested(value: Option<&str>) -> bool {
-    match value {
-        None => false,
-        Some(value) => !matches!(value, "" | "0" | "false" | "no"),
-    }
+    // Core's reading, not a copy of it. The copy that used to live here answered
+    // `std::env::var(..).ok()` and a bare `matches!` over the falsey words, so
+    // `FALSE`, ` no ` and a non-UTF-8 value each meant one thing to the ssh
+    // transport and the other to the prompt below.
+    tty && !ssh::tty_disabled_by_environment()
 }
 
 /// Read one submission from a cooked-mode terminal: the line the user ends with
@@ -511,29 +504,6 @@ fn report_timing() {
     }
 }
 
-#[cfg(test)]
-mod terminal {
-    //! The `DEVLAUNCH_NO_TTY` reading [`interactive_terminal`] shares with the ssh
-    //! transport: unset and the four falsey spellings mean "the terminal stands",
-    //! anything else means "behave as if there were none".
-
-    use super::no_tty_requested;
-
-    #[test]
-    fn unset_and_falsey_values_keep_the_terminal() {
-        for value in [None, Some(""), Some("0"), Some("false"), Some("no")] {
-            assert!(!no_tty_requested(value), "{value:?}");
-        }
-    }
-
-    #[test]
-    fn any_other_value_is_a_request_for_no_terminal() {
-        for value in ["1", "true", "yes", "anything"] {
-            assert!(no_tty_requested(Some(value)), "{value:?}");
-        }
-    }
-}
-
 #[cfg(test)]
 mod build_marker {
     //! What [`BUILD_MARKER`] is, asserted once per build rather than once.