diff --git a/Cargo.lock b/Cargo.lock index 651b28d..b461e4b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -57,6 +57,7 @@ dependencies = [ "event-listener", "interprocess", "jsonschema", + "regex", "rusqlite", "serde", "serde_json", diff --git a/ail-core/CLAUDE.md b/ail-core/CLAUDE.md index 38cfa31..224fe4e 100644 --- a/ail-core/CLAUDE.md +++ b/ail-core/CLAUDE.md @@ -12,7 +12,8 @@ Consumed by `ail` (the binary) and future language-server / SDK targets. | `config/domain.rs` | Validated domain types — no `Deserialize` derives | | `config/validation/mod.rs` | `validate()` entry point, `cfg_err!` macro, `tools_to_policy` helper | | `config/validation/step_body.rs` | `parse_step_body()` — primary field count check + body construction (including `parse_do_while_body`, `parse_for_each_body`, `load_loop_pipeline_steps`) | -| `config/validation/on_result.rs` | `parse_result_branches()` — DTO → domain for result matchers and actions | +| `config/validation/on_result.rs` | `parse_result_branches()` — DTO → domain for result matchers and actions (incl. `expression:` via `parse_condition_expression`, and `matches:` desugared to expression form) | +| `config/validation/regex_literal.rs` | `parse_regex_literal()` — parses `/PATTERN/FLAGS` into a compiled `regex::Regex` with source preservation (SPEC §12.3) | | `config/validation/system_prompt.rs` | `parse_append_system_prompt()` — DTO → domain for system prompt entries | | `config/validation/sampling.rs` | `validate_sampling()` — DTO → domain with range checks; normalizes thinking (f64 OR bool) to `Option` (SPEC §30.6.1) | | `config/inheritance.rs` | FROM inheritance — path resolution, cycle detection, DTO merging, hook operations (SPEC §7, §8) | @@ -76,8 +77,9 @@ pub struct Step { pub id: StepId, pub body: StepBody, pub message: Option }, NamedPipeline { name: String, prompt: Option }, Action(ActionKind), Context(ContextSource), DoWhile { max_iterations, exit_when, steps }, ForEach { over, as_name, max_items, on_max_items, steps } } @@ -90,8 +92,9 @@ pub enum ActionKind { PauseForHuman, ModifyOutput { headless_behavior: HitlHeadl pub enum JoinErrorMode { FailFast, WaitForAll } // SPEC §29.7 — default FailFast pub enum HitlHeadlessBehavior { Skip, Abort, UseDefault } pub struct ResultBranch { pub matcher: ResultMatcher, pub action: ResultAction } -pub enum ResultMatcher { Contains(String), ExitCode(ExitCodeMatch), Field { name: String, equals: serde_json::Value }, Always } +pub enum ResultMatcher { Contains(String), ExitCode(ExitCodeMatch), Field { name: String, equals: serde_json::Value }, Expression { source: String, condition: Condition }, Always } // Field: exact equality match against a named field in validated input JSON (SPEC §26.4); requires input_schema +// Expression: §12.2 condition grammar (SPEC §5.4 `expression:`) — source is original expression string for materialize/diagnostics; condition is Expression(ConditionExpr) or Regex(RegexCondition). Named `matches:` in YAML desugars to Expression at parse time (SPEC §5.4). pub enum ExitCodeMatch { Exact(i32), Any } pub enum ResultAction { Continue, Break, AbortPipeline, PauseForHuman, Pipeline { path: String, prompt: Option } } // Pipeline.path may contain {{ variable }} syntax — resolved at execution time (SPEC §11) diff --git a/ail-core/Cargo.toml b/ail-core/Cargo.toml index 8a83d81..67465dc 100644 --- a/ail-core/Cargo.toml +++ b/ail-core/Cargo.toml @@ -18,6 +18,7 @@ rusqlite = { version = "0.31", features = ["bundled"] } tempfile = "3" ureq = { version = "2", features = ["json"] } event-listener = "5" +regex = "1" [dev-dependencies] stub-llm = { path = "../stub-llm" } diff --git a/ail-core/src/config/domain.rs b/ail-core/src/config/domain.rs index c9f968c..ba2d311 100644 --- a/ail-core/src/config/domain.rs +++ b/ail-core/src/config/domain.rs @@ -162,16 +162,24 @@ impl Pipeline { } /// Controls whether a step executes (SPEC §12). -#[derive(Debug, Clone, PartialEq)] +/// +/// `PartialEq` is intentionally not derived: `Regex` variants carry a +/// compiled [`regex::Regex`], which does not implement `PartialEq`. Compare +/// via pattern-match (`matches!`) or by inspecting the contained source. +#[derive(Debug, Clone)] pub enum Condition { /// Step always executes (same as omitting `condition:`). Always, /// Step is unconditionally skipped. Never, - /// Expression condition evaluated at runtime against session state (SPEC §12.2). + /// Comparison expression evaluated at runtime against session state (SPEC §12.2). /// The string may contain `{{ variable }}` template syntax which is resolved /// before evaluating the expression operator. Expression(ConditionExpr), + /// Regex-match expression evaluated at runtime (SPEC §12.2 `matches` operator, + /// regex semantics from §12.3). The pattern is compiled at parse time; an + /// invalid regex fails pipeline load, not evaluation. + Regex(RegexCondition), } /// A parsed condition expression (SPEC §12.2). @@ -188,6 +196,21 @@ pub struct ConditionExpr { pub rhs: String, } +/// A parsed regex-match condition (SPEC §12.2 `matches` / §12.3). +/// +/// The `lhs` is a template string resolved at evaluation time; the compiled +/// regex is applied to the resolved string. `source` preserves the original +/// `/PATTERN/FLAGS` literal for error messages and materialize output. +#[derive(Debug, Clone)] +pub struct RegexCondition { + /// Left-hand side — a template string resolved at evaluation time. + pub lhs: String, + /// Compiled regex, built at parse time per §12.3. + pub regex: regex::Regex, + /// Original source literal, e.g. `/warn|error/i`. Preserved for diagnostics. + pub source: String, +} + /// Comparison operators for condition expressions (SPEC §12.2). #[derive(Debug, Clone, PartialEq)] pub enum ConditionOp { @@ -427,6 +450,18 @@ pub enum ResultMatcher { name: String, equals: serde_json::Value, }, + /// Arbitrary §12.2 expression matcher (SPEC §5.4 `expression:`). Accepts the full + /// `Condition` union because the condition parser returns `Expression` for + /// comparison operators and `Regex` for the `matches` operator. + /// + /// The `source` field preserves the original expression string for diagnostics + /// and materialize output. `Condition::Always`/`Never` do not occur here — + /// only `Expression` and `Regex` variants are produced by the expression + /// parser used for this matcher. + Expression { + source: String, + condition: Condition, + }, Always, } diff --git a/ail-core/src/config/dto.rs b/ail-core/src/config/dto.rs index d081dc3..2ebe85e 100644 --- a/ail-core/src/config/dto.rs +++ b/ail-core/src/config/dto.rs @@ -226,6 +226,13 @@ pub enum OnResultDto { pub struct OnResultBranchDto { pub contains: Option, pub exit_code: Option, + /// Regex literal `/PATTERN/FLAGS` matched against the step's `response` + /// (SPEC §5.4, §12.3). Shorthand for `expression: '{{ step..response }} + /// matches /.../flags'`. + pub matches: Option, + /// Full §12.2 condition expression (SPEC §5.4 `expression:`). Supports any + /// operator the §12.2 grammar supports, including `matches` for regex. + pub expression: Option, pub always: Option, pub action: Option, /// Optional prompt override passed to the child session when action is `pipeline:`. diff --git a/ail-core/src/config/validation/mod.rs b/ail-core/src/config/validation/mod.rs index 27d14d0..eb5ddfd 100644 --- a/ail-core/src/config/validation/mod.rs +++ b/ail-core/src/config/validation/mod.rs @@ -3,16 +3,19 @@ #![allow(clippy::result_large_err)] mod on_result; +mod regex_literal; mod sampling; mod step_body; mod system_prompt; +pub(crate) use regex_literal::parse_regex_literal; + use std::collections::{HashMap, HashSet}; use std::path::PathBuf; use super::domain::{ ActionKind, Condition, ConditionExpr, ConditionOp, JoinErrorMode, OnError, Pipeline, - ProviderConfig, Step, StepBody, StepId, ToolPolicy, + ProviderConfig, RegexCondition, Step, StepBody, StepId, ToolPolicy, }; use super::dto::{ChainStepDto, PipelineFileDto, StepDto, ToolsDto}; use crate::error::AilError; @@ -39,20 +42,50 @@ fn tools_to_policy(t: ToolsDto) -> ToolPolicy { } } -/// Parse a condition expression string into a `Condition::Expression`. +/// Parse a condition expression string into a [`Condition`] (SPEC §12.2). /// -/// Supported operators: `==`, `!=`, `contains`, `starts_with`, `ends_with`. -/// The LHS is typically a template variable (e.g. `{{ step.test.exit_code }}`), -/// and the RHS is a literal or quoted string. +/// Supported operators: `==`, `!=`, `contains`, `starts_with`, `ends_with`, +/// `matches`. The LHS is typically a template variable (e.g. `{{ step.test.exit_code }}`), +/// and the RHS is a literal for comparison operators or a `/PATTERN/FLAGS` +/// regex literal for `matches` (see §12.3). /// /// Examples: /// `"{{ step.test.exit_code }} == 0"` /// `"{{ step.review.response }} contains 'LGTM'"` /// `"{{ step.build.exit_code }} != 0"` +/// `"{{ step.lint.stdout }} matches /warning|error/i"` pub(in crate::config) fn parse_condition_expression( raw: &str, step_id: &str, ) -> Result { + // The `matches` operator is parsed first because its RHS has its own + // syntax (a regex literal) and must not be confused with other operators. + if let Some(pos) = find_operator_position(raw, "matches") { + let lhs = raw[..pos].trim().to_string(); + let rhs_raw = raw[pos + "matches".len()..].trim(); + + if lhs.is_empty() { + return Err(cfg_err!( + "Step '{step_id}' condition expression has an empty left-hand side" + )); + } + if rhs_raw.is_empty() { + return Err(cfg_err!( + "Step '{step_id}' condition expression 'matches' operator requires a regex \ + literal on the right-hand side (e.g. /pattern/flags) (SPEC §12.3)" + )); + } + + let parsed = parse_regex_literal(rhs_raw) + .map_err(|e| cfg_err!("Step '{step_id}' condition 'matches' operator: {e}"))?; + + return Ok(Condition::Regex(RegexCondition { + lhs, + regex: parsed.regex, + source: parsed.source, + })); + } + // Operator tokens in order of specificity (multi-word first to avoid partial matches). let operators: &[(&str, ConditionOp)] = &[ ("starts_with", ConditionOp::StartsWith), @@ -90,7 +123,7 @@ pub(in crate::config) fn parse_condition_expression( Err(cfg_err!( "Step '{step_id}' specifies condition '{raw}' which is not a recognised \ named condition ('always', 'never') and does not contain a supported operator \ - (==, !=, contains, starts_with, ends_with)" + (==, !=, contains, starts_with, ends_with, matches)" )) } @@ -1220,7 +1253,10 @@ mod tests { step.condition = Some("never".to_string()); let dto = minimal_dto(vec![step]); let pipeline = validate(dto, source()).expect("should succeed"); - assert_eq!(pipeline.steps[0].condition, Some(Condition::Never)); + assert!(matches!( + pipeline.steps[0].condition, + Some(Condition::Never) + )); } #[test] diff --git a/ail-core/src/config/validation/on_result.rs b/ail-core/src/config/validation/on_result.rs index eef331c..6397680 100644 --- a/ail-core/src/config/validation/on_result.rs +++ b/ail-core/src/config/validation/on_result.rs @@ -6,7 +6,7 @@ use crate::config::domain::{ExitCodeMatch, ResultAction, ResultBranch, ResultMat use crate::config::dto::{ExitCodeDto, FieldEqualsActionDto, OnResultDto}; use crate::error::AilError; -use super::cfg_err; +use super::{cfg_err, parse_condition_expression}; /// Parse `on_result` from either the multi-branch array format or the /// field-equals binary branch format (SPEC §5.4, §26.4). @@ -32,6 +32,8 @@ fn parse_result_branches( let matcher_count = [ branch.contains.is_some(), branch.exit_code.is_some(), + branch.matches.is_some(), + branch.expression.is_some(), branch.always.is_some(), ] .iter() @@ -41,7 +43,7 @@ fn parse_result_branches( if matcher_count != 1 { return Err(cfg_err!( "Step '{step_id}' on_result branch {i} must have exactly one matcher \ - (contains, exit_code, always); found {matcher_count}" + (contains, exit_code, matches, expression, always); found {matcher_count}" )); } @@ -65,6 +67,28 @@ fn parse_result_branches( } }; ResultMatcher::ExitCode(exit_code_match) + } else if let Some(regex_literal) = branch.matches { + // Named `matches:` is shorthand for `expression: '{{ step..response }} matches /.../'` + // (SPEC §5.4). Desugar at parse time so both forms share a single evaluator. + let source = format!("{{{{ step.{step_id}.response }}}} matches {regex_literal}"); + let condition = parse_condition_expression(&source, step_id).map_err(|e| { + cfg_err!( + "Step '{step_id}' on_result branch {i} matches: {}", + e.detail() + ) + })?; + ResultMatcher::Expression { source, condition } + } else if let Some(expr_str) = branch.expression { + let condition = parse_condition_expression(&expr_str, step_id).map_err(|e| { + cfg_err!( + "Step '{step_id}' on_result branch {i} expression: {}", + e.detail() + ) + })?; + ResultMatcher::Expression { + source: expr_str, + condition, + } } else { ResultMatcher::Always }; diff --git a/ail-core/src/config/validation/regex_literal.rs b/ail-core/src/config/validation/regex_literal.rs new file mode 100644 index 0000000..a7121d8 --- /dev/null +++ b/ail-core/src/config/validation/regex_literal.rs @@ -0,0 +1,302 @@ +//! Parses `/PATTERN/FLAGS` regex literals per SPEC §12.3. +//! +//! Regex literals use conventional JavaScript/Perl/Ruby-style syntax: +//! a leading `/`, the pattern, a closing `/`, and zero or more flag +//! characters. The parser compiles the regex at parse time, so invalid +//! patterns and unsupported flags fail at pipeline load — never at match +//! time. + +#![allow(clippy::result_large_err)] + +use regex::{Regex, RegexBuilder}; + +/// A parsed regex literal with both the compiled form and the original source. +/// +/// `source` is preserved for error messages and materialize output so the +/// literal the user wrote can be surfaced verbatim rather than reconstructed. +#[derive(Debug, Clone)] +pub struct ParsedRegex { + /// Original source, e.g. `/warn|error/i`. + pub source: String, + /// Compiled regex. + pub regex: Regex, +} + +/// Parse a `/PATTERN/FLAGS` literal per SPEC §12.3. +/// +/// - Delimiters are the *first* `/` and the *last* `/` followed by zero or +/// more flag characters (`[ims]*`) at end-of-string. +/// - Supported flags: `i` (case-insensitive), `m` (multiline), `s` (dotall). +/// - `g` is rejected explicitly — matching is boolean, so "global" is +/// meaningless. +/// - Other trailing letters are rejected as unsupported flags. +/// - `\/` inside the pattern is unescaped to a literal `/` before compilation. +/// +/// On parse or compile failure, returns a human-readable diagnostic string +/// suitable for embedding in a `CONFIG_VALIDATION_FAILED` error. +pub fn parse_regex_literal(raw: &str) -> Result { + let trimmed = raw.trim(); + if !trimmed.starts_with('/') { + return Err(format!( + "regex literal must start with '/', got '{raw}' (SPEC §12.3)" + )); + } + if trimmed.len() < 2 { + return Err(format!( + "regex literal '{raw}' is unterminated (missing closing '/')" + )); + } + + // Scan backward for the last '/' whose trailing characters are all valid + // flag characters. Bytes are safe because '/' is ASCII — a multi-byte + // codepoint never starts with 0x2F. + let bytes = trimmed.as_bytes(); + let mut end: Option = None; + for i in (1..bytes.len()).rev() { + if bytes[i] != b'/' { + continue; + } + let tail = &trimmed[i + 1..]; + if tail.chars().all(|c| matches!(c, 'i' | 'm' | 's')) { + end = Some(i); + break; + } + // If the tail looks like it's meant to be flag characters (all ASCII + // letters) but contains an unsupported one, give a specific error. + // This catches `/pat/gi`, `/pat/x`, etc. + if !tail.is_empty() && tail.chars().all(|c| c.is_ascii_alphabetic()) { + if tail.contains('g') { + return Err(format!( + "regex '{raw}' uses the 'g' flag, which is not meaningful \ + for boolean matching in ail — matching is match/no-match, \ + not iterative. Remove the flag. (SPEC §12.3)" + )); + } + let bad: String = tail + .chars() + .filter(|c| !matches!(c, 'i' | 'm' | 's')) + .collect(); + return Err(format!( + "regex '{raw}' uses unsupported flag(s): '{bad}'. Only 'i', 'm', 's' \ + are accepted as trailing flags. Use inline flag syntax like (?x) \ + inside the pattern for other modes. (SPEC §12.3)" + )); + } + // Otherwise, this '/' is inside the pattern — keep scanning backward. + } + + let end = end.ok_or_else(|| { + format!("regex literal '{raw}' is not terminated (missing closing '/') (SPEC §12.3)") + })?; + + let pattern_raw = &trimmed[1..end]; + let flags = &trimmed[end + 1..]; + + if pattern_raw.is_empty() { + return Err(format!( + "regex literal '{raw}' has an empty pattern (SPEC §12.3)" + )); + } + + // Unescape `\/` → `/`. Any other backslash sequence is left alone so the + // regex engine sees it verbatim (e.g. `\d`, `\b`, `\s`). + let pattern = unescape_forward_slashes(pattern_raw); + + let mut builder = RegexBuilder::new(&pattern); + for ch in flags.chars() { + match ch { + 'i' => { + builder.case_insensitive(true); + } + 'm' => { + builder.multi_line(true); + } + 's' => { + builder.dot_matches_new_line(true); + } + _ => unreachable!("flag chars restricted by scan loop"), + } + } + + let regex = builder + .build() + .map_err(|e| format!("regex '{raw}' failed to compile: {e} (SPEC §12.3)"))?; + + Ok(ParsedRegex { + source: trimmed.to_string(), + regex, + }) +} + +/// Replace `\/` with `/` in a pattern body. Other escape sequences are left +/// untouched so the regex engine sees them verbatim. +fn unescape_forward_slashes(s: &str) -> String { + // Fast path — most patterns don't contain `\/`. + if !s.contains("\\/") { + return s.to_string(); + } + let mut out = String::with_capacity(s.len()); + let mut chars = s.chars().peekable(); + while let Some(c) = chars.next() { + if c == '\\' && chars.peek() == Some(&'/') { + out.push('/'); + chars.next(); + } else { + out.push(c); + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_simple_literal() { + let r = parse_regex_literal("/warn|error/").unwrap(); + assert_eq!(r.source, "/warn|error/"); + assert!(r.regex.is_match("warning")); + assert!(r.regex.is_match("errors here")); + assert!(!r.regex.is_match("ok")); + } + + #[test] + fn parses_literal_with_i_flag() { + let r = parse_regex_literal("/pass/i").unwrap(); + assert!(r.regex.is_match("PASS")); + assert!(r.regex.is_match("passed")); + } + + #[test] + fn parses_literal_with_m_flag() { + let r = parse_regex_literal("/^foo/m").unwrap(); + assert!(r.regex.is_match("bar\nfoo")); + } + + #[test] + fn parses_literal_with_s_flag() { + let r = parse_regex_literal("/a.b/s").unwrap(); + assert!(r.regex.is_match("a\nb")); + } + + #[test] + fn parses_literal_with_multiple_flags() { + let r = parse_regex_literal("/^pass$/im").unwrap(); + assert!(r.regex.is_match("warning\nPASS")); + } + + #[test] + fn unanchored_by_default() { + let r = parse_regex_literal("/PASS/").unwrap(); + assert!(r.regex.is_match("tests PASSED")); + } + + #[test] + fn case_sensitive_by_default() { + let r = parse_regex_literal("/PASS/").unwrap(); + assert!(!r.regex.is_match("pass")); + } + + #[test] + fn anchors_work() { + let r = parse_regex_literal("/^PASS$/").unwrap(); + assert!(r.regex.is_match("PASS")); + assert!(!r.regex.is_match("PASSED")); + } + + #[test] + fn patterns_with_embedded_slashes() { + // The last `/` followed by no flags delimits — so `a/b` is pattern. + let r = parse_regex_literal("/a/b/").unwrap(); + assert!(r.regex.is_match("a/b")); + } + + #[test] + fn patterns_with_embedded_slashes_and_flag() { + let r = parse_regex_literal("/a/b/i").unwrap(); + assert!(r.regex.is_match("A/B")); + } + + #[test] + fn escaped_forward_slashes_work() { + let r = parse_regex_literal("/foo\\/bar/").unwrap(); + assert!(r.regex.is_match("foo/bar")); + } + + #[test] + fn backslash_d_works() { + let r = parse_regex_literal("/\\d{3}/").unwrap(); + assert!(r.regex.is_match("abc123def")); + assert!(!r.regex.is_match("abc12def")); + } + + #[test] + fn alternation_works() { + let r = parse_regex_literal("/LGTM|SHIP IT/").unwrap(); + assert!(r.regex.is_match("LGTM")); + assert!(r.regex.is_match("SHIP IT")); + assert!(!r.regex.is_match("REJECT")); + } + + // ── Error cases ────────────────────────────────────────────────────────── + + #[test] + fn rejects_missing_leading_slash() { + let err = parse_regex_literal("warn/").unwrap_err(); + assert!(err.contains("must start with '/'")); + } + + #[test] + fn rejects_g_flag_specifically() { + let err = parse_regex_literal("/warn/g").unwrap_err(); + assert!(err.contains("'g' flag")); + assert!(err.contains("not meaningful")); + } + + #[test] + fn rejects_gi_flags() { + let err = parse_regex_literal("/warn/gi").unwrap_err(); + assert!(err.contains("'g' flag")); + } + + #[test] + fn rejects_x_flag() { + let err = parse_regex_literal("/warn/x").unwrap_err(); + assert!(err.contains("unsupported")); + assert!(err.contains("'x'")); + } + + #[test] + fn rejects_empty_pattern() { + let err = parse_regex_literal("//").unwrap_err(); + assert!(err.contains("empty pattern")); + } + + #[test] + fn rejects_unterminated() { + // `/foo` — no closing slash. No alphabetic tail to trigger the + // unsupported-flags branch, so falls through to the unterminated path. + let err = parse_regex_literal("/foo").unwrap_err(); + assert!(err.contains("not terminated") || err.contains("unterminated")); + } + + #[test] + fn rejects_invalid_regex_syntax() { + // Unbalanced character class. + let err = parse_regex_literal("/[unclosed/").unwrap_err(); + assert!(err.contains("failed to compile")); + } + + #[test] + fn preserves_source() { + let r = parse_regex_literal("/warn|error/i").unwrap(); + assert_eq!(r.source, "/warn|error/i"); + } + + #[test] + fn trims_outer_whitespace() { + let r = parse_regex_literal(" /pass/i ").unwrap(); + assert_eq!(r.source, "/pass/i"); + } +} diff --git a/ail-core/src/executor/core.rs b/ail-core/src/executor/core.rs index 942c5be..35f7b4c 100644 --- a/ail-core/src/executor/core.rs +++ b/ail-core/src/executor/core.rs @@ -623,7 +623,13 @@ pub(super) fn execute_single_step( let mut matched_action = None; if let Some(branches) = &step.on_result { let last_entry = session.turn_log.entries().last().expect("just appended"); - if let Some(action) = evaluate_on_result(branches, last_entry, validated_input.as_ref()) { + if let Some(action) = evaluate_on_result( + branches, + session, + step.id.as_str(), + last_entry, + validated_input.as_ref(), + )? { matched_action = Some(action.clone()); } } @@ -1681,8 +1687,24 @@ fn execute_core_with_parallel( // Evaluate on_result against the merged response. let matched_action = if let Some(ref branches) = step.on_result { - let last_entry = session.turn_log.entries().last(); - last_entry.and_then(|e| evaluate_on_result(branches, e, None)) + // Clone the entry so we can release the immutable borrow on + // `session.turn_log` before passing `session` itself to the + // evaluator (which needs it for template resolution in + // `expression:` matchers). + let last_entry = session.turn_log.entries().last().cloned(); + match last_entry { + Some(e) => { + match evaluate_on_result(branches, session, &step_id, &e, None) { + Ok(action) => action, + Err(err) => { + observer.on_step_failed(&step_id, err.detail()); + *outcome_cell.borrow_mut() = Some(Err(err)); + return; + } + } + } + None => None, + } } else { None }; diff --git a/ail-core/src/executor/helpers/condition.rs b/ail-core/src/executor/helpers/condition.rs index 31b0854..cefdfca 100644 --- a/ail-core/src/executor/helpers/condition.rs +++ b/ail-core/src/executor/helpers/condition.rs @@ -6,7 +6,7 @@ #![allow(clippy::result_large_err)] -use crate::config::domain::{Condition, ConditionExpr, ConditionOp}; +use crate::config::domain::{Condition, ConditionExpr, ConditionOp, RegexCondition}; use crate::error::AilError; use crate::session::Session; use crate::template; @@ -28,9 +28,38 @@ pub fn evaluate_condition( Condition::Always => Ok(true), Condition::Never => Ok(false), Condition::Expression(expr) => evaluate_expression(expr, session, step_id), + Condition::Regex(reg) => evaluate_regex(reg, session, step_id), } } +fn evaluate_regex( + reg: &RegexCondition, + session: &Session, + step_id: &str, +) -> Result { + let lhs_resolved = + template::resolve(®.lhs, session).map_err(|e| AilError::ConditionInvalid { + detail: format!( + "Step '{step_id}' condition: failed to resolve left-hand side '{}': {}", + reg.lhs, + e.detail() + ), + context: None, + })?; + + let result = reg.regex.is_match(lhs_resolved.trim()); + + tracing::debug!( + step_id = %step_id, + lhs = %lhs_resolved, + regex = %reg.source, + result = %result, + "condition regex evaluated" + ); + + Ok(result) +} + fn evaluate_expression( expr: &ConditionExpr, session: &Session, @@ -207,6 +236,62 @@ mod tests { assert!(evaluate_condition(&Condition::Expression(expr), &session, "s").unwrap()); } + #[test] + fn matches_operator_regex() { + use crate::config::domain::RegexCondition; + let session = session_with_prompt_entry("test", "tests PASSED"); + let parsed = crate::config::validation::parse_regex_literal("/^tests\\s+PASS/").unwrap(); + let cond = Condition::Regex(RegexCondition { + lhs: "{{ step.test.response }}".to_string(), + regex: parsed.regex, + source: parsed.source, + }); + assert!(evaluate_condition(&cond, &session, "s").unwrap()); + } + + #[test] + fn matches_operator_case_insensitive_flag() { + use crate::config::domain::RegexCondition; + let session = session_with_prompt_entry("test", "WARNING: bad things"); + let parsed = crate::config::validation::parse_regex_literal("/warn/i").unwrap(); + let cond = Condition::Regex(RegexCondition { + lhs: "{{ step.test.response }}".to_string(), + regex: parsed.regex, + source: parsed.source, + }); + assert!(evaluate_condition(&cond, &session, "s").unwrap()); + } + + #[test] + fn matches_operator_no_match() { + use crate::config::domain::RegexCondition; + let session = session_with_prompt_entry("test", "all ok"); + let parsed = crate::config::validation::parse_regex_literal("/error/").unwrap(); + let cond = Condition::Regex(RegexCondition { + lhs: "{{ step.test.response }}".to_string(), + regex: parsed.regex, + source: parsed.source, + }); + assert!(!evaluate_condition(&cond, &session, "s").unwrap()); + } + + #[test] + fn matches_operator_unresolvable_template() { + use crate::config::domain::RegexCondition; + let session = make_session(vec![]); + let parsed = crate::config::validation::parse_regex_literal("/x/").unwrap(); + let cond = Condition::Regex(RegexCondition { + lhs: "{{ step.nonexistent.response }}".to_string(), + regex: parsed.regex, + source: parsed.source, + }); + let err = evaluate_condition(&cond, &session, "mycheck").unwrap_err(); + assert_eq!( + err.error_type(), + crate::error::error_types::CONDITION_INVALID + ); + } + #[test] fn unresolvable_template_produces_condition_invalid() { let session = make_session(vec![]); diff --git a/ail-core/src/executor/helpers/on_result.rs b/ail-core/src/executor/helpers/on_result.rs index d63acac..c745d77 100644 --- a/ail-core/src/executor/helpers/on_result.rs +++ b/ail-core/src/executor/helpers/on_result.rs @@ -1,19 +1,31 @@ //! on_result branch evaluation and tool policy construction. +#![allow(clippy::result_large_err)] + use crate::config::domain::{ExitCodeMatch, ResultAction, ResultMatcher}; +use crate::error::AilError; use crate::runner::ToolPermissionPolicy; -use crate::session::TurnEntry; +use crate::session::{Session, TurnEntry}; + +use super::condition::evaluate_condition; /// Evaluate `on_result` branches against the most recent `TurnEntry`. /// Returns the action of the first matching branch, or `None` if no branch matches. /// /// `validated_input` is the parsed JSON from `input_schema` validation (SPEC §26.2). /// When present, `ResultMatcher::Field` can match against named fields in the input JSON. +/// +/// An `Err` return indicates the `expression:` matcher failed to evaluate — +/// typically an unresolvable template variable in the expression's LHS +/// (SPEC §11 contract). The pipeline aborts rather than silently treating the +/// branch as non-matching. pub(in crate::executor) fn evaluate_on_result( branches: &[crate::config::domain::ResultBranch], + session: &Session, + step_id: &str, entry: &TurnEntry, validated_input: Option<&serde_json::Value>, -) -> Option { +) -> Result, AilError> { for branch in branches { let matched = match &branch.matcher { ResultMatcher::Contains(text) => { @@ -36,14 +48,20 @@ pub(in crate::executor) fn evaluate_on_result( .and_then(|json| json.get(name)) .is_some_and(|val| val == equals) } + ResultMatcher::Expression { condition, .. } => { + // SPEC §5.4 `expression:` matcher — delegate to the shared + // condition evaluator so `condition:` and `expression:` stay in + // sync grammatically and semantically. + evaluate_condition(condition, session, step_id)? + } ResultMatcher::Always => true, }; if matched { - return Some(branch.action.clone()); + return Ok(Some(branch.action.clone())); } } - None + Ok(None) } /// Build a `ToolPermissionPolicy` from an optional `ToolPolicy` domain value. diff --git a/ail-core/src/materialize.rs b/ail-core/src/materialize.rs index a758d4c..974fce4 100644 --- a/ail-core/src/materialize.rs +++ b/ail-core/src/materialize.rs @@ -318,6 +318,9 @@ pub fn materialize(pipeline: &Pipeline) -> String { } => { format!("field: {name}, equals: {equals}") } + ResultMatcher::Expression { source, .. } => { + format!("expression: \"{}\"", yaml_quote(source)) + } ResultMatcher::Always => "always: true".to_string(), }; let action = match &branch.action { @@ -528,6 +531,9 @@ fn serialize_step(out: &mut String, step: &Step, indent: &str, origin_comment: O } => { format!("field: {name}, equals: {equals}") } + ResultMatcher::Expression { source, .. } => { + format!("expression: \"{}\"", yaml_quote(source)) + } ResultMatcher::Always => "always: true".to_string(), }; let action = match &branch.action { diff --git a/ail-core/tests/spec/s05_3_on_result.rs b/ail-core/tests/spec/s05_3_on_result.rs index dcbca63..43aba61 100644 --- a/ail-core/tests/spec/s05_3_on_result.rs +++ b/ail-core/tests/spec/s05_3_on_result.rs @@ -5,6 +5,7 @@ use ail_core::executor::{ execute, execute_with_control, ExecuteOutcome, ExecutionControl, ExecutorEvent, }; use ail_core::runner::stub::StubRunner; +use ail_core::session::Session; use ail_core::test_helpers::{make_session, prompt_step}; use std::collections::HashSet; use std::sync::mpsc; @@ -405,3 +406,197 @@ fn on_result_break_skips_remaining_steps() { std::env::set_current_dir(orig).unwrap(); } + +// ───────────────────────────────────────────────────────────────────────────── +// SPEC §5.4 — `expression:` and `matches:` matchers +// ───────────────────────────────────────────────────────────────────────────── + +/// SPEC §5.4 — `expression:` matcher fires on a §12.2-grammar comparison +/// against a previous step's field other than `response`/`exit_code`. +#[test] +fn on_result_expression_matcher_fires_on_stderr() { + let _cwd_guard = crate::spec::CWD_LOCK.lock().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let orig = std::env::current_dir().unwrap(); + std::env::set_current_dir(tmp.path()).unwrap(); + + // Step 1 writes "rate limit" to stderr and exits 0. We can't assert on + // the `response` directly via `contains:` because this is a context + // step (no response) — `expression:` is the only way to branch on + // stderr text from an on_result. + let yaml = r#" +version: "1" +pipeline: + - id: upstream + context: + shell: "echo 'rate limit exceeded' 1>&2; true" + on_result: + - expression: "{{ step.upstream.stderr }} contains 'rate limit'" + action: abort_pipeline + - always: true + action: continue + - id: downstream + prompt: "proceed" +"#; + let pipeline_path = tmp.path().join(".ail.yaml"); + std::fs::write(&pipeline_path, yaml).unwrap(); + let pipeline = ail_core::config::load(&pipeline_path).unwrap(); + let mut session = Session::new(pipeline, "p".to_string()); + + let result = execute(&mut session, &StubRunner::new("x")); + let err = result.expect_err("expression: branch should have aborted"); + assert_eq!( + err.error_type(), + ail_core::error::error_types::PIPELINE_ABORTED, + "should have aborted via expression: + action: abort_pipeline" + ); + + std::env::set_current_dir(orig).unwrap(); +} + +/// SPEC §5.4 — `matches:` named matcher is shorthand for the expression +/// form. Regex is compiled at parse time; runtime just applies it. +#[test] +fn on_result_matches_named_matcher_shorthand() { + let _cwd_guard = crate::spec::CWD_LOCK.lock().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let orig = std::env::current_dir().unwrap(); + std::env::set_current_dir(tmp.path()).unwrap(); + + let yaml = r#" +version: "1" +pipeline: + - id: review + prompt: "review" + on_result: + - matches: "/LGTM|SHIP IT/" + action: continue + - always: true + action: abort_pipeline + - id: ship + prompt: "ship" +"#; + let pipeline_path = tmp.path().join(".ail.yaml"); + std::fs::write(&pipeline_path, yaml).unwrap(); + let pipeline = ail_core::config::load(&pipeline_path).unwrap(); + let mut session = Session::new(pipeline, "p".to_string()); + + // StubRunner's response contains LGTM → first branch matches → pipeline continues. + let result = execute(&mut session, &StubRunner::new("Looks great — LGTM")); + assert!(result.is_ok(), "expected Ok, got: {result:?}"); + assert_eq!( + session.turn_log.entries().len(), + 2, + "both review and ship should run" + ); + + std::env::set_current_dir(orig).unwrap(); +} + +/// SPEC §5.4 — `expression:` with the regex `matches` operator (the full +/// confidence-gating shape users will hit once native LLM runners surface +/// numeric signals). +#[test] +fn on_result_expression_with_matches_operator() { + let _cwd_guard = crate::spec::CWD_LOCK.lock().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let orig = std::env::current_dir().unwrap(); + std::env::set_current_dir(tmp.path()).unwrap(); + + let yaml = r#" +version: "1" +pipeline: + - id: lint + context: + shell: "echo 'warning: unused import'; true" + on_result: + - expression: "{{ step.lint.stdout }} matches /warning|error/i" + action: abort_pipeline + - always: true + action: continue + - id: next + prompt: "continue" +"#; + let pipeline_path = tmp.path().join(".ail.yaml"); + std::fs::write(&pipeline_path, yaml).unwrap(); + let pipeline = ail_core::config::load(&pipeline_path).unwrap(); + let mut session = Session::new(pipeline, "p".to_string()); + + let result = execute(&mut session, &StubRunner::new("x")); + assert_eq!( + result.unwrap_err().error_type(), + ail_core::error::error_types::PIPELINE_ABORTED, + "expression: with matches operator should fire abort_pipeline" + ); + + std::env::set_current_dir(orig).unwrap(); +} + +/// SPEC §5.4 / §11 — expression: LHS that fails to resolve aborts the +/// pipeline rather than silently failing to match. +#[test] +fn on_result_expression_unresolvable_template_aborts() { + let _cwd_guard = crate::spec::CWD_LOCK.lock().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let orig = std::env::current_dir().unwrap(); + std::env::set_current_dir(tmp.path()).unwrap(); + + let yaml = r#" +version: "1" +pipeline: + - id: s1 + prompt: "x" + on_result: + - expression: "{{ step.nonexistent.response }} == 'anything'" + action: continue +"#; + let pipeline_path = tmp.path().join(".ail.yaml"); + std::fs::write(&pipeline_path, yaml).unwrap(); + let pipeline = ail_core::config::load(&pipeline_path).unwrap(); + let mut session = Session::new(pipeline, "p".to_string()); + + let result = execute(&mut session, &StubRunner::new("x")); + let err = result.expect_err("unresolvable template in expression: should error"); + assert_eq!( + err.error_type(), + ail_core::error::error_types::CONDITION_INVALID, + "expression: template failures route through CONDITION_INVALID (same as condition:)" + ); + + std::env::set_current_dir(orig).unwrap(); +} + +/// SPEC §5.4 — multiple matchers on the same branch (e.g. contains: AND +/// expression:) is a parse error. +#[test] +fn on_result_multiple_matchers_rejected_at_parse() { + let _cwd_guard = crate::spec::CWD_LOCK.lock().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let orig = std::env::current_dir().unwrap(); + std::env::set_current_dir(tmp.path()).unwrap(); + + let yaml = r#" +version: "1" +pipeline: + - id: s1 + prompt: "x" + on_result: + - contains: "LGTM" + expression: "{{ step.s1.response }} == 'x'" + action: continue +"#; + let pipeline_path = tmp.path().join(".ail.yaml"); + std::fs::write(&pipeline_path, yaml).unwrap(); + let err = ail_core::config::load(&pipeline_path).unwrap_err(); + assert_eq!( + err.error_type(), + ail_core::error::error_types::CONFIG_VALIDATION_FAILED + ); + assert!( + err.detail().contains("exactly one matcher"), + "expected matcher-count error, got: {}", + err.detail() + ); + + std::env::set_current_dir(orig).unwrap(); +} diff --git a/ail-core/tests/spec/s12_step_conditions.rs b/ail-core/tests/spec/s12_step_conditions.rs index 22bc9d1..dbaf30d 100644 --- a/ail-core/tests/spec/s12_step_conditions.rs +++ b/ail-core/tests/spec/s12_step_conditions.rs @@ -387,6 +387,215 @@ fn condition_expression_starts_with_operator() { std::env::set_current_dir(orig).unwrap(); } +/// SPEC §12.2/§12.3 — `matches` operator with basic regex, unanchored and +/// case-sensitive by default. The RegexCondition is hand-built here (parser +/// coverage lives in the regex_literal unit tests); this exercises the +/// evaluator end-to-end. +#[test] +fn condition_matches_operator_basic() { + use ail_core::config::domain::RegexCondition; + + let _cwd_guard = crate::spec::CWD_LOCK.lock().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let orig = std::env::current_dir().unwrap(); + std::env::set_current_dir(tmp.path()).unwrap(); + + let regex = regex::Regex::new(r"error|warning").unwrap(); + let mut session = make_session(vec![ + prompt_step_with_condition("lint", "Run lint", None), + prompt_step_with_condition( + "triage", + "Triage issues", + Some(Condition::Regex(RegexCondition { + lhs: "{{ step.lint.response }}".to_string(), + regex, + source: "/error|warning/".to_string(), + })), + ), + ]); + + let result = execute(&mut session, &StubRunner::new("3 warnings, 0 errors")); + assert!(result.is_ok()); + assert_eq!( + session.turn_log.entries().len(), + 2, + "triage should run when lint output matches" + ); + + std::env::set_current_dir(orig).unwrap(); +} + +/// SPEC §12.3 — `matches` operator is case-sensitive by default; the `i` +/// flag must be set explicitly to toggle case-insensitive matching. +#[test] +fn condition_matches_operator_case_sensitive_by_default() { + use ail_core::config::domain::RegexCondition; + + let _cwd_guard = crate::spec::CWD_LOCK.lock().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let orig = std::env::current_dir().unwrap(); + std::env::set_current_dir(tmp.path()).unwrap(); + + // No `i` flag — pattern `error` should NOT match "ERROR". + let regex = regex::Regex::new(r"error").unwrap(); + let mut session = make_session(vec![ + prompt_step_with_condition("check", "Check", None), + prompt_step_with_condition( + "act", + "Act on error", + Some(Condition::Regex(RegexCondition { + lhs: "{{ step.check.response }}".to_string(), + regex, + source: "/error/".to_string(), + })), + ), + ]); + + let result = execute(&mut session, &StubRunner::new("FATAL ERROR: boom")); + assert!(result.is_ok()); + assert_eq!( + session.turn_log.entries().len(), + 1, + "act step should be skipped — case-sensitive pattern does not match uppercase" + ); + + std::env::set_current_dir(orig).unwrap(); +} + +/// SPEC §12.3 — `matches` with `i` flag enables case-insensitive matching. +#[test] +fn condition_matches_operator_i_flag() { + use ail_core::config::domain::RegexCondition; + + let _cwd_guard = crate::spec::CWD_LOCK.lock().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let orig = std::env::current_dir().unwrap(); + std::env::set_current_dir(tmp.path()).unwrap(); + + let regex = regex::RegexBuilder::new("error") + .case_insensitive(true) + .build() + .unwrap(); + let mut session = make_session(vec![ + prompt_step_with_condition("check", "Check", None), + prompt_step_with_condition( + "act", + "Act on error", + Some(Condition::Regex(RegexCondition { + lhs: "{{ step.check.response }}".to_string(), + regex, + source: "/error/i".to_string(), + })), + ), + ]); + + let result = execute(&mut session, &StubRunner::new("FATAL ERROR: boom")); + assert!(result.is_ok()); + assert_eq!( + session.turn_log.entries().len(), + 2, + "case-insensitive match should succeed" + ); + + std::env::set_current_dir(orig).unwrap(); +} + +/// SPEC §12.3 — `matches` parsing via the condition expression grammar. +/// Covers the full path: raw YAML expression → condition parser → +/// RegexCondition → evaluator. +#[test] +fn condition_matches_operator_via_parser() { + let _cwd_guard = crate::spec::CWD_LOCK.lock().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let orig = std::env::current_dir().unwrap(); + std::env::set_current_dir(tmp.path()).unwrap(); + + let yaml = r#" +version: "1" +pipeline: + - id: lint + prompt: "run lint" + - id: triage + prompt: "triage" + condition: "{{ step.lint.response }} matches /ERROR|FAIL/i" +"#; + let pipeline_path = tmp.path().join(".ail.yaml"); + std::fs::write(&pipeline_path, yaml).unwrap(); + let pipeline = ail_core::config::load(&pipeline_path).unwrap(); + let mut session = Session::new(pipeline, "p".to_string()); + + let result = execute( + &mut session, + &StubRunner::new("Process failed with FATAL error"), + ); + assert!(result.is_ok()); + assert_eq!( + session.turn_log.entries().len(), + 2, + "triage should run when matches fires (case-insensitive)" + ); + + std::env::set_current_dir(orig).unwrap(); +} + +/// SPEC §12.3 — invalid regex at parse time fails pipeline load. +#[test] +fn condition_matches_invalid_regex_fails_at_parse() { + let _cwd_guard = crate::spec::CWD_LOCK.lock().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let orig = std::env::current_dir().unwrap(); + std::env::set_current_dir(tmp.path()).unwrap(); + + let yaml = r#" +version: "1" +pipeline: + - id: broken + prompt: "x" + condition: "{{ step.x.response }} matches /[unclosed/" +"#; + let pipeline_path = tmp.path().join(".ail.yaml"); + std::fs::write(&pipeline_path, yaml).unwrap(); + let err = ail_core::config::load(&pipeline_path).unwrap_err(); + assert_eq!( + err.error_type(), + ail_core::error::error_types::CONFIG_VALIDATION_FAILED + ); + assert!( + err.detail().contains("failed to compile"), + "detail should explain regex compile failure: {}", + err.detail() + ); + + std::env::set_current_dir(orig).unwrap(); +} + +/// SPEC §12.3 — `g` flag is explicitly rejected with a specific message. +#[test] +fn condition_matches_g_flag_rejected() { + let _cwd_guard = crate::spec::CWD_LOCK.lock().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let orig = std::env::current_dir().unwrap(); + std::env::set_current_dir(tmp.path()).unwrap(); + + let yaml = r#" +version: "1" +pipeline: + - id: broken + prompt: "x" + condition: "{{ step.x.response }} matches /warn/gi" +"#; + let pipeline_path = tmp.path().join(".ail.yaml"); + std::fs::write(&pipeline_path, yaml).unwrap(); + let err = ail_core::config::load(&pipeline_path).unwrap_err(); + assert!( + err.detail().contains("'g' flag"), + "detail should call out the g flag specifically: {}", + err.detail() + ); + + std::env::set_current_dir(orig).unwrap(); +} + /// SPEC §12.2 — expression condition: ends_with operator #[test] fn condition_expression_ends_with_operator() {