scsh 1.9.8

//! Harness definitions — the `.harness/<name>.yml` runnable-job format.
//!
//! A *harness definition* is a parameterized job the daemon (or `scsh run --def`) can run:
//! a one-line `description`, typed `params` that render as a control form and are forwarded
//! to the container as environment variables, a `task` body that becomes the skill's
//! `SKILL.md`, and an `invocations:` agent matrix (the same schema as `.scsh.yml`).
//!
//! Terminology: the code elsewhere calls the AI CLI (claude/codex/opencode/grok/cursor) a
//! "harness" ([`crate::config::Harness`]). To avoid colliding with the user-facing name for
//! these definitions, new code here calls a `.harness/` entry a *harness definition*
//! ([`HarnessDef`]) and the CLI underneath it the definition's *agent*.

use std::collections::BTreeMap;
use std::path::{Path, PathBuf};

use crate::config::{self, EnvRule, EnvVar, InvocationRoute, Node, Skill};

/// Env override pointing directly at the user-global `.harness` directory. Tests set it so
/// discovery never reads the real home; power users may relocate their global definitions.
pub const HARNESS_HOME_ENV: &str = "SCSH_HARNESS_HOME";

/// The built-in definitions, embedded at build time (mirrors `config::demo_yaml`), so
/// `doctor`/`add`/`research` (flat) and `fruits` (a workflow) are always available regardless of
/// the repo. `(name, yaml)`.
pub fn builtin_defs() -> [(&'static str, &'static str); 4] {
  [
    ("doctor", include_str!("harness_defs/doctor.yml")),
    ("add", include_str!("harness_defs/add.yml")),
    ("research", include_str!("harness_defs/research.yml")),
    ("fruits", include_str!("harness_defs/fruits.yml")),
  ]
}

/// Where a discovered definition came from — for the UI badge and discovery precedence.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum DefSource {
  /// Embedded in the scsh binary ([`builtin_defs`]); lowest precedence.
  Builtin,
  /// A file under the running user's `~/.harness/`; overrides a built-in of the same name.
  Home,
  /// A file under the open repo's `.harness/`; overrides both home and built-in.
  Repo,
}

impl DefSource {
  /// A stable lowercase tag for JSON/UI (`"builtin"`, `"home"`, `"repo"`).
  pub fn as_str(self) -> &'static str {
    match self {
      DefSource::Builtin => "builtin",
      DefSource::Home => "home",
      DefSource::Repo => "repo",
    }
  }
}

/// A parameter's value type. Determines the control the UI renders and how a supplied value
/// is validated before a run starts.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ParamType {
  /// Free text (rendered as a text input).
  String,
  /// An integer (rendered as a number input; validated with `i64::parse`).
  Int,
  /// `true`/`false` (rendered as a checkbox).
  Bool,
  /// One of a fixed set of `choices` (rendered as a select).
  Enum,
}

impl ParamType {
  fn parse(s: &str) -> Option<ParamType> {
    match s {
      "string" => Some(ParamType::String),
      "int" => Some(ParamType::Int),
      "bool" => Some(ParamType::Bool),
      "enum" => Some(ParamType::Enum),
      _ => None,
    }
  }

  /// A stable lowercase tag for JSON/UI (`"string"`, `"int"`, `"bool"`, `"enum"`).
  pub fn as_str(self) -> &'static str {
    match self {
      ParamType::String => "string",
      ParamType::Int => "int",
      ParamType::Bool => "bool",
      ParamType::Enum => "enum",
    }
  }
}

/// One declared parameter. Each becomes an environment variable of the same name forwarded
/// into the container (so `params` reuse the existing `${VAR:-default}` env machinery).
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Param {
  /// The variable name (also the env var); a valid POSIX-ish env name.
  pub name: String,
  /// The value type.
  pub ty: ParamType,
  /// The default value, if any. Presence makes the param optional.
  pub default: Option<String>,
  /// Whether a value must be supplied. Defaults to `true` unless a `default:` is given or
  /// `required: false` is set explicitly.
  pub required: bool,
  /// One-line human description shown as the form field's hint.
  pub description: Option<String>,
  /// Allowed values for an `enum` param (empty for other types).
  pub choices: Vec<String>,
}

impl Param {
  /// Build the [`EnvVar`] that forwards this param into the container:
  /// a `default:` param forwards the host value or injects the default; a required param
  /// with no default refuses the run when unset; an optional param with no default injects
  /// an empty value — exactly the `${VAR}` / `${VAR:-default}` semantics of `.scsh.yml`.
  pub fn to_env_var(&self) -> EnvVar {
    let src = self.name.clone();
    let rule = if let Some(default) = &self.default {
      EnvRule::Default { src, default: default.clone() }
    } else if self.required {
      EnvRule::Require { src, message: format!("harness-definition param '{}' is required", self.name) }
    } else {
      EnvRule::Default { src, default: String::new() }
    };
    EnvVar { key: self.name.clone(), rule }
  }

  /// Whether `value` is acceptable for this param's type. Used before a run starts (and by
  /// the UI). Returns a human-readable reason on rejection.
  pub fn validate_value(&self, value: &str) -> Result<(), String> {
    match self.ty {
      ParamType::String => Ok(()),
      ParamType::Int => value
        .trim()
        .parse::<i64>()
        .map(|_| ())
        .map_err(|_| format!("param '{}' must be an integer (got '{value}')", self.name)),
      ParamType::Bool => match value.trim() {
        "true" | "false" => Ok(()),
        other => Err(format!("param '{}' must be true or false (got '{other}')", self.name)),
      },
      ParamType::Enum => {
        if self.choices.iter().any(|c| c == value.trim()) {
          Ok(())
        } else {
          Err(format!("param '{}' must be one of: {} (got '{value}')", self.name, self.choices.join(", ")))
        }
      }
    }
  }
}

/// A reference in a `when:` condition or `inputs:` binding — either a run parameter
/// (`params.NAME`) or a field of an upstream step's validated output (`stepid.field`). This is
/// the ONE reference form workflows use; there is no expression language, only these two shapes.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum Ref {
  /// `params.NAME` — a run parameter.
  Param(String),
  /// `stepid.field` — a field of an upstream step's `output`.
  StepField { step: String, field: String },
}

impl Ref {
  /// Parse a `head.tail` reference. `params.NAME` is a param; anything else is `stepid.field`.
  fn parse(s: &str) -> Option<Ref> {
    let (head, tail) = s.trim().split_once('.')?;
    let (head, tail) = (head.trim(), tail.trim());
    if head.is_empty() || tail.is_empty() || tail.contains('.') {
      return None;
    }
    if head == "params" {
      Some(Ref::Param(tail.to_string()))
    } else {
      Some(Ref::StepField { step: head.to_string(), field: tail.to_string() })
    }
  }
}

/// One `inputs:` binding: the env var name the step receives (its own name), bound to a source.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct InputBinding {
  /// The environment variable the running step sees.
  pub name: String,
  /// Where its value comes from (a param or an upstream step's output field).
  pub source: Ref,
}

/// A comparison operator in a `when:` condition.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum CondOp {
  Eq,
  Ne,
  Lt,
  Lte,
  Gt,
  Gte,
  In,
}

impl CondOp {
  fn parse(s: &str) -> Option<CondOp> {
    match s {
      "eq" => Some(CondOp::Eq),
      "ne" => Some(CondOp::Ne),
      "lt" => Some(CondOp::Lt),
      "lte" => Some(CondOp::Lte),
      "gt" => Some(CondOp::Gt),
      "gte" => Some(CondOp::Gte),
      "in" => Some(CondOp::In),
      _ => None,
    }
  }
}

/// One condition: a reference compared against a literal (a comma-separated list, for `in`).
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Cond {
  pub reference: Ref,
  pub op: CondOp,
  /// The comparison value(s) — one, except `in` which takes several.
  pub values: Vec<String>,
}

/// A step gate: a set of conditions, ALL of which must hold (AND). Disjunction ("run in either
/// case") is expressed as separate steps, so the format needs no OR combinator — which also
/// keeps `when:` a plain block map the minimal YAML reader can parse.
pub type When = Vec<Cond>;

/// One output field a step promises to write to its result JSON (name + type, enum choices).
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct OutputField {
  pub name: String,
  pub ty: ParamType,
  pub choices: Vec<String>,
}

/// The agent (CLI + model) that runs a single step.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct StepAgent {
  pub harness: crate::config::Harness,
  pub model: Option<String>,
  pub effort: Option<String>,
}

/// One node in a workflow DAG. A step is a context-free unit: it receives its `inputs` as
/// named environment variables and writes its `output` fields to `$SCSH_RESULT` — it knows
/// nothing about the graph, other steps, or its own position (scsh resolves all of that).
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Step {
  /// Unique step id (`[A-Za-z0-9_]`).
  pub id: String,
  /// The agent that runs this step.
  pub agent: StepAgent,
  /// The task prompt (intent only; scsh appends the I/O contract from `inputs`/`outputs`).
  pub prompt: String,
  /// Input bindings: each names an env var the step sees and where its value comes from.
  pub inputs: Vec<InputBinding>,
  /// The typed result fields this step must produce (validated against `$SCSH_RESULT`).
  pub outputs: Vec<OutputField>,
  /// Optional gate: the step runs only when this evaluates true.
  pub when: Option<When>,
  /// Steps that must finish (or be skipped) before this one — the DAG edges.
  pub needs: Vec<String>,
}

/// A parsed, validated harness definition — either a flat one-shot task or a workflow of steps.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct HarnessDef {
  /// The definition name (the `.harness/<name>.yml` file stem, or the built-in name).
  pub name: String,
  /// Where this definition was loaded from.
  pub source: DefSource,
  /// One-line description shown in the definition list.
  pub description: String,
  /// Declared parameters, in file order.
  pub params: Vec<Param>,
  /// Flat form: the task prompt (`None` for a workflow), materialized into the run clone as
  /// `.skills/<name>/SKILL.md`.
  pub task: Option<String>,
  /// Flat form: the agent matrix — identical schema to a `.scsh.yml` skill's `invocations:`
  /// (empty for a workflow).
  pub invocations: Vec<InvocationRoute>,
  /// Workflow form: the DAG of steps (empty for a flat definition).
  pub steps: Vec<Step>,
}

impl HarnessDef {
  /// Whether this is a workflow (has `steps:`) rather than a flat one-shot task.
  pub fn is_workflow(&self) -> bool {
    !self.steps.is_empty()
  }

  /// Compile a FLAT definition into a synthetic [`Skill`] so the existing run path
  /// (`expand_invocations` → `build_and_run`) runs it unchanged. Params become the skill's
  /// forwarded `env`; the agent matrix becomes its `invocations`; results land under `tmp/`.
  /// (Workflows do not use this; the orchestrator builds a per-step invocation instead.)
  pub fn to_skill(&self) -> Skill {
    Skill {
      name: self.name.clone(),
      harness: None,
      model: None,
      effort: None,
      timeout: None,
      env: self.params.iter().map(Param::to_env_var).collect(),
      profile: None,
      commits: false,
      autoinstall: false,
      invocations: self.invocations.clone(),
      result: format!("tmp/{}_{{name}}.json", self.name),
    }
  }
}

impl Step {
  /// The full `SKILL.md` body scsh materializes for this step: the author's `prompt` plus the
  /// scsh-generated I/O contract — which env vars carry the inputs, and the exact JSON shape to
  /// write to `$SCSH_RESULT`. The author writes intent; scsh guarantees the machine contract.
  pub fn render_skill_body(&self) -> String {
    let mut s = self.prompt.trim_end().to_string();
    s.push_str("\n\n## Inputs\n\n");
    if self.inputs.is_empty() {
      s.push_str("This step takes no inputs.\n");
    } else {
      s.push_str("These values are provided as environment variables:\n");
      for b in &self.inputs {
        s.push_str(&format!("- `{}`\n", b.name));
      }
    }
    s.push_str("\n## Output\n\nWrite a single JSON object to the file at `$SCSH_RESULT` with exactly these fields:\n");
    for o in &self.outputs {
      let ty = match o.ty {
        ParamType::Enum => format!("one of: {}", o.choices.join(", ")),
        other => other.as_str().to_string(),
      };
      s.push_str(&format!("- `{}` ({ty})\n", o.name));
    }
    s.push_str("\nDo not write anything else to that file.\n");
    s
  }
}

impl Cond {
  /// Evaluate this condition. `value_of` returns the current string value of a reference
  /// (a param value, or a field of an upstream step's result), or `None` if unavailable.
  pub fn eval(&self, value_of: &impl Fn(&Ref) -> Option<String>) -> bool {
    let Some(actual) = value_of(&self.reference) else { return false };
    match self.op {
      CondOp::Eq => self.values.first().is_some_and(|v| *v == actual),
      CondOp::Ne => self.values.first().is_some_and(|v| *v != actual),
      CondOp::In => self.values.iter().any(|v| *v == actual),
      _ => {
        let (Ok(a), Some(Ok(b))) = (actual.trim().parse::<i64>(), self.values.first().map(|v| v.trim().parse::<i64>()))
        else {
          return false;
        };
        match self.op {
          CondOp::Lt => a < b,
          CondOp::Lte => a <= b,
          CondOp::Gt => a > b,
          CondOp::Gte => a >= b,
          _ => unreachable!("non-ordering op handled above"),
        }
      }
    }
  }
}

/// Whether a step's `when:` gate holds — every condition must (they are AND-ed).
pub fn when_holds(when: &When, value_of: &impl Fn(&Ref) -> Option<String>) -> bool {
  when.iter().all(|c| c.eval(value_of))
}

/// The result of discovering the definitions available to a repo: the merged definitions
/// (built-in < home < repo precedence) plus any per-file parse warnings to surface.
#[derive(Debug, Clone, Default)]
pub struct Discovery {
  /// Merged definitions, sorted by name.
  pub defs: Vec<HarnessDef>,
  /// One warning per `.harness/` file that failed to parse (`"<path>: <error>"`).
  pub warnings: Vec<String>,
}

impl Discovery {
  /// Find a definition by name.
  pub fn find(&self, name: &str) -> Option<&HarnessDef> {
    self.defs.iter().find(|d| d.name == name)
  }
}

/// Discover the definitions available to `repo_root`: the built-ins, overlaid by
/// `~/.harness/*.yml`, overlaid by `<repo_root>/.harness/*.yml`. Later sources shadow earlier
/// ones by name, so the effective precedence is repo > home > built-in.
pub fn discover(repo_root: &Path) -> Discovery {
  let mut map: BTreeMap<String, HarnessDef> = BTreeMap::new();
  let mut warnings = Vec::new();

  // Built-ins are embedded and covered by tests; a parse error here is a build-time bug, so
  // surface it as a warning rather than panicking a running daemon.
  for (name, src) in builtin_defs() {
    match validate(name, src, DefSource::Builtin) {
      Ok(def) => {
        map.insert(def.name.clone(), def);
      }
      Err(errs) => warnings.push(format!("built-in '{name}': {}", errs.join("; "))),
    }
  }

  if let Some(dir) = home_harness_dir() {
    load_dir(&dir, DefSource::Home, &mut map, &mut warnings);
  }
  load_dir(&repo_root.join(".harness"), DefSource::Repo, &mut map, &mut warnings);

  Discovery { defs: map.into_values().collect(), warnings }
}

/// The user-global `.harness` directory: `$SCSH_HARNESS_HOME`, else `$HOME/.harness`.
/// `None` when neither is set (headless with no home).
fn home_harness_dir() -> Option<PathBuf> {
  if let Some(dir) = std::env::var_os(HARNESS_HOME_ENV).filter(|s| !s.is_empty()) {
    return Some(PathBuf::from(dir));
  }
  std::env::var_os("HOME").filter(|s| !s.is_empty()).map(|home| PathBuf::from(home).join(".harness"))
}

/// Load every `*.yml` file in `dir` (if it exists) into `map`, keyed by file stem, replacing
/// any existing entry (so a later source shadows an earlier one). Files that fail to parse
/// add a warning and are skipped. Non-`.yml` entries and subdirectories are ignored.
fn load_dir(dir: &Path, source: DefSource, map: &mut BTreeMap<String, HarnessDef>, warnings: &mut Vec<String>) {
  let entries = match std::fs::read_dir(dir) {
    Ok(e) => e,
    Err(_) => return, // absent directory is normal, not an error
  };
  // Sort by path so discovery is deterministic regardless of readdir order.
  let mut paths: Vec<PathBuf> = entries.flatten().map(|e| e.path()).collect();
  paths.sort();
  for path in paths {
    if path.extension().and_then(|e| e.to_str()) != Some("yml") {
      continue;
    }
    let Some(stem) = path.file_stem().and_then(|s| s.to_str()) else { continue };
    if !is_def_name(stem) {
      warnings.push(format!("{}: '{stem}' is not a valid definition name (use [A-Za-z0-9_-])", path.display()));
      continue;
    }
    let src = match std::fs::read_to_string(&path) {
      Ok(s) => s,
      Err(e) => {
        warnings.push(format!("{}: {e}", path.display()));
        continue;
      }
    };
    match validate(stem, &src, source) {
      Ok(def) => {
        map.insert(def.name.clone(), def);
      }
      Err(errs) => warnings.push(format!("{}: {}", path.display(), errs.join("; "))),
    }
  }
}

/// A definition name must be a safe single path component so it can key a `.skills/<name>/`
/// folder and a `tmp/<name>_*.json` result without escaping the repo.
fn is_def_name(s: &str) -> bool {
  !s.is_empty() && s.chars().all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')
}

/// Parse and validate one `.harness/<name>.yml` source, collecting every problem found (like
/// `config::validate`). `source` records where it came from for the UI.
pub fn validate(name: &str, src: &str, source: DefSource) -> Result<HarnessDef, Vec<String>> {
  let entries = match config::parse_yaml(src) {
    Ok(e) => e,
    Err(e) => return Err(vec![format!("invalid YAML: {e}")]),
  };

  let mut errors = Vec::new();
  let mut top: BTreeMap<&str, &Node> = BTreeMap::new();
  for (k, v) in &entries {
    if top.insert(k.as_str(), v).is_some() {
      errors.push(format!("duplicate top-level key '{k}'"));
    }
  }
  const KNOWN: &[&str] = &["description", "params", "task", "invocations", "steps"];
  for (k, _) in &entries {
    if !KNOWN.contains(&k.as_str()) {
      errors.push(format!("unknown top-level key '{k}' (allowed: description, params, task, invocations, steps)"));
    }
  }

  let description = required_scalar(top.get("description").copied(), "description", &mut errors);

  let params = match top.get("params").copied() {
    None => Vec::new(),
    Some(Node::Scalar(_)) => {
      errors.push("'params' must be a mapping of named parameters".into());
      Vec::new()
    }
    Some(Node::Map(m)) => validate_params(m, &mut errors),
  };

  // A definition is EITHER a workflow (`steps:`) OR a flat one-shot task (`task:`+`invocations:`).
  let stepped = top.contains_key("steps");
  let flat = top.contains_key("task") || top.contains_key("invocations");
  let mut task = None;
  let mut invocations = Vec::new();
  let mut steps = Vec::new();
  if stepped && flat {
    errors
      .push("a definition uses either 'steps:' (a workflow) or 'task:'+'invocations:' (a one-shot), not both".into());
  } else if stepped {
    steps = validate_steps(top.get("steps").copied(), &params, &mut errors);
  } else {
    task = required_scalar(top.get("task").copied(), "task", &mut errors);
    invocations = match top.get("invocations").copied() {
      None => {
        errors.push("missing required key 'invocations' (an agent matrix, like a .scsh.yml skill) — or use 'steps:' for a workflow".into());
        Vec::new()
      }
      Some(node) => config::validate_invocations(name, node, &mut errors),
    };
    if top.contains_key("invocations") && invocations.is_empty() && errors.is_empty() {
      errors.push("'invocations' must list at least one agent route".into());
    }
  }

  if errors.is_empty() {
    Ok(HarnessDef {
      name: name.to_string(),
      source,
      description: description.unwrap_or_default(),
      params,
      task,
      invocations,
      steps,
    })
  } else {
    Err(errors)
  }
}

/// Validate the `steps:` block map (keyed by step id) into a DAG: each step has an agent, a
/// prompt, and typed `output` fields; `inputs`/`when` references resolve to a declared param or
/// an upstream step's output field; `needs` names other steps; and the graph is acyclic. The
/// minimal YAML reader has no flow collections, so `steps:` is a block map (not a sequence),
/// `needs:` is a comma-separated scalar, and `when:` is a plain block map (AND of its entries).
fn validate_steps(node: Option<&Node>, params: &[Param], errors: &mut Vec<String>) -> Vec<Step> {
  let entries = match node {
    Some(Node::Map(m)) if !m.is_empty() => m,
    Some(Node::Map(_)) => {
      errors.push("'steps' must define at least one step".into());
      return Vec::new();
    }
    _ => {
      errors.push("'steps' must be a mapping of named steps".into());
      return Vec::new();
    }
  };

  let mut steps: Vec<Step> = Vec::new();
  let mut seen: BTreeMap<&str, ()> = BTreeMap::new();
  for (id, node) in entries {
    let id = id.trim();
    if !config::is_env_name(id) {
      errors.push(format!("step id '{id}' is not a valid identifier ([A-Za-z_][A-Za-z0-9_]*)"));
      continue;
    }
    if seen.insert(id, ()).is_some() {
      errors.push(format!("duplicate step '{id}'"));
    }
    let fields = match node {
      Node::Map(f) => f,
      Node::Scalar(_) => {
        errors.push(format!("step '{id}' must be a mapping (agent, prompt, inputs, output, when, needs)"));
        continue;
      }
    };
    let mut fm: BTreeMap<&str, &Node> = BTreeMap::new();
    for (k, v) in fields {
      if fm.insert(k.as_str(), v).is_some() {
        errors.push(format!("duplicate key 'steps.{id}.{k}'"));
      }
    }
    const SK: &[&str] = &["agent", "prompt", "inputs", "output", "when", "needs"];
    for (k, _) in fields {
      if !SK.contains(&k.as_str()) {
        errors.push(format!("unknown key 'steps.{id}.{k}' (allowed: agent, prompt, inputs, output, when, needs)"));
      }
    }

    let agent = validate_step_agent(id, fm.get("agent").copied(), errors);
    let prompt = required_scalar(fm.get("prompt").copied(), &format!("steps.{id}.prompt"), errors);
    let inputs = validate_step_inputs(id, fm.get("inputs").copied(), errors);
    let outputs = validate_step_outputs(id, fm.get("output").copied(), errors);
    let when = validate_step_when(id, fm.get("when").copied(), errors);
    let needs = parse_needs(fm.get("needs").copied());

    if let (Some(agent), Some(prompt)) = (agent, prompt) {
      steps.push(Step { id: id.to_string(), agent, prompt, inputs, outputs, when, needs });
    }
  }

  validate_step_graph(&steps, params, errors);
  steps
}

/// Validate a step's `agent:` block into a [`StepAgent`] (harness required; model/effort optional).
fn validate_step_agent(id: &str, node: Option<&Node>, errors: &mut Vec<String>) -> Option<StepAgent> {
  let fields = match node {
    None => {
      errors.push(format!("step '{id}' is missing required key 'agent'"));
      return None;
    }
    Some(Node::Map(f)) => f,
    Some(Node::Scalar(_)) => {
      errors.push(format!("'steps.{id}.agent' must be a mapping with 'harness' (and optional 'model'/'effort')"));
      return None;
    }
  };
  let mut fm: BTreeMap<&str, &Node> = BTreeMap::new();
  for (k, v) in fields {
    fm.insert(k.as_str(), v);
  }
  for (k, _) in fields {
    if !["harness", "model", "effort"].contains(&k.as_str()) {
      errors.push(format!("unknown key 'steps.{id}.agent.{k}' (allowed: harness, model, effort)"));
    }
  }
  let harness = match fm.get("harness").copied() {
    Some(Node::Scalar(s)) => match crate::config::Harness::parse(s.trim()) {
      Some(h) => Some(h),
      None => {
        errors.push(format!("'steps.{id}.agent.harness' is '{}', not a known harness", s.trim()));
        None
      }
    },
    _ => {
      errors.push(format!("'steps.{id}.agent' is missing 'harness'"));
      None
    }
  };
  let model = step_opt_scalar(&fm, id, "model", errors);
  let effort = step_opt_scalar(&fm, id, "effort", errors);
  harness.map(|harness| StepAgent { harness, model, effort })
}

/// Validate a step's `inputs:` block into bindings (env var name → source reference).
fn validate_step_inputs(id: &str, node: Option<&Node>, errors: &mut Vec<String>) -> Vec<InputBinding> {
  let entries = match node {
    None => return Vec::new(),
    Some(Node::Map(m)) => m,
    Some(Node::Scalar(_)) => {
      errors.push(format!("'steps.{id}.inputs' must be a mapping of NAME: source"));
      return Vec::new();
    }
  };
  let mut out = Vec::new();
  for (name, node) in entries {
    let name = name.trim();
    if !config::is_env_name(name) {
      errors.push(format!("'steps.{id}.inputs': '{name}' is not a valid variable name"));
      continue;
    }
    let src = match node {
      Node::Scalar(s) => s.trim(),
      Node::Map(_) => {
        errors.push(format!("'steps.{id}.inputs.{name}' must be a reference like params.X or stepid.field"));
        continue;
      }
    };
    match Ref::parse(src) {
      Some(reference) => out.push(InputBinding { name: name.to_string(), source: reference }),
      None => errors.push(format!("'steps.{id}.inputs.{name}' is '{src}', not a params.X or stepid.field reference")),
    }
  }
  out
}

/// Validate a step's `output:` block into typed fields the step must produce.
fn validate_step_outputs(id: &str, node: Option<&Node>, errors: &mut Vec<String>) -> Vec<OutputField> {
  let entries = match node {
    None => {
      errors.push(format!("step '{id}' is missing required key 'output' (the fields it must produce)"));
      return Vec::new();
    }
    Some(Node::Map(m)) if !m.is_empty() => m,
    _ => {
      errors.push(format!("'steps.{id}.output' must declare at least one field"));
      return Vec::new();
    }
  };
  let mut out = Vec::new();
  for (field, node) in entries {
    let field = field.trim();
    if !config::is_env_name(field) {
      errors.push(format!("'steps.{id}.output': '{field}' is not a valid field name"));
      continue;
    }
    let fm = match node {
      Node::Map(m) => m,
      Node::Scalar(_) => {
        errors.push(format!("'steps.{id}.output.{field}' must be a mapping with 'type'"));
        continue;
      }
    };
    let mut m: BTreeMap<&str, &Node> = BTreeMap::new();
    for (k, v) in fm {
      m.insert(k.as_str(), v);
    }
    let ty = match m.get("type").copied() {
      Some(Node::Scalar(s)) => ParamType::parse(s.trim()).unwrap_or(ParamType::String),
      _ => ParamType::String,
    };
    let choices = match m.get("choices").copied() {
      Some(Node::Scalar(s)) => s.split(',').map(|c| c.trim().to_string()).filter(|c| !c.is_empty()).collect(),
      _ => Vec::new(),
    };
    if ty == ParamType::Enum && choices.is_empty() {
      errors.push(format!("'steps.{id}.output.{field}' is an enum but has no 'choices'"));
    }
    out.push(OutputField { name: field.to_string(), ty, choices });
  }
  out
}

/// Validate a step's `when:` block map into a list of AND-ed conditions.
fn validate_step_when(id: &str, node: Option<&Node>, errors: &mut Vec<String>) -> Option<When> {
  let entries = match node {
    None => return None,
    Some(Node::Map(m)) if !m.is_empty() => m,
    _ => {
      errors.push(format!("'steps.{id}.when' must be a non-empty mapping of condition entries"));
      return None;
    }
  };
  let mut conds = Vec::new();
  for (key, node) in entries {
    let Some(reference) = Ref::parse(key.trim()) else {
      errors.push(format!("'steps.{id}.when': '{}' is not a params.X or stepid.field reference", key.trim()));
      continue;
    };
    let (op, values) = match node {
      // A scalar value → equality.
      Node::Scalar(s) => (CondOp::Eq, vec![s.trim().to_string()]),
      // A one-entry mapping → the named operator.
      Node::Map(m) if m.len() == 1 => {
        let (opk, opv) = &m[0];
        let Some(op) = CondOp::parse(opk.trim()) else {
          errors.push(format!("'steps.{id}.when.{}': unknown operator '{}'", key.trim(), opk.trim()));
          continue;
        };
        match opv {
          Node::Scalar(s) if op == CondOp::In => {
            (op, s.split(',').map(|v| v.trim().to_string()).filter(|v| !v.is_empty()).collect())
          }
          Node::Scalar(s) => (op, vec![s.trim().to_string()]),
          Node::Map(_) => {
            errors.push(format!("'steps.{id}.when.{}.{}' must be a value", key.trim(), opk.trim()));
            continue;
          }
        }
      }
      Node::Map(_) => {
        errors.push(format!("'steps.{id}.when.{}' must be a value or a single operator mapping", key.trim()));
        continue;
      }
    };
    conds.push(Cond { reference, op, values });
  }
  (!conds.is_empty()).then_some(conds)
}

/// Parse `needs:` — a comma/space-separated scalar (brackets optional): `needs: a, b` or `[a, b]`.
fn parse_needs(node: Option<&Node>) -> Vec<String> {
  let Some(Node::Scalar(s)) = node else { return Vec::new() };
  s.trim()
    .trim_start_matches('[')
    .trim_end_matches(']')
    .split([',', ' '])
    .map(str::trim)
    .filter(|x| !x.is_empty())
    .map(str::to_string)
    .collect()
}

/// Cross-step checks: `needs` names defined steps; the graph is acyclic; every `inputs`/`when`
/// reference resolves to a declared param or an upstream step's declared output field, and any
/// referenced step is listed in `needs` (so the ordering that makes the value available is explicit).
fn validate_step_graph(steps: &[Step], params: &[Param], errors: &mut Vec<String>) {
  use std::collections::BTreeSet;
  let ids: BTreeSet<&str> = steps.iter().map(|s| s.id.as_str()).collect();
  let param_names: BTreeSet<&str> = params.iter().map(|p| p.name.as_str()).collect();
  let output_of = |step: &str| steps.iter().find(|s| s.id == step).map(|s| &s.outputs);

  for s in steps {
    for need in &s.needs {
      if !ids.contains(need.as_str()) {
        errors.push(format!("step '{}' needs '{need}', which is not a defined step", s.id));
      }
      if need == &s.id {
        errors.push(format!("step '{}' cannot need itself", s.id));
      }
    }
    let check_ref = |reference: &Ref, ctx: &str, errors: &mut Vec<String>| match reference {
      Ref::Param(n) => {
        if !param_names.contains(n.as_str()) {
          errors.push(format!("step '{}' {ctx} references params.{n}, which is not a declared param", s.id));
        }
      }
      Ref::StepField { step, field } => {
        if !s.needs.iter().any(|n| n == step) {
          errors.push(format!("step '{}' {ctx} references {step}.{field} but does not 'needs: {step}'", s.id));
        }
        match output_of(step) {
          None => {
            errors.push(format!("step '{}' {ctx} references {step}.{field}, but '{step}' is not a defined step", s.id))
          }
          Some(outputs) if !outputs.iter().any(|o| &o.name == field) => errors.push(format!(
            "step '{}' {ctx} references {step}.{field}, which '{step}' does not declare in its output",
            s.id
          )),
          Some(_) => {}
        }
      }
    };
    for b in &s.inputs {
      check_ref(&b.source, &format!("input '{}'", b.name), errors);
    }
    for c in s.when.iter().flatten() {
      check_ref(&c.reference, "when", errors);
    }
  }

  if let Some(cycle) = first_cycle(steps) {
    errors.push(format!("steps form a cycle via 'needs': {}", cycle.join(" → ")));
  }
}

/// Return a cycle in the `needs` graph (as a list of step ids) if one exists, via DFS.
fn first_cycle(steps: &[Step]) -> Option<Vec<String>> {
  use std::collections::BTreeMap as Map;
  let deps: Map<&str, &Vec<String>> = steps.iter().map(|s| (s.id.as_str(), &s.needs)).collect();
  // 0 = unvisited, 1 = on stack, 2 = done.
  let mut state: Map<&str, u8> = steps.iter().map(|s| (s.id.as_str(), 0u8)).collect();
  let mut stack: Vec<&str> = Vec::new();
  fn dfs<'a>(
    node: &'a str, deps: &Map<&'a str, &'a Vec<String>>, state: &mut Map<&'a str, u8>, stack: &mut Vec<&'a str>,
  ) -> Option<Vec<String>> {
    state.insert(node, 1);
    stack.push(node);
    if let Some(needs) = deps.get(node) {
      for n in needs.iter() {
        let n = n.as_str();
        match state.get(n).copied().unwrap_or(2) {
          1 => {
            let start = stack.iter().position(|x| *x == n).unwrap_or(0);
            let mut cyc: Vec<String> = stack[start..].iter().map(|s| s.to_string()).collect();
            cyc.push(n.to_string());
            return Some(cyc);
          }
          0 => {
            if let Some(c) = dfs(n, deps, state, stack) {
              return Some(c);
            }
          }
          _ => {}
        }
      }
    }
    stack.pop();
    state.insert(node, 2);
    None
  }
  for s in steps {
    if state.get(s.id.as_str()).copied().unwrap_or(2) == 0 {
      if let Some(c) = dfs(s.id.as_str(), &deps, &mut state, &mut stack) {
        return Some(c);
      }
    }
  }
  None
}

/// A step's optional scalar sub-field of `agent:`.
fn step_opt_scalar(fm: &BTreeMap<&str, &Node>, id: &str, field: &str, errors: &mut Vec<String>) -> Option<String> {
  match fm.get(field).copied() {
    None => None,
    Some(Node::Scalar(s)) if !s.trim().is_empty() => Some(s.trim().to_string()),
    Some(Node::Scalar(_)) => None,
    Some(Node::Map(_)) => {
      errors.push(format!("'steps.{id}.agent.{field}' must be a string"));
      None
    }
  }
}

/// Read a required, non-empty scalar top-level field.
fn required_scalar(node: Option<&Node>, field: &str, errors: &mut Vec<String>) -> Option<String> {
  match node {
    None => {
      errors.push(format!("missing required key '{field}'"));
      None
    }
    Some(Node::Map(_)) => {
      errors.push(format!("'{field}' must be a string"));
      None
    }
    Some(Node::Scalar(s)) => {
      if s.trim().is_empty() {
        errors.push(format!("'{field}' must not be empty"));
        None
      } else {
        Some(s.clone())
      }
    }
  }
}

/// Validate the `params:` mapping into [`Param`]s, pushing every problem found.
fn validate_params(entries: &[(String, Node)], errors: &mut Vec<String>) -> Vec<Param> {
  let mut out = Vec::new();
  let mut seen: BTreeMap<&str, ()> = BTreeMap::new();
  for (name, node) in entries {
    let name = name.trim();
    if !config::is_env_name(name) {
      errors.push(format!("param '{name}' is not a valid variable name ([A-Za-z_][A-Za-z0-9_]*)"));
      continue;
    }
    if seen.insert(name, ()).is_some() {
      errors.push(format!("duplicate param '{name}'"));
    }
    let fields = match node {
      Node::Map(f) => f,
      Node::Scalar(_) => {
        errors.push(format!("param '{name}' must be a mapping (type, default, required, description, choices)"));
        continue;
      }
    };
    let mut fm: BTreeMap<&str, &Node> = BTreeMap::new();
    for (k, v) in fields {
      if fm.insert(k.as_str(), v).is_some() {
        errors.push(format!("duplicate key 'params.{name}.{k}'"));
      }
    }
    const PK: &[&str] = &["type", "default", "required", "description", "choices"];
    for (k, _) in fields {
      if !PK.contains(&k.as_str()) {
        errors
          .push(format!("unknown key 'params.{name}.{k}' (allowed: type, default, required, description, choices)"));
      }
    }

    let ty = match fm.get("type").copied() {
      None => ParamType::String, // default type
      Some(Node::Scalar(s)) => match ParamType::parse(s.trim()) {
        Some(t) => t,
        None => {
          errors.push(format!("'params.{name}.type' is '{}', not one of: string, int, bool, enum", s.trim()));
          ParamType::String
        }
      },
      Some(Node::Map(_)) => {
        errors.push(format!("'params.{name}.type' must be a string"));
        ParamType::String
      }
    };

    let default = opt_scalar(&fm, name, "default", errors);
    let description = opt_scalar(&fm, name, "description", errors);

    let choices = match fm.get("choices").copied() {
      None => Vec::new(),
      Some(Node::Scalar(s)) => s.split(',').map(|c| c.trim().to_string()).filter(|c| !c.is_empty()).collect(),
      Some(Node::Map(_)) => {
        errors.push(format!("'params.{name}.choices' must be a comma-separated string (e.g. \"a, b, c\")"));
        Vec::new()
      }
    };
    if ty == ParamType::Enum && choices.is_empty() {
      errors.push(format!("'params.{name}' is an enum but has no 'choices'"));
    }
    if ty != ParamType::Enum && !choices.is_empty() {
      errors.push(format!("'params.{name}.choices' is only valid for an enum param"));
    }

    // required defaults to true, unless a default is given or required:false is explicit.
    let required = match fm.get("required").copied() {
      None => default.is_none(),
      Some(Node::Scalar(s)) => match s.trim() {
        "true" => true,
        "false" => false,
        other => {
          errors.push(format!("'params.{name}.required' must be true or false (got '{other}')"));
          default.is_none()
        }
      },
      Some(Node::Map(_)) => {
        errors.push(format!("'params.{name}.required' must be true or false"));
        default.is_none()
      }
    };

    let param = Param { name: name.to_string(), ty, default, required, description, choices };
    // A supplied default must itself satisfy the declared type/choices.
    if let Some(d) = &param.default {
      if let Err(e) = param.validate_value(d) {
        errors.push(format!("'params.{name}.default' is invalid: {e}"));
      }
    }
    out.push(param);
  }
  out
}

/// Read an optional non-empty scalar sub-field of a param.
fn opt_scalar(fm: &BTreeMap<&str, &Node>, param: &str, field: &str, errors: &mut Vec<String>) -> Option<String> {
  match fm.get(field).copied() {
    None => None,
    Some(Node::Scalar(s)) => Some(s.clone()),
    Some(Node::Map(_)) => {
      errors.push(format!("'params.{param}.{field}' must be a string"));
      None
    }
  }
}

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

  /// Index into `builtin_defs()` for readability.
  fn builtin(name: &str) -> HarnessDef {
    let (_, src) = builtin_defs().into_iter().find(|(n, _)| *n == name).expect("known built-in");
    validate(name, src, DefSource::Builtin).unwrap_or_else(|e| panic!("{name}: {}", e.join("; ")))
  }

  #[test]
  fn builtins_parse_and_have_expected_shape() {
    let add = builtin("add");
    assert_eq!(add.params.len(), 2);
    assert!(add.params.iter().all(|p| p.ty == ParamType::Int));
    assert_eq!(add.params.iter().find(|p| p.name == "A").unwrap().default.as_deref(), Some("2"));
    // The `task:` block scalar is preserved verbatim across multiple lines.
    let task = add.task.as_deref().expect("flat def has a task");
    assert!(task.contains('\n'), "task should be multi-line");
    assert!(task.contains("SCSH_RESULT"), "task body preserved");
    // add spins up every non-opencode agent: codex, claude, cursor, grok.
    assert_eq!(add.invocations.len(), 4);
    let agents: std::collections::BTreeSet<&str> = add.invocations.iter().map(|r| r.harness.as_str()).collect();
    assert_eq!(agents, ["claude", "codex", "cursor", "grok"].into_iter().collect());
    assert!(!agents.contains("opencode"), "opencode is intentionally excluded");

    let research = builtin("research");
    let city = research.params.iter().find(|p| p.name == "CITY").unwrap();
    assert!(city.required && city.default.is_none(), "CITY is required with no default");
    let area = research.params.iter().find(|p| p.name == "AREA").unwrap();
    assert!(!area.required && area.default.as_deref() == Some(""), "AREA optional, empty default");

    let doctor = builtin("doctor");
    assert!(doctor.params.is_empty());
    // doctor exercises every agent end to end — all five harnesses.
    assert_eq!(doctor.invocations.len(), 5);
    let doc_agents: std::collections::BTreeSet<&str> = doctor.invocations.iter().map(|r| r.harness.as_str()).collect();
    assert_eq!(doc_agents, ["claude", "codex", "cursor", "grok", "opencode"].into_iter().collect());
  }

  #[test]
  fn builtin_fruits_workflow_parses() {
    let f = builtin("fruits");
    assert!(f.is_workflow(), "fruits is a workflow");
    assert!(f.task.is_none() && f.invocations.is_empty(), "a workflow has no flat task/invocations");
    assert_eq!(f.steps.len(), 3);
    let categorize = f.steps.iter().find(|s| s.id == "categorize").unwrap();
    assert!(categorize.needs.is_empty() && categorize.outputs.iter().any(|o| o.name == "fruits"));
    let sort_fruits = f.steps.iter().find(|s| s.id == "sort_fruits").unwrap();
    assert_eq!(sort_fruits.needs, vec!["categorize"]);
    // Its LIST input binds to categorize.fruits.
    let bind = sort_fruits.inputs.iter().find(|b| b.name == "LIST").unwrap();
    assert_eq!(bind.source, Ref::StepField { step: "categorize".into(), field: "fruits".into() });
  }

  /// A minimal two-step workflow source for negative tests.
  fn wf(extra_second: &str) -> String {
    format!(
      "description: \"x\"\nsteps:\n  a:\n    agent:\n      harness: claude\n      model: sonnet\n    prompt: |\n      do a\n    output:\n      kind:\n        type: string\n  b:\n{extra_second}\n"
    )
  }

  #[test]
  fn workflow_rejects_reference_to_undeclared_output() {
    // b references a.missing, which a does not declare in its output.
    let src = wf("    needs: a\n    agent:\n      harness: claude\n      model: sonnet\n    inputs:\n      X: a.missing\n    prompt: |\n      go\n    output:\n      y:\n        type: string");
    let err = validate("t", &src, DefSource::Repo).unwrap_err();
    assert!(err.iter().any(|e| e.contains("a.missing")), "{err:?}");
  }

  #[test]
  fn workflow_rejects_reference_without_needs() {
    // b references a.kind but does not declare needs: a.
    let src = wf("    agent:\n      harness: claude\n      model: sonnet\n    inputs:\n      X: a.kind\n    prompt: |\n      go\n    output:\n      y:\n        type: string");
    let err = validate("t", &src, DefSource::Repo).unwrap_err();
    assert!(err.iter().any(|e| e.contains("does not 'needs: a'")), "{err:?}");
  }

  #[test]
  fn workflow_rejects_cycles() {
    let src = "description: \"x\"\nsteps:\n  a:\n    needs: b\n    agent:\n      harness: claude\n      model: sonnet\n    prompt: |\n      a\n    output:\n      y:\n        type: string\n  b:\n    needs: a\n    agent:\n      harness: claude\n      model: sonnet\n    prompt: |\n      b\n    output:\n      y:\n        type: string\n";
    let err = validate("t", src, DefSource::Repo).unwrap_err();
    assert!(err.iter().any(|e| e.contains("cycle")), "{err:?}");
  }

  #[test]
  fn workflow_and_flat_are_mutually_exclusive() {
    let src = "description: \"x\"\ntask: |\n  do\ninvocations:\n  c:\n    harness: claude\n    model: sonnet\nsteps:\n  a:\n    agent:\n      harness: claude\n      model: sonnet\n    prompt: |\n      a\n    output:\n      y:\n        type: string\n";
    let err = validate("t", src, DefSource::Repo).unwrap_err();
    assert!(err.iter().any(|e| e.contains("either 'steps:'")), "{err:?}");
  }

  #[test]
  fn condition_evaluation() {
    let refv = Ref::StepField { step: "s".into(), field: "n".into() };
    let ge = Cond { reference: refv.clone(), op: CondOp::Gte, values: vec!["3".into()] };
    assert!(ge.eval(&|_| Some("5".into())));
    assert!(!ge.eval(&|_| Some("2".into())));
    let eq = Cond { reference: refv.clone(), op: CondOp::Eq, values: vec!["code".into()] };
    assert!(eq.eval(&|_| Some("code".into())));
    assert!(!eq.eval(&|_| Some("docs".into())));
    // A missing value never satisfies a condition.
    assert!(!eq.eval(&|_| None));
  }

  #[test]
  fn step_body_carries_the_io_contract() {
    let f = builtin("fruits");
    let body = f.steps.iter().find(|s| s.id == "categorize").unwrap().render_skill_body();
    assert!(body.contains("WORDS"), "names the input");
    assert!(body.contains("$SCSH_RESULT"), "points at the result file");
    assert!(body.contains("fruits") && body.contains("vegetables"), "lists the output fields");
  }

  #[test]
  fn params_compile_to_env_rules() {
    let with_default = Param {
      name: "A".into(),
      ty: ParamType::Int,
      default: Some("2".into()),
      required: false,
      description: None,
      choices: vec![],
    };
    match with_default.to_env_var().rule {
      EnvRule::Default { src, default } => {
        assert_eq!(src, "A");
        assert_eq!(default, "2");
      }
      other => panic!("expected Default, got {other:?}"),
    }

    let required = Param {
      name: "CITY".into(),
      ty: ParamType::String,
      default: None,
      required: true,
      description: None,
      choices: vec![],
    };
    assert!(matches!(required.to_env_var().rule, EnvRule::Require { .. }));

    let optional = Param {
      name: "AREA".into(),
      ty: ParamType::String,
      default: None,
      required: false,
      description: None,
      choices: vec![],
    };
    match optional.to_env_var().rule {
      EnvRule::Default { default, .. } => assert_eq!(default, ""),
      other => panic!("expected empty Default, got {other:?}"),
    }
  }

  #[test]
  fn value_validation_by_type() {
    let int =
      Param { name: "N".into(), ty: ParamType::Int, default: None, required: true, description: None, choices: vec![] };
    assert!(int.validate_value("42").is_ok());
    assert!(int.validate_value("x").is_err());

    let boolean = Param {
      name: "B".into(),
      ty: ParamType::Bool,
      default: None,
      required: true,
      description: None,
      choices: vec![],
    };
    assert!(boolean.validate_value("true").is_ok());
    assert!(boolean.validate_value("yes").is_err());

    let choice = Param {
      name: "E".into(),
      ty: ParamType::Enum,
      default: None,
      required: true,
      description: None,
      choices: vec!["a".into(), "b".into()],
    };
    assert!(choice.validate_value("a").is_ok());
    assert!(choice.validate_value("c").is_err());
  }

  #[test]
  fn compiles_to_skill_and_expands() {
    let skill = builtin("add").to_skill();
    assert_eq!(skill.name, "add");
    assert!(skill.harness.is_none());
    assert_eq!(skill.env.len(), 2);
    assert!(skill.result.contains("{name}"));

    let cfg = crate::config::Config { skills: vec![skill], terminal: crate::config::Terminal::default() };
    let inv = crate::config::expand_invocations(&cfg);
    assert_eq!(inv.len(), 4);
    assert!(inv.iter().any(|i| i.name == "add-claude-sonnet-4-6"));
    // Each route substitutes its own name into the result path (no collisions).
    assert!(inv.iter().any(|i| i.result == "tmp/add_claude-sonnet-4-6.json"));
  }

  #[test]
  fn unknown_and_missing_keys_are_rejected() {
    let bad =
      "description: \"x\"\ntask: |\n  go\ninvocations:\n  c:\n    harness: claude\n    model: sonnet\nbogus: 1\n";
    let err = validate("t", bad, DefSource::Repo).unwrap_err();
    assert!(err.iter().any(|e| e.contains("bogus")), "{err:?}");

    let no_task = "description: \"x\"\ninvocations:\n  c:\n    harness: claude\n    model: sonnet\n";
    let err = validate("t", no_task, DefSource::Repo).unwrap_err();
    assert!(err.iter().any(|e| e.contains("task")), "{err:?}");
  }

  #[test]
  fn repo_shadows_builtin_by_name() {
    let mut map: BTreeMap<String, HarnessDef> = BTreeMap::new();
    let add = builtin("add");
    map.insert(add.name.clone(), add);
    assert_eq!(map["add"].source, DefSource::Builtin);

    let base = std::env::temp_dir().join(format!("scsh-hd-{}", crate::runtime::random_nonce_6()));
    let hdir = base.join(".harness");
    std::fs::create_dir_all(&hdir).unwrap();
    std::fs::write(
      hdir.join("add.yml"),
      "description: \"Repo add.\"\ntask: |\n  do it\ninvocations:\n  c:\n    harness: claude\n    model: sonnet\n",
    )
    .unwrap();

    let mut warnings = Vec::new();
    load_dir(&hdir, DefSource::Repo, &mut map, &mut warnings);
    assert!(warnings.is_empty(), "warnings: {warnings:?}");
    assert_eq!(map["add"].source, DefSource::Repo);
    assert_eq!(map["add"].description, "Repo add.");
    std::fs::remove_dir_all(&base).ok();
  }

  #[test]
  fn discover_merges_builtins_home_and_repo() {
    let base = std::env::temp_dir().join(format!("scsh-disc-{}", crate::runtime::random_nonce_6()));
    let home = base.join("home");
    std::fs::create_dir_all(&home).unwrap(); // empty home .harness
    let repo = base.join("repo");
    let rh = repo.join(".harness");
    std::fs::create_dir_all(&rh).unwrap();
    std::fs::write(
      rh.join("mine.yml"),
      "description: \"Mine.\"\ntask: |\n  go\ninvocations:\n  c:\n    harness: claude\n    model: sonnet\n",
    )
    .unwrap();

    std::env::set_var(HARNESS_HOME_ENV, &home);
    let d = discover(&repo);
    std::env::remove_var(HARNESS_HOME_ENV);

    assert!(d.find("doctor").is_some() && d.find("add").is_some() && d.find("research").is_some());
    let mine = d.find("mine").expect("repo def discovered");
    assert_eq!(mine.source, DefSource::Repo);
    assert!(d.warnings.is_empty(), "warnings: {:?}", d.warnings);
    std::fs::remove_dir_all(&base).ok();
  }
}