feat(cli): migrate --log-format/--force-tty-if-json to clap-native env=

crates/deacon/src/cli.rs

@@ -120,7 +120,7 @@ pub enum OutputFormat {
 }
 
 /// Log format options
-#[derive(Debug, Clone, ValueEnum)]
+#[derive(Debug, Clone, ValueEnum, PartialEq, Eq)]
 pub enum LogFormat {
     /// Human-readable text format
     Text,
@@ -879,7 +879,7 @@ pub enum ConfigCommands {
 )]
 pub struct Cli {
     /// Log format (text or json, defaults to text, can be set via DEACON_LOG_FORMAT env var)
-    #[arg(long, global = true, value_enum)]
+    #[arg(long, global = true, value_enum, env = "DEACON_LOG_FORMAT")]
     pub log_format: Option<LogFormat>,
 
     /// Log level (baseline verbosity). Shifted by repeated `-v`/`-q`.
@@ -1025,7 +1025,7 @@ pub struct Cli {
     ///
     /// When disabled (default), lifecycle commands run without PTY allocation. This is suitable
     /// for non-interactive scripts and automated environments.
-    #[arg(long, global = true)]
+    #[arg(long, global = true, env = "DEACON_FORCE_TTY_IF_JSON")]
     pub force_tty_if_json: bool,
 
     /// Default user env probe mode (none|loginInteractiveShell|interactiveShell|loginShell)
@@ -1142,16 +1142,12 @@ impl Cli {
     /// assert!(ctx.workspace_folder.is_none());
     /// ```
     /// Returns true when the effective log format is JSON.
-    /// `--log-format json` wins; if unset, `DEACON_LOG_FORMAT=json` counts too
-    /// (matches the fallback in deacon_core::logging::init).
+    ///
+    /// `self.log_format` already reflects flag-over-env-over-unset: clap's `env=`
+    /// on `--log-format` resolves `DEACON_LOG_FORMAT` into the field, so no manual
+    /// env read is needed here.
     pub fn is_json_log_format(&self) -> bool {
-        match self.log_format {
-            Some(LogFormat::Json) => true,
-            Some(LogFormat::Text) => false,
-            None => std::env::var("DEACON_LOG_FORMAT")
-                .map(|v| v == "json")
-                .unwrap_or(false),
-        }
+        matches!(self.log_format, Some(LogFormat::Json))
     }
 
     #[allow(dead_code)] // Reserved for future command implementations; see runtime_utils
@@ -1423,6 +1419,7 @@ impl Cli {
                     // JSON log format auto-forces PTY allocation so lifecycle exec output
                     // stays usable; the explicit flag remains as a manual override.
                     force_tty_if_json: self.force_tty_if_json || json_format,
+                    json_log_format: json_format,
                     trust_workspace: self.trust_workspace,
                     trust_workspace_persist: self.trust_workspace_persist,
                     host_ca_activation,
@@ -2018,33 +2015,99 @@ mod tests {
 
     /// BEAD-11-T03: --log-format json must auto-force PTY allocation so the
     /// downstream ExecArgs.force_tty_if_json is true even without the explicit flag.
+    ///
+    /// Both fields now read the environment via clap `env=` (#180), so the env is
+    /// pinned explicitly to keep the assertion deterministic under any test runner.
     #[test]
     fn test_json_log_format_implies_force_tty() {
-        let cli = Cli::parse_from(["deacon", "--log-format", "json"]);
-        assert!(cli.is_json_log_format());
-        assert!(!cli.force_tty_if_json); // user didn't pass the explicit flag
-        // The dispatch site ORs these two together; the test of that wiring
-        // is here at the source of truth (cli.is_json_log_format()) since
-        // dispatch is async and harder to unit-test in isolation.
-        let effective_force_tty = cli.force_tty_if_json || cli.is_json_log_format();
-        assert!(effective_force_tty);
+        temp_env::with_vars(
+            [
+                ("DEACON_LOG_FORMAT", None::<&str>),
+                ("DEACON_FORCE_TTY_IF_JSON", None::<&str>),
+            ],
+            || {
+                let cli = Cli::parse_from(["deacon", "--log-format", "json"]);
+                assert!(cli.is_json_log_format());
+                assert!(!cli.force_tty_if_json); // user didn't pass the explicit flag
+                // The dispatch site ORs these two together; the test of that wiring
+                // is here at the source of truth (cli.is_json_log_format()) since
+                // dispatch is async and harder to unit-test in isolation.
+                let effective_force_tty = cli.force_tty_if_json || cli.is_json_log_format();
+                assert!(effective_force_tty);
+            },
+        );
     }
 
     /// Inverse: explicit --log-format text leaves the auto-derive off.
     #[test]
     fn test_text_log_format_does_not_imply_force_tty() {
-        let cli = Cli::parse_from(["deacon", "--log-format", "text"]);
-        assert!(!cli.is_json_log_format());
-        let effective_force_tty = cli.force_tty_if_json || cli.is_json_log_format();
-        assert!(!effective_force_tty);
+        temp_env::with_vars(
+            [
+                ("DEACON_LOG_FORMAT", None::<&str>),
+                ("DEACON_FORCE_TTY_IF_JSON", None::<&str>),
+            ],
+            || {
+                let cli = Cli::parse_from(["deacon", "--log-format", "text"]);
+                assert!(!cli.is_json_log_format());
+                let effective_force_tty = cli.force_tty_if_json || cli.is_json_log_format();
+                assert!(!effective_force_tty);
+            },
+        );
     }
 
     /// Explicit --force-tty-if-json still works without --log-format json.
     #[test]
     fn test_explicit_force_tty_without_json_log_format() {
-        let cli = Cli::parse_from(["deacon", "--force-tty-if-json"]);
-        assert!(cli.force_tty_if_json);
-        assert!(!cli.is_json_log_format());
+        temp_env::with_var_unset("DEACON_LOG_FORMAT", || {
+            let cli = Cli::parse_from(["deacon", "--force-tty-if-json"]);
+            assert!(cli.force_tty_if_json);
+            assert!(!cli.is_json_log_format());
+        });
+    }
+
+    // --- #180 Category B (part 2): clap-native env= for --log-format / --force-tty-if-json ---
+
+    #[test]
+    fn test_log_format_defaults_none() {
+        temp_env::with_var_unset("DEACON_LOG_FORMAT", || {
+            let cli = Cli::parse_from(["deacon"]);
+            assert_eq!(cli.log_format, None);
+            assert!(!cli.is_json_log_format());
+        });
+    }
+
+    #[test]
+    fn test_log_format_reads_env_var() {
+        temp_env::with_var("DEACON_LOG_FORMAT", Some("json"), || {
+            let cli = Cli::parse_from(["deacon"]);
+            assert_eq!(cli.log_format, Some(LogFormat::Json));
+            assert!(cli.is_json_log_format());
+        });
+    }
+
+    #[test]
+    fn test_log_format_flag_beats_env_var() {
+        temp_env::with_var("DEACON_LOG_FORMAT", Some("json"), || {
+            let cli = Cli::parse_from(["deacon", "--log-format", "text"]);
+            assert_eq!(cli.log_format, Some(LogFormat::Text));
+            assert!(!cli.is_json_log_format());
+        });
+    }
+
+    #[test]
+    fn test_force_tty_reads_env_var() {
+        temp_env::with_var("DEACON_FORCE_TTY_IF_JSON", Some("true"), || {
+            let cli = Cli::parse_from(["deacon"]);
+            assert!(cli.force_tty_if_json);
+        });
+    }
+
+    #[test]
+    fn test_force_tty_env_falsey_stays_off() {
+        temp_env::with_var("DEACON_FORCE_TTY_IF_JSON", Some("false"), || {
+            let cli = Cli::parse_from(["deacon"]);
+            assert!(!cli.force_tty_if_json);
+        });
     }
 
     /// Helper for the BEAD-07 tests: parse an exec invocation and pull out the

crates/deacon/src/commands/up/args.rs

@@ -484,6 +484,12 @@ pub struct UpArgs {
     pub secret_registry: deacon_core::redaction::SecretRegistry,
     pub force_tty_if_json: bool,
 
+    /// Whether the effective log format is JSON (resolved by clap from
+    /// `--log-format`/`DEACON_LOG_FORMAT` at the CLI tier). Threaded here so the
+    /// lifecycle/compose PTY gating no longer re-reads the env var itself, which
+    /// missed `--log-format json` passed as a flag (#180).
+    pub json_log_format: bool,
+
     /// `--trust-workspace` flag (one-shot trust, does not persist).
     pub trust_workspace: bool,
     /// `--trust-workspace-persist` flag (one-shot + writes to the trust store).
@@ -555,6 +561,7 @@ impl Default for UpArgs {
             redaction_config: deacon_core::redaction::RedactionConfig::default(),
             secret_registry: deacon_core::redaction::global_registry().clone(),
             force_tty_if_json: false,
+            json_log_format: false,
             trust_workspace: false,
             trust_workspace_persist: false,
             host_ca_activation: deacon_core::host_ca::HostCaActivation::Off,

crates/deacon/src/commands/up/compose.rs

@@ -5,7 +5,6 @@
 //! - `execute_compose_post_create` - Post-create lifecycle for compose
 //! - `handle_compose_shutdown` - Shutdown handling for compose
 
-use super::ENV_LOG_FORMAT;
 use super::args::{MountType, NormalizedMount, UpArgs};
 use super::features_build::{
     FeatureBuildOutput, build_image_with_features, build_image_with_features_from_dockerfile,
@@ -537,11 +536,9 @@ pub(crate) async fn execute_compose_up(
 
     // Execute post-create lifecycle if not skipped
     if !args.skip_post_create {
-        // Resolve PTY preference for compose post-create (same logic as lifecycle commands)
-        let json_mode = std::env::var(ENV_LOG_FORMAT)
-            .map(|v| v == "json")
-            .unwrap_or(false);
-        let force_pty = resolve_force_pty(args.force_tty_if_json, json_mode);
+        // Resolve PTY preference for compose post-create (same logic as lifecycle commands).
+        // json_mode is threaded from the clap-resolved --log-format/DEACON_LOG_FORMAT (#180).
+        let force_pty = resolve_force_pty(args.force_tty_if_json, args.json_log_format);
         execute_compose_post_create(
             &project,
             config,

crates/deacon/src/commands/up/lifecycle.rs

@@ -1,14 +1,13 @@
 //! Lifecycle command execution for the up command.
 //!
 //! This module contains:
-//! - `resolve_force_pty` - Resolve PTY preference based on flags and environment
+//! - `resolve_force_pty` - Resolve PTY preference based on the resolved flag and JSON mode
 //! - `build_invocation_context` - Build InvocationContext from CLI args and prior state
 //! - `execute_lifecycle_commands` - Execute lifecycle phases in container
 //! - `execute_initialize_command` - Execute initializeCommand on host (with workspace-trust gate)
 //! - `HostTrustArgs` / `enforce_host_trust` - Workspace-trust gate primitives
 
 use super::args::UpArgs;
-use super::{ENV_FORCE_TTY_IF_JSON, ENV_LOG_FORMAT};
 use anyhow::{Context, Result};
 use deacon_core::config::DevContainerConfig;
 use deacon_core::container_lifecycle::{
@@ -25,27 +24,15 @@ use std::path::{Path, PathBuf};
 use std::time::Duration;
 use tracing::{Level, debug, info, instrument, span, warn};
 
-/// Resolve PTY preference for lifecycle commands based on flag, environment, and JSON mode
+/// Resolve PTY preference for lifecycle commands based on the resolved flag and JSON mode.
 ///
-/// Per FR-002, FR-005: PTY toggle only applies in JSON log mode.
-/// Precedence: CLI flag > env var `DEACON_FORCE_TTY_IF_JSON` > default (false)
+/// Per FR-002, FR-005: PTY toggle only applies in JSON log mode. `flag` already
+/// reflects `--force-tty-if-json`/`DEACON_FORCE_TTY_IF_JSON` — clap's `env=`
+/// resolves the env var into the field at the CLI tier (#180), so no manual env
+/// read is needed here.
 pub(crate) fn resolve_force_pty(flag: bool, json_mode: bool) -> bool {
-    // PTY toggle only applies when in JSON log mode
-    if !json_mode {
-        return false;
-    }
-
-    // CLI flag takes precedence
-    if flag {
-        return true;
-    }
-
-    // Check environment variable (truthy: true/1/yes; falsey: false/0/no or unset)
-    if let Ok(val) = std::env::var(ENV_FORCE_TTY_IF_JSON) {
-        matches!(val.to_lowercase().as_str(), "true" | "1" | "yes")
-    } else {
-        false // default: no PTY
-    }
+    // PTY toggle only applies when in JSON log mode.
+    json_mode && flag
 }
 
 /// Build an `InvocationContext` from CLI arguments and prior state markers.
@@ -207,21 +194,19 @@ pub(crate) async fn execute_lifecycle_commands(
         args.mount_workspace_git_root,
     );
 
-    // Determine if JSON log mode is active by checking DEACON_LOG_FORMAT env var
-    // Per FR-001, FR-002: PTY toggle only applies in JSON log mode
-    let json_mode = std::env::var(ENV_LOG_FORMAT)
-        .map(|v| v == "json")
-        .unwrap_or(false);
+    // Whether JSON log mode is active — resolved by clap from
+    // `--log-format`/`DEACON_LOG_FORMAT` at the CLI tier and threaded via
+    // UpArgs. Per FR-001, FR-002: PTY toggle only applies in JSON log mode.
+    // (Previously re-read the env var here, which missed `--log-format json`
+    // passed as a flag — #180.)
+    let json_mode = args.json_log_format;
 
-    // Resolve PTY preference based on flag, env, and JSON mode
+    // Resolve PTY preference based on flag and JSON mode.
     let force_pty = resolve_force_pty(args.force_tty_if_json, json_mode);
 
     debug!(
-        "PTY preference resolved: force_pty={}, json_mode={}, flag={}, env={}",
-        force_pty,
-        json_mode,
-        args.force_tty_if_json,
-        std::env::var(ENV_FORCE_TTY_IF_JSON).unwrap_or_else(|_| "unset".to_string())
+        "PTY preference resolved: force_pty={}, json_mode={}, flag={}",
+        force_pty, json_mode, args.force_tty_if_json,
     );
 
     let wait_for = wait_for_phase(config.wait_for.as_deref())?;

crates/deacon/src/commands/up/mod.rs

@@ -74,9 +74,6 @@ use merged_config::merge_image_metadata_into_config;
 /// for lifecycle exec commands during `deacon up` when JSON logging is active.
 pub(crate) const ENV_FORCE_TTY_IF_JSON: &str = "DEACON_FORCE_TTY_IF_JSON";
 
-/// Environment variable name for log format detection.
-pub(crate) const ENV_LOG_FORMAT: &str = "DEACON_LOG_FORMAT";
-
 /// Starts development containers for the current workspace according to the resolved devcontainer configuration.
 ///
 /// This is the top-level entry point for the `up` command. It: