Skip to main content

dataflow_rs/engine/
authoring.rs

1//! Authoring-time validation: checking a workflow definition *before* it
2//! reaches [`EngineBuilder::build`](crate::EngineBuilder::build).
3//!
4//! The engine's own enforcement — parse, [`Workflow::validate`], and
5//! `LoopConfig::validate` — all fires when the *engine* is built. For a host
6//! that stores definitions and builds one engine over many of them, that is the
7//! wrong time (one bad row aborts the whole build, at reload, for every
8//! workflow in the process), the wrong shape (one stringly error, so an
9//! authoring API cannot point a 400 at `tasks[1].tasks[0].id`), and the wrong
10//! cardinality (fail-fast, so the author fixes one violation per round trip).
11//!
12//! [`Workflow::validate_authored`] answers all three, and carries one
13//! guarantee:
14//!
15//! > It returns empty **if and only if** the JSON parses into a [`Workflow`]
16//! > and that workflow validates.
17//!
18//! That biconditional is true *by construction*, not by keeping a rule list in
19//! sync — see the stages below.
20
21use crate::engine::compiler::TEMPLATE_KEY_ESCAPE;
22use crate::engine::functions::config::{BuiltinKind, builtin_function_kind, can_dispatch_in};
23use crate::engine::functions::{BoxedFunctionHandler, FunctionConfig, TemplateCompiler};
24use crate::engine::secrets::{SECRET_OPERATOR, Secrets};
25use crate::engine::steps::{StepKind, walk_authored_steps};
26use crate::engine::task::HaltOn;
27use crate::engine::workflow::Workflow;
28use serde::Deserialize;
29use serde_json::Value;
30use std::collections::HashMap;
31use std::fmt;
32
33/// One problem with a workflow definition.
34///
35/// Shared by [`Workflow::validate_authored`] and, from the registry side,
36/// `EngineBuilder::check_workflow`. Both fill what they genuinely know: a
37/// definition check always has an authored coordinate and knows the step id
38/// when the problem concerns a step; a registry check always has a task id and
39/// reports a path relative to that task.
40#[derive(Debug, Clone, PartialEq, Eq)]
41pub struct WorkflowIssue {
42    /// Stable machine-readable classification.
43    pub code: IssueCode,
44    /// Human-readable explanation. Not stable — branch on [`Self::code`].
45    pub message: String,
46    /// Where the problem is. From [`Workflow::validate_authored`] this is the
47    /// coordinate the author typed, rooted at the workflow document:
48    /// `tasks[1].tasks[0].id`.
49    pub path: Option<String>,
50    /// The step this concerns, when it concerns one. Step ids are unique across
51    /// tasks *and* groups, so this identifies a step on its own.
52    pub task_id: Option<String>,
53}
54
55impl WorkflowIssue {
56    /// When this issue bites — [`IssueCode::severity`] of [`Self::code`].
57    ///
58    /// Screening before a build is the point: everything but
59    /// [`Severity::Advisory`] means the workflow will not do what it says.
60    pub fn severity(&self) -> Severity {
61        self.code.severity()
62    }
63
64    fn at(code: IssueCode, path: impl Into<String>, message: impl Into<String>) -> Self {
65        Self {
66            code,
67            message: message.into(),
68            path: Some(path.into()),
69            task_id: None,
70        }
71    }
72
73    fn with_step(mut self, id: Option<&str>) -> Self {
74        self.task_id = id.map(str::to_string);
75        self
76    }
77}
78
79impl fmt::Display for WorkflowIssue {
80    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
81        match &self.path {
82            Some(path) => write!(f, "{path}: {} [{}]", self.message, self.code.as_str()),
83            None => write!(f, "{} [{}]", self.message, self.code.as_str()),
84        }
85    }
86}
87
88/// What a check found wrong with a workflow definition.
89///
90/// Not every code is a refusal: [`Self::severity`] says whether the definition
91/// is rejected, builds and then fails every message, or merely carries a shape
92/// worth an author's attention. Ask it rather than keeping a list — see
93/// [`Severity`].
94///
95/// `#[non_exhaustive]`: a later minor may add a rule, and a host matching on
96/// the codes it cares about should keep compiling. Use [`Self::as_str`] to
97/// serialize.
98#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
99#[non_exhaustive]
100pub enum IssueCode {
101    /// `id` is missing or empty.
102    EmptyWorkflowId,
103    /// `name` is missing or empty.
104    EmptyWorkflowName,
105    /// `tasks` is missing, not an array, or empty.
106    NoTasks,
107    /// A step carries no `id`.
108    MissingStepId,
109    /// Two steps share an id. Groups share the task id namespace.
110    DuplicateStepId,
111    /// A group's `tasks` is not a non-empty array.
112    EmptyGroup,
113    /// A group is nested at or beyond
114    /// [`MAX_GROUP_DEPTH`](crate::engine::steps::MAX_GROUP_DEPTH).
115    GroupTooDeep,
116    /// A task carries no `function`.
117    MissingFunction,
118    /// `function` is not an object, or its `name` is missing or empty.
119    InvalidFunctionName,
120    /// `terminal` is present but not a boolean.
121    InvalidTerminal,
122    /// A `validation` task whose failure cannot stop anything: it is not
123    /// `terminal`, has no `halt_on`, and every task after it in the workflow is
124    /// unguarded. A failing rule returns `400`, which `continue_on_error` does
125    /// not cover, so the pipeline proceeds as if it had passed.
126    ///
127    /// [`Severity::Advisory`] — reported by
128    /// [`crate::EngineBuilder::check_workflow`] and never by
129    /// [`crate::EngineBuilder::build`]. Validating purely to record errors is a
130    /// legitimate shape; this only says the assertion is not acting as a gate.
131    UnguardedValidation,
132    /// `halt_on` is present and is not one of the accepted strings, or is
133    /// present on a *group* — a group has no outcome of its own, so the
134    /// executor could not honour it.
135    InvalidHaltOn,
136    /// A task group carries `continue_on_error`, which the engine drops. Error
137    /// handling is per task and per workflow; a group only gates a span.
138    ///
139    /// [`Severity::Advisory`] — reported by
140    /// [`crate::EngineBuilder::check_workflow`] and never by
141    /// [`crate::EngineBuilder::build`]. Unlike [`Self::InvalidHaltOn`], which
142    /// the parser refuses outright, this key is real at the other two levels
143    /// and may already sit on a host's group nodes; refusing it would abort
144    /// every workflow in the build.
145    GroupContinueOnError,
146    /// `loop.increment` is below 1 — the counter would never reach `max`.
147    LoopIncrementTooSmall,
148    /// `loop.max` is not greater than `loop.init` — no sweep could ever run.
149    LoopBoundEmpty,
150    /// `loop.counter` is not a non-empty dotted path.
151    LoopCounterInvalid,
152    /// No handler will dispatch this function name, and it is not a built-in.
153    /// Usually a typo or a handler the host forgot to register.
154    UnknownFunction,
155    /// The name *is* a built-in, but one that ships as a config schema only
156    /// (`http_call`, `enrich`, `publish_kafka`) and no handler is registered
157    /// under it. The workflow builds cleanly and then fails every message.
158    MissingHandler,
159    /// A custom task's `input` does not deserialize into its handler's declared
160    /// `Input` type.
161    InputParse,
162    /// A `Template` field of a custom task's input does not compile.
163    TemplateCompile,
164    /// An expression reads `{"secret": "name"}` and no secret of that name is
165    /// declared on the engine (`EngineBuilder::with_secrets`). Only literal
166    /// names are checked; a dynamic name fails at evaluation instead.
167    UnknownSecret,
168    /// An expression whose result the engine writes to the message or emits to
169    /// a log reads a secret — a `map` mapping, or a `log` message or field. The
170    /// store exists so a value is never recorded; an expression that would
171    /// record it is refused outright, derived or not. Compute derived values in
172    /// a custom handler.
173    SecretInMessageWrite,
174    /// The store passed to
175    /// [`EngineBuilder::with_secrets`](crate::EngineBuilder::with_secrets) is
176    /// not a JSON object, so no name resolves and
177    /// [`EngineBuilder::build`](crate::EngineBuilder::build) will fail.
178    /// Reported by
179    /// [`EngineBuilder::check_workflow`](crate::EngineBuilder::check_workflow)
180    /// *instead of* the [`Self::UnknownSecret`] issues every literal name would
181    /// otherwise produce — the workflow is not what is wrong.
182    ///
183    /// [`Severity::Rejected`], but the odd one out: quarantining the workflow
184    /// will not make the build succeed, because the store is what is broken.
185    InvalidSecretStore,
186    /// Two keys in one template object collapse to the same name once the
187    /// template-key escape is stripped — `{"$a": 1, "a": 2}` emits `a` twice.
188    /// The context is a `Vec` of pairs, so both survive: a later read sees only
189    /// the first while serialization emits both. Always a bug, so
190    /// [`crate::EngineBuilder::build`] refuses it.
191    DuplicateTemplateKey,
192    /// A template key carries the escape prefix, so it is emitted with one
193    /// prefix stripped: `$type` emits `type`. [`Severity::Advisory`] — reported
194    /// by [`crate::Engine::check_workflow`] and never by
195    /// [`crate::EngineBuilder::build`].
196    ///
197    /// Exists for migration. The escape strips uniformly from every template
198    /// key, so a workflow written before 3.9 that emits genuinely `$`-prefixed
199    /// keys — MongoDB's `$set`/`$oid`, JSON Schema's `$schema`/`$ref` — changes
200    /// what it produces, silently. This lists every one so the audit is
201    /// mechanical rather than archaeological.
202    EscapedTemplateKey,
203    /// The document does not deserialize into a [`Workflow`]. Carries the
204    /// parser's own message, which names the offending field and type.
205    ParseFailed,
206    /// The document parses but [`Workflow::validate`] rejects it. A backstop:
207    /// reaching this means a rule exists that the checks above do not model.
208    ValidateFailed,
209}
210
211impl IssueCode {
212    /// The stable string form, for serializing into an API response.
213    pub fn as_str(&self) -> &'static str {
214        match self {
215            Self::EmptyWorkflowId => "EMPTY_WORKFLOW_ID",
216            Self::EmptyWorkflowName => "EMPTY_WORKFLOW_NAME",
217            Self::NoTasks => "NO_TASKS",
218            Self::MissingStepId => "MISSING_STEP_ID",
219            Self::DuplicateStepId => "DUPLICATE_STEP_ID",
220            Self::EmptyGroup => "EMPTY_GROUP",
221            Self::GroupTooDeep => "GROUP_TOO_DEEP",
222            Self::MissingFunction => "MISSING_FUNCTION",
223            Self::InvalidFunctionName => "INVALID_FUNCTION_NAME",
224            Self::InvalidTerminal => "INVALID_TERMINAL",
225            Self::InvalidHaltOn => "INVALID_HALT_ON",
226            Self::GroupContinueOnError => "GROUP_CONTINUE_ON_ERROR",
227            Self::UnguardedValidation => "UNGUARDED_VALIDATION",
228            Self::LoopIncrementTooSmall => "LOOP_INCREMENT_TOO_SMALL",
229            Self::LoopBoundEmpty => "LOOP_BOUND_EMPTY",
230            Self::LoopCounterInvalid => "LOOP_COUNTER_INVALID",
231            Self::UnknownFunction => "UNKNOWN_FUNCTION",
232            Self::MissingHandler => "MISSING_HANDLER",
233            Self::InputParse => "INPUT_PARSE",
234            Self::TemplateCompile => "TEMPLATE_COMPILE",
235            Self::UnknownSecret => "UNKNOWN_SECRET",
236            Self::SecretInMessageWrite => "SECRET_IN_MESSAGE_WRITE",
237            Self::InvalidSecretStore => "INVALID_SECRET_STORE",
238            Self::DuplicateTemplateKey => "DUPLICATE_TEMPLATE_KEY",
239            Self::EscapedTemplateKey => "ESCAPED_TEMPLATE_KEY",
240            Self::ParseFailed => "PARSE_FAILED",
241            Self::ValidateFailed => "VALIDATE_FAILED",
242        }
243    }
244
245    /// When an issue carrying this code bites — see [`Severity`].
246    ///
247    /// The match below has **no wildcard arm**, and must never gain one. That
248    /// is the whole mechanism: a code added in a later minor stops this
249    /// compiling until it is deliberately classified, so a new advisory code
250    /// cannot arrive as a silent `Rejected` in a host's pre-build screen — the
251    /// direction that turns a screen into a quarantine.
252    pub fn severity(self) -> Severity {
253        match self {
254            // Reported for an author's attention. `build` accepts all three
255            // and the workflow runs exactly as written.
256            Self::UnguardedValidation | Self::GroupContinueOnError | Self::EscapedTemplateKey => {
257                Severity::Advisory
258            }
259
260            // Builds cleanly, then fails every message. The config parses into
261            // a typed built-in variant, so `precompile_custom_inputs` never
262            // looks at it — which is why the builder cannot catch this one.
263            Self::MissingHandler => Severity::Defect,
264
265            // Refused by `refuse_authoring_issues`, before compilation — and
266            // `InvalidSecretStore` earlier still, by `build` itself.
267            Self::UnknownSecret
268            | Self::SecretInMessageWrite
269            | Self::DuplicateTemplateKey
270            | Self::InvalidSecretStore => Severity::Rejected,
271
272            // Refused by `precompile_custom_inputs`.
273            Self::UnknownFunction | Self::InputParse | Self::TemplateCompile => Severity::Rejected,
274
275            // Never parses into a `Workflow`, or fails `Workflow::validate`
276            // when `compile_workflows` runs it.
277            Self::EmptyWorkflowId
278            | Self::EmptyWorkflowName
279            | Self::NoTasks
280            | Self::MissingStepId
281            | Self::DuplicateStepId
282            | Self::EmptyGroup
283            | Self::GroupTooDeep
284            | Self::MissingFunction
285            | Self::InvalidFunctionName
286            | Self::InvalidTerminal
287            | Self::InvalidHaltOn
288            | Self::LoopIncrementTooSmall
289            | Self::LoopBoundEmpty
290            | Self::LoopCounterInvalid
291            | Self::ParseFailed
292            | Self::ValidateFailed => Severity::Rejected,
293        }
294    }
295}
296
297impl fmt::Display for IssueCode {
298    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
299        f.write_str(self.as_str())
300    }
301}
302
303/// When an issue bites: at [`build`](crate::EngineBuilder::build), at the first
304/// message, or never.
305///
306/// This is what a host screening definitions *before* building needs in order
307/// to quarantine one workflow rather than fail the whole engine. Not every code
308/// `check_workflow` reports is a refusal, and the set that is has grown apart
309/// from the set that is reported — [`Self::Advisory`] is the only class such a
310/// host may safely ignore.
311///
312/// Unlike [`IssueCode`], this is **not** `#[non_exhaustive]`. The axis is
313/// *when* a definition goes wrong, and build time / first message / never
314/// exhausts that by construction, so a host may `match` it and stay correct.
315#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
316pub enum Severity {
317    /// The definition never runs: [`build`](crate::EngineBuilder::build)
318    /// refuses it, or the parser rejected it before a [`Workflow`] existed.
319    Rejected,
320    /// It builds cleanly and then fails every message it processes — a real
321    /// defect, but one the builder cannot catch. Only
322    /// [`IssueCode::MissingHandler`] today.
323    Defect,
324    /// It loads and runs. Names a shape worth an author's attention, nothing
325    /// more.
326    Advisory,
327}
328
329impl Severity {
330    /// The stable string form, for serializing into an API response.
331    pub fn as_str(&self) -> &'static str {
332        match self {
333            Self::Rejected => "REJECTED",
334            Self::Defect => "DEFECT",
335            Self::Advisory => "ADVISORY",
336        }
337    }
338}
339
340impl fmt::Display for Severity {
341    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
342        f.write_str(self.as_str())
343    }
344}
345
346impl Workflow {
347    /// Check authored workflow JSON without building an engine.
348    ///
349    /// Returns empty **if and only if** the JSON parses into a [`Workflow`] and
350    /// that workflow validates.
351    ///
352    /// That is the *shape* question, and it is the whole of it. It is not the
353    /// same as "this engine can run it": [`EngineBuilder::build`](crate::EngineBuilder::build)
354    /// also resolves every task to a handler and parses custom inputs, so a
355    /// structurally perfect definition naming an unregistered function still
356    /// aborts a build. [`Engine::check_workflow`](crate::Engine::check_workflow)
357    /// answers that half; run both.
358    ///
359    /// # How the guarantee holds
360    ///
361    /// Three stages. A structural walk collects *every* semantic violation with
362    /// the coordinate the author typed; if it finds none, the document is then
363    /// actually parsed and validated, and either failure is reported as one
364    /// further issue. The second and third stages are what make the promise
365    /// true by construction: the crate's serde schema is far larger than any
366    /// rule list — a `"priority": "high"` or a `map` task missing `mappings`
367    /// breaks no *semantic* rule and still cannot load — and mirroring it here
368    /// would recreate the very drift this API exists to remove.
369    ///
370    /// # Example
371    ///
372    /// ```
373    /// use dataflow_rs::{IssueCode, Workflow};
374    /// use serde_json::json;
375    ///
376    /// let broken = json!({
377    ///     "id": "w", "name": "w", "priority": 0,
378    ///     "tasks": [
379    ///         {"id": "dup", "name": "a", "function": {"name": "map", "input": {"mappings": []}}},
380    ///         {"id": "dup", "name": "b", "function": {"name": "map", "input": {"mappings": []}}}
381    ///     ]
382    /// });
383    ///
384    /// let issues = Workflow::validate_authored(&broken);
385    /// assert_eq!(issues[0].code, IssueCode::DuplicateStepId);
386    /// assert_eq!(issues[0].path.as_deref(), Some("tasks[1].id"));
387    /// assert_eq!(issues[0].task_id.as_deref(), Some("dup"));
388    /// ```
389    ///
390    /// Every problem is reported, not just the first:
391    ///
392    /// ```
393    /// # use dataflow_rs::{IssueCode, Workflow};
394    /// # use serde_json::json;
395    /// let issues = Workflow::validate_authored(&json!({
396    ///     "id": "", "name": "w", "tasks": [{"id": "t", "name": "t"}]
397    /// }));
398    ///
399    /// let codes: Vec<IssueCode> = issues.iter().map(|i| i.code).collect();
400    /// assert!(codes.contains(&IssueCode::EmptyWorkflowId));
401    /// assert!(codes.contains(&IssueCode::MissingFunction));
402    /// ```
403    pub fn validate_authored(json: &Value) -> Vec<WorkflowIssue> {
404        let mut issues = check_shape(json);
405        if !issues.is_empty() {
406            return issues;
407        }
408
409        // Stage 2 — the schema is much wider than the rules above, and
410        // enumerating it here would be the mirror this API exists to delete.
411        let workflow: Self = match serde_json::from_value(json.clone()) {
412            Ok(w) => w,
413            Err(err) => {
414                issues.push(WorkflowIssue {
415                    code: IssueCode::ParseFailed,
416                    message: err.to_string(),
417                    path: None,
418                    task_id: None,
419                });
420                return issues;
421            }
422        };
423
424        // Stage 3 — a backstop. Reaching this means `check_shape` does not
425        // model some rule `validate` enforces; the caller still gets a correct
426        // answer, and the test suite is what keeps this unreachable.
427        if let Err(err) = workflow.validate() {
428            issues.push(WorkflowIssue {
429                code: IssueCode::ValidateFailed,
430                message: err.to_string(),
431                path: None,
432                task_id: None,
433            });
434        }
435        issues
436    }
437}
438
439/// Check a parsed workflow against a handler registry.
440///
441/// Shared by [`crate::EngineBuilder::check_workflow`] and
442/// [`crate::Engine::check_workflow`] so the two cannot answer differently, and
443/// run against the crate's *real* `TemplateCompiler` rather than a host's
444/// reconstruction of one.
445///
446/// `workflow.tasks` is already flattened, so iterating it covers members of
447/// task groups without any extra traversal.
448pub(crate) fn check_against_registry(
449    workflow: &Workflow,
450    registry: &std::collections::HashMap<String, BoxedFunctionHandler>,
451    template_compiler: &TemplateCompiler,
452    secrets: &Secrets,
453) -> Vec<WorkflowIssue> {
454    let mut issues = check_secrets(workflow, secrets);
455    // All three template-key findings, including the two `build()` does not
456    // refuse — this surface is where a host looks before activating a
457    // definition, and the migration audit is the point of reporting them.
458    issues.extend(check_template_keys(workflow));
459    check_unguarded_validation(workflow, &mut issues);
460    check_group_continue_on_error(workflow, &mut issues);
461
462    for task in &workflow.tasks {
463        let name = task.function.function_name();
464
465        if !can_dispatch_in(registry, name) {
466            // Distinguish the two reasons, because the fixes differ: a
467            // `RequiresHandler` built-in is a real name awaiting a
468            // registration, while anything else is likely a typo.
469            let (code, message) = match builtin_function_kind(name) {
470                Some(BuiltinKind::RequiresHandler) => (
471                    IssueCode::MissingHandler,
472                    format!(
473                        "'{name}' ships as a config schema only — register a handler under \
474                         that name, or this workflow will build cleanly and fail every message"
475                    ),
476                ),
477                _ => (
478                    IssueCode::UnknownFunction,
479                    format!("no handler is registered for '{name}', and it is not a built-in"),
480                ),
481            };
482            issues.push(WorkflowIssue {
483                code,
484                message,
485                path: Some("function.name".to_string()),
486                task_id: Some(task.id.clone()),
487            });
488            continue;
489        }
490
491        // Only `Custom` inputs are still raw at this point: the built-in
492        // variants were typed by serde when the workflow parsed.
493        let FunctionConfig::Custom { name, input, .. } = &task.function else {
494            continue;
495        };
496        let Some(handler) = registry.get(name) else {
497            continue;
498        };
499
500        let mut parsed = match handler.parse_input_box(input) {
501            Ok(parsed) => parsed,
502            Err(err) => {
503                issues.push(WorkflowIssue {
504                    code: IssueCode::InputParse,
505                    message: format!("input does not match the handler's Input type: {err}"),
506                    path: Some("function.input".to_string()),
507                    task_id: Some(task.id.clone()),
508                });
509                continue;
510            }
511        };
512
513        if let Err(err) = handler.compile_input_box(&mut *parsed, template_compiler) {
514            issues.push(WorkflowIssue {
515                code: IssueCode::TemplateCompile,
516                message: format!("a template field does not compile: {err}"),
517                path: Some("function.input".to_string()),
518                task_id: Some(task.id.clone()),
519            });
520        }
521    }
522
523    issues
524}
525
526/// Report a `validation` task whose failure cannot stop anything.
527///
528/// A failing rule returns `400`, and the engine treats `4xx` as "warn and carry
529/// on" — `continue_on_error` governs `5xx` and `Err` only. So a `validation`
530/// followed by unguarded tasks is a *decorative* assertion: it records an error
531/// on the message and the pipeline proceeds exactly as if it had passed. That is
532/// how a CSRF check ships that never rejects anything.
533///
534/// **Informational.** Reported here, never by
535/// [`EngineBuilder::build`](crate::EngineBuilder::build) — the workflow is
536/// legal, and plenty of them validate for reporting rather than for control
537/// flow.
538///
539/// Deliberately blunt: it asks "is there *any* gate after this assertion?", not
540/// "is the gate correct". A guard's condition can read anything — a
541/// host-configured error-context path included — so deciding whether an
542/// arbitrary expression actually tests the failure is not statically knowable.
543/// Anything that could plausibly gate silences it, which keeps false positives
544/// near zero at the cost of missing an author who wrote an unrelated condition.
545fn check_unguarded_validation(workflow: &Workflow, issues: &mut Vec<WorkflowIssue>) {
546    /// A condition that is not the literal `true` default might gate on the
547    /// failure, so it counts as a guard.
548    fn gates(condition: &Value) -> bool {
549        condition != &Value::Bool(true)
550    }
551
552    for (i, task) in workflow.tasks.iter().enumerate() {
553        if !matches!(task.function, FunctionConfig::Validation { .. }) {
554            continue;
555        }
556        // The task already stops the workflow on its own account.
557        if task.terminal || task.halt_on != HaltOn::Never {
558            continue;
559        }
560        // Nothing follows it *in this workflow*, so its `400` survives in
561        // `metadata.progress` for the next workflow to gate on.
562        let Some(following) = workflow.tasks.get(i + 1..).filter(|t| !t.is_empty()) else {
563            continue;
564        };
565        let guarded = following.iter().any(|next| {
566            gates(&next.condition)
567                || matches!(next.function, FunctionConfig::Filter { .. })
568                || next.group_starts.iter().any(|g| gates(&g.condition))
569        });
570        if guarded {
571            continue;
572        }
573
574        issues.push(WorkflowIssue {
575            code: IssueCode::UnguardedValidation,
576            message: format!(
577                "a failing rule here records status 400 and task '{}' still runs — \
578                 `continue_on_error` covers 5xx and Err only. Add `\"halt_on\": \"failure\"` \
579                 to stop the workflow, or gate what follows on \
580                 `metadata.progress.status_code`",
581                following[0].id
582            ),
583            path: Some("halt_on".to_string()),
584            task_id: Some(task.id.clone()),
585        });
586    }
587}
588
589/// Report a task group carrying `continue_on_error`, which the engine drops.
590///
591/// The key is real on a [`Task`](crate::Task) and on a
592/// [`Workflow`](crate::Workflow), and that is exactly what makes a group the
593/// one place it looks like it should work and does not: `TaskGroup` has no such
594/// field, so the parser discards it like any other unknown key and the
595/// definition looks correct in review.
596///
597/// **Informational.** The sibling mistake — `halt_on` on a group — is refused
598/// at parse time instead, because it was a new key with no installed base.
599/// This one is old enough that a host may already carry it on group nodes, and
600/// refusing it would fail `Engine::build`, which aborts every workflow in that
601/// build. Honouring it is not the answer either: propagating it to the member
602/// tasks would change the error semantics of definitions that load today,
603/// silently.
604///
605/// Only a literal `true` is reported. `continue_on_error: false` on a group
606/// already describes what happens, so nothing the author wanted is defeated and
607/// reporting it would be noise.
608fn check_group_continue_on_error(workflow: &Workflow, issues: &mut Vec<WorkflowIssue>) {
609    // Each group is recorded once, on the task that opens its span, so this
610    // visits every group exactly once — outermost first where several open
611    // together.
612    for task in &workflow.tasks {
613        for group in task.group_starts.iter().filter(|g| g.continue_on_error) {
614            issues.push(WorkflowIssue {
615                code: IssueCode::GroupContinueOnError,
616                message: format!(
617                    "group '{}' carries continue_on_error, which the engine does not \
618                     honour — error handling is per task and per workflow, and a group \
619                     only gates a span. Put it on the tasks inside the group, or on the \
620                     workflow",
621                    group.id
622                ),
623                path: Some("continue_on_error".to_string()),
624                task_id: Some(group.id.clone()),
625            });
626        }
627    }
628}
629
630/// Where an expression's result ends up — what decides whether it may read a
631/// secret.
632#[derive(Clone, Copy, PartialEq, Eq)]
633enum Sink {
634    /// Collapses to a bool the engine acts on: workflow, group and task
635    /// conditions, `validation` rules, `filter`. Nothing of the value survives.
636    Bool,
637    /// Handed to a handler: a custom handler's `Template` fields and every
638    /// `http_call` / `enrich` / `publish_kafka` parameter. What happens next is
639    /// the handler's business.
640    Handler,
641    /// A custom task's whole raw input, handed to the handler untyped. The
642    /// same rule as [`Sink::Handler`]; it differs only in that the document
643    /// is not one expression, so an issue carries the deep path to the
644    /// reference rather than the field.
645    Input,
646    /// Written to the message or emitted to a log by the engine itself: a
647    /// `map` mapping's value *and* its destination, a `validation` rule's
648    /// `message`, `log` message and fields, and the `source` / `target` /
649    /// `root_element` a `parse_*` or `publish_*` names. Recorded by
650    /// construction — a destination lands in `Change.path` and the audit trail.
651    Message,
652}
653
654impl Sink {
655    /// Whether the engine itself records this expression's result. Such an
656    /// expression may not read a secret *at all* — the store exists so a value
657    /// is never recorded, and there is no static line between a copy and a
658    /// derived value.
659    ///
660    /// The four variants are a map of where an expression's result goes;
661    /// [`Sink::Bool`] and [`Sink::Handler`] answer this question the same way
662    /// today and are kept apart because the reasons differ — nothing survives
663    /// versus the handler owns what happens next.
664    fn records(self) -> bool {
665        matches!(self, Self::Message)
666    }
667
668    /// Whether an issue points at the reference's own deep path rather than at
669    /// the field. True only where the field is not itself a single expression,
670    /// so the field name alone would not locate the reference.
671    fn points_at_reference(self) -> bool {
672        matches!(self, Self::Input)
673    }
674}
675
676/// Check every expression in `workflow` for `{"secret": …}` references.
677///
678/// One implementation for two callers: `Engine::build` refuses a workflow that
679/// produces any issue here, and `check_workflow` reports the same issues, so the
680/// two cannot disagree about what is loadable.
681///
682/// A reference is any single-key object whose key is the reserved operator
683/// name — datalogic compiles exactly that shape as an operator call in
684/// templating mode, at any depth. A string (or one-element array) argument is
685/// a literal name and is checked against `secrets`; anything else is dynamic
686/// and only fails at evaluation. Both forms are refused in a
687/// [`Sink::Message`] expression.
688pub(crate) fn check_secrets(workflow: &Workflow, secrets: &Secrets) -> Vec<WorkflowIssue> {
689    let mut issues = Vec::new();
690    for_each_expression(workflow, &mut |value, field, task_id, sink| {
691        check_expression(value, field, task_id, sink, secrets, &mut issues);
692    });
693    issues
694}
695
696/// A map-valued config field's keys, in name order.
697///
698/// The maps `for_each_expression` walks (`log.fields`, `http_call.headers`) are
699/// `HashMap`s, so their iteration order is arbitrary — but issue order is what a
700/// host logs, diffs, or asserts on, and what `refuse_authoring_issues` joins into a
701/// `build()` error message. Sorting is what makes that order reproducible.
702fn names_in_order<V>(map: &HashMap<String, V>) -> Vec<&String> {
703    let mut names: Vec<&String> = map.keys().collect();
704    names.sort_unstable();
705    names
706}
707
708/// Visit every JSONLogic expression in `workflow`, with where it lives and
709/// where its result goes.
710///
711/// The single enumeration of "which config fields are expressions". Both
712/// [`check_secrets`] and [`check_template_keys`] walk it, so a parameter added
713/// to a built-in cannot be checked by one and silently skipped by the other.
714fn for_each_expression(
715    workflow: &Workflow,
716    check: &mut impl FnMut(&Value, &str, Option<&str>, Sink),
717) {
718    check(&workflow.condition, "condition", None, Sink::Bool);
719
720    for task in &workflow.tasks {
721        // Groups opening at this task, outermost first — compiled alongside
722        // the task condition, so checked alongside it.
723        for group in &task.group_starts {
724            let id = Some(group.id.as_str());
725            check(&group.condition, "condition", id, Sink::Bool);
726        }
727
728        let id = Some(task.id.as_str());
729        check(&task.condition, "condition", id, Sink::Bool);
730
731        match &task.function {
732            FunctionConfig::Map { input, .. } => {
733                for (i, mapping) in input.mappings.iter().enumerate() {
734                    let field = format!("function.input.mappings[{i}].logic");
735                    check(&mapping.logic, &field, id, Sink::Message);
736                    // The destination is itself recorded — in `Change.path` and
737                    // on the audit trail — so it may not read a secret either.
738                    let field = format!("function.input.mappings[{i}].path");
739                    check(mapping.path.as_json(), &field, id, Sink::Message);
740                }
741            }
742            FunctionConfig::Validation { input, .. } => {
743                for (i, rule) in input.rules.iter().enumerate() {
744                    let field = format!("function.input.rules[{i}].logic");
745                    check(&rule.logic, &field, id, Sink::Bool);
746                    // The message is `Sink::Message`, not `Sink::Bool`: it is
747                    // recorded in `Message::errors`, which is serialized. A
748                    // rule may *test* a secret; it may not *report* one.
749                    let field = format!("function.input.rules[{i}].message");
750                    check(rule.message.as_json(), &field, id, Sink::Message);
751                }
752            }
753            FunctionConfig::Filter { input, .. } => {
754                check(&input.condition, "function.input.condition", id, Sink::Bool);
755            }
756            FunctionConfig::Log { input, .. } => {
757                check(&input.message, "function.input.message", id, Sink::Message);
758                for name in names_in_order(&input.fields) {
759                    let field = format!("function.input.fields.{name}");
760                    check(&input.fields[name], &field, id, Sink::Message);
761                }
762            }
763            FunctionConfig::HttpCall { input, .. } => {
764                // Everything here is handed to the host's handler, so a secret
765                // is allowed — an `Authorization` header reading
766                // `{"secret": "api_token"}` is the reason headers became
767                // expressions at all. What the handler does with it from there
768                // is the handler's business.
769                check(
770                    input.connector.as_json(),
771                    "function.input.connector",
772                    id,
773                    Sink::Handler,
774                );
775                check(
776                    input.timeout_ms.as_json(),
777                    "function.input.timeout_ms",
778                    id,
779                    Sink::Handler,
780                );
781                for name in names_in_order(&input.headers) {
782                    let field = format!("function.input.headers.{name}");
783                    check(input.headers[name].as_json(), &field, id, Sink::Handler);
784                }
785                for (name, template) in [
786                    ("path", &input.path),
787                    ("body", &input.body),
788                    ("body_format", &input.body_format),
789                    ("response_path", &input.response_path),
790                    ("response_format", &input.response_format),
791                ] {
792                    if let Some(t) = template {
793                        let field = format!("function.input.{name}");
794                        check(t.as_json(), &field, id, Sink::Handler);
795                    }
796                }
797            }
798            FunctionConfig::Enrich { input, .. } => {
799                for (name, template) in [
800                    ("connector", &input.connector),
801                    ("merge_path", &input.merge_path),
802                    ("timeout_ms", &input.timeout_ms),
803                ] {
804                    let field = format!("function.input.{name}");
805                    check(template.as_json(), &field, id, Sink::Handler);
806                }
807                if let Some(t) = &input.path {
808                    check(t.as_json(), "function.input.path", id, Sink::Handler);
809                }
810            }
811            FunctionConfig::PublishKafka { input, .. } => {
812                for (name, template) in [("connector", &input.connector), ("topic", &input.topic)] {
813                    let field = format!("function.input.{name}");
814                    check(template.as_json(), &field, id, Sink::Handler);
815                }
816                for (name, template) in [("key", &input.key), ("value", &input.value)] {
817                    if let Some(t) = template {
818                        let field = format!("function.input.{name}");
819                        check(t.as_json(), &field, id, Sink::Handler);
820                    }
821                }
822            }
823            FunctionConfig::Custom { input, .. } => {
824                check(input, "function.input", id, Sink::Input);
825            }
826            // `Sink::Message`, not `Sink::Handler`: these expressions name
827            // where the engine itself writes, and the destination is recorded
828            // in `Change.path` and the audit trail. `root_element` goes further
829            // — it is written into the serialized document that lands in
830            // `data.{target}`.
831            FunctionConfig::ParseJson { input, .. } | FunctionConfig::ParseXml { input, .. } => {
832                check(
833                    input.source.as_json(),
834                    "function.input.source",
835                    id,
836                    Sink::Message,
837                );
838                check(
839                    input.target.as_json(),
840                    "function.input.target",
841                    id,
842                    Sink::Message,
843                );
844            }
845            FunctionConfig::PublishJson { input, .. }
846            | FunctionConfig::PublishXml { input, .. } => {
847                check(
848                    input.source.as_json(),
849                    "function.input.source",
850                    id,
851                    Sink::Message,
852                );
853                check(
854                    input.target.as_json(),
855                    "function.input.target",
856                    id,
857                    Sink::Message,
858                );
859                check(
860                    input.root_element.as_json(),
861                    "function.input.root_element",
862                    id,
863                    Sink::Message,
864                );
865            }
866        }
867    }
868}
869
870/// Check every expression in `workflow` for object keys the template-key
871/// escape makes newly significant.
872///
873/// Three findings, and only the first is fatal:
874///
875/// - [`IssueCode::DuplicateTemplateKey`] — two keys in one object that collapse
876///   to the same name after the escape is stripped. Always a bug, so
877///   `Engine::build` refuses it.
878/// - [`IssueCode::EscapedTemplateKey`] — a `$`-prefixed key, reported so a host
879///   migrating to 3.9 can audit every place the escape changed what a template
880///   emits. Informational: after migration these are deliberate.
881///
882/// A third check — flagging a single-key object whose key names no live
883/// operator — was designed and then dropped, because it cannot be made
884/// precise. In templating mode an unrecognised single key is *not* inert: it
885/// evaluates its argument and emits a structured object, so
886/// `{"result": {"var": "x"}}` yields `{"result": 5}`. That is the ordinary
887/// single-key output template and the most common shape in a `map` mapping,
888/// indistinguishable from a misspelled `lenght`. Flagging it would fire on
889/// almost every correct workflow.
890pub(crate) fn check_template_keys(workflow: &Workflow) -> Vec<WorkflowIssue> {
891    let mut issues = Vec::new();
892    for_each_expression(workflow, &mut |value, field, task_id, sink| {
893        // A custom task's `input` is a config document, not an expression: only
894        // the `Template` fields inside it are JSONLogic, and which those are is
895        // the handler's business. Treating the whole document as a template
896        // would flag ordinary config keys — and `DuplicateTemplateKey` is
897        // fatal, so a false positive there refuses a valid workflow.
898        if sink != Sink::Input {
899            walk_template_keys(value, field, task_id, &mut issues);
900        }
901    });
902    issues
903}
904
905/// The [`Severity::Rejected`] subset of [`check_template_keys`] — what
906/// `Engine::build` refuses.
907///
908/// Filtered on the severity rather than on the one code that has it today, so a
909/// template-key code added later is refused exactly when it is classified as a
910/// rejection. A classification nothing reads is one that drifts.
911pub(crate) fn refusing_template_key_issues(workflow: &Workflow) -> Vec<WorkflowIssue> {
912    let mut issues = check_template_keys(workflow);
913    issues.retain(|i| i.severity() == Severity::Rejected);
914    issues
915}
916
917/// Recursive half of [`check_template_keys`].
918fn walk_template_keys(
919    value: &Value,
920    path: &str,
921    task_id: Option<&str>,
922    issues: &mut Vec<WorkflowIssue>,
923) {
924    match value {
925        Value::Array(items) => {
926            for (i, item) in items.iter().enumerate() {
927                walk_template_keys(item, &format!("{path}[{i}]"), task_id, issues);
928            }
929        }
930        Value::Object(map) => {
931            report_escaped_and_duplicate_keys(map, path, task_id, issues);
932            for (key, child) in map {
933                walk_template_keys(child, &format!("{path}.{key}"), task_id, issues);
934            }
935        }
936        _ => {}
937    }
938}
939
940/// The two key-level findings for one template object: escaped keys, and keys
941/// that collide once the escape is stripped.
942fn report_escaped_and_duplicate_keys(
943    map: &serde_json::Map<String, Value>,
944    path: &str,
945    task_id: Option<&str>,
946    issues: &mut Vec<WorkflowIssue>,
947) {
948    let mut emitted: HashMap<String, &str> = HashMap::new();
949    for key in map.keys() {
950        if let Some(stripped) = key.strip_prefix(TEMPLATE_KEY_ESCAPE) {
951            issues.push(
952                WorkflowIssue::at(
953                    IssueCode::EscapedTemplateKey,
954                    format!("{path}.{key}"),
955                    format!(
956                        "'{key}' is emitted as '{stripped}' — one \
957                         '{TEMPLATE_KEY_ESCAPE}' is stripped from every template key. Double it \
958                         to '{TEMPLATE_KEY_ESCAPE}{key}' to emit '{key}' itself"
959                    ),
960                )
961                .with_step(task_id),
962            );
963        }
964        // What this key actually emits, which is what can collide.
965        let out = key
966            .strip_prefix(TEMPLATE_KEY_ESCAPE)
967            .unwrap_or(key)
968            .to_string();
969        if let Some(other) = emitted.insert(out.clone(), key) {
970            issues.push(
971                WorkflowIssue::at(
972                    IssueCode::DuplicateTemplateKey,
973                    format!("{path}.{key}"),
974                    format!(
975                        "'{other}' and '{key}' both emit the key '{out}', so this object would \
976                         carry it twice — later reads see only the first while serialization \
977                         emits both"
978                    ),
979                )
980                .with_step(task_id),
981            );
982        }
983    }
984}
985
986/// One expression's worth of [`check_secrets`].
987fn check_expression(
988    value: &Value,
989    field: &str,
990    task_id: Option<&str>,
991    sink: Sink,
992    secrets: &Secrets,
993    issues: &mut Vec<WorkflowIssue>,
994) {
995    let mut refs = Vec::new();
996    collect_secret_refs(value, field, &mut refs);
997    if refs.is_empty() {
998        return;
999    }
1000    if sink.records() {
1001        issues.push(
1002            WorkflowIssue::at(
1003                IssueCode::SecretInMessageWrite,
1004                field,
1005                "reads a secret, and the engine records this expression's result — \
1006                 compute derived values in a custom handler instead",
1007            )
1008            .with_step(task_id),
1009        );
1010        return;
1011    }
1012    for (path, key) in &refs {
1013        let Some(key) = key else { continue };
1014        if secrets.get(key).is_some() {
1015            continue;
1016        }
1017        let at = if sink.points_at_reference() {
1018            path.as_str()
1019        } else {
1020            field
1021        };
1022        issues.push(
1023            WorkflowIssue::at(
1024                IssueCode::UnknownSecret,
1025                at,
1026                format!("secret '{key}' is not declared on the engine"),
1027            )
1028            .with_step(task_id),
1029        );
1030    }
1031}
1032
1033/// Collect every `{"secret": …}` reference under `value` as
1034/// `(path, literal name)` — `None` for a dynamic name. Descends into the
1035/// argument too, since a dynamic name may itself contain a reference.
1036fn collect_secret_refs<'v>(value: &'v Value, path: &str, out: &mut Vec<(String, Option<&'v str>)>) {
1037    match value {
1038        Value::Object(map) => {
1039            if map.len() == 1 {
1040                if let Some(arg) = map.get(SECRET_OPERATOR) {
1041                    let literal = match arg {
1042                        Value::String(s) => Some(s.as_str()),
1043                        Value::Array(items) if items.len() == 1 => items[0].as_str(),
1044                        _ => None,
1045                    };
1046                    out.push((path.to_string(), literal));
1047                    collect_secret_refs(arg, &format!("{path}.{SECRET_OPERATOR}"), out);
1048                    return;
1049                }
1050            }
1051            for (key, child) in map {
1052                collect_secret_refs(child, &format!("{path}.{key}"), out);
1053            }
1054        }
1055        Value::Array(items) => {
1056            for (i, child) in items.iter().enumerate() {
1057                collect_secret_refs(child, &format!("{path}[{i}]"), out);
1058            }
1059        }
1060        _ => {}
1061    }
1062}
1063
1064/// Stage 1: every semantic violation, with authored coordinates.
1065fn check_shape(json: &Value) -> Vec<WorkflowIssue> {
1066    let mut issues = Vec::new();
1067
1068    if non_empty_str(json.get("id")).is_none() {
1069        issues.push(WorkflowIssue::at(
1070            IssueCode::EmptyWorkflowId,
1071            "id",
1072            "workflow id must be a non-empty string",
1073        ));
1074    }
1075    if non_empty_str(json.get("name")).is_none() {
1076        issues.push(WorkflowIssue::at(
1077            IssueCode::EmptyWorkflowName,
1078            "name",
1079            "workflow name must be a non-empty string",
1080        ));
1081    }
1082
1083    match json.get("tasks").and_then(Value::as_array) {
1084        Some(tasks) if !tasks.is_empty() => {}
1085        _ => issues.push(WorkflowIssue::at(
1086            IssueCode::NoTasks,
1087            "tasks",
1088            "workflow must have at least one task",
1089        )),
1090    }
1091
1092    check_steps(json.get("tasks").unwrap_or(&Value::Null), &mut issues);
1093
1094    if let Some(loop_config) = json.get("loop") {
1095        check_loop(loop_config, &mut issues);
1096    }
1097
1098    issues
1099}
1100
1101/// Walk the authored step tree, checking each node and the id namespace.
1102///
1103/// Built on [`walk_authored_steps`], so the group test, the traversal order and
1104/// the depth cap have exactly one definition shared with the parser.
1105fn check_steps(tasks: &Value, issues: &mut Vec<WorkflowIssue>) {
1106    // Step id -> the path that first claimed it.
1107    let mut seen: HashMap<&str, String> = HashMap::new();
1108
1109    for step in walk_authored_steps(tasks) {
1110        let id = non_empty_str(step.node.get("id"));
1111
1112        match id {
1113            None => issues.push(
1114                WorkflowIssue::at(
1115                    IssueCode::MissingStepId,
1116                    format!("{}.id", step.path),
1117                    "every step needs a non-empty id",
1118                )
1119                .with_step(None),
1120            ),
1121            Some(id) => {
1122                if let Some(first) = seen.get(id) {
1123                    issues.push(
1124                        WorkflowIssue::at(
1125                            IssueCode::DuplicateStepId,
1126                            format!("{}.id", step.path),
1127                            format!(
1128                                "step id '{id}' is already used at {first} — task groups \
1129                                 share the task id namespace"
1130                            ),
1131                        )
1132                        .with_step(Some(id)),
1133                    );
1134                } else {
1135                    seen.insert(id, step.path.clone());
1136                }
1137            }
1138        }
1139
1140        if let Some(terminal) = step.node.get("terminal") {
1141            if !terminal.is_boolean() {
1142                // `"on_failure"` is the natural wrong guess once `halt_on`
1143                // exists — answer it rather than reporting a bare type error.
1144                let message = if terminal.as_str() == Some("on_failure") {
1145                    "terminal must be a boolean — for \"halt if this task failed\" \
1146                     use `\"halt_on\": \"failure\"`, which is the outcome axis"
1147                } else {
1148                    "terminal must be a boolean"
1149                };
1150                issues.push(
1151                    WorkflowIssue::at(
1152                        IssueCode::InvalidTerminal,
1153                        format!("{}.terminal", step.path),
1154                        message,
1155                    )
1156                    .with_step(id),
1157                );
1158            }
1159        }
1160
1161        // Asking serde rather than keeping a list of accepted spellings, so a
1162        // future `HaltOn` variant needs no edit here.
1163        if let Some(halt_on) = step.node.get("halt_on") {
1164            let problem = if matches!(step.kind, StepKind::Group) {
1165                Some(
1166                    "halt_on is a per-task outcome rule and a group has no outcome \
1167                     of its own — put it on the task that can fail",
1168                )
1169            } else {
1170                HaltOn::deserialize(halt_on)
1171                    .err()
1172                    .map(|_| "halt_on must be one of \"never\", \"failure\"")
1173            };
1174            if let Some(message) = problem {
1175                issues.push(
1176                    WorkflowIssue::at(
1177                        IssueCode::InvalidHaltOn,
1178                        format!("{}.halt_on", step.path),
1179                        message,
1180                    )
1181                    .with_step(id),
1182                );
1183            }
1184        }
1185
1186        match step.kind {
1187            StepKind::Leaf => check_function(&step.path, step.node, id, issues),
1188            StepKind::Group => {
1189                // The parser rejects a group whose `tasks` is not a non-empty
1190                // array; the walker reports the node so we can say which.
1191                let has_members = step
1192                    .node
1193                    .get("tasks")
1194                    .and_then(Value::as_array)
1195                    .is_some_and(|members| !members.is_empty());
1196                if !has_members {
1197                    issues.push(
1198                        WorkflowIssue::at(
1199                            IssueCode::EmptyGroup,
1200                            format!("{}.tasks", step.path),
1201                            "a task group's tasks must be a non-empty array — \
1202                             an empty group can only be a mistake",
1203                        )
1204                        .with_step(id),
1205                    );
1206                }
1207            }
1208            StepKind::TooDeep => issues.push(
1209                WorkflowIssue::at(
1210                    IssueCode::GroupTooDeep,
1211                    step.path.clone(),
1212                    format!(
1213                        "task groups nested deeper than {} levels",
1214                        crate::engine::steps::MAX_GROUP_DEPTH
1215                    ),
1216                )
1217                .with_step(id),
1218            ),
1219        }
1220    }
1221}
1222
1223/// A leaf must carry a `function` object with a non-empty `name`.
1224fn check_function(path: &str, node: &Value, id: Option<&str>, issues: &mut Vec<WorkflowIssue>) {
1225    let Some(function) = node.get("function") else {
1226        issues.push(
1227            WorkflowIssue::at(
1228                IssueCode::MissingFunction,
1229                format!("{path}.function"),
1230                "a task needs a function — an element with neither `function` nor \
1231                 `tasks` is neither a task nor a group",
1232            )
1233            .with_step(id),
1234        );
1235        return;
1236    };
1237
1238    if !function.is_object() || non_empty_str(function.get("name")).is_none() {
1239        issues.push(
1240            WorkflowIssue::at(
1241                IssueCode::InvalidFunctionName,
1242                format!("{path}.function.name"),
1243                "function must be an object with a non-empty name",
1244            )
1245            .with_step(id),
1246        );
1247    }
1248}
1249
1250/// The three `LoopConfig::validate` rules, against the authored JSON.
1251fn check_loop(config: &Value, issues: &mut Vec<WorkflowIssue>) {
1252    // Absent fields take their serde defaults, which are valid; only a present
1253    // field can be wrong here. A non-integer is a *type* error and belongs to
1254    // stage 2, so it is deliberately not reported twice.
1255    if let Some(increment) = config.get("increment").and_then(Value::as_i64) {
1256        if increment < 1 {
1257            issues.push(WorkflowIssue::at(
1258                IssueCode::LoopIncrementTooSmall,
1259                "loop.increment",
1260                format!(
1261                    "loop increment must be >= 1, got {increment} \
1262                     (a non-advancing counter would never reach max)"
1263                ),
1264            ));
1265        }
1266    }
1267
1268    let init = config.get("init").and_then(Value::as_i64).unwrap_or(0);
1269    if let Some(max) = config.get("max").and_then(Value::as_i64) {
1270        if max <= init {
1271            issues.push(WorkflowIssue::at(
1272                IssueCode::LoopBoundEmpty,
1273                "loop.max",
1274                format!(
1275                    "loop max ({max}) must be greater than init ({init}) — \
1276                     the bound is half-open, so this could never run a sweep"
1277                ),
1278            ));
1279        }
1280    }
1281
1282    if let Some(counter) = config.get("counter") {
1283        if let Some(counter) = counter.as_str() {
1284            if counter.is_empty() || counter.split('.').any(str::is_empty) {
1285                issues.push(WorkflowIssue::at(
1286                    IssueCode::LoopCounterInvalid,
1287                    "loop.counter",
1288                    format!(
1289                        "loop counter must be a non-empty temp_data field path, got {counter:?}"
1290                    ),
1291                ));
1292            }
1293        }
1294    }
1295}
1296
1297/// The value at `field` as a non-empty string, if it is one.
1298fn non_empty_str(field: Option<&Value>) -> Option<&str> {
1299    field.and_then(Value::as_str).filter(|s| !s.is_empty())
1300}
1301
1302#[cfg(test)]
1303mod tests {
1304    use super::*;
1305    use std::collections::HashSet;
1306
1307    /// Every [`IssueCode`], for the checks below that must cover the whole
1308    /// vocabulary rather than a hand-picked subset. Kept complete by
1309    /// [`ordinal`] — see [`all_codes_lists_every_variant`].
1310    const ALL_CODES: [IssueCode; 27] = [
1311        IssueCode::EmptyWorkflowId,
1312        IssueCode::EmptyWorkflowName,
1313        IssueCode::NoTasks,
1314        IssueCode::MissingStepId,
1315        IssueCode::DuplicateStepId,
1316        IssueCode::EmptyGroup,
1317        IssueCode::GroupTooDeep,
1318        IssueCode::MissingFunction,
1319        IssueCode::InvalidFunctionName,
1320        IssueCode::InvalidTerminal,
1321        IssueCode::UnguardedValidation,
1322        IssueCode::InvalidHaltOn,
1323        IssueCode::GroupContinueOnError,
1324        IssueCode::LoopIncrementTooSmall,
1325        IssueCode::LoopBoundEmpty,
1326        IssueCode::LoopCounterInvalid,
1327        IssueCode::UnknownFunction,
1328        IssueCode::MissingHandler,
1329        IssueCode::InputParse,
1330        IssueCode::TemplateCompile,
1331        IssueCode::UnknownSecret,
1332        IssueCode::SecretInMessageWrite,
1333        IssueCode::InvalidSecretStore,
1334        IssueCode::DuplicateTemplateKey,
1335        IssueCode::EscapedTemplateKey,
1336        IssueCode::ParseFailed,
1337        IssueCode::ValidateFailed,
1338    ];
1339
1340    /// A distinct index per variant, with **no wildcard arm**. Adding an
1341    /// [`IssueCode`] stops this compiling until it is given one.
1342    fn ordinal(code: IssueCode) -> usize {
1343        match code {
1344            IssueCode::EmptyWorkflowId => 0,
1345            IssueCode::EmptyWorkflowName => 1,
1346            IssueCode::NoTasks => 2,
1347            IssueCode::MissingStepId => 3,
1348            IssueCode::DuplicateStepId => 4,
1349            IssueCode::EmptyGroup => 5,
1350            IssueCode::GroupTooDeep => 6,
1351            IssueCode::MissingFunction => 7,
1352            IssueCode::InvalidFunctionName => 8,
1353            IssueCode::InvalidTerminal => 9,
1354            IssueCode::UnguardedValidation => 10,
1355            IssueCode::InvalidHaltOn => 11,
1356            IssueCode::GroupContinueOnError => 12,
1357            IssueCode::LoopIncrementTooSmall => 13,
1358            IssueCode::LoopBoundEmpty => 14,
1359            IssueCode::LoopCounterInvalid => 15,
1360            IssueCode::UnknownFunction => 16,
1361            IssueCode::MissingHandler => 17,
1362            IssueCode::InputParse => 18,
1363            IssueCode::TemplateCompile => 19,
1364            IssueCode::UnknownSecret => 20,
1365            IssueCode::SecretInMessageWrite => 21,
1366            IssueCode::InvalidSecretStore => 22,
1367            IssueCode::DuplicateTemplateKey => 23,
1368            IssueCode::EscapedTemplateKey => 24,
1369            IssueCode::ParseFailed => 25,
1370            IssueCode::ValidateFailed => 26,
1371        }
1372    }
1373
1374    /// `ALL_CODES` is the whole vocabulary, and stays that way.
1375    ///
1376    /// Two halves, and both are needed. A new variant fails to compile at
1377    /// [`ordinal`]; giving it an index there but forgetting `ALL_CODES` breaks
1378    /// the bijection below. Without this the fix for a list that could only be
1379    /// wrong in one direction would itself ship such a list.
1380    #[test]
1381    fn all_codes_lists_every_variant() {
1382        let indices: HashSet<usize> = ALL_CODES.iter().copied().map(ordinal).collect();
1383        assert_eq!(
1384            indices,
1385            (0..ALL_CODES.len()).collect::<HashSet<_>>(),
1386            "ALL_CODES is not a bijection onto the variants `ordinal` knows"
1387        );
1388    }
1389
1390    /// The severity table, stated as sets rather than read off the match.
1391    ///
1392    /// This is the tripwire the issue asks for: a code added in a later minor
1393    /// is classified by the compiler-forced match in [`IssueCode::severity`],
1394    /// but *promoting* one to advisory has to be a deliberate edit here.
1395    #[test]
1396    fn the_advisory_and_defect_sets_are_exactly_these() {
1397        let by = |want: Severity| -> HashSet<&'static str> {
1398            ALL_CODES
1399                .iter()
1400                .filter(|c| c.severity() == want)
1401                .map(IssueCode::as_str)
1402                .collect()
1403        };
1404
1405        assert_eq!(
1406            by(Severity::Advisory),
1407            HashSet::from([
1408                "UNGUARDED_VALIDATION",
1409                "GROUP_CONTINUE_ON_ERROR",
1410                "ESCAPED_TEMPLATE_KEY",
1411            ]),
1412            "the advisory set is what a host screens on — changing it is a breaking change"
1413        );
1414        assert_eq!(
1415            by(Severity::Defect),
1416            HashSet::from(["MISSING_HANDLER"]),
1417            "only the config-only integrations build clean and then fail every message"
1418        );
1419        assert_eq!(
1420            by(Severity::Rejected).len(),
1421            ALL_CODES.len() - 4,
1422            "every remaining code is a rejection"
1423        );
1424    }
1425
1426    /// The string forms are an API surface: unique, stable, and shouted.
1427    #[test]
1428    fn issue_codes_have_distinct_stable_strings() {
1429        let mut seen = HashSet::new();
1430        for code in ALL_CODES {
1431            assert!(seen.insert(code.as_str()), "duplicate string for {code:?}");
1432            assert_eq!(code.to_string(), code.as_str(), "Display matches as_str");
1433            assert!(
1434                code.as_str()
1435                    .chars()
1436                    .all(|c| c.is_ascii_uppercase() || c == '_'),
1437                "{code:?} is not SCREAMING_SNAKE"
1438            );
1439        }
1440    }
1441
1442    #[test]
1443    fn severities_have_distinct_stable_strings() {
1444        let all = [Severity::Rejected, Severity::Defect, Severity::Advisory];
1445        let mut seen = HashSet::new();
1446        for severity in all {
1447            assert!(seen.insert(severity.as_str()), "duplicate {severity:?}");
1448            assert_eq!(severity.to_string(), severity.as_str());
1449            assert!(
1450                severity
1451                    .as_str()
1452                    .chars()
1453                    .all(|c| c.is_ascii_uppercase() || c == '_'),
1454                "{severity:?} is not SCREAMING_SNAKE"
1455            );
1456        }
1457    }
1458
1459    /// The convenience method is the code's answer, not a second opinion.
1460    #[test]
1461    fn a_workflow_issue_reports_its_codes_severity() {
1462        for code in ALL_CODES {
1463            let issue = WorkflowIssue::at(code, "tasks[0]", "message");
1464            assert_eq!(issue.severity(), code.severity(), "{code:?}");
1465        }
1466    }
1467}