use std::collections::HashMap;
use std::io::Write;
use std::path::Path;
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};

use crate::error::CreftError;
use crate::model::{CodeBlock, LlmConfig, ParsedCommand};
use crate::search::index::SearchIndex;

/// Per-block coverage trace record, serialised as NDJSON on `CREFT_TRACE_FD`.
///
/// One record is emitted per block on completion. The framework reads these from
/// the child process to build per-skill coverage reports without parsing block
/// source code.
#[derive(Debug, serde::Serialize, serde::Deserialize)]
pub(crate) struct TraceRecord {
    /// Zero-based block index in the parsed command's `blocks` vector.
    pub block: usize,
    /// Block language tag as it appeared in the markdown fence (e.g. `"bash"`).
    pub lang: String,
    /// Final exit code. Follows the Unix shell convention:
    /// - Normal exit: the wait-status code.
    /// - `creft_exit <code>`: the side-channel code (not the wait status, which is 0).
    /// - Signal kill: `128 + signal` (e.g. SIGINT → 130).
    pub exit: i32,
    /// Count of side-channel primitive invocations observed by the runner.
    ///
    /// Keys are `ChannelMessage` JSON tag strings. Variants the block did not
    /// invoke are omitted to keep records compact.
    pub primitives: std::collections::BTreeMap<String, u32>,
}

/// Shared trace writer, guarded by a mutex for cross-thread serialisation.
///
/// Pipe chains spawn reader threads that may all emit trace records. A `Mutex`
/// ensures each NDJSON record is written atomically. `Arc` allows cheap cloning
/// across threads without duplicating the underlying `Mutex<File>`.
pub(crate) type TraceWriter = Arc<std::sync::Mutex<std::fs::File>>;

mod blocks;
#[cfg(unix)]
pub(crate) mod channel;
mod pipe;
mod preamble;
#[cfg(unix)]
mod signal;
mod substitute;

pub(crate) use self::blocks::spawn_block;
pub(crate) use self::substitute::substitute;
// Re-exported for use by cmd::run and the Stage 4 scenario runner; not yet
// called in production code until Stage 4 lands.
#[cfg(unix)]
#[allow(unused_imports)]
pub(crate) use self::channel::os_pipe;

/// A runtime search index with its access control flag.
///
/// Created by `creft_index` during skill execution. Accumulates documents
/// across multiple calls to the same index name: each call appends a new
/// document and rebuilds the XOR filter from all accumulated content.
/// Not persisted to disk — runtime indexes are ephemeral, scoped to a
/// single skill invocation.
///
/// The `is_global` flag is set from the most recent `creft_index` call's
/// `global` field. When `true`, other namespaces may query this index via
/// `creft_search` using a dotted name. When `false`, only the owning
/// namespace may query it.
#[derive(Debug)]
pub(crate) struct RuntimeIndex {
    /// The current search index, rebuilt from `documents` on every
    /// `creft_index` call.
    pub index: SearchIndex,
    /// Whether this index is queryable from other namespaces.
    /// Set from the most recent `creft_index` call's `global` flag.
    pub is_global: bool,
    /// Raw documents accumulated by successive `creft_index` calls.
    /// Each entry is `(document_name, content)`. The document name is a
    /// sequential label (e.g., `"doc_0"`, `"doc_1"`) used as a key into the
    /// XOR filter. Search results look up the matched document name here and
    /// return the original content, not the internal label.
    pub documents: Vec<(String, String)>,
}

/// Execution context for a single skill invocation.
///
/// Carries all state that runner functions previously received as individual
/// parameters: working directory, environment variables, runtime flags, and
/// a cancellation token for cooperative shutdown.
///
/// Constructed once per skill invocation in `run_user_command`.
/// Shared across threads via `Arc` (sponge threads, reaper threads).
#[derive(Debug, Clone)]
pub(crate) struct RunContext {
    /// Cancellation token. Set to `true` by the SIGINT handler.
    /// Threads poll this to know when to stop.
    cancel: Arc<AtomicBool>,

    /// Working directory for subprocess execution.
    cwd: std::path::PathBuf,

    /// Extra environment variables injected into every child process.
    env: Vec<(String, String)>,

    /// Whether `--verbose` was passed. Controls block rendering before execution.
    verbose: bool,

    /// Whether `--dry-run` was passed. Controls execution vs. print-only.
    dry_run: bool,

    /// Caller's preferred shell (e.g., "zsh", "bash").
    ///
    /// When set, shell-family blocks (`bash`, `sh`, `zsh`) use this shell
    /// instead of the block's declared language tag. Set from shell detection
    /// at skill invocation time. `None` means use block language literally.
    shell_preference: Option<String>,

    /// Directory for persistent key-value stores.
    ///
    /// Passed into `PrimitiveContext` when spawning the channel reader thread.
    /// Populated from `AppContext::store_dir_for(Scope::Global)` at the call site.
    /// Defaults to an empty path when not set (store operations are no-ops in
    /// contexts that don't configure a store directory, such as dry-run).
    pub(crate) store_dir: std::path::PathBuf,

    /// The fully-qualified name of the skill being executed.
    ///
    /// Used to derive the caller's namespace and plugin context for namespace
    /// resolution when handling `creft_index` and `creft_search` messages.
    /// Empty string when no specific skill is being executed (e.g., dry-run
    /// previews outside of skill invocation).
    pub(crate) skill_name: String,

    /// Plugin name when the skill is sourced from an activated plugin.
    ///
    /// `Some(name)` when the skill comes from `SkillSource::Plugin(name)`.
    /// `None` for user-owned and package skills. Passed to `PrimitiveContext`
    /// so that `namespace::qualify` produces fully-qualified names with the
    /// plugin prefix (e.g., `acme.deploy.beta` instead of `deploy.beta`).
    pub(crate) plugin: Option<String>,

    /// Runtime indexes created by `creft_index` during this execution.
    ///
    /// Keyed by fully-qualified name (e.g., `"deploy.beta"`, `"acme.deploy.beta"`).
    /// Access control is enforced per-index via the `is_global` flag on `RuntimeIndex`.
    ///
    /// `Arc` is required because `RunContext` derives `Clone` and `Mutex` does not
    /// implement `Clone`. All clones of a `RunContext` share the same index map,
    /// so indexes created in one execution phase are visible in subsequent phases.
    pub(crate) runtime_indexes: Arc<std::sync::Mutex<HashMap<String, RuntimeIndex>>>,

    /// Optional NDJSON trace sink for per-block coverage emission.
    ///
    /// `None` for every terminal-driven `creft <skill>` invocation. `Some` only
    /// when `cmd::run::run_user_command` resolved a writer from `CREFT_TRACE_FD`.
    ///
    /// `Arc` allows the rebuild in `run_inner` to clone the field cheaply; the
    /// underlying `Mutex<File>` is shared across every reader and sponge thread
    /// so all threads write through the same lock to the same fd.
    trace_writer: Option<TraceWriter>,
}

impl RunContext {
    pub(crate) fn new(
        cancel: Arc<AtomicBool>,
        cwd: std::path::PathBuf,
        env: Vec<(String, String)>,
        verbose: bool,
        dry_run: bool,
    ) -> Self {
        Self {
            cancel,
            cwd,
            env,
            verbose,
            dry_run,
            shell_preference: None,
            store_dir: std::path::PathBuf::new(),
            skill_name: String::new(),
            plugin: None,
            runtime_indexes: Arc::new(std::sync::Mutex::new(HashMap::new())),
            trace_writer: None,
        }
    }

    /// Set the directory for persistent key-value stores.
    ///
    /// Derived from `AppContext::store_dir_for(Scope::Global)` at the call site
    /// and threaded into `PrimitiveContext` when the channel reader thread is spawned.
    pub(crate) fn with_store_dir(mut self, dir: std::path::PathBuf) -> Self {
        self.store_dir = dir;
        self
    }

    /// Set the fully-qualified skill name for namespace resolution.
    ///
    /// Used by `creft_index` and `creft_search` to derive the caller's namespace
    /// when qualifying or resolving resource names.
    pub(crate) fn with_skill_name(mut self, name: impl Into<String>) -> Self {
        self.skill_name = name.into();
        self
    }

    /// Set the plugin name when the skill is sourced from an activated plugin.
    ///
    /// Pass `Some(plugin_name)` for `SkillSource::Plugin` skills. Pass `None`
    /// for user-owned and package skills. The plugin name is threaded through
    /// to `PrimitiveContext` so that `namespace::qualify` produces correctly
    /// prefixed names (e.g., `acme.deploy.beta` for plugin `acme`).
    pub(crate) fn with_plugin(mut self, plugin: Option<String>) -> Self {
        self.plugin = plugin;
        self
    }

    /// Set the caller's shell preference.
    ///
    /// When `Some(shell)`, shell-family code blocks run under the given shell
    /// rather than the shell named in the block's language tag.
    pub(crate) fn with_shell_preference(mut self, preference: Option<String>) -> Self {
        self.shell_preference = preference;
        self
    }

    /// Attach a writer for per-block trace emission.
    ///
    /// When set, the runner writes one NDJSON `TraceRecord` per block after the
    /// block exits. When `None` (the default for all terminal-driven invocations),
    /// no records are written and there is no measurable overhead.
    pub(crate) fn with_trace_writer(mut self, writer: Option<TraceWriter>) -> Self {
        self.trace_writer = writer;
        self
    }

    /// Borrow the trace writer, if one was attached.
    pub(crate) fn trace_writer(&self) -> Option<&TraceWriter> {
        self.trace_writer.as_ref()
    }

    /// Build a `TraceWriter` from an inherited file descriptor.
    ///
    /// The runner module owns the shape of `TraceWriter` (`Arc<Mutex<File>>`).
    /// Callers outside the runner — currently only `cmd::run::run_user_command`
    /// — go through this constructor rather than building the type directly, so
    /// a future change to the writer's shape requires only a single-site edit.
    ///
    /// # Safety
    ///
    /// The caller must guarantee that:
    /// - `fd` is open and writeable at the moment of the call;
    /// - `fd` is owned exclusively by this process and is not closed or duplicated
    ///   elsewhere for as long as the returned `TraceWriter` lives;
    /// - the fd was acquired by the current process (typical: dup'd in via
    ///   `pre_exec` before exec, then read out of `CREFT_TRACE_FD`).
    ///
    /// The returned `TraceWriter` takes ownership of `fd` via `File::from_raw_fd`.
    /// The fd is closed when the last `Arc` clone is dropped at end-of-run.
    #[cfg(unix)]
    pub(crate) unsafe fn trace_writer_from_fd(fd: std::os::unix::io::RawFd) -> TraceWriter {
        use std::os::unix::io::FromRawFd;
        // SAFETY: precondition documented on this function; ownership of `fd`
        // transfers to the returned `File`.
        let file = unsafe { std::fs::File::from_raw_fd(fd) };
        Arc::new(std::sync::Mutex::new(file))
    }

    /// Borrow the shell preference, if one was detected.
    pub(crate) fn shell_preference(&self) -> Option<&str> {
        self.shell_preference.as_deref()
    }

    /// Check whether cancellation has been requested.
    pub(crate) fn is_cancelled(&self) -> bool {
        self.cancel.load(Ordering::Relaxed)
    }

    /// Borrow the working directory.
    pub(crate) fn cwd(&self) -> &std::path::Path {
        &self.cwd
    }

    /// Borrow the environment variables as a slice of string pairs.
    pub(crate) fn env_pairs(&self) -> Vec<(&str, &str)> {
        self.env
            .iter()
            .map(|(k, v)| (k.as_str(), v.as_str()))
            .collect()
    }

    /// Whether verbose output was requested.
    pub(crate) fn is_verbose(&self) -> bool {
        self.verbose
    }

    /// Whether dry-run mode was requested.
    pub(crate) fn is_dry_run(&self) -> bool {
        self.dry_run
    }

    /// Borrow the environment variable slice for cloning or inspection.
    pub(crate) fn env(&self) -> &[(String, String)] {
        &self.env
    }

    /// Request cancellation. Sets the shared cancel token to `true`.
    ///
    /// Safe to call from any thread. All clones of this context will observe
    /// the cancellation on the next `is_cancelled()` poll.
    pub(crate) fn request_cancel(&self) {
        self.cancel.store(true, Ordering::Relaxed);
    }

    /// Clone the underlying cancel Arc for passing to threads that require
    /// `'static` lifetime (e.g. relay threads in pipe chains).
    pub(super) fn cancel_arc(&self) -> Arc<AtomicBool> {
        Arc::clone(&self.cancel)
    }
}

/// Prefix applied to every arg-derived environment variable name.
///
/// Segregates user-controlled arg values from inherited environment variables
/// (`PATH`, `HOME`, `LANG`, …) and from framework-supplied variables
/// (`CREFT_PROJECT_ROOT`, `CREFT_DRY_RUN`, …). A skill arg named `path` maps
/// to `CREFT_ARG_PATH` and cannot overwrite the inherited `PATH`.
pub(crate) const CREFT_ARG_ENV_PREFIX: &str = "CREFT_ARG_";

/// Bound pairs (name → value) and any passthrough args.
pub type BindResult = (Vec<(String, String)>, Vec<String>);

/// Parse raw CLI args using lexopt, validate values, and apply frontmatter defaults.
///
/// Returns a vec of `(name, value)` pairs for template substitution plus any
/// passthrough args (currently always empty — all args must match the definition).
/// Unknown flags produce a parse error; there is no silent passthrough.
pub fn parse_and_bind(cmd: &ParsedCommand, raw_args: &[String]) -> Result<BindResult, CreftError> {
    use lexopt::prelude::*;

    // Raw parsed values collected during the lexopt loop.
    // Flags are keyed by their long name.
    let mut flag_values: std::collections::HashMap<String, String> =
        std::collections::HashMap::new();
    // Tracks whether a bool flag was set (present = true).
    let mut flag_bools: std::collections::HashSet<String> = std::collections::HashSet::new();
    // Positional values in arrival order.
    let mut positional: Vec<String> = Vec::new();

    // Build a lookup from short char → flag name so we can handle -v, -q, etc.
    let short_to_flag: std::collections::HashMap<char, &str> = cmd
        .def
        .flags
        .iter()
        .filter_map(|f| {
            f.short
                .as_deref()
                .and_then(|s| s.chars().next())
                .map(|ch| (ch, f.name.as_str()))
        })
        .collect();

    let mut parser = lexopt::Parser::from_args(raw_args.iter().map(String::as_str));

    while let Some(arg) = parser
        .next()
        .map_err(|e| CreftError::MissingArg(e.to_string()))?
    {
        match arg {
            Long(name) => {
                // Convert to owned so we can call parser.value() without
                // conflicting borrows from the Long(&str) lifetime.
                let name = name.to_owned();
                if let Some(flag_def) = cmd.def.flags.iter().find(|f| f.name == name) {
                    if flag_def.r#type == "bool" {
                        flag_bools.insert(flag_def.name.clone());
                    } else {
                        let err_name = name.clone();
                        let val = parser
                            .value()
                            .map_err(|e| CreftError::MissingArg(e.to_string()))?
                            .into_string()
                            .map_err(|_| {
                                CreftError::MissingArg(format!(
                                    "--{err_name}: value contains invalid UTF-8"
                                ))
                            })?;
                        flag_values.insert(flag_def.name.clone(), val);
                    }
                } else {
                    return Err(CreftError::MissingArg(format!(
                        "unexpected option --{name}"
                    )));
                }
            }
            Short(ch) => {
                if let Some(&flag_name) = short_to_flag.get(&ch) {
                    let flag_def = cmd
                        .def
                        .flags
                        .iter()
                        .find(|f| f.name == flag_name)
                        .expect("short_to_flag built from cmd.def.flags; name must exist");
                    if flag_def.r#type == "bool" {
                        flag_bools.insert(flag_def.name.clone());
                    } else {
                        let val = parser
                            .value()
                            .map_err(|e| CreftError::MissingArg(e.to_string()))?
                            .into_string()
                            .map_err(|_| {
                                CreftError::MissingArg(format!(
                                    "-{ch}: value contains invalid UTF-8"
                                ))
                            })?;
                        flag_values.insert(flag_def.name.clone(), val);
                    }
                } else {
                    return Err(CreftError::MissingArg(format!("unexpected option -{ch}")));
                }
            }
            Value(v) => {
                let s = v.into_string().map_err(|_| {
                    CreftError::MissingArg("positional argument contains invalid UTF-8".to_string())
                })?;
                positional.push(s);
            }
        }
    }

    let mut pairs: Vec<(String, String)> = Vec::new();

    // Bind positional args in declaration order.
    for (i, arg_def) in cmd.def.args.iter().enumerate() {
        let val = if let Some(v) = positional.get(i) {
            v.clone()
        } else if let Some(d) = &arg_def.default {
            d.clone()
        } else if !arg_def.required {
            // Optional arg with no explicit default — bind to empty string so
            // {{name}} resolves to "" rather than erroring. Users who want a
            // non-empty fallback can still use {{name|fallback}} in the template.
            String::new()
        } else {
            return Err(CreftError::MissingArg(arg_def.name.clone()));
        };

        // Regex validation — skip when optional arg was not provided (value is empty).
        if let Some(pattern) = &arg_def.validation
            && (!val.is_empty() || arg_def.required)
        {
            let re = regex::Regex::new(pattern).map_err(|e| {
                CreftError::Frontmatter(format!("invalid validation regex '{}': {}", pattern, e))
            })?;
            if !re.is_match(&val) {
                return Err(CreftError::ValidationFailed {
                    name: arg_def.name.clone(),
                    value: val,
                    pattern: pattern.clone(),
                });
            }
        }

        pairs.push((arg_def.name.clone(), val));
    }

    // Bind flags in declaration order.
    for flag_def in &cmd.def.flags {
        let val = if flag_def.r#type == "bool" {
            flag_bools.contains(&flag_def.name).to_string()
        } else if let Some(v) = flag_values.get(&flag_def.name) {
            v.clone()
        } else if let Some(d) = &flag_def.default {
            d.clone()
        } else {
            // String flag with no default and not provided — bind to empty
            // string so {{flagname}} resolves to "" rather than erroring.
            String::new()
        };

        // Regex validation for string flags — skip when empty (not provided).
        if flag_def.r#type != "bool"
            && let Some(pattern) = &flag_def.validation
            && !val.is_empty()
        {
            let re = regex::Regex::new(pattern).map_err(|e| {
                CreftError::Frontmatter(format!("invalid validation regex '{}': {}", pattern, e))
            })?;
            if !re.is_match(&val) {
                return Err(CreftError::ValidationFailed {
                    name: flag_def.name.clone(),
                    value: val,
                    pattern: pattern.clone(),
                });
            }
        }

        pairs.push((flag_def.name.clone(), val));
    }

    Ok((pairs, vec![]))
}

/// Check that required env vars are set.
pub fn check_env(cmd: &ParsedCommand) -> Result<(), CreftError> {
    for var in &cmd.def.env {
        if var.required && std::env::var(&var.name).is_err() {
            return Err(CreftError::MissingEnvVar(var.name.clone()));
        }
    }
    Ok(())
}

/// Map a code block language tag to an interpreter command.
pub(crate) fn interpreter(lang: &str) -> &str {
    match lang {
        "bash" => "bash",
        "sh" => "sh",
        "zsh" => "zsh",
        "python" => "python3",
        "python3" => "python3",
        "node" | "javascript" | "js" => "node",
        "typescript" | "ts" => "npx tsx",
        "perl" => "perl",
        other => other,
    }
}

/// File extension for a language.
pub(crate) fn extension(lang: &str) -> &str {
    match lang {
        "bash" | "sh" | "zsh" => "sh",
        "python" | "python3" => "py",
        "node" | "javascript" | "js" => "js",
        "typescript" | "ts" => "ts",
        "perl" => "pl",
        other => other,
    }
}

/// Create a temporary script file for a code block.
///
/// When `preamble` is `Some`, the preamble is written before `expanded_code`.
/// If `expanded_code` starts with a shebang (`#!`), the shebang line is
/// written first and the preamble follows it — so the kernel's script loader
/// sees the interpreter directive at byte 0.
///
/// Returns the temp file handle (must be kept alive until the child exits).
pub(crate) fn prepare_block_script(
    block: &CodeBlock,
    expanded_code: &str,
    preamble: Option<&str>,
) -> Result<tempfile::NamedTempFile, CreftError> {
    // The `# extension:` directive overrides the temp file's suffix. Validation
    // (validate_skill) rejects malformed values at add time; this line trusts
    // that guarantee and uses the value verbatim. Unknown tags without an
    // extension directive fall through to `extension()`, which returns the lang
    // tag verbatim (e.g., "ruby" for a ruby block) — the resulting suffix is
    // unused by StdinRunner, so the fabricated name is harmless.
    let ext: &str = block
        .extension
        .as_deref()
        .unwrap_or_else(|| extension(&block.lang));
    let mut tmp = tempfile::Builder::new()
        .prefix("creft-")
        .suffix(&format!(".{}", ext))
        .tempfile()
        .map_err(CreftError::Io)?;

    if let Some(pre) = preamble {
        // If the script opens with a shebang, the kernel requires it at
        // byte 0. Split the shebang off, write it, then inject the preamble
        // before the rest of the user code.
        if expanded_code.starts_with("#!") {
            let (shebang_line, rest) = expanded_code
                .split_once('\n')
                .unwrap_or((expanded_code, ""));
            tmp.write_all(shebang_line.as_bytes())
                .map_err(CreftError::Io)?;
            tmp.write_all(b"\n").map_err(CreftError::Io)?;
            tmp.write_all(pre.as_bytes()).map_err(CreftError::Io)?;
            tmp.write_all(rest.as_bytes()).map_err(CreftError::Io)?;
        } else {
            tmp.write_all(pre.as_bytes()).map_err(CreftError::Io)?;
            tmp.write_all(expanded_code.as_bytes())
                .map_err(CreftError::Io)?;
        }
    } else {
        tmp.write_all(expanded_code.as_bytes())
            .map_err(CreftError::Io)?;
    }

    tmp.flush().map_err(CreftError::Io)?;

    Ok(tmp)
}

/// Simple which(1) equivalent.
pub(crate) fn which(name: &str) -> Option<std::path::PathBuf> {
    std::env::var_os("PATH").and_then(|paths| {
        std::env::split_paths(&paths)
            .map(|p| p.join(name))
            .find(|p| p.exists())
    })
}

/// Return the plain exit code for a process status, or `None` if the process
/// was killed by a signal (Unix) or terminated abnormally (Windows).
///
/// On Unix this corresponds to `ExitStatusExt::code()`: `Some(n)` for a
/// voluntary exit, `None` for a signal kill.
pub(crate) fn exit_code_of(status: &std::process::ExitStatus) -> Option<i32> {
    status.code()
}

/// Build the appropriate `CreftError` for a failed child process.
///
/// On Unix, if the process was killed by a signal (`ExitStatus::code()` is
/// `None`), returns `ExecutionSignaled` with the signal number. Otherwise
/// returns `ExecutionFailed` with the exit code.
pub(crate) fn make_execution_error(
    block: usize,
    lang: &str,
    status: &std::process::ExitStatus,
) -> CreftError {
    #[cfg(unix)]
    {
        use std::os::unix::process::ExitStatusExt;
        if let Some(sig) = status.signal() {
            return CreftError::ExecutionSignaled {
                block,
                lang: lang.to_string(),
                signal: sig,
            };
        }
    }
    CreftError::ExecutionFailed {
        block,
        lang: lang.to_string(),
        code: status.code().unwrap_or(1),
    }
}

/// Format the command that `build_llm_command` would produce as a display string.
///
/// Used by dry-run and verbose output.
fn format_llm_command(config: &LlmConfig) -> String {
    let provider = if config.provider.is_empty() {
        "claude"
    } else {
        &config.provider
    };

    let mut parts: Vec<String> = vec![provider.to_string()];

    match provider {
        "claude" => {
            parts.push("-p".to_string());
            if !config.model.is_empty() {
                parts.push("--model".to_string());
                parts.push(config.model.clone());
            }
        }
        "gemini" => {
            parts.push("-p".to_string());
            if !config.model.is_empty() {
                parts.push("-m".to_string());
                parts.push(config.model.clone());
            }
        }
        "codex" => {
            parts.push("exec".to_string());
            parts.push("-".to_string());
        }
        "ollama" => {
            parts.push("run".to_string());
            if !config.model.is_empty() {
                parts.push(config.model.clone());
            }
        }
        _ => {
            if !config.model.is_empty() {
                parts.push("--model".to_string());
                parts.push(config.model.clone());
            }
        }
    }

    if !config.params.is_empty() {
        for token in config.params.split_whitespace() {
            parts.push(token.to_string());
        }
    }

    parts.join(" ")
}

/// Write one NDJSON `TraceRecord` to the trace writer if one is attached.
///
/// Silently swallows write errors: a broken read end (parent gave up) returns
/// `EPIPE`, and a poisoned mutex means the process is already in trouble.
/// Either way, trace emission is best-effort; block execution is unaffected.
pub(super) fn emit_trace(ctx: &RunContext, record: &TraceRecord) {
    let Some(writer) = ctx.trace_writer() else {
        return;
    };
    if let Ok(mut file) = writer.lock() {
        // serde_json::to_string is infallible for this well-typed struct.
        let mut line = serde_json::to_string(record).unwrap_or_default();
        line.push('\n');
        // Ignore EPIPE and other write errors — trace emission is best-effort.
        let _ = file.write_all(line.as_bytes());
    }
}

/// Compute the trace exit code from a resolved exit code and the block's status.
///
/// Follows the Unix shell convention:
/// - `creft_exit <code>`: use the side-channel code directly.
/// - Normal exit: use the wait-status code.
/// - Signal kill: use `128 + signal`.
#[cfg(unix)]
fn trace_exit_code(creft_exit_code: Option<i32>, status: &std::process::ExitStatus) -> i32 {
    if let Some(code) = creft_exit_code {
        return code;
    }
    use std::os::unix::process::ExitStatusExt;
    if let Some(sig) = status.signal() {
        return 128 + sig;
    }
    status.code().unwrap_or(1)
}

/// Execute a single code block, capturing and echoing stdout.
///
/// Returns captured stdout as a `String`. Output is also printed to the
/// terminal so the user sees it in real time.
///
/// When `stdin_data` is `Some`, the child's stdin is piped and the data is
/// written on a background thread before `wait_with_output` drains stdout.
/// The background thread prevents the classic deadlock where a large stdin
/// payload fills the OS pipe buffer while the child is also blocked writing
/// to a full stdout pipe buffer. When `stdin_data` is `None`, the child
/// inherits the parent's stdin (terminal or upstream process).
fn execute_block(
    block: &CodeBlock,
    code: &str,
    block_idx: usize,
    ctx: &RunContext,
    bound_refs: &[(&str, &str)],
    stdin_data: Option<&[u8]>,
) -> Result<String, CreftError> {
    // Check cancellation before spawning any block — avoids starting a
    // potentially long-running process when SIGINT already fired.
    if ctx.is_cancelled() {
        return Err(CreftError::EarlyExit);
    }

    let mode = blocks::execution_mode_for(block);
    let flags = blocks::expand_and_split_flags(block, bound_refs)?;

    let (tmp, tmp_path) = match mode {
        blocks::ExecutionMode::File => {
            let pre = preamble::for_language(&block.lang);
            let tmp = prepare_block_script(block, code, pre.as_deref())?;
            let path = tmp.path().to_path_buf();
            (tmp, path)
        }
        blocks::ExecutionMode::Stdin => {
            // Path-only temp file; body is delivered via stdin_data.
            // The file exists for trait-signature uniformity; the runner
            // (StdinRunner) does not use the path.
            let tmp = prepare_block_script(block, "", None)?;
            let path = tmp.path().to_path_buf();
            (tmp, path)
        }
    };
    // Keep `tmp` alive until the child exits.
    let _tmp_guard = tmp;

    let stdin_cfg = if stdin_data.is_some() {
        std::process::Stdio::piped()
    } else {
        std::process::Stdio::inherit()
    };

    // Create the side channel before spawning so the child's pipe ends are
    // ready for inheritance via pre_exec/dup2.
    #[cfg(unix)]
    let mut side_channel = channel::SideChannel::new().map_err(CreftError::Io)?;

    let (mut child, _node_deps_dir) = spawn_block(
        block,
        &flags,
        &tmp_path,
        ctx,
        stdin_cfg,
        std::process::Stdio::piped(),
        #[cfg(unix)]
        None, // single-block mode: no process group management
        #[cfg(unix)]
        false, // single-block mode: do not suppress SIGINT
        #[cfg(unix)]
        Some(&side_channel),
    )?;

    // After spawn, close the parent's copies of the child's pipe ends.
    // If the parent keeps them open, the control-pipe reader will never
    // see EOF (the parent's write end keeps the pipe alive).
    #[cfg(unix)]
    side_channel.close_child_ends();

    // Spawn the side-channel reader thread that parses NDJSON from fd 3
    // and renders print/status messages to the terminal via stderr.
    // Runs concurrently with stdout capture so neither pipe blocks.
    //
    // Interactive prompts are only valid when the child's stdin is the
    // terminal (stdin_data is None). If stdin is piped, creft is using
    // the child's stdin for data — interactive reading from the same
    // terminal is not possible.
    #[cfg(unix)]
    let (reader_thread, prompt_thread, exit_signal, primitive_counter) = {
        let ctrl_reader = side_channel
            .take_control_reader()
            .expect("control reader taken exactly once per block");
        let resp_writer_fd = side_channel
            .take_response_writer()
            .expect("response writer taken exactly once per block");
        let writer = std::sync::Arc::new(channel::TerminalWriter::new());

        // Set up a channel so the reader thread can forward prompt messages
        // to the prompt handler thread.
        let (prompt_tx, prompt_rx) = std::sync::mpsc::channel::<channel::ChannelMessage>();

        // Interactive prompts require an unpiped stdin (terminal access).
        let interactive = stdin_data.is_none();

        let exit_signal: channel::ExitSignal = Arc::new(std::sync::Mutex::new(None));
        let primitive_counter: channel::PrimitiveCounter =
            Arc::new(std::sync::Mutex::new(std::collections::BTreeMap::new()));
        let search_ctx = Some(channel::PrimitiveContext {
            skill_name: ctx.skill_name.clone(),
            plugin: ctx.plugin.clone(),
            runtime_indexes: Arc::clone(&ctx.runtime_indexes),
            store_dir: ctx.store_dir.clone(),
            counter: Arc::clone(&primitive_counter),
        });

        // Duplicate the response writer fd so both the reader thread (which
        // handles creft_search responses) and the prompt handler thread (which
        // handles creft_prompt responses) can write to fd 4 independently.
        // Without this, creft_search messages have nowhere to send their
        // response and the bash block hangs forever on `read <&4`.
        //
        // SAFETY: resp_writer_fd is a valid, open fd we own. dup(2) returns a
        // new fd independent of the original; both are valid write ends of the
        // same pipe. OwnedFd takes ownership and closes on drop.
        let search_resp_writer: Option<std::fs::File> = {
            use std::os::unix::io::{AsRawFd as _, FromRawFd as _};
            let raw = unsafe { libc::dup(resp_writer_fd.as_raw_fd()) };
            if raw < 0 {
                return Err(CreftError::Io(std::io::Error::last_os_error()));
            }
            // SAFETY: dup(2) returned a non-negative fd we own exclusively.
            Some(unsafe { std::fs::File::from_raw_fd(raw) })
        };

        let reader = channel::spawn_reader(
            ctrl_reader,
            std::sync::Arc::clone(&writer),
            Some(prompt_tx),
            search_resp_writer,
            Some(Arc::clone(&exit_signal)),
            search_ctx,
        );
        let prompter =
            channel::spawn_prompt_handler(prompt_rx, resp_writer_fd, writer, interactive);
        (reader, prompter, exit_signal, primitive_counter)
    };

    // When prev_output data must be written to the child's stdin, do it on a
    // background thread so that stdout draining (via wait_with_output) and
    // stdin writes proceed concurrently. This prevents deadlock when
    // prev_output is large enough to fill the OS pipe buffer (~64 KB on Linux,
    // ~16 KB on macOS) before the child has read any of it.
    let stdin_thread = if let Some(data) = stdin_data {
        let owned: Vec<u8> = data.to_vec();
        let mut stdin_handle = child
            .stdin
            .take()
            .expect("stdin was piped but handle is missing");
        Some(std::thread::spawn(move || {
            use std::io::ErrorKind;
            match std::io::Write::write_all(&mut stdin_handle, &owned) {
                Ok(()) => Ok(()),
                // BrokenPipe means the child exited before reading all input.
                // The child's exit status is the authoritative error signal.
                Err(e) if e.kind() == ErrorKind::BrokenPipe => Ok(()),
                Err(e) => Err(e),
            }
        }))
    } else {
        None
    };

    // _node_deps_dir kept alive here so the npm-installed node_modules directory
    // is not deleted before the child process finishes.
    let output = child.wait_with_output().map_err(CreftError::Io)?;

    if let Some(handle) = stdin_thread {
        handle
            .join()
            .expect("stdin write thread panicked")
            .map_err(CreftError::Io)?;
    }

    // Join the reader thread after the child has exited and stdout has been
    // drained. The child's exit closes its fd 3 write end; close_child_ends()
    // above closed the parent's copy, so the reader will have seen EOF.
    // When the reader exits it drops prompt_tx, which closes the mpsc channel
    // and causes the prompt handler thread to exit.
    // Joining here ensures all side-channel messages are rendered before
    // the function returns, and that the exit_signal slot is final before
    // we read it below.
    #[cfg(unix)]
    {
        reader_thread
            .join()
            .expect("side-channel reader thread panicked");
        // Join the prompt handler after the reader: the reader exits first
        // (closes prompt_tx), which signals the prompt handler to finish.
        prompt_thread
            .join()
            .expect("side-channel prompt handler thread panicked");
    }

    // Check the creft_exit side-channel signal. The reader thread was joined
    // above, so the signal slot is final.
    #[cfg(unix)]
    {
        let signal_code = exit_signal
            .lock()
            .expect("exit signal lock poisoned")
            .take();
        if let Some(code) = signal_code {
            // Reader threads are joined — counter is final.
            let primitives = primitive_counter
                .lock()
                .map(|g| g.clone())
                .unwrap_or_default();
            emit_trace(
                ctx,
                &TraceRecord {
                    block: block_idx,
                    lang: block.lang.clone(),
                    exit: code,
                    primitives,
                },
            );
            // Flush any stdout the block produced before creft_exit.
            let stdout = String::from_utf8_lossy(&output.stdout).to_string();
            print!("{stdout}");
            if code == 0 {
                return Err(CreftError::EarlyExit);
            } else {
                return Err(CreftError::ExecutionFailed {
                    block: block_idx,
                    lang: block.lang.clone(),
                    code,
                });
            }
        }
    }

    // Collect the primitive counter now — the reader is joined so the counter is final.
    // On non-Unix, there is no side channel and the counter is always empty.
    #[cfg(unix)]
    let primitives_snapshot = primitive_counter
        .lock()
        .map(|g| g.clone())
        .unwrap_or_default();
    #[cfg(not(unix))]
    let primitives_snapshot = std::collections::BTreeMap::new();

    if !output.status.success() {
        // Suppress child stderr when killed by SIGINT: the user pressed Ctrl+C
        // deliberately and the interpreter's traceback (e.g., Python's
        // KeyboardInterrupt) is noise, not diagnostics.
        #[cfg(unix)]
        let suppress_stderr = {
            use std::os::unix::process::ExitStatusExt;
            output.status.signal() == Some(libc::SIGINT)
        };
        #[cfg(not(unix))]
        let suppress_stderr = false;

        if !suppress_stderr && !output.stderr.is_empty() {
            let _ = std::io::stderr().write_all(&output.stderr);
        }
        #[cfg(unix)]
        let exit_code = trace_exit_code(None, &output.status);
        #[cfg(not(unix))]
        let exit_code = output.status.code().unwrap_or(1);
        emit_trace(
            ctx,
            &TraceRecord {
                block: block_idx,
                lang: block.lang.clone(),
                exit: exit_code,
                primitives: primitives_snapshot,
            },
        );
        return Err(make_execution_error(block_idx, &block.lang, &output.status));
    }

    let stdout = String::from_utf8_lossy(&output.stdout).to_string();
    print!("{}", stdout);

    if ctx.is_verbose() && !output.stderr.is_empty() {
        let _ = writeln!(std::io::stderr(), "[block {} stderr]", block_idx + 1);
        let _ = std::io::stderr().write_all(&output.stderr);
    }

    emit_trace(
        ctx,
        &TraceRecord {
            block: block_idx,
            lang: block.lang.clone(),
            exit: 0,
            primitives: primitives_snapshot,
        },
    );
    Ok(stdout)
}

/// Convert bound pairs from `parse_and_bind` into environment variable pairs
/// for injection into child processes.
///
/// Names are upper-cased, hyphens are replaced with underscores, and the
/// fixed `CREFT_ARG_` prefix is prepended. The prefix segregates user-controlled
/// arg values from inherited environment variables (`PATH`, `HOME`, `LANG`, …)
/// and from framework-supplied vars (`CREFT_PROJECT_ROOT`, `CREFT_DRY_RUN`, …),
/// so that a skill arg named `path` cannot overwrite the inherited `PATH`.
///
/// The original bare names in the `bound` slice are unchanged for template
/// substitution (`{{name}}` and `{{name|default}}`); only the env-var keys
/// produced here carry the prefix.
fn bound_pairs_to_env(pairs: &[(String, String)]) -> Vec<(String, String)> {
    pairs
        .iter()
        .map(|(name, val)| {
            let env_name = format!(
                "{}{}",
                CREFT_ARG_ENV_PREFIX,
                name.replace('-', "_").to_uppercase()
            );
            (env_name, val.clone())
        })
        .collect()
}

/// Core execution logic. Uses `RunContext` for all execution configuration.
fn run_inner(cmd: &ParsedCommand, raw_args: &[String], ctx: &RunContext) -> Result<(), CreftError> {
    if cmd.blocks.is_empty() {
        return Err(CreftError::NoCodeBlocks);
    }

    check_env(cmd)?;

    let (bound, _passthrough) = parse_and_bind(cmd, raw_args)?;
    let bound_refs: Vec<(&str, &str)> = bound
        .iter()
        .map(|(k, v)| (k.as_str(), v.as_str()))
        .collect();

    // Inject bound args and flags as CREFT_ARG_<NAME> environment variables
    // into every child process. The prefix prevents arg names from colliding
    // with inherited variables like PATH or HOME.
    let mut extended_env = ctx.env().to_vec();
    extended_env.extend(bound_pairs_to_env(&bound));
    // Rebuild with extended env; struct-update syntax carries all other fields
    // (including trace_writer) without per-field maintenance. The Arc::clone
    // for trace_writer is cheap — the underlying Mutex<File> is shared across
    // every thread that emits a record.
    let ctx = RunContext {
        env: extended_env,
        ..ctx.clone()
    };

    if cmd.blocks.len() > 1 {
        #[cfg(unix)]
        {
            return pipe::run_pipe_chain(cmd, &bound_refs, &ctx);
        }
        #[cfg(not(unix))]
        {
            let has_llm_blocks = cmd.blocks.iter().any(|b| b.lang == "llm");
            if has_llm_blocks {
                return Err(CreftError::Setup(
                    "Multi-block skills with LLM blocks require Unix (macOS/Linux). \
                     LLM pipe stages use process groups which are not available on this platform."
                        .into(),
                ));
            }
            return pipe::run_pipe_chain(cmd, &bound_refs, &ctx);
        }
    }

    let block = &cmd.blocks[0];
    let expanded = substitute(&block.code, &bound_refs, &block.lang)?;

    // Sponge blocks (LLM and stdin-mode unknown tags) receive their expanded
    // content via stdin. Script-based blocks read from the temp file; stdin_data
    // is None and the child inherits the parent's stdin (terminal).
    let stdin_data = if block.needs_sponge() {
        Some(expanded.as_bytes())
    } else {
        None
    };

    match execute_block(block, &expanded, 0, &ctx, &bound_refs, stdin_data) {
        Ok(_) => Ok(()),
        Err(CreftError::EarlyExit) => Ok(()),
        Err(e) => Err(e),
    }
}

/// Run a full parsed command using a `RunContext`.
///
/// Returns immediately if cancellation has already been requested.
pub(crate) fn run(
    cmd: &ParsedCommand,
    raw_args: &[String],
    ctx: &RunContext,
) -> Result<(), CreftError> {
    if ctx.is_cancelled() {
        return Ok(());
    }
    run_inner(cmd, raw_args, ctx)
}

/// Print the expanded code for each block without executing it, using a `RunContext`.
pub(crate) fn dry_run(
    cmd: &ParsedCommand,
    raw_args: &[String],
    ctx: &RunContext,
) -> Result<(), CreftError> {
    dry_run_inner(cmd, raw_args, ctx.cwd())
}

/// Write rendered (substituted) blocks to stderr for diagnostic inspection.
///
/// Called when `--verbose` is active. Each block is shown with `===` delimiters
/// so the output is visually distinct from `--dry-run`'s `---` format.
pub fn render_blocks(cmd: &ParsedCommand, bound_refs: &[(&str, &str)]) -> Result<(), CreftError> {
    for (i, block) in cmd.blocks.iter().enumerate() {
        let expanded = substitute(&block.code, bound_refs, &block.lang)?;
        if block.lang == "llm" {
            if let Some(config) = &block.llm_config {
                let command_str = format_llm_command(config);
                eprintln!("=== block {} (llm: {}) ===", i + 1, config.provider);
                eprintln!("command: {}", command_str);
                eprintln!("prompt:");
                eprintln!("{}", expanded);
                eprintln!("=== end ===");
            } else {
                eprintln!("=== block {} (llm) ===", i + 1);
                eprintln!("{}", expanded);
                eprintln!("=== end ===");
            }
        } else {
            eprintln!("=== block {} ({}) ===", i + 1, block.lang);
            eprintln!("{}", expanded);
            eprintln!("=== end ===");
        }
    }
    Ok(())
}

/// Print the expanded code for each block without executing it.
fn dry_run_inner(cmd: &ParsedCommand, raw_args: &[String], cwd: &Path) -> Result<(), CreftError> {
    check_env(cmd)?;
    let (bound, _passthrough) = parse_and_bind(cmd, raw_args)?;
    let bound_refs: Vec<(&str, &str)> = bound
        .iter()
        .map(|(k, v)| (k.as_str(), v.as_str()))
        .collect();

    eprintln!("cwd: {}", cwd.display());

    for (i, block) in cmd.blocks.iter().enumerate() {
        let expanded = substitute(&block.code, &bound_refs, &block.lang)?;
        if block.lang == "llm" {
            let provider = block
                .llm_config
                .as_ref()
                .map(|c| c.provider.as_str())
                .unwrap_or("claude");
            eprintln!("--- block {} (llm: {}) ---", i + 1, provider);
            if let Some(config) = &block.llm_config {
                let command_str = format_llm_command(config);
                eprintln!("command: {}", command_str);
            }
            eprintln!("prompt:");
            println!("{}", expanded);
        } else {
            if cmd.blocks.len() > 1 {
                eprintln!("--- block {} ({}) ---", i + 1, block.lang);
            }
            if !block.deps.is_empty() {
                eprintln!("deps: {}", block.deps.join(", "));
            }
            println!("{}", expanded);
        }
    }

    Ok(())
}

#[cfg(test)]
mod tests {
    use super::*;
    use pretty_assertions::assert_eq;
    use rstest::rstest;

    fn make_context() -> RunContext {
        RunContext::new(
            Arc::new(AtomicBool::new(false)),
            std::path::PathBuf::from("/tmp"),
            vec![
                ("FOO".to_string(), "bar".to_string()),
                ("BAZ".to_string(), "qux".to_string()),
            ],
            false,
            false,
        )
    }

    // Run a command with the given env and cwd using a fresh RunContext.
    fn run_for_test(
        cmd: &ParsedCommand,
        raw_args: &[&str],
        extra_env: &[(&str, &str)],
        cwd: &std::path::Path,
    ) -> Result<(), CreftError> {
        let args: Vec<String> = raw_args.iter().map(|s| s.to_string()).collect();
        let env: Vec<(String, String)> = extra_env
            .iter()
            .map(|(k, v)| (k.to_string(), v.to_string()))
            .collect();
        let ctx = RunContext::new(
            Arc::new(AtomicBool::new(false)),
            cwd.to_path_buf(),
            env,
            false,
            false,
        );
        run(cmd, &args, &ctx)
    }

    #[test]
    fn run_context_new_cwd_accessible() {
        let ctx = make_context();
        assert_eq!(ctx.cwd(), std::path::Path::new("/tmp"));
    }

    #[test]
    fn run_context_is_cancelled_default_false() {
        let ctx = make_context();
        assert!(!ctx.is_cancelled());
    }

    #[test]
    fn run_context_cancel_shared_across_clone() {
        let cancel = Arc::new(AtomicBool::new(false));
        let ctx = RunContext::new(
            Arc::clone(&cancel),
            std::path::PathBuf::from("/tmp"),
            vec![],
            false,
            false,
        );
        let cloned = ctx.clone();

        assert!(!ctx.is_cancelled());
        assert!(!cloned.is_cancelled());

        cancel.store(true, Ordering::Relaxed);
        assert!(ctx.is_cancelled());
        assert!(cloned.is_cancelled());
    }

    #[test]
    fn run_context_env_pairs_returns_str_refs() {
        let ctx = make_context();
        let pairs = ctx.env_pairs();
        assert_eq!(pairs.len(), 2);
        assert_eq!(pairs[0], ("FOO", "bar"));
        assert_eq!(pairs[1], ("BAZ", "qux"));
    }

    #[test]
    fn run_context_clone_shares_cwd_and_env() {
        let ctx = make_context();
        let cloned = ctx.clone();
        assert_eq!(ctx.cwd(), cloned.cwd());
        assert_eq!(ctx.env_pairs(), cloned.env_pairs());
    }

    #[test]
    fn run_context_is_cancelled_true_after_flag_set() {
        let cancel = Arc::new(AtomicBool::new(false));
        let ctx = RunContext::new(
            Arc::clone(&cancel),
            std::path::PathBuf::from("/tmp"),
            vec![],
            false,
            false,
        );
        assert!(!ctx.is_cancelled());
        cancel.store(true, Ordering::Relaxed);
        assert!(ctx.is_cancelled());
    }

    use crate::model::{Arg, CodeBlock, CommandDef, Flag};

    fn make_cmd(args: Vec<Arg>, flags: Vec<Flag>) -> ParsedCommand {
        ParsedCommand {
            def: CommandDef {
                name: "test".into(),
                description: "test cmd".into(),
                args,
                flags,
                env: vec![],
                tags: vec![],
                supports: vec![],
            },
            docs: None,
            blocks: vec![],
        }
    }

    #[test]
    fn test_flag_equals_syntax() {
        let cmd = make_cmd(
            vec![],
            vec![Flag {
                name: "output".into(),
                short: Some("o".into()),
                description: "output format".into(),
                r#type: "string".into(),
                default: None,
                validation: None,
            }],
        );
        let raw = vec!["--output=json".to_string()];
        let (pairs, _) = parse_and_bind(&cmd, &raw).unwrap();
        assert_eq!(pairs.iter().find(|(k, _)| k == "output").unwrap().1, "json");
    }

    #[test]
    fn test_flag_bool_presence() {
        let cmd = make_cmd(
            vec![],
            vec![Flag {
                name: "verbose".into(),
                short: Some("v".into()),
                description: "verbose".into(),
                r#type: "bool".into(),
                default: None,
                validation: None,
            }],
        );
        // --verbose sets to true
        let raw = vec!["--verbose".to_string()];
        let (pairs, _) = parse_and_bind(&cmd, &raw).unwrap();
        assert_eq!(
            pairs.iter().find(|(k, _)| k == "verbose").unwrap().1,
            "true"
        );

        // absent defaults to false
        let raw2: Vec<String> = vec![];
        let (pairs2, _) = parse_and_bind(&cmd, &raw2).unwrap();
        assert_eq!(
            pairs2.iter().find(|(k, _)| k == "verbose").unwrap().1,
            "false"
        );
    }

    #[test]
    fn test_combined_short_flags_bool_only() {
        let cmd = make_cmd(
            vec![],
            vec![
                Flag {
                    name: "all".into(),
                    short: Some("a".into()),
                    description: "all".into(),
                    r#type: "bool".into(),
                    default: None,
                    validation: None,
                },
                Flag {
                    name: "verbose".into(),
                    short: Some("v".into()),
                    description: "verbose".into(),
                    r#type: "bool".into(),
                    default: None,
                    validation: None,
                },
                Flag {
                    name: "force".into(),
                    short: Some("f".into()),
                    description: "force".into(),
                    r#type: "bool".into(),
                    default: None,
                    validation: None,
                },
            ],
        );
        let raw = vec!["-avf".to_string()];
        let (pairs, _) = parse_and_bind(&cmd, &raw).unwrap();
        assert_eq!(pairs.iter().find(|(k, _)| k == "all").unwrap().1, "true");
        assert_eq!(
            pairs.iter().find(|(k, _)| k == "verbose").unwrap().1,
            "true"
        );
        assert_eq!(pairs.iter().find(|(k, _)| k == "force").unwrap().1, "true");
    }

    #[test]
    fn test_combined_short_with_value() {
        let cmd = make_cmd(
            vec![],
            vec![
                Flag {
                    name: "verbose".into(),
                    short: Some("v".into()),
                    description: "verbose".into(),
                    r#type: "bool".into(),
                    default: None,
                    validation: None,
                },
                Flag {
                    name: "format".into(),
                    short: Some("f".into()),
                    description: "output format".into(),
                    r#type: "string".into(),
                    default: None,
                    validation: None,
                },
            ],
        );
        // -vf json: v=true, f consumes next arg
        let raw = vec!["-vf".to_string(), "json".to_string()];
        let (pairs, _) = parse_and_bind(&cmd, &raw).unwrap();
        assert_eq!(
            pairs.iter().find(|(k, _)| k == "verbose").unwrap().1,
            "true"
        );
        assert_eq!(pairs.iter().find(|(k, _)| k == "format").unwrap().1, "json");
    }

    #[test]
    fn test_unknown_flag_errors() {
        let cmd = make_cmd(
            vec![],
            vec![Flag {
                name: "verbose".into(),
                short: Some("v".into()),
                description: "verbose".into(),
                r#type: "bool".into(),
                default: None,
                validation: None,
            }],
        );
        // Unknown flag causes an error, not silent passthrough
        let raw = vec!["--unknown".to_string()];
        assert!(parse_and_bind(&cmd, &raw).is_err());

        // Unknown short flag also errors
        let raw2 = vec!["-x".to_string()];
        assert!(parse_and_bind(&cmd, &raw2).is_err());
    }

    // ---- optional arg tests ----

    #[test]
    fn test_optional_arg_not_bound_when_absent() {
        // required: false, no default, no value provided → arg IS in pairs with value ""
        let cmd = make_cmd(
            vec![Arg {
                name: "count".into(),
                description: "how many".into(),
                default: None,
                required: false,
                validation: None,
            }],
            vec![],
        );
        let raw: Vec<String> = vec![];
        let (pairs, _) = parse_and_bind(&cmd, &raw).unwrap();
        assert_eq!(
            pairs
                .iter()
                .find(|(k, _)| k == "count")
                .map(|(_, v)| v.as_str()),
            Some(""),
            "optional arg with no value should be bound to empty string"
        );
    }

    #[test]
    fn test_optional_arg_uses_provided_value() {
        // required: false, no default, value provided → arg IS in pairs
        let cmd = make_cmd(
            vec![Arg {
                name: "count".into(),
                description: "how many".into(),
                default: None,
                required: false,
                validation: None,
            }],
            vec![],
        );
        let raw = vec!["42".to_string()];
        let (pairs, _) = parse_and_bind(&cmd, &raw).unwrap();
        assert_eq!(pairs.iter().find(|(k, _)| k == "count").unwrap().1, "42");
    }

    #[test]
    fn test_optional_arg_with_frontmatter_default() {
        // required: false, default: "10", no value provided → bound to "10"
        let cmd = make_cmd(
            vec![Arg {
                name: "count".into(),
                description: "how many".into(),
                default: Some("10".into()),
                required: false,
                validation: None,
            }],
            vec![],
        );
        let raw: Vec<String> = vec![];
        let (pairs, _) = parse_and_bind(&cmd, &raw).unwrap();
        assert_eq!(pairs.iter().find(|(k, _)| k == "count").unwrap().1, "10");
    }

    #[test]
    fn test_required_arg_missing_errors() {
        // required: true (default), no value → MissingArg error
        let cmd = make_cmd(
            vec![Arg {
                name: "name".into(),
                description: "a name".into(),
                default: None,
                required: true,
                validation: None,
            }],
            vec![],
        );
        let raw: Vec<String> = vec![];
        assert!(parse_and_bind(&cmd, &raw).is_err());
    }

    #[test]
    fn test_optional_arg_template_default_fires() {
        // When parse_and_bind binds an optional arg to "", the bound "" takes
        // precedence and {{count|5}} resolves to "" (nothing), not "5". The
        // template default only fires when the key is absent from pairs entirely.
        let cmd = make_cmd(
            vec![Arg {
                name: "count".into(),
                description: "how many".into(),
                default: None,
                required: false,
                validation: None,
            }],
            vec![],
        );
        let raw: Vec<String> = vec![];
        let (pairs, _) = parse_and_bind(&cmd, &raw).unwrap();
        let bound_refs: Vec<(&str, &str)> = pairs
            .iter()
            .map(|(k, v)| (k.as_str(), v.as_str()))
            .collect();
        // count is bound to "" by parse_and_bind — template default does not fire
        let result = substitute("echo {{count|5}}", &bound_refs, "bash").unwrap();
        assert_eq!(result, "echo ");
    }

    #[test]
    fn test_optional_arg_no_default_template_errors() {
        // required: false + no default → arg bound to "" by parse_and_bind.
        // {{name}} resolves to nothing (placeholder disappears), not `''`.
        let cmd = make_cmd(
            vec![Arg {
                name: "name".into(),
                description: "a name".into(),
                default: None,
                required: false,
                validation: None,
            }],
            vec![],
        );
        let raw: Vec<String> = vec![];
        let (pairs, _) = parse_and_bind(&cmd, &raw).unwrap();
        let bound_refs: Vec<(&str, &str)> = pairs
            .iter()
            .map(|(k, v)| (k.as_str(), v.as_str()))
            .collect();
        let result = substitute("echo {{name}}", &bound_refs, "bash").unwrap();
        assert_eq!(
            result, "echo ",
            "optional arg with no value should expand to nothing, not ''"
        );
    }

    // ---- Validation: arg regex ----

    #[test]
    fn test_arg_validation_valid_value_passes() {
        let cmd = make_cmd(
            vec![Arg {
                name: "env".into(),
                description: "environment".into(),
                default: None,
                required: true,
                validation: Some(r"^(staging|production)$".into()),
            }],
            vec![],
        );
        let raw = vec!["staging".to_string()];
        let (pairs, _) = parse_and_bind(&cmd, &raw).unwrap();
        assert_eq!(pairs[0].1, "staging");
    }

    #[test]
    fn test_arg_validation_invalid_value_errors() {
        let cmd = make_cmd(
            vec![Arg {
                name: "env".into(),
                description: "environment".into(),
                default: None,
                required: true,
                validation: Some(r"^(staging|production)$".into()),
            }],
            vec![],
        );
        let raw = vec!["dev".to_string()];
        let result = parse_and_bind(&cmd, &raw);
        assert!(
            matches!(
                result,
                Err(crate::error::CreftError::ValidationFailed { .. })
            ),
            "expected ValidationFailed, got: {:?}",
            result
        );
    }

    #[test]
    fn test_arg_validation_invalid_regex_errors() {
        let cmd = make_cmd(
            vec![Arg {
                name: "x".into(),
                description: "x".into(),
                default: None,
                required: true,
                validation: Some(r"[invalid(".into()),
            }],
            vec![],
        );
        let raw = vec!["anything".to_string()];
        let result = parse_and_bind(&cmd, &raw);
        assert!(
            matches!(result, Err(crate::error::CreftError::Frontmatter(_))),
            "invalid regex should produce Frontmatter error, got: {:?}",
            result
        );
    }

    #[test]
    fn test_flag_validation_valid_value_passes() {
        let cmd = make_cmd(
            vec![],
            vec![Flag {
                name: "format".into(),
                short: None,
                description: "output format".into(),
                r#type: "string".into(),
                default: None,
                validation: Some(r"^(json|text)$".into()),
            }],
        );
        let raw = vec!["--format=json".to_string()];
        let (pairs, _) = parse_and_bind(&cmd, &raw).unwrap();
        assert_eq!(pairs[0].1, "json");
    }

    #[test]
    fn test_flag_validation_invalid_value_errors() {
        let cmd = make_cmd(
            vec![],
            vec![Flag {
                name: "format".into(),
                short: None,
                description: "output format".into(),
                r#type: "string".into(),
                default: None,
                validation: Some(r"^(json|text)$".into()),
            }],
        );
        let raw = vec!["--format=xml".to_string()];
        let result = parse_and_bind(&cmd, &raw);
        assert!(
            matches!(
                result,
                Err(crate::error::CreftError::ValidationFailed { .. })
            ),
            "expected ValidationFailed, got: {:?}",
            result
        );
    }

    #[test]
    fn test_flag_validation_invalid_regex_errors() {
        let cmd = make_cmd(
            vec![],
            vec![Flag {
                name: "x".into(),
                short: None,
                description: "x".into(),
                r#type: "string".into(),
                default: None,
                validation: Some(r"[bad(".into()),
            }],
        );
        let raw = vec!["--x=val".to_string()];
        let result = parse_and_bind(&cmd, &raw);
        assert!(
            matches!(result, Err(crate::error::CreftError::Frontmatter(_))),
            "invalid flag regex should produce Frontmatter error, got: {:?}",
            result
        );
    }

    #[test]
    fn test_flag_string_with_default_used_when_absent() {
        // string flag with default — when not provided, default is used
        let cmd = make_cmd(
            vec![],
            vec![Flag {
                name: "format".into(),
                short: None,
                description: "format".into(),
                r#type: "string".into(),
                default: Some("json".into()),
                validation: None,
            }],
        );
        let raw: Vec<String> = vec![];
        let (pairs, _) = parse_and_bind(&cmd, &raw).unwrap();
        assert_eq!(pairs[0].1, "json");
    }

    #[test]
    fn test_flag_string_no_default_absent_skipped() {
        // string flag with no default, not provided → bound to "" in pairs
        let cmd = make_cmd(
            vec![],
            vec![Flag {
                name: "filter".into(),
                short: None,
                description: "filter".into(),
                r#type: "string".into(),
                default: None,
                validation: None,
            }],
        );
        let raw: Vec<String> = vec![];
        let (pairs, _) = parse_and_bind(&cmd, &raw).unwrap();
        assert_eq!(
            pairs
                .iter()
                .find(|(k, _)| k == "filter")
                .map(|(_, v)| v.as_str()),
            Some(""),
            "string flag with no default and not provided should be bound to empty string"
        );
    }

    #[test]
    fn test_optional_flag_no_default_binds_empty() {
        // string flag with required: false (implied by flag type), no default, not provided
        // → bound to "" so {{flagname}} expands to nothing rather than injecting `''`
        let cmd = make_cmd(
            vec![],
            vec![Flag {
                name: "format".into(),
                short: None,
                description: "output format".into(),
                r#type: "string".into(),
                default: None,
                validation: None,
            }],
        );
        let raw: Vec<String> = vec![];
        let (pairs, _) = parse_and_bind(&cmd, &raw).unwrap();
        let bound_refs: Vec<(&str, &str)> = pairs
            .iter()
            .map(|(k, v)| (k.as_str(), v.as_str()))
            .collect();
        let result = substitute("output {{format}}", &bound_refs, "bash").unwrap();
        assert_eq!(
            result, "output ",
            "unset string flag should expand to nothing, not ''"
        );
    }

    // ---- check_env ----

    #[test]
    fn test_check_env_missing_required_var_errors() {
        use crate::model::{CommandDef, EnvVar};
        let cmd = ParsedCommand {
            def: CommandDef {
                name: "test".into(),
                description: "test".into(),
                args: vec![],
                flags: vec![],
                env: vec![EnvVar {
                    name: "CREFT_TEST_MISSING_VAR_XYZ_12345".into(),
                    required: true,
                }],
                tags: vec![],
                supports: vec![],
            },
            docs: None,
            blocks: vec![],
        };
        let result = check_env(&cmd);
        assert!(
            matches!(result, Err(crate::error::CreftError::MissingEnvVar(_))),
            "missing required env var should error, got: {:?}",
            result
        );
    }

    #[test]
    fn test_check_env_optional_missing_ok() {
        use crate::model::{CommandDef, EnvVar};
        let cmd = ParsedCommand {
            def: CommandDef {
                name: "test".into(),
                description: "test".into(),
                args: vec![],
                flags: vec![],
                env: vec![EnvVar {
                    name: "CREFT_TEST_OPTIONAL_VAR_XYZ_12345".into(),
                    required: false,
                }],
                tags: vec![],
                supports: vec![],
            },
            docs: None,
            blocks: vec![],
        };
        // Optional missing var should not error
        let result = check_env(&cmd);
        assert!(result.is_ok());
    }

    // ---- extension mapping ----

    #[test]
    fn test_extension_mapping() {
        assert_eq!(extension("bash"), "sh");
        assert_eq!(extension("sh"), "sh");
        assert_eq!(extension("zsh"), "sh");
        assert_eq!(extension("python"), "py");
        assert_eq!(extension("python3"), "py");
        assert_eq!(extension("node"), "js");
        assert_eq!(extension("javascript"), "js");
        assert_eq!(extension("js"), "js");
        assert_eq!(extension("typescript"), "ts");
        assert_eq!(extension("ts"), "ts");
        assert_eq!(extension("perl"), "pl");
        assert_eq!(extension("unknown"), "unknown");
    }

    // ---- interpreter mapping for ts/perl ----

    #[rstest]
    #[case::sh("sh", "sh")]
    #[case::zsh("zsh", "zsh")]
    #[case::python3("python3", "python3")]
    #[case::javascript("javascript", "node")]
    #[case::js("js", "node")]
    #[case::typescript("typescript", "npx tsx")]
    #[case::ts("ts", "npx tsx")]
    #[case::perl("perl", "perl")]
    fn interpreter_maps_lang_to_executable(#[case] lang: &str, #[case] expected: &str) {
        assert_eq!(interpreter(lang), expected);
    }

    // ---- stdin thread: pipe mode tests ----

    /// Build a two-block pipe command. Block 0 uses `block0_code` and block 1
    /// uses `block1_code`. Both blocks run as bash.
    fn make_pipe_cmd(block0_code: &str, block1_code: &str) -> ParsedCommand {
        ParsedCommand {
            def: CommandDef {
                name: "test-pipe".into(),
                description: "test pipe cmd".into(),
                args: vec![],
                flags: vec![],
                env: vec![],
                tags: vec![],
                supports: vec![],
            },
            docs: None,
            blocks: vec![
                CodeBlock {
                    lang: "bash".into(),
                    code: block0_code.into(),
                    deps: vec![],
                    extension: None,
                    flags: None,
                    llm_config: None,
                    llm_parse_error: None,
                },
                CodeBlock {
                    lang: "bash".into(),
                    code: block1_code.into(),
                    deps: vec![],
                    extension: None,
                    flags: None,
                    llm_config: None,
                    llm_parse_error: None,
                },
            ],
        }
    }

    #[test]
    fn test_pipe_large_stdin_no_deadlock() {
        // Block 0 produces 128KB of output (well above the OS pipe buffer of ~64KB).
        // Block 1 reads all stdin and writes it to stdout.
        // Without the thread fix this test would deadlock; with it, it must complete.
        let block0 = r#"python3 -c "print('x' * 131072, end='')" "#;
        let block1 = "cat";
        let cmd = make_pipe_cmd(block0, block1);
        let cwd = std::path::Path::new("/tmp");
        let result = run_for_test(&cmd, &[], &[], cwd);
        assert!(
            result.is_ok(),
            "large pipe should not deadlock: {:?}",
            result
        );
    }

    #[test]
    fn test_pipe_broken_pipe_silenced() {
        // Block 0 produces substantial output. Block 1 exits immediately (exit 0)
        // without reading any stdin. The write thread gets BrokenPipe; this must
        // NOT surface as StdinWriteFailed — the command should succeed because
        // block 1 exits with code 0.
        let block0 = r#"python3 -c "print('x' * 65536, end='')" "#;
        let block1 = "exit 0";
        let cmd = make_pipe_cmd(block0, block1);
        let cwd = std::path::Path::new("/tmp");
        let result = run_for_test(&cmd, &[], &[], cwd);
        assert!(
            result.is_ok(),
            "BrokenPipe from early-exit child must not surface as error: {:?}",
            result
        );
    }

    #[test]
    fn test_pipe_empty_stdin_payload() {
        // Block 0 produces no output (empty string). Block 1 reads stdin and
        // echoes its line count. Should succeed with 0 lines.
        let block0 = "true"; // exits 0, no output
        let block1 = "wc -l"; // reads stdin, prints line count
        let cmd = make_pipe_cmd(block0, block1);
        let cwd = std::path::Path::new("/tmp");
        let result = run_for_test(&cmd, &[], &[], cwd);
        assert!(
            result.is_ok(),
            "empty stdin pipe should succeed: {:?}",
            result
        );
    }

    #[test]
    fn test_multi_block_default_pipes() {
        // Multi-block skills always pipe. Block 0 outputs "hello" on stdout,
        // which becomes stdin for block 1. Block 1 (cat) passes it through.
        // Both blocks run as part of a pipe chain — result is Ok.
        let cmd = ParsedCommand {
            def: CommandDef {
                name: "test-default-pipe".into(),
                description: "test multi-block pipes by default".into(),
                args: vec![],
                flags: vec![],
                env: vec![],
                tags: vec![],
                supports: vec![],
            },
            docs: None,
            blocks: vec![
                CodeBlock {
                    lang: "bash".into(),
                    code: "echo hello".into(),
                    deps: vec![],
                    extension: None,
                    flags: None,
                    llm_config: None,
                    llm_parse_error: None,
                },
                CodeBlock {
                    lang: "bash".into(),
                    code: "cat".into(),
                    deps: vec![],
                    extension: None,
                    flags: None,
                    llm_config: None,
                    llm_parse_error: None,
                },
            ],
        };
        let cwd = std::path::Path::new("/tmp");
        let result = run_for_test(&cmd, &[], &[], cwd);
        assert!(
            result.is_ok(),
            "multi-block command must pipe by default: {:?}",
            result
        );
    }

    /// Build a pipe command with an arbitrary number of bash blocks.
    fn make_pipe_cmd_multi(blocks: &[&str]) -> ParsedCommand {
        ParsedCommand {
            def: CommandDef {
                name: "test-pipe-multi".into(),
                description: "test multi-block pipe".into(),
                args: vec![],
                flags: vec![],
                env: vec![],
                tags: vec![],
                supports: vec![],
            },
            docs: None,
            blocks: blocks
                .iter()
                .map(|code| CodeBlock {
                    lang: "bash".into(),
                    code: code.to_string(),
                    deps: vec![],
                    extension: None,
                    flags: None,
                    llm_config: None,
                    llm_parse_error: None,
                })
                .collect(),
        }
    }

    #[test]
    fn test_pipe_three_blocks() {
        // Three-block concurrent pipe chain: block 0 echoes data, block 1
        // transforms it (tr a-z A-Z), block 2 reads and appends a marker.
        // This verifies the OS pipe chain connects all three blocks correctly.
        let cmd = make_pipe_cmd_multi(&[
            "echo hello",
            "tr a-z A-Z",
            r#"read line; echo "got: $line""#,
        ]);
        let cwd = std::path::Path::new("/tmp");
        let result = run_for_test(&cmd, &[], &[], cwd);
        assert!(
            result.is_ok(),
            "three-block pipe chain should succeed: {:?}",
            result
        );
    }

    #[test]
    fn test_pipe_single_block_passthrough() {
        // Single-block skills skip the pipe path entirely and run via execute_block.
        let cmd = ParsedCommand {
            def: CommandDef {
                name: "test-pipe-single".into(),
                description: "single block skill".into(),
                args: vec![],
                flags: vec![],
                env: vec![],
                tags: vec![],
                supports: vec![],
            },
            docs: None,
            blocks: vec![CodeBlock {
                lang: "bash".into(),
                code: "echo single".into(),
                deps: vec![],
                extension: None,
                flags: None,
                llm_config: None,
                llm_parse_error: None,
            }],
        };
        let cwd = std::path::Path::new("/tmp");
        let result = run_for_test(&cmd, &[], &[], cwd);
        assert!(
            result.is_ok(),
            "single-block skill must succeed: {:?}",
            result
        );
    }

    #[test]
    fn test_pipe_block0_fails() {
        // Block 0 exits non-zero. Block 1 reads EOF on stdin and succeeds
        // (wc -l prints 0). The upstream failure must be reported regardless
        // of whether the last block succeeded — creft reports the root cause,
        // not the last block's exit code.
        let cmd = make_pipe_cmd_multi(&["exit 1", "wc -l"]);
        let cwd = std::path::Path::new("/tmp");
        let result = run_for_test(&cmd, &[], &[], cwd);
        assert!(
            matches!(
                result,
                Err(crate::error::CreftError::ExecutionFailed { block: 0, .. })
            ),
            "block 0 failure must be reported even when the last block succeeds: {:?}",
            result
        );
    }

    #[test]
    fn test_pipe_last_block_fails_reports_error() {
        // Block 0 succeeds. Block 1 exits non-zero. The last block's exit status
        // determines the result — should return ExecutionFailed.
        let cmd = make_pipe_cmd_multi(&["echo hello", "exit 1"]);
        let cwd = std::path::Path::new("/tmp");
        let result = run_for_test(&cmd, &[], &[], cwd);
        assert!(
            matches!(
                result,
                Err(crate::error::CreftError::ExecutionFailed { block: 1, .. })
            ),
            "when last block fails, error should reference block 1: {:?}",
            result
        );
    }

    // ---- actionable OS error messages ----

    #[test]
    #[cfg(unix)]
    fn test_signal_detection() {
        // A block that kills itself with SIGTERM should produce ExecutionSignaled,
        // not ExecutionFailed. We use `kill -TERM $$` in bash to self-signal.
        let cmd = ParsedCommand {
            def: CommandDef {
                name: "test-signal".into(),
                description: "signal test".into(),
                args: vec![],
                flags: vec![],
                env: vec![],
                tags: vec![],
                supports: vec![],
            },
            docs: None,
            blocks: vec![CodeBlock {
                lang: "bash".into(),
                // Self-terminate with SIGTERM. bash propagates the signal exit.
                code: "kill -TERM $$".into(),
                deps: vec![],
                extension: None,
                flags: None,
                llm_config: None,
                llm_parse_error: None,
            }],
        };
        let cwd = std::path::Path::new("/tmp");
        let result = run_for_test(&cmd, &[], &[], cwd);
        assert!(
            matches!(
                result,
                Err(crate::error::CreftError::ExecutionSignaled { block: 0, .. })
            ),
            "self-SIGTERM should produce ExecutionSignaled, got: {:?}",
            result
        );
    }

    #[test]
    fn test_e2big_enrichment() {
        // Directly test enrich_io_error with a synthetic E2BIG error (raw_os_error 7).
        // We can't easily trigger a real E2BIG in a unit test, so we construct the
        // io::Error directly using from_raw_os_error.
        let e2big = std::io::Error::from_raw_os_error(7);
        let err = crate::error::enrich_io_error(e2big, "environment");
        assert!(
            matches!(err, crate::error::CreftError::Setup(_)),
            "E2BIG should produce a Setup error with guidance, got: {:?}",
            err
        );
        let msg = err.to_string();
        assert!(
            msg.contains("OS argument size limit"),
            "E2BIG message should mention OS argument size limit, got: {msg}"
        );
        assert!(
            msg.contains("environment"),
            "E2BIG message should include context 'environment', got: {msg}"
        );
    }

    #[test]
    fn test_enrich_io_error_other_errors_passthrough() {
        // Non-E2BIG, non-NotFound errors should pass through as CreftError::Io.
        let permission_denied = std::io::Error::from_raw_os_error(13 /* EACCES */);
        let err = crate::error::enrich_io_error(permission_denied, "ctx");
        assert!(
            matches!(err, crate::error::CreftError::Io(_)),
            "EACCES should pass through as Io, got: {:?}",
            err
        );
    }

    #[test]
    fn test_pipe_chain_multiple_failures() {
        // Both block 0 and block 1 fail. Block 0 exits non-zero, block 1 also
        // exits non-zero. The error should reference block 0 (the root cause —
        // earliest non-signal failure).
        //
        // Note: wc -l counts lines from stdin. When block 0 exits 1, its pipe
        // closes, block 1 gets EOF and also exits 1 via explicit `exit 1`.
        let cmd = make_pipe_cmd_multi(&["exit 1", "exit 1"]);
        let cwd = std::path::Path::new("/tmp");
        let result = run_for_test(&cmd, &[], &[], cwd);
        // Block 0 fails with exit code (not a signal), so it's the root cause.
        assert!(
            matches!(
                result,
                Err(crate::error::CreftError::ExecutionFailed { block: 0, .. })
            ),
            "when both blocks fail, block 0 should be reported as root cause: {:?}",
            result
        );
    }

    #[test]
    fn test_execution_signaled_exit_code() {
        // ExecutionSignaled.exit_code() should return 128 + signal (Unix convention).
        let err = crate::error::CreftError::ExecutionSignaled {
            block: 0,
            lang: "bash".into(),
            signal: 15, // SIGTERM
        };
        assert_eq!(err.exit_code(), 143, "128 + 15 (SIGTERM) = 143");
    }

    #[test]
    fn run_context_request_cancel_sets_flag() {
        let ctx = RunContext::new(
            Arc::new(AtomicBool::new(false)),
            std::path::PathBuf::from("/tmp"),
            vec![],
            false,
            false,
        );
        assert_eq!(ctx.is_cancelled(), false);
        ctx.request_cancel();
        assert_eq!(ctx.is_cancelled(), true);
    }

    #[test]
    fn run_context_request_cancel_visible_to_clones() {
        let cancel = Arc::new(AtomicBool::new(false));
        let ctx = RunContext::new(
            Arc::clone(&cancel),
            std::path::PathBuf::from("/tmp"),
            vec![],
            false,
            false,
        );
        let cloned = ctx.clone();
        ctx.request_cancel();
        assert_eq!(cloned.is_cancelled(), true);
    }

    #[cfg(unix)]
    #[test]
    fn make_execution_error_signal_returns_execution_signaled() {
        use std::os::unix::process::ExitStatusExt;
        // Raw value N means the process was killed by signal N.
        let status = std::process::ExitStatus::from_raw(9);
        let err = make_execution_error(2, "bash", &status);
        assert!(
            matches!(
                err,
                crate::error::CreftError::ExecutionSignaled {
                    block: 2,
                    signal: 9,
                    ..
                }
            ),
            "signal kill should produce ExecutionSignaled, got: {:?}",
            err
        );
    }

    #[cfg(unix)]
    #[test]
    fn make_execution_error_exit_code_returns_execution_failed() {
        use std::os::unix::process::ExitStatusExt;
        // Raw value N << 8 means the process exited with code N.
        let status = std::process::ExitStatus::from_raw(42 << 8);
        let err = make_execution_error(1, "python", &status);
        assert!(
            matches!(
                err,
                crate::error::CreftError::ExecutionFailed {
                    block: 1,
                    code: 42,
                    ..
                }
            ),
            "non-zero exit code should produce ExecutionFailed, got: {:?}",
            err
        );
    }

    // ---- bound_pairs_to_env ----

    #[test]
    fn bound_pairs_to_env_empty_input_produces_empty_output() {
        let result = bound_pairs_to_env(&[]);
        assert_eq!(result, Vec::<(String, String)>::new());
    }

    #[rstest]
    #[case::lowercase("format", "json", "CREFT_ARG_FORMAT", "json")]
    #[case::hyphenated("always-confirm", "true", "CREFT_ARG_ALWAYS_CONFIRM", "true")]
    #[case::multi_hyphen("output-file-path", "out.txt", "CREFT_ARG_OUTPUT_FILE_PATH", "out.txt")]
    #[case::already_upper("TARGET", "prod", "CREFT_ARG_TARGET", "prod")]
    #[case::mixed_case("myFlag", "val", "CREFT_ARG_MYFLAG", "val")]
    #[case::underscore_unchanged("my_flag", "1", "CREFT_ARG_MY_FLAG", "1")]
    #[case::collision_with_path("path", "/x", "CREFT_ARG_PATH", "/x")]
    fn bound_pairs_to_env_name_normalization(
        #[case] input_name: &str,
        #[case] input_val: &str,
        #[case] expected_name: &str,
        #[case] expected_val: &str,
    ) {
        let pairs = vec![(input_name.to_string(), input_val.to_string())];
        let result = bound_pairs_to_env(&pairs);
        assert_eq!(result.len(), 1);
        assert_eq!(
            result[0],
            (expected_name.to_string(), expected_val.to_string())
        );
    }

    #[test]
    fn bound_pairs_to_env_preserves_order_and_count() {
        let pairs = vec![
            ("alpha".to_string(), "1".to_string()),
            ("beta-flag".to_string(), "2".to_string()),
            ("gamma".to_string(), "3".to_string()),
        ];
        let result = bound_pairs_to_env(&pairs);
        assert_eq!(result.len(), 3);
        assert_eq!(result[0], ("CREFT_ARG_ALPHA".to_string(), "1".to_string()));
        assert_eq!(
            result[1],
            ("CREFT_ARG_BETA_FLAG".to_string(), "2".to_string())
        );
        assert_eq!(result[2], ("CREFT_ARG_GAMMA".to_string(), "3".to_string()));
    }

    #[test]
    fn bound_pairs_to_env_empty_value_preserved() {
        let pairs = vec![("format".to_string(), String::new())];
        let result = bound_pairs_to_env(&pairs);
        assert_eq!(
            result,
            vec![("CREFT_ARG_FORMAT".to_string(), String::new())]
        );
    }

    #[test]
    fn bound_pairs_to_env_path_arg_does_not_produce_bare_path_key() {
        let result = bound_pairs_to_env(&[("path".to_string(), "/tmp".to_string())]);
        assert_eq!(result.len(), 1);
        assert_eq!(result[0].0, "CREFT_ARG_PATH");
        assert_eq!(result[0].1, "/tmp");
        assert!(
            !result.iter().any(|(k, _)| k == "PATH"),
            "bare PATH key must not appear in env pairs"
        );
    }

    // ---- prepare_block_script ----

    fn bash_block() -> CodeBlock {
        CodeBlock {
            lang: "bash".into(),
            code: String::new(),
            deps: vec![],
            extension: None,
            flags: None,
            llm_config: None,
            llm_parse_error: None,
        }
    }

    /// With no preamble, the file contains exactly the expanded code.
    #[test]
    fn prepare_block_script_none_preamble_writes_code_only() {
        let block = bash_block();
        let code = "echo hello\n";
        let tmp = prepare_block_script(&block, code, None).unwrap();
        let contents = std::fs::read_to_string(tmp.path()).unwrap();
        assert_eq!(contents, code);
    }

    /// With a preamble, the preamble is written before the code.
    #[test]
    fn prepare_block_script_some_preamble_prepends_preamble() {
        let block = bash_block();
        let preamble = "# preamble\n";
        let code = "echo hello\n";
        let tmp = prepare_block_script(&block, code, Some(preamble)).unwrap();
        let contents = std::fs::read_to_string(tmp.path()).unwrap();
        assert_eq!(contents, "# preamble\necho hello\n");
    }

    /// When the code starts with a shebang, the shebang is placed first,
    /// then the preamble, then the remainder of the code.
    #[test]
    fn prepare_block_script_shebang_placed_before_preamble() {
        let block = bash_block();
        let preamble = "# preamble\n";
        let code = "#!/bin/bash\necho hello\n";
        let tmp = prepare_block_script(&block, code, Some(preamble)).unwrap();
        let contents = std::fs::read_to_string(tmp.path()).unwrap();
        assert_eq!(contents, "#!/bin/bash\n# preamble\necho hello\n");
    }

    /// A shebang-only script (no trailing newline after shebang) is handled
    /// without panicking — remainder is empty.
    #[test]
    fn prepare_block_script_shebang_only_no_panic() {
        let block = bash_block();
        let preamble = "# preamble\n";
        let code = "#!/bin/bash";
        let tmp = prepare_block_script(&block, code, Some(preamble)).unwrap();
        let contents = std::fs::read_to_string(tmp.path()).unwrap();
        assert_eq!(contents, "#!/bin/bash\n# preamble\n");
    }

    /// Without a preamble, a shebang-prefixed code is written unchanged.
    #[test]
    fn prepare_block_script_shebang_without_preamble_unchanged() {
        let block = bash_block();
        let code = "#!/bin/bash\necho hi\n";
        let tmp = prepare_block_script(&block, code, None).unwrap();
        let contents = std::fs::read_to_string(tmp.path()).unwrap();
        assert_eq!(contents, code);
    }

    /// The file extension matches the block's language.
    #[test]
    fn prepare_block_script_file_has_correct_extension() {
        let block = CodeBlock {
            lang: "python".into(),
            code: String::new(),
            deps: vec![],
            extension: None,
            flags: None,
            llm_config: None,
            llm_parse_error: None,
        };
        let tmp = prepare_block_script(&block, "print('hi')\n", None).unwrap();
        assert!(
            tmp.path().extension().map(|e| e == "py").unwrap_or(false),
            "python blocks must produce .py temp files"
        );
    }

    /// A block with `# extension: .mjs` produces a temp file ending in exactly
    /// `.mjs`, not `..mjs`. The block fixture carries `extension: Some("mjs")`
    /// — the post-parse value after `parse_block_directives` strips the leading
    /// dot — because the runner reads `block.extension` directly and trusts the
    /// parser's invariant.
    #[test]
    fn prepare_block_script_leading_dot_extension_produces_single_dot_suffix() {
        let block = CodeBlock {
            lang: "node".into(),
            code: String::new(),
            deps: vec![],
            extension: Some("mjs".into()),
            flags: None,
            llm_config: None,
            llm_parse_error: None,
        };
        let tmp = prepare_block_script(&block, "console.log('hi')\n", None).unwrap();
        let path = tmp.path().to_string_lossy();
        assert!(
            path.ends_with(".mjs"),
            "extension Some(\"mjs\") must produce a path ending in .mjs, not ..mjs; got: {path}"
        );
        assert!(
            !path.ends_with("..mjs"),
            "path must not contain a double-dot suffix; got: {path}"
        );
    }

    // ── creft_search in single-block mode ────────────────────────────────────

    /// A single-block bash skill that calls creft_index then creft_search must
    /// receive the search response on fd 4 and complete without hanging.
    ///
    /// Before the fix, spawn_reader received None for its response_writer, so
    /// Search messages were silently dropped and the bash block blocked forever
    /// on `read <&4`. This test is the regression guard for that bug.
    #[test]
    #[cfg(unix)]
    fn single_block_creft_search_receives_response_without_hanging() {
        let cmd = ParsedCommand {
            def: CommandDef {
                name: "test-search".into(),
                description: "single block with creft_index then creft_search".into(),
                args: vec![],
                flags: vec![],
                env: vec![],
                tags: vec![],
                supports: vec![],
            },
            docs: None,
            blocks: vec![CodeBlock {
                lang: "bash".into(),
                // Index some content then search for it. If the response writer
                // is missing, creft_search hangs forever on read <&4.
                code: r#"creft_index "docs" "rollback deployment procedure"
creft_search "rollback" "docs" > /dev/null
echo "search completed"
"#
                .into(),
                deps: vec![],
                extension: None,
                flags: None,
                llm_config: None,
                llm_parse_error: None,
            }],
        };

        let ctx = RunContext::new(
            Arc::new(AtomicBool::new(false)),
            std::path::PathBuf::from("/tmp"),
            vec![],
            false,
            false,
        )
        .with_skill_name("test search");

        let args: Vec<String> = vec![];
        let result = run(&cmd, &args, &ctx);
        assert!(
            result.is_ok(),
            "single-block creft_search must complete without hanging: {:?}",
            result
        );
    }

    // ── creft_search / creft_store in pipe-chain blocks ──────────────────────

    /// A block in a multi-block pipe chain can call creft_index and creft_search
    /// and receive actual results.
    ///
    /// Before the fix, spawn_reader was passed `None` for the PrimitiveContext
    /// in pipe-chain mode, so Index messages were silently dropped and Search
    /// queries returned empty results regardless of what was indexed. This test
    /// verifies that the PrimitiveContext is wired correctly: index then search
    /// within the same pipe-chain block returns the indexed content.
    #[test]
    #[cfg(unix)]
    fn pipe_chain_block_creft_index_and_search_receive_results() {
        let cmd = ParsedCommand {
            def: CommandDef {
                name: "test-pipe-search".into(),
                description: "index and search within a pipe-chain block".into(),
                args: vec![],
                flags: vec![],
                env: vec![],
                tags: vec![],
                supports: vec![],
            },
            docs: None,
            blocks: vec![
                CodeBlock {
                    lang: "bash".into(),
                    // Block 0 (non-sponge): index content, search for it, and
                    // write the result to stdout. Block 1 captures that output
                    // and fails if the result is empty.
                    code: r#"creft_index "notes" "rollback deployment procedure"
result=$(creft_search "rollback" "notes")
echo "$result"
"#
                    .into(),
                    deps: vec![],
                    extension: None,
                    flags: None,
                    llm_config: None,
                    llm_parse_error: None,
                },
                CodeBlock {
                    lang: "bash".into(),
                    // Block 1: read block 0's search result from stdin.
                    // If PrimitiveContext was missing, creft_search in block 0
                    // would return an empty string, and this test would fail.
                    code: r#"stdin=$(cat)
if [ -z "$stdin" ]; then
  echo "ERROR: search returned empty result" >&2
  exit 1
fi
exit 0
"#
                    .into(),
                    deps: vec![],
                    extension: None,
                    flags: None,
                    llm_config: None,
                    llm_parse_error: None,
                },
            ],
        };

        let ctx = RunContext::new(
            Arc::new(AtomicBool::new(false)),
            std::path::PathBuf::from("/tmp"),
            vec![],
            false,
            false,
        )
        .with_skill_name("test pipe search");

        let args: Vec<String> = vec![];
        let result = run(&cmd, &args, &ctx);
        assert!(
            result.is_ok(),
            "creft_index + creft_search in a pipe-chain block must return results: {:?}",
            result
        );
    }

    /// A block in a multi-block pipe chain can call creft_store_get and receive
    /// an empty-value response without hanging.
    ///
    /// With PrimitiveContext wired to the reader thread, StoreGet is handled by
    /// the real handler which returns `{"value":""}` when the key is absent.
    /// Without it, the reader falls through to the "no ctx" branch, which also
    /// returns an empty response — but this test documents that the handler is
    /// reached and the block exits cleanly in both cases.
    #[test]
    #[cfg(unix)]
    fn pipe_chain_block_creft_store_get_returns_empty_for_missing_key() {
        let cmd = ParsedCommand {
            def: CommandDef {
                name: "test-pipe-store".into(),
                description: "store_get for absent key in pipe-chain block".into(),
                args: vec![],
                flags: vec![],
                env: vec![],
                tags: vec![],
                supports: vec![],
            },
            docs: None,
            blocks: vec![
                CodeBlock {
                    lang: "bash".into(),
                    code: "echo ready\n".into(),
                    deps: vec![],
                    extension: None,
                    flags: None,
                    llm_config: None,
                    llm_parse_error: None,
                },
                CodeBlock {
                    lang: "bash".into(),
                    // Discard block 0 output, call store_get, verify it exits
                    // cleanly (no hang, no non-zero exit from creft_store_get).
                    code: r#"cat > /dev/null
creft_store_get "mystore" "no_such_key" > /dev/null
echo "get completed"
"#
                    .into(),
                    deps: vec![],
                    extension: None,
                    flags: None,
                    llm_config: None,
                    llm_parse_error: None,
                },
            ],
        };

        let ctx = RunContext::new(
            Arc::new(AtomicBool::new(false)),
            std::path::PathBuf::from("/tmp"),
            vec![],
            false,
            false,
        )
        .with_skill_name("test pipe store");

        let args: Vec<String> = vec![];
        let result = run(&cmd, &args, &ctx);
        assert!(
            result.is_ok(),
            "creft_store_get for a missing key in a pipe-chain block must exit cleanly: {:?}",
            result
        );
    }

    // ── Stage 1: coverage trace emission ──────────────────────────────────────

    /// Parse a small skill markdown string into a `ParsedCommand` using the
    /// same two-call path that production code uses.
    ///
    /// Tests that verify trace emission MUST use this helper rather than
    /// constructing `CodeBlock` literals directly. The preamble (`creft_print`,
    /// `creft_status`, `creft_exit`, etc.) is injected by `preamble::for_language`
    /// inside `execute_block`, which is reached only when the command has real
    /// `CodeBlock` values from parsed markdown.
    fn parse_skill(raw: &str) -> ParsedCommand {
        let (def, body) = crate::frontmatter::parse(raw).expect("frontmatter parse failed");
        let (docs, blocks) = crate::markdown::extract_blocks(&body);
        ParsedCommand { def, docs, blocks }
    }

    /// Build a `RunContext` with an attached `TraceWriter` backed by a pipe,
    /// returning `(ctx, read_end)`. After running a skill, close the write end
    /// (by dropping `ctx`) and read from `read_end` to collect NDJSON records.
    #[cfg(unix)]
    fn make_traced_ctx(cwd: &std::path::Path) -> (RunContext, std::os::unix::io::OwnedFd) {
        use std::os::unix::io::IntoRawFd;
        let (read_fd, write_fd) = os_pipe().expect("os_pipe failed");
        // SAFETY: write_fd is valid and exclusively owned by this call; ownership
        // transfers to the TraceWriter via File::from_raw_fd inside trace_writer_from_fd.
        let writer = unsafe { RunContext::trace_writer_from_fd(write_fd.into_raw_fd()) };
        let ctx = RunContext::new(
            Arc::new(AtomicBool::new(false)),
            cwd.to_path_buf(),
            vec![],
            false,
            false,
        )
        .with_trace_writer(Some(writer));
        (ctx, read_fd)
    }

    /// Drain a read fd to EOF, parse each line as a `serde_json::Value`, and
    /// return the records in the order they were written.
    #[cfg(unix)]
    fn drain_trace(read_fd: std::os::unix::io::OwnedFd) -> Vec<serde_json::Value> {
        use std::io::{BufRead, BufReader};
        use std::os::unix::io::{FromRawFd, IntoRawFd};
        // SAFETY: read_fd is valid and owned by this call; ownership transfers to
        // the File which closes it on drop.
        let file = unsafe { std::fs::File::from_raw_fd(read_fd.into_raw_fd()) };
        let reader = BufReader::new(file);
        reader
            .lines()
            .map_while(Result::ok)
            .filter(|l| !l.is_empty())
            .map(|l| serde_json::from_str(&l).expect("trace line must be valid JSON"))
            .collect()
    }

    /// A single-block bash skill calling `creft_print` twice emits one trace
    /// record with `"print": 2` in the primitives map.
    #[cfg(unix)]
    #[test]
    fn trace_single_block_print_twice_counts_two() {
        let raw = "---\nname: trace-print-test\ndescription: trace test\n---\n\n```bash\ncreft_print \"hello\"\ncreft_print \"world\"\n```\n";
        let cmd = parse_skill(raw);
        let cwd = tempfile::tempdir().unwrap();
        let (ctx, read_fd) = make_traced_ctx(cwd.path());

        run(&cmd, &[], &ctx).expect("skill must succeed");
        drop(ctx); // closes the write end of the trace pipe

        let records = drain_trace(read_fd);
        assert_eq!(records.len(), 1, "exactly one record for one block");
        let rec = &records[0];
        assert_eq!(rec["block"], 0);
        assert_eq!(rec["lang"], "bash");
        assert_eq!(rec["exit"], 0);
        assert_eq!(
            rec["primitives"]["print"], 2,
            "two creft_print calls must yield count 2"
        );
    }

    /// A block that calls `creft_exit 7` emits a record with `"exit": 7`,
    /// not the wait-status exit code (which is 0 when creft_exit fires).
    #[cfg(unix)]
    #[test]
    fn trace_creft_exit_records_side_channel_code() {
        let raw = "---\nname: trace-exit-test\ndescription: trace test\n---\n\n```bash\ncreft_exit 7\n```\n";
        let cmd = parse_skill(raw);
        let cwd = tempfile::tempdir().unwrap();
        let (ctx, read_fd) = make_traced_ctx(cwd.path());

        // creft_exit non-zero surfaces as ExecutionFailed; the trace still arrives.
        let _ = run(&cmd, &[], &ctx);
        drop(ctx);

        let records = drain_trace(read_fd);
        assert_eq!(records.len(), 1);
        assert_eq!(
            records[0]["exit"], 7,
            "trace exit must be the creft_exit code, not the wait-status exit"
        );
    }

    /// With `trace_writer = None`, no bytes are written to a separate sentinel pipe.
    #[cfg(unix)]
    #[test]
    fn trace_no_writer_produces_no_output() {
        let raw = "---\nname: trace-noop-test\ndescription: trace test\n---\n\n```bash\ncreft_print \"hi\"\n```\n";
        let cmd = parse_skill(raw);
        // Sentinel pipe: if anything is written here, the test fails.
        let (read_fd, write_fd) = os_pipe().expect("os_pipe failed");
        let cwd = tempfile::tempdir().unwrap();
        let ctx = RunContext::new(
            Arc::new(AtomicBool::new(false)),
            cwd.path().to_path_buf(),
            vec![],
            false,
            false,
        );
        // No trace writer attached.
        run(&cmd, &[], &ctx).expect("skill must succeed");
        // Close the write end so the read below sees EOF immediately.
        drop(write_fd);
        let records = drain_trace(read_fd);
        assert!(
            records.is_empty(),
            "no trace records expected when trace_writer is None"
        );
    }

    /// A three-block bash | python | node pipe where each block calls a different
    /// primitive produces three records in execution order with the right per-block
    /// counts. This proves the trace contract is language-agnostic: the runner counts
    /// what it sees on fd 3 regardless of which language wrote it.
    ///
    /// The test is skipped when `python3` or `node` is absent from PATH.
    #[cfg(unix)]
    #[test]
    fn trace_pipe_chain_bash_python_node_each_primitive() {
        fn tool_on_path(name: &str) -> bool {
            std::process::Command::new("which")
                .arg(name)
                .stdout(std::process::Stdio::null())
                .stderr(std::process::Stdio::null())
                .status()
                .map(|s| s.success())
                .unwrap_or(false)
        }

        if !tool_on_path("python3") {
            eprintln!("skipping trace_pipe_chain_bash_python_node: python3 not on PATH");
            return;
        }
        if !tool_on_path("node") {
            eprintln!("skipping trace_pipe_chain_bash_python_node: node not on PATH");
            return;
        }

        // bash block 0: calls creft_print once, then echoes a line for block 1 to consume.
        // python block 1: calls creft_status once, then passes stdin through to block 2.
        // node block 2: calls creft_print once, then drains stdin.
        let raw = concat!(
            "---\nname: trace-lang-pipe\ndescription: multi-lang pipe trace test\n---\n\n",
            "```bash\n",
            "creft_print \"hello from bash\"\n",
            "echo \"forward\"\n",
            "```\n\n",
            "```python\n",
            "import sys\n",
            "creft_status(\"step\")\n",
            "for line in sys.stdin:\n",
            "    print(line, end='')\n",
            "```\n\n",
            "```node\n",
            "creft_print('done');\n",
            "process.stdin.resume();\n",
            "```\n",
        );
        let cmd = parse_skill(raw);
        let cwd = tempfile::tempdir().unwrap();
        let (ctx, read_fd) = make_traced_ctx(cwd.path());

        run(&cmd, &[], &ctx).expect("three-language pipe must succeed");
        drop(ctx);

        let records = drain_trace(read_fd);
        assert_eq!(records.len(), 3, "one record per block");
        assert_eq!(records[0]["block"], 0, "block 0 first");
        assert_eq!(records[1]["block"], 1, "block 1 second");
        assert_eq!(records[2]["block"], 2, "block 2 third");
        assert_eq!(records[0]["lang"], "bash");
        assert_eq!(records[1]["lang"], "python");
        assert_eq!(records[2]["lang"], "node");
        assert_eq!(
            records[0]["primitives"]["print"], 1,
            "bash block called creft_print once"
        );
        assert_eq!(
            records[1]["primitives"]["status"], 1,
            "python block called creft_status once"
        );
        assert_eq!(
            records[2]["primitives"]["print"], 1,
            "node block called creft_print once"
        );
    }

    /// A block that calls `creft_exit 0` emits `"exit": 0` in the trace record.
    /// This is distinct from the `creft_exit 7` test: the code == 0 branch in
    /// `execute_block` exits through a different arm, so a regression that forgot
    /// to emit trace before returning `EarlyExit` for code 0 would not be caught
    /// by the non-zero test alone.
    #[cfg(unix)]
    #[test]
    fn trace_creft_exit_zero_emits_zero_exit_code() {
        let raw = "---\nname: trace-exit-zero\ndescription: trace test\n---\n\n```bash\ncreft_exit 0\n```\n";
        let cmd = parse_skill(raw);
        let cwd = tempfile::tempdir().unwrap();
        let (ctx, read_fd) = make_traced_ctx(cwd.path());

        // creft_exit 0 is treated as success; the trace must still arrive.
        let _ = run(&cmd, &[], &ctx);
        drop(ctx);

        let records = drain_trace(read_fd);
        assert_eq!(records.len(), 1, "one record expected");
        assert_eq!(
            records[0]["exit"], 0,
            "trace exit must be 0 when creft_exit 0 fires (side-channel truth, not wait status)"
        );
    }

    /// A block that self-signals with SIGINT emits `"exit": 130` in the trace record.
    /// This pins the signal-exit convention (`128 + signal`) end-to-end through the
    /// `trace_exit_code` helper, not just through unit-level reasoning about the function.
    #[cfg(unix)]
    #[test]
    fn trace_block_sigint_emits_exit_130() {
        let raw =
            "---\nname: trace-sigint\ndescription: trace test\n---\n\n```bash\nkill -INT $$\n```\n";
        let cmd = parse_skill(raw);
        let cwd = tempfile::tempdir().unwrap();
        let (ctx, read_fd) = make_traced_ctx(cwd.path());

        // SIGINT-killed block returns an error; the trace must still arrive.
        let _ = run(&cmd, &[], &ctx);
        drop(ctx);

        let records = drain_trace(read_fd);
        assert_eq!(
            records.len(),
            1,
            "one record expected for signal-killed block"
        );
        assert_eq!(
            records[0]["exit"], 130,
            "SIGINT must produce exit 130 (128 + 2) in the trace record"
        );
    }

    /// A block that exits with a non-zero wait-status code emits the correct code
    /// in the trace record through the `output.status.success() == false` arm of
    /// `execute_block`. This arm is distinct from the `creft_exit` side-channel path.
    #[cfg(unix)]
    #[test]
    fn trace_nonzero_exit_status_propagates_to_record() {
        let raw =
            "---\nname: trace-exit-fail\ndescription: trace test\n---\n\n```bash\nexit 42\n```\n";
        let cmd = parse_skill(raw);
        let cwd = tempfile::tempdir().unwrap();
        let (ctx, read_fd) = make_traced_ctx(cwd.path());

        // Non-zero exit surfaces as ExecutionFailed; the trace must still arrive.
        let _ = run(&cmd, &[], &ctx);
        drop(ctx);

        let records = drain_trace(read_fd);
        assert_eq!(records.len(), 1, "one record expected for failed block");
        assert_eq!(
            records[0]["exit"], 42,
            "wait-status exit code 42 must propagate to the trace record"
        );
    }

    /// A block that exits with code 99 emits a trace record carrying the raw
    /// wait-status (code 99 is a normal failure, not a special sentinel).
    #[cfg(unix)]
    #[test]
    fn trace_code_99_emits_failure_record() {
        let raw =
            "---\nname: trace-early-exit\ndescription: trace test\n---\n\n```bash\nexit 99\n```\n";
        let cmd = parse_skill(raw);
        let cwd = tempfile::tempdir().unwrap();
        let (ctx, read_fd) = make_traced_ctx(cwd.path());

        let _ = run(&cmd, &[], &ctx);
        drop(ctx);

        let records = drain_trace(read_fd);
        assert_eq!(
            records.len(),
            1,
            "one trace record expected for a failed block"
        );
        // code 99 is a normal failure; the trace carries the raw wait-status.
        assert_eq!(
            records[0]["exit"], 99,
            "code 99 must propagate as exit code 99 in the trace record"
        );
    }

    /// A child process spawned with `CREFT_TRACE_FD` set attaches the trace writer
    /// via `cmd::run::run_user_command` and emits records exactly as a
    /// directly-constructed `RunContext` would. This pins the env→writer plumbing
    /// end-to-end so Stage 4 can spawn `creft <skill>` as a subprocess and rely on
    /// the `CREFT_TRACE_FD` contract without additional wiring.
    #[cfg(unix)]
    #[test]
    fn trace_env_fd_plumbing_subprocess_emits_same_shape() {
        use std::os::unix::io::AsRawFd;
        use std::os::unix::process::CommandExt as _;

        // Write the skill file directly into a temp CREFT_HOME directory.
        // CREFT_HOME mode resolves skills at <home>/commands/<name>.md.
        let home = tempfile::tempdir().unwrap();
        let commands_dir = home.path().join("commands");
        std::fs::create_dir_all(&commands_dir).unwrap();
        let skill_raw = concat!(
            "---\nname: trace-fd-skill\ndescription: end-to-end trace fd test\n---\n\n",
            "```bash\n",
            "creft_print \"hello\"\n",
            "creft_print \"world\"\n",
            "```\n",
        );
        std::fs::write(commands_dir.join("trace-fd-skill.md"), skill_raw).unwrap();

        // Create the trace pipe. The write end is dup'd into the child at a
        // fixed target fd (5) so it survives exec despite O_CLOEXEC on the pipe.
        let (read_fd, write_fd) = os_pipe().expect("os_pipe failed");
        let write_raw = write_fd.as_raw_fd();
        const TRACE_TARGET_FD: libc::c_int = 5;

        let creft_bin = assert_cmd::cargo::cargo_bin("creft");
        let mut cmd = std::process::Command::new(&creft_bin);
        cmd.env("CREFT_HOME", home.path())
            .env_remove("CREFT_PROJECT_ROOT")
            .env("CREFT_TRACE_FD", TRACE_TARGET_FD.to_string())
            .arg("trace-fd-skill");

        // SAFETY: dup2 and close are async-signal-safe. write_raw is a valid fd
        // owned by this call until exec; after dup2 the duplicate at TRACE_TARGET_FD
        // has O_CLOEXEC cleared (dup2 always clears it), so the child inherits it.
        unsafe {
            cmd.pre_exec(move || {
                if libc::dup2(write_raw, TRACE_TARGET_FD) == -1 {
                    return Err(std::io::Error::last_os_error());
                }
                if write_raw != TRACE_TARGET_FD {
                    libc::close(write_raw);
                }
                Ok(())
            });
        }

        let mut child = cmd.spawn().expect("failed to spawn creft child");

        // Drop the parent's copy of the write end so the read below sees EOF
        // once the child exits and its copy is closed.
        drop(write_fd);

        let status = child.wait().expect("failed to wait for creft child");
        assert!(
            status.success(),
            "creft child must exit 0; got {:?}",
            status
        );

        let records = drain_trace(read_fd);
        assert_eq!(
            records.len(),
            1,
            "one trace record expected from subprocess"
        );
        assert_eq!(records[0]["block"], 0);
        assert_eq!(records[0]["lang"], "bash");
        assert_eq!(records[0]["exit"], 0);
        assert_eq!(
            records[0]["primitives"]["print"], 2,
            "two creft_print calls must yield count 2 in the subprocess trace"
        );
    }
}
